r256983 - Only instantiate a default argument once.
John McCall via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 6 14:34:54 PST 2016
Author: rjmccall
Date: Wed Jan 6 16:34:54 2016
New Revision: 256983
URL: http://llvm.org/viewvc/llvm-project?rev=256983&view=rev
Log:
Only instantiate a default argument once.
By storing the instantiated expression back in the ParmVarDecl,
we remove the last need for separately storing the sub-expression
of a CXXDefaultArgExpr. This makes PCH/Modules merging quite
simple: CXXDefaultArgExpr records are serialized as references
to the ParmVarDecl, and we ignore redundant attempts to overwrite
the instantiated expression.
This has some extremely marginal impact on user-facing semantics.
However, the major effect is that it avoids IRGen errors about
conflicting definitions due to lambdas in the argument being
instantiated multiple times while sharing the same mangling.
It should also slightly improve memory usage and module file size.
rdar://23810407
Added:
cfe/trunk/test/PCH/chain-default-argument-instantiation.cpp
Modified:
cfe/trunk/include/clang/AST/ASTMutationListener.h
cfe/trunk/include/clang/AST/ExprCXX.h
cfe/trunk/include/clang/Serialization/ASTWriter.h
cfe/trunk/lib/AST/ExprCXX.cpp
cfe/trunk/lib/Frontend/MultiplexConsumer.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Serialization/ASTCommon.h
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
cfe/trunk/test/SemaCXX/conversion.cpp
cfe/trunk/test/SemaTemplate/default-arguments-cxx0x.cpp
Modified: cfe/trunk/include/clang/AST/ASTMutationListener.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTMutationListener.h?rev=256983&r1=256982&r2=256983&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/ASTMutationListener.h (original)
+++ cfe/trunk/include/clang/AST/ASTMutationListener.h Wed Jan 6 16:34:54 2016
@@ -29,6 +29,7 @@ namespace clang {
class ObjCContainerDecl;
class ObjCInterfaceDecl;
class ObjCPropertyDecl;
+ class ParmVarDecl;
class QualType;
class RecordDecl;
class TagDecl;
@@ -88,6 +89,9 @@ public:
/// \brief A function template's definition was instantiated.
virtual void FunctionDefinitionInstantiated(const FunctionDecl *D) {}
+ /// \brief A default argument was instantiated.
+ virtual void DefaultArgumentInstantiated(const ParmVarDecl *D) {}
+
/// \brief A new objc category class was added for an interface.
virtual void AddedObjCCategoryToInterface(const ObjCCategoryDecl *CatD,
const ObjCInterfaceDecl *IFD) {}
Modified: cfe/trunk/include/clang/AST/ExprCXX.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=256983&r1=256982&r2=256983&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/ExprCXX.h (original)
+++ cfe/trunk/include/clang/AST/ExprCXX.h Wed Jan 6 16:34:54 2016
@@ -951,15 +951,9 @@ public:
/// This wraps up a function call argument that was created from the
/// corresponding parameter's default argument, when the call did not
/// explicitly supply arguments for all of the parameters.
-class CXXDefaultArgExpr final
- : public Expr,
- private llvm::TrailingObjects<CXXDefaultArgExpr, Expr *> {
+class CXXDefaultArgExpr final : public Expr {
/// \brief The parameter whose default is being used.
- ///
- /// When the bit is set, the subexpression is stored after the
- /// CXXDefaultArgExpr itself. When the bit is clear, the parameter's
- /// actual default expression is the subexpression.
- llvm::PointerIntPair<ParmVarDecl *, 1, bool> Param;
+ ParmVarDecl *Param;
/// \brief The location where the default argument expression was used.
SourceLocation Loc;
@@ -971,16 +965,7 @@ class CXXDefaultArgExpr final
: param->getDefaultArg()->getType(),
param->getDefaultArg()->getValueKind(),
param->getDefaultArg()->getObjectKind(), false, false, false, false),
- Param(param, false), Loc(Loc) { }
-
- CXXDefaultArgExpr(StmtClass SC, SourceLocation Loc, ParmVarDecl *param,
- Expr *SubExpr)
- : Expr(SC, SubExpr->getType(),
- SubExpr->getValueKind(), SubExpr->getObjectKind(),
- false, false, false, false),
- Param(param, true), Loc(Loc) {
- *getTrailingObjects<Expr *>() = SubExpr;
- }
+ Param(param), Loc(Loc) { }
public:
CXXDefaultArgExpr(EmptyShell Empty) : Expr(CXXDefaultArgExprClass, Empty) {}
@@ -992,24 +977,15 @@ public:
return new (C) CXXDefaultArgExpr(CXXDefaultArgExprClass, Loc, Param);
}
- // \p Param is the parameter whose default argument is used by this
- // expression, and \p SubExpr is the expression that will actually be used.
- static CXXDefaultArgExpr *Create(const ASTContext &C, SourceLocation Loc,
- ParmVarDecl *Param, Expr *SubExpr);
-
// Retrieve the parameter that the argument was created from.
- const ParmVarDecl *getParam() const { return Param.getPointer(); }
- ParmVarDecl *getParam() { return Param.getPointer(); }
+ const ParmVarDecl *getParam() const { return Param; }
+ ParmVarDecl *getParam() { return Param; }
// Retrieve the actual argument to the function call.
const Expr *getExpr() const {
- if (Param.getInt())
- return *getTrailingObjects<Expr *>();
return getParam()->getDefaultArg();
}
Expr *getExpr() {
- if (Param.getInt())
- return *getTrailingObjects<Expr *>();
return getParam()->getDefaultArg();
}
@@ -1033,7 +1009,6 @@ public:
return child_range(child_iterator(), child_iterator());
}
- friend TrailingObjects;
friend class ASTStmtReader;
friend class ASTStmtWriter;
};
Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=256983&r1=256982&r2=256983&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTWriter.h Wed Jan 6 16:34:54 2016
@@ -871,6 +871,7 @@ public:
const FunctionDecl *Delete) override;
void CompletedImplicitDefinition(const FunctionDecl *D) override;
void StaticDataMemberInstantiated(const VarDecl *D) override;
+ void DefaultArgumentInstantiated(const ParmVarDecl *D) override;
void FunctionDefinitionInstantiated(const FunctionDecl *D) override;
void AddedObjCCategoryToInterface(const ObjCCategoryDecl *CatD,
const ObjCInterfaceDecl *IFD) override;
Modified: cfe/trunk/lib/AST/ExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprCXX.cpp?rev=256983&r1=256982&r2=256983&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ExprCXX.cpp (original)
+++ cfe/trunk/lib/AST/ExprCXX.cpp Wed Jan 6 16:34:54 2016
@@ -763,14 +763,6 @@ const IdentifierInfo *UserDefinedLiteral
return cast<FunctionDecl>(getCalleeDecl())->getLiteralIdentifier();
}
-CXXDefaultArgExpr *
-CXXDefaultArgExpr::Create(const ASTContext &C, SourceLocation Loc,
- ParmVarDecl *Param, Expr *SubExpr) {
- void *Mem = C.Allocate(totalSizeToAlloc<Expr *>(1));
- return new (Mem) CXXDefaultArgExpr(CXXDefaultArgExprClass, Loc, Param,
- SubExpr);
-}
-
CXXDefaultInitExpr::CXXDefaultInitExpr(const ASTContext &C, SourceLocation Loc,
FieldDecl *Field, QualType T)
: Expr(CXXDefaultInitExprClass, T.getNonLValueExprType(C),
Modified: cfe/trunk/lib/Frontend/MultiplexConsumer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/MultiplexConsumer.cpp?rev=256983&r1=256982&r2=256983&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/MultiplexConsumer.cpp (original)
+++ cfe/trunk/lib/Frontend/MultiplexConsumer.cpp Wed Jan 6 16:34:54 2016
@@ -119,6 +119,7 @@ public:
const FunctionDecl *Delete) override;
void CompletedImplicitDefinition(const FunctionDecl *D) override;
void StaticDataMemberInstantiated(const VarDecl *D) override;
+ void DefaultArgumentInstantiated(const ParmVarDecl *D) override;
void AddedObjCCategoryToInterface(const ObjCCategoryDecl *CatD,
const ObjCInterfaceDecl *IFD) override;
void FunctionDefinitionInstantiated(const FunctionDecl *D) override;
@@ -193,6 +194,11 @@ void MultiplexASTMutationListener::Stati
for (size_t i = 0, e = Listeners.size(); i != e; ++i)
Listeners[i]->StaticDataMemberInstantiated(D);
}
+void MultiplexASTMutationListener::DefaultArgumentInstantiated(
+ const ParmVarDecl *D) {
+ for (size_t i = 0, e = Listeners.size(); i != e; ++i)
+ Listeners[i]->DefaultArgumentInstantiated(D);
+}
void MultiplexASTMutationListener::AddedObjCCategoryToInterface(
const ObjCCategoryDecl *CatD,
const ObjCInterfaceDecl *IFD) {
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=256983&r1=256982&r2=256983&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Jan 6 16:34:54 2016
@@ -4315,8 +4315,15 @@ ExprResult Sema::BuildCXXDefaultArgExpr(
Expr *Arg = Result.getAs<Expr>();
CheckCompletedExpr(Arg, Param->getOuterLocStart());
+
+ // Remember the instantiated default argument.
+ Param->setDefaultArg(Arg);
+ if (ASTMutationListener *L = getASTMutationListener()) {
+ L->DefaultArgumentInstantiated(Param);
+ }
+
// Build the default argument expression.
- return CXXDefaultArgExpr::Create(Context, CallLoc, Param, Arg);
+ return CXXDefaultArgExpr::Create(Context, CallLoc, Param);
}
// If the default expression creates temporaries, we need to
Modified: cfe/trunk/lib/Serialization/ASTCommon.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTCommon.h?rev=256983&r1=256982&r2=256983&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTCommon.h (original)
+++ cfe/trunk/lib/Serialization/ASTCommon.h Wed Jan 6 16:34:54 2016
@@ -29,6 +29,7 @@ enum DeclUpdateKind {
UPD_CXX_ADDED_FUNCTION_DEFINITION,
UPD_CXX_INSTANTIATED_STATIC_DATA_MEMBER,
UPD_CXX_INSTANTIATED_CLASS_DEFINITION,
+ UPD_CXX_INSTANTIATED_DEFAULT_ARGUMENT,
UPD_CXX_RESOLVED_DTOR_DELETE,
UPD_CXX_RESOLVED_EXCEPTION_SPEC,
UPD_CXX_DEDUCED_RETURN_TYPE,
Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=256983&r1=256982&r2=256983&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Wed Jan 6 16:34:54 2016
@@ -3626,6 +3626,21 @@ void ASTDeclReader::UpdateDecl(Decl *D,
Reader.ReadSourceLocation(ModuleFile, Record, Idx));
break;
+ case UPD_CXX_INSTANTIATED_DEFAULT_ARGUMENT: {
+ auto Param = cast<ParmVarDecl>(D);
+
+ // We have to read the default argument regardless of whether we use it
+ // so that hypothetical further update records aren't messed up.
+ // TODO: Add a function to skip over the next expr record.
+ auto DefaultArg = Reader.ReadExpr(F);
+
+ // Only apply the update if the parameter still has an uninstantiated
+ // default argument.
+ if (Param->hasUninstantiatedDefaultArg())
+ Param->setDefaultArg(DefaultArg);
+ break;
+ }
+
case UPD_CXX_ADDED_FUNCTION_DEFINITION: {
FunctionDecl *FD = cast<FunctionDecl>(D);
if (Reader.PendingBodies[FD]) {
Modified: cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderStmt.cpp?rev=256983&r1=256982&r2=256983&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderStmt.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderStmt.cpp Wed Jan 6 16:34:54 2016
@@ -1364,10 +1364,7 @@ void ASTStmtReader::VisitCXXThrowExpr(CX
void ASTStmtReader::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
VisitExpr(E);
-
- assert((bool)Record[Idx] == E->Param.getInt() && "We messed up at creation ?");
- ++Idx; // HasOtherExprStored and SubExpr was handled during creation.
- E->Param.setPointer(ReadDeclAs<ParmVarDecl>(Record, Idx));
+ E->Param = ReadDeclAs<ParmVarDecl>(Record, Idx);
E->Loc = ReadSourceLocation(Record, Idx);
}
@@ -3205,16 +3202,9 @@ Stmt *ASTReader::ReadStmtFromStream(Modu
case EXPR_CXX_THROW:
S = new (Context) CXXThrowExpr(Empty);
break;
- case EXPR_CXX_DEFAULT_ARG: {
- bool HasOtherExprStored = Record[ASTStmtReader::NumExprFields];
- if (HasOtherExprStored) {
- Expr *SubExpr = ReadSubExpr();
- S = CXXDefaultArgExpr::Create(Context, SourceLocation(), nullptr,
- SubExpr);
- } else
- S = new (Context) CXXDefaultArgExpr(Empty);
+ case EXPR_CXX_DEFAULT_ARG:
+ S = new (Context) CXXDefaultArgExpr(Empty);
break;
- }
case EXPR_CXX_DEFAULT_INIT:
S = new (Context) CXXDefaultInitExpr(Empty);
break;
Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=256983&r1=256982&r2=256983&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Wed Jan 6 16:34:54 2016
@@ -4611,6 +4611,11 @@ void ASTWriter::WriteDeclUpdatesBlocks(R
AddSourceLocation(Update.getLoc(), Record);
break;
+ case UPD_CXX_INSTANTIATED_DEFAULT_ARGUMENT:
+ AddStmt(const_cast<Expr*>(
+ cast<ParmVarDecl>(Update.getDecl())->getDefaultArg()));
+ break;
+
case UPD_CXX_INSTANTIATED_CLASS_DEFINITION: {
auto *RD = cast<CXXRecordDecl>(D);
UpdatedDeclContexts.insert(RD->getPrimaryContext());
@@ -5779,6 +5784,15 @@ void ASTWriter::StaticDataMemberInstanti
D->getMemberSpecializationInfo()->getPointOfInstantiation()));
}
+void ASTWriter::DefaultArgumentInstantiated(const ParmVarDecl *D) {
+ assert(!WritingAST && "Already writing the AST!");
+ if (!D->isFromASTFile())
+ return;
+
+ DeclUpdates[D].push_back(
+ DeclUpdate(UPD_CXX_INSTANTIATED_DEFAULT_ARGUMENT, D));
+}
+
void ASTWriter::AddedObjCCategoryToInterface(const ObjCCategoryDecl *CatD,
const ObjCInterfaceDecl *IFD) {
assert(!WritingAST && "Already writing the AST!");
Modified: cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterStmt.cpp?rev=256983&r1=256982&r2=256983&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriterStmt.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriterStmt.cpp Wed Jan 6 16:34:54 2016
@@ -1336,15 +1336,8 @@ void ASTStmtWriter::VisitCXXThrowExpr(CX
void ASTStmtWriter::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
VisitExpr(E);
-
- bool HasOtherExprStored = E->Param.getInt();
- // Store these first, the reader reads them before creation.
- Record.push_back(HasOtherExprStored);
- if (HasOtherExprStored)
- Writer.AddStmt(E->getExpr());
Writer.AddDeclRef(E->getParam(), Record);
Writer.AddSourceLocation(E->getUsedLocation(), Record);
-
Code = serialization::EXPR_CXX_DEFAULT_ARG;
}
Added: cfe/trunk/test/PCH/chain-default-argument-instantiation.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/chain-default-argument-instantiation.cpp?rev=256983&view=auto
==============================================================================
--- cfe/trunk/test/PCH/chain-default-argument-instantiation.cpp (added)
+++ cfe/trunk/test/PCH/chain-default-argument-instantiation.cpp Wed Jan 6 16:34:54 2016
@@ -0,0 +1,50 @@
+// Test default argument instantiation in chained PCH.
+
+// Without PCH
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -include %s -include %s %s
+
+// With PCH
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s -chain-include %s -chain-include %s
+
+// With modules
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -fmodules %s -chain-include %s -chain-include %s
+
+// expected-no-diagnostics
+
+#ifndef HEADER1
+#define HEADER1
+//===----------------------------------------------------------------------===//
+// Primary header.
+
+namespace rdar23810407 {
+ template<typename T> int f(T t) {
+ extern T rdar23810407_variable;
+ return 0;
+ }
+ template<typename T> int g(int a = f([] {}));
+}
+
+//===----------------------------------------------------------------------===//
+#elif not defined(HEADER2)
+#define HEADER2
+#if !defined(HEADER1)
+#error Header inclusion order messed up
+#endif
+
+//===----------------------------------------------------------------------===//
+// Dependent header.
+
+inline void instantiate_once() {
+ rdar23810407::g<int>();
+}
+
+//===----------------------------------------------------------------------===//
+#else
+//===----------------------------------------------------------------------===//
+
+void test() {
+ rdar23810407::g<int>();
+}
+
+//===----------------------------------------------------------------------===//
+#endif
Modified: cfe/trunk/test/SemaCXX/conversion.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/conversion.cpp?rev=256983&r1=256982&r2=256983&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/conversion.cpp (original)
+++ cfe/trunk/test/SemaCXX/conversion.cpp Wed Jan 6 16:34:54 2016
@@ -94,9 +94,9 @@ namespace test4 {
// FIXME: We should warn for non-dependent args (only when the param type is also non-dependent) only once
// not once for the template + once for every instantiation
template<typename T>
- void tmpl(char c = NULL, // expected-warning 4 {{implicit conversion of NULL constant to 'char'}}
+ void tmpl(char c = NULL, // expected-warning 3 {{implicit conversion of NULL constant to 'char'}}
T a = NULL, // expected-warning {{implicit conversion of NULL constant to 'char'}} \
- expected-warning 2 {{implicit conversion of NULL constant to 'int'}}
+ expected-warning {{implicit conversion of NULL constant to 'int'}}
T b = 1024) { // expected-warning {{implicit conversion from 'int' to 'char' changes value from 1024 to 0}}
}
@@ -107,8 +107,7 @@ namespace test4 {
void func() {
tmpl<char>(); // expected-note 2 {{in instantiation of default function argument expression for 'tmpl<char>' required here}}
tmpl<int>(); // expected-note 2 {{in instantiation of default function argument expression for 'tmpl<int>' required here}}
- // FIXME: We should warn only once for each template instantiation - not once for each call
- tmpl<int>(); // expected-note 2 {{in instantiation of default function argument expression for 'tmpl<int>' required here}}
+ tmpl<int>();
tmpl2<int*>();
}
}
Modified: cfe/trunk/test/SemaTemplate/default-arguments-cxx0x.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/default-arguments-cxx0x.cpp?rev=256983&r1=256982&r2=256983&view=diff
==============================================================================
--- cfe/trunk/test/SemaTemplate/default-arguments-cxx0x.cpp (original)
+++ cfe/trunk/test/SemaTemplate/default-arguments-cxx0x.cpp Wed Jan 6 16:34:54 2016
@@ -56,3 +56,22 @@ namespace PR16975 {
baz data{0};
}
+
+// rdar://23810407
+// An IRGen failure due to a symbol collision due to a default argument
+// being instantiated twice. Credit goes to Richard Smith for this
+// reduction to a -fsyntax-only failure.
+namespace rdar23810407 {
+ // Instantiating the default argument multiple times will produce two
+ // different lambda types and thus instantiate this function multiple
+ // times, which will produce conflicting extern variable declarations.
+ template<typename T> int f(T t) {
+ extern T rdar23810407_variable;
+ return 0;
+ }
+ template<typename T> int g(int a = f([] {}));
+ void test() {
+ g<int>();
+ g<int>();
+ }
+}
More information about the cfe-commits
mailing list