[PATCH] D125622: [clang-tidy] Reject invalid enum initializers in C files

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood created this revision.
LegalizeAdulthood added reviewers: aaron.ballman, njames93.
LegalizeAdulthood added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
LegalizeAdulthood requested review of this revision.

C requires that enum values fit into an int.  Scan the macro tokens
present in an initializing expression and reject macros that contain
tokens that have suffixes making them larger than int.

C forbids the comma operator in enum initializing expressions, so
optionally reject comma operator.

Fixes #55467


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125622

Files:
  clang-tools-extra/clang-tidy/modernize/IntegralLiteralExpressionMatcher.cpp
  clang-tools-extra/clang-tidy/modernize/IntegralLiteralExpressionMatcher.h
  clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.c
  clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp
@@ -36,177 +36,237 @@
   return Tokens;
 }
 
-static bool matchText(const char *Text) {
+static bool matchText(const char *Text, bool AllowComma) {
   std::vector Tokens{tokenify(Text)};
-  modernize::IntegralLiteralExpressionMatcher Matcher(Tokens);
+  modernize::IntegralLiteralExpressionMatcher Matcher(Tokens, AllowComma);
 
   return Matcher.match();
 }
 
+static modernize::LiteralSize sizeText(const char *Text) {
+  std::vector Tokens{tokenify(Text)};
+  modernize::IntegralLiteralExpressionMatcher Matcher(Tokens, true);
+  if (Matcher.match())
+return Matcher.largestLiteralSize();
+  return modernize::LiteralSize::Unknown;
+}
+
+static const char *toString(modernize::LiteralSize Value) {
+  switch (Value) {
+  case modernize::LiteralSize::Int:
+return "Int";
+  case modernize::LiteralSize::UnsignedInt:
+return "UnsignedInt";
+  case modernize::LiteralSize::Long:
+return "Long";
+  case modernize::LiteralSize::UnsignedLong:
+return "UnsignedLong";
+  case modernize::LiteralSize::LongLong:
+return "LongLong";
+  case modernize::LiteralSize::UnsignedLongLong:
+return "UnsignedLongLong";
+  default:
+return "Unknown";
+  }
+}
+
 namespace {
 
-struct Param {
+struct MatchParam {
+  bool AllowComma;
   bool Matched;
   const char *Text;
 
-  friend std::ostream <<(std::ostream , const Param ) {
-return Str << "Matched: " << std::boolalpha << Value.Matched << ", Text: '"
-   << Value.Text << "'";
+  friend std::ostream <<(std::ostream , const MatchParam ) {
+return Str << "Allow operator,: " << std::boolalpha << Value.AllowComma
+   << ", Matched: " << std::boolalpha << Value.Matched
+   << ", Text: '" << Value.Text << '\'';
   }
 };
 
-class MatcherTest : public ::testing::TestWithParam {};
+struct SizeParam {
+  modernize::LiteralSize Size;
+  const char *Text;
+
+  friend std::ostream <<(std::ostream , const SizeParam ) {
+return Str << "Size: " << toString(Value.Size) << ", Text: '" << Value.Text << '\'';
+  }
+};
+
+class MatcherTest : public ::testing::TestWithParam {};
+
+class SizeTest : public ::testing::TestWithParam {};
 
 } // namespace
 
-static const Param Params[] = {
+static const MatchParam MatchParams[] = {
 // Accept integral literals.
-{true, "1"},
-{true, "0177"},
-{true, "0xdeadbeef"},
-{true, "0b1011"},
-{true, "'c'"},
+{true, true, "1"},
+{true, true, "0177"},
+{true, true, "0xdeadbeef"},
+{true, true, "0b1011"},
+{true, true, "'c'"},
 // Reject non-integral literals.
-{false, "1.23"},
-{false, "0x1p3"},
-{false, R"("string")"},
-{false, "1i"},
+{true, false, "1.23"},
+{true, false, "0x1p3"},
+{true, false, R"("string")"},
+{true, false, "1i"},
 
 // Accept literals with these unary operators.
-{true, "-1"},
-{true, "+1"},
-{true, "~1"},
-{true, "!1"},
+{true, true, "-1"},
+{true, true, "+1"},
+{true, true, "~1"},
+{true, true, "!1"},
 // Reject invalid unary operators.
-{false, "1-"},
-{false, "1+"},
-{false, "1~"},
-{false, "1!"},
+{true, false, "1-"},
+{true, false, "1+"},
+{true, false, "1~"},
+{true, false, "1!"},
 
 // Accept valid binary operators.
-{true, "1+1"},
-{true, "1-1"},
-{true, "1*1"},
-{true, "1/1"},
-{true, "1%2"},
-{true, "1<<1"},
-{true, "1>>1"},
-{true, "1<=>1"},
-{true, "1<1"},
-{true, "1>1"},
-{true, "1<=1"},
-{true, "1>=1"},
-{true, "1==1"},
-{true, "1!=1"},
-{true, "1&1"},
-{true, "1^1"},
-{true, "1|1"},
-{true, "1&&1"},
-{true, "1||1"},
-{true, "1+ +1"}, // A space is 

[PATCH] D106888: [LowerTypeTests][clang] Implement cfi-icall and allow -fsanitize=cfi-icall for RISCV

[Fangrui Song 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 rG59afc4038b10: [LowerTypeTests][clang] Implement and allow 
-fsanitize=cfi-icall for RISCV (authored by twd2, committed by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106888

Files:
  clang/lib/Driver/ToolChain.cpp
  clang/test/Driver/fsanitize.c
  llvm/lib/Transforms/IPO/LowerTypeTests.cpp
  llvm/test/Transforms/LowerTypeTests/function-weak.ll
  llvm/test/Transforms/LowerTypeTests/function.ll

Index: llvm/test/Transforms/LowerTypeTests/function.ll
===
--- llvm/test/Transforms/LowerTypeTests/function.ll
+++ llvm/test/Transforms/LowerTypeTests/function.ll
@@ -5,6 +5,8 @@
 ; RUN: opt -S -lowertypetests -mtriple=arm-unknown-linux-gnu < %s | FileCheck --check-prefixes=ARM,NATIVE %s
 ; RUN: opt -S -lowertypetests -mtriple=thumb-unknown-linux-gnu < %s | FileCheck --check-prefixes=THUMB,NATIVE %s
 ; RUN: opt -S -lowertypetests -mtriple=aarch64-unknown-linux-gnu < %s | FileCheck --check-prefixes=ARM,NATIVE %s
+; RUN: opt -S -lowertypetests -mtriple=riscv32-unknown-linux-gnu < %s | FileCheck --check-prefixes=RISCV,NATIVE %s
+; RUN: opt -S -lowertypetests -mtriple=riscv64-unknown-linux-gnu < %s | FileCheck --check-prefixes=RISCV,NATIVE %s
 ; RUN: opt -S -lowertypetests -mtriple=wasm32-unknown-unknown < %s | FileCheck --check-prefix=WASM32 %s
 
 ; Tests that we correctly handle bitsets containing 2 or more functions.
@@ -23,6 +25,7 @@
 ; X86: @g = internal alias void (), bitcast ([8 x i8]* getelementptr inbounds ([2 x [8 x i8]], [2 x [8 x i8]]* bitcast (void ()* @[[JT]] to [2 x [8 x i8]]*), i64 0, i64 1) to void ()*)
 ; ARM: @g = internal alias void (), bitcast ([4 x i8]* getelementptr inbounds ([2 x [4 x i8]], [2 x [4 x i8]]* bitcast (void ()* @[[JT]] to [2 x [4 x i8]]*), i64 0, i64 1) to void ()*)
 ; THUMB: @g = internal alias void (), bitcast ([4 x i8]* getelementptr inbounds ([2 x [4 x i8]], [2 x [4 x i8]]* bitcast (void ()* @[[JT]] to [2 x [4 x i8]]*), i64 0, i64 1) to void ()*)
+; RISCV: @g = internal alias void (), bitcast ([8 x i8]* getelementptr inbounds ([2 x [8 x i8]], [2 x [8 x i8]]* bitcast (void ()* @[[JT]] to [2 x [8 x i8]]*), i64 0, i64 1) to void ()*)
 
 ; NATIVE: define hidden void @f.cfi()
 ; WASM32: define void @f() !type !{{[0-9]+}} !wasm.index ![[I0:[0-9]+]]
@@ -52,6 +55,7 @@
 ; X86-WIN32:   define private void @[[JT]]() #[[ATTR:.*]] align 8 {
 ; ARM:   define private void @[[JT]]() #[[ATTR:.*]] align 4 {
 ; THUMB: define private void @[[JT]]() #[[ATTR:.*]] align 4 {
+; RISCV: define private void @[[JT]]() #[[ATTR:.*]] align 8 {
 
 ; X86:  jmp ${0:c}@plt
 ; X86-SAME: int3
@@ -68,12 +72,16 @@
 ; THUMB:  b.w $0
 ; THUMB-SAME: b.w $1
 
+; RISCV:  tail $0@plt
+; RISCV-SAME: tail $1@plt
+
 ; NATIVE-SAME: "s,s"(void ()* @f.cfi, void ()* @g.cfi)
 
 ; X86-LINUX: attributes #[[ATTR]] = { naked nounwind }
 ; X86-WIN32: attributes #[[ATTR]] = { nounwind }
 ; ARM: attributes #[[ATTR]] = { naked nounwind
 ; THUMB: attributes #[[ATTR]] = { naked nounwind "target-cpu"="cortex-a8" "target-features"="+thumb-mode" }
+; RISCV: attributes #[[ATTR]] = { naked nounwind "target-features"="-c,-relax" }
 
 ; WASM32: ![[I0]] = !{i64 1}
 ; WASM32: ![[I1]] = !{i64 2}
Index: llvm/test/Transforms/LowerTypeTests/function-weak.ll
===
--- llvm/test/Transforms/LowerTypeTests/function-weak.ll
+++ llvm/test/Transforms/LowerTypeTests/function-weak.ll
@@ -2,6 +2,8 @@
 ; RUN: opt -S -lowertypetests -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck --check-prefixes=CHECK,X86 %s
 ; RUN: opt -S -lowertypetests -mtriple=arm-unknown-linux-gnu < %s | FileCheck --check-prefixes=CHECK,ARM %s
 ; RUN: opt -S -lowertypetests -mtriple=aarch64-unknown-linux-gnu < %s | FileCheck --check-prefixes=CHECK,ARM %s
+; RUN: opt -S -lowertypetests -mtriple=riscv32-unknown-linux-gnu < %s | FileCheck --check-prefixes=CHECK,RISCV %s
+; RUN: opt -S -lowertypetests -mtriple=riscv64-unknown-linux-gnu < %s | FileCheck --check-prefixes=CHECK,RISCV %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
@@ -52,6 +54,7 @@
 
 ; X86: define private void @[[JT]]() #{{.*}} align 8 {
 ; ARM: define private void @[[JT]]() #{{.*}} align 4 {
+; RISCV: define private void @[[JT]]() #{{.*}} align 8 {
 
 ; CHECK: define internal void @__cfi_global_var_init() section ".text.startup" {
 ; CHECK-NEXT: entry:
Index: llvm/lib/Transforms/IPO/LowerTypeTests.cpp
===
--- llvm/lib/Transforms/IPO/LowerTypeTests.cpp
+++ llvm/lib/Transforms/IPO/LowerTypeTests.cpp
@@ -1223,6 +1223,7 @@
 static const unsigned kX86JumpTableEntrySize = 8;
 static const unsigned kARMJumpTableEntrySize = 4;
 static 

[clang] 59afc40 - [LowerTypeTests][clang] Implement and allow -fsanitize=cfi-icall for RISCV

[Fangrui Song via cfe-commits]

Author: Wende Tan
Date: 2022-05-14T18:05:06-07:00
New Revision: 59afc4038b1096bc6fea7b322091f6e5e2dc0b38

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

LOG: [LowerTypeTests][clang] Implement and allow -fsanitize=cfi-icall for RISCV

Reviewed By: MaskRay

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

Added: 


Modified: 
clang/lib/Driver/ToolChain.cpp
clang/test/Driver/fsanitize.c
llvm/lib/Transforms/IPO/LowerTypeTests.cpp
llvm/test/Transforms/LowerTypeTests/function-weak.ll
llvm/test/Transforms/LowerTypeTests/function.ll

Removed: 




diff  --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 1b221974a0147..b10c7de96d788 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -1043,7 +1043,7 @@ SanitizerMask ToolChain::getSupportedSanitizers() const {
   if (getTriple().getArch() == llvm::Triple::x86 ||
   getTriple().getArch() == llvm::Triple::x86_64 ||
   getTriple().getArch() == llvm::Triple::arm || getTriple().isWasm() ||
-  getTriple().isAArch64())
+  getTriple().isAArch64() || getTriple().isRISCV())
 Res |= SanitizerKind::CFIICall;
   if (getTriple().getArch() == llvm::Triple::x86_64 ||
   getTriple().isAArch64(64) || getTriple().isRISCV())

diff  --git a/clang/test/Driver/fsanitize.c b/clang/test/Driver/fsanitize.c
index 4b03bfc9dcb73..75587f8ddc5f0 100644
--- a/clang/test/Driver/fsanitize.c
+++ b/clang/test/Driver/fsanitize.c
@@ -584,6 +584,8 @@
 // RUN: %clang -target arm-linux-android -fvisibility=hidden -fsanitize=cfi 
-flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI
 // RUN: %clang -target aarch64-linux-android -fvisibility=hidden 
-fsanitize=cfi -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI
 // RUN: %clang -target aarch64_be -fvisibility=hidden -fsanitize=cfi -flto -c 
%s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI
+// RUN: %clang -target riscv32 -fvisibility=hidden -fsanitize=cfi -flto -c %s 
-### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI
+// RUN: %clang -target riscv64 -fvisibility=hidden -fsanitize=cfi -flto -c %s 
-### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI
 // CHECK-CFI: 
-emit-llvm-bc{{.*}}-fsanitize=cfi-derived-cast,cfi-icall,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall
 // CHECK-CFI-NOMFCALL: 
-emit-llvm-bc{{.*}}-fsanitize=cfi-derived-cast,cfi-icall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall
 // CHECK-CFI-DCAST: -emit-llvm-bc{{.*}}-fsanitize=cfi-derived-cast

diff  --git a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp 
b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
index 8e83d7bcb6c23..d089f8c9c7c48 100644
--- a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
+++ b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
@@ -1223,6 +1223,7 @@ void LowerTypeTestsModule::verifyTypeMDNode(GlobalObject 
*GO, MDNode *Type) {
 static const unsigned kX86JumpTableEntrySize = 8;
 static const unsigned kARMJumpTableEntrySize = 4;
 static const unsigned kARMBTIJumpTableEntrySize = 8;
+static const unsigned kRISCVJumpTableEntrySize = 8;
 
 unsigned LowerTypeTestsModule::getJumpTableEntrySize() {
   switch (Arch) {
@@ -1238,6 +1239,9 @@ unsigned LowerTypeTestsModule::getJumpTableEntrySize() {
 if (BTE->getZExtValue())
   return kARMBTIJumpTableEntrySize;
   return kARMJumpTableEntrySize;
+case Triple::riscv32:
+case Triple::riscv64:
+  return kRISCVJumpTableEntrySize;
 default:
   report_fatal_error("Unsupported architecture for jump tables");
   }
@@ -1265,6 +1269,9 @@ void LowerTypeTestsModule::createJumpTableEntry(
 AsmOS << "b $" << ArgIndex << "\n";
   } else if (JumpTableArch == Triple::thumb) {
 AsmOS << "b.w $" << ArgIndex << "\n";
+  } else if (JumpTableArch == Triple::riscv32 ||
+ JumpTableArch == Triple::riscv64) {
+AsmOS << "tail $" << ArgIndex << "@plt\n";
   } else {
 report_fatal_error("Unsupported architecture for jump tables");
   }
@@ -1282,7 +1289,8 @@ Type *LowerTypeTestsModule::getJumpTableEntryType() {
 void LowerTypeTestsModule::buildBitSetsFromFunctions(
 ArrayRef TypeIds, ArrayRef Functions) {
   if (Arch == Triple::x86 || Arch == Triple::x86_64 || Arch == Triple::arm ||
-  Arch == Triple::thumb || Arch == Triple::aarch64)
+  Arch == Triple::thumb || Arch == Triple::aarch64 ||
+  Arch == Triple::riscv32 || Arch == Triple::riscv64)
 buildBitSetsFromFunctionsNative(TypeIds, Functions);
   else if (Arch == Triple::wasm32 || Arch == Triple::wasm64)
 buildBitSetsFromFunctionsWASM(TypeIds, Functions);
@@ -1427,6 +1435,11 @@ void LowerTypeTestsModule::createJumpTable(
 F->addFnAttr("branch-target-enforcement", "false");
 F->addFnAttr("sign-return-address", "none");
   }
+  if (JumpTableArch == Triple::riscv32 || 

[PATCH] D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum

[Richard via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clang-tidy/modernize/IntegralLiteralExpressionMatcher.cpp:204
+  return true;
+}
+

LegalizeAdulthood wrote:
> aaron.ballman wrote:
> > LegalizeAdulthood wrote:
> > > LegalizeAdulthood wrote:
> > > > aaron.ballman wrote:
> > > > > LegalizeAdulthood wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > Comma operator?
> > > > > > Remember that the use case here is identifying expressions that are 
> > > > > > initializers for an enum.  If you were doing a code review and you 
> > > > > > saw this:
> > > > > > ```
> > > > > > enum {
> > > > > > FOO = (2, 3)
> > > > > > };
> > > > > > ```
> > > > > > Would you be OK with that?  I wouldn't.  Clang even warns about it: 
> > > > > > https://godbolt.org/z/Y641cb8Y9
> > > > > > 
> > > > > > Therefore I deliberately left comma operator out of the grammar.
> > > > > This is another case where I think you're predicting that users won't 
> > > > > be using the full expressivity of the language and we'll get bug 
> > > > > reports later. Again, in insolation, I tend to agree that I wouldn't 
> > > > > be happy seeing that code. However, users write some very creative 
> > > > > code and there's no technical reason why we can't or shouldn't handle 
> > > > > comma operators.
> > > > "Don't let the perfect be the enemy of the good."
> > > > 
> > > > My inclination is to simply explicitly state that comma operator is not 
> > > > recognized in the documentation.  It's already implicit by it's absence 
> > > > from the list of recognized operators.
> > > > 
> > > > Again, the worst that happens is that your macro isn't converted.
> > > > 
> > > > I'm open to being convinced that it's important, but you haven't 
> > > > convinced me yet `:)`
> > > It wasn't much extra work/code to add comma operator support so I've done 
> > > that.
> > > "Don't let the perfect be the enemy of the good."
> > 
> > This is a production compiler toolchain. Correctness is important and that 
> > sometimes means caring more about perfection than you otherwise would like 
> > to.
> > 
> > > I'm open to being convinced that it's important, but you haven't 
> > > convinced me yet :)
> > 
> > It's less about importance and more about maintainability coupled with 
> > correctness. With your approach, we get something that will have a long 
> > tail of bugs. If you used Clang's parser, you don't get the same issue -- 
> > maintenance largely comes along for free, and the bugs are far less likely.
> > 
> > About the only reason I like your current approach over using clang's 
> > parsing is that it quite likely performs much better than doing an actual 
> > token parsing of the expression. But as you pointed out, about the worst 
> > thing for a check can do is take correct code and make it incorrect -- 
> > doing that right requires some amount of semantic evaluation of the 
> > expressions (which you're not doing). For example:
> > ```
> > #define FINE 1LL << 30LL;
> > #define BAD 1LL << 31LL;
> > #define ALSO_BAD 1LL << 32L;
> > ```
> > We'll convert this into an enumeration and break `-pedantic-errors` builds 
> > in C. If we had a `ConstantExpr` object, we could see what it's value is 
> > and note that it's greater than what fits into an `int` and decide to do 
> > something smarter.
> > 
> > So I continue to see the current approach as being somewhat reasonable 
> > (especially for experimentation), but incorrect in the long run. Not 
> > sufficiently incorrect for me to block this patch on, but incorrect enough 
> > that the first time this check becomes a maintenance burden, I'll be asking 
> > more strongly to do this the correct way.
> > > "Don't let the perfect be the enemy of the good."
> > 
> > This is a production compiler toolchain. Correctness is important and that 
> > sometimes means caring more about perfection than you otherwise would like 
> > to.
> 
> That's fair.
> 
> > For example:
> > ```
> > #define FINE 1LL << 30LL;
> > #define BAD 1LL << 31LL;
> > #define ALSO_BAD 1LL << 32L;
> > ```
> 
> Oh this brings up the pesky "semicolons disappear from the AST" issue.  I 
> wonder what happens when we're just processing tokens, though.  I will add a 
> test to see.  This could be a case where my approach results in more 
> correctness than `clangParse`!
> 
> > Not sufficiently incorrect for me to block this patch on, but incorrect 
> > enough that the first time this check becomes a maintenance burden, I'll be 
> > asking more strongly to do this the correct way.
> 
> I agree.
So I was research the C standard for what it says are acceptable initializer 
values for an enum and it //disallows// the comma operator:

https://en.cppreference.com/w/c/language/enum

> integer constant expression whose value is representable as a value of type 
> int

https://en.cppreference.com/w/c/language/constant_expression

> An integer 

[PATCH] D125620: [Clang] Fix diagnostics formatting

[Tee KOBAYASHI via Phabricator via cfe-commits]

xtkoba added a comment.

The issue was first reported by @dev-bz at GitHub in 
https://github.com/termux/termux-packages/issues/9619.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125620

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

[PATCH] D125620: [Clang] Fix diagnostics formatting

[Tee KOBAYASHI via Phabricator via cfe-commits]

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

When a `#warning` directive is followed by tokens containing non-ASCII 
characters encoded in UTF-8, a broken string can be printed out as a diagnostic 
message on some systems. This is because the current code assumes `char` to be 
`signed char`, whereas on some systems `char` is in fact `unsigned char`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125620

Files:
  clang/lib/Basic/Diagnostic.cpp


Index: clang/lib/Basic/Diagnostic.cpp
===
--- clang/lib/Basic/Diagnostic.cpp
+++ clang/lib/Basic/Diagnostic.cpp
@@ -811,7 +811,7 @@
   StringRef(DiagStr, DiagEnd - DiagStr).equals("%0") &&
   getArgKind(0) == DiagnosticsEngine::ak_std_string) {
 const std::string  = getArgStdStr(0);
-for (char c : S) {
+for (signed char c : S) {
   if (llvm::sys::locale::isPrint(c) || c == '\t') {
 OutStr.push_back(c);
   }


Index: clang/lib/Basic/Diagnostic.cpp
===
--- clang/lib/Basic/Diagnostic.cpp
+++ clang/lib/Basic/Diagnostic.cpp
@@ -811,7 +811,7 @@
   StringRef(DiagStr, DiagEnd - DiagStr).equals("%0") &&
   getArgKind(0) == DiagnosticsEngine::ak_std_string) {
 const std::string  = getArgStdStr(0);
-for (char c : S) {
+for (signed char c : S) {
   if (llvm::sys::locale::isPrint(c) || c == '\t') {
 OutStr.push_back(c);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106888: [RISC-V] Implement jump tables for CFI-icall

[Wende Tan via Phabricator via cfe-commits]

twd2 added a comment.

In D106888#3513001 , @MaskRay wrote:

> I'll wait a bit before pushing to check whether that further opinions.

Thanks! I don't have commit access, so could you please commit on my behalf 
when appropriate? Wende Tan 


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

https://reviews.llvm.org/D106888

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

[PATCH] D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum

[Richard via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp:210-214
+std::ostream <<(std::ostream ,
+ const clang::tidy::test::Param ) {
+  return Str << "Matched: " << std::boolalpha << Value.Matched << ", Text: '"
+ << Value.Text << "'";
+}

LegalizeAdulthood wrote:
> aaronpuchert wrote:
> > LegalizeAdulthood wrote:
> > > LegalizeAdulthood wrote:
> > > > aaronpuchert wrote:
> > > > > Seems to have caused a [build 
> > > > > failure](https://lab.llvm.org/buildbot/#/builders/57/builds/17915):
> > > > > 
> > > > > ```
> > > > > FAILED: 
> > > > > tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o
> > > > >  
> > > > > /home/buildbots/clang.11.0.0/bin/clang++ 
> > > > > --gcc-toolchain=/opt/rh/devtoolset-7/root/usr  -DGTEST_HAS_RTTI=0 
> > > > > -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS 
> > > > > -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
> > > > > -Itools/clang/tools/extra/unittests/clang-tidy 
> > > > > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/unittests/clang-tidy
> > > > >  
> > > > > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang/include
> > > > >  -Itools/clang/include -Iinclude 
> > > > > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/include
> > > > >  
> > > > > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/clang-tidy
> > > > >  
> > > > > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/utils/unittest/googletest/include
> > > > >  
> > > > > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/utils/unittest/googlemock/include
> > > > >  -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time 
> > > > > -Werror=unguarded-availability-new -Wall -Wextra 
> > > > > -Wno-unused-parameter -Wwrite-strings -Wcast-qual 
> > > > > -Wmissing-field-initializers -pedantic -Wno-long-long 
> > > > > -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
> > > > > -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
> > > > > -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion 
> > > > > -Wmisleading-indentation -fdiagnostics-color -ffunction-sections 
> > > > > -fdata-sections -fno-common -Woverloaded-virtual 
> > > > > -Wno-nested-anon-types -O3 -DNDEBUG-Wno-variadic-macros 
> > > > > -Wno-gnu-zero-variadic-macro-arguments -fno-exceptions -fno-rtti 
> > > > > -UNDEBUG -Wno-suggest-override -std=c++14 -MD -MT 
> > > > > tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o
> > > > >  -MF 
> > > > > tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o.d
> > > > >  -o 
> > > > > tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o
> > > > >  -c 
> > > > > /home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp
> > > > > /home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp:210:15:
> > > > >  error: unused function 'operator<<' [-Werror,-Wunused-function]
> > > > > std::ostream <<(std::ostream ,
> > > > >   ^
> > > > > 1 error generated.
> > > > > ```
> > > > Simon Pilgrim fixed it, but I don't understand why clang calls this 
> > > > function unused.  When the test fails, gtest uses this function to 
> > > > pretty print the parameter.  I'm rebuilding with a forced test failure 
> > > > to validate.
> > > Yes, without this function the failing test prints results like this:
> > > ```
> > > [ RUN  ] TokenExpressionParserTests/MatcherTest.MatchResult/123
> > > D:\legalize\llvm\llvm-project\clang-tools-extra\unittests\clang-tidy\ModernizeModuleTest.cpp(200):
> > >  error: Value of: matchText(GetParam().Text) == GetParam().Matched
> > >   Actual: false
> > > Expected: true
> > > [  FAILED  ] TokenExpressionParserTests/MatcherTest.MatchResult/123, 
> > > where GetParam() = 16-byte object <00-00 00-00 00-00 00-00 40-EC 3B-D6 
> > > F6-7F 00-00> (1 ms)
> > > ```
> > > which isn't particularly useful.
> > > 
> > > So how do we include pretty printers for tests without clang erroneously 
> > > flagging them as unused?
> > What got me wondering: this definition is last in the file, and there is no 
> > prior 

[PATCH] D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum

[Richard via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp:210-214
+std::ostream <<(std::ostream ,
+ const clang::tidy::test::Param ) {
+  return Str << "Matched: " << std::boolalpha << Value.Matched << ", Text: '"
+ << Value.Text << "'";
+}

aaronpuchert wrote:
> LegalizeAdulthood wrote:
> > LegalizeAdulthood wrote:
> > > aaronpuchert wrote:
> > > > Seems to have caused a [build 
> > > > failure](https://lab.llvm.org/buildbot/#/builders/57/builds/17915):
> > > > 
> > > > ```
> > > > FAILED: 
> > > > tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o
> > > >  
> > > > /home/buildbots/clang.11.0.0/bin/clang++ 
> > > > --gcc-toolchain=/opt/rh/devtoolset-7/root/usr  -DGTEST_HAS_RTTI=0 
> > > > -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS 
> > > > -D__STDC_LIMIT_MACROS -Itools/clang/tools/extra/unittests/clang-tidy 
> > > > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/unittests/clang-tidy
> > > >  
> > > > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang/include
> > > >  -Itools/clang/include -Iinclude 
> > > > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/include
> > > >  
> > > > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/clang-tidy
> > > >  
> > > > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/utils/unittest/googletest/include
> > > >  
> > > > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/utils/unittest/googlemock/include
> > > >  -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time 
> > > > -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
> > > > -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
> > > > -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
> > > > -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
> > > > -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion 
> > > > -Wmisleading-indentation -fdiagnostics-color -ffunction-sections 
> > > > -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types 
> > > > -O3 -DNDEBUG-Wno-variadic-macros 
> > > > -Wno-gnu-zero-variadic-macro-arguments -fno-exceptions -fno-rtti 
> > > > -UNDEBUG -Wno-suggest-override -std=c++14 -MD -MT 
> > > > tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o
> > > >  -MF 
> > > > tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o.d
> > > >  -o 
> > > > tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o
> > > >  -c 
> > > > /home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp
> > > > /home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp:210:15:
> > > >  error: unused function 'operator<<' [-Werror,-Wunused-function]
> > > > std::ostream <<(std::ostream ,
> > > >   ^
> > > > 1 error generated.
> > > > ```
> > > Simon Pilgrim fixed it, but I don't understand why clang calls this 
> > > function unused.  When the test fails, gtest uses this function to pretty 
> > > print the parameter.  I'm rebuilding with a forced test failure to 
> > > validate.
> > Yes, without this function the failing test prints results like this:
> > ```
> > [ RUN  ] TokenExpressionParserTests/MatcherTest.MatchResult/123
> > D:\legalize\llvm\llvm-project\clang-tools-extra\unittests\clang-tidy\ModernizeModuleTest.cpp(200):
> >  error: Value of: matchText(GetParam().Text) == GetParam().Matched
> >   Actual: false
> > Expected: true
> > [  FAILED  ] TokenExpressionParserTests/MatcherTest.MatchResult/123, where 
> > GetParam() = 16-byte object <00-00 00-00 00-00 00-00 40-EC 3B-D6 F6-7F 
> > 00-00> (1 ms)
> > ```
> > which isn't particularly useful.
> > 
> > So how do we include pretty printers for tests without clang erroneously 
> > flagging them as unused?
> What got me wondering: this definition is last in the file, and there is no 
> prior declaration of this function. How can there be any uses of it? We're 
> not in class scope, so all prior uses of `operator<<` or perhaps `PrintTo` 
> must have been resolved to some other 

[PATCH] D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.



Comment at: 
clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp:210-214
+std::ostream <<(std::ostream ,
+ const clang::tidy::test::Param ) {
+  return Str << "Matched: " << std::boolalpha << Value.Matched << ", Text: '"
+ << Value.Text << "'";
+}

LegalizeAdulthood wrote:
> LegalizeAdulthood wrote:
> > aaronpuchert wrote:
> > > Seems to have caused a [build 
> > > failure](https://lab.llvm.org/buildbot/#/builders/57/builds/17915):
> > > 
> > > ```
> > > FAILED: 
> > > tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o
> > >  
> > > /home/buildbots/clang.11.0.0/bin/clang++ 
> > > --gcc-toolchain=/opt/rh/devtoolset-7/root/usr  -DGTEST_HAS_RTTI=0 
> > > -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS 
> > > -D__STDC_LIMIT_MACROS -Itools/clang/tools/extra/unittests/clang-tidy 
> > > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/unittests/clang-tidy
> > >  
> > > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang/include
> > >  -Itools/clang/include -Iinclude 
> > > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/include
> > >  
> > > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/clang-tidy
> > >  
> > > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/utils/unittest/googletest/include
> > >  
> > > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/utils/unittest/googlemock/include
> > >  -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time 
> > > -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
> > > -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
> > > -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
> > > -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
> > > -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion 
> > > -Wmisleading-indentation -fdiagnostics-color -ffunction-sections 
> > > -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types 
> > > -O3 -DNDEBUG-Wno-variadic-macros 
> > > -Wno-gnu-zero-variadic-macro-arguments -fno-exceptions -fno-rtti -UNDEBUG 
> > > -Wno-suggest-override -std=c++14 -MD -MT 
> > > tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o
> > >  -MF 
> > > tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o.d
> > >  -o 
> > > tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o
> > >  -c 
> > > /home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp
> > > /home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp:210:15:
> > >  error: unused function 'operator<<' [-Werror,-Wunused-function]
> > > std::ostream <<(std::ostream ,
> > >   ^
> > > 1 error generated.
> > > ```
> > Simon Pilgrim fixed it, but I don't understand why clang calls this 
> > function unused.  When the test fails, gtest uses this function to pretty 
> > print the parameter.  I'm rebuilding with a forced test failure to validate.
> Yes, without this function the failing test prints results like this:
> ```
> [ RUN  ] TokenExpressionParserTests/MatcherTest.MatchResult/123
> D:\legalize\llvm\llvm-project\clang-tools-extra\unittests\clang-tidy\ModernizeModuleTest.cpp(200):
>  error: Value of: matchText(GetParam().Text) == GetParam().Matched
>   Actual: false
> Expected: true
> [  FAILED  ] TokenExpressionParserTests/MatcherTest.MatchResult/123, where 
> GetParam() = 16-byte object <00-00 00-00 00-00 00-00 40-EC 3B-D6 F6-7F 00-00> 
> (1 ms)
> ```
> which isn't particularly useful.
> 
> So how do we include pretty printers for tests without clang erroneously 
> flagging them as unused?
What got me wondering: this definition is last in the file, and there is no 
prior declaration of this function. How can there be any uses of it? We're not 
in class scope, so all prior uses of `operator<<` or perhaps `PrintTo` must 
have been resolved to some other function already. Or am I missing something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124500

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

In D123235#3513430 , @koops wrote:

> I have tried on x64 RH and x64 SuSe. I could not reproduce the failures seen 
> on x64 debian.  https://reviews.llvm.org/D118550 also has similar failures on 
> x64 debian. There is a comment " I think the test failures are spurious (but 
> not 100% sure)"  So, are these failures pre-existing before the changes in 
> the current support for "atomic compare fail: Parser & Support" were done?

Those tests should be irrelevant.




Comment at: clang/include/clang/AST/ASTNodeTraverser.h:228
 
+  void Visit(const OMPFailClause *C) {
+getNodeDelegate().AddChild([=] {

koops wrote:
> tianshilei1992 wrote:
> > Why would we want a dedicated function since it is only called once?
> The code for this method cannot be put into any other method because it 
> handles only OMPFailClause. All other Visit methods handle either the 
> generalized OMPClause or other types of Clauses.
I mean, it's only used by the function above, no?



Comment at: clang/include/clang/AST/ASTNodeTraverser.h:231-232
+  getNodeDelegate().Visit(C);
+  const OMPClause *mOC = C->const_getMemoryOrderClause();
+  Visit(mOC);
+});





Comment at: clang/include/clang/Parse/Parser.h:434
 
+  OMPClause *ParseOpenMPFailClause(OMPClause *clause);
+





Comment at: clang/lib/AST/OpenMPClause.cpp:417-432
+OMPFailClause *OMPFailClause::Create(const ASTContext ,
+ SourceLocation StartLoc,
+ SourceLocation EndLoc) {
+  void *Mem =
+  C.Allocate(totalSizeToAlloc(2, 1), 
alignof(OMPFailClause));
+  auto *Clause =
+  new (Mem) OMPFailClause(StartLoc, EndLoc);

clang-format plz



Comment at: clang/lib/Basic/OpenMPKinds.cpp:370
+  case OMPC_fail: {
+OpenMPClauseKind ck = static_cast(Type);
+switch (ck) {

maybe something like `CK`? `ck` doesn't conform with LLVM standard.



Comment at: clang/lib/Parse/ParseOpenMP.cpp:3622
+
+  OMPFailClause *failClause = static_cast(clause);
+  SourceLocation LParenLoc;





Comment at: clang/lib/Parse/ParseOpenMP.cpp:3691
 return nullptr;
-  return Actions.ActOnOpenMPClause(Kind, Loc, Tok.getLocation());
+  OMPClause *clause = Actions.ActOnOpenMPClause(Kind, Loc, Tok.getLocation());
+  if (Kind == llvm::omp::Clause::OMPC_fail) {

ditto



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11999
+ SourceLocation *ErrorLoc) {
+  int no_of_fails = 0;
+  ErrorNo = 0;

ditto


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

https://reviews.llvm.org/D123235

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

[PATCH] D123286: [Clang][OpenMP] Support for omp nothing

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

Can we have test for right usage?




Comment at: clang/lib/Parse/ParseOpenMP.cpp:2496
+<< getOpenMPDirectiveName(DKind) << 0;
+}
+ConsumeToken();

clang-format plz, and remove the `{` and `}` as there is only one statement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123286

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

[clang-tools-extra] 9d99cf5 - [clang-tidy] Restore test parameter operator<< function (NFC)

[via cfe-commits]

Author: Richard
Date: 2022-05-14T14:04:32-06:00
New Revision: 9d99cf59a151a715ebebf3a4c4782dfdb48d7f4b

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

LOG: [clang-tidy] Restore test parameter operator<< function (NFC)

Clang erroneously flagged the function as "unused", but it is most
definitely used by gtest to pretty print the parameter value when
a test fails.

Make the pretty printing function a friend function in the parameter
class similar to other clang unit tests.

Added: 


Modified: 
clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp

Removed: 




diff  --git a/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp 
b/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp
index 65bf0691372cc..b772c0054de40 100644
--- a/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp
@@ -48,6 +48,11 @@ namespace {
 struct Param {
   bool Matched;
   const char *Text;
+
+  friend std::ostream <<(std::ostream , const Param ) {
+return Str << "Matched: " << std::boolalpha << Value.Matched << ", Text: '"
+   << Value.Text << "'";
+  }
 };
 
 class MatcherTest : public ::testing::TestWithParam {};



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

[PATCH] D124918: [clang-tidy] Add a new check for non-trivial unused variables.

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst:25-26
+
+A function is considered to "not read an argument unless the return value is
+used" in the following cases:
+

The quotes here don't feel like they're needed.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst:35
+
+- A function listed in FunctionAllowList, if the argument is in index ``i`` and
+  bit ``i`` is set in the corresponding allow list entry.

Single backquotes around `FunctionAllowList`



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst:93
+   {
+  "std::string": ["swap"],
+  "absl::int128": [],

Since `std::string` is just a type alias, shouldn't we be considering 
`basic_string`?
What about `wstring`?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst:103
+  "absl::string_view": ["swap", "copy"],
+  "std::string_view": ["swap", "copy"]
+   }

`string_view` is also just a type alias, there is `wstring_view`, etc.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst:115-152
+  {
+"std::vector": ["swap"],
+"std::pair": [],
+"std::tuple": [],
+"std::optional": ["swap"],
+"std::variant": ["swap"],
+"std::list": ["swap"],

It would be easier to scan this list if it were sorted alphabetically.  I see 
`basic_string` listed here, but not `basic_string_view`.  Is that intentional?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst:172-173
+List of functions that are considered not to read some of their arguments
+unless their return value is read. Arguments are identified by a bitmask, where
+the i-th LSB being set indicates that the i-th argument is unused.
+

This isn't user-friendly at all.  Why not an array of argument indices starting 
at zero?



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-unused-no-side-effect.cpp:118
+  (VecLoops).push_back(3);
+}

Should there be some test cases for user-defined template types with no side 
effects?

Also some test cases for the smart pointer cases?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124918

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

[PATCH] D116280: [clang] adds unary type trait checks as compiler built-ins

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb updated this revision to Diff 429479.
cjdb added a comment.

rebases for D116203 's sake


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116280

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/type-traits.cpp

Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -345,11 +345,19 @@
 }
 
 typedef Enum EnumType;
+typedef EnumClass EnumClassType;
 
 void is_enum()
 {
   { int arr[T(__is_enum(Enum))]; }
   { int arr[T(__is_enum(EnumType))]; }
+  { int arr[T(__is_enum(SignedEnum))]; }
+  { int arr[T(__is_enum(UnsignedEnum))]; }
+
+  { int arr[T(__is_enum(EnumClass))]; }
+  { int arr[T(__is_enum(EnumClassType))]; }
+  { int arr[T(__is_enum(SignedEnumClass))]; }
+  { int arr[T(__is_enum(UnsignedEnumClass))]; }
 
   { int arr[F(__is_enum(int))]; }
   { int arr[F(__is_enum(Union))]; }
@@ -363,6 +371,29 @@
   { int arr[F(__is_enum(HasAnonymousUnion))]; }
 }
 
+void is_scoped_enum() {
+  { int arr[F(__is_scoped_enum(Enum))]; }
+  { int arr[F(__is_scoped_enum(EnumType))]; }
+  { int arr[F(__is_scoped_enum(SignedEnum))]; }
+  { int arr[F(__is_scoped_enum(UnsignedEnum))]; }
+
+  { int arr[T(__is_scoped_enum(EnumClass))]; }
+  { int arr[T(__is_scoped_enum(EnumClassType))]; }
+  { int arr[T(__is_scoped_enum(SignedEnumClass))]; }
+  { int arr[T(__is_scoped_enum(UnsignedEnumClass))]; }
+
+  { int arr[F(__is_scoped_enum(int))]; }
+  { int arr[F(__is_scoped_enum(Union))]; }
+  { int arr[F(__is_scoped_enum(Int))]; }
+  { int arr[F(__is_scoped_enum(IntAr))]; }
+  { int arr[F(__is_scoped_enum(UnionAr))]; }
+  { int arr[F(__is_scoped_enum(Derives))]; }
+  { int arr[F(__is_scoped_enum(ClassType))]; }
+  { int arr[F(__is_scoped_enum(cvoid))]; }
+  { int arr[F(__is_scoped_enum(IntArNB))]; }
+  { int arr[F(__is_scoped_enum(HasAnonymousUnion))]; }
+}
+
 struct FinalClass final {
 };
 
@@ -702,6 +733,106 @@
   int t31[F(__is_array(cvoid*))];
 }
 
+void is_bounded_array(int n) {
+  int t01[T(__is_bounded_array(IntAr))];
+  int t02[F(__is_bounded_array(IntArNB))];
+  int t03[T(__is_bounded_array(UnionAr))];
+
+  int t10[F(__is_bounded_array(void))];
+  int t11[F(__is_bounded_array(cvoid))];
+  int t12[F(__is_bounded_array(float))];
+  int t13[F(__is_bounded_array(double))];
+  int t14[F(__is_bounded_array(long double))];
+  int t15[F(__is_bounded_array(bool))];
+  int t16[F(__is_bounded_array(char))];
+  int t17[F(__is_bounded_array(signed char))];
+  int t18[F(__is_bounded_array(unsigned char))];
+  int t19[F(__is_bounded_array(wchar_t))];
+  int t20[F(__is_bounded_array(short))];
+  int t21[F(__is_bounded_array(unsigned short))];
+  int t22[F(__is_bounded_array(int))];
+  int t23[F(__is_bounded_array(unsigned int))];
+  int t24[F(__is_bounded_array(long))];
+  int t25[F(__is_bounded_array(unsigned long))];
+  int t26[F(__is_bounded_array(Union))];
+  int t27[F(__is_bounded_array(Derives))];
+  int t28[F(__is_bounded_array(ClassType))];
+  int t29[F(__is_bounded_array(Enum))];
+  int t30[F(__is_bounded_array(void *))];
+  int t31[F(__is_bounded_array(cvoid *))];
+
+  int t32[n];
+  (void)__is_bounded_array(decltype(t32)); // expected-error{{variable length arrays are not supported for '__is_bounded_array'}}
+}
+
+void is_unbounded_array(int n) {
+  int t01[F(__is_unbounded_array(IntAr))];
+  int t02[T(__is_unbounded_array(IntArNB))];
+  int t03[F(__is_unbounded_array(UnionAr))];
+
+  int t10[F(__is_unbounded_array(void))];
+  int t11[F(__is_unbounded_array(cvoid))];
+  int t12[F(__is_unbounded_array(float))];
+  int t13[F(__is_unbounded_array(double))];
+  int t14[F(__is_unbounded_array(long double))];
+  int t15[F(__is_unbounded_array(bool))];
+  int t16[F(__is_unbounded_array(char))];
+  int t17[F(__is_unbounded_array(signed char))];
+  int t18[F(__is_unbounded_array(unsigned char))];
+  int t19[F(__is_unbounded_array(wchar_t))];
+  int t20[F(__is_unbounded_array(short))];
+  int t21[F(__is_unbounded_array(unsigned short))];
+  int t22[F(__is_unbounded_array(int))];
+  int t23[F(__is_unbounded_array(unsigned int))];
+  int t24[F(__is_unbounded_array(long))];
+  int t25[F(__is_unbounded_array(unsigned long))];
+  int t26[F(__is_unbounded_array(Union))];
+  int t27[F(__is_unbounded_array(Derives))];
+  int t28[F(__is_unbounded_array(ClassType))];
+  int t29[F(__is_unbounded_array(Enum))];
+  int t30[F(__is_unbounded_array(void *))];
+  int t31[F(__is_unbounded_array(cvoid *))];
+
+  int t32[n];
+  (void)__is_unbounded_array(decltype(t32)); // expected-error{{variable length arrays are not supported for '__is_unbounded_array'}}
+}
+
+void 

[PATCH] D112661: [clangd] reuse preambles from other files when possible

[Quentin Chateau via Phabricator via cfe-commits]

qchateau updated this revision to Diff 429478.
qchateau added a comment.

Small correction


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112661

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -334,6 +334,13 @@
 init(getDefaultAsyncThreadsCount()),
 };
 
+opt ClosedPreambleCacheSize{
+"closed-preamble-cache-size",
+cat(Misc),
+desc("Number of preambles of closed files to keep in the cache."),
+init(1),
+};
+
 opt IndexFile{
 "index-file",
 cat(Misc),
@@ -879,6 +886,7 @@
 Opts.StaticIndex = PAI.get();
   }
   Opts.AsyncThreadsCount = WorkerThreadsCount;
+  Opts.ClosedPreambleCacheSize = ClosedPreambleCacheSize;
   Opts.FoldingRanges = FoldingRanges;
   Opts.MemoryCleanup = getMemoryCleanupFunction();
 
Index: clang-tools-extra/clangd/TUScheduler.h
===
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -190,6 +190,9 @@
 /// If 0, executes actions synchronously on the calling thread.
 unsigned AsyncThreadsCount = getDefaultAsyncThreadsCount();
 
+// Number of preambles of closed files to keep in the cache.
+unsigned ClosedPreambleCacheSize = 1;
+
 /// Cache (large) preamble data in RAM rather than temporary files on disk.
 bool StorePreamblesInMemory = false;
 
@@ -313,6 +316,8 @@
   class ASTCache;
   /// Tracks headers included by open files, to get known-good compile commands.
   class HeaderIncluderCache;
+  /// Store all known preambles
+  class PreambleStore;
 
   // The file being built/processed in the current thread. This is a hack in
   // order to get the file name into the index implementations. Do not depend on
@@ -336,6 +341,7 @@
   llvm::StringMap> Files;
   std::unique_ptr IdleASTs;
   std::unique_ptr HeaderIncluders;
+  std::unique_ptr Preambles;
   // None when running tasks synchronously and non-None when running tasks
   // asynchronously.
   llvm::Optional PreambleTasks;
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -342,6 +342,65 @@
   }
 };
 
+class TUScheduler::PreambleStore {
+public:
+  struct Entry {
+std::shared_ptr Preamble;
+size_t Score;
+Path FileName;
+
+bool operator>(const Entry ) const { return Score > Other.Score; }
+  };
+
+  explicit PreambleStore(size_t ClosedPreamblesToKeep)
+  : ClosedPreamblesToKeep(ClosedPreamblesToKeep) {}
+
+  auto getAll() {
+std::unique_lock Lock(Mut);
+return Store;
+  }
+
+  void push(PathRef FileName, std::shared_ptr Preamble) {
+std::unique_lock Lock(Mut);
+auto It = llvm::find_if(
+Store, [&](const auto ) { return Item.FileName == FileName; });
+if (It == Store.end()) {
+  Store.push_back(Entry{std::move(Preamble), 0, FileName.str()});
+  popWorstPreambles();
+} else {
+  It->Preamble = std::move(Preamble);
+}
+vlog("Store contains {0} preambles", Store.size());
+  }
+
+  void hit(PathRef FileName) {
+std::unique_lock Lock(Mut);
+auto It = llvm::find_if(
+Store, [&](const auto ) { return Item.FileName == FileName; });
+if (It == Store.end()) {
+  return;
+}
+It->Score++;
+  }
+
+private:
+  void popWorstPreambles() {
+auto Begin = llvm::partition(
+Store, [](const auto ) { return Item.Preamble.use_count() > 1; });
+if (static_cast(std::distance(Begin, Store.end())) <=
+ClosedPreamblesToKeep) {
+  return;
+}
+auto Nth = Begin + ClosedPreamblesToKeep;
+std::nth_element(Begin, Nth, Store.end(), std::greater());
+Store.erase(Nth, Store.end());
+  }
+
+  std::mutex Mut;
+  std::vector Store;
+  size_t ClosedPreamblesToKeep;
+};
+
 namespace {
 
 bool isReliable(const tooling::CompileCommand ) {
@@ -541,7 +600,8 @@
   ASTWorker(PathRef FileName, const GlobalCompilationDatabase ,
 TUScheduler::ASTCache ,
 TUScheduler::HeaderIncluderCache ,
-Semaphore , bool RunSync, const TUScheduler::Options ,
+TUScheduler::PreambleStore , Semaphore ,
+bool RunSync, const TUScheduler::Options ,
 ParsingCallbacks );
 
 public:
@@ -554,8 +614,9 @@
   create(PathRef FileName, const GlobalCompilationDatabase ,
  TUScheduler::ASTCache ,
  TUScheduler::HeaderIncluderCache ,
- AsyncTaskRunner 

[PATCH] D124341: [clang-tidy][NFC] Replace many instances of std::string where a StringRef would suffice.

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp:56
 ClangTidyCheck::OptionsView::get(StringRef LocalName) const {
-  const auto  = CheckOptions.find(NamePrefix + LocalName.str());
+  const auto  = CheckOptions.find((NamePrefix + LocalName).str());
   if (Iter != CheckOptions.end())

njames93 wrote:
> LegalizeAdulthood wrote:
> > `find` takes a `StringRef` so why convert to `std::string` here?
> Because a concatenation of StringRef results in a Twine, which cannot be 
> passed to find.
Interactions between `std::string`, `StringRef` and `Twine` always make my 
brain hurt `:)`.

I keep wondering if there's a way to make `StringRef` and `Twine` more 
user-friendly, or perhaps a clang-tidy check that will alert you to things  
that are "considered harmful".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124341

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

[PATCH] D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added inline comments.



Comment at: 
clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp:210-214
+std::ostream <<(std::ostream ,
+ const clang::tidy::test::Param ) {
+  return Str << "Matched: " << std::boolalpha << Value.Matched << ", Text: '"
+ << Value.Text << "'";
+}

LegalizeAdulthood wrote:
> aaronpuchert wrote:
> > Seems to have caused a [build 
> > failure](https://lab.llvm.org/buildbot/#/builders/57/builds/17915):
> > 
> > ```
> > FAILED: 
> > tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o
> >  
> > /home/buildbots/clang.11.0.0/bin/clang++ 
> > --gcc-toolchain=/opt/rh/devtoolset-7/root/usr  -DGTEST_HAS_RTTI=0 -D_DEBUG 
> > -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS 
> > -D__STDC_LIMIT_MACROS -Itools/clang/tools/extra/unittests/clang-tidy 
> > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/unittests/clang-tidy
> >  
> > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang/include
> >  -Itools/clang/include -Iinclude 
> > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/include
> >  
> > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/clang-tidy
> >  
> > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/utils/unittest/googletest/include
> >  
> > -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/utils/unittest/googlemock/include
> >  -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time 
> > -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
> > -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
> > -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
> > -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
> > -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion 
> > -Wmisleading-indentation -fdiagnostics-color -ffunction-sections 
> > -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 
> > -DNDEBUG-Wno-variadic-macros -Wno-gnu-zero-variadic-macro-arguments 
> > -fno-exceptions -fno-rtti -UNDEBUG -Wno-suggest-override -std=c++14 -MD -MT 
> > tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o
> >  -MF 
> > tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o.d
> >  -o 
> > tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o
> >  -c 
> > /home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp
> > /home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp:210:15:
> >  error: unused function 'operator<<' [-Werror,-Wunused-function]
> > std::ostream <<(std::ostream ,
> >   ^
> > 1 error generated.
> > ```
> Simon Pilgrim fixed it, but I don't understand why clang calls this function 
> unused.  When the test fails, gtest uses this function to pretty print the 
> parameter.  I'm rebuilding with a forced test failure to validate.
Yes, without this function the failing test prints results like this:
```
[ RUN  ] TokenExpressionParserTests/MatcherTest.MatchResult/123
D:\legalize\llvm\llvm-project\clang-tools-extra\unittests\clang-tidy\ModernizeModuleTest.cpp(200):
 error: Value of: matchText(GetParam().Text) == GetParam().Matched
  Actual: false
Expected: true
[  FAILED  ] TokenExpressionParserTests/MatcherTest.MatchResult/123, where 
GetParam() = 16-byte object <00-00 00-00 00-00 00-00 40-EC 3B-D6 F6-7F 00-00> 
(1 ms)
```
which isn't particularly useful.

So how do we include pretty printers for tests without clang erroneously 
flagging them as unused?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124500

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

[PATCH] D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added inline comments.



Comment at: 
clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp:210-214
+std::ostream <<(std::ostream ,
+ const clang::tidy::test::Param ) {
+  return Str << "Matched: " << std::boolalpha << Value.Matched << ", Text: '"
+ << Value.Text << "'";
+}

aaronpuchert wrote:
> Seems to have caused a [build 
> failure](https://lab.llvm.org/buildbot/#/builders/57/builds/17915):
> 
> ```
> FAILED: 
> tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o
>  
> /home/buildbots/clang.11.0.0/bin/clang++ 
> --gcc-toolchain=/opt/rh/devtoolset-7/root/usr  -DGTEST_HAS_RTTI=0 -D_DEBUG 
> -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS 
> -D__STDC_LIMIT_MACROS -Itools/clang/tools/extra/unittests/clang-tidy 
> -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/unittests/clang-tidy
>  
> -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang/include
>  -Itools/clang/include -Iinclude 
> -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/include
>  
> -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/clang-tidy
>  
> -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/utils/unittest/googletest/include
>  
> -I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/utils/unittest/googlemock/include
>  -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time 
> -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
> -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
> -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
> -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
> -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion 
> -Wmisleading-indentation -fdiagnostics-color -ffunction-sections 
> -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 
> -DNDEBUG-Wno-variadic-macros -Wno-gnu-zero-variadic-macro-arguments 
> -fno-exceptions -fno-rtti -UNDEBUG -Wno-suggest-override -std=c++14 -MD -MT 
> tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o
>  -MF 
> tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o.d
>  -o 
> tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o
>  -c 
> /home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp
> /home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp:210:15:
>  error: unused function 'operator<<' [-Werror,-Wunused-function]
> std::ostream <<(std::ostream ,
>   ^
> 1 error generated.
> ```
Simon Pilgrim fixed it, but I don't understand why clang calls this function 
unused.  When the test fails, gtest uses this function to pretty print the 
parameter.  I'm rebuilding with a forced test failure to validate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124500

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

[PATCH] D112661: [clangd] reuse preambles from other files when possible

[Quentin Chateau via Phabricator via cfe-commits]

qchateau updated this revision to Diff 429470.
qchateau added a comment.
Herald added a project: All.

Resolve conflicts, fix formatting, simplify patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112661

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -334,6 +334,13 @@
 init(getDefaultAsyncThreadsCount()),
 };
 
+opt ClosedPreambleCacheSize{
+"closed-preamble-cache-size",
+cat(Misc),
+desc("Number of preambles of closed files to keep in the cache."),
+init(1),
+};
+
 opt IndexFile{
 "index-file",
 cat(Misc),
@@ -879,6 +886,7 @@
 Opts.StaticIndex = PAI.get();
   }
   Opts.AsyncThreadsCount = WorkerThreadsCount;
+  Opts.ClosedPreambleCacheSize = ClosedPreambleCacheSize;
   Opts.FoldingRanges = FoldingRanges;
   Opts.MemoryCleanup = getMemoryCleanupFunction();
 
Index: clang-tools-extra/clangd/TUScheduler.h
===
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -190,6 +190,9 @@
 /// If 0, executes actions synchronously on the calling thread.
 unsigned AsyncThreadsCount = getDefaultAsyncThreadsCount();
 
+// Number of preambles of closed files to keep in the cache.
+unsigned ClosedPreambleCacheSize = 1;
+
 /// Cache (large) preamble data in RAM rather than temporary files on disk.
 bool StorePreamblesInMemory = false;
 
@@ -313,6 +316,8 @@
   class ASTCache;
   /// Tracks headers included by open files, to get known-good compile commands.
   class HeaderIncluderCache;
+  /// Store all known preambles
+  class PreambleStore;
 
   // The file being built/processed in the current thread. This is a hack in
   // order to get the file name into the index implementations. Do not depend on
@@ -336,6 +341,7 @@
   llvm::StringMap> Files;
   std::unique_ptr IdleASTs;
   std::unique_ptr HeaderIncluders;
+  std::unique_ptr Preambles;
   // None when running tasks synchronously and non-None when running tasks
   // asynchronously.
   llvm::Optional PreambleTasks;
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -342,6 +342,65 @@
   }
 };
 
+class TUScheduler::PreambleStore {
+public:
+  struct Entry {
+std::shared_ptr Preamble;
+size_t Score;
+Path FileName;
+
+bool operator>(const Entry ) const { return Score > Other.Score; }
+  };
+
+  explicit PreambleStore(size_t ClosedPreamblesToKeep)
+  : ClosedPreamblesToKeep(ClosedPreamblesToKeep) {}
+
+  auto getAll() {
+std::unique_lock Lock(Mut);
+return Store;
+  }
+
+  void push(PathRef FileName, std::shared_ptr Preamble) {
+std::unique_lock Lock(Mut);
+auto It = llvm::find_if(
+Store, [&](const auto ) { return Item.FileName == FileName; });
+if (It == Store.end()) {
+  Store.push_back(Entry{std::move(Preamble), 0, FileName.str()});
+  popWorstPreambles();
+} else {
+  It->Preamble = std::move(Preamble);
+}
+vlog("Store contains {0} preambles", Store.size());
+  }
+
+  void hit(PathRef FileName) {
+std::unique_lock Lock(Mut);
+auto It = llvm::find_if(
+Store, [&](const auto ) { return Item.FileName == FileName; });
+if (It == Store.end()) {
+  return;
+}
+It->Score++;
+  }
+
+private:
+  void popWorstPreambles() {
+auto Begin = llvm::partition(
+Store, [](const auto ) { return Item.Preamble.use_count() > 1; });
+if (static_cast(std::distance(Begin, Store.end())) <=
+ClosedPreamblesToKeep) {
+  return;
+}
+auto Nth = Begin + ClosedPreamblesToKeep;
+std::nth_element(Begin, Nth, Store.end(), std::greater());
+Store.erase(Nth, Store.end());
+  }
+
+  std::mutex Mut;
+  std::vector Store;
+  size_t ClosedPreamblesToKeep;
+};
+
 namespace {
 
 bool isReliable(const tooling::CompileCommand ) {
@@ -541,7 +600,8 @@
   ASTWorker(PathRef FileName, const GlobalCompilationDatabase ,
 TUScheduler::ASTCache ,
 TUScheduler::HeaderIncluderCache ,
-Semaphore , bool RunSync, const TUScheduler::Options ,
+TUScheduler::PreambleStore , Semaphore ,
+bool RunSync, const TUScheduler::Options ,
 ParsingCallbacks );
 
 public:
@@ -554,8 +614,9 @@
   create(PathRef FileName, const GlobalCompilationDatabase ,
  TUScheduler::ASTCache ,
  

[PATCH] D125557: [APInt] Remove all uses of zextOrSelf, sextOrSelf and truncOrSelf

[Jay Foad via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Analysis/ConstantFolding.cpp:2884
 if (IntrinsicID == Intrinsic::smul_fix_sat) {
-  APInt Max = APInt::getSignedMaxValue(Width).sextOrSelf(ExtendedWidth);
-  APInt Min = APInt::getSignedMinValue(Width).sextOrSelf(ExtendedWidth);
+  APInt Max = APInt::getSignedMaxValue(Width).sext(ExtendedWidth);
+  APInt Min = APInt::getSignedMinValue(Width).sext(ExtendedWidth);

lattner wrote:
> I think this can be a zext given the top bit will be zero
Sure the first one could be zext, but the second one can't be, so it feels 
conceptually simpler (to me) to keep them both as sext.



Comment at: llvm/lib/IR/ConstantRange.cpp:724
 auto BW = getBitWidth();
-APInt Min = APInt::getMinValue(BW).zextOrSelf(ResultBitWidth);
-APInt Max = APInt::getMaxValue(BW).zextOrSelf(ResultBitWidth);
+APInt Min = APInt::getMinValue(BW);
+APInt Max = APInt::getMaxValue(BW);

efriedma wrote:
> efriedma wrote:
> > Making the bitwidth of the result here not equal to ResultBitWidth seems 
> > suspect.
> > 
> > I think there should just be an `if (ResultBitWidth < BW) return 
> > getFull(ResultBitWidth);` here.  Then a simple conversion just works.
> Actually, looking at D27294 again, maybe it is actually making the result 
> bitwidth intentionally inflate like this.
> 
> This could use a comment explaining what it's doing, in any case.
I agree it could use a comment but I don't feel qualified to write it - I am 
just trying to preserve the current behaviour.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125557

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

[PATCH] D125557: [APInt] Remove all uses of zextOrSelf, sextOrSelf and truncOrSelf

[Jay Foad via Phabricator via cfe-commits]

foad updated this revision to Diff 429466.
foad added a comment.

Address some review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125557

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
  llvm/lib/Analysis/BasicAliasAnalysis.cpp
  llvm/lib/Analysis/ConstantFolding.cpp
  llvm/lib/Analysis/LazyValueInfo.cpp
  llvm/lib/Analysis/MemoryBuiltins.cpp
  llvm/lib/Analysis/ScalarEvolution.cpp
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
  llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
  llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
  llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
  llvm/lib/IR/ConstantRange.cpp
  llvm/lib/Support/APFixedPoint.cpp
  llvm/lib/Support/APInt.cpp
  llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp
  llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp
  llvm/lib/Target/Hexagon/HexagonConstPropagation.cpp
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86TargetTransformInfo.cpp
  llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
  llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
  llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
  llvm/test/TableGen/VarLenEncoder.td
  llvm/utils/TableGen/VarLenCodeEmitterGen.cpp
  polly/lib/CodeGen/IslExprBuilder.cpp

Index: polly/lib/CodeGen/IslExprBuilder.cpp
===
--- polly/lib/CodeGen/IslExprBuilder.cpp
+++ polly/lib/CodeGen/IslExprBuilder.cpp
@@ -765,7 +765,7 @@
   else
 T = Builder.getIntNTy(BitWidth);
 
-  APValue = APValue.sextOrSelf(T->getBitWidth());
+  APValue = APValue.sext(T->getBitWidth());
   V = ConstantInt::get(T, APValue);
 
   isl_ast_expr_free(Expr);
Index: llvm/utils/TableGen/VarLenCodeEmitterGen.cpp
===
--- llvm/utils/TableGen/VarLenCodeEmitterGen.cpp
+++ llvm/utils/TableGen/VarLenCodeEmitterGen.cpp
@@ -424,7 +424,7 @@
   raw_string_ostream SS(Case);
   // Resize the scratch buffer.
   if (BitWidth && !VLI.isFixedValueOnly())
-SS.indent(6) << "Scratch = Scratch.zextOrSelf(" << BitWidth << ");\n";
+SS.indent(6) << "Scratch = Scratch.zext(" << BitWidth << ");\n";
   // Populate based value.
   SS.indent(6) << "Inst = getInstBits(opcode);\n";
 
Index: llvm/test/TableGen/VarLenEncoder.td
===
--- llvm/test/TableGen/VarLenEncoder.td
+++ llvm/test/TableGen/VarLenEncoder.td
@@ -65,7 +65,7 @@
 // CHECK: UINT64_C(46848), // FOO32
 
 // CHECK-LABEL: case ::FOO16: {
-// CHECK: Scratch = Scratch.zextOrSelf(41);
+// CHECK: Scratch = Scratch.zext(41);
 // src.reg
 // CHECK: getMachineOpValue(MI, MI.getOperand(1), /*Pos=*/0, Scratch, Fixups, STI);
 // CHECK: Inst.insertBits(Scratch.extractBits(8, 0), 0);
@@ -83,7 +83,7 @@
 // CHECK: Inst.insertBits(Scratch.extractBits(2, 0), 39);
 
 // CHECK-LABEL: case ::FOO32: {
-// CHECK: Scratch = Scratch.zextOrSelf(57);
+// CHECK: Scratch = Scratch.zext(57);
 // src.reg
 // CHECK: getMachineOpValue(MI, MI.getOperand(1), /*Pos=*/0, Scratch, Fixups, STI);
 // CHECK: Inst.insertBits(Scratch.extractBits(8, 0), 0);
Index: llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
===
--- llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
+++ llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
@@ -496,7 +496,7 @@
   if (PtrDelta.urem(Stride) != 0)
 return false;
   unsigned IdxBitWidth = OpA->getType()->getScalarSizeInBits();
-  APInt IdxDiff = PtrDelta.udiv(Stride).zextOrSelf(IdxBitWidth);
+  APInt IdxDiff = PtrDelta.udiv(Stride).zext(IdxBitWidth);
 
   // Only look through a ZExt/SExt.
   if (!isa(OpA) && !isa(OpA))
Index: llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
===
--- llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
+++ llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
@@ -741,8 +741,7 @@
   // sdiv/srem is UB if divisor is -1 and divident is INT_MIN, so unless we can
   // prove that such a combination is impossible, we need to bump the bitwidth.
   if (CRs[1]->contains(APInt::getAllOnes(OrigWidth)) &&
-  CRs[0]->contains(
-  APInt::getSignedMinValue(MinSignedBits).sextOrSelf(OrigWidth)))
+  CRs[0]->contains(APInt::getSignedMinValue(MinSignedBits).sext(OrigWidth)))
 ++MinSignedBits;
 
   // Don't shrink below 8 bits wide.
Index: 

[PATCH] D125256: [OpenMP] Add `__CUDA_ARCH__` definition when offloading with OpenMP

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

Sorry - my mistake - its a different test failure now!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125256

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

[PATCH] D125256: [OpenMP] Add `__CUDA_ARCH__` definition when offloading with OpenMP

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D125256#3513596 , @RKSimon wrote:

> @jhuber6 I think this or one of your other openmp commits has caused the 
> Driver/cuda-openmp-driver.cu test failure here: 
> https://lab.llvm.org/buildbot/#/builders/214/builds/1274/steps/6/logs/stdio

Is that still failing? I saw another build-bot fail on that test as well, so I 
pushed a quick change and it went green. When I check a more recent build there 
it doesn't show the test failing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125256

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

[PATCH] D125256: [OpenMP] Add `__CUDA_ARCH__` definition when offloading with OpenMP

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

@jhuber6 I think this or one of your other openmp commits has caused the 
Driver/cuda-openmp-driver.cu test failure here: 
https://lab.llvm.org/buildbot/#/builders/214/builds/1274/steps/6/logs/stdio


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125256

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

[PATCH] D124751: [HLSL] Support -E option for HLSL.

[Xiang Li via Phabricator via cfe-commits]

python3kgae added a comment.

In D124751#3513283 , @MaskRay wrote:

> Is there a specification or reference implementation stating that -E is used?
>
>> Option<["--", "/", "-"], "E",
>
> Do you need the prefix `--`? You may define something like `CLFlag`. I have 
> missed previous patches, but if `/` options are not necessary, removing them 
> will be the best to avoid collision with filenames starting with `/`.

Unfortunately, '/' is necessary. dxc allow both '-' and '/' like 'CLFlag'.
Remove '/' means existing customers may need to change their command line to 
compile hlsl.
And add '--' feels not hurt anyone, that's why I choose to add '--' to 
work-around the conflict.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124751

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

[clang-tools-extra] ffacaa0 - Fix unused function 'operator<<' -Wunused-function warning introduced in D124500

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-05-14T13:48:26+01:00
New Revision: ffacaa0beccbe318090be600f8d2d2c33c33cbd6

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

LOG: Fix unused function 'operator<<' -Wunused-function warning introduced in 
D124500

Added: 


Modified: 
clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp

Removed: 




diff  --git a/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp 
b/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp
index 38347ded439b9..65bf0691372cc 100644
--- a/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp
@@ -206,9 +206,3 @@ INSTANTIATE_TEST_SUITE_P(TokenExpressionParserTests, 
MatcherTest,
 } // namespace test
 } // namespace tidy
 } // namespace clang
-
-std::ostream <<(std::ostream ,
- const clang::tidy::test::Param ) {
-  return Str << "Matched: " << std::boolalpha << Value.Matched << ", Text: '"
- << Value.Text << "'";
-}



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

[PATCH] D125604: [FileCheck] Catch missspelled directives.

[Ivan Kosarev via Phabricator via cfe-commits]

kosarev added a comment.

A work in progress. Contains changes that we likely want to see addressed 
separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125604

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

[PATCH] D125604: [FileCheck] Catch missspelled directives.

[Ivan Kosarev via Phabricator via cfe-commits]

kosarev created this revision.
Herald added subscribers: mattd, gchakrabarti, pmatos, asb, asavonic, wenlei, 
kerbowa, pengfei, Jim, asbirlea, thopre, rupprecht, george.burgess.iv, kbarton, 
hiraditya, jgravelle-google, sbc100, jvesely, nemanjai, dylanmckay, dschuff, 
jholewinski.
Herald added a reviewer: jhenderson.
Herald added a reviewer: MaskRay.
Herald added a project: All.
kosarev requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, StephenFan, aheejin.
Herald added projects: clang, LLVM.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125604

Files:
  clang/test/CodeGen/cmse-clear-return.c
  clang/test/CodeGenCXX/attr-mustprogress.cpp
  clang/test/CodeGenCXX/eh-aggregate-copy-destroy.cpp
  clang/test/CodeGenCXX/inheriting-constructor.cpp
  clang/test/CodeGenObjC/non-runtime-protocol.m
  clang/test/OpenMP/master_taskloop_private_codegen.cpp
  clang/test/OpenMP/master_taskloop_simd_private_codegen.cpp
  clang/test/OpenMP/parallel_master_taskloop_private_codegen.cpp
  clang/test/OpenMP/parallel_master_taskloop_simd_private_codegen.cpp
  clang/test/OpenMP/task_private_codegen.cpp
  clang/test/OpenMP/taskgroup_task_reduction_codegen.cpp
  clang/test/OpenMP/taskloop_private_codegen.cpp
  clang/test/OpenMP/taskloop_simd_private_codegen.cpp
  llvm/include/llvm/FileCheck/FileCheck.h
  llvm/lib/FileCheck/FileCheck.cpp
  llvm/test/Analysis/MemorySSA/phi-translation.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_4.ll
  llvm/test/CodeGen/AArch64/fp16-v8-instructions.ll
  llvm/test/CodeGen/AArch64/neon-vmull-high-p64.ll
  llvm/test/CodeGen/AMDGPU/divergence-driven-bfe-isel.ll
  llvm/test/CodeGen/AMDGPU/hoist-cond.ll
  llvm/test/CodeGen/AMDGPU/llvm.amdgcn.ps.live.ll
  llvm/test/CodeGen/AMDGPU/mode-register.mir
  llvm/test/CodeGen/AMDGPU/phi-vgpr-input-moveimm.mir
  llvm/test/CodeGen/AMDGPU/smrd.ll
  llvm/test/CodeGen/ARM/cmpxchg-O0-be.ll
  llvm/test/CodeGen/AVR/atomics/fence.ll
  llvm/test/CodeGen/BPF/CORE/offset-reloc-middle-chain.ll
  llvm/test/CodeGen/MIR/AMDGPU/extra-imm-operand.mir
  llvm/test/CodeGen/MIR/AMDGPU/extra-reg-operand.mir
  llvm/test/CodeGen/Thumb2/thumb2-execute-only-prologue.ll
  llvm/test/CodeGen/WebAssembly/libcalls.ll
  llvm/test/CodeGen/X86/GlobalISel/select-ext.mir
  llvm/test/CodeGen/X86/coalesce-dead-lanes.mir
  llvm/test/CodeGen/X86/copy-propagation.ll
  llvm/test/CodeGen/X86/lvi-hardening-indirectbr.ll
  llvm/test/CodeGen/X86/statepoint-vreg-details.ll
  llvm/test/DebugInfo/NVPTX/debug-info.ll
  llvm/test/DebugInfo/X86/debug-info-template-parameter.ll
  llvm/test/FileCheck/missspelled-directive.txt
  llvm/test/MC/AMDGPU/data.s
  llvm/test/MC/AsmParser/directive_file-g.s
  llvm/test/MC/PowerPC/ppc64-reloc-directive-pcrel.s
  llvm/test/MC/WebAssembly/unnamed-data.ll
  llvm/test/Transforms/Inline/inline-strictfp.ll
  llvm/test/Transforms/LoopVectorize/X86/gather-vs-interleave.ll
  llvm/test/Transforms/MergeFunc/alias.ll
  llvm/test/Transforms/PGOProfile/PR41279.ll
  llvm/test/Transforms/PGOProfile/memop_clone.ll
  llvm/test/Transforms/PGOProfile/memop_size_from_strlen.ll
  llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
  llvm/test/tools/llvm-dwp/X86/type_dedup_v5.test
  llvm/test/tools/llvm-objdump/MachO/disassemble-all.test
  llvm/test/tools/llvm-readobj/COFF/unwind-arm64-windows.test

Index: llvm/test/tools/llvm-readobj/COFF/unwind-arm64-windows.test
===
--- llvm/test/tools/llvm-readobj/COFF/unwind-arm64-windows.test
+++ llvm/test/tools/llvm-readobj/COFF/unwind-arm64-windows.test
@@ -27,7 +27,7 @@
 UNWIND1-NEXT:0x28; ldp x19, x20, [sp], #64
 UNWIND1-NEXT:0xe4; end
 UNWIND1-NEXT:  ]
-UNWIND1_NEXT:}
+UNWIND1-NEXT:}
 
 
 UNWIND2: ExceptionData {
Index: llvm/test/tools/llvm-objdump/MachO/disassemble-all.test
===
--- llvm/test/tools/llvm-objdump/MachO/disassemble-all.test
+++ llvm/test/tools/llvm-objdump/MachO/disassemble-all.test
@@ -1,39 +1,39 @@
 // RUN: llvm-objdump --macho -d --full-leading-addr --print-imm-hex --no-show-raw-insn %p/Inputs/macho-multiple-text | FileCheck %s --check-prefix=TEXT
 
 TEXT:  (__TEXT,__text) section
-TEXT_NEXT: _main:
-TEXT_NEXT: 00010f60	pushq	%rbp
-TEXT_NEXT: 00010f61	movq	%rsp, %rbp
-TEXT_NEXT: 00010f64	subq	$0x10, %rsp
-TEXT_NEXT: 00010f68	movl	$0x0, -0x4(%rbp)
-TEXT_NEXT: 00010f6f	callq	_hello
-TEXT_NEXT: 00010f74	xorl	%eax, %eax
-TEXT_NEXT: 00010f76	addq	$0x10, %rsp
-TEXT_NEXT: 00010f7a	popq	%rbp
-TEXT_NEXT: 00010f7b	retq
+TEXT-NEXT: _main:
+TEXT-NEXT: 00010f60	pushq	%rbp
+TEXT-NEXT: 00010f61	movq	%rsp, %rbp
+TEXT-NEXT: 00010f64	subq	$0x10, %rsp
+TEXT-NEXT: 00010f68	movl	$0x0, -0x4(%rbp)
+TEXT-NEXT: 00010f6f	callq	_hello
+TEXT-NEXT: 00010f74	xorl	%eax, %eax
+TEXT-NEXT: 00010f76	

[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

[Sunil K via Phabricator via cfe-commits]

koops added a comment.

I have tried on x64 RH and x64 SuSe. I could not reproduce the failures seen on 
x64 debian.  https://reviews.llvm.org/D118550 also has similar failures on x64 
debian. There is a comment " I think the test failures are spurious (but not 
100% sure)"  So, are these failures pre-existing before the above changes were 
done?


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

https://reviews.llvm.org/D123235

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

[PATCH] D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.



Comment at: 
clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp:210-214
+std::ostream <<(std::ostream ,
+ const clang::tidy::test::Param ) {
+  return Str << "Matched: " << std::boolalpha << Value.Matched << ", Text: '"
+ << Value.Text << "'";
+}

Seems to have caused a [build 
failure](https://lab.llvm.org/buildbot/#/builders/57/builds/17915):

```
FAILED: 
tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o
 
/home/buildbots/clang.11.0.0/bin/clang++ 
--gcc-toolchain=/opt/rh/devtoolset-7/root/usr  -DGTEST_HAS_RTTI=0 -D_DEBUG 
-D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS 
-D__STDC_LIMIT_MACROS -Itools/clang/tools/extra/unittests/clang-tidy 
-I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/unittests/clang-tidy
 
-I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang/include
 -Itools/clang/include -Iinclude 
-I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/include
 
-I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/clang-tidy
 
-I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/utils/unittest/googletest/include
 
-I/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/utils/unittest/googlemock/include
 -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time 
-Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
-Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
-Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
-Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
-Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion 
-Wmisleading-indentation -fdiagnostics-color -ffunction-sections 
-fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 
-DNDEBUG-Wno-variadic-macros -Wno-gnu-zero-variadic-macro-arguments 
-fno-exceptions -fno-rtti -UNDEBUG -Wno-suggest-override -std=c++14 -MD -MT 
tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o
 -MF 
tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o.d
 -o 
tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/ModernizeModuleTest.cpp.o
 -c 
/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp
/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp:210:15:
 error: unused function 'operator<<' [-Werror,-Wunused-function]
std::ostream <<(std::ostream ,
  ^
1 error generated.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124500

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

[PATCH] D125580: Resolve overload ambiguity on Mac OS when printing size_t in diagnostics

[Aaron Puchert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGac7a9ef0ae3a: Resolve overload ambiguity on Mac OS when 
printing size_t in diagnostics (authored by aaronpuchert).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125580

Files:
  clang/include/clang/Basic/Diagnostic.h
  clang/lib/AST/CommentParser.cpp


Index: clang/lib/AST/CommentParser.cpp
===
--- clang/lib/AST/CommentParser.cpp
+++ clang/lib/AST/CommentParser.cpp
@@ -414,7 +414,7 @@
   if (Args.size() < Info->NumArgs) {
 Diag(CommandTok.getEndLocation().getLocWithOffset(1),
  diag::warn_doc_inline_command_not_enough_arguments)
-<< CommandTok.is(tok::at_command) << Info->Name << 
(uint64_t)Args.size()
+<< CommandTok.is(tok::at_command) << Info->Name << Args.size()
 << Info->NumArgs
 << SourceRange(CommandTok.getLocation(), CommandTok.getEndLocation());
   }
Index: clang/include/clang/Basic/Diagnostic.h
===
--- clang/include/clang/Basic/Diagnostic.h
+++ clang/include/clang/Basic/Diagnostic.h
@@ -1404,7 +1404,13 @@
 }
 
 inline const StreamingDiagnostic <<(const StreamingDiagnostic ,
- int64_t I) {
+ long I) {
+  DB.AddTaggedVal(I, DiagnosticsEngine::ak_sint);
+  return DB;
+}
+
+inline const StreamingDiagnostic <<(const StreamingDiagnostic ,
+ long long I) {
   DB.AddTaggedVal(I, DiagnosticsEngine::ak_sint);
   return DB;
 }
@@ -1426,7 +1432,13 @@
 }
 
 inline const StreamingDiagnostic <<(const StreamingDiagnostic ,
- uint64_t I) {
+ unsigned long I) {
+  DB.AddTaggedVal(I, DiagnosticsEngine::ak_uint);
+  return DB;
+}
+
+inline const StreamingDiagnostic <<(const StreamingDiagnostic ,
+ unsigned long long I) {
   DB.AddTaggedVal(I, DiagnosticsEngine::ak_uint);
   return DB;
 }


Index: clang/lib/AST/CommentParser.cpp
===
--- clang/lib/AST/CommentParser.cpp
+++ clang/lib/AST/CommentParser.cpp
@@ -414,7 +414,7 @@
   if (Args.size() < Info->NumArgs) {
 Diag(CommandTok.getEndLocation().getLocWithOffset(1),
  diag::warn_doc_inline_command_not_enough_arguments)
-<< CommandTok.is(tok::at_command) << Info->Name << (uint64_t)Args.size()
+<< CommandTok.is(tok::at_command) << Info->Name << Args.size()
 << Info->NumArgs
 << SourceRange(CommandTok.getLocation(), CommandTok.getEndLocation());
   }
Index: clang/include/clang/Basic/Diagnostic.h
===
--- clang/include/clang/Basic/Diagnostic.h
+++ clang/include/clang/Basic/Diagnostic.h
@@ -1404,7 +1404,13 @@
 }
 
 inline const StreamingDiagnostic <<(const StreamingDiagnostic ,
- int64_t I) {
+ long I) {
+  DB.AddTaggedVal(I, DiagnosticsEngine::ak_sint);
+  return DB;
+}
+
+inline const StreamingDiagnostic <<(const StreamingDiagnostic ,
+ long long I) {
   DB.AddTaggedVal(I, DiagnosticsEngine::ak_sint);
   return DB;
 }
@@ -1426,7 +1432,13 @@
 }
 
 inline const StreamingDiagnostic <<(const StreamingDiagnostic ,
- uint64_t I) {
+ unsigned long I) {
+  DB.AddTaggedVal(I, DiagnosticsEngine::ak_uint);
+  return DB;
+}
+
+inline const StreamingDiagnostic <<(const StreamingDiagnostic ,
+ unsigned long long I) {
   DB.AddTaggedVal(I, DiagnosticsEngine::ak_uint);
   return DB;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] ac7a9ef - Resolve overload ambiguity on Mac OS when printing size_t in diagnostics

[Aaron Puchert via cfe-commits]

Author: Aaron Puchert
Date: 2022-05-14T12:37:36+02:00
New Revision: ac7a9ef0ae3a5c63dc4e641f9912d8b659ebd720

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

LOG: Resolve overload ambiguity on Mac OS when printing size_t in diagnostics

Precommit builds cover Linux and Windows, but this ambiguity would only
show up on Mac OS: there we have int32_t = int, int64_t = long long and
size_t = unsigned long. So printing a size_t, while successful on the
other two architectures, cannot be unambiguously resolved on Mac OS.

This is not really meant to support printing arguments of type long or
size_t, but more as a way to prevent build breakage that would not be
detected in precommit builds, as happened in D125429.

Technically we have no guarantee that one of these types has the 64 bits
that afdac5fbcb6a3 wanted to provide, so proposals are welcome. We do
have a guarantee though that these three types are different, so we
should be fine with overload resolution.

Reviewed By: aeubanks

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

Added: 


Modified: 
clang/include/clang/Basic/Diagnostic.h
clang/lib/AST/CommentParser.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/Diagnostic.h 
b/clang/include/clang/Basic/Diagnostic.h
index dc1a0efe1c47..33ad0827c0ca 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -1404,7 +1404,13 @@ inline const StreamingDiagnostic <<(const 
StreamingDiagnostic ,
 }
 
 inline const StreamingDiagnostic <<(const StreamingDiagnostic ,
- int64_t I) {
+ long I) {
+  DB.AddTaggedVal(I, DiagnosticsEngine::ak_sint);
+  return DB;
+}
+
+inline const StreamingDiagnostic <<(const StreamingDiagnostic ,
+ long long I) {
   DB.AddTaggedVal(I, DiagnosticsEngine::ak_sint);
   return DB;
 }
@@ -1426,7 +1432,13 @@ inline const StreamingDiagnostic <<(const 
StreamingDiagnostic ,
 }
 
 inline const StreamingDiagnostic <<(const StreamingDiagnostic ,
- uint64_t I) {
+ unsigned long I) {
+  DB.AddTaggedVal(I, DiagnosticsEngine::ak_uint);
+  return DB;
+}
+
+inline const StreamingDiagnostic <<(const StreamingDiagnostic ,
+ unsigned long long I) {
   DB.AddTaggedVal(I, DiagnosticsEngine::ak_uint);
   return DB;
 }

diff  --git a/clang/lib/AST/CommentParser.cpp b/clang/lib/AST/CommentParser.cpp
index d78b3ace2bb8..7bac1fb99b88 100644
--- a/clang/lib/AST/CommentParser.cpp
+++ b/clang/lib/AST/CommentParser.cpp
@@ -414,7 +414,7 @@ InlineCommandComment *Parser::parseInlineCommand() {
   if (Args.size() < Info->NumArgs) {
 Diag(CommandTok.getEndLocation().getLocWithOffset(1),
  diag::warn_doc_inline_command_not_enough_arguments)
-<< CommandTok.is(tok::at_command) << Info->Name << 
(uint64_t)Args.size()
+<< CommandTok.is(tok::at_command) << Info->Name << Args.size()
 << Info->NumArgs
 << SourceRange(CommandTok.getLocation(), CommandTok.getEndLocation());
   }



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

[PATCH] D123878: [AMDGPU] Add remarks to output some resource usage

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D123878#3506500 , @afanfa wrote:

> If possible, I would like to keep some kind of delimiter. I like the idea of 
> having it at the beginning and at the end of the section. The best option 
> would be to convince clang to print new lines.

But it’s not a section and no actual grouping concept here. You just happen to 
see this printed in order. Any delimiter should be introduced as a display 
function, not emitted as part of the remarks themselves


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123878

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

[PATCH] D124816: [LibTooling] use ToolFileManager to store file managers for each CWD

[Shi Chen via Phabricator via cfe-commits]

Kale added inline comments.



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:171-172
 FileMgr->getFileSystemOpts().WorkingDir = std::string(WorkingDirectory);
 ScanInstance.setFileManager(FileMgr);
 ScanInstance.createSourceManager(*FileMgr);
 

dexonsmith wrote:
> Do these need to be moved lower, after the FileMgr/VFS might change?
Seems `visitPrebuiltModule` in line 179 uses the FileManager set by 
`ScanInstance.setFileManager(FileMgr);`, perhaps it's better to keep it here, 
and redo this after `createVFSFromCompilerInvocation`.



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:202-203
   // filesystem.
-  FileMgr->setVirtualFileSystem(createVFSFromCompilerInvocation(
+  ToolFileMgr->setVirtualFileSystem(createVFSFromCompilerInvocation(
   ScanInstance.getInvocation(), ScanInstance.getDiagnostics(), DepFS));
 

dexonsmith wrote:
> I don't see how calling this on `ToolFileMgr` threads through to updating the 
> `FileMgr` in use, which has already been sent to `ScanInstance`.
> 
> Note that `createVFSFromCompilerInvocation()` *always* returns a new VFS, 
> even if it's equivalent to the one used before (same DepFS options and same 
> `-ivfsoverlay`s from `ScanInstance.getInvocation()`). Threading it through 
> would mean that `FileMgr` is never reused.
> 
> IMO, never-reuse-the-filemanager is the right/correct behaviour for 
> clang-scan-deps, since DepFS provides caching for things that can be cached 
> soundly, but probably better to explicitly stop reusing the FileMgr (in a 
> separate patch ahead of time) rather than leaving code that looks like it 
> might get reused.
That makes sense. Will prepend a patch to explicitly create a new FileManager 
if DepFS is used.



Comment at: clang/lib/Tooling/Tooling.cpp:447-448
   appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
   if (Files)
 Files->setVirtualFileSystem(OverlayFileSystem);
 }

dexonsmith wrote:
> This will *never* reuse the file manager, since it's *always* a new VFS.
> - Maybe that's correct, since we want to inject different things into 
> InMemoryFS?
> - Or, maybe that's overly pessimistic, and we should pass BaseFS into the 
> filemanager if `appendArgumentsAdjuster()` never added anything to InMemoryFS?
Use BaseFS as the condition to judge whether to invalidate previous file 
managers? Sounds worth a try. Will amend later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124816

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

[PATCH] D124974: [clang] Include clang config.h in LangStandards.cpp

[Cassie Jones via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2e2709479636: [clang] Include clang config.h in 
LangStandards.cpp (authored by porglezomp).

Changed prior to commit:
  https://reviews.llvm.org/D124974?vs=429275=429416#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124974

Files:
  clang/lib/Basic/LangStandards.cpp


Index: clang/lib/Basic/LangStandards.cpp
===
--- clang/lib/Basic/LangStandards.cpp
+++ clang/lib/Basic/LangStandards.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "clang/Basic/LangStandard.h"
+#include "clang/Config/config.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/ErrorHandling.h"


Index: clang/lib/Basic/LangStandards.cpp
===
--- clang/lib/Basic/LangStandards.cpp
+++ clang/lib/Basic/LangStandards.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "clang/Basic/LangStandard.h"
+#include "clang/Config/config.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/ErrorHandling.h"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] ae8bbc4 - [clang] Require including config.h for CLANG_DEFAULT_STD_C

[Cassie Jones via cfe-commits]

Author: Cassie Jones
Date: 2022-05-14T01:48:14-07:00
New Revision: ae8bbc43f4709b910cd6c1e1ddc5bc854785a142

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

LOG: [clang] Require including config.h for CLANG_DEFAULT_STD_C

This makes CLANG_DEFAULT_STD_C(XX) always be defined, defaulting to
lang_unspecified, so you are forced to check its value instead of using
an #ifdef. This should help avoid accidentally omitting the include in
places where that's important, so that the default language version bug
isn't re-introduced.

Reviewed By: hokein, dexonsmith

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

Added: 


Modified: 
clang/include/clang/Config/config.h.cmake
clang/lib/Basic/LangStandards.cpp

Removed: 




diff  --git a/clang/include/clang/Config/config.h.cmake 
b/clang/include/clang/Config/config.h.cmake
index 680cc7310f76d..dfd2f757a185b 100644
--- a/clang/include/clang/Config/config.h.cmake
+++ b/clang/include/clang/Config/config.h.cmake
@@ -16,9 +16,21 @@
 
 /* Default C/ObjC standard to use. */
 #cmakedefine CLANG_DEFAULT_STD_C LangStandard::lang_${CLANG_DEFAULT_STD_C}
+/* Always #define something so that missing the config.h #include at use sites
+ * becomes a compile error.
+ */
+#ifndef CLANG_DEFAULT_STD_C
+#define CLANG_DEFAULT_STD_C LangStandard::lang_unspecified
+#endif
 
 /* Default C++/ObjC++ standard to use. */
 #cmakedefine CLANG_DEFAULT_STD_CXX LangStandard::lang_${CLANG_DEFAULT_STD_CXX}
+/* Always #define something so that missing the config.h #include at use sites
+ * becomes a compile error.
+ */
+#ifndef CLANG_DEFAULT_STD_CXX
+#define CLANG_DEFAULT_STD_CXX LangStandard::lang_unspecified
+#endif
 
 /* Default C++ stdlib to use. */
 #define CLANG_DEFAULT_CXX_STDLIB "${CLANG_DEFAULT_CXX_STDLIB}"

diff  --git a/clang/lib/Basic/LangStandards.cpp 
b/clang/lib/Basic/LangStandards.cpp
index 6643af38ba01c..a21898dd3c627 100644
--- a/clang/lib/Basic/LangStandards.cpp
+++ b/clang/lib/Basic/LangStandards.cpp
@@ -58,30 +58,27 @@ LangStandard::Kind 
clang::getDefaultLanguageStandard(clang::Language Lang,
 return LangStandard::lang_cuda;
   case Language::Asm:
   case Language::C:
-#if defined(CLANG_DEFAULT_STD_C)
-return CLANG_DEFAULT_STD_C;
-#else
+if (CLANG_DEFAULT_STD_C != LangStandard::lang_unspecified)
+  return CLANG_DEFAULT_STD_C;
+
 // The PS4 uses C99 as the default C standard.
 if (T.isPS4())
   return LangStandard::lang_gnu99;
 return LangStandard::lang_gnu17;
-#endif
   case Language::ObjC:
-#if defined(CLANG_DEFAULT_STD_C)
-return CLANG_DEFAULT_STD_C;
-#else
+if (CLANG_DEFAULT_STD_C != LangStandard::lang_unspecified)
+  return CLANG_DEFAULT_STD_C;
+
 return LangStandard::lang_gnu11;
-#endif
   case Language::CXX:
   case Language::ObjCXX:
-#if defined(CLANG_DEFAULT_STD_CXX)
-return CLANG_DEFAULT_STD_CXX;
-#else
+if (CLANG_DEFAULT_STD_CXX != LangStandard::lang_unspecified)
+  return CLANG_DEFAULT_STD_CXX;
+
 if (T.isDriverKit())
   return LangStandard::lang_gnucxx17;
 else
   return LangStandard::lang_gnucxx14;
-#endif
   case Language::RenderScript:
 return LangStandard::lang_c99;
   case Language::HIP:



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

[clang] 2e27094 - [clang] Include clang config.h in LangStandards.cpp

[Cassie Jones via cfe-commits]

Author: Cassie Jones
Date: 2022-05-14T01:47:41-07:00
New Revision: 2e270947963659cf9db4099f42387144feb10fec

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

LOG: [clang] Include clang config.h in LangStandards.cpp

This is necessary in order to pick up the default C/C++ standard from
the CLANG_DEFAULT_STD_C(XX) defines. This fixes a bug that was
introduced when this default language standard code was moved from
Frontend to Basic, making compilers ignore the configured default
language version override.

Fixes a bug introduced by D121375.

Reviewed By: hokein, dexonsmith

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

Added: 


Modified: 
clang/lib/Basic/LangStandards.cpp

Removed: 




diff  --git a/clang/lib/Basic/LangStandards.cpp 
b/clang/lib/Basic/LangStandards.cpp
index 11599cf96b33a..6643af38ba01c 100644
--- a/clang/lib/Basic/LangStandards.cpp
+++ b/clang/lib/Basic/LangStandards.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "clang/Basic/LangStandard.h"
+#include "clang/Config/config.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/ErrorHandling.h"



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

[PATCH] D94727: [clangd] Retire some flags for uncontroversial, stable features.

[Ivan Murashko via Phabricator via cfe-commits]

ivanmurashko added inline comments.
Herald added projects: clang-tools-extra, All.



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:762
 Opts.ResourceDir = ResourceDir;
-  Opts.BuildDynamicSymbolIndex = EnableIndex;
+  Opts.BuildDynamicSymbolIndex = true;
   Opts.CollectMainFileRefs = CollectMainFileRefs;

@sammccall The option is always true, do we need it as an option?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94727

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

[PATCH] D124751: [HLSL] Support -E option for HLSL.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Is there a specification or reference implementation stating that -E is used?

> Option<["--", "/", "-"], "E",

Do you need the prefix `--`? You may define something like `CLFlag`. I have 
missed previous patches, but if `/` options are not necessary, removing them 
will be the best to avoid collision with filenames starting with `/`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124751

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

[PATCH] D124974: [clang] Include clang config.h in LangStandards.cpp

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.

LGTM, thanks! One comment online below.




Comment at: clang/include/clang/Config/config.h.cmake:19
 #cmakedefine CLANG_DEFAULT_STD_C LangStandard::lang_${CLANG_DEFAULT_STD_C}
+// Always #define something so that missing the config.h #include at use sites
+// becomes a compile error.

I just noticed that the other comments in this file are C-style. Probably the 
new ones should be too. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124974

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