[BREAKING] Add new reserved keywords. by ekpyron · Pull Request #3627 · argotorg/solidity

ekpyron · 2018-03-01T11:58:41Z

Closes #2182.

chriseth · 2018-03-01T13:18:51Z

If you add Closes #2182 to the description of the pull request, that issue will be automatically closed and also linked to this pull request.

chriseth · 2018-03-01T13:25:07Z

test/libsolidity/SolidityParser.cpp

+	};
+
+
+	for (const auto& keyword : keywords) {


Coding style: no space in front of : except in a ? b : c.

And { on a line of its own.

Thanks for pointing that out :)

chriseth · 2018-03-01T13:27:16Z

Please also add a line to Changelog.txt describing the change. It is probably a good idea for this PR to not add it to the changelog of 0.4.21 but instead start a new version 0.5.0. We will then merge it when 0.5.0 will be the next release.

chriseth · 2018-03-01T13:49:42Z

Looks good! I'm not adding a positive review on purpose, so we do not accidentally merge this PR.

axic · 2018-03-02T08:15:59Z

test/libsolidity/SolidityParser.cpp

 	CHECK_PARSE_ERROR(text, "Expected identifier");
 }

+BOOST_AUTO_TEST_CASE(keyword_is_reserved)


Can this test case be extracted and merged with current keywords?

OK, see #3633

axic

There is a section in the documentation listing the reserved keywords, that should be updated too.

axic · 2018-03-02T14:41:30Z

Please rebase since #3633 was merged.

chriseth · 2018-03-05T14:31:54Z

We should also add copy and copyof.

axic · 2018-03-05T17:11:33Z

If you add Closes #2182 to the description of the pull request

Please do that and do not include it in the commit messages.

Outdated.

ekpyron · 2018-05-09T11:47:51Z

I removed the constructor keyword from this PR, since it breaks the tests unless the parser is updated, so it makes more sense to only add it in #4106.

ekpyron · 2018-05-15T12:25:11Z

@chriseth I find the list of new keywords (and especially copy) to be a bit excessive myself and it seems to me more frequent breaking releases make more sense.
The question is whether there is a subset of this list that should definitely be in 0.5.0.

chriseth · 2018-05-15T14:27:27Z

100%: constructor and emit
80%: mutable, immutable, unchecked, override
60%: alias, typedef

axic · 2018-06-12T09:40:10Z

Rebased.

ekpyron · 2018-06-12T09:43:23Z

@axic We still need to decide whether we want to keep this PR at all or which subset to keep.
constructor and emit are handled in separate commits, but we need to decide which of the other ones should in fact be reserved in 0.5.0 already.

axic · 2018-06-12T10:10:58Z

I think we should just merge this. The only debatable one I think is topic and perhaps fallback because we don't know yet if we'll have wildcard or fallback, etc.

ekpyron · 2018-06-12T10:12:28Z

@axic I think what started the discussion actually was whether to keep copy, since it may be commonly used and break a lot of code. (@chriseth brought that up in an now outdated Changelog.md review comment)

axic · 2018-06-12T10:14:11Z

Also if this is merged first and constructor/emit later then those are just moved from the reserved list to the "new keywords" line.

axic · 2018-06-12T10:16:11Z

It seems copy is used in stringutils so that might be something we need to consider. Nothing else seems to break.

axic · 2018-06-12T10:19:18Z

I'd take a shortcut and merge the non-contentious ones here and open a new PR for the rest.

ekpyron · 2018-06-12T10:35:03Z

Removed copy, topic and fallback from the list.
I would still not include constructor and emit here, since the workarounds will fail once the keywords are introduced, so I think it's best to handle them separately altogether.

axic · 2018-06-12T12:21:51Z

Confirmed against the issue.

ekpyron · 2018-06-12T14:03:45Z

Removed const as well.

erak force-pushed the additional-keywords branch from 6de5f25 to 4522978 Compare March 1, 2018 13:21

chriseth reviewed Mar 1, 2018

View reviewed changes

chriseth changed the title ~~Add new reserved keywords from #2182.~~ [BREAKING] Add new reserved keywords from #2182. Mar 1, 2018

axic reviewed Mar 2, 2018

View reviewed changes

axic previously requested changes Mar 2, 2018

View reviewed changes

ekpyron force-pushed the additional-keywords branch 2 times, most recently from eb163bc to 1a5e5f0 Compare March 2, 2018 09:52

ekpyron mentioned this pull request Mar 2, 2018

Parser: Add test case for reserved keywords. #3633

Merged

ekpyron force-pushed the additional-keywords branch from 1a5e5f0 to 9f30e74 Compare March 2, 2018 09:56

ekpyron force-pushed the additional-keywords branch from d03676d to 8575b5b Compare March 12, 2018 09:43

ekpyron self-assigned this May 4, 2018

ekpyron changed the base branch from develop to 050 May 8, 2018 09:47

ekpyron force-pushed the additional-keywords branch from 8575b5b to f62820c Compare May 8, 2018 09:48

ekpyron changed the title ~~[BREAKING] Add new reserved keywords from #2182.~~ [BREAKING] Add new reserved keywords. May 8, 2018

ekpyron force-pushed the additional-keywords branch 3 times, most recently from 1a680bf to 8e1e611 Compare May 8, 2018 09:50

This was referenced May 9, 2018

Remove workaround for parsing the constructor keyword. #4106

Closed

Remove workaround for parsing the emit keyword. #4107

Closed

chriseth force-pushed the 050 branch from dc857fe to 40cc63c Compare May 14, 2018 12:04

ekpyron force-pushed the additional-keywords branch 2 times, most recently from 5cf64ca to 566b753 Compare May 14, 2018 18:46

chriseth force-pushed the 050 branch from 2d1ffcf to f98c8fa Compare May 15, 2018 13:08

ekpyron force-pushed the additional-keywords branch from 10d2a1e to 911cead Compare May 15, 2018 13:14

chriseth force-pushed the 050 branch from f98c8fa to cb4a9ab Compare May 15, 2018 15:06

ekpyron force-pushed the additional-keywords branch from 911cead to c6434ea Compare May 15, 2018 16:27

chriseth force-pushed the 050 branch from cb4a9ab to 22ff3b7 Compare May 16, 2018 16:33

ekpyron force-pushed the additional-keywords branch from c6434ea to 2106f82 Compare May 17, 2018 15:57

axic changed the base branch from 050 to develop May 30, 2018 06:04

axic force-pushed the additional-keywords branch from 2106f82 to 521f5fc Compare June 12, 2018 09:39

ekpyron force-pushed the additional-keywords branch from 521f5fc to fb9dff1 Compare June 12, 2018 10:33

ekpyron force-pushed the additional-keywords branch from fb9dff1 to f35b5e7 Compare June 12, 2018 11:24

axic previously approved these changes Jun 12, 2018

View reviewed changes

Add new reserved keywords.

e4b7b21

ekpyron dismissed axic’s stale review via e4b7b21 June 12, 2018 14:03

ekpyron force-pushed the additional-keywords branch from f35b5e7 to e4b7b21 Compare June 12, 2018 14:03

axic approved these changes Jun 12, 2018

View reviewed changes

axic merged commit d1e7e9e into develop Jun 12, 2018

axic deleted the additional-keywords branch June 12, 2018 15:33

cameel mentioned this pull request Jul 29, 2025

Reserve keywords and remove identifier exceptions for 0.9.0 #14770

Open

Conversation

ekpyron commented Mar 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chriseth commented Mar 1, 2018

Uh oh!

chriseth Mar 1, 2018

Choose a reason for hiding this comment

Uh oh!

chriseth Mar 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erak Mar 1, 2018

Choose a reason for hiding this comment

Uh oh!

chriseth commented Mar 1, 2018

Uh oh!

chriseth commented Mar 1, 2018

Uh oh!

axic Mar 2, 2018

Choose a reason for hiding this comment

Uh oh!

ekpyron Mar 2, 2018

Choose a reason for hiding this comment

Uh oh!

axic left a comment

Choose a reason for hiding this comment

Uh oh!

axic commented Mar 2, 2018

Uh oh!

chriseth commented Mar 5, 2018

Uh oh!

axic commented Mar 5, 2018

Uh oh!

ekpyron commented May 9, 2018

Uh oh!

ekpyron commented May 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chriseth commented May 15, 2018

Uh oh!

axic commented Jun 12, 2018

Uh oh!

ekpyron commented Jun 12, 2018

Uh oh!

axic commented Jun 12, 2018

Uh oh!

ekpyron commented Jun 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

axic commented Jun 12, 2018

Uh oh!

axic commented Jun 12, 2018

Uh oh!

axic commented Jun 12, 2018

Uh oh!

ekpyron commented Jun 12, 2018

Uh oh!

axic commented Jun 12, 2018

Uh oh!

ekpyron commented Jun 12, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ekpyron commented Mar 1, 2018 •

edited

Loading

chriseth Mar 1, 2018 •

edited

Loading

ekpyron commented May 15, 2018 •

edited

Loading

ekpyron commented Jun 12, 2018 •

edited

Loading