john.brawn created this revision.
john.brawn added reviewers: faisalv, rsmith, jyknight.
john.brawn added a subscriber: cfe-commits.
john.brawn set the repository for this revision to rL LLVM.

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.

Repository:
  rL LLVM

http://reviews.llvm.org/D20732

Files:
  include/clang/AST/DeclBase.h
  include/clang/AST/LambdaCapture.h
  lib/AST/DeclBase.cpp
  lib/AST/ExprCXX.cpp

Index: lib/AST/ExprCXX.cpp
===================================================================
--- lib/AST/ExprCXX.cpp
+++ lib/AST/ExprCXX.cpp
@@ -818,13 +818,10 @@
   }
 }
 
-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)
@@ -836,7 +833,7 @@
     // Fall through
   case LCK_This:
     assert(!Var && "'this' capture cannot have a variable!");
-    CapturedEntityAndBits.setPointer(&ThisSentinel);
+    Bits |= Capture_This;
     break;
 
   case LCK_ByCopy:
@@ -847,19 +844,16 @@
     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;
 }
Index: lib/AST/DeclBase.cpp
===================================================================
--- lib/AST/DeclBase.cpp
+++ lib/AST/DeclBase.cpp
@@ -46,7 +46,7 @@
 }
 
 #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 @@
                          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 @@
     // 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;
Index: include/clang/AST/LambdaCapture.h
===================================================================
--- include/clang/AST/LambdaCapture.h
+++ include/clang/AST/LambdaCapture.h
@@ -33,19 +33,21 @@
     /// 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 @@
   /// \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 @@
   /// (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
Index: include/clang/AST/DeclBase.h
===================================================================
--- include/clang/AST/DeclBase.h
+++ include/clang/AST/DeclBase.h
@@ -73,13 +73,10 @@
 ///
 /// 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,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to