[PATCH] D46501: OpenCL Header: Guard all half float usage based on cl_khr_fp16 extension

[Jerry Crunchtime via Phabricator via cfe-commits]

jerryct created this revision.
Herald added subscribers: cfe-commits, Anastasia, yaxunl.

Hi,

this patch guards all missed half float functions based on the availabiltiy of 
the OpenCL extension cl_khr_fp16.

Best regards,

Jerry


Repository:
  rC Clang

https://reviews.llvm.org/D46501

Files:
  lib/Headers/opencl-c.h

Index: lib/Headers/opencl-c.h
===
--- lib/Headers/opencl-c.h
+++ lib/Headers/opencl-c.h
@@ -11540,7 +11540,7 @@
  *
  * vstoren write sizeof (gentypen) bytes given by data to address (p + (offset * n)).
  *
- * The address computed as (p + (offset * n)) must be 
+ * The address computed as (p + (offset * n)) must be
  * 8-bit aligned if gentype is char, uchar;
  * 16-bit aligned if gentype is short, ushort, half;
  * 32-bit aligned if gentype is int, uint, float;
@@ -12093,6 +12093,7 @@
  * The read address computed as (p + offset)
  * must be 16-bit aligned.
  */
+#ifdef cl_khr_fp16
 float __ovld vload_half(size_t offset, const __constant half *p);
 #if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
 float __ovld vload_half(size_t offset, const half *p);
@@ -12101,6 +12102,7 @@
 float __ovld vload_half(size_t offset, const __local half *p);
 float __ovld vload_half(size_t offset, const __private half *p);
 #endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#endif //cl_khr_fp16
 
 /**
  * Read sizeof (halfn) bytes of data from address
@@ -12110,6 +12112,7 @@
  * value is returned. The read address computed
  * as (p + (offset * n)) must be 16-bit aligned.
  */
+#ifdef cl_khr_fp16
 float2 __ovld vload_half2(size_t offset, const __constant half *p);
 float3 __ovld vload_half3(size_t offset, const __constant half *p);
 float4 __ovld vload_half4(size_t offset, const __constant half *p);
@@ -12138,6 +12141,7 @@
 float8 __ovld vload_half8(size_t offset, const __private half *p);
 float16 __ovld vload_half16(size_t offset, const __private half *p);
 #endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#endif //cl_khr_fp16
 
 /**
  * The float value given by data is first
@@ -12150,6 +12154,7 @@
  * The default current rounding mode is round to
  * nearest even.
  */
+#ifdef cl_khr_fp16
 #if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
 void __ovld vstore_half(float data, size_t offset, half *p);
 void __ovld vstore_half_rte(float data, size_t offset, half *p);
@@ -12197,6 +12202,7 @@
 void __ovld vstore_half_rtn(double data, size_t offset, __private half *p);
 #endif //cl_khr_fp64
 #endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#endif //cl_khr_fp16
 
 /**
  * The floatn value given by data is converted to
@@ -12209,6 +12215,7 @@
  * The default current rounding mode is round to
  * nearest even.
  */
+#ifdef cl_khr_fp16
 #if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
 void __ovld vstore_half2(float2 data, size_t offset, half *p);
 void __ovld vstore_half3(float3 data, size_t offset, half *p);
@@ -12416,6 +12423,7 @@
 void __ovld vstore_half16_rtn(double16 data, size_t offset, __private half *p);
 #endif //cl_khr_fp64
 #endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#endif //cl_khr_fp16
 
 /**
  * For n = 1, 2, 4, 8 and 16 read sizeof (halfn)
@@ -12430,6 +12438,7 @@
  * The address computed as (p + (offset * 4))
  * must be aligned to sizeof (half) * 4 bytes.
  */
+#ifdef cl_khr_fp16
 float __ovld vloada_half(size_t offset, const __constant half *p);
 float2 __ovld vloada_half2(size_t offset, const __constant half *p);
 float3 __ovld vloada_half3(size_t offset, const __constant half *p);
@@ -12463,6 +12472,7 @@
 float8 __ovld vloada_half8(size_t offset, const __private half *p);
 float16 __ovld vloada_half16(size_t offset, const __private half *p);
 #endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#endif //cl_khr_fp16
 
 /**
  * The floatn value given by data is converted to
@@ -12480,6 +12490,7 @@
  * mode. The default current rounding mode is
  * round to nearest even.
  */
+#ifdef cl_khr_fp16
 #if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
 void __ovld vstorea_half(float data, size_t offset, half *p);
 void __ovld vstorea_half2(float2 data, size_t offset, half *p);
@@ -12766,6 +12777,7 @@
 void __ovld vstorea_half16_rtn(double16 data,size_t offset, __private half *p);
 #endif //cl_khr_fp64
 #endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#endif //cl_khr_fp16
 
 // OpenCL v1.1 s6.11.8, v1.2 s6.12.8, v2.0 s6.13.8 - Synchronization Functions
 
@@ -12888,7 +12900,7 @@
 cl_mem_fence_flags __ovld get_fence(const void *ptr);
 cl_mem_fence_flags __ovld get_fence(void *ptr);
 
-/** 
+/**
  * Builtin functions to_global, to_local, and to_private need to be declared as Clang builtin functions
  * and checked in Sema since they should be declared as
  *   addr gentype* to_addr (gentype*);
@@ -13773,7 +13785,7 @@
 // add/sub: atomic type argument can be uintptr_t/intptr_t, value type argument can be ptrdiff_t.
 // or/xor/and/min/max: atomic type argument can be intptr_t/uintptr_t, value type argument can be intptr_t/uintptr_t.
 
-#if defined(cl_khr_int64_base_atomics)

[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

[Eric Fiselier via Phabricator via cfe-commits]

EricWF updated this revision to Diff 145398.
EricWF added a comment.

Generalize `CXXRewrittenExpr` to contain only a "representation" of the 
original expression, and the fully checked rewritten expression. By 
"representation" i mean to imply that we can't actually build and check a full 
representation for the expression as written, because it need not be valid. For 
example:

  struct T { auto operator<=>(T const&) const = default; };
  T{} < T{};

Currently this patch builds a `BinaryOperator` node to represent the expression 
as written, regardless of whether the original expression would actually select 
a builtin comparison operator. A consequence of this pseudo-representation is 
that we can't effectively transform it in `TreeTransform.h` (currently we only 
transform the rewritten expression and not the representation of the original; 
there are some kinks I need to work out there).


https://reviews.llvm.org/D45680

Files:
  include/clang/AST/ExprCXX.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/Basic/StmtNodes.td
  include/clang/Sema/Overload.h
  include/clang/Sema/Sema.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/Expr.cpp
  lib/AST/ExprClassification.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/Sema/SemaExceptionSpec.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/CodeGenCXX/cxx2a-compare.cpp
  test/SemaCXX/compare-cxx2a.cpp

Index: test/SemaCXX/compare-cxx2a.cpp
===
--- test/SemaCXX/compare-cxx2a.cpp
+++ test/SemaCXX/compare-cxx2a.cpp
@@ -293,20 +293,21 @@
 
 template 
 struct Tag {};
-// expected-note@+1 {{candidate}}
-Tag<0> operator<=>(EnumA, EnumA) {
-  return {};
+std::strong_ordering operator<=>(EnumA, EnumA) {
+  return std::strong_ordering::equal;
 }
-Tag<1> operator<=>(EnumA, EnumB) {
-  return {};
+// expected-note@+1 {{candidate function}},
+std::strong_ordering operator<=>(EnumA a, EnumB b) {
+  return ((int)a <=> (int)b);
 }
 
 void test_enum_ovl_provided() {
   auto r1 = (EnumA::A <=> EnumA::A);
-  ASSERT_EXPR_TYPE(r1, Tag<0>);
+  ASSERT_EXPR_TYPE(r1, std::strong_ordering);
   auto r2 = (EnumA::A <=> EnumB::B);
-  ASSERT_EXPR_TYPE(r2, Tag<1>);
-  (void)(EnumB::B <=> EnumA::A); // expected-error {{invalid operands to binary expression ('EnumCompareTests::EnumB' and 'EnumCompareTests::EnumA')}}
+  ASSERT_EXPR_TYPE(r2, std::strong_ordering);
+  (void)(EnumB::B <=> EnumA::A); // OK, chooses reverse order synthesized candidate.
+  (void)(EnumB::B <=> EnumC::C); // expected-error {{invalid operands to binary expression ('EnumCompareTests::EnumB' and 'EnumCompareTests::EnumC')}}
 }
 
 void enum_float_test() {
@@ -422,3 +423,125 @@
 }
 
 } // namespace ComplexTest
+
+namespace TestRewritting {
+
+struct T {
+  int x;
+  // expected-note@+1 {{candidate}}
+  constexpr std::strong_ordering operator<=>(T y) const {
+return (x <=> y.x);
+  }
+};
+
+struct U {
+  int x;
+  // FIXME: This diagnostic is wrong-ish.
+  // expected-note@+1 {{candidate function not viable: requires single argument 'y', but 2 arguments were provided}}
+  constexpr std::strong_equality operator<=>(T y) const {
+if (x == y.x)
+  return std::strong_equality::equal;
+return std::strong_equality::nonequal;
+  }
+};
+
+struct X { int x; };
+struct Y { int x; };
+template 
+struct Tag {};
+// expected-note@+1 2 {{candidate}}
+Tag<0> operator<=>(X, Y) {
+  return {};
+}
+// expected-note@+1 2 {{candidate}}
+constexpr auto operator<=>(Y y, X x) {
+  return y.x <=> x.x;
+}
+
+void foo() {
+  T t{42};
+  T t2{0};
+  U u{101};
+  auto r1 = (t <=> u);
+  ASSERT_EXPR_TYPE(r1, std::strong_equality);
+  auto r2 = (t <=> t2);
+  ASSERT_EXPR_TYPE(r2, std::strong_ordering);
+
+  auto r3 = t == u;
+  ASSERT_EXPR_TYPE(r3, bool);
+
+  (void)(t < u); // expected-error {{invalid operands to binary expression ('TestRewritting::T' and 'TestRewritting::U')}}
+
+  constexpr X x{1};
+  constexpr Y y{2};
+  constexpr auto r4 = (y < x);
+  static_assert(r4 == false);
+  constexpr auto r5 = (x < y);
+  static_assert(r5 == true);
+}
+
+} // namespace TestRewritting
+
+// The implicit object parameter is not considered when performing partial
+// ordering. That makes the reverse synthesized candidates ambiguous with the
+// rewritten candidates if any of them resolve to a member function.
+namespace TestOvlMatchingIgnoresImplicitObject {
+struct U;
+// expected-note@+2 {{candidate}}
+struct T {
+  std::strong_ordering operator<=>(U const &RHS) const;
+};
+// expected-note@+2 {{candidate}}
+struct U {
+  std::strong_ordering operator<=>(T const &RHS) const;
+};
+
+void test() {
+  // expected-error@+1 {{use of overloaded operator '<' is ambiguous}}
+  (void)(T{} < U{});
+}
+
+} // namespace

?????? If I want to disable certain attributes, such as"swiftcall",isthere any way to do it now?

[???? via cfe-commits]

hi, Aaron
   
   Can I ask you some questions?Is there such a demand for me in the community? 
Does my code need to be submitted to the community?
   Thanks.
--  --
??: "Aaron Ballman";
: 2018??5??3??(??) 7:40
??: ""<772847...@qq.com>;
: "cfe-commits"; 
: Re: If I want to disable certain attributes, such as"swiftcall",isthere 
any way to do it now?



On Thu, May 3, 2018 at 1:25 AM,  <772847...@qq.com> wrote:
> hi, Aaron
>
>   The reason why i not use of TargetSpecificAttr is as follows.If I plan to
> support a new platform, I don't want to let users use certain attributes
> because of hardware or simply not want to. Yes, we can use
> TargetSpecificAttr, but if we use TargetSpecificAttr that we need to include
> platforms other than unsupported platforms, but we just need to exclude that
> platform, using TargetSpecificAttr may not be a good idea.

Okay, that makes sense to me. Your design seems like a reasonable one.

~Aaron

>
>
>
> --  --
> ??: "Aaron Ballman";
> : 2018??5??2??(??) 10:05
> ??: ""<772847...@qq.com>;
> : "cfe-commits";
> : Re: If I want to disable certain attributes, such as "swiftcall",isthere
> any way to do it now?
>
> On Wed, May 2, 2018 at 4:59 AM,  <772847...@qq.com> wrote:
>> Thanks.
>>
>> If I remove some specific attributes, now I can think of the following
>> method.First of all, let me talk about my method.
>>
>> 1.Define target platforms that do not support attributes.such as def
>> TargetPower : TargetArch<["ppc", "ppc64", "ppc64le"]>;
>>
>> 2.Define TargetNotSupportedAttr class.such as class
>> TargetNotSupportedAttr ;
>
> Why not make use of TargetSpecificAttr to mark the attributes that are
> specific to a target rather than marking the attributes not supported
> by a target?
>
>> 3.Using this approach, we need to modify the
>> cfe-4.0.1.src/utils/TableGen/ClangAttrEmitter.cpp file.Modifying the code
>> in
>> the GenerateHasAttrSpellingStringSwitch function looks like this:
>>
>> if(Attr->isSubClassOf("TargetNotSupportedAttr")) {   // add
>>
>> #define GenerateTargetNotSupportedAttrChecks
>> GenerateTargetSpecificAttrChecks // add
>>
>> const Record *R = Attr->getValueAsDef("Target");   // add
>>
>> std::vector Arches =
>> R->getValueAsListOfStrings("Arches"); // add
>>
>> GenerateTargetNotSupportedAttrChecks(R, Arches, Test, nullptr);
>> // add
>>
>> TestStr = !Test.empty() ? Test + " ? "  + " 0 :" +
>> llvm::itostr(Version) : "0"; // add
>>
>> } else if (Attr->isSubClassOf("TargetSpecificAttr"))
>>
>> 4.And for classes that inherit from TargetNotSupportedAttr<> class, we
>> don??t
>> need to generate the attribute class, so add the following code in
>> EmitClangAttrClass
>>
>> if (R->getName() != "TargetSpecificAttr" &&
>>
>>   R->getName() != "TargetNotSupportedAttr?? &&  //add
>>
>>   SuperName.empty())
>>
>> SuperName = R->getName();
>>
>>
>> If I use this method, is it correct?
>
> It seems like it would work, yes.
>
> ~Aaron
>
>>
>>
>>
>>
>> --  --
>> ??: "Aaron Ballman";
>> : 2018??4??29??(??) 3:07
>> ??: ""<772847...@qq.com>;
>> : "cfe-commits";
>> : Re: If I want to disable certain attributes, such as "swiftcall",
>> isthere any way to do it now?
>>
>> On Fri, Apr 27, 2018 at 11:23 PM,  via cfe-commits
>>  wrote:
>>>As the title says,thanks.
>>
>> You could build a custom clang with specific attributes removed, but
>> there are no compiler flags that allow you to disable arbitrary
>> attributes like swiftcall.
>>
>> ~Aaron___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: If I want to disable certain attributes, such as"swiftcall", isthere any way to do it now?

[Aaron Ballman via cfe-commits]

On Sun, May 6, 2018 at 10:01 AM, 朴素 <772847...@qq.com> wrote:
> hi, Aaron
>
>Can I ask you some questions?Is there such a demand for me in the
> community? Does my code need to be submitted to the community?
>Thanks.

Generally speaking, the community will accept production-quality
patches that solve a problem with the product or introduce some new,
desirable functionality. What you've described so far sounds like it
solves a specific problem you're having, but it doesn't sound like it
solves a problem the general public is having. Perhaps if you
described how you envision your solution being used in the product and
what immediate problems it solves, that would help me to understand
the situation a bit better. Basically, if you want to propose adding
TargetNotSupportedAttr, can you describe what attributes you would
propose using it on?

Thanks!

~Aaron


> -- 原始邮件 --
> 发件人: "Aaron Ballman";
> 发送时间: 2018年5月3日(星期四) 晚上7:40
> 收件人: "朴素"<772847...@qq.com>;
> 抄送: "cfe-commits";
> 主题: Re: If I want to disable certain attributes, such as"swiftcall",isthere
> any way to do it now?
>
> On Thu, May 3, 2018 at 1:25 AM, 朴素 <772847...@qq.com> wrote:
>> hi, Aaron
>>
>>   The reason why i not use of TargetSpecificAttr is as follows.If I plan
>> to
>> support a new platform, I don't want to let users use certain attributes
>> because of hardware or simply not want to. Yes, we can use
>> TargetSpecificAttr, but if we use TargetSpecificAttr that we need to
>> include
>> platforms other than unsupported platforms, but we just need to exclude
>> that
>> platform, using TargetSpecificAttr may not be a good idea.
>
> Okay, that makes sense to me. Your design seems like a reasonable one.
>
> ~Aaron
>
>>
>>
>>
>> -- 原始邮件 --
>> 发件人: "Aaron Ballman";
>> 发送时间: 2018年5月2日(星期三) 晚上10:05
>> 收件人: "朴素"<772847...@qq.com>;
>> 抄送: "cfe-commits";
>> 主题: Re: If I want to disable certain attributes, such as
>> "swiftcall",isthere
>> any way to do it now?
>>
>> On Wed, May 2, 2018 at 4:59 AM, 朴素 <772847...@qq.com> wrote:
>>> Thanks.
>>>
>>> If I remove some specific attributes, now I can think of the following
>>> method.First of all, let me talk about my method.
>>>
>>> 1.Define target platforms that do not support attributes.such as def
>>> TargetPower : TargetArch<["ppc", "ppc64", "ppc64le"]>;
>>>
>>> 2.Define TargetNotSupportedAttr class.such as class
>>> TargetNotSupportedAttr ;
>>
>> Why not make use of TargetSpecificAttr to mark the attributes that are
>> specific to a target rather than marking the attributes not supported
>> by a target?
>>
>>> 3.Using this approach, we need to modify the
>>> cfe-4.0.1.src/utils/TableGen/ClangAttrEmitter.cpp file.Modifying the code
>>> in
>>> the GenerateHasAttrSpellingStringSwitch function looks like this:
>>>
>>> if(Attr->isSubClassOf("TargetNotSupportedAttr")) {   // add
>>>
>>> #define GenerateTargetNotSupportedAttrChecks
>>> GenerateTargetSpecificAttrChecks // add
>>>
>>> const Record *R = Attr->getValueAsDef("Target");   // add
>>>
>>> std::vector Arches =
>>> R->getValueAsListOfStrings("Arches"); // add
>>>
>>> GenerateTargetNotSupportedAttrChecks(R, Arches, Test, nullptr);
>>> // add
>>>
>>> TestStr = !Test.empty() ? Test + " ? "  + " 0 :" +
>>> llvm::itostr(Version) : "0"; // add
>>>
>>> } else if (Attr->isSubClassOf("TargetSpecificAttr"))
>>>
>>> 4.And for classes that inherit from TargetNotSupportedAttr<> class, we
>>> don’t
>>> need to generate the attribute class, so add the following code in
>>> EmitClangAttrClass
>>>
>>> if (R->getName() != "TargetSpecificAttr" &&
>>>
>>>   R->getName() != "TargetNotSupportedAttr” &&  //add
>>>
>>>   SuperName.empty())
>>>
>>> SuperName = R->getName();
>>>
>>>
>>> If I use this method, is it correct?
>>
>> It seems like it would work, yes.
>>
>> ~Aaron
>>
>>>
>>>
>>>
>>>
>>> -- 原始邮件 --
>>> 发件人: "Aaron Ballman";
>>> 发送时间: 2018年4月29日(星期天) 凌晨3:07
>>> 收件人: "朴素"<772847...@qq.com>;
>>> 抄送: "cfe-commits";
>>> 主题: Re: If I want to disable certain attributes, such as "swiftcall",
>>> isthere any way to do it now?
>>>
>>> On Fri, Apr 27, 2018 at 11:23 PM, 朴素 via cfe-commits
>>>  wrote:
As the title says,thanks.
>>>
>>> You could build a custom clang with specific attributes removed, but
>>> there are no compiler flags that allow you to disable arbitrary
>>> attributes like swiftcall.
>>>
>>> ~Aaron
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24867: Request init/fini array on FreeBSD 12 and later

[Brooks Davis via Phabricator via cfe-commits]

brooks added a reviewer: arichardson.
brooks added a comment.
Herald added a subscriber: krytarowski.

Adding Alex as he made some related changes to CHERI Clang recently 
https://github.com/CTSRD-CHERI/clang/commit/3a648766deabb4ff7f95862213c3c99e7223363c.


https://reviews.llvm.org/D24867



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46504: [clang-tidy] Profile is a per-AST (per-TU) data.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri created this revision.
lebedev.ri added reviewers: alexfh, sbenza.
lebedev.ri added a project: clang-tools-extra.
Herald added subscribers: mgrang, xazax.hun, mgorny.

As discussed in https://reviews.llvm.org/D45931, currently, profiling output of 
clang-tidy is somewhat not great.
It outputs one profile at the end of the execution, and that profile contains 
the data
from the last TU that was processed. So if the tool run on multiple TU's, the 
data is
not accumulated, it is simply discarded.

It would be nice to improve this.

This differential is the first step - make this profiling info per-TU,
and output it after the tool has finished processing each TU.
In particular, when `ClangTidyASTConsumer` destructor runs.

Next step will be to add a CSV (JSON?) printer to store said profiles under 
user-specified directory prefix.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46504

Files:
  clang-tidy/CMakeLists.txt
  clang-tidy/ClangTidy.cpp
  clang-tidy/ClangTidy.h
  clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tidy/ClangTidyProfiling.cpp
  clang-tidy/ClangTidyProfiling.h
  clang-tidy/tool/ClangTidyMain.cpp
  test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp
  test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp

Index: test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp
===
--- /dev/null
+++ test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp
@@ -0,0 +1,19 @@
+// RUN: clang-tidy -enable-check-profile -checks='-*,readability-function-size' %s %s 2>&1 | FileCheck --match-full-lines -implicit-check-not='{{warning:|error:}}' %s
+
+// CHECK: Running without flags.
+// CHECK-NEXT: ===-===
+// CHECK-NEXT: {{.*}}  --- Name ---
+// CHECK-NEXT: {{.*}}  readability-function-size
+// CHECK-NEXT: {{.*}}  Total
+// CHECK-NEXT: ===-===
+
+// CHECK: ===-===
+// CHECK-NEXT: {{.*}}  --- Name ---
+// CHECK-NEXT: {{.*}}  readability-function-size
+// CHECK-NEXT: {{.*}}  Total
+// CHECK-NEXT: ===-===
+
+class A {
+  A() {}
+  ~A() {}
+};
Index: test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp
===
--- /dev/null
+++ test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp
@@ -0,0 +1,13 @@
+// RUN: clang-tidy -enable-check-profile -checks='-*,readability-function-size' %s 2>&1 | FileCheck --match-full-lines -implicit-check-not='{{warning:|error:}}' %s
+
+// CHECK: Running without flags.
+// CHECK-NEXT: ===-===
+// CHECK-NEXT: {{.*}}  --- Name ---
+// CHECK-NEXT: {{.*}}  readability-function-size
+// CHECK-NEXT: {{.*}}  Total
+// CHECK-NEXT: ===-===
+
+class A {
+  A() {}
+  ~A() {}
+};
Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -236,45 +236,6 @@
   }
 }
 
-static void printProfileData(const ProfileData &Profile,
- llvm::raw_ostream &OS) {
-  // Time is first to allow for sorting by it.
-  std::vector> Timers;
-  TimeRecord Total;
-
-  for (const auto &P : Profile.Records) {
-Timers.emplace_back(P.getValue(), P.getKey());
-Total += P.getValue();
-  }
-
-  std::sort(Timers.begin(), Timers.end());
-
-  std::string Line = "===" + std::string(73, '-') + "===\n";
-  OS << Line;
-
-  if (Total.getUserTime())
-OS << "   ---User Time---";
-  if (Total.getSystemTime())
-OS << "   --System Time--";
-  if (Total.getProcessTime())
-OS << "   --User+System--";
-  OS << "   ---Wall Time---";
-  if (Total.getMemUsed())
-OS << "  ---Mem---";
-  OS << "  --- Name ---\n";
-
-  // Loop through all of the timing data, printing it out.
-  for (auto I = Timers.rbegin(), E = Timers.rend(); I != E; ++I) {
-I->first.print(Total, OS);
-OS << I->second << '\n';
-  }
-
-  Total.print(Total, OS);
-  OS << "Total\n";
-  OS << Line << "\n";
-  OS.flush();
-}
-
 static std::unique_ptr createOptionsProvider(
llvm::IntrusiveRefCntPtr FS) {
   ClangTidyGlobalOptions GlobalOptions;
@@ -424,15 +385,14 @@
 llvm::cl::PrintHelpMessage(/*Hidden=*/false, /*Categorized=*/true);
 return 1;
   }
-  ProfileData Profile;
 
   llvm::InitializeAllTargetInfos();
   llvm::InitializeAllTargetMCs();
   llvm::InitializeAllAsmParsers();
 
   ClangTidyContext Context(std::move(OwningOptionsProvider));
   runClangTidy(Context, OptionsParser.getCompilations(), PathList, BaseFS,
-   EnableCheckPr

[PATCH] D45927: [clang-tidy] [modernize-use-auto] Correct way to calculate a type name length for multi-token types

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis added inline comments.



Comment at: clang-tidy/modernize-use-auto-min-type-name-length.cpp:61-83
+long int li = static_cast(foo());
+// CHECK-FIXES-0-0: auto li = {{.*}}
+// CHECK-FIXES-0-5: auto li = {{.*}}
+// CHECK-FIXES-1-0: auto  li = {{.*}}
+// CHECK-FIXES-1-5: auto  li = {{.*}}
+long int *pli = static_cast(foo());
+// CHECK-FIXES-0-0: auto *pli = {{.*}}

zinovy.nis wrote:
> alexfh wrote:
> > These tests could be more useful, if they verified boundary values of the 
> > MinTypeNameLength, e.g. that `long int` is replaced with `auto` when the 
> > option is set to 8 and it stays `long int`with the option set to 9.
> > 
> > Please also add tests with template typenames: e.g. the lenght of `T <  int 
> >  >` should be considered 6 and the length of `T  <  const int >` is 12. I 
> > guess, the algorithm I proposed will be incorrect for pointer type 
> > arguments of templates (the length of `T` should be 7 regardless of 
> > `RemoveStars`), but this can be fixed by conditionally (on RemoveStars) 
> > trimming `*`s from the right of the type name and then treating them as 
> > punctuation. So instead of
> > 
> > ```
> >   for (const unsigned char C : tooling::fixit::getText(SR, Context)) {
> > const CharType NextChar =
> > std::isalnum(C)
> > ? Alpha
> > : (std::isspace(C) || (!RemoveStars && C == '*')) ? Space
> >   : Punctuation;
> > ```
> > 
> > you could use something similar to
> > 
> > ```
> >   StringRef Text = tooling::fixit::getText(SR, Context);
> >   if (RemoveStars)
> > Text = Text.rtrim(" \t\v\n\r*");
> >   for (const unsigned char C : Text) {
> > const CharType NextChar =
> > std::isalnum(C) ? Alpha : std::isspace(C) ? Space : Punctuation;
> > ```
> > These tests could be more useful, if they verified boundary values of the 
> > MinTypeNameLength, e.g. that long int is replaced with auto when the option 
> > is set to 8 and it stays long intwith the option set to 9.
> > 
> 
> `int`-test is just for that :-)
> 
> ```
> int a = static_cast(foo()); // ---> int  a = ...
> ```
I measured lengths for template cases:


```
S=std::string *   len=12
S=std::vector *   len=25
S=std::vector *   len=31
S=std::string*   len=12
S=std::vector   *   len=25
S=std::vector*   len=31
```




https://reviews.llvm.org/D45927



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46112: Allow _Atomic to be specified on incomplete types

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 145409.
aaron.ballman added a comment.

Hopefully address the review comments.


https://reviews.llvm.org/D46112

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaType.cpp
  test/CodeGen/c11atomics.c
  test/Sema/atomic-type.c
  test/Sema/atomic-type.cpp

Index: test/Sema/atomic-type.cpp
===
--- test/Sema/atomic-type.cpp
+++ test/Sema/atomic-type.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+struct S;
+
+void f(_Atomic S *s);
+
+struct S { int a, b; };
+
+void f(_Atomic S *s) {
+  S s2;
+  (void)__atomic_load(s, &s2, 5); // expected-error {{address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(S) *' invalid)}}
+  (void)__c11_atomic_load(s, 5);
+}
+
Index: test/Sema/atomic-type.c
===
--- test/Sema/atomic-type.c
+++ test/Sema/atomic-type.c
@@ -16,7 +16,17 @@
 extern _Atomic(int (*)(int(*)[10], int(*)[10])) mergetest;
 
 _Atomic(int()) error1; // expected-error {{_Atomic cannot be applied to function type}}
-_Atomic(struct ErrorS) error2; // expected-error {{_Atomic cannot be applied to incomplete type}} expected-note {{forward declaration}}
+_Atomic(struct ErrorS) error2;
 _Atomic(int[10]) error3; // expected-error {{_Atomic cannot be applied to array type}}
 _Atomic(const int) error4; // expected-error {{_Atomic cannot be applied to qualified type}}
 _Atomic(_Atomic(int)) error5; // expected-error {{_Atomic cannot be applied to atomic type}}
+
+void g(_Atomic void *ptr);
+void h(_Atomic struct Incomplete *ptr); // expected-warning {{declaration of 'struct Incomplete' will not be visible outside of this function}}
+
+void test_atomic_void_ptr(_Atomic void *addr) {
+  int i;
+  (void)__atomic_load(addr, &i, 5); // expected-error {{address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(void) *' invalid)}}
+  (void)__c11_atomic_load(addr, 5); // expected-error {{invalid use of incomplete type 'void'}}
+  _Static_assert(__atomic_is_lock_free(1, addr), "");
+}
Index: test/CodeGen/c11atomics.c
===
--- test/CodeGen/c11atomics.c
+++ test/CodeGen/c11atomics.c
@@ -474,3 +474,20 @@
   // CHECK:   ret i1 [[RES]]
   return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
 }
+
+int test_atomic_void_ptr(_Atomic void *addr) {
+  // CHECK-LABEL: @test_atomic_void_ptr(i8* %addr)
+  // CHECK:   [[ADDR_ARG:%.*]] = alloca i8*, align 4
+  // CHECK:   [[ATOMIC_RES:%.*]] = alloca i32, align 4
+  // CHECK:   store i8* %addr, i8** [[ADDR_ARG]], align 4
+  // CHECK:   [[ADDR_LOCK_FREE:%.*]] = load i8*, i8** [[ADDR_ARG]], align 4
+  // CHECK:   call arm_aapcscc i32 @__atomic_is_lock_free(i32 1, i8* [[ADDR_LOCK_FREE]])
+  // CHECK:   [[ADDR:%.*]] = load i8*, i8** [[ADDR_ARG]], align 4
+  // CHECK:   [[CAST1:%.*]] = bitcast i8* [[ADDR]] to i32*
+  // CHECK:   [[CAST2:%.*]] = bitcast i32* [[CAST1]] to i8*
+  // CHECK:   call arm_aapcscc i32 @__atomic_load_4(i8* [[CAST2]], i32 5)
+  // CHECK:   [[RES:%.*]] = load i32, i32* [[ATOMIC_RES]], align 4
+  // CHECK:   ret i32 [[RES]]
+  (void)__atomic_is_lock_free(1, addr);
+  return (int)__c11_atomic_load((_Atomic int *)addr, 5);
+}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -8019,25 +8019,21 @@
 
 QualType Sema::BuildAtomicType(QualType T, SourceLocation Loc) {
   if (!T->isDependentType()) {
-// FIXME: It isn't entirely clear whether incomplete atomic types
-// are allowed or not; for simplicity, ban them for the moment.
-if (RequireCompleteType(Loc, T, diag::err_atomic_specifier_bad_type, 0))
-  return QualType();
-
 int DisallowedKind = -1;
 if (T->isArrayType())
-  DisallowedKind = 1;
+  DisallowedKind = 0;
 else if (T->isFunctionType())
-  DisallowedKind = 2;
+  DisallowedKind = 1;
 else if (T->isReferenceType())
-  DisallowedKind = 3;
+  DisallowedKind = 2;
 else if (T->isAtomicType())
-  DisallowedKind = 4;
+  DisallowedKind = 3;
 else if (T.hasQualifiers())
+  DisallowedKind = 4;
+else if (isCompleteType(Loc, T) && !T.isTriviallyCopyableType(Context))
+  // Some other non-trivially-copyable type (probably a C++ class, or a C
+  // struct with a __weak or __strong field).
   DisallowedKind = 5;
-else if (!T.isTriviallyCopyableType(Context))
-  // Some other non-trivially-copyable type (probably a C++ class)
-  DisallowedKind = 6;
 
 if (DisallowedKind != -1) {
   Diag(Loc, diag::err_atomic_specifier_bad_type) << DisallowedKind << T;
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaC

[PATCH] D46112: Allow _Atomic to be specified on incomplete types

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/Sema/SemaType.cpp:8022
+else if (LangOpts.CPlusPlus && isCompleteType(Loc, T) &&
+ !T.isTriviallyCopyableType(Context))
   // Some other non-trivially-copyable type (probably a C++ class)

rsmith wrote:
> efriedma wrote:
> > If you're going to allow incomplete types in C++, you'll need some code to 
> > handle the case where the type is initially incomplete, but completed later.
> Perhaps the trivially-copyable requirement could be deferred in all cases 
> until we reach an actual load or store. Constraints that are only enforced 
> when a type happens to be complete are generally a bad idea (we have a mess 
> like this for constraints on types being non-abstract, and we're still 
> working on unsticking ourselves from that mess in the C++ standard).
> 
> In passing, I'd also note that we need the trivially-copyable requirement 
> even outside C++, for structs containing `__weak` / `__strong` fields.
I'm not quite certain I've properly captured the request here, but I took a 
stab at it. One thing to note is that our behavior for type checking 
`__atomic_load()` appears to diverge from GCC with regards to trivial 
copyability, even before my patch.

https://godbolt.org/g/g2fqLK


https://reviews.llvm.org/D46112



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45927: [clang-tidy] [modernize-use-auto] Correct way to calculate a type name length for multi-token types

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis added inline comments.



Comment at: clang-tidy/modernize-use-auto-min-type-name-length.cpp:61-83
+long int li = static_cast(foo());
+// CHECK-FIXES-0-0: auto li = {{.*}}
+// CHECK-FIXES-0-5: auto li = {{.*}}
+// CHECK-FIXES-1-0: auto  li = {{.*}}
+// CHECK-FIXES-1-5: auto  li = {{.*}}
+long int *pli = static_cast(foo());
+// CHECK-FIXES-0-0: auto *pli = {{.*}}

zinovy.nis wrote:
> zinovy.nis wrote:
> > alexfh wrote:
> > > These tests could be more useful, if they verified boundary values of the 
> > > MinTypeNameLength, e.g. that `long int` is replaced with `auto` when the 
> > > option is set to 8 and it stays `long int`with the option set to 9.
> > > 
> > > Please also add tests with template typenames: e.g. the lenght of `T <  
> > > int  >` should be considered 6 and the length of `T  <  const int >` is 
> > > 12. I guess, the algorithm I proposed will be incorrect for pointer type 
> > > arguments of templates (the length of `T` should be 7 regardless of 
> > > `RemoveStars`), but this can be fixed by conditionally (on RemoveStars) 
> > > trimming `*`s from the right of the type name and then treating them as 
> > > punctuation. So instead of
> > > 
> > > ```
> > >   for (const unsigned char C : tooling::fixit::getText(SR, Context)) {
> > > const CharType NextChar =
> > > std::isalnum(C)
> > > ? Alpha
> > > : (std::isspace(C) || (!RemoveStars && C == '*')) ? Space
> > >   : 
> > > Punctuation;
> > > ```
> > > 
> > > you could use something similar to
> > > 
> > > ```
> > >   StringRef Text = tooling::fixit::getText(SR, Context);
> > >   if (RemoveStars)
> > > Text = Text.rtrim(" \t\v\n\r*");
> > >   for (const unsigned char C : Text) {
> > > const CharType NextChar =
> > > std::isalnum(C) ? Alpha : std::isspace(C) ? Space : Punctuation;
> > > ```
> > > These tests could be more useful, if they verified boundary values of the 
> > > MinTypeNameLength, e.g. that long int is replaced with auto when the 
> > > option is set to 8 and it stays long intwith the option set to 9.
> > > 
> > 
> > `int`-test is just for that :-)
> > 
> > ```
> > int a = static_cast(foo()); // ---> int  a = ...
> > ```
> I measured lengths for template cases:
> 
> 
> ```
> S=std::string *   len=12
> S=std::vector *   len=25
> S=std::vector *   len=31
> S=std::string*   len=12
> S=std::vector   *   len=25
> S=std::vector*   len=31
> ```
> 
> 
RemoveStars==1 here.


https://reviews.llvm.org/D45927



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46382: [OpenCL] Factor out language version printing

[Vedran Miletić via Phabricator via cfe-commits]

rivanvx accepted this revision.
rivanvx added a comment.
This revision is now accepted and ready to land.

Nice cleanup.


Repository:
  rC Clang

https://reviews.llvm.org/D46382



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

[Eric Fiselier via Phabricator via cfe-commits]

EricWF updated this revision to Diff 145419.
EricWF added a comment.

- Further generalize `CXXRewrittenExpr` and get it working with 
`TreeTransform.h`.


https://reviews.llvm.org/D45680

Files:
  include/clang/AST/Expr.h
  include/clang/AST/ExprCXX.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/AST/Stmt.h
  include/clang/Basic/StmtNodes.td
  include/clang/Sema/Overload.h
  include/clang/Sema/Sema.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/Expr.cpp
  lib/AST/ExprClassification.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/Sema/SemaExceptionSpec.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/CodeGenCXX/cxx2a-compare.cpp
  test/SemaCXX/compare-cxx2a.cpp

Index: test/SemaCXX/compare-cxx2a.cpp
===
--- test/SemaCXX/compare-cxx2a.cpp
+++ test/SemaCXX/compare-cxx2a.cpp
@@ -293,20 +293,21 @@
 
 template 
 struct Tag {};
-// expected-note@+1 {{candidate}}
-Tag<0> operator<=>(EnumA, EnumA) {
-  return {};
+std::strong_ordering operator<=>(EnumA, EnumA) {
+  return std::strong_ordering::equal;
 }
-Tag<1> operator<=>(EnumA, EnumB) {
-  return {};
+// expected-note@+1 {{candidate function}},
+std::strong_ordering operator<=>(EnumA a, EnumB b) {
+  return ((int)a <=> (int)b);
 }
 
 void test_enum_ovl_provided() {
   auto r1 = (EnumA::A <=> EnumA::A);
-  ASSERT_EXPR_TYPE(r1, Tag<0>);
+  ASSERT_EXPR_TYPE(r1, std::strong_ordering);
   auto r2 = (EnumA::A <=> EnumB::B);
-  ASSERT_EXPR_TYPE(r2, Tag<1>);
-  (void)(EnumB::B <=> EnumA::A); // expected-error {{invalid operands to binary expression ('EnumCompareTests::EnumB' and 'EnumCompareTests::EnumA')}}
+  ASSERT_EXPR_TYPE(r2, std::strong_ordering);
+  (void)(EnumB::B <=> EnumA::A); // OK, chooses reverse order synthesized candidate.
+  (void)(EnumB::B <=> EnumC::C); // expected-error {{invalid operands to binary expression ('EnumCompareTests::EnumB' and 'EnumCompareTests::EnumC')}}
 }
 
 void enum_float_test() {
@@ -422,3 +423,116 @@
 }
 
 } // namespace ComplexTest
+
+namespace TestRewritting {
+
+struct T {
+  int x;
+  // expected-note@+1 {{candidate}}
+  constexpr std::strong_ordering operator<=>(T y) const {
+return (x <=> y.x);
+  }
+};
+
+struct U {
+  int x;
+  // FIXME: This diagnostic is wrong-ish.
+  // expected-note@+1 {{candidate function not viable: requires single argument 'y', but 2 arguments were provided}}
+  constexpr std::strong_equality operator<=>(T y) const {
+if (x == y.x)
+  return std::strong_equality::equal;
+return std::strong_equality::nonequal;
+  }
+};
+
+struct X { int x; };
+struct Y { int x; };
+template 
+struct Tag {};
+// expected-note@+1 2 {{candidate}}
+Tag<0> operator<=>(X, Y) {
+  return {};
+}
+// expected-note@+1 2 {{candidate}}
+constexpr auto operator<=>(Y y, X x) {
+  return y.x <=> x.x;
+}
+
+void foo() {
+  T t{42};
+  T t2{0};
+  U u{101};
+  auto r1 = (t <=> u);
+  ASSERT_EXPR_TYPE(r1, std::strong_equality);
+  auto r2 = (t <=> t2);
+  ASSERT_EXPR_TYPE(r2, std::strong_ordering);
+
+  auto r3 = t == u;
+  ASSERT_EXPR_TYPE(r3, bool);
+
+  (void)(t < u); // expected-error {{invalid operands to binary expression ('TestRewritting::T' and 'TestRewritting::U')}}
+
+  constexpr X x{1};
+  constexpr Y y{2};
+  constexpr auto r4 = (y < x);
+  static_assert(r4 == false);
+  constexpr auto r5 = (x < y);
+  static_assert(r5 == true);
+}
+
+} // namespace TestRewritting
+
+// The implicit object parameter is not considered when performing partial
+// ordering. That makes the reverse synthesized candidates ambiguous with the
+// rewritten candidates if any of them resolve to a member function.
+namespace TestOvlMatchingIgnoresImplicitObject {
+struct U;
+// expected-note@+2 {{candidate}}
+struct T {
+  std::strong_ordering operator<=>(U const &RHS) const;
+};
+// expected-note@+2 {{candidate}}
+struct U {
+  std::strong_ordering operator<=>(T const &RHS) const;
+};
+
+struct V {
+  int x;
+};
+auto operator<=>(V const &LHS, V &&RHS) { // expected-note 4 {{candidate}}
+  return LHS.x <=> RHS.x;
+}
+auto operator<(V const &, V &&) { // expected-note {{candidate}}
+  return std::strong_equality::equal;
+}
+
+void test() {
+  // expected-error@+1 {{use of overloaded operator '<' is ambiguous}}
+  (void)(T{} < U{});
+  // expected-error@+1 {{use of overloaded operator '<' is ambiguous}}
+  (void)(V{} < V{});
+  // expected-error@+1 {{use of overloaded operator '<=>' is ambiguous}}
+  (void)(V{} <=> V{});
+}
+
+} // namespace TestOvlMatchingIgnoresImplicitObject
+
+namespace TestRewrittenTemplate {
+
+template 
+auto test(T const &LHS, T const &RHS) {
+  // expected-error@+1 {{invalid operands to binary expression ('const TestRewrittenTemplate::None'}}
+  return LHS < RHS;
+}
+struct 

[PATCH] D45927: [clang-tidy] [modernize-use-auto] Correct way to calculate a type name length for multi-token types

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: modernize/UseAutoCheck.cpp:40
+? Alpha
+: (std::isspace(C) || (!RemoveStars && C == '*')) ? Space
+  : Punctuation;

`isspace` here is wrong: it's a locale-sensitive check. Clang assumes UTF-8 
throughout; you can use `isWhitespace` from clang/Basic/CharInfo.h to test if 
the character should be treated as whitespace.


https://reviews.llvm.org/D45927



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46504: [clang-tidy] Profile is a per-AST (per-TU) data.

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:181
 std::unique_ptr OptionsProvider)
-: DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)),
-  Profile(nullptr) {
+: DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)) {
   // Before the first translation unit we can get errors related to 
command-line

Will be good idea to use default member initialization for DiagEngine too.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46504



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r331620 - Non-zero-length bit-fields make a class non-empty.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Sun May  6 23:43:30 2018
New Revision: 331620

URL: http://llvm.org/viewvc/llvm-project?rev=331620&view=rev
Log:
Non-zero-length bit-fields make a class non-empty.

This implements the rule intended by the standard (see LWG 2358)
and the rule intended by the Itanium C++ ABI (see
https://github.com/itanium-cxx-abi/cxx-abi/pull/51), and makes
Clang match the behavior of GCC, ICC, and MSVC.

A pedantic reading of both the standard and the ABI indicate that Clang
is currently technically correct, but that's not worth much when it's
clear that the wording is wrong in both those places.

This is an ABI break for classes that derive from a class that is empty
other than one or more unnamed non-zero-length bit-fields. Such cases
are expected to be rare, but -fclang-abi-compat=6 restores the old
behavior just in case.

Differential Revision: https://reviews.llvm.org/D45174

Added:
cfe/trunk/test/Layout/v6-empty.cpp
Modified:
cfe/trunk/docs/ReleaseNotes.rst
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp
cfe/trunk/test/SemaCXX/type-traits.cpp

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=331620&r1=331619&r2=331620&view=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Sun May  6 23:43:30 2018
@@ -84,6 +84,15 @@ Non-comprehensive list of changes in thi
   Clang 7 and earlier versions. The old behavior can be restored by setting
   ``-fclang-abi-compat`` to ``6`` or earlier.
 
+- Clang implements the proposed resolution of LWG issue 2358, along with the
+  `corresponding change to the Itanium C++ ABI
+  `_, which make classes
+  containing only unnamed non-zero-length bit-fields be considered non-empty.
+  This is an ABI break compared to prior Clang releases, but makes Clang
+  generate code that is ABI-compatible with other compilers. The old
+  behavior can be restored by setting ``-fclang-abi-compat`` to ``6`` or
+  lower.
+
 - ...
 
 New Compiler Flags

Modified: cfe/trunk/lib/AST/DeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=331620&r1=331619&r2=331620&view=diff
==
--- cfe/trunk/lib/AST/DeclCXX.cpp (original)
+++ cfe/trunk/lib/AST/DeclCXX.cpp Sun May  6 23:43:30 2018
@@ -856,10 +856,10 @@ void CXXRecordDecl::addedMember(Decl *D)
 return;
   }
 
-  ASTContext &Context = getASTContext();
-
   // Handle non-static data members.
   if (const auto *Field = dyn_cast(D)) {
+ASTContext &Context = getASTContext();
+
 // C++2a [class]p7:
 //   A standard-layout class is a class that:
 //[...]
@@ -872,8 +872,16 @@ void CXXRecordDecl::addedMember(Decl *D)
 //   A declaration for a bit-field that omits the identifier declares an 
 //   unnamed bit-field. Unnamed bit-fields are not members and cannot be 
 //   initialized.
-if (Field->isUnnamedBitfield())
+if (Field->isUnnamedBitfield()) {
+  // C++ [meta.unary.prop]p4: [LWG2358]
+  //   T is a class type [...] with [...] no unnamed bit-fields of non-zero
+  //   length
+  if (data().Empty && !Field->isZeroLengthBitField(Context) &&
+  Context.getLangOpts().getClangABICompat() >
+  LangOptions::ClangABI::Ver6)
+data().Empty = false;
   return;
+}
 
 // C++11 [class]p7:
 //   A standard-layout class is a class that:
@@ -1220,12 +1228,8 @@ void CXXRecordDecl::addedMember(Decl *D)
 }
 
 // C++14 [meta.unary.prop]p4:
-//   T is a class type [...] with [...] no non-static data members other
-//   than bit-fields of length 0...
-if (data().Empty) {
-  if (!Field->isZeroLengthBitField(Context))
-data().Empty = false;
-}
+//   T is a class type [...] with [...] no non-static data members
+data().Empty = false;
   }
   
   // Handle using declarations of conversion functions.

Modified: cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp?rev=331620&r1=331619&r2=331620&view=diff
==
--- cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp (original)
+++ cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp Sun May  6 23:43:30 2018
@@ -188,12 +188,12 @@ struct YA {
__declspec(align(32)) char : 1;
 };
 // CHECK: *** Dumping AST Record Layout
-// CHECK-NEXT:0 | struct YA (empty)
+// CHECK-NEXT:0 | struct YA
 // CHECK-NEXT:0:0-0 |   char
 // CHECK-NEXT:  | [sizeof=32, align=32
 // CHECK-NEXT:  |  nvsize=32, nvalign=32]
 // CHECK-X64: *** Dumping AST Record Layout
-// CHECK-X64-NEXT:0 | struct YA (empty)
+// CHECK-X64-NEXT:0 | struct YA
 /

r331621 - Remove now-unnecessary check for non-zero nvsize in addition to

[Richard Smith via cfe-commits]

Author: rsmith
Date: Sun May  6 23:43:31 2018
New Revision: 331621

URL: http://llvm.org/viewvc/llvm-project?rev=331621&view=rev
Log:
Remove now-unnecessary check for non-zero nvsize in addition to
emptyness in MS record layout.

Modified:
cfe/trunk/lib/AST/RecordLayoutBuilder.cpp

Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=331621&r1=331620&r2=331621&view=diff
==
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Sun May  6 23:43:31 2018
@@ -2617,8 +2617,8 @@ void MicrosoftRecordLayoutBuilder::layou
   }
 
   if (!FoundBase) {
-if (MDCUsesEBO && BaseDecl->isEmpty() &&
-BaseLayout.getNonVirtualSize() == CharUnits::Zero()) {
+if (MDCUsesEBO && BaseDecl->isEmpty()) {
+  assert(BaseLayout.getNonVirtualSize() == CharUnits::Zero());
   BaseOffset = CharUnits::Zero();
 } else {
   // Otherwise, lay the base out at the end of the MDC.


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45174: non-zero-length bit-fields should make a class non-empty

[Richard Smith - zygoloid via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL331620: Non-zero-length bit-fields make a class non-empty. 
(authored by rsmith, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45174?vs=140654&id=145428#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45174

Files:
  cfe/trunk/docs/ReleaseNotes.rst
  cfe/trunk/lib/AST/DeclCXX.cpp
  cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp
  cfe/trunk/test/Layout/v6-empty.cpp
  cfe/trunk/test/SemaCXX/type-traits.cpp

Index: cfe/trunk/docs/ReleaseNotes.rst
===
--- cfe/trunk/docs/ReleaseNotes.rst
+++ cfe/trunk/docs/ReleaseNotes.rst
@@ -84,6 +84,15 @@
   Clang 7 and earlier versions. The old behavior can be restored by setting
   ``-fclang-abi-compat`` to ``6`` or earlier.
 
+- Clang implements the proposed resolution of LWG issue 2358, along with the
+  `corresponding change to the Itanium C++ ABI
+  `_, which make classes
+  containing only unnamed non-zero-length bit-fields be considered non-empty.
+  This is an ABI break compared to prior Clang releases, but makes Clang
+  generate code that is ABI-compatible with other compilers. The old
+  behavior can be restored by setting ``-fclang-abi-compat`` to ``6`` or
+  lower.
+
 - ...
 
 New Compiler Flags
Index: cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp
===
--- cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp
+++ cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp
@@ -188,12 +188,12 @@
 	__declspec(align(32)) char : 1;
 };
 // CHECK: *** Dumping AST Record Layout
-// CHECK-NEXT:0 | struct YA (empty)
+// CHECK-NEXT:0 | struct YA
 // CHECK-NEXT:0:0-0 |   char
 // CHECK-NEXT:  | [sizeof=32, align=32
 // CHECK-NEXT:  |  nvsize=32, nvalign=32]
 // CHECK-X64: *** Dumping AST Record Layout
-// CHECK-X64-NEXT:0 | struct YA (empty)
+// CHECK-X64-NEXT:0 | struct YA
 // CHECK-X64-NEXT:0:0-0 |   char
 // CHECK-X64-NEXT:  | [sizeof=32, align=32
 // CHECK-X64-NEXT:  |  nvsize=32, nvalign=32]
@@ -206,14 +206,14 @@
 // CHECK: *** Dumping AST Record Layout
 // CHECK-NEXT:0 | struct YB
 // CHECK-NEXT:0 |   char a
-// CHECK-NEXT:1 |   struct YA b (empty)
+// CHECK-NEXT:1 |   struct YA b
 // CHECK-NEXT:1:0-0 | char
 // CHECK-NEXT:  | [sizeof=33, align=1
 // CHECK-NEXT:  |  nvsize=33, nvalign=1]
 // CHECK-X64: *** Dumping AST Record Layout
 // CHECK-X64-NEXT:0 | struct YB
 // CHECK-X64-NEXT:0 |   char a
-// CHECK-X64-NEXT:1 |   struct YA b (empty)
+// CHECK-X64-NEXT:1 |   struct YA b
 // CHECK-X64-NEXT:1:0-0 | char
 // CHECK-X64-NEXT:  | [sizeof=33, align=1
 // CHECK-X64-NEXT:  |  nvsize=33, nvalign=1]
@@ -223,12 +223,12 @@
 	__declspec(align(32)) char : 1;
 };
 // CHECK: *** Dumping AST Record Layout
-// CHECK-NEXT:0 | struct YC (empty)
+// CHECK-NEXT:0 | struct YC
 // CHECK-NEXT:0:0-0 |   char
 // CHECK-NEXT:  | [sizeof=32, align=32
 // CHECK-NEXT:  |  nvsize=32, nvalign=32]
 // CHECK-X64: *** Dumping AST Record Layout
-// CHECK-X64-NEXT:0 | struct YC (empty)
+// CHECK-X64-NEXT:0 | struct YC
 // CHECK-X64-NEXT:0:0-0 |   char
 // CHECK-X64-NEXT:  | [sizeof=8, align=32
 // CHECK-X64-NEXT:  |  nvsize=8, nvalign=32]
@@ -241,14 +241,14 @@
 // CHECK: *** Dumping AST Record Layout
 // CHECK-NEXT:0 | struct YD
 // CHECK-NEXT:0 |   char a
-// CHECK-NEXT:1 |   struct YC b (empty)
+// CHECK-NEXT:1 |   struct YC b
 // CHECK-NEXT:1:0-0 | char
 // CHECK-NEXT:  | [sizeof=33, align=1
 // CHECK-NEXT:  |  nvsize=33, nvalign=1]
 // CHECK-X64: *** Dumping AST Record Layout
 // CHECK-X64-NEXT:0 | struct YD
 // CHECK-X64-NEXT:0 |   char a
-// CHECK-X64-NEXT:1 |   struct YC b (empty)
+// CHECK-X64-NEXT:1 |   struct YC b
 // CHECK-X64-NEXT:1:0-0 | char
 // CHECK-X64-NEXT:  | [sizeof=9, align=1
 // CHECK-X64-NEXT:  |  nvsize=9, nvalign=1]
@@ -258,12 +258,12 @@
 	__declspec(align(32)) char : 1;
 };
 // CHECK: *** Dumping AST Record Layout
-// CHECK-NEXT:0 | struct YE (empty)
+// CHECK-NEXT:0 | struct YE
 // CHECK-NEXT:0:0-0 |   char
 // CHECK-NEXT:  | [sizeof=4, align=32
 // CHECK-NEXT:  |  nvsize=4, nvalign=32]
 // CHECK-X64: *** Dumping AST Record Layout
-// CHECK-X64-NEXT:0 | struct YE (empty)
+// CHECK-X64-NEXT:0 | struct YE
 // CHECK-X64-NEXT:0:0-0 |   char
 // CHECK-X64-NEXT:  | [sizeof=4, align=32
 // CHECK-X64-NEXT:  |  nvsize=4, nvalign=32]
@@ -276,14 +276,14 @@
 // CHECK: *** Dumping AST Record Layout
 // CHECK-NEXT:0 | struct YF
 // CHECK-NEXT:0 |   char a
-// CHECK-NEXT:1 |   struct YE b (empty)
+// CHECK-NEXT:1 |   struct YE b
 // CHECK-NEXT:1:0-0 | char
 // CHECK-NEXT:  | [sizeof=5, align=1
 // CHECK-NEXT:  |  nvsize=5