[clang-tools-extra] 8d9828e - [clang-tidy] Fix all broken links in the comment.

2023-01-03 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2023-01-03T09:25:38+01:00
New Revision: 8d9828ef5aa9688500657d36cd2aefbe12bbd162

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

LOG: [clang-tidy] Fix all broken links in the comment.

Added: 


Modified: 
clang-tools-extra/clang-tidy/altera/SingleWorkItemBarrierCheck.h
clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h
clang-tools-extra/clang-tidy/bugprone/CopyConstructorInitCheck.h

clang-tools-extra/clang-tidy/bugprone/MisplacedPointerArithmeticInAllocCheck.h
clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h
clang-tools-extra/clang-tidy/bugprone/StringIntegerAssignmentCheck.h
clang-tools-extra/clang-tidy/cert/ProperlySeededRandomGeneratorCheck.h
clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.h
clang-tools-extra/clang-tidy/fuchsia/DefaultArgumentsDeclarationsCheck.h
clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.h
clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h
clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.h
clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.h
clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h
clang-tools-extra/clang-tidy/objc/NSDateFormatterCheck.h

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/altera/SingleWorkItemBarrierCheck.h 
b/clang-tools-extra/clang-tidy/altera/SingleWorkItemBarrierCheck.h
index 28cd0e8eb712d..ea9b0a1243f35 100644
--- a/clang-tools-extra/clang-tidy/altera/SingleWorkItemBarrierCheck.h
+++ b/clang-tools-extra/clang-tidy/altera/SingleWorkItemBarrierCheck.h
@@ -20,7 +20,7 @@ namespace altera {
 /// kernels, which may be inefficient or cause an error.
 ///
 /// For the user-facing documentation see:
-/// 
http://clang.llvm.org/extra/clang-tidy/checks/opencl/single-work-item-barrier.html
+/// 
http://clang.llvm.org/extra/clang-tidy/checks/altera/single-work-item-barrier.html
 class SingleWorkItemBarrierCheck : public ClangTidyCheck {
   const unsigned AOCVersion;
 

diff  --git 
a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h 
b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h
index f49dda24c9a91..ea116c73f8362 100644
--- a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h
@@ -18,7 +18,7 @@ namespace bugprone {
 /// Catches assignments within the condition clause of an if statement.
 ///
 /// For the user-facing documentation see:
-/// 
http://clang.llvm.org/extra/clang-tidy/checks/bugprone-assignment-in-if-condition.html
+/// 
http://clang.llvm.org/extra/clang-tidy/checks/bugprone/assignment-in-if-condition.html
 class AssignmentInIfConditionCheck : public ClangTidyCheck {
 public:
   AssignmentInIfConditionCheck(StringRef Name, ClangTidyContext *Context)

diff  --git a/clang-tools-extra/clang-tidy/bugprone/CopyConstructorInitCheck.h 
b/clang-tools-extra/clang-tidy/bugprone/CopyConstructorInitCheck.h
index 02e6807fdd6af..836f6e47e8b72 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CopyConstructorInitCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/CopyConstructorInitCheck.h
@@ -19,7 +19,7 @@ namespace bugprone {
 /// the base class.
 ///
 /// For the user-facing documentation see:
-/// 
http://clang.llvm.org/extra/clang-tidy/checks/misc/copy-constructor-init.html
+/// 
http://clang.llvm.org/extra/clang-tidy/checks/bugprone/copy-constructor-init.html
 class CopyConstructorInitCheck : public ClangTidyCheck {
 public:
   CopyConstructorInitCheck(StringRef Name, ClangTidyContext *Context)

diff  --git 
a/clang-tools-extra/clang-tidy/bugprone/MisplacedPointerArithmeticInAllocCheck.h
 
b/clang-tools-extra/clang-tidy/bugprone/MisplacedPointerArithmeticInAllocCheck.h
index e2e35a1a8c382..ce0ec8b81f2f7 100644
--- 
a/clang-tools-extra/clang-tidy/bugprone/MisplacedPointerArithmeticInAllocCheck.h
+++ 
b/clang-tools-extra/clang-tidy/bugprone/MisplacedPointerArithmeticInAllocCheck.h
@@ -19,7 +19,7 @@ namespace bugprone {
 /// memory allocation function instead of its argument.
 ///
 /// For the user-facing documentation see:
-/// 
http://clang.llvm.org/extra/clang-tidy/checks/bugprone/misplaced-operator-in-alloc.html
+/// 
http://clang.llvm.org/extra/clang-tidy/checks/bugprone/misplaced-pointer-arithmetic-in-alloc.html
 class MisplacedPointerArithmeticInAllocCheck : public ClangTidyCheck {
 public:
   MisplacedPointerArithmeticInAllocCheck(StringRef Name,

diff  --git a/clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h 
b/clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h
index 01090b42d62ad..083e917696e7f 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h
+++ b/clang-tools-extra/clang-tidy/bu

[PATCH] D140395: [clang][analyzer] Extend StreamChecker with some new functions.

2023-01-03 Thread Balázs Kéri via Phabricator via cfe-commits
balazske updated this revision to Diff 485928.
balazske marked 2 inline comments as done.
balazske added a comment.

Removed old commment, updated a test run line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140395

Files:
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  clang/test/Analysis/Inputs/system-header-simulator.h
  clang/test/Analysis/stream-errno-note.c
  clang/test/Analysis/stream-errno.c
  clang/test/Analysis/stream-error.c
  clang/test/Analysis/stream-noopen.c

Index: clang/test/Analysis/stream-noopen.c
===
--- /dev/null
+++ clang/test/Analysis/stream-noopen.c
@@ -0,0 +1,157 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.unix.Errno \
+// RUN:   -analyzer-checker=alpha.unix.Stream \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -analyzer-checker=debug.ExprInspection
+
+// enable only StdCLibraryFunctions checker
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.unix.Errno \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -analyzer-checker=debug.ExprInspection
+
+#include "Inputs/system-header-simulator.h"
+#include "Inputs/errno_var.h"
+
+void clang_analyzer_eval(int);
+
+const char *WBuf = "123456789";
+char RBuf[10];
+
+void test_freopen(FILE *F) {
+  F = freopen("xxx", "w", F);
+  if (F) {
+if (errno) {} // expected-warning{{undefined}}
+  } else {
+clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+  }
+}
+
+void test_fread(FILE *F) {
+  size_t Ret = fread(RBuf, 1, 10, F);
+  if (Ret == 10) {
+if (errno) {} // expected-warning{{undefined}}
+  } else {
+clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+  }
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void test_fwrite(FILE *F) {
+  size_t Ret = fwrite(WBuf, 1, 10, F);
+  if (Ret == 10) {
+if (errno) {} // expected-warning{{undefined}}
+  } else {
+clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+  }
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void test_fclose(FILE *F) {
+  int Ret = fclose(F);
+  if (Ret == 0) {
+if (errno) {} // expected-warning{{undefined}}
+  } else {
+clang_analyzer_eval(Ret == EOF); // expected-warning {{TRUE}}
+clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+  }
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void test_fseek(FILE *F) {
+  int Ret = fseek(F, SEEK_SET, 1);
+  if (Ret == 0) {
+if (errno) {} // expected-warning{{undefined}}
+  } else {
+clang_analyzer_eval(Ret == -1); // expected-warning {{TRUE}}
+clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+  }
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void check_fgetpos(FILE *F) {
+  errno = 0;
+  fpos_t Pos;
+  int Ret = fgetpos(F, &Pos);
+  if (Ret)
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+  else
+clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+ // expected-warning@-1{{FALSE}}
+  if (errno) {} // no-warning
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void check_fsetpos(FILE *F) {
+  errno = 0;
+  fpos_t Pos;
+  int Ret = fsetpos(F, &Pos);
+  if (Ret)
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+  else
+clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+ // expected-warning@-1{{FALSE}}
+  if (errno) {} // no-warning
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void check_ftell(FILE *F) {
+  errno = 0;
+  long Ret = ftell(F);
+  if (Ret == -1) {
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+  } else {
+clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+ // expected-warning@-1{{FALSE}}
+clang_analyzer_eval(Ret >= 0); // expected-warning{{TRUE}}
+  }
+  if (errno) {} // no-warning
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void freadwrite_zerosize(FILE *F) {
+  fwrite(WBuf, 1, 0, F);
+  clang_

[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to enable/disable ExternalNameConversionPass

2023-01-03 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski added a comment.

Hi @madanial , thanks for posting this!

> This patch adds user option -funderscoring/-fnounderscoring which behaves 
> similar to the gfortran option be enabling/disabling the 
> ExternalNameConversionPass

I don't quite understand what this option is for and it's hard to deduce from 
the patch. Please, could you add a link to some documentation? And tests.

-Andrzej


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140795

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


[PATCH] D140800: Precompute OptTable prefixes union table through tablegen

2023-01-03 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 485929.
serge-sans-paille added a comment.

Added to versions of OptTable, one that uses a precomputed Table, generated by 
TableGen, and one that relies on the old, generic approach. Both are tested 
using the same test bench.

Updated benchmark: 
https://llvm-compile-time-tracker.com/compare.php?from=4da6cb3202817ee2897d6b690e4af950459caea4&to=19a492b704e8f5c1dea120b9c0d3859bd78796be&stat=instructions:u


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

https://reviews.llvm.org/D140800

Files:
  clang/lib/Driver/DriverOptions.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
  lld/COFF/Driver.h
  lld/COFF/DriverUtils.cpp
  lld/ELF/Driver.h
  lld/ELF/DriverUtils.cpp
  lld/MachO/Driver.h
  lld/MachO/DriverUtils.cpp
  lld/MinGW/Driver.cpp
  lld/wasm/Driver.cpp
  lldb/tools/driver/Driver.cpp
  lldb/tools/lldb-server/lldb-gdbserver.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp
  llvm/include/llvm/Option/OptTable.h
  llvm/lib/ExecutionEngine/JITLink/COFFDirectiveParser.cpp
  llvm/lib/Option/OptTable.cpp
  llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/dsymutil/dsymutil.cpp
  llvm/tools/llvm-cvtres/llvm-cvtres.cpp
  llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp
  llvm/tools/llvm-dwarfutil/llvm-dwarfutil.cpp
  llvm/tools/llvm-ifs/llvm-ifs.cpp
  llvm/tools/llvm-lipo/llvm-lipo.cpp
  llvm/tools/llvm-ml/llvm-ml.cpp
  llvm/tools/llvm-mt/llvm-mt.cpp
  llvm/tools/llvm-nm/llvm-nm.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/llvm-rc/llvm-rc.cpp
  llvm/tools/llvm-readobj/llvm-readobj.cpp
  llvm/tools/llvm-size/llvm-size.cpp
  llvm/tools/llvm-strings/llvm-strings.cpp
  llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
  llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
  llvm/unittests/Option/OptionParsingTest.cpp
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -237,8 +237,14 @@
   CurPrefix = NewPrefix;
   }
 
-  // Dump prefixes.
+  DenseSet PrefixesUnionSet;
+  for (const auto &Prefix : Prefixes)
+PrefixesUnionSet.insert(Prefix.first.begin(), Prefix.first.end());
+  SmallVector PrefixesUnion(PrefixesUnionSet.begin(),
+   PrefixesUnionSet.end());
+  array_pod_sort(PrefixesUnion.begin(), PrefixesUnion.end());
 
+  // Dump prefixes.
   OS << "/\n";
   OS << "// Prefixes\n\n";
   OS << "#ifdef PREFIX\n";
@@ -259,6 +265,20 @@
   OS << "#undef COMMA\n";
   OS << "#endif // PREFIX\n\n";
 
+  // Dump prefix unions.
+  OS << "/\n";
+  OS << "// Prefix Union\n\n";
+  OS << "#ifdef PREFIX_UNION\n";
+  OS << "#define COMMA ,\n";
+  OS << "PREFIX_UNION({\n";
+  for (const auto &Prefix : PrefixesUnion) {
+OS << "llvm::StringLiteral(\"" << Prefix << "\") COMMA ";
+  }
+  OS << "llvm::StringLiteral(\"\")})\n";
+  OS << "#undef COMMA\n";
+  OS << "#endif // PREFIX_UNION\n\n";
+
+  // Dump groups.
   OS << "/\n";
   OS << "// ValuesCode\n\n";
   OS << "#ifdef OPTTABLE_VALUES_CODE\n";
Index: llvm/unittests/Option/OptionParsingTest.cpp
===
--- llvm/unittests/Option/OptionParsingTest.cpp
+++ llvm/unittests/Option/OptionParsingTest.cpp
@@ -32,6 +32,14 @@
 #include "Opts.inc"
 #undef PREFIX
 
+static constexpr const StringLiteral PrefixTable_init[] =
+#define PREFIX_UNION(VALUES) VALUES
+#include "Opts.inc"
+#undef PREFIX_UNION
+;
+static constexpr const ArrayRef
+PrefixTable(PrefixTable_init, std::size(PrefixTable_init) - 1);
+
 enum OptionFlags {
   OptFlag1 = (1 << 4),
   OptFlag2 = (1 << 5),
@@ -48,10 +56,16 @@
 };
 
 namespace {
-class TestOptTable : public OptTable {
+class TestOptTable : public GenericOptTable {
 public:
   TestOptTable(bool IgnoreCase = false)
-: OptTable(InfoTable, IgnoreCase) {}
+  : GenericOptTable(InfoTable, IgnoreCase) {}
+};
+
+class TestPrecomputedOptTable : public PrecomputedOptTable {
+public:
+  TestPrecomputedOptTable(bool IgnoreCase = false)
+  : PrecomputedOptTable(InfoTable, PrefixTable, IgnoreCase) {}
 };
 }
 
@@ -67,8 +81,20 @@
   "-Gchuu", "2"
   };
 
-TEST(Option, OptionParsing) {
-  TestOptTable T;
+// Test fixture
+template  class OptTableTest : public ::testing::Test {};
+
+template  class DISABLED_OptTableTest : public ::testing::Test {};
+
+// Test both precomputed and computed OptTables with the same suite of tests.
+using OptTableTestTypes =
+::testing::Types;
+
+TYPED_TEST_SUITE(OptTableTest, OptTableTestTypes, );
+TYPED_TEST_SUITE(DISABLED_OptTableTest, OptTableTestTypes, );
+
+TYPED_TEST(OptTableTest, OptionParsing) {
+  TypeParam T;
   unsigned MAI, MAC;
   InputArgList AL = T.ParseArgs(Args, MAI, MAC);
 

[PATCH] D140800: Precompute OptTable prefixes union table through tablegen

2023-01-03 Thread serge via Phabricator via cfe-commits
serge-sans-paille added a comment.

In D140800#4021282 , @thakis wrote:

> I'm guessing on non-empty files this is a negligible win even in clang.

As showcased by 
https://llvm-compile-time-tracker.com/compare.php?from=4da6cb3202817ee2897d6b690e4af950459caea4&to=19a492b704e8f5c1dea120b9c0d3859bd78796be&stat=instructions:u,
 it's not neglectible in -O0 (and obviously even less neglictible for the 
preprocessing-only step)


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

https://reviews.llvm.org/D140800

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


[PATCH] D140800: Precompute OptTable prefixes union table through tablegen

2023-01-03 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 485931.
serge-sans-paille added a comment.

(rebased on main. Note that this patch is applied on top of 
https://reviews.llvm.org/D140699)


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

https://reviews.llvm.org/D140800

Files:
  clang/lib/Driver/DriverOptions.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
  lld/COFF/Driver.h
  lld/COFF/DriverUtils.cpp
  lld/ELF/Driver.h
  lld/ELF/DriverUtils.cpp
  lld/MachO/Driver.h
  lld/MachO/DriverUtils.cpp
  lld/MinGW/Driver.cpp
  lld/wasm/Driver.cpp
  lldb/tools/driver/Driver.cpp
  lldb/tools/lldb-server/lldb-gdbserver.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp
  llvm/include/llvm/Option/OptTable.h
  llvm/lib/ExecutionEngine/JITLink/COFFDirectiveParser.cpp
  llvm/lib/Option/OptTable.cpp
  llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/dsymutil/dsymutil.cpp
  llvm/tools/llvm-cvtres/llvm-cvtres.cpp
  llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp
  llvm/tools/llvm-dwarfutil/llvm-dwarfutil.cpp
  llvm/tools/llvm-ifs/llvm-ifs.cpp
  llvm/tools/llvm-lipo/llvm-lipo.cpp
  llvm/tools/llvm-ml/llvm-ml.cpp
  llvm/tools/llvm-mt/llvm-mt.cpp
  llvm/tools/llvm-nm/llvm-nm.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/llvm-rc/llvm-rc.cpp
  llvm/tools/llvm-readobj/llvm-readobj.cpp
  llvm/tools/llvm-size/llvm-size.cpp
  llvm/tools/llvm-strings/llvm-strings.cpp
  llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
  llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
  llvm/unittests/Option/OptionParsingTest.cpp
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -237,8 +237,14 @@
   CurPrefix = NewPrefix;
   }
 
-  // Dump prefixes.
+  DenseSet PrefixesUnionSet;
+  for (const auto &Prefix : Prefixes)
+PrefixesUnionSet.insert(Prefix.first.begin(), Prefix.first.end());
+  SmallVector PrefixesUnion(PrefixesUnionSet.begin(),
+   PrefixesUnionSet.end());
+  array_pod_sort(PrefixesUnion.begin(), PrefixesUnion.end());
 
+  // Dump prefixes.
   OS << "/\n";
   OS << "// Prefixes\n\n";
   OS << "#ifdef PREFIX\n";
@@ -259,6 +265,20 @@
   OS << "#undef COMMA\n";
   OS << "#endif // PREFIX\n\n";
 
+  // Dump prefix unions.
+  OS << "/\n";
+  OS << "// Prefix Union\n\n";
+  OS << "#ifdef PREFIX_UNION\n";
+  OS << "#define COMMA ,\n";
+  OS << "PREFIX_UNION({\n";
+  for (const auto &Prefix : PrefixesUnion) {
+OS << "llvm::StringLiteral(\"" << Prefix << "\") COMMA ";
+  }
+  OS << "llvm::StringLiteral(\"\")})\n";
+  OS << "#undef COMMA\n";
+  OS << "#endif // PREFIX_UNION\n\n";
+
+  // Dump groups.
   OS << "/\n";
   OS << "// ValuesCode\n\n";
   OS << "#ifdef OPTTABLE_VALUES_CODE\n";
Index: llvm/unittests/Option/OptionParsingTest.cpp
===
--- llvm/unittests/Option/OptionParsingTest.cpp
+++ llvm/unittests/Option/OptionParsingTest.cpp
@@ -32,6 +32,14 @@
 #include "Opts.inc"
 #undef PREFIX
 
+static constexpr const StringLiteral PrefixTable_init[] =
+#define PREFIX_UNION(VALUES) VALUES
+#include "Opts.inc"
+#undef PREFIX_UNION
+;
+static constexpr const ArrayRef
+PrefixTable(PrefixTable_init, std::size(PrefixTable_init) - 1);
+
 enum OptionFlags {
   OptFlag1 = (1 << 4),
   OptFlag2 = (1 << 5),
@@ -48,10 +56,16 @@
 };
 
 namespace {
-class TestOptTable : public OptTable {
+class TestOptTable : public GenericOptTable {
 public:
   TestOptTable(bool IgnoreCase = false)
-: OptTable(InfoTable, IgnoreCase) {}
+  : GenericOptTable(InfoTable, IgnoreCase) {}
+};
+
+class TestPrecomputedOptTable : public PrecomputedOptTable {
+public:
+  TestPrecomputedOptTable(bool IgnoreCase = false)
+  : PrecomputedOptTable(InfoTable, PrefixTable, IgnoreCase) {}
 };
 }
 
@@ -67,8 +81,20 @@
   "-Gchuu", "2"
   };
 
-TEST(Option, OptionParsing) {
-  TestOptTable T;
+// Test fixture
+template  class OptTableTest : public ::testing::Test {};
+
+template  class DISABLED_OptTableTest : public ::testing::Test {};
+
+// Test both precomputed and computed OptTables with the same suite of tests.
+using OptTableTestTypes =
+::testing::Types;
+
+TYPED_TEST_SUITE(OptTableTest, OptTableTestTypes, );
+TYPED_TEST_SUITE(DISABLED_OptTableTest, OptTableTestTypes, );
+
+TYPED_TEST(OptTableTest, OptionParsing) {
+  TypeParam T;
   unsigned MAI, MAC;
   InputArgList AL = T.ParseArgs(Args, MAI, MAC);
 
@@ -114,8 +140,8 @@
   EXPECT_EQ("desu", StringRef(ASL[1]));
 }
 
-TEST(Option, ParseWithFlagExclusions) {
-  TestOptTable T;
+TYPED_TEST(OptTableTest, ParseWithFlagExclusions) {
+  TypeParam T;
   unsigned MAI, MAC;
 
   // Exclude flag3 to avoid parsing as OPT_SLASH_C

[PATCH] D140699: [OptTable] Make ValuesCode initialisation of Options constexpr

2023-01-03 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 485932.
serge-sans-paille retitled this revision from "[clang] Make ValuesCode 
initialisation of Options constexpr" to "[OptTable] Make ValuesCode 
initialisation of Options constexpr".
serge-sans-paille added a comment.

(rebased on main)


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

https://reviews.llvm.org/D140699

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/DriverOptions.cpp
  llvm/include/llvm/Option/OptTable.h
  llvm/lib/Option/OptTable.cpp
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -259,6 +259,21 @@
   OS << "#undef COMMA\n";
   OS << "#endif // PREFIX\n\n";
 
+  OS << "/\n";
+  OS << "// ValuesCode\n\n";
+  OS << "#ifdef OPTTABLE_VALUES_CODE\n";
+  for (const Record &R : llvm::make_pointee_range(Opts)) {
+// The option values, if any;
+if (!isa(R.getValueInit("ValuesCode"))) {
+  assert(!isa(R.getValueInit("Values")) &&
+   "Cannot choose between Values and ValuesCode");
+  OS << "#define VALUES_CODE " << getOptionName(R) << "_Values\n";
+  OS << R.getValueAsString("ValuesCode") << "\n";
+  OS << "#undef VALUES_CODE\n";
+}
+  }
+  OS << "#endif\n";
+
   OS << "/\n";
   OS << "// Groups\n\n";
   OS << "#ifdef OPTION\n";
@@ -388,6 +403,9 @@
 OS << ", ";
 if (!isa(R.getValueInit("Values")))
   write_cstring(OS, R.getValueAsString("Values"));
+else if (!isa(R.getValueInit("ValuesCode"))) {
+  OS << getOptionName(R) << "_Values";
+}
 else
   OS << "nullptr";
   };
@@ -460,29 +478,5 @@
   OS << "\n";
 
   OS << "\n";
-  OS << "#ifdef OPTTABLE_ARG_INIT\n";
-  OS << "//\n";
-  OS << "// Option Values\n\n";
-  for (const Record &R : llvm::make_pointee_range(Opts)) {
-if (isa(R.getValueInit("ValuesCode")))
-  continue;
-OS << "{\n";
-OS << "bool ValuesWereAdded;\n";
-OS << R.getValueAsString("ValuesCode");
-OS << "\n";
-for (StringRef Prefix : R.getValueAsListOfStrings("Prefixes")) {
-  OS << "ValuesWereAdded = Opt.addValues(";
-  std::string S(Prefix);
-  S += R.getValueAsString("Name");
-  write_cstring(OS, S);
-  OS << ", Values);\n";
-  OS << "(void)ValuesWereAdded;\n";
-  OS << "assert(ValuesWereAdded && \"Couldn't add values to "
-"OptTable!\");\n";
-}
-OS << "}\n";
-  }
-  OS << "\n";
-  OS << "#endif // OPTTABLE_ARG_INIT\n";
 }
 } // end namespace llvm
Index: llvm/lib/Option/OptTable.cpp
===
--- llvm/lib/Option/OptTable.cpp
+++ llvm/lib/Option/OptTable.cpp
@@ -300,17 +300,6 @@
   return BestDistance;
 }
 
-bool OptTable::addValues(StringRef Option, const char *Values) {
-  for (size_t I = FirstSearchableIndex, E = OptionInfos.size(); I < E; I++) {
-Info &In = OptionInfos[I];
-if (optionMatches(In, Option)) {
-  In.Values = Values;
-  return true;
-}
-  }
-  return false;
-}
-
 // Parse a single argument, return the new argument, and update Index. If
 // GroupedShortOptions is true, -a matches "-abc" and the argument in Args will
 // be updated to "-bc". This overload does not support
Index: llvm/include/llvm/Option/OptTable.h
===
--- llvm/include/llvm/Option/OptTable.h
+++ llvm/include/llvm/Option/OptTable.h
@@ -60,7 +60,7 @@
 
 private:
   /// The option information table.
-  std::vector OptionInfos;
+  ArrayRef OptionInfos;
   bool IgnoreCase;
   bool GroupedShortOptions = false;
   const char *EnvVar = nullptr;
@@ -174,17 +174,6 @@
unsigned FlagsToInclude = 0, unsigned FlagsToExclude = 0,
unsigned MinimumLength = 4) const;
 
-  /// Add Values to Option's Values class
-  ///
-  /// \param [in] Option - Prefix + Name of the flag which Values will be
-  ///  changed. For example, "-analyzer-checker".
-  /// \param [in] Values - String of Values seperated by ",", such as
-  ///  "foo, bar..", where foo and bar is the argument which the Option flag
-  ///  takes
-  ///
-  /// \return true in success, and false in fail.
-  bool addValues(StringRef Option, const char *Values);
-
   /// Parse a single argument; returning the new argument and
   /// updating Index.
   ///
Index: clang/lib/Driver/DriverOptions.cpp
===
--- clang/lib/Driver/DriverOptions.cpp
+++ clang/lib/Driver/DriverOptions.cpp
@@ -16,6 +16,10 @@
 using namespace clang::driver::options;
 using namespace llvm::opt;
 
+#define OPTTABLE_VALUES_CODE
+#include "clang/Driver/Options.inc"
+#undef OPTTABLE_VALUES_CODE
+
 #define PREFIX(NAME, VALUE)\
   static constexpr llvm::Stri

[PATCH] D140859: [clang][dataflow] Allow analyzing multiple functions in unit tests

2023-01-03 Thread Stanislav Gatev via Phabricator via cfe-commits
sgatev accepted this revision.
sgatev added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp:89-104
+  unsigned FunctionBeginOffset =
+  SourceManager.getFileOffset(Func->getBeginLoc());
+  unsigned FunctionEndOffset = SourceManager.getFileOffset(Func->getEndLoc());
+
   unsigned I = 0;
-  auto Annotations = AnnotatedCode.ranges();
+  std::vector Annotations = AnnotatedCode.ranges();
+  llvm::erase_if(Annotations, [=](llvm::Annotations::Range R) {





Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:177
 /// Runs dataflow specified from `AI.MakeAnalysis` and `AI.PostVisitCFG` on the
 /// body of the function that matches `AI.TargetFuncMatcher` in `AI.Code`.
 /// Given the analysis outputs, `VerifyResults` checks that the results from 
the

"bodies of all functions that match"



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:269
 /// Runs dataflow specified from `AI.MakeAnalysis` and `AI.PostVisitCFG` on the
 /// body of the function that matches `AI.TargetFuncMatcher` in `AI.Code`. 
Given
 /// the annotation line numbers and analysis outputs, `VerifyResults` checks

"bodies of all functions that match"



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:295
 /// Runs dataflow specified from `AI.MakeAnalysis` and `AI.PostVisitCFG` on the
 /// body of the function that matches `AI.TargetFuncMatcher` in `AI.Code`. 
Given
 /// the state computed at each annotated statement and analysis outputs,

"bodies of all functions that match"



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:367
 VerifyResults(AnnotationStates, AO);
+AnnotationStates.clear();
   });

Can you please add a comment to describe why this needs to be cleared?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140859

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


[PATCH] D140874: [clang][Interp] Support pointer types in compound assignment operations

2023-01-03 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This was still missing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140874

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

Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -665,6 +665,44 @@
   // expected-note {{in call to 'IntMul}} \
   // ref-error {{not an integral constant expression}} \
   // ref-note {{in call to 'IntMul}}
+
+  constexpr int arr[] = {1,2,3};
+  constexpr int ptrInc1() {
+const int *p = arr;
+p += 2;
+return *p;
+  }
+  static_assert(ptrInc1() == 3, "");
+
+  constexpr int ptrInc2() {
+const int *p = arr;
+return *(p += 1);
+  }
+  static_assert(ptrInc2() == 2, "");
+
+  constexpr int ptrInc3() { // expected-error {{never produces a constant expression}} \
+// ref-error {{never produces a constant expression}}
+const int *p = arr;
+p += 12; // expected-note {{cannot refer to element 12 of array of 3 elements}} \
+ // ref-note {{cannot refer to element 12 of array of 3 elements}}
+return *p;
+  }
+
+  constexpr int ptrIncDec1() {
+const int *p = arr;
+p += 2;
+p -= 1;
+return *p;
+  }
+  static_assert(ptrIncDec1() == 2, "");
+
+  constexpr int ptrDec1() { // expected-error {{never produces a constant expression}} \
+// ref-error {{never produces a constant expression}}
+const int *p = arr;
+p -= 1;  // expected-note {{cannot refer to element -1 of array of 3 elements}} \
+ // ref-note {{cannot refer to element -1 of array of 3 elements}}
+return *p;
+  }
 };
 #endif
 
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -87,6 +87,7 @@
   bool VisitCharacterLiteral(const CharacterLiteral *E);
   bool VisitCompoundAssignOperator(const CompoundAssignOperator *E);
   bool VisitFloatCompoundAssignOperator(const CompoundAssignOperator *E);
+  bool VisitPointerCompoundAssignOperator(const CompoundAssignOperator *E);
   bool VisitExprWithCleanups(const ExprWithCleanups *E);
   bool VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *E);
   bool VisitCompoundLiteralExpr(const CompoundLiteralExpr *E);
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -698,6 +698,41 @@
   return this->emitStore(*LT, E);
 }
 
+template 
+bool ByteCodeExprGen::VisitPointerCompoundAssignOperator(
+const CompoundAssignOperator *E) {
+  BinaryOperatorKind Op = E->getOpcode();
+  const Expr *LHS = E->getLHS();
+  const Expr *RHS = E->getRHS();
+  std::optional LT = classify(LHS->getType());
+  std::optional RT = classify(RHS->getType());
+
+  assert(*LT == PT_Ptr);
+
+  if (!LT || !RT)
+return false;
+
+  if (!visit(LHS))
+return false;
+
+  if (!this->emitLoadPtr(LHS))
+return false;
+
+  if (!visit(RHS))
+return false;
+
+  if (Op == BO_AddAssign)
+this->emitAddOffset(*RT, E);
+  else if (Op == BO_SubAssign)
+this->emitSubOffset(*RT, E);
+  else
+return false;
+
+  if (DiscardResult)
+return this->emitStorePopPtr(E);
+  return this->emitStorePtr(E);
+}
+
 template 
 bool ByteCodeExprGen::VisitCompoundAssignOperator(
 const CompoundAssignOperator *E) {
@@ -707,6 +742,9 @@
   if (E->getType()->isFloatingType())
 return VisitFloatCompoundAssignOperator(E);
 
+  if (E->getType()->isPointerType())
+return VisitPointerCompoundAssignOperator(E);
+
   const Expr *LHS = E->getLHS();
   const Expr *RHS = E->getRHS();
   std::optional LT = classify(E->getComputationLHSType());
@@ -715,9 +753,7 @@
   if (!LT || !RT)
 return false;
 
-  assert(!E->getType()->isPointerType() &&
- "Support pointer arithmethic in compound assignment operators");
-
+  assert(!E->getType()->isPointerType() && "Handled above");
   assert(!E->getType()->isFloatingType() && "Handled above");
 
   // Get LHS pointer, load its value and get RHS value.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D140875: [clangd] prototype: Implement unused include warnings with include-cleaner library.

2023-01-03 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
hokein requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

A prototype of using include-cleaner library in clangd:

- (re)implement clangd's "unused include" warnings with the library
- the new implementation is hidden under a flag 
`Config::UnusedIncludesPolicy::Experiment`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140875

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/Preamble.h
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
  clang-tools-extra/include-cleaner/lib/Record.cpp

Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -148,7 +148,7 @@
   RecordPragma(const CompilerInstance &CI, PragmaIncludes *Out)
   : SM(CI.getSourceManager()),
 HeaderInfo(CI.getPreprocessor().getHeaderSearchInfo()), Out(Out),
-UniqueStrings(Arena) {}
+UniqueStrings(Out->Arena) {}
 
   void FileChanged(SourceLocation Loc, FileChangeReason Reason,
SrcMgr::CharacteristicKind FileType,
@@ -174,7 +174,6 @@
   std::unique(It.getSecond().begin(), It.getSecond().end()),
   It.getSecond().end());
 }
-Out->Arena = std::move(Arena);
   }
 
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
@@ -287,7 +286,6 @@
   const SourceManager &SM;
   HeaderSearch &HeaderInfo;
   PragmaIncludes *Out;
-  llvm::BumpPtrAllocator Arena;
   /// Intern table for strings. Contents are on the arena.
   llvm::StringSaver UniqueStrings;
 
@@ -316,6 +314,27 @@
   std::vector KeepStack;
 };
 
+PragmaIncludes::PragmaIncludes(const PragmaIncludes & In) {
+  *this = In;
+}
+
+PragmaIncludes &PragmaIncludes::operator=(const PragmaIncludes &In) {
+  ShouldKeep = In.ShouldKeep;
+  NonSelfContainedFiles = In.NonSelfContainedFiles;
+  Arena.Reset();
+  llvm::UniqueStringSaver Strings(Arena);
+  IWYUPublic.clear();
+  for (const auto& It : In.IWYUPublic)
+IWYUPublic.try_emplace(It.first, Strings.save(It.second));
+  IWYUExportBy.clear();
+  for (const auto& UIDAndExporters : In.IWYUExportBy) {
+const auto& [UID, Exporters] = UIDAndExporters;
+for (StringRef ExporterHeader : Exporters)
+   IWYUExportBy.try_emplace(UID).first->second.push_back(ExporterHeader);
+  }
+  return *this;
+ }
+
 void PragmaIncludes::record(const CompilerInstance &CI) {
   auto Record = std::make_unique(CI, this);
   CI.getPreprocessor().addCommentHandler(Record.get());
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
===
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
@@ -48,6 +48,13 @@
 /// defines the symbol.
 class PragmaIncludes {
 public:
+  PragmaIncludes() = default;
+  PragmaIncludes(PragmaIncludes &&) = default;
+  PragmaIncludes &operator=(PragmaIncludes &&) = default;
+
+  PragmaIncludes(const PragmaIncludes &);
+  PragmaIncludes &operator=(const PragmaIncludes &);
+
   /// Installs an analysing PPCallback and CommentHandler and populates results
   /// to the structure.
   void record(const CompilerInstance &CI);
Index: clang-tools-extra/clangd/Preamble.h
===
--- clang-tools-extra/clangd/Preamble.h
+++ clang-tools-extra/clangd/Preamble.h
@@ -27,6 +27,7 @@
 #include "Diagnostics.h"
 #include "FS.h"
 #include "Headers.h"
+#include "clang-include-cleaner/Record.h"
 #include "index/CanonicalIncludes.h"
 #include "support/Path.h"
 #include "clang/Frontend/CompilerInvocation.h"
@@ -57,6 +58,8 @@
   // Processes like code completions and go-to-definitions will need #include
   // information, and their compile action skips preamble range.
   IncludeStructure Includes;
+
+  include_cleaner::PragmaIncludes Pragmas;
   // Macros defined in the preamble section of the main file.
   // Users care about headers vs main-file, not preamble vs non-preamble.
   // These should be treated as main-file entities e.g. for code completion.
Index: clang-tools-extra/clangd/Preamble.cpp
===
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -11,6 +11,7 @@
 #include "Config.h"
 #include "Headers.h"
 #include "SourceCode.h"
+#include "clang-include-

[PATCH] D140876: [clang][C++20] Non-dependent access checks in requires expression.

2023-01-03 Thread Utkarsh Saxena via Phabricator via cfe-commits
usaxena95 created this revision.
Herald added a project: All.
usaxena95 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Access to members in a non-dependent context would always yield an
invalid expression. When it appears in a requires-expression, then this
is a hard error as this would always result in a substitution failure.

https://eel.is/c++draft/expr.prim.req#general-note-1
Note 1: If a requires-expression contains invalid types or expressions in its 
requirements, and it does not appear within the declaration of a templated 
entity, then the program is ill-formed. — end note]
If the substitution of template arguments into a requirement would always 
result in a substitution failure, the program is ill-formed; no diagnostic 
required.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140876

Files:
  clang/lib/Parse/ParseExprCXX.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp


Index: clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp
===
--- clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp
+++ clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp
@@ -155,3 +155,28 @@
 static_assert(!A<1>::faz());
 static_assert(A<0>::faz());
 }
+
+namespace non_dependent_access_check_hard_errors {
+// Dependent access does not cause hard errors.
+template class A;
+
+template <> class A<0> {
+  static void f() {}
+};
+template
+concept C1 = requires() { A::f(); };
+static_assert(!C1<0>);
+
+template <> class A<1> {
+public: 
+  static void f() {}
+};
+static_assert(C1<1>);
+
+// Non-dependent access to private member is a hard error.
+class B{
+   static void f() {} // expected-note {{implicitly declared private here}}
+};
+template
+concept C2 = requires() { B::f(); }; // expected-error {{'f' is a private 
member}}
+}
Index: clang/lib/Parse/ParseExprCXX.cpp
===
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -3507,6 +3507,7 @@
   //   Expressions appearing within a requirement-body are unevaluated 
operands.
   EnterExpressionEvaluationContext Ctx(
   Actions, Sema::ExpressionEvaluationContext::Unevaluated);
+  Sema::ContextRAII SaveContext(Actions, Actions.CurContext);
 
   ParseScope BodyScope(this, Scope::DeclScope);
   RequiresExprBodyDecl *Body = Actions.ActOnStartRequiresExpr(


Index: clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp
===
--- clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp
+++ clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp
@@ -155,3 +155,28 @@
 static_assert(!A<1>::faz());
 static_assert(A<0>::faz());
 }
+
+namespace non_dependent_access_check_hard_errors {
+// Dependent access does not cause hard errors.
+template class A;
+
+template <> class A<0> {
+  static void f() {}
+};
+template
+concept C1 = requires() { A::f(); };
+static_assert(!C1<0>);
+
+template <> class A<1> {
+public: 
+  static void f() {}
+};
+static_assert(C1<1>);
+
+// Non-dependent access to private member is a hard error.
+class B{
+   static void f() {} // expected-note {{implicitly declared private here}}
+};
+template
+concept C2 = requires() { B::f(); }; // expected-error {{'f' is a private member}}
+}
Index: clang/lib/Parse/ParseExprCXX.cpp
===
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -3507,6 +3507,7 @@
   //   Expressions appearing within a requirement-body are unevaluated operands.
   EnterExpressionEvaluationContext Ctx(
   Actions, Sema::ExpressionEvaluationContext::Unevaluated);
+  Sema::ContextRAII SaveContext(Actions, Actions.CurContext);
 
   ParseScope BodyScope(this, Scope::DeclScope);
   RequiresExprBodyDecl *Body = Actions.ActOnStartRequiresExpr(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D140876: [clang][C++20] Non-dependent access checks in requires expression.

2023-01-03 Thread Utkarsh Saxena via Phabricator via cfe-commits
usaxena95 updated this revision to Diff 485939.
usaxena95 added a comment.

More tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140876

Files:
  clang/lib/Parse/ParseExprCXX.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp


Index: clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp
===
--- clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp
+++ clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp
@@ -155,3 +155,42 @@
 static_assert(!A<1>::faz());
 static_assert(A<0>::faz());
 }
+
+namespace non_dependent_access_check_hard_errors {
+// Dependent access does not cause hard errors.
+template class A;
+
+template <> class A<0> {
+  static void f() {}
+};
+template
+concept C1 = requires() { A::f(); };
+static_assert(!C1<0>);
+
+template <> class A<1> {
+public: 
+  static void f() {}
+};
+static_assert(C1<1>);
+
+// Non-dependent access to private member is a hard error.
+class B{
+   static void f() {} // expected-note 2{{implicitly declared private here}}
+};
+template
+concept C2 = requires() { B::f(); }; // expected-error {{'f' is a private 
member}}
+
+constexpr bool non_template_func() {
+  return requires() {
+  B::f(); // expected-error {{'f' is a private member}}
+  };
+}
+template
+constexpr bool template_func() {
+  return requires() {
+  A::f();
+  };
+}
+static_assert(!template_func<0>());
+static_assert(template_func<1>());
+}
Index: clang/lib/Parse/ParseExprCXX.cpp
===
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -3507,6 +3507,7 @@
   //   Expressions appearing within a requirement-body are unevaluated 
operands.
   EnterExpressionEvaluationContext Ctx(
   Actions, Sema::ExpressionEvaluationContext::Unevaluated);
+  Sema::ContextRAII SaveContext(Actions, Actions.CurContext);
 
   ParseScope BodyScope(this, Scope::DeclScope);
   RequiresExprBodyDecl *Body = Actions.ActOnStartRequiresExpr(


Index: clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp
===
--- clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp
+++ clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp
@@ -155,3 +155,42 @@
 static_assert(!A<1>::faz());
 static_assert(A<0>::faz());
 }
+
+namespace non_dependent_access_check_hard_errors {
+// Dependent access does not cause hard errors.
+template class A;
+
+template <> class A<0> {
+  static void f() {}
+};
+template
+concept C1 = requires() { A::f(); };
+static_assert(!C1<0>);
+
+template <> class A<1> {
+public: 
+  static void f() {}
+};
+static_assert(C1<1>);
+
+// Non-dependent access to private member is a hard error.
+class B{
+   static void f() {} // expected-note 2{{implicitly declared private here}}
+};
+template
+concept C2 = requires() { B::f(); }; // expected-error {{'f' is a private member}}
+
+constexpr bool non_template_func() {
+  return requires() {
+  B::f(); // expected-error {{'f' is a private member}}
+  };
+}
+template
+constexpr bool template_func() {
+  return requires() {
+  A::f();
+  };
+}
+static_assert(!template_func<0>());
+static_assert(template_func<1>());
+}
Index: clang/lib/Parse/ParseExprCXX.cpp
===
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -3507,6 +3507,7 @@
   //   Expressions appearing within a requirement-body are unevaluated operands.
   EnterExpressionEvaluationContext Ctx(
   Actions, Sema::ExpressionEvaluationContext::Unevaluated);
+  Sema::ContextRAII SaveContext(Actions, Actions.CurContext);
 
   ParseScope BodyScope(this, Scope::DeclScope);
   RequiresExprBodyDecl *Body = Actions.ActOnStartRequiresExpr(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D140685: [LoongArch] Add intrinsics for MOVFCSR2GR and MOVGR2FCSR instructions

2023-01-03 Thread WÁNG Xuěruì via Phabricator via cfe-commits
xen0n accepted this revision.
xen0n added a comment.

In D140685#4017381 , @xen0n wrote:

>> MOVGR2FCSR modifies the value of the software writable field
>> corresponding to the FCSR (floating-point control and status
>> register) fcsr according to the value of the lower 32 bits of
>> the GR (general purpose register) rj.
>
> The description of `movgr2fcsr` is incorrect, it implies `GPR[rj]`is read, 
> but in fact it's only the //`ui5` immediate in `rj` slot//, i.e. `FCSR[rj]`. 
> I didn't look very closely but the test case changes seem good.

On a closer look it turns out the description is correct after all, the 
description is very much non-intuitive (uses just `fcsr` without any position 
marker) so I've mistaken the GPR operand for the FCSR slot. This is very 
unfortunate but luckily users of the intrinsics won't have to think about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140685

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


[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

2023-01-03 Thread Francesco Petrogalli via Phabricator via cfe-commits
fpetrogalli added a comment.

@craig.topper @lenary @barannikov88 @pcwang-thead - New Year's gentle ping :)

Thank you,

Francesco


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

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


[PATCH] D140843: [clang-format] fix template closer followed by >

2023-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

I'm good with this but I'd be interesting in @owenpan  and @HazardyKnusperkeks 
opinion.




Comment at: clang/lib/Format/TokenAnnotator.cpp:169-171
+CurrentToken->getStartOfNonWhitespace() ==
+CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
+-1)) {

I was wondering this is actually saying   

I think its saying... 2 tokens are next to each other ">>"  its not so much 
formatting but ensuring the existing formatting is maintained, is that what you 
understand?

Whilst this works it won't correct the case where the whitespace is wrong

i.e. I don't think

```
if (std::tuple_size_v < T >> 0) {
}
```

will be corrected to be

```
if (std::tuple_size_v > 0) {
}
```

I'm a little wary of rules that use the existing whitespace, but I tend to 
agree that it might be ok without the extra check. 

It would be good to capture this as an annotator test (I like the verifyformat 
one you put in) but the annotator tests we can assert that its actually a 
templatecloser and binary operatror





Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140843

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


[PATCH] D140835: [clang-format] Improve UnwrappedLineParser::mightFitOnOneLine()

2023-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140835

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


[PATCH] D140339: [clang-format] Remove special logic for parsing concept definitions.

2023-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.

Thank you for this patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140339

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


[PATCH] D140767: [clang-format] Require space before noexcept qualifier

2023-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.

Nice!  Thank you for another great patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140767

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


[PATCH] D140859: [clang][dataflow] Allow analyzing multiple functions in unit tests

2023-01-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel accepted this revision.
ymandel added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp:59
+if (SM.isPointWithin(Loc, BoundingRange.getBegin(),
+ BoundingRange.getEnd())) {
+  LineNumberToContent[SM.getPresumedLineNumber(Loc)] =

This seems subtly wrong if BoundingRange is a TokenRange. It might be ok, if a 
comment can't be part of a token (at least, as a prefix), but still seems 
conceptually wrong in that it's not quite doing what it appears. So, if token 
ranges are ok, I'd recommend at least explaining that in the comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:173
 llvm::DenseMap
-buildLineToAnnotationMapping(SourceManager &SM,
+buildLineToAnnotationMapping(SourceManager &SM, SourceRange BoundingRange,
  llvm::Annotations AnnotatedCode);

please specify (in the comments) the intended interpretation of `SourceRange` 
-- CharRange or TokenRange -- or use `CharSourceRange`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140859

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


[PATCH] D123609: [Clang] Remove support for legacy pass manager

2023-01-03 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment.
Herald added a subscriber: pcwang-thead.

In D123609#3451334 , @nikic wrote:

> In D123609#3451320 , @HaohaiWen 
> wrote:
>
>> Hi @nikic,
>> We recently noticed legacy PM was removed from many places.
>> Does community plan to remove legacy PM completely?
>> Do you know when will CG switch to new PM?
>> Thanks.
>
> At this time, the ability to run a middle-end optimization pipeline using the 
> legacy pass manager (basically anything based on PassManagerBuilder) is being 
> removed. The CodeGen pipeline continues to use the legacy pass manager at 
> this time -- I'm not aware of anyone doing active migration work in that area.

Sorry to resurrect an old patch, but can you please update the docs here 
https://llvm.org/docs/NewPassManager.html#status-of-the-new-and-legacy-pass-managers
 with this info?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123609

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


[PATCH] D140843: [clang-format] fix template closer followed by >

2023-01-03 Thread Zhikai Zeng via Phabricator via cfe-commits
Backl1ght added a comment.

In D140843#4022673 , @MyDeveloperDay 
wrote:

> I'm good with this but I'd be interesting in @owenpan  and 
> @HazardyKnusperkeks opinion.

Yes, this is what I understand.

And I will add an annotator test later.

This patch is just a temporary solution I think, there are further things to do 
with such case, maybe I should add a TODO or FIXME?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140843

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


[PATCH] D140696: [clang][dataflow] Treat unions as structs.

2023-01-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment.

In D140696#4020209 , @merrymeerkat 
wrote:

> @ymandel - thank you for pointing all of this out! And no need to apologise 
> at all :)
>
> I agree that the changes made here are not ideal. But the alternative is also 
> not ideal.
>
> The question becomes: is it better to model unions in a way that is lacking, 
> or to not model them at all?
>
> To model unions properly, we would need to be able to set the storage 
> location of all the members of a union to be the same. But storage location 
> is tied to type, so we can't create the same storage location for members of 
> different types. From what I can tell, correctly modelling unions would 
> require us to do an overhaul of how storage locations are implemented.
>
> Like Dmitri said, I think most of the issues with this simple approach to 
> modelling unions would only be revealed with UB code. So, unless we want to 
> compare the storage locations of pointers to different union members (which I 
> don't think will be a very common use), this modelling should work all right 
> for defined behaviour. I added a documentation comment regarding the lacking 
> implementation of storage location.
>
> Coming back to the question above. Yitzhak, you seem to prefer not modelling 
> union at all rather than having this model that falls short. Dmitri seems to 
> prefer a model that works on most cases than no model. I am a bit torn, but I 
> think I'm leaning a bit on the side of having a model that falls short. I 
> don't think we plan on doing the storage location overhaul anytime soon, and 
> until then it'd be good to at least have some functionality.
>
> Happy new year by the way!!

Happy new year!  I should clarify: Dmitri convinced me that this form of 
modeling is ok for unions. My remaining concern is testing: that we've seen 
many things that seem ok on paper and then results in difficult-to-debug 
crashes when run over a large corpus. Some of those could come from the 
implementation itself (bugs or we reasoned incorrectly about how it works or 
there is UB in the code that breaks our assumptions, etc.); some of it can come 
about simply because we've now enabled analysis of a larger range of code 
paths. So, the prudent approach is to test a change like this thoroughly before 
committing it. In contrast, the change you need just to avoid the crash is 
quite small and limited in its scope.

That said, we don't know of any large users depending on this analysis, and we 
don't (yet) have any automated setup for running over a large corpus, so I'm ok 
going ahead to unblock you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140696

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


[PATCH] D140686: [WIP][Clang][RISCV] Update operand order for vmerge and vcompress

2023-01-03 Thread Yueh-Ting (eop) Chen via Phabricator via cfe-commits
eopXD updated this revision to Diff 485961.
eopXD added a comment.

Update test cases for vfmerge.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140686

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/include/clang/Support/RISCVVIntrinsicUtils.h
  clang/lib/Support/RISCVVIntrinsicUtils.cpp
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vcompress.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmerge.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmerge.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vcompress.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfmerge.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vmerge.c

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


[PATCH] D140891: [analyzer] Fix assertion failure in SMT conversion for unary operator on floats.

2023-01-03 Thread Tomasz Kamiński via Phabricator via cfe-commits
tomasz-kaminski-sonarsource created this revision.
Herald added subscribers: steakhal, manas, ASDenysPetrov, dkrupp, donat.nagy, 
Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
tomasz-kaminski-sonarsource requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

In the handling of the Symbols from the RangExpr, the code assumed that the 
operands of the unary operators need to have integral type.
However, the CSA can create SymExpr with a floating point operand, when the 
integer value is cast into it, like `(float)h == (float)l` where both of `h` 
and `l` are integers.

This patch handles such situations, by using `fromFloatUnOp` instead of 
`fromUnOp`, when the operand have a floating point type.

I have investigated all other calls  of  `fromUnOp`, and for one in:

- `getZeroExpr` is applied only on boolean types, so it correct
- `fromBinOp` is not invoked for floating points
- `fromFloatUnOp` I am uncertain about this case and I was able to produce a 
test that would reach this point, as a negation of floating points numbers seem 
to produce `Unknown` symbols.,

This issue exists since the introduction of `UnarySymExpr` in 
https://reviews.llvm.org/D125318 and their handling for Z3 in 
https://reviews.llvm.org/D125547.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140891

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h
  clang/test/Analysis/z3-crosscheck.c


Index: clang/test/Analysis/z3-crosscheck.c
===
--- clang/test/Analysis/z3-crosscheck.c
+++ clang/test/Analysis/z3-crosscheck.c
@@ -1,7 +1,9 @@
-// RUN: %clang_cc1 -analyze 
-analyzer-checker=core,unix.Malloc,debug.ExprInspection -DNO_CROSSCHECK -verify 
%s
-// RUN: %clang_cc1 -analyze 
-analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config 
crosscheck-with-z3=true -verify %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze 
-analyzer-checker=core,unix.Malloc,debug.ExprInspection -DNO_CROSSCHECK -verify 
%s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze 
-analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config 
crosscheck-with-z3=true -verify %s
 // REQUIRES: z3
 
+void clang_analyzer_dump(float);
+
 int foo(int x) 
 {
   int *z = 0;
@@ -55,3 +57,23 @@
 h(2);
   }
 }
+
+void floatUnaryNegInEq(int h, int l) {
+  int j;
+  clang_analyzer_dump(-(float)h); // expected-warning-re{{-(float) 
(reg_${{[0-9]+}})}}
+  clang_analyzer_dump((float)l); // expected-warning-re {{(float) 
(reg_${{[0-9]+}})}}
+  if (-(float)h != (float)l) {  // should not crash
+j += 10;
+// expected-warning@-1{{garbage}}
+  }
+}
+
+void floatUnaryLNotInEq(int h, int l) {
+  int j;
+  clang_analyzer_dump(!(float)h); // expected-warning{{Unknown}}
+  clang_analyzer_dump((float)l); // expected-warning-re {{(float) 
(reg_${{[0-9]+}})}}
+  if ((!(float)h) != (float)l) {  // should not crash
+j += 10;
+// expected-warning@-1{{garbage}}
+  }
+}
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h
@@ -454,6 +454,8 @@
   llvm::SMTExprRef OperandExp =
   getSymExpr(Solver, Ctx, USE->getOperand(), &OperandTy, 
hasComparison);
   llvm::SMTExprRef UnaryExp =
+  OperandTy->isRealFloatingType() ?
+  fromFloatUnOp(Solver, USE->getOpcode(), OperandExp) :
   fromUnOp(Solver, USE->getOpcode(), OperandExp);
 
   // Currently, without the `support-symbolic-integer-casts=true` option,


Index: clang/test/Analysis/z3-crosscheck.c
===
--- clang/test/Analysis/z3-crosscheck.c
+++ clang/test/Analysis/z3-crosscheck.c
@@ -1,7 +1,9 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -DNO_CROSSCHECK -verify %s
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config crosscheck-with-z3=true -verify %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -DNO_CROSSCHECK -verify %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config crosscheck-with-z3=true -verify %s
 // REQUIRES: z3
 
+void clang_analyzer_dump(float);
+
 int foo(int x) 
 {
   int *z = 0;
@@ -55,3 +57,23 @@
 h(2);
   }
 }
+
+void floatUnaryNegInEq(int h, int l) {
+  int j;
+  clang_analyzer_dump(-(float)h); // expected-warning-re{{-(float) (reg_${{[0-9]+}})}}
+  clang_analyzer_dump((float)l); // expected-warning-re {{(float) (reg_${{[0-9]+}})}}
+  if (-(float)h != (float)l) {  // should not c

[PATCH] D140894: [clang-tidy] Don't emit misc-unused-using-decl warnings for header files.

2023-01-03 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: ymandel.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
hokein requested review of this revision.
Herald added a project: clang-tools-extra.

Using decls in header files are special, usually as part of the
public API, the check should not emit warnings on these.

The check already detects unused using-decls which are in the current main
file, but if the main file happens to be a header file, we still
emit warnings, this patch suppresses that.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140894

Files:
  clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
  clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.h
  clang-tools-extra/docs/clang-tidy/checks/misc/unused-using-decls.rst
  clang-tools-extra/test/clang-tidy/checkers/misc/unused-using-decls.hxx

Index: clang-tools-extra/test/clang-tidy/checkers/misc/unused-using-decls.hxx
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc/unused-using-decls.hxx
@@ -0,0 +1,6 @@
+// RUN: %check_clang_tidy %s misc-unused-using-decls %t -- --fix-notes -- -fno-delayed-template-parsing -isystem %S/Inputs
+
+// Verify that we don't generate the warnings on header files.
+namespace foo { class Foo {}; }
+
+using foo::Foo;
Index: clang-tools-extra/docs/clang-tidy/checks/misc/unused-using-decls.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/misc/unused-using-decls.rst
+++ clang-tools-extra/docs/clang-tidy/checks/misc/unused-using-decls.rst
@@ -5,9 +5,24 @@
 
 Finds unused ``using`` declarations.
 
+Unused ``using``` declarations in header files will not be diagnosed since these
+using declarations are part of the header's public API. Allowed header file
+extensions can be configured via the `HeaderFileExtensions` option (see below).
+
 Example:
 
 .. code-block:: c++
 
+  // main.cpp
   namespace n { class C; }
   using n::C;  // Never actually used.
+
+Options
+---
+
+.. option:: HeaderFileExtensions
+
+   A semicolon-separated list of filename extensions of header files (the filename
+   extensions should not include "." prefix). Default is ";h;hh;hpp;hxx".
+   For extension-less header files, using an empty string or leaving an
+   empty string between ";" if there are other filename extensions.
Index: clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.h
===
--- clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.h
+++ clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNUSED_USING_DECLS_H
 
 #include "../ClangTidyCheck.h"
+#include "../utils/FileExtensionsUtils.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include 
 
@@ -23,8 +24,7 @@
 /// http://clang.llvm.org/extra/clang-tidy/checks/misc/unused-using-decls.html
 class UnusedUsingDeclsCheck : public ClangTidyCheck {
 public:
-  UnusedUsingDeclsCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  UnusedUsingDeclsCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
   void onEndOfTranslationUnit() override;
@@ -48,6 +48,9 @@
   };
 
   std::vector Contexts;
+
+  const StringRef RawStringHeaderFileExtensions;
+  utils::FileExtensionsSet HeaderFileExtensions;
 };
 
 } // namespace misc
Index: clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
@@ -38,6 +38,19 @@
  isa(TargetDecl);
 }
 
+UnusedUsingDeclsCheck::UnusedUsingDeclsCheck(StringRef Name,
+ ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
+  "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
+  if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
+  HeaderFileExtensions,
+  utils::defaultFileExtensionDelimiters())) {
+this->configurationDiag("Invalid header file extension: '%0'")
+<< RawStringHeaderFileExtensions;
+  }
+}
+
 void UnusedUsingDeclsCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(usingDecl(isExpansionInMainFile()).bind("using"), this);
   auto DeclMatcher = hasDeclaration(namedDecl().bind("used"));
@@ -66,6 +79,12 @@
 void UnusedUsingDeclsCheck::check(const MatchFinder::MatchResult &Result) {
   if (Result.Context->getDiagnostics().hasUncompilableErrorOccurred())
 ret

[clang] 80a7803 - [NFC] Update CXXSTatus to show we implement CWG 2061

2023-01-03 Thread Erich Keane via cfe-commits

Author: Erich Keane
Date: 2023-01-03T06:18:08-08:00
New Revision: 80a78033cf5f21c082aa30bfc692df76d296573c

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

LOG: [NFC] Update CXXSTatus to show we implement CWG 2061

Looking through the list, I discovered this was implemented and has been
for as long as Clang shows up on godbolt, so this patch updates the
  CXXStatus list and adds a test.

Added: 


Modified: 
clang/test/CXX/drs/dr20xx.cpp
clang/www/cxx_dr_status.html

Removed: 




diff  --git a/clang/test/CXX/drs/dr20xx.cpp b/clang/test/CXX/drs/dr20xx.cpp
index aef14dd222595..0e39015868c7d 100644
--- a/clang/test/CXX/drs/dr20xx.cpp
+++ b/clang/test/CXX/drs/dr20xx.cpp
@@ -310,3 +310,29 @@ namespace dr2094 { // dr2094: 5
   static_assert(__is_trivially_assignable(A, const A&), "");
   static_assert(__is_trivially_assignable(B, const B&), "");
 }
+
+namespace dr2061 { // dr2061: yes
+  namespace A {
+inline namespace b {
+  namespace C {
+// 'f' is the example from the DR.  'S' is an example where if we 
didn't
+// properly handle the two being the same, we would get an incomplete
+// type error during attempted instantiation.
+template void f();
+template struct S;
+  }
+}
+  }
+
+  namespace A {
+namespace C {
+  template<> void f() { }
+  template<> struct S { };
+}
+  }
+
+  void use() {
+A::C::f();
+A::C::S s;
+  }
+}

diff  --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index 11c90cc9e940d..fe609ea08a63d 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -12173,7 +12173,7 @@ C++ defect report implementation 
status
 https://wg21.link/cwg2061";>2061
 CD4
 Inline namespace after simplifications
-Unknown
+Yes
   
   
 https://wg21.link/cwg2062";>2062



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


[clang] a5ae5af - Revert "[NFC] Update CXXSTatus to show we implement CWG 2061"

2023-01-03 Thread Erich Keane via cfe-commits

Author: Erich Keane
Date: 2023-01-03T06:26:29-08:00
New Revision: a5ae5afa521f75e87f9018d8361aa5a1cadc7a86

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

LOG: Revert "[NFC] Update CXXSTatus to show we implement CWG 2061"

This reverts commit 80a78033cf5f21c082aa30bfc692df76d296573c.

Fails thanks to an inline-ns warning

Added: 


Modified: 
clang/test/CXX/drs/dr20xx.cpp
clang/www/cxx_dr_status.html

Removed: 




diff  --git a/clang/test/CXX/drs/dr20xx.cpp b/clang/test/CXX/drs/dr20xx.cpp
index 0e39015868c7d..aef14dd222595 100644
--- a/clang/test/CXX/drs/dr20xx.cpp
+++ b/clang/test/CXX/drs/dr20xx.cpp
@@ -310,29 +310,3 @@ namespace dr2094 { // dr2094: 5
   static_assert(__is_trivially_assignable(A, const A&), "");
   static_assert(__is_trivially_assignable(B, const B&), "");
 }
-
-namespace dr2061 { // dr2061: yes
-  namespace A {
-inline namespace b {
-  namespace C {
-// 'f' is the example from the DR.  'S' is an example where if we 
didn't
-// properly handle the two being the same, we would get an incomplete
-// type error during attempted instantiation.
-template void f();
-template struct S;
-  }
-}
-  }
-
-  namespace A {
-namespace C {
-  template<> void f() { }
-  template<> struct S { };
-}
-  }
-
-  void use() {
-A::C::f();
-A::C::S s;
-  }
-}

diff  --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index fe609ea08a63d..11c90cc9e940d 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -12173,7 +12173,7 @@ C++ defect report implementation 
status
 https://wg21.link/cwg2061";>2061
 CD4
 Inline namespace after simplifications
-Yes
+Unknown
   
   
 https://wg21.link/cwg2062";>2062



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


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

2023-01-03 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

I did a look through, I'm quite sure I'm not going to be able to accept this, 
but I'm hoping the little feedback I gave helped. I can't help with the review 
of the mangling unfortunately, we probably need to wait for the owners of the 
manglings to make a decision.




Comment at: clang/include/clang/AST/Decl.h:1815
+  void setIsExplicitObjectParameter(bool isExplicitObjectParam,
+SourceLocation Loc = SourceLocation()) {
+ParmVarDeclBits.IsExplicitObjectParameter = isExplicitObjectParam;

What cases are there where we wouldn't have a source location here?  Is that 
case common enough that this should be a default parameter?  



Comment at: clang/include/clang/AST/Expr.h:1447
+  bool Set, const ASTContext &Context) {
+DeclRefExprBits.CapturedByCopyInLambdaWithExplicitObjectParameter = Set;
+setDependence(computeDependence(this, Context));

This bit concerns me a touch, I'm a little scared/shocked that this would 
matter at all... This makes me think more nefarious issues are at play here.



Comment at: clang/include/clang/Sema/Sema.h:2130
 const FunctionProtoType *Superset,
+unsigned SkipSupersetFirstParameter,
 SourceLocation SuperLoc,

These params are definitely going to need documentation, I have no idea what 
that means.



Comment at: clang/lib/AST/MicrosoftMangle.cpp:2560
   IsInLambda = true;
-if (MD->isInstance())
+if (MD->isImplicitObjectMemberFunction())
   HasThisQuals = true;

This bit probably  needs to wait until microsoft decides what the mangling for 
these are (all of microsoft mangle)



Comment at: clang/lib/CodeGen/CGExpr.cpp:2633
+  return CGF.EmitLValueForLambdaField(FD, ThisValue);
+  // QualType TagType = CGF.getContext().getTagDeclType(FD->getParent());
+  // LValue LV = CGF.MakeNaturalAlignAddrLValue(ThisValue, TagType);

Eh?



Comment at: clang/lib/CodeGen/CGExpr.cpp:4254
+ llvm::Value *ThisValue) {
+  bool HasExplicitObjectParameter = false;
+  if (const CXXMethodDecl *MD =

If there is a CodeGen expert in the house, I'd really hope they go through this 
:) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140828

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


[PATCH] D140835: [clang-format] Improve UnwrappedLineParser::mightFitOnOneLine()

2023-01-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks requested changes to this revision.
HazardyKnusperkeks added a comment.
This revision now requires changes to proceed.

On second thought, shouldn't we test for removing the braces?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140835

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


[PATCH] D140874: [clang][Interp] Support pointer types in compound assignment operations

2023-01-03 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

I'm pretty far behind on reviews of the interpreter, but this one I noticed 
while looking th rough it.




Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:710
+
+  assert(*LT == PT_Ptr);
+

This is UB here if LT doesn't contain a value.  Additionally, it makes line 712 
really odd here.  It seems that LT isn't really used anywhere in this function. 
 I know classify is meaningful here (and thus needs to run), but I wonder if we 
want to make this:

`assert(LT && *LT == PT_Ptr);` and take out the 1st test on 712.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140874

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


[PATCH] D140895: [WIP][Clang][RISCV] Remove default tail-undisturbed for vector reduction intrinsics

2023-01-03 Thread Yueh-Ting (eop) Chen via Phabricator via cfe-commits
eopXD created this revision.
Herald added subscribers: sunshaoce, VincentWu, vkmr, frasercrmck, evandro, 
luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, 
PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, 
shiva0217, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, asb, 
arichardson.
Herald added a project: All.
eopXD requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, MaskRay.
Herald added a project: clang.

The destination parameter is removed for non-policy unmasked intrinsics,
they are now tail agnostic. The non-policy masked intrinsics are tail
agnostic and masked undisturbed, just like normal instructions like
vadd.

This is the first commit of a patch-set that aims to remove the
`IsPrototypeDefaultTU` special case for the rvv-intrinsics.

The patch-set work towards the simplification proposal [0] of Nick
Knight, the plan is that after this patch-set, all non-policy
intrinsics will be aligned with default policy behavior of tail
agnostic and mask unsidturbed. Then the next patch-set will aim
towards changing non-policy intrinsics to tail agnostic and mask
agnostic.

[0] https://gist.github.com/nick-knight/6cb0b74b351a25323dfb1821d3a269b9


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140895

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfredmax.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfredmin.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfredsum.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwredsum.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vredand.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vredmax.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vredmin.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vredor.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vredsum.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vredxor.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vwredsum.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfredmax.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfredmin.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfredsum.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfwredsum.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vredand.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vredmax.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vredmin.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vredor.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vredsum.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vredxor.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vwredsum.c

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


[clang] e45c0a9 - Reapply "[NFC] Update CXXSTatus to show we implement CWG 2061""

2023-01-03 Thread Erich Keane via cfe-commits

Author: Erich Keane
Date: 2023-01-03T06:58:45-08:00
New Revision: e45c0a919df3f9a8e5c352878fe9beb61cbc559d

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

LOG: Reapply "[NFC] Update CXXSTatus to show we implement CWG 2061""

This reverts commit a5ae5afa521f75e87f9018d8361aa5a1cadc7a86.

FIx the test that didn't consider the test was running in pre-C++11
mode.

Added: 


Modified: 
clang/test/CXX/drs/dr20xx.cpp
clang/www/cxx_dr_status.html

Removed: 




diff  --git a/clang/test/CXX/drs/dr20xx.cpp b/clang/test/CXX/drs/dr20xx.cpp
index aef14dd222595..c167f41397c19 100644
--- a/clang/test/CXX/drs/dr20xx.cpp
+++ b/clang/test/CXX/drs/dr20xx.cpp
@@ -310,3 +310,31 @@ namespace dr2094 { // dr2094: 5
   static_assert(__is_trivially_assignable(A, const A&), "");
   static_assert(__is_trivially_assignable(B, const B&), "");
 }
+
+namespace dr2061 { // dr2061: yes
+#if __cplusplus >= 201103L
+  namespace A {
+inline namespace b {
+  namespace C {
+// 'f' is the example from the DR.  'S' is an example where if we 
didn't
+// properly handle the two being the same, we would get an incomplete
+// type error during attempted instantiation.
+template void f();
+template struct S;
+  }
+}
+  }
+
+  namespace A {
+namespace C {
+  template<> void f() { }
+  template<> struct S { };
+}
+  }
+
+  void use() {
+A::C::f();
+A::C::S s;
+  }
+#endif // C++11
+}

diff  --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index 11c90cc9e940d..fe609ea08a63d 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -12173,7 +12173,7 @@ C++ defect report implementation 
status
 https://wg21.link/cwg2061";>2061
 CD4
 Inline namespace after simplifications
-Unknown
+Yes
   
   
 https://wg21.link/cwg2062";>2062



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


[PATCH] D139935: [NFC] [Doc] Fix example for AnnotateTypeDocs

2023-01-03 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

I don't particularly know what the intent here was, so I'm hoping that @mboehme 
will review when he gets back from whatever vacation he's taking.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139935

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


[PATCH] D140554: [C++20] Determine the dependency of unevaluated lambdas more accurately

2023-01-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment.

@shafik I'm not sure either it's the right direction but I don't think it makes 
the situation any worse.
And given I still don't really know what a proper fix would look like (except 
that it would probably be pretty involved), I think we might has well do that 
as an improvement of the pre existing workaround.

@lime Can you add a release note indicating the issue has been fixed? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140554

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


[PATCH] D140387: [clang][analyzer] Add stream related functions to StdLibraryFunctionsChecker.

2023-01-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

In D140387#4021806 , @Szelethus wrote:

> Would be possible to test the errno specific changes as well?

I suppose the tests are done in the followup patch mostly?




Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp:310
+  const MemRegion *ErrnoR = State->get();
+  if (!ErrnoR)
+return State;

balazske wrote:
> Szelethus wrote:
> > When can this occur? Maybe we can turn this early return to an assert -- 
> > when `ModelPOSIX` is true, this mustn't be null (if what I'm saying is 
> > correct).
> I am not totally sure that `errno` is found always (definition of `errno` is 
> encountered) if a summary for a standard function is added. The standard 
> function in itself must be there, otherwise no summary is added and this 
> function should not be called. But definition of `errno` may reside in other 
> header (errno.h) that is probably not included in the translation unit. If 
> the errno definition is not found the `ErrnoRegion` is not set (the checker 
> may still turned be on but does nothing, but these functions are allowed to 
> be called). Existence of `errno` is not even dependent on `ModelPOSIX`, 
> `errno` is defined in the (non POSIX) C standard.
Very well, I'm sold.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1705-1707
+  // FIXME: It should be allowed to have a null buffer if any of
+  // args 1 or 2 are zero. Remove NotNull check of arg 0, add a check
+  // for non-null buffer if non-zero size to BufferSizeConstraint?

balazske wrote:
> Szelethus wrote:
> > Maybe `StreamChecker` could take over the handling of that case? Seems like 
> > it also warns about the nullness of the first `fread` parameter.
> The current approach was that the NULL argument warning is generated by one 
> checker only, `StreamChecker` or this checker. The NULL check is already 
> built into these summaries so this is the better place for the warning. The 
> NULL warning is removed from `StreamChecker` in D137790. Additionally the 
> same problem can happen with `BufferSizeConstraint` at other function 
> summaries, we can say generally (or more often) that if a buffer has size of 
> 0 it is allowed to be NULL pointer.
Aha, okay, so we should extend the existing infrastructure to express this as 
well. Please do that in another patch though! Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140387

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


[PATCH] D140835: [clang-format] Improve UnwrappedLineParser::mightFitOnOneLine()

2023-01-03 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment.

In D140835#4023080 , 
@HazardyKnusperkeks wrote:

> On second thought, shouldn't we test for removing the braces?

Hmm. This patch including the added test case //is// for removing braces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140835

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


[PATCH] D140554: [C++20] Determine the dependency of unevaluated lambdas more accurately

2023-01-03 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

I agree with Corentin here, I've got no idea what the RIGHT solution looks like 
here, but this looks to be at least an improvement.  1 nit, plus  need a 
release note.  Otherwise LGTM.




Comment at: clang/test/CodeGenCXX/cxx20-unevaluated-lambda-crash.cpp:4
+// CHECK-LABEL: define linkonce_odr void 
@"_ZN10Issue579601EIiEENS_1FILNS_3$_0v"()
+namespace Issue57960{
+template

Nit: need a space after the name and before the {.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140554

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


[PATCH] D140896: [WIP] Move from llvm::makeArrayRef to ArrayRef deduction guides

2023-01-03 Thread serge via Phabricator via cfe-commits
serge-sans-paille created this revision.
Herald added subscribers: libc-commits, hanchung, kadircet, Moerafaat, 
kmitropoulou, zero9178, Enna1, bzcheeseman, kosarev, mattd, gchakrabarti, 
ThomasRaoux, pmatos, asb, ayermolo, awarzynski, arjunp, sdasgup3, asavonic, 
carlosgalvezp, Groverkss, wenzhicui, wrengr, armkevincheng, ormris, foad, 
jsetoain, jsmolens, sjarus, eric-k256, cota, mravishankar, teijeong, 
frasercrmck, rdzhabarov, ecnelises, tatianashp, wenlei, mehdi_amini, jdoerfert, 
msifontes, jurahul, Kayjukh, grosul1, Joonsoo, kerbowa, liufengdb, aartbik, 
mgester, arpith-jacob, csigg, antiagainst, shauheen, rriddle, luismarques, 
apazos, sameer.abuasal, pengfei, s.egerton, Jim, jocewei, PkmX, arphaman, 
the_o, brucehoult, MartinMosbeck, rogfer01, steven_wu, atanasyan, edward-jones, 
zzheng, jrtc27, martong, niosHD, sabuasal, simoncook, johnrusso, rbar, 
fedor.sergeev, kbarton, hiraditya, jgravelle-google, arichardson, sbc100, 
jvesely, nemanjai, sdardis, emaste, dylanmckay, jyknight, dschuff, arsenm, 
qcolombet, MatzeB.
Herald added a reviewer: JDevlieghere.
Herald added a reviewer: alexander-shaposhnikov.
Herald added a reviewer: shafik.
Herald added a reviewer: jhenderson.
Herald added a reviewer: antiagainst.
Herald added a reviewer: rriddle.
Herald added a reviewer: antiagainst.
Herald added a reviewer: aartbik.
Herald added a reviewer: MaskRay.
Herald added a reviewer: jpienaar.
Herald added a reviewer: ftynse.
Herald added a reviewer: awarzynski.
Herald added a reviewer: bondhugula.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added a reviewer: ThomasRaoux.
Herald added a reviewer: NoQ.
Herald added a reviewer: njames93.
Herald added projects: libc-project, Flang, All.
serge-sans-paille requested review of this revision.
Herald added subscribers: cfe-commits, llvm-commits, lldb-commits, 
pcwang-thead, yota9, StephenFan, stephenneuendorffer, nicolasvasilache, 
aheejin, jholewinski.
Herald added a reviewer: nicolasvasilache.
Herald added a reviewer: herhut.
Herald added a reviewer: dcaballe.
Herald added projects: clang, LLDB, MLIR, LLVM, clang-tools-extra.

Since we're now requiring C++17, Let's get rid of `makeXXX` functions like 
`makeArrayRef`, and use deduction guides instead.

Apart from codebase modernization, there isn't much benefit from that
move, but I can still mention that it would slightly (probably
negligibly) decrease the number of symbols / debug info, as deduction
guides don't generate new code.

The only non-automatic changes have been:

1. Write the deduction guides
2. `ArrayRef(some_uint8_pointer, 0)` needs to be changed into 
`ArrayRef(some_uint8_pointer, (size_t)0)` to avoid an ambiguous call with 
`ArrayRef((uint8_t*), (uint8_t*))`
3. `CVSymbol sym(makeArrayRef(symStorage));` needed to be rewritten as 
`CVSymbol sym{ArrayRef(symStorage)};` otherwise the compiler is confused and 
thinks we have a (bad) function prototype. There was a few similar situation 
across the codebase.
4. ADL doesn't seem to work the same for deductio-guides and functions, so at 
some point the `llvm` namespace must be explicitly stated.
5. The "reference mode" of `makeArrayRef(ArrayRef &)` that acts as no-op is 
not supported (a constructor cannot achieve that).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140896

Files:
  bolt/tools/bat-dump/bat-dump.cpp
  bolt/tools/driver/llvm-bolt.cpp
  bolt/tools/heatmap/heatmap.cpp
  clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
  clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp
  clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.cpp
  clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/CompileCommands.cpp
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
  clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
  clang-tools-extra/pseudo/include/clang-pseudo/Forest.h
  clang-tools-extra/pseudo/include/clang-pseudo/GLR.h
  clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h
  clang-tools-extra/pseudo/lib/Forest.cpp
  clang-tools-extra/pseudo/lib/GLR.cpp
  clang-tools-extra/pseudo/lib/grammar/Grammar.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/AST/DeclOpenMP.h
  clang/includ

[PATCH] D140807: [clang][Interp] Skip calling simple destructors

2023-01-03 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

This still would have to call the destructors of base and member types, right?  
And this just means that it isn't user defined AND isn't deleted for some 
reason, so this doesn't mean 'trivial'.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140807

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


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

2023-01-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments.



Comment at: clang/include/clang/AST/Expr.h:1447
+  bool Set, const ASTContext &Context) {
+DeclRefExprBits.CapturedByCopyInLambdaWithExplicitObjectParameter = Set;
+setDependence(computeDependence(this, Context));

erichkeane wrote:
> This bit concerns me a touch, I'm a little scared/shocked that this would 
> matter at all... This makes me think more nefarious issues are at play here.
See https://eel.is/c++draft/temp.dep.expr#3.6 
I think this is because we don't know if the capture is mutable until the type 
of the object parameter is known.
I still need to find a way to devise a test for that though.

The way we do that is we save that bit and then in `computeDependence` we apply 
that rule.
(This bit doesn't need to be serialized once the dependence is computed)



Comment at: clang/lib/AST/MicrosoftMangle.cpp:2560
   IsInLambda = true;
-if (MD->isInstance())
+if (MD->isImplicitObjectMemberFunction())
   HasThisQuals = true;

erichkeane wrote:
> This bit probably  needs to wait until microsoft decides what the mangling 
> for these are (all of microsoft mangle)
I've been chatting with Cameron, microsoft has decided :)
I still have to hear back from the GCC/Itanium folks though



Comment at: clang/lib/CodeGen/CGExpr.cpp:2633
+  return CGF.EmitLValueForLambdaField(FD, ThisValue);
+  // QualType TagType = CGF.getContext().getTagDeclType(FD->getParent());
+  // LValue LV = CGF.MakeNaturalAlignAddrLValue(ThisValue, TagType);

erichkeane wrote:
> Eh?
I need to remove this commented code.
the removed code was just moved around. we had 2 functions doing the same 
thing, now this one just forward to the non-static one.
I should get rid of it entirely actually.



Comment at: clang/lib/CodeGen/CGExpr.cpp:4254
+ llvm::Value *ThisValue) {
+  bool HasExplicitObjectParameter = false;
+  if (const CXXMethodDecl *MD =

erichkeane wrote:
> If there is a CodeGen expert in the house, I'd really hope they go through 
> this :) 
Oh, me too!
I need to properly write tests for it too. And you know how terrible I am at 
code gen tests...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140828

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


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

2023-01-03 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/lib/AST/MicrosoftMangle.cpp:2560
   IsInLambda = true;
-if (MD->isInstance())
+if (MD->isImplicitObjectMemberFunction())
   HasThisQuals = true;

cor3ntin wrote:
> erichkeane wrote:
> > This bit probably  needs to wait until microsoft decides what the mangling 
> > for these are (all of microsoft mangle)
> I've been chatting with Cameron, microsoft has decided :)
> I still have to hear back from the GCC/Itanium folks though
Ah! Good!  See whatever you can get to make sure we match though!

As far as Itanium, did you try posting on the Itanium github?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140828

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


[PATCH] D140894: [clang-tidy] Don't emit misc-unused-using-decl warnings for header files.

2023-01-03 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc/unused-using-decls.rst:26
+   A semicolon-separated list of filename extensions of header files (the 
filename
+   extensions should not include "." prefix). Default is ";h;hh;hpp;hxx".
+   For extension-less header files, using an empty string or leaving an

Please use single back-ticks for option values.

Actually this is second patch with similar functionality during last month and 
I think this option should be shared between all checks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140894

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


[PATCH] D140387: [clang][analyzer] Add stream related functions to StdLibraryFunctionsChecker.

2023-01-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:838
+  Result += getArgDesc(ArgN);
+  Result += DK == Violation ? " should not be zero" : " is not zero";
+  return Result.c_str();

I don't mean to make you test every single `Case` or `ArgumentConstraint` you 
added, but `NotZeroConstraint` is brand new, and is not tested anywhere.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1105-1113
+  // It is possible that the function was evaluated in a checker callback
+  // where the state constraints are already applied, then no change 
happens
+  // here to the state (if the ErrnoConstraint did not change it either).
+  // If the evaluated function requires a NoteTag for errno change, it is
+  // added here.
+  if (const auto *D = dyn_cast_or_null(Call.getDecl()))
+if (const NoteTag *NT =

Can you name an example for that? `fgetpos`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140387

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


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

2023-01-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments.



Comment at: clang/lib/AST/MicrosoftMangle.cpp:2560
   IsInLambda = true;
-if (MD->isInstance())
+if (MD->isImplicitObjectMemberFunction())
   HasThisQuals = true;

erichkeane wrote:
> cor3ntin wrote:
> > erichkeane wrote:
> > > This bit probably  needs to wait until microsoft decides what the 
> > > mangling for these are (all of microsoft mangle)
> > I've been chatting with Cameron, microsoft has decided :)
> > I still have to hear back from the GCC/Itanium folks though
> Ah! Good!  See whatever you can get to make sure we match though!
> 
> As far as Itanium, did you try posting on the Itanium github?
I wrote the tests from the microsoft implementation 
https://compiler-explorer.com/z/4df8f3e1j 
The github issue is there https://github.com/itanium-cxx-abi/cxx-abi/issues/148 
- I also mailed a few folks this morning


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140828

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


[PATCH] D140617: [clangd] Fix a clangd crash when indexing the standard library.

2023-01-03 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

As I understand, we're crashing if the ignorelist isn't found, and this patch 
fixes the searching so that we find the file (assuming the compile command is 
correct).

But we'll still crash if the compile command is wrong, which seems pretty bad.

Fixing the way the ignorelist is populated in clang would be nice but is a bit 
of a yakshave.
Failing that, I think in this case we can just clear out the langopt in 
`disableUnsupportedOptions()`. This seems much more robust (though doing both 
is also fine)

Most of the uses of the asan ignorelist are in codegen which we don't care 
about.
It also affects record layout: see RecordDecl::mayInsertExtraPadding. We do 
calculate and show record layouts on hover, but I don't think we should care 
about ignorelists when doing so - people almost certainly really care about the 
non-asan size.
(In practice, I couldn't actually see any asan-specific differences through 
clangd in casual inspection).




Comment at: clang-tools-extra/clangd/index/StdLib.cpp:227
   CI->getPreprocessorOpts().clearRemappedFiles();
+  std::string CWD = CI->getFileSystemOpts().WorkingDir;
   auto Clang = prepareCompilerInstance(

I can't see where this is set in clang itself - I thought it was just a 
customization point for programmatic use. However the fix works, so it seems 
like it must be getting the correct value... very misterious


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140617

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


[PATCH] D140395: [clang][analyzer] Extend StreamChecker with some new functions.

2023-01-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

Mostly LGTM, but I see that you have tests for the predecessor patch here as 
well, so I'll accept both at once.




Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:89
   /// The last file operation called in the stream.
+  /// Can be nullptr.
   const FnDescription *LastOperation;

When can this occur?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140395

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


[PATCH] D140897: [clang][dataflow] Fix handling of `DeclRefExpr`s to `BindingDecl`s.

2023-01-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision.
ymandel added reviewers: xazax.hun, gribozavr2.
Herald added subscribers: martong, rnkovacs.
Herald added a reviewer: NoQ.
Herald added a project: All.
ymandel requested review of this revision.
Herald added a project: clang.

The invariants around `ReferenceValues` are subtle (arguably, too much so). That
includes that we need to take care not to double wrap them -- in cases where we
wrap a loc in an `ReferenceValue` we need to be sure that the pointee isn't
already a `ReferenceValue`.  `BindingDecl` introduces another situation in which
this can arise. Previously, the code did not properly handle `BindingDecl`,
resulting in double-wrapped values, which broke other invariants (at least, that
struct values have an `AggregateStorageLocation`).

This patch adjusts the interpretation of `DeclRefExpr` to take `BindingDecl`'s
peculiarities into account. It also fixes the two tests which should have caught
this issue but were themselves (subtly) buggy.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140897

Files:
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -3673,6 +3673,8 @@
 const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
 ASSERT_THAT(BazDecl, NotNull());
 
+// BindingDecls always map to references -- either lvalue or rvalue, so
+// we still need to skip here.
 const Value *BoundFooValue =
 Env1.getValue(*BoundFooDecl, SkipPast::Reference);
 ASSERT_THAT(BoundFooValue, NotNull());
@@ -3684,13 +3686,13 @@
 EXPECT_TRUE(isa(BoundBarValue));
 
 // Test that a `DeclRefExpr` to a `BindingDecl` works as expected.
-EXPECT_EQ(Env1.getValue(*BazDecl, SkipPast::Reference), BoundFooValue);
+EXPECT_EQ(Env1.getValue(*BazDecl, SkipPast::None), BoundFooValue);
 
 const Environment &Env2 = getEnvironmentAtAnnotation(Results, "p2");
 
 // Test that `BoundFooDecl` retains the value we expect, after the join.
 BoundFooValue = Env2.getValue(*BoundFooDecl, SkipPast::Reference);
-EXPECT_EQ(Env2.getValue(*BazDecl, SkipPast::Reference), BoundFooValue);
+EXPECT_EQ(Env2.getValue(*BazDecl, SkipPast::None), BoundFooValue);
   });
 }
 
@@ -3768,16 +3770,15 @@
 // works as expected. We don't test aliasing properties of the
 // reference, because we don't model `std::get` and so have no way to
 // equate separate references into the tuple.
-EXPECT_EQ(Env1.getValue(*BazDecl, SkipPast::Reference), BoundFooValue);
+EXPECT_EQ(Env1.getValue(*BazDecl, SkipPast::None), BoundFooValue);
 
 const Environment &Env2 = getEnvironmentAtAnnotation(Results, "p2");
 
 // Test that `BoundFooDecl` retains the value we expect, after the join.
 BoundFooValue = Env2.getValue(*BoundFooDecl, SkipPast::Reference);
-EXPECT_EQ(Env2.getValue(*BazDecl, SkipPast::Reference), BoundFooValue);
+EXPECT_EQ(Env2.getValue(*BazDecl, SkipPast::None), BoundFooValue);
   });
 }
-// TODO: ref binding
 
 TEST(TransferTest, BinaryOperatorComma) {
   std::string Code = R"(
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -108,7 +108,8 @@
 }
 
 // Unpacks the value (if any) associated with `E` and updates `E` to the new
-// value, if any unpacking occured.
+// value, if any unpacking occured. Also, does the lvalue-to-rvalue conversion,
+// by skipping past the reference.
 static Value *maybeUnpackLValueExpr(const Expr &E, Environment &Env) {
   auto *Loc = Env.getStorageLocation(E, SkipPast::Reference);
   if (Loc == nullptr)
@@ -145,7 +146,9 @@
   if (LHSLoc == nullptr)
 break;
 
-  auto *RHSVal = Env.getValue(*RHS, SkipPast::Reference);
+  // No skipping should be necessary, because any lvalues should have
+  // already been stripped off in evaluating the LValueToRValue cast.
+  auto *RHSVal = Env.getValue(*RHS, SkipPast::None);
   if (RHSVal == nullptr)
 break;
 
@@ -195,7 +198,15 @@
 if (DeclLoc == nullptr)
   return;
 
-if (VD->getType()->isReferenceType()) {
+// If the value is already an lvalue, don't double-wrap it.
+if (isa_and_nonnull(Env.getValue(*DeclLoc))) {
+  // We only expect to encounter a `ReferenceValue` for a reference type
+  // (always) or for `BindingDecl` (sometimes). For the latter, we can't
+  // rely on type, because their type does not indicate whether they are a
+  // reference type. The assert is not strictly necessary, since w

[PATCH] D140807: [clang][Interp] Skip calling simple destructors

2023-01-03 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment.

What a shame. :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140807

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


[PATCH] D140896: [WIP] Move from llvm::makeArrayRef to ArrayRef deduction guides

2023-01-03 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added inline comments.
Herald added a subscriber: Michael137.



Comment at: llvm/include/llvm/ADT/ArrayRef.h:502
+  /// Deduction guide to construct an ArrayRef from a C array.
+  template  ArrayRef(const T (&Arr)[N]) -> ArrayRef;
 

Can we keep the makeArrayRef functions for now and mark them deprecated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140896

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


[PATCH] D140598: [Clang] Add sanity check in Sema::getDestructorName to prevent nullptr dereference

2023-01-03 Thread Tom Honermann via Phabricator via cfe-commits
tahonermann added inline comments.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:393
 // reasonably apply this fallback for dependent nested-name-specifiers.
-if (SS.getScopeRep()->getPrefix()) {
+if (SS.isSet() && SS.getScopeRep()->getPrefix()) {
   if (ParsedType T = LookupInScope()) {

`CXXScopeSpec::isSet()` is apparently (intended to be) deprecated.
```
clang/include/clang/Sema/DeclSpec.h:
 209   /// Deprecated.  Some call sites intend isNotEmpty() while others intend
 210   /// isValid().
 211   bool isSet() const { return getScopeRep() != nullptr; }
```
It sounds like this should instead call `.isValid()` or `.isNotEmpty()`, but 
I'm not sure which.


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

https://reviews.llvm.org/D140598

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


[PATCH] D139564: clang: Don't emit "frame-pointer"="none"

2023-01-03 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added a comment.

ping


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

https://reviews.llvm.org/D139564

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


[PATCH] D140547: Perform access checking to private members in simple requirement.

2023-01-03 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1367
+  if (E->getBody()->isDependentContext()) {
+Sema::SFINAETrap Trap(SemaRef);
+// We recreate the RequiresExpr body, but not by instantiating it.

Other uses of `PerformDependentDiagnostics` do not add an explicit `SFINAETrap` 
AFAICS.
Why is `RequiresExpr` special? Because it should "eat" the errors and only 
return a value?



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1374
+  }
   LocalInstantiationScope Scope(SemaRef, /*CombineWithOuterScope=*/true);
   return inherited::TransformRequiresExpr(E);

Other uses of `PerformDependentDiagnostics` happen inside 
`LocalInstantiationScope` and after substitution of inner parts.

I suggest following this pattern and moving the added code after the call to 
`inherited::TransformRequiresExpr`.
I do not have any concrete examples where the current approach fails, but it's 
better to have a single mode of operation across all opeartions.



Comment at: 
clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp:157
+static_assert(A<0>::faz());
+}

Could you add a check that in the following case we mention access check in the 
note to the `no matching function to call` error?

```
template  struct Use;

class X { int a; friend struct Use; };

template  struct Use {
  static void foo() requires (requires (X x) { x.a; }) {
  }
};

void test() {
Use::foo();
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140547

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


[PATCH] D140894: [clang-tidy] Don't emit misc-unused-using-decl warnings for header files.

2023-01-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel accepted this revision.
ymandel added a comment.
This revision is now accepted and ready to land.

Thanks!




Comment at: clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp:48
+  HeaderFileExtensions,
+  utils::defaultFileExtensionDelimiters())) {
+this->configurationDiag("Invalid header file extension: '%0'")

nit: no need for braces on the if statement.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc/unused-using-decls.rst:26
+   A semicolon-separated list of filename extensions of header files (the 
filename
+   extensions should not include "." prefix). Default is ";h;hh;hpp;hxx".
+   For extension-less header files, using an empty string or leaving an

Eugene.Zelenko wrote:
> Please use single back-ticks for option values.
> 
> Actually this is second patch with similar functionality during last month 
> and I think this option should be shared between all checks.
Eugene -- are you suggesting that is a blocker on this patch, or just a good 
idea for clang-tidy in general?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc/unused-using-decls.rst:27
+   extensions should not include "." prefix). Default is ";h;hh;hpp;hxx".
+   For extension-less header files, using an empty string or leaving an
+   empty string between ";" if there are other filename extensions.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140894

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


[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

2023-01-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment.

@aaron.ballman I don't think this patch is no longer necessary given that we 
merged the math profile


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132877

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


[PATCH] D140894: [clang-tidy] Don't emit misc-unused-using-decl warnings for header files.

2023-01-03 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc/unused-using-decls.rst:26
+   A semicolon-separated list of filename extensions of header files (the 
filename
+   extensions should not include "." prefix). Default is ";h;hh;hpp;hxx".
+   For extension-less header files, using an empty string or leaving an

ymandel wrote:
> Eugene.Zelenko wrote:
> > Please use single back-ticks for option values.
> > 
> > Actually this is second patch with similar functionality during last month 
> > and I think this option should be shared between all checks.
> Eugene -- are you suggesting that is a blocker on this patch, or just a good 
> idea for clang-tidy in general?
Sure, it'll be better to do this as separate patch, but this should be done at 
some point. It;s both code duplication and more complex complex setup for users.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140894

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


[PATCH] D140876: [clang][C++20] Non-dependent access checks in requires expression.

2023-01-03 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

Should access checks should happen in the context where `concept` is written or 
where it's used? Is there a standard wording around it?
If access-checking should happen where concept is defined, having a hard error 
seems fine because of the wording you quoted:

> If the substitution of template arguments into a requirement would always 
> result in a substitution failure, the program is ill-formed; no diagnostic 
> required.

The program is ill-formed and we show the diagnostic even though it's not 
required.

I poked around and found an interesting case where GCC seems to be doing the 
wrong thing:

  template  struct B;
  class A {
 static void f();
 friend struct B;
  };
   
  template  struct B {
  static constexpr int index() requires requires{ A::f(); } {
  return 1;
  }
  static constexpr int index() {
  return 2;
  }
  };
  
  static_assert(B::index() == 1); // GCC picks 1, MSVC picks 1.
  static_assert(B::index() == 2);   // GCC (incorrectly?) picks 1, MSVC 
picks 2!

Is this related to this change? Could we add a test that validates Clang is 
doing the right thing?




Comment at: clang/lib/Parse/ParseExprCXX.cpp:3510
   Actions, Sema::ExpressionEvaluationContext::Unevaluated);
+  Sema::ContextRAII SaveContext(Actions, Actions.CurContext);
 

Could you explain how this changes behaviour and how it leads to fixing the 
issue?

I'm sure there is quite a bit of thought and debugging behind this one-line 
change, but it's not evident by just looking at it how it solves the issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140876

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


[PATCH] D140664: [Sema] Don't mark deleted special member functions as non-trivial

2023-01-03 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision.
erichkeane added a comment.
This revision is now accepted and ready to land.

Code changes LGTM.  Dead CFoo obviously should be used or deleted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140664

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


[PATCH] D139889: [Clang] Fix a crash when encountering an ill-formed delimited UCN.

2023-01-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment.

@tahonermann gentle ping :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139889

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


[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

2023-01-03 Thread Tom Honermann via Phabricator via cfe-commits
tahonermann added a comment.

> I don't think this patch is no longer necessary given that we merged the math 
> profile

I agree; we can revisit this if complaints from users due to use of characters 
not in the math profile materializes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132877

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


[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

2023-01-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman abandoned this revision.
aaron.ballman added a comment.

In D132877#4023269 , @cor3ntin wrote:

> @aaron.ballman I don't think this patch is no longer necessary given that we 
> merged the math profile

Agreed, I'm going to abandon this for now. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132877

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


[PATCH] D139889: [Clang] Fix a crash when encountering an ill-formed delimited UCN.

2023-01-03 Thread Tom Honermann via Phabricator via cfe-commits
tahonermann accepted this revision.
tahonermann added a comment.
This revision is now accepted and ready to land.

Looks good! Sorry, Corentin, this review had fallen off my radar!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139889

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


[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

2023-01-03 Thread Tom Honermann via Phabricator via cfe-commits
tahonermann added a comment.

I don't know the driver code very well, but as best I can tell, this appears to 
match the design agreed to at the last Clang Modules WG meeting.




Comment at: clang/test/Driver/module-output.cppm:5
+//
+// Tests that the .pcm file will be generated in the same direcotry with the 
specified
+// output and the name of the .pcm file should be the same with the input file.





Comment at: clang/test/Driver/module-output.cppm:18
+// RUN: %clang %t/Hello.cppm -fmodule-output -arch i386 -arch x86_64 -### 
-target \
+// RUN:   x86_64-apple-darwin 2>&1 | FileCheck %t/Hello.cppm 
-check-prefix=MULTIPLR-ARCH
+





Comment at: clang/test/Driver/module-output.cppm:26
+
+// MULTIPLR-ARCH: option '-fmodule-output' can't be used with multiple arch 
options
+




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

https://reviews.llvm.org/D137058

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


[PATCH] D140387: [clang][analyzer] Add stream related functions to StdLibraryFunctionsChecker.

2023-01-03 Thread Balázs Kéri via Phabricator via cfe-commits
balazske marked 5 inline comments as done.
balazske added a comment.

> I suppose the tests are done in the followup patch mostly?

Yes the tests in the next patch should cover the cases.




Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:838
+  Result += getArgDesc(ArgN);
+  Result += DK == Violation ? " should not be zero" : " is not zero";
+  return Result.c_str();

Szelethus wrote:
> I don't mean to make you test every single `Case` or `ArgumentConstraint` you 
> added, but `NotZeroConstraint` is brand new, and is not tested anywhere.
This new class is probably not needed at all. It is always possible to use 
`ReturnValueCondition(WithinRange, SingleValue(0))` or similar for nonzero 
cases.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1105-1113
+  // It is possible that the function was evaluated in a checker callback
+  // where the state constraints are already applied, then no change 
happens
+  // here to the state (if the ErrnoConstraint did not change it either).
+  // If the evaluated function requires a NoteTag for errno change, it is
+  // added here.
+  if (const auto *D = dyn_cast_or_null(Call.getDecl()))
+if (const NoteTag *NT =

Szelethus wrote:
> Can you name an example for that? `fgetpos`?
`fgetpos` is a good example, the return value is already bound and a state 
split for success and failure was performed in the `evalCall` function (in 
`StreamChecker`) and the previous line `Case.getErrnoConstraint().apply` did 
not create a new state (this is true for `fgetpos` because it has a 
`ErrnoUnchanged` condition in the summary).



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1848-1849
+// int fseek(FILE *stream, long offset, int whence);
+// FIXME: It is possible to get the 'SEEK_' values (like EOFv) for arg 2
+// condition.
+addToFunctionSummaryMap(

Szelethus wrote:
> Sure, but what should be fixed? We should check whether the the last argument 
> is a `SEEK_*` value?
It could be an improvement to detect possible values for the argument 2 that 
are the `SEEK_*` values. Now the range [0,2] is used (the `SEEK_*` constants 
are usually 0,1,2).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140387

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


[PATCH] D139114: [Clang][Sema] Enabled Wshorten-64-to-32 for CompoundAssignment operator.

2023-01-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a reviewer: libc++.
aaron.ballman added a comment.

It looks like this change breaks libc++ (see the precommit CI failures) by 
making more changes than expected. I'm now seeing `implicit conversion changes 
signedness` diagnostics where we didn't previously get them. Is that expected 
and intentional? (I think it may be a fix: https://godbolt.org/z/hTaaf8c5P so 
I'm adding the libc++ folks just in case they disagree.)

Also, these changes should come with an entry in the release notes.




Comment at: clang/lib/Sema/SemaChecking.cpp:13400-13401
 
+  // Check for implicit conversion loss of precision form 64-to-32 for compound
+  // statements.
+  if (E->getLHS()->getType()->isIntegerType() &&

This comment isn't quite accurate, right? It's checking for any kind of 
implicit conversion issue (such as changing signs even if the integer widths 
stay the same).



Comment at: clang/test/Sema/conversion-64-32.c:22
+// rdar://10466193
+void test3(int i, long long ll) {
+  i += ll; // expected-warning {{implicit conversion loses integer precision}}

Can you also add tests for the shift assign behavior, since that's being 
handled in a special way?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139114

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


[PATCH] D140711: [clang] Fix unused variable warning in SemaConcept.cpp

2023-01-03 Thread Victor K via Phabricator via cfe-commits
vctr added a comment.

Hey @hokein, thanks for the accept. I don't have commit rights, can you please 
land this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140711

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


[PATCH] D137756: [z/OS][pg] Throw error when using -pg on z/OS

2023-01-03 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment.

In D137756#4018666 , @MaskRay wrote:

> Most targets reject `-p` now. It's unnecessary to have another z/OS specific 
> diagnostic. So this patch can be abandoned.
>
>   % fclang -p a.cc
>   clang-16: error: unsupported option '-p' for target 
> 'x86_64-unknown-linux-gnu'

@francii, if you can confirm:

1. that there is a test that covers that error for z/OS, and
2. any comments in the code associated with generating the error above do not 
misrepresent the rationale for why `-p` is unsupported on z/OS,

then I'm good with leaving it at that.

Otherwise, we should make this into an NFC patch that adds the test/adjusts the 
comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137756

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


[PATCH] D140894: [clang-tidy] Don't emit misc-unused-using-decl warnings for header files.

2023-01-03 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc/unused-using-decls.rst:26
+   A semicolon-separated list of filename extensions of header files (the 
filename
+   extensions should not include "." prefix). Default is ";h;hh;hpp;hxx".
+   For extension-less header files, using an empty string or leaving an

Eugene.Zelenko wrote:
> ymandel wrote:
> > Eugene.Zelenko wrote:
> > > Please use single back-ticks for option values.
> > > 
> > > Actually this is second patch with similar functionality during last 
> > > month and I think this option should be shared between all checks.
> > Eugene -- are you suggesting that is a blocker on this patch, or just a 
> > good idea for clang-tidy in general?
> Sure, it'll be better to do this as separate patch, but this should be done 
> at some point. It;s both code duplication and more complex complex setup for 
> users.
I'm planning to add the suggested functionality in a separate patch when I find 
some time. Can fix existing checks as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140894

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


[PATCH] D138807: [RISCV] Support vector crypto extension ISA string and assembly

2023-01-03 Thread Eric Gouriou via Phabricator via cfe-commits
ego added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td:135
+  defm VANDN_V : VALU_IV_V_X_I<"vandn", 0b01>;
+  def VBREV8_V : VALUVs2<0b010010, 0b01000, OPIVV, "vbrev8.v">;
+  defm VCLMUL_V : VALU_IV_V_X_VCLMUL<"vclmul", 0b001100>;

This ought to be OPMVV.

https://github.com/riscv/riscv-crypto/blob/master/doc/vector/insns/vbrev8.adoc

Doing OPIVV->OPMVV sets bit 13, adding 0x20 to the second byte in the 
CHECK-ENCODING tests for vbrev8 and vrev8

```
vbrev8.v v10, v9, v0.t
# CHECK-INST: vbrev8.v v10, v9, v0.t
# CHECK-ENCODING: [0x57,0x25,0x94,0x48]
# CHECK-ERROR: instruction requires the following: 'Zvkb'
# CHECK-UNKNOWN: 57 25 94 48   
...
vrev8.v v10, v9, v0.t
# CHECK-INST: vrev8.v v10, v9, v0.t
# CHECK-ENCODING: [0x57,0xa5,0x94,0x48]
# CHECK-ERROR: instruction requires the following: 'Zvkb'
# CHECK-UNKNOWN: 57 a5 94 48   

```

diff:
```
GIT_PAGER= git diff   
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td 
b/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td
index b44bf7476808..3477c8d860ac 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td
@@ -132,10 +132,10 @@ def rnum_2_14 : Operand, ImmLeaf;
-  def VBREV8_V : VALUVs2<0b010010, 0b01000, OPIVV, "vbrev8.v">;
+  def VBREV8_V : VALUVs2<0b010010, 0b01000, OPMVV, "vbrev8.v">;
   defm VCLMUL_V : VALU_IV_V_X_VCLMUL<"vclmul", 0b001100>;
   defm VCLMULH_V : VALU_IV_V_X_VCLMUL<"vclmulh", 0b001101>;
-  def VREV8_V : VALUVs2<0b010010, 0b01001, OPIVV, "vrev8.v">;
+  def VREV8_V : VALUVs2<0b010010, 0b01001, OPMVV, "vrev8.v">;
   defm VROL_V : VALU_IV_V_X<"vrol", 0b010101>;
   defm VROR_V : VALU_IV_V_X_I_VROR<"vror", 0b010100>;
 } // Predicates = [HasStdExtZvkb]
diff --git a/llvm/test/MC/RISCV/rvv/rv64zvkb.s 
b/llvm/test/MC/RISCV/rvv/rv64zvkb.s
index f4f7d9a038fc..d1600296e7e6 100644
--- a/llvm/test/MC/RISCV/rvv/rv64zvkb.s
+++ b/llvm/test/MC/RISCV/rvv/rv64zvkb.s
@@ -28,9 +28,9 @@ vandn.vi v10, v9, 7, v0.t

 vbrev8.v v10, v9, v0.t
 # CHECK-INST: vbrev8.v v10, v9, v0.t
-# CHECK-ENCODING: [0x57,0x05,0x94,0x48]
+# CHECK-ENCODING: [0x57,0x25,0x94,0x48]
 # CHECK-ERROR: instruction requires the following: 'Zvkb'
-# CHECK-UNKNOWN: 57 05 94 48   
+# CHECK-UNKNOWN: 57 25 94 48   
 
 vclmul.vv v10, v9, v8
 # CHECK-INST: vclmul.vv v10, v9, v8
@@ -58,9 +58,9 @@ vclmulh.vx v10, v9, a0

 vrev8.v v10, v9, v0.t
 # CHECK-INST: vrev8.v v10, v9, v0.t
-# CHECK-ENCODING: [0x57,0x85,0x94,0x48]
+# CHECK-ENCODING: [0x57,0xa5,0x94,0x48]
 # CHECK-ERROR: instruction requires the following: 'Zvkb'
-# CHECK-UNKNOWN: 57 85 94 48   
+# CHECK-UNKNOWN: 57 a5 94 48   
 
 vrol.vv v10, v9, v8, v0.t
 # CHECK-INST: vrol.vv v10, v9, v8, v0.t

```



Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td:138
+  defm VCLMULH_V : VALU_IV_V_X_VCLMUL<"vclmulh", 0b001101>;
+  def VREV8_V : VALUVs2<0b010010, 0b01001, OPIVV, "vrev8.v">;
+  defm VROL_V : VALU_IV_V_X<"vrol", 0b010101>;

This ought to be OPMVV.

https://github.com/riscv/riscv-crypto/blob/master/doc/vector/insns/vrev8.adoc



Comment at: llvm/test/MC/RISCV/rvv/rv64zvknh.s:19-20
+vsha2ms.vv v10, v9, v8
+# CHECK-INST-ZVKNHA: vsha2ms.vv v10, v9, v8
+# CHECK-INST-ZVKNHB: vsha2ms.vv v10, v9, v8
+# CHECK-ENCODING-ZVKNHA: [0x77,0x25,0x94,0xb6]

While I do understand why we want to run the tests with each of the two 
extensions declared, I don't understand the value of replicating the checks in 
the cases where there is no difference between A and B. I do not see any case 
where the expectations differ since the instructions exist in both cases, have 
the same encodings, and the error string covers both extensions.

If we want to communicate that both behave the same, a comment would convey 
this more clearly.



Comment at: llvm/test/MC/RISCV/rvv/rv64zvkns.s:59
+
+vaeskf1.vi v10, v9, 1
+# CHECK-INST: vaeskf1.vi v10, v9, 1

craig.topper wrote:
> ego wrote:
> > If I replaces "v10" with "v0", the test fails with an assertion failure. My 
> > own patch uses a slightly different class hierarchy but hits the same 
> > assertion.
> > 
> > 
> > ```
> > FAIL: LLVM :: MC/RISCV/rvv/rv64zvkns.s (3 of 8)
> >  TEST 'LLVM :: MC/RISCV/rvv/rv64zvkns.s' FAILED 
> > 
> > Script:
> > --
> > : 'RUN: at line 1';   
> > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/llvm-mc 
> > -triple=riscv64 -show-encoding --mattr=+zve32x --mattr=+experimental-zvkns 
> > /Users/ego/root-dirs/scratch/projects/llvm-project/llvm/test/MC/RISCV/rvv/rv64zvkns.s
> >  | 
> > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/FileCheck 
> > /Users/ego/root-dirs/scratch/projects/llvm-project/llvm/test/MC/RISCV/rvv/rv64zvkns.s
> >  --check-prefixes=CHECK-ENCODING,CHECK-INST
> > : 'RUN: at line 3';   not 
> > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/llvm

[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

2023-01-03 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCV.td:572
+  string default_march = "",
+  list tunef = []> :  
ProcessorModel {
+  string DefaultMarch = default_march;

80 columns



Comment at: llvm/lib/Target/RISCV/RISCV.td:623
+  
FeatureStdExtC],
+ "rv32imafc", [TuneSiFive7]>;
+

The formatting is inconsisent. Sometimes the "rv32imafc" is on the end of the 
previous line. Can we be consistent?



Comment at: llvm/lib/TargetParser/RISCVTargetParser.cpp:1
+//===-- TargetParser - Parser for target features ---*- C++ 
-*-===//
+//

This comment should match the name of the file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137517

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


[PATCH] D140876: [clang][C++20] Non-dependent access checks in requires expression.

2023-01-03 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment.

In D140876#4023286 , @ilya-biryukov 
wrote:

> Should access checks should happen in the context where `concept` is written 
> or where it's used? Is there a standard wording around it?

Unfortunately, only a standard wording issue :(
https://cplusplus.github.io/CWG/issues/2589.html

We're also allowed to cache atomic constraints regardless of the their 
(unspecified) scope, which makes this a bit more complicated.

There's some discussion about this in 
https://github.com/llvm/llvm-project/issues/58672


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140876

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


[PATCH] D139705: [clang] fix zero-initialization fix-it for variable template

2023-01-03 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

As far as teh fix itself, I think this is fine.  BUT i think there is value in 
waiting for Aaron to see if there is a deeper issue here.




Comment at: clang/lib/Sema/SemaInit.cpp:3863
 
+  SourceLocation EndLoc = VD->getEndLoc();
+  if (const auto *VTSD = dyn_cast(VD)) {

Hmm... it is strange to me that the variables 'endloc' is the end of the 
identifier and not the end of the variable declaration.  I unfortunately don't 
have a good feeling as to the 'correct' behavior for that (Aaron is typically 
the one who understands source locations better than me!), so hopefully he can 
come along and see.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139705

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


[clang] d862f66 - [clang][dataflow] Treat unions as structs.

2023-01-03 Thread Dani Ferreira Franco Moura via cfe-commits

Author: Dani Ferreira Franco Moura
Date: 2023-01-03T18:36:24Z
New Revision: d862f66221de1463ee7c92fe2e78445edb30a601

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

LOG: [clang][dataflow] Treat unions as structs.

This is a straightfoward way to handle unions in dataflow analysis. Without 
this change, nullability verification crashes on files that contain unions.

Reviewed By: gribozavr2, ymandel

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h 
b/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
index fdfd03129b81e..f7ea7eb174c54 100644
--- a/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
+++ b/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
@@ -65,6 +65,9 @@ class ScalarStorageLocation final : public StorageLocation {
 /// struct with public members. The child map is flat, so when used for a 
struct
 /// or class type, all accessible members of base struct and class types are
 /// directly accesible as children of this location.
+/// FIXME: Currently, the storage location of unions is modelled the same way 
as
+/// that of structs or classes. Eventually, we need to change this modelling so
+/// that all of the members of a given union have the same storage location.
 class AggregateStorageLocation final : public StorageLocation {
 public:
   explicit AggregateStorageLocation(QualType Type)

diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index b8e3e93390602..1a561b31aae9a 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -235,14 +235,12 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
 if (Parent->isLambda())
   MethodDecl = dyn_cast(Parent->getDeclContext());
 
+// FIXME: Initialize the ThisPointeeLoc of lambdas too.
 if (MethodDecl && !MethodDecl->isStatic()) {
   QualType ThisPointeeType = MethodDecl->getThisObjectType();
-  // FIXME: Add support for union types.
-  if (!ThisPointeeType->isUnionType()) {
-ThisPointeeLoc = &createStorageLocation(ThisPointeeType);
-if (Value *ThisPointeeVal = createValue(ThisPointeeType))
-  setValue(*ThisPointeeLoc, *ThisPointeeVal);
-  }
+  ThisPointeeLoc = &createStorageLocation(ThisPointeeType);
+  if (Value *ThisPointeeVal = createValue(ThisPointeeType))
+setValue(*ThisPointeeLoc, *ThisPointeeVal);
 }
   }
 
@@ -570,7 +568,7 @@ void Environment::setValue(const StorageLocation &Loc, 
Value &Val) {
 auto &AggregateLoc = *cast(&Loc);
 
 const QualType Type = AggregateLoc.getType();
-assert(Type->isStructureOrClassType());
+assert(Type->isStructureOrClassType() || Type->isUnionType());
 
 for (const FieldDecl *Field : getObjectFields(Type)) {
   assert(Field != nullptr);
@@ -684,7 +682,7 @@ Value *Environment::createValueUnlessSelfReferential(
 return &takeOwnership(std::make_unique(PointeeLoc));
   }
 
-  if (Type->isStructureOrClassType()) {
+  if (Type->isStructureOrClassType() || Type->isUnionType()) {
 CreatedValuesCount++;
 // FIXME: Initialize only fields that are accessed in the context that is
 // being analyzed.

diff  --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp 
b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 336de81b06538..32e9d6f12e44e 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -487,10 +487,6 @@ class TransferVisitor : public 
ConstStmtVisitor {
 if (BaseLoc == nullptr)
   return;
 
-// FIXME: Add support for union types.
-if (BaseLoc->getType()->isUnionType())
-  return;
-
 auto &MemberLoc = BaseLoc->getChild(*Member);
 if (MemberLoc.getType()->isReferenceType()) {
   Env.setStorageLocation(*S, MemberLoc);

diff  --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index b5e10b24fffb5..a9b1f42f20360 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1518,6 +1518,50 @@ TEST(TransferTest, ClassThisMember) {
   });
 }
 
+TEST(TransferTest, UnionThisMember) {
+  std::string Code = R"(
+union A {
+  int Foo;
+  int Bar;
+
+  void target() {
+(void)0; // [[p]]
+

[PATCH] D140696: [clang][dataflow] Treat unions as structs.

2023-01-03 Thread Dani Ferreira Franco Moura 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 rGd862f66221de: [clang][dataflow] Treat unions as structs. 
(authored by merrymeerkat).

Changed prior to commit:
  https://reviews.llvm.org/D140696?vs=485693&id=486025#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140696

Files:
  clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1518,6 +1518,50 @@
   });
 }
 
+TEST(TransferTest, UnionThisMember) {
+  std::string Code = R"(
+union A {
+  int Foo;
+  int Bar;
+
+  void target() {
+(void)0; // [[p]]
+  }
+};
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+const auto *ThisLoc = dyn_cast(
+Env.getThisPointeeStorageLocation());
+ASSERT_THAT(ThisLoc, NotNull());
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const auto *FooLoc =
+cast(&ThisLoc->getChild(*FooDecl));
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
+
+const Value *FooVal = Env.getValue(*FooLoc);
+ASSERT_TRUE(isa_and_nonnull(FooVal));
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *BarLoc =
+cast(&ThisLoc->getChild(*BarDecl));
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+ASSERT_TRUE(isa_and_nonnull(BarVal));
+  });
+}
+
 TEST(TransferTest, StructThisInLambda) {
   std::string ThisCaptureCode = R"(
 struct A {
@@ -2537,12 +2581,34 @@
 ASSERT_THAT(BazDecl, NotNull());
 ASSERT_TRUE(BazDecl->getType()->isUnionType());
 
+auto BazFields = BazDecl->getType()->getAsRecordDecl()->fields();
+FieldDecl *FooDecl = nullptr;
+for (FieldDecl *Field : BazFields) {
+  if (Field->getNameAsString() == "Foo") {
+FooDecl = Field;
+  } else {
+FAIL() << "Unexpected field: " << Field->getNameAsString();
+  }
+}
+ASSERT_THAT(FooDecl, NotNull());
+
 const auto *BazLoc = dyn_cast_or_null(
 Env.getStorageLocation(*BazDecl, SkipPast::None));
 ASSERT_THAT(BazLoc, NotNull());
+ASSERT_THAT(Env.getValue(*BazLoc), NotNull());
+
+const auto *BazVal = cast(Env.getValue(*BazLoc));
+const auto *FooValFromBazVal = cast(BazVal->getChild(*FooDecl));
+const auto *FooValFromBazLoc = cast(Env.getValue(BazLoc->getChild(*FooDecl)));
+EXPECT_EQ(FooValFromBazLoc, FooValFromBazVal);
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+const auto *BarLoc = Env.getStorageLocation(*BarDecl, SkipPast::None);
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
 
-// FIXME: Add support for union types.
-EXPECT_THAT(Env.getValue(*BazLoc), IsNull());
+EXPECT_EQ(Env.getValue(*BarLoc), FooValFromBazVal);
+EXPECT_EQ(Env.getValue(*BarLoc), FooValFromBazLoc);
   });
 }
 
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -487,10 +487,6 @@
 if (BaseLoc == nullptr)
   return;
 
-// FIXME: Add support for union types.
-if (BaseLoc->getType()->isUnionType())
-  return;
-
 auto &MemberLoc = BaseLoc->getChild(*Member);
 if (MemberLoc.getType()->isReferenceType()) {
   Env.setStorageLocation(*S, MemberLoc);
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -235,14 +235,12 @@
 if (Parent->isLambda())
   MethodDecl = dyn_cast(Parent->getDeclContext());
 
+// FIXME: Initialize the ThisPointeeLoc of lambdas too.
 if (MethodDecl && !MethodDecl->isStatic()) {
   QualType ThisPointeeType = MethodDecl->getThisObjectType();
-  // FIXME: Add support for union types.
-  if (!ThisPointeeTyp

[PATCH] D140696: [clang][dataflow] Treat unions as structs.

2023-01-03 Thread Dani Ferreira Franco Moura via Phabricator via cfe-commits
merrymeerkat added a comment.

Thanks for clarifying! I've gone ahead and landed the change. At least this 
should reduce the number of false negatives we get (hopefully without 
introducing too much complexity).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140696

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


[PATCH] D135488: [codegen] Add a remarks based Stack Layout Analysis pass

2023-01-03 Thread Paul Kirth via Phabricator via cfe-commits
paulkirth updated this revision to Diff 486030.
paulkirth added a comment.
Herald added subscribers: kosarev, kerbowa, jvesely, nemanjai.

Add missing pipline test updates for PowerPC and AMDGPU


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135488

Files:
  clang/test/Frontend/stack-layout-remark.c
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/InitializePasses.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/test/CodeGen/AArch64/O0-pipeline.ll
  llvm/test/CodeGen/AArch64/O3-pipeline.ll
  llvm/test/CodeGen/AArch64/arm64-opt-remarks-lazy-bfi.ll
  llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
  llvm/test/CodeGen/ARM/O3-pipeline.ll
  llvm/test/CodeGen/ARM/stack-frame-layout-remarks.ll
  llvm/test/CodeGen/Generic/llc-start-stop.ll
  llvm/test/CodeGen/PowerPC/O0-pipeline.ll
  llvm/test/CodeGen/PowerPC/O3-pipeline.ll
  llvm/test/CodeGen/RISCV/O0-pipeline.ll
  llvm/test/CodeGen/RISCV/O3-pipeline.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/opt-pipeline.ll
  llvm/test/CodeGen/X86/stack-frame-layout-remarks.ll

Index: llvm/test/CodeGen/X86/stack-frame-layout-remarks.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-frame-layout-remarks.ll
@@ -0,0 +1,303 @@
+; Test remark output for stack-frame-layout
+
+; ensure basic output works
+; RUN: llc -mcpu=corei7 -O1 -pass-remarks-analysis=stack-frame-layout < %s 2>&1 >/dev/null | FileCheck %s
+
+; check additional slots are displayed when stack is not optimized
+; RUN: llc -mcpu=corei7 -O0 -pass-remarks-analysis=stack-frame-layout < %s 2>&1 >/dev/null | FileCheck %s --check-prefix=NO_COLORING
+
+; check more complex cases
+; RUN: llc %s -pass-remarks-analysis=stack-frame-layout -o /dev/null --march=x86 -mcpu=i386 2>&1 | FileCheck %s --check-prefix=BOTH --check-prefix=DEBUG
+
+; check output without debug info
+; RUN: opt %s -passes=strip -S | llc  -pass-remarks-analysis=stack-frame-layout -o /dev/null --march=x86 -mcpu=i386 2>&1 | FileCheck %s --check-prefix=BOTH --check-prefix=STRIPPED
+
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK: FunctionName: stackSizeWarning
+; CHECK: Offset{{.*}}Align{{.*}}Size
+; CHECK: [SP-88]{{.*}}16{{.*}}80
+; CHECK:buffer @ frame-diags.c:30
+; NO_COLORING: [SP-168]{{.*}}16{{.*}}80
+; CHECK:buffer2 @ frame-diags.c:33
+define void @stackSizeWarning() {
+entry:
+  %buffer = alloca [80 x i8], align 16
+  %buffer2 = alloca [80 x i8], align 16
+  call void @llvm.dbg.declare(metadata ptr %buffer, metadata !25, metadata !DIExpression()), !dbg !39
+  call void @llvm.dbg.declare(metadata ptr %buffer2, metadata !31, metadata !DIExpression()), !dbg !40
+  ret void
+}
+
+; Function Attrs: nocallback nofree nosync nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #0
+
+; BOTH: FunctionName: cleanup_array
+; BOTH:  Offset{{.*}}Align{{.*}}Size
+; BOTH:  [SP+4]{{.+}}16{{.+}}4
+; DEBUG: a @ dot.c:13
+; STRIPPED-NOT: a @ dot.c:13
+; BOTH:  [SP-4]{{.+}}Spill 8{{.+}}4
+define void @cleanup_array(ptr %0) #1 {
+  %2 = alloca ptr, align 8
+  store ptr %0, ptr %2, align 8
+  call void @llvm.dbg.declare(metadata ptr %2, metadata !41, metadata !DIExpression()), !dbg !46
+  ret void
+}
+
+; BOTH: FunctionName: cleanup_result
+; BOTH:  Offset{{.+}}Align{{.+}}Size
+; BOTH:  [SP+4]{{.+}}16{{.+}}4
+; DEBUG: res @ dot.c:21
+; STRIPPED-NOT: res @ dot.c:21
+; BOTH:  [SP-4]{{.+}}Spill 8{{.+}}4
+define void @cleanup_result(ptr %0) #1 {
+  %2 = alloca ptr, align 8
+  store ptr %0, ptr %2, align 8
+  call void @llvm.dbg.declare(metadata ptr %2, metadata !47, metadata !DIExpression()), !dbg !51
+  ret void
+}
+
+; BOTH: FunctionName: do_work
+; BOTH:  Offset{{.+}}Align{{.+}}Size
+; BOTH:  [SP+12]{{.+}}8{{.+}}4
+; DEBUG: out @ dot.c:32
+; STRIPPED-NOT: out @ dot.c:32
+; BOTH:  [SP+8]{{.+}}4{{.+}}4
+; BOTH:  [SP+4]{{.+}}16{{.+}}4
+; DEBUG: A @ dot.c:32
+; STRIPPED-NOT: A @ dot.c:32
+; BOTH:  [SP-4]{{.+}}Spill 8{{.+}}4
+; BOTH:  [SP-12]{{.+}}8{{.+}}4
+; DEBUG: AB @ dot.c:38
+; STRIPPED-NOT: AB @ dot.c:38
+; BOTH:  [SP-16]{{.+}}4{{.+}}4
+; DEBUG: i @ dot.c:55
+; STRIPPED-NOT: i @ dot.c:55
+; BOTH:  [SP-20]{{.+}}8{{.+}}4
+; DEBUG: B @ dot.c:32
+; STRIPPED-NOT: B @ dot.c:32
+; BOTH:  [SP-24]{{.+}}4{{.+}}4
+; DEBUG: len @ dot.c:37
+; STRIPPED-NOT: len @ dot.c:37
+; BOTH:  [SP-28]{{.+}}4{{.+}}4
+; BOTH:  [SP-32]{{.+}}4{{.+}}4
+; DEBUG: sum @ dot.c:54
+; STRIPPED-NOT: sum @ dot.c:54
+define i32 @do_work(ptr %0, ptr %1, ptr %2) #1 {
+  %4 = alloca i32, align 4
+  %5 = alloca ptr, align 8
+  %6 = alloca ptr, align 8
+  %7 = alloca ptr, align 8
+  %8 = alloca i32, align 4
+  %9 = alloca ptr, align 8
+  %10 = alloca i32, align 4
+  %11 = alloca i32, align 4
+  store ptr %0, ptr %5, align 8
+  call void @llvm.dbg.declare(metadata ptr

[clang-tools-extra] dde8a0f - [clangd] show underlying type in type hint for `decltype(expr)`

2023-01-03 Thread Nathan Ridge via cfe-commits

Author: v1nh1shungry
Date: 2023-01-03T13:58:39-05:00
New Revision: dde8a0fe91ccd4b54d5bd0c18043e0dd35652e47

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

LOG: [clangd] show underlying type in type hint for `decltype(expr)`

Reviewed By: nridge

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

Added: 


Modified: 
clang-tools-extra/clangd/InlayHints.cpp
clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index 6bbad09b9f7bd..7af39658885be 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -662,7 +662,21 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
  sourceLocToPosition(SM, Spelled->back().endLocation())};
   }
 
+  static bool shouldPrintCanonicalType(QualType QT) {
+// The sugared type is more useful in some cases, and the canonical
+// type in other cases. For now, prefer the sugared type unless
+// we are printing `decltype(expr)`. This could be refined further
+// (see https://github.com/clangd/clangd/issues/1298).
+if (QT->isDecltypeType())
+  return true;
+if (const AutoType *AT = QT->getContainedAutoType())
+  if (AT->getDeducedType()->isDecltypeType())
+return true;
+return false;
+  }
+
   void addTypeHint(SourceRange R, QualType T, llvm::StringRef Prefix) {
+TypeHintPolicy.PrintCanonicalTypes = shouldPrintCanonicalType(T);
 addTypeHint(R, T, Prefix, TypeHintPolicy);
   }
 

diff  --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp 
b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index fa90abec02cbc..53621f97a6049 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1364,7 +1364,6 @@ TEST(TypeHints, Aliased) {
 TEST(TypeHints, Decltype) {
   assertTypeHints(R"cpp(
 $a[[decltype(0)]] a;
-// FIXME: will be nice to show `: int` instead
 $b[[decltype(a)]] b;
 const $c[[decltype(0)]] &c = b;
 
@@ -1377,11 +1376,13 @@ TEST(TypeHints, Decltype) {
 
 template  struct Foo;
 using G = Foo<$g[[decltype(0)]], float>;
+
+auto $h[[h]] = $i[[decltype(0)]]{};
   )cpp",
-  ExpectedHint{": int", "a"},
-  ExpectedHint{": decltype(0)", "b"},
+  ExpectedHint{": int", "a"}, ExpectedHint{": int", "b"},
   ExpectedHint{": int", "c"}, ExpectedHint{": int", "e"},
-  ExpectedHint{": int", "f"}, ExpectedHint{": int", "g"});
+  ExpectedHint{": int", "f"}, ExpectedHint{": int", "g"},
+  ExpectedHint{": int", "h"}, ExpectedHint{": int", "i"});
 }
 
 TEST(DesignatorHints, Basic) {



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


[PATCH] D140814: [clangd] show underlying type in type hint for `decltype(expr)`

2023-01-03 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdde8a0fe91cc: [clangd] show underlying type in type hint for 
`decltype(expr)` (authored by v1nh1shungry, committed by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140814

Files:
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1364,7 +1364,6 @@
 TEST(TypeHints, Decltype) {
   assertTypeHints(R"cpp(
 $a[[decltype(0)]] a;
-// FIXME: will be nice to show `: int` instead
 $b[[decltype(a)]] b;
 const $c[[decltype(0)]] &c = b;
 
@@ -1377,11 +1376,13 @@
 
 template  struct Foo;
 using G = Foo<$g[[decltype(0)]], float>;
+
+auto $h[[h]] = $i[[decltype(0)]]{};
   )cpp",
-  ExpectedHint{": int", "a"},
-  ExpectedHint{": decltype(0)", "b"},
+  ExpectedHint{": int", "a"}, ExpectedHint{": int", "b"},
   ExpectedHint{": int", "c"}, ExpectedHint{": int", "e"},
-  ExpectedHint{": int", "f"}, ExpectedHint{": int", "g"});
+  ExpectedHint{": int", "f"}, ExpectedHint{": int", "g"},
+  ExpectedHint{": int", "h"}, ExpectedHint{": int", "i"});
 }
 
 TEST(DesignatorHints, Basic) {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -662,7 +662,21 @@
  sourceLocToPosition(SM, Spelled->back().endLocation())};
   }
 
+  static bool shouldPrintCanonicalType(QualType QT) {
+// The sugared type is more useful in some cases, and the canonical
+// type in other cases. For now, prefer the sugared type unless
+// we are printing `decltype(expr)`. This could be refined further
+// (see https://github.com/clangd/clangd/issues/1298).
+if (QT->isDecltypeType())
+  return true;
+if (const AutoType *AT = QT->getContainedAutoType())
+  if (AT->getDeducedType()->isDecltypeType())
+return true;
+return false;
+  }
+
   void addTypeHint(SourceRange R, QualType T, llvm::StringRef Prefix) {
+TypeHintPolicy.PrintCanonicalTypes = shouldPrintCanonicalType(T);
 addTypeHint(R, T, Prefix, TypeHintPolicy);
   }
 


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1364,7 +1364,6 @@
 TEST(TypeHints, Decltype) {
   assertTypeHints(R"cpp(
 $a[[decltype(0)]] a;
-// FIXME: will be nice to show `: int` instead
 $b[[decltype(a)]] b;
 const $c[[decltype(0)]] &c = b;
 
@@ -1377,11 +1376,13 @@
 
 template  struct Foo;
 using G = Foo<$g[[decltype(0)]], float>;
+
+auto $h[[h]] = $i[[decltype(0)]]{};
   )cpp",
-  ExpectedHint{": int", "a"},
-  ExpectedHint{": decltype(0)", "b"},
+  ExpectedHint{": int", "a"}, ExpectedHint{": int", "b"},
   ExpectedHint{": int", "c"}, ExpectedHint{": int", "e"},
-  ExpectedHint{": int", "f"}, ExpectedHint{": int", "g"});
+  ExpectedHint{": int", "f"}, ExpectedHint{": int", "g"},
+  ExpectedHint{": int", "h"}, ExpectedHint{": int", "i"});
 }
 
 TEST(DesignatorHints, Basic) {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -662,7 +662,21 @@
  sourceLocToPosition(SM, Spelled->back().endLocation())};
   }
 
+  static bool shouldPrintCanonicalType(QualType QT) {
+// The sugared type is more useful in some cases, and the canonical
+// type in other cases. For now, prefer the sugared type unless
+// we are printing `decltype(expr)`. This could be refined further
+// (see https://github.com/clangd/clangd/issues/1298).
+if (QT->isDecltypeType())
+  return true;
+if (const AutoType *AT = QT->getContainedAutoType())
+  if (AT->getDeducedType()->isDecltypeType())
+return true;
+return false;
+  }
+
   void addTypeHint(SourceRange R, QualType T, llvm::StringRef Prefix) {
+TypeHintPolicy.PrintCanonicalTypes = shouldPrintCanonicalType(T);
 addTypeHint(R, T, Prefix, TypeHintPolicy);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138807: [RISCV] Support vector crypto extension ISA string and assembly

2023-01-03 Thread Eric Gouriou via Phabricator via cfe-commits
ego added a comment.

FYI, more issues with v0 not being accepted as an operand with unmasked 
instructions.




Comment at: llvm/test/MC/RISCV/rvv/rv64zvkns.s:59
+
+vaeskf1.vi v10, v9, 1
+# CHECK-INST: vaeskf1.vi v10, v9, 1

ego wrote:
> craig.topper wrote:
> > ego wrote:
> > > If I replaces "v10" with "v0", the test fails with an assertion failure. 
> > > My own patch uses a slightly different class hierarchy but hits the same 
> > > assertion.
> > > 
> > > 
> > > ```
> > > FAIL: LLVM :: MC/RISCV/rvv/rv64zvkns.s (3 of 8)
> > >  TEST 'LLVM :: MC/RISCV/rvv/rv64zvkns.s' FAILED 
> > > 
> > > Script:
> > > --
> > > : 'RUN: at line 1';   
> > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/llvm-mc 
> > > -triple=riscv64 -show-encoding --mattr=+zve32x 
> > > --mattr=+experimental-zvkns 
> > > /Users/ego/root-dirs/scratch/projects/llvm-project/llvm/test/MC/RISCV/rvv/rv64zvkns.s
> > >  | 
> > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/FileCheck 
> > > /Users/ego/root-dirs/scratch/projects/llvm-project/llvm/test/MC/RISCV/rvv/rv64zvkns.s
> > >  --check-prefixes=CHECK-ENCODING,CHECK-INST
> > > : 'RUN: at line 3';   not 
> > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/llvm-mc 
> > > -triple=riscv64 -show-encoding 
> > > /Users/ego/root-dirs/scratch/projects/llvm-project/llvm/test/MC/RISCV/rvv/rv64zvkns.s
> > >  2>&1 | 
> > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/FileCheck 
> > > /Users/ego/root-dirs/scratch/projects/llvm-project/llvm/test/MC/RISCV/rvv/rv64zvkns.s
> > >  --check-prefix=CHECK-ERROR
> > > : 'RUN: at line 5';   
> > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/llvm-mc 
> > > -triple=riscv64 -filetype=obj --mattr=+zve32x --mattr=+experimental-zvkns 
> > > /Users/ego/root-dirs/scratch/projects/llvm-project/llvm/test/MC/RISCV/rvv/rv64zvkns.s
> > >  | 
> > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/llvm-objdump 
> > > -d --mattr=+zve32x --mattr=+experimental-zvkns  - | 
> > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/FileCheck 
> > > /Users/ego/root-dirs/scratch/projects/llvm-project/llvm/test/MC/RISCV/rvv/rv64zvkns.s
> > >  --check-prefix=CHECK-INST
> > > : 'RUN: at line 8';   
> > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/llvm-mc 
> > > -triple=riscv64 -filetype=obj --mattr=+zve32x --mattr=+experimental-zvkns 
> > > /Users/ego/root-dirs/scratch/projects/llvm-project/llvm/test/MC/RISCV/rvv/rv64zvkns.s
> > >  | 
> > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/llvm-objdump 
> > > -d - | 
> > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/FileCheck 
> > > /Users/ego/root-dirs/scratch/projects/llvm-project/llvm/test/MC/RISCV/rvv/rv64zvkns.s
> > >  --check-prefix=CHECK-UNKNOWN
> > > --
> > > Exit Code: 2
> > > 
> > > Command Output (stderr):
> > > --
> > > Assertion failed: (isReg() && "This is not a register operand!"), 
> > > function getReg, file MCInst.h, line 70.
> > > PLEASE submit a bug report to 
> > > https://github.com/llvm/llvm-project/issues/ and include the crash 
> > > backtrace.
> > > Stack dump:
> > > 0.Program arguments: 
> > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/llvm-mc 
> > > -triple=riscv64 -show-encoding --mattr=+zve32x 
> > > --mattr=+experimental-zvkns 
> > > /Users/ego/root-dirs/scratch/projects/llvm-project/llvm/test/MC/RISCV/rvv/rv64zvkns.s
> > > Stack dump without symbol names (ensure you have llvm-symbolizer in your 
> > > PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
> > > 0  libLLVMSupport.dylib0x0001023015e8 
> > > llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 56
> > > 1  libLLVMSupport.dylib0x000102300840 
> > > llvm::sys::RunSignalHandlers() + 112
> > > 2  libLLVMSupport.dylib0x000102301c28 SignalHandler(int) + 304
> > > 3  libsystem_platform.dylib0x0001914f82a4 _sigtramp + 56
> > > 4  libsystem_pthread.dylib 0x0001914c9cec pthread_kill + 288
> > > 5  libsystem_c.dylib   0x0001914032c8 abort + 180
> > > 6  libsystem_c.dylib   0x000191402620 err + 0
> > > 7  libLLVMRISCVAsmParser.dylib 0x000100666208 (anonymous 
> > > namespace)::RISCVAsmParser::MatchAndEmitInstruction(llvm::SMLoc, unsigned 
> > > int&, 
> > > llvm::SmallVectorImpl > > std::__1::default_delete>>&, llvm::MCStreamer&, 
> > > unsigned long long&, bool) (.cold.42) + 0
> > > 8  libLLVMRISCVAsmParser.dylib 0x000100655084 (anonymous 
> > > namespace)::RISCVAsmParser::MatchAndEmitInstruction(llvm::SMLoc, unsigned 
> > > int&, 
> > > llvm::SmallVectorImpl > > std::__1::default_delete>>&, llvm::MCStreamer&, 
> > > unsigned long long&, bool) + 7268
> > > 9  libLLVMMCParser.dylib   0x000100c36c08 (anonymous 
> > > namespace)::AsmParser::parseAndMatchAndEmitTargetInstruction

[PATCH] D127762: [Clang][AArch64] Add ACLE attributes for SME.

2023-01-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D127762#3960906 , @rsandifo-arm 
wrote:

> Thanks @aaron.ballman for the feedback about spellings.  I've gone with the 
> lower-case names like `__arm_streaming` in what follows, as a plausible 
> strawman.
>
> My main concern with keywords is:
>
>> This approach means there's plenty of flexibility available to parse the 
>> keyword where you think it makes sense.
>
> If everyone parses keywords where it makes sense to them personally, for 
> their specific use case, I'm worried that we'll end up with a lot of slightly 
> different rules.  (This tended to happen for pre-standard features, and like 
> you say, is still the case for GNU attribute handling between Clang and GCC, 
> given GCC's poorly-defined rules.)

Ah, I meant more that we have flexibility on what we want the grammar of the 
extension to be. If other implementations wanted to pick up the functionality, 
hopefully they'd pick up the same grammar as well.

> For example, a type property like `__arm_streaming` only applies to 
> functions, so it wouldn't make sense for bespoke rules to allow the keyword 
> in tagged types or in array types.  If a property only applies to object 
> types then it wouldn't make sense for bespoke rules to allow the keyword 
> after a parameter list.

Agreed, but that's a semantic property of the attribute rather than a syntactic 
property. We have control over both, of course, because this is our extension 
and we can define it how we like. But I was expecting we'd define a syntactic 
location to parse the attribute and we'd handle appertainance issues in 
SemaDeclAttr.cpp when deciding whether to convert the parsed attribute into a 
semantic attribute or not.

> So I think it makes sense to try to make the SME attributes an instance of a 
> (new) generic solution.  I think we want something “like standard attributes, 
> but for semantic information”.  That might sound like wanting something “like 
> ice, but not cold”.  But a lot of valuable work went into defining the 
> parsing rules for standard attributes, and defining what they appertain to.  
> It seems like a good idea to reuse those parts rather than reinvent the wheel.
>
> https://reviews.llvm.org/D139028 is an RFC for adding “attribute-like 
> keywords”: keywords that can appear exactly where a standard attribute can 
> appear.  This means that the keywords are syntactically valid in far more 
> places than necessary for this initial use case, but it provides a simple and 
> mechanical rule.  And the keywords would have to be rejected in those 
> additional places anyway, even if we don't parse them as attributes.

FWIW, we already have a number of attributes implemented via a keyword today: 
`ConstInitAttr`, `C11NoReturnAttr`, a ton of CUDA and OpenCL keywords, calling 
conventions like `__stdcall`, etc. I'll take a look at the other patch to see 
what it's all about, but currently all keyword attributes need explicit parsing 
support added for them, as in: 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Parse/ParseDecl.cpp#L4005
 and 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaDecl.cpp#L7767.

> The covering note has a lot more justification (too much, sorry!).  Does this 
> look like a possible way forward?
>
> Previously I was arguing in favour of the decl-to-type sliding rule, because:
>
>> From discussions I've had with people who will write/are writing SME code, 
>> the distinction between arm_locally_streamng being a property of the 
>> definition and arm_streaming being a property of the type seems contrived to 
>> them, especially when the usage of SME is private to a single translation 
>> unit.
>
> Taking the RFC approach involves abandoning that and sticking strictly to the 
> standard appurtenance rules.  But hopefully it's a reasonable trade-off.  The 
> sliding rule is clearly going against the direction of travel anyway, so I'm 
> probably being too Luddite there.

Err, I'm struggling to see why the distinction is contrived. If the attribute 
applies to the type, then you get type-based semantics like the ability to 
overload or specialize on the presence/absence of the attribute -- you 
shouldn't get that behavior from a declaration attribute. e.g., if you use a 
declaration attribute on a function definition and then redefine the function 
without the attribute, that should give a redefinition error because that's two 
functions with the same name and same type being defined.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127762

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


[PATCH] D140896: [WIP] Move from llvm::makeArrayRef to ArrayRef deduction guides

2023-01-03 Thread Jacques Pienaar via Phabricator via cfe-commits
jpienaar added inline comments.



Comment at: llvm/include/llvm/ADT/ArrayRef.h:502
+  /// Deduction guide to construct an ArrayRef from a C array.
+  template  ArrayRef(const T (&Arr)[N]) -> ArrayRef;
 

mehdi_amini wrote:
> Can we keep the makeArrayRef functions for now and mark them deprecated?
+1 that would also allow for this change to broken up so that the deduction 
guides land first and then the updates (potentially even 3 hops, 1) add guides, 
2) update, 3) mark deprecated - 1 & 2 could be combined but it'll greatly 
simplify review/enable sharding it)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140896

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


[PATCH] D140775: [clangd] Hover: show CalleeArgInfo for literals

2023-01-03 Thread Tom Praschan via Phabricator via cfe-commits
tom-anders updated this revision to Diff 486039.
tom-anders added a comment.

Add test for expression, improve presentation for signature with unnamed 
parameter


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140775

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -900,6 +900,40 @@
  HI.CalleeArgInfo->Type = "int &";
  HI.CallPassType = HoverInfo::PassType{PassMode::Ref, false};
}},
+  {// Literal passed to function call
+   R"cpp(
+  void fun(int arg_a, const int &arg_b) {};
+  void code() {
+int a = 1;
+fun(a, [[^2]]);
+  }
+  )cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "literal";
+ HI.Kind = index::SymbolKind::Unknown;
+ HI.CalleeArgInfo.emplace();
+ HI.CalleeArgInfo->Name = "arg_b";
+ HI.CalleeArgInfo->Type = "const int &";
+ HI.CallPassType = HoverInfo::PassType{PassMode::ConstRef, false};
+   }},
+  {// Expression passed to function call
+   R"cpp(
+  void fun(int arg_a, const int &arg_b) {};
+  void code() {
+int a = 1;
+fun(a, 1 [[^+]] 2);
+  }
+  )cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "expression";
+ HI.Kind = index::SymbolKind::Unknown;
+ HI.Type = "int";
+ HI.Value = "3";
+ HI.CalleeArgInfo.emplace();
+ HI.CalleeArgInfo->Name = "arg_b";
+ HI.CalleeArgInfo->Type = "const int &";
+ HI.CallPassType = HoverInfo::PassType{PassMode::ConstRef, false};
+   }},
   {// Extra info for method call.
R"cpp(
   class C {
@@ -1226,8 +1260,10 @@
   } Tests[] = {
   // Integer tests
   {"int_by_value([[^int_x]]);", PassMode::Value, false},
+  {"int_by_value([[^123]]);", PassMode::Value, false},
   {"int_by_ref([[^int_x]]);", PassMode::Ref, false},
   {"int_by_const_ref([[^int_x]]);", PassMode::ConstRef, false},
+  {"int_by_const_ref([[^123]]);", PassMode::ConstRef, false},
   {"int_by_value([[^int_ref]]);", PassMode::Value, false},
   {"int_by_const_ref([[^int_ref]]);", PassMode::ConstRef, false},
   {"int_by_const_ref([[^int_ref]]);", PassMode::ConstRef, false},
@@ -1245,6 +1281,8 @@
   {"float_by_value([[^int_x]]);", PassMode::Value, true},
   {"float_by_value([[^int_ref]]);", PassMode::Value, true},
   {"float_by_value([[^int_const_ref]]);", PassMode::Value, true},
+  {"float_by_value([[^123.0f]]);", PassMode::Value, false},
+  {"float_by_value([[^123]]);", PassMode::Value, true},
   {"custom_by_value([[^int_x]]);", PassMode::Ref, true},
   {"custom_by_value([[^float_x]]);", PassMode::Value, true},
   {"custom_by_value([[^base]]);", PassMode::ConstRef, true},
@@ -3043,6 +3081,18 @@
 
 // In test::Bar
 int foo = 3)",
+  },
+  {
+  [](HoverInfo &HI) {
+HI.Kind = index::SymbolKind::Variable;
+HI.Name = "foo";
+HI.CalleeArgInfo.emplace();
+HI.CalleeArgInfo->Type = "int";
+HI.CallPassType = HoverInfo::PassType{PassMode::Value, false};
+  },
+  R"(variable foo
+
+Passed by value)",
   },
   {
   [](HoverInfo &HI) {
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -795,7 +795,7 @@
  llvm::isa(E) ||
  llvm::isa(E) || llvm::isa(E) ||
  llvm::isa(E) || llvm::isa(E) ||
- llvm::isa(E);
+ llvm::isa(E) || llvm::isa(E);
 }
 
 llvm::StringLiteral getNameForExpr(const Expr *E) {
@@ -809,34 +809,53 @@
   return llvm::StringLiteral("expression");
 }
 
+void maybeAddCalleeArgInfo(const SelectionTree::Node *N, HoverInfo &HI,
+   const PrintingPolicy &PP);
+
 // Generates hover info for `this` and evaluatable expressions.
 // FIXME: Support hover for literals (esp user-defined)
-std::optional getHoverContents(const Expr *E, ParsedAST &AST,
+std::optional getHoverContents(const SelectionTree::Node *N,
+  const Expr *E, ParsedAST &AST,
   const PrintingPolicy &PP,
   const SymbolIndex *Index) {
-  // There's not much value in hovering over "42" and getting a hover card
-  // saying "42 is an int", similar for other literals.
-  if (isLiteral(E))
+  std::optional HI;
+
+  if (const StringLiteral *SL = dyn_cast(E)) {
+// Print the type and the size for string literals
+HI =

[PATCH] D135488: [codegen] Add a remarks based Stack Layout Analysis pass

2023-01-03 Thread Francis Visoiu Mistrih via Phabricator via cfe-commits
thegameg added a comment.

This is great! Any chance we can use `MachineFrameInfo::StackProtectorIdx` to 
annotate the slot that is reserved for the stack protector?




Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:84
+ORE = &getAnalysis().getORE();
+if (!ORE)
+  return false;

I don't think this should ever be null.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135488

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


[PATCH] D140857: Fix Fuchsia dyld in asan-ubsan variant

2023-01-03 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr added a comment.

I don't think we want this in the generic compiler behavior. It would only make 
sense if the toolchain runtimes have extra versions, and I don't think we want 
that many extra versions.
It may be worthwhile to change the asan multilib builds to enable ubsan checks 
too, but I don't think we want to support two separate asan flavors in the 
driver.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140857

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


[clang] 0d6b26b - [Clang] Fix a crash when encountering an ill-formed delimited UCN.

2023-01-03 Thread Corentin Jabot via cfe-commits

Author: Corentin Jabot
Date: 2023-01-03T20:57:52+01:00
New Revision: 0d6b26b4d3e3991da16f5b7f53e397b0051e8598

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

LOG: [Clang] Fix a crash when encountering an ill-formed delimited UCN.

\u{...} was incorrectly parsed as a valid UCN instead
of emitting a diagnostic, causing an assertion failure.

Reviewed By: tahonermann

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

Added: 


Modified: 
clang/lib/Lex/Lexer.cpp
clang/test/Preprocessor/ucn-pp-identifier.c

Removed: 




diff  --git a/clang/lib/Lex/Lexer.cpp b/clang/lib/Lex/Lexer.cpp
index 2f21f7b069cfe..ce48e7da21bb3 100644
--- a/clang/lib/Lex/Lexer.cpp
+++ b/clang/lib/Lex/Lexer.cpp
@@ -3286,7 +3286,7 @@ llvm::Optional Lexer::tryReadNumericUCN(const 
char *&StartPtr,
   uint32_t CodePoint = 0;
   while (Count != NumHexDigits || Delimited) {
 char C = getCharAndSize(CurPtr, CharSize);
-if (!Delimited && C == '{') {
+if (!Delimited && Count == 0 && C == '{') {
   Delimited = true;
   CurPtr += CharSize;
   continue;

diff  --git a/clang/test/Preprocessor/ucn-pp-identifier.c 
b/clang/test/Preprocessor/ucn-pp-identifier.c
index 8d30a6a2bb239..c47ed21ff3158 100644
--- a/clang/test/Preprocessor/ucn-pp-identifier.c
+++ b/clang/test/Preprocessor/ucn-pp-identifier.c
@@ -117,6 +117,7 @@ C 1
 // CHECK-NEXT: {{^   u}}
 
 #define \u{}   // expected-warning {{empty delimited universal 
character name; treating as '\' 'u' '{' '}'}} expected-error {{macro name must 
be an identifier}}
+#define \u1{123}   // expected-warning {{incomplete universal character 
name; treating as '\' followed by identifier}} expected-error {{macro name must 
be an identifier}}
 #define \u{123456789}  // expected-error {{hex escape sequence out of range}} 
expected-error {{macro name must be an identifier}}
 #define \u{// expected-warning {{incomplete delimited universal 
character name; treating as '\' 'u' '{' identifier}} expected-error {{macro 
name must be an identifier}}
 #define \u{fgh}// expected-warning {{incomplete delimited universal 
character name; treating as '\' 'u' '{' identifier}} expected-error {{macro 
name must be an identifier}}



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


[PATCH] D139889: [Clang] Fix a crash when encountering an ill-formed delimited UCN.

2023-01-03 Thread Corentin Jabot via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0d6b26b4d3e3: [Clang] Fix a crash when encountering an 
ill-formed delimited UCN. (authored by cor3ntin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139889

Files:
  clang/lib/Lex/Lexer.cpp
  clang/test/Preprocessor/ucn-pp-identifier.c


Index: clang/test/Preprocessor/ucn-pp-identifier.c
===
--- clang/test/Preprocessor/ucn-pp-identifier.c
+++ clang/test/Preprocessor/ucn-pp-identifier.c
@@ -117,6 +117,7 @@
 // CHECK-NEXT: {{^   u}}
 
 #define \u{}   // expected-warning {{empty delimited universal 
character name; treating as '\' 'u' '{' '}'}} expected-error {{macro name must 
be an identifier}}
+#define \u1{123}   // expected-warning {{incomplete universal character 
name; treating as '\' followed by identifier}} expected-error {{macro name must 
be an identifier}}
 #define \u{123456789}  // expected-error {{hex escape sequence out of range}} 
expected-error {{macro name must be an identifier}}
 #define \u{// expected-warning {{incomplete delimited universal 
character name; treating as '\' 'u' '{' identifier}} expected-error {{macro 
name must be an identifier}}
 #define \u{fgh}// expected-warning {{incomplete delimited universal 
character name; treating as '\' 'u' '{' identifier}} expected-error {{macro 
name must be an identifier}}
Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -3286,7 +3286,7 @@
   uint32_t CodePoint = 0;
   while (Count != NumHexDigits || Delimited) {
 char C = getCharAndSize(CurPtr, CharSize);
-if (!Delimited && C == '{') {
+if (!Delimited && Count == 0 && C == '{') {
   Delimited = true;
   CurPtr += CharSize;
   continue;


Index: clang/test/Preprocessor/ucn-pp-identifier.c
===
--- clang/test/Preprocessor/ucn-pp-identifier.c
+++ clang/test/Preprocessor/ucn-pp-identifier.c
@@ -117,6 +117,7 @@
 // CHECK-NEXT: {{^   u}}
 
 #define \u{}   // expected-warning {{empty delimited universal character name; treating as '\' 'u' '{' '}'}} expected-error {{macro name must be an identifier}}
+#define \u1{123}   // expected-warning {{incomplete universal character name; treating as '\' followed by identifier}} expected-error {{macro name must be an identifier}}
 #define \u{123456789}  // expected-error {{hex escape sequence out of range}} expected-error {{macro name must be an identifier}}
 #define \u{// expected-warning {{incomplete delimited universal character name; treating as '\' 'u' '{' identifier}} expected-error {{macro name must be an identifier}}
 #define \u{fgh}// expected-warning {{incomplete delimited universal character name; treating as '\' 'u' '{' identifier}} expected-error {{macro name must be an identifier}}
Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -3286,7 +3286,7 @@
   uint32_t CodePoint = 0;
   while (Count != NumHexDigits || Delimited) {
 char C = getCharAndSize(CurPtr, CharSize);
-if (!Delimited && C == '{') {
+if (!Delimited && Count == 0 && C == '{') {
   Delimited = true;
   CurPtr += CharSize;
   continue;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D140915: [clangd] Fix getQueryScopes for using-directive with inline namespace

2023-01-03 Thread Tom Praschan via Phabricator via cfe-commits
tom-anders created this revision.
tom-anders added reviewers: nridge, kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
tom-anders requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

For example, in the folliwong code

  #include 
  
  using namespace std::string_literals;
  
  int main() {
  strin^ // Completes `string` instead of `std::string`
  }

The using declaration would make completion drop the std namespace, even though 
it shouldn't.

printNamespaceScope() skips inline namespaces, so to fix this use
printQualifiedName() instead

See https://github.com/clangd/clangd/issues/1451


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140915

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1700,9 +1700,11 @@
   namespace ns1 {}
   namespace ns2 {} // ignore
   namespace ns3 { namespace nns3 {} }
+  namespace ns4 { inline namespace ns4_inline {} }
   namespace foo {
   using namespace ns1;
   using namespace ns3::nns3;
+  using namespace ns4::ns4_inline;
   }
   namespace ns {
   void f() {
@@ -1711,10 +1713,11 @@
   }
   )cpp");
 
-  EXPECT_THAT(Requests,
-  ElementsAre(Field(
-  &FuzzyFindRequest::Scopes,
-  UnorderedElementsAre("foo::", "ns1::", "ns3::nns3::";
+  EXPECT_THAT(
+  Requests,
+  ElementsAre(Field(&FuzzyFindRequest::Scopes,
+UnorderedElementsAre("foo::", "ns1::", "ns3::nns3::",
+ "ns4::ns4_inline::";
 }
 
 TEST(CompletionTest, UnresolvedQualifierIdQuery) {
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -670,8 +670,12 @@
   for (auto *Context : CCContext.getVisitedContexts()) {
 if (isa(Context))
   Scopes.AccessibleScopes.push_back(""); // global namespace
-else if (isa(Context))
-  Scopes.AccessibleScopes.push_back(printNamespaceScope(*Context));
+else if (const auto *ND = dyn_cast(Context)) {
+  if (ND->isInlineNamespace())
+Scopes.AccessibleScopes.push_back(printQualifiedName(*ND) + "::");
+  else
+Scopes.AccessibleScopes.push_back(printNamespaceScope(*ND));
+}
   }
 
   const CXXScopeSpec *SemaSpecifier =


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1700,9 +1700,11 @@
   namespace ns1 {}
   namespace ns2 {} // ignore
   namespace ns3 { namespace nns3 {} }
+  namespace ns4 { inline namespace ns4_inline {} }
   namespace foo {
   using namespace ns1;
   using namespace ns3::nns3;
+  using namespace ns4::ns4_inline;
   }
   namespace ns {
   void f() {
@@ -1711,10 +1713,11 @@
   }
   )cpp");
 
-  EXPECT_THAT(Requests,
-  ElementsAre(Field(
-  &FuzzyFindRequest::Scopes,
-  UnorderedElementsAre("foo::", "ns1::", "ns3::nns3::";
+  EXPECT_THAT(
+  Requests,
+  ElementsAre(Field(&FuzzyFindRequest::Scopes,
+UnorderedElementsAre("foo::", "ns1::", "ns3::nns3::",
+ "ns4::ns4_inline::";
 }
 
 TEST(CompletionTest, UnresolvedQualifierIdQuery) {
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -670,8 +670,12 @@
   for (auto *Context : CCContext.getVisitedContexts()) {
 if (isa(Context))
   Scopes.AccessibleScopes.push_back(""); // global namespace
-else if (isa(Context))
-  Scopes.AccessibleScopes.push_back(printNamespaceScope(*Context));
+else if (const auto *ND = dyn_cast(Context)) {
+  if (ND->isInlineNamespace())
+Scopes.AccessibleScopes.push_back(printQualifiedName(*ND) + "::");
+  else
+Scopes.AccessibleScopes.push_back(printNamespaceScope(*ND));
+}
   }
 
   const CXXScopeSpec *SemaSpecifier =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D136031: [DirectX backend] support ConstantBuffer to DXILResource.h

2023-01-03 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added a comment.

It is probably worth adding some unit tests to test the `CBufferDataLayout` 
class.

I think the meat of this change is fine. This code mixes `unsigned` and 
`uint32_t` interchangeably. They aren't required by the language to be the same.

I have a general preference toward `uint32_t` which is explicitly sized, but we 
should match existing interfaces where appropriate.

For example, we should also probably be using `llvm::TypeSize` where 
appropriate to match `llvm::DataLayout`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136031

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


[clang] 9ab0d4d - [OpenMP][2/2] Make device functions have hidden visibility

2023-01-03 Thread Johannes Doerfert via cfe-commits

Author: Johannes Doerfert
Date: 2023-01-03T12:18:30-08:00
New Revision: 9ab0d4d66fa14a9c57864fea72590886ace6d9ee

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

LOG: [OpenMP][2/2]  Make device functions have hidden visibility

Similar to https://reviews.llvm.org/D136111, this time for class
methods.

D136111 summary:

In OpenMP target offloading an in other offloading languages, we
maintain a difference between device functions and kernel functions.
Kernel functions must be visible to the host and act as the entry point
to the target device. Device functions however cannot be called directly
by the host and must be called by a kernel function. Currently, we make
all definitions on the device protected by default. Because device
functions cannot be called or used by the host they should have hidden
visibility. This allows for the definitions to be better optimized via
LTO or other passes.

This patch marks every device class methods in the AST as having hidden
visibility. The kernel function is generated later at code-gen and we
set its visibility explicitly so it should not be affected. This
prevents the user from overriding the visibility, but since the user
can't do anything with these symbols anyway there is no point exporting
them right now.

Added: 
clang/test/OpenMP/target_visibility.cpp

Modified: 
clang/lib/AST/Decl.cpp

Removed: 




diff  --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index ccf5d71538e9f..236d4f98bb6a7 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -1020,6 +1020,16 @@ LinkageComputer::getLVForClassMember(const NamedDecl *D,
   explicitSpecSuppressor = MD;
 }
 
+// OpenMP target declare device functions are not callable from the host so
+// they should not be exported from the device image. This applies to all
+// functions as the host-callable kernel functions are emitted at codegen.
+ASTContext &Context = D->getASTContext();
+if (Context.getLangOpts().OpenMP && Context.getLangOpts().OpenMPIsDevice &&
+((Context.getTargetInfo().getTriple().isAMDGPU() ||
+  Context.getTargetInfo().getTriple().isNVPTX()) ||
+ OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(MD)))
+  LV.mergeVisibility(HiddenVisibility, /*newExplicit=*/false);
+
   } else if (const auto *RD = dyn_cast(D)) {
 if (const auto *spec = dyn_cast(RD)) {
   mergeTemplateLV(LV, spec, computation);

diff  --git a/clang/test/OpenMP/target_visibility.cpp 
b/clang/test/OpenMP/target_visibility.cpp
new file mode 100644
index 0..af1a1bb243cc4
--- /dev/null
+++ b/clang/test/OpenMP/target_visibility.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -verify -fopenmp -x c++ -triple 
nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s 
-fopenmp-is-device -o - | FileCheck %s
+// RUN: %clang_cc1 -debug-info-kind=limited -verify -fopenmp -x c++ -triple 
nvptx-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm %s 
-fopenmp-is-device -o - | FileCheck %s
+// expected-no-diagnostics
+
+
+#pragma omp declare target
+
+struct A {
+void foo() {}
+static void sfoo() {}
+};
+
+#pragma omp end declare target
+
+struct B {
+void bar();
+static void sbar();
+};
+
+void B::bar() { A a; a.foo(); }
+void B::sbar() { A::sfoo(); }
+#pragma omp declare target to(B::bar, B::sbar)
+
+// CHECK-DAG: define hidden void @_ZN1B4sbarEv()
+// CHECK-DAG: define linkonce_odr hidden void @_ZN1A4sfooEv()
+// CHECK-DAG: define hidden void @_ZN1B3barEv(
+// CHECK-DAG: define linkonce_odr hidden void @_ZN1A3fooEv(



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


[PATCH] D139889: [Clang] Fix a crash when encountering an ill-formed delimited UCN.

2023-01-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment.

@tahonermann Thanks :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139889

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


[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-01-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

I'm sympathetic to the problem you're trying to solve (not having to set 
environment variable for the temp directory), but I'm also not convinced this 
is the correct way to approach it. This feels like a configuration property of 
the libclang execution -- so it'd be nice to require it to be set up from 
`clang_createIndex()` (IIRC that's the entrypoint for CIndex functionality, but 
I'm not 100% sure), rather than an API that the user can call repeatedly. Did 
you consider a design like that?




Comment at: clang/docs/ReleaseNotes.rst:863
+- Introduced the new function ``clang_setTemporaryDirectory``,
+  which allows to override the temporary directory path used by Clang.
 - ``clang_Cursor_getNumTemplateArguments``, 
``clang_Cursor_getTemplateArgumentKind``,





Comment at: clang/include/clang-c/Index.h:78-79
+ * this function in order to reset the temporary directory to the default value
+ * from the environment. Such a resetting should be done before deleting a
+ * tempDirUtf8 pointer previously passed to this function.
+ */

Should we mention anything about relative vs absolute path requirements? 
Separators? 



Comment at: llvm/include/llvm/Support/Path.h:423
+/// tempDirUtf8 pointer previously passed to this function.
+void set_system_temp_directory_erased_on_reboot(const char *tempDirUtf8);
+

Err, I'm not super excited about this new API. For starters, it's not setting 
the system temp directory at all (it doesn't make any modifications to the host 
system); instead it overrides the system temp directory. But also, this is 
pretty fragile due to allowing the user to override the temp directory after 
the compiler has already queried for the system temp directory, so now you get 
files in two different places. Further, it's fragile because the caller is 
responsible for keeping that pointer valid for the lifetime of the program. 
Finally, we don't allow you to override any other system directory that you can 
query (like the home directory).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139774

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


[PATCH] D140711: [clang] Fix unused variable warning in SemaConcept.cpp

2023-01-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment.

nit: I think that's a cleaner fix :)




Comment at: clang/lib/Sema/SemaConcept.cpp:1348
   bool &Result) {
   if (const auto *FD1 = dyn_cast(D1)) {
 auto IsExpectedEntity = [](const FunctionDecl *FD) {




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140711

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


[PATCH] D140896: [WIP] Move from llvm::makeArrayRef to ArrayRef deduction guides

2023-01-03 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 486049.
serge-sans-paille added a comment.

+ keep deprecated version, flagged appropriately


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

https://reviews.llvm.org/D140896

Files:
  bolt/tools/bat-dump/bat-dump.cpp
  bolt/tools/driver/llvm-bolt.cpp
  bolt/tools/heatmap/heatmap.cpp
  clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
  clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp
  clang-tools-extra/clang-tidy/concurrency/MtUnsafeCheck.cpp
  clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/CompileCommands.cpp
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
  clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
  clang-tools-extra/pseudo/include/clang-pseudo/Forest.h
  clang-tools-extra/pseudo/include/clang-pseudo/GLR.h
  clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h
  clang-tools-extra/pseudo/lib/Forest.cpp
  clang-tools-extra/pseudo/lib/GLR.cpp
  clang-tools-extra/pseudo/lib/grammar/Grammar.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/AST/DeclOpenMP.h
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/ExprObjC.h
  clang/include/clang/AST/ExprOpenMP.h
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/PropertiesBase.td
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/StmtOpenMP.h
  clang/include/clang/AST/TemplateBase.h
  clang/include/clang/AST/TemplateName.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
  clang/include/clang/Basic/Diagnostic.h
  clang/include/clang/Basic/SyncScope.h
  clang/include/clang/CodeGen/CGFunctionInfo.h
  clang/include/clang/Lex/MacroInfo.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/DelayedDiagnostic.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTRecordReader.h
  clang/include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h
  clang/include/clang/Tooling/Transformer/RewriteRule.h
  clang/lib/AST/APValue.cpp
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/CommentParser.cpp
  clang/lib/AST/CommentSema.cpp
  clang/lib/AST/ComputeDependence.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/ParentMapContext.cpp
  clang/lib/AST/TemplateName.cpp
  clang/lib/AST/Type.cpp
  clang/lib/ASTMatchers/Dynamic/Marshallers.cpp
  clang/lib/Analysis/CFG.cpp
  clang/lib/Analysis/CalledOnceCheck.cpp
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Basic/Module.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/ARC.h
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Basic/Targets/AVR.h
  clang/lib/Basic/Targets/BPF.cpp
  clang/lib/Basic/Targets/CSKY.cpp
  clang/lib/Basic/Targets/Hexagon.cpp
  clang/lib/Basic/Targets/Lanai.cpp
  clang/lib/Basic/Targets/LoongArch.cpp
  clang/lib/Basic/Targets/M68k.cpp
  clang/lib/Basic/Targets/MSP430.cpp
  clang/lib/Basic/Targets/MSP430.h
  clang/lib/Basic/Targets/Mips.cpp
  clang/lib/Basic/Targets/Mips.h
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/Sparc.cpp
  clang/lib/Basic/Targets/SystemZ.cpp
  clang/lib/Basic/Targets/VE.cpp
  clang/lib/Basic/Targets/VE.h
  clang/lib/Basic/Targets/WebAssembly.cpp
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/XCore.cpp
  clang/lib/Basic/Targets/XCore.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenPGO.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/ConstantInitBuilder.cpp
  clang/lib/CodeGen/CoverageMappingG

[PATCH] D135488: [codegen] Add a remarks based Stack Layout Analysis pass

2023-01-03 Thread Paul Kirth via Phabricator via cfe-commits
paulkirth updated this revision to Diff 486050.
paulkirth added a comment.

Avoid problems with path separators on windows, and ignore path prefix in 
diagnostic


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135488

Files:
  clang/test/Frontend/stack-layout-remark.c
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/InitializePasses.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/test/CodeGen/AArch64/O0-pipeline.ll
  llvm/test/CodeGen/AArch64/O3-pipeline.ll
  llvm/test/CodeGen/AArch64/arm64-opt-remarks-lazy-bfi.ll
  llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
  llvm/test/CodeGen/ARM/O3-pipeline.ll
  llvm/test/CodeGen/ARM/stack-frame-layout-remarks.ll
  llvm/test/CodeGen/Generic/llc-start-stop.ll
  llvm/test/CodeGen/PowerPC/O0-pipeline.ll
  llvm/test/CodeGen/PowerPC/O3-pipeline.ll
  llvm/test/CodeGen/RISCV/O0-pipeline.ll
  llvm/test/CodeGen/RISCV/O3-pipeline.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/opt-pipeline.ll
  llvm/test/CodeGen/X86/stack-frame-layout-remarks.ll

Index: llvm/test/CodeGen/X86/stack-frame-layout-remarks.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-frame-layout-remarks.ll
@@ -0,0 +1,303 @@
+; Test remark output for stack-frame-layout
+
+; ensure basic output works
+; RUN: llc -mcpu=corei7 -O1 -pass-remarks-analysis=stack-frame-layout < %s 2>&1 >/dev/null | FileCheck %s
+
+; check additional slots are displayed when stack is not optimized
+; RUN: llc -mcpu=corei7 -O0 -pass-remarks-analysis=stack-frame-layout < %s 2>&1 >/dev/null | FileCheck %s --check-prefix=NO_COLORING
+
+; check more complex cases
+; RUN: llc %s -pass-remarks-analysis=stack-frame-layout -o /dev/null --march=x86 -mcpu=i386 2>&1 | FileCheck %s --check-prefix=BOTH --check-prefix=DEBUG
+
+; check output without debug info
+; RUN: opt %s -passes=strip -S | llc  -pass-remarks-analysis=stack-frame-layout -o /dev/null --march=x86 -mcpu=i386 2>&1 | FileCheck %s --check-prefix=BOTH --check-prefix=STRIPPED
+
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK: FunctionName: stackSizeWarning
+; CHECK: Offset{{.*}}Align{{.*}}Size
+; CHECK: [SP-88]{{.*}}16{{.*}}80
+; CHECK:buffer @ frame-diags.c:30
+; NO_COLORING: [SP-168]{{.*}}16{{.*}}80
+; CHECK:buffer2 @ frame-diags.c:33
+define void @stackSizeWarning() {
+entry:
+  %buffer = alloca [80 x i8], align 16
+  %buffer2 = alloca [80 x i8], align 16
+  call void @llvm.dbg.declare(metadata ptr %buffer, metadata !25, metadata !DIExpression()), !dbg !39
+  call void @llvm.dbg.declare(metadata ptr %buffer2, metadata !31, metadata !DIExpression()), !dbg !40
+  ret void
+}
+
+; Function Attrs: nocallback nofree nosync nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #0
+
+; BOTH: FunctionName: cleanup_array
+; BOTH:  Offset{{.*}}Align{{.*}}Size
+; BOTH:  [SP+4]{{.+}}16{{.+}}4
+; DEBUG: a @ dot.c:13
+; STRIPPED-NOT: a @ dot.c:13
+; BOTH:  [SP-4]{{.+}}Spill 8{{.+}}4
+define void @cleanup_array(ptr %0) #1 {
+  %2 = alloca ptr, align 8
+  store ptr %0, ptr %2, align 8
+  call void @llvm.dbg.declare(metadata ptr %2, metadata !41, metadata !DIExpression()), !dbg !46
+  ret void
+}
+
+; BOTH: FunctionName: cleanup_result
+; BOTH:  Offset{{.+}}Align{{.+}}Size
+; BOTH:  [SP+4]{{.+}}16{{.+}}4
+; DEBUG: res @ dot.c:21
+; STRIPPED-NOT: res @ dot.c:21
+; BOTH:  [SP-4]{{.+}}Spill 8{{.+}}4
+define void @cleanup_result(ptr %0) #1 {
+  %2 = alloca ptr, align 8
+  store ptr %0, ptr %2, align 8
+  call void @llvm.dbg.declare(metadata ptr %2, metadata !47, metadata !DIExpression()), !dbg !51
+  ret void
+}
+
+; BOTH: FunctionName: do_work
+; BOTH:  Offset{{.+}}Align{{.+}}Size
+; BOTH:  [SP+12]{{.+}}8{{.+}}4
+; DEBUG: out @ dot.c:32
+; STRIPPED-NOT: out @ dot.c:32
+; BOTH:  [SP+8]{{.+}}4{{.+}}4
+; BOTH:  [SP+4]{{.+}}16{{.+}}4
+; DEBUG: A @ dot.c:32
+; STRIPPED-NOT: A @ dot.c:32
+; BOTH:  [SP-4]{{.+}}Spill 8{{.+}}4
+; BOTH:  [SP-12]{{.+}}8{{.+}}4
+; DEBUG: AB @ dot.c:38
+; STRIPPED-NOT: AB @ dot.c:38
+; BOTH:  [SP-16]{{.+}}4{{.+}}4
+; DEBUG: i @ dot.c:55
+; STRIPPED-NOT: i @ dot.c:55
+; BOTH:  [SP-20]{{.+}}8{{.+}}4
+; DEBUG: B @ dot.c:32
+; STRIPPED-NOT: B @ dot.c:32
+; BOTH:  [SP-24]{{.+}}4{{.+}}4
+; DEBUG: len @ dot.c:37
+; STRIPPED-NOT: len @ dot.c:37
+; BOTH:  [SP-28]{{.+}}4{{.+}}4
+; BOTH:  [SP-32]{{.+}}4{{.+}}4
+; DEBUG: sum @ dot.c:54
+; STRIPPED-NOT: sum @ dot.c:54
+define i32 @do_work(ptr %0, ptr %1, ptr %2) #1 {
+  %4 = alloca i32, align 4
+  %5 = alloca ptr, align 8
+  %6 = alloca ptr, align 8
+  %7 = alloca ptr, align 8
+  %8 = alloca i32, align 4
+  %9 = alloca ptr, align 8
+  %10 = alloca i32, align 4
+  %11 = alloca i32, align 4
+  store ptr %0, ptr %5, align 8
+  call void @llvm.dbg.declare(metadata ptr %5, metadata !52, metadata !DIEx

[PATCH] D140423: [WIP][clang] Add PrintingPolicy callback for identifying default template arguments

2023-01-03 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added inline comments.



Comment at: clang/include/clang/AST/PrettyPrinter.h:52
+
+  enum class TriState : int { kYes, kNo, kUnknown };
+

We don't usually use the `k` prefix for enums (the style guide mentions using 
an acronym like `TS_` - though even that's unnecessary with an enum class, 
where you have to use `EnumClassName` prefix anyway, so there's no issue with 
ambiguity/name collisions of the enumerators)

But also, I'm guessing we probably use `std::optional` for this sort of 
thing more frequently than defining a three-state enum (even though 
`std::optional` can be a bit awkward to use/error prone, it's not too bad 
for limited uses like this).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140423

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


[PATCH] D140868: [C] Make (c ? e1 : e2) noreturn only if both e1 and e2 are noreturn

2023-01-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

LGTM, but I have a suggested elaboration for the comment.




Comment at: clang/lib/AST/ASTContext.cpp:10258
 
-  // FIXME: some uses, e.g. conditional exprs, really want this to be 'both'.
-  bool NoReturn = lbaseInfo.getNoReturn() || rbaseInfo.getNoReturn();
-
+  // For ConditionalOperator, the result type is noreturn if both operands are
+  // noreturn.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140868

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


[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/4)

2023-01-03 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 added a comment.

While I was not suggesting using compilation database instead of the approach 
taken by this patch, I appreciate your explanation. But it still leaves me 
wondering whether generating one compilation database per file-to-be-scanned 
would be a reasonable strategy? That might help us avoid increasing the 
complexity of the `clang-scan-deps` command-line interface.

If we really want to avoid writing compilation databases, I think that 
`clang-scan-deps -format=p1689 --  -std=c++20  -o ` 
is much cleaner/generic/reusable than `clang-scan-deps -format=p1689 
--p1689-targeted-file-name= --p1689-targeted-output= -- 
 -std=c++20`. I understand that `FixedCompilationDatabase` 
currently doesn't know what the input/output is unless you explicitly pass that 
in. What if we were able to construct some kind of `CompilationDatabase` by 
properly parsing the given complete command line into `CompilerInvocation` and 
poking at `FrontendOptions`?




Comment at: clang/lib/Tooling/CompilationDatabase.cpp:378
+  if (!FilePath.empty())
+ToolCommandLine.push_back(std::string(FilePath));
+  CompileCommands.emplace_back(Directory, FilePath, std::move(ToolCommandLine),

I don't think this is correct in cases where the command line already contains 
an input, or is malformed. As an example, if the last element is an option that 
expects a value (e.g. `"-ferror-limit"`), appending `FilePath` will trigger 
diagnostics in the driver that the argument to `-ferror-limit` is not numeric 
and that an input needs to be specified. While the correct/expected behavior 
would be to emit single diagnostic complaining about missing value for the last 
option.

Clang's tooling is littered with ad-hoc command-line parsing/manipulation and 
it's rarely handling all the edge-cases. I'd prefer this complexity lived 
somewhere outside of Clang, in the tool that actually constructed the command 
line in the first place.


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

https://reviews.llvm.org/D137534

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


[PATCH] D140868: [C] Make (c ? e1 : e2) noreturn only if both e1 and e2 are noreturn

2023-01-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 486061.
MaskRay marked an inline comment as done.
MaskRay added a comment.

use @rjmccall's comment (thanks a lot!)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140868

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGen/attr-noreturn.c

Index: clang/test/CodeGen/attr-noreturn.c
===
--- clang/test/CodeGen/attr-noreturn.c
+++ clang/test/CodeGen/attr-noreturn.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -S -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -x c++ -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-CXX
 
 typedef void (*fptrs_t[4])(void);
 fptrs_t p __attribute__((noreturn));
@@ -8,3 +9,24 @@
 }
 // CHECK: call void
 // CHECK-NEXT: unreachable
+
+// CHECK-LABEL: @test_conditional(
+// CHECK: %cond = select i1 %tobool, ptr @t1, ptr @t2
+// CHECK: call void %cond(
+// CHECK: call void %cond2(
+// CHECK-NEXT:unreachable
+
+// CHECK-CXX-LABEL: @_Z16test_conditionali(
+// CHECK-CXX: %cond{{.*}} = phi ptr [ @_Z2t1i, %{{.*}} ], [ @_Z2t2i, %{{.*}} ]
+// CHECK-CXX: call void %cond{{.*}}(
+// CHECK-CXX: %cond{{.*}} = phi ptr [ @_Z2t1i, %{{.*}} ], [ @_Z2t1i, %{{.*}} ]
+// CHECK-CXX: call void %cond{{.*}}(
+// CHECK-CXX-NEXT:unreachable
+void t1(int) __attribute__((noreturn));
+void t2(int);
+__attribute__((noreturn)) void test_conditional(int a) {
+  // The conditional operator isn't noreturn because t2 isn't.
+  (a ? t1 : t2)(a);
+  // The conditional operator is noreturn.
+  (a ? t1 : t1)(a);
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -8262,7 +8262,9 @@
   lhptee = S.Context.getQualifiedType(lhptee.getUnqualifiedType(), lhQual);
   rhptee = S.Context.getQualifiedType(rhptee.getUnqualifiedType(), rhQual);
 
-  QualType CompositeTy = S.Context.mergeTypes(lhptee, rhptee);
+  QualType CompositeTy = S.Context.mergeTypes(
+  lhptee, rhptee, /*OfBlockPointer=*/false, /*Unqualified=*/false,
+  /*BlockReturnType=*/false, /*IsConditionalOperator=*/true);
 
   if (CompositeTy.isNull()) {
 // In this situation, we assume void* type. No especially good
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -10191,7 +10191,8 @@
 
 QualType ASTContext::mergeFunctionTypes(QualType lhs, QualType rhs,
 bool OfBlockPointer, bool Unqualified,
-bool AllowCXX) {
+bool AllowCXX,
+bool IsConditionalOperator) {
   const auto *lbase = lhs->castAs();
   const auto *rbase = rhs->castAs();
   const auto *lproto = dyn_cast(lbase);
@@ -10254,9 +10255,27 @@
   if (lbaseInfo.getNoCfCheck() != rbaseInfo.getNoCfCheck())
 return {};
 
-  // FIXME: some uses, e.g. conditional exprs, really want this to be 'both'.
-  bool NoReturn = lbaseInfo.getNoReturn() || rbaseInfo.getNoReturn();
-
+  // When merging declarations, it's common for supplemental information like
+  // attributes to only be present in one of the declarations, and we generally
+  // want type merging to preserve the union of information.  So a merged
+  // function type should be noreturn if it was noreturn in *either* operand
+  // type.
+  //
+  // But for the conditional operator, this is backwards.  The result of the
+  // operator could be either operand, and its type should conservatively
+  // reflect that.  So a function type in a composite type is noreturn only
+  // if it's noreturn in *both* operand types.
+  //
+  // Arguably, noreturn is a kind of subtype, and the conditional operator
+  // ought to produce the most specific common supertype of its operand types.
+  // That would differ from this rule in contravariant positions.  However,
+  // neither C nor C++ generally uses this kind of subtype reasoning.  Also,
+  // as a practical matter, it would only affect C code that does abstraction of
+  // higher-order functions (taking noreturn callbacks!), which is uncommon to
+  // say the least.  So we use the simpler rule.
+  bool NoReturn = IsConditionalOperator
+  ? lbaseInfo.getNoReturn() && rbaseInfo.getNoReturn()
+  : lbaseInfo.getNoReturn() || rbaseInfo.getNoReturn();
   if (lbaseInfo.getNoReturn() != NoReturn)
 allLTypes = false;
   if (rbaseInfo.getNoReturn() != NoReturn)
@@ -10389,9 +10408,9 @@
   return {};
 }
 
-QualType ASTContext::mergeTypes(QualType LHS, QualType RHS,
-  

[PATCH] D140868: [C] Make (c ? e1 : e2) noreturn only if both e1 and e2 are noreturn

2023-01-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

C2x clarified the behavior of *standard* attributes when determining a 
composite type, and this particular case really straddles that boundary. We 
support `[[noreturn]]` and `_Noreturn` as standard attributes in C as well as 
`__attribute__((noreturn))` (as a nonstandard attribute with the same semantics 
as the standard attribute).

C2x 6.2.7p3 says, in part: "If one of the types has a standard attribute, the 
composite type also has that attribute." which is the current behavior before 
your patch. So this change introduces a regression for C2x for the standard 
attribute spelling but is fine for the GNU attribute spelling, but do we really 
want to have different answers depending on how the user spells the attribute? 
(FWIW, this change came in as part of the "unsequenced functions" attribute 
paper: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2956.htm).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140868

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


[PATCH] D140773: [WebAssembly] Use `shufflevector` for shuffle

2023-01-03 Thread Thomas Lively via Phabricator via cfe-commits
tlively accepted this revision.
tlively added a comment.
This revision is now accepted and ready to land.

LGTM % comment. Thanks for taking this!




Comment at: llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll:159-160
 ; CHECK-NEXT: return $pop[[R]]{{$}}
-declare <16 x i8> @llvm.wasm.shuffle(
-  <16 x i8>, <16 x i8>, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32,
-  i32, i32, i32, i32, i32)
 define <16 x i8> @shuffle_v16i8(<16 x i8> %x, <16 x i8> %y) {
-  %res = call <16 x i8> @llvm.wasm.shuffle(<16 x i8> %x, <16 x i8> %y,
-  i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7,
-  i32 8, i32 9, i32 10, i32 11, i32 12, i32 13, i32 14, i32 35)
+  %res = shufflevector <16 x i8> %x, <16 x i8> %y, <16 x i32> 
   ret <16 x i8> %res

Since this no longer tests codegen for an intrinsic function, could you move it 
to a separate test file? It could be named  something simple and short like 
`simd-shuffle.ll`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140773

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


[PATCH] D140921: [clang][dataflow] Fix bug in optional-checker's handling of nullopt constructor.

2023-01-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision.
ymandel added reviewers: xazax.hun, gribozavr2.
Herald added subscribers: martong, rnkovacs.
Herald added a reviewer: NoQ.
Herald added a project: All.
ymandel requested review of this revision.
Herald added a project: clang.

Currently, the checker only recognizes the nullopt constructor when it is called
without sugar, resulting in a crash in the (rare) case where it has been wrapped
in sugar. This relaxes the constraint by checking the constructor decl directly
(which always contains the same, desugared form) rather than the construct
expression (where the spelling depends on the context).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140921

Files:
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp


Index: 
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1498,6 +1498,23 @@
   )");
 }
 
+TEST_P(UncheckedOptionalAccessTest, NulloptConstructorWithSugaredType) {
+  ExpectDiagnosticsFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+template 
+using wrapper = T;
+
+template 
+wrapper wrap(T);
+
+void target() {
+  $ns::$optional opt(wrap($ns::nullopt));
+  opt.value(); // [[unsafe]]
+}
+  )");
+}
+
 TEST_P(UncheckedOptionalAccessTest, InPlaceConstructor) {
   ExpectDiagnosticsFor(R"(
 #include "unchecked_optional_access_test.h"
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -22,6 +22,7 @@
 #include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "clang/Analysis/FlowSensitive/StorageLocation.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/StringRef.h"
@@ -100,8 +101,10 @@
 }
 
 auto isOptionalNulloptConstructor() {
-  return cxxConstructExpr(hasOptionalType(), argumentCountIs(1),
-  hasArgument(0, hasNulloptType()));
+  return cxxConstructExpr(
+  hasOptionalType(),
+  hasDeclaration(cxxConstructorDecl(parameterCountIs(1),
+hasParameter(0, hasNulloptType();
 }
 
 auto isOptionalInPlaceConstructor() {


Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1498,6 +1498,23 @@
   )");
 }
 
+TEST_P(UncheckedOptionalAccessTest, NulloptConstructorWithSugaredType) {
+  ExpectDiagnosticsFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+template 
+using wrapper = T;
+
+template 
+wrapper wrap(T);
+
+void target() {
+  $ns::$optional opt(wrap($ns::nullopt));
+  opt.value(); // [[unsafe]]
+}
+  )");
+}
+
 TEST_P(UncheckedOptionalAccessTest, InPlaceConstructor) {
   ExpectDiagnosticsFor(R"(
 #include "unchecked_optional_access_test.h"
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -22,6 +22,7 @@
 #include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "clang/Analysis/FlowSensitive/StorageLocation.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/StringRef.h"
@@ -100,8 +101,10 @@
 }
 
 auto isOptionalNulloptConstructor() {
-  return cxxConstructExpr(hasOptionalType(), argumentCountIs(1),
-  hasArgument(0, hasNulloptType()));
+  return cxxConstructExpr(
+  hasOptionalType(),
+  hasDeclaration(cxxConstructorDecl(parameterCountIs(1),
+hasParameter(0, hasNulloptType();
 }
 
 auto isOptionalInPlaceConstructor() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


  1   2   >