r254876 - [PGO] Instrument only base constructors and destructors.
Author: sepavloff
Date: Sun Dec 6 08:32:39 2015
New Revision: 254876
URL: http://llvm.org/viewvc/llvm-project?rev=254876&view=rev
Log:
[PGO] Instrument only base constructors and destructors.
Constructors and destructors may be represented by several functions
in IR. Only base structors correspond to source code, others are
small pieces of code and eventually call the base variant. In this
case instrumentation of non-base structors has little sense, this
fix remove it. Now profile data of a declaration corresponds to
exactly one function in IR, it agrees with the current logic of the
profile data loading.
This change fixes PR24996.
Differential Revision: http://reviews.llvm.org/D15158
Added:
cfe/trunk/test/Profile/cxx-structors.cpp
Modified:
cfe/trunk/lib/CodeGen/CGBlocks.cpp
cfe/trunk/lib/CodeGen/CGObjC.cpp
cfe/trunk/lib/CodeGen/CGStmt.cpp
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/lib/CodeGen/CodeGenPGO.cpp
cfe/trunk/lib/CodeGen/CodeGenPGO.h
cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp
Modified: cfe/trunk/lib/CodeGen/CGBlocks.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBlocks.cpp?rev=254876&r1=254875&r2=254876&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBlocks.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBlocks.cpp Sun Dec 6 08:32:39 2015
@@ -1241,7 +1241,7 @@ CodeGenFunction::GenerateBlockFunction(G
if (IsLambdaConversionToBlock)
EmitLambdaBlockInvokeBody();
else {
-PGO.assignRegionCounters(blockDecl, fn);
+PGO.assignRegionCounters(GlobalDecl(blockDecl), fn);
incrementProfileCounter(blockDecl->getBody());
EmitStmt(blockDecl->getBody());
}
Modified: cfe/trunk/lib/CodeGen/CGObjC.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjC.cpp?rev=254876&r1=254875&r2=254876&view=diff
==
--- cfe/trunk/lib/CodeGen/CGObjC.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjC.cpp Sun Dec 6 08:32:39 2015
@@ -557,7 +557,7 @@ static llvm::Value *emitARCRetainLoadOfS
/// its pointer, name, and types registered in the class struture.
void CodeGenFunction::GenerateObjCMethod(const ObjCMethodDecl *OMD) {
StartObjCMethod(OMD, OMD->getClassInterface());
- PGO.assignRegionCounters(OMD, CurFn);
+ PGO.assignRegionCounters(GlobalDecl(OMD), CurFn);
assert(isa(OMD->getBody()));
incrementProfileCounter(OMD->getBody());
EmitCompoundStmtWithoutScope(*cast(OMD->getBody()));
Modified: cfe/trunk/lib/CodeGen/CGStmt.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmt.cpp?rev=254876&r1=254875&r2=254876&view=diff
==
--- cfe/trunk/lib/CodeGen/CGStmt.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGStmt.cpp Sun Dec 6 08:32:39 2015
@@ -2170,7 +2170,7 @@ CodeGenFunction::GenerateCapturedStmtFun
CXXThisValue = EmitLoadOfLValue(ThisLValue, Loc).getScalarVal();
}
- PGO.assignRegionCounters(CD, F);
+ PGO.assignRegionCounters(GlobalDecl(CD), F);
CapturedStmtInfo->EmitBody(*this, CD->getBody());
FinishFunction(CD->getBodyRBrace());
Modified: cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp?rev=254876&r1=254875&r2=254876&view=diff
==
--- cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp Sun Dec 6 08:32:39 2015
@@ -185,7 +185,7 @@ CodeGenFunction::GenerateOpenMPCapturedS
++Cnt, ++I;
}
- PGO.assignRegionCounters(CD, F);
+ PGO.assignRegionCounters(GlobalDecl(CD), F);
CapturedStmtInfo->EmitBody(*this, CD->getBody());
FinishFunction(CD->getBodyRBrace());
Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=254876&r1=254875&r2=254876&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Sun Dec 6 08:32:39 2015
@@ -957,8 +957,7 @@ void CodeGenFunction::GenerateCode(Globa
StartFunction(GD, ResTy, Fn, FnInfo, Args, Loc, BodyRange.getBegin());
// Generate the body of the function.
- PGO.checkGlobalDecl(GD);
- PGO.assignRegionCounters(GD.getDecl(), CurFn);
+ PGO.assignRegionCounters(GD, CurFn);
if (isa(FD))
EmitDestructorBody(Args);
else if (isa(FD))
Modified: cfe/trunk/lib/CodeGen/CodeGenPGO.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenPGO.cpp?rev=254876&r1=254875&r2=254876&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenPGO.cpp (original)
+++ cfe/trunk/lib/Co
Re: [PATCH] D15158: [PGO] Instrument only base constructors and destructors.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL254876: [PGO] Instrument only base constructors and
destructors. (authored by sepavloff).
Changed prior to commit:
http://reviews.llvm.org/D15158?vs=41744&id=42011#toc
Repository:
rL LLVM
http://reviews.llvm.org/D15158
Files:
cfe/trunk/lib/CodeGen/CGBlocks.cpp
cfe/trunk/lib/CodeGen/CGObjC.cpp
cfe/trunk/lib/CodeGen/CGStmt.cpp
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/lib/CodeGen/CodeGenPGO.cpp
cfe/trunk/lib/CodeGen/CodeGenPGO.h
cfe/trunk/test/Profile/cxx-structors.cpp
cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp
Index: cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp
===
--- cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp
+++ cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp
@@ -13,18 +13,25 @@
virtual ~B();
};
-// Complete dtor
-// CHECK: @__llvm_profile_name__ZN1BD1Ev = private constant [9 x i8] c"_ZN1BD1Ev"
+// Base dtor
+// CHECK: @__llvm_profile_name__ZN1BD2Ev = private constant [9 x i8] c"_ZN1BD2Ev"
-// Deleting dtor
-// CHECK: @__llvm_profile_name__ZN1BD0Ev = private constant [9 x i8] c"_ZN1BD0Ev"
+// Complete dtor must not be instrumented
+// CHECK-NOT: @__llvm_profile_name__ZN1BD1Ev = private constant [9 x i8] c"_ZN1BD1Ev"
-// Complete dtor counters and profile data
-// CHECK: @__llvm_profile_counters__ZN1BD1Ev = private global [1 x i64] zeroinitializer
-// CHECK: @__llvm_profile_data__ZN1BD1Ev =
-
-// Deleting dtor counters and profile data
-// CHECK: @__llvm_profile_counters__ZN1BD0Ev = private global [1 x i64] zeroinitializer
-// CHECK: @__llvm_profile_data__ZN1BD0Ev =
+// Deleting dtor must not be instrumented
+// CHECK-NOT: @__llvm_profile_name__ZN1BD0Ev = private constant [9 x i8] c"_ZN1BD0Ev"
+
+// Base dtor counters and profile data
+// CHECK: @__llvm_profile_counters__ZN1BD2Ev = private global [1 x i64] zeroinitializer
+// CHECK: @__llvm_profile_data__ZN1BD2Ev =
+
+// Complete dtor counters and profile data must absent
+// CHECK-NOT: @__llvm_profile_counters__ZN1BD1Ev = private global [1 x i64] zeroinitializer
+// CHECK-NOT: @__llvm_profile_data__ZN1BD1Ev =
+
+// Deleting dtor counters and profile data must absent
+// CHECK-NOT: @__llvm_profile_counters__ZN1BD0Ev = private global [1 x i64] zeroinitializer
+// CHECK-NOT: @__llvm_profile_data__ZN1BD0Ev =
B::~B() { }
Index: cfe/trunk/test/Profile/cxx-structors.cpp
===
--- cfe/trunk/test/Profile/cxx-structors.cpp
+++ cfe/trunk/test/Profile/cxx-structors.cpp
@@ -0,0 +1,32 @@
+// Tests for instrumentation of C++ constructors and destructors.
+//
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.11.0 -x c++ %s -o - -emit-llvm -fprofile-instr-generate | FileCheck %s
+
+struct Foo {
+ Foo() {}
+ Foo(int) {}
+ ~Foo() {}
+};
+
+struct Bar : public Foo {
+ Bar() {}
+ Bar(int x) : Foo(x) {}
+ ~Bar();
+};
+
+Foo foo;
+Foo foo2(1);
+Bar bar;
+
+// Profile data for complete constructors and destructors must absent.
+
+// CHECK-NOT: @__llvm_profile_name__ZN3FooC1Ev
+// CHECK-NOT: @__llvm_profile_name__ZN3FooC1Ei
+// CHECK-NOT: @__llvm_profile_name__ZN3FooD1Ev
+// CHECK-NOT: @__llvm_profile_name__ZN3BarC1Ev
+// CHECK-NOT: @__llvm_profile_name__ZN3BarD1Ev
+// CHECK-NOT: @__llvm_profile_counters__ZN3FooD1Ev
+// CHECK-NOT: @__llvm_profile_data__ZN3FooD1Ev
+
+int main() {
+}
Index: cfe/trunk/lib/CodeGen/CodeGenPGO.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenPGO.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenPGO.cpp
@@ -605,27 +605,24 @@
return endian::read(Result);
}
-void CodeGenPGO::checkGlobalDecl(GlobalDecl GD) {
- // Make sure we only emit coverage mapping for one constructor/destructor.
- // Clang emits several functions for the constructor and the destructor of
- // a class. Every function is instrumented, but we only want to provide
- // coverage for one of them. Because of that we only emit the coverage mapping
- // for the base constructor/destructor.
- if ((isa(GD.getDecl()) &&
- GD.getCtorType() != Ctor_Base) ||
- (isa(GD.getDecl()) &&
- GD.getDtorType() != Dtor_Base)) {
-SkipCoverageMapping = true;
- }
-}
-
-void CodeGenPGO::assignRegionCounters(const Decl *D, llvm::Function *Fn) {
+void CodeGenPGO::assignRegionCounters(GlobalDecl GD, llvm::Function *Fn) {
+ const Decl *D = GD.getDecl();
bool InstrumentRegions = CGM.getCodeGenOpts().ProfileInstrGenerate;
llvm::IndexedInstrProfReader *PGOReader = CGM.getPGOReader();
if (!InstrumentRegions && !PGOReader)
return;
if (D->isImplicit())
return;
+ // Constructors and destructors may be represented by several functions in IR.
+ // If so, instrument only base variant, others are implemented by delegation
+ // to the base one, it would be counted twice othe
Re: [PATCH] D15062: [clang-format] Add test for AlignAfterOpenBracket = AlwaysBreak in C++.
jeanphilippeD added a comment. Hi Daniel, Realized that I should have added you when I submitted it. Not sure if this review slipped through, or you were too busy, or if this change is not desired since the code was added to support JS. Kind Regards, Jean-Philippe. http://reviews.llvm.org/D15062 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15062: [clang-format] Add test for AlignAfterOpenBracket = AlwaysBreak in C++.
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. http://reviews.llvm.org/D15062 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D15266: [clang-format] Handle \n the same way as std::endl with stream operator.
jeanphilippeD created this revision.
jeanphilippeD added a reviewer: djasper.
jeanphilippeD added a subscriber: cfe-commits.
Herald added a subscriber: klimek.
clang-format break multi-line streams after std::endl.
It now also break for '\n', the suggested replacement for std::endl:
http://llvm.org/docs/CodingStandards.html#avoid-std-endl
Before:
llvm::errs() << aa << '\n' << bb
<< '\n';
llvm::errs() << << "aa\n" <<
<< "bb\n";
After:
llvm::errs() << aa << '\n'
<< bb << '\n';
llvm::errs() << << "aa\n"
<< << "bb\n";
This changeset ensure that multiline streams have a line break after:
-std::endl
-'\n'
-"\n"
-"Some Text\n"
Test:
-Run unit tests
-Run clang format on Format.cpp without it making changes compared to current
clang-format.
http://reviews.llvm.org/D15266
Files:
lib/Format/ContinuationIndenter.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -4964,6 +4964,15 @@
verifyFormat("llvm::errs() << aa << endl\n"
" << bb << endl;");
verifyFormat("llvm::errs() << endl << bb << endl;");
+
+ // Handle '\n'.
+ verifyFormat("llvm::errs() << aa << \"\\n\"\n"
+ " << bb << \"\\n\";");
+ verifyFormat("llvm::errs() << aa << \'\\n\'\n"
+ " << bb << \'\\n\';");
+ verifyFormat("llvm::errs() << << \"aa\\n\"\n"
+ " << << \"bb\\n\";");
+ verifyFormat("llvm::errs() << \"\\n\" << bb <<
\"\\n\";");
}
TEST_F(FormatTest, UnderstandsEquals) {
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -246,8 +246,10 @@
Previous.is(tok::l_brace) && !Current.isOneOf(tok::r_brace,
tok::comment))
return true;
- if (Current.is(tok::lessless) && Previous.is(tok::identifier) &&
- Previous.TokenText == "endl")
+ if (Current.is(tok::lessless) &&
+ ((Previous.is(tok::identifier) && Previous.TokenText == "endl") ||
+ (Previous.Tok.isLiteral() && (Previous.TokenText.endswith("\\n\"") ||
+ Previous.TokenText == "\'\\n\'"
return true;
return false;
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -4964,6 +4964,15 @@
verifyFormat("llvm::errs() << aa << endl\n"
" << bb << endl;");
verifyFormat("llvm::errs() << endl << bb << endl;");
+
+ // Handle '\n'.
+ verifyFormat("llvm::errs() << aa << \"\\n\"\n"
+ " << bb << \"\\n\";");
+ verifyFormat("llvm::errs() << aa << \'\\n\'\n"
+ " << bb << \'\\n\';");
+ verifyFormat("llvm::errs() << << \"aa\\n\"\n"
+ " << << \"bb\\n\";");
+ verifyFormat("llvm::errs() << \"\\n\" << bb << \"\\n\";");
}
TEST_F(FormatTest, UnderstandsEquals) {
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -246,8 +246,10 @@
Previous.is(tok::l_brace) && !Current.isOneOf(tok::r_brace, tok::comment))
return true;
- if (Current.is(tok::lessless) && Previous.is(tok::identifier) &&
- Previous.TokenText == "endl")
+ if (Current.is(tok::lessless) &&
+ ((Previous.is(tok::identifier) && Previous.TokenText == "endl") ||
+ (Previous.Tok.isLiteral() && (Previous.TokenText.endswith("\\n\"") ||
+ Previous.TokenText == "\'\\n\'"
return true;
return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15062: [clang-format] Add test for AlignAfterOpenBracket = AlwaysBreak in C++.
jeanphilippeD added a comment. Thanks a lot, I do not have commit access. I'm happy for you to commit it when you have time. http://reviews.llvm.org/D15062 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D15267: For MS ABI, emit dllexport friend functions defined inline in class
sberg created this revision.
sberg added a reviewer: cfe-commits.
...as that is apparently what MSVC does
http://reviews.llvm.org/D15267
Files:
include/clang/AST/ASTConsumer.h
include/clang/Frontend/MultiplexConsumer.h
include/clang/Sema/Sema.h
lib/CodeGen/CodeGenAction.cpp
lib/CodeGen/ModuleBuilder.cpp
lib/Frontend/MultiplexConsumer.cpp
lib/Parse/ParseCXXInlineMethods.cpp
lib/Sema/SemaDecl.cpp
test/CodeGenCXX/dllexport.cpp
Index: test/CodeGenCXX/dllexport.cpp
===
--- test/CodeGenCXX/dllexport.cpp
+++ test/CodeGenCXX/dllexport.cpp
@@ -255,9 +255,11 @@
// GNU-DAG: define dllexport void @_Z7friend1v()
// MSC-DAG: define dllexport void @"\01?friend2@@YAXXZ"()
// GNU-DAG: define dllexport void @_Z7friend2v()
+// MSC-DAG: define weak_odr dllexport void @"\01?friend3@@YAXXZ"()
struct FuncFriend {
friend __declspec(dllexport) void friend1();
friend __declspec(dllexport) void friend2();
+ friend __declspec(dllexport) void friend3() {}
};
__declspec(dllexport) void friend1() {}
void friend2() {}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -10655,6 +10655,10 @@
Consumer.HandleInlineMethodDefinition(D);
}
+void Sema::ActOnFinishInlineFriendFunctionDef(FunctionDecl *D) {
+ Consumer.HandleInlineFriendFunctionDefinition(D);
+}
+
static bool ShouldWarnAboutMissingPrototype(const FunctionDecl *FD,
const FunctionDecl*& PossibleZeroParamPrototype) {
// Don't warn about invalid declarations.
Index: lib/Parse/ParseCXXInlineMethods.cpp
===
--- lib/Parse/ParseCXXInlineMethods.cpp
+++ lib/Parse/ParseCXXInlineMethods.cpp
@@ -565,6 +565,8 @@
if (CXXMethodDecl *MD = dyn_cast_or_null(LM.D))
Actions.ActOnFinishInlineMethodDef(MD);
+ else if (auto *FD = dyn_cast_or_null(LM.D))
+Actions.ActOnFinishInlineFriendFunctionDef(FD);
}
/// ParseLexedMemberInitializers - We finished parsing the member specification
Index: lib/Frontend/MultiplexConsumer.cpp
===
--- lib/Frontend/MultiplexConsumer.cpp
+++ lib/Frontend/MultiplexConsumer.cpp
@@ -271,6 +271,11 @@
Consumer->HandleInlineMethodDefinition(D);
}
+void MultiplexConsumer::HandleInlineFriendFunctionDefinition(FunctionDecl *D) {
+ for (auto &Consumer : Consumers)
+Consumer->HandleInlineFriendFunctionDefinition(D);
+}
+
void MultiplexConsumer::HandleCXXStaticMemberVarInstantiation(VarDecl *VD) {
for (auto &Consumer : Consumers)
Consumer->HandleCXXStaticMemberVarInstantiation(VD);
Index: lib/CodeGen/ModuleBuilder.cpp
===
--- lib/CodeGen/ModuleBuilder.cpp
+++ lib/CodeGen/ModuleBuilder.cpp
@@ -163,6 +163,18 @@
Builder->AddDeferredUnusedCoverageMapping(D);
}
+void HandleInlineFriendFunctionDefinition(FunctionDecl *D) override {
+ if (Diags.hasErrorOccurred())
+return;
+
+ assert(D->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend) &&
+ "Must be a friend decl");
+ assert(D->doesThisDeclarationHaveABody());
+
+ if (Ctx->getTargetInfo().getCXXABI().isMicrosoft())
+Builder->EmitTopLevelDecl(D);
+}
+
/// HandleTagDeclDefinition - This callback is invoked each time a TagDecl
/// to (e.g. struct, union, enum, class) is completed. This allows the
/// client hack on the type, which can occur at any point in the file
Index: lib/CodeGen/CodeGenAction.cpp
===
--- lib/CodeGen/CodeGenAction.cpp
+++ lib/CodeGen/CodeGenAction.cpp
@@ -132,6 +132,19 @@
LLVMIRGeneration.stopTimer();
}
+void HandleInlineFriendFunctionDefinition(FunctionDecl *D) override {
+ PrettyStackTraceDecl CrashInfo(D, SourceLocation(),
+ Context->getSourceManager(),
+ "LLVM IR generation of inline friend function");
+ if (llvm::TimePassesIsEnabled)
+LLVMIRGeneration.startTimer();
+
+ Gen->HandleInlineFriendFunctionDefinition(D);
+
+ if (llvm::TimePassesIsEnabled)
+LLVMIRGeneration.stopTimer();
+}
+
void HandleTranslationUnit(ASTContext &C) override {
{
PrettyStackTraceString CrashInfo("Per-file LLVM IR generation");
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -1769,6 +1769,7 @@
Decl *ActOnFinishFunctionBody(Decl *Decl, Stmt *Body, bool IsInstantiation);
Decl *ActOnSkippedFunctionBody(Decl *Decl);
void ActOnFinishInlineMethodDef(CXXMethodDecl *D);
+ void ActOnFinishInlineFriendFunctionDef(FunctionDecl *D);
/// ActOnFin
Re: [PATCH] D14814: [libcxx] Use __make_integer_seq builtin when available
majnemer added a comment. This LGTM but @mclow.lists or @EricWF should give the final say-so. http://reviews.llvm.org/D14814 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r254889 - Fix PR20334: invalid assertion while diagnosing list initialization failure
Author: faisalv
Date: Sun Dec 6 20:37:44 2015
New Revision: 254889
URL: http://llvm.org/viewvc/llvm-project?rev=254889&view=rev
Log:
Fix PR20334: invalid assertion while diagnosing list initialization failure
https://llvm.org/bugs/show_bug.cgi?id=20334
Unfortunately, clang currently checks for a certain brokenness of
implementations of std::initializer_list in CodeGen (void
AggExprEmitter::VisitCXXStdInitializerListExpr), not in SemaInit. Until that
is fixed, make sure we don't let broken attempts that are aggregates leak
through into sema, which allows maintenance of expected invariants, and avoids
triggering an assertion.
Added:
cfe/trunk/test/SemaCXX/PR20334-std_initializer_list_diagnosis_assertion.cpp
Modified:
cfe/trunk/lib/Sema/SemaInit.cpp
Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=254889&r1=254888&r2=254889&view=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Sun Dec 6 20:37:44 2015
@@ -3732,7 +3732,9 @@ static void TryListInitialization(Sema &
// C++11 [dcl.init.list]p3:
// - If T is an aggregate, aggregate initialization is performed.
- if (DestType->isRecordType() && !DestType->isAggregateType()) {
+ if ((DestType->isRecordType() && !DestType->isAggregateType()) ||
+ (S.getLangOpts().CPlusPlus11 &&
+ S.isStdInitializerList(DestType, nullptr))) {
if (S.getLangOpts().CPlusPlus11) {
// - Otherwise, if the initializer list has no elements and T is a
// class type with a default constructor, the object is
Added:
cfe/trunk/test/SemaCXX/PR20334-std_initializer_list_diagnosis_assertion.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/PR20334-std_initializer_list_diagnosis_assertion.cpp?rev=254889&view=auto
==
--- cfe/trunk/test/SemaCXX/PR20334-std_initializer_list_diagnosis_assertion.cpp
(added)
+++ cfe/trunk/test/SemaCXX/PR20334-std_initializer_list_diagnosis_assertion.cpp
Sun Dec 6 20:37:44 2015
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -std=c++11 -verify -emit-llvm-only %s
+// RUN: %clang_cc1 -std=c++98 -fsyntax-only -verify %s -DCPP98
+
+namespace std {
+ template
+ class initializer_list
+ {};
+}
+
+template int f(std::initializer_list il);
+
+
+int F = f({1, 2, 3});
+#ifdef CPP98
+//expected-error@-2{{expected expression}}
+#else
+//expected-error@-4{{cannot compile}}
+#endif
+
+
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.
ABataev marked 2 inline comments as done.
Comment at: lib/Sema/SemaPseudoObject.cpp:232
@@ -231,3 +231,3 @@
/// Return true if assignments have a non-void result.
-bool CanCaptureValue(Expr *exp) {
+bool CanCaptureValue(Expr *exp) const {
if (exp->isGLValue())
rjmccall wrote:
> Just make this static.
Ok
Comment at: lib/Sema/SemaPseudoObject.cpp:464
@@ -461,1 +463,3 @@
+ if (useSetterResultAsExprResult(result.get()))
+setResultToLastSemantic();
rjmccall wrote:
> This will leave the result as the captured set value if it can't capture the
> setter result. Is that desired?
The fact that the value can be captured must be checked in
useSetterResultAsExprResult()
http://reviews.llvm.org/D15174
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15220: [OPENMP] dist_schedule clause for distribute directive
ABataev added a comment. Tests? Repository: rL LLVM http://reviews.llvm.org/D15220 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.
ABataev updated this revision to Diff 42026.
ABataev marked 2 inline comments as done.
ABataev added a comment.
Update after review
http://reviews.llvm.org/D15174
Files:
lib/Sema/SemaPseudoObject.cpp
test/CodeGenCXX/ms-property.cpp
test/SemaCXX/ms-property-error.cpp
test/SemaCXX/ms-property.cpp
Index: lib/Sema/SemaPseudoObject.cpp
===
--- lib/Sema/SemaPseudoObject.cpp
+++ lib/Sema/SemaPseudoObject.cpp
@@ -229,7 +229,7 @@
}
/// Return true if assignments have a non-void result.
-bool CanCaptureValue(Expr *exp) {
+static bool CanCaptureValue(Expr *exp) {
if (exp->isGLValue())
return true;
QualType ty = exp->getType();
@@ -245,6 +245,7 @@
virtual ExprResult buildGet() = 0;
virtual ExprResult buildSet(Expr *, SourceLocation,
bool captureSetValueAsResult) = 0;
+virtual bool useSetterResultAsExprResult(Expr *) const { return false; }
};
/// A PseudoOpBuilder for Objective-C \@properties.
@@ -339,6 +340,7 @@
Expr *rebuildAndCaptureObject(Expr *) override;
ExprResult buildGet() override;
ExprResult buildSet(Expr *op, SourceLocation, bool) override;
+ bool useSetterResultAsExprResult(Expr *) const override;
};
}
@@ -458,6 +460,8 @@
result = buildSet(result.get(), opcLoc, /*captureSetValueAsResult*/ true);
if (result.isInvalid()) return ExprError();
addSemanticExpr(result.get());
+ if (useSetterResultAsExprResult(result.get()))
+setResultToLastSemantic();
return complete(syntactic);
}
@@ -502,6 +506,9 @@
result = buildSet(result.get(), opcLoc, UnaryOperator::isPrefix(opcode));
if (result.isInvalid()) return ExprError();
addSemanticExpr(result.get());
+ if (UnaryOperator::isPrefix(opcode) &&
+ useSetterResultAsExprResult(result.get()))
+setResultToLastSemantic();
UnaryOperator *syntactic =
new (S.Context) UnaryOperator(syntacticOp, opcode, resultType,
@@ -1497,6 +1504,12 @@
op->getSourceRange().getEnd());
}
+bool MSPropertyOpBuilder::useSetterResultAsExprResult(Expr *ResExpr) const {
+ assert(ResExpr);
+ return !ResExpr->getType()->isVoidType() &&
+ (ResExpr->getType()->isDependentType() || CanCaptureValue(ResExpr));
+}
+
//===--===//
// General Sema routines.
//===--===//
Index: test/SemaCXX/ms-property.cpp
===
--- test/SemaCXX/ms-property.cpp
+++ test/SemaCXX/ms-property.cpp
@@ -29,7 +29,7 @@
public:
__declspec(property(get=GetX,put=PutX)) T x[];
T GetX(T i, T j) { return i+j; }
- void PutX(T i, T j, T k) { j = i = k; }
+ T PutX(T i, T j, T k) { return j = i = k; }
~St() { x[0][0] = x[1][1]; }
};
@@ -52,6 +52,8 @@
((p2->x)[23])[1] = j1;
// CHECK-NEXT: ++(((p2->x)[23])[1]);
++(((p2->x)[23])[1]);
+ // CHECK-NEXT: j1 = ((p2->x)[23])[1] = j1;
+ j1 = ((p2->x)[23])[1] = j1;
// CHECK-NEXT: return Test1::GetTest1()->X;
return Test1::GetTest1()->X;
}
Index: test/SemaCXX/ms-property-error.cpp
===
--- test/SemaCXX/ms-property-error.cpp
+++ test/SemaCXX/ms-property-error.cpp
@@ -7,17 +7,19 @@
void PutX(int i, int j, int k) { j = i = k; } // expected-note {{'PutX' declared here}}
};
+char *ptr;
template
class St {
public:
__declspec(property(get=GetX,put=PutX)) T x[];
T GetX(T i, T j) { return i+j; } // expected-note 3 {{'GetX' declared here}}
- void PutX(T i, T j, T k) { j = i = k; } // expected-note 2 {{'PutX' declared here}}
+ T PutX(T i, T j, T k) { return j = i = k; } // expected-note 2 {{'PutX' declared here}}
~St() {
x[1] = 0; // expected-error {{too few arguments to function call, expected 3, have 2}}
x[2][3] = 4;
++x[2][3];
x[1][2] = x[3][4][5]; // expected-error {{too many arguments to function call, expected 2, have 3}}
+ptr = x[1][2] = x[3][4]; // expected-error {{assigning to 'char *' from incompatible type 'int'}}
}
};
@@ -30,5 +32,6 @@
(p1->x[23]) = argc; // expected-error {{too few arguments to function call, expected 3, have 2}}
float j1 = (p2->x); // expected-error {{too few arguments to function call, expected 2, have 0}}
((p2->x)[23])[1][2] = *argv; // expected-error {{too many arguments to function call, expected 3, have 4}}
+ argv = p2->x[11][22] = argc; // expected-error {{assigning to 'char **' from incompatible type 'float'}}
return ++(((p2->x)[23])); // expected-error {{too few arguments to function call, expected 2, have 1}}
}
Index: test/CodeGenCXX/ms-property.cpp
===
--- test/CodeGenCXX/ms-property.cpp
+++ test/CodeGenCXX/ms-property.cpp
@@ -31,7 +31,7 @@
__declspec(property(get=GetX,put=PutX)) T
Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.
rjmccall added inline comments.
Comment at: lib/Sema/SemaPseudoObject.cpp:464
@@ -461,1 +463,3 @@
+ if (useSetterResultAsExprResult(result.get()))
+setResultToLastSemantic();
Sure, but that's not what I'm saying. I'm saying that the code, as you've
written it, will use the value passed to the setter (if capturable) as the
result of the expression if it can't capture the setter result.
That is, given:
struct A {
__declspec(property(get=GetX,put=PutX)) int x;
int GetX() const { return 0; }
void SetX(long y) {}
};
this expression will have type long instead of void, because it's implicitly
falling back on capturing the value passed to the setter:
a.x = 5;
I suspect that that's not the MSVC semantics, and the language rule is that you
should always use the setter result. If that's true, you should be asking the
subclass which rule to use abstractly, i.e. before constructing the setter, and
then (1) suppressing the capture of the set value and (2) using the setter
result (if capturable) or else nothing.
http://reviews.llvm.org/D15174
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15120: Add support for __float128 type to be used by targets that support it
nemanjai added a comment. Just a friendly reminder to the reviewers that this patch has been up for almost a week with no review comments. Please take some time to review. Repository: rL LLVM http://reviews.llvm.org/D15120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.
John,
Your example won't be compiled at all. Clang and MSVC emit error
messages in this case.
The next code
struct A {
__declspec(property(get=GetX,put=SetX)) int x;
int GetX() const { return 0; }
void SetX(long y) {}
};
a.x = 5;
compiled fine by MSVC and clang and provides correct result on clang
(i.e. the result value can't be captured at all, because SetX() returns
void)
return a.x = 5;
clang: error: cannot initialize return object of type 'int' with an
rvalue of type 'void'
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10833: Retrieve BinaryOperator::getOpcode and BinaryOperator::getOpcodeStr via libclang and its python interface
RedX2501 added a comment. Ping http://reviews.llvm.org/D10833 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10834: Added functions to retrieve information about whether a vardecl is local in libclang and its python bindings.
RedX2501 added a comment. Ping http://reviews.llvm.org/D10834 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15120: Add support for __float128 type to be used by targets that support it
rsmith added a comment.
+rjmccall for `@encode` and USR mangling.
Comment at: include/clang-c/Index.h:2879-2885
@@ -2878,8 +2878,9 @@
CXType_LongDouble = 23,
- CXType_NullPtr = 24,
- CXType_Overload = 25,
- CXType_Dependent = 26,
- CXType_ObjCId = 27,
- CXType_ObjCClass = 28,
- CXType_ObjCSel = 29,
+ CXType_Float128 = 24,
+ CXType_NullPtr = 25,
+ CXType_Overload = 26,
+ CXType_Dependent = 27,
+ CXType_ObjCId = 28,
+ CXType_ObjCClass = 29,
+ CXType_ObjCSel = 30,
CXType_FirstBuiltin = CXType_Void,
Do not renumber these, the libclang ABI has stability guarantees. Add your new
enumerator with value 30 instead.
Comment at: include/clang/Analysis/Analyses/ThreadSafetyTIL.h:244-245
@@ -243,2 +243,4 @@
}
+// FIXME: does this need a __float128 specialization as well?
+//Presumably not since most targets don't support it.
I don't know, and this template looks suspicious already. We likely shouldn't
be assuming that `long double`'s size is 128 in the previous function.
Comment at: include/clang/Basic/TokenKinds.def:260
@@ -259,2 +259,3 @@
KEYWORD(double , KEYALL)
+KEYWORD(__float128 , KEYALL)
KEYWORD(else, KEYALL)
Please keep the list in alphabetical order, and in any case add this to the GNU
extensions list below, not here.
Comment at: include/clang/Serialization/ASTBitCodes.h:813
@@ +812,3 @@
+ PREDEF_TYPE_OMP_ARRAY_SECTION = 55,
+ /// \brief The IEEE 754R 128-bit floating point type
+ PREDEF_TYPE_FLOAT128_ID = 56
I would say "The '__float128' type." here -- we don't need to assume that it's
*this* 128-bit floating point type on any particular platform (except in the
setup for that particular target).
Comment at: lib/AST/ASTContext.cpp:972
@@ -971,2 +971,3 @@
TypeDecl *ASTContext::getFloat128StubType() const {
+ // FIXME: Is the assert (and indeed the function) needed?
assert(LangOpts.CPlusPlus && "should only be called for c++");
This function only existed to allow us to parse libstdc++'s `numeric_limits`
specialization for `__float128`. We won't need it any more once we have real
support for `__float128`.
Comment at: lib/AST/ASTContext.cpp:8031
@@ -8015,2 +8030,3 @@
break;
+ // FIXME: we will probably need to handle __float128
case 'd':
We can defer that until we want to add a builtin that takes a `__float128`.
Comment at: lib/AST/ItaniumMangle.cpp:2061
@@ -2060,1 +2060,3 @@
break;
+ // FIXME: is this the right thing to do (always)?
+ case BuiltinType::Float128:
We should test a platform that uses `useFloat128ManglingForLongDouble`.
Presumably they must either not support `__float128` or use a different
mangling here. (IIRC, they can't make `long double` and `__float128` the same
type because that would break libstdc++.)
Comment at: lib/AST/MicrosoftMangle.cpp:1708
@@ -1707,1 +1707,3 @@
+ // FIXME: We probably want to bail on this for now.
+ case BuiltinType::Float128:
Yes, that seems like the best thing to do for now; you can just drop this FIXME.
Comment at: lib/AST/NSAPI.cpp:444-445
@@ -443,2 +443,4 @@
case BuiltinType::UInt128:
+ // FIXME: Added for completeness. Not sure if it's needed.
+ case BuiltinType::Float128:
case BuiltinType::NullPtr:
This looks appropriate.
Comment at: lib/AST/StmtPrinter.cpp:1212-1213
@@ -1211,2 +1211,4 @@
case BuiltinType::LongDouble: OS << 'L'; break;
+ // FIXME: Need to figure out what this should be
+ case BuiltinType::Float128: OS << 'Q'; break;
}
Does GCC support a suffix for `__int128`? If not, just add a FIXME like we do
for `Half`. This case -- presumably -- should never happen if there are no
literals of this type.
Comment at: lib/Basic/TargetInfo.cpp:225-228
@@ -223,2 +224,6 @@
case 128:
+// FIXME: Return __float128 or long double on targets that support both?
+//This placement of the condition will favour the former.
+if (hasFloat128Type())
+ return Float128;
if (&getLongDoubleFormat() == &llvm::APFloat::PPCDoubleDouble ||
Return `long double`. We should prefer the standard type over the GNU extension
here.
Comment at: lib/Basic/Targets.cpp:1037-1044
@@ -1035,7 +1036,10 @@
bool useFloat128ManglingForLongDouble() const override {
return LongDoubleWidth == 128 &&
LongDoubleFormat == &llvm::APFloat::PPCDoubleDouble &&
getTriple().isOSBinFormatELF();
}
+ bool hasFloat128Type() const override {
+return HasFloat128;
+ }
};
Should we really support both `__float128` and using the `__float128` mangling
for `long double` simul
