Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking: fixed keywords added in for token type #416

Closed
wants to merge 10 commits into from

Conversation

VoR0220
Copy link
Member

@VoR0220 VoR0220 commented Mar 7, 2016

Note: Do not merge until 0.3.0.

@chriseth
Copy link
Contributor

chriseth commented Mar 7, 2016

Is that all we need? Don't we need additional checks in Token::fromIdentifierOrKeyword so that e.g. fixed8000x2 is not parsed as a keyword but fixed8x8 is?

@VoR0220
Copy link
Member Author

VoR0220 commented Mar 7, 2016

Ah, you wanted those in there too. I figured you only wanted the keywords and we could handle the fromIdentifier package when we actually implement fixed. But I suppose we can do that here too.

@VoR0220
Copy link
Member Author

VoR0220 commented Mar 7, 2016

just realized...if an invalid argument gets thrown in extractUnsigned for a bytes type, it might actually be able to make it through. Going to change this to a zero.

// the variable's identity as an identifier. If an invalid conversion
// error is thrown (usually in the case of grabbing N from a fixed type)
// then a 0 is thrown to purposely ensure that it will declare itself as an identifier
static unsigned extractUnsigned(std::string const& _literal);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we change that to take two const iterators (_begin and _end) instead of a string? In that case we do not have to copy the string.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we can do that. Good idea.

Copy link
Member Author

Choose a reason for hiding this comment

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

done and on the way.

@@ -101,17 +108,21 @@ char const Token::m_tokenType[] =
{
TOKEN_LIST(KT, KK)
};
unsigned Token::extractM(string const& _literal)
unsigned Token::extractUnsigned(string::const_iterator const& _begin, string::const_iterator const& _end)
Copy link
Contributor

Choose a reason for hiding this comment

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

Iterators should be passed by value

Copy link
Member Author

Choose a reason for hiding this comment

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

I've been trying to figure out how to make this work with iterators but nothing is clicking so far.

return m;
}
catch(out_of_range& e)
catch(const boost::bad_lexical_cast &)
Copy link
Contributor

Choose a reason for hiding this comment

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

boost::bad_lexical_cast const&

@chriseth
Copy link
Contributor

Please also add timestamp as a new keyword (it will be a type).

@VoR0220
Copy link
Member Author

VoR0220 commented Mar 10, 2016

either we need to pick a different name for timestamp or we need to redo something in the compiler because currently it's causing a conflict with block.timestamp if it's set as a keyword type. No problems if it's a non-keyword type.

@chriseth
Copy link
Contributor

Subsumed by #429

@VoR0220 VoR0220 closed this Mar 11, 2016
@VoR0220 VoR0220 deleted the fixedKeyword branch May 12, 2016 18:01
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