r311640 - Fixups to FE tests affected by D36793

[Coby Tayree via cfe-commits]

Author: coby
Date: Thu Aug 24 01:47:26 2017
New Revision: 311640

URL: http://llvm.org/viewvc/llvm-project?rev=311640&view=rev
Log:
Fixups to FE tests affected by D36793

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

Modified:
cfe/trunk/test/CodeGen/mozilla-ms-inline-asm.c
cfe/trunk/test/CodeGen/ms-inline-asm-64.c
cfe/trunk/test/CodeGen/ms-inline-asm.c
cfe/trunk/test/CodeGen/ms-inline-asm.cpp
cfe/trunk/test/CodeGenCXX/ms-inline-asm-return.cpp
cfe/trunk/test/Sema/ms-inline-asm.c

Modified: cfe/trunk/test/CodeGen/mozilla-ms-inline-asm.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/mozilla-ms-inline-asm.c?rev=311640&r1=311639&r2=311640&view=diff
==
--- cfe/trunk/test/CodeGen/mozilla-ms-inline-asm.c (original)
+++ cfe/trunk/test/CodeGen/mozilla-ms-inline-asm.c Thu Aug 24 01:47:26 2017
@@ -33,7 +33,7 @@ void invoke(void* that, unsigned methodI
 // CHECK-SAME: push ecx
 // CHECK-SAME: mov edx,[ecx]
 // CHECK-SAME: mov eax,$4
-// CHECK-SAME: call dword ptr[edx+eax*$$4]
+// CHECK-SAME: call dword ptr[edx + eax * $$4]
 // CHECK-SAME: mov esp,ebp
 // CHECK-SAME: pop ebp
 // CHECK-SAME: ret

Modified: cfe/trunk/test/CodeGen/ms-inline-asm-64.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-inline-asm-64.c?rev=311640&r1=311639&r2=311640&view=diff
==
--- cfe/trunk/test/CodeGen/ms-inline-asm-64.c (original)
+++ cfe/trunk/test/CodeGen/ms-inline-asm-64.c Thu Aug 24 01:47:26 2017
@@ -34,8 +34,8 @@ int t3() {
 // CHECK: t3
 // CHECK: call void asm sideeffect inteldialect
 // CHECK-SAME: lea ebx, $0
-// CHECK-SAME: mov eax, [ebx].0
-// CHECK-SAME: mov [ebx].4, ecx
+// CHECK-SAME: mov eax, [ebx]
+// CHECK-SAME: mov [ebx + $$4], ecx
 // CHECK-SAME: "*m,~{eax},~{ebx},~{dirflag},~{fpsr},~{flags}"(%struct.t3_type* 
%{{.*}})
 }
 
@@ -54,7 +54,7 @@ int t4() {
 // CHECK: t4
 // CHECK: call void asm sideeffect inteldialect
 // CHECK-SAME: lea ebx, $0
-// CHECK-SAME: mov eax, [ebx].0
-// CHECK-SAME: mov [ebx].4, ecx
+// CHECK-SAME: mov eax, [ebx]
+// CHECK-SAME: mov [ebx + $$4], ecx
 // CHECK-SAME: "*m,~{eax},~{ebx},~{dirflag},~{fpsr},~{flags}"(%struct.t3_type* 
%{{.*}})
 }

Modified: cfe/trunk/test/CodeGen/ms-inline-asm.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-inline-asm.c?rev=311640&r1=311639&r2=311640&view=diff
==
--- cfe/trunk/test/CodeGen/ms-inline-asm.c (original)
+++ cfe/trunk/test/CodeGen/ms-inline-asm.c Thu Aug 24 01:47:26 2017
@@ -42,7 +42,7 @@ void t5(void) {
 void t6(void) {
   __asm int 0x2c
 // CHECK: t6
-// CHECK: call void asm sideeffect inteldialect "int $$0x2c", 
"~{dirflag},~{fpsr},~{flags}"()
+// CHECK: call void asm sideeffect inteldialect "int $$44", 
"~{dirflag},~{fpsr},~{flags}"()
 }
 
 void t7() {
@@ -61,7 +61,7 @@ void t7() {
 mov eax, ebx
   }
 // CHECK: t7
-// CHECK: call void asm sideeffect inteldialect "int $$0x2cU", 
"~{dirflag},~{fpsr},~{flags}"()
+// CHECK: call void asm sideeffect inteldialect "int $$44", 
"~{dirflag},~{fpsr},~{flags}"()
 // CHECK: call void asm sideeffect inteldialect "", 
"~{dirflag},~{fpsr},~{flags}"()
 // CHECK: call void asm sideeffect inteldialect "mov eax, ebx", 
"~{eax},~{dirflag},~{fpsr},~{flags}"()
 }
@@ -94,7 +94,7 @@ void t9() {
 // CHECK: t9
 // CHECK: call void asm sideeffect inteldialect
 // CHECK-SAME: push ebx
-// CHECK-SAME: mov ebx, $$0x07
+// CHECK-SAME: mov ebx, $$7
 // CHECK-SAME: pop ebx
 // CHECK-SAME: "~{ebx},~{esp},~{dirflag},~{fpsr},~{flags}"()
 }
@@ -229,7 +229,7 @@ void t20() {
   __asm mov eax, LENGTH _bar
 // CHECK: mov eax, $$2
   __asm mov eax, [eax + LENGTH foo * 4]
-// CHECK: mov eax, [eax + $$1 * $$4]
+// CHECK: mov eax, [eax + $$4]
 
   __asm mov eax, TYPE foo
 // CHECK: mov eax, $$4
@@ -240,7 +240,7 @@ void t20() {
   __asm mov eax, TYPE _bar
 // CHECK: mov eax, $$1
   __asm mov eax, [eax + TYPE foo * 4]
-// CHECK: mov eax, [eax + $$4 * $$4]
+// CHECK: mov eax, [eax + $$16]
 
   __asm mov eax, SIZE foo
 // CHECK: mov eax, $$4
@@ -249,7 +249,7 @@ void t20() {
   __asm mov eax, SIZE _foo
 // CHECK: mov eax, $$16
   __asm mov eax, [eax + SIZE _foo * 4]
-// CHECK: mov eax, [eax + $$16 * $$4]
+// CHECK: mov eax, [eax + $$64]
   __asm mov eax, SIZE _bar
 // CHECK: mov eax, $$2
 // CHECK: "~{eax},~{dirflag},~{fpsr},~{flags}"()
@@ -265,7 +265,7 @@ void t21() {
 // CHECK: t21
 // CHECK: call void asm sideeffect inteldialect
 // CHECK-SAME: push ebx
-// CHECK-SAME: mov ebx, $$07H
+// CHECK-SAME: mov ebx, $$7
 // CHECK-SAME: pop ebx
 // CHECK-SAME: "~{ebx},~{esp},~{dirflag},~{fpsr},~{flags}"()
 }
@@ -312,13 +312,13 @@ void t24() {
 void t25() {
 // CHECK: t25
   __asm mov eax, 0h
-// CHECK: mov eax, $$0h
+// CHECK: mov eax, $$4294967295
   __asm mov eax, 0fhU
 // CHECK: mov eax, $$15
   __asm mov eax, 0a2h
-// CHE

[PATCH] D36794: Fixups to FE tests affected by D36793

[coby via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL311640: Fixups to FE tests affected by D36793 (authored by 
coby).

Changed prior to commit:
  https://reviews.llvm.org/D36794?vs=111340&id=112518#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D36794

Files:
  cfe/trunk/test/CodeGen/mozilla-ms-inline-asm.c
  cfe/trunk/test/CodeGen/ms-inline-asm-64.c
  cfe/trunk/test/CodeGen/ms-inline-asm.c
  cfe/trunk/test/CodeGen/ms-inline-asm.cpp
  cfe/trunk/test/CodeGenCXX/ms-inline-asm-return.cpp
  cfe/trunk/test/Sema/ms-inline-asm.c

Index: cfe/trunk/test/CodeGen/ms-inline-asm.c
===
--- cfe/trunk/test/CodeGen/ms-inline-asm.c
+++ cfe/trunk/test/CodeGen/ms-inline-asm.c
@@ -42,7 +42,7 @@
 void t6(void) {
   __asm int 0x2c
 // CHECK: t6
-// CHECK: call void asm sideeffect inteldialect "int $$0x2c", "~{dirflag},~{fpsr},~{flags}"()
+// CHECK: call void asm sideeffect inteldialect "int $$44", "~{dirflag},~{fpsr},~{flags}"()
 }
 
 void t7() {
@@ -61,7 +61,7 @@
 mov eax, ebx
   }
 // CHECK: t7
-// CHECK: call void asm sideeffect inteldialect "int $$0x2cU", "~{dirflag},~{fpsr},~{flags}"()
+// CHECK: call void asm sideeffect inteldialect "int $$44", "~{dirflag},~{fpsr},~{flags}"()
 // CHECK: call void asm sideeffect inteldialect "", "~{dirflag},~{fpsr},~{flags}"()
 // CHECK: call void asm sideeffect inteldialect "mov eax, ebx", "~{eax},~{dirflag},~{fpsr},~{flags}"()
 }
@@ -94,7 +94,7 @@
 // CHECK: t9
 // CHECK: call void asm sideeffect inteldialect
 // CHECK-SAME: push ebx
-// CHECK-SAME: mov ebx, $$0x07
+// CHECK-SAME: mov ebx, $$7
 // CHECK-SAME: pop ebx
 // CHECK-SAME: "~{ebx},~{esp},~{dirflag},~{fpsr},~{flags}"()
 }
@@ -229,7 +229,7 @@
   __asm mov eax, LENGTH _bar
 // CHECK: mov eax, $$2
   __asm mov eax, [eax + LENGTH foo * 4]
-// CHECK: mov eax, [eax + $$1 * $$4]
+// CHECK: mov eax, [eax + $$4]
 
   __asm mov eax, TYPE foo
 // CHECK: mov eax, $$4
@@ -240,16 +240,16 @@
   __asm mov eax, TYPE _bar
 // CHECK: mov eax, $$1
   __asm mov eax, [eax + TYPE foo * 4]
-// CHECK: mov eax, [eax + $$4 * $$4]
+// CHECK: mov eax, [eax + $$16]
 
   __asm mov eax, SIZE foo
 // CHECK: mov eax, $$4
   __asm mov eax, SIZE bar
 // CHECK: mov eax, $$1
   __asm mov eax, SIZE _foo
 // CHECK: mov eax, $$16
   __asm mov eax, [eax + SIZE _foo * 4]
-// CHECK: mov eax, [eax + $$16 * $$4]
+// CHECK: mov eax, [eax + $$64]
   __asm mov eax, SIZE _bar
 // CHECK: mov eax, $$2
 // CHECK: "~{eax},~{dirflag},~{fpsr},~{flags}"()
@@ -265,7 +265,7 @@
 // CHECK: t21
 // CHECK: call void asm sideeffect inteldialect
 // CHECK-SAME: push ebx
-// CHECK-SAME: mov ebx, $$07H
+// CHECK-SAME: mov ebx, $$7
 // CHECK-SAME: pop ebx
 // CHECK-SAME: "~{ebx},~{esp},~{dirflag},~{fpsr},~{flags}"()
 }
@@ -312,13 +312,13 @@
 void t25() {
 // CHECK: t25
   __asm mov eax, 0h
-// CHECK: mov eax, $$0h
+// CHECK: mov eax, $$4294967295
   __asm mov eax, 0fhU
 // CHECK: mov eax, $$15
   __asm mov eax, 0a2h
-// CHECK: mov eax, $$0a2h
+// CHECK: mov eax, $$162
   __asm mov eax, 10100010b
-// CHECK: mov eax, $$10100010b
+// CHECK: mov eax, $$162
   __asm mov eax, 10100010BU
 // CHECK: mov eax, $$162
 // CHECK: "~{eax},~{dirflag},~{fpsr},~{flags}"()
@@ -344,7 +344,7 @@
 void t27() {
   __asm mov eax, fs:[0h]
 // CHECK: t27
-// CHECK: call void asm sideeffect inteldialect "mov eax, fs:[$$0h]", "~{eax},~{dirflag},~{fpsr},~{flags}"()
+// CHECK: call void asm sideeffect inteldialect "mov eax, fs:[$$0]", "~{eax},~{dirflag},~{fpsr},~{flags}"()
 }
 
 void t28() {
@@ -426,49 +426,49 @@
 void t34() {
 // CHECK: t34
   __asm prefetchnta 64[eax]
-// CHECK: prefetchnta $$64[eax]
+// CHECK: prefetchnta [eax + $$64]
   __asm mov eax, dword ptr 4[eax]
-// CHECK: mov eax, dword ptr $$4[eax]
+// CHECK: mov eax, dword ptr [eax + $$4]
 // CHECK: "~{eax},~{dirflag},~{fpsr},~{flags}"()
 }
 
 void t35() {
 // CHECK: t35
   __asm prefetchnta [eax + (200*64)]
-// CHECK: prefetchnta [eax + ($$200*$$64)]
+// CHECK: prefetchnta [eax + $$12800]
   __asm mov eax, dword ptr [eax + (200*64)]
-// CHECK: mov eax, dword ptr [eax + ($$200*$$64)]
+// CHECK: mov eax, dword ptr [eax + $$12800]
 // CHECK: "~{eax},~{dirflag},~{fpsr},~{flags}"()
 }
 
 void t36() {
 // CHECK: t36
   int arr[4];
   // Work around PR20368: These should be single line blocks
   __asm { mov eax, 4[arr] }
-// CHECK: call void asm sideeffect inteldialect "mov eax, $$4$0", "*m,~{eax},~{dirflag},~{fpsr},~{flags}"([4 x i32]* %{{.*}})
+// CHECK: call void asm sideeffect inteldialect "mov eax, $0[$$4]", "*m,~{eax},~{dirflag},~{fpsr},~{flags}"([4 x i32]* %{{.*}})
   __asm { mov eax, 4[arr + 4] }
-// CHECK: call void asm sideeffect inteldialect "mov eax, $$8$0", "*m,~{eax},~{dirflag},~{fpsr},~{flags}"([4 x i32]* %{{.*}})
+// CHECK: call void asm sideeffect inteldialect "mov eax, $0[$$8]", "*m,~{eax},~{dirflag},~{fpsr},~{flags}"([4 x i32]* %{{.*}})
   __asm { mov eax, 8[arr + 4 + 32*2 - 4] }
-// CHECK: call void asm sideeffect intel

[PATCH] D37066: [clang-tidy] A follow-up fix of braced-init-list constructors in make-unique check.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 112519.
hokein marked an inline comment as done.
hokein added a comment.

restrict to implicit conversion from std::initializer_list.


https://reviews.llvm.org/D37066

Files:
  clang-tidy/modernize/MakeSmartPtrCheck.cpp
  test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
  test/clang-tidy/modernize-make-unique.cpp


Index: test/clang-tidy/modernize-make-unique.cpp
===
--- test/clang-tidy/modernize-make-unique.cpp
+++ test/clang-tidy/modernize-make-unique.cpp
@@ -43,6 +43,14 @@
   G(int);
 };
 
+struct H {
+  H(std::vector);
+};
+
+struct I {
+  I(G);
+};
+
 namespace {
 class Foo {};
 } // namespace
@@ -316,6 +324,20 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
   // CHECK-FIXES: std::unique_ptr PG3 = std::unique_ptr(new G{1, 2});
 
+  std::unique_ptr PH1 = std::unique_ptr(new H({1, 2, 3}));
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr PH1 = std::unique_ptr(new H({1, 2, 
3}));
+  PH1.reset(new H({1, 2, 3}));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
+  // CHECK-FIXES: PH1.reset(new H({1, 2, 3}));
+
+  std::unique_ptr PI1 = std::unique_ptr(new I(G({1, 2, 3})));
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr PI1 = std::make_unique(G({1, 2, 3}));
+  PI1.reset(new I(G({1, 2, 3})));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
+  // CHECK-FIXES: PI1 = std::make_unique(G({1, 2, 3}));
+
   std::unique_ptr FF = std::unique_ptr(new Foo());
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning:
   // CHECK-FIXES: std::unique_ptr FF = std::make_unique();
Index: test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
===
--- test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
+++ test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
@@ -22,4 +22,10 @@
   const _E *begin() const { return __begin_; }
   const _E *end() const { return __begin_ + __size_; }
 };
+
+template 
+class vector {
+ public:
+  vector(initializer_list<_E> init);
+};
 } // namespace std
Index: clang-tidy/modernize/MakeSmartPtrCheck.cpp
===
--- clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -262,20 +262,32 @@
 break;
   }
   case CXXNewExpr::CallInit: {
-// FIXME: Add fixes for constructors with initializer-list parameters.
+// FIXME: Add fixes for constructors with parameters that can be created
+// with a C++11 braced-init-list (e.g. std::vector, std::map).
 // Unlike ordinal cases, braced list can not be deduced in
 // std::make_smart_ptr, we need to specify the type explicitly in the 
fixes:
 //   struct S { S(std::initializer_list, int); };
+//   struct S2 { S2(std::vector); };
 //   smart_ptr(new S({1, 2, 3}, 1));  // C++98 call-style initialization
 //   smart_ptr(new S({}, 1));
+//   smart_ptr(new S2({1})); // implicit conversion:
+//   //   std::initializer_list => std::vector
 // The above samples have to be replaced with:
 //   std::make_smart_ptr(std::initializer_list({1, 2, 3}), 1);
 //   std::make_smart_ptr(std::initializer_list({}), 1);
+//   std::make_smart_ptr(std::vector({1}));
 if (const auto *CE = New->getConstructExpr()) {
   for (const auto *Arg : CE->arguments()) {
-if (llvm::isa(Arg)) {
+if (isa(Arg)) {
   return false;
 }
+// Check the implicit conversion from the std::initializer_list type to
+// a class type.
+if (const auto *ImplicitCE = dyn_cast(Arg)) {
+  if (ImplicitCE->isStdInitListInitialization()) {
+return false;
+  }
+}
   }
 }
 if (ArraySizeExpr.empty()) {


Index: test/clang-tidy/modernize-make-unique.cpp
===
--- test/clang-tidy/modernize-make-unique.cpp
+++ test/clang-tidy/modernize-make-unique.cpp
@@ -43,6 +43,14 @@
   G(int);
 };
 
+struct H {
+  H(std::vector);
+};
+
+struct I {
+  I(G);
+};
+
 namespace {
 class Foo {};
 } // namespace
@@ -316,6 +324,20 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
   // CHECK-FIXES: std::unique_ptr PG3 = std::unique_ptr(new G{1, 2});
 
+  std::unique_ptr PH1 = std::unique_ptr(new H({1, 2, 3}));
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr PH1 = std::unique_ptr(new H({1, 2, 3}));
+  PH1.reset(new H({1, 2, 3}));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
+  // CHECK-FIXES: PH1.reset(new H({1, 2, 3}));
+
+  std::unique_ptr PI1 = std::unique_ptr(new I(G({1

r311641 - Revert "[clang-format] Break non-trailing block comments"

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Thu Aug 24 01:55:07 2017
New Revision: 311641

URL: http://llvm.org/viewvc/llvm-project?rev=311641&view=rev
Log:
Revert "[clang-format] Break non-trailing block comments"

This reverts commit r311457. It reveals some dormant bugs in comment
reflowing, like breaking a single line jsdoc type annotation before a
parameter into multiple lines.

Modified:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/unittests/Format/FormatTestComments.cpp

Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=311641&r1=311640&r2=311641&view=diff
==
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Thu Aug 24 01:55:07 2017
@@ -1282,7 +1282,7 @@ unsigned ContinuationIndenter::breakProt
   return 0;
 }
   } else if (Current.is(TT_BlockComment)) {
-if (!Style.ReflowComments ||
+if (!Current.isTrailingComment() || !Style.ReflowComments ||
 // If a comment token switches formatting, like
 // /* clang-format on */, we don't want to break it further,
 // but we may still want to adjust its indentation.

Modified: cfe/trunk/unittests/Format/FormatTestComments.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestComments.cpp?rev=311641&r1=311640&r2=311641&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestComments.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestComments.cpp Thu Aug 24 01:55:07 2017
@@ -2787,22 +2787,6 @@ TEST_F(FormatTestComments, AlignsBlockCo
"* long */",
getLLVMStyleWithColumns(20)));
 }
-
-TEST_F(FormatTestComments, NonTrailingBlockComments) {
-  verifyFormat("const /** comment comment */ A = B;",
-   getLLVMStyleWithColumns(40));
-
-  verifyFormat("const /** comment comment comment */ A =\n"
-   "B;",
-   getLLVMStyleWithColumns(40));
-
-  EXPECT_EQ("const /** comment comment comment\n"
-" comment */\n"
-"A = B;",
-format("const /** comment comment comment comment */\n"
-   "A = B;",
-   getLLVMStyleWithColumns(40)));
-}
 } // end namespace
 } // end namespace format
 } // end namespace clang


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

[PATCH] D37066: [clang-tidy] A follow-up fix of braced-init-list constructors in make-unique check.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:279
 if (const auto *CE = New->getConstructExpr()) {
-  for (const auto *Arg : CE->arguments()) {
-if (llvm::isa(Arg)) {
-  return false;
-}
+  if (!ast_matchers::match(findAll(cxxStdInitializerListExpr()), *CE,
+   CE->getConstructor()->getASTContext())

alexfh wrote:
> I wonder whether this exception can be too broad, e.g. if the initializer 
> list is a part of a more deeply nested expression inside the CXXConstructExpr?
Done. I overthought the cases here. The case we need to ignore is the implicit 
conversion from std::initailizer_list.


https://reviews.llvm.org/D37066



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

r311643 - [Clang][x86][Inline Asm] support for GCC style inline asm - Y constraints

[Coby Tayree via cfe-commits]

Author: coby
Date: Thu Aug 24 02:07:34 2017
New Revision: 311643

URL: http://llvm.org/viewvc/llvm-project?rev=311643&view=rev
Log:
[Clang][x86][Inline Asm] support for GCC style inline asm - Y constraints

This patch is intended to enable the use of basic double letter constraints 
used in GCC extended inline asm {Yi Y2 Yz Y0 Ym Yt}.
Supersedes D35205
llvm counterpart: D36369

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

Added:
cfe/trunk/test/CodeGen/x86-GCC-inline-asm-Y-constraints.c   (with props)
Modified:
cfe/trunk/lib/Basic/Targets/X86.cpp
cfe/trunk/lib/Basic/Targets/X86.h
cfe/trunk/lib/CodeGen/TargetInfo.cpp

Modified: cfe/trunk/lib/Basic/Targets/X86.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=311643&r1=311642&r2=311643&view=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/X86.cpp Thu Aug 24 02:07:34 2017
@@ -1386,7 +1386,9 @@ bool X86TargetInfo::validateAsmConstrain
 switch (*Name) {
 default:
   return false;
+case 'z':
 case '0': // First SSE register.
+case '2':
 case 't': // Any SSE register, when SSE2 is enabled.
 case 'i': // Any SSE register, when SSE2 and inter-unit moves enabled.
 case 'm': // Any MMX register, when inter-unit moves enabled.
@@ -1455,33 +1457,39 @@ bool X86TargetInfo::validateOperandSize(
   case 't':
   case 'u':
 return Size <= 128;
-  case 'v':
-  case 'x':
-if (SSELevel >= AVX512F)
-  // 512-bit zmm registers can be used if target supports AVX512F.
-  return Size <= 512U;
-else if (SSELevel >= AVX)
-  // 256-bit ymm registers can be used if target supports AVX.
-  return Size <= 256U;
-return Size <= 128U;
   case 'Y':
 // 'Y' is the first character for several 2-character constraints.
 switch (Constraint[1]) {
 default:
-  break;
+  return false;
 case 'm':
   // 'Ym' is synonymous with 'y'.
 case 'k':
   return Size <= 64;
+case 'z':
+case '0':
+  // XMM0
+  if (SSELevel >= SSE1)
+return Size <= 128U;
+  return false;
 case 'i':
 case 't':
-  // 'Yi' and 'Yt' are synonymous with 'x' when SSE2 is enabled.
-  if (SSELevel >= AVX512F)
-return Size <= 512U;
-  else if (SSELevel >= AVX)
-return Size <= 256U;
-  return SSELevel >= SSE2 && Size <= 128U;
+case '2':
+  // 'Yi','Yt','Y2' are synonymous with 'x' when SSE2 is enabled.
+  if (SSELevel < SSE2)
+return false;
+  break;
 }
+  case 'v':
+  case 'x':
+if (SSELevel >= AVX512F)
+  // 512-bit zmm registers can be used if target supports AVX512F.
+  return Size <= 512U;
+else if (SSELevel >= AVX)
+  // 256-bit ymm registers can be used if target supports AVX.
+  return Size <= 256U;
+return Size <= 128U;
+
   }
 
   return true;
@@ -1515,6 +1523,12 @@ std::string X86TargetInfo::convertConstr
   // the return string.
   break;
 case 'k':
+case 'm':
+case 'i':
+case 't':
+case 'z':
+case '0':
+case '2':
   // "^" hints llvm that this is a 2 letter constraint.
   // "Constraint++" is used to promote the string iterator
   // to the next constraint.

Modified: cfe/trunk/lib/Basic/Targets/X86.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.h?rev=311643&r1=311642&r2=311643&view=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.h (original)
+++ cfe/trunk/lib/Basic/Targets/X86.h Thu Aug 24 02:07:34 2017
@@ -437,9 +437,12 @@ public:
 // In case the constraint is 'r' we need to return Expression
 case 'r':
   return Expression;
+// Double letters Y constraints
+case 'Y':
+  if ((++I != E) && ((*I == '0') || (*I == 'z')))
+return "xmm0";
 default:
-  // Default value if there is no constraint for the register
-  return "";
+  break;
 }
 return "";
   }

Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=311643&r1=311642&r2=311643&view=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Thu Aug 24 02:07:34 2017
@@ -22,6 +22,7 @@
 #include "clang/CodeGen/SwiftCallingConv.h"
 #include "clang/Frontend/CodeGenOptions.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Type.h"
@@ -870,7 +871,10 @@ bool IsX86_MMXType(llvm::Type *IRType) {
 static llvm::Type* X86AdjustInlineAsmType(CodeGen::CodeGenFunction &CGF,
   StringRef Constraint,
   llvm::Type* Ty

[PATCH] D36371: [Clang][x86][Inline Asm] support for GCC style inline asm - Y constraints

[coby via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL311643: [Clang][x86][Inline Asm] support for GCC style 
inline asm - Y constraints (authored by coby).

Changed prior to commit:
  https://reviews.llvm.org/D36371?vs=111885&id=112521#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D36371

Files:
  cfe/trunk/lib/Basic/Targets/X86.cpp
  cfe/trunk/lib/Basic/Targets/X86.h
  cfe/trunk/lib/CodeGen/TargetInfo.cpp
  cfe/trunk/test/CodeGen/x86-GCC-inline-asm-Y-constraints.c

Index: cfe/trunk/test/CodeGen/x86-GCC-inline-asm-Y-constraints.c
===
--- cfe/trunk/test/CodeGen/x86-GCC-inline-asm-Y-constraints.c
+++ cfe/trunk/test/CodeGen/x86-GCC-inline-asm-Y-constraints.c
@@ -0,0 +1,68 @@
+// RUN: %clang_cc1 -ffreestanding  -triple=x86_64-apple-darwin -target-cpu skx %s -emit-llvm -o - | FileCheck %s
+#include 
+// This test is complimented by the .ll test under llvm/test/MC/X86/. 
+// At this level we can only check if the constarints are passed correctly
+// from inline asm to llvm IR.
+
+// CHECK-LABEL: @f_Ym
+void f_Ym(__m64 m)
+  {
+  // CHECK: movq  $0, %mm1
+  // CHECK-SAME: "=^Ym,~{dirflag},~{fpsr},~{flags}"
+  __asm__ volatile ("movq %0, %%mm1\n\t"
+  :"=Ym" (m));
+}
+
+// CHECK-LABEL: f_Yi
+void f_Yi(__m128 x, __m128 y, __m128 z)
+  {
+  // CHECK: vpaddq
+  // CHECK-SAME: "=^Yi,^Yi,^Yi,~{dirflag},~{fpsr},~{flags}"
+  __asm__ volatile ("vpaddq %0, %1, %2\n\t"
+  :"=Yi" (x)
+  :"Yi" (y),"Yi"(z));
+}
+
+// CHECK-LABEL: f_Yt
+void f_Yt(__m128 x, __m128 y, __m128 z)
+  {
+  // CHECK: vpaddq
+  // CHECK-SAME: "=^Yt,^Yt,^Yt,~{dirflag},~{fpsr},~{flags}"
+  __asm__ volatile ("vpaddq %0, %1, %2\n\t"
+  :"=Yt" (x)
+  :"Yt" (y),"Yt"(z));
+}
+
+// CHECK-LABEL: f_Y2
+void f_Y2(__m128 x, __m128 y, __m128 z)
+{
+  // CHECK: vpaddq
+  // CHECK-SAME: "=^Y2,^Y2,^Y2,~{dirflag},~{fpsr},~{flags}"
+  __asm__ volatile ("vpaddq %0, %1, %2\n\t"
+:"=Y2" (x)
+:"Y2" (y),"Y2"(z));
+}
+
+// CHECK-LABEL: f_Yz
+void f_Yz(__m128 x, __m128 y, __m128 z)
+  {
+  // CHECK: vpaddq
+  // CHECK-SAME: vpaddq
+  // CHECK-SAME: "=^Yi,=^Yz,^Yi,0,~{dirflag},~{fpsr},~{flags}"
+  __asm__ volatile ("vpaddq %0,%2,%1\n\t"
+   "vpaddq %1,%0,%2\n\t"
+  :"+Yi"(z),"=Yz" (x)
+  :"Yi" (y) );
+}
+
+// CHECK-LABEL: f_Y0
+void f_Y0(__m128 x, __m128 y, __m128 z)
+  {
+  // CHECK: vpaddq
+  // CHECK-SAME: "=^Yi,=^Y0,^Yi,0,~{dirflag},~{fpsr},~{flags}"
+  __asm__ volatile ("vpaddq %0,%2,%1\n\t"
+  "vpaddq %1,%0,%2\n\t"
+  :"+Yi"(z),"=Y0" (x)
+  :"Yi" (y) );
+}
+
Index: cfe/trunk/lib/CodeGen/TargetInfo.cpp
===
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp
@@ -22,6 +22,7 @@
 #include "clang/CodeGen/SwiftCallingConv.h"
 #include "clang/Frontend/CodeGenOptions.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Type.h"
@@ -870,7 +871,10 @@
 static llvm::Type* X86AdjustInlineAsmType(CodeGen::CodeGenFunction &CGF,
   StringRef Constraint,
   llvm::Type* Ty) {
-  if ((Constraint == "y" || Constraint == "&y") && Ty->isVectorTy()) {
+  bool IsMMXCons = llvm::StringSwitch(Constraint)
+ .Cases("y", "&y", "^Ym", true)
+ .Default(false);
+  if (IsMMXCons && Ty->isVectorTy()) {
 if (cast(Ty)->getBitWidth() != 64) {
   // Invalid MMX constraint
   return nullptr;
Index: cfe/trunk/lib/Basic/Targets/X86.h
===
--- cfe/trunk/lib/Basic/Targets/X86.h
+++ cfe/trunk/lib/Basic/Targets/X86.h
@@ -437,9 +437,12 @@
 // In case the constraint is 'r' we need to return Expression
 case 'r':
   return Expression;
+// Double letters Y constraints
+case 'Y':
+  if ((++I != E) && ((*I == '0') || (*I == 'z')))
+return "xmm0";
 default:
-  // Default value if there is no constraint for the register
-  return "";
+  break;
 }
 return "";
   }
Index: cfe/trunk/lib/Basic/Targets/X86.cpp
===
--- cfe/trunk/lib/Basic/Targets/X86.cpp
+++ cfe/trunk/lib/Basic/Targets/X86.cpp
@@ -1386,7 +1386,9 @@
 switch (*Name) {
 default:
   return false;
+case 'z':
 case '0': // First SSE register.
+case '2':
 case 't': // Any SSE register, when SSE2 is enabled.
 case 'i': // Any SSE register, when SSE2 and inter-unit moves enabled.
 case 'm': // Any MMX register, when inter-unit moves enabled.
@@ -1455,33 +1457,39 @@
   case 't':
   case 'u':
 return Size <= 128;
-  case 'v':
-  case 'x':
-if (SSELevel >= AVX512F)
-  // 512-bit zmm registers can be used if 

[PATCH] D37101: [clangd] [WIP] Add support for snippet completions

[Raoul Wols via Phabricator via cfe-commits]

rwols created this revision.

Enhances CompletionItemsCollector in such a way that snippet
completions are presented to the client.
This is a work-in-progress. It currently works in Sublime Text 3 using the new
"LSP" plugin. In VSCode, the snippets are inserted into the buffer as plaintext
without any processing (bug). Untested in Atom.
See: 
https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#completion-request
See: 
https://github.com/Microsoft/vscode/blob/master/src/vs/editor/contrib/snippet/browser/snippet.md


https://reviews.llvm.org/D37101

Files:
  clangd/ClangdUnit.cpp
  clangd/Protocol.cpp

Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -724,8 +724,8 @@
   if (!CI.insertText.empty())
 Os << R"("insertText":")" << llvm::yaml::escape(CI.insertText) << R"(",)";
   if (CI.insertTextFormat != InsertTextFormat::Missing) {
-Os << R"("insertTextFormat":")" << static_cast(CI.insertTextFormat)
-   << R"(",)";
+Os << R"("insertTextFormat":)" << static_cast(CI.insertTextFormat)
+   << R"(,)";
   }
   if (CI.textEdit)
 Os << R"("textEdit":)" << TextEdit::unparse(*CI.textEdit) << ',';
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -288,45 +288,180 @@
   void ProcessCodeCompleteResults(Sema &S, CodeCompletionContext Context,
   CodeCompletionResult *Results,
   unsigned NumResults) override {
+// Sort the completions by their priority. A higher priority should come
+// earlier in the sequence.
+std::stable_sort(Results, Results + NumResults,
+ [](const CodeCompletionResult &lhs,
+const CodeCompletionResult &rhs) -> bool {
+   return lhs.Priority > rhs.Priority;
+ });
 for (unsigned I = 0; I != NumResults; ++I) {
-  CodeCompletionResult &Result = Results[I];
-  CodeCompletionString *CCS = Result.CreateCodeCompletionString(
-  S, Context, *Allocator, CCTUInfo,
-  CodeCompleteOpts.IncludeBriefComments);
-  if (CCS) {
+  if (Results[I].Availability == CXAvailability_NotAvailable ||
+  Results[I].Availability == CXAvailability_NotAccessible) {
+// A private member variable outside of the class, for example.
+continue;
+  } else {
 CompletionItem Item;
-for (CodeCompletionString::Chunk C : *CCS) {
-  switch (C.Kind) {
-  case CodeCompletionString::CK_ResultType:
-Item.detail = C.Text;
-break;
-  case CodeCompletionString::CK_Optional:
-break;
-  default:
-Item.label += C.Text;
-break;
-  }
-}
-assert(CCS->getTypedText());
-Item.kind = getKind(Result.CursorKind);
-// Priority is a 16-bit integer, hence at most 5 digits.
-assert(CCS->getPriority() < 9 && "Expecting code completion result "
- "priority to have at most "
- "5-digits");
-llvm::raw_string_ostream(Item.sortText)
-<< llvm::format("%05d%s", CCS->getPriority(), CCS->getTypedText());
-Item.insertText = Item.filterText = CCS->getTypedText();
-if (CCS->getBriefComment())
-  Item.documentation = CCS->getBriefComment();
+this->ProcessCodeCompleteResult(S, Context, Results[I], Item);
 Items->push_back(std::move(Item));
   }
 }
   }
 
   GlobalCodeCompletionAllocator &getAllocator() override { return *Allocator; }
 
   CodeCompletionTUInfo &getCodeCompletionTUInfo() override { return CCTUInfo; }
-};
+
+private:
+  void ProcessCodeCompleteResult(Sema &S, CodeCompletionContext Context,
+ CodeCompletionResult &Result,
+ CompletionItem &Item) {
+unsigned ArgCount = 0;
+
+// Fill in the label, detail, documentation and insertText fields of the
+// CompletionItem.
+switch (Result.Kind) {
+case CodeCompletionResult::RK_Declaration: {
+  auto Completion = Result.CreateCodeCompletionString(
+  S, Context, *Allocator, CCTUInfo,
+  CodeCompleteOpts.IncludeBriefComments);
+  if (!Completion) {
+Item.label = Result.Declaration->getNameAsString();
+  } else {
+ProcessCodeCompleteString(*Completion, ArgCount, Item);
+  }
+  break;
+}
+case CodeCompletionResult::RK_Keyword:
+  Item.label = Result.Keyword;
+  break;
+case CodeCompletionResult::RK_Macro: {
+  auto Completion = Result.CreateCodeCompletionString(
+  S, Context, *Allocator, CCTUInfo,
+  CodeCompleteOpts.IncludeBriefComments);
+  if (!Completion) {
+Item.labe

[PATCH] D36075: [refactor] Initial support for refactoring action rules

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

Thanks for the changes! The code is much clearer.

I am wondering if the current design could be extended to support tools (or 
rules) that use AST matchers? Or is the selection expected to be powerful 
enough to replace AST matchers?

We have a few tools in `clang-tools-extra` (e.g. `clang-move` and 
`change-namespace`) and many internal tools that are based on AST matchers, but 
we would really like to move them into `clang-refactor` in the future :)




Comment at: 
include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h:26
+template 
+detail::SourceSelectionRequirement<
+typename selection::detail::EvaluateSelectionChecker<

Could you help me understand this class? 

This seems to be a selection-specific requirement and should live in 
`selection`. It is also used in `BaseSpecializedRule` which seems to be a 
higher level of abstraction. 



Comment at: include/clang/Tooling/Refactoring/RefactoringResult.h:27
+
+  RefactoringResult(AtomicChange Change) : Kind(AtomicChanges) {
+Changes.push_back(std::move(Change));

arphaman wrote:
> alexshap wrote:
> > explicit ?
> Nah, it's more convenient to be able to return a single `AtomicChanges` 
> without an explicit initializer I think.
+1 to `explicit` which could prevent unintentional conversion. 
`RefactoringResult(Change);` isn't too bad IMO.



Comment at: include/clang/Tooling/Refactoring/RefactoringResult.h:21
+struct RefactoringResult {
+  enum ResultKind {
+/// A set of source replacements represented using a vector of

I'm a bit unsure about the abstraction of the refactoring result. I would 
expected refactoring results to be source changes always. Do you have any 
refactoring tool that outputs occurrences in mind?



Comment at: include/clang/Tooling/Refactoring/SourceSelectionConstraints.h:55
+
+namespace detail {
+

Does `detail` mean internal implementation? Maybe use `internal` which is more 
commonly used for this?


Repository:
  rL LLVM

https://reviews.llvm.org/D36075



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

[PATCH] D35982: [mips] Introducing option -mabs=[legacy/2008]

[Aleksandar Beserminji via Phabricator via cfe-commits]

abeserminji updated this revision to Diff 112534.
abeserminji added a comment.

Added test //test/Driver/mips-mabs-warning.c// as suggested in comments.


Repository:
  rL LLVM

https://reviews.llvm.org/D35982

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Driver/Options.td
  lib/Basic/Targets/Mips.cpp
  lib/Basic/Targets/Mips.h
  lib/Driver/ToolChains/Arch/Mips.cpp
  test/Driver/mips-features.c
  test/Driver/mips-mabs-warning.c
  test/Preprocessor/init.c

Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -4860,6 +4860,16 @@
 // RUN:   | FileCheck -match-full-lines -check-prefix NOMIPS-NAN2008 %s
 // NOMIPS-NAN2008-NOT:#define __mips_nan2008 1
 //
+// RUN: %clang_cc1 -target-cpu mips32r3 -target-feature +abs2008 \
+// RUN:   -E -dM -triple=mips-none-none < /dev/null \
+// RUN:   | FileCheck -match-full-lines -check-prefix MIPS-ABS2008 %s
+// MIPS-ABS2008:#define __mips_abs2008 1
+//
+// RUN: %clang_cc1 -target-cpu mips32r3 -target-feature -abs2008 \
+// RUN:   -E -dM -triple=mips-none-none < /dev/null \
+// RUN:   | FileCheck -match-full-lines -check-prefix NOMIPS-ABS2008 %s
+// NOMIPS-ABS2008-NOT:#define __mips_abs2008 1
+//
 // RUN: %clang_cc1 -target-feature -fp64 \
 // RUN:   -E -dM -triple=mips-none-none < /dev/null \
 // RUN:   | FileCheck -match-full-lines -check-prefix MIPS32-MFP32 %s
Index: test/Driver/mips-mabs-warning.c
===
--- /dev/null
+++ test/Driver/mips-mabs-warning.c
@@ -0,0 +1,6 @@
+// REQUIRES: mips-registered-target
+// RUN: %clang -c -target mips-unknown-gnu -mcpu=mips32 -mabs=2008 %s 2>&1 | FileCheck -check-prefix=NO2008 %s
+// NO2008: warning: ignoring '-mabs=2008' option because the 'mips32' architecture does not support it [-Wunsupported-abs]
+
+// RUN: %clang -c -target mips-unknown-gnu -mcpu=mips32r6 -mabs=legacy %s 2>&1 | FileCheck -check-prefix=NOLEGACY %s
+// NOLEGACY: warning: ignoring '-mabs=legacy' option because the 'mips32r6' architecture does not support it [-Wunsupported-abs]
Index: test/Driver/mips-features.c
===
--- test/Driver/mips-features.c
+++ test/Driver/mips-features.c
@@ -221,6 +221,31 @@
 // RUN:   | FileCheck --check-prefix=CHECK-NANLEGACY %s
 // CHECK-NANLEGACY: "-target-feature" "-nan2008"
 //
+// -mabs=2008 on pre R2
+// RUN: %clang -target mips-linux-gnu -march=mips32 -### -c %s \
+// RUN: -mabs=2008 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ABSLEGACY %s
+//
+// -mabs=2008
+// RUN: %clang -target mips-linux-gnu -march=mips32r3 -### -c %s \
+// RUN: -mabs=2008 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ABS2008 %s
+//
+// -mabs=legacy
+// RUN: %clang -target mips-linux-gnu -march=mips32r3 -### -c %s \
+// RUN: -mabs=legacy 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ABSLEGACY %s
+//
+// -mabs=legacy on R6
+// RUN: %clang -target mips-linux-gnu -march=mips32r6 -### -c %s \
+// RUN: -mabs=legacy 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ABS2008 %s
+//
+// CHECK-ABSLEGACY: "-target-feature" "-abs2008"
+// CHECK-ABSLEGACY-NOT: "-target-feature" "+abs2008"
+// CHECK-ABS2008: "-target-feature" "+abs2008"
+// CHECK-ABS2008-NOT: "-target-feature" "-abs2008"
+//
 // -mcompact-branches=never
 // RUN: %clang -target mips-linux-gnu -march=mips32r6 -### -c %s \
 // RUN: -mcompact-branches=never 2>&1 \
Index: lib/Driver/ToolChains/Arch/Mips.cpp
===
--- lib/Driver/ToolChains/Arch/Mips.cpp
+++ lib/Driver/ToolChains/Arch/Mips.cpp
@@ -283,6 +283,28 @@
   << A->getOption().getName() << Val;
   }
 
+  if (Arg *A = Args.getLastArg(options::OPT_mabs_EQ)) {
+StringRef Val = StringRef(A->getValue());
+if (Val == "2008") {
+  if (mips::getIEEE754Standard(CPUName) & mips::Std2008) {
+Features.push_back("+abs2008");
+  } else {
+Features.push_back("-abs2008");
+D.Diag(diag::warn_target_unsupported_abs2008) << CPUName;
+  }
+} else if (Val == "legacy") {
+  if (mips::getIEEE754Standard(CPUName) & mips::Legacy) {
+Features.push_back("-abs2008");
+  } else {
+Features.push_back("+abs2008");
+D.Diag(diag::warn_target_unsupported_abslegacy) << CPUName;
+  }
+} else {
+  D.Diag(diag::err_drv_unsupported_option_argument)
+  << A->getOption().getName() << Val;
+}
+  }
+
   AddTargetFeature(Args, Features, options::OPT_msingle_float,
options::OPT_mdouble_float, "single-float");
   AddTargetFeature(Args, Features, options::OPT_mips16, options::OPT_mno_mips16,
Index: lib/Basic/Targets/Mips.h
===
--- lib/Basic/Targets/Mips.h
+++ lib/Basic/Targets/Mips.h
@@ -46,6 +46,7 @@
   bool IsMips16;

[PATCH] D37101: [clangd] [WIP] Add support for snippet completions

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov requested changes to this revision.
ilya-biryukov added a comment.
This revision now requires changes to proceed.

Thanks for taking your time to implement this!

We need to add some test cases for new completion features. However, adding 
them may be a bit of a pain, so feel free to ask for best practices if you'll 
run into trouble while doing that.
Please also see my inline comments.




Comment at: clangd/ClangdUnit.cpp:293
+// earlier in the sequence.
+std::stable_sort(Results, Results + NumResults,
+ [](const CodeCompletionResult &lhs,

Is this required for snippets?
I suggest we move the removal of `sortText` into a separate commit/discussion.



Comment at: clangd/ClangdUnit.cpp:295
+ [](const CodeCompletionResult &lhs,
+const CodeCompletionResult &rhs) -> bool {
+   return lhs.Priority > rhs.Priority;

Code style: use `UpperCamelCase` for parameter names



Comment at: clangd/ClangdUnit.cpp:299
 for (unsigned I = 0; I != NumResults; ++I) {
-  CodeCompletionResult &Result = Results[I];
-  CodeCompletionString *CCS = Result.CreateCodeCompletionString(
-  S, Context, *Allocator, CCTUInfo,
-  CodeCompleteOpts.IncludeBriefComments);
-  if (CCS) {
+  if (Results[I].Availability == CXAvailability_NotAvailable ||
+  Results[I].Availability == CXAvailability_NotAccessible) {

Could you please also move this to a separate review?
I think we should show inaccessible items, but they should have a much lower 
priority than accessible ones. Others may disagree, but let's have this 
discussion in a separate thread.



Comment at: clangd/ClangdUnit.cpp:305
 CompletionItem Item;
-for (CodeCompletionString::Chunk C : *CCS) {
-  switch (C.Kind) {
-  case CodeCompletionString::CK_ResultType:
-Item.detail = C.Text;
-break;
-  case CodeCompletionString::CK_Optional:
-break;
-  default:
-Item.label += C.Text;
-break;
-  }
-}
-assert(CCS->getTypedText());
-Item.kind = getKind(Result.CursorKind);
-// Priority is a 16-bit integer, hence at most 5 digits.
-assert(CCS->getPriority() < 9 && "Expecting code completion result 
"
- "priority to have at most "
- "5-digits");
-llvm::raw_string_ostream(Item.sortText)
-<< llvm::format("%05d%s", CCS->getPriority(), CCS->getTypedText());
-Item.insertText = Item.filterText = CCS->getTypedText();
-if (CCS->getBriefComment())
-  Item.documentation = CCS->getBriefComment();
+this->ProcessCodeCompleteResult(S, Context, Results[I], Item);
 Items->push_back(std::move(Item));

Code style: we usually don't use explicit `this->` qualifiers.



Comment at: clangd/ClangdUnit.cpp:316
+private:
+  void ProcessCodeCompleteResult(Sema &S, CodeCompletionContext Context,
+ CodeCompletionResult &Result,

Maybe return `CompletionItem` instead of modifying a reference parameter?



Comment at: clangd/ClangdUnit.cpp:316
+private:
+  void ProcessCodeCompleteResult(Sema &S, CodeCompletionContext Context,
+ CodeCompletionResult &Result,

ilya-biryukov wrote:
> Maybe return `CompletionItem` instead of modifying a reference parameter?
Any reason why `Context` is passed by value?  Maybe pass by `const &`?



Comment at: clangd/ClangdUnit.cpp:317
+  void ProcessCodeCompleteResult(Sema &S, CodeCompletionContext Context,
+ CodeCompletionResult &Result,
+ CompletionItem &Item) {

This function does not modify `Result`. Maybe pass by `const &` instead of `&`?



Comment at: clangd/ClangdUnit.cpp:321
+
+// Fill in the label, detail, documentation and insertText fields of the
+// CompletionItem.

Maybe split writes into `Item.label` and other `Item` fields?
That would simplify the function, i.e. something like this:

```
// Set Item.label approriately.
switch (Result.Kind) {
  case RK_Declaration: Item.label = Result.Declaration->getNameAsString();
  //... other enum value
}


// Set other Item fields.
// Note that is also works as expected for RK_Pattern.
auto Completion = Result.CreateCodeCompletionString(/**/);
ProcessCodeCompleteString(/*...*/, Item);
// 
```



Comment at: clangd/ClangdUnit.cpp:362
+// Always a snippet for now.
+Item.insertTextFormat = InsertTextFormat::Snippet;
+  }

Maybe only set it to `Snippet` if we encountered 

[PATCH] D37104: [libc++] PR34298: Change std::function constructor and move assignment operator SFINAE checks to allow std::function with an incomplete return type

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.

This patch fixes PR34298 (https://bugs.llvm.org/show_bug.cgi?id=34298). Since 
Clang changed in r284549, Clang and libc++ prohibit the use of `std::function` 
with an incomplete return type, like in the example below:

  struct Continuation {
std::function fn;
  };

This code failed to compile because of the way SFINAE checks were performed in 
libc++. Essentially when `Continuation` was defining a copy-constructor, it 
tried to find a matching overload for the copy constructor of `fn`, and thus 
tried to instantiate `std::function`s `function(_Fp)` constructor with a 
`std::function` substitute for `_Fp`. That constructor did 
check against `function` in its SFINAE checks, but it did so after trying to 
invoke `_Fp`. This caused an error while evaluating `__invokable_r` because 
`Continuation` is incomplete, and the incomplete type checker for 
`__invokable_r` didn't take `std::function<>` types into account.

This patch ensures that the SFINAE check verify that `_Fp` is not a 
`std::function` before trying to figure out if `_Fp` is invokable.


Repository:
  rL LLVM

https://reviews.llvm.org/D37104

Files:
  include/functional
  
test/libcxx/utilities/function.objects/func.require/incomplete_return_type.pass.cpp


Index: 
test/libcxx/utilities/function.objects/func.require/incomplete_return_type.pass.cpp
===
--- /dev/null
+++ 
test/libcxx/utilities/function.objects/func.require/incomplete_return_type.pass.cpp
@@ -0,0 +1,23 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// Non-standard extension:
+// std::function is allowed to take a function with an incomplete return type.
+
+// [func.require]
+
+#include 
+
+struct IncompleteReturnType {
+  std::function fn;
+};
+
+int main() {
+  return 0;
+}
Index: include/functional
===
--- include/functional
+++ include/functional
@@ -1623,7 +1623,10 @@
 function(const function&);
 function(function&&) _NOEXCEPT;
 template::value && !is_same<_Fp, function>::value
+is_same::value>::type,
+void
+>::value &&
+__callable<_Fp>::value
 >::type>
 function(_Fp);
 
@@ -1648,8 +1651,11 @@
 template
   typename enable_if
   <
-__callable::type>::value &&
-!is_same::type, function>::value,
+is_same::type, 
function>::value>::type,
+ void
+>::value &&
+__callable::type>::value,
 function&
   >::type
   operator=(_Fp&&);
@@ -1857,8 +1863,11 @@
 template 
 typename enable_if
 <
-function<_Rp(_ArgTypes...)>::template __callable::type>::value &&
-!is_same::type, 
function<_Rp(_ArgTypes...)>>::value,
+is_same::type, 
function<_Rp(_ArgTypes...)>>::value>::type,
+ void
+>::value &&
+function<_Rp(_ArgTypes...)>::template __callable::type>::value,
 function<_Rp(_ArgTypes...)>&
 >::type
 function<_Rp(_ArgTypes...)>::operator=(_Fp&& __f)


Index: test/libcxx/utilities/function.objects/func.require/incomplete_return_type.pass.cpp
===
--- /dev/null
+++ test/libcxx/utilities/function.objects/func.require/incomplete_return_type.pass.cpp
@@ -0,0 +1,23 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// Non-standard extension:
+// std::function is allowed to take a function with an incomplete return type.
+
+// [func.require]
+
+#include 
+
+struct IncompleteReturnType {
+  std::function fn;
+};
+
+int main() {
+  return 0;
+}
Index: include/functional
===
--- include/functional
+++ include/functional
@@ -1623,7 +1623,10 @@
 function(const function&);
 function(function&&) _NOEXCEPT;
 template::value && !is_same<_Fp, function>::value
+is_same::value>::type,
+void
+>::value &&
+__callable<_Fp>::value
 >::type>
 function(_Fp);
 
@@ -1648,8 +1651,11 @@
 template
   typename enable_if
   <
-__callable::type>::value &&
-!is_same::type, function>::value,
+is_same::type, function>::value>::type,
+ void
+>::value &&
+__callable::type>::value,
 function&
   >::type
   operator=(_Fp&&);
@@ -

[PATCH] D36075: [refactor] Initial support for refactoring action rules

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: include/clang/Tooling/Refactoring/RefactoringResult.h:21
+struct RefactoringResult {
+  enum ResultKind {
+/// A set of source replacements represented using a vector of

ioeric wrote:
> I'm a bit unsure about the abstraction of the refactoring result. I would 
> expected refactoring results to be source changes always. Do you have any 
> refactoring tool that outputs occurrences in mind?
In Xcode we require rename to return symbol occurrences because the IDE is 
responsible for figuring out: 
1) The new name. Thus we can't produce replacements when renaming because we 
don't know what the new name is when gathering the occurrences.
2) Whether these occurrences should be actually applied. Thus we can't produce 
replacements because it's up to the user to decide if they want to rename some 
occurrence in a comment for example.

In general 2) can be applied to tools like clang-refactor that could allow 
users to select occurrences that don't guarantee a direct semantic match 
(comments, etc.) in an interactive manner.

I think clangd has a rather naive rename support, so these points may not 
apply, but we may want to extend clangd's support for rename in the future as 
well.


Repository:
  rL LLVM

https://reviews.llvm.org/D36075



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

Re: [PATCH] D37090: Implement CFG construction for __finally.

[Nico Weber via cfe-commits]

I just realized that this doesn't do the right thing if the try body
contains jumps out of the try. That's rare and the CFG is mostly used for
warnings, but we should get that right anyhow I suppose. To fix this, I'd
probably add a "ActiveFinally" member, and when building a node for a
return, I'd make the return jump to that if it's active and then add an
edge to the exit node (or a containing finally) to the ActiveFinally. When
building jump nodes (continue, break, go-to) those would also go to the
ActiveFinally which in turn would go to the right place.

I don't think the code that's currently in this CL should change much so
you can look at that if you want, but I'll upload a revision with the above
mentioned changes eventually, so you can also wait for that.

On Aug 23, 2017 9:35 PM, "Nico Weber via Phabricator via cfe-commits" <
cfe-commits@lists.llvm.org> wrote:

> thakis created this revision.
>
> This completes CFG construction for SEH AST nodes.
>
> Also tweak test `g3` to explicitly check that `__leave` in an `__except`
> leaves the outer `__try`.
>
>
> https://reviews.llvm.org/D37090
>
> Files:
>   lib/Analysis/CFG.cpp
>   test/Sema/warn-unreachable-ms.c
>
>
> ___
> 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

[PATCH] D36075: [refactor] Initial support for refactoring action rules

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

In https://reviews.llvm.org/D36075#851278, @ioeric wrote:

> Thanks for the changes! The code is much clearer.
>
> I am wondering if the current design could be extended to support tools (or 
> rules) that use AST matchers? Or is the selection expected to be powerful 
> enough to replace AST matchers?
>
> We have a few tools in `clang-tools-extra` (e.g. `clang-move` and 
> `change-namespace`) and many internal tools that are based on AST matchers, 
> but we would really like to move them into `clang-refactor` in the future :)


The selection requirement is supposed to be orthogonal to AST matchers, not a 
replacement. It should be used when working with source selection in editors.

I did mess around with moving over `clang-reorder-fields` using this kind of 
model and didn't see any issues when using AST matchers. Essentially I used the 
`requiredOption` requirements and mapped them to run my matching code instead 
of using the selection requirement. Thus this requirement was satisfied only 
when the AST matchers were successful. It might be possible to simplify that 
pattern even further to make it simpler.

In https://reviews.llvm.org/D36075#851278, @ioeric wrote:

> Thanks for the changes! The code is much clearer.
>
> I am wondering if the current design could be extended to support tools (or 
> rules) that use AST matchers? Or is the selection expected to be powerful 
> enough to replace AST matchers?
>
> We have a few tools in `clang-tools-extra` (e.g. `clang-move` and 
> `change-namespace`) and many internal tools that are based on AST matchers, 
> but we would really like to move them into `clang-refactor` in the future :)





Repository:
  rL LLVM

https://reviews.llvm.org/D36075



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

[PATCH] D36075: [refactor] Initial support for refactoring action rules

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: 
include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h:26
+template 
+detail::SourceSelectionRequirement<
+typename selection::detail::EvaluateSelectionChecker<

ioeric wrote:
> Could you help me understand this class? 
> 
> This seems to be a selection-specific requirement and should live in 
> `selection`. It is also used in `BaseSpecializedRule` which seems to be a 
> higher level of abstraction. 
It's just a container class that stores all information about the 
`requiredSelection` requirement. I agree about `BaseSpecializedRule`, that 
connection should be chopped. I will move the evaluation code into the 
requirement itself when I update the patch.

I would tend to disagree about moving it though, as 
`SourceSelectionRequirement` is a requirement first and I think that's why it 
should live with other requirements. Yes, it's related to selections, but it 
uses them to implement the requirement. I think it's better to keep 
requirements together, as opposed to having `option` requirements close to 
options, selection requirements close to selection, and so on. WDYT?


Repository:
  rL LLVM

https://reviews.llvm.org/D36075



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

[clang-tools-extra] r311651 - [clang-tidy] bugprone-undefined-memory-manipulation: include type into the message

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Thu Aug 24 05:11:05 2017
New Revision: 311651

URL: http://llvm.org/viewvc/llvm-project?rev=311651&view=rev
Log:
[clang-tidy] bugprone-undefined-memory-manipulation: include type into the 
message

Having the actual type in the message helps a lot understanding warnings in 
templates ;)

+ fix a false negative for type aliases.

Modified:

clang-tools-extra/trunk/clang-tidy/bugprone/UndefinedMemoryManipulationCheck.cpp

clang-tools-extra/trunk/test/clang-tidy/bugprone-undefined-memory-manipulation.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/bugprone/UndefinedMemoryManipulationCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/UndefinedMemoryManipulationCheck.cpp?rev=311651&r1=311650&r2=311651&view=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/bugprone/UndefinedMemoryManipulationCheck.cpp
 (original)
+++ 
clang-tools-extra/trunk/clang-tidy/bugprone/UndefinedMemoryManipulationCheck.cpp
 Thu Aug 24 05:11:05 2017
@@ -26,7 +26,8 @@ AST_MATCHER(CXXRecordDecl, isNotTriviall
 
 void UndefinedMemoryManipulationCheck::registerMatchers(MatchFinder *Finder) {
   const auto NotTriviallyCopyableObject =
-  hasType(pointsTo(cxxRecordDecl(isNotTriviallyCopyable(;
+  hasType(ast_matchers::hasCanonicalType(
+  pointsTo(cxxRecordDecl(isNotTriviallyCopyable();
 
   // Check whether destination object is not TriviallyCopyable.
   // Applicable to all three memory manipulation functions.
@@ -47,13 +48,21 @@ void UndefinedMemoryManipulationCheck::r
 
 void UndefinedMemoryManipulationCheck::check(
 const MatchFinder::MatchResult &Result) {
-  if (const auto *Destination = Result.Nodes.getNodeAs("dest")) {
-diag(Destination->getLocStart(), "undefined behavior, destination "
- "object is not TriviallyCopyable");
+  if (const auto *Call = Result.Nodes.getNodeAs("dest")) {
+QualType DestType = Call->getArg(0)->IgnoreImplicit()->getType();
+if (!DestType->getPointeeType().isNull())
+  DestType = DestType->getPointeeType();
+diag(Call->getLocStart(), "undefined behavior, destination object type %0 "
+  "is not TriviallyCopyable")
+<< DestType;
   }
-  if (const auto *Source = Result.Nodes.getNodeAs("src")) {
-diag(Source->getLocStart(), "undefined behavior, source object is not "
-"TriviallyCopyable");
+  if (const auto *Call = Result.Nodes.getNodeAs("src")) {
+QualType SourceType = Call->getArg(1)->IgnoreImplicit()->getType();
+if (!SourceType->getPointeeType().isNull())
+  SourceType = SourceType->getPointeeType();
+diag(Call->getLocStart(),
+ "undefined behavior, source object type %0 is not TriviallyCopyable")
+<< SourceType;
   }
 }
 

Modified: 
clang-tools-extra/trunk/test/clang-tidy/bugprone-undefined-memory-manipulation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-undefined-memory-manipulation.cpp?rev=311651&r1=311650&r2=311651&view=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/bugprone-undefined-memory-manipulation.cpp
 (original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/bugprone-undefined-memory-manipulation.cpp
 Thu Aug 24 05:11:05 2017
@@ -10,6 +10,7 @@ using ::memmove;
 using ::memset;
 }
 
+namespace types {
 // TriviallyCopyable types:
 struct Plain {
   int n;
@@ -55,88 +56,115 @@ struct VirtualBase : virtual Base {
 // Incomplete type, assume it is TriviallyCopyable.
 struct NoDef;
 
-void f(NoDef *s) {
+} // end namespace types
+
+void f(types::NoDef *s) {
   memset(s, 0, 5);
 }
 
 template 
 void memset_temp(T *b) {
   memset(b, 0, sizeof(T));
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: undefined behavior, destination 
object is not TriviallyCopyable [bugprone-undefined-memory-manipulation]
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: undefined behavior, destination 
object type 'types::VirtualFunc' is not TriviallyCopyable 
[bugprone-undefined-memory-manipulation]
 }
 
 template 
 void memcpy_temp(S *a, T *b) {
   memcpy(a, b, sizeof(T));
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: undefined behavior, source 
object is not TriviallyCopyable [bugprone-undefined-memory-manipulation]
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: undefined behavior, source 
object type 'types::VirtualFunc'
 }
 
 template 
 void memmove_temp(S *a, T *b) {
   memmove(a, b, sizeof(T));
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: undefined behavior, source 
object is not TriviallyCopyable [bugprone-undefined-memory-manipulation]
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: undefined behavior, source 
object type 'types::VirtualFunc'
+}
+
+namespace aliases {
+using Copy2 = types::Copy;
+typedef types::Move Move2;
 }
 
 void notTriviallyCopyable() {
- 

[PATCH] D37066: [clang-tidy] A follow-up fix of braced-init-list constructors in make-unique check.

[Alexander Kornienko via Phabricator via cfe-commits]

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

LG. Thanks!


https://reviews.llvm.org/D37066



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

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

[wangxin via Phabricator via cfe-commits]

wangxindsb added a comment.

There are 105 alarms running the checker on the LibreOffice, 92 True positive, 
13 not sure.


https://reviews.llvm.org/D34275



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

[PATCH] D37106: [Driver][AArch64] Tests for rdm feature.

[Chad Rosier via Phabricator via cfe-commits]

mcrosier created this revision.
Herald added subscribers: kristof.beyls, javed.absar, rengolin, aemerson.

Clang side of https://reviews.llvm.org/D37081.

Chad


https://reviews.llvm.org/D37106

Files:
  test/Driver/aarch64-rdm.c


Index: test/Driver/aarch64-rdm.c
===
--- /dev/null
+++ test/Driver/aarch64-rdm.c
@@ -0,0 +1,9 @@
+// RUN: %clang -target aarch64-none-none-eabi -march=armv8a+rdm -### -c %s 
2>&1 | FileCheck --check-prefix=CHECK-RDM %s
+// RUN: %clang -target aarch64-none-none-eabi -mcpu=generic+rdm -### -c %s 
2>&1 | FileCheck --check-prefix=CHECK-RDM %s
+// RUN: %clang -target aarch64-none-none-eabi -mcpu=falkor -### -c %s 2>&1 | 
FileCheck --check-prefix=CHECK-RDM %s
+// RUN: %clang -target aarch64-none-none-eabi -mcpu=thunderx2t99 -### -c %s 
2>&1 | FileCheck --check-prefix=CHECK-RDM %s
+// CHECK-RDM: "-target-feature" "+rdm"
+
+// RUN: %clang -target aarch64-none-none-eabi -march=armv8a+nordm -### -c %s 
2>&1 | FileCheck --check-prefix=CHECK-NORDM %s
+// RUN: %clang -target aarch64-none-none-eabi -mcpu=generic+nordm -### -c %s 
2>&1 | FileCheck --check-prefix=CHECK-NORDM %s
+// CHECK-NORDM: "-target-feature" "-rdm"


Index: test/Driver/aarch64-rdm.c
===
--- /dev/null
+++ test/Driver/aarch64-rdm.c
@@ -0,0 +1,9 @@
+// RUN: %clang -target aarch64-none-none-eabi -march=armv8a+rdm -### -c %s 2>&1 | FileCheck --check-prefix=CHECK-RDM %s
+// RUN: %clang -target aarch64-none-none-eabi -mcpu=generic+rdm -### -c %s 2>&1 | FileCheck --check-prefix=CHECK-RDM %s
+// RUN: %clang -target aarch64-none-none-eabi -mcpu=falkor -### -c %s 2>&1 | FileCheck --check-prefix=CHECK-RDM %s
+// RUN: %clang -target aarch64-none-none-eabi -mcpu=thunderx2t99 -### -c %s 2>&1 | FileCheck --check-prefix=CHECK-RDM %s
+// CHECK-RDM: "-target-feature" "+rdm"
+
+// RUN: %clang -target aarch64-none-none-eabi -march=armv8a+nordm -### -c %s 2>&1 | FileCheck --check-prefix=CHECK-NORDM %s
+// RUN: %clang -target aarch64-none-none-eabi -mcpu=generic+nordm -### -c %s 2>&1 | FileCheck --check-prefix=CHECK-NORDM %s
+// CHECK-NORDM: "-target-feature" "-rdm"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37101: [clangd] [WIP] Add support for snippet completions

[Raoul Wols via Phabricator via cfe-commits]

rwols added a comment.

Thanks for the quick review! I'm new to Phabricator and the `arc` CLI tool. Is 
the workflow like this: "address a comment, change a few lines, do `arc diff`, 
do this multiple times", or is it like this: "address all the comments, change 
lots of lines, do `arc diff`, do this once"?


https://reviews.llvm.org/D37101



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

[PATCH] D36564: [analyzer] Fix SimpleSValBuilder::simplifySVal

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

I suspect this might have caused http://llvm.org/PR34309. Could you take a look?


Repository:
  rL LLVM

https://reviews.llvm.org/D36564



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

[PATCH] D37004: [clang-diff] Fix the html output for CXXOperatorCallExpr

[Alex Lorenz via Phabricator via cfe-commits]

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

LGTM with one request below:




Comment at: tools/clang-diff/ClangDiff.cpp:319
+   "A Binary operator is supposed to have two arguments.");
+for (int I : {1, 0, 2})
+  Offset = printHtmlForNode(OS, Diff, Tree, IsLeft, Children[I], Offset);

Please add a short comment that describes why this out-of-order traversal is 
required


https://reviews.llvm.org/D37004



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

[PATCH] D37066: [clang-tidy] A follow-up fix of braced-init-list constructors in make-unique check.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL311652: [clang-tidy] A follow-up fix of braced-init-list 
constructors in make-unique… (authored by hokein).

Repository:
  rL LLVM

https://reviews.llvm.org/D37066

Files:
  clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  
clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
  clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp


Index: clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -262,20 +262,32 @@
 break;
   }
   case CXXNewExpr::CallInit: {
-// FIXME: Add fixes for constructors with initializer-list parameters.
+// FIXME: Add fixes for constructors with parameters that can be created
+// with a C++11 braced-init-list (e.g. std::vector, std::map).
 // Unlike ordinal cases, braced list can not be deduced in
 // std::make_smart_ptr, we need to specify the type explicitly in the 
fixes:
 //   struct S { S(std::initializer_list, int); };
+//   struct S2 { S2(std::vector); };
 //   smart_ptr(new S({1, 2, 3}, 1));  // C++98 call-style initialization
 //   smart_ptr(new S({}, 1));
+//   smart_ptr(new S2({1})); // implicit conversion:
+//   //   std::initializer_list => std::vector
 // The above samples have to be replaced with:
 //   std::make_smart_ptr(std::initializer_list({1, 2, 3}), 1);
 //   std::make_smart_ptr(std::initializer_list({}), 1);
+//   std::make_smart_ptr(std::vector({1}));
 if (const auto *CE = New->getConstructExpr()) {
   for (const auto *Arg : CE->arguments()) {
-if (llvm::isa(Arg)) {
+if (isa(Arg)) {
   return false;
 }
+// Check the implicit conversion from the std::initializer_list type to
+// a class type.
+if (const auto *ImplicitCE = dyn_cast(Arg)) {
+  if (ImplicitCE->isStdInitListInitialization()) {
+return false;
+  }
+}
   }
 }
 if (ArraySizeExpr.empty()) {
Index: 
clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
===
--- 
clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
+++ 
clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
@@ -22,4 +22,10 @@
   const _E *begin() const { return __begin_; }
   const _E *end() const { return __begin_ + __size_; }
 };
+
+template 
+class vector {
+ public:
+  vector(initializer_list<_E> init);
+};
 } // namespace std
Index: clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp
@@ -43,6 +43,14 @@
   G(int);
 };
 
+struct H {
+  H(std::vector);
+};
+
+struct I {
+  I(G);
+};
+
 namespace {
 class Foo {};
 } // namespace
@@ -316,6 +324,20 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
   // CHECK-FIXES: std::unique_ptr PG3 = std::unique_ptr(new G{1, 2});
 
+  std::unique_ptr PH1 = std::unique_ptr(new H({1, 2, 3}));
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr PH1 = std::unique_ptr(new H({1, 2, 
3}));
+  PH1.reset(new H({1, 2, 3}));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
+  // CHECK-FIXES: PH1.reset(new H({1, 2, 3}));
+
+  std::unique_ptr PI1 = std::unique_ptr(new I(G({1, 2, 3})));
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr PI1 = std::make_unique(G({1, 2, 3}));
+  PI1.reset(new I(G({1, 2, 3})));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
+  // CHECK-FIXES: PI1 = std::make_unique(G({1, 2, 3}));
+
   std::unique_ptr FF = std::unique_ptr(new Foo());
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning:
   // CHECK-FIXES: std::unique_ptr FF = std::make_unique();


Index: clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -262,20 +262,32 @@
 break;
   }
   case CXXNewExpr::CallInit: {
-// FIXME: Add fixes for constructors with initializer-list parameters.
+// FIXME: Add fixes for constructors with parameters that can be created
+// with a C++11 braced-init-list (e.g. std::vector, std::map).
 // Unlike ordinal c

[clang-tools-extra] r311652 - [clang-tidy] A follow-up fix of braced-init-list constructors in make-unique check.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Thu Aug 24 06:35:55 2017
New Revision: 311652

URL: http://llvm.org/viewvc/llvm-project?rev=311652&view=rev
Log:
[clang-tidy] A follow-up fix of braced-init-list constructors in make-unique 
check.

Reviewers: alexfh

Reviewed By: alexfh

Subscribers: JDevlieghere, xazax.hun, cfe-commits

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

Modified:
clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp

clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp

Modified: clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp?rev=311652&r1=311651&r2=311652&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp Thu Aug 
24 06:35:55 2017
@@ -262,20 +262,32 @@ bool MakeSmartPtrCheck::replaceNew(Diagn
 break;
   }
   case CXXNewExpr::CallInit: {
-// FIXME: Add fixes for constructors with initializer-list parameters.
+// FIXME: Add fixes for constructors with parameters that can be created
+// with a C++11 braced-init-list (e.g. std::vector, std::map).
 // Unlike ordinal cases, braced list can not be deduced in
 // std::make_smart_ptr, we need to specify the type explicitly in the 
fixes:
 //   struct S { S(std::initializer_list, int); };
+//   struct S2 { S2(std::vector); };
 //   smart_ptr(new S({1, 2, 3}, 1));  // C++98 call-style initialization
 //   smart_ptr(new S({}, 1));
+//   smart_ptr(new S2({1})); // implicit conversion:
+//   //   std::initializer_list => std::vector
 // The above samples have to be replaced with:
 //   std::make_smart_ptr(std::initializer_list({1, 2, 3}), 1);
 //   std::make_smart_ptr(std::initializer_list({}), 1);
+//   std::make_smart_ptr(std::vector({1}));
 if (const auto *CE = New->getConstructExpr()) {
   for (const auto *Arg : CE->arguments()) {
-if (llvm::isa(Arg)) {
+if (isa(Arg)) {
   return false;
 }
+// Check the implicit conversion from the std::initializer_list type to
+// a class type.
+if (const auto *ImplicitCE = dyn_cast(Arg)) {
+  if (ImplicitCE->isStdInitListInitialization()) {
+return false;
+  }
+}
   }
 }
 if (ArraySizeExpr.empty()) {

Modified: 
clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h?rev=311652&r1=311651&r2=311652&view=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
 (original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
 Thu Aug 24 06:35:55 2017
@@ -22,4 +22,10 @@ public:
   const _E *begin() const { return __begin_; }
   const _E *end() const { return __begin_ + __size_; }
 };
+
+template 
+class vector {
+ public:
+  vector(initializer_list<_E> init);
+};
 } // namespace std

Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp?rev=311652&r1=311651&r2=311652&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp Thu Aug 
24 06:35:55 2017
@@ -43,6 +43,14 @@ struct G {
   G(int);
 };
 
+struct H {
+  H(std::vector);
+};
+
+struct I {
+  I(G);
+};
+
 namespace {
 class Foo {};
 } // namespace
@@ -316,6 +324,20 @@ void initialization(int T, Base b) {
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
   // CHECK-FIXES: std::unique_ptr PG3 = std::unique_ptr(new G{1, 2});
 
+  std::unique_ptr PH1 = std::unique_ptr(new H({1, 2, 3}));
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr PH1 = std::unique_ptr(new H({1, 2, 
3}));
+  PH1.reset(new H({1, 2, 3}));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
+  // CHECK-FIXES: PH1.reset(new H({1, 2, 3}));
+
+  std::unique_ptr PI1 = std::unique_ptr(new I(G({1, 2, 3})));
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr PI1 = std::make_unique(G({1, 2, 3}));
+  PI1.reset(new I(G({1, 2, 3})));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make

[PATCH] D37101: [clangd] [WIP] Add support for snippet completions

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In https://reviews.llvm.org/D37101#851380, @rwols wrote:

> Thanks for the quick review! I'm new to Phabricator and the `arc` CLI tool. 
> Is the workflow like this: "address a comment, change a few lines, do `arc 
> diff`, do this multiple times", or is it like this: "address all the 
> comments, change lots of lines, do `arc diff`, do this once"?


Both workflows work,  but I would suggest to address a batch of comments before 
doing `arc diff`, as it may become hard to follow inline comments after 
multiple `arc diff`s.
You can also send multiple commits in a single `arc diff` if you use `git-svn`. 
I highly suggest looking into that workflow if you're familiar with `git`. 
There's also an `arc patch` that you can use to apply a patch from this review 
after doing a new `git-svn` checkout.


https://reviews.llvm.org/D37101



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

[PATCH] D36564: [analyzer] Fix SimpleSValBuilder::simplifySVal

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In https://reviews.llvm.org/D36564#851384, @alexfh wrote:

> I suspect this might have caused http://llvm.org/PR34309. Could you take a 
> look?


FYI, PR34309 doesn't reproduce with this patch reverted.


Repository:
  rL LLVM

https://reviews.llvm.org/D36564



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

Re: r310887 - [analyzer] Fix SimpleSValBuilder::simplifySVal

[Alexander Kornienko via cfe-commits]

This commit causes http://llvm.org/PR34309. Could you take a look?

On Mon, Aug 14, 2017 at 11:23 PM, Alexander Shaposhnikov via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: alexshap
> Date: Mon Aug 14 14:23:08 2017
> New Revision: 310887
>
> URL: http://llvm.org/viewvc/llvm-project?rev=310887&view=rev
> Log:
> [analyzer] Fix SimpleSValBuilder::simplifySVal
>
> This diff fixes a crash (triggered assert) on the newly added test case.
> In the method Simplifier::VisitSymbolData we check the type of S and return
> Loc/NonLoc accordingly.
>
> Differential revision: https://reviews.llvm.org/D36564
>
> Modified:
> cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
> cfe/trunk/test/Analysis/ptr-arith.cpp
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/
> StaticAnalyzer/Core/SimpleSValBuilder.cpp?rev=310887&r1=310886&r2=310887&
> view=diff
> 
> ==
> --- cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp Mon Aug 14
> 14:23:08 2017
> @@ -1016,7 +1016,8 @@ SVal SimpleSValBuilder::simplifySVal(Pro
>SVB.getKnownValue(State, nonloc::SymbolVal(S)))
>  return Loc::isLocType(S->getType()) ? (SVal)SVB.makeIntLocVal(*I)
>  : (SVal)SVB.makeIntVal(*I);
> -  return nonloc::SymbolVal(S);
> +  return Loc::isLocType(S->getType()) ? (SVal)SVB.makeLoc(S)
> +  : nonloc::SymbolVal(S);
>  }
>
>  // TODO: Support SymbolCast. Support IntSymExpr when/if we actually
>
> Modified: cfe/trunk/test/Analysis/ptr-arith.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> Analysis/ptr-arith.cpp?rev=310887&r1=310886&r2=310887&view=diff
> 
> ==
> --- cfe/trunk/test/Analysis/ptr-arith.cpp (original)
> +++ cfe/trunk/test/Analysis/ptr-arith.cpp Mon Aug 14 14:23:08 2017
> @@ -98,3 +98,10 @@ void checkMultiDimansionalArray() {
>int a[5][5];
> *(*(a+1)+2) = 2;
>  }
> +
> +unsigned ptrSubtractionNoCrash(char *Begin, char *End) {
> +  auto N = End - Begin;
> +  if (Begin)
> +return 0;
> +  return N;
> +}
>
>
> ___
> 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

[PATCH] D16403: Add scope information to CFG for If/While/For/Do/Compound/CXXRangeFor statements

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko updated this revision to Diff 112547.
m.ostapenko added a comment.

Ping^4


Repository:
  rL LLVM

https://reviews.llvm.org/D16403

Files:
  include/clang/Analysis/AnalysisContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/CFG.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/scopes-cfg-output.cpp

Index: test/Analysis/scopes-cfg-output.cpp
===
--- /dev/null
+++ test/Analysis/scopes-cfg-output.cpp
@@ -0,0 +1,1098 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-scopes=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+class A {
+public:
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  A() {}
+
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  ~A() {}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2: 1
+// CHECK-NEXT:   3: return [B1.2];
+// CHECK-NEXT:   4: CFGScopeEnd(ReturnStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  operator int() const { return 1; }
+};
+
+int getX();
+extern const bool UV;
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: a
+// CHECK-NEXT:   5: [B1.4] (ImplicitCastExpr, NoOp, const class A)
+// CHECK-NEXT:   6: const A &b = a;
+// CHECK-NEXT:   7: A() (CXXConstructExpr, class A)
+// CHECK-NEXT:   8: [B1.7] (BindTemporary)
+// CHECK-NEXT:   9: [B1.8] (ImplicitCastExpr, NoOp, const class A)
+// CHECK-NEXT:  10: [B1.9]
+// CHECK-NEXT:  11: const A &c = A();
+// CHECK-NEXT:  12: [B1.11].~A() (Implicit destructor)
+// CHECK-NEXT:  13: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:  14: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+void test_const_ref() {
+  A a;
+  const A& b = a;
+  const A& c = A();
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A [2])
+// CHECK-NEXT:   3: A a[2];
+// CHECK-NEXT:   4:  (CXXConstructExpr, class A [0])
+// CHECK-NEXT:   5: A b[0];
+// CHECK-NEXT:   6: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:   7: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_array() {
+  A a[2];
+  A b[0];
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   5:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   6: A c;
+// CHECK-NEXT:   7:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   8: A d;
+// CHECK-NEXT:   9: [B1.8].~A() (Implicit destructor)
+// CHECK-NEXT:  10: [B1.6].~A() (Implicit destructor)
+// CHECK-NEXT:  11: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:  12:  (CXXConstructExpr, class A)
+// CHECK-NEXT:  13: A b;
+// CHECK-NEXT:  14: [B1.13].~A() (Implicit destructor)
+// CHECK-NEXT:  15: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:  16: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_scope() {
+  A a;
+  { A c;
+A d;
+  }
+  A b;
+}
+
+// CHECK:  [B4 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B3
+// CHECK:  [B1]
+// CHECK-NEXT:   1:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   2: A c;
+// CHECK-NEXT:   3: [B1.2].~A() (Implicit destructor)
+// CHECK-NEXT:   4: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   5: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   6: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B2]
+// CHECK-NEXT:   1: CFGScopeBegin(IfStmt)
+// CHECK-NEXT:   2: return;
+// CHECK-NEXT:   3: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   4: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   5: CFGScopeEnd(ReturnStmt)
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B3]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK

[PATCH] D37003: [clang-diff] Support templates

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:299
   }
+// We want nodes to be in the same order as in the source code.
+// So we traverse template parameters before the remainder of the declaration.

Interesting, I didn't know that this iteration happened out-of-order. My 
https://reviews.llvm.org/D35012 patch (I'll commit it right now) has a 
RecursiveASTVisitor subclass that tries to traverse the tree in the source 
order, I think you should move this code:

```
  DEF_TRAVERSE_TMPL_DECL_(Class)
  DEF_TRAVERSE_TMPL_DECL_(Var)
  DEF_TRAVERSE_TMPL_DECL_(Function)
```

to `LexicallyOrderedRecursiveASTVisitor` subclass instead and change your 
visitor to derive from `LexicallyOrderedRecursiveASTVisitor`. You can probably 
repurpose https://reviews.llvm.org/D36998 and have both the visibility change 
and the move to `LexicallyOrderedRecursiveASTVisitor` in that patch.


https://reviews.llvm.org/D37003



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

[clang-tools-extra] r311654 - [clang-tidy] Add missing IgnoreMacros doc for modernize-use-equals-default.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Thu Aug 24 06:45:18 2017
New Revision: 311654

URL: http://llvm.org/viewvc/llvm-project?rev=311654&view=rev
Log:
[clang-tidy] Add missing IgnoreMacros doc for modernize-use-equals-default.

A followup of rL311136.

Modified:

clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-equals-default.rst

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-equals-default.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-equals-default.rst?rev=311654&r1=311653&r2=311654&view=diff
==
--- 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-equals-default.rst 
(original)
+++ 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-equals-default.rst 
Thu Aug 24 06:45:18 2017
@@ -26,3 +26,11 @@ defaulted functions as trivial.
 
 .. note::
   Move-constructor and move-assignment operator are not supported yet.
+
+Options
+---
+
+.. option:: IgnoreMacros
+
+   If set to non-zero, the check will not give warnings inside macros. Default
+   is `1`.


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

Re: [clang-tools-extra] r311654 - [clang-tidy] Add missing IgnoreMacros doc for modernize-use-equals-default.

[Alexander Kornienko via cfe-commits]

Thank you!

On Thu, Aug 24, 2017 at 3:45 PM, Haojian Wu via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: hokein
> Date: Thu Aug 24 06:45:18 2017
> New Revision: 311654
>
> URL: http://llvm.org/viewvc/llvm-project?rev=311654&view=rev
> Log:
> [clang-tidy] Add missing IgnoreMacros doc for modernize-use-equals-default.
>
> A followup of rL311136.
>
> Modified:
> clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-
> use-equals-default.rst
>
> Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-
> use-equals-default.rst
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/docs/clang-tidy/checks/modernize-use-equals-default.
> rst?rev=311654&r1=311653&r2=311654&view=diff
> 
> ==
> --- 
> clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-equals-default.rst
> (original)
> +++ 
> clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-equals-default.rst
> Thu Aug 24 06:45:18 2017
> @@ -26,3 +26,11 @@ defaulted functions as trivial.
>
>  .. note::
>Move-constructor and move-assignment operator are not supported yet.
> +
> +Options
> +---
> +
> +.. option:: IgnoreMacros
> +
> +   If set to non-zero, the check will not give warnings inside macros.
> Default
> +   is `1`.
>
>
> ___
> 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

r311655 - [refactor] Add the AST source selection component

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Thu Aug 24 06:51:09 2017
New Revision: 311655

URL: http://llvm.org/viewvc/llvm-project?rev=311655&view=rev
Log:
[refactor] Add the AST source selection component

This commit adds the base AST source selection component to the refactoring
library. AST selection is represented using a tree of SelectedASTNode values.
Each selected node gets its own selection kind, which can actually be None even
in the middle of tree (e.g. statement in a macro whose child is in a macro
argument). The initial version constructs a "raw" selection tree, without
applying filters and canonicalisation operations to the nodes.

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

Added:
cfe/trunk/include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
cfe/trunk/include/clang/Tooling/Refactoring/ASTSelection.h
cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp
cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp
cfe/trunk/unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp
Modified:
cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
cfe/trunk/include/clang/Basic/SourceLocation.h
cfe/trunk/include/clang/Basic/SourceManager.h
cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt
cfe/trunk/unittests/Tooling/CMakeLists.txt

Added: cfe/trunk/include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h?rev=311655&view=auto
==
--- cfe/trunk/include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h (added)
+++ cfe/trunk/include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h Thu Aug 
24 06:51:09 2017
@@ -0,0 +1,137 @@
+//===--- LexicallyOrderedRecursiveASTVisitor.h - *- 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 LexicallyOrderedRecursiveASTVisitor interface, which
+//  recursively traverses the entire AST in a lexical order.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_AST_LEXICALLY_ORDERED_RECURSIVEASTVISITOR_H
+#define LLVM_CLANG_AST_LEXICALLY_ORDERED_RECURSIVEASTVISITOR_H
+
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/SourceManager.h"
+#include "llvm/Support/SaveAndRestore.h"
+
+namespace clang {
+
+/// A RecursiveASTVisitor subclass that guarantees that AST traversal is
+/// performed in a lexical order (i.e. the order in which declarations are
+/// written in the source).
+///
+/// RecursiveASTVisitor doesn't guarantee lexical ordering because there are
+/// some declarations, like Objective-C @implementation declarations
+/// that might be represented in the AST differently to how they were written
+/// in the source.
+/// In particular, Objective-C @implementation declarations may contain
+/// non-Objective-C declarations, like functions:
+///
+///   @implementation MyClass
+///
+///   - (void) method { }
+///   void normalFunction() { }
+///
+///   @end
+///
+/// Clang's AST stores these declarations outside of the @implementation
+/// declaration, so the example above would be represented using the following
+/// AST:
+///   |-ObjCImplementationDecl ... MyClass
+///   | `-ObjCMethodDecl ... method
+///   |...
+///   `-FunctionDecl ... normalFunction
+///   ...
+///
+/// This class ensures that these declarations are traversed before the
+/// corresponding TraverseDecl for the @implementation returns. This ensures
+/// that the lexical parent relationship between these declarations and the
+/// @implementation is preserved while traversing the AST. Note that the
+/// current implementation doesn't mix these declarations with the declarations
+/// contained in the @implementation, so the traversal of all of the
+/// declarations in the @implementation still doesn't follow the lexical order.
+template 
+class LexicallyOrderedRecursiveASTVisitor
+: public RecursiveASTVisitor {
+  using BaseType = RecursiveASTVisitor;
+
+public:
+  LexicallyOrderedRecursiveASTVisitor(const SourceManager &SM) : SM(SM) {}
+
+  bool TraverseObjCImplementationDecl(ObjCImplementationDecl *D) {
+// Objective-C @implementation declarations should not trigger early exit
+// until the additional decls are traversed as their children are not
+// lexically ordered.
+bool Result = BaseType::TraverseObjCImplementationDecl(D);
+return TraverseAdditionalLexicallyNestedDeclarations() ? Result : false;
+  }
+
+  bool TraverseObjCCategoryImplDecl(ObjCCategoryImplDecl *D) {
+bool Result = BaseType::TraverseObjCCategoryImplDecl(D);
+return TraverseAdditionalLexicallyNestedDeclarations() ? Result : fal

[PATCH] D35012: [refactor] Add the AST source selection component

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL311655: [refactor] Add the AST source selection component 
(authored by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D35012?vs=108078&id=112550#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D35012

Files:
  cfe/trunk/include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h
  cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
  cfe/trunk/include/clang/Basic/SourceLocation.h
  cfe/trunk/include/clang/Basic/SourceManager.h
  cfe/trunk/include/clang/Tooling/Refactoring/ASTSelection.h
  cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp
  cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt
  cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp
  cfe/trunk/unittests/Tooling/CMakeLists.txt
  cfe/trunk/unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp

Index: cfe/trunk/include/clang/Basic/SourceLocation.h
===
--- cfe/trunk/include/clang/Basic/SourceLocation.h
+++ cfe/trunk/include/clang/Basic/SourceLocation.h
@@ -172,6 +172,11 @@
 return getFromRawEncoding((unsigned)(uintptr_t)Encoding);
   }
 
+  static bool isPairOfFileLocations(SourceLocation Start, SourceLocation End) {
+return Start.isValid() && Start.isFileID() && End.isValid() &&
+   End.isFileID();
+  }
+
   void print(raw_ostream &OS, const SourceManager &SM) const;
   std::string printToString(const SourceManager &SM) const;
   void dump(const SourceManager &SM) const;
Index: cfe/trunk/include/clang/Basic/SourceManager.h
===
--- cfe/trunk/include/clang/Basic/SourceManager.h
+++ cfe/trunk/include/clang/Basic/SourceManager.h
@@ -1520,6 +1520,14 @@
 return LHSLoaded;
   }
 
+  /// Return true if the Point is within Start and End.
+  bool isPointWithin(SourceLocation Location, SourceLocation Start,
+ SourceLocation End) const {
+return Location == Start || Location == End ||
+   (isBeforeInTranslationUnit(Start, Location) &&
+isBeforeInTranslationUnit(Location, End));
+  }
+
   // Iterators over FileInfos.
   typedef llvm::DenseMap
   ::const_iterator fileinfo_iterator;
Index: cfe/trunk/include/clang/Tooling/Refactoring/ASTSelection.h
===
--- cfe/trunk/include/clang/Tooling/Refactoring/ASTSelection.h
+++ cfe/trunk/include/clang/Tooling/Refactoring/ASTSelection.h
@@ -0,0 +1,74 @@
+//===--- ASTSelection.h - Clang refactoring library ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLING_REFACTOR_AST_SELECTION_H
+#define LLVM_CLANG_TOOLING_REFACTOR_AST_SELECTION_H
+
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/SourceLocation.h"
+#include 
+
+namespace clang {
+
+class ASTContext;
+
+namespace tooling {
+
+enum class SourceSelectionKind {
+  /// A node that's not selected.
+  None,
+
+  /// A node that's considered to be selected because the whole selection range
+  /// is inside of its source range.
+  ContainsSelection,
+  /// A node that's considered to be selected because the start of the selection
+  /// range is inside its source range.
+  ContainsSelectionStart,
+  /// A node that's considered to be selected because the end of the selection
+  /// range is inside its source range.
+  ContainsSelectionEnd,
+
+  /// A node that's considered to be selected because the node is entirely in
+  /// the selection range.
+  InsideSelection,
+};
+
+/// Represents a selected AST node.
+///
+/// AST selection is represented using a tree of \c SelectedASTNode. The tree
+/// follows the top-down shape of the actual AST. Each selected node has
+/// a selection kind. The kind might be none as the node itself might not
+/// actually be selected, e.g. a statement in macro whose child is in a macro
+/// argument.
+struct SelectedASTNode {
+  ast_type_traits::DynTypedNode Node;
+  SourceSelectionKind SelectionKind;
+  std::vector Children;
+
+  SelectedASTNode(const ast_type_traits::DynTypedNode &Node,
+  SourceSelectionKind SelectionKind)
+  : Node(Node), SelectionKind(SelectionKind) {}
+  SelectedASTNode(SelectedASTNode &&) = default;
+  SelectedASTNode &operator=(SelectedASTNode &&) = default;
+
+  void dump(llvm::raw_ostream &OS = llvm::errs()) const;
+};
+
+/// Traverses the given ASTContext and creates a tree of selected AST nodes.
+///
+/// \returns None if no nodes are selected in the AST, or a selected AST node
+/// that corresponds to the TranslationUnitDecl otherwise.
+Optional findSelectedASTNodes(const ASTContext &Context,
+

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:19
+//   of casting an integer value that is out of range
+//===--===//
+

If this check is intended to conform to CERT's INT50-CPP rule, you should put a 
link to the wiki entry for it here as well.



Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:72
+  std::distance(ED->enumerator_begin(), ED->enumerator_end()));
+  std::transform(ED->enumerator_begin(), ED->enumerator_end(),
+ DeclValues.begin(),

You can use `llvm::transform(ED->enumerators(), ...)` instead -- the semantics 
are identical to `std::transform()`, but it takes a range instead of a pair of 
iterators.


https://reviews.llvm.org/D33672



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

r311656 - Add missing std::move call

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Thu Aug 24 07:08:18 2017
New Revision: 311656

URL: http://llvm.org/viewvc/llvm-project?rev=311656&view=rev
Log:
Add missing std::move call

This should fix
http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental_build/41578/

Modified:
cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp

Modified: cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp?rev=311656&r1=311655&r2=311656&view=diff
==
--- cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp Thu Aug 24 07:08:18 2017
@@ -42,7 +42,7 @@ public:
 SelectionStack.pop_back();
 if (Result.Children.empty())
   return None;
-return Result;
+return std::move(Result);
   }
 
   bool TraverseDecl(Decl *D) {


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

[PATCH] D36564: [analyzer] Fix SimpleSValBuilder::simplifySVal

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I see, it seems that we've constructed `$x - 1` somewhere, where `$x` is a 
pointer, while such stuff normally looks as `&element{SymRegion{$x}, -1}`. I 
guess i'd have to take a more careful look at it soon.


Repository:
  rL LLVM

https://reviews.llvm.org/D36564



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

[PATCH] D37109: [clang-format] Emit absolute splits before lines for comments, try 2

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir created this revision.
Herald added a subscriber: klimek.

This recommits https://reviews.llvm.org/D36956 with an update to the added test
case to not use raw string literals, since this makes gcc unhappy.


https://reviews.llvm.org/D37109

Files:
  lib/Format/BreakableToken.cpp
  unittests/Format/FormatTestComments.cpp


Index: unittests/Format/FormatTestComments.cpp
===
--- unittests/Format/FormatTestComments.cpp
+++ unittests/Format/FormatTestComments.cpp
@@ -2787,6 +2787,24 @@
"* long */",
getLLVMStyleWithColumns(20)));
 }
+
+TEST_F(FormatTestComments, NoCrush_Bug34236) {
+  // This is a test case from a crasher reported in:
+  // https://bugs.llvm.org/show_bug.cgi?id=34236
+  // Temporarily disable formatting for readability.
+  // clang-format off
+  EXPECT_EQ(
+"/**/ /*\n"
+"  *   
a\n"
+"  * b c\n"
+"  * d*/",
+  format(
+"/**/ /*\n"
+" *   a b\n"
+" *   c d*/",
+  getLLVMStyleWithColumns(80)));
+  // clang-format on
+}
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/BreakableToken.cpp
===
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -545,15 +545,18 @@
 }
 
 BreakableToken::Split BreakableBlockComment::getSplitBefore(
-unsigned LineIndex,
-unsigned PreviousEndColumn,
-unsigned ColumnLimit,
+unsigned LineIndex, unsigned PreviousEndColumn, unsigned ColumnLimit,
 llvm::Regex &CommentPragmasRegex) const {
   if (!mayReflow(LineIndex, CommentPragmasRegex))
 return Split(StringRef::npos, 0);
   StringRef TrimmedContent = Content[LineIndex].ltrim(Blanks);
-  return getReflowSplit(TrimmedContent, ReflowPrefix, PreviousEndColumn,
-ColumnLimit);
+  Split Result = getReflowSplit(TrimmedContent, ReflowPrefix, 
PreviousEndColumn,
+ColumnLimit);
+  // Result is relative to TrimmedContent. Adapt it relative to
+  // Content[LineIndex].
+  if (Result.first != StringRef::npos)
+Result.first += Content[LineIndex].size() - TrimmedContent.size();
+  return Result;
 }
 
 unsigned BreakableBlockComment::getReflownColumn(
@@ -633,17 +636,12 @@
 /*CurrentPrefix=*/ReflowPrefix, InPPDirective, /*Newlines=*/0,
 /*Spaces=*/0);
 // Check if we need to also insert a break at the whitespace range.
-// For this we first adapt the reflow split relative to the beginning of 
the
-// content.
 // Note that we don't need a penalty for this break, since it doesn't 
change
 // the total number of lines.
-Split BreakSplit = SplitBefore;
-BreakSplit.first += TrimmedContent.data() - Content[LineIndex].data();
 unsigned ReflownColumn =
 getReflownColumn(TrimmedContent, LineIndex, PreviousEndColumn);
-if (ReflownColumn > ColumnLimit) {
-  insertBreak(LineIndex, 0, BreakSplit, Whitespaces);
-}
+if (ReflownColumn > ColumnLimit)
+  insertBreak(LineIndex, 0, SplitBefore, Whitespaces);
 return;
   }
 


Index: unittests/Format/FormatTestComments.cpp
===
--- unittests/Format/FormatTestComments.cpp
+++ unittests/Format/FormatTestComments.cpp
@@ -2787,6 +2787,24 @@
"* long */",
getLLVMStyleWithColumns(20)));
 }
+
+TEST_F(FormatTestComments, NoCrush_Bug34236) {
+  // This is a test case from a crasher reported in:
+  // https://bugs.llvm.org/show_bug.cgi?id=34236
+  // Temporarily disable formatting for readability.
+  // clang-format off
+  EXPECT_EQ(
+"/**/ /*\n"
+"  *   a\n"
+"  * b c\n"
+"  * d*/",
+  format(
+"/**/ /*\n"
+" *   a b\n"
+" *   c d*/",
+  getLLVMStyleWithColumns(80)));
+  // clang-format on
+}
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/BreakableToken.cpp
===
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -545,15 +545,18 @@
 }
 
 BreakableToken::Split BreakableBlockComment::getSplitBefore(
-unsigned LineIndex,
-unsigned PreviousEndColumn,
-unsigned ColumnLimit,
+unsigned Li

[PATCH] D37106: [Driver][AArch64] Tests for rdm feature.

[Sjoerd Meijer via Phabricator via cfe-commits]

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

Thanks for fixing this.


https://reviews.llvm.org/D37106



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

[PATCH] D37109: [clang-format] Emit absolute splits before lines for comments, try 2

[Daniel Jasper via Phabricator via cfe-commits]

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

Does the test still test the same thing if you set the column limit to 60 and 
remove 20 spaces? If not, this is fine.


https://reviews.llvm.org/D37109



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

[PATCH] D37109: [clang-format] Emit absolute splits before lines for comments, try 2

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

@djasper: No. The test case is a special constellation.


https://reviews.llvm.org/D37109



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

r311660 - [Driver][AArch64] Add tests for RDM feature.

[Chad Rosier via cfe-commits]

Author: mcrosier
Date: Thu Aug 24 07:32:55 2017
New Revision: 311660

URL: http://llvm.org/viewvc/llvm-project?rev=311660&view=rev
Log:
[Driver][AArch64] Add tests for RDM feature.

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

Added:
cfe/trunk/test/Driver/aarch64-rdm.c

Added: cfe/trunk/test/Driver/aarch64-rdm.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-rdm.c?rev=311660&view=auto
==
--- cfe/trunk/test/Driver/aarch64-rdm.c (added)
+++ cfe/trunk/test/Driver/aarch64-rdm.c Thu Aug 24 07:32:55 2017
@@ -0,0 +1,9 @@
+// RUN: %clang -target aarch64-none-none-eabi -march=armv8a+rdm -### -c %s 
2>&1 | FileCheck --check-prefix=CHECK-RDM %s
+// RUN: %clang -target aarch64-none-none-eabi -mcpu=generic+rdm -### -c %s 
2>&1 | FileCheck --check-prefix=CHECK-RDM %s
+// RUN: %clang -target aarch64-none-none-eabi -mcpu=falkor -### -c %s 2>&1 | 
FileCheck --check-prefix=CHECK-RDM %s
+// RUN: %clang -target aarch64-none-none-eabi -mcpu=thunderx2t99 -### -c %s 
2>&1 | FileCheck --check-prefix=CHECK-RDM %s
+// CHECK-RDM: "-target-feature" "+rdm"
+
+// RUN: %clang -target aarch64-none-none-eabi -march=armv8a+nordm -### -c %s 
2>&1 | FileCheck --check-prefix=CHECK-NORDM %s
+// RUN: %clang -target aarch64-none-none-eabi -mcpu=generic+nordm -### -c %s 
2>&1 | FileCheck --check-prefix=CHECK-NORDM %s
+// CHECK-NORDM: "-target-feature" "-rdm"


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

[PATCH] D37106: [Driver][AArch64] Tests for rdm feature.

[Chad Rosier via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL311660: [Driver][AArch64] Add tests for RDM feature. 
(authored by mcrosier).

Changed prior to commit:
  https://reviews.llvm.org/D37106?vs=112543&id=112559#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37106

Files:
  cfe/trunk/test/Driver/aarch64-rdm.c


Index: cfe/trunk/test/Driver/aarch64-rdm.c
===
--- cfe/trunk/test/Driver/aarch64-rdm.c
+++ cfe/trunk/test/Driver/aarch64-rdm.c
@@ -0,0 +1,9 @@
+// RUN: %clang -target aarch64-none-none-eabi -march=armv8a+rdm -### -c %s 
2>&1 | FileCheck --check-prefix=CHECK-RDM %s
+// RUN: %clang -target aarch64-none-none-eabi -mcpu=generic+rdm -### -c %s 
2>&1 | FileCheck --check-prefix=CHECK-RDM %s
+// RUN: %clang -target aarch64-none-none-eabi -mcpu=falkor -### -c %s 2>&1 | 
FileCheck --check-prefix=CHECK-RDM %s
+// RUN: %clang -target aarch64-none-none-eabi -mcpu=thunderx2t99 -### -c %s 
2>&1 | FileCheck --check-prefix=CHECK-RDM %s
+// CHECK-RDM: "-target-feature" "+rdm"
+
+// RUN: %clang -target aarch64-none-none-eabi -march=armv8a+nordm -### -c %s 
2>&1 | FileCheck --check-prefix=CHECK-NORDM %s
+// RUN: %clang -target aarch64-none-none-eabi -mcpu=generic+nordm -### -c %s 
2>&1 | FileCheck --check-prefix=CHECK-NORDM %s
+// CHECK-NORDM: "-target-feature" "-rdm"


Index: cfe/trunk/test/Driver/aarch64-rdm.c
===
--- cfe/trunk/test/Driver/aarch64-rdm.c
+++ cfe/trunk/test/Driver/aarch64-rdm.c
@@ -0,0 +1,9 @@
+// RUN: %clang -target aarch64-none-none-eabi -march=armv8a+rdm -### -c %s 2>&1 | FileCheck --check-prefix=CHECK-RDM %s
+// RUN: %clang -target aarch64-none-none-eabi -mcpu=generic+rdm -### -c %s 2>&1 | FileCheck --check-prefix=CHECK-RDM %s
+// RUN: %clang -target aarch64-none-none-eabi -mcpu=falkor -### -c %s 2>&1 | FileCheck --check-prefix=CHECK-RDM %s
+// RUN: %clang -target aarch64-none-none-eabi -mcpu=thunderx2t99 -### -c %s 2>&1 | FileCheck --check-prefix=CHECK-RDM %s
+// CHECK-RDM: "-target-feature" "+rdm"
+
+// RUN: %clang -target aarch64-none-none-eabi -march=armv8a+nordm -### -c %s 2>&1 | FileCheck --check-prefix=CHECK-NORDM %s
+// RUN: %clang -target aarch64-none-none-eabi -mcpu=generic+nordm -### -c %s 2>&1 | FileCheck --check-prefix=CHECK-NORDM %s
+// CHECK-NORDM: "-target-feature" "-rdm"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

[Artem Dergachev via Phabricator via cfe-commits]

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

All right then, i approve!

> There are 105 alarms running the checker on the LibreOffice, 92 True 
> positive, 13 not sure.

That's impressively loud. I guess you can try reporting some of the bugs you've 
found and seeing their reactions, if you enjoy this sort of stuff (it's always 
the ultimate test for bug-finding tools, and it'd also hopefully make 
libreoffice better).


https://reviews.llvm.org/D34275



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

[PATCH] D35012: [refactor] Add the AST source selection component

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

The test `CursorAtStartOfFunction` is segfaulting.


Repository:
  rL LLVM

https://reviews.llvm.org/D35012



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

r311664 - Fix use-after-free in Clang's ASTSelection unittest

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Thu Aug 24 07:53:48 2017
New Revision: 311664

URL: http://llvm.org/viewvc/llvm-project?rev=311664&view=rev
Log:
Fix use-after-free in Clang's ASTSelection unittest

Modified:
cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp

Modified: cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp?rev=311664&r1=311663&r2=311664&view=diff
==
--- cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp Thu Aug 24 07:53:48 2017
@@ -29,13 +29,14 @@ using FileRange = std::pair {
   FileLocation Location;
   Optional SelectionRange;
+  llvm::function_ref)> Consumer;
 
 public:
-  Optional Selection;
-
-  SelectionFinderVisitor(FileLocation Location,
- Optional SelectionRange)
-  : Location(Location), SelectionRange(SelectionRange) {}
+  SelectionFinderVisitor(
+  FileLocation Location, Optional SelectionRange,
+  llvm::function_ref)> Consumer)
+  : Location(Location), SelectionRange(SelectionRange), Consumer(Consumer) 
{
+  }
 
   bool VisitTranslationUnitDecl(const TranslationUnitDecl *TU) {
 const ASTContext &Context = TU->getASTContext();
@@ -49,19 +50,18 @@ public:
   SourceLocation Loc = Location.translate(SM);
   SelRange = SourceRange(Loc, Loc);
 }
-Selection = findSelectedASTNodes(Context, SelRange);
+Consumer(findSelectedASTNodes(Context, SelRange));
 return false;
   }
 };
 
-Optional
-findSelectedASTNodes(StringRef Source, FileLocation Location,
- Optional SelectionRange,
- SelectionFinderVisitor::Language Language =
- SelectionFinderVisitor::Lang_CXX11) {
-  SelectionFinderVisitor Visitor(Location, SelectionRange);
+void findSelectedASTNodes(
+StringRef Source, FileLocation Location, Optional 
SelectionRange,
+llvm::function_ref)> Consumer,
+SelectionFinderVisitor::Language Language =
+SelectionFinderVisitor::Lang_CXX11) {
+  SelectionFinderVisitor Visitor(Location, SelectionRange, Consumer);
   EXPECT_TRUE(Visitor.runOver(Source, Language));
-  return std::move(Visitor.Selection);
 }
 
 void checkNodeImpl(bool IsTypeMatched, const SelectedASTNode &Node,
@@ -125,123 +125,126 @@ ForAllChildrenOf allChildrenOf(const Sel
 }
 
 TEST(ASTSelectionFinder, CursorNoSelection) {
-  Optional Node =
-  findSelectedASTNodes(" void f() { }", {1, 1}, None);
-  EXPECT_FALSE(Node);
+  findSelectedASTNodes(
+  " void f() { }", {1, 1}, None,
+  [](Optional Node) { EXPECT_FALSE(Node); });
 }
 
 TEST(ASTSelectionFinder, CursorAtStartOfFunction) {
-  Optional Node =
-  findSelectedASTNodes("void f() { }", {1, 1}, None);
-  EXPECT_TRUE(Node);
-  checkNode(*Node, SourceSelectionKind::None,
- /*NumChildren=*/1);
-  checkNode(Node->Children[0],
-  SourceSelectionKind::ContainsSelection,
-  /*NumChildren=*/0, /*Name=*/"f");
-
-  // Check that the dumping works.
-  std::string DumpValue;
-  llvm::raw_string_ostream OS(DumpValue);
-  Node->Children[0].dump(OS);
-  ASSERT_EQ(OS.str(), "FunctionDecl \"f\" contains-selection\n");
+  findSelectedASTNodes(
+  "void f() { }", {1, 1}, None, [](Optional Node) {
+EXPECT_TRUE(Node);
+checkNode(*Node, SourceSelectionKind::None,
+   /*NumChildren=*/1);
+checkNode(Node->Children[0],
+SourceSelectionKind::ContainsSelection,
+/*NumChildren=*/0, /*Name=*/"f");
+
+// Check that the dumping works.
+std::string DumpValue;
+llvm::raw_string_ostream OS(DumpValue);
+Node->Children[0].dump(OS);
+ASSERT_EQ(OS.str(), "FunctionDecl \"f\" contains-selection\n");
+  });
 }
 
 TEST(ASTSelectionFinder, RangeNoSelection) {
-  {
-Optional Node = findSelectedASTNodes(
-" void f() { }", {1, 1}, FileRange{{1, 1}, {1, 1}});
-EXPECT_FALSE(Node);
-  }
-  {
-Optional Node = findSelectedASTNodes(
-"  void f() { }", {1, 1}, FileRange{{1, 1}, {1, 2}});
-EXPECT_FALSE(Node);
-  }
+  findSelectedASTNodes(
+  " void f() { }", {1, 1}, FileRange{{1, 1}, {1, 1}},
+  [](Optional Node) { EXPECT_FALSE(Node); });
+  findSelectedASTNodes(
+  "  void f() { }", {1, 1}, FileRange{{1, 1}, {1, 2}},
+  [](Optional Node) { EXPECT_FALSE(Node); });
 }
 
 TEST(ASTSelectionFinder, EmptyRangeFallbackToCursor) {
-  Optional Node =
-  findSelectedASTNodes("void f() { }", {1, 1}, FileRange{{1, 1}, {1, 1}});
-  EXPECT_TRUE(Node);
-  checkNode(Node->Children[0],
-  SourceSelectionKind::ContainsSelection,
-  /*NumChildren=*/0, /*Name=*/"f");
+  findSelectedASTNodes("void f() { }", {1, 1}, FileRange{{1, 1}, {

[PATCH] D35955: clang-format: Add preprocessor directive indentation

[Daniel Jasper via Phabricator via cfe-commits]

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

From my side this looks good for now (we can always improve more later). 
Krasimir, what do you think?


https://reviews.llvm.org/D35955



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

[PATCH] D35012: [refactor] Add the AST source selection component

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

r311664


Repository:
  rL LLVM

https://reviews.llvm.org/D35012



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

[PATCH] D35955: clang-format: Add preprocessor directive indentation

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

Thank you! I understand this patch better now. Looks good from my side!


https://reviews.llvm.org/D35955



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

[PATCH] D36075: [refactor] Initial support for refactoring action rules

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 112566.
arphaman marked 2 inline comments as done.
arphaman added a comment.

- Rename `detail` to `internal`.
- Remove the `BaseSpecializedRule`.
- Simplify selection requirement and constraint evaluation.


Repository:
  rL LLVM

https://reviews.llvm.org/D36075

Files:
  include/clang/Basic/LLVM.h
  include/clang/Tooling/Refactoring/AtomicChange.h
  include/clang/Tooling/Refactoring/RefactoringActionRule.h
  include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h
  include/clang/Tooling/Refactoring/RefactoringActionRuleRequirementsInternal.h
  include/clang/Tooling/Refactoring/RefactoringActionRules.h
  include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
  include/clang/Tooling/Refactoring/RefactoringOperation.h
  include/clang/Tooling/Refactoring/RefactoringResult.h
  include/clang/Tooling/Refactoring/SourceSelectionConstraints.h
  lib/Tooling/Refactoring/CMakeLists.txt
  lib/Tooling/Refactoring/SourceSelectionConstraints.cpp
  unittests/Tooling/CMakeLists.txt
  unittests/Tooling/RefactoringActionRulesTest.cpp

Index: unittests/Tooling/RefactoringActionRulesTest.cpp
===
--- /dev/null
+++ unittests/Tooling/RefactoringActionRulesTest.cpp
@@ -0,0 +1,157 @@
+//===- unittest/Tooling/RefactoringTestActionRulesTest.cpp ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ReplacementTest.h"
+#include "RewriterTestContext.h"
+#include "clang/Tooling/Refactoring.h"
+#include "clang/Tooling/Refactoring/RefactoringActionRules.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/Errc.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace tooling;
+using namespace refactoring_action_rules;
+
+namespace {
+
+class RefactoringActionRulesTest : public ::testing::Test {
+protected:
+  void SetUp() override {
+Context.Sources.setMainFileID(
+Context.createInMemoryFile("input.cpp", DefaultCode));
+  }
+
+  RewriterTestContext Context;
+  std::string DefaultCode = std::string(100, 'a');
+};
+
+TEST_F(RefactoringActionRulesTest, MyFirstRefactoringRule) {
+  auto ReplaceAWithB =
+  [](std::pair Selection)
+  -> Expected {
+const SourceManager &SM = Selection.first.getSources();
+SourceLocation Loc = Selection.first.getRange().getBegin().getLocWithOffset(
+Selection.second);
+AtomicChange Change(SM, Loc);
+llvm::Error E = Change.replace(SM, Loc, 1, "b");
+if (E)
+  return std::move(E);
+return RefactoringResult(Change);
+  };
+  class SelectionRequirement : public selection::Requirement {
+  public:
+std::pair
+evaluateSelection(selection::SourceSelectionRange Selection) const {
+  return std::make_pair(Selection, 20);
+}
+  };
+  auto Rule = createRefactoringRule(ReplaceAWithB,
+requiredSelection(SelectionRequirement()));
+
+  // When the requirements are satisifed, the rule's function must be invoked.
+  {
+RefactoringOperation Operation(Context.Sources);
+SourceLocation Cursor =
+Context.Sources.getLocForStartOfFile(Context.Sources.getMainFileID())
+.getLocWithOffset(10);
+Operation.setSelectionRange({Cursor, Cursor});
+
+Expected> ErrorOrResult =
+Rule->perform(Operation);
+ASSERT_FALSE(!ErrorOrResult);
+ASSERT_FALSE(!*ErrorOrResult);
+RefactoringResult Result = std::move(**ErrorOrResult);
+ASSERT_EQ(Result.getKind(), RefactoringResult::AtomicChanges);
+ASSERT_EQ(Result.getChanges().size(), 1u);
+std::string YAMLString =
+const_cast(Result.getChanges()[0]).toYAMLString();
+
+ASSERT_STREQ("---\n"
+ "Key: 'input.cpp:30'\n"
+ "FilePath:input.cpp\n"
+ "Error:   ''\n"
+ "InsertedHeaders: \n"
+ "RemovedHeaders:  \n"
+ "Replacements:\n" // Extra whitespace here!
+ "  - FilePath:input.cpp\n"
+ "Offset:  30\n"
+ "Length:  1\n"
+ "ReplacementText: b\n"
+ "...\n",
+ YAMLString.c_str());
+  }
+
+  // When one of the requirements is not satisfied, perform should return either
+  // None or a valid diagnostic.
+  {
+RefactoringOperation Operation(Context.Sources);
+Expected> ErrorOrResult =
+Rule->perform(Operation);
+
+ASSERT_FALSE(!ErrorOrResult);
+Optional Value = std::move(*ErrorOrResult);
+EXPECT_TRUE(!Value);
+  }
+}
+
+TEST_F(RefactoringActionRulesTest, ReturnError) {
+  Expected (*Func)(selection::SourceSelectionRange) =
+  [](selection::SourceSelectionRange

[PATCH] D37106: [Driver][AArch64] Tests for rdm feature.

[Chad Rosier via Phabricator via cfe-commits]

mcrosier added a comment.

In https://reviews.llvm.org/D37106#851441, @SjoerdMeijer wrote:

> Thanks for fixing this.


np!  Thanks for the review!


Repository:
  rL LLVM

https://reviews.llvm.org/D37106



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

[PATCH] D35955: clang-format: Add preprocessor directive indentation

[Erik Uhlmann via Phabricator via cfe-commits]

euhlmann added a comment.

I'm glad this is finally in a state to land. Thanks for the helpful reviews!


https://reviews.llvm.org/D35955



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

[PATCH] D36586: [clang-tidy] hicpp bitwise operations on signed integers

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/hicpp/SignedBitwiseCheck.cpp:23
+  const auto SignedIntegerOperand =
+  
expr(ignoringImpCasts(hasType(isSignedInteger(.bind("signed_operand");
+

aaron.ballman wrote:
> JonasToth wrote:
> > aaron.ballman wrote:
> > > JonasToth wrote:
> > > > aaron.ballman wrote:
> > > > > Is ignoring implicit casts the correct behavior here as far as the 
> > > > > coding standard is concerned? Consider:
> > > > > ```
> > > > > unsigned char c = 128;
> > > > > f(~c); // c promotes to int before ~ is applied
> > > > > ```
> > > > > Looking at the coding standard, I get the impression this is not 
> > > > > allowed, but I'm not *super* familiar with HIC++.
> > > > I first implemented it without ignoring the implicit integer casts, the 
> > > > result was, that most cases (in test cases) where not found. therefore 
> > > > i implemented it that way. I add an testcase for this and see how i 
> > > > need to adjust the matcher.
> > > > 
> > > > Could you help me there with the semantic, since i am not so fluent in 
> > > > C/C++ standardese, but i think the findings are reasonable.
> > > It kind of boils down to the intention from the HIC++. Consider a test 
> > > case like:
> > > ```
> > > void f(int i);
> > > 
> > > void g() {
> > >   unsigned char c = 127;
> > >   f(~c);
> > > }
> > > 
> > > ```
> > > Does `f()` expect to receive `-128` or `128`? I think this code will pass 
> > > your check (ignoring the promotion means the type is `unsigned char`), 
> > > but the actual bitwise operation is on a signed integer value because 
> > > there is an integer promotion. So 127 is promoted to int, then ~ is 
> > > applied, resulting in the value `-128` being passed to the function.
> > Yeah i see, i have such cases added in the tests.
> > TBH. i don't know if the standard wants this covered, but the demonstrated 
> > case is definitly bad.
> > 
> > Would it be a good idea, to warn on assigning/initializing `signed` 
> > integers with `unsigned` integers?
> > 
> > The CppCoreGuidelines have some sections on that as well: [[ 
> > https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#arithmetic
> >  | Section ES.100-103 ]]
> > 
> > Not sure if this check should care. On the other hand, would it  be nice to 
> > have a check that covers all "integer problems".
> > Yeah i see, i have such cases added in the tests.
> > TBH. i don't know if the standard wants this covered, but the demonstrated 
> > case is definitly bad.
> 
> I think you should ask the HIC++ people what they think; the rule text does 
> not make it clear what the behavior should be here.
> 
> > Would it be a good idea, to warn on assigning/initializing signed integers 
> > with unsigned integers?
> 
> We already have such a warning in clang (-Wsign-conversion).
> 
> >The CppCoreGuidelines have some sections on that as well: Section ES.100-103
> >
> >Not sure if this check should care. On the other hand, would it be nice to 
> >have a check that covers all "integer problems".
> 
> Any such check will require so many options that it is likely to be almost 
> unusable. However, I would not be opposed to seeing a clang-tidy module that 
> has a bunch of checks in it that relate to integer problems.
i think, those could land in `bugprone-`, and be aliases to hicpp and the 
cppcoreguidelines if appropriate.

depending on my time (i should have a lot of free time for 2 months) 
i will try to implement some.
is there a resource with all common problems found with integers?


https://reviews.llvm.org/D36586



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

[PATCH] D36915: [Sema] Diagnose local variables and parameters captured by lambda and block expressions in a default argument

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

In https://reviews.llvm.org/D36915#849622, @faisalv wrote:

> I don't think this approach is entirely correct for at least the following 
> reasons:
>
> 1. in the lambda case the machinery that diagnoses capture failures should be 
> the machinery erroring on the lambda (when the parameter is odr-used)
> 2. in the unevaluated case, once you disable the error, the instantiation 
> machinery will fail to find the corresponding instantiated parmvardecl.


Oh right, it stills assert in the unevaluated case. I should have a test case 
for that too.

I also found that the following code, which has a lambda that doesn't capture 
anything, asserts (in IRGen):

  struct S {
template
S(T&&) {}
  };
  
  template
  void foo1() {
struct S2 {
  void foo2(S s = [](){}) {
  }
};
  
S2 s2;
s2.foo2();
  }
  
  void foo3() {
foo1();
  }


https://reviews.llvm.org/D36915



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

r311669 - [mips] Introducing option -mabs=[legacy/2008]

[Petar Jovanovic via cfe-commits]

Author: petarj
Date: Thu Aug 24 09:06:30 2017
New Revision: 311669

URL: http://llvm.org/viewvc/llvm-project?rev=311669&view=rev
Log:
[mips] Introducing option -mabs=[legacy/2008]

In patch r205628 using abs.[ds] instruction is forced, as they should behave
in accordance with flags Has2008 and ABS2008. Unfortunately for revisions
prior mips32r6 and mips64r6, abs.[ds] is not generating correct result when
working with NaNs. To generate a sequence which always produce a correct
result but also to allow user more control on how his code is compiled,
option -mabs is added where user can choose legacy or 2008.
By default legacy mode is used on revisions prior R6. Mips32r6 and mips64r6
use abs2008 mode by default.

Patch by Aleksandar Beserminji

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

Added:
cfe/trunk/test/Driver/mips-mabs-warning.c
Modified:
cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Basic/Targets/Mips.cpp
cfe/trunk/lib/Basic/Targets/Mips.h
cfe/trunk/lib/Driver/ToolChains/Arch/Mips.cpp
cfe/trunk/test/Driver/mips-features.c
cfe/trunk/test/Preprocessor/init.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=311669&r1=311668&r2=311669&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Thu Aug 24 09:06:30 
2017
@@ -285,6 +285,12 @@ def warn_target_unsupported_nan2008 : Wa
 def warn_target_unsupported_nanlegacy : Warning<
   "ignoring '-mnan=legacy' option because the '%0' architecture does not 
support it">,
   InGroup;
+def warn_target_unsupported_abslegacy : Warning<
+  "ignoring '-mabs=legacy' option because the '%0' architecture does not 
support it">,
+  InGroup;
+def warn_target_unsupported_abs2008 : Warning<
+  "ignoring '-mabs=2008' option because the '%0' architecture does not support 
it">,
+  InGroup;
 def warn_target_unsupported_compact_branches : Warning<
   "ignoring '-mcompact-branches=' option because the '%0' architecture does 
not"
   " support it">, InGroup;

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=311669&r1=311668&r2=311669&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Thu Aug 24 09:06:30 2017
@@ -61,6 +61,7 @@ def FloatConversion :
 def DoublePromotion : DiagGroup<"double-promotion">;
 def EnumTooLarge : DiagGroup<"enum-too-large">;
 def UnsupportedNan : DiagGroup<"unsupported-nan">;
+def UnsupportedAbs : DiagGroup<"unsupported-abs">;
 def UnsupportedCB : DiagGroup<"unsupported-cb">;
 def UnsupportedGPOpt : DiagGroup<"unsupported-gpopt">;
 def NonLiteralNullConversion : DiagGroup<"non-literal-null-conversion">;

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=311669&r1=311668&r2=311669&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Thu Aug 24 09:06:30 2017
@@ -2066,6 +2066,7 @@ def mno_embedded_data : Flag<["-"], "mno
   HelpText<"Do not place constants in the .rodata section instead of the "
".sdata if they meet the -G  threshold (MIPS)">;
 def mnan_EQ : Joined<["-"], "mnan=">, Group;
+def mabs_EQ : Joined<["-"], "mabs=">, Group;
 def mabicalls : Flag<["-"], "mabicalls">, Group,
   HelpText<"Enable SVR4-style position-independent code (Mips only)">;
 def mno_abicalls : Flag<["-"], "mno-abicalls">, Group,

Modified: cfe/trunk/lib/Basic/Targets/Mips.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/Mips.cpp?rev=311669&r1=311668&r2=311669&view=diff
==
--- cfe/trunk/lib/Basic/Targets/Mips.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/Mips.cpp Thu Aug 24 09:06:30 2017
@@ -149,6 +149,9 @@ void MipsTargetInfo::getTargetDefines(co
   if (IsNan2008)
 Builder.defineMacro("__mips_nan2008", Twine(1));
 
+  if (IsAbs2008)
+Builder.defineMacro("__mips_abs2008", Twine(1));
+
   switch (DspRev) {
   default:
 break;

Modified: cfe/trunk/lib/Basic/Targets/Mips.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/Mips.h?rev=311669&r1=311668&r2=311669&view=diff
==
--- cfe/trunk/lib/Basic/Targets/Mips.h (original)
+++ cfe/trunk/lib/Basic/Targets

[PATCH] D35982: [mips] Introducing option -mabs=[legacy/2008]

[Petar Jovanovic via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL311669: [mips] Introducing option -mabs=[legacy/2008] 
(authored by petarj).

Changed prior to commit:
  https://reviews.llvm.org/D35982?vs=112534&id=112571#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D35982

Files:
  cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
  cfe/trunk/include/clang/Basic/DiagnosticGroups.td
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/Basic/Targets/Mips.cpp
  cfe/trunk/lib/Basic/Targets/Mips.h
  cfe/trunk/lib/Driver/ToolChains/Arch/Mips.cpp
  cfe/trunk/test/Driver/mips-features.c
  cfe/trunk/test/Driver/mips-mabs-warning.c
  cfe/trunk/test/Preprocessor/init.c

Index: cfe/trunk/lib/Basic/Targets/Mips.cpp
===
--- cfe/trunk/lib/Basic/Targets/Mips.cpp
+++ cfe/trunk/lib/Basic/Targets/Mips.cpp
@@ -149,6 +149,9 @@
   if (IsNan2008)
 Builder.defineMacro("__mips_nan2008", Twine(1));
 
+  if (IsAbs2008)
+Builder.defineMacro("__mips_abs2008", Twine(1));
+
   switch (DspRev) {
   default:
 break;
Index: cfe/trunk/lib/Basic/Targets/Mips.h
===
--- cfe/trunk/lib/Basic/Targets/Mips.h
+++ cfe/trunk/lib/Basic/Targets/Mips.h
@@ -46,6 +46,7 @@
   bool IsMips16;
   bool IsMicromips;
   bool IsNan2008;
+  bool IsAbs2008;
   bool IsSingleFloat;
   bool IsNoABICalls;
   bool CanUseBSDABICalls;
@@ -61,9 +62,9 @@
 public:
   MipsTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
   : TargetInfo(Triple), IsMips16(false), IsMicromips(false),
-IsNan2008(false), IsSingleFloat(false), IsNoABICalls(false),
-CanUseBSDABICalls(false), FloatABI(HardFloat), DspRev(NoDSP),
-HasMSA(false), DisableMadd4(false), HasFP64(false) {
+IsNan2008(false), IsAbs2008(false), IsSingleFloat(false),
+IsNoABICalls(false), CanUseBSDABICalls(false), FloatABI(HardFloat),
+DspRev(NoDSP), HasMSA(false), DisableMadd4(false), HasFP64(false) {
 TheCXXABI.set(TargetCXXABI::GenericMIPS);
 
 setABI((getTriple().getArch() == llvm::Triple::mips ||
@@ -300,6 +301,7 @@
 IsMips16 = false;
 IsMicromips = false;
 IsNan2008 = isIEEE754_2008Default();
+IsAbs2008 = isIEEE754_2008Default();
 IsSingleFloat = false;
 FloatABI = HardFloat;
 DspRev = NoDSP;
@@ -330,6 +332,10 @@
 IsNan2008 = true;
   else if (Feature == "-nan2008")
 IsNan2008 = false;
+  else if (Feature == "+abs2008")
+IsAbs2008 = true;
+  else if (Feature == "-abs2008")
+IsAbs2008 = false;
   else if (Feature == "+noabicalls")
 IsNoABICalls = true;
 }
Index: cfe/trunk/lib/Driver/ToolChains/Arch/Mips.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/Mips.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/Mips.cpp
@@ -283,6 +283,28 @@
   << A->getOption().getName() << Val;
   }
 
+  if (Arg *A = Args.getLastArg(options::OPT_mabs_EQ)) {
+StringRef Val = StringRef(A->getValue());
+if (Val == "2008") {
+  if (mips::getIEEE754Standard(CPUName) & mips::Std2008) {
+Features.push_back("+abs2008");
+  } else {
+Features.push_back("-abs2008");
+D.Diag(diag::warn_target_unsupported_abs2008) << CPUName;
+  }
+} else if (Val == "legacy") {
+  if (mips::getIEEE754Standard(CPUName) & mips::Legacy) {
+Features.push_back("-abs2008");
+  } else {
+Features.push_back("+abs2008");
+D.Diag(diag::warn_target_unsupported_abslegacy) << CPUName;
+  }
+} else {
+  D.Diag(diag::err_drv_unsupported_option_argument)
+  << A->getOption().getName() << Val;
+}
+  }
+
   AddTargetFeature(Args, Features, options::OPT_msingle_float,
options::OPT_mdouble_float, "single-float");
   AddTargetFeature(Args, Features, options::OPT_mips16, options::OPT_mno_mips16,
Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -2066,6 +2066,7 @@
   HelpText<"Do not place constants in the .rodata section instead of the "
".sdata if they meet the -G  threshold (MIPS)">;
 def mnan_EQ : Joined<["-"], "mnan=">, Group;
+def mabs_EQ : Joined<["-"], "mabs=">, Group;
 def mabicalls : Flag<["-"], "mabicalls">, Group,
   HelpText<"Enable SVR4-style position-independent code (Mips only)">;
 def mno_abicalls : Flag<["-"], "mno-abicalls">, Group,
Index: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td
@@ -61,6 +61,7 @@
 def DoublePromotion : DiagGroup<"double-promotion">;
 def EnumTooLarge : DiagGroup<"enum-too-large

[PATCH] D36586: [clang-tidy] hicpp bitwise operations on signed integers

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 112572.
JonasToth added a comment.

- get up to date with master
- added testcase for enums


https://reviews.llvm.org/D36586

Files:
  clang-tidy/hicpp/CMakeLists.txt
  clang-tidy/hicpp/HICPPTidyModule.cpp
  clang-tidy/hicpp/SignedBitwiseCheck.cpp
  clang-tidy/hicpp/SignedBitwiseCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/hicpp-signed-bitwise.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/hicpp-signed-bitwise.cpp

Index: test/clang-tidy/hicpp-signed-bitwise.cpp
===
--- /dev/null
+++ test/clang-tidy/hicpp-signed-bitwise.cpp
@@ -0,0 +1,239 @@
+// RUN: %check_clang_tidy %s hicpp-signed-bitwise %t
+
+// These could cause false positives and should not be considered.
+struct StreamClass {
+};
+StreamClass &operator<<(StreamClass &os, unsigned int i) {
+  return os;
+}
+StreamClass &operator<<(StreamClass &os, int i) {
+  return os;
+}
+StreamClass &operator>>(StreamClass &os, unsigned int i) {
+  return os;
+}
+StreamClass &operator>>(StreamClass &os, int i) {
+  return os;
+}
+struct AnotherStream {
+  AnotherStream &operator<<(unsigned char c) { return *this; }
+  AnotherStream &operator<<(char c) { return *this; }
+
+  AnotherStream &operator>>(unsigned char c) { return *this; }
+  AnotherStream &operator>>(char c) { return *this; }
+};
+
+void binary_bitwise() {
+  int SValue = 42;
+  int SResult;
+
+  unsigned int UValue = 42;
+  unsigned int UResult;
+
+  SResult = SValue & 1; // Bad, one operand signed and result is signed, maybe fix with suffix
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  SResult = SValue & -1; // Bad, both are signed and result is signed
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  SResult = SValue & SValue; // Bad, both are sigend and result is signed
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+
+  UResult = SValue & 1; // Bad, operation on signed, maybe fix with suffix
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = SValue & -1; // Bad, operations are signed, and even negative
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+
+  UResult = UValue & 1u; // Ok
+  UResult = UValue & UValue; // Ok
+
+  unsigned char UByte1 = 0u;
+  unsigned char UByte2 = 16u;
+  char SByte1 = 0;
+  char SByte2 = 16;
+
+  UByte1 = UByte1 & UByte2; // Ok
+  UByte1 = SByte1 & UByte2; // Bad, one is signed
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+  UByte1 = SByte1 & SByte2; // Bad, both are signed
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+  SByte1 = SByte1 & SByte2; // Bad, both are signed and result is signed
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  // More complex expressions.
+  UResult = UValue & (SByte1 + (SByte1 | SByte2)); // Bad, RHS is signed
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-2]]:33: warning: use of a signed integer operand with a binary bitwise operator
+
+  // The rest is to demonstrate functionality but all operators are matched equally.
+  // Therefore functionality is the same for all binary operations.
+  UByte1 = UByte1 | UByte2; // Ok
+  UByte1 = UByte1 | SByte2; // Bad
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  UByte1 = UByte1 ^ UByte2; // Ok
+  UByte1 = UByte1 ^ SByte2; // Bad
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  UByte1 = UByte1 >> UByte2; // Ok
+  UByte1 = UByte1 >> SByte2; // Bad
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  UByte1 = UByte1 << UByte2; // Ok
+  UByte1 = UByte1 << SByte2; // Bad
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+}
+
+void f1(unsigned char c) {}
+void f2(char c) {}
+void f3(int c) {}
+
+void unary_bitwise() {
+  unsigned char UByte1 = 0u;
+  char SByte1 = 0;
+
+  UByte1 = ~UByte1; // Ok
+  SByte1 = ~UByte1; // Bad?
+  SByte1 = ~SByte1; // Bad
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a unary bitwise operator
+  UByte1 = ~SByte1; // Bad
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a unary bitwise operator
+
+  unsigned int UInt = 0u;
+  int SInt = 0;
+
+  f1(~UByte1); // Ok
+  f1(~SByte1); // Bad
+  // CHECK-MESSAGE

[PATCH] D36586: [clang-tidy] hicpp bitwise operations on signed integers

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 112573.
JonasToth added a comment.

- fix indendation in testcase


https://reviews.llvm.org/D36586

Files:
  clang-tidy/hicpp/CMakeLists.txt
  clang-tidy/hicpp/HICPPTidyModule.cpp
  clang-tidy/hicpp/SignedBitwiseCheck.cpp
  clang-tidy/hicpp/SignedBitwiseCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/hicpp-signed-bitwise.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/hicpp-signed-bitwise.cpp

Index: test/clang-tidy/hicpp-signed-bitwise.cpp
===
--- /dev/null
+++ test/clang-tidy/hicpp-signed-bitwise.cpp
@@ -0,0 +1,240 @@
+// RUN: %check_clang_tidy %s hicpp-signed-bitwise %t
+
+// These could cause false positives and should not be considered.
+struct StreamClass {
+};
+StreamClass &operator<<(StreamClass &os, unsigned int i) {
+  return os;
+}
+StreamClass &operator<<(StreamClass &os, int i) {
+  return os;
+}
+StreamClass &operator>>(StreamClass &os, unsigned int i) {
+  return os;
+}
+StreamClass &operator>>(StreamClass &os, int i) {
+  return os;
+}
+struct AnotherStream {
+  AnotherStream &operator<<(unsigned char c) { return *this; }
+  AnotherStream &operator<<(char c) { return *this; }
+
+  AnotherStream &operator>>(unsigned char c) { return *this; }
+  AnotherStream &operator>>(char c) { return *this; }
+};
+
+void binary_bitwise() {
+  int SValue = 42;
+  int SResult;
+
+  unsigned int UValue = 42;
+  unsigned int UResult;
+
+  SResult = SValue & 1; // Bad, one operand signed and result is signed, maybe fix with suffix
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  SResult = SValue & -1; // Bad, both are signed and result is signed
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  SResult = SValue & SValue; // Bad, both are sigend and result is signed
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+
+  UResult = SValue & 1; // Bad, operation on signed, maybe fix with suffix
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = SValue & -1; // Bad, operations are signed, and even negative
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+
+  UResult = UValue & 1u; // Ok
+  UResult = UValue & UValue; // Ok
+
+  unsigned char UByte1 = 0u;
+  unsigned char UByte2 = 16u;
+  char SByte1 = 0;
+  char SByte2 = 16;
+
+  UByte1 = UByte1 & UByte2; // Ok
+  UByte1 = SByte1 & UByte2; // Bad, one is signed
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+  UByte1 = SByte1 & SByte2; // Bad, both are signed
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+  SByte1 = SByte1 & SByte2; // Bad, both are signed and result is signed
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  // More complex expressions.
+  UResult = UValue & (SByte1 + (SByte1 | SByte2)); // Bad, RHS is signed
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-2]]:33: warning: use of a signed integer operand with a binary bitwise operator
+
+  // The rest is to demonstrate functionality but all operators are matched equally.
+  // Therefore functionality is the same for all binary operations.
+  UByte1 = UByte1 | UByte2; // Ok
+  UByte1 = UByte1 | SByte2; // Bad
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  UByte1 = UByte1 ^ UByte2; // Ok
+  UByte1 = UByte1 ^ SByte2; // Bad
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  UByte1 = UByte1 >> UByte2; // Ok
+  UByte1 = UByte1 >> SByte2; // Bad
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  UByte1 = UByte1 << UByte2; // Ok
+  UByte1 = UByte1 << SByte2; // Bad
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+}
+
+void f1(unsigned char c) {}
+void f2(char c) {}
+void f3(int c) {}
+
+void unary_bitwise() {
+  unsigned char UByte1 = 0u;
+  char SByte1 = 0;
+
+  UByte1 = ~UByte1; // Ok
+  SByte1 = ~UByte1; // Bad?
+  SByte1 = ~SByte1; // Bad
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a unary bitwise operator
+  UByte1 = ~SByte1; // Bad
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a unary bitwise operator
+
+  unsigned int UInt = 0u;
+  int SInt = 0;
+
+  f1(~UByte1); // Ok
+  f1(~SByte1); // Bad
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning

[PATCH] D31370: [clang-tidy] Prototype to check for exception specification

[Jonas Toth via Phabricator via cfe-commits]

JonasToth abandoned this revision.
JonasToth added a comment.

exception checking seems to end in the front end for such cases, so nothing to 
do here anymore. reimplement if there is need later.


Repository:
  rL LLVM

https://reviews.llvm.org/D31370



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

[PATCH] D36075: [refactor] Initial support for refactoring action rules

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: 
include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h:26
+template 
+detail::SourceSelectionRequirement<
+typename selection::detail::EvaluateSelectionChecker<

arphaman wrote:
> ioeric wrote:
> > Could you help me understand this class? 
> > 
> > This seems to be a selection-specific requirement and should live in 
> > `selection`. It is also used in `BaseSpecializedRule` which seems to be a 
> > higher level of abstraction. 
> It's just a container class that stores all information about the 
> `requiredSelection` requirement. I agree about `BaseSpecializedRule`, that 
> connection should be chopped. I will move the evaluation code into the 
> requirement itself when I update the patch.
> 
> I would tend to disagree about moving it though, as 
> `SourceSelectionRequirement` is a requirement first and I think that's why it 
> should live with other requirements. Yes, it's related to selections, but it 
> uses them to implement the requirement. I think it's better to keep 
> requirements together, as opposed to having `option` requirements close to 
> options, selection requirements close to selection, and so on. WDYT?
Thanks! 

Makes sense. We might want to put individual requirements into their own 
headers so that this doesn't grow into a huge file when more requirements are 
supported.



Comment at: 
include/clang/Tooling/Refactoring/RefactoringActionRuleRequirementsInternal.h:42
+ RequirementBase>::type {
+  using OutputType = typename DropExpectedOptional::Type;
+

It might worth having a comment explaining why and how `Expected` is 
wrapped and unwrapped during the evaluation.



Comment at: include/clang/Tooling/Refactoring/RefactoringActionRules.h:33
+///
+///  - requiredSelection: The refactoring function won't be invoked unless the
+///   given selection requirement is satisfied.

We might want to document supported requirements somewhere else so that we 
don't need to update this file every time a new requirement is added.



Comment at: include/clang/Tooling/Refactoring/RefactoringOperation.h:1
+//===--- RefactoringOperationController.h - Clang refactoring library 
-===//
+//

s/RefactoringOperationController.h/RefactoringOperation.h/ :)



Comment at: include/clang/Tooling/Refactoring/RefactoringOperation.h:29
+/// to represent a selection in an editor.
+class RefactoringOperation {
+public:

I found the name a bit confusing - `RefactoringOperation` sounds a bit like 
`RefactoringAction`.

Would it make sense to call this `RefactoringContext` or 
`RefactoringRuleContext`, if this stores states of a refactoring rule?



Comment at: include/clang/Tooling/Refactoring/RefactoringResult.h:21
+struct RefactoringResult {
+  enum ResultKind {
+/// A set of source replacements represented using a vector of

arphaman wrote:
> ioeric wrote:
> > I'm a bit unsure about the abstraction of the refactoring result. I would 
> > expected refactoring results to be source changes always. Do you have any 
> > refactoring tool that outputs occurrences in mind?
> In Xcode we require rename to return symbol occurrences because the IDE is 
> responsible for figuring out: 
> 1) The new name. Thus we can't produce replacements when renaming because we 
> don't know what the new name is when gathering the occurrences.
> 2) Whether these occurrences should be actually applied. Thus we can't 
> produce replacements because it's up to the user to decide if they want to 
> rename some occurrence in a comment for example.
> 
> In general 2) can be applied to tools like clang-refactor that could allow 
> users to select occurrences that don't guarantee a direct semantic match 
> (comments, etc.) in an interactive manner.
> 
> I think clangd has a rather naive rename support, so these points may not 
> apply, but we may want to extend clangd's support for rename in the future as 
> well.
I feel occurrences are more of an intermediate state of a refactoring action 
than a result. I'm wondering if it makes sense to introduce a separate class to 
represent such intermediate states? I am a bit nervous to fuse multiple classes 
into one; the interfaces can get pretty ugly when more result kinds are added. 



Comment at: lib/Tooling/Refactoring/SourceSelectionConstraints.cpp:13
+
+using namespace clang;
+using namespace tooling;

We are tempted to avoid `using namespace` if possible. 


Repository:
  rL LLVM

https://reviews.llvm.org/D36075



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

Re: r311601 - Fix a bug in CGDebugInfo::EmitInlineFunctionStart causing DILocations to be

[Hans Wennborg via cfe-commits]

Merged to 5.0 in r311671.

On Wed, Aug 23, 2017 at 2:24 PM, Adrian Prantl via cfe-commits
 wrote:
> Author: adrian
> Date: Wed Aug 23 14:24:12 2017
> New Revision: 311601
>
> URL: http://llvm.org/viewvc/llvm-project?rev=311601&view=rev
> Log:
> Fix a bug in CGDebugInfo::EmitInlineFunctionStart causing DILocations to be
> parented in function declarations.
>
> Fixes PR33997.
> https://bugs.llvm.org/show_bug.cgi?id=33997
>
> Added:
> cfe/trunk/test/CodeGenCXX/debug-info-inlined.cpp
> Modified:
> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=311601&r1=311600&r2=311601&view=diff
> ==
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Wed Aug 23 14:24:12 2017
> @@ -3287,7 +3287,7 @@ void CGDebugInfo::EmitInlineFunctionStar
>llvm::DISubprogram *SP = nullptr;
>if (FI != SPCache.end())
>  SP = dyn_cast_or_null(FI->second);
> -  if (!SP)
> +  if (!SP || !SP->isDefinition())
>  SP = getFunctionStub(GD);
>FnBeginRegionCount.push_back(LexicalBlockStack.size());
>LexicalBlockStack.emplace_back(SP);
>
> Added: cfe/trunk/test/CodeGenCXX/debug-info-inlined.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-inlined.cpp?rev=311601&view=auto
> ==
> --- cfe/trunk/test/CodeGenCXX/debug-info-inlined.cpp (added)
> +++ cfe/trunk/test/CodeGenCXX/debug-info-inlined.cpp Wed Aug 23 14:24:12 2017
> @@ -0,0 +1,45 @@
> +// RUN: %clang_cc1 -emit-llvm -triple i686-pc-windows-msvc19.0.24213 
> -gcodeview -debug-info-kind=limited -std=c++14 %s -o - | FileCheck %s
> +// PR33997.
> +struct already_AddRefed {
> +  ~already_AddRefed();
> +};
> +struct RefPtr {
> +  operator int *();
> +};
> +struct ServoCssRulesStrong {
> +  already_AddRefed Consume();
> +};
> +struct GroupRule {
> +  GroupRule(already_AddRefed);
> +};
> +class ConditionRule : GroupRule {
> +  using GroupRule::GroupRule;
> +};
> +class CSSMediaRule : ConditionRule {
> +  using ConditionRule::ConditionRule;
> +};
> +class CSSMozDocumentRule : ConditionRule {
> +  using ConditionRule::ConditionRule;
> +};
> +class ServoDocumentRule : CSSMozDocumentRule {
> +  ServoDocumentRule(RefPtr);
> +};
> +class ServoMediaRule : CSSMediaRule {
> +  ServoMediaRule(RefPtr);
> +};
> +ServoCssRulesStrong Servo_MediaRule_GetRules(int *);
> +ServoCssRulesStrong Servo_DocumentRule_GetRules(int *);
> +ServoDocumentRule::ServoDocumentRule(RefPtr aRawRule)
> +: CSSMozDocumentRule(Servo_DocumentRule_GetRules(aRawRule).Consume()) {}
> +
> +ServoMediaRule::ServoMediaRule(RefPtr aRawRule)
> +: CSSMediaRule(Servo_MediaRule_GetRules(aRawRule).Consume()) {}
> +
> +// CHECK: define{{.*}}ServoMediaRule
> +// CHECK-NOT: {{ ret }}
> +// CHECK: store %class.ConditionRule* %
> +// CHECK-SAME: %class.ConditionRule** %
> +// CHECK-SAME: !dbg ![[INL:[0-9]+]]
> +
> +// CHECK: ![[INL]] = !DILocation(line: 16, scope: ![[SP:[0-9]+]], inlinedAt:
> +// CHECK: ![[SP]] = distinct !DISubprogram(name: "GroupRule", 
> {{.*}}isDefinition: true
>
>
> ___
> 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

[PATCH] D37091: Expose -mllvm -accurate-sample-profile to clang.

[Dehao Chen via Phabricator via cfe-commits]

danielcdh updated this revision to Diff 112574.
danielcdh marked 2 inline comments as done.
danielcdh added a comment.

updated the patch to put it into function attribute so that it works with 
ThinLTO


https://reviews.llvm.org/D37091

Files:
  docs/ClangCommandLineReference.rst
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CodeGenFunction.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/Driver/clang_f_opts.c

Index: test/Driver/clang_f_opts.c
===
--- test/Driver/clang_f_opts.c
+++ test/Driver/clang_f_opts.c
@@ -53,6 +53,9 @@
 // CHECK-REROLL-LOOPS: "-freroll-loops"
 // CHECK-NO-REROLL-LOOPS-NOT: "-freroll-loops"
 
+// RUN: %clang -### -S -faccurate-sample-profile %s 2>&1 | FileCheck -check-prefix=CHECK-ACCURATE-SAMPLE-PROFILE %s
+// CHECK-ACCURATE-SAMPLE-PROFILE: "-faccurate-sample-profile"
+
 // RUN: %clang -### -S -fprofile-sample-use=%S/Inputs/file.prof %s 2>&1 | FileCheck -check-prefix=CHECK-SAMPLE-PROFILE %s
 // CHECK-SAMPLE-PROFILE: "-fprofile-sample-use={{.*}}/file.prof"
 
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -652,6 +652,8 @@
 
   Opts.NoUseJumpTables = Args.hasArg(OPT_fno_jump_tables);
 
+  Opts.AccurateSampleProfile = Args.hasArg(OPT_faccurate_sample_profile);
+
   Opts.PrepareForLTO = Args.hasArg(OPT_flto, OPT_flto_EQ);
   Opts.EmitSummaryIndex = false;
   if (Arg *A = Args.getLastArg(OPT_flto_EQ)) {
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2340,6 +2340,10 @@
 true))
 CmdArgs.push_back("-fno-jump-tables");
 
+  if (Args.hasFlag(options::OPT_faccurate_sample_profile,
+   options::OPT_fno_accurate_sample_profile, false))
+CmdArgs.push_back("-faccurate-sample-profile");
+
   if (!Args.hasFlag(options::OPT_fpreserve_as_comments,
 options::OPT_fno_preserve_as_comments, true))
 CmdArgs.push_back("-fno-preserve-as-comments");
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -837,6 +837,10 @@
   Fn->addFnAttr("no-jump-tables",
 llvm::toStringRef(CGM.getCodeGenOpts().NoUseJumpTables));
 
+  // Add accurate-sample-profile value.
+  if (CGM.getCodeGenOpts().AccurateSampleProfile)
+Fn->addFnAttr("accurate-sample-profile");
+
   if (getLangOpts().OpenCL) {
 // Add metadata for a kernel function.
 if (const FunctionDecl *FD = dyn_cast_or_null(D))
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -183,6 +183,7 @@
 CODEGENOPT(UnwindTables  , 1, 0) ///< Emit unwind tables.
 CODEGENOPT(VectorizeLoop , 1, 0) ///< Run loop vectorizer.
 CODEGENOPT(VectorizeSLP  , 1, 0) ///< Run SLP vectorizer.
+CODEGENOPT(AccurateSampleProfile, 1, 0) ///< Sample profile is accurate.
 
   /// Attempt to use register sized accesses to bit-fields in structures, when
   /// possible.
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -587,6 +587,14 @@
 def fPIE : Flag<["-"], "fPIE">, Group;
 def fno_PIE : Flag<["-"], "fno-PIE">, Group;
 def faccess_control : Flag<["-"], "faccess-control">, Group;
+def faccurate_sample_profile : Flag<["-"], "faccurate-sample-profile">,
+  Group, Flags<[DriverOption, CC1Option]>,
+  HelpText<"If sample profile is accurate, we will mark all un-sampled "
+   "callsite as cold. Otherwise, treat callsites without profile "
+   "samples as if we have no profile">;
+def fno_accurate_sample_profile : Flag<["-"], "fno-accurate-sample-profile">,
+  Group, Flags<[DriverOption]>;
+
 def fallow_unsupported : Flag<["-"], "fallow-unsupported">, Group;
 def fapple_kext : Flag<["-"], "fapple-kext">, Group, Flags<[CC1Option]>,
   HelpText<"Use Apple's kernel extensions ABI">;
@@ -643,6 +651,10 @@
 Alias;
 def fauto_profile_EQ : Joined<["-"], "fauto-profile=">,
 Alias;
+def fauto_profile_accurate : Flag<["-"], "fauto-profile-accurate">,
+Group, Alias;
+def fno_auto_profile_accurate : Flag<["-"], "fno-auto-profile-accurate">,
+Group, Alias;
 def fdebug_info_for_profiling : Flag<["-"], "fdebug-info-for-profiling">, Group,
 Flags<[CC1Option]>,
 HelpText<"Emit extra debug info to make sample profile more accurate.">;
Index: docs/ClangCommandLineReference.rst
===
--

[PATCH] D36075: [refactor] Initial support for refactoring action rules

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: include/clang/Tooling/Refactoring/RefactoringResult.h:21
+struct RefactoringResult {
+  enum ResultKind {
+/// A set of source replacements represented using a vector of

ioeric wrote:
> arphaman wrote:
> > ioeric wrote:
> > > I'm a bit unsure about the abstraction of the refactoring result. I would 
> > > expected refactoring results to be source changes always. Do you have any 
> > > refactoring tool that outputs occurrences in mind?
> > In Xcode we require rename to return symbol occurrences because the IDE is 
> > responsible for figuring out: 
> > 1) The new name. Thus we can't produce replacements when renaming because 
> > we don't know what the new name is when gathering the occurrences.
> > 2) Whether these occurrences should be actually applied. Thus we can't 
> > produce replacements because it's up to the user to decide if they want to 
> > rename some occurrence in a comment for example.
> > 
> > In general 2) can be applied to tools like clang-refactor that could allow 
> > users to select occurrences that don't guarantee a direct semantic match 
> > (comments, etc.) in an interactive manner.
> > 
> > I think clangd has a rather naive rename support, so these points may not 
> > apply, but we may want to extend clangd's support for rename in the future 
> > as well.
> I feel occurrences are more of an intermediate state of a refactoring action 
> than a result. I'm wondering if it makes sense to introduce a separate class 
> to represent such intermediate states? I am a bit nervous to fuse multiple 
> classes into one; the interfaces can get pretty ugly when more result kinds 
> are added. 
Good point. I agree.

I think it would be better to differentiate between `RefactoringActionRules` 
then. Ordinary rules return a set of AtomicChanges instead of 
RefactoringResult. But then we could also have "interactive" rules that return 
"partial" results like symbol occurrences.

I think I'll try the following approach:

```
class RefactoringActionRule {
  virtual ~RefactoringActionRule() {}
};

class RefactoringActionSourceChangeRule: public RefactoringActionRule {
public:
  virtual Expected>
  createSourceReplacements(RefactoringOperation &Operation) = 0;
};

class RefactoringActionSymbolOccurrencesRule: public RefactoringActionRule {
public:
  virtual Expected>
  findSymbolOccurrences(RefactoringOperation &Operation) = 0;
};
```


Repository:
  rL LLVM

https://reviews.llvm.org/D36075



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

[PATCH] D36075: [refactor] Initial support for refactoring action rules

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: lib/Tooling/Refactoring/SourceSelectionConstraints.cpp:13
+
+using namespace clang;
+using namespace tooling;

ioeric wrote:
> We are tempted to avoid `using namespace` if possible. 
Why? It's not in a header. `using namespace clang` is the common practice 
across all of Clang's sources.


Repository:
  rL LLVM

https://reviews.llvm.org/D36075



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

r311672 - [clang-format] Emit absolute splits before lines for comments, try 2

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Thu Aug 24 09:41:10 2017
New Revision: 311672

URL: http://llvm.org/viewvc/llvm-project?rev=311672&view=rev
Log:
[clang-format] Emit absolute splits before lines for comments, try 2

Summary:
This recommits https://reviews.llvm.org/D36956 with an update to the added test
case to not use raw string literals, since this makes gcc unhappy.

Reviewers: djasper

Reviewed By: djasper

Subscribers: cfe-commits, klimek

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

Modified:
cfe/trunk/lib/Format/BreakableToken.cpp
cfe/trunk/unittests/Format/FormatTestComments.cpp

Modified: cfe/trunk/lib/Format/BreakableToken.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=311672&r1=311671&r2=311672&view=diff
==
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Thu Aug 24 09:41:10 2017
@@ -545,15 +545,18 @@ void BreakableBlockComment::insertBreak(
 }
 
 BreakableToken::Split BreakableBlockComment::getSplitBefore(
-unsigned LineIndex,
-unsigned PreviousEndColumn,
-unsigned ColumnLimit,
+unsigned LineIndex, unsigned PreviousEndColumn, unsigned ColumnLimit,
 llvm::Regex &CommentPragmasRegex) const {
   if (!mayReflow(LineIndex, CommentPragmasRegex))
 return Split(StringRef::npos, 0);
   StringRef TrimmedContent = Content[LineIndex].ltrim(Blanks);
-  return getReflowSplit(TrimmedContent, ReflowPrefix, PreviousEndColumn,
-ColumnLimit);
+  Split Result = getReflowSplit(TrimmedContent, ReflowPrefix, 
PreviousEndColumn,
+ColumnLimit);
+  // Result is relative to TrimmedContent. Adapt it relative to
+  // Content[LineIndex].
+  if (Result.first != StringRef::npos)
+Result.first += Content[LineIndex].size() - TrimmedContent.size();
+  return Result;
 }
 
 unsigned BreakableBlockComment::getReflownColumn(
@@ -633,17 +636,12 @@ void BreakableBlockComment::replaceWhite
 /*CurrentPrefix=*/ReflowPrefix, InPPDirective, /*Newlines=*/0,
 /*Spaces=*/0);
 // Check if we need to also insert a break at the whitespace range.
-// For this we first adapt the reflow split relative to the beginning of 
the
-// content.
 // Note that we don't need a penalty for this break, since it doesn't 
change
 // the total number of lines.
-Split BreakSplit = SplitBefore;
-BreakSplit.first += TrimmedContent.data() - Content[LineIndex].data();
 unsigned ReflownColumn =
 getReflownColumn(TrimmedContent, LineIndex, PreviousEndColumn);
-if (ReflownColumn > ColumnLimit) {
-  insertBreak(LineIndex, 0, BreakSplit, Whitespaces);
-}
+if (ReflownColumn > ColumnLimit)
+  insertBreak(LineIndex, 0, SplitBefore, Whitespaces);
 return;
   }
 

Modified: cfe/trunk/unittests/Format/FormatTestComments.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestComments.cpp?rev=311672&r1=311671&r2=311672&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestComments.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestComments.cpp Thu Aug 24 09:41:10 2017
@@ -2787,6 +2787,24 @@ TEST_F(FormatTestComments, AlignsBlockCo
"* long */",
getLLVMStyleWithColumns(20)));
 }
+
+TEST_F(FormatTestComments, NoCrush_Bug34236) {
+  // This is a test case from a crasher reported in:
+  // https://bugs.llvm.org/show_bug.cgi?id=34236
+  // Temporarily disable formatting for readability.
+  // clang-format off
+  EXPECT_EQ(
+"/**/ /*\n"
+"  *   
a\n"
+"  * b c\n"
+"  * d*/",
+  format(
+"/**/ /*\n"
+" *   a b\n"
+" *   c d*/",
+  getLLVMStyleWithColumns(80)));
+  // clang-format on
+}
 } // end namespace
 } // end namespace format
 } // end namespace clang


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

[PATCH] D37109: [clang-format] Emit absolute splits before lines for comments, try 2

[Krasimir Georgiev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL311672: [clang-format] Emit absolute splits before lines for 
comments, try 2 (authored by krasimir).

Repository:
  rL LLVM

https://reviews.llvm.org/D37109

Files:
  cfe/trunk/lib/Format/BreakableToken.cpp
  cfe/trunk/unittests/Format/FormatTestComments.cpp


Index: cfe/trunk/lib/Format/BreakableToken.cpp
===
--- cfe/trunk/lib/Format/BreakableToken.cpp
+++ cfe/trunk/lib/Format/BreakableToken.cpp
@@ -545,15 +545,18 @@
 }
 
 BreakableToken::Split BreakableBlockComment::getSplitBefore(
-unsigned LineIndex,
-unsigned PreviousEndColumn,
-unsigned ColumnLimit,
+unsigned LineIndex, unsigned PreviousEndColumn, unsigned ColumnLimit,
 llvm::Regex &CommentPragmasRegex) const {
   if (!mayReflow(LineIndex, CommentPragmasRegex))
 return Split(StringRef::npos, 0);
   StringRef TrimmedContent = Content[LineIndex].ltrim(Blanks);
-  return getReflowSplit(TrimmedContent, ReflowPrefix, PreviousEndColumn,
-ColumnLimit);
+  Split Result = getReflowSplit(TrimmedContent, ReflowPrefix, 
PreviousEndColumn,
+ColumnLimit);
+  // Result is relative to TrimmedContent. Adapt it relative to
+  // Content[LineIndex].
+  if (Result.first != StringRef::npos)
+Result.first += Content[LineIndex].size() - TrimmedContent.size();
+  return Result;
 }
 
 unsigned BreakableBlockComment::getReflownColumn(
@@ -633,17 +636,12 @@
 /*CurrentPrefix=*/ReflowPrefix, InPPDirective, /*Newlines=*/0,
 /*Spaces=*/0);
 // Check if we need to also insert a break at the whitespace range.
-// For this we first adapt the reflow split relative to the beginning of 
the
-// content.
 // Note that we don't need a penalty for this break, since it doesn't 
change
 // the total number of lines.
-Split BreakSplit = SplitBefore;
-BreakSplit.first += TrimmedContent.data() - Content[LineIndex].data();
 unsigned ReflownColumn =
 getReflownColumn(TrimmedContent, LineIndex, PreviousEndColumn);
-if (ReflownColumn > ColumnLimit) {
-  insertBreak(LineIndex, 0, BreakSplit, Whitespaces);
-}
+if (ReflownColumn > ColumnLimit)
+  insertBreak(LineIndex, 0, SplitBefore, Whitespaces);
 return;
   }
 
Index: cfe/trunk/unittests/Format/FormatTestComments.cpp
===
--- cfe/trunk/unittests/Format/FormatTestComments.cpp
+++ cfe/trunk/unittests/Format/FormatTestComments.cpp
@@ -2787,6 +2787,24 @@
"* long */",
getLLVMStyleWithColumns(20)));
 }
+
+TEST_F(FormatTestComments, NoCrush_Bug34236) {
+  // This is a test case from a crasher reported in:
+  // https://bugs.llvm.org/show_bug.cgi?id=34236
+  // Temporarily disable formatting for readability.
+  // clang-format off
+  EXPECT_EQ(
+"/**/ /*\n"
+"  *   
a\n"
+"  * b c\n"
+"  * d*/",
+  format(
+"/**/ /*\n"
+" *   a b\n"
+" *   c d*/",
+  getLLVMStyleWithColumns(80)));
+  // clang-format on
+}
 } // end namespace
 } // end namespace format
 } // end namespace clang


Index: cfe/trunk/lib/Format/BreakableToken.cpp
===
--- cfe/trunk/lib/Format/BreakableToken.cpp
+++ cfe/trunk/lib/Format/BreakableToken.cpp
@@ -545,15 +545,18 @@
 }
 
 BreakableToken::Split BreakableBlockComment::getSplitBefore(
-unsigned LineIndex,
-unsigned PreviousEndColumn,
-unsigned ColumnLimit,
+unsigned LineIndex, unsigned PreviousEndColumn, unsigned ColumnLimit,
 llvm::Regex &CommentPragmasRegex) const {
   if (!mayReflow(LineIndex, CommentPragmasRegex))
 return Split(StringRef::npos, 0);
   StringRef TrimmedContent = Content[LineIndex].ltrim(Blanks);
-  return getReflowSplit(TrimmedContent, ReflowPrefix, PreviousEndColumn,
-ColumnLimit);
+  Split Result = getReflowSplit(TrimmedContent, ReflowPrefix, PreviousEndColumn,
+ColumnLimit);
+  // Result is relative to TrimmedContent. Adapt it relative to
+  // Content[LineIndex].
+  if (Result.first != StringRef::npos)
+Result.first += Content[LineIndex].size() - TrimmedContent.size();
+  return Result;
 }
 
 unsigned BreakableBlockComment::getReflownColumn(
@@ -633,17 +636,12 @@
 /*CurrentPrefix=*/ReflowPrefix, InPPDirective, /*Newlines=*/0,
 /*Spaces=*/0);
 // Check if we need to also insert a break at the whitespace range.
-// For this we first adapt the reflow split relative to the beginni

[PATCH] D37091: Expose -mllvm -accurate-sample-profile to clang.

[David Li via Phabricator via cfe-commits]

davidxl added a comment.

Looks fine to me, but please wait for Richard's comment.


https://reviews.llvm.org/D37091



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

[PATCH] D37090: Implement CFG construction for __finally.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Re: jumps out of __try, I wonder if you can tie __finally into whatever the CFG 
does for C++ destructors.




Comment at: test/Sema/warn-unreachable-ms.c:49
 __try {
-  f();
+  throw 1;
 } __except (1) {

Nice. Would any noreteurn call work here to eliminate the re-run and ifdef?


https://reviews.llvm.org/D37090



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

[PATCH] D37079: [Preprocessor] Correct internal token parsing of newline characters in CRLF

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: lib/Lex/Lexer.cpp:3076-3077
   case '\r':
+if (CurPtr[0] != Char && (CurPtr[0] == '\n' || CurPtr[0] == '\r'))
+  Char = getAndAdvanceChar(CurPtr, Result);
 // If we are inside a preprocessor directive and we see the end of line,

Should we only do this in the `\r` case? If I understand correctly, we're 
basically saying, if this is a CR, and the next byte is an LF, advance one more 
and do the pre-processor stuff.


https://reviews.llvm.org/D37079



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

[PATCH] D37079: [Preprocessor] Correct internal token parsing of newline characters in CRLF

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: lib/Lex/Lexer.cpp:3076-3077
   case '\r':
+if (CurPtr[0] != Char && (CurPtr[0] == '\n' || CurPtr[0] == '\r'))
+  Char = getAndAdvanceChar(CurPtr, Result);
 // If we are inside a preprocessor directive and we see the end of line,

rnk wrote:
> Should we only do this in the `\r` case? If I understand correctly, we're 
> basically saying, if this is a CR, and the next byte is an LF, advance one 
> more and do the pre-processor stuff.
That is exactly what we're doing.

I debated that personally, and am a bit on the fence.  It seems a number of 
places like to treat a '\r\n' and a '\n\r' as the same thing, though it seems  
a little foolish to me.  If you fall toward that opinion, I'll definitely 
change it, just say the word :)


https://reviews.llvm.org/D37079



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

Re: r311589 - [ubsan] PR34266: When sanitizing the 'this' value for a member function that happens to be a lambda call operator, use the lambda's 'this' pointer, not the captured enclosing 'this' poin

[Matt Morehouse via cfe-commits]

Hi Richard,

It looks like this revision is breaking the x86_64-linux-bootstrap bot
.
Most of the UBSan checks are failing with the attached error.
Full log at:
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/2104/steps/check-llvm%20ubsan/logs/stdio

I haven't looked in much detail, so I'm not sure if your change uncovered a
bug in LLVM or if the change is faulty.  Could you please take a look?

Thanks,
Matt Morehouse



On Wed, Aug 23, 2017 at 12:39 PM, Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rsmith
> Date: Wed Aug 23 12:39:04 2017
> New Revision: 311589
>
> URL: http://llvm.org/viewvc/llvm-project?rev=311589&view=rev
> Log:
> [ubsan] PR34266: When sanitizing the 'this' value for a member function
> that happens to be a lambda call operator, use the lambda's 'this' pointer,
> not the captured enclosing 'this' pointer (if any).
>
> Modified:
> cfe/trunk/include/clang/AST/DeclCXX.h
> cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
> cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp
>
> Modified: cfe/trunk/include/clang/AST/DeclCXX.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/AST/DeclCXX.h?rev=311589&r1=311588&r2=311589&view=diff
> 
> ==
> --- cfe/trunk/include/clang/AST/DeclCXX.h (original)
> +++ cfe/trunk/include/clang/AST/DeclCXX.h Wed Aug 23 12:39:04 2017
> @@ -2027,7 +2027,10 @@ public:
>
>/// \brief Returns the type of the \c this pointer.
>///
> -  /// Should only be called for instance (i.e., non-static) methods.
> +  /// Should only be called for instance (i.e., non-static) methods. Note
> +  /// that for the call operator of a lambda closure type, this returns
> the
> +  /// desugared 'this' type (a pointer to the closure type), not the
> captured
> +  /// 'this' type.
>QualType getThisType(ASTContext &C) const;
>
>unsigned getTypeQualifiers() const {
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/
> CodeGenFunction.cpp?rev=311589&r1=311588&r2=311589&view=diff
> 
> ==
> --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Aug 23 12:39:04 2017
> @@ -1014,11 +1014,11 @@ void CodeGenFunction::StartFunction(Glob
>  }
>
>  // Check the 'this' pointer once per function, if it's available.
> -if (CXXThisValue) {
> +if (CXXABIThisValue) {
>SanitizerSet SkippedChecks;
>SkippedChecks.set(SanitizerKind::ObjectSize, true);
>QualType ThisTy = MD->getThisType(getContext());
> -  EmitTypeCheck(TCK_Load, Loc, CXXThisValue, ThisTy,
> +  EmitTypeCheck(TCK_Load, Loc, CXXABIThisValue, ThisTy,
>  getContext().getTypeAlignInChars(ThisTy->
> getPointeeType()),
>  SkippedChecks);
>  }
>
> Modified: cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> CodeGenCXX/catch-undef-behavior.cpp?rev=311589&r1=
> 311588&r2=311589&view=diff
> 
> ==
> --- cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp Wed Aug 23
> 12:39:04 2017
> @@ -449,6 +449,27 @@ void upcast_to_vbase() {
>  }
>  }
>
> +struct ThisAlign {
> +  void this_align_lambda();
> +};
> +void ThisAlign::this_align_lambda() {
> +  // CHECK-LABEL: define {{.*}}@"_ZZN9ThisAlign17this_
> align_lambdaEvENK3$_0clEv"
> +  // CHECK-SAME: (%{{.*}}* %[[this:[^)]*]])
> +  // CHECK: %[[this_addr:.*]] = alloca
> +  // CHECK: store %{{.*}}* %[[this]], %{{.*}}** %[[this_addr]],
> +  // CHECK: %[[this_inner:.*]] = load %{{.*}}*, %{{.*}}** %[[this_addr]],
> +  // CHECK: %[[this_outer_addr:.*]] = getelementptr inbounds %{{.*}},
> %{{.*}}* %[[this_inner]], i32 0, i32 0
> +  // CHECK: %[[this_outer:.*]] = load %{{.*}}*, %{{.*}}**
> %[[this_outer_addr]],
> +  //
> +  // CHECK: %[[this_inner_isnonnull:.*]] = icmp ne %{{.*}}*
> %[[this_inner]], null
> +  // CHECK: %[[this_inner_asint:.*]] = ptrtoint %{{.*}}* %[[this_inner]]
> to i
> +  // CHECK: %[[this_inner_misalignment:.*]] = and i{{32|64}}
> %[[this_inner_asint]], {{3|7}},
> +  // CHECK: %[[this_inner_isaligned:.*]] = icmp eq i{{32|64}}
> %[[this_inner_misalignment]], 0
> +  // CHECK: %[[this_inner_valid:.*]] = and i1 %[[this_inner_isnonnull]],
> %[[this_inner_isaligned]],
> +  // CHECK: br i1 %[[this_inner_valid:.*]]
> +  [&] { return this; } ();
> +}
> +
>  namespace CopyValueRepresentation {
>// CHECK-LABEL: define {{.*}} @_ZN23CopyValueRepresentation2S3aSERKS0_
>// CHECK-NOT: call {{.*}} @__ubsan_handle_load_invalid_value
>
>
> 

[PATCH] D36272: [CodeGen][x86_64] Enable 'force_align_arg_pointer' attribute at x86_64

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Mind doing the diff with -U9 so I can see TargetInfo.cpp with the full 
context?


Repository:
  rL LLVM

https://reviews.llvm.org/D36272



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

[PATCH] D37079: [Preprocessor] Correct internal token parsing of newline characters in CRLF

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: lib/Lex/Lexer.cpp:3076-3077
   case '\r':
+if (CurPtr[0] != Char && (CurPtr[0] == '\n' || CurPtr[0] == '\r'))
+  Char = getAndAdvanceChar(CurPtr, Result);
 // If we are inside a preprocessor directive and we see the end of line,

erichkeane wrote:
> rnk wrote:
> > Should we only do this in the `\r` case? If I understand correctly, we're 
> > basically saying, if this is a CR, and the next byte is an LF, advance one 
> > more and do the pre-processor stuff.
> That is exactly what we're doing.
> 
> I debated that personally, and am a bit on the fence.  It seems a number of 
> places like to treat a '\r\n' and a '\n\r' as the same thing, though it seems 
>  a little foolish to me.  If you fall toward that opinion, I'll definitely 
> change it, just say the word :)
The bug probably doesn't happen in the \n\r case, because don't we count '\n's 
to compute our line numbers?

Anyway, yeah, I think we should make this specific to '\r'. In that case, we 
peek one ahead, and if we see a simple '\n' byte, we advance one more so that 
our line numbers stay correct.


https://reviews.llvm.org/D37079



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

[PATCH] D37079: [Preprocessor] Correct internal token parsing of newline characters in CRLF

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 112581.
erichkeane added a comment.

Switched to simply \r\n instead of both cases.  This fixes the issue, is likely 
faster (important, since this is a performance critical part of code), and a 
smaller-hammer.


https://reviews.llvm.org/D37079

Files:
  lib/Lex/Lexer.cpp
  test/Frontend/.gitattributes
  test/Frontend/system-header-line-directive-ms-lineendings.c


Index: test/Frontend/system-header-line-directive-ms-lineendings.c
===
--- /dev/null
+++ test/Frontend/system-header-line-directive-ms-lineendings.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 %s -E -o - -I %S/Inputs -isystem 
%S/Inputs/SystemHeaderPrefix | FileCheck %s
+#include 
+#include 
+
+#include "line-directive.h"
+
+// This tests that the line numbers for the current file are correctly 
outputted
+// for the include-file-completed test case.  
+
+// CHECK: # 1 "{{.*}}system-header-line-directive-ms-lineendings.c" 2
+// CHECK: # 1 "{{.*}}noline.h" 1 3
+// CHECK: foo();
+// CHECK: # 3 "{{.*}}system-header-line-directive-ms-lineendings.c" 2
+// CHECK: # 1 "{{.*}}line-directive-in-system.h" 1 3
+//  The "3" below indicates that "foo.h" is considered a system header.
+// CHECK: # 1 "foo.h" 3
+// CHECK: foo();
+// CHECK: # 4 "{{.*}}system-header-line-directive-ms-lineendings.c" 2
+// CHECK: # 1 "{{.*}}line-directive.h" 1
+// CHECK: # 10 "foo.h"{{$}}
+// CHECK: # 6 "{{.*}}system-header-line-directive-ms-lineendings.c" 2
Index: test/Frontend/.gitattributes
===
--- /dev/null
+++ test/Frontend/.gitattributes
@@ -0,0 +1,2 @@
+# Below test validates crlf line endings, so it should stay crlf.
+system-header-line-directive-ms-lineendings.c text eol=crlf
Index: lib/Lex/Lexer.cpp
===
--- lib/Lex/Lexer.cpp
+++ lib/Lex/Lexer.cpp
@@ -3070,9 +3070,12 @@
 // If Microsoft extensions are disabled, this is just random garbage.
 Kind = tok::unknown;
 break;
-  
-  case '\n':
+
   case '\r':
+if (CurPtr[0] == '\n')
+  Char = getAndAdvanceChar(CurPtr, Result);
+LLVM_FALLTHROUGH;
+  case '\n':
 // If we are inside a preprocessor directive and we see the end of line,
 // we know we are done with the directive, so return an EOD token.
 if (ParsingPreprocessorDirective) {


Index: test/Frontend/system-header-line-directive-ms-lineendings.c
===
--- /dev/null
+++ test/Frontend/system-header-line-directive-ms-lineendings.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 %s -E -o - -I %S/Inputs -isystem %S/Inputs/SystemHeaderPrefix | FileCheck %s
+#include 
+#include 
+
+#include "line-directive.h"
+
+// This tests that the line numbers for the current file are correctly outputted
+// for the include-file-completed test case.  
+
+// CHECK: # 1 "{{.*}}system-header-line-directive-ms-lineendings.c" 2
+// CHECK: # 1 "{{.*}}noline.h" 1 3
+// CHECK: foo();
+// CHECK: # 3 "{{.*}}system-header-line-directive-ms-lineendings.c" 2
+// CHECK: # 1 "{{.*}}line-directive-in-system.h" 1 3
+//  The "3" below indicates that "foo.h" is considered a system header.
+// CHECK: # 1 "foo.h" 3
+// CHECK: foo();
+// CHECK: # 4 "{{.*}}system-header-line-directive-ms-lineendings.c" 2
+// CHECK: # 1 "{{.*}}line-directive.h" 1
+// CHECK: # 10 "foo.h"{{$}}
+// CHECK: # 6 "{{.*}}system-header-line-directive-ms-lineendings.c" 2
Index: test/Frontend/.gitattributes
===
--- /dev/null
+++ test/Frontend/.gitattributes
@@ -0,0 +1,2 @@
+# Below test validates crlf line endings, so it should stay crlf.
+system-header-line-directive-ms-lineendings.c text eol=crlf
Index: lib/Lex/Lexer.cpp
===
--- lib/Lex/Lexer.cpp
+++ lib/Lex/Lexer.cpp
@@ -3070,9 +3070,12 @@
 // If Microsoft extensions are disabled, this is just random garbage.
 Kind = tok::unknown;
 break;
-  
-  case '\n':
+
   case '\r':
+if (CurPtr[0] == '\n')
+  Char = getAndAdvanceChar(CurPtr, Result);
+LLVM_FALLTHROUGH;
+  case '\n':
 // If we are inside a preprocessor directive and we see the end of line,
 // we know we are done with the directive, so return an EOD token.
 if (ParsingPreprocessorDirective) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37079: [Preprocessor] Correct internal token parsing of newline characters in CRLF

[Erich Keane via Phabricator via cfe-commits]

erichkeane marked 2 inline comments as done.
erichkeane added a comment.

Decided there were a few additional advantages to just handling \r\n, so Added 
them.


https://reviews.llvm.org/D37079



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

[PATCH] D37079: [Preprocessor] Correct internal token parsing of newline characters in CRLF

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm


https://reviews.llvm.org/D37079



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

[PATCH] D36272: [CodeGen][x86_64] Enable 'force_align_arg_pointer' attribute at x86_64

[Anatol Pomozov via Phabricator via cfe-commits]

anatol.pomozov updated this revision to Diff 112582.

https://reviews.llvm.org/D36272

Files:
  include/clang/Basic/Attr.td
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/function-attributes.c


Index: test/CodeGen/function-attributes.c
===
--- test/CodeGen/function-attributes.c
+++ test/CodeGen/function-attributes.c
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -triple i386-unknown-unknown -emit-llvm 
-disable-llvm-passes -Os -o - %s | FileCheck %s
 // RUN: %clang_cc1 -triple i386-unknown-unknown -emit-llvm 
-disable-llvm-passes -Os -std=c99 -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm 
-disable-llvm-passes -Os -std=c99 -o - %s | FileCheck %s
 // CHECK: define signext i8 @f0(i32 %x) [[NUW:#[0-9]+]]
 // CHECK: define zeroext i8 @f1(i32 %x) [[NUW]]
 // CHECK: define void @f2(i8 signext %x) [[NUW]]
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -2288,6 +2288,15 @@
 if (!IsForDefinition)
   return;
 if (const FunctionDecl *FD = dyn_cast_or_null(D)) {
+  if (FD->hasAttr()) {
+// Get the LLVM function.
+llvm::Function *Fn = cast(GV);
+
+// Now add the 'alignstack' attribute with a value of 16.
+llvm::AttrBuilder B;
+B.addStackAlignmentAttr(16);
+Fn->addAttributes(llvm::AttributeList::FunctionIndex, B);
+  }
   if (FD->hasAttr()) {
 llvm::Function *Fn = cast(GV);
 Fn->setCallingConv(llvm::CallingConv::X86_INTR);
@@ -2416,6 +2425,15 @@
   if (!IsForDefinition)
 return;
   if (const FunctionDecl *FD = dyn_cast_or_null(D)) {
+if (FD->hasAttr()) {
+  // Get the LLVM function.
+  llvm::Function *Fn = cast(GV);
+
+  // Now add the 'alignstack' attribute with a value of 16.
+  llvm::AttrBuilder B;
+  B.addStackAlignmentAttr(16);
+  Fn->addAttributes(llvm::AttributeList::FunctionIndex, B);
+}
 if (FD->hasAttr()) {
   llvm::Function *Fn = cast(GV);
   Fn->setCallingConv(llvm::CallingConv::X86_INTR);
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -2042,7 +2042,7 @@
   let Documentation = [AnyX86NoCallerSavedRegistersDocs];
 }
 
-def X86ForceAlignArgPointer : InheritableAttr, TargetSpecificAttr {
+def X86ForceAlignArgPointer : InheritableAttr, 
TargetSpecificAttr {
   let Spellings = [GNU<"force_align_arg_pointer">];
   // Technically, this appertains to a FunctionDecl, but the target-specific
   // code silently allows anything function-like (such as typedefs or function


Index: test/CodeGen/function-attributes.c
===
--- test/CodeGen/function-attributes.c
+++ test/CodeGen/function-attributes.c
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -triple i386-unknown-unknown -emit-llvm -disable-llvm-passes -Os -o - %s | FileCheck %s
 // RUN: %clang_cc1 -triple i386-unknown-unknown -emit-llvm -disable-llvm-passes -Os -std=c99 -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -disable-llvm-passes -Os -std=c99 -o - %s | FileCheck %s
 // CHECK: define signext i8 @f0(i32 %x) [[NUW:#[0-9]+]]
 // CHECK: define zeroext i8 @f1(i32 %x) [[NUW]]
 // CHECK: define void @f2(i8 signext %x) [[NUW]]
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -2288,6 +2288,15 @@
 if (!IsForDefinition)
   return;
 if (const FunctionDecl *FD = dyn_cast_or_null(D)) {
+  if (FD->hasAttr()) {
+// Get the LLVM function.
+llvm::Function *Fn = cast(GV);
+
+// Now add the 'alignstack' attribute with a value of 16.
+llvm::AttrBuilder B;
+B.addStackAlignmentAttr(16);
+Fn->addAttributes(llvm::AttributeList::FunctionIndex, B);
+  }
   if (FD->hasAttr()) {
 llvm::Function *Fn = cast(GV);
 Fn->setCallingConv(llvm::CallingConv::X86_INTR);
@@ -2416,6 +2425,15 @@
   if (!IsForDefinition)
 return;
   if (const FunctionDecl *FD = dyn_cast_or_null(D)) {
+if (FD->hasAttr()) {
+  // Get the LLVM function.
+  llvm::Function *Fn = cast(GV);
+
+  // Now add the 'alignstack' attribute with a value of 16.
+  llvm::AttrBuilder B;
+  B.addStackAlignmentAttr(16);
+  Fn->addAttributes(llvm::AttributeList::FunctionIndex, B);
+}
 if (FD->hasAttr()) {
   llvm::Function *Fn = cast(GV);
   Fn->setCallingConv(llvm::CallingConv::X86_INTR);
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -2042,7 +2042,7 @@
   let Documentation = [AnyX86NoCallerSavedRegistersDocs];
 }
 
-def X86Fo

[PATCH] D36272: [CodeGen][x86_64] Enable 'force_align_arg_pointer' attribute at x86_64

[Anatol Pomozov via Phabricator via cfe-commits]

anatol.pomozov added a comment.

Erich, done.

I also rerun tests and this time they are green.


https://reviews.llvm.org/D36272



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

[PATCH] D36564: [analyzer] Fix SimpleSValBuilder::simplifySVal

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexshap added a comment.

@alexfh, thanks for letting me know, i will take a closer look at 
https://bugs.llvm.org/show_bug.cgi?id=34309 soon.


Repository:
  rL LLVM

https://reviews.llvm.org/D36564



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

[PATCH] D36272: [CodeGen][x86_64] Enable 'force_align_arg_pointer' attribute at x86_64

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Are we SURE the stack alignment for this type is supposed to be 16 bit as well? 
 I didn't see any discussion about it in the email conversation.

I have very little understanding of this attribute, but I would (perhaps 
naiively) presume that it would be different on 64 bit targets.

Additionally, there is likely value to split the test run-line into 2, one that 
specifies 64 bit Windows and one that is 64 bit Linux.  The value being that 
they validate two different code paths (whereas 32 is the same code path).


https://reviews.llvm.org/D36272



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

Re: r311589 - [ubsan] PR34266: When sanitizing the 'this' value for a member function that happens to be a lambda call operator, use the lambda's 'this' pointer, not the captured enclosing 'this' poin

[Adrian Prantl via cfe-commits]

It looks like this broke / found errors on the green dragon bot:

http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan_check/4115/consoleFull#15752874848254eaf0-7326-4999-85b0-388101f2d404

 TEST 'LLVM-Unit :: 
ADT/./ADTTests/FilterIteratorTest.FunctionPointer' FAILED 

Note: Google Test filter = FilterIteratorTest.FunctionPointer
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from FilterIteratorTest
[ RUN  ] FilterIteratorTest.FunctionPointer
/Users/buildslave/jenkins/sharedspace/clang-stage2-cmake-RgSan@2/llvm/unittests/ADT/IteratorTest.cpp:160:24:
 runtime error: load of null pointer of type 'const (lambda at 
/Users/buildslave/jenkins/sharedspace/clang-stage2-cmake-RgSan@2/llvm/unittests/ADT/IteratorTest.cpp:160:24)
 *'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior 
/Users/buildslave/jenkins/sharedspace/clang-stage2-cmake-RgSan@2/llvm/unittests/ADT/IteratorTest.cpp:160:24
 in 



-- adrian
> On Aug 23, 2017, at 12:39 PM, Richard Smith via cfe-commits 
>  wrote:
> 
> Author: rsmith
> Date: Wed Aug 23 12:39:04 2017
> New Revision: 311589
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=311589&view=rev
> Log:
> [ubsan] PR34266: When sanitizing the 'this' value for a member function that 
> happens to be a lambda call operator, use the lambda's 'this' pointer, not 
> the captured enclosing 'this' pointer (if any).
> 
> Modified:
>cfe/trunk/include/clang/AST/DeclCXX.h
>cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
>cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp
> 
> Modified: cfe/trunk/include/clang/AST/DeclCXX.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=311589&r1=311588&r2=311589&view=diff
> ==
> --- cfe/trunk/include/clang/AST/DeclCXX.h (original)
> +++ cfe/trunk/include/clang/AST/DeclCXX.h Wed Aug 23 12:39:04 2017
> @@ -2027,7 +2027,10 @@ public:
> 
>   /// \brief Returns the type of the \c this pointer.
>   ///
> -  /// Should only be called for instance (i.e., non-static) methods.
> +  /// Should only be called for instance (i.e., non-static) methods. Note
> +  /// that for the call operator of a lambda closure type, this returns the
> +  /// desugared 'this' type (a pointer to the closure type), not the captured
> +  /// 'this' type.
>   QualType getThisType(ASTContext &C) const;
> 
>   unsigned getTypeQualifiers() const {
> 
> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=311589&r1=311588&r2=311589&view=diff
> ==
> --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Aug 23 12:39:04 2017
> @@ -1014,11 +1014,11 @@ void CodeGenFunction::StartFunction(Glob
> }
> 
> // Check the 'this' pointer once per function, if it's available.
> -if (CXXThisValue) {
> +if (CXXABIThisValue) {
>   SanitizerSet SkippedChecks;
>   SkippedChecks.set(SanitizerKind::ObjectSize, true);
>   QualType ThisTy = MD->getThisType(getContext());
> -  EmitTypeCheck(TCK_Load, Loc, CXXThisValue, ThisTy,
> +  EmitTypeCheck(TCK_Load, Loc, CXXABIThisValue, ThisTy,
> 
> getContext().getTypeAlignInChars(ThisTy->getPointeeType()),
> SkippedChecks);
> }
> 
> Modified: cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp?rev=311589&r1=311588&r2=311589&view=diff
> ==
> --- cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp Wed Aug 23 12:39:04 
> 2017
> @@ -449,6 +449,27 @@ void upcast_to_vbase() {
> }
> }
> 
> +struct ThisAlign {
> +  void this_align_lambda();
> +};
> +void ThisAlign::this_align_lambda() {
> +  // CHECK-LABEL: define 
> {{.*}}@"_ZZN9ThisAlign17this_align_lambdaEvENK3$_0clEv"
> +  // CHECK-SAME: (%{{.*}}* %[[this:[^)]*]])
> +  // CHECK: %[[this_addr:.*]] = alloca
> +  // CHECK: store %{{.*}}* %[[this]], %{{.*}}** %[[this_addr]],
> +  // CHECK: %[[this_inner:.*]] = load %{{.*}}*, %{{.*}}** %[[this_addr]],
> +  // CHECK: %[[this_outer_addr:.*]] = getelementptr inbounds %{{.*}}, 
> %{{.*}}* %[[this_inner]], i32 0, i32 0
> +  // CHECK: %[[this_outer:.*]] = load %{{.*}}*, %{{.*}}** 
> %[[this_outer_addr]],
> +  //
> +  // CHECK: %[[this_inner_isnonnull:.*]] = icmp ne %{{.*}}* %[[this_inner]], 
> null
> +  // CHECK: %[[this_inner_asint:.*]] = ptrtoint %{{.*}}* %[[this_inner]] to i
> +  // CHECK: %[[this_inner_misalignment:.*]] = and i{{32|64}} 
> %[[this_inner_asint]], {{3|7}},
> +  // CHECK: %[[this_inner_isaligned:.*]]

[PATCH] D36501: add flag to undo ABI change in r310401

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

rsmith added a comment.

In https://reviews.llvm.org/D36501#836257, @rjmccall wrote:

> Yeah, I think having an internal C++ ABI version makes a lot more sense than 
> having a million different flags.  Is there a reason to expose this as a knob 
> to users at all?


I don't see any reason anyone would want to use this other than to restore the 
ABI to that of Clang <= 4, so an overall "clang ABI version" flag would seem 
reasonable to me.


Repository:
  rL LLVM

https://reviews.llvm.org/D36501



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

Re: r298574 - Fix issues in clang-format's AlignConsecutive modes.

[Hans Wennborg via cfe-commits]

For reference, this was reviewed in https://reviews.llvm.org/D21279

(Please always include review links in the future.)

On Wed, Mar 22, 2017 at 7:51 PM, Nikola Smiljanic via cfe-commits
 wrote:
> Author: nikola
> Date: Wed Mar 22 21:51:25 2017
> New Revision: 298574
>
> URL: http://llvm.org/viewvc/llvm-project?rev=298574&view=rev
> Log:
> Fix issues in clang-format's AlignConsecutive modes.
>
> Patch by Ben Harper.
>
> Modified:
> cfe/trunk/lib/Format/WhitespaceManager.cpp
> cfe/trunk/lib/Format/WhitespaceManager.h
> cfe/trunk/unittests/Format/FormatTest.cpp
>
> Modified: cfe/trunk/lib/Format/WhitespaceManager.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.cpp?rev=298574&r1=298573&r2=298574&view=diff
> ==
> --- cfe/trunk/lib/Format/WhitespaceManager.cpp (original)
> +++ cfe/trunk/lib/Format/WhitespaceManager.cpp Wed Mar 22 21:51:25 2017
> @@ -50,9 +50,9 @@ void WhitespaceManager::replaceWhitespac
>if (Tok.Finalized)
>  return;
>Tok.Decision = (Newlines > 0) ? FD_Break : FD_Continue;
> -  Changes.push_back(Change(Tok, /*CreateReplacement=*/true,
> -   Tok.WhitespaceRange, Spaces, StartOfTokenColumn,
> -   Newlines, "", "", InPPDirective && !Tok.IsFirst,
> +  Changes.push_back(Change(Tok, /*CreateReplacement=*/true, 
> Tok.WhitespaceRange,
> +   Spaces, StartOfTokenColumn, Newlines, "", "",
> +   InPPDirective && !Tok.IsFirst,
> /*IsInsideToken=*/false));
>  }
>
> @@ -192,21 +192,56 @@ AlignTokenSequence(unsigned Start, unsig
> SmallVector &Changes) {
>bool FoundMatchOnLine = false;
>int Shift = 0;
> +
> +  // ScopeStack keeps track of the current scope depth. It contains indices 
> of
> +  // the first token on each scope.
> +  // We only run the "Matches" function on tokens from the outer-most scope.
> +  // However, we do need to pay special attention to one class of tokens
> +  // that are not in the outer-most scope, and that is function parameters
> +  // which are split across multiple lines, as illustrated by this example:
> +  //   double a(int x);
> +  //   intb(int  y,
> +  //  double z);
> +  // In the above example, we need to take special care to ensure that
> +  // 'double z' is indented along with it's owning function 'b'.
> +  SmallVector ScopeStack;
> +
>for (unsigned i = Start; i != End; ++i) {
> -if (Changes[i].NewlinesBefore > 0) {
> -  FoundMatchOnLine = false;
> +if (ScopeStack.size() != 0 &&
> +Changes[i].nestingAndIndentLevel() <
> +Changes[ScopeStack.back()].nestingAndIndentLevel())
> +  ScopeStack.pop_back();
> +
> +if (i != Start && Changes[i].nestingAndIndentLevel() >
> +  Changes[i - 1].nestingAndIndentLevel())
> +  ScopeStack.push_back(i);
> +
> +bool InsideNestedScope = ScopeStack.size() != 0;
> +
> +if (Changes[i].NewlinesBefore > 0 && !InsideNestedScope) {
>Shift = 0;
> +  FoundMatchOnLine = false;
>  }
>
>  // If this is the first matching token to be aligned, remember by how 
> many
>  // spaces it has to be shifted, so the rest of the changes on the line 
> are
>  // shifted by the same amount
> -if (!FoundMatchOnLine && Matches(Changes[i])) {
> +if (!FoundMatchOnLine && !InsideNestedScope && Matches(Changes[i])) {
>FoundMatchOnLine = true;
>Shift = Column - Changes[i].StartOfTokenColumn;
>Changes[i].Spaces += Shift;
>  }
>
> +// This is for function parameters that are split across multiple lines,
> +// as mentioned in the ScopeStack comment.
> +if (InsideNestedScope && Changes[i].NewlinesBefore > 0) {
> +  unsigned ScopeStart = ScopeStack.back();
> +  if (Changes[ScopeStart - 1].Tok->is(TT_FunctionDeclarationName) ||
> +  (ScopeStart > Start + 1 &&
> +   Changes[ScopeStart - 2].Tok->is(TT_FunctionDeclarationName)))
> +Changes[i].Spaces += Shift;
> +}
> +
>  assert(Shift >= 0);
>  Changes[i].StartOfTokenColumn += Shift;
>  if (i + 1 != Changes.size())
> @@ -214,15 +249,37 @@ AlignTokenSequence(unsigned Start, unsig
>}
>  }
>
> -// Walk through all of the changes and find sequences of matching tokens to
> -// align. To do so, keep track of the lines and whether or not a matching 
> token
> -// was found on a line. If a matching token is found, extend the current
> -// sequence. If the current line cannot be part of a sequence, e.g. because
> -// there is an empty line before it or it contains only non-matching tokens,
> -// finalize the previous sequence.
> +// Walk through a subset of the changes, starting at StartAt, and find
> +// sequences of matching tokens to align. To do so, keep track of the lines 
> and
> +// whether or not a matching token was found on a l

[PATCH] D36075: [refactor] Initial support for refactoring action rules

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexshap added a comment.

@arphaman

  The selection requirement is supposed to be orthogonal to AST matchers, not a 
replacement. It should be used when working with source selection in editors. 
  I did mess around with moving over clang-reorder-fields using this kind of 
model and didn't see any issues when using AST matchers. Essentially I used the 
 requiredOption requirements and mapped them to run my 
  matching code instead of using the selection requirement. Thus this 
requirement was satisfied only when the AST matchers were successful. 
  It might be possible to simplify that pattern even further to make it simpler.

that's great, i'm interested in this too and would be happy to see 
clang-reorder-fields moving to clang-refactor (pls, let me know if i can help 
make this happen)


Repository:
  rL LLVM

https://reviews.llvm.org/D36075



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

r311680 - Revert "[ubsan] PR34266: When sanitizing the 'this' value for a member function that happens to be a lambda call operator, use the lambda's 'this' pointer, not the captured enclosing 'this'

[Adrian Prantl via cfe-commits]

Author: adrian
Date: Thu Aug 24 11:18:24 2017
New Revision: 311680

URL: http://llvm.org/viewvc/llvm-project?rev=311680&view=rev
Log:
Revert "[ubsan] PR34266: When sanitizing the 'this' value for a member function 
that happens to be a lambda call operator, use the lambda's 'this' pointer, not 
the captured enclosing 'this' pointer (if any)."

This reverts commit r311589 because of bot breakage.
http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan_check/4115/consoleFull#15752874848254eaf0-7326-4999-85b0-388101f2d404.

Modified:
cfe/trunk/include/clang/AST/DeclCXX.h
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=311680&r1=311679&r2=311680&view=diff
==
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Thu Aug 24 11:18:24 2017
@@ -2027,10 +2027,7 @@ public:
 
   /// \brief Returns the type of the \c this pointer.
   ///
-  /// Should only be called for instance (i.e., non-static) methods. Note
-  /// that for the call operator of a lambda closure type, this returns the
-  /// desugared 'this' type (a pointer to the closure type), not the captured
-  /// 'this' type.
+  /// Should only be called for instance (i.e., non-static) methods.
   QualType getThisType(ASTContext &C) const;
 
   unsigned getTypeQualifiers() const {

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=311680&r1=311679&r2=311680&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Thu Aug 24 11:18:24 2017
@@ -1014,11 +1014,11 @@ void CodeGenFunction::StartFunction(Glob
 }
 
 // Check the 'this' pointer once per function, if it's available.
-if (CXXABIThisValue) {
+if (CXXThisValue) {
   SanitizerSet SkippedChecks;
   SkippedChecks.set(SanitizerKind::ObjectSize, true);
   QualType ThisTy = MD->getThisType(getContext());
-  EmitTypeCheck(TCK_Load, Loc, CXXABIThisValue, ThisTy,
+  EmitTypeCheck(TCK_Load, Loc, CXXThisValue, ThisTy,
 getContext().getTypeAlignInChars(ThisTy->getPointeeType()),
 SkippedChecks);
 }

Modified: cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp?rev=311680&r1=311679&r2=311680&view=diff
==
--- cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp Thu Aug 24 11:18:24 2017
@@ -449,27 +449,6 @@ void upcast_to_vbase() {
 }
 }
 
-struct ThisAlign {
-  void this_align_lambda();
-};
-void ThisAlign::this_align_lambda() {
-  // CHECK-LABEL: define 
{{.*}}@"_ZZN9ThisAlign17this_align_lambdaEvENK3$_0clEv"
-  // CHECK-SAME: (%{{.*}}* %[[this:[^)]*]])
-  // CHECK: %[[this_addr:.*]] = alloca
-  // CHECK: store %{{.*}}* %[[this]], %{{.*}}** %[[this_addr]],
-  // CHECK: %[[this_inner:.*]] = load %{{.*}}*, %{{.*}}** %[[this_addr]],
-  // CHECK: %[[this_outer_addr:.*]] = getelementptr inbounds %{{.*}}, %{{.*}}* 
%[[this_inner]], i32 0, i32 0
-  // CHECK: %[[this_outer:.*]] = load %{{.*}}*, %{{.*}}** %[[this_outer_addr]],
-  //
-  // CHECK: %[[this_inner_isnonnull:.*]] = icmp ne %{{.*}}* %[[this_inner]], 
null
-  // CHECK: %[[this_inner_asint:.*]] = ptrtoint %{{.*}}* %[[this_inner]] to i
-  // CHECK: %[[this_inner_misalignment:.*]] = and i{{32|64}} 
%[[this_inner_asint]], {{3|7}},
-  // CHECK: %[[this_inner_isaligned:.*]] = icmp eq i{{32|64}} 
%[[this_inner_misalignment]], 0
-  // CHECK: %[[this_inner_valid:.*]] = and i1 %[[this_inner_isnonnull]], 
%[[this_inner_isaligned]],
-  // CHECK: br i1 %[[this_inner_valid:.*]]
-  [&] { return this; } ();
-}
-
 namespace CopyValueRepresentation {
   // CHECK-LABEL: define {{.*}} @_ZN23CopyValueRepresentation2S3aSERKS0_
   // CHECK-NOT: call {{.*}} @__ubsan_handle_load_invalid_value


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

Re: r311589 - [ubsan] PR34266: When sanitizing the 'this' value for a member function that happens to be a lambda call operator, use the lambda's 'this' pointer, not the captured enclosing 'this' poin

[Adrian Prantl via cfe-commits]

I temporarily reverted the commit in r311680 to get the bots going again.

-- adrian

> On Aug 24, 2017, at 11:12 AM, Adrian Prantl via cfe-commits 
>  wrote:
> 
> It looks like this broke / found errors on the green dragon bot:
> 
> http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan_check/4115/consoleFull#15752874848254eaf0-7326-4999-85b0-388101f2d404
> 
>  TEST 'LLVM-Unit :: 
> ADT/./ADTTests/FilterIteratorTest.FunctionPointer' FAILED 
> 
> Note: Google Test filter = FilterIteratorTest.FunctionPointer
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from FilterIteratorTest
> [ RUN  ] FilterIteratorTest.FunctionPointer
> /Users/buildslave/jenkins/sharedspace/clang-stage2-cmake-RgSan@2/llvm/unittests/ADT/IteratorTest.cpp:160:24:
>  runtime error: load of null pointer of type 'const (lambda at 
> /Users/buildslave/jenkins/sharedspace/clang-stage2-cmake-RgSan@2/llvm/unittests/ADT/IteratorTest.cpp:160:24)
>  *'
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior 
> /Users/buildslave/jenkins/sharedspace/clang-stage2-cmake-RgSan@2/llvm/unittests/ADT/IteratorTest.cpp:160:24
>  in 
> 
> 
> 
> -- adrian
>> On Aug 23, 2017, at 12:39 PM, Richard Smith via cfe-commits 
>>  wrote:
>> 
>> Author: rsmith
>> Date: Wed Aug 23 12:39:04 2017
>> New Revision: 311589
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=311589&view=rev
>> Log:
>> [ubsan] PR34266: When sanitizing the 'this' value for a member function that 
>> happens to be a lambda call operator, use the lambda's 'this' pointer, not 
>> the captured enclosing 'this' pointer (if any).
>> 
>> Modified:
>>   cfe/trunk/include/clang/AST/DeclCXX.h
>>   cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
>>   cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp
>> 
>> Modified: cfe/trunk/include/clang/AST/DeclCXX.h
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=311589&r1=311588&r2=311589&view=diff
>> ==
>> --- cfe/trunk/include/clang/AST/DeclCXX.h (original)
>> +++ cfe/trunk/include/clang/AST/DeclCXX.h Wed Aug 23 12:39:04 2017
>> @@ -2027,7 +2027,10 @@ public:
>> 
>>  /// \brief Returns the type of the \c this pointer.
>>  ///
>> -  /// Should only be called for instance (i.e., non-static) methods.
>> +  /// Should only be called for instance (i.e., non-static) methods. Note
>> +  /// that for the call operator of a lambda closure type, this returns the
>> +  /// desugared 'this' type (a pointer to the closure type), not the 
>> captured
>> +  /// 'this' type.
>>  QualType getThisType(ASTContext &C) const;
>> 
>>  unsigned getTypeQualifiers() const {
>> 
>> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=311589&r1=311588&r2=311589&view=diff
>> ==
>> --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Aug 23 12:39:04 2017
>> @@ -1014,11 +1014,11 @@ void CodeGenFunction::StartFunction(Glob
>>}
>> 
>>// Check the 'this' pointer once per function, if it's available.
>> -if (CXXThisValue) {
>> +if (CXXABIThisValue) {
>>  SanitizerSet SkippedChecks;
>>  SkippedChecks.set(SanitizerKind::ObjectSize, true);
>>  QualType ThisTy = MD->getThisType(getContext());
>> -  EmitTypeCheck(TCK_Load, Loc, CXXThisValue, ThisTy,
>> +  EmitTypeCheck(TCK_Load, Loc, CXXABIThisValue, ThisTy,
>>
>> getContext().getTypeAlignInChars(ThisTy->getPointeeType()),
>>SkippedChecks);
>>}
>> 
>> Modified: cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp?rev=311589&r1=311588&r2=311589&view=diff
>> ==
>> --- cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp (original)
>> +++ cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp Wed Aug 23 12:39:04 
>> 2017
>> @@ -449,6 +449,27 @@ void upcast_to_vbase() {
>> }
>> }
>> 
>> +struct ThisAlign {
>> +  void this_align_lambda();
>> +};
>> +void ThisAlign::this_align_lambda() {
>> +  // CHECK-LABEL: define 
>> {{.*}}@"_ZZN9ThisAlign17this_align_lambdaEvENK3$_0clEv"
>> +  // CHECK-SAME: (%{{.*}}* %[[this:[^)]*]])
>> +  // CHECK: %[[this_addr:.*]] = alloca
>> +  // CHECK: store %{{.*}}* %[[this]], %{{.*}}** %[[this_addr]],
>> +  // CHECK: %[[this_inner:.*]] = load %{{.*}}*, %{{.*}}** %[[this_addr]],
>> +  // CHECK: %[[this_outer_addr:.*]] = getelementptr inbounds %{{.*}}, 
>> %{{.*}}* %[[this_inner]], i32 0, i32 0
>> +  // CHECK: %[[this_outer:.*]] = load %{{.*}}*, %{{.*}}** 
>> %[[this_outer_addr]],
>> +  //
>> +  // CHECK: %[[this_inner

[PATCH] D37115: [coroutines] Do not attempt to typo-correct when coroutine is looking for required members

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov created this revision.

When SemaCoroutine looks for await_resume, it means it. No need for helpful: 
"Did you mean await_ready?" messages.

Fixes PR33477 and a couple of FIXMEs in test/SemaCXX/coroutines.cpp


https://reviews.llvm.org/D37115

Files:
  lib/Sema/SemaCoroutine.cpp
  test/SemaCXX/coroutines.cpp


Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -500,8 +500,7 @@
 
 struct bad_promise_2 {
   coro get_return_object();
-  // FIXME: We shouldn't offer a typo-correction here!
-  suspend_always final_suspend(); // expected-note {{here}}
+  suspend_always final_suspend();
   void unhandled_exception();
   void return_void();
 };
@@ -512,8 +511,7 @@
 
 struct bad_promise_3 {
   coro get_return_object();
-  // FIXME: We shouldn't offer a typo-correction here!
-  suspend_always initial_suspend(); // expected-note {{here}}
+  suspend_always initial_suspend();
   void unhandled_exception();
   void return_void();
 };
@@ -1162,3 +1160,22 @@
 template CoroMemberTag DepTestType::test_static_template(const char 
*volatile &, unsigned);
 
 } // namespace CoroHandleMemberFunctionTest
+
+struct missing_await_ready {
+  void await_suspend(std::experimental::coroutine_handle<>);
+  void await_resume();
+};
+struct missing_await_suspend {
+  bool await_ready();
+  void await_resume();
+};
+struct missing_await_resume {
+  bool await_ready();
+  void await_suspend(std::experimental::coroutine_handle<>);
+};
+
+void test_missing_awaitable_members() {
+  co_await missing_await_ready{}; // expected-error {{no member named 
'await_ready' in 'missing_await_ready'}}
+  co_await missing_await_suspend{}; // expected-error {{no member named 
'await_suspend' in 'missing_await_suspend'}}
+  co_await missing_await_resume{}; // expected-error {{no member named 
'await_resume' in 'missing_await_resume'}}
+}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -360,6 +360,15 @@
   if (Result.isInvalid())
 return ExprError();
 
+  // We meant exactly what we asked for. No need for typo correction.
+  if (auto *TE = dyn_cast(Result.get())) {
+S.clearDelayedTypo(TE);
+S.Diag(Loc, diag::err_no_member)
+<< NameInfo.getName() << Base->getType()->getAsCXXRecordDecl()
+<< Base->getSourceRange();
+return ExprError();
+  }
+
   return S.ActOnCallExpr(nullptr, Result.get(), Loc, Args, Loc, nullptr);
 }
 


Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -500,8 +500,7 @@
 
 struct bad_promise_2 {
   coro get_return_object();
-  // FIXME: We shouldn't offer a typo-correction here!
-  suspend_always final_suspend(); // expected-note {{here}}
+  suspend_always final_suspend();
   void unhandled_exception();
   void return_void();
 };
@@ -512,8 +511,7 @@
 
 struct bad_promise_3 {
   coro get_return_object();
-  // FIXME: We shouldn't offer a typo-correction here!
-  suspend_always initial_suspend(); // expected-note {{here}}
+  suspend_always initial_suspend();
   void unhandled_exception();
   void return_void();
 };
@@ -1162,3 +1160,22 @@
 template CoroMemberTag DepTestType::test_static_template(const char *volatile &, unsigned);
 
 } // namespace CoroHandleMemberFunctionTest
+
+struct missing_await_ready {
+  void await_suspend(std::experimental::coroutine_handle<>);
+  void await_resume();
+};
+struct missing_await_suspend {
+  bool await_ready();
+  void await_resume();
+};
+struct missing_await_resume {
+  bool await_ready();
+  void await_suspend(std::experimental::coroutine_handle<>);
+};
+
+void test_missing_awaitable_members() {
+  co_await missing_await_ready{}; // expected-error {{no member named 'await_ready' in 'missing_await_ready'}}
+  co_await missing_await_suspend{}; // expected-error {{no member named 'await_suspend' in 'missing_await_suspend'}}
+  co_await missing_await_resume{}; // expected-error {{no member named 'await_resume' in 'missing_await_resume'}}
+}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -360,6 +360,15 @@
   if (Result.isInvalid())
 return ExprError();
 
+  // We meant exactly what we asked for. No need for typo correction.
+  if (auto *TE = dyn_cast(Result.get())) {
+S.clearDelayedTypo(TE);
+S.Diag(Loc, diag::err_no_member)
+<< NameInfo.getName() << Base->getType()->getAsCXXRecordDecl()
+<< Base->getSourceRange();
+return ExprError();
+  }
+
   return S.ActOnCallExpr(nullptr, Result.get(), Loc, Args, Loc, nullptr);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/

r311683 - [Preprocessor] Correct internal token parsing of newline characters in CRLF

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Thu Aug 24 11:36:07 2017
New Revision: 311683

URL: http://llvm.org/viewvc/llvm-project?rev=311683&view=rev
Log:
[Preprocessor] Correct internal token parsing of newline characters in CRLF

Discovered due to a goofy git setup, the test system-headerline-directive.c 
(and a few others) failed because the token-consumption will consume only the 
'\r' in CRLF, making the preprocessor's printed value give the wrong line 
number 
when returning from an include. For example:

(line 1):#include \r\n

The "file exit" code causes the printer to try to print the 'returned to the 
main file' line. It looks up what the current line number is. However, since 
the 
current 'token' is the '\n' (since only the \r was consumed), it will give the 
line number as '1", not '2'. This results in a few failed tests, but more 
importantly, results in error messages being incorrect when compiling a 
previously preprocessed file.

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

Added:
cfe/trunk/test/Frontend/.gitattributes
cfe/trunk/test/Frontend/system-header-line-directive-ms-lineendings.c   
(with props)
Modified:
cfe/trunk/lib/Lex/Lexer.cpp

Modified: cfe/trunk/lib/Lex/Lexer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Lexer.cpp?rev=311683&r1=311682&r2=311683&view=diff
==
--- cfe/trunk/lib/Lex/Lexer.cpp (original)
+++ cfe/trunk/lib/Lex/Lexer.cpp Thu Aug 24 11:36:07 2017
@@ -3073,6 +3073,8 @@ LexNextToken:
   
   case '\n':
   case '\r':
+if (CurPtr[0] != Char && (CurPtr[0] == '\n' || CurPtr[0] == '\r'))
+  Char = getAndAdvanceChar(CurPtr, Result);
 // If we are inside a preprocessor directive and we see the end of line,
 // we know we are done with the directive, so return an EOD token.
 if (ParsingPreprocessorDirective) {

Added: cfe/trunk/test/Frontend/.gitattributes
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Frontend/.gitattributes?rev=311683&view=auto
==
--- cfe/trunk/test/Frontend/.gitattributes (added)
+++ cfe/trunk/test/Frontend/.gitattributes Thu Aug 24 11:36:07 2017
@@ -0,0 +1,2 @@
+# Below test validates crlf line endings, so it should stay crlf.
+system-header-line-directive-ms-lineendings.c text eol=crlf

Added: cfe/trunk/test/Frontend/system-header-line-directive-ms-lineendings.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Frontend/system-header-line-directive-ms-lineendings.c?rev=311683&view=auto
==
--- cfe/trunk/test/Frontend/system-header-line-directive-ms-lineendings.c 
(added)
+++ cfe/trunk/test/Frontend/system-header-line-directive-ms-lineendings.c Thu 
Aug 24 11:36:07 2017
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 %s -E -o - -I %S/Inputs -isystem 
%S/Inputs/SystemHeaderPrefix | FileCheck %s
+#include 
+#include 
+
+#include "line-directive.h"
+
+// This tests that the line numbers for the current file are correctly 
outputted
+// for the include-file-completed test case.  
+
+// CHECK: # 1 "{{.*}}system-header-line-directive-ms-lineendings.c" 2
+// CHECK: # 1 "{{.*}}noline.h" 1 3
+// CHECK: foo();
+// CHECK: # 3 "{{.*}}system-header-line-directive-ms-lineendings.c" 2
+// CHECK: # 1 "{{.*}}line-directive-in-system.h" 1 3
+//  The "3" below indicates that "foo.h" is considered a system header.
+// CHECK: # 1 "foo.h" 3
+// CHECK: foo();
+// CHECK: # 4 "{{.*}}system-header-line-directive-ms-lineendings.c" 2
+// CHECK: # 1 "{{.*}}line-directive.h" 1
+// CHECK: # 10 "foo.h"{{$}}
+// CHECK: # 6 "{{.*}}system-header-line-directive-ms-lineendings.c" 2

Propchange: 
cfe/trunk/test/Frontend/system-header-line-directive-ms-lineendings.c
--
svn:eol-style = CRLF


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

[PATCH] D37079: [Preprocessor] Correct internal token parsing of newline characters in CRLF

[Erich Keane via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL311683: [Preprocessor] Correct internal token parsing of 
newline characters in CRLF (authored by erichkeane).

Changed prior to commit:
  https://reviews.llvm.org/D37079?vs=112581&id=112589#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37079

Files:
  cfe/trunk/lib/Lex/Lexer.cpp
  cfe/trunk/test/Frontend/.gitattributes
  cfe/trunk/test/Frontend/system-header-line-directive-ms-lineendings.c


Index: cfe/trunk/test/Frontend/.gitattributes
===
--- cfe/trunk/test/Frontend/.gitattributes
+++ cfe/trunk/test/Frontend/.gitattributes
@@ -0,0 +1,2 @@
+# Below test validates crlf line endings, so it should stay crlf.
+system-header-line-directive-ms-lineendings.c text eol=crlf
Index: cfe/trunk/test/Frontend/system-header-line-directive-ms-lineendings.c
===
--- cfe/trunk/test/Frontend/system-header-line-directive-ms-lineendings.c
+++ cfe/trunk/test/Frontend/system-header-line-directive-ms-lineendings.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 %s -E -o - -I %S/Inputs -isystem 
%S/Inputs/SystemHeaderPrefix | FileCheck %s
+#include 
+#include 
+
+#include "line-directive.h"
+
+// This tests that the line numbers for the current file are correctly 
outputted
+// for the include-file-completed test case.  
+
+// CHECK: # 1 "{{.*}}system-header-line-directive-ms-lineendings.c" 2
+// CHECK: # 1 "{{.*}}noline.h" 1 3
+// CHECK: foo();
+// CHECK: # 3 "{{.*}}system-header-line-directive-ms-lineendings.c" 2
+// CHECK: # 1 "{{.*}}line-directive-in-system.h" 1 3
+//  The "3" below indicates that "foo.h" is considered a system header.
+// CHECK: # 1 "foo.h" 3
+// CHECK: foo();
+// CHECK: # 4 "{{.*}}system-header-line-directive-ms-lineendings.c" 2
+// CHECK: # 1 "{{.*}}line-directive.h" 1
+// CHECK: # 10 "foo.h"{{$}}
+// CHECK: # 6 "{{.*}}system-header-line-directive-ms-lineendings.c" 2
Index: cfe/trunk/lib/Lex/Lexer.cpp
===
--- cfe/trunk/lib/Lex/Lexer.cpp
+++ cfe/trunk/lib/Lex/Lexer.cpp
@@ -3073,6 +3073,8 @@
   
   case '\n':
   case '\r':
+if (CurPtr[0] != Char && (CurPtr[0] == '\n' || CurPtr[0] == '\r'))
+  Char = getAndAdvanceChar(CurPtr, Result);
 // If we are inside a preprocessor directive and we see the end of line,
 // we know we are done with the directive, so return an EOD token.
 if (ParsingPreprocessorDirective) {


Index: cfe/trunk/test/Frontend/.gitattributes
===
--- cfe/trunk/test/Frontend/.gitattributes
+++ cfe/trunk/test/Frontend/.gitattributes
@@ -0,0 +1,2 @@
+# Below test validates crlf line endings, so it should stay crlf.
+system-header-line-directive-ms-lineendings.c text eol=crlf
Index: cfe/trunk/test/Frontend/system-header-line-directive-ms-lineendings.c
===
--- cfe/trunk/test/Frontend/system-header-line-directive-ms-lineendings.c
+++ cfe/trunk/test/Frontend/system-header-line-directive-ms-lineendings.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 %s -E -o - -I %S/Inputs -isystem %S/Inputs/SystemHeaderPrefix | FileCheck %s
+#include 
+#include 
+
+#include "line-directive.h"
+
+// This tests that the line numbers for the current file are correctly outputted
+// for the include-file-completed test case.  
+
+// CHECK: # 1 "{{.*}}system-header-line-directive-ms-lineendings.c" 2
+// CHECK: # 1 "{{.*}}noline.h" 1 3
+// CHECK: foo();
+// CHECK: # 3 "{{.*}}system-header-line-directive-ms-lineendings.c" 2
+// CHECK: # 1 "{{.*}}line-directive-in-system.h" 1 3
+//  The "3" below indicates that "foo.h" is considered a system header.
+// CHECK: # 1 "foo.h" 3
+// CHECK: foo();
+// CHECK: # 4 "{{.*}}system-header-line-directive-ms-lineendings.c" 2
+// CHECK: # 1 "{{.*}}line-directive.h" 1
+// CHECK: # 10 "foo.h"{{$}}
+// CHECK: # 6 "{{.*}}system-header-line-directive-ms-lineendings.c" 2
Index: cfe/trunk/lib/Lex/Lexer.cpp
===
--- cfe/trunk/lib/Lex/Lexer.cpp
+++ cfe/trunk/lib/Lex/Lexer.cpp
@@ -3073,6 +3073,8 @@
   
   case '\n':
   case '\r':
+if (CurPtr[0] != Char && (CurPtr[0] == '\n' || CurPtr[0] == '\r'))
+  Char = getAndAdvanceChar(CurPtr, Result);
 // If we are inside a preprocessor directive and we see the end of line,
 // we know we are done with the directive, so return an EOD token.
 if (ParsingPreprocessorDirective) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36272: [CodeGen][x86_64] Enable 'force_align_arg_pointer' attribute at x86_64

[Anatol Pomozov via Phabricator via cfe-commits]

anatol.pomozov added a comment.

Related maillist discussion is 
http://lists.llvm.org/pipermail/cfe-dev/2017-June/054359.html

> Are we SURE the stack alignment for this type is supposed to be 16 bit as 
> well?  I didn't see any discussion about it in the email conversation.
>  I have very little understanding of this attribute, but I would (perhaps 
> naiively) presume that it would be different on 64 bit targets.

x86_64 ABI requires 16-byte stack alignment and compiler already enforces it at 
the caller side. It does not work when we jump from 32bit code (where stack 
might not be 16-byte aligned) to 64bit code. So we need a way to enforce the 
stack alignment at  callee side. In this case the attribute above is helpful.


https://reviews.llvm.org/D36272



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

[PATCH] D36272: [CodeGen][x86_64] Enable 'force_align_arg_pointer' attribute at x86_64

[Anatol Pomozov via Phabricator via cfe-commits]

anatol.pomozov added a comment.

> Additionally, there is likely value to split the test run-line into 2, one 
> that specifies 64 bit Windows and one that is 64 bit Linux.  The value being 
> that they validate two different code paths (whereas 32 is the same code 
> path).

I added a RUN for triplet '-triple x86_64-pc-win32' and tests are failing for 
me. I rebuilt clang without my patches and still see the same failure. It looks 
like the original function-attributes currently does not work with win32 
triplet.

  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  /mnt/cold/sources/llvm/tools/clang/test/CodeGen/function-attributes.c:76:21: 
warning: unknown attribute 'force_align_arg_pointer' ignored
  void __attribute__((force_align_arg_pointer)) f16(void) {
  ^
  1 warning generated.
  /mnt/cold/sources/llvm/tools/clang/test/CodeGen/function-attributes.c:4:11: 
error: expected string not found in input
  // CHECK: define signext i8 @f0(i32 %x) [[NUW:#[0-9]+]]
^
  :1:1: note: scanning from here
  ; ModuleID = 
'/mnt/cold/sources/llvm/tools/clang/test/CodeGen/function-attributes.c'
  ^
  :7:1: note: possible intended match here
  define i8 @f0(i32 %x) #0 {
  ^
  /mnt/cold/sources/llvm/tools/clang/test/CodeGen/function-attributes.c:67:11: 
error: expected string not found in input
  // CHECK: [[NUW]]
^
  :158:17: note: scanning from here
  define void @f15() #0 {
  ^
  :158:17: note: uses undefined variable "NUW"
  define void @f15() #0 {
  ^
  /mnt/cold/sources/llvm/tools/clang/test/CodeGen/function-attributes.c:104:11: 
error: expected string not found in input
  // CHECK: call i32 @_setjmp(i32* null)
^
  :191:1: note: scanning from here
  entry:
  ^
  :193:7: note: possible intended match here
   %1 = call i32 @_setjmp(i8* null, i8* %0) #7
^
  
  --


https://reviews.llvm.org/D36272



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

Re: r298574 - Fix issues in clang-format's AlignConsecutive modes.

[Hans Wennborg via cfe-commits]

This caused https://bugs.llvm.org/show_bug.cgi?id=33507 which is one
of the last release blockers for 5.0.0.

Can someone who's familiar with this code please take a look?

On Thu, Aug 24, 2017 at 11:12 AM, Hans Wennborg  wrote:
> For reference, this was reviewed in https://reviews.llvm.org/D21279
>
> (Please always include review links in the future.)
>
> On Wed, Mar 22, 2017 at 7:51 PM, Nikola Smiljanic via cfe-commits
>  wrote:
>> Author: nikola
>> Date: Wed Mar 22 21:51:25 2017
>> New Revision: 298574
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=298574&view=rev
>> Log:
>> Fix issues in clang-format's AlignConsecutive modes.
>>
>> Patch by Ben Harper.
>>
>> Modified:
>> cfe/trunk/lib/Format/WhitespaceManager.cpp
>> cfe/trunk/lib/Format/WhitespaceManager.h
>> cfe/trunk/unittests/Format/FormatTest.cpp
>>
>> Modified: cfe/trunk/lib/Format/WhitespaceManager.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.cpp?rev=298574&r1=298573&r2=298574&view=diff
>> ==
>> --- cfe/trunk/lib/Format/WhitespaceManager.cpp (original)
>> +++ cfe/trunk/lib/Format/WhitespaceManager.cpp Wed Mar 22 21:51:25 2017
>> @@ -50,9 +50,9 @@ void WhitespaceManager::replaceWhitespac
>>if (Tok.Finalized)
>>  return;
>>Tok.Decision = (Newlines > 0) ? FD_Break : FD_Continue;
>> -  Changes.push_back(Change(Tok, /*CreateReplacement=*/true,
>> -   Tok.WhitespaceRange, Spaces, StartOfTokenColumn,
>> -   Newlines, "", "", InPPDirective && !Tok.IsFirst,
>> +  Changes.push_back(Change(Tok, /*CreateReplacement=*/true, 
>> Tok.WhitespaceRange,
>> +   Spaces, StartOfTokenColumn, Newlines, "", "",
>> +   InPPDirective && !Tok.IsFirst,
>> /*IsInsideToken=*/false));
>>  }
>>
>> @@ -192,21 +192,56 @@ AlignTokenSequence(unsigned Start, unsig
>> SmallVector &Changes) {
>>bool FoundMatchOnLine = false;
>>int Shift = 0;
>> +
>> +  // ScopeStack keeps track of the current scope depth. It contains indices 
>> of
>> +  // the first token on each scope.
>> +  // We only run the "Matches" function on tokens from the outer-most scope.
>> +  // However, we do need to pay special attention to one class of tokens
>> +  // that are not in the outer-most scope, and that is function parameters
>> +  // which are split across multiple lines, as illustrated by this example:
>> +  //   double a(int x);
>> +  //   intb(int  y,
>> +  //  double z);
>> +  // In the above example, we need to take special care to ensure that
>> +  // 'double z' is indented along with it's owning function 'b'.
>> +  SmallVector ScopeStack;
>> +
>>for (unsigned i = Start; i != End; ++i) {
>> -if (Changes[i].NewlinesBefore > 0) {
>> -  FoundMatchOnLine = false;
>> +if (ScopeStack.size() != 0 &&
>> +Changes[i].nestingAndIndentLevel() <
>> +Changes[ScopeStack.back()].nestingAndIndentLevel())
>> +  ScopeStack.pop_back();
>> +
>> +if (i != Start && Changes[i].nestingAndIndentLevel() >
>> +  Changes[i - 1].nestingAndIndentLevel())
>> +  ScopeStack.push_back(i);
>> +
>> +bool InsideNestedScope = ScopeStack.size() != 0;
>> +
>> +if (Changes[i].NewlinesBefore > 0 && !InsideNestedScope) {
>>Shift = 0;
>> +  FoundMatchOnLine = false;
>>  }
>>
>>  // If this is the first matching token to be aligned, remember by how 
>> many
>>  // spaces it has to be shifted, so the rest of the changes on the line 
>> are
>>  // shifted by the same amount
>> -if (!FoundMatchOnLine && Matches(Changes[i])) {
>> +if (!FoundMatchOnLine && !InsideNestedScope && Matches(Changes[i])) {
>>FoundMatchOnLine = true;
>>Shift = Column - Changes[i].StartOfTokenColumn;
>>Changes[i].Spaces += Shift;
>>  }
>>
>> +// This is for function parameters that are split across multiple lines,
>> +// as mentioned in the ScopeStack comment.
>> +if (InsideNestedScope && Changes[i].NewlinesBefore > 0) {
>> +  unsigned ScopeStart = ScopeStack.back();
>> +  if (Changes[ScopeStart - 1].Tok->is(TT_FunctionDeclarationName) ||
>> +  (ScopeStart > Start + 1 &&
>> +   Changes[ScopeStart - 2].Tok->is(TT_FunctionDeclarationName)))
>> +Changes[i].Spaces += Shift;
>> +}
>> +
>>  assert(Shift >= 0);
>>  Changes[i].StartOfTokenColumn += Shift;
>>  if (i + 1 != Changes.size())
>> @@ -214,15 +249,37 @@ AlignTokenSequence(unsigned Start, unsig
>>}
>>  }
>>
>> -// Walk through all of the changes and find sequences of matching tokens to
>> -// align. To do so, keep track of the lines and whether or not a matching 
>> token
>> -// was found on a line. If a matching token is found, extend the current
>> -// sequence. If the current line cannot be part of a sequence,

[PATCH] D36272: [CodeGen][x86_64] Enable 'force_align_arg_pointer' attribute at x86_64

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Aaron would likely know better than me... but could it be the spelling type 
should be GCC instead of GNU?


https://reviews.llvm.org/D36272



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

[PATCH] D37091: Expose -mllvm -accurate-sample-profile to clang.

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

rsmith added inline comments.



Comment at: docs/ClangCommandLineReference.rst:173-180
+.. option:: -faccurate-sample-profile, -fno-accurate-sample-profile
+.. program:: clang
+
+If the sample profile is accurate, callsites without profile samples are marked
+as cold. Otherwise, treat un-sampled callsites as if we have no profile. This
+option can be used to enable more aggressive size optimization based on
+profiles.

This is a generated file; please don't modify it by hand.



Comment at: include/clang/Driver/Options.td:590
 def faccess_control : Flag<["-"], "faccess-control">, Group;
+def faccurate_sample_profile : Flag<["-"], "faccurate-sample-profile">,
+  Group, Flags<[DriverOption, CC1Option]>,

We generally try to group similar options together under a common prefix. Would 
`-fprofile-sample-accurate` work here?



Comment at: include/clang/Driver/Options.td:592-594
+  HelpText<"If sample profile is accurate, we will mark all un-sampled "
+   "callsite as cold. Otherwise, treat callsites without profile "
+   "samples as if we have no profile">;

`HelpText` should be a very brief string that can be included in a one-line 
description of the flag for `--help`. Longer text for inclusion in the option 
reference should be in a `DocBrief<{blah blah blah.}>`.

Also, it seems to me that this help text doesn't actually say what the option 
does. Does this request that accurate sample profiles be generated, or specify 
that an accurate sample profile was provided, or what? Suggestion:

```
HelpText<"Specifies that the sample profile is accurate">,
DocBrief<{Specifies that the sample profile is accurate. If the sample profile 
is accurate, callsites without profile samples are marked as cold. [...same as 
current reference documentation text...]}>
```


https://reviews.llvm.org/D37091



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

[PATCH] D36501: add flag to undo ABI change in r310401

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

Locally we have a couple different tactics for dealing with changes that we 
can't support.  A more coherent approach would be great.
For example we defined a new TargetCXXABI::Kind value that is mostly 
GenericItaniumABI except where it isn't.
I personally did not do most of the various ABI-related tweaks so I don't claim 
to have a good handle on them, and we have been slow to get these things 
upstream; but I'd love to make that happen.


Repository:
  rL LLVM

https://reviews.llvm.org/D36501



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

Re: [PATCH] D36501: add flag to undo ABI change in r310401

[Richard Smith via cfe-commits]

On 24 August 2017 at 12:24, Paul Robinson via Phabricator via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> probinson added a comment.
>
> Locally we have a couple different tactics for dealing with changes that
> we can't support.  A more coherent approach would be great.
> For example we defined a new TargetCXXABI::Kind value that is mostly
> GenericItaniumABI except where it isn't.
> I personally did not do most of the various ABI-related tweaks so I don't
> claim to have a good handle on them, and we have been slow to get these
> things upstream; but I'd love to make that happen.


I'm looking into adding a more general mechanism to request ABI (bug)
compatibility with a prior version of Clang.

Paul: is the PS4 toolchain's ABI based on that of a particular Clang
release, or is it a branch from trunk at some point? Or something else?
(And which release / revision?)

John: X86_64ABIInfo::classifyIntegerMMXAsSSE() has a special case for
Darwin OSes for compatibility with older Clang compilers; likewise for
X86_64ABIInfo::honorsRevision0_98(). Is this a case of wanting
compatibility with an older version of Clang by default when targeting
Darwin, or is it just the case that the platform ABI is different for that
target (where the historical reason for the different choice in the
platform ABI was a bug in older versions of Clang)?
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34367: CodeGen: Fix address space of indirect function argument

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 5 inline comments as done.
yaxunl added inline comments.



Comment at: lib/CodeGen/CGCall.cpp:3832
+ "indirect-arg-temp");
+IRCallArgs[FirstIRArg] = CastToAllocaAddrSpace(Addr.getPointer());
 

rjmccall wrote:
> Isn't the original code here correct?  You're basically just adding 
> unnecessary casts.
will remove.



Comment at: lib/CodeGen/CGCall.cpp:3851
+ ->getType()
+ ->getPointerAddressSpace();
 const unsigned ArgAddrSpace =

rjmccall wrote:
> Hmm.  Is there actually a test case where Addr.getType()->getAddressSpace() 
> is not the lowering of LangAS::Default?  The correctness of the 
> performAddrSpaceCast call you make depends on that.
> 
> If there isn't, I think it would be better to add a target hook to attempt to 
> peephole an addrspace cast:
>   llvm::Value *tryPeepholeAddrSpaceCast(llvm::Value*, unsigned valueAS, 
> unsigned targetAS);
> 
> And then you can just do
>   bool HasAddrSpaceMismatch = CGM.getASTAllocaAddrSpace() != LangAS::Default);
>   if (HasAddrSpaceMismatch) {
> if (llvm::Value *ptr = tryPeepholeAddrSpaceCast(Addr.getPointer(), 
> LangAS::Default, getASTAllocAddrSpace())) {
>   Addr = Address(ptr, Addr.getAlignment()); // might need to cast the 
> element type for this
>   HasAddrSpaceMismatch = false;
> }
>   }
It is possible RVAddrSpace is not default addr space. e.g. in OpenCL

```
global struct S g_s;

void f(struct S s);

void g() {
  f(g_s);
}

```

One thing that confuses me is that why creating temp and copying can be skipped 
if RVAddrSpace equals alloca addr space. The callee is supposed to work on a 
copy of the arg, not the arg itself, right? Shouldn't the arg always be coped 
to a temp then pass to the callee?



Comment at: lib/CodeGen/CGCall.cpp:3865
+ "byval-temp");
+  IRCallArgs[FirstIRArg] = CastToAllocaAddrSpace(AI.getPointer());
   EmitAggregateCopy(AI, Addr, I->Ty, RV.isVolatileQualified());

rjmccall wrote:
> Same thing, no reason to do the casts here.
will remove



Comment at: lib/CodeGen/CGDecl.cpp:1828
+auto DestAS = getContext().getTargetAddressSpace(LangAS::Default);
+if (SrcAS != DestAS) {
+  assert(SrcAS == CGM.getDataLayout().getAllocaAddrSpace());

rjmccall wrote:
> This should be comparing AST address spaces.
will do.


https://reviews.llvm.org/D34367



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

Re: [PATCH] D36501: add flag to undo ABI change in r310401

[John McCall via cfe-commits]

> On Aug 24, 2017, at 3:48 PM, Richard Smith  wrote:
> On 24 August 2017 at 12:24, Paul Robinson via Phabricator via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> probinson added a comment.
> 
> Locally we have a couple different tactics for dealing with changes that we 
> can't support.  A more coherent approach would be great.
> For example we defined a new TargetCXXABI::Kind value that is mostly 
> GenericItaniumABI except where it isn't.
> I personally did not do most of the various ABI-related tweaks so I don't 
> claim to have a good handle on them, and we have been slow to get these 
> things upstream; but I'd love to make that happen.
> 
> I'm looking into adding a more general mechanism to request ABI (bug) 
> compatibility with a prior version of Clang.
> 
> Paul: is the PS4 toolchain's ABI based on that of a particular Clang release, 
> or is it a branch from trunk at some point? Or something else? (And which 
> release / revision?)
> 
> John: X86_64ABIInfo::classifyIntegerMMXAsSSE() has a special case for Darwin 
> OSes for compatibility with older Clang compilers; likewise for 
> X86_64ABIInfo::honorsRevision0_98(). Is this a case of wanting compatibility 
> with an older version of Clang by default when targeting Darwin, or is it 
> just the case that the platform ABI is different for that target (where the 
> historical reason for the different choice in the platform ABI was a bug in 
> older versions of Clang)?

The latter.  There was a bug, and we decided that we weren't allowed to fix it, 
so it's the ABI now.  Note that there's no way to override the default.

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

r311695 - [ubsan] PR34266: When sanitizing the 'this' value for a member function that happens to be a lambda call operator, use the lambda's 'this' pointer, not the captured enclosing 'this' pointer

[Richard Smith via cfe-commits]

Author: rsmith
Date: Thu Aug 24 13:10:33 2017
New Revision: 311695

URL: http://llvm.org/viewvc/llvm-project?rev=311695&view=rev
Log:
[ubsan] PR34266: When sanitizing the 'this' value for a member function that 
happens to be a lambda call operator, use the lambda's 'this' pointer, not the 
captured enclosing 'this' pointer (if any).

Do not sanitize the 'this' pointer of a member call operator for a lambda with
no capture-default, since that call operator can legitimately be called with a
null this pointer from the static invoker function. Any actual call with a null
this pointer should still be caught in the caller (if it is being sanitized).

This reinstates r311589 (reverted in r311680) with the above fix.

Modified:
cfe/trunk/include/clang/AST/DeclCXX.h
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=311695&r1=311694&r2=311695&view=diff
==
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Thu Aug 24 13:10:33 2017
@@ -2027,7 +2027,10 @@ public:
 
   /// \brief Returns the type of the \c this pointer.
   ///
-  /// Should only be called for instance (i.e., non-static) methods.
+  /// Should only be called for instance (i.e., non-static) methods. Note
+  /// that for the call operator of a lambda closure type, this returns the
+  /// desugared 'this' type (a pointer to the closure type), not the captured
+  /// 'this' type.
   QualType getThisType(ASTContext &C) const;
 
   unsigned getTypeQualifiers() const {

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=311695&r1=311694&r2=311695&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Thu Aug 24 13:10:33 2017
@@ -22,6 +22,7 @@
 #include "CodeGenPGO.h"
 #include "TargetInfo.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTLambda.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/StmtCXX.h"
@@ -1014,11 +1015,22 @@ void CodeGenFunction::StartFunction(Glob
 }
 
 // Check the 'this' pointer once per function, if it's available.
-if (CXXThisValue) {
+if (CXXABIThisValue) {
   SanitizerSet SkippedChecks;
   SkippedChecks.set(SanitizerKind::ObjectSize, true);
   QualType ThisTy = MD->getThisType(getContext());
-  EmitTypeCheck(TCK_Load, Loc, CXXThisValue, ThisTy,
+
+  // If this is the call operator of a lambda with no capture-default, it
+  // may have a static invoker function, which may call this operator with
+  // a null 'this' pointer.
+  if (isLambdaCallOperator(MD) &&
+  cast(MD->getParent())->getLambdaCaptureDefault() ==
+  LCD_None)
+SkippedChecks.set(SanitizerKind::Null, true);
+
+  EmitTypeCheck(isa(MD) ? TCK_ConstructorCall
+: TCK_MemberCall,
+Loc, CXXABIThisValue, ThisTy,
 getContext().getTypeAlignInChars(ThisTy->getPointeeType()),
 SkippedChecks);
 }

Modified: cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp?rev=311695&r1=311694&r2=311695&view=diff
==
--- cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp Thu Aug 24 13:10:33 2017
@@ -449,6 +449,28 @@ void upcast_to_vbase() {
 }
 }
 
+struct ThisAlign {
+  void this_align_lambda();
+  void this_align_lambda_2();
+};
+void ThisAlign::this_align_lambda() {
+  // CHECK-LABEL: define 
{{.*}}@"_ZZN9ThisAlign17this_align_lambdaEvENK3$_0clEv"
+  // CHECK-SAME: (%{{.*}}* %[[this:[^)]*]])
+  // CHECK: %[[this_addr:.*]] = alloca
+  // CHECK: store %{{.*}}* %[[this]], %{{.*}}** %[[this_addr]],
+  // CHECK: %[[this_inner:.*]] = load %{{.*}}*, %{{.*}}** %[[this_addr]],
+  // CHECK: %[[this_outer_addr:.*]] = getelementptr inbounds %{{.*}}, %{{.*}}* 
%[[this_inner]], i32 0, i32 0
+  // CHECK: %[[this_outer:.*]] = load %{{.*}}*, %{{.*}}** %[[this_outer_addr]],
+  //
+  // CHECK: %[[this_inner_isnonnull:.*]] = icmp ne %{{.*}}* %[[this_inner]], 
null
+  // CHECK: %[[this_inner_asint:.*]] = ptrtoint %{{.*}}* %[[this_inner]] to i
+  // CHECK: %[[this_inner_misalignment:.*]] = and i{{32|64}} 
%[[this_inner_asint]], {{3|7}},
+  // CHECK: %[[this_inner_isaligned:.*]] = icmp eq i{{32|64}} 
%[[this_inner_misalignment]], 0
+  // CHECK: %[[this_inner_valid:.*]] = and i1 %[[this_inner_isnonnull]], 
%[[this_inner_isaligned]],

[PATCH] D37091: Expose -mllvm -accurate-sample-profile to clang.

[Dehao Chen via Phabricator via cfe-commits]

danielcdh updated this revision to Diff 112597.
danielcdh marked 3 inline comments as done.
danielcdh added a comment.

update


https://reviews.llvm.org/D37091

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CodeGenFunction.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/Driver/clang_f_opts.c


Index: test/Driver/clang_f_opts.c
===
--- test/Driver/clang_f_opts.c
+++ test/Driver/clang_f_opts.c
@@ -53,6 +53,9 @@
 // CHECK-REROLL-LOOPS: "-freroll-loops"
 // CHECK-NO-REROLL-LOOPS-NOT: "-freroll-loops"
 
+// RUN: %clang -### -S -fprofile-sample-accurate %s 2>&1 | FileCheck 
-check-prefix=CHECK-PROFILE-SAMPLE-ACCURATE %s
+// CHECK-PROFILE-SAMPLE-ACCURATE: "-fprofile-sample-accurate"
+
 // RUN: %clang -### -S -fprofile-sample-use=%S/Inputs/file.prof %s 2>&1 | 
FileCheck -check-prefix=CHECK-SAMPLE-PROFILE %s
 // CHECK-SAMPLE-PROFILE: "-fprofile-sample-use={{.*}}/file.prof"
 
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -652,6 +652,8 @@
 
   Opts.NoUseJumpTables = Args.hasArg(OPT_fno_jump_tables);
 
+  Opts.ProfileSampleAccurate = Args.hasArg(OPT_fprofile_sample_accurate);
+
   Opts.PrepareForLTO = Args.hasArg(OPT_flto, OPT_flto_EQ);
   Opts.EmitSummaryIndex = false;
   if (Arg *A = Args.getLastArg(OPT_flto_EQ)) {
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2340,6 +2340,10 @@
 true))
 CmdArgs.push_back("-fno-jump-tables");
 
+  if (Args.hasFlag(options::OPT_fprofile_sample_accurate,
+   options::OPT_fno_profile_sample_accurate, false))
+CmdArgs.push_back("-fprofile-sample-accurate");
+
   if (!Args.hasFlag(options::OPT_fpreserve_as_comments,
 options::OPT_fno_preserve_as_comments, true))
 CmdArgs.push_back("-fno-preserve-as-comments");
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -837,6 +837,10 @@
   Fn->addFnAttr("no-jump-tables",
 llvm::toStringRef(CGM.getCodeGenOpts().NoUseJumpTables));
 
+  // Add profile-sample-accurate value.
+  if (CGM.getCodeGenOpts().ProfileSampleAccurate)
+Fn->addFnAttr("profile-sample-accurate");
+
   if (getLangOpts().OpenCL) {
 // Add metadata for a kernel function.
 if (const FunctionDecl *FD = dyn_cast_or_null(D))
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -183,6 +183,7 @@
 CODEGENOPT(UnwindTables  , 1, 0) ///< Emit unwind tables.
 CODEGENOPT(VectorizeLoop , 1, 0) ///< Run loop vectorizer.
 CODEGENOPT(VectorizeSLP  , 1, 0) ///< Run SLP vectorizer.
+CODEGENOPT(ProfileSampleAccurate, 1, 0) ///< Sample profile is accurate.
 
   /// Attempt to use register sized accesses to bit-fields in structures, when
   /// possible.
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -637,12 +637,25 @@
 def fprofile_sample_use_EQ : Joined<["-"], "fprofile-sample-use=">,
 Group, Flags<[DriverOption, CC1Option]>,
 HelpText<"Enable sample-based profile guided optimizations">;
+def fprofile_sample_accurate : Flag<["-"], "fprofile-sample-accurate">,
+Group, Flags<[DriverOption, CC1Option]>,
+HelpText<"Specifies that the sample profile is accurate">,
+DocBrief<[{Specifies that the sample profile is accurate. If the sample
+   profile is accurate, callsites without profile samples are 
marked
+   as cold. Otherwise, treat callsites without profile samples as 
if
+   we have no profile}]>;
+def fno_profile_sample_accurate : Flag<["-"], "fno-profile-sample-accurate">,
+  Group, Flags<[DriverOption]>;
 def fauto_profile : Flag<["-"], "fauto-profile">, Group,
 Alias;
 def fno_auto_profile : Flag<["-"], "fno-auto-profile">, Group,
 Alias;
 def fauto_profile_EQ : Joined<["-"], "fauto-profile=">,
 Alias;
+def fauto_profile_accurate : Flag<["-"], "fauto-profile-accurate">,
+Group, Alias;
+def fno_auto_profile_accurate : Flag<["-"], "fno-auto-profile-accurate">,
+Group, Alias;
 def fdebug_info_for_profiling : Flag<["-"], "fdebug-info-for-profiling">, 
Group,
 Flags<[CC1Option]>,
 HelpText<"Emit extra debug info to make sample profile more accurate.">;


Index: test/Driver/clang_f_opts.c
===
--- test/Drive