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

2016-12-09 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL289225: Store decls in prototypes on the declarator instead 
of in the AST (authored by rnk).

Changed prior to commit:
  https://reviews.llvm.org/D27279?vs=80827=80910#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27279

Files:
  cfe/trunk/include/clang/AST/Decl.h
  cfe/trunk/include/clang/Sema/DeclSpec.h
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/AST/ASTDumper.cpp
  cfe/trunk/lib/AST/Decl.cpp
  cfe/trunk/lib/Parse/ParseDecl.cpp
  cfe/trunk/lib/Parse/ParseExpr.cpp
  cfe/trunk/lib/Parse/ParseExprCXX.cpp
  cfe/trunk/lib/Sema/DeclSpec.cpp
  cfe/trunk/lib/Sema/SemaDecl.cpp
  cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp
  cfe/trunk/lib/Sema/SemaType.cpp
  cfe/trunk/test/Misc/ast-dump-decl.c
  cfe/trunk/test/PCH/decl-in-prototype.c
  cfe/trunk/test/Sema/decl-in-prototype.c
  cfe/trunk/test/SemaCXX/type-definition-in-specifier.cpp

Index: cfe/trunk/include/clang/Sema/DeclSpec.h
===
--- cfe/trunk/include/clang/Sema/DeclSpec.h
+++ cfe/trunk/include/clang/Sema/DeclSpec.h
@@ -1248,9 +1248,10 @@
 /// declarator.
 unsigned NumParams;
 
-/// NumExceptions - This is the number of types in the dynamic-exception-
-/// decl, if the function has one.
-unsigned NumExceptions;
+/// NumExceptionsOrDecls - This is the number of types in the
+/// dynamic-exception-decl, if the function has one. In C, this is the
+/// number of declarations in the function prototype.
+unsigned NumExceptionsOrDecls;
 
 /// \brief The location of the ref-qualifier, if any.
 ///
@@ -1300,6 +1301,11 @@
   /// \brief Pointer to the cached tokens for an exception-specification
   /// that has not yet been parsed.
   CachedTokens *ExceptionSpecTokens;
+
+  /// Pointer to a new[]'d array of declarations that need to be available
+  /// for lookup inside the function body, if one exists. Does not exist in
+  /// C++.
+  NamedDecl **DeclsInPrototype;
 };
 
 /// \brief If HasTrailingReturnType is true, this is the trailing return
@@ -1322,10 +1328,20 @@
 void destroy() {
   if (DeleteParams)
 delete[] Params;
-  if (getExceptionSpecType() == EST_Dynamic)
+  switch (getExceptionSpecType()) {
+  default:
+break;
+  case EST_Dynamic:
 delete[] Exceptions;
-  else if (getExceptionSpecType() == EST_Unparsed)
+break;
+  case EST_Unparsed:
 delete ExceptionSpecTokens;
+break;
+  case EST_None:
+if (NumExceptionsOrDecls != 0)
+  delete[] DeclsInPrototype;
+break;
+  }
 }
 
 /// isKNRPrototype - Return true if this is a K style identifier list,
@@ -1395,6 +1411,19 @@
   return static_cast(ExceptionSpecType);
 }
 
+/// \brief Get the number of dynamic exception specifications.
+unsigned getNumExceptions() const {
+  assert(ExceptionSpecType != EST_None);
+  return NumExceptionsOrDecls;
+}
+
+/// \brief Get the non-parameter decls defined within this function
+/// prototype. Typically these are tag declarations.
+ArrayRef getDeclsInPrototype() const {
+  assert(ExceptionSpecType == EST_None);
+  return llvm::makeArrayRef(DeclsInPrototype, NumExceptionsOrDecls);
+}
+
 /// \brief Determine whether this function declarator had a
 /// trailing-return-type.
 bool hasTrailingReturnType() const { return HasTrailingReturnType; }
@@ -1540,6 +1569,7 @@
  unsigned NumExceptions,
  Expr *NoexceptExpr,
  CachedTokens *ExceptionSpecTokens,
+ ArrayRef DeclsInPrototype,
  SourceLocation LocalRangeBegin,
  SourceLocation LocalRangeEnd,
  Declarator ,
Index: cfe/trunk/include/clang/Sema/Sema.h
===
--- cfe/trunk/include/clang/Sema/Sema.h
+++ cfe/trunk/include/clang/Sema/Sema.h
@@ -1527,12 +1527,6 @@
 NamedDecl *Previous;
   };
 
-  /// List of decls defined in a function prototype. This contains EnumConstants
-  /// that incorrectly end up in translation unit scope because there is no
-  /// function to pin them on. ActOnFunctionDeclarator reads this list and patches
-  /// them into the FunctionDecl.
-  std::vector DeclsInPrototypeScope;
-
   DeclGroupPtrTy ConvertDeclToDeclGroup(Decl *Ptr, Decl *OwnedType = nullptr);
 
   void DiagnoseUseOfUnimplementedSelectors();
Index: cfe/trunk/include/clang/AST/Decl.h
===
--- cfe/trunk/include/clang/AST/Decl.h
+++ cfe/trunk/include/clang/AST/Decl.h
@@ -1601,11 +1601,6 @@
   /// no formals.
   ParmVarDecl **ParamInfo;
 
-  /// 

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

2016-12-09 Thread Richard Smith via Phabricator via cfe-commits
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

I'd much prefer we take this size reduction now and worry about the minor loss 
of ast dump fidelity later. Please delete the commented out code though :)


https://reviews.llvm.org/D27279



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-12-08 Thread Reid Kleckner via Phabricator via cfe-commits
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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-12-08 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 80827.
rnk marked an inline comment as done.
rnk added a comment.

- Reuse EH specifier storage in C
- Revert dumper change


https://reviews.llvm.org/D27279

Files:
  include/clang/AST/Decl.h
  include/clang/Sema/DeclSpec.h
  include/clang/Sema/Sema.h
  lib/AST/ASTDumper.cpp
  lib/AST/Decl.cpp
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaTemplateVariadic.cpp
  lib/Sema/SemaType.cpp
  test/Misc/ast-dump-decl.c
  test/PCH/decl-in-prototype.c
  test/Sema/decl-in-prototype.c
  test/SemaCXX/type-definition-in-specifier.cpp

Index: test/SemaCXX/type-definition-in-specifier.cpp
===
--- test/SemaCXX/type-definition-in-specifier.cpp
+++ test/SemaCXX/type-definition-in-specifier.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fexceptions -fcxx-exceptions -verify %s
 
 struct S0;
 struct S1;
@@ -30,7 +30,7 @@
 
 void pr19018_1 (enum e19018_1 {qq} x); // expected-error{{cannot be defined in a parameter type}}
 void pr19018_1a (enum e19018_1 {qq} x); // expected-error{{cannot be defined in a parameter type}}
-e19018_1 x2;  // expected-error{{unknown type name 'e19018_1'}}
+e19018_1 x2;
 
 void pr19018_2 (enum {qq} x); // expected-error{{cannot be defined in a parameter type}}
 void pr19018_3 (struct s19018_2 {int qq;} x); // expected-error{{cannot be defined in a parameter type}}
@@ -53,14 +53,19 @@
 
 struct s19018b {
   void func1 (enum en_2 {qq} x); // expected-error{{cannot be defined in a parameter type}}
-  en_2 x1;  // expected-error{{unknown type name 'en_2'}}
+  en_2 x1;
   void func2 (enum en_3 {qq} x); // expected-error{{cannot be defined in a parameter type}}
-  enum en_3 x2; // expected-error{{ISO C++ forbids forward references to 'enum' types}} \
-// expected-error{{field has incomplete type 'enum en_3'}} \
-// expected-note{{forward declaration of 'en_3'}}
+  enum en_3 x2;
 };
 
 struct pr18963 {
-  short bar5 (struct foo4 {} bar2); // expected-error{{'foo4' cannot be defined in a parameter type}}
-  long foo5 (float foo6 = foo4);  // expected-error{{use of undeclared identifier 'foo4'}}
+  short bar5 (struct foo4 {} bar2); // expected-error{{'foo4' cannot be defined in a parameter type}} \
+// expected-note{{declared here}}
+
+  long foo5 (float foo6 = foo4);  // expected-error{{'foo4' does not refer to a value}}
 };
+
+// expected-error@+2 {{cannot be defined in a parameter type}}
+// expected-note@+1 {{previous definition is here}}
+void func_with_eh_and_type(struct type_in_eh {} o) throw(int) {}
+struct type_in_eh {}; // expected-error {{redefinition of 'type_in_eh'}}
Index: test/Sema/decl-in-prototype.c
===
--- test/Sema/decl-in-prototype.c
+++ test/Sema/decl-in-prototype.c
@@ -1,13 +1,19 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
+#define SA(n, c) int arr##n[(c) ? 1 : -1] = {}
+
 const int AA = 5;
 
 int f1(enum {AA,BB} E) { // expected-warning {{will not be visible outside of this function}}
-return BB;
+  SA(1, AA == 0);
+  SA(2, BB == 1);
+  return BB;
 }
 
 int f2(enum {AA=7,BB} E) { // expected-warning {{will not be visible outside of this function}}
-return AA;
+  SA(1, AA == 7);
+  SA(2, BB == 8);
+  return AA;
 }
 
 struct a {
@@ -38,3 +44,11 @@
 
 // Only warn once, even if we create two declarations.
 void f(struct q *, struct __attribute__((aligned(4))) q *); // expected-warning {{will not be visible outside}}
+
+// This enum inside the function pointer parameter shouldn't leak into the
+// function.
+enum { BB = 0 };
+void enum_in_fun_in_fun(void (*fp)(enum { AA, BB } e)) { // expected-warning {{will not be visible}}
+  SA(1, AA == 5);
+  SA(2, BB == 0);
+}
Index: test/PCH/decl-in-prototype.c
===
--- /dev/null
+++ test/PCH/decl-in-prototype.c
@@ -0,0 +1,27 @@
+// Test that we serialize the enum decl in the function prototype somehow.
+// These decls aren't serialized quite the same way as parameters.
+
+// Test this without pch.
+// RUN: %clang_cc1 -include %s -emit-llvm -o - %s | FileCheck %s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: define i32 @main()
+// CHECK:   ret i32 1
+
+#ifndef HEADER
+#define HEADER
+
+static inline __attribute__((always_inline)) f(enum { x, y } p) {
+  return y;
+}
+
+#else
+
+int main() {
+  return f(0);
+}
+
+#endif
Index: test/Misc/ast-dump-decl.c
===
--- test/Misc/ast-dump-decl.c
+++ test/Misc/ast-dump-decl.c
@@ -106,12 +106,16 @@
   return x;
 }
 // CHECK:  FunctionDecl{{.*}} TestFunctionDecl 'int (int, enum {{.*}})'
-// CHECK-NEXT:   EnumDecl

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

2016-12-07 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 80682.
rnk marked an inline comment as done.
rnk added a comment.

- Allow more decls in prototypes, dump fd decls


https://reviews.llvm.org/D27279

Files:
  include/clang/AST/Decl.h
  include/clang/Sema/DeclSpec.h
  include/clang/Sema/Sema.h
  lib/AST/ASTDumper.cpp
  lib/AST/Decl.cpp
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaType.cpp
  test/Misc/ast-dump-decl.c
  test/PCH/decl-in-prototype.c
  test/Sema/decl-in-prototype.c
  test/SemaCXX/type-definition-in-specifier.cpp

Index: test/SemaCXX/type-definition-in-specifier.cpp
===
--- test/SemaCXX/type-definition-in-specifier.cpp
+++ test/SemaCXX/type-definition-in-specifier.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fexceptions -fcxx-exceptions -verify %s
 
 struct S0;
 struct S1;
@@ -64,3 +64,6 @@
   short bar5 (struct foo4 {} bar2); // expected-error{{'foo4' cannot be defined in a parameter type}}
   long foo5 (float foo6 = foo4);  // expected-error{{use of undeclared identifier 'foo4'}}
 };
+
+void func_with_eh_and_type(struct type_in_eh {} o) throw(int) {} // expected-error{{cannot be defined in a parameter type}}
+struct type_in_eh {}; // no diagnostics
Index: test/Sema/decl-in-prototype.c
===
--- test/Sema/decl-in-prototype.c
+++ test/Sema/decl-in-prototype.c
@@ -1,13 +1,19 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
+#define SA(n, c) int arr##n[(c) ? 1 : -1] = {}
+
 const int AA = 5;
 
 int f1(enum {AA,BB} E) { // expected-warning {{will not be visible outside of this function}}
-return BB;
+  SA(1, AA == 0);
+  SA(2, BB == 1);
+  return BB;
 }
 
 int f2(enum {AA=7,BB} E) { // expected-warning {{will not be visible outside of this function}}
-return AA;
+  SA(1, AA == 7);
+  SA(2, BB == 8);
+  return AA;
 }
 
 struct a {
@@ -38,3 +44,11 @@
 
 // Only warn once, even if we create two declarations.
 void f(struct q *, struct __attribute__((aligned(4))) q *); // expected-warning {{will not be visible outside}}
+
+// This enum inside the function pointer parameter shouldn't leak into the
+// function.
+enum { BB = 0 };
+void enum_in_fun_in_fun(void (*fp)(enum { AA, BB } e)) { // expected-warning {{will not be visible}}
+  SA(1, AA == 5);
+  SA(2, BB == 0);
+}
Index: test/PCH/decl-in-prototype.c
===
--- /dev/null
+++ test/PCH/decl-in-prototype.c
@@ -0,0 +1,27 @@
+// Test that we serialize the enum decl in the function prototype somehow.
+// These decls aren't serialized quite the same way as parameters.
+
+// Test this without pch.
+// RUN: %clang_cc1 -include %s -emit-llvm -o - %s | FileCheck %s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: define i32 @main()
+// CHECK:   ret i32 1
+
+#ifndef HEADER
+#define HEADER
+
+static inline __attribute__((always_inline)) f(enum { x, y } p) {
+  return y;
+}
+
+#else
+
+int main() {
+  return f(0);
+}
+
+#endif
Index: test/Misc/ast-dump-decl.c
===
--- test/Misc/ast-dump-decl.c
+++ test/Misc/ast-dump-decl.c
@@ -106,12 +106,19 @@
   return x;
 }
 // CHECK:  FunctionDecl{{.*}} TestFunctionDecl 'int (int, enum {{.*}})'
+// CHECK-NEXT:   EnumConstantDecl{{.*}} e
+// CHECK-NEXT:   ParmVarDecl{{.*}} x
+// CHECK-NEXT:   ParmVarDecl{{.*}} y
+// CHECK-NEXT:   CompoundStmt
+
+int TestFunctionDecl2(enum Enum { e } x) { return x; }
+// CHECK:  FunctionDecl{{.*}} TestFunctionDecl2 'int (enum {{.*}})'
 // CHECK-NEXT:   EnumDecl
 // CHECK-NEXT: EnumConstantDecl{{.*}} e
 // CHECK-NEXT:   ParmVarDecl{{.*}} x
-// CHECK-NEXT:   ParmVarDecl{{.*}} y
 // CHECK-NEXT:   CompoundStmt
 
+
 int TestFunctionDeclProto(int x);
 // CHECK:  FunctionDecl{{.*}} TestFunctionDeclProto 'int (int)'
 // CHECK-NEXT:   ParmVarDecl{{.*}} x
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -718,6 +718,7 @@
   /*NumExceptions=*/0,
   /*NoexceptExpr=*/nullptr,
   /*ExceptionSpecTokens=*/nullptr,
+  /*DeclsInPrototype=*/None,
   loc, loc, declarator));
 
   // For consistency, make sure the state still has us as processing
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -8222,8 +8222,9 @@
   // Copy the parameter declarations from the declarator D to the function
   // declaration NewFD, if they are available.  First scavenge them into Params.
   SmallVector Params;
-  if (D.isFunctionDeclarator()) {
-DeclaratorChunk::FunctionTypeInfo  = 

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

2016-12-07 Thread Reid Kleckner via Phabricator via cfe-commits
rnk marked an inline comment as done.
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;
+

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.



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

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?


https://reviews.llvm.org/D27279



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-12-07 Thread Richard Smith via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: include/clang/Sema/DeclSpec.h:1240
+/// in the prototype. These are generally tag types or enumerators.
+unsigned NumDeclsInPrototype : 8;
+

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++.



Comment at: include/clang/Sema/DeclSpec.h:1311
+/// for lookup inside the function body, if one exists.
+NamedDecl **DeclsInPrototype;
+

Might make sense to put this into the preceding union, if we care about how big 
this type is.



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

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.


https://reviews.llvm.org/D27279



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-12-07 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 80650.
rnk added a comment.

- Remove an assert and test that edge case


https://reviews.llvm.org/D27279

Files:
  include/clang/AST/Decl.h
  include/clang/Sema/DeclSpec.h
  include/clang/Sema/Sema.h
  lib/AST/ASTDumper.cpp
  lib/AST/Decl.cpp
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaType.cpp
  test/Misc/ast-dump-decl.c
  test/PCH/decl-in-prototype.c
  test/Sema/decl-in-prototype.c
  test/SemaCXX/type-definition-in-specifier.cpp

Index: test/SemaCXX/type-definition-in-specifier.cpp
===
--- test/SemaCXX/type-definition-in-specifier.cpp
+++ test/SemaCXX/type-definition-in-specifier.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fexceptions -fcxx-exceptions -verify %s
 
 struct S0;
 struct S1;
@@ -64,3 +64,6 @@
   short bar5 (struct foo4 {} bar2); // expected-error{{'foo4' cannot be defined in a parameter type}}
   long foo5 (float foo6 = foo4);  // expected-error{{use of undeclared identifier 'foo4'}}
 };
+
+void func_with_eh_and_type(struct type_in_eh {} o) throw(int) {} // expected-error{{cannot be defined in a parameter type}}
+struct type_in_eh {}; // no diagnostics
Index: test/Sema/decl-in-prototype.c
===
--- test/Sema/decl-in-prototype.c
+++ test/Sema/decl-in-prototype.c
@@ -1,13 +1,19 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
+#define SA(n, c) int arr##n[(c) ? 1 : -1] = {}
+
 const int AA = 5;
 
 int f1(enum {AA,BB} E) { // expected-warning {{will not be visible outside of this function}}
-return BB;
+  SA(1, AA == 0);
+  SA(2, BB == 1);
+  return BB;
 }
 
 int f2(enum {AA=7,BB} E) { // expected-warning {{will not be visible outside of this function}}
-return AA;
+  SA(1, AA == 7);
+  SA(2, BB == 8);
+  return AA;
 }
 
 struct a {
@@ -38,3 +44,11 @@
 
 // Only warn once, even if we create two declarations.
 void f(struct q *, struct __attribute__((aligned(4))) q *); // expected-warning {{will not be visible outside}}
+
+// This enum inside the function pointer parameter shouldn't leak into the
+// function.
+enum { BB = 0 };
+void enum_in_fun_in_fun(void (*fp)(enum { AA, BB } e)) { // expected-warning {{will not be visible}}
+  SA(1, AA == 5);
+  SA(2, BB == 0);
+}
Index: test/PCH/decl-in-prototype.c
===
--- /dev/null
+++ test/PCH/decl-in-prototype.c
@@ -0,0 +1,27 @@
+// Test that we serialize the enum decl in the function prototype somehow.
+// These decls aren't serialized quite the same way as parameters.
+
+// Test this without pch.
+// RUN: %clang_cc1 -include %s -emit-llvm -o - %s | FileCheck %s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: define i32 @main()
+// CHECK:   ret i32 1
+
+#ifndef HEADER
+#define HEADER
+
+static inline __attribute__((always_inline)) f(enum { x, y } p) {
+  return y;
+}
+
+#else
+
+int main() {
+  return f(0);
+}
+
+#endif
Index: test/Misc/ast-dump-decl.c
===
--- test/Misc/ast-dump-decl.c
+++ test/Misc/ast-dump-decl.c
@@ -106,8 +106,6 @@
   return x;
 }
 // CHECK:  FunctionDecl{{.*}} TestFunctionDecl 'int (int, enum {{.*}})'
-// CHECK-NEXT:   EnumDecl
-// CHECK-NEXT: EnumConstantDecl{{.*}} e
 // CHECK-NEXT:   ParmVarDecl{{.*}} x
 // CHECK-NEXT:   ParmVarDecl{{.*}} y
 // CHECK-NEXT:   CompoundStmt
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -718,6 +718,7 @@
   /*NumExceptions=*/0,
   /*NoexceptExpr=*/nullptr,
   /*ExceptionSpecTokens=*/nullptr,
+  /*DeclsInPrototype=*/None,
   loc, loc, declarator));
 
   // For consistency, make sure the state still has us as processing
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -8222,8 +8222,9 @@
   // Copy the parameter declarations from the declarator D to the function
   // declaration NewFD, if they are available.  First scavenge them into Params.
   SmallVector Params;
-  if (D.isFunctionDeclarator()) {
-DeclaratorChunk::FunctionTypeInfo  = D.getFunctionTypeInfo();
+  unsigned FTIIdx;
+  if (D.isFunctionDeclarator(FTIIdx)) {
+DeclaratorChunk::FunctionTypeInfo  = D.getTypeObject(FTIIdx).Fun;
 
 // Check for C99 6.7.5.3p10 - foo(void) is a non-varargs
 // function that takes no arguments, not a function that takes a
@@ -8241,6 +8242,18 @@
   NewFD->setInvalidDecl();
   }
 }
+
+// Find all the non-parameter declarations from the prototype and move them
+// into the new function decl context 

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

2016-12-02 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

(Unrelated, but if you're looking at memory: When I was looking at it a while 
ago, IIRC a surprising amount of memory was taken up by CXXBasePaths objects, 
and just reordering fields to pack it better made that object several bytes 
smaller. IIRC I accidentally reverted my local patch for this, but if you're in 
the mood it's probably not too hard to recreate)


https://reviews.llvm.org/D27279



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2016-11-30 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision.
rnk added a reviewer: rsmith.
rnk added subscribers: cfe-commits, jmolloy.

This saves two pointers (!) from FunctionDecl that were being used for
some rare and questionable C-only functionality.  The
DeclsInPrototypeScope ArrayRef was added in r151712 in order to parse
this kind of C code:

  enum e {x, y};
  int f(enum {y, x} n) {
   return x; // should return 1, not 0
  }

The challenge is that we parse 'int f(enum {y, x} n)' it its own
function prototype scope that gets popped before we build the
FunctionDecl for 'f'. The original change was doing two questionable
things:

1. Saving all tag decls introduced in prototype scope on a TU-global

Sema variable. This is problematic when you have cases like this, where
'x' and 'y' shouldn't be visible in 'f':

  void f(void (*fp)(enum { x, y } e)) { /* no x */ }

This patch fixes that, so now 'f' can't see 'x', which is consistent
with GCC.

2. Storing the decls in FunctionDecl in ActOnFunctionDeclarator so that

they could be used in ActOnStartOfFunctionDef. This is just an
inefficient way to move information around. The AST lives forever, but
the list of non-parameter decls in prototype scope is short lived. By
moving this stuff to the Declarator, we get the right (short) lifetime.

As the original change was the author's first major Clang patch, they
can be forgiven.


https://reviews.llvm.org/D27279

Files:
  include/clang/AST/Decl.h
  include/clang/Sema/DeclSpec.h
  include/clang/Sema/Sema.h
  lib/AST/ASTDumper.cpp
  lib/AST/Decl.cpp
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaType.cpp
  test/Misc/ast-dump-decl.c
  test/PCH/decl-in-prototype.c
  test/Sema/decl-in-prototype.c

Index: test/Sema/decl-in-prototype.c
===
--- test/Sema/decl-in-prototype.c
+++ test/Sema/decl-in-prototype.c
@@ -1,13 +1,19 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
+#define SA(n, c) int arr##n[(c) ? 1 : -1] = {}
+
 const int AA = 5;
 
 int f1(enum {AA,BB} E) { // expected-warning {{will not be visible outside of this function}}
-return BB;
+  SA(1, AA == 0);
+  SA(2, BB == 1);
+  return BB;
 }
 
 int f2(enum {AA=7,BB} E) { // expected-warning {{will not be visible outside of this function}}
-return AA;
+  SA(1, AA == 7);
+  SA(2, BB == 8);
+  return AA;
 }
 
 struct a {
@@ -38,3 +44,11 @@
 
 // Only warn once, even if we create two declarations.
 void f(struct q *, struct __attribute__((aligned(4))) q *); // expected-warning {{will not be visible outside}}
+
+// This enum inside the function pointer parameter shouldn't leak into the
+// function.
+enum { BB = 0 };
+void enum_in_fun_in_fun(void (*fp)(enum { AA, BB } e)) { // expected-warning {{will not be visible}}
+  SA(1, AA == 5);
+  SA(2, BB == 0);
+}
Index: test/PCH/decl-in-prototype.c
===
--- /dev/null
+++ test/PCH/decl-in-prototype.c
@@ -0,0 +1,27 @@
+// Test that we serialize the enum decl in the function prototype somehow.
+// These decls aren't serialized quite the same way as parameters.
+
+// Test this without pch.
+// RUN: %clang_cc1 -include %s -emit-llvm -o - %s | FileCheck %s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: define i32 @main()
+// CHECK:   ret i32 1
+
+#ifndef HEADER
+#define HEADER
+
+static inline __attribute__((always_inline)) f(enum { x, y } p) {
+  return y;
+}
+
+#else
+
+int main() {
+  return f(0);
+}
+
+#endif
Index: test/Misc/ast-dump-decl.c
===
--- test/Misc/ast-dump-decl.c
+++ test/Misc/ast-dump-decl.c
@@ -106,8 +106,6 @@
   return x;
 }
 // CHECK:  FunctionDecl{{.*}} TestFunctionDecl 'int (int, enum {{.*}})'
-// CHECK-NEXT:   EnumDecl
-// CHECK-NEXT: EnumConstantDecl{{.*}} e
 // CHECK-NEXT:   ParmVarDecl{{.*}} x
 // CHECK-NEXT:   ParmVarDecl{{.*}} y
 // CHECK-NEXT:   CompoundStmt
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -718,6 +718,7 @@
   /*NumExceptions=*/0,
   /*NoexceptExpr=*/nullptr,
   /*ExceptionSpecTokens=*/nullptr,
+  /*DeclsInPrototype=*/None,
   loc, loc, declarator));
 
   // For consistency, make sure the state still has us as processing
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -8222,8 +8222,9 @@
   // Copy the parameter declarations from the declarator D to the function
   // declaration NewFD, if they are available.  First scavenge them into Params.
   SmallVector Params;
-  if (D.isFunctionDeclarator()) {
-DeclaratorChunk::FunctionTypeInfo  = D.getFunctionTypeInfo();
+