[PATCH] D85401: [clang-tidy] Fix bugprone-bad-signal-to-kill-thread crash when `SIGTERM` was undefined after definition.

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp:1
+// RUN: clang-tidy %s --checks="-*,bugprone-bad-signal-to-kill-thread"
+

nit: `// RUN: clang-tidy %s -checks=-*,bugprone-bad-signal-to-kill-thread -- | 
count 0`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85401

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

[PATCH] D85398: [clang-tidy] Fix bugprone-bad-signal-to-kill-thread crash when `SIGTERM` is not a literal.

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp:1
+// RUN: clang-tidy %s --checks="-*,bugprone-bad-signal-to-kill-thread"
+

the same here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85398

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

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Also with a name like compare op parenthesis. It sounds like this would 
consider `==` and `!=`




Comment at: clang/lib/Sema/SemaExpr.cpp:14034
+  << FixItHint::CreateInsertion(LHSExpr->getEndLoc(), ") && ")
+  << FixItHint::CreateInsertion(LHSBO->getRHS()->getBeginLoc(), "(y ")
+  << FixItHint::CreateInsertion(RHSExpr->getEndLoc(), ")");

You don't want to insert `y` but the source code for `y`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85097

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

[PATCH] D85401: [clang-tidy] Fix bugprone-bad-signal-to-kill-thread crash when `SIGTERM` was undefined after definition.

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX updated this revision to Diff 283503.
ArcsinX added a comment.

Fix test according to review comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85401

Files:
  clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp
@@ -0,0 +1,11 @@
+// RUN: clang-tidy %s --checks=-*,bugprone-bad-signal-to-kill-thread -- | 
count 0
+
+#define SIGTERM 15
+#undef SIGTERM // no-crash
+using pthread_t = int;
+int pthread_kill(pthread_t thread, int sig);
+
+int func() {
+  pthread_t thread;
+  return pthread_kill(thread, 0);
+}
Index: clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
@@ -30,7 +30,8 @@
 
 void BadSignalToKillThreadCheck::check(const MatchFinder::MatchResult &Result) 
{
   const auto IsSigterm = [](const auto &KeyValue) -> bool {
-return KeyValue.first->getName() == "SIGTERM";
+return KeyValue.first->getName() == "SIGTERM" &&
+   KeyValue.first->hasMacroDefinition();
   };
   const auto TryExpandAsInteger =
   [](Preprocessor::macro_iterator It) -> Optional {


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp
@@ -0,0 +1,11 @@
+// RUN: clang-tidy %s --checks=-*,bugprone-bad-signal-to-kill-thread -- | count 0
+
+#define SIGTERM 15
+#undef SIGTERM // no-crash
+using pthread_t = int;
+int pthread_kill(pthread_t thread, int sig);
+
+int func() {
+  pthread_t thread;
+  return pthread_kill(thread, 0);
+}
Index: clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
@@ -30,7 +30,8 @@
 
 void BadSignalToKillThreadCheck::check(const MatchFinder::MatchResult &Result) {
   const auto IsSigterm = [](const auto &KeyValue) -> bool {
-return KeyValue.first->getName() == "SIGTERM";
+return KeyValue.first->getName() == "SIGTERM" &&
+   KeyValue.first->hasMacroDefinition();
   };
   const auto TryExpandAsInteger =
   [](Preprocessor::macro_iterator It) -> Optional {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85401: [clang-tidy] Fix bugprone-bad-signal-to-kill-thread crash when `SIGTERM` was undefined after definition.

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp:1
+// RUN: clang-tidy %s --checks="-*,bugprone-bad-signal-to-kill-thread"
+

hokein wrote:
> nit: `// RUN: clang-tidy %s -checks=-*,bugprone-bad-signal-to-kill-thread -- 
> | count 0`
Fixed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85401

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

[PATCH] D85330: [SyntaxTree] Extend the syntax tree dump

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 283506.
eduucaldas added a comment.

Answer comment.

Please suggest more descriptive markers.
Options I thought of:
`I` -> `unmodifiable`
`M` -> `not backed by source code` / `synthesized`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85330

Files:
  clang/include/clang/Tooling/Syntax/Tree.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Tree.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -159,7 +159,8 @@
  << "Source file has syntax errors, they were printed to the test "
 "log";
 }
-std::string Actual = std::string(StringRef(Root->dump(*Arena)).trim());
+std::string Actual =
+std::string(StringRef(Root->dump(Arena->sourceManager())).trim());
 // EXPECT_EQ shows the diff between the two strings if they are different.
 EXPECT_EQ(Tree.trim().str(), Actual);
 if (Actual != Tree.trim().str()) {
Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -135,46 +135,45 @@
 }
 
 namespace {
-static void dumpTokens(llvm::raw_ostream &OS, ArrayRef Tokens,
-   const SourceManager &SM) {
-  assert(!Tokens.empty());
-  bool First = true;
-  for (const auto &T : Tokens) {
-if (!First)
-  OS << " ";
-else
-  First = false;
-// Handle 'eof' separately, calling text() on it produces an empty string.
-if (T.kind() == tok::eof) {
-  OS << "";
-  continue;
-}
-OS << T.text(SM);
-  }
+static void dumpLeaf(llvm::raw_ostream &OS, const syntax::Leaf *L,
+ const SourceManager &SM) {
+  assert(L);
+  const auto *Token = L->token();
+  assert(Token);
+  // Handle 'eof' separately, calling text() on it produces an empty string.
+  if (Token->kind() == tok::eof)
+OS << "";
+  else
+OS << Token->text(SM);
 }
 
-static void dumpTree(llvm::raw_ostream &OS, const syntax::Node *N,
- const syntax::Arena &A, std::vector IndentMask) {
-  std::string Marks;
-  if (!N->isOriginal())
-Marks += "M";
-  if (N->role() == syntax::NodeRole::Detached)
-Marks += "*"; // FIXME: find a nice way to print other roles.
-  if (!N->canModify())
-Marks += "I";
-  if (!Marks.empty())
-OS << Marks << ": ";
-
+static void dumpNode(llvm::raw_ostream &OS, const syntax::Node *N,
+ const SourceManager &SM, std::vector IndentMask) {
+  auto dumpExtraInfo = [&OS](const syntax::Node *N) {
+if (N->role() != syntax::NodeRole::Unknown)
+  OS << " " << N->role();
+if (!N->isOriginal())
+  OS << " M";
+if (!N->canModify())
+  OS << " I";
+  };
+
+  assert(N);
   if (auto *L = llvm::dyn_cast(N)) {
-dumpTokens(OS, *L->token(), A.sourceManager());
+OS << "'";
+dumpLeaf(OS, L, SM);
+OS << "'";
+dumpExtraInfo(N);
 OS << "\n";
 return;
   }
 
   auto *T = llvm::cast(N);
-  OS << T->kind() << "\n";
+  OS << T->kind();
+  dumpExtraInfo(N);
+  OS << "\n";
 
-  for (auto It = T->firstChild(); It != nullptr; It = It->nextSibling()) {
+  for (const auto *It = T->firstChild(); It; It = It->nextSibling()) {
 for (bool Filled : IndentMask) {
   if (Filled)
 OS << "| ";
@@ -188,28 +187,27 @@
   OS << "|-";
   IndentMask.push_back(true);
 }
-dumpTree(OS, It, A, IndentMask);
+dumpNode(OS, It, SM, IndentMask);
 IndentMask.pop_back();
   }
 }
 } // namespace
 
-std::string syntax::Node::dump(const Arena &A) const {
+std::string syntax::Node::dump(const SourceManager &SM) const {
   std::string Str;
   llvm::raw_string_ostream OS(Str);
-  dumpTree(OS, this, A, /*IndentMask=*/{});
+  dumpNode(OS, this, SM, /*IndentMask=*/{});
   return std::move(OS.str());
 }
 
-std::string syntax::Node::dumpTokens(const Arena &A) const {
+std::string syntax::Node::dumpTokens(const SourceManager &SM) const {
   std::string Storage;
   llvm::raw_string_ostream OS(Storage);
   traverse(this, [&](const syntax::Node *N) {
-auto *L = llvm::dyn_cast(N);
-if (!L)
-  return;
-::dumpTokens(OS, *L->token(), A.sourceManager());
-OS << " ";
+if (auto *L = llvm::dyn_cast(N)) {
+  dumpLeaf(OS, L, SM);
+  OS << " ";
+}
   });
   return OS.str();
 }
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -526,7 +526,7 @@
 R += std::string(llvm::formatv(
 "- '{0}' covers '{1}'+{2} tokens\n", It->second->kind(),
 It->first->text(A.sour

[clang] 504a197 - [X86] Rename X86::getImpliedFeatures to X86::updateImpliedFeatures and pass clang's StringMap directly to it.

[Craig Topper via cfe-commits]

Author: Craig Topper
Date: 2020-08-06T00:20:46-07:00
New Revision: 504a197fe54de83be791ea0aa2ed290f6b9285b0

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

LOG: [X86] Rename X86::getImpliedFeatures to X86::updateImpliedFeatures and 
pass clang's StringMap directly to it.

No point in building a vector of StringRefs for clang to apply to the
StringMap. Just pass the StringMap and modify it directly.

Added: 


Modified: 
clang/lib/Basic/Targets/X86.cpp
llvm/include/llvm/Support/X86TargetParser.h
llvm/lib/Support/X86TargetParser.cpp

Removed: 




diff  --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
index 543f232d2459..8b8f7d43b277 100644
--- a/clang/lib/Basic/Targets/X86.cpp
+++ b/clang/lib/Basic/Targets/X86.cpp
@@ -159,11 +159,7 @@ void 
X86TargetInfo::setFeatureEnabled(llvm::StringMap &Features,
   }
 
   Features[Name] = Enabled;
-
-  SmallVector ImpliedFeatures;
-  llvm::X86::getImpliedFeatures(Name, Enabled, ImpliedFeatures);
-  for (const auto &F : ImpliedFeatures)
-Features[F] = Enabled;
+  llvm::X86::updateImpliedFeatures(Name, Enabled, Features);
 }
 
 /// handleTargetFeatures - Perform initialization based on the user

diff  --git a/llvm/include/llvm/Support/X86TargetParser.h 
b/llvm/include/llvm/Support/X86TargetParser.h
index 66c474b5c275..a26ac8dac3ba 100644
--- a/llvm/include/llvm/Support/X86TargetParser.h
+++ b/llvm/include/llvm/Support/X86TargetParser.h
@@ -14,6 +14,7 @@
 #define LLVM_SUPPORT_X86TARGETPARSERCOMMON_H
 
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
 
 namespace llvm {
 class StringRef;
@@ -137,10 +138,10 @@ ProcessorFeatures getKeyFeature(CPUKind Kind);
 /// Fill in the features that \p CPU supports into \p Features.
 void getFeaturesForCPU(StringRef CPU, SmallVectorImpl &Features);
 
-/// Fill \p Features with the features that are implied to be enabled/disabled
+/// Set or clear entries in \p Features that are implied to be enabled/disabled
 /// by the provided \p Feature.
-void getImpliedFeatures(StringRef Feature, bool Enabled,
-SmallVectorImpl &Features);
+void updateImpliedFeatures(StringRef Feature, bool Enabled,
+   StringMap &Features);
 
 } // namespace X86
 } // namespace llvm

diff  --git a/llvm/lib/Support/X86TargetParser.cpp 
b/llvm/lib/Support/X86TargetParser.cpp
index c629f872df12..680ec91dc8ef 100644
--- a/llvm/lib/Support/X86TargetParser.cpp
+++ b/llvm/lib/Support/X86TargetParser.cpp
@@ -536,14 +536,6 @@ static constexpr FeatureInfo 
FeatureInfos[X86::CPU_FEATURE_MAX] = {
 #include "llvm/Support/X86TargetParser.def"
 };
 
-// Convert the set bits in FeatureBitset to a list of strings.
-static void getFeatureBitsAsStrings(const FeatureBitset &Bits,
-SmallVectorImpl &Features) {
-  for (unsigned i = 0; i != CPU_FEATURE_MAX; ++i)
-if (Bits[i] && !FeatureInfos[i].Name.empty())
-  Features.push_back(FeatureInfos[i].Name);
-}
-
 void llvm::X86::getFeaturesForCPU(StringRef CPU,
   SmallVectorImpl &EnabledFeatures) 
{
   auto I = llvm::find_if(Processors,
@@ -557,7 +549,9 @@ void llvm::X86::getFeaturesForCPU(StringRef CPU,
   Bits &= ~Feature64BIT;
 
   // Add the string version of all set bits.
-  getFeatureBitsAsStrings(Bits, EnabledFeatures);
+  for (unsigned i = 0; i != CPU_FEATURE_MAX; ++i)
+if (Bits[i] && !FeatureInfos[i].Name.empty())
+  EnabledFeatures.push_back(FeatureInfos[i].Name);
 }
 
 // For each feature that is (transitively) implied by this feature, set it.
@@ -591,9 +585,9 @@ static void getImpliedDisabledFeatures(FeatureBitset &Bits, 
unsigned Value) {
   } while (Prev != Bits);
 }
 
-void llvm::X86::getImpliedFeatures(
+void llvm::X86::updateImpliedFeatures(
 StringRef Feature, bool Enabled,
-SmallVectorImpl &ImpliedFeatures) {
+StringMap &Features) {
   auto I = llvm::find_if(
   FeatureInfos, [&](const FeatureInfo &FI) { return FI.Name == Feature; });
   if (I == std::end(FeatureInfos)) {
@@ -609,6 +603,8 @@ void llvm::X86::getImpliedFeatures(
 getImpliedDisabledFeatures(ImpliedBits,
std::distance(std::begin(FeatureInfos), I));
 
-  // Convert all the found bits into strings.
-  getFeatureBitsAsStrings(ImpliedBits, ImpliedFeatures);
+  // Update the map entry for all implied features.
+  for (unsigned i = 0; i != CPU_FEATURE_MAX; ++i)
+if (ImpliedBits[i] && !FeatureInfos[i].Name.empty())
+  Features[FeatureInfos[i].Name] = Enabled;
 }



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

[PATCH] D85398: [clang-tidy] Fix bugprone-bad-signal-to-kill-thread crash when `SIGTERM` is not a literal.

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX updated this revision to Diff 283509.
ArcsinX added a comment.

Fix test according to review comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85398

Files:
  clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp
@@ -0,0 +1,10 @@
+// RUN: clang-tidy %s --checks=-*,bugprone-bad-signal-to-kill-thread -- | 
count 0
+
+#define SIGTERM ((unsigned)15) // no-crash
+using pthread_t = int;
+int pthread_kill(pthread_t thread, int sig);
+
+int func() {
+  pthread_t thread;
+  return pthread_kill(thread, 0);
+}
Index: clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
@@ -38,6 +38,8 @@
   return llvm::None;
 const MacroInfo *MI = PP->getMacroInfo(It->first);
 const Token &T = MI->tokens().back();
+if (!T.isLiteral())
+  return llvm::None;
 StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
 
 llvm::APInt IntValue;


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp
@@ -0,0 +1,10 @@
+// RUN: clang-tidy %s --checks=-*,bugprone-bad-signal-to-kill-thread -- | count 0
+
+#define SIGTERM ((unsigned)15) // no-crash
+using pthread_t = int;
+int pthread_kill(pthread_t thread, int sig);
+
+int func() {
+  pthread_t thread;
+  return pthread_kill(thread, 0);
+}
Index: clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
@@ -38,6 +38,8 @@
   return llvm::None;
 const MacroInfo *MI = PP->getMacroInfo(It->first);
 const Token &T = MI->tokens().back();
+if (!T.isLiteral())
+  return llvm::None;
 StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
 
 llvm::APInt IntValue;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85398: [clang-tidy] Fix bugprone-bad-signal-to-kill-thread crash when `SIGTERM` is not a literal.

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX added a comment.

Thank you for review!




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp:1
+// RUN: clang-tidy %s --checks="-*,bugprone-bad-signal-to-kill-thread"
+

hokein wrote:
> the same here.
Fixed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85398

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

[PATCH] D85401: [clang-tidy] Fix bugprone-bad-signal-to-kill-thread crash when `SIGTERM` was undefined after definition.

[Aleksandr Platonov 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 rG86711668330c: [clang-tidy] Fix 
bugprone-bad-signal-to-kill-thread crash when `SIGTERM` was… (authored by 
ArcsinX).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85401

Files:
  clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp
@@ -0,0 +1,11 @@
+// RUN: clang-tidy %s --checks=-*,bugprone-bad-signal-to-kill-thread -- | 
count 0
+
+#define SIGTERM 15
+#undef SIGTERM // no-crash
+using pthread_t = int;
+int pthread_kill(pthread_t thread, int sig);
+
+int func() {
+  pthread_t thread;
+  return pthread_kill(thread, 0);
+}
Index: clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
@@ -30,7 +30,8 @@
 
 void BadSignalToKillThreadCheck::check(const MatchFinder::MatchResult &Result) 
{
   const auto IsSigterm = [](const auto &KeyValue) -> bool {
-return KeyValue.first->getName() == "SIGTERM";
+return KeyValue.first->getName() == "SIGTERM" &&
+   KeyValue.first->hasMacroDefinition();
   };
   const auto TryExpandAsInteger =
   [](Preprocessor::macro_iterator It) -> Optional {


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp
@@ -0,0 +1,11 @@
+// RUN: clang-tidy %s --checks=-*,bugprone-bad-signal-to-kill-thread -- | count 0
+
+#define SIGTERM 15
+#undef SIGTERM // no-crash
+using pthread_t = int;
+int pthread_kill(pthread_t thread, int sig);
+
+int func() {
+  pthread_t thread;
+  return pthread_kill(thread, 0);
+}
Index: clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
@@ -30,7 +30,8 @@
 
 void BadSignalToKillThreadCheck::check(const MatchFinder::MatchResult &Result) {
   const auto IsSigterm = [](const auto &KeyValue) -> bool {
-return KeyValue.first->getName() == "SIGTERM";
+return KeyValue.first->getName() == "SIGTERM" &&
+   KeyValue.first->hasMacroDefinition();
   };
   const auto TryExpandAsInteger =
   [](Preprocessor::macro_iterator It) -> Optional {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 8671166 - [clang-tidy] Fix bugprone-bad-signal-to-kill-thread crash when `SIGTERM` was undefined after definition.

[Aleksandr Platonov via cfe-commits]

Author: Aleksandr Platonov
Date: 2020-08-06T10:28:20+03:00
New Revision: 86711668330cf48325a5e960f30d33f6a7364db2

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

LOG: [clang-tidy] Fix bugprone-bad-signal-to-kill-thread crash when `SIGTERM` 
was undefined after definition.

`PP->getMacroInfo()` returns nullptr for undefined macro, which leads to 
null-dereference at `MI->tockens().back()`.
Stack dump:
```
 #0 0x0217d15a llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
(/llvm-project/build/bin/clang-tidy+0x217d15a)
 #1 0x0217b17c llvm::sys::RunSignalHandlers() 
(/llvm-project/build/bin/clang-tidy+0x217b17c)
 #2 0x0217b2e3 SignalHandler(int) 
(/llvm-project/build/bin/clang-tidy+0x217b2e3)
 #3 0x7f39be5b1390 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x11390)
 #4 0x00593532 
clang::tidy::bugprone::BadSignalToKillThreadCheck::check(clang::ast_matchers::MatchFinder::MatchResult
 const&) (/llvm-project/build/bin/clang-tidy+0x593532)
```

Reviewed By: hokein

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

Added: 

clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp

Modified: 
clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
index 3591a7da8eb7..2c7b7b9faa9b 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
@@ -30,7 +30,8 @@ static Preprocessor *PP;
 
 void BadSignalToKillThreadCheck::check(const MatchFinder::MatchResult &Result) 
{
   const auto IsSigterm = [](const auto &KeyValue) -> bool {
-return KeyValue.first->getName() == "SIGTERM";
+return KeyValue.first->getName() == "SIGTERM" &&
+   KeyValue.first->hasMacroDefinition();
   };
   const auto TryExpandAsInteger =
   [](Preprocessor::macro_iterator It) -> Optional {

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp
new file mode 100644
index ..7c2ae16a08e8
--- /dev/null
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp
@@ -0,0 +1,11 @@
+// RUN: clang-tidy %s --checks=-*,bugprone-bad-signal-to-kill-thread -- | 
count 0
+
+#define SIGTERM 15
+#undef SIGTERM // no-crash
+using pthread_t = int;
+int pthread_kill(pthread_t thread, int sig);
+
+int func() {
+  pthread_t thread;
+  return pthread_kill(thread, 0);
+}



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

[PATCH] D85316: [SyntaxTree] Proposition of new tree dump

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 283513.
eduucaldas added a comment.

Reflect comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85316

Files:
  clang/unittests/Tooling/Syntax/TreeTest.cpp


Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -3348,34 +3348,34 @@
   HALF_IF HALF_IF_2 else {}
 })cpp",
   R"txt(
-*: TranslationUnit
+TranslationUnit Detached
 `-SimpleDeclaration
-  |-void
-  |-SimpleDeclarator
-  | |-test
+  |-'void'
+  |-SimpleDeclarator SimpleDeclaration_declarator
+  | |-'test'
   | `-ParametersAndQualifiers
-  |   |-(
-  |   `-)
+  |   |-'(' OpenParen
+  |   `-')' CloseParen
   `-CompoundStatement
-|-{
-|-IfStatement
-| |-I: if
-| |-I: (
-| |-I: BinaryOperatorExpression
-| | |-I: IntegerLiteralExpression
-| | | `-I: 1
-| | |-I: +
-| | `-I: IntegerLiteralExpression
-| |   `-I: 1
-| |-I: )
-| |-I: CompoundStatement
-| | |-I: {
-| | `-I: }
-| |-else
-| `-CompoundStatement
-|   |-{
-|   `-}
-`-}
+|-'{' OpenParen
+|-IfStatement CompoundStatement_statement
+| |-'if' IntroducerKeyword I
+| |-'(' I
+| |-BinaryOperatorExpression I
+| | |-IntegerLiteralExpression BinaryOperatorExpression_leftHandSide I
+| | | `-'1' LiteralToken I
+| | |-'+' OperatorExpression_operatorToken I
+| | `-IntegerLiteralExpression BinaryOperatorExpression_rightHandSide I
+| |   `-'1' LiteralToken I
+| |-')' I
+| |-CompoundStatement IfStatement_thenStatement I
+| | |-'{' OpenParen I
+| | `-'}' CloseParen I
+| |-'else' IfStatement_elseKeyword
+| `-CompoundStatement IfStatement_elseStatement
+|   |-'{' OpenParen
+|   `-'}' CloseParen
+`-'}' CloseParen
 )txt"));
 }
 


Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -3348,34 +3348,34 @@
   HALF_IF HALF_IF_2 else {}
 })cpp",
   R"txt(
-*: TranslationUnit
+TranslationUnit Detached
 `-SimpleDeclaration
-  |-void
-  |-SimpleDeclarator
-  | |-test
+  |-'void'
+  |-SimpleDeclarator SimpleDeclaration_declarator
+  | |-'test'
   | `-ParametersAndQualifiers
-  |   |-(
-  |   `-)
+  |   |-'(' OpenParen
+  |   `-')' CloseParen
   `-CompoundStatement
-|-{
-|-IfStatement
-| |-I: if
-| |-I: (
-| |-I: BinaryOperatorExpression
-| | |-I: IntegerLiteralExpression
-| | | `-I: 1
-| | |-I: +
-| | `-I: IntegerLiteralExpression
-| |   `-I: 1
-| |-I: )
-| |-I: CompoundStatement
-| | |-I: {
-| | `-I: }
-| |-else
-| `-CompoundStatement
-|   |-{
-|   `-}
-`-}
+|-'{' OpenParen
+|-IfStatement CompoundStatement_statement
+| |-'if' IntroducerKeyword I
+| |-'(' I
+| |-BinaryOperatorExpression I
+| | |-IntegerLiteralExpression BinaryOperatorExpression_leftHandSide I
+| | | `-'1' LiteralToken I
+| | |-'+' OperatorExpression_operatorToken I
+| | `-IntegerLiteralExpression BinaryOperatorExpression_rightHandSide I
+| |   `-'1' LiteralToken I
+| |-')' I
+| |-CompoundStatement IfStatement_thenStatement I
+| | |-'{' OpenParen I
+| | `-'}' CloseParen I
+| |-'else' IfStatement_elseKeyword
+| `-CompoundStatement IfStatement_elseStatement
+|   |-'{' OpenParen
+|   `-'}' CloseParen
+`-'}' CloseParen
 )txt"));
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85316: [SyntaxTree] Proposition of new tree dump

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas added inline comments.



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:3363
+| |-'if' IntroducerKeyword I
+| |-'(' I
+| |-BinaryOperatorExpression I

Some points to make a decision. 
What should be the order of these extra information?
I like the order now, Most frequent in the left.

More decriptive markers?
We could choose to have more descriptive markers
`I` -> `unmodifieable`
`M`-> `not backed by source code` / `synthesized`

Ambiguity.
Since we choose to not dump a role for `NodeRole::Unknown` we might have some 
ambiguity between NodeRoles and markers. We can avoid that by surrounding 
markers, or making them lower-case. or even by just leaving one character 
markers




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85316

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

[PATCH] D85398: [clang-tidy] Fix bugprone-bad-signal-to-kill-thread crash when `SIGTERM` is not a literal.

[Aleksandr Platonov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG216ad2da74f0: [clang-tidy] Fix 
bugprone-bad-signal-to-kill-thread crash when `SIGTERM` is not… (authored by 
ArcsinX).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85398

Files:
  clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp
@@ -0,0 +1,10 @@
+// RUN: clang-tidy %s --checks=-*,bugprone-bad-signal-to-kill-thread -- | 
count 0
+
+#define SIGTERM ((unsigned)15) // no-crash
+using pthread_t = int;
+int pthread_kill(pthread_t thread, int sig);
+
+int func() {
+  pthread_t thread;
+  return pthread_kill(thread, 0);
+}
Index: clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
@@ -39,6 +39,8 @@
   return llvm::None;
 const MacroInfo *MI = PP->getMacroInfo(It->first);
 const Token &T = MI->tokens().back();
+if (!T.isLiteral())
+  return llvm::None;
 StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
 
 llvm::APInt IntValue;


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp
@@ -0,0 +1,10 @@
+// RUN: clang-tidy %s --checks=-*,bugprone-bad-signal-to-kill-thread -- | count 0
+
+#define SIGTERM ((unsigned)15) // no-crash
+using pthread_t = int;
+int pthread_kill(pthread_t thread, int sig);
+
+int func() {
+  pthread_t thread;
+  return pthread_kill(thread, 0);
+}
Index: clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
@@ -39,6 +39,8 @@
   return llvm::None;
 const MacroInfo *MI = PP->getMacroInfo(It->first);
 const Token &T = MI->tokens().back();
+if (!T.isLiteral())
+  return llvm::None;
 StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
 
 llvm::APInt IntValue;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 216ad2d - [clang-tidy] Fix bugprone-bad-signal-to-kill-thread crash when `SIGTERM` is not a literal.

[Aleksandr Platonov via cfe-commits]

Author: Aleksandr Platonov
Date: 2020-08-06T10:32:03+03:00
New Revision: 216ad2da74f0e952af8f1c8ac84e19146d739ff2

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

LOG: [clang-tidy] Fix bugprone-bad-signal-to-kill-thread crash when `SIGTERM` 
is not a literal.

If `SIGTERM` is not a literal (e.g. `#define SIGTERM ((unsigned)15)`) 
bugprone-bad-signal-to-kill-thread check crashes.
Stack dump:
```
 #0 0x0217d15a llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
(/llvm-project/build/bin/clang-tidy+0x217d15a)
 #1 0x0217b17c llvm::sys::RunSignalHandlers() 
(/llvm-project/build/bin/clang-tidy+0x217b17c)
 #2 0x0217b2e3 SignalHandler(int) 
(/llvm-project/build/bin/clang-tidy+0x217b2e3)
 #3 0x7f6a7efb1390 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x11390)
 #4 0x0212ac9b llvm::StringRef::getAsInteger(unsigned int, 
llvm::APInt&) const (/llvm-project/build/bin/clang-tidy+0x212ac9b)
 #5 0x00593501 
clang::tidy::bugprone::BadSignalToKillThreadCheck::check(clang::ast_matchers::MatchFinder::MatchResult
 const&) (/llvm-project/build/bin/clang-tidy+0x593501)
```

Reviewed By: hokein

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

Added: 

clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp

Modified: 
clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
index 2c7b7b9faa9b..0720c7e689a1 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
@@ -39,6 +39,8 @@ void BadSignalToKillThreadCheck::check(const 
MatchFinder::MatchResult &Result) {
   return llvm::None;
 const MacroInfo *MI = PP->getMacroInfo(It->first);
 const Token &T = MI->tokens().back();
+if (!T.isLiteral())
+  return llvm::None;
 StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
 
 llvm::APInt IntValue;

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp
new file mode 100644
index ..f96f270dbf29
--- /dev/null
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp
@@ -0,0 +1,10 @@
+// RUN: clang-tidy %s --checks=-*,bugprone-bad-signal-to-kill-thread -- | 
count 0
+
+#define SIGTERM ((unsigned)15) // no-crash
+using pthread_t = int;
+int pthread_kill(pthread_t thread, int sig);
+
+int func() {
+  pthread_t thread;
+  return pthread_kill(thread, 0);
+}



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

[PATCH] D85115: [update_cc_test_checks.py] Add test for D84511

[Alexander Richardson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe150d2cab868: [update_cc_test_checks.py] Add test for D84511 
(authored by arichardson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85115

Files:
  clang/test/utils/update_cc_test_checks/basic-cplusplus.test
  clang/test/utils/update_cc_test_checks/lit.local.cfg


Index: clang/test/utils/update_cc_test_checks/lit.local.cfg
===
--- clang/test/utils/update_cc_test_checks/lit.local.cfg
+++ clang/test/utils/update_cc_test_checks/lit.local.cfg
@@ -23,3 +23,5 @@
 ('%update_cc_test_checks', "%s %s %s" % (
 shell_quote(config.python_executable), shell_quote(script_path),
 extra_args)))
+config.substitutions.append(
+('%clang_tools_dir', shell_quote(config.clang_tools_dir)))
Index: clang/test/utils/update_cc_test_checks/basic-cplusplus.test
===
--- clang/test/utils/update_cc_test_checks/basic-cplusplus.test
+++ clang/test/utils/update_cc_test_checks/basic-cplusplus.test
@@ -2,6 +2,9 @@
 
 # RUN: cp %S/Inputs/basic-cplusplus.cpp %t.cpp && %update_cc_test_checks %t.cpp
 # RUN: diff -u %S/Inputs/basic-cplusplus.cpp.expected %t.cpp
+## Check that it also works with the --llvm-bin flag instead of --clang
+# RUN: cp %S/Inputs/basic-cplusplus.cpp %t.cpp && %update_cc_test_checks 
--clang='' --llvm-bin=%clang_tools_dir %t.cpp
+# RUN: diff -u %S/Inputs/basic-cplusplus.cpp.expected %t.cpp
 ## Check that re-running update_cc_test_checks doesn't change the output
 # RUN: %update_cc_test_checks %t.cpp
 # RUN: diff -u %S/Inputs/basic-cplusplus.cpp.expected %t.cpp


Index: clang/test/utils/update_cc_test_checks/lit.local.cfg
===
--- clang/test/utils/update_cc_test_checks/lit.local.cfg
+++ clang/test/utils/update_cc_test_checks/lit.local.cfg
@@ -23,3 +23,5 @@
 ('%update_cc_test_checks', "%s %s %s" % (
 shell_quote(config.python_executable), shell_quote(script_path),
 extra_args)))
+config.substitutions.append(
+('%clang_tools_dir', shell_quote(config.clang_tools_dir)))
Index: clang/test/utils/update_cc_test_checks/basic-cplusplus.test
===
--- clang/test/utils/update_cc_test_checks/basic-cplusplus.test
+++ clang/test/utils/update_cc_test_checks/basic-cplusplus.test
@@ -2,6 +2,9 @@
 
 # RUN: cp %S/Inputs/basic-cplusplus.cpp %t.cpp && %update_cc_test_checks %t.cpp
 # RUN: diff -u %S/Inputs/basic-cplusplus.cpp.expected %t.cpp
+## Check that it also works with the --llvm-bin flag instead of --clang
+# RUN: cp %S/Inputs/basic-cplusplus.cpp %t.cpp && %update_cc_test_checks --clang='' --llvm-bin=%clang_tools_dir %t.cpp
+# RUN: diff -u %S/Inputs/basic-cplusplus.cpp.expected %t.cpp
 ## Check that re-running update_cc_test_checks doesn't change the output
 # RUN: %update_cc_test_checks %t.cpp
 # RUN: diff -u %S/Inputs/basic-cplusplus.cpp.expected %t.cpp
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] e150d2c - [update_cc_test_checks.py] Add test for D84511

[Alex Richardson via cfe-commits]

Author: Alex Richardson
Date: 2020-08-06T09:15:56+01:00
New Revision: e150d2cab8688a2b3b7d395bfbbc152b08df9033

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

LOG: [update_cc_test_checks.py] Add test for D84511

Reviewed By: MaskRay

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

Added: 


Modified: 
clang/test/utils/update_cc_test_checks/basic-cplusplus.test
clang/test/utils/update_cc_test_checks/lit.local.cfg

Removed: 




diff  --git a/clang/test/utils/update_cc_test_checks/basic-cplusplus.test 
b/clang/test/utils/update_cc_test_checks/basic-cplusplus.test
index 10be017388c6..3557e18f5ac9 100644
--- a/clang/test/utils/update_cc_test_checks/basic-cplusplus.test
+++ b/clang/test/utils/update_cc_test_checks/basic-cplusplus.test
@@ -2,6 +2,9 @@
 
 # RUN: cp %S/Inputs/basic-cplusplus.cpp %t.cpp && %update_cc_test_checks %t.cpp
 # RUN: 
diff  -u %S/Inputs/basic-cplusplus.cpp.expected %t.cpp
+## Check that it also works with the --llvm-bin flag instead of --clang
+# RUN: cp %S/Inputs/basic-cplusplus.cpp %t.cpp && %update_cc_test_checks 
--clang='' --llvm-bin=%clang_tools_dir %t.cpp
+# RUN: 
diff  -u %S/Inputs/basic-cplusplus.cpp.expected %t.cpp
 ## Check that re-running update_cc_test_checks doesn't change the output
 # RUN: %update_cc_test_checks %t.cpp
 # RUN: 
diff  -u %S/Inputs/basic-cplusplus.cpp.expected %t.cpp

diff  --git a/clang/test/utils/update_cc_test_checks/lit.local.cfg 
b/clang/test/utils/update_cc_test_checks/lit.local.cfg
index 0250446423cb..3fef02881853 100644
--- a/clang/test/utils/update_cc_test_checks/lit.local.cfg
+++ b/clang/test/utils/update_cc_test_checks/lit.local.cfg
@@ -23,3 +23,5 @@ config.substitutions.append(
 ('%update_cc_test_checks', "%s %s %s" % (
 shell_quote(config.python_executable), shell_quote(script_path),
 extra_args)))
+config.substitutions.append(
+('%clang_tools_dir', shell_quote(config.clang_tools_dir)))



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

[clang] 7109494 - [clang][Driver] Don't hardcode --as-needed/--no-as-needed on Illumos

[Rainer Orth via cfe-commits]

Author: Rainer Orth
Date: 2020-08-06T10:47:16+02:00
New Revision: 710949482edb96a329104317164909979f2387aa

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

LOG: [clang][Driver] Don't hardcode --as-needed/--no-as-needed on Illumos

`ninja check-all` currently fails on Illumos:

  [84/716] Generating default/Asan-i386-inline-Test
  FAILED: projects/compiler-rt/lib/asan/tests/default/Asan-i386-inline-Test
  cd /var/llvm/dist-amd64-release/projects/compiler-rt/lib/asan/tests && 
/var/llvm/dist-amd64-release/./bin/clang 
ASAN_INST_TEST_OBJECTS.gtest-all.cc.i386-inline.o 
ASAN_INST_TEST_OBJECTS.asan_globals_test.cpp.i386-inline.o 
ASAN_INST_TEST_OBJECTS.asan_interface_test.cpp.i386-inline.o 
ASAN_INST_TEST_OBJECTS.asan_internal_interface_test.cpp.i386-inline.o 
ASAN_INST_TEST_OBJECTS.asan_test.cpp.i386-inline.o 
ASAN_INST_TEST_OBJECTS.asan_oob_test.cpp.i386-inline.o 
ASAN_INST_TEST_OBJECTS.asan_mem_test.cpp.i386-inline.o 
ASAN_INST_TEST_OBJECTS.asan_str_test.cpp.i386-inline.o 
ASAN_INST_TEST_OBJECTS.asan_test_main.cpp.i386-inline.o -o 
/var/llvm/dist-amd64-release/projects/compiler-rt/lib/asan/tests/default/./Asan-i386-inline-Test
 -g --driver-mode=g++ -fsanitize=address -m32
  ld: fatal: unrecognized option '--no-as-needed'
  ld: fatal: use the -z help option for usage information
  clang-11: error: linker command failed with exit code 1 (use -v to see 
invocation)

`clang` unconditionally passes `--as-needed`/`--no-as-needed` to the
linker.  This works on Solaris 11.[34] which added a couple of option
aliases to the native linker to improve compatibility with GNU `ld`.
Illumos `ld` didn't do this, so one needs to use the corresponding
native options `-z ignore`/`-z record` instead.

Because this works on both Solaris and Illumos, the current patch always
passes the native options on Solaris.  This isn't fully correct, however:
when using GNU `ld` on Solaris (not yet supported; I'm working on that),
one still needs `--as-needed` instead.

I'm hardcoding this decision because a generic detection via a `cmake` test
is hard: many systems have their own implementation of `getDefaultLinker`
and `cmake` would have to duplicate the information encoded there.
Besides, it would still break when `-fuse-ld` is used.

Tested on `amd64-pc-solaris2.11` (Solaris 11.4 and OpenIndiana 2020.04),
`sparcv9-sun-solaris2.11`, and `x86_64-pc-linux-gnu`.

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/CommonArgs.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp 
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index acde6d9e2111..bb88dd405c67 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -630,6 +630,16 @@ static bool addSanitizerDynamicList(const ToolChain &TC, 
const ArgList &Args,
   return false;
 }
 
+static const char *getAsNeededOption(const ToolChain &TC, bool as_needed) {
+  // While the Solaris 11.2 ld added --as-needed/--no-as-needed as aliases
+  // for the native forms -z ignore/-z record, they are missing in Illumos,
+  // so always use the native form.
+  if (TC.getTriple().isOSSolaris())
+return as_needed ? "-zignore" : "-zrecord";
+  else
+return as_needed ? "--as-needed" : "--no-as-needed";
+}
+
 void tools::linkSanitizerRuntimeDeps(const ToolChain &TC,
  ArgStringList &CmdArgs) {
   // Fuchsia never needs these.  Any sanitizer runtimes with system
@@ -639,7 +649,7 @@ void tools::linkSanitizerRuntimeDeps(const ToolChain &TC,
 
   // Force linking against the system libraries sanitizers depends on
   // (see PR15823 why this is necessary).
-  CmdArgs.push_back("--no-as-needed");
+  CmdArgs.push_back(getAsNeededOption(TC, false));
   // There's no libpthread or librt on RTEMS & Android.
   if (TC.getTriple().getOS() != llvm::Triple::RTEMS &&
   !TC.getTriple().isAndroid()) {
@@ -836,7 +846,7 @@ bool tools::addXRayRuntime(const ToolChain&TC, const 
ArgList &Args, ArgStringLis
 }
 
 void tools::linkXRayRuntimeDeps(const ToolChain &TC, ArgStringList &CmdArgs) {
-  CmdArgs.push_back("--no-as-needed");
+  CmdArgs.push_back(getAsNeededOption(TC, false));
   CmdArgs.push_back("-lpthread");
   if (!TC.getTriple().isOSOpenBSD())
 CmdArgs.push_back("-lrt");
@@ -1261,7 +1271,7 @@ static void AddUnwindLibrary(const ToolChain &TC, const 
Driver &D,
   bool AsNeeded = LGT == LibGccType::UnspecifiedLibGcc &&
   !TC.getTriple().isAndroid() && !TC.getTriple().isOSCygMing();
   if (AsNeeded)
-CmdArgs.push_back("--as-needed");
+CmdArgs.push_back(getAsNeededOption(TC, true));
 
   switch (UNW) {
   case ToolChain::UNW_None:
@@ -1289,7 +1299,7 @@ static void AddUnwindLibrary(const ToolChain &TC, c

[PATCH] D84412: [clang][Driver] Don't hardcode --as-needed/--no-as-needed on Illumos

[Rainer Orth via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
ro marked 3 inline comments as done.
Closed by commit rG710949482edb: [clang][Driver] Don't hardcode 
--as-needed/--no-as-needed on Illumos (authored by ro).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84412

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -630,6 +630,16 @@
   return false;
 }
 
+static const char *getAsNeededOption(const ToolChain &TC, bool as_needed) {
+  // While the Solaris 11.2 ld added --as-needed/--no-as-needed as aliases
+  // for the native forms -z ignore/-z record, they are missing in Illumos,
+  // so always use the native form.
+  if (TC.getTriple().isOSSolaris())
+return as_needed ? "-zignore" : "-zrecord";
+  else
+return as_needed ? "--as-needed" : "--no-as-needed";
+}
+
 void tools::linkSanitizerRuntimeDeps(const ToolChain &TC,
  ArgStringList &CmdArgs) {
   // Fuchsia never needs these.  Any sanitizer runtimes with system
@@ -639,7 +649,7 @@
 
   // Force linking against the system libraries sanitizers depends on
   // (see PR15823 why this is necessary).
-  CmdArgs.push_back("--no-as-needed");
+  CmdArgs.push_back(getAsNeededOption(TC, false));
   // There's no libpthread or librt on RTEMS & Android.
   if (TC.getTriple().getOS() != llvm::Triple::RTEMS &&
   !TC.getTriple().isAndroid()) {
@@ -836,7 +846,7 @@
 }
 
 void tools::linkXRayRuntimeDeps(const ToolChain &TC, ArgStringList &CmdArgs) {
-  CmdArgs.push_back("--no-as-needed");
+  CmdArgs.push_back(getAsNeededOption(TC, false));
   CmdArgs.push_back("-lpthread");
   if (!TC.getTriple().isOSOpenBSD())
 CmdArgs.push_back("-lrt");
@@ -1261,7 +1271,7 @@
   bool AsNeeded = LGT == LibGccType::UnspecifiedLibGcc &&
   !TC.getTriple().isAndroid() && !TC.getTriple().isOSCygMing();
   if (AsNeeded)
-CmdArgs.push_back("--as-needed");
+CmdArgs.push_back(getAsNeededOption(TC, true));
 
   switch (UNW) {
   case ToolChain::UNW_None:
@@ -1289,7 +1299,7 @@
   }
 
   if (AsNeeded)
-CmdArgs.push_back("--no-as-needed");
+CmdArgs.push_back(getAsNeededOption(TC, false));
 }
 
 static void AddLibgcc(const ToolChain &TC, const Driver &D,


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -630,6 +630,16 @@
   return false;
 }
 
+static const char *getAsNeededOption(const ToolChain &TC, bool as_needed) {
+  // While the Solaris 11.2 ld added --as-needed/--no-as-needed as aliases
+  // for the native forms -z ignore/-z record, they are missing in Illumos,
+  // so always use the native form.
+  if (TC.getTriple().isOSSolaris())
+return as_needed ? "-zignore" : "-zrecord";
+  else
+return as_needed ? "--as-needed" : "--no-as-needed";
+}
+
 void tools::linkSanitizerRuntimeDeps(const ToolChain &TC,
  ArgStringList &CmdArgs) {
   // Fuchsia never needs these.  Any sanitizer runtimes with system
@@ -639,7 +649,7 @@
 
   // Force linking against the system libraries sanitizers depends on
   // (see PR15823 why this is necessary).
-  CmdArgs.push_back("--no-as-needed");
+  CmdArgs.push_back(getAsNeededOption(TC, false));
   // There's no libpthread or librt on RTEMS & Android.
   if (TC.getTriple().getOS() != llvm::Triple::RTEMS &&
   !TC.getTriple().isAndroid()) {
@@ -836,7 +846,7 @@
 }
 
 void tools::linkXRayRuntimeDeps(const ToolChain &TC, ArgStringList &CmdArgs) {
-  CmdArgs.push_back("--no-as-needed");
+  CmdArgs.push_back(getAsNeededOption(TC, false));
   CmdArgs.push_back("-lpthread");
   if (!TC.getTriple().isOSOpenBSD())
 CmdArgs.push_back("-lrt");
@@ -1261,7 +1271,7 @@
   bool AsNeeded = LGT == LibGccType::UnspecifiedLibGcc &&
   !TC.getTriple().isAndroid() && !TC.getTriple().isOSCygMing();
   if (AsNeeded)
-CmdArgs.push_back("--as-needed");
+CmdArgs.push_back(getAsNeededOption(TC, true));
 
   switch (UNW) {
   case ToolChain::UNW_None:
@@ -1289,7 +1299,7 @@
   }
 
   if (AsNeeded)
-CmdArgs.push_back("--no-as-needed");
+CmdArgs.push_back(getAsNeededOption(TC, false));
 }
 
 static void AddLibgcc(const ToolChain &TC, const Driver &D,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: clang/lib/Analysis/TextPathDiagnosticConsumer.cpp:1
 //===--- TextDiagnostics.cpp - Text Diagnostics for Paths ---*- C++ 
-*-===//
 //

TextPathDiagnosticConsumer.cpp?



Comment at: clang/lib/Analysis/TextPathDiagnosticConsumer.cpp:36
 /// diagnostics in textual format for the 'text' output type.
 class TextDiagnostics : public PathDiagnosticConsumer {
   PathDiagnosticConsumerOptions DiagOpts;

`TextPathDiagnosticConsumer`?


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

https://reviews.llvm.org/D67422

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

[PATCH] D85309: [WIP][clang][Driver] Support GNU ld on Solaris

[Rainer Orth via Phabricator via cfe-commits]

ro added a comment.

In D85309#2198505 , @MaskRay wrote:

> Please find a suitable test file (clang/test/Driver/solaris-*.c) and add some 
> tests there (for example, solaris-ld.c)
> The challenge is that CLANG_ENABLE_GLD is not a default configuration, so 
> leaving out tests may be fine.

On top of that, GNU `ld` may or may not be installed on a particular Solaris 
systems.  So even if the configuration were dynamic instead of hardcoding GNU 
`ld`, one would still have to make the test conditional on its presence on the 
actual system.  Same as the somewhat hacky test in D84029 
.

That said, I don't consider this patch even remotely ready for inclusion. It 
was primarily meant to show what I'd been using to test that configuration.  It 
has been useful in pinpointing differences between Solaris `ld` and GNU `ld` 
testresults.  However, there are quite a number of unexpected asan failures 
that need to be investigated first.

I'm also not happy with selecting the linker flavour at build time: maybe this 
can be made dynamic (e.g. parsing `ld -V` output) to allow for switching 
between Solaris `ld` and GNU `ld` at runtime.




Comment at: clang/CMakeLists.txt:281
 
+option(CLANG_ENABLE_GLD "Default to GNU ld." OFF)
+

MaskRay wrote:
> Mention SOLARIS in the variable name? Users on other OSes don't need to read 
> the description of this variable.
> 
> It will also make the abbreviation `GLD` more legitimate. GLD isn't a 
> well-recognized abbreviation for GNU ld.
As I said, I've only done it this way to allow selecting GNU `ld` at all.  This 
would become moot if linker flavour detection were
implemented at runtime.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:252
+  // FIXME: Hack around /usr/gnu/bin/ld being configure with --with-sysroot.
+  return "/vol/gcc/bin/gld-2.35";
+  //return "/usr/gnu/bin/ld";

MaskRay wrote:
> The hard-coded version doesn't sound great.
Certainly not: this is a site-local path used to hack around a configuration 
error in the current bundled GNU `ld` on Solaris.
That one is being fixed as we speak.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85309

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

[PATCH] D85295: [SyntaxTree] Implement the List construct.

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/Tree.h:197
+  MaybeTerminated,
+  Separated,
+};

gribozavr2 wrote:
> Add a "WithoutDelimiters" case as well?
I think we might want to treat non-delimited-list in another way. as many of 
the member functions stop making sense. 
`getElementsAsNodesAndDelimiters` 
`getDelimiterTokenKind`
`getTerminationKind`



Comment at: clang/include/clang/Tooling/Syntax/Tree.h:229
+  /// elements to empty or one-element lists.
+  clang::tok::TokenKind getDelimiterTokenKind();
+

gribozavr2 wrote:
> Should we change this function to return optional to allow representing lists 
> that don't have any delimiters?
Cf, previous comment on non-delimited-lists



Comment at: clang/lib/Tooling/Syntax/Tree.cpp:277
+  if (!firstChild()) {
+assert(canBeEmpty());
+return {};

gribozavr2 wrote:
> This assert is only correct for valid code. When a syntax tree represents 
> invalid code, any list can be empty.
Yeah, of course, we had already discussed that verifying those things in the 
casa of valid code would be the task of the verifier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85295

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

[PATCH] D85295: [SyntaxTree] Implement the List construct.

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 283536.
eduucaldas marked 9 inline comments as done.
eduucaldas added a comment.

Answer comments

non-delimited-lists need further discussion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85295

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/include/clang/Tooling/Syntax/Tree.h
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/lib/Tooling/Syntax/Tree.cpp

Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -270,3 +270,107 @@
   }
   return nullptr;
 }
+
+std::vector>
+syntax::List::getElementsAsNodesAndDelimiters() {
+  if (!firstChild())
+return {};
+
+  auto children = std::vector>();
+  syntax::Node *elementWithoutDelimiter = nullptr;
+  for (auto *C = firstChild(); C; C = C->nextSibling()) {
+switch (C->role()) {
+case syntax::NodeRole::List_element: {
+  if (elementWithoutDelimiter) {
+children.push_back({elementWithoutDelimiter, nullptr});
+  }
+  elementWithoutDelimiter = C;
+  break;
+}
+case syntax::NodeRole::List_delimiter: {
+  children.push_back({elementWithoutDelimiter, cast(C)});
+  elementWithoutDelimiter = nullptr;
+  break;
+}
+default:
+  llvm_unreachable(
+  "A list can have only elements and delimiters as children.");
+}
+  }
+
+  switch (getTerminationKind()) {
+  case syntax::List::TerminationKind::Separated: {
+children.push_back({elementWithoutDelimiter, nullptr});
+break;
+  }
+  case syntax::List::TerminationKind::Terminated:
+  case syntax::List::TerminationKind::MaybeTerminated: {
+if (elementWithoutDelimiter) {
+  children.push_back({elementWithoutDelimiter, nullptr});
+}
+break;
+  }
+  }
+
+  return children;
+}
+
+// Almost the same implementation of `getElementsAsNodesAndDelimiters` but
+// ignoring delimiters
+std::vector syntax::List::getElementsAsNodes() {
+  if (!firstChild())
+return {};
+
+  auto children = std::vector();
+  syntax::Node *elementWithoutDelimiter = nullptr;
+  for (auto *C = firstChild(); C; C = C->nextSibling()) {
+switch (C->role()) {
+case syntax::NodeRole::List_element: {
+  if (elementWithoutDelimiter) {
+children.push_back(elementWithoutDelimiter);
+  }
+  elementWithoutDelimiter = C;
+  break;
+}
+case syntax::NodeRole::List_delimiter: {
+  children.push_back(elementWithoutDelimiter);
+  elementWithoutDelimiter = nullptr;
+  break;
+}
+default:
+  llvm_unreachable("A list has only elements or delimiters.");
+}
+  }
+
+  switch (getTerminationKind()) {
+  case syntax::List::TerminationKind::Separated: {
+children.push_back(elementWithoutDelimiter);
+break;
+  }
+  case syntax::List::TerminationKind::Terminated:
+  case syntax::List::TerminationKind::MaybeTerminated: {
+if (elementWithoutDelimiter) {
+  children.push_back(elementWithoutDelimiter);
+}
+break;
+  }
+  }
+
+  return children;
+}
+
+// The methods below can't be implemented without information about the derived
+// list. These methods will be implemented by switching on the derived list's
+// `NodeKind`
+
+clang::tok::TokenKind syntax::List::getDelimiterTokenKind() {
+  llvm_unreachable("A list can have only elements and delimiters as children.");
+}
+
+syntax::List::TerminationKind syntax::List::getTerminationKind() {
+  llvm_unreachable("A list can have only elements and delimiters as children.");
+}
+
+bool syntax::List::canBeEmpty() {
+  llvm_unreachable("A list can have only elements and delimiters as children.");
+}
Index: clang/lib/Tooling/Syntax/Nodes.cpp
===
--- clang/lib/Tooling/Syntax/Nodes.cpp
+++ clang/lib/Tooling/Syntax/Nodes.cpp
@@ -144,6 +144,10 @@
 return OS << "ExternKeyword";
   case syntax::NodeRole::BodyStatement:
 return OS << "BodyStatement";
+  case syntax::NodeRole::List_element:
+return OS << "List_element";
+  case syntax::NodeRole::List_delimiter:
+return OS << "List_delimiter";
   case syntax::NodeRole::CaseStatement_value:
 return OS << "CaseStatement_value";
   case syntax::NodeRole::IfStatement_thenStatement:
Index: clang/include/clang/Tooling/Syntax/Tree.h
===
--- clang/include/clang/Tooling/Syntax/Tree.h
+++ clang/include/clang/Tooling/Syntax/Tree.h
@@ -191,6 +191,68 @@
   Node *FirstChild = nullptr;
 };
 
+/// A Tree that represents a syntactic list of elements.
+///
+/// We try to model with this type the artificial grammar-construct:
+/// delimited-list(element, delimiter, termination, canBeEmpty)
+///
+/// For example, from C++ [dcl.decl]:
+/// init-declarator-list:
+///   init-declarator
+///   init-declarator-list , init-declarator
+/// 

[PATCH] D85009: [Sema][BFloat] Forbid arithmetic on vectors of bfloat.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham added a comment.

This discussion seems to have wound down. I'll land this patch tomorrow on the 
strength of @LukeGeeson's review, unless you have strong objections, @jfb?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85009

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

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:14034
+  << FixItHint::CreateInsertion(LHSExpr->getEndLoc(), ") && ")
+  << FixItHint::CreateInsertion(LHSBO->getRHS()->getBeginLoc(), "(y ")
+  << FixItHint::CreateInsertion(RHSExpr->getEndLoc(), ")");

njames93 wrote:
> You don't want to insert `y` but the source code for `y`
Gotcha, I'll address.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85097

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

[PATCH] D85311: [analyzer][tests] Understand when diagnostics change between builds

[Valeriy Savchenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6ddef9247458: [analyzer][tests] Understand when diagnostics 
change between builds (authored by vsavchenko).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85311

Files:
  clang/utils/analyzer/CmpRuns.py

Index: clang/utils/analyzer/CmpRuns.py
===
--- clang/utils/analyzer/CmpRuns.py
+++ clang/utils/analyzer/CmpRuns.py
@@ -35,14 +35,16 @@
 from collections import defaultdict
 from copy import copy
 from enum import Enum
-from typing import (Any, cast, Dict, List, NamedTuple, Optional, Sequence,
-TextIO, TypeVar, Tuple, Union)
+from typing import (Any, DefaultDict, Dict, List, NamedTuple, Optional,
+Sequence, TextIO, TypeVar, Tuple, Union)
 
 
 Number = Union[int, float]
 Stats = Dict[str, Dict[str, Number]]
 Plist = Dict[str, Any]
 JSON = Dict[str, Any]
+# Diff in a form: field -> (before, after)
+JSONDiff = Dict[str, Tuple[str, str]]
 # Type for generics
 T = TypeVar('T')
 
@@ -136,6 +138,9 @@
 def get_description(self) -> str:
 return self._data['description']
 
+def get_location(self) -> str:
+return f"{self.get_file_name()}:{self.get_line()}:{self.get_column()}"
+
 def get_issue_identifier(self) -> str:
 id = self.get_file_name() + "+"
 
@@ -172,11 +177,32 @@
 return f"{file_prefix}{funcname_postfix}:{line}:{col}" \
 f", {self.get_category()}: {self.get_description()}"
 
+KEY_FIELDS = ["check_name", "category", "description"]
+
+def is_similar_to(self, other: "AnalysisDiagnostic") -> bool:
+# We consider two diagnostics similar only if at least one
+# of the key fields is the same in both diagnostics.
+return len(self.get_diffs(other)) != len(self.KEY_FIELDS)
+
+def get_diffs(self, other: "AnalysisDiagnostic") -> JSONDiff:
+return {field: (self._data[field], other._data[field])
+for field in self.KEY_FIELDS
+if self._data[field] != other._data[field]}
+
 # Note, the data format is not an API and may change from one analyzer
 # version to another.
 def get_raw_data(self) -> Plist:
 return self._data
 
+def __eq__(self, other: object) -> bool:
+return hash(self) == hash(other)
+
+def __ne__(self, other: object) -> bool:
+return hash(self) != hash(other)
+
+def __hash__(self) -> int:
+return hash(self.get_issue_identifier())
+
 
 class AnalysisRun:
 def __init__(self, info: SingleRunInfo):
@@ -283,12 +309,39 @@
 return d.get_issue_identifier()
 
 
-PresentInBoth = Tuple[AnalysisDiagnostic, AnalysisDiagnostic]
-PresentOnlyInOld = Tuple[AnalysisDiagnostic, None]
-PresentOnlyInNew = Tuple[None, AnalysisDiagnostic]
-ComparisonResult = List[Union[PresentInBoth,
-  PresentOnlyInOld,
-  PresentOnlyInNew]]
+AnalysisDiagnosticPair = Tuple[AnalysisDiagnostic, AnalysisDiagnostic]
+
+
+class ComparisonResult:
+def __init__(self):
+self.present_in_both: List[AnalysisDiagnostic] = []
+self.present_only_in_old: List[AnalysisDiagnostic] = []
+self.present_only_in_new: List[AnalysisDiagnostic] = []
+self.changed_between_new_and_old: List[AnalysisDiagnosticPair] = []
+
+def add_common(self, issue: AnalysisDiagnostic):
+self.present_in_both.append(issue)
+
+def add_removed(self, issue: AnalysisDiagnostic):
+self.present_only_in_old.append(issue)
+
+def add_added(self, issue: AnalysisDiagnostic):
+self.present_only_in_new.append(issue)
+
+def add_changed(self, old_issue: AnalysisDiagnostic,
+new_issue: AnalysisDiagnostic):
+self.changed_between_new_and_old.append((old_issue, new_issue))
+
+
+GroupedDiagnostics = DefaultDict[str, List[AnalysisDiagnostic]]
+
+
+def get_grouped_diagnostics(diagnostics: List[AnalysisDiagnostic]
+) -> GroupedDiagnostics:
+result: GroupedDiagnostics = defaultdict(list)
+for diagnostic in diagnostics:
+result[diagnostic.get_location()].append(diagnostic)
+return result
 
 
 def compare_results(results_old: AnalysisRun, results_new: AnalysisRun,
@@ -302,52 +355,79 @@
 each element {a,b} is None or a matching element from the respective run
 """
 
-res: ComparisonResult = []
+res = ComparisonResult()
 
 # Map size_before -> size_after
 path_difference_data: List[float] = []
 
-# Quickly eliminate equal elements.
-neq_old: List[AnalysisDiagnostic] = []
-neq_new: List[AnalysisDiagnostic] = []
-
-diags_old = copy(results_old.diagnostics)
-diags_new = copy(results_new.diagnostics)
-
-diags_old.sort(key=cmp_analysis_diagnostic)
-diags_new.sort(key=cmp_analysis_diagnostic)
-
-whi

[clang] 6ddef92 - [analyzer][tests] Understand when diagnostics change between builds

[Valeriy Savchenko via cfe-commits]

Author: Valeriy Savchenko
Date: 2020-08-06T12:53:20+03:00
New Revision: 6ddef92474583ef3c183da9bdc8c8e81ec578fd8

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

LOG: [analyzer][tests] Understand when diagnostics change between builds

Before the patch `SATest compare`, produced quite obscure results
when something about the diagnostic have changed (i.e. its description
or the name of the corresponding checker) because it was simply two
lists of warnings, ADDED and REMOVED.  It was up to the developer
to match those warnings, understand that they are essentially the
same, and figure out what caused the difference.

This patch introduces another category of results: MODIFIED.
It tries to match new warnings against the old ones and prints out
clues on what is different between two builds.

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

Added: 


Modified: 
clang/utils/analyzer/CmpRuns.py

Removed: 




diff  --git a/clang/utils/analyzer/CmpRuns.py b/clang/utils/analyzer/CmpRuns.py
index f7f28d9dc72e..9d5e00767067 100644
--- a/clang/utils/analyzer/CmpRuns.py
+++ b/clang/utils/analyzer/CmpRuns.py
@@ -35,14 +35,16 @@
 from collections import defaultdict
 from copy import copy
 from enum import Enum
-from typing import (Any, cast, Dict, List, NamedTuple, Optional, Sequence,
-TextIO, TypeVar, Tuple, Union)
+from typing import (Any, DefaultDict, Dict, List, NamedTuple, Optional,
+Sequence, TextIO, TypeVar, Tuple, Union)
 
 
 Number = Union[int, float]
 Stats = Dict[str, Dict[str, Number]]
 Plist = Dict[str, Any]
 JSON = Dict[str, Any]
+# Diff in a form: field -> (before, after)
+JSONDiff = Dict[str, Tuple[str, str]]
 # Type for generics
 T = TypeVar('T')
 
@@ -136,6 +138,9 @@ def get_category(self) -> str:
 def get_description(self) -> str:
 return self._data['description']
 
+def get_location(self) -> str:
+return f"{self.get_file_name()}:{self.get_line()}:{self.get_column()}"
+
 def get_issue_identifier(self) -> str:
 id = self.get_file_name() + "+"
 
@@ -172,11 +177,32 @@ def get_readable_name(self) -> str:
 return f"{file_prefix}{funcname_postfix}:{line}:{col}" \
 f", {self.get_category()}: {self.get_description()}"
 
+KEY_FIELDS = ["check_name", "category", "description"]
+
+def is_similar_to(self, other: "AnalysisDiagnostic") -> bool:
+# We consider two diagnostics similar only if at least one
+# of the key fields is the same in both diagnostics.
+return len(self.get_
diff s(other)) != len(self.KEY_FIELDS)
+
+def get_
diff s(self, other: "AnalysisDiagnostic") -> JSONDiff:
+return {field: (self._data[field], other._data[field])
+for field in self.KEY_FIELDS
+if self._data[field] != other._data[field]}
+
 # Note, the data format is not an API and may change from one analyzer
 # version to another.
 def get_raw_data(self) -> Plist:
 return self._data
 
+def __eq__(self, other: object) -> bool:
+return hash(self) == hash(other)
+
+def __ne__(self, other: object) -> bool:
+return hash(self) != hash(other)
+
+def __hash__(self) -> int:
+return hash(self.get_issue_identifier())
+
 
 class AnalysisRun:
 def __init__(self, info: SingleRunInfo):
@@ -283,12 +309,39 @@ def cmp_analysis_diagnostic(d):
 return d.get_issue_identifier()
 
 
-PresentInBoth = Tuple[AnalysisDiagnostic, AnalysisDiagnostic]
-PresentOnlyInOld = Tuple[AnalysisDiagnostic, None]
-PresentOnlyInNew = Tuple[None, AnalysisDiagnostic]
-ComparisonResult = List[Union[PresentInBoth,
-  PresentOnlyInOld,
-  PresentOnlyInNew]]
+AnalysisDiagnosticPair = Tuple[AnalysisDiagnostic, AnalysisDiagnostic]
+
+
+class ComparisonResult:
+def __init__(self):
+self.present_in_both: List[AnalysisDiagnostic] = []
+self.present_only_in_old: List[AnalysisDiagnostic] = []
+self.present_only_in_new: List[AnalysisDiagnostic] = []
+self.changed_between_new_and_old: List[AnalysisDiagnosticPair] = []
+
+def add_common(self, issue: AnalysisDiagnostic):
+self.present_in_both.append(issue)
+
+def add_removed(self, issue: AnalysisDiagnostic):
+self.present_only_in_old.append(issue)
+
+def add_added(self, issue: AnalysisDiagnostic):
+self.present_only_in_new.append(issue)
+
+def add_changed(self, old_issue: AnalysisDiagnostic,
+new_issue: AnalysisDiagnostic):
+self.changed_between_new_and_old.append((old_issue, new_issue))
+
+
+GroupedDiagnostics = DefaultDict[str, List[AnalysisDiagnostic]]
+
+
+def get_grouped_diagnostics(diagnostics: List[AnalysisDiagnostic]

[PATCH] D83942: [analyzer][tests] Add a notion of project sizes

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko updated this revision to Diff 283548.
vsavchenko added a comment.
Herald added a subscriber: steakhal.

Keep SATest.py Python2 compatible


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83942

Files:
  clang/utils/analyzer/ProjectMap.py
  clang/utils/analyzer/SATest.py
  clang/utils/analyzer/projects/projects.json

Index: clang/utils/analyzer/projects/projects.json
===
--- clang/utils/analyzer/projects/projects.json
+++ clang/utils/analyzer/projects/projects.json
@@ -4,139 +4,159 @@
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/jarro2783/cxxopts.git";,
-"commit": "794c975"
+"commit": "794c975",
+"size": "tiny"
   },
   {
 "name": "box2d",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/erincatto/box2d.git";,
-"commit": "1025f9a"
+"commit": "1025f9a",
+"size": "small"
   },
   {
 "name": "tinyexpr",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/codeplea/tinyexpr.git";,
-"commit": "ffb0d41"
+"commit": "ffb0d41",
+"size": "tiny"
   },
   {
 "name": "symengine",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/symengine/symengine.git";,
-"commit": "4f669d59"
+"commit": "4f669d59",
+"size": "small"
   },
   {
 "name": "termbox",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/nsf/termbox.git";,
-"commit": "0df1355"
+"commit": "0df1355",
+"size": "tiny"
   },
   {
 "name": "tinyvm",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/jakogut/tinyvm.git";,
-"commit": "10c25d8"
+"commit": "10c25d8",
+"size": "tiny"
   },
   {
 "name": "tinyspline",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/msteinbeck/tinyspline.git";,
-"commit": "f8b1ab7"
+"commit": "f8b1ab7",
+"size": "tiny"
   },
   {
 "name": "oatpp",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/oatpp/oatpp.git";,
-"commit": "d3e60fb"
+"commit": "d3e60fb",
+"size": "small"
   },
   {
 "name": "libsoundio",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/andrewrk/libsoundio.git";,
-"commit": "b810bf2"
+"commit": "b810bf2",
+"size": "tiny"
   },
   {
 "name": "zstd",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/facebook/zstd.git";,
-"commit": "2af4e073"
+"commit": "2af4e073",
+"size": "small"
   },
   {
 "name": "simbody",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/simbody/simbody.git";,
-"commit": "5cf513d"
+"commit": "5cf513d",
+"size": "big"
   },
   {
 "name": "duckdb",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/cwida/duckdb.git";,
-"commit": "d098c9f"
+"commit": "d098c9f",
+"size": "big"
   },
   {
 "name": "drogon",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/an-tao/drogon.git";,
-"commit": "fd2a612"
+"commit": "fd2a612",
+"size": "small"
   },
   {
 "name": "fmt",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/fmtlib/fmt.git";,
-"commit": "5e7c70e"
+"commit": "5e7c70e",
+"size": "small"
   },
   {
 "name": "re2",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/google/re2.git";,
-"commit": "2b25567"
+"commit": "2b25567",
+"size": "small"
   },
   {
 "name": "cppcheck",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/danmar/cppcheck.git";,
-"commit": "5fa3d53"
+"commit": "5fa3d53",
+"size": "small"
   },
   {
 "name": "harfbuzz",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/harfbuzz/harfbuzz.git";,
-"commit": "f8d345e"
+"commit": "f8d345e",
+"size": "small"
   },
   {
 "name": "capnproto",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/capnproto/capnproto.git";,
-"commit": "8be1c9f"
+"commit": "8be1c9f",
+"size": "small"
   },
   {
 "name": "tmux",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/tmux/tmux.git";,
-"commit": "a5f99e1"
+"commit": "a5f99e1",
+"size": "big"
   },
   {
 "name": "faiss",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/facebookresearch/faiss.git";,
-"commit": "9e5d5b7"
+"commit": "9e5d5b7",
+"size": "small"
   }
 ]
Index: clang/utils/analyzer/SATest.py
===
--- clang/utils/analyzer/SATest.py
+++ clang/utils/analyzer/SATest.py
@@ -37,7 +37,7 @@
 
 SATestBuild.VERBOSE = args.verbose
 
-projects = get_projects(parser, args.projects)
+projects = get_projects(parser, args)

[PATCH] D85417: [clangd] Fix crash in bugprone-bad-signal-to-kill-thread clang-tidy check.

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, 
dexonsmith, steven_wu, jkorous, hiraditya.
Herald added a project: clang.
ArcsinX requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Inside clangd, clang-tidy checks don't see preprocessor events in the preamble.
This leads to `Token::PtrData == nullptr` for tokens that the macro is defined 
to.
E.g. `#define SIGTERM 15`:

- Token::Kind == tok::numeric_constant (Token::isLiteral() == true)
- Token::UintData == 2
- Token::PtrData == nullptr

As the result of this, bugprone-bad-signal-to-kill-thread check crashes at 
null-dereference inside clangd.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85417

Files:
  clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -438,6 +438,21 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
+TEST(DiagnosticTest, ClangTidyBadSignalToKillThread) {
+  Annotations Main(R"cpp(
+#define SIGTERM 15
+using pthread_t = int;
+int pthread_kill(pthread_t thread, int sig);
+int func() {
+  pthread_t thread;
+  return pthread_kill(thread, 0);
+}
+  )cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.ClangTidyChecks = "-*,bugprone-bad-signal-to-kill-thread";
+  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash
+}
+
 TEST(DiagnosticsTest, Preprocessor) {
   // This looks like a preamble, but there's an #else in the middle!
   // Check that:
Index: clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
@@ -39,7 +39,7 @@
   return llvm::None;
 const MacroInfo *MI = PP->getMacroInfo(It->first);
 const Token &T = MI->tokens().back();
-if (!T.isLiteral())
+if (!T.isLiteral() || !T.getLiteralData())
   return llvm::None;
 StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
 


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -438,6 +438,21 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
+TEST(DiagnosticTest, ClangTidyBadSignalToKillThread) {
+  Annotations Main(R"cpp(
+#define SIGTERM 15
+using pthread_t = int;
+int pthread_kill(pthread_t thread, int sig);
+int func() {
+  pthread_t thread;
+  return pthread_kill(thread, 0);
+}
+  )cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.ClangTidyChecks = "-*,bugprone-bad-signal-to-kill-thread";
+  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash
+}
+
 TEST(DiagnosticsTest, Preprocessor) {
   // This looks like a preamble, but there's an #else in the middle!
   // Check that:
Index: clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
@@ -39,7 +39,7 @@
   return llvm::None;
 const MacroInfo *MI = PP->getMacroInfo(It->first);
 const Token &T = MI->tokens().back();
-if (!T.isLiteral())
+if (!T.isLiteral() || !T.getLiteralData())
   return llvm::None;
 StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

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

balazske added a comment.
Herald added a subscriber: whisperity.

Test results for tmux are available here 
.
 (Could not manage to get other results uploaded.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72705

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

[PATCH] D85256: Add -Wtautological-value-range-compare warning.

[Richard Trieu via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85256

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

[PATCH] D85337: [AMDGPU] gfx1031 target

[James Henderson via Phabricator via cfe-commits]

jhenderson added inline comments.



Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:1844
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1030),
+  LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1031),
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_XNACK),

Where's the dedicated llvm-readobj test for this? Please add one. If there 
exists one for the other AMDGPU flags, extend that one, otherwise, it might be 
best to write one for the whole set at once.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85337

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

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

[Petr Hosek via Phabricator via cfe-commits]

phosek updated this revision to Diff 283398.
phosek marked an inline comment as done.

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

https://reviews.llvm.org/D79219

Files:
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/cmake/modules/LLDBStandalone.cmake
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp
  llvm/utils/gn/secondary/clang/test/BUILD.gn
  llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
  llvm/utils/gn/secondary/lld/test/BUILD.gn
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  llvm/utils/gn/secondary/llvm/test/BUILD.gn
  mlir/examples/standalone/CMakeLists.txt

Index: mlir/examples/standalone/CMakeLists.txt
===
--- mlir/examples/standalone/CMakeLists.txt
+++ mlir/examples/standalone/CMakeLists.txt
@@ -28,6 +28,7 @@
 
 list(APPEND CMAKE_MODULE_PATH "${MLIR_CMAKE_DIR}")
 list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_DIR}")
+
 include(TableGen)
 include(AddLLVM)
 include(AddMLIR)
Index: llvm/utils/gn/secondary/llvm/test/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/test/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/test/BUILD.gn
@@ -174,9 +174,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 }
 
Index: llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
@@ -295,20 +295,10 @@
 values += [ "LLVM_ENABLE_DIA_SDK=" ]
   }
 
-  # FIXME: Once https://reviews.llvm.org/D79219 is in, remove the two
-  # redundant HAVE_ variables.
   if (llvm_enable_zlib) {
-values += [
-  "HAVE_LIBZ=1",
-  "HAVE_ZLIB_H=1",
-  "LLVM_ENABLE_ZLIB=1",
-]
+values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-values += [
-  "HAVE_LIBZ=",
-  "HAVE_ZLIB_H=",
-  "LLVM_ENABLE_ZLIB=",
-]
+values += [ "LLVM_ENABLE_ZLIB=" ]
   }
 
   if (llvm_enable_libxml2) {
Index: llvm/utils/gn/secondary/lld/test/BUILD.gn
===
--- llvm/utils/gn/secondary/lld/test/BUILD.gn
+++ llvm/utils/gn/secondary/lld/test/BUILD.gn
@@ -49,9 +49,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 
   if (current_cpu == "x64" || current_cpu == "arm64" ||
Index: llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
===
--- llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
+++ llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
@@ -86,8 +86,8 @@
   }
 
   if (llvm_enable_zlib) {
-values += [ "HAVE_LIBZ=1" ]
+values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-values += [ "HAVE_LIBZ=0" ]
+values += [ "LLVM_ENABLE_ZLIB=0" ]
   }
 }
Index: llvm/utils/gn/secondary/clang/test/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/test/BUILD.gn
+++ llvm/utils/gn/secondary/clang/test/BUILD.gn
@@ -79,9 +79,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 
   if (host_cpu == "x64") {
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitiz

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1adc494bce44: [CMake] Simplify CMake handling for zlib 
(authored by phosek).

Changed prior to commit:
  https://reviews.llvm.org/D79219?vs=283398&id=283434#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

Files:
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/cmake/modules/LLDBStandalone.cmake
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp
  llvm/utils/gn/secondary/clang/test/BUILD.gn
  llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
  llvm/utils/gn/secondary/lld/test/BUILD.gn
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  llvm/utils/gn/secondary/llvm/test/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/test/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/test/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/test/BUILD.gn
@@ -174,9 +174,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 }
 
Index: llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
@@ -295,20 +295,10 @@
 values += [ "LLVM_ENABLE_DIA_SDK=" ]
   }
 
-  # FIXME: Once https://reviews.llvm.org/D79219 is in, remove the two
-  # redundant HAVE_ variables.
   if (llvm_enable_zlib) {
-values += [
-  "HAVE_LIBZ=1",
-  "HAVE_ZLIB_H=1",
-  "LLVM_ENABLE_ZLIB=1",
-]
+values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-values += [
-  "HAVE_LIBZ=",
-  "HAVE_ZLIB_H=",
-  "LLVM_ENABLE_ZLIB=",
-]
+values += [ "LLVM_ENABLE_ZLIB=" ]
   }
 
   if (llvm_enable_libxml2) {
Index: llvm/utils/gn/secondary/lld/test/BUILD.gn
===
--- llvm/utils/gn/secondary/lld/test/BUILD.gn
+++ llvm/utils/gn/secondary/lld/test/BUILD.gn
@@ -49,9 +49,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 
   if (current_cpu == "x64" || current_cpu == "arm64" ||
Index: llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
===
--- llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
+++ llvm/utils/gn/secondary/compiler-rt/test/BUILD.gn
@@ -86,8 +86,8 @@
   }
 
   if (llvm_enable_zlib) {
-values += [ "HAVE_LIBZ=1" ]
+values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-values += [ "HAVE_LIBZ=0" ]
+values += [ "LLVM_ENABLE_ZLIB=0" ]
   }
 }
Index: llvm/utils/gn/secondary/clang/test/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/test/BUILD.gn
+++ llvm/utils/gn/secondary/clang/test/BUILD.gn
@@ -79,9 +79,9 @@
   }
 
   if (llvm_enable_zlib) {
-extra_values += [ "HAVE_LIBZ=1" ]
+extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
-extra_values += [ "HAVE_LIBZ=0" ]  # Must be 0.
+extra_values += [ "LLVM_ENABLE_ZLIB=0" ]  # Must be 0.
   }
 
   if (host_cpu == "x64") {
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

[Petr Hosek via Phabricator via cfe-commits]

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

Sorry about the breakage, that was an unintentional change. I have updated the 
patch and restored the original behavior on Windows.


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

https://reviews.llvm.org/D79219

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

[PATCH] D85337: [AMDGPU] gfx1031 target

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGea7d0e2996ec: [AMDGPU] gfx1031 target (authored by rampitec).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85337

Files:
  clang/include/clang/Basic/Cuda.h
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/test/CodeGenOpenCL/amdgpu-features.cl
  clang/test/Driver/amdgpu-macros.cl
  clang/test/Driver/amdgpu-mcpu.cl
  llvm/docs/AMDGPUUsage.rst
  llvm/include/llvm/BinaryFormat/ELF.h
  llvm/include/llvm/Support/TargetParser.h
  llvm/lib/ObjectYAML/ELFYAML.cpp
  llvm/lib/Support/TargetParser.cpp
  llvm/lib/Target/AMDGPU/GCNProcessors.td
  llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUTargetStreamer.cpp
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-fmad.s32.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.global.atomic.csub.ll
  llvm/test/CodeGen/AMDGPU/elf-header-flags-mach.ll
  llvm/test/CodeGen/AMDGPU/hsa-note-no-func.ll
  llvm/test/CodeGen/AMDGPU/idot8s.ll
  llvm/test/CodeGen/AMDGPU/llvm.amdgcn.atomic.csub.ll
  llvm/test/CodeGen/AMDGPU/llvm.amdgcn.sdot4.ll
  llvm/test/CodeGen/AMDGPU/llvm.amdgcn.sdot8.ll
  llvm/test/MC/AMDGPU/gfx1030_err.s
  llvm/test/MC/AMDGPU/gfx1030_new.s
  llvm/test/MC/Disassembler/AMDGPU/gfx1030_dasm_new.txt
  llvm/tools/llvm-readobj/ELFDumper.cpp

Index: llvm/tools/llvm-readobj/ELFDumper.cpp
===
--- llvm/tools/llvm-readobj/ELFDumper.cpp
+++ llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -1841,6 +1841,7 @@
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1011),
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1012),
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1030),
+  LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1031),
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_XNACK),
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_SRAM_ECC)
 };
Index: llvm/test/MC/Disassembler/AMDGPU/gfx1030_dasm_new.txt
===
--- llvm/test/MC/Disassembler/AMDGPU/gfx1030_dasm_new.txt
+++ llvm/test/MC/Disassembler/AMDGPU/gfx1030_dasm_new.txt
@@ -1,4 +1,5 @@
 # RUN: llvm-mc -arch=amdgcn -mcpu=gfx1030 -disassemble -show-encoding < %s | FileCheck -check-prefix=GFX10 %s
+# RUN: llvm-mc -arch=amdgcn -mcpu=gfx1031 -disassemble -show-encoding < %s | FileCheck -check-prefix=GFX10 %s
 
 # GFX10: global_load_dword_addtid v1, s[2:3] offset:16
 0x10,0x80,0x58,0xdc,0x00,0x00,0x02,0x01
Index: llvm/test/MC/AMDGPU/gfx1030_new.s
===
--- llvm/test/MC/AMDGPU/gfx1030_new.s
+++ llvm/test/MC/AMDGPU/gfx1030_new.s
@@ -1,4 +1,5 @@
 // RUN: llvm-mc -arch=amdgcn -mcpu=gfx1030 -show-encoding %s | FileCheck --check-prefix=GFX10 %s
+// RUN: llvm-mc -arch=amdgcn -mcpu=gfx1031 -show-encoding %s | FileCheck --check-prefix=GFX10 %s
 
 global_load_dword_addtid v1, s[2:3] offset:16
 // GFX10: encoding: [0x10,0x80,0x58,0xdc,0x00,0x00,0x02,0x01]
Index: llvm/test/MC/AMDGPU/gfx1030_err.s
===
--- llvm/test/MC/AMDGPU/gfx1030_err.s
+++ llvm/test/MC/AMDGPU/gfx1030_err.s
@@ -1,4 +1,5 @@
 // RUN: not llvm-mc -arch=amdgcn -mcpu=gfx1030 -show-encoding %s 2>&1 | FileCheck --check-prefix=GFX10 %s
+// RUN: not llvm-mc -arch=amdgcn -mcpu=gfx1031 -show-encoding %s 2>&1 | FileCheck --check-prefix=GFX10 %s
 
 v_dot8c_i32_i4 v5, v1, v2
 // GFX10: error:
Index: llvm/test/CodeGen/AMDGPU/llvm.amdgcn.sdot8.ll
===
--- llvm/test/CodeGen/AMDGPU/llvm.amdgcn.sdot8.ll
+++ llvm/test/CodeGen/AMDGPU/llvm.amdgcn.sdot8.ll
@@ -3,6 +3,7 @@
 ; RUN: llc -march=amdgcn -mcpu=gfx1011 -verify-machineinstrs < %s | FileCheck %s --check-prefixes=GCN,GFX10
 ; RUN: llc -march=amdgcn -mcpu=gfx1012 -verify-machineinstrs < %s | FileCheck %s --check-prefixes=GCN,GFX10
 ; RUN: llc -march=amdgcn -mcpu=gfx1030 -verify-machineinstrs < %s | FileCheck %s --check-prefixes=GCN,GFX10
+; RUN: llc -march=amdgcn -mcpu=gfx1031 -verify-machineinstrs < %s | FileCheck %s --check-prefixes=GCN,GFX10
 
 declare i32 @llvm.amdgcn.sdot8(i32 %a, i32 %b, i32 %c, i1 %clamp)
 
Index: llvm/test/CodeGen/AMDGPU/llvm.amdgcn.sdot4.ll
===
--- llvm/test/CodeGen/AMDGPU/llvm.amdgcn.sdot4.ll
+++ llvm/test/CodeGen/AMDGPU/llvm.amdgcn.sdot4.ll
@@ -2,6 +2,7 @@
 ; RUN: llc -march=amdgcn -mcpu=gfx1011 -verify-machineinstrs < %s | FileCheck %s --check-prefixes=GCN,GFX10
 ; RUN: llc -march=amdgcn -mcpu=gfx1012 -verify-machineinstrs < %s | FileCheck %s --check-prefixes=GCN,GFX10
 ; RUN: llc -march=amdgcn -mcpu=gfx1030 -verify-machineinstrs < %s | FileCheck %s --check-prefixes=GCN,GFX10
+; RUN: llc -march=amdgcn -mcpu=gfx1031 -verify-machineinstrs < %s | FileCheck %s --check-prefixes=

[PATCH] D62574: Initial draft of target-configurable address spaces.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

In D62574#2183294 , @Anastasia wrote:

> In D62574#2136423 , @ebevhan wrote:
>
>> It seems that D70605  attempted to 
>> ameliorate the issues that I observed (pointer-conversion doing too much), 
>> but it didn't manage to solve the problem fully.
>
> Can you explain what's left to be done?

My cache is a bit flushed on this at the moment but I do believe that if the 
issue described in D83325  is resolved, this 
patch should work.




Comment at: lib/Sema/SemaOverload.cpp:3171
+  // qualification conversion for disjoint address spaces make address space
+  // casts *work*?
   Qualifiers FromQuals = FromType.getQualifiers();

Anastasia wrote:
> ebevhan wrote:
> > Anastasia wrote:
> > > ebevhan wrote:
> > > > Anastasia wrote:
> > > > > Anastasia wrote:
> > > > > > ebevhan wrote:
> > > > > > > Anastasia wrote:
> > > > > > > > I guess if address spaces don't overlap we don't have a valid 
> > > > > > > > qualification conversion. This seems to align with the logic 
> > > > > > > > for cv. My guess is that none of the other conversions will be 
> > > > > > > > valid for non overlapping address spaces and the error will 
> > > > > > > > occur.
> > > > > > > > 
> > > > > > > > I think at this point we might not need to know if it's 
> > > > > > > > implicit or explicit? I believe we might have a separate check 
> > > > > > > > for this somewhere because it works for OpenCL. I don't know 
> > > > > > > > though if it might simplify the flow if we move this logic 
> > > > > > > > rather here.
> > > > > > > > 
> > > > > > > > The cv checks above seem to use `CStyle` flag. I am wondering 
> > > > > > > > if we could use it to detect implicit or explicit. Because we 
> > > > > > > > can only cast address space with C style cast at the moment.  
> > > > > > > > Although after adding `addrspace_cast` operator that will no 
> > > > > > > > longer be the only way.
> > > > > > > > I guess if address spaces don't overlap we don't have a valid 
> > > > > > > > qualification conversion. This seems to align with the logic 
> > > > > > > > for cv. My guess is that none of the other conversions will be 
> > > > > > > > valid for non overlapping address spaces and the error will 
> > > > > > > > occur.
> > > > > > > 
> > > > > > > Right. So the reasoning is that if the address spaces are 
> > > > > > > disjoint according to the overlap rule, then it cannot be 
> > > > > > > considered a qualification conversion.
> > > > > > > 
> > > > > > > But with the new hooks, it's possible for a target to allow 
> > > > > > > explicit conversion even if address spaces do not overlap 
> > > > > > > according to the rules. So even though there is no overlap, such 
> > > > > > > a conversion could still be a qualification conversion if it was 
> > > > > > > explicit (either via a c-style cast or an `addrspace_cast`). This 
> > > > > > > is in fact the default for all targets (see the patch in 
> > > > > > > TargetInfo.h).
> > > > > > > 
> > > > > > > I think I need a refresher on how the casts were meant to work; 
> > > > > > > were both `static_cast` and `reinterpret_cast` supposed to be 
> > > > > > > capable of implicit conversion (say, private -> generic) but only 
> > > > > > > `addrspace_cast` for the other direction (generic -> private)? Or 
> > > > > > > was `reinterpret_cast` supposed to fail in the first case?
> > > > > > Just as a summary:
> > > > > > 
> > > > > > - All casts should allow safe/implicit AS conversions i.e. 
> > > > > > `__private`/`__local`/`__global` -> `__generic` in OpenCL
> > > > > > - All casts except for C-style and `addrspace_cast` should disallow 
> > > > > > unsafe/explicit conversions i.e. generic -> 
> > > > > > `__private`/`__local`/`__global` in OpenCL
> > > > > > - All casts should disallow forbidden conversions with no address 
> > > > > > space overlap i.e. `__constant` <-> any other in OpenCL
> > > > > > 
> > > > > > In OpenCL overlapping logic is only used for explicit i.e. unsafe 
> > > > > > conversion. So it seems we might only need those conversions here 
> > > > > > then? 
> > > > > Did you have time to look into this?
> > > > I still don't really understand why the code checks for overlap here. 
> > > > Removing this check causes one test case to break, 
> > > > CodeGenCXX/address-space-cast.cpp. Specifically, this:
> > > > 
> > > > ```
> > > > #define __private__ __attribute__((address_space(5)))
> > > > 
> > > > void test_cast(char *gen_char_ptr, void *gen_void_ptr, int 
> > > > *gen_int_ptr) {
> > > >   __private__ void *priv_void_ptr = (__private__ void *)gen_char_ptr;
> > > > }
> > > > ```
> > > > 
> > > > It tries to resolve the C-style cast with TryAddressSpaceCast, but 
> > > > fails as the underlying pointee types (`char` and `void`) are 
> > > > different. Then it tries to do it a

[PATCH] D85390: [Clang] Add note for bad conversion error when expression is of forward-declared type

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Thanks! This looks promising.

In addition to updating existing tests, it would be good to add a test aimed 
explicitly at this. Maybe you can reduce something from the example in the bug 
report. Maybe looking at git blame for emitBadConversionNotes() will provide 
hints for where existing tests for such notes were added.




Comment at: clang/lib/Sema/SemaInit.cpp:8710
   }
+  QualType fromType = op->getType();
+  auto *fromDecl = fromType.getTypePtr()->getPointeeCXXRecordDecl();

Can the reverse situation happen, where it's destType that's forward declared?



Comment at: clang/lib/Sema/SemaInit.cpp:8713
+  if (fromDecl && !fromDecl->hasDefinition() && !fromDecl->isInvalidDecl() &&
+  fromDecl->getDeclKind() == Decl::CXXRecord)
+S.Diag(fromDecl->getLocation(), diag::note_forward_declaration)

Is the getDeclKind() still necessary even though you're doing 
getPointeeCXXRecordDecl() above and fromDecl is os type CXXRecordDecl*?



Comment at: clang/lib/Sema/SemaInit.cpp:8714
+  fromDecl->getDeclKind() == Decl::CXXRecord)
+S.Diag(fromDecl->getLocation(), diag::note_forward_declaration)
+<< S.getASTContext().getTagDeclType(fromDecl);

I think we could add a new note instead of just note_forward_declaration which 
is more helpful, explaining that maybe the conversion would be valid if the 
type was defined and not just forward declared.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85390

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

[PATCH] D85423: [AST][RecoveryExpr] Fix the missing type when rebuilding RecoveryExpr in TreeTransform.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: clang.
hokein requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85423

Files:
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaTemplate/recovery-tree-transform-crash.cpp


Index: clang/test/SemaTemplate/recovery-tree-transform-crash.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/recovery-tree-transform-crash.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -verify -frecovery-ast -frecovery-ast-type %s
+
+template  struct Ptr { T *operator->() const; };
+
+struct ABC {
+  void run();
+};
+
+Ptr call(int); // expected-note {{candidate function not viable}}
+
+void test() {
+  call()->run(undef); // expected-error {{no matching function for call to 
'call'}} \
+ expected-error {{use of undeclared identifier}}
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -3616,8 +3616,8 @@
   }
 
   ExprResult RebuildRecoveryExpr(SourceLocation BeginLoc, SourceLocation 
EndLoc,
- ArrayRef SubExprs) {
-return getSema().CreateRecoveryExpr(BeginLoc, EndLoc, SubExprs);
+ ArrayRef SubExprs, QualType Type) {
+return getSema().CreateRecoveryExpr(BeginLoc, EndLoc, SubExprs, Type);
   }
 
 private:
@@ -10207,7 +10207,7 @@
   if (!getDerived().AlwaysRebuild() && !Changed)
 return E;
   return getDerived().RebuildRecoveryExpr(E->getBeginLoc(), E->getEndLoc(),
-  Children);
+  Children, E->getType());
 }
 
 template


Index: clang/test/SemaTemplate/recovery-tree-transform-crash.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/recovery-tree-transform-crash.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -verify -frecovery-ast -frecovery-ast-type %s
+
+template  struct Ptr { T *operator->() const; };
+
+struct ABC {
+  void run();
+};
+
+Ptr call(int); // expected-note {{candidate function not viable}}
+
+void test() {
+  call()->run(undef); // expected-error {{no matching function for call to 'call'}} \
+ expected-error {{use of undeclared identifier}}
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -3616,8 +3616,8 @@
   }
 
   ExprResult RebuildRecoveryExpr(SourceLocation BeginLoc, SourceLocation EndLoc,
- ArrayRef SubExprs) {
-return getSema().CreateRecoveryExpr(BeginLoc, EndLoc, SubExprs);
+ ArrayRef SubExprs, QualType Type) {
+return getSema().CreateRecoveryExpr(BeginLoc, EndLoc, SubExprs, Type);
   }
 
 private:
@@ -10207,7 +10207,7 @@
   if (!getDerived().AlwaysRebuild() && !Changed)
 return E;
   return getDerived().RebuildRecoveryExpr(E->getBeginLoc(), E->getEndLoc(),
-  Children);
+  Children, E->getType());
 }
 
 template
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: llvm/cmake/config-ix.cmake:178
 
-if (LLVM_ENABLE_ZLIB STREQUAL "FORCE_ON" AND NOT HAVE_LIBZ)
-  message(FATAL_ERROR "Failed to configure zlib")

Could this check be put back? E.g. for now it seems building with 
-DLLVM_USE_SANITIZERS=Memory and -DLLVM_ENABLE_ZLIB=FORCE_ON will silently not 
use zlib.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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

[PATCH] D85424: [Analyzer] Crash fix for alpha.cplusplus.IteratorRange

[Balogh , Ádám via Phabricator via cfe-commits]

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

If the non-iterator side of an iterator operation `+`, `+=`, `-` or `-=` is 
`UndefinedVal` an assertions happens. This small fix prevents this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85424

Files:
  clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
@@ -228,7 +228,7 @@
 Value = State->getRawSVal(*ValAsLoc);
   }
 
-  if (Value.isUnknown())
+  if (Value.isUnknownOrUndef())
 return;
 
   // Incremention or decremention by 0 is never a bug.


Index: clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
@@ -228,7 +228,7 @@
 Value = State->getRawSVal(*ValAsLoc);
   }
 
-  if (Value.isUnknown())
+  if (Value.isUnknownOrUndef())
 return;
 
   // Incremention or decremention by 0 is never a bug.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85287: Extend -Wtautological-bitwise-compare "bitwise or with non-zero value" warnings

[Richard Trieu via Phabricator via cfe-commits]

rtrieu added a subscriber: AndersRonnholm.
rtrieu added a comment.

I looked back on the commits and while I did commit, I did it on the behalf of 
Anders Rönnholm, who did not have commit access at the time.  I haven't seen 
activity from Anders in a while, but added to subscribers just in case.

Way back then, the warning only did operands of a DeclRefExpr and an 
IntegerLiteral.  Over time, that has been extended, case by case, to include 
whatever new cases people can think up.  I don't mind extending the warnings, 
but we need to be mindful of how the warnings appear.  If the sub-expression 
becomes too large, it will be difficult for the user to understand where the 
problem is and which constants the compiler is talking about.  We may already 
be at that point.  The example could have a more complex initializer for the 
constant variables, and the warning would be harder to follow.  Maybe we also 
look at the variable initializers and only allow for simple ones.  I need to 
give this some more thought.

> To avoid potential further false positives, restrict this change only to the
> "bitwise or with non-zero value" warnings while keeping all other
> -Wtautological-bitwise-compare warnings as-is, at least for now.

Are you planning to allow this change to other warnings that use the same 
helper functions?
Also, have you tried running warning over a codebase?




Comment at: clang/lib/Analysis/CFG.cpp:96-97
 
-/// Helper for tryNormalizeBinaryOperator. Attempts to extract an 
IntegerLiteral
-/// constant expression or EnumConstantDecl from the given Expr. If it fails,
+/// Helper for tryNormalizeBinaryOperator. Attempts to extract a suitable
+/// integer or enum constant from the given Expr. If it fails,
 /// returns nullptr.

The original comment specifies the allowed Expr's by the specific AST nodes 
they represent.  Please use that.  I think "IntegerLiteral constant expression, 
EnumConstantDecl, or constant value VarDecl" would work.



Comment at: clang/lib/Analysis/CFG.cpp:110
+  if (VD->isUsableInConstantExpressions(Ctx))
+return DR;
+  }

IntergerLiteral and EnumConstantDecl are known to have integer types.  However, 
it is possible for a VarDecl to have other types.  There should be a check for 
integer types here.



Comment at: clang/lib/Analysis/CFG.cpp:175
 
-  assert(isa(DC1) && isa(DC2));
-  return DC1 == DC2;
+  return false;
 }

Need a comment here about how tryTransformToIntOrEnumConstant also allows a 
DeclRefExpr to have a constant VarDecl, but this case is currently excluded for 
this warning.


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

https://reviews.llvm.org/D85287

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

[PATCH] D85424: [Analyzer] Crash fix for alpha.cplusplus.IteratorRange

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Unfortunately, I could not create test for it. It is extremely rare that the 
//Analyzer// creates an `UndefinedVal`. I had the following output:

  1. parser at end of file
  2.While analyzing stack: 
#0 Calling llvm::object::BindRebaseSegInfo::BindRebaseSegInfo at line 
/usr/include/c++/7/bits/unique_ptr.h:821:34
#1 Calling std::make_unique at line 4094
#2 Calling llvm::object::MachOObjectFile::bindTable at line 4115
#3 Calling llvm::object::MachOObjectFile::weakBindTable
  3./home/edmbalo/llvm-project/llvm/lib/Object/MachOObjectFile.cpp:4004:28: 
Error evaluating statement
  4./home/edmbalo/llvm-project/llvm/lib/Object/MachOObjectFile.cpp:4004:28: 
Error evaluating statement

I was analyzing `llvm/lib/Object/MachOObjectFile.cpp` with all the 
iterator-related checkers enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85424

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

[PATCH] D85287: Extend -Wtautological-bitwise-compare "bitwise or with non-zero value" warnings

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

In D85287#2199463 , @rtrieu wrote:

> Are you planning to allow this change to other warnings that use the same 
> helper functions?

No, I didn't plan to work on this further.  Just scratching that itch of why 
Clang didn't emit that one warning it "obviously" should have emitted.

> Also, have you tried running warning over a codebase?

As I wrote: "At least building LibreOffice with this change caused no false 
positives."  (Or maybe I misunderstand your question.)


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

https://reviews.llvm.org/D85287

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

[PATCH] D85426: [clangd] Implement FileFilter for the indexer

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Tests failing so far: BackgroundIndexTests.IndexTwoFiles,
BackgroundIndexTests.ShardStorageLoad.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85426

Files:
  clang-tools-extra/clangd/index/SymbolCollector.cpp


Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -340,8 +340,9 @@
 // TU, because in practice they are definitions.
 BasicSymbol = addDeclaration(*OriginalDecl, std::move(*ID), 
IsMainFileOnly);
 
-  if (Roles & static_cast(index::SymbolRole::Definition))
-addDefinition(*OriginalDecl, *BasicSymbol);
+  if (BasicSymbol != nullptr)
+if (Roles & static_cast(index::SymbolRole::Definition))
+  addDefinition(*OriginalDecl, *BasicSymbol);
 
   return true;
 }
@@ -434,8 +435,8 @@
   S.SymInfo = index::getSymbolInfoForMacro(*MI);
   S.Origin = Opts.Origin;
   std::string FileURI;
-  // FIXME: use the result to filter out symbols.
-  shouldIndexFile(SM.getFileID(Loc));
+  if (!shouldIndexFile(SM.getFileID(Loc)))
+return false;
   if (auto DeclLoc =
   getTokenLocation(DefLoc, SM, Opts, PP->getLangOpts(), FileURI))
 S.CanonicalDeclaration = *DeclLoc;
@@ -561,8 +562,8 @@
   const std::pair &LocAndRole,
   bool Spelled = false) {
 auto FileID = SM.getFileID(LocAndRole.first);
-// FIXME: use the result to filter out references.
-shouldIndexFile(FileID);
+if (!shouldIndexFile(FileID))
+  return;
 if (auto FileURI = GetURI(FileID)) {
   auto Range =
   getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts());
@@ -639,8 +640,8 @@
   std::string FileURI;
   auto Loc = nameLocation(ND, SM);
   assert(Loc.isValid() && "Invalid source location for NamedDecl");
-  // FIXME: use the result to filter out symbols.
-  shouldIndexFile(SM.getFileID(Loc));
+  if (!shouldIndexFile(SM.getFileID(Loc)))
+return nullptr;
   if (auto DeclLoc =
   getTokenLocation(Loc, SM, Opts, ASTCtx->getLangOpts(), FileURI))
 S.CanonicalDeclaration = *DeclLoc;
@@ -699,8 +700,8 @@
   std::string FileURI;
   const auto &SM = ND.getASTContext().getSourceManager();
   auto Loc = nameLocation(ND, SM);
-  // FIXME: use the result to filter out symbols.
-  shouldIndexFile(SM.getFileID(Loc));
+  if (!shouldIndexFile(SM.getFileID(Loc)))
+return;
   if (auto DefLoc =
   getTokenLocation(Loc, SM, Opts, ASTCtx->getLangOpts(), FileURI))
 S.Definition = *DefLoc;


Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -340,8 +340,9 @@
 // TU, because in practice they are definitions.
 BasicSymbol = addDeclaration(*OriginalDecl, std::move(*ID), IsMainFileOnly);
 
-  if (Roles & static_cast(index::SymbolRole::Definition))
-addDefinition(*OriginalDecl, *BasicSymbol);
+  if (BasicSymbol != nullptr)
+if (Roles & static_cast(index::SymbolRole::Definition))
+  addDefinition(*OriginalDecl, *BasicSymbol);
 
   return true;
 }
@@ -434,8 +435,8 @@
   S.SymInfo = index::getSymbolInfoForMacro(*MI);
   S.Origin = Opts.Origin;
   std::string FileURI;
-  // FIXME: use the result to filter out symbols.
-  shouldIndexFile(SM.getFileID(Loc));
+  if (!shouldIndexFile(SM.getFileID(Loc)))
+return false;
   if (auto DeclLoc =
   getTokenLocation(DefLoc, SM, Opts, PP->getLangOpts(), FileURI))
 S.CanonicalDeclaration = *DeclLoc;
@@ -561,8 +562,8 @@
   const std::pair &LocAndRole,
   bool Spelled = false) {
 auto FileID = SM.getFileID(LocAndRole.first);
-// FIXME: use the result to filter out references.
-shouldIndexFile(FileID);
+if (!shouldIndexFile(FileID))
+  return;
 if (auto FileURI = GetURI(FileID)) {
   auto Range =
   getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts());
@@ -639,8 +640,8 @@
   std::string FileURI;
   auto Loc = nameLocation(ND, SM);
   assert(Loc.isValid() && "Invalid source location for NamedDecl");
-  // FIXME: use the result to filter out symbols.
-  shouldIndexFile(SM.getFileID(Loc));
+  if (!shouldIndexFile(SM.getFileID(Loc)))
+return nullptr;
   if (auto DeclLoc =
   getTokenLocation(Loc, SM, Opts, ASTCtx->getLangOpts(), FileURI))
 S.CanonicalDeclaration = *DeclLoc;
@@ -699,8 +700,8 @@
   std::string FileURI;
   const auto &SM = ND.getASTContext().getSourceManager();
   auto Loc = nameLocation(ND, SM);
-  //

[PATCH] D80514: [clang-tidy] modernize-use-trailing-return-type support for C++20 concepts and decltype

[Bernhard Manfred Gruber via Phabricator via cfe-commits]

bernhardmgruber added a comment.

Ping.


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

https://reviews.llvm.org/D80514

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

[PATCH] D85426: [clangd] Implement FileFilter for the indexer

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev planned changes to this revision.
kbobyrev added a comment.

@sammccall This is the change I was talking about during the standup: the code 
looks legit, but `BackgroundIndexTests.IndexTwoFiles` fails because with the 
following code path in `Background.cpp`'s `FileFilter`:

  const auto *F = SM.getFileEntryForID(FID);
  if (!F)
return false; // Skip invalid files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85426

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

[PATCH] D85427: [SyntaxTree][NFC] remove redundant namespace-specifiers

[Eduardo Caldas via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85427

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/include/clang/Tooling/Syntax/Tree.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Tree.cpp

Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -36,9 +36,7 @@
  const TokenBuffer &Tokens)
 : SourceMgr(SourceMgr), LangOpts(LangOpts), Tokens(Tokens) {}
 
-const clang::syntax::TokenBuffer &syntax::Arena::tokenBuffer() const {
-  return Tokens;
-}
+const syntax::TokenBuffer &syntax::Arena::tokenBuffer() const { return Tokens; }
 
 std::pair>
 syntax::Arena::lexBuffer(std::unique_ptr Input) {
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -45,7 +45,7 @@
 using namespace clang;
 
 LLVM_ATTRIBUTE_UNUSED
-static bool isImplicitExpr(clang::Expr *E) { return E->IgnoreImplicit() != E; }
+static bool isImplicitExpr(Expr *E) { return E->IgnoreImplicit() != E; }
 
 namespace {
 /// Get start location of the Declarator from the TypeLoc.
@@ -384,7 +384,7 @@
   }
 
   llvm::ArrayRef getDeclarationRange(Decl *D) {
-llvm::ArrayRef Tokens;
+llvm::ArrayRef Tokens;
 // We want to drop the template parameters for specializations.
 if (const auto *S = llvm::dyn_cast(D))
   Tokens = getRange(S->TypeDecl::getBeginLoc(), S->getEndLoc());
@@ -722,16 +722,16 @@
   syntax::UserDefinedLiteralExpression *
   buildUserDefinedLiteral(UserDefinedLiteral *S) {
 switch (S->getLiteralOperatorKind()) {
-case clang::UserDefinedLiteral::LOK_Integer:
+case UserDefinedLiteral::LOK_Integer:
   return new (allocator()) syntax::IntegerUserDefinedLiteralExpression;
-case clang::UserDefinedLiteral::LOK_Floating:
+case UserDefinedLiteral::LOK_Floating:
   return new (allocator()) syntax::FloatUserDefinedLiteralExpression;
-case clang::UserDefinedLiteral::LOK_Character:
+case UserDefinedLiteral::LOK_Character:
   return new (allocator()) syntax::CharUserDefinedLiteralExpression;
-case clang::UserDefinedLiteral::LOK_String:
+case UserDefinedLiteral::LOK_String:
   return new (allocator()) syntax::StringUserDefinedLiteralExpression;
-case clang::UserDefinedLiteral::LOK_Raw:
-case clang::UserDefinedLiteral::LOK_Template:
+case UserDefinedLiteral::LOK_Raw:
+case UserDefinedLiteral::LOK_Template:
   // For raw literal operator and numeric literal operator template we
   // cannot get the type of the operand in the semantic AST. We get this
   // information from the token. As integer and floating point have the same
Index: clang/include/clang/Tooling/Syntax/Tree.h
===
--- clang/include/clang/Tooling/Syntax/Tree.h
+++ clang/include/clang/Tooling/Syntax/Tree.h
@@ -50,7 +50,7 @@
   /// Add \p Buffer to the underlying source manager, tokenize it and store the
   /// resulting tokens. Useful when there is a need to materialize tokens that
   /// were not written in user code.
-  std::pair>
+  std::pair>
   lexBuffer(std::unique_ptr Buffer);
 
 private:
@@ -58,7 +58,7 @@
   const LangOptions &LangOpts;
   const TokenBuffer &Tokens;
   /// IDs and storage for additional tokenized files.
-  llvm::DenseMap> ExtraTokens;
+  llvm::DenseMap> ExtraTokens;
   /// Keeps all the allocated nodes and their intermediate data structures.
   llvm::BumpPtrAllocator Allocator;
 };
@@ -139,13 +139,13 @@
 /// A leaf node points to a single token inside the expanded token stream.
 class Leaf final : public Node {
 public:
-  Leaf(const syntax::Token *T);
+  Leaf(const Token *T);
   static bool classof(const Node *N);
 
-  const syntax::Token *token() const { return Tok; }
+  const Token *token() const { return Tok; }
 
 private:
-  const syntax::Token *Tok;
+  const Token *Tok;
 };
 
 /// A node that has children and represents a syntactic language construct.
@@ -167,7 +167,7 @@
 
 protected:
   /// Find the first node with a corresponding role.
-  syntax::Node *findChild(NodeRole R);
+  Node *findChild(NodeRole R);
 
 private:
   /// Prepend \p Child to the list of children and and sets the parent pointer.
Index: clang/include/clang/Tooling/Syntax/Nodes.h
===
--- clang/include/clang/Tooling/Syntax/Nodes.h
+++ clang/include/clang/Tooling/Syntax/Nodes.h
@@ -212,7 +212,7 @@
   static bool classof(const Node *N) {
 return N->kind() <= NodeKind::NestedNameSpecifier;
   }
-  std::vector specifiers();
+  std::vector specifiers();
 };
 
 /// Models an `un

[PATCH] D84811: [clangd] Add an option to exclude symbols outside of project root from index

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev requested review of this revision.
kbobyrev added a comment.

Even despite `FileFilter` not being fully implemented yet (an issue for a 
separate patch) I think this change should still be correct and is probably OK 
to land, WDYT @hokein?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84811

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

[PATCH] D85113: [ABI][NFC] Fix the confusion of ByVal and ByRef argument names

[Anatoly Trosinenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5a07490d7611: [ABI][NFC] Fix the confusion of ByVal and 
ByRef argument names (authored by atrosinenko).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85113

Files:
  clang/lib/CodeGen/ABIInfo.h
  clang/lib/CodeGen/TargetInfo.cpp


Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -80,17 +80,17 @@
  T->isMemberFunctionPointerType();
 }
 
-ABIArgInfo
-ABIInfo::getNaturalAlignIndirect(QualType Ty, bool ByRef, bool Realign,
- llvm::Type *Padding) const {
-  return ABIArgInfo::getIndirect(getContext().getTypeAlignInChars(Ty),
- ByRef, Realign, Padding);
+ABIArgInfo ABIInfo::getNaturalAlignIndirect(QualType Ty, bool ByVal,
+bool Realign,
+llvm::Type *Padding) const {
+  return ABIArgInfo::getIndirect(getContext().getTypeAlignInChars(Ty), ByVal,
+ Realign, Padding);
 }
 
 ABIArgInfo
 ABIInfo::getNaturalAlignIndirectInReg(QualType Ty, bool Realign) const {
   return ABIArgInfo::getIndirectInReg(getContext().getTypeAlignInChars(Ty),
-  /*ByRef*/ false, Realign);
+  /*ByVal*/ false, Realign);
 }
 
 Address ABIInfo::EmitMSVAArg(CodeGenFunction &CGF, Address VAListAddr,
@@ -8555,7 +8555,7 @@
 if (RAA == CGCXXABI::RAA_Indirect) {
   return getIndirectResult(Ty, /*ByVal=*/false, State);
 } else if (RAA == CGCXXABI::RAA_DirectInMemory) {
-  return getNaturalAlignIndirect(Ty, /*ByRef=*/true);
+  return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
 }
   }
 
Index: clang/lib/CodeGen/ABIInfo.h
===
--- clang/lib/CodeGen/ABIInfo.h
+++ clang/lib/CodeGen/ABIInfo.h
@@ -111,7 +111,7 @@
 /// A convenience method to return an indirect ABIArgInfo with an
 /// expected alignment equal to the ABI alignment of the given type.
 CodeGen::ABIArgInfo
-getNaturalAlignIndirect(QualType Ty, bool ByRef = true,
+getNaturalAlignIndirect(QualType Ty, bool ByVal = true,
 bool Realign = false,
 llvm::Type *Padding = nullptr) const;
 


Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -80,17 +80,17 @@
  T->isMemberFunctionPointerType();
 }
 
-ABIArgInfo
-ABIInfo::getNaturalAlignIndirect(QualType Ty, bool ByRef, bool Realign,
- llvm::Type *Padding) const {
-  return ABIArgInfo::getIndirect(getContext().getTypeAlignInChars(Ty),
- ByRef, Realign, Padding);
+ABIArgInfo ABIInfo::getNaturalAlignIndirect(QualType Ty, bool ByVal,
+bool Realign,
+llvm::Type *Padding) const {
+  return ABIArgInfo::getIndirect(getContext().getTypeAlignInChars(Ty), ByVal,
+ Realign, Padding);
 }
 
 ABIArgInfo
 ABIInfo::getNaturalAlignIndirectInReg(QualType Ty, bool Realign) const {
   return ABIArgInfo::getIndirectInReg(getContext().getTypeAlignInChars(Ty),
-  /*ByRef*/ false, Realign);
+  /*ByVal*/ false, Realign);
 }
 
 Address ABIInfo::EmitMSVAArg(CodeGenFunction &CGF, Address VAListAddr,
@@ -8555,7 +8555,7 @@
 if (RAA == CGCXXABI::RAA_Indirect) {
   return getIndirectResult(Ty, /*ByVal=*/false, State);
 } else if (RAA == CGCXXABI::RAA_DirectInMemory) {
-  return getNaturalAlignIndirect(Ty, /*ByRef=*/true);
+  return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
 }
   }
 
Index: clang/lib/CodeGen/ABIInfo.h
===
--- clang/lib/CodeGen/ABIInfo.h
+++ clang/lib/CodeGen/ABIInfo.h
@@ -111,7 +111,7 @@
 /// A convenience method to return an indirect ABIArgInfo with an
 /// expected alignment equal to the ABI alignment of the given type.
 CodeGen::ABIArgInfo
-getNaturalAlignIndirect(QualType Ty, bool ByRef = true,
+getNaturalAlignIndirect(QualType Ty, bool ByVal = true,
 bool Realign = false,
 llvm::Type *Padding = nullptr) const;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 5a07490 - [ABI][NFC] Fix the confusion of ByVal and ByRef argument names

[Anatoly Trosinenko via cfe-commits]

Author: Anatoly Trosinenko
Date: 2020-08-06T15:20:18+03:00
New Revision: 5a07490d7611088277dca4c1c06c2002403b7039

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

LOG: [ABI][NFC] Fix the confusion of ByVal and ByRef argument names

The second argument of getNaturalAlignIndirect() was `bool ByRef`, but
the implementation was just delegating to getIndirect() with `ByRef`
passed unchanged to `bool ByVal` parameter of getIndirect().

Fix a couple of /*ByRef=*/ comments as well.

Reviewed By: rjmccall

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

Added: 


Modified: 
clang/lib/CodeGen/ABIInfo.h
clang/lib/CodeGen/TargetInfo.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/ABIInfo.h b/clang/lib/CodeGen/ABIInfo.h
index bb40dace8a84..56f0dd4322d2 100644
--- a/clang/lib/CodeGen/ABIInfo.h
+++ b/clang/lib/CodeGen/ABIInfo.h
@@ -111,7 +111,7 @@ namespace swiftcall {
 /// A convenience method to return an indirect ABIArgInfo with an
 /// expected alignment equal to the ABI alignment of the given type.
 CodeGen::ABIArgInfo
-getNaturalAlignIndirect(QualType Ty, bool ByRef = true,
+getNaturalAlignIndirect(QualType Ty, bool ByVal = true,
 bool Realign = false,
 llvm::Type *Padding = nullptr) const;
 

diff  --git a/clang/lib/CodeGen/TargetInfo.cpp 
b/clang/lib/CodeGen/TargetInfo.cpp
index e011cfa81167..8c89e6bc2a64 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -80,17 +80,17 @@ static bool isAggregateTypeForABI(QualType T) {
  T->isMemberFunctionPointerType();
 }
 
-ABIArgInfo
-ABIInfo::getNaturalAlignIndirect(QualType Ty, bool ByRef, bool Realign,
- llvm::Type *Padding) const {
-  return ABIArgInfo::getIndirect(getContext().getTypeAlignInChars(Ty),
- ByRef, Realign, Padding);
+ABIArgInfo ABIInfo::getNaturalAlignIndirect(QualType Ty, bool ByVal,
+bool Realign,
+llvm::Type *Padding) const {
+  return ABIArgInfo::getIndirect(getContext().getTypeAlignInChars(Ty), ByVal,
+ Realign, Padding);
 }
 
 ABIArgInfo
 ABIInfo::getNaturalAlignIndirectInReg(QualType Ty, bool Realign) const {
   return ABIArgInfo::getIndirectInReg(getContext().getTypeAlignInChars(Ty),
-  /*ByRef*/ false, Realign);
+  /*ByVal*/ false, Realign);
 }
 
 Address ABIInfo::EmitMSVAArg(CodeGenFunction &CGF, Address VAListAddr,
@@ -8555,7 +8555,7 @@ ABIArgInfo LanaiABIInfo::classifyArgumentType(QualType Ty,
 if (RAA == CGCXXABI::RAA_Indirect) {
   return getIndirectResult(Ty, /*ByVal=*/false, State);
 } else if (RAA == CGCXXABI::RAA_DirectInMemory) {
-  return getNaturalAlignIndirect(Ty, /*ByRef=*/true);
+  return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
 }
   }
 



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

[PATCH] D85113: [ABI][NFC] Fix the confusion of ByVal and ByRef argument names

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko added a comment.

@rjmccall Uploaded, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85113

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

[PATCH] D85324: [z/OS] Add z/OS Target and define macros

[Abhina Sreeskantharajan via Phabricator via cfe-commits]

abhina.sreeskantharajan added inline comments.



Comment at: clang/lib/Basic/Targets/OSTargets.h:780
+public:
+  ZOSTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
+  : OSTargetInfo(Triple, Opts) {}

tatyana-krasnukha wrote:
> It is possible to use inheriting constructor here instead (i.e. `using 
> OSTargetInfo::OSTargetInfo;`).
Thanks Tatyana for your review! I think an inheriting constructor is also a 
good solution, but in order to maintain consistency with the other  targets' 
constructors, I would prefer to keep it this way. Please let me know if there 
is a reason for using inherited constructors for this target, I may have missed 
something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85324

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

[clang] 839d974 - [DOCS] Add more detail to stack protector documentation

[Peter Smith via cfe-commits]

Author: Peter Smith
Date: 2020-08-06T13:47:21+01:00
New Revision: 839d974ee0e45f09b9665b4eed734ca1ba174d25

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

LOG: [DOCS] Add more detail to stack protector documentation

The Clang -fstack-protector documentation mentions what functions are considered
vulnerable but does not mention the details of the implementation such as the 
use
of a global variable for the guard value. This brings the documentation more in
line with the GCC documentation at:
https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html
and gives someone using the option more idea about what is protected.

This partly addresses https://bugs.llvm.org/show_bug.cgi?id=42764

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

Added: 


Modified: 
clang/docs/ClangCommandLineReference.rst
clang/include/clang/Driver/Options.td

Removed: 




diff  --git a/clang/docs/ClangCommandLineReference.rst 
b/clang/docs/ClangCommandLineReference.rst
index 8eb010eae265..4caa08a82a72 100644
--- a/clang/docs/ClangCommandLineReference.rst
+++ b/clang/docs/ClangCommandLineReference.rst
@@ -2136,7 +2136,7 @@ Enable stack clash protection
 
 .. option:: -fstack-protector, -fno-stack-protector
 
-Enable stack protectors for some functions vulnerable to stack smashing. This 
uses a loose heuristic which considers functions vulnerable if they contain a 
char (or 8bit integer) array or constant sized calls to alloca, which are of 
greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls 
to alloca are considered vulnerable
+Enable stack protectors for some functions vulnerable to stack smashing. This 
uses a loose heuristic which considers functions vulnerable if they contain a 
char (or 8bit integer) array or constant sized calls to alloca , which are of 
greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls 
to alloca are considered vulnerable. A function witha stack protector has a 
guard value added to the stack frame that is checked on function exit. The 
guard value must be positioned in the stack frame such that a buffer overflow 
from a vulnerable variable will overwrite the guard value before overwriting 
the function's return address. The reference stack guard value is stored in a 
global variable.
 
 .. option:: -fstack-protector-all
 

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 786a3c362842..fc31c23e4240 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1801,10 +1801,15 @@ def fstack_protector_strong : Flag<["-"], 
"fstack-protector-strong">, Group;
 def fstack_protector : Flag<["-"], "fstack-protector">, Group,
   HelpText<"Enable stack protectors for some functions vulnerable to stack 
smashing. "
-   "This uses a loose heuristic which considers functions vulnerable "
-   "if they contain a char (or 8bit integer) array or constant sized 
calls to "
-   "alloca, which are of greater size than ssp-buffer-size (default: 8 
bytes). "
-   "All variable sized calls to alloca are considered vulnerable">;
+   "This uses a loose heuristic which considers functions vulnerable 
if they "
+   "contain a char (or 8bit integer) array or constant sized calls to 
alloca "
+   ", which are of greater size than ssp-buffer-size (default: 8 
bytes). All "
+   "variable sized calls to alloca are considered vulnerable. A 
function with"
+   "a stack protector has a guard value added to the stack frame that 
is "
+   "checked on function exit. The guard value must be positioned in 
the "
+   "stack frame such that a buffer overflow from a vulnerable variable 
will "
+   "overwrite the guard value before overwriting the function's return 
"
+   "address. The reference stack guard value is stored in a global 
variable.">;
 def ftrivial_auto_var_init : Joined<["-"], "ftrivial-auto-var-init=">, 
Group,
   Flags<[CC1Option, CoreOption]>, HelpText<"Initialize trivial automatic stack 
variables: uninitialized (default)"
   " | pattern">, Values<"uninitialized,pattern">;



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

[PATCH] D85239: [DOCS] Add more detail to stack protector documentation

[Peter Smith via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
psmith marked an inline comment as done.
Closed by commit rG839d974ee0e4: [DOCS] Add more detail to stack protector 
documentation (authored by psmith).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85239

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1801,10 +1801,15 @@
"as well as any calls to alloca or the taking of an address from a 
local variable">;
 def fstack_protector : Flag<["-"], "fstack-protector">, Group,
   HelpText<"Enable stack protectors for some functions vulnerable to stack 
smashing. "
-   "This uses a loose heuristic which considers functions vulnerable "
-   "if they contain a char (or 8bit integer) array or constant sized 
calls to "
-   "alloca, which are of greater size than ssp-buffer-size (default: 8 
bytes). "
-   "All variable sized calls to alloca are considered vulnerable">;
+   "This uses a loose heuristic which considers functions vulnerable 
if they "
+   "contain a char (or 8bit integer) array or constant sized calls to 
alloca "
+   ", which are of greater size than ssp-buffer-size (default: 8 
bytes). All "
+   "variable sized calls to alloca are considered vulnerable. A 
function with"
+   "a stack protector has a guard value added to the stack frame that 
is "
+   "checked on function exit. The guard value must be positioned in 
the "
+   "stack frame such that a buffer overflow from a vulnerable variable 
will "
+   "overwrite the guard value before overwriting the function's return 
"
+   "address. The reference stack guard value is stored in a global 
variable.">;
 def ftrivial_auto_var_init : Joined<["-"], "ftrivial-auto-var-init=">, 
Group,
   Flags<[CC1Option, CoreOption]>, HelpText<"Initialize trivial automatic stack 
variables: uninitialized (default)"
   " | pattern">, Values<"uninitialized,pattern">;
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -2136,7 +2136,7 @@
 
 .. option:: -fstack-protector, -fno-stack-protector
 
-Enable stack protectors for some functions vulnerable to stack smashing. This 
uses a loose heuristic which considers functions vulnerable if they contain a 
char (or 8bit integer) array or constant sized calls to alloca, which are of 
greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls 
to alloca are considered vulnerable
+Enable stack protectors for some functions vulnerable to stack smashing. This 
uses a loose heuristic which considers functions vulnerable if they contain a 
char (or 8bit integer) array or constant sized calls to alloca , which are of 
greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls 
to alloca are considered vulnerable. A function witha stack protector has a 
guard value added to the stack frame that is checked on function exit. The 
guard value must be positioned in the stack frame such that a buffer overflow 
from a vulnerable variable will overwrite the guard value before overwriting 
the function's return address. The reference stack guard value is stored in a 
global variable.
 
 .. option:: -fstack-protector-all
 


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1801,10 +1801,15 @@
"as well as any calls to alloca or the taking of an address from a local variable">;
 def fstack_protector : Flag<["-"], "fstack-protector">, Group,
   HelpText<"Enable stack protectors for some functions vulnerable to stack smashing. "
-   "This uses a loose heuristic which considers functions vulnerable "
-   "if they contain a char (or 8bit integer) array or constant sized calls to "
-   "alloca, which are of greater size than ssp-buffer-size (default: 8 bytes). "
-   "All variable sized calls to alloca are considered vulnerable">;
+   "This uses a loose heuristic which considers functions vulnerable if they "
+   "contain a char (or 8bit integer) array or constant sized calls to alloca "
+   ", which are of greater size than ssp-buffer-size (default: 8 bytes). All "
+   "variable sized calls to alloca are considered vulnerable. A function with"
+   "a stack protector has a guard value added to the stack frame that is "
+   "checked 

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

In D85191#2197550 , @bjope wrote:

> In D85191#2196863 , @rsmith wrote:
>
>> In D85191#2195923 , @ebevhan wrote:
>>
>>> In D85191#2193645 , @rsmith wrote:
>>>
> This is not ideal, since it makes the calculations char size dependent, 
> and breaks for sizes that are not a multiple of the char size.

 How can we have a non-bitfield member whose size is not a multiple of the 
 size of a char?
>>>
>>> Downstream, we have fixed-point types that are 24 bits large, but where the 
>>> char size is 16. The type then takes up 2 chars, where 8 of the bits are 
>>> padding. The only way in Clang to express that the width of the bit 
>>> representation of a type should be smaller than the number of chars it 
>>> takes up in memory -- and consequently, produce an `i24` in IR -- is to 
>>> return a non-charsize multiple from getTypeInfo.
>>
>> This violates the C and C++ language rules, which require the size of every 
>> type to be a multiple of the size of char. A type with 24 value bits and 8 
>> padding bits should report a type size of 32 bits, just like `bool` reports 
>> a size of `CHAR_BIT` bits despite having only 1 value bit, and x86_64 `long 
>> double` reports a type size of 128 bits despite having only 80 value bits.
>
> I don't see that it breaks the language rules. The sizeof result for the 24 
> bit type should be 2 in the target described by @ebevhan  (two 16-bit bytes). 
> But I imagine that without this patch it is reported as 24/16=1, right?

Yes, this is what's happening. The sizeof should be reported as 2 (32 bits), 
but isn't, because toCharUnitsFromBits always rounds down.

> So isn't the problem that toCharUnitsFromBits is rounding down when given a 
> bitsize that isn't a multiple of CHAR_BIT? Would it perhaps make sense to let 
> it round up instead?

The issue with toCharUnitsFromBits is that it's an inherently dangerous API. 
There could be cases where you want to round down, and cases where you want to 
round up. The function cannot know.

It could be better if toCharUnitsFromBits took an extra parameter that 
explicitly specifies the rounding, and if that parameter is set to a default 
(for unspecified rounding) and the amount passed is not a multiple of the char 
size, it asserts. This would make a lot of tests fail until all of the uses are 
corrected, though.

In D85191#2196863 , @rsmith wrote:

> In D85191#2195923 , @ebevhan wrote:
>
>> In D85191#2193645 , @rsmith wrote:
>>
 This is not ideal, since it makes the calculations char size dependent, 
 and breaks for sizes that are not a multiple of the char size.
>>>
>>> How can we have a non-bitfield member whose size is not a multiple of the 
>>> size of a char?
>>
>> Downstream, we have fixed-point types that are 24 bits large, but where the 
>> char size is 16. The type then takes up 2 chars, where 8 of the bits are 
>> padding. The only way in Clang to express that the width of the bit 
>> representation of a type should be smaller than the number of chars it takes 
>> up in memory -- and consequently, produce an `i24` in IR -- is to return a 
>> non-charsize multiple from getTypeInfo.
>
> This violates the C and C++ language rules, which require the size of every 
> type to be a multiple of the size of char. A type with 24 value bits and 8 
> padding bits should report a type size of 32 bits, just like `bool` reports a 
> size of `CHAR_BIT` bits despite having only 1 value bit, and x86_64 `long 
> double` reports a type size of 128 bits despite having only 80 value bits.

But this is the crux of the matter; if you aren't allowed to return 
non-char-sizes from getTypeInfo, then there's no way to specify via 
TargetInfo/getTypeInfo that the number of value bits of a type is less than the 
size in chars. That is, that a type is padded. And as your examples show, C 
does not disallow that.

In D85191#2197663 , @efriedma wrote:

>> If the intent is for getTypeInfo to always return sizes that are multiples 
>> of the char size, then the design should be inverted and getTypeInfo should 
>> simply be calling getTypeInfoInChars and multiply that result by the char 
>> size. But that isn't how it works.
>
> The reason it doesn't work this way is just that someone made the wrong 
> choice a decade ago, and nobody has spent the time to rewrite it since.  
> Patch welcome.

This does sound like a good thing to do, but it would be problematic downstream 
since it would completely prohibit the design that we're trying to use.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85191

___

[PATCH] D85429: [OpenCL] Allow for variadic macros in C++ for OpenCL

[Ole Strohm via Phabricator via cfe-commits]

olestrohm created this revision.
olestrohm added a reviewer: Anastasia.
olestrohm added a project: clang.
Herald added subscribers: cfe-commits, yaxunl.
olestrohm requested review of this revision.

C++ for OpenCL intends to support variadic macros, a decision motivated by 
OpenCL 3.0. This patch simply skips the warning when compiling in C++ for 
OpenCL mode.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85429

Files:
  clang/lib/Lex/PPDirectives.cpp


Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2394,7 +2394,7 @@
  diag::ext_variadic_macro);
 
   // OpenCL v1.2 s6.9.e: variadic macros are not supported.
-  if (LangOpts.OpenCL) {
+  if (LangOpts.OpenCL && !LangOpts.OpenCLCPlusPlus) {
 Diag(Tok, diag::ext_pp_opencl_variadic_macros);
   }
 


Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2394,7 +2394,7 @@
  diag::ext_variadic_macro);
 
   // OpenCL v1.2 s6.9.e: variadic macros are not supported.
-  if (LangOpts.OpenCL) {
+  if (LangOpts.OpenCL && !LangOpts.OpenCLCPlusPlus) {
 Diag(Tok, diag::ext_pp_opencl_variadic_macros);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85295: [SyntaxTree] Implement the List construct.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/Tree.h:197
+  MaybeTerminated,
+  Separated,
+};

eduucaldas wrote:
> gribozavr2 wrote:
> > Add a "WithoutDelimiters" case as well?
> I think we might want to treat non-delimited-list in another way. as many of 
> the member functions stop making sense. 
> `getElementsAsNodesAndDelimiters` 
> `getDelimiterTokenKind`
> `getTerminationKind`
OK, I'm convinced.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85295

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

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

I don't know whether the name of your downstream target is a secret. Wouldn't 
it help you to add a fake 16bit per char target and add units tests to prevent 
regressions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85191

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

[PATCH] D85424: [Analyzer] Crash fix for alpha.cplusplus.IteratorRange

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

Szelethus added a comment.

Yeah, this looks straightforward, but how come you didn't manage to get a small 
test case? creduce gave up?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85424

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

[PATCH] D85431: [analyzer] Implement a new checker ThreadPrimitivesChecker

[Denys Petrov via Phabricator via cfe-commits]

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

This checker finds STL thread primitives misuse.

- std::mutex::unlock without std::mutex::lock
- std::mutex::lock twice

In future:

- std::mutex::lock without std::mutex::unlock (discuss)
- std::recursive_mutex support

P.S. This is an alpha version of my first checker. Do not hesitate to express 
your complaints!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85431

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/ThreadPrimitivesChecker.cpp
  clang/test/Analysis/Checkers/ThreadPrimitivesChecker.cpp

Index: clang/test/Analysis/Checkers/ThreadPrimitivesChecker.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/ThreadPrimitivesChecker.cpp
@@ -0,0 +1,48 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.ThreadPrimitives -verify %s
+
+namespace std {
+struct mutex {
+  void lock();
+  void unlock();
+};
+template 
+struct guard_lock {
+  T &t;
+  guard_lock(T &m) : t(m) {
+t.lock();
+  }
+  ~guard_lock() {
+t.unlock();
+  }
+};
+} // namespace std
+
+void correct_lock_unlock(std::mutex m) {
+  m.lock();
+  m.unlock();
+}
+
+void incorrect_lock_unlock(std::mutex m1, std::mutex m2) {
+  m1.lock();
+  m2.unlock(); // expected-warning{{TRUE}}
+}
+
+void incorrect_lock_unlock2(std::mutex m, bool b) {
+  m.lock();
+  if(b)
+m.unlock();
+}
+
+void unlock_without_lock(std::mutex m) {
+  m.unlock(); // expected-warning{{TRUE}}
+}
+
+void twice_lock(std::mutex m) {
+  m.lock();
+  m.lock(); // expected-warning{{TRUE}}
+}
+
+void twice_lock2(std::mutex m) {
+  while (true)
+m.lock(); // expected-warning{{TRUE}}
+}
\ No newline at end of file
Index: clang/lib/StaticAnalyzer/Checkers/ThreadPrimitivesChecker.cpp
===
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/ThreadPrimitivesChecker.cpp
@@ -0,0 +1,118 @@
+//=== ConversionChecker.cpp -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This checker finds STL thread primitives misuse.
+// - std::mutex::unlock without std::mutex::lock
+// - std::mutex::lock twice
+//
+//===--===//
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/AST/StmtVisitor.h"
+#include "clang/AST/ParentMap.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "llvm/ADT/APFloat.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class ThreadPrimitivesChecker
+: public Checker {
+public:
+  ThreadPrimitivesChecker();
+
+  void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
+private:
+  mutable std::unique_ptr BT;
+  CallDescription LockFunc, UnlockFunc;
+
+  std::pair FindMutexLockOrUnlock(const CallEvent &Call,
+  CheckerContext &C) const;
+  void reportBug(CheckerContext &C, const char Msg[]) const;
+};
+
+} // namespace
+
+REGISTER_SET_WITH_PROGRAMSTATE(LockedMutexes, SVal)
+
+ThreadPrimitivesChecker::ThreadPrimitivesChecker()
+: LockFunc({"std","mutex","lock"}, 0, 0), UnlockFunc({"std","mutex","unlock"}, 0, 0) {}
+
+
+std::pair ThreadPrimitivesChecker::FindMutexLockOrUnlock(
+const CallEvent &Call, CheckerContext &C) const {
+
+  if (const auto *MCall = dyn_cast(&Call)) {
+const bool IsLockFunc = MCall->isCalled(LockFunc);
+const bool IsUnlockFunc = IsLockFunc ? false : MCall->isCalled(UnlockFunc);
+return {IsLockFunc, IsUnlockFunc};
+  }
+  return {false, false};
+}
+
+void ThreadPrimitivesChecker::checkPostCall(const CallEvent &Call,
+CheckerContext &C) const {
+  // Find mutex::lock or mutex::unlock functions.
+  bool IsLockFunc;
+  bool IsUnlockFunc;
+  std::tie(IsLockFunc, IsUnlockFunc) = FindMutexLockOrUnloc

[PATCH] D85276: [PGO][CUDA][HIP] Skip generating profile on the device stub and wrong-side functions.

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

Do we need to disable pgo and coverage mapping for device compilation? Or it is 
already disabled?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85276

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

[PATCH] D67253: clang-misexpect: a standalone tool for verifying the use of __builtin_expect with PGO data

[Roman Lebedev via Phabricator via cfe-commits]

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

Abandon this? D66324  landed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67253

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

[PATCH] D85417: [clangd] Fix crash in bugprone-bad-signal-to-kill-thread clang-tidy check.

[Haojian Wu via Phabricator via cfe-commits]

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

Thanks, this is a nice catch.

Looks like NotNullTerminatedResultCheck.cpp also has this pattern, we may want 
to fix that as well.




Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:441
 
+TEST(DiagnosticTest, ClangTidyBadSignalToKillThread) {
+  Annotations Main(R"cpp(

ClangTidyBadSignalToKillThread doesn't seen ti provide much information about 
what is testcase testing. Maybe `NoLiteralDataInMacroToken`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85417

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

[PATCH] D85431: [analyzer] Implement a new checker ThreadPrimitivesChecker

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

It is an interesting checker, but it seems that this kind of checker is 
extremely hard in single TU/top-down analysis.  It feels like it's going to 
produce hell a lot of false positives in the wild.
Also `mutex` is usually a global variable - the nemesis of any static analysis 
tool.

In general, it would be great to have checkers for multi-threaded programs.  
However, it seems like we would need some sort of conventions that we enforce.  
For example, every function containing a `lock` for a mutex should also have 
the `unlock`.  And the checker would verify that the user follows the 
convention rather than has correct code.  The convention, of course, should be 
designed so it implies correctness.  This is the model we have with 
`RetainCountChecker`.




Comment at: clang/lib/StaticAnalyzer/Checkers/ThreadPrimitivesChecker.cpp:36
+private:
+  mutable std::unique_ptr BT;
+  CallDescription LockFunc, UnlockFunc;

These days we simply do `BugType` without `unique_ptr`s



Comment at: clang/lib/StaticAnalyzer/Checkers/ThreadPrimitivesChecker.cpp:46
+
+REGISTER_SET_WITH_PROGRAMSTATE(LockedMutexes, SVal)
+

You should also cleanup and remove dead symbols from the set.



Comment at: clang/lib/StaticAnalyzer/Checkers/ThreadPrimitivesChecker.cpp:46
+
+REGISTER_SET_WITH_PROGRAMSTATE(LockedMutexes, SVal)
+

vsavchenko wrote:
> You should also cleanup and remove dead symbols from the set.
Maybe `SymbolRef` is more suitable here?



Comment at: clang/lib/StaticAnalyzer/Checkers/ThreadPrimitivesChecker.cpp:49
+ThreadPrimitivesChecker::ThreadPrimitivesChecker()
+: LockFunc({"std","mutex","lock"}, 0, 0), 
UnlockFunc({"std","mutex","unlock"}, 0, 0) {}
+

Does it conform with `clang-format`?



Comment at: clang/lib/StaticAnalyzer/Checkers/ThreadPrimitivesChecker.cpp:52
+
+std::pair ThreadPrimitivesChecker::FindMutexLockOrUnlock(
+const CallEvent &Call, CheckerContext &C) const {

I think this is better expressed with `enum` because this contract implies only 
3 possible values.  I prefer to bake such contracts into the types of the 
actual solution so that the code is more robust.



Comment at: clang/lib/StaticAnalyzer/Checkers/ThreadPrimitivesChecker.cpp:64
+void ThreadPrimitivesChecker::checkPostCall(const CallEvent &Call,
+CheckerContext &C) const {
+  // Find mutex::lock or mutex::unlock functions.

Looks overindented



Comment at: clang/lib/StaticAnalyzer/Checkers/ThreadPrimitivesChecker.cpp:74
+  // We are sure about cast here, because mutex::lock/unlock met before.
+  const auto *MCall = dyn_cast(&Call);
+  assert(MCall);

If we are sure then why are we using `dyn_cast` instead of `cast`?



Comment at: clang/lib/StaticAnalyzer/Checkers/ThreadPrimitivesChecker.cpp:97
+void ThreadPrimitivesChecker::reportBug(CheckerContext &C,
+  const char Msg[]) const {
+  ExplodedNode *ErrNode = C.generateNonFatalErrorNode();

Looks under indented



Comment at: clang/test/Analysis/Checkers/ThreadPrimitivesChecker.cpp:27
+  m1.lock();
+  m2.unlock(); // expected-warning{{TRUE}}
+}

I think we should also add a TODO for having a warning for no `m1.unlock()` 



Comment at: clang/test/Analysis/Checkers/ThreadPrimitivesChecker.cpp:49
+}
\ No newline at end of file


😱😱😱


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85431

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

[PATCH] D84932: [builtins] Add more test cases for __div[sdt]f3 LibCalls

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84932

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

[PATCH] D85417: [clangd] Fix crash in bugprone-bad-signal-to-kill-thread clang-tidy check.

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX updated this revision to Diff 283600.
ArcsinX added a comment.

Test rename: ClangTidyBadSignalToKillThread => 
ClangTidyNoLiteralDataInMacroToken


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85417

Files:
  clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -438,6 +438,21 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
+TEST(DiagnosticTest, ClangTidyNoLiteralDataInMacroToken) {
+  Annotations Main(R"cpp(
+#define SIGTERM 15
+using pthread_t = int;
+int pthread_kill(pthread_t thread, int sig);
+int func() {
+  pthread_t thread;
+  return pthread_kill(thread, 0);
+}
+  )cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.ClangTidyChecks = "-*,bugprone-bad-signal-to-kill-thread";
+  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash
+}
+
 TEST(DiagnosticsTest, Preprocessor) {
   // This looks like a preamble, but there's an #else in the middle!
   // Check that:
Index: clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
@@ -39,7 +39,7 @@
   return llvm::None;
 const MacroInfo *MI = PP->getMacroInfo(It->first);
 const Token &T = MI->tokens().back();
-if (!T.isLiteral())
+if (!T.isLiteral() || !T.getLiteralData())
   return llvm::None;
 StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
 


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -438,6 +438,21 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
+TEST(DiagnosticTest, ClangTidyNoLiteralDataInMacroToken) {
+  Annotations Main(R"cpp(
+#define SIGTERM 15
+using pthread_t = int;
+int pthread_kill(pthread_t thread, int sig);
+int func() {
+  pthread_t thread;
+  return pthread_kill(thread, 0);
+}
+  )cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.ClangTidyChecks = "-*,bugprone-bad-signal-to-kill-thread";
+  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash
+}
+
 TEST(DiagnosticsTest, Preprocessor) {
   // This looks like a preamble, but there's an #else in the middle!
   // Check that:
Index: clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
@@ -39,7 +39,7 @@
   return llvm::None;
 const MacroInfo *MI = PP->getMacroInfo(It->first);
 const Token &T = MI->tokens().back();
-if (!T.isLiteral())
+if (!T.isLiteral() || !T.getLiteralData())
   return llvm::None;
 StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85417: [clangd] Fix crash in bugprone-bad-signal-to-kill-thread clang-tidy check.

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX added inline comments.



Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:441
 
+TEST(DiagnosticTest, ClangTidyBadSignalToKillThread) {
+  Annotations Main(R"cpp(

hokein wrote:
> ClangTidyBadSignalToKillThread doesn't seen ti provide much information about 
> what is testcase testing. Maybe `NoLiteralDataInMacroToken`?
Thanks, renamed. But I want to keep ClangTidy prefix because the problem 
related with clang-tidy limitations inside clangd.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85417

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

[PATCH] D84534: [AIX] Static init frontend recovery and backend support

[Jason Liu via Phabricator via cfe-commits]

jasonliu added inline comments.



Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:391
+  /// @param[out] Structors Sorted Structor structs by Priority.
+  /// @return false if List is not an array of '{ i32, void ()*, i8* }' 
structs.
+  bool preprocessXXStructorList(const DataLayout &DL, const Constant *List,

This description is not entirely true. We only see if the array is a 
ConstantArray and returning false. We are not returning false if the array's 
element is not `{ i32, void ()*, i8* }`.



Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:466
+
+  bool preprocessStructorList(const DataLayout &DL, const Constant *List,
+  SmallVector &Structors);

Xiangling_L wrote:
> jasonliu wrote:
> > A doxygen comment describe what this function does, and what its return 
> > value means, and mention `Structors` is an output argument.
> > By looking at what this function does, it seems `buildStructorList` is a 
> > better name.
> I meant to and should've named this function to `preprocessXXStructorList`, 
> let me know if you still prefer `buildStructorList`. And if you do, since the 
> underneath of `SmallVector` is a variable-sized array, maybe we should try 
> `buildSortedStructorArray`?
`preprocess` sounds like we are already having a XXStructorList and now we try 
to do something on it. 
But right now, we are actually passing in an empty StructorList/Array and build 
it from scratch. So I would still prefer the name of `build` in it.
I don't mind changing to a more accurate name as you suggested. 



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2107
+  if (!isa(List))
+return false;
 

Return of boolean seems unnecessary. 
Callee could check the size of the Structors to decide if they want an early 
return or not (or in this particular case, the for loop would just do nothing 
and no need for extra condition if you don't mind the call to 
getPointerPrefAlignment or assign to 0 to Index)?
So we could just return void for this function?



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2129
   }
-
   // Emit the function pointers in the target-specific order
   llvm::stable_sort(Structors, [](const Structor &L, const Structor &R) {

nit: Missing blank line.



Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1966
+
+Index++;
+  }

nit: Use `++Index` instead. 
https://llvm.org/docs/CodingStandards.html#prefer-preincrement

Or use `Index++` at line 1963 so that we don't need this line.


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

https://reviews.llvm.org/D84534

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

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

[Nithin VR via Phabricator via cfe-commits]

vrnithinkumar updated this revision to Diff 283602.
vrnithinkumar marked 13 inline comments as done.
vrnithinkumar added a comment.



- Changes from review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84600

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

Index: clang/test/Analysis/smart-ptr.cpp
===
--- clang/test/Analysis/smart-ptr.cpp
+++ clang/test/Analysis/smart-ptr.cpp
@@ -12,7 +12,7 @@
   std::unique_ptr Q = std::move(P);
   if (Q)
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
-  *Q.get() = 1; // no-warning
+  *Q.get() = 1; // no-warning
   if (P)
 clang_analyzer_warnIfReached(); // no-warning
   // TODO: Report a null dereference (instead).
@@ -50,37 +50,38 @@
 
 void derefAfterDefaultCtr() {
   std::unique_ptr P;
-  P->foo(); // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
 
 void derefAfterCtrWithNull() {
   std::unique_ptr P(nullptr);
-  *P; // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+  *P; // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
 
-void derefAfterCtrWithNullReturnMethod() {
-  std::unique_ptr P(return_null());
-  P->foo(); // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+void derefAfterCtrWithNullVariable() {
+  A *InnerPtr = nullptr;
+  std::unique_ptr P(InnerPtr);
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
 
 void derefAfterRelease() {
   std::unique_ptr P(new A());
   P.release();
   clang_analyzer_numTimesReached(); // expected-warning {{1}}
-  P->foo(); // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
 
 void derefAfterReset() {
   std::unique_ptr P(new A());
   P.reset();
   clang_analyzer_numTimesReached(); // expected-warning {{1}}
-  P->foo(); // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
 
 void derefAfterResetWithNull() {
   std::unique_ptr P(new A());
   P.reset(nullptr);
-  P->foo(); // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
 
 void derefAfterResetWithNonNull() {
@@ -102,6 +103,12 @@
   AP->foo(); // expected-warning {{Called C++ object pointer is null [core.CallAndMessage]}}
 }
 
+void derefOnReleasedValidRawPtr() {
+  std::unique_ptr P(new A());
+  A *AP = P.release();
+  AP->foo(); // No warning.
+}
+
 void pass_smart_ptr_by_ref(std::unique_ptr &a);
 void pass_smart_ptr_by_const_ref(const std::unique_ptr &a);
 void pass_smart_ptr_by_rvalue_ref(std::unique_ptr &&a);
@@ -118,7 +125,7 @@
   {
 std::unique_ptr P;
 pass_smart_ptr_by_const_ref(P);
-P->foo(); // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
   }
   {
 std::unique_ptr P;
@@ -128,7 +135,7 @@
   {
 std::unique_ptr P;
 pass_smart_ptr_by_const_rvalue_ref(std::move(P));
-P->foo(); // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
   }
   {
 std::unique_ptr P;
@@ -138,7 +145,7 @@
   {
 std::unique_ptr P;
 pass_smart_ptr_by_const_ptr(&P);
-P->foo(); // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
   }
 }
 
@@ -162,7 +169,7 @@
   {
 StructWithSmartPtr S;
 pass_struct_with_smart_ptr_by_const_ref(S);
-S.P->foo(); // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
   }
   {
 StructWithSmartPtr S;
@@ -172,7 +179,7 @@
   {
 StructWithSmartPtr S;
 pass_struct_with_smart_ptr_by_const_rvalue_ref(std::move(S));
-S.P->foo(); // expected-wa

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

[Nithin VR via Phabricator via cfe-commits]

vrnithinkumar added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:312
+  const NoteTag *getNoteTag(
+  std::function 
Cb,
+  bool IsPrunable = false) {

xazax.hun wrote:
> The callback is taken is an rvalue reference in other `getNoteTag` APIs. I 
> think these overloads should be consistent.
> Also, I wonder if the caller should be able to manipulate the buffer size of 
> the small string (as a template parameter), but I do not have strong feelings 
> about this. 
> 
> As a side note, since Clang is using C++14, maybe the lambda captures in the 
> `getNoteTag` overloads above should utilize it and capture the callback by 
> move. This is more of a note to ourselves independent of this patch. 
> 
> Side note 2: maybe a modernize tidy check would be useful to discover where 
> rvalue references are captured by value in lambdas?
Changed to rvalue reference.



Comment at: clang/lib/StaticAnalyzer/Checkers/NullDereference.h:21
+namespace ento {
+namespace nullDereference {
+

NoQ wrote:
> Namespaces are traditionally snake_case rather than camelCase.
removed the new namespace



Comment at: clang/lib/StaticAnalyzer/Checkers/NullDereference.h:23
+
+/// Returns NullDereferenceBugType.
+const BugType *getNullDereferenceBugType();

xazax.hun wrote:
> I think this comment does not really add much value.
Yeah.
removed it.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp:110
+  SmartPtrChecker *Checker = Mgr.registerChecker();
+  NullDereferenceBugTypePtr = &Checker->NullDereferenceBugType;
 }

NoQ wrote:
> vrnithinkumar wrote:
> > xazax.hun wrote:
> > > NoQ wrote:
> > > > vrnithinkumar wrote:
> > > > > NoQ wrote:
> > > > > > Wait, i don't understand again. You're taking the bug type from 
> > > > > > that checker and using it in that checker. Why did you need to make 
> > > > > > it global? I thought your problem was about capturing the bug type 
> > > > > > from the //raw// null dereference checker?
> > > > > > Wait, i don't understand again. You're taking the bug type from 
> > > > > > that checker and using it in that checker. Why did you need to make 
> > > > > > it global? I thought your problem was about capturing the bug type 
> > > > > > from the //raw// null dereference checker?
> > > > > 
> > > > > The check for bug type is in `SmartPtrModeling` but the bug type is 
> > > > > defined in `SmartPtrChecker`
> > > > Aha, ok, i misunderstood again. So i guess we didn't need a new header 
> > > > then. That said, it's an interesting layering violation that we 
> > > > encounter in this design: it used to be up to the checker to attach a 
> > > > visitor and so should be activating a note tag, but note tags are part 
> > > > of modeling, not checking.
> > > > 
> > > > I guess that's just how it is and it's the responsibility of the 
> > > > checkers to inform the modeling about bug types. I guess the ultimate 
> > > > API could look like `BugReport->activateNoteTag(NoteTagTag TT)` (where 
> > > > `TT` is a crying smiley that cries about "tag tags" T_T). But that's a 
> > > > story for another time.
> > > I hope we will be able to figure out a nicer solution at some point. But 
> > > I do not have a better alternative now, so I am ok with committing as is.
> > > 
> > > The API Artem is suggesting looks good to me (although it is out of scope 
> > > for the GSoC). But I think that might not solve the problem of how 
> > > multiple checkers should share the information about those tags. (Or at 
> > > least I do not see how.)
> > > 
> > > Note that if we had both checks in the same file we might be able to work 
> > > this problem around using `CheckerManager::getChecker`. I am not 
> > > suggesting merging them, only wanted to point out.
> > >  So i guess we didn't need a new header then.
> > So we should remove the `NullDereference.h` and add the inter checker api 
> > to get the BugType to `SmartPtr.h`?
> > 
> > >  I guess the ultimate API could look like 
> > > `BugReport->activateNoteTag(NoteTagTag TT)`
> > Do we have to make these api changes on this review? Or some follow up 
> > changes?
> > So we should remove the `NullDereference.h` and add the inter checker api 
> > to get the BugType to `SmartPtr.h`?
> Yes.
> 
> > Do we have to make these api changes on this review? Or some follow up 
> > changes?
> You don't have to do this at all; i think you have enough stuff on your 
> plate. But if you like you can put up another patch later.
Removed the new header file and added to `SmartPtr.h`



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:131
+  }
+  return nullptr;
+}

NoQ wrote:
> You never ever check for this case. Therefore this function entirely boils 
> down to `Call.getArgExpr(0)` which is shorter than `getFirstArgExpr(Call)` 
> anyway.
remov

[PATCH] D85315: [AIX][Clang][Driver] Generate reference to the C++ library on the link step

[Shuhong Liu via Phabricator via cfe-commits]

ShuhongL updated this revision to Diff 283607.
ShuhongL added a comment.

Added testcases for lc and lc++


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85315

Files:
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/lib/Driver/ToolChains/AIX.h
  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
@@ -192,3 +192,131 @@
 // CHECK-LD32-CXX-ARG-ORDER: "-bcdtors:all:0:s"
 // CHECK-LD32-CXX-ARG-ORDER: "-bnocdtors"
 // CHECK-LD32-CXX-ARG-ORDER-NOT: "-bcdtors:all:0:s"
+// CHECK-LD32-CXX-ARG-ORDER: "-lc++"
+// CHECK-LD32-CXX-ARG-ORDER: "-lc"
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. lc++ and lc order.
+// RUN: %clangxx -no-canonical-prefixes %s 2>&1 -### \
+// RUN:  -target powerpc-ibm-aix7.1.0.0 \
+// RUN:  --sysroot %S/Inputs/aix_ppc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD32-CXX-ARG-LCXX %s
+// CHECK-LD32-CXX-ARG-LCXX: {{.*}}clang{{.*}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-LD32-CXX-ARG-LCXX: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD32-CXX-ARG-LCXX: "{{.*}}ld{{(.exe)?}}"
+// CHECK-LD32-CXX-ARG-LCXX: "-b32"
+// CHECK-LD32-CXX-ARG-LCXX: "-bpT:0x1000" "-bpD:0x2000"
+// CHECK-LD32-CXX-ARG-LCXX: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-LD32-CXX-ARG-LCXX: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD32-CXX-ARG-LCXX: "-lc++"
+// CHECK-LD32-CXX-ARG-LCXX: "-lc"
+
+// Check powerpc64-ibm-aix7.1.0.0, 64-bit. lc++ and lc order.
+// RUN: %clangxx -no-canonical-prefixes %s 2>&1 -### \
+// RUN:  -target powerpc64-ibm-aix7.1.0.0 \
+// RUN:  --sysroot %S/Inputs/aix_ppc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD64-CXX-ARG-LCXX %s
+// CHECK-LD64-CXX-ARG-LCXX: {{.*}}clang{{.*}}" "-cc1" "-triple" "powerpc64-ibm-aix7.1.0.0"
+// CHECK-LD64-CXX-ARG-LCXX: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD64-CXX-ARG-LCXX: "{{.*}}ld{{(.exe)?}}"
+// CHECK-LD64-CXX-ARG-LCXX: "-b64"
+// CHECK-LD64-CXX-ARG-LCXX: "-bpT:0x1" "-bpD:0x11000"
+// CHECK-LD64-CXX-ARG-LCXX: "[[SYSROOT]]/usr/lib{{/|}}crt0_64.o"
+// CHECK-LD64-CXX-ARG-LCXX: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD64-CXX-ARG-LCXX: "-lc++"
+// CHECK-LD64-CXX-ARG-LCXX: "-lc"
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. -nodefaultlibs.
+// RUN: %clangxx -no-canonical-prefixes %s 2>&1 -### \
+// RUN:  -nodefaultlibs \
+// RUN:  -target powerpc-ibm-aix7.1.0.0 \
+// RUN:  --sysroot %S/Inputs/aix_ppc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD32-NODEFLIB-LCXX %s
+// CHECK-LD32-NODEFLIB-LCXX: {{.*}}clang{{.*}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-LD32-NODEFLIB-LCXX: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD32-NODEFLIB-LCXX: "{{.*}}ld{{(.exe)?}}"
+// CHECK-LD32-NODEFLIB-LCXX: "-b32"
+// CHECK-LD32-NODEFLIB-LCXX: "-bpT:0x1000" "-bpD:0x2000"
+// CHECK-LD32-NODEFLIB-LCXX: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-LD32-NODEFLIB-LCXX: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD32-NODEFLIB-LCXX-NOT: "-lc++"
+// CHECK-LD32-NODEFLIB-LCXX-NOT: "-lc"
+
+// Check powerpc64-ibm-aix7.1.0.0, 64-bit. -nodefaultlibs.
+// RUN: %clangxx -no-canonical-prefixes %s 2>&1 -### \
+// RUN:  -nodefaultlibs \
+// RUN:  -target powerpc64-ibm-aix7.1.0.0 \
+// RUN:  --sysroot %S/Inputs/aix_ppc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD64-NODEFLIB-LCXX %s
+// CHECK-LD64-NODEFLIB-LCXX: {{.*}}clang{{.*}}" "-cc1" "-triple" "powerpc64-ibm-aix7.1.0.0"
+// CHECK-LD64-NODEFLIB-LCXX: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD64-NODEFLIB-LCXX: "{{.*}}ld{{(.exe)?}}"
+// CHECK-LD64-NODEFLIB-LCXX: "-b64"
+// CHECK-LD64-NODEFLIB-LCXX: "-bpT:0x1" "-bpD:0x11000"
+// CHECK-LD64-NODEFLIB-LCXX: "[[SYSROOT]]/usr/lib{{/|}}crt0_64.o"
+// CHECK-LD64-NODEFLIB-LCXX: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD64-NODEFLIB-LCXX-NOT: "-lc++"
+// CHECK-LD64-NODEFLIB-LCXX-NOT: "-lc"
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. -nostdlib.
+// RUN: %clangxx -no-canonical-prefixes %s 2>&1 -### \
+// RUN:  -nostdlib \
+// RUN:  -target powerpc-ibm-aix7.1.0.0 \
+// RUN:  --sysroot %S/Inputs/aix_ppc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD32-NOSTDLIB-LCXX %s
+// CHECK-LD32-NOSTDLIB-LCXX: {{.*}}clang{{.*}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-LD32-NOSTDLIB-LCXX: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD32-NOSTDLIB-LCXX: "{{.*}}ld{{(.exe)?}}"
+// CHECK-LD32-NOSTDLIB-LCXX: "-b32"
+// CHECK-LD32-NOSTDLIB-LCXX: "-bpT:0x1000" "-bpD:0x2000"
+// CHECK-LD32-NOSTDLIB-LCXX-NOT: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-LD32-NOSTDLIB-LCXX: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD32-NOSTDLIB-LCXX-NOT: "-lc++"
+// CHECK-LD32-NOSTDLIB-LCXX

[PATCH] D84811: [clangd] Add an option to exclude symbols outside of project root from index

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D84811#2199510 , @kbobyrev wrote:

> Even despite `FileFilter` not being fully implemented yet (an issue for a 
> separate patch) I think this change should still be correct and is probably 
> OK to land, WDYT @hokein?

Yes, personally no objection on landing this, but landing it doesn't seem to 
help much for your remote-index case (STL symbols are not filtered out)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84811

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

[PATCH] D84811: [clangd] Add an option to exclude symbols outside of project root from index

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

In D84811#2199820 , @hokein wrote:

> In D84811#2199510 , @kbobyrev wrote:
>
>> Even despite `FileFilter` not being fully implemented yet (an issue for a 
>> separate patch) I think this change should still be correct and is probably 
>> OK to land, WDYT @hokein?
>
> Yes, personally no objection on landing this, but landing it doesn't seem to 
> help much for your remote-index case (STL symbols are not filtered out)?

True, it doesn't filter _all_ of them, but it partially filters some symbols 
which is still a win I guess :) My rationale is probably that having this 
blocked on more implementation details would be more reasonable if there was no 
implementation at all but since it still filters out some symbols this should 
probably be fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84811

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

[PATCH] D85426: [clangd] Implement FileFilter for the indexer

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:643
   assert(Loc.isValid() && "Invalid source location for NamedDecl");
-  // FIXME: use the result to filter out symbols.
-  shouldIndexFile(SM.getFileID(Loc));
+  if (!shouldIndexFile(SM.getFileID(Loc)))
+return nullptr;

A drive-by comment from D84811: the file granularity vs symbol granularity is 
tricky here.

Note that a *full* symbol (with declaration, definition, etc) may be formed 
from different files (.h, .cc), thinking of a following case:

```
// foo.h
void func();

// user.cc
#include "foo.h"

// foo.cc
#include "foo.h"
void func() {}
```

if our indexer indexes `user.cc` first, then `foo.h` is considered indexed, 
later when indexing `foo.cc`, we will skip the `func` symbol. so the symbol 
`foo` will not have definition.
 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85426

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

[PATCH] D85417: [clangd] Fix crash in bugprone-bad-signal-to-kill-thread clang-tidy check.

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX added a comment.

In D85417#2199694 , @hokein wrote:

> Looks like NotNullTerminatedResultCheck.cpp also has this pattern, we may 
> want to fix that as well.

Yes, you are right. I will fix this in the next patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85417

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

[PATCH] D85285: [clangd] WIP experimentation with finding static grpc++ libraries

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev planned changes to this revision.
kbobyrev added a comment.

WIP, nothing important here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85285

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

[PATCH] D85426: [clangd] Implement FileFilter for the indexer

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:643
   assert(Loc.isValid() && "Invalid source location for NamedDecl");
-  // FIXME: use the result to filter out symbols.
-  shouldIndexFile(SM.getFileID(Loc));
+  if (!shouldIndexFile(SM.getFileID(Loc)))
+return nullptr;

hokein wrote:
> A drive-by comment from D84811: the file granularity vs symbol granularity is 
> tricky here.
> 
> Note that a *full* symbol (with declaration, definition, etc) may be formed 
> from different files (.h, .cc), thinking of a following case:
> 
> ```
> // foo.h
> void func();
> 
> // user.cc
> #include "foo.h"
> 
> // foo.cc
> #include "foo.h"
> void func() {}
> ```
> 
> if our indexer indexes `user.cc` first, then `foo.h` is considered indexed, 
> later when indexing `foo.cc`, we will skip the `func` symbol. so the symbol 
> `foo` will not have definition.
>  
> 
Oh, you're right, good catch! That's why the post-filtering would probably work 
but maybe won't be as fancy :(

Thank you for mentioning it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85426

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

[PATCH] D82502: [PowerPC] Implement Load VSX Vector and Sign Extend and Zero Extend

[Amy Kwan via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGen/builtins-ppc-p10vector.c:703
+vector signed __int128 test_vec_xl_sext_i8(void) {
+  // CHECK: load i8
+  // CHECK: sext i8

It would be good to be a little more specific with the CHECK lines. 



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:13500
   SDValue Ext2 = N->getOperand(1).getOperand(0);
-  if(Ext1.getOpcode() != ISD::EXTRACT_VECTOR_ELT ||
- Ext2.getOpcode() != ISD::EXTRACT_VECTOR_ELT)
+  if (Ext1.getOpcode() != ISD::EXTRACT_VECTOR_ELT ||
+  Ext2.getOpcode() != ISD::EXTRACT_VECTOR_ELT)

This line here and below looks like unintended changes from clang-format I am 
guessing? I am OK with them being removed during commit. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82502

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

[PATCH] D85429: [OpenCL] Allow for variadic macros in C++ for OpenCL

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

Can you extend test test/Preprocessor/macro_variadic.cl to cover C++ for OpenCL 
mode, otherwise LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85429

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

[PATCH] D84811: [clangd] Add an option to exclude symbols outside of project root from index

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D84811#2199829 , @kbobyrev wrote:

> In D84811#2199820 , @hokein wrote:
>
>> In D84811#2199510 , @kbobyrev wrote:
>>
>>> Even despite `FileFilter` not being fully implemented yet (an issue for a 
>>> separate patch) I think this change should still be correct and is probably 
>>> OK to land, WDYT @hokein?
>>
>> Yes, personally no objection on landing this, but landing it doesn't seem to 
>> help much for your remote-index case (STL symbols are not filtered out)?
>
> True, it doesn't filter _all_ of them, but it partially filters some symbols 
> which is still a win I guess :) My rationale is probably that having this 
> blocked on more implementation details would be more reasonable if there was 
> no implementation at all but since it still filters out some symbols this 
> should probably be fine.

It just reduces symptoms, not solving the problem. I think remote-index is also 
blocked the FileFiltering stuff (we can't launch remote-index without fixing 
this issue entirely).

As you may have noticed implementing FileFiltering is tricky, I think the 
indexer here is a good opportunity to test/verify your implementation 
(comparing the index data before vs after), so my preference would be to 
implement the FileFilter, test it with this patch together, then finally land 
this patch after making sure everything works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84811

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

[PATCH] D84348: WIP: Add complete id-expression support to syntax trees

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 283622.
eduucaldas added a comment.

- Update comments to reflect change in API.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84348

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -874,24 +874,47 @@
   }
   EXPECT_TRUE(treeDumpEqual(
   R"cpp(
-namespace a {
+namespace n {
   struct S {
 template
-static T f(){}
+struct ST {
+  static void f();
+};
   };
 }
+template
+struct ST {
+  struct S {
+template
+static U f();
+  };
+};
 void test() {
-  ::  // global-namespace-specifier
-  a:: // namespace-specifier
-  S:: // type-name-specifier
+  :: // global-namespace-specifier
+  n::// namespace-specifier
+  S::// type-name-specifier
+  template ST:: // type-template-instantiation-specifier
+  f();
+
+  n::// namespace-specifier
+  S::// type-name-specifier
+  ST::  // type-template-instantiation-specifier
+  f();
+
+  ST:: // type-name-specifier
+  S::   // type-name-specifier
   f();
+
+  ST:: // type-name-specifier
+  S::   // type-name-specifier
+  template f();
 }
 )cpp",
   R"txt(
 *: TranslationUnit
 |-NamespaceDefinition
 | |-namespace
-| |-a
+| |-n
 | |-{
 | |-SimpleDeclaration
 | | |-struct
@@ -905,19 +928,58 @@
 | | | | `-T
 | | | |->
 | | | `-SimpleDeclaration
-| | |   |-static
-| | |   |-T
-| | |   |-SimpleDeclarator
-| | |   | |-f
-| | |   | `-ParametersAndQualifiers
-| | |   |   |-(
-| | |   |   `-)
-| | |   `-CompoundStatement
-| | | |-{
-| | | `-}
+| | |   |-struct
+| | |   |-ST
+| | |   |-{
+| | |   |-SimpleDeclaration
+| | |   | |-static
+| | |   | |-void
+| | |   | |-SimpleDeclarator
+| | |   | | |-f
+| | |   | | `-ParametersAndQualifiers
+| | |   | |   |-(
+| | |   | |   `-)
+| | |   | `-;
+| | |   |-}
+| | |   `-;
 | | |-}
 | | `-;
 | `-}
+|-TemplateDeclaration
+| |-template
+| |-<
+| |-UnknownDeclaration
+| | |-typename
+| | `-T
+| |->
+| `-SimpleDeclaration
+|   |-struct
+|   |-ST
+|   |-{
+|   |-SimpleDeclaration
+|   | |-struct
+|   | |-S
+|   | |-{
+|   | |-TemplateDeclaration
+|   | | |-template
+|   | | |-<
+|   | | |-UnknownDeclaration
+|   | | | |-typename
+|   | | | `-U
+|   | | |->
+|   | | `-SimpleDeclaration
+|   | |   |-static
+|   | |   |-U
+|   | |   |-SimpleDeclarator
+|   | |   | |-f
+|   | |   | `-ParametersAndQualifiers
+|   | |   |   |-(
+|   | |   |   `-)
+|   | |   `-;
+|   | |-}
+|   | `-;
+|   |-}
+|   `-;
 `-SimpleDeclaration
   |-void
   |-SimpleDeclarator
@@ -931,14 +993,81 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-NameSpecifier
-| | | | | `-::
-| | | | |-NameSpecifier
-| | | | | |-a
-| | | | | `-::
-| | | | `-NameSpecifier
-| | | |   |-S
-| | | |   `-::
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-n
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-S
+| | | | |-::
+| | | | |-SimpleTemplateNameSpecifier
+| | | | | |-template
+| | | | | |-ST
+| | | | | |-<
+| | | | | |-int
+| | | | | `->
+| | | | `-::
+| | | `-UnqualifiedId
+| | |   `-f
+| | |-(
+| | `-)
+| `-;
+|-ExpressionStatement
+| |-UnknownExpression
+| | |-IdExpression
+| | | |-NestedNameSpecifier
+| | | | |-IdentifierNameSpecifier
+| | | | | `-n
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-S
+| | | | |-::
+| | | | |-SimpleTemplateNameSpecifier
+| | | | | |-ST
+| | | | | |-<
+| | | | | |-int
+| | | | | `->
+| | | | `-::
+| | | `-UnqualifiedId
+| | |   `-f
+| | |-(
+| | `-)
+| `-;
+|-ExpressionStatement
+| |-UnknownExpression
+| | |-IdExpression
+| | | |-NestedNameSpecifier
+| | | | |-SimpleTemplateNameSpecifier
+| | | | | |-ST
+| | | | | |-<
+| | | | | |-int
+| | | | | `->
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-S
+| | | | `-::
+| | | `-UnqualifiedId
+| | |   |-f
+| | |   |-<
+| | |   |-int
+| | |   `->
+| | |-(
+| | `-)
+| `-;
+|-ExpressionStatement
+| |-UnknownExpression
+| | |-IdExpression
+| | | |-NestedNameSpecifier
+| | | | |-SimpleTemplateNameSpecifier
+| | | | | |-ST
+| | | | | |-<
+| | | | | |-int
+| | | | | `->
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-S
+| | |

[PATCH] D84811: [clangd] Add an option to exclude symbols outside of project root from index

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev planned changes to this revision.
kbobyrev added a comment.

In D84811#2199891 , @hokein wrote:

> In D84811#2199829 , @kbobyrev wrote:
>
>> In D84811#2199820 , @hokein wrote:
>>
>>> In D84811#2199510 , @kbobyrev 
>>> wrote:
>>>
 Even despite `FileFilter` not being fully implemented yet (an issue for a 
 separate patch) I think this change should still be correct and is 
 probably OK to land, WDYT @hokein?
>>>
>>> Yes, personally no objection on landing this, but landing it doesn't seem 
>>> to help much for your remote-index case (STL symbols are not filtered out)?
>>
>> True, it doesn't filter _all_ of them, but it partially filters some symbols 
>> which is still a win I guess :) My rationale is probably that having this 
>> blocked on more implementation details would be more reasonable if there was 
>> no implementation at all but since it still filters out some symbols this 
>> should probably be fine.
>
> It just reduces symptoms, not solving the problem. I think remote-index is 
> also blocked the FileFiltering stuff (we can't launch remote-index without 
> fixing this issue entirely).
>
> As you may have noticed implementing FileFiltering is tricky, I think the 
> indexer here is a good opportunity to test/verify your implementation 
> (comparing the index data before vs after), so my preference would be to 
> implement the FileFilter, test it with this patch together, then finally land 
> this patch after making sure everything works.

Fair enough, this is a viable strategy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84811

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

[PATCH] D84781: Use PointerUnion instead of inheritance for alternative clauses in NNS

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 283627.
eduucaldas added a comment.

- [SyntaxTree] Fix crash on name specifier.

This diff revision is based on https://reviews.llvm.org/D84348


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84781

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/include/clang/Tooling/Syntax/Tree.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/lib/Tooling/Syntax/Tree.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -994,18 +994,19 @@
 | | |-IdExpression
 | | | |-NestedNameSpecifier
 | | | | |-::
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-n
 | | | | |-::
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-S
 | | | | |-::
-| | | | |-SimpleTemplateNameSpecifier
-| | | | | |-template
-| | | | | |-ST
-| | | | | |-<
-| | | | | |-int
-| | | | | `->
+| | | | |-NameSpecifier
+| | | | | `-SimpleTemplateSpecifier
+| | | | |   |-template
+| | | | |   |-ST
+| | | | |   |-<
+| | | | |   |-int
+| | | | |   `->
 | | | | `-::
 | | | `-UnqualifiedId
 | | |   `-f
@@ -1016,17 +1017,18 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-n
 | | | | |-::
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-S
 | | | | |-::
-| | | | |-SimpleTemplateNameSpecifier
-| | | | | |-ST
-| | | | | |-<
-| | | | | |-int
-| | | | | `->
+| | | | |-NameSpecifier
+| | | | | `-SimpleTemplateSpecifier
+| | | | |   |-ST
+| | | | |   |-<
+| | | | |   |-int
+| | | | |   `->
 | | | | `-::
 | | | `-UnqualifiedId
 | | |   `-f
@@ -1037,13 +1039,14 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-SimpleTemplateNameSpecifier
-| | | | | |-ST
-| | | | | |-<
-| | | | | |-int
-| | | | | `->
+| | | | |-NameSpecifier
+| | | | | `-SimpleTemplateSpecifier
+| | | | |   |-ST
+| | | | |   |-<
+| | | | |   |-int
+| | | | |   `->
 | | | | |-::
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-S
 | | | | `-::
 | | | `-UnqualifiedId
@@ -1058,13 +1061,14 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-SimpleTemplateNameSpecifier
-| | | | | |-ST
-| | | | | |-<
-| | | | | |-int
-| | | | | `->
+| | | | |-NameSpecifier
+| | | | | `-SimpleTemplateSpecifier
+| | | | |   |-ST
+| | | | |   |-<
+| | | | |   |-int
+| | | | |   `->
 | | | | |-::
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-S
 | | | | `-::
 | | | |-template
@@ -1122,15 +1126,16 @@
   | |-UnknownExpression
   | | |-IdExpression
   | | | |-NestedNameSpecifier
-  | | | | |-IdentifierNameSpecifier
+  | | | | |-NameSpecifier
   | | | | | `-T
   | | | | |-::
-  | | | | |-SimpleTemplateNameSpecifier
-  | | | | | |-template
-  | | | | | |-U
-  | | | | | |-<
-  | | | | | |-int
-  | | | | | `->
+  | | | | |-NameSpecifier
+  | | | | | `-SimpleTemplateSpecifier
+  | | | | |   |-template
+  | | | | |   |-U
+  | | | | |   |-<
+  | | | | |   |-int
+  | | | | |   `->
   | | | | `-::
   | | | `-UnqualifiedId
   | | |   `-f
@@ -1141,10 +1146,10 @@
   | |-UnknownExpression
   | | |-IdExpression
   | | | |-NestedNameSpecifier
-  | | | | |-IdentifierNameSpecifier
+  | | | | |-NameSpecifier
   | | | | | `-T
   | | | | |-::
-  | | | | |-IdentifierNameSpecifier
+  | | | | |-NameSpecifier
   | | | | | `-U
   | | | | `-::
   | | | `-UnqualifiedId
@@ -1156,7 +1161,7 @@
   | |-UnknownExpression
   | | |-IdExpression
   | | | |-NestedNameSpecifier
-  | | | | |-IdentifierNameSpecifier
+  | | | | |-NameSpecifier
   | | | | | `-T
   | | | | `-::
   | | | |-template
@@ -1223,13 +1228,14 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-DecltypeNameSpecifier
-| | | | | |-decltype
-| | | | | |-(
-| | | | | |-IdExpression
-| | | | | | `-UnqualifiedId
-| | | | | |   `-s
-| | | | | `-)
+| | | | |-NameSpecifier
+| | | | | `-DecltypeSpecifier
+| | | | |   |-decltype
+| | | | |   |-(
+| | | | |   |-IdExpression
+| | | | |   | `-UnqualifiedId
+| | | | |   |   `-s
+

[PATCH] D85439: [SyntaxTree] Expand support for `NestedNameSpecifier`

[Eduardo Caldas via Phabricator via cfe-commits]

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

We want NestedNameSpecifier syntax nodes to be generally supported, not
only for `DeclRefExpr` and `DependentScopedDeclRefExpr`.

To achieve this we:

- Use the `RecursiveASTVisitor`'s API to traverse

`NestedNameSpecifierLoc`s and automatically create its syntax nodes

- Add links from the `NestedNameSpecifierLoc`s to their syntax nodes.

In this way, from any semantic construct that has a `NestedNameSpecifier`,
we implicitly generate its syntax node via RAV and we can easily access
this syntax node via the links we added.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85439

Files:
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -1232,9 +1232,7 @@
 | | | | | `-DecltypeSpecifier
 | | | | |   |-decltype
 | | | | |   |-(
-| | | | |   |-IdExpression
-| | | | |   | `-UnqualifiedId
-| | | | |   |   `-s
+| | | | |   |-s
 | | | | |   `-)
 | | | | `-::
 | | | `-UnqualifiedId
@@ -2980,7 +2978,8 @@
 `-UsingNamespaceDirective
   |-using
   |-namespace
-  |-::
+  |-NestedNameSpecifier
+  | `-::
   |-ns
   `-;
 )txt"));
@@ -3009,8 +3008,10 @@
 | `-}
 `-UsingDeclaration
   |-using
-  |-ns
-  |-::
+  |-NestedNameSpecifier
+  | |-NameSpecifier
+  | | `-ns
+  | `-::
   |-a
   `-;
 )txt"));
@@ -3214,11 +3215,14 @@
   |->
   `-SimpleDeclaration
 |-struct
-|-X
-|-<
-|-T
-|->
-|-::
+|-NestedNameSpecifier
+| |-NameSpecifier
+| | `-SimpleTemplateSpecifier
+| |   |-X
+| |   |-<
+| |   |-T
+| |   `->
+| `-::
 |-Y
 |-{
 |-}
@@ -3252,15 +3256,19 @@
 |-{
 |-UsingDeclaration
 | |-using
-| |-T
-| |-::
+| |-NestedNameSpecifier
+| | |-NameSpecifier
+| | | `-T
+| | `-::
 | |-foo
 | `-;
 |-UsingDeclaration
 | |-using
 | |-typename
-| |-T
-| |-::
+| |-NestedNameSpecifier
+| | |-NameSpecifier
+| | | `-T
+| | `-::
 | |-bar
 | `-;
 |-}
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -224,6 +224,34 @@
   return SourceRange(Start, End);
 }
 
+namespace llvm {
+template <> struct DenseMapInfo {
+  using FirstInfo = DenseMapInfo;
+  using SecondInfo = DenseMapInfo;
+
+  static inline NestedNameSpecifierLoc getEmptyKey() {
+return NestedNameSpecifierLoc(FirstInfo::getEmptyKey(),
+  SecondInfo::getEmptyKey());
+  }
+
+  static inline NestedNameSpecifierLoc getTombstoneKey() {
+return NestedNameSpecifierLoc(FirstInfo::getTombstoneKey(),
+  SecondInfo::getTombstoneKey());
+  }
+
+  static unsigned getHashValue(const clang::NestedNameSpecifierLoc &PairVal) {
+return detail::combineHashValue(
+FirstInfo::getHashValue(PairVal.getNestedNameSpecifier()),
+SecondInfo::getHashValue(PairVal.getOpaqueData()));
+  }
+
+  static bool isEqual(const NestedNameSpecifierLoc &LHS,
+  const NestedNameSpecifierLoc &RHS) {
+return LHS == RHS;
+  }
+};
+} // namespace llvm
+
 namespace {
 /// All AST hierarchy roots that can be represented as pointers.
 using ASTPtr = llvm::PointerUnion;
@@ -243,8 +271,22 @@
 
   syntax::Tree *find(ASTPtr P) const { return Nodes.lookup(P); }
 
+  void add(NestedNameSpecifierLoc From, syntax::Tree *To) {
+assert(To != nullptr);
+assert(From.hasQualifier());
+
+bool Added = NNSNodes.insert({From, To}).second;
+(void)Added;
+assert(Added && "mapping added twice");
+  }
+
+  syntax::Tree *find(NestedNameSpecifierLoc P) const {
+return NNSNodes.lookup(P);
+  }
+
 private:
   llvm::DenseMap Nodes;
+  llvm::DenseMap NNSNodes;
 };
 } // namespace
 
@@ -289,9 +331,11 @@
   }
 
   void foldNode(llvm::ArrayRef Range, syntax::Tree *New,
-NestedNameSpecifierLoc L) {
-// FIXME: add mapping for NestedNameSpecifierLoc
-foldNode(Range, New, nullptr);
+NestedNameSpecifierLoc From) {
+assert(New);
+Pending.foldChildren(Arena, Range, New);
+if (From)
+  Mapping.add(From, New);
   }
   /// Notifies that we should not consume trailing semicolon when computing
   /// token range of \p D.
@@ -315,6 +359,9 @@
   /// Set role for the syntax node matching \p N.
   void markChild(ASTPtr N, NodeRole R);
 
+  /// Set role for the syntax node matching \p N.
+  void markChild(NestedNameSpecifierLoc N, NodeRole R);
+
   /// Finish building the tree and consume the root node.
   syntax::TranslationUnit *fina

[PATCH] D85440: [SyntaxTree] Implement `NNS` using the `List` base API

[Eduardo Caldas via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85440

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/include/clang/Tooling/Syntax/Tree.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp

Index: clang/lib/Tooling/Syntax/Nodes.cpp
===
--- clang/lib/Tooling/Syntax/Nodes.cpp
+++ clang/lib/Tooling/Syntax/Nodes.cpp
@@ -200,10 +200,6 @@
 return OS << "IdExpression_id";
   case syntax::NodeRole::IdExpression_qualifier:
 return OS << "IdExpression_qualifier";
-  case syntax::NodeRole::NestedNameSpecifier_specifier:
-return OS << "NestedNameSpecifier_specifier";
-  case syntax::NodeRole::NestedNameSpecifier_delimiter:
-return OS << "NestedNameSpecifier_delimiter";
   case syntax::NodeRole::ParenExpression_subExpression:
 return OS << "ParenExpression_subExpression";
   }
@@ -219,23 +215,29 @@
   syntax::SimpleTemplateSpecifier *>::getFromOpaqueValue(firstChild());
 }
 
-std::vector syntax::NestedNameSpecifier::delimiters() {
-  std::vector Children;
-  for (auto *C = firstChild(); C; C = C->nextSibling()) {
-assert(C->role() == syntax::NodeRole::NestedNameSpecifier_delimiter);
-Children.push_back(llvm::cast(C));
+// We could have an interator in list to not pay memory costs of temporary
+// vector
+std::vector syntax::NestedNameSpecifier::specifiers() {
+  auto specifiersAsNodes = getElementsAsNodes();
+  std::vector Children;
+  for (const auto &element : specifiersAsNodes) {
+Children.push_back(llvm::cast(element));
   }
   return Children;
 }
 
-std::vector syntax::NestedNameSpecifier::specifiers() {
-  std::vector Children;
-  for (auto *C = firstChild(); C; C = C->nextSibling()) {
-assert(C->role() == syntax::NodeRole::NestedNameSpecifier_specifier);
-Children.push_back(llvm::cast(C));
+std::vector>
+syntax::NestedNameSpecifier::specifiersAndDoubleColons() {
+  auto specifiersAsNodesAndDoubleColons = getElementsAsNodesAndDelimiters();
+  std::vector>
+  Children;
+  for (const auto &specifierAndDoubleColon : specifiersAsNodesAndDoubleColons) {
+Children.push_back(
+{llvm::cast(specifierAndDoubleColon.element),
+ specifierAndDoubleColon.delimiter});
   }
   return Children;
-}
+};
 
 syntax::NestedNameSpecifier *syntax::IdExpression::qualifier() {
   return llvm::cast_or_null(
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -895,9 +895,8 @@
 for (auto it = QualifierLoc; it; it = it.getPrefix()) {
   auto *NS = BuildNameSpecifier(it);
   assert(NS);
-  Builder.markChild(NS, syntax::NodeRole::NestedNameSpecifier_specifier);
-  Builder.markChildToken(it.getEndLoc(),
- syntax::NodeRole::NestedNameSpecifier_delimiter);
+  Builder.markChild(NS, syntax::NodeRole::List_element);
+  Builder.markChildToken(it.getEndLoc(), syntax::NodeRole::List_delimiter);
 }
 Builder.foldNode(Builder.getRange(QualifierLoc.getSourceRange()),
  new (allocator()) syntax::NestedNameSpecifier,
Index: clang/include/clang/Tooling/Syntax/Tree.h
===
--- clang/include/clang/Tooling/Syntax/Tree.h
+++ clang/include/clang/Tooling/Syntax/Tree.h
@@ -214,6 +214,7 @@
 /// canBeEmpty() returning `true`
 /// getDelimiterTokenKind() returning `,`
 class List : public Tree {
+public:
   template  struct ElementAndDelimiter {
 Element *element;
 Leaf *delimiter;
@@ -225,6 +226,7 @@
 Separated,
   };
 
+  using Tree::Tree;
   /// Returns the elements and corresponding delimiters. Missing elements
   /// and delimiters are represented as null pointers.
   ///
Index: clang/include/clang/Tooling/Syntax/Nodes.h
===
--- clang/include/clang/Tooling/Syntax/Nodes.h
+++ clang/include/clang/Tooling/Syntax/Nodes.h
@@ -174,8 +174,6 @@
   ParametersAndQualifiers_trailingReturn,
   IdExpression_id,
   IdExpression_qualifier,
-  NestedNameSpecifier_specifier,
-  NestedNameSpecifier_delimiter,
   ParenExpression_subExpression
 };
 /// For debugging purposes.
@@ -235,14 +233,15 @@
 
 /// Models a `nested-name-specifier`. C++ [expr.prim.id.qual]
 /// e.g. the `std::vector::` in `std::vector::size`.
-class NestedNameSpecifier final : public Tree {
+class NestedNameSpecifier final : public List {
 public:
-  NestedNameSpecifier() : Tree(NodeKind::NestedNameSpecifier) {}
+  NestedNameSpecifier() : List(NodeKind::NestedNameSpecifier) {}
   static bool classof(const Node *N) {
 return N->kind() <= NodeKind::NestedNameSpecifier;
   }
   std::vector specifiers();
-  std::

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 283632.
simoll added a comment.

Fixed debug info representation for bool vectors.
Interpret 8*N with the N in vector_size(N) as the bool numbers of bits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/debug-info-vector-bool.c
  clang/test/SemaCXX/constexpr-vectors.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int &reference_to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/test/SemaCXX/constexpr-vectors.cpp
===
--- clang/test/SemaCXX/constexpr-vectors.cpp
+++ clang/test/SemaCXX/constexpr-vectors.cpp
@@ -204,35 +204,35 @@
 
   constexpr auto w = FourCharsVecSize{1, 2, 3, 4} <
  FourCharsVecSize{4, 3, 2, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto x = FourCharsVecSize{1, 2, 3, 4} >
  FourCharsVecSize{4, 3, 2, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto y = FourCharsVecSize{1, 2, 3, 4} <=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto z = FourCharsVecSize{1, 2, 3, 4} >=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto A = FourCharsVecSize{1, 2, 3, 4} ==
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto B = FourCharsVecSize{1, 2, 3, 4} !=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto C = FourCharsVecSize{1, 2, 3, 4} < 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto D = FourCharsVecSize{1, 2, 3, 4} > 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto E = FourCharsVecSize{1, 2, 3, 4} <= 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto F = FourCharsVecSize{1, 2, 3, 4} >= 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto G = FourCharsVecSize{1, 2, 3, 4} == 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto H = FourCharsVecSize{1, 2, 3, 4} != 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto I = FourCharsVecSize{1, 2, 3, 4} &
  FourCharsVecSize{4, 3, 2, 1};
@@ -252,15 +252,15 @@
 
   constexpr auto O = FourCharsVecSize{5, 0, 6, 0} &&
  FourCharsVecSize{5, 5, 0, 0};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto P = FourCharsVecSize{5, 0, 6, 0} ||
  FourCharsVecSize{5, 5, 0, 0};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto Q = FourCharsVecSize{5, 0, 6, 0} && 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto R = FourCharsVecSize{5, 0, 6, 0} || 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: 

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 283636.
simoll added a comment.

NFC. Cleanup.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/debug-info-vector-bool.c
  clang/test/SemaCXX/constexpr-vectors.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int &reference_to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/test/SemaCXX/constexpr-vectors.cpp
===
--- clang/test/SemaCXX/constexpr-vectors.cpp
+++ clang/test/SemaCXX/constexpr-vectors.cpp
@@ -204,35 +204,35 @@
 
   constexpr auto w = FourCharsVecSize{1, 2, 3, 4} <
  FourCharsVecSize{4, 3, 2, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto x = FourCharsVecSize{1, 2, 3, 4} >
  FourCharsVecSize{4, 3, 2, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto y = FourCharsVecSize{1, 2, 3, 4} <=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto z = FourCharsVecSize{1, 2, 3, 4} >=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto A = FourCharsVecSize{1, 2, 3, 4} ==
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto B = FourCharsVecSize{1, 2, 3, 4} !=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto C = FourCharsVecSize{1, 2, 3, 4} < 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto D = FourCharsVecSize{1, 2, 3, 4} > 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto E = FourCharsVecSize{1, 2, 3, 4} <= 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto F = FourCharsVecSize{1, 2, 3, 4} >= 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto G = FourCharsVecSize{1, 2, 3, 4} == 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto H = FourCharsVecSize{1, 2, 3, 4} != 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto I = FourCharsVecSize{1, 2, 3, 4} &
  FourCharsVecSize{4, 3, 2, 1};
@@ -252,15 +252,15 @@
 
   constexpr auto O = FourCharsVecSize{5, 0, 6, 0} &&
  FourCharsVecSize{5, 5, 0, 0};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto P = FourCharsVecSize{5, 0, 6, 0} ||
  FourCharsVecSize{5, 5, 0, 0};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto Q = FourCharsVecSize{5, 0, 6, 0} && 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto R = FourCharsVecSize{5, 0, 6, 0} || 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto T = CmpMul(a, b);
   // CHECK: store <4 x i8> 
Index: clang/test/CodeGen/debug-info-v

[PATCH] D85439: [SyntaxTree] Expand support for `NestedNameSpecifier`

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas added a reviewer: gribozavr2.
eduucaldas added inline comments.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:227-254
+namespace llvm {
+template <> struct DenseMapInfo {
+  using FirstInfo = DenseMapInfo;
+  using SecondInfo = DenseMapInfo;
+
+  static inline NestedNameSpecifierLoc getEmptyKey() {
+return NestedNameSpecifierLoc(FirstInfo::getEmptyKey(),

Inpired on the definition of: 
* `template struct DenseMapInfo`
* `template struct DenseMapInfo>`

Please tell me if this is all silly.



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1235
 | | | | |   |-(
-| | | | |   |-IdExpression
-| | | | |   | `-UnqualifiedId
-| | | | |   |   `-s
+| | | | |   |-s
 | | | | |   `-)

standard `TraverseNestedNameSpecifierLoc` fired `TraverseTypeLoc`, once we 
override it we lost this. This will be fixed when refining the Node for 
`DecltypeSpecifier`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85439

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

[PATCH] D85276: [PGO][CUDA][HIP] Skip generating profile on the device stub and wrong-side functions.

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D85276#2199655 , @yaxunl wrote:

> Do we need to disable pgo and coverage mapping for device compilation? Or it 
> is already disabled?

We already disable profiling during device compilation for NVIDIA and AMD GPUs:
https://github.com/llvm/llvm-project/blob/394db2259575ef3cac8d3d37836b11eb2373c435/clang/lib/Driver/ToolChains/Clang.cpp#L4876


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85276

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

[PATCH] D85440: [SyntaxTree] Implement `NNS` using the `List` base API

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 283638.
eduucaldas added a comment.

Clean List specific code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85440

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp

Index: clang/lib/Tooling/Syntax/Nodes.cpp
===
--- clang/lib/Tooling/Syntax/Nodes.cpp
+++ clang/lib/Tooling/Syntax/Nodes.cpp
@@ -200,10 +200,6 @@
 return OS << "IdExpression_id";
   case syntax::NodeRole::IdExpression_qualifier:
 return OS << "IdExpression_qualifier";
-  case syntax::NodeRole::NestedNameSpecifier_specifier:
-return OS << "NestedNameSpecifier_specifier";
-  case syntax::NodeRole::NestedNameSpecifier_delimiter:
-return OS << "NestedNameSpecifier_delimiter";
   case syntax::NodeRole::ParenExpression_subExpression:
 return OS << "ParenExpression_subExpression";
   }
@@ -219,23 +215,29 @@
   syntax::SimpleTemplateSpecifier *>::getFromOpaqueValue(firstChild());
 }
 
-std::vector syntax::NestedNameSpecifier::delimiters() {
-  std::vector Children;
-  for (auto *C = firstChild(); C; C = C->nextSibling()) {
-assert(C->role() == syntax::NodeRole::NestedNameSpecifier_delimiter);
-Children.push_back(llvm::cast(C));
+// We could have an interator in list to not pay memory costs of temporary
+// vector
+std::vector syntax::NestedNameSpecifier::specifiers() {
+  auto specifiersAsNodes = getElementsAsNodes();
+  std::vector Children;
+  for (const auto &element : specifiersAsNodes) {
+Children.push_back(llvm::cast(element));
   }
   return Children;
 }
 
-std::vector syntax::NestedNameSpecifier::specifiers() {
-  std::vector Children;
-  for (auto *C = firstChild(); C; C = C->nextSibling()) {
-assert(C->role() == syntax::NodeRole::NestedNameSpecifier_specifier);
-Children.push_back(llvm::cast(C));
+std::vector>
+syntax::NestedNameSpecifier::specifiersAndDoubleColons() {
+  auto specifiersAsNodesAndDoubleColons = getElementsAsNodesAndDelimiters();
+  std::vector>
+  Children;
+  for (const auto &specifierAndDoubleColon : specifiersAsNodesAndDoubleColons) {
+Children.push_back(
+{llvm::cast(specifierAndDoubleColon.element),
+ specifierAndDoubleColon.delimiter});
   }
   return Children;
-}
+};
 
 syntax::NestedNameSpecifier *syntax::IdExpression::qualifier() {
   return llvm::cast_or_null(
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -895,9 +895,8 @@
 for (auto it = QualifierLoc; it; it = it.getPrefix()) {
   auto *NS = BuildNameSpecifier(it);
   assert(NS);
-  Builder.markChild(NS, syntax::NodeRole::NestedNameSpecifier_specifier);
-  Builder.markChildToken(it.getEndLoc(),
- syntax::NodeRole::NestedNameSpecifier_delimiter);
+  Builder.markChild(NS, syntax::NodeRole::List_element);
+  Builder.markChildToken(it.getEndLoc(), syntax::NodeRole::List_delimiter);
 }
 Builder.foldNode(Builder.getRange(QualifierLoc.getSourceRange()),
  new (allocator()) syntax::NestedNameSpecifier,
Index: clang/include/clang/Tooling/Syntax/Nodes.h
===
--- clang/include/clang/Tooling/Syntax/Nodes.h
+++ clang/include/clang/Tooling/Syntax/Nodes.h
@@ -174,8 +174,6 @@
   ParametersAndQualifiers_trailingReturn,
   IdExpression_id,
   IdExpression_qualifier,
-  NestedNameSpecifier_specifier,
-  NestedNameSpecifier_delimiter,
   ParenExpression_subExpression
 };
 /// For debugging purposes.
@@ -235,14 +233,15 @@
 
 /// Models a `nested-name-specifier`. C++ [expr.prim.id.qual]
 /// e.g. the `std::vector::` in `std::vector::size`.
-class NestedNameSpecifier final : public Tree {
+class NestedNameSpecifier final : public List {
 public:
-  NestedNameSpecifier() : Tree(NodeKind::NestedNameSpecifier) {}
+  NestedNameSpecifier() : List(NodeKind::NestedNameSpecifier) {}
   static bool classof(const Node *N) {
 return N->kind() <= NodeKind::NestedNameSpecifier;
   }
   std::vector specifiers();
-  std::vector delimiters();
+  std::vector>
+  specifiersAndDoubleColons();
 };
 
 /// Models an `unqualified-id`. C++ [expr.prim.id.unqual]
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85440: [SyntaxTree] Implement `NNS` using the `List` base API

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 283643.
eduucaldas added a comment.

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85440

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp

Index: clang/lib/Tooling/Syntax/Nodes.cpp
===
--- clang/lib/Tooling/Syntax/Nodes.cpp
+++ clang/lib/Tooling/Syntax/Nodes.cpp
@@ -200,10 +200,6 @@
 return OS << "IdExpression_id";
   case syntax::NodeRole::IdExpression_qualifier:
 return OS << "IdExpression_qualifier";
-  case syntax::NodeRole::NestedNameSpecifier_specifier:
-return OS << "NestedNameSpecifier_specifier";
-  case syntax::NodeRole::NestedNameSpecifier_delimiter:
-return OS << "NestedNameSpecifier_delimiter";
   case syntax::NodeRole::ParenExpression_subExpression:
 return OS << "ParenExpression_subExpression";
   }
@@ -219,23 +215,29 @@
   syntax::SimpleTemplateSpecifier *>::getFromOpaqueValue(firstChild());
 }
 
-std::vector syntax::NestedNameSpecifier::delimiters() {
-  std::vector Children;
-  for (auto *C = firstChild(); C; C = C->nextSibling()) {
-assert(C->role() == syntax::NodeRole::NestedNameSpecifier_delimiter);
-Children.push_back(llvm::cast(C));
+// We could have an interator in list to not pay memory costs of temporary
+// vector
+std::vector syntax::NestedNameSpecifier::specifiers() {
+  auto specifiersAsNodes = getElementsAsNodes();
+  std::vector Children;
+  for (const auto &element : specifiersAsNodes) {
+Children.push_back(llvm::cast(element));
   }
   return Children;
 }
 
-std::vector syntax::NestedNameSpecifier::specifiers() {
-  std::vector Children;
-  for (auto *C = firstChild(); C; C = C->nextSibling()) {
-assert(C->role() == syntax::NodeRole::NestedNameSpecifier_specifier);
-Children.push_back(llvm::cast(C));
+std::vector>
+syntax::NestedNameSpecifier::specifiersAndDoubleColons() {
+  auto specifiersAsNodesAndDoubleColons = getElementsAsNodesAndDelimiters();
+  std::vector>
+  Children;
+  for (const auto &specifierAndDoubleColon : specifiersAsNodesAndDoubleColons) {
+Children.push_back(
+{llvm::cast(specifierAndDoubleColon.element),
+ specifierAndDoubleColon.delimiter});
   }
   return Children;
-}
+};
 
 syntax::NestedNameSpecifier *syntax::IdExpression::qualifier() {
   return llvm::cast_or_null(
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -895,9 +895,8 @@
 for (auto it = QualifierLoc; it; it = it.getPrefix()) {
   auto *NS = BuildNameSpecifier(it);
   assert(NS);
-  Builder.markChild(NS, syntax::NodeRole::NestedNameSpecifier_specifier);
-  Builder.markChildToken(it.getEndLoc(),
- syntax::NodeRole::NestedNameSpecifier_delimiter);
+  Builder.markChild(NS, syntax::NodeRole::List_element);
+  Builder.markChildToken(it.getEndLoc(), syntax::NodeRole::List_delimiter);
 }
 Builder.foldNode(Builder.getRange(QualifierLoc.getSourceRange()),
  new (allocator()) syntax::NestedNameSpecifier,
Index: clang/include/clang/Tooling/Syntax/Nodes.h
===
--- clang/include/clang/Tooling/Syntax/Nodes.h
+++ clang/include/clang/Tooling/Syntax/Nodes.h
@@ -174,8 +174,6 @@
   ParametersAndQualifiers_trailingReturn,
   IdExpression_id,
   IdExpression_qualifier,
-  NestedNameSpecifier_specifier,
-  NestedNameSpecifier_delimiter,
   ParenExpression_subExpression
 };
 /// For debugging purposes.
@@ -235,14 +233,15 @@
 
 /// Models a `nested-name-specifier`. C++ [expr.prim.id.qual]
 /// e.g. the `std::vector::` in `std::vector::size`.
-class NestedNameSpecifier final : public Tree {
+class NestedNameSpecifier final : public List {
 public:
-  NestedNameSpecifier() : Tree(NodeKind::NestedNameSpecifier) {}
+  NestedNameSpecifier() : List(NodeKind::NestedNameSpecifier) {}
   static bool classof(const Node *N) {
 return N->kind() <= NodeKind::NestedNameSpecifier;
   }
   std::vector specifiers();
-  std::vector delimiters();
+  std::vector>
+  specifiersAndDoubleColons();
 };
 
 /// Models an `unqualified-id`. C++ [expr.prim.id.unqual]
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 8d072a4 - [OPENMP]Fix for Windows buildbots, NFC.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2020-08-06T12:36:52-04:00
New Revision: 8d072a4405213623a1b13dbac2e451df81457343

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

LOG: [OPENMP]Fix for Windows buildbots, NFC.

Added: 


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

Removed: 




diff  --git a/clang/include/clang/AST/OpenMPClause.h 
b/clang/include/clang/AST/OpenMPClause.h
index 01e915d5ce9d..35ab8ff39efa 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -7884,8 +7884,8 @@ struct TargetOMPContext final : public 
llvm::omp::OMPContext {
 /// any.
 class OMPChildren final
 : private llvm::TrailingObjects {
+  friend TrailingObjects;
   friend class OMPClauseReader;
-  friend class TrailingObjects;
   friend class OMPExecutableDirective;
   template  friend class OMPDeclarativeDirective;
 



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

[PATCH] D85337: [AMDGPU] gfx1031 target

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added inline comments.



Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:1844
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1030),
+  LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1031),
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_XNACK),

jhenderson wrote:
> Where's the dedicated llvm-readobj test for this? Please add one. If there 
> exists one for the other AMDGPU flags, extend that one, otherwise, it might 
> be best to write one for the whole set at once.
It is in the lvm/test/CodeGen/AMDGPU/elf-header-flags-mach.ll above along with 
all other targets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85337

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

[PATCH] D85276: [PGO][CUDA][HIP] Skip generating profile on the device stub and wrong-side functions.

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

In D85276#2200108 , @tra wrote:

> In D85276#2199655 , @yaxunl wrote:
>
>> Do we need to disable pgo and coverage mapping for device compilation? Or it 
>> is already disabled?
>
> We already disable profiling during device compilation for NVIDIA and AMD 
> GPUs:
> https://github.com/llvm/llvm-project/blob/394db2259575ef3cac8d3d37836b11eb2373c435/clang/lib/Driver/ToolChains/Clang.cpp#L4876

Anyway, this patch just fixes the caused by that device stub function. As it's 
"emitted" in the host compilation, we need to skip generating instrumentation 
on it explicitly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85276

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

[PATCH] D85295: [SyntaxTree] Implement the List construct.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/Tree.h:247
+
+  TerminationKind getTerminationKind();
+

I just realized that the rest of the syntax tree API does not use the `get~` 
prefix. However, most of Clang does, as far as I know. Which way should we 
repaint?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85295

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

[PATCH] D85440: [SyntaxTree] Implement `NNS` using the `List` base API

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

Please also add appropriate handling to `syntax::List::getDelimiterTokenKind`, 
`getTerminationKind`, and `canBeEmpty`.




Comment at: clang/lib/Tooling/Syntax/Nodes.cpp:240
   return Children;
-}
+};
 




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85440

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

[PATCH] D85440: [SyntaxTree] Implement `NNS` using the `List` base API

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

Could you add unit tests for methods in List and NNS? OK if they are in a 
separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85440

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

[PATCH] D84602: [MSP430] Expose msp430_builtin calling convention to C code

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko added a comment.

I suspect there might be some terminology clash, so clarifying a bit just in 
case.

It was probably better to refer to these functions as //LibCalls// - functions 
from the compiler support library (such as `libgcc`) such as `__adddf3`. While 
there are `__popcount[sdt]i2` among them, most of the times they are implicitly 
called when the high-level code performs division on `__uint128`, for example. 
Unfortunately, they reside under `compiler-rt/lib/builtins` - so that name was 
used... :) So, if I get it right, you have proposed something like 
`clang/include/clang/Basic/BuiltinsMips.def` and those are another kind of 
builtins.

The `CallingConv::MSP430_BUILTIN` was the name of a calling convention that the 
MSP430 codegen of LLVM had to use //for a small subset of those support 
functions//, as defined by MSP430 EABI. While it can be useful to add some 
other CC for those functions for internal use by compiler-rt someday, now there 
are only two CCs defined for MSP430 LibCalls: that "special convention" for 13 
functions only with two 64-bit arguments (including 64-bit integer shifts) and 
the `CallingConv::C` for everything else both from the support library and 
regular object files.

Technically, these functions could probably be listed by names somewhere in the 
backend code, completely detangling the upstreaming of MSP430 compiler-rt port 
from D84602  (this one), D84605 
 and, the most scary of them, D84636 
. That may probably look a bit hackish, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84602

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

[PATCH] D85276: [PGO][CUDA][HIP] Skip generating profile on the device stub and wrong-side functions.

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM. thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85276

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