Emit warnings for keywords selected to be reserved in the future #16206
Conversation
matheusaaguiar
force-pushed
the
futureKeywordWarnings
branch
3 times, most recently
from
586c3ca to
268a776
Compare
|
This pull request is stale because it has been open for 14 days with no activity. |
github-actions
bot
added
the
stale
matheusaaguiar
removed
the
stale
matheusaaguiar
force-pushed
the
futureKeywordWarnings
branch
from
268a776 to
fb8ef20
Compare
|
This pull request is stale because it has been open for 14 days with no activity. |
github-actions
bot
added
the
stale
matheusaaguiar
removed
the
stale
|
This pull request is stale because it has been open for 14 days with no activity. |
github-actions
bot
added
the
stale
matheusaaguiar
removed
the
stale
| pragma solidity >=0.4.16 <0.9.0; | ||
| library GetCode { | ||
| // This will report a warning - at you will be promoted to reserved keyword |
There was a problem hiding this comment.
| // This will report a warning - at you will be promoted to reserved keyword | |
| // This will report a warning - `at` will be promoted to a reserved keyword |
| if (futureKeywords.count(_declaration.name())) | ||
| m_errorReporter.warning( |
There was a problem hiding this comment.
if is indented too much.
| if (futureKeywords.count(_declaration.name())) | |
| m_errorReporter.warning( | |
| if (futureKeywords.contains(_declaration.name())) | |
| m_errorReporter.warning( |
| @@ -1,3 +1,4 @@ | |||
| contract at layout at type(uint).max { } | |||
There was a problem hiding this comment.
I'd rename the contract here and in other similar tests to something other than at or layout. The name is not the point of the test and we already have it covered by contract_named_at.sol and contract_named_layout.sol.
| if (m_evmVersion < langutil::EVMVersion::london()) | ||
| futureReservedKeywords.insert("basefee"); | ||
| if (m_evmVersion < langutil::EVMVersion::paris()) | ||
| futureReservedKeywords.insert("prevrandao"); | ||
| if (m_evmVersion < langutil::EVMVersion::cancun()) | ||
| { |
There was a problem hiding this comment.
This warning should not be EVM-version dependent. When we make these into keywords, they will be disallowed regardless of EVM version.
| "\"{}\" will be promoted to reserved keyword in the next breaking version" | ||
| " and will not be allowed as an identifier anymore.", |
There was a problem hiding this comment.
Not necessarily something to change here, but just wanted to note that our terminology is all over the place: "keyword" vs "reserved keyword" vs "reserved identifier". We should get that straight at some point, because these are not synonyms.
There was a problem hiding this comment.
I'd add a few more tests:
- That the future keywords trigger the warning when used as:
- module name (
import ... as Xorimport * as X from ...) - alias name (
import {... as X} from ...) - UDVT name
- struct/enum field name
- Yul variable/function name
- module name (
- That the future Yul keywords (i.e.
leave) do trigger the warning outside of Yul.- It's actually up for discussion whether these should be reserved at Solidity level, but so far that has been the case with all other Yul keywords (including
switchandlet, which have no functionality in Solidity).
- It's actually up for discussion whether these should be reserved at Solidity level, but so far that has been the case with all other Yul keywords (including
- That the future Yul reserved identifiers (e.g.
blobhash) do not trigger the warning outside of Yul.
There was a problem hiding this comment.
The PR needs a changelog entry.
There was a problem hiding this comment.
Perhaps we should also add a note/warning about the new keywords under Reserved Keywords in the docs.
| std::set<ASTString> const futureKeywords = { | ||
| "transient", | ||
| "layout", | ||
| "at", | ||
| "error", | ||
| "super", | ||
| "this" | ||
| }; |
There was a problem hiding this comment.
We should put the list in a more general location. Token.h seems like the most appropriate one. We already have isYulKeyword() there. We can add isFutureSolidityKeyword() and isFutureYulKeyword().
Partially solves #15795 and #14770.