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

Fix precedences of * (deref) and & (address-of) #21

Merged
merged 1 commit into from
Jun 7, 2019

Conversation

calixteman
Copy link
Contributor

No description provided.

Copy link
Contributor

@maxbrunsfeld maxbrunsfeld left a comment

Choose a reason for hiding this comment

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

Looks good! Two questions.

@@ -260,7 +260,6 @@ module.exports = grammar({

type_qualifier: $ => choice(
'const',
'restrict',
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to remove restrict here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my bad I should have done this in another commit.
Anyway the keyword is duplicated in the list (just after volatile)

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, no I didn't realize! That's fine I don't mind them being in the same commit. I usually squash PRs anyway, to avoid extra commits of the generated parser.c file.

prec.left(PREC.UNARY, seq('*', $._expression)),
prec.left(PREC.UNARY, seq('&', $._expression))
prec.left(PREC.CAST, seq('*', $._expression)),
prec.left(PREC.CAST, seq('&', $._expression))
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should introduce a new PREC.POINTER constant here, rather than using CAST? Or do you think it's good to explicitly state that pointer operations and casts have the same precedence?

Copy link
Contributor Author

@calixteman calixteman Jun 5, 2019

Choose a reason for hiding this comment

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

According to this doc:
https://en.cppreference.com/w/c/language/operator_precedence
cast and deref should have the same precedence.
At the beginning I thought to add PREC.DEREF but after reading this doc I thought it was better to do the same.
Feel free to disagree :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, ok great, thanks for explaining. Let's leave it exactly as you have it.

@calixteman calixteman force-pushed the fix_deref_precedence branch from f8f2cb5 to aae6ad3 Compare June 6, 2019 08:09
@maxbrunsfeld maxbrunsfeld merged commit 2945de0 into tree-sitter:master Jun 7, 2019
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.

2 participants