solidity
solidity copied to clipboard
ethereum
adding parser support for code keyword for data location
For the record: we need to decide how to deal with the .code member of address types to proceed here.
For the record: we need to decide how to deal with the
.codemember ofaddresstypes to proceed here.
@ekpyron Why did we decide to use code for data location over codedata? To me, code in that context looks a bit weird :)
For the record: we need to decide how to deal with the
.codemember ofaddresstypes to proceed here.@ekpyron Why did we decide to use
codefor data location overcodedata? To me,codein that context looks a bit weird :)
The choice is primarly motivated by the names of the opcodes on the EVM level. It's codecopy, not codedatacopy :-).
@nishant-sachdeva See https://github.com/ethereum/solidity/issues/13368#issuecomment-1239448558 for the solution to the issue with the .code member of address types. Let me know if you need more guidance with it than that comment, but since we already have a very similar workaround, it shouldn't cause too much issues.
I feel as though this ought be covered more by the docs, but at the same time, this is only one part of the feature, so I'm assuming the docs will be covered in another PR?
I feel as though this ought be covered more by the docs, but at the same time, this is only one part of the feature, so I'm assuming the docs will be covered in another PR?
Yeah, this PR is just the bare-bone that would allow us to actually implement the actual feature non-breakingly on top of it - so I'd document this only once it actually starts working with a subsequent PR.
@nishant-sachdeva 13 tests are failing.
These are all external tests, and the reason they're failing is that somewhere in these external repositories, code is likely used as a variable name, or a function name, whereas with @nishant-sachdeva's changes, code is now a reserved keyword, causing these tests to fail (compilation failures).
Also note that the destination branch for this is breaking and not develop, so I guess the merging procedure is different, but someone else will have to chime in on this since I have no idea :smile:
The one thing that was supposed to be done was to either fix theses tests in their respective repositories, or to fix them in our own forks and then change the repo address in our bash scripts that run these external tests. In any case, I forgot what the outcome of this discussion was as it was a while ago.
@nikola-matic @NunoFilipeSantos
So, about the CI tests. As Nikola mentioned some tests had this issue where they were using the code as an identifier.
However, all of that has been fixed. I've made the changes in our forks in the Solidity External Tests repo. At the moment, most of these tests that are failing are showing wrong version detected as the error.
For fixing that, I think, to begin with, we need to merge develop into breaking and then rebase this branch on top of breaking. Once we've done that, we'll get a much better estimation of where the changes need to be made.