[PATCH] D119525: [clang] Fix crash when array size is missing in initializer

[Timm Bäder via Phabricator via cfe-commits]

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



Comment at: clang/docs/ReleaseNotes.rst:54
+--
+- ``CXXNewExpr::getArraySize()`` previously returned a ``llvm::Optional``
+  wrapping a ``nullptr`` when the ``CXXNewExpr`` did not have an array

aaron.ballman wrote:
> tbaeder wrote:
> > aaron.ballman wrote:
> > > Just added a reference to the bug that was fixed.
> > I was wondering about this. Where does the "PRXX" syntax come from? 
> > Since the repo and issues are on github now, I don't see how it makes sense 
> > to call it a PR (it's an issue, not a pull request) and github doesn't 
> > linkify those, while it does when using the `#xx` syntax (which isn't 
> > relevant in this case, but it is when using it in git commit messages). I 
> > have seen other people use the same syntax to refer to issues. I'd probably 
> > just add an actual link to the github issue here if that's permitted. 
> > Thanks for the suggestion though, I'm on PTO this week so don't land this 
> > until Monday  :)
> > I was wondering about this. Where does the "PRXX" syntax come from?
> 
> "Problem Report" -- ancient terminology.
> 
> > I have seen other people use the same syntax to refer to issues. I'd 
> > probably just add an actual link to the github issue here if that's 
> > permitted.
> 
> TBH, I think that's an even better suggestion (linking to the issue). One 
> concern I have is that it's really hard to tell whether the number is a 
> bugzilla issue number or a GitHub issue number (I suppose we should all 
> assume they're always github issue numbers these days though), so I wasn't 
> keen on having a number with no prefix to it. But if we're linking to what's 
> been fixed, then there's never a chance for confusion.
> 
> > Thanks for the suggestion though, I'm on PTO this week so don't land this 
> > until Monday :)
> 
> Sounds good to me, enjoy your PTO!
I added a link for the new entry and replaced the old PR mention with a link as 
well, so it's consistent at least.


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

https://reviews.llvm.org/D119525

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

[PATCH] D119525: [clang] Fix crash when array size is missing in initializer

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 410233.

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

https://reviews.llvm.org/D119525

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ExprCXX.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/AST/issue53742.cpp

Index: clang/test/AST/issue53742.cpp
===
--- /dev/null
+++ clang/test/AST/issue53742.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+
+struct Data {
+  char *a;
+  char *b;
+  bool *c;
+};
+
+int main() {
+  Data in;
+  in.a = new char[](); // expected-error {{cannot determine allocated array size from initializer}}
+  in.c = new bool[100]();
+  in.b = new char[100]();
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -11912,9 +11912,9 @@
 
   // Transform the size of the array we're allocating (if any).
   Optional ArraySize;
-  if (Optional OldArraySize = E->getArraySize()) {
+  if (E->isArray()) {
 ExprResult NewArraySize;
-if (*OldArraySize) {
+if (Optional OldArraySize = E->getArraySize()) {
   NewArraySize = getDerived().TransformExpr(*OldArraySize);
   if (NewArraySize.isInvalid())
 return ExprError();
Index: clang/lib/AST/StmtPrinter.cpp
===
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -2132,10 +2132,10 @@
   if (E->isParenTypeId())
 OS << "(";
   std::string TypeS;
-  if (Optional Size = E->getArraySize()) {
+  if (E->isArray()) {
 llvm::raw_string_ostream s(TypeS);
 s << '[';
-if (*Size)
+if (Optional Size = E->getArraySize())
   (*Size)->printPretty(s, Helper, Policy);
 s << ']';
   }
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -9427,7 +9427,7 @@
   bool ValueInit = false;
 
   QualType AllocType = E->getAllocatedType();
-  if (Optional ArraySize = E->getArraySize()) {
+  if (Optional ArraySize = E->getArraySize()) {
 const Expr *Stripped = *ArraySize;
 for (; auto *ICE = dyn_cast(Stripped);
  Stripped = ICE->getSubExpr())
Index: clang/include/clang/AST/ExprCXX.h
===
--- clang/include/clang/AST/ExprCXX.h
+++ clang/include/clang/AST/ExprCXX.h
@@ -2261,15 +2261,32 @@
 
   bool isArray() const { return CXXNewExprBits.IsArray; }
 
+  /// This might return None even if isArray() returns true,
+  /// since there might not be an array size expression.
+  /// If the result is not-None, it will never wrap a nullptr.
   Optional getArraySize() {
 if (!isArray())
   return None;
-return cast_or_null(getTrailingObjects()[arraySizeOffset()]);
+
+if (auto *Result =
+cast_or_null(getTrailingObjects()[arraySizeOffset()]))
+  return Result;
+
+return None;
   }
+
+  /// This might return None even if isArray() returns true,
+  /// since there might not be an array size expression.
+  /// If the result is not-None, it will never wrap a nullptr.
   Optional getArraySize() const {
 if (!isArray())
   return None;
-return cast_or_null(getTrailingObjects()[arraySizeOffset()]);
+
+if (auto *Result =
+cast_or_null(getTrailingObjects()[arraySizeOffset()]))
+  return Result;
+
+return None;
   }
 
   unsigned getNumPlacementArgs() const {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -54,6 +54,14 @@
   There is an analogous ``zero_call_used_regs`` attribute to allow for finer
   control of this feature.
 
+Bug Fixes
+--
+- ``CXXNewExpr::getArraySize()`` previously returned a ``llvm::Optional``
+  wrapping a ``nullptr`` when the ``CXXNewExpr`` did not have an array
+  size expression. This was fixed and ``::getArraySize()`` will now always
+  either return ``None`` or a ``llvm::Optional`` wrapping a valid ``Expr*``.
+  This fixes `Issue #53742`_.
+
 Improvements to Clang's diagnostics
 ^^^
 
@@ -83,7 +91,8 @@
 - Added support for parameter pack expansion in `clang::annotate`.
 
 - The ``overloadable`` attribute can now be written in all of the syntactic
-  locations a declaration attribute may appear. Fixes PR53805.
+  locations a declaration attribute may appear.
+  This fixes `Issue #53805`_.
 
 Windows Support
 ---
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listin

[PATCH] D120220: [clang-format][NFC] Fix typos and inconsistencies

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/Format.cpp:1664
 
+  // Ensure the list has 'type' in it
   auto type = std::find(Style->QualifierOrder.begin(),

HazardyKnusperkeks wrote:
> ;)
Good catch! I will add it when landing D120217.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120220

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

[PATCH] D120111: [AArch64] Default HBC/MOPS features in clang

[Son Tuan Vu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:464-473
+  const char *v8691OrLater[] = {"+v8.6a", "+v8.7a", "+v8.8a",
+   "+v9.1a", "+v9.2a", "+v9.3a"};
   auto Pos = std::find_first_of(Features.begin(), Features.end(),
-std::begin(Archs), std::end(Archs));
+std::begin(v8691OrLater), 
std::end(v8691OrLater));
   if (Pos != std::end(Features))
 Pos = Features.insert(std::next(Pos), {"+i8mm", "+bf16"});
 

nickdesaulniers wrote:
> Is it possible to re-use some of the many calls to std::find (L395-403)? 
> Seems like we re-scan the feature list A LOT.
I cached the architecture feature when it is defined to avoid these calls to 
std::find.



Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:466-474
   auto Pos = std::find_first_of(Features.begin(), Features.end(),
-std::begin(Archs), std::end(Archs));
+std::begin(v8691OrLater), 
std::end(v8691OrLater));
   if (Pos != std::end(Features))
 Pos = Features.insert(std::next(Pos), {"+i8mm", "+bf16"});
 
+  // For Armv8.8-a/Armv9.3-a or later, FEAT_HBC and FEAT_MOPS are enabled by
+  // default.

nickdesaulniers wrote:
> Can we reuse `ItBegin` and `ItEnd` here rather than `Features.begin()` and 
> `Features.end()`?
This is not relevant anymore, but for the initial code, I guess we can't reuse 
`ItBegin/End` because `Features` has been changed since then (a lot of 
`push_back`). The reason why we need to keep these default features (`+hbc`, 
`+mops`, ...) //right after //the architecture feature is that we want to be 
able to disable the default features `X` if `+noX` option is given in the 
command line, knowing that `+noX` features will come after the architecture 
feature in the `Features` list.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120111

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

[clang] c1f17b0 - [RISCV] Fix the include search path order between sysroot and resource folder (Recommit again)

[Kito Cheng via cfe-commits]

Author: Kito Cheng
Date: 2022-02-21T15:25:21+08:00
New Revision: c1f17b0a9ea0d467eaa9589cc28db2787efe3ebf

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

LOG: [RISCV] Fix the include search path order between sysroot and resource 
folder (Recommit again)

Resource folder[1] should include before sysroot[2] in general (Linux clang
toolchain, BareMetal clang toolchain, and GCC using that order), and that
prevent sysroot's header file override resource folder's one, this change is
reference from BareMetal::addclangsystemincludea...@baremetal.cpp[3].

And also fix the behavior of `-nobuiltininc`.

[1] Include path from resource folder is something like this: 
`/lib/clang/13.0.0/include/`
[2] Include path from sysroot is something like this: 
`/riscv32-unknown-elf/include`
[3] 
https://github.com/llvm/llvm-project/blob/llvmorg-13.0.1/clang/lib/Driver/ToolChains/BareMetal.cpp#L193

Reviewed By: asb

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

The recommit fixes the Windows build failure due to path issue.

Added: 
clang/test/Driver/Inputs/resource_dir/include/.keep

Modified: 
clang/lib/Driver/ToolChains/RISCVToolchain.cpp
clang/test/Driver/riscv32-toolchain.c
clang/test/Driver/riscv64-toolchain.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/RISCVToolchain.cpp 
b/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
index 714325a2db39e..a63ada0cbb7e4 100644
--- a/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
+++ b/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
@@ -98,6 +98,12 @@ void RISCVToolChain::AddClangSystemIncludeArgs(const ArgList 
&DriverArgs,
   if (DriverArgs.hasArg(options::OPT_nostdinc))
 return;
 
+  if (!DriverArgs.hasArg(options::OPT_nobuiltininc)) {
+SmallString<128> Dir(getDriver().ResourceDir);
+llvm::sys::path::append(Dir, "include");
+addSystemInclude(DriverArgs, CC1Args, Dir.str());
+  }
+
   if (!DriverArgs.hasArg(options::OPT_nostdlibinc)) {
 SmallString<128> Dir(computeSysRoot());
 llvm::sys::path::append(Dir, "include");

diff  --git a/clang/test/Driver/Inputs/resource_dir/include/.keep 
b/clang/test/Driver/Inputs/resource_dir/include/.keep
new file mode 100644
index 0..e69de29bb2d1d

diff  --git a/clang/test/Driver/riscv32-toolchain.c 
b/clang/test/Driver/riscv32-toolchain.c
index fb262a19a0439..50859aaccd7da 100644
--- a/clang/test/Driver/riscv32-toolchain.c
+++ b/clang/test/Driver/riscv32-toolchain.c
@@ -197,6 +197,20 @@
 // C-RV32-RTLIB-COMPILERRT-ILP32: "--start-group" "-lc" "-lgloss" 
"--end-group" "{{.*}}libclang_rt.builtins-riscv32.a"
 // C-RV32-RTLIB-COMPILERRT-ILP32: "{{.*}}clang_rt.crtend-riscv32.o"
 
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree \
+// RUN:   -resource-dir=%s/Inputs/resource_dir 2>&1 \
+// RUN:   | FileCheck -check-prefix=RESOURCE-INC %s
+// RESOURCE-INC: "-internal-isystem" 
"{{.*}}/Inputs/resource_dir{{/|}}include"
+// RESOURCE-INC: "-internal-isystem" 
"{{.*}}/basic_riscv32_tree{{.*}}riscv32-unknown-elf{{/|}}include"
+
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree \
+// RUN:   -resource-dir=%s/Inputs/resource_dir -nobuiltininc 2>&1 \
+// RUN:   | FileCheck -check-prefix=NO-RESOURCE-INC %s
+// NO-RESOURCE-INC-NOT: "-internal-isystem" 
"{{.*}}/Inputs/resource_dir{{/|}}include"
+// NO-RESOURCE-INC: "-internal-isystem" 
"{{.*}}/basic_riscv32_tree/{{.*}}riscv32-unknown-elf{{/|}}include"
+
 // RUN: %clang -target riscv32 %s -emit-llvm -S -o - | FileCheck %s
 
 typedef __builtin_va_list va_list;

diff  --git a/clang/test/Driver/riscv64-toolchain.c 
b/clang/test/Driver/riscv64-toolchain.c
index 2774e004854c3..59580370c0b34 100644
--- a/clang/test/Driver/riscv64-toolchain.c
+++ b/clang/test/Driver/riscv64-toolchain.c
@@ -153,6 +153,20 @@
 // C-RV64-RTLIB-COMPILERRT-LP64: "--start-group" "-lc" "-lgloss" "--end-group" 
"{{.*}}libclang_rt.builtins-riscv64.a"
 // C-RV64-RTLIB-COMPILERRT-LP64: "{{.*}}clang_rt.crtend-riscv64.o"
 
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree \
+// RUN:   -resource-dir=%s/Inputs/resource_dir 2>&1 \
+// RUN:   | FileCheck -check-prefix=RESOURCE-INC %s
+// RESOURCE-INC: "-internal-isystem" 
"{{.*}}/Inputs/resource_dir{{/|}}include"
+// RESOURCE-INC: "-internal-isystem" 
"{{.*}}/basic_riscv64_tree/{{.*}}riscv64-unknown-elf{{/|}}include"
+
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree \
+// RUN:   -resource-dir=%s/Inputs/resource_dir -nobuiltininc 2>&1 \
+// RUN:   | FileCheck -check-prefix=NO-RESOURCE-INC %s
+// NO-RESOURCE-INC-NOT

[PATCH] D120111: [AArch64] Default HBC/MOPS features in clang

[Son Tuan Vu via Phabricator via cfe-commits]

tyb0807 updated this revision to Diff 410225.
tyb0807 added a comment.

Cache architecture feature to avoid scanning the feature list over and over 
again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120111

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/test/Driver/aarch64-hbc.c
  clang/test/Driver/aarch64-mops.c
  clang/test/Preprocessor/aarch64-target-features.c

Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -521,13 +521,17 @@
 // CHECK-LSE: __ARM_FEATURE_ATOMICS 1
 
 // == Check Armv8.8-A/Armv9.3-A memcpy and memset acceleration instructions (MOPS)
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.7-a  -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-NOMOPS %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.7-a+mops -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.8-a  -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.8-a+mops -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv9.2-a  -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-NOMOPS %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv9.2-a+mops -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv9.3-a  -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv9.3-a+mops -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.7-a -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-NOMOPS %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.7-a+mops-x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.8-a -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.8-a+nomops  -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-NOMOPS %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.8-a+nomops+mops -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.8-a+mops-x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.8-a+mops+nomops -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-NOMOPS %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv9.2-a -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-NOMOPS %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv9.2-a+mops-x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv9.3-a -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv9.3-a+nomops  -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-NOMOPS %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv9.3-a+mops-x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
 // CHECK-MOPS: __ARM_FEATURE_MOPS 1
 // CHECK-NOMOPS-NOT: __ARM_FEATURE_MOPS 1
Index: clang/test/Driver/aarch64-mops.c
===
--- clang/test/Driver/aarch64-mops.c
+++ clang/test/Driver/aarch64-mops.c
@@ -1,6 +1,12 @@
 // Test that target feature mops is implemented and available correctly
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a+mops %s 2>&1 | FileCheck %s
-// CHECK: "-target-feature" "+mops"
-
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.7-a+mops   %s 2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a%s 2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a+mops   %s 2>&1 | FileCheck %s
 // RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a+nomops %s 2>&1 | FileCheck %s --check-prefix=NO_MOPS
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.2-a+mops   %s 2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.3-a%s 2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.3-a+mops   %s 2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.3-a+nomops %s 2>&1 | FileCheck %s --check-prefix=NO_MOPS
+
+// CHECK: "-target-feature" "+mops"
 // NO_MOPS: "-target-

[PATCH] D120220: [clang-format][NFC] Fix typos and inconsistencies

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

A bit late, but maybe you want to do a follow up. ;)




Comment at: clang/lib/Format/Format.cpp:1664
 
+  // Ensure the list has 'type' in it
   auto type = std::find(Style->QualifierOrder.begin(),

;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120220

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

[clang] cc27952 - Revert "[RISCV] Fix the include search path order between sysroot and resource folder (Recommit)"

[Kito Cheng via cfe-commits]

Author: Kito Cheng
Date: 2022-02-21T14:56:58+08:00
New Revision: cc279529e8317301492f9625b6acc9a0bf52db56

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

LOG: Revert "[RISCV] Fix the include search path order between sysroot and 
resource folder (Recommit)"

This reverts commit 47b1fa5fc48821eefefd157ed4af2f2cf3bacef4.

Added: 


Modified: 
clang/lib/Driver/ToolChains/RISCVToolchain.cpp
clang/test/Driver/riscv32-toolchain.c
clang/test/Driver/riscv64-toolchain.c

Removed: 
clang/test/Driver/Inputs/resource_dir/include/.keep



diff  --git a/clang/lib/Driver/ToolChains/RISCVToolchain.cpp 
b/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
index a63ada0cbb7e4..714325a2db39e 100644
--- a/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
+++ b/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
@@ -98,12 +98,6 @@ void RISCVToolChain::AddClangSystemIncludeArgs(const ArgList 
&DriverArgs,
   if (DriverArgs.hasArg(options::OPT_nostdinc))
 return;
 
-  if (!DriverArgs.hasArg(options::OPT_nobuiltininc)) {
-SmallString<128> Dir(getDriver().ResourceDir);
-llvm::sys::path::append(Dir, "include");
-addSystemInclude(DriverArgs, CC1Args, Dir.str());
-  }
-
   if (!DriverArgs.hasArg(options::OPT_nostdlibinc)) {
 SmallString<128> Dir(computeSysRoot());
 llvm::sys::path::append(Dir, "include");

diff  --git a/clang/test/Driver/Inputs/resource_dir/include/.keep 
b/clang/test/Driver/Inputs/resource_dir/include/.keep
deleted file mode 100644
index e69de29bb2d1d..0

diff  --git a/clang/test/Driver/riscv32-toolchain.c 
b/clang/test/Driver/riscv32-toolchain.c
index 25aaca78dab2c..fb262a19a0439 100644
--- a/clang/test/Driver/riscv32-toolchain.c
+++ b/clang/test/Driver/riscv32-toolchain.c
@@ -197,20 +197,6 @@
 // C-RV32-RTLIB-COMPILERRT-ILP32: "--start-group" "-lc" "-lgloss" 
"--end-group" "{{.*}}libclang_rt.builtins-riscv32.a"
 // C-RV32-RTLIB-COMPILERRT-ILP32: "{{.*}}clang_rt.crtend-riscv32.o"
 
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 \
-// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree \
-// RUN:   -resource-dir=%s/Inputs/resource_dir 2>&1 \
-// RUN:   | FileCheck -check-prefix=RESOURCE-INC %s
-// RESOURCE-INC: "-internal-isystem" 
"{{.*}}/Inputs/resource_dir{{/|}}include"
-// RESOURCE-INC: "-internal-isystem" 
"{{.*}}/basic_riscv32_tree/{{.*}}/riscv32-unknown-elf{{/|}}include"
-
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 \
-// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree \
-// RUN:   -resource-dir=%s/Inputs/resource_dir -nobuiltininc 2>&1 \
-// RUN:   | FileCheck -check-prefix=NO-RESOURCE-INC %s
-// NO-RESOURCE-INC-NOT: "-internal-isystem" 
"{{.*}}/Inputs/resource_dir{{/|}}include"
-// NO-RESOURCE-INC: "-internal-isystem" 
"{{.*}}/basic_riscv32_tree/{{.*}}/riscv32-unknown-elf{{/|}}include"
-
 // RUN: %clang -target riscv32 %s -emit-llvm -S -o - | FileCheck %s
 
 typedef __builtin_va_list va_list;

diff  --git a/clang/test/Driver/riscv64-toolchain.c 
b/clang/test/Driver/riscv64-toolchain.c
index 91358298ecdd8..2774e004854c3 100644
--- a/clang/test/Driver/riscv64-toolchain.c
+++ b/clang/test/Driver/riscv64-toolchain.c
@@ -153,20 +153,6 @@
 // C-RV64-RTLIB-COMPILERRT-LP64: "--start-group" "-lc" "-lgloss" "--end-group" 
"{{.*}}libclang_rt.builtins-riscv64.a"
 // C-RV64-RTLIB-COMPILERRT-LP64: "{{.*}}clang_rt.crtend-riscv64.o"
 
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 \
-// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree \
-// RUN:   -resource-dir=%s/Inputs/resource_dir 2>&1 \
-// RUN:   | FileCheck -check-prefix=RESOURCE-INC %s
-// RESOURCE-INC: "-internal-isystem" "{{.*}}/Inputs/resource_dir/include"
-// RESOURCE-INC: "-internal-isystem" 
"{{.*}}/basic_riscv64_tree/{{.*}}/riscv64-unknown-elf/include"
-
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 \
-// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree \
-// RUN:   -resource-dir=%s/Inputs/resource_dir -nobuiltininc 2>&1 \
-// RUN:   | FileCheck -check-prefix=NO-RESOURCE-INC %s
-// NO-RESOURCE-INC-NOT: "-internal-isystem" "{{.*}}Inputs/resource_dir/include"
-// NO-RESOURCE-INC: "-internal-isystem" 
"{{.*}}/basic_riscv64_tree/{{.*}}/riscv64-unknown-elf/include"
-
 // RUN: %clang -target riscv64 %s -emit-llvm -S -o - | FileCheck %s
 
 typedef __builtin_va_list va_list;



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

[PATCH] D119837: [RISCV] Fix the include search path order between sysroot and resource folder

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added a comment.

Recommit with windows path fix: 
https://reviews.llvm.org/rG47b1fa5fc48821eefefd157ed4af2f2cf3bacef4


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119837

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

[clang] 47b1fa5 - [RISCV] Fix the include search path order between sysroot and resource folder (Recommit)

[Kito Cheng via cfe-commits]

Author: Kito Cheng
Date: 2022-02-21T14:39:43+08:00
New Revision: 47b1fa5fc48821eefefd157ed4af2f2cf3bacef4

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

LOG: [RISCV] Fix the include search path order between sysroot and resource 
folder (Recommit)

Resource folder[1] should include before sysroot[2] in general (Linux clang
toolchain, BareMetal clang toolchain, and GCC using that order), and that
prevent sysroot's header file override resource folder's one, this change is
reference from BareMetal::addclangsystemincludea...@baremetal.cpp[3].

And also fix the behavior of `-nobuiltininc`.

[1] Include path from resource folder is something like this: 
`/lib/clang/13.0.0/include/`
[2] Include path from sysroot is something like this: 
`/riscv32-unknown-elf/include`
[3] 
https://github.com/llvm/llvm-project/blob/llvmorg-13.0.1/clang/lib/Driver/ToolChains/BareMetal.cpp#L193

Reviewed By: asb

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

The recommit fixes the Windows build failure due to path issue.

Added: 
clang/test/Driver/Inputs/resource_dir/include/.keep

Modified: 
clang/lib/Driver/ToolChains/RISCVToolchain.cpp
clang/test/Driver/riscv32-toolchain.c
clang/test/Driver/riscv64-toolchain.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/RISCVToolchain.cpp 
b/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
index 714325a2db39e..a63ada0cbb7e4 100644
--- a/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
+++ b/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
@@ -98,6 +98,12 @@ void RISCVToolChain::AddClangSystemIncludeArgs(const ArgList 
&DriverArgs,
   if (DriverArgs.hasArg(options::OPT_nostdinc))
 return;
 
+  if (!DriverArgs.hasArg(options::OPT_nobuiltininc)) {
+SmallString<128> Dir(getDriver().ResourceDir);
+llvm::sys::path::append(Dir, "include");
+addSystemInclude(DriverArgs, CC1Args, Dir.str());
+  }
+
   if (!DriverArgs.hasArg(options::OPT_nostdlibinc)) {
 SmallString<128> Dir(computeSysRoot());
 llvm::sys::path::append(Dir, "include");

diff  --git a/clang/test/Driver/Inputs/resource_dir/include/.keep 
b/clang/test/Driver/Inputs/resource_dir/include/.keep
new file mode 100644
index 0..e69de29bb2d1d

diff  --git a/clang/test/Driver/riscv32-toolchain.c 
b/clang/test/Driver/riscv32-toolchain.c
index fb262a19a0439..25aaca78dab2c 100644
--- a/clang/test/Driver/riscv32-toolchain.c
+++ b/clang/test/Driver/riscv32-toolchain.c
@@ -197,6 +197,20 @@
 // C-RV32-RTLIB-COMPILERRT-ILP32: "--start-group" "-lc" "-lgloss" 
"--end-group" "{{.*}}libclang_rt.builtins-riscv32.a"
 // C-RV32-RTLIB-COMPILERRT-ILP32: "{{.*}}clang_rt.crtend-riscv32.o"
 
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree \
+// RUN:   -resource-dir=%s/Inputs/resource_dir 2>&1 \
+// RUN:   | FileCheck -check-prefix=RESOURCE-INC %s
+// RESOURCE-INC: "-internal-isystem" 
"{{.*}}/Inputs/resource_dir{{/|}}include"
+// RESOURCE-INC: "-internal-isystem" 
"{{.*}}/basic_riscv32_tree/{{.*}}/riscv32-unknown-elf{{/|}}include"
+
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree \
+// RUN:   -resource-dir=%s/Inputs/resource_dir -nobuiltininc 2>&1 \
+// RUN:   | FileCheck -check-prefix=NO-RESOURCE-INC %s
+// NO-RESOURCE-INC-NOT: "-internal-isystem" 
"{{.*}}/Inputs/resource_dir{{/|}}include"
+// NO-RESOURCE-INC: "-internal-isystem" 
"{{.*}}/basic_riscv32_tree/{{.*}}/riscv32-unknown-elf{{/|}}include"
+
 // RUN: %clang -target riscv32 %s -emit-llvm -S -o - | FileCheck %s
 
 typedef __builtin_va_list va_list;

diff  --git a/clang/test/Driver/riscv64-toolchain.c 
b/clang/test/Driver/riscv64-toolchain.c
index 2774e004854c3..91358298ecdd8 100644
--- a/clang/test/Driver/riscv64-toolchain.c
+++ b/clang/test/Driver/riscv64-toolchain.c
@@ -153,6 +153,20 @@
 // C-RV64-RTLIB-COMPILERRT-LP64: "--start-group" "-lc" "-lgloss" "--end-group" 
"{{.*}}libclang_rt.builtins-riscv64.a"
 // C-RV64-RTLIB-COMPILERRT-LP64: "{{.*}}clang_rt.crtend-riscv64.o"
 
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree \
+// RUN:   -resource-dir=%s/Inputs/resource_dir 2>&1 \
+// RUN:   | FileCheck -check-prefix=RESOURCE-INC %s
+// RESOURCE-INC: "-internal-isystem" "{{.*}}/Inputs/resource_dir/include"
+// RESOURCE-INC: "-internal-isystem" 
"{{.*}}/basic_riscv64_tree/{{.*}}/riscv64-unknown-elf/include"
+
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree \
+// RUN:   -resource-dir=%s/Inputs/resource_dir -nobuiltininc 2>&1 \
+// RUN:   | FileCheck -check-prefix=NO-RESOURCE-INC %s
+// NO-RESOURCE-INC-NOT: "-internal-isystem"

[PATCH] D119837: [RISCV] Fix the include search path order between sysroot and resource folder

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added a comment.

Revert due to windows build regression fail: 
https://lab.llvm.org/buildbot/#/builders/216/builds/195


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119837

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

[clang] 0a17ee1 - Revert "[RISCV] Fix the include search path order between sysroot and resource folder"

[Kito Cheng via cfe-commits]

Author: Kito Cheng
Date: 2022-02-21T14:25:49+08:00
New Revision: 0a17ee1ebe0c3384520ea14fdc1d33e38217341a

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

LOG: Revert "[RISCV] Fix the include search path order between sysroot and 
resource folder"

This reverts commit 079d13668bf1b7f929f1897af90f64caae41c81d.

Added: 


Modified: 
clang/lib/Driver/ToolChains/RISCVToolchain.cpp
clang/test/Driver/riscv32-toolchain.c
clang/test/Driver/riscv64-toolchain.c

Removed: 
clang/test/Driver/Inputs/resource_dir/include/.keep



diff  --git a/clang/lib/Driver/ToolChains/RISCVToolchain.cpp 
b/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
index a63ada0cbb7e4..714325a2db39e 100644
--- a/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
+++ b/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
@@ -98,12 +98,6 @@ void RISCVToolChain::AddClangSystemIncludeArgs(const ArgList 
&DriverArgs,
   if (DriverArgs.hasArg(options::OPT_nostdinc))
 return;
 
-  if (!DriverArgs.hasArg(options::OPT_nobuiltininc)) {
-SmallString<128> Dir(getDriver().ResourceDir);
-llvm::sys::path::append(Dir, "include");
-addSystemInclude(DriverArgs, CC1Args, Dir.str());
-  }
-
   if (!DriverArgs.hasArg(options::OPT_nostdlibinc)) {
 SmallString<128> Dir(computeSysRoot());
 llvm::sys::path::append(Dir, "include");

diff  --git a/clang/test/Driver/Inputs/resource_dir/include/.keep 
b/clang/test/Driver/Inputs/resource_dir/include/.keep
deleted file mode 100644
index e69de29bb2d1d..0

diff  --git a/clang/test/Driver/riscv32-toolchain.c 
b/clang/test/Driver/riscv32-toolchain.c
index c480a7c00a367..fb262a19a0439 100644
--- a/clang/test/Driver/riscv32-toolchain.c
+++ b/clang/test/Driver/riscv32-toolchain.c
@@ -197,20 +197,6 @@
 // C-RV32-RTLIB-COMPILERRT-ILP32: "--start-group" "-lc" "-lgloss" 
"--end-group" "{{.*}}libclang_rt.builtins-riscv32.a"
 // C-RV32-RTLIB-COMPILERRT-ILP32: "{{.*}}clang_rt.crtend-riscv32.o"
 
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 \
-// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree \
-// RUN:   -resource-dir=%s/Inputs/resource_dir 2>&1 \
-// RUN:   | FileCheck -check-prefix=RESOURCE-INC %s
-// RESOURCE-INC: "-internal-isystem" "{{.*}}/Inputs/resource_dir/include"
-// RESOURCE-INC: "-internal-isystem" 
"{{.*}}/basic_riscv32_tree/{{.*}}/riscv32-unknown-elf/include"
-
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 \
-// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree \
-// RUN:   -resource-dir=%s/Inputs/resource_dir -nobuiltininc 2>&1 \
-// RUN:   | FileCheck -check-prefix=NO-RESOURCE-INC %s
-// NO-RESOURCE-INC-NOT: "-internal-isystem" 
"{{.*}}/Inputs/resource_dir/include"
-// NO-RESOURCE-INC: "-internal-isystem" 
"{{.*}}/basic_riscv32_tree/{{.*}}/riscv32-unknown-elf/include"
-
 // RUN: %clang -target riscv32 %s -emit-llvm -S -o - | FileCheck %s
 
 typedef __builtin_va_list va_list;

diff  --git a/clang/test/Driver/riscv64-toolchain.c 
b/clang/test/Driver/riscv64-toolchain.c
index 91358298ecdd8..2774e004854c3 100644
--- a/clang/test/Driver/riscv64-toolchain.c
+++ b/clang/test/Driver/riscv64-toolchain.c
@@ -153,20 +153,6 @@
 // C-RV64-RTLIB-COMPILERRT-LP64: "--start-group" "-lc" "-lgloss" "--end-group" 
"{{.*}}libclang_rt.builtins-riscv64.a"
 // C-RV64-RTLIB-COMPILERRT-LP64: "{{.*}}clang_rt.crtend-riscv64.o"
 
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 \
-// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree \
-// RUN:   -resource-dir=%s/Inputs/resource_dir 2>&1 \
-// RUN:   | FileCheck -check-prefix=RESOURCE-INC %s
-// RESOURCE-INC: "-internal-isystem" "{{.*}}/Inputs/resource_dir/include"
-// RESOURCE-INC: "-internal-isystem" 
"{{.*}}/basic_riscv64_tree/{{.*}}/riscv64-unknown-elf/include"
-
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 \
-// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree \
-// RUN:   -resource-dir=%s/Inputs/resource_dir -nobuiltininc 2>&1 \
-// RUN:   | FileCheck -check-prefix=NO-RESOURCE-INC %s
-// NO-RESOURCE-INC-NOT: "-internal-isystem" "{{.*}}Inputs/resource_dir/include"
-// NO-RESOURCE-INC: "-internal-isystem" 
"{{.*}}/basic_riscv64_tree/{{.*}}/riscv64-unknown-elf/include"
-
 // RUN: %clang -target riscv64 %s -emit-llvm -S -o - | FileCheck %s
 
 typedef __builtin_va_list va_list;



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

[PATCH] D119837: [RISCV] Fix the include search path order between sysroot and resource folder

[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 rG079d13668bf1: [RISCV] Fix the include search path order 
between sysroot and resource folder (authored by Kito Cheng 
).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119837

Files:
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/test/Driver/Inputs/resource_dir/include/.keep
  clang/test/Driver/riscv32-toolchain.c
  clang/test/Driver/riscv64-toolchain.c


Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -153,6 +153,20 @@
 // C-RV64-RTLIB-COMPILERRT-LP64: "--start-group" "-lc" "-lgloss" "--end-group" 
"{{.*}}libclang_rt.builtins-riscv64.a"
 // C-RV64-RTLIB-COMPILERRT-LP64: "{{.*}}clang_rt.crtend-riscv64.o"
 
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree \
+// RUN:   -resource-dir=%s/Inputs/resource_dir 2>&1 \
+// RUN:   | FileCheck -check-prefix=RESOURCE-INC %s
+// RESOURCE-INC: "-internal-isystem" "{{.*}}/Inputs/resource_dir/include"
+// RESOURCE-INC: "-internal-isystem" 
"{{.*}}/basic_riscv64_tree/{{.*}}/riscv64-unknown-elf/include"
+
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree \
+// RUN:   -resource-dir=%s/Inputs/resource_dir -nobuiltininc 2>&1 \
+// RUN:   | FileCheck -check-prefix=NO-RESOURCE-INC %s
+// NO-RESOURCE-INC-NOT: "-internal-isystem" "{{.*}}Inputs/resource_dir/include"
+// NO-RESOURCE-INC: "-internal-isystem" 
"{{.*}}/basic_riscv64_tree/{{.*}}/riscv64-unknown-elf/include"
+
 // RUN: %clang -target riscv64 %s -emit-llvm -S -o - | FileCheck %s
 
 typedef __builtin_va_list va_list;
Index: clang/test/Driver/riscv32-toolchain.c
===
--- clang/test/Driver/riscv32-toolchain.c
+++ clang/test/Driver/riscv32-toolchain.c
@@ -197,6 +197,20 @@
 // C-RV32-RTLIB-COMPILERRT-ILP32: "--start-group" "-lc" "-lgloss" 
"--end-group" "{{.*}}libclang_rt.builtins-riscv32.a"
 // C-RV32-RTLIB-COMPILERRT-ILP32: "{{.*}}clang_rt.crtend-riscv32.o"
 
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree \
+// RUN:   -resource-dir=%s/Inputs/resource_dir 2>&1 \
+// RUN:   | FileCheck -check-prefix=RESOURCE-INC %s
+// RESOURCE-INC: "-internal-isystem" "{{.*}}/Inputs/resource_dir/include"
+// RESOURCE-INC: "-internal-isystem" 
"{{.*}}/basic_riscv32_tree/{{.*}}/riscv32-unknown-elf/include"
+
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree \
+// RUN:   -resource-dir=%s/Inputs/resource_dir -nobuiltininc 2>&1 \
+// RUN:   | FileCheck -check-prefix=NO-RESOURCE-INC %s
+// NO-RESOURCE-INC-NOT: "-internal-isystem" 
"{{.*}}/Inputs/resource_dir/include"
+// NO-RESOURCE-INC: "-internal-isystem" 
"{{.*}}/basic_riscv32_tree/{{.*}}/riscv32-unknown-elf/include"
+
 // RUN: %clang -target riscv32 %s -emit-llvm -S -o - | FileCheck %s
 
 typedef __builtin_va_list va_list;
Index: clang/lib/Driver/ToolChains/RISCVToolchain.cpp
===
--- clang/lib/Driver/ToolChains/RISCVToolchain.cpp
+++ clang/lib/Driver/ToolChains/RISCVToolchain.cpp
@@ -98,6 +98,12 @@
   if (DriverArgs.hasArg(options::OPT_nostdinc))
 return;
 
+  if (!DriverArgs.hasArg(options::OPT_nobuiltininc)) {
+SmallString<128> Dir(getDriver().ResourceDir);
+llvm::sys::path::append(Dir, "include");
+addSystemInclude(DriverArgs, CC1Args, Dir.str());
+  }
+
   if (!DriverArgs.hasArg(options::OPT_nostdlibinc)) {
 SmallString<128> Dir(computeSysRoot());
 llvm::sys::path::append(Dir, "include");


Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -153,6 +153,20 @@
 // C-RV64-RTLIB-COMPILERRT-LP64: "--start-group" "-lc" "-lgloss" "--end-group" "{{.*}}libclang_rt.builtins-riscv64.a"
 // C-RV64-RTLIB-COMPILERRT-LP64: "{{.*}}clang_rt.crtend-riscv64.o"
 
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree \
+// RUN:   -resource-dir=%s/Inputs/resource_dir 2>&1 \
+// RUN:   | FileCheck -check-prefix=RESOURCE-INC %s
+// RESOURCE-INC: "-internal-isystem" "{{.*}}/Inputs/resource_dir/include"
+// RESOURCE-INC: "-internal-isystem" "{{.*}}/basic_riscv64_tree/{{.*}}/riscv64-unknown-elf/include"
+
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree \
+// RUN:   -resource-dir=%s/Inputs/re

[clang] 079d136 - [RISCV] Fix the include search path order between sysroot and resource folder

[Kito Cheng via cfe-commits]

Author: Kito Cheng
Date: 2022-02-21T14:06:47+08:00
New Revision: 079d13668bf1b7f929f1897af90f64caae41c81d

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

LOG: [RISCV] Fix the include search path order between sysroot and resource 
folder

Resource folder[1] should include before sysroot[2] in general (Linux clang
toolchain, BareMetal clang toolchain, and GCC using that order), and that
prevent sysroot's header file override resource folder's one, this change is
reference from BareMetal::addclangsystemincludea...@baremetal.cpp[3].

And also fix the behavior of `-nobuiltininc`.

[1] Include path from resource folder is something like this: 
`/lib/clang/13.0.0/include/`
[2] Include path from sysroot is something like this: 
`/riscv32-unknown-elf/include`
[3] 
https://github.com/llvm/llvm-project/blob/llvmorg-13.0.1/clang/lib/Driver/ToolChains/BareMetal.cpp#L193

Reviewed By: asb

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

Added: 
clang/test/Driver/Inputs/resource_dir/include/.keep

Modified: 
clang/lib/Driver/ToolChains/RISCVToolchain.cpp
clang/test/Driver/riscv32-toolchain.c
clang/test/Driver/riscv64-toolchain.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/RISCVToolchain.cpp 
b/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
index 714325a2db39e..a63ada0cbb7e4 100644
--- a/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
+++ b/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
@@ -98,6 +98,12 @@ void RISCVToolChain::AddClangSystemIncludeArgs(const ArgList 
&DriverArgs,
   if (DriverArgs.hasArg(options::OPT_nostdinc))
 return;
 
+  if (!DriverArgs.hasArg(options::OPT_nobuiltininc)) {
+SmallString<128> Dir(getDriver().ResourceDir);
+llvm::sys::path::append(Dir, "include");
+addSystemInclude(DriverArgs, CC1Args, Dir.str());
+  }
+
   if (!DriverArgs.hasArg(options::OPT_nostdlibinc)) {
 SmallString<128> Dir(computeSysRoot());
 llvm::sys::path::append(Dir, "include");

diff  --git a/clang/test/Driver/Inputs/resource_dir/include/.keep 
b/clang/test/Driver/Inputs/resource_dir/include/.keep
new file mode 100644
index 0..e69de29bb2d1d

diff  --git a/clang/test/Driver/riscv32-toolchain.c 
b/clang/test/Driver/riscv32-toolchain.c
index fb262a19a0439..c480a7c00a367 100644
--- a/clang/test/Driver/riscv32-toolchain.c
+++ b/clang/test/Driver/riscv32-toolchain.c
@@ -197,6 +197,20 @@
 // C-RV32-RTLIB-COMPILERRT-ILP32: "--start-group" "-lc" "-lgloss" 
"--end-group" "{{.*}}libclang_rt.builtins-riscv32.a"
 // C-RV32-RTLIB-COMPILERRT-ILP32: "{{.*}}clang_rt.crtend-riscv32.o"
 
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree \
+// RUN:   -resource-dir=%s/Inputs/resource_dir 2>&1 \
+// RUN:   | FileCheck -check-prefix=RESOURCE-INC %s
+// RESOURCE-INC: "-internal-isystem" "{{.*}}/Inputs/resource_dir/include"
+// RESOURCE-INC: "-internal-isystem" 
"{{.*}}/basic_riscv32_tree/{{.*}}/riscv32-unknown-elf/include"
+
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree \
+// RUN:   -resource-dir=%s/Inputs/resource_dir -nobuiltininc 2>&1 \
+// RUN:   | FileCheck -check-prefix=NO-RESOURCE-INC %s
+// NO-RESOURCE-INC-NOT: "-internal-isystem" 
"{{.*}}/Inputs/resource_dir/include"
+// NO-RESOURCE-INC: "-internal-isystem" 
"{{.*}}/basic_riscv32_tree/{{.*}}/riscv32-unknown-elf/include"
+
 // RUN: %clang -target riscv32 %s -emit-llvm -S -o - | FileCheck %s
 
 typedef __builtin_va_list va_list;

diff  --git a/clang/test/Driver/riscv64-toolchain.c 
b/clang/test/Driver/riscv64-toolchain.c
index 2774e004854c3..91358298ecdd8 100644
--- a/clang/test/Driver/riscv64-toolchain.c
+++ b/clang/test/Driver/riscv64-toolchain.c
@@ -153,6 +153,20 @@
 // C-RV64-RTLIB-COMPILERRT-LP64: "--start-group" "-lc" "-lgloss" "--end-group" 
"{{.*}}libclang_rt.builtins-riscv64.a"
 // C-RV64-RTLIB-COMPILERRT-LP64: "{{.*}}clang_rt.crtend-riscv64.o"
 
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree \
+// RUN:   -resource-dir=%s/Inputs/resource_dir 2>&1 \
+// RUN:   | FileCheck -check-prefix=RESOURCE-INC %s
+// RESOURCE-INC: "-internal-isystem" "{{.*}}/Inputs/resource_dir/include"
+// RESOURCE-INC: "-internal-isystem" 
"{{.*}}/basic_riscv64_tree/{{.*}}/riscv64-unknown-elf/include"
+
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree \
+// RUN:   -resource-dir=%s/Inputs/resource_dir -nobuiltininc 2>&1 \
+// RUN:   | FileCheck -check-prefix=NO-RESOURCE-INC %s
+// NO-RESOURCE-INC-NOT: "-internal-isystem" "{{.*}}Inputs/resource_dir/include"
+// NO-RESOURCE-INC: "-internal-isystem" 
"{{.*}}/basic_riscv64_tree/{{.*}}/

[PATCH] D120217: [clang-format] Add an option to insert braces after control statements

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 410216.
owenpan added a comment.

Rebased and addressed review comments.


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

https://reviews.llvm.org/D120217

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -19474,6 +19474,7 @@
   CHECK_PARSE_BOOL_FIELD(IndentRequiresClause, "IndentRequires");
   CHECK_PARSE_BOOL(IndentRequiresClause);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
+  CHECK_PARSE_BOOL(InsertBraces);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
   CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);
@@ -24300,6 +24301,200 @@
   verifyFormat("template  struct Foo {};", Style);
 }
 
+TEST_F(FormatTest, InsertBraces) {
+  FormatStyle Style = getLLVMStyle();
+  Style.InsertBraces = true;
+
+  verifyFormat("// clang-format off\n"
+   "if (a) f();\n"
+   "// clang-format on\n"
+   "if (b) {\n"
+   "  g();\n"
+   "}",
+   "// clang-format off\n"
+   "if (a) f();\n"
+   "// clang-format on\n"
+   "if (b) g();",
+   Style);
+
+  verifyFormat("if (a) {\n"
+   "  switch (b) {\n"
+   "  case 1:\n"
+   "c = 0;\n"
+   "break;\n"
+   "  default:\n"
+   "c = 1;\n"
+   "  }\n"
+   "}",
+   "if (a)\n"
+   "  switch (b) {\n"
+   "  case 1:\n"
+   "c = 0;\n"
+   "break;\n"
+   "  default:\n"
+   "c = 1;\n"
+   "  }",
+   Style);
+
+  verifyFormat("for (auto node : nodes) {\n"
+   "  if (node) {\n"
+   "break;\n"
+   "  }\n"
+   "}",
+   "for (auto node : nodes)\n"
+   "  if (node)\n"
+   "break;",
+   Style);
+
+  verifyFormat("for (auto node : nodes) {\n"
+   "  if (node)\n"
+   "}",
+   "for (auto node : nodes)\n"
+   "  if (node)",
+   Style);
+
+  verifyFormat("do {\n"
+   "  --a;\n"
+   "} while (a);",
+   "do\n"
+   "  --a;\n"
+   "while (a);",
+   Style);
+
+  verifyFormat("if (i) {\n"
+   "  ++i;\n"
+   "} else {\n"
+   "  --i;\n"
+   "}",
+   "if (i)\n"
+   "  ++i;\n"
+   "else {\n"
+   "  --i;\n"
+   "}",
+   Style);
+
+  verifyFormat("void f() {\n"
+   "  while (j--) {\n"
+   "while (i) {\n"
+   "  --i;\n"
+   "}\n"
+   "  }\n"
+   "}",
+   "void f() {\n"
+   "  while (j--)\n"
+   "while (i)\n"
+   "  --i;\n"
+   "}",
+   Style);
+
+  verifyFormat("f({\n"
+   "  if (a) {\n"
+   "g();\n"
+   "  }\n"
+   "});",
+   "f({\n"
+   "  if (a)\n"
+   "g();\n"
+   "});",
+   Style);
+
+  verifyFormat("if (a) {\n"
+   "  f();\n"
+   "} else if (b) {\n"
+   "  g();\n"
+   "} else {\n"
+   "  h();\n"
+   "}",
+   "if (a)\n"
+   "  f();\n"
+   "else if (b)\n"
+   "  g();\n"
+   "else\n"
+   "  h();",
+   Style);
+
+  verifyFormat("if (a) {\n"
+   "  f();\n"
+   "}\n"
+   "// comment\n"
+   "/* comment */",
+   "if (a)\n"
+   "  f();\n"
+   "// comment\n"
+   "/* comment */",
+   Style);
+
+  verifyFormat("if (a) {\n"
+   "  // foo\n"
+   "  // bar\n"
+   "  f();\n"
+   "}",
+   "if (a)\n"
+   "  // foo\n"
+   "  // bar\n"
+   "  f();",
+   Style);
+
+  verifyFormat("if (a) { // comment\n"
+   "  // comment\n"
+   "  f();\n"
+   "}",
+   "if (a) // comment\n"
+   "  // comment\n"
+   "  f();",
+   Style);
+
+  verifyFormat("if (a) {\n"
+  

[PATCH] D120217: [clang-format] Add an option to insert braces after control statements

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2303
+static FormatToken *getLastNonComment(const UnwrappedLine &Line) {
+  for (const auto &Token : llvm::reverse(Line.Tokens))
+if (Token.Tok->isNot(tok::comment))

HazardyKnusperkeks wrote:
> Get the last token, and if it's a comment `getPreviousNonComment()`?
Almost. I tried to use `getPreviousNonComment()` but sometimes got `nullptr` 
when a nonnull was always expected (line 2316).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120217

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

[PATCH] D120228: [RISCV] Add policy operand for masked compare and vmsbf/vmsif/vmsof IR intrinsics.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: llvm/test/CodeGen/RISCV/rvv/vmsif.ll:34
 ; CHECK-NEXT:vmv1r.v v10, v0
-; CHECK-NEXT:vsetvli zero, a0, e8, mf8, tu, mu
+; CHECK-NEXT:vsetvli zero, a0, e8, mf8, ta, mu
 ; CHECK-NEXT:vmv1r.v v0, v9

craig.topper wrote:
> Should vmsif, vmsof, and vmsbf have ForceTailAgnostic like vmand, vmxor, etc.?
Nevermind. Ignore this comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120228

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

[PATCH] D120228: [RISCV] Add policy operand for masked compare and vmsbf/vmsif/vmsof IR intrinsics.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: llvm/test/CodeGen/RISCV/rvv/vmsif.ll:34
 ; CHECK-NEXT:vmv1r.v v10, v0
-; CHECK-NEXT:vsetvli zero, a0, e8, mf8, tu, mu
+; CHECK-NEXT:vsetvli zero, a0, e8, mf8, ta, mu
 ; CHECK-NEXT:vmv1r.v v0, v9

Should vmsif, vmsof, and vmsbf have ForceTailAgnostic like vmand, vmxor, etc.?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120228

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

[PATCH] D120228: [RISCV] Add policy operand for masked compare and vmsbf/vmsif/vmsof IR intrinsics.

[Zakk Chen via Phabricator via cfe-commits]

khchen created this revision.
khchen added reviewers: craig.topper, rogfer01, frasercrmck, kito-cheng, 
arcbbb, monkchiang, eopXD.
Herald added subscribers: VincentWu, luke957, achieveartificialintelligence, 
vkmr, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, 
psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, edward-jones, zzheng, 
jrtc27, niosHD, sabuasal, simoncook, johnrusso, rbar, asb, hiraditya.
khchen requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, pcwang-thead, MaskRay.
Herald added projects: clang, LLVM.

Those operations are updated under a tail agnostic policy, but they
could have mask agnostic or undisturbed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120228

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmsbf.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmsif.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmsof.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vmsbf.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vmsif.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vmsof.c
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
  llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
  llvm/test/CodeGen/RISCV/rvv/masked-tama.ll
  llvm/test/CodeGen/RISCV/rvv/masked-tamu.ll
  llvm/test/CodeGen/RISCV/rvv/masked-tuma.ll
  llvm/test/CodeGen/RISCV/rvv/masked-tumu.ll
  llvm/test/CodeGen/RISCV/rvv/vmfeq-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmfeq-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmfge-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmfge-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmfgt-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmfgt-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmfle-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmfle-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmflt-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmflt-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmfne-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmfne-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmsbf-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmsbf-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmseq-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmseq-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmsge-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmsge-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmsgeu-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmsgeu-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmsgt-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmsgt-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmsgtu-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmsgtu-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmsif.ll
  llvm/test/CodeGen/RISCV/rvv/vmsle-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmsle-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmsleu-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmsleu-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmslt-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmslt-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmsltu-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmsltu-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmsne-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmsne-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmsof.ll

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

[PATCH] D120227: [RISCV] Add policy operand for masked vid and viota IR intrinsics.

[Zakk Chen via Phabricator via cfe-commits]

khchen created this revision.
khchen added reviewers: craig.topper, rogfer01, frasercrmck, kito-cheng, 
arcbbb, monkchiang, eopXD.
Herald added subscribers: VincentWu, luke957, achieveartificialintelligence, 
vkmr, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, 
psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, edward-jones, zzheng, 
jrtc27, niosHD, sabuasal, simoncook, johnrusso, rbar, asb, hiraditya.
khchen requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, pcwang-thead, MaskRay.
Herald added projects: clang, LLVM.

Those masked operations are missed the policy operand.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120227

Files:
  clang/include/clang/Basic/riscv_vector.td
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
  llvm/test/CodeGen/RISCV/rvv/masked-tama.ll
  llvm/test/CodeGen/RISCV/rvv/masked-tamu.ll
  llvm/test/CodeGen/RISCV/rvv/masked-tuma.ll
  llvm/test/CodeGen/RISCV/rvv/masked-tumu.ll
  llvm/test/CodeGen/RISCV/rvv/vid.ll
  llvm/test/CodeGen/RISCV/rvv/viota.ll

Index: llvm/test/CodeGen/RISCV/rvv/viota.ll
===
--- llvm/test/CodeGen/RISCV/rvv/viota.ll
+++ llvm/test/CodeGen/RISCV/rvv/viota.ll
@@ -27,7 +27,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv1i8_nxv1i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv1i8_nxv1i1:
@@ -40,7 +40,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -69,7 +69,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv2i8_nxv2i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv2i8_nxv2i1:
@@ -82,7 +82,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -111,7 +111,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv4i8_nxv4i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv4i8_nxv4i1:
@@ -124,7 +124,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -153,7 +153,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv8i8_nxv8i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv8i8_nxv8i1:
@@ -166,7 +166,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -195,7 +195,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv16i8_nxv16i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv16i8_nxv16i1:
@@ -208,7 +208,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -237,7 +237,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv32i8_nxv32i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv32i8_nxv32i1:
@@ -250,7 +250,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -279,7 +279,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv64i8_nxv64i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv64i8_nxv64i1:
@@ -292,7 +292,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -321,7 +321,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv1i16_nxv1i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv1i16_nxv1i1:
@@ -334,7 +334,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -363,7 +363,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv2i16_nxv2i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv2i16_nxv2i1:
@@ -376,7 +376,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -405,7 +405,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv4i16_nxv4i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv4i16_nxv4i1:
@@ -418,7 +418,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -447,7 +447,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv8i16_nxv8i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv8i16_nxv8i1:
@@ -460,7 +460,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -489,7 +489,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv16i16_nxv16i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv16i16_nxv16i1:
@@ -502,7 +502,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -531,7 +531,7 @@
   ,
   ,
 

[PATCH] D119409: [C++20] [Modules] Remain dynamic initializing internal-linkage variables in module interface unit

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D119409#3332313 , @dblaikie wrote:

> (maybe relevant: For what it's worth: I originally implemented inline 
> function homing in modules codegen for Clang Header Modules - the results I 
> got for object file size in an -O0 build were marginal - a /slight/ win in 
> object file size, but not as much as we might've expected. Part of the reason 
> might be that there can be inline functions that are never called, or at 
> higher optimization levels, inline functions that always get inlined (via 
> "available externally" definitions) - in that case, providing "homed" 
> definitions creates inline function definitions that are unused during 
> linking/a waste of space. It's possible the workload I was dealing with 
> (common Google internal programs) skewed compared to broader C++ code - for 
> instance heavy use of protobufs could be leading to a lot of generated 
> code/inline functions that are mostly unused. I didn't iterate further to 
> tweak/implement heuristics about which inline functions should be homed. I'm 
> not sure if Richard Smith made a choice about not homing inline functions in 
> C++20 modules because of these results, or for other reasons, or just as a 
> consequence of the implementation - but given we had the logic in Clang to do 
> inline function homing for Clang Header Modules, I'm guessing it was an 
> intentional choice to not use that functionality in C++20 modules when they 
> have to have an object file anyway)

Thanks for sharing this. I didn't consider code size before. I agree the result 
should depends on the pattern of the program. I guess the code size may 
increase or decrease between different projects.

> Richard and I discussed taking advantage of this kind of new home location, 
> certainly for key-less polymorphic classes. I was against it as it was more 
> work :) Blame me.

From my experience, it depends on how well we want to implement. A plain 
implementation is relatively easy. It would speed up the compilation 
significantly in **O0**. But it doesn't work well with optimization turned on, 
since we need to do optimization and we must import all the function  by 
`available_externally` to enable a complete optimization. In this case (with 
optimization), we could only save the time for Preprocessor, Parser, Semantic 
analysis and backend. But the big part of compilation takes on the middle end 
and we need to pay for the serialization and deserialization. My experiment 
shows that we could only get 5% improvement on compilation time with named 
module in optimization turned on. (We could offer an option to make it compile 
fast at On by not emitting functions in other module unit, but it would hurt 
the performance obviously).

A good implementation may attach optimized IR to the PCM files. Since the 
standard talks nothing about CMI/BMI (PCM), we are free to compile it during 
the middle end and attach the optimized IR to PCM files. And when we imports 
the optimized PCM, we could extract the optimized function on need. We could 
mark such functions with a special attribute (like 
'optimized_available_externally'?) to tell the compiler not optimize it and 
delete it after middle end optimization gets done. Such functions is only 
available for inlining (or any other IPO). I think this wouldn't hurt 
performance and we could get a win for the compilation speed. But I agree this 
is not easy to implement.


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

https://reviews.llvm.org/D119409

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

[PATCH] D120225: [Clang][Sema] Check unexpected else statement in cond-update-stmt

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 created this revision.
tianshilei1992 added reviewers: jdoerfert, ABataev.
tianshilei1992 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

In 'cond-update-stmt', `else` statement is not expected. This patch adds
the check in Sema.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120225

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_messages.c


Index: clang/test/OpenMP/atomic_messages.c
===
--- clang/test/OpenMP/atomic_messages.c
+++ clang/test/OpenMP/atomic_messages.c
@@ -473,6 +473,15 @@
   x = e;
 d = e;
   }
+// omp51-error@+7 {{the statement for 'atomic compare' must be a compound 
statement of form '{x = expr ordop x ? expr : x;}', '{x = x ordop expr? expr : 
x;}', '{x = x == e ? d : x;}', '{x = e == x ? d : x;}', or 'if(expr ordop x) {x 
= expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', 'if(e == x) 
{x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 
'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+6 {{unexpected 'else' statement}}
+#pragma omp atomic compare
+  {
+if (x > e)
+  x = e;
+else
+  d = e;
+  }
   float fx = 0.0f;
   float fd = 0.0f;
   float fe = 0.0f;
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -10974,6 +10974,8 @@
 NotScalar,
 /// Not an integer.
 NotInteger,
+/// 'else' statement is not expected.
+UnexpectedElse,
 /// No error.
 NoError,
   };
@@ -1,6 +3,13 @@
 return false;
   }
 
+  if (S->getElse()) {
+ErrorInfo.Error = ErrorTy::UnexpectedElse;
+ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = S->getElse()->getBeginLoc();
+ErrorInfo.ErrorRange = ErrorInfo.NoteRange = 
S->getElse()->getSourceRange();
+return false;
+  }
+
   return true;
 }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10526,7 +10526,7 @@
 def note_omp_atomic_compare: Note<
   "%select{expected compound statement|expected exactly one expression 
statement|expected assignment statement|expected conditional operator|expect 
result value to be at false expression|"
   "expect binary operator in conditional expression|expect '<', '>' or '==' as 
order operator|expect comparison in a form of 'x == e', 'e == x', 'x ordop 
expr', or 'expr ordop x'|"
-  "expect lvalue for result value|expect scalar value|expect integer value}0">;
+  "expect lvalue for result value|expect scalar value|expect integer 
value|unexpected 'else' statement}0">;
 def err_omp_atomic_several_clauses : Error<
   "directive '#pragma omp atomic' cannot contain more than one 'read', 
'write', 'update', 'capture', or 'compare' clause">;
 def err_omp_several_mem_order_clauses : Error<


Index: clang/test/OpenMP/atomic_messages.c
===
--- clang/test/OpenMP/atomic_messages.c
+++ clang/test/OpenMP/atomic_messages.c
@@ -473,6 +473,15 @@
   x = e;
 d = e;
   }
+// omp51-error@+7 {{the statement for 'atomic compare' must be a compound statement of form '{x = expr ordop x ? expr : x;}', '{x = x ordop expr? expr : x;}', '{x = x == e ? d : x;}', '{x = e == x ? d : x;}', or 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+6 {{unexpected 'else' statement}}
+#pragma omp atomic compare
+  {
+if (x > e)
+  x = e;
+else
+  d = e;
+  }
   float fx = 0.0f;
   float fd = 0.0f;
   float fe = 0.0f;
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -10974,6 +10974,8 @@
 NotScalar,
 /// Not an integer.
 NotInteger,
+/// 'else' statement is not expected.
+UnexpectedElse,
 /// No error.
 NoError,
   };
@@ -1,6 +3,13 @@
 return false;
   }
 
+  if (S->getElse()) {
+ErrorInfo.Error = ErrorTy::UnexpectedElse;
+ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = S->getElse()->getBeginLoc();
+ErrorInfo.ErrorRange = ErrorInfo.NoteRange = S->getElse()->getSourceRange();
+return false;
+  }
+
   return true;
 }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticS

[PATCH] D120200: [Clang][OpenMP] Add Sema support for atomic compare capture

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 410203.
tianshilei1992 added a comment.

set `C` accordingly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120200

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_messages.c

Index: clang/test/OpenMP/atomic_messages.c
===
--- clang/test/OpenMP/atomic_messages.c
+++ clang/test/OpenMP/atomic_messages.c
@@ -491,4 +491,199 @@
   fx = fe;
   }
 }
+
+void compare_capture(void) {
+  int x = 0;
+  int d = 0;
+  int e = 0;
+  int v = 0;
+  int r = 0;
+// omp51-error@+3 {{the statement for 'atomic compare capture' must be a compound statement of form '{v = x; cond-up-stmt}', ''{cond-up-stmt v = x;}', '{if(x == e) {x = d;} else {v = x;}}', '{r = x == e; if(r) {x = d;}}', or '{r = x == e; if(r) {x = d;} else {v = x;}}', where 'cond-update-stmt' can have one of the following forms: 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', or 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+2 {{expected compound statement}}
+#pragma omp atomic compare capture
+  if (x == e) {}
+// omp51-error@+3 {{the statement for 'atomic compare capture' must be a compound statement of form '{v = x; cond-up-stmt}', ''{cond-up-stmt v = x;}', '{if(x == e) {x = d;} else {v = x;}}', '{r = x == e; if(r) {x = d;}}', or '{r = x == e; if(r) {x = d;} else {v = x;}}', where 'cond-update-stmt' can have one of the following forms: 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', or 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+2 {{expected exactly one expression statement}}
+#pragma omp atomic compare capture
+  if (x == e) {
+x = d;
+v = x;
+  }
+// omp51-error@+4 {{the statement for 'atomic compare capture' must be a compound statement of form '{v = x; cond-up-stmt}', ''{cond-up-stmt v = x;}', '{if(x == e) {x = d;} else {v = x;}}', '{r = x == e; if(r) {x = d;}}', or '{r = x == e; if(r) {x = d;} else {v = x;}}', where 'cond-update-stmt' can have one of the following forms: 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', or 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+3 {{expected assignment statement}}
+#pragma omp atomic compare capture
+  if (x == e) {
+bbar();
+  }
+// omp51-error@+4 {{the statement for 'atomic compare capture' must be a compound statement of form '{v = x; cond-up-stmt}', ''{cond-up-stmt v = x;}', '{if(x == e) {x = d;} else {v = x;}}', '{r = x == e; if(r) {x = d;}}', or '{r = x == e; if(r) {x = d;} else {v = x;}}', where 'cond-update-stmt' can have one of the following forms: 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', or 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+3 {{expected assignment statement}}
+#pragma omp atomic compare capture
+  if (x == e) {
+x += d;
+  }
+// omp51-error@+3 {{the statement for 'atomic compare capture' must be a compound statement of form '{v = x; cond-up-stmt}', ''{cond-up-stmt v = x;}', '{if(x == e) {x = d;} else {v = x;}}', '{r = x == e; if(r) {x = d;}}', or '{r = x == e; if(r) {x = d;} else {v = x;}}', where 'cond-update-stmt' can have one of the following forms: 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', or 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+2 {{expect binary operator in conditional expression}}
+#pragma omp atomic compare capture
+  if (ffoo()) {
+x = d;
+  }
+// omp51-error@+3 {{the statement for 'atomic compare capture' must be a compound statement of form '{v = x; cond-up-stmt}', ''{cond-up-stmt v = x;}', '{if(x == e) {x = d;} else {v = x;}}', '{r = x == e; if(r) {x = d;}}', or '{r = x == e; if(r) {x = d;} else {v = x;}}', where 'cond-update-stmt' can have one of the following forms: 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', or 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+2 {{expect '==' operator}}
+#pragma omp atomic compare capture
+  if (x > e) {
+x = d;
+  }

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu accepted this revision.
ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/SemaModule.cpp:177
+  if (IsPartition) {
+ModuleName += ":";
+ModuleName += stringFromPath(Partition);

urnathan wrote:
> iains wrote:
> > ChuanqiXu wrote:
> > > iains wrote:
> > > > urnathan wrote:
> > > > > iains wrote:
> > > > > > ChuanqiXu wrote:
> > > > > > > I chose '-' in my implementation since here ModuleName refers to 
> > > > > > > the name in filesystem, right? And I remember '-' is the choice 
> > > > > > > of GCC. So let's keep consistency.
> > > > > > This is not my understanding.
> > > > > > 
> > > > > > ModuleName is the "user-facing" name of the module that is 
> > > > > > described in the source, and we use this in diagnostics etc.
> > > > > > 
> > > > > > The translation of that name to an on-disk name can be arbitrary 
> > > > > > and there are several mechanisms in clang already (e.g. 
> > > > > > -fmodule-file=A:B=foo.pcm) the module loader uses these to find the 
> > > > > > module on the basis of its user-facing name where required.
> > > > > > 
> > > > > > When we have P1184, it is even more important that the interface is 
> > > > > > supplied with the name that the user will put into the source code.
> > > > > > 
> > > > > > 
> > > > > I agree with Iain, we should use ':' is module names here.  When 
> > > > > mapping a named module to the file system some translation is needed 
> > > > > because ':' is not permitted in file names on some filesystems (you 
> > > > > know the ones!)
> > > > (just to expand a little more)
> > > > 
> > > > the on-disk name needs to be chosen suitably for the platform and by 
> > > > the user and/or the build system.
> > > > 
> > > > When the FE chooses a default filename (which is done in building jobs, 
> > > > not in the Parser of course) it chooses one based on the source 
> > > > filename.  It would be most unfortunate if the Parser/Sema needed to 
> > > > understand platform filename rules.
> > > > 
> > > > When you do  'clang -module-file-info' (with the existing or updated 
> > > > version) you should see the module name as per the source code, the 
> > > > translation would only apply to the filename itself.
> > > > 
> > > > - to answer a later comment:
> > > > 
> > > > when we do -fmodule-file=A_B.pcm  and A_B.pcm contains a module named 
> > > > A:B the loader notices this pairing when it pre-loads the module, so 
> > > > that when we ask for "A:B" the loader already knows which on-disk file 
> > > > contains. it.
> > > > 
> > > > if we use the HeaderSearch mechanisms (when we want to figure out a 
> > > > module-name<=> filename pairing without loading the module) we can use 
> > > > -fmodule-name=A:B=A_B.pcm,
> > > > 
> > > > These mechanisms work today, but P1184 is a more flexible mechanism and 
> > > > avoids having  massive command lines with many -fmodule-file= cases.
> > > > 
> > > But what if we need to import multiple modules? In our current workflow, 
> > > we would like to compile importing module in following style:
> > > ```
> > > clang++ -std=c++20 -fprebuilt-module-path=path1 
> > > -fprebuilt-module-path=path2 ... unit.cpp(m) ...
> > > ```
> > > 
> > > And unit.cpp(m) may look like:
> > > ```
> > > export module M;
> > > import :parta;
> > > import :partb;
> > > import :partc;
> > > ```
> > > 
> > > And our compiler would lookup `M-parta.pcm`, `M-partb.pcm` and 
> > > `M-partc.pcm` in the path given in the command line. However, in current 
> > > implementation, it would lookup `M:parta.pcm`, `M:partb.pcm` and 
> > > `M:partc.pcm` in the path but it might fail. So my point here is that the 
> > > current implementation doesn't work well with `fprebuilt-module-path`. 
> > > And I don't think we should give up `fprebuilt-module-path` to support 
> > > multiple `fmodule-file`. The `fprebuilt-module-path` is easier to 
> > > understand and use for real projects. Even if we support multiple 
> > > `-fmodule-file`, people might be frustrating to add many `fmodule-file` 
> > > if they need to import many units.
> > > 
> > > So I really insist on this. Or at least let's add one option, something 
> > > like `-fmodule-partition-separator` here.
> > The semantics of clang hierarchical module names are different from C++20 
> > module names.
> > 
> > C++20 does not specify any relationship between the module name and the 
> > filename - that task is for the user, build system (or in the case we have 
> > with clang where it can integrate some of that functionality, the lookup 
> > mechanism).
> > 
> > out of interest if the user puts:
> > 
> > ```
> > export module A.B:C.D;
> > ```
> > 
> > how do you represent that on disk in your implementation?
> > 
> > 
> > 
> > In my understanding, if the user does:
> > 
> > ```
> > export module A.B:C.D;
> > 
> > clang -cc1 -std=c++20 -emit-module-interface foo.cpp -o xyzzy.pcm
> > ```
> > then we should see :
> > 
> > ```
> > clang -module-file-info xyzz

[PATCH] D120200: [Clang][OpenMP] Add Sema support for atomic compare capture

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 410200.
tianshilei1992 added a comment.

fix error message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120200

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_messages.c

Index: clang/test/OpenMP/atomic_messages.c
===
--- clang/test/OpenMP/atomic_messages.c
+++ clang/test/OpenMP/atomic_messages.c
@@ -491,4 +491,199 @@
   fx = fe;
   }
 }
+
+void compare_capture(void) {
+  int x = 0;
+  int d = 0;
+  int e = 0;
+  int v = 0;
+  int r = 0;
+// omp51-error@+3 {{the statement for 'atomic compare capture' must be a compound statement of form '{v = x; cond-up-stmt}', ''{cond-up-stmt v = x;}', '{if(x == e) {x = d;} else {v = x;}}', '{r = x == e; if(r) {x = d;}}', or '{r = x == e; if(r) {x = d;} else {v = x;}}', where 'cond-update-stmt' can have one of the following forms: 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', or 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+2 {{expected compound statement}}
+#pragma omp atomic compare capture
+  if (x == e) {}
+// omp51-error@+3 {{the statement for 'atomic compare capture' must be a compound statement of form '{v = x; cond-up-stmt}', ''{cond-up-stmt v = x;}', '{if(x == e) {x = d;} else {v = x;}}', '{r = x == e; if(r) {x = d;}}', or '{r = x == e; if(r) {x = d;} else {v = x;}}', where 'cond-update-stmt' can have one of the following forms: 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', or 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+2 {{expected exactly one expression statement}}
+#pragma omp atomic compare capture
+  if (x == e) {
+x = d;
+v = x;
+  }
+// omp51-error@+4 {{the statement for 'atomic compare capture' must be a compound statement of form '{v = x; cond-up-stmt}', ''{cond-up-stmt v = x;}', '{if(x == e) {x = d;} else {v = x;}}', '{r = x == e; if(r) {x = d;}}', or '{r = x == e; if(r) {x = d;} else {v = x;}}', where 'cond-update-stmt' can have one of the following forms: 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', or 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+3 {{expected assignment statement}}
+#pragma omp atomic compare capture
+  if (x == e) {
+bbar();
+  }
+// omp51-error@+4 {{the statement for 'atomic compare capture' must be a compound statement of form '{v = x; cond-up-stmt}', ''{cond-up-stmt v = x;}', '{if(x == e) {x = d;} else {v = x;}}', '{r = x == e; if(r) {x = d;}}', or '{r = x == e; if(r) {x = d;} else {v = x;}}', where 'cond-update-stmt' can have one of the following forms: 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', or 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+3 {{expected assignment statement}}
+#pragma omp atomic compare capture
+  if (x == e) {
+x += d;
+  }
+// omp51-error@+3 {{the statement for 'atomic compare capture' must be a compound statement of form '{v = x; cond-up-stmt}', ''{cond-up-stmt v = x;}', '{if(x == e) {x = d;} else {v = x;}}', '{r = x == e; if(r) {x = d;}}', or '{r = x == e; if(r) {x = d;} else {v = x;}}', where 'cond-update-stmt' can have one of the following forms: 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', or 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+2 {{expect binary operator in conditional expression}}
+#pragma omp atomic compare capture
+  if (ffoo()) {
+x = d;
+  }
+// omp51-error@+3 {{the statement for 'atomic compare capture' must be a compound statement of form '{v = x; cond-up-stmt}', ''{cond-up-stmt v = x;}', '{if(x == e) {x = d;} else {v = x;}}', '{r = x == e; if(r) {x = d;}}', or '{r = x == e; if(r) {x = d;} else {v = x;}}', where 'cond-update-stmt' can have one of the following forms: 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', or 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+2 {{expect '==' operator}}
+#pragma omp atomic compare capture
+  if (x > e) {
+x = d;
+  }
+/

[PATCH] D120220: [clang-format][NFC] Fix typos and inconsistencies

[Owen Pan 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 rGaacc110bdce7: [clang-format][NFC] Fix typos and 
inconsistencies (authored by kuzkry, committed by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120220

Files:
  clang/lib/Format/Format.cpp


Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -990,11 +990,11 @@
   case ParseError::InvalidQualifierSpecified:
 return "Invalid qualifier specified in QualifierOrder";
   case ParseError::DuplicateQualifierSpecified:
-return "Duplicate qualifier specified in QualfierOrder";
+return "Duplicate qualifier specified in QualifierOrder";
   case ParseError::MissingQualifierType:
-return "Missing type in QualfierOrder";
+return "Missing type in QualifierOrder";
   case ParseError::MissingQualifierOrder:
-return "Missing QualfierOrder";
+return "Missing QualifierOrder";
   }
   llvm_unreachable("unexpected parse error");
 }
@@ -1650,7 +1650,8 @@
 if (token == tok::identifier)
   return ParseError::InvalidQualifierSpecified;
   }
-  // Ensure the list is unqiue (no duplicates).
+
+  // Ensure the list is unique (no duplicates).
   std::set UniqueQualifiers(Style->QualifierOrder.begin(),
  Style->QualifierOrder.end());
   if (Style->QualifierOrder.size() != UniqueQualifiers.size()) {
@@ -1660,10 +1661,12 @@
 return ParseError::DuplicateQualifierSpecified;
   }
 
+  // Ensure the list has 'type' in it
   auto type = std::find(Style->QualifierOrder.begin(),
 Style->QualifierOrder.end(), "type");
   if (type == Style->QualifierOrder.end())
 return ParseError::MissingQualifierType;
+
   return ParseError::Success;
 }
 


Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -990,11 +990,11 @@
   case ParseError::InvalidQualifierSpecified:
 return "Invalid qualifier specified in QualifierOrder";
   case ParseError::DuplicateQualifierSpecified:
-return "Duplicate qualifier specified in QualfierOrder";
+return "Duplicate qualifier specified in QualifierOrder";
   case ParseError::MissingQualifierType:
-return "Missing type in QualfierOrder";
+return "Missing type in QualifierOrder";
   case ParseError::MissingQualifierOrder:
-return "Missing QualfierOrder";
+return "Missing QualifierOrder";
   }
   llvm_unreachable("unexpected parse error");
 }
@@ -1650,7 +1650,8 @@
 if (token == tok::identifier)
   return ParseError::InvalidQualifierSpecified;
   }
-  // Ensure the list is unqiue (no duplicates).
+
+  // Ensure the list is unique (no duplicates).
   std::set UniqueQualifiers(Style->QualifierOrder.begin(),
  Style->QualifierOrder.end());
   if (Style->QualifierOrder.size() != UniqueQualifiers.size()) {
@@ -1660,10 +1661,12 @@
 return ParseError::DuplicateQualifierSpecified;
   }
 
+  // Ensure the list has 'type' in it
   auto type = std::find(Style->QualifierOrder.begin(),
 Style->QualifierOrder.end(), "type");
   if (type == Style->QualifierOrder.end())
 return ParseError::MissingQualifierType;
+
   return ParseError::Success;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] aacc110 - [clang-format][NFC] Fix typos and inconsistencies

[via cfe-commits]

Author: Krystian Kuzniarek
Date: 2022-02-20T17:20:34-08:00
New Revision: aacc110bdce71d1405d820cf282196855afeee26

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

LOG: [clang-format][NFC] Fix typos and inconsistencies

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

Added: 


Modified: 
clang/lib/Format/Format.cpp

Removed: 




diff  --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 6acd850cac2cb..bc3f0c93426bf 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -990,11 +990,11 @@ std::string ParseErrorCategory::message(int EV) const {
   case ParseError::InvalidQualifierSpecified:
 return "Invalid qualifier specified in QualifierOrder";
   case ParseError::DuplicateQualifierSpecified:
-return "Duplicate qualifier specified in QualfierOrder";
+return "Duplicate qualifier specified in QualifierOrder";
   case ParseError::MissingQualifierType:
-return "Missing type in QualfierOrder";
+return "Missing type in QualifierOrder";
   case ParseError::MissingQualifierOrder:
-return "Missing QualfierOrder";
+return "Missing QualifierOrder";
   }
   llvm_unreachable("unexpected parse error");
 }
@@ -1650,7 +1650,8 @@ ParseError validateQualifierOrder(FormatStyle *Style) {
 if (token == tok::identifier)
   return ParseError::InvalidQualifierSpecified;
   }
-  // Ensure the list is unqiue (no duplicates).
+
+  // Ensure the list is unique (no duplicates).
   std::set UniqueQualifiers(Style->QualifierOrder.begin(),
  Style->QualifierOrder.end());
   if (Style->QualifierOrder.size() != UniqueQualifiers.size()) {
@@ -1660,10 +1661,12 @@ ParseError validateQualifierOrder(FormatStyle *Style) {
 return ParseError::DuplicateQualifierSpecified;
   }
 
+  // Ensure the list has 'type' in it
   auto type = std::find(Style->QualifierOrder.begin(),
 Style->QualifierOrder.end(), "type");
   if (type == Style->QualifierOrder.end())
 return ParseError::MissingQualifierType;
+
   return ParseError::Success;
 }
 



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

[PATCH] D120217: [clang-format] Add an option to insert braces after control statements

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2449
 } else {
-  addUnwrappedLine();
-  ++Line->Level;
-  parseStructuralElement();
-  if (FormatTok->is(tok::eof))
-addUnwrappedLine();
-  --Line->Level;
+  parseUnbracedBody(/*CheckEOF=*/true);
 }

curdeius wrote:
> Is it possible to get rid of the `CheckEOF` parameter and do `if 
> (FormatTok->is(tok::eof)) addUnwrappedLine();` only here?
> (I'm unsure about the dependency between `Line` and `addUnwrappedLine`)
Unfortunately, no. Decrementing `Line->Level` before calling 
`addUnwrappedLine()` would fail `MacroDefinitionsWithIncompleteCode` in 
`FormatTest.cpp`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120217

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

[PATCH] D120200: [WIP][Clang][OpenMP] Add Sema support for atomic compare capture

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 410191.
tianshilei1992 added a comment.

add test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120200

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_messages.c

Index: clang/test/OpenMP/atomic_messages.c
===
--- clang/test/OpenMP/atomic_messages.c
+++ clang/test/OpenMP/atomic_messages.c
@@ -491,4 +491,199 @@
   fx = fe;
   }
 }
+
+void compare_capture(void) {
+  int x = 0;
+  int d = 0;
+  int e = 0;
+  int v = 0;
+  int r = 0;
+// omp51-error@+3 {{the statement for 'atomic compare' must be a compound statement of form '{x = expr ordop x ? expr : x;}', '{x = x ordop expr? expr : x;}', '{x = x == e ? d : x;}', '{x = e == x ? d : x;}', or 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+2 {{expected compound statement}}
+#pragma omp atomic compare capture
+  if (x == e) {}
+// omp51-error@+3 {{the statement for 'atomic compare' must be a compound statement of form '{x = expr ordop x ? expr : x;}', '{x = x ordop expr? expr : x;}', '{x = x == e ? d : x;}', '{x = e == x ? d : x;}', or 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+2 {{expected exactly one expression statement}}
+#pragma omp atomic compare capture
+  if (x == e) {
+x = d;
+v = x;
+  }
+// omp51-error@+4 {{the statement for 'atomic compare' must be a compound statement of form '{x = expr ordop x ? expr : x;}', '{x = x ordop expr? expr : x;}', '{x = x == e ? d : x;}', '{x = e == x ? d : x;}', or 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+3 {{expected assignment statement}}
+#pragma omp atomic compare capture
+  if (x == e) {
+bbar();
+  }
+// omp51-error@+4 {{the statement for 'atomic compare' must be a compound statement of form '{x = expr ordop x ? expr : x;}', '{x = x ordop expr? expr : x;}', '{x = x == e ? d : x;}', '{x = e == x ? d : x;}', or 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+3 {{expected assignment statement}}
+#pragma omp atomic compare capture
+  if (x == e) {
+x += d;
+  }
+// omp51-error@+3 {{the statement for 'atomic compare' must be a compound statement of form '{x = expr ordop x ? expr : x;}', '{x = x ordop expr? expr : x;}', '{x = x == e ? d : x;}', '{x = e == x ? d : x;}', or 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+2 {{expect binary operator in conditional expression}}
+#pragma omp atomic compare capture
+  if (ffoo()) {
+x = d;
+  }
+// omp51-error@+3 {{the statement for 'atomic compare' must be a compound statement of form '{x = expr ordop x ? expr : x;}', '{x = x ordop expr? expr : x;}', '{x = x == e ? d : x;}', '{x = e == x ? d : x;}', or 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+2 {{expect '==' operator}}
+#pragma omp atomic compare capture
+  if (x > e) {
+x = d;
+  }
+// omp51-error@+3 {{the statement for 'atomic compare' must be a compound statement of form '{x = expr ordop x ? expr : x;}', '{x = x ordop expr? expr : x;}', '{x = x == e ? d : x;}', '{x = e == x ? d : x;}', or 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+2 {{expect comparison in a form of 'x == e', 'e == x', 'x ordop expr', or 'expr ordop x'}}
+#pragma omp atomic compare capture
+  if (d == e) {
+x = d;
+  }
+// omp51-error@+3 {{the statement for 'atomic compare' must be a compound statement of form '{x = expr ordop x ?

[PATCH] D120221: [AST] Fix typos

[Krystian Kuzniarek via Phabricator via cfe-commits]

kuzkry added a comment.

Can I ask you to deliver this one for me? I don't have permissions. My name and 
email in git format is "Krystian Kuzniarek "


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120221

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

[PATCH] D120221: [AST] Fix typos

[Krystian Kuzniarek via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120221

Files:
  clang/lib/AST/ASTDiagnostic.cpp


Index: clang/lib/AST/ASTDiagnostic.cpp
===
--- clang/lib/AST/ASTDiagnostic.cpp
+++ clang/lib/AST/ASTDiagnostic.cpp
@@ -372,7 +372,7 @@
 default: llvm_unreachable("unknown ArgumentKind");
 case DiagnosticsEngine::ak_addrspace: {
   assert(Modifier.empty() && Argument.empty() &&
- "Invalid modifier for Qualfiers argument");
+ "Invalid modifier for Qualifiers argument");
 
   auto S = Qualifiers::getAddrSpaceAsString(static_cast(Val));
   if (S.empty()) {
@@ -387,7 +387,7 @@
 }
 case DiagnosticsEngine::ak_qual: {
   assert(Modifier.empty() && Argument.empty() &&
- "Invalid modifier for Qualfiers argument");
+ "Invalid modifier for Qualifiers argument");
 
   Qualifiers Q(Qualifiers::fromOpaqueValue(Val));
   auto S = Q.getAsString();


Index: clang/lib/AST/ASTDiagnostic.cpp
===
--- clang/lib/AST/ASTDiagnostic.cpp
+++ clang/lib/AST/ASTDiagnostic.cpp
@@ -372,7 +372,7 @@
 default: llvm_unreachable("unknown ArgumentKind");
 case DiagnosticsEngine::ak_addrspace: {
   assert(Modifier.empty() && Argument.empty() &&
- "Invalid modifier for Qualfiers argument");
+ "Invalid modifier for Qualifiers argument");
 
   auto S = Qualifiers::getAddrSpaceAsString(static_cast(Val));
   if (S.empty()) {
@@ -387,7 +387,7 @@
 }
 case DiagnosticsEngine::ak_qual: {
   assert(Modifier.empty() && Argument.empty() &&
- "Invalid modifier for Qualfiers argument");
+ "Invalid modifier for Qualifiers argument");
 
   Qualifiers Q(Qualifiers::fromOpaqueValue(Val));
   auto S = Q.getAsString();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120220: [clang-format] Fix typos and consistency

[Krystian Kuzniarek via Phabricator via cfe-commits]

kuzkry added a comment.

Can I ask you to deliver this one for me? My name and email in git format is 
"Krystian Kuzniarek "


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120220

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

[PATCH] D120220: [clang-format] fix typos and consistency

[Krystian Kuzniarek via Phabricator via cfe-commits]

kuzkry created this revision.
kuzkry added reviewers: MyDeveloperDay, HazardyKnusperkeks.
kuzkry added a project: clang-format.
kuzkry requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120220

Files:
  clang/lib/Format/Format.cpp


Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -990,11 +990,11 @@
   case ParseError::InvalidQualifierSpecified:
 return "Invalid qualifier specified in QualifierOrder";
   case ParseError::DuplicateQualifierSpecified:
-return "Duplicate qualifier specified in QualfierOrder";
+return "Duplicate qualifier specified in QualifierOrder";
   case ParseError::MissingQualifierType:
-return "Missing type in QualfierOrder";
+return "Missing type in QualifierOrder";
   case ParseError::MissingQualifierOrder:
-return "Missing QualfierOrder";
+return "Missing QualifierOrder";
   }
   llvm_unreachable("unexpected parse error");
 }
@@ -1650,7 +1650,8 @@
 if (token == tok::identifier)
   return ParseError::InvalidQualifierSpecified;
   }
-  // Ensure the list is unqiue (no duplicates).
+
+  // Ensure the list is unique (no duplicates).
   std::set UniqueQualifiers(Style->QualifierOrder.begin(),
  Style->QualifierOrder.end());
   if (Style->QualifierOrder.size() != UniqueQualifiers.size()) {
@@ -1660,10 +1661,12 @@
 return ParseError::DuplicateQualifierSpecified;
   }
 
+  // Ensure the list has 'type' in it
   auto type = std::find(Style->QualifierOrder.begin(),
 Style->QualifierOrder.end(), "type");
   if (type == Style->QualifierOrder.end())
 return ParseError::MissingQualifierType;
+
   return ParseError::Success;
 }
 


Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -990,11 +990,11 @@
   case ParseError::InvalidQualifierSpecified:
 return "Invalid qualifier specified in QualifierOrder";
   case ParseError::DuplicateQualifierSpecified:
-return "Duplicate qualifier specified in QualfierOrder";
+return "Duplicate qualifier specified in QualifierOrder";
   case ParseError::MissingQualifierType:
-return "Missing type in QualfierOrder";
+return "Missing type in QualifierOrder";
   case ParseError::MissingQualifierOrder:
-return "Missing QualfierOrder";
+return "Missing QualifierOrder";
   }
   llvm_unreachable("unexpected parse error");
 }
@@ -1650,7 +1650,8 @@
 if (token == tok::identifier)
   return ParseError::InvalidQualifierSpecified;
   }
-  // Ensure the list is unqiue (no duplicates).
+
+  // Ensure the list is unique (no duplicates).
   std::set UniqueQualifiers(Style->QualifierOrder.begin(),
  Style->QualifierOrder.end());
   if (Style->QualifierOrder.size() != UniqueQualifiers.size()) {
@@ -1660,10 +1661,12 @@
 return ParseError::DuplicateQualifierSpecified;
   }
 
+  // Ensure the list has 'type' in it
   auto type = std::find(Style->QualifierOrder.begin(),
 Style->QualifierOrder.end(), "type");
   if (type == Style->QualifierOrder.end())
 return ParseError::MissingQualifierType;
+
   return ParseError::Success;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118927: [clang-tidy] Fix invalid fix-it for cppcoreguidelines-prefer-member-initializer

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 410187.
njames93 marked 2 inline comments as done.
njames93 added a comment.

Remove double negative


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118927

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
@@ -526,14 +526,32 @@
   }
 };
 
-struct AlreadyHasInit {
+struct HasInClassInit {
   int m = 4;
-  AlreadyHasInit() {
+  HasInClassInit() {
 m = 3;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'm' should be initialized in a member initializer of the constructor
   }
 };
 
+struct HasInitListInit {
+  int M;
+  // CHECK-MESSAGES: :[[@LINE+5]]:5: warning: 'M' should be initialized in a member initializer of the constructor
+  // CHECK-FIXES: HasInitListInit(const HasInitListInit &Other) : M(Other.M) {
+  // CHECK-FIXES-NEXT: {{^$}}
+  // CHECK-FIXES-NEXT: }
+  HasInitListInit(const HasInitListInit &Other) : M(4) {
+M = Other.M;
+  }
+  // CHECK-MESSAGES: :[[@LINE+5]]:5: warning: 'M' should be initialized in a member initializer of the constructor
+  // CHECK-FIXES: HasInitListInit(HasInitListInit &&Other) : M(Other.M) {
+  // CHECK-FIXES-NEXT: {{^$}}
+  // CHECK-FIXES-NEXT: }
+  HasInitListInit(HasInitListInit &&Other) : M() {
+M = Other.M;
+  }
+};
+
 #define ASSIGN_IN_MACRO(FIELD, VALUE) FIELD = (VALUE);
 
 struct MacroCantFix {
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -109,6 +109,12 @@
 Changes in existing checks
 ^^
 
+- Improved :doc:`cppcoreguidelines-prefer-member-initializer
+  ` check.
+
+  Fixed an issue when there was already an initializer in the constructor and
+  the check would try to create another initializer for the same member.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
@@ -201,30 +201,42 @@
 diag(S->getBeginLoc(), "%0 should be initialized in an in-class"
" default member initializer")
 << Field;
-if (!InvalidFix) {
-  CharSourceRange StmtRange =
-  CharSourceRange::getCharRange(S->getBeginLoc(), SemiColonEnd);
-
-  SmallString<128> Insertion(
-  {UseAssignment ? " = " : "{",
-   Lexer::getSourceText(
-   CharSourceRange(InitValue->getSourceRange(), true),
-   *Result.SourceManager, getLangOpts()),
-   UseAssignment ? "" : "}"});
-
-  Diag << FixItHint::CreateInsertion(FieldEnd, Insertion)
-   << FixItHint::CreateRemoval(StmtRange);
-}
+if (InvalidFix)
+  continue;
+CharSourceRange StmtRange =
+CharSourceRange::getCharRange(S->getBeginLoc(), SemiColonEnd);
+
+SmallString<128> Insertion(
+{UseAssignment ? " = " : "{",
+ Lexer::getSourceText(
+ CharSourceRange(InitValue->getSourceRange(), true),
+ *Result.SourceManager, getLangOpts()),
+ UseAssignment ? "" : "}"});
+
+Diag << FixItHint::CreateInsertion(FieldEnd, Insertion)
+ << FixItHint::CreateRemoval(StmtRange);
+
   } else {
 StringRef InsertPrefix = "";
+bool HasInitAlready = false;
 SourceLocation InsertPos;
+SourceRange ReplaceRange;
 bool AddComma = false;
 bool InvalidFix = false;
 unsigned Index = Field->getFieldIndex();
 const CXXCtorInitializer *LastInListInit = nullptr;
 for (const CXXCtorInitializer *Init : Ctor->inits()) {
-  if (!Init->isWritten())
+  if (!Init->isWritten() || Init->isInClassMemberInitializer())
 continue;
+  if (Init->getMember() == Field) {
+HasInitAlready = true;
+if (isa(Init->getInit()))
+  InsertPos = Init->getRParenLoc();
+else {
+  ReplaceRange = Init->getInit()->getSourceRange();
+}
+break;
+  }
   if (Init

[PATCH] D118900: [ASTMatchers] Expand isInline matcher to VarDecl

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 410186.
njames93 marked an inline comment as done.
njames93 added a comment.

Update description and release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118900

Files:
  clang/docs/LibASTMatchersReference.html
  clang/docs/ReleaseNotes.rst
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
@@ -197,6 +197,8 @@
   functionDecl(isInline(), hasName("f";
   EXPECT_TRUE(matches("namespace n { inline namespace m {} }",
   namespaceDecl(isInline(), hasName("m";
+  EXPECT_TRUE(matches("inline int Foo = 5;",
+  varDecl(isInline(), hasName("Foo")), {Lang_CXX17}));
 }
 
 // FIXME: Figure out how to specify paths so the following tests pass on
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -7673,7 +7673,7 @@
   return InnerMatcher.matches(*ES.getExpr(), Finder, Builder);
 }
 
-/// Matches function and namespace declarations that are marked with
+/// Matches functions, variables and namespace declarations that are marked with
 /// the inline keyword.
 ///
 /// Given
@@ -7683,18 +7683,22 @@
 ///   namespace n {
 ///   inline namespace m {}
 ///   }
+///   inline int Foo = 5;
 /// \endcode
 /// functionDecl(isInline()) will match ::f().
 /// namespaceDecl(isInline()) will match n::m.
-AST_POLYMORPHIC_MATCHER(isInline,
-AST_POLYMORPHIC_SUPPORTED_TYPES(NamespaceDecl,
-FunctionDecl)) {
+/// varDecl(isInline()) will match Foo;
+AST_POLYMORPHIC_MATCHER(isInline, AST_POLYMORPHIC_SUPPORTED_TYPES(NamespaceDecl,
+  FunctionDecl,
+  VarDecl)) {
   // This is required because the spelling of the function used to determine
   // whether inline is specified or not differs between the polymorphic types.
   if (const auto *FD = dyn_cast(&Node))
 return FD->isInlineSpecified();
-  else if (const auto *NSD = dyn_cast(&Node))
+  if (const auto *NSD = dyn_cast(&Node))
 return NSD->isInline();
+  if (const auto *VD = dyn_cast(&Node))
+return VD->isInline();
   llvm_unreachable("Not a valid polymorphic type");
 }
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -177,6 +177,8 @@
 AST Matchers
 
 
+- Expanded ``isInline`` narrowing matcher to support c++17 inline variables.
+
 clang-format
 
 
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -4322,7 +4322,7 @@
 
 
 MatcherFunctionDecl>isInline
-Matches function and namespace declarations that are marked with
+Matches functions, variables and namespace declarations that are marked with
 the inline keyword.
 
 Given
@@ -4331,8 +4331,10 @@
   namespace n {
   inline namespace m {}
   }
+  inline int Foo = 5;
 functionDecl(isInline()) will match ::f().
 namespaceDecl(isInline()) will match n::m.
+varDecl(isInline()) will match Foo;
 
 
 
@@ -4697,7 +4699,7 @@
 
 
 MatcherNamespaceDecl>isInline
-Matches function and namespace declarations that are marked with
+Matches functions, variables and namespace declarations that are marked with
 the inline keyword.
 
 Given
@@ -4706,8 +4708,10 @@
   namespace n {
   inline namespace m {}
   }
+  inline int Foo = 5;
 functionDecl(isInline()) will match ::f().
 namespaceDecl(isInline()) will match n::m.
+varDecl(isInline()) will match Foo;
 
 
 
@@ -5728,6 +5732,23 @@
 
 
 
+MatcherVarDecl>isInline
+Matches functions, variables and namespace declarations that are marked with
+the inline keyword.
+
+Given
+  inline void f();
+  void g();
+  namespace n {
+  inline namespace m {}
+  }
+  inline int Foo = 5;
+functionDecl(isInline()) will match ::f().
+namespaceDecl(isInline()) will match n::m.
+varDecl(isInline()) will match Foo;
+
+
+
 MatcherVarDecl>isStaticLocal
 Matches a static variable with local scope.
 

[PATCH] D120217: [clang-format] Add an option to insert braces after control statements

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2303
+static FormatToken *getLastNonComment(const UnwrappedLine &Line) {
+  for (const auto &Token : llvm::reverse(Line.Tokens))
+if (Token.Tok->isNot(tok::comment))

Get the last token, and if it's a comment `getPreviousNonComment()`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120217

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

[PATCH] D119562: [clang-tidy] Provide fine control of color in run-clang-tidy

[Nathan James via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc57b8ca721dd: [clang-tidy] Provide fine control of color in 
run-clang-tidy (authored by kesyog, committed by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119562

Files:
  clang-tools-extra/clang-tidy/tool/run-clang-tidy.py


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -62,6 +62,21 @@
 import queue as queue
 
 
+def strtobool(val):
+  """Convert a string representation of truth to a bool following LLVM's CLI 
argument parsing."""
+
+  val = val.lower()
+  if val in ['', 'true', '1']:
+return True
+  elif val in ['false', '0']:
+return False
+
+  # Return ArgumentTypeError so that argparse does not substitute its own 
error message
+  raise argparse.ArgumentTypeError(
+"'{}' is invalid value for boolean argument! Try 0 or 1.".format(val)
+  )
+
+
 def find_compilation_database(path):
   """Adjusts the directory until a compilation database is found."""
   result = './'
@@ -82,15 +97,20 @@
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
 header_filter, allow_enabling_alpha_checkers,
 extra_arg, extra_arg_before, quiet, config,
-line_filter):
+line_filter, use_color):
   """Gets a command line for clang-tidy."""
-  start = [clang_tidy_binary, '--use-color']
+  start = [clang_tidy_binary]
   if allow_enabling_alpha_checkers:
 start.append('-allow-enabling-analyzer-alpha-checkers')
   if header_filter is not None:
 start.append('-header-filter=' + header_filter)
   if line_filter is not None:
 start.append('-line-filter=' + line_filter)
+  if use_color is not None:
+if use_color:
+  start.append('--use-color')
+else:
+  start.append('--use-color=false')
   if checks:
 start.append('-checks=' + checks)
   if tmpdir is not None:
@@ -168,7 +188,8 @@
  tmpdir, build_path, args.header_filter,
  args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg_before,
- args.quiet, args.config, args.line_filter)
+ args.quiet, args.config, args.line_filter,
+ args.use_color)
 
 proc = subprocess.Popen(invocation, stdout=subprocess.PIPE, 
stderr=subprocess.PIPE)
 output, err = proc.communicate()
@@ -231,6 +252,10 @@
   'after applying fixes')
   parser.add_argument('-style', default='file', help='The style of reformat '
   'code after applying fixes')
+  parser.add_argument('-use-color', type=strtobool, nargs='?', const=True,
+  help='Use colors in diagnostics, overriding 
clang-tidy\'s'
+  ' default behavior. This option overrides the \'UseColor'
+  '\' option in .clang-tidy file, if any.')
   parser.add_argument('-p', dest='build_path',
   help='Path used to read a compile command database.')
   parser.add_argument('-extra-arg', dest='extra_arg',
@@ -258,7 +283,8 @@
  None, build_path, args.header_filter,
  args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg_before,
- args.quiet, args.config, args.line_filter)
+ args.quiet, args.config, args.line_filter,
+ args.use_color)
 invocation.append('-list-checks')
 invocation.append('-')
 if args.quiet:


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -62,6 +62,21 @@
 import queue as queue
 
 
+def strtobool(val):
+  """Convert a string representation of truth to a bool following LLVM's CLI argument parsing."""
+
+  val = val.lower()
+  if val in ['', 'true', '1']:
+return True
+  elif val in ['false', '0']:
+return False
+
+  # Return ArgumentTypeError so that argparse does not substitute its own error message
+  raise argparse.ArgumentTypeError(
+"'{}' is invalid value for boolean argument! Try 0 or 1.".format(val)
+  )
+
+
 def find_compilation_database(path):
   """Adjusts the directory until a compilation database is found."""
   result = './'
@@ -82,15 +97,20 @@
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_p

[clang-tools-extra] c57b8ca - [clang-tidy] Provide fine control of color in run-clang-tidy

[Nathan James via cfe-commits]

Author: Kesavan Yogeswaran
Date: 2022-02-20T22:00:28Z
New Revision: c57b8ca721dd2e88ed96b7df65a518fdab738445

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

LOG: [clang-tidy] Provide fine control of color in run-clang-tidy

D90110 modified the behavior of `run-clang-tidy` to always pass the
`--use-color` option to clang-tidy, which enabled colored diagnostics
output regardless of TTY status or .clang-tidy settings. This left the
user with no option to disable the colored output.

This presents an issue when trying to parse the output of run-clang-tidy
programmaticall, as the output is polluted with ANSI escape characters.

This PR fixes this issue in two ways:
1. It restores the default behavior of `run-clang-tidy` to let
   `clang-tidy` decide whether to color output. This allows the user to
   configure color via the `UseColor` option in a .clang-tidy file.
2. It adds mutually exclusive, optional `-use-color` and `-no-use-color`
   argument flags that let the user explicitly set the color option via
   the invocation.

After this change the default behavior of `run-clang-tidy` when no
.clang-tidy file is available is now to show no color, presumably
because `clang-tidy` detects that the output is being piped and defaults
to not showing colored output. This seems like an acceptable tradeoff
to respect .clang-tidy configurations, as users can still use the
`-use-color` option to explicitly enable color.

Fixes #49441 (50097 in Bugzilla)

Reviewed By: njames93

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/tool/run-clang-tidy.py

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py 
b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
index 090646c1b061b..fa98c217e2381 100755
--- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -62,6 +62,21 @@
 import queue as queue
 
 
+def strtobool(val):
+  """Convert a string representation of truth to a bool following LLVM's CLI 
argument parsing."""
+
+  val = val.lower()
+  if val in ['', 'true', '1']:
+return True
+  elif val in ['false', '0']:
+return False
+
+  # Return ArgumentTypeError so that argparse does not substitute its own 
error message
+  raise argparse.ArgumentTypeError(
+"'{}' is invalid value for boolean argument! Try 0 or 1.".format(val)
+  )
+
+
 def find_compilation_database(path):
   """Adjusts the directory until a compilation database is found."""
   result = './'
@@ -82,15 +97,20 @@ def make_absolute(f, directory):
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
 header_filter, allow_enabling_alpha_checkers,
 extra_arg, extra_arg_before, quiet, config,
-line_filter):
+line_filter, use_color):
   """Gets a command line for clang-tidy."""
-  start = [clang_tidy_binary, '--use-color']
+  start = [clang_tidy_binary]
   if allow_enabling_alpha_checkers:
 start.append('-allow-enabling-analyzer-alpha-checkers')
   if header_filter is not None:
 start.append('-header-filter=' + header_filter)
   if line_filter is not None:
 start.append('-line-filter=' + line_filter)
+  if use_color is not None:
+if use_color:
+  start.append('--use-color')
+else:
+  start.append('--use-color=false')
   if checks:
 start.append('-checks=' + checks)
   if tmpdir is not None:
@@ -168,7 +188,8 @@ def run_tidy(args, tmpdir, build_path, queue, lock, 
failed_files):
  tmpdir, build_path, args.header_filter,
  args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg_before,
- args.quiet, args.config, args.line_filter)
+ args.quiet, args.config, args.line_filter,
+ args.use_color)
 
 proc = subprocess.Popen(invocation, stdout=subprocess.PIPE, 
stderr=subprocess.PIPE)
 output, err = proc.communicate()
@@ -231,6 +252,10 @@ def main():
   'after applying fixes')
   parser.add_argument('-style', default='file', help='The style of reformat '
   'code after applying fixes')
+  parser.add_argument('-use-color', type=strtobool, nargs='?', const=True,
+  help='Use colors in diagnostics, overriding 
clang-tidy\'s'
+  ' default behavior. This option overrides the \'UseColor'
+  '\' option in .clang-tidy file, if any.')
   parser.add_argument('-p', dest='build_path',
   help='Path used to

[PATCH] D119682: [clang-format][docs] Fix incorrect 'clang-format 13' configuration options markers

[Björn Schäpers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8f310d1967c2: [clang-format][docs] Fix incorrect 
'clang-format 13' configuration ... (authored by kuzkry, committed by 
HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119682

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1791,7 +1791,7 @@
   };
 
   /// The concept declaration style to use.
-  /// \version 13
+  /// \version 12
   BreakBeforeConceptDeclarationsStyle BreakBeforeConceptDeclarations;
 
   /// If ``true``, ternary operators will be placed after line breaks.
@@ -2185,7 +2185,7 @@
   };
 
   /// Defines in which cases to put empty line before access modifiers.
-  /// \version 13
+  /// \version 12
   EmptyLineBeforeAccessModifierStyle EmptyLineBeforeAccessModifier;
 
   /// If ``true``, clang-format detects whether function calls and
@@ -2523,6 +2523,8 @@
 
   /// Indent the requires clause in a template. This only applies when
   /// ``RequiresClausePosition`` is ``OwnLine``, or ``WithFollowing``.
+  ///
+  /// In clang-format 13 and 14 it was named ``IndentRequires``.
   /// \code
   ///true:
   ///template 
@@ -2538,7 +2540,7 @@
   ///  //
   ///}
   /// \endcode
-  /// \version 13
+  /// \version 15
   bool IndentRequiresClause;
 
   /// The number of columns to use for indentation.
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1988,7 +1988,7 @@
 
 
 
-**BreakBeforeConceptDeclarations** (``BreakBeforeConceptDeclarationsStyle``) 
:versionbadge:`clang-format 13`
+**BreakBeforeConceptDeclarations** (``BreakBeforeConceptDeclarationsStyle``) 
:versionbadge:`clang-format 12`
   The concept declaration style to use.
 
   Possible values:
@@ -2278,7 +2278,7 @@
 
 
 
-**EmptyLineBeforeAccessModifier** (``EmptyLineBeforeAccessModifierStyle``) 
:versionbadge:`clang-format 13`
+**EmptyLineBeforeAccessModifier** (``EmptyLineBeforeAccessModifierStyle``) 
:versionbadge:`clang-format 12`
   Defines in which cases to put empty line before access modifiers.
 
   Possible values:
@@ -2706,10 +2706,12 @@
 
 
 
-**IndentRequiresClause** (``Boolean``) :versionbadge:`clang-format 13`
+**IndentRequiresClause** (``Boolean``) :versionbadge:`clang-format 15`
   Indent the requires clause in a template. This only applies when
   ``RequiresClausePosition`` is ``OwnLine``, or ``WithFollowing``.
 
+  In clang-format 13 and 14 it was named ``IndentRequires``.
+
   .. code-block:: c++
 
  true:


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1791,7 +1791,7 @@
   };
 
   /// The concept declaration style to use.
-  /// \version 13
+  /// \version 12
   BreakBeforeConceptDeclarationsStyle BreakBeforeConceptDeclarations;
 
   /// If ``true``, ternary operators will be placed after line breaks.
@@ -2185,7 +2185,7 @@
   };
 
   /// Defines in which cases to put empty line before access modifiers.
-  /// \version 13
+  /// \version 12
   EmptyLineBeforeAccessModifierStyle EmptyLineBeforeAccessModifier;
 
   /// If ``true``, clang-format detects whether function calls and
@@ -2523,6 +2523,8 @@
 
   /// Indent the requires clause in a template. This only applies when
   /// ``RequiresClausePosition`` is ``OwnLine``, or ``WithFollowing``.
+  ///
+  /// In clang-format 13 and 14 it was named ``IndentRequires``.
   /// \code
   ///true:
   ///template 
@@ -2538,7 +2540,7 @@
   ///  //
   ///}
   /// \endcode
-  /// \version 13
+  /// \version 15
   bool IndentRequiresClause;
 
   /// The number of columns to use for indentation.
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1988,7 +1988,7 @@
 
 
 
-**BreakBeforeConceptDeclarations** (``BreakBeforeConceptDeclarationsStyle``) :versionbadge:`clang-format 13`
+**BreakBeforeConceptDeclarations** (``BreakBeforeConceptDeclarationsStyle``) :versionbadge:`clang-format 12`
   The concept declaration style to use.
 
   Possible values:
@@ -2278,7 +2278,7 @@
 
 
 
-**EmptyLineBeforeAccessModifier** (``EmptyLineBeforeAccessModifierStyle``) :versionbadge:`clang-format 13`
+**EmptyLineBeforeAccessModifier** (``EmptyLineBeforeAccessModifierStyle``) :versionbadge:`clang-format 12`
   Defines in which cases to put empty line before access modifiers.
 
   Possible values:
@@

[PATCH] D119923: [clang-format][NFC] Return early in ContinuationIndenter::mustBreak

[Björn Schäpers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9b139923bc66: [clang-format][NFC] Return early in 
ContinuationIndenter::mustBreak (authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119923

Files:
  clang/lib/Format/ContinuationIndenter.cpp


Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -448,26 +448,31 @@
   // current style uses wrapping before or after operators for the given
   // operator.
   if (Previous.is(TT_BinaryOperator) && Current.CanBreakBefore) {
-// If we need to break somewhere inside the LHS of a binary expression, we
-// should also break after the operator. Otherwise, the formatting would
-// hide the operator precedence, e.g. in:
-//   if (aa ==
-//   bb && c) {..
-// For comparisons, we only apply this rule, if the LHS is a binary
-// expression itself as otherwise, the line breaks seem superfluous.
-// We need special cases for ">>" which we have split into two ">" while
-// lexing in order to make template parsing easier.
-bool IsComparison = (Previous.getPrecedence() == prec::Relational ||
- Previous.getPrecedence() == prec::Equality ||
- Previous.getPrecedence() == prec::Spaceship) &&
-Previous.Previous &&
-Previous.Previous->isNot(TT_BinaryOperator); // For >>.
-bool LHSIsBinaryExpr =
-Previous.Previous && Previous.Previous->EndsBinaryExpression;
-if ((!IsComparison || LHSIsBinaryExpr) && !Current.isTrailingComment() &&
-Previous.getPrecedence() != prec::Assignment &&
-CurrentState.BreakBeforeParameter)
-  return true;
+const auto PreviousPrecedence = Previous.getPrecedence();
+if (PreviousPrecedence != prec::Assignment &&
+CurrentState.BreakBeforeParameter && !Current.isTrailingComment()) {
+  const bool LHSIsBinaryExpr =
+  Previous.Previous && Previous.Previous->EndsBinaryExpression;
+  if (LHSIsBinaryExpr)
+return true;
+  // If we need to break somewhere inside the LHS of a binary expression, 
we
+  // should also break after the operator. Otherwise, the formatting would
+  // hide the operator precedence, e.g. in:
+  //   if (aa ==
+  //   bb && c) {..
+  // For comparisons, we only apply this rule, if the LHS is a binary
+  // expression itself as otherwise, the line breaks seem superfluous.
+  // We need special cases for ">>" which we have split into two ">" while
+  // lexing in order to make template parsing easier.
+  const bool IsComparison =
+  (PreviousPrecedence == prec::Relational ||
+   PreviousPrecedence == prec::Equality ||
+   PreviousPrecedence == prec::Spaceship) &&
+  Previous.Previous &&
+  Previous.Previous->isNot(TT_BinaryOperator); // For >>.
+  if (!IsComparison)
+return true;
+}
   } else if (Current.is(TT_BinaryOperator) && Current.CanBreakBefore &&
  CurrentState.BreakBeforeParameter) {
 return true;


Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -448,26 +448,31 @@
   // current style uses wrapping before or after operators for the given
   // operator.
   if (Previous.is(TT_BinaryOperator) && Current.CanBreakBefore) {
-// If we need to break somewhere inside the LHS of a binary expression, we
-// should also break after the operator. Otherwise, the formatting would
-// hide the operator precedence, e.g. in:
-//   if (aa ==
-//   bb && c) {..
-// For comparisons, we only apply this rule, if the LHS is a binary
-// expression itself as otherwise, the line breaks seem superfluous.
-// We need special cases for ">>" which we have split into two ">" while
-// lexing in order to make template parsing easier.
-bool IsComparison = (Previous.getPrecedence() == prec::Relational ||
- Previous.getPrecedence() == prec::Equality ||
- Previous.getPrecedence() == prec::Spaceship) &&
-Previous.Previous &&
-Previous.Previous->isNot(TT_BinaryOperator); // For >>.
-bool LHSIsBinaryExpr =
-Previous.Previous && Previous.Previous->EndsBinaryExpression;
-if ((!IsComparison || LHSIsBinaryExpr) && !Current.isTrailingComment() &&
-Previous.getPrecedence() != prec::Assignment &&
-CurrentState.BreakBeforeParameter)

[PATCH] D119893: [clang-format] Fixed handling of requires clauses followed by attributes

[Björn Schäpers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbe9a7fdd6a8a: [clang-format] Fixed handling of requires 
clauses followed by attributes (authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119893

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -232,6 +232,20 @@
 "Namespace::Outer::Inner::Constant) {}");
   ASSERT_EQ(Tokens.size(), 24u) << Tokens;
   EXPECT_TOKEN(Tokens[7], tok::kw_requires, TT_RequiresClause);
+
+  Tokens = annotate("struct [[nodiscard]] zero_t {\n"
+"  template\n"
+"requires requires { number_zero_v; }\n"
+"  [[nodiscard]] constexpr operator T() const { "
+"return number_zero_v; }\n"
+"};");
+  ASSERT_EQ(Tokens.size(), 44u);
+  EXPECT_TOKEN(Tokens[13], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[14], tok::kw_requires, TT_RequiresExpression);
+  EXPECT_TOKEN(Tokens[15], tok::l_brace, TT_RequiresExpressionLBrace);
+  EXPECT_TOKEN(Tokens[21], tok::r_brace, TT_Unknown);
+  EXPECT_EQ(Tokens[21]->MatchingParen, Tokens[15]);
+  EXPECT_TRUE(Tokens[21]->ClosesRequiresClause);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsRequiresExpressions) {
@@ -507,6 +521,35 @@
   NumberOfAdditionalRequiresClauseTokens = 14u;
   NumberOfTokensBeforeRequires = 5u;
 
+  ASSERT_EQ(BaseTokens.size(), NumberOfBaseTokens) << BaseTokens;
+  ASSERT_EQ(ConstrainedTokens.size(),
+NumberOfBaseTokens + NumberOfAdditionalRequiresClauseTokens)
+  << ConstrainedTokens;
+
+  for (auto I = 0u; I < NumberOfBaseTokens; ++I)
+if (I < NumberOfTokensBeforeRequires)
+  EXPECT_EQ(*BaseTokens[I], *ConstrainedTokens[I]) << I;
+else
+  EXPECT_EQ(*BaseTokens[I],
+*ConstrainedTokens[I + NumberOfAdditionalRequiresClauseTokens])
+  << I;
+
+  BaseTokens = annotate("struct [[nodiscard]] zero_t {\n"
+"  template\n"
+"  [[nodiscard]] constexpr operator T() const { "
+"return number_zero_v; }\n"
+"};");
+
+  ConstrainedTokens = annotate("struct [[nodiscard]] zero_t {\n"
+   "  template\n"
+   "requires requires { number_zero_v; }\n"
+   "  [[nodiscard]] constexpr operator T() const { "
+   "return number_zero_v; }\n"
+   "};");
+  NumberOfBaseTokens = 35u;
+  NumberOfAdditionalRequiresClauseTokens = 9u;
+  NumberOfTokensBeforeRequires = 13u;
+
   ASSERT_EQ(BaseTokens.size(), NumberOfBaseTokens) << BaseTokens;
   ASSERT_EQ(ConstrainedTokens.size(),
 NumberOfBaseTokens + NumberOfAdditionalRequiresClauseTokens)
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -23784,7 +23784,7 @@
"concept C = [] && requires(T t) { typename T::size_type; };");
 }
 
-TEST_F(FormatTest, RequiresClauses) {
+TEST_F(FormatTest, RequiresClausesPositions) {
   auto Style = getLLVMStyle();
   EXPECT_EQ(Style.RequiresClausePosition, FormatStyle::RCPS_OwnLine);
   EXPECT_EQ(Style.IndentRequiresClause, true);
@@ -24007,6 +24007,16 @@
ColumnStyle);
 }
 
+TEST_F(FormatTest, RequiresClauses) {
+  verifyFormat("struct [[nodiscard]] zero_t {\n"
+   "  template \n"
+   "requires requires { number_zero_v; }\n"
+   "  [[nodiscard]] constexpr operator T() const {\n"
+   "return number_zero_v;\n"
+   "  }\n"
+   "};");
+}
+
 TEST_F(FormatTest, StatementAttributeLikeMacros) {
   FormatStyle Style = getLLVMStyle();
   StringRef Source = "void Foo::slot() {\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -19,6 +19,7 @@
 #include "llvm/Support/raw_ostream.h"
 
 #include 
+#include 
 
 #define DEBUG_TYPE "format-parser"
 
@@ -3007,7 +3008,16 @@
 /// clause. It returns, when the parsing is complete, or the expression is
 /// incorrect.
 void UnwrappedLineParser::parseConstraintExpression() {
+  // The special handling for lambdas is needed since tryToParseLambda() eats a
+  // token and if a requires expression is the last part of a requires clause
+  // and followed by

[clang] 8f310d1 - [clang-format][docs] Fix incorrect 'clang-format 13' configuration ...

[Björn Schäpers via cfe-commits]

Author: Krystian Kuzniarek
Date: 2022-02-20T22:33:27+01:00
New Revision: 8f310d1967c20d348c617af3a30999031c71fee0

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

LOG: [clang-format][docs] Fix incorrect 'clang-format 13' configuration ...

...options markers

Note: Option 'IndentRequiresClause' was previously known as
'IndentRequires' but the version marker should still indicate
'clang-format 15' as this option most recent name wasn't accessible
earlier and it would produce:
error: unknown key 'IndentRequiresClause'

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

Added: 


Modified: 
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h

Removed: 




diff  --git a/clang/docs/ClangFormatStyleOptions.rst 
b/clang/docs/ClangFormatStyleOptions.rst
index 8d6c80fb87e5..0cddf022ead3 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -1988,7 +1988,7 @@ the configuration (without a prefix: ``Auto``).
 
 
 
-**BreakBeforeConceptDeclarations** (``BreakBeforeConceptDeclarationsStyle``) 
:versionbadge:`clang-format 13`
+**BreakBeforeConceptDeclarations** (``BreakBeforeConceptDeclarationsStyle``) 
:versionbadge:`clang-format 12`
   The concept declaration style to use.
 
   Possible values:
@@ -2278,7 +2278,7 @@ the configuration (without a prefix: ``Auto``).
 
 
 
-**EmptyLineBeforeAccessModifier** (``EmptyLineBeforeAccessModifierStyle``) 
:versionbadge:`clang-format 13`
+**EmptyLineBeforeAccessModifier** (``EmptyLineBeforeAccessModifierStyle``) 
:versionbadge:`clang-format 12`
   Defines in which cases to put empty line before access modifiers.
 
   Possible values:
@@ -2706,10 +2706,12 @@ the configuration (without a prefix: ``Auto``).
 
 
 
-**IndentRequiresClause** (``Boolean``) :versionbadge:`clang-format 13`
+**IndentRequiresClause** (``Boolean``) :versionbadge:`clang-format 15`
   Indent the requires clause in a template. This only applies when
   ``RequiresClausePosition`` is ``OwnLine``, or ``WithFollowing``.
 
+  In clang-format 13 and 14 it was named ``IndentRequires``.
+
   .. code-block:: c++
 
  true:

diff  --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index c86a70009716..d4a479e7c512 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1791,7 +1791,7 @@ struct FormatStyle {
   };
 
   /// The concept declaration style to use.
-  /// \version 13
+  /// \version 12
   BreakBeforeConceptDeclarationsStyle BreakBeforeConceptDeclarations;
 
   /// If ``true``, ternary operators will be placed after line breaks.
@@ -2185,7 +2185,7 @@ struct FormatStyle {
   };
 
   /// Defines in which cases to put empty line before access modifiers.
-  /// \version 13
+  /// \version 12
   EmptyLineBeforeAccessModifierStyle EmptyLineBeforeAccessModifier;
 
   /// If ``true``, clang-format detects whether function calls and
@@ -2523,6 +2523,8 @@ struct FormatStyle {
 
   /// Indent the requires clause in a template. This only applies when
   /// ``RequiresClausePosition`` is ``OwnLine``, or ``WithFollowing``.
+  ///
+  /// In clang-format 13 and 14 it was named ``IndentRequires``.
   /// \code
   ///true:
   ///template 
@@ -2538,7 +2540,7 @@ struct FormatStyle {
   ///  //
   ///}
   /// \endcode
-  /// \version 13
+  /// \version 15
   bool IndentRequiresClause;
 
   /// The number of columns to use for indentation.



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

[clang] 9b13992 - [clang-format][NFC] Return early in ContinuationIndenter::mustBreak

[Björn Schäpers via cfe-commits]

Author: Björn Schäpers
Date: 2022-02-20T22:33:27+01:00
New Revision: 9b139923bc6634c2d1667c54000debe00e7858f4

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

LOG: [clang-format][NFC] Return early in ContinuationIndenter::mustBreak

We can return as early as possible and only calculate IsComparison if we
really need to. Also cache getPrecedence() instead of querying it at
most 4 times.

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

Added: 


Modified: 
clang/lib/Format/ContinuationIndenter.cpp

Removed: 




diff  --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index a49e0f307cef..f4a755268eae 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -448,26 +448,31 @@ bool ContinuationIndenter::mustBreak(const LineState 
&State) {
   // current style uses wrapping before or after operators for the given
   // operator.
   if (Previous.is(TT_BinaryOperator) && Current.CanBreakBefore) {
-// If we need to break somewhere inside the LHS of a binary expression, we
-// should also break after the operator. Otherwise, the formatting would
-// hide the operator precedence, e.g. in:
-//   if (aa ==
-//   bb && c) {..
-// For comparisons, we only apply this rule, if the LHS is a binary
-// expression itself as otherwise, the line breaks seem superfluous.
-// We need special cases for ">>" which we have split into two ">" while
-// lexing in order to make template parsing easier.
-bool IsComparison = (Previous.getPrecedence() == prec::Relational ||
- Previous.getPrecedence() == prec::Equality ||
- Previous.getPrecedence() == prec::Spaceship) &&
-Previous.Previous &&
-Previous.Previous->isNot(TT_BinaryOperator); // For >>.
-bool LHSIsBinaryExpr =
-Previous.Previous && Previous.Previous->EndsBinaryExpression;
-if ((!IsComparison || LHSIsBinaryExpr) && !Current.isTrailingComment() &&
-Previous.getPrecedence() != prec::Assignment &&
-CurrentState.BreakBeforeParameter)
-  return true;
+const auto PreviousPrecedence = Previous.getPrecedence();
+if (PreviousPrecedence != prec::Assignment &&
+CurrentState.BreakBeforeParameter && !Current.isTrailingComment()) {
+  const bool LHSIsBinaryExpr =
+  Previous.Previous && Previous.Previous->EndsBinaryExpression;
+  if (LHSIsBinaryExpr)
+return true;
+  // If we need to break somewhere inside the LHS of a binary expression, 
we
+  // should also break after the operator. Otherwise, the formatting would
+  // hide the operator precedence, e.g. in:
+  //   if (aa ==
+  //   bb && c) {..
+  // For comparisons, we only apply this rule, if the LHS is a binary
+  // expression itself as otherwise, the line breaks seem superfluous.
+  // We need special cases for ">>" which we have split into two ">" while
+  // lexing in order to make template parsing easier.
+  const bool IsComparison =
+  (PreviousPrecedence == prec::Relational ||
+   PreviousPrecedence == prec::Equality ||
+   PreviousPrecedence == prec::Spaceship) &&
+  Previous.Previous &&
+  Previous.Previous->isNot(TT_BinaryOperator); // For >>.
+  if (!IsComparison)
+return true;
+}
   } else if (Current.is(TT_BinaryOperator) && Current.CanBreakBefore &&
  CurrentState.BreakBeforeParameter) {
 return true;



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

[clang] be9a7fd - [clang-format] Fixed handling of requires clauses followed by attributes

[Björn Schäpers via cfe-commits]

Author: Björn Schäpers
Date: 2022-02-20T22:33:26+01:00
New Revision: be9a7fdd6a8aec669bcb1f6a68087ab4a70ddb2e

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

LOG: [clang-format] Fixed handling of requires clauses followed by attributes

Fixes https://github.com/llvm/llvm-project/issues/53820.

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

Added: 


Modified: 
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/TokenAnnotatorTest.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index c1bd45beb7b3..4c5ab5346b7d 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -19,6 +19,7 @@
 #include "llvm/Support/raw_ostream.h"
 
 #include 
+#include 
 
 #define DEBUG_TYPE "format-parser"
 
@@ -3007,7 +3008,16 @@ void 
UnwrappedLineParser::parseRequiresExpression(FormatToken *RequiresToken) {
 /// clause. It returns, when the parsing is complete, or the expression is
 /// incorrect.
 void UnwrappedLineParser::parseConstraintExpression() {
+  // The special handling for lambdas is needed since tryToParseLambda() eats a
+  // token and if a requires expression is the last part of a requires clause
+  // and followed by an attribute like [[nodiscard]] the ClosesRequiresClause 
is
+  // not set on the correct token. Thus we need to be aware if we even expect a
+  // lambda to be possible.
+  // template  requires requires { ... } [[nodiscard]] ...;
+  bool LambdaNextTimeAllowed = true;
   do {
+bool LambdaThisTimeAllowed = std::exchange(LambdaNextTimeAllowed, false);
+
 switch (FormatTok->Tok.getKind()) {
 case tok::kw_requires: {
   auto RequiresToken = FormatTok;
@@ -3021,7 +3031,7 @@ void UnwrappedLineParser::parseConstraintExpression() {
   break;
 
 case tok::l_square:
-  if (!tryToParseLambda())
+  if (!LambdaThisTimeAllowed || !tryToParseLambda())
 return;
   break;
 
@@ -3064,10 +3074,15 @@ void UnwrappedLineParser::parseConstraintExpression() {
 case tok::pipepipe:
   FormatTok->setType(TT_BinaryOperator);
   nextToken();
+  LambdaNextTimeAllowed = true;
+  break;
+
+case tok::comma:
+case tok::comment:
+  LambdaNextTimeAllowed = LambdaThisTimeAllowed;
+  nextToken();
   break;
 
-case tok::kw_true:
-case tok::kw_false:
 case tok::kw_sizeof:
 case tok::greater:
 case tok::greaterequal:
@@ -3082,11 +3097,16 @@ void UnwrappedLineParser::parseConstraintExpression() {
 case tok::minus:
 case tok::star:
 case tok::slash:
-case tok::numeric_constant:
 case tok::kw_decltype:
-case tok::comment:
-case tok::comma:
+  LambdaNextTimeAllowed = true;
+  // Just eat them.
+  nextToken();
+  break;
+
+case tok::numeric_constant:
 case tok::coloncolon:
+case tok::kw_true:
+case tok::kw_false:
   // Just eat them.
   nextToken();
   break;

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index f71f8dc5de45..f6810766d83d 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -23784,7 +23784,7 @@ TEST_F(FormatTest, Concepts) {
"concept C = [] && requires(T t) { typename T::size_type; };");
 }
 
-TEST_F(FormatTest, RequiresClauses) {
+TEST_F(FormatTest, RequiresClausesPositions) {
   auto Style = getLLVMStyle();
   EXPECT_EQ(Style.RequiresClausePosition, FormatStyle::RCPS_OwnLine);
   EXPECT_EQ(Style.IndentRequiresClause, true);
@@ -24007,6 +24007,16 @@ TEST_F(FormatTest, RequiresClauses) {
ColumnStyle);
 }
 
+TEST_F(FormatTest, RequiresClauses) {
+  verifyFormat("struct [[nodiscard]] zero_t {\n"
+   "  template \n"
+   "requires requires { number_zero_v; }\n"
+   "  [[nodiscard]] constexpr operator T() const {\n"
+   "return number_zero_v;\n"
+   "  }\n"
+   "};");
+}
+
 TEST_F(FormatTest, StatementAttributeLikeMacros) {
   FormatStyle Style = getLLVMStyle();
   StringRef Source = "void Foo::slot() {\n"

diff  --git a/clang/unittests/Format/TokenAnnotatorTest.cpp 
b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 0abe533dd5fc..e7bc26b5a9b5 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -232,6 +232,20 @@ TEST_F(TokenAnnotatorTest, 
UnderstandsRequiresClausesAndConcepts) {
 "Namespace::Outer::Inner::Constant) {}");
   ASSERT_EQ(Tokens.size(), 24u) << Tokens;
   EXPECT_TOKEN(Tokens[7], tok::kw_requires, TT_RequiresClause);
+
+  Tokens = annotate("struct [[nodisca

[PATCH] D120217: [clang-format] Add an option to insert braces after control statements

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Nice! A few minor comments though.




Comment at: clang/include/clang/Format/Format.h:2572-2573
 
+  /// Insert braces after control statements (``if``, ``else``, ``for``, 
``do``,
+  /// and ``while``) in C++.
+  /// \warning

Please add a comment in which cases the braces are added. If I understand 
correctly, everywhere except for PP directives, right?



Comment at: clang/lib/Format/Format.cpp:1839
+private:
+  // Insert optional braces.
+  void insertBraces(SmallVectorImpl &Lines,

This comment seems not very useful, remove please.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2449
 } else {
-  addUnwrappedLine();
-  ++Line->Level;
-  parseStructuralElement();
-  if (FormatTok->is(tok::eof))
-addUnwrappedLine();
-  --Line->Level;
+  parseUnbracedBody(/*CheckEOF=*/true);
 }

Is it possible to get rid of the `CheckEOF` parameter and do `if 
(FormatTok->is(tok::eof)) addUnwrappedLine();` only here?
(I'm unsure about the dependency between `Line` and `addUnwrappedLine`)



Comment at: clang/unittests/Format/FormatTest.cpp:24298
+
+  verifyFormat("if (a) {\n"
+   "  switch (b) {\n"

Could you add a test with a `// clang-format off` part?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120217

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

[PATCH] D120140: [clang-format] Avoid inserting space after C++ casts.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius planned changes to this revision.
curdeius added a comment.

This commit provokes failures in formatting tests of polly.
Cf. https://lab.llvm.org/buildbot/#/builders/205/builds/3320.

That's probably because of ) being annotated as CastRParen instead of Unknown 
before, hence being kept on the same line with the next token.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120140

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

[clang] 4701bca - Revert "[clang-format] Avoid inserting space after C++ casts."

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2022-02-20T22:19:51+01:00
New Revision: 4701bcae974704a336ac8e111d5b104f4834099c

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

LOG: Revert "[clang-format] Avoid inserting space after C++ casts."

This reverts commit e021987273bece6e94bc6f43b6b5232de10637c8.

This commit provokes failures in formatting tests of polly.
Cf. https://lab.llvm.org/buildbot/#/builders/205/builds/3320.

That's probably because of `)` being annotated as `CastRParen` instead of 
`Unknown` before, hence being kept on the same line with the next token.

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 51e7c32b7d4c7..9a020eb6ca7dc 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1734,11 +1734,8 @@ class AnnotatingParser {
   else
 Current.setType(TT_LineComment);
 } else if (Current.is(tok::r_paren)) {
-  if (rParenEndsCast(Current)) {
+  if (rParenEndsCast(Current))
 Current.setType(TT_CastRParen);
-assert(Current.MatchingParen);
-Current.MatchingParen->setType(TT_Unknown);
-  }
   if (Current.MatchingParen && Current.Next &&
   !Current.Next->isBinaryOperator() &&
   !Current.Next->isOneOf(tok::semi, tok::colon, tok::l_brace,
@@ -1941,20 +1938,8 @@ class AnnotatingParser {
 
   // Certain other tokens right before the parentheses are also signals 
that
   // this cannot be a cast.
-  if (LeftOfParens->is(TT_TemplateCloser)) {
-if (LeftOfParens->MatchingParen) {
-  auto *Prev = LeftOfParens->MatchingParen->getPreviousNonComment();
-  if (Prev &&
-  Prev->isOneOf(tok::kw_const_cast, tok::kw_dynamic_cast,
-tok::kw_reinterpret_cast, tok::kw_static_cast))
-// FIXME: Maybe we should handle identifiers ending with "_cast",
-// e.g. any_cast?
-return true;
-}
-return false;
-  }
   if (LeftOfParens->isOneOf(tok::at, tok::r_square, TT_OverloadedOperator,
-tok::ellipsis))
+TT_TemplateCloser, tok::ellipsis))
 return false;
 }
 

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index d45146d5242fb..f71f8dc5de456 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -10565,13 +10565,6 @@ TEST_F(FormatTest, 
FormatsBinaryOperatorsPrecedingEquals) {
 
 TEST_F(FormatTest, FormatsCasts) {
   verifyFormat("Type *A = static_cast(P);");
-  verifyFormat("static_cast(P);");
-  verifyFormat("static_cast(Fun)(Args);");
-  verifyFormat("static_cast(*Fun)(Args);");
-  verifyFormat("a = static_cast(*Fun)(Args);");
-  verifyFormat("const_cast(*Fun)(Args);");
-  verifyFormat("dynamic_cast(*Fun)(Args);");
-  verifyFormat("reinterpret_cast(*Fun)(Args);");
   verifyFormat("Type *A = (Type *)P;");
   verifyFormat("Type *A = (vector)P;");
   verifyFormat("int a = (int)(2.0f);");



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

[PATCH] D120200: [WIP][Clang][OpenMP] Add Sema support for atomic compare capture

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 410180.
tianshilei1992 added a comment.

fnish all checks and add note messages


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120200

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp

Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -10974,6 +10974,16 @@
 NotScalar,
 /// Not an integer.
 NotInteger,
+/// Not an equality operator.
+NotEQ,
+/// Invalid assignment (not v == x).
+InvalidAssignment,
+/// Not if statement
+NotIfStmt,
+/// More than two statements in a compund statement.
+MoreThanTwoStmts,
+/// Not a compound statement.
+NotCompoundStmt,
 /// No error.
 NoError,
   };
@@ -10997,7 +11007,7 @@
   Expr *getCond() const { return C; }
   bool isXBinopExpr() const { return IsXBinopExpr; }
 
-private:
+protected:
   /// Reference to ASTContext
   ASTContext &ContextRef;
   /// 'x' lvalue part of the source atomic expression.
@@ -11024,6 +11034,35 @@
 
   /// Check if all captured values have right type.
   bool checkType(ErrorInfoTy &ErrorInfo) const;
+
+  static bool CheckValue(const Expr *E, ErrorInfoTy &ErrorInfo,
+ bool ShouldBeLValue) {
+if (ShouldBeLValue && !E->isLValue()) {
+  ErrorInfo.Error = ErrorTy::XNotLValue;
+  ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = E->getExprLoc();
+  ErrorInfo.ErrorRange = ErrorInfo.NoteRange = E->getSourceRange();
+  return false;
+}
+
+if (!E->isInstantiationDependent()) {
+  QualType QTy = E->getType();
+  if (!QTy->isScalarType()) {
+ErrorInfo.Error = ErrorTy::NotScalar;
+ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = E->getExprLoc();
+ErrorInfo.ErrorRange = ErrorInfo.NoteRange = E->getSourceRange();
+return false;
+  }
+
+  if (!QTy->isIntegerType()) {
+ErrorInfo.Error = ErrorTy::NotInteger;
+ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = E->getExprLoc();
+ErrorInfo.ErrorRange = ErrorInfo.NoteRange = E->getSourceRange();
+return false;
+  }
+}
+
+return true;
+  }
 };
 
 bool OpenMPAtomicCompareChecker::checkCondUpdateStmt(IfStmt *S,
@@ -11206,41 +11245,13 @@
   // 'x' and 'e' cannot be nullptr
   assert(X && E && "X and E cannot be nullptr");
 
-  auto CheckValue = [&ErrorInfo](const Expr *E, bool ShouldBeLValue) {
-if (ShouldBeLValue && !E->isLValue()) {
-  ErrorInfo.Error = ErrorTy::XNotLValue;
-  ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = E->getExprLoc();
-  ErrorInfo.ErrorRange = ErrorInfo.NoteRange = E->getSourceRange();
-  return false;
-}
-
-if (!E->isInstantiationDependent()) {
-  QualType QTy = E->getType();
-  if (!QTy->isScalarType()) {
-ErrorInfo.Error = ErrorTy::NotScalar;
-ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = E->getExprLoc();
-ErrorInfo.ErrorRange = ErrorInfo.NoteRange = E->getSourceRange();
-return false;
-  }
-
-  if (!QTy->isIntegerType()) {
-ErrorInfo.Error = ErrorTy::NotInteger;
-ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = E->getExprLoc();
-ErrorInfo.ErrorRange = ErrorInfo.NoteRange = E->getSourceRange();
-return false;
-  }
-}
-
-return true;
-  };
-
-  if (!CheckValue(X, true))
+  if (!CheckValue(X, ErrorInfo, true))
 return false;
 
-  if (!CheckValue(E, false))
+  if (!CheckValue(E, ErrorInfo, false))
 return false;
 
-  if (D && !CheckValue(D, false))
+  if (D && !CheckValue(D, ErrorInfo, false))
 return false;
 
   return true;
@@ -11288,6 +11299,375 @@
 
   return checkType(ErrorInfo);
 }
+
+class OpenMPAtomicCompareCaptureChecker final
+: public OpenMPAtomicCompareChecker {
+public:
+  OpenMPAtomicCompareCaptureChecker(Sema &S) : OpenMPAtomicCompareChecker(S) {}
+
+  Expr *getV() const { return V; }
+  Expr *getR() const { return R; }
+  bool isFailOnly() const { return IsFailOnly; }
+
+  /// Check if statement \a S is valid for atomic compare capture.
+  bool checkStmt(Stmt *S, ErrorInfoTy &ErrorInfo);
+
+private:
+  bool checkType(ErrorInfoTy &ErrorInfo);
+
+  // NOTE: Form 3, 4, 5 in the following comments mean the 3rd, 4th, and 5th
+  // form of 'conditional-update-capture-atomic' structured block on the v5.2
+  // spec p.p. 82:
+  // (1) { v = x; cond-update-stmt }
+  // (2) { cond-update-stmt v = x; }
+  // (3) if(x == e) { x = d; } else { v = x; }
+  // (4) { r = x == e; if(r) { x = d; } }
+  // (5) { r = x == e; if(r) { x = d; } else { v = x; } }
+
+  /// Check if it is valid 'if(x == e) { x = d; } else { v = x; }' (form 3)
+  bool checkForm3(IfStmt *S, ErrorInfoTy &ErrorInfo);
+
+  /// Check if it is valid '{ r = x == e; if(r) { x = d; } }',
+  /// or '{ r = x == e; if(r) { x = d; } else { v = x; } }' (form 4 a

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Jonas Toth via Phabricator via cfe-commits]

JonasToth marked 3 inline comments as done.
JonasToth added inline comments.



Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt:33
   LINK_LIBS
+  clangAnalysis
   clangTidy

sammccall wrote:
> JonasToth wrote:
> > njames93 wrote:
> > > Will this work under clangd as I don't think that links in the 
> > > clangAnalysis libraries @sammccall?
> > I did not check if it works and `clangd` does not explicitly link to 
> > `Analysis`, but only to the checks.
> > Doesn't the `LINK_LIBS` induce transitive dependency resolution for linking?
> I don't know whether you need to add it everywhere, I suspect transitive is 
> OK.
> 
> In any case this isn't a new dependency, Sema depends on Analysis and clangd 
> depends on Sema.
i verified that the check works with clangd without any additional build 
settings.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:29
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  return ast_matchers::internal::matchesFirstInPointerRange(
+  InnerMatcher, Node.decl_begin(), Node.decl_end(), Finder, Builder);

sammccall wrote:
> sammccall wrote:
> > This returns an iterator (i.e. a pointer), which is being converted to a 
> > boolean.
> > 
> > i.e. it's always returning true. I think this is why you're seeing nullptr 
> > crashes on Variable.
> this is depending on `::internal` details, which doesn't seem OK.
> I think you'd need to find another way to do it, or move this to ASTMatchers 
> (in a separate patch).
it is common to write custom matchers in clang-tidy first and use the internal 
matcher apis as well in clang-tidy.

```
$ cd llvm-project/clang-tools-extra/clang-tidy
$ git grep "internal::" | wc -l
86
```

Usually, they are extracted once they are generally useful. And many checks 
introduce custom matchers for better readability.
I can extract this matcher later if you want, but right now with the longevity 
of the patch and it being common practice in clang-tidy I am opposed to it.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:29
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  return ast_matchers::internal::matchesFirstInPointerRange(
+  InnerMatcher, Node.decl_begin(), Node.decl_end(), Finder, Builder);

JonasToth wrote:
> sammccall wrote:
> > sammccall wrote:
> > > This returns an iterator (i.e. a pointer), which is being converted to a 
> > > boolean.
> > > 
> > > i.e. it's always returning true. I think this is why you're seeing 
> > > nullptr crashes on Variable.
> > this is depending on `::internal` details, which doesn't seem OK.
> > I think you'd need to find another way to do it, or move this to 
> > ASTMatchers (in a separate patch).
> it is common to write custom matchers in clang-tidy first and use the 
> internal matcher apis as well in clang-tidy.
> 
> ```
> $ cd llvm-project/clang-tools-extra/clang-tidy
> $ git grep "internal::" | wc -l
> 86
> ```
> 
> Usually, they are extracted once they are generally useful. And many checks 
> introduce custom matchers for better readability.
> I can extract this matcher later if you want, but right now with the 
> longevity of the patch and it being common practice in clang-tidy I am 
> opposed to it.
thank you for that hint! that would explain the issues indeed. I try to verify 
your proposed fix by re-transforming LLVM.
some other subscribers checked their own codebases as well. this would give us 
more confidence too.

if the crash is still there, I would like to keep the 'safety-if' in `check` 
and try to find the issue with the matcher.
The path @njames93 linked seemed like it would help there a lot.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:84
+  compoundStmt(
+  findAll(declStmt(allOf(containsDeclaration2(
+ LocalValDecl.bind("local-value")),

njames93 wrote:
> `allOf` is unnecessary here as `declStmt` is implicitly an `allOf` matcher.
> Also `findAll` isn't the right matcher in this case, you likely want 
> `forEachDescendant`. `findAll` will try and match on the `CompoundStmt` which 
> is always going to fail.
switching to `forEachDescendant`, both seem to work.
I am unsure why I used `findAll`, but I know that I started out with 
`forEachDescendant` for some bits of the check and it was insufficent.
But it could very well be, that these pieces are now in the `ExprMutAnalyzer`.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:102-105
+  // There have been crashes on 'Variable == nullptr', even though the matcher
+  // is not conditional. This comes probably from 'findAll'-matching.
+  if (!Variable || !LocalScope)
+return;

njames93 wrote:
> sammccall

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 410179.
JonasToth marked 14 inline comments as done.
JonasToth added a comment.

- address reviews comments, part 1
- Merge branch 'main' into feature_rebase_const_transform_20210808
- fixing iterator-to-bool conversion and addressing other more review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-const-correctness.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-cxx17.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-pointer-as-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-templates.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-pointer-as-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-unaligned.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
  clang/lib/Analysis/ExprMutationAnalyzer.cpp
  clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp

Index: clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
===
--- clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -1251,13 +1251,13 @@
   AST =
   buildASTFromCode("void f() { int* x[2]; for (int* e : x) e = nullptr; }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_FALSE(isMutated(Results, AST.get()));
+  EXPECT_TRUE(isMutated(Results, AST.get()));
 
   AST = buildASTFromCode(
   "typedef int* IntPtr;"
   "void f() { int* x[2]; for (IntPtr e : x) e = nullptr; }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_FALSE(isMutated(Results, AST.get()));
+  EXPECT_TRUE(isMutated(Results, AST.get()));
 }
 
 TEST(ExprMutationAnalyzerTest, RangeForArrayByConstRef) {
Index: clang/lib/Analysis/ExprMutationAnalyzer.cpp
===
--- clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -445,14 +445,16 @@
   // array is considered modified if the loop-variable is a non-const reference.
   const auto DeclStmtToNonRefToArray = declStmt(hasSingleDecl(varDecl(hasType(
   hasUnqualifiedDesugaredType(referenceType(pointee(arrayType(;
-  const auto RefToArrayRefToElements = match(
-  findAll(stmt(cxxForRangeStmt(
-   hasLoopVariable(varDecl(hasType(nonConstReferenceType()))
-   .bind(NodeID::value)),
-   hasRangeStmt(DeclStmtToNonRefToArray),
-   hasRangeInit(canResolveToExpr(equalsNode(Exp)
-  .bind("stmt")),
-  Stm, Context);
+  const auto RefToArrayRefToElements =
+  match(findAll(stmt(cxxForRangeStmt(
+ hasLoopVariable(
+ varDecl(anyOf(hasType(nonConstReferenceType()),
+   hasType(nonConstPointerType(
+ .bind(NodeID::value)),
+ hasRangeStmt(DeclStmtToNonRefToArray),
+ hasRangeInit(canResolveToExpr(equalsNode(Exp)
+.bind("stmt")),
+Stm, Context);
 
   if (const auto *BadRangeInitFromArray =
   selectFirst("stmt", RefToArrayRefToElements))
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
@@ -0,0 +1,958 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-const-correctness %t -- \
+// RUN:   -config="{CheckOptions: [\
+// RUN:   {key: 'cppcoreguidelines-const-correctness.TransformValues', value: true}, \
+// RUN:   {key: 'cppcoreguidelines-const-correctness.WarnPointersAsValues', value: false}, \
+// RUN:   {key: 'cppcoreguidelines-const-correctness.TransformPointersAsValues', value: false}, \
+// RUN:   ]}" -- -fno-delayed-template-parsing
+
+// --- Provide test samples for primitive builtins -

[PATCH] D120140: [clang-format] Avoid inserting space after C++ casts.

[Marek Kurdej via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
curdeius marked an inline comment as done.
Closed by commit rGe021987273be: [clang-format] Avoid inserting space after C++ 
casts. (authored by curdeius).

Changed prior to commit:
  https://reviews.llvm.org/D120140?vs=409959&id=410178#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120140

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10565,6 +10565,13 @@
 
 TEST_F(FormatTest, FormatsCasts) {
   verifyFormat("Type *A = static_cast(P);");
+  verifyFormat("static_cast(P);");
+  verifyFormat("static_cast(Fun)(Args);");
+  verifyFormat("static_cast(*Fun)(Args);");
+  verifyFormat("a = static_cast(*Fun)(Args);");
+  verifyFormat("const_cast(*Fun)(Args);");
+  verifyFormat("dynamic_cast(*Fun)(Args);");
+  verifyFormat("reinterpret_cast(*Fun)(Args);");
   verifyFormat("Type *A = (Type *)P;");
   verifyFormat("Type *A = (vector)P;");
   verifyFormat("int a = (int)(2.0f);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1734,8 +1734,11 @@
   else
 Current.setType(TT_LineComment);
 } else if (Current.is(tok::r_paren)) {
-  if (rParenEndsCast(Current))
+  if (rParenEndsCast(Current)) {
 Current.setType(TT_CastRParen);
+assert(Current.MatchingParen);
+Current.MatchingParen->setType(TT_Unknown);
+  }
   if (Current.MatchingParen && Current.Next &&
   !Current.Next->isBinaryOperator() &&
   !Current.Next->isOneOf(tok::semi, tok::colon, tok::l_brace,
@@ -1938,8 +1941,20 @@
 
   // Certain other tokens right before the parentheses are also signals 
that
   // this cannot be a cast.
+  if (LeftOfParens->is(TT_TemplateCloser)) {
+if (LeftOfParens->MatchingParen) {
+  auto *Prev = LeftOfParens->MatchingParen->getPreviousNonComment();
+  if (Prev &&
+  Prev->isOneOf(tok::kw_const_cast, tok::kw_dynamic_cast,
+tok::kw_reinterpret_cast, tok::kw_static_cast))
+// FIXME: Maybe we should handle identifiers ending with "_cast",
+// e.g. any_cast?
+return true;
+}
+return false;
+  }
   if (LeftOfParens->isOneOf(tok::at, tok::r_square, TT_OverloadedOperator,
-TT_TemplateCloser, tok::ellipsis))
+tok::ellipsis))
 return false;
 }
 


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10565,6 +10565,13 @@
 
 TEST_F(FormatTest, FormatsCasts) {
   verifyFormat("Type *A = static_cast(P);");
+  verifyFormat("static_cast(P);");
+  verifyFormat("static_cast(Fun)(Args);");
+  verifyFormat("static_cast(*Fun)(Args);");
+  verifyFormat("a = static_cast(*Fun)(Args);");
+  verifyFormat("const_cast(*Fun)(Args);");
+  verifyFormat("dynamic_cast(*Fun)(Args);");
+  verifyFormat("reinterpret_cast(*Fun)(Args);");
   verifyFormat("Type *A = (Type *)P;");
   verifyFormat("Type *A = (vector)P;");
   verifyFormat("int a = (int)(2.0f);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1734,8 +1734,11 @@
   else
 Current.setType(TT_LineComment);
 } else if (Current.is(tok::r_paren)) {
-  if (rParenEndsCast(Current))
+  if (rParenEndsCast(Current)) {
 Current.setType(TT_CastRParen);
+assert(Current.MatchingParen);
+Current.MatchingParen->setType(TT_Unknown);
+  }
   if (Current.MatchingParen && Current.Next &&
   !Current.Next->isBinaryOperator() &&
   !Current.Next->isOneOf(tok::semi, tok::colon, tok::l_brace,
@@ -1938,8 +1941,20 @@
 
   // Certain other tokens right before the parentheses are also signals that
   // this cannot be a cast.
+  if (LeftOfParens->is(TT_TemplateCloser)) {
+if (LeftOfParens->MatchingParen) {
+  auto *Prev = LeftOfParens->MatchingParen->getPreviousNonComment();
+  if (Prev &&
+  Prev->isOneOf(tok::kw_const_cast, tok::kw_dynamic_cast,
+tok::kw_reinterpret_cast, tok::kw_static_cast))
+// FIXME: Maybe we should handle identifiers ending with "_cast",
+// e.g. any_cast?
+return true;
+}
+return false;
+  }
   if (Left

[clang] e021987 - [clang-format] Avoid inserting space after C++ casts.

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2022-02-20T21:58:37+01:00
New Revision: e021987273bece6e94bc6f43b6b5232de10637c8

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

LOG: [clang-format] Avoid inserting space after C++ casts.

Fixes https://github.com/llvm/llvm-project/issues/53876.

This is a solution for standard C++ casts: const_cast, dynamic_cast, 
reinterpret_cast, static_cast.

A general approach handling all possible casts is not possible without semantic 
information.
Consider the code:
```
static_cast(*function_pointer_variable)(arguments);
```
vs.
```
some_return_type (*function_pointer_variable)(parameters);
// Later used as:
function_pointer_variable = &some_function;
return function_pointer_variable(args);
```
In the latter case, it's not a cast but a variable declaration of a pointer to 
function.
Without knowing what `some_return_type` is (and clang-format does not know 
it), it's hard to distinguish between the two cases. Theoretically, one could 
check whether "parameters" are types (not a cast) and "arguments" are 
value/expressions (a cast), but that might be inefficient (needs lots of 
lookahead).

Reviewed By: MyDeveloperDay, HazardyKnusperkeks, owenpan

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 9a020eb6ca7dc..51e7c32b7d4c7 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1734,8 +1734,11 @@ class AnnotatingParser {
   else
 Current.setType(TT_LineComment);
 } else if (Current.is(tok::r_paren)) {
-  if (rParenEndsCast(Current))
+  if (rParenEndsCast(Current)) {
 Current.setType(TT_CastRParen);
+assert(Current.MatchingParen);
+Current.MatchingParen->setType(TT_Unknown);
+  }
   if (Current.MatchingParen && Current.Next &&
   !Current.Next->isBinaryOperator() &&
   !Current.Next->isOneOf(tok::semi, tok::colon, tok::l_brace,
@@ -1938,8 +1941,20 @@ class AnnotatingParser {
 
   // Certain other tokens right before the parentheses are also signals 
that
   // this cannot be a cast.
+  if (LeftOfParens->is(TT_TemplateCloser)) {
+if (LeftOfParens->MatchingParen) {
+  auto *Prev = LeftOfParens->MatchingParen->getPreviousNonComment();
+  if (Prev &&
+  Prev->isOneOf(tok::kw_const_cast, tok::kw_dynamic_cast,
+tok::kw_reinterpret_cast, tok::kw_static_cast))
+// FIXME: Maybe we should handle identifiers ending with "_cast",
+// e.g. any_cast?
+return true;
+}
+return false;
+  }
   if (LeftOfParens->isOneOf(tok::at, tok::r_square, TT_OverloadedOperator,
-TT_TemplateCloser, tok::ellipsis))
+tok::ellipsis))
 return false;
 }
 

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index f71f8dc5de456..d45146d5242fb 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -10565,6 +10565,13 @@ TEST_F(FormatTest, 
FormatsBinaryOperatorsPrecedingEquals) {
 
 TEST_F(FormatTest, FormatsCasts) {
   verifyFormat("Type *A = static_cast(P);");
+  verifyFormat("static_cast(P);");
+  verifyFormat("static_cast(Fun)(Args);");
+  verifyFormat("static_cast(*Fun)(Args);");
+  verifyFormat("a = static_cast(*Fun)(Args);");
+  verifyFormat("const_cast(*Fun)(Args);");
+  verifyFormat("dynamic_cast(*Fun)(Args);");
+  verifyFormat("reinterpret_cast(*Fun)(Args);");
   verifyFormat("Type *A = (Type *)P;");
   verifyFormat("Type *A = (vector)P;");
   verifyFormat("int a = (int)(2.0f);");



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

[PATCH] D120188: Fix extraneous whitespace addition in line comments on clang-format directives

[Marek Kurdej via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd9567babef30: Fix extraneous whitespace addition in line 
comments on clang-format directives (authored by penagos, committed by 
curdeius).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120188

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


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -91,6 +91,11 @@
"// line 2\n"
"void f() {}\n");
 
+  EXPECT_EQ("// comment\n"
+"// clang-format on\n",
+format("//comment\n"
+   "// clang-format on\n"));
+
   verifyFormat("void f() {\n"
"  // Doesn't do anything\n"
"}");
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -815,10 +815,13 @@
 
 assert(Lines[i].size() > IndentPrefix.size());
 const auto FirstNonSpace = Lines[i][IndentPrefix.size()];
-const auto AllowsSpaceChange =
-SpacesInPrefix != 0 ||
-(!NoSpaceBeforeFirstCommentChar() ||
- (FirstNonSpace == '}' && FirstLineSpaceChange != 0));
+const bool IsFormatComment = LineTok && switchesFormatting(*LineTok);
+const bool LineRequiresLeadingSpace =
+!NoSpaceBeforeFirstCommentChar() ||
+(FirstNonSpace == '}' && FirstLineSpaceChange != 0);
+const bool AllowsSpaceChange =
+!IsFormatComment &&
+(SpacesInPrefix != 0 || LineRequiresLeadingSpace);
 
 if (PrefixSpaceChange[i] > 0 && AllowsSpaceChange) {
   Prefix[i] = IndentPrefix.str();


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -91,6 +91,11 @@
"// line 2\n"
"void f() {}\n");
 
+  EXPECT_EQ("// comment\n"
+"// clang-format on\n",
+format("//comment\n"
+   "// clang-format on\n"));
+
   verifyFormat("void f() {\n"
"  // Doesn't do anything\n"
"}");
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -815,10 +815,13 @@
 
 assert(Lines[i].size() > IndentPrefix.size());
 const auto FirstNonSpace = Lines[i][IndentPrefix.size()];
-const auto AllowsSpaceChange =
-SpacesInPrefix != 0 ||
-(!NoSpaceBeforeFirstCommentChar() ||
- (FirstNonSpace == '}' && FirstLineSpaceChange != 0));
+const bool IsFormatComment = LineTok && switchesFormatting(*LineTok);
+const bool LineRequiresLeadingSpace =
+!NoSpaceBeforeFirstCommentChar() ||
+(FirstNonSpace == '}' && FirstLineSpaceChange != 0);
+const bool AllowsSpaceChange =
+!IsFormatComment &&
+(SpacesInPrefix != 0 || LineRequiresLeadingSpace);
 
 if (PrefixSpaceChange[i] > 0 && AllowsSpaceChange) {
   Prefix[i] = IndentPrefix.str();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] d9567ba - Fix extraneous whitespace addition in line comments on clang-format directives

[Marek Kurdej via cfe-commits]

Author: Luis Penagos
Date: 2022-02-20T21:53:50+01:00
New Revision: d9567babef302cfd7e827df64138151ba2614b83

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

LOG: Fix extraneous whitespace addition in line comments on clang-format 
directives

Fixes https://github.com/llvm/llvm-project/issues/53844.
I believe this regression was caused by not accounting for clang-format 
directives in https://reviews.llvm.org/D92257.

Reviewed By: HazardyKnusperkeks, curdeius

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

Added: 


Modified: 
clang/lib/Format/BreakableToken.cpp
clang/unittests/Format/FormatTestComments.cpp

Removed: 




diff  --git a/clang/lib/Format/BreakableToken.cpp 
b/clang/lib/Format/BreakableToken.cpp
index 5138c7cd42cc6..967ddeb82383a 100644
--- a/clang/lib/Format/BreakableToken.cpp
+++ b/clang/lib/Format/BreakableToken.cpp
@@ -815,10 +815,13 @@ BreakableLineCommentSection::BreakableLineCommentSection(
 
 assert(Lines[i].size() > IndentPrefix.size());
 const auto FirstNonSpace = Lines[i][IndentPrefix.size()];
-const auto AllowsSpaceChange =
-SpacesInPrefix != 0 ||
-(!NoSpaceBeforeFirstCommentChar() ||
- (FirstNonSpace == '}' && FirstLineSpaceChange != 0));
+const bool IsFormatComment = LineTok && switchesFormatting(*LineTok);
+const bool LineRequiresLeadingSpace =
+!NoSpaceBeforeFirstCommentChar() ||
+(FirstNonSpace == '}' && FirstLineSpaceChange != 0);
+const bool AllowsSpaceChange =
+!IsFormatComment &&
+(SpacesInPrefix != 0 || LineRequiresLeadingSpace);
 
 if (PrefixSpaceChange[i] > 0 && AllowsSpaceChange) {
   Prefix[i] = IndentPrefix.str();

diff  --git a/clang/unittests/Format/FormatTestComments.cpp 
b/clang/unittests/Format/FormatTestComments.cpp
index c988a2869e568..f83ffb393ac2f 100644
--- a/clang/unittests/Format/FormatTestComments.cpp
+++ b/clang/unittests/Format/FormatTestComments.cpp
@@ -91,6 +91,11 @@ TEST_F(FormatTestComments, UnderstandsSingleLineComments) {
"// line 2\n"
"void f() {}\n");
 
+  EXPECT_EQ("// comment\n"
+"// clang-format on\n",
+format("//comment\n"
+   "// clang-format on\n"));
+
   verifyFormat("void f() {\n"
"  // Doesn't do anything\n"
"}");



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

[PATCH] D120217: [clang-format] Add an option to insert braces after control statements

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added reviewers: MyDeveloperDay, curdeius, HazardyKnusperkeks.
owenpan added a project: clang-format.
owenpan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This new option `InsertBraces` has been tested on `make clang` and `make 
check-clang`. It has almost zero overhead when turned off.

- Why this new patch?

Because of this comment 
, I ran 
https://reviews.llvm.org/D95168?id=322240 (the last diff before @MyDeveloperDay 
took over) on `make clang` and indeed it failed a lot.

- Why isn't this option an `enum` or a `struct`?

We plan to re-configure/re-package `RemoveBracesLLVM` to support other styles. 
After that, one can simply turn on `InsertBraces` and specify the suboptions of 
`RemoveBraces`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120217

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -19474,6 +19474,7 @@
   CHECK_PARSE_BOOL_FIELD(IndentRequiresClause, "IndentRequires");
   CHECK_PARSE_BOOL(IndentRequiresClause);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
+  CHECK_PARSE_BOOL(InsertBraces);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
   CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);
@@ -24290,6 +24291,188 @@
   verifyFormat("template  struct Foo {};", Style);
 }
 
+TEST_F(FormatTest, InsertBraces) {
+  FormatStyle Style = getLLVMStyle();
+  Style.InsertBraces = true;
+
+  verifyFormat("if (a) {\n"
+   "  switch (b) {\n"
+   "  case 1:\n"
+   "c = 0;\n"
+   "break;\n"
+   "  default:\n"
+   "c = 1;\n"
+   "  }\n"
+   "}",
+   "if (a)\n"
+   "  switch (b) {\n"
+   "  case 1:\n"
+   "c = 0;\n"
+   "break;\n"
+   "  default:\n"
+   "c = 1;\n"
+   "  }",
+   Style);
+
+  verifyFormat("for (auto node : nodes) {\n"
+   "  if (node) {\n"
+   "break;\n"
+   "  }\n"
+   "}",
+   "for (auto node : nodes)\n"
+   "  if (node)\n"
+   "break;",
+   Style);
+
+  verifyFormat("for (auto node : nodes) {\n"
+   "  if (node)\n"
+   "}",
+   "for (auto node : nodes)\n"
+   "  if (node)",
+   Style);
+
+  verifyFormat("do {\n"
+   "  --a;\n"
+   "} while (a);",
+   "do\n"
+   "  --a;\n"
+   "while (a);",
+   Style);
+
+  verifyFormat("if (i) {\n"
+   "  ++i;\n"
+   "} else {\n"
+   "  --i;\n"
+   "}",
+   "if (i)\n"
+   "  ++i;\n"
+   "else {\n"
+   "  --i;\n"
+   "}",
+   Style);
+
+  verifyFormat("void f() {\n"
+   "  while (j--) {\n"
+   "while (i) {\n"
+   "  --i;\n"
+   "}\n"
+   "  }\n"
+   "}",
+   "void f() {\n"
+   "  while (j--)\n"
+   "while (i)\n"
+   "  --i;\n"
+   "}",
+   Style);
+
+  verifyFormat("f({\n"
+   "  if (a) {\n"
+   "g();\n"
+   "  }\n"
+   "});",
+   "f({\n"
+   "  if (a)\n"
+   "g();\n"
+   "});",
+   Style);
+
+  verifyFormat("if (a) {\n"
+   "  f();\n"
+   "} else if (b) {\n"
+   "  g();\n"
+   "} else {\n"
+   "  h();\n"
+   "}",
+   "if (a)\n"
+   "  f();\n"
+   "else if (b)\n"
+   "  g();\n"
+   "else\n"
+   "  h();",
+   Style);
+
+  verifyFormat("if (a) {\n"
+   "  f();\n"
+   "}\n"
+   "// comment\n"
+   "/* comment */",
+   "if (a)\n"
+   "  f();\n"
+   "// comment\n"
+   "/* comment */",
+   Style);
+
+  verifyFormat("if (a) {\n"
+   "  // foo\n"
+   "  // bar\n"
+   "  f();\n"
+   "}",
+   "if (a)\n"
+   "  // foo\n"
+  

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

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Will you still work on this?


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] D119562: [clang-tidy] Provide fine control of color in run-clang-tidy

[Kesavan Yogeswaran via Phabricator via cfe-commits]

kesyog added a comment.

I don't have commit privileges, so I'll need help committing this to `main`. My 
name is "Kesavan Yogeswaran" and my email is hikes [at] google [dot] com


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119562

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

[clang] ce0fdf1 - Put back err_drv_negative_columns/err_drv_small_columns for flang

[Benjamin Kramer via cfe-commits]

Author: Benjamin Kramer
Date: 2022-02-20T15:42:20+01:00
New Revision: ce0fdf116334506bd5c4609ab86111f8136a1408

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

LOG: Put back err_drv_negative_columns/err_drv_small_columns for flang

These are unused by Clang, but Flang references them.

Added: 


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

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 276e83434d03..b608b8ec5068 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -323,6 +323,10 @@ def err_drv_unsupported_embed_bitcode
 : Error<"%0 is not supported with -fembed-bitcode">;
 def err_drv_bitcode_unsupported_on_toolchain : Error<
   "-fembed-bitcode is not supported on versions of iOS prior to 6.0">;
+def err_drv_negative_columns : Error<
+  "invalid value '%1' in '%0', value must be 'none' or a positive integer">;
+def err_drv_small_columns : Error<
+  "invalid value '%1' in '%0', value must be '%2' or greater">;
 
 def err_drv_invalid_malign_branch_EQ : Error<
   "invalid argument '%0' to -malign-branch=; each element must be one of: %1">;



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

[PATCH] D120188: Fix extraneous whitespace addition in line comments on clang-format directives

[Luis Penagos via Phabricator via cfe-commits]

penagos added a comment.

I don’t have commit rights; if someone could commit on my behalf that’d be 
great.

- Luis Penagos
- l...@penagos.co

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120188

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

[clang-tools-extra] 3a1d6a3 - [clangd] Remove uuidof warning. Clang never emits this one.

[Benjamin Kramer via cfe-commits]

Author: Benjamin Kramer
Date: 2022-02-20T14:44:47+01:00
New Revision: 3a1d6a361c822173abd87ff47fd8613892fc747f

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

LOG: [clangd] Remove uuidof warning. Clang never emits this one.

Added: 


Modified: 
clang-tools-extra/clangd/IncludeFixer.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/IncludeFixer.cpp 
b/clang-tools-extra/clangd/IncludeFixer.cpp
index 1f0515c1df702..7994e5f499200 100644
--- a/clang-tools-extra/clangd/IncludeFixer.cpp
+++ b/clang-tools-extra/clangd/IncludeFixer.cpp
@@ -224,8 +224,6 @@ std::vector IncludeFixer::fix(DiagnosticsEngine::Level 
DiagLevel,
 return only(insertHeader(""));
   case diag::err_need_header_before_typeid:
 return only(insertHeader(""));
-  case diag::err_need_header_before_ms_uuidof:
-return only(insertHeader(""));
   case diag::err_need_header_before_placement_new:
   case diag::err_implicit_coroutine_std_nothrow_type_not_found:
 return only(insertHeader(""));



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

[clang] 52fcdc8 - Prune unused diagnostics. NFC.

[Benjamin Kramer via cfe-commits]

Author: Benjamin Kramer
Date: 2022-02-20T14:06:58+01:00
New Revision: 52fcdc8d69d20b48fb5266b00f505dc89b19be9b

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

LOG: Prune unused diagnostics. NFC.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticASTKinds.td
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/include/clang/Basic/DiagnosticSemaKinds.td

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticASTKinds.td 
b/clang/include/clang/Basic/DiagnosticASTKinds.td
index a89bdff1a10c2..56662bcd0cc25 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -439,8 +439,6 @@ def note_odr_tag_kind_here: Note<
 def note_odr_field : Note<"field %0 has type %1 here">;
 def note_odr_field_name : Note<"field has name %0 here">;
 def note_odr_missing_field : Note<"no corresponding field here">;
-def note_odr_bit_field : Note<"bit-field %0 with type %1 and length %2 here">;
-def note_odr_not_bit_field : Note<"field %0 is not a bit-field">;
 def note_odr_base : Note<"class has base type %0">;
 def note_odr_virtual_base : Note<
   "%select{non-virtual|virtual}0 derivation here">;

diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index b688c121b1c07..276e83434d030 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -29,8 +29,6 @@ def err_drv_invalid_arch_name : Error<
   "invalid arch name '%0'">;
 def err_drv_invalid_riscv_arch_name : Error<
   "invalid arch name '%0', %1">;
-def err_drv_invalid_riscv_ext_arch_name : Error<
-  "invalid arch name '%0', %1 '%2'">;
 def warn_drv_invalid_arch_name_with_suggestion : Warning<
   "ignoring invalid /arch: argument '%0'; for %select{64|32}1-bit expected one 
of %2">,
   InGroup;
@@ -302,7 +300,6 @@ def err_drv_optimization_remark_format : Error<
   "unknown remark serializer format: '%0'">;
 def err_drv_no_neon_modifier : Error<"[no]neon is not accepted as modifier, 
please use [no]simd instead">;
 def err_drv_invalid_omp_target : Error<"OpenMP target is invalid: '%0'">;
-def err_drv_debug_no_new_runtime : Error<"OpenMP target device debugging 
enabled with incompatible runtime">;
 def err_drv_incompatible_omp_arch : Error<"OpenMP target architecture '%0' 
pointer size is incompatible with host '%1'">;
 def err_drv_omp_host_ir_file_not_found : Error<
   "provided host compiler IR file '%0' is required to generate code for OpenMP 
"
@@ -326,10 +323,6 @@ def err_drv_unsupported_embed_bitcode
 : Error<"%0 is not supported with -fembed-bitcode">;
 def err_drv_bitcode_unsupported_on_toolchain : Error<
   "-fembed-bitcode is not supported on versions of iOS prior to 6.0">;
-def err_drv_negative_columns : Error<
-"invalid value '%1' in '%0', value must be 'none' or a positive integer">;
-def err_drv_small_columns : Error<
-"invalid value '%1' in '%0', value must be '%2' or greater">;
 
 def err_drv_invalid_malign_branch_EQ : Error<
   "invalid argument '%0' to -malign-branch=; each element must be one of: %1">;
@@ -531,7 +524,6 @@ def warn_drv_ps4_sdk_dir : Warning<
   "environment variable SCE_ORBIS_SDK_DIR is set, but points to invalid or 
nonexistent directory '%0'">,
   InGroup;
 
-def err_drv_unsupported_linker : Error<"unsupported value '%0' for -linker 
option">;
 def err_drv_defsym_invalid_format : Error<"defsym must be of the form: 
sym=value: %0">;
 def err_drv_defsym_invalid_symval : Error<"value is not an integer: %0">;
 def warn_drv_msvc_not_found : Warning<

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 8af1bed7b67f1..1719db4871ff3 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -556,9 +556,6 @@ def err_using_decl_can_not_refer_to_class_member : Error<
 def warn_cxx17_compat_using_decl_class_member_enumerator : Warning<
   "member using declaration naming a non-member enumerator is incompatible "
   "with C++ standards before C++20">, InGroup, DefaultIgnore;
-def ext_using_decl_class_member_enumerator : ExtWarn<
-  "member using declaration naming a non-member enumerator is "
-  "a C++20 extension">, InGroup;
 def err_using_enum_is_dependent : Error<
   "using-enum cannot name a dependent type">;
 def err_ambiguous_inherited_constructor : Error<
@@ -1696,8 +1693,6 @@ def err_missing_exception_specification : Error<
 def ext_missing_exception_specification : ExtWarn<
   err_missing_exception_specification.Text>,
   InGroup>;
-def err_noexcept_needs_constant_expression : Error<
-  "argument to noexcept specifier must be a constant expression">;
 def err_exc

[clang] 51c0650 - Unionize clang::DynTypedNodeList. NFC.

[Benjamin Kramer via cfe-commits]

Author: Benjamin Kramer
Date: 2022-02-20T12:12:39+01:00
New Revision: 51c0650f6ba8128fb07036b4be8512bb5f727c1a

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

LOG: Unionize clang::DynTypedNodeList. NFC.

Added: 


Modified: 
clang/include/clang/AST/ParentMapContext.h

Removed: 




diff  --git a/clang/include/clang/AST/ParentMapContext.h 
b/clang/include/clang/AST/ParentMapContext.h
index 2edbc987850d2..3c2e2f9640ca3 100644
--- a/clang/include/clang/AST/ParentMapContext.h
+++ b/clang/include/clang/AST/ParentMapContext.h
@@ -90,29 +90,27 @@ class TraversalKindScope {
 /// Container for either a single DynTypedNode or for an ArrayRef to
 /// DynTypedNode. For use with ParentMap.
 class DynTypedNodeList {
-  llvm::AlignedCharArrayUnion> Storage;
+  union {
+DynTypedNode SingleNode;
+ArrayRef Nodes;
+  };
   bool IsSingleNode;
 
 public:
   DynTypedNodeList(const DynTypedNode &N) : IsSingleNode(true) {
-new (&Storage) DynTypedNode(N);
+new (&SingleNode) DynTypedNode(N);
   }
 
   DynTypedNodeList(ArrayRef A) : IsSingleNode(false) {
-new (&Storage) ArrayRef(A);
+new (&Nodes) ArrayRef(A);
   }
 
   const DynTypedNode *begin() const {
-if (!IsSingleNode)
-  return reinterpret_cast *>(&Storage)
-  ->begin();
-return reinterpret_cast(&Storage);
+return !IsSingleNode ? Nodes.begin() : &SingleNode;
   }
 
   const DynTypedNode *end() const {
-if (!IsSingleNode)
-  return reinterpret_cast *>(&Storage)->end();
-return reinterpret_cast(&Storage) + 1;
+return !IsSingleNode ? Nodes.end() : &SingleNode + 1;
   }
 
   size_t size() const { return end() - begin(); }



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

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

I tried to download the diff and apply it from the root of llvm-project, but I 
must be doing something wrong...

  $ git apply D117522.diff
  D117522.diff:808: trailing whitespace.
  - attempt to free an illegal 
  D117522.diff:809: trailing whitespace.
cmap entry 
  D117522.diff:810: trailing whitespace.
  - attempt to store into a read-only 
  D117522.diff:824: trailing whitespace.
  // CHECK-FIXES-NEXT: - attempt to free an 
illegal 
  D117522.diff:825: trailing whitespace.
  // CHECK-FIXES-NEXT:   cmap entry 
  error: clang-tidy/modernize/CMakeLists.txt: No such file or directory
  error: clang-tidy/modernize/ModernizeTidyModule.cpp: No such file or directory
  error: docs/ReleaseNotes.rst: No such file or directory
  error: docs/clang-tidy/checks/list.rst: No such file or directory




Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:382
+
+  if (std::strncmp("pragma", Text, 6) != 0)
+return;

Maybe wrap the raw strings inside a StringRef for a more robust and readable 
comparison? Not a big fan of the magic 6 there :) 



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:389
+
+  if (std::strncmp("once", Text, 4) == 0)
+CurrentFile->GuardScanner = IncludeGuard::IfGuard;

Same here



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-macro-to-enum.rst:7-8
+===
+
+The cppcoreguidelines-macro-to-enum check is an alias, please see
+:doc:`modernize-macro-to-enum ` for more information.

Would it make sense to mention this check covers the rule [[ 
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Renum-macro
 | Enum.1 ]]?

I see that we follow a standard pattern for aliases where we simply redirect to 
the main check, but maybe it's good to point this out anyway? Otherwise it 
might be unclear exactly which rule this check is referring to.

One drawback though is that aliases redirect to the main check after 5 
seconds...



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst:9-13
+
+This check can be used to enforce the C++ core guideline `Enum.1:
+Prefer enumerations over macros
+`_,
+within the constraints outlined below.

Oh, it's right here :) I suppose as a user I would expect to find this info in 
the cppcoreguidelines doc, not here. But again I don't know what the de-facto 
pattern is with aliases so I'll leave that to someone that knows better.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst:19
+- Macros must be defined on sequential source file lines, or with
+  only comment lines in between macro definitions.
+- Macros must all be defined in the same source file.

Hmm, what about this situation?


```
// This is some macro
#define FOO 123
// This is some other unrelated macro
#define BAR 321
```

Would the check group these 2 together? Personally I'd put an empty line 
between the two to show they are unrelated, but I know many people prefer to 
save vertical space and keep everything tight without empty lines.


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

https://reviews.llvm.org/D117522

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

[PATCH] D119562: [clang-tidy] Provide fine control of color in run-clang-tidy

[Nathan James via Phabricator via cfe-commits]

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

LGTM




Comment at: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py:69
+  val = val.lower()
+  if val in ['', 'true', '1']:
+return True

Didn't even realise an empty string was acceptable in LLVM CLI. Learn something 
new everyday.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119562

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

[PATCH] D120185: [ASTMatchers] Output currently processing match and nodes on crash

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:1536
 void MatchFinder::match(const clang::DynTypedNode &Node, ASTContext &Context) {
   internal::MatchASTVisitor Visitor(&Matchers, Options);
   Visitor.set_active_ast_context(&Context);

Is there a precedent to instrument this function too. AFAIK this is only used 
when running 1 matcher on a predetermined node so doesn't have the same issues 
when debugging as below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120185

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

[PATCH] D120185: [ASTMatchers] Output currently processing match and nodes on crash

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 410139.
njames93 added a comment.

Moved TraceReporter into the MatchASTVisitor class
Added some missing new lines
Fixed patch not being based off a commit in trunk


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120185

Files:
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/CMakeLists.txt
  clang-tools-extra/test/clang-tidy/CTCrashTestTrace.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/crash-trace.c
  clang-tools-extra/test/lit.cfg.py
  clang/lib/ASTMatchers/ASTMatchFinder.cpp

Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Timer.h"
 #include 
 #include 
@@ -760,11 +761,64 @@
 D);
   }
 
+  class TraceReporter : llvm::PrettyStackTraceEntry {
+  public:
+TraceReporter(const MatchASTVisitor &MV) : MV(MV) {}
+void print(raw_ostream &OS) const override {
+  if (!MV.CurMatched)
+return;
+  assert(MV.ActiveASTContext &&
+ "ActiveASTContext should be set if there is a matched callback");
+
+  OS << "Processing '" << MV.CurMatched->getID() << "'\n";
+  const BoundNodes::IDToNodeMap &Map = MV.CurBoundNodes->getMap();
+  if (Map.empty()) {
+OS << "No bound nodes\n";
+return;
+  }
+  OS << "--- Bound Nodes Begin ---\n";
+  for (const auto &Item : Map) {
+OS << "" << Item.first << " - { ";
+if (const auto *D = Item.second.get()) {
+  OS << D->getDeclKindName() << "Decl ";
+  if (const auto *ND = dyn_cast(D))
+OS << ND->getDeclName() << " : ";
+  else
+OS << " ";
+  D->getSourceRange().print(OS,
+MV.ActiveASTContext->getSourceManager());
+} else if (const auto *S = Item.second.get()) {
+  OS << S->getStmtClassName() << " : ";
+  S->getSourceRange().print(OS,
+MV.ActiveASTContext->getSourceManager());
+} else if (const auto *T = Item.second.get()) {
+  OS << T->getTypeClassName() << "Type : ";
+  QualType(T, 0).print(OS, MV.ActiveASTContext->getPrintingPolicy());
+} else if (const auto *QT = Item.second.get()) {
+  OS << "QualType : ";
+  QT->print(OS, MV.ActiveASTContext->getPrintingPolicy());
+} else {
+  OS << Item.second.getNodeKind().asStringRef() << " : ";
+  Item.second.getSourceRange().print(
+  OS, MV.ActiveASTContext->getSourceManager());
+}
+OS << " }\n";
+  }
+  OS << "--- Bound Nodes End ---\n";
+}
+
+  private:
+const MatchASTVisitor &MV;
+  };
+
 private:
   bool TraversingASTNodeNotSpelledInSource = false;
   bool TraversingASTNodeNotAsIs = false;
   bool TraversingASTChildrenNotSpelledInSource = false;
 
+  const MatchCallback *CurMatched = nullptr;
+  const BoundNodes *CurBoundNodes = nullptr;
+
   struct ASTNodeNotSpelledInSourceScope {
 ASTNodeNotSpelledInSourceScope(MatchASTVisitor *V, bool B)
 : MV(V), MB(V->TraversingASTNodeNotSpelledInSource) {
@@ -831,7 +885,7 @@
 Timer.setBucket(&TimeByBucket[MP.second->getID()]);
   BoundNodesTreeBuilder Builder;
   if (MP.first.matches(Node, this, &Builder)) {
-MatchVisitor Visitor(ActiveASTContext, MP.second);
+MatchVisitor Visitor(*this, ActiveASTContext, MP.second);
 Builder.visitMatches(&Visitor);
   }
 }
@@ -863,7 +917,7 @@
   }
 
   if (MP.first.matches(DynNode, this, &Builder)) {
-MatchVisitor Visitor(ActiveASTContext, MP.second);
+MatchVisitor Visitor(*this, ActiveASTContext, MP.second);
 Builder.visitMatches(&Visitor);
   }
 }
@@ -1049,18 +1103,36 @@
   // Implements a BoundNodesTree::Visitor that calls a MatchCallback with
   // the aggregated bound nodes for each match.
   class MatchVisitor : public BoundNodesTreeBuilder::Visitor {
+struct CurBoundScope {
+  CurBoundScope(MatchASTVisitor &MV, const BoundNodes &BN) : MV(MV) {
+assert(MV.CurMatched && !MV.CurBoundNodes);
+MV.CurBoundNodes = &BN;
+  }
+
+  ~CurBoundScope() { MV.CurBoundNodes = nullptr; }
+
+private:
+  MatchASTVisitor &MV;
+};
+
   public:
-MatchVisitor(ASTContext* Context,
- MatchFinder::MatchCallback* Callback)
-  : Context(Context),
-Callback(Callback) {}
+MatchVisitor(MatchASTVisitor &MV, ASTContext *Context,
+ MatchFinder::MatchCallback *Callback)
+: MV(MV), Context(Context), Callback(Callback) {
+  assert(!MV.C

[PATCH] D119095: [clang] Fix redundant functional cast in ConstantExpr

[Kim Gräsman via Phabricator via cfe-commits]

kimgr added a comment.

Having looked at this some more, I wonder if this patch is covering for another 
problem. Running even the simplest example fails on an assertion in 
`Sema::BuildCXXTypeConstructExpr`: https://godbolt.org/z/fMPcsh4f3.

It looks like that function is creating the majority of 
`CXXFunctionalCastExpr`s, so the way the patch shaves off 
`CXXFunctionalCastExpr` might be compensating for something that should never 
happen. I don't know much about this, so I can't say for sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119095

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

[PATCH] D113422: [clang-tidy][NFC] Move CachedGlobList to GlobList.h

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D113422#992 , @carlosgalvezp 
wrote:

> Nothing other than readability, `CachedGlobList` //is a// `GlobList` so it 
> made sense to implement it with standard, by-the-book inheritance. But no, 
> it's not used polymorphically at the moment so technically `virtual` could be 
> removed, even though it would probably go against developer expectations and 
> might cause confusion for the above reasons in the future. Perhaps doing 
> private instead of public inheritance (//is implemented in terms of//) would 
> communicate the design better in that case?
>
> Do you have profiling data that shows how much overhead the virtual function 
> call actually adds in this particular case? In the projects I've worked with 
> this has typically been a micro-optimization done prematurely without 
> profiling data that supported the need for it, so the benefit was negligible. 
> It did cause a lot of headache to developers reading the code though.

It's definitely negligible as reading globs isn't exactly a hot path, was more 
just an irk when i was looking over the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113422

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

[clang] 6738792 - Revert "[C++20][Modules][1/8] Track valid import state."

[Iain Sandoe via cfe-commits]

Author: Iain Sandoe
Date: 2022-02-20T10:22:07Z
New Revision: 673879249d4d1c4e6d763a6db4a4812d721b41b6

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

LOG: Revert "[C++20][Modules][1/8] Track valid import state."

This reverts commit 8a3f9a584ad43369cf6a034dc875ebfca76d9033.

need to investigate build failures that do not show on CI or local
testing.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Parse/Parser.h
clang/include/clang/Sema/Sema.h
clang/lib/Interpreter/IncrementalParser.cpp
clang/lib/Parse/ParseAST.cpp
clang/lib/Parse/ParseObjc.cpp
clang/lib/Parse/Parser.cpp
clang/lib/Sema/SemaModule.cpp

Removed: 
clang/test/Modules/cxx20-import-diagnostics-a.cpp



diff  --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index f21e841bcdd38..e23810f402365 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1539,10 +1539,6 @@ def err_private_module_fragment_expected_semi : Error<
 def err_missing_before_module_end : Error<"expected %0 at end of module">;
 def err_unsupported_module_partition : Error<
   "sorry, module partitions are not yet supported">;
-def err_import_not_allowed_here : Error<
-  "imports must immediately follow the module declaration">;
-def err_import_in_wrong_fragment : Error<
-  "module%select{| partition}0 imports cannot be in the 
%select{global|private}1 module fragment">;
 
 def err_export_empty : Error<"export declaration cannot be empty">;
 }

diff  --git a/clang/include/clang/Parse/Parser.h 
b/clang/include/clang/Parse/Parser.h
index 08d492a7ec721..981800a7e2356 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -464,17 +464,14 @@ class Parser : public CodeCompletionHandler {
   void Initialize();
 
   /// Parse the first top-level declaration in a translation unit.
-  bool ParseFirstTopLevelDecl(DeclGroupPtrTy &Result,
-  Sema::ModuleImportState &ImportState);
+  bool ParseFirstTopLevelDecl(DeclGroupPtrTy &Result);
 
   /// ParseTopLevelDecl - Parse one top-level declaration. Returns true if
   /// the EOF was encountered.
-  bool ParseTopLevelDecl(DeclGroupPtrTy &Result,
- Sema::ModuleImportState &ImportState);
+  bool ParseTopLevelDecl(DeclGroupPtrTy &Result, bool IsFirstDecl = false);
   bool ParseTopLevelDecl() {
 DeclGroupPtrTy Result;
-Sema::ModuleImportState IS = Sema::ModuleImportState::NotACXX20Module;
-return ParseTopLevelDecl(Result, IS);
+return ParseTopLevelDecl(Result);
   }
 
   /// ConsumeToken - Consume the current 'peek token' and lex the next one.
@@ -3494,9 +3491,8 @@ class Parser : public CodeCompletionHandler {
 
   
//======//
   // Modules
-  DeclGroupPtrTy ParseModuleDecl(Sema::ModuleImportState &ImportState);
-  Decl *ParseModuleImport(SourceLocation AtLoc,
-  Sema::ModuleImportState &ImportState);
+  DeclGroupPtrTy ParseModuleDecl(bool IsFirstDecl);
+  Decl *ParseModuleImport(SourceLocation AtLoc);
   bool parseMisplacedModuleImport();
   bool tryParseMisplacedModuleImport() {
 tok::TokenKind Kind = Tok.getKind();

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index dfa12ad40b72a..c1e846c55dee7 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2949,24 +2949,11 @@ class Sema final {
 Implementation, ///< 'module X;'
   };
 
-  /// An enumeration to represent the transition of states in parsing module
-  /// fragments and imports.  If we are not parsing a C++20 TU, or we find
-  /// an error in state transition, the state is set to NotACXX20Module.
-  enum class ModuleImportState {
-FirstDecl,   ///< Parsing the first decl in a TU.
-GlobalFragment,  ///< after 'module;' but before 'module X;'
-ImportAllowed,   ///< after 'module X;' but before any non-import decl.
-ImportFinished,  ///< after any non-import decl.
-PrivateFragment, ///< after 'module :private;'.
-NotACXX20Module  ///< Not a C++20 TU, or an invalid state was found.
-  };
-
   /// The parser has processed a module-declaration that begins the definition
   /// of a module interface or implementation.
   DeclGroupPtrTy ActOnModuleDecl(SourceLocation StartLoc,
  SourceLocation ModuleLoc, ModuleDeclKind MDK,
- ModuleIdPath Path,
- ModuleImportState &ImportState);
+ ModuleIdPath Path, bool IsFirstDecl);
 
   /// The parser has processed a g

[PATCH] D113422: [clang-tidy][NFC] Move CachedGlobList to GlobList.h

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

In D113422#748 , @njames93 wrote:

> Was there any real use case for making this polymorphic, The overhead for a 
> virtual function call seems a little unnecessary IMO?

Nothing other than readability, `CachedGlobList` //is a// `GlobList` so it made 
sense to implement it with standard, by-the-book inheritance. But no, it's not 
used polymorphically at the moment so technically `virtual` could be removed, 
even though it would probably go against developer expectations and might cause 
confusion for the above reasons in the future. Perhaps doing private instead of 
public inheritance (//is implemented in terms of//) would communicate the 
design better in that case?

Do you have profiling data that shows how much overhead the virtual function 
call actually adds in this particular case? In the projects I've worked with 
this has typically been a micro-optimization done prematurely without profiling 
data that supported the need for it, so the benefit was negligible. It did 
cause a lot of headache to developers reading the code though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113422

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

[PATCH] D118893: [C++20][Modules][1/8] Track valid import state.

[Iain Sandoe via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8a3f9a584ad4: [C++20][Modules][1/8] Track valid import 
state. (authored by iains).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118893

Files:
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/Modules/cxx20-import-diagnostics-a.cpp

Index: clang/test/Modules/cxx20-import-diagnostics-a.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-import-diagnostics-a.cpp
@@ -0,0 +1,140 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=0 -x c++ %s \
+// RUN:  -o %t/B.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=1 -x c++ %s \
+// RUN:  -o %t/C.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=2 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -fmodule-file=%t/C.pcm -o %t/AOK1.pcm
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=3 -x c++ %s \
+// RUN:  -fmodule-file=%t/AOK1.pcm -o %t/tu_3.s -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=4 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -fmodule-file=%t/C.pcm -o %t/BC.pcm -verify
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=5 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -fmodule-file=%t/C.pcm -o %t/tu_5.s -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=6 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -o %t/D.pcm -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=7 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -o %t/D.pcm -verify
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=8 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm -o %t/tu_8.s -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=9 -x c++ %s \
+// RUN:  -o %t/B.pcm -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj -D TU=10 -x c++ %s \
+// RUN:  -fmodule-file=%t/C.pcm  -o %t/impl.o
+
+// Test diagnostics for incorrect module import sequences.
+
+#if TU == 0
+
+export module B;
+
+int foo ();
+
+// expected-no-diagnostics
+
+#elif TU == 1
+
+export module C;
+
+int bar ();
+
+// expected-no-diagnostics
+
+#elif TU == 2
+
+export module AOK1;
+
+import B;
+export import C;
+
+export int theAnswer ();
+
+// expected-no-diagnostics
+
+#elif TU == 3
+
+module;
+
+module AOK1;
+
+export import C; // expected-error {{export declaration can only be used within a module interface unit}}
+
+int theAnswer () { return 42; }
+
+#elif TU == 4
+
+export module BC;
+
+export import B;
+
+int foo () { return 10; }
+
+import C; // expected-error {{imports must immediately follow the module declaration}}
+
+#elif TU == 5
+
+module B; // implicitly imports B.
+
+int foo () { return 10; }
+
+import C; // expected-error {{imports must immediately follow the module declaration}}
+
+#elif TU == 6
+
+module;
+// We can only have preprocessor commands here, which could include an include
+// translated header unit.  However those are identified specifically by the
+// preprocessor; non-preprocessed user code should not contain an import here.
+import B; // expected-error {{module imports cannot be in the global module fragment}}
+
+export module D;
+
+int delta ();
+
+#elif TU == 7
+
+export module D;
+
+int delta ();
+
+module :private;
+
+import B; // expected-error {{module imports cannot be in the private module fragment}}
+
+#elif TU == 8
+
+module B;
+
+import B; // expected-error {{import of module 'B' appears within same top-level module 'B'}}
+
+#elif TU == 9
+
+export module B;
+
+import B; // expected-error {{import of module 'B' appears within same top-level module 'B'}}
+
+#elif TU == 10
+
+int x;
+
+import C;
+
+int baz() { return 6174; }
+
+// expected-no-diagnostics
+
+#else
+#error "no MODE set"
+#endif
Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -80,12 +80,20 @@
   return nullptr;
 }
 
-Sema::DeclGroupPtrTy
-Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
-  ModuleDeclKind MDK, ModuleIdPath Path, bool IsFirstDecl) {
+Sema::DeclGroupPtrTy Sema::ActOnModuleDecl(SourceLocation StartLoc,
+   SourceLocation ModuleLoc,
+   ModuleDeclKind MDK,
+   ModuleIdPath Path,
+   ModuleImportState &ImportState) {
   assert((getLangOpts().ModulesTS || getLangOpts().CPlusPlusModules) &&
  "should only have module decl in Modules TS or C++20");
 
+  bool IsFirstDecl =

[clang] 8a3f9a5 - [C++20][Modules][1/8] Track valid import state.

[Iain Sandoe via cfe-commits]

Author: Iain Sandoe
Date: 2022-02-20T10:13:57Z
New Revision: 8a3f9a584ad43369cf6a034dc875ebfca76d9033

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

LOG: [C++20][Modules][1/8] Track valid import state.

In C++20 modules imports must be together and at the start of the module.
Rather than growing more ad-hoc flags to test state, this keeps track of the
phase of of a valid module TU (first decl, global module frag, module,
private module frag).  If the phasing is broken (with some diagnostic) the
pattern does not conform to a valid C++20 module, and we set the state
accordingly.

We can thus issue diagnostics when imports appear in the wrong places and
decouple the C++20 modules state from other module variants (modules-ts and
clang modules).  Additionally, we attempt to diagnose wrong imports before
trying to find the module where possible (the latter will generally emit an
unhelpful diagnostic about the module not being available).

Although this generally simplifies the handling of C++20 module import
diagnostics, the motivation was that, in particular, it allows detecting
invalid imports like:

import module A;

int some_decl();

import module B;

where being in a module purview is insufficient to identify them.

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

Added: 
clang/test/Modules/cxx20-import-diagnostics-a.cpp

Modified: 
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Parse/Parser.h
clang/include/clang/Sema/Sema.h
clang/lib/Interpreter/IncrementalParser.cpp
clang/lib/Parse/ParseAST.cpp
clang/lib/Parse/ParseObjc.cpp
clang/lib/Parse/Parser.cpp
clang/lib/Sema/SemaModule.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index e23810f402365..f21e841bcdd38 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1539,6 +1539,10 @@ def err_private_module_fragment_expected_semi : Error<
 def err_missing_before_module_end : Error<"expected %0 at end of module">;
 def err_unsupported_module_partition : Error<
   "sorry, module partitions are not yet supported">;
+def err_import_not_allowed_here : Error<
+  "imports must immediately follow the module declaration">;
+def err_import_in_wrong_fragment : Error<
+  "module%select{| partition}0 imports cannot be in the 
%select{global|private}1 module fragment">;
 
 def err_export_empty : Error<"export declaration cannot be empty">;
 }

diff  --git a/clang/include/clang/Parse/Parser.h 
b/clang/include/clang/Parse/Parser.h
index 981800a7e2356..08d492a7ec721 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -464,14 +464,17 @@ class Parser : public CodeCompletionHandler {
   void Initialize();
 
   /// Parse the first top-level declaration in a translation unit.
-  bool ParseFirstTopLevelDecl(DeclGroupPtrTy &Result);
+  bool ParseFirstTopLevelDecl(DeclGroupPtrTy &Result,
+  Sema::ModuleImportState &ImportState);
 
   /// ParseTopLevelDecl - Parse one top-level declaration. Returns true if
   /// the EOF was encountered.
-  bool ParseTopLevelDecl(DeclGroupPtrTy &Result, bool IsFirstDecl = false);
+  bool ParseTopLevelDecl(DeclGroupPtrTy &Result,
+ Sema::ModuleImportState &ImportState);
   bool ParseTopLevelDecl() {
 DeclGroupPtrTy Result;
-return ParseTopLevelDecl(Result);
+Sema::ModuleImportState IS = Sema::ModuleImportState::NotACXX20Module;
+return ParseTopLevelDecl(Result, IS);
   }
 
   /// ConsumeToken - Consume the current 'peek token' and lex the next one.
@@ -3491,8 +3494,9 @@ class Parser : public CodeCompletionHandler {
 
   
//======//
   // Modules
-  DeclGroupPtrTy ParseModuleDecl(bool IsFirstDecl);
-  Decl *ParseModuleImport(SourceLocation AtLoc);
+  DeclGroupPtrTy ParseModuleDecl(Sema::ModuleImportState &ImportState);
+  Decl *ParseModuleImport(SourceLocation AtLoc,
+  Sema::ModuleImportState &ImportState);
   bool parseMisplacedModuleImport();
   bool tryParseMisplacedModuleImport() {
 tok::TokenKind Kind = Tok.getKind();

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index c1e846c55dee7..dfa12ad40b72a 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2949,11 +2949,24 @@ class Sema final {
 Implementation, ///< 'module X;'
   };
 
+  /// An enumeration to represent the transition of states in parsing module
+  /// fragments and imports.  If we are not parsing a C++20 TU, or we find
+  /// an error in state transition, the state is set t

[PATCH] D120188: Fix extraneous whitespace addition in line comments on clang-format directives

[Marek Kurdej via Phabricator via cfe-commits]

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

Great! Thanks a lot!
Do you have commit rights or you want someone to land it for you?
For the latter, we'll need your name and email for the commit attribution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120188

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

[PATCH] D111400: [Clang] Implement P2242R3

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

@hubert.reinterpretcast Gentle ping in case you didn't see Aaron's message - 
there isn't too much urgency though


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

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

[PATCH] D120205: Restore documentation for __builtin_assume

[serge via Phabricator via cfe-commits]

serge-sans-paille created this revision.
serge-sans-paille added reviewers: aaron.ballman, vdelvecc.
serge-sans-paille requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This got removed by 6cacd420a1d72bca7809e6b516fb1e18ac6056c8 
, and that 
was a mistake.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120205

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -2205,6 +2205,39 @@
 
 .. _langext-__builtin_assume:
 
+``__builtin_assume``
+
+
+``__builtin_assume`` is used to provide the optimizer with a boolean
+invariant that is defined to be true.
+
+**Syntax**:
+
+.. code-block:: c++
+
+__builtin_assume(bool)
+
+**Example of Use**:
+
+.. code-block:: c++
+
+  int foo(int x) {
+  __builtin_assume(x != 0);
+  // The optimizer may short-circuit this check using the invariant.
+  if (x == 0)
+return do_something();
+  return do_something_else();
+  }
+
+**Description**:
+
+The boolean argument to this function is defined to be true. The optimizer may
+analyze the form of the expression provided as the argument and deduce from
+that information used to optimize the program. If the condition is violated
+during execution, the behavior is undefined. The argument itself is 
+
+Query for this feature with ``__has_builtin(__builtin_assume)``.
+
 ``__builtin_call_with_static_chain``
 
 


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -2205,6 +2205,39 @@
 
 .. _langext-__builtin_assume:
 
+``__builtin_assume``
+
+
+``__builtin_assume`` is used to provide the optimizer with a boolean
+invariant that is defined to be true.
+
+**Syntax**:
+
+.. code-block:: c++
+
+__builtin_assume(bool)
+
+**Example of Use**:
+
+.. code-block:: c++
+
+  int foo(int x) {
+  __builtin_assume(x != 0);
+  // The optimizer may short-circuit this check using the invariant.
+  if (x == 0)
+return do_something();
+  return do_something_else();
+  }
+
+**Description**:
+
+The boolean argument to this function is defined to be true. The optimizer may
+analyze the form of the expression provided as the argument and deduce from
+that information used to optimize the program. If the condition is violated
+during execution, the behavior is undefined. The argument itself is 
+
+Query for this feature with ``__has_builtin(__builtin_assume)``.
+
 ``__builtin_call_with_static_chain``
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits