r272788 - Don't use static variables in LambdaCapture
John Brawn via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 15 07:14:52 PDT 2016
Author: john.brawn
Date: Wed Jun 15 09:14:51 2016
New Revision: 272788
URL: http://llvm.org/viewvc/llvm-project?rev=272788&view=rev
Log:
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
Modified:
cfe/trunk/include/clang/AST/DeclBase.h
cfe/trunk/include/clang/AST/LambdaCapture.h
cfe/trunk/lib/AST/DeclBase.cpp
cfe/trunk/lib/AST/ExprCXX.cpp
Modified: cfe/trunk/include/clang/AST/DeclBase.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=272788&r1=272787&r2=272788&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclBase.h (original)
+++ cfe/trunk/include/clang/AST/DeclBase.h Wed Jun 15 09:14:51 2016
@@ -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,
Modified: cfe/trunk/include/clang/AST/LambdaCapture.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/LambdaCapture.h?rev=272788&r1=272787&r2=272788&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/LambdaCapture.h (original)
+++ cfe/trunk/include/clang/AST/LambdaCapture.h Wed Jun 15 09:14:51 2016
@@ -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
Modified: cfe/trunk/lib/AST/DeclBase.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=272788&r1=272787&r2=272788&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclBase.cpp (original)
+++ cfe/trunk/lib/AST/DeclBase.cpp Wed Jun 15 09:14:51 2016
@@ -46,7 +46,7 @@ void Decl::updateOutOfDate(IdentifierInf
}
#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 Siz
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 Siz
// 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;
Modified: cfe/trunk/lib/AST/ExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprCXX.cpp?rev=272788&r1=272787&r2=272788&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ExprCXX.cpp (original)
+++ cfe/trunk/lib/AST/ExprCXX.cpp Wed Jun 15 09:14:51 2016
@@ -808,13 +808,10 @@ CXXConstructExpr::CXXConstructExpr(const
}
}
-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(SourceLocat
// 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(SourceLocat
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;
}
More information about the cfe-commits
mailing list