-
-
Notifications
You must be signed in to change notification settings - Fork 124
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: allow preprocessor directives in an enum_specifier
#182
Conversation
Preprocessor directives inside of an `enum_specifier` variant list cause ERROR nodes to appear in the syntax tree. Both the `struct_specifier` and the `union_specifier` allow the preprocessor directives to exist to selectively enable or disable fields in the `struct` or `union`. It is fairly common in C to also selectively add variants to an `enum` in the same manner. This modifies the grammar for the `enumerator_list` so that the preprocessor directives are allowed. My initial approach was to just make the commas optional at the end of the `enumerator` and, while that did allow a simple `_enumerator_list_item` in much the same style as the `_field_declaration_list_item`, it was a change in behavior since it did not require a comma in between every `enum` variant. By splitting the `_enumerator_list_item` into a version with a comma required and an optional `_enumerator_list_item_end` that does NOT require a comma, this keeps the same behvior of requiring the comma separated list of variants without requiring a comma at the end of the list of variants. Additionally, it does not require a comma at the end of the preprocessor directives, which would not be correct either. The conflict additions were required to get the grammar to compile with the two different versions of `enumerator_list_item`. Additionally, the modifications to the grammar introduced some ambiguity in the precedence of the `concatenated_string`, so right precedence was remove that ambiguity. Fixes tree-sitter#181
The AppVeyor failure appears to be completely unrelated to the changes in this PR (looks like some Python/Node environmental issue). While the PR diff looks terrible, the actual changes I made were in |
Hey, thanks for the PR. I tweaked it a bit since the conflicts and additions were a bit excessive - I think only preproc ifs and ifdefs should be here, despite technically defines and other stuff being allowed, for the purpose of not increasing the state count too much. This is now fixed on master, thanks! |
So, I agree with you on the preprocessor defines and function defines. However, the #define FOO_BUILDER(sym) FOO_##sym
enum foo {
#ifdef A
FOO_BUILDER(BAR),
#else
FOO_BUILDER(MAX),
#endif
FOO_END,
}; That is also throwing the error nodes (it is kind of a dumb example, but a simplified version of what I am seeing in the codebase). The following diff (still smaller than the full PR branch I had) resolves it: diff --git a/grammar.js b/grammar.js
index d223f35..881406e 100644
--- a/grammar.js
+++ b/grammar.js
@@ -67,6 +67,7 @@ module.exports = grammar({
[$.enum_specifier],
[$._type_specifier, $._old_style_parameter_list],
[$.parameter_list, $._old_style_parameter_list],
+ [$.enumerator_list],
],
word: $ => $.identifier,
@@ -622,12 +623,14 @@ module.exports = grammar({
'{',
repeat(choice(
seq($.enumerator, ','),
+ $.preproc_call,
alias($.preproc_if_in_enumerator_list, $.preproc_if),
alias($.preproc_ifdef_in_enumerator_list, $.preproc_ifdef),
)),
optional(seq(
choice(
$.enumerator,
+ $.preproc_call,
alias($.preproc_if_in_enumerator_list_no_comma, $.preproc_if),
alias($.preproc_ifdef_in_enumerator_list_no_comma, $.preproc_ifdef),
), |
ah my bad, I should've added calls as well. Can we go with |
Yeah, doing |
done |
Preprocessor directives inside of an
enum_specifier
variant list cause ERROR nodes to appear in the syntax tree. Both thestruct_specifier
and theunion_specifier
allow the preprocessor directives to exist to selectively enable or disable fields in thestruct
orunion
. It is fairly common in C to also selectively add variants to anenum
in the same manner.This modifies the grammar for the
enumerator_list
so that the preprocessor directives are allowed. My initial approach was to just make the commas optional at the end of theenumerator
and, while that did allow a simple_enumerator_list_item
in much the same style as the_field_declaration_list_item
, it was a change in behavior since it did not require a comma in between everyenum
variant. By splitting the_enumerator_list_item
into a version with a comma required and an optional_enumerator_list_item_end
that does NOT require a comma, this keeps the same behvior of requiring the comma separated list of variants without requiring a comma at the end of the list of variants. Additionally, it does not require a comma at the end of the preprocessor directives, which would not be correct either. The conflict additions were required to get the grammar to compile with the two different versions ofenumerator_list_item
.Additionally, the modifications to the grammar introduced some ambiguity in the precedence of the
concatenated_string
, so right precedence was remove that ambiguity.Fixes #181