[PATCH] D158967: [clangd] Record the stack bottom before building AST

[Younan Zhang via Phabricator via cfe-commits]

zyounan created this revision.
zyounan added reviewers: sammccall, nridge.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
zyounan requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

`clang::runWithSufficientStackSpace` requires the address of the
initial stack bottom to prevent potential stack overflows.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158967

Files:
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/test/infinite-instantiation.test


Index: clang-tools-extra/clangd/test/infinite-instantiation.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/infinite-instantiation.test
@@ -0,0 +1,13 @@
+// RUN: cp %s %t.cpp
+// RUN: not clangd -check=%t.cpp 2>&1 | FileCheck -strict-whitespace %s
+
+// CHECK: [template_recursion_depth_exceeded]
+
+template 
+constexpr int f(T... args) {
+  return f(0, args...);
+}
+
+int main() {
+  auto i = f();
+}
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -44,6 +44,7 @@
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Stack.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
@@ -388,6 +389,7 @@
  std::unique_ptr CI,
  llvm::ArrayRef CompilerInvocationDiags,
  std::shared_ptr Preamble) {
+  clang::noteBottomOfStack();
   trace::Span Tracer("BuildAST");
   SPAN_ATTACH(Tracer, "File", Filename);
 


Index: clang-tools-extra/clangd/test/infinite-instantiation.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/infinite-instantiation.test
@@ -0,0 +1,13 @@
+// RUN: cp %s %t.cpp
+// RUN: not clangd -check=%t.cpp 2>&1 | FileCheck -strict-whitespace %s
+
+// CHECK: [template_recursion_depth_exceeded]
+
+template 
+constexpr int f(T... args) {
+  return f(0, args...);
+}
+
+int main() {
+  auto i = f();
+}
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -44,6 +44,7 @@
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Stack.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
@@ -388,6 +389,7 @@
  std::unique_ptr CI,
  llvm::ArrayRef CompilerInvocationDiags,
  std::shared_ptr Preamble) {
+  clang::noteBottomOfStack();
   trace::Span Tracer("BuildAST");
   SPAN_ATTACH(Tracer, "File", Filename);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 779353e - [Driver] Improve legibility of ld -z options on Solaris

[Rainer Orth via cfe-commits]

Author: Rainer Orth
Date: 2023-08-28T09:25:47+02:00
New Revision: 779353e576eaa6ababdfc242784484f3656ecfe0

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

LOG: [Driver] Improve legibility of ld -z options on Solaris

Following the lead of the Linux code, this patch passes the `ld -z` options
as two separate args on Solaris, improving legibility.  For lack of a
variadic `std::push_back`, `getAsNeededOption` had to be changed to
`addAsNeededOption`, matching other `add*Options` functions, changing
callers accordingly.  The additional args are also used in a WIP revision
of the Solaris GNU ld patch D85309 , which
will allow runtime selection of the linker to use.

Tested on `amd64-pc-solaris2.11` and `x86_64-pc-linux-gnu`.

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/lib/Driver/ToolChains/CommonArgs.h
clang/lib/Driver/ToolChains/FreeBSD.cpp
clang/lib/Driver/ToolChains/Gnu.cpp
clang/lib/Driver/ToolChains/Hexagon.cpp
clang/lib/Driver/ToolChains/NetBSD.cpp
clang/lib/Driver/ToolChains/OpenBSD.cpp
clang/lib/Driver/ToolChains/Solaris.cpp
clang/test/Driver/solaris-ld-sanitizer.c
clang/test/Driver/solaris-ld.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp 
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 3b99389fdbf228..813f80223bd830 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -999,24 +999,30 @@ static bool addSanitizerDynamicList(const ToolChain &TC, 
const ArgList &Args,
   return false;
 }
 
-const char *tools::getAsNeededOption(const ToolChain &TC, bool as_needed) {
+void tools::addAsNeededOption(const ToolChain &TC,
+  const llvm::opt::ArgList &Args,
+  llvm::opt::ArgStringList &CmdArgs,
+  bool as_needed) {
   assert(!TC.getTriple().isOSAIX() &&
  "AIX linker does not support any form of --as-needed option yet.");
 
   // While the Solaris 11.2 ld added --as-needed/--no-as-needed as aliases
   // for the native forms -z ignore/-z record, they are missing in Illumos,
   // so always use the native form.
-  if (TC.getTriple().isOSSolaris())
-return as_needed ? "-zignore" : "-zrecord";
-  else
-return as_needed ? "--as-needed" : "--no-as-needed";
+  if (TC.getTriple().isOSSolaris()) {
+CmdArgs.push_back("-z");
+CmdArgs.push_back(as_needed ? "ignore" : "record");
+  } else {
+CmdArgs.push_back(as_needed ? "--as-needed" : "--no-as-needed");
+  }
 }
 
 void tools::linkSanitizerRuntimeDeps(const ToolChain &TC,
+ const llvm::opt::ArgList &Args,
  ArgStringList &CmdArgs) {
   // Force linking against the system libraries sanitizers depends on
   // (see PR15823 why this is necessary).
-  CmdArgs.push_back(getAsNeededOption(TC, false));
+  addAsNeededOption(TC, Args, CmdArgs, false);
   // There's no libpthread or librt on RTEMS & Android.
   if (TC.getTriple().getOS() != llvm::Triple::RTEMS &&
   !TC.getTriple().isAndroid() && !TC.getTriple().isOHOSFamily()) {
@@ -1260,8 +1266,10 @@ bool tools::addXRayRuntime(const ToolChain&TC, const 
ArgList &Args, ArgStringLis
   return false;
 }
 
-void tools::linkXRayRuntimeDeps(const ToolChain &TC, ArgStringList &CmdArgs) {
-  CmdArgs.push_back(getAsNeededOption(TC, false));
+void tools::linkXRayRuntimeDeps(const ToolChain &TC,
+const llvm::opt::ArgList &Args,
+ArgStringList &CmdArgs) {
+  addAsNeededOption(TC, Args, CmdArgs, false);
   CmdArgs.push_back("-lpthread");
   if (!TC.getTriple().isOSOpenBSD())
 CmdArgs.push_back("-lrt");
@@ -1805,7 +1813,7 @@ static void AddUnwindLibrary(const ToolChain &TC, const 
Driver &D,
   !TC.getTriple().isAndroid() &&
   !TC.getTriple().isOSCygMing() && !TC.getTriple().isOSAIX();
   if (AsNeeded)
-CmdArgs.push_back(getAsNeededOption(TC, true));
+addAsNeededOption(TC, Args, CmdArgs, true);
 
   switch (UNW) {
   case ToolChain::UNW_None:
@@ -1839,7 +1847,7 @@ static void AddUnwindLibrary(const ToolChain &TC, const 
Driver &D,
   }
 
   if (AsNeeded)
-CmdArgs.push_back(getAsNeededOption(TC, false));
+addAsNeededOption(TC, Args, CmdArgs, false);
 }
 
 static void AddLibgcc(const ToolChain &TC, const Driver &D,

diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.h 
b/clang/lib/Driver/ToolChains/CommonArgs.h
index b1c8441226310c..7585d24c3c0eb6 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.h
+++ b/clang/lib/Driver/ToolChains/CommonArgs.h
@@ -40,12 +40,13 @@ bool addSanitiz

[PATCH] D158955: [Driver] Improve legibility of ld -z options on Solaris

[Rainer Orth 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 rG779353e576ea: [Driver] Improve legibility of ld -z options 
on Solaris (authored by ro).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158955

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Hexagon.cpp
  clang/lib/Driver/ToolChains/NetBSD.cpp
  clang/lib/Driver/ToolChains/OpenBSD.cpp
  clang/lib/Driver/ToolChains/Solaris.cpp
  clang/test/Driver/solaris-ld-sanitizer.c
  clang/test/Driver/solaris-ld.c

Index: clang/test/Driver/solaris-ld.c
===
--- clang/test/Driver/solaris-ld.c
+++ clang/test/Driver/solaris-ld.c
@@ -16,7 +16,7 @@
 // CHECK-LD-SPARC32-SAME: "-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2"
 // CHECK-LD-SPARC32-SAME: "-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/../../.."
 // CHECK-LD-SPARC32-SAME: "-L[[SYSROOT]]/usr/lib"
-// CHECK-LD-SPARC32-SAME: "-zignore" "-latomic" "-zrecord"
+// CHECK-LD-SPARC32-SAME: "-z" "ignore" "-latomic" "-z" "record"
 // CHECK-LD-SPARC32-SAME: "-lgcc_s"
 // CHECK-LD-SPARC32-SAME: "-lc"
 // CHECK-LD-SPARC32-SAME: "-lgcc"
Index: clang/test/Driver/solaris-ld-sanitizer.c
===
--- clang/test/Driver/solaris-ld-sanitizer.c
+++ clang/test/Driver/solaris-ld-sanitizer.c
@@ -6,49 +6,49 @@
 // RUN: %clang --target=sparc-sun-solaris2.11 %s -### 2>&1 \
 // RUN: --gcc-toolchain="" --sysroot=%S/Inputs/solaris_sparc_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-LD-SPARC32 %s
-// CHECK-LD-SPARC32-NOT: -zrelax=transtls
+// CHECK-LD-SPARC32-NOT: "-z" "relax=transtls"
 
 /// Check sparc-sun-solaris2.11, 32bit
 // RUN: %clang -fsanitize=undefined --target=sparc-sun-solaris2.11 %s -### 2>&1 \
 // RUN: --gcc-toolchain="" --sysroot=%S/Inputs/solaris_sparc_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-LD-SPARC32 %s
-// CHECK-LD-SPARC32-NOT: -zrelax=transtls
+// CHECK-LD-SPARC32-NOT: "-z" "relax=transtls"
 
 /// Check sparc-sun-solaris2.11, 64bit
 // RUN: %clang -m64 --target=sparc-sun-solaris2.11 %s -### 2>&1 \
 // RUN: --gcc-toolchain="" --sysroot=%S/Inputs/solaris_sparc_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-LD-SPARC64 %s
-// CHECK-LD-SPARC64-NOT: -zrelax=transtls
+// CHECK-LD-SPARC64-NOT: "-z" "relax=transtls"
 
 /// Check sparc-sun-solaris2.11, 64bit
 // RUN: %clang -m64 -fsanitize=undefined --target=sparc-sun-solaris2.11 %s -### 2>&1 \
 // RUN: --gcc-toolchain="" --sysroot=%S/Inputs/solaris_sparc_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-LD-SPARC64 %s
-// CHECK-LD-SPARC64-NOT: -zrelax=transtls
+// CHECK-LD-SPARC64-NOT: "-z" "relax=transtls"
 
 /// Check i386-pc-solaris2.11, 32bit
 // RUN: %clang --target=i386-pc-solaris2.11 %s -### 2>&1 \
 // RUN: --gcc-toolchain="" --sysroot=%S/Inputs/solaris_x86_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-LD-X32 %s
-// CHECK-LD-X32-NOT: -zrelax=transtls
+// CHECK-LD-X32-NOT: "-z" "relax=transtls"
 
 /// Check i386-pc-solaris2.11, 32bit
 // RUN: %clang -fsanitize=undefined --target=i386-pc-solaris2.11 %s -### 2>&1 \
 // RUN: --gcc-toolchain="" --sysroot=%S/Inputs/solaris_x86_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-LD-X32 %s
-// CHECK-LD-X32-NOT: -zrelax=transtls
+// CHECK-LD-X32-NOT: "-z" "relax=transtls"
 
 /// Check i386-pc-solaris2.11, 64bit
 // RUN: %clang -m64 --target=i386-pc-solaris2.11 %s -### 2>&1 \
 // RUN: --gcc-toolchain="" --sysroot=%S/Inputs/solaris_x86_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-LD-X64 %s
-// CHECK-LD-X64-NOT: -zrelax=transtls
+// CHECK-LD-X64-NOT: "-z" "relax=transtls"
 
 /// Check i386-pc-solaris2.11, 64bit
 // RUN: %clang -m64 -fsanitize=undefined --target=i386-pc-solaris2.11 %s -### 2>&1 \
 // RUN: --gcc-toolchain="" --sysroot=%S/Inputs/solaris_x86_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-LD-X64-UBSAN %s
-// CHECK-LD-X64-UBSAN: -zrelax=transtls
+// CHECK-LD-X64-UBSAN: "-z" "relax=transtls"
 
 /// General tests that the ld -z now workaround is only applied on
 /// Solaris/i386 with shared libclang_rt.asan.. Note that we use sysroot to
@@ -58,10 +58,10 @@
 // RUN: %clang -fsanitize=address -shared-libasan --target=i386-pc-solaris2.11 %s -### 2>&1 \
 // RUN: --gcc-toolchain="" --sysroot=%S/Inputs/solaris_x86_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-LD-X32-ASAN-SHARED %s
-// CHECK-LD-X32-ASAN-SHARED: -znow
+// CHECK-LD-X32-ASAN-SHARED: "-z" "now"
 
 /// Check i386-pc-solaris2.11, 32bit, static libclang_rt.asan
 // RUN: %clang -fsanitize=address --target=i386-pc-solaris2.11 %s -### 2>&1 \
 // RUN: --gcc-toolchain="" --sysroot=%S/Inputs/solaris_x86_tree \
 // RUN:   

[clang-tools-extra] ab090e9 - [include-cleaner] Make handling of enum constants similar to members

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2023-08-28T09:31:58+02:00
New Revision: ab090e9e49ff85d031a263abf327e9e436ce3873

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

LOG: [include-cleaner] Make handling of enum constants similar to members

We were treating enum constants more like regular decls, which results
in ignoring type aliases/exports.
This patch brings the handling to be closer to member-like decls, with
one caveat. When we encounter reference to an enum constant we still
report an explicit reference to the particular enum constant, as
otherwise we might not see any references to the enum itself.

Also drops implicit references from qualified names to containers, as
we already have explicit references from the qualifier to relevant
container.

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

Added: 


Modified: 
clang-tools-extra/include-cleaner/lib/WalkAST.cpp
clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp

Removed: 




diff  --git a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp 
b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
index febdf19e695cd9..e3ba2d3b604072 100644
--- a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -126,13 +126,22 @@ class ASTWalker : public RecursiveASTVisitor {
   }
 
   bool VisitDeclRefExpr(DeclRefExpr *DRE) {
-// Static class members are handled here, as they don't produce 
MemberExprs.
-if (DRE->getFoundDecl()->isCXXClassMember()) {
-  if (auto *Qual = DRE->getQualifier())
-report(DRE->getLocation(), Qual->getAsRecordDecl(), RefType::Implicit);
-} else {
-  report(DRE->getLocation(), DRE->getFoundDecl());
+auto *FD = DRE->getFoundDecl();
+// For refs to non-meber-like decls, use the found decl.
+// For member-like decls, we should have a reference from the qualifier to
+// the container decl instead, which is preferred as it'll handle
+// aliases/exports properly.
+if (!FD->isCXXClassMember() && !llvm::isa(FD)) {
+  report(DRE->getLocation(), FD);
+  return true;
 }
+// If the ref is without a qualifier, and is a member, ignore it. As it is
+// available in current context due to some other construct (e.g. base
+// specifiers, using decls) that has to spell the name explicitly.
+// If it's an enum constant, it must be due to prior decl. Report 
references
+// to it instead.
+if (llvm::isa(FD) && !DRE->hasQualifier())
+  report(DRE->getLocation(), FD);
 return true;
   }
 

diff  --git a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp 
b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
index 3a86f36e3964f6..67248d4eedb915 100644
--- a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -50,7 +50,7 @@ std::vector testWalk(llvm::StringRef TargetCode,
   Inputs.ExtraFiles["target.h"] = Target.code().str();
   Inputs.ExtraArgs.push_back("-include");
   Inputs.ExtraArgs.push_back("target.h");
-  Inputs.ExtraArgs.push_back("-std=c++17");
+  Inputs.ExtraArgs.push_back("-std=c++20");
   TestAST AST(Inputs);
   const auto &SM = AST.sourceManager();
 
@@ -114,7 +114,7 @@ TEST(WalkAST, DeclRef) {
   testWalk("int $explicit^x;", "int y = ^x;");
   testWalk("int $explicit^foo();", "int y = ^foo();");
   testWalk("namespace ns { int $explicit^x; }", "int y = ns::^x;");
-  testWalk("struct $implicit^S { static int x; };", "int y = S::^x;");
+  testWalk("struct S { static int x; };", "int y = S::^x;");
   // Canonical declaration only.
   testWalk("extern int $explicit^x; int x;", "int y = ^x;");
   // Return type of `foo` isn't used.
@@ -309,6 +309,14 @@ TEST(WalkAST, Alias) {
 namespace ns { using ::$explicit^Foo; }
 template<> struct ns::Foo {};)cpp",
"ns::^Foo x;");
+  testWalk(R"cpp(
+namespace ns { enum class foo { bar }; }
+using ns::foo;)cpp",
+   "auto x = foo::^bar;");
+  testWalk(R"cpp(
+namespace ns { enum foo { bar }; }
+using ns::foo::$explicit^bar;)cpp",
+   "auto x = ^bar;");
 }
 
 TEST(WalkAST, Using) {
@@ -338,6 +346,8 @@ TEST(WalkAST, Using) {
 }
 using ns::$explicit^Y;)cpp",
"^Y x;");
+  testWalk("namespace ns { enum E {A}; } using enum ns::$explicit^E;",
+   "auto x = ^A;");
 }
 
 TEST(WalkAST, Namespaces) {
@@ -399,10 +409,10 @@ TEST(WalkAST, NestedTypes) {
 }
 
 TEST(WalkAST, MemberExprs) {
-  testWalk("struct $implicit^S { static int f; };", "void foo() { S::^f; }");
-  testWalk("struct B { static int f; }; struct $implicit^S : B {};",
+  testWalk("struct S { static int f; };", "void foo() { S::^f; }");
+  testWalk("struct B { static int f; }; struct S : B {};",
 

[PATCH] D158515: [include-cleaner] Make handling of enum constants similar to members

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
kadircet marked 2 inline comments as done.
Closed by commit rGab090e9e49ff: [include-cleaner] Make handling of enum 
constants similar to members (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158515

Files:
  clang-tools-extra/include-cleaner/lib/WalkAST.cpp
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -50,7 +50,7 @@
   Inputs.ExtraFiles["target.h"] = Target.code().str();
   Inputs.ExtraArgs.push_back("-include");
   Inputs.ExtraArgs.push_back("target.h");
-  Inputs.ExtraArgs.push_back("-std=c++17");
+  Inputs.ExtraArgs.push_back("-std=c++20");
   TestAST AST(Inputs);
   const auto &SM = AST.sourceManager();
 
@@ -114,7 +114,7 @@
   testWalk("int $explicit^x;", "int y = ^x;");
   testWalk("int $explicit^foo();", "int y = ^foo();");
   testWalk("namespace ns { int $explicit^x; }", "int y = ns::^x;");
-  testWalk("struct $implicit^S { static int x; };", "int y = S::^x;");
+  testWalk("struct S { static int x; };", "int y = S::^x;");
   // Canonical declaration only.
   testWalk("extern int $explicit^x; int x;", "int y = ^x;");
   // Return type of `foo` isn't used.
@@ -309,6 +309,14 @@
 namespace ns { using ::$explicit^Foo; }
 template<> struct ns::Foo {};)cpp",
"ns::^Foo x;");
+  testWalk(R"cpp(
+namespace ns { enum class foo { bar }; }
+using ns::foo;)cpp",
+   "auto x = foo::^bar;");
+  testWalk(R"cpp(
+namespace ns { enum foo { bar }; }
+using ns::foo::$explicit^bar;)cpp",
+   "auto x = ^bar;");
 }
 
 TEST(WalkAST, Using) {
@@ -338,6 +346,8 @@
 }
 using ns::$explicit^Y;)cpp",
"^Y x;");
+  testWalk("namespace ns { enum E {A}; } using enum ns::$explicit^E;",
+   "auto x = ^A;");
 }
 
 TEST(WalkAST, Namespaces) {
@@ -399,10 +409,10 @@
 }
 
 TEST(WalkAST, MemberExprs) {
-  testWalk("struct $implicit^S { static int f; };", "void foo() { S::^f; }");
-  testWalk("struct B { static int f; }; struct $implicit^S : B {};",
+  testWalk("struct S { static int f; };", "void foo() { S::^f; }");
+  testWalk("struct B { static int f; }; struct S : B {};",
"void foo() { S::^f; }");
-  testWalk("struct B { static void f(); }; struct $implicit^S : B {};",
+  testWalk("struct B { static void f(); }; struct S : B {};",
"void foo() { S::^f; }");
   testWalk("struct B { static void f(); }; ",
"struct S : B { void foo() { ^f(); } };");
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -126,13 +126,22 @@
   }
 
   bool VisitDeclRefExpr(DeclRefExpr *DRE) {
-// Static class members are handled here, as they don't produce 
MemberExprs.
-if (DRE->getFoundDecl()->isCXXClassMember()) {
-  if (auto *Qual = DRE->getQualifier())
-report(DRE->getLocation(), Qual->getAsRecordDecl(), RefType::Implicit);
-} else {
-  report(DRE->getLocation(), DRE->getFoundDecl());
+auto *FD = DRE->getFoundDecl();
+// For refs to non-meber-like decls, use the found decl.
+// For member-like decls, we should have a reference from the qualifier to
+// the container decl instead, which is preferred as it'll handle
+// aliases/exports properly.
+if (!FD->isCXXClassMember() && !llvm::isa(FD)) {
+  report(DRE->getLocation(), FD);
+  return true;
 }
+// If the ref is without a qualifier, and is a member, ignore it. As it is
+// available in current context due to some other construct (e.g. base
+// specifiers, using decls) that has to spell the name explicitly.
+// If it's an enum constant, it must be due to prior decl. Report 
references
+// to it instead.
+if (llvm::isa(FD) && !DRE->hasQualifier())
+  report(DRE->getLocation(), FD);
 return true;
   }
 


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -50,7 +50,7 @@
   Inputs.ExtraFiles["target.h"] = Target.code().str();
   Inputs.ExtraArgs.push_back("-include");
   Inputs.ExtraArgs.push_back("target.h");
-  Inputs.ExtraArgs.push_back("-std=c++17");
+  Inputs.ExtraArgs.push_back("-std=c++20");
   TestAST AST(Inputs);
   const auto &SM = AST.sourceManager();
 
@@ -114,7 +114,7 @@
   testWalk("int $explicit^x;", "int y = ^x;");
   testWalk("int $expl

[PATCH] D158968: [analyzer] Fix assertion on casting SVal to NonLoc inside the IteratorRange checker

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

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

The checker assumed that it could safely cast an SVal to Nonloc.
This surfaced because, with std::ranges, we can unintentionally match
on other APIs as well, thus increasing the likelihood of violating
checker assumptions about the context it's invoked.

See the discourse post on CallDescriptions and std::ranges here.
https://discourse.llvm.org/t/calldescriptions-should-not-skip-the-ranges-part-in-std-names-when-matching/73076

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158968

Files:
  clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
  clang/test/Analysis/iterator-range.cpp


Index: clang/test/Analysis/iterator-range.cpp
===
--- clang/test/Analysis/iterator-range.cpp
+++ clang/test/Analysis/iterator-range.cpp
@@ -946,3 +946,14 @@
   // expected-warning@-1 {{The right operand of '-' is a garbage value}}
   // expected-note@-2 {{The right operand of '-' is a garbage value}}
 }
+
+namespace std {
+namespace ranges {
+  template 
+  InOutIter next(InOutIter, Sentinel);
+} // namespace ranges
+} // namespace std
+
+void gh65009__no_crash_on_ranges_next(int **begin, int **end) {
+  (void)std::ranges::next(begin, end); // no-crash
+}
Index: clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
@@ -228,7 +228,7 @@
 Value = State->getRawSVal(*ValAsLoc);
   }
 
-  if (Value.isUnknownOrUndef())
+  if (Value.isUnknownOrUndef() || !isa(Value))
 return;
 
   // Incremention or decremention by 0 is never a bug.


Index: clang/test/Analysis/iterator-range.cpp
===
--- clang/test/Analysis/iterator-range.cpp
+++ clang/test/Analysis/iterator-range.cpp
@@ -946,3 +946,14 @@
   // expected-warning@-1 {{The right operand of '-' is a garbage value}}
   // expected-note@-2 {{The right operand of '-' is a garbage value}}
 }
+
+namespace std {
+namespace ranges {
+  template 
+  InOutIter next(InOutIter, Sentinel);
+} // namespace ranges
+} // namespace std
+
+void gh65009__no_crash_on_ranges_next(int **begin, int **end) {
+  (void)std::ranges::next(begin, end); // no-crash
+}
Index: clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
@@ -228,7 +228,7 @@
 Value = State->getRawSVal(*ValAsLoc);
   }
 
-  if (Value.isUnknownOrUndef())
+  if (Value.isUnknownOrUndef() || !isa(Value))
 return;
 
   // Incremention or decremention by 0 is never a bug.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

[Corentin Jabot via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/AST/ASTLambda.h:45
+  return isLambdaCallOperator(DC) &&
+ !cast(DC)->getType().isNull() &&
+ !cast(DC)->isExplicitObjectMemberFunction();

erichkeane wrote:
> Under what cases does this end up being null?  It seems like this condition 
> shouldn't be necessary, and it doesn't seem like we should have a case where 
> we create a method/function without a type set?
If you have invalid captures for example, you can end up with no type. (but we 
do need to create a method because you need to have a context to which attach 
the captures to)



Comment at: clang/test/CodeGenCXX/cxx2b-deducing-this.cpp:4
+
+struct TrivialStruct {
+void explicit_object_function(this TrivialStruct) {}

aaron.ballman wrote:
> I'd like to see more codegen tests in general -- for example, a test that 
> demonstrates we properly handle code like:
> ```
> struct B {
>   virtual void f();
> };
> 
> struct T {};
> struct D3 : B {
>   void f(this T&); // Okay, not an override
> };
> 
> void func() {
>   T t;
>   t.f(); // Verify this calls D3::f() and not B::f()
> }
> ```
> but also tests that show that we do the correct thing for calling conventions 
> (do explicit object parameter functions act as `__fastcall` functions?), 
> explicit object parameters in lambdas, call through a pointer to member 
> function, and so on.
> 
> Another test that could be interesting is how chained calls look (roughly):
> ```
> struct S {
>   void foo(this const S&);
> };
> 
> struct T {
>   S bar(this const &T);
> };
> 
> void func() {
>   T t;
>   t.bar().foo();
> }
> ```
That first example is ill-formed (it is an override, not allowed)

I will need help for codegen tests.
For `__thiscall`, are we even doing the same thing? 
https://compiler-explorer.com/z/KTea6W36T


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140828

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

[PATCH] D158808: [Clang] Modify Parser::ParseLambdaExpressionAfterIntroducer to check whether the lambda-declarator is valid

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

Still missing a release note.


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

https://reviews.llvm.org/D158808

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

[PATCH] D158848: [clang][dataflow] Support range-for loops in fixpoint algorithm.

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

mboehme accepted this revision.
mboehme added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:113
+  // Don't do anything special for CXXForRangeStmt, because the condition 
(being
+  // implicitly generated) isn't visible from the loop body.
+

Making this purely a comment might risk getting it attached to the wrong method.

Consider adding an empty `VisitCXXForRangeStmt()`, which would make the "do 
nothing" behavior explicit in code?



Comment at: 
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:1631
+ const AnalysisOutputs &) {
+EXPECT_THAT(Results.keys(), UnorderedElementsAre("after_loop"));
+  });

Is this check (and the `(void)0` with the `[[after_loop]` annotation) actually 
needed?

IIUC, the condition we want to test is that the analysis converges. The fact 
that it produces a `Results` entry for `[[after_loop]]` is really more a check 
of the testing infrastructure rather than the analysis itself?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158848

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

[PATCH] D158855: [analyzer][NFC] Remove useless class BuiltinBug

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

donat.nagy marked an inline comment as done.
donat.nagy added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp:27
   class BoolAssignmentChecker : public Checker< check::Bind > {
-mutable std::unique_ptr BT;
+mutable std::unique_ptr BT;
 void emitReport(ProgramStateRef state, CheckerContext &C,

xazax.hun wrote:
> In case you are down to some additional cleanup efforts, we no longer need 
> lazy initialization for `BugType`s. This is an old pattern, we used to need 
> it due to some initialization order problems, but those have been worked 
> around in engine. See the FuchsiaHandleChecker as an example how we do it in 
> newer checks: 
> https://github.com/llvm/llvm-project/blob/ea82a822d990824c58c6fa4b503ca84c4870c940/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp#L190
> 
> Feel free to ignore this or do it in a follow-up PR. 
Great news! I'll keep it in mind and I'll try to do this clean-up if I have 
enough free time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158855

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

[PATCH] D158968: [analyzer] Fix assertion on casting SVal to NonLoc inside the IteratorRange checker

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

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

LGTM. I'm not familiar with the Iterator checker family, but this is a very 
straightforward change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158968

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

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/AST/DeclCXX.cpp:841
   const auto *ParamTy =
-  Method->getParamDecl(0)->getType()->getAs();
+  Method->getNonObjectParameter(0)->getType()->getAs();
   if (!ParamTy || ParamTy->getPointeeType().isConstQualified())

aaron.ballman wrote:
> cor3ntin wrote:
> > cor3ntin wrote:
> > > aaron.ballman wrote:
> > > > Under what circumstances should existing calls to `getParamDecl()` be 
> > > > converted to using `getNonObjectParameter()` instead? Similar for 
> > > > `getNumParams()`.
> > > everytime you want to ignore (or handle differently) the explicit object 
> > > parameter. I'll do another survey at some point, to be sure i didn't miss 
> > > a spot
> > I found a bug https://github.com/cplusplus/CWG/issues/390 ! 
> > (`AreSpecialMemberFunctionsSameKind`) - So far nothing else but I'll do 
> > several passes
> This adds an unfortunate amount of complexity to the compiler because now we 
> have to explicitly remember to think about this weird scenario, but I'm not 
> seeing much of a way around it. I suspect this will be a bit of a bug factory 
> until we're used to thinking explicitly about this.
> 
> Be sure to double-check things like attributes that take arguments which 
> represent an index into a function parameter list (like the `format` 
> attribute), as those have to do a special dance to handle the implicit `this` 
> parameter. I'm guessing the static analyzer and clang-tidy will both also run 
> into this in some places as well.
https://cplusplus.github.io/CWG/issues/2787.html 
Do we want to implement that resolution now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140828

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

[clang] 7590b76 - Revert "[clang-format] Annotate constructor/destructor names"

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2023-08-28T10:31:07+02:00
New Revision: 7590b7657004b33b1a12b05f8e9174db3e192f8c

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

LOG: Revert "[clang-format] Annotate constructor/destructor names"

This reverts commit 0e63f1aacc0040e9a16fa2fab15a140b1686e2ab.

clang-format started to crash with contents like:
a.h:
```
```
$ clang-format a.h
```
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and 
include the crash backtrace.
Stack dump:
0.  Program arguments: ../llvm/build/bin/clang-format a.h
 #0 0x560b689fe177 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
/usr/local/google/home/kadircet/repos/llvm/llvm/lib/Support/Unix/Signals.inc:723:13
 #1 0x560b689fbfbe llvm::sys::RunSignalHandlers() 
/usr/local/google/home/kadircet/repos/llvm/llvm/lib/Support/Signals.cpp:106:18
 #2 0x560b689feaca SignalHandler(int) 
/usr/local/google/home/kadircet/repos/llvm/llvm/lib/Support/Unix/Signals.inc:413:1
 #3 0x7f030405a540 (/lib/x86_64-linux-gnu/libc.so.6+0x3c540)
 #4 0x560b68a9a980 is 
/usr/local/google/home/kadircet/repos/llvm/clang/include/clang/Lex/Token.h:98:44
 #5 0x560b68a9a980 is 
/usr/local/google/home/kadircet/repos/llvm/clang/lib/Format/FormatToken.h:562:51
 #6 0x560b68a9a980 startsSequenceInternal 
/usr/local/google/home/kadircet/repos/llvm/clang/lib/Format/FormatToken.h:831:9
 #7 0x560b68a9a980 startsSequence 
/usr/local/google/home/kadircet/repos/llvm/clang/lib/Format/FormatToken.h:600:12
 #8 0x560b68a9a980 getFunctionName 
/usr/local/google/home/kadircet/repos/llvm/clang/lib/Format/TokenAnnotator.cpp:3131:17
 #9 0x560b68a9a980 
clang::format::TokenAnnotator::annotate(clang::format::AnnotatedLine&) 
/usr/local/google/home/kadircet/repos/llvm/clang/lib/Format/TokenAnnotator.cpp:3191:17
Segmentation fault
```

Added: 


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

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 97df0b447b4baa..b6b4172818d171 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3097,76 +3097,6 @@ static unsigned maxNestingDepth(const AnnotatedLine 
&Line) {
   return Result;
 }
 
-// Returns the name of a function with no return type, e.g. a constructor or
-// destructor.
-static FormatToken *getFunctionName(const AnnotatedLine &Line) {
-  for (FormatToken *Tok = Line.getFirstNonComment(), *Name = nullptr; Tok;
-   Tok = Tok->getNextNonComment()) {
-// Skip C++11 attributes both before and after the function name.
-if (Tok->is(tok::l_square) && Tok->is(TT_AttributeSquare)) {
-  Tok = Tok->MatchingParen;
-  if (!Tok)
-break;
-  continue;
-}
-
-// Make sure the name is followed by a pair of parentheses.
-if (Name)
-  return Tok->is(tok::l_paren) && Tok->MatchingParen ? Name : nullptr;
-
-// Skip keywords that may precede the constructor/destructor name.
-if (Tok->isOneOf(tok::kw_friend, tok::kw_inline, tok::kw_virtual,
- tok::kw_constexpr, tok::kw_consteval, tok::kw_explicit)) {
-  continue;
-}
-
-// A qualified name may start from the global namespace.
-if (Tok->is(tok::coloncolon)) {
-  Tok = Tok->Next;
-  if (!Tok)
-break;
-}
-
-// Skip to the unqualified part of the name.
-while (Tok->startsSequence(tok::identifier, tok::coloncolon)) {
-  assert(Tok->Next);
-  Tok = Tok->Next->Next;
-  if (!Tok)
-break;
-}
-
-// Skip the `~` if a destructor name.
-if (Tok->is(tok::tilde)) {
-  Tok = Tok->Next;
-  if (!Tok)
-break;
-}
-
-// Make sure the name is not already annotated, e.g. as NamespaceMacro.
-if (Tok->isNot(tok::identifier) || Tok->isNot(TT_Unknown))
-  break;
-
-Name = Tok;
-  }
-
-  return nullptr;
-}
-
-// Checks if Tok is a constructor/destructor name qualified by its class name.
-static bool isCtorOrDtorName(const FormatToken *Tok) {
-  assert(Tok && Tok->is(tok::identifier));
-  const auto *Prev = Tok->Previous;
-
-  if (Prev && Prev->is(tok::tilde))
-Prev = Prev->Previous;
-
-  if (!Prev || !Prev->endsSequence(tok::coloncolon, tok::identifier))
-return false;
-
-  assert(Prev->Previous);
-  return Prev->Previous->TokenText == Tok->TokenText;
-}
-
 void TokenAnnotator::annotate(AnnotatedLine &Line) {
   for (auto &Child : Line.Children)
 annotate(*Child);
@@ -3187,14 +3117,6 @@ void TokenAnnotator::annotate(AnnotatedLine &Line) {
   ExpressionParser ExprParser(Style, Keywords, Line);
   ExprParser.parse();
 
-  if (Style.isCpp()) {
-auto *Tok = getFunctionName

[PATCH] D158967: [clangd] Record the stack bottom before building AST

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Would it be better to do this in the code that runs the parser?
This should help not only Clangd, but all the tools as well.

`clang::ParseAST` or `ASTFrontendAction::ExecuteAction` look like good 
candidates to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158967

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

[clang] b32aa72 - Recommit [C++20] [Coroutines] Mark await_suspend as noinline if the awaiter is not empty

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2023-08-28T17:07:30+08:00
New Revision: b32aa72afc1d6a094fde6ca04d8a1124af34a2ad

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

LOG: Recommit [C++20] [Coroutines] Mark await_suspend as noinline if the 
awaiter is not empty

The original patch is incorrect since it marks too many calls to be
noinline. It shows that it is bad to do analysis in the frontend again.
This patch tries to mark the await_suspend function as noinlne only.

---

Close https://github.com/llvm/llvm-project/issues/56301
Close https://github.com/llvm/llvm-project/issues/64151
Close https://github.com/llvm/llvm-project/issues/65018

See the summary and the discussion of
https://reviews.llvm.org/D157070
to get the full context.

As @rjmccall pointed out, the key point of the root cause is that
currently we didn't implement the semantics for '@llvm.coro.save'
well ("after the await-ready returns false, the coroutine is considered
to be suspended ") well.
Since the semantics implies that we (the compiler) shouldn't write
the spills into the coroutine frame in the await_suspend. But now it is
possible due to some combinations of the optimizations so the semantics are
broken. And the inlining is the root optimization of such optimizations.
So in this patch, we tried to add the `noinline` attribute to the
await_suspend function.

This looks slightly problematic since the users are able to call the
await_suspend function standalone. This is limited by the
implementation. On the one hand, we don't want the workaround solution
(See the proposed solution later) to be too complex. On the other hand,
it is rare to call await_suspend standalone. Also it is not semantically
incorrect to do so since the inlining is not part of the C++ standard.

Also as an optimization, we don't add the `noinline` attribute to
the await_suspend function if the awaiter is an empty class. This should be
correct since the programmers can't access the local variables in
await_suspend if the awaiter is empty. I think this is necessary for
the performance since it is pretty common.

The long term solution is:

call @llvm.coro.await_suspend(ptr %awaiter, ptr %handle,
  ptr @awaitSuspendFn)

Then it is much easier to perform the safety analysis in the middle
end. If it is safe to inline the call to awaitSuspend, we can replace it
in the CoroEarly pass. Otherwise we could replace it in the CoroSplit
pass.

Reviewed By: rjmccall

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

Added: 
clang/test/CodeGenCoroutines/coro-awaiter-noinline-suspend.cpp
clang/test/CodeGenCoroutines/pr56301.cpp
clang/test/CodeGenCoroutines/pr65018.cpp

Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/CodeGen/CGCoroutine.cpp
clang/lib/Sema/SemaCoroutine.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 767861df912998..6dc3c1c5fbcef8 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -174,6 +174,13 @@ Bug Fixes in This Version
   ``abs`` builtins.
   (`#45129 `_,
   `#45794 `_)
+- Fixed an issue where accesses to the local variables of a coroutine during
+  ``await_suspend`` could be misoptimized, including accesses to the awaiter
+  object itself.
+  (`#56301 `_)
+  The current solution may bring performance regressions if the awaiters have
+  non-static data members. See
+  `#64945 `_ for details.
 - Clang now prints unnamed members in diagnostic messages instead of giving an
   empty ''. Fixes
   (`#63759 `_)

diff  --git a/clang/lib/CodeGen/CGCoroutine.cpp 
b/clang/lib/CodeGen/CGCoroutine.cpp
index cf260570510176..47c077be4138a6 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -201,6 +201,7 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction 
&CGF, CGCoroData &Co
   CGF.CurCoro.InSuspendBlock = true;
   auto *SuspendRet = CGF.EmitScalarExpr(S.getSuspendExpr());
   CGF.CurCoro.InSuspendBlock = false;
+
   if (SuspendRet != nullptr && SuspendRet->getType()->isIntegerTy(1)) {
 // Veto suspension if requested by bool returning await_suspend.
 BasicBlock *RealSuspendBlock =

diff  --git a/clang/lib/Sema/SemaCoroutine.cpp 
b/clang/lib/Sema/SemaCoroutine.cpp
index c7d88f7784c187..0b2987376b8b3b 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -360,6 +360,63 @@ static Expr *maybeTailCall(Sema &S, QualType RetType, Expr 
*E,
 JustAddress);
 }
 

[clang] 5ade434 - [clang] Fix assertion fail when function has cleanups and fatal errors

[via cfe-commits]

Author: Podchishchaeva, Mariya
Date: 2023-08-28T02:25:57-07:00
New Revision: 5ade434a7e74f44d59a3863ddaf4762c97e1181b

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

LOG: [clang] Fix assertion fail when function has cleanups and fatal errors

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

Reviewed By: shafik

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

Added: 
clang/test/SemaCXX/gh48974.cpp

Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaDecl.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 6dc3c1c5fbcef8..deb53303a21b44 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -187,6 +187,8 @@ Bug Fixes in This Version
 - Fix crash in __builtin_strncmp and related builtins when the size value
   exceeded the maximum value representable by int64_t. Fixes
   (`#64876 `_)
+- Fixed an assertion if a function has cleanups and fatal erors.
+  (`#48974 `_)
 
 Bug Fixes to Compiler Builtins
 ^^

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 4eacc05f85e69e..664af4ccf4c635 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -16013,6 +16013,7 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt 
*Body,
   // been leftover. This ensures that these temporaries won't be picked up
   // for deletion in some later function.
   if (hasUncompilableErrorOccurred() ||
+  hasAnyUnrecoverableErrorsInThisFunction() ||
   getDiagnostics().getSuppressAllDiagnostics()) {
 DiscardCleanupsInEvaluationContext();
   }

diff  --git a/clang/test/SemaCXX/gh48974.cpp b/clang/test/SemaCXX/gh48974.cpp
new file mode 100644
index 00..7963107b06f5ea
--- /dev/null
+++ b/clang/test/SemaCXX/gh48974.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -Werror=unused-parameter -Wfatal-errors -verify %s
+
+void a(int &&s) {} // expected-error{{unused parameter 's'}}
+
+void b() {
+  int sum = a(0);
+}



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

[PATCH] D158827: [clang] Fix assertion fail when function has cleanups and fatal errors

[Mariya Podchishchaeva via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5ade434a7e74: [clang] Fix assertion fail when function has 
cleanups and fatal errors (authored by Fznamznon).

Changed prior to commit:
  https://reviews.llvm.org/D158827?vs=553420&id=553872#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158827

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/gh48974.cpp


Index: clang/test/SemaCXX/gh48974.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/gh48974.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -Werror=unused-parameter -Wfatal-errors -verify %s
+
+void a(int &&s) {} // expected-error{{unused parameter 's'}}
+
+void b() {
+  int sum = a(0);
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16013,6 +16013,7 @@
   // been leftover. This ensures that these temporaries won't be picked up
   // for deletion in some later function.
   if (hasUncompilableErrorOccurred() ||
+  hasAnyUnrecoverableErrorsInThisFunction() ||
   getDiagnostics().getSuppressAllDiagnostics()) {
 DiscardCleanupsInEvaluationContext();
   }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -187,6 +187,8 @@
 - Fix crash in __builtin_strncmp and related builtins when the size value
   exceeded the maximum value representable by int64_t. Fixes
   (`#64876 `_)
+- Fixed an assertion if a function has cleanups and fatal erors.
+  (`#48974 `_)
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/test/SemaCXX/gh48974.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/gh48974.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -Werror=unused-parameter -Wfatal-errors -verify %s
+
+void a(int &&s) {} // expected-error{{unused parameter 's'}}
+
+void b() {
+  int sum = a(0);
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16013,6 +16013,7 @@
   // been leftover. This ensures that these temporaries won't be picked up
   // for deletion in some later function.
   if (hasUncompilableErrorOccurred() ||
+  hasAnyUnrecoverableErrorsInThisFunction() ||
   getDiagnostics().getSuppressAllDiagnostics()) {
 DiscardCleanupsInEvaluationContext();
   }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -187,6 +187,8 @@
 - Fix crash in __builtin_strncmp and related builtins when the size value
   exceeded the maximum value representable by int64_t. Fixes
   (`#64876 `_)
+- Fixed an assertion if a function has cleanups and fatal erors.
+  (`#48974 `_)
 
 Bug Fixes to Compiler Builtins
 ^^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158855: [analyzer][NFC] Remove useless class BuiltinBug

[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.

I haven't checked the details, but it makes sense.
It's reassuring that all the tests pass :D, which is good enough for me.

Thanks for the cleanup!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158855

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

[clang] 985e399 - [analyzer] Fix assertion on casting SVal to NonLoc inside the IteratorRange checker

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2023-08-28T12:02:48+02:00
New Revision: 985e399647d591d6130ba6fe08c5b5f6cb87d9f6

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

LOG: [analyzer] Fix assertion on casting SVal to NonLoc inside the 
IteratorRange checker

The checker assumed that it could safely cast an SVal to Nonloc.
This surfaced because, with std::ranges, we can unintentionally match
on other APIs as well, thus increasing the likelihood of violating
checker assumptions about the context it's invoked.
https://godbolt.org/z/13vEb3K76

See the discourse post on CallDescriptions and std::ranges here.
https://discourse.llvm.org/t/calldescriptions-should-not-skip-the-ranges-part-in-std-names-when-matching/73076

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

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
clang/test/Analysis/iterator-range.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
index c682449921acc6..7740c3d4da1ec2 100644
--- a/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
@@ -228,7 +228,7 @@ void 
IteratorRangeChecker::verifyRandomIncrOrDecr(CheckerContext &C,
 Value = State->getRawSVal(*ValAsLoc);
   }
 
-  if (Value.isUnknownOrUndef())
+  if (Value.isUnknownOrUndef() || !isa(Value))
 return;
 
   // Incremention or decremention by 0 is never a bug.

diff  --git a/clang/test/Analysis/iterator-range.cpp 
b/clang/test/Analysis/iterator-range.cpp
index 849a1e9814ae39..ba5d0144775e92 100644
--- a/clang/test/Analysis/iterator-range.cpp
+++ b/clang/test/Analysis/iterator-range.cpp
@@ -946,3 +946,14 @@ int uninit_var(int n) {
   // expected-warning@-1 {{The right operand of '-' is a garbage value}}
   // expected-note@-2 {{The right operand of '-' is a garbage value}}
 }
+
+namespace std {
+namespace ranges {
+  template 
+  InOutIter next(InOutIter, Sentinel);
+} // namespace ranges
+} // namespace std
+
+void gh65009__no_crash_on_ranges_next(int **begin, int **end) {
+  (void)std::ranges::next(begin, end); // no-crash
+}



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

[PATCH] D158968: [analyzer] Fix assertion on casting SVal to NonLoc inside the IteratorRange checker

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG985e399647d5: [analyzer] Fix assertion on casting SVal to 
NonLoc inside the IteratorRange… (authored by steakhal).

Changed prior to commit:
  https://reviews.llvm.org/D158968?vs=553858&id=553879#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158968

Files:
  clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
  clang/test/Analysis/iterator-range.cpp


Index: clang/test/Analysis/iterator-range.cpp
===
--- clang/test/Analysis/iterator-range.cpp
+++ clang/test/Analysis/iterator-range.cpp
@@ -946,3 +946,14 @@
   // expected-warning@-1 {{The right operand of '-' is a garbage value}}
   // expected-note@-2 {{The right operand of '-' is a garbage value}}
 }
+
+namespace std {
+namespace ranges {
+  template 
+  InOutIter next(InOutIter, Sentinel);
+} // namespace ranges
+} // namespace std
+
+void gh65009__no_crash_on_ranges_next(int **begin, int **end) {
+  (void)std::ranges::next(begin, end); // no-crash
+}
Index: clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
@@ -228,7 +228,7 @@
 Value = State->getRawSVal(*ValAsLoc);
   }
 
-  if (Value.isUnknownOrUndef())
+  if (Value.isUnknownOrUndef() || !isa(Value))
 return;
 
   // Incremention or decremention by 0 is never a bug.


Index: clang/test/Analysis/iterator-range.cpp
===
--- clang/test/Analysis/iterator-range.cpp
+++ clang/test/Analysis/iterator-range.cpp
@@ -946,3 +946,14 @@
   // expected-warning@-1 {{The right operand of '-' is a garbage value}}
   // expected-note@-2 {{The right operand of '-' is a garbage value}}
 }
+
+namespace std {
+namespace ranges {
+  template 
+  InOutIter next(InOutIter, Sentinel);
+} // namespace ranges
+} // namespace std
+
+void gh65009__no_crash_on_ranges_next(int **begin, int **end) {
+  (void)std::ranges::next(begin, end); // no-crash
+}
Index: clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
@@ -228,7 +228,7 @@
 Value = State->getRawSVal(*ValAsLoc);
   }
 
-  if (Value.isUnknownOrUndef())
+  if (Value.isUnknownOrUndef() || !isa(Value))
 return;
 
   // Incremention or decremention by 0 is never a bug.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158968: [analyzer] Fix assertion on casting SVal to NonLoc inside the IteratorRange checker

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

steakhal added a comment.

In D158968#4620721 , @donat.nagy 
wrote:

> LGTM. I'm not familiar with the Iterator checker family, but this is a very 
> straightforward change.

Thanks for the review!
Well, it's an Ericsson checker ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158968

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

[PATCH] D133361: [BPF] Attribute btf_decl_tag("ctx") for structs

[Eduard Zingerman via Phabricator via cfe-commits]

eddyz87 added inline comments.



Comment at: llvm/include/llvm/IR/Intrinsics.td:2432
   ImmArg>]>;
+def int_context_marker_bpf : DefaultAttrsIntrinsic<[llvm_ptr_ty],
+   [llvm_ptr_ty],

yonghong-song wrote:
> Is it possible to make this builtin as BPF specific one?
Currently `llvm::Intrinsic::context_marker_bpf` gets defined in 
`llvm/IR/Intrinsics.h` (via include of generated file `IntrinsicEnums.inc`, 
same as `preserve_{struct,union,array}_access_index`).

BPF specific intrinsics are defined in `llvm/IR/IntrinsicsBPF.h` (generated 
directly w/o .inc intermediary).

Thus, if I move `context_marker_bpf` to `IntrinsicsBPF.td` I would have to 
include `IntrinsicsBPF.h` in `CGExpr.cpp`. However, I don't see any target 
specific includes in that file.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133361

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

[PATCH] D152054: [OpenMP] Codegen support for thread_limit on target directive

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

This new test is failing on Windows, due to `__kmpc_set_thread_limit` not being 
exported - see e.g. 
https://github.com/mstorsjo/llvm-mingw/actions/runs/5994183421/job/16264501555. 
Can someone add it to `openmp/runtime/src/dllexports`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152054

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

[PATCH] D158967: [clangd] Record the stack bottom before building AST

[Younan Zhang via Phabricator via cfe-commits]

zyounan added a comment.

> `clang::ParseAST` or `ASTFrontendAction::ExecuteAction` look like good 
> candidates to me.

We had already placed the initialization in ASTFrontendAction::ExecuteAction 
,
 but we don't have such if we prefer invoking the action outside the libclang. 
(Just as what we're doing now at the clangd site.)

I'm not 100% sure if `clang::ParseAST` is appropriate since this might cause a 
side effect to the global state. Would this break the encapsulation hierarchy?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158967

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

[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

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

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

Thanks for the updates! I didn't spot any important issue, so I'm accepting 
this commit, but let's wait for the opinion of @steakhal before merging this.

In D158707#4613955 , @danix800 wrote:

> In D158707#4613743 , @donat.nagy 
> wrote:
>
>> [...] Does this commit fix the "root" of the issue by eliminating some 
>> misuse of correctly implemented (but perhaps surprising) `SVal` / `APSInt` 
>> arithmetic; or is there an underlying bug in the `SVal` / `APSInt` 
>> arithmetic which is just avoided (and not eliminated) by this commit? In the 
>> latter case, what obstacles prevent us from fixing the root cause?
>
> For the observed signed compared to unsigned unexpectation from ArrayBoundV2,
> the constraint manager does give the CORRECT result.
>
> It's a misuse of the constraint API, mainly caused by unexpected unsigned 
> extent
> set by memory modeling. Actually `ArrayBoundV2::getSimplifiedOffsets()` has 
> clear
> comment that this `signed <-> unsigned` comparison is problematic.
>
>   // 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) {

In fact, the "NOTE:" comment was added by my patch D135375 
 (which eliminated lots of false positives 
caused by a situation when the argument `offset` was unsigned), but I was still 
confused by this new bug. (Where the other argument `extent` was unsigned and 
this led to incorrect conclusions like "`len` cannot be larger than `3u`, so 
`len` cannot be `-1`, because `-1` is larger than `3u`" 🙃 .) Thanks for 
troubleshooting this issue!




Comment at: clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp:34
+if (auto SSize =
+SVB.convertToArrayIndex(*Size).getAs())
+  return *SSize;

I think it's a good convention if `getDynamicExtent()` will always return 
concrete values as `ArrayIndexTy`. (If I didn't miss some very obscure case, 
then this will be true when this commit is merged.) 



Comment at: clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp:49
+return UnknownVal();
+  MR = MR->StripCasts();
+

Note that `StripCasts()` is overzealous and strips array indexing when the 
index is zero. For example if you have a rectangular matrix `int m[6][8];` then 
this function would (correctly) say that the element count of `m[1]` is 8, but 
it would claim that the element count of `m[0]` is 6 (because it strips the 
'cast' and calculates the element count of `m`).

This is caused by the hacky "solution" that casts are represented by 
`ElementRegion{original region, 0, new type}` which cannot be distinguished 
from a real element access where the index happens to be 0. (This is just a 
public service announcement, this already affects e.g. `getDynamicExtent` and I 
don't think that you can find a local solution. For more information, see also 
https://github.com/llvm/llvm-project/issues/42709 which plans to refactor the 
memory model.)



Comment at: clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp:72-75
+  auto ElementCount =
+  SVB.evalBinOp(State, BO_Div, Size, ElementSize, SVB.getArrayIndexType())
+  .getAs();
+  return ElementCount ? *ElementCount : UnknownVal();

Perhaps use the method `value_or` of std::optional?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158707

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

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/test/CodeGenCXX/cxx2b-deducing-this.cpp:4
+
+struct TrivialStruct {
+void explicit_object_function(this TrivialStruct) {}

cor3ntin wrote:
> aaron.ballman wrote:
> > I'd like to see more codegen tests in general -- for example, a test that 
> > demonstrates we properly handle code like:
> > ```
> > struct B {
> >   virtual void f();
> > };
> > 
> > struct T {};
> > struct D3 : B {
> >   void f(this T&); // Okay, not an override
> > };
> > 
> > void func() {
> >   T t;
> >   t.f(); // Verify this calls D3::f() and not B::f()
> > }
> > ```
> > but also tests that show that we do the correct thing for calling 
> > conventions (do explicit object parameter functions act as `__fastcall` 
> > functions?), explicit object parameters in lambdas, call through a pointer 
> > to member function, and so on.
> > 
> > Another test that could be interesting is how chained calls look (roughly):
> > ```
> > struct S {
> >   void foo(this const S&);
> > };
> > 
> > struct T {
> >   S bar(this const &T);
> > };
> > 
> > void func() {
> >   T t;
> >   t.bar().foo();
> > }
> > ```
> That first example is ill-formed (it is an override, not allowed)
> 
> I will need help for codegen tests.
> For `__thiscall`, are we even doing the same thing? 
> https://compiler-explorer.com/z/KTea6W36T
NVM, just different optimization levels


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140828

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

[PATCH] D157813: [VE][Clang] Change to enable VPU flag by default

[Kazushi Marukawa via Phabricator via cfe-commits]

kaz7 added a comment.

Hi @MaskRay , thank you for reviewing this patch last time.  Is it possible to 
review updated this patch again?  Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157813

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

[PATCH] D158977: [clang][dataflow] Don't associate prvalue expressions with storage locations.

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

mboehme created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Instead, map prvalue expressions directly to values in a newly introduced map
`Environment::ExprToVal`.

This change introduces an additional member variable in `Environment` but is
an overall win:

- It is more conceptually correctly, since prvalues don't have storage 
locations.

- It eliminates complexity from `Environment::setValue(const Expr &E, Value 
&Val)`.

- It reduces the amount of data stored in `Environment`: A prvalue now has a 
single entry in `ExprToVal` instead of one in `ExprToLoc` and one in `LocToVal`.

- Not allocating `StorageLocation`s for prvalues additionally reduces memory 
usage.

This patch is the last step in the migration to strict handling of value
categories (see https://discourse.llvm.org/t/70086 for details). The changes
here are almost entirely internal to `Environment`.

The only externally observable change is that when associating a `RecordValue`
with the location returned by `Environment::getResultObjectLocation()` for a
given expression, callers additionally need to associate the `RecordValue` with
the expression themselves.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158977

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp

Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -488,6 +488,7 @@
 // we've got a record type.
 if (S->getType()->isRecordType()) {
   auto &InitialVal = *cast(Env.createValue(S->getType()));
+  Env.setValue(*S, InitialVal);
   copyRecord(InitialVal.getLoc(), Env.getResultObjectLocation(*S), Env);
 }
 
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -458,8 +458,9 @@
 void transferMakeOptionalCall(const CallExpr *E,
   const MatchFinder::MatchResult &,
   LatticeTransferState &State) {
-  createOptionalValue(State.Env.getResultObjectLocation(*E),
-  State.Env.getBoolLiteralValue(true), State.Env);
+  State.Env.setValue(
+  *E, createOptionalValue(State.Env.getResultObjectLocation(*E),
+  State.Env.getBoolLiteralValue(true), State.Env));
 }
 
 void transferOptionalHasValueCall(const CXXMemberCallExpr *CallExpr,
@@ -543,7 +544,10 @@
 }
   }
 
-  createOptionalValue(*Loc, State.Env.makeAtomicBoolValue(), State.Env);
+  RecordValue &Val =
+  createOptionalValue(*Loc, State.Env.makeAtomicBoolValue(), State.Env);
+  if (E->isPRValue())
+State.Env.setValue(*E, Val);
 }
 
 void constructOptionalValue(const Expr &E, Environment &Env,
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -171,6 +171,105 @@
   return Current;
 }
 
+// Returns whether the values in `Map1` and `Map2` compare equal for those
+// keys that `Map1` and `Map2` have in common.
+template 
+bool compareKeyToValueMaps(const llvm::MapVector &Map1,
+   const llvm::MapVector &Map2,
+   const Environment &Env1, const Environment &Env2,
+   Environment::ValueModel &Model) {
+  for (auto &Entry : Map1) {
+Key K = Entry.first;
+assert(K != nullptr);
+
+Value *Val = Entry.second;
+assert(Val != nullptr);
+
+auto It = Map2.find(K);
+if (It == Map2.end())
+  continue;
+assert(It->second != nullptr);
+
+if (!areEquivalentValues(*Val, *It->second) &&
+!compareDistinctValues(K->getType(), *Val, Env1, *It->second, Env2,
+   Model))
+  return false;
+  }
+
+  return true;
+}
+
+// Perform a join on either `LocToVal` or `ExprToVal`. `Key` must be either
+// `const StorageLocation *` or `const Expr *`.
+template 
+llvm::MapVector
+joinKeyToValueMap(const llvm::MapVector &Map1,
+  const llvm::MapVector &Map2,
+  const Environment &Env1, const Environment &Env2,
+  Environment &JoinedEnv, Environment::ValueModel &Model) {
+  llvm::MapVector MergedMap;
+  for (auto &Entry : Map

[clang] aebe312 - Silence a "not all control paths return a value" warning; NFC

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2023-08-28T07:52:47-04:00
New Revision: aebe312c03a686af397c5e572e1ac9f325624734

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

LOG: Silence a "not all control paths return a value" warning; NFC

Added: 


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

Removed: 




diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 8d1235be78a4ad..c9315de8024e2f 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -6488,6 +6488,7 @@ class AtomicExpr : public Expr {
 return #ID;
 #include "clang/Basic/Builtins.def"
 }
+llvm_unreachable("not an atomic operator?");
   }
   unsigned getNumSubExprs() const { return NumSubExprs; }
 



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

[PATCH] D155064: [clang][SemaCXX] Diagnose tautological uses of consteval if and is_constant_evaluated

[Takuya Shimizu via Phabricator via cfe-commits]

hazohelet added a comment.

Ping


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

https://reviews.llvm.org/D155064

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

[clang-tools-extra] 43ca99a - Silence an illegal conversion warning in MSVC; NFC

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2023-08-28T08:06:03-04:00
New Revision: 43ca99a2516f107847271652a9be2cc694f1e6e5

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

LOG: Silence an illegal conversion warning in MSVC; NFC

The code previously required two levels of conversion, one from
SmallString to StringRef and one from StringRef to Regex. This made the
implicit conversion to StringRef be explicit instead.

Added: 


Modified: 
clang-tools-extra/clang-tidy/GlobList.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/GlobList.cpp 
b/clang-tools-extra/clang-tidy/GlobList.cpp
index 1d5b3a882fa648..4ff9d951110630 100644
--- a/clang-tools-extra/clang-tidy/GlobList.cpp
+++ b/clang-tools-extra/clang-tidy/GlobList.cpp
@@ -39,7 +39,7 @@ static llvm::Regex consumeGlob(StringRef &GlobList) {
 RegexText.push_back(C);
   }
   RegexText.push_back('$');
-  return {RegexText};
+  return {RegexText.str()};
 }
 
 GlobList::GlobList(StringRef Globs, bool KeepNegativeGlobs /* =true */) {



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

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D112921#4620399 , @wangpc wrote:

> Gentle ping. Can I move forward and land this?

What name and email address would you like us to use for patch attribution? 
@ldionne do you have any remaining concerns (I think all the ones you laid out 
have already been addressed, so if I don't hear back, I'll assume anything 
remaining can be handled post-commit)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112921

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

[PATCH] D158967: [clangd] Record the stack bottom before building AST

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D158967#4620931 , @zyounan wrote:

> We had already placed the initialization in ASTFrontendAction::ExecuteAction 
> ,
>  but we don't have such if we prefer invoking the action outside the 
> libclang. (Just as what we're doing now at the clangd site.)

That's `CompilerInstance::ExecuteAction`, Clangd invokes 
`ASTFrontendAction::ExecuteAction` and had the `noteBottomOfStack()` been 
placed there, no change would be needed on the Clangd side.

> I'm not 100% sure if `clang::ParseAST` is appropriate since this might cause 
> a side effect to the global state. Would this break the encapsulation 
> hierarchy?

Could you clarify what kind of encapsulation would it break?
Setting the global variable is suspicious, but the same argument applies for 
`CompilerInstance::ExecuteAction`.

Pragmatically, it's hard to ensure that `noteBottomOfStack` is called for each 
Clang tool (I'm sure there is a long tail of tools that don't do that).
Currently the code in `CompilerInstance::ExecuteAction` seems to acknowledge 
that there should be a fallback. I'm suggesting to move this fallback down to a 
function that actually runs parsing.
This would ensure the same workaround applies to more tools (including Clangd, 
IIUC) that don't call `noteBottomOfStack`.
Tools that do call `noteBottomOfStack` will not be affected as the function 
only remembers the first value for each thread.

Another alternative to enforce the right contract would be to assert that 
bottom of stack is set when parser and other recursive computations are called.
It's easy enough to achieve, but would be breaking many clients and would be a 
bigger cleanup.

That being said, it's perfectly reasonable to make sure Clangd calls 
`noteBottomOfStack` at the right places.
According to documentation of `noteBottomOfStack`, those should be tied to 
thread creation, i.e. `main` and `AsyncTaskRunner` are two most common entry 
points for threads that do parsing.
Clangd also creates other threads, but IIUC they are not running any recursive 
computations, so `noteBottomOfStack` is not strictly necessary there (but 
shouldn't hurt either).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158967

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

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

[Wang Pengcheng via Phabricator via cfe-commits]

wangpc added a comment.

In D112921#4621019 , @aaron.ballman 
wrote:

> In D112921#4620399 , @wangpc wrote:
>
>> Gentle ping. Can I move forward and land this?
>
> What name and email address would you like us to use for patch attribution?

Thanks! I have commit access.

> @ldionne do you have any remaining concerns (I think all the ones you laid 
> out have already been addressed, so if I don't hear back, I'll assume 
> anything remaining can be handled post-commit)?

I will leave it for some days and commit it before Friday if it's OK. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112921

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

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D112921#4621039 , @wangpc wrote:

> In D112921#4621019 , @aaron.ballman 
> wrote:
>
>> In D112921#4620399 , @wangpc wrote:
>>
>>> Gentle ping. Can I move forward and land this?
>>
>> What name and email address would you like us to use for patch attribution?
>
> Thanks! I have commit access.
>
>> @ldionne do you have any remaining concerns (I think all the ones you laid 
>> out have already been addressed, so if I don't hear back, I'll assume 
>> anything remaining can be handled post-commit)?
>
> I will leave it for some days and commit it before Friday if it's OK. :-)

Oh great! Yeah, I think committing later in the week if we don't hear back is 
perfectly reasonable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112921

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

[PATCH] D158981: [clang][dataflow][NFC] Eliminate `getStorageLocation()` / `setStorageLocation()` in `DataflowAnalysisContext`.

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

mboehme created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Instead, inline them into the `getStableStorageLocation()` overloads, which is
the only place they were called from (and should be called from).

`getStorageLocation()` / `setStorageLocation()` were confusing because neither
their name nor their documentation made reference to the fact that the storage
location is stable.

It didn't make sense to keep these as private member functions either. The code
for the two `getStableStorageLocation()` overloads has become only marginally
more complex by inlining these functions, and the `Expr` version is actually
more efficient because we only call `ignoreCFGOmittedNodes()` once instead of
twice.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158981

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp


Index: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -74,19 +74,21 @@
 
 StorageLocation &
 DataflowAnalysisContext::getStableStorageLocation(const VarDecl &D) {
-  if (auto *Loc = getStorageLocation(D))
+  if (auto *Loc = DeclToLoc.lookup(&D))
 return *Loc;
   auto &Loc = createStorageLocation(D.getType().getNonReferenceType());
-  setStorageLocation(D, Loc);
+  DeclToLoc[&D] = &Loc;
   return Loc;
 }
 
 StorageLocation &
 DataflowAnalysisContext::getStableStorageLocation(const Expr &E) {
-  if (auto *Loc = getStorageLocation(E))
+  const Expr &CanonE = ignoreCFGOmittedNodes(E);
+
+  if (auto *Loc = ExprToLoc.lookup(&CanonE))
 return *Loc;
-  auto &Loc = createStorageLocation(E.getType());
-  setStorageLocation(E, Loc);
+  auto &Loc = createStorageLocation(CanonE.getType());
+  ExprToLoc[&CanonE] = &Loc;
   return Loc;
 }
 
Index: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -103,39 +103,6 @@
   /// Returns a stable storage location for `E`.
   StorageLocation &getStableStorageLocation(const Expr &E);
 
-  /// Assigns `Loc` as the storage location of `D`.
-  ///
-  /// Requirements:
-  ///
-  ///  `D` must not be assigned a storage location.
-  void setStorageLocation(const ValueDecl &D, StorageLocation &Loc) {
-assert(!DeclToLoc.contains(&D));
-DeclToLoc[&D] = &Loc;
-  }
-
-  /// Returns the storage location assigned to `D` or null if `D` has no
-  /// assigned storage location.
-  StorageLocation *getStorageLocation(const ValueDecl &D) const {
-return DeclToLoc.lookup(&D);
-  }
-
-  /// Assigns `Loc` as the storage location of `E`.
-  ///
-  /// Requirements:
-  ///
-  ///  `E` must not be assigned a storage location.
-  void setStorageLocation(const Expr &E, StorageLocation &Loc) {
-const Expr &CanonE = ignoreCFGOmittedNodes(E);
-assert(!ExprToLoc.contains(&CanonE));
-ExprToLoc[&CanonE] = &Loc;
-  }
-
-  /// Returns the storage location assigned to `E` or null if `E` has no
-  /// assigned storage location.
-  StorageLocation *getStorageLocation(const Expr &E) const {
-return ExprToLoc.lookup(&ignoreCFGOmittedNodes(E));
-  }
-
   /// Returns a pointer value that represents a null pointer. Calls with
   /// `PointeeType` that are canonically equivalent will return the same 
result.
   /// A null `PointeeType` can be used for the pointee of `std::nullptr_t`.


Index: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -74,19 +74,21 @@
 
 StorageLocation &
 DataflowAnalysisContext::getStableStorageLocation(const VarDecl &D) {
-  if (auto *Loc = getStorageLocation(D))
+  if (auto *Loc = DeclToLoc.lookup(&D))
 return *Loc;
   auto &Loc = createStorageLocation(D.getType().getNonReferenceType());
-  setStorageLocation(D, Loc);
+  DeclToLoc[&D] = &Loc;
   return Loc;
 }
 
 StorageLocation &
 DataflowAnalysisContext::getStableStorageLocation(const Expr &E) {
-  if (auto *Loc = getStorageLocation(E))
+  const Expr &CanonE = ignoreCFGOmittedNodes(E);
+
+  if (auto *Loc = ExprToLoc.lookup(&CanonE))
 return *Loc;
-  auto &Loc = createStorageLocation(E.getType());
-  setStorageLocation(E, Loc);
+  auto &Loc = createStorageLocation(CanonE.getType());
+  ExprToLoc[&CanonE] = &Loc;
   return Loc;
 }
 
Index: clang/include/clang/A

[PATCH] D153339: [clang] Support vectors in __builtin_isfpclass

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 553908.
sepavloff added a comment.
Herald added a subscriber: sunshaoce.

Use representation of logical vectors similar to OpenCL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153339

Files:
  clang/docs/LanguageExtensions.rst
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/isfpclass.c

Index: clang/test/CodeGen/isfpclass.c
===
--- clang/test/CodeGen/isfpclass.c
+++ clang/test/CodeGen/isfpclass.c
@@ -14,7 +14,7 @@
 // CHECK-LABEL: define dso_local i1 @check_isfpclass_finite_strict
 // CHECK-SAME: (float noundef [[X:%.*]]) local_unnamed_addr #[[ATTR2:[0-9]+]] {
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f32(float [[X]], i32 504) #[[ATTR4:[0-9]+]]
+// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f32(float [[X]], i32 504) #[[ATTR6:[0-9]+]]
 // CHECK-NEXT:ret i1 [[TMP0]]
 //
 _Bool check_isfpclass_finite_strict(float x) {
@@ -35,7 +35,7 @@
 // CHECK-LABEL: define dso_local i1 @check_isfpclass_nan_f32_strict
 // CHECK-SAME: (float noundef [[X:%.*]]) local_unnamed_addr #[[ATTR2]] {
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f32(float [[X]], i32 3) #[[ATTR4]]
+// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f32(float [[X]], i32 3) #[[ATTR6]]
 // CHECK-NEXT:ret i1 [[TMP0]]
 //
 _Bool check_isfpclass_nan_f32_strict(float x) {
@@ -56,7 +56,7 @@
 // CHECK-LABEL: define dso_local i1 @check_isfpclass_snan_f64_strict
 // CHECK-SAME: (double noundef [[X:%.*]]) local_unnamed_addr #[[ATTR2]] {
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f64(double [[X]], i32 1) #[[ATTR4]]
+// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f64(double [[X]], i32 1) #[[ATTR6]]
 // CHECK-NEXT:ret i1 [[TMP0]]
 //
 _Bool check_isfpclass_snan_f64_strict(double x) {
@@ -77,7 +77,7 @@
 // CHECK-LABEL: define dso_local i1 @check_isfpclass_zero_f16_strict
 // CHECK-SAME: (half noundef [[X:%.*]]) local_unnamed_addr #[[ATTR2]] {
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f16(half [[X]], i32 96) #[[ATTR4]]
+// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f16(half [[X]], i32 96) #[[ATTR6]]
 // CHECK-NEXT:ret i1 [[TMP0]]
 //
 _Bool check_isfpclass_zero_f16_strict(_Float16 x) {
@@ -85,23 +85,88 @@
   return __builtin_isfpclass(x, 96 /*Zero*/);
 }
 
+// CHECK-LABEL: define dso_local i1 @check_isnan
+// CHECK-SAME: (float noundef [[X:%.*]]) local_unnamed_addr #[[ATTR2]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f32(float [[X]], i32 3) #[[ATTR6]]
+// CHECK-NEXT:ret i1 [[TMP0]]
+//
 _Bool check_isnan(float x) {
 #pragma STDC FENV_ACCESS ON
   return __builtin_isnan(x);
 }
 
+// CHECK-LABEL: define dso_local i1 @check_isinf
+// CHECK-SAME: (float noundef [[X:%.*]]) local_unnamed_addr #[[ATTR2]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f32(float [[X]], i32 516) #[[ATTR6]]
+// CHECK-NEXT:ret i1 [[TMP0]]
+//
 _Bool check_isinf(float x) {
 #pragma STDC FENV_ACCESS ON
   return __builtin_isinf(x);
 }
 
+// CHECK-LABEL: define dso_local i1 @check_isfinite
+// CHECK-SAME: (float noundef [[X:%.*]]) local_unnamed_addr #[[ATTR2]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f32(float [[X]], i32 504) #[[ATTR6]]
+// CHECK-NEXT:ret i1 [[TMP0]]
+//
 _Bool check_isfinite(float x) {
 #pragma STDC FENV_ACCESS ON
   return __builtin_isfinite(x);
 }
 
+// CHECK-LABEL: define dso_local i1 @check_isnormal
+// CHECK-SAME: (float noundef [[X:%.*]]) local_unnamed_addr #[[ATTR2]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f32(float [[X]], i32 264) #[[ATTR6]]
+// CHECK-NEXT:ret i1 [[TMP0]]
+//
 _Bool check_isnormal(float x) {
 #pragma STDC FENV_ACCESS ON
   return __builtin_isnormal(x);
 }
 
+
+typedef float __attribute__((ext_vector_type(4))) float4;
+typedef double __attribute__((ext_vector_type(4))) double4;
+typedef int __attribute__((ext_vector_type(4))) int4;
+typedef long __attribute__((ext_vector_type(4))) long4;
+
+// CHECK-LABEL: define dso_local <4 x i32> @check_isfpclass_nan_v4f32
+// CHECK-SAME: (<4 x float> noundef [[X:%.*]]) local_unnamed_addr #[[ATTR3]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = fcmp uno <4 x float> [[X]], zeroinitializer
+// CHECK-NEXT:[[TMP1:%.*]] = zext <4 x i1> [[TMP0]] to <4 x i32>
+// CHECK-NEXT:ret <4 x i32> [[TMP1]]
+//
+int4 check_isfpclass_nan_v4f32(float4 x) {
+  return __builtin_isfpclass(x, 3 /*NaN*/);
+}
+
+// CHECK-LABEL: define dso_local <4 x i32> @check_isfpclass_nan_strict_v4f32
+// CHECK-SAME: (<4 x float> noundef [[X:%.*]]) local_unnamed_addr #[[ATTR2]] {
+// CHECK-

[PATCH] D158855: [analyzer][NFC] Remove useless class BuiltinBug

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

This revision was automatically updated to reflect the committed changes.
donat.nagy marked an inline comment as done.
Closed by commit rG8a5cfdf7851d: [analyzer][NFC] Remove useless class 
BuiltinBug (authored by donat.nagy).

Changed prior to commit:
  https://reviews.llvm.org/D158855?vs=553498&id=553909#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158855

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
  clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
  clang/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
  clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp
  
clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/unittests/StaticAnalyzer/CallEventTest.cpp
  clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
  clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Index: clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
===
--- clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
+++ clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
@@ -89,8 +89,8 @@
 
 class CheckerRegistrationOrderPrinter
 : public Checker> {
-  std::unique_ptr BT =
-  std::make_unique(this, "Registration order");
+  std::unique_ptr BT =
+  std::make_unique(this, "Registration order");
 
 public:
   void checkPreStmt(const DeclStmt *DS, CheckerContext &C) const {
@@ -125,8 +125,7 @@
 
 #define UNITTEST_CHECKER(CHECKER_NAME, DIAG_MSG)   \
   class CHECKER_NAME : public Checker> {  \
-std::unique_ptr BT =   \
-std::make_unique(this, DIAG_MSG);  \
+std::unique_ptr BT = std::make_unique(this, DIAG_MSG);   \
\
   public:  \
 void checkPreStmt(const DeclStmt *DS, CheckerContext &C) const {}  \
Index: clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
===
--- clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
+++ clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
@@ -26,7 +26,7 @@
 
 class FalsePositiveGenerator : public Checker {
   using Self = FalsePositiveGenerator;
-  const BuiltinBug FalsePositiveGeneratorBug{this, "FalsePositiveGenerator"};
+  const BugType FalsePositiveGeneratorBug{this, "FalsePositiveGenerator"};
   using HandlerFn = bool (Self::*)(const CallEvent &Call,
CheckerContext &) const;
   CallDescriptionMap Callbacks = {
Index: clang/unittests/StaticAnalyzer/CallEventTest.cpp
===
--- clang/unittests/StaticAnalyzer/CallEventTest.cpp
+++ clang/unittests/StaticAnalyzer/CallEventTest.cpp
@@ -36,11 +36,11 @@
 }
 
 class CXXDeallocatorChecker : public Checker {
-  std::unique_ptr BT_uninitField;
+  std::unique_ptr BT_uninitField;
 
 public:
   CXXDeallocatorChecker()
-  : BT_uninitField(new BuiltinBug(this, "CXXDeallocator")) {}
+  : BT_uninitField(new BugType(this, "CXXDeallocator")) {}
 
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const {
 const auto *DC = dyn_cast(&Call);
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===

[PATCH] D153339: [clang] Support vectors in __builtin_isfpclass

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In D153339#4489025 , @aaron.ballman 
wrote:

> This seems reasonable to me, but I do wonder if changing the return type from 
> int to bool will cause surprises for anyone. I see why it's done, but this 
> seems more like a C interface (to me) which would return an `int`; returning 
> a `bool` gets promoted to `int` when you sneeze near it, so I wonder about 
> performance implications. But at the same time, we wouldn't want a vector of 
> ints returns in the vector case, I would suspect.

The new implementation uses the representation close to OpenCL, when it makes 
result of comparisons: 
https://registry.khronos.org/OpenCL/specs/3.0-unified/html/OpenCL_C.html#operators-relational.
 It is a vector of integers, where each element is of the same bit size as the 
source vector element. They must be already supported by the codegen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153339

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

[clang] 8a5cfdf - [analyzer][NFC] Remove useless class BuiltinBug

[Donát Nagy via cfe-commits]

Author: Donát Nagy
Date: 2023-08-28T15:20:14+02:00
New Revision: 8a5cfdf7851dcdb4e16c510b133d7d0e79e43fc4

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

LOG: [analyzer][NFC] Remove useless class BuiltinBug

...because it provides no useful functionality compared to its base
class `BugType`.

A long time ago there were substantial differences between `BugType` and
`BuiltinBug`, but they were eliminated by commit 1bd58233 in 2009 (!).
Since then the only functionality provided by `BuiltinBug` was that it
specified `categories::LogicError` as the bug category and it stored an
extra data member `desc`.

This commit sets `categories::LogicError` as the default value of the
third argument (bug category) in the constructors of BugType and
replaces use of the `desc` field with simpler logic.

Note that `BugType` has a data member `Description` and a non-virtual
method `BugType::getDescription()` which queries it; these are distinct
from the member `desc` of `BuiltinBug` and the identically named method
`BuiltinBug::getDescription()` which queries it. This confusing name
collision was a major motivation for the elimination of `BuiltinBug`.

As this commit touches many files, I avoided functional changes and left
behind FIXME notes to mark minor issues that should be fixed later.

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
clang/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
clang/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp
clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp
clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp
clang/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp
clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
clang/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp
clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp
clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
clang/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp
clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
clang/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp
clang/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp
clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp
clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
clang/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp
clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp

clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
clang/lib/StaticAnalyzer/Core/BugReporter.cpp
clang/unittests/StaticAnalyzer/CallEventTest.cpp
clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h 
b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
index 5588811763273b..e50afd6d0da7e5 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
+++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
@@ -35,13 +35,13 @@ class BugType {
   virtual void anchor();
 
 public:
-  BugType(CheckerNameRef CheckerName, StringRef Name, StringRef Cat,
-  bool SuppressOnSink = false)
-  : CheckerName(CheckerName), Description(Name), Category(Cat),
+  BugType(CheckerNameRef CheckerName, StringRef Desc,
+  StringRef Cat = categories::LogicError, bool SuppressOnSink = false)
+  : CheckerName(CheckerName), Description(Desc), Category(Cat),
 Checker(nullptr), SuppressOnSink(SuppressOnSink) {}
-  BugType(const CheckerBase *Checker, StringRef Name, StringRef Cat,
-  bool SuppressOnSink = false)
-  : CheckerName(Checker->getCheckerName()), Description(Name),
+  BugType(const CheckerBase *Checker, StringRef Desc,
+  StringRef Cat = categories::LogicError, bool SuppressOnSink = false)
+  : CheckerName(Checker->getCheckerName()), Description(Desc),
 Category(Cat), Checker(Checker), SuppressOnSink(SuppressOnSink) {}
   virtual ~BugType() = default;
 
@@ -64,27 +64,6 @@ class BugType {
   bool isSuppressOnSink() const { return SuppressOnSin

[PATCH] D158824: [RISCV][MC] MC layer support for xcvmem and xcvelw extensions

[Liao Chunyu via Phabricator via cfe-commits]

liaolucy updated this revision to Diff 553911.
liaolucy added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Address comments from Jim, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158824

Files:
  clang/test/Preprocessor/riscv-target-features.c
  llvm/docs/RISCVUsage.rst
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
  llvm/lib/Target/RISCV/RISCVFeatures.td
  llvm/lib/Target/RISCV/RISCVInstrInfoXCV.td
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/corev/XCVelw-invalid.s
  llvm/test/MC/RISCV/corev/XCVelw-valid.s
  llvm/test/MC/RISCV/corev/XCVmem-invalid.s
  llvm/test/MC/RISCV/corev/XCVmem-valid.s

Index: llvm/test/MC/RISCV/corev/XCVmem-valid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/corev/XCVmem-valid.s
@@ -0,0 +1,247 @@
+# RUN: llvm-mc -triple=riscv32 --mattr=+xcvmem -show-encoding %s \
+# RUN: | FileCheck %s --check-prefixes=CHECK-ENCODING,CHECK-INSTR
+# RUN: llvm-mc -filetype=obj -triple=riscv32 -mattr=+xcvmem < %s \
+# RUN: | llvm-objdump --mattr=+xcvmem -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefix=CHECK-INSTR %s
+# RUN: not llvm-mc -triple riscv32 %s 2>&1 \
+# RUN: | FileCheck -check-prefix=CHECK-NO-EXT %s
+
+cv.lb t0, (t1), 0
+# CHECK-INSTR: cv.lb t0, (t1), 0
+# CHECK-ENCODING: [0x8b,0x02,0x03,0x00]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lb a0, (a1), 2047
+# CHECK-INSTR: cv.lb a0, (a1), 2047
+# CHECK-ENCODING: [0x0b,0x85,0xf5,0x7f]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lb t0, (t1), t2
+# CHECK-INSTR: cv.lb t0, (t1), t2
+# CHECK-ENCODING: [0xab,0x32,0x73,0x00]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lb a0, (a1), a2
+# CHECK-INSTR: cv.lb a0, (a1), a2
+# CHECK-ENCODING: [0x2b,0xb5,0xc5,0x00]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lb t0, t2(t1)
+# CHECK-INSTR: cv.lb t0, t2(t1)
+# CHECK-ENCODING: [0xab,0x32,0x73,0x08]
+# CHECK-NO-EXT: unexpected token
+
+cv.lb a0, a2(a1)
+# CHECK-INSTR: cv.lb a0, a2(a1)
+# CHECK-ENCODING: [0x2b,0xb5,0xc5,0x08]
+# CHECK-NO-EXT: unexpected token
+
+cv.lbu t0, (t1), 0
+# CHECK-INSTR: cv.lbu t0, (t1), 0
+# CHECK-ENCODING: [0x8b,0x42,0x03,0x00]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lbu a0, (a1), 2047
+# CHECK-INSTR: cv.lbu a0, (a1), 2047
+# CHECK-ENCODING: [0x0b,0xc5,0xf5,0x7f]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lbu t0, (t1), t2
+# CHECK-INSTR: cv.lbu t0, (t1), t2
+# CHECK-ENCODING: [0xab,0x32,0x73,0x10]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lbu a0, (a1), a2
+# CHECK-INSTR: cv.lbu a0, (a1), a2
+# CHECK-ENCODING: [0x2b,0xb5,0xc5,0x10]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lbu t0, t2(t1)
+# CHECK-INSTR: cv.lbu t0, t2(t1)
+# CHECK-ENCODING: [0xab,0x32,0x73,0x18]
+# CHECK-NO-EXT: unexpected token
+
+cv.lbu a0, a2(a1)
+# CHECK-INSTR: cv.lbu a0, a2(a1)
+# CHECK-ENCODING: [0x2b,0xb5,0xc5,0x18]
+# CHECK-NO-EXT: unexpected token
+
+cv.lh t0, (t1), 0
+# CHECK-INSTR: cv.lh t0, (t1), 0
+# CHECK-ENCODING: [0x8b,0x12,0x03,0x00]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lh a0, (a1), 2047
+# CHECK-INSTR: cv.lh a0, (a1), 2047
+# CHECK-ENCODING: [0x0b,0x95,0xf5,0x7f]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lh t0, (t1), t2
+# CHECK-INSTR: cv.lh t0, (t1), t2
+# CHECK-ENCODING: [0xab,0x32,0x73,0x02]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lh a0, (a1), a2
+# CHECK-INSTR: cv.lh a0, (a1), a2
+# CHECK-ENCODING: [0x2b,0xb5,0xc5,0x02]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lh t0, t2(t1)
+# CHECK-INSTR: cv.lh t0, t2(t1)
+# CHECK-ENCODING: [0xab,0x32,0x73,0x0a]
+# CHECK-NO-EXT: unexpected token
+
+cv.lh a0, a2(a1)
+# CHECK-INSTR: cv.lh a0, a2(a1)
+# CHECK-ENCODING: [0x2b,0xb5,0xc5,0x0a]
+# CHECK-NO-EXT: unexpected token
+
+cv.lhu t0, (t1), 0
+# CHECK-INSTR: cv.lhu t0, (t1), 0
+# CHECK-ENCODING: [0x8b,0x52,0x03,0x00]
+# CHECK-NO-EXT: instruction requires the following: 'XCVmem' (CORE-V Post-incrementing Load & Store){{$}}
+
+cv.lhu a0, (a1), 2047
+# CHECK-INST

[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

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

steakhal added a comment.

Oh, so we would canonicalize towards a signed extent type. I see. I think I'm 
okay with that.
However, I think this needs a little bit of polishing and testing when the 
region does not point to the beginning of an array or object, but rather inside 
an object, or like this:

  int nums[] = {1,2,3};
  char *p = (char*)&nums[1];
  
  clang_analyzer_dumpExtent(p);
  clang_analyzer_dumpElementCount(p);
  ++p;
  clang_analyzer_dumpExtent(p);
  clang_analyzer_dumpElementCount(p);
  ++p;
  int *q = (int*)p;
  clang_analyzer_dumpExtent(q);
  clang_analyzer_dumpElementCount(q);




Comment at: clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp:47-48
+ const MemRegion *MR) {
+  if (!MR)
+return UnknownVal();
+  MR = MR->StripCasts();

I'd prefer hoisting this check to the callsite.
Usually, we assume `MR` is non-null. This is already the case for a sibling 
API, `getDynamicExtent()`.
Let's keep these "overloads" behave the same.



Comment at: clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp:56-57
+  QualType Ty = TVR->getValueType();
+  if (const auto *ConstArrayTy =
+  dyn_cast_or_null(Ty->getAsArrayTypeUnsafe()))
+return SVB.makeIntVal(ConstArrayTy->getSize(), false);

Funnily enough, one must use the `ASTContext::getAsConstantArrayType()` to 
achieve this.
I'm not sure why, but I was bitten by this.
It says something like this at the ASTContext:
```
  /// Type Query functions.  If the type is an instance of the specified class,
  /// return the Type pointer for the underlying maximally pretty type.  This
  /// is a member of ASTContext because this may need to do some amount of
  /// canonicalization, e.g. to move type qualifiers into the element type.
  const ArrayType *getAsArrayType(QualType T) const;
  const ConstantArrayType *getAsConstantArrayType(QualType T) const {
return dyn_cast_or_null(getAsArrayType(T));
  }
```



Comment at: clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp:58
+  dyn_cast_or_null(Ty->getAsArrayTypeUnsafe()))
+return SVB.makeIntVal(ConstArrayTy->getSize(), false);
+

For bool literal arguments we usually use "named parameter passing", aka. 
`/*paramname=*/false` constructs.



Comment at: clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp:66-75
+  assert(!ElementSize.isZeroConstant() && "Non-zero element size expected");
+  if (Size.isUndef())
+return UnknownVal();
+
+  SValBuilder &SVB = State->getStateManager().getSValBuilder();
+
+  auto ElementCount =

If `ElementSize` would be some symbol, constrained to null, we would pass the 
assertion, but still end up modeling a division by zero, resulting in 
`Undefined`, which then turned into `Unknown` - I see.

Looking at this, the assertion seems misleading as it won't protect the 
division.
That said, the early return on undef could be also dropped.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158707

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

[PATCH] D158135: [Clang][CodeGen] Add __builtin_bcopy

[Carlos Eduardo Seo via Phabricator via cfe-commits]

cseo added a comment.

FWIW, the pre-merge checks failed due to a problem with git-clang-format, not 
the tests themselves (they are all green).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158135

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

[PATCH] D158985: [AST] Fix crash in evaluation of OpaqueExpr in Clangd

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
ilya-biryukov requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added projects: clang, clang-tools-extra.

The code mistakenly returns the value as evaluated whenever the
temporary is allocated, but not yet evaluated.

I could only reproduce in Clangd and could not come up with an example
that would crash the compiler. Clangd runs more evaluations for hover
than the language would allow.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158985

Files:
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang/lib/AST/ExprConstant.cpp


Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -7646,7 +7646,8 @@
   }
 
   bool VisitOpaqueValueExpr(const OpaqueValueExpr *E) {
-if (APValue *Value = Info.CurrentCall->getCurrentTemporary(E))
+if (APValue *Value = Info.CurrentCall->getCurrentTemporary(E);
+Value && !Value->isAbsent())
   return DerivedSuccess(*Value, E);
 
 const Expr *Source = E->getSourceExpr();
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2954,6 +2954,32 @@
  HI.NamespaceScope = "";
  HI.Value = "0";
}},
+  // Should not crash.
+  {R"objc(
+typedef struct MyRect {} MyRect;
+
+@interface IFace
+@property(nonatomic) MyRect frame;
+@end
+
+MyRect foobar() {
+  MyRect mr;
+  return mr;
+}
+void test() {
+  IFace *v;
+  v.frame = [[foo^bar]]();
+}
+)objc",
+   [](HoverInfo &HI) {
+ HI.Name = "foobar";
+ HI.Kind = index::SymbolKind::Function;
+ HI.NamespaceScope = "";
+ HI.Definition = "MyRect foobar()";
+ HI.Type = {"MyRect ()", "MyRect ()"};
+ HI.ReturnType = {"MyRect", "MyRect"};
+ HI.Parameters.emplace();
+   }},
   {R"cpp(
  void foo(int * __attribute__(([[non^null]], noescape)) );
  )cpp",


Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -7646,7 +7646,8 @@
   }
 
   bool VisitOpaqueValueExpr(const OpaqueValueExpr *E) {
-if (APValue *Value = Info.CurrentCall->getCurrentTemporary(E))
+if (APValue *Value = Info.CurrentCall->getCurrentTemporary(E);
+Value && !Value->isAbsent())
   return DerivedSuccess(*Value, E);
 
 const Expr *Source = E->getSourceExpr();
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2954,6 +2954,32 @@
  HI.NamespaceScope = "";
  HI.Value = "0";
}},
+  // Should not crash.
+  {R"objc(
+typedef struct MyRect {} MyRect;
+
+@interface IFace
+@property(nonatomic) MyRect frame;
+@end
+
+MyRect foobar() {
+  MyRect mr;
+  return mr;
+}
+void test() {
+  IFace *v;
+  v.frame = [[foo^bar]]();
+}
+)objc",
+   [](HoverInfo &HI) {
+ HI.Name = "foobar";
+ HI.Kind = index::SymbolKind::Function;
+ HI.NamespaceScope = "";
+ HI.Definition = "MyRect foobar()";
+ HI.Type = {"MyRect ()", "MyRect ()"};
+ HI.ReturnType = {"MyRect", "MyRect"};
+ HI.Parameters.emplace();
+   }},
   {R"cpp(
  void foo(int * __attribute__(([[non^null]], noescape)) );
  )cpp",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158709: [Headers][Modules] Make separate headers for the stdarg.h and stddef.h pieces so that they can be modularized

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D158709#4617783 , @iana wrote:

> In D158709#4617295 , @aaron.ballman 
> wrote:
>
>> I'm a bit concerned about the impact on (non-modules) build time 
>> performance. This splits stddef.h and stdarg.h into needing to open 14 
>> different files instead of 2. Hopefully that's not a lot of overhead, but 
>> given that stddef.h is included in approximately every TU and that sometimes 
>> external forces cause files to be slow to open (network drives, AV software, 
>> etc), I think we should be cautious and measure the impact. I also really do 
>> not like how poorly this scales -- the fact that this is only needed for 
>> these two files helps a bit, but this is a lot of C++-specific hoops to jump 
>> through for C standard library headers.
>
> This is in support of clang modules, not C++ modules.

Ah, good to know, thank you!

> The plan is to make modules like this.
>
>   module _Builtin_stddef [system] {
> textual header "stddef.h"
>   
> explicit module max_align_t {
>   header "__stddef_max_align_t.h"
>   export *
> }
>   
> explicit module null {
>   header "__stddef_null.h"
>   export *
> }
> ...
>   }
>
> I do agree it's a bit of a file explosion, but I can't really think of any 
> better alternatives. It's tough to measure build time performance impact. If 
> you have a slow enough file system (and the fs on our builders at Apple is 
> quite slow), and you aren't using modules or pch's, and you have quite a lot 
> of source files, then possibly this would be noticeable? I still don't know 
> what we could really do about it besides have `#if !__has_feature(modules)` 
> and inline the contents in that case. That seems like a maintenance headache 
> and a half though. I would hope that most sizable projects either use clang 
> modules or pch's?

Yeah, measuring file system impacts is always tricky. :-( The reason I'm 
concerned is actually because a fair number of projects explicitly don't use 
PCH or Clang modules (or C++ modules), especially in C. Because of how fast 
compilations go in C, it's possible that the extra filesystem overhead really 
will be noticeable by users. However, I also can't think of a way around this 
in code...

At a high-level (not just for this patch, but for the entire series): is this 
need specific to your downstream or is this a more general need? e.g., should 
this complexity be kept downstream until we've got a second platform needing it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158709

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

[PATCH] D157963: [clang-format] Annotate constructor/destructor names

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

HazardyKnusperkeks added a subscriber: kadircet.
HazardyKnusperkeks added a comment.

@kadircet shouldn't you at least say why it got reverted? Even better state 
your problem and give a chance to fix before you revert?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157963

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

[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

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

donat.nagy added a comment.

In D158707#4621135 , @steakhal wrote:

> Oh, so we would canonicalize towards a signed extent type. I see. I think I'm 
> okay with that.
> However, I think this needs a little bit of polishing and testing when the 
> region does not point to the beginning of an array or object, but rather 
> inside an object, or like this:
>
>   int nums[] = {1,2,3};
>   char *p = (char*)&nums[1];
>   
>   clang_analyzer_dumpExtent(p);
>   clang_analyzer_dumpElementCount(p);
>   ++p;
>   clang_analyzer_dumpExtent(p);
>   clang_analyzer_dumpElementCount(p);
>   ++p;
>   int *q = (int*)p;
>   clang_analyzer_dumpExtent(q);
>   clang_analyzer_dumpElementCount(q);

Unfortunately the analyzer engine does not distinguish pointer arithmetic and 
element access, and we cannot handle these situations while that limitation 
exists. For example, if we have an array `int nums[3];`, then the element 
`nums[1]` and the incremented pointer `int *ptr = nums + 1;` are represented by 
the same `ElementRegion` object; so we cannot simultaneously say that (1) 
`nums[1]` is an int-sized memory region, and (2) it's valid to access the 
elements of the array as `ptr[-1]`, `ptr[0]` and `ptr[1]`.

The least bad heuristic is probably `RegionRawOffsetV2::computeOffset()` from 
ArrayBoundCheckerV2, which strips away all the `ElementRegion` layers, acting 
as if they are all coming from pointer arithmetic. This behavior is incorrect 
if we want to query the extent/elementcount of a "real" ElementRegion (e.g. a 
slice of a multidimensional array or `(char*)&nums[1]` in your example), but 
mostly leads to false negatives. On the other hand, if we process a pointer 
increment as if it were an element access, we can easily run into false 
positive reports -- this is why I had to abandon D150446 
.

I'd suggest that we should ignore pointer arithmetic in this commit (or create 
a testcase that documents the current insufficient behavior) and increment the 
priority of a through rewrite of the memory model. This is related to the plan 
https://github.com/llvm/llvm-project/issues/42709 suggested by @NoQ, but 
perhaps it would be enough to do a less drastic change in that direction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158707

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

[PATCH] D158990: [NFC][Clang] Remove unused function `CodeGenModule::addDefaultFunctionDefinitionAttributes`

[Juan Manuel Martinez Caamaño via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158990

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenModule.h


Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1276,7 +1276,6 @@
   /// on the function more conservative.  But it's unsafe to call this on a
   /// function which relies on particular fast-math attributes for correctness.
   /// It's up to you to ensure that this is safe.
-  void addDefaultFunctionDefinitionAttributes(llvm::Function &F);
   void mergeDefaultFunctionDefinitionAttributes(llvm::Function &F,
 bool WillInternalize);
 
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -2094,14 +2094,6 @@
 addMergableDefaultFunctionAttributes(CodeGenOpts, FuncAttrs);
 }
 
-void CodeGenModule::addDefaultFunctionDefinitionAttributes(llvm::Function &F) {
-  llvm::AttrBuilder FuncAttrs(F.getContext());
-  getDefaultFunctionAttributes(F.getName(), F.hasOptNone(),
-   /* AttrOnCallSite = */ false, FuncAttrs);
-  // TODO: call GetCPUAndFeaturesAttributes?
-  F.addFnAttrs(FuncAttrs);
-}
-
 /// Apply default attributes to \p F, accounting for merge semantics of
 /// attributes that should not overwrite existing attributes.
 void CodeGenModule::mergeDefaultFunctionDefinitionAttributes(


Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1276,7 +1276,6 @@
   /// on the function more conservative.  But it's unsafe to call this on a
   /// function which relies on particular fast-math attributes for correctness.
   /// It's up to you to ensure that this is safe.
-  void addDefaultFunctionDefinitionAttributes(llvm::Function &F);
   void mergeDefaultFunctionDefinitionAttributes(llvm::Function &F,
 bool WillInternalize);
 
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -2094,14 +2094,6 @@
 addMergableDefaultFunctionAttributes(CodeGenOpts, FuncAttrs);
 }
 
-void CodeGenModule::addDefaultFunctionDefinitionAttributes(llvm::Function &F) {
-  llvm::AttrBuilder FuncAttrs(F.getContext());
-  getDefaultFunctionAttributes(F.getName(), F.hasOptNone(),
-   /* AttrOnCallSite = */ false, FuncAttrs);
-  // TODO: call GetCPUAndFeaturesAttributes?
-  F.addFnAttrs(FuncAttrs);
-}
-
 /// Apply default attributes to \p F, accounting for merge semantics of
 /// attributes that should not overwrite existing attributes.
 void CodeGenModule::mergeDefaultFunctionDefinitionAttributes(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158848: [clang][dataflow] Support range-for loops in fixpoint algorithm.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 553929.
ymandel marked 2 inline comments as done.
ymandel added a comment.

address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158848

Files:
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -1612,4 +1612,21 @@
 
   });
 }
+
+TEST_F(TopTest, ForRangeStmtConverges) {
+  std::string Code = R"(
+void target(bool Foo) {
+  int Ints[10];
+  bool B = false;
+  for (int I : Ints)
+B = true;
+}
+  )";
+  runDataflow(Code,
+  [](const llvm::StringMap> &,
+ const AnalysisOutputs &) {
+// No additional expectations. We're only checking that the
+// analysis converged.
+  });
+}
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/ASTDumper.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/OperationKinds.h"
+#include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Analysis/Analyses/PostOrderCFGView.h"
 #include "clang/Analysis/CFG.h"
@@ -58,6 +59,7 @@
   case Stmt::WhileStmtClass:
   case Stmt::DoStmtClass:
   case Stmt::ForStmtClass:
+  case Stmt::CXXForRangeStmtClass:
 return true;
   default:
 return false;
@@ -108,6 +110,12 @@
 return {nullptr, false};
   }
 
+  TerminatorVisitorRetTy  VisitCXXForRangeStmt(const CXXForRangeStmt*) {
+// Don't do anything special for CXXForRangeStmt, because the condition
+// (being implicitly generated) isn't visible from the loop body.
+return {nullptr, false};
+  }
+
   TerminatorVisitorRetTy VisitBinaryOperator(const BinaryOperator *S) {
 assert(S->getOpcode() == BO_LAnd || S->getOpcode() == BO_LOr);
 auto *LHS = S->getLHS();


Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -1612,4 +1612,21 @@
 
   });
 }
+
+TEST_F(TopTest, ForRangeStmtConverges) {
+  std::string Code = R"(
+void target(bool Foo) {
+  int Ints[10];
+  bool B = false;
+  for (int I : Ints)
+B = true;
+}
+  )";
+  runDataflow(Code,
+  [](const llvm::StringMap> &,
+ const AnalysisOutputs &) {
+// No additional expectations. We're only checking that the
+// analysis converged.
+  });
+}
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/ASTDumper.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/OperationKinds.h"
+#include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Analysis/Analyses/PostOrderCFGView.h"
 #include "clang/Analysis/CFG.h"
@@ -58,6 +59,7 @@
   case Stmt::WhileStmtClass:
   case Stmt::DoStmtClass:
   case Stmt::ForStmtClass:
+  case Stmt::CXXForRangeStmtClass:
 return true;
   default:
 return false;
@@ -108,6 +110,12 @@
 return {nullptr, false};
   }
 
+  TerminatorVisitorRetTy  VisitCXXForRangeStmt(const CXXForRangeStmt*) {
+// Don't do anything special for CXXForRangeStmt, because the condition
+// (being implicitly generated) isn't visible from the loop body.
+return {nullptr, false};
+  }
+
   TerminatorVisitorRetTy VisitBinaryOperator(const BinaryOperator *S) {
 assert(S->getOpcode() == BO_LAnd || S->getOpcode() == BO_LOr);
 auto *LHS = S->getLHS();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158848: [clang][dataflow] Support range-for loops in fixpoint algorithm.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added inline comments.



Comment at: 
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:1631
+ const AnalysisOutputs &) {
+EXPECT_THAT(Results.keys(), UnorderedElementsAre("after_loop"));
+  });

mboehme wrote:
> Is this check (and the `(void)0` with the `[[after_loop]` annotation) 
> actually needed?
> 
> IIUC, the condition we want to test is that the analysis converges. The fact 
> that it produces a `Results` entry for `[[after_loop]]` is really more a 
> check of the testing infrastructure rather than the analysis itself?
Correct. I thought it would be useful to include some expectations, but those 
are really quite independent, so I've removed them.  We could go further and 
call `runAnalysis` instead of `runDataflow`, but I prefer the latter for 
consistency (in terms of build args and the like) with the other tests in this 
suite.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158848

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

[PATCH] D158991: [NFC][Clang] Add missing & to function argument

[Juan Manuel Martinez Caamaño via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158991

Files:
  clang/lib/CodeGen/CGCall.cpp


Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -2006,7 +2006,7 @@
 /// though we had emitted it ourselves. We remove any attributes on F that
 /// conflict with the attributes we add here.
 static void mergeDefaultFunctionDefinitionAttributes(
-llvm::Function &F, const CodeGenOptions CodeGenOpts,
+llvm::Function &F, const CodeGenOptions &CodeGenOpts,
 const LangOptions &LangOpts, const TargetOptions &TargetOpts,
 bool WillInternalize) {
 


Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -2006,7 +2006,7 @@
 /// though we had emitted it ourselves. We remove any attributes on F that
 /// conflict with the attributes we add here.
 static void mergeDefaultFunctionDefinitionAttributes(
-llvm::Function &F, const CodeGenOptions CodeGenOpts,
+llvm::Function &F, const CodeGenOptions &CodeGenOpts,
 const LangOptions &LangOpts, const TargetOptions &TargetOpts,
 bool WillInternalize) {
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157963: [clang-format] Annotate constructor/destructor names

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D157963#4621206 , 
@HazardyKnusperkeks wrote:

> @kadircet shouldn't you at least say why it got reverted? Even better state 
> your problem and give a chance to fix before you revert?

Hi, https://reviews.llvm.org/rG7590b7657004b33b1a12b05f8e9174db3e192f8c already 
has that context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157963

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

[PATCH] D158981: [clang][dataflow][NFC] Eliminate `getStorageLocation()` / `setStorageLocation()` in `DataflowAnalysisContext`.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

I love this! (specifically, that it simplifies the API surface)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158981

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

[PATCH] D158967: [clangd] Record the stack bottom before building AST

[Younan Zhang via Phabricator via cfe-commits]

zyounan added a subscriber: rsmith.
zyounan added a comment.

Thanks for the reply. And I understand (and agree with) the point that it's 
better to solve this problem once and for all.

> Currently the code in CompilerInstance::ExecuteAction seems to acknowledge 
> that there should be a fallback. I'm suggesting to move this fallback down to 
> a function that actually runs parsing.

One thing I'm afraid of is, that there are/were some compatible reasons that 
made us decide not to insert this call at `ASTFrontendAction::ExecuteAction` 
nor `clang::ParseAST`. (I didn't see the explanation in D66361 
. Richard, could you kindly explain why? 
@rsmith)

> That's `CompilerInstance::ExecuteAction`

Sorry for my mistake. I thought placing that in `CompilerInstance` was enough 
for most tools since it's less likely for developers to invoke the 
FrontendAction on their own, and for "advanced" users (like clangd) who'd like 
to control every step handling the AST, maybe it's fine to give them the 
opportunity to decide if it should crash on infinite recursion?

However, creating such a discrepancy doesn't make much sense. I'm happy to move 
this call to libclang if this won't break many things.

> Could you clarify what kind of encapsulation would it break?

(That's just my spitballing, please don't mind. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158967

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

[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

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

steakhal added a comment.

In D158707#4621270 , @donat.nagy 
wrote:

> In D158707#4621135 , @steakhal 
> wrote:
>
>> Oh, so we would canonicalize towards a signed extent type. I see. I think 
>> I'm okay with that.
>> However, I think this needs a little bit of polishing and testing when the 
>> region does not point to the beginning of an array or object, but rather 
>> inside an object, or like this:
>>
>>   int nums[] = {1,2,3};
>>   char *p = (char*)&nums[1];
>>   
>>   clang_analyzer_dumpExtent(p);
>>   clang_analyzer_dumpElementCount(p);
>>   ++p;
>>   clang_analyzer_dumpExtent(p);
>>   clang_analyzer_dumpElementCount(p);
>>   ++p;
>>   int *q = (int*)p;
>>   clang_analyzer_dumpExtent(q);
>>   clang_analyzer_dumpElementCount(q);
>
> Unfortunately the analyzer engine does not distinguish pointer arithmetic and 
> element access, and we cannot handle these situations while that limitation 
> exists. For example, if we have an array `int nums[3];`, then the element 
> `nums[1]` and the incremented pointer `int *ptr = nums + 1;` are represented 
> by the same `ElementRegion` object; so we cannot simultaneously say that (1) 
> `nums[1]` is an int-sized memory region, and (2) it's valid to access the 
> elements of the array as `ptr[-1]`, `ptr[0]` and `ptr[1]`.
>
> The least bad heuristic is probably `RegionRawOffsetV2::computeOffset()` from 
> ArrayBoundCheckerV2, which strips away all the `ElementRegion` layers, acting 
> as if they are all coming from pointer arithmetic. This behavior is incorrect 
> if we want to query the extent/elementcount of a "real" ElementRegion (e.g. a 
> slice of a multidimensional array or `(char*)&nums[1]` in your example), but 
> mostly leads to false negatives. On the other hand, if we process a pointer 
> increment as if it were an element access, we can easily run into false 
> positive reports -- this is why I had to abandon D150446 
> .
>
> I'd suggest that we should ignore pointer arithmetic in this commit (or 
> create a testcase that documents the current insufficient behavior) and 
> remember this as yet another reason to do through rewrite of the memory 
> model. This is related to the plan 
> https://github.com/llvm/llvm-project/issues/42709 suggested by @NoQ, but 
> perhaps it would be enough to do a less drastic change in that direction.

I'm aware of this, and I didn't mean to ask to fix this here.
I just want to see how these ExprInspection APIs will be affected in these 
edge-cases, where previously it returned "unknown", and now will return 
something else.
It's useful to demonstrate this if we ever come back to this commit.
Pinning down some tests and having some FIXMEs there should bring enough 
visibility.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158707

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

[PATCH] D158995: [clang] Add a Windows build in the Clang pre-commit CI

[Louis Dionne via Phabricator via cfe-commits]

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

This patch adds a CI job for Clang on Windows that is separate from
the monolithic job that gets added automatically via the Phabricator
integration with Buildkite. This way, we will retain the Windows testing
for Clang when we move to GitHub Pull Requests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158995

Files:
  clang/utils/ci/buildkite-pipeline.yml
  clang/utils/ci/run-buildbot


Index: clang/utils/ci/run-buildbot
===
--- clang/utils/ci/run-buildbot
+++ clang/utils/ci/run-buildbot
@@ -95,6 +95,25 @@
 
 ninja -C ${BUILD_DIR} check-clang
 ;;
+build-clang-windows)
+cmake -S llvm -B ${BUILD_DIR} -G Ninja 
 \
+-D CMAKE_C_COMPILER_LAUNCHER=sccache   
 \
+-D CMAKE_CXX_COMPILER_LAUNCHER=sccache 
 \
+-D CMAKE_BUILD_TYPE=Release
 \
+-D CMAKE_INSTALL_PREFIX=install-windows
 \
+-D LLVM_ENABLE_PROJECTS="clang;compiler-rt"
 \
+-D LLVM_ENABLE_ASSERTIONS=ON   
 \
+-D LLVM_BUILD_EXAMPLES=ON  
 \
+-D COMPILER_RT_BUILD_LIBFUZZER=OFF 
 \
+-D COMPILER_RT_BUILD_ORC=OFF
+
+ninja -C ${BUILD_DIR} install-clang install-clang-resource-headers
+ccache -s
+tar -cJvf install-windows.tar.xz install-windows/
+buildkite-agent artifact upload --debug install-windows.tar.xz
+
+ninja -C ${BUILD_DIR} check-clang
+;;
 generic-cxx03)
 buildkite-agent artifact download install.tar.xz .
 tar -xvf install.tar.xz
Index: clang/utils/ci/buildkite-pipeline.yml
===
--- clang/utils/ci/buildkite-pipeline.yml
+++ clang/utils/ci/buildkite-pipeline.yml
@@ -31,7 +31,7 @@
 
   - wait
 
-  - label: "Building and testing clang"
+  - label: "Building and testing clang (Linux)"
 commands:
   - "clang/utils/ci/run-buildbot build-clang"
 agents:
@@ -42,6 +42,17 @@
   limit: 2
 timeout_in_minutes: 120
 
+  - label: "Building and testing clang (Windows)"
+commands:
+  - "clang/utils/ci/run-buildbot build-clang-windows"
+agents:
+  queue: "windows"
+retry:
+  automatic:
+- exit_status: -1  # Agent was lost
+  limit: 2
+timeout_in_minutes: 120
+
   - wait
 
   - label: "Running libc++ test suite in C++03"


Index: clang/utils/ci/run-buildbot
===
--- clang/utils/ci/run-buildbot
+++ clang/utils/ci/run-buildbot
@@ -95,6 +95,25 @@
 
 ninja -C ${BUILD_DIR} check-clang
 ;;
+build-clang-windows)
+cmake -S llvm -B ${BUILD_DIR} -G Ninja  \
+-D CMAKE_C_COMPILER_LAUNCHER=sccache\
+-D CMAKE_CXX_COMPILER_LAUNCHER=sccache  \
+-D CMAKE_BUILD_TYPE=Release \
+-D CMAKE_INSTALL_PREFIX=install-windows \
+-D LLVM_ENABLE_PROJECTS="clang;compiler-rt" \
+-D LLVM_ENABLE_ASSERTIONS=ON\
+-D LLVM_BUILD_EXAMPLES=ON   \
+-D COMPILER_RT_BUILD_LIBFUZZER=OFF  \
+-D COMPILER_RT_BUILD_ORC=OFF
+
+ninja -C ${BUILD_DIR} install-clang install-clang-resource-headers
+ccache -s
+tar -cJvf install-windows.tar.xz install-windows/
+buildkite-agent artifact upload --debug install-windows.tar.xz
+
+ninja -C ${BUILD_DIR} check-clang
+;;
 generic-cxx03)
 buildkite-agent artifact download install.tar.xz .
 tar -xvf install.tar.xz
Index: clang/utils/ci/buildkite-pipeline.yml
===
--- clang/utils/ci/buildkite-pipeline.yml
+++ clang/utils/ci/buildkite-pipeline.yml
@@ -31,7 +31,7 @@
 
   - wait
 
-  - label: "Building and testing clang"
+  - label: "Building and testing clang (Linux)"
 commands:
   - "clang/utils/ci/run-buildbot build-clang"
 agents:
@@ -42,6 +42,17 @@
   limit: 2
 timeout_in_minutes: 120
 
+  - label: "Building and testing clang (Windows)"
+commands:
+  - "clang/utils/ci/run-buildbot build-clang-windows"
+agents:
+  queue: "windows"
+retry:
+  automatic:
+- exit_status: -1  # Agent was lost
+  limit: 2
+timeout_in_minutes: 120
+
   - wait
 
   - label: "Running lib

[PATCH] D158995: [clang] Add a Windows build in the Clang pre-commit CI

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a subscriber: aaron.ballman.
ldionne added a comment.

@aaron.ballman for awareness


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158995

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

[clang-tools-extra] d703dcd - [AST] Fix crash in evaluation of OpaqueExpr in Clangd

[Ilya Biryukov via cfe-commits]

Author: Ilya Biryukov
Date: 2023-08-28T17:10:44+02:00
New Revision: d703dcdfb08dbd591589a12e0ca0852c5fd30a72

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

LOG: [AST] Fix crash in evaluation of OpaqueExpr in Clangd

The code mistakenly returns the value as evaluated whenever the
temporary is allocated, but not yet evaluated.

I could only reproduce in Clangd and could not come up with an example
that would crash the compiler. Clangd runs more evaluations for hover
than the language would allow.

Reviewed By: kadircet

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

Added: 


Modified: 
clang-tools-extra/clangd/unittests/HoverTests.cpp
clang/lib/AST/ExprConstant.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp 
b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 9cab9a086958d6..c26bda898687cb 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2967,6 +2967,32 @@ TEST(Hover, All) {
  HI.NamespaceScope = "";
  HI.Value = "0";
}},
+  // Should not crash.
+  {R"objc(
+typedef struct MyRect {} MyRect;
+
+@interface IFace
+@property(nonatomic) MyRect frame;
+@end
+
+MyRect foobar() {
+  MyRect mr;
+  return mr;
+}
+void test() {
+  IFace *v;
+  v.frame = [[foo^bar]]();
+}
+)objc",
+   [](HoverInfo &HI) {
+ HI.Name = "foobar";
+ HI.Kind = index::SymbolKind::Function;
+ HI.NamespaceScope = "";
+ HI.Definition = "MyRect foobar()";
+ HI.Type = {"MyRect ()", "MyRect ()"};
+ HI.ReturnType = {"MyRect", "MyRect"};
+ HI.Parameters.emplace();
+   }},
   {R"cpp(
  void foo(int * __attribute__(([[non^null]], noescape)) );
  )cpp",

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 130cf23737f104..630fb02d0531e9 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -7653,7 +7653,8 @@ class ExprEvaluatorBase
   }
 
   bool VisitOpaqueValueExpr(const OpaqueValueExpr *E) {
-if (APValue *Value = Info.CurrentCall->getCurrentTemporary(E))
+if (APValue *Value = Info.CurrentCall->getCurrentTemporary(E);
+Value && !Value->isAbsent())
   return DerivedSuccess(*Value, E);
 
 const Expr *Source = E->getSourceExpr();



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

[PATCH] D158985: [AST] Fix crash in evaluation of OpaqueExpr in Clangd

[Ilya Biryukov 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 rGd703dcdfb08d: [AST] Fix crash in evaluation of OpaqueExpr in 
Clangd (authored by ilya-biryukov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158985

Files:
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang/lib/AST/ExprConstant.cpp


Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -7653,7 +7653,8 @@
   }
 
   bool VisitOpaqueValueExpr(const OpaqueValueExpr *E) {
-if (APValue *Value = Info.CurrentCall->getCurrentTemporary(E))
+if (APValue *Value = Info.CurrentCall->getCurrentTemporary(E);
+Value && !Value->isAbsent())
   return DerivedSuccess(*Value, E);
 
 const Expr *Source = E->getSourceExpr();
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2967,6 +2967,32 @@
  HI.NamespaceScope = "";
  HI.Value = "0";
}},
+  // Should not crash.
+  {R"objc(
+typedef struct MyRect {} MyRect;
+
+@interface IFace
+@property(nonatomic) MyRect frame;
+@end
+
+MyRect foobar() {
+  MyRect mr;
+  return mr;
+}
+void test() {
+  IFace *v;
+  v.frame = [[foo^bar]]();
+}
+)objc",
+   [](HoverInfo &HI) {
+ HI.Name = "foobar";
+ HI.Kind = index::SymbolKind::Function;
+ HI.NamespaceScope = "";
+ HI.Definition = "MyRect foobar()";
+ HI.Type = {"MyRect ()", "MyRect ()"};
+ HI.ReturnType = {"MyRect", "MyRect"};
+ HI.Parameters.emplace();
+   }},
   {R"cpp(
  void foo(int * __attribute__(([[non^null]], noescape)) );
  )cpp",


Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -7653,7 +7653,8 @@
   }
 
   bool VisitOpaqueValueExpr(const OpaqueValueExpr *E) {
-if (APValue *Value = Info.CurrentCall->getCurrentTemporary(E))
+if (APValue *Value = Info.CurrentCall->getCurrentTemporary(E);
+Value && !Value->isAbsent())
   return DerivedSuccess(*Value, E);
 
 const Expr *Source = E->getSourceExpr();
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2967,6 +2967,32 @@
  HI.NamespaceScope = "";
  HI.Value = "0";
}},
+  // Should not crash.
+  {R"objc(
+typedef struct MyRect {} MyRect;
+
+@interface IFace
+@property(nonatomic) MyRect frame;
+@end
+
+MyRect foobar() {
+  MyRect mr;
+  return mr;
+}
+void test() {
+  IFace *v;
+  v.frame = [[foo^bar]]();
+}
+)objc",
+   [](HoverInfo &HI) {
+ HI.Name = "foobar";
+ HI.Kind = index::SymbolKind::Function;
+ HI.NamespaceScope = "";
+ HI.Definition = "MyRect foobar()";
+ HI.Type = {"MyRect ()", "MyRect ()"};
+ HI.ReturnType = {"MyRect", "MyRect"};
+ HI.Parameters.emplace();
+   }},
   {R"cpp(
  void foo(int * __attribute__(([[non^null]], noescape)) );
  )cpp",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

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

donat.nagy added a comment.

Yes, adding tests that demonstrate the current behavior is a good idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158707

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

[PATCH] D158967: [clangd] Record the stack bottom before building AST

[Younan Zhang via Phabricator via cfe-commits]

zyounan added a comment.

> According to documentation of noteBottomOfStack, those should be tied to 
> thread creation, i.e. main and AsyncTaskRunner are two most common entry 
> points for threads that do parsing.

Yeah, I was about to put this call there. But they both boil down to the single 
`ParsedAST::build()`, right? (Admittedly, this breaks the "encapsulation" if we 
say 'the build process for AST should not bring any other side effects besides 
the AST itself', and this doesn't adhere to the document that it should be 
created at the beginning of the thread/program.)

> Currently the code in CompilerInstance::ExecuteAction seems to acknowledge 
> that there should be a fallback. I'm suggesting to move this fallback down to 
> a function that actually runs parsing.

Upon the implementation, the result will be more accurate if it occurs earlier 
before diving into parsing. Maybe we can do both, that put this call at clangd 
site and libclang's `ASTFrontendAction::ExecuteAction`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158967

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

[PATCH] D139730: [OpenMP][DeviceRTL][AMDGPU] Support code object version 5

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

Just a few more nits. I think it's looking fine but I haven't tested it. Anyone 
else?




Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:406
 
+  // pass on -mllvm options to the clang
+  for (const opt::Arg *Arg : Args.filtered(OPT_mllvm)) {





Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:415-417
+  if (SaveTemps) {
 CmdArgs.push_back("-save-temps");
+  }

No braces around a single line if.



Comment at: openmp/libomptarget/plugins-nextgen/amdgpu/utils/UtilitiesRTL.h:54
+
+uint16_t getImplicitArgsSize(uint16_t Version) {
+  return Version < ELF::ELFABIVERSION_AMDGPU_HSA_V5

We return uint16_t here? These are sizes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139730

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

[PATCH] D158787: [clang-tidy][readability] add Leading_upper_snake_case

[Steven Lewis via Phabricator via cfe-commits]

MasterCopy8GB added a comment.

Hello @PiotrZSL, I do not have rights to commit to the LLVM repository. Could 
you please commit these changes for me? Thank you for all of your time and 
effort!


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

https://reviews.llvm.org/D158787

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

[PATCH] D158247: [CUDA][HIP] Fix overloading resolution in global variable initializer

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

ping


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

https://reviews.llvm.org/D158247

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

[PATCH] D159000: Reland "[Profile] Allow online merging with debug info correlation."

[Zequan Wu via Phabricator via cfe-commits]

zequanwu created this revision.
zequanwu added reviewers: ellis, aeubanks.
Herald added subscribers: Enna1, ormris.
Herald added a project: All.
zequanwu requested review of this revision.
Herald added projects: clang, Sanitizers.
Herald added subscribers: Sanitizers, cfe-commits.

Fixed the issue when data is 0 by relaxing the condition.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159000

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  compiler-rt/lib/profile/InstrProfilingMerge.c
  compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
  compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
  compiler-rt/test/profile/instrprof-merge-empty-data.test
  compiler-rt/test/profile/instrprof-merge-error.c

Index: compiler-rt/test/profile/instrprof-merge-error.c
===
--- compiler-rt/test/profile/instrprof-merge-error.c
+++ compiler-rt/test/profile/instrprof-merge-error.c
@@ -1,11 +1,5 @@
 // RUN: rm -rf %t; mkdir %t
 
-// RUN: %clang_pgogen -o %t/dbg -g -mllvm --debug-info-correlate -mllvm --disable-vp=true %s
-// RUN: env LLVM_PROFILE_FILE=%t/dbg_%m.profdata %run %t/dbg 2>&1 | count 0
-// RUN: env LLVM_PROFILE_FILE=%t/dbg_%m.profdata %run %t/dbg 2>&1 | FileCheck %s --check-prefix=DBG
-
-// DBG: Debug info correlation does not support profile merging at runtime.
-
 // RUN: %clang_pgogen -o %t/timeprof -mllvm -pgo-temporal-instrumentation %s
 // RUN: env LLVM_PROFILE_FILE=%t/timeprof_%m.profdata %run %t/timeprof 2>&1 | count 0
 // RUN: env LLVM_PROFILE_FILE=%t/timeprof_%m.profdata %run %t/timeprof 2>&1 | FileCheck %s --check-prefix=TIMEPROF
Index: compiler-rt/test/profile/instrprof-merge-empty-data.test
===
--- /dev/null
+++ compiler-rt/test/profile/instrprof-merge-empty-data.test
@@ -0,0 +1,14 @@
+// Test 
+// RUN: rm -rf %t.dir && split-file %s %t.dir && cd %t.dir
+// RUN: %clangxx_profgen -fcoverage-mapping -o %t main.c -fprofile-list=funlist
+// RUN: LLVM_PROFILE_FILE='a%m.profraw' %t
+// RUN: LLVM_PROFILE_FILE='a%m.profraw' %t 2>&1 | FileCheck %s --allow-empty
+
+// CHECK-NOT: LLVM Profile Error
+
+//--- main.c
+int main() {}
+
+//--- funlist
+[clang]
+default:skip
Index: compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
===
--- compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
+++ compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
@@ -24,3 +24,23 @@
 // RUN: llvm-profdata merge -o %t.cov.normal.profdata %t.cov.profraw
 
 // RUN: diff <(llvm-profdata show --all-functions --counts %t.cov.normal.profdata) <(llvm-profdata show --all-functions --counts %t.cov.profdata)
+
+// Test debug info correlate with online merging.
+
+// RUN: env LLVM_PROFILE_FILE=%t-1.profraw %run %t.normal
+// RUN: env LLVM_PROFILE_FILE=%t-2.profraw %run %t.normal
+// RUN: llvm-profdata merge -o %t.normal.profdata %t-1.profraw %t-2.profraw
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: llvm-profdata merge -o %t.profdata --debug-info=%t %t.profdir/
+
+// RUN: diff <(llvm-profdata show --all-functions --counts %t.normal.profdata) <(llvm-profdata show --all-functions --counts %t.profdata)
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.cov.proflite %run %t.cov
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.cov.proflite %run %t.cov
+// RUN: llvm-profdata merge -o %t.cov.profdata --debug-info=%t.cov %t.profdir/
+
+// RUN: diff <(llvm-profdata show --all-functions --counts %t.cov.normal.profdata) <(llvm-profdata show --all-functions --counts %t.cov.profdata)
Index: compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
===
--- compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
+++ compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
@@ -18,3 +18,23 @@
 // RUN: llvm-profdata merge -o %t.cov.normal.profdata %t.cov.profraw
 
 // RUN: diff <(llvm-profdata show --all-functions --counts %t.cov.normal.profdata) <(llvm-profdata show --all-functions --counts %t.cov.profdata)
+
+// Test debug info correlate with online merging.
+
+// RUN: env LLVM_PROFILE_FILE=%t-1.profraw %run %t.normal
+// RUN: env LLVM_PROFILE_FILE=%t-2.profraw %run %t.normal
+// RUN: llvm-profdata merge -o %t.normal.profdata %t-1.profraw %t-2.profraw
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: llvm-profdata merge -o %t.profdata --debug-info=%t.dSYM %t.profdir/
+
+// RUN: diff <(llvm-profdata show --all-functions --counts %t.normal.profdata) <(llvm-profdata show --all-functions --counts %t.profdata)
+
+// RU

[PATCH] D158071: [clang] Remove rdar links; NFC

[Anna Zaks via Phabricator via cfe-commits]

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

Looks like @NoQ wetted the remaining code changes. The rest LGTM.

Thank you for preparing the patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158071

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

[PATCH] D158848: [clang][dataflow] Support range-for loops in fixpoint algorithm.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 553958.
ymandel added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158848

Files:
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -1612,4 +1612,21 @@
 
   });
 }
+
+TEST_F(TopTest, ForRangeStmtConverges) {
+  std::string Code = R"(
+void target(bool Foo) {
+  int Ints[10];
+  bool B = false;
+  for (int I : Ints)
+B = true;
+}
+  )";
+  runDataflow(Code,
+  [](const llvm::StringMap> &,
+ const AnalysisOutputs &) {
+// No additional expectations. We're only checking that the
+// analysis converged.
+  });
+}
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/ASTDumper.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/OperationKinds.h"
+#include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Analysis/Analyses/PostOrderCFGView.h"
 #include "clang/Analysis/CFG.h"
@@ -58,6 +59,7 @@
   case Stmt::WhileStmtClass:
   case Stmt::DoStmtClass:
   case Stmt::ForStmtClass:
+  case Stmt::CXXForRangeStmtClass:
 return true;
   default:
 return false;
@@ -108,6 +110,12 @@
 return {nullptr, false};
   }
 
+  TerminatorVisitorRetTy VisitCXXForRangeStmt(const CXXForRangeStmt *) {
+// Don't do anything special for CXXForRangeStmt, because the condition
+// (being implicitly generated) isn't visible from the loop body.
+return {nullptr, false};
+  }
+
   TerminatorVisitorRetTy VisitBinaryOperator(const BinaryOperator *S) {
 assert(S->getOpcode() == BO_LAnd || S->getOpcode() == BO_LOr);
 auto *LHS = S->getLHS();


Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -1612,4 +1612,21 @@
 
   });
 }
+
+TEST_F(TopTest, ForRangeStmtConverges) {
+  std::string Code = R"(
+void target(bool Foo) {
+  int Ints[10];
+  bool B = false;
+  for (int I : Ints)
+B = true;
+}
+  )";
+  runDataflow(Code,
+  [](const llvm::StringMap> &,
+ const AnalysisOutputs &) {
+// No additional expectations. We're only checking that the
+// analysis converged.
+  });
+}
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/ASTDumper.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/OperationKinds.h"
+#include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Analysis/Analyses/PostOrderCFGView.h"
 #include "clang/Analysis/CFG.h"
@@ -58,6 +59,7 @@
   case Stmt::WhileStmtClass:
   case Stmt::DoStmtClass:
   case Stmt::ForStmtClass:
+  case Stmt::CXXForRangeStmtClass:
 return true;
   default:
 return false;
@@ -108,6 +110,12 @@
 return {nullptr, false};
   }
 
+  TerminatorVisitorRetTy VisitCXXForRangeStmt(const CXXForRangeStmt *) {
+// Don't do anything special for CXXForRangeStmt, because the condition
+// (being implicitly generated) isn't visible from the loop body.
+return {nullptr, false};
+  }
+
   TerminatorVisitorRetTy VisitBinaryOperator(const BinaryOperator *S) {
 assert(S->getOpcode() == BO_LAnd || S->getOpcode() == BO_LOr);
 auto *LHS = S->getLHS();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153339: [clang] Support vectors in __builtin_isfpclass

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 553960.
sepavloff added a comment.

Fix documentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153339

Files:
  clang/docs/LanguageExtensions.rst
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/isfpclass.c

Index: clang/test/CodeGen/isfpclass.c
===
--- clang/test/CodeGen/isfpclass.c
+++ clang/test/CodeGen/isfpclass.c
@@ -14,7 +14,7 @@
 // CHECK-LABEL: define dso_local i1 @check_isfpclass_finite_strict
 // CHECK-SAME: (float noundef [[X:%.*]]) local_unnamed_addr #[[ATTR2:[0-9]+]] {
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f32(float [[X]], i32 504) #[[ATTR4:[0-9]+]]
+// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f32(float [[X]], i32 504) #[[ATTR6:[0-9]+]]
 // CHECK-NEXT:ret i1 [[TMP0]]
 //
 _Bool check_isfpclass_finite_strict(float x) {
@@ -35,7 +35,7 @@
 // CHECK-LABEL: define dso_local i1 @check_isfpclass_nan_f32_strict
 // CHECK-SAME: (float noundef [[X:%.*]]) local_unnamed_addr #[[ATTR2]] {
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f32(float [[X]], i32 3) #[[ATTR4]]
+// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f32(float [[X]], i32 3) #[[ATTR6]]
 // CHECK-NEXT:ret i1 [[TMP0]]
 //
 _Bool check_isfpclass_nan_f32_strict(float x) {
@@ -56,7 +56,7 @@
 // CHECK-LABEL: define dso_local i1 @check_isfpclass_snan_f64_strict
 // CHECK-SAME: (double noundef [[X:%.*]]) local_unnamed_addr #[[ATTR2]] {
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f64(double [[X]], i32 1) #[[ATTR4]]
+// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f64(double [[X]], i32 1) #[[ATTR6]]
 // CHECK-NEXT:ret i1 [[TMP0]]
 //
 _Bool check_isfpclass_snan_f64_strict(double x) {
@@ -77,7 +77,7 @@
 // CHECK-LABEL: define dso_local i1 @check_isfpclass_zero_f16_strict
 // CHECK-SAME: (half noundef [[X:%.*]]) local_unnamed_addr #[[ATTR2]] {
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f16(half [[X]], i32 96) #[[ATTR4]]
+// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f16(half [[X]], i32 96) #[[ATTR6]]
 // CHECK-NEXT:ret i1 [[TMP0]]
 //
 _Bool check_isfpclass_zero_f16_strict(_Float16 x) {
@@ -85,23 +85,88 @@
   return __builtin_isfpclass(x, 96 /*Zero*/);
 }
 
+// CHECK-LABEL: define dso_local i1 @check_isnan
+// CHECK-SAME: (float noundef [[X:%.*]]) local_unnamed_addr #[[ATTR2]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f32(float [[X]], i32 3) #[[ATTR6]]
+// CHECK-NEXT:ret i1 [[TMP0]]
+//
 _Bool check_isnan(float x) {
 #pragma STDC FENV_ACCESS ON
   return __builtin_isnan(x);
 }
 
+// CHECK-LABEL: define dso_local i1 @check_isinf
+// CHECK-SAME: (float noundef [[X:%.*]]) local_unnamed_addr #[[ATTR2]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f32(float [[X]], i32 516) #[[ATTR6]]
+// CHECK-NEXT:ret i1 [[TMP0]]
+//
 _Bool check_isinf(float x) {
 #pragma STDC FENV_ACCESS ON
   return __builtin_isinf(x);
 }
 
+// CHECK-LABEL: define dso_local i1 @check_isfinite
+// CHECK-SAME: (float noundef [[X:%.*]]) local_unnamed_addr #[[ATTR2]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f32(float [[X]], i32 504) #[[ATTR6]]
+// CHECK-NEXT:ret i1 [[TMP0]]
+//
 _Bool check_isfinite(float x) {
 #pragma STDC FENV_ACCESS ON
   return __builtin_isfinite(x);
 }
 
+// CHECK-LABEL: define dso_local i1 @check_isnormal
+// CHECK-SAME: (float noundef [[X:%.*]]) local_unnamed_addr #[[ATTR2]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = tail call i1 @llvm.is.fpclass.f32(float [[X]], i32 264) #[[ATTR6]]
+// CHECK-NEXT:ret i1 [[TMP0]]
+//
 _Bool check_isnormal(float x) {
 #pragma STDC FENV_ACCESS ON
   return __builtin_isnormal(x);
 }
 
+
+typedef float __attribute__((ext_vector_type(4))) float4;
+typedef double __attribute__((ext_vector_type(4))) double4;
+typedef int __attribute__((ext_vector_type(4))) int4;
+typedef long __attribute__((ext_vector_type(4))) long4;
+
+// CHECK-LABEL: define dso_local <4 x i32> @check_isfpclass_nan_v4f32
+// CHECK-SAME: (<4 x float> noundef [[X:%.*]]) local_unnamed_addr #[[ATTR3]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = fcmp uno <4 x float> [[X]], zeroinitializer
+// CHECK-NEXT:[[TMP1:%.*]] = zext <4 x i1> [[TMP0]] to <4 x i32>
+// CHECK-NEXT:ret <4 x i32> [[TMP1]]
+//
+int4 check_isfpclass_nan_v4f32(float4 x) {
+  return __builtin_isfpclass(x, 3 /*NaN*/);
+}
+
+// CHECK-LABEL: define dso_local <4 x i32> @check_isfpclass_nan_strict_v4f32
+// CHECK-SAME: (<4 x float> noundef [[X:%.*]]) local_unnamed_addr #[[ATTR2]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = tail call <4 x i1> @llvm.is.

[PATCH] D158995: [clang] Add a Windows build in the Clang pre-commit CI

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D158995#4621441 , @ldionne wrote:

> @aaron.ballman for awareness

Thank you for this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158995

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

[PATCH] D158929: [clang-tidy] Add exit code support to clang-tidy-diff.py

[Flash Sheridan via Phabricator via cfe-commits]

FlashSheridan added a comment.

A couple of low-priority suggestions from Pylint 3 at 326 and 95:




Comment at: clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py:95
 
-def start_workers(max_tasks, tidy_caller, task_queue, lock, timeout):
+def start_workers(max_tasks, tidy_caller, task_queue, lock, timeout, 
failed_files):
 for _ in range(max_tasks):

Pylint 3 says > “[R0913(too-many-arguments), start_workers] Too many arguments 
(6/5),” 

I think you just pushed start_workers() over the edge; personally, I don’t 
think a rewrite to reduce the number of arguments would improve clarity.



Comment at: clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py:326
+return_code = 0
+if len(failed_files):
+return_code = 1

Pylint 3 says: > [C1802(use-implicit-booleaness-not-len), main] Do not use 
len(SEQUENCE) without comparison to determine if a sequence is empty
  (I have no strong feelings.)




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158929

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

[PATCH] D158538: [MS-ABI] Remove comdat attribute for inheriting ctor.

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 added a comment.

Ping,

Our customer fond problem I submit bug: in 
https://github.com/llvm/llvm-project/issues/65045

This issue related with big change of 5179eb78210a2ad01a18c37b75048ccfe78414ac

Where internal linkage for inheriting ctor were set in change set at following 
code.  Could some one help to code review the change?  Or any suggestion on how 
to fix this in other ways?

Thank you so much for the help.
Jennifer

CodeGenModule.cpp
if (isa(D) &&

  cast(D)->isInheritingConstructor() &&
  Context.getTargetInfo().getCXXABI().isMicrosoft()) {
// Our approach to inheriting constructors is fundamentally different from
// that used by the MS ABI, so keep our inheriting constructor thunks
// internal rather than trying to pick an unambiguous mangling for them.
return llvm::GlobalValue::InternalLinkage;
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158538

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

[PATCH] D158641: [AArch64][Android][DRAFT] Fix FMV ifunc resolver usage on old Android APIs.

[Elliott Hughes via Phabricator via cfe-commits]

enh added inline comments.



Comment at: compiler-rt/lib/builtins/cpu_model.c:1382
+return;
+#if defined(__ANDROID__)
+  // ifunc resolvers don't have hwcaps in arguments on Android API lower

srhines wrote:
> MaskRay wrote:
> > enh wrote:
> > > ilinpv wrote:
> > > > MaskRay wrote:
> > > > > I am unfamiliar with how Android ndk builds compiler-rt.
> > > > > 
> > > > > If `__ANDROID_API__ >= 30`, shall we use the regular Linux code path?
> > > > I think that leads to shipping different compile-rt libraries depend on 
> > > > ANDROID_API. If this is an option to consider than runtime check 
> > > > android_get_device_api_level() < 30 can be replaced by `__ANDROID_API__ 
> > > > < 30`
> > > depends what you mean... in 10 years or so, yes, no-one is likely to 
> > > still care about the older API levels and we can just delete this. but 
> > > until then, no, there's _one_ copy of compiler-rt that everyone uses, and 
> > > although _OS developers_ don't need to support anything more than a 
> > > couple of years old, most app developers are targeting far lower API 
> > > levels than that (to maximize the number of possible customers).
> > > 
> > > TL;DR: "you could add that condition to the `#if`, but no-one would use 
> > > it for a decade". (and i think the comment and `if` below should make it 
> > > clear enough to future archeologists when this code block can be removed 
> > > :-) )
> > My thought was that people build Android with a specific `__ANDROID_API__`, 
> > and only systems >= this level are supported.
> > ```
> > #If __ANDROID_API__ < 30
> > ...
> > #endif
> > ```
> > 
> > This code has a greater chance to be removed when it becomes obsoleted. The 
> > argument is similar to how we find obsoleted GCC workarounds.
> Yes, the NDK currently just ships the oldest supported target API level for 
> compiler-rt, while the Android platform build does have access to both the 
> oldest supported target API level + the most recent target API level, so that 
> we can make better use of features there.
> 
> Maybe I'm missing something, but it's feeling like the NDK users won't be 
> able to make great use of FMV without us either bumping the minimum level or 
> shipping multiple runtimes (and then using the #ifdefs properly here).
> Maybe I'm missing something, but it's feeling like the NDK users won't be 
> able to make great use of FMV without us either bumping the minimum level or 
> shipping multiple runtimes (and then using the #ifdefs properly here).

yeah, that's the point of this code --- it's a runtime check so the NDK "just 
works".

but if people want the `__ANDROID_API__` `#if` too, that's fine. (and, like you 
say, the platform's two variants mean that we'll be testing both code paths, so 
i'm not worried about "one of these is the only one that anyone's actually 
building" problem.)

i have no opinion on whether anyone llvm is more/less likely to understand 
if/when `if (android_get_device_api_level() < 30)` versus `#if __ANDROID_API__ 
< 30` can be deleted.

i think the best argument for leaving this change as-is would be "anyone 
building their own is less likely to screw up" (since developers struggle all 
the time with the difference between "target" and "min" api, because the ndk 
terminology is different to the AndroidManifest.xml stuff that developers are 
more familiar with, which causes confusion). so if this was in libc++ (where i 
know people do build their own), i'd argue for the code as-is. but since it's 
compiler-rt (and i'm not aware that anyone's building that themselves) i don't 
think it matters either way?

to be clear, i'm imagining:
```
#if __ANDROID_API__ < 30
  if (android_get_device_api_level() < 30) {
setCPUFeature(FEAT_MAX);
return;
  }
#endif
```
(which brings us back to the "this is confusing" --- _just_ having the `#if` 
would be subtly different, which is why if i'd written this, i'd have written 
it as-is too!)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158641

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

[PATCH] D158803: [Sema][HLSL] Consolidate handling of HLSL attributes

[Justin Bogner via Phabricator via cfe-commits]

bogner updated this revision to Diff 553964.
bogner added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158803

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenHLSL/GlobalDestructors.hlsl
  clang/test/SemaHLSL/Semantics/entry_parameter.hlsl
  clang/test/SemaHLSL/Semantics/groupindex.hlsl
  clang/test/SemaHLSL/entry.hlsl
  clang/test/SemaHLSL/entry_shader_redecl.hlsl
  clang/test/SemaHLSL/num_threads.hlsl

Index: clang/test/SemaHLSL/num_threads.hlsl
===
--- clang/test/SemaHLSL/num_threads.hlsl
+++ clang/test/SemaHLSL/num_threads.hlsl
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s | FileCheck %s 
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-mesh -x hlsl -ast-dump -o - %s | FileCheck %s 
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-amplification -x hlsl -ast-dump -o - %s | FileCheck %s 
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -ast-dump -o - %s | FileCheck %s 
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-mesh -x hlsl -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-amplification -x hlsl -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -ast-dump -o - %s | FileCheck %s
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-pixel -x hlsl -ast-dump -o - %s -verify
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-vertex -x hlsl -ast-dump -o - %s -verify
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-hull -x hlsl -ast-dump -o - %s -verify
@@ -97,14 +97,20 @@
   return 1;
 }
 
+[numthreads(4,2,1)]
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}}  4 2 1
+int onlyOnForwardDecl();
+
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}}  Inherited 4 2 1
+int onlyOnForwardDecl() {
+  return 1;
+}
 
 #else // Vertex and Pixel only beyond here
-// expected-error-re@+1 {{attribute 'numthreads' is unsupported in {{[A-Za-z]+}} shaders, requires Compute, Amplification, Mesh or Library}}
+// expected-error-re@+1 {{attribute 'numthreads' is unsupported in {{[A-Za-z]+}} shaders, requires compute, amplification, or mesh}}
 [numthreads(1,1,1)]
 int main() {
  return 1;
 }
 
 #endif
-
-
Index: clang/test/SemaHLSL/entry_shader_redecl.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/entry_shader_redecl.hlsl
@@ -0,0 +1,75 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry cs1 -o - %s -ast-dump -verify | FileCheck -DSHADERFN=cs1 -check-prefix=CHECK-ENV %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry cs2 -o - %s -ast-dump -verify | FileCheck -DSHADERFN=cs2 -check-prefix=CHECK-ENV %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry cs3 -o - %s -ast-dump -verify | FileCheck -DSHADERFN=cs3 -check-prefix=CHECK-ENV %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s -ast-dump -verify | FileCheck -check-prefix=CHECK-LIB %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3 -x hlsl -o - %s -ast-dump -verify | FileCheck -check-prefix=CHECK-LIB %s
+
+// expected-no-diagnostics
+
+// CHECK-ENV: FunctionDecl [[PROTO:0x[0-9a-f]+]] {{.*}} [[SHADERFN]] 'void ()'
+// CHECK-ENV: FunctionDecl 0x{{.*}} prev [[PROTO]] {{.*}} [[SHADERFN]] 'void ()'
+// CHECK-ENV-NEXT: CompoundStmt 0x
+// CHECK-ENV-NEXT: HLSLNumThreadsAttr 0x
+// CHECK-ENV-NEXT: HLSLShaderAttr 0x{{.*}} Implicit Compute
+void cs1();
+[numthreads(1,1,1)] void cs1() {}
+[numthreads(1,1,1)] void cs2();
+void cs2() {}
+[numthreads(1,1,1)] void cs3();
+[numthreads(1,1,1)] void cs3() {}
+
+// CHECK-LIB: FunctionDecl [[PROTO:0x[0-9a-f]+]] {{.*}} s1 'void ()'
+// CHECK-LIB: FunctionDecl 0x{{.*}} prev [[PROTO]] {{.*}} s1 'void ()'
+// CHECK-LIB-NEXT: CompoundStmt 0x
+// CHECK-LIB-NEXT: HLSLShaderAttr 0x{{.*}} Compute
+// CHECK-LIB-NEXT: HLSLNumThreadsAttr 0x
+void s1();
+[shader("compute"), numthreads(1,1,1)] void s1() {}
+
+// CHECK-LIB: FunctionDecl [[PROTO:0x[0-9a-f]+]] {{.*}} s2 'void ()'
+// CHECK-LIB: FunctionDecl 0x{{.*}} prev [[PROTO]] {{.*}} s2 'void ()'
+// CHECK-LIB-NEXT: CompoundStmt 0x
+// CHECK-LIB-NEXT: HLSLShaderAttr 0x{{.*}} Compute
+// CHECK-LIB-NEXT: HLSLNumThreadsAttr 0x
+[shader("compute")] void s2();
+[shader("compute"), numthreads(1,1,1)] void s2() {}
+
+// CHECK-LIB: FunctionDecl [[PROTO:0x[0-9a-f]+]] {{.*}} s3 'void ()'
+// CHECK-LIB: FunctionDecl 0x{{.*}} prev [[PROTO]] {{.*}} s3 'void ()'
+// CHECK-LIB-NEXT: CompoundStmt 0x
+// CHECK-LIB-NEXT: HLSLShaderAttr 0x{{.*}} Compute
+// CHECK-LIB-NE

[PATCH] D158071: [clang] Remove rdar links; NFC

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D158071#4621631 , @zaks.anna wrote:

> Looks like @NoQ wetted the remaining code changes. The rest LGTM.
>
> Thank you for preparing the patch!

Thank you for the help with review (to both you and @NoQ)! I've landed the 
changes but anyone who wants to add more context from any of the former links 
is encouraged to do so with NFC commits as they see fit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158071

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

[PATCH] D159000: Reland "[Profile] Allow online merging with debug info correlation."

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

looks fine to me but I'll let ellis lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159000

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

[PATCH] D158803: [Sema][HLSL] Consolidate handling of HLSL attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:3008-3011
+  void ActOnHLSLTopLevelFunction(FunctionDecl *FD);
   void CheckHLSLEntryPoint(FunctionDecl *FD);
+  void CheckHLSLSemanticAnnotation(FunctionDecl *EntryPoint, Decl *Param,
+   HLSLAnnotationAttr *AnnotationAttr);

Any const correctness you can add here to the pointees would be appreciated.



Comment at: clang/lib/Sema/SemaDecl.cpp:12375
+  if (HLSLShaderAttr::ConvertStrToShaderType(Env, ShaderType)) {
+if (HLSLShaderAttr *Shader = FD->getAttr()) {
+  // The entry point is already annotated - check that it matches the

`const auto *`



Comment at: clang/lib/Sema/SemaDecl.cpp:12395-12398
+  // TODO: This should probably just be llvm_unreachable and we should
+  // reject triples with random ABIs and such when we build the target.
+  // For now, crash.
+  llvm::report_fatal_error("Unhandled environment in triple");

Hmmm, is this going to be done as a follow-up relatively soon? 
`report_fatal_error` isn't acceptable in lieu of actual diagnostic reporting, 
so this should either be fixed in this patch or Really Soon After™ it lands.



Comment at: clang/lib/Sema/SemaDecl.cpp:12420
+  case HLSLShaderAttr::Callable:
+if (auto *NT = FD->getAttr()) {
+  Diag(NT->getLoc(), diag::err_hlsl_attr_unsupported_in_stage)

`const auto *`



Comment at: clang/lib/Sema/SemaDecl.cpp:12423
+  << NT << HLSLShaderAttr::ConvertShaderTypeToStr(ST)
+  << "compute, amplification, or mesh";
+  FD->setInvalidDecl();

This should be part of the diagnostic wording itself (using `%select`) rather 
than passed in as a string argument. The same suggestion applies elsewhere -- 
we want to keep English strings out of the streaming arguments as much as 
possible to make localization efforts possible.



Comment at: clang/lib/Sema/SemaDecl.cpp:12439-12440
 
-  for (const auto *Param : FD->parameters()) {
-if (!Param->hasAttr()) {
+  for (ParmVarDecl *Param : FD->parameters()) {
+if (auto *AnnotationAttr = Param->getAttr()) {
+  CheckHLSLSemanticAnnotation(FD, Param, AnnotationAttr);

const the pointees?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158803

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

[PATCH] D61670: [clang] [MinGW] Add the option -fno-autoimport

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

I don't have all the context here, but seems fine once the commit description 
is updated with the new spelling


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61670

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

[PATCH] D158615: [clang] Emit an error if variable ends up with incomplete array type

[Shafik Yaghmour via Phabricator via cfe-commits]

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

I Think this change makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158615

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

[PATCH] D159000: Reland "[Profile] Allow online merging with debug info correlation."

[Ellis Hoag via Phabricator via cfe-commits]

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

Thanks for adding the test case! Can you also link the original diff in the 
summary?




Comment at: compiler-rt/test/profile/instrprof-merge-empty-data.test:1
+// Test 
+// RUN: rm -rf %t.dir && split-file %s %t.dir && cd %t.dir

Please add a more descriptive explanation for this test 😜


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159000

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

[PATCH] D158995: [clang] Add a Windows build in the Clang pre-commit CI

[Louis Dionne via Phabricator via cfe-commits]

ldionne updated this revision to Diff 553968.
ldionne added a comment.

Run script through bash on Windows.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158995

Files:
  clang/utils/ci/buildkite-pipeline.yml
  clang/utils/ci/run-buildbot


Index: clang/utils/ci/run-buildbot
===
--- clang/utils/ci/run-buildbot
+++ clang/utils/ci/run-buildbot
@@ -95,6 +95,24 @@
 
 ninja -C ${BUILD_DIR} check-clang
 ;;
+build-clang-windows)
+cmake -S llvm -B ${BUILD_DIR} -G Ninja 
 \
+-D CMAKE_C_COMPILER_LAUNCHER=sccache   
 \
+-D CMAKE_CXX_COMPILER_LAUNCHER=sccache 
 \
+-D CMAKE_BUILD_TYPE=Release
 \
+-D CMAKE_INSTALL_PREFIX=install-windows
 \
+-D LLVM_ENABLE_PROJECTS="clang;compiler-rt"
 \
+-D LLVM_ENABLE_ASSERTIONS=ON   
 \
+-D LLVM_BUILD_EXAMPLES=ON  
 \
+-D COMPILER_RT_BUILD_LIBFUZZER=OFF 
 \
+-D COMPILER_RT_BUILD_ORC=OFF
+
+ninja -C ${BUILD_DIR} install-clang install-clang-resource-headers
+tar -cJvf install-windows.tar.xz install-windows/
+buildkite-agent artifact upload --debug install-windows.tar.xz
+
+ninja -C ${BUILD_DIR} check-clang
+;;
 generic-cxx03)
 buildkite-agent artifact download install.tar.xz .
 tar -xvf install.tar.xz
Index: clang/utils/ci/buildkite-pipeline.yml
===
--- clang/utils/ci/buildkite-pipeline.yml
+++ clang/utils/ci/buildkite-pipeline.yml
@@ -31,7 +31,7 @@
 
   - wait
 
-  - label: "Building and testing clang"
+  - label: "Building and testing clang (Linux)"
 commands:
   - "clang/utils/ci/run-buildbot build-clang"
 agents:
@@ -42,6 +42,17 @@
   limit: 2
 timeout_in_minutes: 120
 
+  - label: "Building and testing clang (Windows)"
+commands:
+  - "bash clang/utils/ci/run-buildbot build-clang-windows"
+agents:
+  queue: "windows"
+retry:
+  automatic:
+- exit_status: -1  # Agent was lost
+  limit: 2
+timeout_in_minutes: 120
+
   - wait
 
   - label: "Running libc++ test suite in C++03"


Index: clang/utils/ci/run-buildbot
===
--- clang/utils/ci/run-buildbot
+++ clang/utils/ci/run-buildbot
@@ -95,6 +95,24 @@
 
 ninja -C ${BUILD_DIR} check-clang
 ;;
+build-clang-windows)
+cmake -S llvm -B ${BUILD_DIR} -G Ninja  \
+-D CMAKE_C_COMPILER_LAUNCHER=sccache\
+-D CMAKE_CXX_COMPILER_LAUNCHER=sccache  \
+-D CMAKE_BUILD_TYPE=Release \
+-D CMAKE_INSTALL_PREFIX=install-windows \
+-D LLVM_ENABLE_PROJECTS="clang;compiler-rt" \
+-D LLVM_ENABLE_ASSERTIONS=ON\
+-D LLVM_BUILD_EXAMPLES=ON   \
+-D COMPILER_RT_BUILD_LIBFUZZER=OFF  \
+-D COMPILER_RT_BUILD_ORC=OFF
+
+ninja -C ${BUILD_DIR} install-clang install-clang-resource-headers
+tar -cJvf install-windows.tar.xz install-windows/
+buildkite-agent artifact upload --debug install-windows.tar.xz
+
+ninja -C ${BUILD_DIR} check-clang
+;;
 generic-cxx03)
 buildkite-agent artifact download install.tar.xz .
 tar -xvf install.tar.xz
Index: clang/utils/ci/buildkite-pipeline.yml
===
--- clang/utils/ci/buildkite-pipeline.yml
+++ clang/utils/ci/buildkite-pipeline.yml
@@ -31,7 +31,7 @@
 
   - wait
 
-  - label: "Building and testing clang"
+  - label: "Building and testing clang (Linux)"
 commands:
   - "clang/utils/ci/run-buildbot build-clang"
 agents:
@@ -42,6 +42,17 @@
   limit: 2
 timeout_in_minutes: 120
 
+  - label: "Building and testing clang (Windows)"
+commands:
+  - "bash clang/utils/ci/run-buildbot build-clang-windows"
+agents:
+  queue: "windows"
+retry:
+  automatic:
+- exit_status: -1  # Agent was lost
+  limit: 2
+timeout_in_minutes: 120
+
   - wait
 
   - label: "Running libc++ test suite in C++03"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158803: [Sema][HLSL] Consolidate handling of HLSL attributes

[Justin Bogner via Phabricator via cfe-commits]

bogner added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:12395-12398
+  // TODO: This should probably just be llvm_unreachable and we should
+  // reject triples with random ABIs and such when we build the target.
+  // For now, crash.
+  llvm::report_fatal_error("Unhandled environment in triple");

aaron.ballman wrote:
> Hmmm, is this going to be done as a follow-up relatively soon? 
> `report_fatal_error` isn't acceptable in lieu of actual diagnostic reporting, 
> so this should either be fixed in this patch or Really Soon After™ it lands.
Yeah, I'm happy to go and tackle that this week. Really I want an 
assert/unreachable here but since it is in fact reachable at the moment this 
seemed better as a temporary fix. Note that before this patch we just index out 
of bounds of an array and get all of the fun benefits of UB.



Comment at: clang/lib/Sema/SemaDecl.cpp:12423
+  << NT << HLSLShaderAttr::ConvertShaderTypeToStr(ST)
+  << "compute, amplification, or mesh";
+  FD->setInvalidDecl();

aaron.ballman wrote:
> This should be part of the diagnostic wording itself (using `%select`) rather 
> than passed in as a string argument. The same suggestion applies elsewhere -- 
> we want to keep English strings out of the streaming arguments as much as 
> possible to make localization efforts possible.
That's a good point, though in this case the words we want are the value to the 
`compute(...)` attribute or part of the triple, so translating them wouldn't 
really make sense. Would it be better to do something using 
`ConvertShaderTypeToStr`? Also I'll probably update the error message text to 
quote the other use of ConvertShaderTypeToStr so it's clear that it isn't just 
a random word.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158803

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

[clang] d4a9121 - Reland "[clang][modules] Use relative offsets for input files"

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2023-08-28T09:54:39-07:00
New Revision: d4a912153488308ea9ab96a0267150460cdfc697

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

LOG: Reland "[clang][modules] Use relative offsets for input files"

This reverts commit 870f1583eed0f22ddfb1a1979a90198c3dc09927, effectively 
relanding commit b9d78bdc730b2fcfe029a7579c24020536c3fa25.

Added: 


Modified: 
clang/include/clang/Serialization/ModuleFile.h
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp

Removed: 




diff  --git a/clang/include/clang/Serialization/ModuleFile.h 
b/clang/include/clang/Serialization/ModuleFile.h
index 8c4067853d6646..0af5cae6aebc37 100644
--- a/clang/include/clang/Serialization/ModuleFile.h
+++ b/clang/include/clang/Serialization/ModuleFile.h
@@ -245,7 +245,10 @@ class ModuleFile {
   /// The cursor to the start of the input-files block.
   llvm::BitstreamCursor InputFilesCursor;
 
-  /// Offsets for all of the input file entries in the AST file.
+  /// Absolute offset of the start of the input-files block.
+  uint64_t InputFilesOffsetBase = 0;
+
+  /// Relative offsets for all of the input file entries in the AST file.
   const llvm::support::unaligned_uint64_t *InputFileOffsets = nullptr;
 
   /// The input files that have been loaded from this AST file.

diff  --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index 9491733d47df3d..82dac5fbfc888d 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2326,7 +2326,8 @@ InputFileInfo ASTReader::getInputFileInfo(ModuleFile &F, 
unsigned ID) {
   // Go find this input file.
   BitstreamCursor &Cursor = F.InputFilesCursor;
   SavedStreamPosition SavedPosition(Cursor);
-  if (llvm::Error Err = Cursor.JumpToBit(F.InputFileOffsets[ID - 1])) {
+  if (llvm::Error Err = Cursor.JumpToBit(F.InputFilesOffsetBase +
+ F.InputFileOffsets[ID - 1])) {
 // FIXME this drops errors on the floor.
 consumeError(std::move(Err));
   }
@@ -2410,7 +2411,8 @@ InputFile ASTReader::getInputFile(ModuleFile &F, unsigned 
ID, bool Complain) {
   // Go find this input file.
   BitstreamCursor &Cursor = F.InputFilesCursor;
   SavedStreamPosition SavedPosition(Cursor);
-  if (llvm::Error Err = Cursor.JumpToBit(F.InputFileOffsets[ID - 1])) {
+  if (llvm::Error Err = Cursor.JumpToBit(F.InputFilesOffsetBase +
+ F.InputFileOffsets[ID - 1])) {
 // FIXME this drops errors on the floor.
 consumeError(std::move(Err));
   }
@@ -2788,6 +2790,7 @@ ASTReader::ReadControlBlock(ModuleFile &F,
   Error("malformed block record in AST file");
   return Failure;
 }
+F.InputFilesOffsetBase = F.InputFilesCursor.GetCurrentBitNo();
 continue;
 
   case OPTIONS_BLOCK_ID:
@@ -5328,6 +5331,7 @@ bool ASTReader::readASTFileControlBlock(
   bool NeedsSystemInputFiles = Listener.needsSystemInputFileVisitation();
   bool NeedsImports = Listener.needsImportVisitation();
   BitstreamCursor InputFilesCursor;
+  uint64_t InputFilesOffsetBase = 0;
 
   RecordData Record;
   std::string ModuleDir;
@@ -5363,6 +5367,7 @@ bool ASTReader::readASTFileControlBlock(
 if (NeedsInputFiles &&
 ReadBlockAbbrevs(InputFilesCursor, INPUT_FILES_BLOCK_ID))
   return true;
+InputFilesOffsetBase = InputFilesCursor.GetCurrentBitNo();
 break;
 
   default:
@@ -5435,7 +5440,8 @@ bool ASTReader::readASTFileControlBlock(
 
 BitstreamCursor &Cursor = InputFilesCursor;
 SavedStreamPosition SavedPosition(Cursor);
-if (llvm::Error Err = Cursor.JumpToBit(InputFileOffs[I])) {
+if (llvm::Error Err =
+Cursor.JumpToBit(InputFilesOffsetBase + InputFileOffs[I])) {
   // FIXME this drops errors on the floor.
   consumeError(std::move(Err));
 }

diff  --git a/clang/lib/Serialization/ASTWriter.cpp 
b/clang/lib/Serialization/ASTWriter.cpp
index 22420aab9ca08c..2a12779b0d7d12 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1570,6 +1570,8 @@ void ASTWriter::WriteInputFiles(SourceManager &SourceMgr,
   IFHAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
   unsigned IFHAbbrevCode = Stream.EmitAbbrev(std::move(IFHAbbrev));
 
+  uint64_t InputFilesOffsetBase = Stream.GetCurrentBitNo();
+
   // Get all ContentCache objects for files.
   std::vector UserFiles;
   std::vector SystemFiles;
@@ -1633,7 +1635,7 @@ void ASTWriter::WriteInputFiles(SourceManager &SourceMgr,
   continue; // already recorded this file.
 
 // Record this entry's offset.
-InputFileOffsets.push_back(Stream.GetCurrentBitNo());
+

[clang] 6fb08d8 - Reland "[clang][modules] Move `UNHASHED_CONTROL_BLOCK` up in the AST file"

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2023-08-28T09:54:39-07:00
New Revision: 6fb08d8f558a6f28db7835acdb88cab83aea2eb4

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

LOG: Reland "[clang][modules] Move `UNHASHED_CONTROL_BLOCK` up in the AST file"

This reverts commit b6ba804f7775f89f230ee1e62526a2f8225c7966, effectively 
relanding commit 7d1565727dad3acb54fe76a908630843835d7bc8.

The original commit incorrectly called `ASTWriter::writeUnhashedControlBlock()` 
before `ASTWriter::collectNonAffectingInputFiles()`, causing 
SourceLocations/FileIDs in the pragma diagnostic mappings block to be invalid. 
This is now tested by `clang/test/Modules/diag-mappings-affecting.c`.

Added: 
clang/test/Modules/diag-mappings-affecting.c

Modified: 
clang/include/clang/Basic/Module.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/include/clang/Serialization/ASTWriter.h
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Serialization/GlobalModuleIndex.cpp
clang/test/Modules/ASTSignature.c

Removed: 




diff  --git a/clang/include/clang/Basic/Module.h 
b/clang/include/clang/Basic/Module.h
index a4ad8ad2f768fe..7f1eb6faecf4e7 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -81,6 +81,12 @@ struct ASTFileSignature : std::array {
 return Sentinel;
   }
 
+  static ASTFileSignature createDummy() {
+ASTFileSignature Dummy;
+Dummy.fill(0x00);
+return Dummy;
+  }
+
   template 
   static ASTFileSignature create(InputIt First, InputIt Last) {
 assert(std::distance(First, Last) == size &&

diff  --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index dcc7bdf9b4eaac..9e115f2a5cce3f 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -41,7 +41,7 @@ namespace serialization {
 /// Version 4 of AST files also requires that the version control branch and
 /// revision match exactly, since there is no backward compatibility of
 /// AST files at this time.
-const unsigned VERSION_MAJOR = 28;
+const unsigned VERSION_MAJOR = 29;
 
 /// AST file minor version number supported by this version of
 /// Clang.

diff  --git a/clang/include/clang/Serialization/ASTWriter.h 
b/clang/include/clang/Serialization/ASTWriter.h
index b9d197c15c1344..f2c7c03ff09360 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -128,10 +128,17 @@ class ASTWriter : public ASTDeserializationListener,
   /// The module we're currently writing, if any.
   Module *WritingModule = nullptr;
 
-  /// The offset of the first bit inside the AST_BLOCK.
+  /// The byte range representing all the UNHASHED_CONTROL_BLOCK.
+  std::pair UnhashedControlBlockRange;
+  /// The bit offset of the AST block hash blob.
+  uint64_t ASTBlockHashOffset = 0;
+  /// The bit offset of the signature blob.
+  uint64_t SignatureOffset = 0;
+
+  /// The bit offset of the first bit inside the AST_BLOCK.
   uint64_t ASTBlockStartOffset = 0;
 
-  /// The range representing all the AST_BLOCK.
+  /// The byte range representing all the AST_BLOCK.
   std::pair ASTBlockRange;
 
   /// The base directory for any relative paths we emit.
@@ -495,12 +502,11 @@ class ASTWriter : public ASTDeserializationListener,
  StringRef isysroot);
 
   /// Write out the signature and diagnostic options, and return the signature.
-  ASTFileSignature writeUnhashedControlBlock(Preprocessor &PP,
- ASTContext &Context);
+  void writeUnhashedControlBlock(Preprocessor &PP, ASTContext &Context);
+  ASTFileSignature backpatchSignature();
 
   /// Calculate hash of the pcm content.
-  static std::pair
-  createSignature(StringRef AllBytes, StringRef ASTBlockBytes);
+  std::pair createSignature() const;
 
   void WriteInputFiles(SourceManager &SourceMgr, HeaderSearchOptions &HSOpts);
   void WriteSourceManagerBlock(SourceManager &SourceMgr,

diff  --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index 82dac5fbfc888d..e707f346a2c652 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -4734,12 +4734,6 @@ ASTReader::ReadASTCore(StringRef FileName,
   ShouldFinalizePCM = true;
   return Success;
 
-case UNHASHED_CONTROL_BLOCK_ID:
-  // This block is handled using look-ahead during ReadControlBlock.  We
-  // shouldn't get here!
-  Error("malformed block record in AST file");
-  return Failure;
-
 default:
   if (llvm::Error Err = Stream.SkipBlock()) {
 Error(std::move(Err));
@@ -4859,13 +4853,18 @@ ASTReader::ASTReadResult 

[clang] b9b81e3 - [clang][modules] NFC: Use local instead of `ModuleFile` member

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2023-08-28T09:54:39-07:00
New Revision: b9b81e37fe63d661afa356eacc97208ccc85da29

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

LOG: [clang][modules] NFC: Use local instead of `ModuleFile` member

Added: 


Modified: 
clang/lib/Serialization/ASTReader.cpp

Removed: 




diff  --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index e707f346a2c652..b087af25025a04 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -6531,8 +6531,7 @@ void 
ASTReader::ReadPragmaDiagnosticMappings(DiagnosticsEngine &Diag) {
 // Read the final state.
 assert(Idx < Record.size() &&
"Invalid data, missing final pragma diagnostic state");
-SourceLocation CurStateLoc =
-ReadSourceLocation(F, F.PragmaDiagMappings[Idx++]);
+SourceLocation CurStateLoc = ReadSourceLocation(F, Record[Idx++]);
 auto *CurState = ReadDiagState(*FirstState, false);
 
 if (!F.isModule()) {



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

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

[Louis Dionne via Phabricator via cfe-commits]

ldionne accepted this revision.
ldionne added a comment.

Sorry for missing a couple of direct pings. I have no concerns with the patch, 
perhaps I should not have marked my review as blocking in the first place. 
Thanks for doing this work, it's about time we updated this default!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112921

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

[PATCH] D157572: [clang] Add `[[clang::library_extension]]` attribute

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D157572#4617504 , @aaron.ballman 
wrote:

> In D157572#4614561 , @cjdb wrote:
>
>> In D157572#4613622 , 
>> @aaron.ballman wrote:
>>
>>> In D157572#4612141 , @cjdb wrote:
>>>
 I don't dislike it, but I am a bit concerned about misuse being noisy.
>>>
>>> So you're concerned that a library author uses `diagnose_if` to add a 
>>> diagnostic to a warning group that makes the diagnostic seem too chatty, so 
>>> the user disables the group and loses the compiler's diagnostics? Or are 
>>> there other kinds of misuse you're worried about?
>>
>> More or less the former. I don't know if it'll actually manifest in practice 
>> though, I don't see many `diagnose_if` diagnostics to begin with.
>
> I think the former is sort of a "doctor, doctor, it hurts" situation; the 
> issue isn't really with `diagnose_if`, it's with the library author's use of 
> it. But at the same time, I can see why it would be beneficial to be able to 
> work around it if needed.
>
 As much as I hate suppressing diagnostics, I think there needs to be a way 
 to suppress the `diagnose_if` forms of warning without suppressing 
 something that the compiler would otherwise generate. Something like:

 - `-Wno-deprecated`: suppresses anything that `-Wdeprecated` would turn on.
 - `-Wno-deprecated=diagnose_if`: just the ones flagged by `diagnose_if`.
 - `-Wno-deprecated=non-diagnose_if`: complement to #2.

 (and similarly for `-Wno-error=`.)

 I'm not sure about the system header knob though: `[[deprecated]]` and 
 `[[nodiscard]]` still show up even when the declaration is in a system 
 header?
>>>
>>> Correct, those will still show up when the declaration is in a system 
>>> header but not when the use is in a system header: 
>>> https://godbolt.org/z/PjqKbGsrr
>>
>> Right, my question was more "what is this knob doing?"
>
> Ah! We have various knobs on our diagnostics, like:
> `ShowInSystemHeader` -- 
> https://github.com/llvm/llvm-project/blob/2dc6281b98d07f43a64d0ef34405d9a12d59e8b6/clang/include/clang/Basic/DiagnosticSemaKinds.td#L7818
>  (if used, the diagnostic will be shown in a system header)
> `SFINAEFailure` -- 
> https://github.com/llvm/llvm-project/blob/2dc6281b98d07f43a64d0ef34405d9a12d59e8b6/clang/include/clang/Basic/DiagnosticSemaKinds.td#L93
>  (if used, the diagnostic causes SFINAE to fail in a SFINAE context)
> `DefaultIgnore` -- 
> https://github.com/llvm/llvm-project/blob/2dc6281b98d07f43a64d0ef34405d9a12d59e8b6/clang/include/clang/Basic/DiagnosticSemaKinds.td#L141
>  (if used, the warning is off by default and must be explicitly enabled via 
> its group)
> `DefaultError` -- 
> https://github.com/llvm/llvm-project/blob/2dc6281b98d07f43a64d0ef34405d9a12d59e8b6/clang/include/clang/Basic/DiagnosticSemaKinds.td#L262
>  (if used, the warning defaults to being an error but users can disable the 
> error with `-Wno`)
> (etc)
>
> All of these have been of use to us as implementers, so it seems likely that 
> these same knobs would be of use to library authors adding their own compiler 
> diagnostics, so perhaps we should consider that as part of the design?
>
>>> We currently have `-Wuser-defined-warnings` as the warning group for 
>>> `diagnose_if` warning diagnostics, so I wonder if it would make sense to 
>>> allow `-Wno-deprecated` suppresses anything that `-Wdeprecated` would turn 
>>> on, while `-Wdeprecated -Wno-user-defined-warnings` would turn on only the 
>>> compiler-generated deprecation warnings and not the diagnose_if-generated 
>>> ones?
>>
>> Oh neat, this simplifies things a lot!
>
> I think it could make for a reasonable user experience.
>
> I'm curious what @erichkeane thinks as attributes code owner, but personally, 
> I like the idea of extending `diagnose_if` over the idea of adding 
> `clang::library_extension` because it's a more general solution that I think 
> will give more utility to our users. However, I also want to know if @philnik 
> @Mordante @ldionne (and others) share that preference because I think they're 
> going to be the guinea pigs^W^Wearly adopters of this functionality.

I like this idea, but we need to acknowledge that it'll potentially lead to 
diagnostics with varying quality. This conversation may be better suited to a 
Discourse thread at this point: there seems to be a //lot// of design before we 
reach consenus, and a CL isn't the optimal place to do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157572

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

[PATCH] D156616: [clang-tidy] Fix c_str() removal and cast addition when re-ordering arguments

[Mike Crowe via Phabricator via cfe-commits]

mikecrowe added a comment.

@PiotrZSL, I think that this is quite an important fix since without it the 
check completely mangles the code. Should it be put in the 17.x release branch 
too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156616

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

[PATCH] D158787: [clang-tidy][readability] add Leading_upper_snake_case

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

In D158787#4621587 , @MasterCopy8GB 
wrote:

> Hello @PiotrZSL, I do not have rights to commit to the LLVM repository. Could 
> you please commit these changes for me? Thank you for all of your time and 
> effort!

Under which user ? Name / Email ?


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

https://reviews.llvm.org/D158787

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

[PATCH] D158787: [clang-tidy][readability] add Leading_upper_snake_case

[Steven Lewis via Phabricator via cfe-commits]

MasterCopy8GB added a comment.

In D158787#4621915 , @PiotrZSL wrote:

> In D158787#4621587 , @MasterCopy8GB 
> wrote:
>
>> Hello @PiotrZSL, I do not have rights to commit to the LLVM repository. 
>> Could you please commit these changes for me? Thank you for all of your time 
>> and effort!
>
> Under which user ? Name / Email ?

MasterCopy8GB / gdayman...@hotmail.com Please


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

https://reviews.llvm.org/D158787

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

[PATCH] D157572: [clang] Add `[[clang::library_extension]]` attribute

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

Started a thread: 
https://discourse.llvm.org/t/exposing-the-diagnostic-engine-to-c/73092


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157572

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

[PATCH] D158688: [Driver,ARM,AArch64] Ignore -mbranch-protection= diagnostics for assembler input

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Ping:) This is probably a good candidate for `release/17.x` backporting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158688

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

[PATCH] D158538: [MS-ABI] Remove comdat attribute for inheriting ctor.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

It took me a while to find this code to understand the issue. The problem is 
that there is an inconsistency between the GVA linkage and the LLVM IR linkage 
for these constructors. In this godbolt example 
, `??0?$TInputDataVertexModel..` is marked 
`internal` and marked `comdat`, and that is uncommon. PGO instrumentation 
appears to do the wrong thing in that case. Is that more or less what's 
happening?

The suggested fix here is to move this logic up from CodeGen into the earlier, 
"GVA level" linkage calculation, so we don't mark this thing comdat, right? The 
logic was added in this 2016 change from @rsmith :
https://github.com/llvm/llvm-project/commit/5179eb78210a2#diff-e724febedab9c1a2832bf2056d208ff02ddcb2e6f90b5a653afc9b19ac78a5d7R768

Ideally, I'd like to come up with our own Clang mangling and emit a thunk with 
`linkonce_odr` linkage or `GVA_DisardableODR` linkage, which means we need to 
understand why Richard thought this was necessary earlier.

Before we do that, you should be able to delete the logic in 
CodeGenModule::getFunctionLinkage to handle inheriting constructor thunks. Your 
change should have the same effect. Can you do that, and check that it passes 
tests? If we do that, we're don't have to accumulate extra special case code, 
we've just moved the special case code earlier, up from codegen into the 
earlier linkage calculation.




Comment at: clang/test/CodeGenCXX/ms-inheriting-ctor.cpp:41
+
+// CHECK-LABEL: define internal noundef ptr 
@"??0?$B@_N@@QEAA@AEBVF@@AEBUA@@@Z"(ptr noundef nonnull returned align 1 
dereferenceable(1) %this, ptr noundef nonnull align 1 dereferenceable(1) %0, 
ptr noundef nonnull align 1 dereferenceable(1) %1) unnamed_addr #2 align 2
+// CHECK-LABEL: define linkonce_odr dso_local noundef ptr 
@"??0?$c@_NUbQEAA@AEBVF@@AEBUA@@@Z"(ptr noundef nonnull returned align 1 
dereferenceable(1) %this, ptr noundef nonnull align 1 dereferenceable(1) %p1, 
ptr noundef nonnull align 1 dereferenceable(1) %d) unnamed_addr #2 comdat align 
2

To make this less fragile, can you come up with a way to use `CHECK-NOT: 
comdat` since that's the key thing we're testing for here? You will need some 
subsequent anchor like `entry:` or something else.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158538

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

[PATCH] D156616: [clang-tidy] Fix c_str() removal and cast addition when re-ordering arguments

[Piotr Zegar via Phabricator via cfe-commits]

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

In D156616#4621914 , @mikecrowe wrote:

> @PiotrZSL, I think that this is quite an important fix since without it the 
> check completely mangles the code. Should it be put in the 17.x release 
> branch too?

If issue happens only when StrictMode is set to True, then I wouldn't worry too 
much, and then this could wait for Clang 18 that is just half year away.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156616

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

[clang] d809920 - Reland "[Profile] Allow online merging with debug info correlation."

[Zequan Wu via cfe-commits]

Author: Zequan Wu
Date: 2023-08-28T13:32:55-04:00
New Revision: d80992032fd0d7da70b2bd1a59066703c3c21fbb

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

LOG: Reland "[Profile] Allow online merging with debug info correlation."

Original patch: D157632.

Fixed the issue when data is 0 by relaxing the condition.

Reviewed By: ellis

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

Added: 
compiler-rt/test/profile/instrprof-merge-empty-data.test

Modified: 
clang/lib/CodeGen/BackendUtil.cpp
compiler-rt/lib/profile/InstrProfilingMerge.c
compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
compiler-rt/test/profile/instrprof-merge-error.c

Removed: 




diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index 373d38672284d6..3e8b2b78a3928b 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -102,7 +102,7 @@ namespace {
 
 // Default filename used for profile generation.
 std::string getDefaultProfileGenName() {
-  return DebugInfoCorrelate ? "default_%p.proflite" : "default_%m.profraw";
+  return DebugInfoCorrelate ? "default_%m.proflite" : "default_%m.profraw";
 }
 
 class EmitAssemblyHelper {

diff  --git a/compiler-rt/lib/profile/InstrProfilingMerge.c 
b/compiler-rt/lib/profile/InstrProfilingMerge.c
index 241891da454bb4..9cf12f251f7262 100644
--- a/compiler-rt/lib/profile/InstrProfilingMerge.c
+++ b/compiler-rt/lib/profile/InstrProfilingMerge.c
@@ -47,7 +47,6 @@ uint64_t lprofGetLoadModuleSignature(void) {
 COMPILER_RT_VISIBILITY
 int __llvm_profile_check_compatibility(const char *ProfileData,
uint64_t ProfileSize) {
-  /* Check profile header only for now  */
   __llvm_profile_header *Header = (__llvm_profile_header *)ProfileData;
   __llvm_profile_data *SrcDataStart, *SrcDataEnd, *SrcData, *DstData;
   SrcDataStart =
@@ -102,13 +101,6 @@ static uintptr_t signextIfWin64(void *V) {
 COMPILER_RT_VISIBILITY
 int __llvm_profile_merge_from_buffer(const char *ProfileData,
  uint64_t ProfileSize) {
-  if (__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE) {
-PROF_ERR(
-"%s\n",
-"Debug info correlation does not support profile merging at runtime. "
-"Instead, merge raw profiles using the llvm-profdata tool.");
-return 1;
-  }
   if (__llvm_profile_get_version() & VARIANT_MASK_TEMPORAL_PROF) {
 PROF_ERR("%s\n",
  "Temporal profiles do not support profile merging at runtime. "
@@ -118,7 +110,8 @@ int __llvm_profile_merge_from_buffer(const char 
*ProfileData,
 
   __llvm_profile_data *SrcDataStart, *SrcDataEnd, *SrcData, *DstData;
   __llvm_profile_header *Header = (__llvm_profile_header *)ProfileData;
-  char *SrcCountersStart;
+  char *SrcCountersStart, *DstCounter;
+  const char *SrcCountersEnd, *SrcCounter;
   const char *SrcNameStart;
   const char *SrcValueProfDataStart, *SrcValueProfData;
   uintptr_t CountersDelta = Header->CountersDelta;
@@ -128,14 +121,32 @@ int __llvm_profile_merge_from_buffer(const char 
*ProfileData,
   Header->BinaryIdsSize);
   SrcDataEnd = SrcDataStart + Header->NumData;
   SrcCountersStart = (char *)SrcDataEnd;
-  SrcNameStart = SrcCountersStart +
- Header->NumCounters * __llvm_profile_counter_entry_size();
+  SrcCountersEnd = SrcCountersStart +
+   Header->NumCounters * __llvm_profile_counter_entry_size();
+  SrcNameStart = SrcCountersEnd;
   SrcValueProfDataStart =
   SrcNameStart + Header->NamesSize +
   __llvm_profile_get_num_padding_bytes(Header->NamesSize);
   if (SrcNameStart < SrcCountersStart)
 return 1;
 
+  // Merge counters by iterating the entire counter section when debug info
+  // correlation is enabled.
+  if (__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE) {
+for (SrcCounter = SrcCountersStart,
+DstCounter = __llvm_profile_begin_counters();
+ SrcCounter < SrcCountersEnd;) {
+  if (__llvm_profile_get_version() & VARIANT_MASK_BYTE_COVERAGE) {
+*DstCounter &= *SrcCounter;
+  } else {
+*(uint64_t *)DstCounter += *(uint64_t *)SrcCounter;
+  }
+  SrcCounter += __llvm_profile_counter_entry_size();
+  DstCounter += __llvm_profile_counter_entry_size();
+}
+return 0;
+  }
+
   for (SrcData = SrcDataStart,
   DstData = (__llvm_profile_data *)__llvm_profile_begin_data(),
   SrcValueProfData = SrcValueProfDataStart;

diff  --git a/compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c 
b/compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
index 325e27503f3530..f347d439e2e067 10

[PATCH] D159000: Reland "[Profile] Allow online merging with debug info correlation."

[Zequan Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
zequanwu marked an inline comment as done.
Closed by commit rGd80992032fd0: Reland "[Profile] Allow online merging 
with debug info correlation." (authored by zequanwu).

Changed prior to commit:
  https://reviews.llvm.org/D159000?vs=553956&id=553988#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159000

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  compiler-rt/lib/profile/InstrProfilingMerge.c
  compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
  compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
  compiler-rt/test/profile/instrprof-merge-empty-data.test
  compiler-rt/test/profile/instrprof-merge-error.c

Index: compiler-rt/test/profile/instrprof-merge-error.c
===
--- compiler-rt/test/profile/instrprof-merge-error.c
+++ compiler-rt/test/profile/instrprof-merge-error.c
@@ -1,11 +1,5 @@
 // RUN: rm -rf %t; mkdir %t
 
-// RUN: %clang_pgogen -o %t/dbg -g -mllvm --debug-info-correlate -mllvm --disable-vp=true %s
-// RUN: env LLVM_PROFILE_FILE=%t/dbg_%m.profdata %run %t/dbg 2>&1 | count 0
-// RUN: env LLVM_PROFILE_FILE=%t/dbg_%m.profdata %run %t/dbg 2>&1 | FileCheck %s --check-prefix=DBG
-
-// DBG: Debug info correlation does not support profile merging at runtime.
-
 // RUN: %clang_pgogen -o %t/timeprof -mllvm -pgo-temporal-instrumentation %s
 // RUN: env LLVM_PROFILE_FILE=%t/timeprof_%m.profdata %run %t/timeprof 2>&1 | count 0
 // RUN: env LLVM_PROFILE_FILE=%t/timeprof_%m.profdata %run %t/timeprof 2>&1 | FileCheck %s --check-prefix=TIMEPROF
Index: compiler-rt/test/profile/instrprof-merge-empty-data.test
===
--- /dev/null
+++ compiler-rt/test/profile/instrprof-merge-empty-data.test
@@ -0,0 +1,14 @@
+// Test online merging with empty data section.
+// RUN: rm -rf %t.dir && split-file %s %t.dir && cd %t.dir
+// RUN: %clangxx_profgen -fcoverage-mapping -o %t main.c -fprofile-list=funlist
+// RUN: LLVM_PROFILE_FILE='a%m.profraw' %t
+// RUN: LLVM_PROFILE_FILE='a%m.profraw' %t 2>&1 | FileCheck %s --allow-empty
+
+// CHECK-NOT: LLVM Profile Error
+
+//--- main.c
+int main() {}
+
+//--- funlist
+[clang]
+default:skip
Index: compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
===
--- compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
+++ compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
@@ -24,3 +24,23 @@
 // RUN: llvm-profdata merge -o %t.cov.normal.profdata %t.cov.profraw
 
 // RUN: diff <(llvm-profdata show --all-functions --counts %t.cov.normal.profdata) <(llvm-profdata show --all-functions --counts %t.cov.profdata)
+
+// Test debug info correlate with online merging.
+
+// RUN: env LLVM_PROFILE_FILE=%t-1.profraw %run %t.normal
+// RUN: env LLVM_PROFILE_FILE=%t-2.profraw %run %t.normal
+// RUN: llvm-profdata merge -o %t.normal.profdata %t-1.profraw %t-2.profraw
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: llvm-profdata merge -o %t.profdata --debug-info=%t %t.profdir/
+
+// RUN: diff <(llvm-profdata show --all-functions --counts %t.normal.profdata) <(llvm-profdata show --all-functions --counts %t.profdata)
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.cov.proflite %run %t.cov
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.cov.proflite %run %t.cov
+// RUN: llvm-profdata merge -o %t.cov.profdata --debug-info=%t.cov %t.profdir/
+
+// RUN: diff <(llvm-profdata show --all-functions --counts %t.cov.normal.profdata) <(llvm-profdata show --all-functions --counts %t.cov.profdata)
Index: compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
===
--- compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
+++ compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
@@ -18,3 +18,23 @@
 // RUN: llvm-profdata merge -o %t.cov.normal.profdata %t.cov.profraw
 
 // RUN: diff <(llvm-profdata show --all-functions --counts %t.cov.normal.profdata) <(llvm-profdata show --all-functions --counts %t.cov.profdata)
+
+// Test debug info correlate with online merging.
+
+// RUN: env LLVM_PROFILE_FILE=%t-1.profraw %run %t.normal
+// RUN: env LLVM_PROFILE_FILE=%t-2.profraw %run %t.normal
+// RUN: llvm-profdata merge -o %t.normal.profdata %t-1.profraw %t-2.profraw
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: llvm-profdata merge -o %t.profdata --debug-info=%t.dSYM %t.profdir/
+
+// RUN: diff <(llvm-profdata show --all-functions --c

[PATCH] D157813: [VE][Clang] Change to enable VPU flag by default

[Fangrui Song via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/Arch/VE.cpp:22
+ std::vector &Features) {
+  // Defaults.
+  bool EnableVPU = true;

Delete the comment. The code speaks itself. 

```
if (Args.hasArg(options::OPT_mvevpu, options::OPT_mno_vevpu, true)
  Features.push_back("+vpu");
```



Comment at: clang/test/Driver/ve-features.c:1
+// RUN: %clang -target ve-unknown-linux-gnu -### %s -mvevpu 2>&1 | FileCheck 
%s -check-prefix=VEVPU
+// RUN: %clang -target ve-unknown-linux-gnu -### %s -mno-vevpu 2>&1 | 
FileCheck %s -check-prefix=NO-VEVPU

kaz7 wrote:
> MaskRay wrote:
> > `-target ` has been deprecated since Clang 3.4. Use `--target=`
> I didn't know that.  Thank you!
I think we typically spend just two RUN lines:

* one for the default
* one for `-mvevpu -mno-vevpu`

The additional coverage for having 3 RUN lines is probably not useful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157813

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

[clang] 824b136 - [clang][dataflow] Support range-for loops in fixpoint algorithm.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2023-08-28T17:35:34Z
New Revision: 824b136591303a2bec62cc752ec89843fbbc0ca0

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

LOG: [clang][dataflow] Support range-for loops in fixpoint algorithm.

Adds support for recognizing range-for loops in the main algorithm for computing
the model fixpoint.

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

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp 
b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index ae108a702c86aa..626b57b43755ec 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/ASTDumper.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/OperationKinds.h"
+#include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Analysis/Analyses/PostOrderCFGView.h"
 #include "clang/Analysis/CFG.h"
@@ -58,6 +59,7 @@ static bool isLoopHead(const CFGBlock &B) {
   case Stmt::WhileStmtClass:
   case Stmt::DoStmtClass:
   case Stmt::ForStmtClass:
+  case Stmt::CXXForRangeStmtClass:
 return true;
   default:
 return false;
@@ -108,6 +110,12 @@ class TerminatorVisitor
 return {nullptr, false};
   }
 
+  TerminatorVisitorRetTy VisitCXXForRangeStmt(const CXXForRangeStmt *) {
+// Don't do anything special for CXXForRangeStmt, because the condition
+// (being implicitly generated) isn't visible from the loop body.
+return {nullptr, false};
+  }
+
   TerminatorVisitorRetTy VisitBinaryOperator(const BinaryOperator *S) {
 assert(S->getOpcode() == BO_LAnd || S->getOpcode() == BO_LOr);
 auto *LHS = S->getLHS();

diff  --git 
a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
index 8e43ef73c6b78b..de0b5c7b239055 100644
--- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -1612,4 +1612,21 @@ TEST_F(TopTest, TopUnusedBeforeLoopHeadJoinsToTop) {
 
   });
 }
+
+TEST_F(TopTest, ForRangeStmtConverges) {
+  std::string Code = R"(
+void target(bool Foo) {
+  int Ints[10];
+  bool B = false;
+  for (int I : Ints)
+B = true;
+}
+  )";
+  runDataflow(Code,
+  [](const llvm::StringMap> &,
+ const AnalysisOutputs &) {
+// No additional expectations. We're only checking that the
+// analysis converged.
+  });
+}
 } // namespace



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

[PATCH] D158848: [clang][dataflow] Support range-for loops in fixpoint algorithm.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG824b13659130: [clang][dataflow] Support range-for loops in 
fixpoint algorithm. (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158848

Files:
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -1612,4 +1612,21 @@
 
   });
 }
+
+TEST_F(TopTest, ForRangeStmtConverges) {
+  std::string Code = R"(
+void target(bool Foo) {
+  int Ints[10];
+  bool B = false;
+  for (int I : Ints)
+B = true;
+}
+  )";
+  runDataflow(Code,
+  [](const llvm::StringMap> &,
+ const AnalysisOutputs &) {
+// No additional expectations. We're only checking that the
+// analysis converged.
+  });
+}
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/ASTDumper.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/OperationKinds.h"
+#include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Analysis/Analyses/PostOrderCFGView.h"
 #include "clang/Analysis/CFG.h"
@@ -58,6 +59,7 @@
   case Stmt::WhileStmtClass:
   case Stmt::DoStmtClass:
   case Stmt::ForStmtClass:
+  case Stmt::CXXForRangeStmtClass:
 return true;
   default:
 return false;
@@ -108,6 +110,12 @@
 return {nullptr, false};
   }
 
+  TerminatorVisitorRetTy VisitCXXForRangeStmt(const CXXForRangeStmt *) {
+// Don't do anything special for CXXForRangeStmt, because the condition
+// (being implicitly generated) isn't visible from the loop body.
+return {nullptr, false};
+  }
+
   TerminatorVisitorRetTy VisitBinaryOperator(const BinaryOperator *S) {
 assert(S->getOpcode() == BO_LAnd || S->getOpcode() == BO_LOr);
 auto *LHS = S->getLHS();


Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -1612,4 +1612,21 @@
 
   });
 }
+
+TEST_F(TopTest, ForRangeStmtConverges) {
+  std::string Code = R"(
+void target(bool Foo) {
+  int Ints[10];
+  bool B = false;
+  for (int I : Ints)
+B = true;
+}
+  )";
+  runDataflow(Code,
+  [](const llvm::StringMap> &,
+ const AnalysisOutputs &) {
+// No additional expectations. We're only checking that the
+// analysis converged.
+  });
+}
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/ASTDumper.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/OperationKinds.h"
+#include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Analysis/Analyses/PostOrderCFGView.h"
 #include "clang/Analysis/CFG.h"
@@ -58,6 +59,7 @@
   case Stmt::WhileStmtClass:
   case Stmt::DoStmtClass:
   case Stmt::ForStmtClass:
+  case Stmt::CXXForRangeStmtClass:
 return true;
   default:
 return false;
@@ -108,6 +110,12 @@
 return {nullptr, false};
   }
 
+  TerminatorVisitorRetTy VisitCXXForRangeStmt(const CXXForRangeStmt *) {
+// Don't do anything special for CXXForRangeStmt, because the condition
+// (being implicitly generated) isn't visible from the loop body.
+return {nullptr, false};
+  }
+
   TerminatorVisitorRetTy VisitBinaryOperator(const BinaryOperator *S) {
 assert(S->getOpcode() == BO_LAnd || S->getOpcode() == BO_LOr);
 auto *LHS = S->getLHS();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156616: [clang-tidy] Fix c_str() removal and cast addition when re-ordering arguments

[Mike Crowe via Phabricator via cfe-commits]

mikecrowe added a comment.

In D156616#4621945 , @PiotrZSL wrote:

> In D156616#4621914 , @mikecrowe 
> wrote:
>
>> @PiotrZSL, I think that this is quite an important fix since without it the 
>> check completely mangles the code. Should it be put in the 17.x release 
>> branch too?
>
> If issue happens only when StrictMode is set to True, then I wouldn't worry 
> too much, and then this could wait for Clang 18 that is just half year away.

The removal of `c_str()` happens even without StrictMode set to True.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156616

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

[PATCH] D157813: [VE][Clang] Change to enable VPU flag by default

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

> [VE][Clang] Change to enable VPU flag by default

For components like llvm/, clang/, we usually use a more specific tag. I'd pick 
`[Driver]` over `[Clang]`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157813

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