eduucaldas marked 3 inline comments as done.
eduucaldas added inline comments.
================
Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:2932
|-'void'
- |-SimpleDeclarator Declarator
- | |-'foo'
- | `-ParametersAndQualifiers
- | |-'(' OpenParen
- | `-')' CloseParen
+ |-DeclaratorList Declarators
+ | `-SimpleDeclarator ListElement
----------------
gribozavr2 wrote:
> eduucaldas wrote:
> > This sounds weird and is probably the reason why SO many tests failed.
> >
> > According to the [[
> > https://eel.is/c++draft/dcl.fct.def.general#nt:function-definition |
> > grammar ]] a function definition/declaration might only have one declarator:
> > ```
> > function-definition:
> > attribute-specifier-seq_opt decl-specifier-seq_opt declarator
> > virt-specifier-seq_opt function-body
> > ...
> > ```
> > But our implementation calls `processDeclaratorAndDeclaration` for
> > `DeclaratorDecl`, which englobes `FunctionDecl`.
> >
> > I also noticed that we seem to have quite extensive support for
> > declarations even rarer ones:
> > `StaticAssertDeclaration`, `LinkageSpecificationDeclaration` ... Moreover,
> > their names and definitions seem to adequately follow the grammar. However
> > we don't have a `FunctionDefinition` as defined in the grammar. Was
> > grouping `FunctionDefintion` and `FunctionDeclaration` as
> > `SimpleDeclaration` a conscious choice? (I looked quickly at commits that
> > added `processDeclaratorAndDeclaration` but couldn't find any clue)
> It seemed reasonable for uniformity. However, I can definitely see an
> argument for defining a special kind of tree node for them, especially now
> that if we use SimpleDeclaration for functions we would always get a
> 1-element list wrapper.
>
> On the other hand, the following is a valid declaration:
>
> ```
> int x, f(int);
> ```
>
> Wouldn't it be weird if function definitions and declarations were modeled
> differently? I understand they are different in the C++ grammar, but I think
> it would be at least a bit surprising for users.
Oh my god... that hurts my eyes...
I don't know what is more weird to be fair. In my view unifying both seems like
a similar decision as unifying expressions and statements under the same
ClangAST node because:
1. I think they have other syntactic differences.
2. I think generally people see declarations and definitions as different
things. Whereas having a list in the definition of a function is completely
unexpected.
Looking back at the grammar for declarations I think we would also benefit of
trying to simplify it before mapping it to syntax trees. Although I would love
to do that, I'm afraid there's too much on my plate already.
Should I leave a `FIXME` regarding this and adapt the other tests? Or do you
wanna discuss this more deeply? Both options are fine for me :)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88403/new/
https://reviews.llvm.org/D88403
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
