[PATCH] D112881: [clang-tidy] Allow disabling integer to floating-point narrowing conversions for cppcoreguidelines-narrowing-conversions

[Paul Altin via Phabricator via cfe-commits]

paulaltin added a comment.

> LGTM, with a few small nits. It'd be nice to update the patch summary with 
> more information about why the option is needed.

Thanks @aaron.ballman. I've made the changes to the Release Notes and added 
more info to the summary.

In general, would you be happy to consider other changes like this in 
clang-tidy? Our company uses the tool as part of our CI (and I personally find 
it extremely useful), but there are some checks that we have to disable 
entirely because they do multiple things, some of which we don't want to (or 
can't easily) fix. If these had options to disable part of the check then we 
would still be able to use the remaining parts, which would be great.

If adding more options to existing checks in clang-tidy is a direction that the 
developers are happy to support, then I'd be keen to submit more changes like 
this.


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

https://reviews.llvm.org/D112881

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

[PATCH] D114776: [clang-offload-bundler] Reuse original file extension for device archive member

[Sergey Dmitriev via Phabricator via cfe-commits]

sdmitriev created this revision.
sdmitriev added reviewers: saiislam, ABataev.
sdmitriev requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

This patch changes clang-offload-bundler to use the original file extension for
the device archive member when unbundling archives instead of printing a warning
and defaulting to ".o".


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114776

Files:
  clang/test/Driver/fat-archive-unbundle-ext.c
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp


Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -180,21 +180,19 @@
   }
 };
 
-static StringRef getDeviceFileExtension(StringRef Device) {
+static StringRef getDeviceFileExtension(StringRef Device,
+StringRef BundleFileName) {
   if (Device.contains("gfx"))
 return ".bc";
   if (Device.contains("sm_"))
 return ".cubin";
-
-  WithColor::warning() << "Could not determine extension for archive"
-  "members, using \".o\"\n";
-  return ".o";
+  return sys::path::extension(BundleFileName);
 }
 
 static std::string getDeviceLibraryFileName(StringRef BundleFileName,
 StringRef Device) {
   StringRef LibName = sys::path::stem(BundleFileName);
-  StringRef Extension = getDeviceFileExtension(Device);
+  StringRef Extension = getDeviceFileExtension(Device, BundleFileName);
 
   std::string Result;
   Result += LibName;
Index: clang/test/Driver/fat-archive-unbundle-ext.c
===
--- /dev/null
+++ clang/test/Driver/fat-archive-unbundle-ext.c
@@ -0,0 +1,21 @@
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: windows, darwin, aix
+
+// Generate dummy fat object
+// RUN: %clang -O0 -target %itanium_abi_triple %s -c -o %t.host.o
+// RUN: echo 'Content of device file' > %t.tgt.o
+// RUN: clang-offload-bundler -type=o 
-targets=host-%itanium_abi_triple,openmp-%itanium_abi_triple 
-inputs=%t.host.o,%t.tgt.o -outputs=%t.fat.obj
+
+// Then create a static archive with that object
+// RUN: rm -f %t.fat.a
+// RUN: llvm-ar cr %t.fat.a %t.fat.obj
+
+// Unbundle device part from the archive. Check that bundler does not print 
warnings.
+// RUN: clang-offload-bundler -unbundle -type=a 
-targets=openmp-%itanium_abi_triple -inputs=%t.fat.a -outputs=%t.tgt.a 2>&1 | 
FileCheck --allow-empty --check-prefix=CHECK-WARNING %s
+// CHECK-WARNING-NOT: warning
+
+// Check that device archive member inherited file extension from the original 
file.
+// RUN: llvm-ar t %t.tgt.a | FileCheck --check-prefix=CHECK-ARCHIVE %s
+// CHECK-ARCHIVE: {{\.obj$}}
+
+void foo(void) {}


Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -180,21 +180,19 @@
   }
 };
 
-static StringRef getDeviceFileExtension(StringRef Device) {
+static StringRef getDeviceFileExtension(StringRef Device,
+StringRef BundleFileName) {
   if (Device.contains("gfx"))
 return ".bc";
   if (Device.contains("sm_"))
 return ".cubin";
-
-  WithColor::warning() << "Could not determine extension for archive"
-  "members, using \".o\"\n";
-  return ".o";
+  return sys::path::extension(BundleFileName);
 }
 
 static std::string getDeviceLibraryFileName(StringRef BundleFileName,
 StringRef Device) {
   StringRef LibName = sys::path::stem(BundleFileName);
-  StringRef Extension = getDeviceFileExtension(Device);
+  StringRef Extension = getDeviceFileExtension(Device, BundleFileName);
 
   std::string Result;
   Result += LibName;
Index: clang/test/Driver/fat-archive-unbundle-ext.c
===
--- /dev/null
+++ clang/test/Driver/fat-archive-unbundle-ext.c
@@ -0,0 +1,21 @@
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: windows, darwin, aix
+
+// Generate dummy fat object
+// RUN: %clang -O0 -target %itanium_abi_triple %s -c -o %t.host.o
+// RUN: echo 'Content of device file' > %t.tgt.o
+// RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-%itanium_abi_triple -inputs=%t.host.o,%t.tgt.o -outputs=%t.fat.obj
+
+// Then create a static archive with that object
+// RUN: rm -f %t.fat.a
+// RUN: llvm-ar cr %t.fat.a %t.fat.obj
+
+// Unbundle device part from the archive. Check that bundler does not print warnings.
+// RUN: clang-offload-bundler -unbundle -type=a -targets=openmp-%itanium_abi_triple -inputs=%t.fat.a -ou

[PATCH] D114427: [clang-tidy] Warn on functional C-style casts

[Carlos Galvez 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 rG5bbe50148f3b: [clang-tidy] Warn on functional C-style casts 
(authored by carlosgalvezp).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114427

Files:
  clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp
@@ -143,11 +143,10 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant cast to the same type
   // CHECK-FIXES: {{^}}  kZero;
 
-  int b2 = int(b);
-  int b3 = static_cast(b);
-  int b4 = b;
+  int b2 = static_cast(b);
+  int b3 = b;
   double aa = a;
-  (void)b2;
+  (void)aa;
   return (void)g();
 }
 
@@ -321,3 +320,50 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: C-style casts are discouraged; use constructor call syntax [
   // CHECK-FIXES: auto s6 = S(cr);
 }
+
+template 
+T functional_cast_template_used_by_class(float i) {
+  return T(i);
+}
+
+template 
+T functional_cast_template_used_by_int(float i) {
+  return T(i);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: C-style casts are discouraged; use static_cast
+  // CHECK-FIXES: return static_cast(i);
+}
+
+struct S2 {
+  S2(float);
+};
+using T = S2;
+using U = S2 &;
+
+void functional_casts() {
+  float x = 1.5F;
+  auto y = int(x);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: C-style casts are discouraged; use static_cast
+  // CHECK-FIXES: auto y = static_cast(x);
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wc++11-narrowing"
+  // This if fine, compiler will warn about implicit conversions with brace initialization
+  auto z = int{x};
+#pragma clang diagnostic pop
+
+  // Functional casts are allowed if S is of class type
+  const char *str = "foo";
+  auto s = S(str);
+
+  // Functional casts in template functions
+  functional_cast_template_used_by_class(x);
+  functional_cast_template_used_by_int(x);
+
+  // New expressions are not functional casts
+  auto w = new int(x);
+
+  // Typedefs
+  S2 t = T(x); // OK, constructor call
+  S2 u = U(x); // NOK, it's a reinterpret_cast in disguise
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: C-style casts are discouraged; use static_cast/const_cast/reinterpret_cast
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -133,9 +133,12 @@
 Changes in existing checks
 ^^
 
-- Removed default setting `cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"`,
+- Removed default setting ``cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"``,
   to match the current state of the C++ Core Guidelines.
 
+- Updated :doc:`google-readability-casting
+  ` to diagnose and fix functional
+  casts, to achieve feature parity with the corresponding ``cpplint.py`` check.
 
 Removed checks
 ^^
Index: clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
+++ clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
@@ -31,6 +31,11 @@
   unless(isInTemplateInstantiation()))
   .bind("cast"),
   this);
+  Finder->addMatcher(
+  cxxFunctionalCastExpr(unless(hasDescendant(cxxConstructExpr())),
+unless(hasDescendant(initListExpr(
+  .bind("cast"),
+  this);
 }
 
 static bool needsConstCast(QualType SourceType, QualType DestType) {
@@ -55,8 +60,39 @@
   return T1.getUnqualifiedType() == T2.getUnqualifiedType();
 }
 
+static clang::CharSourceRange getReplaceRange(const ExplicitCastExpr *Expr) {
+  if (const auto *CastExpr = dyn_cast(Expr)) {
+return CharSourceRange::getCharRange(
+CastExpr->getLParenLoc(),
+CastExpr->getSubExprAsWritten()->getBeginLoc());
+  } else if (const auto *CastExpr = dyn_cast(Expr)) {
+return CharSourceRange::getCharRange(CastExpr->getBeginLoc(),
+ CastExpr->getLParenLoc());
+  } else
+llvm_unreachable("Unsupported CastExpr");
+}
+
+static StringRef getDestTypeString(const SourceManager &SM,
+   const LangOptions &LangOpts,
+   const ExplicitCastExpr *Expr) {
+  SourceLocation BeginLoc;
+  SourceLocation EndLoc;
+
+  if (const auto *CastExpr 

[clang-tools-extra] 5bbe501 - [clang-tidy] Warn on functional C-style casts

[Carlos Galvez via cfe-commits]

Author: Carlos Galvez
Date: 2021-11-30T07:31:17Z
New Revision: 5bbe50148f3b515c170be22209395b72890f5b8c

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

LOG: [clang-tidy] Warn on functional C-style casts

The google-readability-casting check is meant to be on par
with cpplint's readability/casting check, according to the
documentation. However it currently does not diagnose
functional casts, like:

float x = 1.5F;
int y = int(x);

This is detected by cpplint, however, and the guidelines
are clear that such a cast is only allowed when the type
is a class type (constructor call):

> You may use cast formats like `T(x)` only when `T` is a class type.

Therefore, update the clang-tidy check to check this
case.

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp 
b/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
index a32603dc2cb0..82804a670959 100644
--- a/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
@@ -31,6 +31,11 @@ void AvoidCStyleCastsCheck::registerMatchers(
   unless(isInTemplateInstantiation()))
   .bind("cast"),
   this);
+  Finder->addMatcher(
+  cxxFunctionalCastExpr(unless(hasDescendant(cxxConstructExpr())),
+unless(hasDescendant(initListExpr(
+  .bind("cast"),
+  this);
 }
 
 static bool needsConstCast(QualType SourceType, QualType DestType) {
@@ -55,8 +60,39 @@ static bool pointedUnqualifiedTypesAreEqual(QualType T1, 
QualType T2) {
   return T1.getUnqualifiedType() == T2.getUnqualifiedType();
 }
 
+static clang::CharSourceRange getReplaceRange(const ExplicitCastExpr *Expr) {
+  if (const auto *CastExpr = dyn_cast(Expr)) {
+return CharSourceRange::getCharRange(
+CastExpr->getLParenLoc(),
+CastExpr->getSubExprAsWritten()->getBeginLoc());
+  } else if (const auto *CastExpr = dyn_cast(Expr)) {
+return CharSourceRange::getCharRange(CastExpr->getBeginLoc(),
+ CastExpr->getLParenLoc());
+  } else
+llvm_unreachable("Unsupported CastExpr");
+}
+
+static StringRef getDestTypeString(const SourceManager &SM,
+   const LangOptions &LangOpts,
+   const ExplicitCastExpr *Expr) {
+  SourceLocation BeginLoc;
+  SourceLocation EndLoc;
+
+  if (const auto *CastExpr = dyn_cast(Expr)) {
+BeginLoc = CastExpr->getLParenLoc().getLocWithOffset(1);
+EndLoc = CastExpr->getRParenLoc().getLocWithOffset(-1);
+  } else if (const auto *CastExpr = dyn_cast(Expr)) {
+BeginLoc = CastExpr->getBeginLoc();
+EndLoc = CastExpr->getLParenLoc().getLocWithOffset(-1);
+  } else
+llvm_unreachable("Unsupported CastExpr");
+
+  return Lexer::getSourceText(CharSourceRange::getTokenRange(BeginLoc, EndLoc),
+  SM, LangOpts);
+}
+
 void AvoidCStyleCastsCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *CastExpr = Result.Nodes.getNodeAs("cast");
+  const auto *CastExpr = Result.Nodes.getNodeAs("cast");
 
   // Ignore casts in macros.
   if (CastExpr->getExprLoc().isMacroID())
@@ -80,8 +116,7 @@ void AvoidCStyleCastsCheck::check(const 
MatchFinder::MatchResult &Result) {
   const QualType SourceType = SourceTypeAsWritten.getCanonicalType();
   const QualType DestType = DestTypeAsWritten.getCanonicalType();
 
-  auto ReplaceRange = CharSourceRange::getCharRange(
-  CastExpr->getLParenLoc(), 
CastExpr->getSubExprAsWritten()->getBeginLoc());
+  CharSourceRange ReplaceRange = getReplaceRange(CastExpr);
 
   bool FnToFnCast =
   IsFunction(SourceTypeAsWritten) && IsFunction(DestTypeAsWritten);
@@ -124,18 +159,14 @@ void AvoidCStyleCastsCheck::check(const 
MatchFinder::MatchResult &Result) {
 
   // Leave type spelling exactly as it was (unlike
   // getTypeAsWritten().getAsString() which would spell enum types 'enum X').
-  StringRef DestTypeString =
-  Lexer::getSourceText(CharSourceRange::getTokenRange(
-   CastExpr->getLParenLoc().getLocWithOffset(1),
-   CastExpr->getRParenLoc().getLocWithOffset(-1)),
-   SM, getLangOpts());
+  StringRef DestTypeString = getDestTypeString(SM, getLangOpts(), CastExpr);
 
   auto Diag =
   diag(CastExpr->getBeginLoc(), "C-style casts are discouraged; use %0");
 
   auto ReplaceWithCast = [&](std::string CastText) {
 const Expr *SubExp

[PATCH] D114427: [clang-tidy] Warn on functional C-style casts

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

Thanks a lot for the review! I've fixed the last comment by @salman-javed-nz . 
Since that was the last standing comment I'll push :)


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

https://reviews.llvm.org/D114427

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

[PATCH] D114427: [clang-tidy] Warn on functional C-style casts

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp updated this revision to Diff 390580.
carlosgalvezp added a comment.

Fixed doc.


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

https://reviews.llvm.org/D114427

Files:
  clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp
@@ -143,11 +143,10 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant cast to the same type
   // CHECK-FIXES: {{^}}  kZero;
 
-  int b2 = int(b);
-  int b3 = static_cast(b);
-  int b4 = b;
+  int b2 = static_cast(b);
+  int b3 = b;
   double aa = a;
-  (void)b2;
+  (void)aa;
   return (void)g();
 }
 
@@ -321,3 +320,50 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: C-style casts are discouraged; use constructor call syntax [
   // CHECK-FIXES: auto s6 = S(cr);
 }
+
+template 
+T functional_cast_template_used_by_class(float i) {
+  return T(i);
+}
+
+template 
+T functional_cast_template_used_by_int(float i) {
+  return T(i);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: C-style casts are discouraged; use static_cast
+  // CHECK-FIXES: return static_cast(i);
+}
+
+struct S2 {
+  S2(float);
+};
+using T = S2;
+using U = S2 &;
+
+void functional_casts() {
+  float x = 1.5F;
+  auto y = int(x);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: C-style casts are discouraged; use static_cast
+  // CHECK-FIXES: auto y = static_cast(x);
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wc++11-narrowing"
+  // This if fine, compiler will warn about implicit conversions with brace initialization
+  auto z = int{x};
+#pragma clang diagnostic pop
+
+  // Functional casts are allowed if S is of class type
+  const char *str = "foo";
+  auto s = S(str);
+
+  // Functional casts in template functions
+  functional_cast_template_used_by_class(x);
+  functional_cast_template_used_by_int(x);
+
+  // New expressions are not functional casts
+  auto w = new int(x);
+
+  // Typedefs
+  S2 t = T(x); // OK, constructor call
+  S2 u = U(x); // NOK, it's a reinterpret_cast in disguise
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: C-style casts are discouraged; use static_cast/const_cast/reinterpret_cast
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -133,9 +133,12 @@
 Changes in existing checks
 ^^
 
-- Removed default setting `cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"`,
+- Removed default setting ``cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"``,
   to match the current state of the C++ Core Guidelines.
 
+- Updated :doc:`google-readability-casting
+  ` to diagnose and fix functional
+  casts, to achieve feature parity with the corresponding ``cpplint.py`` check.
 
 Removed checks
 ^^
Index: clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
+++ clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
@@ -31,6 +31,11 @@
   unless(isInTemplateInstantiation()))
   .bind("cast"),
   this);
+  Finder->addMatcher(
+  cxxFunctionalCastExpr(unless(hasDescendant(cxxConstructExpr())),
+unless(hasDescendant(initListExpr(
+  .bind("cast"),
+  this);
 }
 
 static bool needsConstCast(QualType SourceType, QualType DestType) {
@@ -55,8 +60,39 @@
   return T1.getUnqualifiedType() == T2.getUnqualifiedType();
 }
 
+static clang::CharSourceRange getReplaceRange(const ExplicitCastExpr *Expr) {
+  if (const auto *CastExpr = dyn_cast(Expr)) {
+return CharSourceRange::getCharRange(
+CastExpr->getLParenLoc(),
+CastExpr->getSubExprAsWritten()->getBeginLoc());
+  } else if (const auto *CastExpr = dyn_cast(Expr)) {
+return CharSourceRange::getCharRange(CastExpr->getBeginLoc(),
+ CastExpr->getLParenLoc());
+  } else
+llvm_unreachable("Unsupported CastExpr");
+}
+
+static StringRef getDestTypeString(const SourceManager &SM,
+   const LangOptions &LangOpts,
+   const ExplicitCastExpr *Expr) {
+  SourceLocation BeginLoc;
+  SourceLocation EndLoc;
+
+  if (const auto *CastExpr = dyn_cast(Expr)) {
+BeginLoc = CastExpr->getLParenLoc().getLocWithOffset(1);
+EndLoc = CastExpr->getRParenLoc().getLocWithOffset(-1);
+  } else if (const auto *CastExpr = dy

[PATCH] D112881: [clang-tidy] Allow disabling integer to floating-point narrowing conversions for cppcoreguidelines-narrowing-conversions

[Paul Altin via Phabricator via cfe-commits]

paulaltin updated this revision to Diff 390573.
paulaltin edited the summary of this revision.
paulaltin added a comment.

Fixing doc formatting.


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

https://reviews.llvm.org/D112881

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-narrowingintegertofloatingpoint-option.cpp

Index: clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
@@ -98,6 +98,7 @@
   const BuiltinType &ToType) const;
 
   const bool WarnOnIntegerNarrowingConversion;
+  const bool WarnOnIntegerToFloatingPointNarrowingConversion;
   const bool WarnOnFloatingPointNarrowingConversion;
   const bool WarnWithinTemplateInstantiation;
   const bool WarnOnEquivalentBitWidth;
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
@@ -37,6 +37,8 @@
 : ClangTidyCheck(Name, Context),
   WarnOnIntegerNarrowingConversion(
   Options.get("WarnOnIntegerNarrowingConversion", true)),
+  WarnOnIntegerToFloatingPointNarrowingConversion(
+  Options.get("WarnOnIntegerToFloatingPointNarrowingConversion", true)),
   WarnOnFloatingPointNarrowingConversion(
   Options.get("WarnOnFloatingPointNarrowingConversion", true)),
   WarnWithinTemplateInstantiation(
@@ -49,6 +51,8 @@
 ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "WarnOnIntegerNarrowingConversion",
 WarnOnIntegerNarrowingConversion);
+  Options.store(Opts, "WarnOnIntegerToFloatingPointNarrowingConversion",
+WarnOnIntegerToFloatingPointNarrowingConversion);
   Options.store(Opts, "WarnOnFloatingPointNarrowingConversion",
 WarnOnFloatingPointNarrowingConversion);
   Options.store(Opts, "WarnWithinTemplateInstantiation",
@@ -376,19 +380,21 @@
 void NarrowingConversionsCheck::handleIntegralToFloating(
 const ASTContext &Context, SourceLocation SourceLoc, const Expr &Lhs,
 const Expr &Rhs) {
-  const BuiltinType *ToType = getBuiltinType(Lhs);
-  llvm::APSInt IntegerConstant;
-  if (getIntegerConstantExprValue(Context, Rhs, IntegerConstant)) {
-if (!isWideEnoughToHold(Context, IntegerConstant, *ToType))
-  diagNarrowIntegerConstant(SourceLoc, Lhs, Rhs, IntegerConstant);
-return;
-  }
+  if (WarnOnIntegerToFloatingPointNarrowingConversion) {
+const BuiltinType *ToType = getBuiltinType(Lhs);
+llvm::APSInt IntegerConstant;
+if (getIntegerConstantExprValue(Context, Rhs, IntegerConstant)) {
+  if (!isWideEnoughToHold(Context, IntegerConstant, *ToType))
+diagNarrowIntegerConstant(SourceLoc, Lhs, Rhs, IntegerConstant);
+  return;
+}
 
-  const BuiltinType *FromType = getBuiltinType(Rhs);
-  if (isWarningInhibitedByEquivalentSize(Context, *FromType, *ToType))
-return;
-  if (!isWideEnoughToHold(Context, *FromType, *ToType))
-diagNarrowType(SourceLoc, Lhs, Rhs);
+const BuiltinType *FromType = getBuiltinType(Rhs);
+if (isWarningInhibitedByEquivalentSize(Context, *FromType, *ToType))
+  return;
+if (!isWideEnoughToHold(Context, *FromType, *ToType))
+  diagNarrowType(SourceLoc, Lhs, Rhs);
+  }
 }
 
 void NarrowingConversionsCheck::handleFloatingToIntegral(
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -119,6 +119,11 @@
 - Removed default setting `cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"`,
   to match the current state of the C++ Core Guidelines.
 
+- :doc:`cppcoreguidelines-narrowing-conversions `
+  check now supports a `WarnOnIntegerToFloatingPointNarrowingConversion`
+  option to control whether to warn on narrowing integer to floating-point
+  conversions.
+
 
 Removed checks
 ^^
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.r

[PATCH] D114769: [C++20] [Modules] [Concepts] Recognize same concepts more precisely in Serialization

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 390572.
ChuanqiXu added a comment.

Clean code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114769

Files:
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/test/Modules/Inputs/concept/A.cppm
  clang/test/Modules/Inputs/concept/foo.h
  clang/test/Modules/concept.cppm


Index: clang/test/Modules/concept.cppm
===
--- /dev/null
+++ clang/test/Modules/concept.cppm
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: %clang -std=c++20 %S/Inputs/concept/A.cppm --precompile -o %t/A.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t -I%S/Inputs/concept %s -c 
-Xclang -verify
+// expected-no-diagnostics
+
+module;
+#include "foo.h"
+export module B;
+import A;
Index: clang/test/Modules/Inputs/concept/foo.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/concept/foo.h
@@ -0,0 +1,13 @@
+#ifndef FOO_H
+#define FOO_H
+
+template< class T >
+concept Range = requires(T& t) { t.begin(); };
+
+struct A {
+public:
+template 
+using range_type = T;
+};
+
+#endif
Index: clang/test/Modules/Inputs/concept/A.cppm
===
--- /dev/null
+++ clang/test/Modules/Inputs/concept/A.cppm
@@ -0,0 +1,3 @@
+module;
+#include "foo.h"
+export module A;
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2948,6 +2948,7 @@
 static bool isSameTemplateParameterList(const ASTContext &C,
 const TemplateParameterList *X,
 const TemplateParameterList *Y);
+static bool isSameEntity(NamedDecl *X, NamedDecl *Y);
 
 /// Determine whether two template parameters are similar enough
 /// that they may be used in declarations of the same template.
@@ -2967,7 +2968,7 @@
 if (!TXTC != !TYTC)
   return false;
 if (TXTC && TYTC) {
-  if (TXTC->getNamedConcept() != TYTC->getNamedConcept())
+  if (!isSameEntity(TXTC->getNamedConcept(), TYTC->getNamedConcept()))
 return false;
   if (TXTC->hasExplicitTemplateArgs() != TYTC->hasExplicitTemplateArgs())
 return false;


Index: clang/test/Modules/concept.cppm
===
--- /dev/null
+++ clang/test/Modules/concept.cppm
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: %clang -std=c++20 %S/Inputs/concept/A.cppm --precompile -o %t/A.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t -I%S/Inputs/concept %s -c -Xclang -verify
+// expected-no-diagnostics
+
+module;
+#include "foo.h"
+export module B;
+import A;
Index: clang/test/Modules/Inputs/concept/foo.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/concept/foo.h
@@ -0,0 +1,13 @@
+#ifndef FOO_H
+#define FOO_H
+
+template< class T >
+concept Range = requires(T& t) { t.begin(); };
+
+struct A {
+public:
+template 
+using range_type = T;
+};
+
+#endif
Index: clang/test/Modules/Inputs/concept/A.cppm
===
--- /dev/null
+++ clang/test/Modules/Inputs/concept/A.cppm
@@ -0,0 +1,3 @@
+module;
+#include "foo.h"
+export module A;
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2948,6 +2948,7 @@
 static bool isSameTemplateParameterList(const ASTContext &C,
 const TemplateParameterList *X,
 const TemplateParameterList *Y);
+static bool isSameEntity(NamedDecl *X, NamedDecl *Y);
 
 /// Determine whether two template parameters are similar enough
 /// that they may be used in declarations of the same template.
@@ -2967,7 +2968,7 @@
 if (!TXTC != !TYTC)
   return false;
 if (TXTC && TYTC) {
-  if (TXTC->getNamedConcept() != TYTC->getNamedConcept())
+  if (!isSameEntity(TXTC->getNamedConcept(), TYTC->getNamedConcept()))
 return false;
   if (TXTC->hasExplicitTemplateArgs() != TYTC->hasExplicitTemplateArgs())
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114769: [C++20] [Modules] [Concepts] Recognize same concepts more precisely in Serialization

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu created this revision.
ChuanqiXu added reviewers: rsmith, aaron.ballman, urnathan, saar.raz.
ChuanqiXu added a project: clang.
ChuanqiXu requested review of this revision.
Herald added a subscriber: cfe-commits.

When we use concept in modules, we would meet a curious error. For example:

  C++
  // foo.h
  #ifndef FOO_H
  #define FOO_H
  
  template< class T >
  concept Range = requires(T& t) {
t.begin();
  };
  
  namespace workspace {
  struct A {
  public:
  template 
  using range_type = T;
  };
  }
  
  #endif
  // A.cppm
  module;
  #include "foo.h"
  export module A;
  // B.cppm
  module;
  #include "foo.h"
  export module B;
  import A;

The compiler would tell us that the definition of `workspace::A::range_type` in 
B.cppm is not the same with the one in A.cppm!
The reason here is that the implementation would judge two concepts is same by 
their addresses. However, when we use modules, the addresses wouldn't be the 
same all the time since one is parsed in their TU and another is imported in 
another TU.
This patch fixes this by using `isSameEntity` to judge the two concepts. I 
think it should be a good fix.

Test Plan: check-all


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114769

Files:
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/test/Modules/Inputs/concept/A.cppm
  clang/test/Modules/Inputs/concept/foo.h
  clang/test/Modules/concept.cppm


Index: clang/test/Modules/concept.cppm
===
--- /dev/null
+++ clang/test/Modules/concept.cppm
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: %clang -std=c++20 %S/Inputs/concept/A.cppm --precompile -o %t/A.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t -I%S/Inputs/concept %s -c 
-Xclang -verify
+// expected-no-diagnostics
+
+module;
+#include "foo.h"
+export module B;
+import A;
Index: clang/test/Modules/Inputs/concept/foo.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/concept/foo.h
@@ -0,0 +1,19 @@
+#ifndef FOO_H
+#define FOO_H
+
+template< class T >
+concept Range = requires(T& t) {
+  t.begin();
+};
+
+namespace workspace {
+struct A {
+struct B {
+public:
+template 
+using range_type = T;
+};
+};
+}
+
+#endif
Index: clang/test/Modules/Inputs/concept/A.cppm
===
--- /dev/null
+++ clang/test/Modules/Inputs/concept/A.cppm
@@ -0,0 +1,3 @@
+module;
+#include "foo.h"
+export module A;
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2948,6 +2948,7 @@
 static bool isSameTemplateParameterList(const ASTContext &C,
 const TemplateParameterList *X,
 const TemplateParameterList *Y);
+static bool isSameEntity(NamedDecl *X, NamedDecl *Y);
 
 /// Determine whether two template parameters are similar enough
 /// that they may be used in declarations of the same template.
@@ -2967,7 +2968,9 @@
 if (!TXTC != !TYTC)
   return false;
 if (TXTC && TYTC) {
-  if (TXTC->getNamedConcept() != TYTC->getNamedConcept())
+  ConceptDecl *CDX = TXTC->getNamedConcept();
+  ConceptDecl *CDY = TYTC->getNamedConcept();
+  if (!isSameEntity(CDX, CDY))
 return false;
   if (TXTC->hasExplicitTemplateArgs() != TYTC->hasExplicitTemplateArgs())
 return false;


Index: clang/test/Modules/concept.cppm
===
--- /dev/null
+++ clang/test/Modules/concept.cppm
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: %clang -std=c++20 %S/Inputs/concept/A.cppm --precompile -o %t/A.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t -I%S/Inputs/concept %s -c -Xclang -verify
+// expected-no-diagnostics
+
+module;
+#include "foo.h"
+export module B;
+import A;
Index: clang/test/Modules/Inputs/concept/foo.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/concept/foo.h
@@ -0,0 +1,19 @@
+#ifndef FOO_H
+#define FOO_H
+
+template< class T >
+concept Range = requires(T& t) {
+  t.begin();
+};
+
+namespace workspace {
+struct A {
+struct B {
+public:
+template 
+using range_type = T;
+};
+};
+}
+
+#endif
Index: clang/test/Modules/Inputs/concept/A.cppm
===
--- /dev/null
+++ clang/test/Modules/Inputs/concept/A.cppm
@@ -0,0 +1,3 @@
+module;
+#include "foo.h"
+export module A;
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang

[PATCH] D114162: [X86][clang] Enable floating-point type for -mno-x87 option on 32-bits

[Phoebe Wang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG42c15c7edf17: [X86][clang] Enable floating-point type for 
-mno-x87 option on 32-bits (authored by pengfei).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114162

Files:
  clang/lib/Basic/Targets/X86.cpp
  clang/test/Sema/x86-no-x87.cpp


Index: clang/test/Sema/x86-no-x87.cpp
===
--- clang/test/Sema/x86-no-x87.cpp
+++ clang/test/Sema/x86-no-x87.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -triple i686-linux-gnu 
-target-feature -x87 -DRET_ERROR
+// RUN: %clang_cc1 -fsyntax-only -verify %s -triple i686-linux-gnu 
-target-feature -x87
 // RUN: %clang_cc1 -fsyntax-only -verify %s -triple i686-linux-gnu -DNOERROR
 
 #ifdef NOERROR
@@ -123,42 +123,22 @@
   long_double ld = 0.42;
 }
 
-#ifndef NOERROR
-// expected-note@+3{{'d_ret1' defined here}}
-// expected-error@+2{{'d_ret1' requires  'double' return type support, but 
target 'i686-unknown-linux-gnu' does not support it}}
-#endif
 double d_ret1(float x) {
   return 0.0;
 }
 
-#ifndef NOERROR
-// expected-note@+2{{'d_ret2' defined here}}
-#endif
 double d_ret2(float x);
 
 int d_ret3(float x) {
-#ifndef NOERROR
-  // expected-error@+2{{'d_ret2' requires  'double' return type support, but 
target 'i686-unknown-linux-gnu' does not support it}}
-#endif
   return (int)d_ret2(x);
 }
 
-#ifndef NOERROR
-// expected-note@+3{{'f_ret1' defined here}}
-// expected-error@+2{{'f_ret1' requires  'float' return type support, but 
target 'i686-unknown-linux-gnu' does not support it}}
-#endif
 float f_ret1(float x) {
   return 0.0f;
 }
 
-#ifndef NOERROR
-// expected-note@+2{{'f_ret2' defined here}}
-#endif
 float f_ret2(float x);
 
 int f_ret3(float x) {
-#ifndef NOERROR
-  // expected-error@+2{{'f_ret2' requires  'float' return type support, but 
target 'i686-unknown-linux-gnu' does not support it}}
-#endif
   return (int)f_ret2(x);
 }
Index: clang/lib/Basic/Targets/X86.cpp
===
--- clang/lib/Basic/Targets/X86.cpp
+++ clang/lib/Basic/Targets/X86.cpp
@@ -382,12 +382,10 @@
   SimdDefaultAlign =
   hasFeature("avx512f") ? 512 : hasFeature("avx") ? 256 : 128;
 
-  if (!HasX87) {
-if (LongDoubleFormat == &llvm::APFloat::x87DoubleExtended())
-  HasLongDouble = false;
-if (getTriple().getArch() == llvm::Triple::x86)
-  HasFPReturn = false;
-  }
+  // FIXME: We should allow long double type on 32-bits to match with GCC.
+  // This requires backend to be able to lower f80 without x87 first.
+  if (!HasX87 && LongDoubleFormat == &llvm::APFloat::x87DoubleExtended())
+HasLongDouble = false;
 
   return true;
 }


Index: clang/test/Sema/x86-no-x87.cpp
===
--- clang/test/Sema/x86-no-x87.cpp
+++ clang/test/Sema/x86-no-x87.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -triple i686-linux-gnu -target-feature -x87 -DRET_ERROR
+// RUN: %clang_cc1 -fsyntax-only -verify %s -triple i686-linux-gnu -target-feature -x87
 // RUN: %clang_cc1 -fsyntax-only -verify %s -triple i686-linux-gnu -DNOERROR
 
 #ifdef NOERROR
@@ -123,42 +123,22 @@
   long_double ld = 0.42;
 }
 
-#ifndef NOERROR
-// expected-note@+3{{'d_ret1' defined here}}
-// expected-error@+2{{'d_ret1' requires  'double' return type support, but target 'i686-unknown-linux-gnu' does not support it}}
-#endif
 double d_ret1(float x) {
   return 0.0;
 }
 
-#ifndef NOERROR
-// expected-note@+2{{'d_ret2' defined here}}
-#endif
 double d_ret2(float x);
 
 int d_ret3(float x) {
-#ifndef NOERROR
-  // expected-error@+2{{'d_ret2' requires  'double' return type support, but target 'i686-unknown-linux-gnu' does not support it}}
-#endif
   return (int)d_ret2(x);
 }
 
-#ifndef NOERROR
-// expected-note@+3{{'f_ret1' defined here}}
-// expected-error@+2{{'f_ret1' requires  'float' return type support, but target 'i686-unknown-linux-gnu' does not support it}}
-#endif
 float f_ret1(float x) {
   return 0.0f;
 }
 
-#ifndef NOERROR
-// expected-note@+2{{'f_ret2' defined here}}
-#endif
 float f_ret2(float x);
 
 int f_ret3(float x) {
-#ifndef NOERROR
-  // expected-error@+2{{'f_ret2' requires  'float' return type support, but target 'i686-unknown-linux-gnu' does not support it}}
-#endif
   return (int)f_ret2(x);
 }
Index: clang/lib/Basic/Targets/X86.cpp
===
--- clang/lib/Basic/Targets/X86.cpp
+++ clang/lib/Basic/Targets/X86.cpp
@@ -382,12 +382,10 @@
   SimdDefaultAlign =
   hasFeature("avx512f") ? 512 : hasFeature("avx") ? 256 : 128;
 
-  if (!HasX87) {
-if (LongDoubleFormat == &llvm::APFloat::x87DoubleExtended())
-  HasLongDouble = false;
-if (getTriple().getArch() == llvm::Triple::x86)
-  HasFPReturn = false;
-  }
+  // FIXME: We should allow long doubl

[clang] 42c15c7 - [X86][clang] Enable floating-point type for -mno-x87 option on 32-bits

[Phoebe Wang via cfe-commits]

Author: Phoebe Wang
Date: 2021-11-30T14:08:10+08:00
New Revision: 42c15c7edf174fc7a45131a1b89ee816fada7633

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

LOG: [X86][clang] Enable floating-point type for -mno-x87 option on 32-bits

We should match GCC's behavior which allows floating-point type for -mno-x87 
option on 32-bits. https://godbolt.org/z/KrbhfWc9o

The previous block issues have partially been fixed by D112143.

Reviewed By: asavonic, nickdesaulniers

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

Added: 


Modified: 
clang/lib/Basic/Targets/X86.cpp
clang/test/Sema/x86-no-x87.cpp

Removed: 




diff  --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
index 454a7743dded3..5c4bd364b06a3 100644
--- a/clang/lib/Basic/Targets/X86.cpp
+++ b/clang/lib/Basic/Targets/X86.cpp
@@ -382,12 +382,10 @@ bool 
X86TargetInfo::handleTargetFeatures(std::vector &Features,
   SimdDefaultAlign =
   hasFeature("avx512f") ? 512 : hasFeature("avx") ? 256 : 128;
 
-  if (!HasX87) {
-if (LongDoubleFormat == &llvm::APFloat::x87DoubleExtended())
-  HasLongDouble = false;
-if (getTriple().getArch() == llvm::Triple::x86)
-  HasFPReturn = false;
-  }
+  // FIXME: We should allow long double type on 32-bits to match with GCC.
+  // This requires backend to be able to lower f80 without x87 first.
+  if (!HasX87 && LongDoubleFormat == &llvm::APFloat::x87DoubleExtended())
+HasLongDouble = false;
 
   return true;
 }

diff  --git a/clang/test/Sema/x86-no-x87.cpp b/clang/test/Sema/x86-no-x87.cpp
index 112f6bff7e1c8..2421ea15a57af 100644
--- a/clang/test/Sema/x86-no-x87.cpp
+++ b/clang/test/Sema/x86-no-x87.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -triple i686-linux-gnu 
-target-feature -x87 -DRET_ERROR
+// RUN: %clang_cc1 -fsyntax-only -verify %s -triple i686-linux-gnu 
-target-feature -x87
 // RUN: %clang_cc1 -fsyntax-only -verify %s -triple i686-linux-gnu -DNOERROR
 
 #ifdef NOERROR
@@ -123,42 +123,22 @@ void assign5() {
   long_double ld = 0.42;
 }
 
-#ifndef NOERROR
-// expected-note@+3{{'d_ret1' defined here}}
-// expected-error@+2{{'d_ret1' requires  'double' return type support, but 
target 'i686-unknown-linux-gnu' does not support it}}
-#endif
 double d_ret1(float x) {
   return 0.0;
 }
 
-#ifndef NOERROR
-// expected-note@+2{{'d_ret2' defined here}}
-#endif
 double d_ret2(float x);
 
 int d_ret3(float x) {
-#ifndef NOERROR
-  // expected-error@+2{{'d_ret2' requires  'double' return type support, but 
target 'i686-unknown-linux-gnu' does not support it}}
-#endif
   return (int)d_ret2(x);
 }
 
-#ifndef NOERROR
-// expected-note@+3{{'f_ret1' defined here}}
-// expected-error@+2{{'f_ret1' requires  'float' return type support, but 
target 'i686-unknown-linux-gnu' does not support it}}
-#endif
 float f_ret1(float x) {
   return 0.0f;
 }
 
-#ifndef NOERROR
-// expected-note@+2{{'f_ret2' defined here}}
-#endif
 float f_ret2(float x);
 
 int f_ret3(float x) {
-#ifndef NOERROR
-  // expected-error@+2{{'f_ret2' requires  'float' return type support, but 
target 'i686-unknown-linux-gnu' does not support it}}
-#endif
   return (int)f_ret2(x);
 }



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

[PATCH] D114408: Fold a lot of ffixed_x if judgments

[Shao-Ce SUN via Phabricator via cfe-commits]

achieveartificialintelligence abandoned this revision.
achieveartificialintelligence added a comment.

In D114408#3148311 , @jrtc27 wrote:

> This has clearly not been tested whatsoever, it’s totally broken, the 
> preprocessor does not work like that

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114408

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

[PATCH] D114163: Use VersionTuple for parsing versions in Triple. This makes it possible to distinguish between "16" and "16.0" after parsing, which previously was not possible.

[Alex Lorenz via Phabricator via cfe-commits]

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

Sorry for delay, this LGTM. Thanks for doing this cleanup!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114163

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

[PATCH] D114764: Use `thin` that starts with lowercase letter. - `Thin` gives an error when following https://clang.llvm.org/docs/ThinLTO.html#clang-bootstrap, and `thin` works.

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D114764#3160212 , @luna wrote:

> In D114764#3160204 , @tejohnson 
> wrote:
>
>> There are some other docs that also use "Thin" (and "Full", etc). I see my 
>> own script uses "THIN" for this cmake variable. Looks like we convert to 
>> uppercase so I wouldn't think it should matter:
>> See llvm/cmake/modules/HandleLLVMOptions.cmake:30
>>
>> Can you clarify what problem you are encountering? I'm not sure I understand 
>> the reference to WholeProgramDevirt.cpp in the title.
>
> Thanks for pointing this out! The title should be 
> https://clang.llvm.org/docs/ThinLTO.html#clang-bootstrap (WholeProgramDevirt 
> should be from stale paste board).
>
> I made a mistake in the command; re-run the command works for me [1]; 
> previously I remember seeing  `flto` 'Thin' in the terminal, so must have 
> mixed the `clang` cmmand and `cmake` command.

Ah ok, glad that works now. Yes, it is case sensitive lowercase for the clang 
option, but not for the cmake variable.

> Going to revert this.
>
> [1] cmake -G Ninja -DCMAKE_BUILD_TYPE=Debug -DLLVM_ENABLE_SPHINX=true 
> -DLLVM_USE_LINKER=lld -DLLVM_CCACHE_BUILD=On -DSPHINX_OUTPUT_HTML=true 
> -DLLVM_ENABLE_PROJECTS='clang;compiler-rt;lld' /path/to/llvm-project/llvm 
> -DLLVM_BINUTILS_INCDIR=/path/to/binutils/include -DLLVM_ENABLE_LTO=Thin 
> -DLLVM_PARALLEL_LINK_JOBS=1 -DCMAKE_C_COMPILER=/usr/local/bin/clang 
> -DCMAKE_CXX_COMPILER=/usr/local/bin/clang++ 
> -DCMAKE_RANLIB=/usr/local/bin/llvm-ranlib -DCMAKE_AR=/usr/local/bin/llvm-ar




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114764

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

[PATCH] D114764: Use `thin` that starts with lowercase letter. - `Thin` gives an error when following https://clang.llvm.org/docs/ThinLTO.html#clang-bootstrap, and `thin` works.

[Mingming Liu via Phabricator via cfe-commits]

luna added a comment.

In D114764#3160204 , @tejohnson wrote:

> There are some other docs that also use "Thin" (and "Full", etc). I see my 
> own script uses "THIN" for this cmake variable. Looks like we convert to 
> uppercase so I wouldn't think it should matter:
> See llvm/cmake/modules/HandleLLVMOptions.cmake:30
>
> Can you clarify what problem you are encountering? I'm not sure I understand 
> the reference to WholeProgramDevirt.cpp in the title.

Thanks for pointing this out! The title should be 
https://clang.llvm.org/docs/ThinLTO.html#clang-bootstrap (WholeProgramDevirt 
should be from stale paste board).

I made a mistake in the command; re-run the command works for me [1]; 
previously I remember seeing  `flto` 'Thin' in the terminal, so must have mixed 
the `clang` cmmand and `cmake` command.

Going to revert this.

[1] cmake -G Ninja -DCMAKE_BUILD_TYPE=Debug -DLLVM_ENABLE_SPHINX=true 
-DLLVM_USE_LINKER=lld -DLLVM_CCACHE_BUILD=On -DSPHINX_OUTPUT_HTML=true 
-DLLVM_ENABLE_PROJECTS='clang;compiler-rt;lld' /path/to/llvm-project/llvm 
-DLLVM_BINUTILS_INCDIR=/path/to/binutils/include -DLLVM_ENABLE_LTO=Thin 
-DLLVM_PARALLEL_LINK_JOBS=1 -DCMAKE_C_COMPILER=/usr/local/bin/clang 
-DCMAKE_CXX_COMPILER=/usr/local/bin/clang++ 
-DCMAKE_RANLIB=/usr/local/bin/llvm-ranlib -DCMAKE_AR=/usr/local/bin/llvm-ar


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114764

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

[PATCH] D114764: Use `thin` that starts with lowercase letter. - `Thin` gives an error when following lib/Transforms/IPO/WholeProgramDevirt.cpp, and `thin` works.

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

There are some other docs that also use "Thin" (and "Full", etc). I see my own 
script uses "THIN" for this cmake variable. Looks like we convert to uppercase 
so I wouldn't think it should matter:
See llvm/cmake/modules/HandleLLVMOptions.cmake:30

Can you clarify what problem you are encountering? I'm not sure I understand 
the reference to WholeProgramDevirt.cpp in the title.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114764

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

[PATCH] D114764: Use `thin` that starts with lowercase letter. - `Thin` gives an error when following lib/Transforms/IPO/WholeProgramDevirt.cpp, and `thin` works.

[Mingming Liu via Phabricator via cfe-commits]

luna created this revision.
luna added a reviewer: tejohnson.
Herald added subscribers: ormris, steven_wu, hiraditya.
luna requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fix a typo.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114764

Files:
  clang/docs/ThinLTO.rst


Index: clang/docs/ThinLTO.rst
===
--- clang/docs/ThinLTO.rst
+++ clang/docs/ThinLTO.rst
@@ -213,7 +213,7 @@
`_
when configuring the bootstrap compiler build:
 
-  * ``-DLLVM_ENABLE_LTO=Thin``
+  * ``-DLLVM_ENABLE_LTO=thin``
   * ``-DCMAKE_C_COMPILER=/path/to/host/clang``
   * ``-DCMAKE_CXX_COMPILER=/path/to/host/clang++``
   * ``-DCMAKE_RANLIB=/path/to/host/llvm-ranlib``
@@ -221,7 +221,7 @@
 
   Or, on Windows:
 
-  * ``-DLLVM_ENABLE_LTO=Thin``
+  * ``-DLLVM_ENABLE_LTO=thin``
   * ``-DCMAKE_C_COMPILER=/path/to/host/clang-cl.exe``
   * ``-DCMAKE_CXX_COMPILER=/path/to/host/clang-cl.exe``
   * ``-DCMAKE_LINKER=/path/to/host/lld-link.exe``


Index: clang/docs/ThinLTO.rst
===
--- clang/docs/ThinLTO.rst
+++ clang/docs/ThinLTO.rst
@@ -213,7 +213,7 @@
`_
when configuring the bootstrap compiler build:
 
-  * ``-DLLVM_ENABLE_LTO=Thin``
+  * ``-DLLVM_ENABLE_LTO=thin``
   * ``-DCMAKE_C_COMPILER=/path/to/host/clang``
   * ``-DCMAKE_CXX_COMPILER=/path/to/host/clang++``
   * ``-DCMAKE_RANLIB=/path/to/host/llvm-ranlib``
@@ -221,7 +221,7 @@
 
   Or, on Windows:
 
-  * ``-DLLVM_ENABLE_LTO=Thin``
+  * ``-DLLVM_ENABLE_LTO=thin``
   * ``-DCMAKE_C_COMPILER=/path/to/host/clang-cl.exe``
   * ``-DCMAKE_CXX_COMPILER=/path/to/host/clang-cl.exe``
   * ``-DCMAKE_LINKER=/path/to/host/lld-link.exe``
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114565: [InstrProf] Attach debug info to counters

[Kyungwoo Lee via Phabricator via cfe-commits]

kyulee added inline comments.



Comment at: compiler-rt/lib/profile/InstrProfilingWriter.c:273
 
-  if (!DataSize)
+  if (!CountersSize)
 return 0;

Is it no diff change? I wonder if there is a case where `CountersSize` is 0 but 
not `DataSize`  from a value probe.
If not, should we make a separate change?



Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:647
+  PD.NumValueSites[ValueKind] =
+  std::max(PD.NumValueSites[ValueKind], (uint32_t)(Index + 1));
 }

It appears this is a NFC. Should we make a separate change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114565

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

[PATCH] D114095: [clang][lex] Include tracking: simplify and move to preprocessor

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

I've mentioned it in D112915  as we've 
discussed the stored data format there. But my concern was that bitvector 
packing might be not the most space-efficient encoding. I haven't done proper 
testing, just off-the-cuff comparison and it looks like for the most of 
frameworks in iOS SDK storing included headers per submodule takes less space 
than encoding them as a bitvector. I have an idea why that might be happening 
but I haven't checked it in debugger, so'll keep it to myself to avoid 
derailing the discussion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114095

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

[PATCH] D114758: [analyzer][solver] Introduce reasoning for not equal to operator

[Manas Gupta via Phabricator via cfe-commits]

manas added a comment.

Closing this as this is mistakenly a duplicate of existing differential D112621 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114758

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

[PATCH] D112621: [analyzer][solver] Introduce reasoning for not equal to operator

[Manas Gupta via Phabricator via cfe-commits]

manas added a comment.

I have made few changes:

1. The failed assertions  due to comparison 
between different types have been fixed by converting all the Ranges to a given 
type. This will allow comparisons without throwing errors.

2. There was also a build error due to `explicit specialization in 
non-namespace scope`. This was fixed by @martong previously, but that patch led 
to the above mentioned bug. So I used @martong 's patch here to make GCC happy.

3. I have added a small check for comparison between different types.

https://reviews.llvm.org/D106102 differential contains the background story.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112621

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

[PATCH] D112621: [analyzer][solver] Introduce reasoning for not equal to operator

[Manas Gupta via Phabricator via cfe-commits]

manas updated this revision to Diff 390526.
manas added a comment.

Fix comparison between different types and add a test to check it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112621

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/constant-folding.c

Index: clang/test/Analysis/constant-folding.c
===
--- clang/test/Analysis/constant-folding.c
+++ clang/test/Analysis/constant-folding.c
@@ -281,3 +281,55 @@
 clang_analyzer_eval((b % a) < x + 10); // expected-warning{{TRUE}}
   }
 }
+
+void testDisequalityRules(unsigned int u1, unsigned int u2, int s1, int s2) {
+  // Checks when ranges are not overlapping
+  if (u1 <= 10 && u2 >= 20) {
+// u1: [0,10], u2: [20,UINT_MAX]
+clang_analyzer_eval((u1 != u2) != 0); // expected-warning{{TRUE}}
+  }
+
+  if (s1 <= INT_MIN + 10 && s2 >= INT_MAX - 10) {
+// s1: [INT_MIN,INT_MIN + 10], s2: [INT_MAX - 10,INT_MAX]
+clang_analyzer_eval((s1 != s2) == 0); // expected-warning{{FALSE}}
+  }
+
+  // Checks when ranges are completely overlapping and have more than one point
+  if (u1 >= 20 && u1 <= 50 && u2 >= 20 && u2 <= 50) {
+// u1: [20,50], u2: [20,50]
+clang_analyzer_eval((u1 != u2) != 0); // expected-warning{{UNKNOWN}}
+  }
+
+  if (s1 >= -20 && s1 <= 20 && s2 >= -20 && s2 <= 20) {
+// s1: [-20,20], s2: [-20,20]
+clang_analyzer_eval((s1 != s2) != 0); // expected-warning{{UNKNOWN}}
+  }
+
+  // Checks when ranges are partially overlapping
+  if (u1 >= 100 && u1 <= 200 && u2 >= 150 && u2 <= 300) {
+// u1: [100,200], u2: [150,300]
+clang_analyzer_eval((u1 != u2) != 0); // expected-warning{{UNKNOWN}}
+  }
+
+  if (s1 >= -80 && s1 <= -50 && s2 >= -100 && s2 <= -75) {
+// s1: [-80,-50], s2: [-100,-75]
+clang_analyzer_eval((s1 != s2) == 0); // expected-warning{{UNKNOWN}}
+  }
+
+  // Checks for ranges which are subset of one-another
+  if (u1 >= 500 && u1 <= 1000 && u2 >= 750 && u2 <= 1000) {
+// u1: [500,1000], u2: [750,1000]
+clang_analyzer_eval((u1 != u2) == 0); // expected-warning{{UNKNOWN}}
+  }
+
+  if (s1 >= -1000 && s1 <= -500 && s2 <= -500 && s2 >= -750) {
+// s1: [-1000,-500], s2: [-500,-750]
+clang_analyzer_eval((s1 != s2) == 0); // expected-warning{{UNKNOWN}}
+  }
+
+  // Checks for comparison between different types
+  if (u1 <= 255 && s1 <= 0) {
+// u1: [0, 255], s1: [INT_MIN, 0]
+clang_analyzer_eval((u1 != s1) == 0); // expected-warning{{UNKNOWN}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -20,8 +20,8 @@
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/ImmutableSet.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -960,18 +960,7 @@
   }
 
   RangeSet VisitBinaryOperator(RangeSet LHS, BinaryOperator::Opcode Op,
-   RangeSet RHS, QualType T) {
-switch (Op) {
-case BO_Or:
-  return VisitBinaryOperator(LHS, RHS, T);
-case BO_And:
-  return VisitBinaryOperator(LHS, RHS, T);
-case BO_Rem:
-  return VisitBinaryOperator(LHS, RHS, T);
-default:
-  return infer(T);
-}
-  }
+   RangeSet RHS, QualType T);
 
   //===--===//
   // Ranges and operators
@@ -1236,6 +1225,67 @@
 //   Range-based reasoning about symbolic operations
 //===--===//
 
+template <>
+RangeSet SymbolicRangeInferrer::VisitBinaryOperator(RangeSet LHS,
+   RangeSet RHS,
+   QualType T) {
+
+  // We must check for empty RangeSets. This gets done via VisitBinaryOperator
+  // for other operators, but BO_NE is handled specifically.
+  if (LHS.isEmpty() || RHS.isEmpty()) {
+return RangeFactory.getEmptySet();
+  }
+
+  APSIntType ResultType = ValueFactory.getAPSIntType(T);
+
+  RangeSet ConvertedLHS = RangeFactory.getEmptySet();
+  RangeSet ConvertedRHS = RangeFactory.getEmptySet();
+
+  // Convert each Range inside the RangeSet to given type. This will make it
+  // possible to have comparisons between different APSInts.
+  for (auto LRange : LHS) {
+auto ConvertedLRange = convert(LRange, ResultType);
+
+// If any of the converted Range is not bounded within the limits of the
+// type, then we must not compare the APSInt values. Hence, we have no
+// option 

[PATCH] D114565: [InstrProf] Attach debug info to counters

[Ellis Hoag via Phabricator via cfe-commits]

ellis created this revision.
Herald added subscribers: ormris, wenlei, hiraditya.
ellis edited the summary of this revision.
ellis added reviewers: MaskRay, alanphipps, wenlei, kyulee, davidxl.
ellis edited the summary of this revision.
ellis published this revision for review.
Herald added projects: clang, Sanitizers, LLVM.
Herald added subscribers: llvm-commits, Sanitizers, cfe-commits.

Add the llvm flag `-debug-info-correlate` to attach debug info to 
instrumentation counters so we can correlate raw profile data to their 
functions. Raw profiles are dumped as `.proflite` files. The next diff enables 
`llvm-profdata` to consume `.proflite` and debug info files to produce a normal 
`.profdata` profile.

Part of the "lightweight instrumentation" work: 
https://groups.google.com/g/llvm-dev/c/r03Z6JoN7d4


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114565

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  compiler-rt/include/profile/InstrProfData.inc
  compiler-rt/lib/profile/InstrProfiling.c
  compiler-rt/lib/profile/InstrProfilingMerge.c
  compiler-rt/lib/profile/InstrProfilingWriter.c
  llvm/include/llvm/ProfileData/InstrProf.h
  llvm/include/llvm/ProfileData/InstrProfData.inc
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/lib/ProfileData/InstrProf.cpp
  llvm/lib/ProfileData/InstrProfWriter.cpp
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/test/Instrumentation/InstrProfiling/debug-info-correlate.ll

Index: llvm/test/Instrumentation/InstrProfiling/debug-info-correlate.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/InstrProfiling/debug-info-correlate.ll
@@ -0,0 +1,51 @@
+; RUN: opt < %s -instrprof -debug-info-correlate -S > %t.ll
+; RUN: FileCheck < %t.ll %s
+; RUN: llc < %t.ll -mtriple=arm64-unknown-linux-gnu | FileCheck %s --check-prefix CHECK-ASM
+
+target triple = "aarch64-unknown-linux-gnu"
+
+@__profn_foo = private constant [3 x i8] c"foo"
+; CHECK:  @__profc_foo =
+; CHECK-SAME: !dbg ![[EXPR:[0-9]+]]
+
+; CHECK:  ![[EXPR]] = !DIGlobalVariableExpression(var: ![[GLOBAL:[0-9]+]]
+; CHECK:  ![[GLOBAL]] = {{.*}} !DIGlobalVariable(name: "__profc_foo"
+; CHECK-SAME: scope: ![[SCOPE:[0-9]+]]
+; CHECK-SAME: annotations: ![[ANNOTATIONS:[0-9]+]]
+; CHECK:  ![[SCOPE]] = {{.*}} !DISubprogram(name: "foo"
+; CHECK:  ![[ANNOTATIONS]] = !{![[NAME:[0-9]+]], ![[HASH:[0-9]+]], ![[COUNTERS:[0-9]+]]}
+; CHECK:  ![[NAME]] = !{!"Function Name", !"foo"}
+; CHECK:  ![[HASH]] = !{!"CFG Hash", !DIExpression(DW_OP_constu, 12345678,
+; CHECK:  ![[COUNTERS]] = !{!"Num Counters", !DIExpression(DW_OP_constu, 2,
+
+; CHECK-ASM-NOT: .section   __llvm_prf_data
+; CHECK-ASM-NOT: .section   __llvm_prf_names
+
+define void @_Z3foov() !dbg !12 {
+  call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__profn_foo, i32 0, i32 0), i64 12345678, i32 2, i32 0)
+  ret void
+}
+
+declare void @llvm.instrprof.increment(i8*, i64, i32, i32)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8, !9, !10}
+!llvm.ident = !{!11}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 14.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "debug-info-correlate.cpp", directory: "")
+!2 = !{i32 7, !"Dwarf Version", i32 4}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 1, !"branch-target-enforcement", i32 0}
+!6 = !{i32 1, !"sign-return-address", i32 0}
+!7 = !{i32 1, !"sign-return-address-all", i32 0}
+!8 = !{i32 1, !"sign-return-address-with-bkey", i32 0}
+!9 = !{i32 7, !"uwtable", i32 1}
+!10 = !{i32 7, !"frame-pointer", i32 1}
+!11 = !{!"clang version 14.0.0"}
+!12 = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov", scope: !13, file: !13, line: 1, type: !14, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !16)
+!13 = !DIFile(filename: "debug-info-correlate.cpp", directory: "")
+!14 = !DISubroutineType(types: !15)
+!15 = !{null}
+!16 = !{}
Index: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
===
--- llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -291,6 +291,8 @@
 // Command line option to specify the name of the function for CFG dump
 // Defined in Analysis/BlockFrequencyInfo.cpp:  -view-bfi-func-name=
 extern cl::opt ViewBlockFreqFuncName;
+
+extern cl::opt DebugInfoCorrelate;
 } // namespace llvm
 
 static cl::opt
@@ -467,8 +469,9 @@
 createProfileFileNameVar(M, InstrProfileOutput);
 // The variable in a comdat may be discarded by LTO. Ensure the
 // declaration will be retained.
-appendToCompilerUsed(

[PATCH] D114758: [analyzer][solver] Introduce reasoning for not equal to operator

[Manas Gupta via Phabricator via cfe-commits]

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

With this patch, the solver can infer results for not equal to operator
over Ranges as well. This also fixes the issue of comparison between
different types, by first converting the RangeSets to resulting type,
which then can be used for comparisons.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114758

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/constant-folding.c

Index: clang/test/Analysis/constant-folding.c
===
--- clang/test/Analysis/constant-folding.c
+++ clang/test/Analysis/constant-folding.c
@@ -281,3 +281,55 @@
 clang_analyzer_eval((b % a) < x + 10); // expected-warning{{TRUE}}
   }
 }
+
+void testDisequalityRules(unsigned int u1, unsigned int u2, int s1, int s2) {
+  // Checks when ranges are not overlapping
+  if (u1 <= 10 && u2 >= 20) {
+// u1: [0,10], u2: [20,UINT_MAX]
+clang_analyzer_eval((u1 != u2) != 0); // expected-warning{{TRUE}}
+  }
+
+  if (s1 <= INT_MIN + 10 && s2 >= INT_MAX - 10) {
+// s1: [INT_MIN,INT_MIN + 10], s2: [INT_MAX - 10,INT_MAX]
+clang_analyzer_eval((s1 != s2) == 0); // expected-warning{{FALSE}}
+  }
+
+  // Checks when ranges are completely overlapping and have more than one point
+  if (u1 >= 20 && u1 <= 50 && u2 >= 20 && u2 <= 50) {
+// u1: [20,50], u2: [20,50]
+clang_analyzer_eval((u1 != u2) != 0); // expected-warning{{UNKNOWN}}
+  }
+
+  if (s1 >= -20 && s1 <= 20 && s2 >= -20 && s2 <= 20) {
+// s1: [-20,20], s2: [-20,20]
+clang_analyzer_eval((s1 != s2) != 0); // expected-warning{{UNKNOWN}}
+  }
+
+  // Checks when ranges are partially overlapping
+  if (u1 >= 100 && u1 <= 200 && u2 >= 150 && u2 <= 300) {
+// u1: [100,200], u2: [150,300]
+clang_analyzer_eval((u1 != u2) != 0); // expected-warning{{UNKNOWN}}
+  }
+
+  if (s1 >= -80 && s1 <= -50 && s2 >= -100 && s2 <= -75) {
+// s1: [-80,-50], s2: [-100,-75]
+clang_analyzer_eval((s1 != s2) == 0); // expected-warning{{UNKNOWN}}
+  }
+
+  // Checks for ranges which are subset of one-another
+  if (u1 >= 500 && u1 <= 1000 && u2 >= 750 && u2 <= 1000) {
+// u1: [500,1000], u2: [750,1000]
+clang_analyzer_eval((u1 != u2) == 0); // expected-warning{{UNKNOWN}}
+  }
+
+  if (s1 >= -1000 && s1 <= -500 && s2 <= -500 && s2 >= -750) {
+// s1: [-1000,-500], s2: [-500,-750]
+clang_analyzer_eval((s1 != s2) == 0); // expected-warning{{UNKNOWN}}
+  }
+
+  // Checks for comparison between different types
+  if (u1 <= 255 && s1 <= 0) {
+// u1: [0, 255], s1: [INT_MIN, 0]
+clang_analyzer_eval((u1 != s1) == 0); // expected-warning{{UNKNOWN}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -20,8 +20,8 @@
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/ImmutableSet.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -960,18 +960,7 @@
   }
 
   RangeSet VisitBinaryOperator(RangeSet LHS, BinaryOperator::Opcode Op,
-   RangeSet RHS, QualType T) {
-switch (Op) {
-case BO_Or:
-  return VisitBinaryOperator(LHS, RHS, T);
-case BO_And:
-  return VisitBinaryOperator(LHS, RHS, T);
-case BO_Rem:
-  return VisitBinaryOperator(LHS, RHS, T);
-default:
-  return infer(T);
-}
-  }
+   RangeSet RHS, QualType T);
 
   //===--===//
   // Ranges and operators
@@ -1236,6 +1225,67 @@
 //   Range-based reasoning about symbolic operations
 //===--===//
 
+template <>
+RangeSet SymbolicRangeInferrer::VisitBinaryOperator(RangeSet LHS,
+   RangeSet RHS,
+   QualType T) {
+
+  // We must check for empty RangeSets. This gets done via VisitBinaryOperator
+  // for other operators, but BO_NE is handled specifically.
+  if (LHS.isEmpty() || RHS.isEmpty()) {
+return RangeFactory.getEmptySet();
+  }
+
+  APSIntType ResultType = ValueFactory.getAPSIntType(T);
+
+  RangeSet ConvertedLHS = RangeFactory.getEmptySet();
+  RangeSet ConvertedRHS = RangeFactory.getEmptySet();
+
+  // Convert each Range inside the RangeSet t

[PATCH] D114639: Raise the minimum Visual Studio version to VS2019

[Stella Stamenova via Phabricator via cfe-commits]

stella.stamenova added a comment.

In D114639#3160031 , @mehdi_amini 
wrote:

> In D114639#3158401 , @RKSimon wrote:
>
>>> Have you checked whether there are any bots in the lab that will need to be 
>>> updated?
>>
>> I did find a number of bots that I think need addressing, I am intending to 
>> privately email the owners but I haven't done that yet - @rnk maintains the 
>> sanitizer-windows and windows-gcebot2 machines, but there are others.
>
> This bot as well: https://lab.llvm.org/buildbot/#/builders/13 ; adding 
> @stella.stamenova here.

The mlir and lldb windows bots will both need to be updated to 2019/2022. We 
haven't been building with 2022, so I am not sure how stable the builds will be 
if we moved to it (though I hope it just works). We can plan to update both 
bots to use 2019/2022 this week before our whole teams goes on vacation.




Comment at: clang/docs/UsersManual.rst:3546
 
-cmake -G"Visual Studio 15 2017" -T LLVM ..
+cmake -G"Visual Studio 17 2022" -T LLVM ..
 

compnerd wrote:
> Meinersbur wrote:
> > jhenderson wrote:
> > > RKSimon wrote:
> > > > aaron.ballman wrote:
> > > > > jhenderson wrote:
> > > > > > I think the missing space should be fixed to :)
> > > > > +1 to the missing space.
> > > > This one is confusing - it isn't in my local diff, the raw diff, or if 
> > > > I reapply the raw diff - it looks to have just appeared when you quote 
> > > > it in phab comments?
> > > The fact that the space is missing? It's missing in the current repo too.
> > It works with and without the space, like `-GNinja` and `-G Ninja` both 
> > work.
> It would be nice to mention the CMake minimum version.  I think that you need 
> 3.22 or newer for the 2022 generator/toolset definition.
It works with or without the space, but it reads better with the space.



Comment at: clang/docs/UsersManual.rst:3546
 
-cmake -G"Visual Studio 15 2017" -T LLVM ..
+cmake -G"Visual Studio 16 2019" -T LLVM ..
 

jhenderson wrote:
> Maybe make this VS2022 instead, to help it last longer?
I think it makes more sense to make it 2019 because I expect most people to 
still be using 2019 and it's convenient to have instructions that just work. 
Maybe add both?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114639

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

[PATCH] D114639: Raise the minimum Visual Studio version to VS2019

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

In D114639#3158401 , @RKSimon wrote:

>> Have you checked whether there are any bots in the lab that will need to be 
>> updated?
>
> I did find a number of bots that I think need addressing, I am intending to 
> privately email the owners but I haven't done that yet - @rnk maintains the 
> sanitizer-windows and windows-gcebot2 machines, but there are others.

This bot as well: https://lab.llvm.org/buildbot/#/builders/13 ; adding 
@stella.stamenova here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114639

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

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

[Sami Tolvanen via Phabricator via cfe-commits]

samitolvanen updated this revision to Diff 390521.
samitolvanen added a comment.

Use standard l-value conversions, and add a test case for `constexpr`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108479

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
  clang/test/CodeGen/builtin-function-start.cpp
  clang/test/SemaCXX/builtins.cpp

Index: clang/test/SemaCXX/builtins.cpp
===
--- clang/test/SemaCXX/builtins.cpp
+++ clang/test/SemaCXX/builtins.cpp
@@ -39,6 +39,13 @@
   S *ptmp = __builtin_addressof(S{}); // expected-error {{taking the address of a temporary}}
 }
 
+namespace function_start {
+void a(void) {}
+int n;
+void *p = __builtin_function_start(n);   // expected-error {{argument must be a function}}
+static_assert(__builtin_function_start(a) == a, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+} // namespace function_start
+
 void no_ms_builtins() {
   __assume(1); // expected-error {{use of undeclared}}
   __noop(1); // expected-error {{use of undeclared}}
Index: clang/test/CodeGen/builtin-function-start.cpp
===
--- /dev/null
+++ clang/test/CodeGen/builtin-function-start.cpp
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=cfi-icall -o - %s | FileCheck %s
+
+#if !__has_builtin(__builtin_function_start)
+#error "missing __builtin_function_start"
+#endif
+
+void a(void) {}
+void b(void) {}
+void c(void *p) {}
+constexpr void (*d)() = &a;
+
+// CHECK: @e = global i8* bitcast (void ()* no_cfi @_Z1av to i8*)
+const void *e = __builtin_function_start(a);
+// CHECK: @f = global i8* bitcast (void ()* no_cfi @_Z1av to i8*)
+const void *f = __builtin_function_start(a);
+// CHECK: @g = global [2 x i8*] [i8* bitcast (void ()* @_Z1bv to i8*), i8* bitcast (void ()* no_cfi @_Z1bv to i8*)]
+void *g[] = {(void *)b, __builtin_function_start(b)};
+
+class A {
+public:
+  void f();
+  virtual void g();
+  static void h();
+  int i() const;
+  int i(int n) const;
+};
+
+void A::f() {}
+void A::g() {}
+void A::h() {}
+
+// CHECK: define {{.*}}i32 @_ZNK1A1iEv(%class.A* {{.*}}%this)
+int A::i() const { return 0; }
+
+// CHECK: define {{.*}}i32 @_ZNK1A1iEi(%class.A* {{.*}}%this, i32 %n)
+int A::i(int n) const { return 0; }
+
+void h(void) {
+  // CHECK: store i8* bitcast (void ()* no_cfi @_Z1bv to i8*), i8** %g
+  void *g = __builtin_function_start(b);
+  // call void @_Z1cPv(i8* bitcast (void ()* no_cfi @_Z1av to i8*))
+  c(__builtin_function_start(a));
+
+  // CHECK: store i8* bitcast (void (%class.A*)* no_cfi @_ZN1A1fEv to i8*), i8** %Af
+  void *Af = __builtin_function_start(&A::f);
+  // CHECK: store i8* bitcast (void (%class.A*)* no_cfi @_ZN1A1gEv to i8*), i8** %Ag
+  void *Ag = __builtin_function_start(&A::g);
+  // CHECK: store i8* bitcast (void ()* no_cfi @_ZN1A1hEv to i8*), i8** %Ah
+  void *Ah = __builtin_function_start(&A::h);
+  // CHECK: store i8* bitcast (i32 (%class.A*)* no_cfi @_ZNK1A1iEv to i8*), i8** %Ai1
+  void *Ai1 = __builtin_function_start((int(A::*)() const) & A::i);
+  // CHECK: store i8* bitcast (i32 (%class.A*, i32)* no_cfi @_ZNK1A1iEi to i8*), i8** %Ai2
+  void *Ai2 = __builtin_function_start((int(A::*)(int) const) & A::i);
+}
Index: clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -66,7 +66,8 @@
   case Builtin::BI__builtin_expect:
   case Builtin::BI__builtin_expect_with_probability:
   case Builtin::BI__builtin_assume_aligned:
-  case Builtin::BI__builtin_addressof: {
+  case Builtin::BI__builtin_addressof:
+  case Builtin::BI__builtin_function_start: {
 // For __builtin_unpredictable, __builtin_expect,
 // __builtin_expect_with_probability and __builtin_assume_aligned,
 // just return the value of the subexpression.
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -195,6 +195,29 @@
   return false;
 }
 
+/// Check that the argument to __builtin_function_start is a function.
+static bool SemaBuiltinFunctionStart(Sema &S, CallExpr *TheCall) {
+  if (checkArgCount(S, TheCall, 1))
+return true;
+
+  ExprResult Arg 

[PATCH] D113995: [clangd] Dex Trigrams: Improve query trigram generation

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Basically LG now, some of the changes are a bit mysterious to me though




Comment at: clang-tools-extra/clangd/index/dex/Trigram.cpp:47
+  const size_t NPOS = std::numeric_limits::max();
+  llvm::SmallVector> 
Next(LowercaseIdentifier.size());
+  size_t NextTail = NPOS, NextHead = NPOS;

why the change from array to pair and 0 to NPOS?



Comment at: clang-tools-extra/clangd/index/dex/Trigram.cpp:82
+  //
+  // - First separator + first head
+  // - First head

Fist -> first
You're missing first separator only

I think examples might be clearer than a description of these.



Comment at: clang-tools-extra/clangd/index/dex/Trigram.cpp:102
+  LowercaseIdentifier[NextHead]));
+Position = NextHead;
+  }

(if the change to NPOS is just to avoid hitting Position == 0 on the first 
iteration, you could check it here instead)



Comment at: clang-tools-extra/clangd/index/dex/Trigram.cpp:148
 
+  // For queries with very few letters, emulate what generateIdentifierTrigrams
+  // outputs for the beginning of the Identifier.

Since the query trigram corresponds so closely to the query for short queries, 
I think it makes more sense to consider it the other way: 
generateIdentifierTrigrams is deciding which queries it wants to match and 
emulating this function.



Comment at: clang-tools-extra/clangd/index/dex/Trigram.cpp:151
+  if (UniqueTrigrams.empty()) {
+// If a bigram can't be formed, we might prepend a separator.
+std::string Result(1, LowercaseQuery.front());

nit: by prepending a separator, we form a bigram!
If a bigram can't be formed out of letters/numbers...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113995

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

[PATCH] D114149: [clang-tidy] Fix pr48613: "llvm-header-guard uses a reserved identifier"

[Salman Javed via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc7aa358798e6: [clang-tidy] Fix pr48613: 
"llvm-header-guard uses a reserved identifier" (authored by 
salman-javed-nz).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114149

Files:
  clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
  clang-tools-extra/clang-tidy/utils/HeaderGuard.h
  clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp


Index: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -219,6 +219,16 @@
 "", "/llvm-project/clang-tools-extra/clangd/foo.h",
 StringRef("header is missing header guard")));
 
+  // Substitution of characters should not result in a header guard starting
+  // with "_".
+  EXPECT_EQ("#ifndef BAR_H\n"
+"#define BAR_H\n"
+"\n"
+"\n"
+"#endif\n",
+runHeaderGuardCheck("", "include/--bar.h",
+StringRef("header is missing header guard")));
+
 #ifdef WIN32
   // Check interaction with Windows-style path separators (\).
   EXPECT_EQ(
Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.h
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.h
@@ -38,6 +38,9 @@
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
 
+  /// Ensure that the provided header guard is a non-reserved identifier.
+  std::string sanitizeHeaderGuard(StringRef Guard);
+
   /// Returns ``true`` if the check should suggest inserting a trailing comment
   /// on the ``#endif`` of the header guard. It will use the same name as
   /// returned by ``HeaderGuardCheck::getHeaderGuard``.
Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
@@ -164,10 +164,11 @@
  StringRef CurHeaderGuard,
  std::vector &FixIts) {
 std::string CPPVar = Check->getHeaderGuard(FileName, CurHeaderGuard);
+CPPVar = Check->sanitizeHeaderGuard(CPPVar);
 std::string CPPVarUnder = CPPVar + '_';
 
-// Allow a trailing underscore iff we don't have to change the endif 
comment
-// too.
+// Allow a trailing underscore if and only if we don't have to change the
+// endif comment too.
 if (Ifndef.isValid() && CurHeaderGuard != CPPVar &&
 (CurHeaderGuard != CPPVarUnder ||
  wouldFixEndifComment(FileName, EndIf, CurHeaderGuard))) {
@@ -216,6 +217,7 @@
 continue;
 
   std::string CPPVar = Check->getHeaderGuard(FileName);
+  CPPVar = Check->sanitizeHeaderGuard(CPPVar);
   std::string CPPVarUnder = CPPVar + '_'; // Allow a trailing underscore.
   // If there's a macro with a name that follows the header guard 
convention
   // but was not recognized by the preprocessor as a header guard there 
must
@@ -278,6 +280,11 @@
   PP->addPPCallbacks(std::make_unique(PP, this));
 }
 
+std::string HeaderGuardCheck::sanitizeHeaderGuard(StringRef Guard) {
+  // Only reserved identifiers are allowed to start with an '_'.
+  return Guard.drop_while([](char C) { return C == '_'; }).str();
+}
+
 bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef FileName) {
   return utils::isFileExtension(FileName, HeaderFileExtensions);
 }


Index: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -219,6 +219,16 @@
 "", "/llvm-project/clang-tools-extra/clangd/foo.h",
 StringRef("header is missing header guard")));
 
+  // Substitution of characters should not result in a header guard starting
+  // with "_".
+  EXPECT_EQ("#ifndef BAR_H\n"
+"#define BAR_H\n"
+"\n"
+"\n"
+"#endif\n",
+runHeaderGuardCheck("", "include/--bar.h",
+StringRef("header is missing header guard")));
+
 #ifdef WIN32
   // Check interaction with Windows-style path separators (\).
   EXPECT_EQ(
Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.h
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.h
@@ -38,6 +38,9 @@
   void re

[clang-tools-extra] c7aa358 - [clang-tidy] Fix pr48613: "llvm-header-guard uses a reserved identifier"

[Salman Javed via cfe-commits]

Author: Salman Javed
Date: 2021-11-30T12:43:35+13:00
New Revision: c7aa358798e6330593fd5cc2ff4caf6bc15ba3c9

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

LOG: [clang-tidy] Fix pr48613: "llvm-header-guard uses a reserved identifier"

Fixes https://bugs.llvm.org/show_bug.cgi?id=48613.

llvm-header-guard is suggesting header guards with leading underscores
if the header file path begins with a '/' or similar special character.
Only reserved identifiers should begin with an underscore.

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
clang-tools-extra/clang-tidy/utils/HeaderGuard.h
clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp 
b/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
index 1cae618dfd093..36bf64110a774 100644
--- a/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
+++ b/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
@@ -164,10 +164,11 @@ class HeaderGuardPPCallbacks : public PPCallbacks {
  StringRef CurHeaderGuard,
  std::vector &FixIts) {
 std::string CPPVar = Check->getHeaderGuard(FileName, CurHeaderGuard);
+CPPVar = Check->sanitizeHeaderGuard(CPPVar);
 std::string CPPVarUnder = CPPVar + '_';
 
-// Allow a trailing underscore iff we don't have to change the endif 
comment
-// too.
+// Allow a trailing underscore if and only if we don't have to change the
+// endif comment too.
 if (Ifndef.isValid() && CurHeaderGuard != CPPVar &&
 (CurHeaderGuard != CPPVarUnder ||
  wouldFixEndifComment(FileName, EndIf, CurHeaderGuard))) {
@@ -216,6 +217,7 @@ class HeaderGuardPPCallbacks : public PPCallbacks {
 continue;
 
   std::string CPPVar = Check->getHeaderGuard(FileName);
+  CPPVar = Check->sanitizeHeaderGuard(CPPVar);
   std::string CPPVarUnder = CPPVar + '_'; // Allow a trailing underscore.
   // If there's a macro with a name that follows the header guard 
convention
   // but was not recognized by the preprocessor as a header guard there 
must
@@ -278,6 +280,11 @@ void HeaderGuardCheck::registerPPCallbacks(const 
SourceManager &SM,
   PP->addPPCallbacks(std::make_unique(PP, this));
 }
 
+std::string HeaderGuardCheck::sanitizeHeaderGuard(StringRef Guard) {
+  // Only reserved identifiers are allowed to start with an '_'.
+  return Guard.drop_while([](char C) { return C == '_'; }).str();
+}
+
 bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef FileName) {
   return utils::isFileExtension(FileName, HeaderFileExtensions);
 }

diff  --git a/clang-tools-extra/clang-tidy/utils/HeaderGuard.h 
b/clang-tools-extra/clang-tidy/utils/HeaderGuard.h
index 86c0f3dc8fda2..584a09c756d49 100644
--- a/clang-tools-extra/clang-tidy/utils/HeaderGuard.h
+++ b/clang-tools-extra/clang-tidy/utils/HeaderGuard.h
@@ -38,6 +38,9 @@ class HeaderGuardCheck : public ClangTidyCheck {
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
 
+  /// Ensure that the provided header guard is a non-reserved identifier.
+  std::string sanitizeHeaderGuard(StringRef Guard);
+
   /// Returns ``true`` if the check should suggest inserting a trailing comment
   /// on the ``#endif`` of the header guard. It will use the same name as
   /// returned by ``HeaderGuardCheck::getHeaderGuard``.

diff  --git a/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp 
b/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
index 6b312413b870d..2a3fae05f585f 100644
--- a/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -219,6 +219,16 @@ TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
 "", "/llvm-project/clang-tools-extra/clangd/foo.h",
 StringRef("header is missing header guard")));
 
+  // Substitution of characters should not result in a header guard starting
+  // with "_".
+  EXPECT_EQ("#ifndef BAR_H\n"
+"#define BAR_H\n"
+"\n"
+"\n"
+"#endif\n",
+runHeaderGuardCheck("", "include/--bar.h",
+StringRef("header is missing header guard")));
+
 #ifdef WIN32
   // Check interaction with Windows-style path separators (\).
   EXPECT_EQ(



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

[PATCH] D113254: [clang] Fix a misadjusted path style comparison in a unittest

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Ping @dexonsmith


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113254

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

[PATCH] D114149: [clang-tidy] Fix pr48613: "llvm-header-guard uses a reserved identifier"

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz updated this revision to Diff 390464.
salman-javed-nz added a comment.

Update "iff" comment based on review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114149

Files:
  clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
  clang-tools-extra/clang-tidy/utils/HeaderGuard.h
  clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp


Index: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -219,6 +219,16 @@
 "", "/llvm-project/clang-tools-extra/clangd/foo.h",
 StringRef("header is missing header guard")));
 
+  // Substitution of characters should not result in a header guard starting
+  // with "_".
+  EXPECT_EQ("#ifndef BAR_H\n"
+"#define BAR_H\n"
+"\n"
+"\n"
+"#endif\n",
+runHeaderGuardCheck("", "include/--bar.h",
+StringRef("header is missing header guard")));
+
 #ifdef WIN32
   // Check interaction with Windows-style path separators (\).
   EXPECT_EQ(
Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.h
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.h
@@ -38,6 +38,9 @@
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
 
+  /// Ensure that the provided header guard is a non-reserved identifier.
+  std::string sanitizeHeaderGuard(StringRef Guard);
+
   /// Returns ``true`` if the check should suggest inserting a trailing comment
   /// on the ``#endif`` of the header guard. It will use the same name as
   /// returned by ``HeaderGuardCheck::getHeaderGuard``.
Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
@@ -164,10 +164,11 @@
  StringRef CurHeaderGuard,
  std::vector &FixIts) {
 std::string CPPVar = Check->getHeaderGuard(FileName, CurHeaderGuard);
+CPPVar = Check->sanitizeHeaderGuard(CPPVar);
 std::string CPPVarUnder = CPPVar + '_';
 
-// Allow a trailing underscore iff we don't have to change the endif 
comment
-// too.
+// Allow a trailing underscore if and only if we don't have to change the
+// endif comment too.
 if (Ifndef.isValid() && CurHeaderGuard != CPPVar &&
 (CurHeaderGuard != CPPVarUnder ||
  wouldFixEndifComment(FileName, EndIf, CurHeaderGuard))) {
@@ -216,6 +217,7 @@
 continue;
 
   std::string CPPVar = Check->getHeaderGuard(FileName);
+  CPPVar = Check->sanitizeHeaderGuard(CPPVar);
   std::string CPPVarUnder = CPPVar + '_'; // Allow a trailing underscore.
   // If there's a macro with a name that follows the header guard 
convention
   // but was not recognized by the preprocessor as a header guard there 
must
@@ -278,6 +280,11 @@
   PP->addPPCallbacks(std::make_unique(PP, this));
 }
 
+std::string HeaderGuardCheck::sanitizeHeaderGuard(StringRef Guard) {
+  // Only reserved identifiers are allowed to start with an '_'.
+  return Guard.drop_while([](char C) { return C == '_'; }).str();
+}
+
 bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef FileName) {
   return utils::isFileExtension(FileName, HeaderFileExtensions);
 }


Index: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -219,6 +219,16 @@
 "", "/llvm-project/clang-tools-extra/clangd/foo.h",
 StringRef("header is missing header guard")));
 
+  // Substitution of characters should not result in a header guard starting
+  // with "_".
+  EXPECT_EQ("#ifndef BAR_H\n"
+"#define BAR_H\n"
+"\n"
+"\n"
+"#endif\n",
+runHeaderGuardCheck("", "include/--bar.h",
+StringRef("header is missing header guard")));
+
 #ifdef WIN32
   // Check interaction with Windows-style path separators (\).
   EXPECT_EQ(
Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.h
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.h
@@ -38,6 +38,9 @@
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
 

[PATCH] D114724: [clangd][StdSymbolMap] Prefer std::remove from algorithm

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/StdSymbolMap.inc:958
 SYMBOL(remainder, std::, )
-SYMBOL(remove, std::, )
 SYMBOL(remove_all_extents, std::, )

Is the stdio version so uncommon we are willing to be wrong about it? For 
`move` we omit it from here and consider it case-by-case, I'd have imagined 
doing the same here 

(BTW at some point we may want to extend this to list alternative headers for 
ambiguous symbols)



Comment at: clang-tools-extra/clangd/include-mapping/cppreference_parser.py:138
   # FIXME: use these as a fallback rather than ignoring entirely.
-  if variant:
+  if variant != (namespace+symbol_name in variants_to_accept):
 continue

This logic seems a bit weird and non-general: it allows us to accept any 
variant and reject any non-variant, but it doesn't allow us to accept 
a*specific* variant (they are named) and doesn't allow us to accept both (which 
currently would lead to the symbol being dropped as ambiguous).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114724

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

[PATCH] D113118: [clang][AST] Check context of record in structural equivalence.

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/unittests/AST/StructuralEquivalenceTest.cpp:1037
+ "void foo(struct Param { int a; } *p);", Lang_C89);
+  EXPECT_TRUE(testStructuralMatch(Decls));
+}

I expect this to be `false` the outer `Param` is not the same type as the 
`Param` in the parameter.



Comment at: clang/unittests/AST/StructuralEquivalenceTest.cpp:1051
+  "Param");
+  EXPECT_TRUE(testStructuralMatch(Decls));
+}

I expect this to be `false` as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113118

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

[PATCH] D114095: [clang][lex] Include tracking: simplify and move to preprocessor

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

dexonsmith added a comment.

This LGTM by the way (not sure if that was already clear, since you abandoned a 
different review than I anticipated when squashing, and I'd "accepted" the 
other one); also see my suggestion to move the call to getFileInfo inside of 
markIncluded (might be error prone not to make that change).

Haven't clicked "accept" here in case @vsapsai has more comments.




Comment at: clang/lib/Lex/Preprocessor.cpp:552
+if (const FileEntry *FE = SourceMgr.getFileEntryForID(MainFileID)) {
+  HeaderInfo.getFileInfo(FE);
+  markIncluded(FE);

jansvoboda11 wrote:
> vsapsai wrote:
> > Why do you need to `getFileInfo` but don't use it? I have no objections but 
> > it looks like it deserves a comment because it's not obvious.
> Without the call, I'm hitting some assertions when running C++20 modules 
> tests:
> 
> ```
> Assertion failed: (CurDiagID == std::numeric_limits::max() && 
> "Multiple diagnostics in flight at once!"), function Report, file 
> Diagnostic.h, line 1526.
> ```
> 
> ```
> fatal error: error in backend: -verify directives found after rather than 
> during normal parsing of 
> /clang/test/CXX/modules-ts/basic/basic.def.odr/p6/global-vs-module.cpp
> ```
> 
> Might need to investigate more to be able to write up a reasonable comment 
> here.
Not sure precisely why that assertion fires, but the call to `getFileInfo` 
makes sense since it's mutating -- it adds a HeaderFileInfo entry (and 
`IncrementIncludeCount()` used to call it). There are code paths that call 
`getExistingFileInfo` that will expect it to have been created on inclusion.

I suggest moving the `getFileInfo()` call to inside `markIncluded()` and adding 
a comment that says "Create the HeaderFileInfo if it doesn't already exist" or 
something. Parting of marking it included should be to ensure that callers of 
HeaderSearch::getExistingFileInfo get something back.

(This'd be more obvious (and the comment unnecessary) if getFileInfo were 
renamed to getOrCreateFileInfo, after which getExistingFileInfo could be 
renamed to getFileInfo.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114095

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

[PATCH] D114326: Update the list of CUDA versions up to 11.5

[Artem Belevich via Phabricator via cfe-commits]

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

With D114601 , this patch would no longer be 
needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114326

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

[PATCH] D114601: Read path to CUDA from env. variable CUDA_PATH on Windows

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Driver/ToolChains/Cuda.cpp:137
   } else if (HostTriple.isOSWindows()) {
-for (const char *Ver : Versions)
-  Candidates.emplace_back(

mojca wrote:
> tra wrote:
> > Do we want to keep this as the fall-back for cases when `CUDA_PATH` is not 
> > set?
> > 
> > Otherwise, we're risking to break existing users.
> That was going to be my question as well (for which I wanted your input).
> From the initial comments in the other review (D114326) I understood that 
> eventually you wanted to get rid of the hardcoded list of versions (but I 
> didn't fully understand the intent).
> 
> (Technically speaking this won't break existing users since they were only 
> able to access CUDA versions up to 8.0 until now, and we would just remove 
> support for anything older that CUDA 10.0.)
Good point. Then it's a good opportunity to decommission the auto-search by 
version magic and replace it with something more predictable.

I think this patch would obviate the D114326, too, and we should be able to 
remove the multi-version search everywhere.


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

https://reviews.llvm.org/D114601

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

[PATCH] D114728: [Coroutine] Remove the prologue data of `-fsanitize=function` for split functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D114728#3159303 , @rjmccall wrote:

> I agree that coroutine resumption functions have a different formal type from 
> the ramp function and so would need different treatment from 
> `-fsanitize=functions` if it wants to sanitize the resumption calls, which I 
> guess it currently doesn't.  So something here may be a necessary fix.
>
> However, I don't think it's a sufficient fix for PR 50345, because the way 
> that the frontend currently creates these prologue attributes is deeply 
> problematic for any number of function transformations, not just coroutine 
> splitting.  For example, any sort of function-cloning transformation will end 
> up constructing an incorrect relative reference.  I expect that this 
> self-reference will also interfere with DCE.  So in addition to whatever 
> function-type fix we need for coroutines, we just need to change how we 
> create this prologue.  I recommend the design I laid out in the PR:
>
> - Have the frontend emit a more abstract attribute, like 
> `sanitize_function_type(i8** @1)`
> - Either lower this abstract attribute in a codegen pass by turning it into a 
> `prologue` attribute or just handle it directly in the appropriate backend.
>
> The coroutine part of the fix would then simply be to remove the 
> `sanitize_function_type` attribute from the resumption function clones; or 
> better yet, switch the coro.switch lowering to use the "prototype" design 
> used by coro.retcon and coro.async, and then set the appropriate attribute 
> (if any) on the prototype so that it will be cloned into the resumption 
> functions.

@rjmccall Yeah agreed that the attribute method is a better approach forward (I 
would work on a patch). I was intended to propose this as an alternative to 
disabling `-fsanitize=function` for 13.x.

> In the meantime, this sanitizer should be disabled in 13.x.

@pcc does this sound good to you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114728

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

[PATCH] D114326: Update the list of CUDA versions up to 11.5

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D114326#3159122 , @mojca wrote:

> @tra: this is not yet 100% ready since the unit tests are now failing 
> (expecting to find CUDA 8.0).
> I can fix the unit test, but I suppose that someone needs to install 
> additional SDK somewhere into the infrastructure as well?

Clang tests run w/o CUDA SDK itself. We have some mock installations under 
`Inputs` in the test directory. You may need to adjust both the inputs and the 
tests to work with your changes.

> In D114326#3158913 , @tra wrote:
>
>> So, yes, you could automatically add linking flags in your example, but it's 
>> not feasible to do so for the linking phase in general. It would also 
>> introduce inconsistency in clang's linking phase behavior and would lead to 
>> questions -- why things work with `clang hello.co`, but not with `clang -c 
>> hello.cu; clang hello.o` ?
>
> Please note that I'm a complete newbie to compilers.
> The flag `--cuda-path=` slightly reminds me of the `--framework` keyword on 
> macOS. I'm not behind a mac right now, so please forgive me any typo or wrong 
> paths (it's also not exact since the frameworks are searched for in different 
> folders etc.), below is just a general idea.

Mac is... special in more than one way. "We do it on Mac" alone is not a very 
strong argument for clang's behavior everywhere else.

My general rule of thumb is that a flag should serve specific purpose and 
should not duplicate the work of existing flags. 
The way I see it is that CUDA libraries are not any different than any other 
libraries an app must be linked with and we already have `-L` to tell the 
linker where to find them.

> What would be cool to me was if `--cuda-path` was also implicitly adding the 
> `-L` flag, so that the following would work:
>
>   clang++ --cuda-path=/usr/local/cuda-11.5 -l cudart hello.cu
>   clang++ --cuda-path=/usr/local/cuda-11.5 -l cudart hello.o
>
> I don't know whether it would be acceptable to add (the default) cuda library 
> path to the linker though.

TBH, I don't see much of a benefit over `clang++ -L/usr/local/cuda-11.5/lib64 
-l cudart hello.o`, and I do see a few downsides in form of implicit `-L` 
interfering with explicit `-L`. E.g. will a user need to consider where on the 
command line they place `--cuda-path` relative to `-L`/`-l` now? Where (or 
whether?) will the linker path be added if clang is run w/o `--cuda-path`? How 
many users will be surprised by magically materialized linking paths (that they 
were not aware of) interfering with explicitly specified `-L`? E.g. user runs 
`clang -L/non/default/cuda/lib64` and the implicitly added `-L` picks the 
library from the auto-found CUDA location, instead of the one specified by the 
user. Etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114326

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

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre added a comment.

Wow, thanks for the quick response! I really appreciate it.

In D114732#3159247 , @Quuxplusone 
wrote:

> - (Major point of contention) To me, `[[trivial_abi]]` does not imply 
> `[[trivially_relocatable]]` at all. I believe @rjmccall disagreed a few years 
> ago: basically I said "The two attributes are orthogonal," with practical 
> demonstration, and basically he said "yes, in practice you're right, but 
> philosophically that's a bug and we do actually //intend// `[[trivial_abi]]` 
> to imply `[[trivially_relocatable]]` even though the compiler doesn't enforce 
> it" (but then AFAIK nothing was ever done about that).

I agree with you that `[[clang::trivial_abi]]` could be taken not to imply 
`[[trivially_relocatable]]`. However, I also think we can choose for it to 
imply it -- the meaning of `[[clang::trivial_abi]]` is up to us! It does not 
seem very **useful** to have a type which is `trivial_abi`, but not trivially 
relocatable: such a type can be relocated when being passed by value, but is 
leaving performance on the floor everywhere else. It might be that such a type 
really depends on the number of paired constructor/destructor calls, but this 
seems unlikely to be a realistic concern, especially since the number of paired 
constructor/destructor calls can change as a result of implementation changes 
in e.g. `std::vector`'s reallocation algorithm anyway.

As you noted, "nothing was ever done about that". This change seeks to address 
that problem. :B

> Therefore, I support something close to this patch, but I strongly believe 
> that you need to split up the "trivially relocatable" attribute itself from 
> "trivial abi". They are represented orthogonally in the AST, and they should 
> be toggleable orthogonally by the end-user. The end-user might want a 
> trivial-abi type that is not trivially-relocatable, or a 
> trivially-relocatable type that is not trivial-abi.

Leaving aside differences of opinion on how important it is, a version of the 
standards proposal could maintain linear independence, though not 
orthogonality: we could imagine writing `struct [[clang::trivial_abi]] 
[[trivially_relocatable(false]] Foo { ... };` and the like to achieve every 
combination.

I do agree that it's not true that every trivially relocatable type should be 
trivial for calls. This can produce bad performance outcomes in the case of 
e.g. non-inline functions which accept the trivial-for-calls value by 
pointer/reference, such as methods. Calling those can force the value to be put 
back in the stack, adding additional (trivial) copies where a non-trivial type 
would've had none. And so, even with this change, we would still want to be 
able to specify trivial relocatability separately, without implying 
trivial-for-calls.

A bigger problem is that my perception from reading that review thread was that 
any `[[trivially_relocatable]]` attribute proposal was a non-starter: this is 
the subject of a standards proposal which has not yet been accepted and clang 
doesn't want to "pick a winner". So my hope is that rather than picking a 
winner there, we extend the behavior of `trivial_abi`, in a way that is 
compatible with and desirable to have in combination with any accepted 
standards proposal. Hopefully, by being a bit less ambitious and complete, and 
reducing scope only to changes in behavior for `trivial_abi`, the resulting 
change to clang will be more acceptable.

This does mean that some performance is left on the floor -- any types which 
can't be marked `trivial_abi` will not get the benefit of trivial 
relocatability -- but that's what the standards proposal can aim to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114732

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

[PATCH] D114718: [analyzer] Implement a new checker for Strict Aliasing Rule.

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StrictAliasingChecker.cpp:58
+  : From(From), To(To), Ctx(Ctx) {}
+  bool canAccessImpl() {
+return isSame() || isCharOrByte() || isOppositeSign();

I'd love to see some detailed descriptions of the strict aliasing rule, what 
parts are we checking for and what parts we don't. E.g. it would be nice to 
document the differences between C and C++ aliasing rules. I do remember 
something about prefixes, i.e.: it would be well defined to access something 
with the dynamic type `struct { int x, y; };` and read `struct{ int x; };` from 
it. Does that not apply to C++?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114718

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

[PATCH] D114718: [analyzer] Implement a new checker for Strict Aliasing Rule.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Awesome, I love it!

What would it take to enable this by default? Note that we can enable the 
checker without emitting any warnings; sinking exotic execution paths (on which 
strict aliasing is known to be violated) is valuable on its own.

Other than that, we'll probably need a bug visitor to add a note for where the 
original type comes from (it should probably be the place where the location 
becomes *live*), and then some real-world testing.




Comment at: clang/lib/StaticAnalyzer/Checkers/StrictAliasingChecker.cpp:48-49
+  //
+  // NOTE: User must provide canonical and unqualified QualType's for the
+  // correct result.
+  static bool canAccess(QualType From, QualType To, ASTContext &Ctx) {

`assert()` it? Or maybe canonicalize defensively so that not to duplicate code 
in the caller, given that there's really only one correct way to do that?



Comment at: clang/lib/StaticAnalyzer/Checkers/StrictAliasingChecker.cpp:80
+class StrictAliasingChecker : public Checker {
+  mutable std::unique_ptr BT;
+

The modern solution is
```lang=c++
BugType BT{this, "...", "..."};
```



Comment at: clang/lib/StaticAnalyzer/Checkers/StrictAliasingChecker.cpp:113
+  QualType getOriginalType(CheckerContext &C, SVal V, QualType T) const {
+assert(V.getAs() && "Location shall be a Loc.");
+V = C.getState()->getSVal(V.castAs(), T);

I suspect it might also be `UnknownVal` (?) It usually makes sense to protect 
against such scenarios with an early return.



Comment at: clang/lib/StaticAnalyzer/Checkers/StrictAliasingChecker.cpp:141-145
+auto ER = dyn_cast(R);
+while (ER) {
+  R = ER->getSuperRegion();
+  ER = dyn_cast(R);
+}

You're basically running into https://bugs.llvm.org/show_bug.cgi?id=43364 here. 
The element region is not necessarily a consequence of a pointer cast; it may 
also be a legitimate array element access, and there's no way to figure out 
what it really was. So I suspect that you may fail the following test case:
```lang=c++
void foo() {
  int x[10];
  int *p = &x[1];
  *p = 1; // int incompatible with int[10]
}
```

Separately from that, ultimatetly you'll most likely have to handle //regions 
with symbolic base// separately. These regions are special because they are 
built when the information about the original type is not really unavailable. 
For now you're dodging this problem by only supporting `VarRegion`s with 
element sub-regions. But in the general case you may encounter code like this
```lang=c++
void foo(void *p) {
  int *x = p;
  float *y = x;
  *y = 1.0;
}
```
which may be valid if the original pointer `p` points to a `float`. But this 
can be delayed until later.



Comment at: clang/lib/StaticAnalyzer/Checkers/StrictAliasingChecker.cpp:157
+
+ExplodedNode *Node = C.generateNonFatalErrorNode();
+if (!BT)

I suggest a fatal error node. Undefined behavior already occured, there's 
nothing interesting in the follow-up.



Comment at: clang/lib/StaticAnalyzer/Checkers/StrictAliasingChecker.cpp:178-179
+  const LangOptions &LO = CM.getLangOpts();
+  // Ideally, the condition should be `LO.CPlusPlus11 || LO.CPlusPlus2b` but
+  // implemented checks can be partially applied for C++17 and lower versions.
+  return LO.CPlusPlus;

We should probably keep the checker disabled in these language modes until 
we're sure it works correctly (in a conservative sense, i.e. doesn't emit false 
positives).

Can we check whether `-fstrict-aliasing` is enabled here? That's probably 
appropriate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114718

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

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

[Vaibhav Yenamandra via Phabricator via cfe-commits]

vaibhav.y updated this revision to Diff 390448.
vaibhav.y added a comment.

Rename enum members


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109701

Files:
  clang/include/clang/Basic/Sarif.h
  clang/include/clang/Basic/SourceLocation.h
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/Sarif.cpp
  clang/lib/Basic/SourceLocation.cpp
  clang/unittests/Basic/CMakeLists.txt
  clang/unittests/Basic/SarifTest.cpp

Index: clang/unittests/Basic/SarifTest.cpp
===
--- /dev/null
+++ clang/unittests/Basic/SarifTest.cpp
@@ -0,0 +1,138 @@
+//===- unittests/Basic/SarifTest.cpp - Test writing SARIF documents ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Basic/Sarif.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/JSON.h"
+#include 
+
+#include "gmock/gmock.h"
+#include "gtest/gtest-death-test.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace llvm;
+
+namespace {
+
+TEST(SarifDocumentWriterTest, createEmptyDocument) {
+  // GIVEN:
+  SarifDocumentWriter writer;
+
+  // WHEN:
+  const json::Object &emptyDocument = writer.createDocument();
+  std::vector keys(emptyDocument.size());
+  std::transform(emptyDocument.begin(), emptyDocument.end(), keys.begin(),
+ [](auto item) { return item.getFirst(); });
+
+  // THEN:
+  ASSERT_THAT(keys, testing::UnorderedElementsAre("$schema", "version"));
+}
+
+// Test that a newly inserted run will associate correct tool names
+TEST(SarifDocumentWriterTest, documentWithARun) {
+  // GIVEN:
+  SarifDocumentWriter writer;
+  const char *shortName = "sariftest";
+  const char *longName = "sarif writer test";
+
+  // WHEN:
+  writer.createRun(shortName, longName);
+  writer.endRun();
+  const json::Object &document = writer.createDocument();
+  const json::Array *runs = document.getArray("runs");
+
+  // THEN:
+  // A run was created
+  ASSERT_THAT(runs, testing::NotNull());
+
+  // It is the only run
+  ASSERT_EQ(runs->size(), 1UL);
+
+  // The tool associated with the run was the tool
+  const json::Object *driver =
+  runs->begin()->getAsObject()->getObject("tool")->getObject("driver");
+  ASSERT_THAT(driver, testing::NotNull());
+
+  ASSERT_TRUE(driver->getString("name").hasValue());
+  ASSERT_TRUE(driver->getString("fullName").hasValue());
+  ASSERT_TRUE(driver->getString("language").hasValue());
+
+  EXPECT_EQ(driver->getString("name").getValue(), shortName);
+  EXPECT_EQ(driver->getString("fullName").getValue(), longName);
+  EXPECT_EQ(driver->getString("language").getValue(), "en-US");
+}
+
+// Test adding result without a run causes a crash
+TEST(SarifDocumentWriterTest, addingResultsWillCrashIfThereIsNoRun) {
+  // GIVEN:
+  SarifDocumentWriter writer;
+
+  // WHEN:
+  //  A SarifDocumentWriter::createRun(...) was not called prior to
+  //  SarifDocumentWriter::appendResult(...)
+  // But a rule exists
+  auto ruleIdx = writer.createRule(SarifRule::create());
+  SarifResult &&emptyResult = SarifResult::create(ruleIdx);
+
+  // THEN:
+  ASSERT_DEATH({ writer.appendResult(emptyResult); }, ".*create a run first.*");
+}
+
+// Test adding rule and result shows up in the final document
+TEST(SarifDocumentWriterTest, addResultWithValidRuleIsOk) {
+  // GIVEN:
+  SarifDocumentWriter writer;
+  const SarifRule &rule =
+  SarifRule::create()
+  .setRuleId("clang.unittest")
+  .setDescription("Example rule created during unit tests")
+  .setName("clang unit test");
+
+  // WHEN:
+  writer.createRun("sarif test", "sarif test runner");
+  unsigned ruleIdx = writer.createRule(rule);
+  const SarifResult &result = SarifResult::create(ruleIdx);
+
+  writer.appendResult(result);
+  const json::Object &document = writer.createDocument();
+
+  // THEN:
+  // A document with a valid schema and version exists
+  ASSERT_THAT(document.get("$schema"), ::testing::NotNull());
+  ASSERT_THAT(document.get("version"), ::testing::NotNull());
+  const json::Array *runs = document.getArray("runs");
+
+  // A run exists on this document
+  ASSERT_THAT(runs, ::testing::NotNull());
+  ASSERT_EQ(runs->size(), 1UL);
+  const json::Object *theRun = runs->back().getAsObject();
+
+  // The run has slots for tools, results, rules and artifacts
+  ASSERT_THAT(theRun->get("tool"), ::testing::NotNull());
+  ASSERT_THAT(theRun->get("results"), ::testing::NotNull());
+  ASSERT_THAT(theRun->get("artifacts"), ::testing::NotNull());
+  const json::Object *driver = theRun->getObject("tool")->getObject("driver");
+  const json::Array *results = theRun->getArray("results");
+  const json::Array *artifacts = theRun->getArray("artif

[PATCH] D114728: [Coroutine] Remove the prologue data of `-fsanitize=function` for split functions

[John McCall via Phabricator via cfe-commits]

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

I agree that coroutine resumption functions have a different formal type from 
the ramp function and so would need different treatment from 
`-fsanitize=functions` if it wants to sanitize the resumption calls, which I 
guess it currently doesn't.  So something here may be a necessary fix.

However, I don't think it's a sufficient fix for PR 50345, because the way that 
the frontend currently creates these prologue attributes is deeply problematic 
for any number of function transformations, not just coroutine splitting.  For 
example, any sort of function-cloning transformation will end up constructing 
an incorrect relative reference.  I expect that this self-reference will also 
interfere with DCE.  So in addition to whatever function-type fix we need for 
coroutines, we just need to change how we create this prologue.  I recommend 
the design I laid out in the PR:

- Have the frontend emit a more abstract attribute, like 
`sanitize_function_type(i8** @1)`
- Either lower this abstract attribute in a codegen pass by turning it into a 
`prologue` attribute or just handle it directly in the appropriate backend.

The coroutine part of the fix would then simply be to remove the 
`sanitize_function_type` attribute from the resumption function clones; or 
better yet, switch the coro.switch lowering to use the "prototype" design used 
by coro.retcon and coro.async, and then set the appropriate attribute (if any) 
on the prototype so that it will be cloned into the resumption functions.

In the meantime, this sanitizer should be disabled in 13.x.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114728

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

[clang] 25eb7fa - Revert "OpenMP: Start calling setTargetAttributes for generated kernels"

[Matt Arsenault via cfe-commits]

Author: Matt Arsenault
Date: 2021-11-29T15:47:10-05:00
New Revision: 25eb7fa01d7ebbe67648ea03841cda55b4239ab2

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

LOG: Revert "OpenMP: Start calling setTargetAttributes for generated kernels"

This reverts commit 6c27d389c8a00040aad998fe959f38ba709a8750.

This is failing on the buildbots

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/TargetInfo.cpp

Removed: 
clang/test/OpenMP/amdgcn-attributes.cpp



diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index c3a01448389b3..75709b3c7e782 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -15,7 +15,6 @@
 #include "CGCleanup.h"
 #include "CGRecordLayout.h"
 #include "CodeGenFunction.h"
-#include "TargetInfo.h"
 #include "clang/AST/APValue.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
@@ -6621,8 +6620,6 @@ void CGOpenMPRuntime::emitTargetOutlinedFunctionHelper(
 OutlinedFn->addFnAttr("omp_target_thread_limit",
   std::to_string(DefaultValThreads));
   }
-
-  CGM.getTargetCodeGenInfo().setTargetAttributes(nullptr, OutlinedFn, CGM);
 }
 
 /// Checks if the expression is constant or does not have non-trivial function

diff  --git a/clang/lib/CodeGen/TargetInfo.cpp 
b/clang/lib/CodeGen/TargetInfo.cpp
index e94436d2e72ae..4360269f8af19 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -9143,10 +9143,6 @@ class AMDGPUTargetCodeGenInfo : public TargetCodeGenInfo 
{
 public:
   AMDGPUTargetCodeGenInfo(CodeGenTypes &CGT)
   : TargetCodeGenInfo(std::make_unique(CGT)) {}
-
-  void setFunctionDeclAttributes(const FunctionDecl *FD, llvm::Function *F,
- CodeGenModule &CGM) const;
-
   void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &M) const override;
   unsigned getOpenCLKernelCallingConv() const override;
@@ -9186,13 +9182,36 @@ static bool requiresAMDGPUProtectedVisibility(const 
Decl *D,
cast(D)->getType()->isCUDADeviceBuiltinTextureType()));
 }
 
-void AMDGPUTargetCodeGenInfo::setFunctionDeclAttributes(
-const FunctionDecl *FD, llvm::Function *F, CodeGenModule &M) const {
-  const auto *ReqdWGS =
-  M.getLangOpts().OpenCL ? FD->getAttr() : nullptr;
-  const bool IsOpenCLKernel =
-  M.getLangOpts().OpenCL && FD->hasAttr();
-  const bool IsHIPKernel = M.getLangOpts().HIP && 
FD->hasAttr();
+void AMDGPUTargetCodeGenInfo::setTargetAttributes(
+const Decl *D, llvm::GlobalValue *GV, CodeGen::CodeGenModule &M) const {
+  if (requiresAMDGPUProtectedVisibility(D, GV)) {
+GV->setVisibility(llvm::GlobalValue::ProtectedVisibility);
+GV->setDSOLocal(true);
+  }
+
+  if (GV->isDeclaration())
+return;
+  const FunctionDecl *FD = dyn_cast_or_null(D);
+  if (!FD)
+return;
+
+  llvm::Function *F = cast(GV);
+
+  const auto *ReqdWGS = M.getLangOpts().OpenCL ?
+FD->getAttr() : nullptr;
+
+
+  const bool IsOpenCLKernel = M.getLangOpts().OpenCL &&
+  FD->hasAttr();
+  const bool IsHIPKernel = M.getLangOpts().HIP &&
+   FD->hasAttr();
+  if ((IsOpenCLKernel || IsHIPKernel) &&
+  (M.getTriple().getOS() == llvm::Triple::AMDHSA))
+F->addFnAttr("amdgpu-implicitarg-num-bytes", "56");
+
+  if (IsHIPKernel)
+F->addFnAttr("uniform-work-group-size", "true");
+
 
   const auto *FlatWGS = FD->getAttr();
   if (ReqdWGS || FlatWGS) {
@@ -9260,38 +9279,6 @@ void AMDGPUTargetCodeGenInfo::setFunctionDeclAttributes(
 if (NumVGPR != 0)
   F->addFnAttr("amdgpu-num-vgpr", llvm::utostr(NumVGPR));
   }
-}
-
-void AMDGPUTargetCodeGenInfo::setTargetAttributes(
-const Decl *D, llvm::GlobalValue *GV, CodeGen::CodeGenModule &M) const {
-  if (requiresAMDGPUProtectedVisibility(D, GV)) {
-GV->setVisibility(llvm::GlobalValue::ProtectedVisibility);
-GV->setDSOLocal(true);
-  }
-
-  if (GV->isDeclaration())
-return;
-
-  llvm::Function *F = dyn_cast(GV);
-  if (!F)
-return;
-
-  const FunctionDecl *FD = dyn_cast_or_null(D);
-  if (FD)
-setFunctionDeclAttributes(FD, F, M);
-
-  const bool IsOpenCLKernel =
-  M.getLangOpts().OpenCL && FD && FD->hasAttr();
-  const bool IsHIPKernel =
-  M.getLangOpts().HIP && FD && FD->hasAttr();
-
-  const bool IsOpenMP = M.getLangOpts().OpenMP && !FD;
-  if ((IsOpenCLKernel || IsHIPKernel || IsOpenMP) &&
-  (M.getTriple().getOS() == llvm::Triple::AMDHSA))
-F->addFnAttr("amdgpu-implicitarg-num-bytes", "56");
-
-  if (IsHIPKernel)
-F->addFnAttr("uniform-work-group-size", "true");
 
   if (M.getContext().getTargetInfo().allowAMDGPUUnsafeFPAtomics())
 F->a

[PATCH] D114725: [clang-format] add back tests which didn't cause a regression

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 390445.
MyDeveloperDay added a comment.

Remove unrelated file


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

https://reviews.llvm.org/D114725

Files:
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14122,9 +14122,8 @@
   verifyFormat("static_assert (sizeof (char) == 1, \"Impossible!\");", Space);
   verifyFormat("int f () throw (Deprecated);", Space);
   verifyFormat("typedef void (*cb) (int);", Space);
-  // FIXME these tests regressed behaviour.
-  // verifyFormat("T A::operator() ();", Space);
-  // verifyFormat("X A::operator++ (T);", Space);
+  verifyFormat("T A::operator() ();", Space);
+  verifyFormat("X A::operator++ (T);", Space);
   verifyFormat("auto lambda = [] () { return 0; };", Space);
   verifyFormat("int x = int (y);", Space);
 
@@ -14180,8 +14179,7 @@
   verifyFormat("int f() throw (Deprecated);", SomeSpace);
   verifyFormat("typedef void (*cb) (int);", SomeSpace);
   verifyFormat("T A::operator()();", SomeSpace);
-  // FIXME these tests regressed behaviour.
-  // verifyFormat("X A::operator++ (T);", SomeSpace);
+  verifyFormat("X A::operator++ (T);", SomeSpace);
   verifyFormat("int x = int (y);", SomeSpace);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 
@@ -14274,6 +14272,7 @@
   verifyFormat("typedef void (*cb)(int);", SpaceFuncDef);
   verifyFormat("T A::operator()();", SpaceFuncDef);
   verifyFormat("X A::operator++(T);", SpaceFuncDef);
+  // FIXME these tests regressed behaviour.
   // verifyFormat("T A::operator() () {}", SpaceFuncDef);
   verifyFormat("auto lambda = [] () { return 0; };", SpaceFuncDef);
   verifyFormat("int x = int(y);", SpaceFuncDef);
@@ -14349,7 +14348,7 @@
   verifyFormat("int f() throw (Deprecated);", SomeSpace2);
   verifyFormat("typedef void (*cb) (int);", SomeSpace2);
   verifyFormat("T A::operator()();", SomeSpace2);
-  // verifyFormat("X A::operator++ (T);", SomeSpace2);
+  verifyFormat("X A::operator++ (T);", SomeSpace2);
   verifyFormat("int x = int (y);", SomeSpace2);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace2);
 }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14122,9 +14122,8 @@
   verifyFormat("static_assert (sizeof (char) == 1, \"Impossible!\");", Space);
   verifyFormat("int f () throw (Deprecated);", Space);
   verifyFormat("typedef void (*cb) (int);", Space);
-  // FIXME these tests regressed behaviour.
-  // verifyFormat("T A::operator() ();", Space);
-  // verifyFormat("X A::operator++ (T);", Space);
+  verifyFormat("T A::operator() ();", Space);
+  verifyFormat("X A::operator++ (T);", Space);
   verifyFormat("auto lambda = [] () { return 0; };", Space);
   verifyFormat("int x = int (y);", Space);
 
@@ -14180,8 +14179,7 @@
   verifyFormat("int f() throw (Deprecated);", SomeSpace);
   verifyFormat("typedef void (*cb) (int);", SomeSpace);
   verifyFormat("T A::operator()();", SomeSpace);
-  // FIXME these tests regressed behaviour.
-  // verifyFormat("X A::operator++ (T);", SomeSpace);
+  verifyFormat("X A::operator++ (T);", SomeSpace);
   verifyFormat("int x = int (y);", SomeSpace);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 
@@ -14274,6 +14272,7 @@
   verifyFormat("typedef void (*cb)(int);", SpaceFuncDef);
   verifyFormat("T A::operator()();", SpaceFuncDef);
   verifyFormat("X A::operator++(T);", SpaceFuncDef);
+  // FIXME these tests regressed behaviour.
   // verifyFormat("T A::operator() () {}", SpaceFuncDef);
   verifyFormat("auto lambda = [] () { return 0; };", SpaceFuncDef);
   verifyFormat("int x = int(y);", SpaceFuncDef);
@@ -14349,7 +14348,7 @@
   verifyFormat("int f() throw (Deprecated);", SomeSpace2);
   verifyFormat("typedef void (*cb) (int);", SomeSpace2);
   verifyFormat("T A::operator()();", SomeSpace2);
-  // verifyFormat("X A::operator++ (T);", SomeSpace2);
+  verifyFormat("X A::operator++ (T);", SomeSpace2);
   verifyFormat("int x = int (y);", SomeSpace2);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace2);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114427: [clang-tidy] Warn on functional C-style casts

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

LGTM!




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp:318
   // FIXME: This should be a static_cast.
   // C HECK-FIXES: auto s5 = static_cast(cr);
   auto s6 = (S)cr;

salman-javed-nz wrote:
> Isn't this intentional? Isn't it saying what the CHECK-MESSAGES should look 
> like in the future once the FIXME is done?
Ah, I guess that must be it.
(`C HECK-FIXES: ...` is a really subtle idiom for `FIXME the line above should 
become: ...`, but OK!)


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

https://reviews.llvm.org/D114427

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

[PATCH] D114725: [clang-format] add back tests which didn't cause a regression

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 390443.
MyDeveloperDay marked 3 inline comments as done.
MyDeveloperDay added a comment.
Herald added subscribers: sdasgup3, wenzhicui, wrengr, Chia-hungDuan, dcaballe, 
cota, mravishankar, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, 
Kayjukh, grosul1, Joonsoo, stephenneuendorffer, liufengdb, aartbik, lucyrfox, 
mgester, arpith-jacob, nicolasvasilache, antiagainst, shauheen, rriddle, 
mehdi_amini.
Herald added a reviewer: nicolasvasilache.
Herald added a project: MLIR.

Address review comments


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

https://reviews.llvm.org/D114725

Files:
  clang/unittests/Format/FormatTest.cpp
  mlir/lib/Dialect/Linalg/ComprehensiveBufferize/ComprehensiveBufferize.cpp


Index: mlir/lib/Dialect/Linalg/ComprehensiveBufferize/ComprehensiveBufferize.cpp
===
--- mlir/lib/Dialect/Linalg/ComprehensiveBufferize/ComprehensiveBufferize.cpp
+++ mlir/lib/Dialect/Linalg/ComprehensiveBufferize/ComprehensiveBufferize.cpp
@@ -785,4 +785,3 @@
 // callbacks to their default functions.
 BufferizationOptions::BufferizationOptions()
 : allocationFns(defaultAllocationCallbacks()) {}
-
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14122,9 +14122,8 @@
   verifyFormat("static_assert (sizeof (char) == 1, \"Impossible!\");", Space);
   verifyFormat("int f () throw (Deprecated);", Space);
   verifyFormat("typedef void (*cb) (int);", Space);
-  // FIXME these tests regressed behaviour.
-  // verifyFormat("T A::operator() ();", Space);
-  // verifyFormat("X A::operator++ (T);", Space);
+  verifyFormat("T A::operator() ();", Space);
+  verifyFormat("X A::operator++ (T);", Space);
   verifyFormat("auto lambda = [] () { return 0; };", Space);
   verifyFormat("int x = int (y);", Space);
 
@@ -14180,8 +14179,7 @@
   verifyFormat("int f() throw (Deprecated);", SomeSpace);
   verifyFormat("typedef void (*cb) (int);", SomeSpace);
   verifyFormat("T A::operator()();", SomeSpace);
-  // FIXME these tests regressed behaviour.
-  // verifyFormat("X A::operator++ (T);", SomeSpace);
+  verifyFormat("X A::operator++ (T);", SomeSpace);
   verifyFormat("int x = int (y);", SomeSpace);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 
@@ -14274,6 +14272,7 @@
   verifyFormat("typedef void (*cb)(int);", SpaceFuncDef);
   verifyFormat("T A::operator()();", SpaceFuncDef);
   verifyFormat("X A::operator++(T);", SpaceFuncDef);
+  // FIXME these tests regressed behaviour.
   // verifyFormat("T A::operator() () {}", SpaceFuncDef);
   verifyFormat("auto lambda = [] () { return 0; };", SpaceFuncDef);
   verifyFormat("int x = int(y);", SpaceFuncDef);
@@ -14349,7 +14348,7 @@
   verifyFormat("int f() throw (Deprecated);", SomeSpace2);
   verifyFormat("typedef void (*cb) (int);", SomeSpace2);
   verifyFormat("T A::operator()();", SomeSpace2);
-  // verifyFormat("X A::operator++ (T);", SomeSpace2);
+  verifyFormat("X A::operator++ (T);", SomeSpace2);
   verifyFormat("int x = int (y);", SomeSpace2);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace2);
 }


Index: mlir/lib/Dialect/Linalg/ComprehensiveBufferize/ComprehensiveBufferize.cpp
===
--- mlir/lib/Dialect/Linalg/ComprehensiveBufferize/ComprehensiveBufferize.cpp
+++ mlir/lib/Dialect/Linalg/ComprehensiveBufferize/ComprehensiveBufferize.cpp
@@ -785,4 +785,3 @@
 // callbacks to their default functions.
 BufferizationOptions::BufferizationOptions()
 : allocationFns(defaultAllocationCallbacks()) {}
-
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14122,9 +14122,8 @@
   verifyFormat("static_assert (sizeof (char) == 1, \"Impossible!\");", Space);
   verifyFormat("int f () throw (Deprecated);", Space);
   verifyFormat("typedef void (*cb) (int);", Space);
-  // FIXME these tests regressed behaviour.
-  // verifyFormat("T A::operator() ();", Space);
-  // verifyFormat("X A::operator++ (T);", Space);
+  verifyFormat("T A::operator() ();", Space);
+  verifyFormat("X A::operator++ (T);", Space);
   verifyFormat("auto lambda = [] () { return 0; };", Space);
   verifyFormat("int x = int (y);", Space);
 
@@ -14180,8 +14179,7 @@
   verifyFormat("int f() throw (Deprecated);", SomeSpace);
   verifyFormat("typedef void (*cb) (int);", SomeSpace);
   verifyFormat("T A::operator()();", SomeSpace);
-  // FIXME these tests regressed behaviour.
-  // verifyFormat("X A::operator++ (T);", SomeSpace);
+  verifyFormat("X A::operator++ (T);", SomeSpace);
   verifyFormat("int x = int (y);", SomeSpace);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 
@@ -14274,6 

[PATCH] D114427: [clang-tidy] Warn on functional C-style casts

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz added a comment.

Nothing else that comes to mind that I want to see.
@Quuxplusone are you OK with the latest round of diffs?




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp:318
   // FIXME: This should be a static_cast.
   // C HECK-FIXES: auto s5 = static_cast(cr);
   auto s6 = (S)cr;

Isn't this intentional? Isn't it saying what the CHECK-MESSAGES should look 
like in the future once the FIXME is done?


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

https://reviews.llvm.org/D114427

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

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

On the one hand, I like that someone else is pushing a patch that's basically 
the same as D50119 , because maybe if enough 
people reimplement the same thing, eventually Clang will accept one of them. ;)
However, I'd like to point out:

- D50119  has more extensive tests, and has 
been live on https://p1144.godbolt.org/z/axx9Wj3r5 for a couple years now; if 
the consensus is that Clang wants "part of D50119 
 but not all of it," then IMHO it would be 
reasonable to put some comments on D50119  
with the goal of stripping it //down//, rather than trying to build //up// a 
smaller version of it in parallel.
- (Major point of contention) To me, `[[trivial_abi]]` does not imply 
`[[trivially_relocatable]]` at all. I believe @rjmccall disagreed a few years 
ago: basically I said "The two attributes are orthogonal," with practical 
demonstration, and basically he said "yes, in practice you're right, but 
philosophically that's a bug and we do actually //intend// `[[trivial_abi]]` to 
imply `[[trivially_relocatable]]` even though the compiler doesn't enforce it" 
(but then AFAIK nothing was ever done about that).

Here are my practical examples: https://p1144.godbolt.org/z/EYcMM1nTW
With `ATTR=[[clang::trivial_abi]]`, we see that `take(g)` passes in a register; 
but notice that it still calls the copy-ctor, and it will still call the dtor 
(inside `take` — the ABI changes to "callee-destroy" for arguments of these 
types). And we see that `std::swap` is completely unaffected: we know that the 
calling convention for passing `T` by value is altered, but we don't know 
anything //semantic// about the behavior of `T` itself (e.g. when it is 
relocated or swapped).
With `ATTR=[[clang::trivially_relocatable]]` (D50119 
), we see that `take(g)` still passes on the 
stack (p1144 trivial relocation is backward-compatible with C++20 by design: it 
doesn't alter the calling convention at all). But notice that because we know 
something //semantic// about the behavior of `T` (namely, that it is trivially 
relocatable), `std::swap` is able to skip calling its ctors and dtors and just 
swap the bits directly.

Therefore, I support something close to this patch, but I strongly believe that 
you need to split up the "trivially relocatable" attribute itself from "trivial 
abi". They are represented orthogonally in the AST, and they should be 
toggleable orthogonally by the end-user. The end-user might want a trivial-abi 
type that is not trivially-relocatable, or a trivially-relocatable type that is 
not trivial-abi.

Related reading: https://quuxplusone.github.io/blog/2018/05/02/trivial-abi-101/
Related watching, on p1144 `[[trivially_relocatable]]` and adjacent topics such 
as `trivial_abi` and `move_relocates`: 
https://www.youtube.com/watch?v=SGdfPextuAU


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114732

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

[PATCH] D114149: [clang-tidy] Fix pr48613: "llvm-header-guard uses a reserved identifier"

[Aaron Ballman via Phabricator via cfe-commits]

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

In D114149#3144614 , @salman-javed-nz 
wrote:

> I reverted my changes to do with the invalid character substitution. Doing 
> something akin to `isAllowedInitiallyIDChar()` and `isAllowedIDChar()` in 
> Lexer.cpp will require converting from `char*` to `UTF32*`. Windows 
> complicates things by using its own code page (typically Windows-1252). Will 
> require a lot of careful consideration to implement correctly.
>
> I have reverted this part of the patch for now, as it is not mentioned by the 
> PR.

I think reverting the changes was a good idea -- we can handle that separately 
as it is more complex and this solves a real problem. However, I'm not certain 
that Windows complicates anything here as Clang accepts UTF-8 input. We have 
`-finput-charset` but that only supports UTF-8 currently.

LGTM, modulo a commenting nit.




Comment at: clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp:169
 
-// Allow a trailing underscore iff we don't have to change the endif 
comment
 // too.

This wasn't actually a typo -- I think it meant "if and only if" 
(https://en.wikipedia.org/wiki/If_and_only_if). Feel free to expand the comment 
instead of using `iff`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114149

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

[PATCH] D114651: [clang-cl] Expose -Wall to clang-cl by unaliasing -Wall, keeping /Wall as alias to -Weverything

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D114651#3158807 , @rnk wrote:

> I will add that multiple users have run into this problem, and I think it 
> might be more practical to consider unaliasing Wall altogether. Clang doesn't 
> emit the same set of warnings as MSVC. Anyone seriously using `clang-cl 
> /Wall` is going to receive a pile of `-WcxxNN-compat` warnings that are 
> self-contradictory, and they are going to need to curate a set of 
> clang-specific warning flags anyway.

One possible downside with that, is that you'd easily end up making 
cl-compatible build files that works nicely with clang-cl but spew warnings 
with cl.exe. (Although maybe cl.exe's -Wall produce a much safer/saner set of 
warnings?) But I don't feel strongly about it..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114651

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

[PATCH] D114427: [clang-tidy] Warn on functional C-style casts

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz added inline comments.



Comment at: clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp:63-73
+static clang::CharSourceRange getReplaceRange(const ExplicitCastExpr *Expr) {
+  if (const auto *CastExpr = dyn_cast(Expr)) {
+return CharSourceRange::getCharRange(
+CastExpr->getLParenLoc(),
+CastExpr->getSubExprAsWritten()->getBeginLoc());
+  } else if (const auto *CastExpr = dyn_cast(Expr)) {
+return CharSourceRange::getCharRange(CastExpr->getBeginLoc(),

Looks much better than my suggested change. Good stuff.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:139
 
+- Updated ``google-readability-casting`` to diagnose and fix functional casts, 
to achieve feature
+  parity with the corresponding ``cpplint.py`` check.

Should this be like the
```
:doc:`bugprone-suspicious-memory-comparison
  `
```
a few lines above.


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

https://reviews.llvm.org/D114427

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

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devinj.jeanpierre added a comment.

Just a heads up, I think this is my first change to clang or llvm, and I'd 
appreciate any feedback you have on the code, review process, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114732

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

[PATCH] D114413: [OpenMPIRBuilder] Implement static-chunked workshare-loop schedules.

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

In D114413#3154936 , @peixin wrote:

> @Meinersbur Please rebase on main. The function "getPreheader()" was moved 
> into OMPIRBuilder.h.

I rebased, but I am not sure what you are referring to. `getPreheader()` always 
was in `OMPIRBuilder.h`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114413

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

[PATCH] D113518: [clang][Sema] Create delegating constructors even in templates

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:4524-4525
   if (!Dependent) {
-if (Context.hasSameUnqualifiedType(QualType(ClassDecl->getTypeForDecl(),0),
-   BaseType))
+if (Delegating)
   return BuildDelegatingInitializer(BaseTInfo, Init, ClassDecl);
 

Looking at the source for `BuildDelegatingInitializer()`, it looks like we 
should still be able to call this in a dependent context. In fact, it has a 
fixme comment specifically about that:
```
// If we are in a dependent context, template instantiation will
// perform this type-checking again. Just save the arguments that we
// received in a ParenListExpr.
// FIXME: This isn't quite ideal, since our ASTs don't capture all
// of the information that we have about the base
// initializer. However, deconstructing the ASTs is a dicey process,
// and this approach is far more likely to get the corner cases right.
```
I'm wondering if the better fix here is to hoist the delegation check out of 
the `if (!Dependent)`. Did you try that and run into issues?



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5071
 
-  if (CXXDestructorDecl *Dtor = LookupDestructor(Constructor->getParent())) {
-MarkFunctionReferenced(Initializer->getSourceLocation(), Dtor);
-DiagnoseUseOfDecl(Dtor, Initializer->getSourceLocation());
-  }
+  if (!Constructor->isDependentContext()) {
+if (CXXDestructorDecl *Dtor = LookupDestructor(Constructor->getParent())) {

Can you explain why this change was needed?


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

https://reviews.llvm.org/D113518

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

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devinj.jeanpierre created this revision.
devinj.jeanpierre added reviewers: rsmith, aaron.ballman, rjmccall, ahatanak.
Herald added a subscriber: dexonsmith.
devinj.jeanpierre requested review of this revision.
Herald added a project: clang.

This change enables library code to skip paired move-construction and
destruction for `trivial_abi` types, as if they were trivially-movable and
trivially-destructible. This offers an extension to the performance fix offered
by `trivial_abi`: rather than only offering trivial-type-like performance for
pass-by-value, it also offers it for library code that moves values but not as
arguments.

For example, if we use `memcpy` for trivially relocatable types inside of
vector reallocation, and mark `unique_ptr` as `trivial_abi` (via
`_LIBCPP_ABI_ENABLE_UNIQUE_PTR_TRIVIAL_ABI` / `_LIBCPP_ABI_UNSTABLE` / etc.),
this would speed up `vector::push_back` by 40% on my benchmarks.
(Though note that in this case, the compiler could have done this anyway, but
happens not to due to the inlining horizon.)

If accepted, I intend to follow up with exactly such changes to library code,
including and especially `std::vector`, making them use a trivial relocation
operation on trivially relocatable types.

D50119  and P1144 
:

This change is very similar to D50119 , which 
was rejected from Clang. (That
change was an implementation of P1144 , which 
is not yet part of the C++
standard.)

The intent of this change, rather than trying to pick a winning proposal for
trivial relocation operations, is to extend the behavior of `trivial_abi` in a
way that could be made compatible with any such proposal. If P1144 
 or any
similar proposal were accepted, then `trivial_abi`,
`__is_trivially_relocatable`, and everything else in this change would be
redefined in terms of that.

Safety:

It's worth pointing out, specifically, that `trivial_abi` already implies
trivial relocatability in a narrow sense: a `trivial_abi` type, when passed by
value, has its constructor run in one location, and its destructor run in
another, after the type has been trivially relocated (through registers).

Trivial relocatability optimizations could change the number of paired
constructor/destructor calls, but this seems unlikely to matter for
`trivial_abi` types.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114732

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/attr-trivial-abi.cpp
  clang/test/SemaCXX/type-traits.cpp
  clang/test/SemaObjCXX/arc-type-traits.mm
  clang/test/SemaObjCXX/objc-weak-type-traits.mm

Index: clang/test/SemaObjCXX/objc-weak-type-traits.mm
===
--- clang/test/SemaObjCXX/objc-weak-type-traits.mm
+++ clang/test/SemaObjCXX/objc-weak-type-traits.mm
@@ -8,7 +8,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) static_assert(!Trait(Type), "")
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) static_assert(Trait(Type1, Type2), "")
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) static_assert(!Trait(Type1, Type2), "")
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -208,3 +208,12 @@
 TRAIT_IS_FALSE_2(__is_trivially_constructible, HasWeak, HasWeak&&);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained&&);
+
+// __is_trivially_relocatable
+TRAIT_IS_TRUE(__is_trivially_relocatable, __strong id);
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __unsafe_unretained id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasStrong);
+TRAIT_IS_FALSE(__is_trivially_relocatable, HasWeak);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasUnsafeUnretained);
Index: clang/test/SemaObjCXX/arc-type-traits.mm
===
--- clang/test/SemaObjCXX/arc-type-traits.mm
+++ clang/test/SemaObjCXX/arc-type-traits.mm
@@ -12,7 +12,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) char JOIN2(Trait,__LINE__)[Trait(Type)? -1 : 1]
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? 1 : -1]
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? -1 : 1]
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -213,3 +213,11 @@
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnr

[PATCH] D114601: Read path to CUDA from env. variable CUDA_PATH on Windows

[Mojca Miklavec via Phabricator via cfe-commits]

mojca added inline comments.



Comment at: clang/lib/Driver/ToolChains/Cuda.cpp:137
   } else if (HostTriple.isOSWindows()) {
-for (const char *Ver : Versions)
-  Candidates.emplace_back(

tra wrote:
> Do we want to keep this as the fall-back for cases when `CUDA_PATH` is not 
> set?
> 
> Otherwise, we're risking to break existing users.
That was going to be my question as well (for which I wanted your input).
From the initial comments in the other review (D114326) I understood that 
eventually you wanted to get rid of the hardcoded list of versions (but I 
didn't fully understand the intent).

(Technically speaking this won't break existing users since they were only able 
to access CUDA versions up to 8.0 until now, and we would just remove support 
for anything older that CUDA 10.0.)


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

https://reviews.llvm.org/D114601

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

[PATCH] D40319: [libcxx] Support getentropy as a source of randomness for std::random_device

[Florian Weimer via Phabricator via cfe-commits]

fweimer added a comment.

`getentropy` comes from OpenBSD (among others), and it's in `` there: 
https://man.openbsd.org/getentropy.2


Repository:
  rL LLVM

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

https://reviews.llvm.org/D40319

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

[PATCH] D40319: [libcxx] Support getentropy as a source of randomness for std::random_device

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added subscribers: fweimer, dalias.
jwakely added inline comments.



Comment at: libcxx/trunk/src/random.cpp:29
+#if defined(_LIBCPP_USING_GETENTROPY)
+#include 
+#elif defined(_LIBCPP_USING_DEV_RANDOM)

mcgrathr wrote:
> jwakely wrote:
> > jwakely wrote:
> > > musl only declares `getentropy` in `` not ``. 
> > > Glibc declares it in both. Should `` also be included when 
> > > `_LIBCPP_USING_GETENTROPY` is defined, so it can work more portably?
> > I suppose it doesn't really matter, since `_LIBCPP_USING_GETENTROPY` is 
> > currently only defined for fuchsia and wasi, which both declare 
> > `getentropy` in ``.
> FWIW, if there is an emerging norm to declare getentropy in  then 
> Fuchsia's libc can add it to our  and there's no real problem 
> relying on that in libc++ sources within about a week of when we land that 
> change in Fuchsia's trunk.
> 
@fweimer added it to glibc's unistd.h for compatibility with existing BSD code:
https://sourceware.org/bugzilla/show_bug.cgi?id=17252#c9

Maybe @dalias can confirm whether musl did it for the same reason?

Does it make sense to "standardize" on declaring `getentropy` in `unistd.h`?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D40319

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

[PATCH] D114326: Update the list of CUDA versions up to 11.5

[Mojca Miklavec via Phabricator via cfe-commits]

mojca added a comment.

@tra: this is not yet 100% ready since the unit tests are now failing 
(expecting to find CUDA 8.0).
I can fix the unit test, but I suppose that someone needs to install additional 
SDK somewhere into the infrastructure as well?

In D114326#3158913 , @tra wrote:

> So, yes, you could automatically add linking flags in your example, but it's 
> not feasible to do so for the linking phase in general. It would also 
> introduce inconsistency in clang's linking phase behavior and would lead to 
> questions -- why things work with `clang hello.co`, but not with `clang -c 
> hello.cu; clang hello.o` ?

Please note that I'm a complete newbie to compilers.
Te flag `--cuda-path=` slightly reminds me of the `--framework` keyword on 
macOS. I'm not behind a mac right now, so please forgive me any typo or wrong 
paths (it's also not exact since the frameworks are searched for in different 
folders etc.), below is just a general idea.

  # this one effectively adds 
-I/System/Library/Frameworks/OpenGL.framework/Headers
  clang++ -c hello.cpp --framework OpenGL
  
  # this one effectively adds -L/System/Library/Frameworks/OpenGL.framework/lib 
as well as the library itself (-l)
  clang++ hello.o --framework OpenGL
  
  # this one adds both include (-I) and linker directory (-L) flags
  clang++ hello.cpp --framework OpenGL

What would be cool to me was if `--cuda-path` was also implicitly adding the 
`-L` flag, so that the following would work:

  clang++ --cuda-path=/usr/local/cuda-11.5 -l cudart hello.cu
  clang++ --cuda-path=/usr/local/cuda-11.5 -l cudart hello.o

I don't know whether it would be acceptable to add (the default) cuda library 
path to the linker though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114326

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

[PATCH] D113943: Add `withTag` matcher.

[James King via Phabricator via cfe-commits]

jcking1034 updated this revision to Diff 390429.
jcking1034 added a comment.

Chang `withTag` to `withIntrospection`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113943

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h

Index: clang/include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- clang/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ clang/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -43,6 +43,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/NestedNameSpecifier.h"
+#include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
@@ -1057,6 +1058,46 @@
   std::vector Names;
 };
 
+template 
+class WithIntrospectionMatcher : public MatcherInterface {
+public:
+  explicit WithIntrospectionMatcher(std::string _BeforeTag,
+std::string _AfterTag,
+internal::Matcher _InnerMatcher)
+  : BeforeTag(std::move(_BeforeTag)), AfterTag(std::move(_AfterTag)),
+InnerMatcher(_InnerMatcher) {}
+  explicit WithIntrospectionMatcher(internal::Matcher _InnerMatcher)
+  : BeforeTag("⭐ Attempting new match"),
+AfterTag("✔️ Concluding attempt"), InnerMatcher(_InnerMatcher) {}
+  bool matches(const T &Node, ASTMatchFinder *Finder,
+   BoundNodesTreeBuilder *Builder) const override {
+DynTypedNode DTN = DynTypedNode::create(Node);
+
+llvm::errs() << BeforeTag << "\n";
+
+llvm::errs() << "Matcher Name: " << InnerMatcher.getName() << "\n";
+
+llvm::errs() << "Node Kind: " << DTN.getNodeKind().asStringRef() << "\n"
+ << "Node Value:\n```\n";
+DTN.print(llvm::errs(), PrintingPolicy(LangOptions()));
+llvm::errs() << "\n```\n"
+ << "Node AST:\n";
+DTN.dump(llvm::errs(), Finder->getASTContext());
+
+bool result = InnerMatcher.matches(Node, Finder, Builder);
+llvm::errs() << "Result: " << (result ? "Successful\n" : "Unsuccessful\n");
+
+llvm::errs() << AfterTag << "\n\n";
+
+return result;
+  }
+
+private:
+  std::string BeforeTag;
+  std::string AfterTag;
+  internal::Matcher InnerMatcher;
+};
+
 /// Matches named declarations with a specific name.
 ///
 /// See \c hasName() and \c hasAnyName() in ASTMatchers.h for details.
@@ -1734,6 +1775,38 @@
   std::tuple Params;
 };
 
+template  class MatcherT,
+  typename ReturnTypesF, typename... ParamTypes>
+class PolymorphicWithIntrospectionMatcher {
+public:
+  PolymorphicWithIntrospectionMatcher(
+  internal::PolymorphicMatcher
+  _InnerMatcher)
+  : InnerMatcher(_InnerMatcher) {}
+
+  using ReturnTypes = typename ExtractFunctionArgMeta::type;
+
+  template  operator Matcher() const LLVM_LVALUE_FUNCTION {
+static_assert(TypeListContainsSuperOf::value,
+  "right polymorphic conversion");
+return internal::Matcher(new internal::WithIntrospectionMatcher(
+internal::Matcher(InnerMatcher)));
+  }
+
+#if LLVM_HAS_RVALUE_REFERENCE_THIS
+  template  operator Matcher() && {
+static_assert(TypeListContainsSuperOf::value,
+  "right polymorphic conversion");
+return internal::Matcher(new internal::WithIntrospectionMatcher(
+internal::Matcher(std::move(InnerMatcher;
+  }
+#endif
+
+private:
+  internal::PolymorphicMatcher
+  InnerMatcher;
+};
+
 /// Matches nodes of type T that have child nodes of type ChildT for
 /// which a specified child matcher matches.
 ///
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2993,6 +2993,46 @@
   new internal::HasNameMatcher({std::string(Name)}));
 }
 
+template 
+inline internal::Matcher
+withIntrospection(std::string BeforeTag, std::string AfterTag,
+  const internal::Matcher &InnerMatcher) {
+  return internal::Matcher(new internal::WithIntrospectionMatcher(
+  BeforeTag, AfterTag, InnerMatcher));
+}
+
+template  class MatcherT,
+  typename ReturnTypesF, typename... ParamTypes>
+inline internal::PolymorphicWithIntrospectionMatcher
+withIntrospection(
+std::string BeforeTag, std::string AfterTag,
+const internal::PolymorphicMatcher
+&InnerMatcher) {
+  return internal::PolymorphicWithIntrospectionMatcher(
+  BeforeTag, AfterTag, InnerMatcher);
+}
+
+template 
+inline internal::Matcher
+withIntrospection(const internal::Matcher &InnerMatcher) {
+  return internal::Matcher(
+  new internal::WithIntrospectionMatcher(InnerMatcher));
+}
+
+template  class MatcherT,
+  typename ReturnTypesF, typename... ParamTypes>
+inline in

[PATCH] D114231: [clang][docs][dataflow] Added an introduction to dataflow analysis

[Gábor Horváth via Phabricator via cfe-commits]

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

Thanks!




Comment at: clang/docs/DataFlowAnalysisIntro.md:111-112
+information that we track. In this case, we can, for example, arbitrarily limit
+the size of sets to 3 elements. If at a certain program point `x` has more than
+3 possible values, we stop tracking specific values at that program point.
+Instead, we denote possible values of `x` with the symbol `⊤` (pronounced "top"

gribozavr2 wrote:
> xazax.hun wrote:
> > I wonder if we want to cite widening and narrowing in this context to give 
> > people some pointers where to look for more info.
> I think it should be a separate section, with a slightly different example. 
> To show the value of widening we need a lattice that can abstract the 
> infinite set `{0; 1; 2; ...}` more precisely than "⊤" (e.g., "non-negative"). 
> There would be also the usual handwaving about figuring out how to make those 
> types of conclusions in the general case. I think adding all that here would 
> break the flow and make the explanation less crisp. Adding a separate section 
> on widening would be good one day, when the dataflow framework gets the 
> feature. I don't think it is necessary right now.
Sounds good.



Comment at: clang/docs/DataFlowAnalysisIntro.md:182
+void Example(int n) {
+  int x;  // x is ⊥
+  if (n > 0) {

gribozavr2 wrote:
> xazax.hun wrote:
> > I think it would make sense to include the whole program state in the 
> > comments. E.g. `// x is ⊥, n is ⊤`.
> > This could showcase how we constrain the value of `n` to `42` in one of the 
> > branches.
> So far we have been focusing on figuring out the values of `x` the local 
> variable, not `n` the parameter, so I'd rather not deviate. Also, comparing 
> `n` to 42 wouldn't necessarily lead to constraining the set of values here 
> (for example, in the framework we do it in the flow condition).
I see. Yeah, if the framework does not do that (yet), I see how it might be 
confusing. Let's leave this as is.



Comment at: clang/docs/DataFlowAnalysisIntro.md:279
+```
+out = transfer(basic_block, join(in_1, in_2, ..., in_n))
+```

gribozavr2 wrote:
> xazax.hun wrote:
> > While I agree that this is the general formula, people sometimes do small 
> > variations, e.g.:
> > ```
> > out =  join(transfer(basic_block,in_1), transfer(basic_block,in_2), ..., 
> > transfer(basic_block,in_n))
> > ```
> > 
> > This is less efficient as we end up invoking the transfer function more 
> > often, but it can be more precise. E.g. with some ad-hoc notation:
> > 
> > ```
> > Given the branches: x: {1}, x: {2}, x: {3}, x: {4}
> > join(in...) : x : {⊤}
> > transfer("x/2", ...) == x : {⊤}
> > 
> > vs.
> > Given the branches: x: {1}, x: {2}, x: {3}, x: {4}
> > transfer("x/2", ...) ==x : {0}, x : {1}, x : {1}, x: {2} == outs
> > join(outs) == x: {0, 1, 2}
> > ```
> This section is only attempting to give the readers an intuition of why 
> dataflow works, hence it must be straightforward, not necessarily exhaustive 
> or rigorous. I tried to add your suggestion here, but it breaks the flow. I 
> tried to add it at the end of the section, but it looks out of place. So, 
> sorry, I couldn't fit it here. If you have a specific wording suggestion that 
> works here, I would be happy to apply it.
> 
> I think one would need to add a section like "variations on dataflow 
> equations" to properly introduce the idea. It also seems to me that this 
> specific variation is also a special case of a more general idea of deferring 
> join operations to improve precision; that is, instead of doing a join 
> everywhere where classic dataflow prescribes it, we can instead keep 
> exploring separate paths, and only do a join at some later point in the CFG. 
> Similarly we can also unroll loops in a precise way up to a certain number of 
> iterations, and attempt to join/widen only at after that. These are of course 
> important ideas, but they don't help with what this document is trying to 
> achieve.
Agreed, we cannot introduce everything in an introductory document. I mainly 
wanted to avoid creating the impression that there is one definitive way to 
data flow analysis and this is it. Maybe one sentence at the end mentioning 
there are several variations that we do not discuss here could be sufficient. 



Comment at: clang/docs/DataFlowAnalysisIntro.md:478-479
+
+To analyze the function body we can use a lattice which consists of normal
+states and failure states. A normal state describes program points where we are
+sure that no behaviors that block the refactoring have occurred. Normal states

gribozavr2 wrote:
> xazax.hun wrote:
> > I wonder if the distinction between normal states and failure states is 
> > useful. In general, we can combine arbitrary number of lattices and 
> > propagate al

[PATCH] D114601: Read path to CUDA from env. variable CUDA_PATH on Windows

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Driver/ToolChains/Cuda.cpp:137
   } else if (HostTriple.isOSWindows()) {
-for (const char *Ver : Versions)
-  Candidates.emplace_back(

Do we want to keep this as the fall-back for cases when `CUDA_PATH` is not set?

Otherwise, we're risking to break existing users.


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

https://reviews.llvm.org/D114601

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

[PATCH] D114728: [Coroutine] Remove the prologue data of `-fsanitize=function` for split functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen created this revision.
ychen added reviewers: pcc, rjmccall.
Herald added subscribers: ChuanqiXu, lxfind, hiraditya.
ychen requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

There are no proper RTTI for these split functions. So just delete the
prologue data.

Fixes PR50345.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114728

Files:
  clang/test/CodeGenCXX/catch-undef-behavior.cpp
  llvm/lib/Transforms/Coroutines/CoroSplit.cpp
  llvm/test/Transforms/Coroutines/coro-split-00.ll


Index: llvm/test/Transforms/Coroutines/coro-split-00.ll
===
--- llvm/test/Transforms/Coroutines/coro-split-00.ll
+++ llvm/test/Transforms/Coroutines/coro-split-00.ll
@@ -1,7 +1,7 @@
 ; Tests that coro-split pass splits the coroutine into f, f.resume and 
f.destroy
 ; RUN: opt < %s -passes='cgscc(coro-split),simplifycfg,early-cse' -S | 
FileCheck %s
 
-define i8* @f() "coroutine.presplit"="1" {
+define i8* @f() "coroutine.presplit"="1" prologue <{ i32, i32 }> <{ i32 
846595819, i32 1 }> {
 entry:
   %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
   %need.alloc = call i1 @llvm.coro.alloc(token %id)
@@ -32,7 +32,7 @@
   ret i8* %hdl
 }
 
-; CHECK-LABEL: @f(
+; CHECK-LABEL: @f() prologue <{ i32, i32 }> <{ i32 846595819, i32 1 }> {
 ; CHECK: call i8* @malloc
 ; CHECK: @llvm.coro.begin(token %id, i8* %phi)
 ; CHECK: store void (%f.Frame*)* @f.resume, void (%f.Frame*)** %resume.addr
@@ -43,7 +43,7 @@
 ; CHECK-NOT: call void @free(
 ; CHECK: ret i8* %hdl
 
-; CHECK-LABEL: @f.resume(
+; CHECK-LABEL: @f.resume({{.*}}) {
 ; CHECK-NOT: call i8* @malloc
 ; CHECK-NOT: call void @print(i32 0)
 ; CHECK: call void @print(i32 1)
@@ -51,13 +51,13 @@
 ; CHECK: call void @free(
 ; CHECK: ret void
 
-; CHECK-LABEL: @f.destroy(
+; CHECK-LABEL: @f.destroy({{.*}}) {
 ; CHECK-NOT: call i8* @malloc
 ; CHECK-NOT: call void @print(
 ; CHECK: call void @free(
 ; CHECK: ret void
 
-; CHECK-LABEL: @f.cleanup(
+; CHECK-LABEL: @f.cleanup({{.*}}) {
 ; CHECK-NOT: call i8* @malloc
 ; CHECK-NOT: call void @print(
 ; CHECK-NOT: call void @free(
Index: llvm/lib/Transforms/Coroutines/CoroSplit.cpp
===
--- llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -896,6 +896,20 @@
   NewF->setUnnamedAddr(savedUnnamedAddr);
   NewF->setDLLStorageClass(savedDLLStorageClass);
 
+  if (Shape.ABI == coro::ABI::Switch && NewF->hasPrologueData()) {
+if (auto *CS = dyn_cast(NewF->getPrologueData())) {
+  if (auto *CI = dyn_cast(CS->getOperand(0))) {
+// This value should match the value returned by
+// TargetCodeGenInfo::getUBSanFunctionSignature().
+unsigned Sig = (0xeb << 0) | // jmp rel8
+   (0x06 << 8) | //   .+0x08
+   ('v' << 16) | ('2' << 24);
+if (CI->getZExtValue() == Sig)
+  NewF->setPrologueData(nullptr);
+  }
+}
+  }
+
   // Replace the attributes of the new function:
   auto OrigAttrs = NewF->getAttributes();
   auto NewAttrs = AttributeList();
Index: clang/test/CodeGenCXX/catch-undef-behavior.cpp
===
--- clang/test/CodeGenCXX/catch-undef-behavior.cpp
+++ clang/test/CodeGenCXX/catch-undef-behavior.cpp
@@ -399,7 +399,12 @@
   // CHECK-NEXT: br i1 [[AND]]
 }
 
-//
+// FIXME: If the function signature value is changed, make the same change in
+//CoroSplit. Making them share a single API to avoid this is better.
+//However, function prefix/prologue data mechansim do not work well 
with
+//coroutine split in general, we may want to redesign that or
+//deprecate some features (see intended usages in
+//https://reviews.llvm.org/D6454) in the future.
 // CHECK-LABEL: @_Z22indirect_function_callPFviE({{.*}} prologue <{ i32, i32 
}> <{ i32 846595819, i32 trunc (i64 sub (i64 ptrtoint (i8** {{.*}} to i64), i64 
ptrtoint (void (void (i32)*)* @_Z22indirect_function_callPFviE to i64)) to i32) 
}>
 // CHECK-X32: @_Z22indirect_function_callPFviE({{.*}} prologue <{ i32, i32 }> 
<{ i32 846595819, i32 sub (i32 ptrtoint (i8** [[IndirectRTTI_ZTIFvPFviEE]] to 
i32), i32 ptrtoint (void (void (i32)*)* @_Z22indirect_function_callPFviE to 
i32)) }>
 // CHECK-X86: @_Z22indirect_function_callPFviE({{.*}} prologue <{ i32, i32 }> 
<{ i32 846595819, i32 sub (i32 ptrtoint (i8** [[IndirectRTTI_ZTIFvPFviEE]] to 
i32), i32 ptrtoint (void (void (i32)*)* @_Z22indirect_function_callPFviE to 
i32)) }>


Index: llvm/test/Transforms/Coroutines/coro-split-00.ll
===
--- llvm/test/Transforms/Coroutines/coro-split-00.ll
+++ llvm/test/Transforms/Coroutines/coro-split-00.ll
@@ -1,7 +1,7 @@
 ; Tests that coro-split pass splits the coroutine into f, f.resume and f.

[PATCH] D40319: [libcxx] Support getentropy as a source of randomness for std::random_device

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: libcxx/trunk/src/random.cpp:29
+#if defined(_LIBCPP_USING_GETENTROPY)
+#include 
+#elif defined(_LIBCPP_USING_DEV_RANDOM)

jwakely wrote:
> jwakely wrote:
> > musl only declares `getentropy` in `` not ``. Glibc 
> > declares it in both. Should `` also be included when 
> > `_LIBCPP_USING_GETENTROPY` is defined, so it can work more portably?
> I suppose it doesn't really matter, since `_LIBCPP_USING_GETENTROPY` is 
> currently only defined for fuchsia and wasi, which both declare `getentropy` 
> in ``.
FWIW, if there is an emerging norm to declare getentropy in  then 
Fuchsia's libc can add it to our  and there's no real problem relying 
on that in libc++ sources within about a week of when we land that change in 
Fuchsia's trunk.



Repository:
  rL LLVM

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

https://reviews.llvm.org/D40319

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

[PATCH] D114639: Raise the minimum Visual Studio version to VS2019

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.



Comment at: clang/docs/UsersManual.rst:3546
 
-cmake -G"Visual Studio 15 2017" -T LLVM ..
+cmake -G"Visual Studio 17 2022" -T LLVM ..
 

Meinersbur wrote:
> jhenderson wrote:
> > RKSimon wrote:
> > > aaron.ballman wrote:
> > > > jhenderson wrote:
> > > > > I think the missing space should be fixed to :)
> > > > +1 to the missing space.
> > > This one is confusing - it isn't in my local diff, the raw diff, or if I 
> > > reapply the raw diff - it looks to have just appeared when you quote it 
> > > in phab comments?
> > The fact that the space is missing? It's missing in the current repo too.
> It works with and without the space, like `-GNinja` and `-G Ninja` both work.
It would be nice to mention the CMake minimum version.  I think that you need 
3.22 or newer for the 2022 generator/toolset definition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114639

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

[PATCH] D114615: [NFC][clang]Increase the number of driver diagnostics

[Steven Wan 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 rG3c32c568844c: [NFC][clang]Increase the number of driver 
diagnostics (authored by stevewan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114615

Files:
  clang/include/clang/Basic/DiagnosticIDs.h


Index: clang/include/clang/Basic/DiagnosticIDs.h
===
--- clang/include/clang/Basic/DiagnosticIDs.h
+++ clang/include/clang/Basic/DiagnosticIDs.h
@@ -30,7 +30,7 @@
 // Size of each of the diagnostic categories.
 enum {
   DIAG_SIZE_COMMON=  300,
-  DIAG_SIZE_DRIVER=  250,
+  DIAG_SIZE_DRIVER=  300,
   DIAG_SIZE_FRONTEND  =  150,
   DIAG_SIZE_SERIALIZATION =  120,
   DIAG_SIZE_LEX   =  400,


Index: clang/include/clang/Basic/DiagnosticIDs.h
===
--- clang/include/clang/Basic/DiagnosticIDs.h
+++ clang/include/clang/Basic/DiagnosticIDs.h
@@ -30,7 +30,7 @@
 // Size of each of the diagnostic categories.
 enum {
   DIAG_SIZE_COMMON=  300,
-  DIAG_SIZE_DRIVER=  250,
+  DIAG_SIZE_DRIVER=  300,
   DIAG_SIZE_FRONTEND  =  150,
   DIAG_SIZE_SERIALIZATION =  120,
   DIAG_SIZE_LEX   =  400,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 3c32c56 - [NFC][clang]Increase the number of driver diagnostics

[Steven Wan via cfe-commits]

Author: Steven Wan
Date: 2021-11-29T14:12:03-05:00
New Revision: 3c32c568844c745e3fe7fa72ce3aa65340e545bc

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

LOG: [NFC][clang]Increase the number of driver diagnostics

We're close to hitting the limited number of driver diagnostics, increase 
`DIAG_SIZE_DRIVER` to accommodate more.

Reviewed By: erichkeane

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticIDs.h

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticIDs.h 
b/clang/include/clang/Basic/DiagnosticIDs.h
index aef86516707c..375930c14848 100644
--- a/clang/include/clang/Basic/DiagnosticIDs.h
+++ b/clang/include/clang/Basic/DiagnosticIDs.h
@@ -30,7 +30,7 @@ namespace clang {
 // Size of each of the diagnostic categories.
 enum {
   DIAG_SIZE_COMMON=  300,
-  DIAG_SIZE_DRIVER=  250,
+  DIAG_SIZE_DRIVER=  300,
   DIAG_SIZE_FRONTEND  =  150,
   DIAG_SIZE_SERIALIZATION =  120,
   DIAG_SIZE_LEX   =  400,



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

[PATCH] D114553: [HIP] Add atomic load, atomic store and atomic cmpxchng_weak builtin support in HIP-clang

[Anshil Gandhi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdf0560ca0018: [HIP] Add atomic load, atomic store and atomic 
cmpxchng_weak builtin support in… (authored by gandhi21299).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114553

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/Builtins.def
  clang/lib/AST/Expr.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGenCUDA/atomic-ops.cu
  clang/test/SemaCUDA/atomic-ops.cu

Index: clang/test/SemaCUDA/atomic-ops.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/atomic-ops.cu
@@ -0,0 +1,86 @@
+// RUN: %clang_cc1 -x hip -std=c++11 -triple amdgcn -fcuda-is-device -verify -fsyntax-only %s
+
+#include "Inputs/cuda.h"
+
+__device__ int test_hip_atomic_load(int *pi32, unsigned int *pu32, long long *pll, unsigned long long *pull, float *fp, double *dbl) {
+  int val = __hip_atomic_load(0);  // expected-error {{too few arguments to function call, expected 3, have 1}}
+  val = __hip_atomic_load(0, 0, 0, 0); // expected-error {{too many arguments to function call, expected 3, have 4}}
+  val = __hip_atomic_load(0, 0, 0);// expected-error {{address argument to atomic builtin must be a pointer ('int' invalid)}}
+  val = __hip_atomic_load(pi32, 0, 0); // expected-error {{synchronization scope argument to atomic operation is invalid}}
+  val = __hip_atomic_load(pi32, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_load(pi32, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  val = __hip_atomic_load(pi32, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WORKGROUP);
+  val = __hip_atomic_load(pi32, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_AGENT);
+  val = __hip_atomic_load(pi32, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SYSTEM);
+  val = __hip_atomic_load(pi32, __ATOMIC_RELAXED, 6); // expected-error {{synchronization scope argument to atomic operation is invalid}}
+  val = __hip_atomic_load(pi32, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_load(pi32, __ATOMIC_SEQ_CST, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_load(pi32, __ATOMIC_CONSUME, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_load(pi32, __ATOMIC_ACQUIRE, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_load(pi32, __ATOMIC_ACQ_REL, __HIP_MEMORY_SCOPE_SINGLETHREAD); // expected-warning{{memory order argument to atomic operation is invalid}}
+  val = __hip_atomic_load(pu32, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_load(pll, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_load(pull, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_load(fp, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_load(dbl, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  return val;
+}
+
+__device__ int test_hip_atomic_store(int *pi32, unsigned int *pu32, long long *pll, unsigned long long *pull, float *fp, double *dbl,
+ int i32, unsigned int u32, long long i64, unsigned long long u64, float f32, double f64) {
+  __hip_atomic_store(0); // expected-error {{too few arguments to function call, expected 4, have 1}}
+  __hip_atomic_store(0, 0, 0, 0, 0); // expected-error {{too many arguments to function call, expected 4, have 5}}
+  __hip_atomic_store(0, 0, 0, 0);// expected-error {{address argument to atomic builtin must be a pointer ('int' invalid)}}
+  __hip_atomic_store(pi32, 0, 0, 0); // expected-error {{synchronization scope argument to atomic operation is invalid}}
+  __hip_atomic_store(pi32, 0, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  __hip_atomic_store(pi32, 0, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  __hip_atomic_store(pi32, 0, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WORKGROUP);
+  __hip_atomic_store(pi32, 0, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_AGENT);
+  __hip_atomic_store(pi32, 0, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SYSTEM);
+  __hip_atomic_store(pi32, 0, __ATOMIC_RELAXED, 6); // expected-error {{synchronization scope argument to atomic operation is invalid}}
+  __hip_atomic_store(pi32, 0, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  __hip_atomic_store(pi32, 0, __ATOMIC_SEQ_CST, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  __hip_atomic_store(pi32, 0, __ATOMIC_CONSUME, __HIP_MEMORY_SCOPE_SINGLETHREAD); // expected-warning{{memory order argument to atomic operation is invalid}}
+  __hip_atomic_store(pi32, 0, __ATOMIC_ACQUIRE, __HIP_MEMORY_SCOPE_SINGLETHREAD); // expected-warning{{memory order argument to atomic operation is invalid}}
+  __hip_atomic_store(pi32, 0, __ATOMIC_ACQ_REL, __HIP_MEMORY_SCOPE_SINGLETHREAD); // expected-warning{{memory order argument to atomic operation is invalid}}
+  __hip_atomic_store(p

[clang] df0560c - [HIP] Add atomic load, atomic store and atomic cmpxchng_weak builtin support in HIP-clang

[Anshil Gandhi via cfe-commits]

Author: Anshil Gandhi
Date: 2021-11-29T12:07:13-07:00
New Revision: df0560ca00182364e0a786d35adb294c3c98dbd0

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

LOG: [HIP] Add atomic load, atomic store and atomic cmpxchng_weak builtin 
support in HIP-clang

Introduce `__hip_atomic_load`, `__hip_atomic_store` and 
`__hip_atomic_compare_exchange_weak`
builtins in HIP.

Reviewed By: yaxunl

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

Added: 
clang/test/SemaCUDA/atomic-ops.cu

Modified: 
clang/include/clang/AST/Expr.h
clang/include/clang/Basic/Builtins.def
clang/lib/AST/Expr.cpp
clang/lib/AST/StmtPrinter.cpp
clang/lib/CodeGen/CGAtomic.cpp
clang/lib/Sema/SemaChecking.cpp
clang/test/CodeGenCUDA/atomic-ops.cu

Removed: 




diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 246585e1205fa..2c63406fba18d 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -6308,6 +6308,7 @@ class AtomicExpr : public Expr {
getOp() == AO__hip_atomic_compare_exchange_strong ||
getOp() == AO__opencl_atomic_compare_exchange_strong ||
getOp() == AO__opencl_atomic_compare_exchange_weak ||
+   getOp() == AO__hip_atomic_compare_exchange_weak ||
getOp() == AO__atomic_compare_exchange ||
getOp() == AO__atomic_compare_exchange_n;
   }
@@ -6342,10 +6343,9 @@ class AtomicExpr : public Expr {
 auto Kind =
 (Op >= AO__opencl_atomic_load && Op <= AO__opencl_atomic_fetch_max)
 ? AtomicScopeModelKind::OpenCL
-: (Op >= AO__hip_atomic_compare_exchange_strong &&
-   Op <= AO__hip_atomic_fetch_max)
-  ? AtomicScopeModelKind::HIP
-  : AtomicScopeModelKind::None;
+: (Op >= AO__hip_atomic_load && Op <= AO__hip_atomic_fetch_max)
+? AtomicScopeModelKind::HIP
+: AtomicScopeModelKind::None;
 return AtomicScopeModel::create(Kind);
   }
 

diff  --git a/clang/include/clang/Basic/Builtins.def 
b/clang/include/clang/Basic/Builtins.def
index 1f7680e0d923c..ad8b66aa490be 100644
--- a/clang/include/clang/Basic/Builtins.def
+++ b/clang/include/clang/Basic/Builtins.def
@@ -855,8 +855,9 @@ ATOMIC_BUILTIN(__atomic_fetch_min, "v.", "t")
 ATOMIC_BUILTIN(__atomic_fetch_max, "v.", "t")
 
 // HIP atomic builtins.
-// FIXME: Is `__hip_atomic_compare_exchange_n` or
-// `__hip_atomic_compare_exchange_weak` needed?
+ATOMIC_BUILTIN(__hip_atomic_load, "v.", "t")
+ATOMIC_BUILTIN(__hip_atomic_store, "v.", "t")
+ATOMIC_BUILTIN(__hip_atomic_compare_exchange_weak, "v.", "t")
 ATOMIC_BUILTIN(__hip_atomic_compare_exchange_strong, "v.", "t")
 ATOMIC_BUILTIN(__hip_atomic_exchange, "v.", "t")
 ATOMIC_BUILTIN(__hip_atomic_fetch_add, "v.", "t")

diff  --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index ce6e30697f856..d3cb2ff3734cb 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -4681,6 +4681,7 @@ unsigned AtomicExpr::getNumSubExprs(AtomicOp Op) {
 return 2;
 
   case AO__opencl_atomic_load:
+  case AO__hip_atomic_load:
   case AO__c11_atomic_store:
   case AO__c11_atomic_exchange:
   case AO__atomic_load:
@@ -4721,6 +4722,7 @@ unsigned AtomicExpr::getNumSubExprs(AtomicOp Op) {
   case AO__hip_atomic_fetch_min:
   case AO__hip_atomic_fetch_max:
   case AO__opencl_atomic_store:
+  case AO__hip_atomic_store:
   case AO__opencl_atomic_exchange:
   case AO__opencl_atomic_fetch_add:
   case AO__opencl_atomic_fetch_sub:
@@ -4738,6 +4740,7 @@ unsigned AtomicExpr::getNumSubExprs(AtomicOp Op) {
   case AO__hip_atomic_compare_exchange_strong:
   case AO__opencl_atomic_compare_exchange_strong:
   case AO__opencl_atomic_compare_exchange_weak:
+  case AO__hip_atomic_compare_exchange_weak:
   case AO__atomic_compare_exchange:
   case AO__atomic_compare_exchange_n:
 return 6;

diff  --git a/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp
index fc267d7006a1b..b65a38d1e5665 100644
--- a/clang/lib/AST/StmtPrinter.cpp
+++ b/clang/lib/AST/StmtPrinter.cpp
@@ -1691,7 +1691,8 @@ void StmtPrinter::VisitAtomicExpr(AtomicExpr *Node) {
   PrintExpr(Node->getPtr());
   if (Node->getOp() != AtomicExpr::AO__c11_atomic_load &&
   Node->getOp() != AtomicExpr::AO__atomic_load_n &&
-  Node->getOp() != AtomicExpr::AO__opencl_atomic_load) {
+  Node->getOp() != AtomicExpr::AO__opencl_atomic_load &&
+  Node->getOp() != AtomicExpr::AO__hip_atomic_load) {
 OS << ", ";
 PrintExpr(Node->getVal1());
   }

diff  --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp
index 9b507b87213a3..b68e6328acdfd 100644
--- a/clang/lib/CodeGen/CGAtomic.cpp
+++ b/clang/lib/CodeGen/CGAtomic.cpp
@@ -531,6 +531,7 @@ static void EmitAtomicOp(CodeGenFunction

[PATCH] D114553: [HIP] Add atomic load, atomic store and atomic cmpxchng_weak builtin support in HIP-clang

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 added a comment.

Thanks for the review, I will merge this patch in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114553

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

[PATCH] D114725: [clang-format] add back tests which didn't cause a regression

[C. Rayroud via Phabricator via cfe-commits]

crayroud added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:14125
   verifyFormat("typedef void (*cb) (int);", Space);
   // FIXME these tests regressed behaviour.
+  verifyFormat("T A::operator() ();", Space);

Remove the comment too.



Comment at: clang/unittests/Format/FormatTest.cpp:14277
+  // FIXME these tests regressed behaviour.
   // verifyFormat("T A::operator() () {}", SpaceFuncDef);
   verifyFormat("auto lambda = [] () { return 0; };", SpaceFuncDef);

What do you think of enabling this test and to remove the space before the 
opening parenthesis ? As it is now the expected behaviour to have no space.



Comment at: clang/unittests/Format/FormatTest.cpp:14353
+  // FIXME these tests regressed behaviour.
   // verifyFormat("X A::operator++ (T);", SomeSpace2);
   verifyFormat("int x = int (y);", SomeSpace2);

As the the condition is to add a space before non empty parentheses, the test 
is valid.


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

https://reviews.llvm.org/D114725

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

[PATCH] D114162: [X86][clang] Enable floating-point type for -mno-x87 option on 32-bits

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers accepted this revision.
nickdesaulniers added a comment.

Thanks for the patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114162

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

[PATCH] D114706: [analyzer] Fix sensitive argument logic in GenericTaintChecker

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

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

E.g. `execl()` and `execlp` functions are actually variadic. You should also 
account for them.
I would rather map directly to a full-fledged Propagation rule instead of to a 
sensitive arg index list. That way you could express this property.
Demonstrate this in a test.

Actually, the `CallDescriptionFlags::CDF_MaybeBuiltin` CallDescriptions will 
use the `CCtx::isCLibraryFunction()` for matching the call.
By using that the code would get significantly shorter and more readable as 
well.




Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:951-957
+  // Find the first argument that receives a tainted value.
+  // The report is emitted as a side-effect.
+  return llvm::find_if(SensitiveArgs, [this, &C, &Call](unsigned Idx) {
+   return Idx < Call.getNumArgs() &&
+  generateReportIfTainted(Call.getArgExpr(Idx), MsgCustomSink,
+  C);
+ }) != SensitiveArgs.end();

These algorithms are not supposed to have side-effects. Even though we 
technically could have side-effects, I would still apply it separately.
That being said, this seems to be a copy-paste version of the previous hunk, 
like 3rd time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114706

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

[clang] 6c27d38 - OpenMP: Start calling setTargetAttributes for generated kernels

[Matt Arsenault via cfe-commits]

Author: Matt Arsenault
Date: 2021-11-29T13:43:34-05:00
New Revision: 6c27d389c8a00040aad998fe959f38ba709a8750

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

LOG: OpenMP: Start calling setTargetAttributes for generated kernels

This wasn't setting any of the attributes the target would expect to
emit for kernels.

Added: 
clang/test/OpenMP/amdgcn-attributes.cpp

Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/TargetInfo.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 75709b3c7e782..c3a01448389b3 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -15,6 +15,7 @@
 #include "CGCleanup.h"
 #include "CGRecordLayout.h"
 #include "CodeGenFunction.h"
+#include "TargetInfo.h"
 #include "clang/AST/APValue.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
@@ -6620,6 +6621,8 @@ void CGOpenMPRuntime::emitTargetOutlinedFunctionHelper(
 OutlinedFn->addFnAttr("omp_target_thread_limit",
   std::to_string(DefaultValThreads));
   }
+
+  CGM.getTargetCodeGenInfo().setTargetAttributes(nullptr, OutlinedFn, CGM);
 }
 
 /// Checks if the expression is constant or does not have non-trivial function

diff  --git a/clang/lib/CodeGen/TargetInfo.cpp 
b/clang/lib/CodeGen/TargetInfo.cpp
index 4360269f8af19..e94436d2e72ae 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -9143,6 +9143,10 @@ class AMDGPUTargetCodeGenInfo : public TargetCodeGenInfo 
{
 public:
   AMDGPUTargetCodeGenInfo(CodeGenTypes &CGT)
   : TargetCodeGenInfo(std::make_unique(CGT)) {}
+
+  void setFunctionDeclAttributes(const FunctionDecl *FD, llvm::Function *F,
+ CodeGenModule &CGM) const;
+
   void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &M) const override;
   unsigned getOpenCLKernelCallingConv() const override;
@@ -9182,36 +9186,13 @@ static bool requiresAMDGPUProtectedVisibility(const 
Decl *D,
cast(D)->getType()->isCUDADeviceBuiltinTextureType()));
 }
 
-void AMDGPUTargetCodeGenInfo::setTargetAttributes(
-const Decl *D, llvm::GlobalValue *GV, CodeGen::CodeGenModule &M) const {
-  if (requiresAMDGPUProtectedVisibility(D, GV)) {
-GV->setVisibility(llvm::GlobalValue::ProtectedVisibility);
-GV->setDSOLocal(true);
-  }
-
-  if (GV->isDeclaration())
-return;
-  const FunctionDecl *FD = dyn_cast_or_null(D);
-  if (!FD)
-return;
-
-  llvm::Function *F = cast(GV);
-
-  const auto *ReqdWGS = M.getLangOpts().OpenCL ?
-FD->getAttr() : nullptr;
-
-
-  const bool IsOpenCLKernel = M.getLangOpts().OpenCL &&
-  FD->hasAttr();
-  const bool IsHIPKernel = M.getLangOpts().HIP &&
-   FD->hasAttr();
-  if ((IsOpenCLKernel || IsHIPKernel) &&
-  (M.getTriple().getOS() == llvm::Triple::AMDHSA))
-F->addFnAttr("amdgpu-implicitarg-num-bytes", "56");
-
-  if (IsHIPKernel)
-F->addFnAttr("uniform-work-group-size", "true");
-
+void AMDGPUTargetCodeGenInfo::setFunctionDeclAttributes(
+const FunctionDecl *FD, llvm::Function *F, CodeGenModule &M) const {
+  const auto *ReqdWGS =
+  M.getLangOpts().OpenCL ? FD->getAttr() : nullptr;
+  const bool IsOpenCLKernel =
+  M.getLangOpts().OpenCL && FD->hasAttr();
+  const bool IsHIPKernel = M.getLangOpts().HIP && 
FD->hasAttr();
 
   const auto *FlatWGS = FD->getAttr();
   if (ReqdWGS || FlatWGS) {
@@ -9279,6 +9260,38 @@ void AMDGPUTargetCodeGenInfo::setTargetAttributes(
 if (NumVGPR != 0)
   F->addFnAttr("amdgpu-num-vgpr", llvm::utostr(NumVGPR));
   }
+}
+
+void AMDGPUTargetCodeGenInfo::setTargetAttributes(
+const Decl *D, llvm::GlobalValue *GV, CodeGen::CodeGenModule &M) const {
+  if (requiresAMDGPUProtectedVisibility(D, GV)) {
+GV->setVisibility(llvm::GlobalValue::ProtectedVisibility);
+GV->setDSOLocal(true);
+  }
+
+  if (GV->isDeclaration())
+return;
+
+  llvm::Function *F = dyn_cast(GV);
+  if (!F)
+return;
+
+  const FunctionDecl *FD = dyn_cast_or_null(D);
+  if (FD)
+setFunctionDeclAttributes(FD, F, M);
+
+  const bool IsOpenCLKernel =
+  M.getLangOpts().OpenCL && FD && FD->hasAttr();
+  const bool IsHIPKernel =
+  M.getLangOpts().HIP && FD && FD->hasAttr();
+
+  const bool IsOpenMP = M.getLangOpts().OpenMP && !FD;
+  if ((IsOpenCLKernel || IsHIPKernel || IsOpenMP) &&
+  (M.getTriple().getOS() == llvm::Triple::AMDHSA))
+F->addFnAttr("amdgpu-implicitarg-num-bytes", "56");
+
+  if (IsHIPKernel)
+F->addFnAttr("uniform-work-group-size", "true");
 
   if (M.getContext().getTargetInfo().allowAMDGPUUnsafeFPAtomics())
 F->addFnAttr("amdgpu-unsafe-

[PATCH] D114143: [OpenMP][IRBuilder] Fix createSections

[Michael Kruse via Phabricator via cfe-commits]

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

AllocaIP could be handled better, but as a fix LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114143

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

[PATCH] D114326: Update the list of CUDA versions up to 11.5

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D114326#3154228 , @mojca wrote:

> Somewhat off-topic from a discussion earlier in the thread.
> What's the purpose of the following code then if users are supposed to 
> explicitly specify the `-L` flag anyway?

Good point, it is indeed unused.

> clang++.exe hello.cu -o hello --cuda-path="C:/Program Files/NVIDIA GPU 
> Computing Toolkit/CUDA/v11.4" -l cudart -L "C:/Program Files/NVIDIA GPU 
> Computing Toolkit/CUDA/v11.4/lib/x64"
>
>   Also, it's probably destined to lead into issues if the user needs to 
> manually specify the library path, while the cuda path is determined 
> automatically.

Yup. That's why I'm a strong proponent of not autosearching beyond the 
canonical location and ask users to explicitly specify the CUDA SDK they want 
to use. 
In case of the canonical location on windows, users can then use `-L 
"$CUDA_PATH%/lib/x64"` everywhere.

As for automatically adding linker flage, it's been considered before and the 
conclusion was that it's not very useful.
Unlike nvcc, clang's linking phase has no idea about CUDA and we can not assume 
that any of the objects or libraries we're linking with have anything to do 
with CUDA. 
It is useful in a niche case when one compiles source-to-executable in one 
clang invocation and where we do know that we're dealing with CUDA, but this is 
not how clang is used most of the time, when compilation and linking are done 
separately.

So, yes, you could automatically add linking flags in your example, but it's 
not feasible to do so for the linking phase in general. It would also introduce 
inconsistency in clang's linking phase behavior and would lead to questions -- 
why things work with `clang hello.co`, but not with `clang -c hello.cu; clang 
hello.o` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114326

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

[PATCH] D114721: [clang][dataflow] Add unit tests for PostOrderCFGView

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

I don't have strong opinions, just some small preferences, feel free to ignore 
them:

- Using ASTMatchers feels a bit heavy weight for this task to me. Simply 
enumerating the function definitions in the `TranslationUnitDecl` should be 
sufficient for these tests.
- The tests might be a bit too strict, but this might be by design. In fact, 
there are more than one (reverse) post orders traversals of the same Cfg. Do we 
want to bake that particular order into the tests? This makes it harder to do 
changes but makes sure the tests notify us about such changes. Alternatively, 
we could do property-based testing, and verify that the view has the 
fundamental property we are interested in. Namely, if we visit nodes in the 
right order, we always visit all the predecessors of a node first (before 
visiting the node itself). Property-based tests are resilient to changes in 
which order we pick, and they can also make it somewhat easier to write tests 
(we no longer need to know about basic block IDs).




Comment at: clang/unittests/Analysis/Analyses/PostOrderCFGViewTest.cpp:23
+
+namespace clang {
+namespace analysis {

We could use `using namespace` here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114721

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

[PATCH] D114484: [NFC][AIX]Disable unsupported hip test on AIX

[Steven Wan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG23dc88622630: [NFC][AIX]Disable unsupported hip test on AIX 
(authored by stevewan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114484

Files:
  clang/test/Driver/hip-version.hip


Index: clang/test/Driver/hip-version.hip
===
--- clang/test/Driver/hip-version.hip
+++ clang/test/Driver/hip-version.hip
@@ -1,6 +1,7 @@
 // REQUIRES: clang-driver
 // REQUIRES: x86-registered-target
 // REQUIRES: amdgpu-registered-target
+// UNSUPPORTED: aix
 
 // RUN: %clang -v --rocm-path=%S/Inputs/rocm 2>&1 \
 // RUN:   | FileCheck -check-prefixes=FOUND %s


Index: clang/test/Driver/hip-version.hip
===
--- clang/test/Driver/hip-version.hip
+++ clang/test/Driver/hip-version.hip
@@ -1,6 +1,7 @@
 // REQUIRES: clang-driver
 // REQUIRES: x86-registered-target
 // REQUIRES: amdgpu-registered-target
+// UNSUPPORTED: aix
 
 // RUN: %clang -v --rocm-path=%S/Inputs/rocm 2>&1 \
 // RUN:   | FileCheck -check-prefixes=FOUND %s
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 23dc886 - [NFC][AIX]Disable unsupported hip test on AIX

[Steven Wan via cfe-commits]

Author: Steven Wan
Date: 2021-11-29T13:26:26-05:00
New Revision: 23dc886226306d961c31987db9aad137a69ad539

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

LOG: [NFC][AIX]Disable unsupported hip test on AIX

AIX doesn't support GPU. There is no point testing HIP on it.

Reviewed By: Jake-Egan

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

Added: 


Modified: 
clang/test/Driver/hip-version.hip

Removed: 




diff  --git a/clang/test/Driver/hip-version.hip 
b/clang/test/Driver/hip-version.hip
index 83b8dbddf228c..af98e025a 100644
--- a/clang/test/Driver/hip-version.hip
+++ b/clang/test/Driver/hip-version.hip
@@ -1,6 +1,7 @@
 // REQUIRES: clang-driver
 // REQUIRES: x86-registered-target
 // REQUIRES: amdgpu-registered-target
+// UNSUPPORTED: aix
 
 // RUN: %clang -v --rocm-path=%S/Inputs/rocm 2>&1 \
 // RUN:   | FileCheck -check-prefixes=FOUND %s



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

[PATCH] D114725: [clang-format] add back tests which didn't cause a regression

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 390407.
MyDeveloperDay added a comment.

bring back another 2 verifyFormats


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

https://reviews.llvm.org/D114725

Files:
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14123,8 +14123,8 @@
   verifyFormat("int f () throw (Deprecated);", Space);
   verifyFormat("typedef void (*cb) (int);", Space);
   // FIXME these tests regressed behaviour.
-  // verifyFormat("T A::operator() ();", Space);
-  // verifyFormat("X A::operator++ (T);", Space);
+  verifyFormat("T A::operator() ();", Space);
+  verifyFormat("X A::operator++ (T);", Space);
   verifyFormat("auto lambda = [] () { return 0; };", Space);
   verifyFormat("int x = int (y);", Space);
 
@@ -14180,8 +14180,7 @@
   verifyFormat("int f() throw (Deprecated);", SomeSpace);
   verifyFormat("typedef void (*cb) (int);", SomeSpace);
   verifyFormat("T A::operator()();", SomeSpace);
-  // FIXME these tests regressed behaviour.
-  // verifyFormat("X A::operator++ (T);", SomeSpace);
+  verifyFormat("X A::operator++ (T);", SomeSpace);
   verifyFormat("int x = int (y);", SomeSpace);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 
@@ -14274,6 +14273,7 @@
   verifyFormat("typedef void (*cb)(int);", SpaceFuncDef);
   verifyFormat("T A::operator()();", SpaceFuncDef);
   verifyFormat("X A::operator++(T);", SpaceFuncDef);
+  // FIXME these tests regressed behaviour.
   // verifyFormat("T A::operator() () {}", SpaceFuncDef);
   verifyFormat("auto lambda = [] () { return 0; };", SpaceFuncDef);
   verifyFormat("int x = int(y);", SpaceFuncDef);
@@ -14349,6 +14349,7 @@
   verifyFormat("int f() throw (Deprecated);", SomeSpace2);
   verifyFormat("typedef void (*cb) (int);", SomeSpace2);
   verifyFormat("T A::operator()();", SomeSpace2);
+  // FIXME these tests regressed behaviour.
   // verifyFormat("X A::operator++ (T);", SomeSpace2);
   verifyFormat("int x = int (y);", SomeSpace2);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace2);


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14123,8 +14123,8 @@
   verifyFormat("int f () throw (Deprecated);", Space);
   verifyFormat("typedef void (*cb) (int);", Space);
   // FIXME these tests regressed behaviour.
-  // verifyFormat("T A::operator() ();", Space);
-  // verifyFormat("X A::operator++ (T);", Space);
+  verifyFormat("T A::operator() ();", Space);
+  verifyFormat("X A::operator++ (T);", Space);
   verifyFormat("auto lambda = [] () { return 0; };", Space);
   verifyFormat("int x = int (y);", Space);
 
@@ -14180,8 +14180,7 @@
   verifyFormat("int f() throw (Deprecated);", SomeSpace);
   verifyFormat("typedef void (*cb) (int);", SomeSpace);
   verifyFormat("T A::operator()();", SomeSpace);
-  // FIXME these tests regressed behaviour.
-  // verifyFormat("X A::operator++ (T);", SomeSpace);
+  verifyFormat("X A::operator++ (T);", SomeSpace);
   verifyFormat("int x = int (y);", SomeSpace);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 
@@ -14274,6 +14273,7 @@
   verifyFormat("typedef void (*cb)(int);", SpaceFuncDef);
   verifyFormat("T A::operator()();", SpaceFuncDef);
   verifyFormat("X A::operator++(T);", SpaceFuncDef);
+  // FIXME these tests regressed behaviour.
   // verifyFormat("T A::operator() () {}", SpaceFuncDef);
   verifyFormat("auto lambda = [] () { return 0; };", SpaceFuncDef);
   verifyFormat("int x = int(y);", SpaceFuncDef);
@@ -14349,6 +14349,7 @@
   verifyFormat("int f() throw (Deprecated);", SomeSpace2);
   verifyFormat("typedef void (*cb) (int);", SomeSpace2);
   verifyFormat("T A::operator()();", SomeSpace2);
+  // FIXME these tests regressed behaviour.
   // verifyFormat("X A::operator++ (T);", SomeSpace2);
   verifyFormat("int x = int (y);", SomeSpace2);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace2);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114725: [clang-format] add back tests which didn't cause a regression

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added a reviewer: crayroud.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay requested review of this revision.

bring back the valid tests removed as part of D114696: [clang-format] regressed 
default behavior for operator parentheses 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114725

Files:
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14180,8 +14180,7 @@
   verifyFormat("int f() throw (Deprecated);", SomeSpace);
   verifyFormat("typedef void (*cb) (int);", SomeSpace);
   verifyFormat("T A::operator()();", SomeSpace);
-  // FIXME these tests regressed behaviour.
-  // verifyFormat("X A::operator++ (T);", SomeSpace);
+  verifyFormat("X A::operator++ (T);", SomeSpace);
   verifyFormat("int x = int (y);", SomeSpace);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 
@@ -14274,6 +14273,7 @@
   verifyFormat("typedef void (*cb)(int);", SpaceFuncDef);
   verifyFormat("T A::operator()();", SpaceFuncDef);
   verifyFormat("X A::operator++(T);", SpaceFuncDef);
+  // FIXME these tests regressed behaviour.
   // verifyFormat("T A::operator() () {}", SpaceFuncDef);
   verifyFormat("auto lambda = [] () { return 0; };", SpaceFuncDef);
   verifyFormat("int x = int(y);", SpaceFuncDef);
@@ -14349,6 +14349,7 @@
   verifyFormat("int f() throw (Deprecated);", SomeSpace2);
   verifyFormat("typedef void (*cb) (int);", SomeSpace2);
   verifyFormat("T A::operator()();", SomeSpace2);
+  // FIXME these tests regressed behaviour.
   // verifyFormat("X A::operator++ (T);", SomeSpace2);
   verifyFormat("int x = int (y);", SomeSpace2);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace2);


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14180,8 +14180,7 @@
   verifyFormat("int f() throw (Deprecated);", SomeSpace);
   verifyFormat("typedef void (*cb) (int);", SomeSpace);
   verifyFormat("T A::operator()();", SomeSpace);
-  // FIXME these tests regressed behaviour.
-  // verifyFormat("X A::operator++ (T);", SomeSpace);
+  verifyFormat("X A::operator++ (T);", SomeSpace);
   verifyFormat("int x = int (y);", SomeSpace);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
 
@@ -14274,6 +14273,7 @@
   verifyFormat("typedef void (*cb)(int);", SpaceFuncDef);
   verifyFormat("T A::operator()();", SpaceFuncDef);
   verifyFormat("X A::operator++(T);", SpaceFuncDef);
+  // FIXME these tests regressed behaviour.
   // verifyFormat("T A::operator() () {}", SpaceFuncDef);
   verifyFormat("auto lambda = [] () { return 0; };", SpaceFuncDef);
   verifyFormat("int x = int(y);", SpaceFuncDef);
@@ -14349,6 +14349,7 @@
   verifyFormat("int f() throw (Deprecated);", SomeSpace2);
   verifyFormat("typedef void (*cb) (int);", SomeSpace2);
   verifyFormat("T A::operator()();", SomeSpace2);
+  // FIXME these tests regressed behaviour.
   // verifyFormat("X A::operator++ (T);", SomeSpace2);
   verifyFormat("int x = int (y);", SomeSpace2);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace2);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114724: [clangd][StdSymbolMap] Prefer std::remove from algorithm

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

std::remove from algorithm is a lot more common than the overload from
the cstdio (which deletes files). This patch introduces a set of symbols
for which we should prefer the overloaded versions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114724

Files:
  clang-tools-extra/clangd/CSymbolMap.inc
  clang-tools-extra/clangd/StdSymbolMap.inc
  clang-tools-extra/clangd/include-mapping/cppreference_parser.py


Index: clang-tools-extra/clangd/include-mapping/cppreference_parser.py
===
--- clang-tools-extra/clangd/include-mapping/cppreference_parser.py
+++ clang-tools-extra/clangd/include-mapping/cppreference_parser.py
@@ -116,7 +116,7 @@
 return _ParseSymbolPage(f.read(), name)
 
 
-def _GetSymbols(pool, root_dir, index_page_name, namespace):
+def _GetSymbols(pool, root_dir, index_page_name, namespace, 
variants_to_accept):
   """Get all symbols listed in the index page. All symbols should be in the
   given namespace.
 
@@ -135,7 +135,7 @@
 for symbol_name, symbol_page_path, variant in _ParseIndexPage(f.read()):
   # Variant symbols (e.g. the std::locale version of isalpha) add 
ambiguity.
   # FIXME: use these as a fallback rather than ignoring entirely.
-  if variant:
+  if variant != (namespace+symbol_name in variants_to_accept):
 continue
   path = os.path.join(root_dir, symbol_page_path)
   results.append((symbol_name,
@@ -158,6 +158,10 @@
   Args:
 parse_pages: a list of tuples (page_root_dir, index_page_name, namespace)
   """
+  # By default we prefer the non-variant versions, as they're more common. But
+  # there are some symbols, whose variant is more common. This list describes
+  # those symbols.
+  variants_to_accept = set(["std::remove"])
   symbols = []
   # Run many workers to process individual symbol pages under the symbol index.
   # Don't allow workers to capture Ctrl-C.
@@ -165,7 +169,8 @@
   initializer=lambda: signal.signal(signal.SIGINT, signal.SIG_IGN))
   try:
 for root_dir, page_name, namespace in parse_pages:
-  symbols.extend(_GetSymbols(pool, root_dir, page_name, namespace))
+  symbols.extend(_GetSymbols(pool, root_dir, page_name, namespace,
+ variants_to_accept))
   finally:
 pool.terminate()
 pool.join()
Index: clang-tools-extra/clangd/StdSymbolMap.inc
===
--- clang-tools-extra/clangd/StdSymbolMap.inc
+++ clang-tools-extra/clangd/StdSymbolMap.inc
@@ -955,7 +955,7 @@
 SYMBOL(regex_traits, std::, )
 SYMBOL(reinterpret_pointer_cast, std::, )
 SYMBOL(remainder, std::, )
-SYMBOL(remove, std::, )
+SYMBOL(remove, std::, )
 SYMBOL(remove_all_extents, std::, )
 SYMBOL(remove_all_extents_t, std::, )
 SYMBOL(remove_const, std::, )
Index: clang-tools-extra/clangd/CSymbolMap.inc
===
--- clang-tools-extra/clangd/CSymbolMap.inc
+++ clang-tools-extra/clangd/CSymbolMap.inc
@@ -698,7 +698,6 @@
 SYMBOL(remainder, None, )
 SYMBOL(remainderf, None, )
 SYMBOL(remainderl, None, )
-SYMBOL(remove, None, )
 SYMBOL(remquo, None, )
 SYMBOL(remquof, None, )
 SYMBOL(remquol, None, )


Index: clang-tools-extra/clangd/include-mapping/cppreference_parser.py
===
--- clang-tools-extra/clangd/include-mapping/cppreference_parser.py
+++ clang-tools-extra/clangd/include-mapping/cppreference_parser.py
@@ -116,7 +116,7 @@
 return _ParseSymbolPage(f.read(), name)
 
 
-def _GetSymbols(pool, root_dir, index_page_name, namespace):
+def _GetSymbols(pool, root_dir, index_page_name, namespace, variants_to_accept):
   """Get all symbols listed in the index page. All symbols should be in the
   given namespace.
 
@@ -135,7 +135,7 @@
 for symbol_name, symbol_page_path, variant in _ParseIndexPage(f.read()):
   # Variant symbols (e.g. the std::locale version of isalpha) add ambiguity.
   # FIXME: use these as a fallback rather than ignoring entirely.
-  if variant:
+  if variant != (namespace+symbol_name in variants_to_accept):
 continue
   path = os.path.join(root_dir, symbol_page_path)
   results.append((symbol_name,
@@ -158,6 +158,10 @@
   Args:
 parse_pages: a list of tuples (page_root_dir, index_page_name, namespace)
   """
+  # By default we prefer the non-variant versions, as they're more common. But
+  # there are some symbols, whose variant is more common. This list describes
+  # those symbols.
+  variants_to_accept = set(["std::remove"])
   symbols = []
   # Run many workers to process individual symbol pages under the symbol index.
   # Don'

[PATCH] D114553: [HIP] Add atomic load, atomic store and atomic cmpxchng_weak builtin support in HIP-clang

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114553

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

[PATCH] D114723: [clangd] Make std symbol generation script python3 friendly

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114723

Files:
  clang-tools-extra/clangd/include-mapping/gen_std.py


Index: clang-tools-extra/clangd/include-mapping/gen_std.py
===
--- clang-tools-extra/clangd/include-mapping/gen_std.py
+++ clang-tools-extra/clangd/include-mapping/gen_std.py
@@ -99,12 +99,12 @@
   index_page_path = os.path.join(page_root, "index.html")
   cppreference_modified_date = datetime.datetime.fromtimestamp(
 os.stat(index_page_path).st_mtime).strftime('%Y-%m-%d')
-  print CODE_PREFIX % (args.language.upper(), cppreference_modified_date)
+  print(CODE_PREFIX % (args.language.upper(), cppreference_modified_date))
   for symbol in symbols:
 if len(symbol.headers) == 1:
   # SYMBOL(unqualified_name, namespace, header)
-  print "SYMBOL(%s, %s, %s)" % (symbol.name, symbol.namespace,
-symbol.headers[0])
+  print("SYMBOL(%s, %s, %s)" % (symbol.name, symbol.namespace,
+symbol.headers[0]))
 elif len(symbol.headers) == 0:
   sys.stderr.write("No header found for symbol %s\n" % symbol.name)
 else:


Index: clang-tools-extra/clangd/include-mapping/gen_std.py
===
--- clang-tools-extra/clangd/include-mapping/gen_std.py
+++ clang-tools-extra/clangd/include-mapping/gen_std.py
@@ -99,12 +99,12 @@
   index_page_path = os.path.join(page_root, "index.html")
   cppreference_modified_date = datetime.datetime.fromtimestamp(
 os.stat(index_page_path).st_mtime).strftime('%Y-%m-%d')
-  print CODE_PREFIX % (args.language.upper(), cppreference_modified_date)
+  print(CODE_PREFIX % (args.language.upper(), cppreference_modified_date))
   for symbol in symbols:
 if len(symbol.headers) == 1:
   # SYMBOL(unqualified_name, namespace, header)
-  print "SYMBOL(%s, %s, %s)" % (symbol.name, symbol.namespace,
-symbol.headers[0])
+  print("SYMBOL(%s, %s, %s)" % (symbol.name, symbol.namespace,
+symbol.headers[0]))
 elif len(symbol.headers) == 0:
   sys.stderr.write("No header found for symbol %s\n" % symbol.name)
 else:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114721: [clang][dataflow] Add unit tests for PostOrderCFGView

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

Looks good, but why split the test into its own directory? I see that the 
implementation file is in clang/lib/Analysis and, in general, the lib and 
unittest directories are often flatter than the corresponding include 
directories. Maybe just put it directly into unittests/Analysis?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114721

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

[PATCH] D114639: Raise the minimum Visual Studio version to VS2019

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added inline comments.



Comment at: clang/docs/UsersManual.rst:3546
 
-cmake -G"Visual Studio 15 2017" -T LLVM ..
+cmake -G"Visual Studio 17 2022" -T LLVM ..
 

jhenderson wrote:
> RKSimon wrote:
> > aaron.ballman wrote:
> > > jhenderson wrote:
> > > > I think the missing space should be fixed to :)
> > > +1 to the missing space.
> > This one is confusing - it isn't in my local diff, the raw diff, or if I 
> > reapply the raw diff - it looks to have just appeared when you quote it in 
> > phab comments?
> The fact that the space is missing? It's missing in the current repo too.
It works with and without the space, like `-GNinja` and `-G Ninja` both work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114639

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

[PATCH] D114651: [clang-cl] Expose -Wall to clang-cl by unaliasing -Wall, keeping /Wall as alias to -Weverything

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I will add that multiple users have run into this problem, and I think it might 
be more practical to consider unaliasing Wall altogether. Clang doesn't emit 
the same set of warnings as MSVC. Anyone seriously using `clang-cl /Wall` is 
going to receive a pile of `-WcxxNN-compat` warnings that are 
self-contradictory, and they are going to need to curate a set of 
clang-specific warning flags anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114651

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

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

[David Rector via Phabricator via cfe-commits]

davrec added a comment.

A couple thoughts/cases to consider...




Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:52
 
 static bool areEquivalentNameSpecifier(const NestedNameSpecifier *Left,
const NestedNameSpecifier *Right) {

This function should probably either be made into a lambda private to 
`areEquivalentDeclRefs()`, or renamed to make clear it does not apply 
generically to all NNS's.  

Part of the reason is that this is only implemented for type NNS's.  But the 
more important reason is that, when either a) `!Left || !Right`, or b) 
`!Left->getAsType() || !Right->getAsType()`, this returns true, since 
(presumably*) this gives us the desired behavior within 
areEquivalentDeclRefs(), despite that in general a null NNS should probably not 
be considered the same as a nonnull NNS.  

(*Is this indeed the desired behavior? Probably should add some tests that 
check qualified DeclRefExprs against unqualified DeclRefExprs, to be sure.)



Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:53
 static bool areEquivalentNameSpecifier(const NestedNameSpecifier *Left,
const NestedNameSpecifier *Right) {
+  const Type *LTy = Left->getAsType();

Suggest you move the null check from `areEquivalentDeclRefs()` here, i.e.
```
if (!Left || !Right)
  return true;
```
mainly since this is needs to be done in recursive calls (see below), but also 
since you do the same logic on LTy and RTy subsequently.





Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:59
+   RTy->getCanonicalTypeUnqualified();
+
+  // FIXME: We should probably check the other kinds of nested name specifiers.

It occurs to me this needs to be recursive, to check a sequence of qualifiers, 
i.e.
```
  const Type *LTy = Left->getAsType();
  const Type *RTy = Right->getAsType();
  if (!LTy || !RTy) 
return true;
  if (LTy->getCanonicalTypeUnqualified() !=
   RTy->getCanonicalTypeUnqualified())
return false;
  return areEquivalentNameSpecifier(Left->getPrefix(), Right->getPrefix());
```

The reason is, if there is a prefix qualifier to this qualifier, we run into 
the original problem, e.g.:
```
struct Base {
  struct Inner { 
static const bool value = true;
  };
};
struct Outer1 : Base {};
struct Outer2 : Base {};

// We do not want the following to warn, but without checking the prefix of 
Inner, 
// I believe these will be interpreted as equivalent DeclRefs and will warn:
auto sink = Outer1::Inner::value || Outer2::Inner::value;
```




Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:72
+  // The decls were already matched, so just return true at any later point.
+  if (!Left->hasQualifier() || !Right->hasQualifier())
+return true;

Suggest you move this null check into areEquivalentNameSpecifier, see above


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

https://reviews.llvm.org/D114622

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

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

[Vaibhav Yenamandra via Phabricator via cfe-commits]

vaibhav.y added inline comments.



Comment at: clang/include/clang/Basic/Sarif.h:80-82
+  static SarifArtifactLocation create(StringRef URI) {
+return SarifArtifactLocation{URI};
+  }

aaron.ballman wrote:
> One thing that's worth calling out is that `StringRef` is non-owning which 
> means that the argument passed to create the `SarifArtifactLocation` has to 
> outlive the returned object to avoid dangling references. This makes the 
> class a bit more dangerous to use because `Twine` or automatic `std::string` 
> objects may cause lifetime concerns.
> 
> Should these classes be storing a `std::string` so that the memory is owned 
> by SARIF?
Good point! Will change it to `std::string` to start with.

Some fields such as MimeType, RuleID would probably do better with 
`SmallString`, I haven't been able to find any good measurements on how the 
lengths of those two are distributed, can it be made into `SmallString` in a 
future PR?



Comment at: clang/include/clang/Basic/Sarif.h:116
+  static SarifArtifact create(const SarifArtifactLocation &Loc) {
+return SarifArtifactLocation{Loc};
+  }

aaron.ballman wrote:
> `Loc` is already a `SarifArtifactLocation`, did you mean `SarifArtifact` by 
> any chance?
> 
> (Note, this suggests to me we should mark the ctor's here as `explicit`.)
Agree, I've marked all constructors taking a single parameter as explicit.



Comment at: clang/include/clang/Basic/Sarif.h:243-245
+  // While this cannot be negative, since this type needs to be serialized
+  // to JSON, it needs to be `int64_t`. The best we can do is assert that
+  // a negative value isn't used to create it

aaron.ballman wrote:
> This comment looks incorrect to me.
Ack, fixed that as well. the right rationale for `uint32_t` is that it is the 
largest non-negative type that can be safely promoted to `int64_t` (which is 
what LLVM's json encoder supports)



Comment at: clang/include/clang/Basic/SourceLocation.h:441-456
+bool operator()(const FullSourceLoc &lhs, const FullSourceLoc &rhs) const {
   return lhs.isBeforeInTranslationUnitThan(rhs);
 }
   };
 
   /// Prints information about this FullSourceLoc to stderr.
   ///

aaron.ballman wrote:
> Looks like unrelated formatting changes; feel free to submit these as an NFC 
> change if you'd like, but the changes should be reverted from this patch for 
> clarity.
Ack, these are definitely a result of a bad rebase. 



Comment at: clang/lib/Basic/Sarif.cpp:199
+const SarifArtifactLocation &Location =
+SarifArtifactLocation::create(FileURI).setIndex(Idx);
+const SarifArtifact &Artifact = SarifArtifact::create(Location)

aaron.ballman wrote:
> This seems like it'll be a use-after-free because the local `std::string` 
> will be destroyed before the lifetime of the `SarifArtifactLocation` ends.
Will run it through a pass of asan & msan, is the best way to add: 
`-fsanitize=memory -fsanitize=address` to the test CMakeLists.txt & run them?

I've changed all strings to `std::string`, so this one should no longer be a 
problem but I wonder if there's any others I have missed as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109701

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

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

[Vaibhav Yenamandra via Phabricator via cfe-commits]

vaibhav.y updated this revision to Diff 390396.
vaibhav.y marked 6 inline comments as done.
vaibhav.y added a comment.

Rebase on upstream/main:

- [clangBasic] Format code
- [clangBasic] Mark all constructors taking single values `explicit`
- [clangBasic] Convert `StringRef` to `std::string` fields that own the data
- [clangBasic] Fixup outdated comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109701

Files:
  clang/include/clang/Basic/Sarif.h
  clang/include/clang/Basic/SourceLocation.h
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/Sarif.cpp
  clang/lib/Basic/SourceLocation.cpp
  clang/unittests/Basic/CMakeLists.txt
  clang/unittests/Basic/SarifTest.cpp

Index: clang/unittests/Basic/SarifTest.cpp
===
--- /dev/null
+++ clang/unittests/Basic/SarifTest.cpp
@@ -0,0 +1,138 @@
+//===- unittests/Basic/SarifTest.cpp - Test writing SARIF documents ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Basic/Sarif.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/JSON.h"
+#include 
+
+#include "gmock/gmock.h"
+#include "gtest/gtest-death-test.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace llvm;
+
+namespace {
+
+TEST(SarifDocumentWriterTest, createEmptyDocument) {
+  // GIVEN:
+  SarifDocumentWriter writer;
+
+  // WHEN:
+  const json::Object &emptyDocument = writer.createDocument();
+  std::vector keys(emptyDocument.size());
+  std::transform(emptyDocument.begin(), emptyDocument.end(), keys.begin(),
+ [](auto item) { return item.getFirst(); });
+
+  // THEN:
+  ASSERT_THAT(keys, testing::UnorderedElementsAre("$schema", "version"));
+}
+
+// Test that a newly inserted run will associate correct tool names
+TEST(SarifDocumentWriterTest, documentWithARun) {
+  // GIVEN:
+  SarifDocumentWriter writer;
+  const char *shortName = "sariftest";
+  const char *longName = "sarif writer test";
+
+  // WHEN:
+  writer.createRun(shortName, longName);
+  writer.endRun();
+  const json::Object &document = writer.createDocument();
+  const json::Array *runs = document.getArray("runs");
+
+  // THEN:
+  // A run was created
+  ASSERT_THAT(runs, testing::NotNull());
+
+  // It is the only run
+  ASSERT_EQ(runs->size(), 1UL);
+
+  // The tool associated with the run was the tool
+  const json::Object *driver =
+  runs->begin()->getAsObject()->getObject("tool")->getObject("driver");
+  ASSERT_THAT(driver, testing::NotNull());
+
+  ASSERT_TRUE(driver->getString("name").hasValue());
+  ASSERT_TRUE(driver->getString("fullName").hasValue());
+  ASSERT_TRUE(driver->getString("language").hasValue());
+
+  EXPECT_EQ(driver->getString("name").getValue(), shortName);
+  EXPECT_EQ(driver->getString("fullName").getValue(), longName);
+  EXPECT_EQ(driver->getString("language").getValue(), "en-US");
+}
+
+// Test adding result without a run causes a crash
+TEST(SarifDocumentWriterTest, addingResultsWillCrashIfThereIsNoRun) {
+  // GIVEN:
+  SarifDocumentWriter writer;
+
+  // WHEN:
+  //  A SarifDocumentWriter::createRun(...) was not called prior to
+  //  SarifDocumentWriter::appendResult(...)
+  // But a rule exists
+  auto ruleIdx = writer.createRule(SarifRule::create());
+  SarifResult &&emptyResult = SarifResult::create(ruleIdx);
+
+  // THEN:
+  ASSERT_DEATH({ writer.appendResult(emptyResult); }, ".*create a run first.*");
+}
+
+// Test adding rule and result shows up in the final document
+TEST(SarifDocumentWriterTest, addResultWithValidRuleIsOk) {
+  // GIVEN:
+  SarifDocumentWriter writer;
+  const SarifRule &rule =
+  SarifRule::create()
+  .setRuleId("clang.unittest")
+  .setDescription("Example rule created during unit tests")
+  .setName("clang unit test");
+
+  // WHEN:
+  writer.createRun("sarif test", "sarif test runner");
+  unsigned ruleIdx = writer.createRule(rule);
+  const SarifResult &result = SarifResult::create(ruleIdx);
+
+  writer.appendResult(result);
+  const json::Object &document = writer.createDocument();
+
+  // THEN:
+  // A document with a valid schema and version exists
+  ASSERT_THAT(document.get("$schema"), ::testing::NotNull());
+  ASSERT_THAT(document.get("version"), ::testing::NotNull());
+  const json::Array *runs = document.getArray("runs");
+
+  // A run exists on this document
+  ASSERT_THAT(runs, ::testing::NotNull());
+  ASSERT_EQ(runs->size(), 1UL);
+  const json::Object *theRun = runs->back().getAsObject();
+
+  // The run has slots for tools, results, rules and artifacts
+  ASSERT_THAT(theRun->get("tool"), ::testing::NotNull());
+  ASSERT_THAT(theRun->get("results"), ::testing::NotNull());

[PATCH] D114721: [clang][dataflow] Add unit tests for PostOrderCFGView

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev created this revision.
sgatev added reviewers: ymandel, xazax.hun, gribozavr2.
Herald added subscribers: rnkovacs, mgorny.
sgatev requested review of this revision.
Herald added a project: clang.

This adds unit tests for the PostOrderCFGView class which will be
used as part of the implementation of the dataflow analysis framework.
See "[RFC] A dataflow analysis framework for Clang AST" on cfe-dev.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114721

Files:
  clang/unittests/Analysis/Analyses/CMakeLists.txt
  clang/unittests/Analysis/Analyses/PostOrderCFGViewTest.cpp
  clang/unittests/Analysis/CMakeLists.txt

Index: clang/unittests/Analysis/CMakeLists.txt
===
--- clang/unittests/Analysis/CMakeLists.txt
+++ clang/unittests/Analysis/CMakeLists.txt
@@ -28,3 +28,5 @@
   PRIVATE
   LLVMTestingSupport
   )
+
+add_subdirectory(Analyses)
Index: clang/unittests/Analysis/Analyses/PostOrderCFGViewTest.cpp
===
--- /dev/null
+++ clang/unittests/Analysis/Analyses/PostOrderCFGViewTest.cpp
@@ -0,0 +1,384 @@
+//===- unittests/Analysis/Analyses/PostOrderCFGViewTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Analysis/Analyses/PostOrderCFGView.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace analysis {
+namespace analyses {
+namespace {
+
+using ::testing::ElementsAre;
+using ::testing::NotNull;
+
+class PostOrderBlockIDMatchCallback
+: public ast_matchers::MatchFinder::MatchCallback {
+public:
+  const std::vector &getPostOrderBlockIDs() const {
+return PostOrderBlockIDs;
+  }
+
+  void run(const ast_matchers::MatchFinder::MatchResult &Result) override {
+const auto *Func = Result.Nodes.getNodeAs("func");
+ASSERT_THAT(Func, NotNull());
+
+Stmt *Body = Func->getBody();
+ASSERT_THAT(Body, NotNull());
+
+CFG::BuildOptions Options;
+Options.AddImplicitDtors = true;
+Options.AddTemporaryDtors = true;
+Options.setAllAlwaysAdd();
+
+std::unique_ptr Cfg =
+CFG::buildCFG(nullptr, Body, Result.Context, Options);
+ASSERT_THAT(Cfg, NotNull());
+
+PostOrderCFGView POV(Cfg.get());
+std::priority_queue,
+PostOrderCFGView::BlockOrderCompare>
+BlocksQueue(POV.getComparator());
+for (const CFGBlock *Block : *Cfg)
+  BlocksQueue.push(Block);
+
+while (!BlocksQueue.empty()) {
+  PostOrderBlockIDs.push_back(BlocksQueue.top()->getBlockID());
+  BlocksQueue.pop();
+}
+  }
+
+private:
+  std::vector PostOrderBlockIDs;
+};
+
+template 
+void expectPostOrderBlockIDs(const char *Code, T MatchesPostOrderBlockIDs) {
+  std::unique_ptr AST =
+  tooling::buildASTFromCodeWithArgs(Code, {"-std=c++11"});
+  ASSERT_THAT(AST, NotNull());
+
+  PostOrderBlockIDMatchCallback Callback;
+  ast_matchers::MatchFinder Finder;
+  Finder.addMatcher(
+  ast_matchers::functionDecl(ast_matchers::hasName("target")).bind("func"),
+  &Callback);
+  Finder.matchAST(AST->getASTContext());
+
+  EXPECT_THAT(Callback.getPostOrderBlockIDs(), MatchesPostOrderBlockIDs);
+}
+
+TEST(PostOrderCFGViewTest, EmptyFunc) {
+  expectPostOrderBlockIDs(R"(
+// B1 (succs: B0)
+void target() {
+}
+// B0
+  )",
+  ElementsAre(0, 1));
+}
+
+TEST(PostOrderCFGViewTest, IfStmt) {
+  expectPostOrderBlockIDs(R"(
+bool foo();
+
+// B5 (succs: B4)
+void target() {
+  // B4 (succs: B3, B2)
+  foo();
+  if (foo()) {
+// B3 (succs: B1)
+foo();
+  } else {
+// B2 (succs: B1)
+foo();
+  }
+  // B1 (succs: B0)
+  foo();
+}
+// B0
+  )",
+  ElementsAre(0, 1, 3, 2, 4, 5));
+}
+
+TEST(PostOrderCFGViewTest, IfStmtLogicalAnd) {
+  expectPostOrderBlockIDs(R"(
+bool foo();
+
+// B6 (succs: B5)
+void target() {
+  // B5 (succs: B4, B2)
+  foo();
+  if (foo() && /*B4 (succs: B3, B2)*/ foo()) {
+// B3 (succs: B1)
+foo();
+  } else {
+// B2 (succs: B1)
+foo();
+  }
+  // B1 (succs: B0)
+  foo();
+}
+// B0
+  )",
+  ElementsAre(0, 1, 3, 2, 4, 5, 6));
+}
+
+TEST(PostOrderCFGViewTest, IfStmtLogicalOr) {
+  expectPostOrderBlockIDs(R"(
+bool foo();
+
+// B6 (succs: B5)
+void target() {
+  // B5 (succs: B3, B4)
+  foo();
+  if (f

[PATCH] D114427: [clang-tidy] Warn on functional C-style casts

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp updated this revision to Diff 390394.
carlosgalvezp added a comment.

Move logic into single functions.


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

https://reviews.llvm.org/D114427

Files:
  clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp
@@ -143,11 +143,10 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant cast to the same type
   // CHECK-FIXES: {{^}}  kZero;
 
-  int b2 = int(b);
-  int b3 = static_cast(b);
-  int b4 = b;
+  int b2 = static_cast(b);
+  int b3 = b;
   double aa = a;
-  (void)b2;
+  (void)aa;
   return (void)g();
 }
 
@@ -321,3 +320,50 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: C-style casts are discouraged; use constructor call syntax [
   // CHECK-FIXES: auto s6 = S(cr);
 }
+
+template 
+T functional_cast_template_used_by_class(float i) {
+  return T(i);
+}
+
+template 
+T functional_cast_template_used_by_int(float i) {
+  return T(i);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: C-style casts are discouraged; use static_cast
+  // CHECK-FIXES: return static_cast(i);
+}
+
+struct S2 {
+  S2(float);
+};
+using T = S2;
+using U = S2 &;
+
+void functional_casts() {
+  float x = 1.5F;
+  auto y = int(x);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: C-style casts are discouraged; use static_cast
+  // CHECK-FIXES: auto y = static_cast(x);
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wc++11-narrowing"
+  // This if fine, compiler will warn about implicit conversions with brace initialization
+  auto z = int{x};
+#pragma clang diagnostic pop
+
+  // Functional casts are allowed if S is of class type
+  const char *str = "foo";
+  auto s = S(str);
+
+  // Functional casts in template functions
+  functional_cast_template_used_by_class(x);
+  functional_cast_template_used_by_int(x);
+
+  // New expressions are not functional casts
+  auto w = new int(x);
+
+  // Typedefs
+  S2 t = T(x); // OK, constructor call
+  S2 u = U(x); // NOK, it's a reinterpret_cast in disguise
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: C-style casts are discouraged; use static_cast/const_cast/reinterpret_cast
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -133,9 +133,11 @@
 Changes in existing checks
 ^^
 
-- Removed default setting `cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"`,
+- Removed default setting ``cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"``,
   to match the current state of the C++ Core Guidelines.
 
+- Updated ``google-readability-casting`` to diagnose and fix functional casts, to achieve feature
+  parity with the corresponding ``cpplint.py`` check.
 
 Removed checks
 ^^
Index: clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
+++ clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
@@ -31,6 +31,11 @@
   unless(isInTemplateInstantiation()))
   .bind("cast"),
   this);
+  Finder->addMatcher(
+  cxxFunctionalCastExpr(unless(hasDescendant(cxxConstructExpr())),
+unless(hasDescendant(initListExpr(
+  .bind("cast"),
+  this);
 }
 
 static bool needsConstCast(QualType SourceType, QualType DestType) {
@@ -55,8 +60,39 @@
   return T1.getUnqualifiedType() == T2.getUnqualifiedType();
 }
 
+static clang::CharSourceRange getReplaceRange(const ExplicitCastExpr *Expr) {
+  if (const auto *CastExpr = dyn_cast(Expr)) {
+return CharSourceRange::getCharRange(
+CastExpr->getLParenLoc(),
+CastExpr->getSubExprAsWritten()->getBeginLoc());
+  } else if (const auto *CastExpr = dyn_cast(Expr)) {
+return CharSourceRange::getCharRange(CastExpr->getBeginLoc(),
+ CastExpr->getLParenLoc());
+  } else
+llvm_unreachable("Unsupported CastExpr");
+}
+
+static StringRef getDestTypeString(const SourceManager &SM,
+   const LangOptions &LangOpts,
+   const ExplicitCastExpr *Expr) {
+  SourceLocation BeginLoc;
+  SourceLocation EndLoc;
+
+  if (const auto *CastExpr = dyn_cast(Expr)) {
+BeginLoc = CastExpr->getLParenLoc().getLocWithOffset(1);
+EndLoc = CastExpr->getRParenLoc().getLocWithOffset(-1);
+  } else if (const aut

[PATCH] D114718: [analyzer] Implement a new checker for Strict Aliasing Rule.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@rsmith, @aaron.ballman I kindly invite you to join the review process 
especially in a part of the Standard interpretation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114718

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

[PATCH] D114718: [analyzer] Implement a new checker for Strict Aliasing Rule.

[Denys Petrov via Phabricator via cfe-commits]

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

`StrictAliasingChecker` implements checks on violation of the next paragraph of 
the Standard which is known as **Strict Aliasing Rule**. It operates on 
variable loads and stores. The checker compares the original type of the value 
with the type of the pointer with which the value is aliased.
The paragraph:

> C++20 7.2.1 p11 [basic.lval]:
>  If a program attempts to access the stored value of an object through a 
> glvalue whose type is not similar to one of the following types the behavior 
> is undefined:
>
> - the dynamic type of the object,
> - a type that is the signed or unsigned type corresponding to the dynamic 
> type of the object, or
> - a char, unsigned char, or std​::​byte type.

Example:

  int x = 42;
  auto c = *((char*)&x); // The original type is `int`. The aliased type is 
`char`. OK. 
  auto f = *((float*)&x); // The original type is `int`. The aliased type is 
`float`. UB. 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114718

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/StrictAliasingChecker.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/strict-aliasing.cpp

Index: clang/test/Analysis/Checkers/StrictAliasingChecker/strict-aliasing.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/StrictAliasingChecker/strict-aliasing.cpp
@@ -0,0 +1,160 @@
+// RUN: %clang_cc1 -analyze -analyzer-config eagerly-assume=false -analyzer-checker=debug.ExprInspection,alpha.core.StrictAliasing -verify %s
+
+template 
+void clang_analyzer_dump(T x);
+void clang_analyzer_eval(int);
+
+namespace std {
+enum class byte : unsigned char {};
+enum class otherByte : unsigned char {};
+}; // namespace std
+enum class intEnum : int {};
+
+class Class {};
+class ClassInt {
+  int x;
+};
+
+using AliasedStdByte = std::byte;
+using AliasedChar = char;
+using AliasedSChar = signed char;
+using AliasedInt = int;
+using AliasedUInt = unsigned int;
+using AliasedULong = unsigned long;
+
+namespace ns1 {
+
+void var_cast() {
+  using MyInt = int;
+  MyInt x = {};
+  {
+auto *ptr = (std::byte *)&x;
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (std::otherByte *)&x;
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (intEnum *)&x;
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (Class *)&x;
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (ClassInt *)&x;
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (char *)&x;
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (unsigned char *)&x;
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (const char *)&x;
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (const unsigned char *)&x;
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (signed char *)&x;
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (short *)&x;
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (unsigned short *)&x;
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (signed short *)&x;
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (int *)&x;
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (unsigned int *)&x;
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (signed int *)&x;
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (long *)&x;
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (unsigned long *)&x;
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (signed long *)&x;
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (long long *)&x;
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (unsigned long long *)&x;
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (signed long long *)&x;
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (float *)&x;
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (double *)&x;
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (long double *)&x;
+

[PATCH] D114427: [clang-tidy] Warn on functional C-style casts

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp:63-79
+static clang::CharSourceRange getReplaceRange(const CStyleCastExpr *CastExpr) {
+  return CharSourceRange::getCharRange(
+  CastExpr->getLParenLoc(), 
CastExpr->getSubExprAsWritten()->getBeginLoc());
+}
+
+static clang::CharSourceRange
+getReplaceRange(const CXXFunctionalCastExpr *CastExpr) {

Ditto here:
```
static clang::CharSourceRange
getReplaceRange(const ExplicitCastExpr *Expr) {
  if (const auto *CastExpr = dyn_cast(Expr)) {
return CharSourceRange::getCharRange(
CastExpr->getLParenLoc(), 
CastExpr->getSubExprAsWritten()->getBeginLoc());
  } else if (const auto *CastExpr = dyn_cast(Expr)) {
return CharSourceRange::getCharRange(CastExpr->getBeginLoc(),
 CastExpr->getLParenLoc());
  } else {
// however Clang spells "unreachable"
  }
}
```
Besides saving some reader-brain-cells, this also makes it clearer that there's 
a potential null dereference in the old code (if both `dyn_cast`s fail) that 
we're hoping is unreachable. This way, we have a place we can annotate that 
with `assert(false)` or whatever, instead of just dereferencing null.


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

https://reviews.llvm.org/D114427

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

[PATCH] D114639: Raise the minimum Visual Studio version to VS2019

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D114639#3158401 , @RKSimon wrote:

>> Have you checked whether there are any bots in the lab that will need to be 
>> updated?
>
> I did find a number of bots that I think need addressing, I am intending to 
> privately email the owners but I haven't done that yet - @rnk maintains the 
> sanitizer-windows and windows-gcebot2 machines, but there are others.

Thanks! I think we'll need the bot situation sorted out before we land this, 
otherwise we'll have bots red and failing to report newly introduced 
regressions from other patches. (Hopefully we'll replace the bots with ones 
that test newer VS configurations rather than lose the coverage entirely.)




Comment at: llvm/cmake/modules/CheckCompilerVersion.cmake:17-18
+# _MSC_VER == 1929 MSVC++ 14.29 Visual Studio 2019 Version 16.10 & 16.11
+set(MSVC_MIN 19.20)
+set(MSVC_SOFT_ERROR 19.29)
 

RKSimon wrote:
> aaron.ballman wrote:
> > I'm a bit less certain that this is reasonable. 16.11.0 is from August 
> > 2021, which seems incredibly new for giving soft errors on.
> Our docs do say the latest update of the compiler, but I take your point.
> 
> 16.10 (same _MSC_VER) was released in May 2021
> https://docs.microsoft.com/en-us/visualstudio/install/visual-studio-build-numbers-and-release-dates?view=vs-2019
> 
> If we want something a year old, 16.8 + 16.9 (1928) is about right.
16.7 (1927) would also be quite reasonable (Aug 2020). I don't have a strong 
preference for 16.7 vs 16.8, so I took a look at the release notes. 16.8 added 
support for newer C++ features, but not ones we care about consuming in our 
code base (coroutines, concepts, , some STL improvements, etc), so I 
think we'd be fine with 16.7 if we wanted to be more conservative on the window 
for the soft error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114639

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

[PATCH] D114427: [clang-tidy] Warn on functional C-style casts

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp:101-110
+static StringRef getDestTypeString(const SourceManager &SM,
+   const LangOptions &LangOpts,
+   const ExplicitCastExpr *CastExpr) {
+  return isa(CastExpr)
+ ? getDestTypeString(SM, LangOpts,
+ dyn_cast(CastExpr))
+ : getDestTypeString(

I actually meant to move the conditional //inside// the body, like this. ^
Now there's only one function, and the reader doesn't have to do mental 
overload resolution to figure out what the caller's behavior is intended to be.




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp:318
   // FIXME: This should be a static_cast.
   // C HECK-FIXES: auto s5 = static_cast(cr);
   auto s6 = (S)cr;

Pre-existing: This looks accidental.


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

https://reviews.llvm.org/D114427

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

[PATCH] D114619: [Analyzer][solver] Do not remove the simplified symbol from the eq class

[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 think it looks great.




Comment at: clang/test/Analysis/symbol-simplification-disequality-info.cpp:15-26
+  // CHECK:  "disequality_info": [
+  // CHECK-NEXT:   {
+  // CHECK-NEXT: "class": [ "((reg_$0) + (reg_$1)) + 
(reg_$2)" ],
+  // CHECK-NEXT: "disequal_to": [
+  // CHECK-NEXT:   [ "reg_$3" ]]
+  // CHECK-NEXT:   },
+  // CHECK-NEXT:   {

martong wrote:
> steakhal wrote:
> > Please try to omit unnecessary changes.
> I've made this change intentionally, so all hunks for each simplification 
> steps are indented similarly. E.g., after the `CHECK-NEXT:` string comes 3 
> space characters until we the `{` character, see L16 and L34 and L51. 
> Actually, I've made an indentation error in a previous patch, which I though 
> I can correct now.
> 
Fine by me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114619

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