Skip to content

[BREAKING] Disallow trailing dot not followed by number#4172

Merged
chriseth merged 1 commit intodevelopfrom
trailing_dot
May 30, 2018
Merged

[BREAKING] Disallow trailing dot not followed by number#4172
chriseth merged 1 commit intodevelopfrom
trailing_dot

Conversation

@leonardoalt
Copy link
Copy Markdown

@leonardoalt leonardoalt commented May 22, 2018

Closes #3210.

The trick here is that version tokens are also read as numbers, so there it should be fine to have other stuff after a .. So if the parser sees something like 1., 1 is returned as a number token and parsing goes on with ..

@leonardoalt
Copy link
Copy Markdown
Author

Missing syntax tests

@axic
Copy link
Copy Markdown
Contributor

axic commented May 23, 2018

The trick here is that version tokens are also read as numbers, so there it should be fine to have other stuff after a .

Why should versions like 0. be valid?

if (m_char == '.')
{
// A '.' has to be followed by a number.
if (!isDecimalDigit(m_source.get(1)))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should also use m_source.isPastEndOfInput. To ensure it is tested, please add a test case of contract C { uint a = 2.

Copy link
Copy Markdown
Contributor

@axic axic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add tests to the SolidityScanner file too.

@leonardoalt
Copy link
Copy Markdown
Author

Versions like 0. shouldn't be valid, but what I meant were things like 0.*

@leonardoalt leonardoalt changed the base branch from 050 to develop May 28, 2018 15:49
@leonardoalt
Copy link
Copy Markdown
Author

Rebased

if (error.locationStart >= 0 && error.locationEnd >= 0)
{
assert(static_cast<size_t>(error.locationStart) <= source.length());
assert(static_cast<size_t>(error.locationEnd) <= source.length());
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fails if the error is in the end of the file, that is, if the contract is incomplete.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would still be good to have these. What are the exact numbers? This could be a bug in the parser or scanner.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't know what happened, but I just ran it now to get the numbers and it didn't crash 😕

if (error.locationStart >= 0 && error.locationEnd >= 0)
{
assert(static_cast<size_t>(error.locationStart) <= source.length());
assert(static_cast<size_t>(error.locationEnd) <= source.length());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would still be good to have these. What are the exact numbers? This could be a bug in the parser or scanner.

@leonardoalt
Copy link
Copy Markdown
Author

Squashed and rebased.

scanner.reset(CharStream(".5"), "");
BOOST_CHECK_EQUAL(scanner.currentToken(), Token::Number);
scanner.reset(CharStream(".5e10"), "");
BOOST_CHECK_EQUAL(scanner.currentToken(), Token::Number);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these above should have another expectation for EOS

@axic axic changed the title [BREAKING][050]Disallow trailing dot not followed by number [BREAKING] Disallow trailing dot not followed by number May 30, 2018
@axic axic dismissed chriseth’s stale review May 30, 2018 14:25

Original issue is gone.

@axic
Copy link
Copy Markdown
Contributor

axic commented May 30, 2018

@chriseth please review

@axic
Copy link
Copy Markdown
Contributor

axic commented May 30, 2018

There are optimiser test failures, not sure they are just a CI issue, so restarted it.

@chriseth
Copy link
Copy Markdown
Contributor

@axic can you come to https://meet.jit.si/SolidityWeekly, please?

@chriseth chriseth merged commit 0a1a8bf into develop May 30, 2018
@axic axic deleted the trailing_dot branch June 3, 2018 23:27
Xanewok added a commit to Xanewok/slang that referenced this pull request Mar 9, 2024
See argotorg/solidity#4172.

This unblocks parsing member access expressions where the primary
expression can't consume the dot in order to parse the `.member` postfix
expression.
github-merge-queue bot pushed a commit to NomicFoundation/slang that referenced this pull request Mar 19, 2024
…it since 0.5.0 (#891)

See argotorg/solidity#4172.

This unblocks parsing member access expressions where the primary
expression can't consume the dot in order to parse the `.member` postfix
expression.
rollup-smithbm0p added a commit to rollup-smithbm0p/slang that referenced this pull request Dec 26, 2025
…it since 0.5.0 (#891)

See argotorg/solidity#4172.

This unblocks parsing member access expressions where the primary
expression can't consume the dot in order to parse the `.member` postfix
expression.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants