[PATCH] D54550: Mark lambda decl as invalid if a captured variable has an invalid type.
jgorbe created this revision.
jgorbe added a reviewer: rsmith.
This causes the compiler to crash when trying to compute a layout for
the lambda closure type (see included test).
Repository:
rC Clang
https://reviews.llvm.org/D54550
Files:
clang/lib/Sema/SemaExpr.cpp
clang/test/SemaCXX/lambda-invalid-capture.cpp
Index: clang/test/SemaCXX/lambda-invalid-capture.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/lambda-invalid-capture.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// Don't crash.
+
+struct g {
+ j; // expected-error {{C++ requires a type specifier for all declarations}}
+};
+
+void h() {
+ g child;
+ auto q = [child]{};
+ const int n = sizeof(q);
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14941,6 +14941,13 @@
= FieldDecl::Create(S.Context, Lambda, Loc, Loc, nullptr, FieldType,
S.Context.getTrivialTypeSourceInfo(FieldType, Loc),
nullptr, false, ICIS_NoInit);
+ // If the variable being captured has an invalid type, mark the lambda class
+ // as invalid as well.
+ if (const CXXRecordDecl *RD = FieldType->getAsCXXRecordDecl()) {
+if (RD->isInvalidDecl()) {
+ Lambda->setInvalidDecl();
+}
+ }
Field->setImplicit(true);
Field->setAccess(AS_private);
Lambda->addDecl(Field);
Index: clang/test/SemaCXX/lambda-invalid-capture.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/lambda-invalid-capture.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// Don't crash.
+
+struct g {
+ j; // expected-error {{C++ requires a type specifier for all declarations}}
+};
+
+void h() {
+ g child;
+ auto q = [child]{};
+ const int n = sizeof(q);
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14941,6 +14941,13 @@
= FieldDecl::Create(S.Context, Lambda, Loc, Loc, nullptr, FieldType,
S.Context.getTrivialTypeSourceInfo(FieldType, Loc),
nullptr, false, ICIS_NoInit);
+ // If the variable being captured has an invalid type, mark the lambda class
+ // as invalid as well.
+ if (const CXXRecordDecl *RD = FieldType->getAsCXXRecordDecl()) {
+if (RD->isInvalidDecl()) {
+ Lambda->setInvalidDecl();
+}
+ }
Field->setImplicit(true);
Field->setAccess(AS_private);
Lambda->addDecl(Field);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54550: Mark lambda decl as invalid if a captured variable has an invalid type.
jgorbe updated this revision to Diff 174118.
jgorbe added a comment.
Fixed some issues pointed out in review comments:
- call to getBaseElementType before checking type validity.
- when the type is incomplete, mark not only the lambda closure type as invalid
but also the field
Also, added a couple of checks to resemble more closely the code that checks
user-declared fields in classes.
https://reviews.llvm.org/D54550
Files:
clang/lib/Sema/SemaExpr.cpp
clang/test/SemaCXX/lambda-invalid-capture.cpp
Index: clang/test/SemaCXX/lambda-invalid-capture.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/lambda-invalid-capture.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// Don't crash.
+
+struct g {
+ j; // expected-error {{C++ requires a type specifier for all declarations}}
+};
+
+void h() {
+ g child;
+ auto q = [child]{};
+ const int n = sizeof(q);
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14941,6 +14941,22 @@
= FieldDecl::Create(S.Context, Lambda, Loc, Loc, nullptr, FieldType,
S.Context.getTrivialTypeSourceInfo(FieldType, Loc),
nullptr, false, ICIS_NoInit);
+ // If the variable being captured has an invalid type, mark the lambda class
+ // as invalid as well.
+ QualType EltTy = S.Context.getBaseElementType(FieldType);
+ if (!EltTy->isDependentType()) {
+if (S.RequireCompleteType(Loc, EltTy, diag::err_field_incomplete)) {
+ Lambda->setInvalidDecl();
+ Field->setInvalidDecl();
+} else {
+ NamedDecl *Def;
+ EltTy->isIncompleteType(&Def);
+ if (Def && Def->isInvalidDecl()) {
+Lambda->setInvalidDecl();
+Field->setInvalidDecl();
+ }
+}
+ }
Field->setImplicit(true);
Field->setAccess(AS_private);
Lambda->addDecl(Field);
Index: clang/test/SemaCXX/lambda-invalid-capture.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/lambda-invalid-capture.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// Don't crash.
+
+struct g {
+ j; // expected-error {{C++ requires a type specifier for all declarations}}
+};
+
+void h() {
+ g child;
+ auto q = [child]{};
+ const int n = sizeof(q);
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14941,6 +14941,22 @@
= FieldDecl::Create(S.Context, Lambda, Loc, Loc, nullptr, FieldType,
S.Context.getTrivialTypeSourceInfo(FieldType, Loc),
nullptr, false, ICIS_NoInit);
+ // If the variable being captured has an invalid type, mark the lambda class
+ // as invalid as well.
+ QualType EltTy = S.Context.getBaseElementType(FieldType);
+ if (!EltTy->isDependentType()) {
+if (S.RequireCompleteType(Loc, EltTy, diag::err_field_incomplete)) {
+ Lambda->setInvalidDecl();
+ Field->setInvalidDecl();
+} else {
+ NamedDecl *Def;
+ EltTy->isIncompleteType(&Def);
+ if (Def && Def->isInvalidDecl()) {
+Lambda->setInvalidDecl();
+Field->setInvalidDecl();
+ }
+}
+ }
Field->setImplicit(true);
Field->setAccess(AS_private);
Lambda->addDecl(Field);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54550: Mark lambda decl as invalid if a captured variable has an invalid type.
jgorbe marked 2 inline comments as done.
jgorbe added inline comments.
Comment at: clang/lib/Sema/SemaExpr.cpp:14946
+ // as invalid as well.
+ if (const CXXRecordDecl *RD = FieldType->getAsCXXRecordDecl()) {
+if (RD->isInvalidDecl()) {
rsmith wrote:
> You'll need to use `FieldType->getBaseElementTypeUnsafe()` or similar here to
> handle the case where the field type is an array whose element type is a
> class. (That can happen in a lambda if you capture a local array variable by
> value.)
Done, I ended up using Context.getBaseElementType because the call to
RequireCompleteType below required a QualType.
Comment at: clang/lib/Sema/SemaExpr.cpp:14948
+if (RD->isInvalidDecl()) {
+ Lambda->setInvalidDecl();
+}
rsmith wrote:
> For consistency with the class case, I think we should mark the field invalid
> in this case too.
Done!
https://reviews.llvm.org/D54550
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54550: Mark lambda decl as invalid if a captured variable has an invalid type.
jgorbe updated this revision to Diff 174853.
jgorbe marked 2 inline comments as done.
https://reviews.llvm.org/D54550
Files:
clang/lib/Sema/SemaExpr.cpp
clang/test/SemaCXX/lambda-invalid-array-capture.cpp
clang/test/SemaCXX/lambda-invalid-capture.cpp
Index: clang/test/SemaCXX/lambda-invalid-capture.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/lambda-invalid-capture.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// Don't crash.
+
+struct g {
+ j; // expected-error {{C++ requires a type specifier for all declarations}}
+};
+
+void h() {
+ g child;
+ auto q = [child]{};
+ const int n = sizeof(q);
+}
Index: clang/test/SemaCXX/lambda-invalid-array-capture.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/lambda-invalid-array-capture.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// Don't crash.
+
+struct g {
+ j; // expected-error {{C++ requires a type specifier for all declarations}}
+};
+
+void h() {
+ g child[100];
+ auto q = [child]{};
+ const int n = sizeof(q);
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14967,6 +14967,21 @@
= FieldDecl::Create(S.Context, Lambda, Loc, Loc, nullptr, FieldType,
S.Context.getTrivialTypeSourceInfo(FieldType, Loc),
nullptr, false, ICIS_NoInit);
+ // If the variable being captured has an invalid type, mark the lambda class
+ // as invalid as well.
+ if (!FieldType->isDependentType()) {
+if (S.RequireCompleteType(Loc, FieldType, diag::err_field_incomplete)) {
+ Lambda->setInvalidDecl();
+ Field->setInvalidDecl();
+} else {
+ NamedDecl *Def;
+ FieldType->isIncompleteType(&Def);
+ if (Def && Def->isInvalidDecl()) {
+Lambda->setInvalidDecl();
+Field->setInvalidDecl();
+ }
+}
+ }
Field->setImplicit(true);
Field->setAccess(AS_private);
Lambda->addDecl(Field);
Index: clang/test/SemaCXX/lambda-invalid-capture.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/lambda-invalid-capture.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// Don't crash.
+
+struct g {
+ j; // expected-error {{C++ requires a type specifier for all declarations}}
+};
+
+void h() {
+ g child;
+ auto q = [child]{};
+ const int n = sizeof(q);
+}
Index: clang/test/SemaCXX/lambda-invalid-array-capture.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/lambda-invalid-array-capture.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// Don't crash.
+
+struct g {
+ j; // expected-error {{C++ requires a type specifier for all declarations}}
+};
+
+void h() {
+ g child[100];
+ auto q = [child]{};
+ const int n = sizeof(q);
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14967,6 +14967,21 @@
= FieldDecl::Create(S.Context, Lambda, Loc, Loc, nullptr, FieldType,
S.Context.getTrivialTypeSourceInfo(FieldType, Loc),
nullptr, false, ICIS_NoInit);
+ // If the variable being captured has an invalid type, mark the lambda class
+ // as invalid as well.
+ if (!FieldType->isDependentType()) {
+if (S.RequireCompleteType(Loc, FieldType, diag::err_field_incomplete)) {
+ Lambda->setInvalidDecl();
+ Field->setInvalidDecl();
+} else {
+ NamedDecl *Def;
+ FieldType->isIncompleteType(&Def);
+ if (Def && Def->isInvalidDecl()) {
+Lambda->setInvalidDecl();
+Field->setInvalidDecl();
+ }
+}
+ }
Field->setImplicit(true);
Field->setAccess(AS_private);
Lambda->addDecl(Field);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54550: Mark lambda decl as invalid if a captured variable has an invalid type.
jgorbe added a comment.
Added a test for the "capturing an array of incomplete type" case. See also
responses to inline comments below.
Comment at: clang/lib/Sema/SemaExpr.cpp:14946
+ // as invalid as well.
+ if (const CXXRecordDecl *RD = FieldType->getAsCXXRecordDecl()) {
+if (RD->isInvalidDecl()) {
rsmith wrote:
> jgorbe wrote:
> > rsmith wrote:
> > > You'll need to use `FieldType->getBaseElementTypeUnsafe()` or similar
> > > here to handle the case where the field type is an array whose element
> > > type is a class. (That can happen in a lambda if you capture a local
> > > array variable by value.)
> > Done, I ended up using Context.getBaseElementType because the call to
> > RequireCompleteType below required a QualType.
> Hah, OK, now you've switched to using `RequireCompleteType` this has become
> redundant again (it'll walk to the base element type for you). =)
Removed call to `getBaseElementType()`, changed appearances of `EltTy` to
`FieldType` instead.
Comment at: clang/lib/Sema/SemaExpr.cpp:14952-14958
+ NamedDecl *Def;
+ EltTy->isIncompleteType(&Def);
+ if (Def && Def->isInvalidDecl()) {
+Lambda->setInvalidDecl();
+Field->setInvalidDecl();
+ }
+}
rsmith wrote:
> I believe the `else` case here is redundant: if `RequireCompleteType` returns
> `false`, the type is complete and there's nothing more you need to do.
If I just remove the `else` block, the test case included with the patch keeps
crashing. Is there any way a `Decl` can be invalid even after
`RequireCompleteType` returns `false`?
https://reviews.llvm.org/D54550
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54550: Mark lambda decl as invalid if a captured variable has an invalid type.
jgorbe updated this revision to Diff 174944.
jgorbe added a comment.
Folded the two test cases (capturing an invalid type and capturing an invalid
array type) into a single file.
https://reviews.llvm.org/D54550
Files:
clang/lib/Sema/SemaExpr.cpp
clang/test/SemaCXX/lambda-invalid-capture.cpp
Index: clang/test/SemaCXX/lambda-invalid-capture.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/lambda-invalid-capture.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// Don't crash.
+
+struct g {
+ j; // expected-error {{C++ requires a type specifier for all declarations}}
+};
+
+void captures_invalid_type() {
+ g child;
+ auto q = [child]{};
+ const int n = sizeof(q);
+}
+
+void captures_invalid_array_type() {
+ g child[100];
+ auto q = [child]{};
+ const int n = sizeof(q);
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14967,6 +14967,21 @@
= FieldDecl::Create(S.Context, Lambda, Loc, Loc, nullptr, FieldType,
S.Context.getTrivialTypeSourceInfo(FieldType, Loc),
nullptr, false, ICIS_NoInit);
+ // If the variable being captured has an invalid type, mark the lambda class
+ // as invalid as well.
+ if (!FieldType->isDependentType()) {
+if (S.RequireCompleteType(Loc, FieldType, diag::err_field_incomplete)) {
+ Lambda->setInvalidDecl();
+ Field->setInvalidDecl();
+} else {
+ NamedDecl *Def;
+ FieldType->isIncompleteType(&Def);
+ if (Def && Def->isInvalidDecl()) {
+Lambda->setInvalidDecl();
+Field->setInvalidDecl();
+ }
+}
+ }
Field->setImplicit(true);
Field->setAccess(AS_private);
Lambda->addDecl(Field);
Index: clang/test/SemaCXX/lambda-invalid-capture.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/lambda-invalid-capture.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// Don't crash.
+
+struct g {
+ j; // expected-error {{C++ requires a type specifier for all declarations}}
+};
+
+void captures_invalid_type() {
+ g child;
+ auto q = [child]{};
+ const int n = sizeof(q);
+}
+
+void captures_invalid_array_type() {
+ g child[100];
+ auto q = [child]{};
+ const int n = sizeof(q);
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14967,6 +14967,21 @@
= FieldDecl::Create(S.Context, Lambda, Loc, Loc, nullptr, FieldType,
S.Context.getTrivialTypeSourceInfo(FieldType, Loc),
nullptr, false, ICIS_NoInit);
+ // If the variable being captured has an invalid type, mark the lambda class
+ // as invalid as well.
+ if (!FieldType->isDependentType()) {
+if (S.RequireCompleteType(Loc, FieldType, diag::err_field_incomplete)) {
+ Lambda->setInvalidDecl();
+ Field->setInvalidDecl();
+} else {
+ NamedDecl *Def;
+ FieldType->isIncompleteType(&Def);
+ if (Def && Def->isInvalidDecl()) {
+Lambda->setInvalidDecl();
+Field->setInvalidDecl();
+ }
+}
+ }
Field->setImplicit(true);
Field->setAccess(AS_private);
Lambda->addDecl(Field);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics
jgorbe added a comment. Hi, this patch is causing floating point exceptions for us during profile generation. On a debug build I get this assertion failure (see stack trace below): clang: /usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/BranchProbability.cpp:41: llvm::BranchProbability::BranchProbability(uint32_t, uint32_t): Assertion `Denominator > 0 && "Denominator cannot be 0!"' failed. Printing some values around the problem I got TotalBranchWeight = 4294967296 LikelyBranchWeight = 2147483648 UnlikelyBranchWeight = 2147483648 NumUnlikelyTargets = 1 I see the `BranchProbability` constructor takes `uint32_t`s, so this looks like it's overflowing to 0 (4294967296 is exactly 2**32). I'm going to revert the patch to unbreak our build. Please let me know if you need more details and I'll try to come up with a reproducer I can share. Here's the stack trace for the assertion. #0 0x0a7f992a llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/Unix/Signals.inc:565:11 #1 0x0a7f9afb PrintStackTraceSignalHandler(void*) /usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/Unix/Signals.inc:632:1 #2 0x0a7f80bb llvm::sys::RunSignalHandlers() /usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/Signals.cpp:102:5 #3 0x0a7fa271 SignalHandler(int) /usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/Unix/Signals.inc:407:1 #4 0x7ff57cfe2200 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x13200) #5 0x7ff57ca678a1 raise ./signal/../sysdeps/unix/sysv/linux/raise.c:50:1 #6 0x7ff57ca51546 abort ./stdlib/abort.c:81:7 #7 0x7ff57ca5142f get_sysdep_segment_value ./intl/loadmsgcat.c:509:8 #8 0x7ff57ca5142f _nl_load_domain ./intl/loadmsgcat.c:970:34 #9 0x7ff57ca60222 (/lib/x86_64-linux-gnu/libc.so.6+0x35222) #10 0x0a6cb517 llvm::BranchProbability::BranchProbability(unsigned int, unsigned int) /usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/BranchProbability.cpp:0:3 #11 0x0a937a4d llvm::misexpect::verifyMisExpect(llvm::Instruction&, llvm::ArrayRef, llvm::ArrayRef) /usr/local/google/home/jgorbe/code/llvm/llvm/lib/Transforms/Utils/MisExpect.cpp:190:54 #12 0x0a937ef3 llvm::misexpect::checkBackendInstrumentation(llvm::Instruction&, llvm::ArrayRef) /usr/local/google/home/jgorbe/code/llvm/llvm/lib/Transforms/Utils/MisExpect.cpp:217:1 #13 0x0a93807b llvm::misexpect::checkExpectAnnotations(llvm::Instruction&, llvm::ArrayRef, bool) /usr/local/google/home/jgorbe/code/llvm/llvm/lib/Transforms/Utils/MisExpect.cpp:236:1 #14 0x09e1339c (anonymous namespace)::SampleProfileLoader::generateMDProfMetadata(llvm::Function&) /usr/local/google/home/jgorbe/code/llvm/llvm/lib/Transforms/IPO/SampleProfile.cpp:1755:19 #15 0x09e10d94 (anonymous namespace)::SampleProfileLoader::emitAnnotations(llvm::Function&) /usr/local/google/home/jgorbe/code/llvm/llvm/lib/Transforms/IPO/SampleProfile.cpp:0:5 #16 0x09e1022b (anonymous namespace)::SampleProfileLoader::runOnFunction(llvm::Function&, llvm::AnalysisManager*) /usr/local/google/home/jgorbe/code/llvm/llvm/lib/Transforms/IPO/SampleProfile.cpp:2199:5
[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics
jgorbe added a comment. By the way, the line number for `llvm::misexpect::verifyMisExpect` in the stack trace above includes the debug `printf`s I inserted to get the variable values so it won't match the exact line number in the repo. But I think that's the only call to `BranchProbability` in that function so I hope it's not very confusing anyway. Sorry about that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115907/new/ https://reviews.llvm.org/D115907 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D121450: [clang-format] Handle attributes before case label.
jgorbe added a comment.
Hi,
We've observed that this patch introduces infinite loops in some cases. Here's
a reduced test case:
export class Foo extends Bar {
get case(): Case {
return (
(this.Bar$has('case')) ? (this.Bar$get('case')) :
(this.case = new Case()));
}
}
Saving this as `/tmp/test.ts` and running `clang-format /tmp/test.ts` loops
indefinitely with this patch (I stopped it after 1m27s) and finishes instantly
(0.12s) after reverting the patch locally. I'm going to go ahead and push a
revert. Please let me know if you need help reproducing the problem.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121450/new/
https://reviews.llvm.org/D121450
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D113472: [clang-tidy] Fix lint warning in ClangTidyDiagnosticConsumer.cpp (NFC)
jgorbe added a comment. This change is causing asan errors when running the clang-tidy checks under ASan, most likely because of the reason akuegel pointed out in his comment. I'm going to revert the patch to unbreak HEAD until the problem can be fixed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113472/new/ https://reviews.llvm.org/D113472 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51650: Implement target_clones multiversioning
jgorbe added a comment.
This breaks some existing code (we have found this issue building the JPEG XL
library at github.com/libjxl). This is a very simplified version of the problem:
#pragma clang attribute push(__attribute__((target("sse2,ssse3"))), apply_to
= function)
__attribute__((target("sse2"))) void f() {
}
#pragma clang attribute pop
This used to build before this patch, but now fails with
$ clang -c a.cc
a.cc:1:45: error: attribute 'target' cannot appear more than once on a
declaration
#pragma clang attribute push(__attribute__((target("sse2,ssse3"))), apply_to
= function)
^
a.cc:2:1: note: when applied to this declaration
__attribute__((target("sse2"))) void f() {
^
a.cc:2:16: note: conflicting attribute is here
__attribute__((target("sse2"))) void f() {
^
1 error generated.
Before this patch, the function-level attribute would win. Here's the relevant
part of the generated IR for that example:
; Function Attrs: mustprogress noinline nounwind optnone uwtable
define dso_local void @_Z1fv() #0 {
ret void
}
attributes #0 = { mustprogress noinline nounwind optnone uwtable
"frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true"
"stack-protector-buffer-size"="8" "target-cpu"="x86-64"
"target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
Note how there's `sse2` (which was in the function-level attribute) but no
`ssse3` (which wasn't).
Was this semantic change intentional?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D51650/new/
https://reviews.llvm.org/D51650
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang
jgorbe added inline comments.
Comment at: clang/lib/AST/ASTContext.cpp:4012-4018
+ if (Target->getTriple().isWasm() && Target->hasFeature("reference-types")) {
+#define WASM_REF_TYPE(Name, MangledName, Id, SingletonId, AS)
\
+ if (BuiltinType::Id == BuiltinType::WasmExternRef)
\
+return SingletonId;
+#include "clang/Basic/WebAssemblyReferenceTypes.def"
+ }
+ return QualType();
aaron.ballman wrote:
> pmatos wrote:
> > aaron.ballman wrote:
> > > Rather than returning a null type, should we assert we're in a wasm
> > > triple? We shouldn't be trying to form the type outside of a wasm target,
> > > right?
> > asserting false with message rather than llvm_unreachable is the right way,
> > correct?
> I'd recommend something like:
> ```
> assert(Target->getTriple().isWasm() && Target->hasFeature("reference-types")
> && "shouldn't try to generate type externref outsid of WebAssembly target");
> #define WASM_REF_TYPE(Name, MangledName, Id, SingletonId, AS)
> \
> if (BuiltinType::Id == BuiltinType::WasmExternRef)
> \
> return SingletonId;
> #include "clang/Basic/WebAssemblyReferenceTypes.def"
> }
> llvm_unreachable("couldn't find the externref type?");
> ```
> so it's clear that you shouldn't call this outside of a wasm target and that
> it's not an option to not find the type.
In a non-assert build, doing just `assert(false)` causes the build to fail if
you use `-Werror -Wreturn-type`.
```
clang/lib/AST/ASTContext.cpp:4097:1: error: non-void function does not return a
value in all control paths [-Werror,-Wreturn-type]
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122215/new/
https://reviews.llvm.org/D122215
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior
jgorbe added a comment.
Hi,
I found a clang crash that seems to be caused by this patch. Here's a reduced
test case:
template
class a {
public:
~a();
void b();
};
template
a::~a() try {
b();
} catch (...) {
}
class d {
public:
d(const char *, int);
a e;
}
d("", 1);
Building it with `clang -c -frounding-math` results in the following crash:
Stack dump:
0. Program arguments:
/usr/local/google/home/jgorbe/code/llvm-build/bin/clang-10 -cc1 -triple
x86_64-unknown-linux-gnu -emit-obj -mrelax-all -disable-free -main-fi
le-name repro.cc -mrelocation-model static -mthread-model posix
-mframe-pointer=all -fmath-errno -frounding-math -masm-verbose
-mconstructor-aliases -munwind-tables -fu
se-init-array -target-cpu x86-64 -dwarf-column-info -debugger-tuning=gdb
-resource-dir /usr/local/google/home/jgorbe/code/llvm-build/lib/clang/10.0.0
-internal-isystem
/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8 -internal-isystem
/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/x86_64-linux-gnu/c++/8
-internal-isystem
/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/x86_64-linux-gnu/c++/8
-internal-isystem
/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/backward -intern
al-isystem /usr/local/include -internal-isystem
/usr/local/google/home/jgorbe/code/llvm-build/lib/clang/10.0.0/include
-internal-externc-isystem /usr/include/x86_64-lin
ux-gnu -internal-externc-isystem /include -internal-externc-isystem
/usr/include -fdeprecated-macro -fdebug-compilation-dir
/usr/local/google/home/jgorbe/repro4 -ferror
-limit 19 -fmessage-length 0 -fgnuc-version=4.2.1 -fobjc-runtime=gcc
-fcxx-exceptions -fexceptions -fdiagnostics-show-option -fcolor-diagnostics
-faddrsig -o /tmp/repro
-c9cae0.o -x c++ repro.cc
1. parser at end of file
2. Per-file LLVM IR generation
3. repro.cc:4:3: Generating code for declaration 'a::~a'
#0 0x07fb2b27 llvm::sys::PrintStackTrace(llvm::raw_ostream&)
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/Unix/Signals.inc:532:11
#1 0x07fb2cc9 PrintStackTraceSignalHandler(void*)
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/Unix/Signals.inc:593:1
#2 0x07fb160b llvm::sys::RunSignalHandlers()
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/Signals.cpp:67:5
#3 0x07fb3415 SignalHandler(int)
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/Unix/Signals.inc:384:1
#4 0x7f6789ec03a0 __restore_rt
(/lib/x86_64-linux-gnu/libpthread.so.0+0x123a0)
#5 0x0739af45
llvm::AttributeList::hasFnAttribute(llvm::Attribute::AttrKind) const
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/IR/Attributes.cpp:1315:10
#6 0x051a1964
llvm::Function::hasFnAttribute(llvm::Attribute::AttrKind) const
/usr/local/google/home/jgorbe/code/llvm/llvm/include/llvm/IR/Function.h:324:5
#7 0x052421a1 llvm::IRBuilderBase::setConstrainedFPFunctionAttr()
/usr/local/google/home/jgorbe/code/llvm/llvm/include/llvm/IR/IRBuilder.h:262:9
#8 0x05241b60
llvm::IRBuilderBase::setConstrainedFPCallAttr(llvm::CallInst*)
/usr/local/google/home/jgorbe/code/llvm/llvm/include/llvm/IR/IRBuilder.h:271:3
#9 0x084a516b llvm::IRBuilder::CreateCall(llvm::FunctionType*,
llvm::Value*, llvm::ArrayRef,
llvm::ArrayRef >, llvm::Twine const&,
llvm::MDNode*)
/usr/local/google/home/jgorbe/code/llvm/llvm/include/llvm/IR/IRBuilder.h:2274:9
#10 0x084a4488 llvm::IRBuilder::CreateCall(llvm::FunctionCallee,
llvm::ArrayRef,
llvm::ArrayRef >, llvm::Twine const&,
llvm::MDNode*)
/usr/local/google/home/jgorbe/code/llvm/llvm/include/llvm/IR/IRBuilder.h:2288:5
#11 0x0849288c
clang::CodeGen::CodeGenFunction::EmitRuntimeCall(llvm::FunctionCallee,
llvm::ArrayRef, llvm::Twine const&)
/usr/local/google/home/jgorbe/code/llvm/clang/lib/CodeGen/CGCall.cpp:3726:26
#12 0x0849278d
clang::CodeGen::CodeGenFunction::EmitNounwindRuntimeCall(llvm::FunctionCallee,
llvm::ArrayRef, llvm::Twine const&)
/usr/local/google/home/jgorbe/code/llvm/clang/lib/CodeGen/CGCall.cpp:3691:19
#13 0x0897b3e4 (anonymous
namespace)::ItaniumCXXABI::emitTerminateForUnexpectedException(clang::CodeGen::CodeGenFunction&,
llvm::Value*)
/usr/local/google/home/jgorbe/code/llvm/clang/lib/CodeGen/ItaniumCXXABI.cpp:4364:5
#14 0x0865698d clang::CodeGen::CodeGenFunction::getTerminateHandler()
/usr/local/google/home/jgorbe/code/llvm/clang/lib/CodeGen/
[PATCH] D100466: clang-format: [JS] merge import lines.
jgorbe added a comment.
Hi, one of our tests is failing because of this patch and it looks like an
actual bug. clang-format now turns this file:
import './a';
import {bar} from './a';
into this:
barimport './a';
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100466/new/
https://reviews.llvm.org/D100466
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
