[PATCH] D27279: Store decls in prototypes on the declarator instead of in the AST

Reid Kleckner via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 8 14:26:34 PST 2016


rnk added inline comments.


================
Comment at: include/clang/Sema/DeclSpec.h:1240
+    /// in the prototype. These are generally tag types or enumerators.
+    unsigned NumDeclsInPrototype : 8;
+
----------------
rnk wrote:
> rsmith wrote:
> > It seems plausible that generated code could have more than 256 such declarations. This class has a pointer, a set of bit-fields, 12 `unsigned`s, and another pointer, so we could give this a full 32 bits without increasing the size of the class. Alternatively we could share the storage with `NumExceptions`, since this can only be non-zero in C and that can only be non-zero in C++.
> I did it as a union with the EH machinery initially, but it changes our behavior on this test case:
>   void f(struct Foo {} o) {} // error in C++
>   struct Foo {}; // follow on error for redefining type Foo
> 
> We actually have test cases for this at clang/test/SemaCXX/type-definition-in-specifier.cpp. I don't think it's important to preserve that exact behavior, though, so performance may be more important. I just did it this way to avoid premature optimization.
As discussed offline, I updated the C++ test case and made this stuff not take up any space in C++ mode.


================
Comment at: test/Misc/ast-dump-decl.c:109-110
 // CHECK:      FunctionDecl{{.*}} TestFunctionDecl 'int (int, enum {{.*}})'
-// CHECK-NEXT:   EnumDecl
-// CHECK-NEXT:     EnumConstantDecl{{.*}} e
 // CHECK-NEXT:   ParmVarDecl{{.*}} x
----------------
rnk wrote:
> rsmith wrote:
> > Why is this not here any more? Looks like SemaDecl.cpp:8255 should have added it to this `DeclContext`. It'd be nice for `-ast-dump` to still dump these declarations somewhere.
> I changed things to dump non-parameter decls in the function now, but we only move NamedDecls into the function DeclContext, so the output is a little different. Think it matters?
I reverted my change to the dumper. Without more work, it's hard to know what decls came from the function prototype and which come from the function body. Filtering our parameters isn't enough. Think it's OK as is?


https://reviews.llvm.org/D27279





More information about the cfe-commits mailing list