[PATCH] D149165: [clangd] Deduplicate missing-include findings

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo added a comment.

Thank you!




Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:419
   });
+  // Put possibly equal diagnostics together for deduplication.
+  // The duplicates might be from macro arguments that get expanded multiple

Could you move the below to a separate function with a descriptive name? IMO 
taking care of the special cases inline makes functions very long and distracts 
the reader from their main purpose.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:432
+  });
+  MissingIncludes.erase(llvm::unique(MissingIncludes), MissingIncludes.end());
   std::vector UnusedIncludes =

Nit: maybe add a comment explaining that `llvm::unique` returns a past-the-end 
iterator after deduplicating elements? I've spent a bit of time deciphering 
this line.



Comment at: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp:417
+#include "all.h"
+FOO([[Foo]]);)cpp");
+

Nit: move `)cpp");` to the next line please.



Comment at: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp:421
+  TU.AdditionalFiles["foo.h"] = guard("struct Foo {};");
+  TU.AdditionalFiles["all.h"] = guard(R"(
+#include "foo.h"

Nit: use the same style for multiline strings, i.e., `R"cpp(`.



Comment at: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp:423
+#include "foo.h"
+#define FOO(X) X y; X z)");
+

Nit: move closing tokens of the multiline string to next line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149165

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

[PATCH] D149119: [CMake] Use llvm-nm to extract symbols for staged LTO builds on Windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D149119#4298024 , @phosek wrote:

> Supporting only a single tool and simplifying the script would be my 
> preference as well. I see that the script already supports `llvm-readobj`, do 
> we need the `llvm-nm` support in that case?

I remember seeing it mentioned somewhere (I don't immediately see where right 
now though), `llvm-readobj` only handles regular object files, while `llvm-nm` 
handles bitcode too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149119

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

[PATCH] D149165: [clangd] Deduplicate missing-include findings

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 517076.
kadircet added a comment.

- Reorganize string literals in tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149165

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -403,15 +403,36 @@
   ParsedAST AST = TU.build();
 
   auto Findings = computeIncludeCleanerFindings(AST).MissingIncludes;
-  // FIXME: Deduplicate references resulting from expansion of the same macro 
in
-  // multiple places.
-  EXPECT_THAT(Findings, testing::SizeIs(2));
+  EXPECT_THAT(Findings, testing::SizeIs(1));
   auto RefRange = Findings.front().SymRefRange;
   auto &SM = AST.getSourceManager();
   EXPECT_EQ(RefRange.file(), SM.getMainFileID());
   // FIXME: Point at the spelling location, rather than the include.
   EXPECT_EQ(halfOpenToRange(SM, RefRange.toCharRange(SM)), MainFile.range());
-  EXPECT_EQ(RefRange, Findings[1].SymRefRange);
+}
+
+TEST(IncludeCleaner, MissingIncludesAreUnique) {
+  Annotations MainFile(R"cpp(
+#include "all.h"
+FOO([[Foo]]);
+  )cpp");
+
+  TestTU TU;
+  TU.AdditionalFiles["foo.h"] = guard("struct Foo {};");
+  TU.AdditionalFiles["all.h"] = guard(R"cpp(
+#include "foo.h"
+#define FOO(X) X y; X z
+  )cpp");
+
+  TU.Code = MainFile.code();
+  ParsedAST AST = TU.build();
+
+  auto Findings = computeIncludeCleanerFindings(AST).MissingIncludes;
+  EXPECT_THAT(Findings, testing::SizeIs(1));
+  auto RefRange = Findings.front().SymRefRange;
+  auto &SM = AST.getSourceManager();
+  EXPECT_EQ(RefRange.file(), SM.getMainFileID());
+  EXPECT_EQ(halfOpenToRange(SM, RefRange.toCharRange(SM)), MainFile.range());
 }
 
 TEST(IncludeCleaner, NoCrash) {
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -40,6 +40,7 @@
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/GenericUniformityImpl.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/SmallString.h"
@@ -415,6 +416,20 @@
 Ref.Target, TouchingTokens.back().range(SM), Providers};
 MissingIncludes.push_back(std::move(DiagInfo));
   });
+  // Put possibly equal diagnostics together for deduplication.
+  // The duplicates might be from macro arguments that get expanded multiple
+  // times.
+  llvm::stable_sort(MissingIncludes, [](const MissingIncludeDiagInfo &LHS,
+const MissingIncludeDiagInfo &RHS) {
+if (LHS.Symbol == RHS.Symbol) {
+  // We can get away just by comparing the offsets as all the ranges are in
+  // main file.
+  return LHS.SymRefRange.beginOffset() < RHS.SymRefRange.beginOffset();
+}
+// If symbols are different we don't care about the ordering.
+return true;
+  });
+  MissingIncludes.erase(llvm::unique(MissingIncludes), MissingIncludes.end());
   std::vector UnusedIncludes =
   getUnused(AST, Used, /*ReferencedPublicHeaders*/ {});
   return {std::move(UnusedIncludes), std::move(MissingIncludes)};


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -403,15 +403,36 @@
   ParsedAST AST = TU.build();
 
   auto Findings = computeIncludeCleanerFindings(AST).MissingIncludes;
-  // FIXME: Deduplicate references resulting from expansion of the same macro in
-  // multiple places.
-  EXPECT_THAT(Findings, testing::SizeIs(2));
+  EXPECT_THAT(Findings, testing::SizeIs(1));
   auto RefRange = Findings.front().SymRefRange;
   auto &SM = AST.getSourceManager();
   EXPECT_EQ(RefRange.file(), SM.getMainFileID());
   // FIXME: Point at the spelling location, rather than the include.
   EXPECT_EQ(halfOpenToRange(SM, RefRange.toCharRange(SM)), MainFile.range());
-  EXPECT_EQ(RefRange, Findings[1].SymRefRange);
+}
+
+TEST(IncludeCleaner, MissingIncludesAreUnique) {
+  Annotations MainFile(R"cpp(
+#include "all.h"
+FOO([[Foo]]);
+  )cpp");
+
+  TestTU TU;
+  TU.AdditionalFiles["foo.h"] = guard("struct Foo {};");
+  TU.AdditionalFiles["all.h"] = guard(R"cpp(
+#include "foo.h"
+#define FOO(X) X y; X z
+  )cpp");
+
+  TU.Code = MainFile.code();
+  ParsedAST AST = TU.build();
+
+  auto Findings = computeIncludeCleanerFindings(AST).MissingIncludes;
+  EXPECT_THAT(Findings, testing::SizeIs(1));
+  auto Re

[PATCH] D149165: [clangd] Deduplicate missing-include findings

[Kadir Cetinkaya via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:419
   });
+  // Put possibly equal diagnostics together for deduplication.
+  // The duplicates might be from macro arguments that get expanded multiple

VitaNuo wrote:
> Could you move the below to a separate function with a descriptive name? IMO 
> taking care of the special cases inline makes functions very long and 
> distracts the reader from their main purpose.
but this is not a special case, this is still part of "missing include 
information gathering", and is being applied to all of the findings (just like 
how all the used headers get accumulated in a `set`, the only problem is 
members of `MissingIncludeDiagInfo` is hard to order, hence we can't directly 
use a `set`)



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:432
+  });
+  MissingIncludes.erase(llvm::unique(MissingIncludes), MissingIncludes.end());
   std::vector UnusedIncludes =

VitaNuo wrote:
> Nit: maybe add a comment explaining that `llvm::unique` returns a 
> past-the-end iterator after deduplicating elements? I've spent a bit of time 
> deciphering this line.
yeah the language isn't really helpful in that regard but unfortunately `unique 
& erase` pattern is pretty much the idiomatic way of deduplicating a sorted 
container. I can add it here but we have some more mentions of it across the 
code that doesn't have any explanations. so I am not sure what kind of weight 
it would carry.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149165

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

[PATCH] D149210: [IR] Change shufflevector undef mask to poison

[Nikita Popov via Phabricator via cfe-commits]

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

Could you please split the change to printing + the test updates from all the 
other changes? The code changes get lost in the large diff.




Comment at: llvm/include/llvm-c/Core.h:4090
  */
-int LLVMGetUndefMaskElem(void);
+int LLVMGetPoisonMaskElem(void);
 

This breaks C API compatibility without good cause. Either leave this name 
alone or add a function with the new name and mark this one as deprecated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149210

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

[PATCH] D149206: [clang][driver] Enable MisExpect diagnostics flag outside of CC1

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149206

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

[clang] 3185e47 - [clang][Interp] Fix post-inc/dec operator result

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2023-04-26T09:36:08+02:00
New Revision: 3185e47b5a8444e9fd70b746a7ad679dd131ffe4

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

LOG: [clang][Interp] Fix post-inc/dec operator result

We pushed the wrong value on the stack, always leaving a 0 behind.

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

Added: 


Modified: 
clang/lib/AST/Interp/Interp.h
clang/test/AST/Interp/literals.cpp

Removed: 




diff  --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
index 1eda38e9fa5b1..152a876a42964 100644
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -436,7 +436,7 @@ bool IncDecHelper(InterpState &S, CodePtr OpPC, const 
Pointer &Ptr) {
   T Result;
 
   if constexpr (DoPush == PushVal::Yes)
-S.Stk.push(Result);
+S.Stk.push(Value);
 
   if constexpr (Op == IncDecOp::Inc) {
 if (!T::increment(Value, &Result)) {

diff  --git a/clang/test/AST/Interp/literals.cpp 
b/clang/test/AST/Interp/literals.cpp
index 54f3aefcef2b1..598d748c266b4 100644
--- a/clang/test/AST/Interp/literals.cpp
+++ b/clang/test/AST/Interp/literals.cpp
@@ -719,6 +719,23 @@ namespace IncDec {
  // ref-note {{cannot refer to element -1 of array of 3 elements}}
 return *p;
   }
+
+  /// This used to leave a 0 on the stack instead of the previous
+  /// value of a.
+  constexpr int bug1Inc() {
+int a = 3;
+int b = a++;
+return b;
+  }
+  static_assert(bug1Inc() == 3);
+
+  constexpr int bug1Dec() {
+int a = 3;
+int b = a--;
+return b;
+  }
+  static_assert(bug1Dec() == 3);
+
 };
 #endif
 



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

[PATCH] D148901: [clang][Interp] Fix post-inc/dec operator result

[Timm Bäder 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 rG3185e47b5a84: [clang][Interp] Fix post-inc/dec operator 
result (authored by tbaeder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148901

Files:
  clang/lib/AST/Interp/Interp.h
  clang/test/AST/Interp/literals.cpp


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -719,6 +719,23 @@
  // ref-note {{cannot refer to element -1 of array of 3 elements}}
 return *p;
   }
+
+  /// This used to leave a 0 on the stack instead of the previous
+  /// value of a.
+  constexpr int bug1Inc() {
+int a = 3;
+int b = a++;
+return b;
+  }
+  static_assert(bug1Inc() == 3);
+
+  constexpr int bug1Dec() {
+int a = 3;
+int b = a--;
+return b;
+  }
+  static_assert(bug1Dec() == 3);
+
 };
 #endif
 
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -436,7 +436,7 @@
   T Result;
 
   if constexpr (DoPush == PushVal::Yes)
-S.Stk.push(Result);
+S.Stk.push(Value);
 
   if constexpr (Op == IncDecOp::Inc) {
 if (!T::increment(Value, &Result)) {


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -719,6 +719,23 @@
  // ref-note {{cannot refer to element -1 of array of 3 elements}}
 return *p;
   }
+
+  /// This used to leave a 0 on the stack instead of the previous
+  /// value of a.
+  constexpr int bug1Inc() {
+int a = 3;
+int b = a++;
+return b;
+  }
+  static_assert(bug1Inc() == 3);
+
+  constexpr int bug1Dec() {
+int a = 3;
+int b = a--;
+return b;
+  }
+  static_assert(bug1Dec() == 3);
+
 };
 #endif
 
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -436,7 +436,7 @@
   T Result;
 
   if constexpr (DoPush == PushVal::Yes)
-S.Stk.push(Result);
+S.Stk.push(Value);
 
   if constexpr (Op == IncDecOp::Inc) {
 if (!T::increment(Value, &Result)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133863: [RISCV] Add MC support of RISCV zcmt Extension

[Xinlong Wu via Phabricator via cfe-commits]

VincentWu updated this revision to Diff 517078.
VincentWu marked 2 inline comments as done.
VincentWu added a comment.

address comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133863

Files:
  clang/test/Preprocessor/riscv-target-features.c
  llvm/docs/RISCVUsage.rst
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/RISCVFeatures.td
  llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
  llvm/lib/Target/RISCV/RISCVSchedRocket.td
  llvm/lib/Target/RISCV/RISCVSchedSiFive7.td
  llvm/lib/Target/RISCV/RISCVSystemOperands.td
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/rv32zcmt-invalid.s
  llvm/test/MC/RISCV/rv32zcmt-valid.s
  llvm/test/MC/RISCV/rvzcmt-user-csr-name.s

Index: llvm/test/MC/RISCV/rvzcmt-user-csr-name.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rvzcmt-user-csr-name.s
@@ -0,0 +1,29 @@
+# RUN: llvm-mc %s -triple=riscv32 -riscv-no-aliases -mattr=+experimental-zcmt -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-INST,CHECK-ENC %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+experimental-zcmt < %s \
+# RUN: | llvm-objdump -d --mattr=+experimental-zcmt - \
+# RUN: | FileCheck -check-prefix=CHECK-INST-ALIAS %s
+#
+# RUN: llvm-mc %s -triple=riscv64 -riscv-no-aliases -mattr=+experimental-zcmt -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-INST,CHECK-ENC %s
+# RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+experimental-zcmt < %s \
+# RUN: | llvm-objdump -d --mattr=+experimental-zcmt - \
+# RUN: | FileCheck -check-prefix=CHECK-INST-ALIAS %s
+
+##
+# Jump Vector Table CSR
+##
+
+# jvt
+# name
+# CHECK-INST: csrrs t1, jvt, zero
+# CHECK-ENC:  encoding: [0x73,0x23,0x70,0x01]
+# CHECK-INST-ALIAS: csrr t1, jvt
+# uimm12
+# CHECK-INST: csrrs t2, jvt, zero
+# CHECK-ENC:  encoding: [0xf3,0x23,0x70,0x01]
+# CHECK-INST-ALIAS: csrr t2, jvt
+# name
+csrrs t1, jvt, zero
+# uimm12
+csrrs t2, 0x017, zero
Index: llvm/test/MC/RISCV/rv32zcmt-valid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rv32zcmt-valid.s
@@ -0,0 +1,39 @@
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+experimental-zcmt\
+# RUN:  -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv32 -mattr=+experimental-zcmt\
+# RUN:  -mattr=m < %s \
+# RUN: | llvm-objdump --mattr=+experimental-zcmt\
+# RUN:  -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefixes=CHECK-OBJ,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+experimental-zcmt\
+# RUN:  -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+experimental-zcmt\
+# RUN:  -mattr=m < %s \
+# RUN: | llvm-objdump --mattr=+experimental-zcmt\
+# RUN:  -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefixes=CHECK-OBJ,CHECK-ASM-AND-OBJ %s
+#
+# RUN: not llvm-mc -triple riscv32 \
+# RUN: -riscv-no-aliases -show-encoding < %s 2>&1 \
+# RUN: | FileCheck -check-prefixes=CHECK-NO-EXT %s
+# RUN: not llvm-mc -triple riscv64 \
+# RUN: -riscv-no-aliases -show-encoding < %s 2>&1 \
+# RUN: | FileCheck -check-prefixes=CHECK-NO-EXT %s
+
+# CHECK-ASM-AND-OBJ: cm.jt 1
+# CHECK-ASM: encoding: [0x06,0xa0]
+# CHECK-NO-EXT: error: instruction requires the following: 'Zcmt' (table jump instuctions for code-size reduction){{$}}
+cm.jt 1
+
+# CHECK-ASM: cm.jalt 1
+# CHECK-OBJ: cm.jt 1
+# CHECK-ASM: encoding: [0x06,0xa0]
+# CHECK-NO-EXT: error: instruction requires the following: 'Zcmt' (table jump instuctions for code-size reduction){{$}}
+cm.jalt 1
+
+# CHECK-ASM-AND-OBJ: cm.jalt 32
+# CHECK-ASM: encoding: [0x82,0xa0]
+# CHECK-NO-EXT: error: instruction requires the following: 'Zcmt' (table jump instuctions for code-size reduction){{$}}
+cm.jalt 32
Index: llvm/test/MC/RISCV/rv32zcmt-invalid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rv32zcmt-invalid.s
@@ -0,0 +1,10 @@
+# RUN: not llvm-mc -triple=riscv32 -mattr=+experimental-zcmt -riscv-no-aliases -show-encoding < %s 2>&1 \
+# RUN: | FileCheck -check-prefixes=CHECK-ERROR %s
+# RUN: not llvm-mc -triple=riscv64 -mattr=+experimental-zcmt -riscv-no-aliases -show-encoding < %s 2>&1 \
+# RUN: | FileCheck -check-prefixes=CHECK-ERROR %s
+
+# CHECK-ERROR: error: immediate must be an integer in the range [0, 31]
+cm.jt 64
+
+# CHECK-ERROR: error: immediate must be an integer in the range [0, 255]
+cm.jalt 256
Index: llvm/test/MC/RISCV/attribute-arch.s
===
--- llvm/test/MC/RISCV/attribute-arch.s
+++ llvm/test/MC/RISCV/attribute-arc

[PATCH] D133863: [RISCV] Add MC support of RISCV zcmt Extension

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: llvm/lib/Support/RISCVISAInfo.cpp:915
+return createStringError(errc::invalid_argument,
+ "'zcmt' is incompatible with 'c' or 'zcd' "
+ "extensions when 'd' extensions is set");

The ‘or’ in the error message makes it unnecessarily complex for users. Tell 
them exactly what extensions they used that are in compatible. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133863

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

[PATCH] D146358: [clang][AST] Print name instead of type when diagnosing uninitialized subobject in constexpr variables

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

tbaeder added a comment.

@aaron.ballman Are you OK with the approach taken here?


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

https://reviews.llvm.org/D146358

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

[clang-tools-extra] 08f0725 - [clangd] Deduplicate missing-include findings

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2023-04-26T10:33:55+02:00
New Revision: 08f0725a3a5471d16b8a1e9d1d55b9952300ac41

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

LOG: [clangd] Deduplicate missing-include findings

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

Added: 


Modified: 
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/IncludeCleaner.cpp 
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index d15dd70efdb70..b88ad0ff5c5b5 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -40,6 +40,7 @@
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/GenericUniformityImpl.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/SmallString.h"
@@ -415,6 +416,20 @@ IncludeCleanerFindings 
computeIncludeCleanerFindings(ParsedAST &AST) {
 Ref.Target, TouchingTokens.back().range(SM), Providers};
 MissingIncludes.push_back(std::move(DiagInfo));
   });
+  // Put possibly equal diagnostics together for deduplication.
+  // The duplicates might be from macro arguments that get expanded multiple
+  // times.
+  llvm::stable_sort(MissingIncludes, [](const MissingIncludeDiagInfo &LHS,
+const MissingIncludeDiagInfo &RHS) {
+if (LHS.Symbol == RHS.Symbol) {
+  // We can get away just by comparing the offsets as all the ranges are in
+  // main file.
+  return LHS.SymRefRange.beginOffset() < RHS.SymRefRange.beginOffset();
+}
+// If symbols are 
diff erent we don't care about the ordering.
+return true;
+  });
+  MissingIncludes.erase(llvm::unique(MissingIncludes), MissingIncludes.end());
   std::vector UnusedIncludes =
   getUnused(AST, Used, /*ReferencedPublicHeaders*/ {});
   return {std::move(UnusedIncludes), std::move(MissingIncludes)};

diff  --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp 
b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
index 39901fda31656..32b7c5444e06f 100644
--- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -403,15 +403,36 @@ TEST(IncludeCleaner, MacroExpandedThroughIncludes) {
   ParsedAST AST = TU.build();
 
   auto Findings = computeIncludeCleanerFindings(AST).MissingIncludes;
-  // FIXME: Deduplicate references resulting from expansion of the same macro 
in
-  // multiple places.
-  EXPECT_THAT(Findings, testing::SizeIs(2));
+  EXPECT_THAT(Findings, testing::SizeIs(1));
   auto RefRange = Findings.front().SymRefRange;
   auto &SM = AST.getSourceManager();
   EXPECT_EQ(RefRange.file(), SM.getMainFileID());
   // FIXME: Point at the spelling location, rather than the include.
   EXPECT_EQ(halfOpenToRange(SM, RefRange.toCharRange(SM)), MainFile.range());
-  EXPECT_EQ(RefRange, Findings[1].SymRefRange);
+}
+
+TEST(IncludeCleaner, MissingIncludesAreUnique) {
+  Annotations MainFile(R"cpp(
+#include "all.h"
+FOO([[Foo]]);
+  )cpp");
+
+  TestTU TU;
+  TU.AdditionalFiles["foo.h"] = guard("struct Foo {};");
+  TU.AdditionalFiles["all.h"] = guard(R"cpp(
+#include "foo.h"
+#define FOO(X) X y; X z
+  )cpp");
+
+  TU.Code = MainFile.code();
+  ParsedAST AST = TU.build();
+
+  auto Findings = computeIncludeCleanerFindings(AST).MissingIncludes;
+  EXPECT_THAT(Findings, testing::SizeIs(1));
+  auto RefRange = Findings.front().SymRefRange;
+  auto &SM = AST.getSourceManager();
+  EXPECT_EQ(RefRange.file(), SM.getMainFileID());
+  EXPECT_EQ(halfOpenToRange(SM, RefRange.toCharRange(SM)), MainFile.range());
 }
 
 TEST(IncludeCleaner, NoCrash) {



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

[PATCH] D149165: [clangd] Deduplicate missing-include findings

[Kadir Cetinkaya 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 rG08f0725a3a54: [clangd] Deduplicate missing-include findings 
(authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149165

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -403,15 +403,36 @@
   ParsedAST AST = TU.build();
 
   auto Findings = computeIncludeCleanerFindings(AST).MissingIncludes;
-  // FIXME: Deduplicate references resulting from expansion of the same macro 
in
-  // multiple places.
-  EXPECT_THAT(Findings, testing::SizeIs(2));
+  EXPECT_THAT(Findings, testing::SizeIs(1));
   auto RefRange = Findings.front().SymRefRange;
   auto &SM = AST.getSourceManager();
   EXPECT_EQ(RefRange.file(), SM.getMainFileID());
   // FIXME: Point at the spelling location, rather than the include.
   EXPECT_EQ(halfOpenToRange(SM, RefRange.toCharRange(SM)), MainFile.range());
-  EXPECT_EQ(RefRange, Findings[1].SymRefRange);
+}
+
+TEST(IncludeCleaner, MissingIncludesAreUnique) {
+  Annotations MainFile(R"cpp(
+#include "all.h"
+FOO([[Foo]]);
+  )cpp");
+
+  TestTU TU;
+  TU.AdditionalFiles["foo.h"] = guard("struct Foo {};");
+  TU.AdditionalFiles["all.h"] = guard(R"cpp(
+#include "foo.h"
+#define FOO(X) X y; X z
+  )cpp");
+
+  TU.Code = MainFile.code();
+  ParsedAST AST = TU.build();
+
+  auto Findings = computeIncludeCleanerFindings(AST).MissingIncludes;
+  EXPECT_THAT(Findings, testing::SizeIs(1));
+  auto RefRange = Findings.front().SymRefRange;
+  auto &SM = AST.getSourceManager();
+  EXPECT_EQ(RefRange.file(), SM.getMainFileID());
+  EXPECT_EQ(halfOpenToRange(SM, RefRange.toCharRange(SM)), MainFile.range());
 }
 
 TEST(IncludeCleaner, NoCrash) {
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -40,6 +40,7 @@
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/GenericUniformityImpl.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/SmallString.h"
@@ -415,6 +416,20 @@
 Ref.Target, TouchingTokens.back().range(SM), Providers};
 MissingIncludes.push_back(std::move(DiagInfo));
   });
+  // Put possibly equal diagnostics together for deduplication.
+  // The duplicates might be from macro arguments that get expanded multiple
+  // times.
+  llvm::stable_sort(MissingIncludes, [](const MissingIncludeDiagInfo &LHS,
+const MissingIncludeDiagInfo &RHS) {
+if (LHS.Symbol == RHS.Symbol) {
+  // We can get away just by comparing the offsets as all the ranges are in
+  // main file.
+  return LHS.SymRefRange.beginOffset() < RHS.SymRefRange.beginOffset();
+}
+// If symbols are different we don't care about the ordering.
+return true;
+  });
+  MissingIncludes.erase(llvm::unique(MissingIncludes), MissingIncludes.end());
   std::vector UnusedIncludes =
   getUnused(AST, Used, /*ReferencedPublicHeaders*/ {});
   return {std::move(UnusedIncludes), std::move(MissingIncludes)};


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -403,15 +403,36 @@
   ParsedAST AST = TU.build();
 
   auto Findings = computeIncludeCleanerFindings(AST).MissingIncludes;
-  // FIXME: Deduplicate references resulting from expansion of the same macro in
-  // multiple places.
-  EXPECT_THAT(Findings, testing::SizeIs(2));
+  EXPECT_THAT(Findings, testing::SizeIs(1));
   auto RefRange = Findings.front().SymRefRange;
   auto &SM = AST.getSourceManager();
   EXPECT_EQ(RefRange.file(), SM.getMainFileID());
   // FIXME: Point at the spelling location, rather than the include.
   EXPECT_EQ(halfOpenToRange(SM, RefRange.toCharRange(SM)), MainFile.range());
-  EXPECT_EQ(RefRange, Findings[1].SymRefRange);
+}
+
+TEST(IncludeCleaner, MissingIncludesAreUnique) {
+  Annotations MainFile(R"cpp(
+#include "all.h"
+FOO([[Foo]]);
+  )cpp");
+
+  TestTU TU;
+  TU.AdditionalFiles["foo.h"] = guard("struct Foo {};");
+  TU.AdditionalFiles["all.h"] = guard(R"cpp(
+#include "foo.h"
+#define FOO(X) X y; X z
+  )cpp");
+
+  TU.Code = MainFile.code();
+  ParsedAST AST = TU.build();
+
+  aut

[PATCH] D147684: [clangd] Add batch fixes for include-cleaner diagnostics

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 517093.
hokein marked 8 inline comments as done.
hokein added a comment.

address review comments:

- emit one annotation per textedit;
- remove unnecessary std::optional in the protocol structure
- add a lit test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147684

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/Diagnostics.h
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/test/Inputs/include-cleaner/all1.h
  clang-tools-extra/clangd/test/Inputs/include-cleaner/all2.h
  clang-tools-extra/clangd/test/Inputs/include-cleaner/bar.h
  clang-tools-extra/clangd/test/Inputs/include-cleaner/foo.h
  clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -45,8 +45,8 @@
 using ::testing::Pointee;
 using ::testing::UnorderedElementsAre;
 
-Matcher withFix(::testing::Matcher FixMatcher) {
-  return Field(&Diag::Fixes, ElementsAre(FixMatcher));
+Matcher withFix(std::vector<::testing::Matcher> FixMatcheres) {
+  return Field(&Diag::Fixes, testing::UnorderedElementsAreArray(FixMatcheres));
 }
 
 MATCHER_P2(Diag, Range, Message,
@@ -60,6 +60,8 @@
   return arg.Message == Message && arg.Edits.size() == 1 &&
  arg.Edits[0].range == Range && arg.Edits[0].newText == Replacement;
 }
+MATCHER_P(FixMessage, Message, "") { return arg.Message == Message; }
+
 
 std::string guard(llvm::StringRef Code) {
   return "#pragma once\n" + Code.str();
@@ -255,42 +257,51 @@
   UnorderedElementsAre(
   AllOf(Diag(MainFile.range("b"),
  "No header providing \"b\" is directly included"),
-withFix(Fix(MainFile.range("insert_b"), "#include \"b.h\"\n",
-"#include \"b.h\""))),
+withFix({Fix(MainFile.range("insert_b"), "#include \"b.h\"\n",
+ "#include \"b.h\""),
+ FixMessage("add all missing includes")})),
   AllOf(Diag(MainFile.range("bar"),
  "No header providing \"ns::Bar\" is directly included"),
-withFix(Fix(MainFile.range("insert_d"),
-"#include \"dir/d.h\"\n", "#include \"dir/d.h\""))),
+withFix({Fix(MainFile.range("insert_d"),
+ "#include \"dir/d.h\"\n", "#include \"dir/d.h\""),
+ FixMessage("add all missing includes")})),
   AllOf(Diag(MainFile.range("f"),
  "No header providing \"f\" is directly included"),
-withFix(Fix(MainFile.range("insert_f"), "#include \n",
-"#include "))),
+withFix({Fix(MainFile.range("insert_f"), "#include \n",
+ "#include "),
+ FixMessage("add all missing includes")})),
   AllOf(
   Diag(MainFile.range("foobar"),
"No header providing \"foobar\" is directly included"),
-  withFix(Fix(MainFile.range("insert_foobar"),
-  "#include \"public.h\"\n", "#include \"public.h\""))),
+  withFix({Fix(MainFile.range("insert_foobar"),
+   "#include \"public.h\"\n", "#include \"public.h\""),
+   FixMessage("add all missing includes")})),
   AllOf(
   Diag(MainFile.range("vector"),
"No header providing \"std::vector\" is directly included"),
-  withFix(Fix(MainFile.range("insert_vector"),
-  "#include \n", "#include "))),
+  withFix({Fix(MainFile.range("insert_vector"),
+   "#include \n", "#include "),
+   FixMessage("add all missing includes"),})),
   AllOf(Diag(MainFile.range("FOO"),
  "No header providing \"FOO\" is directly included"),
-withFix(Fix(MainFile.range("insert_foo"),
-"#include \"foo.h\"\n", "#include \"foo.h\""))),
+withFix({Fix(MainFile.range("insert_foo"),
+ "#include \"foo.h\"\n", "#include \"foo.h\""),
+ FixMessage("add all missing includes")})),
   AllOf(Diag(MainFile.range("DEF"),
  "No header providing \"Foo\" is directly included"),
-withFix(Fix(MainFile.range("

[PATCH] D148110: [clang-tidy] bugprone-use-after-move: Ctor arguments should be sequenced if ctor call is written as list-initialization.

[Martin Böhme via Phabricator via cfe-commits]

mboehme updated this revision to Diff 517094.
mboehme added a comment.

Reworded release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148110

Files:
  clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
@@ -1160,42 +1160,63 @@
   }
 }
 
+namespace InitializerListSequences {
+
+struct S1 {
+  int i;
+  A a;
+};
+
+struct S2 {
+  A a;
+  int i;
+};
+
+struct S3 {
+  S3() {}
+  S3(int, A) {}
+  S3(A, int) {}
+};
+
 // An initializer list sequences its initialization clauses.
 void initializerListSequences() {
   {
-struct S1 {
-  int i;
-  A a;
-};
-{
-  A a;
-  S1 s1{a.getInt(), std::move(a)};
-}
-{
-  A a;
-  S1 s1{.i = a.getInt(), .a = std::move(a)};
-}
+A a;
+S1 s1{a.getInt(), std::move(a)};
   }
   {
-struct S2 {
-  A a;
-  int i;
-};
-{
-  A a;
-  S2 s2{std::move(a), a.getInt()};
-  // CHECK-NOTES: [[@LINE-1]]:27: warning: 'a' used after it was moved
-  // CHECK-NOTES: [[@LINE-2]]:13: note: move occurred here
-}
-{
-  A a;
-  S2 s2{.a = std::move(a), .i = a.getInt()};
-  // CHECK-NOTES: [[@LINE-1]]:37: warning: 'a' used after it was moved
-  // CHECK-NOTES: [[@LINE-2]]:13: note: move occurred here
-}
+A a;
+S1 s1{.i = a.getInt(), .a = std::move(a)};
+  }
+  {
+A a;
+S2 s2{std::move(a), a.getInt()};
+// CHECK-NOTES: [[@LINE-1]]:25: warning: 'a' used after it was moved
+// CHECK-NOTES: [[@LINE-2]]:11: note: move occurred here
+  }
+  {
+A a;
+S2 s2{.a = std::move(a), .i = a.getInt()};
+// CHECK-NOTES: [[@LINE-1]]:35: warning: 'a' used after it was moved
+// CHECK-NOTES: [[@LINE-2]]:11: note: move occurred here
+  }
+  {
+// Check the case where the constructed type has a constructor and the
+// initializer list therefore manifests as a `CXXConstructExpr` instead of
+// an `InitListExpr`.
+A a;
+S3 s3{a.getInt(), std::move(a)};
+  }
+  {
+A a;
+S3 s3{std::move(a), a.getInt()};
+// CHECK-NOTES: [[@LINE-1]]:25: warning: 'a' used after it was moved
+// CHECK-NOTES: [[@LINE-2]]:11: note: move occurred here
   }
 }
 
+} // namespace InitializerListSequences
+
 // A declaration statement containing multiple declarations sequences the
 // initializer expressions.
 void declarationSequences() {
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -214,8 +214,9 @@
   to ``std::forward``.
 
 - Improved :doc:`bugprone-use-after-move
-  ` check to also cover constructor
-  initializers.
+  ` check: Detect uses and moves in
+  constructor initializers. Fix check to understand that constructor arguments
+  are sequenced when constructor call is written as list-initialization.
 
 - Deprecated :doc:`cert-dcl21-cpp
   ` check.
Index: clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
===
--- clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
+++ clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
@@ -131,6 +131,16 @@
   }
 }
   }
+} else if (const auto *ConstructExpr = dyn_cast(Parent)) {
+  // Constructor arguments are sequenced if the constructor call is written
+  // as list-initialization.
+  if (ConstructExpr->isListInitialization()) {
+for (unsigned I = 1; I < ConstructExpr->getNumArgs(); ++I) {
+  if (ConstructExpr->getArg(I - 1) == S) {
+return ConstructExpr->getArg(I);
+  }
+}
+  }
 } else if (const auto *Compound = dyn_cast(Parent)) {
   // Compound statement: Each sub-statement is sequenced after the
   // statements that precede it.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147684: [clangd] Add batch fixes for include-cleaner diagnostics

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 517096.
hokein added a comment.

more cleanup


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147684

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/Diagnostics.h
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/test/Inputs/include-cleaner/all1.h
  clang-tools-extra/clangd/test/Inputs/include-cleaner/all2.h
  clang-tools-extra/clangd/test/Inputs/include-cleaner/bar.h
  clang-tools-extra/clangd/test/Inputs/include-cleaner/foo.h
  clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -45,8 +45,8 @@
 using ::testing::Pointee;
 using ::testing::UnorderedElementsAre;
 
-Matcher withFix(::testing::Matcher FixMatcher) {
-  return Field(&Diag::Fixes, ElementsAre(FixMatcher));
+Matcher withFix(std::vector<::testing::Matcher> FixMatcheres) {
+  return Field(&Diag::Fixes, testing::UnorderedElementsAreArray(FixMatcheres));
 }
 
 MATCHER_P2(Diag, Range, Message,
@@ -60,6 +60,8 @@
   return arg.Message == Message && arg.Edits.size() == 1 &&
  arg.Edits[0].range == Range && arg.Edits[0].newText == Replacement;
 }
+MATCHER_P(FixMessage, Message, "") { return arg.Message == Message; }
+
 
 std::string guard(llvm::StringRef Code) {
   return "#pragma once\n" + Code.str();
@@ -255,42 +257,51 @@
   UnorderedElementsAre(
   AllOf(Diag(MainFile.range("b"),
  "No header providing \"b\" is directly included"),
-withFix(Fix(MainFile.range("insert_b"), "#include \"b.h\"\n",
-"#include \"b.h\""))),
+withFix({Fix(MainFile.range("insert_b"), "#include \"b.h\"\n",
+ "#include \"b.h\""),
+ FixMessage("add all missing includes")})),
   AllOf(Diag(MainFile.range("bar"),
  "No header providing \"ns::Bar\" is directly included"),
-withFix(Fix(MainFile.range("insert_d"),
-"#include \"dir/d.h\"\n", "#include \"dir/d.h\""))),
+withFix({Fix(MainFile.range("insert_d"),
+ "#include \"dir/d.h\"\n", "#include \"dir/d.h\""),
+ FixMessage("add all missing includes")})),
   AllOf(Diag(MainFile.range("f"),
  "No header providing \"f\" is directly included"),
-withFix(Fix(MainFile.range("insert_f"), "#include \n",
-"#include "))),
+withFix({Fix(MainFile.range("insert_f"), "#include \n",
+ "#include "),
+ FixMessage("add all missing includes")})),
   AllOf(
   Diag(MainFile.range("foobar"),
"No header providing \"foobar\" is directly included"),
-  withFix(Fix(MainFile.range("insert_foobar"),
-  "#include \"public.h\"\n", "#include \"public.h\""))),
+  withFix({Fix(MainFile.range("insert_foobar"),
+   "#include \"public.h\"\n", "#include \"public.h\""),
+   FixMessage("add all missing includes")})),
   AllOf(
   Diag(MainFile.range("vector"),
"No header providing \"std::vector\" is directly included"),
-  withFix(Fix(MainFile.range("insert_vector"),
-  "#include \n", "#include "))),
+  withFix({Fix(MainFile.range("insert_vector"),
+   "#include \n", "#include "),
+   FixMessage("add all missing includes"),})),
   AllOf(Diag(MainFile.range("FOO"),
  "No header providing \"FOO\" is directly included"),
-withFix(Fix(MainFile.range("insert_foo"),
-"#include \"foo.h\"\n", "#include \"foo.h\""))),
+withFix({Fix(MainFile.range("insert_foo"),
+ "#include \"foo.h\"\n", "#include \"foo.h\""),
+ FixMessage("add all missing includes")})),
   AllOf(Diag(MainFile.range("DEF"),
  "No header providing \"Foo\" is directly included"),
-withFix(Fix(MainFile.range("insert_foo"),
-"#include \"foo.h\"\n", "#include \"foo.h\""))),
+withFix({Fix(MainFile.range("insert_foo"),
+   

[PATCH] D147684: [clangd] Add batch fixes for include-cleaner diagnostics

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:443
+  RemoveAll.Annotations.push_back({RemoveAllUnusedID,
+   {/*label=*/"", /*needsConfirmation=*/true,
+/*description=*/std::nullopt}});

kadircet wrote:
> rather than an empty label, let's duplicate `RemoveAll.Message` here. As that 
> context will probably be lost after executing the code action.
I don't see much value on setting the label the the RemoveAll message (the 
preview window shows the content diff, it is clear enough for users to 
understand the purpose by viewing the diff).

And since we use one annotation per edit, the `RemoveAll` message doesn't seem 
to suit anymore.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:464
+  "AddAllMissingIncludes";
+  for (auto &E : AddAllMissing.Edits) {
+E.annotationId.emplace();

kadircet wrote:
> i think you want to populate `AddAllMissing.Edits` from `Edits` first
oops, good catch!



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:468
+  }
+  // FIXME(hokein): emit used symbol reference in the annotation.
+  AddAllMissing.Annotations.push_back(

kadircet wrote:
> unless we want a really wordy description, this would actually require us to 
> attach one annotation per edit, rather than using the same annotation for all 
> the edits.
> can you check what changes in the UI when we have multiple annotations for a 
> single workspace edit?
as discussed, there is not UI difference (at least in VSCode) between these 
two, but we should swich to one annotation per edit (since the UI is based on 
the annotation, and we want each edit is controlled by the user).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147684

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

[clang] 5794ea4 - [Sema] Fix _Alignas/isCXX11Attribute() FIXME

[Richard Sandiford via cfe-commits]

Author: Richard Sandiford
Date: 2023-04-26T09:49:07+01:00
New Revision: 5794ea421a0d60802141a9f289cd2c9fa8f7416c

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

LOG: [Sema] Fix _Alignas/isCXX11Attribute() FIXME

When doing https://reviews.llvm.org/D148105 , I hadn't noticed that
there was also a FIXME about the misclassification of _Alignas in
ProcessDeclAttribute.

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

Added: 


Modified: 
clang/lib/Sema/SemaDeclAttr.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 2517dd501efe8..3cb3250f67318 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -8591,13 +8591,7 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, 
const ParsedAttr &AL,
 
   // Ignore C++11 attributes on declarator chunks: they appertain to the type
   // instead.
-  // FIXME: We currently check the attribute syntax directly instead of using
-  // isCXX11Attribute(), which currently erroneously classifies the C11
-  // `_Alignas` attribute as a C++11 attribute. `_Alignas` can appear on the
-  // `DeclSpec`, so we need to let it through here to make sure it is processed
-  // appropriately. Once the behavior of isCXX11Attribute() is fixed, we can
-  // go back to using that here.
-  if (AL.getSyntax() == ParsedAttr::AS_CXX11 && 
!Options.IncludeCXX11Attributes)
+  if (AL.isCXX11Attribute() && !Options.IncludeCXX11Attributes)
 return;
 
   // Unknown attributes are automatically warned on. Target-specific attributes



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

[PATCH] D149148: [Sema] Fix _Alignas/isCXX11Attribute() FIXME

[Richard Sandiford 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 rG5794ea421a0d: [Sema] Fix _Alignas/isCXX11Attribute() FIXME 
(authored by rsandifo-arm).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149148

Files:
  clang/lib/Sema/SemaDeclAttr.cpp


Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -8591,13 +8591,7 @@
 
   // Ignore C++11 attributes on declarator chunks: they appertain to the type
   // instead.
-  // FIXME: We currently check the attribute syntax directly instead of using
-  // isCXX11Attribute(), which currently erroneously classifies the C11
-  // `_Alignas` attribute as a C++11 attribute. `_Alignas` can appear on the
-  // `DeclSpec`, so we need to let it through here to make sure it is processed
-  // appropriately. Once the behavior of isCXX11Attribute() is fixed, we can
-  // go back to using that here.
-  if (AL.getSyntax() == ParsedAttr::AS_CXX11 && 
!Options.IncludeCXX11Attributes)
+  if (AL.isCXX11Attribute() && !Options.IncludeCXX11Attributes)
 return;
 
   // Unknown attributes are automatically warned on. Target-specific attributes


Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -8591,13 +8591,7 @@
 
   // Ignore C++11 attributes on declarator chunks: they appertain to the type
   // instead.
-  // FIXME: We currently check the attribute syntax directly instead of using
-  // isCXX11Attribute(), which currently erroneously classifies the C11
-  // `_Alignas` attribute as a C++11 attribute. `_Alignas` can appear on the
-  // `DeclSpec`, so we need to let it through here to make sure it is processed
-  // appropriately. Once the behavior of isCXX11Attribute() is fixed, we can
-  // go back to using that here.
-  if (AL.getSyntax() == ParsedAttr::AS_CXX11 && !Options.IncludeCXX11Attributes)
+  if (AL.isCXX11Attribute() && !Options.IncludeCXX11Attributes)
 return;
 
   // Unknown attributes are automatically warned on. Target-specific attributes
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] a31b3a1 - [clang[[Interp][NFC] Make some Descriptor pointers const

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2023-04-26T10:57:43+02:00
New Revision: a31b3a1a6d60cbb2ca9a110e70f0592aac574f1f

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

LOG: [clang[[Interp][NFC] Make some Descriptor pointers const

Added: 


Modified: 
clang/lib/AST/Interp/Pointer.cpp

Removed: 




diff  --git a/clang/lib/AST/Interp/Pointer.cpp 
b/clang/lib/AST/Interp/Pointer.cpp
index 8f1dfa346c63..7f127143a99d 100644
--- a/clang/lib/AST/Interp/Pointer.cpp
+++ b/clang/lib/AST/Interp/Pointer.cpp
@@ -147,7 +147,7 @@ APValue Pointer::toAPValue() const {
 
 bool Pointer::isInitialized() const {
   assert(Pointee && "Cannot check if null pointer was initialized");
-  Descriptor *Desc = getFieldDesc();
+  const Descriptor *Desc = getFieldDesc();
   assert(Desc);
   if (Desc->isPrimitiveArray()) {
 if (isStatic() && Base == 0)
@@ -167,7 +167,7 @@ bool Pointer::isInitialized() const {
 
 void Pointer::initialize() const {
   assert(Pointee && "Cannot initialize null pointer");
-  Descriptor *Desc = getFieldDesc();
+  const Descriptor *Desc = getFieldDesc();
 
   assert(Desc);
   if (Desc->isPrimitiveArray()) {



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

[PATCH] D148110: [clang-tidy] bugprone-use-after-move: Ctor arguments should be sequenced if ctor call is written as list-initialization.

[Martin Böhme via Phabricator via cfe-commits]

mboehme updated this revision to Diff 517101.
mboehme added a comment.

Reworded release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148110

Files:
  clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
@@ -1160,42 +1160,63 @@
   }
 }
 
+namespace InitializerListSequences {
+
+struct S1 {
+  int i;
+  A a;
+};
+
+struct S2 {
+  A a;
+  int i;
+};
+
+struct S3 {
+  S3() {}
+  S3(int, A) {}
+  S3(A, int) {}
+};
+
 // An initializer list sequences its initialization clauses.
 void initializerListSequences() {
   {
-struct S1 {
-  int i;
-  A a;
-};
-{
-  A a;
-  S1 s1{a.getInt(), std::move(a)};
-}
-{
-  A a;
-  S1 s1{.i = a.getInt(), .a = std::move(a)};
-}
+A a;
+S1 s1{a.getInt(), std::move(a)};
   }
   {
-struct S2 {
-  A a;
-  int i;
-};
-{
-  A a;
-  S2 s2{std::move(a), a.getInt()};
-  // CHECK-NOTES: [[@LINE-1]]:27: warning: 'a' used after it was moved
-  // CHECK-NOTES: [[@LINE-2]]:13: note: move occurred here
-}
-{
-  A a;
-  S2 s2{.a = std::move(a), .i = a.getInt()};
-  // CHECK-NOTES: [[@LINE-1]]:37: warning: 'a' used after it was moved
-  // CHECK-NOTES: [[@LINE-2]]:13: note: move occurred here
-}
+A a;
+S1 s1{.i = a.getInt(), .a = std::move(a)};
+  }
+  {
+A a;
+S2 s2{std::move(a), a.getInt()};
+// CHECK-NOTES: [[@LINE-1]]:25: warning: 'a' used after it was moved
+// CHECK-NOTES: [[@LINE-2]]:11: note: move occurred here
+  }
+  {
+A a;
+S2 s2{.a = std::move(a), .i = a.getInt()};
+// CHECK-NOTES: [[@LINE-1]]:35: warning: 'a' used after it was moved
+// CHECK-NOTES: [[@LINE-2]]:11: note: move occurred here
+  }
+  {
+// Check the case where the constructed type has a constructor and the
+// initializer list therefore manifests as a `CXXConstructExpr` instead of
+// an `InitListExpr`.
+A a;
+S3 s3{a.getInt(), std::move(a)};
+  }
+  {
+A a;
+S3 s3{std::move(a), a.getInt()};
+// CHECK-NOTES: [[@LINE-1]]:25: warning: 'a' used after it was moved
+// CHECK-NOTES: [[@LINE-2]]:11: note: move occurred here
   }
 }
 
+} // namespace InitializerListSequences
+
 // A declaration statement containing multiple declarations sequences the
 // initializer expressions.
 void declarationSequences() {
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -214,8 +214,9 @@
   to ``std::forward``.
 
 - Improved :doc:`bugprone-use-after-move
-  ` check to also cover constructor
-  initializers.
+  ` check: Detect uses and moves in
+  constructor initializers. Correctly handle constructor arguments as being
+  sequenced when constructor call is written as list-initialization.
 
 - Deprecated :doc:`cert-dcl21-cpp
   ` check.
Index: clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
===
--- clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
+++ clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
@@ -131,6 +131,16 @@
   }
 }
   }
+} else if (const auto *ConstructExpr = dyn_cast(Parent)) {
+  // Constructor arguments are sequenced if the constructor call is written
+  // as list-initialization.
+  if (ConstructExpr->isListInitialization()) {
+for (unsigned I = 1; I < ConstructExpr->getNumArgs(); ++I) {
+  if (ConstructExpr->getArg(I - 1) == S) {
+return ConstructExpr->getArg(I);
+  }
+}
+  }
 } else if (const auto *Compound = dyn_cast(Parent)) {
   // Compound statement: Each sub-statement is sequenced after the
   // statements that precede it.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148110: [clang-tidy] bugprone-use-after-move: Ctor arguments should be sequenced if ctor call is written as list-initialization.

[Martin Böhme via Phabricator via cfe-commits]

mboehme added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:216-219
 - Improved :doc:`bugprone-use-after-move
-  ` check to also cover constructor
-  initializers.
+  ` check: Also cover constructor
+  initializers. Fix check to understand that constructor arguments are
+  sequenced when constructor call is written as list-initialization.

PiotrZSL wrote:
> NOTE: Consider: "Improved XYZ check by including coverage for constructor 
> initializers, and by correcting the handling of constructor arguments when 
> they are sequenced during a constructor call that is written as 
> list-initialization." or
> "The XYZ check now covers constructor initializers and handles constructor 
> arguments correctly during list-initialization."
> 
> I  just pointing this because I don't like this "check: Also"
Makes sense. How about this? (I have more fixes in the pipeline, so I've gone 
for two separate sentences for the two fixes instead of a single sentence.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148110

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

[PATCH] D149119: [CMake] Use llvm-nm to extract symbols for staged LTO builds on Windows

[Tomas Matheson via Phabricator via cfe-commits]

tmatheson added a comment.

In D149119#4297540 , @ikudrin wrote:

> If I understand it right, we might not be able to build `llvm-nm` in cases 
> like cross-platform building, right?

LLVM has a way to build tools that need to run on the build machine as part of 
the build (`tablegen` for example), `llvm-nm` could be added to that system and 
then it would be available when `extract_symbols.py` is run. It would be an 
issue if `llvm-nm` ever had to depend on `extract_symbols.py` but that is not 
currently the case afaik.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149119

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

[PATCH] D149246: [RISCV] Relax rules for ordering s/z/x prefixed extensions in ISA naming strings

[Alex Bradbury via Phabricator via cfe-commits]

asb created this revision.
asb added reviewers: reames, kito-cheng, craig.topper, jrtc27, joshua-arch1.
Herald added subscribers: jobnoorman, luke, wingo, pmatos, VincentWu, vkmr, 
frasercrmck, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, 
benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, 
edward-jones, zzheng, shiva0217, niosHD, sabuasal, simoncook, johnrusso, rbar, 
hiraditya, arichardson.
Herald added a project: All.
asb requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, eopXD, MaskRay.
Herald added projects: clang, LLVM.

This was discussed somewhat in D148315 . As 
it stands, we require in RISCVISAInfo::parseArchString (used for e.g. `-march` 
parsing in Clang) that extensions are given in the order of z, then s, then x 
prefixed extensions (after the standard single-letter extensions). However, we 
recently (in D148315 ) moved to that order 
from z/x/s as the canonical ordering was changed in the spec. In addition, 
recent GCC seems to require z* extensions before s*.

My recollection of the history here is that we thought keeping `-march` as 
close to the rules for ISA naming strings as possible would simplify things, as 
there's an existing spec to point to. My feeling is that now we've had 
incompatible changes, and an incompatibility with GCC there's no real benefit 
to sticking to this restriction, and it risks making it much more painful than 
it needs to be to copy a `-march=` string between GCC and Clang.

This patch actually removes all ordering restrictions so you can freely mix 
x/s/z extensions. Arguably this is more freedom than we want to allow, on the 
other hand it might be less hassle for build systems assembling their arch 
strings.

To be very explicit, this doesn't change our behaviour when emitting a 
canonically ordered extension string (e.g. in build attributes). We of course 
sort according to the canonical order (as we understand it) in that case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149246

Files:
  clang/docs/ReleaseNotes.rst
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/unittests/Support/RISCVISAInfoTest.cpp


Index: llvm/unittests/Support/RISCVISAInfoTest.cpp
===
--- llvm/unittests/Support/RISCVISAInfoTest.cpp
+++ llvm/unittests/Support/RISCVISAInfoTest.cpp
@@ -192,7 +192,7 @@
   EXPECT_EQ(InfoRV64G.getFLen(), 64U);
 }
 
-TEST(ParseArchString, RequiresCanonicalOrderForExtensions) {
+TEST(ParseArchString, RequiresCanonicalOrderForSingleLetterExtensions) {
   EXPECT_EQ(
   toString(RISCVISAInfo::parseArchString("rv64idf", true).takeError()),
   "standard user-level extension not given in canonical order 'f'");
@@ -203,12 +203,10 @@
   toString(
   RISCVISAInfo::parseArchString("rv32i_zfinx_a", true).takeError()),
   "invalid extension prefix 'a'");
-  EXPECT_EQ(
-  toString(RISCVISAInfo::parseArchString("rv64i_svnapot_zicsr", true)
-   .takeError()),
-  "standard user-level extension not given in canonical order 'zicsr'");
+  // Canonical ordering not required for z*, s*, and x* extensions.
   EXPECT_THAT_EXPECTED(
-  RISCVISAInfo::parseArchString("rv64imafdc_zicsr_svnapot", true),
+  RISCVISAInfo::parseArchString(
+  "rv64imafdc_xsfvcp_zicsr_xtheadba_svnapot_zawrs", true),
   Succeeded());
 }
 
Index: llvm/lib/Support/RISCVISAInfo.cpp
===
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -767,9 +767,6 @@
   OtherExts.split(Split, '_');
 
   SmallVector AllExts;
-  std::array Prefix{"z", "s", "x"};
-  auto I = Prefix.begin();
-  auto E = Prefix.end();
   if (Split.size() > 1 || Split[0] != "") {
 for (StringRef Ext : Split) {
   if (Ext.empty())
@@ -789,18 +786,6 @@
  "invalid extension prefix '" + Ext + "'");
   }
 
-  // Check ISA extensions are specified in the canonical order.
-  while (I != E && *I != Type)
-++I;
-
-  if (I == E) {
-if (IgnoreUnknown)
-  continue;
-return createStringError(errc::invalid_argument,
- "%s not given in canonical order '%s'",
- Desc.str().c_str(), Ext.str().c_str());
-  }
-
   if (!IgnoreUnknown && Name.size() == Type.size()) {
 return createStringError(errc::invalid_argument,
  "%s name missing after '%s'",
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -422,6 +422,9 @@
 - Fixed incorrect ABI lowering of ``_Float16`` in the case of structs
   containing ``_Float16`` that are eligible for passing via GPR+FPR or
   FPR+FPR.
+- The rul

[PATCH] D149246: [RISCV] Relax rules for ordering s/z/x prefixed extensions in ISA naming strings

[Alex Bradbury via Phabricator via cfe-commits]

asb updated this revision to Diff 517105.

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

https://reviews.llvm.org/D149246

Files:
  clang/docs/ReleaseNotes.rst
  clang/test/Driver/riscv-arch.c
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/unittests/Support/RISCVISAInfoTest.cpp


Index: llvm/unittests/Support/RISCVISAInfoTest.cpp
===
--- llvm/unittests/Support/RISCVISAInfoTest.cpp
+++ llvm/unittests/Support/RISCVISAInfoTest.cpp
@@ -192,7 +192,7 @@
   EXPECT_EQ(InfoRV64G.getFLen(), 64U);
 }
 
-TEST(ParseArchString, RequiresCanonicalOrderForExtensions) {
+TEST(ParseArchString, RequiresCanonicalOrderForSingleLetterExtensions) {
   EXPECT_EQ(
   toString(RISCVISAInfo::parseArchString("rv64idf", true).takeError()),
   "standard user-level extension not given in canonical order 'f'");
@@ -203,12 +203,10 @@
   toString(
   RISCVISAInfo::parseArchString("rv32i_zfinx_a", true).takeError()),
   "invalid extension prefix 'a'");
-  EXPECT_EQ(
-  toString(RISCVISAInfo::parseArchString("rv64i_svnapot_zicsr", true)
-   .takeError()),
-  "standard user-level extension not given in canonical order 'zicsr'");
+  // Canonical ordering not required for z*, s*, and x* extensions.
   EXPECT_THAT_EXPECTED(
-  RISCVISAInfo::parseArchString("rv64imafdc_zicsr_svnapot", true),
+  RISCVISAInfo::parseArchString(
+  "rv64imafdc_xsfvcp_zicsr_xtheadba_svnapot_zawrs", true),
   Succeeded());
 }
 
Index: llvm/lib/Support/RISCVISAInfo.cpp
===
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -767,9 +767,6 @@
   OtherExts.split(Split, '_');
 
   SmallVector AllExts;
-  std::array Prefix{"z", "s", "x"};
-  auto I = Prefix.begin();
-  auto E = Prefix.end();
   if (Split.size() > 1 || Split[0] != "") {
 for (StringRef Ext : Split) {
   if (Ext.empty())
@@ -789,18 +786,6 @@
  "invalid extension prefix '" + Ext + "'");
   }
 
-  // Check ISA extensions are specified in the canonical order.
-  while (I != E && *I != Type)
-++I;
-
-  if (I == E) {
-if (IgnoreUnknown)
-  continue;
-return createStringError(errc::invalid_argument,
- "%s not given in canonical order '%s'",
- Desc.str().c_str(), Ext.str().c_str());
-  }
-
   if (!IgnoreUnknown && Name.size() == Type.size()) {
 return createStringError(errc::invalid_argument,
  "%s name missing after '%s'",
Index: clang/test/Driver/riscv-arch.c
===
--- clang/test/Driver/riscv-arch.c
+++ clang/test/Driver/riscv-arch.c
@@ -333,8 +333,7 @@
 // RUN: %clang --target=riscv32-unknown-elf -march=rv32ixdef_sabc -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-X-ORDER %s
 // RV32-X-ORDER: error: invalid arch name 'rv32ixdef_sabc',
-// RV32-X-ORDER: standard supervisor-level extension not given
-// RV32-X-ORDER: in canonical order 'sabc'
+// RV32-X-ORDER  unsupported non-standard user-level extension 'xdef'
 
 // RUN: %clang --target=riscv32-unknown-elf -march=rv32ixabc_xabc -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-XDUP %s
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -422,6 +422,9 @@
 - Fixed incorrect ABI lowering of ``_Float16`` in the case of structs
   containing ``_Float16`` that are eligible for passing via GPR+FPR or
   FPR+FPR.
+- The rules for ordering of extensions in ``-march`` strings were relaxed. A
+  canonical ordering is no longer enforced on ``z*``, ``s*``, and ``x*``
+  prefixed extensions.
 
 CUDA/HIP Language Changes
 ^


Index: llvm/unittests/Support/RISCVISAInfoTest.cpp
===
--- llvm/unittests/Support/RISCVISAInfoTest.cpp
+++ llvm/unittests/Support/RISCVISAInfoTest.cpp
@@ -192,7 +192,7 @@
   EXPECT_EQ(InfoRV64G.getFLen(), 64U);
 }
 
-TEST(ParseArchString, RequiresCanonicalOrderForExtensions) {
+TEST(ParseArchString, RequiresCanonicalOrderForSingleLetterExtensions) {
   EXPECT_EQ(
   toString(RISCVISAInfo::parseArchString("rv64idf", true).takeError()),
   "standard user-level extension not given in canonical order 'f'");
@@ -203,12 +203,10 @@
   toString(
   RISCVISAInfo::parseArchString("rv32i_zfinx_a", true).takeError()),
   "invalid extension prefix 'a'");
-  EXPECT_EQ(
-  toString(RISCVISAInfo::parseArchString("rv64i_svnapot_zicsr", true)
-   .takeError()),
-  "standard user-level extension not given in canonical order 'zicsr'");
+  // Canonical ordering not required for z*, s*, and x* exten

[PATCH] D149246: [RISCV] Relax rules for ordering s/z/x prefixed extensions in ISA naming strings

[Alex Bradbury via Phabricator via cfe-commits]

asb updated this revision to Diff 517106.
asb added a comment.

Add missing doc comment update.


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

https://reviews.llvm.org/D149246

Files:
  clang/docs/ReleaseNotes.rst
  clang/test/Driver/riscv-arch.c
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/unittests/Support/RISCVISAInfoTest.cpp

Index: llvm/unittests/Support/RISCVISAInfoTest.cpp
===
--- llvm/unittests/Support/RISCVISAInfoTest.cpp
+++ llvm/unittests/Support/RISCVISAInfoTest.cpp
@@ -192,7 +192,7 @@
   EXPECT_EQ(InfoRV64G.getFLen(), 64U);
 }
 
-TEST(ParseArchString, RequiresCanonicalOrderForExtensions) {
+TEST(ParseArchString, RequiresCanonicalOrderForSingleLetterExtensions) {
   EXPECT_EQ(
   toString(RISCVISAInfo::parseArchString("rv64idf", true).takeError()),
   "standard user-level extension not given in canonical order 'f'");
@@ -203,12 +203,10 @@
   toString(
   RISCVISAInfo::parseArchString("rv32i_zfinx_a", true).takeError()),
   "invalid extension prefix 'a'");
-  EXPECT_EQ(
-  toString(RISCVISAInfo::parseArchString("rv64i_svnapot_zicsr", true)
-   .takeError()),
-  "standard user-level extension not given in canonical order 'zicsr'");
+  // Canonical ordering not required for z*, s*, and x* extensions.
   EXPECT_THAT_EXPECTED(
-  RISCVISAInfo::parseArchString("rv64imafdc_zicsr_svnapot", true),
+  RISCVISAInfo::parseArchString(
+  "rv64imafdc_xsfvcp_zicsr_xtheadba_svnapot_zawrs", true),
   Succeeded());
 }
 
Index: llvm/lib/Support/RISCVISAInfo.cpp
===
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -756,9 +756,9 @@
   // Parse the ISA string containing non-standard user-level
   // extensions, standard supervisor-level extensions and
   // non-standard supervisor-level extensions.
-  // These extensions start with 'z', 's', 'x' prefixes, follow a
-  // canonical order, might have a version number (major, minor)
-  // and are separated by a single underscore '_'.
+  // These extensions start with 'z', 's', 'x' prefixes, might have a version
+  // number (major, minor) and are separated by a single underscore '_'. We do
+  // not enforce a canonical order for them.
   // Set the hardware features for the extensions that are supported.
 
   // Multi-letter extensions are seperated by a single underscore
@@ -767,9 +767,6 @@
   OtherExts.split(Split, '_');
 
   SmallVector AllExts;
-  std::array Prefix{"z", "s", "x"};
-  auto I = Prefix.begin();
-  auto E = Prefix.end();
   if (Split.size() > 1 || Split[0] != "") {
 for (StringRef Ext : Split) {
   if (Ext.empty())
@@ -789,18 +786,6 @@
  "invalid extension prefix '" + Ext + "'");
   }
 
-  // Check ISA extensions are specified in the canonical order.
-  while (I != E && *I != Type)
-++I;
-
-  if (I == E) {
-if (IgnoreUnknown)
-  continue;
-return createStringError(errc::invalid_argument,
- "%s not given in canonical order '%s'",
- Desc.str().c_str(), Ext.str().c_str());
-  }
-
   if (!IgnoreUnknown && Name.size() == Type.size()) {
 return createStringError(errc::invalid_argument,
  "%s name missing after '%s'",
Index: clang/test/Driver/riscv-arch.c
===
--- clang/test/Driver/riscv-arch.c
+++ clang/test/Driver/riscv-arch.c
@@ -333,8 +333,7 @@
 // RUN: %clang --target=riscv32-unknown-elf -march=rv32ixdef_sabc -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-X-ORDER %s
 // RV32-X-ORDER: error: invalid arch name 'rv32ixdef_sabc',
-// RV32-X-ORDER: standard supervisor-level extension not given
-// RV32-X-ORDER: in canonical order 'sabc'
+// RV32-X-ORDER  unsupported non-standard user-level extension 'xdef'
 
 // RUN: %clang --target=riscv32-unknown-elf -march=rv32ixabc_xabc -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-XDUP %s
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -422,6 +422,9 @@
 - Fixed incorrect ABI lowering of ``_Float16`` in the case of structs
   containing ``_Float16`` that are eligible for passing via GPR+FPR or
   FPR+FPR.
+- The rules for ordering of extensions in ``-march`` strings were relaxed. A
+  canonical ordering is no longer enforced on ``z*``, ``s*``, and ``x*``
+  prefixed extensions.
 
 CUDA/HIP Language Changes
 ^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148110: [clang-tidy] bugprone-use-after-move: Ctor arguments should be sequenced if ctor call is written as list-initialization.

[Piotr Zegar via Phabricator via cfe-commits]

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

+- LGTM




Comment at: clang-tools-extra/docs/ReleaseNotes.rst:217
 - Improved :doc:`bugprone-use-after-move
-  ` check to also cover constructor
-  initializers.
+  ` check: Detect uses and moves in
+  constructor initializers. Correctly handle constructor arguments as being

':' -> '.'


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148110

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

[PATCH] D148949: [dataflow] HTMLLogger - show the value of the current expr

[Martin Böhme via Phabricator via cfe-commits]

mboehme added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp:96
+for (const auto& Prop : V.properties())
+  JOS.attributeObject(Prop.first(), [&] { dump(*Prop.second); });
+

sammccall wrote:
> mboehme wrote:
> > sammccall wrote:
> > > mboehme wrote:
> > > > IIUC, this places properties on the current HTML element as attributes, 
> > > > just like the built-in attributes that we add for other purposes (e.g. 
> > > > "value_id", "kind").
> > > > 
> > > >   - What happens if we have a property whose name conflicts with one of 
> > > > the built-in attributes?
> > > >   - Even if we don't have a naming conflict, I think it could be 
> > > > potentially confusing to have user-defined properties appear in the 
> > > > same list and in the same way as built-in attributes.
> > > > 
> > > > Suggestion: Can we nest all properties inside of a "properties" 
> > > > attribute?
> > > > 
> > > > Edit: Having looked at the HTML template now, I see that we exclude 
> > > > certain attributes there ('kind', 'value_id', 'type', 'location') when 
> > > > listing properties. I still think naming conflicts are a potential 
> > > > problem though. I think it would also be clearer to explicitly pick the 
> > > > properties out of a `properties` attribute rather than excluding a 
> > > > blocklist of attributes.
> > > Right, the data model is: a value (really, a Value/StorageLocation mashed 
> > > together) is just a bag of attributes.
> > > 
> > > I don't think making it more complicated is an improvement: being 
> > > built-in isn't the same thing as being custom-rendered.
> > > e.g. "pointee" and "truth" want the default key-value rendering despite 
> > > being built-in.
> > > Having the exclude list in the template is ugly, but either you end up 
> > > encoding the rendering info twice in the template like that, or you 
> > > encode it once in the template and once in the JSON generation (by what 
> > > goes in the "properties" map vs the main map). I'd rather call this 
> > > purely a template concern.
> > > 
> > > Namespace conflict could be a problem: the current behavior is that the 
> > > last value wins (per JS rules).
> > > IMO the simplest fix is to prepend "p:" and "f:" to properties/struct 
> > > fields. These would be shown - otherwise the user can't distinguish 
> > > between a property & field with the same name.
> > > 
> > > I had this in the prototype, but dropped them because they seemed a bit 
> > > ugly and conflicts unlikely in practice. WDYT?
> > > Namespace conflict could be a problem: the current behavior is that the 
> > > last value wins (per JS rules).
> > > IMO the simplest fix is to prepend "p:" and "f:" to properties/struct 
> > > fields. These would be shown - otherwise the user can't distinguish 
> > > between a property & field with the same name.
> > 
> > Yes, this makes sense to me. I looked at your example screenshot and wasn't 
> > sure whether they were both fields or whether one of them was a property -- 
> > I think there's value in indicating explicitly what they are.
> > 
> > > I had this in the prototype, but dropped them because they seemed a bit 
> > > ugly and conflicts unlikely in practice. WDYT?
> > 
> > I do think there's a fair chance of conflicts -- many of the attribute 
> > names here are short and generic and look like likely field names (e.g. 
> > `kind`, `type`). Even if the chance of a conflict is relatively low, a 
> > conflict will be pretty confusing when it does happen -- and given that 
> > we'll be using this feature when we're debugging (i.e. already confused), I 
> > think this is worth avoiding.
> > 
> > One more question: How do the "p:" and "f:" items sort in the output? I 
> > think these should be sorted together and grouped -- e.g. builtins first, 
> > then fields, then properties. (Yes, I know this is more work... but I think 
> > it's worth it.)
> > One more question: How do the "p:" and "f:" items sort in the output? I 
> > think these should be sorted together and grouped -- e.g. builtins first, 
> > then fields, then properties. (Yes, I know this is more work... but I think 
> > it's worth it.)
> 
> Javascript objects are ordered these days, so they'll display in the order we 
> output them here.
> So they're already grouped, I rearranged to put properties at the end.
> > One more question: How do the "p:" and "f:" items sort in the output? I 
> > think these should be sorted together and grouped -- e.g. builtins first, 
> > then fields, then properties. (Yes, I know this is more work... but I think 
> > it's worth it.)
> 
> Javascript objects are ordered these days, so they'll display in the order we 
> output them here.

Got it, thanks.

> So they're already grouped, I rearranged to put properties at the end.

SG



Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp:121-123
+  for (const auto &Child : cast(V).children())
+JOS.attributeObject(C

[PATCH] D148783: [clangd] Add support TextDocumentEdit.

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

Hi, these tests are failing on our (Linaro's) Windows on Arm buildbot. First 
appearance here https://lab.llvm.org/buildbot/#/builders/65/builds/9346

Still failing as of a few hours ago 
https://lab.llvm.org/buildbot/#/builders/65/builds/9360 (maybe you have fixed 
this already, I am checking in infrequently over the next few days).

If you need any help debugging it let me know. It appears there is a drive 
letter on Windows:

  # CHECK-NEXT:  "uri": "file:///clangd-test/foo.c",
 ^
  :224:30: note: scanning from here
  "textDocument": {
   ^
  :225:15: note: possible intended match here
"uri": "file:///C:/clangd-test/foo.c",
^


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148783

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

[PATCH] D148730: [C11] Allow initialization of an atomic-qualified pointer from a null pointer constant

[Bogdan Graur via Phabricator via cfe-commits]

bgraur added a comment.

@aaron.ballman your patch (1395cde24b3641e284bb1daae7d56c189a2635e3 
) fixed 
all issues we encountered. Thanks!!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148730

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

[PATCH] D138810: [RISCV] Support vector crypto extension C intrinsics

[Brandon Wu via Phabricator via cfe-commits]

4vtomat updated this revision to Diff 517113.
4vtomat added a comment.

Update to version 0.5.1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138810

Files:
  clang/include/clang/Basic/riscv_vector.td
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaesdf.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaesdm.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaesef.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaesem.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaeskf1.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaeskf2.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaesz.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vandn.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vbrev.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vbrev8.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vclmul.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vclmulh.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vclz.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vcpopv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vctz.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vghsh.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vgmul.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vrev8.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vrol.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vror.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vsha2ch.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vsha2cl.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vsha2ms.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vsm3c.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vsm3me.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vsm4k.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vsm4r.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vwsll.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vaesdf.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vaesdm.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vaesef.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vaesem.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vaeskf1.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vaeskf2.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vaesz.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vandn.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vbrev.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vbrev8.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vclmul.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vclmulh.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vclz.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vcpopv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vctz.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vghsh.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vgmul.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vrev8.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vrol.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vror.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vsha2ch.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vsha2cl.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vsha2ms.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vsm3c.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics

[PATCH] D148663: [RFC][clangd] Use interpolation for CDB pushed via LSP protocol

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin added a comment.

@kadircet @nridge friendly ping, could you please take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148663

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

[PATCH] D149248: [RISCV][MC] MC layer support for the experimental zacas extension

[Alex Bradbury via Phabricator via cfe-commits]

asb created this revision.
asb added reviewers: reames, craig.topper, kito-cheng.
Herald added subscribers: jobnoorman, luke, wingo, pmatos, VincentWu, vkmr, 
frasercrmck, jdoerfert, evandro, luismarques, apazos, sameer.abuasal, 
s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, 
rogfer01, edward-jones, zzheng, jrtc27, shiva0217, niosHD, sabuasal, simoncook, 
johnrusso, rbar, hiraditya, arichardson.
Herald added a project: All.
asb requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, eopXD, MaskRay.
Herald added projects: clang, LLVM.

This implements the v0.1 draft extension.

amocas.d on RV32 and amocas.q have the restriction that rd and rs2 must be even 
registers. I've opted to implement this restriction in 
RISCVAsmParser::validateInstruction even though for codegen we'll need a new 
register class and can then remove this validation. My reasoning is that this 
validation is easy to implement and review, while ensuring the register class 
is correct as described isn't so easy without testing that is only added when 
codegen is implemented. Admittedly, the need to check for the aq/rl/aqrl opcode 
variants makes the validateInstruction
logic a bit uglier. Happy to switch approach if preferred by reviewers.

It's not a pre-requisite for this patch, but we likely want to have an asm 
constraint for even registers. I've filed an issue here 
https://github.com/riscv-non-isa/riscv-c-api-doc/issues/37


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149248

Files:
  clang/test/Preprocessor/riscv-target-features.c
  llvm/docs/RISCVUsage.rst
  llvm/docs/ReleaseNotes.rst
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/RISCVFeatures.td
  llvm/lib/Target/RISCV/RISCVInstrInfoA.td
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/rv32zacas-invalid.s
  llvm/test/MC/RISCV/rv32zacas-valid.s
  llvm/test/MC/RISCV/rv64zacas-invalid.s
  llvm/test/MC/RISCV/rv64zacas-valid.s

Index: llvm/test/MC/RISCV/rv64zacas-valid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rv64zacas-valid.s
@@ -0,0 +1,51 @@
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+experimental-zacas -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+experimental-zacas < %s \
+# RUN: | llvm-objdump --mattr=+experimental-zacas -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefix=CHECK-ASM-AND-OBJ %s
+# RUN: not llvm-mc -triple=riscv64 -mattr=+a -show-encoding %s 2>&1 \
+# RUN:| FileCheck %s --check-prefix=CHECK-ERROR
+
+# Odd register numbers for rd and rs2 are allowed for amocas.d on RV64.
+
+# CHECK-ASM-AND-OBJ: amocas.d a1, a3, (a5)
+# CHECK-ASM: encoding: [0xaf,0xb5,0xd7,0x28]
+# CHECK-ERROR: instruction requires the following: 'Zacas' (Atomic Compare-And-Swap Instructions){{$}}
+amocas.d a1, a3, (a5)
+# CHECK-ASM-AND-OBJ: amocas.d.aq a1, a3, (a5)
+# CHECK-ASM: encoding: [0xaf,0xb5,0xd7,0x2c]
+# CHECK-ERROR: instruction requires the following: 'Zacas' (Atomic Compare-And-Swap Instructions){{$}}
+amocas.d.aq a1, a3, (a5)
+# CHECK-ASM-AND-OBJ: amocas.d.rl a1, a3, (a5)
+# CHECK-ASM: encoding: [0xaf,0xb5,0xd7,0x2a]
+# CHECK-ERROR: instruction requires the following: 'Zacas' (Atomic Compare-And-Swap Instructions){{$}}
+amocas.d.rl a1, a3, (a5)
+# CHECK-ASM-AND-OBJ: amocas.d.aqrl a1, a3, (a5)
+# CHECK-ASM: encoding: [0xaf,0xb5,0xd7,0x2e]
+# CHECK-ERROR: instruction requires the following: 'Zacas' (Atomic Compare-And-Swap Instructions){{$}}
+amocas.d.aqrl a1, a3, (a5)
+
+# CHECK-ASM-AND-OBJ: amocas.q a0, a2, (a1)
+# CHECK-ASM: encoding: [0x2f,0xc5,0xc5,0x28]
+# CHECK-ERROR: instruction requires the following: 'Zacas' (Atomic Compare-And-Swap Instructions){{$}}
+amocas.q a0, a2, (a1)
+# CHECK-ASM-AND-OBJ: amocas.q a0, a2, (a1)
+# CHECK-ASM: encoding: [0x2f,0xc5,0xc5,0x28]
+# CHECK-ERROR: instruction requires the following: 'Zacas' (Atomic Compare-And-Swap Instructions){{$}}
+amocas.q a0, a2, 0(a1)
+# CHECK-ASM-AND-OBJ: amocas.q zero, zero, (a1)
+# CHECK-ASM: encoding: [0x2f,0xc0,0x05,0x28]
+# CHECK-ERROR: instruction requires the following: 'Zacas' (Atomic Compare-And-Swap Instructions){{$}}
+amocas.q zero, zero, (a1)
+# CHECK-ASM-AND-OBJ: amocas.q.aq zero, zero, (a1)
+# CHECK-ASM: encoding: [0x2f,0xc0,0x05,0x2c]
+# CHECK-ERROR: instruction requires the following: 'Zacas' (Atomic Compare-And-Swap Instructions){{$}}
+amocas.q.aq zero, zero, (a1)
+# CHECK-ASM-AND-OBJ: amocas.q.rl zero, zero, (a1)
+# CHECK-ASM: encoding: [0x2f,0xc0,0x05,0x2a]
+# CHECK-ERROR: instruction requires the following: 'Zacas' (Atomic Compare-And-Swap Instructions){{$}}
+amocas.q.rl zero, zero, (a1)
+# CHECK-ASM-AND-OBJ: amocas.q.aqrl zero, zero, (a1)
+# CHECK-ASM: encoding: [0x2f,0xc0,0x05,0x2e]
+# CHECK-ERROR: instruc

[clang] 343bdb1 - [analyzer] Show taint origin and propagation correctly

[Daniel Krupp via cfe-commits]

Author: Daniel Krupp
Date: 2023-04-26T12:43:36+02:00
New Revision: 343bdb10940cb2387c0b9bd3caccee7bb56c937b

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

LOG: [analyzer] Show taint origin and propagation correctly

This patch improves the diagnostics of the alpha.security.taint.TaintPropagation
checker and taint related checkers by showing the "Taint originated here" note
at the correct place, where the attacker may inject it. This greatly improves
the understandability of the taint reports.

In the baseline the taint source was pointing to an invalid location, typically
somewhere between the real taint source and sink.

After the fix, the "Taint originated here" tag is correctly shown at the taint
source. This is the function call where the attacker can inject a malicious data
(e.g. reading from environment variable, reading from file, reading from
standard input etc.).

This patch removes the BugVisitor from the implementation and replaces it with 2
new NoteTags. One, in the taintOriginTrackerTag() prints the "taint originated
here" Note and the other in taintPropagationExplainerTag() explaining how the
taintedness is propagating from argument to argument or to the return value
("Taint propagated to the Xth argument"). This implementation uses the
interestingess BugReport utility to track back the tainted symbols through
propagating function calls to the point where the taintedness was introduced by
a source function call.

The checker which wishes to emit a Taint related diagnostic must use the
categories::TaintedData BugType category and must mark the tainted symbols as
interesting. Then the TaintPropagationChecker will automatically generate the
"Taint originated here" and the "Taint propagated to..." diagnostic notes.

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Checkers/Taint.h
clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h
clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
clang/lib/StaticAnalyzer/Checkers/Taint.cpp
clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
clang/test/Analysis/taint-diagnostic-visitor.c
clang/test/Analysis/taint-tester.c

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Taint.h 
b/clang/include/clang/StaticAnalyzer/Checkers/Taint.h
index df863a2495413..3ec8dbfb09ee3 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Taint.h
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Taint.h
@@ -79,26 +79,48 @@ bool isTainted(ProgramStateRef State, SymbolRef Sym,
 bool isTainted(ProgramStateRef State, const MemRegion *Reg,
TaintTagType Kind = TaintTagGeneric);
 
+/// Returns the tainted Symbols for a given Statement and state.
+std::vector getTaintedSymbols(ProgramStateRef State, const Stmt *S,
+ const LocationContext *LCtx,
+ TaintTagType Kind = TaintTagGeneric);
+
+/// Returns the tainted Symbols for a given SVal and state.
+std::vector getTaintedSymbols(ProgramStateRef State, SVal V,
+ TaintTagType Kind = TaintTagGeneric);
+
+/// Returns the tainted Symbols for a SymbolRef and state.
+std::vector getTaintedSymbols(ProgramStateRef State, SymbolRef Sym,
+ TaintTagType Kind = TaintTagGeneric);
+
+/// Returns the tainted (index, super/sub region, symbolic region) symbols
+/// for a given memory region.
+std::vector getTaintedSymbols(ProgramStateRef State,
+ const MemRegion *Reg,
+ TaintTagType Kind = TaintTagGeneric);
+
+std::vector getTaintedSymbolsImpl(ProgramStateRef State,
+ const Stmt *S,
+ const LocationContext *LCtx,
+ TaintTagType Kind,
+ bool returnFirstOnly);
+
+std::vector getTaintedSymbolsImpl(ProgramStateRef State, SVal V,
+ TaintTagType Kind,
+ bool returnFirstOnly);
+
+std::vector getTaintedSymbolsImpl(ProgramStateRef State,
+ SymbolRef Sym, TaintTagType Kind,
+ bool returnFirstOnly);
+
+std::vector getTaintedSymbolsImpl(ProgramStateRef State,
+ const MemRegion *Reg,
+ TaintTagType 

[PATCH] D148783: [clangd] Add support TextDocumentEdit.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D148783#4298398 , @DavidSpickett 
wrote:

> Hi, these tests are failing on our (Linaro's) Windows on Arm buildbot. First 
> appearance here https://lab.llvm.org/buildbot/#/builders/65/builds/9346
>
> Still failing as of a few hours ago 
> https://lab.llvm.org/buildbot/#/builders/65/builds/9360 (maybe you have fixed 
> this already, I am checking in infrequently over the next few days).
>
> If you need any help debugging it let me know. It appears there is a drive 
> letter on Windows:
>
>   # CHECK-NEXT:  "uri": "file:///clangd-test/foo.c",
>  ^
>   :224:30: note: scanning from here
>   "textDocument": {
>^
>   :225:15: note: possible intended match here
> "uri": "file:///C:/clangd-test/foo.c",
> ^

sorry for the breakage, looking now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148783

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

[clang-tools-extra] 4294619 - [clangd] Fix the window buildbot test failures

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2023-04-26T12:59:45+02:00
New Revision: 4294619b4cdfed49502dcebc7efd5f044e587267

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

LOG: [clangd] Fix the window buildbot test failures

Added: 


Modified: 
clang-tools-extra/clangd/test/fixits-codeaction-documentchanges.test
clang-tools-extra/clangd/test/fixits-command-documentchanges.test

Removed: 




diff  --git 
a/clang-tools-extra/clangd/test/fixits-codeaction-documentchanges.test 
b/clang-tools-extra/clangd/test/fixits-codeaction-documentchanges.test
index 0e15f34f321de..7a591319a7405 100644
--- a/clang-tools-extra/clangd/test/fixits-codeaction-documentchanges.test
+++ b/clang-tools-extra/clangd/test/fixits-codeaction-documentchanges.test
@@ -81,7 +81,7 @@
 # CHECK-NEXT:  }
 # CHECK-NEXT:],
 # CHECK-NEXT:"textDocument": {
-# CHECK-NEXT:  "uri": "file:///clangd-test/foo.c",
+# CHECK-NEXT:  "uri": "file://{{.*}}/foo.c",
 # CHECK-NEXT:  "version": 1
 # CHECK-NEXT:}
 # CHECK-NEXT:  }
@@ -128,7 +128,7 @@
 # CHECK-NEXT:  }
 # CHECK-NEXT:],
 # CHECK-NEXT:"textDocument": {
-# CHECK-NEXT:  "uri": "file:///clangd-test/foo.c",
+# CHECK-NEXT:  "uri": "file://{{.*}}/foo.c",
 # CHECK-NEXT:  "version": 1
 # CHECK-NEXT:}
 # CHECK-NEXT:  }

diff  --git a/clang-tools-extra/clangd/test/fixits-command-documentchanges.test 
b/clang-tools-extra/clangd/test/fixits-command-documentchanges.test
index 26b21c49ef9bd..cd636c4df387a 100644
--- a/clang-tools-extra/clangd/test/fixits-command-documentchanges.test
+++ b/clang-tools-extra/clangd/test/fixits-command-documentchanges.test
@@ -64,7 +64,7 @@
 # CHECK-NEXT:}
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  "textDocument": {
-# CHECK-NEXT:"uri": "file:///clangd-test/foo.c",
+# CHECK-NEXT:"uri": "file://{{.*}}/foo.c",
 # CHECK-NEXT:"version": 0
 # CHECK-NEXT:  }
 # CHECK-NEXT:}
@@ -95,7 +95,7 @@
 # CHECK-NEXT:}
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  "textDocument": {
-# CHECK-NEXT:"uri": "file:///clangd-test/foo.c",
+# CHECK-NEXT:"uri": "file://{{.*}}/foo.c",
 # CHECK-NEXT:"version": 0
 # CHECK-NEXT:  }
 # CHECK-NEXT:}
@@ -146,7 +146,7 @@
 # CHECK-NEXT:}
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  "textDocument": {
-# CHECK-NEXT:"uri": "file:///clangd-test/foo.c",
+# CHECK-NEXT:"uri": "file://{{.*}}/foo.c",
 # CHECK-NEXT:"version": 0
 # CHECK-NEXT:  }
 # CHECK-NEXT:}
@@ -177,7 +177,7 @@
 # CHECK-NEXT:}
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  "textDocument": {
-# CHECK-NEXT:"uri": "file:///clangd-test/foo.c",
+# CHECK-NEXT:"uri": "file://{{.*}}/foo.c",
 # CHECK-NEXT:"version": 0
 # CHECK-NEXT:  }
 # CHECK-NEXT:}



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

[PATCH] D148783: [clangd] Add support TextDocumentEdit.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

should be fixed in 
https://github.com/llvm/llvm-project/commit/4294619b4cdfed49502dcebc7efd5f044e587267.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148783

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

[PATCH] D148663: [RFC][clangd] Use interpolation for CDB pushed via LSP protocol

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I wanted to chime in and provide a bit of context.
This was a long time ago, so I might misremember, so take this with a grain of 
salt.

Idea behind pushing the CDB over LSP was to allow the capable client to 
**fully** control the commands produced for the files.
Decisions like interpolation were pushed towards the clients intentionally, not 
accidentally.
IIRC, the motivation back in the day was either sourcekit-lsp or Theia.

I will let other do the actual review, just wanted to bring up the history for 
a complete picutre.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148663

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

[PATCH] D147684: [clangd] Add batch fixes for include-cleaner diagnostics

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:443
+  RemoveAll.Annotations.push_back({RemoveAllUnusedID,
+   {/*label=*/"", /*needsConfirmation=*/true,
+/*description=*/std::nullopt}});

hokein wrote:
> kadircet wrote:
> > rather than an empty label, let's duplicate `RemoveAll.Message` here. As 
> > that context will probably be lost after executing the code action.
> I don't see much value on setting the label the the RemoveAll message (the 
> preview window shows the content diff, it is clear enough for users to 
> understand the purpose by viewing the diff).
> 
> And since we use one annotation per edit, the `RemoveAll` message doesn't 
> seem to suit anymore.
> I don't see much value on setting the label the the RemoveAll message (the 
> preview window shows the content diff, it is clear enough for users to 
> understand the purpose by viewing the diff).

i think you're focusing too much on the way VSCode chooses to implement this 
interaction.

Showing the diff in the edit panel is a strategy chosen by VSCode, not per LSP. 
Spec indicates `label` as `A human-readable string describing the actual 
change. The string is rendered prominent in the user interface.`. So there's a 
good chance an editor might chose to just display the `label`. This makes even 
more sense especially when changes are complicated and don't fit a single line.

> And since we use one annotation per edit, the RemoveAll message doesn't seem 
> to suit anymore.

Right, but we actually now have the opportunity to describe each fix properly. 
We can change the fix message we generate in `generateUnusedIncludeDiagnostics` 
to contain proper include and use them as annotation labels instead. We already 
have the logic inside Diagnostics.cpp to synthesize messages for fixes (this 
would also unify the behaviour), i think we should re-use it here by exposing 
it in diagnostics.h. WDYT?



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:463
+  // FIXME(hokein): emit used symbol reference in the annotation.
+  ChangeAnnotation Annotation = {/*label=*/"",
+ /*needsConfirmation=*/true,

similar to above, let's at least mention the fix properly in label.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:471
+ChangeAnnotationIdentifier ID = AddAllMissingID + std::to_string(I++);
+AddAllMissing.Edits.push_back(It.getValue());
+AddAllMissing.Edits.back().annotationId.emplace(ID);

nit: you can `std::move(It.getValue())` if you drop the `const` from `It`.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:481
+  FixAll.Message = "fix all includes";
+  ChangeAnnotation Annotation = {/*label=*/"",
+ /*needsConfirmation=*/true,

why are we creating new annotations here? rather than just merging everything 
from `RemoveAllUnused` and `AddAllMissing` ?



Comment at: clang-tools-extra/clangd/Protocol.h:253
+  /// The actual annotation identifier.
+  std::optional annotationId = std::nullopt;
 };

despite keeping this one as-is, we still don't need to optional, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147684

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

[PATCH] D148730: [C11] Allow initialization of an atomic-qualified pointer from a null pointer constant

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D148730#4298401 , @bgraur wrote:

> @aaron.ballman your patch (1395cde24b3641e284bb1daae7d56c189a2635e3 
> ) fixed 
> all issues we encountered. Thanks!!

Excellent, thank you for the confirmation!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148730

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

[PATCH] D147844: [clang][Sema]Print diagnostic warning about precedence when integer expression is used without parentheses in an conditional operator expression

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D147844#4293743 , @cjdb wrote:

> In D147844#4293693 , @dblaikie 
> wrote:
>
>>> I think some of the cases are ambiguous while others are not.
>>
>> Data would be good to have - if this assessment is true, we'd expect this to 
>> bear out in terms of bug finding, yeah? (that the cases you find to be 
>> ambiguous turn up as real bugs with some significant frequency in a 
>> codebase?)
>
> I disagree that there's a need for bugs to add this warning. Reading 
> ambiguous-but-correct code isn't going to be buggy, but it is going to cause 
> readability issues for any reviewers or maintainers.

The number of changes to test cases convinces me that we definitely need data 
on whether it's worth it or not. This warning now looks chatty enough that it 
needs to be disabled by default, and that doesn't meet our usual bar.

>> Readability improvements that don't cross the threshold to be the cause of a 
>> significant number of bugs are moreso the domain of clang-tidy, not clang 
>> warnings.

Strong +1, this is starting to feel more like a style warning due to the 
chattiness.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

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

[PATCH] D148987: [clang][Interp] Check Neg ops for errors

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM with a request to add a message to the assertion.




Comment at: clang/lib/AST/Interp/Interp.h:436
+
+  assert(isIntegralType(Name));
   S.Stk.push(Result);




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

https://reviews.llvm.org/D148987

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

[PATCH] D149172: [clang][Interp] Emit proper diagnostic when comparing unrelated pointers

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149172

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

[PATCH] D149154: [clang][Interp] Emit diagnostic when comparing function pointers

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/Interp.h:652-653
+  const SourceInfo &Loc = S.Current->getSource(OpPC);
+  S.FFDiag(Loc, diag::note_constexpr_pointer_comparison_unspecified)
+  << LS << RS;
+  return false;

tbaeder wrote:
> aaron.ballman wrote:
> > tbaeder wrote:
> > > aaron.ballman wrote:
> > > > Can we pass in the result of `getType()` instead of doing this string 
> > > > conversion dance?
> > > Well the diagnostic doesn't print the result of the LHS/RHS:
> > > ```
> > > ./array.cpp:202:18: error: constexpr variable 'u13' must be initialized 
> > > by a constant expression
> > >   202 |   constexpr bool u13 = pf < pg; // ref-warning {{ordered 
> > > comparison of function pointers}}
> > >   |  ^ ~~~
> > > ./array.cpp:202:27: note: comparison between '&f' and '&g' has 
> > > unspecified value
> > >   202 |   constexpr bool u13 = pf < pg; // ref-warning {{ordered 
> > > comparison of function pointers}}
> > >   |   ^
> > > ```
> > > 
> > > I'm not exactly a fan of how the code looks though. I might add a helper 
> > > function for this later.
> > Ah of course, good point. And yeah, a helper function for this would 
> > probably not be a bad idea.
> Do you like the `toDiagnosticString()` from https://reviews.llvm.org/D149172 
> better?
I do, that's a nice helper. Feel free to either land those changes and use the 
function here, or land these changes and do a later NFC commit to the helper 
method.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149154

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

[PATCH] D144269: [Analyzer] Show "taint originated here" note of alpha.security.taint.TaintPropagation checker at the correct place

[Daniel Krupp via Phabricator via cfe-commits]

dkrupp closed this revision.
dkrupp added a comment.

Committed in 343bdb10940cb2387c0b9bd3caccee7bb56c937b 
.


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

https://reviews.llvm.org/D144269

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

[PATCH] D148439: [clang-rename] Exit gracefully when no input provided

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM, but please add a release note when landing the changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148439

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

[PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

[Pavel Kosov via Phabricator via cfe-commits]

kpdev42 added a comment.

So what are next steps? Are we going for implementation of 
`DW_AT_no_unique_address` (which is going to be a non-standard extension) ? 
@dblaikie @aprantl @Michael137


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

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

[clang] 8b39527 - [NFC] Wrap entire debug logging loop in LLVM_DEBUG

[Jordan Rupprecht via cfe-commits]

Author: Jordan Rupprecht
Date: 2023-04-26T05:28:15-07:00
New Revision: 8b39527535ff50837da08509392e3268fb6129f0

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

LOG: [NFC] Wrap entire debug logging loop in LLVM_DEBUG

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
index 8340fe4c8270b..4bf8ce13e0572 100644
--- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -173,10 +173,10 @@ const NoteTag *taintOriginTrackerTag(CheckerContext &C,
 for (auto Sym : TaintedSymbols) {
   BR.markInteresting(Sym);
 }
-for (auto Arg : TaintedArgs) {
-  LLVM_DEBUG(llvm::dbgs()
- << "Taint Propagated from argument " << Arg + 1 << "\n");
-}
+LLVM_DEBUG(for (auto Arg
+: TaintedArgs) {
+  llvm::dbgs() << "Taint Propagated from argument " << Arg + 1 << "\n";
+});
 return "";
   });
 }



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

[PATCH] D149158: [clang][analyzer] Cleanup tests of StdCLibraryFunctionsChecker (NFC)

[Balázs Benics via Phabricator via cfe-commits]

steakhal added inline comments.



Comment at: clang/test/Analysis/Inputs/std-c-library-functions.h:1-2
+typedef typeof(sizeof(int)) size_t;
+typedef signed long ssize_t;
+typedef struct {

`ssize_t`'s size should match the size of `size_t`. In this implementation, it 
would be true only if `size_t` is `long`.




Comment at: clang/test/Analysis/Inputs/std-c-library-functions.h:31
+int getchar(void);
+size_t fread(void *restrict, size_t, size_t, FILE *restrict);
+size_t fwrite(const void *restrict, size_t, size_t, FILE *restrict);

`restrict` will only work if this header is included from `C` files. In `C++` 
files we will have a surprising behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149158

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

[PATCH] D149160: [clang][analyzer] Handle special value AT_FDCWD in affected standard functions

[Balázs Benics via Phabricator via cfe-commits]

steakhal added inline comments.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1530-1541
+  // Get platform dependent values of some macros.
+  // Try our best to parse this from the Preprocessor, otherwise fallback to a
+  // default value (what is found in a library header).
+  auto GetMacroValue = [&C](const char *Name, int Default) -> RangeInt {
 if (const std::optional OptInt =
-tryExpandAsInteger("EOF", C.getPreprocessor()))
+tryExpandAsInteger(Name, C.getPreprocessor()))
   return *OptInt;





Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:2137-2138
 .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
+.ArgConstraint(ArgumentCondition(
+0, WithinRange, Range({AT_FDCWDv, AT_FDCWDv}, {0, IntMax})))
 .ArgConstraint(NotNull(ArgNo(1;

I think I would recommend hoisting this common subexpression. This would save 
repetition on subsequent occurrences.

```lang=C++
const auto ValidDescriptorOrAtCWDRange = Range({AT_FDCWDv, AT_FDCWDv}, {0, 
IntMax});
```

You can explore if hoisting the whole `ArgumentCondition` would make it even 
more readable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149160

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

[PATCH] D149000: Update with warning message for comparison to NULL pointer

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/Sema/conditional-expr.c:89
   char x;
-  return &x) != ((void *) 0)) ? (*(&x) = ((char) 1)) : (void) ((void *) 
0)), (unsigned long) ((void *) 0)); // expected-warning {{C99 forbids 
conditional expressions with only one void side}}
 }

The C99 warning should not be dropped -- that was a valid pedantic diagnostic: 
https://godbolt.org/z/Tz3zGY8d4



Comment at: clang/test/SemaCXX/constant-expression-cxx2a.cpp:252
   // ... but in the complete object, the same is not true, so the runtime 
fails.
-  static_assert(dynamic_cast(static_cast(&g)) == nullptr);
+  static_assert(dynamic_cast(static_cast(&g)) == 
nullptr); // expected-warning {{comparison of address of 'g' equal to a null 
pointer is always false}}
 

shafik wrote:
> I don't believe we should be emitting a diagnostic for this case. The 
> `static_assert` passes. CC @aaron.ballman 
Agreed, this new diagnostic should not be triggered


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149000

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

[PATCH] D149210: [IR] Change shufflevector undef mask to poison

[Manuel Brito via Phabricator via cfe-commits]

ManuelJBrito added a comment.

In D149210#4298124 , @nikic wrote:

> Could you please split the change to printing + the test updates from all the 
> other changes? The code changes get lost in the large diff.

Yes, that makes sense. The internal representation can be addressed separately 
from the printing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149210

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

[clang] de25473 - [analyzer] Fix comparison logic in ArrayBoundCheckerV2

[Donát Nagy via cfe-commits]

Author: Donát Nagy
Date: 2023-04-26T15:02:23+02:00
New Revision: de2547329b41ad6ea4ea876d12731bde5a6b64c5

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

LOG: [analyzer] Fix comparison logic in ArrayBoundCheckerV2

The prototype checker alpha.security.ArrayBoundV2 performs two
comparisons to check that in an expression like Array[Index]
0 <= Index < length(Array)
holds. These comparisons are handled by almost identical logic: the
inequality is first rearranged by getSimplifiedOffsets(), then evaluated
with evalBinOpNN().

However the simplification used "naive" elementary mathematical
schematics, but evalBinOpNN() performed the signed -> unsigned
conversions described in the C/C++ standards, and this confusion led to
wildly inaccurate results: false positives from the lower bound check
and false negatives from the upper bound check.

This commit eliminates the code duplication by moving the comparison
logic into a separate function, then adds an explicit check to this
unified code path, which handles the problematic case separately.

In addition to this, the commit also cleans up a testcase that was
demonstrating the presence of this problem. Note that while that
testcase was failing with an overflow error, its actual problem was in
the underflow handler logic:
(0) The testcase introduces a five-element array "char a[5]" and an
unknown argument "size_t len"; then evaluates "a[len+1]".
(1) The underflow check tries to determine whether "len+1 < 0" holds.
(2) This inequality is rearranged to "len < -1".
(3) evalBinOpNN() evaluates this with the schematics of C/C++ and
converts -1 to the size_t value SIZE_MAX.
(4) The engine concludes that len == SIZE_MAX, because otherwise we'd
have an underflow here.
(5) The overflow check tries to determine whether "len+1 >= 5".
(6) This inequality is rearranged to "len >= 4".
(7) The engine substitutes len == SIZE_MAX and reports that we have
an overflow.

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

Added: 
clang/test/Analysis/array-bound-v2-constraint-check.c

Modified: 
clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp

Removed: 
clang/test/Analysis/out-of-bounds-false-positive.c



diff  --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
index 142577174ead8..a476613b6dcc7 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -57,8 +57,8 @@ class RegionRawOffsetV2 {
 : baseRegion(nullptr), byteOffset(UnknownVal()) {}
 
 public:
-  RegionRawOffsetV2(const SubRegion* base, SVal offset)
-: baseRegion(base), byteOffset(offset) {}
+  RegionRawOffsetV2(const SubRegion *base, NonLoc offset)
+  : baseRegion(base), byteOffset(offset) { assert(base); }
 
   NonLoc getByteOffset() const { return byteOffset.castAs(); }
   const SubRegion *getRegion() const { return baseRegion; }
@@ -72,19 +72,12 @@ class RegionRawOffsetV2 {
 };
 }
 
-static SVal computeExtentBegin(SValBuilder &svalBuilder,
-   const MemRegion *region) {
-  const MemSpaceRegion *SR = region->getMemorySpace();
-  if (SR->getKind() == MemRegion::UnknownSpaceRegionKind)
-return UnknownVal();
-  else
-return svalBuilder.makeZeroArrayIndex();
-}
-
 // TODO: once the constraint manager is smart enough to handle non simplified
 // symbolic expressions remove this function. Note that this can not be used in
 // the constraint manager as is, since this does not handle overflows. It is
 // safe to assume, however, that memory offsets will not overflow.
+// NOTE: callers of this function need to be aware of the effects of overflows
+// and signed<->unsigned conversions!
 static std::pair
 getSimplifiedOffsets(NonLoc offset, nonloc::ConcreteInt extent,
  SValBuilder &svalBuilder) {
@@ -117,6 +110,38 @@ getSimplifiedOffsets(NonLoc offset, nonloc::ConcreteInt 
extent,
   return std::pair(offset, extent);
 }
 
+// Evaluate the comparison Value < Threshold with the help of the custom
+// simplification algorithm defined for this checker. Return a pair of states,
+// where the first one corresponds to "value below threshold" and the second
+// corresponds to "value at or above threshold". Returns {nullptr, nullptr} in
+// the case when the evaluation fails.
+static std::pair
+compareValueToThreshold(ProgramStateRef State, NonLoc Value, NonLoc Threshold,
+SValBuilder &SVB) {
+  if (auto ConcreteThreshold = Threshold.getAs()) {
+std::tie(Value, Threshold) = getSimplifiedOffsets(Value, 
*ConcreteThreshold, SVB);
+  }
+  if (auto ConcreteThreshold = Threshold.getAs()) {
+QualType T = Value.getType(SVB.getContex

[PATCH] D148355: [analyzer] Fix comparison logic in ArrayBoundCheckerV2

[Donát Nagy via Phabricator via cfe-commits]

donat.nagy closed this revision.
donat.nagy added a comment.

Committed in rGde2547329b41ad6ea4ea876d12731bde5a6b64c5 
 (which 
accidentally refers to a wrong Phabricator review).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148355

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

[PATCH] D135375: [analyzer] Initialize regions returned by CXXNew to undefined

[Donát Nagy via Phabricator via cfe-commits]

donat.nagy added a comment.

Commit rGde2547329b41ad6ea4ea876d12731bde5a6b64c5 
 
accidentally refers to this review, but in fact it belongs to D148355 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135375

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

[PATCH] D149187: [clang] Canonicalize system headers in dependency file when -canonical-prefixes

[Hans Wennborg via Phabricator via cfe-commits]

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

Thanks! I like it.




Comment at: clang/include/clang/Frontend/Utils.h:44
 class Preprocessor;
+class FileManager;
 class PreprocessorOptions;

nit: sort?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149187

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

[PATCH] D148216: Add support for annotations in UpdateTestChecks (NFC)

[Henrik G Olsson via Phabricator via cfe-commits]

hnrklssn added inline comments.



Comment at: 
clang/test/utils/update_cc_test_checks/Inputs/annotations.c.expected:12
+// CHECK-NEXT:[[TMP1:%.*]] = load i32, ptr [[X]], align 4
+// CHECK-NEXT:ret i32 [[TMP1]]
+//

delcypher wrote:
> @hnrklssn I just noticed we don't have a `CHECK` for what `META2` actually 
> refers to. Should we?
> 
> Not something that has to be fixed in this patch, more just an observation.
Indeed this is true for metadata in general, presumably because the RHS often 
refer to things like other metadata identifiers. In the case of annotations 
they seem to always refer to simple strings however, so it would be feasible to 
do a straight match without having to do recursive matching or complex regexes 
to determine which part of the metadata to match against.

In many cases with metadata attached to IR nodes, multiple nodes refer to the 
same metadata node, so at least you verify that they still are consistent. But 
I agree that verifying the content would be a great future addition.



Comment at: llvm/utils/UpdateTestChecks/common.py:703
   def get_ir_prefix_from_ir_value_match(self, match):
-return self.ir_prefix, self.check_prefix
+return re.search(self.ir_prefix, match[0])[0], self.check_prefix
 

delcypher wrote:
> Is this call to `re.search(...)` guaranteed to always succeed?
Yes. `match` is a match object from a combined regex concatenated from both the 
IR prefix (`![a-z.]+ `) and the IR regexp (`![0-9]+`), so it will always 
contain a substring matching the IR prefix.



Comment at: llvm/utils/UpdateTestChecks/common.py:779
 NamelessValue(r'ACC_GRP', '!' , r'!llvm.access.group ' , r'![0-9]+'
 , None ) ,
+NamelessValue(r'META'   , '!' , r'![a-z.]+ '   , r'![0-9]+'
 , None ) ,
 ]

delcypher wrote:
> There may be some value in having `!annotations` matched explicitly as well 
> as there being a fallback. In the current patch it looks like:
> 
> * `metadata` is assigned variables with the `META` prefix on `CHECK` lines
> * `!annotation` is assigned variables with the `META` prefix on `CHECK` lines
> * Anything else that matches `r'![a-z.]+ ' `  is assigned variables with the 
> `META` prefix on `CHECK` lines
> 
> When looking a large test having everything lumped into `META` variables 
> could make the generated tests a little harder to read.
In a way, yes. At the same time, since we're not matching against the 
definition of the metadata node, all uses (while named META#) will still be 
prefixed by !annotation. On the other hand again, we have explicitly enumerated 
so many other types of metadata, so it is a bit inconsistent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148216

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

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a subscriber: cjdb.
aaron.ballman added inline comments.



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:863
+  InliningDecisions.push_back(D.getCaller().str());
+  for (size_t i = 0, e = InliningDecisions.size(); i != e; ++i) {
+std::string S = llvm::demangle(InliningDecisions[i]);

Alternatively:
```
for (auto Dec : llvm::enumerate(InliningDecisions)) {
  std::string S = llvm::demangle(Dec.value());
  if (Dec.index() == 0)
Diags.Report(...);
  ...
}
```



Comment at: clang/test/Frontend/backend-attribute-error-warning-optimize.c:12
   foo(); // expected-error {{call to 'foo' declared with 'error' attribute: oh 
no foo}}
+ // expected-note@* {{In function 'baz'}}
   if (x())

nickdesaulniers wrote:
> nickdesaulniers wrote:
> > aaron.ballman wrote:
> > > Instead of allowing this note to appear anywhere in the file, I think 
> > > it's better to use "bookmark" comments. e.g.,
> > > ```
> > > void baz() { // #baz_defn
> > > }
> > > 
> > > void foo() {
> > >   baz(); // expected-note@#baz_defn {{whatever note text}}
> > > }
> > > ```
> > > so that we're testing where the diagnostic is emitted. (This is 
> > > especially important given that the changes are about location tracking.)
> > oh, that's new (to me)! will do
> It looks like the "bookmarks" don't work because the notes do not line+col 
> info. They follow the warning/error diagnostic which does, on the bottom most 
> call site.
> 
> The warning is supposed to be emitted on the callsite, not the definition.
I still don't think this is reasonable for test coverage because this means 
we'll accept the note *anywhere* in the file. This makes it much too easy to 
regress the behavior accidentally (e.g., accidentally emit all the notes on 
line 1 of the file). I think we need *some* way to nail down where these notes 
are emitted in the test. However, I might be asking you to solve "please track 
location information through the backend" for that, so perhaps this isn't a 
reasonable request?

Also, CC @cjdb for awareness of how this kind of diagnostic is produced (Chris 
is working on an idea for how we emit diagnostics so we get better structured 
information from them, so knowing about this novel approach might impact that 
idea).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141451

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

[PATCH] D148216: Add support for annotations in UpdateTestChecks (NFC)

[Nikita Popov via Phabricator via cfe-commits]

nikic added inline comments.



Comment at: 
clang/test/utils/update_cc_test_checks/Inputs/annotations.c.expected:12
+// CHECK-NEXT:[[TMP1:%.*]] = load i32, ptr [[X]], align 4
+// CHECK-NEXT:ret i32 [[TMP1]]
+//

hnrklssn wrote:
> delcypher wrote:
> > @hnrklssn I just noticed we don't have a `CHECK` for what `META2` actually 
> > refers to. Should we?
> > 
> > Not something that has to be fixed in this patch, more just an observation.
> Indeed this is true for metadata in general, presumably because the RHS often 
> refer to things like other metadata identifiers. In the case of annotations 
> they seem to always refer to simple strings however, so it would be feasible 
> to do a straight match without having to do recursive matching or complex 
> regexes to determine which part of the metadata to match against.
> 
> In many cases with metadata attached to IR nodes, multiple nodes refer to the 
> same metadata node, so at least you verify that they still are consistent. 
> But I agree that verifying the content would be a great future addition.
You need to pass `--check-globals` to check the actual metadata.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148216

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

[PATCH] D149206: [clang][driver] Enable MisExpect diagnostics flag outside of CC1

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: clang/test/Profile/misexpect-branch.c:10
 // RUN: %clang_cc1 %s -O2 -o - -emit-llvm 
-fprofile-instrument-use-path=%t.profdata -verify=foo 
-fdiagnostics-misexpect-tolerance=10 -Wmisexpect 
-debug-info-kind=line-tables-only
+// RUN: %clang -c -S %s -O2 -o - -emit-llvm -fprofile-instr-use=%t.profdata 
-Xclang -verify=foo -fdiagnostics-misexpect-tolerance=10 -Wmisexpect 
-gline-tables-only
 // RUN: %clang_cc1 %s -O2 -o - -disable-llvm-passes -emit-llvm 
-fprofile-instrument-use-path=%t.profdata -verify=foo

The more common way to test this would be a test under clang/test/Driver/ that 
just checks that forwarding the flag works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149206

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

[PATCH] D149210: [IR] Change shufflevector undef mask to poison

[Manuel Brito via Phabricator via cfe-commits]

ManuelJBrito updated this revision to Diff 517150.
ManuelJBrito added a comment.

Split patch into print update and internal representation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149210

Files:
  clang/test/CodeGen/PowerPC/builtins-ppc-p10vector.c
  clang/test/CodeGen/PowerPC/builtins-ppc-p9vector.c
  clang/test/CodeGen/builtins-nondeterministic-value.c
  clang/test/CodeGen/builtinshufflevector2.c
  clang/test/CodeGen/ext-vector.c
  clang/test/CodeGenOpenCL/as_type.cl
  clang/test/CodeGenOpenCL/partial_initializer.cl
  clang/test/CodeGenOpenCL/preserve_vec3.cl
  clang/test/CodeGenOpenCL/vector_literals.cl
  clang/test/CodeGenOpenCL/vector_shufflevector.cl
  llvm/lib/IR/AsmWriter.cpp
  llvm/test/Analysis/CostModel/RISCV/shuffle-extract_subvector.ll
  llvm/test/Analysis/CostModel/RISCV/shuffle-insert_subvector.ll
  llvm/test/Analysis/CostModel/X86/reduction.ll
  llvm/test/Analysis/CostModel/X86/shuffle-extract_subvector-codesize.ll
  llvm/test/Analysis/CostModel/X86/shuffle-extract_subvector-latency.ll
  llvm/test/Analysis/CostModel/X86/shuffle-extract_subvector-sizelatency.ll
  llvm/test/Analysis/CostModel/X86/shuffle-extract_subvector.ll
  llvm/test/Analysis/CostModel/X86/shuffle-insert_subvector-codesize.ll
  llvm/test/Analysis/CostModel/X86/shuffle-insert_subvector-latency.ll
  llvm/test/Analysis/CostModel/X86/shuffle-insert_subvector-sizelatency.ll
  llvm/test/Analysis/CostModel/X86/shuffle-insert_subvector.ll
  llvm/test/Analysis/CostModel/X86/shuffle-single-src-codesize.ll
  llvm/test/Analysis/CostModel/X86/shuffle-single-src-latency.ll
  llvm/test/Analysis/CostModel/X86/shuffle-single-src-sizelatency.ll
  llvm/test/Analysis/CostModel/X86/shuffle-single-src.ll
  llvm/test/CodeGen/AMDGPU/amdgpu-codegenprepare-break-large-phis-heuristics.ll
  llvm/test/CodeGen/AMDGPU/rewrite-out-arguments.ll
  llvm/test/CodeGen/Generic/expand-experimental-reductions.ll
  llvm/test/CodeGen/PowerPC/arg_promotion.ll
  llvm/test/Instrumentation/MemorySanitizer/avx-intrinsics-x86.ll
  llvm/test/Transforms/Attributor/nofpclass.ll
  llvm/test/Transforms/CodeGenPrepare/X86/x86-shuffle-sink-inseltpoison.ll
  llvm/test/Transforms/CodeGenPrepare/X86/x86-shuffle-sink.ll
  llvm/test/Transforms/DeadStoreElimination/masked-dead-store-inseltpoison.ll
  llvm/test/Transforms/DeadStoreElimination/masked-dead-store.ll
  llvm/test/Transforms/InstCombine/AArch64/demandelts.ll
  
llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-demanded-vector-elts-inseltpoison.ll
  llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-demanded-vector-elts.ll
  llvm/test/Transforms/InstCombine/X86/x86-addsub-inseltpoison.ll
  llvm/test/Transforms/InstCombine/X86/x86-addsub.ll
  llvm/test/Transforms/InstCombine/X86/x86-avx2-inseltpoison.ll
  llvm/test/Transforms/InstCombine/X86/x86-avx2.ll
  llvm/test/Transforms/InstCombine/X86/x86-avx512-inseltpoison.ll
  llvm/test/Transforms/InstCombine/X86/x86-avx512.ll
  llvm/test/Transforms/InstCombine/X86/x86-muldq-inseltpoison.ll
  llvm/test/Transforms/InstCombine/X86/x86-muldq.ll
  llvm/test/Transforms/InstCombine/X86/x86-pack-inseltpoison.ll
  llvm/test/Transforms/InstCombine/X86/x86-pack.ll
  llvm/test/Transforms/InstCombine/X86/x86-pshufb-inseltpoison.ll
  llvm/test/Transforms/InstCombine/X86/x86-pshufb.ll
  llvm/test/Transforms/InstCombine/X86/x86-sse4a-inseltpoison.ll
  llvm/test/Transforms/InstCombine/X86/x86-sse4a.ll
  llvm/test/Transforms/InstCombine/X86/x86-vpermil-inseltpoison.ll
  llvm/test/Transforms/InstCombine/X86/x86-vpermil.ll
  llvm/test/Transforms/InstCombine/bitreverse.ll
  llvm/test/Transforms/InstCombine/broadcast-inseltpoison.ll
  llvm/test/Transforms/InstCombine/broadcast.ll
  llvm/test/Transforms/InstCombine/bswap-inseltpoison.ll
  llvm/test/Transforms/InstCombine/bswap.ll
  llvm/test/Transforms/InstCombine/canonicalize-vector-insert.ll
  llvm/test/Transforms/InstCombine/extractelement-inseltpoison.ll
  llvm/test/Transforms/InstCombine/extractelement.ll
  llvm/test/Transforms/InstCombine/insert-const-shuf-inseltpoison.ll
  llvm/test/Transforms/InstCombine/insert-const-shuf.ll
  llvm/test/Transforms/InstCombine/insert-extract-shuffle-inseltpoison.ll
  llvm/test/Transforms/InstCombine/insert-extract-shuffle.ll
  llvm/test/Transforms/InstCombine/masked_intrinsics-inseltpoison.ll
  llvm/test/Transforms/InstCombine/masked_intrinsics.ll
  llvm/test/Transforms/InstCombine/minmax-intrinsics.ll
  llvm/test/Transforms/InstCombine/nsw-inseltpoison.ll
  llvm/test/Transforms/InstCombine/nsw.ll
  llvm/test/Transforms/InstCombine/reduction-shufflevector.ll
  llvm/test/Transforms/InstCombine/select-extractelement-inseltpoison.ll
  llvm/test/Transforms/InstCombine/select-extractelement.ll
  llvm/test/Transforms/InstCombine/select-select.ll
  llvm/test/Transforms/InstCombine/shuffle-binop.ll
  llvm/test/Transforms/InstCombine/shuffle-select-narrow-inseltpoison.ll
  llvm/test/Transforms/InstCombine/shuffle-select-narrow.ll
  llv

[PATCH] D149000: Update with warning message for comparison to NULL pointer

[Krishna Narayanan via Phabricator via cfe-commits]

Krishna-13-cyber updated this revision to Diff 517151.
Krishna-13-cyber added a comment.

- Updated with re-adding the required diagnostic message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149000

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/conditional-expr.c
  clang/test/Sema/warn-tautological-compare.c


Index: clang/test/Sema/warn-tautological-compare.c
===
--- clang/test/Sema/warn-tautological-compare.c
+++ clang/test/Sema/warn-tautological-compare.c
@@ -93,3 +93,9 @@
   x = array ? 1 : 0; // expected-warning {{address of array}}
   x = &x ? 1 : 0;// expected-warning {{address of 'x'}}
 }
+
+void test4(void)
+{
+int *a = (void *) 0;
+int b = (&a) == ((void *) 0); // expected-warning {{comparison of address of 
'a' equal to a null pointer is always false}}
+}
Index: clang/test/Sema/conditional-expr.c
===
--- clang/test/Sema/conditional-expr.c
+++ clang/test/Sema/conditional-expr.c
@@ -86,7 +86,8 @@
 
 int Postgresql(void) {
   char x;
-  return &x) != ((void *) 0)) ? (*(&x) = ((char) 1)) : (void) ((void *) 
0)), (unsigned long) ((void *) 0)); // expected-warning {{C99 forbids 
conditional expressions with only one void side}}
+  return &x) != ((void *) 0)) ? (*(&x) = ((char) 1)) : (void) ((void *) 
0)), (unsigned long) ((void *) 0)); /* expected-warning {{C99 forbids 
conditional expressions with only one void side}}
+   
  expected-warning {{comparison of address of 
'x' not equal to a null pointer is always true}} */
 }
 
 #define nil ((void*) 0)
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -14723,7 +14723,7 @@
 
   bool IsAddressOf = false;
 
-  if (UnaryOperator *UO = dyn_cast(E)) {
+  if (UnaryOperator *UO = dyn_cast(E->IgnoreParenCasts())) {
 if (UO->getOpcode() != UO_AddrOf)
   return;
 IsAddressOf = true;


Index: clang/test/Sema/warn-tautological-compare.c
===
--- clang/test/Sema/warn-tautological-compare.c
+++ clang/test/Sema/warn-tautological-compare.c
@@ -93,3 +93,9 @@
   x = array ? 1 : 0; // expected-warning {{address of array}}
   x = &x ? 1 : 0;// expected-warning {{address of 'x'}}
 }
+
+void test4(void)
+{
+int *a = (void *) 0;
+int b = (&a) == ((void *) 0); // expected-warning {{comparison of address of 'a' equal to a null pointer is always false}}
+}
Index: clang/test/Sema/conditional-expr.c
===
--- clang/test/Sema/conditional-expr.c
+++ clang/test/Sema/conditional-expr.c
@@ -86,7 +86,8 @@
 
 int Postgresql(void) {
   char x;
-  return &x) != ((void *) 0)) ? (*(&x) = ((char) 1)) : (void) ((void *) 0)), (unsigned long) ((void *) 0)); // expected-warning {{C99 forbids conditional expressions with only one void side}}
+  return &x) != ((void *) 0)) ? (*(&x) = ((char) 1)) : (void) ((void *) 0)), (unsigned long) ((void *) 0)); /* expected-warning {{C99 forbids conditional expressions with only one void side}}
+ expected-warning {{comparison of address of 'x' not equal to a null pointer is always true}} */
 }
 
 #define nil ((void*) 0)
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -14723,7 +14723,7 @@
 
   bool IsAddressOf = false;
 
-  if (UnaryOperator *UO = dyn_cast(E)) {
+  if (UnaryOperator *UO = dyn_cast(E->IgnoreParenCasts())) {
 if (UO->getOpcode() != UO_AddrOf)
   return;
 IsAddressOf = true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147684: [clangd] Add batch fixes for include-cleaner diagnostics

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 517154.
hokein marked 2 inline comments as done.
hokein added a comment.

address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147684

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/Diagnostics.h
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/test/Inputs/include-cleaner/all1.h
  clang-tools-extra/clangd/test/Inputs/include-cleaner/all2.h
  clang-tools-extra/clangd/test/Inputs/include-cleaner/bar.h
  clang-tools-extra/clangd/test/Inputs/include-cleaner/foo.h
  clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -45,8 +45,8 @@
 using ::testing::Pointee;
 using ::testing::UnorderedElementsAre;
 
-Matcher withFix(::testing::Matcher FixMatcher) {
-  return Field(&Diag::Fixes, ElementsAre(FixMatcher));
+Matcher withFix(std::vector<::testing::Matcher> FixMatcheres) {
+  return Field(&Diag::Fixes, testing::UnorderedElementsAreArray(FixMatcheres));
 }
 
 MATCHER_P2(Diag, Range, Message,
@@ -60,6 +60,8 @@
   return arg.Message == Message && arg.Edits.size() == 1 &&
  arg.Edits[0].range == Range && arg.Edits[0].newText == Replacement;
 }
+MATCHER_P(FixMessage, Message, "") { return arg.Message == Message; }
+
 
 std::string guard(llvm::StringRef Code) {
   return "#pragma once\n" + Code.str();
@@ -255,42 +257,51 @@
   UnorderedElementsAre(
   AllOf(Diag(MainFile.range("b"),
  "No header providing \"b\" is directly included"),
-withFix(Fix(MainFile.range("insert_b"), "#include \"b.h\"\n",
-"#include \"b.h\""))),
+withFix({Fix(MainFile.range("insert_b"), "#include \"b.h\"\n",
+ "#include \"b.h\""),
+ FixMessage("add all missing includes")})),
   AllOf(Diag(MainFile.range("bar"),
  "No header providing \"ns::Bar\" is directly included"),
-withFix(Fix(MainFile.range("insert_d"),
-"#include \"dir/d.h\"\n", "#include \"dir/d.h\""))),
+withFix({Fix(MainFile.range("insert_d"),
+ "#include \"dir/d.h\"\n", "#include \"dir/d.h\""),
+ FixMessage("add all missing includes")})),
   AllOf(Diag(MainFile.range("f"),
  "No header providing \"f\" is directly included"),
-withFix(Fix(MainFile.range("insert_f"), "#include \n",
-"#include "))),
+withFix({Fix(MainFile.range("insert_f"), "#include \n",
+ "#include "),
+ FixMessage("add all missing includes")})),
   AllOf(
   Diag(MainFile.range("foobar"),
"No header providing \"foobar\" is directly included"),
-  withFix(Fix(MainFile.range("insert_foobar"),
-  "#include \"public.h\"\n", "#include \"public.h\""))),
+  withFix({Fix(MainFile.range("insert_foobar"),
+   "#include \"public.h\"\n", "#include \"public.h\""),
+   FixMessage("add all missing includes")})),
   AllOf(
   Diag(MainFile.range("vector"),
"No header providing \"std::vector\" is directly included"),
-  withFix(Fix(MainFile.range("insert_vector"),
-  "#include \n", "#include "))),
+  withFix({Fix(MainFile.range("insert_vector"),
+   "#include \n", "#include "),
+   FixMessage("add all missing includes"),})),
   AllOf(Diag(MainFile.range("FOO"),
  "No header providing \"FOO\" is directly included"),
-withFix(Fix(MainFile.range("insert_foo"),
-"#include \"foo.h\"\n", "#include \"foo.h\""))),
+withFix({Fix(MainFile.range("insert_foo"),
+ "#include \"foo.h\"\n", "#include \"foo.h\""),
+ FixMessage("add all missing includes")})),
   AllOf(Diag(MainFile.range("DEF"),
  "No header providing \"Foo\" is directly included"),
-withFix(Fix(MainFile.range("insert_foo"),
-"#include \"foo.h\"\n", "#include \"foo.h\""))),
+withFix({Fix(

[PATCH] D147684: [clangd] Add batch fixes for include-cleaner diagnostics

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 517157.
hokein added a comment.

fix the lit-test uri path on windows


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147684

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/Diagnostics.h
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/test/Inputs/include-cleaner/all1.h
  clang-tools-extra/clangd/test/Inputs/include-cleaner/all2.h
  clang-tools-extra/clangd/test/Inputs/include-cleaner/bar.h
  clang-tools-extra/clangd/test/Inputs/include-cleaner/foo.h
  clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -45,8 +45,8 @@
 using ::testing::Pointee;
 using ::testing::UnorderedElementsAre;
 
-Matcher withFix(::testing::Matcher FixMatcher) {
-  return Field(&Diag::Fixes, ElementsAre(FixMatcher));
+Matcher withFix(std::vector<::testing::Matcher> FixMatcheres) {
+  return Field(&Diag::Fixes, testing::UnorderedElementsAreArray(FixMatcheres));
 }
 
 MATCHER_P2(Diag, Range, Message,
@@ -60,6 +60,8 @@
   return arg.Message == Message && arg.Edits.size() == 1 &&
  arg.Edits[0].range == Range && arg.Edits[0].newText == Replacement;
 }
+MATCHER_P(FixMessage, Message, "") { return arg.Message == Message; }
+
 
 std::string guard(llvm::StringRef Code) {
   return "#pragma once\n" + Code.str();
@@ -255,42 +257,51 @@
   UnorderedElementsAre(
   AllOf(Diag(MainFile.range("b"),
  "No header providing \"b\" is directly included"),
-withFix(Fix(MainFile.range("insert_b"), "#include \"b.h\"\n",
-"#include \"b.h\""))),
+withFix({Fix(MainFile.range("insert_b"), "#include \"b.h\"\n",
+ "#include \"b.h\""),
+ FixMessage("add all missing includes")})),
   AllOf(Diag(MainFile.range("bar"),
  "No header providing \"ns::Bar\" is directly included"),
-withFix(Fix(MainFile.range("insert_d"),
-"#include \"dir/d.h\"\n", "#include \"dir/d.h\""))),
+withFix({Fix(MainFile.range("insert_d"),
+ "#include \"dir/d.h\"\n", "#include \"dir/d.h\""),
+ FixMessage("add all missing includes")})),
   AllOf(Diag(MainFile.range("f"),
  "No header providing \"f\" is directly included"),
-withFix(Fix(MainFile.range("insert_f"), "#include \n",
-"#include "))),
+withFix({Fix(MainFile.range("insert_f"), "#include \n",
+ "#include "),
+ FixMessage("add all missing includes")})),
   AllOf(
   Diag(MainFile.range("foobar"),
"No header providing \"foobar\" is directly included"),
-  withFix(Fix(MainFile.range("insert_foobar"),
-  "#include \"public.h\"\n", "#include \"public.h\""))),
+  withFix({Fix(MainFile.range("insert_foobar"),
+   "#include \"public.h\"\n", "#include \"public.h\""),
+   FixMessage("add all missing includes")})),
   AllOf(
   Diag(MainFile.range("vector"),
"No header providing \"std::vector\" is directly included"),
-  withFix(Fix(MainFile.range("insert_vector"),
-  "#include \n", "#include "))),
+  withFix({Fix(MainFile.range("insert_vector"),
+   "#include \n", "#include "),
+   FixMessage("add all missing includes"),})),
   AllOf(Diag(MainFile.range("FOO"),
  "No header providing \"FOO\" is directly included"),
-withFix(Fix(MainFile.range("insert_foo"),
-"#include \"foo.h\"\n", "#include \"foo.h\""))),
+withFix({Fix(MainFile.range("insert_foo"),
+ "#include \"foo.h\"\n", "#include \"foo.h\""),
+ FixMessage("add all missing includes")})),
   AllOf(Diag(MainFile.range("DEF"),
  "No header providing \"Foo\" is directly included"),
-withFix(Fix(MainFile.range("insert_foo"),
-"#include \"foo.h\"\n", "#include \"foo.h\""))),
+withFix({Fix(MainFile.range("insert

[PATCH] D147684: [clangd] Add batch fixes for include-cleaner diagnostics

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:443
+  RemoveAll.Annotations.push_back({RemoveAllUnusedID,
+   {/*label=*/"", /*needsConfirmation=*/true,
+/*description=*/std::nullopt}});

kadircet wrote:
> hokein wrote:
> > kadircet wrote:
> > > rather than an empty label, let's duplicate `RemoveAll.Message` here. As 
> > > that context will probably be lost after executing the code action.
> > I don't see much value on setting the label the the RemoveAll message (the 
> > preview window shows the content diff, it is clear enough for users to 
> > understand the purpose by viewing the diff).
> > 
> > And since we use one annotation per edit, the `RemoveAll` message doesn't 
> > seem to suit anymore.
> > I don't see much value on setting the label the the RemoveAll message (the 
> > preview window shows the content diff, it is clear enough for users to 
> > understand the purpose by viewing the diff).
> 
> i think you're focusing too much on the way VSCode chooses to implement this 
> interaction.
> 
> Showing the diff in the edit panel is a strategy chosen by VSCode, not per 
> LSP. Spec indicates `label` as `A human-readable string describing the actual 
> change. The string is rendered prominent in the user interface.`. So there's 
> a good chance an editor might chose to just display the `label`. This makes 
> even more sense especially when changes are complicated and don't fit a 
> single line.
> 
> > And since we use one annotation per edit, the RemoveAll message doesn't 
> > seem to suit anymore.
> 
> Right, but we actually now have the opportunity to describe each fix 
> properly. We can change the fix message we generate in 
> `generateUnusedIncludeDiagnostics` to contain proper include and use them as 
> annotation labels instead. We already have the logic inside Diagnostics.cpp 
> to synthesize messages for fixes (this would also unify the behaviour), i 
> think we should re-use it here by exposing it in diagnostics.h. WDYT?
Thanks, the plan sounds good to me. As discussed, I will address it in a 
followup patch (added a FIXME here).



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:481
+  FixAll.Message = "fix all includes";
+  ChangeAnnotation Annotation = {/*label=*/"",
+ /*needsConfirmation=*/true,

kadircet wrote:
> why are we creating new annotations here? rather than just merging everything 
> from `RemoveAllUnused` and `AddAllMissing` ?
right, we can do that, it is simpler.



Comment at: clang-tools-extra/clangd/Protocol.h:253
+  /// The actual annotation identifier.
+  std::optional annotationId = std::nullopt;
 };

kadircet wrote:
> despite keeping this one as-is, we still don't need to optional, right?
right, I missed this field when updating other fields.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147684

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

[PATCH] D147684: [clangd] Add batch fixes for include-cleaner diagnostics

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147684

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

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Ivan Murashko via Phabricator via cfe-commits]

ivanmurashko updated this revision to Diff 517159.
ivanmurashko added a comment.

Commandeer the diff from @andrewjcg and made some chnages at the code (get it 
compatible with latest clang source code) and at the tests (move modules 
artefacts to temp folder to make the test execution more stable).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Modules/Inputs/implicit-module-header-maps/a.h
  clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
  clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
  clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
  clang/test/Modules/implicit-module-header-maps.cpp

Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,32 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap -fmodules-cache-path=%t
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: sed -e "s|OUTPUTS_DIR|%t|g" %S/Inputs/implicit-module-header-maps/b.hmap.json > %t/hmap.json
+// RUN: %hmaptool write %t/hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap -fmodules-cache-path=%t
+
+#define FOO
+// This include will fail if:
+// 1) modules are't used, as the `FOO` define will propagate into the included
+//header and trip a `#error`, or
+// 2) header maps aren't usesd, as the header name doesn't exist and relies on
+//the header map to remap it to the real header.
+#include "Before/Mapping.h"
Index: clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
@@ -0,0 +1,6 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "OUTPUTS_DIR/After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
@@ -0,0 +1,3 @@
+module a {
+  header "After/Mapping.h"
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
@@ -0,0 +1,7 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "After/Mapping.h",
+ "After/Mapping.h" : "After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.h
@@ -0,0 +1,3 @@
+#ifdef FOO
+#error foo
+#endif
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -491,7 +491,8 @@
 
   IsInHeaderMap = true;
 
-  auto FixupSearchPath = [&]() {
+  auto FixupSearchPathAndFindUsableModule =
+  [&](auto File) -> OptionalFileEntryRef {
 if (SearchPath) {
   StringRef SearchPathRef(getName());
   SearchPath->clear();
@@ -501,6 +502,12 @@
   RelativePath->clear();
   RelativePath->append(Filename.begin(), Filename.end());
 }
+if (!HS.findUsableModuleForHeader(
+&File.getFileEntry(), File.getFileEntry().getDir(),
+RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+  return std::nullopt;
+}
+return File;
   };
 
   // Check if the headermap maps the filename to a framework include
@@ -513,8 +520,7 @@
   }
 
   if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest, OpenFile)) {
-FixupSearchPath();
-return *Res;
+return FixupSearchPathAndFindUsableModule(*Res);
   }
 
   // Header maps need to be marked as used whenever the filename matches.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe

[PATCH] D149162: [Clang][OpenMP][IRBuilder] Move registerTargetGlobalVariable & getAddrOfDeclareTargetVar into the OMPIRBuilder

[Jan Sjödin via Phabricator via cfe-commits]

jsjodin requested changes to this revision.
jsjodin added inline comments.
This revision now requires changes to proceed.



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:817
+  /// LLVMModule.
+  llvm::Constant *getAddrOfDeclareTargetVar(
+  llvm::OffloadEntriesInfoManager::OMPTargetGlobalVarEntryKind

No llvm:: prefix needed.



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:876
+  uint64_t Line, llvm::StringRef MangledName, llvm::Module *LlvmModule,
+  std::vector &GeneratedRefs, bool OpenMPSIMD,
+  bool OpenMPIsDevice, std::vector TargetTriple,

Instead of passing in the various components I think it is better to pass in 
TargetRegionEntryInfo in case the inputs to create one changes. That way we 
avoid changing the parameters to this and the other functions.



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:877
+  std::vector &GeneratedRefs, bool OpenMPSIMD,
+  bool OpenMPIsDevice, std::vector TargetTriple,
+  std::function GlobalInitializer,

Passing OpenMPISDevice shouldn't be necessary, the IsEmbedded flag in the 
Config should be used instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149162

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

[PATCH] D148783: [clangd] Add support TextDocumentEdit.

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148783

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

[PATCH] D149009: [Sema]Select correct lexical context during template instantiate

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:3598
+  FD->isDefined(FDFriend, true) &&
+  FDFriend->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None) {
+// if Function defined by inline friend, use inline fried as DeclContext

HerrCai0907 wrote:
> erichkeane wrote:
> > HerrCai0907 wrote:
> > > erichkeane wrote:
> > > > So in what case would the currently-instantiated definition NOT also be 
> > > > a friend?  I would think this last condition should be able to be an 
> > > > assert instead.
> > > Last condition cannot be an assert, define and declare in difference 
> > > place is common case, what we need to identifier in here is inlined 
> > > friend define.
> > Can you be more clear here?  WHEN can a definition and declaration NOT have 
> > the same friend object kind?  THAT is probably incorrect a bug.
> Sorry I cannot get the point. 
> Here I have 3 conditions:
> 1. FD->getFriendObjectKind() == Decl::FriendObjectKind::FOK_None
> FD(declaration) is not friend object.
> 2. FD->isDefined(FDFriend, true)
> get FDFriend(definition) from FD(declaration).
> 3. FDFriend->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None)
> FDFriend(definition) is friend object.
> 
> matching those 3 condition and then we can say FDFriend is a inline friend 
> like
> ```
> template  int foo(F1 X); // FD
> template  struct A {
> template  friend int foo(F1 X) { return A1; } // FDFriend
> };
> ```
Ok, my question is: WHEN is #3 "false" but #1 and #2 are "true"?  It should not 
be possible for #1 to be "true" but #3 to be "false", as far as I know.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149009

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

[PATCH] D149246: [RISCV] Relax rules for ordering s/z/x prefixed extensions in ISA naming strings

[Philip Reames via Phabricator via cfe-commits]

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

I generally think this is a good idea.  I'd ask you add this to RISCVUsage 
under "The current known variances from the specification are", and mention it 
at the next sync up call, but otherwise, I think we should proceed with this.

LGTM once the above are done.


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

https://reviews.llvm.org/D149246

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

[PATCH] D148094: [DRAFT][clang][CodeGen] Break up TargetInfo.cpp [6/6]

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

+1 on this refactoring being a good idea. The RISC-V changes seem fine to me 
(haven't done a detailed line by line review).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148094

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

[PATCH] D139010: [clang][WebAssembly] Implement support for table types and builtins

[Paulo Matos via Phabricator via cfe-commits]

pmatos updated this revision to Diff 517166.
pmatos marked 3 inline comments as done.
pmatos added a comment.

Fix Wasm table tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139010

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Type.cpp
  clang/lib/Basic/Targets/WebAssembly.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/WebAssembly/builtins-table.c
  clang/test/Sema/builtins-wasm.c
  clang/test/Sema/wasm-refs-and-tables.c
  clang/test/Sema/wasm-refs.c
  clang/test/SemaCXX/wasm-refs-and-tables.cpp
  clang/test/SemaCXX/wasm-refs.cpp
  llvm/include/llvm/CodeGen/WasmAddressSpaces.h
  llvm/include/llvm/IR/Type.h
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyLowerRefTypesIntPtrConv.cpp

Index: llvm/lib/Target/WebAssembly/WebAssemblyLowerRefTypesIntPtrConv.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyLowerRefTypesIntPtrConv.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyLowerRefTypesIntPtrConv.cpp
@@ -62,8 +62,9 @@
   for (inst_iterator I = inst_begin(F), E = inst_end(F); I != E; ++I) {
 PtrToIntInst *PTI = dyn_cast(&*I);
 IntToPtrInst *ITP = dyn_cast(&*I);
-if (!(PTI && WebAssembly::isRefType(PTI->getPointerOperand()->getType())) &&
-!(ITP && WebAssembly::isRefType(ITP->getDestTy(
+if (!(PTI &&
+  PTI->getPointerOperand()->getType()->isWebAssemblyReferenceType()) &&
+!(ITP && ITP->getDestTy()->isWebAssemblyReferenceType()))
   continue;
 
 UndefValue *U = UndefValue::get(I->getType());
Index: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
@@ -1203,7 +1203,7 @@
   // Lastly, if this is a call to a funcref we need to add an instruction
   // table.set to the chain and transform the call.
   if (CLI.CB &&
-  WebAssembly::isFuncrefType(CLI.CB->getCalledOperand()->getType())) {
+  CLI.CB->getCalledOperand()->getType()->isWebAssemblyFuncrefType()) {
 // In the absence of function references proposal where a funcref call is
 // lowered to call_ref, using reference types we generate a table.set to set
 // the funcref to a special table used solely for this purpose, followed by
Index: llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h
===
--- llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h
+++ llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h
@@ -27,43 +27,6 @@
 
 namespace WebAssembly {
 
-enum WasmAddressSpace : unsigned {
-  // Default address space, for pointers to linear memory (stack, heap, data).
-  WASM_ADDRESS_SPACE_DEFAULT = 0,
-  // A non-integral address space for pointers to named objects outside of
-  // linear memory: WebAssembly globals or WebAssembly locals.  Loads and stores
-  // to these pointers are lowered to global.get / global.set or local.get /
-  // local.set, as appropriate.
-  WASM_ADDRESS_SPACE_VAR = 1,
-  // A non-integral address space for externref values
-  WASM_ADDRESS_SPACE_EXTERNREF = 10,
-  // A non-integral address space for funcref values
-  WASM_ADDRESS_SPACE_FUNCREF = 20,
-};
-
-inline bool isDefaultAddressSpace(unsigned AS) {
-  return AS == WASM_ADDRESS_SPACE_DEFAULT;
-}
-inline bool isWasmVarAddressSpace(unsigned AS) {
-  return AS == WASM_ADDRESS_SPACE_VAR;
-}
-inline bool isValidAddressSpace(unsigned AS) {
-  return isDefaultAddressSpace(AS) || isWasmVarAddressSpace(AS);
-}
-inline bool isFuncrefType(const Type *Ty) {
-  return isa(Ty) &&
- Ty->getPointerAddressSpace() ==
- WasmAddressSpace::WASM_ADDRESS_SPACE_FUNCREF;
-}
-inline bool isExternrefType(const Type *Ty) {
-  return isa(Ty) &&
- Ty->getPointerAddressSpace() ==
- WasmAddressSpace::WASM_ADDRESS_SPACE_EXTERNREF;
-}
-inline bool isRefType(const Type *Ty) {
-  return isFuncrefType(Ty) || isExternrefType(Ty);
-}
-
 // Convert StringRef to ValType / HealType / BlockType
 
 MVT parseMVT(StringRef Type);
Index: llvm/lib/Target/WebAssembly/Utils/Web

[PATCH] D149259: [analyzer][NFC] Use std::optional instead of custom "empty" state

[Donát Nagy via Phabricator via cfe-commits]

donat.nagy created this revision.
donat.nagy added reviewers: steakhal, dkrupp, gamesh411.
Herald added subscribers: manas, ASDenysPetrov, martong, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
donat.nagy requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This commit eliminates the uninitialized error state from the class 
RegionRawOffsetV2 (which is locally used by the Clang Static Analyzer
checker alpha.security.ArrayBoundV2) and replaces its use with std::optional.

Motivated by https://reviews.llvm.org/D148355#inline-1437928


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149259

Files:
  clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp

Index: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -51,19 +51,16 @@
 class RegionRawOffsetV2 {
 private:
   const SubRegion *baseRegion;
-  SVal byteOffset;
-
-  RegionRawOffsetV2()
-: baseRegion(nullptr), byteOffset(UnknownVal()) {}
+  NonLoc byteOffset;
 
 public:
   RegionRawOffsetV2(const SubRegion *base, NonLoc offset)
   : baseRegion(base), byteOffset(offset) { assert(base); }
 
-  NonLoc getByteOffset() const { return byteOffset.castAs(); }
+  NonLoc getByteOffset() const { return byteOffset; }
   const SubRegion *getRegion() const { return baseRegion; }
 
-  static RegionRawOffsetV2 computeOffset(ProgramStateRef state,
+  static std::optional computeOffset(ProgramStateRef state,
  SValBuilder &svalBuilder,
  SVal location);
 
@@ -158,16 +155,16 @@
   ProgramStateRef state = checkerContext.getState();
 
   SValBuilder &svalBuilder = checkerContext.getSValBuilder();
-  const RegionRawOffsetV2 &rawOffset =
+  const std::optional &rawOffset =
 RegionRawOffsetV2::computeOffset(state, svalBuilder, location);
 
-  if (!rawOffset.getRegion())
+  if (!rawOffset)
 return;
 
-  NonLoc ByteOffset = rawOffset.getByteOffset();
+  NonLoc ByteOffset = rawOffset->getByteOffset();
 
   // CHECK LOWER BOUND
-  const MemSpaceRegion *SR = rawOffset.getRegion()->getMemorySpace();
+  const MemSpaceRegion *SR = rawOffset->getRegion()->getMemorySpace();
   if (!llvm::isa(SR)) {
 // A pointer to UnknownSpaceRegion may point to the middle of
 // an allocated region.
@@ -188,7 +185,7 @@
 
   // CHECK UPPER BOUND
   DefinedOrUnknownSVal Size =
-  getDynamicExtent(state, rawOffset.getRegion(), svalBuilder);
+  getDynamicExtent(state, rawOffset->getRegion(), svalBuilder);
   if (auto KnownSize = Size.getAs()) {
 auto [state_withinUpperBound, state_exceedsUpperBound] =
 compareValueToThreshold(state, ByteOffset, *KnownSize, svalBuilder);
@@ -310,7 +307,7 @@
 
 /// Compute a raw byte offset from a base region.  Used for array bounds
 /// checking.
-RegionRawOffsetV2 RegionRawOffsetV2::computeOffset(ProgramStateRef state,
+std::optional RegionRawOffsetV2::computeOffset(ProgramStateRef state,
SValBuilder &svalBuilder,
SVal location)
 {
@@ -324,18 +321,18 @@
   if (auto Offset = getValue(offset, svalBuilder).getAs())
 return RegionRawOffsetV2(subReg, *Offset);
 }
-return RegionRawOffsetV2();
+return {};
   }
   case MemRegion::ElementRegionKind: {
 const ElementRegion *elemReg = cast(region);
 SVal index = elemReg->getIndex();
 if (!isa(index))
-  return RegionRawOffsetV2();
+  return {};
 QualType elemType = elemReg->getElementType();
 // If the element is an incomplete type, go no further.
 ASTContext &astContext = svalBuilder.getContext();
 if (elemType->isIncompleteType())
-  return RegionRawOffsetV2();
+  return {};
 
 // Update the offset.
 offset = addValue(state,
@@ -347,14 +344,14 @@
   svalBuilder);
 
 if (offset.isUnknownOrUndef())
-  return RegionRawOffsetV2();
+  return {};
 
 region = elemReg->getSuperRegion();
 continue;
   }
 }
   }
-  return RegionRawOffsetV2();
+  return {};
 }
 
 void ento::registerArrayBoundCheckerV2(CheckerManager &mgr) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138810: [RISCV] Support vector crypto extension C intrinsics

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

I just wanted to check on whether this is ready to review? Also, to what degree 
are these intrinsics standardised and where is the relevant specification for 
them? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138810

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

[PATCH] D142393: [OpenMP] Add 'amdgpu-flat-work-group-size' to OpenMP kernels

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.def:271
 LANGOPT(GPUAllowDeviceInit, 1, 0, "allowing device side global init functions 
for HIP")
-LANGOPT(GPUMaxThreadsPerBlock, 32, 1024, "default max threads per block for 
kernel launch bounds for HIP")
+LANGOPT(GPUMaxThreadsPerBlock, 32, 1024, "default max threads per block for 
kernel launch bounds for OpenMP/HIP")
 LANGOPT(GPUDeferDiag, 1, 0, "defer host/device related diagnostic messages for 
CUDA/HIP")

arsenm wrote:
> probably should drop the language and describe what it is
CUDA does not use it. Drop the language may cause confusion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142393

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

[PATCH] D148066: [RISCV] Add Smaia and Ssaia extensions support

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

In D148066#4294924 , @kito-cheng 
wrote:

>> My concern would be that as we don't gate CSR names on enabling the relevant 
>> extension, people could start using CSR names and encodings that could 
>> change, without opting in via -menable-experimental-extensions, perhaps not 
>> realising that they're using the unratified version. OTOH, you could argue 
>> it was user error from the start by not trying to specify all the needed 
>> extensions in the ISA naming string.
>
> We decide don't gate CSR before, but I am wondering maybe we should gate 
> those CSR if they are defined by a unratified/experimental ext., and remove 
> the checking once it ratified, since it might change the name or CSR number 
> before ratified.

That's definitely a risk, but at least when people try to do the "right" thing 
and specify the extension name in the ISA string, they'll quickly find out that 
the version supported in the compiler is experimental. Given the cost of 
marking such CSR extensions as experimental is near-zero, I think we might as 
well. I agree gating the CSR names might also make sense, but then we're back 
to the effort of testing this. I think our conclusions before were that nobody 
is particularly opposed to doing finer grained control of CSRs for new 
extensions at least (it's more problematic for the older CSRs given spec 
changes to move CSRs to separate extensions), so I think it would be find if 
someone wanted to implement that. I don't think we need that as a pre-requisite 
for this patch though, IMHO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148066

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

[PATCH] D139010: [clang][WebAssembly] Implement support for table types and builtins

[Paulo Matos via Phabricator via cfe-commits]

pmatos added a comment.

Thanks for the comments - I am working on addressing these at the moment.
The LLVM part of the patch is just some refactoring and therefore should be 
pretty trivial, pinging @tlively in case he has some time.




Comment at: clang/test/Sema/wasm-refs-and-tables.c:17
+static __externref_t t6[] = {0}; // expected-error {{only zero-length 
WebAssembly tables are currently supported}}
+__externref_t t7[0]; // expected-error {{WebAssembly table must be 
static}}
+static __externref_t t8[0][0];   // expected-error {{multi-dimensional arrays 
of WebAssembly references are not allowed}}

aaron.ballman wrote:
> So why is `extern __externref_t r2;` allowed? Is it because it's not an array 
> declaration?
I am not sure I understand the question. The externref value can be declared in 
another module and here we just define that. Array declarations of externref 
just define tables of externref values.



Comment at: clang/test/Sema/wasm-refs-and-tables.c:48
+void illegal_argument_4(__externref_t ***table);// expected-error 
{{pointer to WebAssembly reference type is not allowed}}
+void illegal_argument_5(__externref_t (*table)[0]); // expected-error {{cannot 
form a pointer to a WebAssembly table}}
+

aaron.ballman wrote:
> How about:
> ```
> void foo(__externref_t table[0]);
> ```
> I'd expect this to also not be allowed (that's just a fancy spelling for a 
> weird pointer).
That's correct, that's not allowed. Added the test case.



Comment at: clang/test/Sema/wasm-refs-and-tables.c:104
+  table[0] = ref; // expected-error {{cannot subscript a 
WebAssembly table}}
+
+  return ref;

aaron.ballman wrote:
> Please don't hate me, but... what about:
> ```
> int i = 0;
> __externref_t oh_no_vlas[i];
> ```
> 
:) Test added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139010

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

[PATCH] D148066: [RISCV] Add Smaia and Ssaia extensions support

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

In D148066#4294924 , @kito-cheng 
wrote:

>> My concern would be that as we don't gate CSR names on enabling the relevant 
>> extension, people could start using CSR names and encodings that could 
>> change, without opting in via -menable-experimental-extensions, perhaps not 
>> realising that they're using the unratified version. OTOH, you could argue 
>> it was user error from the start by not trying to specify all the needed 
>> extensions in the ISA naming string.
>
> We decide don't gate CSR before, but I am wondering maybe we should gate 
> those CSR if they are defined by a unratified/experimental ext., and remove 
> the checking once it ratified, since it might change the name or CSR number 
> before ratified.

I think I agree with Kito here.  Requiring gating for experimental extensions 
even if we don't gate ratified ones seems like a good way to address the issue 
of name collision risk in experimental extensions.

I'm less concerned about number reassignment.  I'm most concerned that the same 
name might be reused across two extensions in a mutually incompatible way.  
This would clearly get fixed before both were ratified, but well, the entire 
point of experimental is that they not yet ratified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148066

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

[PATCH] D148066: [RISCV] Add Smaia and Ssaia extensions support

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

FWIW, I've reviewed the CSR numbers vs the spec so LGTM from that perspective.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148066

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

[clang] 62eec15 - [clang-rename] Exit gracefully when no input provided

[Shivam Gupta via cfe-commits]

Author: Shivam Gupta
Date: 2023-04-26T20:23:14+05:30
New Revision: 62eec1584d2cd7634c31d4b82215fa8e260cf3b8

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

LOG: [clang-rename] Exit gracefully when no input provided

clang-rename on a non existing file segfaults

Command to run -
$ clang-rename -offset=0 -new-name=plop asdasd

Error while processing llvm-project/asdasd.
clang-rename: llvm-project/llvm/include/llvm/Support/ErrorOr.h:237:
llvm::ErrorOr::storage_type* llvm::ErrorOr::getStorage()
[with T = const clang::FileEntry*; llvm::ErrorOr::storage_type = const 
clang::FileEntry*]:
Assertion `!HasError && "Cannot get value when an error exists!"' failed.

[1]827497 IOT instruction  clang-rename -offset=0 -new-name=plop asdasd

This fixes https://github.com/llvm/llvm-project/issues/36471.

Reviewed By: aaron.ballman

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

Added: 
clang/test/clang-rename/NonExistFile.cpp

Modified: 
clang/docs/ReleaseNotes.rst
clang/tools/clang-rename/ClangRename.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 55ec1cdef52fa..7dfca82c00876 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -235,6 +235,8 @@ Improvements to Clang's diagnostics
 Bug Fixes in This Version
 -
 
+- Fix segfault while running clang-rename on a non existing file.
+  (`#36471 `_)
 - Fix crash when diagnosing incorrect usage of ``_Nullable`` involving alias
   templates.
   (`#60344 `_)

diff  --git a/clang/test/clang-rename/NonExistFile.cpp 
b/clang/test/clang-rename/NonExistFile.cpp
new file mode 100644
index 0..f45839be80473
--- /dev/null
+++ b/clang/test/clang-rename/NonExistFile.cpp
@@ -0,0 +1,2 @@
+// RUN: not clang-rename -offset=0 -new-name=bar non-existing-file 2>&1 | 
FileCheck %s
+// CHECK: clang-rename: non-existing-file does not exist.

diff  --git a/clang/tools/clang-rename/ClangRename.cpp 
b/clang/tools/clang-rename/ClangRename.cpp
index e7ceac7dbf303..24c9d8521dc27 100644
--- a/clang/tools/clang-rename/ClangRename.cpp
+++ b/clang/tools/clang-rename/ClangRename.cpp
@@ -229,6 +229,10 @@ int main(int argc, const char **argv) {
 Tool.applyAllReplacements(Rewrite);
 for (const auto &File : Files) {
   auto Entry = FileMgr.getFile(File);
+  if (!Entry) {
+errs() << "clang-rename: " << File << " does not exist.\n";
+return 1;
+  }
   const auto ID = Sources.getOrCreateFileID(*Entry, SrcMgr::C_User);
   Rewrite.getEditBuffer(ID).write(outs());
 }



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

[PATCH] D148439: [clang-rename] Exit gracefully when no input provided

[Shivam Gupta via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG62eec1584d2c: [clang-rename] Exit gracefully when no input 
provided (authored by xgupta).

Changed prior to commit:
  https://reviews.llvm.org/D148439?vs=514842&id=517178#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148439

Files:
  clang/docs/ReleaseNotes.rst
  clang/test/clang-rename/NonExistFile.cpp
  clang/tools/clang-rename/ClangRename.cpp


Index: clang/tools/clang-rename/ClangRename.cpp
===
--- clang/tools/clang-rename/ClangRename.cpp
+++ clang/tools/clang-rename/ClangRename.cpp
@@ -229,6 +229,10 @@
 Tool.applyAllReplacements(Rewrite);
 for (const auto &File : Files) {
   auto Entry = FileMgr.getFile(File);
+  if (!Entry) {
+errs() << "clang-rename: " << File << " does not exist.\n";
+return 1;
+  }
   const auto ID = Sources.getOrCreateFileID(*Entry, SrcMgr::C_User);
   Rewrite.getEditBuffer(ID).write(outs());
 }
Index: clang/test/clang-rename/NonExistFile.cpp
===
--- /dev/null
+++ clang/test/clang-rename/NonExistFile.cpp
@@ -0,0 +1,2 @@
+// RUN: not clang-rename -offset=0 -new-name=bar non-existing-file 2>&1 | 
FileCheck %s
+// CHECK: clang-rename: non-existing-file does not exist.
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -235,6 +235,8 @@
 Bug Fixes in This Version
 -
 
+- Fix segfault while running clang-rename on a non existing file.
+  (`#36471 `_)
 - Fix crash when diagnosing incorrect usage of ``_Nullable`` involving alias
   templates.
   (`#60344 `_)


Index: clang/tools/clang-rename/ClangRename.cpp
===
--- clang/tools/clang-rename/ClangRename.cpp
+++ clang/tools/clang-rename/ClangRename.cpp
@@ -229,6 +229,10 @@
 Tool.applyAllReplacements(Rewrite);
 for (const auto &File : Files) {
   auto Entry = FileMgr.getFile(File);
+  if (!Entry) {
+errs() << "clang-rename: " << File << " does not exist.\n";
+return 1;
+  }
   const auto ID = Sources.getOrCreateFileID(*Entry, SrcMgr::C_User);
   Rewrite.getEditBuffer(ID).write(outs());
 }
Index: clang/test/clang-rename/NonExistFile.cpp
===
--- /dev/null
+++ clang/test/clang-rename/NonExistFile.cpp
@@ -0,0 +1,2 @@
+// RUN: not clang-rename -offset=0 -new-name=bar non-existing-file 2>&1 | FileCheck %s
+// CHECK: clang-rename: non-existing-file does not exist.
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -235,6 +235,8 @@
 Bug Fixes in This Version
 -
 
+- Fix segfault while running clang-rename on a non existing file.
+  (`#36471 `_)
 - Fix crash when diagnosing incorrect usage of ``_Nullable`` involving alias
   templates.
   (`#60344 `_)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149205: [Headers][doc] Add "gather" intrinsic descriptions to avx2intrin.h

[Paul Robinson via Phabricator via cfe-commits]

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



Comment at: clang/lib/Headers/avx2intrin.h:942
+///
+/// \code
+/// FOR element := 0 to 1

pengfei wrote:
> Use `\code{.operation}` please, the same below. Our internal tool will 
> recognize this pattern.
Ok. I'll modify our tooling to ignore it.


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

https://reviews.llvm.org/D149205

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

[PATCH] D148439: [clang-rename] Exit gracefully when no input provided

[Shivam Gupta via Phabricator via cfe-commits]

xgupta added a comment.

In D148439#4298606 , @aaron.ballman 
wrote:

> LGTM, but please add a release note when landing the changes.

Thanks for the review, added a release note for change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148439

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

[clang] 039ae62 - [Headers][doc] Add "gather" intrinsic descriptions to avx2intrin.h

[Paul Robinson via cfe-commits]

Author: Paul Robinson
Date: 2023-04-26T08:04:31-07:00
New Revision: 039ae62405b6ea130b6f84cd54fea1e8599f1634

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

LOG: [Headers][doc] Add "gather" intrinsic descriptions to avx2intrin.h

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

Added: 


Modified: 
clang/lib/Headers/avx2intrin.h

Removed: 




diff  --git a/clang/lib/Headers/avx2intrin.h b/clang/lib/Headers/avx2intrin.h
index f8521e7d72b5e..33f24f2443b3a 100644
--- a/clang/lib/Headers/avx2intrin.h
+++ b/clang/lib/Headers/avx2intrin.h
@@ -935,102 +935,810 @@ _mm_srlv_epi64(__m128i __X, __m128i __Y)
   return (__m128i)__builtin_ia32_psrlv2di((__v2di)__X, (__v2di)__Y);
 }
 
+/// Conditionally gathers two 64-bit floating-point values, either from the
+///128-bit vector of [2 x double] in \a a, or from memory \a m using scaled
+///indexes from the 128-bit vector of [4 x i32] in \a i. The 128-bit vector
+///of [2 x double] in \a mask determines the source for each element.
+///
+/// \code{.operation}
+/// FOR element := 0 to 1
+///   j := element*64
+///   k := element*32
+///   IF mask[j+63] == 0
+/// result[j+63:j] := a[j+63:j]
+///   ELSE
+/// result[j+63:j] := Load64(m + SignExtend(i[k+31:k])*s)
+///   FI
+/// ENDFOR
+/// \endcode
+///
+/// \headerfile 
+///
+/// \code
+/// __m128d _mm_mask_i32gather_pd(__m128d a, const double *m, __m128i i,
+///   __m128d mask, const int s);
+/// \endcode
+///
+/// This intrinsic corresponds to the \c VGATHERDPD instruction.
+///
+/// \param a
+///A 128-bit vector of [2 x double] used as the source when a mask bit is
+///zero.
+/// \param m
+///A pointer to the memory used for loading values.
+/// \param i
+///A 128-bit vector of [4 x i32] containing signed indexes into \a m. Only
+///the first two elements are used.
+/// \param mask
+///A 128-bit vector of [2 x double] containing the mask. The most
+///significant bit of each element in the mask vector represents the mask
+///bits. If a mask bit is zero, the corresponding value from vector \a a
+///is gathered; otherwise the value is loaded from memory.
+/// \param s
+///A literal constant scale factor for the indexes in \a i. Must be
+///1, 2, 4, or 8.
+/// \returns A 128-bit vector of [2 x double] containing the gathered values.
 #define _mm_mask_i32gather_pd(a, m, i, mask, s) \
   ((__m128d)__builtin_ia32_gatherd_pd((__v2df)(__m128i)(a), \
   (double const *)(m), \
   (__v4si)(__m128i)(i), \
   (__v2df)(__m128d)(mask), (s)))
 
+/// Conditionally gathers four 64-bit floating-point values, either from the
+///256-bit vector of [4 x double] in \a a, or from memory \a m using scaled
+///indexes from the 128-bit vector of [4 x i32] in \a i. The 256-bit vector
+///of [4 x double] in \a mask determines the source for each element.
+///
+/// \code{.operation}
+/// FOR element := 0 to 3
+///   j := element*64
+///   k := element*32
+///   IF mask[j+63] == 0
+/// result[j+63:j] := a[j+63:j]
+///   ELSE
+/// result[j+63:j] := Load64(m + SignExtend(i[k+31:k])*s)
+///   FI
+/// ENDFOR
+/// \endcode
+///
+/// \headerfile 
+///
+/// \code
+/// __m256d _mm256_mask_i32gather_pd(__m256d a, const double *m, __m128i i,
+///  __m256d mask, const int s);
+/// \endcode
+///
+/// This intrinsic corresponds to the \c VGATHERDPD instruction.
+///
+/// \param a
+///A 256-bit vector of [4 x double] used as the source when a mask bit is
+///zero.
+/// \param m
+///A pointer to the memory used for loading values.
+/// \param i
+///A 128-bit vector of [4 x i32] containing signed indexes into \a m.
+/// \param mask
+///A 256-bit vector of [4 x double] containing the mask. The most
+///significant bit of each element in the mask vector represents the mask
+///bits. If a mask bit is zero, the corresponding value from vector \a a
+///is gathered; otherwise the value is loaded from memory.
+/// \param s
+///A literal constant scale factor for the indexes in \a i. Must be
+///1, 2, 4, or 8.
+/// \returns A 256-bit vector of [4 x double] containing the gathered values.
 #define _mm256_mask_i32gather_pd(a, m, i, mask, s) \
   ((__m256d)__builtin_ia32_gatherd_pd256((__v4df)(__m256d)(a), \
  (double const *)(m), \
  (__v4si)(__m128i)(i), \
  (__v4df)(__m256d)(mask), (s)))
 
+/// Conditionally gathers two 64-bit floating-point values, either from the
+///128-bit vector of [2 x double] in \a a, or from memory \a m u

[PATCH] D149259: [analyzer][NFC] Use std::optional instead of custom "empty" state

[Balázs Benics via Phabricator via cfe-commits]

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

Approved, assuming we prefer `std::nullopt` over the default construction of 
`std::optional` and you `clang-format` the affected hunks, such as the 
declaration of `computeOffset`.
Make sure the commit message complies with:

- https://www.llvm.org/docs/Phabricator.html#committing-a-change
- https://llvm.org/docs/DeveloperPolicy.html#commit-messages




Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:324
 }
-return RegionRawOffsetV2();
+return {};
   }

I'd recommend using the more explicit version for such cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149259

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

[PATCH] D149205: [Headers][doc] Add "gather" intrinsic descriptions to avx2intrin.h

[Paul Robinson via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
probinson marked an inline comment as done.
Closed by commit rG039ae62405b6: [Headers][doc] Add "gather" 
intrinsic descriptions to avx2intrin.h (authored by probinson).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D149205?vs=516917&id=517182#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149205

Files:
  clang/lib/Headers/avx2intrin.h

Index: clang/lib/Headers/avx2intrin.h
===
--- clang/lib/Headers/avx2intrin.h
+++ clang/lib/Headers/avx2intrin.h
@@ -935,102 +935,810 @@
   return (__m128i)__builtin_ia32_psrlv2di((__v2di)__X, (__v2di)__Y);
 }
 
+/// Conditionally gathers two 64-bit floating-point values, either from the
+///128-bit vector of [2 x double] in \a a, or from memory \a m using scaled
+///indexes from the 128-bit vector of [4 x i32] in \a i. The 128-bit vector
+///of [2 x double] in \a mask determines the source for each element.
+///
+/// \code{.operation}
+/// FOR element := 0 to 1
+///   j := element*64
+///   k := element*32
+///   IF mask[j+63] == 0
+/// result[j+63:j] := a[j+63:j]
+///   ELSE
+/// result[j+63:j] := Load64(m + SignExtend(i[k+31:k])*s)
+///   FI
+/// ENDFOR
+/// \endcode
+///
+/// \headerfile 
+///
+/// \code
+/// __m128d _mm_mask_i32gather_pd(__m128d a, const double *m, __m128i i,
+///   __m128d mask, const int s);
+/// \endcode
+///
+/// This intrinsic corresponds to the \c VGATHERDPD instruction.
+///
+/// \param a
+///A 128-bit vector of [2 x double] used as the source when a mask bit is
+///zero.
+/// \param m
+///A pointer to the memory used for loading values.
+/// \param i
+///A 128-bit vector of [4 x i32] containing signed indexes into \a m. Only
+///the first two elements are used.
+/// \param mask
+///A 128-bit vector of [2 x double] containing the mask. The most
+///significant bit of each element in the mask vector represents the mask
+///bits. If a mask bit is zero, the corresponding value from vector \a a
+///is gathered; otherwise the value is loaded from memory.
+/// \param s
+///A literal constant scale factor for the indexes in \a i. Must be
+///1, 2, 4, or 8.
+/// \returns A 128-bit vector of [2 x double] containing the gathered values.
 #define _mm_mask_i32gather_pd(a, m, i, mask, s) \
   ((__m128d)__builtin_ia32_gatherd_pd((__v2df)(__m128i)(a), \
   (double const *)(m), \
   (__v4si)(__m128i)(i), \
   (__v2df)(__m128d)(mask), (s)))
 
+/// Conditionally gathers four 64-bit floating-point values, either from the
+///256-bit vector of [4 x double] in \a a, or from memory \a m using scaled
+///indexes from the 128-bit vector of [4 x i32] in \a i. The 256-bit vector
+///of [4 x double] in \a mask determines the source for each element.
+///
+/// \code{.operation}
+/// FOR element := 0 to 3
+///   j := element*64
+///   k := element*32
+///   IF mask[j+63] == 0
+/// result[j+63:j] := a[j+63:j]
+///   ELSE
+/// result[j+63:j] := Load64(m + SignExtend(i[k+31:k])*s)
+///   FI
+/// ENDFOR
+/// \endcode
+///
+/// \headerfile 
+///
+/// \code
+/// __m256d _mm256_mask_i32gather_pd(__m256d a, const double *m, __m128i i,
+///  __m256d mask, const int s);
+/// \endcode
+///
+/// This intrinsic corresponds to the \c VGATHERDPD instruction.
+///
+/// \param a
+///A 256-bit vector of [4 x double] used as the source when a mask bit is
+///zero.
+/// \param m
+///A pointer to the memory used for loading values.
+/// \param i
+///A 128-bit vector of [4 x i32] containing signed indexes into \a m.
+/// \param mask
+///A 256-bit vector of [4 x double] containing the mask. The most
+///significant bit of each element in the mask vector represents the mask
+///bits. If a mask bit is zero, the corresponding value from vector \a a
+///is gathered; otherwise the value is loaded from memory.
+/// \param s
+///A literal constant scale factor for the indexes in \a i. Must be
+///1, 2, 4, or 8.
+/// \returns A 256-bit vector of [4 x double] containing the gathered values.
 #define _mm256_mask_i32gather_pd(a, m, i, mask, s) \
   ((__m256d)__builtin_ia32_gatherd_pd256((__v4df)(__m256d)(a), \
  (double const *)(m), \
  (__v4si)(__m128i)(i), \
  (__v4df)(__m256d)(mask), (s)))
 
+/// Conditionally gathers two 64-bit floating-point values, either from the
+///128-bit vector of [2 x double] in \a a, or from memory \a m using scaled
+///indexes from the 128-bit vecto

[PATCH] D149009: [Sema]Select correct lexical context during template instantiate

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:3598
+  FD->isDefined(FDFriend, true) &&
+  FDFriend->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None) {
+// if Function defined by inline friend, use inline fried as DeclContext

erichkeane wrote:
> HerrCai0907 wrote:
> > erichkeane wrote:
> > > HerrCai0907 wrote:
> > > > erichkeane wrote:
> > > > > So in what case would the currently-instantiated definition NOT also 
> > > > > be a friend?  I would think this last condition should be able to be 
> > > > > an assert instead.
> > > > Last condition cannot be an assert, define and declare in difference 
> > > > place is common case, what we need to identifier in here is inlined 
> > > > friend define.
> > > Can you be more clear here?  WHEN can a definition and declaration NOT 
> > > have the same friend object kind?  THAT is probably incorrect a bug.
> > Sorry I cannot get the point. 
> > Here I have 3 conditions:
> > 1. FD->getFriendObjectKind() == Decl::FriendObjectKind::FOK_None
> > FD(declaration) is not friend object.
> > 2. FD->isDefined(FDFriend, true)
> > get FDFriend(definition) from FD(declaration).
> > 3. FDFriend->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None)
> > FDFriend(definition) is friend object.
> > 
> > matching those 3 condition and then we can say FDFriend is a inline friend 
> > like
> > ```
> > template  int foo(F1 X); // FD
> > template  struct A {
> > template  friend int foo(F1 X) { return A1; } // FDFriend
> > };
> > ```
> Ok, my question is: WHEN is #3 "false" but #1 and #2 are "true"?  It should 
> not be possible for #1 to be "true" but #3 to be "false", as far as I know.
It looks like `getFriendObjectKind()` returns a declaration specific value such 
that, for the example given, it returns `FOK_None` for the first declaration 
and `FOK_Declared` for the friend definition. (I haven't tested that, but that 
is what my brief reading of the code suggests).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149009

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

[PATCH] D149009: [Sema]Select correct lexical context during template instantiate

[Erich Keane via Phabricator via cfe-commits]

erichkeane accepted this revision.
erichkeane added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:3598
+  FD->isDefined(FDFriend, true) &&
+  FDFriend->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None) {
+// if Function defined by inline friend, use inline fried as DeclContext

tahonermann wrote:
> erichkeane wrote:
> > HerrCai0907 wrote:
> > > erichkeane wrote:
> > > > HerrCai0907 wrote:
> > > > > erichkeane wrote:
> > > > > > So in what case would the currently-instantiated definition NOT 
> > > > > > also be a friend?  I would think this last condition should be able 
> > > > > > to be an assert instead.
> > > > > Last condition cannot be an assert, define and declare in difference 
> > > > > place is common case, what we need to identifier in here is inlined 
> > > > > friend define.
> > > > Can you be more clear here?  WHEN can a definition and declaration NOT 
> > > > have the same friend object kind?  THAT is probably incorrect a bug.
> > > Sorry I cannot get the point. 
> > > Here I have 3 conditions:
> > > 1. FD->getFriendObjectKind() == Decl::FriendObjectKind::FOK_None
> > > FD(declaration) is not friend object.
> > > 2. FD->isDefined(FDFriend, true)
> > > get FDFriend(definition) from FD(declaration).
> > > 3. FDFriend->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None)
> > > FDFriend(definition) is friend object.
> > > 
> > > matching those 3 condition and then we can say FDFriend is a inline 
> > > friend like
> > > ```
> > > template  int foo(F1 X); // FD
> > > template  struct A {
> > > template  friend int foo(F1 X) { return A1; } // FDFriend
> > > };
> > > ```
> > Ok, my question is: WHEN is #3 "false" but #1 and #2 are "true"?  It should 
> > not be possible for #1 to be "true" but #3 to be "false", as far as I know.
> It looks like `getFriendObjectKind()` returns a declaration specific value 
> such that, for the example given, it returns `FOK_None` for the first 
> declaration and `FOK_Declared` for the friend definition. (I haven't tested 
> that, but that is what my brief reading of the code suggests).
Ah, right! thanks for that Tom! Yeah, its just that the 1st declaration is NOT 
a friend, I had these reversed in my head so was very confused.  OK, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149009

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

[clang] b56b15e - [dataflow] HTMLLogger - show the value of the current expr

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2023-04-26T17:15:23+02:00
New Revision: b56b15ed719b4b6a5fe1a0a8b5ece54467ab2fee

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

LOG: [dataflow] HTMLLogger - show the value of the current expr

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/Value.h
clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
clang/lib/Analysis/FlowSensitive/HTMLLogger.css
clang/lib/Analysis/FlowSensitive/HTMLLogger.html
clang/lib/Analysis/FlowSensitive/HTMLLogger.js
clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/Value.h 
b/clang/include/clang/Analysis/FlowSensitive/Value.h
index 32d10a3489483..861a9963e6689 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Value.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Value.h
@@ -73,6 +73,11 @@ class Value {
 Properties.insert_or_assign(Name, &Val);
   }
 
+  llvm::iterator_range::const_iterator>
+  properties() const {
+return {Properties.begin(), Properties.end()};
+  }
+
 private:
   Kind ValKind;
   llvm::StringMap Properties;
@@ -307,6 +312,12 @@ class StructValue final : public Value {
   /// Assigns `Val` as the child value for `D`.
   void setChild(const ValueDecl &D, Value &Val) { Children[&D] = &Val; }
 
+  llvm::iterator_range<
+  llvm::DenseMap::const_iterator>
+  children() const {
+return {Children.begin(), Children.end()};
+  }
+
 private:
   llvm::DenseMap Children;
 };

diff  --git a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp 
b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
index b229194bc8f44..14293a3043f98 100644
--- a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
+++ b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
@@ -67,6 +67,7 @@
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/Program.h"
+#include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/raw_ostream.h"
 // Defines assets: HTMLLogger_{html_js,css}
 #include "HTMLLogger.inc"
@@ -79,6 +80,96 @@ llvm::Expected renderSVG(llvm::StringRef 
DotGraph);
 
 using StreamFactory = std::function()>;
 
+// Recursively dumps Values/StorageLocations as JSON
+class ModelDumper {
+public:
+  ModelDumper(llvm::json::OStream &JOS, const Environment &Env)
+  : JOS(JOS), Env(Env) {}
+
+  void dump(Value &V) {
+JOS.attribute("value_id", llvm::to_string(&V));
+if (!Visited.insert(&V).second)
+  return;
+
+JOS.attribute("kind", debugString(V.getKind()));
+
+switch (V.getKind()) {
+case Value::Kind::Integer:
+case Value::Kind::TopBool:
+case Value::Kind::AtomicBool:
+  break;
+case Value::Kind::Reference:
+  JOS.attributeObject(
+  "referent", [&] { dump(cast(V).getReferentLoc()); });
+  break;
+case Value::Kind::Pointer:
+  JOS.attributeObject(
+  "pointee", [&] { dump(cast(V).getPointeeLoc()); });
+  break;
+case Value::Kind::Struct:
+  for (const auto &Child : cast(V).children())
+JOS.attributeObject("f:" + Child.first->getNameAsString(),
+[&] { dump(*Child.second); });
+  break;
+case Value::Kind::Disjunction: {
+  auto &VV = cast(V);
+  JOS.attributeObject("lhs", [&] { dump(VV.getLeftSubValue()); });
+  JOS.attributeObject("rhs", [&] { dump(VV.getRightSubValue()); });
+  break;
+}
+case Value::Kind::Conjunction: {
+  auto &VV = cast(V);
+  JOS.attributeObject("lhs", [&] { dump(VV.getLeftSubValue()); });
+  JOS.attributeObject("rhs", [&] { dump(VV.getRightSubValue()); });
+  break;
+}
+case Value::Kind::Negation: {
+  auto &VV = cast(V);
+  JOS.attributeObject("not", [&] { dump(VV.getSubVal()); });
+  break;
+}
+case Value::Kind::Implication: {
+  auto &VV = cast(V);
+  JOS.attributeObject("if", [&] { dump(VV.getLeftSubValue()); });
+  JOS.attributeObject("then", [&] { dump(VV.getRightSubValue()); });
+  break;
+}
+case Value::Kind::Biconditional: {
+  auto &VV = cast(V);
+  JOS.attributeObject("lhs", [&] { dump(VV.getLeftSubValue()); });
+  JOS.attributeObject("rhs", [&] { dump(VV.getRightSubValue()); });
+  break;
+}
+}
+
+for (const auto& Prop : V.properties())
+  JOS.attributeObject(("p:" + Prop.first()).str(),
+  [&] { dump(*Prop.second); });
+
+// Running the SAT solver is expensive, but knowing which booleans are
+// guaranteed true/false here is valuable and hard to determine by hand.
+if (auto *B = llvm::dyn_cast(&V)) {
+  JOS.attribute("truth", Env.flowConditionImplies(*B) ? "true"
+ : Env.flowC

[PATCH] D148949: [dataflow] HTMLLogger - show the value of the current expr

[Sam McCall 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 rGb56b15ed719b: [dataflow] HTMLLogger - show the value of the 
current expr (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148949

Files:
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
  clang/lib/Analysis/FlowSensitive/HTMLLogger.css
  clang/lib/Analysis/FlowSensitive/HTMLLogger.html
  clang/lib/Analysis/FlowSensitive/HTMLLogger.js
  clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
@@ -176,6 +176,7 @@
   << "has analysis point state";
   EXPECT_THAT(Logs[0], HasSubstr("transferBranch(0)")) << "has analysis logs";
   EXPECT_THAT(Logs[0], HasSubstr("LocToVal")) << "has built-in lattice dump";
+  EXPECT_THAT(Logs[0], HasSubstr("\"type\": \"int\"")) << "has value dump";
 }
 
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/HTMLLogger.js
===
--- clang/lib/Analysis/FlowSensitive/HTMLLogger.js
+++ clang/lib/Analysis/FlowSensitive/HTMLLogger.js
@@ -73,6 +73,9 @@
 }
 return parent.insertBefore(clone, next);
   }
+  // data-use="xyz": use  instead. (Allows recursion.)
+  if ('use' in tmpl.dataset)
+return inflate(document.getElementById(tmpl.dataset.use), data, parent, next);
   //  tag handling. Base case: recursively inflate.
   function handle(data) {
 for (c of tmpl.content.childNodes)
Index: clang/lib/Analysis/FlowSensitive/HTMLLogger.html
===
--- clang/lib/Analysis/FlowSensitive/HTMLLogger.html
+++ clang/lib/Analysis/FlowSensitive/HTMLLogger.html
@@ -10,6 +10,30 @@
 
 
 
+
+
+  
+
+  {{v.kind}}
+#{{v.value_id}}
+  
+  
+{{v.type}} @{{v.location}}
+  
+
+
+  {{kv[0]}}
+{{kv[1]}}
+
+  
+
+  
+
+  
+
+
 
 
 
@@ -56,6 +80,10 @@
   {{state.block}} (iteration {{state.iter}}) initial state
   Element {{selection.elt}} (iteration {{state.iter}})
 
+
+  Value
+  
+
 
   Logs
   {{state.logs}}
Index: clang/lib/Analysis/FlowSensitive/HTMLLogger.css
===
--- clang/lib/Analysis/FlowSensitive/HTMLLogger.css
+++ clang/lib/Analysis/FlowSensitive/HTMLLogger.css
@@ -116,3 +116,27 @@
   position: relative;
   margin-left: 0.5em;
 }
+
+.value {
+  border: 1px solid #888;
+  font-size: x-small;
+  flex-grow: 1;
+}
+.value summary {
+  background-color: #ace;
+  display: flex;
+  justify-content: space-between;
+}
+.value .address {
+  font-size: xx-small;
+  font-family: monospace;
+  color: #888;
+}
+.value .property {
+  display: flex;
+  margin-top: 0.5em;
+}
+.value .property .key {
+  font-weight: bold;
+  min-width: 5em;
+}
Index: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
===
--- clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
+++ clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
@@ -67,6 +67,7 @@
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/Program.h"
+#include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/raw_ostream.h"
 // Defines assets: HTMLLogger_{html_js,css}
 #include "HTMLLogger.inc"
@@ -79,6 +80,96 @@
 
 using StreamFactory = std::function()>;
 
+// Recursively dumps Values/StorageLocations as JSON
+class ModelDumper {
+public:
+  ModelDumper(llvm::json::OStream &JOS, const Environment &Env)
+  : JOS(JOS), Env(Env) {}
+
+  void dump(Value &V) {
+JOS.attribute("value_id", llvm::to_string(&V));
+if (!Visited.insert(&V).second)
+  return;
+
+JOS.attribute("kind", debugString(V.getKind()));
+
+switch (V.getKind()) {
+case Value::Kind::Integer:
+case Value::Kind::TopBool:
+case Value::Kind::AtomicBool:
+  break;
+case Value::Kind::Reference:
+  JOS.attributeObject(
+  "referent", [&] { dump(cast(V).getReferentLoc()); });
+  break;
+case Value::Kind::Pointer:
+  JOS.attributeObject(
+  "pointee", [&] { dump(cast(V).getPointeeLoc()); });
+  break;
+case Value::Kind::Struct:
+  for (const auto &Child : cast(V).children())
+JOS.attributeObject("f:" + Child.first->getNameAsString(),
+[&] { dump(*Child.second); });
+  break;
+case Value::Kind::Disjunction: {
+  auto &VV = cast(V);
+  JOS.attributeObject("lhs", [&] { dump(VV.getLeftSubValue()); });
+  JOS.attributeObject("rhs", [&] { dump(VV.getRightSub

[PATCH] D146987: [Assignment Tracking] Enable by default

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

@jmorse Sorry it took me a bit to set up, but I ran an experiment in our CI 
that reverted the change that disabled this as default and tried it for x86. It 
looks like this patch is working OK for us now: 
https://ci.chromium.org/raw/build/logs.chromium.org/fuchsia/led/paulkirth_google.com/df38ff794f44284ff34f63e5dc1f1d41b25225b1241c14eff15a9f0a4b189afb/+/build.proto?server=chromium-swarm.appspot.com

I'm reasonably confident that should mean you won't run into issues. The caveat 
here is that I can't easily launch experiments across all combinations of 
platforms and hardware, so it's possible there may still be an issue. I think 
that's unlikely given how this has been manifesting, so I'd say you're good to 
try again in our perspective.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146987

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

[PATCH] D149206: [clang][driver] Enable MisExpect diagnostics flag outside of CC1

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added inline comments.



Comment at: clang/test/Profile/misexpect-branch.c:10
 // RUN: %clang_cc1 %s -O2 -o - -emit-llvm 
-fprofile-instrument-use-path=%t.profdata -verify=foo 
-fdiagnostics-misexpect-tolerance=10 -Wmisexpect 
-debug-info-kind=line-tables-only
+// RUN: %clang -c -S %s -O2 -o - -emit-llvm -fprofile-instr-use=%t.profdata 
-Xclang -verify=foo -fdiagnostics-misexpect-tolerance=10 -Wmisexpect 
-gline-tables-only
 // RUN: %clang_cc1 %s -O2 -o - -disable-llvm-passes -emit-llvm 
-fprofile-instrument-use-path=%t.profdata -verify=foo

hans wrote:
> The more common way to test this would be a test under clang/test/Driver/ 
> that just checks that forwarding the flag works.
Good point, I just defaulted to adding the RUN line here, but that's definitely 
the right way to test this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149206

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

[PATCH] D148767: Restore CodeGen/LowLevelType

[NAKAMURA Takumi via Phabricator via cfe-commits]

chapuni updated this revision to Diff 517193.
chapuni added a comment.

- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148767

Files:
  clang/lib/CodeGen/CMakeLists.txt
  llvm/include/llvm/CodeGen/GlobalISel/CallLowering.h
  llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
  llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h
  llvm/include/llvm/CodeGen/GlobalISel/LegacyLegalizerInfo.h
  llvm/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h
  llvm/include/llvm/CodeGen/GlobalISel/Utils.h
  llvm/include/llvm/CodeGen/LowLevelType.h
  llvm/include/llvm/CodeGen/LowLevelTypeUtils.h
  llvm/include/llvm/CodeGen/MachineMemOperand.h
  llvm/include/llvm/CodeGen/RegisterBankInfo.h
  llvm/include/llvm/Support/LowLevelTypeImpl.h
  llvm/include/llvm/module.modulemap
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
  llvm/lib/CodeGen/GlobalISel/LegalizerInfo.cpp
  llvm/lib/CodeGen/LowLevelType.cpp
  llvm/lib/CodeGen/MIRParser/MIParser.cpp
  llvm/lib/CodeGen/MIRPrinter.cpp
  llvm/lib/CodeGen/MachineInstr.cpp
  llvm/lib/CodeGen/MachineVerifier.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/LowLevelType.cpp
  llvm/lib/Target/AArch64/AsmParser/CMakeLists.txt
  llvm/lib/Target/AArch64/MCTargetDesc/CMakeLists.txt
  llvm/lib/Target/AMDGPU/AMDGPUGlobalISelUtils.cpp
  llvm/lib/Target/AMDGPU/AsmParser/CMakeLists.txt
  llvm/lib/Target/AMDGPU/Disassembler/CMakeLists.txt
  llvm/lib/Target/AMDGPU/MCA/CMakeLists.txt
  llvm/lib/Target/AMDGPU/MCTargetDesc/CMakeLists.txt
  llvm/lib/Target/AMDGPU/Utils/CMakeLists.txt
  llvm/lib/Target/ARC/Disassembler/CMakeLists.txt
  llvm/lib/Target/ARM/ARMCallLowering.cpp
  llvm/lib/Target/ARM/AsmParser/CMakeLists.txt
  llvm/lib/Target/ARM/Disassembler/CMakeLists.txt
  llvm/lib/Target/ARM/MCTargetDesc/CMakeLists.txt
  llvm/lib/Target/AVR/AsmParser/CMakeLists.txt
  llvm/lib/Target/AVR/Disassembler/CMakeLists.txt
  llvm/lib/Target/CSKY/MCTargetDesc/CMakeLists.txt
  llvm/lib/Target/Lanai/AsmParser/CMakeLists.txt
  llvm/lib/Target/Lanai/Disassembler/CMakeLists.txt
  llvm/lib/Target/M68k/AsmParser/CMakeLists.txt
  llvm/lib/Target/M68k/Disassembler/CMakeLists.txt
  llvm/lib/Target/MSP430/AsmParser/CMakeLists.txt
  llvm/lib/Target/Mips/MCTargetDesc/CMakeLists.txt
  llvm/lib/Target/Mips/MCTargetDesc/MipsABIInfo.cpp
  llvm/lib/Target/PowerPC/MCTargetDesc/CMakeLists.txt
  llvm/lib/Target/RISCV/MCA/CMakeLists.txt
  llvm/lib/Target/SPIRV/MCTargetDesc/CMakeLists.txt
  llvm/lib/Target/SystemZ/MCTargetDesc/CMakeLists.txt
  llvm/lib/Target/VE/AsmParser/CMakeLists.txt
  llvm/lib/Target/VE/Disassembler/CMakeLists.txt
  llvm/lib/Target/VE/MCTargetDesc/CMakeLists.txt
  llvm/lib/Target/WebAssembly/AsmParser/CMakeLists.txt
  llvm/lib/Target/WebAssembly/Disassembler/CMakeLists.txt
  llvm/lib/Target/WebAssembly/MCTargetDesc/CMakeLists.txt
  llvm/lib/Target/X86/MCA/CMakeLists.txt
  llvm/lib/Target/X86/MCTargetDesc/CMakeLists.txt
  llvm/lib/Target/X86/X86CallLowering.cpp
  llvm/lib/Target/X86/X86InstructionSelector.cpp
  llvm/lib/Target/XCore/Disassembler/CMakeLists.txt
  llvm/tools/llvm-dwarfutil/CMakeLists.txt
  llvm/tools/llvm-exegesis/CMakeLists.txt
  llvm/tools/llvm-exegesis/lib/AArch64/CMakeLists.txt
  llvm/tools/llvm-exegesis/lib/Mips/CMakeLists.txt
  llvm/tools/llvm-exegesis/lib/PowerPC/CMakeLists.txt
  llvm/unittests/CodeGen/MachineOperandTest.cpp
  llvm/unittests/DebugInfo/DWARF/CMakeLists.txt
  llvm/utils/TableGen/CMakeLists.txt
  llvm/utils/TableGen/GlobalISel/CMakeLists.txt
  llvm/utils/TableGen/GlobalISelEmitter.cpp
  utils/bazel/llvm-project-overlay/llvm/BUILD.bazel

Index: utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
===
--- utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
+++ utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
@@ -616,6 +616,7 @@
 features = ["-header_modules"],
 strip_include_prefix = "utils/TableGen",
 deps = [
+":CodeGen",
 ":Support",
 ":TableGen",
 ":config",
@@ -642,11 +643,11 @@
 copts = llvm_copts,
 stamp = 0,
 deps = [
+":CodeGen",
 ":Support",
 ":TableGen",
 ":TableGenGlobalISel",
 ":config",
-":intrinsic_enums_gen",
 ":llvm-tblgen-headers",
 ],
 )
@@ -2336,6 +2337,7 @@
 copts = llvm_copts,
 features = ["-layering_check"],
 deps = [
+":CodeGen",
 ":MC",
 ":MCA",
 ":MCParser",
@@ -3404,6 +3406,7 @@
 deps = [
 ":AllTargetsAsmParsers",
 ":AllTargetsCodeGens",
+":CodeGen",
 ":DWARFLinker",
 ":DebugInfoDWARF",
 ":DwarfutilOptionsTableGen",
@@ -3444,6 +3447,7 @@
 ":AllTargetsAsmParsers",
 ":AllTargetsCodeGens",
 ":AllTargetsDisassemblers",
+":CodeGen",
 ":Exegesis",
 ":MC",
 ":MCParser",
In

[PATCH] D149158: [clang][analyzer] Cleanup tests of StdCLibraryFunctionsChecker (NFC)

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 517196.
balazske added a comment.

using `__restrict` instead of `restrict`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149158

Files:
  clang/test/Analysis/Inputs/std-c-library-functions-POSIX.h
  clang/test/Analysis/Inputs/std-c-library-functions.h
  clang/test/Analysis/std-c-library-functions-POSIX.c
  clang/test/Analysis/std-c-library-functions-arg-constraints.c
  clang/test/Analysis/std-c-library-functions.c

Index: clang/test/Analysis/std-c-library-functions.c
===
--- clang/test/Analysis/std-c-library-functions.c
+++ clang/test/Analysis/std-c-library-functions.c
@@ -50,6 +50,9 @@
 // CHECK-NEXT: Loaded summary for: int isspace(int)
 // CHECK-NEXT: Loaded summary for: int isupper(int)
 // CHECK-NEXT: Loaded summary for: int isxdigit(int)
+// CHECK-NEXT: Loaded summary for: int toupper(int)
+// CHECK-NEXT: Loaded summary for: int tolower(int)
+// CHECK-NEXT: Loaded summary for: int toascii(int)
 // CHECK-NEXT: Loaded summary for: int getc(FILE *)
 // CHECK-NEXT: Loaded summary for: int fgetc(FILE *)
 // CHECK-NEXT: Loaded summary for: int getchar(void)
@@ -59,16 +62,14 @@
 // CHECK-NEXT: Loaded summary for: ssize_t write(int, const void *, size_t)
 // CHECK-NEXT: Loaded summary for: ssize_t getline(char **restrict, size_t *restrict, FILE *restrict)
 // CHECK-NEXT: Loaded summary for: ssize_t getdelim(char **restrict, size_t *restrict, int, FILE *restrict)
+// CHECK-NEXT: Loaded summary for: char *getenv(const char *)
 
+#include "Inputs/std-c-library-functions.h"
 
 void clang_analyzer_eval(int);
 
 int glob;
 
-typedef struct FILE FILE;
-#define EOF -1
-
-int getc(FILE *);
 void test_getc(FILE *fp) {
   int x;
   while ((x = getc(fp)) != EOF) {
@@ -77,17 +78,11 @@
   }
 }
 
-int fgetc(FILE *);
 void test_fgets(FILE *fp) {
   clang_analyzer_eval(fgetc(fp) < 256); // expected-warning{{TRUE}}
   clang_analyzer_eval(fgetc(fp) >= 0); // expected-warning{{UNKNOWN}}
 }
 
-
-typedef typeof(sizeof(int)) size_t;
-typedef signed long ssize_t;
-ssize_t read(int, void *, size_t);
-ssize_t write(int, const void *, size_t);
 void test_read_write(int fd, char *buf) {
   glob = 1;
   ssize_t x = write(fd, buf, 10);
@@ -106,8 +101,6 @@
   }
 }
 
-size_t fread(void *restrict, size_t, size_t, FILE *restrict);
-size_t fwrite(const void *restrict, size_t, size_t, FILE *restrict);
 void test_fread_fwrite(FILE *fp, int *buf) {
 
   size_t x = fwrite(buf, sizeof(int), 10, fp);
@@ -128,8 +121,6 @@
   (void)fread(ptr, sz, nmem, fp); // expected-warning {{1st function call argument is an uninitialized value}}
 }
 
-ssize_t getline(char **restrict, size_t *restrict, FILE *restrict);
-ssize_t getdelim(char **restrict, size_t *restrict, int, FILE *restrict);
 void test_getline(FILE *fp) {
   char *line = 0;
   size_t n = 0;
@@ -139,7 +130,6 @@
   }
 }
 
-int isascii(int);
 void test_isascii(int x) {
   clang_analyzer_eval(isascii(123)); // expected-warning{{TRUE}}
   clang_analyzer_eval(isascii(-1)); // expected-warning{{FALSE}}
@@ -157,7 +147,6 @@
   clang_analyzer_eval(glob); // expected-warning{{TRUE}}
 }
 
-int islower(int);
 void test_islower(int x) {
   clang_analyzer_eval(islower('x')); // expected-warning{{TRUE}}
   clang_analyzer_eval(islower('X')); // expected-warning{{FALSE}}
@@ -165,7 +154,6 @@
 clang_analyzer_eval(x < 'a'); // expected-warning{{FALSE}}
 }
 
-int getchar(void);
 void test_getchar(void) {
   int x = getchar();
   if (x == EOF)
@@ -174,27 +162,23 @@
   clang_analyzer_eval(x < 256); // expected-warning{{TRUE}}
 }
 
-int isalpha(int);
 void test_isalpha(void) {
   clang_analyzer_eval(isalpha(']')); // expected-warning{{FALSE}}
   clang_analyzer_eval(isalpha('Q')); // expected-warning{{TRUE}}
   clang_analyzer_eval(isalpha(128)); // expected-warning{{UNKNOWN}}
 }
 
-int isalnum(int);
 void test_alnum(void) {
   clang_analyzer_eval(isalnum('1')); // expected-warning{{TRUE}}
   clang_analyzer_eval(isalnum(')')); // expected-warning{{FALSE}}
 }
 
-int isblank(int);
 void test_isblank(void) {
   clang_analyzer_eval(isblank('\t')); // expected-warning{{TRUE}}
   clang_analyzer_eval(isblank(' ')); // expected-warning{{TRUE}}
   clang_analyzer_eval(isblank('\n')); // expected-warning{{FALSE}}
 }
 
-int ispunct(int);
 void test_ispunct(int x) {
   clang_analyzer_eval(ispunct(' ')); // expected-warning{{FALSE}}
   clang_analyzer_eval(ispunct(-1)); // expected-warning{{FALSE}}
@@ -204,21 +188,17 @@
 clang_analyzer_eval(x < 127); // expected-warning{{TRUE}}
 }
 
-int isupper(int);
 void test_isupper(int x) {
   if (isupper(x))
 clang_analyzer_eval(x < 'A'); // expected-warning{{FALSE}}
 }
 
-int isgraph(int);
-int isprint(int);
 void test_isgraph_isprint(int x) {
   char y = x;
   if (isgraph(y))
 clang_analyzer_eval(isprint(x)); // expected-warning{{TRUE}}
 }
 
-int isdigit(int);
 void test_mixed_branches(int x) 

[PATCH] D149158: [clang][analyzer] Cleanup tests of StdCLibraryFunctionsChecker (NFC)

[Balázs Kéri via Phabricator via cfe-commits]

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



Comment at: clang/test/Analysis/Inputs/std-c-library-functions.h:1-2
+typedef typeof(sizeof(int)) size_t;
+typedef signed long ssize_t;
+typedef struct {

steakhal wrote:
> `ssize_t`'s size should match the size of `size_t`. In this implementation, 
> it would be true only if `size_t` is `long`.
> 
I could not find a working way of defining the type in that way (there is no 
`__sizte_t`). The current definition should work well in the test code, the 
property of being the same size is supposedly not used in the tests. The 
previous definition was not better than this (and different in different 
places).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149158

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

[PATCH] D149206: [clang][driver] Enable MisExpect diagnostics flag outside of CC1

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 517199.
paulkirth added a comment.

Migrate to a Driver test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149206

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/clang_f_opts.c


Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -140,6 +140,9 @@
 // CHECK-PROFILE-USE-DIR: 
"-fprofile-instrument-use-path={{.*}}.d/some/dir{{/|}}default.profdata"
 // CHECK-PROFILE-USE-FILE: "-fprofile-instrument-use-path=/tmp/somefile.prof"
 
+// RUN: %clang -### -S -fprofile-instr-use=%t.profdata 
-fdiagnostics-misexpect-tolerance=10 -Wmisexpect %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-MISEXPECT-TOLLERANCE
+// CHECK-MISEXPECT-TOLLERANCE-NOT: argument unused 
{{.*}}-fdiagnostics-misexpect-tolerance=
+
 // RUN: %clang -### -S -fvectorize %s 2>&1 | FileCheck 
-check-prefix=CHECK-VECTORIZE %s
 // RUN: %clang -### -S -fno-vectorize -fvectorize %s 2>&1 | FileCheck 
-check-prefix=CHECK-VECTORIZE %s
 // RUN: %clang -### -S -fno-vectorize %s 2>&1 | FileCheck 
-check-prefix=CHECK-NO-VECTORIZE %s
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4027,6 +4027,13 @@
 CmdArgs.push_back(Args.MakeArgString(Opt));
   }
 
+  if (const Arg *A =
+  Args.getLastArg(options::OPT_fdiagnostics_misexpect_tolerance_EQ)) {
+std::string Opt =
+std::string("-fdiagnostics-misexpect-tolerance=") + A->getValue();
+CmdArgs.push_back(Args.MakeArgString(Opt));
+  }
+
   if (const Arg *A = Args.getLastArg(options::OPT_fdiagnostics_format_EQ)) {
 CmdArgs.push_back("-fdiagnostics-format");
 CmdArgs.push_back(A->getValue());


Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -140,6 +140,9 @@
 // CHECK-PROFILE-USE-DIR: "-fprofile-instrument-use-path={{.*}}.d/some/dir{{/|}}default.profdata"
 // CHECK-PROFILE-USE-FILE: "-fprofile-instrument-use-path=/tmp/somefile.prof"
 
+// RUN: %clang -### -S -fprofile-instr-use=%t.profdata -fdiagnostics-misexpect-tolerance=10 -Wmisexpect %s 2>&1 | FileCheck %s --check-prefix=CHECK-MISEXPECT-TOLLERANCE
+// CHECK-MISEXPECT-TOLLERANCE-NOT: argument unused {{.*}}-fdiagnostics-misexpect-tolerance=
+
 // RUN: %clang -### -S -fvectorize %s 2>&1 | FileCheck -check-prefix=CHECK-VECTORIZE %s
 // RUN: %clang -### -S -fno-vectorize -fvectorize %s 2>&1 | FileCheck -check-prefix=CHECK-VECTORIZE %s
 // RUN: %clang -### -S -fno-vectorize %s 2>&1 | FileCheck -check-prefix=CHECK-NO-VECTORIZE %s
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4027,6 +4027,13 @@
 CmdArgs.push_back(Args.MakeArgString(Opt));
   }
 
+  if (const Arg *A =
+  Args.getLastArg(options::OPT_fdiagnostics_misexpect_tolerance_EQ)) {
+std::string Opt =
+std::string("-fdiagnostics-misexpect-tolerance=") + A->getValue();
+CmdArgs.push_back(Args.MakeArgString(Opt));
+  }
+
   if (const Arg *A = Args.getLastArg(options::OPT_fdiagnostics_format_EQ)) {
 CmdArgs.push_back("-fdiagnostics-format");
 CmdArgs.push_back(A->getValue());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 22a748a - [Headers] Revise conditional for rdrand64_step

[Paul Robinson via cfe-commits]

Author: Paul Robinson
Date: 2023-04-26T09:00:29-07:00
New Revision: 22a748a51f2a2644f4dbc5e93615c9f3106e50d5

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

LOG: [Headers] Revise conditional for rdrand64_step

Downstream doc tooling doesn't like an #if between the doc and the
function prototype. This change also guarantees that the prototype
stays the same for 32/64 bit users.

Added: 


Modified: 
clang/lib/Headers/immintrin.h

Removed: 




diff  --git a/clang/lib/Headers/immintrin.h b/clang/lib/Headers/immintrin.h
index dff4da2465d2..c5f84ae0286b 100644
--- a/clang/lib/Headers/immintrin.h
+++ b/clang/lib/Headers/immintrin.h
@@ -323,18 +323,14 @@ _rdrand32_step(unsigned int *__p)
 /// \param __p
 ///A pointer to a 64-bit memory location to place the random value.
 /// \returns 1 if the value was successfully generated, 0 otherwise.
-#ifdef __x86_64__
 static __inline__ int __attribute__((__always_inline__, __nodebug__, 
__target__("rdrnd")))
 _rdrand64_step(unsigned long long *__p)
 {
+#ifdef __x86_64__
   return (int)__builtin_ia32_rdrand64_step(__p);
-}
 #else
-// We need to emulate the functionality of 64-bit rdrand with 2 32-bit
-// rdrand instructions.
-static __inline__ int __attribute__((__always_inline__, __nodebug__, 
__target__("rdrnd")))
-_rdrand64_step(unsigned long long *__p)
-{
+  // We need to emulate the functionality of 64-bit rdrand with 2 32-bit
+  // rdrand instructions.
   unsigned int __lo, __hi;
   unsigned int __res_lo = __builtin_ia32_rdrand32_step(&__lo);
   unsigned int __res_hi = __builtin_ia32_rdrand32_step(&__hi);
@@ -345,8 +341,8 @@ _rdrand64_step(unsigned long long *__p)
 *__p = 0;
 return 0;
   }
-}
 #endif
+}
 #endif /* __RDRND__ */
 
 #if !(defined(_MSC_VER) || defined(__SCE__)) || __has_feature(modules) ||  
\



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

[PATCH] D148439: [clang-rename] Exit gracefully when no input provided

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

Thanks Aaron!

Yes, fair enough, that looks good to me, probably no need to move a sanity 
check to the front of `main()` in this case.

Also, just as a note: I don't know how many people use `clang-rename` anymore 
(especially with `clangd` being present), I rewrote a lot of what clang-rename 
has to offer for `clangd` with better precision and results. I think the tool 
is still useful, but not as much as before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148439

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

[PATCH] D148330: [clang] Do not crash on undefined template partial specialization

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik accepted this revision.
shafik added a comment.

LGTM




Comment at: clang/lib/Sema/SemaCXXScopeSpec.cpp:134
  "specifier in SFINAE context?");
-if (!hasReachableDefinition(PartialSpec))
+if (PartialSpec->hasDefinition() &&
+!hasReachableDefinition(PartialSpec))

Fznamznon wrote:
> Fznamznon wrote:
> > shafik wrote:
> > > So I see in another location we are basically checking 
> > > `!isBeginDefined()` and in another place `hasCompleteDefinition()`. It is 
> > > not clear to me if these are all basically checking the same thing or if 
> > > we should figure out what is the right thing to check and do that 
> > > everywhere. 
> > I suppose you meant `isBeingDefined()`. So, IMO `isBeingDefined()` is not 
> > exactly the same as having a definition, this is set to `true` when 
> > `TagDecl::startDefinition` I suppose this should be done when parser meets 
> > a definition so at this point it can't be or have a complete definition. 
> > But at the places where `!isBeingDefined()` is checked prior 
> > `hasReachableDefinition` I saw a pointer representing a definition checked 
> > for not being null. Interestingly, `hasReachableDefinition()` early exits 
> > if `isBeingDefined()` returns true, so probably this check additional 
> > doesn't make sense at all.
> > Maybe we should just move both checks on having a definition and not being 
> > in the process of definition under `hasReachableDefinition` after all. That 
> > will require changing unrelated places, so I would prefer doing this 
> > separately in any case.
> > 
> > I can't grep `hasCompleteDefinition()` either, so I suppose you meant 
> > `isCompleteDefinition()`, I think this is basically the same thing as 
> > having definition and additionally the declaration through which the method 
> > called is checked that IT IS the definition. I'm not sure we have to 
> > require it here.
> > 
> @shafik , are you satisfied with the explanation?
I think we are good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148330

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