Re: [PATCH] D24516: [Driver][Diagnostics] Make 'show option names' default for driver warnings
bruno added a comment. Ping! https://reviews.llvm.org/D24516 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24472: [Sema] Support lax conversions for compound assignments
bruno marked an inline comment as done. Comment at: lib/Sema/SemaExpr.cpp:8084 @@ +8083,3 @@ + *RHSExpr = ImpCastExprToType(RHSExpr->get(), LHSType, CK_BitCast); + return LHSType; +} ahatanak wrote: > My understanding is that, when we have a compound assign like "LHS += RHS", > this function (CheckVectorOperands) is supposed to return the result type > (LHS + RHS). However, it is returning different types for "<1 x T> += T" and > "T += <1 x T>" (the former returns <1 x T> and the latter returns T). Would > CheckAssignmentOperands reject the compound statement if you returned the > vector type here? > > Also, are you planning to allow the same kind of conversions done above for > non-compound assignment statements (e.g., <4 x short> += <2 x int>) in the > future? 1) CheckAssignmentOperands doesn't reject the compound statement if we return the vector type instead, because it already supports vector to scalar cast idiom. It makes more sense to return a vector indeed, gonna change that. 2) It would be nice to catch up with GCC with the idioms supported for regular (non-ext) vectors (like splatting scalar operands to vectors in a arith binops), and those, AFAIK, don't include support for truncation as in the example you suggested. I guess this is supported with ext-vectors, but I don't see any reason to support it for "regular" vectors. https://reviews.llvm.org/D24472 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24472: [Sema] Support lax conversions for compound assignments
bruno updated this revision to Diff 71869.
bruno marked an inline comment as done.
bruno added a comment.
Update after Akira's comment
https://reviews.llvm.org/D24472
Files:
lib/Sema/SemaExpr.cpp
test/Sema/vector-cast.c
Index: test/Sema/vector-cast.c
===
--- test/Sema/vector-cast.c
+++ test/Sema/vector-cast.c
@@ -53,14 +53,13 @@
float2 f2;
double d, a, b, c;
float64x2_t v = {0.0, 1.0};
- f2 += d;
+ // FIXME: These diagnostics are inaccurate: should complain that 'double' to
vector 'float2' involves truncation
+ f2 += d; // expected-error {{cannot convert between vector values of
different size ('float2' (vector of 2 'float' values) and 'double')}}
+ d += f2; // expected-error {{cannot convert between vector values of
different size}}
a = 3.0 + vget_low_f64(v);
b = vget_low_f64(v) + 3.0;
c = vget_low_f64(v);
- // LAX conversions within compound assignments are not supported.
- // FIXME: This diagnostic is inaccurate.
- d += f2; // expected-error {{cannot convert between vector values of
different size}}
- c -= vget_low_f64(v); // expected-error {{cannot convert between vector
values of different size}}
+ c -= vget_low_f64(v);
// LAX conversions between scalar and vector types require same size and one
element sized vectors.
d = f2; // expected-error {{assigning to 'double' from incompatible type
'float2'}}
d = d + f2; // expected-error {{assigning to 'double' from incompatible type
'float2'}}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -8051,6 +8051,7 @@
// If there's an ext-vector type and a scalar, try to convert the scalar to
// the vector element type and splat.
+ // FIXME: this should also work for regular vector types as supported in GCC.
if (!RHSVecType && isa(LHSVecType)) {
if (!tryVectorConvertAndSplat(*this, &RHS, RHSType,
LHSVecType->getElementType(), LHSType))
@@ -8063,16 +8064,31 @@
return RHSType;
}
- // If we're allowing lax vector conversions, only the total (data) size needs
- // to be the same. If one of the types is scalar, the result is always the
- // vector type. Don't allow this if the scalar operand is an lvalue.
+ // FIXME: The code below also handles convertion between vectors and
+ // non-scalars, we should break this down into fine grained specific checks
+ // and emit proper diagnostics.
QualType VecType = LHSVecType ? LHSType : RHSType;
- QualType ScalarType = LHSVecType ? RHSType : LHSType;
- ExprResult *ScalarExpr = LHSVecType ? &RHS : &LHS;
- if (isLaxVectorConversion(ScalarType, VecType) &&
- !ScalarExpr->get()->isLValue()) {
-*ScalarExpr = ImpCastExprToType(ScalarExpr->get(), VecType, CK_BitCast);
-return VecType;
+ const VectorType *VT = LHSVecType ? LHSVecType : RHSVecType;
+ QualType OtherType = LHSVecType ? RHSType : LHSType;
+ ExprResult *OtherExpr = LHSVecType ? &RHS : &LHS;
+ if (isLaxVectorConversion(OtherType, VecType)) {
+// If we're allowing lax vector conversions, only the total (data) size
+// needs to be the same. For non compound assignment, if one of the types
is
+// scalar, the result is always the vector type.
+if (!IsCompAssign) {
+ *OtherExpr = ImpCastExprToType(OtherExpr->get(), VecType, CK_BitCast);
+ return VecType;
+// In a compound assignment, lhs += rhs, 'lhs' is a lvalue src, forbidding
+// any implicit cast. Here, the 'rhs' should be implicit casted to 'lhs'
+// type. Note that this is already done by non-compound assignments in
+// CheckAssignmentConstraints. If it's a scalar type, only biscast for
+// <1 x T> -> T.
+} else if (OtherType->isExtVectorType() ||
+ (OtherType->isScalarType() && VT->getNumElements() == 1)) {
+ ExprResult *RHSExpr = &RHS;
+ *RHSExpr = ImpCastExprToType(RHSExpr->get(), LHSType, CK_BitCast);
+ return LHSType;
+}
}
// Okay, the expression is invalid.
Index: test/Sema/vector-cast.c
===
--- test/Sema/vector-cast.c
+++ test/Sema/vector-cast.c
@@ -53,14 +53,13 @@
float2 f2;
double d, a, b, c;
float64x2_t v = {0.0, 1.0};
- f2 += d;
+ // FIXME: These diagnostics are inaccurate: should complain that 'double' to vector 'float2' involves truncation
+ f2 += d; // expected-error {{cannot convert between vector values of different size ('float2' (vector of 2 'float' values) and 'double')}}
+ d += f2; // expected-error {{cannot convert between vector values of different size}}
a = 3.0 + vget_low_f64(v);
b = vget_low_f64(v) + 3.0;
c = vget_low_f64(v);
- // LAX conversions within compound assignments are not supported.
- // FIXME: This diagnostic is inaccurate.
- d += f2; // expected-error {{cannot convert between vector values of different si
Re: [PATCH] D24472: [Sema] Support lax conversions for compound assignments
bruno updated this revision to Diff 71870.
bruno added a comment.
Update again (now with the right patch) after Akira's comment
https://reviews.llvm.org/D24472
Files:
lib/Sema/SemaExpr.cpp
test/Sema/vector-cast.c
Index: test/Sema/vector-cast.c
===
--- test/Sema/vector-cast.c
+++ test/Sema/vector-cast.c
@@ -53,14 +53,13 @@
float2 f2;
double d, a, b, c;
float64x2_t v = {0.0, 1.0};
- f2 += d;
+ // FIXME: These diagnostics are inaccurate: should complain that 'double' to
vector 'float2' involves truncation
+ f2 += d; // expected-error {{cannot convert between vector values of
different size ('float2' (vector of 2 'float' values) and 'double')}}
+ d += f2; // expected-error {{cannot convert between vector values of
different size}}
a = 3.0 + vget_low_f64(v);
b = vget_low_f64(v) + 3.0;
c = vget_low_f64(v);
- // LAX conversions within compound assignments are not supported.
- // FIXME: This diagnostic is inaccurate.
- d += f2; // expected-error {{cannot convert between vector values of
different size}}
- c -= vget_low_f64(v); // expected-error {{cannot convert between vector
values of different size}}
+ c -= vget_low_f64(v);
// LAX conversions between scalar and vector types require same size and one
element sized vectors.
d = f2; // expected-error {{assigning to 'double' from incompatible type
'float2'}}
d = d + f2; // expected-error {{assigning to 'double' from incompatible type
'float2'}}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -8051,6 +8051,7 @@
// If there's an ext-vector type and a scalar, try to convert the scalar to
// the vector element type and splat.
+ // FIXME: this should also work for regular vector types as supported in GCC.
if (!RHSVecType && isa(LHSVecType)) {
if (!tryVectorConvertAndSplat(*this, &RHS, RHSType,
LHSVecType->getElementType(), LHSType))
@@ -8063,16 +8064,31 @@
return RHSType;
}
- // If we're allowing lax vector conversions, only the total (data) size needs
- // to be the same. If one of the types is scalar, the result is always the
- // vector type. Don't allow this if the scalar operand is an lvalue.
+ // FIXME: The code below also handles convertion between vectors and
+ // non-scalars, we should break this down into fine grained specific checks
+ // and emit proper diagnostics.
QualType VecType = LHSVecType ? LHSType : RHSType;
- QualType ScalarType = LHSVecType ? RHSType : LHSType;
- ExprResult *ScalarExpr = LHSVecType ? &RHS : &LHS;
- if (isLaxVectorConversion(ScalarType, VecType) &&
- !ScalarExpr->get()->isLValue()) {
-*ScalarExpr = ImpCastExprToType(ScalarExpr->get(), VecType, CK_BitCast);
-return VecType;
+ const VectorType *VT = LHSVecType ? LHSVecType : RHSVecType;
+ QualType OtherType = LHSVecType ? RHSType : LHSType;
+ ExprResult *OtherExpr = LHSVecType ? &RHS : &LHS;
+ if (isLaxVectorConversion(OtherType, VecType)) {
+// If we're allowing lax vector conversions, only the total (data) size
+// needs to be the same. For non compound assignment, if one of the types
is
+// scalar, the result is always the vector type.
+if (!IsCompAssign) {
+ *OtherExpr = ImpCastExprToType(OtherExpr->get(), VecType, CK_BitCast);
+ return VecType;
+// In a compound assignment, lhs += rhs, 'lhs' is a lvalue src, forbidding
+// any implicit cast. Here, the 'rhs' should be implicit casted to 'lhs'
+// type. Note that this is already done by non-compound assignments in
+// CheckAssignmentConstraints. If it's a scalar type, only biscast for
+// <1 x T> -> T. The result is also a vector type.
+} else if (OtherType->isExtVectorType() ||
+ (OtherType->isScalarType() && VT->getNumElements() == 1)) {
+ ExprResult *RHSExpr = &RHS;
+ *RHSExpr = ImpCastExprToType(RHSExpr->get(), LHSType, CK_BitCast);
+ return VecType;
+}
}
// Okay, the expression is invalid.
Index: test/Sema/vector-cast.c
===
--- test/Sema/vector-cast.c
+++ test/Sema/vector-cast.c
@@ -53,14 +53,13 @@
float2 f2;
double d, a, b, c;
float64x2_t v = {0.0, 1.0};
- f2 += d;
+ // FIXME: These diagnostics are inaccurate: should complain that 'double' to vector 'float2' involves truncation
+ f2 += d; // expected-error {{cannot convert between vector values of different size ('float2' (vector of 2 'float' values) and 'double')}}
+ d += f2; // expected-error {{cannot convert between vector values of different size}}
a = 3.0 + vget_low_f64(v);
b = vget_low_f64(v) + 3.0;
c = vget_low_f64(v);
- // LAX conversions within compound assignments are not supported.
- // FIXME: This diagnostic is inaccurate.
- d += f2; // expected-error {{cannot convert between ve
Re: [PATCH] D24820: Add -stats-file option
bruno added inline comments. Comment at: lib/Driver/Tools.cpp:6102 @@ +6101,3 @@ +StatsFile.assign(Output.getFilename()); +llvm::sys::path::remove_filename(StatsFile); + } Why removing StatsFile here? IIUC, at this point StatsFile is still the same as the output (if it's a file). Repository: rL LLVM https://reviews.llvm.org/D24820 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24820: Add -stats-file option
bruno added a comment.
Maybe add some docs to explain the new flags?
Comment at: lib/Driver/Tools.cpp:6102
@@ +6101,3 @@
+StatsFile.assign(Output.getFilename());
+llvm::sys::path::remove_filename(StatsFile);
+ }
MatzeB wrote:
> bruno wrote:
> > Why removing StatsFile here? IIUC, at this point StatsFile is still the
> > same as the output (if it's a file).
> a) It behaves like -save-temps=obj (`clang -save-temps=obj foo.c -o bar` will
> give you foo.i, foo.o, ...;
> b) This makes sense when you have multiple (`clang -save-stats=obj foo.c
> bar.c -o baz`) inputs for which you need multiple .stats filenames but you
> only have 1 output name
> c) It magically works for `-o -` as well
>
Ok, I see now, I misread `remove_filename` as `remove`
Comment at: test/Driver/save-stats.c:1
@@ +1,2 @@
+// RUN: %clang -target x86_64-apple-darwin -save-stats %s -### 2>&1 |
FileCheck %s
+// RUN: %clang -target x86_64-apple-darwin -save-stats=cwd %s -### 2>&1 |
FileCheck %s
Is -save-stats == -save-stats=cwd? It doesn't seem so by looking at
lib/Driver/Tools.cpp. Need test for the diag::err_drv_invalid_value as well.
Comment at: test/Driver/save-stats.c:12
@@ +11,3 @@
+// RUN: %clang -target x86_64-apple-darwin -save-stats=obj -c -o
obj/dir/save-stats.o %s -### 2>&1 | FileCheck %s -check-prefix=CHECK-OBJ
+// CHECK-OBJ: "-stats-file=obj/dir{{/|}}save-stats.stats"
+// CHECK-OBJ: "-o" "obj/dir{{/|}}save-stats.o"
aprantl wrote:
> This is up to taste, but just accepting {{.}} as the path separator would be
> sufficient IMO and might be more readable.
+1 to Adrian's suggestion
Repository:
rL LLVM
https://reviews.llvm.org/D24820
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24472: [Sema] Support lax conversions for compound assignments
bruno added inline comments. Comment at: lib/Sema/SemaExpr.cpp:8090 @@ +8089,3 @@ + *RHSExpr = ImpCastExprToType(RHSExpr->get(), LHSType, CK_BitCast); + return VecType; +} ahatanak wrote: > Sorry I wasn't clear, but I was asking whether you were planning to allow > the following conversions for compound statements. > > ``` > typedef short s4 __attribute__ ((vector_size(8))); > typedef int i2 __attribute__ ((vector_size(8))); > s4 a; > i2 b; > a = a + b; // clang accepts this. > a += b; // currently clang rejects this. > ``` > > Also, I feel clang is inconsistent in warning about incompatible types. In > the following example, it issues a warning for the first line, but is silent > about the second line: > > ``` > a = b + a; // incompatible vector types warning > a = a + b; // no warning > ``` > > I don't think we have to fix everything in this patch, but just wanted to > know what your thoughts were. You're right, the diagnostics are bad here, this patch adds some FIXMEs so we can later work on it. A PR would be nice though (we have an internal track for that as well, see rdar://problem/28067874). Given your example: a = a + b; // clang accepts this. a += b; // currently clang rejects this. IMO, clang should reject `a = a + b` too if not in OpenCL mode, which means whenever (a) `a` and `b` have same size but different num elt and (b) `a` and `b` are generic vectors (non ext-vectors). However, It looks like we have tests that rely on this behavior, we should probably find out why first and clean it up. I also think we should support splatting when one of the operands is a scalar and the other a non ext-vector (as of now we currently only do it for OpenCL). This is basically what GCC supports https://gcc.gnu.org/onlinedocs/gcc/Vector-Extensions.html. https://reviews.llvm.org/D24472 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24820: Add -stats-stats option
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Thanks Matthias LGTM! Repository: rL LLVM https://reviews.llvm.org/D24820 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24516: [Driver][Diagnostics] Make 'show option names' default for driver warnings
bruno added a comment. Ping https://reviews.llvm.org/D24516 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24752: [Modules] Add missing dependencies to clang builtins modulemap
bruno added a subscriber: bruno.
Comment at: lib/Headers/module.modulemap:133
@@ -131,2 +132,3 @@
explicit module aes {
+ export sse2
header "__wmmintrin_aes.h"
The mmx case above makes sense to me, but I find conceptually odd that we need
to re-export sse2 in aes module, why not explicitly #include in
the source file?
https://reviews.llvm.org/D24752
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24861: [Sema] extend Wshift-op-parentheses so it warns for multiplicative operators
bruno added a subscriber: bruno. bruno added a comment. Hi Daniel, This is very nice. In https://reviews.llvm.org/D24861#553606, @danielmarjamaki wrote: > Compiling 2064 projects resulted in 904 warnings > > Here are the results: > https://drive.google.com/file/d/0BykPmWrCOxt2N04tYl8zVHA3MXc/view?usp=sharing > > The results looks acceptable imho. The code looks intentional in many cases > so I believe there are users that will disable this warning. Probably there > are true positives where the evaluation order is not really known. There were > many warnings about macro arguments where the macro bitshifts the argument - > these macros look very shaky to me. > > I saw some warnings about such code: > > a * b << c > > > Maybe we should not warn about this. As far as I see, the result will be the > same if (a*b) or (b< signedness issues. What do you think? I'll keep these warnings for now. Any idea on how expensive would be to reason about these false positives and avoid them? Repository: rL LLVM https://reviews.llvm.org/D24861 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24472: [Sema] Support lax conversions for compound assignments
bruno added a comment. @rnk, do you have any concerns about this patch? https://reviews.llvm.org/D24472 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r282968 - [Sema] Support lax conversions for compound assignments
Author: bruno
Date: Fri Sep 30 17:19:38 2016
New Revision: 282968
URL: http://llvm.org/viewvc/llvm-project?rev=282968&view=rev
Log:
[Sema] Support lax conversions for compound assignments
Support lax convertions on compound assignment expressions like:
typedef __attribute__((vector_size(8))) double float64x1_t;
typedef __attribute__((vector_size(16))) double float64x2_t;
float64x1_t vget_low_f64(float64x2_t __p0);
double c = 3.0;
float64x2_t v = {0.0, 1.0};
c += vget_low_f64(v);
This restores one more valid behavior pre r266366, and is a incremental
follow up from work committed in r274646.
While here, make the check more strict, add FIXMEs, clean up variable
names to match what they can actually be and update testcases to reflect
that. We now reject:
typedef float float2 __attribute__ ((vector_size (8)));
double d;
f2 += d;
which doesn't fit as a direct bitcast anyway.
Differential Revision: https://reviews.llvm.org/D24472
rdar://problem/28033929
Modified:
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/vector-cast.c
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=282968&r1=282967&r2=282968&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Sep 30 17:19:38 2016
@@ -8052,6 +8052,7 @@ QualType Sema::CheckVectorOperands(ExprR
// If there's an ext-vector type and a scalar, try to convert the scalar to
// the vector element type and splat.
+ // FIXME: this should also work for regular vector types as supported in GCC.
if (!RHSVecType && isa(LHSVecType)) {
if (!tryVectorConvertAndSplat(*this, &RHS, RHSType,
LHSVecType->getElementType(), LHSType))
@@ -8064,16 +8065,31 @@ QualType Sema::CheckVectorOperands(ExprR
return RHSType;
}
- // If we're allowing lax vector conversions, only the total (data) size needs
- // to be the same. If one of the types is scalar, the result is always the
- // vector type. Don't allow this if the scalar operand is an lvalue.
+ // FIXME: The code below also handles convertion between vectors and
+ // non-scalars, we should break this down into fine grained specific checks
+ // and emit proper diagnostics.
QualType VecType = LHSVecType ? LHSType : RHSType;
- QualType ScalarType = LHSVecType ? RHSType : LHSType;
- ExprResult *ScalarExpr = LHSVecType ? &RHS : &LHS;
- if (isLaxVectorConversion(ScalarType, VecType) &&
- !ScalarExpr->get()->isLValue()) {
-*ScalarExpr = ImpCastExprToType(ScalarExpr->get(), VecType, CK_BitCast);
-return VecType;
+ const VectorType *VT = LHSVecType ? LHSVecType : RHSVecType;
+ QualType OtherType = LHSVecType ? RHSType : LHSType;
+ ExprResult *OtherExpr = LHSVecType ? &RHS : &LHS;
+ if (isLaxVectorConversion(OtherType, VecType)) {
+// If we're allowing lax vector conversions, only the total (data) size
+// needs to be the same. For non compound assignment, if one of the types
is
+// scalar, the result is always the vector type.
+if (!IsCompAssign) {
+ *OtherExpr = ImpCastExprToType(OtherExpr->get(), VecType, CK_BitCast);
+ return VecType;
+// In a compound assignment, lhs += rhs, 'lhs' is a lvalue src, forbidding
+// any implicit cast. Here, the 'rhs' should be implicit casted to 'lhs'
+// type. Note that this is already done by non-compound assignments in
+// CheckAssignmentConstraints. If it's a scalar type, only bitcast for
+// <1 x T> -> T. The result is also a vector type.
+} else if (OtherType->isExtVectorType() ||
+ (OtherType->isScalarType() && VT->getNumElements() == 1)) {
+ ExprResult *RHSExpr = &RHS;
+ *RHSExpr = ImpCastExprToType(RHSExpr->get(), LHSType, CK_BitCast);
+ return VecType;
+}
}
// Okay, the expression is invalid.
Modified: cfe/trunk/test/Sema/vector-cast.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/vector-cast.c?rev=282968&r1=282967&r2=282968&view=diff
==
--- cfe/trunk/test/Sema/vector-cast.c (original)
+++ cfe/trunk/test/Sema/vector-cast.c Fri Sep 30 17:19:38 2016
@@ -53,14 +53,13 @@ void f4() {
float2 f2;
double d, a, b, c;
float64x2_t v = {0.0, 1.0};
- f2 += d;
+ // FIXME: These diagnostics are inaccurate: should complain that 'double' to
vector 'float2' involves truncation
+ f2 += d; // expected-error {{cannot convert between vector values of
different size ('float2' (vector of 2 'float' values) and 'double')}}
+ d += f2; // expected-error {{cannot convert between vector values of
different size}}
a = 3.0 + vget_low_f64(v);
b = vget_low_f64(v) + 3.0;
c = vget_low_f64(v);
- // LAX conversions within compound assignments are not supported.
- // FIXME: This diagnostic is inaccurate.
- d += f2; // exp
[PATCH] D24472: [Sema] Support lax conversions for compound assignments
bruno closed this revision. bruno added a comment. Thanks Reid & Akira, Committed r282968 https://reviews.llvm.org/D24472 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24516: [Driver][Diagnostics] Make 'show option names' default for driver warnings
bruno added a comment. Ping! https://reviews.llvm.org/D24516 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24752: [Modules] Add missing dependencies to clang builtins modulemap
bruno added inline comments. > eladcohen wrote in module.modulemap:133 > emmintrin.h is already included explicitly in wmmintrin.h & __wmmintrin_aes.h. > > If a user includes / there is no problem, since the > intel submodule has an 'export *' directive and both aes & sse2 will be > imported. > > However, if the user only includes (like in the 2nd half of the > test I added), and uses modules, then any use of the '___m128i' type (which > is used in the aes intrinsics and declared in sse2) will result with the > error: > "File tst.c Line 11: missing '#include '; declaration of > '___m128i' must be imported from module '_Builtin_intrinsics.intel.sse2' > before it is required" > > IIUC the possible fixes for this are adding 'export *' or 'export sse2' to > the aes submodule. I see, if you need the type to use the exported functions it makes sense to re-export it. It might be worth adding a comment // note: for ___m128i https://reviews.llvm.org/D24752 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25337: [Modules] Add a command line option for enabling the modules feature exclusively for the Clang builtins.
bruno added a reviewer: bruno. bruno added a comment. Hi Elad, Is there any reason why you can't explicit model this in your build system by pre-building the intrinsics and pointing to a module cache path containing the pcm files? By doing that we don't need to have a specific compile flag. https://reviews.llvm.org/D25337 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25338: [Driver] Make -print-libgcc-file-name print compiler-rt lib when used
bruno added a reviewer: bruno.
bruno added a comment.
Testcase?
> clang.rst:398
> + Print the library path for the currently used compiler runtime library
> + ("libgcc.a" or "libclang_rt.builtins.*.a" appropriately).
>
You can probably drop the "appropriately"
> Options.td:1865
> + HelpText<"Print the library path for the currently used compiler runtime "
> + "library (\"libgcc.a\" or \"libclang_rt.builtins.*.a\"
> appropriately)">;
> def print_multi_directory : Flag<["-", "--"], "print-multi-directory">;
Same here
https://reviews.llvm.org/D25338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25338: [Driver] Make -print-libgcc-file-name print compiler-rt lib when used
bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D25338 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24669: {Sema] Gcc compatibility of vector shift.
bruno added a reviewer: bruno.
bruno added inline comments.
Comment at: llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td:2306
+ "vector operands do not have the same elements sizes (%0 and %1)">,
+ InGroup>, DefaultError;
def err_ext_vector_component_exceeds_length : Error<
Although the motivation is to support the same warning present in GCC, I think
this is helpful enough anyway so that we might skip calling it
"gnu-vec-elem-size" and have a more generic name instead? How about plain
"vec-elem-size"?
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8787
}
+if (!S.LangOpts.OpenCL && !S.LangOpts.ZVector) {
+ const BuiltinType *LHSBT = LHSEleType->getAs();
Besides `__ext_vector_type__`, would this also trigger for `vector_size`? Right
now this is an error for `vector_size` primarily because the number of elements
is different, can you confirm this won't change?
https://reviews.llvm.org/D24669
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25338: [Driver] Make -print-libgcc-file-name print compiler-rt lib when used
bruno accepted this revision. bruno added a comment. LGTM https://reviews.llvm.org/D25338 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24954: [ToolChains] Disable OpenSUSE rules for SLES10
bruno added a reviewer: bruno.
bruno added inline comments.
Comment at: lib/Driver/ToolChains.cpp:3915
- if (D.getVFS().exists("/etc/SuSE-release"))
-return OpenSUSE;
+ File = llvm::MemoryBuffer::getFile("/etc/SuSE-release");
+ if (File)
You should keep using the VFS to obtain the file. You probably also want to
add a comment here to describe what you mentioned in the patch Summary.
https://reviews.llvm.org/D24954
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25263: [Driver] Allow setting the default linker during build
bruno added inline comments. Comment at: CMakeLists.txt:198 +set(CLANG_DEFAULT_LINKER "" CACHE STRING + "Default linker to use (\"bfd\" or \"gold\" or \"lld\", empty for platform default") mgorny wrote: > Is there a reason not to allow using the absolute path here, like for the > command-line option? I agree here, if we're adding a cmake options for this, it should accept full paths to the linker to be used (without any need for its type like gold, bfd, etc) as well. Additionally, if "" maps to "ld", plain CLANG_DEFAULT_LINKER="ld" should also work here. Repository: rL LLVM https://reviews.llvm.org/D25263 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24954: [ToolChains] Disable OpenSUSE rules for SLES10
bruno added inline comments.
Comment at: lib/Driver/ToolChains.cpp:3915
- if (D.getVFS().exists("/etc/SuSE-release"))
-return OpenSUSE;
+ File = llvm::MemoryBuffer::getFile("/etc/SuSE-release");
+ if (File)
mgorny wrote:
> bruno wrote:
> > You should keep using the VFS to obtain the file. You probably also want
> > to add a comment here to describe what you mentioned in the patch Summary.
> Hmm, this method is consistent with all the other distributions in the
> method. It seems that `getVFS()` is only used for `exists()` checks there.
> Are you sure I should change this, without touching the other reads first?
Right, there seems to be some inconsistent usage here. Ideally we should go
through the VFS, but given the precedence it's understandable if you don't make
it in this patch.
AFAIU, this drops support for SLES10 and I guess that this would also be true
for SLES9? It seems that the checking could be improved a bit by checking the
digits (as in the distros above)? Also add a comment explaining why.
https://reviews.llvm.org/D24954
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24669: {Sema] Gcc compatibility of vector shift.
bruno added inline comments.
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8787
}
+if (!S.LangOpts.OpenCL && !S.LangOpts.ZVector) {
+ const BuiltinType *LHSBT = LHSEleType->getAs();
vbyakovlcl wrote:
> bruno wrote:
> > Besides `__ext_vector_type__`, would this also trigger for `vector_size`?
> > Right now this is an error for `vector_size` primarily because the number
> > of elements is different, can you confirm this won't change?
> I compare vector element sizes, so there must not be any differencies caused
> by different triggers. I added additional type definitions to the tests. All
> compiles fain.
I don't think this is right. When I try to compile similar code for
`vector_size` without your patch, I get the errors:
/tmp/x.c:80:15: error: vector operands do not have the same number of elements
('vector_int8n' (vector of 2 'int' values) and 'vector_uchar8n' (vector of 8
'unsigned char' values))
vi8n = vi8n << vuc8n; // expected-warning {{vector operands do not have the
same elements sizes}}
^ ~
/tmp/x.c:81:17: error: vector operands do not have the same number of elements
('vector_uchar8n' (vector of 8 'unsigned char' values) and 'vector_int8n'
(vector of 2 'int' values))
vuc8n = vuc8n << vi8n; // expected-warning {{vector operands do not have the
same elements sizes}}
~ ^
/tmp/x.c:82:17: error: vector operands do not have the same number of elements
('vector_ushort8n' (vector of 4 'unsigned short' values) and 'vector_uint8n'
(vector of 2 'unsigned int' values))
vus8n = vus8n << vui8n; // expected-warning {{vector operands do not have the
same elements sizes}}
~ ^ ~
/tmp/x.c:83:17: error: vector operands do not have the same number of elements
('vector_uint8n' (vector of 2 'unsigned int' values) and 'vector_short8n'
(vector of 4 'short' values))
vui8n = vui8n << vs8n; // expected-warning {{vector operands do not have the
same elements sizes}}
~ ^
Given your test changes, it seems that now, instead of "vector operands do not
have the same number of elements" we would get "vector operands do not have the
same elements sizes". I rather we stay with the first. Additionally, even if we
had "vector operands do not have the same elements sizes" for `vector_size`,
this should never be demoted to a warning.
https://reviews.llvm.org/D24669
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24516: [Driver][Diagnostics] Make 'show option names' default for driver warnings
bruno added a reviewer: vsk. bruno added a comment. Ping!! https://reviews.llvm.org/D24516 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24516: [Driver][Diagnostics] Make 'show option names' default for driver warnings
bruno updated this revision to Diff 74185.
bruno added a comment.
Update after Vedant's review!
https://reviews.llvm.org/D24516
Files:
include/clang/Frontend/CompilerInvocation.h
lib/Frontend/CompilerInvocation.cpp
test/Driver/show-option-names.c
Index: test/Driver/show-option-names.c
===
--- /dev/null
+++ test/Driver/show-option-names.c
@@ -0,0 +1,8 @@
+// RUN: %clang -c -target i386-apple-darwin10 -isysroot /FOO %s 2>&1 |
FileCheck --check-prefix=CHECK-SHOW-OPTION-NAMES %s
+// CHECK-SHOW-OPTION-NAMES: warning: no such sysroot directory:
'{{([A-Za-z]:.*)?}}/FOO' [-Wmissing-sysroot]
+
+// RUN: %clang -c -target i386-apple-darwin10 -fno-diagnostics-show-option
-isysroot /FOO %s 2>&1 | FileCheck --check-prefix=CHECK-NO-SHOW-OPTION-NAMES-A
%s
+// CHECK-NO-SHOW-OPTION-NAMES-A: warning: no such sysroot directory:
'{{([A-Za-z]:.*)?}}/FOO'
+
+// RUN: %clang -c -target i386-apple-darwin10 -fno-diagnostics-show-option
-isysroot /FOO %s 2>&1 | FileCheck --check-prefix=CHECK-NO-SHOW-OPTION-NAMES-B
%s
+// CHECK-NO-SHOW-OPTION-NAMES-B-NOT: warning: no such sysroot directory:
'{{([A-Za-z]:.*)?}}/FOO' [-Wmissing-sysroot]
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -950,7 +950,7 @@
bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
DiagnosticsEngine *Diags,
-bool DefaultDiagColor) {
+bool DefaultDiagColor, bool DefaultShowOpt) {
using namespace options;
bool Success = true;
@@ -970,7 +970,9 @@
Opts.ShowFixits = !Args.hasArg(OPT_fno_diagnostics_fixit_info);
Opts.ShowLocation = !Args.hasArg(OPT_fno_show_source_location);
Opts.AbsolutePath = Args.hasArg(OPT_fdiagnostics_absolute_paths);
- Opts.ShowOptionNames = Args.hasArg(OPT_fdiagnostics_show_option);
+ Opts.ShowOptionNames =
+ Args.hasFlag(OPT_fdiagnostics_show_option,
+ OPT_fno_diagnostics_show_option, DefaultShowOpt);
llvm::sys::Process::UseANSIEscapeCodes(Args.hasArg(OPT_fansi_escape_codes));
@@ -2383,8 +2385,9 @@
Success &= ParseAnalyzerArgs(*Res.getAnalyzerOpts(), Args, Diags);
Success &= ParseMigratorArgs(Res.getMigratorOpts(), Args);
ParseDependencyOutputArgs(Res.getDependencyOutputOpts(), Args);
- Success &= ParseDiagnosticArgs(Res.getDiagnosticOpts(), Args, &Diags,
- false /*DefaultDiagColor*/);
+ Success &=
+ ParseDiagnosticArgs(Res.getDiagnosticOpts(), Args, &Diags,
+ false /*DefaultDiagColor*/, false
/*DefaultShowOpt*/);
ParseCommentArgs(LangOpts.CommentOpts, Args);
ParseFileSystemArgs(Res.getFileSystemOpts(), Args);
// FIXME: We shouldn't have to pass the DashX option around here
Index: include/clang/Frontend/CompilerInvocation.h
===
--- include/clang/Frontend/CompilerInvocation.h
+++ include/clang/Frontend/CompilerInvocation.h
@@ -48,7 +48,8 @@
/// report the error(s).
bool ParseDiagnosticArgs(DiagnosticOptions &Opts, llvm::opt::ArgList &Args,
DiagnosticsEngine *Diags = nullptr,
- bool DefaultDiagColor = true);
+ bool DefaultDiagColor = true,
+ bool DefaultShowOpt = true);
class CompilerInvocationBase : public RefCountedBase {
void operator=(const CompilerInvocationBase &) = delete;
Index: test/Driver/show-option-names.c
===
--- /dev/null
+++ test/Driver/show-option-names.c
@@ -0,0 +1,8 @@
+// RUN: %clang -c -target i386-apple-darwin10 -isysroot /FOO %s 2>&1 | FileCheck --check-prefix=CHECK-SHOW-OPTION-NAMES %s
+// CHECK-SHOW-OPTION-NAMES: warning: no such sysroot directory: '{{([A-Za-z]:.*)?}}/FOO' [-Wmissing-sysroot]
+
+// RUN: %clang -c -target i386-apple-darwin10 -fno-diagnostics-show-option -isysroot /FOO %s 2>&1 | FileCheck --check-prefix=CHECK-NO-SHOW-OPTION-NAMES-A %s
+// CHECK-NO-SHOW-OPTION-NAMES-A: warning: no such sysroot directory: '{{([A-Za-z]:.*)?}}/FOO'
+
+// RUN: %clang -c -target i386-apple-darwin10 -fno-diagnostics-show-option -isysroot /FOO %s 2>&1 | FileCheck --check-prefix=CHECK-NO-SHOW-OPTION-NAMES-B %s
+// CHECK-NO-SHOW-OPTION-NAMES-B-NOT: warning: no such sysroot directory: '{{([A-Za-z]:.*)?}}/FOO' [-Wmissing-sysroot]
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -950,7 +950,7 @@
bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
DiagnosticsEngine *Diags,
-bool DefaultDiagColor) {
+
r283827 - [Driver][Diagnostics] Make 'show option names' default for driver warnings
Author: bruno
Date: Mon Oct 10 19:01:22 2016
New Revision: 283827
URL: http://llvm.org/viewvc/llvm-project?rev=283827&view=rev
Log:
[Driver][Diagnostics] Make 'show option names' default for driver warnings
Currently, driver level warnings do not show option names (e.g. warning:
complain about foo [-Woption-name]) in a diagnostic unless
-fdiagnostics-show-option is explictly specified. OTOH, the driver by
default turn this option on for CC1. Change the logic to show option
names by default in the driver as well.
Differential Revision: https://reviews.llvm.org/D24516
rdar://problem/27300909
Added:
cfe/trunk/test/Driver/show-option-names.c
Modified:
cfe/trunk/include/clang/Frontend/CompilerInvocation.h
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
Modified: cfe/trunk/include/clang/Frontend/CompilerInvocation.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CompilerInvocation.h?rev=283827&r1=283826&r2=283827&view=diff
==
--- cfe/trunk/include/clang/Frontend/CompilerInvocation.h (original)
+++ cfe/trunk/include/clang/Frontend/CompilerInvocation.h Mon Oct 10 19:01:22
2016
@@ -48,7 +48,8 @@ class DiagnosticsEngine;
/// report the error(s).
bool ParseDiagnosticArgs(DiagnosticOptions &Opts, llvm::opt::ArgList &Args,
DiagnosticsEngine *Diags = nullptr,
- bool DefaultDiagColor = true);
+ bool DefaultDiagColor = true,
+ bool DefaultShowOpt = true);
class CompilerInvocationBase : public RefCountedBase {
void operator=(const CompilerInvocationBase &) = delete;
Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=283827&r1=283826&r2=283827&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Mon Oct 10 19:01:22 2016
@@ -953,7 +953,7 @@ static bool parseShowColorsArgs(const Ar
bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
DiagnosticsEngine *Diags,
-bool DefaultDiagColor) {
+bool DefaultDiagColor, bool DefaultShowOpt) {
using namespace options;
bool Success = true;
@@ -973,7 +973,9 @@ bool clang::ParseDiagnosticArgs(Diagnost
Opts.ShowFixits = !Args.hasArg(OPT_fno_diagnostics_fixit_info);
Opts.ShowLocation = !Args.hasArg(OPT_fno_show_source_location);
Opts.AbsolutePath = Args.hasArg(OPT_fdiagnostics_absolute_paths);
- Opts.ShowOptionNames = Args.hasArg(OPT_fdiagnostics_show_option);
+ Opts.ShowOptionNames =
+ Args.hasFlag(OPT_fdiagnostics_show_option,
+ OPT_fno_diagnostics_show_option, DefaultShowOpt);
llvm::sys::Process::UseANSIEscapeCodes(Args.hasArg(OPT_fansi_escape_codes));
@@ -2400,8 +2402,9 @@ bool CompilerInvocation::CreateFromArgs(
Success &= ParseAnalyzerArgs(*Res.getAnalyzerOpts(), Args, Diags);
Success &= ParseMigratorArgs(Res.getMigratorOpts(), Args);
ParseDependencyOutputArgs(Res.getDependencyOutputOpts(), Args);
- Success &= ParseDiagnosticArgs(Res.getDiagnosticOpts(), Args, &Diags,
- false /*DefaultDiagColor*/);
+ Success &=
+ ParseDiagnosticArgs(Res.getDiagnosticOpts(), Args, &Diags,
+ false /*DefaultDiagColor*/, false
/*DefaultShowOpt*/);
ParseCommentArgs(LangOpts.CommentOpts, Args);
ParseFileSystemArgs(Res.getFileSystemOpts(), Args);
// FIXME: We shouldn't have to pass the DashX option around here
Added: cfe/trunk/test/Driver/show-option-names.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/show-option-names.c?rev=283827&view=auto
==
--- cfe/trunk/test/Driver/show-option-names.c (added)
+++ cfe/trunk/test/Driver/show-option-names.c Mon Oct 10 19:01:22 2016
@@ -0,0 +1,5 @@
+// RUN: %clang -c -target i386-apple-darwin10 -isysroot /FOO %s 2>&1 |
FileCheck --check-prefix=CHECK-SHOW-OPTION-NAMES %s
+// CHECK-SHOW-OPTION-NAMES: warning: no such sysroot directory:
'{{([A-Za-z]:.*)?}}/FOO' [-Wmissing-sysroot]
+
+// RUN: %clang -c -target i386-apple-darwin10 -fno-diagnostics-show-option
-isysroot /FOO %s 2>&1 | FileCheck --check-prefix=CHECK-NO-SHOW-OPTION-NAMES %s
+// CHECK-NO-SHOW-OPTION-NAMES: warning: no such sysroot directory:
'{{([A-Za-z]:.*)?}}/FOO'{{$}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24516: [Driver][Diagnostics] Make 'show option names' default for driver warnings
bruno closed this revision.
bruno added a comment.
Committed r283827
Comment at: test/Driver/show-option-names.c:4
+// RUN: %clang -c -target i386-apple-darwin10 -fno-diagnostics-show-option
-isysroot /FOO %s 2>&1 | FileCheck --check-prefix=CHECK-NO-SHOW-OPTION-NAMES %s
+// CHECK-NO-SHOW-OPTION-NAMES-NOT: warning: no such sysroot directory:
'{{([A-Za-z]:.*)?}}/FOO' [-Wmissing-sysroot]
vsk wrote:
> vsk wrote:
> > Can you split this into two checks? One to check that the warning appear,
> > and another to check that the option isn't printed.
> Argh, I suppose there's no "CHECK-SAME-NOT" functionality in FileCheck. If
> you want to shave off a compiler invocation, maybe you could write:
>
> CHECK: ... {{$}}
The "-NOT" functionality is there, but doesn't help in this case. The {{$}}
worked well enough though! Thanks
https://reviews.llvm.org/D24516
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r283827 - [Driver][Diagnostics] Make 'show option names' default for driver warnings
Thanks Renato! On Tue, Oct 11, 2016 at 3:36 AM, Renato Golin wrote: > On 11 October 2016 at 10:14, Renato Golin wrote: >> clang-4.0: warning: no such sysroot directory: '/FOO' [-Wmissing-sysroot] >> error: unable to create target: 'No available targets are compatible >> with this triple.' >> 1 error generated. > > Reverted in r283868, as it was also breaking ARM bots. > > cheers, > --renato -- Bruno Cardoso Lopes http://www.brunocardoso.cc ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24669: {Sema] Gcc compatibility of vector shift.
bruno added inline comments.
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8787
}
+if (!S.LangOpts.OpenCL && !S.LangOpts.ZVector) {
+ const BuiltinType *LHSBT = LHSEleType->getAs();
vbyakovlcl wrote:
> bruno wrote:
> > vbyakovlcl wrote:
> > > bruno wrote:
> > > > Besides `__ext_vector_type__`, would this also trigger for
> > > > `vector_size`? Right now this is an error for `vector_size` primarily
> > > > because the number of elements is different, can you confirm this won't
> > > > change?
> > > I compare vector element sizes, so there must not be any differencies
> > > caused by different triggers. I added additional type definitions to the
> > > tests. All compiles fain.
> >
> > I don't think this is right. When I try to compile similar code for
> > `vector_size` without your patch, I get the errors:
> >
> > /tmp/x.c:80:15: error: vector operands do not have the same number of
> > elements ('vector_int8n' (vector of 2 'int' values) and 'vector_uchar8n'
> > (vector of 8 'unsigned char' values))
> > vi8n = vi8n << vuc8n; // expected-warning {{vector operands do not have
> > the same elements sizes}}
> > ^ ~
> > /tmp/x.c:81:17: error: vector operands do not have the same number of
> > elements ('vector_uchar8n' (vector of 8 'unsigned char' values) and
> > 'vector_int8n' (vector of 2 'int' values))
> > vuc8n = vuc8n << vi8n; // expected-warning {{vector operands do not have
> > the same elements sizes}}
> > ~ ^
> > /tmp/x.c:82:17: error: vector operands do not have the same number of
> > elements ('vector_ushort8n' (vector of 4 'unsigned short' values) and
> > 'vector_uint8n' (vector of 2 'unsigned int' values))
> > vus8n = vus8n << vui8n; // expected-warning {{vector operands do not have
> > the same elements sizes}}
> > ~ ^ ~
> > /tmp/x.c:83:17: error: vector operands do not have the same number of
> > elements ('vector_uint8n' (vector of 2 'unsigned int' values) and
> > 'vector_short8n' (vector of 4 'short' values))
> > vui8n = vui8n << vs8n; // expected-warning {{vector operands do not have
> > the same elements sizes}}
> > ~ ^
> >
> > Given your test changes, it seems that now, instead of "vector operands do
> > not have the same number of elements" we would get "vector operands do not
> > have the same elements sizes". I rather we stay with the first.
> > Additionally, even if we had "vector operands do not have the same elements
> > sizes" for `vector_size`, this should never be demoted to a warning.
> Argument of a GNU vector size attribute specifies vector size measured in
> bytes(see https://gcc.gnu.org/onlinedocs/gcc/Vector-Extensions.html). So you
> got right diagnostics. Both compilers with and without my changes print the
> same diagnostics for yours case. Here is a small testcase used both GNU and
> clang extensions
>
> $ cat bruno.c
>
>
> typedef __attribute__((__ext_vector_type__(8))) int vector_int8;
> typedef __attribute__((__ext_vector_type__(8))) unsigned char vector_uchar8;
>
> typedef __attribute__((vector_size(8))) int vector_int8n;
> typedef __attribute__((vector_size(8))) unsigned char vector_uchar8n;
>
> vector_int8 vi8;
> vector_uchar8 vuc8;
> vector_int8n vi8n;
> vector_uchar8n vuc8n;
>
> int foo() {
> vi8 = vi8 << vuc8;
> vi8n = vi8n << vuc8n;
>
> $ clang -c bruno.c -Wno-error-vec-elem-size
>
>
> bruno.c:13:13: warning: vector operands do not have the same elements sizes
> ('vector_int8' (vector of 8 'int' values) and 'vector_uchar8' (vector of 8
> 'unsigned char' values)) [-Wvec-elem-size]
> vi8 = vi8 << vuc8;
> ~~~ ^
> bruno.c:14:15: error: vector operands do not have the same number of elements
> ('vector_int8n' (vector of 2 'int' values) and 'vector_uchar8n' (vector of 8
> 'unsigned char' values))
> vi8n = vi8n << vuc8n;
>
> The compiler without the changes prints the second error only (bruno.c:14:15).
What actually concerns me here is the following: if you invoke `clang -c
bruno.c -Wvec-elem-size`, will that override the `error: vector operands do not
have the same number of elements ` message for `vector_size` typed vectors? If
so, I don't think this is right.
https://reviews.llvm.org/D24669
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r283827 - [Driver][Diagnostics] Make 'show option names' default for driver warnings
On Tue, Oct 11, 2016 at 10:09 AM, Renato Golin wrote: > On 11 October 2016 at 16:34, Bruno Cardoso Lopes > wrote: >> Thanks Renato! > > So, Daniel Jasper did a trick on r283853 (clang || true) to make it > not fail when it returns on error. However, I wasn't able to make it > return anything but 0, so I'm at odds why this fails on the bot. > > I was expecting it to actually return non-zero, since it emits an > error, which is even weirder. So, using "not clang" doesn't work in > this case. Do you have any ideas why this behaviour is like that? I have no idea. I'm gonna try to re-introduce the commit without a target specifc triple, that hopefully should be enough to get it working (there was no specific reason this test should be using a specific target anyway) Thanks again Renato -- Bruno Cardoso Lopes http://www.brunocardoso.cc ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r283913 - Reapply [Driver][Diagnostics] Make 'show option names' default for driver warnings
Author: bruno
Date: Tue Oct 11 13:21:26 2016
New Revision: 283913
URL: http://llvm.org/viewvc/llvm-project?rev=283913&view=rev
Log:
Reapply [Driver][Diagnostics] Make 'show option names' default for driver
warnings
Reapply r283827 by fixing the tests to not be target specific
Currently, driver level warnings do not show option names (e.g. warning:
complain about foo [-Woption-name]) in a diagnostic unless
-fdiagnostics-show-option is explictly specified. OTOH, the driver by
default turn this option on for CC1. Change the logic to show option
names by default in the driver as well.
Differential Revision: https://reviews.llvm.org/D24516
rdar://problem/27300909
Added:
cfe/trunk/test/Driver/show-option-names.c
Modified:
cfe/trunk/include/clang/Frontend/CompilerInvocation.h
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
Modified: cfe/trunk/include/clang/Frontend/CompilerInvocation.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CompilerInvocation.h?rev=283913&r1=283912&r2=283913&view=diff
==
--- cfe/trunk/include/clang/Frontend/CompilerInvocation.h (original)
+++ cfe/trunk/include/clang/Frontend/CompilerInvocation.h Tue Oct 11 13:21:26
2016
@@ -48,7 +48,8 @@ class DiagnosticsEngine;
/// report the error(s).
bool ParseDiagnosticArgs(DiagnosticOptions &Opts, llvm::opt::ArgList &Args,
DiagnosticsEngine *Diags = nullptr,
- bool DefaultDiagColor = true);
+ bool DefaultDiagColor = true,
+ bool DefaultShowOpt = true);
class CompilerInvocationBase : public RefCountedBase {
void operator=(const CompilerInvocationBase &) = delete;
Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=283913&r1=283912&r2=283913&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Tue Oct 11 13:21:26 2016
@@ -957,7 +957,7 @@ static bool parseShowColorsArgs(const Ar
bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
DiagnosticsEngine *Diags,
-bool DefaultDiagColor) {
+bool DefaultDiagColor, bool DefaultShowOpt) {
using namespace options;
bool Success = true;
@@ -977,7 +977,9 @@ bool clang::ParseDiagnosticArgs(Diagnost
Opts.ShowFixits = !Args.hasArg(OPT_fno_diagnostics_fixit_info);
Opts.ShowLocation = !Args.hasArg(OPT_fno_show_source_location);
Opts.AbsolutePath = Args.hasArg(OPT_fdiagnostics_absolute_paths);
- Opts.ShowOptionNames = Args.hasArg(OPT_fdiagnostics_show_option);
+ Opts.ShowOptionNames =
+ Args.hasFlag(OPT_fdiagnostics_show_option,
+ OPT_fno_diagnostics_show_option, DefaultShowOpt);
llvm::sys::Process::UseANSIEscapeCodes(Args.hasArg(OPT_fansi_escape_codes));
@@ -2404,8 +2406,9 @@ bool CompilerInvocation::CreateFromArgs(
Success &= ParseAnalyzerArgs(*Res.getAnalyzerOpts(), Args, Diags);
Success &= ParseMigratorArgs(Res.getMigratorOpts(), Args);
ParseDependencyOutputArgs(Res.getDependencyOutputOpts(), Args);
- Success &= ParseDiagnosticArgs(Res.getDiagnosticOpts(), Args, &Diags,
- false /*DefaultDiagColor*/);
+ Success &=
+ ParseDiagnosticArgs(Res.getDiagnosticOpts(), Args, &Diags,
+ false /*DefaultDiagColor*/, false
/*DefaultShowOpt*/);
ParseCommentArgs(LangOpts.CommentOpts, Args);
ParseFileSystemArgs(Res.getFileSystemOpts(), Args);
// FIXME: We shouldn't have to pass the DashX option around here
Added: cfe/trunk/test/Driver/show-option-names.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/show-option-names.c?rev=283913&view=auto
==
--- cfe/trunk/test/Driver/show-option-names.c (added)
+++ cfe/trunk/test/Driver/show-option-names.c Tue Oct 11 13:21:26 2016
@@ -0,0 +1,5 @@
+// RUN: %clang -c -isysroot /FOO %s 2>&1 | FileCheck
--check-prefix=CHECK-SHOW-OPTION-NAMES %s
+// CHECK-SHOW-OPTION-NAMES: warning: no such sysroot directory:
'{{([A-Za-z]:.*)?}}/FOO' [-Wmissing-sysroot]
+
+// RUN: %clang -c -fno-diagnostics-show-option -isysroot /FOO %s 2>&1 |
FileCheck --check-prefix=CHECK-NO-SHOW-OPTION-NAMES %s
+// CHECK-NO-SHOW-OPTION-NAMES: warning: no such sysroot directory:
'{{([A-Za-z]:.*)?}}/FOO'{{$}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r283827 - [Driver][Diagnostics] Make 'show option names' default for driver warnings
Let's see how it goes: Committed r283913 On Tue, Oct 11, 2016 at 11:13 AM, Bruno Cardoso Lopes wrote: > On Tue, Oct 11, 2016 at 10:09 AM, Renato Golin > wrote: >> On 11 October 2016 at 16:34, Bruno Cardoso Lopes >> wrote: >>> Thanks Renato! >> >> So, Daniel Jasper did a trick on r283853 (clang || true) to make it >> not fail when it returns on error. However, I wasn't able to make it >> return anything but 0, so I'm at odds why this fails on the bot. >> >> I was expecting it to actually return non-zero, since it emits an >> error, which is even weirder. So, using "not clang" doesn't work in >> this case. Do you have any ideas why this behaviour is like that? > > I have no idea. I'm gonna try to re-introduce the commit without a > target specifc triple, that hopefully should be enough to get it > working (there was no specific reason this test should be using a > specific target anyway) > > Thanks again Renato > > > > -- > Bruno Cardoso Lopes > http://www.brunocardoso.cc -- Bruno Cardoso Lopes http://www.brunocardoso.cc ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r283915 - [Driver] Fix test from r283913 to unbreak bots
Author: bruno
Date: Tue Oct 11 13:31:45 2016
New Revision: 283915
URL: http://llvm.org/viewvc/llvm-project?rev=283915&view=rev
Log:
[Driver] Fix test from r283913 to unbreak bots
Followup from r283913 & r283827
http://bb.pgr.jp/builders/cmake-clang-x86_64-linux/builds/55135
Modified:
cfe/trunk/test/Driver/show-option-names.c
Modified: cfe/trunk/test/Driver/show-option-names.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/show-option-names.c?rev=283915&r1=283914&r2=283915&view=diff
==
--- cfe/trunk/test/Driver/show-option-names.c (original)
+++ cfe/trunk/test/Driver/show-option-names.c Tue Oct 11 13:31:45 2016
@@ -1,5 +1,7 @@
-// RUN: %clang -c -isysroot /FOO %s 2>&1 | FileCheck
--check-prefix=CHECK-SHOW-OPTION-NAMES %s
+// REQUIRES: x86-registered-target
+
+// RUN: %clang -target x86_64-apple-darwin -c -isysroot /FOO %s 2>&1 |
FileCheck --check-prefix=CHECK-SHOW-OPTION-NAMES %s
// CHECK-SHOW-OPTION-NAMES: warning: no such sysroot directory:
'{{([A-Za-z]:.*)?}}/FOO' [-Wmissing-sysroot]
-// RUN: %clang -c -fno-diagnostics-show-option -isysroot /FOO %s 2>&1 |
FileCheck --check-prefix=CHECK-NO-SHOW-OPTION-NAMES %s
+// RUN: %clang -target x86_64-apple-darwin -c -fno-diagnostics-show-option
-isysroot /FOO %s 2>&1 | FileCheck --check-prefix=CHECK-NO-SHOW-OPTION-NAMES %s
// CHECK-NO-SHOW-OPTION-NAMES: warning: no such sysroot directory:
'{{([A-Za-z]:.*)?}}/FOO'{{$}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r283917 - [Driver] Use -fsyntax-only in test/Driver/show-option-names.c
Author: bruno
Date: Tue Oct 11 13:38:33 2016
New Revision: 283917
URL: http://llvm.org/viewvc/llvm-project?rev=283917&view=rev
Log:
[Driver] Use -fsyntax-only in test/Driver/show-option-names.c
Make the test less expensive, follow up from r283915.
Modified:
cfe/trunk/test/Driver/show-option-names.c
Modified: cfe/trunk/test/Driver/show-option-names.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/show-option-names.c?rev=283917&r1=283916&r2=283917&view=diff
==
--- cfe/trunk/test/Driver/show-option-names.c (original)
+++ cfe/trunk/test/Driver/show-option-names.c Tue Oct 11 13:38:33 2016
@@ -1,7 +1,7 @@
// REQUIRES: x86-registered-target
-// RUN: %clang -target x86_64-apple-darwin -c -isysroot /FOO %s 2>&1 |
FileCheck --check-prefix=CHECK-SHOW-OPTION-NAMES %s
+// RUN: %clang -target x86_64-apple-darwin -fsyntax-only -isysroot /FOO %s
2>&1 | FileCheck --check-prefix=CHECK-SHOW-OPTION-NAMES %s
// CHECK-SHOW-OPTION-NAMES: warning: no such sysroot directory:
'{{([A-Za-z]:.*)?}}/FOO' [-Wmissing-sysroot]
-// RUN: %clang -target x86_64-apple-darwin -c -fno-diagnostics-show-option
-isysroot /FOO %s 2>&1 | FileCheck --check-prefix=CHECK-NO-SHOW-OPTION-NAMES %s
+// RUN: %clang -target x86_64-apple-darwin -fsyntax-only
-fno-diagnostics-show-option -isysroot /FOO %s 2>&1 | FileCheck
--check-prefix=CHECK-NO-SHOW-OPTION-NAMES %s
// CHECK-NO-SHOW-OPTION-NAMES: warning: no such sysroot directory:
'{{([A-Za-z]:.*)?}}/FOO'{{$}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [libcxx] r249738 - Split out of .
Hi Richard, I have a patch on top of your suggested patch from a year ago, that break the cyclic dependency we're seeing, with this (and a few changes to the SDK) we can bootstrap clang with submodule local visibility on darwin. I've attached the patch with a reduced, standalone testcase that doesn't depend on the SDK. The issues that are not covered by your patch, that I cover in mine, are related to built-in and textual headers: they can be found in paths where they don't map to any modules, triggering other cycles. I fix that by looking further to find a matching module for the header in question, instead the first found header in header search. Can you take a look? Thanks, On Thu, Jul 28, 2016 at 3:55 PM, Adrian Prantl via cfe-commits wrote: > +Bruno > > On Jul 27, 2016, at 11:58 PM, Nico Weber wrote: > > I played with modules a bit today, and as far as I can tell this is still > broken. If this proves difficult to fix, should this change be reverted for > now? It breaks using modules on Darwin. > > On Sun, Mar 13, 2016 at 12:53 AM, Adrian Prantl via cfe-commits > wrote: >> >> > On Mar 11, 2016, at 4:11 PM, Duncan P. N. Exon Smith >> > wrote: >> > >> > Did anyone file a PR for this? >> > >> >> I filed PR 26928 - Prune the include path for modules >> https://llvm.org/bugs/show_bug.cgi?id=26928 >> as a starting point. >> >> -- adrian >> >> ___ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > > > > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > -- Bruno Cardoso Lopes http://www.brunocardoso.cc 0001-Add-NoUndeclaredIncludes-attribute-and-logic.patch Description: Binary data 0002-Changes-on-top-of-Richard-s-patch.patch Description: Binary data ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25597: Try to fix buildbot failure in VirtualFileSystem caused by r284129.
bruno added a comment. One possible reason: remove_dots is called upon a path with a leading "..", which then gets appended in front of another path to form the absolute path. I'm taking a look right now to try to figure out if there's any code path that might lead to this. https://reviews.llvm.org/D25597 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25597: Try to fix buildbot failure in VirtualFileSystem caused by r284129.
bruno added a comment. Krzysztof, do you have a backtrace that you can paste here or point me to the buidbot stderr log? There's no point in looking for relative paths inside the VFS, it would be nice if we fix the root cause here. https://reviews.llvm.org/D25597 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] cffb0dd - [SemaTemplate] Stop passing insertion position around during VarTemplate instantiation
Author: Bruno Cardoso Lopes
Date: 2020-10-12T16:48:50-07:00
New Revision: cffb0dd54d41d8e249d2009467c4beb5b681ba26
URL:
https://github.com/llvm/llvm-project/commit/cffb0dd54d41d8e249d2009467c4beb5b681ba26
DIFF:
https://github.com/llvm/llvm-project/commit/cffb0dd54d41d8e249d2009467c4beb5b681ba26.diff
LOG: [SemaTemplate] Stop passing insertion position around during VarTemplate
instantiation
They can get stale at use time because of updates from other recursive
specializations. Instead, rely on the existence of previous declarations to add
the specialization.
Differential Revision: https://reviews.llvm.org/D87853
Added:
Modified:
clang/include/clang/Sema/Sema.h
clang/include/clang/Sema/Template.h
clang/lib/Sema/SemaTemplate.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/test/SemaTemplate/instantiate-var-template.cpp
Removed:
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index ca1f87cfdb2b..b5f0c08300bf 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -9171,7 +9171,7 @@ class Sema final {
const TemplateArgumentList &TemplateArgList,
const TemplateArgumentListInfo &TemplateArgsInfo,
SmallVectorImpl &Converted,
- SourceLocation PointOfInstantiation, void *InsertPos,
+ SourceLocation PointOfInstantiation,
LateInstantiatedAttrVec *LateAttrs = nullptr,
LocalInstantiationScope *StartingScope = nullptr);
VarTemplateSpecializationDecl *CompleteVarTemplateSpecializationDecl(
diff --git a/clang/include/clang/Sema/Template.h
b/clang/include/clang/Sema/Template.h
index 91d175fdd050..0dcaf565591b 100644
--- a/clang/include/clang/Sema/Template.h
+++ b/clang/include/clang/Sema/Template.h
@@ -600,7 +600,7 @@ enum class TemplateSubstitutionKind : char {
TagDecl *NewDecl);
Decl *VisitVarTemplateSpecializationDecl(
-VarTemplateDecl *VarTemplate, VarDecl *FromVar, void *InsertPos,
+VarTemplateDecl *VarTemplate, VarDecl *FromVar,
const TemplateArgumentListInfo &TemplateArgsInfo,
ArrayRef Converted,
VarTemplateSpecializationDecl *PrevDecl = nullptr);
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 8baf5b96fbf8..4ecae8faad66 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -4584,7 +4584,7 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template,
SourceLocation TemplateLoc,
// FIXME: LateAttrs et al.?
VarTemplateSpecializationDecl *Decl = BuildVarTemplateInstantiation(
Template, InstantiationPattern, *InstantiationArgs, TemplateArgs,
- Converted, TemplateNameLoc, InsertPos /*, LateAttrs, StartingScope*/);
+ Converted, TemplateNameLoc /*, LateAttrs, StartingScope*/);
if (!Decl)
return true;
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 9420bd04b7a9..7200dc72825d 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3628,11 +3628,11 @@ Decl
*TemplateDeclInstantiator::VisitVarTemplateSpecializationDecl(
return nullptr;
return VisitVarTemplateSpecializationDecl(
- InstVarTemplate, D, InsertPos, VarTemplateArgsInfo, Converted, PrevDecl);
+ InstVarTemplate, D, VarTemplateArgsInfo, Converted, PrevDecl);
}
Decl *TemplateDeclInstantiator::VisitVarTemplateSpecializationDecl(
-VarTemplateDecl *VarTemplate, VarDecl *D, void *InsertPos,
+VarTemplateDecl *VarTemplate, VarDecl *D,
const TemplateArgumentListInfo &TemplateArgsInfo,
ArrayRef Converted,
VarTemplateSpecializationDecl *PrevDecl) {
@@ -3655,8 +3655,11 @@ Decl
*TemplateDeclInstantiator::VisitVarTemplateSpecializationDecl(
SemaRef.Context, Owner, D->getInnerLocStart(), D->getLocation(),
VarTemplate, DI->getType(), DI, D->getStorageClass(), Converted);
Var->setTemplateArgsInfo(TemplateArgsInfo);
- if (InsertPos)
+ if (!PrevDecl) {
+void *InsertPos = nullptr;
+VarTemplate->findSpecialization(Converted, InsertPos);
VarTemplate->AddSpecialization(Var, InsertPos);
+ }
if (SemaRef.getLangOpts().OpenCL)
SemaRef.deduceOpenCLAddressSpace(Var);
@@ -4865,7 +4868,7 @@ VarTemplateSpecializationDecl
*Sema::BuildVarTemplateInstantiation(
const TemplateArgumentList &TemplateArgList,
const TemplateArgumentListInfo &TemplateArgsInfo,
SmallVectorImpl &Converted,
-SourceLocation PointOfInstantiation, void *InsertPos,
+SourceLocation PointOfInstantiation,
LateInstantiatedAttrVec *LateAttrs,
LocalInstantiationScope *StartingScope) {
if (FromVar->isInvalidDecl())
@@ -4904,7 +4907,7 @@ VarTemplateSpecializationDecl
*Sema::BuildVarTemplateInstantiation(
return cast_or_null(
Instantiator.VisitVarTemplateSpecial
[clang] c9aaf34 - [SemaCXX] Handle lack of TypeSourceInfo on special member functions in templated lambdas
Author: Bruno Cardoso Lopes
Date: 2021-06-22T17:26:05-07:00
New Revision: c9aaf34b8db884faa3d3ced4d2fb88fd45697408
URL:
https://github.com/llvm/llvm-project/commit/c9aaf34b8db884faa3d3ced4d2fb88fd45697408
DIFF:
https://github.com/llvm/llvm-project/commit/c9aaf34b8db884faa3d3ced4d2fb88fd45697408.diff
LOG: [SemaCXX] Handle lack of TypeSourceInfo on special member functions in
templated lambdas
During template instantiation involving templated lambdas, clang
could hit an assertion in `TemplateDeclInstantiator::SubstFunctionType`
since the functions are not associated with any `TypeSourceInfo`:
`assert(OldTInfo && "substituting function without type source info");`
This path is triggered when using templated lambdas like the one added as
a test to this patch. To fix this:
- Create `TypeSourceInfo`s for special members and make sure the template
instantiator can get through all patterns.
- Introduce a `SpecialMemberTypeInfoRebuilder` tree transform to rewrite
such member function arguments. Without this, we get errors like:
`error: only special member functions and comparison operators may be defaulted`
since `getDefaultedFunctionKind` can't properly recognize these functions
as special members as part of `SetDeclDefaulted`.
Fixes PR45828 and PR44848
Differential Revision: https://reviews.llvm.org/D88327
Added:
clang/test/SemaCXX/lambdas-implicit-explicit-template.cpp
Modified:
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/lib/Sema/TreeTransform.h
Removed:
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 6d2710f1774ce..a68a06eb4d270 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -13202,6 +13202,16 @@ void
Sema::setupImplicitSpecialMemberType(CXXMethodDecl *SpecialMem,
auto QT = Context.getFunctionType(ResultTy, Args, EPI);
SpecialMem->setType(QT);
+
+ // During template instantiation of implicit special member functions we need
+ // a reliable TypeSourceInfo for the function prototype in order to allow
+ // functions to be substituted.
+ if (inTemplateInstantiation() &&
+ cast(SpecialMem->getParent())->isLambda()) {
+TypeSourceInfo *TSI =
+Context.getTrivialTypeSourceInfo(SpecialMem->getType());
+SpecialMem->setTypeSourceInfo(TSI);
+ }
}
CXXConstructorDecl *Sema::DeclareImplicitDefaultConstructor(
@@ -14880,12 +14890,18 @@ CXXConstructorDecl
*Sema::DeclareImplicitCopyConstructor(
setupImplicitSpecialMemberType(CopyConstructor, Context.VoidTy, ArgType);
+ // During template instantiation of special member functions we need a
+ // reliable TypeSourceInfo for the parameter types in order to allow
functions
+ // to be substituted.
+ TypeSourceInfo *TSI = nullptr;
+ if (inTemplateInstantiation() && ClassDecl->isLambda())
+TSI = Context.getTrivialTypeSourceInfo(ArgType);
+
// Add the parameter to the constructor.
- ParmVarDecl *FromParam = ParmVarDecl::Create(Context, CopyConstructor,
- ClassLoc, ClassLoc,
- /*IdentifierInfo=*/nullptr,
- ArgType, /*TInfo=*/nullptr,
- SC_None, nullptr);
+ ParmVarDecl *FromParam =
+ ParmVarDecl::Create(Context, CopyConstructor, ClassLoc, ClassLoc,
+ /*IdentifierInfo=*/nullptr, ArgType,
+ /*TInfo=*/TSI, SC_None, nullptr);
CopyConstructor->setParams(FromParam);
CopyConstructor->setTrivial(
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp
b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index f9d40a2ed4b7b..f18f77d3442a1 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2817,7 +2817,8 @@ Sema::InstantiateClass(SourceLocation
PointOfInstantiation,
if (!Instantiation->isInvalidDecl()) {
// Perform any dependent diagnostics from the pattern.
-PerformDependentDiagnostics(Pattern, TemplateArgs);
+if (Pattern->isDependentContext())
+ PerformDependentDiagnostics(Pattern, TemplateArgs);
// Instantiate any out-of-line class template partial
// specializations now.
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 0fa42c5494213..be4c519307898 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -9,6 +9,7 @@
//
//===--===/
+#include "TreeTransform.h"
#include "clang/AST/ASTConsumer.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTMutationListener.h"
@@ -1825,9 +1826,16 @@ Decl
*TemplateDeclInstantiator::Visi
[clang] 819e0d1 - [CGAtomic] Lift strong requirement for remaining compare_exchange combinations
Author: Bruno Cardoso Lopes
Date: 2021-05-06T21:05:20-07:00
New Revision: 819e0d105e84c6081cfcfa0e38fd257b6124553a
URL:
https://github.com/llvm/llvm-project/commit/819e0d105e84c6081cfcfa0e38fd257b6124553a
DIFF:
https://github.com/llvm/llvm-project/commit/819e0d105e84c6081cfcfa0e38fd257b6124553a.diff
LOG: [CGAtomic] Lift strong requirement for remaining compare_exchange
combinations
Follow up on 431e3138a and complete the other possible combinations.
Besides enforcing the new behavior, it also mitigates TSAN false positives when
combining orders that used to be stronger.
Added:
Modified:
clang/lib/CodeGen/CGAtomic.cpp
clang/test/CodeGen/atomic-ops.c
clang/test/CodeGenOpenCL/atomic-ops.cl
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/IR/Instructions.cpp
llvm/lib/IR/Verifier.cpp
Removed:
diff --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp
index ef016cf24f134..cc85375f08ddf 100644
--- a/clang/lib/CodeGen/CGAtomic.cpp
+++ b/clang/lib/CodeGen/CGAtomic.cpp
@@ -451,47 +451,38 @@ static void emitAtomicCmpXchgFailureSet(CodeGenFunction
&CGF, AtomicExpr *E,
}
// Create all the relevant BB's
- llvm::BasicBlock *MonotonicBB = nullptr, *AcquireBB = nullptr,
- *SeqCstBB = nullptr;
- MonotonicBB = CGF.createBasicBlock("monotonic_fail", CGF.CurFn);
- if (SuccessOrder != llvm::AtomicOrdering::Monotonic)
-AcquireBB = CGF.createBasicBlock("acquire_fail", CGF.CurFn);
- if (SuccessOrder == llvm::AtomicOrdering::SequentiallyConsistent)
-SeqCstBB = CGF.createBasicBlock("seqcst_fail", CGF.CurFn);
-
- llvm::BasicBlock *ContBB = CGF.createBasicBlock("atomic.continue",
CGF.CurFn);
-
- llvm::SwitchInst *SI = CGF.Builder.CreateSwitch(FailureOrderVal,
MonotonicBB);
-
- // Emit all the
diff erent atomics
+ auto *MonotonicBB = CGF.createBasicBlock("monotonic_fail", CGF.CurFn);
+ auto *AcquireBB = CGF.createBasicBlock("acquire_fail", CGF.CurFn);
+ auto *SeqCstBB = CGF.createBasicBlock("seqcst_fail", CGF.CurFn);
+ auto *ContBB = CGF.createBasicBlock("atomic.continue", CGF.CurFn);
// MonotonicBB is arbitrarily chosen as the default case; in practice, this
// doesn't matter unless someone is crazy enough to use something that
// doesn't fold to a constant for the ordering.
+ llvm::SwitchInst *SI = CGF.Builder.CreateSwitch(FailureOrderVal,
MonotonicBB);
+ // Implemented as acquire, since it's the closest in LLVM.
+ SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::consume),
+ AcquireBB);
+ SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::acquire),
+ AcquireBB);
+ SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::seq_cst),
+ SeqCstBB);
+
+ // Emit all the
diff erent atomics
CGF.Builder.SetInsertPoint(MonotonicBB);
emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2,
Size, SuccessOrder, llvm::AtomicOrdering::Monotonic,
Scope);
CGF.Builder.CreateBr(ContBB);
- if (AcquireBB) {
-CGF.Builder.SetInsertPoint(AcquireBB);
-emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2,
- Size, SuccessOrder, llvm::AtomicOrdering::Acquire,
Scope);
-CGF.Builder.CreateBr(ContBB);
-if (SuccessOrder != llvm::AtomicOrdering::Release)
- SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::consume),
- AcquireBB);
-SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::acquire),
-AcquireBB);
- }
- if (SeqCstBB) {
-CGF.Builder.SetInsertPoint(SeqCstBB);
-emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2, Size,
SuccessOrder,
- llvm::AtomicOrdering::SequentiallyConsistent, Scope);
-CGF.Builder.CreateBr(ContBB);
-SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::seq_cst),
-SeqCstBB);
- }
+ CGF.Builder.SetInsertPoint(AcquireBB);
+ emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2, Size, SuccessOrder,
+llvm::AtomicOrdering::Acquire, Scope);
+ CGF.Builder.CreateBr(ContBB);
+
+ CGF.Builder.SetInsertPoint(SeqCstBB);
+ emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2, Size, SuccessOrder,
+llvm::AtomicOrdering::SequentiallyConsistent, Scope);
+ CGF.Builder.CreateBr(ContBB);
CGF.Builder.SetInsertPoint(ContBB);
}
diff --git a/clang/test/CodeGen/atomic-ops.c b/clang/test/CodeGen/atomic-ops.c
index 269406fc3c50f..02edec19bca93 100644
--- a/clang/test/CodeGen/atomic-ops.c
+++ b/clang/test/CodeGen/atomic-ops.c
@@ -490,29 +490,36 @@ void generalFailureOrder(_Atomic(int) *ptr, int *ptr2,
int success, int fail) {
// CHECK: [[MONOTONIC]]
// CHECK: switch {{.*}}, label %[[MONOTONIC_MONOTONIC:[0-9a-zA-Z._]+]] [
+ // CHECK-NEXT: i32 1, label %[[MONOTONIC_ACQUIRE:[0-9a-zA-Z._]+]]
+ // CHECK-NEXT: i32 2, label %[
[clang] 431e313 - [CGAtomic] Lift stronger requirements on cmpxch and support acquire failure mode
Author: Bruno Cardoso Lopes
Date: 2021-03-23T16:45:37-07:00
New Revision: 431e3138a1f3fd2bb7b25e1f4766d935058136f8
URL:
https://github.com/llvm/llvm-project/commit/431e3138a1f3fd2bb7b25e1f4766d935058136f8
DIFF:
https://github.com/llvm/llvm-project/commit/431e3138a1f3fd2bb7b25e1f4766d935058136f8.diff
LOG: [CGAtomic] Lift stronger requirements on cmpxch and support acquire
failure mode
- Fix `emitAtomicCmpXchgFailureSet` to support release/acquire (succ/fail)
memory order.
- Remove stronger checks for cmpxch.
Effectively, this addresses http://wg21.link/p0418
Differential Revision: https://reviews.llvm.org/D98995
Added:
Modified:
clang/lib/CodeGen/CGAtomic.cpp
clang/test/CodeGen/atomic-ops.c
clang/test/CodeGenOpenCL/atomic-ops.cl
llvm/docs/LangRef.rst
Removed:
diff --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp
index c7256e240a31..8ac36e4a6b64 100644
--- a/clang/lib/CodeGen/CGAtomic.cpp
+++ b/clang/lib/CodeGen/CGAtomic.cpp
@@ -427,6 +427,8 @@ static void emitAtomicCmpXchgFailureSet(CodeGenFunction
&CGF, AtomicExpr *E,
else
switch ((llvm::AtomicOrderingCABI)FOS) {
case llvm::AtomicOrderingCABI::relaxed:
+ // 31.7.2.18: "The failure argument shall not be memory_order_release
+ // nor memory_order_acq_rel". Fallback to monotonic.
case llvm::AtomicOrderingCABI::release:
case llvm::AtomicOrderingCABI::acq_rel:
FailureOrder = llvm::AtomicOrdering::Monotonic;
@@ -439,12 +441,10 @@ static void emitAtomicCmpXchgFailureSet(CodeGenFunction
&CGF, AtomicExpr *E,
FailureOrder = llvm::AtomicOrdering::SequentiallyConsistent;
break;
}
-if (isStrongerThan(FailureOrder, SuccessOrder)) {
- // Don't assert on undefined behavior "failure argument shall be no
- // stronger than the success argument".
- FailureOrder =
- llvm::AtomicCmpXchgInst::getStrongestFailureOrdering(SuccessOrder);
-}
+// Prior to c++17, "the failure argument shall be no stronger than the
+// success argument". This condition has been lifted and the only
+// precondition is 31.7.2.18. Effectively treat this as a DR and skip
+// language version checks.
emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2, Size,
SuccessOrder,
FailureOrder, Scope);
return;
@@ -454,8 +454,7 @@ static void emitAtomicCmpXchgFailureSet(CodeGenFunction
&CGF, AtomicExpr *E,
llvm::BasicBlock *MonotonicBB = nullptr, *AcquireBB = nullptr,
*SeqCstBB = nullptr;
MonotonicBB = CGF.createBasicBlock("monotonic_fail", CGF.CurFn);
- if (SuccessOrder != llvm::AtomicOrdering::Monotonic &&
- SuccessOrder != llvm::AtomicOrdering::Release)
+ if (SuccessOrder != llvm::AtomicOrdering::Monotonic)
AcquireBB = CGF.createBasicBlock("acquire_fail", CGF.CurFn);
if (SuccessOrder == llvm::AtomicOrdering::SequentiallyConsistent)
SeqCstBB = CGF.createBasicBlock("seqcst_fail", CGF.CurFn);
@@ -479,8 +478,9 @@ static void emitAtomicCmpXchgFailureSet(CodeGenFunction
&CGF, AtomicExpr *E,
emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2,
Size, SuccessOrder, llvm::AtomicOrdering::Acquire,
Scope);
CGF.Builder.CreateBr(ContBB);
-SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::consume),
-AcquireBB);
+if (SuccessOrder != llvm::AtomicOrdering::Release)
+ SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::consume),
+ AcquireBB);
SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::acquire),
AcquireBB);
}
diff --git a/clang/test/CodeGen/atomic-ops.c b/clang/test/CodeGen/atomic-ops.c
index 4deb1322e0ff..269406fc3c50 100644
--- a/clang/test/CodeGen/atomic-ops.c
+++ b/clang/test/CodeGen/atomic-ops.c
@@ -500,6 +500,7 @@ void generalFailureOrder(_Atomic(int) *ptr, int *ptr2, int
success, int fail) {
// CHECK: [[RELEASE]]
// CHECK: switch {{.*}}, label %[[RELEASE_MONOTONIC:[0-9a-zA-Z._]+]] [
+ // CHECK-NEXT: i32 2, label %[[RELEASE_ACQUIRE:[0-9a-zA-Z._]+]]
// CHECK-NEXT: ]
// CHECK: [[ACQREL]]
@@ -527,6 +528,14 @@ void generalFailureOrder(_Atomic(int) *ptr, int *ptr2, int
success, int fail) {
// CHECK: cmpxchg {{.*}} acquire acquire, align
// CHECK: br
+ // CHECK: [[RELEASE_MONOTONIC]]
+ // CHECK: cmpxchg {{.*}} release monotonic, align
+ // CHECK: br
+
+ // CHECK: [[RELEASE_ACQUIRE]]
+ // CHECK: cmpxchg {{.*}} release acquire, align
+ // CHECK: br
+
// CHECK: [[ACQREL_MONOTONIC]]
// CHECK: cmpxchg {{.*}} acq_rel monotonic, align
// CHECK: br
@@ -562,6 +571,20 @@ void generalWeakness(int *ptr, int *ptr2, _Bool weak) {
// CHECK-NOT: br
// CHECK: cmpxchg weak {{.*}} seq_cst seq_cst, align
// CHECK: br
+
+ __atomic_compare_exchange_n(ptr, ptr2, 42, weak, memory_order_release,
[clang] ce54b22 - [Clang][CoverageMapping] Fix switch counter codegen compile time explosion
Author: Bruno Cardoso Lopes
Date: 2022-05-26T11:05:15-07:00
New Revision: ce54b22657f01d1c40de4941ceb6e7119848aecf
URL:
https://github.com/llvm/llvm-project/commit/ce54b22657f01d1c40de4941ceb6e7119848aecf
DIFF:
https://github.com/llvm/llvm-project/commit/ce54b22657f01d1c40de4941ceb6e7119848aecf.diff
LOG: [Clang][CoverageMapping] Fix switch counter codegen compile time explosion
C++ generated code with huge amount of switch cases chokes badly while emitting
coverage mapping, in our specific testcase (~72k cases), it won't stop after
hours.
After this change, the frontend job now finishes in 4.5s and shrinks down
`@__covrec_`
by 288k when compared to disabling simplification altogether.
There's probably no good way to create a testcase for this, but it's easy to
reproduce, just add thousands of cases in the below switch, and build with
`-fprofile-instr-generate -fcoverage-mapping`.
```
enum type : int {
FEATURE_INVALID = 0,
FEATURE_A = 1,
...
};
const char *to_string(type e) {
switch (e) {
case type::FEATURE_INVALID: return "FEATURE_INVALID";
case type::FEATURE_A: return "FEATURE_A";}
...
}
```
Differential Revision: https://reviews.llvm.org/D126345
Added:
Modified:
clang/lib/CodeGen/CoverageMappingGen.cpp
llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
Removed:
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 9b81c8a670f59..8952125eeefcb 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -550,17 +550,18 @@ struct CounterCoverageMappingBuilder
Counter GapRegionCounter;
/// Return a counter for the subtraction of \c RHS from \c LHS
- Counter subtractCounters(Counter LHS, Counter RHS) {
-return Builder.subtract(LHS, RHS);
+ Counter subtractCounters(Counter LHS, Counter RHS, bool Simplify = true) {
+return Builder.subtract(LHS, RHS, Simplify);
}
/// Return a counter for the sum of \c LHS and \c RHS.
- Counter addCounters(Counter LHS, Counter RHS) {
-return Builder.add(LHS, RHS);
+ Counter addCounters(Counter LHS, Counter RHS, bool Simplify = true) {
+return Builder.add(LHS, RHS, Simplify);
}
- Counter addCounters(Counter C1, Counter C2, Counter C3) {
-return addCounters(addCounters(C1, C2), C3);
+ Counter addCounters(Counter C1, Counter C2, Counter C3,
+ bool Simplify = true) {
+return addCounters(addCounters(C1, C2, Simplify), C3, Simplify);
}
/// Return the region counter for the given statement.
@@ -1317,11 +1318,16 @@ struct CounterCoverageMappingBuilder
const SwitchCase *Case = S->getSwitchCaseList();
for (; Case; Case = Case->getNextSwitchCase()) {
HasDefaultCase = HasDefaultCase || isa(Case);
- CaseCountSum = addCounters(CaseCountSum, getRegionCounter(Case));
+ CaseCountSum =
+ addCounters(CaseCountSum, getRegionCounter(Case),
/*Simplify=*/false);
createSwitchCaseRegion(
Case, getRegionCounter(Case),
subtractCounters(ParentCount, getRegionCounter(Case)));
}
+// Simplify is skipped while building the counters above: it can get really
+// slow on top of switches with thousands of cases. Instead, trigger
+// simplification by adding zero to the last counter.
+CaseCountSum = addCounters(CaseCountSum, Counter::getZero());
// If no explicit default case exists, create a branch region to represent
// the hidden branch, which will be added later by the CodeGen. This region
diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
index e1f45019b1a92..e35751512245d 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -195,11 +195,11 @@ class CounterExpressionBuilder {
ArrayRef getExpressions() const { return Expressions; }
/// Return a counter that represents the expression that adds LHS and RHS.
- Counter add(Counter LHS, Counter RHS);
+ Counter add(Counter LHS, Counter RHS, bool Simplify = true);
/// Return a counter that represents the expression that subtracts RHS from
/// LHS.
- Counter subtract(Counter LHS, Counter RHS);
+ Counter subtract(Counter LHS, Counter RHS, bool Simplify = true);
};
using LineColPair = std::pair;
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index 94c2bee3590cb..f9e58fd6afa50 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -123,13 +123,15 @@ Counter CounterExpressionBuilder::simplify(Counter
ExpressionTree) {
return C;
}
-Counter CounterExpressionBuilder::add(Counter LHS, Counter
[clang] e6a76a4 - [Clang][CoverageMapping] Fix compile time explosions by adjusting only appropriated skipped ranges
Author: Bruno Cardoso Lopes
Date: 2022-06-08T23:13:39-07:00
New Revision: e6a76a49356efd11f5f36690181f0f60cecb2e01
URL:
https://github.com/llvm/llvm-project/commit/e6a76a49356efd11f5f36690181f0f60cecb2e01
DIFF:
https://github.com/llvm/llvm-project/commit/e6a76a49356efd11f5f36690181f0f60cecb2e01.diff
LOG: [Clang][CoverageMapping] Fix compile time explosions by adjusting only
appropriated skipped ranges
D83592 added comments to be part of skipped regions, and as part of that, it
also shrinks a skipped range if it spans a line that contains a non-comment
token. This is done by `adjustSkippedRange`.
The `adjustSkippedRange` currently runs on skipped regions that are not
comments, causing a 5min regression while building a big C++ files without any
comments.
Fix the compile time introduced in D83592 by tagging SkippedRange with kind
information and use that to decide what needs additional processing.
Differential Revision: https://reviews.llvm.org/D127338
Added:
Modified:
clang/lib/CodeGen/CoverageMappingGen.cpp
clang/lib/CodeGen/CoverageMappingGen.h
Removed:
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 8952125eeefcb..d1cbe109a6cf8 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -60,26 +60,27 @@
CoverageMappingModuleGen::setUpCoverageCallbacks(Preprocessor &PP) {
return CoverageInfo;
}
-void CoverageSourceInfo::AddSkippedRange(SourceRange Range) {
+void CoverageSourceInfo::AddSkippedRange(SourceRange Range,
+ SkippedRange::Kind RangeKind) {
if (EmptyLineCommentCoverage && !SkippedRanges.empty() &&
PrevTokLoc == SkippedRanges.back().PrevTokLoc &&
SourceMgr.isWrittenInSameFile(SkippedRanges.back().Range.getEnd(),
Range.getBegin()))
SkippedRanges.back().Range.setEnd(Range.getEnd());
else
-SkippedRanges.push_back({Range, PrevTokLoc});
+SkippedRanges.push_back({Range, RangeKind, PrevTokLoc});
}
void CoverageSourceInfo::SourceRangeSkipped(SourceRange Range, SourceLocation)
{
- AddSkippedRange(Range);
+ AddSkippedRange(Range, SkippedRange::PPIfElse);
}
void CoverageSourceInfo::HandleEmptyline(SourceRange Range) {
- AddSkippedRange(Range);
+ AddSkippedRange(Range, SkippedRange::EmptyLine);
}
bool CoverageSourceInfo::HandleComment(Preprocessor &PP, SourceRange Range) {
- AddSkippedRange(Range);
+ AddSkippedRange(Range, SkippedRange::Comment);
return false;
}
@@ -335,6 +336,8 @@ class CoverageMappingBuilder {
/// This shrinks the skipped range if it spans a line that contains a
/// non-comment token. If shrinking the skipped range would make it empty,
/// this returns None.
+ /// Note this function can potentially be expensive because
+ /// getSpellingLineNumber uses getLineNumber, which is expensive.
Optional adjustSkippedRange(SourceManager &SM,
SourceLocation LocStart,
SourceLocation LocEnd,
@@ -382,8 +385,13 @@ class CoverageMappingBuilder {
auto CovFileID = getCoverageFileID(LocStart);
if (!CovFileID)
continue;
- Optional SR =
- adjustSkippedRange(SM, LocStart, LocEnd, I.PrevTokLoc, I.NextTokLoc);
+ Optional SR;
+ if (I.isComment())
+SR = adjustSkippedRange(SM, LocStart, LocEnd, I.PrevTokLoc,
+I.NextTokLoc);
+ else if (I.isPPIfElse() || I.isEmptyLine())
+SR = {SM, LocStart, LocEnd};
+
if (!SR.hasValue())
continue;
auto Region = CounterMappingRegion::makeSkipped(
diff --git a/clang/lib/CodeGen/CoverageMappingGen.h
b/clang/lib/CodeGen/CoverageMappingGen.h
index ae4f435d4ff34..f5282601b6406 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.h
+++ b/clang/lib/CodeGen/CoverageMappingGen.h
@@ -31,15 +31,29 @@ class Decl;
class Stmt;
struct SkippedRange {
+ enum Kind {
+PPIfElse, // Preprocessor #if/#else ...
+EmptyLine,
+Comment,
+ };
+
SourceRange Range;
// The location of token before the skipped source range.
SourceLocation PrevTokLoc;
// The location of token after the skipped source range.
SourceLocation NextTokLoc;
+ // The nature of this skipped range
+ Kind RangeKind;
+
+ bool isComment() { return RangeKind == Comment; }
+ bool isEmptyLine() { return RangeKind == EmptyLine; }
+ bool isPPIfElse() { return RangeKind == PPIfElse; }
- SkippedRange(SourceRange Range, SourceLocation PrevTokLoc = SourceLocation(),
+ SkippedRange(SourceRange Range, Kind K,
+ SourceLocation PrevTokLoc = SourceLocation(),
SourceLocation NextTokLoc = SourceLocation())
- : Range(Range), PrevTokLoc(PrevTokLoc), NextTokLoc(NextTokLoc) {}
+ : Range(Range), PrevTokLoc(P
[clang] [Clang] Fix finding instantiated decls for class template specializations during instantiation (PR #72346)
bcardosolopes wrote: I'm not 100% confident here but the fix makes sense and seems good (nice testcase!). https://github.com/llvm/llvm-project/pull/72346 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [libc] [clang-tools-extra] [lld] [libcxx] [clang] [flang] [lldb] [OpenACC] Initial commits to support OpenACC (PR #70234)
@@ -0,0 +1,14 @@
+// RUN: %clang -S -### -fopenacc %s 2>&1 | FileCheck %s
--check-prefix=CHECK-DRIVER
+// CHECK-DRIVER: "-cc1" {{.*}} "-fopenacc"
bcardosolopes wrote:
Thanks for the explanation!
https://github.com/llvm/llvm-project/pull/70234
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-apply-replacements] Apply format only if --format is specified (PR #70801)
https://github.com/bcardosolopes commented: Can you please add a testcase? https://github.com/llvm/llvm-project/pull/70801 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Preserve coroutine parameter referenced state (PR #70973)
https://github.com/bcardosolopes commented: Thanks for improving this! I haven't looked at your previous review of this PR, but I like the simplicity of this one. https://github.com/llvm/llvm-project/pull/70973 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Preserve coroutine parameter referenced state (PR #70973)
@@ -1965,9 +1965,15 @@ bool Sema::buildCoroutineParameterMoves(SourceLocation
Loc) {
if (PD->getType()->isDependentType())
continue;
+// Preserve the referenced state for unused parameter diagnostics.
+bool DeclReferenced = PD->isReferenced();
+
ExprResult PDRefExpr =
BuildDeclRefExpr(PD, PD->getType().getNonReferenceType(),
ExprValueKind::VK_LValue, Loc); // FIXME: scope?
+
+PD->setReferenced(DeclReferenced);
bcardosolopes wrote:
What happens in `BuildDeclRefExpr` such that `PD` loses track of whether it is
referenced? If something is changing `PD` in a bad way, perhaps at that point
might be better to set the appropriated method instead.
https://github.com/llvm/llvm-project/pull/70973
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Preserve coroutine parameter referenced state (PR #70973)
https://github.com/bcardosolopes edited https://github.com/llvm/llvm-project/pull/70973 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Preserve coroutine parameter referenced state (PR #70973)
https://github.com/bcardosolopes edited https://github.com/llvm/llvm-project/pull/70973 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Preserve coroutine parameter referenced state (PR #70973)
@@ -1965,9 +1965,15 @@ bool Sema::buildCoroutineParameterMoves(SourceLocation
Loc) {
if (PD->getType()->isDependentType())
continue;
+// Preserve the referenced state for unused parameter diagnostics.
+bool DeclReferenced = PD->isReferenced();
+
ExprResult PDRefExpr =
BuildDeclRefExpr(PD, PD->getType().getNonReferenceType(),
ExprValueKind::VK_LValue, Loc); // FIXME: scope?
+
+PD->setReferenced(DeclReferenced);
bcardosolopes wrote:
As you mentioned offline, it's because creating the expression will mark it
referenced, gotcha.
https://github.com/llvm/llvm-project/pull/70973
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Preserve coroutine parameter referenced state (PR #70973)
https://github.com/bcardosolopes approved this pull request. LGTM. @ChuanqiXu9 should give the final blessing though. https://github.com/llvm/llvm-project/pull/70973 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libc] [clang] [lld] [llvm] [lldb] [libcxx] [clang-tools-extra] [flang] [OpenACC] Initial commits to support OpenACC (PR #70234)
@@ -1342,6 +1342,15 @@ def err_opencl_logical_exclusive_or : Error<
def err_openclcxx_virtual_function : Error<
"virtual functions are not supported in C++ for OpenCL">;
+// OpenACC Support.
+def warn_pragma_acc_ignored : Warning<
+ "unexpected '#pragma acc ...' in program">, InGroup,
DefaultIgnore;
+def err_acc_unexpected_directive : Error<
+ "unexpected OpenACC directive %select{|'#pragma acc %1'}0">;
+def warn_pragma_acc_unimplemented
+: Warning<"OpenACC Directives not yet implemented, pragma ignored">,
bcardosolopes wrote:
Does `Directives` need to be capitalized?
https://github.com/llvm/llvm-project/pull/70234
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] [llvm] [lld] [clang] [libc] [flang] [lldb] [clang-tools-extra] [OpenACC] Initial commits to support OpenACC (PR #70234)
https://github.com/bcardosolopes edited https://github.com/llvm/llvm-project/pull/70234 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] [lld] [flang] [clang] [llvm] [lldb] [clang-tools-extra] [libc] [OpenACC] Initial commits to support OpenACC (PR #70234)
https://github.com/bcardosolopes commented: The changes in this patch looks pretty straightforward! Left some inline comments. https://github.com/llvm/llvm-project/pull/70234 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [llvm] [libc] [lld] [libcxx] [clang-tools-extra] [lldb] [OpenACC] Initial commits to support OpenACC (PR #70234)
@@ -0,0 +1,14 @@
+// RUN: %clang -S -### -fopenacc %s 2>&1 | FileCheck %s
--check-prefix=CHECK-DRIVER
+// CHECK-DRIVER: "-cc1" {{.*}} "-fopenacc"
bcardosolopes wrote:
How does the interaction between using both `-fopenmp` and `-fopenacc` at the
same time should look like? One takes precedence? Or should it be rejected?
https://github.com/llvm/llvm-project/pull/70234
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libc] [flang] [lldb] [libcxx] [llvm] [lld] [clang-tools-extra] [OpenACC] Initial commits to support OpenACC (PR #70234)
@@ -605,6 +605,17 @@ static void InitializeStandardPredefinedMacros(const
TargetInfo &TI,
Builder.defineMacro("HIP_API_PER_THREAD_DEFAULT_STREAM");
}
}
+
+ if (LangOpts.OpenACC) {
+// FIXME: When we have full support for OpenACC, we should set this to the
+// version we support. Until then, set as '1' by default, but provide a
+// temporary mechanism for users to override this so real-world examples
can
+// be tested against.
+if (!LangOpts.OpenACCMacroOverride.empty())
+ Builder.defineMacro("_OPENACC", LangOpts.OpenACCMacroOverride);
+else
+ Builder.defineMacro("_OPENACC", "1");
bcardosolopes wrote:
This probably deserves a test akin to `-E -dM` and FileCheck for the macro.
https://github.com/llvm/llvm-project/pull/70234
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [RFC] Perform lifetime bound checks for arguments to coroutine (PR #69360)
@@ -7584,11 +7584,22 @@ static void
visitLifetimeBoundArguments(IndirectLocalPath &Path, Expr *Call,
if (ObjectArg && implicitObjectParamIsLifetimeBound(Callee))
VisitLifetimeBoundArg(Callee, ObjectArg);
-
+ bool checkCoroCall = false;
+ if (const auto *RD = Callee->getReturnType()->getAsRecordDecl()) {
+for (const auto &attr :
+ RD->getUnderlyingDecl()->specific_attrs()) {
+ // Only for demonstration: Get feedback and add a clang annotation as an
+ // extension.
+ if (attr->getAnnotation() == "coro_type") {
bcardosolopes wrote:
I don't think you need this to find if a record is a "coroutine type", there
are other ways, for example:
```
if (rec->getDeclName().isIdentifier() && rec->getName() == "promise_type") {
```
https://github.com/llvm/llvm-project/pull/69360
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r308225 - Check for _MSC_VER before defining _LIBCPP_MSVCRT
Author: bruno Date: Mon Jul 17 14:52:31 2017 New Revision: 308225 URL: http://llvm.org/viewvc/llvm-project?rev=308225&view=rev Log: Check for _MSC_VER before defining _LIBCPP_MSVCRT Some targets (e.g. Darwin) might have the Win32 API available, but they do not use MSVC CRT. Assume _LIBCPP_MSVCRT only when _MSC_VER is available and __MINGW32__ isn't defined. Differential Revision: https://reviews.llvm.org/D34588 rdar://problem/32628786 Modified: libcxx/trunk/include/__config Modified: libcxx/trunk/include/__config URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config?rev=308225&r1=308224&r2=308225&view=diff == --- libcxx/trunk/include/__config (original) +++ libcxx/trunk/include/__config Mon Jul 17 14:52:31 2017 @@ -229,8 +229,9 @@ # define _LIBCPP_SHORT_WCHAR 1 // Both MinGW and native MSVC provide a "MSVC"-like enviroment # define _LIBCPP_MSVCRT_LIKE -// If mingw not explicitly detected, assume using MS C runtime only. -# ifndef __MINGW32__ +// If mingw not explicitly detected, assume using MS C runtime only if +// a MS compatibility version is specified. +# if defined(_MSC_VER) && !defined(__MINGW32__) #define _LIBCPP_MSVCRT // Using Microsoft's C Runtime library # endif # if (defined(_M_AMD64) || defined(__x86_64__)) || (defined(_M_ARM) || defined(__arm__)) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r309722 - [Sema] Fix lax conversion between non ext vectors
Author: bruno
Date: Tue Aug 1 12:05:25 2017
New Revision: 309722
URL: http://llvm.org/viewvc/llvm-project?rev=309722&view=rev
Log:
[Sema] Fix lax conversion between non ext vectors
r282968 introduced a regression due to the lack of proper testing.
Re-add lax conversion support between non ext vectors for compound
assignments and add a test for that.
rdar://problem/28639467
Modified:
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/vector-cast.c
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=309722&r1=309721&r2=309722&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Aug 1 12:05:25 2017
@@ -8288,7 +8288,7 @@ QualType Sema::CheckVectorOperands(ExprR
// type. Note that this is already done by non-compound assignments in
// CheckAssignmentConstraints. If it's a scalar type, only bitcast for
// <1 x T> -> T. The result is also a vector type.
-} else if (OtherType->isExtVectorType() ||
+} else if (OtherType->isExtVectorType() || OtherType->isVectorType() ||
(OtherType->isScalarType() && VT->getNumElements() == 1)) {
ExprResult *RHSExpr = &RHS;
*RHSExpr = ImpCastExprToType(RHSExpr->get(), LHSType, CK_BitCast);
Modified: cfe/trunk/test/Sema/vector-cast.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/vector-cast.c?rev=309722&r1=309721&r2=309722&view=diff
==
--- cfe/trunk/test/Sema/vector-cast.c (original)
+++ cfe/trunk/test/Sema/vector-cast.c Tue Aug 1 12:05:25 2017
@@ -48,6 +48,9 @@ typedef float float2 __attribute__ ((vec
typedef __attribute__((vector_size(8))) double float64x1_t;
typedef __attribute__((vector_size(16))) double float64x2_t;
float64x1_t vget_low_f64(float64x2_t __p0);
+typedef float float16 __attribute__((__vector_size__(16)));
+typedef signed int vSInt32 __attribute__((__vector_size__(16)));
+typedef unsigned int vUInt32 __attribute__((__vector_size__(16)));
void f4() {
float2 f2;
@@ -73,3 +76,8 @@ void f5() {
v = ptr; // expected-error-re {{assigning to 'short_sizeof_pointer' (vector
of {{[0-9]+}} 'short' values) from incompatible type 'void *'}}
ptr = v; // expected-error {{assigning to 'void *' from incompatible type
'short_sizeof_pointer'}}
}
+
+void f6(vSInt32 a0) {
+ vUInt32 counter = (float16){0.0f, 0.0f, 0.0f, 0.0f}; // expected-warning
{{incompatible vector types initializing 'vUInt32' (vector of 4 'unsigned int'
values) with an expression of type 'float16' (vector of 4 'float' values)}}
+ counter -= a0;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r309752 - [Headers][Darwin] Allow #include_next to work on Darwin prior to 10.7
Author: bruno Date: Tue Aug 1 15:10:36 2017 New Revision: 309752 URL: http://llvm.org/viewvc/llvm-project?rev=309752&view=rev Log: [Headers][Darwin] Allow #include_next to work on Darwin prior to 10.7 This fixes PR31504 and it's a follow up from adding #include_next for Darwin in r289018. rdar://problem/29856682 Modified: cfe/trunk/lib/Headers/float.h Modified: cfe/trunk/lib/Headers/float.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/float.h?rev=309752&r1=309751&r2=309752&view=diff == --- cfe/trunk/lib/Headers/float.h (original) +++ cfe/trunk/lib/Headers/float.h Tue Aug 1 15:10:36 2017 @@ -33,6 +33,15 @@ */ #if (defined(__APPLE__) || (defined(__MINGW32__) || defined(_MSC_VER))) && \ __STDC_HOSTED__ && __has_include_next() + +/* Prior to Apple's 10.7 SDK, float.h SDK header used to apply an extra level + * of #include_next to keep Metrowerks compilers happy. Avoid this + * extra indirection. + */ +#ifdef __APPLE__ +#define _FLOAT_H_ +#endif + # include_next /* Undefine anything that we'll be redefining below. */ ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r309752 - [Headers][Darwin] Allow #include_next to work on Darwin prior to 10.7
On Tue, Aug 1, 2017 at 4:34 PM, Hans Wennborg wrote: > Merged in r309764. > > You cc'd me on the bug, so I noticed that :-) > > Just marking a bug a release blocker doesn't generally work though; it > needs to be marked as blocking PR33849. (Though I will search for > "release blocker" severity bugs now and then.) > > On Tue, Aug 1, 2017 at 3:15 PM, Bruno Cardoso Lopes > wrote: >> Hi Hans, >> >> Can we merge this to 5.0 too? I tried to set up the bugzilla to mark >> it as "release blocker", is that enough for you to see it? or next >> time is something I can do to make your life easier? Cool, thanks again! >> Thanks, >> >> On Tue, Aug 1, 2017 at 3:10 PM, Bruno Cardoso Lopes via cfe-commits >> wrote: >>> Author: bruno >>> Date: Tue Aug 1 15:10:36 2017 >>> New Revision: 309752 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=309752&view=rev >>> Log: >>> [Headers][Darwin] Allow #include_next to work on Darwin prior to >>> 10.7 >>> >>> This fixes PR31504 and it's a follow up from adding #include_next >>> for Darwin in r289018. >>> >>> rdar://problem/29856682 >>> >>> Modified: >>> cfe/trunk/lib/Headers/float.h >>> >>> Modified: cfe/trunk/lib/Headers/float.h >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/float.h?rev=309752&r1=309751&r2=309752&view=diff >>> == >>> --- cfe/trunk/lib/Headers/float.h (original) >>> +++ cfe/trunk/lib/Headers/float.h Tue Aug 1 15:10:36 2017 >>> @@ -33,6 +33,15 @@ >>> */ >>> #if (defined(__APPLE__) || (defined(__MINGW32__) || defined(_MSC_VER))) && >>> \ >>> __STDC_HOSTED__ && __has_include_next() >>> + >>> +/* Prior to Apple's 10.7 SDK, float.h SDK header used to apply an extra >>> level >>> + * of #include_next to keep Metrowerks compilers happy. Avoid this >>> + * extra indirection. >>> + */ >>> +#ifdef __APPLE__ >>> +#define _FLOAT_H_ >>> +#endif >>> + >>> # include_next >>> >>> /* Undefine anything that we'll be redefining below. */ >>> >>> >>> ___ >>> cfe-commits mailing list >>> cfe-commits@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >> >> >> -- >> Bruno Cardoso Lopes >> http://www.brunocardoso.cc -- Bruno Cardoso Lopes http://www.brunocardoso.cc ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r310605 - [Modules] Prevent #import to reenter header if not building a module.
Author: bruno Date: Thu Aug 10 08:16:24 2017 New Revision: 310605 URL: http://llvm.org/viewvc/llvm-project?rev=310605&view=rev Log: [Modules] Prevent #import to reenter header if not building a module. When non-modular headers are imported while not building a module but in -fmodules mode, be conservative and preserve the default #import semantic: do not reenter headers. rdar://problem/33745031 Added: cfe/trunk/test/Modules/Inputs/import-textual/x.h cfe/trunk/test/Modules/import-textual-nomodules.m Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=310605&r1=310604&r2=310605&view=diff == --- cfe/trunk/lib/Lex/HeaderSearch.cpp (original) +++ cfe/trunk/lib/Lex/HeaderSearch.cpp Thu Aug 10 08:16:24 2017 @@ -1143,7 +1143,7 @@ bool HeaderSearch::ShouldEnterIncludeFil // headers find in the wild might rely only on #import and do not contain // controlling macros, be conservative and only try to enter textual headers // if such macro is present. -if (!FileInfo.isModuleHeader && +if (FileInfo.isCompilingModuleHeader && !FileInfo.isModuleHeader && FileInfo.getControllingMacro(ExternalLookup)) TryEnterHdr = true; return TryEnterHdr; Added: cfe/trunk/test/Modules/Inputs/import-textual/x.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/import-textual/x.h?rev=310605&view=auto == --- cfe/trunk/test/Modules/Inputs/import-textual/x.h (added) +++ cfe/trunk/test/Modules/Inputs/import-textual/x.h Thu Aug 10 08:16:24 2017 @@ -0,0 +1,6 @@ +#ifndef RANDOM_DEP + +@interface X +@end + +#endif // RANDOM_DEP Added: cfe/trunk/test/Modules/import-textual-nomodules.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/import-textual-nomodules.m?rev=310605&view=auto == --- cfe/trunk/test/Modules/import-textual-nomodules.m (added) +++ cfe/trunk/test/Modules/import-textual-nomodules.m Thu Aug 10 08:16:24 2017 @@ -0,0 +1,8 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -I%S/Inputs/import-textual -fmodules-cache-path=%t %s -verify + +// expected-no-diagnostics + +#import "x.h" +#import "x.h" + ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r310605 - [Modules] Prevent #import to reenter header if not building a module.
Hi Hans, can we please get this merged into 5.0? Thanks, On Thu, Aug 10, 2017 at 12:16 PM, Bruno Cardoso Lopes via cfe-commits wrote: > Author: bruno > Date: Thu Aug 10 08:16:24 2017 > New Revision: 310605 > > URL: http://llvm.org/viewvc/llvm-project?rev=310605&view=rev > Log: > [Modules] Prevent #import to reenter header if not building a module. > > When non-modular headers are imported while not building a module but > in -fmodules mode, be conservative and preserve the default #import > semantic: do not reenter headers. > > rdar://problem/33745031 > > Added: > cfe/trunk/test/Modules/Inputs/import-textual/x.h > cfe/trunk/test/Modules/import-textual-nomodules.m > Modified: > cfe/trunk/lib/Lex/HeaderSearch.cpp > > Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=310605&r1=310604&r2=310605&view=diff > == > --- cfe/trunk/lib/Lex/HeaderSearch.cpp (original) > +++ cfe/trunk/lib/Lex/HeaderSearch.cpp Thu Aug 10 08:16:24 2017 > @@ -1143,7 +1143,7 @@ bool HeaderSearch::ShouldEnterIncludeFil > // headers find in the wild might rely only on #import and do not contain > // controlling macros, be conservative and only try to enter textual > headers > // if such macro is present. > -if (!FileInfo.isModuleHeader && > +if (FileInfo.isCompilingModuleHeader && !FileInfo.isModuleHeader && > FileInfo.getControllingMacro(ExternalLookup)) >TryEnterHdr = true; > return TryEnterHdr; > > Added: cfe/trunk/test/Modules/Inputs/import-textual/x.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/import-textual/x.h?rev=310605&view=auto > == > --- cfe/trunk/test/Modules/Inputs/import-textual/x.h (added) > +++ cfe/trunk/test/Modules/Inputs/import-textual/x.h Thu Aug 10 08:16:24 2017 > @@ -0,0 +1,6 @@ > +#ifndef RANDOM_DEP > + > +@interface X > +@end > + > +#endif // RANDOM_DEP > > Added: cfe/trunk/test/Modules/import-textual-nomodules.m > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/import-textual-nomodules.m?rev=310605&view=auto > == > --- cfe/trunk/test/Modules/import-textual-nomodules.m (added) > +++ cfe/trunk/test/Modules/import-textual-nomodules.m Thu Aug 10 08:16:24 2017 > @@ -0,0 +1,8 @@ > +// RUN: rm -rf %t > +// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps > -I%S/Inputs/import-textual -fmodules-cache-path=%t %s -verify > + > +// expected-no-diagnostics > + > +#import "x.h" > +#import "x.h" > + > > > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits -- Bruno Cardoso Lopes http://www.brunocardoso.cc ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r310605 - [Modules] Prevent #import to reenter header if not building a module.
On Thu, Aug 10, 2017 at 5:36 PM, Richard Smith wrote: > On 10 August 2017 at 10:42, Hans Wennborg via cfe-commits > wrote: >> >> Sounds good to me. >> >> Richard, what do you think? >> >> On Thu, Aug 10, 2017 at 9:38 AM, Bruno Cardoso Lopes >> wrote: >> > Hi Hans, can we please get this merged into 5.0? >> > >> > Thanks, >> > >> > On Thu, Aug 10, 2017 at 12:16 PM, Bruno Cardoso Lopes via cfe-commits >> > wrote: >> >> Author: bruno >> >> Date: Thu Aug 10 08:16:24 2017 >> >> New Revision: 310605 >> >> >> >> URL: http://llvm.org/viewvc/llvm-project?rev=310605&view=rev >> >> Log: >> >> [Modules] Prevent #import to reenter header if not building a module. >> >> >> >> When non-modular headers are imported while not building a module but >> >> in -fmodules mode, be conservative and preserve the default #import >> >> semantic: do not reenter headers. > > > This comment doesn't appear to describe what this patch does: even when > building a module it does not re-enter headers except for ones declared > 'textual header' within the module being built. (isCompilingModuleHeader > doesn't mean "are we compiling a module header right now?", it means "is > this file declared as some kind of header within the current > CompilingModule?".) What I'm trying to achieve here is: do not try to enter headers that are not described as part of any module (textual) unless while building a module. AFAIU, FileInfo.isCompilingModuleHeader is basically LangOpts.isCompilingModule() && Mod->getTopLevelModule() == SourceModule, shouldn't it account for a more restrictive way to say "are we compiling a textual header while building a module"? Checking for FileInfo.isCompilingModuleHeader has the additional advantage that it would track previous attempts to import that header while building a module, in which case it's going to try to re-enter. It won't try to re-enter only in cases where that header was never imported while building a header -> the same behavior #import has without modules. Prior to r291644, clang would always avoid to re-enter a header. After r291644 clang has a relaxed version of that, which I now proposing to be less relaxed for #imports. > I'm nervous about taking this change: it will presumably break some cases > (particularly with local submodule visibility enabled) where a #pragma once > header is #included into multiple headers in the same module, by refusing to > re-enter such headers -- and it will fix other such cases, depending on what > exactly the header does and the structure of the module. I'd be interested in a such testcase to make sure we don't regress here! > If this were restricted to the case where we're not building a module > (!LangOpts.isCompilingModule()), then I'd be OK putting it on the branch. Ok. It might not be a good idea to have this in 5.0 anyway, better bake this for a while. But thinking forward, am I missing something here? What about: if ((LangOpts.isCompilingModule() || !isImport) && !FileInfo.isModuleHeader && FileInfo.getControllingMacro(ExternalLookup)) TryEnterHdr = true; Thanks, -- Bruno Cardoso Lopes http://www.brunocardoso.cc ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r310706 - [modules] Set the lexical DC for dummy tag decls that refer to hidden
Hi Alex,
On Fri, Aug 11, 2017 at 9:06 AM, Alex Lorenz via cfe-commits
wrote:
> Author: arphaman
> Date: Fri Aug 11 05:06:52 2017
> New Revision: 310706
>
> URL: http://llvm.org/viewvc/llvm-project?rev=310706&view=rev
> Log:
> [modules] Set the lexical DC for dummy tag decls that refer to hidden
> declarations that are made visible after the dummy is parsed and ODR verified
>
> Prior to this commit the
> "(getContainingDC(DC) == CurContext && "The next DeclContext should be
> lexically contained in the current one."),"
> assertion failure was triggered during semantic analysis of the dummy
> tag declaration that was declared in another tag declaration because its
> lexical context did not point to the outer tag decl.
>
> rdar://32292196
>
> Added:
> cfe/trunk/test/Modules/Inputs/innerstructredef.h
> cfe/trunk/test/Modules/inner-struct-redefines-invisible.m
> Modified:
> cfe/trunk/lib/Sema/SemaDecl.cpp
> cfe/trunk/test/Modules/Inputs/module.map
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=310706&r1=310705&r2=310706&view=diff
> ==
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Aug 11 05:06:52 2017
> @@ -13722,6 +13722,7 @@ Decl *Sema::ActOnTag(Scope *S, unsigned
>// comparison.
>SkipBody->CheckSameAsPrevious = true;
>SkipBody->New = createTagFromNewDecl();
> + SkipBody->New->setLexicalDeclContext(CurContext);
I think it would be cleaner to do this inside "createTagFromNewDecl" than here.
>SkipBody->Previous = Hidden;
> } else {
>SkipBody->ShouldSkip = true;
>
> Added: cfe/trunk/test/Modules/Inputs/innerstructredef.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/innerstructredef.h?rev=310706&view=auto
> ==
> --- cfe/trunk/test/Modules/Inputs/innerstructredef.h (added)
> +++ cfe/trunk/test/Modules/Inputs/innerstructredef.h Fri Aug 11 05:06:52 2017
> @@ -0,0 +1,6 @@
> +struct Outer {
> +// This definition is actually hidden since only submodule 'one' is imported.
> +struct Inner {
> + int x;
> +} field;
> +};
>
> Modified: cfe/trunk/test/Modules/Inputs/module.map
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/module.map?rev=310706&r1=310705&r2=310706&view=diff
> ==
> --- cfe/trunk/test/Modules/Inputs/module.map (original)
> +++ cfe/trunk/test/Modules/Inputs/module.map Fri Aug 11 05:06:52 2017
> @@ -451,3 +451,12 @@ module DebugNestedB {
> module objcAtKeywordMissingEnd {
>header "objcAtKeywordMissingEnd.h"
> }
> +
> +module innerstructredef {
> + module one {
> +header "empty.h"
> + }
> + module two {
> + header "innerstructredef.h"
> + }
> +}
>
> Added: cfe/trunk/test/Modules/inner-struct-redefines-invisible.m
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/inner-struct-redefines-invisible.m?rev=310706&view=auto
> ==
> --- cfe/trunk/test/Modules/inner-struct-redefines-invisible.m (added)
> +++ cfe/trunk/test/Modules/inner-struct-redefines-invisible.m Fri Aug 11
> 05:06:52 2017
> @@ -0,0 +1,12 @@
> +// RUN: rm -rf %t
> +// RUN: %clang_cc1 -fsyntax-only -I%S/Inputs -fmodules
> -fimplicit-module-maps -fmodules-cache-path=%t -verify %s
> +// expected-no-diagnostics
> +
> +@import innerstructredef.one;
> +
> +struct Outer {
> +// Should set lexical context when parsing 'Inner' here, otherwise there's a
> crash:
> +struct Inner {
> + int x;
> +} field;
> +};
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
--
Bruno Cardoso Lopes
http://www.brunocardoso.cc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r310775 - Revert "[Modules] Prevent #import to reenter header if not building a module."
Author: bruno Date: Fri Aug 11 18:38:26 2017 New Revision: 310775 URL: http://llvm.org/viewvc/llvm-project?rev=310775&view=rev Log: Revert "[Modules] Prevent #import to reenter header if not building a module." This reverts commit r310605. Richard pointed out a better way to achieve this, which I'll post a patch for soon. Removed: cfe/trunk/test/Modules/Inputs/import-textual/x.h cfe/trunk/test/Modules/import-textual-nomodules.m Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=310775&r1=310774&r2=310775&view=diff == --- cfe/trunk/lib/Lex/HeaderSearch.cpp (original) +++ cfe/trunk/lib/Lex/HeaderSearch.cpp Fri Aug 11 18:38:26 2017 @@ -1143,7 +1143,7 @@ bool HeaderSearch::ShouldEnterIncludeFil // headers find in the wild might rely only on #import and do not contain // controlling macros, be conservative and only try to enter textual headers // if such macro is present. -if (FileInfo.isCompilingModuleHeader && !FileInfo.isModuleHeader && +if (!FileInfo.isModuleHeader && FileInfo.getControllingMacro(ExternalLookup)) TryEnterHdr = true; return TryEnterHdr; Removed: cfe/trunk/test/Modules/Inputs/import-textual/x.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/import-textual/x.h?rev=310774&view=auto == --- cfe/trunk/test/Modules/Inputs/import-textual/x.h (original) +++ cfe/trunk/test/Modules/Inputs/import-textual/x.h (removed) @@ -1,6 +0,0 @@ -#ifndef RANDOM_DEP - -@interface X -@end - -#endif // RANDOM_DEP Removed: cfe/trunk/test/Modules/import-textual-nomodules.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/import-textual-nomodules.m?rev=310774&view=auto == --- cfe/trunk/test/Modules/import-textual-nomodules.m (original) +++ cfe/trunk/test/Modules/import-textual-nomodules.m (removed) @@ -1,8 +0,0 @@ -// RUN: rm -rf %t -// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -I%S/Inputs/import-textual -fmodules-cache-path=%t %s -verify - -// expected-no-diagnostics - -#import "x.h" -#import "x.h" - ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r310605 - [Modules] Prevent #import to reenter header if not building a module.
On Fri, Aug 11, 2017 at 9:27 PM, Richard Smith wrote:
> On 11 August 2017 at 16:51, Bruno Cardoso Lopes via cfe-commits
> wrote:
>>
>> On Thu, Aug 10, 2017 at 5:36 PM, Richard Smith
>> wrote:
>> > On 10 August 2017 at 10:42, Hans Wennborg via cfe-commits
>> > wrote:
>> >>
>> >> Sounds good to me.
>> >>
>> >> Richard, what do you think?
>> >>
>> >> On Thu, Aug 10, 2017 at 9:38 AM, Bruno Cardoso Lopes
>> >> wrote:
>> >> > Hi Hans, can we please get this merged into 5.0?
>> >> >
>> >> > Thanks,
>> >> >
>> >> > On Thu, Aug 10, 2017 at 12:16 PM, Bruno Cardoso Lopes via cfe-commits
>> >> > wrote:
>> >> >> Author: bruno
>> >> >> Date: Thu Aug 10 08:16:24 2017
>> >> >> New Revision: 310605
>> >> >>
>> >> >> URL: http://llvm.org/viewvc/llvm-project?rev=310605&view=rev
>> >> >> Log:
>> >> >> [Modules] Prevent #import to reenter header if not building a
>> >> >> module.
>> >> >>
>> >> >> When non-modular headers are imported while not building a module
>> >> >> but
>> >> >> in -fmodules mode, be conservative and preserve the default #import
>> >> >> semantic: do not reenter headers.
>> >
>> >
>> > This comment doesn't appear to describe what this patch does: even when
>> > building a module it does not re-enter headers except for ones declared
>> > 'textual header' within the module being built. (isCompilingModuleHeader
>> > doesn't mean "are we compiling a module header right now?", it means "is
>> > this file declared as some kind of header within the current
>> > CompilingModule?".)
>>
>> What I'm trying to achieve here is: do not try to enter headers that
>> are not described as part of any module (textual) unless while
>> building a module. AFAIU, FileInfo.isCompilingModuleHeader is
>> basically LangOpts.isCompilingModule() && Mod->getTopLevelModule() ==
>> SourceModule, shouldn't it account for a more restrictive way to say
>> "are we compiling a textual header while building a module"?
>
>
> The "more restrictive" part is the problem: this also changes the behavior
> when we *are* building a module. Consider:
>
> // once.h
> #ifndef ONCE_H
> #define ONCE_H
> #pragma once
> extern int once;
> #endif
>
> // a.h
> #include "once.h"
>
> // b.h
> #include "once.h"
>
> // module.map
> module X { module A { header "a.h" } module B { header "b.h" } }
>
> This change appears to break the above module (when built with local
> submodule visibility enabled): module X.B no longer includes "once.h" and so
> no longer exports the "once" variable.
Nice testcase. I'll add it soon.
>> Checking for FileInfo.isCompilingModuleHeader has the additional
>> advantage that it would track previous attempts to import that header
>> while building a module, in which case it's going to try to re-enter.
>> It won't try to re-enter only in cases where that header was never
>> imported while building a header -> the same behavior #import has
>> without modules.
>>
>> Prior to r291644, clang would always avoid to re-enter a header. After
>> r291644 clang has a relaxed version of that, which I now proposing to
>> be less relaxed for #imports.
>>
>> > I'm nervous about taking this change: it will presumably break some
>> > cases
>> > (particularly with local submodule visibility enabled) where a #pragma
>> > once
>> > header is #included into multiple headers in the same module, by
>> > refusing to
>> > re-enter such headers -- and it will fix other such cases, depending on
>> > what
>> > exactly the header does and the structure of the module.
>>
>> I'd be interested in a such testcase to make sure we don't regress here!
>>
>> > If this were restricted to the case where we're not building a module
>> > (!LangOpts.isCompilingModule()), then I'd be OK putting it on the
>> > branch.
>>
>> Ok. It might not be a good idea to have this in 5.0 anyway, better
>> bake this for a while. But thinking forward, am I missing something
>> here? What about:
>>
>> if ((LangOpts.isCompilingModule() || !isImport) &
r348789 - [constexpr][c++2a] Try-catch blocks in constexpr functions
Author: bruno
Date: Mon Dec 10 11:03:12 2018
New Revision: 348789
URL: http://llvm.org/viewvc/llvm-project?rev=348789&view=rev
Log:
[constexpr][c++2a] Try-catch blocks in constexpr functions
Implement support for try-catch blocks in constexpr functions, as
proposed in http://wg21.link/P1002 and voted in San Diego for c++20.
The idea is that we can still never throw inside constexpr, so the catch
block is never entered. A try-catch block like this:
try { f(); } catch (...) { }
is then morally equivalent to just
{ f(); }
Same idea should apply for function/constructor try blocks.
rdar://problem/45530773
Differential Revision: https://reviews.llvm.org/D55097
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
cfe/trunk/test/CXX/drs/dr6xx.cpp
cfe/trunk/www/cxx_status.html
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=348789&r1=348788&r2=348789&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Dec 10 11:03:12
2018
@@ -2357,6 +2357,13 @@ def warn_cxx11_compat_constexpr_body_inv
"use of this statement in a constexpr %select{function|constructor}0 "
"is incompatible with C++ standards before C++14">,
InGroup, DefaultIgnore;
+def ext_constexpr_body_invalid_stmt_cxx2a : ExtWarn<
+ "use of this statement in a constexpr %select{function|constructor}0 "
+ "is a C++2a extension">, InGroup;
+def warn_cxx17_compat_constexpr_body_invalid_stmt : Warning<
+ "use of this statement in a constexpr %select{function|constructor}0 "
+ "is incompatible with C++ standards before C++2a">,
+ InGroup, DefaultIgnore;
def ext_constexpr_type_definition : ExtWarn<
"type definition in a constexpr %select{function|constructor}0 "
"is a C++14 extension">, InGroup;
@@ -2409,6 +2416,16 @@ def note_constexpr_body_previous_return
"previous return statement is here">;
def err_constexpr_function_try_block : Error<
"function try block not allowed in constexpr
%select{function|constructor}0">;
+
+// c++2a function try blocks in constexpr
+def ext_constexpr_function_try_block_cxx2a : ExtWarn<
+ "function try block in constexpr %select{function|constructor}0 is "
+ "a C++2a extension">, InGroup;
+def warn_cxx17_compat_constexpr_function_try_block : Warning<
+ "function try block in constexpr %select{function|constructor}0 is "
+ "incompatible with C++ standards before C++2a">,
+ InGroup, DefaultIgnore;
+
def err_constexpr_union_ctor_no_init : Error<
"constexpr union constructor does not initialize any member">;
def err_constexpr_ctor_missing_init : Error<
Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=348789&r1=348788&r2=348789&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Mon Dec 10 11:03:12 2018
@@ -4279,6 +4279,9 @@ static EvalStmtResult EvaluateStmt(StmtR
case Stmt::CaseStmtClass:
case Stmt::DefaultStmtClass:
return EvaluateStmt(Result, Info, cast(S)->getSubStmt(), Case);
+ case Stmt::CXXTryStmtClass:
+// Evaluate try blocks by evaluating all sub statements.
+return EvaluateStmt(Result, Info, cast(S)->getTryBlock(),
Case);
}
}
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=348789&r1=348788&r2=348789&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Dec 10 11:03:12 2018
@@ -1803,7 +1803,7 @@ static void CheckConstexprCtorInitialize
static bool
CheckConstexprFunctionStmt(Sema &SemaRef, const FunctionDecl *Dcl, Stmt *S,
SmallVectorImpl &ReturnStmts,
- SourceLocation &Cxx1yLoc) {
+ SourceLocation &Cxx1yLoc, SourceLocation &Cxx2aLoc)
{
// - its function-body shall be [...] a compound-statement that contains only
switch (S->getStmtClass()) {
case Stmt::NullStmtClass:
@@ -1840,7 +1840,7 @@ CheckConstexprFunctionStmt(Sema &SemaRef
CompoundStmt *CompStmt = cast(S);
for (auto *BodyIt : CompStmt->body()) {
if (!CheckConstexprFunctionStmt(SemaRef, Dcl, BodyIt, ReturnStmts,
- Cxx1yLoc))
+ Cxx1yLoc, Cxx2aLoc))
return false;
}
return true;
@@ -1858,11 +1858,11 @@ CheckConstex
r340117 - Revert "[analyzer] [NFC] Split up RetainSummaryManager from RetainCountChecker"
Author: bruno
Date: Fri Aug 17 20:22:11 2018
New Revision: 340117
URL: http://llvm.org/viewvc/llvm-project?rev=340117&view=rev
Log:
Revert "[analyzer] [NFC] Split up RetainSummaryManager from RetainCountChecker"
This reverts commit a786521fa66c72edd308baff0c08961b6d964fb1.
Bots haven't caught up yet, but broke modules build with:
../tools/clang/include/clang/StaticAnalyzer/Checkers/MPIFunctionClassifier.h:18:10:
fatal error: cyclic dependency in module 'Clang_StaticAnalyzer_Core':
Clang_StaticAnalyzer_Core -> Clang_Analysis ->
Clang_StaticAnalyzer_Checkers -> Clang_StaticAnalyzer_Core
^
Added:
cfe/trunk/include/clang/Analysis/ObjCRetainCount.h
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.cpp
- copied, changed from r340114,
cfe/trunk/lib/Analysis/RetainSummaryManager.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.h
- copied, changed from r340114,
cfe/trunk/include/clang/Analysis/RetainSummaryManager.h
cfe/trunk/lib/StaticAnalyzer/Checkers/SelectorExtras.h
- copied, changed from r340114,
cfe/trunk/include/clang/StaticAnalyzer/Checkers/SelectorExtras.h
Removed:
cfe/trunk/include/clang/Analysis/RetainSummaryManager.h
cfe/trunk/include/clang/StaticAnalyzer/Checkers/SelectorExtras.h
cfe/trunk/lib/Analysis/RetainSummaryManager.cpp
Modified:
cfe/trunk/lib/ARCMigrate/CMakeLists.txt
cfe/trunk/lib/ARCMigrate/ObjCMT.cpp
cfe/trunk/lib/Analysis/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
cfe/trunk/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
Added: cfe/trunk/include/clang/Analysis/ObjCRetainCount.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ObjCRetainCount.h?rev=340117&view=auto
==
--- cfe/trunk/include/clang/Analysis/ObjCRetainCount.h (added)
+++ cfe/trunk/include/clang/Analysis/ObjCRetainCount.h Fri Aug 17 20:22:11 2018
@@ -0,0 +1,231 @@
+//==-- ObjCRetainCount.h - Retain count summaries for Cocoa ---*- C++
-*--//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file defines the core data structures for retain count "summaries"
+// for Objective-C and Core Foundation APIs. These summaries are used
+// by the static analyzer to summarize the retain/release effects of
+// function and method calls. This drives a path-sensitive typestate
+// analysis in the static analyzer, but can also potentially be used by
+// other clients.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_STATICANALYZER_CHECKERS_OBJCRETAINCOUNT_H
+#define LLVM_CLANG_STATICANALYZER_CHECKERS_OBJCRETAINCOUNT_H
+
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallVector.h"
+
+namespace clang {
+class FunctionDecl;
+class ObjCMethodDecl;
+
+namespace ento { namespace objc_retain {
+
+/// An ArgEffect summarizes the retain count behavior on an argument or
receiver
+/// to a function or method.
+enum ArgEffect {
+ /// There is no effect.
+ DoNothing,
+
+ /// The argument is treated as if an -autorelease message had been sent to
+ /// the referenced object.
+ Autorelease,
+
+ /// The argument is treated as if an -dealloc message had been sent to
+ /// the referenced object.
+ Dealloc,
+
+ /// The argument has its reference count decreased by 1. This is as
+ /// if CFRelease has been called on the argument.
+ DecRef,
+
+ /// The argument has its reference count decreased by 1. This is as
+ /// if a -release message has been sent to the argument. This differs
+ /// in behavior from DecRef when GC is enabled.
+ DecRefMsg,
+
+ /// The argument has its reference count decreased by 1 to model
+ /// a transferred bridge cast under ARC.
+ DecRefBridgedTransferred,
+
+ /// The argument has its reference count increased by 1. This is as
+ /// if a -retain message has been sent to the argument. This differs
+ /// in behavior from IncRef when GC is enabled.
+ IncRefMsg,
+
+ /// The argument has its reference count increased by 1. This is as
+ /// if CFRetain has been called on the argument.
+ IncRef,
+
+ /// Used to mark an argument as collectible in GC mode, currently a noop.
+ MakeCollectable,
+
+ /// The argument is a poin
Re: r340114 - [analyzer] [NFC] Split up RetainSummaryManager from RetainCountChecker
Hi George,
This broke the modules build, reverted in r340117 for now. I can help
you figure out any module map change if necessary next week.
Thanks,
On Fri, Aug 17, 2018 at 6:46 PM George Karpenkov via cfe-commits
wrote:
>
> Author: george.karpenkov
> Date: Fri Aug 17 18:45:50 2018
> New Revision: 340114
>
> URL: http://llvm.org/viewvc/llvm-project?rev=340114&view=rev
> Log:
> [analyzer] [NFC] Split up RetainSummaryManager from RetainCountChecker
>
> ARCMigrator is using code from RetainCountChecker, which is a layering
> violation (and it also does it badly, by using a different header, and
> then relying on implementation being present in a header file).
>
> This change splits up RetainSummaryManager into a separate library in
> lib/Analysis, which can be used independently of a checker.
>
> Differential Revision: https://reviews.llvm.org/D50934
>
> Added:
> cfe/trunk/include/clang/Analysis/RetainSummaryManager.h
> cfe/trunk/include/clang/StaticAnalyzer/Checkers/SelectorExtras.h
> cfe/trunk/lib/Analysis/RetainSummaryManager.cpp
> Removed:
> cfe/trunk/include/clang/Analysis/ObjCRetainCount.h
>
> cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.cpp
>
> cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.h
> cfe/trunk/lib/StaticAnalyzer/Checkers/SelectorExtras.h
> Modified:
> cfe/trunk/lib/ARCMigrate/CMakeLists.txt
> cfe/trunk/lib/ARCMigrate/ObjCMT.cpp
> cfe/trunk/lib/Analysis/CMakeLists.txt
> cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
> cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
> cfe/trunk/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
>
> cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
>
> cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
>
> cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
> cfe/trunk/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
>
> Removed: cfe/trunk/include/clang/Analysis/ObjCRetainCount.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ObjCRetainCount.h?rev=340113&view=auto
> ==
> --- cfe/trunk/include/clang/Analysis/ObjCRetainCount.h (original)
> +++ cfe/trunk/include/clang/Analysis/ObjCRetainCount.h (removed)
> @@ -1,231 +0,0 @@
> -//==-- ObjCRetainCount.h - Retain count summaries for Cocoa ---*- C++
> -*--//
> -//
> -// The LLVM Compiler Infrastructure
> -//
> -// This file is distributed under the University of Illinois Open Source
> -// License. See LICENSE.TXT for details.
> -//
> -//===--===//
> -//
> -// This file defines the core data structures for retain count "summaries"
> -// for Objective-C and Core Foundation APIs. These summaries are used
> -// by the static analyzer to summarize the retain/release effects of
> -// function and method calls. This drives a path-sensitive typestate
> -// analysis in the static analyzer, but can also potentially be used by
> -// other clients.
> -//
> -//===--===//
> -
> -#ifndef LLVM_CLANG_STATICANALYZER_CHECKERS_OBJCRETAINCOUNT_H
> -#define LLVM_CLANG_STATICANALYZER_CHECKERS_OBJCRETAINCOUNT_H
> -
> -#include "clang/Basic/LLVM.h"
> -#include "llvm/ADT/ArrayRef.h"
> -#include "llvm/ADT/SmallVector.h"
> -
> -namespace clang {
> -class FunctionDecl;
> -class ObjCMethodDecl;
> -
> -namespace ento { namespace objc_retain {
> -
> -/// An ArgEffect summarizes the retain count behavior on an argument or
> receiver
> -/// to a function or method.
> -enum ArgEffect {
> - /// There is no effect.
> - DoNothing,
> -
> - /// The argument is treated as if an -autorelease message had been sent to
> - /// the referenced object.
> - Autorelease,
> -
> - /// The argument is treated as if an -dealloc message had been sent to
> - /// the referenced object.
> - Dealloc,
> -
> - /// The argument has its reference count decreased by 1. This is as
> - /// if CFRelease has been called on the argument.
> - DecRef,
> -
> - /// The argument has its reference count decreased by 1. This is as
> - /// if a -release message has been sent to the argument. This differs
> - /// in behavior from DecRef when GC is enabled.
> - DecRefMsg,
> -
> - /// The argument has its reference count decreased by 1 to model
> - /// a transferred bridge cast under ARC.
> - DecRefBridgedTransferred,
> -
> - /// The argument has its reference count increased by 1. This is as
> - /// if a -retain message has been sent to the argument. This differs
> - /// in behavior from IncRef when GC is enabled.
> - IncRefMsg,
> -
> - /// The argument has its reference count increased by 1. This is as
> - /// if CFRetain has been call
[PATCH] D22183: [SemObjC] Fix TypoExpr handling in TransformObjCDictionaryLiteral
bruno created this revision.
bruno added reviewers: manmanren, doug.gregor.
bruno added a subscriber: cfe-commits.
Calls to TransformExpr for NSDictionary elements (keys and values) in
TransformObjCDictionaryLiteral might fail to obtain TypoCorrections. This is OK,
but the early exits with ExprError() don't allow TransformExpr to run for other
elements.
This avoids typo correction suggestions to kick in and has a major
drawback to compile time; it forces Transform to call TryTransform more
times than it should. Before this patch, the testcase added used to take
5s to compile!!! A bit more elaborate NSDictionary literal with some
undeclared enums would make the compiler take 22min to run, followed by
a crash.
http://reviews.llvm.org/D22183
Files:
lib/Sema/TreeTransform.h
test/SemaObjC/objc-dictionary-literal.m
Index: test/SemaObjC/objc-dictionary-literal.m
===
--- test/SemaObjC/objc-dictionary-literal.m
+++ test/SemaObjC/objc-dictionary-literal.m
@@ -63,3 +63,11 @@
return 0;
}
+enum XXXYYYZZZType { XXXYYYZZZTypeAny }; // expected-note {{'XXXYYYZZZTypeAny'
declared here}}
+
+void foo() {
+ NSDictionary *d = @{
+@"A" : @(XXXYYYZZZTypeA), // expected-error {{use of undeclared identifier
'XXXYYYZZZTypeA'; did you mean 'XXXYYYZZZTypeAny'}}
+@"F" : @(XXXYYYZZZTypeSomethingSomething), // expected-error {{use of
undeclared identifier 'XXXYYYZZZTypeSomethingSomething'}}
+ };
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -10956,23 +10956,19 @@
// Transform and check key.
ExprResult Key = getDerived().TransformExpr(OrigElement.Key);
-if (Key.isInvalid())
- return ExprError();
-
-if (Key.get() != OrigElement.Key)
+if (!Key.isInvalid() && Key.get() != OrigElement.Key)
ArgChanged = true;
// Transform and check value.
ExprResult Value
= getDerived().TransformExpr(OrigElement.Value);
-if (Value.isInvalid())
- return ExprError();
-
-if (Value.get() != OrigElement.Value)
+if (!Value.isInvalid() && Value.get() != OrigElement.Value)
ArgChanged = true;
ObjCDictionaryElement Element = {
- Key.get(), Value.get(), SourceLocation(), None
+Key.isInvalid() ? OrigElement.Key : Key.get(),
+Value.isInvalid() ? OrigElement.Value : Value.get(), SourceLocation(),
+None
};
Elements.push_back(Element);
}
Index: test/SemaObjC/objc-dictionary-literal.m
===
--- test/SemaObjC/objc-dictionary-literal.m
+++ test/SemaObjC/objc-dictionary-literal.m
@@ -63,3 +63,11 @@
return 0;
}
+enum XXXYYYZZZType { XXXYYYZZZTypeAny }; // expected-note {{'XXXYYYZZZTypeAny' declared here}}
+
+void foo() {
+ NSDictionary *d = @{
+@"A" : @(XXXYYYZZZTypeA), // expected-error {{use of undeclared identifier 'XXXYYYZZZTypeA'; did you mean 'XXXYYYZZZTypeAny'}}
+@"F" : @(XXXYYYZZZTypeSomethingSomething), // expected-error {{use of undeclared identifier 'XXXYYYZZZTypeSomethingSomething'}}
+ };
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -10956,23 +10956,19 @@
// Transform and check key.
ExprResult Key = getDerived().TransformExpr(OrigElement.Key);
-if (Key.isInvalid())
- return ExprError();
-
-if (Key.get() != OrigElement.Key)
+if (!Key.isInvalid() && Key.get() != OrigElement.Key)
ArgChanged = true;
// Transform and check value.
ExprResult Value
= getDerived().TransformExpr(OrigElement.Value);
-if (Value.isInvalid())
- return ExprError();
-
-if (Value.get() != OrigElement.Value)
+if (!Value.isInvalid() && Value.get() != OrigElement.Value)
ArgChanged = true;
ObjCDictionaryElement Element = {
- Key.get(), Value.get(), SourceLocation(), None
+Key.isInvalid() ? OrigElement.Key : Key.get(),
+Value.isInvalid() ? OrigElement.Value : Value.get(), SourceLocation(),
+None
};
Elements.push_back(Element);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D22248: [Sema] Create a separate group for incompatible function pointer warning
bruno created this revision.
bruno added reviewers: bob.wilson, doug.gregor, krememek.
bruno added subscribers: cfe-commits, dexonsmith.
Give incompatible pointer warning for function pointers its own diagnostic
group while leaving it as a subgroup of incompatible-pointer-types. This is in
preparation to promote -Wincompatible-function-pointer-types to error on some
darwin targets.
http://reviews.llvm.org/D22248
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaExpr.cpp
test/Sema/incompatible-function-pointer-types.c
test/Sema/initialize-noreturn.c
test/Sema/overloadable.c
Index: test/Sema/overloadable.c
===
--- test/Sema/overloadable.c
+++ test/Sema/overloadable.c
@@ -109,7 +109,7 @@
void (*ambiguous)(int *) = &foo; // expected-error{{initializing 'void (*)(int *)' with an expression of incompatible type ''}} expected-note@105{{candidate function}} expected-note@106{{candidate function}}
void *vp_ambiguous = &foo; // expected-error{{initializing 'void *' with an expression of incompatible type ''}} expected-note@105{{candidate function}} expected-note@106{{candidate function}}
- void (*specific1)(int *) = (void (*)(void *))&foo; // expected-warning{{incompatible pointer types initializing 'void (*)(int *)' with an expression of type 'void (*)(void *)'}}
+ void (*specific1)(int *) = (void (*)(void *))&foo; // expected-warning{{incompatible function pointer types initializing 'void (*)(int *)' with an expression of type 'void (*)(void *)'}}
void *specific2 = (void (*)(void *))&foo;
void disabled(void *c) __attribute__((overloadable, enable_if(0, "")));
Index: test/Sema/initialize-noreturn.c
===
--- test/Sema/initialize-noreturn.c
+++ test/Sema/initialize-noreturn.c
@@ -8,7 +8,7 @@
void foo_noret(void) __attribute__((noreturn));
void test() {
- Fn_noret fn2 = &foo; // expected-warning {{incompatible pointer types initializing 'Fn_noret'}}
+ Fn_noret fn2 = &foo; // expected-warning {{incompatible function pointer types initializing 'Fn_noret'}}
Fn_noret fn3 = &foo_noret;
Fn_ret fn4 = &foo_noret;
Fn_ret fn5 = &foo;
Index: test/Sema/incompatible-function-pointer-types.c
===
--- /dev/null
+++ test/Sema/incompatible-function-pointer-types.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only %s -Wincompatible-pointer-types -verify
+// RUN: %clang_cc1 -fsyntax-only %s -Wincompatible-function-pointer-types -verify
+
+// This test ensures that the subgroup of -Wincompatible-pointer-types warnings
+// that concern function pointers can be promoted (or not promoted) to an error
+// *separately* from the other -Wincompatible-pointer-type warnings.
+typedef int (*MyFnTyA)(int *, char *);
+
+int bar(char *a, int *b) { return 0; }
+int foo(MyFnTyA x) { return 0; } // expected-note {{passing argument to parameter 'x' here}}
+
+void baz() {
+ foo(&bar); // expected-warning {{incompatible function pointer types passing 'int (*)(char *, int *)' to parameter of type 'MyFnTyA' (aka 'int (*)(int *, char *)')}}
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -12420,10 +12420,14 @@
MayHaveConvFixit = true;
break;
case IncompatiblePointer:
- DiagKind =
-(Action == AA_Passing_CFAudited ?
- diag::err_arc_typecheck_convert_incompatible_pointer :
- diag::ext_typecheck_convert_incompatible_pointer);
+if (Action == AA_Passing_CFAudited)
+ DiagKind = diag::err_arc_typecheck_convert_incompatible_pointer;
+else if (SrcType->isFunctionPointerType() &&
+ DstType->isFunctionPointerType())
+ DiagKind = diag::ext_typecheck_convert_incompatible_function_pointer;
+else
+ DiagKind = diag::ext_typecheck_convert_incompatible_pointer;
+
CheckInferredResultType = DstType->isObjCObjectPointerType() &&
SrcType->isObjCObjectPointerType();
if (Hint.isNull() && !CheckInferredResultType) {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -6344,6 +6344,24 @@
"; remove *|"
"; remove &}3">,
InGroup;
+def ext_typecheck_convert_incompatible_function_pointer : ExtWarn<
+ "incompatible function pointer types "
+ "%select{%diff{assigning to $ from $|assigning to different types}0,1"
+ "|%diff{passing $ to parameter of type $|"
+ "passing to parameter of different type}0,1"
+ "|%diff{returning $ from a function with result type $|"
+ "returning from function with different return type}0,1"
+ "|%diff{converting $ to type $|converting between types}0,1"
+ "|%diff{initializing $ with an expres
Re: [PATCH] D22248: [Sema] Create a separate group for incompatible function pointer warning
bruno added a comment. Ping! https://reviews.llvm.org/D22248 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22183: [SemObjC] Fix TypoExpr handling in TransformObjCDictionaryLiteral
bruno updated this revision to Diff 64162.
bruno added a comment.
Thanks for the review Manman, I found a much better approach. Updated the patch
to reflect that!
https://reviews.llvm.org/D22183
Files:
lib/Parse/ParseObjc.cpp
test/SemaObjC/objc-array-literal.m
test/SemaObjC/objc-dictionary-literal.m
Index: test/SemaObjC/objc-dictionary-literal.m
===
--- test/SemaObjC/objc-dictionary-literal.m
+++ test/SemaObjC/objc-dictionary-literal.m
@@ -63,3 +63,10 @@
return 0;
}
+enum XXXYYYZZZType { XXXYYYZZZTypeAny }; // expected-note {{'XXXYYYZZZTypeAny' declared here}}
+void foo() {
+ NSDictionary *d = @{
+@"A" : @(XXXYYYZZZTypeA), // expected-error {{use of undeclared identifier 'XXXYYYZZZTypeA'; did you mean 'XXXYYYZZZTypeAny'}}
+@"F" : @(XXXYYYZZZTypeSomethingSomething), // expected-error {{use of undeclared identifier 'XXXYYYZZZTypeSomethingSomething'}}
+ };
+}
Index: test/SemaObjC/objc-array-literal.m
===
--- test/SemaObjC/objc-array-literal.m
+++ test/SemaObjC/objc-array-literal.m
@@ -67,3 +67,11 @@
x = @[ @"stuff", @"hello" "world"]; // expected-warning {{concatenated NSString literal for an NSArray expression}}
return x;
}
+
+enum XXXYYYZZZType { XXXYYYZZZTypeAny };
+void foo() {
+ NSArray *array = @[
+@(XXXYYYZZZTypeA), // expected-error {{use of undeclared identifier 'XXXYYYZZZTypeA'; did you mean 'XXXYYYZZZTypeAny'}}
+@(XXXYYYZZZTypeSomethingSomething) // expected-error {{use of undeclared identifier 'XXXYYYZZZTypeSomethingSomething'}}
+ ];
+}
Index: lib/Parse/ParseObjc.cpp
===
--- lib/Parse/ParseObjc.cpp
+++ lib/Parse/ParseObjc.cpp
@@ -3414,6 +3414,7 @@
ExprVector ElementExprs; // array elements.
ConsumeBracket(); // consume the l_square.
+ bool HasInvalidEltExpr = false;
while (Tok.isNot(tok::r_square)) {
// Parse list of array element expressions (all must be id types).
ExprResult Res(ParseAssignmentExpression());
@@ -3425,11 +3426,15 @@
return Res;
}
+Res = Actions.CorrectDelayedTyposInExpr(Res.get());
+if (Res.isInvalid())
+ HasInvalidEltExpr = true;
+
// Parse the ellipsis that indicates a pack expansion.
if (Tok.is(tok::ellipsis))
Res = Actions.ActOnPackExpansion(Res.get(), ConsumeToken());
if (Res.isInvalid())
- return true;
+ HasInvalidEltExpr = true;
ElementExprs.push_back(Res.get());
@@ -3440,13 +3445,18 @@
<< tok::comma);
}
SourceLocation EndLoc = ConsumeBracket(); // location of ']'
+
+ if (HasInvalidEltExpr)
+return ExprError();
+
MultiExprArg Args(ElementExprs);
return Actions.BuildObjCArrayLiteral(SourceRange(AtLoc, EndLoc), Args);
}
ExprResult Parser::ParseObjCDictionaryLiteral(SourceLocation AtLoc) {
SmallVector Elements; // dictionary elements.
ConsumeBrace(); // consume the l_square.
+ bool HasInvalidEltExpr = false;
while (Tok.isNot(tok::r_brace)) {
// Parse the comma separated key : value expressions.
ExprResult KeyExpr;
@@ -3476,8 +3486,15 @@
return ValueExpr;
}
-// Parse the ellipsis that designates this as a pack expansion.
-SourceLocation EllipsisLoc;
+// Check the key and value for possible typos
+KeyExpr = Actions.CorrectDelayedTyposInExpr(KeyExpr.get());
+ValueExpr = Actions.CorrectDelayedTyposInExpr(ValueExpr.get());
+if (KeyExpr.isInvalid() || ValueExpr.isInvalid())
+ HasInvalidEltExpr = true;
+
+// Parse the ellipsis that designates this as a pack expansion. Do not
+// ActOnPackExpansion here, leave it to template instantiation time where
+// we can get better diagnostics.
if (getLangOpts().CPlusPlus)
TryConsumeToken(tok::ellipsis, EllipsisLoc);
@@ -3493,6 +3510,9 @@
<< tok::comma);
}
SourceLocation EndLoc = ConsumeBrace();
+
+ if (HasInvalidEltExpr)
+return ExprError();
// Create the ObjCDictionaryLiteral.
return Actions.BuildObjCDictionaryLiteral(SourceRange(AtLoc, EndLoc),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22200: [libc++] Fix macOS Sierra build issues by declaring basic_string(const allocator_type& __a) noexcept.
bruno added a subscriber: bruno. bruno added a comment. Hi, Can you provide more details on what the build issues are? It seems that the line you introduce is covered in the ifdef below, wonder why it's not triggering in your build? Repository: rL LLVM https://reviews.llvm.org/D22200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22285: Support -masm= flag for x86 assembly targets
bruno added a subscriber: bruno.
bruno accepted this revision.
bruno added a reviewer: bruno.
bruno added a comment.
This revision is now accepted and ready to land.
LGTM with one small fix, see below.
Comment at: lib/Driver/Tools.cpp:6379
@@ +6378,3 @@
+void ClangAs::AddX86TargetArgs(const ArgList &Args,
+ArgStringList &CmdArgs) const {
+ if (Arg *A = Args.getLastArg(options::OPT_masm_EQ)) {
Fix the param indentation.
https://reviews.llvm.org/D22285
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22010: Add more gcc compatibility names to clang's cpuid.h
bruno added a subscriber: bruno. Comment at: lib/Headers/cpuid.h:114 @@ -109,2 +113,3 @@ #define bit_AVX 0x1000 +#define bit_F16C0x2000 #define bit_RDRND 0x4000 Isn't this one also meant for gcc compat? https://reviews.llvm.org/D22010 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D22474: [CodeGen] Suppress C++ static destructor registration
bruno created this revision.
bruno added reviewers: rsmith, doug.gregor.
bruno added subscribers: cfe-commits, dexonsmith.
C++ static destructors can be problematic in multi-threaded environment. Some
of the issues users often complain about include:
1. Teardown ordering: crashes when one thread is exiting the process and
calling destructors while another thread is still running and accessing the
destructing variables
2. Shared code that is compiled both as an application and as a library. When
library mode is chosen, goto (1).
3. Some projects currently override __cxa_atexit to avoid the behavior in
question.
To get around that, I propose we add the compiler flag
-fno-cxx-static-destructors, which allows clang to suppress static destructor
registration (suppress emitting __cxa_atexit, atexit, etc).
https://reviews.llvm.org/D22474
Files:
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
lib/CodeGen/ItaniumCXXABI.cpp
lib/Driver/Tools.cpp
lib/Frontend/CompilerInvocation.cpp
test/CodeGenCXX/static-destructor.cpp
Index: test/CodeGenCXX/static-destructor.cpp
===
--- test/CodeGenCXX/static-destructor.cpp
+++ test/CodeGenCXX/static-destructor.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 %s -triple=x86_64-pc-linux -emit-llvm -o - | FileCheck
--check-prefix=X86 %s
+// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin -emit-llvm
-fno-cxx-static-destructors -o - | FileCheck --check-prefix=X86-NOATEXIT %s
// RUN: %clang_cc1 %s -triple=wasm32 -emit-llvm -o - | FileCheck
--check-prefix=WASM %s
// RUN: %clang_cc1 %s -triple=armv7-apple-darwin9 -emit-llvm -o - | FileCheck
--check-prefix=ARM %s
@@ -19,6 +20,11 @@
// X86: define internal void @__cxx_global_var_init()
// X86: call i32 @__cxa_atexit(void (i8*)* bitcast (void (%class.Foo*)*
@_ZN3FooD1Ev to void (i8*)*), i8* getelementptr inbounds (%class.Foo,
%class.Foo* @global, i32 0, i32 0), i8* @__dso_handle)
+// X86 destructors have void return, and are registered directly with
__cxa_atexit.
+// X86-NOATEXIT: define internal void @__cxx_global_var_init()
+// X86-NOATEXIT-NOT: call i32 @__cxa_atexit(void (i8*)* bitcast (void
(%class.Foo*)* @_ZN3FooD1Ev to void (i8*)*), i8* getelementptr inbounds
(%class.Foo, %class.Foo* @global, i32 0, i32 0), i8* @__dso_handle)
+// X86-NOATEXIT-NOT: declare i32 @__cxa_atexit(void (i8*)*, i8*, i8*)
+
// ARM destructors return this, but can be registered directly with
__cxa_atexit
// because the calling conventions tolerate the mismatch.
// ARM: define internal void @__cxx_global_var_init()
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -546,6 +546,7 @@
Opts.AssumeSaneOperatorNew = !Args.hasArg(OPT_fno_assume_sane_operator_new);
Opts.ObjCAutoRefCountExceptions = Args.hasArg(OPT_fobjc_arc_exceptions);
Opts.CXAAtExit = !Args.hasArg(OPT_fno_use_cxa_atexit);
+ Opts.CXXNoExitTimeDtor = Args.hasArg(OPT_fno_cxx_static_destructors);
Opts.CXXCtorDtorAliases = Args.hasArg(OPT_mconstructor_aliases);
Opts.CodeModel = getCodeModel(Args, Diags);
Opts.DebugPass = Args.getLastArgValue(OPT_mdebug_pass);
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -5963,6 +5963,9 @@
CmdArgs.push_back("-fwhole-program-vtables");
}
+ if (Args.hasArg(options::OPT_fno_cxx_static_destructors))
+CmdArgs.push_back("-fno-cxx-static-destructors");
+
// Finally add the compile command to the compilation.
if (Args.hasArg(options::OPT__SLASH_fallback) &&
Output.getType() == types::TY_Object &&
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -2132,6 +2132,10 @@
const VarDecl &D,
llvm::Constant *dtor,
llvm::Constant *addr) {
+ // Do not register global dtor at all
+ if (CGM.getCodeGenOpts().CXXNoExitTimeDtor)
+return;
+
// Use __cxa_atexit if available.
if (CGM.getCodeGenOpts().CXAAtExit)
return emitGlobalDtorWithCXAAtExit(CGF, dtor, addr, D.getTLSKind());
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -40,6 +40,7 @@
CODEGENOPT(CoverageNoFunctionNamesInData, 1, 0) ///< Do not include function
names in GCDA files.
CODEGENOPT(CoverageExitBlockBeforeBody, 1, 0) ///< Whether to emit the exit
block before the body blocks in GCNO files.
CODEGENOPT(CXAAtExit , 1, 1) ///< Use __cxa_atexit for calling
destructors.
+CODEGENOPT(CXXNoExitTime
r275907 - [Sema] Create a separate group for incompatible function pointer warning
Author: bruno
Date: Mon Jul 18 15:37:06 2016
New Revision: 275907
URL: http://llvm.org/viewvc/llvm-project?rev=275907&view=rev
Log:
[Sema] Create a separate group for incompatible function pointer warning
Give incompatible function pointer warning its own diagnostic group
but still leave it as a subgroup of incompatible-pointer-types. This is in
preparation to promote -Wincompatible-function-pointer-types to error on
darwin.
Differential Revision: https://reviews.llvm.org/D22248
rdar://problem/12907612
Added:
cfe/trunk/test/Sema/incompatible-function-pointer-types.c
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/initialize-noreturn.c
cfe/trunk/test/Sema/overloadable.c
Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=275907&r1=275906&r2=275907&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Mon Jul 18 15:37:06 2016
@@ -225,9 +225,12 @@ def GNUIncludeNext : DiagGroup<"gnu-incl
def IncompatibleMSStruct : DiagGroup<"incompatible-ms-struct">;
def IncompatiblePointerTypesDiscardsQualifiers
: DiagGroup<"incompatible-pointer-types-discards-qualifiers">;
+def IncompatibleFunctionPointerTypes
+ : DiagGroup<"incompatible-function-pointer-types">;
def IncompatiblePointerTypes
: DiagGroup<"incompatible-pointer-types",
-[IncompatiblePointerTypesDiscardsQualifiers]>;
+[IncompatiblePointerTypesDiscardsQualifiers,
+ IncompatibleFunctionPointerTypes]>;
def IncompleteUmbrella : DiagGroup<"incomplete-umbrella">;
def NonModularIncludeInFrameworkModule
: DiagGroup<"non-modular-include-in-framework-module">;
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=275907&r1=275906&r2=275907&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Jul 18 15:37:06
2016
@@ -6351,6 +6351,24 @@ def ext_typecheck_convert_incompatible_p
"; remove *|"
"; remove &}3">,
InGroup;
+def ext_typecheck_convert_incompatible_function_pointer : ExtWarn<
+ "incompatible function pointer types "
+ "%select{%diff{assigning to $ from $|assigning to different types}0,1"
+ "|%diff{passing $ to parameter of type $|"
+ "passing to parameter of different type}0,1"
+ "|%diff{returning $ from a function with result type $|"
+ "returning from function with different return type}0,1"
+ "|%diff{converting $ to type $|converting between types}0,1"
+ "|%diff{initializing $ with an expression of type $|"
+ "initializing with expression of different type}0,1"
+ "|%diff{sending $ to parameter of type $|"
+ "sending to parameter of different type}0,1"
+ "|%diff{casting $ to type $|casting between types}0,1}2"
+ "%select{|; dereference with *|"
+ "; take the address with &|"
+ "; remove *|"
+ "; remove &}3">,
+ InGroup;
def ext_typecheck_convert_discards_qualifiers : ExtWarn<
"%select{%diff{assigning to $ from $|assigning to different types}0,1"
"|%diff{passing $ to parameter of type $|"
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=275907&r1=275906&r2=275907&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Jul 18 15:37:06 2016
@@ -12423,10 +12423,14 @@ bool Sema::DiagnoseAssignmentResult(Assi
MayHaveConvFixit = true;
break;
case IncompatiblePointer:
- DiagKind =
-(Action == AA_Passing_CFAudited ?
- diag::err_arc_typecheck_convert_incompatible_pointer :
- diag::ext_typecheck_convert_incompatible_pointer);
+if (Action == AA_Passing_CFAudited)
+ DiagKind = diag::err_arc_typecheck_convert_incompatible_pointer;
+else if (SrcType->isFunctionPointerType() &&
+ DstType->isFunctionPointerType())
+ DiagKind = diag::ext_typecheck_convert_incompatible_function_pointer;
+else
+ DiagKind = diag::ext_typecheck_convert_incompatible_pointer;
+
CheckInferredResultType = DstType->isObjCObjectPointerType() &&
SrcType->isObjCObjectPointerType();
if (Hint.isNull() && !CheckInferredResultType) {
Added: cfe/trunk/test/Sema/incompatible-function-pointer-types.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/incompatible-function-pointer-types.c?rev=275907&view=auto
==
--- cf
Re: [PATCH] D22248: [Sema] Create a separate group for incompatible function pointer warning
bruno closed this revision. bruno added a comment. Thanks, r275907. https://reviews.llvm.org/D22248 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22183: [SemObjC] Fix TypoExpr handling in TransformObjCDictionaryLiteral
bruno added a comment. Ping! https://reviews.llvm.org/D22183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D22525: [Sema] Add sizeof diagnostics for bzero
bruno created this revision.
bruno added a reviewer: zaks.anna.
bruno added a subscriber: cfe-commits.
For memset (and others) we can get diagnostics like:
struct stat { int x; };
void foo(struct stat *stamps) {
memset(stamps, 0, sizeof(stamps));
}
t.c:7:28: warning: 'memset' call operates on objects of type 'struct stat'
while the size is based on a different type 'struct stat *'
[-Wsizeof-pointer-memaccess]
memset(stamps, 0, sizeof(stamps));
~~^~
t.c:7:28: note: did you mean to dereference the argument to 'sizeof' (and
multiply it by the number of elements)?
memset(stamps, 0, sizeof(stamps));
^~
This patch adds support for the same class of warnings for bzero.
https://reviews.llvm.org/D22525
Files:
lib/AST/Decl.cpp
lib/Sema/SemaChecking.cpp
test/SemaCXX/warn-memset-bad-sizeof.cpp
Index: test/SemaCXX/warn-memset-bad-sizeof.cpp
===
--- test/SemaCXX/warn-memset-bad-sizeof.cpp
+++ test/SemaCXX/warn-memset-bad-sizeof.cpp
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -fsyntax-only -verify -Wno-sizeof-array-argument %s
//
+extern "C" void *bzero(void *, unsigned);
extern "C" void *memset(void *, int, unsigned);
extern "C" void *memmove(void *s1, const void *s2, unsigned n);
extern "C" void *memcpy(void *s1, const void *s2, unsigned n);
@@ -47,6 +48,19 @@
memset(heap_buffer, 0, sizeof(heap_buffer)); // \
// expected-warning {{'memset' call operates on objects of type 'char' while the size is based on a different type 'char *'}} expected-note{{did you mean to provide an explicit length?}}
+ bzero(&s, sizeof(&s)); // \
+ // expected-warning {{'bzero' call operates on objects of type 'S' while the size is based on a different type 'S *'}} expected-note{{did you mean to remove the addressof in the argument to 'sizeof' (and multiply it by the number of elements)?}}
+ bzero(ps, sizeof(ps)); // \
+ // expected-warning {{'bzero' call operates on objects of type 'S' while the size is based on a different type 'S *'}} expected-note{{did you mean to dereference the argument to 'sizeof' (and multiply it by the number of elements)?}}
+ bzero(ps2, sizeof(ps2)); // \
+ // expected-warning {{'bzero' call operates on objects of type 'S' while the size is based on a different type 'PS' (aka 'S *')}} expected-note{{did you mean to dereference the argument to 'sizeof' (and multiply it by the number of elements)?}}
+ bzero(ps2, sizeof(typeof(ps2))); // \
+ // expected-warning {{argument to 'sizeof' in 'bzero' call is the same pointer type}}
+ bzero(ps2, sizeof(PS)); // \
+ // expected-warning {{argument to 'sizeof' in 'bzero' call is the same pointer type}}
+ bzero(heap_buffer, sizeof(heap_buffer)); // \
+ // expected-warning {{'bzero' call operates on objects of type 'char' while the size is based on a different type 'char *'}} expected-note{{did you mean to provide an explicit length?}}
+
memcpy(&s, 0, sizeof(&s)); // \
// expected-warning {{'memcpy' call operates on objects of type 'S' while the size is based on a different type 'S *'}} expected-note{{did you mean to remove the addressof in the argument to 'sizeof' (and multiply it by the number of elements)?}}
memcpy(0, &s, sizeof(&s)); // \
@@ -73,6 +87,21 @@
memset(arr, 0, sizeof(arr));
memset(parr, 0, sizeof(parr));
+ bzero((void*)&s, sizeof(&s));
+ bzero(&s, sizeof(s));
+ bzero(&s, sizeof(S));
+ bzero(&s, sizeof(const S));
+ bzero(&s, sizeof(volatile S));
+ bzero(&s, sizeof(volatile const S));
+ bzero(&foo, sizeof(CFoo));
+ bzero(&foo, sizeof(VFoo));
+ bzero(&foo, sizeof(CVFoo));
+ bzero(ps, sizeof(*ps));
+ bzero(ps2, sizeof(*ps2));
+ bzero(ps2, sizeof(typeof(*ps2)));
+ bzero(arr, sizeof(arr));
+ bzero(parr, sizeof(parr));
+
memcpy(&foo, &const_foo, sizeof(Foo));
memcpy((void*)&s, 0, sizeof(&s));
memcpy(0, (void*)&s, sizeof(&s));
@@ -96,12 +125,17 @@
int iarr[14];
memset(&iarr[0], 0, sizeof iarr);
memset(iarr, 0, sizeof iarr);
+ bzero(&iarr[0], sizeof iarr);
+ bzero(iarr, sizeof iarr);
int* iparr[14];
memset(&iparr[0], 0, sizeof iparr);
memset(iparr, 0, sizeof iparr);
+ bzero(&iparr[0], sizeof iparr);
+ bzero(iparr, sizeof iparr);
memset(m, 0, sizeof(Mat));
+ bzero(m, sizeof(Mat));
// Copy to raw buffer shouldn't warn either
memcpy(&foo, &arr, sizeof(Foo));
@@ -114,12 +148,21 @@
for (;;) {}
&s;
}), 0, sizeof(s));
+
+ bzero(({
+if (0) {}
+while (0) {}
+for (;;) {}
+&s;
+ }), sizeof(s));
}
namespace ns {
void memset(void* s, char c, int n);
+void bzero(void* s, int n);
void f(int* i) {
memset(i, 0, sizeof(i));
+ bzero(i, sizeof(i));
}
}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -6125,13 +6125,15 @@
// It is possibl
r276020 - [SemaObjC] Improve ObjCDictionaryLiteral and ObjCArryLiteral diagnostics
Author: bruno
Date: Tue Jul 19 15:21:18 2016
New Revision: 276020
URL: http://llvm.org/viewvc/llvm-project?rev=276020&view=rev
Log:
[SemaObjC] Improve ObjCDictionaryLiteral and ObjCArryLiteral diagnostics
Sema actions on ObjCDictionaryLiteral and ObjCArryLiteral are currently
done as a side-effect of Sema upon parent expressions, which incurs of
delayed typo corrections for such literals to be performed by TypoTransforms
upon the ObjCDictionaryLiteral and ObjCArryLiteral themselves instead of
its elements individually.
This is specially bad because it was not designed to act on several
elements; searching through all possible combinations of corrections for
several elements is very expensive. Additionally, when one of the
elements has no correction candidate, we still explore all options and
at the end emit no typo corrections whatsoever.
Do the proper sema actions by acting on each element alone during appropriate
literal parsing time to get proper diagonistics and decent compile time
behavior.
Differential Revision: http://reviews.llvm.org/D22183
rdar://problem/21046678
Modified:
cfe/trunk/lib/Parse/ParseObjc.cpp
cfe/trunk/test/SemaObjC/objc-array-literal.m
cfe/trunk/test/SemaObjC/objc-dictionary-literal.m
Modified: cfe/trunk/lib/Parse/ParseObjc.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseObjc.cpp?rev=276020&r1=276019&r2=276020&view=diff
==
--- cfe/trunk/lib/Parse/ParseObjc.cpp (original)
+++ cfe/trunk/lib/Parse/ParseObjc.cpp Tue Jul 19 15:21:18 2016
@@ -3416,6 +3416,7 @@ ExprResult Parser::ParseObjCArrayLiteral
ExprVector ElementExprs; // array elements.
ConsumeBracket(); // consume the l_square.
+ bool HasInvalidEltExpr = false;
while (Tok.isNot(tok::r_square)) {
// Parse list of array element expressions (all must be id types).
ExprResult Res(ParseAssignmentExpression());
@@ -3427,11 +3428,15 @@ ExprResult Parser::ParseObjCArrayLiteral
return Res;
}
+Res = Actions.CorrectDelayedTyposInExpr(Res.get());
+if (Res.isInvalid())
+ HasInvalidEltExpr = true;
+
// Parse the ellipsis that indicates a pack expansion.
if (Tok.is(tok::ellipsis))
Res = Actions.ActOnPackExpansion(Res.get(), ConsumeToken());
if (Res.isInvalid())
- return true;
+ HasInvalidEltExpr = true;
ElementExprs.push_back(Res.get());
@@ -3442,6 +3447,10 @@ ExprResult Parser::ParseObjCArrayLiteral
<< tok::comma);
}
SourceLocation EndLoc = ConsumeBracket(); // location of ']'
+
+ if (HasInvalidEltExpr)
+return ExprError();
+
MultiExprArg Args(ElementExprs);
return Actions.BuildObjCArrayLiteral(SourceRange(AtLoc, EndLoc), Args);
}
@@ -3449,6 +3458,7 @@ ExprResult Parser::ParseObjCArrayLiteral
ExprResult Parser::ParseObjCDictionaryLiteral(SourceLocation AtLoc) {
SmallVector Elements; // dictionary elements.
ConsumeBrace(); // consume the l_square.
+ bool HasInvalidEltExpr = false;
while (Tok.isNot(tok::r_brace)) {
// Parse the comma separated key : value expressions.
ExprResult KeyExpr;
@@ -3478,7 +3488,15 @@ ExprResult Parser::ParseObjCDictionaryLi
return ValueExpr;
}
-// Parse the ellipsis that designates this as a pack expansion.
+// Check the key and value for possible typos
+KeyExpr = Actions.CorrectDelayedTyposInExpr(KeyExpr.get());
+ValueExpr = Actions.CorrectDelayedTyposInExpr(ValueExpr.get());
+if (KeyExpr.isInvalid() || ValueExpr.isInvalid())
+ HasInvalidEltExpr = true;
+
+// Parse the ellipsis that designates this as a pack expansion. Do not
+// ActOnPackExpansion here, leave it to template instantiation time where
+// we can get better diagnostics.
SourceLocation EllipsisLoc;
if (getLangOpts().CPlusPlus)
TryConsumeToken(tok::ellipsis, EllipsisLoc);
@@ -3495,6 +3513,9 @@ ExprResult Parser::ParseObjCDictionaryLi
<< tok::comma);
}
SourceLocation EndLoc = ConsumeBrace();
+
+ if (HasInvalidEltExpr)
+return ExprError();
// Create the ObjCDictionaryLiteral.
return Actions.BuildObjCDictionaryLiteral(SourceRange(AtLoc, EndLoc),
Modified: cfe/trunk/test/SemaObjC/objc-array-literal.m
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/objc-array-literal.m?rev=276020&r1=276019&r2=276020&view=diff
==
--- cfe/trunk/test/SemaObjC/objc-array-literal.m (original)
+++ cfe/trunk/test/SemaObjC/objc-array-literal.m Tue Jul 19 15:21:18 2016
@@ -67,3 +67,11 @@ id radar15147688() {
x = @[ @"stuff", @"hello" "world"]; // expected-warning {{concatenated
NSString literal for an NSArray expression}}
return x;
}
+
+enum XXXYYYZZZType { XXXYYYZZZTypeAny }; // expected-note {{'XXXYYY
Re: [PATCH] D22183: [SemObjC] Fix TypoExpr handling in TransformObjCDictionaryLiteral
bruno closed this revision. bruno added a comment. Thanks Manman, Committed r276020 https://reviews.llvm.org/D22183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r276907 - Add flags to toggle preservation of assembly comments
Hi Nirav,
This test is failing on darwin:
http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-incremental_check/26574
Can you take a look?
On Wed, Jul 27, 2016 at 12:57 PM, Nirav Dave via cfe-commits
wrote:
> Author: niravd
> Date: Wed Jul 27 14:57:40 2016
> New Revision: 276907
>
> URL: http://llvm.org/viewvc/llvm-project?rev=276907&view=rev
> Log:
> Add flags to toggle preservation of assembly comments
>
> Summary: Add -fpreserve-as-comments and -fno-preserve-as-comments.
>
> Reviewers: echristo, rnk
>
> Subscribers: mehdi_amini, llvm-commits
>
> Differential Revision: https://reviews.llvm.org/D22883
>
> Added:
> cfe/trunk/test/CodeGen/preserve-as-comments.c
> Modified:
> cfe/trunk/include/clang/Driver/Options.td
> cfe/trunk/include/clang/Frontend/CodeGenOptions.def
> cfe/trunk/lib/CodeGen/BackendUtil.cpp
> cfe/trunk/lib/Driver/Tools.cpp
> cfe/trunk/lib/Frontend/CompilerInvocation.cpp
>
> Modified: cfe/trunk/include/clang/Driver/Options.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=276907&r1=276906&r2=276907&view=diff
> ==
> --- cfe/trunk/include/clang/Driver/Options.td (original)
> +++ cfe/trunk/include/clang/Driver/Options.td Wed Jul 27 14:57:40 2016
> @@ -1089,6 +1089,9 @@ def fpie : Flag<["-"], "fpie">, Group def fno_pie : Flag<["-"], "fno-pie">, Group;
> def fplugin_EQ : Joined<["-"], "fplugin=">, Group,
> Flags<[DriverOption]>, MetaVarName<"">,
>HelpText<"Load the named plugin (dynamic shared object)">;
> +def fpreserve_as_comments : Flag<["-"], "fpreserve-as-comments">,
> Group;
> +def fno_preserve_as_comments : Flag<["-"], "fno-preserve-as-comments">,
> Group, Flags<[CC1Option]>,
> + HelpText<"Do not preserve comments in inline assembly">;
> def fprofile_arcs : Flag<["-"], "fprofile-arcs">, Group;
> def fno_profile_arcs : Flag<["-"], "fno-profile-arcs">, Group;
> def framework : Separate<["-"], "framework">, Flags<[LinkerInput]>;
>
> Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.def?rev=276907&r1=276906&r2=276907&view=diff
> ==
> --- cfe/trunk/include/clang/Frontend/CodeGenOptions.def (original)
> +++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def Wed Jul 27 14:57:40
> 2016
> @@ -32,6 +32,7 @@ CODEGENOPT(DisableIntegratedAS, 1, 0) //
> CODEGENOPT(CompressDebugSections, 1, 0) ///< -Wa,-compress-debug-sections
> CODEGENOPT(RelaxELFRelocations, 1, 0) ///< -Wa,--mrelax-relocations
> CODEGENOPT(AsmVerbose, 1, 0) ///< -dA, -fverbose-asm.
> +CODEGENOPT(PreserveAsmComments, 1, 1) ///< -dA, -fno-preserve-as-comments.
> CODEGENOPT(AssumeSaneOperatorNew , 1, 1) ///< implicit
> __attribute__((malloc)) operator new
> CODEGENOPT(Autolink , 1, 1) ///< -fno-autolink
> CODEGENOPT(ObjCAutoRefCountExceptions , 1, 0) ///< Whether ARC should be
> EH-safe.
>
> Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=276907&r1=276906&r2=276907&view=diff
> ==
> --- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
> +++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Wed Jul 27 14:57:40 2016
> @@ -604,6 +604,7 @@ void EmitAssemblyHelper::CreateTargetMac
>CodeGenOpts.IncrementalLinkerCompatible;
>Options.MCOptions.MCFatalWarnings = CodeGenOpts.FatalWarnings;
>Options.MCOptions.AsmVerbose = CodeGenOpts.AsmVerbose;
> + Options.MCOptions.PreserveAsmComments = CodeGenOpts.PreserveAsmComments;
>Options.MCOptions.ABIName = TargetOpts.ABI;
>
>TM.reset(TheTarget->createTargetMachine(Triple, TargetOpts.CPU,
> FeaturesStr,
>
> Modified: cfe/trunk/lib/Driver/Tools.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=276907&r1=276906&r2=276907&view=diff
> ==
> --- cfe/trunk/lib/Driver/Tools.cpp (original)
> +++ cfe/trunk/lib/Driver/Tools.cpp Wed Jul 27 14:57:40 2016
> @@ -4193,6 +4193,10 @@ void Clang::ConstructJob(Compilation &C,
> true))
> CmdArgs.push_back("-fno-jump-tables");
>
> + if (!Args.hasFlag(options::OPT_fpreserve_as_comments,
> +options::OPT_fno_preserve_as_comments, true))
> +CmdArgs.push_back("-fno-preserve-as-comments");
> +
>if (Arg *A = Args.getLastArg(options::OPT_mregparm_EQ)) {
> CmdArgs.push_back("-mregparm");
> CmdArgs.push_back(A->getValue());
>
> Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=276907&r1=276906&r2=276907&view=diff
> ==
Re: r277542 - [CUDA] Fix libdevice selection.
Hi Artem,
This broke
http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-incremental_check/26856,
can you take a look?
Thanks,
On Tue, Aug 2, 2016 at 4:12 PM, Artem Belevich via cfe-commits
wrote:
> Author: tra
> Date: Tue Aug 2 18:12:51 2016
> New Revision: 277542
>
> URL: http://llvm.org/viewvc/llvm-project?rev=277542&view=rev
> Log:
> [CUDA] Fix libdevice selection.
>
> This makes clang's libdevice selection match that of NVCC as described in
> http://docs.nvidia.com/cuda/libdevice-users-guide/basic-usage.html#version-selection
>
> If required libdevice variant is not found, driver now fails with an error.
>
> Differential Revision: https://reviews.llvm.org/D23037
>
> Added:
>
> cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/cuda/nvvm/libdevice/libdevice.compute_30.10.bc
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
> cfe/trunk/lib/Driver/ToolChains.cpp
> cfe/trunk/test/Driver/cuda-detect.cu
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=277542&r1=277541&r2=277542&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Tue Aug 2
> 18:12:51 2016
> @@ -26,6 +26,9 @@ def err_drv_cuda_bad_gpu_arch : Error<"U
> def err_drv_no_cuda_installation : Error<
>"cannot find CUDA installation. Provide its path via --cuda-path, or pass
> "
>"-nocudainc to build without CUDA includes.">;
> +def err_drv_no_cuda_libdevice : Error<
> + "cannot find libdevice for %0. Provide path to different CUDA installation
> "
> + "via --cuda-path, or pass -nocudalib to build without linking with
> libdevice.">;
> def err_drv_cuda_version_too_low : Error<
>"GPU arch %1 requires CUDA version at least %3, but installation at %0 is
> %2. "
>"Use --cuda-path to specify a different CUDA install, or pass "
>
> Modified: cfe/trunk/lib/Driver/ToolChains.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.cpp?rev=277542&r1=277541&r2=277542&view=diff
> ==
> --- cfe/trunk/lib/Driver/ToolChains.cpp (original)
> +++ cfe/trunk/lib/Driver/ToolChains.cpp Tue Aug 2 18:12:51 2016
> @@ -1791,22 +1791,32 @@ void Generic_GCC::CudaInstallationDetect
>LibDeviceName.size(), FileName.find('.', LibDeviceName.size()));
>LibDeviceMap[GpuArch] = FilePath.str();
>// Insert map entries for specifc devices with this compute capability.
> + // NVCC's choice of libdevice library version is rather peculiar:
> + //
> http://docs.nvidia.com/cuda/libdevice-users-guide/basic-usage.html#version-selection
> + // TODO: this will need to be updated once CUDA-8 is released.
>if (GpuArch == "compute_20") {
> LibDeviceMap["sm_20"] = FilePath;
> LibDeviceMap["sm_21"] = FilePath;
> +LibDeviceMap["sm_32"] = FilePath;
>} else if (GpuArch == "compute_30") {
> LibDeviceMap["sm_30"] = FilePath;
> -LibDeviceMap["sm_32"] = FilePath;
> - } else if (GpuArch == "compute_35") {
> -LibDeviceMap["sm_35"] = FilePath;
> -LibDeviceMap["sm_37"] = FilePath;
> - } else if (GpuArch == "compute_50") {
> +// compute_30 is the fallback libdevice variant for sm_30+,
> +// unless CUDA specifies different version for specific GPU
> +// arch.
> LibDeviceMap["sm_50"] = FilePath;
> LibDeviceMap["sm_52"] = FilePath;
> LibDeviceMap["sm_53"] = FilePath;
> +// sm_6? are currently all aliases for sm_53 in LLVM and
> +// should use compute_30.
> LibDeviceMap["sm_60"] = FilePath;
> LibDeviceMap["sm_61"] = FilePath;
> LibDeviceMap["sm_62"] = FilePath;
> + } else if (GpuArch == "compute_35") {
> +LibDeviceMap["sm_35"] = FilePath;
> +LibDeviceMap["sm_37"] = FilePath;
> + } else if (GpuArch == "compute_50") {
> +// NVCC does not use compute_50 libdevice at all at the moment.
> +// The version that's shipped with CUDA-7.5 is a copy of compute_30.
>}
> }
>
> @@ -4759,18 +4769,23 @@ CudaToolChain::addClangTargetOptions(con
>if (DriverArgs.hasArg(options::OPT_nocudalib))
> return;
>
> - std::string LibDeviceFile = CudaInstallation.getLibDeviceFile(
> - DriverArgs.getLastArgValue(options::OPT_march_EQ));
> - if (!LibDeviceFile.empty()) {
> -CC1Args.push_back("-mlink-cuda-bitcode");
> -CC1Args.push_back(DriverArgs.MakeArgString(LibDeviceFile));
> -
> -// Libdevice in CUDA-7.0 requires PTX version that's more recent
> -// than LLVM defaults to. Use PTX4.2 which is the PTX version that
> -// came with CUDA-7.0.
> -CC1Args.push_back("-target-featur
Re: [libcxx] r273034 - Add Filesystem TS -- Complete
Hi Eric,
After we upgraded our green dragon bots to El Captain (10.11), the
test below started to fail on some of our machines:
--
test/std/experimental/filesystem/fs.op.funcs/fs.op.hard_lk_ct/hard_link_count.pass.cpp
TEST_CASE(hard_link_count_for_directory)
{
uintmax_t DirExpect = 3;
uintmax_t Dir3Expect = 2;
#if defined(__APPLE__)
DirExpect += 2;
Dir3Expect += 1;
#endif
TEST_CHECK(hard_link_count(StaticEnv::Dir) == DirExpect);
TEST_CHECK(hard_link_count(StaticEnv::Dir3) == Dir3Expect);
std::error_code ec;
TEST_CHECK(hard_link_count(StaticEnv::Dir, ec) == DirExpect);
TEST_CHECK(hard_link_count(StaticEnv::Dir3, ec) == Dir3Expect);
}
You can see the issue in every recent (past 10 days) run on
http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-expensive/433
I noticed that commenting out the snippet around "#if
defined(__APPLE__)" would make it work. What's the rationale behind
the "#if defined(__APPLE__)" above?
Thanks,
On Tue, Jun 21, 2016 at 3:03 PM, Eric Fiselier via cfe-commits
wrote:
> The issue should be fixed in r273323.
>
> Thanks for the report and for your patience.
>
> /Eric
>
> On Mon, Jun 20, 2016 at 11:27 PM, Eric Fiselier wrote:
>>
>> Hi Artem,
>>
>> Sorry for the delay, I've been busy with school all day.
>> I'll check in a fix tomorrow morning.
>>
>> Sorry again,
>>
>> /Eric
>>
>> On Mon, Jun 20, 2016 at 2:31 PM, Eric Fiselier wrote:
>>>
>>> Oh shoot, I definitely didn't take that into account. I'll put together a
>>> fix.
>>>
>>> /Eric
>>>
>>>
>>>
>>> On Mon, Jun 20, 2016 at 2:27 PM, Artem Belevich wrote:
Eric,
Some tests appear to fail if the path to the tests' current directory
has some symlinks in it.
In my case source and build tree are in directory 'work' that's
symlinked to from my home directory:
/usr/local/home/tra/work -> /work/tra
This causes multiple failures in libcxx tests. One example:
projects/libcxx/test/std/experimental/filesystem/fs.op.funcs/fs.op.rename/rename.pass.cpp:121
121TEST_CHECK(read_symlink(bad_sym_dest) == dne);
dne:
/usr/local/home/tra/work/llvm/build/gpu/debug/projects/libcxx/test/filesystem/Output/dynamic_env/test.529143/dne
bad_sym_dest:
/usr/local/home/tra/work/llvm/build/gpu/debug/projects/libcxx/test/filesystem/Output/dynamic_env/test.529143/bad_sym2
These are the paths that traverse the 'work' symlink in my home
directory. However, the symlink that we're reading contains physical path
to
the directory. While it does link to the right place, it's not the path the
test expects.
#readlink
/usr/local/home/tra/work/llvm/build/gpu/debug/projects/libcxx/test/filesystem/Output/dynamic_env/test.529143/bad_sym2
/work/tra/llvm/build/gpu/debug/projects/libcxx/test/filesystem/Output/dynamic_env/test.529143/dne
I think we need to normalize paths before we compare them.
--Artem
On Sat, Jun 18, 2016 at 12:03 PM, Eric Fiselier via cfe-commits
wrote:
>
> > I assume the correct way to fix this is to disable
> > -Wcovered-switch-default while compiling
> > libcxx/src/experimental/filesystem/operations.cpp
>
> Agreed. Disabled in r273092.
>
> Thanks for your patience with this latest change,
>
> /Eric
>
> On Sat, Jun 18, 2016 at 12:54 PM, Adrian Prantl
> wrote:
>>
>> Hello Eric,
>>
>> this commit causes new warnings on our bots:
>>
>> clang/src/projects/libcxx/include/fstream:816:5: warning: default
>> label in switch which covers all enumeration values
>> [-Wcovered-switch-default]
>> default:
>>
>> The problem is with this defensive default statement in fstream:
>>
>>
>> template
>> 0792 typename basic_filebuf<_CharT, _Traits>::pos_type
>> 0793 basic_filebuf<_CharT, _Traits>::seekoff(off_type __off,
>> ios_base::seekdir __way,
>> 0794 ios_base::openmode)
>> 0795 {
>> 0796 #ifndef _LIBCPP_NO_EXCEPTIONS
>> 0797 if (!__cv_)
>> 0798 throw bad_cast();
>> 0799 #endif
>> 0800 int __width = __cv_->encoding();
>> 0801 if (__file_ == 0 || (__width <= 0 && __off != 0) || sync())
>> 0802 return pos_type(off_type(-1));
>> 0803 // __width > 0 || __off == 0
>> 0804 int __whence;
>> 0805 switch (__way)
>> 0806 {
>> 0807 case ios_base::beg:
>> 0808 __whence = SEEK_SET;
>> 0809 break;
>> 0810 case ios_base::cur:
>> 0811 __whence = SEEK_CUR;
>> 0812 break;
>> 0813 case ios_base::end:
>> 0814 __whence = SEEK_END;
>> 0815 break;
>> 0816 default:
>> 0817 return pos_type(off_type(-1));
>> 0818 }
>>
>>
>> I assume th
r277787 - [Sema] Add sizeof diagnostics for bzero
Author: bruno
Date: Thu Aug 4 18:55:22 2016
New Revision: 277787
URL: http://llvm.org/viewvc/llvm-project?rev=277787&view=rev
Log:
[Sema] Add sizeof diagnostics for bzero
For memset (and others) we can get diagnostics like:
struct stat { int x; };
void foo(struct stat *stamps) {
bzero(stamps, sizeof(stamps));
memset(stamps, 0, sizeof(stamps));
}
t.c:7:28: warning: 'memset' call operates on objects of type 'struct stat'
while the size is based on a different type 'struct stat *'
[-Wsizeof-pointer-memaccess]
memset(stamps, 0, sizeof(stamps));
~~^~
t.c:7:28: note: did you mean to dereference the argument to 'sizeof' (and
multiply it by the number of elements)?
memset(stamps, 0, sizeof(stamps));
^~
This patch implements the same class of warnings for bzero.
Differential Revision: https://reviews.llvm.org/D22525
rdar://problem/18963514
Modified:
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp
Modified: cfe/trunk/lib/AST/Decl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=277787&r1=277786&r2=277787&view=diff
==
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Thu Aug 4 18:55:22 2016
@@ -3408,6 +3408,10 @@ unsigned FunctionDecl::getMemoryFunction
case Builtin::BIstrlen:
return Builtin::BIstrlen;
+ case Builtin::BI__builtin_bzero:
+ case Builtin::BIbzero:
+return Builtin::BIbzero;
+
default:
if (isExternC()) {
if (FnInfo->isStr("memset"))
@@ -3430,6 +3434,8 @@ unsigned FunctionDecl::getMemoryFunction
return Builtin::BIstrndup;
else if (FnInfo->isStr("strlen"))
return Builtin::BIstrlen;
+ else if (FnInfo->isStr("bzero"))
+return Builtin::BIbzero;
}
break;
}
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=277787&r1=277786&r2=277787&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Aug 4 18:55:22 2016
@@ -6179,13 +6179,15 @@ void Sema::CheckMemaccessArguments(const
// It is possible to have a non-standard definition of memset. Validate
// we have enough arguments, and if not, abort further checking.
- unsigned ExpectedNumArgs = (BId == Builtin::BIstrndup ? 2 : 3);
+ unsigned ExpectedNumArgs =
+ (BId == Builtin::BIstrndup || Builtin::BIbzero ? 2 : 3);
if (Call->getNumArgs() < ExpectedNumArgs)
return;
- unsigned LastArg = (BId == Builtin::BImemset ||
+ unsigned LastArg = (BId == Builtin::BImemset || BId == Builtin::BIbzero ||
BId == Builtin::BIstrndup ? 1 : 2);
- unsigned LenArg = (BId == Builtin::BIstrndup ? 1 : 2);
+ unsigned LenArg =
+ (BId == Builtin::BIbzero || BId == Builtin::BIstrndup ? 1 : 2);
const Expr *LenExpr = Call->getArg(LenArg)->IgnoreParenImpCasts();
if (CheckMemorySizeofForComparison(*this, LenExpr, FnName,
Modified: cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp?rev=277787&r1=277786&r2=277787&view=diff
==
--- cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp Thu Aug 4 18:55:22 2016
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -fsyntax-only -verify -Wno-sizeof-array-argument %s
//
+extern "C" void *bzero(void *, unsigned);
extern "C" void *memset(void *, int, unsigned);
extern "C" void *memmove(void *s1, const void *s2, unsigned n);
extern "C" void *memcpy(void *s1, const void *s2, unsigned n);
@@ -47,6 +48,19 @@ void f(Mat m, const Foo& const_foo, char
memset(heap_buffer, 0, sizeof(heap_buffer)); // \
// expected-warning {{'memset' call operates on objects of type 'char'
while the size is based on a different type 'char *'}} expected-note{{did you
mean to provide an explicit length?}}
+ bzero(&s, sizeof(&s)); // \
+ // expected-warning {{'bzero' call operates on objects of type 'S' while
the size is based on a different type 'S *'}} expected-note{{did you mean to
remove the addressof in the argument to 'sizeof' (and multiply it by the number
of elements)?}}
+ bzero(ps, sizeof(ps)); // \
+ // expected-warning {{'bzero' call operates on objects of type 'S' while
the size is based on a different type 'S *'}} expected-note{{did you mean to
dereference the argument to 'sizeof' (and multiply it by the number of
elements)?}}
+ bzero(ps2, sizeof(ps2)); // \
+ // expected-warning {{'bzero' call operates on objects of type 'S' while
the size is based on a different type 'PS' (a
Re: [PATCH] D22525: [Sema] Add sizeof diagnostics for bzero
bruno closed this revision. bruno added a comment. Thanks! Committed in r277787 https://reviews.llvm.org/D22525 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r277830 - Revert "[Sema] Add sizeof diagnostics for bzero"
Author: bruno
Date: Fri Aug 5 11:41:00 2016
New Revision: 277830
URL: http://llvm.org/viewvc/llvm-project?rev=277830&view=rev
Log:
Revert "[Sema] Add sizeof diagnostics for bzero"
This reverts commit r277787, which caused PR28870.
Modified:
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp
Modified: cfe/trunk/lib/AST/Decl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=277830&r1=277829&r2=277830&view=diff
==
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Fri Aug 5 11:41:00 2016
@@ -3408,10 +3408,6 @@ unsigned FunctionDecl::getMemoryFunction
case Builtin::BIstrlen:
return Builtin::BIstrlen;
- case Builtin::BI__builtin_bzero:
- case Builtin::BIbzero:
-return Builtin::BIbzero;
-
default:
if (isExternC()) {
if (FnInfo->isStr("memset"))
@@ -3434,8 +3430,6 @@ unsigned FunctionDecl::getMemoryFunction
return Builtin::BIstrndup;
else if (FnInfo->isStr("strlen"))
return Builtin::BIstrlen;
- else if (FnInfo->isStr("bzero"))
-return Builtin::BIbzero;
}
break;
}
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=277830&r1=277829&r2=277830&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Aug 5 11:41:00 2016
@@ -6179,15 +6179,13 @@ void Sema::CheckMemaccessArguments(const
// It is possible to have a non-standard definition of memset. Validate
// we have enough arguments, and if not, abort further checking.
- unsigned ExpectedNumArgs =
- (BId == Builtin::BIstrndup || Builtin::BIbzero ? 2 : 3);
+ unsigned ExpectedNumArgs = (BId == Builtin::BIstrndup ? 2 : 3);
if (Call->getNumArgs() < ExpectedNumArgs)
return;
- unsigned LastArg = (BId == Builtin::BImemset || BId == Builtin::BIbzero ||
+ unsigned LastArg = (BId == Builtin::BImemset ||
BId == Builtin::BIstrndup ? 1 : 2);
- unsigned LenArg =
- (BId == Builtin::BIbzero || BId == Builtin::BIstrndup ? 1 : 2);
+ unsigned LenArg = (BId == Builtin::BIstrndup ? 1 : 2);
const Expr *LenExpr = Call->getArg(LenArg)->IgnoreParenImpCasts();
if (CheckMemorySizeofForComparison(*this, LenExpr, FnName,
Modified: cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp?rev=277830&r1=277829&r2=277830&view=diff
==
--- cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp Fri Aug 5 11:41:00 2016
@@ -1,6 +1,5 @@
// RUN: %clang_cc1 -fsyntax-only -verify -Wno-sizeof-array-argument %s
//
-extern "C" void *bzero(void *, unsigned);
extern "C" void *memset(void *, int, unsigned);
extern "C" void *memmove(void *s1, const void *s2, unsigned n);
extern "C" void *memcpy(void *s1, const void *s2, unsigned n);
@@ -48,19 +47,6 @@ void f(Mat m, const Foo& const_foo, char
memset(heap_buffer, 0, sizeof(heap_buffer)); // \
// expected-warning {{'memset' call operates on objects of type 'char'
while the size is based on a different type 'char *'}} expected-note{{did you
mean to provide an explicit length?}}
- bzero(&s, sizeof(&s)); // \
- // expected-warning {{'bzero' call operates on objects of type 'S' while
the size is based on a different type 'S *'}} expected-note{{did you mean to
remove the addressof in the argument to 'sizeof' (and multiply it by the number
of elements)?}}
- bzero(ps, sizeof(ps)); // \
- // expected-warning {{'bzero' call operates on objects of type 'S' while
the size is based on a different type 'S *'}} expected-note{{did you mean to
dereference the argument to 'sizeof' (and multiply it by the number of
elements)?}}
- bzero(ps2, sizeof(ps2)); // \
- // expected-warning {{'bzero' call operates on objects of type 'S' while
the size is based on a different type 'PS' (aka 'S *')}} expected-note{{did you
mean to dereference the argument to 'sizeof' (and multiply it by the number of
elements)?}}
- bzero(ps2, sizeof(typeof(ps2))); // \
- // expected-warning {{argument to 'sizeof' in 'bzero' call is the same
pointer type}}
- bzero(ps2, sizeof(PS)); // \
- // expected-warning {{argument to 'sizeof' in 'bzero' call is the same
pointer type}}
- bzero(heap_buffer, sizeof(heap_buffer)); // \
- // expected-warning {{'bzero' call operates on objects of type 'char'
while the size is based on a different type 'char *'}} expected-note{{did you
mean to provide an explicit length?}}
-
memcpy(&s, 0, sizeof(&s)); // \
// expected-warning {{'memcp
Re: r277787 - [Sema] Add sizeof diagnostics for bzero
Hi Gabor,
Thanks for the PR, I reverted it in r277830 until I can manage to fix it.
On Fri, Aug 5, 2016 at 6:44 AM, Gabor Ballabas wrote:
> Hi Bruno,
>
> My name is Gabor Ballabas, I work at the University of Szeged, Hungary.
> We run daily code size benchmarks testing trunk Clang. After your patch
> landed
> we started to get a Clang crash when compiling one of the files in our
> benchmark system.
>
> I created a bugreport with the details:
> https://llvm.org/bugs/show_bug.cgi?id=28870
>
> I hope you can take a look at this issue.
>
>
> Best regards,
> Gabor Ballabas
>
>
> On 08/05/2016 01:55 AM, Bruno Cardoso Lopes via cfe-commits wrote:
>>
>> Author: bruno
>> Date: Thu Aug 4 18:55:22 2016
>> New Revision: 277787
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=277787&view=rev
>> Log:
>> [Sema] Add sizeof diagnostics for bzero
>>
>> For memset (and others) we can get diagnostics like:
>>
>>struct stat { int x; };
>>void foo(struct stat *stamps) {
>> bzero(stamps, sizeof(stamps));
>> memset(stamps, 0, sizeof(stamps));
>>}
>>
>>t.c:7:28: warning: 'memset' call operates on objects of type 'struct
>> stat' while the size is based on a different type 'struct stat *'
>> [-Wsizeof-pointer-memaccess]
>> memset(stamps, 0, sizeof(stamps));
>> ~~^~
>>t.c:7:28: note: did you mean to dereference the argument to 'sizeof'
>> (and multiply it by the number of elements)?
>> memset(stamps, 0, sizeof(stamps));
>> ^~
>>
>> This patch implements the same class of warnings for bzero.
>>
>> Differential Revision: https://reviews.llvm.org/D22525
>>
>> rdar://problem/18963514
>>
>> Modified:
>> cfe/trunk/lib/AST/Decl.cpp
>> cfe/trunk/lib/Sema/SemaChecking.cpp
>> cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp
>>
>> Modified: cfe/trunk/lib/AST/Decl.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=277787&r1=277786&r2=277787&view=diff
>>
>> ==
>> --- cfe/trunk/lib/AST/Decl.cpp (original)
>> +++ cfe/trunk/lib/AST/Decl.cpp Thu Aug 4 18:55:22 2016
>> @@ -3408,6 +3408,10 @@ unsigned FunctionDecl::getMemoryFunction
>> case Builtin::BIstrlen:
>> return Builtin::BIstrlen;
>> + case Builtin::BI__builtin_bzero:
>> + case Builtin::BIbzero:
>> +return Builtin::BIbzero;
>> +
>> default:
>> if (isExternC()) {
>> if (FnInfo->isStr("memset"))
>> @@ -3430,6 +3434,8 @@ unsigned FunctionDecl::getMemoryFunction
>> return Builtin::BIstrndup;
>> else if (FnInfo->isStr("strlen"))
>> return Builtin::BIstrlen;
>> + else if (FnInfo->isStr("bzero"))
>> +return Builtin::BIbzero;
>> }
>> break;
>> }
>>
>> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=277787&r1=277786&r2=277787&view=diff
>>
>> ==
>> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Aug 4 18:55:22 2016
>> @@ -6179,13 +6179,15 @@ void Sema::CheckMemaccessArguments(const
>> // It is possible to have a non-standard definition of memset.
>> Validate
>> // we have enough arguments, and if not, abort further checking.
>> - unsigned ExpectedNumArgs = (BId == Builtin::BIstrndup ? 2 : 3);
>> + unsigned ExpectedNumArgs =
>> + (BId == Builtin::BIstrndup || Builtin::BIbzero ? 2 : 3);
>> if (Call->getNumArgs() < ExpectedNumArgs)
>> return;
>> - unsigned LastArg = (BId == Builtin::BImemset ||
>> + unsigned LastArg = (BId == Builtin::BImemset || BId == Builtin::BIbzero
>> ||
>> BId == Builtin::BIstrndup ? 1 : 2);
>> - unsigned LenArg = (BId == Builtin::BIstrndup ? 1 : 2);
>> + unsigned LenArg =
>> + (BId == Builtin::BIbzero || BId == Builtin::BIstrndup ? 1 : 2);
>> const Expr *LenExpr = Call->getArg(LenArg)->IgnoreParenImpCasts();
>> if (CheckMemorySizeofForComparison(*this, LenExpr, FnName,
>>
>> Modified: cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp
>> URL:
>> http://ll
Re: [libcxx] r273034 - Add Filesystem TS -- Complete
Ping!
On Thu, Aug 4, 2016 at 8:03 AM, Adrian Prantl wrote:
>
>> On Aug 3, 2016, at 1:56 PM, Bruno Cardoso Lopes
>> wrote:
>>
>> Hi Eric,
>>
>> After we upgraded our green dragon bots to El Captain (10.11), the
>> test below started to fail on some of our machines:
>>
>> --
>> test/std/experimental/filesystem/fs.op.funcs/fs.op.hard_lk_ct/hard_link_count.pass.cpp
>>
>> TEST_CASE(hard_link_count_for_directory)
>> {
>> uintmax_t DirExpect = 3;
>> uintmax_t Dir3Expect = 2;
>> #if defined(__APPLE__)
>> DirExpect += 2;
>> Dir3Expect += 1;
>> #endif
>
> Just as a general code review comment: When committing a platform-specific
> workaround, I would expect there to be a comment explaining what
> bug/peculiarity of the platform is being worked around :-)
>
> thanks,
> Adrian
>
>> TEST_CHECK(hard_link_count(StaticEnv::Dir) == DirExpect);
>> TEST_CHECK(hard_link_count(StaticEnv::Dir3) == Dir3Expect);
>>
>> std::error_code ec;
>> TEST_CHECK(hard_link_count(StaticEnv::Dir, ec) == DirExpect);
>> TEST_CHECK(hard_link_count(StaticEnv::Dir3, ec) == Dir3Expect);
>> }
>>
>> You can see the issue in every recent (past 10 days) run on
>> http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-expensive/433
>>
>> I noticed that commenting out the snippet around "#if
>> defined(__APPLE__)" would make it work. What's the rationale behind
>> the "#if defined(__APPLE__)" above?
>>
>> Thanks,
>>
>>
>> On Tue, Jun 21, 2016 at 3:03 PM, Eric Fiselier via cfe-commits
>> wrote:
>>> The issue should be fixed in r273323.
>>>
>>> Thanks for the report and for your patience.
>>>
>>> /Eric
>>>
>>> On Mon, Jun 20, 2016 at 11:27 PM, Eric Fiselier wrote:
Hi Artem,
Sorry for the delay, I've been busy with school all day.
I'll check in a fix tomorrow morning.
Sorry again,
/Eric
On Mon, Jun 20, 2016 at 2:31 PM, Eric Fiselier wrote:
>
> Oh shoot, I definitely didn't take that into account. I'll put together a
> fix.
>
> /Eric
>
>
>
> On Mon, Jun 20, 2016 at 2:27 PM, Artem Belevich wrote:
>>
>> Eric,
>>
>> Some tests appear to fail if the path to the tests' current directory
>> has some symlinks in it.
>> In my case source and build tree are in directory 'work' that's
>> symlinked to from my home directory:
>> /usr/local/home/tra/work -> /work/tra
>>
>> This causes multiple failures in libcxx tests. One example:
>>
>>
>> projects/libcxx/test/std/experimental/filesystem/fs.op.funcs/fs.op.rename/rename.pass.cpp:121
>> 121TEST_CHECK(read_symlink(bad_sym_dest) == dne);
>>
>> dne:
>>
>> /usr/local/home/tra/work/llvm/build/gpu/debug/projects/libcxx/test/filesystem/Output/dynamic_env/test.529143/dne
>> bad_sym_dest:
>>
>> /usr/local/home/tra/work/llvm/build/gpu/debug/projects/libcxx/test/filesystem/Output/dynamic_env/test.529143/bad_sym2
>>
>> These are the paths that traverse the 'work' symlink in my home
>> directory. However, the symlink that we're reading contains physical
>> path to
>> the directory. While it does link to the right place, it's not the path
>> the
>> test expects.
>>
>> #readlink
>> /usr/local/home/tra/work/llvm/build/gpu/debug/projects/libcxx/test/filesystem/Output/dynamic_env/test.529143/bad_sym2
>>
>> /work/tra/llvm/build/gpu/debug/projects/libcxx/test/filesystem/Output/dynamic_env/test.529143/dne
>>
>> I think we need to normalize paths before we compare them.
>>
>> --Artem
>>
>>
>> On Sat, Jun 18, 2016 at 12:03 PM, Eric Fiselier via cfe-commits
>> wrote:
>>>
I assume the correct way to fix this is to disable
-Wcovered-switch-default while compiling
libcxx/src/experimental/filesystem/operations.cpp
>>>
>>> Agreed. Disabled in r273092.
>>>
>>> Thanks for your patience with this latest change,
>>>
>>> /Eric
>>>
>>> On Sat, Jun 18, 2016 at 12:54 PM, Adrian Prantl
>>> wrote:
Hello Eric,
this commit causes new warnings on our bots:
clang/src/projects/libcxx/include/fstream:816:5: warning: default
label in switch which covers all enumeration values
[-Wcovered-switch-default]
default:
The problem is with this defensive default statement in fstream:
template
0792 typename basic_filebuf<_CharT, _Traits>::pos_type
0793 basic_filebuf<_CharT, _Traits>::seekoff(off_type __off,
ios_base::seekdir __way,
0794 ios_base::openmode)
0795 {
0796 #ifndef _LIBCPP_NO_EXCEPTIONS
0797 if (!__cv_)
0798 throw bad_cast();
0799 #endif
0800 int __width = __cv_->encoding();
0801 if (__file_ ==
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,88 @@
+//===--- CIRGenAction.cpp - LLVM Code generation Frontend Action -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/CIRFrontendAction/CIRGenAction.h"
+#include "clang/CIR/CIRGenerator.h"
+#include "clang/Frontend/CompilerInstance.h"
+
+#include "mlir/IR/MLIRContext.h"
+#include "mlir/IR/OwningOpRef.h"
+
+using namespace cir;
+using namespace clang;
+
+namespace cir {
+
+class CIRGenConsumer : public clang::ASTConsumer {
+
+ virtual void anchor();
+
+ [[maybe_unused]] CIRGenAction::OutputType action;
bcardosolopes wrote:
To silence warnings for things we are about to introduce in follow up commits.
Sometimes we prefetch the skeleton in NFC fashion, in order to make
functionality changes have a smaller / more relevant diff. But we could surely
remove them too.
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,61 @@
+//===--- CIRGenModule.h - Per-Module state for CIR gen --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This is the internal per-translation-unit state used for CIR translation.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENMODULE_H
+#define LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENMODULE_H
+
+#include "CIRGenTypeCache.h"
+
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/Diagnostic.h"
+
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/MLIRContext.h"
+
+using namespace clang;
+namespace cir {
+
+/// This class organizes the cross-function state that is used while generating
+/// CIR code.
+class CIRGenModule : public CIRGenTypeCache {
+ CIRGenModule(CIRGenModule &) = delete;
+ CIRGenModule &operator=(CIRGenModule &) = delete;
+
+public:
+ CIRGenModule(mlir::MLIRContext &context, clang::ASTContext &astctx,
+ const clang::CodeGenOptions &CGO,
+ clang::DiagnosticsEngine &Diags);
+
+ ~CIRGenModule();
+
+private:
+ /// Hold Clang AST information.
+ clang::ASTContext &astCtx;
+
+ const clang::LangOptions &langOpts;
bcardosolopes wrote:
Agreed, we've been using a mix so we didn't had to go the extra length for
something the community could ask us to change as part of upstreaming, so
indeed this is the right time to make it happen.
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -42,6 +47,14 @@ CreateFrontendBaseAction(CompilerInstance &CI) {
StringRef Action("unknown");
(void)Action;
+ auto UseCIR = CI.getFrontendOpts().UseClangIRPipeline;
bcardosolopes wrote:
Right, it's a different discussion than naming. @joker-eph: what's the `auto`
policy for MLIR, is it different from LLVM/Clang? I'm also having trouble to
find the previous discussions, let me search a bit more.
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
bcardosolopes wrote: Thanks for everyone's input so far. Let me try to summarize two discussions in this PR so we can set on an approach and give advice to our CIR community (and encode on our webpage) on how to move forward in upcoming patches. Useful resources: - [LLVM Coding Standard](https://llvm.org/docs/CodingStandards.html) - [MLIR Style Guide](https://mlir.llvm.org/getting_started/DeveloperGuide/#style-guide) CC'ing more people who might care: @seven-mile, @Lancern. ## Use of `auto` As @joker-eph mentioned, MLIR isn't meant to differ from LLVM/Clang, though they encode the differences as part of their guidelines. The `auto` [guidance](https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable) is still in favor of us keeping it for all `isa`, `dyn_cast` and `cast`, which is used in CIR, so it probably doesn't affect most of what we currently care about. Here's my suggestion for the entire `lib/CIR`: 1. Use it for function templates such as `isa`, `dyn_cast`, `cast`, `create` and `rewriteOp.*` variants. 2. Use it for things considered obvious/common in MLIR space, examples: - `auto loc = op->getLoc()`. - Getting operands and results from operations (they obey Value Semantics), e.g.: `DepthwiseConv2DNhwcHwcmOp op; ...; auto stride = op.getStrides();` - Other examples we are happy to provide upon reviewer feedback if it makes sense. Using the logic above, all `auto`s in this current PR have to be changed (since none apply). ## Namings: CamelCase vs camelBack >From this discussion, seems like @AaronBallman and @erichkeane are fine with >us using camelBack and all the other differences from [MLIR Style >Guide](https://mlir.llvm.org/getting_started/DeveloperGuide/#style-guide) in >CIRGen and the rest of CIR. Is that right? This is based on the comment: > The mixed naming conventions in the header should be fixed (preference is to > follow LLVM style if we're changing code around, but if the local preference > is for MLIR, that's fine so long as it's consistent). However, @AaronBallman also wrote: > Also, the fact that the naming keeps being inconsistent is a pretty strong > reason to change to use the LLVM naming convention, at least for external > interfaces. Should we ignore this in light of your first comment? If not, can you clarify what do you mean by external interfaces? Just want to make sure we get it right looking fwd. Does this makes sense? Thoughts? https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
bcardosolopes wrote: > What is obvious in the MLIR space is not necessarily what's obvious in Clang; Sure. > I have no idea whether that returns a `SourceLocation`, a `PresumedLoc`, a > `FullSourceLoc`, etc, so I don't think that is a use of `auto` I would want > to have to reason about as a reviewer. That said, if the only use of `loc` > is: `Diag(loc, diag::err_something);` then the type really doesn't matter and > use of `auto` is probably fine. It's a judgment call. For location, `getLoc` always return `mlir::Location`, it's not related to Clang's source location. CIRGen use of `loc` is mostly to pass it down to MLIR APIs, because MLIR source locations are required whenever one is building or rewriting an operation. We usually convert Clang's location into MLIR and pass those around as expected. Has that changed your opinion for this specific case? > Again, I have no idea what the type of that is and I have no way to verify > that it actually _uses_ value semantics because the pointer and qualifiers > can be inferred and references can be dropped. That makes it harder to review > the code; I would spell out the type in this case as well. Makes sense. For some background: MLIR "instructions", usually called "operations", usually have pretty default return types when querying operands, `mlir::Value` for SSA values or when an `mlir::Attribute` is returned, the getter/setter is suffixed with `Attr`. Would it be reasonable to suggest a guideline where in CIRGen we get more strict on not using `auto` but for other passes and transformations it should be fine to use auto on those? My rationale here is that post CIRGen it's very likely the reviwer audience is going to be more among MLIR experts than Clang experts and the former are more familiar with the idiom. > The big things for reviewers are: don't use `auto` if the type isn't > incredibly obvious (spelled in the initialization or extremely obvious based > on immediately surrounding code) and don't make us infer important parts the > declarator (if it's a `const Foo *` and `auto` makes sense for some reason, > spell it `const auto *` rather than `auto`). Agreed. Btw, I'm not advocating for plain `auto` here/everywhere. This is more about `auto` usage big picture. > And if a reviewer asks for something to be converted from `auto` to an > explicit type name, assume that means the code isn't as readable as it could > be and switch to a concrete type (we should not be arguing "I think this is > clear therefore you should also think it's clear" during review, that just > makes the review process painful for everyone involved). Agreed. If it makes the reviewer job easier we should just abide. > My thinking is: 1) (most important) the convention should be consistent with > other nearby code, whatever that convention happens to be. Ok, for camelBack, it seems like `clang/lib/CodeGen` is already adopting it in multiple places (unsure if it's intentional or not?), and looks incomplete because probably no one took the effort to make it uniform? The signal this gives me is that we should just do it (use camelBack) for CIRGen. 2) if there's not already a convention to follow and if it's code that lives in `clang/include/clang`, it should generally follow the LLVM style (it's an "external" interface) because those are the APIs that folks outside of CIR will be calling into. If it's code that lives in `clang/lib/`, following the MLIR style is less of a concern. Great & agreed, thanks for the clarification. > ... changing to the LLVM style is generally preferred over changing to the > MLIR style. One caveat to this is: avoid busywork but use your best judgement > on how we eventually get to a consistent use of the LLVM style. ... it's > probably best to just bite the bullet and switch to LLVM style even though > MLIR would be less effort. Does this all seem reasonable? Yep, thank you @AaronBallman for putting the time here to help us set the right approach looking fwd. To be more specific, I'd suggest: - `clang/include` -> LLVM - `lib/Driver/*` -> LLVM - `lib/FrontendTool` -> LLVM - `lib/CIR/FrontendAction` -> LLVM - `lib/CIR/CodeGen` -> MLIR - `lib/CIR/*` -> MLIR What do you think? https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
