Sitelet https://web.archive.org/web/20201113050253/https://github.com/ethereum/solidity/pull/8949
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Sol->Yul] Add Arity struct (refactor) #8949

Merged
merged 2 commits into from May 19, 2020
Merged

Conversation

@cameel
Copy link
Member

@cameel cameel commented May 14, 2020

Based on #8948 which needs to be merged first.

Refactoring changes extracted from #8943 (and before that from #8797). Does not affect functionality.
Related to #6788 and #8485.

@cameel cameel force-pushed the sol-yul-refactor-add-arity branch from 6b76e97 to e31d66c May 15, 2020
@cameel cameel changed the base branch from sol-yul-refactor-move-name-functions-from-context-to-common to develop May 15, 2020
@cameel cameel force-pushed the sol-yul-refactor-add-arity branch from e31d66c to 75e809d May 18, 2020
#include <string>

namespace solidity::frontend
{

/**
* Structure that describes arity and co-arity of a function, i.e. the number of its inputs and outputs.

This comment has been minimized.

@chriseth

chriseth May 19, 2020
Contributor

Maybe clarify: Arity of the yul function representing an internal call to a Solidity function.

This comment has been minimized.

@cameel

cameel May 19, 2020
Author Member

I'll rename it to YulArity. It could actually be used for both Solidity and Yul arity but I guess, if we ever have a need for something like that for Solidity functions it might be better to have separate types.

This comment has been minimized.

@cameel

cameel May 19, 2020
Author Member

Done.

@cameel cameel force-pushed the sol-yul-refactor-add-arity branch from 75e809d to 97378d0 May 19, 2020
@chriseth
Copy link
Contributor

@chriseth chriseth commented May 19, 2020

Typo: ./libsolidity/codegen/ir/Common.h:68: ambigous ==> ambiguous

{
explicit YulArity(size_t _in, size_t _out): in(_in), out(_out) {}

static YulArity fromDefinition(FunctionDefinition const& _function);

This comment has been minimized.

@chriseth

chriseth May 19, 2020
Contributor

Either you extend the description of the struct above or you shortly explain how you get from the Solidity function definiton to the arity of the yul function.

This comment has been minimized.

@cameel

cameel May 19, 2020
Author Member

OK. I'll add more info.

This comment has been minimized.

@cameel

cameel May 19, 2020
Author Member

Well, maybe it's better to remove it after all. There seem to be many ways to go from a definition to the type. I realized that I should probably include internal somewhere in the name and even then it would still be ambiguous. I could document it but it's probably better to have something like this in the code instead:

YulArity::fromType(*TypeProvider::function(*function, FunctionType::Kind::Internal))
@cameel
Copy link
Member Author

@cameel cameel commented May 19, 2020

Typo: ./libsolidity/codegen/ir/Common.h:68: ambigous ==> ambiguous

Thanks. I don't think I can run this spellchecker locally and I'm still trying to figure out how to go around the CircleCI login requirement :)

@cameel cameel force-pushed the sol-yul-refactor-add-arity branch from 97378d0 to fc85050 May 19, 2020
@cameel cameel force-pushed the sol-yul-refactor-add-arity branch from fc85050 to 1a521cc May 19, 2020
@chriseth chriseth merged commit d7b434f into develop May 19, 2020
34 of 36 checks passed
34 of 36 checks passed
ci/circleci: t_ubu_asan_constantinople CircleCI is running your tests
Details
ci/circleci: t_ubu_release_soltest CircleCI is running your tests
Details
ci/circleci: b_archlinux Your tests passed on CircleCI!
Details
ci/circleci: b_docs Your tests passed on CircleCI!
Details
ci/circleci: b_ems Your tests passed on CircleCI!
Details
ci/circleci: b_osx Your tests passed on CircleCI!
Details
ci/circleci: b_ubu Your tests passed on CircleCI!
Details
ci/circleci: b_ubu18 Your tests passed on CircleCI!
Details
ci/circleci: b_ubu_asan Your tests passed on CircleCI!
Details
ci/circleci: b_ubu_asan_clang Your tests passed on CircleCI!
Details
ci/circleci: b_ubu_clang Your tests passed on CircleCI!
Details
ci/circleci: b_ubu_cxx20 Your tests passed on CircleCI!
Details
ci/circleci: b_ubu_ossfuzz Your tests passed on CircleCI!
Details
ci/circleci: b_ubu_release Your tests passed on CircleCI!
Details
ci/circleci: chk_antlr_grammar Your tests passed on CircleCI!
Details
ci/circleci: chk_buglist Your tests passed on CircleCI!
Details
ci/circleci: chk_coding_style Your tests passed on CircleCI!
Details
ci/circleci: chk_docs_pragma_min_version Your tests passed on CircleCI!
Details
ci/circleci: chk_proofs Your tests passed on CircleCI!
Details
ci/circleci: chk_pylint Your tests passed on CircleCI!
Details
ci/circleci: chk_spelling Your tests passed on CircleCI!
Details
ci/circleci: t_ems_compile_ext_colony Your tests passed on CircleCI!
Details
ci/circleci: t_ems_compile_ext_gnosis Your tests passed on CircleCI!
Details
ci/circleci: t_ems_compile_ext_zeppelin Your tests passed on CircleCI!
Details
ci/circleci: t_ems_solcjs Your tests passed on CircleCI!
Details
ci/circleci: t_osx_cli Your tests passed on CircleCI!
Details
ci/circleci: t_osx_soltest Your tests passed on CircleCI!
Details
ci/circleci: t_ubu_asan_cli Your tests passed on CircleCI!
Details
ci/circleci: t_ubu_asan_constantinople_clang Your tests passed on CircleCI!
Details
ci/circleci: t_ubu_clang_soltest Your tests passed on CircleCI!
Details
ci/circleci: t_ubu_cli Your tests passed on CircleCI!
Details
ci/circleci: t_ubu_release_cli Your tests passed on CircleCI!
Details
ci/circleci: t_ubu_soltest Your tests passed on CircleCI!
Details
ci/circleci: t_ubu_soltest_enforce_yul Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Solidity automation moved this from Review in progress to Done May 19, 2020
@chriseth chriseth deleted the sol-yul-refactor-add-arity branch May 19, 2020
@cameel cameel added this to In progress in Sol -> Yul CodeGen via automation Jun 3, 2020
@cameel cameel moved this from In progress to Done in Sol -> Yul CodeGen Jun 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Solidity
  
Done
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.