date:20211228

[PATCH] D116003: [NFC] Specify targets for clang stack-protector-guard.c

2021-12-28 Thread Qiu Chaofan via Phabricator via cfe-commits

qiucf updated this revision to Diff 396379.
qiucf marked 5 inline comments as done.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116003/new/

https://reviews.llvm.org/D116003

Files:
  clang/test/CodeGen/stack-protector-guard.c


Index: clang/test/CodeGen/stack-protector-guard.c
===
--- clang/test/CodeGen/stack-protector-guard.c
+++ clang/test/CodeGen/stack-protector-guard.c
@@ -1,16 +1,25 @@
-// RUN: %clang_cc1 -mstack-protector-guard=sysreg \
-// RUN:-mstack-protector-guard-reg=sp_el0 \
-// RUN:-mstack-protector-guard-offset=1024 \
-// RUN:-emit-llvm %s -o - | FileCheck %s
-// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck --check-prefix=CHECK-NONE %s
+// RUN: %clang_cc1 -mstack-protector-guard=sysreg -triple x86_64-linux-gnu \
+// RUN:   -mstack-protector-guard-offset=1024 -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -mstack-protector-guard=sysreg -triple 
powerpc64le-linux-gnu \
+// RUN:   -mstack-protector-guard-offset=1024 -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -mstack-protector-guard=sysreg -triple arm-linux-gnueabi \
+// RUN:   -mstack-protector-guard-offset=1024 -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -mstack-protector-guard=sysreg -triple 
thumbv7-linux-gnueabi \
+// RUN:   -mstack-protector-guard-offset=1024 -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -mstack-protector-guard=sysreg -triple aarch64-linux-gnu \
+// RUN:   -mstack-protector-guard-offset=1024 
-mstack-protector-guard-reg=sp_el0 \
+// RUN:   -emit-llvm %s -o - | FileCheck %s --check-prefix=AARCH64
 void foo(int*);
 void bar(int x) {
   int baz[x];
   foo(baz);
 }
 
-// CHECK: !llvm.module.flags = !{{{.*}}[[ATTR1:![0-9]+]], [[ATTR2:![0-9]+]], 
[[ATTR3:![0-9]+]]}
+// CHECK: !llvm.module.flags = !{{{.*}}[[ATTR1:![0-9]+]], [[ATTR2:![0-9]+]]}
 // CHECK: [[ATTR1]] = !{i32 1, !"stack-protector-guard", !"sysreg"}
-// CHECK: [[ATTR2]] = !{i32 1, !"stack-protector-guard-reg", !"sp_el0"}
-// CHECK: [[ATTR3]] = !{i32 1, !"stack-protector-guard-offset", i32 1024}
-// CHECK-NONE-NOT: !"stack-protector-guard
+// CHECK: [[ATTR2]] = !{i32 1, !"stack-protector-guard-offset", i32 1024}
+
+// AARCH64: !llvm.module.flags = !{{{.*}}[[ATTR1:![0-9]+]], [[ATTR2:![0-9]+]], 
[[ATTR3:![0-9]+]]}
+// AARCH64: [[ATTR1]] = !{i32 1, !"stack-protector-guard", !"sysreg"}
+// AARCH64: [[ATTR2]] = !{i32 1, !"stack-protector-guard-reg", !"sp_el0"}
+// AARCH64: [[ATTR3]] = !{i32 1, !"stack-protector-guard-offset", i32 1024}


Index: clang/test/CodeGen/stack-protector-guard.c
===
--- clang/test/CodeGen/stack-protector-guard.c
+++ clang/test/CodeGen/stack-protector-guard.c
@@ -1,16 +1,25 @@
-// RUN: %clang_cc1 -mstack-protector-guard=sysreg \
-// RUN:-mstack-protector-guard-reg=sp_el0 \
-// RUN:-mstack-protector-guard-offset=1024 \
-// RUN:-emit-llvm %s -o - | FileCheck %s
-// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck --check-prefix=CHECK-NONE %s
+// RUN: %clang_cc1 -mstack-protector-guard=sysreg -triple x86_64-linux-gnu \
+// RUN:   -mstack-protector-guard-offset=1024 -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -mstack-protector-guard=sysreg -triple powerpc64le-linux-gnu \
+// RUN:   -mstack-protector-guard-offset=1024 -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -mstack-protector-guard=sysreg -triple arm-linux-gnueabi \
+// RUN:   -mstack-protector-guard-offset=1024 -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -mstack-protector-guard=sysreg -triple thumbv7-linux-gnueabi \
+// RUN:   -mstack-protector-guard-offset=1024 -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -mstack-protector-guard=sysreg -triple aarch64-linux-gnu \
+// RUN:   -mstack-protector-guard-offset=1024 -mstack-protector-guard-reg=sp_el0 \
+// RUN:   -emit-llvm %s -o - | FileCheck %s --check-prefix=AARCH64
 void foo(int*);
 void bar(int x) {
   int baz[x];
   foo(baz);
 }
 
-// CHECK: !llvm.module.flags = !{{{.*}}[[ATTR1:![0-9]+]], [[ATTR2:![0-9]+]], [[ATTR3:![0-9]+]]}
+// CHECK: !llvm.module.flags = !{{{.*}}[[ATTR1:![0-9]+]], [[ATTR2:![0-9]+]]}
 // CHECK: [[ATTR1]] = !{i32 1, !"stack-protector-guard", !"sysreg"}
-// CHECK: [[ATTR2]] = !{i32 1, !"stack-protector-guard-reg", !"sp_el0"}
-// CHECK: [[ATTR3]] = !{i32 1, !"stack-protector-guard-offset", i32 1024}
-// CHECK-NONE-NOT: !"stack-protector-guard
+// CHECK: [[ATTR2]] = !{i32 1, !"stack-protector-guard-offset", i32 1024}
+
+// AARCH64: !llvm.module.flags = !{{{.*}}[[ATTR1:![0-9]+]], [[ATTR2:![0-9]+]], [[ATTR3:![0-9]+]]}
+// AARCH64: [[ATTR1]] = !{i32 1, !"stack-protector-guard", !"sysreg"}
+// AARCH64: [[ATTR2]] = !{i32 1, !"stack-protector-guard-reg", !"sp_el0"}
+// AARCH64: [[ATTR3]] = !{i32 1, !"stack-protector-guard-offset", i32 1024}
___
cfe-commits mailing list
cfe-commits@

[PATCH] D116161: [Clang] Add an overload for emitUnaryBuiltin.

2021-12-28 Thread Jun Zhang via Phabricator via cfe-commits

junaire updated this revision to Diff 396380.
junaire added a comment.

In order to use `emitUnaryBuiltin` in other cases, I changed the function 
interface.
This allows us to use it in all `Builder.CreateUnaryIntrinsic()` cases, but 
will make
the function body very small.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116161/new/

https://reviews.llvm.org/D116161

Files:
  clang/lib/CodeGen/CGBuiltin.cpp


Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -541,6 +541,11 @@
   return CGF.Builder.CreateCall(F, Src0);
 }
 
+static Value *emitUnaryBuiltin(CodeGenFunction &CGF, llvm::Value *V,
+   unsigned IntrinsicID, llvm::StringRef Name) {
+  return CGF.Builder.CreateUnaryIntrinsic(IntrinsicID, V, nullptr, Name);
+}
+
 // Emit an intrinsic that has 2 operands of the same type as its result.
 static Value *emitBinaryBuiltin(CodeGenFunction &CGF,
 const CallExpr *E,
@@ -3128,18 +3133,15 @@
   Result = Builder.CreateBinaryIntrinsic(
   llvm::Intrinsic::abs, Op0, Builder.getFalse(), nullptr, "elt.abs");
 else
-  Result = Builder.CreateUnaryIntrinsic(llvm::Intrinsic::fabs, Op0, 
nullptr,
-"elt.abs");
-return RValue::get(Result);
-  }
+  Result = emitUnaryBuiltin(*this, Op0, llvm::Intrinsic::fabs, "elt.abs");
 
-  case Builtin::BI__builtin_elementwise_ceil: {
-Value *Op0 = EmitScalarExpr(E->getArg(0));
-Value *Result = Builder.CreateUnaryIntrinsic(llvm::Intrinsic::ceil, Op0,
- nullptr, "elt.ceil");
 return RValue::get(Result);
   }
 
+  case Builtin::BI__builtin_elementwise_ceil:
+return RValue::get(emitUnaryBuiltin(*this, EmitScalarExpr(E->getArg(0)),
+llvm::Intrinsic::ceil, "elt.ceil"));
+
   case Builtin::BI__builtin_elementwise_max: {
 Value *Op0 = EmitScalarExpr(E->getArg(0));
 Value *Op1 = EmitScalarExpr(E->getArg(1));
@@ -3186,10 +3188,9 @@
   return llvm::Intrinsic::vector_reduce_fmax;
 };
 Value *Op0 = EmitScalarExpr(E->getArg(0));
-Value *Result = Builder.CreateUnaryIntrinsic(
-GetIntrinsicID(E->getArg(0)->getType(), Op0->getType()), Op0, nullptr,
-"rdx.min");
-return RValue::get(Result);
+return RValue::get(emitUnaryBuiltin(
+*this, Op0, GetIntrinsicID(E->getArg(0)->getType(), Op0->getType()),
+"rd.min"));
   }
 
   case Builtin::BI__builtin_reduce_min: {
@@ -3205,18 +3206,15 @@
   return llvm::Intrinsic::vector_reduce_fmin;
 };
 Value *Op0 = EmitScalarExpr(E->getArg(0));
-Value *Result = Builder.CreateUnaryIntrinsic(
-GetIntrinsicID(E->getArg(0)->getType(), Op0->getType()), Op0, nullptr,
-"rdx.min");
-return RValue::get(Result);
+return RValue::get(emitUnaryBuiltin(
+*this, Op0, GetIntrinsicID(E->getArg(0)->getType(), Op0->getType()),
+"rd.min"));
   }
 
-  case Builtin::BI__builtin_reduce_xor: {
-Value *Op0 = EmitScalarExpr(E->getArg(0));
-Value *Result = Builder.CreateUnaryIntrinsic(
-llvm::Intrinsic::vector_reduce_xor, Op0, nullptr, "rdx.xor");
-return RValue::get(Result);
-  }
+  case Builtin::BI__builtin_reduce_xor:
+return RValue::get(emitUnaryBuiltin(*this, EmitScalarExpr(E->getArg(0)),
+llvm::Intrinsic::vector_reduce_xor,
+"rdx.xor"));
 
   case Builtin::BI__builtin_matrix_transpose: {
 const auto *MatrixTy = 
E->getArg(0)->getType()->getAs();


Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -541,6 +541,11 @@
   return CGF.Builder.CreateCall(F, Src0);
 }
 
+static Value *emitUnaryBuiltin(CodeGenFunction &CGF, llvm::Value *V,
+   unsigned IntrinsicID, llvm::StringRef Name) {
+  return CGF.Builder.CreateUnaryIntrinsic(IntrinsicID, V, nullptr, Name);
+}
+
 // Emit an intrinsic that has 2 operands of the same type as its result.
 static Value *emitBinaryBuiltin(CodeGenFunction &CGF,
 const CallExpr *E,
@@ -3128,18 +3133,15 @@
   Result = Builder.CreateBinaryIntrinsic(
   llvm::Intrinsic::abs, Op0, Builder.getFalse(), nullptr, "elt.abs");
 else
-  Result = Builder.CreateUnaryIntrinsic(llvm::Intrinsic::fabs, Op0, nullptr,
-"elt.abs");
-return RValue::get(Result);
-  }
+  Result = emitUnaryBuiltin(*this, Op0, llvm::Intrinsic::fabs, "elt.abs");
 
-  case Builtin::BI__builtin_elementwise_ceil: {
-Value *Op0 = EmitScalarExpr(E->getArg(0));
-Value *Result = Builder.CreateUnaryIntrinsic(llvm::Intrinsic:

[PATCH] D107450: [clang-tidy] Fix wrong FixIt in performance-move-const-arg

2021-12-28 Thread gehry via Phabricator via cfe-commits

Sockke updated this revision to Diff 396385.
Sockke added a comment.

Sorry for the late reply. I made improvements to the patch in response to the 
questions you raised. @aaron.ballman


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107450/new/

https://reviews.llvm.org/D107450

Files:
  clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
  clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp
@@ -246,3 +246,97 @@
   };
   f(MoveSemantics());
 }
+
+void showInt(int &&v);
+void showInt(int v1, int &&v2);
+void showPointer(const char *&&s);
+void showPointer2(const char *const &&s);
+void showTriviallyCopyable(TriviallyCopyable &&obj);
+void showTriviallyCopyablePointer(const TriviallyCopyable *&&obj);
+void testFunctions() {
+  int a = 10;
+  showInt(std::move(a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect [performance-move-const-arg]
+  // CHECK-MESSAGES: :[[@LINE-10]]:20: note: consider changing the 1st parameter of 'showInt' from 'int &&' to 'const int &'
+  showInt(int());
+  showInt(a, std::move(a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect [performance-move-const-arg]
+  // CHECK-MESSAGES: :[[@LINE-13]]:28: note: consider changing the 2nd parameter of 'showInt' from 'int &&' to 'const int &'
+  const char* s = "";
+  showPointer(std::move(s));
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: std::move of the variable 's' of the trivially-copyable type 'const char *' has no effect [performance-move-const-arg]
+  // CHECK-MESSAGES: :[[@LINE-16]]:32: note: consider changing the 1st parameter of 'showPointer' from 'const char *&&' to 'const char *'
+  showPointer2(std::move(s)); 
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: std::move of the variable 's' of the trivially-copyable type 'const char *' has no effect [performance-move-const-arg]
+  // CHECK-MESSAGES: :[[@LINE-18]]:39: note: consider changing the 1st parameter of 'showPointer2' from 'const char *const &&' to 'const char *const'
+  TriviallyCopyable *obj = new TriviallyCopyable();
+  showTriviallyCopyable(std::move(*obj));
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: std::move of the expression of the trivially-copyable type 'TriviallyCopyable' has no effect [performance-move-const-arg]
+  // CHECK-MESSAGES: :[[@LINE-21]]:48: note: consider changing the 1st parameter of 'showTriviallyCopyable' from 'TriviallyCopyable &&' to 'const TriviallyCopyable &'
+  showTriviallyCopyablePointer(std::move(obj));
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: std::move of the variable 'obj' of the trivially-copyable type 'TriviallyCopyable *' has no effect [performance-move-const-arg]
+  // CHECK-MESSAGES: :[[@LINE-23]]:62: note: consider changing the 1st parameter of 'showTriviallyCopyablePointer' from 'const TriviallyCopyable *&&' to 'const TriviallyCopyable *'
+}
+template 
+void forwardToShowInt(T && t) {
+  showInt(static_cast(t));
+}
+void testTemplate() {
+  int a = 10;
+  forwardToShowInt(std::move(a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect [performance-move-const-arg]
+}
+
+struct Tmp {
+  Tmp();
+  Tmp(int &&a);
+  Tmp(int v1, int &&a);
+  Tmp(const char *&&s);
+  Tmp(TriviallyCopyable&& obj);
+  Tmp(const TriviallyCopyable *&&obj);
+  void showTmp(TriviallyCopyable&& t);
+  static void showTmpStatic(TriviallyCopyable&& t);
+};
+void testMethods() {
+  Tmp t;
+  int a = 10;
+  Tmp t1(std::move(a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect [performance-move-const-arg]
+  // CHECK-MESSAGES: :[[@LINE-13]]:13: note: consider changing the 1st parameter of 'Tmp' from 'int &&' to 'const int &'
+  Tmp t2(a, std::move(a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect [performance-move-const-arg]
+  // CHECK-MESSAGES: :[[@LINE-15]]:21: note: consider changing the 2nd parameter of 'Tmp' from 'int &&' to 'const int &'
+  const char* s = "";
+  Tmp t3(std::move(s));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the variable 's' of the trivially-copyable type 'const char *' has no effect [performance-move-const-arg]
+  // CHECK-MESSAGES: :[[@LINE-18]]:21: note: consider changing the 1st parameter of 'Tmp' from 'const char *&&' to 'const char *'
+  TriviallyCopyable *obj

[PATCH] D107450: [clang-tidy] Fix wrong FixIt in performance-move-const-arg

2021-12-28 Thread gehry via Phabricator via cfe-commits

Sockke marked 13 inline comments as done.
Sockke added inline comments.



Comment at: clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp:145
+  if ((!ReceivingCallExpr ||
+   ReceivingCallExpr->getDirectCallee()->isTemplateInstantiation()) &&
+  (!ReceivingConstructExpr ||

aaron.ballman wrote:
> Not all `CallExpr` objects have a direct callee, so this will crash (such as 
> calls through a function pointer rather than a direct function call). It may 
> be worth adding test coverage for this.
Use `InvocationParm` to determine whether objects have a direct callee.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107450/new/

https://reviews.llvm.org/D107450

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

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2021-12-28 Thread Marek Kurdej via Phabricator via cfe-commits

curdeius added a comment.

Looks nice already. Thanks for working on this.




Comment at: clang/include/clang/Format/Format.h:3691
+  ///  COULD lead to incorrect code formatting due to incorrect decisions made
+  ///  due to clang-formats lack of complete semantic information. As such 
extra
+  ///  care should be taken to review code changes made by the use of this

Nit.



Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.cpp:55
+// For `auto` language version, be conservative and assume we are < C++17
+KeepTemplateTemplateKW = (Style.Standard == FormatStyle::LS_Auto) ||
+ (Style.Standard < FormatStyle::LS_Cpp17);

Isn't it a better name?



Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.cpp:65
+  }
+
+  for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) {

Could we check `KeepTemplateTypenameKW` and early return here?
The optimizer might do some job, but I doubt it will get rid of the unnecessary 
finding of template keyword etc.
We could maybe avoid this variable altogether and return inside the switch, no?



Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.cpp:69
+
+// Find the first `template` keyword on this line
+while (Tok && Tok->isNot(tok::kw_template))

Please finish comments with full stops, here and below.



Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.cpp:75
+
+// Find the corresponding `<`. Might be on the next line
+Tok = Tok->Next;





Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.h:12
+/// This file declares TemplateArgumentKeywordFixer, a TokenAnalyzer that
+// /enforces / consistent usage of `typename`/`class` for template arguments.
+///

Or even better, make it closer to what's in cpp file.



Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.h:36
+} // end namespace format
+} // end namespace clang
+

Please make the comments consistent with other files.



Comment at: clang/unittests/Format/TemplateArgumentKeywordFixerTest.cpp:87
+  // `typename` outside of a template should stay unchanged
+  verifyFormat("typename T::nested", "typename T::nested", Style);
+  // `typename` inside a template should stay unchanged if it refers to a 
nested

Second argument can be removed if you add an overload having Expected=Code, so 
as to match `verifyFormat` in other files.



Comment at: clang/unittests/Format/TemplateArgumentKeywordFixerTest.cpp:155
+  // We also format template argument lists for `using`.
+  // There are no special semantics re. deduction guides and the normal
+  // implementation just works. But better test it before this breaks due to

Don't repeat comments, please. It's the same as below in deduction guides test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116290/new/

https://reviews.llvm.org/D116290

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

[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

2021-12-28 Thread Marek Kurdej via Phabricator via cfe-commits

curdeius requested changes to this revision.
curdeius added a comment.
This revision now requires changes to proceed.

Wasn't assert enough? Could you add a test with code that provokes the problem?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116318/new/

https://reviews.llvm.org/D116318

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

[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

2021-12-28 Thread Marek Kurdej via Phabricator via cfe-commits

curdeius added a comment.

Could you have a look at preceding reviews and see if there wasn't a similar 
patch before?
I think that this option is a bit too limited.
Only removing braces doesn't seem enough.
Also, one should probably be able to decide when to add/remove them by e.g. 
setting the number of lines in what's considered short blocks.




Comment at: clang/docs/ClangFormatStyleOptions.rst:3398
 
+**RemoveBracesLLVM** (``Boolean``) :versionbadge:`clang-format 14`
+  Remove optional braces of control statements (``if``, ``else``, ``for``,

I don't think it's a good idea to make this option dependent on a particular 
style. Even if it works for LLVM style only for the moment.



Comment at: clang/docs/ClangFormatStyleOptions.rst:3419
+   if (isa(D)) {  vs. if (isa(D)) {
+ for (auto *A : D.attrs()) { for (auto *A : D.attrs())
+   if (shouldProcessAttr(A)) { if (shouldProcessAttr(A))

I'm not sure if the braces on the right should be removed in the for loop.
There should probably be an option to set the  minimum number of 
lines/statements inside a control statement to control adding/removing braces.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116316/new/

https://reviews.llvm.org/D116316

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

[PATCH] D116314: [Clangfmt] Add style to separate definition blocks

2021-12-28 Thread Marek Kurdej via Phabricator via cfe-commits

curdeius requested changes to this revision.
curdeius added a comment.
This revision now requires changes to proceed.

Thanks for working on this.
Please add unit tests that verify the expected behaviour. Keep them simple. You 
can inspire yourself from existing tests.




Comment at: clang/docs/ClangFormatStyleOptions.rst:3398
 
+**SeparateDefinitionBlocks** (``SeparateDefinitionStyle``) 
:versionbadge:`clang-format 15`
+  Specifies the use of empty lines to separate definition blocks, including 
classes,

Unless it takes a lot of time to review this patch 🙂



Comment at: clang/lib/Format/Format.cpp:2027
+// namespaces by inserting a line break between them.
+class DefinitionBlockSeparator : public TokenAnalyzer {
+public:

Please consider moving it to a different file.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116314/new/

https://reviews.llvm.org/D116314

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

[PATCH] D116329: [clang-check] Adjust argument adjusters for clang-check to strip options blocking the static analyzer

2021-12-28 Thread Pavel Labath via Phabricator via cfe-commits

labath resigned from this revision.
labath added a comment.

I'm sorry, but I don't feel qualified to review this. Last time I saw this file 
was in 2013.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116329/new/

https://reviews.llvm.org/D116329

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

[PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

2021-12-28 Thread John Ericson via Phabricator via cfe-commits

Ericson2314 updated this revision to Diff 396338.
Ericson2314 added a comment.



1. Updating D99484 : Use `GNUInstallDirs` to 
support custom installation dirs. #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

rebase atop other PRs


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99484/new/

https://reviews.llvm.org/D99484

Files:
  clang-tools-extra/clang-doc/tool/CMakeLists.txt
  clang-tools-extra/clang-include-fixer/find-all-symbols/tool/CMakeLists.txt
  clang-tools-extra/clang-include-fixer/tool/CMakeLists.txt
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/modularize/CMakeLists.txt
  clang/CMakeLists.txt
  clang/cmake/modules/AddClang.cmake
  clang/tools/c-index-test/CMakeLists.txt
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-nvlink-wrapper/CMakeLists.txt
  clang/tools/clang-rename/CMakeLists.txt
  clang/tools/libclang/CMakeLists.txt
  clang/tools/scan-build-py/CMakeLists.txt
  clang/tools/scan-build/CMakeLists.txt
  clang/tools/scan-view/CMakeLists.txt
  clang/utils/hmaptool/CMakeLists.txt
  compiler-rt/cmake/base-config-ix.cmake
  libc/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/cmake/Modules/HandleLibCXXABI.cmake
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  mlir/CMakeLists.txt
  mlir/cmake/modules/AddMLIR.cmake
  openmp/CMakeLists.txt
  openmp/libompd/src/CMakeLists.txt
  openmp/runtime/src/CMakeLists.txt
  openmp/tools/multiplex/CMakeLists.txt
  polly/CMakeLists.txt
  polly/cmake/CMakeLists.txt
  polly/lib/External/CMakeLists.txt
  pstl/CMakeLists.txt

Index: pstl/CMakeLists.txt
===
--- pstl/CMakeLists.txt
+++ pstl/CMakeLists.txt
@@ -7,6 +7,8 @@
 #===--===##
 cmake_minimum_required(VERSION 3.13.4)
 
+include(GNUInstallDirs)
+
 set(PARALLELSTL_VERSION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/include/pstl/internal/pstl_config.h")
 file(STRINGS "${PARALLELSTL_VERSION_FILE}" PARALLELSTL_VERSION_SOURCE REGEX "#define _PSTL_VERSION .*$")
 string(REGEX REPLACE "#define _PSTL_VERSION (.*)$" "\\1" PARALLELSTL_VERSION_SOURCE "${PARALLELSTL_VERSION_SOURCE}")
@@ -90,10 +92,10 @@
   "${CMAKE_CURRENT_BINARY_DIR}/ParallelSTLConfigVersion.cmake"
 DESTINATION lib/cmake/ParallelSTL)
 install(DIRECTORY include/
-DESTINATION include
+DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}"
 PATTERN "*.in" EXCLUDE)
 install(FILES "${PSTL_CONFIG_SITE_PATH}"
-DESTINATION include)
+DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}")
 
 add_custom_target(install-pstl
   COMMAND "${CMAKE_COMMAND}" -P "${PROJECT_BINARY_DIR}/cmake_install.cmake" -DCOMPONENT=ParallelSTL)
Index: polly/lib/External/CMakeLists.txt
===
--- polly/lib/External/CMakeLists.txt
+++ polly/lib/External/CMakeLists.txt
@@ -290,7 +290,7 @@
 install(DIRECTORY
   ${ISL_SOURCE_DIR}/include/
   ${ISL_BINARY_DIR}/include/
-  DESTINATION include/polly
+  DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/polly"
   FILES_MATCHING
   PATTERN "*.h"
   PATTERN "CMakeFiles" EXCLUDE
Index: polly/cmake/CMakeLists.txt
===
--- polly/cmake/CMakeLists.txt
+++ polly/cmake/CMakeLists.txt
@@ -83,14 +83,15 @@
 set(POLLY_CONFIG_LLVM_CMAKE_DIR "${LLVM_BINARY_DIR}/${LLVM_INSTALL_PACKAGE_DIR}")
 set(POLLY_CONFIG_CMAKE_DIR "${POLLY_INSTALL_PREFIX}/${POLLY_INSTALL_PACKAGE_DIR}")
 set(POLLY_CONFIG_LIBRARY_DIRS "${POLLY_INSTALL_PREFIX}/lib${LLVM_LIBDIR_SUFFIX}")
+get_filename_component(base_includedir "${CMAKE_INSTALL_INCLUDEDIR}" ABSOLUTE BASE_DIR "${POLLY_INSTALL_PREFIX}")
 if (POLLY_BUNDLED_ISL)
   set(POLLY_CONFIG_INCLUDE_DIRS
-"${POLLY_INSTALL_PREFIX}/include"
-"${POLLY_INSTALL_PREFIX}/include/polly"
+"${base_includedir}"
+"${base_includedir}/polly"
 )
 else()
   set(POLLY_CONFIG_INCLUDE_DIRS
-"${POLLY_INSTALL_PREFIX}/include"
+"${base_includedir}"
 ${ISL_INCLUDE_DIRS}
 )
 endif()
@@ -100,12 +101,12 @@
 foreach(tgt IN LISTS POLLY_CONFIG_EXPORTED_TARGETS)
   get_target_property(tgt_type ${tgt} TYPE)
   if (tgt_type STREQUAL "EXECUTABLE")
-set(tgt_prefix "bin/")
+set(tgt_prefix "${CMAKE_INSTALL_FULL_BINDIR}/")
   else()
-set(tgt_prefix "lib/")
+set(tgt_prefix "${CMAKE_INSTALL_FULL_LIBDIR}/")
   endif()
 
-  set(tgt_path "${CMAKE_INSTALL_PREFIX}/${tgt_prefix}$")
+  set(tgt_path "${tgt_prefix}$")
   file(RELATIVE_PATH tgt_path ${POLLY_CONFIG_CMAKE_DIR} ${tgt_path})
 
   if (NOT tgt_type STREQUAL "INTERFACE_LIBRARY")
Index: polly/CMakeLists.txt
===

[PATCH] D70401: [RISCV] Complete RV32E/ilp32e implementation

2021-12-28 Thread Wang Pengcheng via Phabricator via cfe-commits

pcwang-thead added a comment.

ping.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70401/new/

https://reviews.llvm.org/D70401

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

[PATCH] D116328: [ast-matchers] Add hasSubstmt() traversal matcher for caseStmt(), defaultStmt()

2021-12-28 Thread Richard via Phabricator via cfe-commits

LegalizeAdulthood updated this revision to Diff 396392.
LegalizeAdulthood added a comment.

Update based on comments from lint check


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116328/new/

https://reviews.llvm.org/D116328

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -311,6 +311,18 @@
   EXPECT_TRUE(notMatches("struct Foo {};", NamedNotRecord));
 }
 
+TEST_P(ASTMatchersTest, HasCaseSubstmt) {
+  EXPECT_TRUE(matches(
+  "void f() { switch (1) { case 1: return; break; default: break; } }",
+  traverse(TK_AsIs, caseStmt(hasSubstmt(returnStmt());
+}
+
+TEST_P(ASTMatchersTest, HasDefaultSubstmt) {
+  EXPECT_TRUE(matches(
+  "void f() { switch (1) { case 1: return; break; default: break; } }",
+  traverse(TK_AsIs, defaultStmt(hasSubstmt(breakStmt());
+}
+
 TEST_P(ASTMatchersTest, HasCastKind) {
   EXPECT_TRUE(
   matches("char *p = 0;",
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -66,7 +66,7 @@
 
 #define REGISTER_MATCHER(name) \
   registerMatcher(#name, internal::makeMatcherAutoMarshall(\
- ::clang::ast_matchers::name, #name));
+ ::clang::ast_matchers::name, #name))
 
 #define REGISTER_MATCHER_OVERLOAD(name)\
   registerMatcher(#name,   \
@@ -143,7 +143,7 @@
   REGISTER_MATCHER(atomicType);
   REGISTER_MATCHER(attr);
   REGISTER_MATCHER(autoType);
-  REGISTER_MATCHER(autoreleasePoolStmt)
+  REGISTER_MATCHER(autoreleasePoolStmt);
   REGISTER_MATCHER(binaryConditionalOperator);
   REGISTER_MATCHER(binaryOperator);
   REGISTER_MATCHER(binaryOperation);
@@ -355,6 +355,7 @@
   REGISTER_MATCHER(hasSpecializedTemplate);
   REGISTER_MATCHER(hasStaticStorageDuration);
   REGISTER_MATCHER(hasStructuredBlock);
+  REGISTER_MATCHER(hasSubstmt);
   REGISTER_MATCHER(hasSyntacticForm);
   REGISTER_MATCHER(hasTargetDecl);
   REGISTER_MATCHER(hasTemplateArgument);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2429,6 +2429,7 @@
 /// Matches co_await expressions where the type of the promise is dependent
 extern const internal::VariadicDynCastAllOfMatcher
 dependentCoawaitExpr;
+
 /// Matches co_yield expressions.
 ///
 /// Given
@@ -7716,6 +7717,25 @@
   return InnerMatcher.matches(*Node.getLHS(), Finder, Builder);
 }
 
+/// Matches the substatement associated with a case or default statement.
+///
+/// Given
+/// \code
+///   switch (1) { case 1: break; case 2: return; break; default: return; break;
+///   }
+/// \endcode
+///
+/// caseStmt(hasSubstmt(returnStmt()))
+///   matches "case 2: return;"
+/// defaultStmt(hasSubstmt(returnStmt()))
+///   matches "default: return;"
+AST_POLYMORPHIC_MATCHER_P(hasSubstmt,
+  AST_POLYMORPHIC_SUPPORTED_TYPES(CaseStmt,
+  DefaultStmt),
+  internal::Matcher, InnerMatcher) {
+  return InnerMatcher.matches(*Node.getSubStmt(), Finder, Builder);
+}
+
 /// Matches declaration that has a given attribute.
 ///
 /// Given
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -7298,6 +7298,16 @@
 
 
 
+MatcherCaseStmt>hasSubstmtMatcherStmt> InnerMatcher
+Matches the substatement associated with a case statement.
+
+Given
+  switch (1) { case 1: break; case 2: return; break; default: return; break; }
+caseStmt(hasSubstmt(returnStmt()))
+  matches "case 2: return;"
+
+
+
 MatcherCastExpr>hasSourceExpressionMatcherExpr> InnerMatcher
 Matches if the cast's source expression
 or opaque value's source expression matches the given matcher.
@@ -7653,6 +7663,16 @@
 
 
 
+MatcherDefaultStmt>hasSubstmtMatcher

[PATCH] D116283: Added an option to add a space between operator overloading and opening parentheses in clang-format

2021-12-28 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:14545
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace2);
+
+  FormatStyle SpaceAfterOperatorOverloading = getLLVMStyle();

There should be a PARSE unit test too please



Comment at: clang/unittests/Format/FormatTest.cpp:14546
+
+  FormatStyle SpaceAfterOperatorOverloading = getLLVMStyle();
+  SpaceAfterOperatorOverloading.SpaceBeforeParens = FormatStyle::SBPO_Custom;




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116283/new/

https://reviews.llvm.org/D116283

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

[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

2021-12-28 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

See D95168: [clang-format] Add Insert/Remove Braces option 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116316/new/

https://reviews.llvm.org/D116316

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

[PATCH] D116170: [clang-format] Add penalty for breaking after '('

2021-12-28 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:18515
 
+TEST_F(FormatTest, BreakPenaltyAfterLParen) {
+   FormatStyle Style = getLLVMStyle();

1) fix the formatting
2) add a default case which shows how the bracket will be without setting it to 
100
3) show other uses of '(' like casts, declarations etc..


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116170/new/

https://reviews.llvm.org/D116170

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

[PATCH] D116314: [Clangfmt] Add style to separate definition blocks

2021-12-28 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:3404
+
+  * ``SDS_Leave`` (in configuration: ``Leave``)
+Leave definition blocks separated as they are.

This enumeration surely needs 3 settings


```
Leave, // don't touch the code, if I have a line leave it alone if not 
don't add one
Always,   // always add a line
Never,// never add a line and remove the lint if one exists
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116314/new/

https://reviews.llvm.org/D116314

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

[PATCH] D116161: [Clang] Add an overload for emitUnaryBuiltin.

2021-12-28 Thread Florian Hahn via Phabricator via cfe-commits

fhahn added a comment.

In D116161#3211178 , @junaire wrote:

> In order to use `emitUnaryBuiltin` in other cases, I changed the function 
> interface.
> This allows us to use it in all `Builder.CreateUnaryIntrinsic()` cases, but 
> will make
> the function body very small.

I think we should extend & use the existing `emitUnaryBuiltin`. I think in most 
cases where it cannot be used straight away you should be able to slightly 
rewrite the existing code to not rely on `llvm::Type`. Then there should be no 
need to call `EmitScalarExpr` early (an example is in the inline comments)




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:535
 // matching the argument type.
 static Value *emitUnaryBuiltin(CodeGenFunction &CGF,
const CallExpr *E,

I think we should extend this `emitUnaryBuiltin` function, rather than having a 
second one.

e.g.

```
 static Value *emitUnaryBuiltin(CodeGenFunction &CGF,
const CallExpr *E,
-   unsigned IntrinsicID) {
+   unsigned IntrinsicID, llvm::StringRef Name = 
"") {
   llvm::Value *Src0 = CGF.EmitScalarExpr(E->getArg(0));

   Function *F = CGF.CGM.getIntrinsic(IntrinsicID, Src0->getType());
-  return CGF.Builder.CreateCall(F, Src0);
+  return CGF.Builder.CreateCall(F, Src0, Name);
+}
```



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3190
 };
 Value *Op0 = EmitScalarExpr(E->getArg(0));
+return RValue::get(emitUnaryBuiltin(

IICU the only reason to call `EmitScalarExpr` early is the use in 
`GetIntrinsicID`, but it could solely rely on `QualType`:


```
-auto GetIntrinsicID = [](QualType QT, llvm::Type *IrTy) {
-  if (IrTy->isIntOrIntVectorTy()) {
-if (auto *VecTy = QT->getAs())
-  QT = VecTy->getElementType();
-if (QT->isSignedIntegerType())
-  return llvm::Intrinsic::vector_reduce_smax;
-else
-  return llvm::Intrinsic::vector_reduce_umax;
-  }
+auto GetIntrinsicID = [](QualType QT) {
+  if (auto *VecTy = QT->getAs())
+QT = VecTy->getElementType();
+  if (QT->isSignedIntegerType())
+return llvm::Intrinsic::vector_reduce_smax;
+  if (QT->isUnsignedIntegerType())
+return llvm::Intrinsic::vector_reduce_umax;
+  assert(QT->isFloatingType() && "must have a float here");
   return llvm::Intrinsic::vector_reduce_fmax;
 };
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116161/new/

https://reviews.llvm.org/D116161

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

[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

2021-12-28 Thread Owen Pan via Phabricator via cfe-commits

owenpan added a comment.

In D116318#3211258 , @curdeius wrote:

> Wasn't assert enough? Could you add a test with code that provokes the 
> problem?

This fix is an NFC, so there will be no (user) test case for it. :)

The current behavior does not match the description on line 35, and I need this 
function to be able to return null to simply my code. Otherwise, l would have 
to do something like the following:

  if (Position == 0)
return false;
  const FormatToken *Previous = AllTokens[Position - 1];
  assert(Previous);
  return Previous->is(tok::comment);

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116318/new/

https://reviews.llvm.org/D116318

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

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2021-12-28 Thread Adrian Vogelsgesang via Phabricator via cfe-commits

avogelsgesang updated this revision to Diff 396401.
avogelsgesang marked 7 inline comments as done.
avogelsgesang added a comment.

Thank you for your review, @curdeius!

I updated my change to address most of your comments.
Regarding "KeepTemplateTemplateKW", I think we had a misunderstanding (see 
replies to inline comments).
Could you take a 2nd look at those comments?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116290/new/

https://reviews.llvm.org/D116290

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/CMakeLists.txt
  clang/lib/Format/Format.cpp
  clang/lib/Format/TemplateArgumentKeywordFixer.cpp
  clang/lib/Format/TemplateArgumentKeywordFixer.h
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/TemplateArgumentKeywordFixerTest.cpp

Index: clang/unittests/Format/TemplateArgumentKeywordFixerTest.cpp
===
--- /dev/null
+++ clang/unittests/Format/TemplateArgumentKeywordFixerTest.cpp
@@ -0,0 +1,219 @@
+//===- unittest/Format/TemplateArgumentKeywordFixerTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Format/Format.h"
+
+#include "FormatTestUtils.h"
+#include "TestLexer.h"
+#include "gtest/gtest.h"
+
+#include "../../lib/Format/TemplateArgumentKeywordFixer.h"
+
+using testing::ScopedTrace;
+
+namespace clang {
+namespace format {
+namespace {
+
+class TemplateArgumentKeywordFixerTest : public ::testing::Test {
+protected:
+  std::string format(llvm::StringRef Code,
+ const std::vector &Ranges,
+ const FormatStyle &Style) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+FormattingAttemptStatus Status;
+tooling::Replacements Replaces =
+reformat(Style, Code, Ranges, "", &Status);
+EXPECT_TRUE(Status.FormatComplete);
+auto Result = applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  std::string format(llvm::StringRef Code, const FormatStyle &Style) {
+return format(Code,
+  /*Ranges=*/{1, tooling::Range(0, Code.size())}, Style);
+  }
+
+  void _verifyFormat(const char *File, int Line, llvm::StringRef Expected,
+ llvm::StringRef Code, const FormatStyle &Style) {
+ScopedTrace t(File, Line, ::testing::Message() << Code.str());
+EXPECT_EQ(Expected.str(), format(Expected, Style))
+<< "Expected code is not stable";
+EXPECT_EQ(Expected.str(), format(Code, Style));
+EXPECT_EQ(Expected.str(), format(test::messUp(Code), Style));
+  }
+
+  void _verifyFormat(const char *File, int Line, llvm::StringRef Code,
+ const FormatStyle &Style) {
+_verifyFormat(File, Line, Code, Code, Style);
+  }
+};
+
+#define verifyFormat(...) _verifyFormat(__FILE__, __LINE__, __VA_ARGS__)
+
+TEST_F(TemplateArgumentKeywordFixerTest, DisabledByDefault) {
+  FormatStyle Style = getLLVMStyle();
+  EXPECT_EQ(Style.TemplateArgumentKeyword, FormatStyle::TAS_Leave);
+  verifyFormat("template  class A", "template  class A",
+   Style);
+  verifyFormat("template  class A", "template  class A",
+   Style);
+  verifyFormat("template  class A",
+   "template  class A", Style);
+}
+
+TEST_F(TemplateArgumentKeywordFixerTest, FormatsAsClass) {
+  FormatStyle Style = getLLVMStyle();
+  Style.TemplateArgumentKeyword = FormatStyle::TAS_Class;
+  verifyFormat("template  class A", "template  class A",
+   Style);
+  verifyFormat("template  class A", "template  class A",
+   Style);
+  verifyFormat("template  class A;",
+   "template  class A;", Style);
+  verifyFormat("template  class A;",
+   "template  class A;", Style);
+  verifyFormat("template  class Y> class A;",
+   "template  typename Y> class A;", Style);
+}
+
+TEST_F(TemplateArgumentKeywordFixerTest, ClassLeavesOtherTypenames) {
+  FormatStyle Style = getLLVMStyle();
+  Style.TemplateArgumentKeyword = FormatStyle::TAS_Class;
+  // `typename` outside of a template should stay unchanged.
+  verifyFormat("typename T::nested", Style);
+  // `typename` inside a template should stay unchanged if it refers to a nested
+  // type.
+  verifyFormat(
+  "template  class A;",
+  "template  class A;", Style);
+  // A legitimate `typename` might also occur in a nested list, directly after
+  // the `,` token It's not sufficient to just check for a comma in front of
+  // `typename`.
+  verifyFormat("templat

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2021-12-28 Thread Adrian Vogelsgesang via Phabricator via cfe-commits

avogelsgesang added inline comments.



Comment at: clang/include/clang/Format/Format.h:3691
+  ///  COULD lead to incorrect code formatting due to incorrect decisions made
+  ///  due to clang-formats lack of complete semantic information. As such 
extra
+  ///  care should be taken to review code changes made by the use of this

curdeius wrote:
> Nit.
Good catch!

I just copied that phrase from `QualifierAlignment` and overlooked that I also 
copied the typo. Fixed the typo in both locations.



Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.cpp:55
+// For `auto` language version, be conservative and assume we are < C++17
+KeepTemplateTemplateKW = (Style.Standard == FormatStyle::LS_Auto) ||
+ (Style.Standard < FormatStyle::LS_Cpp17);

curdeius wrote:
> Isn't it a better name?
This flag is actually about the usage of the `class` keyword instead of the 
`typename` keyword for template-template arguments.
`true` means: "Keep using the `class` instead of the `typename` keyword for 
template-template arguments."

I think the name `KeepTemplateTypenameKW` is wrong. "[...]TypenameKW = true" 
would mean "use `typename` instead of `class`" to me, and that's exactly the 
opposite way around.

As such, I think `KeepTemplateTemplateKW` is in fact the better name. If we 
want to make it even more explicit, we could also use 
`KeepTemplateTemplateClassKW`. What do you think?



Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.cpp:65
+  }
+
+  for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) {

curdeius wrote:
> Could we check `KeepTemplateTypenameKW` and early return here?
> The optimizer might do some job, but I doubt it will get rid of the 
> unnecessary finding of template keyword etc.
> We could maybe avoid this variable altogether and return inside the switch, 
> no?
I think you misunderstood the semantics of `KeepTemplateTypenameKW`? Or did I 
misunderstand your comment?

For both `KeepTemplateTemplateKW = true` and `KeepTemplateTemplateKW = false`, 
the loop below still reformats `class` to `typename`.

E.g., for the input
```
template class X>
```
and `KeepTemplateTemplateKW = true`, we produce
```
template class X>
```
For the same input with `KeepTemplateTemplateKW = false`, we produce
```
template typename X>
```



Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.cpp:69
+
+// Find the first `template` keyword on this line
+while (Tok && Tok->isNot(tok::kw_template))

curdeius wrote:
> Please finish comments with full stops, here and below.
Fixed here and all other places I could find in my change



Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.h:36
+} // end namespace format
+} // end namespace clang
+

curdeius wrote:
> Please make the comments consistent with other files.
Consistent with which other files?
My comments here are consistent with `NamespaceEndCommentsFixer.h` and 
`QualifierAlignmentFixer.h` which I used for reference while writing this change



Comment at: clang/unittests/Format/TemplateArgumentKeywordFixerTest.cpp:155
+  // We also format template argument lists for `using`.
+  // There are no special semantics re. deduction guides and the normal
+  // implementation just works. But better test it before this breaks due to

curdeius wrote:
> Don't repeat comments, please. It's the same as below in deduction guides 
> test.
removed comment in both test cases. Reading it again, I agree that the comment 
didn't provide much value in the first place


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116290/new/

https://reviews.llvm.org/D116290

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

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2021-12-28 Thread Adrian Vogelsgesang via Phabricator via cfe-commits

avogelsgesang updated this revision to Diff 396403.
avogelsgesang marked an inline comment as done.
avogelsgesang added a comment.

Fix namespace comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116290/new/

https://reviews.llvm.org/D116290

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/CMakeLists.txt
  clang/lib/Format/Format.cpp
  clang/lib/Format/TemplateArgumentKeywordFixer.cpp
  clang/lib/Format/TemplateArgumentKeywordFixer.h
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/TemplateArgumentKeywordFixerTest.cpp

Index: clang/unittests/Format/TemplateArgumentKeywordFixerTest.cpp
===
--- /dev/null
+++ clang/unittests/Format/TemplateArgumentKeywordFixerTest.cpp
@@ -0,0 +1,219 @@
+//===- unittest/Format/TemplateArgumentKeywordFixerTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Format/Format.h"
+
+#include "FormatTestUtils.h"
+#include "TestLexer.h"
+#include "gtest/gtest.h"
+
+#include "../../lib/Format/TemplateArgumentKeywordFixer.h"
+
+using testing::ScopedTrace;
+
+namespace clang {
+namespace format {
+namespace {
+
+class TemplateArgumentKeywordFixerTest : public ::testing::Test {
+protected:
+  std::string format(llvm::StringRef Code,
+ const std::vector &Ranges,
+ const FormatStyle &Style) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+FormattingAttemptStatus Status;
+tooling::Replacements Replaces =
+reformat(Style, Code, Ranges, "", &Status);
+EXPECT_TRUE(Status.FormatComplete);
+auto Result = applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  std::string format(llvm::StringRef Code, const FormatStyle &Style) {
+return format(Code,
+  /*Ranges=*/{1, tooling::Range(0, Code.size())}, Style);
+  }
+
+  void _verifyFormat(const char *File, int Line, llvm::StringRef Expected,
+ llvm::StringRef Code, const FormatStyle &Style) {
+ScopedTrace t(File, Line, ::testing::Message() << Code.str());
+EXPECT_EQ(Expected.str(), format(Expected, Style))
+<< "Expected code is not stable";
+EXPECT_EQ(Expected.str(), format(Code, Style));
+EXPECT_EQ(Expected.str(), format(test::messUp(Code), Style));
+  }
+
+  void _verifyFormat(const char *File, int Line, llvm::StringRef Code,
+ const FormatStyle &Style) {
+_verifyFormat(File, Line, Code, Code, Style);
+  }
+};
+
+#define verifyFormat(...) _verifyFormat(__FILE__, __LINE__, __VA_ARGS__)
+
+TEST_F(TemplateArgumentKeywordFixerTest, DisabledByDefault) {
+  FormatStyle Style = getLLVMStyle();
+  EXPECT_EQ(Style.TemplateArgumentKeyword, FormatStyle::TAS_Leave);
+  verifyFormat("template  class A", "template  class A",
+   Style);
+  verifyFormat("template  class A", "template  class A",
+   Style);
+  verifyFormat("template  class A",
+   "template  class A", Style);
+}
+
+TEST_F(TemplateArgumentKeywordFixerTest, FormatsAsClass) {
+  FormatStyle Style = getLLVMStyle();
+  Style.TemplateArgumentKeyword = FormatStyle::TAS_Class;
+  verifyFormat("template  class A", "template  class A",
+   Style);
+  verifyFormat("template  class A", "template  class A",
+   Style);
+  verifyFormat("template  class A;",
+   "template  class A;", Style);
+  verifyFormat("template  class A;",
+   "template  class A;", Style);
+  verifyFormat("template  class Y> class A;",
+   "template  typename Y> class A;", Style);
+}
+
+TEST_F(TemplateArgumentKeywordFixerTest, ClassLeavesOtherTypenames) {
+  FormatStyle Style = getLLVMStyle();
+  Style.TemplateArgumentKeyword = FormatStyle::TAS_Class;
+  // `typename` outside of a template should stay unchanged.
+  verifyFormat("typename T::nested", Style);
+  // `typename` inside a template should stay unchanged if it refers to a nested
+  // type.
+  verifyFormat(
+  "template  class A;",
+  "template  class A;", Style);
+  // A legitimate `typename` might also occur in a nested list, directly after
+  // the `,` token It's not sufficient to just check for a comma in front of
+  // `typename`.
+  verifyFormat("template , class Z "
+   "= void> class A;",
+   "template , "
+   "typename Z = void> class A;",
+   Style);
+  // `typename` might also occur in a function call, not only in a ty

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2021-12-28 Thread Adrian Vogelsgesang via Phabricator via cfe-commits

avogelsgesang added inline comments.



Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.h:36
+} // end namespace format
+} // end namespace clang
+

avogelsgesang wrote:
> curdeius wrote:
> > Please make the comments consistent with other files.
> Consistent with which other files?
> My comments here are consistent with `NamespaceEndCommentsFixer.h` and 
> `QualifierAlignmentFixer.h` which I used for reference while writing this 
> change
ok, now I understood what you meant...
You meant consistency with the other files in this commit, in particular with 
the corresponding cpp file `TemplateArgumentKeywordFixer.h`.

Fun fact: I copied this inconsistency from `NamespaceEndCommentsFixer.h`/`.cpp` 
which also use different comment styles between `.h` and `.cpp`.

I updated my change to consistently use the `// namespace` style


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116290/new/

https://reviews.llvm.org/D116290

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

[PATCH] D115031: [AST] Print NTTP args as string-literals when possible

2021-12-28 Thread Zhihao Yuan via Phabricator via cfe-commits

lichray updated this revision to Diff 396408.
lichray added a comment.

- Put tests in TypePrinterTest.cpp


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115031/new/

https://reviews.llvm.org/D115031

Files:
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/AST/PrettyPrinter.h
  clang/include/clang/Basic/CharInfo.h
  clang/lib/AST/APValue.cpp
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/TemplateBase.cpp
  clang/test/SemaTemplate/temp_arg_string_printing.cpp
  clang/unittests/AST/TypePrinterTest.cpp

Index: clang/unittests/AST/TypePrinterTest.cpp
===
--- clang/unittests/AST/TypePrinterTest.cpp
+++ clang/unittests/AST/TypePrinterTest.cpp
@@ -62,4 +62,35 @@
   ASSERT_TRUE(PrintedTypeMatches(
   Code, {}, Matcher, "const N::Type &",
   [](PrintingPolicy &Policy) { Policy.FullyQualifiedName = true; }));
-}
\ No newline at end of file
+}
+
+TEST(TypePrinter, TemplateIdWithNTTP) {
+  constexpr char Code[] = R"cpp(
+template 
+struct Str {
+  constexpr Str(char const (&s)[N]) { __builtin_memcpy(value, s, N); }
+  char value[N];
+};
+template  class ASCII {};
+
+ASCII<"this nontype template argument is too long to print"> x;
+  )cpp";
+  auto Matcher = classTemplateSpecializationDecl(
+  hasName("ASCII"), has(cxxConstructorDecl(
+isMoveConstructor(),
+has(parmVarDecl(hasType(qualType().bind("id")));
+
+  ASSERT_TRUE(PrintedTypeMatches(
+  Code, {"-std=c++20"}, Matcher,
+  R"(ASCII<{"this nontype template argument is [...]"}> &&)",
+  [](PrintingPolicy &Policy) {
+Policy.EntireContentsOfLargeArray = false;
+  }));
+
+  ASSERT_TRUE(PrintedTypeMatches(
+  Code, {"-std=c++20"}, Matcher,
+  R"(ASCII<{"this nontype template argument is too long to print"}> &&)",
+  [](PrintingPolicy &Policy) {
+Policy.EntireContentsOfLargeArray = true;
+  }));
+}
Index: clang/test/SemaTemplate/temp_arg_string_printing.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/temp_arg_string_printing.cpp
@@ -0,0 +1,141 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -ast-print %s | FileCheck %s
+
+using size_t = __SIZE_TYPE__;
+static_assert(__has_builtin(__make_integer_seq));
+
+template  class idx_seq {};
+template  using make_idx_seq = __make_integer_seq;
+
+template 
+struct Str {
+  constexpr Str(CharT const (&s)[N]) : Str(s, make_idx_seq()) {}
+  CharT value[N];
+
+private:
+  template 
+  constexpr Str(CharT const (&s)[N], idx_seq) : value{s[I]...} {}
+};
+
+template  class ASCII {};
+
+void not_string() {
+  // CHECK{LITERAL}: ASCII<{{9, -1, 42}}>
+  new ASCII<(int[]){9, -1, 42}>;
+  // CHECK{LITERAL}: ASCII<{{3.14e+00, 0.00e+00, 4.20e+01}}>
+  new ASCII<(double[]){3.14, 0., 42.}>;
+}
+
+void narrow() {
+  // CHECK{LITERAL}: ASCII<{""}>
+  new ASCII<"">;
+  // CHECK{LITERAL}: ASCII<{"the quick brown fox jumps"}>
+  new ASCII<"the quick brown fox jumps">;
+  // CHECK{LITERAL}: ASCII<{"OVER THE LAZY DOG 0123456789"}>
+  new ASCII<"OVER THE LAZY DOG 0123456789">;
+  // CHECK{LITERAL}: ASCII<{"\\`~!@#$%^&*()_+-={}[]|\'\";:,.<>?/"}>
+  new ASCII?/)">;
+  // CHECK{LITERAL}: ASCII<{{101, 115, 99, 97, 112, 101, 0, 0}}>
+  new ASCII<"escape\0">;
+  // CHECK{LITERAL}: ASCII<{"escape\r\n"}>
+  new ASCII<"escape\r\n">;
+  // CHECK{LITERAL}: ASCII<{"escape\\\t\f\v"}>
+  new ASCII<"escape\\\t\f\v">;
+  // CHECK{LITERAL}: ASCII<{"escape\a\bc"}>
+  new ASCII<"escape\a\b\c">;
+  // CHECK{LITERAL}: ASCII<{{110, 111, 116, 17, 0}}>
+  new ASCII<"not\x11">;
+  // CHECK{LITERAL}: ASCII<{{18, 20, 127, 16, 1, 32, 97, 98, 99, 0}}>
+  new ASCII<"\x12\x14\x7f\x10\x01 abc">;
+  // CHECK{LITERAL}: ASCII<{{18, 20, 127, 16, 1, 32, 97, 98, 99, 100, ...}}>
+  new ASCII<"\x12\x14\x7f\x10\x01 abcd">;
+  // CHECK{LITERAL}: ASCII<{"print more characters as string"}>
+  new ASCII<"print more characters as string">;
+  // CHECK{LITERAL}: ASCII<{"print even more characters as string"}>
+  new ASCII<"print even more characters as string">;
+  // CHECK{LITERAL}: ASCII<{"print many characters no more than[...]"}>
+  new ASCII<"print many characters no more than a limit">;
+  // CHECK{LITERAL}: ASCII<{"\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r"}>
+  new ASCII<"\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r">;
+  // CHECK{LITERAL}: ASCII<{"\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n[...]"}>
+  new ASCII<"\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n">;
+}
+
+void wide() {
+  // CHECK{LITERAL}: ASCII<{L""}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"the quick brown fox jumps"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"OVER THE LAZY DOG 0123456789"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"\\

[clang] e6e7bdd - Drop unnecessary const from return types (NFC)

2021-12-28 Thread Kazu Hirata via cfe-commits


Author: Kazu Hirata
Date: 2021-12-28T10:01:39-08:00
New Revision: e6e7bdd6a90ca8ba896fb92f6e0e642d42c84efc

URL: 
https://github.com/llvm/llvm-project/commit/e6e7bdd6a90ca8ba896fb92f6e0e642d42c84efc
DIFF: 
https://github.com/llvm/llvm-project/commit/e6e7bdd6a90ca8ba896fb92f6e0e642d42c84efc.diff

LOG: Drop unnecessary const from return types (NFC)

Identified with readability-const-return-type.

Added: 


Modified: 
clang/lib/Frontend/CompilerInvocation.cpp
llvm/include/llvm/Passes/StandardInstrumentations.h

Removed: 




diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index b71addd84bfd9..7727d70adfb1e 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -438,7 +438,7 @@ static T extractMaskValue(T KeyPath) {
 }(EXTRACTOR(KEYPATH)); 
\
   }
 
-static const StringRef GetInputKindName(InputKind IK);
+static StringRef GetInputKindName(InputKind IK);
 
 static bool FixupInvocation(CompilerInvocation &Invocation,
 DiagnosticsEngine &Diags, const ArgList &Args,
@@ -3291,7 +3291,7 @@ static bool IsInputCompatibleWithStandard(InputKind IK,
 }
 
 /// Get language name for given input kind.
-static const StringRef GetInputKindName(InputKind IK) {
+static StringRef GetInputKindName(InputKind IK) {
   switch (IK.getLanguage()) {
   case Language::C:
 return "C";

diff  --git a/llvm/include/llvm/Passes/StandardInstrumentations.h 
b/llvm/include/llvm/Passes/StandardInstrumentations.h
index 6cab4ce7d138b..9eb754a4d8245 100644
--- a/llvm/include/llvm/Passes/StandardInstrumentations.h
+++ b/llvm/include/llvm/Passes/StandardInstrumentations.h
@@ -432,7 +432,7 @@ class DCData {
   }
 
   // Return the label of the basic block reached on a transition on \p S.
-  const StringRef getSuccessorLabel(StringRef S) const {
+  StringRef getSuccessorLabel(StringRef S) const {
 assert(Successors.count(S) == 1 && "Expected to find successor.");
 return Successors.find(S)->getValue();
   }



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

[PATCH] D115604: [Support] Expand `` as the base directory in response files.

2021-12-28 Thread Jack Andersen via Phabricator via cfe-commits

jackoalan updated this revision to Diff 396410.
jackoalan added a comment.

Rebase, use the slightly more intuitive `` token to expand base paths.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115604/new/

https://reviews.llvm.org/D115604

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UsersManual.rst
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -1038,25 +1038,34 @@
   llvm::SmallVector Argv;
 
   TempDir TestDir("unittest", /*Unique*/ true);
+  TempDir TestSubDir(TestDir.path("subdir"), /*Unique*/ false);
 
-  llvm::SmallString<128> TestCfg;
-  llvm::sys::path::append(TestCfg, TestDir.path(), "foo");
-
+  llvm::SmallString<128> TestCfg = TestDir.path("foo");
   TempFile ConfigFile(TestCfg, "",
   "# Comment\n"
   "-option_1\n"
+  "-option_2=/dir1\n"
   "@subconfig\n"
-  "-option_3=abcd\n"
-  "-option_4=\\\n"
+  "-option_7=abcd\n"
+  "-option_8=\\\n"
   "cdef\n");
 
-  llvm::SmallString<128> TestCfg2;
-  llvm::sys::path::append(TestCfg2, TestDir.path(), "subconfig");
+  llvm::SmallString<128> TestCfg2 = TestDir.path("subconfig");
   TempFile ConfigFile2(TestCfg2, "",
-   "-option_2\n"
+   "-option_3\n"
+   "-option_4=/dir2\n"
+   "@subdir/subfoo\n"
"\n"
"   # comment\n");
 
+  llvm::SmallString<128> TestCfg3 = TestSubDir.path("subfoo");
+  TempFile ConfigFile3(TestCfg3, "",
+   "-option_5=/dir3\n"
+   "@/subfoo2\n");
+
+  llvm::SmallString<128> TestCfg4 = TestSubDir.path("subfoo2");
+  TempFile ConfigFile4(TestCfg4, "", "-option_6=qwerty\n");
+
   // Make sure the current directory is not the directory where config files
   // resides. In this case the code that expands response files will not find
   // 'subconfig' unless it resolves nested inclusions relative to the including
@@ -1071,11 +1080,18 @@
   bool Result = llvm::cl::readConfigFile(ConfigFile.path(), Saver, Argv);
 
   EXPECT_TRUE(Result);
-  EXPECT_EQ(Argv.size(), 4U);
+  EXPECT_EQ(Argv.size(), 8U);
   EXPECT_STREQ(Argv[0], "-option_1");
-  EXPECT_STREQ(Argv[1], "-option_2");
-  EXPECT_STREQ(Argv[2], "-option_3=abcd");
-  EXPECT_STREQ(Argv[3], "-option_4=cdef");
+  EXPECT_STREQ(Argv[1],
+   ("-option_2=" + TestDir.path() + "/dir1").str().c_str());
+  EXPECT_STREQ(Argv[2], "-option_3");
+  EXPECT_STREQ(Argv[3],
+   ("-option_4=" + TestDir.path() + "/dir2").str().c_str());
+  EXPECT_STREQ(Argv[4],
+   ("-option_5=" + TestSubDir.path() + "/dir3").str().c_str());
+  EXPECT_STREQ(Argv[5], "-option_6=qwerty");
+  EXPECT_STREQ(Argv[6], "-option_7=abcd");
+  EXPECT_STREQ(Argv[7], "-option_8=cdef");
 }
 
 TEST(CommandLineTest, PositionalEatArgsError) {
Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -1078,11 +1078,44 @@
   return (S.size() >= 3 && S[0] == '\xef' && S[1] == '\xbb' && S[2] == '\xbf');
 }
 
+// Substitute token with the file's base path.
+static void ExpandBasePaths(StringRef BasePath, StringRef Token,
+StringSaver &Saver, const char *&Arg) {
+  assert(sys::path::is_absolute(BasePath));
+  const StringRef ArgString(Arg);
+
+  SmallString<128> ResponseFile;
+  StringRef::size_type StartPos = 0;
+  for (StringRef::size_type TokenPos = ArgString.find(Token);
+   TokenPos != StringRef::npos;
+   TokenPos = ArgString.find(Token, StartPos)) {
+// Token may appear more than once per arg (e.g. comma-separated linker
+// args). Support by using path-append on any subsequent appearances.
+const StringRef LHS = ArgString.substr(StartPos, TokenPos - StartPos);
+if (ResponseFile.empty())
+  ResponseFile = LHS;
+else
+  llvm::sys::path::append(ResponseFile, LHS);
+ResponseFile.append(BasePath);
+StartPos = TokenPos + Token.size();
+  }
+
+  if (!ResponseFile.empty()) {
+// Path-append the remaining arg substring if at least one token appeared.
+const StringRef Remaining = ArgString.substr(StartPos);
+if (!Remaining.empty())
+  llvm::sys::path::append(ResponseFile, Remaining);
+Arg = Saver.save(ResponseFile.str()).data();
+  }
+}
+
 // FName must be an absolute path.
-static llvm::Error ExpandResponseFile(
-StringRef FName, StringSaver &Saver, TokenizerCallback Tokenizer,
-SmallVectorImpl &NewArgv, bool Mar

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2021-12-28 Thread Marek Kurdej via Phabricator via cfe-commits

curdeius added a comment.

Thanks for addressing the comments quickly. I'll have one more thorough look in 
the coming days.




Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.cpp:55
+// For `auto` language version, be conservative and assume we are < C++17
+KeepTemplateTemplateKW = (Style.Standard == FormatStyle::LS_Auto) ||
+ (Style.Standard < FormatStyle::LS_Cpp17);

avogelsgesang wrote:
> curdeius wrote:
> > Isn't it a better name?
> This flag is actually about the usage of the `class` keyword instead of the 
> `typename` keyword for template-template arguments.
> `true` means: "Keep using the `class` instead of the `typename` keyword for 
> template-template arguments."
> 
> I think the name `KeepTemplateTypenameKW` is wrong. "[...]TypenameKW = true" 
> would mean "use `typename` instead of `class`" to me, and that's exactly the 
> opposite way around.
> 
> As such, I think `KeepTemplateTemplateKW` is in fact the better name. If we 
> want to make it even more explicit, we could also use 
> `KeepTemplateTemplateClassKW`. What do you think?
I did understand it correctly that it is about class keyword in template 
template parameters, but my brain somehow melted down the road:). You can keep 
the name as is.



Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.cpp:65
+  }
+
+  for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) {

avogelsgesang wrote:
> curdeius wrote:
> > Could we check `KeepTemplateTypenameKW` and early return here?
> > The optimizer might do some job, but I doubt it will get rid of the 
> > unnecessary finding of template keyword etc.
> > We could maybe avoid this variable altogether and return inside the switch, 
> > no?
> I think you misunderstood the semantics of `KeepTemplateTypenameKW`? Or did I 
> misunderstand your comment?
> 
> For both `KeepTemplateTemplateKW = true` and `KeepTemplateTemplateKW = 
> false`, the loop below still reformats `class` to `typename`.
> 
> E.g., for the input
> ```
> template class X>
> ```
> and `KeepTemplateTemplateKW = true`, we produce
> ```
> template class X>
> ```
> For the same input with `KeepTemplateTemplateKW = false`, we produce
> ```
> template typename X>
> ```
As above, forget my previous comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116290/new/

https://reviews.llvm.org/D116290

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

[PATCH] D116283: [clang-format] Add an option to add a space between operator overloading and opening parentheses

2021-12-28 Thread Marek Kurdej via Phabricator via cfe-commits

curdeius added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:3755
 
+  * ``bool AfterOperatorOverloading`` If ``true``, put a space between 
operator overloading and opening parentheses.
+

I'm not fond of the current name, exactly the "Overloading" part (but have no 
better suggestion right now). It seems a bit misleading to me.

Maybe AfterOverloadedOperator(Name)??? At least it would match the token kind 
name.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116283/new/

https://reviews.llvm.org/D116283

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

[clang] b5d3bbc - [CMake] Remove unneeded CLANG_DEFAULT_PIE_ON_LINUX canonicalization after D115751

2021-12-28 Thread Fangrui Song via cfe-commits


Author: Fangrui Song
Date: 2021-12-28T10:49:52-08:00
New Revision: b5d3bbcc9433193351a22b738c7ff4b007cb1e68

URL: 
https://github.com/llvm/llvm-project/commit/b5d3bbcc9433193351a22b738c7ff4b007cb1e68
DIFF: 
https://github.com/llvm/llvm-project/commit/b5d3bbcc9433193351a22b738c7ff4b007cb1e68.diff

LOG: [CMake] Remove unneeded CLANG_DEFAULT_PIE_ON_LINUX canonicalization after 
D115751

Added: 


Modified: 
clang/CMakeLists.txt

Removed: 




diff  --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index 00243d8b13b9..69d639fcec1b 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -228,9 +228,6 @@ set(CLANG_SPAWN_CC1 OFF CACHE BOOL
 "Whether clang should use a new process for the CC1 invocation")
 
 option(CLANG_DEFAULT_PIE_ON_LINUX "Default to -fPIE and -pie on Linux" OFF)
-if(CLANG_DEFAULT_PIE_ON_LINUX)
-  set(CLANG_DEFAULT_PIE_ON_LINUX 1)
-endif()
 
 # TODO: verify the values against LangStandards.def?
 set(CLANG_DEFAULT_STD_C "" CACHE STRING



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

[PATCH] D113372: [Driver] Add CLANG_DEFAULT_PIE_ON_LINUX to emulate GCC --enable-default-pie

2021-12-28 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay marked an inline comment as done.
MaskRay added inline comments.



Comment at: clang/CMakeLists.txt:232
+if(CLANG_DEFAULT_PIE_ON_LINUX)
+  set(CLANG_DEFAULT_PIE_ON_LINUX 1)
+endif()

arichardson wrote:
> Can these 3 lines be removed after D115751?
Thx. Removed in b5d3bbcc9433193351a22b738c7ff4b007cb1e68


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113372/new/

https://reviews.llvm.org/D113372

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

[PATCH] D116283: [clang-format] Add an option to add a space between operator overloading and opening parentheses

2021-12-28 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:3755
 
+  * ``bool AfterOperatorOverloading`` If ``true``, put a space between 
operator overloading and opening parentheses.
+

curdeius wrote:
> I'm not fond of the current name, exactly the "Overloading" part (but have no 
> better suggestion right now). It seems a bit misleading to me.
> 
> Maybe AfterOverloadedOperator(Name)??? At least it would match the token kind 
> name.
+1 here,  AfterOperator?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116283/new/

https://reviews.llvm.org/D116283

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

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2021-12-28 Thread Arthur O'Dwyer via Phabricator via cfe-commits

Quuxplusone added inline comments.



Comment at: clang/unittests/Format/TemplateArgumentKeywordFixerTest.cpp:119
+   Style);
+}
+

My personal recommended style is that the programmer uses `template` 
consistently and therefore anytime you see the (more arcane) `typename` keyword 
it's probably necessary for (more arcane) reasons. You cover some arcane uses 
of `typename` above, but I think this one is conspicuously missing:
https://godbolt.org/z/fa9TM4nrr
```
template
void f();
```
You can handle this by looking for `::` after the identifier, and if you find 
it, then `typename` should be kept as-is. In fact, if you find //any// token 
other than `,` `=` `>` after the identifier, something arcane might be 
happening and it'd be best to keep `typename` as-is.

Vice versa, `template` can't be rewritten as `template`; 
again, I think the right approach is to leave the human's choice alone if you 
see anything other than `,` `=` `>` after the identifier.

In general, the problems of "Can this `typename` be rewritten as `class`" and 
vice versa are probably undecidable, so don't get too carried away with trying 
to be perfect as long as you hit all the most common cases anyone can think of. 
https://quuxplusone.github.io/blog/2019/12/27/template-typename-fun/


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116290/new/

https://reviews.llvm.org/D116290

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

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2021-12-28 Thread Arthur O'Dwyer via Phabricator via cfe-commits

Quuxplusone added inline comments.



Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.cpp:55
+// For `auto` language version, be conservative and assume we are < C++17
+KeepTemplateTemplateKW = (Style.Standard == FormatStyle::LS_Auto) ||
+ (Style.Standard < FormatStyle::LS_Cpp17);

curdeius wrote:
> avogelsgesang wrote:
> > curdeius wrote:
> > > Isn't it a better name?
> > This flag is actually about the usage of the `class` keyword instead of the 
> > `typename` keyword for template-template arguments.
> > `true` means: "Keep using the `class` instead of the `typename` keyword for 
> > template-template arguments."
> > 
> > I think the name `KeepTemplateTypenameKW` is wrong. "[...]TypenameKW = 
> > true" would mean "use `typename` instead of `class`" to me, and that's 
> > exactly the opposite way around.
> > 
> > As such, I think `KeepTemplateTemplateKW` is in fact the better name. If we 
> > want to make it even more explicit, we could also use 
> > `KeepTemplateTemplateClassKW`. What do you think?
> I did understand it correctly that it is about class keyword in template 
> template parameters, but my brain somehow melted down the road:). You can 
> keep the name as is.
IIUC, it feels like the boolean should be named 
`UseClassKWInTemplateTemplates`, and it shouldn't have anything to do with 
`Keep`ing the human's choice. If the human feeds you some C++17 code using 
`template typename T>` and asks you to format it as C++14 with 
`TAS_Typename`, I think it would be quite appropriate to output 
`template class T>`. (Change `class` to `typename` because 
the human asked for `TAS_Typename`; change `typename` to `class` because C++14 
implies `UseClassKWInTemplateTemplates`.)
Anyway, this should have a unit test too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116290/new/

https://reviews.llvm.org/D116290

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

[clang] 89aa87c - [clang] Fix AttrDocs.td formatting.

2021-12-28 Thread Michael Benfield via cfe-commits


Author: Michael Benfield
Date: 2021-12-28T19:13:03Z
New Revision: 89aa87c4e601985dae4b41206f0c5594e8742c78

URL: 
https://github.com/llvm/llvm-project/commit/89aa87c4e601985dae4b41206f0c5594e8742c78
DIFF: 
https://github.com/llvm/llvm-project/commit/89aa87c4e601985dae4b41206f0c5594e8742c78.diff

LOG: [clang] Fix AttrDocs.td formatting.

This should fix the builder clang-sphinx-docs.

Added: 


Modified: 
clang/include/clang/Basic/AttrDocs.td

Removed: 




diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index 8a7424a88c9f8..a24218a9c82b6 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -5987,7 +5987,7 @@ attribute requires a string literal argument to identify 
the handle being releas
 def DiagnoseAsBuiltinDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
-The ``diagnose_as_builtin` attribute indicates that Fortify diagnostics are to
+The ``diagnose_as_builtin`` attribute indicates that Fortify diagnostics are to
 be applied to the declared function as if it were the function specified by the
 attribute. The builtin function whose diagnostics are to be mimicked should be
 given. In addition, the order in which arguments should be applied must also
@@ -5995,12 +5995,12 @@ be given.
 
 For example, the attribute can be used as follows.
 
-  .. code-block:: c
+.. code-block:: c
 
-__attribute__((diagnose_as_builtin(__builtin_memset, 3, 2, 1)))
-void *mymemset(int n, int c, void *s) {
-  // ...
-}
+  __attribute__((diagnose_as_builtin(__builtin_memset, 3, 2, 1)))
+  void *mymemset(int n, int c, void *s) {
+// ...
+  }
 
 This indicates that calls to ``mymemset`` should be diagnosed as if they were
 calls to ``__builtin_memset``. The arguments ``3, 2, 1`` indicate by index the
@@ -6015,7 +6015,8 @@ they would to the builtin function, after all normal 
arguments. For instance,
 to diagnose a new function as if it were `sscanf`, we can use the attribute as
 follows.
 
-  .. code-block:: c
+.. code-block:: c
+
   __attribute__((diagnose_as_builtin(sscanf, 1, 2)))
   int mysscanf(const char *str, const char *format, ...)  {
 // ...



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

[PATCH] D116337: [clang] set __NO_MATH_ERRNO__ if -fno-math-errno

2021-12-28 Thread Alex Xu (Hello71) via Phabricator via cfe-commits

alxu created this revision.
alxu added a project: clang.
alxu requested review of this revision.
Herald added a subscriber: cfe-commits.

This causes modern glibc to unset math_errhandling MATH_ERRNO. gcc 12 also sets 
some other macros, but most of them are associated with flags ignored by clang, 
so without library examples, it is difficult to determine whether they should 
be set. I think setting this one macro is OK for now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116337

Files:
  clang/lib/Frontend/InitPreprocessor.cpp


Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -1019,6 +1019,9 @@

   Builder.defineMacro("__USER_LABEL_PREFIX__", TI.getUserLabelPrefix());

+  if (LangOpts.FastMath || !LangOpts.MathErrno)
+Builder.defineMacro("__NO_MATH_ERRNO__");
+
   if (LangOpts.FastMath || LangOpts.FiniteMathOnly)
 Builder.defineMacro("__FINITE_MATH_ONLY__", "1");
   else


Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -1019,6 +1019,9 @@

   Builder.defineMacro("__USER_LABEL_PREFIX__", TI.getUserLabelPrefix());

+  if (LangOpts.FastMath || !LangOpts.MathErrno)
+Builder.defineMacro("__NO_MATH_ERRNO__");
+
   if (LangOpts.FastMath || LangOpts.FiniteMathOnly)
 Builder.defineMacro("__FINITE_MATH_ONLY__", "1");
   else
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116159: [ARM][AArch64] clang support for Armv9.3-A

2021-12-28 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added inline comments.



Comment at: clang/test/Driver/arm-cortex-cpus.c:445
+// RUN: %clang -target armv9.3a -mlittle-endian -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-V93A %s
+// RUN: %clang -target arm -march=armv9.3a -mlittle-endian -### -c %s 2>&1 | 
FileCheck -check-prefix=CHECK-V93A %s
+// RUN: %clang -target arm -mlittle-endian -march=armv9.3-a -mlittle-endian 
-### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V93A %s

Is this `RUN` line a duplicate of L443? Maybe you meant to test a hyphen in the 
`march`?



Comment at: clang/test/Driver/arm-cortex-cpus.c:446
+// RUN: %clang -target arm -march=armv9.3a -mlittle-endian -### -c %s 2>&1 | 
FileCheck -check-prefix=CHECK-V93A %s
+// RUN: %clang -target arm -mlittle-endian -march=armv9.3-a -mlittle-endian 
-### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V93A %s
+// CHECK-V93A: "-cc1"{{.*}} "-triple" "armv9.3{{.*}}" "-target-cpu" "generic"

how about thumb targets? I recognize this test doesn't already do so for older 
extensions.



Comment at: clang/test/Driver/arm-cortex-cpus.c:446
+// RUN: %clang -target arm -march=armv9.3a -mlittle-endian -### -c %s 2>&1 | 
FileCheck -check-prefix=CHECK-V93A %s
+// RUN: %clang -target arm -mlittle-endian -march=armv9.3-a -mlittle-endian 
-### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V93A %s
+// CHECK-V93A: "-cc1"{{.*}} "-triple" "armv9.3{{.*}}" "-target-cpu" "generic"

nickdesaulniers wrote:
> how about thumb targets? I recognize this test doesn't already do so for 
> older extensions.
This test sets `-mlittle-endian` twice. Is that intentional? Should the aarch64 
test above `clang/test/Driver/aarch64-cpus.c` also have such a test, or should 
this `RUN` line be removed? Or was something else meant to be tested?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116159/new/

https://reviews.llvm.org/D116159

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

[PATCH] D93844: [clang-format] Add possibility to be based on parent directory

2021-12-28 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks marked 2 inline comments as done.
HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/Format.cpp:3067
+  if (!ChildFormatTextToApply.empty()) {
+assert(ChildFormatTextToApply.size() == 1);
+

zwliew wrote:
> HazardyKnusperkeks wrote:
> > zwliew wrote:
> > > Is there a reason behind limiting the number of children configs to 1 in 
> > > this case? When the fallback is not used, more than 1 children configs 
> > > are allowed (line 3036).
> > > 
> > > Sorry for digging this up, I came across this seemingly arbitrary 
> > > decision when working on some changes to https://reviews.llvm.org/D72326
> > Yeah but it doesn't happen, there is at most only one `.clang-format` in 
> > the parent directory path which is found. So we assert on that and then 
> > don't need to loop over what is exactly one element big.
> Thanks for the reply. However, I don't think that's true. Yes, it's only 
> possible to find one `.clang-format` file in the first parent directory. But 
> if it has `BasedOnStyle: InheritParentConfig` set, then we could find another 
> `.clang-format` file in the "grandparent" directory. In this case, we'll have 
> 2 elements in `ChildFormatTextToApply`, but only the very first one will 
> actually be applied.
> 
> To illustrate, suppose we have the following file structure:
> ```
> - .clang-format
> - foo
>   |- .clang-format
>   |- input.cpp
> ```
> 
> Both `.clang-format` files have `BasedOnStyle: InheritParentConfig` set. When 
> running `clang-format --style=file foo/input.cpp`, only the inner config is 
> applied on the fallback style, while the outer config is ignored. When 
> testing on a debug build, I encountered a crash due to the failed assert. 
> When removing the assert, and using a loop to apply the configs, both the 
> inner and outer configs are applied, which I believe is the expected 
> behaviour.
> 
I can not explain it to you anymore, I would have to dig into it again. But if 
my tests are correct everything works. You can see that for `Test 9.4` I create 
a `.clang-format` in `/e/sub/sub/` which is based on `/e/sub/.clang-format` 
which again is based on `/e/.clang-format`.

The tests do not fail, the parsed style is as the three files suggest, and the 
assert holds. I'm pretty sure I have thought about that case, it happens in 
some kind of recursion.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93844/new/

https://reviews.llvm.org/D93844

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

[PATCH] D116283: [clang-format] Add an option to add a space between operator overloading and opening parentheses

2021-12-28 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added a comment.

Does it affect calling code? `a.operator++(5);` Should it? But please add tests 
for that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116283/new/

https://reviews.llvm.org/D116283

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

[PATCH] D116170: [clang-format] Add penalty for breaking after '('

2021-12-28 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added inline comments.



Comment at: clang/include/clang/Format/Format.h:2900
+  /// \version 14
+  unsigned PenaltyBreakOpenParenthesis;
+

Please resort.



Comment at: clang/lib/Format/Format.cpp:1238
   LLVMStyle.PenaltyBreakTemplateDeclaration = prec::Relational;
+  LLVMStyle.PenaltyBreakOpenParenthesis = 0;
   LLVMStyle.PenaltyIndentedWhitespace = 0;

O before T, please resort.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116170/new/

https://reviews.llvm.org/D116170

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

[PATCH] D116314: [clang-format] Add style to separate definition blocks

2021-12-28 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/Format.cpp:2027
+// namespaces by inserting a line break between them.
+class DefinitionBlockSeparator : public TokenAnalyzer {
+public:

curdeius wrote:
> Please consider moving it to a different file.
Yes please.



Comment at: clang/lib/Format/Format.cpp:2114
+  if (Result.add(R))
+return;
+  }

Why only add the first replacement?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116314/new/

https://reviews.llvm.org/D116314

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

[PATCH] D116314: [clang-format] Add style to separate definition blocks

2021-12-28 Thread ksyx via Phabricator via cfe-commits

ksyx updated this revision to Diff 396421.
ksyx marked 4 inline comments as done.
ksyx added a comment.
Herald added a subscriber: mgorny.

- Change version from 15 to 14
- Add option `SDS_Never`, rename `SDS_Between` => `SDS_Always`
- Add unit tests
- Separate class into new file


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116314/new/

https://reviews.llvm.org/D116314

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/CMakeLists.txt
  clang/lib/Format/DefinitionBlockSeparator.cpp
  clang/lib/Format/DefinitionBlockSeparator.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/DefinitionBlockSeparatorTest.cpp

Index: clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
===
--- /dev/null
+++ clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
@@ -0,0 +1,273 @@
+//===- DefinitionBlockSeparatorTest.cpp - Formatting unit tests ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Format/Format.h"
+
+#include "llvm/Support/Debug.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "definition-block-separator-test"
+
+namespace clang {
+namespace format {
+namespace {
+
+class DefinitionBlockSeparatorTest : public ::testing::Test {
+protected:
+  std::string
+  separateDefinitionBlocks(llvm::StringRef Code,
+   const std::vector &Ranges,
+   const FormatStyle &Style = getLLVMStyle()) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+tooling::Replacements Replaces =
+clang::format::separateDefinitionBlocks(Style, Code, Ranges, "");
+auto Result = applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  std::string
+  separateDefinitionBlocks(llvm::StringRef Code,
+   const FormatStyle &Style = getLLVMStyle()) {
+return separateDefinitionBlocks(
+Code,
+/*Ranges=*/{1, tooling::Range(0, Code.size())}, Style);
+  }
+};
+
+TEST_F(DefinitionBlockSeparatorTest, Basic) {
+  FormatStyle Style = getLLVMStyle();
+  Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+  EXPECT_EQ("int foo() {\n"
+"  int i, j;\n"
+"}\n"
+"\n"
+"int bar() {\n"
+"  int j, k;\n"
+"}",
+separateDefinitionBlocks("int foo() {\n"
+ "  int i, j;\n"
+ "}\n"
+ "int bar() {\n"
+ "  int j, k;\n"
+ "}", Style));
+
+  EXPECT_EQ("struct foo {\n"
+"  int i, j;\n"
+"};\n"
+"\n"
+"struct bar {\n"
+"  int j, k;\n"
+"};",
+separateDefinitionBlocks("struct foo {\n"
+ "  int i, j;\n"
+ "};\n"
+ "struct bar {\n"
+ "  int j, k;\n"
+ "};", Style));
+
+  EXPECT_EQ("class foo {\n"
+"  int i, j;\n"
+"};\n"
+"\n"
+"class bar {\n"
+"  int j, k;\n"
+"};",
+separateDefinitionBlocks("class foo {\n"
+ "  int i, j;\n"
+ "};\n"
+ "class bar {\n"
+ "  int j, k;\n"
+ "};", Style));
+
+  EXPECT_EQ("namespace foo {\n"
+"  int i, j;\n"
+"}\n"
+"\n"
+"namespace bar {\n"
+"  int j, k;\n"
+"}",
+separateDefinitionBlocks("namespace foo {\n"
+ "  int i, j;\n"
+ "}\n"
+ "namespace bar {\n"
+ "  int j, k;\n"
+ "}", Style));
+
+  EXPECT_EQ("enum Foo {\n"
+"  FOO, BAR\n"
+"};\n"
+"\n"
+"enum Bar {\n"
+"  FOOBAR, BARFOO\n"
+"};",
+separateDefinitionBlocks("enum Foo {\n"
+ "  FOO, BAR\n"
+ "};\n"
+

[PATCH] D116314: [clang-format] Add style to separate definition blocks

2021-12-28 Thread ksyx via Phabricator via cfe-commits

ksyx marked an inline comment as done.
ksyx added inline comments.



Comment at: clang/lib/Format/Format.cpp:2114
+  if (Result.add(R))
+return;
+  }

HazardyKnusperkeks wrote:
> Why only add the first replacement?
This worked well as the `Error` class `add` method returned has overloaded 
boolean operator, returning true (nonzero) when failing, which simulates 
program exit code scheme? (ref: [[ 
https://llvm.org/doxygen/classllvm_1_1Error.html#a981b4992a3b7cce718c7995a7d6193a0
 | Doxygen/Error/operator bool ]])


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116314/new/

https://reviews.llvm.org/D116314

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

[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

2021-12-28 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added a comment.

In D116316#3211269 , @curdeius wrote:

> Could you have a look at preceding reviews and see if there wasn't a similar 
> patch before?
> I think that this option is a bit too limited.
> Only removing braces doesn't seem enough.
> Also, one should probably be able to decide when to add/remove them by e.g. 
> setting the number of lines in what's considered short blocks.

There is D95168 .




Comment at: clang/lib/Format/UnwrappedLineParser.cpp:478
+  if (HasOpeningBrace) {
+return StatementCount == 1 && !PrecededByCommentOrPPDirective &&
+   /* !StartsWithBrace && */ !precededByCommentOrPPDirective();

This is very confusing a variable with the same name as the function.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2173
+void UnwrappedLineParser::KeepAncestorBraces() {
+  const int MaxNestingLevels = 2;
+  const int Size = NestedTooDeep.size();

constexpr or configurable?



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2338
+  KeepAncestorBraces();
+  NestedTooDeep.push_back(false);
   if (FormatTok->is(tok::l_brace)) {

I think it is worth to create a RAII type for that.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2937
   // Parse the class body after the enum's ";" if any.
-  parseLevel(/*HasOpeningBrace=*/true);
+  parseLevel();
   nextToken();

I would like it better if you insert both arguments.
I don't really like default arguments.



Comment at: clang/lib/Format/UnwrappedLineParser.h:196
   bool isOnNewLine(const FormatToken &FormatTok);
+  bool precededByCommentOrPPDirective();
+  void KeepAncestorBraces();

I find it nicely if the order of functions in the cpp is the same as in the hpp.



Comment at: clang/lib/Format/UnwrappedLineParser.h:197
+  bool precededByCommentOrPPDirective();
+  void KeepAncestorBraces();
 




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116316/new/

https://reviews.llvm.org/D116316

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

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2021-12-28 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added inline comments.



Comment at: clang/include/clang/Format/Format.h:1903
   ///  lead to incorrect code formatting due to incorrect decisions made due to
-  ///  clang-formats lack of complete semantic information.
+  ///  clang-format's lack of complete semantic information.
   ///  As such extra care should be taken to review code changes made by the 
use

Please don't change anything unrelated. You are very welcome to change this in 
a different patch.



Comment at: clang/include/clang/Format/Format.h:3698
+
+  /// \brief Use ``\r\n`` instead of ``\n`` for line breaks.
+  /// Also used as fallback if ``DeriveLineEnding`` is true.

Also unrelated.



Comment at: clang/lib/Format/Format.cpp:144-145
+IO.enumCase(Value, "Leave", FormatStyle::TAS_Leave);
+IO.enumCase(Value, "typename", FormatStyle::TAS_Typename);
+IO.enumCase(Value, "class", FormatStyle::TAS_Class);
+  }

avogelsgesang wrote:
> HazardyKnusperkeks wrote:
> > This is what is printed in the documentation, if generated automatically.
> The documentation is currently automatically generated. I used the additional 
> comments in `Format.h` to overwrite the "in configuration" values:
> 
> ```
> /// ...
> TAS_Leave,
> /// ...
> TAS_Typename, // typename
> /// ...
> TAS_Class // class
> ```
> 
> Note the additional comments after `TAS_Typename` and `TAS_Class`. They 
> overwrite the "in configuration" names used by dump_format_style.py. The same 
> trick is used, e.g., for the `LanguageStandard` enum.
> 
> Given that using lowercase `typename` and `class` still allow for 
> auto-generated documentation, do yous still want me to switch to uppercase 
> `Typename`/`Class`?
> 
> Personally, I do prefer lowercase here because the language keywords are also 
> lowercase. Are there any other reasons except for auto-generated 
> documentation why you would prefer uppercase config values?
Okay, didn't know that.

Than I just have my personal opinion that I would use upper case, but you can 
choose what ever you want. :)



Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.cpp:80
+}
+FormatToken *EndTok = Tok->MatchingParen;
+

avogelsgesang wrote:
> HazardyKnusperkeks wrote:
> > assert on non nullness
> nullness was checked already in line 75:
> 
> ```
> if (Tok->isNot(TT_TemplateOpener) || !Tok->MatchingParen) {
> ```
> 
> Do you still want me to add this `assert`? Should I maybe restructure the 
> code somehow to make it easier to understand? Or are you fine with leaving it 
> as is?
I meant assert on `EndTok`.



Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.cpp:93
+  bool StartedDefaultArg = Tok->is(tok::equal);
+  auto advanceTok = [&]() {
+Tok = Tok->Next;

avogelsgesang wrote:
> HazardyKnusperkeks wrote:
> > Could be located outside of the loop.
> I agree it could be moved outside the loop. Should it, though?
> 
> Having it inside the loop makes the code slightly easier to read by keeping 
> the definition closer to its usage. And performancewise, I do trust the 
> compiler to do a decent job here.
> 
> Or am I somehow misguided in my judgement here?
Most likely you are right. That's why I said could not should.



Comment at: clang/unittests/Format/TemplateArgumentKeywordFixerTest.cpp:181
+  // `typename`.
+  verifyFormat("template", "template", Style);
+  verifyFormat("template <", "template<", Style);

Can drop the second argument.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116290/new/

https://reviews.llvm.org/D116290

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

[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

2021-12-28 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added a comment.

I'm against that patch.

> Don't pay for what you don't use.

We should not put the size check into every call.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116318/new/

https://reviews.llvm.org/D116318

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

[PATCH] D116314: [clang-format] Add style to separate definition blocks

2021-12-28 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added inline comments.



Comment at: clang/include/clang/Format/Format.h:3054
+  enum SeparateDefinitionStyle {
+/// Leave definition blocks separated as they are without changing anything
+SDS_Leave,

Add full stop at the end of sentences.



Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:55
+  auto insertReplacement = [&]() {
+// Do not handle EOF newlines
+assert(TargetToken);

Here also full stop, please. And following comments.



Comment at: clang/lib/Format/DefinitionBlockSeparator.h:38-39
+};
+} // end namespace format
+} // end namespace clang
+

I know you copied it. It is wrong where you copied it from. :)



Comment at: clang/lib/Format/Format.cpp:2114
+  if (Result.add(R))
+return;
+  }

ksyx wrote:
> HazardyKnusperkeks wrote:
> > Why only add the first replacement?
> This worked well as the `Error` class `add` method returned has overloaded 
> boolean operator, returning true (nonzero) when failing, which simulates 
> program exit code scheme? (ref: [[ 
> https://llvm.org/doxygen/classllvm_1_1Error.html#a981b4992a3b7cce718c7995a7d6193a0
>  | Doxygen/Error/operator bool ]])
Okay, but maybe safe the result in `Error` to make it clearer.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116314/new/

https://reviews.llvm.org/D116314

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

[PATCH] D72326: [clang-format] Rebased on master: Add option to specify explicit config file

2021-12-28 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added a comment.

Do you plan to refactor something for this review, or do you think you are 
done? Then I will look at it again as a whole.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72326/new/

https://reviews.llvm.org/D72326

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

[clang] cd284b7 - [clang][ARM] re-use arm::isHardTPSupported for hardware TLS check

2021-12-28 Thread Nick Desaulniers via cfe-commits


Author: Nick Desaulniers
Date: 2021-12-28T13:28:34-08:00
New Revision: cd284b7ac0615afc6e0f1a30da2777e361de27a3

URL: 
https://github.com/llvm/llvm-project/commit/cd284b7ac0615afc6e0f1a30da2777e361de27a3
DIFF: 
https://github.com/llvm/llvm-project/commit/cd284b7ac0615afc6e0f1a30da2777e361de27a3.diff

LOG: [clang][ARM] re-use arm::isHardTPSupported for hardware TLS check

This conditional check for -mstack-protector-guard=tls got out of sync
with the conditional check for -mtp=cp15 by me in D114116, because I
forgot about the similar check added in D113026.

Re-use the code in arm::isHardTPSupported so that these aren't out of
sync.

Interestingly, our CI reported this when testing
-mstack-protector-guard=tls; it was only reproducible with Debian's LLVM
and not upstream LLVM due to this out of tree patch:
https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/blob/snapshot/debian/patches/930008-arm.diff

Fixes: https://github.com/ClangBuiltLinux/linux/issues/1502

Reviewed By: ardb

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Clang.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 65347a38490ee..2c34392150938 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -3217,9 +3217,7 @@ static void RenderSSPOptions(const Driver &D, const 
ToolChain &TC,
 return;
   }
   // Check whether the target subarch supports the hardware TLS register
-  if (arm::getARMSubArchVersionNumber(EffectiveTriple) < 7 &&
-  llvm::ARM::parseArch(EffectiveTriple.getArchName()) !=
-  llvm::ARM::ArchKind::ARMV6T2) {
+  if (!arm::isHardTPSupported(EffectiveTriple)) {
 D.Diag(diag::err_target_unsupported_tp_hard)
 << EffectiveTriple.getArchName();
 return;



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

[PATCH] D116233: [clang][ARM] re-use arm::isHardTPSupported for hardware TLS check

2021-12-28 Thread Nick Desaulniers via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcd284b7ac061: [clang][ARM] re-use arm::isHardTPSupported for 
hardware TLS check (authored by nickdesaulniers).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116233/new/

https://reviews.llvm.org/D116233

Files:
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3217,9 +3217,7 @@
 return;
   }
   // Check whether the target subarch supports the hardware TLS register
-  if (arm::getARMSubArchVersionNumber(EffectiveTriple) < 7 &&
-  llvm::ARM::parseArch(EffectiveTriple.getArchName()) !=
-  llvm::ARM::ArchKind::ARMV6T2) {
+  if (!arm::isHardTPSupported(EffectiveTriple)) {
 D.Diag(diag::err_target_unsupported_tp_hard)
 << EffectiveTriple.getArchName();
 return;


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3217,9 +3217,7 @@
 return;
   }
   // Check whether the target subarch supports the hardware TLS register
-  if (arm::getARMSubArchVersionNumber(EffectiveTriple) < 7 &&
-  llvm::ARM::parseArch(EffectiveTriple.getArchName()) !=
-  llvm::ARM::ArchKind::ARMV6T2) {
+  if (!arm::isHardTPSupported(EffectiveTriple)) {
 D.Diag(diag::err_target_unsupported_tp_hard)
 << EffectiveTriple.getArchName();
 return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116342: [clang][AST] Fix crash when printing error

2021-12-28 Thread Ellis Hoag via Phabricator via cfe-commits

ellis created this revision.
ellis added a reviewer: dblaikie.
ellis published this revision for review.
ellis added inline comments.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.



Comment at: clang/lib/AST/TypePrinter.cpp:242
 case Type::ObjCObjectPointer:
   CanPrefixQualifiers = T->isObjCIdType() || T->isObjCClassType() ||
 T->isObjCQualifiedIdType() || T->isObjCQualifiedClassType();

Hi @dblaikie, I'm not familiar with this code but I'm wondering if these uses 
of `T` should actually be `UnderlyingType`. Will this cause a crash similar to 
what I found?


Clang will crash if it tries to compile the following code. This commit
fixes it.

  $ cat foo.c
  void foo(_Nullable int *ptr) {
  __auto_type _Nonnull a = ptr;
  };
  $ clang foo.c -c -Wnullable-to-nonnull-conversion


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116342

Files:
  clang/lib/AST/TypePrinter.cpp
  clang/test/Sema/nullability.c


Index: clang/test/Sema/nullability.c
===
--- clang/test/Sema/nullability.c
+++ clang/test/Sema/nullability.c
@@ -125,6 +125,7 @@
   int *a = ptr; // okay
   _Nonnull int *b = ptr; // expected-warning{{implicit conversion from 
nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * 
_Nonnull'}}
   b = ptr; // expected-warning{{implicit conversion from nullable pointer 'int 
* _Nullable' to non-nullable pointer type 'int * _Nonnull'}}
+  __auto_type _Nonnull c = ptr; // expected-warning{{implicit conversion from 
nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * 
_Nullable _Nonnull'}}
 
   accepts_nonnull_1(ptr); // expected-warning{{implicit conversion from 
nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * 
_Nonnull'}}
 }
Index: clang/lib/AST/TypePrinter.cpp
===
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -280,7 +280,7 @@
 case Type::Attributed: {
   // We still want to print the address_space before the type if it is an
   // address_space attribute.
-  const auto *AttrTy = cast(T);
+  const auto *AttrTy = cast(UnderlyingType);
   CanPrefixQualifiers = AttrTy->getAttrKind() == attr::AddressSpace;
 }
   }


Index: clang/test/Sema/nullability.c
===
--- clang/test/Sema/nullability.c
+++ clang/test/Sema/nullability.c
@@ -125,6 +125,7 @@
   int *a = ptr; // okay
   _Nonnull int *b = ptr; // expected-warning{{implicit conversion from nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * _Nonnull'}}
   b = ptr; // expected-warning{{implicit conversion from nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * _Nonnull'}}
+  __auto_type _Nonnull c = ptr; // expected-warning{{implicit conversion from nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * _Nullable _Nonnull'}}
 
   accepts_nonnull_1(ptr); // expected-warning{{implicit conversion from nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * _Nonnull'}}
 }
Index: clang/lib/AST/TypePrinter.cpp
===
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -280,7 +280,7 @@
 case Type::Attributed: {
   // We still want to print the address_space before the type if it is an
   // address_space attribute.
-  const auto *AttrTy = cast(T);
+  const auto *AttrTy = cast(UnderlyingType);
   CanPrefixQualifiers = AttrTy->getAttrKind() == attr::AddressSpace;
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116314: [clang-format] Add style to separate definition blocks

2021-12-28 Thread ksyx via Phabricator via cfe-commits

ksyx updated this revision to Diff 396428.
ksyx marked 5 inline comments as done.
ksyx added a comment.

- Apply suggestions from clangfmt
- Add missing period of comments
- Fix namespace ending comment
- Add comment for notes on return value of WhitespaceManager's method


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116314/new/

https://reviews.llvm.org/D116314

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/CMakeLists.txt
  clang/lib/Format/DefinitionBlockSeparator.cpp
  clang/lib/Format/DefinitionBlockSeparator.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/DefinitionBlockSeparatorTest.cpp

Index: clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
===
--- /dev/null
+++ clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
@@ -0,0 +1,281 @@
+//===- DefinitionBlockSeparatorTest.cpp - Formatting unit tests ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Format/Format.h"
+
+#include "llvm/Support/Debug.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "definition-block-separator-test"
+
+namespace clang {
+namespace format {
+namespace {
+
+class DefinitionBlockSeparatorTest : public ::testing::Test {
+protected:
+  std::string
+  separateDefinitionBlocks(llvm::StringRef Code,
+   const std::vector &Ranges,
+   const FormatStyle &Style = getLLVMStyle()) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+tooling::Replacements Replaces =
+clang::format::separateDefinitionBlocks(Style, Code, Ranges, "");
+auto Result = applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  std::string
+  separateDefinitionBlocks(llvm::StringRef Code,
+   const FormatStyle &Style = getLLVMStyle()) {
+return separateDefinitionBlocks(
+Code,
+/*Ranges=*/{1, tooling::Range(0, Code.size())}, Style);
+  }
+};
+
+TEST_F(DefinitionBlockSeparatorTest, Basic) {
+  FormatStyle Style = getLLVMStyle();
+  Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+  EXPECT_EQ("int foo() {\n"
+"  int i, j;\n"
+"}\n"
+"\n"
+"int bar() {\n"
+"  int j, k;\n"
+"}",
+separateDefinitionBlocks("int foo() {\n"
+ "  int i, j;\n"
+ "}\n"
+ "int bar() {\n"
+ "  int j, k;\n"
+ "}",
+ Style));
+
+  EXPECT_EQ("struct foo {\n"
+"  int i, j;\n"
+"};\n"
+"\n"
+"struct bar {\n"
+"  int j, k;\n"
+"};",
+separateDefinitionBlocks("struct foo {\n"
+ "  int i, j;\n"
+ "};\n"
+ "struct bar {\n"
+ "  int j, k;\n"
+ "};",
+ Style));
+
+  EXPECT_EQ("class foo {\n"
+"  int i, j;\n"
+"};\n"
+"\n"
+"class bar {\n"
+"  int j, k;\n"
+"};",
+separateDefinitionBlocks("class foo {\n"
+ "  int i, j;\n"
+ "};\n"
+ "class bar {\n"
+ "  int j, k;\n"
+ "};",
+ Style));
+
+  EXPECT_EQ("namespace foo {\n"
+"  int i, j;\n"
+"}\n"
+"\n"
+"namespace bar {\n"
+"  int j, k;\n"
+"}",
+separateDefinitionBlocks("namespace foo {\n"
+ "  int i, j;\n"
+ "}\n"
+ "namespace bar {\n"
+ "  int j, k;\n"
+ "}",
+ Style));
+
+  EXPECT_EQ("enum Foo {\n"
+"  FOO, BAR\n"
+"};\n"
+"\n"
+"enum Bar {\n"
+"  FOOBAR, BARFOO\n"
+"};",
+separateDefini

[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

2021-12-28 Thread Owen Pan via Phabricator via cfe-commits

owenpan added a comment.

In D116318#3211689 , 
@HazardyKnusperkeks wrote:

> I'm against that patch.
>
>> Don't pay for what you don't use.
>
> We should not put the size check into every call.

That's what you must do now **without **this patch, which will fix the problem. 
I should be able to just call `getPreviousToken()` without having to first 
check `Position > 0`.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116318/new/

https://reviews.llvm.org/D116318

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

[PATCH] D116314: [clang-format] Add style to separate definition blocks

2021-12-28 Thread ksyx via Phabricator via cfe-commits

ksyx updated this revision to Diff 396430.
ksyx added a comment.

Fix typo.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116314/new/

https://reviews.llvm.org/D116314

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/CMakeLists.txt
  clang/lib/Format/DefinitionBlockSeparator.cpp
  clang/lib/Format/DefinitionBlockSeparator.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/DefinitionBlockSeparatorTest.cpp

Index: clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
===
--- /dev/null
+++ clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
@@ -0,0 +1,281 @@
+//===- DefinitionBlockSeparatorTest.cpp - Formatting unit tests ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Format/Format.h"
+
+#include "llvm/Support/Debug.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "definition-block-separator-test"
+
+namespace clang {
+namespace format {
+namespace {
+
+class DefinitionBlockSeparatorTest : public ::testing::Test {
+protected:
+  std::string
+  separateDefinitionBlocks(llvm::StringRef Code,
+   const std::vector &Ranges,
+   const FormatStyle &Style = getLLVMStyle()) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+tooling::Replacements Replaces =
+clang::format::separateDefinitionBlocks(Style, Code, Ranges, "");
+auto Result = applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  std::string
+  separateDefinitionBlocks(llvm::StringRef Code,
+   const FormatStyle &Style = getLLVMStyle()) {
+return separateDefinitionBlocks(
+Code,
+/*Ranges=*/{1, tooling::Range(0, Code.size())}, Style);
+  }
+};
+
+TEST_F(DefinitionBlockSeparatorTest, Basic) {
+  FormatStyle Style = getLLVMStyle();
+  Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+  EXPECT_EQ("int foo() {\n"
+"  int i, j;\n"
+"}\n"
+"\n"
+"int bar() {\n"
+"  int j, k;\n"
+"}",
+separateDefinitionBlocks("int foo() {\n"
+ "  int i, j;\n"
+ "}\n"
+ "int bar() {\n"
+ "  int j, k;\n"
+ "}",
+ Style));
+
+  EXPECT_EQ("struct foo {\n"
+"  int i, j;\n"
+"};\n"
+"\n"
+"struct bar {\n"
+"  int j, k;\n"
+"};",
+separateDefinitionBlocks("struct foo {\n"
+ "  int i, j;\n"
+ "};\n"
+ "struct bar {\n"
+ "  int j, k;\n"
+ "};",
+ Style));
+
+  EXPECT_EQ("class foo {\n"
+"  int i, j;\n"
+"};\n"
+"\n"
+"class bar {\n"
+"  int j, k;\n"
+"};",
+separateDefinitionBlocks("class foo {\n"
+ "  int i, j;\n"
+ "};\n"
+ "class bar {\n"
+ "  int j, k;\n"
+ "};",
+ Style));
+
+  EXPECT_EQ("namespace foo {\n"
+"  int i, j;\n"
+"}\n"
+"\n"
+"namespace bar {\n"
+"  int j, k;\n"
+"}",
+separateDefinitionBlocks("namespace foo {\n"
+ "  int i, j;\n"
+ "}\n"
+ "namespace bar {\n"
+ "  int j, k;\n"
+ "}",
+ Style));
+
+  EXPECT_EQ("enum Foo {\n"
+"  FOO, BAR\n"
+"};\n"
+"\n"
+"enum Bar {\n"
+"  FOOBAR, BARFOO\n"
+"};",
+separateDefinitionBlocks("enum Foo {\n"
+ "  FOO, BAR\n"
+ "};\n"
+ "enum Bar {\n"
+

[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

2021-12-28 Thread Owen Pan via Phabricator via cfe-commits

owenpan added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:3419
+   if (isa(D)) {  vs. if (isa(D)) {
+ for (auto *A : D.attrs()) { for (auto *A : D.attrs())
+   if (shouldProcessAttr(A)) { if (shouldProcessAttr(A))

curdeius wrote:
> I'm not sure if the braces on the right should be removed in the for loop.
> There should probably be an option to set the  minimum number of 
> lines/statements inside a control statement to control adding/removing braces.
> I'm not sure if the braces on the right should be removed in the for loop.
This //is// the LLVM [[ 
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
 | style ]]. See also line 23086 in FormatTest.cpp below.
> There should probably be an option to set the  minimum number of 
> lines/statements inside a control statement to control adding/removing braces.
If you have more than one statement inside a control statement block, the 
braces must stay.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116316/new/

https://reviews.llvm.org/D116316

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

[PATCH] D93844: [clang-format] Add possibility to be based on parent directory

2021-12-28 Thread Zhao Wei Liew via Phabricator via cfe-commits

zwliew added inline comments.



Comment at: clang/lib/Format/Format.cpp:3067
+  if (!ChildFormatTextToApply.empty()) {
+assert(ChildFormatTextToApply.size() == 1);
+

HazardyKnusperkeks wrote:
> zwliew wrote:
> > HazardyKnusperkeks wrote:
> > > zwliew wrote:
> > > > Is there a reason behind limiting the number of children configs to 1 
> > > > in this case? When the fallback is not used, more than 1 children 
> > > > configs are allowed (line 3036).
> > > > 
> > > > Sorry for digging this up, I came across this seemingly arbitrary 
> > > > decision when working on some changes to https://reviews.llvm.org/D72326
> > > Yeah but it doesn't happen, there is at most only one `.clang-format` in 
> > > the parent directory path which is found. So we assert on that and then 
> > > don't need to loop over what is exactly one element big.
> > Thanks for the reply. However, I don't think that's true. Yes, it's only 
> > possible to find one `.clang-format` file in the first parent directory. 
> > But if it has `BasedOnStyle: InheritParentConfig` set, then we could find 
> > another `.clang-format` file in the "grandparent" directory. In this case, 
> > we'll have 2 elements in `ChildFormatTextToApply`, but only the very first 
> > one will actually be applied.
> > 
> > To illustrate, suppose we have the following file structure:
> > ```
> > - .clang-format
> > - foo
> >   |- .clang-format
> >   |- input.cpp
> > ```
> > 
> > Both `.clang-format` files have `BasedOnStyle: InheritParentConfig` set. 
> > When running `clang-format --style=file foo/input.cpp`, only the inner 
> > config is applied on the fallback style, while the outer config is ignored. 
> > When testing on a debug build, I encountered a crash due to the failed 
> > assert. When removing the assert, and using a loop to apply the configs, 
> > both the inner and outer configs are applied, which I believe is the 
> > expected behaviour.
> > 
> I can not explain it to you anymore, I would have to dig into it again. But 
> if my tests are correct everything works. You can see that for `Test 9.4` I 
> create a `.clang-format` in `/e/sub/sub/` which is based on 
> `/e/sub/.clang-format` which again is based on `/e/.clang-format`.
> 
> The tests do not fail, the parsed style is as the three files suggest, and 
> the assert holds. I'm pretty sure I have thought about that case, it happens 
> in some kind of recursion.
I took a look through `Test 9.4`, and it doesn't test the case I'm thinking of, 
as it doesn't execute the fallback case.

In ` Test 9.4` the file structure is as follows:
```
- e/
  |- .clang-format (BasedOnStyle: Google) <-- outermost config
  |- sub/
|- .clang-format (BasedOnStyle: `InheritParentConfig)
|- sub/
  |- .clang-format (BasedOnStyle: InheritParentConfig)
  |- code.cpp
```

The reason it doesn't execute the fallback case is that the outermost config 
file doesn't have `BasedOnStyle: InheritParentConfig` set.

On the other hand, in the following directory structure, the fallback case 
would execute, the assert would fail (on debug builds), and only the innermost 
config would be applied (on release builds):
```
- e/
  |- .clang-format (BasedOnStyle: InheritParentConfig) <-- outermost config
  |- sub/
|- .clang-format (BasedOnStyle: `InheritParentConfig)
|- sub/
  |- .clang-format (BasedOnStyle: InheritParentConfig)
  |- code.cpp
```

In order to verify my claims, I think I should write a new test case for this. 
However, I do not know how to run test cases for only clang-format. Is there a 
way to do so? Thanks! 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93844/new/

https://reviews.llvm.org/D93844

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

[PATCH] D72326: [clang-format] Rebased on master: Add option to specify explicit config file

2021-12-28 Thread Zhao Wei Liew via Phabricator via cfe-commits

zwliew added a comment.

In D72326#3211701 , 
@HazardyKnusperkeks wrote:

> Do you plan to refactor something for this review, or do you think you are 
> done? Then I will look at it again as a whole.

I'm going to try refactoring the code for loading and parsing the config file 
into a separate function. I'll update the diff in no longer than a day.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72326/new/

https://reviews.llvm.org/D72326

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

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

2021-12-28 Thread Hyeongyu Kim via Phabricator via cfe-commits

hyeongyukim added a comment.

In D105169#3116810 , @nathanchance 
wrote:

> Prior to the latest revert (fd9b099906c61e46574d1ea2d99b973321fe1d21 
> ), the 
> Linux kernel's binary verifier (`objtool`) points out an issue when building 
> with ThinLTO and `-fsanitize=integer-divide-by-zero`. I have no idea if this 
> is an issue with the tool or this series. A simplified reproducer:
>
>   $ cat ravb_main.i
>   int ravb_set_gti_ndev_rate;
>   unsigned int ravb_set_gti_ndev_inc;
>   void ravb_set_gti_ndev() {
> ravb_set_gti_ndev_inc = 10;
> ravb_set_gti_ndev_inc = ravb_set_gti_ndev_inc / ravb_set_gti_ndev_rate;
> if (ravb_set_gti_ndev_inc)
>   _dev_err(ravb_set_gti_ndev_inc);
>   }
>   
>   $ clang -std=gnu89 -O2 -flto=thin -fsanitize=integer-divide-by-zero -c -o 
> ravb_main.o ravb_main.i
>   
>   $ llvm-ar cDPrsT ravb.o ravb_main.o
>   
>   $ ld.lld -m elf_x86_64 -r -o ravb.lto.o --whole-archive ravb.o
>   
>   $ ./objtool orc generate --no-fp --no-unreachable --retpoline --uaccess 
> --mcount --module ravb.lto.o
>   ravb.lto.o: warning: objtool: .text.ravb_set_gti_ndev: unexpected end of 
> section
>
> With LLVM 13.0.0, there is no warning with those commands. The original and 
> reduced `.i` file, interestingness test, and static `objtool` binary are 
> available here 
> .

I checked the reason why Objtool makes a warning.

  cont.thread:  ; preds = %entry
tail call void @__ubsan_handle_divrem_overflow(i8* bitcast ({ { [12 x i8]*, 
i32, i32 }, { i16, i16, [15 x i8] }* }* @1 to i8*), i64 10, i64 0) #3, 
!nosanitize !8
br label %if.then
  
  cont: ; preds = %entry
%div = udiv i32 10, %0
store i32 %div, i32* @ravb_set_gti_ndev_inc, align 4, !tbaa !4
%tobool.not = icmp ugt i32 %0, 10
br i1 %tobool.not, label %if.end, label %if.then
  
  if.then:  ; preds = %cont.thread, 
%cont
%div3 = phi i32 [ poison, %cont.thread ], [ %div, %cont ]
%call = tail call i32 (i32, ...) bitcast (i32 (...)* @_dev_err to i32 (i32, 
...)*)(i32 noundef %div3) #3
br label %if.end

This IR code is an IR that has not passed the optimization pass completely.
This code calculates the division only if `ravb_set_gti_ndev_rate` is non-zero 
and it calls `ubsan_handle_divrem_overflow` function to handle UB if 
`ravb_set_gti_ndev_rate` is zero.
So far, there is no warning. But a warning occurs when this code passes the 
SimpleCFG optimization.

  cont.thread:  ; preds = %entry
tail call void @__ubsan_handle_divrem_overflow(i8* bitcast ({ { [12 x i8]*, 
i32, i32 }, { i16, i16, [15 x i8] }* }* @1 to i8*), i64 10, i64 0) #3, 
!nosanitize !8
unreachable
  
  cont: ; preds = %entry
%div = udiv i32 10, %0
store i32 %div, i32* @ravb_set_gti_ndev_inc, align 4, !tbaa !4
%tobool.not = icmp ugt i32 %0, 10
br i1 %tobool.not, label %if.end, label %if.then
  
  if.then:  ; preds = %cont
%call = tail call i32 (i32, ...) bitcast (i32 (...)* @_dev_err to i32 (i32, 
...)*)(i32 noundef %div) #3
br label %if.end
  
  if.end:   ; preds = %if.then, %cont
ret void
  }

After it passes the SimplyCFG, the `br` instruction was changed to the 
`unreachable` instruction in `cont.thread` block.

This patch added noundef to the parameter of the `_dev_err` function, making 
the `%div3` unable to be Poison.
It is impossible to jump from the `cont.thread` block to `if.then` block, so 
`br` instruction was changed to `unreachable` instruction.
It would be nice to remove the unreachable block, but the above IR is not wrong 
because it is UB when `ravb_set_gti_ndev_rate` is zero.

There seems to be no existing problem in clang, and I think we can bypass this 
warning by adding a code that checks whether the `gravb_set_gti_ndev_rate` is 
zero or not as follows.

  int ravb_set_gti_ndev_rate;
  unsigned int ravb_set_gti_ndev_inc;
  void ravb_set_gti_ndev() {
ravb_set_gti_ndev_inc = 10;
ravb_set_gti_ndev_inc = ravb_set_gti_ndev_inc / ravb_set_gti_ndev_rate;
if (ravb_set_gti_ndev_rate != 0)
  if (ravb_set_gti_ndev_inc)
_dev_err(ravb_set_gti_ndev_inc);
  }

@nathanchance How about changing the existing test code as above?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105169/new/

https://reviews.llvm.org/D105169

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

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

2021-12-28 Thread Nathan Chancellor via Phabricator via cfe-commits

nathanchance added a comment.

@hyeongyukim I am currently offline for the evening but it seems like my 
reduction might have been too aggressive. It looks like this code comes from 
`ravb_set_gti()` 
(https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/renesas/ravb_main.c?h=v5.16-rc7#n2480),
 which checks that `rate` is not zero before using it as a divisor. I will see 
if I can get a reproducer without any undefined behavior such as this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105169/new/

https://reviews.llvm.org/D105169

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

LLVM buildmaster will be restarted soon

2021-12-28 Thread Galina Kistanova via cfe-commits

 Hello everyone,

LLVM buildmaster will be restarted at 8 PM PST today.

Thanks

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

[PATCH] D113126: [OpenMP][NFCI] Embed the source location string size in the ident_t

2021-12-28 Thread Shilei Tian via Phabricator via cfe-commits

tianshilei1992 added a comment.

In D113126#3122947 , @jdoerfert wrote:

> In D113126#3122659 , 
> @tianshilei1992 wrote:
>
>> I'm not convinced. `std::strlen` can do the job. Can you explain more why 
>> need it?
>
> Because the ident_t is on the device and you want to grab it from the host.

So you mean `strlen` is not available on device?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113126/new/

https://reviews.llvm.org/D113126

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

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

2021-12-28 Thread Hyeongyu Kim via Phabricator via cfe-commits

hyeongyukim added a comment.

In D105169#3211929 , @nathanchance 
wrote:

> @hyeongyukim I am currently offline for the evening but it seems like my 
> reduction might have been too aggressive. It looks like this code comes from 
> `ravb_set_gti()` 
> (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/renesas/ravb_main.c?h=v5.16-rc7#n2480),
>  which checks that `rate` is not zero before using it as a divisor. I will 
> see if I can get a reproducer without any undefined behavior such as this.

Okay! Thank you for your response.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105169/new/

https://reviews.llvm.org/D105169

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

[PATCH] D116352: [CodeCompletion] Signature help for template argument lists

2021-12-28 Thread Sam McCall via Phabricator via cfe-commits

sammccall created this revision.
sammccall added reviewers: kadircet, usaxena95.
Herald added a subscriber: arphaman.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.
Herald added projects: clang, clang-tools-extra.

Provide signature while typing template arguments: Foo< ^here >
Here the parameters are e.g. "typename x", and the result type is e.g.
"struct" (class template) or "int" (variable template) or "bool (std::string)"
(function template).

Multiple overloads are possible when a template name is used for several
overloaded function templates.

Fixes https://github.com/clangd/clangd/issues/299


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116352

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/CodeCompleteConsumer.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Sema/CodeCompleteConsumer.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/template-signature.cpp

Index: clang/test/CodeCompletion/template-signature.cpp
===
--- /dev/null
+++ clang/test/CodeCompletion/template-signature.cpp
@@ -0,0 +1,25 @@
+template  float overloaded(int);
+template  bool overloaded(char);
+
+auto m = overloaded<1, 2>(0);
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:4:21 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+// CHECK-CC1: OPENING_PAREN_LOC: {{.*}}4:20
+// CHECK-CC1-DAG: OVERLOAD: [#float (int)#]overloaded<<#int#>, char y>
+// CHECK-CC1-DAG: OVERLOAD: [#bool (char)#]overloaded<<#class#>, int x>
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:4:24 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+// CHECK-CC2-NOT: OVERLOAD: {{.*}}int x
+// CHECK-CC2: OVERLOAD: [#float (int)#]overloaded>
+
+template  int n = 0;
+int val = n;
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:18 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
+// CHECK-CC3: OVERLOAD: [#int#]n>
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:24 %s -o - | FileCheck -check-prefix=CHECK-CC4 %s
+// CHECK-CC4: OVERLOAD: [#int#]n
+
+template  struct Vector {};
+template  class Container = Vector>
+struct Collection { Container container; };
+Collection collection;
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:23:12 %s -o - | FileCheck -check-prefix=CHECK-CC5 %s
+// CHECK-CC5: OVERLOAD: [#struct#]Collection<<#typename Element#>>
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -36,6 +36,7 @@
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Overload.h"
 #include "clang/Sema/ParsedAttr.h"
+#include "clang/Sema/ParsedTemplate.h"
 #include "clang/Sema/Scope.h"
 #include "clang/Sema/ScopeInfo.h"
 #include "clang/Sema/Sema.h"
@@ -3758,6 +3759,74 @@
   }
 }
 
+std::string formatTemplateParameterPlaceholder(const NamedDecl *Param,
+   bool &Optional,
+   const PrintingPolicy &Policy) {
+  if (const auto *Type = dyn_cast(Param)) {
+Optional = Type->hasDefaultArgument();
+  } else if (const auto *NonType = dyn_cast(Param)) {
+Optional = NonType->hasDefaultArgument();
+  } else if (const auto *Template = dyn_cast(Param)) {
+Optional = Template->hasDefaultArgument();
+  }
+  std::string Result;
+  llvm::raw_string_ostream OS(Result);
+  Param->print(OS, Policy);
+  return Result;
+}
+
+static std::string templateResultType(const TemplateDecl *TD,
+  const PrintingPolicy &Policy) {
+  if (const auto *CTD = dyn_cast(TD))
+return CTD->getTemplatedDecl()->getKindName().str();
+  if (const auto *VTD = dyn_cast(TD))
+return VTD->getTemplatedDecl()->getType().getAsString(Policy);
+  if (const auto *FTD = dyn_cast(TD))
+return FTD->getTemplatedDecl()->getType().getAsString(Policy);
+  if (isa(TD))
+return "type";
+  if (isa(TD))
+return "class";
+  if (isa(TD))
+return "concept";
+  return "";
+}
+
+static CodeCompletionString *createTemplateSignatureString(
+const TemplateDecl *TD, CodeCompletionBuilder &Builder, unsigned CurrentArg,
+const PrintingPolicy &Policy) {
+  llvm::ArrayRef Params = TD->getTemplateParameters()->asArray();
+  CodeCompletionBuilder OptionalBuilder(Builder.getAllocator(),
+Builder.getCodeCompletionTUInfo());
+  std::string ResultType = templateResultType(TD, Policy);
+  if (!ResultType.empty())
+Builder.AddResultTypeChunk(Builder.getAllocator().CopyString(ResultType));
+  Builder.AddTextChunk(
+  Bu

[PATCH] D116352: [CodeCompletion] Signature help for template argument lists

2021-12-28 Thread Sam McCall via Phabricator via cfe-commits

sammccall updated this revision to Diff 396455.
sammccall added a comment.

Clean up style, remove fixed fixme, add an extra testcase.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116352/new/

https://reviews.llvm.org/D116352

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/CodeCompleteConsumer.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Sema/CodeCompleteConsumer.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/template-signature.cpp

Index: clang/test/CodeCompletion/template-signature.cpp
===
--- /dev/null
+++ clang/test/CodeCompletion/template-signature.cpp
@@ -0,0 +1,28 @@
+template  float overloaded(int);
+template  bool overloaded(char);
+
+auto m = overloaded<1, 2>(0);
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:4:21 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+// CHECK-CC1: OPENING_PAREN_LOC: {{.*}}4:20
+// CHECK-CC1-DAG: OVERLOAD: [#float (int)#]overloaded<<#int#>, char y>
+// CHECK-CC1-DAG: OVERLOAD: [#bool (char)#]overloaded<<#class#>, int x>
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:4:24 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+// CHECK-CC2-NOT: OVERLOAD: {{.*}}int x
+// CHECK-CC2: OVERLOAD: [#float (int)#]overloaded>
+
+template  int n = 0;
+int val = n;
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:18 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
+// CHECK-CC3: OVERLOAD: [#int#]n>
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:24 %s -o - | FileCheck -check-prefix=CHECK-CC4 %s
+// CHECK-CC4: OVERLOAD: [#int#]n
+
+template  struct Vector {};
+template  class Container = Vector>
+struct Collection { Container container; };
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:22:31 %s -o - | FileCheck -check-prefix=CHECK-CC5 %s
+// CHECK-CC5: OVERLOAD: [#class#]Container<<#typename E#>>
+Collection collection;
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:25:12 %s -o - | FileCheck -check-prefix=CHECK-CC6 %s
+// CHECK-CC6: OVERLOAD: [#struct#]Collection<<#typename Element#>>
+
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -36,6 +36,7 @@
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Overload.h"
 #include "clang/Sema/ParsedAttr.h"
+#include "clang/Sema/ParsedTemplate.h"
 #include "clang/Sema/Scope.h"
 #include "clang/Sema/ScopeInfo.h"
 #include "clang/Sema/Sema.h"
@@ -3758,6 +3759,74 @@
   }
 }
 
+static std::string
+formatTemplateParameterPlaceholder(const NamedDecl *Param, bool &Optional,
+   const PrintingPolicy &Policy) {
+  if (const auto *Type = dyn_cast(Param)) {
+Optional = Type->hasDefaultArgument();
+  } else if (const auto *NonType = dyn_cast(Param)) {
+Optional = NonType->hasDefaultArgument();
+  } else if (const auto *Template = dyn_cast(Param)) {
+Optional = Template->hasDefaultArgument();
+  }
+  std::string Result;
+  llvm::raw_string_ostream OS(Result);
+  Param->print(OS, Policy);
+  return Result;
+}
+
+static std::string templateResultType(const TemplateDecl *TD,
+  const PrintingPolicy &Policy) {
+  if (const auto *CTD = dyn_cast(TD))
+return CTD->getTemplatedDecl()->getKindName().str();
+  if (const auto *VTD = dyn_cast(TD))
+return VTD->getTemplatedDecl()->getType().getAsString(Policy);
+  if (const auto *FTD = dyn_cast(TD))
+return FTD->getTemplatedDecl()->getType().getAsString(Policy);
+  if (isa(TD))
+return "type";
+  if (isa(TD))
+return "class";
+  if (isa(TD))
+return "concept";
+  return "";
+}
+
+static CodeCompletionString *createTemplateSignatureString(
+const TemplateDecl *TD, CodeCompletionBuilder &Builder, unsigned CurrentArg,
+const PrintingPolicy &Policy) {
+  llvm::ArrayRef Params = TD->getTemplateParameters()->asArray();
+  CodeCompletionBuilder OptionalBuilder(Builder.getAllocator(),
+Builder.getCodeCompletionTUInfo());
+  std::string ResultType = templateResultType(TD, Policy);
+  if (!ResultType.empty())
+Builder.AddResultTypeChunk(Builder.getAllocator().CopyString(ResultType));
+  Builder.AddTextChunk(
+  Builder.getAllocator().CopyString(TD->getNameAsString()));
+  Builder.AddChunk(CodeCompletionString::CK_LeftAngle);
+  // Initially we're writing into the main string. Once we see an optional arg
+  // (with default), we're writing into the nested optional chunk.
+  CodeCompletionBuilder *Current = &Builder;
+  for (unsigned I = 0; I <

[PATCH] D116218: [clangd] Fix selection on multi-dimensional array.

2021-12-28 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

Ooh, this is messy indeed. Thanks for digging!




Comment at: clang-tools-extra/clangd/Selection.cpp:526
+  // the traversal order of SizeExpr and ElementTypeLoc, which gives a chance
+  // for the SizeExpr to claim its tokens.
+  bool TraverseConstantArrayTypeLoc(ConstantArrayTypeLoc X) {

I don't think this is a complete solution: won't the inner ArrayTypeLoc still 
end up owning both sets of brackets?

I think maybe a better solution is making getSourceRange(ArrayTypeLoc) return 
only ATL.getBracketRange(), and then modify canSafelySkipNode to to avoid 
pruning based on this small range.

This is vaguely similar to how DeclTypeLoc works today (though in that case the 
reduced range is the one reported by the AST).



Comment at: clang-tools-extra/clangd/Selection.cpp:527
+  // for the SizeExpr to claim its tokens.
+  bool TraverseConstantArrayTypeLoc(ConstantArrayTypeLoc X) {
+if (!Base::TraverseStmt(X.getSizeExpr()))

ConstantArrayType isn't the only kind of array, see the other subclasses of 
ArrayType


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116218/new/

https://reviews.llvm.org/D116218

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

[PATCH] D72326: [clang-format] Rebased on master: Add option to specify explicit config file

2021-12-28 Thread Zhao Wei Liew via Phabricator via cfe-commits

zwliew updated this revision to Diff 396458.
zwliew added a comment.

Support inheritance in a chain of more than 1 parents

I made the following changes:

1. Refactor the code for loading and parsing configs into a separate function

2. Add a new test case (9.1.2) to test the case mentioned in 
https://reviews.llvm.org/D93844#inline-1112285.

3. Fix the test case failure for the newly added test 9.1.2.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72326/new/

https://reviews.llvm.org/D72326

Files:
  clang/docs/ClangFormat.rst
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -21480,7 +21480,7 @@
   ASSERT_TRUE((bool)StyleTd);
   ASSERT_EQ(*StyleTd, getLLVMStyle(FormatStyle::LK_TableGen));
 
-  // Test 9.1: overwriting a file style, when parent no file exists with no
+  // Test 9.1.1: overwriting a file style, when parent no file exists with no
   // fallback style
   ASSERT_TRUE(FS.addFile(
   "/e/sub/.clang-format", 0,
@@ -21496,6 +21496,25 @@
 return Style;
   }());
 
+  // Test 9.1.2: propagate more than one level with no parent file
+  ASSERT_TRUE(FS.addFile("/e/sub/sub/code.cpp", 0,
+ llvm::MemoryBuffer::getMemBuffer("int i;")));
+  ASSERT_TRUE(FS.addFile("/e/sub/sub/.clang-format", 0,
+ llvm::MemoryBuffer::getMemBuffer(
+ "BasedOnStyle: InheritParentConfig\n"
+ "WhitespaceSensitiveMacros: ['FOO', 'BAR']")));
+  std::vector NonDefaultWhiteSpaceMacros{"FOO", "BAR"};
+
+  ASSERT_NE(Style9->WhitespaceSensitiveMacros, NonDefaultWhiteSpaceMacros);
+  Style9 = getStyle("file", "/e/sub/sub/code.cpp", "none", "", &FS);
+  ASSERT_TRUE(static_cast(Style9));
+  ASSERT_EQ(*Style9, [&NonDefaultWhiteSpaceMacros] {
+auto Style = getNoStyle();
+Style.ColumnLimit = 20;
+Style.WhitespaceSensitiveMacros = NonDefaultWhiteSpaceMacros;
+return Style;
+  }());
+
   // Test 9.2: with LLVM fallback style
   Style9 = getStyle("file", "/e/sub/code.cpp", "LLVM", "", &FS);
   ASSERT_TRUE(static_cast(Style9));
@@ -21519,15 +21538,7 @@
 return Style;
   }());
 
-  // Test 9.4: propagate more than one level
-  ASSERT_TRUE(FS.addFile("/e/sub/sub/code.cpp", 0,
- llvm::MemoryBuffer::getMemBuffer("int i;")));
-  ASSERT_TRUE(FS.addFile("/e/sub/sub/.clang-format", 0,
- llvm::MemoryBuffer::getMemBuffer(
- "BasedOnStyle: InheritParentConfig\n"
- "WhitespaceSensitiveMacros: ['FOO', 'BAR']")));
-  std::vector NonDefaultWhiteSpaceMacros{"FOO", "BAR"};
-
+  // Test 9.4: propagate more than one level with a parent file
   const auto SubSubStyle = [&NonDefaultWhiteSpaceMacros] {
 auto Style = getGoogleStyle();
 Style.ColumnLimit = 20;
@@ -21584,6 +21595,70 @@
 Style.IndentWidth = 7;
 return Style;
   }());
+
+  // Test 9.9: use inheritance from a specific config file
+  Style9 = getStyle("file:/e/sub/sub/.clang-format", "/e/sub/sub/code.cpp",
+"none", "", &FS);
+  ASSERT_TRUE(static_cast(Style9));
+  ASSERT_EQ(*Style9, SubSubStyle);
+}
+
+TEST(FormatStyle, GetStyleFromExternalFile) {
+  llvm::vfs::InMemoryFileSystem FS;
+  // Explicit format file in parent directory.
+  ASSERT_TRUE(
+  FS.addFile("/e/.clang-format", 0,
+ llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: LLVM")));
+  ASSERT_TRUE(
+  FS.addFile("/e/explicit.clang-format", 0,
+ llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google")));
+  ASSERT_TRUE(FS.addFile("/e/sub/sub/sub/test.cpp", 0,
+ llvm::MemoryBuffer::getMemBuffer("int i;")));
+  auto Style = getStyle("file:/e/explicit.clang-format",
+"/e/sub/sub/sub/test.cpp", "LLVM", "", &FS);
+  ASSERT_TRUE(static_cast(Style));
+  ASSERT_EQ(*Style, getGoogleStyle());
+
+  // Relative path to a format file
+  ASSERT_TRUE(
+  FS.addFile("../../e/explicit.clang-format", 0,
+ llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google")));
+  Style = getStyle("file:../../e/explicit.clang-format",
+   "/e/sub/sub/sub/test.cpp", "LLVM", "", &FS);
+  ASSERT_TRUE(static_cast(Style));
+  ASSERT_EQ(*Style, getGoogleStyle());
+
+  // Missing explicit format file
+  Style = getStyle("file:/e/missing.clang-format", "/e/sub/sub/sub/test.cpp",
+   "LLVM", "", &FS);
+  ASSERT_FALSE(static_cast(Style));
+  llvm::consumeError(Style.takeError());
+
+  // Format file from the filesystem
+  SmallString<128> FormatFilePath;
+  std::error_code ECF = llvm::sys::fs::createTemporaryFile(
+

[PATCH] D72326: [clang-format] Rebased on master: Add option to specify explicit config file

2021-12-28 Thread Zhao Wei Liew via Phabricator via cfe-commits

zwliew added inline comments.



Comment at: clang/lib/Format/Format.cpp:3393
   if (!ChildFormatTextToApply.empty()) {
 assert(ChildFormatTextToApply.size() == 1);
 

HazardyKnusperkeks wrote:
> zwliew wrote:
> > Is there a reason for this to be limited to 1? I came across this case 
> > during testing, but I can't think of why this should be so.
> See comment on D93844.
I've added a new test case (9.1.2) to illustrate the test failure, and also 
fixed the code to fix the test case.



Comment at: clang/lib/Format/Format.cpp:3276
+  // Check for explicit config filename
+  if (StyleName.startswith_insensitive("file:")) {
+auto StyleNameFile = StyleName.substr(5);

zwliew wrote:
> HazardyKnusperkeks wrote:
> > zwliew wrote:
> > > Following my code suggestion above, I think this code block could be 
> > > refactored to this: 
> > > ```
> > > // Check for explicit config filename
> > > if (StyleName.startswith_insensitive("file:")) {
> > > auto StyleFileName = StyleName.substr(5);
> > > if (auto ec = loadConfigFile(StyleFileName, FS, &Style)) {
> > > return make_string_error(ec.message());
> > > }
> > > LLVM_DEBUG(llvm::dbgs() << "Using configuration file " << 
> > > StyleFileName << '\n');
> > > return Style;
> > > }
> > > ```
> > > 
> > > What do you think?
> > > 
> > > Also, on another note, I feel like the `-style` option is too overloaded. 
> > > Would it be better to use a separate option like `-style-file` instead?
> > You can certainly refactor code.
> > 
> > What happens with `-style="Google" -style-file="Foo/Bar"` is it different 
> > from `-style-file="Foo/Bar" -style="Google"`?
> > I do not perse disagree, but I think it makes stuff clearer if there is 
> > only one option.
> Fair enough. Some ideas off the top of my head that resolve the ambiguity 
> sound needlessly complicated. Thanks!
I've refactored the code in the latest revision (under 
`loadAndParseConfigFile()`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72326/new/

https://reviews.llvm.org/D72326

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

[PATCH] D93844: [clang-format] Add possibility to be based on parent directory

2021-12-28 Thread Zhao Wei Liew via Phabricator via cfe-commits

zwliew added inline comments.



Comment at: clang/lib/Format/Format.cpp:3067
+  if (!ChildFormatTextToApply.empty()) {
+assert(ChildFormatTextToApply.size() == 1);
+

zwliew wrote:
> HazardyKnusperkeks wrote:
> > zwliew wrote:
> > > HazardyKnusperkeks wrote:
> > > > zwliew wrote:
> > > > > Is there a reason behind limiting the number of children configs to 1 
> > > > > in this case? When the fallback is not used, more than 1 children 
> > > > > configs are allowed (line 3036).
> > > > > 
> > > > > Sorry for digging this up, I came across this seemingly arbitrary 
> > > > > decision when working on some changes to 
> > > > > https://reviews.llvm.org/D72326
> > > > Yeah but it doesn't happen, there is at most only one `.clang-format` 
> > > > in the parent directory path which is found. So we assert on that and 
> > > > then don't need to loop over what is exactly one element big.
> > > Thanks for the reply. However, I don't think that's true. Yes, it's only 
> > > possible to find one `.clang-format` file in the first parent directory. 
> > > But if it has `BasedOnStyle: InheritParentConfig` set, then we could find 
> > > another `.clang-format` file in the "grandparent" directory. In this 
> > > case, we'll have 2 elements in `ChildFormatTextToApply`, but only the 
> > > very first one will actually be applied.
> > > 
> > > To illustrate, suppose we have the following file structure:
> > > ```
> > > - .clang-format
> > > - foo
> > >   |- .clang-format
> > >   |- input.cpp
> > > ```
> > > 
> > > Both `.clang-format` files have `BasedOnStyle: InheritParentConfig` set. 
> > > When running `clang-format --style=file foo/input.cpp`, only the inner 
> > > config is applied on the fallback style, while the outer config is 
> > > ignored. When testing on a debug build, I encountered a crash due to the 
> > > failed assert. When removing the assert, and using a loop to apply the 
> > > configs, both the inner and outer configs are applied, which I believe is 
> > > the expected behaviour.
> > > 
> > I can not explain it to you anymore, I would have to dig into it again. But 
> > if my tests are correct everything works. You can see that for `Test 9.4` I 
> > create a `.clang-format` in `/e/sub/sub/` which is based on 
> > `/e/sub/.clang-format` which again is based on `/e/.clang-format`.
> > 
> > The tests do not fail, the parsed style is as the three files suggest, and 
> > the assert holds. I'm pretty sure I have thought about that case, it 
> > happens in some kind of recursion.
> I took a look through `Test 9.4`, and it doesn't test the case I'm thinking 
> of, as it doesn't execute the fallback case.
> 
> In ` Test 9.4` the file structure is as follows:
> ```
> - e/
>   |- .clang-format (BasedOnStyle: Google) <-- outermost config
>   |- sub/
> |- .clang-format (BasedOnStyle: `InheritParentConfig)
> |- sub/
>   |- .clang-format (BasedOnStyle: InheritParentConfig)
>   |- code.cpp
> ```
> 
> The reason it doesn't execute the fallback case is that the outermost config 
> file doesn't have `BasedOnStyle: InheritParentConfig` set.
> 
> On the other hand, in the following directory structure, the fallback case 
> would execute, the assert would fail (on debug builds), and only the 
> innermost config would be applied (on release builds):
> ```
> - e/
>   |- .clang-format (BasedOnStyle: InheritParentConfig) <-- outermost config
>   |- sub/
> |- .clang-format (BasedOnStyle: `InheritParentConfig)
> |- sub/
>   |- .clang-format (BasedOnStyle: InheritParentConfig)
>   |- code.cpp
> ```
> 
> In order to verify my claims, I think I should write a new test case for 
> this. However, I do not know how to run test cases for only clang-format. Is 
> there a way to do so? Thanks! 
I added a new test case for the latter scenario and ran all the regression test 
cases with `make check-clang`. The test case does indeed fail due to the 
assertion failure. I've updated https://reviews.llvm.org/D72326 with the test 
case and the fix for it. Please have a look, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93844/new/

https://reviews.llvm.org/D93844

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

[PATCH] D113126: [OpenMP][NFCI] Embed the source location string size in the ident_t

2021-12-28 Thread Johannes Doerfert via Phabricator via cfe-commits

jdoerfert added a comment.

In D113126#3211935 , @tianshilei1992 
wrote:

> In D113126#3122947 , @jdoerfert 
> wrote:
>
>> In D113126#3122659 , 
>> @tianshilei1992 wrote:
>>
>>> I'm not convinced. `std::strlen` can do the job. Can you explain more why 
>>> need it?
>>
>> Because the ident_t is on the device and you want to grab it from the host.
>
> So you mean `strlen` is not available on device?

We have a device pointer to an ident_t object. We don't know the corresponding 
host address and we are on the host. We can copy over the ident_t with a single 
d2h but for the string we now need to either launch a kernel that computes 
`strlen` on the device or copy it byte-by-byte, neither are good options. 
Instead, we can simply embed the string length in the otherwise unused ident_t 
field and copy it over with a second d2h memcpy.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113126/new/

https://reviews.llvm.org/D113126

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

[PATCH] D113126: [OpenMP][NFCI] Embed the source location string size in the ident_t

2021-12-28 Thread Shilei Tian via Phabricator via cfe-commits

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

In D113126#3212033 , @jdoerfert wrote:

> In D113126#3211935 , 
> @tianshilei1992 wrote:
>
>> In D113126#3122947 , @jdoerfert 
>> wrote:
>>
>>> In D113126#3122659 , 
>>> @tianshilei1992 wrote:
>>>
 I'm not convinced. `std::strlen` can do the job. Can you explain more why 
 need it?
>>>
>>> Because the ident_t is on the device and you want to grab it from the host.
>>
>> So you mean `strlen` is not available on device?
>
> We have a device pointer to an ident_t object. We don't know the 
> corresponding host address and we are on the host. We can copy over the 
> ident_t with a single d2h but for the string we now need to either launch a 
> kernel that computes `strlen` on the device or copy it byte-by-byte, neither 
> are good options. Instead, we can simply embed the string length in the 
> otherwise unused ident_t field and copy it over with a second d2h memcpy.

I see. Yeah, we don't have the map. Then that makes sense.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113126/new/

https://reviews.llvm.org/D113126

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

[PATCH] D109751: [Clang] Support conversion between PPC double-double and IEEE float128

2021-12-28 Thread Qiu Chaofan via Phabricator via cfe-commits

qiucf added a comment.

In D109751#3191148 , 
@hubert.reinterpretcast wrote:

> In D109751#3178494 , @qiucf wrote:
>
>> Thanks for the reminder. Here GCC and Clang diverges in the handling of 
>> `__ibm128`/`__float128` and `long double`. Not sure whether GCC will 'fix' 
>> the behavior, but here (and in most of the use case in glibc headers) it's 
>> `__builtin_types_compatible_p(..., _Float128)` where GCC/Clang behaves the 
>> same.
>
> I thought Clang doesn't have `_Float128` yet; that's D111382 
> , which makes `_Float128` act like 
> `__float128` (and, in turn, like `__ieee128`).
>
> With `-mabi=ieeelongdouble`:
>
>   extern char x[__builtin_types_compatible_p(long double, __float128) ? 1 : 
> -1]; // GCC accepts; Clang rejects
>
> https://godbolt.org/z/fGbY1Y1PT

I tried making them 'compatible', but that only affects to C (and C++ doesn't 
have this builtin), `std::is_same` still says they're 
different. Should that be an issue?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109751/new/

https://reviews.llvm.org/D109751

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

[clang] 7f1eaea - [OpenMP][NFC] Extract assumption helpers into own header file

2021-12-28 Thread Johannes Doerfert via cfe-commits


Author: Johannes Doerfert
Date: 2021-12-28T23:53:29-06:00
New Revision: 7f1eaeafe7a13fc642e9510f43ec19390b32157d

URL: 
https://github.com/llvm/llvm-project/commit/7f1eaeafe7a13fc642e9510f43ec19390b32157d
DIFF: 
https://github.com/llvm/llvm-project/commit/7f1eaeafe7a13fc642e9510f43ec19390b32157d.diff

LOG: [OpenMP][NFC] Extract assumption helpers into own header file

Added: 
llvm/include/llvm/Frontend/OpenMP/OMPAssume.h

Modified: 
clang/include/clang/AST/OpenMPClause.h
clang/lib/Parse/ParseOpenMP.cpp
clang/lib/Sema/SemaOpenMP.cpp
llvm/include/llvm/Frontend/OpenMP/OMPConstants.h

Removed: 




diff  --git a/clang/include/clang/AST/OpenMPClause.h 
b/clang/include/clang/AST/OpenMPClause.h
index 3fd1b6d300803..3ecc1d40fafc6 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -32,6 +32,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/iterator.h"
 #include "llvm/ADT/iterator_range.h"
+#include "llvm/Frontend/OpenMP/OMPAssume.h"
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
 #include "llvm/Frontend/OpenMP/OMPContext.h"
 #include "llvm/Support/Casting.h"

diff  --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 300b022d83b99..7c783ef0b02b2 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -23,6 +23,7 @@
 #include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/UniqueVector.h"
+#include "llvm/Frontend/OpenMP/OMPAssume.h"
 #include "llvm/Frontend/OpenMP/OMPContext.h"
 
 using namespace clang;

diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index ba0481874577e..61e6d4995b9bc 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -36,6 +36,7 @@
 #include "llvm/ADT/PointerEmbeddedInt.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Frontend/OpenMP/OMPAssume.h"
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
 #include 
 

diff  --git a/llvm/include/llvm/Frontend/OpenMP/OMPAssume.h 
b/llvm/include/llvm/Frontend/OpenMP/OMPAssume.h
new file mode 100644
index 0..c7462ffe6bc0f
--- /dev/null
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPAssume.h
@@ -0,0 +1,55 @@
+//===- OpenMP/OMPAssume.h --- OpenMP assumption helper functions  - C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+/// \file
+///
+/// This file provides helper functions and classes to deal with OpenMP
+/// assumptions, e.g., as used by `[begin/end] assumes` and `assume`.
+///
+//===--===//
+
+#ifndef LLVM_FRONTEND_OPENMP_OMPASSUME_H
+#define LLVM_FRONTEND_OPENMP_OMPASSUME_H
+
+#include "llvm/ADT/StringRef.h"
+
+namespace llvm {
+
+namespace omp {
+
+/// Helper to describe assume clauses.
+struct AssumptionClauseMappingInfo {
+  /// The identifier describing the (beginning of the) clause.
+  llvm::StringLiteral Identifier;
+  /// Flag to determine if the identifier is a full name or the start of a 
name.
+  bool StartsWith;
+  /// Flag to determine if a directive lists follows.
+  bool HasDirectiveList;
+  /// Flag to determine if an expression follows.
+  bool HasExpression;
+};
+
+/// All known assume clauses.
+static constexpr AssumptionClauseMappingInfo AssumptionClauseMappings[] = {
+#define OMP_ASSUME_CLAUSE(Identifier, StartsWith, HasDirectiveList,
\
+  HasExpression)   
\
+  {Identifier, StartsWith, HasDirectiveList, HasExpression},
+#include "llvm/Frontend/OpenMP/OMPKinds.def"
+};
+
+inline std::string getAllAssumeClauseOptions() {
+  std::string S;
+  for (const AssumptionClauseMappingInfo &ACMI : AssumptionClauseMappings)
+S += (S.empty() ? "'" : "', '") + ACMI.Identifier.str();
+  return S + "'";
+}
+
+} // namespace omp
+
+} // namespace llvm
+
+#endif // LLVM_FRONTEND_OPENMP_OMPASSUME_H

diff  --git a/llvm/include/llvm/Frontend/OpenMP/OMPConstants.h 
b/llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
index c82f8e69b7438..0479e305fcb50 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
@@ -25,7 +25,6 @@ class Module;
 class ArrayType;
 class StructType;
 class PointerType;
-class StringRef;
 class FunctionType;
 
 namespace omp {
@@ -80,33 +79,6 @@ enum class IdentFlag {
 #define OMP_IDENT_FLAG(Enum, ...) constexpr auto Enum = omp::IdentFlag::Enum;
 #include "llvm/Frontend/OpenMP/OMPKinds.def"
 
-/// Helper to describe assume clauses.
-struct AssumptionClauseMappingInfo {
-  /// The identifier describing the (beginning of t

[PATCH] D116161: [Clang] Add an overload for emitUnaryBuiltin.

2021-12-28 Thread Jun Zhang via Phabricator via cfe-commits

junaire updated this revision to Diff 396490.
junaire added a comment.

Extend emitUnaryBuiltin instead of adding a new overload, also apply it to 
__builtin_elementwise_abs.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116161/new/

https://reviews.llvm.org/D116161

Files:
  clang/lib/CodeGen/CGBuiltin.cpp

Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -532,13 +532,13 @@
 
 // Emit a simple mangled intrinsic that has 1 argument and a return type
 // matching the argument type.
-static Value *emitUnaryBuiltin(CodeGenFunction &CGF,
-   const CallExpr *E,
-   unsigned IntrinsicID) {
+static Value *emitUnaryBuiltin(CodeGenFunction &CGF, const CallExpr *E,
+   unsigned IntrinsicID,
+   llvm::StringRef Name = "") {
   llvm::Value *Src0 = CGF.EmitScalarExpr(E->getArg(0));
 
   Function *F = CGF.CGM.getIntrinsic(IntrinsicID, Src0->getType());
-  return CGF.Builder.CreateCall(F, Src0);
+  return CGF.Builder.CreateCall(F, Src0, Name);
 }
 
 // Emit an intrinsic that has 2 operands of the same type as its result.
@@ -3122,24 +3122,26 @@
   }
 
   case Builtin::BI__builtin_elementwise_abs: {
-Value *Op0 = EmitScalarExpr(E->getArg(0));
-Value *Result;
-if (Op0->getType()->isIntOrIntVectorTy())
-  Result = Builder.CreateBinaryIntrinsic(
-  llvm::Intrinsic::abs, Op0, Builder.getFalse(), nullptr, "elt.abs");
-else
-  Result = Builder.CreateUnaryIntrinsic(llvm::Intrinsic::fabs, Op0, nullptr,
-"elt.abs");
-return RValue::get(Result);
-  }
+QualType ArgTy = E->getArg(0)->getType();
 
-  case Builtin::BI__builtin_elementwise_ceil: {
-Value *Op0 = EmitScalarExpr(E->getArg(0));
-Value *Result = Builder.CreateUnaryIntrinsic(llvm::Intrinsic::ceil, Op0,
- nullptr, "elt.ceil");
-return RValue::get(Result);
+if (ArgTy->isFloatingType())
+  return RValue::get(
+  emitUnaryBuiltin(*this, E, llvm::Intrinsic::fabs, "elt.abs"));
+if (auto *VecTy = ArgTy->getAs())
+  if (VecTy->getElementType()->isFloatingType())
+return RValue::get(
+emitUnaryBuiltin(*this, E, llvm::Intrinsic::fabs, "elt.abs"));
+
+// If it's type is integer or integer vector.
+return RValue::get(Builder.CreateBinaryIntrinsic(
+llvm::Intrinsic::abs, EmitScalarExpr(E->getArg(0)), Builder.getFalse(),
+nullptr, "elt.abs"));
   }
 
+  case Builtin::BI__builtin_elementwise_ceil:
+return RValue::get(
+emitUnaryBuiltin(*this, E, llvm::Intrinsic::ceil, "elt.ceil"));
+
   case Builtin::BI__builtin_elementwise_max: {
 Value *Op0 = EmitScalarExpr(E->getArg(0));
 Value *Op1 = EmitScalarExpr(E->getArg(1));
@@ -3174,50 +3176,40 @@
   }
 
   case Builtin::BI__builtin_reduce_max: {
-auto GetIntrinsicID = [](QualType QT, llvm::Type *IrTy) {
-  if (IrTy->isIntOrIntVectorTy()) {
-if (auto *VecTy = QT->getAs())
-  QT = VecTy->getElementType();
-if (QT->isSignedIntegerType())
-  return llvm::Intrinsic::vector_reduce_smax;
-else
-  return llvm::Intrinsic::vector_reduce_umax;
-  }
+auto GetIntrinsicID = [](QualType QT) {
+  if (auto *VecTy = QT->getAs())
+QT = VecTy->getElementType();
+  if (QT->isSignedIntegerType())
+return llvm::Intrinsic::vector_reduce_smax;
+  if (QT->isUnsignedIntegerType())
+return llvm::Intrinsic::vector_reduce_umax;
+  assert(QT->isFloatingType() && "must have a float here");
   return llvm::Intrinsic::vector_reduce_fmax;
 };
-Value *Op0 = EmitScalarExpr(E->getArg(0));
-Value *Result = Builder.CreateUnaryIntrinsic(
-GetIntrinsicID(E->getArg(0)->getType(), Op0->getType()), Op0, nullptr,
-"rdx.min");
-return RValue::get(Result);
+return RValue::get(emitUnaryBuiltin(
+*this, E, GetIntrinsicID(E->getArg(0)->getType()), "rd.min"));
   }
 
   case Builtin::BI__builtin_reduce_min: {
-auto GetIntrinsicID = [](QualType QT, llvm::Type *IrTy) {
-  if (IrTy->isIntOrIntVectorTy()) {
-if (auto *VecTy = QT->getAs())
-  QT = VecTy->getElementType();
-if (QT->isSignedIntegerType())
-  return llvm::Intrinsic::vector_reduce_smin;
-else
-  return llvm::Intrinsic::vector_reduce_umin;
-  }
+auto GetIntrinsicID = [](QualType QT) {
+  if (auto *VecTy = QT->getAs())
+QT = VecTy->getElementType();
+  if (QT->isSignedIntegerType())
+return llvm::Intrinsic::vector_reduce_smin;
+  if (QT->isUnsignedIntegerType())
+return llvm::Intrinsic::vector_reduce_umin;
+  assert(QT->isFloatingType() && "must have a float here");
   r

[PATCH] D116161: [Clang] Extend emitUnaryBuiltin to avoid duplicate logic.

2021-12-28 Thread Jun Zhang via Phabricator via cfe-commits

junaire updated this revision to Diff 396492.
junaire edited the summary of this revision.
junaire added a comment.

Refactor code a little bit and fix wrong names.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116161/new/

https://reviews.llvm.org/D116161

Files:
  clang/lib/CodeGen/CGBuiltin.cpp

Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -532,13 +532,13 @@
 
 // Emit a simple mangled intrinsic that has 1 argument and a return type
 // matching the argument type.
-static Value *emitUnaryBuiltin(CodeGenFunction &CGF,
-   const CallExpr *E,
-   unsigned IntrinsicID) {
+static Value *emitUnaryBuiltin(CodeGenFunction &CGF, const CallExpr *E,
+   unsigned IntrinsicID,
+   llvm::StringRef Name = "") {
   llvm::Value *Src0 = CGF.EmitScalarExpr(E->getArg(0));
 
   Function *F = CGF.CGM.getIntrinsic(IntrinsicID, Src0->getType());
-  return CGF.Builder.CreateCall(F, Src0);
+  return CGF.Builder.CreateCall(F, Src0, Name);
 }
 
 // Emit an intrinsic that has 2 operands of the same type as its result.
@@ -3122,24 +3122,25 @@
   }
 
   case Builtin::BI__builtin_elementwise_abs: {
-Value *Op0 = EmitScalarExpr(E->getArg(0));
 Value *Result;
-if (Op0->getType()->isIntOrIntVectorTy())
+QualType QT = E->getArg(0)->getType();
+
+if (auto *VecTy = QT->getAs())
+  QT = VecTy->getElementType();
+if (QT->isIntegerType())
   Result = Builder.CreateBinaryIntrinsic(
-  llvm::Intrinsic::abs, Op0, Builder.getFalse(), nullptr, "elt.abs");
+  llvm::Intrinsic::abs, EmitScalarExpr(E->getArg(0)),
+  Builder.getFalse(), nullptr, "elt.abs");
 else
-  Result = Builder.CreateUnaryIntrinsic(llvm::Intrinsic::fabs, Op0, nullptr,
-"elt.abs");
-return RValue::get(Result);
-  }
+  Result = emitUnaryBuiltin(*this, E, llvm::Intrinsic::fabs, "elt.abs");
 
-  case Builtin::BI__builtin_elementwise_ceil: {
-Value *Op0 = EmitScalarExpr(E->getArg(0));
-Value *Result = Builder.CreateUnaryIntrinsic(llvm::Intrinsic::ceil, Op0,
- nullptr, "elt.ceil");
 return RValue::get(Result);
   }
 
+  case Builtin::BI__builtin_elementwise_ceil:
+return RValue::get(
+emitUnaryBuiltin(*this, E, llvm::Intrinsic::ceil, "elt.ceil"));
+
   case Builtin::BI__builtin_elementwise_max: {
 Value *Op0 = EmitScalarExpr(E->getArg(0));
 Value *Op1 = EmitScalarExpr(E->getArg(1));
@@ -3174,50 +3175,40 @@
   }
 
   case Builtin::BI__builtin_reduce_max: {
-auto GetIntrinsicID = [](QualType QT, llvm::Type *IrTy) {
-  if (IrTy->isIntOrIntVectorTy()) {
-if (auto *VecTy = QT->getAs())
-  QT = VecTy->getElementType();
-if (QT->isSignedIntegerType())
-  return llvm::Intrinsic::vector_reduce_smax;
-else
-  return llvm::Intrinsic::vector_reduce_umax;
-  }
+auto GetIntrinsicID = [](QualType QT) {
+  if (auto *VecTy = QT->getAs())
+QT = VecTy->getElementType();
+  if (QT->isSignedIntegerType())
+return llvm::Intrinsic::vector_reduce_smax;
+  if (QT->isUnsignedIntegerType())
+return llvm::Intrinsic::vector_reduce_umax;
+  assert(QT->isFloatingType() && "must have a float here");
   return llvm::Intrinsic::vector_reduce_fmax;
 };
-Value *Op0 = EmitScalarExpr(E->getArg(0));
-Value *Result = Builder.CreateUnaryIntrinsic(
-GetIntrinsicID(E->getArg(0)->getType(), Op0->getType()), Op0, nullptr,
-"rdx.min");
-return RValue::get(Result);
+return RValue::get(emitUnaryBuiltin(
+*this, E, GetIntrinsicID(E->getArg(0)->getType()), "rdx.min"));
   }
 
   case Builtin::BI__builtin_reduce_min: {
-auto GetIntrinsicID = [](QualType QT, llvm::Type *IrTy) {
-  if (IrTy->isIntOrIntVectorTy()) {
-if (auto *VecTy = QT->getAs())
-  QT = VecTy->getElementType();
-if (QT->isSignedIntegerType())
-  return llvm::Intrinsic::vector_reduce_smin;
-else
-  return llvm::Intrinsic::vector_reduce_umin;
-  }
+auto GetIntrinsicID = [](QualType QT) {
+  if (auto *VecTy = QT->getAs())
+QT = VecTy->getElementType();
+  if (QT->isSignedIntegerType())
+return llvm::Intrinsic::vector_reduce_smin;
+  if (QT->isUnsignedIntegerType())
+return llvm::Intrinsic::vector_reduce_umin;
+  assert(QT->isFloatingType() && "must have a float here");
   return llvm::Intrinsic::vector_reduce_fmin;
 };
-Value *Op0 = EmitScalarExpr(E->getArg(0));
-Value *Result = Builder.CreateUnaryIntrinsic(
-GetIntrinsicID(E->getArg(0)->getType(), Op0->getType()), Op0, nullptr,
-"rdx.min");
-return RVal

72 matches

Mail list logo