[PATCH] D103678: [Format] Fix incorrect pointer/reference detection

[Yilong Guo via Phabricator via cfe-commits]

Nuu added a comment.

In D103678#2811302 , 
@HazardyKnusperkeks wrote:

> Do you need some one to commit this? If yes please state name and email, some 
> one will chime in to commit it.

Yes, please someone help commit this.

Name: Yilong Guo
Email: yilong@intel.com

Thanks in advance!


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

https://reviews.llvm.org/D103678

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

[PATCH] D104300: [analyzer] Handle std::swap for std::unique_ptr

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD updated this revision to Diff 352627.
RedDocMD added a comment.

Marking and un-marking interestingness


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104300

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/test/Analysis/smart-ptr-text-output.cpp

Index: clang/test/Analysis/smart-ptr-text-output.cpp
===
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -69,20 +69,17 @@
 
 void derefOnSwappedNullPtr() {
   std::unique_ptr P(new A()); // expected-note {{Smart pointer 'P' is constructed}}
-  std::unique_ptr PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
-  P.swap(PNull); // expected-note {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
+  std::unique_ptr PNull;
+  P.swap(PNull);
   PNull->foo(); // No warning.
   (*P).foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
   // expected-note@-1{{Dereference of null smart pointer 'P'}}
 }
 
-// FIXME: Fix this test when "std::swap" is modeled seperately.
 void derefOnStdSwappedNullPtr() {
   std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
-  std::unique_ptr PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
-  std::swap(P, PNull); // expected-note@Inputs/system-header-simulator-cxx.h:979 {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
-  // expected-note@-1 {{Calling 'swap'}}
-  // expected-note@-2 {{Returning from 'swap'}}
+  std::unique_ptr PNull;
+  std::swap(P, PNull);
   P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
   // expected-note@-1{{Dereference of null smart pointer 'P'}}
 }
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2261,6 +2261,14 @@
 markInteresting(meta->getRegion(), TKind);
 }
 
+void PathSensitiveBugReport::markNotInteresting(SymbolRef sym) {
+  if (!sym)
+return;
+  InterestingSymbols.erase(sym);
+  if (const auto *meta = dyn_cast(sym))
+markNotInteresting(meta->getRegion());
+}
+
 void PathSensitiveBugReport::markInteresting(const MemRegion *R,
  bugreporter::TrackingKind TKind) {
   if (!R)
@@ -2273,6 +2281,17 @@
 markInteresting(SR->getSymbol(), TKind);
 }
 
+void PathSensitiveBugReport::markNotInteresting(const MemRegion *R) {
+  if (!R)
+return;
+
+  R = R->getBaseRegion();
+  InterestingRegions.erase(R);
+
+  if (const auto *SR = dyn_cast(R))
+markNotInteresting(SR->getSymbol());
+}
+
 void PathSensitiveBugReport::markInteresting(SVal V,
  bugreporter::TrackingKind TKind) {
   markInteresting(V.getAsRegion(), TKind);
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -60,7 +60,7 @@
 private:
   void handleReset(const CallEvent &Call, CheckerContext &C) const;
   void handleRelease(const CallEvent &Call, CheckerContext &C) const;
-  void handleSwap(const CallEvent &Call, CheckerContext &C) const;
+  void handleSwapMethod(const CallEvent &Call, CheckerContext &C) const;
   void handleGet(const CallEvent &Call, CheckerContext &C) const;
   bool handleAssignOp(const CallEvent &Call, CheckerContext &C) const;
   bool handleMoveCtr(const CallEvent &Call, CheckerContext &C,
@@ -68,14 +68,17 @@
   bool updateMovedSmartPointers(CheckerContext &C, const MemRegion *ThisRegion,
 const MemRegion *OtherSmartPtrRegion) const;
   void handleBoolConversion(const CallEvent &Call, CheckerContext &C) const;
+  bool handleSwap(ProgramStateRef State, const MemRegion *FirstRegion,
+  const MemRegion *SecondRegion, CheckerContext &C) const;
 
   using SmartPtrMethodHandlerFn =
   void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const;
   CallDescriptionMap SmartPtrMethodHandlers{
   {{"reset"}, &SmartPtrModeling::handleReset},
   {{"release"}, &SmartPtrModeling::handleRelease},
-  {{"swap", 1}, &SmartPtrModeling::handleSwap},
+  {{"swap", 1}, &SmartPtrModeling::handleSwapMethod},
   {{"get"}, &SmartPtrModeling::handleGet}};
+  const CallDescription StdSwapCall{{"std", "swap"}, 2};
 };
 } // end of anonymous namespace
 
@@ -91,11 +94,15 @@
 return false;
 
   const auto *Record

[PATCH] D104136: [analyzer] Add better tracking for RetainCountChecker leak warnings

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Aha, alright! So the tracker tracked back to where the tracked expression got 
computed to node `N`, which is the return value of some identity function. 
Unless its explicitly told that there is a link in between the return value and 
the parameter, the tracker can't figure this out that it could track further 
(why would it, right?). So it asks its handlers whether they want to do 
anything with `N`, which `IdentityHandler` does -- it digs out the node where 
the parameter was evaluated, and tells the tracker to go on. Awesome!

I guess we could put `IdentityHandler` in an anonymous namespace?




Comment at: 
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp:945-947
+ExprEngine &Eng = C.getStateManager().getOwningEngine();
+// Let's mark this place with a special tag.
+Tag = Eng.getDataTags().make(CE, BindReturnTo);

I don't know ObjC at all, but is this identity obvious? Do you not need a 
`NoteTag` to say that "Foo is an identity function, its return value equals the 
parameter"? Or, in the possession of the actual `PathSensitiveBugReport` that 
you can retrieve in the handler, you could drop a note there, maybe?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104136

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

[PATCH] D103387: [clangd] Fix feature modules to drop diagnostics

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 352628.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Bail out early before filling in diag info
- Move isExcluded check into handleDiagnostics, rather than handling it during

flushing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103387

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/Diagnostics.h
  clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp

Index: clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
===
--- clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
+++ clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
@@ -6,6 +6,7 @@
 //
 //===--===//
 
+#include "Annotations.h"
 #include "FeatureModule.h"
 #include "Selection.h"
 #include "TestTU.h"
@@ -53,6 +54,37 @@
   EXPECT_EQ(Actual->get()->id(), TweakID);
 }
 
+TEST(FeatureModulesTest, SuppressDiags) {
+  struct DiagModifierModule final : public FeatureModule {
+struct Listener : public FeatureModule::ASTListener {
+  void sawDiagnostic(const clang::Diagnostic &Info,
+ clangd::Diag &Diag) override {
+Diag.Severity = DiagnosticsEngine::Ignored;
+  }
+};
+std::unique_ptr astListeners() override {
+  return std::make_unique();
+};
+  };
+  FeatureModuleSet FMS;
+  FMS.add(std::make_unique());
+
+  Annotations Code("[[test]]; /* error-ok */");
+  TestTU TU;
+  TU.Code = Code.code().str();
+
+  {
+auto AST = TU.build();
+EXPECT_THAT(*AST.getDiagnostics(), testing::Not(testing::IsEmpty()));
+  }
+
+  TU.FeatureModules = &FMS;
+  {
+auto AST = TU.build();
+EXPECT_THAT(*AST.getDiagnostics(), testing::IsEmpty());
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Diagnostics.h
===
--- clang-tools-extra/clangd/Diagnostics.h
+++ clang-tools-extra/clangd/Diagnostics.h
@@ -171,7 +171,6 @@
   SourceManager *OrigSrcMgr = nullptr;
 
   llvm::DenseSet> IncludedErrorLocations;
-  bool LastPrimaryDiagnosticWasSuppressed = false;
 };
 
 /// Determine whether a (non-clang-tidy) diagnostic is suppressed by config.
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -76,10 +76,10 @@
   return false;
 }
 
-bool isExcluded(const Diag &D) {
+bool isExcluded(unsigned DiagID) {
   // clang will always fail parsing MS ASM, we don't link in desc + asm parser.
-  if (D.ID == clang::diag::err_msasm_unable_to_create_target ||
-  D.ID == clang::diag::err_msasm_unsupported_arch)
+  if (DiagID == clang::diag::err_msasm_unable_to_create_target ||
+  DiagID == clang::diag::err_msasm_unsupported_arch)
 return true;
   return false;
 }
@@ -726,44 +726,42 @@
 // Handle the new main diagnostic.
 flushLastDiag();
 
-if (Adjuster) {
+LastDiag = Diag();
+// FIXME: Merge with feature modules.
+if (Adjuster)
   DiagLevel = Adjuster(DiagLevel, Info);
-  if (DiagLevel == DiagnosticsEngine::Ignored) {
-LastPrimaryDiagnosticWasSuppressed = true;
-return;
-  }
-}
-LastPrimaryDiagnosticWasSuppressed = false;
 
-LastDiag = Diag();
 FillDiagBase(*LastDiag);
+if (isExcluded(LastDiag->ID))
+  LastDiag->Severity = DiagnosticsEngine::Ignored;
+if (DiagCB)
+  DiagCB(Info, *LastDiag);
+// Don't bother filling in the rest if diag is going to be dropped.
+if (LastDiag->Severity == DiagnosticsEngine::Ignored)
+  return;
+
 LastDiagLoc.emplace(Info.getLocation(), Info.getSourceManager());
 LastDiagOriginallyError = OriginallyError;
-
 if (!Info.getFixItHints().empty())
   AddFix(true /* try to invent a message instead of repeating the diag */);
 if (Fixer) {
-  auto ExtraFixes = Fixer(DiagLevel, Info);
+  auto ExtraFixes = Fixer(LastDiag->Severity, Info);
   LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(),
  ExtraFixes.end());
 }
-if (DiagCB)
-  DiagCB(Info, *LastDiag);
   } else {
 // Handle a note to an existing diagnostic.
-
-// If a diagnostic was suppressed due to the suppression filter,
-// also suppress notes associated with it.
-if (LastPrimaryDiagnosticWasSuppressed) {
-  return;
-}
-
 if (!LastDiag) {
   assert(false && "Adding a note without main diagnostic");
   IgnoreDiagnostics::log(DiagLevel, Info);
   return;
 }
 
+// If a diagnostic was suppressed due to the suppression filter,
+// also suppress notes associated with it.
+if (LastDiag->Severity == DiagnosticsEngi

[PATCH] D103527: [Clang][RISCV] Implement vlseg and vlsegff.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Do you think you can split `vlseg` and `vlseg_ff` tests in two different files? 
So we (potentially) reduce the test latency by half in multicore systems.

Other than this, LGTM.

We're a bit split in https://github.com/riscv/rvv-intrinsic-doc/issues/95 
However there are mechanisms to assist in any transition should we be able to 
implement in the future alternative interfaces that we prefer over these ones.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103527

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

[clang-tools-extra] b662651 - [clangd] Use command line adjusters for inserting compile flags

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2021-06-17T09:24:53+02:00
New Revision: b662651586bedd5350914f64463fe415105785c8

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

LOG: [clangd] Use command line adjusters for inserting compile flags

This fixes issues with `--` in the compile flags.

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

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

Added: 


Modified: 
clang-tools-extra/clangd/CompileCommands.cpp
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CompileCommands.cpp 
b/clang-tools-extra/clangd/CompileCommands.cpp
index 7966b7dfa8a3d..633d13b8b9f0d 100644
--- a/clang-tools-extra/clangd/CompileCommands.cpp
+++ b/clang-tools-extra/clangd/CompileCommands.cpp
@@ -20,6 +20,8 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -209,14 +211,20 @@ void CommandMangler::adjust(std::vector 
&Cmd) const {
   Cmd = tooling::getStripPluginsAdjuster()(Cmd, "");
   Cmd = tooling::getClangSyntaxOnlyAdjuster()(Cmd, "");
 
+  std::vector ToAppend;
   if (ResourceDir && !Has("-resource-dir"))
-Cmd.push_back(("-resource-dir=" + *ResourceDir));
+ToAppend.push_back(("-resource-dir=" + *ResourceDir));
 
   // Don't set `-isysroot` if it is already set or if `--sysroot` is set.
   // `--sysroot` is a superset of the `-isysroot` argument.
   if (Sysroot && !Has("-isysroot") && !Has("--sysroot")) {
-Cmd.push_back("-isysroot");
-Cmd.push_back(*Sysroot);
+ToAppend.push_back("-isysroot");
+ToAppend.push_back(*Sysroot);
+  }
+
+  if (!ToAppend.empty()) {
+Cmd = tooling::getInsertArgumentAdjuster(
+std::move(ToAppend), tooling::ArgumentInsertPosition::END)(Cmd, "");
   }
 
   if (!Cmd.empty()) {
@@ -504,7 +512,6 @@ void ArgStripper::process(std::vector &Args) 
const {
   Args.resize(Write);
 }
 
-
 std::string printArgv(llvm::ArrayRef Args) {
   std::string Buf;
   llvm::raw_string_ostream OS(Buf);

diff  --git a/clang-tools-extra/clangd/ConfigCompile.cpp 
b/clang-tools-extra/clangd/ConfigCompile.cpp
index 16d66f6154f25..438dd74d866c6 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -47,6 +47,7 @@
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/SourceMgr.h"
+#include 
 #include 
 
 namespace clang {
@@ -270,7 +271,9 @@ struct FragmentCompiler {
 Add.push_back(std::move(*A));
   Out.Apply.push_back([Add(std::move(Add))](const Params &, Config &C) {
 C.CompileFlags.Edits.push_back([Add](std::vector &Args) {
-  Args.insert(Args.end(), Add.begin(), Add.end());
+  // The point to insert at. Just append when `--` isn't present.
+  auto It = llvm::find(Args, "--");
+  Args.insert(It, Add.begin(), Add.end());
 });
   });
 }

diff  --git a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp 
b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
index 33a4afb6374c4..7fb2ae7664fa7 100644
--- a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -41,13 +41,14 @@ TEST(CommandMangler, Everything) {
   Mangler.ClangPath = testPath("fake/clang");
   Mangler.ResourceDir = testPath("fake/resources");
   Mangler.Sysroot = testPath("fake/sysroot");
-  std::vector Cmd = {"clang++", "-Xclang", "-load", "-Xclang",
-  "plugin",  "-MF", "dep",   "foo.cc"};
+  std::vector Cmd = {"clang++", "-Xclang", "-load",
+  "-Xclang", "plugin",  "-MF",
+  "dep", "--",  "foo.cc"};
   Mangler.adjust(Cmd);
-  EXPECT_THAT(Cmd, ElementsAre(testPath("fake/clang++"), "foo.cc",
-   "-fsyntax-only",
+  EXPECT_THAT(Cmd, ElementsAre(testPath("fake/clang++"), "-fsyntax-only",
"-resource-dir=" + testPath("fake/resources"),
-   "-isysroot", testPath("fake/sysroot")));
+   "-isysroot", testPath("fake/sysroot"), "--",
+   "foo.cc"));
 }
 
 TEST(CommandMangler, ResourceDir) {
@@ -378,4 +379,3 @@ TEST(PrintArgvTest, All) {
 } // namespace
 } // namespace clangd
 } // namespace clang
-

diff  --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp 
b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
index 5e7fce8016f12..381180381f36f 100644
--- a/clang-tools-e

[PATCH] D99523: [clangd] Use command line adjusters for inserting compile flags

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb662651586be: [clangd] Use command line adjusters for 
inserting compile flags (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99523

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -123,12 +123,12 @@
 TEST_F(ConfigCompileTests, CompileCommands) {
   Frag.CompileFlags.Add.emplace_back("-foo");
   Frag.CompileFlags.Remove.emplace_back("--include-directory=");
-  std::vector Argv = {"clang", "-I", "bar/", "a.cc"};
+  std::vector Argv = {"clang", "-I", "bar/", "--", "a.cc"};
   EXPECT_TRUE(compileAndApply());
   EXPECT_THAT(Conf.CompileFlags.Edits, SizeIs(2));
   for (auto &Edit : Conf.CompileFlags.Edits)
 Edit(Argv);
-  EXPECT_THAT(Argv, ElementsAre("clang", "a.cc", "-foo"));
+  EXPECT_THAT(Argv, ElementsAre("clang", "-foo", "--", "a.cc"));
 }
 
 TEST_F(ConfigCompileTests, CompilationDatabase) {
Index: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
===
--- clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -41,13 +41,14 @@
   Mangler.ClangPath = testPath("fake/clang");
   Mangler.ResourceDir = testPath("fake/resources");
   Mangler.Sysroot = testPath("fake/sysroot");
-  std::vector Cmd = {"clang++", "-Xclang", "-load", "-Xclang",
-  "plugin",  "-MF", "dep",   "foo.cc"};
+  std::vector Cmd = {"clang++", "-Xclang", "-load",
+  "-Xclang", "plugin",  "-MF",
+  "dep", "--",  "foo.cc"};
   Mangler.adjust(Cmd);
-  EXPECT_THAT(Cmd, ElementsAre(testPath("fake/clang++"), "foo.cc",
-   "-fsyntax-only",
+  EXPECT_THAT(Cmd, ElementsAre(testPath("fake/clang++"), "-fsyntax-only",
"-resource-dir=" + testPath("fake/resources"),
-   "-isysroot", testPath("fake/sysroot")));
+   "-isysroot", testPath("fake/sysroot"), "--",
+   "foo.cc"));
 }
 
 TEST(CommandMangler, ResourceDir) {
@@ -378,4 +379,3 @@
 } // namespace
 } // namespace clangd
 } // namespace clang
-
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -47,6 +47,7 @@
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/SourceMgr.h"
+#include 
 #include 
 
 namespace clang {
@@ -270,7 +271,9 @@
 Add.push_back(std::move(*A));
   Out.Apply.push_back([Add(std::move(Add))](const Params &, Config &C) {
 C.CompileFlags.Edits.push_back([Add](std::vector &Args) {
-  Args.insert(Args.end(), Add.begin(), Add.end());
+  // The point to insert at. Just append when `--` isn't present.
+  auto It = llvm::find(Args, "--");
+  Args.insert(It, Add.begin(), Add.end());
 });
   });
 }
Index: clang-tools-extra/clangd/CompileCommands.cpp
===
--- clang-tools-extra/clangd/CompileCommands.cpp
+++ clang-tools-extra/clangd/CompileCommands.cpp
@@ -20,6 +20,8 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -209,14 +211,20 @@
   Cmd = tooling::getStripPluginsAdjuster()(Cmd, "");
   Cmd = tooling::getClangSyntaxOnlyAdjuster()(Cmd, "");
 
+  std::vector ToAppend;
   if (ResourceDir && !Has("-resource-dir"))
-Cmd.push_back(("-resource-dir=" + *ResourceDir));
+ToAppend.push_back(("-resource-dir=" + *ResourceDir));
 
   // Don't set `-isysroot` if it is already set or if `--sysroot` is set.
   // `--sysroot` is a superset of the `-isysroot` argument.
   if (Sysroot && !Has("-isysroot") && !Has("--sysroot")) {
-Cmd.push_back("-isysroot");
-Cmd.push_back(*Sysroot);
+ToAppend.push_back("-isysroot");
+ToAppend.push_back(*Sysroot);
+  }
+
+  if (!ToAppend.empty()) {
+Cmd = tooling::getInsertArgumentAdjuster(
+std::move(ToAppend), tooling::ArgumentInsertPosition::END)(Cmd, "");
   }
 
   if (!Cmd.empty()) {
@@ -504,7 +512,6 @@
   Args.resize(Write);
 }
 
-
 std::string printArgv(llvm::ArrayRef Args

[PATCH] D103387: [clangd] Fix feature modules to drop diagnostics

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.

LG, I'm a bit sad to see we're not able to skip much work in the 
diagnostics-outside-main-file case but that's a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103387

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

[clang-tools-extra] 204014e - [clangd] Fix feature modules to drop diagnostics

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2021-06-17T09:29:29+02:00
New Revision: 204014ec7557cde214ee8aae1927bf9976f2

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

LOG: [clangd] Fix feature modules to drop diagnostics

Ignored diagnostics were only checked after level adjusters and assumed
it would stay the same for the rest. But it can also be modified by
FeatureModules.

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

Added: 


Modified: 
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/Diagnostics.h
clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Diagnostics.cpp 
b/clang-tools-extra/clangd/Diagnostics.cpp
index 88ec7aaa9b26..089802db4f69 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -76,10 +76,10 @@ bool mentionsMainFile(const Diag &D) {
   return false;
 }
 
-bool isExcluded(const Diag &D) {
+bool isExcluded(unsigned DiagID) {
   // clang will always fail parsing MS ASM, we don't link in desc + asm parser.
-  if (D.ID == clang::diag::err_msasm_unable_to_create_target ||
-  D.ID == clang::diag::err_msasm_unsupported_arch)
+  if (DiagID == clang::diag::err_msasm_unable_to_create_target ||
+  DiagID == clang::diag::err_msasm_unsupported_arch)
 return true;
   return false;
 }
@@ -726,44 +726,42 @@ void 
StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
 // Handle the new main diagnostic.
 flushLastDiag();
 
-if (Adjuster) {
+LastDiag = Diag();
+// FIXME: Merge with feature modules.
+if (Adjuster)
   DiagLevel = Adjuster(DiagLevel, Info);
-  if (DiagLevel == DiagnosticsEngine::Ignored) {
-LastPrimaryDiagnosticWasSuppressed = true;
-return;
-  }
-}
-LastPrimaryDiagnosticWasSuppressed = false;
 
-LastDiag = Diag();
 FillDiagBase(*LastDiag);
+if (isExcluded(LastDiag->ID))
+  LastDiag->Severity = DiagnosticsEngine::Ignored;
+if (DiagCB)
+  DiagCB(Info, *LastDiag);
+// Don't bother filling in the rest if diag is going to be dropped.
+if (LastDiag->Severity == DiagnosticsEngine::Ignored)
+  return;
+
 LastDiagLoc.emplace(Info.getLocation(), Info.getSourceManager());
 LastDiagOriginallyError = OriginallyError;
-
 if (!Info.getFixItHints().empty())
   AddFix(true /* try to invent a message instead of repeating the diag */);
 if (Fixer) {
-  auto ExtraFixes = Fixer(DiagLevel, Info);
+  auto ExtraFixes = Fixer(LastDiag->Severity, Info);
   LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(),
  ExtraFixes.end());
 }
-if (DiagCB)
-  DiagCB(Info, *LastDiag);
   } else {
 // Handle a note to an existing diagnostic.
-
-// If a diagnostic was suppressed due to the suppression filter,
-// also suppress notes associated with it.
-if (LastPrimaryDiagnosticWasSuppressed) {
-  return;
-}
-
 if (!LastDiag) {
   assert(false && "Adding a note without main diagnostic");
   IgnoreDiagnostics::log(DiagLevel, Info);
   return;
 }
 
+// If a diagnostic was suppressed due to the suppression filter,
+// also suppress notes associated with it.
+if (LastDiag->Severity == DiagnosticsEngine::Ignored)
+  return;
+
 if (!Info.getFixItHints().empty()) {
   // A clang note with fix-it is not a separate diagnostic in clangd. We
   // attach it as a Fix to the main diagnostic instead.
@@ -788,7 +786,7 @@ void StoreDiags::flushLastDiag() {
 LastDiag.reset();
   });
 
-  if (isExcluded(*LastDiag))
+  if (LastDiag->Severity == DiagnosticsEngine::Ignored)
 return;
   // Move errors that occur from headers into main file.
   if (!LastDiag->InsideMainFile && LastDiagLoc && LastDiagOriginallyError) {

diff  --git a/clang-tools-extra/clangd/Diagnostics.h 
b/clang-tools-extra/clangd/Diagnostics.h
index 9c2235dce9a9..169b4aee6895 100644
--- a/clang-tools-extra/clangd/Diagnostics.h
+++ b/clang-tools-extra/clangd/Diagnostics.h
@@ -171,7 +171,6 @@ class StoreDiags : public DiagnosticConsumer {
   SourceManager *OrigSrcMgr = nullptr;
 
   llvm::DenseSet> IncludedErrorLocations;
-  bool LastPrimaryDiagnosticWasSuppressed = false;
 };
 
 /// Determine whether a (non-clang-tidy) diagnostic is suppressed by config.

diff  --git a/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp 
b/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
index 2686c43b3530..124cf9320c3e 100644
--- a/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
@@ -6,6 +6,7 @@
 //
 
//===

[PATCH] D103387: [clangd] Fix feature modules to drop diagnostics

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG204014ec7557: [clangd] Fix feature modules to drop 
diagnostics (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103387

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/Diagnostics.h
  clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp

Index: clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
===
--- clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
+++ clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
@@ -6,6 +6,7 @@
 //
 //===--===//
 
+#include "Annotations.h"
 #include "FeatureModule.h"
 #include "Selection.h"
 #include "TestTU.h"
@@ -53,6 +54,37 @@
   EXPECT_EQ(Actual->get()->id(), TweakID);
 }
 
+TEST(FeatureModulesTest, SuppressDiags) {
+  struct DiagModifierModule final : public FeatureModule {
+struct Listener : public FeatureModule::ASTListener {
+  void sawDiagnostic(const clang::Diagnostic &Info,
+ clangd::Diag &Diag) override {
+Diag.Severity = DiagnosticsEngine::Ignored;
+  }
+};
+std::unique_ptr astListeners() override {
+  return std::make_unique();
+};
+  };
+  FeatureModuleSet FMS;
+  FMS.add(std::make_unique());
+
+  Annotations Code("[[test]]; /* error-ok */");
+  TestTU TU;
+  TU.Code = Code.code().str();
+
+  {
+auto AST = TU.build();
+EXPECT_THAT(*AST.getDiagnostics(), testing::Not(testing::IsEmpty()));
+  }
+
+  TU.FeatureModules = &FMS;
+  {
+auto AST = TU.build();
+EXPECT_THAT(*AST.getDiagnostics(), testing::IsEmpty());
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Diagnostics.h
===
--- clang-tools-extra/clangd/Diagnostics.h
+++ clang-tools-extra/clangd/Diagnostics.h
@@ -171,7 +171,6 @@
   SourceManager *OrigSrcMgr = nullptr;
 
   llvm::DenseSet> IncludedErrorLocations;
-  bool LastPrimaryDiagnosticWasSuppressed = false;
 };
 
 /// Determine whether a (non-clang-tidy) diagnostic is suppressed by config.
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -76,10 +76,10 @@
   return false;
 }
 
-bool isExcluded(const Diag &D) {
+bool isExcluded(unsigned DiagID) {
   // clang will always fail parsing MS ASM, we don't link in desc + asm parser.
-  if (D.ID == clang::diag::err_msasm_unable_to_create_target ||
-  D.ID == clang::diag::err_msasm_unsupported_arch)
+  if (DiagID == clang::diag::err_msasm_unable_to_create_target ||
+  DiagID == clang::diag::err_msasm_unsupported_arch)
 return true;
   return false;
 }
@@ -726,44 +726,42 @@
 // Handle the new main diagnostic.
 flushLastDiag();
 
-if (Adjuster) {
+LastDiag = Diag();
+// FIXME: Merge with feature modules.
+if (Adjuster)
   DiagLevel = Adjuster(DiagLevel, Info);
-  if (DiagLevel == DiagnosticsEngine::Ignored) {
-LastPrimaryDiagnosticWasSuppressed = true;
-return;
-  }
-}
-LastPrimaryDiagnosticWasSuppressed = false;
 
-LastDiag = Diag();
 FillDiagBase(*LastDiag);
+if (isExcluded(LastDiag->ID))
+  LastDiag->Severity = DiagnosticsEngine::Ignored;
+if (DiagCB)
+  DiagCB(Info, *LastDiag);
+// Don't bother filling in the rest if diag is going to be dropped.
+if (LastDiag->Severity == DiagnosticsEngine::Ignored)
+  return;
+
 LastDiagLoc.emplace(Info.getLocation(), Info.getSourceManager());
 LastDiagOriginallyError = OriginallyError;
-
 if (!Info.getFixItHints().empty())
   AddFix(true /* try to invent a message instead of repeating the diag */);
 if (Fixer) {
-  auto ExtraFixes = Fixer(DiagLevel, Info);
+  auto ExtraFixes = Fixer(LastDiag->Severity, Info);
   LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(),
  ExtraFixes.end());
 }
-if (DiagCB)
-  DiagCB(Info, *LastDiag);
   } else {
 // Handle a note to an existing diagnostic.
-
-// If a diagnostic was suppressed due to the suppression filter,
-// also suppress notes associated with it.
-if (LastPrimaryDiagnosticWasSuppressed) {
-  return;
-}
-
 if (!LastDiag) {
   assert(false && "Adding a note without main diagnostic");
   IgnoreDiagnostics::log(DiagLevel, Info);
   return;
 }
 
+// If a diagnostic was suppressed due to the suppression filter,
+// also suppress notes associated with it.
+if (LastDiag->Severity == DiagnosticsEngine::Ignored)
+ 

[PATCH] D104136: [analyzer] Add better tracking for RetainCountChecker leak warnings

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D104136#2823834 , @Szelethus wrote:

> Aha, alright! So the tracker tracked back to where the tracked expression got 
> computed in node `N`, which is the return value of some identity function. 
> Unless its explicitly told that there is a link in between the return value 
> and the parameter, the tracker can't figure this out that it could track 
> further (why would it, right?). So it asks its handlers whether they want to 
> do anything with `N`, which `IdentityHandler` does -- it digs out the node 
> where the parameter was evaluated, and tells the tracker to go on with that. 
> Awesome!

That's exactly it!

> I guess we could put `IdentityHandler` in an anonymous namespace?

Sure thing!




Comment at: 
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp:945-947
+ExprEngine &Eng = C.getStateManager().getOwningEngine();
+// Let's mark this place with a special tag.
+Tag = Eng.getDataTags().make(CE, BindReturnTo);

Szelethus wrote:
> I don't know ObjC at all, but is this identity obvious? Do you not need a 
> `NoteTag` to say that "Foo is an identity function, its return value equals 
> the parameter"? Or, in the possession of the actual `PathSensitiveBugReport` 
> that you can retrieve in the handler, you could drop a note there, maybe?
These are usually some type of casts.  And we already provide notes when this 
value gets into some other region (like `'newPtr' initialized to the value of 
'originalPtr'`).  I think that they are enough.  And if we want something 
better, I think we need to tweak that messaging (`StoreHandler` now allows 
that) instead of crowding user with even more notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104136

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2506-2520
+  for (auto It = NTL.findByWidth(C.getIntWidth(T)) - 1; It >= NTL.begin();
+   --It) {
+SymbolRef S = State->getSymbolManager().getCastSymbol(
+RootSym, RootSym->getType(), *It);
+if (const RangeSet *RS = getConstraint(State, S)) {
+  RangeSet TruncR = F.castTo(R, *It);
+  TruncR = F.intersect(*RS, TruncR);

What's the point of this when you do reverse operation in `Inferrer`?

As far as I understood, in `VisitSymbolCast`, you iterate over larger types and 
see if the same symbol was casted to any of those, and if yes you truncate the 
result and use that range.
Here, when we are about to set the constraint for a casted symbol, you iterate 
over smaller types, truncate this range for a smaller type, construct a cast to 
that smaller type, and add constraint for that symbol as well.

So, if this is correct, these two pieces of code DO THE SAME WORK and ONLY ONE 
should remain.


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

https://reviews.llvm.org/D103096

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

[PATCH] D104442: [libclang] Fix error handler in translateSourceLocation.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham created this revision.
simon_tatham added reviewers: akyrtzi, rsmith.
simon_tatham requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Given an invalid SourceLocation, translateSourceLocation will call
clang_getNullLocation, and then do nothing with the result. But
clang_getNullLocation has no side effects: it just constructs and
returns a null CXSourceLocation value.

Surely the intention was to //return// that null CXSourceLocation to
the caller, instead of throwing it away and pressing on anyway.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104442

Files:
  clang/tools/libclang/CXSourceLocation.h


Index: clang/tools/libclang/CXSourceLocation.h
===
--- clang/tools/libclang/CXSourceLocation.h
+++ clang/tools/libclang/CXSourceLocation.h
@@ -29,7 +29,7 @@
 translateSourceLocation(const SourceManager &SM, const LangOptions &LangOpts,
 SourceLocation Loc) {
   if (Loc.isInvalid())
-clang_getNullLocation();
+return clang_getNullLocation();
 
   CXSourceLocation Result = { { &SM, &LangOpts, },
   Loc.getRawEncoding() };


Index: clang/tools/libclang/CXSourceLocation.h
===
--- clang/tools/libclang/CXSourceLocation.h
+++ clang/tools/libclang/CXSourceLocation.h
@@ -29,7 +29,7 @@
 translateSourceLocation(const SourceManager &SM, const LangOptions &LangOpts,
 SourceLocation Loc) {
   if (Loc.isInvalid())
-clang_getNullLocation();
+return clang_getNullLocation();
 
   CXSourceLocation Result = { { &SM, &LangOpts, },
   Loc.getRawEncoding() };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 9cca5c1 - [analyzer] Make checker silencing work for non-pathsensitive bug reports

[Kristóf Umann via cfe-commits]

Author: Kirstóf Umann
Date: 2021-06-17T10:27:34+02:00
New Revision: 9cca5c1391d637b5500ada646cf136ddb38254a3

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

LOG: [analyzer] Make checker silencing work for non-pathsensitive bug reports

D66572 separated BugReport and BugReporter into basic and path sensitive
versions. As a result, checker silencing, which worked deep in the path
sensitive report generation facilities became specific to it. DeadStoresChecker,
for instance, despite being in the static analyzer, emits non-pathsensitive
reports, and was impossible to silence.

This patch moves the corresponding code before the call to the virtual function
generateDiagnosticForConsumerMap (which is overriden by the specific kinds of
bug reporters). Although we see bug reporting as relatively lightweight compared
to the analysis, this will get rid of several steps we used to throw away.

Quoting from D65379:

At a very high level, this consists of 3 steps:

For all BugReports in the same BugReportEquivClass, collect all their error
nodes in a set. With that set, create a new, trimmed ExplodedGraph whose leafs
are all error nodes.
Until a valid report is found, construct a bug path, which is yet another
ExplodedGraph, that is linear from a given error node to the root of the graph.
Run all visitors on the constructed bug path. If in this process the report got
invalidated, start over from step 2.
Checker silencing used to kick in after all of these. Now it does before any of
them :^)

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

Change-Id: Ice42939304516f2bebd05a1ea19878b89c96a25d

Added: 
clang/test/Analysis/silence-checkers.cpp

Modified: 
clang/lib/StaticAnalyzer/Core/BugReporter.cpp

Removed: 
clang/test/Analysis/silence-checkers-malloc.cpp



diff  --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp 
b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
index 61734808d1ba..6ace986b4e0d 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -1989,13 +1989,6 @@ PathDiagnosticBuilder::generate(const 
PathDiagnosticConsumer *PDC) const {
   const SourceManager &SM = getSourceManager();
   const AnalyzerOptions &Opts = getAnalyzerOptions();
 
-  // See whether we need to silence the checker/package.
-  // FIXME: This will not work if the report was emitted with an incorrect tag.
-  for (const std::string &CheckerOrPackage : Opts.SilencedCheckersAndPackages) 
{
-if (R->getBugType().getCheckerName().startswith(CheckerOrPackage))
-  return nullptr;
-  }
-
   if (!PDC->shouldGenerateDiagnostics())
 return generateEmptyDiagnosticForReport(R, getSourceManager());
 
@@ -3040,6 +3033,14 @@ void BugReporter::FlushReport(BugReportEquivClass& EQ) {
   if (!report)
 return;
 
+  // See whether we need to silence the checker/package.
+  for (const std::string &CheckerOrPackage :
+   getAnalyzerOptions().SilencedCheckersAndPackages) {
+if (report->getBugType().getCheckerName().startswith(
+CheckerOrPackage))
+  return;
+  }
+
   ArrayRef Consumers = getPathDiagnosticConsumers();
   std::unique_ptr Diagnostics =
   generateDiagnosticForConsumerMap(report, Consumers, bugReports);

diff  --git a/clang/test/Analysis/silence-checkers-malloc.cpp 
b/clang/test/Analysis/silence-checkers.cpp
similarity index 81%
rename from clang/test/Analysis/silence-checkers-malloc.cpp
rename to clang/test/Analysis/silence-checkers.cpp
index 2f6a9dd2d5b8..a2f1e60a4522 100644
--- a/clang/test/Analysis/silence-checkers-malloc.cpp
+++ b/clang/test/Analysis/silence-checkers.cpp
@@ -11,6 +11,12 @@
 // RUN:   -analyzer-checker=cplusplus.NewDelete\
 // RUN:   -analyzer-config silence-checkers="unix"
 
+// RUN: %clang_analyze_cc1 -verify="deadstore-silenced" %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling \
+// RUN:   -analyzer-checker=deadcode \
+// RUN:   -analyzer-config silence-checkers="deadcode.DeadStores"
+
 #include "Inputs/system-header-simulator-cxx.h"
 
 typedef __typeof(sizeof(int)) size_t;
@@ -38,3 +44,17 @@ void test_delete_ZERO_SIZE_PTR() {
   delete Ptr; // no-silence-warning{{Argument to 'delete' is a constant 
address (16), which is not memory allocated by 'new' [cplusplus.NewDelete]}}
   // unix-silenced-warning@-1{{Argument to 'delete' is a constant 
address (16), which is not memory allocated by 'new' [cplusplus.NewDelete]}}
 }
+
+// deadstore-silenced-no-diagnostics
+
+int foo() {
+  int x = 42;
+  return x;
+}
+
+void g() {
+  int y;
+  y = 7;
+  int x = foo();
+  y = 10;
+}



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

[PATCH] D102914: [analyzer] Make checker silencing work for non-pathsensitive bug reports

[Kristóf Umann 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 rG9cca5c1391d6: [analyzer] Make checker silencing work for 
non-pathsensitive bug reports (authored by Szelethus).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102914

Files:
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/test/Analysis/silence-checkers-malloc.cpp
  clang/test/Analysis/silence-checkers.cpp


Index: clang/test/Analysis/silence-checkers.cpp
===
--- clang/test/Analysis/silence-checkers.cpp
+++ clang/test/Analysis/silence-checkers.cpp
@@ -11,6 +11,12 @@
 // RUN:   -analyzer-checker=cplusplus.NewDelete\
 // RUN:   -analyzer-config silence-checkers="unix"
 
+// RUN: %clang_analyze_cc1 -verify="deadstore-silenced" %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling \
+// RUN:   -analyzer-checker=deadcode \
+// RUN:   -analyzer-config silence-checkers="deadcode.DeadStores"
+
 #include "Inputs/system-header-simulator-cxx.h"
 
 typedef __typeof(sizeof(int)) size_t;
@@ -38,3 +44,17 @@
   delete Ptr; // no-silence-warning{{Argument to 'delete' is a constant 
address (16), which is not memory allocated by 'new' [cplusplus.NewDelete]}}
   // unix-silenced-warning@-1{{Argument to 'delete' is a constant 
address (16), which is not memory allocated by 'new' [cplusplus.NewDelete]}}
 }
+
+// deadstore-silenced-no-diagnostics
+
+int foo() {
+  int x = 42;
+  return x;
+}
+
+void g() {
+  int y;
+  y = 7;
+  int x = foo();
+  y = 10;
+}
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -1989,13 +1989,6 @@
   const SourceManager &SM = getSourceManager();
   const AnalyzerOptions &Opts = getAnalyzerOptions();
 
-  // See whether we need to silence the checker/package.
-  // FIXME: This will not work if the report was emitted with an incorrect tag.
-  for (const std::string &CheckerOrPackage : Opts.SilencedCheckersAndPackages) 
{
-if (R->getBugType().getCheckerName().startswith(CheckerOrPackage))
-  return nullptr;
-  }
-
   if (!PDC->shouldGenerateDiagnostics())
 return generateEmptyDiagnosticForReport(R, getSourceManager());
 
@@ -3040,6 +3033,14 @@
   if (!report)
 return;
 
+  // See whether we need to silence the checker/package.
+  for (const std::string &CheckerOrPackage :
+   getAnalyzerOptions().SilencedCheckersAndPackages) {
+if (report->getBugType().getCheckerName().startswith(
+CheckerOrPackage))
+  return;
+  }
+
   ArrayRef Consumers = getPathDiagnosticConsumers();
   std::unique_ptr Diagnostics =
   generateDiagnosticForConsumerMap(report, Consumers, bugReports);


Index: clang/test/Analysis/silence-checkers.cpp
===
--- clang/test/Analysis/silence-checkers.cpp
+++ clang/test/Analysis/silence-checkers.cpp
@@ -11,6 +11,12 @@
 // RUN:   -analyzer-checker=cplusplus.NewDelete\
 // RUN:   -analyzer-config silence-checkers="unix"
 
+// RUN: %clang_analyze_cc1 -verify="deadstore-silenced" %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling \
+// RUN:   -analyzer-checker=deadcode \
+// RUN:   -analyzer-config silence-checkers="deadcode.DeadStores"
+
 #include "Inputs/system-header-simulator-cxx.h"
 
 typedef __typeof(sizeof(int)) size_t;
@@ -38,3 +44,17 @@
   delete Ptr; // no-silence-warning{{Argument to 'delete' is a constant address (16), which is not memory allocated by 'new' [cplusplus.NewDelete]}}
   // unix-silenced-warning@-1{{Argument to 'delete' is a constant address (16), which is not memory allocated by 'new' [cplusplus.NewDelete]}}
 }
+
+// deadstore-silenced-no-diagnostics
+
+int foo() {
+  int x = 42;
+  return x;
+}
+
+void g() {
+  int y;
+  y = 7;
+  int x = foo();
+  y = 10;
+}
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -1989,13 +1989,6 @@
   const SourceManager &SM = getSourceManager();
   const AnalyzerOptions &Opts = getAnalyzerOptions();
 
-  // See whether we need to silence the checker/package.
-  // FIXME: This will not work if the report was emitted with an incorrect tag.
-  for (const std::string &CheckerOrPackage : Opts.SilencedCheckersAndPackages) {
-if (R->getBugType().getCheckerName().startswith(CheckerOrPackage))
-  return nullptr;
-  }
-
   if (!PDC->shouldGenerateDiagnostics())
 return generateEmptyDiagnosticForReport(R, getSourceManager());
 
@@ -3040,6 +3033,14 @@
   if (!report)
 return;
 
+  // See whether we need t

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

For the record, the one other breakage of a LibreOffice build with this is the 
handful of places that needed 
https://git.libreoffice.org/core/+/433ab39b2175bdadb4916373cd2dc8e1aabc08a5%5E%21
 "Adapt implicit OString return value construction to C++23 P2266R1":  In a 
nutshell, LO has an `OString` class with (non-explicit) constructors from any 
kind of `char` pointer or array, where the "array of `N` `const char`" variant 
(no. 3 below) is special, as it determines the string length as `N - 1` 
(whereas all the other variants search for the terminating NUL to determine the 
length).  That requires another constructor overload (no. 2 below) for 
non-`const` arrays, taking a non-`const` lvalue reference, which now causes 
issues when that constructor shall implicitly be used in `return` statements:

  $ cat test.cc
  #include 
  
  template struct CharPtrDetector {};
  template<> struct CharPtrDetector
  { using type = int; };
  template<> struct CharPtrDetector
  { using type = int; };
  
  template struct NonConstCharArrayDetector {};
  template struct NonConstCharArrayDetector
  { using type = int; };
  
  template struct ConstCharArrayDetector {};
  template struct ConstCharArrayDetector
  { using type = int; };
  
  struct OString {
  template
  OString(T const &, typename CharPtrDetector::type = 0); // #1
  template
  OString(T &, typename NonConstCharArrayDetector::type = 0); // #2
  template
  OString(T &, typename ConstCharArrayDetector::type = 0); // #3
  };
  
  OString good1a() {
  char * p;
  //...
  return p; // use #1
  };
  
  OString good1b() {
  char const * p;
  //...
  return p; // use #1
  };
  
  OString bad2() {
  char buf[10];
  //...
  return buf; // use #2
  }
  
  OString good3() {
  return "foo"; // use #3
  }
  
  $ clang++ -std=c++20 -fsyntax-only test.cc
  
  $ clang++ -std=c++2b -fsyntax-only test.cc
  test.cc:41:12: error: no viable conversion from returned value of type 'char 
[10]' to function return type 'OString'
  return buf; // use #2
 ^~~
  test.cc:17:8: note: candidate constructor (the implicit copy constructor) not 
viable: no known conversion from 'char [10]' to 'const OString &' for 1st 
argument
  struct OString {
 ^
  test.cc:17:8: note: candidate constructor (the implicit move constructor) not 
viable: no known conversion from 'char [10]' to 'OString &&' for 1st argument
  struct OString {
 ^
  test.cc:21:5: note: candidate constructor [with T = char [10]] not viable: 
expects an lvalue for 1st argument
  OString(T &, typename NonConstCharArrayDetector::type = 0); // #2
  ^
  test.cc:19:5: note: candidate template ignored: substitution failure [with T 
= char [10]]: no type named 'type' in 'CharPtrDetector'
  OString(T const &, typename CharPtrDetector::type = 0); // #1
  ^   
  test.cc:23:5: note: candidate template ignored: substitution failure [with T 
= char [10]]: no type named 'type' in 'ConstCharArrayDetector'
  OString(T &, typename ConstCharArrayDetector::type = 0); // #3
  ^
  1 error generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99005

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

[clang] 873308f - [Format] Fix incorrect pointer/reference detection

[via cfe-commits]

Author: Yilong Guo
Date: 2021-06-17T09:34:06+01:00
New Revision: 873308fd8c96a54f0024251425daac1b78f70119

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

LOG: [Format] Fix incorrect pointer/reference detection

https://llvm.org/PR50568

When an overloaded operator is called, its argument must be an
expression.

Before:
void f() { a.operator()(a *a); }

After:
void f() { a.operator()(a * a); }

Reviewed By: HazardyKnusperkeks, curdeius, MyDeveloperDay

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index a3d863745297..48309af24aa8 100755
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -229,7 +229,19 @@ class AnnotatingParser {
 }
 
 if (Left->is(TT_OverloadedOperatorLParen)) {
-  Contexts.back().IsExpression = false;
+  // Find the previous kw_operator token.
+  FormatToken *Prev = Left;
+  while (!Prev->is(tok::kw_operator)) {
+Prev = Prev->Previous;
+assert(Prev && "Expect a kw_operator prior to the OperatorLParen!");
+  }
+
+  // If faced with "a.operator*(argument)" or "a->operator*(argument)",
+  // i.e. the operator is called as a member function,
+  // then the argument must be an expression.
+  bool OperatorCalledAsMemberFunction =
+  Prev->Previous && Prev->Previous->isOneOf(tok::period, tok::arrow);
+  Contexts.back().IsExpression = OperatorCalledAsMemberFunction;
 } else if (Style.Language == FormatStyle::LK_JavaScript &&
(Line.startsWith(Keywords.kw_type, tok::identifier) ||
 Line.startsWith(tok::kw_export, Keywords.kw_type,

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 3df5d23a930f..6a2c0d007589 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -8274,6 +8274,16 @@ TEST_F(FormatTest, UnderstandsOverloadedOperators) {
   verifyFormat("using A::operator+;");
   verifyFormat("inline A operator^(const A &lhs, const A &rhs) {}\n"
"int i;");
+
+  // Calling an operator as a member function.
+  verifyFormat("void f() { a.operator*(); }");
+  verifyFormat("void f() { a.operator*(b & b); }");
+  verifyFormat("void f() { a->operator&(a * b); }");
+  verifyFormat("void f() { NS::a.operator+(*b * *b); }");
+  // TODO: Calling an operator as a non-member function is hard to distinguish.
+  // https://llvm.org/PR50629
+  // verifyFormat("void f() { operator*(a & a); }");
+  // verifyFormat("void f() { operator&(a, b * b); }");
 }
 
 TEST_F(FormatTest, UnderstandsFunctionRefQualification) {
@@ -8766,6 +8776,13 @@ TEST_F(FormatTest, UnderstandsUsesOfStarAndAmp) {
"operator()() && {}");
   verifyGoogleFormat("template \n"
  "auto x() & -> int {}");
+
+  // Should be binary operators when used as an argument expression (overloaded
+  // operator invoked as a member function).
+  verifyFormat("void f() { a.operator()(a * a); }");
+  verifyFormat("void f() { a->operator()(a & a); }");
+  verifyFormat("void f() { a.operator()(*a & *a); }");
+  verifyFormat("void f() { a->operator()(*a * *a); }");
 }
 
 TEST_F(FormatTest, UnderstandsAttributes) {



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

[PATCH] D103678: [Format] Fix incorrect pointer/reference detection

[MyDeveloperDay via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG873308fd8c96: [Format] Fix incorrect pointer/reference 
detection (authored by Nuu, committed by MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103678

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8274,6 +8274,16 @@
   verifyFormat("using A::operator+;");
   verifyFormat("inline A operator^(const A &lhs, const A &rhs) {}\n"
"int i;");
+
+  // Calling an operator as a member function.
+  verifyFormat("void f() { a.operator*(); }");
+  verifyFormat("void f() { a.operator*(b & b); }");
+  verifyFormat("void f() { a->operator&(a * b); }");
+  verifyFormat("void f() { NS::a.operator+(*b * *b); }");
+  // TODO: Calling an operator as a non-member function is hard to distinguish.
+  // https://llvm.org/PR50629
+  // verifyFormat("void f() { operator*(a & a); }");
+  // verifyFormat("void f() { operator&(a, b * b); }");
 }
 
 TEST_F(FormatTest, UnderstandsFunctionRefQualification) {
@@ -8766,6 +8776,13 @@
"operator()() && {}");
   verifyGoogleFormat("template \n"
  "auto x() & -> int {}");
+
+  // Should be binary operators when used as an argument expression (overloaded
+  // operator invoked as a member function).
+  verifyFormat("void f() { a.operator()(a * a); }");
+  verifyFormat("void f() { a->operator()(a & a); }");
+  verifyFormat("void f() { a.operator()(*a & *a); }");
+  verifyFormat("void f() { a->operator()(*a * *a); }");
 }
 
 TEST_F(FormatTest, UnderstandsAttributes) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -229,7 +229,19 @@
 }
 
 if (Left->is(TT_OverloadedOperatorLParen)) {
-  Contexts.back().IsExpression = false;
+  // Find the previous kw_operator token.
+  FormatToken *Prev = Left;
+  while (!Prev->is(tok::kw_operator)) {
+Prev = Prev->Previous;
+assert(Prev && "Expect a kw_operator prior to the OperatorLParen!");
+  }
+
+  // If faced with "a.operator*(argument)" or "a->operator*(argument)",
+  // i.e. the operator is called as a member function,
+  // then the argument must be an expression.
+  bool OperatorCalledAsMemberFunction =
+  Prev->Previous && Prev->Previous->isOneOf(tok::period, tok::arrow);
+  Contexts.back().IsExpression = OperatorCalledAsMemberFunction;
 } else if (Style.Language == FormatStyle::LK_JavaScript &&
(Line.startsWith(Keywords.kw_type, tok::identifier) ||
 Line.startsWith(tok::kw_export, Keywords.kw_type,


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8274,6 +8274,16 @@
   verifyFormat("using A::operator+;");
   verifyFormat("inline A operator^(const A &lhs, const A &rhs) {}\n"
"int i;");
+
+  // Calling an operator as a member function.
+  verifyFormat("void f() { a.operator*(); }");
+  verifyFormat("void f() { a.operator*(b & b); }");
+  verifyFormat("void f() { a->operator&(a * b); }");
+  verifyFormat("void f() { NS::a.operator+(*b * *b); }");
+  // TODO: Calling an operator as a non-member function is hard to distinguish.
+  // https://llvm.org/PR50629
+  // verifyFormat("void f() { operator*(a & a); }");
+  // verifyFormat("void f() { operator&(a, b * b); }");
 }
 
 TEST_F(FormatTest, UnderstandsFunctionRefQualification) {
@@ -8766,6 +8776,13 @@
"operator()() && {}");
   verifyGoogleFormat("template \n"
  "auto x() & -> int {}");
+
+  // Should be binary operators when used as an argument expression (overloaded
+  // operator invoked as a member function).
+  verifyFormat("void f() { a.operator()(a * a); }");
+  verifyFormat("void f() { a->operator()(a & a); }");
+  verifyFormat("void f() { a.operator()(*a & *a); }");
+  verifyFormat("void f() { a->operator()(*a * *a); }");
 }
 
 TEST_F(FormatTest, UnderstandsAttributes) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -229,7 +229,19 @@
 }
 
 if (Left->is(TT_OverloadedOperatorLParen)) {
-  Contexts.back().IsExpression = false;
+  // Find the previous kw_operator token.
+  FormatToken *Prev = Left;
+  while (!Prev->is(tok::kw_operator)) {
+Prev = Prev->Previous;
+assert(Prev && "Expect

[PATCH] D104300: [analyzer] Handle std::swap for std::unique_ptr

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

I agree with @NoQ that notes are pretty much straightforward and we shouldn't 
abandon them altogether.  Refinements about what is null or non-null pointer 
are purely cosmetic and we definitely can tweak this messaging.




Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:196-202
+const MemRegion *FirstArgThisRegion = Call.getArgSVal(0).getAsRegion();
+if (!FirstArgThisRegion)
+  return false;
+const MemRegion *SecondArgThisRegion = Call.getArgSVal(1).getAsRegion();
+if (!SecondArgThisRegion)
+  return false;
+

I guess `handleSwap` can take `SVal`s instead of `MemRegion` and we can mostly 
cut on this boilerplate as well.
```
   return handleSwap(State, Call.getArgSVal(0), Call.getArgSVal(1), C);
```
and
```
  return handleSwap(State, IC->getCXXThisVal(), Call.getArgSVal(0), C);
```



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:458-467
+if (BR.isInteresting(FirstThisRegion) &&
+!BR.isInteresting(SecondThisRegion)) {
+  BR.markInteresting(SecondThisRegion);
+  BR.markNotInteresting(FirstThisRegion);
+}
+if (BR.isInteresting(SecondThisRegion) &&
+!BR.isInteresting(FirstThisRegion)) {

nit: these two pieces of code are very much the same


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104300

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

[PATCH] D104096: [Clang-Format] Add ReferenceAlignment directive

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

This patch needs rebasing, please also make comments as "Done" once you have 
addressed them.




Comment at: clang/lib/Format/TokenAnnotator.cpp:4162
+return FormatStyle::PAS_Middle;
+  }
+}

you are missing a return value here

```
C:\cygwin64\buildareas\clang\llvm-project\clang\lib\Format\TokenAnnotator.cpp(4264)
 : warning C4715: 'clang::format::TokenAnnotator::getTokenReferenceAlignment': 
not all control paths return a value
```




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104096

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

[PATCH] D104198: [Matrix] Add documentation for compound assignment and type conversion of matrix types

[Florian Hahn via Phabricator via cfe-commits]

fhahn added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:535
+  int d = 12;
+  a = a + c;
+  return a * d;

Is there any reason we use variables for the scalars? If not, it might be good 
to keep the examples as compact as possible. Using the extra variables seems 
overly verbose, when it could be just `return (a + 23) * 12`  and may give the 
impression only scalar variables are supported.

same also applies to the other examples below.



Comment at: clang/docs/LanguageExtensions.rst:578
+f = (fx5x5)i;
+return f;
+  }

this could also just be `return (fx5x5) i;`, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104198

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

[clang] 05e95d2 - [clang][AST] Set correct DeclContext in ASTImporter lookup table for template params.

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

Author: Balázs Kéri
Date: 2021-06-17T11:20:27+02:00
New Revision: 05e95d2dd74973dd5163b7d44828fac61e416452

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

LOG: [clang][AST] Set correct DeclContext in ASTImporter lookup table for 
template params.

Template parameters are created in ASTImporter with the translation unit as 
DeclContext.
The DeclContext is later updated (by the create function of template classes).
ASTImporterLookupTable was not updated after these changes of the DC. The patch
adds update of the DeclContext in ASTImporterLookupTable.

Reviewed By: martong

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

Added: 


Modified: 
clang/lib/AST/ASTImporter.cpp
clang/unittests/AST/ASTImporterTest.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 0fd1401e9e9f..0d0cabc96556 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -322,6 +322,21 @@ namespace clang {
   }
 }
 
+void updateLookupTableForTemplateParameters(TemplateParameterList &Params,
+DeclContext *OldDC) {
+  ASTImporterLookupTable *LT = Importer.SharedState->getLookupTable();
+  if (!LT)
+return;
+
+  for (NamedDecl *TP : Params)
+LT->update(TP, OldDC);
+}
+
+void updateLookupTableForTemplateParameters(TemplateParameterList &Params) 
{
+  updateLookupTableForTemplateParameters(
+  Params, Importer.getToContext().getTranslationUnitDecl());
+}
+
   public:
 explicit ASTNodeImporter(ASTImporter &Importer) : Importer(Importer) {}
 
@@ -2609,6 +2624,8 @@ 
ASTNodeImporter::VisitTypeAliasTemplateDecl(TypeAliasTemplateDecl *D) {
   ToAlias->setAccess(D->getAccess());
   ToAlias->setLexicalDeclContext(LexicalDC);
   LexicalDC->addDeclInternal(ToAlias);
+  if (DC != Importer.getToContext().getTranslationUnitDecl())
+updateLookupTableForTemplateParameters(*ToTemplateParameters);
   return ToAlias;
 }
 
@@ -5511,6 +5528,7 @@ ExpectedDecl 
ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
   D2->setLexicalDeclContext(LexicalDC);
 
   addDeclToContexts(D, D2);
+  updateLookupTableForTemplateParameters(**TemplateParamsOrErr);
 
   if (FoundByLookup) {
 auto *Recent =
@@ -5654,6 +5672,7 @@ ExpectedDecl 
ASTNodeImporter::VisitClassTemplateSpecializationDecl(
   // Add this partial specialization to the class template.
   ClassTemplate->AddPartialSpecialization(PartSpec2, InsertPos);
 
+updateLookupTableForTemplateParameters(*ToTPList);
   } else { // Not a partial specialization.
 if (GetImportedOrCreateDecl(
 D2, D, Importer.getToContext(), D->getTagKind(), DC,
@@ -5803,6 +5822,8 @@ ExpectedDecl 
ASTNodeImporter::VisitVarTemplateDecl(VarTemplateDecl *D) {
   ToVarTD->setAccess(D->getAccess());
   ToVarTD->setLexicalDeclContext(LexicalDC);
   LexicalDC->addDeclInternal(ToVarTD);
+  if (DC != Importer.getToContext().getTranslationUnitDecl())
+updateLookupTableForTemplateParameters(**TemplateParamsOrErr);
 
   if (FoundByLookup) {
 auto *Recent =
@@ -5928,6 +5949,9 @@ ExpectedDecl 
ASTNodeImporter::VisitVarTemplateSpecializationDecl(
 
   D2 = ToPartial;
 
+  // FIXME: Use this update if VarTemplatePartialSpecializationDecl is 
fixed
+  // to adopt template parameters.
+  // updateLookupTableForTemplateParameters(**ToTPListOrErr);
 } else { // Full specialization
   if (GetImportedOrCreateDecl(D2, D, Importer.getToContext(), DC,
   *BeginLocOrErr, *IdLocOrErr, VarTemplate,
@@ -6016,14 +6040,30 @@ 
ASTNodeImporter::VisitFunctionTemplateDecl(FunctionTemplateDecl *D) {
   auto ParamsOrErr = import(D->getTemplateParameters());
   if (!ParamsOrErr)
 return ParamsOrErr.takeError();
+  TemplateParameterList *Params = *ParamsOrErr;
 
   FunctionDecl *TemplatedFD;
   if (Error Err = importInto(TemplatedFD, D->getTemplatedDecl()))
 return std::move(Err);
 
+  // Template parameters of the ClassTemplateDecl and FunctionTemplateDecl are
+  // shared, if the FunctionTemplateDecl is a deduction guide for the class.
+  // At import the ClassTemplateDecl object is always created first (FIXME: is
+  // this really true?) because the dependency, then the FunctionTemplateDecl.
+  // The DeclContext of the template parameters is changed when the
+  // FunctionTemplateDecl is created, but was set already when the class
+  // template was created. So here it is not the TU (default value) any more.
+  // FIXME: The DeclContext of the parameters is now set finally to the
+  // CXXDeductionGuideDecl object that was imported later. This may not be the
+  // same that is in the original AST, specially if there are multiple 
dedu

[PATCH] D103792: [clang][AST] Set correct DeclContext in ASTImporter lookup table for template params.

[Balázs Kéri 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 rG05e95d2dd749: [clang][AST] Set correct DeclContext in 
ASTImporter lookup table for template… (authored by balazske).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103792

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4382,6 +4382,245 @@
   EXPECT_EQ(ToD->getNumTemplateParameterLists(), 1u);
 }
 
+const internal::VariadicDynCastAllOfMatcher
+varTemplateDecl;
+
+const internal::VariadicDynCastAllOfMatcher<
+Decl, VarTemplatePartialSpecializationDecl>
+varTemplatePartialSpecializationDecl;
+
+TEST_P(ASTImporterOptionSpecificTestBase,
+   FunctionTemplateParameterDeclContext) {
+  constexpr auto Code =
+  R"(
+  template
+  void f() {};
+  )";
+
+  Decl *FromTU = getTuDecl(Code, Lang_CXX11);
+
+  auto *FromD = FirstDeclMatcher().match(
+  FromTU, functionTemplateDecl(hasName("f")));
+
+  ASSERT_EQ(FromD->getTemplateParameters()->getParam(0)->getDeclContext(),
+FromD->getTemplatedDecl());
+
+  auto *ToD = Import(FromD, Lang_CXX11);
+  EXPECT_EQ(ToD->getTemplateParameters()->getParam(0)->getDeclContext(),
+ToD->getTemplatedDecl());
+  EXPECT_TRUE(SharedStatePtr->getLookupTable()->contains(
+  ToD->getTemplatedDecl(), ToD->getTemplateParameters()->getParam(0)));
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, ClassTemplateParameterDeclContext) {
+  constexpr auto Code =
+  R"(
+  template
+  struct S {};
+  template
+  struct S {};
+  )";
+
+  Decl *FromTU = getTuDecl(Code, Lang_CXX11);
+
+  auto *FromD = FirstDeclMatcher().match(
+  FromTU, classTemplateDecl(hasName("S")));
+  auto *FromDPart =
+  FirstDeclMatcher().match(
+  FromTU, classTemplatePartialSpecializationDecl(hasName("S")));
+
+  ASSERT_EQ(FromD->getTemplateParameters()->getParam(0)->getDeclContext(),
+FromD->getTemplatedDecl());
+  ASSERT_EQ(FromDPart->getTemplateParameters()->getParam(0)->getDeclContext(),
+FromDPart);
+
+  auto *ToD = Import(FromD, Lang_CXX11);
+  auto *ToDPart = Import(FromDPart, Lang_CXX11);
+
+  EXPECT_EQ(ToD->getTemplateParameters()->getParam(0)->getDeclContext(),
+ToD->getTemplatedDecl());
+  EXPECT_TRUE(SharedStatePtr->getLookupTable()->contains(
+  ToD->getTemplatedDecl(), ToD->getTemplateParameters()->getParam(0)));
+
+  EXPECT_EQ(ToDPart->getTemplateParameters()->getParam(0)->getDeclContext(),
+ToDPart);
+  EXPECT_TRUE(SharedStatePtr->getLookupTable()->contains(
+  ToDPart, ToDPart->getTemplateParameters()->getParam(0)));
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase,
+   CXXDeductionGuideTemplateParameterDeclContext) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  template  struct A {
+A(T);
+  };
+  A a{(int)0};
+  )",
+  Lang_CXX17, "input.cc");
+// clang-format off
+/*
+|-ClassTemplateDecl 0x1fe5000  line:2:36 A
+| |-TemplateTypeParmDecl 0x1fe4eb0  col:26 referenced typename depth 0 index 0 T
+| |-CXXRecordDecl 0x1fe4f70  line:2:36 struct A definition
+
+|-FunctionTemplateDecl 0x1fe5860  col:9 implicit 
+| |-TemplateTypeParmDecl 0x1fe4eb0  col:26 referenced typename depth 0 index 0 T
+| |-CXXDeductionGuideDecl 0x1fe57a8  col:9 implicit  'auto (T) -> A'
+| | `-ParmVarDecl 0x1fe56b0  col:12 'T'
+| `-CXXDeductionGuideDecl 0x20515d8  col:9 implicit used  'auto (int) -> A'
+|   |-TemplateArgument type 'int'
+|   | `-BuiltinType 0x20587e0 'int'
+|   `-ParmVarDecl 0x2051388  col:12 'int':'int'
+`-FunctionTemplateDecl 0x1fe5a78  col:36 implicit 
+  |-TemplateTypeParmDecl 0x1fe4eb0  col:26 referenced typename depth 0 index 0 T
+  `-CXXDeductionGuideDecl 0x1fe59c0  col:36 implicit  'auto (A) -> A'
+`-ParmVarDecl 0x1fe5958  col:36 'A'
+*/
+// clang-format on
+  auto *FromD1 = FirstDeclMatcher().match(
+  FromTU, cxxDeductionGuideDecl());
+  auto *FromD2 = LastDeclMatcher().match(
+  FromTU, cxxDeductionGuideDecl());
+
+  NamedDecl *P1 =
+  FromD1->getDescribedFunctionTemplate()->getTemplateParameters()->getParam(
+  0);
+  NamedDecl *P2 =
+  FromD2->getDescribedFunctionTemplate()->getTemplateParameters()->getParam(
+  0);
+  DeclContext *DC = P1->getDeclContext();
+
+  ASSERT_EQ(P1, P2);
+  ASSERT_TRUE(DC == FromD1 || DC == FromD2);
+
+  auto *ToD1 = Import(FromD1, Lang_CXX17);
+  auto *ToD2 = Import(FromD2, Lang_CXX17);
+  ASSERT_TRUE(ToD1 && ToD2);
+
+  P1 = ToD1->getDescribedFunctionTemplate()->getTemplateParameters()->getParam(
+  0);
+  P2 = ToD2->getDescribedFunctionTemplate()->getTemplateParameters()->getParam(
+  0);
+ 

[PATCH] D104136: [analyzer] Add better tracking for RetainCountChecker leak warnings

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko updated this revision to Diff 352652.
vsavchenko added a comment.

Move IdentityHandler into the anonymous namespace


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104136

Files:
  clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
  
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
  clang/test/Analysis/Inputs/expected-plists/retain-release.m.objc.plist
  clang/test/Analysis/Inputs/expected-plists/retain-release.m.objcpp.plist
  clang/test/Analysis/osobject-retain-release.cpp

Index: clang/test/Analysis/osobject-retain-release.cpp
===
--- clang/test/Analysis/osobject-retain-release.cpp
+++ clang/test/Analysis/osobject-retain-release.cpp
@@ -536,6 +536,7 @@
 
 void check_dynamic_cast_alias() {
   OSObject *originalPtr = OSObject::generateObject(1);   // expected-note {{Call to method 'OSObject::generateObject' returns an OSObject}}
+ // expected-note@-1 {{'originalPtr' initialized here}}
   OSArray *newPtr = OSDynamicCast(OSArray, originalPtr); // expected-note {{'newPtr' initialized to the value of 'originalPtr'}}
   if (newPtr) {  // expected-note {{'newPtr' is non-null}}
  // expected-note@-1 {{Taking true branch}}
@@ -548,6 +549,7 @@
 
 void check_dynamic_cast_alias_cond() {
   OSObject *originalPtr = OSObject::generateObject(1); // expected-note {{Call to method 'OSObject::generateObject' returns an OSObject}}
+   // expected-note@-1 {{'originalPtr' initialized here}}
   OSArray *newPtr = 0;
   if ((newPtr = OSDynamicCast(OSArray, originalPtr))) { // expected-note {{The value of 'originalPtr' is assigned to 'newPtr'}}
 // expected-note@-1 {{'newPtr' is non-null}}
@@ -561,7 +563,8 @@
 
 void check_dynamic_cast_alias_intermediate() {
   OSObject *originalPtr = OSObject::generateObject(1); // expected-note {{Call to method 'OSObject::generateObject' returns an OSObject of type 'OSObject' with a +1 retain count}}
-  OSObject *intermediate = originalPtr;// TODO: add note here as well
+   // expected-note@-1 {{'originalPtr' initialized here}}
+  OSObject *intermediate = originalPtr;// expected-note {{'intermediate' initialized to the value of 'originalPtr'}}
   OSArray *newPtr = 0;
   if ((newPtr = OSDynamicCast(OSArray, intermediate))) { // expected-note {{The value of 'intermediate' is assigned to 'newPtr'}}
  // expected-note@-1 {{'newPtr' is non-null}}
@@ -575,7 +578,8 @@
 
 void check_dynamic_cast_alias_intermediate_2() {
   OSObject *originalPtr = OSObject::generateObject(1); // expected-note {{Call to method 'OSObject::generateObject' returns an OSObject of type 'OSObject' with a +1 retain count}}
-  OSObject *intermediate = originalPtr;// TODO: add note here as well
+   // expected-note@-1 {{'originalPtr' initialized here}}
+  OSObject *intermediate = originalPtr;// expected-note {{'intermediate' initialized to the value of 'originalPtr'}}
   OSArray *newPtr = 0;
   if ((newPtr = OSDynamicCast(OSArray, intermediate))) { // expected-note {{Value assigned to 'newPtr'}}
  // expected-note@-1 {{'newPtr' is non-null}}
Index: clang/test/Analysis/Inputs/expected-plists/retain-release.m.objcpp.plist
===
--- clang/test/Analysis/Inputs/expected-plists/retain-release.m.objcpp.plist
+++ clang/test/Analysis/Inputs/expected-plists/retain-release.m.objcpp.plist
@@ -25282,6 +25282,35 @@
  message
  Call to function 'CFCreateSomething' returns a Core Foundation object of type 'CFTypeRef' with a +1 retain count
 
+
+ kindevent
+ location
+ 
+  line2285
+  col3
+  file0
+ 
+ ranges
+ 
+   
+
+ line2285
+ col3
+ file0
+
+
+ line2285
+ col15
+ file0
+
+   
+ 
+ depth0
+ extended_message
+ 'obj' initialized here
+ message
+ 'obj' initialized here
+
 
  kindcontrol
  edges
@@ -25569,6 +25598,35 @@
  message
  Call to function 'CFArrayCreateMutable' returns a Core Foundation object of type 'CFMutableArrayRef' with a +1 retain count
 
+
+ kindevent
+ location
+ 
+  line2305
+  col3
+  file0
+ 
+ ranges
+ 
+   
+

[PATCH] D98710: [clang-tidy] Add --skip-headers, part 1 (use setTraversalScope)

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

I think the layering can be improved here.

The


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

https://reviews.llvm.org/D98710

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

[PATCH] D104136: [analyzer] Add better tracking for RetainCountChecker leak warnings

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp:945-947
+ExprEngine &Eng = C.getStateManager().getOwningEngine();
+// Let's mark this place with a special tag.
+Tag = Eng.getDataTags().make(CE, BindReturnTo);

vsavchenko wrote:
> Szelethus wrote:
> > I don't know ObjC at all, but is this identity obvious? Do you not need a 
> > `NoteTag` to say that "Foo is an identity function, its return value equals 
> > the parameter"? Or, in the possession of the actual 
> > `PathSensitiveBugReport` that you can retrieve in the handler, you could 
> > drop a note there, maybe?
> These are usually some type of casts.  And we already provide notes when this 
> value gets into some other region (like `'newPtr' initialized to the value of 
> 'originalPtr'`).  I think that they are enough.  And if we want something 
> better, I think we need to tweak that messaging (`StoreHandler` now allows 
> that) instead of crowding user with even more notes.
Fair enough!



Comment at: 
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp:953-957
+  // Let's traverse...
+  for (const ExplodedNode *N = ExprNode;
+   // ...all the nodes corresponding to the given expression...
+   N != nullptr && N->getStmtForDiagnostics() == E;
+   N = N->getFirstPred()) {

vsavchenko wrote:
> NoQ wrote:
> > I guess this part should ultimately be written in one place, eg. 
> > `ExplodedNode::findTag()` or something like that.
> > 
> > I'd also really like to explore the possibility to further limit the 
> > variety of nodes traversed here. What nodes are typically traversed here? 
> > Is it checker-tagged nodes or is it purge dead symbol nodes or something 
> > else?
> Yes, `ExplodedNode::findTag()` sounds like a great idea!
> 
> I mean it is hard to tell without calculating statistics right here and 
> running it on a bunch of projects.  However, it is always possible to write 
> the code that will have it the other way.  My take on it is that it is 
> probably a mix of things.
> 
> I'd also prefer to traverse less, do you have any specific ideas here?
> I'd also really like to explore the possibility to further limit the variety 
> of nodes traversed here. What nodes are typically traversed here? Is it 
> checker-tagged nodes or is it purge dead symbol nodes or something else?

Is there something I'm not seeing here? Trackers basically ascend //a// path 
from the error node to at most the root of the ExplodedGraph (not the trimmed 
version, as `Tracker::track()` is called before any of that process happens), 
so its not much slower than `trackExpressionValue`, right?

Or does this, and likely many other future handlers run such a loop more often 
then what I imagine?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104136

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

[PATCH] D104311: [clang] Fix a race condition in the build of clangInterpreter

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
This revision is now accepted and ready to land.

Thanks, @stella.stamenova!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104311

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

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

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

HazardyKnusperkeks added a comment.

In D102706#2823087 , @vlovich wrote:

> I don't think I have permissions. Happy to do it if I'm given permissions 
> (I'm assuming the instructions are the general LLVM ones). Otherwise:
>
> Name: Vitali Lovich
> E-mail: vlov...@gmail.com

You can apply for commit access, yes that's for whole of LLVM, but for me it 
was no problem to get that right.


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

https://reviews.llvm.org/D102706

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

[PATCH] D103855: [clang] Exclude function pointers on DefaultedComparisonAnalyzer

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 352667.
mizvekov added a comment.

Instead of excluding function pointers from the overload set, just implicitly 
delete
with new diagnostic in case we end up selecting it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103855

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/class/class.compare/class.spaceship/p2.cpp

Index: clang/test/CXX/class/class.compare/class.spaceship/p2.cpp
===
--- clang/test/CXX/class/class.compare/class.spaceship/p2.cpp
+++ clang/test/CXX/class/class.compare/class.spaceship/p2.cpp
@@ -159,7 +159,7 @@
 namespace PR48856 {
   struct A {
 auto operator<=>(const A &) const = default; // expected-warning {{implicitly deleted}}
-void (*x)(); // expected-note {{because there is no viable three-way comparison function for member 'x'}}
+void (*x)(); // expected-note {{does not support relational comparisons}}
   };
 
   struct B {
@@ -197,8 +197,7 @@
 operator fp() const;
   };
   struct b3 {
-auto operator<=>(b3 const &) const = default; // expected-error {{cannot be deduced because three-way comparison for member 'f' would compare as builtin type 'void (*)()' which is not currently supported}}
-// expected-warning@-1 {{implicitly deleted}}
-a3 f;
+auto operator<=>(b3 const &) const = default; // expected-warning {{implicitly deleted}}
+a3 f; // expected-note {{would compare member 'f' as 'void (*)()', which does not support relational comparisons}}
   };
 }
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7749,16 +7749,11 @@
 
 if (Args[0]->getType()->isOverloadableType())
   S.LookupOverloadedBinOp(CandidateSet, OO, Fns, Args);
-else if (OO == OO_EqualEqual ||
- !Args[0]->getType()->isFunctionPointerType()) {
+else
   // FIXME: We determine whether this is a valid expression by checking to
   // see if there's a viable builtin operator candidate for it. That isn't
   // really what the rules ask us to do, but should give the right results.
-  //
-  // Note that the builtin operator for relational comparisons on function
-  // pointers is the only known case which cannot be used.
   S.AddBuiltinOperatorCandidates(OO, FD->getLocation(), Args, CandidateSet);
-}
 
 Result R;
 
@@ -7802,11 +7797,14 @@
   return Result::deleted();
   }
 
-  // C++2a [class.compare.default]p3 [P2002R0]:
-  //   A defaulted comparison function is constexpr-compatible if [...]
-  //   no overlod resolution performed [...] results in a non-constexpr
-  //   function.
+  bool NeedsDeducing =
+  OO == OO_Spaceship && FD->getReturnType()->isUndeducedAutoType();
+
   if (FunctionDecl *BestFD = Best->Function) {
+// C++2a [class.compare.default]p3 [P2002R0]:
+//   A defaulted comparison function is constexpr-compatible if
+//   [...] no overlod resolution performed [...] results in a
+//   non-constexpr function.
 assert(!BestFD->isDeleted() && "wrong overload resolution result");
 // If it's not constexpr, explain why not.
 if (Diagnose == ExplainConstexpr && !BestFD->isConstexpr()) {
@@ -7819,10 +7817,8 @@
   return Result::deleted();
 }
 R.Constexpr &= BestFD->isConstexpr();
-  }
 
-  if (OO == OO_Spaceship && FD->getReturnType()->isUndeducedAutoType()) {
-if (auto *BestFD = Best->Function) {
+if (NeedsDeducing) {
   // If any callee has an undeduced return type, deduce it now.
   // FIXME: It's not clear how a failure here should be handled. For
   // now, we produce an eager diagnostic, because that is forward
@@ -7848,10 +7844,9 @@
 }
 return Result::deleted();
   }
-  if (auto *Info = S.Context.CompCategories.lookupInfoForType(
-  BestFD->getCallResultType())) {
-R.Category = Info->Kind;
-  } else {
+  auto *Info = S.Context.CompCategories.lookupInfoForType(
+  BestFD->getCallResultType());
+  if (!Info) {
 if (Diagnose == ExplainDeleted) {
   S.Diag(Subobj.Loc, diag::note_defaulted_comparison_cannot_deduce)
   << Subobj.Kind << Subobj.Decl
@@ -7862,12 +7857,25 @@
 }
 return Result::deleted();
   }
-} else {
-  QualType T = Best->BuiltinParamTypes[0];
-  assert(T == Best->BuiltinParamTypes[1] &&
- "builtin comparison for different types?");
-  assert(Best->BuiltinParamTypes[2].isNull() &&
- 

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

I think this iteration is much better, it requires way more description as it 
has now.  You didn't actually describe anywhere how this algorithm actually 
works.




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:200-201
+}
+
+// Build a new range.
+while (true) {

At this point, we know for a fact that the next range we are going to add to 
our result set starts with `I1->From()`.  No need to check `F` for null or 
anything



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:210
+
+  // Check if the second range intersects or adjucent to the first one.
+  if (I1->To() < I2->From() - One)

nit: "**is** adj**a**cent"



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:212
+  if (I1->To() < I2->From() - One)
+break;
+

Why `break`?  At this point we know that what we peaked into is actually a part 
of another range (in terms of the end result).
And this range is over, you just need to add it to the final result!



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:214-218
+  // We use `F` as a flag to notify that we are in a building of a new
+  // range. Set `From` of a new range if it is not set yet. If it has
+  // already been set, then we are inside this range and just looking for
+  // its end.
+  if (!F)

You actually don't need it, the moment you swap two iterators and find out that 
`I1->From() <= I2->From()`, that's it `I1->From()` is the beginning of the new 
range.  No need to carry extra flag here.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:227
+  // Now we can surely swap the iterators without any check, as we know
+  // I2-From() to be lower than I1-From().
+  std::swap(I1, I2);

 nit
Additionally, you didn't tell your readers WHY you are even swapping iterators 
in the first place and what relationship they have.  You need to communicate in 
terms of invariants.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:244
+if (++I1 == E1)
+  goto end1;
+  };

This goto is always finishing with a return, so we can refactor `goto end1` 
into something like `return copyIntoResult(I1, E1);` and `goto end2` into 
`return copyIntoResult(I2, E2);`.
As I mentioned before, optional `F` should be removed.  On every path we should 
know deterministically what is the beginning and what is the end of the range 
that we are trying to add.


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

https://reviews.llvm.org/D99797

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

[PATCH] D99439: Update @llvm.powi to handle different int sizes for the exponent

[Eli Friedman via Phabricator via cfe-commits]

efriedma accepted this revision.
efriedma 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/D99439/new/

https://reviews.llvm.org/D99439

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

[PATCH] D104291: [Debug-Info] strict dwarf for DW_LANG_C_plus_plus_14

[ChenZheng via Phabricator via cfe-commits]

shchenz added a comment.

In D104291#2821581 , @stuart wrote:

>> There is no `CPlusPlus03` in `LangOptions`, so it is better not to merge 
>> `DW_LANG_C_plus_plus_03` support with D99250 
>> .
>
> Oh - I see, `c++03` is defined in LangStandards.def an alias for `c++98`, as 
> the former essentially consists of bugfixes for the latter. This loosely 
> suggests to me that C++03 implementations are (likely to be / mostly?) 
> conformant to C++98, but that C++98 implementations may not be fully 
> conformant to C++03. Given this alias, it doesn't seem at all clear to me 
> which of DW_LANG_C_plus_plus_98 and DW_LANG_C_plus_plus_03 would be the 
> better choice, if both C++98 and C++03 must share a language tag... but I 
> presume this has been discussed before. (It also doesn't seem clear whether 
> it would be better to model "c++98" as an alias for "c++03".)
>
>> Yes, we don't have `DW_LANG_C_plus_plus_17` and `DW_LANG_C_plus_plus_20` in 
>> clang for now. I guess this is because clang does not support DWARF 6. DWARF 
>> 6 is not officially released? Once DWARF 6 is released and clang starts to 
>> support DWARF 6, I think we should add the support for 
>> `DW_LANG_C_plus_plus_17` and `DW_LANG_C_plus_plus_20` in the place that this 
>> patch changes.
>
> New DWARF language codes  are published 
> ahead of the release of the next version of DWARF, so that they may be used 
> by implementations without having to wait for new DWARF version.
>
> It would therefore make sense to go ahead and add `DW_LANG_C_plus_plus_17` 
> and `DW_LANG_C_plus_plus_20` now, without waiting, but only in the non-strict 
> DWARF mode. (There would be the question of whether it would still make sense 
> in the code to say "DwarfVersion >= 6" given that DWARF 6 would be otherwise 
> unsupported... but I don't have a strong view on that question.)

I think it would be strange to check `DwarfVersion >= 6` if we can not set 
dwarf version to 6 by anyways. Maybe we can first add the DWARF 6 support to 
the front end first. This is the patch where `-gdwarf-5` was introduced 
https://reviews.llvm.org/rG3cb592d6b60c. Seems it was also before DWARF 5 was 
official released.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104291

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

[PATCH] D104291: [Debug-Info] strict dwarf for DW_LANG_C_plus_plus_14

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D104291#2823594 , @shchenz wrote:

> In D104291#2821581 , @stuart wrote:
>
>>> There is no `CPlusPlus03` in `LangOptions`, so it is better not to merge 
>>> `DW_LANG_C_plus_plus_03` support with D99250 
>>> .
>>
>> Oh - I see, `c++03` is defined in LangStandards.def an alias for `c++98`, as 
>> the former essentially consists of bugfixes for the latter. This loosely 
>> suggests to me that C++03 implementations are (likely to be / mostly?) 
>> conformant to C++98, but that C++98 implementations may not be fully 
>> conformant to C++03. Given this alias, it doesn't seem at all clear to me 
>> which of DW_LANG_C_plus_plus_98 and DW_LANG_C_plus_plus_03 would be the 
>> better choice, if both C++98 and C++03 must share a language tag... but I 
>> presume this has been discussed before. (It also doesn't seem clear whether 
>> it would be better to model "c++98" as an alias for "c++03".)
>>
>>> Yes, we don't have `DW_LANG_C_plus_plus_17` and `DW_LANG_C_plus_plus_20` in 
>>> clang for now. I guess this is because clang does not support DWARF 6. 
>>> DWARF 6 is not officially released? Once DWARF 6 is released and clang 
>>> starts to support DWARF 6, I think we should add the support for 
>>> `DW_LANG_C_plus_plus_17` and `DW_LANG_C_plus_plus_20` in the place that 
>>> this patch changes.
>>
>> New DWARF language codes  are published 
>> ahead of the release of the next version of DWARF, so that they may be used 
>> by implementations without having to wait for new DWARF version.
>>
>> It would therefore make sense to go ahead and add `DW_LANG_C_plus_plus_17` 
>> and `DW_LANG_C_plus_plus_20` now, without waiting, but only in the 
>> non-strict DWARF mode. (There would be the question of whether it would 
>> still make sense in the code to say "DwarfVersion >= 6" given that DWARF 6 
>> would be otherwise unsupported... but I don't have a strong view on that 
>> question.)
>
> I think it would be strange to check `DwarfVersion >= 6` if we can not set 
> dwarf version to 6 by anyways. Maybe we can first add the DWARF 6 support to 
> the front end first(Need a wider discussion). This is the patch where 
> `-gdwarf-5` was introduced https://reviews.llvm.org/rG3cb592d6b60c. Seems it 
> was also before DWARF 5 was official released.

Yeah, sounds alright to me. Certainly if we add the version check, there should 
be test coverage for it - and that can only be done with the flag support for 
it. (& it's understandable/expected that the flag is not complete from day 1 - 
same thing is true of language versions (though they get speculative version 
numbering because the number isn't known until it's released (because it's 
based on the year) - but even then, once known/added, it still may not be 
complete because features may not have been implemented yet))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104291

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

[PATCH] D99439: Update @llvm.powi to handle different int sizes for the exponent

[Bjorn Pettersson via Phabricator via cfe-commits]

bjope added a comment.

In D99439#2823338 , @efriedma wrote:

> LGTM

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99439

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

[PATCH] D104040: [OpenCL] Add TableGen emitter for OpenCL builtin header

[Anton Zabaznov via Phabricator via cfe-commits]

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

Thanks for working on this! That's cool, I managed to reproduce something 
similar to https://reviews.llvm.org/D99577.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104040

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

[PATCH] D103094: [analyzer] Implemented RangeSet::Factory::castTo function to perform promotions, truncations and conversions.

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

OK, about the quadratic algorithm.  I can think of one solution that is 
actually linear asymptotically, but because of the constant factor can be worse 
in practice (we usually have extremely small range sets).  So, while I don't 
like it, it's probably fine.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:245-246
+///
+/// Complexity: O(N^2)
+/// where N = size(What)
+RangeSet castTo(RangeSet What, APSIntType Ty);

That is a bit of a red flag because instantly my intuition tells me that there 
should be a linear option.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:248
+RangeSet castTo(RangeSet What, APSIntType Ty);
+RangeSet castTo(RangeSet What, QualType T);
 

If we talk about types for ranges, the interface we have is incomplete.
We should introduce something like `getType` for range sets, so that the user 
can have a more holistic picture about types and ranges.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:692
+  ContainerType Result;
+  ContainerType RHS;
+

We have LHS (left-hand side) and RHS (right-hand side) as a common convention 
for naming operands of binary operators.
The fact that you later on use it as an RHS for `unite`, doesn't make it a good 
name.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:697
+  // But in fact we use `number of values` - 1, because
+  // we can't hold `UINT64 number of values` inside uint64_t.
+  // And it's OK, it's enough to do correct calculations.

That's incorrect. `uint64_t` holds **exactly** this many values.  If you count 
every number from `0` to `UINT64_MAX`, you get `UINT64_MAX + 1`, which is the 
number you are talking about.
Maybe you want to talk about the maximum stored value?



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:701-730
+  // Convert each range from the original range set.
+  for (const Range &R : What) {
+// Get bounds of the next range.
+APSInt FromInt = R.From();
+APSInt ToInt = R.To();
+RHS.clear();
+// CurrentRangeSize is a number of all possible values of the current range

I think we should have a shortcut for widening casts, where we don't need to go 
and unite them.  We just literally need to convert every range from one type to 
another and be done with it.



Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:129-141
+  template  RangeSet from(APSIntType Ty, RawRangeSetT Init) {
+llvm::APSInt First, Second;
+Ty.apply(First);
+Ty.apply(Second);
 RangeSet RangeSet = F.getEmptySet();
 for (const auto &Raw : Init) {
+  First = Raw.first;

I don't really understand what's going on here.



Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:646-647
+TYPED_TEST(RangeSetTest, RangeSetCastToTest) {
+  // TODO: Simplify calls below using list of types (int8_t, int16_t,
+  // int32_t, etc.).
+

Yes, please.



Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:695
+
+template  void RangeSetTest::checkCastTo_NOOP() {
+  // Just to reduce the verbosity.

Test cases don't belong in `RangeSetTest`.

Each piece of code like this should be a separate test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103094

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

[PATCH] D98710: [clang-tidy] Add --skip-headers, part 1 (use setTraversalScope)

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Sorry, previous sent too soon. +hokein as I'll be out next week.

I think we should be careful about which layers the filtering logic is added 
to, and how much surface area it has.

The minimal thing seems to be adding the clang-tidy options, and modifying the 
AST-consumer bits of the clang-tidy driver to compute and set the traversal 
scope based on these options.
I don't think we should bake the "filter" concept into other layers in clang 
unless there's a strong reason we need to do so.
(As discussed offline, providing a "filter" API requires all the candidates to 
be enumerated, which can be an expensive operation in the presence of 
modules/PCH - that's why the model in common layers today is "optional list of 
top-level decls that should be traversed").




Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:31
+
+struct DeclFilter : public OptionsDeclFilter {
+  DeclFilter(ClangTidyContext &Ctx, SourceManager &SM)

The layering in clang-tidy is... not particularly great or clear, but there is 
a distinction between driver-y type code (the clang-tidy application) vs the 
checks themselves.

As I understand our plan, this filter is "just" a way to configure the 
traversal scope before running, i.e. what information the AST will expose to 
the checks.
That is, this is a driver concern and should not be visible to the checks - 
probably should go in `ClangTidy.h` or even just `ClangTidy.cpp` as part of the 
`ClangTidyASTConsumer`. My litmus test here is that we do have other drivers 
(at least clangd that I know of), and they shouldn't be seeing/reusing this 
mechanism, and so neither should checks.

(Similarly if we do something with PP in the future, hopefully this can *also* 
be transparent to checks)





Comment at: clang/include/clang/ASTMatchers/ASTMatchFinder.h:148
+/// Check if a Decl should be skipped.
+std::shared_ptr Filter;
   };

I don't think the filter belongs here.
The design of traversal scope is that it's a property of the AST that affects 
all traversals, so it shouldn't be a configuration property of particular 
traversals like ASTMatchFinder.

There's a question of what to do about `MatchFinder::newASTConsumer()`, which 
runs the finder immediately after an AST is created, and so covers the point at 
which we might set a scope. There are several good options, e.g.:
 - Don't provide any facility for setting traversal scope when newASTConsumer() 
is used - it's not commonly needed, and the ASTConsumer is trivial to 
reimplement where that's actually needed
 - Accept an optional `function` which should run just 
before matching starts

This seems a bit subtle, but the difference between having this in MatchFinder 
proper vs just in newASTConsumer() is important I think, precisely because it's 
common to run the matchers directly on an existing AST.



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

https://reviews.llvm.org/D98710

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

[PATCH] D102689: [C++] Ignore top-level qualifiers in casts

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks!

I think the diagnostic wording can be improved separately if it's considered 
important as it seems this issue already existed.

I suggest adding `[Sema]` tag in your final commit title too and it would be 
good to explain generic C++ rules in the commit message.


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

https://reviews.llvm.org/D102689

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

[PATCH] D104040: [OpenCL] Add TableGen emitter for OpenCL builtin header

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia accepted this revision.
Anastasia added a comment.

Do you think we could add any test for this yet? Or do we have to wait until we 
have other pieces in place for diffing to `opencl-c.h`? We could add a test for 
parsing the header but it is probably going to be very slow test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104040

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

[PATCH] D103911: [OpenCL] Add support of __opencl_c_images feature macro

[Anton Zabaznov via Phabricator via cfe-commits]

azabaznov updated this revision to Diff 352681.
azabaznov added a comment.

Set SPIR target for added test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103911

Files:
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaOpenCL/unsupported-image.cl

Index: clang/test/SemaOpenCL/unsupported-image.cl
===
--- /dev/null
+++ clang/test/SemaOpenCL/unsupported-image.cl
@@ -0,0 +1,61 @@
+// RUN: %clang_cc1 -triple spir-unknown-unknown -verify -cl-std=CL3.0 -cl-ext=-__opencl_c_images %s
+// RUN: %clang_cc1 -triple spir-unknown-unknown -verify -cl-std=CL3.0 -cl-ext=+__opencl_c_images %s
+
+#ifdef __opencl_c_images
+//expected-no-diagnostics
+#endif
+
+void test1(image1d_t i) {}
+#if !defined(__opencl_c_images)
+// expected-error@-2{{use of type '__read_only image1d_t' requires __opencl_c_images support}}
+#endif
+
+void test2(image2d_t i) {}
+#if !defined(__opencl_c_images)
+// expected-error@-2{{use of type '__read_only image2d_t' requires __opencl_c_images support}}
+#endif
+
+void test3(image1d_array_t i) {}
+#if !defined(__opencl_c_images)
+// expected-error@-2{{use of type '__read_only image1d_array_t' requires __opencl_c_images support}}
+#endif
+
+void test4(image2d_array_t i) {}
+#if !defined(__opencl_c_images)
+// expected-error@-2{{use of type '__read_only image2d_array_t' requires __opencl_c_images support}}
+#endif
+
+void test5(image2d_depth_t i) {}
+#if !defined(__opencl_c_images)
+// expected-error@-2{{use of type '__read_only image2d_depth_t' requires __opencl_c_images support}}
+#endif
+
+void test6(image1d_buffer_t i) {}
+#if !defined(__opencl_c_images)
+// expected-error@-2{{use of type '__read_only image1d_buffer_t' requires __opencl_c_images support}}
+#endif
+
+void test7(image2d_msaa_t i) {}
+#if !defined(__opencl_c_images)
+// expected-error@-2{{use of type '__read_only image2d_msaa_t' requires __opencl_c_images support}}
+#endif
+
+void test8(image2d_array_msaa_t i) {}
+#if !defined(__opencl_c_images)
+// expected-error@-2{{use of type '__read_only image2d_array_msaa_t' requires __opencl_c_images support}}
+#endif
+
+void test9(image2d_msaa_depth_t i) {}
+#if !defined(__opencl_c_images)
+// expected-error@-2{{use of type '__read_only image2d_msaa_depth_t' requires __opencl_c_images support}}
+#endif
+
+void test10(image2d_array_msaa_depth_t i) {}
+#if !defined(__opencl_c_images)
+// expected-error@-2{{use of type '__read_only image2d_array_msaa_depth_t' requires __opencl_c_images support}}
+#endif
+
+void test11(sampler_t s) {}
+#if !defined(__opencl_c_images)
+// expected-error@-2{{use of type 'sampler_t' requires __opencl_c_images support}}
+#endif
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1721,11 +1721,25 @@
   if (Result->containsErrors())
 declarator.setInvalidType();
 
-  if (S.getLangOpts().OpenCL && Result->isOCLImage3dWOType() &&
-  !S.getOpenCLOptions().isSupported("cl_khr_3d_image_writes", S.getLangOpts())) {
-S.Diag(DS.getTypeSpecTypeLoc(), diag::err_opencl_requires_extension)
-<< 0 << Result << "cl_khr_3d_image_writes";
-declarator.setInvalidType();
+  if (S.getLangOpts().OpenCL) {
+const auto &OpenCLOptions = S.getOpenCLOptions();
+StringRef OptName;
+// OpenCL C v3.0 s6.3.3 - OpenCL image types require __opencl_c_images
+// support
+if ((Result->isImageType() || Result->isSamplerT()) &&
+(S.getLangOpts().OpenCLVersion >= 300 &&
+ !OpenCLOptions.isSupported("__opencl_c_images", S.getLangOpts(
+  OptName = "__opencl_c_images";
+else if (Result->isOCLImage3dWOType() &&
+ !OpenCLOptions.isSupported("cl_khr_3d_image_writes",
+S.getLangOpts()))
+  OptName = "cl_khr_3d_image_writes";
+
+if (!OptName.empty()) {
+  S.Diag(DS.getTypeSpecTypeLoc(), diag::err_opencl_requires_extension)
+  << 0 << Result << OptName;
+  declarator.setInvalidType();
+}
   }
 
   bool IsFixedPointType = DS.getTypeSpecType() == DeclSpec::TST_accum ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104376: [clangd] Correct SelectionTree behavior around anonymous field access.

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/Selection.cpp:71
+  // so we reduce its range to match the CXXConstructExpr.
+  // (It's not clear that changing the clang AST would be correct in general).
+  if (const auto *ME = N.get()) {

I still think it is worth to explore this direction (will take a look on this 
if I have time), but also ok with the current approach. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104376

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

[PATCH] D103094: [analyzer] Implemented RangeSet::Factory::castTo function to perform promotions, truncations and conversions.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@vsavchenko Repled in inlines. Preparing an update.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:245-246
+///
+/// Complexity: O(N^2)
+/// where N = size(What)
+RangeSet castTo(RangeSet What, APSIntType Ty);

vsavchenko wrote:
> That is a bit of a red flag because instantly my intuition tells me that 
> there should be a linear option.
Unfortunately, `castTo` is N^2 because technically we can call `unite`(which is 
N) inside it N times.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:248
+RangeSet castTo(RangeSet What, APSIntType Ty);
+RangeSet castTo(RangeSet What, QualType T);
 

vsavchenko wrote:
> If we talk about types for ranges, the interface we have is incomplete.
> We should introduce something like `getType` for range sets, so that the user 
> can have a more holistic picture about types and ranges.
+1. Is there any place where we can put tasks as tech debt or just keep noting 
with FIXME/TODO's?



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:692
+  ContainerType Result;
+  ContainerType RHS;
+

vsavchenko wrote:
> We have LHS (left-hand side) and RHS (right-hand side) as a common convention 
> for naming operands of binary operators.
> The fact that you later on use it as an RHS for `unite`, doesn't make it a 
> good name.
What about `Tmp`?



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:697
+  // But in fact we use `number of values` - 1, because
+  // we can't hold `UINT64 number of values` inside uint64_t.
+  // And it's OK, it's enough to do correct calculations.

vsavchenko wrote:
> That's incorrect. `uint64_t` holds **exactly** this many values.  If you 
> count every number from `0` to `UINT64_MAX`, you get `UINT64_MAX + 1`, which 
> is the number you are talking about.
> Maybe you want to talk about the maximum stored value?
>Maybe you want to talk about the maximum stored value?
Yes. I should rephrase the comment.
E.g.: 128 is an amount of all possible values of `char` and we can't keep it 
inside `char`.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:701-730
+  // Convert each range from the original range set.
+  for (const Range &R : What) {
+// Get bounds of the next range.
+APSInt FromInt = R.From();
+APSInt ToInt = R.To();
+RHS.clear();
+// CurrentRangeSize is a number of all possible values of the current range

vsavchenko wrote:
> I think we should have a shortcut for widening casts, where we don't need to 
> go and unite them.  We just literally need to convert every range from one 
> type to another and be done with it.
>we don't need to go and unite them.
Um, yes. 'unite' only needed for truncated range sets which have more then one 
range. I'll try to refactor this.
That will help make the algorithm //linear //for most cases.



Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:129-141
+  template  RangeSet from(APSIntType Ty, RawRangeSetT Init) {
+llvm::APSInt First, Second;
+Ty.apply(First);
+Ty.apply(Second);
 RangeSet RangeSet = F.getEmptySet();
 for (const auto &Raw : Init) {
+  First = Raw.first;

vsavchenko wrote:
> I don't really understand what's going on here.
This is a generalized version of `from` function. But this one you can use 
independently from which type the current test case uses. You can set a custom 
type to get a range set for a given type. This allows to get range sets based 
on different types (e.g. `char` and `int`)  in a single test case.
Previously `from` produced range sets only for the type specified by a test 
case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103094

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

[PATCH] D103094: [analyzer] Implemented RangeSet::Factory::castTo function to perform promotions, truncations and conversions.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:245-246
+///
+/// Complexity: O(N^2)
+/// where N = size(What)
+RangeSet castTo(RangeSet What, APSIntType Ty);

ASDenysPetrov wrote:
> vsavchenko wrote:
> > That is a bit of a red flag because instantly my intuition tells me that 
> > there should be a linear option.
> Unfortunately, `castTo` is N^2 because technically we can call `unite`(which 
> is N) inside it N times.
Corrected.
Unfortunately, castTo is N^2 because technically we can call unite(which is 
**N+M**) inside it N times.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103094

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

[PATCH] D103094: [analyzer] Implemented RangeSet::Factory::castTo function to perform promotions, truncations and conversions.

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:245-246
+///
+/// Complexity: O(N^2)
+/// where N = size(What)
+RangeSet castTo(RangeSet What, APSIntType Ty);

ASDenysPetrov wrote:
> ASDenysPetrov wrote:
> > vsavchenko wrote:
> > > That is a bit of a red flag because instantly my intuition tells me that 
> > > there should be a linear option.
> > Unfortunately, `castTo` is N^2 because technically we can call 
> > `unite`(which is N) inside it N times.
> Corrected.
> Unfortunately, castTo is N^2 because technically we can call unite(which is 
> **N+M**) inside it N times.
> Unfortunately, castTo is N^2 because technically we can call unite(which is 
> N) inside it N times.

I know why you put N^2, and what I meant was that there is probably a better 
algorithm than that. 

And as I said there is a linear algorithm:
If you have a smaller type that is, let's say, 8 times smaller than the larger 
type.  We can split the original range set into 8 range sets and unite them.
This difference between type sizes is limited (because we don't support integer 
types of arbitrary bit width) aka is a constant.  However, IRL the size of 
range sets is very small, and this constant factor can make it much slower than 
your N^2 algorithm.

But it doesn't mean that there is no better algorithm and that you HAVE TO call 
`unite` in a loop N times.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103094

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

[PATCH] D103911: [OpenCL] Add support of __opencl_c_images feature macro

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh accepted this revision.
svenvh 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/D103911/new/

https://reviews.llvm.org/D103911

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

[PATCH] D104455: [clangd] Explicitly fail if the file passed to --check is not valid.

[Haojian Wu via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104455

Files:
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -909,7 +909,11 @@
 
   if (CheckFile.getNumOccurrences()) {
 llvm::SmallString<256> Path;
-llvm::sys::fs::real_path(CheckFile, Path, /*expand_tilde=*/true);
+if (auto Error =
+llvm::sys::fs::real_path(CheckFile, Path, /*expand_tilde=*/true)) {
+  elog("Failed to resolve path {0}: {1}", CheckFile, Error.message());
+  return 1;
+}
 log("Entering check mode (no LSP server)");
 uint32_t Begin = 0, End = std::numeric_limits::max();
 if (!CheckFileLines.empty()) {


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -909,7 +909,11 @@
 
   if (CheckFile.getNumOccurrences()) {
 llvm::SmallString<256> Path;
-llvm::sys::fs::real_path(CheckFile, Path, /*expand_tilde=*/true);
+if (auto Error =
+llvm::sys::fs::real_path(CheckFile, Path, /*expand_tilde=*/true)) {
+  elog("Failed to resolve path {0}: {1}", CheckFile, Error.message());
+  return 1;
+}
 log("Entering check mode (no LSP server)");
 uint32_t Begin = 0, End = std::numeric_limits::max();
 if (!CheckFileLines.empty()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103094: [analyzer] Implemented RangeSet::Factory::castTo function to perform promotions, truncations and conversions.

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:692
+  ContainerType Result;
+  ContainerType RHS;
+

ASDenysPetrov wrote:
> vsavchenko wrote:
> > We have LHS (left-hand side) and RHS (right-hand side) as a common 
> > convention for naming operands of binary operators.
> > The fact that you later on use it as an RHS for `unite`, doesn't make it a 
> > good name.
> What about `Tmp`?
Maybe something like `Dummy` or `Intermediate`.



Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:129-141
+  template  RangeSet from(APSIntType Ty, RawRangeSetT Init) {
+llvm::APSInt First, Second;
+Ty.apply(First);
+Ty.apply(Second);
 RangeSet RangeSet = F.getEmptySet();
 for (const auto &Raw : Init) {
+  First = Raw.first;

ASDenysPetrov wrote:
> vsavchenko wrote:
> > I don't really understand what's going on here.
> This is a generalized version of `from` function. But this one you can use 
> independently from which type the current test case uses. You can set a 
> custom type to get a range set for a given type. This allows to get range 
> sets based on different types (e.g. `char` and `int`)  in a single test case.
> Previously `from` produced range sets only for the type specified by a test 
> case.
OK then, if you make this `from` templated, make all of them templated and you 
won't need to duplicate the logic with `sizeof * 8` everywhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103094

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

[PATCH] D104136: [analyzer] Add better tracking for RetainCountChecker leak warnings

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp:953-957
+  // Let's traverse...
+  for (const ExplodedNode *N = ExprNode;
+   // ...all the nodes corresponding to the given expression...
+   N != nullptr && N->getStmtForDiagnostics() == E;
+   N = N->getFirstPred()) {

Szelethus wrote:
> vsavchenko wrote:
> > NoQ wrote:
> > > I guess this part should ultimately be written in one place, eg. 
> > > `ExplodedNode::findTag()` or something like that.
> > > 
> > > I'd also really like to explore the possibility to further limit the 
> > > variety of nodes traversed here. What nodes are typically traversed here? 
> > > Is it checker-tagged nodes or is it purge dead symbol nodes or something 
> > > else?
> > Yes, `ExplodedNode::findTag()` sounds like a great idea!
> > 
> > I mean it is hard to tell without calculating statistics right here and 
> > running it on a bunch of projects.  However, it is always possible to write 
> > the code that will have it the other way.  My take on it is that it is 
> > probably a mix of things.
> > 
> > I'd also prefer to traverse less, do you have any specific ideas here?
> > I'd also really like to explore the possibility to further limit the 
> > variety of nodes traversed here. What nodes are typically traversed here? 
> > Is it checker-tagged nodes or is it purge dead symbol nodes or something 
> > else?
> 
> Is there something I'm not seeing here? Trackers basically ascend //a// path 
> from the error node to at most the root of the ExplodedGraph (not the trimmed 
> version, as `Tracker::track()` is called before any of that process happens), 
> so its not much slower than `trackExpressionValue`, right?
> 
> Or does this, and likely many other future handlers run such a loop more 
> often then what I imagine?
> 
> 
Essentially, yes.  `trackExpressionValue` (and `Tracker::track`) ascend the 
graph searching for a node where the give expression is evaluated.  The problem 
here is that whenever we ask to track some expression, not only 
`trackExpressionValue` will do its traversal (much longer), but also this 
handler will do its (shorter).  It would be better not to do this altogether, 
but I personally don't see any solutions how we can cut on this part.
At the same time, I don't think that it is performance critical, considering 
that the part of the graph where `N->getStmtForDiagnostics() == E` is small.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104136

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

[PATCH] D104387: [clang-cl] Implement /external:I, /external:env, and EXTERNAL_INCLUDE support (PR36003)

[Hans Wennborg via Phabricator via cfe-commits]

hans updated this revision to Diff 352695.
hans marked 2 inline comments as done.
hans added a comment.
Herald added a subscriber: ormris.

More tests, add help text for command-line options.


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

https://reviews.llvm.org/D104387

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/test/Driver/cl-include.c
  clang/test/Driver/cl-options.c

Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -38,6 +38,10 @@
 // EP: "-P"
 // EP: "-o" "-"
 
+// RUN: %clang_cl /external:Ipath  -### -- %s 2>&1 | FileCheck -check-prefix=EXTERNAL_I %s
+// RUN: %clang_cl /external:I path -### -- %s 2>&1 | FileCheck -check-prefix=EXTERNAL_I %s
+// EXTERNAL_I: "-isystem" "path"
+
 // RUN: %clang_cl /fp:fast /fp:except -### -- %s 2>&1 | FileCheck -check-prefix=fpexcept %s
 // fpexcept-NOT: -menable-unsafe-fp-math
 
@@ -434,8 +438,6 @@
 // RUN: /experimental:preprocessor \
 // RUN: /exportHeader /headerName:foo \
 // RUN: /external:anglebrackets \
-// RUN: /external:Ipath \
-// RUN: /external:I path \
 // RUN: /external:env:var \
 // RUN: /external:W0 \
 // RUN: /external:W1 \
Index: clang/test/Driver/cl-include.c
===
--- clang/test/Driver/cl-include.c
+++ clang/test/Driver/cl-include.c
@@ -7,19 +7,37 @@
 // RUN: %clang_cl -nobuiltininc -### -- %s 2>&1 | FileCheck %s --check-prefix=NOBUILTIN
 // NOBUILTIN-NOT: "-internal-isystem" "{{.*lib.*clang.*include}}"
 
-// RUN: env INCLUDE=/my/system/inc %clang_cl -### -- %s 2>&1 | FileCheck %s --check-prefix=STDINC
+// RUN: env INCLUDE=/my/system/inc env EXTERNAL_INCLUDE=/my/system/inc2 %clang_cl -### -- %s 2>&1 | FileCheck %s --check-prefix=STDINC
 // STDINC: "-internal-isystem" "/my/system/inc"
+// STDINC: "-internal-isystem" "/my/system/inc2"
 
 // -nostdinc suppresses all of %INCLUDE%, clang resource dirs, and -imsvc dirs.
-// RUN: env INCLUDE=/my/system/inc %clang_cl -nostdinc -imsvc /my/other/inc -### -- %s 2>&1 | FileCheck %s --check-prefix=NOSTDINC
+// RUN: env INCLUDE=/my/system/inc env EXTERNAL_INCLUDE=/my/system/inc2 %clang_cl -nostdinc -imsvc /my/other/inc -### -- %s 2>&1 | FileCheck %s --check-prefix=NOSTDINC
 // NOSTDINC: argument unused{{.*}}-imsvc
 // NOSTDINC-NOT: "-internal-isystem" "/my/system/inc"
+// NOSTDINC-NOT: "-internal-isystem" "/my/system/inc2"
 // NOSTDINC-NOT: "-internal-isystem" "{{.*lib.*clang.*include}}"
 // NOSTDINC-NOT: "-internal-isystem" "/my/other/inc"
 
-// /X suppresses %INCLUDE% but not clang resource dirs or -imsvc dirs.
-// RUN: env INCLUDE=/my/system/inc %clang_cl /X -imsvc /my/other/inc -### -- %s 2>&1 | FileCheck %s --check-prefix=SLASHX
+// /X suppresses %INCLUDE% and %EXTERNAL_INCLUDE% but not clang resource dirs, -imsvc dirs, or /external: flags.
+// RUN: env INCLUDE=/my/system/inc env EXTERNAL_INCLUDE=/my/system/inc2 env FOO=/my/other/inc2 %clang_cl /X -imsvc /my/other/inc /external:env:FOO -### -- %s 2>&1 | FileCheck %s --check-prefix=SLASHX
 // SLASHX-NOT: "argument unused{{.*}}-imsvc"
 // SLASHX-NOT: "-internal-isystem" "/my/system/inc"
+// SLASHX-NOT: "-internal-isystem" "/my/system/inc2"
 // SLASHX: "-internal-isystem" "{{.*lib.*clang.*include}}"
 // SLASHX: "-internal-isystem" "/my/other/inc"
+// SLASHX: "-internal-isystem" "/my/other/inc2"
+
+// /winsysroot suppresses %EXTERNAL_INCLUDE% but not -imsvc dirs or /external: flags.
+// RUN: env env EXTERNAL_INCLUDE=/my/system/inc env FOO=/my/other/inc2 %clang_cl /winsysroot /foo -imsvc /my/other/inc /external:env:FOO -### -- %s 2>&1 | FileCheck %s --check-prefix=SYSROOT
+// SYSROOT-NOT: "argument unused{{.*}}-imsvc"
+// SYSROOT-NOT: "argument unused{{.*}}/external:"
+// SYSROOT-NOT: "/my/system/inc"
+// SYSROOT: "-internal-isystem" "/my/other/inc"
+// SYSROOT: "-internal-isystem" "/my/other/inc2"
+// SYSROOT: "-internal-isystem" "/foo{{.*}}"
+
+// RUN: env "FOO=/dir1;/dir2" env "BAR=/dir3" %clang_cl /external:env:FOO /external:env:BAR -### -- %s 2>&1 | FileCheck %s --check-prefix=EXTERNAL_ENV
+// EXTERNAL_ENV: "-internal-isystem" "/dir1"
+// EXTERNAL_ENV: "-internal-isystem" "/dir2"
+// EXTERNAL_ENV: "-internal-isystem" "/dir3"
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -1243,22 +1243,45 @@
   for (const auto &Path : DriverArgs.getAllArgValues(options::OPT__SLASH_imsvc))
 addSystemInclude(DriverArgs, CC1Args, Path);
 
+  // Add %INCLUDE%-like dirs via the /external:env: flag.
+  for (const auto &Var :
+   DriverArgs.getAllArgValues(options::OPT__SLASH_external_env)) {
+if (auto Val = llvm::sys::Process::GetEnv(Var)) {
+  SmallVector Dirs;
+  StringRef(*Val).split(Dirs, ";", /*MaxSplit=*/-1, /*KeepEm

[PATCH] D104387: [clang-cl] Implement /external:I, /external:env, and EXTERNAL_INCLUDE support (PR36003)

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:1255
+}
+  }
+

thakis wrote:
> `/external:env` should happen after winsysroot I think. sysroots try to make 
> builds hermetic and env vars defeat that.
> 
> I.e. this flag is more like the env var reads below than imsvc above – it 
> reads the env mostly, instead of being a flag mostly.
Hmm. I guess it depends on how people will use this flag, which isn't entirely 
clear. 

The way I think about it, users might use this to point to third-party 
libraries besides the system headers. For that reason it's different from the 
env var reads below, which are about finding the system headers.

In particular, I don't think /nostdlibinc (/X) should disable these flags, 
which is why I put it before that check.

I don't think people would combine this with /winsysroot, but if they want to, 
I'm not sure we should prevent it.



Comment at: clang/test/Driver/cl-include.c:10
 
-// RUN: env INCLUDE=/my/system/inc %clang_cl -### -- %s 2>&1 | FileCheck %s 
--check-prefix=STDINC
+// RUN: env INCLUDE=/my/system/inc env EXTERNAL_INCLUDE=/my/system/inc2 
%clang_cl -### -- %s 2>&1 | FileCheck %s --check-prefix=STDINC
 // STDINC: "-internal-isystem" "/my/system/inc"

thakis wrote:
> Should there be tests for the interaction with `/X`, `/winsysroot:`?
Yes, adding that.


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

https://reviews.llvm.org/D104387

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

[PATCH] D104351: [HeaderSearch] Use `isImport` only for imported headers and not for `#pragma once`.

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

This seems reasonable on first look. Can you add a test that demonstrates the 
problem this patch solves?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104351

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

[PATCH] D104459: [clang][lex] Ensure minimizer output is never larger than input

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, arphaman.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch ensures the output of source minimizer is never larger than the 
input. This property will be handy in a follow up patch that makes it possible 
to pad the minimized output to the size of the original input.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104459

Files:
  clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
  clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp

Index: clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
===
--- clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
+++ clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
@@ -94,7 +94,7 @@
 
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectives("#define MACRO", Out, Tokens));
-  EXPECT_STREQ("#define MACRO\n", Out.data());
+  EXPECT_STREQ("#define MACRO", Out.data());
   ASSERT_EQ(2u, Tokens.size());
   ASSERT_EQ(pp_define, Tokens.front().K);
 }
@@ -123,7 +123,7 @@
   SmallVector Out;
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define MACRO()", Out));
-  EXPECT_STREQ("#define MACRO()\n", Out.data());
+  EXPECT_STREQ("#define MACRO()", Out.data());
 
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectives("#define MACRO(a, b...)", Out));
@@ -131,7 +131,7 @@
 
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectives("#define MACRO content", Out));
-  EXPECT_STREQ("#define MACRO content\n", Out.data());
+  EXPECT_STREQ("#define MACRO content", Out.data());
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives(
   "#define MACRO   con  tent   ", Out));
@@ -146,14 +146,14 @@
   SmallVector Out;
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define MACRO((a))", Out));
-  EXPECT_STREQ("#define MACRO(/* invalid */\n", Out.data());
+  EXPECT_STREQ("#define MACRO\n", Out.data());
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define MACRO(", Out));
-  EXPECT_STREQ("#define MACRO(/* invalid */\n", Out.data());
+  EXPECT_STREQ("#define MACRO\n", Out.data());
 
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectives("#define MACRO(a * b)", Out));
-  EXPECT_STREQ("#define MACRO(/* invalid */\n", Out.data());
+  EXPECT_STREQ("#define MACRO\n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, DefineHorizontalWhitespace) {
@@ -259,7 +259,7 @@
   SmallVector Out;
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define AND&\n", Out));
-  EXPECT_STREQ("#define AND &\n", Out.data());
+  EXPECT_STREQ("#define AND&\n", Out.data());
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define AND\\\n"
 "&\n",
Index: clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
===
--- clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
+++ clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
@@ -93,6 +93,7 @@
   void printDirectiveBody(const char *&First, const char *const End);
   void printAdjacentMacroArgs(const char *&First, const char *const End);
   LLVM_NODISCARD bool printMacroArgs(const char *&First, const char *const End);
+  void maybePrintNewLine(const char *&Last, const char *const End);
 
   /// Reports a diagnostic if the diagnostic engine is provided. Always returns
   /// true at the end.
@@ -446,13 +447,15 @@
   }
 }
 
-static void skipWhitespace(const char *&First, const char *const End) {
+static size_t skipWhitespace(const char *&First, const char *const End) {
+  const char *FirstBefore = First;
+
   for (;;) {
 assert(First <= End);
 skipOverSpaces(First, End);
 
 if (End - First < 2)
-  return;
+  break;
 
 if (First[0] == '\\' && isVerticalWhitespace(First[1])) {
   skipNewline(++First, End);
@@ -461,21 +464,23 @@
 
 // Check for a non-comment character.
 if (First[0] != '/')
-  return;
+  break;
 
 // "// ...".
 if (First[1] == '/') {
   skipLineComment(First, End);
-  return;
+  break;
 }
 
 // Cannot be a comment.
 if (First[1] != '*')
-  return;
+  break;
 
 // "/*...*/".
 skipBlockComment(First, End);
   }
+
+  return First - FirstBefore;
 }
 
 void Minimizer::printAdjacentModuleNameParts(const char *&First,
@@ -519,7 +524,7 @@
   printToNewline(First, End);
   while (Out.back() == ' ')
 Out.pop_back();
-  put('\n');
+  maybePrintNewLine(First, End);
 }
 
 LLVM_NODISCARD static const char *lexRawIdentifier(const char *First,
@@ -595,6 +600,12 @@
   }
 }
 
+void Minimizer::maybePrintNewLine(const char *&Last, const char *const End) {
+  // Only print newline if doing so won't make the output larger than the input.
+  if (Last != End)
+put('\n');
+}
+
 /// Looks for an identifi

[PATCH] D104460: [clang][lex] NFC: Extract source variable in minimizer tests

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, arphaman.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch extracts the to-be-minimized source strings into variables. This 
will be useful in a follow up patch.

Depends on D104459 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104460

Files:
  clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp

Index: clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
===
--- clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
+++ clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
@@ -30,13 +30,14 @@
   SmallVector Out;
   SmallVector Tokens;
 
-  ASSERT_FALSE(minimizeSourceToDependencyDirectives("", Out, Tokens));
+  StringRef Source = "";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out, Tokens));
   EXPECT_TRUE(Out.empty());
   ASSERT_EQ(1u, Tokens.size());
   ASSERT_EQ(pp_eof, Tokens.back().K);
 
-  ASSERT_FALSE(
-  minimizeSourceToDependencyDirectives("abc def\nxyz", Out, Tokens));
+  Source = "abc def\nxyz";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out, Tokens));
   EXPECT_TRUE(Out.empty());
   ASSERT_EQ(1u, Tokens.size());
   ASSERT_EQ(pp_eof, Tokens.back().K);
@@ -92,8 +93,8 @@
   SmallVector Out;
   SmallVector Tokens;
 
-  ASSERT_FALSE(
-  minimizeSourceToDependencyDirectives("#define MACRO", Out, Tokens));
+  StringRef Source = "#define MACRO";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out, Tokens));
   EXPECT_STREQ("#define MACRO", Out.data());
   ASSERT_EQ(2u, Tokens.size());
   ASSERT_EQ(pp_define, Tokens.front().K);
@@ -102,95 +103,96 @@
 TEST(MinimizeSourceToDependencyDirectivesTest, DefineSpacing) {
   SmallVector Out;
 
-  ASSERT_FALSE(
-  minimizeSourceToDependencyDirectives("#define MACRO\n\n\n", Out));
+  StringRef Source = "#define MACRO\n\n\n";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
   EXPECT_STREQ("#define MACRO\n", Out.data());
 
-  ASSERT_FALSE(
-  minimizeSourceToDependencyDirectives("#define MACRO \n\n\n", Out));
+  Source = "#define MACRO \n\n\n";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
   EXPECT_STREQ("#define MACRO\n", Out.data());
 
-  ASSERT_FALSE(
-  minimizeSourceToDependencyDirectives("#define MACRO a \n\n\n", Out));
+  Source = "#define MACRO a \n\n\n";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
   EXPECT_STREQ("#define MACRO a\n", Out.data());
 
-  ASSERT_FALSE(
-  minimizeSourceToDependencyDirectives("#define   MACRO\n\n\n", Out));
+  Source = "#define   MACRO\n\n\n";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
   EXPECT_STREQ("#define MACRO\n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, DefineMacroArguments) {
   SmallVector Out;
 
-  ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define MACRO()", Out));
+  StringRef Source = "#define MACRO()";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
   EXPECT_STREQ("#define MACRO()", Out.data());
 
-  ASSERT_FALSE(
-  minimizeSourceToDependencyDirectives("#define MACRO(a, b...)", Out));
+  Source = "#define MACRO(a, b...)";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
   EXPECT_STREQ("#define MACRO(a,b...)\n", Out.data());
 
-  ASSERT_FALSE(
-  minimizeSourceToDependencyDirectives("#define MACRO content", Out));
+  Source = "#define MACRO content";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
   EXPECT_STREQ("#define MACRO content", Out.data());
 
-  ASSERT_FALSE(minimizeSourceToDependencyDirectives(
-  "#define MACRO   con  tent   ", Out));
+  Source = "#define MACRO   con  tent   ";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
   EXPECT_STREQ("#define MACRO con  tent\n", Out.data());
 
-  ASSERT_FALSE(minimizeSourceToDependencyDirectives(
-  "#define MACRO()   con  tent   ", Out));
+  Source = "#define MACRO()   con  tent   ";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
   EXPECT_STREQ("#define MACRO() con  tent\n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, DefineInvalidMacroArguments) {
   SmallVector Out;
 
-  ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define MACRO((a))", Out));
+  StringRef Source = "#define MACRO((a))";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
   EXPECT_STREQ("#define MACRO\n", Out.data());
 
-  ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define MACRO(", Out));
+  Source = "#define MACRO(";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
   EXPECT_STREQ("#define MACRO\n", Out.data());
 
-  ASSERT_FALSE(
-  minimizeSourceToDependencyDirectives("#define MACR

[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D102107#2823706 , @jdoerfert wrote:

> In D102107#2821976 , @ABataev wrote:
>
>> We used this kind of codegen initially but later found out that it causes a 
>> large overhead when gathering pointers into a record. What about hybrid 
>> scheme where the first args are passed as arguments and others (if any) are 
>> gathered into a record?
>
> I'm confused, maybe I misunderstand the problem. The parallel function 
> arguments need to go from the main thread to the workers somehow, I don't see 
> how this is done w/o a record. This patch makes it explicit though.

Pass it in a record for workers only? And use a hybrid scheme for all other 
parallel regions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102107

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

[PATCH] D103793: [Clang][OpenMP] Monotonic does not apply to SIMD

[Graham Hunter via Phabricator via cfe-commits]

huntergr added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103793

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

[PATCH] D102875: [PowerPC] Add PowerPC compare and multiply related builtins and instrinsics for XL compatibility

[Victor Huang via Phabricator via cfe-commits]

NeHuang updated this revision to Diff 352560.
NeHuang added a comment.

- Add AIX 32&64 bit run line checks (front and back end test cases)
- Create builtin-ppc-xlcompat-error.c for arguments related error check, add 
error test case for `__builtin_ppc_cmprb`
- Remove 32 bit linux run line checks
- Update bulitin test cases to remove redundant definitions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102875

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-ppc-xlcompat-compare-64bit-only.c
  clang/test/CodeGen/builtins-ppc-xlcompat-compare.c
  clang/test/CodeGen/builtins-ppc-xlcompat-error.c
  clang/test/CodeGen/builtins-ppc-xlcompat-multiply-64bit-only.c
  clang/test/CodeGen/builtins-ppc-xlcompat-multiply.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCInstr64Bit.td
  llvm/lib/Target/PowerPC/PPCInstrInfo.td
  llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-compare-64bit-only.ll
  llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-compare.ll
  llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-multiply-64bit-only.ll
  llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-multiply.ll

Index: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-multiply.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-multiply.ll
@@ -0,0 +1,52 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   -mcpu=pwr9 < %s | FileCheck %s --check-prefix=CHECK-64
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
+; RUN:   -mcpu=pwr9 < %s | FileCheck %s --check-prefix=CHECK-64
+; RUN: llc -verify-machineinstrs -mtriple=powerpc-unknown-aix \
+; RUN:   -mcpu=pwr9 < %s | FileCheck %s --check-prefix=CHECK-32
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix \
+; RUN:   -mcpu=pwr9 < %s | FileCheck %s --check-prefix=CHECK-64
+
+; Function Attrs: noinline nounwind optnone
+define dso_local signext i32 @test_builtin_ppc_mulhw(i32 %a, i32%b) #0 {
+; CHECK-32-LABEL: test_builtin_ppc_mulhw:
+; CHECK-32:   # %bb.0: # %entry
+; CHECK-32-NEXT:mulhw 3, 3, 4
+; CHECK-32-NEXT:blr
+;
+; CHECK-64-LABEL: test_builtin_ppc_mulhw:
+; CHECK-64:   # %bb.0: # %entry
+; CHECK-64-NEXT:mulhw 3, 3, 4
+; CHECK-64-NEXT:extsw 3, 3
+; CHECK-64-NEXT:blr
+entry:
+  %0 = call i32 @llvm.ppc.mulhw(i32 %a, i32 %b)
+  ret i32 %0
+}
+
+; Function Attrs: nounwind readnone
+declare i32 @llvm.ppc.mulhw(i32, i32) #1
+
+; Function Attrs: noinline nounwind optnone
+define dso_local zeroext i32 @test_builtin_ppc_mulhwu(i32 %a, i32%b) #0 {
+; CHECK-32-LABEL: test_builtin_ppc_mulhwu:
+; CHECK-32:   # %bb.0: # %entry
+; CHECK-32-NEXT:mulhwu 3, 3, 4
+; CHECK-32-NEXT:blr
+;
+; CHECK-64-LABEL: test_builtin_ppc_mulhwu:
+; CHECK-64:   # %bb.0: # %entry
+; CHECK-64-NEXT:mulhwu 3, 3, 4
+; CHECK-64-NEXT:clrldi 3, 3, 32
+; CHECK-64-NEXT:blr
+entry:
+  %0 = call i32 @llvm.ppc.mulhwu(i32 %a, i32 %b)
+  ret i32 %0
+}
+
+; Function Attrs: nounwind readnone
+declare i32 @llvm.ppc.mulhwu(i32, i32) #1
+
+attributes #0 = { noinline nounwind optnone "frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="pwr9" "target-features"="+altivec,+bpermd,+crypto,+direct-move,+extdiv,+htm,+power8-vector,+power9-vector,+vsx,-privileged,-rop-protect,-spe" }
+attributes #1 = { nounwind readnone }
Index: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-multiply-64bit-only.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-multiply-64bit-only.ll
@@ -0,0 +1,82 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   -mcpu=pwr9 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
+; RUN:   -mcpu=pwr9 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix \
+; RUN:   -mcpu=pwr9 < %s | FileCheck %s
+
+; Function Attrs: noinline nounwind optnone
+define dso_local i64 @test_builtin_ppc_mulhd(i64 %a, i64 %b) #0 {
+; CHECK-LABEL: test_builtin_ppc_mulhd:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mulhd 3, 3, 4
+; CHECK-NEXT:blr
+entry:
+  %0 = call i64 @llvm.ppc.mulhd(i64 %a, i64 %b)
+  ret i64 %0
+}
+
+; Function Attrs: nounwind readnone
+declare i64 @llvm.ppc.mulhd(i64, i64) #1
+
+; Function Attrs: noinline nounwind optnone
+define dso_local i64 @test_builtin_ppc_mulhdu(i64 %a, i64 %b) #0 {
+; CHECK-LABEL: test_builtin_ppc_mulhdu:
+; CHECK:   # %bb.0: # %entry
+; CHEC

[PATCH] D103793: [Clang][OpenMP] Monotonic does not apply to SIMD

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103793

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

[PATCH] D102875: [PowerPC] Add PowerPC compare and multiply related builtins and instrinsics for XL compatibility

[Lei Huang via Phabricator via cfe-commits]

lei added inline comments.



Comment at: clang/lib/Basic/Targets/PPC.h:354
 
-  void defineXLCompatMacros(MacroBuilder &Builder) const {
-Builder.defineMacro("__popcntb", "__builtin_ppc_popcntb");

Can you pleases rebase your patch again?  The removal of this function was part 
of D104125.



Comment at: clang/test/CodeGen/builtins-ppc-xlcompat-compare-64bit-only.c:2
+// RUN: %clang_cc1 -triple powerpc64-unknown-unknown \
+// RUN:   -emit-llvm %s -o - -target-cpu pwr9 | FileCheck %s 
--check-prefix=CHECK-64
+// RUN: %clang_cc1 -triple powerpc64le-unknown-unknown \

Are these pwr9 specific as well?  If not please targe pwr7 for big endian and 
pwr8 for little endian



Comment at: clang/test/CodeGen/builtins-ppc-xlcompat-multiply-64bit-only.c:2
+// RUN: %clang_cc1 -triple powerpc64-unknown-unknown \
+// RUN:   -emit-llvm %s -o - -target-cpu pwr9 | FileCheck %s 
--check-prefix=CHECK-64
+// RUN: %clang_cc1 -triple powerpc64le-unknown-unknown \

same comment as above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102875

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

[PATCH] D104040: [OpenCL] Add TableGen emitter for OpenCL builtin header

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added a comment.

In D104040#2824281 , @Anastasia wrote:

> Do you think we could add any test for this yet? Or do we have to wait until 
> we have other pieces in place for diffing to `opencl-c.h`? We could add a 
> test for parsing the header but it is probably going to be very slow test?

The question is what we actually want to test at this point?  I am wondering if 
it would make more sense to work towards equivalence of `opencl-c.h` and the 
generated header file.  We can prove equivalence offline, as a one-off without 
needing to commit changes to the AST printer.  If they are equivalent, we can 
commit the generated header file in place of `opencl-c.h`.  After that, adding 
a test for diffing both should be fairly trivial.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104040

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

[PATCH] D104424: [Sema] Fix for PR50741

[Chris Hamilton via Phabricator via cfe-commits]

chrish_ericsson_atx updated this revision to Diff 352711.
chrish_ericsson_atx edited the summary of this revision.
chrish_ericsson_atx added a comment.

Addressed review feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104424

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/unbounded-array-bounds.c

Index: clang/test/Sema/unbounded-array-bounds.c
===
--- clang/test/Sema/unbounded-array-bounds.c
+++ clang/test/Sema/unbounded-array-bounds.c
@@ -1,9 +1,6 @@
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-X86-ADDR64 %s  \
-// RUN:  --implicit-check-not 'past the last possible element'
-// RUN: %clang_cc1 -triple i386-pc-linux-gnu   -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-I386-ADDR32 %s \
-// RUN:  --implicit-check-not 'past the last possible element'
-// RUN: %clang_cc1 -triple avr-pc-linux-gnu-fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-AVR-ADDR16 %s  \
-// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -Wno-unused -verify=addr64,expected %s
+// RUN: %clang_cc1 -triple i386-pc-linux-gnu   -fsyntax-only -Wno-unused -verify=addr32,expected %s
+// RUN: %clang_cc1 -triple avr-pc-linux-gnu-fsyntax-only -Wno-unused -verify=addr16,expected %s
 
 struct S {
   long long a;
@@ -12,61 +9,61 @@
   short d;
 };
 
-struct S s[];
+struct S s[]; // expected-warning {{tentative array definition}} expected-note {{declared here}} addr16-note {{declared here}}
 
 void f1() {
   ++s[3].a;
   ++s[7073650413200313099].b;
-  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: array index 7073650413200313099 refers past the last possible element for an array in 64-bit address space containing 256-bit (32-byte) elements (max possible 576460752303423488 elements)
-  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
-  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  // addr16-warning@-1 {{array index 7073650413200313099 refers past the last possible element for an array in 16-bit address space containing 160-bit (20-byte) elements (max possible 3276 elements)}}
+  // addr32-warning@-2 {{array index 7073650413200313099 refers past the last possible element for an array in 32-bit address space containing 192-bit (24-byte) elements (max possible 178956970 elements)}}
+  // addr64-warning@-3 {{array index 7073650413200313099 refers past the last possible element for an array in 64-bit address space containing 256-bit (32-byte) elements (max possible 576460752303423488 elements)}}
   ++s[7073650].c;
-  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  // addr16-warning@-1 {{array index 7073650 refers past the last possible element for an array in 16-bit address space containing 160-bit (20-byte) elements (max possible 3276 elements)}}
 }
 
-long long ll[];
+long long ll[]; // expected-warning {{tentative array definition}} expected-note {{declared here}} addr16-note {{declared here}} addr32-note {{declared here}}
 
 void f2() {
   ++ll[3];
   ++ll[2705843009213693952];
-  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 2305843009213693952 elements)
-  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
-  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+  // addr16-warning@-1 {{array index 2705843009213693952 refers past the last possible element for an array in 16-bit address space containing 64-bit (8-byte) elements (max possible 8192 elements)}}
+  // addr32-warning@-2 {{array index 2705843009213693952 refers past the last possible element for an array in 32-bit address space containing 64-bit (8-byte) elements (max possible 536870912 elements)}}
+  // addr64-warning@-3 {{array index 2705843009213693952 refers past the last possible element for an array in 64-bit address space containing 64-bit (8-byte) elements (max possible 2305843009213693952 elements)}}
   ++ll[847073650];
-  // CHECK-I386-ADDR32: :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
-  // CHECK-AVR-ADDR16:  :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+  // addr16-warning@-1 {{array index 847073650 refers past the last possible element for an array in 

[PATCH] D104424: [Sema] Fix for PR50741

[Erich Keane via Phabricator via cfe-commits]

erichkeane accepted this revision.
erichkeane added a comment.

Yep, that works, still approved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104424

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

[PATCH] D104424: [Sema] Fix for PR50741

[Chris Hamilton via Phabricator via cfe-commits]

chrish_ericsson_atx marked 2 inline comments as done.
chrish_ericsson_atx added a comment.

Thanks for the hints, @erichkeane !  Especially thanks for pointing out the 
root cause early on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104424

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

[clang] fc6ec9b - [Sema] Fix for PR50741

[via cfe-commits]

Author: eahcmrh
Date: 2021-06-17T16:16:59+02:00
New Revision: fc6ec9b98cf96bca8f68cc65395f861919c06c2f

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

LOG: [Sema] Fix for PR50741

Fixed crash when doing pointer math on a void pointer.

Also, reworked test to use -verify rather than FileCheck.

Reviewed By: erichkeane

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

Added: 


Modified: 
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/unbounded-array-bounds.c

Removed: 




diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index a9915cb6d720e..32b7861f7f003 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -14567,8 +14567,13 @@ void Sema::CheckArrayAccess(const Expr *BaseExpr, 
const Expr *IndexExpr,
   EffectiveType->getCanonicalTypeInternal()));
   if (index.getBitWidth() < AddrBits)
 index = index.zext(AddrBits);
-  CharUnits ElemCharUnits = ASTC.getTypeSizeInChars(EffectiveType);
-  llvm::APInt ElemBytes(index.getBitWidth(), ElemCharUnits.getQuantity());
+  Optional ElemCharUnits =
+  ASTC.getTypeSizeInCharsIfKnown(EffectiveType);
+  // PR50741 - If EffectiveType has unknown size (e.g., if it's a void
+  // pointer) bounds-checking isn't meaningful.
+  if (!ElemCharUnits)
+return;
+  llvm::APInt ElemBytes(index.getBitWidth(), ElemCharUnits->getQuantity());
   // If index has more active bits than address space, we already know
   // we have a bounds violation to warn about.  Otherwise, compute
   // address of (index + 1)th element, and warn about bounds violation
@@ -14599,7 +14604,7 @@ void Sema::CheckArrayAccess(const Expr *BaseExpr, const 
Expr *IndexExpr,
   DiagRuntimeBehavior(BaseExpr->getBeginLoc(), BaseExpr,
   PDiag(DiagID)
   << toString(index, 10, true) << AddrBits
-  << (unsigned)ASTC.toBits(ElemCharUnits)
+  << (unsigned)ASTC.toBits(*ElemCharUnits)
   << toString(ElemBytes, 10, false)
   << toString(MaxElems, 10, false)
   << (unsigned)MaxElems.getLimitedValue(~0U)

diff  --git a/clang/test/Sema/unbounded-array-bounds.c 
b/clang/test/Sema/unbounded-array-bounds.c
index d47463ff94345..061dfdbd17f25 100644
--- a/clang/test/Sema/unbounded-array-bounds.c
+++ b/clang/test/Sema/unbounded-array-bounds.c
@@ -1,9 +1,6 @@
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only %s 2>&1 | 
FileCheck --check-prefix=CHECK-X86-ADDR64 %s  \
-// RUN:  --implicit-check-not 'past the last possible element'
-// RUN: %clang_cc1 -triple i386-pc-linux-gnu   -fsyntax-only %s 2>&1 | 
FileCheck --check-prefix=CHECK-I386-ADDR32 %s \
-// RUN:  --implicit-check-not 'past the last possible element'
-// RUN: %clang_cc1 -triple avr-pc-linux-gnu-fsyntax-only %s 2>&1 | 
FileCheck --check-prefix=CHECK-AVR-ADDR16 %s  \
-// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -Wno-unused 
-verify=addr64,expected %s
+// RUN: %clang_cc1 -triple i386-pc-linux-gnu   -fsyntax-only -Wno-unused 
-verify=addr32,expected %s
+// RUN: %clang_cc1 -triple avr-pc-linux-gnu-fsyntax-only -Wno-unused 
-verify=addr16,expected %s
 
 struct S {
   long long a;
@@ -12,61 +9,61 @@ struct S {
   short d;
 };
 
-struct S s[];
+struct S s[]; // expected-warning {{tentative array definition}} expected-note 
{{declared here}} addr16-note {{declared here}}
 
 void f1() {
   ++s[3].a;
   ++s[7073650413200313099].b;
-  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: array index 
7073650413200313099 refers past the last possible element for an array in 
64-bit address space containing 256-bit (32-byte) elements (max possible 
576460752303423488 elements)
-  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible 
element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
-  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible 
element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  // addr16-warning@-1 {{array index 7073650413200313099 refers past the last 
possible element for an array in 16-bit address space containing 160-bit 
(20-byte) elements (max possible 3276 elements)}}
+  // addr32-warning@-2 {{array index 7073650413200313099 refers past the last 
possible element for an array in 32-bit address space containing 192-bit 
(24-byte) elements (max possible 178956970 elements)}}
+  // addr64-warning@-3 {{array index 7073650413200313099 refers past the last 
possible element for an array in 64-bit ad

[PATCH] D104424: [Sema] Fix for PR50741

[Chris Hamilton 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 rGfc6ec9b98cf9: [Sema] Fix for PR50741 (authored by 
chrish_ericsson_atx).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104424

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/unbounded-array-bounds.c

Index: clang/test/Sema/unbounded-array-bounds.c
===
--- clang/test/Sema/unbounded-array-bounds.c
+++ clang/test/Sema/unbounded-array-bounds.c
@@ -1,9 +1,6 @@
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-X86-ADDR64 %s  \
-// RUN:  --implicit-check-not 'past the last possible element'
-// RUN: %clang_cc1 -triple i386-pc-linux-gnu   -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-I386-ADDR32 %s \
-// RUN:  --implicit-check-not 'past the last possible element'
-// RUN: %clang_cc1 -triple avr-pc-linux-gnu-fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-AVR-ADDR16 %s  \
-// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -Wno-unused -verify=addr64,expected %s
+// RUN: %clang_cc1 -triple i386-pc-linux-gnu   -fsyntax-only -Wno-unused -verify=addr32,expected %s
+// RUN: %clang_cc1 -triple avr-pc-linux-gnu-fsyntax-only -Wno-unused -verify=addr16,expected %s
 
 struct S {
   long long a;
@@ -12,61 +9,61 @@
   short d;
 };
 
-struct S s[];
+struct S s[]; // expected-warning {{tentative array definition}} expected-note {{declared here}} addr16-note {{declared here}}
 
 void f1() {
   ++s[3].a;
   ++s[7073650413200313099].b;
-  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: array index 7073650413200313099 refers past the last possible element for an array in 64-bit address space containing 256-bit (32-byte) elements (max possible 576460752303423488 elements)
-  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
-  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  // addr16-warning@-1 {{array index 7073650413200313099 refers past the last possible element for an array in 16-bit address space containing 160-bit (20-byte) elements (max possible 3276 elements)}}
+  // addr32-warning@-2 {{array index 7073650413200313099 refers past the last possible element for an array in 32-bit address space containing 192-bit (24-byte) elements (max possible 178956970 elements)}}
+  // addr64-warning@-3 {{array index 7073650413200313099 refers past the last possible element for an array in 64-bit address space containing 256-bit (32-byte) elements (max possible 576460752303423488 elements)}}
   ++s[7073650].c;
-  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  // addr16-warning@-1 {{array index 7073650 refers past the last possible element for an array in 16-bit address space containing 160-bit (20-byte) elements (max possible 3276 elements)}}
 }
 
-long long ll[];
+long long ll[]; // expected-warning {{tentative array definition}} expected-note {{declared here}} addr16-note {{declared here}} addr32-note {{declared here}}
 
 void f2() {
   ++ll[3];
   ++ll[2705843009213693952];
-  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 2305843009213693952 elements)
-  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
-  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+  // addr16-warning@-1 {{array index 2705843009213693952 refers past the last possible element for an array in 16-bit address space containing 64-bit (8-byte) elements (max possible 8192 elements)}}
+  // addr32-warning@-2 {{array index 2705843009213693952 refers past the last possible element for an array in 32-bit address space containing 64-bit (8-byte) elements (max possible 536870912 elements)}}
+  // addr64-warning@-3 {{array index 2705843009213693952 refers past the last possible element for an array in 64-bit address space containing 64-bit (8-byte) elements (max possible 2305843009213693952 elements)}}
   ++ll[847073650];
-  // CHECK-I386-ADDR32: :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
-  // CHECK-AVR-ADDR16:  :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+  // addr16-warning@-1 {{array index 847073650 refers past 

[PATCH] D104424: [Sema] Fix for PR50741

[Chris Hamilton via Phabricator via cfe-commits]

chrish_ericsson_atx added a comment.

Delivered as rGfc6ec9b98cf9 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104424

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

[PATCH] D104462: [clang][lex] Add minimizer option to pad the output to the input size

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, arphaman.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch adds a configuration point to the source minimizer to pad the output 
to the input size with whitespace. This is useful in Dxx.

Depends on D104459 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104462

Files:
  clang/include/clang/Lex/DependencyDirectivesSourceMinimizer.h
  clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
  clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp

Index: clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
===
--- clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
+++ clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
@@ -14,7 +14,15 @@
 using namespace clang;
 using namespace clang::minimize_source_to_dependency_directives;
 
-namespace clang {
+namespace {
+
+bool minimizeSourceToDependencyDirectives(
+StringRef Input, SmallVectorImpl &Out,
+SmallVectorImpl &Tokens) {
+  return minimizeSourceToDependencyDirectives(Input, Out, Tokens, nullptr,
+  SourceLocation(),
+  /*PadOutputToInputSize=*/true);
+}
 
 bool minimizeSourceToDependencyDirectives(StringRef Input,
   SmallVectorImpl &Out) {
@@ -22,22 +30,22 @@
   return minimizeSourceToDependencyDirectives(Input, Out, Tokens);
 }
 
-} // end namespace clang
-
-namespace {
+StringRef rtrim(SmallVectorImpl &Vec) {
+  return StringRef(Vec.data()).rtrim();
+}
 
 TEST(MinimizeSourceToDependencyDirectivesTest, Empty) {
   SmallVector Out;
   SmallVector Tokens;
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("", Out, Tokens));
-  EXPECT_TRUE(Out.empty());
+  EXPECT_EQ("", rtrim(Out));
   ASSERT_EQ(1u, Tokens.size());
   ASSERT_EQ(pp_eof, Tokens.back().K);
 
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectives("abc def\nxyz", Out, Tokens));
-  EXPECT_TRUE(Out.empty());
+  EXPECT_EQ("", rtrim(Out));
   ASSERT_EQ(1u, Tokens.size());
   ASSERT_EQ(pp_eof, Tokens.back().K);
 }
@@ -94,7 +102,7 @@
 
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectives("#define MACRO", Out, Tokens));
-  EXPECT_STREQ("#define MACRO", Out.data());
+  EXPECT_EQ("#define MACRO", rtrim(Out));
   ASSERT_EQ(2u, Tokens.size());
   ASSERT_EQ(pp_define, Tokens.front().K);
 }
@@ -104,56 +112,56 @@
 
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectives("#define MACRO\n\n\n", Out));
-  EXPECT_STREQ("#define MACRO\n", Out.data());
+  EXPECT_EQ("#define MACRO", rtrim(Out));
 
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectives("#define MACRO \n\n\n", Out));
-  EXPECT_STREQ("#define MACRO\n", Out.data());
+  EXPECT_EQ("#define MACRO", rtrim(Out));
 
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectives("#define MACRO a \n\n\n", Out));
-  EXPECT_STREQ("#define MACRO a\n", Out.data());
+  EXPECT_EQ("#define MACRO a", rtrim(Out));
 
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectives("#define   MACRO\n\n\n", Out));
-  EXPECT_STREQ("#define MACRO\n", Out.data());
+  EXPECT_EQ("#define MACRO", rtrim(Out));
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, DefineMacroArguments) {
   SmallVector Out;
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define MACRO()", Out));
-  EXPECT_STREQ("#define MACRO()", Out.data());
+  EXPECT_EQ("#define MACRO()", rtrim(Out));
 
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectives("#define MACRO(a, b...)", Out));
-  EXPECT_STREQ("#define MACRO(a,b...)\n", Out.data());
+  EXPECT_EQ("#define MACRO(a,b...)", rtrim(Out));
 
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectives("#define MACRO content", Out));
-  EXPECT_STREQ("#define MACRO content", Out.data());
+  EXPECT_EQ("#define MACRO content", rtrim(Out));
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives(
   "#define MACRO   con  tent   ", Out));
-  EXPECT_STREQ("#define MACRO con  tent\n", Out.data());
+  EXPECT_EQ("#define MACRO con  tent", rtrim(Out));
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives(
   "#define MACRO()   con  tent   ", Out));
-  EXPECT_STREQ("#define MACRO() con  tent\n", Out.data());
+  EXPECT_EQ("#define MACRO() con  tent", rtrim(Out));
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, DefineInvalidMacroArguments) {
   SmallVector Out;
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define MACRO((a))", Out));
-  EXPECT_STREQ("#define MACRO\n", Out.data());
+  EXPECT_EQ("#define MACRO", rtrim(Out));
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define MACRO(", Out));
-  EXPECT_STREQ("#define MACRO\n", Out.data());
+  EXPECT_EQ("#define MACRO", rtrim(Out));
 
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectiv

[PATCH] D104460: [clang][lex] NFC: Extract source variable in minimizer tests

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 abandoned this revision.
jansvoboda11 added a comment.

Found a way to do the thing in a follow up with less churn. Sorry for the noise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104460

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

[clang-tools-extra] 6765b9c - [clangd] Explicitly fail if the file passed to --check is not valid.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2021-06-17T16:41:06+02:00
New Revision: 6765b9c3f1192bd63bdcafc92ee8ff5d7b97d90b

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

LOG: [clangd] Explicitly fail if the file passed to --check is not valid.

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

Added: 


Modified: 
clang-tools-extra/clangd/tool/ClangdMain.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp 
b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index 0f9e5c89e42c8..6d70a9cf03f6e 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -909,7 +909,11 @@ clangd accepts flags on the commandline, and in the 
CLANGD_FLAGS environment var
 
   if (CheckFile.getNumOccurrences()) {
 llvm::SmallString<256> Path;
-llvm::sys::fs::real_path(CheckFile, Path, /*expand_tilde=*/true);
+if (auto Error =
+llvm::sys::fs::real_path(CheckFile, Path, /*expand_tilde=*/true)) {
+  elog("Failed to resolve path {0}: {1}", CheckFile, Error.message());
+  return 1;
+}
 log("Entering check mode (no LSP server)");
 uint32_t Begin = 0, End = std::numeric_limits::max();
 if (!CheckFileLines.empty()) {



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

[PATCH] D104455: [clangd] Explicitly fail if the file passed to --check is not valid.

[Haojian Wu 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 rG6765b9c3f119: [clangd] Explicitly fail if the file passed to 
--check is not valid. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104455

Files:
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -909,7 +909,11 @@
 
   if (CheckFile.getNumOccurrences()) {
 llvm::SmallString<256> Path;
-llvm::sys::fs::real_path(CheckFile, Path, /*expand_tilde=*/true);
+if (auto Error =
+llvm::sys::fs::real_path(CheckFile, Path, /*expand_tilde=*/true)) {
+  elog("Failed to resolve path {0}: {1}", CheckFile, Error.message());
+  return 1;
+}
 log("Entering check mode (no LSP server)");
 uint32_t Begin = 0, End = std::numeric_limits::max();
 if (!CheckFileLines.empty()) {


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -909,7 +909,11 @@
 
   if (CheckFile.getNumOccurrences()) {
 llvm::SmallString<256> Path;
-llvm::sys::fs::real_path(CheckFile, Path, /*expand_tilde=*/true);
+if (auto Error =
+llvm::sys::fs::real_path(CheckFile, Path, /*expand_tilde=*/true)) {
+  elog("Failed to resolve path {0}: {1}", CheckFile, Error.message());
+  return 1;
+}
 log("Entering check mode (no LSP server)");
 uint32_t Begin = 0, End = std::numeric_limits::max();
 if (!CheckFileLines.empty()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103191: [OpenCL] Add support of __opencl_c_program_scope_global_variables feature macro

[Anton Zabaznov via Phabricator via cfe-commits]

azabaznov updated this revision to Diff 352723.
azabaznov added a comment.

Restructured test, added comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103191

Files:
  clang/include/clang/Basic/OpenCLOptions.h
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGenOpenCL/addr-space-struct-arg.cl
  clang/test/SemaOpenCL/storageclass.cl

Index: clang/test/SemaOpenCL/storageclass.cl
===
--- clang/test/SemaOpenCL/storageclass.cl
+++ clang/test/SemaOpenCL/storageclass.cl
@@ -1,28 +1,91 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL1.2
-
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL3.0 -cl-ext=-__opencl_c_program_scope_global_variables -DNO_GENERIC_AS
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL3.0 -cl-ext=+__opencl_c_program_scope_global_variables -DNO_GENERIC_AS
 static constant int G1 = 0;
 constant int G2 = 0;
-int G3 = 0;// expected-error{{program scope variable must reside in constant address space}}
-global int G4 = 0; // expected-error{{program scope variable must reside in constant address space}}
 
-static float g_implicit_static_var = 0; // expected-error {{program scope variable must reside in constant address space}}
+int G3 = 0;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{program scope variable must reside in constant address space}}
+#endif
+
+global int G4 = 0;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{program scope variable must reside in constant address space}}
+#endif
+
+static float g_implicit_static_var = 0;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{program scope variable must reside in constant address space}}
+#endif
+
 static constant float g_constant_static_var = 0;
-static global float g_global_static_var = 0;   // expected-error {{program scope variable must reside in constant address space}}
-static local float g_local_static_var = 0; // expected-error {{program scope variable must reside in constant address space}}
-static private float g_private_static_var = 0; // expected-error {{program scope variable must reside in constant address space}}
-static generic float g_generic_static_var = 0; // expected-error{{OpenCL C version 1.2 does not support the 'generic' type qualifier}} // expected-error {{program scope variable must reside in constant address space}}
 
-extern float g_implicit_extern_var; // expected-error {{extern variable must reside in constant address space}}
+static global float g_global_static_var = 0;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{program scope variable must reside in constant address space}}
+#endif
+
+static local float g_local_static_var = 0;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{program scope variable must reside in constant address space}}
+#else
+// expected-error@-4 {{program scope variable must reside in global or constant address space}}
+#endif
+
+static private float g_private_static_var = 0;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{program scope variable must reside in constant address space}}
+#else
+// expected-error@-4 {{program scope variable must reside in global or constant address space}}
+#endif
+
+static generic float g_generic_static_var = 0; // expected-error-re {{OpenCL C version {{1.2|3.0}} does not support the 'generic' type qualifier}}
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{program scope variable must reside in constant address space}}
+#endif
+
+extern float g_implicit_extern_var;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{extern variable must reside in constant address space}}
+#endif
+
 extern constant float g_constant_extern_var;
-extern global float g_global_extern_var;   // expected-error {{extern variable must reside in constant address space}}
-extern local float g_local_extern_var; // expected-error {{extern variable must reside in constant address space}}
-extern private float g_private_extern_var; // expected-error {{extern variable must reside in constant address space}}
-extern generic float g_generic_extern_var; // expected-error{{OpenCL C version 1.2 does not support the 'generic' type qualifier}} // expected-error {{extern variable must reside in constant address space}}
+
+extern global float g_global_extern_var;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{extern variable must reside in constant address space}}
+#endif
+
+extern local float g_local_extern_var;
+#ifndef __opencl_c_program_scope_global_variables
+// expected-error@-2 {{extern variable must reside in constant address space}}
+#else
+// expected-error@-4 {{extern variable must reside in global or constant address space}}
+#endif

[PATCH] D104465: [clang][deps] Prevent PCH validation failures by padding minimized files

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, arphaman.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch fixes dependency scanning of a TU with prebuilt modular/PCH 
dependencies.

Such modules/PCH might have been built using non-minimized files, while 
dependency scanner does use the minimized versions of source files. Implicit 
build doesn't like the discrepancy in file sizes and errors out.

The issues is worked around by padding the minimized files with whitespace in 
such scenarios. This approach throws away one advantage of source minimization 
(the memory savings), but still keeps the benefits of faster 
preprocessing/lexing.

The padding solution comes with a 14% runtime regression compared to not 
padding the inputs. (I tested this by forcing padding and running 
`clang-scan-deps -j 20` on LLVM's `compile_commands.json` with modules 
disabled.) Interestingly, padding only with newlines (compared to spaces), the 
regression is 39%.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104465

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/test/ClangScanDeps/modules-pch.c

Index: clang/test/ClangScanDeps/modules-pch.c
===
--- clang/test/ClangScanDeps/modules-pch.c
+++ clang/test/ClangScanDeps/modules-pch.c
@@ -6,7 +6,7 @@
 // RUN: sed "s|DIR|%/t|g" %S/Inputs/modules-pch/cdb_pch.json > %t/cdb.json
 // RUN: echo -%t > %t/result_pch.json
 // RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full \
-// RUN:   -generate-modules-path-args -module-files-dir %t/build -mode preprocess >> %t/result_pch.json
+// RUN:   -generate-modules-path-args -module-files-dir %t/build >> %t/result_pch.json
 // RUN: cat %t/result_pch.json | sed 's:\?:/:g' | FileCheck %s -check-prefix=CHECK-PCH
 //
 // Check we didn't build the PCH during dependency scanning.
@@ -127,9 +127,8 @@
 //
 // RUN: sed "s|DIR|%/t|g" %S/Inputs/modules-pch/cdb_tu.json > %t/cdb.json
 // RUN: echo -%t > %t/result_tu.json
-// FIXME: Make this work with '-mode preprocess-minimized-sources'.
 // RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full \
-// RUN:   -generate-modules-path-args -module-files-dir %t/build -mode preprocess >> %t/result_tu.json
+// RUN:   -generate-modules-path-args -module-files-dir %t/build >> %t/result_tu.json
 // RUN: cat %t/result_tu.json | sed 's:\?:/:g' | FileCheck %s -check-prefix=CHECK-TU
 //
 // CHECK-TU:  -[[PREFIX:.*]]
@@ -193,7 +192,7 @@
 // RUN: sed "s|DIR|%/t|g" %S/Inputs/modules-pch/cdb_tu_with_common.json > %t/cdb.json
 // RUN: echo -%t > %t/result_tu_with_common.json
 // RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full \
-// RUN:   -generate-modules-path-args -module-files-dir %t/build -mode preprocess >> %t/result_tu_with_common.json
+// RUN:   -generate-modules-path-args -module-files-dir %t/build >> %t/result_tu_with_common.json
 // RUN: cat %t/result_tu_with_common.json | sed 's:\?:/:g' | FileCheck %s -check-prefix=CHECK-TU-WITH-COMMON
 //
 // CHECK-TU-WITH-COMMON:  -[[PREFIX:.*]]
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -139,6 +139,15 @@
   FileMgr->setVirtualFileSystem(createVFSFromCompilerInvocation(
   CI, Compiler.getDiagnostics(), DepFS));
 
+  // If we are about to import a PCH, make sure minimized files are padded
+  // to sizes of the originals. The PCH might have been built from
+  // non-minimized files and any disagreement on the file size causes issues
+  // in the implicit build.
+  // TODO: Consider padding only files that contributed to the PCH and its
+  // modules.
+  DepFS->PadMinimizedToOriginalSize =
+  !Compiler.getPreprocessorOpts().ImplicitPCHInclude.empty();
+
   // Pass the skip mappings which should speed up excluded conditional block
   // skipping in the preprocessor.
   if (PPSkipMappings)
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -15,8 +15,10 @@
 using namespace tooling;
 using namespace dependencies;
 
-CachedFileSystemEntry CachedFileSystemEntry::createFileEntry(
-StringRef Filename, llvm::vfs::FileSystem &FS, bool Minimize) {
+CachedFileSy

[PATCH] D104459: [clang][lex] Ensure minimizer output is never larger than input

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 352726.
jansvoboda11 added a comment.

Fix assert wording


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104459

Files:
  clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
  clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp

Index: clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
===
--- clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
+++ clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
@@ -94,7 +94,7 @@
 
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectives("#define MACRO", Out, Tokens));
-  EXPECT_STREQ("#define MACRO\n", Out.data());
+  EXPECT_STREQ("#define MACRO", Out.data());
   ASSERT_EQ(2u, Tokens.size());
   ASSERT_EQ(pp_define, Tokens.front().K);
 }
@@ -123,7 +123,7 @@
   SmallVector Out;
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define MACRO()", Out));
-  EXPECT_STREQ("#define MACRO()\n", Out.data());
+  EXPECT_STREQ("#define MACRO()", Out.data());
 
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectives("#define MACRO(a, b...)", Out));
@@ -131,7 +131,7 @@
 
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectives("#define MACRO content", Out));
-  EXPECT_STREQ("#define MACRO content\n", Out.data());
+  EXPECT_STREQ("#define MACRO content", Out.data());
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives(
   "#define MACRO   con  tent   ", Out));
@@ -146,14 +146,14 @@
   SmallVector Out;
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define MACRO((a))", Out));
-  EXPECT_STREQ("#define MACRO(/* invalid */\n", Out.data());
+  EXPECT_STREQ("#define MACRO\n", Out.data());
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define MACRO(", Out));
-  EXPECT_STREQ("#define MACRO(/* invalid */\n", Out.data());
+  EXPECT_STREQ("#define MACRO\n", Out.data());
 
   ASSERT_FALSE(
   minimizeSourceToDependencyDirectives("#define MACRO(a * b)", Out));
-  EXPECT_STREQ("#define MACRO(/* invalid */\n", Out.data());
+  EXPECT_STREQ("#define MACRO\n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, DefineHorizontalWhitespace) {
@@ -259,7 +259,7 @@
   SmallVector Out;
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define AND&\n", Out));
-  EXPECT_STREQ("#define AND &\n", Out.data());
+  EXPECT_STREQ("#define AND&\n", Out.data());
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define AND\\\n"
 "&\n",
Index: clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
===
--- clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
+++ clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
@@ -93,6 +93,7 @@
   void printDirectiveBody(const char *&First, const char *const End);
   void printAdjacentMacroArgs(const char *&First, const char *const End);
   LLVM_NODISCARD bool printMacroArgs(const char *&First, const char *const End);
+  void maybePrintNewLine(const char *&Last, const char *const End);
 
   /// Reports a diagnostic if the diagnostic engine is provided. Always returns
   /// true at the end.
@@ -446,13 +447,15 @@
   }
 }
 
-static void skipWhitespace(const char *&First, const char *const End) {
+static size_t skipWhitespace(const char *&First, const char *const End) {
+  const char *FirstBefore = First;
+
   for (;;) {
 assert(First <= End);
 skipOverSpaces(First, End);
 
 if (End - First < 2)
-  return;
+  break;
 
 if (First[0] == '\\' && isVerticalWhitespace(First[1])) {
   skipNewline(++First, End);
@@ -461,21 +464,23 @@
 
 // Check for a non-comment character.
 if (First[0] != '/')
-  return;
+  break;
 
 // "// ...".
 if (First[1] == '/') {
   skipLineComment(First, End);
-  return;
+  break;
 }
 
 // Cannot be a comment.
 if (First[1] != '*')
-  return;
+  break;
 
 // "/*...*/".
 skipBlockComment(First, End);
   }
+
+  return First - FirstBefore;
 }
 
 void Minimizer::printAdjacentModuleNameParts(const char *&First,
@@ -519,7 +524,7 @@
   printToNewline(First, End);
   while (Out.back() == ' ')
 Out.pop_back();
-  put('\n');
+  maybePrintNewLine(First, End);
 }
 
 LLVM_NODISCARD static const char *lexRawIdentifier(const char *First,
@@ -595,6 +600,12 @@
   }
 }
 
+void Minimizer::maybePrintNewLine(const char *&Last, const char *const End) {
+  // Only print newline if doing so won't make the output larger than the input.
+  if (Last != End)
+put('\n');
+}
+
 /// Looks for an identifier starting from Last.
 ///
 /// Updates "First" to just past the next identifier, if any.  Returns true iff
@@ -704,15 +715,17 @@
   // Be robust to bad macro arguments, since they can show up in disabled
   // code.
   Out.resize(Size);
-  append("(/* 

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

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

I don't have any comments regarding header maps.




Comment at: clang/test/Modules/implicit-module-header-maps.cpp:17
+//
+// RUN: sed -e "s:OUTPUTS_DIR:%t:g" 
%S/Inputs/implicit-module-header-maps/b.hmap.json > %t/hmap.json
+// RUN: %hmaptool write %t/hmap.json %t/hmap

The Windows CI is failing here, you should use a separator in `sed` that can't 
appear in `%t`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930

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

[PATCH] D103929: [clang] P2085R0: Consistent defaulted comparisons

[Alexandru Octavian Buțiu via Phabricator via cfe-commits]

predator5047 added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103929

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

[PATCH] D103962: [C++4OpenCL] Fix qualifiers check on binding references to temporaries

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Yes it seems the only situation where the check would evaluate to a 
different value with and without your patch is when we have `RefRelationship == 
Sema::Ref_Related` as false while the address space is not a superset then we 
would get an incorrect diagnostic. But I feel this would be diagnosed elsewhere 
before we end up in this check.

FYI I was trying the following test case:

  void square(int num) {
private int i = 0;
private const float &ref = ++i;
  }

but it also fails with your patch which makes me feel there is a bug elsewhere 
too:

`error: reference of type 'const __private float &__private' cannot bind to a 
temporary object because of address space mismatch`

However I do believe that the fix makes sense because that particular check was 
intended for when `RefRelationship == Sema::Ref_Related` evaluates to true.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103962

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

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

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg updated this revision to Diff 352740.
andrewjcg added a comment.

fix sed for windows test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930

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

Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,29 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: sed -e "s|OUTPUTS_DIR|%t|g" %S/Inputs/implicit-module-header-maps/b.hmap.json > %t/hmap.json
+// RUN: %hmaptool write %t/hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+
+#define FOO
+// This include will fail if modules weren't used.  The include name itself
+// doesn't exist and relies on the header map to remap it to the real header.
+#include "Before/Mapping.h"
Index: clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
@@ -0,0 +1,6 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "OUTPUTS_DIR/After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
@@ -0,0 +1,3 @@
+module a {
+  header "After/Mapping.h"
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
@@ -0,0 +1,7 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "After/Mapping.h",
+ "After/Mapping.h" : "After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.h
@@ -0,0 +1,3 @@
+#ifdef FOO
+#error foo
+#endif
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -417,7 +417,8 @@
 
   IsInHeaderMap = true;
 
-  auto FixupSearchPath = [&]() {
+  auto FixupSearchPathAndFindUsableModule =
+  [&](auto file) -> Optional {
 if (SearchPath) {
   StringRef SearchPathRef(getName());
   SearchPath->clear();
@@ -427,6 +428,12 @@
   RelativePath->clear();
   RelativePath->append(Filename.begin(), Filename.end());
 }
+if (!HS.findUsableModuleForHeader(
+&file.getFileEntry(), file.getFileEntry().getDir(),
+RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+  return None;
+}
+return file;
   };
 
   // Check if the headermap maps the filename to a framework include
@@ -437,12 +444,10 @@
 Filename = StringRef(MappedName.begin(), MappedName.size());
 Optional Result = HM->LookupFile(Filename, HS.getFileMgr());
 if (Result) {
-  FixupSearchPath();
-  return *Result;
+  return FixupSearchPathAndFindUsableModule(*Result);
 }
   } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
-FixupSearchPath();
-return *Res;
+return FixupSearchPathAndFindUsableModule(*Res);
   }
 
   return None;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93528: [clang-format] Add basic support for formatting JSON

[MyDeveloperDay via Phabricator via cfe-commits]

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

I have reworked this patch to utilise the fact that clang-format can format a 
JSON object in javascript reasonably well.

This technique adds for JSON files only a replacement to the front of the JSON 
transforming:

  {
  "A": 1,
  "B": [ 2,3 ]
  }

into

  x = {
  "A": 1,
  "B": [ 2,3 ]
  }

This is then parsed by clang-format the a more traditional sense, allowing a 
similar technique as would be used when formatting the code, (but with some 
JSON specific rules, to make it follow primarily the style used by 
https://github.com/kapilratnani/JSON-Viewer (Notepad++ plugin, which at least 
I've used extensively)

After formatter, a subsequent replacement is merged to remove the x = from the 
front of the json, rendering it back to its original form.

  {
  "A": 1,
  "B": [ 2,3 ]
  }

I prefer to use this approach rather than our Support/JSON libraries which 
aren't really setup for formatting options, this give us greater flexibility 
later on if users want custom rules from brace wrapping.


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

https://reviews.llvm.org/D93528

Files:
  clang/docs/ClangFormat.rst
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/CMakeLists.txt

Index: clang/unittests/Format/CMakeLists.txt
===
--- clang/unittests/Format/CMakeLists.txt
+++ clang/unittests/Format/CMakeLists.txt
@@ -9,6 +9,7 @@
   FormatTestCSharp.cpp
   FormatTestJS.cpp
   FormatTestJava.cpp
+  FormatTestJson.cpp
   FormatTestObjC.cpp
   FormatTestProto.cpp
   FormatTestRawStrings.cpp
Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -411,6 +411,17 @@
   unsigned CursorPosition = Cursor;
   Replacements Replaces = sortIncludes(*FormatStyle, Code->getBuffer(), Ranges,
AssumedFileName, &CursorPosition);
+
+  // To format JSON insert a variable to trick the code into thinking its
+  // JavaScript.
+  if (FormatStyle->isJson()) {
+auto Err = Replaces.add(tooling::Replacement(
+tooling::Replacement(AssumedFileName, 0, 0, "x = ")));
+if (Err) {
+  llvm::errs() << "Bad Json variable insertion\n";
+}
+  }
+
   auto ChangedCode = tooling::applyAllReplacements(Code->getBuffer(), Replaces);
   if (!ChangedCode) {
 llvm::errs() << llvm::toString(ChangedCode.takeError()) << "\n";
@@ -506,7 +517,8 @@
   cl::SetVersionPrinter(PrintVersion);
   cl::ParseCommandLineOptions(
   argc, argv,
-  "A tool to format C/C++/Java/JavaScript/Objective-C/Protobuf/C# code.\n\n"
+  "A tool to format C/C++/Java/JavaScript/JSON/Objective-C/Protobuf/C# "
+  "code.\n\n"
   "If no arguments are specified, it formats the code from standard input\n"
   "and writes the result to the standard output.\n"
   "If s are given, it reformats the files. If -i is specified\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2900,6 +2900,8 @@
   const FormatToken &Right) {
   if (Left.is(tok::kw_return) && Right.isNot(tok::semi))
 return true;
+  if (Style.isJson() && Left.is(tok::string_literal) && Right.is(tok::colon))
+return false;
   if (Left.is(Keywords.kw_assert) && Style.Language == FormatStyle::LK_Java)
 return true;
   if (Style.ObjCSpaceAfterProperty && Line.Type == LT_ObjCProperty &&
@@ -3221,6 +3223,9 @@
 // and "%d %d"
 if (Left.is(tok::numeric_constant) && Right.is(tok::percent))
   return HasExistingWhitespace();
+  } else if (Style.isJson()) {
+if (Right.is(tok::colon))
+  return false;
   } else if (Style.isCSharp()) {
 // Require spaces around '{' and  before '}' unless they appear in
 // interpolated strings. Interpolated strings are merged into a single token
@@ -3695,6 +3700,26 @@
   return true;
   }
 
+  // Basic JSON newline processing.
+  if (Style.isJson()) {
+// Always break after a JSON record opener.
+// {
+// }
+if (Left.is(TT_DictLiteral) && Left.is(tok::l_brace))
+  return true;
+// Always break after a JSON array opener.
+// [
+// ]
+if (Left.is(TT_ArrayInitializerLSquare) && Left.is(tok::l_square) &&
+!Right.is(tok::r_square))
+  return true;
+// Always break afer successive entries.
+// 1,
+// 2
+if (Left.is(tok::comma))
+  return true;
+  }
+
   // If the last token before a '}', ']', or ')' is a comma or a trailing
   // comment, th

[PATCH] D93528: [clang-format] Add basic support for formatting JSON

[MyDeveloperDay via Phabricator via cfe-commits]

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

Missing new test file


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

https://reviews.llvm.org/D93528

Files:
  clang/docs/ClangFormat.rst
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/FormatTestJson.cpp

Index: clang/unittests/Format/FormatTestJson.cpp
===
--- /dev/null
+++ clang/unittests/Format/FormatTestJson.cpp
@@ -0,0 +1,130 @@
+//===- unittest/Format/FormatTestJson.cpp - Formatting tests for Json -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "FormatTestUtils.h"
+#include "clang/Format/Format.h"
+#include "llvm/Support/Debug.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "format-test-json"
+
+namespace clang {
+namespace format {
+
+class FormatTestJson : public ::testing::Test {
+protected:
+  static std::string format(llvm::StringRef Code, unsigned Offset,
+unsigned Length, const FormatStyle &Style) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+
+tooling::Replacements Replaces;
+
+// Mock up what ClangFormat.cpp will do for JSON by adding a variable
+// to trick JSON into being JavaScript
+if (Style.isJson()) {
+  auto Err = Replaces.add(
+  tooling::Replacement(tooling::Replacement("", 0, 0, "x = ")));
+  if (Err) {
+llvm::errs() << "Bad Json variable insertion\n";
+  }
+}
+auto ChangedCode = applyAllReplacements(Code, Replaces);
+if (!ChangedCode) {
+  llvm::errs() << "Bad Json varibale replacement\n";
+}
+StringRef NewCode = *ChangedCode;
+
+std::vector Ranges(1, tooling::Range(0, NewCode.size()));
+Replaces = reformat(Style, NewCode, Ranges);
+auto Result = applyAllReplacements(NewCode, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  static std::string
+  format(llvm::StringRef Code,
+ const FormatStyle &Style = getLLVMStyle(FormatStyle::LK_Json)) {
+return format(Code, 0, Code.size(), Style);
+  }
+
+  static FormatStyle getStyleWithColumns(unsigned ColumnLimit) {
+FormatStyle Style = getLLVMStyle(FormatStyle::LK_Json);
+Style.ColumnLimit = ColumnLimit;
+return Style;
+  }
+
+  static void
+  verifyFormat(llvm::StringRef Code,
+   const FormatStyle &Style = getLLVMStyle(FormatStyle::LK_Json)) {
+EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
+EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
+  }
+};
+
+TEST_F(FormatTestJson, JsonRecord) {
+  verifyFormat("{}");
+  verifyFormat("{\n"
+   "  \"name\": 1\n"
+   "}");
+  verifyFormat("{\n"
+   "  \"name\": \"Foo\"\n"
+   "}");
+}
+
+TEST_F(FormatTestJson, JsonArray) {
+  verifyFormat("[]");
+  verifyFormat("[\n"
+   "  1\n"
+   "]");
+  verifyFormat("[\n"
+   "  1,\n"
+   "  2\n"
+   "]");
+  verifyFormat("[\n"
+   "  {},\n"
+   "  {}\n"
+   "]");
+  verifyFormat("[\n"
+   "  {\n"
+   "\"name\": 1\n"
+   "  },\n"
+   "  {}\n"
+   "]");
+}
+
+TEST_F(FormatTestJson, JsonIndent) {
+  FormatStyle Style = getLLVMStyle(FormatStyle::LK_Json);
+  Style.IndentWidth = 4;
+  verifyFormat("[]", Style);
+  verifyFormat("[\n"
+   "10\n"
+   "]",
+   Style);
+  verifyFormat("[\n"
+   "1,\n"
+   "2\n"
+   "]",
+   Style);
+  verifyFormat("[\n"
+   "{},\n"
+   "{}\n"
+   "]",
+   Style);
+  verifyFormat("[\n"
+   "{\n"
+   "\"name\": 1\n"
+   "},\n"
+   "{}\n"
+   "]",
+   Style);
+}
+
+} // namespace format
+} // end namespace clang
Index: clang/unittests/Format/CMakeLists.txt
===
--- clang/unittests/Format/CMakeLists.txt
+++ clang/unittests/Format/CMakeLists.txt
@@ -9,6 +9,7 @@
   FormatTestCSharp.cpp
   FormatTestJS.cpp
   FormatTestJava.cpp
+  FormatTestJson.cpp
   FormatTestObjC.cpp
   FormatTestProto.cpp
   FormatTestRawStrings.cpp
Index: clang/tools/clang-format/ClangFormat.cpp
==

[PATCH] D104058: ThinLTO: Fix inline assembly references to static functions with CFI

[Peter Collingbourne via Phabricator via cfe-commits]

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

LGTM




Comment at: clang/test/CodeGen/thinlto-cfi-icall-static-inline-asm.c:3
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm-bc -flto=thin 
-fsanitize=cfi-icall -fsplit-lto-unit -o - %s | llvm-dis - | FileCheck %s
+

Can the test be moved to `llvm/test/Transforms/ThinLTOBitcodeWriter` and made 
to use `opt -thinlto-bc`?



Comment at: llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp:79
+  GlobalValue::InternalLinkage, Name, F, &ExportM);
+  appendToCompilerUsed(ExportM, A);
+}

samitolvanen wrote:
> Note that I'm adding the alias to llvm.compiler.used because it's otherwise 
> removed during optimization. Is there are better way to accomplish this?
Not as far as I know, that's what I'd recommend.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104058

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

[PATCH] D104096: [Clang-Format] Add ReferenceAlignment directive

[Seraphime Kirkovski (VMware) via Phabricator via cfe-commits]

skirkovski updated this revision to Diff 352753.
skirkovski added a comment.

Rebase and fix warning


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104096

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1230,6 +1230,98 @@
getLLVMStyleWithColumns(62));
 }
 
+TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
+  FormatStyle Style = getLLVMStyle();
+  // Check first the default LLVM style
+  // Style.PointerAlignment = FormatStyle::PAS_Right;
+  // Style.ReferenceAlignment = FormatStyle::RAS_Pointer;
+  verifyFormat("int *f1(int *a, int &b, int &&c);", Style);
+  verifyFormat("int &f2(int &&c, int *a, int &b);", Style);
+  verifyFormat("int &&f3(int &b, int &&c, int *a);", Style);
+  verifyFormat("int *f1(int &a) const &;", Style);
+  verifyFormat("int *f1(int &a) const & = 0;", Style);
+  verifyFormat("int *a = f1();", Style);
+  verifyFormat("int &b = f2();", Style);
+  verifyFormat("int &&c = f3();", Style);
+
+  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  verifyFormat("Const unsigned int *c;\n"
+   "const unsigned int *d;\n"
+   "Const unsigned int &e;\n"
+   "const unsigned int &f;\n"
+   "const unsigned&&g;\n"
+   "Const unsigned  h;",
+   Style);
+
+  Style.PointerAlignment = FormatStyle::PAS_Left;
+  Style.ReferenceAlignment = FormatStyle::RAS_Pointer;
+  verifyFormat("int* f1(int* a, int& b, int&& c);", Style);
+  verifyFormat("int& f2(int&& c, int* a, int& b);", Style);
+  verifyFormat("int&& f3(int& b, int&& c, int* a);", Style);
+  verifyFormat("int* f1(int& a) const& = 0;", Style);
+  verifyFormat("int* a = f1();", Style);
+  verifyFormat("int& b = f2();", Style);
+  verifyFormat("int&& c = f3();", Style);
+
+  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  verifyFormat("Const unsigned int* c;\n"
+   "const unsigned int* d;\n"
+   "Const unsigned int& e;\n"
+   "const unsigned int& f;\n"
+   "const unsigned&&g;\n"
+   "Const unsigned  h;",
+   Style);
+
+  Style.PointerAlignment = FormatStyle::PAS_Right;
+  Style.ReferenceAlignment = FormatStyle::RAS_Left;
+  verifyFormat("int *f1(int *a, int& b, int&& c);", Style);
+  verifyFormat("int& f2(int&& c, int *a, int& b);", Style);
+  verifyFormat("int&& f3(int& b, int&& c, int *a);", Style);
+  verifyFormat("int *a = f1();", Style);
+  verifyFormat("int& b = f2();", Style);
+  verifyFormat("int&& c = f3();", Style);
+
+  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  verifyFormat("Const unsigned int *c;\n"
+   "const unsigned int *d;\n"
+   "Const unsigned int& e;\n"
+   "const unsigned int& f;\n"
+   "const unsigned  g;\n"
+   "Const unsigned  h;",
+   Style);
+
+  Style.PointerAlignment = FormatStyle::PAS_Left;
+  Style.ReferenceAlignment = FormatStyle::RAS_Middle;
+  verifyFormat("int* f1(int* a, int & b, int && c);", Style);
+  verifyFormat("int & f2(int && c, int* a, int & b);", Style);
+  verifyFormat("int && f3(int & b, int && c, int* a);", Style);
+  verifyFormat("int* a = f1();", Style);
+  verifyFormat("int & b = f2();", Style);
+  verifyFormat("int && c = f3();", Style);
+
+  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  verifyFormat("Const unsigned int*  c;\n"
+   "const unsigned int*  d;\n"
+   "Const unsigned int & e;\n"
+   "const unsigned int & f;\n"
+   "const unsigned &&g;\n"
+   "Const unsigned   h;",
+   Style);
+
+  Style.PointerAlignment = FormatStyle::PAS_Middle;
+  Style.ReferenceAlignment = FormatStyle::RAS_Right;
+  verifyFormat("int * f1(int * a, int &b, int &&c);", Style);
+  verifyFormat("int &f2(int &&c, int * a, int &b);", Style);
+  verifyFormat("int &&f3(int &b, int &&c, int * a);", Style);
+  verifyFormat("int * a = f1();", Style);
+  verifyFormat("int &b = f2();", Style);
+  verifyFormat("int &&c = f3();", Style);
+
+  // FIXME: we don't handle this yet, so output may be arbitrary until it's
+  // specifically handled
+  // verifyFormat("int Add2(BTree * &Root, char * szToAdd)", Style);
+}
+
 TEST_F(FormatTest, FormatsForLoop) {
   verifyFormat(
   "for (int VeryVeryLongLoopVariable = 0; VeryVeryLongLoopVariable < 10;\n"
@@ -17454,6 +17546,15 @@
   FormatStyle::PAS_Right);
   CHECK_PARSE("PointerAlignment: Mid

[PATCH] D104376: [clangd] Correct SelectionTree behavior around anonymous field access.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/Selection.cpp:71
+  // so we reduce its range to match the CXXConstructExpr.
+  // (It's not clear that changing the clang AST would be correct in general).
+  if (const auto *ME = N.get()) {

hokein wrote:
> I still think it is worth to explore this direction (will take a look on this 
> if I have time), but also ok with the current approach. 
SG, this isn't urgent so I'll leave this open while out on vacation. I agree 
it's suspect/suggestive that we need to override the AST source range in 
exactly one case.

My concern was conceptual: in `A().b` if you want to describe where the anon 
field ref happens, I'd pick `.` or `b` so placing those outside the range is 
suspect. Conversely a range of `A()` seems weird because that's a perfectly 
valid expression with no dereferencing.

OTOH if you squint I guess this looks a bit like an implicit cast from the `A` 
to the anon member. So maybe it's fine and just a question of mental model.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104376

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

[PATCH] D104096: [Clang-Format] Add ReferenceAlignment directive

[Seraphime Kirkovski (VMware) via Phabricator via cfe-commits]

skirkovski marked 5 inline comments as done.
skirkovski added a comment.

Rebased and fixed warning. My local compiler was yelling at me for adding a 
default case to switch which already covers everything.

For the commit:

Author: Seraphime Kirkovski
email: skirkovski at vmware.com


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104096

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

[PATCH] D93528: [clang-format] Add basic support for formatting JSON

[MyDeveloperDay via Phabricator via cfe-commits]

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

JSON strings can't be split when the line is too long like c++ strings


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

https://reviews.llvm.org/D93528

Files:
  clang/docs/ClangFormat.rst
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/FormatTestJson.cpp

Index: clang/unittests/Format/FormatTestJson.cpp
===
--- /dev/null
+++ clang/unittests/Format/FormatTestJson.cpp
@@ -0,0 +1,117 @@
+//===- unittest/Format/FormatTestJson.cpp - Formatting tests for Json -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "FormatTestUtils.h"
+#include "clang/Format/Format.h"
+#include "llvm/Support/Debug.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "format-test-json"
+
+namespace clang {
+namespace format {
+
+class FormatTestJson : public ::testing::Test {
+protected:
+  static std::string format(llvm::StringRef Code, unsigned Offset,
+unsigned Length, const FormatStyle &Style) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+
+tooling::Replacements Replaces;
+
+// Mock up what ClangFormat.cpp will do for JSON by adding a variable
+// to trick JSON into being JavaScript
+if (Style.isJson()) {
+  auto Err = Replaces.add(
+  tooling::Replacement(tooling::Replacement("", 0, 0, "x = ")));
+  if (Err) {
+llvm::errs() << "Bad Json variable insertion\n";
+  }
+}
+auto ChangedCode = applyAllReplacements(Code, Replaces);
+if (!ChangedCode) {
+  llvm::errs() << "Bad Json varibale replacement\n";
+}
+StringRef NewCode = *ChangedCode;
+
+std::vector Ranges(1, tooling::Range(0, NewCode.size()));
+Replaces = reformat(Style, NewCode, Ranges);
+auto Result = applyAllReplacements(NewCode, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  static std::string
+  format(llvm::StringRef Code,
+ const FormatStyle &Style = getLLVMStyle(FormatStyle::LK_Json)) {
+return format(Code, 0, Code.size(), Style);
+  }
+
+  static FormatStyle getStyleWithColumns(unsigned ColumnLimit) {
+FormatStyle Style = getLLVMStyle(FormatStyle::LK_Json);
+Style.ColumnLimit = ColumnLimit;
+return Style;
+  }
+
+  static void
+  verifyFormat(llvm::StringRef Code,
+   const FormatStyle &Style = getLLVMStyle(FormatStyle::LK_Json)) {
+EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
+EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
+  }
+};
+
+TEST_F(FormatTestJson, JsonRecord) {
+  verifyFormat("{}");
+  verifyFormat("{\n"
+   "  \"name\": 1\n"
+   "}");
+  verifyFormat("{\n"
+   "  \"name\": \"Foo\"\n"
+   "}");
+}
+
+TEST_F(FormatTestJson, JsonArray) {
+  verifyFormat("[]");
+  verifyFormat("[\n"
+   "  1\n"
+   "]");
+  verifyFormat("[\n"
+   "  1,\n"
+   "  2\n"
+   "]");
+  verifyFormat("[\n"
+   "  {},\n"
+   "  {}\n"
+   "]");
+  verifyFormat("[\n"
+   "  {\n"
+   "\"name\": 1\n"
+   "  },\n"
+   "  {}\n"
+   "]");
+}
+
+TEST_F(FormatTestJson, JsonNoStringSplit) {
+  FormatStyle Style = getLLVMStyle(FormatStyle::LK_Json);
+  Style.IndentWidth = 4;
+  verifyFormat("[\n"
+   "{\n"
+   ""
+   "\"n\":"
+   " \"foo oo\"\n"
+   "},\n"
+   "{}\n"
+   "]",
+   Style);
+}
+
+} // namespace format
+} // end namespace clang
Index: clang/unittests/Format/CMakeLists.txt
===
--- clang/unittests/Format/CMakeLists.txt
+++ clang/unittests/Format/CMakeLists.txt
@@ -9,6 +9,7 @@
   FormatTestCSharp.cpp
   FormatTestJS.cpp
   FormatTestJava.cpp
+  FormatTestJson.cpp
   FormatTestObjC.cpp
   FormatTestProto.cpp
   FormatTestRawStrings.cpp
Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -411,6 +411,17 @@
 

[clang] 734d688 - [clang] Fix a race condition in the build of clangInterpreter

[Stella Stamenova via cfe-commits]

Author: Stella Stamenova
Date: 2021-06-17T10:03:33-07:00
New Revision: 734d688fbce8a453aa61764b9b5a43b26455dc0d

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

LOG: [clang] Fix a race condition in the build of clangInterpreter

The library depends on Attributes.inc, so it has to depend on the 
intrinsics_gen target

Reviewed By: v.g.vassilev

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

Added: 


Modified: 
clang/lib/Interpreter/CMakeLists.txt

Removed: 




diff  --git a/clang/lib/Interpreter/CMakeLists.txt 
b/clang/lib/Interpreter/CMakeLists.txt
index e08779945b5fc..88a0a716e1269 100644
--- a/clang/lib/Interpreter/CMakeLists.txt
+++ b/clang/lib/Interpreter/CMakeLists.txt
@@ -12,6 +12,9 @@ add_clang_library(clangInterpreter
   IncrementalParser.cpp
   Interpreter.cpp
 
+  DEPENDS
+  intrinsics_gen
+
   LINK_LIBS
   clangAST
   clangAnalysis



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

[PATCH] D104311: [clang] Fix a race condition in the build of clangInterpreter

[Stella Stamenova via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG734d688fbce8: [clang] Fix a race condition in the build of 
clangInterpreter (authored by stella.stamenova).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104311

Files:
  clang/lib/Interpreter/CMakeLists.txt


Index: clang/lib/Interpreter/CMakeLists.txt
===
--- clang/lib/Interpreter/CMakeLists.txt
+++ clang/lib/Interpreter/CMakeLists.txt
@@ -12,6 +12,9 @@
   IncrementalParser.cpp
   Interpreter.cpp
 
+  DEPENDS
+  intrinsics_gen
+
   LINK_LIBS
   clangAST
   clangAnalysis


Index: clang/lib/Interpreter/CMakeLists.txt
===
--- clang/lib/Interpreter/CMakeLists.txt
+++ clang/lib/Interpreter/CMakeLists.txt
@@ -12,6 +12,9 @@
   IncrementalParser.cpp
   Interpreter.cpp
 
+  DEPENDS
+  intrinsics_gen
+
   LINK_LIBS
   clangAST
   clangAnalysis
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101976: [OpenMP] Unified entry point for SPMD & generic kernels in the device RTL

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert updated this revision to Diff 352776.
jdoerfert added a comment.

Fix tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101976

Files:
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
  clang/test/OpenMP/amdgcn_target_codegen.cpp
  clang/test/OpenMP/assumes_include_nvptx.cpp
  clang/test/OpenMP/declare_target_codegen_globalization.cpp
  clang/test/OpenMP/nvptx_SPMD_codegen.cpp
  clang/test/OpenMP/nvptx_data_sharing.cpp
  clang/test/OpenMP/nvptx_distribute_parallel_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_force_full_runtime_SPMD_codegen.cpp
  clang/test/OpenMP/nvptx_lambda_capturing.cpp
  clang/test/OpenMP/nvptx_multi_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_nested_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_for_codegen.cpp
  clang/test/OpenMP/nvptx_target_codegen.cpp
  clang/test/OpenMP/nvptx_target_firstprivate_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_num_threads_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_proc_bind_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
  clang/test/OpenMP/nvptx_target_printf_codegen.c
  clang/test/OpenMP/nvptx_target_simd_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
  
clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_simd_codegen.cpp
  clang/test/OpenMP/nvptx_teams_codegen.cpp
  clang/test/OpenMP/nvptx_teams_reduction_codegen.cpp
  clang/test/OpenMP/remarks_parallel_in_multiple_target_state_machines.c
  clang/test/OpenMP/remarks_parallel_in_target_state_machine.c
  clang/test/OpenMP/target_parallel_debug_codegen.cpp
  clang/test/OpenMP/target_parallel_for_debug_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/test/Transforms/OpenMP/single_threaded_execution.ll
  openmp/libomptarget/deviceRTLs/common/include/target.h
  openmp/libomptarget/deviceRTLs/common/src/omptarget.cu
  openmp/libomptarget/deviceRTLs/common/src/parallel.cu
  openmp/libomptarget/deviceRTLs/interface.h
  openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.cu

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

[clang] 4e2aee8 - [AIX] Remove --as-needed passing into aix linker

[via cfe-commits]

Author: jasonliu
Date: 2021-06-17T17:16:41Z
New Revision: 4e2aee8d3bab6010420d9be96480f1d8ae0f35c1

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

LOG: [AIX] Remove --as-needed passing into aix linker

Summary:
AIX does not support --as-needed linker options. Remove that option from
aix linker when -lunwind is needed.
For unwinder library, nothing special is needed because by default aix
linker has the as-needed effect for library that's an archive (which is
the case for libunwind on AIX).

Reviewed By: daltenty

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/test/Driver/aix-ld.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp 
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 2c176b207bc1d..cfda0ff1852c3 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -738,6 +738,9 @@ static bool addSanitizerDynamicList(const ToolChain &TC, 
const ArgList &Args,
 }
 
 static const char *getAsNeededOption(const ToolChain &TC, bool as_needed) {
+  assert(!TC.getTriple().isOSAIX() &&
+ "AIX linker does not support any form of --as-needed option yet.");
+
   // While the Solaris 11.2 ld added --as-needed/--no-as-needed as aliases
   // for the native forms -z ignore/-z record, they are missing in Illumos,
   // so always use the native form.
@@ -1423,7 +1426,8 @@ static void AddUnwindLibrary(const ToolChain &TC, const 
Driver &D,
 
   LibGccType LGT = getLibGccType(TC, D, Args);
   bool AsNeeded = LGT == LibGccType::UnspecifiedLibGcc &&
-  !TC.getTriple().isAndroid() && !TC.getTriple().isOSCygMing();
+  !TC.getTriple().isAndroid() &&
+  !TC.getTriple().isOSCygMing() && !TC.getTriple().isOSAIX();
   if (AsNeeded)
 CmdArgs.push_back(getAsNeededOption(TC, true));
 

diff  --git a/clang/test/Driver/aix-ld.c b/clang/test/Driver/aix-ld.c
index 62f152fd0eb69..c66b235cb1e2c 100644
--- a/clang/test/Driver/aix-ld.c
+++ b/clang/test/Driver/aix-ld.c
@@ -20,7 +20,9 @@
 // CHECK-LD32-NOT: "-lc++"
 // CHECK-LD32-NOT: "-lc++abi"
 // CHECK-LD32: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-NOT: "--as-needed"
 // CHECK-LD32: "-lunwind"
+// CHECK-LD32-NOT: "--no-as-needed"
 // CHECK-LD32-NOT: "-lm"
 // CHECK-LD32: "-lc"
 
@@ -43,7 +45,9 @@
 // CHECK-LD64-NOT: "-lc++"
 // CHECK-LD64-NOT: "-lc++abi"
 // CHECK-LD64: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-NOT: "--as-needed"
 // CHECK-LD64: "-lunwind"
+// CHECK-LD64-NOT: "--no-as-needed"
 // CHECK-LD64-NOT: "-lm"
 // CHECK-LD64: "-lc"
 
@@ -67,7 +71,9 @@
 // CHECK-LD32-PTHREAD-NOT: "-lc++"
 // CHECK-LD32-PTHREAD-NOT: "-lc++abi"
 // CHECK-LD32-PTHREAD: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-PTHREAD-NOT: "--as-needed"
 // CHECK-LD32-PTHREAD: "-lunwind"
+// CHECK-LD32-PTHREAD-NOT: "--no-as-needed"
 // CHECK-LD32-PTHREAD: "-lpthreads"
 // CHECK-LD32-PTHREAD-NOT: "-lm"
 // CHECK-LD32-PTHREAD: "-lc"
@@ -92,7 +98,9 @@
 // CHECK-LD64-PTHREAD-NOT: "-lc++"
 // CHECK-LD64-PTHREAD-NOT: "-lc++abi"
 // CHECK-LD64-PTHREAD: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-PTHREAD-NOT: "--as-needed"
 // CHECK-LD64-PTHREAD: "-lunwind"
+// CHECK-LD64-PTHREAD-NOT: "--no-as-needed"
 // CHECK-LD64-PTHREAD: "-lpthreads"
 // CHECK-LD64-PTHREAD-NOT: "-lm"
 // CHECK-LD64-PTHREAD: "-lc"
@@ -117,7 +125,9 @@
 // CHECK-LD32-PROF-NOT: "-lc++"
 // CHECK-LD32-PROF-NOT: "-lc++abi"
 // CHECK-LD32-PROF: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-PROF-NOT: "--as-needed"
 // CHECK-LD32-PROF: "-lunwind"
+// CHECK-LD32-PROF-NOT: "--no-as-needed"
 // CHECK-LD32-PROF-NOT: "-lm"
 // CHECK-LD32-PROF: "-lc"
 
@@ -141,7 +151,9 @@
 // CHECK-LD64-GPROF-NOT: "-lc++"
 // CHECK-LD64-GPROF-NOT: "-lc++abi"
 // CHECK-LD64-GPROF: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-GPROF-NOT: "--as-needed"
 // CHECK-LD64-GPROF: "-lunwind"
+// CHECK-LD64-GPROF-NOT: "--no-as-needed"
 // CHECK-LD64-GPROF-NOT: "-lm"
 // CHECK-LD64-GPROF: "-lc"
 
@@ -165,7 +177,9 @@
 // CHECK-LD32-STATIC-NOT: "-lc++"
 // CHECK-LD32-STATIC-NOT: "-lc++abi"
 // CHECK-LD32-STATIC: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-STATIC-NOT: "--as-needed"
 // CHECK-LD32-STATIC-NOT: "-lunwind"
+// CHECK-LD32-STATIC-NOT: "--no-as-needed"
 // CHECK-LD32-STA

[PATCH] D104314: [AIX] Remove --as-needed passing into aix linker

[Jason Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4e2aee8d3bab: [AIX] Remove --as-needed passing into aix 
linker (authored by jasonliu).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104314

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/aix-ld.c

Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -20,7 +20,9 @@
 // CHECK-LD32-NOT: "-lc++"
 // CHECK-LD32-NOT: "-lc++abi"
 // CHECK-LD32: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-NOT: "--as-needed"
 // CHECK-LD32: "-lunwind"
+// CHECK-LD32-NOT: "--no-as-needed"
 // CHECK-LD32-NOT: "-lm"
 // CHECK-LD32: "-lc"
 
@@ -43,7 +45,9 @@
 // CHECK-LD64-NOT: "-lc++"
 // CHECK-LD64-NOT: "-lc++abi"
 // CHECK-LD64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-NOT: "--as-needed"
 // CHECK-LD64: "-lunwind"
+// CHECK-LD64-NOT: "--no-as-needed"
 // CHECK-LD64-NOT: "-lm"
 // CHECK-LD64: "-lc"
 
@@ -67,7 +71,9 @@
 // CHECK-LD32-PTHREAD-NOT: "-lc++"
 // CHECK-LD32-PTHREAD-NOT: "-lc++abi"
 // CHECK-LD32-PTHREAD: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-PTHREAD-NOT: "--as-needed"
 // CHECK-LD32-PTHREAD: "-lunwind"
+// CHECK-LD32-PTHREAD-NOT: "--no-as-needed"
 // CHECK-LD32-PTHREAD: "-lpthreads"
 // CHECK-LD32-PTHREAD-NOT: "-lm"
 // CHECK-LD32-PTHREAD: "-lc"
@@ -92,7 +98,9 @@
 // CHECK-LD64-PTHREAD-NOT: "-lc++"
 // CHECK-LD64-PTHREAD-NOT: "-lc++abi"
 // CHECK-LD64-PTHREAD: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-PTHREAD-NOT: "--as-needed"
 // CHECK-LD64-PTHREAD: "-lunwind"
+// CHECK-LD64-PTHREAD-NOT: "--no-as-needed"
 // CHECK-LD64-PTHREAD: "-lpthreads"
 // CHECK-LD64-PTHREAD-NOT: "-lm"
 // CHECK-LD64-PTHREAD: "-lc"
@@ -117,7 +125,9 @@
 // CHECK-LD32-PROF-NOT: "-lc++"
 // CHECK-LD32-PROF-NOT: "-lc++abi"
 // CHECK-LD32-PROF: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-PROF-NOT: "--as-needed"
 // CHECK-LD32-PROF: "-lunwind"
+// CHECK-LD32-PROF-NOT: "--no-as-needed"
 // CHECK-LD32-PROF-NOT: "-lm"
 // CHECK-LD32-PROF: "-lc"
 
@@ -141,7 +151,9 @@
 // CHECK-LD64-GPROF-NOT: "-lc++"
 // CHECK-LD64-GPROF-NOT: "-lc++abi"
 // CHECK-LD64-GPROF: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-GPROF-NOT: "--as-needed"
 // CHECK-LD64-GPROF: "-lunwind"
+// CHECK-LD64-GPROF-NOT: "--no-as-needed"
 // CHECK-LD64-GPROF-NOT: "-lm"
 // CHECK-LD64-GPROF: "-lc"
 
@@ -165,7 +177,9 @@
 // CHECK-LD32-STATIC-NOT: "-lc++"
 // CHECK-LD32-STATIC-NOT: "-lc++abi"
 // CHECK-LD32-STATIC: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-STATIC-NOT: "--as-needed"
 // CHECK-LD32-STATIC-NOT: "-lunwind"
+// CHECK-LD32-STATIC-NOT: "--no-as-needed"
 // CHECK-LD32-STATIC-NOT: "-lm"
 // CHECK-LD32-STATIC: "-lc"
 
@@ -190,7 +204,9 @@
 // CHECK-LD32-LIBP-NOT: "-lc++"
 // CHECK-LD32-LIBP-NOT: "-lc++abi"
 // CHECK-LD32-LIBP: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-LIBP-NOT: "--as-needed"
 // CHECK-LD32-LIBP: "-lunwind"
+// CHECK-LD32-LIBP-NOT: "--no-as-needed"
 // CHECK-LD32-LIBP-NOT: "-lm"
 // CHECK-LD32-LIBP: "-lc"
 
@@ -215,7 +231,9 @@
 // CHECK-LD32-NO-STD-LIB-NOT: "-lc++"
 // CHECK-LD32-NO-STD-LIB-NOT: "-lc++abi"
 // CHECK-LD32-NO-STD-LIB-NOT: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-NO-STD-LIB-NOT: "--as-needed"
 // CHECK-LD32-NO-STD-LIB-NOT: "-lunwind"
+// CHECK-LD32-NO-STD-LIB-NOT: "--no-as-needed"
 // CHECK-LD32-NO-STD-LIB-NOT: "-lpthreads"
 // CHECK-LD32-NO-STD-LIB-NOT: "-lm"
 // CHECK-LD32-NO-STD-LIB-NOT: "-lc"
@@ -241,7 +259,9 @@
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lc++"
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lc++abi"
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-NO-DEFAULT-LIBS-NOT: "--as-needed"
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lunwind"
+// CHECK-LD64-NO-DEFAULT-LIBS-NOT: "--no-as-needed"
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lpthreads"
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lm"
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lc"
@@ -268,7 +288,9 @@
 // CHECK-LD32-ARG-ORDER-NOT: "-lc++"
 // CHECK-LD32-ARG-ORDER-NOT: "-lc++abi"
 // CHECK-LD32-ARG-ORDER: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-ARG-ORDER-NOT: "--as-needed"
 // CHE

[PATCH] D104198: [Matrix] Add documentation for compound assignment and type conversion of matrix types

[Saurabh Jha via Phabricator via cfe-commits]

SaurabhJha updated this revision to Diff 352783.
SaurabhJha added a comment.

Address comment: replace scalar variables by values


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104198

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -523,6 +523,64 @@
 return a + b * c;
   }
 
+The matrix type extension also supports operations between a matrix and a 
scalar.
+
+.. code-block:: c++
+
+  typedef float m4x4_t __attribute__((matrix_type(4, 4)));
+
+  m4x4_t f(m4x4_t a) {
+  return (a + 23) * 12;
+  }
+
+The matrix type extension supports division between a matrix and a scalar but 
not between a matrix and a matrix.
+
+.. code-block:: c++
+  typedef float m4x4_t __attribute__((matrix_type(4, 4)));
+
+  m4x4_t f(m4x4_t a) {
+a = a / 3.0;
+return a;
+  }
+
+The matrix type extension supports compound assignments for addition, 
subtraction, and multiplication between matrices
+and between a matrix and a scalar, provided their types are consistent.
+
+.. code-block:: c++
+
+  typedef float m4x4_t __attribute__((matrix_type(4, 4)));
+
+  m4x4_t f(m4x4_t a, m4x4_t b) {
+a += b;
+a -= b;
+a *= b;
+a += 23;
+a -= 12;
+return a;
+  }
+
+The matrix type extension supports explicit casts. The casts we support are 
C-style casts in C and C++ and
+static casts. Implicit type conversion between matrix types is not allowed.
+
+.. code-block:: c++
+
+  typedef int ix5x5 __attribute__((matrix_type(5, 5)));
+  typedef float fx5x5 __attribute__((matrix_type(5, 5)));
+
+  fx5x5 f1(ix5x5 i, fx5x5 f) {
+return (fx5x5) i;
+  }
+
+
+  template 
+  using matrix_4_4 = X __attribute__((matrix_type(4, 4)));
+
+  void f2() {
+matrix_5_5 d;
+matrix_5_5 i;
+i = (matrix_5_5)d;
+i = static_cast>(d);
+  }
 
 Half-Precision Floating Point
 =


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -523,6 +523,64 @@
 return a + b * c;
   }
 
+The matrix type extension also supports operations between a matrix and a scalar.
+
+.. code-block:: c++
+
+  typedef float m4x4_t __attribute__((matrix_type(4, 4)));
+
+  m4x4_t f(m4x4_t a) {
+  return (a + 23) * 12;
+  }
+
+The matrix type extension supports division between a matrix and a scalar but not between a matrix and a matrix.
+
+.. code-block:: c++
+  typedef float m4x4_t __attribute__((matrix_type(4, 4)));
+
+  m4x4_t f(m4x4_t a) {
+a = a / 3.0;
+return a;
+  }
+
+The matrix type extension supports compound assignments for addition, subtraction, and multiplication between matrices
+and between a matrix and a scalar, provided their types are consistent.
+
+.. code-block:: c++
+
+  typedef float m4x4_t __attribute__((matrix_type(4, 4)));
+
+  m4x4_t f(m4x4_t a, m4x4_t b) {
+a += b;
+a -= b;
+a *= b;
+a += 23;
+a -= 12;
+return a;
+  }
+
+The matrix type extension supports explicit casts. The casts we support are C-style casts in C and C++ and
+static casts. Implicit type conversion between matrix types is not allowed.
+
+.. code-block:: c++
+
+  typedef int ix5x5 __attribute__((matrix_type(5, 5)));
+  typedef float fx5x5 __attribute__((matrix_type(5, 5)));
+
+  fx5x5 f1(ix5x5 i, fx5x5 f) {
+return (fx5x5) i;
+  }
+
+
+  template 
+  using matrix_4_4 = X __attribute__((matrix_type(4, 4)));
+
+  void f2() {
+matrix_5_5 d;
+matrix_5_5 i;
+i = (matrix_5_5)d;
+i = static_cast>(d);
+  }
 
 Half-Precision Floating Point
 =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104285: [analyzer] Retrieve value by direct index from list initialization of constant array declaration.

[Chris Hamilton via Phabricator via cfe-commits]

chrish_ericsson_atx added a comment.

In D104285#2823305 , 
@chrish_ericsson_atx wrote:

> I'm not sure about whether or not this patch would only work for constant 
> arrays with initializer lists.  If it does only work for such arrays, then I 
> wonder whether the fix is broad enough -- I haven't tested (yet), but I think 
> the presence of the initializer list in the test case is not necessary to 
> trigger the spurious warnings about garbage/undefined values.  I'll try it 
> tomorrow morning...

I tested with an unpatched build using a reproducer without an initializer 
list, and didn't get the spurious warnings -- so your approach seems correct to 
me now.   I've also tested your patch and I believe it gives correct behavior.




Comment at: clang/lib/AST/Type.cpp:149
+Extents.push_back(*CAT->getSize().getRawData());
+  } while (CAT = dyn_cast(CAT->getElementType()));
+  return Extents;




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104285

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

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

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

dexonsmith added a subscriber: arphaman.
dexonsmith added a comment.

In D103930#2820725 , @bruno wrote:

> Thanks for working on this, comments inline. @vsapsai @jansvoboda11 
> @dexonsmith any headermap related concerns on your side?

@jansvoboda11, I think it'd be prudent for us to test this patch out internally 
before it's landed, since I don't really trust that the existing unit tests 
cover all the interactions between header maps and modules. Might you be able 
to coordinate something with @arphaman?




Comment at: clang/test/Modules/implicit-module-header-maps.cpp:27
+#define FOO
+// This include will fail if modules weren't used.  The include name itself
+// doesn't exist and relies on the header map to remap it to the real header.

Do you really mean if modules aren't used, or do you mean if header maps aren't 
used?

(I think we want to find the same headers on disk whether or not modules are 
on... if this patch changes that, then I guess I'm not totally understanding 
why...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930

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

[PATCH] D99381: [compiler-rt][hwasan] Add Fuchsia-specific sanitizer platform limits

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

*ping*


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99381

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

[PATCH] D103544: [compiler-rt][Fuchsia] Disable interceptors while enabling new/delete replacements

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

*ping*


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103544

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

[PATCH] D103936: [compiler-rt][hwasan] Define fuchsia implementations of required hwasan functions

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

*ping* Anything else that should be updated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103936

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

[PATCH] D104381: [analyzer] Added a test case for PR46264

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Thanks!

I guess there might be some value in building an older Clang from around when 
the bug was reported so that to see if the crash was indeed there and we're not 
just reproducing it wrong. It would also allow us to understand where we fixed 
it through bisecting. But I don't insist.




Comment at: clang/test/Analysis/diagnostics/PR46264.cpp:4
+// PR46264
+// This case shall not warn about dereference of void*
+namespace ns1 {

I think you mean this case shall not crash with an assertion failure.

"Warn" exclusively means "display a warning to the user".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104381

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

[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: MaskRay, melver, void, davidxl, vsk, phosek.
Herald added a reviewer: aaron.ballman.
Herald added subscribers: wenlei, jdoerfert.
nickdesaulniers requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

noprofile IR attribute already exists to prevent profiling with PGO;
emit that when a function uses the newly added no_profile function
attribute.

The Linux kernel would like to avoid compiler generated code in
functions annotated with such attribute. We already respect this for
libcalls to fentry() and mcount().

Alternate implementation to: https://reviews.llvm.org/D104253
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223
Link: 
https://lore.kernel.org/lkml/cakwvodmpti93n2l0_yqkrzldmpxzror7zggszonyaw2pgsh...@mail.gmail.com/


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104475

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/fprofile.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test


Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -99,6 +99,7 @@
 // CHECK-NEXT: NoMerge (SubjectMatchRule_function)
 // CHECK-NEXT: NoMicroMips (SubjectMatchRule_function)
 // CHECK-NEXT: NoMips16 (SubjectMatchRule_function)
+// CHECK-NEXT: NoProfileFunction (SubjectMatchRule_function)
 // CHECK-NEXT: NoSanitize (SubjectMatchRule_function, 
SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: NoSanitizeSpecific (SubjectMatchRule_function, 
SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: NoSpeculativeLoadHardening (SubjectMatchRule_function, 
SubjectMatchRule_objc_method)
Index: clang/test/CodeGen/fprofile.c
===
--- /dev/null
+++ clang/test/CodeGen/fprofile.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fprofile-instrument=llvm -disable-llvm-passes \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fprofile-instrument=csllvm -disable-llvm-passes \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fprofile-instrument=clang -disable-llvm-passes \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fprofile-arcs -disable-llvm-passes \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+int g(int);
+
+void __attribute__((no_profile)) no_instr() {
+// CHECK: define {{.*}} void @no_instr() [[ATTR:#[0-9]+]]
+}
+
+void instr(void) {
+// CHECK: define {{.*}} void @instr() [[ATTR2:#[0-9]+]]
+}
+// CHECK: attributes [[ATTR]] = {{.*}} noprofile
+// CHECK-NOT: attributes [[ATTR2]] = {{.*}} noprofile
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -893,6 +893,9 @@
   if (D && D->hasAttr())
 Fn->addFnAttr("cfi-canonical-jump-table");
 
+  if (D && D->hasAttr())
+Fn->addFnAttr(llvm::Attribute::NoProfile);
+
   if (getLangOpts().OpenCL) {
 // Add metadata for a kernel function.
 if (const FunctionDecl *FD = dyn_cast_or_null(D))
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -2559,6 +2559,17 @@
   }];
 }
 
+def NoProfileDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+Use the ``no_profile`` attribute on a function to declaration to denote that
+the compiler should not instrument the function with profile related
+instrumentation, such as via the
+``-fprofile-generate`` / ``-fprofile-instr-generate`` /
+``-fcs-profile-generate`` / ``-fprofile-arcs`` flags.
+}];
+}
+
 def NoSanitizeDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1970,6 +1970,13 @@
   let SimpleHandler = 1;
 }
 
+def NoProfileFunction : InheritableAttr {
+  let Spellings = [GCC<"no_profile">];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [NoProfileDocs];
+  let SimpleHandler = 1;
+}
+
 def NotTailCalled : InheritableAttr {
   let Spellings = [Clang<"not_tail_called">];
   let Subjects = SubjectList<[Function]>;


Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -9

[PATCH] D103750: [analyzer] Handle std::make_unique for SmartPtrModeling

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/test/Analysis/smart-ptr-text-output.cpp:3
+// RUN:  -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.SmartPtr\
+// RUN:  -analyzer-config 
cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\
+// RUN:  -analyzer-output=text -std=c++20 %s -verify=expected

RedDocMD wrote:
> NoQ wrote:
> > Could you double-check that this flag correctly disables all the newly 
> > introduced modeling so that it wasn't accidentally enabled for all users 
> > before it's ready?
> Yup. There are 133 notes and warnings (just search for "// expected-" and see 
> the count). And on setting the flag to `false`, there are 133 errors.
> So it works.
I mean, specifically the modeling added by this patch? I see you wrote an 
entire checker callback that doesn't seem to have an analyzer-config option 
check anywhere in it. Simply having different results isn't sufficient to 
verify this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103750

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

[PATCH] D104459: [clang][lex] Ensure minimizer output is never larger than input

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

dexonsmith added a comment.

> This patch ensures the output of source minimizer is never larger than the 
> input. This property will be handy in a follow up patch that makes it 
> possible to pad the minimized output to the size of the original input.

I suppose when I wrote first wrote this I was thinking of a secondary purpose 
of canonicalizing the sources; in which case, adding a trailing newline (for 
example) seems like feature rather than a bug. I'm not too attached to that, 
but I am curious for more context about why it's important to be able to pad 
the file to the same size, and whether that indicates a bug that should be 
fixed somewhere else instead. (I'll read the follow-up patches in the stack...)




Comment at: clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp:149
   ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define MACRO((a))", 
Out));
-  EXPECT_STREQ("#define MACRO(/* invalid */\n", Out.data());
+  EXPECT_STREQ("#define MACRO\n", Out.data());
 

I'm not comfortable with this change... the idea of the `(/*invalid*/` was to 
guarantee that an invalid macro argument list didn't get minimized to something 
valid.

Perhaps the result could be `"#define MACRO(\n"`, and just strip the comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104459

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

[PATCH] D104253: [Clang][Codegen] emit noprofile IR Fn Attr for no_instrument_function Fn Attr

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers abandoned this revision.
nickdesaulniers added a comment.

Jotting down driver to front end flag translations:

-fprofile-generate -> -fprofile-instrument=llvm
-fprofile-instr-generate -> -fprofile-instrument=clang
-fcs-profile-generate -> -fprofile-instrument=csllvm
-fprofile-arcs -> -fprofile-arcs

In D104253#2819995 , @MaskRay wrote:

> For the function attributing disabling instrumentation of 
> -fprofile-generate/-fprofile-instr-generate/-fcs-profile-generate/-fprofile-arcs,
>  how about `no_profile`?

Sure, abandoning. Prefer D104475 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104253

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

[PATCH] D88666: DirectoryWatcher: add an implementation for Windows

[Stella Stamenova via Phabricator via cfe-commits]

stella.stamenova added a comment.

We still occasionally (every couple of runs) see these tests hang on Windows in 
both Debug and Release. Unfortunately, I don't have access to the machines 
running the tests to debug the tests while they are hanging and I haven't had a 
chance to try to reproduce locally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88666

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