[PATCH] D108194: [clangd] IncludeCleaner: Mark used headers

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 376430.
kbobyrev added a comment.

Fix the rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108194

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

Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -16,6 +16,8 @@
 namespace clangd {
 namespace {
 
+using ::testing::UnorderedElementsAre;
+
 TEST(IncludeCleaner, ReferencedLocations) {
   struct TestCase {
 std::string HeaderCode;
@@ -131,6 +133,42 @@
   }
 }
 
+TEST(IncludeCleaner, GetUnusedHeaders) {
+  llvm::StringLiteral MainFile = R"cpp(
+#include "a.h"
+#include "b.h"
+#include "dir/c.h"
+#include "dir/unused.h"
+#include "unused.h"
+void foo() {
+  a();
+  b();
+  c();
+})cpp";
+  // Build expected ast with symbols coming from headers.
+  TestTU TU;
+  TU.Filename = "foo.cpp";
+  TU.AdditionalFiles["foo.h"] = "void foo();";
+  TU.AdditionalFiles["a.h"] = "void a();";
+  TU.AdditionalFiles["b.h"] = "void b();";
+  TU.AdditionalFiles["dir/c.h"] = "void c();";
+  TU.AdditionalFiles["unused.h"] = "void unused();";
+  TU.AdditionalFiles["dir/unused.h"] = "void dirUnused();";
+  TU.AdditionalFiles["not_included.h"] = "void notIncluded();";
+  TU.ExtraArgs = {"-I" + testPath("dir")};
+  TU.Code = MainFile.str();
+  ParsedAST AST = TU.build();
+  auto UnusedIncludes = AST.computeUnusedIncludes();
+  std::vector UnusedHeaders;
+  UnusedHeaders.reserve(UnusedIncludes.size());
+  for (const auto &Include : UnusedIncludes) {
+// Strip enclosing "".
+UnusedHeaders.push_back(
+Include.Written.substr(1, Include.Written.size() - 2));
+  }
+  EXPECT_THAT(UnusedHeaders, UnorderedElementsAre("unused.h", "dir/unused.h"));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/ParsedAST.h
===
--- clang-tools-extra/clangd/ParsedAST.h
+++ clang-tools-extra/clangd/ParsedAST.h
@@ -118,6 +118,8 @@
 return Resolver.get();
   }
 
+  std::vector computeUnusedIncludes();
+
 private:
   ParsedAST(llvm::StringRef Version,
 std::shared_ptr Preamble,
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -18,6 +18,7 @@
 #include "FeatureModule.h"
 #include "Headers.h"
 #include "HeuristicResolver.h"
+#include "IncludeCleaner.h"
 #include "IncludeFixer.h"
 #include "Preamble.h"
 #include "SourceCode.h"
@@ -624,5 +625,34 @@
 return llvm::None;
   return llvm::StringRef(Preamble->Version);
 }
+
+std::vector ParsedAST::computeUnusedIncludes() {
+  const auto &SM = getSourceManager();
+
+  auto Refs = findReferencedLocations(*this);
+  auto ReferencedFileIDs = findReferencedFiles(Refs, SM);
+  llvm::DenseSet ReferencedFiles;
+  ReferencedFiles.reserve(ReferencedFileIDs.size());
+  for (FileID FID : ReferencedFileIDs) {
+const FileEntry *FE = SM.getFileEntryForID(FID);
+if (!FE) {
+  elog("Missing FE for {0}", SM.getComposedLoc(FID, 0).printToString(SM));
+  continue;
+}
+const auto File = Includes.getID(FE);
+if (!File) {
+  elog("Missing FE for {0}", SM.getComposedLoc(FID, 0).printToString(SM));
+  continue;
+}
+ReferencedFiles.insert(*File);
+  }
+  auto MainFileIndex = Includes.getID(SM.getFileEntryForID(SM.getMainFileID()));
+  if (!MainFileIndex) {
+elog("Missing MainFile in the IncludeStructure");
+return {};
+  }
+  return getUnused(*MainFileIndex, Includes, ReferencedFiles, SM);
+}
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/IncludeCleaner.h
===
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -25,6 +25,7 @@
 #include "ParsedAST.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/DenseSet.h"
+#include 
 
 namespace clang {
 namespace clangd {
@@ -46,6 +47,31 @@
 /// - err on the side of reporting all possible locations
 ReferencedLocations findReferencedLocations(ParsedAST &AST);
 
+/// Retrieves IDs of all files containing SourceLocations from \p Locs.
+/// FIXME: Those locations could be within macro expansions and are resolved to
+/// their spelling/expansion locations.
+llvm::DenseSet findReferencedFiles(const ReferencedLocations &Locs,
+   const SourceManager &SM);
+
+inline llvm:

[PATCH] D108194: [clangd] IncludeCleaner: Mark used headers

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 376431.
kbobyrev added a comment.

Tiny refactoring.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108194

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

Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -16,6 +16,8 @@
 namespace clangd {
 namespace {
 
+using ::testing::UnorderedElementsAre;
+
 TEST(IncludeCleaner, ReferencedLocations) {
   struct TestCase {
 std::string HeaderCode;
@@ -131,6 +133,42 @@
   }
 }
 
+TEST(IncludeCleaner, GetUnusedHeaders) {
+  llvm::StringLiteral MainFile = R"cpp(
+#include "a.h"
+#include "b.h"
+#include "dir/c.h"
+#include "dir/unused.h"
+#include "unused.h"
+void foo() {
+  a();
+  b();
+  c();
+})cpp";
+  // Build expected ast with symbols coming from headers.
+  TestTU TU;
+  TU.Filename = "foo.cpp";
+  TU.AdditionalFiles["foo.h"] = "void foo();";
+  TU.AdditionalFiles["a.h"] = "void a();";
+  TU.AdditionalFiles["b.h"] = "void b();";
+  TU.AdditionalFiles["dir/c.h"] = "void c();";
+  TU.AdditionalFiles["unused.h"] = "void unused();";
+  TU.AdditionalFiles["dir/unused.h"] = "void dirUnused();";
+  TU.AdditionalFiles["not_included.h"] = "void notIncluded();";
+  TU.ExtraArgs = {"-I" + testPath("dir")};
+  TU.Code = MainFile.str();
+  ParsedAST AST = TU.build();
+  auto UnusedIncludes = AST.computeUnusedIncludes();
+  std::vector UnusedHeaders;
+  UnusedHeaders.reserve(UnusedIncludes.size());
+  for (const auto &Include : UnusedIncludes) {
+// Strip enclosing "".
+UnusedHeaders.push_back(
+Include.Written.substr(1, Include.Written.size() - 2));
+  }
+  EXPECT_THAT(UnusedHeaders, UnorderedElementsAre("unused.h", "dir/unused.h"));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/ParsedAST.h
===
--- clang-tools-extra/clangd/ParsedAST.h
+++ clang-tools-extra/clangd/ParsedAST.h
@@ -118,6 +118,8 @@
 return Resolver.get();
   }
 
+  std::vector computeUnusedIncludes();
+
 private:
   ParsedAST(llvm::StringRef Version,
 std::shared_ptr Preamble,
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -18,6 +18,7 @@
 #include "FeatureModule.h"
 #include "Headers.h"
 #include "HeuristicResolver.h"
+#include "IncludeCleaner.h"
 #include "IncludeFixer.h"
 #include "Preamble.h"
 #include "SourceCode.h"
@@ -624,5 +625,32 @@
 return llvm::None;
   return llvm::StringRef(Preamble->Version);
 }
+
+std::vector ParsedAST::computeUnusedIncludes() {
+  const auto &SM = getSourceManager();
+
+  auto Refs = findReferencedLocations(*this);
+  auto ReferencedFileIDs = findReferencedFiles(Refs, SM);
+  llvm::DenseSet ReferencedFiles;
+  ReferencedFiles.reserve(ReferencedFileIDs.size());
+  for (FileID FID : ReferencedFileIDs) {
+const FileEntry *FE = SM.getFileEntryForID(FID);
+if (!FE) {
+  elog("Missing FE for {0}", SM.getComposedLoc(FID, 0).printToString(SM));
+  continue;
+}
+const auto File = Includes.getID(FE, SM);
+if (!File) {
+  elog("Missing FE for {0}", SM.getComposedLoc(FID, 0).printToString(SM));
+  continue;
+}
+ReferencedFiles.insert(*File);
+  }
+  auto MainFileIndex =
+  Includes.getID(SM.getFileEntryForID(SM.getMainFileID()), SM);
+  assert(MainFileIndex && "MainFile should always have HeaderID (0)");
+  return getUnused(*MainFileIndex, Includes, ReferencedFiles, SM);
+}
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/IncludeCleaner.h
===
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -25,6 +25,7 @@
 #include "ParsedAST.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/DenseSet.h"
+#include 
 
 namespace clang {
 namespace clangd {
@@ -46,6 +47,31 @@
 /// - err on the side of reporting all possible locations
 ReferencedLocations findReferencedLocations(ParsedAST &AST);
 
+/// Retrieves IDs of all files containing SourceLocations from \p Locs.
+/// FIXME: Those locations could be within macro expansions and are resolved to
+/// their spelling/expansion locations.
+llvm::DenseSet findReferencedFiles(const ReferencedLocations &Locs,
+   const SourceManager &SM);
+
+inline llvm::DenseMap d

[clang] a3a0b06 - [clang][ASTImporter] Import InheritedConstructor and ConstructorUsingShadowDecl.

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

Author: Gabor Marton
Date: 2021-10-01T10:16:11+02:00
New Revision: a3a0b066264fd132a2014edf2aef53072a1fe53a

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

LOG: [clang][ASTImporter] Import InheritedConstructor and 
ConstructorUsingShadowDecl.

Reviewed By: martong

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

Added: 


Modified: 
clang/include/clang/AST/ASTImporter.h
clang/include/clang/ASTMatchers/ASTMatchers.h
clang/lib/AST/ASTImporter.cpp
clang/unittests/AST/ASTImporterTest.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTImporter.h 
b/clang/include/clang/AST/ASTImporter.h
index 17e673a8471a7..c8bdae10a6e6c 100644
--- a/clang/include/clang/AST/ASTImporter.h
+++ b/clang/include/clang/AST/ASTImporter.h
@@ -379,6 +379,9 @@ class TypeSourceInfo;
   return Import(const_cast(FromD));
 }
 
+llvm::Expected
+Import(const InheritedConstructor &From);
+
 /// Return the copy of the given declaration in the "to" context if
 /// it has already been imported from the "from" context.  Otherwise return
 /// nullptr.

diff  --git a/clang/include/clang/ASTMatchers/ASTMatchers.h 
b/clang/include/clang/ASTMatchers/ASTMatchers.h
index 2baa17b59da83..5887263a43e8c 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -5877,6 +5877,10 @@ AST_MATCHER(CXXMethodDecl, isVirtualAsWritten) {
   return Node.isVirtualAsWritten();
 }
 
+AST_MATCHER(CXXConstructorDecl, isInheritingConstructor) {
+  return Node.isInheritingConstructor();
+}
+
 /// Matches if the given method or class declaration is final.
 ///
 /// Given:

diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index a93049b766e74..0dbead08aca4c 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -464,6 +464,9 @@ namespace clang {
 Error ImportDefaultArgOfParmVarDecl(const ParmVarDecl *FromParam,
 ParmVarDecl *ToParam);
 
+Expected
+ImportInheritedConstructor(const InheritedConstructor &From);
+
 template 
 bool hasSameVisibilityContextAndLinkage(T *Found, T *From);
 
@@ -3479,13 +3482,19 @@ ExpectedDecl 
ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
 importExplicitSpecifier(Err, FromConstructor->getExplicitSpecifier());
 if (Err)
   return std::move(Err);
+auto ToInheritedConstructor = InheritedConstructor();
+if (FromConstructor->isInheritingConstructor()) {
+  Expected ImportedInheritedCtor =
+  import(FromConstructor->getInheritedConstructor());
+  if (!ImportedInheritedCtor)
+return ImportedInheritedCtor.takeError();
+  ToInheritedConstructor = *ImportedInheritedCtor;
+}
 if (GetImportedOrCreateDecl(
 ToFunction, D, Importer.getToContext(), cast(DC),
 ToInnerLocStart, NameInfo, T, TInfo, ESpec, D->UsesFPIntrin(),
 D->isInlineSpecified(), D->isImplicit(), D->getConstexprKind(),
-InheritedConstructor(), // FIXME: Properly import inherited
-// constructor info
-TrailingRequiresClause))
+ToInheritedConstructor, TrailingRequiresClause))
   return ToFunction;
   } else if (CXXDestructorDecl *FromDtor = dyn_cast(D)) {
 
@@ -4219,6 +4228,17 @@ Error ASTNodeImporter::ImportDefaultArgOfParmVarDecl(
   return Error::success();
 }
 
+Expected
+ASTNodeImporter::ImportInheritedConstructor(const InheritedConstructor &From) {
+  Error Err = Error::success();
+  CXXConstructorDecl *ToBaseCtor = importChecked(Err, From.getConstructor());
+  ConstructorUsingShadowDecl *ToShadow =
+  importChecked(Err, From.getShadowDecl());
+  if (Err)
+return std::move(Err);
+  return InheritedConstructor(ToShadow, ToBaseCtor);
+}
+
 ExpectedDecl ASTNodeImporter::VisitParmVarDecl(ParmVarDecl *D) {
   // Parameters are created in the translation unit's context, then moved
   // into the function declaration's context afterward.
@@ -4758,9 +4778,23 @@ ExpectedDecl 
ASTNodeImporter::VisitUsingShadowDecl(UsingShadowDecl *D) {
 return ToTargetOrErr.takeError();
 
   UsingShadowDecl *ToShadow;
-  if (GetImportedOrCreateDecl(ToShadow, D, Importer.getToContext(), DC, Loc,
-  Name, *ToIntroducerOrErr, *ToTargetOrErr))
-return ToShadow;
+  if (auto *FromConstructorUsingShadow =
+  dyn_cast(D)) {
+if (GetImportedOrCreateDecl(
+ToShadow, D, Importer.getToContext(), DC, Loc,
+cast(*ToIntroducerOrErr), *ToTargetOrErr,
+FromConstructorUsingShadow->constructsVirtualBase()))
+  return ToShadow;
+// FIXME import the below members!
+// FromConstructorUsingShadow->getNominatedBaseCl

[PATCH] D110395: [clang][ASTImporter] Import InheritedConstructor and ConstructorUsingShadowDecl.

[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 rGa3a0b066264f: [clang][ASTImporter] Import 
InheritedConstructor and ConstructorUsingShadowDecl. (authored by martong, 
committed by balazske).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110395

Files:
  clang/include/clang/AST/ASTImporter.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  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
@@ -6881,6 +6881,31 @@
 ToD->getTemplateSpecializationKind());
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportIsInheritingConstructorBit) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  class a {
+  public:
+a(int);
+  };
+  struct b : a {
+using a::a; // Ihnerited ctor.
+  };
+  void c() {
+(b(0));
+  }
+  )",
+  Lang_CXX11);
+  auto *FromD = FirstDeclMatcher().match(
+  FromTU, cxxConstructorDecl(isInheritingConstructor()));
+  ASSERT_TRUE(FromD);
+  ASSERT_TRUE(FromD->isInheritingConstructor());
+
+  auto *ToD = Import(FromD, Lang_CXX11);
+  ASSERT_TRUE(ToD);
+  EXPECT_TRUE(ToD->isInheritingConstructor());
+}
+
 INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
  DefaultTestValuesForRunOptions);
 
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -464,6 +464,9 @@
 Error ImportDefaultArgOfParmVarDecl(const ParmVarDecl *FromParam,
 ParmVarDecl *ToParam);
 
+Expected
+ImportInheritedConstructor(const InheritedConstructor &From);
+
 template 
 bool hasSameVisibilityContextAndLinkage(T *Found, T *From);
 
@@ -3479,13 +3482,19 @@
 importExplicitSpecifier(Err, FromConstructor->getExplicitSpecifier());
 if (Err)
   return std::move(Err);
+auto ToInheritedConstructor = InheritedConstructor();
+if (FromConstructor->isInheritingConstructor()) {
+  Expected ImportedInheritedCtor =
+  import(FromConstructor->getInheritedConstructor());
+  if (!ImportedInheritedCtor)
+return ImportedInheritedCtor.takeError();
+  ToInheritedConstructor = *ImportedInheritedCtor;
+}
 if (GetImportedOrCreateDecl(
 ToFunction, D, Importer.getToContext(), cast(DC),
 ToInnerLocStart, NameInfo, T, TInfo, ESpec, D->UsesFPIntrin(),
 D->isInlineSpecified(), D->isImplicit(), D->getConstexprKind(),
-InheritedConstructor(), // FIXME: Properly import inherited
-// constructor info
-TrailingRequiresClause))
+ToInheritedConstructor, TrailingRequiresClause))
   return ToFunction;
   } else if (CXXDestructorDecl *FromDtor = dyn_cast(D)) {
 
@@ -4219,6 +4228,17 @@
   return Error::success();
 }
 
+Expected
+ASTNodeImporter::ImportInheritedConstructor(const InheritedConstructor &From) {
+  Error Err = Error::success();
+  CXXConstructorDecl *ToBaseCtor = importChecked(Err, From.getConstructor());
+  ConstructorUsingShadowDecl *ToShadow =
+  importChecked(Err, From.getShadowDecl());
+  if (Err)
+return std::move(Err);
+  return InheritedConstructor(ToShadow, ToBaseCtor);
+}
+
 ExpectedDecl ASTNodeImporter::VisitParmVarDecl(ParmVarDecl *D) {
   // Parameters are created in the translation unit's context, then moved
   // into the function declaration's context afterward.
@@ -4758,9 +4778,23 @@
 return ToTargetOrErr.takeError();
 
   UsingShadowDecl *ToShadow;
-  if (GetImportedOrCreateDecl(ToShadow, D, Importer.getToContext(), DC, Loc,
-  Name, *ToIntroducerOrErr, *ToTargetOrErr))
-return ToShadow;
+  if (auto *FromConstructorUsingShadow =
+  dyn_cast(D)) {
+if (GetImportedOrCreateDecl(
+ToShadow, D, Importer.getToContext(), DC, Loc,
+cast(*ToIntroducerOrErr), *ToTargetOrErr,
+FromConstructorUsingShadow->constructsVirtualBase()))
+  return ToShadow;
+// FIXME import the below members!
+// FromConstructorUsingShadow->getNominatedBaseClassShadowDecl();
+// FromConstructorUsingShadow->getConstructedBaseClassShadowDecl();
+// FromConstructorUsingShadow->getNominatedBaseClass();
+// FromConstructorUsingShadow->getConstructedBaseClass();
+  } else {
+if (GetImportedOrCreateDecl(ToShadow, D, Importer.getToContext(), DC, Loc,
+Name, *ToIntroducerOrErr, *ToTargetOrErr))
+  return ToShadow;
+  }
 
   ToShadow->setLexicalDeclContext(LexicalDC);
   

[PATCH] D110810: [clang][ASTImporter] Simplify code of attribute import [NFC].

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

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

AttrImporter owns now the result of import (until it is read),
instead of referenced error and returned pointer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110810

Files:
  clang/lib/AST/ASTImporter.cpp

Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -8453,7 +8453,8 @@
 };
 
 class AttrImporter {
-  Error Err = Error::success();
+  Error Err{Error::success()};
+  Attr *ToAttr = nullptr;
   ASTImporter &Importer;
   ASTNodeImporter NImporter;
 
@@ -8483,8 +8484,10 @@
   // (The 'Create' with 'ASTContext' first and 'AttributeCommonInfo' last is
   // used here.) As much data is copied or imported from the old attribute
   // as possible. The passed arguments should be already imported.
+  // If any import error happens, nullptr is returned and the internal error
+  // is set to the error. From now on any further import attempt is ignored.
   template 
-  Expected createImportedAttr(const T *FromAttr, Arg &&...ImportedArg) {
+  void importAttr(const T *FromAttr, Arg &&...ImportedArg) {
 static_assert(std::is_base_of::value,
   "T should be subclass of Attr.");
 
@@ -8497,262 +8500,171 @@
 NImporter.importChecked(Err, FromAttr->getScopeLoc());
 
 if (Err)
-  return std::move(Err);
+  return;
 
 AttributeCommonInfo ToI(ToAttrName, ToScopeName, ToAttrRange, ToScopeLoc,
 FromAttr->getParsedKind(), FromAttr->getSyntax(),
 FromAttr->getAttributeSpellingListIndex());
 // The "SemanticSpelling" is not needed to be passed to the constructor.
 // That value is recalculated from the SpellingListIndex if needed.
-T *ToAttr = T::Create(Importer.getToContext(),
-  std::forward(ImportedArg)..., ToI);
+ToAttr = T::Create(Importer.getToContext(),
+   std::forward(ImportedArg)..., ToI);
 
 ToAttr->setImplicit(FromAttr->isImplicit());
 ToAttr->setPackExpansion(FromAttr->isPackExpansion());
 if (auto *ToInheritableAttr = dyn_cast(ToAttr))
   ToInheritableAttr->setInherited(FromAttr->isInherited());
+  }
+
+  void cloneAttr(const Attr *FromAttr) {
+SourceRange ToRange = NImporter.importChecked(Err, FromAttr->getRange());
+if (Err)
+  return;
+
+ToAttr = FromAttr->clone(Importer.getToContext());
+ToAttr->setRange(ToRange);
+  }
 
+  llvm::Expected getResult() {
+if (Err)
+  return std::move(Err);
+assert(ToAttr && "Attribute should be created.");
 return ToAttr;
   }
 };
 
 Expected ASTImporter::Import(const Attr *FromAttr) {
-  Attr *ToAttr = nullptr;
-  // FIXME: Use AttrImporter as much as possible, try to remove the import
-  // of range from here.
-  SourceRange ToRange;
-  if (Error Err = importInto(ToRange, FromAttr->getRange()))
-return std::move(Err);
+  AttrImporter AI(*this);
 
   // FIXME: Is there some kind of AttrVisitor to use here?
   switch (FromAttr->getKind()) {
   case attr::Aligned: {
 auto *From = cast(FromAttr);
-AlignedAttr *To;
-auto CreateAlign = [&](bool IsAlignmentExpr, void *Alignment) {
-  return AlignedAttr::Create(ToContext, IsAlignmentExpr, Alignment, ToRange,
- From->getSyntax(),
- From->getSemanticSpelling());
-};
-if (From->isAlignmentExpr()) {
-  if (auto ToEOrErr = Import(From->getAlignmentExpr()))
-To = CreateAlign(true, *ToEOrErr);
-  else
-return ToEOrErr.takeError();
-} else {
-  if (auto ToTOrErr = Import(From->getAlignmentType()))
-To = CreateAlign(false, *ToTOrErr);
-  else
-return ToTOrErr.takeError();
-}
-To->setInherited(From->isInherited());
-To->setPackExpansion(From->isPackExpansion());
-To->setImplicit(From->isImplicit());
-ToAttr = To;
+if (From->isAlignmentExpr())
+  AI.importAttr(From, true, AI.importArg(From->getAlignmentExpr()).value());
+else
+  AI.importAttr(From, false,
+AI.importArg(From->getAlignmentType()).value());
 break;
   }
+
   case attr::Format: {
 const auto *From = cast(FromAttr);
-FormatAttr *To;
-IdentifierInfo *ToAttrType = Import(From->getType());
-To = FormatAttr::Create(ToContext, ToAttrType, From->getFormatIdx(),
-From->getFirstArg(), ToRange, From->getSyntax());
-To->setInherited(From->isInherited());
-ToAttr = To;
+AI.importAttr(From, Import(From->getType()), From->getFormatIdx(),
+  From->getFirstArg());
 break;
   }
 
   case attr::AssertCapability: {
 const auto *From = cast(FromAttr);
-AttrImporter AI(*this);
-Expected ToAttrOrErr = AI.createImportedAttr(
-

Re: [llvm-dev] Phabricator Creator Pulling the Plug

[James Henderson via cfe-commits]

+1 to the review experience in Github being far worse than Phabricator,
with basically all my specific concerns already being covered in this
thread. I just wanted to add that our downstream LLVM port is based in a
local Github Enterprise instance, and I find it far harder to review and
respond to reviews there, compared to Phabricator. I'm not just opposed to
change because I fear something new - I have active day-to-day experience
with the something new, based on several years of experience, and I don't
like it! I do acknowledge however, that some things have improved (e.g.
multi-line commenting is now a thing, when it didn't used to be), so it's
not an "absolutely never" from me, if the issues can be solved.

James

On Fri, 1 Oct 2021 at 04:11, Mehdi AMINI via llvm-dev <
llvm-...@lists.llvm.org> wrote:

> On Thu, Sep 30, 2021 at 8:05 PM Hubert Tong
>  wrote:
> >
> > On Thu, Sep 30, 2021 at 6:56 PM Mehdi AMINI via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> >>
> >> We talked about this with the IWG (Infrastructure Working Group) just
> >> last week coincidentally.
> >> Two major blocking tracks that were identified at the roundtable
> >> during the LLVM Dev Meeting exactly 2 years ago are still an issue
> >> today:
> >>
> >> 1) Replacement for Herald rules. This is what allows us to subscribe
> >> and track new revisions or commits based on paths in the repo or other
> >> criteria. We could build a replacement based on GitHub action or any
> >> other kind of service, but this is a bit tricky (how do you store
> >> emails privately? etc.). I have looked around online but I didn't find
> >> another OSS project (or external company) providing a similar service
> >> for GitHub unfortunately, does anyone know of any?
> >>
> >> 2) Support for stacked commits. I can see how to structure this
> >> somehow assuming we would push pull-request branches in the main repo
> >> (with one new commit per branch and cascading the pull-requests from
> >> one branch to the other), otherwise this will be a major regression
> >> compared to the current workflow.
> >>
> >> What remains unknown to me is the current state of GitHub management
> >> of comments across `git commit --amend` and force push to update a
> >> branch.
> >
> >
> > Force pushing to a PR branch does make it harder for reviewers to see
> how review comments were addressed or what was done since they last looked
> at the PR. Are your use cases addressed if the workflow consists of pushing
> additional commits to address comments or pushing a merge commit to refresh
> the PR branch? When the PR is approved, the "squash and merge" option can
> be used to commit the patch as a single commit.
>
> This isn't compatible with stacked commits / stacked PR unfortunately.
> Also while merging main back into a branch of commits is "OK",
> rebasing multiple commits is much less friendly (the same conflict may
> have to be fixed over and over in each commit).
>
> > I find the code review experience in GitHub to be a productivity drain
> compared to Phabricator.
> >
> > Older inline comments are much harder to find in GitHub.
> > Much more clicking needed in GitHub to actually load everything (blocks
> of comments folded away, comments collapsed not because you want them
> collapsed but because someone else or maybe just GitHub thought it should
> be collapsed, source files not loaded).
> > GitHub does not allow inline comments further away than a few lines from
> a change.
>
> Thanks! I have the same feeling, but I didn't have anything specific
> to point to and figured that this is in the scope of "I'll get used to
> it", but you mention some good points here.
>
> Best,
>
> --
> Mehdi
> ___
> LLVM Developers mailing list
> llvm-...@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110810: [clang][ASTImporter] Simplify code of attribute import [NFC].

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

balazske updated this revision to Diff 376443.
balazske marked an inline comment as done.
balazske added a comment.

Update the code comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110810

Files:
  clang/lib/AST/ASTImporter.cpp

Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -8453,7 +8453,8 @@
 };
 
 class AttrImporter {
-  Error Err = Error::success();
+  Error Err{Error::success()};
+  Attr *ToAttr = nullptr;
   ASTImporter &Importer;
   ASTNodeImporter NImporter;
 
@@ -8462,15 +8463,14 @@
 
   // Create an "importer" for an attribute parameter.
   // Result of the 'value()' of that object is to be passed to the function
-  // 'createImpoprtedAttr', in the order that is expected by the attribute
-  // class.
+  // 'importAttr', in the order that is expected by the attribute class.
   template  AttrArgImporter importArg(const T &From) {
 return AttrArgImporter(NImporter, Err, From);
   }
 
   // Create an "importer" for an attribute parameter that has array type.
   // Result of the 'value()' of that object is to be passed to the function
-  // 'createImpoprtedAttr', then the size of the array as next argument.
+  // 'importAttr', then the size of the array as next argument.
   template 
   AttrArgArrayImporter importArrayArg(const llvm::iterator_range &From,
  unsigned ArraySize) {
@@ -8483,8 +8483,10 @@
   // (The 'Create' with 'ASTContext' first and 'AttributeCommonInfo' last is
   // used here.) As much data is copied or imported from the old attribute
   // as possible. The passed arguments should be already imported.
+  // If an import error happens, the internal error is set to it, and any
+  // further import attempt is ignored.
   template 
-  Expected createImportedAttr(const T *FromAttr, Arg &&...ImportedArg) {
+  void importAttr(const T *FromAttr, Arg &&...ImportedArg) {
 static_assert(std::is_base_of::value,
   "T should be subclass of Attr.");
 
@@ -8497,262 +8499,175 @@
 NImporter.importChecked(Err, FromAttr->getScopeLoc());
 
 if (Err)
-  return std::move(Err);
+  return;
 
 AttributeCommonInfo ToI(ToAttrName, ToScopeName, ToAttrRange, ToScopeLoc,
 FromAttr->getParsedKind(), FromAttr->getSyntax(),
 FromAttr->getAttributeSpellingListIndex());
 // The "SemanticSpelling" is not needed to be passed to the constructor.
 // That value is recalculated from the SpellingListIndex if needed.
-T *ToAttr = T::Create(Importer.getToContext(),
-  std::forward(ImportedArg)..., ToI);
+ToAttr = T::Create(Importer.getToContext(),
+   std::forward(ImportedArg)..., ToI);
 
 ToAttr->setImplicit(FromAttr->isImplicit());
 ToAttr->setPackExpansion(FromAttr->isPackExpansion());
 if (auto *ToInheritableAttr = dyn_cast(ToAttr))
   ToInheritableAttr->setInherited(FromAttr->isInherited());
+  }
+
+  // Create a clone of the 'FromAttr' and import its source range only.
+  // This causes objects with invalid references to be created if the 'FromAttr'
+  // contains other data that should be imported.
+  void cloneAttr(const Attr *FromAttr) {
+SourceRange ToRange = NImporter.importChecked(Err, FromAttr->getRange());
+if (Err)
+  return;
+
+ToAttr = FromAttr->clone(Importer.getToContext());
+ToAttr->setRange(ToRange);
+  }
 
+  // Get the result of the previous import attempt (can be used only once).
+  llvm::Expected getResult() {
+if (Err)
+  return std::move(Err);
+assert(ToAttr && "Attribute should be created.");
 return ToAttr;
   }
 };
 
 Expected ASTImporter::Import(const Attr *FromAttr) {
-  Attr *ToAttr = nullptr;
-  // FIXME: Use AttrImporter as much as possible, try to remove the import
-  // of range from here.
-  SourceRange ToRange;
-  if (Error Err = importInto(ToRange, FromAttr->getRange()))
-return std::move(Err);
+  AttrImporter AI(*this);
 
   // FIXME: Is there some kind of AttrVisitor to use here?
   switch (FromAttr->getKind()) {
   case attr::Aligned: {
 auto *From = cast(FromAttr);
-AlignedAttr *To;
-auto CreateAlign = [&](bool IsAlignmentExpr, void *Alignment) {
-  return AlignedAttr::Create(ToContext, IsAlignmentExpr, Alignment, ToRange,
- From->getSyntax(),
- From->getSemanticSpelling());
-};
-if (From->isAlignmentExpr()) {
-  if (auto ToEOrErr = Import(From->getAlignmentExpr()))
-To = CreateAlign(true, *ToEOrErr);
-  else
-return ToEOrErr.takeError();
-} else {
-  if (auto ToTOrErr = Import(From->getAlignmentType()))
-To = CreateAlign(false, *ToTOrErr);
-  else
-return ToTOrErr.take

[PATCH] D110825: [clangd] Handle members of anon structs in SelectionTree

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- update comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110825

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -365,6 +365,27 @@
   ElementsAre(Sym("Forward", SymbolHeader.range("forward"), 
Test.range(;
 }
 
+TEST(LocateSymbol, AnonymousStructFields) {
+  auto Code = Annotations(R"cpp(
+struct $2[[Foo]] {
+  struct { int $1[[x]]; };
+  void foo() {
+// Make sure the implicit base is skipped.
+$1^x = 42;
+  }
+};
+// Check that we don't skip explicit bases.
+int a = $2^Foo{}.x;
+  )cpp");
+  TestTU TU = TestTU::withCode(Code.code());
+  auto AST = TU.build();
+  EXPECT_THAT(locateSymbolAt(AST, Code.point("1"), TU.index().get()),
+  UnorderedElementsAre(Sym("x", Code.range("1"), 
Code.range("1";
+  EXPECT_THAT(
+  locateSymbolAt(AST, Code.point("2"), TU.index().get()),
+  UnorderedElementsAre(Sym("Foo", Code.range("2"), Code.range("2";
+}
+
 TEST(LocateSymbol, FindOverrides) {
   auto Code = Annotations(R"cpp(
 class Foo {
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -443,6 +443,15 @@
   if (auto *CTI = llvm::dyn_cast(S))
 if (CTI->isImplicit())
   return true;
+  // Make sure implicit access of anonymous structs don't end up owning tokens.
+  if (auto *ME = llvm::dyn_cast(S)) {
+if (auto *FD = llvm::dyn_cast(ME->getMemberDecl()))
+  if (FD->isAnonymousStructOrUnion())
+// If Base is an implicit CXXThis, then the whole MemberExpr has no
+// tokens. If it's a normal e.g. DeclRef, we treat the MemberExpr like
+// an implicit cast.
+return isImplicit(ME->getBase());
+  }
   // Refs to operator() and [] are (almost?) always implicit as part of calls.
   if (auto *DRE = llvm::dyn_cast(S)) {
 if (auto *FD = llvm::dyn_cast(DRE->getDecl())) {


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -365,6 +365,27 @@
   ElementsAre(Sym("Forward", SymbolHeader.range("forward"), Test.range(;
 }
 
+TEST(LocateSymbol, AnonymousStructFields) {
+  auto Code = Annotations(R"cpp(
+struct $2[[Foo]] {
+  struct { int $1[[x]]; };
+  void foo() {
+// Make sure the implicit base is skipped.
+$1^x = 42;
+  }
+};
+// Check that we don't skip explicit bases.
+int a = $2^Foo{}.x;
+  )cpp");
+  TestTU TU = TestTU::withCode(Code.code());
+  auto AST = TU.build();
+  EXPECT_THAT(locateSymbolAt(AST, Code.point("1"), TU.index().get()),
+  UnorderedElementsAre(Sym("x", Code.range("1"), Code.range("1";
+  EXPECT_THAT(
+  locateSymbolAt(AST, Code.point("2"), TU.index().get()),
+  UnorderedElementsAre(Sym("Foo", Code.range("2"), Code.range("2";
+}
+
 TEST(LocateSymbol, FindOverrides) {
   auto Code = Annotations(R"cpp(
 class Foo {
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -443,6 +443,15 @@
   if (auto *CTI = llvm::dyn_cast(S))
 if (CTI->isImplicit())
   return true;
+  // Make sure implicit access of anonymous structs don't end up owning tokens.
+  if (auto *ME = llvm::dyn_cast(S)) {
+if (auto *FD = llvm::dyn_cast(ME->getMemberDecl()))
+  if (FD->isAnonymousStructOrUnion())
+// If Base is an implicit CXXThis, then the whole MemberExpr has no
+// tokens. If it's a normal e.g. DeclRef, we treat the MemberExpr like
+// an implicit cast.
+return isImplicit(ME->getBase());
+  }
   // Refs to operator() and [] are (almost?) always implicit as part of calls.
   if (auto *DRE = llvm::dyn_cast(S)) {
 if (auto *FD = llvm::dyn_cast(DRE->getDecl())) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110810: [clang][ASTImporter] Simplify code of attribute import [NFC].

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

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



Comment at: clang/lib/AST/ASTImporter.cpp:8530-8531
 
+  // Get the result of the previous import attempt (can be used only once).
+  llvm::Expected getResult() {
+if (Err)

If it can be used only once, we should mark this function as an `r-value` 
function.
There is a similar macro already defined as `LLVM_LVALUE_FUNCTION`.

Later, when you would actually call this function, you need to `std::move()` 
the object, signifying that the original object gets destroyed in the process.

@aaron.ballman Do you think we need to define `LLVM_RVALUE_FUNCTION` or we can 
simply use the `&&` in the function declaration?
I've seen that you tried to substitute all `LLVM_LVALUE_FUNCTION` macros in the 
past. What's the status on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110810

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

[PATCH] D110910: [analyzer][solver] Fix CmpOpTable handling bug

[Gabor Marton via Phabricator via cfe-commits]

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

There is an error in the implementation of the logic of reaching the `Unknonw` 
tristate in CmpOpTable.

  void cmp_op_table_unknownX2(int x, int y, int z) {
if (x >= y) {
  // x >= y[1, 1]
  if (x + z < y)
return;
  // x + z < y [0, 0]
  if (z != 0)
return;
  // x < y [0, 0]
  clang_analyzer_eval(x > y);  // expected-warning{{TRUE}} 
expected-warning{{FALSE}}
}
  }

We miss the `FALSE` warning because the false branch is infeasible.

We have to exploit simplification to discover the bug. If we had `x < y`
as the second condition then the analyzer would return the parent state
on the false path and the new constraint would not be part of the State.
But adding `z` to the condition makes both paths feasible.

The root cause of the bug is that we reach the `Unknown` tristate
twice, but in both occasions we reach the same `Op` that is `>=` in the
test case. So, we reached `>=` twice, but we never reached `!=`, thus
querying the `Unknonw2x` column with `getCmpOpStateForUnknownX2` is
wrong.

The solution is to ensure that we reached both **different** `Op`s once.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110910

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/constraint_manager_conditions.cpp


Index: clang/test/Analysis/constraint_manager_conditions.cpp
===
--- clang/test/Analysis/constraint_manager_conditions.cpp
+++ clang/test/Analysis/constraint_manager_conditions.cpp
@@ -211,3 +211,17 @@
   clang_analyzer_eval(y != x); // expected-warning{{FALSE}}
 }
 }
+
+// Test the logic of reaching the `Unknonw` tristate in CmpOpTable.
+void cmp_op_table_unknownX2(int x, int y, int z) {
+  if (x >= y) {
+// x >= y[1, 1]
+if (x + z < y)
+  return;
+// x + z < y [0, 0]
+if (z != 0)
+  return;
+// x < y [0, 0]
+clang_analyzer_eval(x > y);  // expected-warning{{TRUE}} 
expected-warning{{FALSE}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -1126,7 +1126,7 @@
 
 SymbolManager &SymMgr = State->getSymbolManager();
 
-int UnknownStates = 0;
+llvm::SmallSet QueriedToUnknown;
 
 // Loop goes through all of the columns exept the last one ('UnknownX2').
 // We treat `UnknownX2` column separately at the end of the loop body.
@@ -1163,7 +1163,8 @@
   CmpOpTable.getCmpOpState(CurrentOP, QueriedOP);
 
   if (BranchState == OperatorRelationsTable::Unknown) {
-if (++UnknownStates == 2)
+QueriedToUnknown.insert(QueriedOP);
+if (QueriedToUnknown.size() == 2)
   // If we met both Unknown states.
   // if (x <= y)// assume true
   //   if (x != y)  // assume true


Index: clang/test/Analysis/constraint_manager_conditions.cpp
===
--- clang/test/Analysis/constraint_manager_conditions.cpp
+++ clang/test/Analysis/constraint_manager_conditions.cpp
@@ -211,3 +211,17 @@
   clang_analyzer_eval(y != x); // expected-warning{{FALSE}}
 }
 }
+
+// Test the logic of reaching the `Unknonw` tristate in CmpOpTable.
+void cmp_op_table_unknownX2(int x, int y, int z) {
+  if (x >= y) {
+// x >= y[1, 1]
+if (x + z < y)
+  return;
+// x + z < y [0, 0]
+if (z != 0)
+  return;
+// x < y [0, 0]
+clang_analyzer_eval(x > y);  // expected-warning{{TRUE}} expected-warning{{FALSE}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -1126,7 +1126,7 @@
 
 SymbolManager &SymMgr = State->getSymbolManager();
 
-int UnknownStates = 0;
+llvm::SmallSet QueriedToUnknown;
 
 // Loop goes through all of the columns exept the last one ('UnknownX2').
 // We treat `UnknownX2` column separately at the end of the loop body.
@@ -1163,7 +1163,8 @@
   CmpOpTable.getCmpOpState(CurrentOP, QueriedOP);
 
   if (BranchState == OperatorRelationsTable::Unknown) {
-   

[PATCH] D110395: [clang][ASTImporter] Import InheritedConstructor and ConstructorUsingShadowDecl.

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

steakhal added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5880
 
+AST_MATCHER(CXXConstructorDecl, isInheritingConstructor) {
+  return Node.isInheritingConstructor();

We need to add doc comments as well.
Please address this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110395

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

[PATCH] D110911: [analyzer][NFC] Add RangeSet::dump

[Gabor Marton via Phabricator via cfe-commits]

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

This tiny change improves the debugging experience of the solver a lot!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110911

Files:
  
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp


Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -504,6 +504,8 @@
   OS << " }";
 }
 
+void RangeSet::dump() const { dump(llvm::errs()); }
+
 REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(SymbolSet, SymbolRef)
 
 namespace {
Index: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
===
--- 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
+++ 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
@@ -282,6 +282,7 @@
   bool contains(llvm::APSInt Point) const { return containsImpl(Point); }
 
   void dump(raw_ostream &OS) const;
+  void dump() const;
 
   bool operator==(const RangeSet &Other) const { return *Impl == *Other.Impl; }
   bool operator!=(const RangeSet &Other) const { return !(*this == Other); }


Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -504,6 +504,8 @@
   OS << " }";
 }
 
+void RangeSet::dump() const { dump(llvm::errs()); }
+
 REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(SymbolSet, SymbolRef)
 
 namespace {
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
@@ -282,6 +282,7 @@
   bool contains(llvm::APSInt Point) const { return containsImpl(Point); }
 
   void dump(raw_ostream &OS) const;
+  void dump() const;
 
   bool operator==(const RangeSet &Other) const { return *Impl == *Other.Impl; }
   bool operator!=(const RangeSet &Other) const { return !(*this == Other); }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110625: [analyzer] canonicalize special case of structure/pointer deref

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D110625#3035616 , @steakhal wrote:

> 'using' is the same as 'typedef' AFAIK.
> So, you could simply use only typedefs and implement the test in the c test 
> file. Seeing all the tests close together would aid readability IMO.

You're right. I'm OK with `typedef`.

> WDYT Denys? Btw does the SVval::getType return a canonical type in all cases?

`SVval::getType` returns a `QualType` which can be aliased and cv-qualified. 
So, I may mismatch in comparison `BT->getPointeeType() == elementType`. That is 
my concern.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110625

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

[PATCH] D110625: [analyzer] canonicalize special case of structure/pointer deref

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers updated this revision to Diff 376450.
vabridgers added a comment.

added typedef case to C file for comparison, discussion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110625

Files:
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/ptr-arith.c
  clang/test/Analysis/ptr-arith.cpp

Index: clang/test/Analysis/ptr-arith.cpp
===
--- clang/test/Analysis/ptr-arith.cpp
+++ clang/test/Analysis/ptr-arith.cpp
@@ -1,4 +1,8 @@
 // RUN: %clang_analyze_cc1 -Wno-unused-value -std=c++14 -analyzer-checker=core,debug.ExprInspection,alpha.core.PointerArithm -verify %s
+
+void clang_analyzer_eval(int);
+void clang_analyzer_dump(int);
+
 struct X {
   int *p;
   int zero;
@@ -117,3 +121,26 @@
   auto n = p - reinterpret_cast((__UINTPTR_TYPE__)1);
   return n == m;
 }
+
+struct s {
+  int v;
+};
+using T1 = s;
+typedef s T2;
+void struct_pointer_canon(T1 *ps) {
+  T2 ss = *ps;
+  clang_analyzer_dump((*ps).v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps[0].v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps->v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_eval((*ps).v == ps[0].v); // expected-warning{{TRUE}}
+  clang_analyzer_eval((*ps).v == ps->v);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ps[0].v == ps->v);   // expected-warning{{TRUE}}
+}
+
+void struct_pointer_canon_bidim(T1 **ps) {
+  T2 ss = **ps;
+  clang_analyzer_eval(&(ps[0][0].v) == &((*ps)->v)); // expected-warning{{TRUE}}
+}
Index: clang/test/Analysis/ptr-arith.c
===
--- clang/test/Analysis/ptr-arith.c
+++ clang/test/Analysis/ptr-arith.c
@@ -2,6 +2,7 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.FixedAddr,alpha.core.PointerArithm,alpha.core.PointerSub,debug.ExprInspection -analyzer-store=region -Wno-pointer-to-int-cast -verify -triple i686-apple-darwin9 -Wno-tautological-pointer-compare -analyzer-config eagerly-assume=false %s
 
 void clang_analyzer_eval(int);
+void clang_analyzer_dump(int);
 
 void f1() {
   int a[10];
@@ -330,3 +331,46 @@
   simd_float2 x = {0, 1};
   return x[1]; // no-warning
 }
+
+struct s {
+  int v;
+};
+
+// These three expressions should produce the same sym vals.
+void struct_pointer_canon(struct s *ps) {
+  struct s ss = *ps;
+  clang_analyzer_dump((*ps).v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps[0].v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps->v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_eval((*ps).v == ps[0].v); // expected-warning{{TRUE}}
+  clang_analyzer_eval((*ps).v == ps->v);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ps[0].v == ps->v);   // expected-warning{{TRUE}}
+}
+
+void struct_pointer_canon_bidim(struct s **ps) {
+  struct s ss = **ps;
+  clang_analyzer_eval(&(ps[0][0].v) == &((*ps)->v)); // expected-warning{{TRUE}}
+}
+
+typedef struct s T1;
+typedef struct s T2;
+void struct_pointer_canon_typedef(T1 *ps) {
+  T2 ss = *ps;
+  clang_analyzer_dump((*ps).v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps[0].v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps->v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_eval((*ps).v == ps[0].v); // expected-warning{{TRUE}}
+  clang_analyzer_eval((*ps).v == ps->v);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ps[0].v == ps->v);   // expected-warning{{TRUE}}
+}
+
+void struct_pointer_canon_bidim_typedef(T1 **ps) {
+  T2 ss = **ps;
+  clang_analyzer_eval(&(ps[0][0].v) == &((*ps)->v)); // expected-warning{{TRUE}}
+}
Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -442,6 +442,15 @@
 
 SVal StoreManager::getLValueElement(QualType elementType, NonLoc Offset,
 SVal Base) {
+
+  // Special case, if index is 0, return the same type as if
+  // this was not an array dereference.
+  if (Offset.isZeroConstant()) {
+QualType BT = Base.getType(this->Ctx);
+if (!BT.isNull() && BT->getPointeeType() == elementType)
+  return Base;
+  }
+
   // If the base is an unknown or undefined value, just return it back.
   // FIXME: For absolute pointer addresses, we just return that value back as
   //  well, although in reality we should return the offset added to that
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110625: [analyzer] canonicalize special case of structure/pointer deref

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers updated this revision to Diff 376452.
vabridgers added a comment.

move test case from cpp file to c file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110625

Files:
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/ptr-arith.c


Index: clang/test/Analysis/ptr-arith.c
===
--- clang/test/Analysis/ptr-arith.c
+++ clang/test/Analysis/ptr-arith.c
@@ -2,6 +2,7 @@
 // RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.core.FixedAddr,alpha.core.PointerArithm,alpha.core.PointerSub,debug.ExprInspection
 -analyzer-store=region -Wno-pointer-to-int-cast -verify -triple 
i686-apple-darwin9 -Wno-tautological-pointer-compare -analyzer-config 
eagerly-assume=false %s
 
 void clang_analyzer_eval(int);
+void clang_analyzer_dump(int);
 
 void f1() {
   int a[10];
@@ -330,3 +331,46 @@
   simd_float2 x = {0, 1};
   return x[1]; // no-warning
 }
+
+struct s {
+  int v;
+};
+
+// These three expressions should produce the same sym vals.
+void struct_pointer_canon(struct s *ps) {
+  struct s ss = *ps;
+  clang_analyzer_dump((*ps).v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps[0].v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps->v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_eval((*ps).v == ps[0].v); // expected-warning{{TRUE}}
+  clang_analyzer_eval((*ps).v == ps->v);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ps[0].v == ps->v);   // expected-warning{{TRUE}}
+}
+
+void struct_pointer_canon_bidim(struct s **ps) {
+  struct s ss = **ps;
+  clang_analyzer_eval(&(ps[0][0].v) == &((*ps)->v)); // 
expected-warning{{TRUE}}
+}
+
+typedef struct s T1;
+typedef struct s T2;
+void struct_pointer_canon_typedef(T1 *ps) {
+  T2 ss = *ps;
+  clang_analyzer_dump((*ps).v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps[0].v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps->v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_eval((*ps).v == ps[0].v); // expected-warning{{TRUE}}
+  clang_analyzer_eval((*ps).v == ps->v);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ps[0].v == ps->v);   // expected-warning{{TRUE}}
+}
+
+void struct_pointer_canon_bidim_typedef(T1 **ps) {
+  T2 ss = **ps;
+  clang_analyzer_eval(&(ps[0][0].v) == &((*ps)->v)); // 
expected-warning{{TRUE}}
+}
Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -442,6 +442,15 @@
 
 SVal StoreManager::getLValueElement(QualType elementType, NonLoc Offset,
 SVal Base) {
+
+  // Special case, if index is 0, return the same type as if
+  // this was not an array dereference.
+  if (Offset.isZeroConstant()) {
+QualType BT = Base.getType(this->Ctx);
+if (!BT.isNull() && BT->getPointeeType() == elementType)
+  return Base;
+  }
+
   // If the base is an unknown or undefined value, just return it back.
   // FIXME: For absolute pointer addresses, we just return that value back as
   //  well, although in reality we should return the offset added to that


Index: clang/test/Analysis/ptr-arith.c
===
--- clang/test/Analysis/ptr-arith.c
+++ clang/test/Analysis/ptr-arith.c
@@ -2,6 +2,7 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.FixedAddr,alpha.core.PointerArithm,alpha.core.PointerSub,debug.ExprInspection -analyzer-store=region -Wno-pointer-to-int-cast -verify -triple i686-apple-darwin9 -Wno-tautological-pointer-compare -analyzer-config eagerly-assume=false %s
 
 void clang_analyzer_eval(int);
+void clang_analyzer_dump(int);
 
 void f1() {
   int a[10];
@@ -330,3 +331,46 @@
   simd_float2 x = {0, 1};
   return x[1]; // no-warning
 }
+
+struct s {
+  int v;
+};
+
+// These three expressions should produce the same sym vals.
+void struct_pointer_canon(struct s *ps) {
+  struct s ss = *ps;
+  clang_analyzer_dump((*ps).v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps[0].v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps->v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_eval((*ps).v == ps[0].v); // expected-warning{{TRUE}}
+  clang_analyzer_eval((*ps).v == ps->v);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ps[0].v == ps->v);   // expected-warning{{TRUE}}
+}
+
+void struct_pointer_canon_bidim(struct s **ps) {
+  struct s ss = **ps;
+  clang_analyzer_eval(&(ps[0][0].v) == &((*ps)->v)); // expected-warning{{TRUE}}
+}
+
+typedef struct s T1;
+typedef struct s T2;
+void struct_pointer_canon_typedef(T1 *ps) {
+  T2 ss = *

[PATCH] D110625: [analyzer] canonicalize special case of structure/pointer deref

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers updated this revision to Diff 376453.
vabridgers added a comment.

add const test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110625

Files:
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/ptr-arith.c


Index: clang/test/Analysis/ptr-arith.c
===
--- clang/test/Analysis/ptr-arith.c
+++ clang/test/Analysis/ptr-arith.c
@@ -2,6 +2,7 @@
 // RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.core.FixedAddr,alpha.core.PointerArithm,alpha.core.PointerSub,debug.ExprInspection
 -analyzer-store=region -Wno-pointer-to-int-cast -verify -triple 
i686-apple-darwin9 -Wno-tautological-pointer-compare -analyzer-config 
eagerly-assume=false %s
 
 void clang_analyzer_eval(int);
+void clang_analyzer_dump(int);
 
 void f1() {
   int a[10];
@@ -330,3 +331,59 @@
   simd_float2 x = {0, 1};
   return x[1]; // no-warning
 }
+
+struct s {
+  int v;
+};
+
+// These three expressions should produce the same sym vals.
+void struct_pointer_canon(struct s *ps) {
+  struct s ss = *ps;
+  clang_analyzer_dump((*ps).v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps[0].v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps->v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_eval((*ps).v == ps[0].v); // expected-warning{{TRUE}}
+  clang_analyzer_eval((*ps).v == ps->v);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ps[0].v == ps->v);   // expected-warning{{TRUE}}
+}
+
+void struct_pointer_canon_bidim(struct s **ps) {
+  struct s ss = **ps;
+  clang_analyzer_eval(&(ps[0][0].v) == &((*ps)->v)); // 
expected-warning{{TRUE}}
+}
+
+typedef struct s T1;
+typedef struct s T2;
+void struct_pointer_canon_typedef(T1 *ps) {
+  T2 ss = *ps;
+  clang_analyzer_dump((*ps).v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps[0].v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps->v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_eval((*ps).v == ps[0].v); // expected-warning{{TRUE}}
+  clang_analyzer_eval((*ps).v == ps->v);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ps[0].v == ps->v);   // expected-warning{{TRUE}}
+}
+
+void struct_pointer_canon_bidim_typedef(T1 **ps) {
+  T2 ss = **ps;
+  clang_analyzer_eval(&(ps[0][0].v) == &((*ps)->v)); // 
expected-warning{{TRUE}}
+}
+
+void struct_pointer_canon_const(const struct s *ps) {
+  struct s ss = *ps;
+  clang_analyzer_dump((*ps).v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps[0].v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps->v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_eval((*ps).v == ps[0].v); // expected-warning{{TRUE}}
+  clang_analyzer_eval((*ps).v == ps->v);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ps[0].v == ps->v);   // expected-warning{{TRUE}}
+}
Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -442,6 +442,15 @@
 
 SVal StoreManager::getLValueElement(QualType elementType, NonLoc Offset,
 SVal Base) {
+
+  // Special case, if index is 0, return the same type as if
+  // this was not an array dereference.
+  if (Offset.isZeroConstant()) {
+QualType BT = Base.getType(this->Ctx);
+if (!BT.isNull() && BT->getPointeeType() == elementType)
+  return Base;
+  }
+
   // If the base is an unknown or undefined value, just return it back.
   // FIXME: For absolute pointer addresses, we just return that value back as
   //  well, although in reality we should return the offset added to that


Index: clang/test/Analysis/ptr-arith.c
===
--- clang/test/Analysis/ptr-arith.c
+++ clang/test/Analysis/ptr-arith.c
@@ -2,6 +2,7 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.FixedAddr,alpha.core.PointerArithm,alpha.core.PointerSub,debug.ExprInspection -analyzer-store=region -Wno-pointer-to-int-cast -verify -triple i686-apple-darwin9 -Wno-tautological-pointer-compare -analyzer-config eagerly-assume=false %s
 
 void clang_analyzer_eval(int);
+void clang_analyzer_dump(int);
 
 void f1() {
   int a[10];
@@ -330,3 +331,59 @@
   simd_float2 x = {0, 1};
   return x[1]; // no-warning
 }
+
+struct s {
+  int v;
+};
+
+// These three expressions should produce the same sym vals.
+void struct_pointer_canon(struct s *ps) {
+  struct s ss = *ps;
+  clang_analyzer_dump((*ps).v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps[0].v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dum

[PATCH] D110625: [analyzer] canonicalize special case of structure/pointer deref

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers added a comment.

I moved the test cases from ptr-arith.cpp to ptr-arith - using to typedef, and 
created an extra test case that uses const. I believe this may address all 
concerns discussed thus far? Please let me know if we need anything more and 
I'll get it done. Best!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110625

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

[clang] cad9ff5 - [clang][ASTImporter] Import ConstructorUsingShadowDecl correctly.

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

Author: Balázs Kéri
Date: 2021-10-01T11:41:08+02:00
New Revision: cad9ff531c71e7c28d7bd5a64a26f9b214156b59

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

LOG: [clang][ASTImporter] Import ConstructorUsingShadowDecl correctly.

Fix import of ConstructorUsingShadowDecl and add tests.

Reviewed By: martong

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

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 0dbead08aca4..ece9e7fc0112 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -4780,16 +4780,22 @@ ExpectedDecl 
ASTNodeImporter::VisitUsingShadowDecl(UsingShadowDecl *D) {
   UsingShadowDecl *ToShadow;
   if (auto *FromConstructorUsingShadow =
   dyn_cast(D)) {
+Error Err = Error::success();
+ConstructorUsingShadowDecl *Nominated = importChecked(
+Err, FromConstructorUsingShadow->getNominatedBaseClassShadowDecl());
+if (Err)
+  return std::move(Err);
+// The 'Target' parameter of ConstructorUsingShadowDecl constructor
+// is really the "NominatedBaseClassShadowDecl" value if it exists
+// (see code of ConstructorUsingShadowDecl::ConstructorUsingShadowDecl).
+// We should pass the NominatedBaseClassShadowDecl to it (if non-null) to
+// get the correct values.
 if (GetImportedOrCreateDecl(
 ToShadow, D, Importer.getToContext(), DC, Loc,
-cast(*ToIntroducerOrErr), *ToTargetOrErr,
+cast(*ToIntroducerOrErr),
+Nominated ? Nominated : *ToTargetOrErr,
 FromConstructorUsingShadow->constructsVirtualBase()))
   return ToShadow;
-// FIXME import the below members!
-// FromConstructorUsingShadow->getNominatedBaseClassShadowDecl();
-// FromConstructorUsingShadow->getConstructedBaseClassShadowDecl();
-// FromConstructorUsingShadow->getNominatedBaseClass();
-// FromConstructorUsingShadow->getConstructedBaseClass();
   } else {
 if (GetImportedOrCreateDecl(ToShadow, D, Importer.getToContext(), DC, Loc,
 Name, *ToIntroducerOrErr, *ToTargetOrErr))

diff  --git a/clang/unittests/AST/ASTImporterTest.cpp 
b/clang/unittests/AST/ASTImporterTest.cpp
index b9641d77bea4..b7be56b36610 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -6884,15 +6884,14 @@ TEST_P(ASTImporterOptionSpecificTestBase, 
ImportEnumMemberSpecialization) {
 TEST_P(ASTImporterOptionSpecificTestBase, ImportIsInheritingConstructorBit) {
   Decl *FromTU = getTuDecl(
   R"(
-  class a {
-  public:
-a(int);
+  struct A {
+A(int);
   };
-  struct b : a {
-using a::a; // Ihnerited ctor.
+  struct B : A {
+using A::A; // Inherited ctor.
   };
-  void c() {
-(b(0));
+  void f() {
+(B(0));
   }
   )",
   Lang_CXX11);
@@ -6906,6 +6905,220 @@ TEST_P(ASTImporterOptionSpecificTestBase, 
ImportIsInheritingConstructorBit) {
   EXPECT_TRUE(ToD->isInheritingConstructor());
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportConstructorUsingShadow) {
+  TranslationUnitDecl *FromTU = getTuDecl(
+  R"(
+  struct A {
+A(int, int);
+  };
+  struct B : A {
+using A::A;
+  };
+  struct C : B {
+using B::B;
+  };
+  )",
+  Lang_CXX11);
+
+  auto CheckAST = [](TranslationUnitDecl *TU, CXXRecordDecl *RecordC) {
+auto *RecordA = FirstDeclMatcher().match(
+TU, cxxRecordDecl(hasName("A")));
+auto *RecordB = FirstDeclMatcher().match(
+TU, cxxRecordDecl(hasName("B")));
+auto *ConstrA = FirstDeclMatcher().match(
+TU, cxxConstructorDecl(hasParent(equalsNode(RecordA)),
+   parameterCountIs(2)));
+auto *ShadowBA = cast(
+FirstDeclMatcher().match(
+TU, usingShadowDecl(hasParent(equalsNode(RecordB)),
+hasTargetDecl(equalsNode(ConstrA);
+auto *ShadowCA = cast(
+FirstDeclMatcher().match(
+TU, usingShadowDecl(hasParent(equalsNode(RecordC)),
+hasTargetDecl(equalsNode(ConstrA);
+EXPECT_EQ(ShadowBA->getTargetDecl(), ConstrA);
+EXPECT_EQ(ShadowBA->getNominatedBaseClass(), RecordA);
+EXPECT_EQ(ShadowBA->getConstructedBaseClass(), RecordA);
+EXPECT_EQ(ShadowBA->getNominatedBaseClassShadowDecl(), nullptr);
+EXPECT_EQ(ShadowBA->getConstructedBaseClassShadowDecl(), nullptr);
+EXPECT_FALSE(ShadowBA->constructsVirtualBase());
+EXPECT_EQ(ShadowCA->getTargetDecl(), ConstrA);
+EXPECT_EQ(ShadowCA->getNominatedBaseCla

[PATCH] D110398: [clang][ASTImporter] Import ConstructorUsingShadowDecl correctly.

[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 rGcad9ff531c71: [clang][ASTImporter] Import 
ConstructorUsingShadowDecl correctly. (authored by balazske).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110398

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
@@ -6884,15 +6884,14 @@
 TEST_P(ASTImporterOptionSpecificTestBase, ImportIsInheritingConstructorBit) {
   Decl *FromTU = getTuDecl(
   R"(
-  class a {
-  public:
-a(int);
+  struct A {
+A(int);
   };
-  struct b : a {
-using a::a; // Ihnerited ctor.
+  struct B : A {
+using A::A; // Inherited ctor.
   };
-  void c() {
-(b(0));
+  void f() {
+(B(0));
   }
   )",
   Lang_CXX11);
@@ -6906,6 +6905,220 @@
   EXPECT_TRUE(ToD->isInheritingConstructor());
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportConstructorUsingShadow) {
+  TranslationUnitDecl *FromTU = getTuDecl(
+  R"(
+  struct A {
+A(int, int);
+  };
+  struct B : A {
+using A::A;
+  };
+  struct C : B {
+using B::B;
+  };
+  )",
+  Lang_CXX11);
+
+  auto CheckAST = [](TranslationUnitDecl *TU, CXXRecordDecl *RecordC) {
+auto *RecordA = FirstDeclMatcher().match(
+TU, cxxRecordDecl(hasName("A")));
+auto *RecordB = FirstDeclMatcher().match(
+TU, cxxRecordDecl(hasName("B")));
+auto *ConstrA = FirstDeclMatcher().match(
+TU, cxxConstructorDecl(hasParent(equalsNode(RecordA)),
+   parameterCountIs(2)));
+auto *ShadowBA = cast(
+FirstDeclMatcher().match(
+TU, usingShadowDecl(hasParent(equalsNode(RecordB)),
+hasTargetDecl(equalsNode(ConstrA);
+auto *ShadowCA = cast(
+FirstDeclMatcher().match(
+TU, usingShadowDecl(hasParent(equalsNode(RecordC)),
+hasTargetDecl(equalsNode(ConstrA);
+EXPECT_EQ(ShadowBA->getTargetDecl(), ConstrA);
+EXPECT_EQ(ShadowBA->getNominatedBaseClass(), RecordA);
+EXPECT_EQ(ShadowBA->getConstructedBaseClass(), RecordA);
+EXPECT_EQ(ShadowBA->getNominatedBaseClassShadowDecl(), nullptr);
+EXPECT_EQ(ShadowBA->getConstructedBaseClassShadowDecl(), nullptr);
+EXPECT_FALSE(ShadowBA->constructsVirtualBase());
+EXPECT_EQ(ShadowCA->getTargetDecl(), ConstrA);
+EXPECT_EQ(ShadowCA->getNominatedBaseClass(), RecordB);
+EXPECT_EQ(ShadowCA->getConstructedBaseClass(), RecordB);
+EXPECT_EQ(ShadowCA->getNominatedBaseClassShadowDecl(), ShadowBA);
+EXPECT_EQ(ShadowCA->getConstructedBaseClassShadowDecl(), ShadowBA);
+EXPECT_FALSE(ShadowCA->constructsVirtualBase());
+  };
+
+  auto *FromC = FirstDeclMatcher().match(
+  FromTU, cxxRecordDecl(hasName("C")));
+
+  auto *ToC = Import(FromC, Lang_CXX11);
+  TranslationUnitDecl *ToTU = ToC->getTranslationUnitDecl();
+
+  CheckAST(FromTU, FromC);
+  CheckAST(ToTU, ToC);
+}
+
+AST_MATCHER_P(UsingShadowDecl, hasIntroducerDecl, internal::Matcher,
+  InnerMatcher) {
+  return InnerMatcher.matches(*Node.getIntroducer(), Finder, Builder);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase,
+   ImportConstructorUsingShadowVirtualBase) {
+  TranslationUnitDecl *FromTU = getTuDecl(
+  R"(
+  struct A { A(int, int); };
+  struct B : A { using A::A; };
+
+  struct V1 : virtual B { using B::B; };
+  struct V2 : virtual B { using B::B; };
+
+  struct D2 : V1, V2 {
+using V1::V1;
+using V2::V2;
+  };
+  )",
+  Lang_CXX11);
+
+  auto CheckAST = [](TranslationUnitDecl *TU, CXXRecordDecl *RecordD2) {
+auto *RecordA = FirstDeclMatcher().match(
+TU, cxxRecordDecl(hasName("A")));
+auto *RecordB = FirstDeclMatcher().match(
+TU, cxxRecordDecl(hasName("B")));
+auto *RecordV1 = FirstDeclMatcher().match(
+TU, cxxRecordDecl(hasName("V1")));
+auto *RecordV2 = FirstDeclMatcher().match(
+TU, cxxRecordDecl(hasName("V2")));
+auto *ConstrA = FirstDeclMatcher().match(
+TU, cxxConstructorDecl(hasParent(equalsNode(RecordA)),
+   parameterCountIs(2)));
+auto *ConstrB = FirstDeclMatcher().match(
+TU, cxxConstructorDecl(hasParent(equalsNode(RecordB)),
+   isCopyConstructor()));
+auto *UsingD2V1 = FirstDeclMatcher().match(
+TU, usingDecl(hasParent(equalsNode(RecordD2;
+auto *UsingD2V2 = LastDeclMatcher().match(
+TU, usingDecl(hasParent(equalsNode(RecordD2;
+auto *ShadowBA = cast

[PATCH] D110911: [analyzer][NFC] Add RangeSet::dump

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

steakhal added a comment.

Please mark both of them `LLVM_DUMP_METHOD`s. This way they will be stripped 
from release builds according to their documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110911

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

[PATCH] D110913: [analyzer][solver] Handle simplification to ConcreteInt

[Gabor Marton via Phabricator via cfe-commits]

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

The solver's symbol simplification mechanism was not able to handle cases
when a symbol is simplified to a concrete integer. This patch adds the
capability.

E.g., in the attached lit test case, the original symbol is `c + 1` and
it has a `[0, 0]` range associated with it. Then, a new condition `c == 0`
is assumed, so a new range constraint `[0, 0]` comes in for `c` and
simplification kicks in. `c + 1` becomes `0 + 1`, but the associated
range is `[0, 0]`, so now we are able to realize the contradiction.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110913

Files:
  
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  clang/test/Analysis/solver-sym-simplification-concreteint.c


Index: clang/test/Analysis/solver-sym-simplification-concreteint.c
===
--- /dev/null
+++ clang/test/Analysis/solver-sym-simplification-concreteint.c
@@ -0,0 +1,30 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -verify
+
+void clang_analyzer_warnIfReached();
+void clang_analyzer_eval();
+
+void test_simplification_to_concrete_int(int b, int c) {
+  if (b < 0 || b > 1) // b: [0,1]
+return;
+  if (c < 0 || c > 1) // c: [0,1]
+return;
+  if (c + b != 0) // c + b == 0
+return;
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (b != 1) // b == 1  --> c + 1 == 0
+return;
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+
+  // c == 0 --> 0 + 1 == 0 contradiction
+  clang_analyzer_eval(c == 0);// expected-warning{{FALSE}}
+
+  // c == 1 --> 1 + 1 == 0 contradiction
+  clang_analyzer_eval(c != 0);// expected-warning{{FALSE}}
+
+  // Keep the symbols and the constraints! alive.
+  (void)(b * c);
+  return;
+}
Index: clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
@@ -226,9 +226,13 @@
   }
 }
 
-SymbolRef simplify(ProgramStateRef State, SymbolRef Sym) {
+SVal simplifyToSVal(ProgramStateRef State, SymbolRef Sym) {
   SValBuilder &SVB = State->getStateManager().getSValBuilder();
-  SVal SimplifiedVal = SVB.simplifySVal(State, SVB.makeSymbolVal(Sym));
+  return SVB.simplifySVal(State, SVB.makeSymbolVal(Sym));
+}
+
+SymbolRef simplify(ProgramStateRef State, SymbolRef Sym) {
+  SVal SimplifiedVal = simplifyToSVal(State, Sym);
   if (SymbolRef SimplifiedSym = SimplifiedVal.getAsSymbol())
 return SimplifiedSym;
   return Sym;
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -2096,7 +2096,19 @@
ProgramStateRef State, EquivalenceClass Class) {
   SymbolSet ClassMembers = Class.getClassMembers(State);
   for (const SymbolRef &MemberSym : ClassMembers) {
-SymbolRef SimplifiedMemberSym = ento::simplify(State, MemberSym);
+
+SymbolRef SimplifiedMemberSym = nullptr;
+SVal SimplifiedMemberVal = simplifyToSVal(State, MemberSym);
+if (const auto CI = SimplifiedMemberVal.getAs()) {
+  const llvm::APSInt &SV = CI->getValue();
+  const RangeSet *ClassConstraint = getConstraint(State, Class);
+  // We have found a contradiction.
+  if (ClassConstraint && !ClassConstraint->contains(SV))
+return nullptr;
+} else {
+  SimplifiedMemberSym = SimplifiedMemberVal.getAsSymbol();
+}
+
 if (SimplifiedMemberSym && MemberSym != SimplifiedMemberSym) {
   // The simplified symbol should be the member of the original Class,
   // however, it might be in another existing class at the moment. We
Index: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
===
--- 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
+++ 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
@@ -390,6 +390,9 @@
 /// Try to simplify a given symbolic expression's associated value based on the
 /// constraints in State. This is needed because the Environment bind

[PATCH] D110745: Redefine deref(N) attribute family as point-in-time semantics (aka deref-at-point)

[Nuno Lopes via Phabricator via cfe-commits]

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

LGTM. We have discussed this before. It's important to fix the reference types 
in C++.




Comment at: llvm/test/Analysis/BasicAA/dereferenceable.ll:66
 ; CHECK: Function: local_and_deref_ret_2: 2 pointers, 2 call sites
-; CHECK-NEXT:  NoAlias:i32* %obj, i32* %ret
+; CHECK-NEXT:  MayAlias:   i32* %obj, i32* %ret
 bb:

this one is unfortunate.. They can't alias because objsize(%obj) < 
dereferenceable size of %ret. This case should be trivial to catch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110745

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

[PATCH] D110625: [analyzer] canonicalize special case of structure/pointer deref

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

steakhal added a comment.

In D110625#3035843 , @ASDenysPetrov 
wrote:

> In D110625#3035616 , @steakhal 
> wrote:
>
>> WDYT Denys? Btw does the SVval::getType return a canonical type in all cases?
>
> `SVal::getType` returns a `QualType` which can be aliased and cv-qualified. 
> So, it may mismatch in comparison `BT->getPointeeType() == elementType`. That 
> is my concern.

I thought that `SVal::getType` should return an already canonical `QualType`. 
If it doesn't do that we would need to do canonicalization at each callsite, 
which is less than ideal IMO.
If it's not yet canonical, we should probably canonicalize within the 
`getType()` function probably. WDYT?

---

In D110625#3035866 , @vabridgers 
wrote:

> I moved the test cases from ptr-arith.cpp to ptr-arith - using to typedef, 
> and created an extra test case that uses const. I believe this may address 
> all concerns discussed thus far? Please let me know if we need anything more 
> and I'll get it done. Best!

I think your change is good, but we need to sort some things out before moving 
forward with that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110625

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

[PATCH] D110065: [AArch64] Add support for the 'R' architecture profile.

[Tomas Matheson via Phabricator via cfe-commits]

tmatheson added inline comments.



Comment at: clang/lib/Basic/Targets/AArch64.h:62
   std::string ABI;
+  StringRef getArchProfile() const;
 

The equivalent in the ARM backend is named `getCPUProfile`


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

https://reviews.llvm.org/D110065

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

[PATCH] D110065: [AArch64] Add support for the 'R' architecture profile.

[Tomas Matheson via Phabricator via cfe-commits]

tmatheson added inline comments.



Comment at: clang/lib/Basic/Targets/AArch64.h:62
   std::string ABI;
+  StringRef getArchProfile() const;
 

tmatheson wrote:
> The equivalent in the ARM backend is named `getCPUProfile`
That's arguably a worse name though.


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

https://reviews.llvm.org/D110065

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

[PATCH] D110911: [analyzer][NFC] Add RangeSet::dump

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D110911#3035911 , @steakhal wrote:

> Please mark both of them `LLVM_DUMP_METHOD`s. This way they will be stripped 
> from release builds according to their documentation.

And `Range::dump` as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110911

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

[PATCH] D110913: [analyzer][solver] Handle simplification to ConcreteInt

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

steakhal added a comment.

The patch seems reasonable, but I will need slightly more time to digest it.
I'll get back to this.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:390-396
 /// Try to simplify a given symbolic expression's associated value based on the
 /// constraints in State. This is needed because the Environment bindings are
 /// not getting updated when a new constraint is added to the State.
+SVal simplifyToSVal(ProgramStateRef State, SymbolRef Sym);
+/// If the symbol is simplified to a constant then it returns the original
+/// symbol.
 SymbolRef simplify(ProgramStateRef State, SymbolRef Sym);

I'm confused about which comment corresponds to which function.
You should also signify the difference between the two APIs.

Shouldn't be one of them an implementation detail? If so, why do we have the 
same visibility?



Comment at: clang/test/Analysis/solver-sym-simplification-concreteint.c:22
+  // c == 0 --> 0 + 1 == 0 contradiction
+  clang_analyzer_eval(c == 0);// expected-warning{{FALSE}}
+

Could we have an `eval(c == -1) // TRUE`?
Also, please disable eager bifurcation to prevent state-splits in the eval 
arguments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110913

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

[PATCH] D110833: [clang-format] Add ControlStatementsAndFunctionDefinitionsExceptControlMacros option to SpaceBeforeParens

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:3649
 ``SBPO_ControlStatementsExceptForEachMacros`` remains an alias for
 backward compatibility.
 

Now I look back here, why where these Macro considered the same as for loops? 
why would we want

```
for ()
Q_FOREACH(...)
```

So this really does need a struct or we'll be eventually be adding

`SBPO_ControlStatementsAndFunctionDefinitionsExceptControlMacrosButNotIfAndDefinatelyWhilesAndSometimesSwitchButOnlyOnTheSecondThursdayOfTheMonth`

```
SpaceBeforeParen:
 AfterFunctionDefinitionName: false
 AfterFunctionDeclarationName: true
 AfterSwitch: true
 AfterForeachMacros: false
  (there are likely others)
```
 
`







Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110833

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

[PATCH] D110833: [clang-format] Add ControlStatementsAndFunctionDefinitionsExceptControlMacros option to SpaceBeforeParens

[Christian Rayroud via Phabricator via cfe-commits]

crayroud added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:3649
 ``SBPO_ControlStatementsExceptForEachMacros`` remains an alias for
 backward compatibility.
 

MyDeveloperDay wrote:
> Now I look back here, why where these Macro considered the same as for loops? 
> why would we want
> 
> ```
> for ()
> Q_FOREACH(...)
> ```
> 
> So this really does need a struct or we'll be eventually be adding
> 
> `SBPO_ControlStatementsAndFunctionDefinitionsExceptControlMacrosButNotIfAndDefinatelyWhilesAndSometimesSwitchButOnlyOnTheSecondThursdayOfTheMonth`
> 
> ```
> SpaceBeforeParen:
>  AfterFunctionDefinitionName: false
>  AfterFunctionDeclarationName: true
>  AfterSwitch: true
>  AfterForeachMacros: false
>   (there are likely others)
> ```
>  
> `
> 
> 
> 
> 
> 
Indeed replacing the enum with a struct as suggested is better to support the 
different possible combinations, compare to the current version of 
SpaceBeforeParens that results in very long names.

To support existing configuration files, I propose to keep the enum and to add 
a struct to handle the custom use cases and to cleanup the code. What do you 
think ?

```
SpaceBeforeParens: Custom
SpaceBeforeParensCustom:
 AfterFunctionDefinitionName: false
 AfterFunctionDeclarationName: true
 AfterSwitch: true
 AfterForeachMacros: false
…
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110833

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

[PATCH] D110625: [analyzer] canonicalize special case of structure/pointer deref

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D110625#3035929 , @steakhal wrote:

> I thought that `SVal::getType` should return an already canonical `QualType`. 
> If it doesn't do that we would need to do canonicalization at each callsite, 
> which is less than ideal IMO.
> If it's not yet canonical, we should probably canonicalize within the 
> `getType()` function probably. WDYT?

I believe that it returns non-canonical type. Yes, we shall get canonical 
versions separately for most cases in practice. This is not ideal but it would 
be worse to return canonical types at once along with loss of extra information.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110625

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

[PATCH] D110745: Redefine deref(N) attribute family as point-in-time semantics (aka deref-at-point)

[Nikita Popov via Phabricator via cfe-commits]

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

Sorry, but the fact that there is still no way to opt-in to the old behavior is 
still a blocker from my side. If we can't use `dereferenceable + nofree` 
arguments for that purpose, then we need to provide a different way to do that. 
Like `dereferenceable + really_nofree`. It looks like the current 
implementation doesn't even accept the `dereferenceable + nofree + noalias` 
case originally proposed (which is pretty bad from a design perspective, but 
would at least work fairly well for rustc in practice). I don't think that our 
current analysis capabilities are sufficient to land this change at this time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110745

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

[PATCH] D110625: [analyzer] canonicalize special case of structure/pointer deref

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

steakhal added a comment.

In D110625#3035974 , @ASDenysPetrov 
wrote:

> In D110625#3035929 , @steakhal 
> wrote:
>
>> I thought that `SVal::getType` should return an already canonical 
>> `QualType`. If it doesn't do that we would need to do canonicalization at 
>> each callsite, which is less than ideal IMO.
>> If it's not yet canonical, we should probably canonicalize within the 
>> `getType()` function probably. WDYT?
>
> I believe that it returns non-canonical type. Yes, we shall get canonical 
> versions separately for most cases in practice. This is not ideal but it 
> would be worse to return canonical types at once along with loss of extra 
> information.

Okay, if this is the case, we will need to use `getCanonicalType()` on both 
`BT` and `elementType`.
Vince, could you update your change, just to make sure it will work in the 
future, even if the test doesn't show much difference with the current behavior?
Note that you cannot call `getCanonicalType()` on a null-type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110625

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

[clang-tools-extra] 512aa84 - [clangd] Handle members of anon structs in SelectionTree

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2021-10-01T12:38:18+02:00
New Revision: 512aa8485010009f6ec1b8d9deea3effe67e0106

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

LOG: [clangd] Handle members of anon structs in SelectionTree

References to fields inside anon structs contain an implicit children
for the container, which has the same SourceLocation with the field.
This was resulting in SelectionTree always picking the anon-struct rather than
the field as the selection.

This patch prevents that by claiming the range for the field early.

https://github.com/clangd/clangd/issues/877.

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

Added: 


Modified: 
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Selection.cpp 
b/clang-tools-extra/clangd/Selection.cpp
index e3a2e31997669..a53673e074804 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -443,6 +443,15 @@ bool isImplicit(const Stmt *S) {
   if (auto *CTI = llvm::dyn_cast(S))
 if (CTI->isImplicit())
   return true;
+  // Make sure implicit access of anonymous structs don't end up owning tokens.
+  if (auto *ME = llvm::dyn_cast(S)) {
+if (auto *FD = llvm::dyn_cast(ME->getMemberDecl()))
+  if (FD->isAnonymousStructOrUnion())
+// If Base is an implicit CXXThis, then the whole MemberExpr has no
+// tokens. If it's a normal e.g. DeclRef, we treat the MemberExpr like
+// an implicit cast.
+return isImplicit(ME->getBase());
+  }
   // Refs to operator() and [] are (almost?) always implicit as part of calls.
   if (auto *DRE = llvm::dyn_cast(S)) {
 if (auto *FD = llvm::dyn_cast(DRE->getDecl())) {

diff  --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp 
b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index 6a9d355792a67..e8b64e9200bb5 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -365,6 +365,27 @@ TEST(LocateSymbol, WithIndex) {
   ElementsAre(Sym("Forward", SymbolHeader.range("forward"), 
Test.range(;
 }
 
+TEST(LocateSymbol, AnonymousStructFields) {
+  auto Code = Annotations(R"cpp(
+struct $2[[Foo]] {
+  struct { int $1[[x]]; };
+  void foo() {
+// Make sure the implicit base is skipped.
+$1^x = 42;
+  }
+};
+// Check that we don't skip explicit bases.
+int a = $2^Foo{}.x;
+  )cpp");
+  TestTU TU = TestTU::withCode(Code.code());
+  auto AST = TU.build();
+  EXPECT_THAT(locateSymbolAt(AST, Code.point("1"), TU.index().get()),
+  UnorderedElementsAre(Sym("x", Code.range("1"), 
Code.range("1";
+  EXPECT_THAT(
+  locateSymbolAt(AST, Code.point("2"), TU.index().get()),
+  UnorderedElementsAre(Sym("Foo", Code.range("2"), Code.range("2";
+}
+
 TEST(LocateSymbol, FindOverrides) {
   auto Code = Annotations(R"cpp(
 class Foo {



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

[PATCH] D110825: [clangd] Handle members of anon structs in SelectionTree

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG512aa8485010: [clangd] Handle members of anon structs in 
SelectionTree (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110825

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -365,6 +365,27 @@
   ElementsAre(Sym("Forward", SymbolHeader.range("forward"), 
Test.range(;
 }
 
+TEST(LocateSymbol, AnonymousStructFields) {
+  auto Code = Annotations(R"cpp(
+struct $2[[Foo]] {
+  struct { int $1[[x]]; };
+  void foo() {
+// Make sure the implicit base is skipped.
+$1^x = 42;
+  }
+};
+// Check that we don't skip explicit bases.
+int a = $2^Foo{}.x;
+  )cpp");
+  TestTU TU = TestTU::withCode(Code.code());
+  auto AST = TU.build();
+  EXPECT_THAT(locateSymbolAt(AST, Code.point("1"), TU.index().get()),
+  UnorderedElementsAre(Sym("x", Code.range("1"), 
Code.range("1";
+  EXPECT_THAT(
+  locateSymbolAt(AST, Code.point("2"), TU.index().get()),
+  UnorderedElementsAre(Sym("Foo", Code.range("2"), Code.range("2";
+}
+
 TEST(LocateSymbol, FindOverrides) {
   auto Code = Annotations(R"cpp(
 class Foo {
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -443,6 +443,15 @@
   if (auto *CTI = llvm::dyn_cast(S))
 if (CTI->isImplicit())
   return true;
+  // Make sure implicit access of anonymous structs don't end up owning tokens.
+  if (auto *ME = llvm::dyn_cast(S)) {
+if (auto *FD = llvm::dyn_cast(ME->getMemberDecl()))
+  if (FD->isAnonymousStructOrUnion())
+// If Base is an implicit CXXThis, then the whole MemberExpr has no
+// tokens. If it's a normal e.g. DeclRef, we treat the MemberExpr like
+// an implicit cast.
+return isImplicit(ME->getBase());
+  }
   // Refs to operator() and [] are (almost?) always implicit as part of calls.
   if (auto *DRE = llvm::dyn_cast(S)) {
 if (auto *FD = llvm::dyn_cast(DRE->getDecl())) {


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -365,6 +365,27 @@
   ElementsAre(Sym("Forward", SymbolHeader.range("forward"), Test.range(;
 }
 
+TEST(LocateSymbol, AnonymousStructFields) {
+  auto Code = Annotations(R"cpp(
+struct $2[[Foo]] {
+  struct { int $1[[x]]; };
+  void foo() {
+// Make sure the implicit base is skipped.
+$1^x = 42;
+  }
+};
+// Check that we don't skip explicit bases.
+int a = $2^Foo{}.x;
+  )cpp");
+  TestTU TU = TestTU::withCode(Code.code());
+  auto AST = TU.build();
+  EXPECT_THAT(locateSymbolAt(AST, Code.point("1"), TU.index().get()),
+  UnorderedElementsAre(Sym("x", Code.range("1"), Code.range("1";
+  EXPECT_THAT(
+  locateSymbolAt(AST, Code.point("2"), TU.index().get()),
+  UnorderedElementsAre(Sym("Foo", Code.range("2"), Code.range("2";
+}
+
 TEST(LocateSymbol, FindOverrides) {
   auto Code = Annotations(R"cpp(
 class Foo {
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -443,6 +443,15 @@
   if (auto *CTI = llvm::dyn_cast(S))
 if (CTI->isImplicit())
   return true;
+  // Make sure implicit access of anonymous structs don't end up owning tokens.
+  if (auto *ME = llvm::dyn_cast(S)) {
+if (auto *FD = llvm::dyn_cast(ME->getMemberDecl()))
+  if (FD->isAnonymousStructOrUnion())
+// If Base is an implicit CXXThis, then the whole MemberExpr has no
+// tokens. If it's a normal e.g. DeclRef, we treat the MemberExpr like
+// an implicit cast.
+return isImplicit(ME->getBase());
+  }
   // Refs to operator() and [] are (almost?) always implicit as part of calls.
   if (auto *DRE = llvm::dyn_cast(S)) {
 if (auto *FD = llvm::dyn_cast(DRE->getDecl())) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109740: [OpenCL] Add atomic_half type builtins

[Yang Haonan via Phabricator via cfe-commits]

haonanya updated this revision to Diff 376467.
haonanya added a comment.

Fix format issue


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

https://reviews.llvm.org/D109740

Files:
  clang/lib/Headers/opencl-c.h
  clang/lib/Sema/OpenCLBuiltins.td
  clang/lib/Sema/Sema.cpp
  clang/test/SemaOpenCL/atomic-ops.cl

Index: clang/test/SemaOpenCL/atomic-ops.cl
===
--- clang/test/SemaOpenCL/atomic-ops.cl
+++ clang/test/SemaOpenCL/atomic-ops.cl
@@ -19,7 +19,7 @@
 
 atomic_int gn;
 void f(atomic_int *i, const atomic_int *ci,
-   atomic_intptr_t *p, atomic_float *f, atomic_double *d, atomic_half *h, // expected-error {{unknown type name 'atomic_half'}}
+   atomic_intptr_t *p, atomic_float *f, atomic_double *d, atomic_half *h,
int *I, const int *CI,
intptr_t *P, float *D, struct S *s1, struct S *s2,
global atomic_int *i_g, local atomic_int *i_l, private atomic_int *i_p,
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -367,6 +367,11 @@
 AddPointerSizeDependentTypes();
   }
 
+  if (getOpenCLOptions().isSupported("cl_khr_fp16", getLangOpts())) {
+auto AtomicHalfT = Context.getAtomicType(Context.HalfTy);
+addImplicitTypedef("atomic_half", AtomicHalfT);
+  }
+
   std::vector Atomic64BitTypes;
   if (getOpenCLOptions().isSupported("cl_khr_int64_base_atomics",
  getLangOpts()) &&
Index: clang/lib/Sema/OpenCLBuiltins.td
===
--- clang/lib/Sema/OpenCLBuiltins.td
+++ clang/lib/Sema/OpenCLBuiltins.td
@@ -85,16 +85,25 @@
 
 def FuncExtOpenCLCPipes  : FunctionExtension<"__opencl_c_pipes">;
 def FuncExtOpenCLCWGCollectiveFunctions  : FunctionExtension<"__opencl_c_work_group_collective_functions">;
+def FuncExtFloatAtomicsFp16GlobalLoadStore  : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp16_global_atomic_load_store">;
+def FuncExtFloatAtomicsFp16LocalLoadStore   : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp16_local_atomic_load_store">;
+def FuncExtFloatAtomicsFp16GenericLoadStore : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp16_global_atomic_load_store __opencl_c_ext_fp16_local_atomic_load_store">;
+def FuncExtFloatAtomicsFp16GlobalAdd : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp16_global_atomic_add">;
 def FuncExtFloatAtomicsFp32GlobalAdd : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp32_global_atomic_add">;
 def FuncExtFloatAtomicsFp64GlobalAdd : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp64_global_atomic_add">;
+def FuncExtFloatAtomicsFp16LocalAdd  : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp16_local_atomic_add">;
 def FuncExtFloatAtomicsFp32LocalAdd  : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp32_local_atomic_add">;
 def FuncExtFloatAtomicsFp64LocalAdd  : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp64_local_atomic_add">;
+def FuncExtFloatAtomicsFp16GenericAdd: FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp16_local_atomic_add __opencl_c_ext_fp16_global_atomic_add">;
 def FuncExtFloatAtomicsFp32GenericAdd: FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp32_local_atomic_add __opencl_c_ext_fp32_global_atomic_add">;
 def FuncExtFloatAtomicsFp64GenericAdd: FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp64_local_atomic_add __opencl_c_ext_fp64_global_atomic_add">;
+def FuncExtFloatAtomicsFp16GlobalMinMax  : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp16_global_atomic_min_max">;
 def FuncExtFloatAtomicsFp32GlobalMinMax  : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp32_global_atomic_min_max">;
 def FuncExtFloatAtomicsFp64GlobalMinMax  : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp64_global_atomic_min_max">;
+def FuncExtFloatAtomicsFp16LocalMinMax   : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp16_local_atomic_min_max">;
 def FuncExtFloatAtomicsFp32LocalMinMax   : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp32_local_atomic_min_max">;
 def FuncExtFloatAtomicsFp64LocalMinMax   : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp64_local_atomic_min_max">;
+def FuncExtFloatAtomicsFp16GenericMinMax : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp16_local_atomic_min_max __opencl_c_ext_fp16_global_atomic_min_max">;
 def FuncExtFloatAtomicsFp32GenericMinMax : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp32_local_atomic_min_max __opencl_c_ext_fp32_global_atomic_min_max">;
 def FuncExtFloatAtomicsFp64GenericMinMax : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp64_local_atomic_min_max __opencl_c_ext_fp64_global_atomic_min_max">;
 
@@ -362,6 +371,7 @@
 def AtomicULong   

[PATCH] D110833: [clang-format] Add ControlStatementsAndFunctionDefinitionsExceptControlMacros option to SpaceBeforeParens

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

The use of "Custom" would kind of match the BraceWrapping, as for 
`SpaceBeforeParensCustom` either that or `SpaceBeforeParensStyles` , 
`SpaceBeforeParensSettings`, `SpaceBeforeParensOptions` I guess I don't really 
mind, (I always find naming things hard!) maybe the others might chip in on 
their preference of names

  BreakBeforeBraces: Custom
  BraceWrapping:
  AfterClass: true
  AfterControlStatement: false
  AfterEnum: true
  BeforeElse: true
  BeforeCatch: true
  AfterFunction: true
  AfterNamespace: true
  IndentBraces: false
  AfterStruct: true
  AfterUnion: true


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110833

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

[PATCH] D110833: [clang-format] Add ControlStatementsAndFunctionDefinitionsExceptControlMacros option to SpaceBeforeParens

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Just adding a few more clang-format big guns as reviewers, its probably good to 
get some more input before embarking on what is going to probably be a 
reasonably sized change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110833

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

RE: [llvm-dev] Phabricator Creator Pulling the Plug

[Chris Tetreault via cfe-commits]

As I, and others have noticed, it seems that as of today, there’s some 
certificate issue with arcanist. (See: 
https://lists.llvm.org/pipermail/llvm-dev/2021-September/153019.html) The fix 
seems simple, and a PR is up, but looking through the PR activity, it seems 
that the PR will not be accepted because Phabricator is no longer being 
maintained. It seems that arc has become the first casualty of the 
discontinuation of maintenance of phabricator.

I know that arc is not universally used, but I think it’s a serious blow to 
many people’s workflows. I think that MyDeveloperDay’s question might have just 
become a bit more urgent.

I suppose in the short-term, we could fork the phabricator repos in order to 
fix little issues like this. Alternately, we should probably stop recommending 
arcanist (unless we want to provide instructions on how to fix any breakages 
that come along).

Thanks,
   Chris Tetreault

From: llvm-dev  On Behalf Of MyDeveloper Day 
via llvm-dev
Sent: Wednesday, August 18, 2021 10:17 AM
To: llvm-dev ; cfe-commits 
Subject: [llvm-dev] Phabricator Creator Pulling the Plug


WARNING: This email originated from outside of Qualcomm. Please be wary of any 
links or attachments, and do not enable macros.
All

I'm a massive fan of Phabricator, and I know there is often lots of contentious 
discussion about its relative merits vs github,

But unless I missed this, was there any discussion regarding the recent 
"Winding Down" announcement of Phabricator? and what it might mean for us in 
LLVM

See:
https://admin.phacility.com/phame/post/view/11/phacility_is_winding_down_operations/
https://www.phacility.com/phabricator/

Personally I'm excited by the concept of a community driven replacement ( 
https://we.phorge.it/) .
epriestley did a truly amazing job, it wasn't open to public contributions. 
Perhaps more open development could lead to closing some of the github gaps 
that were of concern.

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

Re: [llvm-dev] Phabricator Creator Pulling the Plug

[Brian Cain via cfe-commits]

How far are we from a workflow that leverages Github's Pull Requests?  Is
there some consensus that it's a desired end goal, but some features are
missing?  Or do we prefer to use a workflow like this for the long term?

On Thu, Sep 30, 2021, 4:54 PM Chris Tetreault via llvm-dev <
llvm-...@lists.llvm.org> wrote:

> As I, and others have noticed, it seems that as of today, there’s some
> certificate issue with arcanist. (See:
> https://lists.llvm.org/pipermail/llvm-dev/2021-September/153019.html) The
> fix seems simple, and a PR is up, but looking through the PR activity, it
> seems that the PR will not be accepted because Phabricator is no longer
> being maintained. It seems that arc has become the first casualty of the
> discontinuation of maintenance of phabricator.
>
>
>
> I know that arc is not universally used, but I think it’s a serious blow
> to many people’s workflows. I think that MyDeveloperDay’s question might
> have just become a bit more urgent.
>
>
>
> I suppose in the short-term, we could fork the phabricator repos in order
> to fix little issues like this. Alternately, we should probably stop
> recommending arcanist (unless we want to provide instructions on how to fix
> any breakages that come along).
>
>
>
> Thanks,
>
>Chris Tetreault
>
>
>
> *From:* llvm-dev  *On Behalf Of *MyDeveloper
> Day via llvm-dev
> *Sent:* Wednesday, August 18, 2021 10:17 AM
> *To:* llvm-dev ; cfe-commits <
> cfe-commits@lists.llvm.org>
> *Subject:* [llvm-dev] Phabricator Creator Pulling the Plug
>
>
>
> *WARNING:* This email originated from outside of Qualcomm. Please be wary
> of any links or attachments, and do not enable macros.
>
> All
>
>
>
> I'm a massive fan of Phabricator, and I know there is often lots of
> contentious discussion about its relative merits vs github,
>
>
>
> But unless I missed this, was there any discussion regarding the recent
> "Winding Down" announcement of Phabricator? and what it might mean for us
> in LLVM
>
>
>
> See:
>
>
> https://admin.phacility.com/phame/post/view/11/phacility_is_winding_down_operations/
>
> https://www.phacility.com/phabricator/
>
>
>
> Personally I'm excited by the concept of a community driven replacement (
> https://we.phorge.it/) .
>
> epriestley did a truly amazing job, it wasn't open to public
> contributions. Perhaps more open development could lead to closing some of
> the github gaps that were of concern.
>
>
>
> MyDeveloperDay
> ___
> LLVM Developers mailing list
> llvm-...@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [llvm-dev] Phabricator Creator Pulling the Plug

[Brian Cain via cfe-commits]

Does something like Rust's "bors" bot satisfy the herald rules need?

re: #2 I have done this on GHE and it's mildly awkward but it does work.

And yes normalizing force pushes is the unfortunate state of GitHub PRs.
Comments are preserved. Code-anchored comments like review comments are
marked as referring to out-of-date code, IIRC.

On Thu, Sep 30, 2021, 5:56 PM Mehdi AMINI  wrote:

> We talked about this with the IWG (Infrastructure Working Group) just
> last week coincidentally.
> Two major blocking tracks that were identified at the roundtable
> during the LLVM Dev Meeting exactly 2 years ago are still an issue
> today:
>
> 1) Replacement for Herald rules. This is what allows us to subscribe
> and track new revisions or commits based on paths in the repo or other
> criteria. We could build a replacement based on GitHub action or any
> other kind of service, but this is a bit tricky (how do you store
> emails privately? etc.). I have looked around online but I didn't find
> another OSS project (or external company) providing a similar service
> for GitHub unfortunately, does anyone know of any?
>
> 2) Support for stacked commits. I can see how to structure this
> somehow assuming we would push pull-request branches in the main repo
> (with one new commit per branch and cascading the pull-requests from
> one branch to the other), otherwise this will be a major regression
> compared to the current workflow.
>
> What remains unknown to me is the current state of GitHub management
> of comments across `git commit --amend` and force push to update a
> branch.
>
> Others may have other items to add!
>
> --
> Mehdi
>
> On Thu, Sep 30, 2021 at 3:39 PM Brian Cain via llvm-dev
>  wrote:
> >
> > How far are we from a workflow that leverages Github's Pull Requests?
> Is there some consensus that it's a desired end goal, but some features are
> missing?  Or do we prefer to use a workflow like this for the long term?
> >
> > On Thu, Sep 30, 2021, 4:54 PM Chris Tetreault via llvm-dev <
> llvm-...@lists.llvm.org> wrote:
> >>
> >> As I, and others have noticed, it seems that as of today, there’s some
> certificate issue with arcanist. (See:
> https://lists.llvm.org/pipermail/llvm-dev/2021-September/153019.html) The
> fix seems simple, and a PR is up, but looking through the PR activity, it
> seems that the PR will not be accepted because Phabricator is no longer
> being maintained. It seems that arc has become the first casualty of the
> discontinuation of maintenance of phabricator.
> >>
> >>
> >>
> >> I know that arc is not universally used, but I think it’s a serious
> blow to many people’s workflows. I think that MyDeveloperDay’s question
> might have just become a bit more urgent.
> >>
> >>
> >>
> >> I suppose in the short-term, we could fork the phabricator repos in
> order to fix little issues like this. Alternately, we should probably stop
> recommending arcanist (unless we want to provide instructions on how to fix
> any breakages that come along).
> >>
> >>
> >>
> >> Thanks,
> >>
> >>Chris Tetreault
> >>
> >>
> >>
> >> From: llvm-dev  On Behalf Of
> MyDeveloper Day via llvm-dev
> >> Sent: Wednesday, August 18, 2021 10:17 AM
> >> To: llvm-dev ; cfe-commits <
> cfe-commits@lists.llvm.org>
> >> Subject: [llvm-dev] Phabricator Creator Pulling the Plug
> >>
> >>
> >>
> >> WARNING: This email originated from outside of Qualcomm. Please be wary
> of any links or attachments, and do not enable macros.
> >>
> >> All
> >>
> >>
> >>
> >> I'm a massive fan of Phabricator, and I know there is often lots of
> contentious discussion about its relative merits vs github,
> >>
> >>
> >>
> >> But unless I missed this, was there any discussion regarding the recent
> "Winding Down" announcement of Phabricator? and what it might mean for us
> in LLVM
> >>
> >>
> >>
> >> See:
> >>
> >>
> https://admin.phacility.com/phame/post/view/11/phacility_is_winding_down_operations/
> >>
> >> https://www.phacility.com/phabricator/
> >>
> >>
> >>
> >> Personally I'm excited by the concept of a community driven replacement
> ( https://we.phorge.it/) .
> >>
> >> epriestley did a truly amazing job, it wasn't open to public
> contributions. Perhaps more open development could lead to closing some of
> the github gaps that were of concern.
> >>
> >>
> >>
> >> MyDeveloperDay
> >>
> >> ___
> >> LLVM Developers mailing list
> >> llvm-...@lists.llvm.org
> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> >
> > ___
> > LLVM Developers mailing list
> > llvm-...@lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [llvm-dev] Phabricator Creator Pulling the Plug

[Brian Cain via cfe-commits]

On Thu, Sep 30, 2021, 6:04 PM Brian Cain  wrote:

> Does something like Rust's "bors" bot satisfy the herald rules need?
>


sorry, maybe I was thinking of the high-five bot. And it looks like that's
not quite a match for herald.



> re: #2 I have done this on GHE and it's mildly awkward but it does work.
>
> And yes normalizing force pushes is the unfortunate state of GitHub PRs.
> Comments are preserved. Code-anchored comments like review comments are
> marked as referring to out-of-date code, IIRC.
>
> On Thu, Sep 30, 2021, 5:56 PM Mehdi AMINI  wrote:
>
>> We talked about this with the IWG (Infrastructure Working Group) just
>> last week coincidentally.
>> Two major blocking tracks that were identified at the roundtable
>> during the LLVM Dev Meeting exactly 2 years ago are still an issue
>> today:
>>
>> 1) Replacement for Herald rules. This is what allows us to subscribe
>> and track new revisions or commits based on paths in the repo or other
>> criteria. We could build a replacement based on GitHub action or any
>> other kind of service, but this is a bit tricky (how do you store
>> emails privately? etc.). I have looked around online but I didn't find
>> another OSS project (or external company) providing a similar service
>> for GitHub unfortunately, does anyone know of any?
>>
>> 2) Support for stacked commits. I can see how to structure this
>> somehow assuming we would push pull-request branches in the main repo
>> (with one new commit per branch and cascading the pull-requests from
>> one branch to the other), otherwise this will be a major regression
>> compared to the current workflow.
>>
>> What remains unknown to me is the current state of GitHub management
>> of comments across `git commit --amend` and force push to update a
>> branch.
>>
>> Others may have other items to add!
>>
>> --
>> Mehdi
>>
>> On Thu, Sep 30, 2021 at 3:39 PM Brian Cain via llvm-dev
>>  wrote:
>> >
>> > How far are we from a workflow that leverages Github's Pull Requests?
>> Is there some consensus that it's a desired end goal, but some features are
>> missing?  Or do we prefer to use a workflow like this for the long term?
>> >
>> > On Thu, Sep 30, 2021, 4:54 PM Chris Tetreault via llvm-dev <
>> llvm-...@lists.llvm.org> wrote:
>> >>
>> >> As I, and others have noticed, it seems that as of today, there’s some
>> certificate issue with arcanist. (See:
>> https://lists.llvm.org/pipermail/llvm-dev/2021-September/153019.html)
>> The fix seems simple, and a PR is up, but looking through the PR activity,
>> it seems that the PR will not be accepted because Phabricator is no longer
>> being maintained. It seems that arc has become the first casualty of the
>> discontinuation of maintenance of phabricator.
>> >>
>> >>
>> >>
>> >> I know that arc is not universally used, but I think it’s a serious
>> blow to many people’s workflows. I think that MyDeveloperDay’s question
>> might have just become a bit more urgent.
>> >>
>> >>
>> >>
>> >> I suppose in the short-term, we could fork the phabricator repos in
>> order to fix little issues like this. Alternately, we should probably stop
>> recommending arcanist (unless we want to provide instructions on how to fix
>> any breakages that come along).
>> >>
>> >>
>> >>
>> >> Thanks,
>> >>
>> >>Chris Tetreault
>> >>
>> >>
>> >>
>> >> From: llvm-dev  On Behalf Of
>> MyDeveloper Day via llvm-dev
>> >> Sent: Wednesday, August 18, 2021 10:17 AM
>> >> To: llvm-dev ; cfe-commits <
>> cfe-commits@lists.llvm.org>
>> >> Subject: [llvm-dev] Phabricator Creator Pulling the Plug
>> >>
>> >>
>> >>
>> >> WARNING: This email originated from outside of Qualcomm. Please be
>> wary of any links or attachments, and do not enable macros.
>> >>
>> >> All
>> >>
>> >>
>> >>
>> >> I'm a massive fan of Phabricator, and I know there is often lots of
>> contentious discussion about its relative merits vs github,
>> >>
>> >>
>> >>
>> >> But unless I missed this, was there any discussion regarding the
>> recent "Winding Down" announcement of Phabricator? and what it might mean
>> for us in LLVM
>> >>
>> >>
>> >>
>> >> See:
>> >>
>> >>
>> https://admin.phacility.com/phame/post/view/11/phacility_is_winding_down_operations/
>> >>
>> >> https://www.phacility.com/phabricator/
>> >>
>> >>
>> >>
>> >> Personally I'm excited by the concept of a community driven
>> replacement ( https://we.phorge.it/) .
>> >>
>> >> epriestley did a truly amazing job, it wasn't open to public
>> contributions. Perhaps more open development could lead to closing some of
>> the github gaps that were of concern.
>> >>
>> >>
>> >>
>> >> MyDeveloperDay
>> >>
>> >> ___
>> >> LLVM Developers mailing list
>> >> llvm-...@lists.llvm.org
>> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>> >
>> > ___
>> > LLVM Developers mailing list
>> > llvm-...@lists.llvm.org
>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>
__

Re: [llvm-dev] Phabricator Creator Pulling the Plug

[Christian Kühnel via cfe-commits]

I would like to add a third blocker to Mehdi's list:

3) We first would need to finish our ongoing Bugzilla to GitHub Issues
migration: At the moment the plan is to use the old bug ID in bugzilla as
issue ID on GitHub. However issues and Pull Requests share the same
namespace. So once we start using Pull Requests this would eat up the
numbers for the bugs/issues.

This is certainly not a show stopper, but something to decide on. I see two
options here:

1) We delay using Pull Requests until the Bugzilla to Issues migration is
completed.
2) We give up the requirement to keep the bug IDs and assign the new IDs as
they are available.


Best,
Christian


On Fri, Oct 1, 2021 at 12:56 AM Mehdi AMINI via llvm-dev <
llvm-...@lists.llvm.org> wrote:

> We talked about this with the IWG (Infrastructure Working Group) just
> last week coincidentally.
> Two major blocking tracks that were identified at the roundtable
> during the LLVM Dev Meeting exactly 2 years ago are still an issue
> today:
>
> 1) Replacement for Herald rules. This is what allows us to subscribe
> and track new revisions or commits based on paths in the repo or other
> criteria. We could build a replacement based on GitHub action or any
> other kind of service, but this is a bit tricky (how do you store
> emails privately? etc.). I have looked around online but I didn't find
> another OSS project (or external company) providing a similar service
> for GitHub unfortunately, does anyone know of any?
>
> 2) Support for stacked commits. I can see how to structure this
> somehow assuming we would push pull-request branches in the main repo
> (with one new commit per branch and cascading the pull-requests from
> one branch to the other), otherwise this will be a major regression
> compared to the current workflow.
>
> What remains unknown to me is the current state of GitHub management
> of comments across `git commit --amend` and force push to update a
> branch.
>
> Others may have other items to add!
>
> --
> Mehdi
>
> On Thu, Sep 30, 2021 at 3:39 PM Brian Cain via llvm-dev
>  wrote:
> >
> > How far are we from a workflow that leverages Github's Pull Requests?
> Is there some consensus that it's a desired end goal, but some features are
> missing?  Or do we prefer to use a workflow like this for the long term?
> >
> > On Thu, Sep 30, 2021, 4:54 PM Chris Tetreault via llvm-dev <
> llvm-...@lists.llvm.org> wrote:
> >>
> >> As I, and others have noticed, it seems that as of today, there’s some
> certificate issue with arcanist. (See:
> https://lists.llvm.org/pipermail/llvm-dev/2021-September/153019.html) The
> fix seems simple, and a PR is up, but looking through the PR activity, it
> seems that the PR will not be accepted because Phabricator is no longer
> being maintained. It seems that arc has become the first casualty of the
> discontinuation of maintenance of phabricator.
> >>
> >>
> >>
> >> I know that arc is not universally used, but I think it’s a serious
> blow to many people’s workflows. I think that MyDeveloperDay’s question
> might have just become a bit more urgent.
> >>
> >>
> >>
> >> I suppose in the short-term, we could fork the phabricator repos in
> order to fix little issues like this. Alternately, we should probably stop
> recommending arcanist (unless we want to provide instructions on how to fix
> any breakages that come along).
> >>
> >>
> >>
> >> Thanks,
> >>
> >>Chris Tetreault
> >>
> >>
> >>
> >> From: llvm-dev  On Behalf Of
> MyDeveloper Day via llvm-dev
> >> Sent: Wednesday, August 18, 2021 10:17 AM
> >> To: llvm-dev ; cfe-commits <
> cfe-commits@lists.llvm.org>
> >> Subject: [llvm-dev] Phabricator Creator Pulling the Plug
> >>
> >>
> >>
> >> WARNING: This email originated from outside of Qualcomm. Please be wary
> of any links or attachments, and do not enable macros.
> >>
> >> All
> >>
> >>
> >>
> >> I'm a massive fan of Phabricator, and I know there is often lots of
> contentious discussion about its relative merits vs github,
> >>
> >>
> >>
> >> But unless I missed this, was there any discussion regarding the recent
> "Winding Down" announcement of Phabricator? and what it might mean for us
> in LLVM
> >>
> >>
> >>
> >> See:
> >>
> >>
> https://admin.phacility.com/phame/post/view/11/phacility_is_winding_down_operations/
> >>
> >> https://www.phacility.com/phabricator/
> >>
> >>
> >>
> >> Personally I'm excited by the concept of a community driven replacement
> ( https://we.phorge.it/) .
> >>
> >> epriestley did a truly amazing job, it wasn't open to public
> contributions. Perhaps more open development could lead to closing some of
> the github gaps that were of concern.
> >>
> >>
> >>
> >> MyDeveloperDay
> >>
> >> ___
> >> LLVM Developers mailing list
> >> llvm-...@lists.llvm.org
> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> >
> > ___
> > LLVM Developers mailing list
> > llvm-...@lists.llvm.org
> > https://lists.llv

Re: [llvm-dev] Phabricator Creator Pulling the Plug

[Christian Kühnel via cfe-commits]

Hi folks,

Arcanist not working any longer is really unfortunate. Phroge also has a
fork of Arcanist, however I haven't seen any SSL-related patches:
https://we.phorge.it/source/arcanist/browse/master/

1) Replacement for Herald rules.


I suppose the notification problem is solvable. I just took a quick look at
the GitHub APIs and wrote a proposal for a quick-and-dirty solution:
https://github.com/ChristianKuehnel/llvm-iwg/blob/gerald_design/pull_request_migration/gerald_design.md

Concerning privacy and storing user emails this solution is less than
ideal. However contributors are already sharing their email addresses in
the git logs. If we want to hide all personal information, a simple config
file is not enough, then we would need to have some UI where users
configure their rules. However this would be an even larger effort (web UI,
database, user authentication, Github SSO, ...).

I just checked GitHUb's public roadmap and did not find anything that
sounds like Herald:
https://github.com/github/roadmap/projects/1

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

[PATCH] D110810: [clang][ASTImporter] Simplify code of attribute import [NFC].

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:8530-8531
 
+  // Get the result of the previous import attempt (can be used only once).
+  llvm::Expected getResult() {
+if (Err)

steakhal wrote:
> If it can be used only once, we should mark this function as an `r-value` 
> function.
> There is a similar macro already defined as `LLVM_LVALUE_FUNCTION`.
> 
> Later, when you would actually call this function, you need to `std::move()` 
> the object, signifying that the original object gets destroyed in the process.
> 
> @aaron.ballman Do you think we need to define `LLVM_RVALUE_FUNCTION` or we 
> can simply use the `&&` in the function declaration?
> I've seen that you tried to substitute all `LLVM_LVALUE_FUNCTION` macros in 
> the past. What's the status on this?
The expected pattern is:
```
#if LLVM_HAS_RVALUE_REFERENCE_THIS
  void func() && {
  }
#endif
```
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/ASTMatchers/ASTMatchersInternal.h#L609
 (and elsewhere). However, I note that there are some unprotected uses (such as 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Tooling/Syntax/BuildTree.cpp#L437)
 and so it may be a sign that we're fine to remove 
`LLVM_HAS_RVALUE_REFERENCE_THIS` because all our supported compilers do the 
right thing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110810

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

[PATCH] D110810: [clang][ASTImporter] Simplify code of attribute import [NFC].

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

steakhal added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:8530-8531
 
+  // Get the result of the previous import attempt (can be used only once).
+  llvm::Expected getResult() {
+if (Err)

aaron.ballman wrote:
> steakhal wrote:
> > If it can be used only once, we should mark this function as an `r-value` 
> > function.
> > There is a similar macro already defined as `LLVM_LVALUE_FUNCTION`.
> > 
> > Later, when you would actually call this function, you need to 
> > `std::move()` the object, signifying that the original object gets 
> > destroyed in the process.
> > 
> > @aaron.ballman Do you think we need to define `LLVM_RVALUE_FUNCTION` or we 
> > can simply use the `&&` in the function declaration?
> > I've seen that you tried to substitute all `LLVM_LVALUE_FUNCTION` macros in 
> > the past. What's the status on this?
> The expected pattern is:
> ```
> #if LLVM_HAS_RVALUE_REFERENCE_THIS
>   void func() && {
>   }
> #endif
> ```
> https://github.com/llvm/llvm-project/blob/main/clang/include/clang/ASTMatchers/ASTMatchersInternal.h#L609
>  (and elsewhere). However, I note that there are some unprotected uses (such 
> as 
> https://github.com/llvm/llvm-project/blob/main/clang/lib/Tooling/Syntax/BuildTree.cpp#L437)
>  and so it may be a sign that we're fine to remove 
> `LLVM_HAS_RVALUE_REFERENCE_THIS` because all our supported compilers do the 
> right thing?
You tried to eliminate that on Jan 22, 2020 with 
rGdfe9f130e07c929d21f8122272077495de531a38.
But according to git blame, the BuildTree.cpp#L437 was committed on Jul 9, 2019 
with rG9b3f38f99085e2bbf9db01bb00d4c6d837f0fc00.
I'm confused.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110810

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

[PATCH] D110925: [clangd] Follow-up on rGdea48079b90d

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110925

Files:
  clang-tools-extra/clangd/Headers.cpp
  clang-tools-extra/clangd/Headers.h
  llvm/include/llvm/Support/FileSystem/UniqueID.h

Index: llvm/include/llvm/Support/FileSystem/UniqueID.h
===
--- llvm/include/llvm/Support/FileSystem/UniqueID.h
+++ llvm/include/llvm/Support/FileSystem/UniqueID.h
@@ -14,7 +14,9 @@
 #ifndef LLVM_SUPPORT_FILESYSTEM_UNIQUEID_H
 #define LLVM_SUPPORT_FILESYSTEM_UNIQUEID_H
 
+#include "llvm/ADT/DenseMap.h"
 #include 
+#include 
 
 namespace llvm {
 namespace sys {
@@ -47,6 +49,31 @@
 
 } // end namespace fs
 } // end namespace sys
+
+// Support UniqueIDs as DenseMap keys.
+template <> struct DenseMapInfo {
+  static inline llvm::sys::fs::UniqueID getEmptyKey() {
+auto EmptyKey = DenseMapInfo>::getEmptyKey();
+return {EmptyKey.first, EmptyKey.second};
+  }
+
+  static inline llvm::sys::fs::UniqueID getTombstoneKey() {
+auto TombstoneKey =
+DenseMapInfo>::getTombstoneKey();
+return {TombstoneKey.first, TombstoneKey.second};
+  }
+
+  static unsigned getHashValue(const llvm::sys::fs::UniqueID &Tag) {
+return hash_value(
+std::pair(Tag.getDevice(), Tag.getFile()));
+  }
+
+  static bool isEqual(const llvm::sys::fs::UniqueID &LHS,
+  const llvm::sys::fs::UniqueID &RHS) {
+return LHS == RHS;
+  }
+};
+
 } // end namespace llvm
 
 #endif // LLVM_SUPPORT_FILESYSTEM_UNIQUEID_H
Index: clang-tools-extra/clangd/Headers.h
===
--- clang-tools-extra/clangd/Headers.h
+++ clang-tools-extra/clangd/Headers.h
@@ -14,6 +14,7 @@
 #include "index/Symbol.h"
 #include "support/Logger.h"
 #include "support/Path.h"
+#include "clang/Basic/FileEntry.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Format/Format.h"
 #include "clang/Lex/HeaderSearch.h"
@@ -119,6 +120,12 @@
 RealPathNames.emplace_back();
   }
 
+  void setMainFileEntry(const FileEntry *Entry) {
+assert(Entry && Entry->isValid());
+assert(!RealPathNames.empty());
+this->MainFileEntry = Entry;
+  }
+
   // HeaderID identifies file in the include graph. It corresponds to a
   // FileEntry rather than a FileID, but stays stable across preamble & main
   // file builds.
@@ -126,8 +133,7 @@
 
   llvm::Optional getID(const FileEntry *Entry,
  const SourceManager &SM) const;
-  HeaderID getOrCreateID(const FileEntry *Entry,
- const SourceManager &SM);
+  HeaderID getOrCreateID(const FileEntry *Entry, const SourceManager &SM);
 
   StringRef getRealPath(HeaderID ID) const {
 assert(static_cast(ID) <= RealPathNames.size());
@@ -141,8 +147,8 @@
   // All transitive includes (absolute paths), with their minimum include depth.
   // Root --> 0, #included file --> 1, etc.
   // Root is the ID of the header being visited first.
-  // Usually it is getID(SM.getFileEntryForID(SM.getMainFileID()), SM).
-  llvm::DenseMap includeDepth(HeaderID Root) const;
+  llvm::DenseMap
+  includeDepth(HeaderID Root = static_cast(0u)) const;
 
   // Maps HeaderID to the ids of the files included from it.
   llvm::DenseMap> IncludeChildren;
@@ -150,16 +156,18 @@
   std::vector MainFileIncludes;
 
 private:
+  const FileEntry *MainFileEntry;
+
   std::vector RealPathNames; // In HeaderID order.
-  // HeaderID maps the FileEntry::UniqueID to the internal representation.
+  // FileEntry::UniqueID is mapped to the internal representation (HeaderID).
   // Identifying files in a way that persists from preamble build to subsequent
-  // builds is surprisingly hard. FileID is unavailable in
-  // InclusionDirective(), and RealPathName and UniqueID are not preserved in
+  // builds is surprisingly hard. FileID is unavailable in InclusionDirective(),
+  // and RealPathName and UniqueID are not preserved in
   // the preamble.
   //
-  // We reserve 0 to the main file and will manually check for that in getID
-  // and getOrCreateID because llvm::sys::fs::UniqueID is not stable when their
-  // content of the main file changes.
+  // We reserve HeaderID(0) for the main file and will manually check for that
+  // in getID and getOrCreateID because llvm::sys::fs::UniqueID is not stable
+  // when their content of the main file changes.
   llvm::DenseMap UIDToIndex;
 };
 
@@ -228,7 +236,7 @@
 
 namespace llvm {
 
-// Support Tokens as DenseMap keys.
+// Support HeaderIDs as DenseMap keys.
 template <> struct DenseMapInfo {
   static inline clang::clangd::IncludeStructure::HeaderID getEmptyKey() {
 return static_cast(
@@ -251,30 +259,6 @@
   }
 }

[PATCH] D110925: [clangd] Follow-up on rGdea48079b90d

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 376488.
kbobyrev added a comment.

Get rid of SM in getOrCreateID & getID


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110925

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Headers.cpp
  clang-tools-extra/clangd/Headers.h
  clang-tools-extra/clangd/unittests/HeadersTests.cpp
  clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
  llvm/include/llvm/Support/FileSystem/UniqueID.h

Index: llvm/include/llvm/Support/FileSystem/UniqueID.h
===
--- llvm/include/llvm/Support/FileSystem/UniqueID.h
+++ llvm/include/llvm/Support/FileSystem/UniqueID.h
@@ -14,7 +14,9 @@
 #ifndef LLVM_SUPPORT_FILESYSTEM_UNIQUEID_H
 #define LLVM_SUPPORT_FILESYSTEM_UNIQUEID_H
 
+#include "llvm/ADT/DenseMap.h"
 #include 
+#include 
 
 namespace llvm {
 namespace sys {
@@ -47,6 +49,31 @@
 
 } // end namespace fs
 } // end namespace sys
+
+// Support UniqueIDs as DenseMap keys.
+template <> struct DenseMapInfo {
+  static inline llvm::sys::fs::UniqueID getEmptyKey() {
+auto EmptyKey = DenseMapInfo>::getEmptyKey();
+return {EmptyKey.first, EmptyKey.second};
+  }
+
+  static inline llvm::sys::fs::UniqueID getTombstoneKey() {
+auto TombstoneKey =
+DenseMapInfo>::getTombstoneKey();
+return {TombstoneKey.first, TombstoneKey.second};
+  }
+
+  static unsigned getHashValue(const llvm::sys::fs::UniqueID &Tag) {
+return hash_value(
+std::pair(Tag.getDevice(), Tag.getFile()));
+  }
+
+  static bool isEqual(const llvm::sys::fs::UniqueID &LHS,
+  const llvm::sys::fs::UniqueID &RHS) {
+return LHS == RHS;
+  }
+};
+
 } // end namespace llvm
 
 #endif // LLVM_SUPPORT_FILESYSTEM_UNIQUEID_H
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -516,10 +516,10 @@
   IncludeStructure Includes = PatchedAST->getIncludeStructure();
   auto MainFE = FM.getFile(testPath("foo.cpp"));
   ASSERT_TRUE(MainFE);
-  auto MainID = Includes.getID(*MainFE, SM);
+  auto MainID = Includes.getID(*MainFE);
   auto AuxFE = FM.getFile(testPath("sub/aux.h"));
   ASSERT_TRUE(AuxFE);
-  auto AuxID = Includes.getID(*AuxFE, SM);
+  auto AuxID = Includes.getID(*AuxFE);
   EXPECT_THAT(Includes.IncludeChildren[*MainID], Contains(*AuxID));
 }
 
@@ -560,12 +560,12 @@
   IncludeStructure Includes = ExpectedAST.getIncludeStructure();
   auto MainFE = FM.getFile(testPath("foo.cpp"));
   ASSERT_TRUE(MainFE);
-  auto MainID = Includes.getOrCreateID(*MainFE, SM);
+  auto MainID = Includes.getOrCreateID(*MainFE);
   auto &PatchedFM = PatchedAST->getSourceManager().getFileManager();
   IncludeStructure PatchedIncludes = PatchedAST->getIncludeStructure();
   auto PatchedMainFE = PatchedFM.getFile(testPath("foo.cpp"));
   ASSERT_TRUE(PatchedMainFE);
-  auto PatchedMainID = PatchedIncludes.getOrCreateID(*PatchedMainFE, SM);
+  auto PatchedMainID = PatchedIncludes.getOrCreateID(*PatchedMainFE);
   EXPECT_EQ(Includes.includeDepth(MainID)[MainID],
 PatchedIncludes.includeDepth(PatchedMainID)[PatchedMainID]);
 }
Index: clang-tools-extra/clangd/unittests/HeadersTests.cpp
===
--- clang-tools-extra/clangd/unittests/HeadersTests.cpp
+++ clang-tools-extra/clangd/unittests/HeadersTests.cpp
@@ -72,7 +72,7 @@
 auto &SM = Clang->getSourceManager();
 auto Entry = SM.getFileManager().getFile(Filename);
 EXPECT_TRUE(Entry);
-return Includes.getOrCreateID(*Entry, SM);
+return Includes.getOrCreateID(*Entry);
   }
 
   IncludeStructure collectIncludes() {
Index: clang-tools-extra/clangd/Headers.h
===
--- clang-tools-extra/clangd/Headers.h
+++ clang-tools-extra/clangd/Headers.h
@@ -14,6 +14,7 @@
 #include "index/Symbol.h"
 #include "support/Logger.h"
 #include "support/Path.h"
+#include "clang/Basic/FileEntry.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Format/Format.h"
 #include "clang/Lex/HeaderSearch.h"
@@ -119,15 +120,19 @@
 RealPathNames.emplace_back();
   }
 
+  void setMainFileEntry(const FileEntry *Entry) {
+assert(Entry && Entry->isValid());
+assert(!RealPathNames.empty());
+this->MainFileEntry = Entry;
+  }
+
   // HeaderID identifies file in the include graph. It corresponds to a
   // FileEntry rather than a FileID, but stays stable across preamble & main
   // file builds.
   enum class HeaderID : unsigned {};
 
-  llvm::Optional getID(const FileEntry *Entry,
- const SourceManager &SM) const;
-  HeaderID getOrCreateID(const FileEntry *Entry,
- const SourceManager &SM);
+  llvm::Optional getID(const F

[PATCH] D110927: [analyzer] Access stored value of a constant array through a pointer to another type

[Denys Petrov via Phabricator via cfe-commits]

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

Fixed cases in which RegionStore is able to get a stored value of a constant 
array through a pointer of inappropriate type. Adjust 
`RegionStoreManager::getBindingForElement` to the C++20 Standard.
Example:

  const int arr[42] = {1,2,3};
  int x1 = ((unsigned*)arr)[0];  // valid
  int x2 = ((short*)arr)[0]; // invalid
  int x3 = ((char*)arr)[0];  // valid
  int x4 = ((char*)arr)[1];  // invalid


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110927

Files:
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/initialization.cpp

Index: clang/test/Analysis/initialization.cpp
===
--- clang/test/Analysis/initialization.cpp
+++ clang/test/Analysis/initialization.cpp
@@ -2,6 +2,10 @@
 
 void clang_analyzer_eval(int);
 
+namespace std {
+enum class byte : unsigned char {};
+};
+
 struct S {
   int a = 3;
 };
@@ -48,6 +52,46 @@
   auto x = ptr[idx]; // expected-warning{{garbage or undefined}}
 }
 
+void glob_cast_same() {
+  auto *ptr = (int *)glob_arr2;
+  auto x1 = ptr[0]; // no-warning
+  auto x2 = ptr[1]; // no-warning
+}
+
+void glob_cast_char() {
+  const auto *ptr = (char *)glob_arr2;
+  auto x1 = ptr[0]; // no-warning
+  auto x2 = ptr[1]; // expected-warning{{garbage or undefined}}
+}
+
+void glob_cast_uchar() {
+  auto *ptr = (unsigned char *)glob_arr2;
+  auto x1 = ptr[0]; // no-warning
+  auto x2 = ptr[1]; // expected-warning{{garbage or undefined}}
+}
+
+void glob_cast_byte() {
+  auto *ptr = (const std::byte *)glob_arr2;
+  auto x1 = ptr[0]; // no-warning
+  auto x2 = ptr[1]; // expected-warning{{garbage or undefined}}
+}
+
+void glob_cast_opposite_sign() {
+  auto *ptr = (unsigned int *)glob_arr2;
+  auto x1 = ptr[0]; // no-warning
+  auto x2 = ptr[1]; // expected-warning{{garbage or undefined}}
+}
+
+void glob_cast_invalid1() {
+  auto *ptr = (signed char *)glob_arr2;
+  auto x = ptr[0]; // expected-warning{{garbage or undefined}}
+}
+
+void glob_cast_invalid2() {
+  using T = short *;
+  auto x = ((T)glob_arr2)[0]; // expected-warning{{garbage or undefined}}
+}
+
 const float glob_arr3[] = {
 0., 0.0235, 0.0470, 0.0706, 0.0941, 0.1176};
 float no_warn_garbage_value() {
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -437,6 +437,8 @@
 
   RegionBindingsRef removeSubRegionBindings(RegionBindingsConstRef B,
 const SubRegion *R);
+  bool canAccessStoredValue(QualType OrigT, QualType ThroughT,
+uint64_t Index) const;
 
 public: // Part of public interface to class.
 
@@ -1625,6 +1627,56 @@
   return Result;
 }
 
+/// Returns true if the stored value can be accessed through the pointer to
+/// another type:
+///  const int arr[42] = {};
+///  auto* pchar = (char*)arr;
+///  auto* punsigned = (unsigned int*)arr;
+///  auto* pshort= (short*)arr;
+///  auto x1 = pchar[0]; // valid
+///  auto x2 = pchar[1]; // invalid
+///  auto x3 = punsigned[0]; // valid
+///  auto x4 = pshort;   // invalid
+bool RegionStoreManager::canAccessStoredValue(QualType OrigT, QualType ThroughT,
+  uint64_t Index) const {
+  // Remove cv-qualifiers.
+  OrigT = OrigT->getCanonicalTypeUnqualified();
+  ThroughT = ThroughT->getCanonicalTypeUnqualified();
+
+  // C++20 7.2.1.11 [basic.lval] (excerpt):
+  //  A program can access the stored value of an object through:
+  //  - the same type of the object;
+  //  - a signed or unsigned type corresponding to the type of the
+  //object;
+  //  - a char, unsigned char, std::byte. (NOTE:
+  //  Otherwise, the behavior is undefined.
+  return
+  // - is same
+  (OrigT == ThroughT) ||
+  // - is another sign
+  (((OrigT == Ctx.CharTy && ThroughT == Ctx.UnsignedCharTy) ||
+(OrigT == Ctx.SignedCharTy && ThroughT == Ctx.UnsignedCharTy) ||
+(OrigT == Ctx.ShortTy && ThroughT == Ctx.UnsignedShortTy) ||
+(OrigT == Ctx.IntTy && ThroughT == Ctx.UnsignedIntTy) ||
+(OrigT == Ctx.LongTy && ThroughT == Ctx.UnsignedLongTy) ||
+(OrigT == Ctx.LongLongTy && ThroughT == Ctx.UnsignedLongLongTy) ||
+(ThroughT == Ctx.CharTy && OrigT == Ctx.UnsignedCharTy) ||
+(ThroughT == Ctx.SignedCharTy && OrigT == Ctx.UnsignedCharTy) ||
+(ThroughT == Ctx.ShortTy && OrigT == Ctx.UnsignedShortTy) ||
+(ThroughT == Ctx.IntTy && OrigT == Ctx.Unsign

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

[Manas Gupta via Phabricator via cfe-commits]

manas added a comment.

In D106102#3035598 , @steakhal wrote:

> Good work. Land it.

I do not have landing rights.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106102

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

[PATCH] D107312: [analyzer] Fix deprecated plistlib functions

[Manas Gupta via Phabricator via cfe-commits]

manas added a comment.

Gentle ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107312

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

[PATCH] D110798: [NFC] Use CHECK-NEXT instead of CHECK-SAME in target-invalid-cpu-note.c

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

> Can you read the latest example I comment? I think you misunderstand the 
> extra outputs I mentioned. Or if I'm wrong, can you give an inline example?

I understood but yes your example is one that a SAME cannot catch, that's true. 
My point was that SAME does catch some changes, it's not like it's useless.

The only drawback to a single NEXT is when it fails the output from FileCheck 
isn't useful until you manually compare the lines it found, since they're so 
long. Then again a lot of FileCheck output can be like that and using NEXT 
makes the test more robust so sure, let's go with NEXT.

> Yeah, I suppose this is the only one test to check this valid CPU list? Then 
> I suppose to add check whole list for Arm targets.

Correct. (though I think downstream Arm Compiler tests this in other ways but 
that's beside the point)




Comment at: clang/test/Misc/target-invalid-cpu-note.c:1
 // RUN: not %clang_cc1 -triple armv5--- -target-cpu not-a-cpu -fsyntax-only %s 
2>&1 | FileCheck %s --check-prefix ARM
 // ARM: error: unknown target CPU 'not-a-cpu'

Please add a comment as the first line explaining that why we use NEXT and 
check `{{$}}` ending.



Comment at: clang/test/Misc/target-invalid-cpu-note.c:27
 // TUNE_X86_64: error: unknown target CPU 'not-a-cpu'
-// TUNE_X86_64: note: valid target CPU values are: i386, i486, winchip-c6, 
winchip2, c3,
-// TUNE_X86_64-SAME: i586, pentium, pentium-mmx, pentiumpro, i686, pentium2, 
pentium3,
-// TUNE_X86_64-SAME: pentium3m, pentium-m, c3-2, yonah, pentium4, pentium4m, 
prescott,
-// TUNE_X86_64-SAME: nocona, core2, penryn, bonnell, atom, silvermont, slm, 
goldmont, goldmont-plus, tremont,
-// TUNE_X86_64-SAME: nehalem, corei7, westmere, sandybridge, corei7-avx, 
ivybridge,
-// TUNE_X86_64-SAME: core-avx-i, haswell, core-avx2, broadwell, skylake, 
skylake-avx512,
-// TUNE_X86_64-SAME: skx, cascadelake, cooperlake, cannonlake, icelake-client, 
rocketlake, icelake-server, tigerlake, sapphirerapids, alderlake, knl, knm, 
lakemont, k6, k6-2, k6-3,
-// TUNE_X86_64-SAME: athlon, athlon-tbird, athlon-xp, athlon-mp, athlon-4, k8, 
athlon64,
-// TUNE_X86_64-SAME: athlon-fx, opteron, k8-sse3, athlon64-sse3, opteron-sse3, 
amdfam10,
-// TUNE_X86_64-SAME: barcelona, btver1, btver2, bdver1, bdver2, bdver3, 
bdver4, znver1, znver2, znver3,
-// TUNE_X86_64-SAME: x86-64, geode{{$}}
+// TUNE_X86_64-NEXT: note: valid target CPU values are: i386, i486, 
winchip-c6, winchip2, c3, i586, pentium, pentium-mmx, pentiumpro, i686, 
pentium2, pentium3, pentium3m, pentium-m, c3-2, yonah, pentium4, pentium4m, 
prescott, nocona, core2, penryn, bonnell, atom, silvermont, slm, goldmont, 
goldmont-plus, tremont, nehalem, corei7, westmere, sandybridge, corei7-avx, 
ivybridge, core-avx-i, haswell, core-avx2, broadwell, skylake, skylake-avx512, 
skx, cascadelake, cooperlake, cannonlake, icelake-client, rocketlake, 
icelake-server, tigerlake, sapphirerapids, alderlake, knl, knm, lakemont, k6, 
k6-2, k6-3, athlon, athlon-tbird, athlon-xp, athlon-mp, athlon-4, k8, athlon64, 
athlon-fx, opteron, k8-sse3, athlon64-sse3, opteron-sse3, amdfam10, barcelona, 
btver1, btver2, bdver1, bdver2, bdver3, bdver4, znver1, znver2, znver3, x86-64, 
geode{{$}}
 

This one uses `{{$}}` which I think (please check) means that anything extra on 
the end of the line will be a mistmatch. I'd add that to the rest as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110798

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

[PATCH] D110925: [clangd] Follow-up on rGdea48079b90d

[Sam McCall via Phabricator via cfe-commits]

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

Thanks! Just nits

(This review addresses comments on 
https://reviews.llvm.org/rGdea48079b90d40f2087435b778544dffb0ab1793)




Comment at: clang-tools-extra/clangd/Headers.cpp:175
+  if (Entry == MainFileEntry) {
+RealPathNames[0] = MainFileEntry->tryGetRealPathName().str();
+return static_cast(0u);

if RealPathNames.front().empty...



Comment at: clang-tools-extra/clangd/Headers.h:123
 
+  void setMainFileEntry(const FileEntry *Entry) {
+assert(Entry && Entry->isValid());

This is a strange public member, and would need to be documented.

But it seems better yet to make collectIncludeStructureCallback a member 
(`IncludeStructure::collect()`?) so we can just encapsulate this.



Comment at: clang-tools-extra/clangd/Headers.h:125
+assert(Entry && Entry->isValid());
+assert(!RealPathNames.empty());
+this->MainFileEntry = Entry;

nit: this assertion feels a little out-of-place/unneccesary



Comment at: clang-tools-extra/clangd/Headers.h:150
+  llvm::DenseMap
+  includeDepth(HeaderID Root = static_cast(0u)) const;
 

if we're going to expose the root headerID, we should do it in a named constant 
and reference that here.



Comment at: clang-tools-extra/clangd/Headers.h:158
 private:
+  const FileEntry *MainFileEntry;
+

this member should be documented (I think you can just move the bit about 
HeaderID(0) from below to here.



Comment at: clang-tools-extra/clangd/Headers.h:158
 private:
+  const FileEntry *MainFileEntry;
+

sammccall wrote:
> this member should be documented (I think you can just move the bit about 
> HeaderID(0) from below to here.
`= nullptr`



Comment at: clang-tools-extra/clangd/Headers.h:168
+  // We reserve HeaderID(0) for the main file and will manually check for that
+  // in getID and getOrCreateID because llvm::sys::fs::UniqueID is not stable
+  // when their content of the main file changes.

nit: it's the *value* that's unstable, not the type. So just "the UniqueID"



Comment at: clang-tools-extra/clangd/Headers.h:169
+  // in getID and getOrCreateID because llvm::sys::fs::UniqueID is not stable
+  // when their content of the main file changes.
   llvm::DenseMap UIDToIndex;

their -> the



Comment at: llvm/include/llvm/Support/FileSystem/UniqueID.h:17
 
+#include "llvm/ADT/DenseMap.h"
 #include 

you only need DenseMapInfo here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110925

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

[PATCH] D110668: [clang-cl] Accept `#pragma warning(disable : N)` for some N

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D110668#3034576 , @xbolva00 wrote:

> Please next time give a bit more time to potential reviewers / other folks 
> outside your org. The whole lifecycle of this patch (posted - landed) took < 
> 24h.

Is there anything wrong with the patch?

I agree that it's good to let larger changes sit for a bit, but this seems like 
a fairly small and inconsequential change to me. Many patches land with a 
review time < 24h.

In any case, happy to address post-commit review comments too of course.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110668

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

[clang] 369d785 - [PowerPC] Optimal sequence for doubleword vec_all_{eq|ne} on Power7

[Nemanja Ivanovic via cfe-commits]

Author: Nemanja Ivanovic
Date: 2021-10-01T08:27:15-05:00
New Revision: 369d785574f5a22c086d0c40268a39a64bdd7217

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

LOG: [PowerPC] Optimal sequence for doubleword vec_all_{eq|ne} on Power7

These builtins produce inefficient code for CPU's prior to Power8
due to vcmpequd being unavailable. The predicate forms can actually
leverage the available vcmpequw along with xxlxor to produce a better
sequence.

Added: 


Modified: 
clang/lib/Headers/altivec.h
clang/test/CodeGen/builtins-ppc-vsx.c

Removed: 




diff  --git a/clang/lib/Headers/altivec.h b/clang/lib/Headers/altivec.h
index 6a179d86d71f9..5da4fbf72ce97 100644
--- a/clang/lib/Headers/altivec.h
+++ b/clang/lib/Headers/altivec.h
@@ -14815,42 +14815,43 @@ static __inline__ int __ATTRS_o_ai vec_all_eq(vector 
bool int __a,
 #ifdef __VSX__
 static __inline__ int __ATTRS_o_ai vec_all_eq(vector signed long long __a,
   vector signed long long __b) {
+#ifdef __POWER8_VECTOR__
   return __builtin_altivec_vcmpequd_p(__CR6_LT, __a, __b);
+#else
+  // No vcmpequd on Power7 so we xor the two vectors and compare against zero 
as
+  // 32-bit elements.
+  return vec_all_eq((vector signed int)vec_xor(__a, __b), (vector signed 
int)0);
+#endif
 }
 
 static __inline__ int __ATTRS_o_ai vec_all_eq(vector long long __a,
   vector bool long long __b) {
-  return __builtin_altivec_vcmpequd_p(__CR6_LT, __a, (vector long long)__b);
+  return vec_all_eq((vector signed long long)__a, (vector signed long 
long)__b);
 }
 
 static __inline__ int __ATTRS_o_ai vec_all_eq(vector unsigned long long __a,
   vector unsigned long long __b) {
-  return __builtin_altivec_vcmpequd_p(__CR6_LT, (vector long long)__a,
-  (vector long long)__b);
+  return vec_all_eq((vector signed long long)__a, (vector signed long 
long)__b);
 }
 
 static __inline__ int __ATTRS_o_ai vec_all_eq(vector unsigned long long __a,
   vector bool long long __b) {
-  return __builtin_altivec_vcmpequd_p(__CR6_LT, (vector long long)__a,
-  (vector long long)__b);
+  return vec_all_eq((vector signed long long)__a, (vector signed long 
long)__b);
 }
 
 static __inline__ int __ATTRS_o_ai vec_all_eq(vector bool long long __a,
   vector long long __b) {
-  return __builtin_altivec_vcmpequd_p(__CR6_LT, (vector long long)__a,
-  (vector long long)__b);
+  return vec_all_eq((vector signed long long)__a, (vector signed long 
long)__b);
 }
 
 static __inline__ int __ATTRS_o_ai vec_all_eq(vector bool long long __a,
   vector unsigned long long __b) {
-  return __builtin_altivec_vcmpequd_p(__CR6_LT, (vector long long)__a,
-  (vector long long)__b);
+  return vec_all_eq((vector signed long long)__a, (vector signed long 
long)__b);
 }
 
 static __inline__ int __ATTRS_o_ai vec_all_eq(vector bool long long __a,
   vector bool long long __b) {
-  return __builtin_altivec_vcmpequd_p(__CR6_LT, (vector long long)__a,
-  (vector long long)__b);
+  return vec_all_eq((vector signed long long)__a, (vector signed long 
long)__b);
 }
 #endif
 
@@ -17038,43 +17039,43 @@ static __inline__ int __ATTRS_o_ai vec_any_ne(vector 
bool int __a,
 #ifdef __VSX__
 static __inline__ int __ATTRS_o_ai vec_any_ne(vector signed long long __a,
   vector signed long long __b) {
+#ifdef __POWER8_VECTOR__
   return __builtin_altivec_vcmpequd_p(__CR6_LT_REV, __a, __b);
+#else
+  // Take advantage of the optimized sequence for vec_all_eq when vcmpequd is
+  // not available.
+  return !vec_all_eq(__a, __b);
+#endif
 }
 
 static __inline__ int __ATTRS_o_ai vec_any_ne(vector unsigned long long __a,
   vector unsigned long long __b) {
-  return __builtin_altivec_vcmpequd_p(__CR6_LT_REV, (vector long long)__a,
-  (vector long long)__b);
+  return vec_any_ne((vector signed long long)__a, (vector signed long 
long)__b);
 }
 
 static __inline__ int __ATTRS_o_ai vec_any_ne(vector signed long long __a,
   vector bool long long __b) {
-  return __builtin_altivec_vcmpequd_p(__CR6_LT_REV, __a,
-  (vector signed long long)__b);
+  return vec_any_ne((vector signed long long)__a, (vector signed long 
long)__b);
 }
 
 static __inl

[clang] ec4a822 - [clang] Try to unbreak crash-report.cpp on PS4 bot after 8dfbe9b0a

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2021-10-01T09:33:13-04:00
New Revision: ec4a82286674c44c9216e9585235b0fa5df4ae9f

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

LOG: [clang] Try to unbreak crash-report.cpp on PS4 bot after 8dfbe9b0a

Looks like exceptions are off-by-default with the PS4 triple.
Since adding -fexceptions defeats the purpose of the test change
in 8dfbe9b0a, pass an explicit triple instead.

Added: 


Modified: 
clang/test/Driver/crash-report.cpp

Removed: 




diff  --git a/clang/test/Driver/crash-report.cpp 
b/clang/test/Driver/crash-report.cpp
index 293bc1f0ad9f0..7da94885080be 100644
--- a/clang/test/Driver/crash-report.cpp
+++ b/clang/test/Driver/crash-report.cpp
@@ -7,7 +7,7 @@
 // RUN:  -Xclang -internal-isystem -Xclang /tmp/ \
 // RUN:  -Xclang -internal-externc-isystem -Xclang /tmp/ \
 // RUN:  -Xclang -main-file-name -Xclang foo.cpp \
-// RUN:  -DFOO=BAR -DBAR="BAZ QUX"' > %t.rsp
+// RUN:  -DFOO=BAR -DBAR="BAZ QUX"' --target=x86_64-linux-gnu > %t.rsp
 
 // RUN: env TMPDIR=%t TEMP=%t TMP=%t RC_DEBUG_OPTIONS=1  \
 // RUN:  CC_PRINT_HEADERS=1 CC_LOG_DIAGNOSTICS=1 \



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

[PATCH] D110783: [clang] Make crash reproducer work with clang-cl

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D110783#3035158 , @dyung wrote:

> Hi, the test you modified Driver/crash-report.cpp is failing on the PS4 bot 
> after your change. Can you take a look?
>
> https://lab.llvm.org/buildbot/#/builders/139/builds/10939

Hopefully better after ec4a82286674c44c9216e9585235b0fa5df4ae9f 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110783

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

[PATCH] D110783: [clang] Make crash reproducer work with clang-cl

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

> Hopefully better after ec4a82286674c44c9216e9585235b0fa5df4ae9f 
> 

Your bot cycled green after that change: 
https://lab.llvm.org/buildbot/#/builders/139/builds/10976


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110783

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

[PATCH] D106823: [analyzer][solver] Iterate to a fixpoint during symbol simplification with constants

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2108
+LLVM_NODISCARD ProgramStateRef
+EquivalenceClass::removeMember(ProgramStateRef State, const SymbolRef Old) {
+

Remove `const `



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2123
+  ClsMembers = F.remove(ClsMembers, Old);
+  assert(!ClsMembers.isEmpty() &&
+ "Class should have had at least two members before member removal");

Comment that this is a precondition.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2156
+  assert(find(State, MemberSym) == find(State, SimplifiedMemberSym));
+  // Remove the old and more complex symbol.
+  State = find(State, MemberSym).removeMember(State, MemberSym);

TODO add Performance and complexity essay here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106823

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

[PATCH] D107312: [analyzer] Fix deprecated plistlib functions

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

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

I'm not using this script. I'm assuming you run it and verified that it works.
Thanks for cleaning this up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107312

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

[PATCH] D110927: [analyzer] Access stored value of a constant array through a pointer to another type

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

steakhal added a subscriber: shafik.
steakhal added a comment.

I'm pretty sure that `int x4 = ((char*)arr)[1];` is supposed to be valid in 
your summary.
I think it's allowed by the standard to access any valid object via a `char*` - 
according to the strict aliasing rule.
@shafik WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110927

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

[PATCH] D110934: [NFC] Update return type of vec_popcnt to vector unsigned.

[Amy Kwan via Phabricator via cfe-commits]

amyk created this revision.
amyk added reviewers: PowerPC, nemanjai, stefanp.
amyk added projects: LLVM, PowerPC, clang.
amyk requested review of this revision.

This patch updates the vec_popcnt builtins to take a signed int as the second 
parameter, as defined by the Power Vector Intrinsics Programming Reference.
This patch is NFC and all existing tests pass.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110934

Files:
  clang/lib/Headers/altivec.h


Index: clang/lib/Headers/altivec.h
===
--- clang/lib/Headers/altivec.h
+++ clang/lib/Headers/altivec.h
@@ -2482,7 +2482,7 @@
 #ifdef __POWER8_VECTOR__
 /* vec_popcnt */
 
-static __inline__ vector signed char __ATTRS_o_ai
+static __inline__ vector unsigned char __ATTRS_o_ai
 vec_popcnt(vector signed char __a) {
   return __builtin_altivec_vpopcntb(__a);
 }
@@ -2490,7 +2490,7 @@
 vec_popcnt(vector unsigned char __a) {
   return __builtin_altivec_vpopcntb(__a);
 }
-static __inline__ vector signed short __ATTRS_o_ai
+static __inline__ vector unsigned short __ATTRS_o_ai
 vec_popcnt(vector signed short __a) {
   return __builtin_altivec_vpopcnth(__a);
 }
@@ -2498,7 +2498,7 @@
 vec_popcnt(vector unsigned short __a) {
   return __builtin_altivec_vpopcnth(__a);
 }
-static __inline__ vector signed int __ATTRS_o_ai
+static __inline__ vector unsigned int __ATTRS_o_ai
 vec_popcnt(vector signed int __a) {
   return __builtin_altivec_vpopcntw(__a);
 }
@@ -2506,7 +2506,7 @@
 vec_popcnt(vector unsigned int __a) {
   return __builtin_altivec_vpopcntw(__a);
 }
-static __inline__ vector signed long long __ATTRS_o_ai
+static __inline__ vector unsigned long long __ATTRS_o_ai
 vec_popcnt(vector signed long long __a) {
   return __builtin_altivec_vpopcntd(__a);
 }


Index: clang/lib/Headers/altivec.h
===
--- clang/lib/Headers/altivec.h
+++ clang/lib/Headers/altivec.h
@@ -2482,7 +2482,7 @@
 #ifdef __POWER8_VECTOR__
 /* vec_popcnt */
 
-static __inline__ vector signed char __ATTRS_o_ai
+static __inline__ vector unsigned char __ATTRS_o_ai
 vec_popcnt(vector signed char __a) {
   return __builtin_altivec_vpopcntb(__a);
 }
@@ -2490,7 +2490,7 @@
 vec_popcnt(vector unsigned char __a) {
   return __builtin_altivec_vpopcntb(__a);
 }
-static __inline__ vector signed short __ATTRS_o_ai
+static __inline__ vector unsigned short __ATTRS_o_ai
 vec_popcnt(vector signed short __a) {
   return __builtin_altivec_vpopcnth(__a);
 }
@@ -2498,7 +2498,7 @@
 vec_popcnt(vector unsigned short __a) {
   return __builtin_altivec_vpopcnth(__a);
 }
-static __inline__ vector signed int __ATTRS_o_ai
+static __inline__ vector unsigned int __ATTRS_o_ai
 vec_popcnt(vector signed int __a) {
   return __builtin_altivec_vpopcntw(__a);
 }
@@ -2506,7 +2506,7 @@
 vec_popcnt(vector unsigned int __a) {
   return __builtin_altivec_vpopcntw(__a);
 }
-static __inline__ vector signed long long __ATTRS_o_ai
+static __inline__ vector unsigned long long __ATTRS_o_ai
 vec_popcnt(vector signed long long __a) {
   return __builtin_altivec_vpopcntd(__a);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110935: [NFC] Update vec_extract builtin signatures to take signed int.

[Amy Kwan via Phabricator via cfe-commits]

amyk created this revision.
amyk added reviewers: PowerPC, nemanjai, stefanp.
amyk added projects: LLVM, PowerPC, clang.
amyk requested review of this revision.

This patch updates the vec_extract builtins to take a signed int as the second 
parameter, as defined by the Power Vector Intrinsics Programming Reference.
This patch is NFC and all existing tests pass.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110935

Files:
  clang/lib/Headers/altivec.h


Index: clang/lib/Headers/altivec.h
===
--- clang/lib/Headers/altivec.h
+++ clang/lib/Headers/altivec.h
@@ -13444,74 +13444,74 @@
 /* vec_extract */
 
 static __inline__ signed char __ATTRS_o_ai vec_extract(vector signed char __a,
-   unsigned int __b) {
+   signed int __b) {
   return __a[__b & 0xf];
 }
 
 static __inline__ unsigned char __ATTRS_o_ai
-vec_extract(vector unsigned char __a, unsigned int __b) {
+vec_extract(vector unsigned char __a, signed int __b) {
   return __a[__b & 0xf];
 }
 
 static __inline__ unsigned char __ATTRS_o_ai vec_extract(vector bool char __a,
- unsigned int __b) {
+ signed int __b) {
   return __a[__b & 0xf];
 }
 
 static __inline__ signed short __ATTRS_o_ai vec_extract(vector signed short 
__a,
-unsigned int __b) {
+signed int __b) {
   return __a[__b & 0x7];
 }
 
 static __inline__ unsigned short __ATTRS_o_ai
-vec_extract(vector unsigned short __a, unsigned int __b) {
+vec_extract(vector unsigned short __a, signed int __b) {
   return __a[__b & 0x7];
 }
 
 static __inline__ unsigned short __ATTRS_o_ai vec_extract(vector bool short 
__a,
-  unsigned int __b) {
+  signed int __b) {
   return __a[__b & 0x7];
 }
 
 static __inline__ signed int __ATTRS_o_ai vec_extract(vector signed int __a,
-  unsigned int __b) {
+  signed int __b) {
   return __a[__b & 0x3];
 }
 
 static __inline__ unsigned int __ATTRS_o_ai vec_extract(vector unsigned int 
__a,
-unsigned int __b) {
+signed int __b) {
   return __a[__b & 0x3];
 }
 
 static __inline__ unsigned int __ATTRS_o_ai vec_extract(vector bool int __a,
-unsigned int __b) {
+signed int __b) {
   return __a[__b & 0x3];
 }
 
 #ifdef __VSX__
 static __inline__ signed long long __ATTRS_o_ai
-vec_extract(vector signed long long __a, unsigned int __b) {
+vec_extract(vector signed long long __a, signed int __b) {
   return __a[__b & 0x1];
 }
 
 static __inline__ unsigned long long __ATTRS_o_ai
-vec_extract(vector unsigned long long __a, unsigned int __b) {
+vec_extract(vector unsigned long long __a, signed int __b) {
   return __a[__b & 0x1];
 }
 
 static __inline__ unsigned long long __ATTRS_o_ai
-vec_extract(vector bool long long __a, unsigned int __b) {
+vec_extract(vector bool long long __a, signed int __b) {
   return __a[__b & 0x1];
 }
 
 static __inline__ double __ATTRS_o_ai vec_extract(vector double __a,
-  unsigned int __b) {
+  signed int __b) {
   return __a[__b & 0x1];
 }
 #endif
 
 static __inline__ float __ATTRS_o_ai vec_extract(vector float __a,
- unsigned int __b) {
+ signed int __b) {
   return __a[__b & 0x3];
 }
 


Index: clang/lib/Headers/altivec.h
===
--- clang/lib/Headers/altivec.h
+++ clang/lib/Headers/altivec.h
@@ -13444,74 +13444,74 @@
 /* vec_extract */
 
 static __inline__ signed char __ATTRS_o_ai vec_extract(vector signed char __a,
-   unsigned int __b) {
+   signed int __b) {
   return __a[__b & 0xf];
 }
 
 static __inline__ unsigned char __ATTRS_o_ai
-vec_extract(vector unsigned char __a, unsigned int __b) {
+vec_extract(vector unsigned char __a, signed int __b) {
   return __a[__b & 0xf];
 }
 
 static __inline__ unsigned char __ATTRS_o_ai vec_extract(vector bool char __a,
- unsigned int __b) {
+ signed int __b) {
   return __a[__b & 0xf];
 }
 
 static __inline__ signe

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

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

steakhal added a comment.

In D106102#3036220 , @manas wrote:

> I do not have landing rights.

Please add your name and email on whom behalf I should commit this patch. Mine 
is `Balazs Benics`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106102

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

[PATCH] D110625: [analyzer] canonicalize special case of structure/pointer deref

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers updated this revision to Diff 376526.
vabridgers added a comment.

Use canonical types for comparison


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110625

Files:
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/ptr-arith.c

Index: clang/test/Analysis/ptr-arith.c
===
--- clang/test/Analysis/ptr-arith.c
+++ clang/test/Analysis/ptr-arith.c
@@ -2,6 +2,7 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.FixedAddr,alpha.core.PointerArithm,alpha.core.PointerSub,debug.ExprInspection -analyzer-store=region -Wno-pointer-to-int-cast -verify -triple i686-apple-darwin9 -Wno-tautological-pointer-compare -analyzer-config eagerly-assume=false %s
 
 void clang_analyzer_eval(int);
+void clang_analyzer_dump(int);
 
 void f1() {
   int a[10];
@@ -330,3 +331,59 @@
   simd_float2 x = {0, 1};
   return x[1]; // no-warning
 }
+
+struct s {
+  int v;
+};
+
+// These three expressions should produce the same sym vals.
+void struct_pointer_canon(struct s *ps) {
+  struct s ss = *ps;
+  clang_analyzer_dump((*ps).v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps[0].v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps->v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_eval((*ps).v == ps[0].v); // expected-warning{{TRUE}}
+  clang_analyzer_eval((*ps).v == ps->v);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ps[0].v == ps->v);   // expected-warning{{TRUE}}
+}
+
+void struct_pointer_canon_bidim(struct s **ps) {
+  struct s ss = **ps;
+  clang_analyzer_eval(&(ps[0][0].v) == &((*ps)->v)); // expected-warning{{TRUE}}
+}
+
+typedef struct s T1;
+typedef struct s T2;
+void struct_pointer_canon_typedef(T1 *ps) {
+  T2 ss = *ps;
+  clang_analyzer_dump((*ps).v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps[0].v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps->v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_eval((*ps).v == ps[0].v); // expected-warning{{TRUE}}
+  clang_analyzer_eval((*ps).v == ps->v);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ps[0].v == ps->v);   // expected-warning{{TRUE}}
+}
+
+void struct_pointer_canon_bidim_typedef(T1 **ps) {
+  T2 ss = **ps;
+  clang_analyzer_eval(&(ps[0][0].v) == &((*ps)->v)); // expected-warning{{TRUE}}
+}
+
+void struct_pointer_canon_const(const struct s *ps) {
+  struct s ss = *ps;
+  clang_analyzer_dump((*ps).v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps[0].v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps->v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_eval((*ps).v == ps[0].v); // expected-warning{{TRUE}}
+  clang_analyzer_eval((*ps).v == ps->v);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ps[0].v == ps->v);   // expected-warning{{TRUE}}
+}
Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -442,6 +442,26 @@
 
 SVal StoreManager::getLValueElement(QualType elementType, NonLoc Offset,
 SVal Base) {
+
+  // Special case, if index is 0, return the same type as if
+  // this was not an array dereference.
+  if (Offset.isZeroConstant()) {
+ASTContext &Ctx = StateMgr.getContext();
+QualType BT = Base.getType(this->Ctx);
+if (!BT.isNull() && !elementType.isNull()) {
+  if (!BT->getPointeeType().isNull()) {
+QualType CanonBT = BT->getPointeeType().isCanonical()
+   ? BT->getPointeeType()
+   : Ctx.getCanonicalType(BT->getPointeeType());
+QualType CanonEleTy = elementType.isCanonical()
+  ? elementType
+  : Ctx.getCanonicalType(elementType);
+if (CanonBT == CanonEleTy)
+  return Base;
+  }
+}
+  }
+
   // If the base is an unknown or undefined value, just return it back.
   // FIXME: For absolute pointer addresses, we just return that value back as
   //  well, although in reality we should return the offset added to that
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

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

steakhal added a comment.

🛬


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106102

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

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

[Manas Gupta via Phabricator via cfe-commits]

manas added a comment.

In D106102#3036474 , @steakhal wrote:

> In D106102#3036220 , @manas wrote:
>
>> I do not have landing rights.
>
> Please add your name and email on whom behalf I should commit this patch. 
> Mine is `Balazs Benics`

It is `Manas `

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106102

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

[PATCH] D107312: [analyzer] Fix deprecated plistlib functions

[Manas Gupta via Phabricator via cfe-commits]

manas added a comment.

In D107312#3036421 , @steakhal wrote:

> I'm not using this script. I'm assuming you run it and verified that it works.
> Thanks for cleaning this up.

I have run it. It is working.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107312

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

[libunwind] 532783f - [libunwind] Fix cfi_register for float registers.

[Daniel Kiss via cfe-commits]

Author: Daniel Kiss
Date: 2021-10-01T16:51:51+02:00
New Revision: 532783f9e1e65c7bd48b1592d2376e9dd47c5a73

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

LOG: [libunwind] Fix cfi_register for float registers.

Fixes D110144.
registers.getFloatRegister is not const in ARM therefor can't be called here.

Reviewed By: mstorsjo, #libunwind

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

Added: 


Modified: 
libunwind/src/DwarfInstructions.hpp

Removed: 




diff  --git a/libunwind/src/DwarfInstructions.hpp 
b/libunwind/src/DwarfInstructions.hpp
index 53baf6a148f33..b58c51bb7a604 100644
--- a/libunwind/src/DwarfInstructions.hpp
+++ b/libunwind/src/DwarfInstructions.hpp
@@ -115,10 +115,12 @@ double DwarfInstructions::getSavedFloatRegister(
 return addressSpace.getDouble(
 evaluateExpression((pint_t)savedReg.value, addressSpace,
 registers, cfa));
-  case CFI_Parser::kRegisterInRegister:
-return registers.getFloatRegister((int)savedReg.value);
   case CFI_Parser::kRegisterUndefined:
 return 0.0;
+  case CFI_Parser::kRegisterInRegister:
+#ifndef _LIBUNWIND_TARGET_ARM
+return registers.getFloatRegister((int)savedReg.value);
+#endif
   case CFI_Parser::kRegisterIsExpression:
   case CFI_Parser::kRegisterUnused:
   case CFI_Parser::kRegisterOffsetFromCFA:



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

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

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

steakhal added a comment.

It seems like it doesn't build with GCC 8.3.0 on a Debian system.
Could you investigate?

  
llvm-project/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1029:13: 
error: explicit specialization in non-namespace scope 'class 
{anonymous}::SymbolicRangeInferrer'
 template <>
   ^
  
llvm-project/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1030:77: 
error: template-id 'VisitBinaryOperator' in declaration of primary 
template
 RangeSet VisitBinaryOperator(RangeSet LHS, RangeSet RHS, QualType 
T) {
   ^

I think the issue is this: 
https://stackoverflow.com/questions/3052579/explicit-specialization-in-non-namespace-scope


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106102

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

[PATCH] D106823: [analyzer][solver] Iterate to a fixpoint during symbol simplification with constants

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

steakhal added a comment.

I'm gonna get back to this on Monday.




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1699-1701
+  ProgramStateRef OldState;
+  do {
+OldState = State;

IMO we should have a `llvm::Statistic` here, tracking the maximum iteration 
count to reach the fixed point and an average iteration count.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1732
+  if (!State)
+return false;
+}

I'd love to see a coverage report of the tests you add with this patch.



Comment at: clang/test/Analysis/expr-inspection-printState-eq-classes.c:11
 return;
-  if (b != 0)
+  if (a != c)
 return;

Why do you need to change this?



Comment at: 
clang/test/Analysis/symbol-simplification-fixpoint-iteration-unreachable-code.cpp:16
+return;
+  if (c + b != 0)
+return;

Is it important to have this instead of `b + c`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106823

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

[PATCH] D110783: [clang] Make crash reproducer work with clang-cl

[Orlando Cazalet-Hyams via Phabricator via cfe-commits]

Orlando added a comment.

In D110783#3036403 , @thakis wrote:

>> Hopefully better after ec4a82286674c44c9216e9585235b0fa5df4ae9f 
>> 
>
> Your bot cycled green after that change: 
> https://lab.llvm.org/buildbot/#/builders/139/builds/10976

Thanks for the fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110783

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

[clang] a3d0b58 - [analyzer] Fix deprecated plistlib functions

[Balazs Benics via cfe-commits]

Author: Manas
Date: 2021-10-01T17:07:24+02:00
New Revision: a3d0b5805e5ff2fd870df5be5c3197eee0bb74a0

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

LOG: [analyzer] Fix deprecated plistlib functions

It replaces the usage of readPlist,writePlist functions with load,dump
in plistlib package.

This fixes deprecation issues when analyzer reports are being generated
outside of docker.

Patch by Manas!

Reviewed By: steakhal

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

Added: 


Modified: 
clang/utils/analyzer/SATestBuild.py

Removed: 




diff  --git a/clang/utils/analyzer/SATestBuild.py 
b/clang/utils/analyzer/SATestBuild.py
index 1977a8fc2aeff..cf02f26ef267b 100644
--- a/clang/utils/analyzer/SATestBuild.py
+++ b/clang/utils/analyzer/SATestBuild.py
@@ -856,7 +856,8 @@ def normalize_reference_results(directory: str, output_dir: 
str,
 continue
 
 plist = os.path.join(dir_path, filename)
-data = plistlib.readPlist(plist)
+with open(plist, "rb") as plist_file:
+data = plistlib.load(plist_file)
 path_prefix = directory
 
 if build_mode == 1:
@@ -875,7 +876,8 @@ def normalize_reference_results(directory: str, output_dir: 
str,
 if 'clang_version' in data:
 data.pop('clang_version')
 
-plistlib.writePlist(data, plist)
+with open(plist, "wb") as plist_file:
+plistlib.dump(data, plist_file)
 
 
 def get_build_log_path(output_dir: str) -> str:



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

[PATCH] D107312: [analyzer] Fix deprecated plistlib functions

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa3d0b5805e5f: [analyzer] Fix deprecated plistlib functions 
(authored by manas, committed by steakhal).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107312

Files:
  clang/utils/analyzer/SATestBuild.py


Index: clang/utils/analyzer/SATestBuild.py
===
--- clang/utils/analyzer/SATestBuild.py
+++ clang/utils/analyzer/SATestBuild.py
@@ -856,7 +856,8 @@
 continue
 
 plist = os.path.join(dir_path, filename)
-data = plistlib.readPlist(plist)
+with open(plist, "rb") as plist_file:
+data = plistlib.load(plist_file)
 path_prefix = directory
 
 if build_mode == 1:
@@ -875,7 +876,8 @@
 if 'clang_version' in data:
 data.pop('clang_version')
 
-plistlib.writePlist(data, plist)
+with open(plist, "wb") as plist_file:
+plistlib.dump(data, plist_file)
 
 
 def get_build_log_path(output_dir: str) -> str:


Index: clang/utils/analyzer/SATestBuild.py
===
--- clang/utils/analyzer/SATestBuild.py
+++ clang/utils/analyzer/SATestBuild.py
@@ -856,7 +856,8 @@
 continue
 
 plist = os.path.join(dir_path, filename)
-data = plistlib.readPlist(plist)
+with open(plist, "rb") as plist_file:
+data = plistlib.load(plist_file)
 path_prefix = directory
 
 if build_mode == 1:
@@ -875,7 +876,8 @@
 if 'clang_version' in data:
 data.pop('clang_version')
 
-plistlib.writePlist(data, plist)
+with open(plist, "wb") as plist_file:
+plistlib.dump(data, plist_file)
 
 
 def get_build_log_path(output_dir: str) -> str:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110927: [analyzer] Access stored value of a constant array through a pointer to another type

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D110927#3036436 , @steakhal wrote:

> I'm pretty sure that `int x4 = ((char*)arr)[1];` is supposed to be valid in 
> your summary.
> I think it's allowed by the standard to access any valid object via a `char*` 
> - according to the strict aliasing rule.
> @shafik WDYT?

As I found we can legaly treat `char*` as the object of type `char` but not as 
an array of objects. This is mentioned in 
http://eel.is/c++draft/basic.compound#3.4 //For purposes of pointer arithmetic 
... an object of type T that is not an array element is considered to belong to 
an array with one element of type T.// That means that we can get only the 
first element of `char*`, otherwise it would be an UB.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110927

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

[PATCH] D107339: [analyzer] Retrieve a character from StringLiteral as an initializer for constant arrays.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

@ASDenysPetrov This looks promising! Please address the nits which @steakhal 
found, than I think it is good to land.




Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1641
+  // FIXME: Take array dimension into account to prevent exceeding its size.
+  const int64_t I = Idx.getExtValue();
+  uint32_t Code =

steakhal wrote:
> You could use the `uint64_t` type here, and spare the subsequent explicit 
> cast. This operation would be safe since `Idx` must be non-negative here.
+1 for using `uint64_t` if possible


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

https://reviews.llvm.org/D107339

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

[PATCH] D110428: [AIX] Define WCHAR_T_TYPE as unsigned short on AIX for wchar.c test case.

[Amy Kwan via Phabricator via cfe-commits]

amyk added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110428

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

[PATCH] D109652: [PowerPC] Restrict various P10 options to P10 only.

[Amy Kwan via Phabricator via cfe-commits]

amyk added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109652

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

[PATCH] D110670: [Sema] Allow comparisons between different ms ptr size address space types.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/Sema/MicrosoftExtensions.cpp:9-10
+  return (p32u == p32s) +
+ (p32u == p64) +
+ (p32s == p64);
+}

akhuang wrote:
> aaron.ballman wrote:
> > (Side question, not directly about this patch but sorta related.) Should 
> > there be a diagnostic about conversion potentially causing a possible loss 
> > of data (on the 64-bit target)?
> Hmm, it seems reasonable, but I also don't know how motivated I am to add a 
> diagnostic -- 
I don't insist -- we are missing that warning in general here (we don't warn on 
assignment yet). But it might be a nice follow-up for anyone who's interested 
in working on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110670

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

[PATCH] D110810: [clang][ASTImporter] Simplify code of attribute import [NFC].

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:8530-8531
 
+  // Get the result of the previous import attempt (can be used only once).
+  llvm::Expected getResult() {
+if (Err)

steakhal wrote:
> aaron.ballman wrote:
> > steakhal wrote:
> > > If it can be used only once, we should mark this function as an `r-value` 
> > > function.
> > > There is a similar macro already defined as `LLVM_LVALUE_FUNCTION`.
> > > 
> > > Later, when you would actually call this function, you need to 
> > > `std::move()` the object, signifying that the original object gets 
> > > destroyed in the process.
> > > 
> > > @aaron.ballman Do you think we need to define `LLVM_RVALUE_FUNCTION` or 
> > > we can simply use the `&&` in the function declaration?
> > > I've seen that you tried to substitute all `LLVM_LVALUE_FUNCTION` macros 
> > > in the past. What's the status on this?
> > The expected pattern is:
> > ```
> > #if LLVM_HAS_RVALUE_REFERENCE_THIS
> >   void func() && {
> >   }
> > #endif
> > ```
> > https://github.com/llvm/llvm-project/blob/main/clang/include/clang/ASTMatchers/ASTMatchersInternal.h#L609
> >  (and elsewhere). However, I note that there are some unprotected uses 
> > (such as 
> > https://github.com/llvm/llvm-project/blob/main/clang/lib/Tooling/Syntax/BuildTree.cpp#L437)
> >  and so it may be a sign that we're fine to remove 
> > `LLVM_HAS_RVALUE_REFERENCE_THIS` because all our supported compilers do the 
> > right thing?
> You tried to eliminate that on Jan 22, 2020 with 
> rGdfe9f130e07c929d21f8122272077495de531a38.
> But according to git blame, the BuildTree.cpp#L437 was committed on Jul 9, 
> 2019 with rG9b3f38f99085e2bbf9db01bb00d4c6d837f0fc00.
> I'm confused.
> I'm confused.

As am I. I went back and looked at the history here, and as best I can tell, I 
tried to get rid of lvalue functions, we threw it at bots, and for reasons I 
didn't capture anywhere I can find, had to revert it. Sorry for the troubles 
with not tracking that information! :-(

However, in my simple testing on godbolt, I can't find a version of MSVC that 
has issues with lvalue or rvalue reference overloads. We claim to still support 
MSVC 2017 (perhaps it's time to bump that to something more recent now that 
MSVC has changed its release schedule), so maybe there's an older version 
that's still an issue, but I would expect us to have spotted that given there 
are unprotected uses of rvalue ref overloads.

My gut feeling is that we should explore getting rid of `LLVM_LVALUE_FUNCTION` 
and `LLVM_HAS_RVALUE_REFERENCE_THIS` again as it's been almost two years since 
the last try. If there's a problematic version of MSVC, we might want to 
consider dropping support for it unless it persists in newer MSVC versions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110810

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

[PATCH] D110586: Update `DynTypedNode` to support the conversion of `TypeLoc`s.

[James King via Phabricator via cfe-commits]

jcking1034 updated this revision to Diff 376573.
jcking1034 marked an inline comment as done.
jcking1034 added a comment.

Fix typing for `create` and add unit tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110586

Files:
  clang/include/clang/AST/ASTTypeTraits.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/unittests/AST/ASTTypeTraitsTest.cpp

Index: clang/unittests/AST/ASTTypeTraitsTest.cpp
===
--- clang/unittests/AST/ASTTypeTraitsTest.cpp
+++ clang/unittests/AST/ASTTypeTraitsTest.cpp
@@ -199,5 +199,34 @@
   EXPECT_FALSE(Node < Node);
 }
 
+TEST(DynTypedNode, TypeLoc) {
+  std::string code = R"cc(void example() { int abc; })cc";
+  auto AST = clang::tooling::buildASTFromCode(code);
+  auto &context = AST->getASTContext();
+  auto matches = match(traverse(TK_AsIs, typeLoc().bind("tl")), context);
+  for (auto &nodes : matches) {
+const auto &tl = *nodes.getNodeAs("tl");
+DynTypedNode Node = DynTypedNode::create(tl);
+EXPECT_TRUE(Node == Node);
+EXPECT_FALSE(Node < Node);
+  }
+}
+
+TEST(DynTypedNode, PointerTypeLoc) {
+  std::string code = R"cc(void example() { int* abc; })cc";
+  auto AST = clang::tooling::buildASTFromCode(code);
+  auto &context = AST->getASTContext();
+  auto matches =
+  match(traverse(TK_AsIs, varDecl(hasName("abc"),
+  hasTypeLoc(typeLoc().bind("ptl",
+context);
+  for (auto &nodes : matches) {
+const auto ptl = nodes.getNodeAs("ptl")->castAs();
+DynTypedNode Node = DynTypedNode::create(ptl);
+EXPECT_TRUE(Node == Node);
+EXPECT_FALSE(Node < Node);
+  }
+}
+
 } // namespace
 }  // namespace clang
Index: clang/lib/AST/ASTTypeTraits.cpp
===
--- clang/lib/AST/ASTTypeTraits.cpp
+++ clang/lib/AST/ASTTypeTraits.cpp
@@ -18,6 +18,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/OpenMPClause.h"
+#include "clang/AST/TypeLoc.h"
 
 using namespace clang;
 
@@ -28,6 +29,8 @@
 {NKI_None, "TemplateName"},
 {NKI_None, "NestedNameSpecifierLoc"},
 {NKI_None, "QualType"},
+#define TYPELOC(CLASS, PARENT) {NKI_##PARENT, #CLASS "TypeLoc"},
+#include "clang/AST/TypeLocNodes.def"
 {NKI_None, "TypeLoc"},
 {NKI_None, "CXXBaseSpecifier"},
 {NKI_None, "CXXCtorInitializer"},
@@ -127,6 +130,17 @@
   llvm_unreachable("invalid type kind");
  }
 
+ ASTNodeKind ASTNodeKind::getFromNode(const TypeLoc &T) {
+   switch (T.getTypeLocClass()) {
+#define ABSTRACT_TYPELOC(CLASS, PARENT)
+#define TYPELOC(CLASS, PARENT) \
+  case TypeLoc::CLASS: \
+return ASTNodeKind(NKI_##CLASS##TypeLoc);
+#include "clang/AST/TypeLocNodes.def"
+   }
+   llvm_unreachable("invalid typeloc kind");
+ }
+
 ASTNodeKind ASTNodeKind::getFromNode(const OMPClause &C) {
   switch (C.getClauseKind()) {
 #define GEN_CLANG_CLAUSE_CLASS
Index: clang/include/clang/AST/ASTTypeTraits.h
===
--- clang/include/clang/AST/ASTTypeTraits.h
+++ clang/include/clang/AST/ASTTypeTraits.h
@@ -63,6 +63,7 @@
   static ASTNodeKind getFromNode(const Decl &D);
   static ASTNodeKind getFromNode(const Stmt &S);
   static ASTNodeKind getFromNode(const Type &T);
+  static ASTNodeKind getFromNode(const TypeLoc &T);
   static ASTNodeKind getFromNode(const OMPClause &C);
   static ASTNodeKind getFromNode(const Attr &A);
   /// \}
@@ -133,6 +134,8 @@
 NKI_TemplateName,
 NKI_NestedNameSpecifierLoc,
 NKI_QualType,
+#define TYPELOC(CLASS, PARENT) NKI_##CLASS##TypeLoc,
+#include "clang/AST/TypeLocNodes.def"
 NKI_TypeLoc,
 NKI_LastKindWithoutPointerIdentity = NKI_TypeLoc,
 NKI_CXXBaseSpecifier,
@@ -198,6 +201,8 @@
 KIND_TO_KIND_ID(NestedNameSpecifier)
 KIND_TO_KIND_ID(NestedNameSpecifierLoc)
 KIND_TO_KIND_ID(QualType)
+#define TYPELOC(CLASS, PARENT) KIND_TO_KIND_ID(CLASS##TypeLoc)
+#include "clang/AST/TypeLocNodes.def"
 KIND_TO_KIND_ID(TypeLoc)
 KIND_TO_KIND_ID(Decl)
 KIND_TO_KIND_ID(Stmt)
@@ -304,7 +309,7 @@
   return getUnchecked().getAsOpaquePtr() <
  Other.getUnchecked().getAsOpaquePtr();
 
-if (ASTNodeKind::getFromNodeKind().isSame(NodeKind)) {
+if (ASTNodeKind::getFromNodeKind().isBaseOf(NodeKind)) {
   auto TLA = getUnchecked();
   auto TLB = Other.getUnchecked();
   return std::make_pair(TLA.getType().getAsOpaquePtr(),
@@ -336,7 +341,7 @@
 if (ASTNodeKind::getFromNodeKind().isSame(NodeKind))
   return getUnchecked() == Other.getUnchecked();
 
-if (ASTNodeKind::getFromNodeKind().isSame(NodeKind))
+if (ASTNodeKind::getFromNodeKind().isBaseOf(NodeKind))
   return getUnchecked() == Other.getUnchecked();
 
 if (ASTNodeKind::getFromNodeKind().isSame(NodeKind))

[PATCH] D110891: [inliner] Mandatory inlining decisions produce remarks

[Wenlei He via Phabricator via cfe-commits]

wenlei added inline comments.



Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:52
 
+namespace {
+using namespace llvm::ore;

curious why do we need anonymous namespace here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110891

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

[PATCH] D110891: [inliner] Mandatory inlining decisions produce remarks

[Mircea Trofin via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:52
 
+namespace {
+using namespace llvm::ore;

wenlei wrote:
> curious why do we need anonymous namespace here?
iiuc it's preferred we place file-local types inside an anonymous namespace. 

Looking now at the [[ 
https://llvm.org/docs/CodingStandards.html#anonymous-namespaces | style 
guideline ]], it touts their benefits but also says I should have only placed 
de decl there and the impl of those members out... but the members are quite 
trivial. Happy to move them out though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110891

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

[PATCH] D106302: Implement P1937 consteval in unevaluated contexts

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/www/cxx_status.html:1106
   https://wg21.link/p1073r3";>P1073R3
-  No
+  Partial
 

I'm trying to evaluate our consteval support, and I am having trouble finding 
any unsuperceded part of P1073R3 that is not implemented in Clang14.  Can you 
share what Delta you know of that has you considering this as 'partial'?  Is it 
something in a different patch awaiting review?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106302

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

[PATCH] D110891: [inliner] Mandatory inlining decisions produce remarks

[Wenlei He via Phabricator via cfe-commits]

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

lgtm, thanks.




Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:52
 
+namespace {
+using namespace llvm::ore;

mtrofin wrote:
> wenlei wrote:
> > curious why do we need anonymous namespace here?
> iiuc it's preferred we place file-local types inside an anonymous namespace. 
> 
> Looking now at the [[ 
> https://llvm.org/docs/CodingStandards.html#anonymous-namespaces | style 
> guideline ]], it touts their benefits but also says I should have only placed 
> de decl there and the impl of those members out... but the members are quite 
> trivial. Happy to move them out though.
Thanks for the pointer. I don't have a strong opinion but slightly leaning 
towards moving out of anonymous namespace be consistent with how other 
InlineAdvice is organized (DefaultInlineAdvice, MLInlineAdvice not in anonymous 
namespace).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110891

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

[PATCH] D110891: [inliner] Mandatory inlining decisions produce remarks

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added inline comments.



Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:72-89
+  void recordUnsuccessfulInliningImpl(const InlineResult &Result) override {
+if (IsInliningRecommended)
+  ORE.emit([&]() {
+return OptimizationRemarkMissed(DEBUG_TYPE, "NotInlined", DLoc, Block)
+   << "'" << NV("Callee", Callee) << "' is not AlwaysInline into '"
+   << NV("Caller", Caller)
+   << "': " << NV("Reason", Result.getFailureReason());

can we add a test for these?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110891

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

[PATCH] D110586: Update `DynTypedNode` to support the conversion of `TypeLoc`s.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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



Comment at: clang/unittests/AST/ASTTypeTraitsTest.cpp:207
+  auto matches = match(traverse(TK_AsIs, typeLoc().bind("tl")), context);
+  for (auto &nodes : matches) {
+const auto &tl = *nodes.getNodeAs("tl");

let's assert that `matches.size() == 1` and then just use `nodes[0]`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110586

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

[PATCH] D110891: [inliner] Mandatory inlining decisions produce remarks

[Mircea Trofin via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:52
 
+namespace {
+using namespace llvm::ore;

wenlei wrote:
> mtrofin wrote:
> > wenlei wrote:
> > > curious why do we need anonymous namespace here?
> > iiuc it's preferred we place file-local types inside an anonymous 
> > namespace. 
> > 
> > Looking now at the [[ 
> > https://llvm.org/docs/CodingStandards.html#anonymous-namespaces | style 
> > guideline ]], it touts their benefits but also says I should have only 
> > placed de decl there and the impl of those members out... but the members 
> > are quite trivial. Happy to move them out though.
> Thanks for the pointer. I don't have a strong opinion but slightly leaning 
> towards moving out of anonymous namespace be consistent with how other 
> InlineAdvice is organized (DefaultInlineAdvice, MLInlineAdvice not in 
> anonymous namespace).
Ah, those are public (i.e. in a .h file)



Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:72-89
+  void recordUnsuccessfulInliningImpl(const InlineResult &Result) override {
+if (IsInliningRecommended)
+  ORE.emit([&]() {
+return OptimizationRemarkMissed(DEBUG_TYPE, "NotInlined", DLoc, Block)
+   << "'" << NV("Callee", Callee) << "' is not AlwaysInline into '"
+   << NV("Caller", Caller)
+   << "': " << NV("Reason", Result.getFailureReason());

aeubanks wrote:
> can we add a test for these?
I think that would be tricky, because they should not actually happen - the way 
we determine whether a site is alwaysinlinable checks (but not thoroughly) for 
legality. Let me see if I can find a regression test. It may be we can 
synthesize such a case in IR only, though, so not much of a help for the 
frontend tests?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110891

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

[PATCH] D110891: [inliner] Mandatory inlining decisions produce remarks

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:52
 
+namespace {
+using namespace llvm::ore;

mtrofin wrote:
> wenlei wrote:
> > mtrofin wrote:
> > > wenlei wrote:
> > > > curious why do we need anonymous namespace here?
> > > iiuc it's preferred we place file-local types inside an anonymous 
> > > namespace. 
> > > 
> > > Looking now at the [[ 
> > > https://llvm.org/docs/CodingStandards.html#anonymous-namespaces | style 
> > > guideline ]], it touts their benefits but also says I should have only 
> > > placed de decl there and the impl of those members out... but the members 
> > > are quite trivial. Happy to move them out though.
> > Thanks for the pointer. I don't have a strong opinion but slightly leaning 
> > towards moving out of anonymous namespace be consistent with how other 
> > InlineAdvice is organized (DefaultInlineAdvice, MLInlineAdvice not in 
> > anonymous namespace).
> Ah, those are public (i.e. in a .h file)
Generally if a type is declared/defined inside a .cpp file it should be in an 
anonymous namespace so it can't collide with other implementation type names in 
other .cpp files. (& .cpp-local functions should be static or in an anonymous 
namespace for the same reason) 

Looks like the other two (`DefaultInlineAdvice` and `MLInlineAdvice`) are 
defined in headers, so they must not be in anonymous namespaces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110891

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

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 376559.
gandhi21299 added a comment.

- Since callees may alias to a function pointer, it makes sense for 
`getCalleeFunction(...)` to return a `Function` which is a cast of the operand 
of a `GlobalAlias`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109707

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGenCUDA/amdgpu-alias-undef-symbols.cu
  llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp
  llvm/lib/Target/AMDGPU/AMDGPUCallLowering.cpp
  llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/test/CodeGen/AMDGPU/inline-calls.ll

Index: llvm/test/CodeGen/AMDGPU/inline-calls.ll
===
--- llvm/test/CodeGen/AMDGPU/inline-calls.ll
+++ llvm/test/CodeGen/AMDGPU/inline-calls.ll
@@ -1,6 +1,5 @@
 ; RUN: llc -march=amdgcn -mcpu=tahiti -verify-machineinstrs < %s | FileCheck  %s
 ; RUN: llc -march=amdgcn -mcpu=tonga -verify-machineinstrs < %s | FileCheck  %s
-; RUN: llc -march=r600 -mcpu=redwood -verify-machineinstrs < %s | FileCheck %s
 
 ; ALL-NOT: {{^}}func:
 define internal i32 @func(i32 %a) {
@@ -18,8 +17,8 @@
   ret void
 }
 
-; CHECK-NOT: func_alias
-; ALL-NOT: func_alias
+; CHECK: func_alias
+; ALL: func_alias
 @func_alias = alias i32 (i32), i32 (i32)* @func
 
 ; ALL: {{^}}kernel3:
Index: llvm/lib/Target/AMDGPU/SIISelLowering.cpp
===
--- llvm/lib/Target/AMDGPU/SIISelLowering.cpp
+++ llvm/lib/Target/AMDGPU/SIISelLowering.cpp
@@ -3007,6 +3007,7 @@
   bool IsSibCall = false;
   bool IsThisReturn = false;
   MachineFunction &MF = DAG.getMachineFunction();
+  GlobalAddressSDNode *GSD = dyn_cast(Callee);
 
   if (Callee.isUndef() || isNullConstant(Callee)) {
 if (!CLI.IsTailCall) {
@@ -3264,7 +3265,7 @@
   Ops.push_back(Callee);
   // Add a redundant copy of the callee global which will not be legalized, as
   // we need direct access to the callee later.
-  if (GlobalAddressSDNode *GSD = dyn_cast(Callee)) {
+  if (GSD) {
 const GlobalValue *GV = GSD->getGlobal();
 Ops.push_back(DAG.getTargetGlobalAddress(GV, DL, MVT::i64));
   } else {
Index: llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
===
--- llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
@@ -29,6 +29,8 @@
 #include "SIMachineFunctionInfo.h"
 #include "llvm/Analysis/CallGraph.h"
 #include "llvm/CodeGen/TargetPassConfig.h"
+#include "llvm/IR/GlobalAlias.h"
+#include "llvm/IR/GlobalValue.h"
 #include "llvm/Target/TargetMachine.h"
 
 using namespace llvm;
@@ -61,7 +63,8 @@
 assert(Op.getImm() == 0);
 return nullptr;
   }
-
+  if (auto *GA = dyn_cast(Op.getGlobal()))
+return cast(GA->getOperand(0));
   return cast(Op.getGlobal());
 }
 
Index: llvm/lib/Target/AMDGPU/AMDGPUCallLowering.cpp
===
--- llvm/lib/Target/AMDGPU/AMDGPUCallLowering.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUCallLowering.cpp
@@ -913,14 +913,17 @@
   if (Info.Callee.isReg()) {
 CallInst.addReg(Info.Callee.getReg());
 CallInst.addImm(0);
-  } else if (Info.Callee.isGlobal() && Info.Callee.getOffset() == 0) {
-// The call lowering lightly assumed we can directly encode a call target in
-// the instruction, which is not the case. Materialize the address here.
+  } else if (Info.Callee.isGlobal()) {
 const GlobalValue *GV = Info.Callee.getGlobal();
-auto Ptr = MIRBuilder.buildGlobalValue(
-  LLT::pointer(GV->getAddressSpace(), 64), GV);
-CallInst.addReg(Ptr.getReg(0));
-CallInst.add(Info.Callee);
+if (Info.Callee.getOffset() == 0) {
+  // The call lowering lightly assumed we can directly encode a call target
+  // in the instruction, which is not the case. Materialize the address
+  // here.
+  auto Ptr = MIRBuilder.buildGlobalValue(
+  LLT::pointer(GV->getAddressSpace(), 64), GV);
+  CallInst.addReg(Ptr.getReg(0));
+  CallInst.add(Info.Callee);
+}
   } else
 return false;
 
Index: llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp
===
--- llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp
@@ -93,6 +93,8 @@
 
   for (GlobalAlias &A : M.aliases()) {
 if (Function* F = dyn_cast(A.getAliasee())) {
+  if (A.getLinkage() != GlobalValue::InternalLinkage)
+continue;
   A.replaceAllUsesWith(F);
   AliasesToRemove.push_back(&A);
 }
Index: clang/test/CodeGenCUDA/amdgpu-alias-undef-symbols.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/amdgpu-alias-undef-symbols.cu
@@ -0,0 +1,17 @@
+// RE

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 376564.
gandhi21299 added a comment.

- eliminated changes in SIISelLowering


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109707

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGenCUDA/amdgpu-alias-undef-symbols.cu
  llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp
  llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
  llvm/test/CodeGen/AMDGPU/inline-calls.ll


Index: llvm/test/CodeGen/AMDGPU/inline-calls.ll
===
--- llvm/test/CodeGen/AMDGPU/inline-calls.ll
+++ llvm/test/CodeGen/AMDGPU/inline-calls.ll
@@ -1,6 +1,5 @@
 ; RUN: llc -march=amdgcn -mcpu=tahiti -verify-machineinstrs < %s | FileCheck  
%s
 ; RUN: llc -march=amdgcn -mcpu=tonga -verify-machineinstrs < %s | FileCheck  %s
-; RUN: llc -march=r600 -mcpu=redwood -verify-machineinstrs < %s | FileCheck %s
 
 ; ALL-NOT: {{^}}func:
 define internal i32 @func(i32 %a) {
@@ -18,8 +17,8 @@
   ret void
 }
 
-; CHECK-NOT: func_alias
-; ALL-NOT: func_alias
+; CHECK: func_alias
+; ALL: func_alias
 @func_alias = alias i32 (i32), i32 (i32)* @func
 
 ; ALL: {{^}}kernel3:
Index: llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
===
--- llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
@@ -29,6 +29,8 @@
 #include "SIMachineFunctionInfo.h"
 #include "llvm/Analysis/CallGraph.h"
 #include "llvm/CodeGen/TargetPassConfig.h"
+#include "llvm/IR/GlobalAlias.h"
+#include "llvm/IR/GlobalValue.h"
 #include "llvm/Target/TargetMachine.h"
 
 using namespace llvm;
@@ -61,7 +63,8 @@
 assert(Op.getImm() == 0);
 return nullptr;
   }
-
+  if (auto *GA = dyn_cast(Op.getGlobal()))
+return cast(GA->getOperand(0));
   return cast(Op.getGlobal());
 }
 
Index: llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp
===
--- llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp
@@ -93,6 +93,8 @@
 
   for (GlobalAlias &A : M.aliases()) {
 if (Function* F = dyn_cast(A.getAliasee())) {
+  if (A.getLinkage() != GlobalValue::InternalLinkage)
+continue;
   A.replaceAllUsesWith(F);
   AliasesToRemove.push_back(&A);
 }
Index: clang/test/CodeGenCUDA/amdgpu-alias-undef-symbols.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/amdgpu-alias-undef-symbols.cu
@@ -0,0 +1,17 @@
+// REQUIRES: amdgpu-registered-target, clang-driver
+
+// RUN: %clang --offload-arch=gfx906 --cuda-device-only -x hip -emit-llvm -S 
-o - %s \
+// RUN:   -fgpu-rdc -O3 -mllvm -amdgpu-early-inline-all=true -mllvm 
-amdgpu-function-calls=false | \
+// RUN:   FileCheck %s
+
+#include "Inputs/cuda.h"
+
+// CHECK: %struct.B = type { i8 }
+struct B {
+
+  // CHECK: @_ZN1BC1Ei = hidden unnamed_addr alias void (%struct.B*, i32), 
void (%struct.B*, i32)* @_ZN1BC2Ei
+  __device__ B(int x);
+};
+
+__device__ B::B(int x) {
+}
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5102,9 +5102,9 @@
   }
 
   // Enable -mconstructor-aliases except on darwin, where we have to work 
around
-  // a linker bug (see ), and CUDA/AMDGPU device code,
-  // where aliases aren't supported.
-  if (!RawTriple.isOSDarwin() && !RawTriple.isNVPTX() && !RawTriple.isAMDGPU())
+  // a linker bug (see ), and CUDA device code, where
+  // aliases aren't supported.
+  if (!RawTriple.isOSDarwin() && !RawTriple.isNVPTX())
 CmdArgs.push_back("-mconstructor-aliases");
 
   // Darwin's kernel doesn't support guard variables; just die if we


Index: llvm/test/CodeGen/AMDGPU/inline-calls.ll
===
--- llvm/test/CodeGen/AMDGPU/inline-calls.ll
+++ llvm/test/CodeGen/AMDGPU/inline-calls.ll
@@ -1,6 +1,5 @@
 ; RUN: llc -march=amdgcn -mcpu=tahiti -verify-machineinstrs < %s | FileCheck  %s
 ; RUN: llc -march=amdgcn -mcpu=tonga -verify-machineinstrs < %s | FileCheck  %s
-; RUN: llc -march=r600 -mcpu=redwood -verify-machineinstrs < %s | FileCheck %s
 
 ; ALL-NOT: {{^}}func:
 define internal i32 @func(i32 %a) {
@@ -18,8 +17,8 @@
   ret void
 }
 
-; CHECK-NOT: func_alias
-; ALL-NOT: func_alias
+; CHECK: func_alias
+; ALL: func_alias
 @func_alias = alias i32 (i32), i32 (i32)* @func
 
 ; ALL: {{^}}kernel3:
Index: llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
===
--- llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
@@ -29,6 +29,8 @@
 #include "SIMachineFunctionInfo.h"
 #include "llvm/Analysis/CallGraph.

[PATCH] D110428: [AIX] Define WCHAR_T_TYPE as unsigned short on AIX for wchar.c test case.

[David Tenty via Phabricator via cfe-commits]

daltenty added a comment.

This doesn't appear to be true for 64-bit AIX:

  extern "C" int printf(const char *, ...);
  int main() {
  printf("wchar_t: %ld\nunsigned short: 
%ld\n",sizeof(wchar_t),sizeof(unsigned short));
  return 0;
  }
  $ clang++ -m64 foo.cc
  $ ./a.out
  wchar_t: 4
  unsigned short: 2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110428

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

[PATCH] D106302: Implement P1937 consteval in unevaluated contexts

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/www/cxx_status.html:1106
   https://wg21.link/p1073r3";>P1073R3
-  No
+  Partial
 

erichkeane wrote:
> I'm trying to evaluate our consteval support, and I am having trouble finding 
> any unsuperceded part of P1073R3 that is not implemented in Clang14.  Can you 
> share what Delta you know of that has you considering this as 'partial'?  Is 
> it something in a different patch awaiting review?
This thing https://reviews.llvm.org/D74130 is what I believe the last bit of 
missing functionality.
Note that I only implemented P1937 myself


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106302

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

[PATCH] D106302: Implement P1937 consteval in unevaluated contexts

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/www/cxx_status.html:1106
   https://wg21.link/p1073r3";>P1073R3
-  No
+  Partial
 

cor3ntin wrote:
> erichkeane wrote:
> > I'm trying to evaluate our consteval support, and I am having trouble 
> > finding any unsuperceded part of P1073R3 that is not implemented in 
> > Clang14.  Can you share what Delta you know of that has you considering 
> > this as 'partial'?  Is it something in a different patch awaiting review?
> This thing https://reviews.llvm.org/D74130 is what I believe the last bit of 
> missing functionality.
> Note that I only implemented P1937 myself
Ah, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106302

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

[PATCH] D77598: Integral template argument suffix and cast printing

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D77598#3035591 , @v.g.vassilev 
wrote:

> In D77598#3035449 , @dblaikie wrote:
>
>> Came across this while trying to do "simplified template names" - producing 
>> template names in DWARF without template parameter lists as part of the 
>> textual name, then rebuilding that name from the structural representation 
>> of template parameters in DWARF (DW_TAG_template_*_parameter, etc). The 
>> roundtripping is implemented to ensure that the simplified names are 
>> non-lossy - that all the data is still available through the structural 
>> representation. (some names are harder or not currently possible to rebuild)
>>
>> The selective use of suffixes, especially contextually (overloading) seems 
>> like something I'll probably want to avoid entirely for DWARF to keep the 
>> names consistent across different contexts - but I am also just a bit 
>> confused about some things I'm seeing with this change.
>>
>> For instance, it looks like 
>> https://github.com/llvm/llvm-project/blob/fcdefc8575866a36e80e91024b876937ae6a9965/clang/lib/AST/Decl.cpp#L2900
>>  doesn't pass the list of template parameters, so function template names 
>> always get suffixes on their integer parameters.
>>
>> Whereas the equivalent calls for printing class template specialization 
>> names here: 
>> https://github.com/llvm/llvm-project/blob/fcdefc8575866a36e80e91024b876937ae6a9965/clang/lib/AST/DeclTemplate.cpp#L949-L959
>>  - is that just a minor bug/missing functionality?
>>
>> I'm testing a fix for that:
>>
>>   diff --git clang/lib/AST/Decl.cpp clang/lib/AST/Decl.cpp
>>   index 60ca8633224b..11cf068d2c4c 100644
>>   --- clang/lib/AST/Decl.cpp
>>   +++ clang/lib/AST/Decl.cpp
>>   @@ -2897,7 +2897,7 @@ void FunctionDecl::getNameForDiagnostic(
>>  NamedDecl::getNameForDiagnostic(OS, Policy, Qualified);
>>  const TemplateArgumentList *TemplateArgs = 
>> getTemplateSpecializationArgs();
>>  if (TemplateArgs)
>>   -printTemplateArgumentList(OS, TemplateArgs->asArray(), Policy);
>>   +printTemplateArgumentList(OS, TemplateArgs->asArray(), Policy, 
>> getPrimaryTemplate()->getTemplateParameters());
>>}
>>
>>bool FunctionDecl::isVariadic() const {
>>
>> I've sent out a patch with ^ applied/cleanups applied: D77598 
>> 
>
> This seems an oversight on our end, thanks! You probably meant to link 
> https://reviews.llvm.org/D110898

Yep, thanks for the correction!

Do you have any interest/bandwidth to look into the one with nested namespaces 
(the other code I pointed to, in NestedNameSpecifier.cpp, above) case? At a 
quick glance I wasn't able to quite figure out how to get the template 
parameters from the TemplateSpecializationType or 
DependentTemplateSpecializationType to pass in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77598

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

[PATCH] D110665: [clang] Don't use the AST to display backend diagnostics

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110665

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

[PATCH] D110891: [inliner] Mandatory inlining decisions produce remarks

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added inline comments.



Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:72-89
+  void recordUnsuccessfulInliningImpl(const InlineResult &Result) override {
+if (IsInliningRecommended)
+  ORE.emit([&]() {
+return OptimizationRemarkMissed(DEBUG_TYPE, "NotInlined", DLoc, Block)
+   << "'" << NV("Callee", Callee) << "' is not AlwaysInline into '"
+   << NV("Caller", Caller)
+   << "': " << NV("Reason", Result.getFailureReason());

mtrofin wrote:
> aeubanks wrote:
> > can we add a test for these?
> I think that would be tricky, because they should not actually happen - the 
> way we determine whether a site is alwaysinlinable checks (but not 
> thoroughly) for legality. Let me see if I can find a regression test. It may 
> be we can synthesize such a case in IR only, though, so not much of a help 
> for the frontend tests?
yeah some IR tests is what I was thinking


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110891

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

[PATCH] D110935: [NFC] Update vec_extract builtin signatures to take signed int.

[Quinn Pham via Phabricator via cfe-commits]

quinnp added a comment.

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110935

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

[PATCH] D110954: [clangd] Improve PopulateSwitch tweak

[David Goldman via Phabricator via cfe-commits]

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

- Support enums in C and ObjC as their AST representations differ slightly.

- Add support for typedef'ed enums.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110954

Files:
  clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
  clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp
@@ -23,6 +23,7 @@
 CodeContext Context;
 llvm::StringRef TestSource;
 llvm::StringRef ExpectedSource;
+llvm::StringRef FileName = "TestTU.cpp";
   };
 
   Case Cases[]{
@@ -206,10 +207,56 @@
   R""(template void f() {enum Enum {A}; ^switch (A) {}})"",
   "unavailable",
   },
+  {// C: Only filling in missing enumerators
+   Function,
+   R""(
+enum CEnum {A,B,C};
+enum CEnum val = A;
+^switch (val) {case B:break;}
+  )"",
+   R""(
+enum CEnum {A,B,C};
+enum CEnum val = A;
+switch (val) {case B:break;case A:case C:break;}
+  )"",
+   "TestTU.c"},
+  {// ObjC: Only filling in missing enumerators
+   Function,
+   R""(
+enum ObjCEnum {A,B,C};
+enum ObjCEnum val = B;
+^switch (val) {case A:break;}
+  )"",
+   R""(
+enum ObjCEnum {A,B,C};
+enum ObjCEnum val = B;
+switch (val) {case A:break;case B:case C:break;}
+  )"",
+   "TestTU.m"},
+  {// ObjC: Only filling in missing enumerators w/ typedefs
+   Function,
+   R""(
+typedef unsigned long UInteger;
+enum ControlState : UInteger;
+typedef enum ControlState ControlState;
+enum ControlState : UInteger {A,B,C};
+ControlState controlState = A;
+switch (^controlState) {case A:break;}
+  )"",
+   R""(
+typedef unsigned long UInteger;
+enum ControlState : UInteger;
+typedef enum ControlState ControlState;
+enum ControlState : UInteger {A,B,C};
+ControlState controlState = A;
+switch (controlState) {case A:break;case B:case C:break;}
+  )"",
+   "TestTU.m"},
   };
 
   for (const auto &Case : Cases) {
 Context = Case.Context;
+FileName = Case.FileName;
 EXPECT_EQ(apply(Case.TestSource), Case.ExpectedSource);
   }
 }
Index: clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
@@ -113,7 +113,8 @@
 return false;
   // Ignore implicit casts, since enums implicitly cast to integer types.
   Cond = Cond->IgnoreParenImpCasts();
-  EnumT = Cond->getType()->getAsAdjusted();
+  // Get the canonical type to handle typedefs.
+  EnumT = Cond->getType().getCanonicalType()->getAsAdjusted();
   if (!EnumT)
 return false;
   EnumD = EnumT->getDecl();
@@ -152,14 +153,30 @@
 if (CS->caseStmtIsGNURange())
   return false;
 
+// Support for direct references to enum constants. This is required to
+// support C and ObjC which don't contain values in their ConstantExprs.
+// The general way to get the value of a case is EvaluateAsRValue, but we'd
+// rather not deal with that in case the AST is broken.
+if (auto *DRE = dyn_cast(CS->getLHS()->IgnoreParenCasts())) {
+  if (auto *Enumerator = dyn_cast(DRE->getDecl())) {
+auto Iter = ExpectedCases.find(Normalize(Enumerator->getInitVal()));
+if (Iter != ExpectedCases.end())
+  Iter->second.setCovered();
+continue;
+  }
+}
+
+// ConstantExprs with values are expected for C++, otherwise the storage
+// kind will be None.
+
 // Case expression is not a constant expression or is value-dependent,
 // so we may not be able to work out which cases are covered.
 const ConstantExpr *CE = dyn_cast(CS->getLHS());
 if (!CE || CE->isValueDependent())
   return false;
 
-// Unsure if this case could ever come up, but prevents an unreachable
-// executing in getResultAsAPSInt.
+// We need a storage kind in order to be able to fetch the value type,
+// currently both C and ObjC enums will return none.
 if (CE->getResultStorageKind() == ConstantExpr::RSK_None)
   return false;
 auto Iter = ExpectedCases.find(Normalize(CE->getR

[PATCH] D110955: [AIX] Don't pass namedsects in LTO mode

[Jinsong Ji via Phabricator via cfe-commits]

jsji created this revision.
jsji added reviewers: PowerPC, Whitney.
Herald added a subscriber: inglorion.
jsji requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

LTO don't need binder option , don't pass it in LTO mode.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110955

Files:
  clang/lib/Driver/ToolChains/AIX.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
@@ -649,3 +649,60 @@
 // CHECK-LD64-SHARED-NOT: "--no-as-needed"
 // CHECK-LD64-SHARED: "-lm"
 // CHECK-LD64-SHARED: "-lc"
+
+// Check powerpc-ibm-aix7.3.0.0, -fprofile-generate
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:-static \
+// RUN:-fprofile-generate\
+// RUN:-target powerpc-ibm-aix7.3.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:-unwindlib=libunwind \
+// RUN:   | FileCheck --check-prefix=CHECK-PGO-NON-LTO %s
+// CHECK-PGO-NON-LTO-NOT: warning:
+// CHECK-PGO-NON-LTO: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc-ibm-aix7.3.0.0"
+// CHECK-PGO-NON-LTO: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-PGO-NON-LTO: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-PGO-NON-LTO: "{{.*}}ld{{(.exe)?}}"
+// CHECK-PGO-NON-LTO: "-bdbg:namedsects"
+// CHECK-PGO-NON-LTO: "-b32"
+// CHECK-PGO-NON-LTO: "-bpT:0x1000" "-bpD:0x2000"
+// CHECK-PGO-NON-LTO: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-PGO-NON-LTO: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
+// CHECK-PGO-NON-LTO-NOT: "-lc++"
+// CHECK-PGO-NON-LTO-NOT: "-lc++abi"
+// CHECK-PGO-NON-LTO: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-PGO-NON-LTO-NOT: "--as-needed"
+// CHECK-PGO-NON-LTO-NOT: "-lunwind"
+// CHECK-PGO-NON-LTO-NOT: "--no-as-needed"
+// CHECK-PGO-NON-LTO-NOT: "-lm"
+// CHECK-PGO-NON-LTO: "-lc"
+
+// Check powerpc-ibm-aix7.2.5.3, -fprofile-generate, -flto
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:-static \
+// RUN:-fprofile-generate\
+// RUN:-flto\
+// RUN:-target powerpc-ibm-aix7.2.5.3 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:-unwindlib=libunwind \
+// RUN:   | FileCheck --check-prefix=CHECK-PGO-LTO %s
+// CHECK-PGO-LTO-NOT: warning:
+// CHECK-PGO-LTO: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc-ibm-aix7.2.5.3"
+// CHECK-PGO-LTO: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-PGO-LTO: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-PGO-LTO: "{{.*}}ld{{(.exe)?}}"
+// CHECK-PGO-LTO-NOT: "-bdbg:namedsects"
+// CHECK-PGO-LTO: "-b32"
+// CHECK-PGO-LTO: "-bpT:0x1000" "-bpD:0x2000"
+// CHECK-PGO-LTO: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-PGO-LTO: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
+// CHECK-PGO-LTO-NOT: "-lc++"
+// CHECK-PGO-LTO-NOT: "-lc++abi"
+// CHECK-PGO-LTO: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-PGO-LTO-NOT: "--as-needed"
+// CHECK-PGO-LTO-NOT: "-lunwind"
+// CHECK-PGO-LTO-NOT: "--no-as-needed"
+// CHECK-PGO-LTO-NOT: "-lm"
+// CHECK-PGO-LTO: "-lc"
Index: clang/lib/Driver/ToolChains/AIX.cpp
===
--- clang/lib/Driver/ToolChains/AIX.cpp
+++ clang/lib/Driver/ToolChains/AIX.cpp
@@ -98,8 +98,9 @@
 CmdArgs.push_back("-bnoentry");
   }
 
-  // Specify PGO linker option
-  if ((Args.hasFlag(options::OPT_fprofile_arcs, options::OPT_fno_profile_arcs,
+  // Specify PGO linker option without LTO
+  if (!D.isUsingLTO() &&
+  (Args.hasFlag(options::OPT_fprofile_arcs, options::OPT_fno_profile_arcs,
 false) ||
Args.hasFlag(options::OPT_fprofile_generate,
 options::OPT_fno_profile_generate, false) ||


Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -649,3 +649,60 @@
 // CHECK-LD64-SHARED-NOT: "--no-as-needed"
 // CHECK-LD64-SHARED: "-lm"
 // CHECK-LD64-SHARED: "-lc"
+
+// Check powerpc-ibm-aix7.3.0.0, -fprofile-generate
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:-static \
+// RUN:-fprofile-generate\
+// RUN:-target powerpc-ibm-aix7.3.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:-unwindlib=libunwind \
+// RUN:   | FileCheck --check-prefix=CHECK-PGO-NON-LTO %s
+// CHECK-PGO-NON-LTO-NOT: warning:
+// CHECK-PGO-NON-LTO: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.3.0.0"
+// CHECK-PGO-NON-LTO: "-resource-dir" "[[RESOUR

[PATCH] D110934: [NFC] Update return type of vec_popcnt to vector unsigned.

[Quinn Pham via Phabricator via cfe-commits]

quinnp added a comment.

I think the commit message needs to be updated.

> This patch updates the vec_popcnt builtins to take a signed int as the second 
> parameter...

Should be: This patch updates the return type of the vec_popcnt builtins to 
vector unsigned...

Other than that, lgtm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110934

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