solidity icon indicating copy to clipboard operation
solidity copied to clipboard

Reserve keywords and remove identifier exceptions for 0.9.0

Open nikola-matic opened this issue 2 years ago • 9 comments

In this release cycle we had to introduce a bunch parser hacks to avoid breaking existing code:

  • In order to introduce new Yul opcodes in a non-breaking way, we had to mark them as exceptions for older EVM versions in createReservedIdentifiers():
    • basefee
    • prevrandao
    • tstore
    • tload
    • blobbasefee
    • blobhash
    • mcopy
    • clz
  • Existing builtins have not been made reserved yet
    • memoryguard
  • New keywords needed for storage layout syntax (#597):
    • at
    • layout
  • New keywords needed for transient storage (#15007)
    • transient
  • Other identifiers that should become keywords:
    • error
    • super
    • this
    • leave (in Yul)

In the next breaking release the exceptions should be removed and keywords added.

nikola-matic avatar Jan 09 '24 10:01 nikola-matic

Added memoryguard to the list. It's currently not reserved unlike all other Yul builtins.

cameel avatar Feb 03 '25 18:02 cameel

Added EOF identifiers that we already reserved on EOF but had to leave non-reserved in legacy to keep backwards compatibility.

See eof_identifiers_not_reserved_in_legacy.yul.

cameel avatar Feb 10 '25 17:02 cameel

Another one we could consider reserving is error: #11743. We already did that on breaking branch, which would put it in 0.9.0 (#11218), but decided to revert after some discussion (#11859).

The main objection seemed to be that "error" is a very common name and is sometimes used in contract interfaces. We still do not have the mechanism for overriding those (#11819).

I think we should reconsider this decision given that we're planning to make things like at and code reserved anyway. Seems inconsistent to single out just error. It looks like some tools cannot deal properly with contextual keywords, making people avoid it already: https://github.com/0xKitsune/solstat/issues/77.

My suggestion would be to at least add a deprecation warning and see if we get any significant feedback to keep it as is.

cameel avatar Jul 29 '25 12:07 cameel

BTW, in https://github.com/ethereum/solidity/issues/11743#issuecomment-900258969 @ekpyron also argues that fallback and receive should be promoted to keywords as well. I thought about adding those here as well, but... they are already keywords. They were marked as such in Token.h by #7385 which introduced fallback and receive functions (2 years prior to that discussion). The PR did almost all of the things we do when adding keywords (see e.g. #3627). They were not documented, but it does not matter, because for some reason the established convention is to do that only for unused ones and those were in use from the beginning. The only exception is that it was not stated explicitly in the changelog that they are keywords, which we usually do.

It seems to me like there may have been some intent originally not to admit they are keywords by not documenting them or something. If that was the case, it failed - I can't see how they could not be considered reserved in the current situation.

cameel avatar Jul 29 '25 12:07 cameel

We should consider adding Yul's leave to the list.

TBH I'm not sure what it's exact status is. It is already a token, but with a comment that it's not a keyword: https://github.com/ethereum/solidity/blob/6260712738fe67a752bf191dfa22d2c102d25d8e/liblangutil/Token.h#L269-L270

Due to this it cannot be used as an identifier in Yul (it's it's fine in Solidity).

It was added there in #9331, but as a refactor, without any mention in the changelog.

cameel avatar Jul 29 '25 16:07 cameel

Removed EOF keywords from the list.

cameel avatar Aug 05 '25 11:08 cameel

Removed code/codedata from the list (#13800/#13723). It's not yet certain if we will end up staying with immutable as suggested in #13368, but at least we won't be introducing a new keyword in 0.9.0.

cameel avatar Aug 06 '25 15:08 cameel

Decision from today's call: We're making error, this, super and leave into keywords in 0.9.0.

The main reason for having contextual keywords is to avoid breaking changes and that's not a concern here. Global keywords are simpler in terms of implementation and reduce the potential for misleading shadowing. It could be argued that some keywords might be better off being contextual, but we have no clear guidelines when to do that and we should really review existing keywords in that light as well. In the absence of that, we're going forward with converting them into global ones.

In case of leave, it appears that the identifier has a corresponding token but is special-cased to be recognized as an identifier in the Scanner. The token is not recognized as a keyword, but the identifier is. The special-casing needs to be removed and the token turned into a proper keyword.

this and super actually seem to be already reserved in some contexts, while in others they can be shadowed:

contract super {} // Error: The name "super" is reserved.
contract this {}  // Error: The name "this" is reserved.

contract C {
    function super() public pure {}  // Warning: This declaration shadows an existing declaration.
    function this() public pure {}   // Warning: This declaration shadows an existing declaration.
}

In 0.9.0 they will become parser keywords, reserved in all contexts.

cameel avatar Aug 13 '25 16:08 cameel

Adding clz (#16118) to the list of Yul builtin names to reserve.

cameel avatar Oct 16 '25 09:10 cameel