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

Issue 15127 - Parser assertion on wrong code #5143

Merged
merged 1 commit into from
Sep 30, 2015
Merged

Conversation

9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented Sep 29, 2015

@@ -1100,7 +1100,6 @@ public:
TemplateParameters* tpl = null;
if (token.value == TOKlparen)
tpl = parseTemplateParameterList();
assert(token.value == TOKassign);
nextToken(); // skip over '='
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand this fix. The comment here says that the code expects the next token to be = but we've just removed the assertion that checks this assumption. Doesn't that mean that malformed user input may cause the following code to malfunction? Shouldn't this be turned into a check with a compile error message when the next token isn't = instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't that mean that malformed user input may cause the following code to malfunction?

Parser should accept any possible token streams. If the input tokens could be treated as valid D code representation, parser will generate AST. If not, parser will report some parsing errors. Both are valid as "parser program output".
In other words, any input token streams must not be treated as malformed. If any assumptions exist for the input token streams, it's just wrong. In here, I've just removed wrong assertion.

Shouldn't this be turned into a check with a compile error message when the next token isn't = instead?

In parseTemplateParameterList function, it's already reported errors for the parsing failures. But, indeed it would be better to have own error check here. I'll replace the bare nextToken() call with check(TOKassign).

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense. Thanks!

@yebblies
Copy link
Member

Auto-merge toggled on

yebblies added a commit that referenced this pull request Sep 30, 2015
Issue 15127 - Parser assertion on wrong code
@yebblies yebblies merged commit eb1c764 into dlang:master Sep 30, 2015
@9rnsr
Copy link
Contributor Author

9rnsr commented Sep 30, 2015

@yebblies Thanks!

@9rnsr 9rnsr deleted the fix15127 branch September 30, 2015 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants