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

Make the varargs ellipsis a named node #31

Open
genotrance opened this issue Jan 28, 2019 · 3 comments
Open

Make the varargs ellipsis a named node #31

genotrance opened this issue Jan 28, 2019 · 3 comments

Comments

@genotrance
Copy link

No information is provided about the ellipsis in the AST.

extern void eggx_gsetinitialparsegeometry( const char *, ... ) ;

AST is:

(translation_unit 1 1 64
 (declaration 1 1 64
  (storage_class_specifier 1 1 6)
  (primitive_type 1 8 4)
  (function_declarator 1 13 50
   (identifier 1 13 29)
   (parameter_list 1 42 21
    (parameter_declaration 1 44 12
     (type_qualifier 1 44 5)
     (primitive_type 1 50 4)
     (abstract_pointer_declarator 1 55 1)
    )
   )
  )
 )
)
@maxbrunsfeld
Copy link
Contributor

The ... token is in there and you can access it, but it isn't named, so it doesn't show up in the printed tree. Maybe it should be given a name though.

@maxbrunsfeld maxbrunsfeld changed the title tree-sitter skips varargs Make the varargs ellipsis a named node Apr 18, 2019
@genotrance
Copy link
Author

genotrance commented Apr 14, 2020

Can you provide some info on how to detect this in the tree? I see there's two extra unnamed nodes but not sure how to detect that it is ... Thanks.

Edit: never mind, figured this out.

@shi-yan
Copy link

shi-yan commented Nov 24, 2021

I checked the code that handles variadic arguments, I think it is flawed and won't be simply fixed by giving ... a name:

    parameter_list: $ => seq(
      '(',
      commaSep(choice(
        $.parameter_declaration,
        $.optional_parameter_declaration,
        $.variadic_parameter_declaration,
        '...'
      )),
      ')'
    ),
...
    variadic_parameter_declaration: $ => seq(
      $._declaration_specifiers,
      field('declarator', choice(
        $.variadic_declarator,
        alias($.variadic_reference_declarator, $.reference_declarator)
      ))
    ),

    variadic_declarator: $ => seq(
      '...',
      optional($.identifier)
    ),

  1. ... has to be the last token of a parameter_list. whereas the grammar allows it to be in front of other arguments:
commaSep(choice(
        $.parameter_declaration,
        $.optional_parameter_declaration,
        $.variadic_parameter_declaration,
        '...'
      )),
  1. the last comma should be optional: https://en.cppreference.com/w/cpp/language/variadic_arguments , but the above would enforce it.

  2. the current grammar requires _declaration_specifiers is in front of ...

    variadic_parameter_declaration: $ => seq(
      $._declaration_specifiers,
      field('declarator', choice(
        $.variadic_declarator,
        alias($.variadic_reference_declarator, $.reference_declarator)
      ))
    ),

this will result in char ... fmt being accepted, whereas char fmt... being rejected. it seems that the current grammar wants to handle parameter pack? (https://en.cppreference.com/w/cpp/language/parameter_pack)

  1. also here,
    variadic_declarator: $ => seq(
      '...',
      optional($.identifier)
    ),

the ... token is defined as a prefix of an identifier, not a suffix? https://en.cppreference.com/w/cpp/language/variadic_arguments

some concrete examples:

tree-sitter-cpp:

void function(..., int a)
{}

accept

(translation_unit [0, 0] - [1, 2]
  (function_definition [0, 0] - [1, 2]
    type: (primitive_type [0, 0] - [0, 4])
    declarator: (function_declarator [0, 5] - [0, 25]
      declarator: (identifier [0, 5] - [0, 13])
      parameters: (parameter_list [0, 13] - [0, 25]
        (variadic_declarator [0, 14] - [0, 17])
        (parameter_declaration [0, 19] - [0, 24]
          type: (primitive_type [0, 19] - [0, 22])
          declarator: (identifier [0, 23] - [0, 24]))))
    body: (compound_statement [1, 0] - [1, 2])))

clang:

./test.cpp:1:6: error: variable has incomplete type 'void'
void function(..., int a)
     ^
./test.cpp:1:15: error: expected expression
void function(..., int a)
              ^
./test.cpp:1:24: error: expected '(' for function-style cast or type construction
void function(..., int a)
                   ~~~ ^
./test.cpp:1:26: error: expected ';' after top level declarator
void function(..., int a)
                         ^
                         ;
4 errors generated.

tree-sitter-cpp

void function(char ... a)
{}

accept

(translation_unit [0, 0] - [1, 2]
  (function_definition [0, 0] - [1, 2]
    type: (primitive_type [0, 0] - [0, 4])
    declarator: (function_declarator [0, 5] - [0, 25]
      declarator: (identifier [0, 5] - [0, 13])
      parameters: (parameter_list [0, 13] - [0, 25]
        (variadic_parameter_declaration [0, 14] - [0, 24]
          type: (primitive_type [0, 14] - [0, 18])
          declarator: (variadic_declarator [0, 19] - [0, 24]
            (identifier [0, 23] - [0, 24])))))
    body: (compound_statement [1, 0] - [1, 2])))

clang:

./test.cpp:1:20: error: type 'char' of function parameter pack does not contain any unexpanded parameter packs
void function(char ... a)
              ~~~~~^~~~~
1 error generated.

I think this should be labeled as bug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants