Don't use static variables in LambdaCapture

When static variables are used in inline functions in header files anything that
uses that function ends up with a reference to the variable. Because
RecursiveASTVisitor uses the inline functions in LambdaCapture that use static
variables any AST plugin that uses RecursiveASTVisitor, such as the
PrintFunctionNames example, ends up with a reference to these variables. This is
bad on Windows when building with MSVC with LLVM_EXPORT_SYMBOLS_FOR_PLUGINS=ON
as variables used across a DLL boundary need to be explicitly dllimported in
the DLL using them.

This patch avoids that by adjusting LambdaCapture to be similar to before
r263921, with a capture of either 'this' or a VLA represented by a null Decl
pointer in DeclAndBits with an extra flag added to the bits to distinguish
between the two. This requires the use of an extra bit, and while Decl does
happen to be sufficiently aligned to allow this it's done in a way that means
PointerIntPair doesn't realise it and gives an assertion failure. Therefore I
also adjust Decl slightly to use LLVM_ALIGNAS to allow this.

Differential Revision: http://reviews.llvm.org/D20732


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@272788 91177308-0d34-0410-b5e6-96231b3b80d8

4 files changed, 31 insertions, 38 deletions

diff --git a/include/clang/AST/DeclBase.h b/include/clang/AST/DeclBase.h
index 26cbd3802f..ec8bb3aaa3 100644
--- a/include/clang/AST/DeclBase.h
+++ b/include/clang/AST/DeclBase.h
@@ -73,13 +73,10 @@ namespace clang {
 ///
 /// Note: There are objects tacked on before the *beginning* of Decl
 /// (and its subclasses) in its Decl::operator new(). Proper alignment
-/// of all subclasses (not requiring more than DeclObjAlignment) is
+/// of all subclasses (not requiring more than the alignment of Decl) is
 /// asserted in DeclBase.cpp.
-class Decl {
+class LLVM_ALIGNAS(/*alignof(uint64_t)*/ 8) Decl {
 public:
-  /// \brief Alignment guaranteed when allocating Decl and any subtypes.
-  enum { DeclObjAlignment = llvm::AlignOf<uint64_t>::Alignment };
-
   /// \brief Lists the kind of concrete classes of Decl.
   enum Kind {
 #define DECL(DERIVED, BASE) DERIVED,
diff --git a/include/clang/AST/LambdaCapture.h b/include/clang/AST/LambdaCapture.h
index c71de96299..6517d65687 100644
--- a/include/clang/AST/LambdaCapture.h
+++ b/include/clang/AST/LambdaCapture.h
@@ -33,19 +33,21 @@ class LambdaCapture {
     /// given capture was by-copy.
     ///
     /// This includes the case of a non-reference init-capture.
-    Capture_ByCopy = 0x02
+    Capture_ByCopy = 0x02,
+
+    /// \brief Flag used by the Capture class to distinguish between a capture
+    /// of '*this' and a capture of a VLA type.
+    Capture_This = 0x04
   };
-  struct LLVM_ALIGNAS(4) OpaqueCapturedEntity {};

-  static OpaqueCapturedEntity ThisSentinel;

-  static OpaqueCapturedEntity VLASentinel;
-  
-  // Captured Entity could represent:
+
+  // Decl could represent:
   // - a VarDecl* that represents the variable that was captured or the 
   //   init-capture.
-  // - or, points to the ThisSentinel if this represents a capture of '*this'
-  //   by value or reference.
-  // - or, points to the VLASentinel if this represents a capture of a VLA type.
-  llvm::PointerIntPair<void*, 2> CapturedEntityAndBits;
+  // - or, is a nullptr and Capture_This is set in Bits if this represents a
+  //   capture of '*this' by value or reference.
+  // - or, is a nullptr and Capture_This is not set in Bits if this represents
+  //   a capture of a VLA type.
+  llvm::PointerIntPair<Decl*, 3> DeclAndBits;
 
   SourceLocation Loc;
   SourceLocation EllipsisLoc;
@@ -79,21 +81,20 @@ public:
   /// \brief Determine whether this capture handles the C++ \c this
   /// pointer.
   bool capturesThis() const {
-    return CapturedEntityAndBits.getPointer() == &ThisSentinel;
+    return DeclAndBits.getPointer() == nullptr &&
+          (DeclAndBits.getInt() & Capture_This);
   }
 
   /// \brief Determine whether this capture handles a variable.
   bool capturesVariable() const {
-    void *Ptr = CapturedEntityAndBits.getPointer();
-    if (Ptr != &ThisSentinel && Ptr != &VLASentinel)
-      return dyn_cast_or_null<VarDecl>(static_cast<Decl *>(Ptr));
-    return false;
+    return dyn_cast_or_null<VarDecl>(DeclAndBits.getPointer());
   }
 
   /// \brief Determine whether this captures a variable length array bound
   /// expression.
   bool capturesVLAType() const {
-    return CapturedEntityAndBits.getPointer() == &VLASentinel;
+    return DeclAndBits.getPointer() == nullptr &&
+           !(DeclAndBits.getInt() & Capture_This);
   }
 
   /// \brief Retrieve the declaration of the local variable being
@@ -103,13 +104,13 @@ public:
   /// (other than a capture of \c this).
   VarDecl *getCapturedVar() const {
     assert(capturesVariable() && "No variable available for capture");
-    return static_cast<VarDecl *>(CapturedEntityAndBits.getPointer());
+    return static_cast<VarDecl *>(DeclAndBits.getPointer());
   }
 
   /// \brief Determine whether this was an implicit capture (not
   /// written between the square brackets introducing the lambda).
   bool isImplicit() const {
-    return CapturedEntityAndBits.getInt() & Capture_Implicit;
+    return DeclAndBits.getInt() & Capture_Implicit;
   }
 
   /// \brief Determine whether this was an explicit capture (written
diff --git a/lib/AST/DeclBase.cpp b/lib/AST/DeclBase.cpp
index dd0bab811e..f68ca602c2 100644
--- a/lib/AST/DeclBase.cpp
+++ b/lib/AST/DeclBase.cpp
@@ -46,7 +46,7 @@ void Decl::updateOutOfDate(IdentifierInfo &II) const {
 }
 
 #define DECL(DERIVED, BASE)                                                    \
-  static_assert(Decl::DeclObjAlignment >=                                      \
+  static_assert(llvm::AlignOf<Decl>::Alignment >=                              \
                     llvm::AlignOf<DERIVED##Decl>::Alignment,                   \
                 "Alignment sufficient after objects prepended to " #DERIVED);
 #define ABSTRACT_DECL(DECL)
@@ -56,7 +56,7 @@ void *Decl::operator new(std::size_t Size, const ASTContext &Context,
                          unsigned ID, std::size_t Extra) {
   // Allocate an extra 8 bytes worth of storage, which ensures that the
   // resulting pointer will still be 8-byte aligned.
-  static_assert(sizeof(unsigned) * 2 >= DeclObjAlignment,
+  static_assert(sizeof(unsigned) * 2 >= llvm::AlignOf<Decl>::Alignment,
                 "Decl won't be misaligned");
   void *Start = Context.Allocate(Size + Extra + 8);
   void *Result = (char*)Start + 8;
@@ -81,7 +81,8 @@ void *Decl::operator new(std::size_t Size, const ASTContext &Ctx,
     // Ensure required alignment of the resulting object by adding extra
     // padding at the start if required.
     size_t ExtraAlign =
-        llvm::OffsetToAlignment(sizeof(Module *), DeclObjAlignment);
+        llvm::OffsetToAlignment(sizeof(Module *),
+                                llvm::AlignOf<Decl>::Alignment);
     char *Buffer = reinterpret_cast<char *>(
         ::operator new(ExtraAlign + sizeof(Module *) + Size + Extra, Ctx));
     Buffer += ExtraAlign;
diff --git a/lib/AST/ExprCXX.cpp b/lib/AST/ExprCXX.cpp
index 09401b704e..031332d7ce 100644
--- a/lib/AST/ExprCXX.cpp
+++ b/lib/AST/ExprCXX.cpp
@@ -808,13 +808,10 @@ CXXConstructExpr::CXXConstructExpr(const ASTContext &C, StmtClass SC,
   }
 }
 
-LambdaCapture::OpaqueCapturedEntity LambdaCapture::ThisSentinel;
-LambdaCapture::OpaqueCapturedEntity LambdaCapture::VLASentinel;
-
 LambdaCapture::LambdaCapture(SourceLocation Loc, bool Implicit,
                              LambdaCaptureKind Kind, VarDecl *Var,
                              SourceLocation EllipsisLoc)
-  : CapturedEntityAndBits(Var, 0), Loc(Loc), EllipsisLoc(EllipsisLoc)
+  : DeclAndBits(Var, 0), Loc(Loc), EllipsisLoc(EllipsisLoc)
 {
   unsigned Bits = 0;
   if (Implicit)
@@ -826,7 +823,7 @@ LambdaCapture::LambdaCapture(SourceLocation Loc, bool Implicit,
     // Fall through
   case LCK_This:
     assert(!Var && "'this' capture cannot have a variable!");
-    CapturedEntityAndBits.setPointer(&ThisSentinel);
+    Bits |= Capture_This;
     break;
 
   case LCK_ByCopy:
@@ -837,19 +834,16 @@ LambdaCapture::LambdaCapture(SourceLocation Loc, bool Implicit,
     break;
   case LCK_VLAType:
     assert(!Var && "VLA type capture cannot have a variable!");
-    CapturedEntityAndBits.setPointer(&VLASentinel);
     break;
   }
-  CapturedEntityAndBits.setInt(Bits);
+  DeclAndBits.setInt(Bits);
 }
 
 LambdaCaptureKind LambdaCapture::getCaptureKind() const {
-  void *Ptr = CapturedEntityAndBits.getPointer();
-  if (Ptr == &VLASentinel)
+  if (capturesVLAType())
     return LCK_VLAType;
-  const unsigned Bits = CapturedEntityAndBits.getInt();
-  bool CapByCopy = Bits & Capture_ByCopy;
-  if (Ptr == &ThisSentinel)
+  bool CapByCopy = DeclAndBits.getInt() & Capture_ByCopy;
+  if (capturesThis())
     return CapByCopy ? LCK_StarThis : LCK_This;
   return CapByCopy ? LCK_ByCopy : LCK_ByRef;
 }