eduucaldas added a reviewer: gribozavr2.
eduucaldas added inline comments.
================
Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:594
+ /// Shrink \p Range to a subrange that only contains tokens of a list.
+ ArrayRef<syntax::Token> shrinkToFitList(ArrayRef<syntax::Token> Range) {
+ auto BeginChildren = Trees.lower_bound(Range.begin());
----------------
WDYT about the naming?
================
Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:2932
|-'void'
- |-SimpleDeclarator Declarator
- | |-'foo'
- | `-ParametersAndQualifiers
- | |-'(' OpenParen
- | `-')' CloseParen
+ |-DeclaratorList Declarators
+ | `-SimpleDeclarator ListElement
----------------
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)
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
