date:20201005

[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

2020-10-05 Thread Serge Pavlov via Phabricator via cfe-commits

sepavloff added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.h:410-411
 setRoundingMode(LO.getFPRoundingMode());
+// FENV access is true if rounding math is enabled.
+setAllowFEnvAccess((getRoundingMode() == llvm::RoundingMode::Dynamic));
 setFPExceptionMode(LO.getFPExceptionMode());


I don't think this deduction is correct.

`AllowFEnvAccess` implies that user can access FP environment in a way unknown 
to the compiler, for example by calling external functions or by using inline 
assembler. So if `AllowFEnvAccess` is set, the compiler must assume that 
rounding mode is dynamic, as user can set it to any value unknown to the 
compiler. Similarly, `FPExceptionMode` must be set to strict as user may 
read/write exception bits in any way or may associate them with traps. These 
deductions are valid and compiler must apply them.

The reverse is not valid. `AllowFEnvAccess` is a very coarse instrument to 
manipulate FP environment, it prevents from many optimization techniques. In 
contrast, setting `RoundingMode` to dynamic means only that rounding mode may 
be different from the default value. If a user changes rounding mode by calls 
to external functions like `fesetmode` or by using some intrinsic, the compiler 
still can, for example, reorder FP operations, if `FPExceptionMode` is 
`ignore`. Impact of performance in this case can be minimized.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

[PATCH] D88810: [clangd] Add isKeyword function.

2020-10-05 Thread Haojian Wu via Phabricator via cfe-commits

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

This will be used in rename for doing basic name validation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88810

Files:
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/unittests/SourceCodeTests.cpp


Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -789,6 +789,18 @@
   EXPECT_TRUE(isHeaderFile("header.h", LangOpts));
 }
 
+TEST(SourceCodeTests, isKeywords) {
+  LangOptions LangOpts;
+  LangOpts.CPlusPlus11 = true;
+  EXPECT_TRUE(isKeyword("int", LangOpts));
+  EXPECT_TRUE(isKeyword("return", LangOpts));
+
+  // these are identifiers (not keywords!) with special meaning in some
+  // contexts.
+  EXPECT_FALSE(isKeyword("final", LangOpts));
+  EXPECT_FALSE(isKeyword("override", LangOpts));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/SourceCode.h
===
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -248,6 +248,10 @@
   const LangOptions &LangOpts);
 };
 
+/// Return true if the \p TokenName is in the list of reversed keywords of the
+/// language.
+bool isKeyword(llvm::StringRef TokenName, const LangOptions &LangOpts);
+
 /// Heuristically determine namespaces visible at a point, without parsing 
Code.
 /// This considers using-directives and enclosing namespace-declarations that
 /// are visible (and not obfuscated) in the file itself (not headers).
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -633,6 +633,12 @@
   return Ranges;
 }
 
+bool isKeyword(llvm::StringRef NewName, const LangOptions &LangOpts) {
+  // Keywords are initialized in constructor.
+  clang::IdentifierTable KeywordsTable(LangOpts);
+  return KeywordsTable.find(NewName) != KeywordsTable.end();
+}
+
 namespace {
 struct NamespaceEvent {
   enum {


Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -789,6 +789,18 @@
   EXPECT_TRUE(isHeaderFile("header.h", LangOpts));
 }
 
+TEST(SourceCodeTests, isKeywords) {
+  LangOptions LangOpts;
+  LangOpts.CPlusPlus11 = true;
+  EXPECT_TRUE(isKeyword("int", LangOpts));
+  EXPECT_TRUE(isKeyword("return", LangOpts));
+
+  // these are identifiers (not keywords!) with special meaning in some
+  // contexts.
+  EXPECT_FALSE(isKeyword("final", LangOpts));
+  EXPECT_FALSE(isKeyword("override", LangOpts));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/SourceCode.h
===
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -248,6 +248,10 @@
   const LangOptions &LangOpts);
 };
 
+/// Return true if the \p TokenName is in the list of reversed keywords of the
+/// language.
+bool isKeyword(llvm::StringRef TokenName, const LangOptions &LangOpts);
+
 /// Heuristically determine namespaces visible at a point, without parsing Code.
 /// This considers using-directives and enclosing namespace-declarations that
 /// are visible (and not obfuscated) in the file itself (not headers).
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -633,6 +633,12 @@
   return Ranges;
 }
 
+bool isKeyword(llvm::StringRef NewName, const LangOptions &LangOpts) {
+  // Keywords are initialized in constructor.
+  clang::IdentifierTable KeywordsTable(LangOpts);
+  return KeywordsTable.find(NewName) != KeywordsTable.end();
+}
+
 namespace {
 struct NamespaceEvent {
   enum {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88469: [clangd] Heuristic resolution for dependent type and template names

2020-10-05 Thread Haojian Wu via Phabricator via cfe-commits

hokein added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.cpp:578
+for (const NamedDecl *ND : getMembersReferencedViaDependentName(
+ resolveNestedNameSpecifierToType(DTST->getQualifier()),
+ [DTST](ASTContext &) { return DTST->getIdentifier(); },

we need to be robust in `resolveNestedNameSpecifierToType` -- we may get a null 
qualifier for DependentTemplateSpecializationType, see 
https://reviews.llvm.org/D76320.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88469

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

[PATCH] D86048: [AST][RecoveryExpr] Popagate the error-bit from a VarDecl's initializer to DeclRefExpr.

2020-10-05 Thread Haojian Wu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3423d5c9da81: [AST][RecoveryExpr] Popagate the error-bit 
from a VarDecl's initializer to… (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86048

Files:
  clang/lib/AST/ComputeDependence.cpp
  clang/test/Sema/invalid-member.cpp


Index: clang/test/Sema/invalid-member.cpp
===
--- clang/test/Sema/invalid-member.cpp
+++ clang/test/Sema/invalid-member.cpp
@@ -19,3 +19,11 @@
 };
 // Should be able to evaluate sizeof without crashing.
 static_assert(sizeof(Z) == 1, "No valid members");
+
+constexpr int N = undef; // expected-error {{use of undeclared identifier}}
+template
+class ABC {};
+class T {
+  ABC abc;
+};
+static_assert(sizeof(T) == 1, "No valid members");
Index: clang/lib/AST/ComputeDependence.cpp
===
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -466,10 +466,12 @@
  : Var->getType()->isIntegralOrEnumerationType()) &&
 (Var->getType().isConstQualified() ||
  Var->getType()->isReferenceType())) {
-  if (const Expr *Init = Var->getAnyInitializer())
-if (Init->isValueDependent()) {
+  if (const Expr *Init = Var->getAnyInitializer()) {
+if (Init->isValueDependent())
   Deps |= ExprDependence::ValueInstantiation;
-}
+if (Init->containsErrors())
+  Deps |= ExprDependence::Error;
+  }
 }
 
 // (VD) - FIXME: Missing from the standard:


Index: clang/test/Sema/invalid-member.cpp
===
--- clang/test/Sema/invalid-member.cpp
+++ clang/test/Sema/invalid-member.cpp
@@ -19,3 +19,11 @@
 };
 // Should be able to evaluate sizeof without crashing.
 static_assert(sizeof(Z) == 1, "No valid members");
+
+constexpr int N = undef; // expected-error {{use of undeclared identifier}}
+template
+class ABC {};
+class T {
+  ABC abc;
+};
+static_assert(sizeof(T) == 1, "No valid members");
Index: clang/lib/AST/ComputeDependence.cpp
===
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -466,10 +466,12 @@
  : Var->getType()->isIntegralOrEnumerationType()) &&
 (Var->getType().isConstQualified() ||
  Var->getType()->isReferenceType())) {
-  if (const Expr *Init = Var->getAnyInitializer())
-if (Init->isValueDependent()) {
+  if (const Expr *Init = Var->getAnyInitializer()) {
+if (Init->isValueDependent())
   Deps |= ExprDependence::ValueInstantiation;
-}
+if (Init->containsErrors())
+  Deps |= ExprDependence::Error;
+  }
 }
 
 // (VD) - FIXME: Missing from the standard:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86048: [AST][RecoveryExpr] Popagate the error-bit from a VarDecl's initializer to DeclRefExpr.

2020-10-05 Thread Haojian Wu via Phabricator via cfe-commits

hokein added a comment.

@rsmith I'm landing this patch now. Happy to follow-up if you have other 
comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86048

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

[clang] 3423d5c - [AST][RecoveryExpr] Popagate the error-bit from a VarDecl's initializer to DeclRefExpr.

2020-10-05 Thread Haojian Wu via cfe-commits


Author: Haojian Wu
Date: 2020-10-05T10:35:29+02:00
New Revision: 3423d5c9da812b0076d1cf14e96ce453e35257b6

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

LOG: [AST][RecoveryExpr] Popagate the error-bit from a VarDecl's initializer to 
DeclRefExpr.

The error-bit was missing, if a DeclRefExpr (which refers to a VarDecl
with a contains-errors initializer).

It could cause different violations in clang -- the DeclRefExpr is 
value-dependent,
but not contains-errors, `ABC` could produce a non-error
and non-dependent type in non-template context, which will lead to
crashes in constexpr evaluation.

Reviewed By: sammccall

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

Added: 


Modified: 
clang/lib/AST/ComputeDependence.cpp
clang/test/Sema/invalid-member.cpp

Removed: 




diff  --git a/clang/lib/AST/ComputeDependence.cpp 
b/clang/lib/AST/ComputeDependence.cpp
index 320025e5fc82..f8dfeed0962e 100644
--- a/clang/lib/AST/ComputeDependence.cpp
+++ b/clang/lib/AST/ComputeDependence.cpp
@@ -466,10 +466,12 @@ ExprDependence clang::computeDependence(DeclRefExpr *E, 
const ASTContext &Ctx) {
  : Var->getType()->isIntegralOrEnumerationType()) &&
 (Var->getType().isConstQualified() ||
  Var->getType()->isReferenceType())) {
-  if (const Expr *Init = Var->getAnyInitializer())
-if (Init->isValueDependent()) {
+  if (const Expr *Init = Var->getAnyInitializer()) {
+if (Init->isValueDependent())
   Deps |= ExprDependence::ValueInstantiation;
-}
+if (Init->containsErrors())
+  Deps |= ExprDependence::Error;
+  }
 }
 
 // (VD) - FIXME: Missing from the standard:

diff  --git a/clang/test/Sema/invalid-member.cpp 
b/clang/test/Sema/invalid-member.cpp
index 9559ead082f0..57ee187ccf4d 100644
--- a/clang/test/Sema/invalid-member.cpp
+++ b/clang/test/Sema/invalid-member.cpp
@@ -19,3 +19,11 @@ class Z {
 };
 // Should be able to evaluate sizeof without crashing.
 static_assert(sizeof(Z) == 1, "No valid members");
+
+constexpr int N = undef; // expected-error {{use of undeclared identifier}}
+template
+class ABC {};
+class T {
+  ABC abc;
+};
+static_assert(sizeof(T) == 1, "No valid members");



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

[PATCH] D85354: [clangd] Reduce availability of extract function

2020-10-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits

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

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85354

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -593,6 +593,8 @@
   EXPECT_EQ(apply("auto lam = [](){ [[int x;]] }; "), "unavailable");
   // Partial statements aren't extracted.
   EXPECT_THAT(apply("int [[x = 0]];"), "unavailable");
+  // FIXME: Support hoisting.
+  EXPECT_THAT(apply(" [[int a = 5;]] a++; "), "unavailable");
 
   // Ensure that end of Zone and Beginning of PostZone being adjacent doesn't
   // lead to break being included in the extraction zone.
@@ -600,8 +602,6 @@
   // FIXME: ExtractFunction should be unavailable inside loop construct
   // initializer/condition.
   EXPECT_THAT(apply(" for([[int i = 0;]];);"), HasSubstr("extracted"));
-  // Don't extract because needs hoisting.
-  EXPECT_THAT(apply(" [[int a = 5;]] a++; "), StartsWith("fail"));
   // Extract certain return
   EXPECT_THAT(apply(" if(true) [[{ return; }]] "), HasSubstr("extracted"));
   // Don't extract uncertain return
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -47,6 +47,7 @@
 //===--===//
 
 #include "AST.h"
+#include "FindTarget.h"
 #include "ParsedAST.h"
 #include "Selection.h"
 #include "SourceCode.h"
@@ -54,6 +55,7 @@
 #include "support/Logger.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Stmt.h"
@@ -65,6 +67,8 @@
 #include "clang/Tooling/Refactoring/Extract/SourceExtraction.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
@@ -152,6 +156,9 @@
   const FunctionDecl *EnclosingFunction = nullptr;
   // The half-open file range of the enclosing function.
   SourceRange EnclosingFuncRange;
+  // Set of statements that form the ExtractionZone.
+  llvm::DenseSet RootStmts;
+
   SourceLocation getInsertionPoint() const {
 return EnclosingFuncRange.getBegin();
   }
@@ -159,10 +166,45 @@
   // The last root statement is important to decide where we need to insert a
   // semicolon after the extraction.
   const Node *getLastRootStmt() const { return Parent->Children.back(); }
-  void generateRootStmts();
 
-private:
-  llvm::DenseSet RootStmts;
+  // Checks if declarations inside extraction zone are accessed afterwards.
+  //
+  // This performs a partial AST traversal proportional to the size of the
+  // enclosing function, so it is possibly expensive.
+  bool requiresHoisting(const SourceManager &SM) const {
+// First find all the declarations that happened inside extraction zone.
+llvm::SmallSet DeclsInExtZone;
+for (auto *RootStmt : RootStmts) {
+  findExplicitReferences(RootStmt,
+ [&DeclsInExtZone](const ReferenceLoc &Loc) {
+   if (!Loc.IsDecl)
+ return;
+   DeclsInExtZone.insert(Loc.Targets.front());
+ });
+}
+// Early exit without performing expensive traversal below.
+if (DeclsInExtZone.empty())
+  return false;
+// Then make sure they are not used outside the zone.
+for (const auto *S : EnclosingFunction->getBody()->children()) {
+  if (SM.isBeforeInTranslationUnit(S->getSourceRange().getEnd(),
+   ZoneRange.getEnd()))
+continue;
+  bool HasPostUse = false;
+  findExplicitReferences(S, [&](const ReferenceLoc &Loc) {
+if (HasPostUse ||
+SM.isBeforeInTranslationUnit(Loc.NameLoc, ZoneRange.getEnd()))
+  return;
+HasPostUse =
+llvm::any_of(Loc.Targets, [&DeclsInExtZone](const Decl *Target) {
+  return DeclsInExtZone.contains(Target);
+});
+  });
+  if (HasPostUse)
+return true;
+}
+return false;
+  }
 };
 
 // Whether the code in the extraction zone is guaranteed to return, assuming
@@ -185,12 +227,6 @@
   return RootStmts.find(S) != RootStmts.end();
 }
 
-// Generate RootStmts set
-vo

[PATCH] D88411: [clangd] Introduce MemoryTrees

2020-10-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet updated this revision to Diff 296111.
kadircet marked 7 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88411

Files:
  clang-tools-extra/clangd/support/CMakeLists.txt
  clang-tools-extra/clangd/support/MemoryTree.cpp
  clang-tools-extra/clangd/support/MemoryTree.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/support/MemoryTreeTests.cpp

Index: clang-tools-extra/clangd/unittests/support/MemoryTreeTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/support/MemoryTreeTests.cpp
@@ -0,0 +1,131 @@
+//===-- MemoryTreeTests.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
+//
+//===--===//
+
+#include "support/MemoryTree.h"
+#include "llvm/Support/Allocator.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+
+namespace clang {
+namespace clangd {
+namespace {
+using testing::Contains;
+using testing::UnorderedElementsAre;
+
+struct Component {
+  std::string Name;
+  size_t Size;
+};
+std::ostream &operator<<(std::ostream &OS, const Component &Comp) {
+  return OS << Comp.Name << ',' << Comp.Size;
+}
+
+MATCHER_P2(WithNameAndSize, Name, Size, "") {
+  return std::tie(arg.Name, arg.Size) == std::tie(Name, Size);
+}
+
+std::vector collectComponents(const MemoryTree &MT) {
+  std::vector SeenComponents;
+  MT.traverseTree(
+  [&SeenComponents](size_t Size, llvm::StringRef CompName) {
+Component C;
+C.Name = CompName.str();
+C.Size = Size;
+SeenComponents.emplace_back(std::move(C));
+  },
+  "root");
+  return SeenComponents;
+}
+
+TEST(MemoryTree, Basics) {
+  MemoryTree MT;
+  EXPECT_THAT(collectComponents(MT),
+  UnorderedElementsAre(WithNameAndSize("root", 0)));
+
+  MT.addUsage(42);
+  EXPECT_THAT(collectComponents(MT),
+  UnorderedElementsAre(WithNameAndSize("root", 42)));
+
+  MT.addChild("leaf")->addUsage(1);
+  EXPECT_THAT(collectComponents(MT),
+  UnorderedElementsAre(WithNameAndSize("root", 42),
+   WithNameAndSize("root.leaf", 1)));
+
+  // addChild should be idempotent.
+  MT.addChild("leaf")->addUsage(1);
+  EXPECT_THAT(collectComponents(MT),
+  UnorderedElementsAre(WithNameAndSize("root", 42),
+   WithNameAndSize("root.leaf", 2)));
+}
+
+TEST(MemoryTree, DetailedNodesWithoutDetails) {
+  MemoryTree MT;
+  MT.addDetail("should_be_ignored")->addUsage(2);
+  EXPECT_THAT(collectComponents(MT),
+  UnorderedElementsAre(WithNameAndSize("root", 2)));
+
+  // Make sure children from details are merged.
+  MT.addDetail("first_detail")->addChild("leaf")->addUsage(1);
+  MT.addDetail("second_detail")->addChild("leaf")->addUsage(1);
+  EXPECT_THAT(collectComponents(MT), Contains(WithNameAndSize("root.leaf", 2)));
+}
+
+TEST(MemoryTree, DetailedNodesWithDetails) {
+  llvm::BumpPtrAllocator Alloc;
+  MemoryTree MT(&Alloc);
+
+  MT.addDetail("first_detail")->addChild("leaf")->addUsage(1);
+  EXPECT_THAT(collectComponents(MT),
+  Contains(WithNameAndSize("root.first_detail.leaf", 1)));
+  MT.addDetail("second_detail")->addChild("leaf")->addUsage(1);
+  EXPECT_THAT(collectComponents(MT),
+  Contains(WithNameAndSize("root.second_detail.leaf", 1)));
+}
+
+TEST(MemoryTree, Traversal) {
+  auto AddNodes = [](MemoryTree Root) {
+Root.addChild("leaf")->addUsage(1);
+
+{
+  auto *Detail = Root.addDetail("detail");
+  Detail->addUsage(1);
+  Detail->addChild("leaf")->addUsage(1);
+  auto *Child = Detail->addChild("child");
+  Child->addUsage(1);
+  Child->addChild("leaf")->addUsage(1);
+}
+
+{
+  auto *Child = Root.addChild("child");
+  Child->addUsage(1);
+  Child->addChild("leaf")->addUsage(1);
+}
+return Root;
+  };
+
+  EXPECT_THAT(collectComponents(AddNodes(MemoryTree())),
+  UnorderedElementsAre(WithNameAndSize("root", 1),
+   WithNameAndSize("root.leaf", 2),
+   WithNameAndSize("root.child", 2),
+   WithNameAndSize("root.child.leaf", 2)));
+
+  llvm::BumpPtrAllocator Alloc;
+  EXPECT_THAT(collectComponents(AddNodes(MemoryTree(&Alloc))),
+  UnorderedElementsAre(WithNameAndSize("root", 0),
+   WithNameAndSize("root.leaf", 1),
+   WithNameAndSize("root.detail", 1),
+   WithNameAndSize("ro

[PATCH] D88413: [clangd] Add a metric for tracking memory usage

2020-10-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet updated this revision to Diff 296112.
kadircet added a comment.

- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88413

Files:
  clang-tools-extra/clangd/support/Trace.cpp
  clang-tools-extra/clangd/support/Trace.h


Index: clang-tools-extra/clangd/support/Trace.h
===
--- clang-tools-extra/clangd/support/Trace.h
+++ clang-tools-extra/clangd/support/Trace.h
@@ -68,6 +68,10 @@
   const llvm::StringLiteral LabelName;
 };
 
+/// Convenient helper for collecting memory usage metrics from across multiple
+/// components. Results are recorded under a metric named "memory_usage".
+void recordMemoryUsage(double Value, llvm::StringRef ComponentName);
+
 /// A consumer of trace events and measurements. The events are produced by
 /// Spans and trace::log, the measurements are produced by Metrics::record.
 /// Implementations of this interface must be thread-safe.
Index: clang-tools-extra/clangd/support/Trace.cpp
===
--- clang-tools-extra/clangd/support/Trace.cpp
+++ clang-tools-extra/clangd/support/Trace.cpp
@@ -326,6 +326,13 @@
 Context EventTracer::beginSpan(llvm::StringRef Name, llvm::json::Object *Args) 
{
   return Context::current().clone();
 }
+
+void recordMemoryUsage(double Value, llvm::StringRef ComponentName) {
+  static constexpr Metric MemoryUsage("memory_usage", Metric::Value,
+  "component_name");
+
+  MemoryUsage.record(Value, ComponentName);
+}
 } // namespace trace
 } // namespace clangd
 } // namespace clang


Index: clang-tools-extra/clangd/support/Trace.h
===
--- clang-tools-extra/clangd/support/Trace.h
+++ clang-tools-extra/clangd/support/Trace.h
@@ -68,6 +68,10 @@
   const llvm::StringLiteral LabelName;
 };
 
+/// Convenient helper for collecting memory usage metrics from across multiple
+/// components. Results are recorded under a metric named "memory_usage".
+void recordMemoryUsage(double Value, llvm::StringRef ComponentName);
+
 /// A consumer of trace events and measurements. The events are produced by
 /// Spans and trace::log, the measurements are produced by Metrics::record.
 /// Implementations of this interface must be thread-safe.
Index: clang-tools-extra/clangd/support/Trace.cpp
===
--- clang-tools-extra/clangd/support/Trace.cpp
+++ clang-tools-extra/clangd/support/Trace.cpp
@@ -326,6 +326,13 @@
 Context EventTracer::beginSpan(llvm::StringRef Name, llvm::json::Object *Args) {
   return Context::current().clone();
 }
+
+void recordMemoryUsage(double Value, llvm::StringRef ComponentName) {
+  static constexpr Metric MemoryUsage("memory_usage", Metric::Value,
+  "component_name");
+
+  MemoryUsage.record(Value, ComponentName);
+}
 } // namespace trace
 } // namespace clangd
 } // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88414: [clangd] Introduce memory dumping to FileIndex, FileSymbols and BackgroundIndex

2020-10-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet updated this revision to Diff 296113.
kadircet added a comment.

- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88414

Files:
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/index/Background.h
  clang-tools-extra/clangd/index/FileIndex.cpp
  clang-tools-extra/clangd/index/FileIndex.h

Index: clang-tools-extra/clangd/index/FileIndex.h
===
--- clang-tools-extra/clangd/index/FileIndex.h
+++ clang-tools-extra/clangd/index/FileIndex.h
@@ -24,6 +24,7 @@
 #include "index/Relation.h"
 #include "index/Serialization.h"
 #include "index/Symbol.h"
+#include "support/MemoryTree.h"
 #include "support/Path.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/CompilationDatabase.h"
@@ -87,6 +88,8 @@
  DuplicateHandling DuplicateHandle = DuplicateHandling::PickOne,
  size_t *Version = nullptr);
 
+  void attachMemoryUsage(MemoryTree &MT) const;
+
 private:
   struct RefSlabAndCountReferences {
 std::shared_ptr Slab;
@@ -116,6 +119,8 @@
   /// `indexMainDecls`.
   void updateMain(PathRef Path, ParsedAST &AST);
 
+  void attachMemoryUsage(MemoryTree &MT) const;
+
 private:
   bool UseDex; // FIXME: this should be always on.
   bool CollectMainFileRefs;
Index: clang-tools-extra/clangd/index/FileIndex.cpp
===
--- clang-tools-extra/clangd/index/FileIndex.cpp
+++ clang-tools-extra/clangd/index/FileIndex.cpp
@@ -22,6 +22,7 @@
 #include "index/SymbolOrigin.h"
 #include "index/dex/Dex.h"
 #include "support/Logger.h"
+#include "support/MemoryTree.h"
 #include "support/Path.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/Index/IndexingAction.h"
@@ -373,6 +374,16 @@
   llvm_unreachable("Unknown clangd::IndexType");
 }
 
+void FileSymbols::attachMemoryUsage(MemoryTree &MT) const {
+  std::lock_guard Lock(Mutex);
+  for (const auto &SymSlab : SymbolsSnapshot)
+MT.addDetail(SymSlab.first())->addUsage(SymSlab.second->bytes());
+  for (const auto &RefSlab : RefsSnapshot)
+MT.addDetail(RefSlab.first())->addUsage(RefSlab.second.Slab->bytes());
+  for (const auto &RelSlab : SymbolsSnapshot)
+MT.addDetail(RelSlab.first())->addUsage(RelSlab.second->bytes());
+}
+
 FileIndex::FileIndex(bool UseDex, bool CollectMainFileRefs)
 : MergedIndex(&MainFileIndex, &PreambleIndex), UseDex(UseDex),
   CollectMainFileRefs(CollectMainFileRefs),
@@ -442,5 +453,11 @@
   }
 }
 
+void FileIndex::attachMemoryUsage(MemoryTree &MT) const {
+  PreambleSymbols.attachMemoryUsage(*MT.addChild("preamble_symbols"));
+  MT.addChild("preamble_index")->addUsage(PreambleIndex.estimateMemoryUsage());
+  MainFileSymbols.attachMemoryUsage(*MT.addChild("main_file_symbols"));
+  MT.addChild("main_file_index")->addUsage(MainFileIndex.estimateMemoryUsage());
+}
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/index/Background.h
===
--- clang-tools-extra/clangd/index/Background.h
+++ clang-tools-extra/clangd/index/Background.h
@@ -16,9 +16,11 @@
 #include "index/Index.h"
 #include "index/Serialization.h"
 #include "support/Context.h"
+#include "support/MemoryTree.h"
 #include "support/Path.h"
 #include "support/Threading.h"
 #include "support/ThreadsafeFS.h"
+#include "support/Trace.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/Threading.h"
@@ -172,6 +174,8 @@
 return Queue.blockUntilIdleForTest(TimeoutSeconds);
   }
 
+  void attachMemoryUsage(MemoryTree &MT) const;
+
 private:
   /// Represents the state of a single file when indexing was performed.
   struct ShardVersion {
Index: clang-tools-extra/clangd/index/Background.cpp
===
--- clang-tools-extra/clangd/index/Background.cpp
+++ clang-tools-extra/clangd/index/Background.cpp
@@ -414,5 +414,9 @@
   return {TUsToIndex.begin(), TUsToIndex.end()};
 }
 
+void BackgroundIndex::attachMemoryUsage(MemoryTree &MT) const {
+  IndexedSymbols.attachMemoryUsage(*MT.addChild("symbols"));
+  MT.addChild("index")->addUsage(estimateMemoryUsage());
+}
 } // namespace clangd
 } // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88415: [clangd] Introduce memory usage dumping to TUScheduler, for Preambles and ASTCache

2020-10-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet updated this revision to Diff 296114.
kadircet added a comment.

- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88415

Files:
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/unittests/ClangdTests.cpp

Index: clang-tools-extra/clangd/unittests/ClangdTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdTests.cpp
@@ -565,7 +565,9 @@
 }
 
 MATCHER_P4(Stats, Name, UsesMemory, PreambleBuilds, ASTBuilds, "") {
-  return arg.first() == Name && (arg.second.UsedBytes != 0) == UsesMemory &&
+  return arg.first() == Name &&
+ (arg.second.UsedBytesAST + arg.second.UsedBytesPreamble != 0) ==
+ UsesMemory &&
  std::tie(arg.second.PreambleBuilds, ASTBuilds) ==
  std::tie(PreambleBuilds, ASTBuilds);
 }
Index: clang-tools-extra/clangd/TUScheduler.h
===
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -14,6 +14,7 @@
 #include "GlobalCompilationDatabase.h"
 #include "index/CanonicalIncludes.h"
 #include "support/Function.h"
+#include "support/MemoryTree.h"
 #include "support/Path.h"
 #include "support/Threading.h"
 #include "llvm/ADT/Optional.h"
@@ -207,7 +208,8 @@
   ~TUScheduler();
 
   struct FileStats {
-std::size_t UsedBytes = 0;
+std::size_t UsedBytesAST = 0;
+std::size_t UsedBytesPreamble = 0;
 unsigned PreambleBuilds = 0;
 unsigned ASTBuilds = 0;
   };
@@ -311,6 +313,8 @@
   // FIXME: move to ClangdServer via createProcessingContext.
   static llvm::Optional getFileBeingProcessedInContext();
 
+  void attachMemoryUsage(MemoryTree &MT) const;
+
 private:
   const GlobalCompilationDatabase &CDB;
   Options Opts;
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -56,6 +56,7 @@
 #include "support/Cancellation.h"
 #include "support/Context.h"
 #include "support/Logger.h"
+#include "support/MemoryTree.h"
 #include "support/Path.h"
 #include "support/Threading.h"
 #include "support/Trace.h"
@@ -164,6 +165,17 @@
 return llvm::Optional>(std::move(V));
   }
 
+  void attachMemoryUsage(MemoryTree &MT) const {
+std::lock_guard Lock(Mut);
+for (const auto &Elem : LRU) {
+  auto &SM = Elem.second->getSourceManager();
+  MT.addDetail(
+// FIXME: Store the filename directly in the cache or ParsedAST.
+SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName())
+  ->addUsage(Elem.second->getUsedBytes());
+}
+  }
+
 private:
   using KVPair = std::pair>;
 
@@ -171,7 +183,7 @@
 return llvm::find_if(LRU, [K](const KVPair &P) { return P.first == K; });
   }
 
-  std::mutex Mut;
+  mutable std::mutex Mut;
   unsigned MaxRetainedASTs;
   /// Items sorted in LRU order, i.e. first item is the most recently accessed
   /// one.
@@ -932,9 +944,9 @@
   // Note that we don't report the size of ASTs currently used for processing
   // the in-flight requests. We used this information for debugging purposes
   // only, so this should be fine.
-  Result.UsedBytes = IdleASTs.getUsedBytes(this);
+  Result.UsedBytesAST = IdleASTs.getUsedBytes(this);
   if (auto Preamble = getPossiblyStalePreamble())
-Result.UsedBytes += Preamble->Preamble.getSize();
+Result.UsedBytesPreamble = Preamble->Preamble.getSize();
   return Result;
 }
 
@@ -1429,5 +1441,15 @@
   return P;
 }
 
+void TUScheduler::attachMemoryUsage(MemoryTree &MT) const {
+  IdleASTs->attachMemoryUsage(*MT.addChild("ast_cahe"));
+  // Otherwise preambles are stored on disk and we only keep filename in memory.
+  if (Opts.StorePreamblesInMemory) {
+MemoryTree &Preambles = *MT.addChild("preambles");
+for (const auto &Elem : fileStats())
+  Preambles.addDetail(Elem.first())
+  ->addUsage(Elem.second.UsedBytesPreamble);
+  }
+}
 } // namespace clangd
 } // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88417: [clangd] Record memory usages after each notification

2020-10-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet updated this revision to Diff 296115.
kadircet added a comment.

- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88417

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/unittests/ClangdTests.cpp

Index: clang-tools-extra/clangd/unittests/ClangdTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdTests.cpp
@@ -17,6 +17,7 @@
 #include "TestFS.h"
 #include "TestTU.h"
 #include "URI.h"
+#include "support/MemoryTree.h"
 #include "support/Path.h"
 #include "support/Threading.h"
 #include "clang/Config/config.h"
@@ -27,6 +28,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Allocator.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
@@ -48,6 +50,7 @@
 namespace {
 
 using ::testing::AllOf;
+using ::testing::Contains;
 using ::testing::ElementsAre;
 using ::testing::Field;
 using ::testing::Gt;
@@ -1236,6 +1239,31 @@
   EXPECT_FALSE(DiagConsumer.HadDiagsInLastCallback);
 }
 
+TEST(ClangdServer, MemoryUsageTest) {
+  MockFS FS;
+  MockCompilationDatabase CDB;
+  ClangdServer Server(CDB, FS, ClangdServer::optsForTest());
+
+  std::vector SeenComponents;
+  auto CB = [&SeenComponents](size_t Size, llvm::StringRef CompName) {
+SeenComponents.emplace_back(CompName.str());
+  };
+
+  llvm::BumpPtrAllocator Alloc;
+  MemoryTree MT(&Alloc);
+  Server.attachMemoryUsage(MT);
+  MT.traverseTree(CB, "clangd_server");
+  EXPECT_THAT(SeenComponents, Contains("clangd_server"));
+  SeenComponents.clear();
+
+  auto FooCpp = testPath("foo.cpp");
+  Server.addDocument(FooCpp, "");
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
+  Server.attachMemoryUsage(MT);
+  MT.traverseTree(CB, "clangd_server");
+  EXPECT_THAT(SeenComponents,
+  Contains("clangd_server.tuscheduler.preambles." + FooCpp));
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -25,6 +25,7 @@
 #include "refactor/Tweak.h"
 #include "support/Cancellation.h"
 #include "support/Function.h"
+#include "support/MemoryTree.h"
 #include "support/ThreadsafeFS.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Core/Replacement.h"
@@ -337,6 +338,9 @@
   LLVM_NODISCARD bool
   blockUntilIdleForTest(llvm::Optional TimeoutSeconds = 10);
 
+  /// Builds a nested representation of memory used by components.
+  void attachMemoryUsage(MemoryTree &MT) const;
+
 private:
   void formatCode(PathRef File, llvm::StringRef Code,
   ArrayRef Ranges,
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -28,6 +28,7 @@
 #include "refactor/Tweak.h"
 #include "support/Logger.h"
 #include "support/Markup.h"
+#include "support/MemoryTree.h"
 #include "support/ThreadsafeFS.h"
 #include "support/Trace.h"
 #include "clang/Format/Format.h"
@@ -833,5 +834,12 @@
   BackgroundIdx->blockUntilIdleForTest(TimeoutSeconds));
 }
 
+void ClangdServer::attachMemoryUsage(MemoryTree &MT) const {
+  if (DynamicIdx)
+DynamicIdx->attachMemoryUsage(*MT.addChild("dynamic_index"));
+  if (BackgroundIdx)
+BackgroundIdx->attachMemoryUsage(*MT.addChild("background_index"));
+  WorkScheduler.attachMemoryUsage(*MT.addChild("tuscheduler"));
+}
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -163,14 +163,23 @@
 log("<-- {0}", Method);
 if (Method == "exit")
   return false;
-if (!Server.Server)
+if (!Server.Server) {
   elog("Notification {0} before initialization", Method);
-else if (Method == "$/cancelRequest")
+  return true;
+}
+if (Method == "$/cancelRequest")
   onCancel(std::move(Params));
 else if (auto Handler = Notifications.lookup(Method))
   Handler(std::move(Params));
 else
   log("unhandled notification {0}", Method);
+
+// Record memory usage after each memory usage. This currently takes <1ms,
+// so it is safe to do frequently.
+trace::Span Tracer("RecordMemoryUsage");
+MemoryTree MT;
+Server.Server->attachMemoryUsage(MT);
+MT.traverseTree(trace::recordMemoryUsage, "clangd_server");
 return true;
   }

[PATCH] D87256: [clangd] Avoid relations being overwritten in a header shard

2020-10-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits

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

thanks! (and sorry for taking too long on this one.)




Comment at: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp:232
 
+TEST_F(BackgroundIndexTest, RelationsMultiFile) {
+  MockFS FS;

kadircet wrote:
> nridge wrote:
> > kadircet wrote:
> > > do we still need this test?
> > I think it's still useful, yes. If someone makes a change to the way 
> > relations are stored in the future, they could regress this use case 
> > without a test specifically for it.
> this one was marked as resolved but i still don't see the reasoning. can we 
> test this in fileindextests instead?
> 
> we already test the sharding logic, we need to test the merging logic now. 
> can we rather test it at FileSymbols layer directly? or is there something 
> extra that i am misisng?
okay, i still think it is better to test on FileSymbols layer but not that 
important.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87256

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

[PATCH] D87962: [clang] Change the multi-character character constants from extension to implementation-defined.

2020-10-05 Thread Chuyang Chen via Phabricator via cfe-commits

nomanous updated this revision to Diff 296118.
nomanous added a comment.

In this revision I delete the useless main function in the new test case and 
rename ext_multichar_character_literal & ext_four_char_character_literal to 
warn_multichar_character_literal & warn_four_char_character_literal.


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

https://reviews.llvm.org/D87962

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/LiteralSupport.cpp
  clang/test/Lexer/constants.c
  clang/test/Lexer/multi-char-constants.c


Index: clang/test/Lexer/multi-char-constants.c
===
--- /dev/null
+++ clang/test/Lexer/multi-char-constants.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wfour-char-constants 
-pedantic-errors %s
+
+int x = 'ab'; // expected-warning {{multi-character character constant}}
+int y = 'abcd'; // expected-warning {{multi-character character constant}}
Index: clang/test/Lexer/constants.c
===
--- clang/test/Lexer/constants.c
+++ clang/test/Lexer/constants.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -pedantic -ftrigraphs %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wfour-char-constants -pedantic 
-ftrigraphs %s
 
 int x = 00080;  // expected-error {{invalid digit}}
 
Index: clang/lib/Lex/LiteralSupport.cpp
===
--- clang/lib/Lex/LiteralSupport.cpp
+++ clang/lib/Lex/LiteralSupport.cpp
@@ -1373,9 +1373,9 @@
 if (isWide())
   PP.Diag(Loc, diag::warn_extraneous_char_constant);
 else if (isAscii() && NumCharsSoFar == 4)
-  PP.Diag(Loc, diag::ext_four_char_character_literal);
+  PP.Diag(Loc, diag::warn_four_char_character_literal);
 else if (isAscii())
-  PP.Diag(Loc, diag::ext_multichar_character_literal);
+  PP.Diag(Loc, diag::warn_multichar_character_literal);
 else
   PP.Diag(Loc, diag::err_multichar_utf_character_literal);
 IsMultiChar = true;
Index: clang/include/clang/Basic/DiagnosticLexKinds.td
===
--- clang/include/clang/Basic/DiagnosticLexKinds.td
+++ clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -104,10 +104,10 @@
   "raw string literals are incompatible with C++98">,
   InGroup, DefaultIgnore;
 
-def ext_multichar_character_literal : ExtWarn<
+def warn_multichar_character_literal : Warning<
   "multi-character character constant">, InGroup;
-def ext_four_char_character_literal : Extension<
-  "multi-character character constant">, InGroup;
+def warn_four_char_character_literal : Warning<
+  "multi-character character constant">, InGroup, 
DefaultIgnore;
 
 
 // Unicode and UCNs


Index: clang/test/Lexer/multi-char-constants.c
===
--- /dev/null
+++ clang/test/Lexer/multi-char-constants.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wfour-char-constants -pedantic-errors %s
+
+int x = 'ab'; // expected-warning {{multi-character character constant}}
+int y = 'abcd'; // expected-warning {{multi-character character constant}}
Index: clang/test/Lexer/constants.c
===
--- clang/test/Lexer/constants.c
+++ clang/test/Lexer/constants.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -pedantic -ftrigraphs %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wfour-char-constants -pedantic -ftrigraphs %s
 
 int x = 00080;  // expected-error {{invalid digit}}
 
Index: clang/lib/Lex/LiteralSupport.cpp
===
--- clang/lib/Lex/LiteralSupport.cpp
+++ clang/lib/Lex/LiteralSupport.cpp
@@ -1373,9 +1373,9 @@
 if (isWide())
   PP.Diag(Loc, diag::warn_extraneous_char_constant);
 else if (isAscii() && NumCharsSoFar == 4)
-  PP.Diag(Loc, diag::ext_four_char_character_literal);
+  PP.Diag(Loc, diag::warn_four_char_character_literal);
 else if (isAscii())
-  PP.Diag(Loc, diag::ext_multichar_character_literal);
+  PP.Diag(Loc, diag::warn_multichar_character_literal);
 else
   PP.Diag(Loc, diag::err_multichar_utf_character_literal);
 IsMultiChar = true;
Index: clang/include/clang/Basic/DiagnosticLexKinds.td
===
--- clang/include/clang/Basic/DiagnosticLexKinds.td
+++ clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -104,10 +104,10 @@
   "raw string literals are incompatible with C++98">,
   InGroup, DefaultIgnore;
 
-def ext_multichar_character_literal : ExtWarn<
+def warn_multichar_character_literal : Warning<
   "multi-character character constant">, InGroup;
-def ext_four_char_character_literal : Extension<
-  "multi-character character constant">, InGroup;
+def warn_four_char_character_literal : Warning<
+  "multi-character character c

[PATCH] D88810: [clangd] Add isKeyword function.

2020-10-05 Thread Sam McCall via Phabricator via cfe-commits

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



Comment at: clang-tools-extra/clangd/SourceCode.cpp:638
+  // Keywords are initialized in constructor.
+  clang::IdentifierTable KeywordsTable(LangOpts);
+  return KeywordsTable.find(NewName) != KeywordsTable.end();

this builds a pretty decent sized hashtable for each call, because LangOpts 
isn't constant.

We could consider exposing getKeywordStatus from IdentifierTable.h, then we 
could build a StringMap (once, statically) and then call 
getKeywordStatus to apply the per-langopts logic.
In fact that StringMap might also be generally nice to expose in 
TokenKinds.h... as long as there are no conflicts.

Or we could have cache map from LangOpts to IdentifierTable, since we won't see 
many sets in practice.

Or maybe it doesn't matter, I suppose!



Comment at: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp:794
+  LangOptions LangOpts;
+  LangOpts.CPlusPlus11 = true;
+  EXPECT_TRUE(isKeyword("int", LangOpts));

demonstrate some sensitivity to langopts?

e.g. set CPlusPlus20 to true then check co_await


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88810

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

[PATCH] D87989: [Flang][Driver] Add infrastructure for basic frontend actions and file I/O

2020-10-05 Thread Andrzej Warzynski via Phabricator via cfe-commits

awarzynski added a comment.

LGTM, thanks for working on this!

As this is a fairly large change, could you wait for one more reviewer to 
approve? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87989

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

[PATCH] D87774: [flang] Introduce DiagnosticConsumer classes in libflangFrontend

2020-10-05 Thread Andrzej Warzynski via Phabricator via cfe-commits

awarzynski added a comment.

Ping :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87774

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

[PATCH] D87350: [AST][RecoveryExpr] Fix a crash on undeduced type.

2020-10-05 Thread Haojian Wu via Phabricator via cfe-commits

hokein updated this revision to Diff 296120.
hokein added a comment.

address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87350

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/AST/ast-dump-recovery.cpp
  clang/test/SemaCXX/recovery-expr-type.cpp


Index: clang/test/SemaCXX/recovery-expr-type.cpp
===
--- clang/test/SemaCXX/recovery-expr-type.cpp
+++ clang/test/SemaCXX/recovery-expr-type.cpp
@@ -105,3 +105,9 @@
 int v = arr(); // expected-error {{array types cannot be value-initialized}} \
   expected-error {{cannot initialize a variable of type 'int' 
with an rvalue of type 'test8::arr'}}
 }
+
+namespace test9 {
+auto f(); // expected-note {{candidate function not viable}}
+// verify no crash on evaluating the size of undeduced auto type.
+static_assert(sizeof(f(1)), ""); // expected-error {{no matching function for 
call to 'f'}}
+}
Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -126,6 +126,12 @@
 // CHECK-NEXT:|   `-UnresolvedLookupExpr {{.*}} 'invalid'
 struct alignas(invalid()) Aligned {};
 
+auto f();
+int f(double);
+// CHECK:  VarDecl {{.*}} unknown_type_call 'int'
+// CHECK-NEXT: `-RecoveryExpr {{.*}} ''
+int unknown_type_call = f(0, 0);
+
 void InvalidInitalizer(int x) {
   struct Bar { Bar(); };
   // CHECK: `-VarDecl {{.*}} a1 'Bar'
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -12880,7 +12880,12 @@
 for (const auto &C : CS)
   ConsiderCandidate(C);
 
-  return Result.getValueOr(QualType());
+  if (!Result)
+return QualType();
+  auto Value = Result.getValue();
+  if (Value.isNull() || Value->isUndeducedType())
+return QualType();
+  return Value;
 }
 
 /// FinishOverloadedCallExpr - given an OverloadCandidateSet, builds and 
returns


Index: clang/test/SemaCXX/recovery-expr-type.cpp
===
--- clang/test/SemaCXX/recovery-expr-type.cpp
+++ clang/test/SemaCXX/recovery-expr-type.cpp
@@ -105,3 +105,9 @@
 int v = arr(); // expected-error {{array types cannot be value-initialized}} \
   expected-error {{cannot initialize a variable of type 'int' with an rvalue of type 'test8::arr'}}
 }
+
+namespace test9 {
+auto f(); // expected-note {{candidate function not viable}}
+// verify no crash on evaluating the size of undeduced auto type.
+static_assert(sizeof(f(1)), ""); // expected-error {{no matching function for call to 'f'}}
+}
Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -126,6 +126,12 @@
 // CHECK-NEXT:|   `-UnresolvedLookupExpr {{.*}} 'invalid'
 struct alignas(invalid()) Aligned {};
 
+auto f();
+int f(double);
+// CHECK:  VarDecl {{.*}} unknown_type_call 'int'
+// CHECK-NEXT: `-RecoveryExpr {{.*}} ''
+int unknown_type_call = f(0, 0);
+
 void InvalidInitalizer(int x) {
   struct Bar { Bar(); };
   // CHECK: `-VarDecl {{.*}} a1 'Bar'
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -12880,7 +12880,12 @@
 for (const auto &C : CS)
   ConsiderCandidate(C);
 
-  return Result.getValueOr(QualType());
+  if (!Result)
+return QualType();
+  auto Value = Result.getValue();
+  if (Value.isNull() || Value->isUndeducedType())
+return QualType();
+  return Value;
 }
 
 /// FinishOverloadedCallExpr - given an OverloadCandidateSet, builds and returns
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87350: [AST][RecoveryExpr] Fix a crash on undeduced type.

2020-10-05 Thread Haojian Wu via Phabricator via cfe-commits

hokein added inline comments.



Comment at: clang/lib/Sema/SemaOverload.cpp:12880
 
   return Result.getValueOr(QualType());
 }

sammccall wrote:
> hokein wrote:
> > sammccall wrote:
> > > Wouldn't we be better to handle undeduced type here rather than in the 
> > > loop?
> > > 
> > > Not sure if it practically matters, but given: `auto f(); double f(int); 
> > > f(0,0);`
> > > the code in this patch will discard auto and yield `double`, while 
> > > treating this as multiple candidates --> unknown return type seems more 
> > > correct.
> > > 
> > I feel like yield `double` is better in your given example -- in this case, 
> > double is the only candidate, so it is reasonable to preserve the `double` 
> > type.
> There are two overloads, one with `double` and one with an unknown type. 
> They're both candidates for the function being called, and I'm not sure we 
> have any reason to believe the first is more likely.
> 
> Two counterarguments I can think of:
>  - the unknown type is probably also double, as overloads tend to have the 
> same return type
>  - the cost of being wrong isn't so high, so it's OK to not be totally sure
> 
> And maybe there are others... these are probably OK justifications but subtle 
> so I'd like to see a comment tothat effect.
thanks, I think the behavior you suggest is more reasonable -- in the given 
example, go-to-def will yield two candidates, it is wired to catch one of the 
return type (not the other).

Addressed it.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87350

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

[PATCH] D88411: [clangd] Introduce MemoryTrees

2020-10-05 Thread Sam McCall via Phabricator via cfe-commits

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

Looks good - some concerns that the traversal isn't flexible enough but we can 
address that later once it's used.

Comment at: clang-tools-extra/clangd/support/MemoryTree.cpp:10
+std::string ComponentName) const {
+  CB(Size, ComponentName);
+  if (!ComponentName.empty())

If you're only going to pass one size, I'd think total (rather than self) is 
the most meaningful.
This is easier if you do postorder of course...

Comment at: clang-tools-extra/clangd/support/MemoryTree.cpp:11
+  CB(Size, ComponentName);
+  if (!ComponentName.empty())
+ComponentName += '.';

if you pass ComponentName as a StringRef you can avoid lots of allocations by 
using the same underlying string (which will grow and shrink, but not 
reallocate much).
You'll need a recursion helper with a different signature though.

Comment at: clang-tools-extra/clangd/support/MemoryTree.h:33
+  /// If Alloc is nullptr, tree is in brief mode and will ignore detail edges.
+  MemoryTree(llvm::BumpPtrAllocator *Alloc = nullptr) : Alloc(Alloc) {}
+

nit: I'd call this parameter DetailAlloc to hint at what's stored there

Comment at: clang-tools-extra/clangd/support/MemoryTree.h:41
+  MemoryTree *addDetail(llvm::StringRef Name) {
+return Alloc ? &createChild(llvm::StringSaver(*Alloc).save(Name)) : this;
+  }

nit: `Name.copy(*Alloc)` is a little more direct

Comment at: clang-tools-extra/clangd/support/MemoryTree.h:49
+  /// with dots(".").
+  void traverseTree(llvm::function_ref

nit: need to be explicit whether this is self or subtree (or could pass both)

Comment at: clang-tools-extra/clangd/support/MemoryTree.h:31
+  /// detail to root, and doesn't report any descendants.
+  void traverseTree(bool CollapseDetails,
+llvm::function_ref sammccall wrote:
> > sammccall wrote:
> > > In earlier discussions we talked about avoiding the cost of *assembling* 
> > > the detailed tree, not just summarizing it at output time.
> > > 
> > > This requires `CollapseDetails` to be passed into the profiling function, 
> > > which in turn suggests making it part of a tree and passing a reference 
> > > to the tree in. e.g. an API like
> > > 
> > > ```
> > > class MemoryTree {
> > >   MemoryTree(bool Detailed);
> > >   MemoryTree* addChild(StringLiteral); // no copy
> > >   MemoryTree* addDetail(StringRef); // returns `this` unless detailed
> > >   void addUsage(size_t);
> > > }
> > > ```
> > It's hard to evaluate a tree-traversal API without its usages... how will 
> > this be used?
> you can find some in https://reviews.llvm.org/D88414
I guess you mean D88417?

This works well when exporting all nodes somewhere keyed by dotted path, but 
it's pretty awkward+inefficient if you want to - totally hypothetical example 
here - dump the memory tree as a JSON structure over RPC.

So I think we need a more flexible API, at which point... maybe we should punt, 
expose `const DenseMap &children() const`, and move the 
traverse implementation to the metrics exporter?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88411

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

[PATCH] D87774: [flang] Introduce DiagnosticConsumer classes in libflangFrontend

2020-10-05 Thread Caroline via Phabricator via cfe-commits

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

Thank you @awarzynski for working on this.
It is good because now we do not depend on clang frontend to build flang 
frontend.
LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87774

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

[PATCH] D87350: [AST][RecoveryExpr] Fix a crash on undeduced type.

2020-10-05 Thread Sam McCall via Phabricator via cfe-commits

sammccall accepted this revision.
sammccall added a comment.

LG, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87350

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

[PATCH] D88411: [clangd] Introduce MemoryTrees

2020-10-05 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/support/MemoryTree.h:35
+
+  /// No copy of the \p Name.
+  MemoryTree *addChild(llvm::StringLiteral Name) { return &createChild(Name); }

Maybe mention that child pointers are invalidated by future addChild/addDetail



Comment at: clang-tools-extra/clangd/support/MemoryTree.h:36
+  /// No copy of the \p Name.
+  MemoryTree *addChild(llvm::StringLiteral Name) { return &createChild(Name); }
+

actually, why do these return pointers rather than references?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88411

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

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

2020-10-05 Thread Raphael Isemann via Phabricator via cfe-commits

teemperor accepted this revision.
teemperor 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/D88665/new/

https://reviews.llvm.org/D88665

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

[PATCH] D88814: [clangd] Enable partial namespace matches for workspace symbols

2020-10-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits

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

This will enable queries like "clangd::" to find symbols under clangd
namespace, without requiring full "clang::clangd::" qualification.

Since Fuzzyfind performs the search under all scopes and only boosts the symbols
from relevant namespaces, we might get symbols from non-matching namespaces.
This patch chooses to drop those as they clearly do not match the query.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88814

Files:
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp


Index: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
@@ -141,7 +141,10 @@
   EXPECT_THAT(getSymbols(TU, "::"), ElementsAre(QName("ans1")));
   EXPECT_THAT(getSymbols(TU, "::a"), ElementsAre(QName("ans1")));
   EXPECT_THAT(getSymbols(TU, "ans1::"),
-  UnorderedElementsAre(QName("ans1::ai1"), QName("ans1::ans2")));
+  UnorderedElementsAre(QName("ans1::ai1"), QName("ans1::ans2"),
+   QName("ans1::ans2::ai2")));
+  EXPECT_THAT(getSymbols(TU, "ans2::"),
+  UnorderedElementsAre(QName("ans1::ans2::ai2")));
   EXPECT_THAT(getSymbols(TU, "::ans1"), ElementsAre(QName("ans1")));
   EXPECT_THAT(getSymbols(TU, "::ans1::"),
   UnorderedElementsAre(QName("ans1::ai1"), QName("ans1::ans2")));
Index: clang-tools-extra/clangd/FindSymbols.cpp
===
--- clang-tools-extra/clangd/FindSymbols.cpp
+++ clang-tools-extra/clangd/FindSymbols.cpp
@@ -76,20 +76,26 @@
   FuzzyFindRequest Req;
   Req.Query = std::string(Names.second);
 
-  // FuzzyFind doesn't want leading :: qualifier
-  bool IsGlobalQuery = Names.first.consume_front("::");
-  // Restrict results to the scope in the query string if present (global or
-  // not).
-  if (IsGlobalQuery || !Names.first.empty())
+  // FuzzyFind doesn't want leading :: qualifier. Also limit the query to
+  // specific namespace if it is fully-qualified.
+  Req.AnyScope = !Names.first.consume_front("::");
+  // Boost symbols from desired namespace.
+  if (!Req.AnyScope || !Names.first.empty())
 Req.Scopes = {std::string(Names.first)};
-  else
-Req.AnyScope = true;
   if (Limit)
 Req.Limit = Limit;
   TopN Top(
   Req.Limit ? *Req.Limit : std::numeric_limits::max());
   FuzzyMatcher Filter(Req.Query);
-  Index->fuzzyFind(Req, [HintPath, &Top, &Filter](const Symbol &Sym) {
+  Index->fuzzyFind(Req, [HintPath, &Top, &Filter, &Names](const Symbol &Sym) {
+std::string Scope = std::string(Sym.Scope);
+llvm::StringRef ScopeRef = Scope;
+// Fuzzfind might return symbols from irrelevant namespaces if query was 
not
+// fully-qualified, drop those.
+if (!ScopeRef.contains(Names.first))
+  return;
+ScopeRef.consume_back("::");
+
 auto Loc = symbolToLocation(Sym, HintPath);
 if (!Loc) {
   log("Workspace symbols: {0}", Loc.takeError());
@@ -97,9 +103,6 @@
 }
 
 SymbolKind SK = indexSymbolKindToSymbolKind(Sym.SymInfo.Kind);
-std::string Scope = std::string(Sym.Scope);
-llvm::StringRef ScopeRef = Scope;
-ScopeRef.consume_back("::");
 SymbolInformation Info = {(Sym.Name + 
Sym.TemplateSpecializationArgs).str(),
   SK, *Loc, std::string(ScopeRef)};
 


Index: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
@@ -141,7 +141,10 @@
   EXPECT_THAT(getSymbols(TU, "::"), ElementsAre(QName("ans1")));
   EXPECT_THAT(getSymbols(TU, "::a"), ElementsAre(QName("ans1")));
   EXPECT_THAT(getSymbols(TU, "ans1::"),
-  UnorderedElementsAre(QName("ans1::ai1"), QName("ans1::ans2")));
+  UnorderedElementsAre(QName("ans1::ai1"), QName("ans1::ans2"),
+   QName("ans1::ans2::ai2")));
+  EXPECT_THAT(getSymbols(TU, "ans2::"),
+  UnorderedElementsAre(QName("ans1::ans2::ai2")));
   EXPECT_THAT(getSymbols(TU, "::ans1"), ElementsAre(QName("ans1")));
   EXPECT_THAT(getSymbols(TU, "::ans1::"),
   UnorderedElementsAre(QName("ans1::ai1"), QName("ans1::ans2")));
Index: clang-tools-extra/clangd/FindSymbols.cpp
===
--- clang-tools-extra/clangd/FindSymbols.cpp
+++ clang-tools-extra/clangd/FindSymbols.cpp
@@ -76,20 +76,26 @@
   FuzzyFindRequest Req;
   Req.Query = std::st

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

2020-10-05 Thread Raphael Isemann via Phabricator via cfe-commits

teemperor added a comment.

> I am not sure how much do you guys value these diags in LLDB

I think we don't even display them most of the time (IIUC they go to the 
diagnostic engine which is not always hooked up for the ASTs in LLDB as only a 
few actually come from Clang).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88665

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

[PATCH] D88414: [clangd] Introduce memory dumping to FileIndex, FileSymbols and BackgroundIndex

2020-10-05 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/index/Background.cpp:419
+  IndexedSymbols.attachMemoryUsage(*MT.addChild("symbols"));
+  MT.addChild("index")->addUsage(estimateMemoryUsage());
+}

Hmm, we should be careful now, estimateMemoryUsage() is "total" while addUsage 
takes "self".

We're not overriding estimateMemoryUsage here to include IndexedSymbols, but 
this is probably just an oversight. Consider calling 
SwapIndex::estimateMemoryUsage() explicitly to guard/communicate against this.

In the long-run, we should put attachMemoryUsage on SymbolIndex and either 
remove estimateMemoryUsage() or make it non-virtual (`MemoryTree R; 
attachMemoryUsage(R); return R.total()`)



Comment at: clang-tools-extra/clangd/index/Background.h:177
 
+  void attachMemoryUsage(MemoryTree &MT) const;
+

As a conventional name for these, profile() or so might be slightly more 
evocative. And I think we should push this into progressively more places (that 
currently have ad-hoc "estimate" accessors) so the brevity is reasonable I 
think.

(If not, I'd even slightly prefer "record"/"measure" over "attach" as 
emphasizing the high-level operation rather than the data structure used can 
help readability)



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:380
+  for (const auto &SymSlab : SymbolsSnapshot)
+MT.addDetail(SymSlab.first())->addUsage(SymSlab.second->bytes());
+  for (const auto &RefSlab : RefsSnapshot)

addChild("symbols"/"refs"/"relations")?
This seems like really useful information



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:457
+void FileIndex::attachMemoryUsage(MemoryTree &MT) const {
+  PreambleSymbols.attachMemoryUsage(*MT.addChild("preamble_symbols"));
+  MT.addChild("preamble_index")->addUsage(PreambleIndex.estimateMemoryUsage());

addChild("preamble").addChild("symbols")?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88414

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

[PATCH] D88411: [clangd] Introduce MemoryTrees

2020-10-05 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/support/MemoryTree.h:36
+  /// No copy of the \p Name.
+  MemoryTree *addChild(llvm::StringLiteral Name) { return &createChild(Name); }
+

sammccall wrote:
> actually, why do these return pointers rather than references?
reading call sites, `child()` might be both more fluent and more accurate than 
`addChild` - we're not calling it for the side effect and there may or may not 
be one.



Comment at: clang-tools-extra/clangd/support/MemoryTree.h:49
+  /// with dots(".").
+  void traverseTree(llvm::function_ref

sammccall wrote:
> nit: need to be explicit whether this is self or subtree (or could pass both)
I think it'd be worth having `size_t total()` with a comment that it traverses 
the tree.

We have places where we only want this info (e.g. log messages) and it's 
probably nice in unit tests too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88411

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

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

2020-10-05 Thread Simon Moll via Phabricator via cfe-commits

simoll updated this revision to Diff 296135.
simoll added a comment.
Herald added subscribers: llvm-commits, nikic, pengfei, hiraditya, mgorny.
Herald added a project: LLVM.

fixed for privatized ElementCount members.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  llvm/include/llvm/Analysis/TargetTransformInfo.h
  llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
  llvm/include/llvm/CodeGen/ExpandVectorPredication.h
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/IR/IntrinsicInst.h
  llvm/include/llvm/InitializePasses.h
  llvm/lib/Analysis/TargetTransformInfo.cpp
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/ExpandVectorPredication.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/IR/IntrinsicInst.cpp
  llvm/test/CodeGen/AArch64/O0-pipeline.ll
  llvm/test/CodeGen/AArch64/O3-pipeline.ll
  llvm/test/CodeGen/ARM/O3-pipeline.ll
  llvm/test/CodeGen/Generic/expand-vp.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/tools/llc/llc.cpp
  llvm/tools/opt/opt.cpp

Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -578,6 +578,7 @@
   initializePostInlineEntryExitInstrumenterPass(Registry);
   initializeUnreachableBlockElimLegacyPassPass(Registry);
   initializeExpandReductionsPass(Registry);
+  initializeExpandVectorPredicationPass(Registry);
   initializeWasmEHPreparePass(Registry);
   initializeWriteBitcodePassPass(Registry);
   initializeHardwareLoopsPass(Registry);
Index: llvm/tools/llc/llc.cpp
===
--- llvm/tools/llc/llc.cpp
+++ llvm/tools/llc/llc.cpp
@@ -318,6 +318,7 @@
   initializeVectorization(*Registry);
   initializeScalarizeMaskedMemIntrinPass(*Registry);
   initializeExpandReductionsPass(*Registry);
+  initializeExpandVectorPredicationPass(*Registry);
   initializeHardwareLoopsPass(*Registry);
   initializeTransformUtils(*Registry);
 
Index: llvm/test/CodeGen/X86/O0-pipeline.ll
===
--- llvm/test/CodeGen/X86/O0-pipeline.ll
+++ llvm/test/CodeGen/X86/O0-pipeline.ll
@@ -24,6 +24,7 @@
 ; CHECK-NEXT:   Lower constant intrinsics
 ; CHECK-NEXT:   Remove unreachable blocks from the CFG
 ; CHECK-NEXT:   Instrument function entry/exit with calls to e.g. mcount() (post inlining)
+; CHECK-NEXT:   Expand vector predication intrinsics
 ; CHECK-NEXT:   Scalarize Masked Memory Intrinsics
 ; CHECK-NEXT:   Expand reduction intrinsics
 ; CHECK-NEXT:   Expand indirectbr instructions
Index: llvm/test/CodeGen/Generic/expand-vp.ll
===
--- /dev/null
+++ llvm/test/CodeGen/Generic/expand-vp.ll
@@ -0,0 +1,84 @@
+; RUN: opt --expand-vec-pred -S < %s | FileCheck %s
+
+; All VP intrinsics have to be lowered into non-VP ops
+; CHECK-NOT: {{call.* @llvm.vp.add}}
+; CHECK-NOT: {{call.* @llvm.vp.sub}}
+; CHECK-NOT: {{call.* @llvm.vp.mul}}
+; CHECK-NOT: {{call.* @llvm.vp.sdiv}}
+; CHECK-NOT: {{call.* @llvm.vp.srem}}
+; CHECK-NOT: {{call.* @llvm.vp.udiv}}
+; CHECK-NOT: {{call.* @llvm.vp.urem}}
+; CHECK-NOT: {{call.* @llvm.vp.and}}
+; CHECK-NOT: {{call.* @llvm.vp.or}}
+; CHECK-NOT: {{call.* @llvm.vp.xor}}
+; CHECK-NOT: {{call.* @llvm.vp.ashr}}
+; CHECK-NOT: {{call.* @llvm.vp.lshr}}
+; CHECK-NOT: {{call.* @llvm.vp.shl}}
+
+define void @test_vp_int_v8(<8 x i32> %i0, <8 x i32> %i1, <8 x i32> %i2, <8 x i32> %f3, <8 x i1> %m, i32 %n) {
+  %r0 = call <8 x i32> @llvm.vp.add.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %r1 = call <8 x i32> @llvm.vp.sub.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %r2 = call <8 x i32> @llvm.vp.mul.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %r3 = call <8 x i32> @llvm.vp.sdiv.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %r4 = call <8 x i32> @llvm.vp.srem.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %r5 = call <8 x i32> @llvm.vp.udiv.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %r6 = call <8 x i32> @llvm.vp.urem.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %r7 = call <8 x i32> @llvm.vp.and.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %r8 = call <8 x i32> @llvm.vp.or.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %r9 = call <8 x i32> @llvm.vp.xor.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %rA = call <8 x i32> @llvm.vp.ashr.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %rB = call <8 x i32> @llvm.vp.lshr.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %rC = call <8 x i32> @llvm.vp.shl.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  ret void
+}
+
+; fixed-width vectors
+; integer arith
+declare <8 x i32> @llvm.vp.add.v8i32(<8 x i32>, <8 x i32>, <8 x i1>, i32)
+declare <8 x i32> @llvm.vp.sub.v8i32(<

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

2020-10-05 Thread Simon Moll via Phabricator via cfe-commits

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

(wrong Diff attached in earlier update)

- rebased


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/Basic/TargetInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/Hexagon.h
  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/AST/ast-print-vector-size-bool.c
  clang/test/CodeGen/debug-info-vector-bool-hvx64.c
  clang/test/CodeGen/debug-info-vector-bool.c
  clang/test/CodeGen/vector-alignment.c
  clang/test/SemaCXX/constexpr-vectors.cpp
  clang/test/SemaCXX/vector-bool.cpp
  clang/test/SemaCXX/vector-conditional.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/vector-conditional.cpp
===
--- clang/test/SemaCXX/vector-conditional.cpp
+++ clang/test/SemaCXX/vector-conditional.cpp
@@ -13,6 +13,7 @@
 using FourFloats = float __attribute__((__vector_size__(16)));
 using TwoDoubles = double __attribute__((__vector_size__(16)));
 using FourDoubles = double __attribute__((__vector_size__(32)));
+using EightBools = bool __attribute__((__vector_size__(2)));
 
 FourShorts four_shorts;
 TwoInts two_ints;
@@ -25,6 +26,8 @@
 FourFloats four_floats;
 TwoDoubles two_doubles;
 FourDoubles four_doubles;
+EightBools eight_bools;
+EightBools other_eight_bools;
 
 enum E {};
 enum class SE {};
@@ -95,6 +98,9 @@
   (void)(four_ints ? four_uints : 3.0f);
   (void)(four_ints ? four_ints : 3.0f);
 
+  // Allow conditional select on bool vectors.
+  (void)(eight_bools ? eight_bools : other_eight_bools);
+
   // When there is a vector and a scalar, conversions must be legal.
   (void)(four_ints ? four_floats : 3); // should work, ints can convert to floats.
   (void)(four_ints ? four_uints : e);  // expected-error {{cannot convert between scalar type 'E' and vector type 'FourUInts'}}
@@ -163,10 +169,10 @@
 void Templates() {
   dependent_cond(two_ints);
   dependent_operand(two_floats);
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
   all_dependent(four_ints, four_uints, four_doubles); // expected-note {{in instantiation of}}
 
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int unsigned int' (vector of 2 'unsigned int' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int uns

[PATCH] D84226: [AST][RecoveryExpr] Part1: Support dependent binary operator in C for error recovery.

2020-10-05 Thread Haojian Wu via Phabricator via cfe-commits

hokein added a comment.

I think this patch is ready for another round of review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84226

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

[PATCH] D87350: [AST][RecoveryExpr] Fix a crash on undeduced type.

2020-10-05 Thread Haojian Wu via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7f05fe1aeeb0: [AST][RecoveryExpr] Fix a crash on undeduced 
type. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87350

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/AST/ast-dump-recovery.cpp
  clang/test/SemaCXX/recovery-expr-type.cpp


Index: clang/test/SemaCXX/recovery-expr-type.cpp
===
--- clang/test/SemaCXX/recovery-expr-type.cpp
+++ clang/test/SemaCXX/recovery-expr-type.cpp
@@ -105,3 +105,9 @@
 int v = arr(); // expected-error {{array types cannot be value-initialized}} \
   expected-error {{cannot initialize a variable of type 'int' 
with an rvalue of type 'test8::arr'}}
 }
+
+namespace test9 {
+auto f(); // expected-note {{candidate function not viable}}
+// verify no crash on evaluating the size of undeduced auto type.
+static_assert(sizeof(f(1)), ""); // expected-error {{no matching function for 
call to 'f'}}
+}
Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -126,6 +126,12 @@
 // CHECK-NEXT:|   `-UnresolvedLookupExpr {{.*}} 'invalid'
 struct alignas(invalid()) Aligned {};
 
+auto f();
+int f(double);
+// CHECK:  VarDecl {{.*}} unknown_type_call 'int'
+// CHECK-NEXT: `-RecoveryExpr {{.*}} ''
+int unknown_type_call = f(0, 0);
+
 void InvalidInitalizer(int x) {
   struct Bar { Bar(); };
   // CHECK: `-VarDecl {{.*}} a1 'Bar'
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -12880,7 +12880,12 @@
 for (const auto &C : CS)
   ConsiderCandidate(C);
 
-  return Result.getValueOr(QualType());
+  if (!Result)
+return QualType();
+  auto Value = Result.getValue();
+  if (Value.isNull() || Value->isUndeducedType())
+return QualType();
+  return Value;
 }
 
 /// FinishOverloadedCallExpr - given an OverloadCandidateSet, builds and 
returns


Index: clang/test/SemaCXX/recovery-expr-type.cpp
===
--- clang/test/SemaCXX/recovery-expr-type.cpp
+++ clang/test/SemaCXX/recovery-expr-type.cpp
@@ -105,3 +105,9 @@
 int v = arr(); // expected-error {{array types cannot be value-initialized}} \
   expected-error {{cannot initialize a variable of type 'int' with an rvalue of type 'test8::arr'}}
 }
+
+namespace test9 {
+auto f(); // expected-note {{candidate function not viable}}
+// verify no crash on evaluating the size of undeduced auto type.
+static_assert(sizeof(f(1)), ""); // expected-error {{no matching function for call to 'f'}}
+}
Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -126,6 +126,12 @@
 // CHECK-NEXT:|   `-UnresolvedLookupExpr {{.*}} 'invalid'
 struct alignas(invalid()) Aligned {};
 
+auto f();
+int f(double);
+// CHECK:  VarDecl {{.*}} unknown_type_call 'int'
+// CHECK-NEXT: `-RecoveryExpr {{.*}} ''
+int unknown_type_call = f(0, 0);
+
 void InvalidInitalizer(int x) {
   struct Bar { Bar(); };
   // CHECK: `-VarDecl {{.*}} a1 'Bar'
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -12880,7 +12880,12 @@
 for (const auto &C : CS)
   ConsiderCandidate(C);
 
-  return Result.getValueOr(QualType());
+  if (!Result)
+return QualType();
+  auto Value = Result.getValue();
+  if (Value.isNull() || Value->isUndeducedType())
+return QualType();
+  return Value;
 }
 
 /// FinishOverloadedCallExpr - given an OverloadCandidateSet, builds and returns
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 7f05fe1 - [AST][RecoveryExpr] Fix a crash on undeduced type.

2020-10-05 Thread Haojian Wu via cfe-commits


Author: Haojian Wu
Date: 2020-10-05T12:52:04+02:00
New Revision: 7f05fe1aeeb005b552c6a3093b61659e7b578b14

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

LOG: [AST][RecoveryExpr] Fix a crash on undeduced type.

We should not capture the type if the function return type is undeduced.

Reviewed By: adamcz

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

Added: 


Modified: 
clang/lib/Sema/SemaOverload.cpp
clang/test/AST/ast-dump-recovery.cpp
clang/test/SemaCXX/recovery-expr-type.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 0c252a488fea..4696ed56dc71 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -12880,7 +12880,12 @@ static QualType 
chooseRecoveryType(OverloadCandidateSet &CS,
 for (const auto &C : CS)
   ConsiderCandidate(C);
 
-  return Result.getValueOr(QualType());
+  if (!Result)
+return QualType();
+  auto Value = Result.getValue();
+  if (Value.isNull() || Value->isUndeducedType())
+return QualType();
+  return Value;
 }
 
 /// FinishOverloadedCallExpr - given an OverloadCandidateSet, builds and 
returns

diff  --git a/clang/test/AST/ast-dump-recovery.cpp 
b/clang/test/AST/ast-dump-recovery.cpp
index fd7c923b7e51..69d5f80427cb 100644
--- a/clang/test/AST/ast-dump-recovery.cpp
+++ b/clang/test/AST/ast-dump-recovery.cpp
@@ -126,6 +126,12 @@ void test(int x) {
 // CHECK-NEXT:|   `-UnresolvedLookupExpr {{.*}} 'invalid'
 struct alignas(invalid()) Aligned {};
 
+auto f();
+int f(double);
+// CHECK:  VarDecl {{.*}} unknown_type_call 'int'
+// CHECK-NEXT: `-RecoveryExpr {{.*}} ''
+int unknown_type_call = f(0, 0);
+
 void InvalidInitalizer(int x) {
   struct Bar { Bar(); };
   // CHECK: `-VarDecl {{.*}} a1 'Bar'

diff  --git a/clang/test/SemaCXX/recovery-expr-type.cpp 
b/clang/test/SemaCXX/recovery-expr-type.cpp
index 075d0147c684..8186a812790d 100644
--- a/clang/test/SemaCXX/recovery-expr-type.cpp
+++ b/clang/test/SemaCXX/recovery-expr-type.cpp
@@ -105,3 +105,9 @@ typedef int arr[];
 int v = arr(); // expected-error {{array types cannot be value-initialized}} \
   expected-error {{cannot initialize a variable of type 'int' 
with an rvalue of type 'test8::arr'}}
 }
+
+namespace test9 {
+auto f(); // expected-note {{candidate function not viable}}
+// verify no crash on evaluating the size of undeduced auto type.
+static_assert(sizeof(f(1)), ""); // expected-error {{no matching function for 
call to 'f'}}
+}



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

[PATCH] D88013: [AArch64] Correct parameter type for unsigned Neon scalar shift intrinsics

2020-10-05 Thread David Spickett via Phabricator via cfe-commits

DavidSpickett added a comment.

ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88013

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

[PATCH] D88822: [clangd] Describe non-handling of most IWYU pragmas. NFC

2020-10-05 Thread Sam McCall via Phabricator via cfe-commits

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88822

Files:
  clang-tools-extra/clangd/index/CanonicalIncludes.h


Index: clang-tools-extra/clangd/index/CanonicalIncludes.h
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.h
+++ clang-tools-extra/clangd/index/CanonicalIncludes.h
@@ -71,6 +71,21 @@
 ///
 /// Currently it only supports IWYU private pragma:
 /// 
https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/IWYUPragmas.md#iwyu-pragma-private
+///
+/// We ignore other pragmas:
+/// - keep: this is common but irrelevant: we do not currently remove includes
+/// - export: this is common and potentially interesting, there are three 
cases:
+///* Points to a public header (common): we can suppress include2 if you
+///  already have include1. Only marginally useful.
+///* Points to a private header annotated with `private` (somewhat 
commmon):
+///  Not incrementally useful as we support private.
+///* Points to a private header without pragmas (rare). This is a reversed
+///  private pragma, and is valuable but too rare to be worthwhile.
+/// - no_include: this is about as common as private, but only affects the
+///   current file, so the value is smaller. We could add support.
+/// - friend: this is less common than private, has implementation 
difficulties,
+///   and affects behavior in a limited scope.
+/// - associated: extremely rare
 std::unique_ptr
 collectIWYUHeaderMaps(CanonicalIncludes *Includes);
 


Index: clang-tools-extra/clangd/index/CanonicalIncludes.h
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.h
+++ clang-tools-extra/clangd/index/CanonicalIncludes.h
@@ -71,6 +71,21 @@
 ///
 /// Currently it only supports IWYU private pragma:
 /// https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/IWYUPragmas.md#iwyu-pragma-private
+///
+/// We ignore other pragmas:
+/// - keep: this is common but irrelevant: we do not currently remove includes
+/// - export: this is common and potentially interesting, there are three cases:
+///* Points to a public header (common): we can suppress include2 if you
+///  already have include1. Only marginally useful.
+///* Points to a private header annotated with `private` (somewhat commmon):
+///  Not incrementally useful as we support private.
+///* Points to a private header without pragmas (rare). This is a reversed
+///  private pragma, and is valuable but too rare to be worthwhile.
+/// - no_include: this is about as common as private, but only affects the
+///   current file, so the value is smaller. We could add support.
+/// - friend: this is less common than private, has implementation difficulties,
+///   and affects behavior in a limited scope.
+/// - associated: extremely rare
 std::unique_ptr
 collectIWYUHeaderMaps(CanonicalIncludes *Includes);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 96c8a17 - [clangd] Remove unused using-decls in TypeHierarchyTests, NFC.

2020-10-05 Thread Haojian Wu via cfe-commits


Author: Haojian Wu
Date: 2020-10-05T13:14:53+02:00
New Revision: 96c8a17c800b2370f6d43fe67559ca10d5e44196

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

LOG: [clangd] Remove unused using-decls in TypeHierarchyTests, NFC.

Added: 


Modified: 
clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp 
b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
index 9f021ade0278..64831724d1be 100644
--- a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
@@ -29,11 +29,8 @@ namespace {
 
 using ::testing::AllOf;
 using ::testing::ElementsAre;
-using ::testing::Eq;
 using ::testing::Field;
-using ::testing::IsEmpty;
 using ::testing::Matcher;
-using ::testing::Pointee;
 using ::testing::UnorderedElementsAre;
 
 // GMock helpers for matching TypeHierarchyItem.



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

[PATCH] D88680: Add ability to turn off -fpch-instantiate-templates in clang-cl

2020-10-05 Thread Hans Wennborg via Phabricator via cfe-commits

hans added a comment.

In D88680#2308601 , @zahen wrote:

> This patch doesn't need a test case outside of the one that @rnk requested to 
> make sure that the flag flows from the clang-cl driver appropriately.  
> `pch-instantiate-templates` as authored doesn't match cl.exe behavior and 
> being unable to turn it off will prevent our adoption of clang 11.  This is a 
> release blocking issue at least for 11.1 if not 11.0 (I leave it to @hans to 
> make the decision).  I suspect there are other users that will run into the 
> same problem we have.
>
> Of course we'll continue to work on a reduced repro to understand where the 
> feature and msvc diverge but that will be a follow up patch or new bug.

It's very late in the process for 11, but since this is just a flags change 
which seems safe, I'll consider it if we'll do another release candidate. Of 
course the patch has to land first :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88680

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

[PATCH] D78902: [Driver] Add output file to properties of Command

2020-10-05 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl added inline comments.



Comment at: clang/lib/Driver/ToolChains/HIP.cpp:135
+ Bundler, BundlerArgs, Inputs,
+ InputInfo(&JA, Output.c_str(;
 }

I would suggest to use `Args.MakeArgString(OutputFileName)` instead of 
`Output.c_str()` to make sure it persists through the whole compilation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78902

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

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

2020-10-05 Thread Gabor Marton 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 rG007dd12d5468: [ASTImporter][AST] Fix structural equivalency 
crash on dependent FieldDecl (authored by martong).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88665

Files:
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/test/ASTMerge/struct/test.c
  clang/unittests/AST/StructuralEquivalenceTest.cpp

Index: clang/unittests/AST/StructuralEquivalenceTest.cpp
===
--- clang/unittests/AST/StructuralEquivalenceTest.cpp
+++ clang/unittests/AST/StructuralEquivalenceTest.cpp
@@ -976,6 +976,39 @@
   EXPECT_FALSE(testStructuralMatch(t));
 }
 
+TEST_F(StructuralEquivalenceTemplateTest, BitFieldDecl) {
+  const char *Code = "class foo { int a : 2; };";
+  auto t = makeNamedDecls(Code, Code, Lang_CXX03);
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceTemplateTest, BitFieldDeclDifferentWidth) {
+  auto t = makeNamedDecls("class foo { int a : 2; };",
+  "class foo { int a : 4; };", Lang_CXX03);
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceTemplateTest, DependentBitFieldDecl) {
+  const char *Code = "template  class foo { int a : sizeof(T); };";
+  auto t = makeNamedDecls(Code, Code, Lang_CXX03);
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceTemplateTest, DependentBitFieldDeclDifferentVal) {
+  auto t = makeNamedDecls(
+  "template  class foo { int a : sizeof(A); };",
+  "template  class foo { int a : sizeof(B); };",
+  Lang_CXX03);
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceTemplateTest, DependentBitFieldDeclDifferentVal2) {
+  auto t = makeNamedDecls(
+  "template  class foo { int a : sizeof(A); };",
+  "template  class foo { int a : sizeof(A) + 1; };", Lang_CXX03);
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
 TEST_F(StructuralEquivalenceTemplateTest, ExplicitBoolSame) {
   auto Decls = makeNamedDecls(
   "template  struct foo {explicit(b) foo(int);};",
Index: clang/test/ASTMerge/struct/test.c
===
--- clang/test/ASTMerge/struct/test.c
+++ clang/test/ASTMerge/struct/test.c
@@ -21,16 +21,6 @@
 // CHECK: struct1.c:27:8: note: no corresponding field here
 // CHECK: struct2.c:24:31: warning: external variable 'x4' declared with incompatible types in different translation units ('struct S4' vs. 'struct S4')
 // CHECK: struct1.c:27:22: note: declared here with type 'struct S4'
-// CHECK: struct1.c:33:8: warning: type 'struct S6' has incompatible definitions in different translation units
-// CHECK: struct1.c:33:33: note: bit-field 'j' with type 'unsigned int' and length 8 here
-// CHECK: struct2.c:30:33: note: field 'j' is not a bit-field
-// CHECK: struct2.c:30:38: warning: external variable 'x6' declared with incompatible types in different translation units ('struct S6' vs. 'struct S6')
-// CHECK: struct1.c:33:42: note: declared here with type 'struct S6'
-// CHECK: struct1.c:36:8: warning: type 'struct S7' has incompatible definitions in different translation units
-// CHECK: struct1.c:36:33: note: bit-field 'j' with type 'unsigned int' and length 8 here
-// CHECK: struct2.c:33:33: note: bit-field 'j' with type 'unsigned int' and length 16 here
-// CHECK: struct2.c:33:43: warning: external variable 'x7' declared with incompatible types in different translation units ('struct S7' vs. 'struct S7')
-// CHECK: struct1.c:36:42: note: declared here with type 'struct S7'
 // CHECK: struct1.c:56:10: warning: type 'struct DeeperError' has incompatible definitions in different translation units
 // CHECK: struct1.c:56:35: note: field 'f' has type 'int' here
 // CHECK: struct2.c:53:37: note: field 'f' has type 'float' here
@@ -52,4 +42,4 @@
 // CHECK: struct2.c:129:9: note: field 'S' has type 'struct (anonymous struct at [[PATH_TO_INPUTS]]struct2.c:127:7)' here
 // CHECK: struct2.c:138:3: warning: external variable 'x16' declared with incompatible types in different translation units ('struct DeepUnnamedError' vs. 'struct DeepUnnamedError')
 // CHECK: struct1.c:141:3: note: declared here with type 'struct DeepUnnamedError'
-// CHECK: 20 warnings generated
+// CHECK: 17 warnings generated
Index: clang/lib/AST/ASTStructuralEquivalence.cpp
===
--- clang/lib/AST/ASTStructuralEquivalence.cpp
+++ clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -1256,48 +1256,9 @@
 return false;
   }
 
-  if (Field1->isBitField() != Field2->isBitField()) {
-if (Context.Complain) {
-  Context.Diag2(
-  Owner2->getLocation(),
-  Context.getApplicableDiagnostic(diag::err_odr_tag_type_inconsistent))
-  << Context.ToCtx.getTypeDeclType(Owner2

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

2020-10-05 Thread Gabor Marton via Phabricator via cfe-commits

martong added a comment.

> Currently, we are totally inconsistent about the diagnostics. Either we 
> should emit a diagnostic before all return false or we should not ever emit 
> any diags. The diagnostics in their current form are misleading, because 
> there could be many notes missing. I am not sure how much do you guys value 
> these diags in LLDB, but in CTU we simply don't care about them. I'd rather 
> remove these diags from the equivalency check code, because they are causing 
> only confusion. (Or we should properly implement and test all aspects of the 
> diags.)



In D88665#2311344 , @teemperor wrote:

> LGTM, thanks!

Thanks for the review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88665

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

[clang] 007dd12 - [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

2020-10-05 Thread Gabor Marton via cfe-commits


Author: Gabor Marton
Date: 2020-10-05T14:06:09+02:00
New Revision: 007dd12d546814977519b33ca38b1cc8b31fee26

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

LOG: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

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

Added: 


Modified: 
clang/lib/AST/ASTStructuralEquivalence.cpp
clang/test/ASTMerge/struct/test.c
clang/unittests/AST/StructuralEquivalenceTest.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTStructuralEquivalence.cpp 
b/clang/lib/AST/ASTStructuralEquivalence.cpp
index 98e1b7eeb8c4..2bc5f39b817e 100644
--- a/clang/lib/AST/ASTStructuralEquivalence.cpp
+++ b/clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -1256,48 +1256,9 @@ static bool 
IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
 return false;
   }
 
-  if (Field1->isBitField() != Field2->isBitField()) {
-if (Context.Complain) {
-  Context.Diag2(
-  Owner2->getLocation(),
-  Context.getApplicableDiagnostic(diag::err_odr_tag_type_inconsistent))
-  << Context.ToCtx.getTypeDeclType(Owner2);
-  if (Field1->isBitField()) {
-Context.Diag1(Field1->getLocation(), diag::note_odr_bit_field)
-<< Field1->getDeclName() << Field1->getType()
-<< Field1->getBitWidthValue(Context.FromCtx);
-Context.Diag2(Field2->getLocation(), diag::note_odr_not_bit_field)
-<< Field2->getDeclName();
-  } else {
-Context.Diag2(Field2->getLocation(), diag::note_odr_bit_field)
-<< Field2->getDeclName() << Field2->getType()
-<< Field2->getBitWidthValue(Context.ToCtx);
-Context.Diag1(Field1->getLocation(), diag::note_odr_not_bit_field)
-<< Field1->getDeclName();
-  }
-}
-return false;
-  }
-
-  if (Field1->isBitField()) {
-// Make sure that the bit-fields are the same length.
-unsigned Bits1 = Field1->getBitWidthValue(Context.FromCtx);
-unsigned Bits2 = Field2->getBitWidthValue(Context.ToCtx);
-
-if (Bits1 != Bits2) {
-  if (Context.Complain) {
-Context.Diag2(Owner2->getLocation(),
-  Context.getApplicableDiagnostic(
-  diag::err_odr_tag_type_inconsistent))
-<< Context.ToCtx.getTypeDeclType(Owner2);
-Context.Diag2(Field2->getLocation(), diag::note_odr_bit_field)
-<< Field2->getDeclName() << Field2->getType() << Bits2;
-Context.Diag1(Field1->getLocation(), diag::note_odr_bit_field)
-<< Field1->getDeclName() << Field1->getType() << Bits1;
-  }
-  return false;
-}
-  }
+  if (Field1->isBitField())
+return IsStructurallyEquivalent(Context, Field1->getBitWidth(),
+Field2->getBitWidth());
 
   return true;
 }

diff  --git a/clang/test/ASTMerge/struct/test.c 
b/clang/test/ASTMerge/struct/test.c
index 9ac66d17f60e..3e11dd9e07a8 100644
--- a/clang/test/ASTMerge/struct/test.c
+++ b/clang/test/ASTMerge/struct/test.c
@@ -21,16 +21,6 @@
 // CHECK: struct1.c:27:8: note: no corresponding field here
 // CHECK: struct2.c:24:31: warning: external variable 'x4' declared with 
incompatible types in 
diff erent translation units ('struct S4' vs. 'struct S4')
 // CHECK: struct1.c:27:22: note: declared here with type 'struct S4'
-// CHECK: struct1.c:33:8: warning: type 'struct S6' has incompatible 
definitions in 
diff erent translation units
-// CHECK: struct1.c:33:33: note: bit-field 'j' with type 'unsigned int' and 
length 8 here
-// CHECK: struct2.c:30:33: note: field 'j' is not a bit-field
-// CHECK: struct2.c:30:38: warning: external variable 'x6' declared with 
incompatible types in 
diff erent translation units ('struct S6' vs. 'struct S6')
-// CHECK: struct1.c:33:42: note: declared here with type 'struct S6'
-// CHECK: struct1.c:36:8: warning: type 'struct S7' has incompatible 
definitions in 
diff erent translation units
-// CHECK: struct1.c:36:33: note: bit-field 'j' with type 'unsigned int' and 
length 8 here
-// CHECK: struct2.c:33:33: note: bit-field 'j' with type 'unsigned int' and 
length 16 here
-// CHECK: struct2.c:33:43: warning: external variable 'x7' declared with 
incompatible types in 
diff erent translation units ('struct S7' vs. 'struct S7')
-// CHECK: struct1.c:36:42: note: declared here with type 'struct S7'
 // CHECK: struct1.c:56:10: warning: type 'struct DeeperError' has incompatible 
definitions in 
diff erent translation units
 // CHECK: struct1.c:56:35: note: field 'f' has type 'int' here
 // CHECK: struct2.c:53:37: note: field 'f' has type 'float' here
@@ -52,4 +42,4 @@
 // CHECK: struct2.c:129:9: note: field 'S' has type 'struct (anonymous struct 
at [[PATH_TO_INPUTS]]struct2.c:127:7)' here
 // CHECK: struct

[PATCH] D88745: [clangd][WIP] Add new code completion signals to improve MRR by 3%.

2020-10-05 Thread Utkarsh Saxena via Phabricator via cfe-commits

usaxena95 updated this revision to Diff 296159.
usaxena95 edited the summary of this revision.
usaxena95 added a comment.

Updated the references to old signals.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88745

Files:
  clang-tools-extra/clangd/Quality.cpp
  clang-tools-extra/clangd/Quality.h
  
clang-tools-extra/clangd/benchmarks/CompletionModel/DecisionForestBenchmark.cpp
  clang-tools-extra/clangd/quality/model/features.json
  clang-tools-extra/clangd/quality/model/forest.json

Index: clang-tools-extra/clangd/quality/model/features.json
===
--- clang-tools-extra/clangd/quality/model/features.json
+++ clang-tools-extra/clangd/quality/model/features.json
@@ -20,7 +20,7 @@
 "kind": "NUMBER"
 },
 {
-"name": "IsNameInContext",
+"name": "NumNameInContext",
 "kind": "NUMBER"
 },
 {
@@ -63,6 +63,10 @@
 "name": "TypeMatchesPreferred",
 "kind": "NUMBER"
 },
+{
+"name": "SemaPriority",
+"kind": "NUMBER"
+},
 {
 "name": "SymbolCategory",
 "kind": "ENUM",
@@ -81,4 +85,4 @@
 "type": "clang::clangd::SymbolRelevanceSignals::AccessibleScope",
 "header": "Quality.h"
 }
-]
\ No newline at end of file
+]
Index: clang-tools-extra/clangd/benchmarks/CompletionModel/DecisionForestBenchmark.cpp
===
--- clang-tools-extra/clangd/benchmarks/CompletionModel/DecisionForestBenchmark.cpp
+++ clang-tools-extra/clangd/benchmarks/CompletionModel/DecisionForestBenchmark.cpp
@@ -39,7 +39,7 @@
 E.setNumReferences(RandInt(1)); // Can be large integer.
 E.setSymbolCategory(RandInt(10));   // 10 Symbol Category.
 
-E.setIsNameInContext(FlipCoin(0.5)); // Boolean.
+E.setNumNameInContext(RandInt(5));   // Boolean.
 E.setIsForbidden(FlipCoin(0.1)); // Boolean.
 E.setIsInBaseClass(FlipCoin(0.3));   // Boolean.
 E.setFileProximityDistance(
@@ -57,6 +57,7 @@
 E.setHadSymbolType(FlipCoin(0.6));// Boolean.
 E.setTypeMatchesPreferred(FlipCoin(0.5)); // Boolean.
 E.setFilterLength(RandInt(15));
+E.setSemaPriority(RandInt(100));
 Examples.push_back(E);
   }
   return Examples;
Index: clang-tools-extra/clangd/Quality.h
===
--- clang-tools-extra/clangd/Quality.h
+++ clang-tools-extra/clangd/Quality.h
@@ -140,11 +140,14 @@
   /// CompletionPrefix.
   unsigned FilterLength = 0;
 
+  /// Priority of the completion item as computed by Sema.
+  unsigned SemaPriority = 0;
+
   /// Set of derived signals computed by calculateDerivedSignals(). Must not be
   /// set explicitly.
   struct DerivedSignals {
-/// Whether Name contains some word from context.
-bool NameMatchesContext = false;
+/// Number of words in Context that matches Name.
+unsigned NumNameInContext = 0;
 /// Min distance between SymbolURI and all the headers included by the TU.
 unsigned FileProximityDistance = FileDistance::Unreachable;
 /// Min distance between SymbolScope and all the available scopes.
Index: clang-tools-extra/clangd/Quality.cpp
===
--- clang-tools-extra/clangd/Quality.cpp
+++ clang-tools-extra/clangd/Quality.cpp
@@ -300,6 +300,8 @@
   SemaCCResult.Availability == CXAvailability_NotAccessible)
 Forbidden = true;
 
+  SemaPriority = SemaCCResult.Priority;
+
   if (SemaCCResult.Declaration) {
 SemaSaysInScope = true;
 // We boost things that have decls in the main file. We give a fixed score
@@ -352,7 +354,12 @@
 SymbolRelevanceSignals::DerivedSignals
 SymbolRelevanceSignals::calculateDerivedSignals() const {
   DerivedSignals Derived;
-  Derived.NameMatchesContext = wordMatching(Name, ContextWords).hasValue();
+  int NumNameInContext = 0;
+  if (ContextWords)
+for (const auto &Word : ContextWords->keys())
+  if (Name.contains_lower(Word))
+NumNameInContext++;
+  Derived.NumNameInContext = NumNameInContext;
   Derived.FileProximityDistance = !FileProximityMatch || SymbolURI.empty()
   ? FileDistance::Unreachable
   : FileProximityMatch->distance(SymbolURI);
@@ -387,7 +394,7 @@
  ? 2.0
  : scopeProximityScore(Derived.ScopeProximityDistance);
 
-  if (Derived.NameMatchesContext)
+  if (Derived.NumNameInContext > 0)
 Score *= 1.5;
 
   // Symbols like local variables may only be referenced within their scope.
@@ -498,7 +505,7 @@
 
   SymbolRelevanceSignals::DerivedSignals Derived =
   Relevance.calculateDerivedSignals();
-  E.setIsNameInContext(Derived.NameMatchesContext);
+  E.setNumNameInContext(Derived.NumNameInContext);
   E.setIsForbidden(Relevance.Forbidden);

[PATCH] D88828: [clangd] Verify the diagnostic code in include-fixer diagnostic tests, NFC.

2020-10-05 Thread Haojian Wu via Phabricator via cfe-commits

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88828

Files:
  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
@@ -720,13 +720,16 @@
   UnorderedElementsAre(
   AllOf(Diag(Test.range("nested"),
  "incomplete type 'ns::X' named in nested name specifier"),
+DiagName("incomplete_nested_name_spec"),
 WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
 "Add include \"x.h\" for symbol ns::X"))),
   AllOf(Diag(Test.range("base"), "base class has incomplete type"),
+DiagName("incomplete_base_class"),
 WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
 "Add include \"x.h\" for symbol ns::X"))),
   AllOf(Diag(Test.range("access"),
  "member access into incomplete type 'ns::X'"),
+DiagName("incomplete_member_access"),
 WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
 "Add include \"x.h\" for symbol ns::X");
 }
@@ -789,19 +792,23 @@
   TU.build().getDiagnostics(),
   UnorderedElementsAre(
   AllOf(Diag(Test.range("unqualified1"), "unknown type name 'X'"),
+DiagName("unknown_typename"),
 WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
 "Add include \"x.h\" for symbol ns::X"))),
   Diag(Test.range("unqualified2"), "use of undeclared identifier 'X'"),
   AllOf(Diag(Test.range("qualified1"),
  "no type named 'X' in namespace 'ns'"),
+DiagName("typename_nested_not_found"),
 WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
 "Add include \"x.h\" for symbol ns::X"))),
   AllOf(Diag(Test.range("qualified2"),
  "no member named 'X' in namespace 'ns'"),
+DiagName("no_member"),
 WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
 "Add include \"x.h\" for symbol ns::X"))),
   AllOf(Diag(Test.range("global"),
  "no type named 'Global' in the global namespace"),
+DiagName("typename_nested_not_found"),
 WithFix(Fix(Test.range("insert"), "#include \"global.h\"\n",
 "Add include \"global.h\" for symbol Global");
 }
@@ -825,6 +832,7 @@
   EXPECT_THAT(TU.build().getDiagnostics(),
   UnorderedElementsAre(AllOf(
   Diag(Test.range("unqualified"), "unknown type name 'X'"),
+  DiagName("unknown_typename"),
   WithFix(Fix(Test.range("insert"), "#include \"a.h\"\n",
   "Add include \"a.h\" for symbol na::X"),
   Fix(Test.range("insert"), "#include \"b.h\"\n",
@@ -905,6 +913,7 @@
   TU.build().getDiagnostics(),
   UnorderedElementsAre(AllOf(
   Diag(Test.range(), "no member named 'scope' in namespace 'ns'"),
+  DiagName("no_member"),
   WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
   "Add include \"x.h\" for symbol ns::scope::X_Y");
 }
@@ -934,22 +943,26 @@
   AllOf(
   Diag(Test.range("q1"), "use of undeclared identifier 'clangd'; "
  "did you mean 'clang'?"),
+  DiagName("undeclared_var_use_suggest"),
   WithFix(_, // change clangd to clang
   Fix(Test.range("insert"), "#include \"x.h\"\n",
   "Add include \"x.h\" for symbol clang::clangd::X"))),
   AllOf(
   Diag(Test.range("x"), "no type named 'X' in namespace 'clang'"),
+  DiagName("typename_nested_not_found"),
   WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
   "Add include \"x.h\" for symbol clang::clangd::X"))),
   AllOf(
   Diag(Test.range("q2"), "use of undeclared identifier 'clangd'; "
  "did you mean 'clang'?"),
+  DiagName("undeclared_var_use_suggest"),
   WithFix(
-  _, // change clangd to clangd
+  _, // change clangd to clang
   Fix(Test.range("insert"), "#include \"y.h\"\n",
   "Add include \"y.h\" for symbol clang::clangd:

[PATCH] D88828: [clangd] Verify the diagnostic code in include-fixer diagnostic tests, NFC.

2020-10-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits

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

LGTM, but can you add some description about why you've decided to do it now :D


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88828

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

[PATCH] D84226: [AST][RecoveryExpr] Part1: Support dependent binary operator in C for error recovery.

2020-10-05 Thread Sam McCall via Phabricator via cfe-commits

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



Comment at: clang/include/clang/AST/ASTContext.h:668
 
+  // clang's C-only codepath doesn't support dependent types yet, it is used to
+  // perform early typo correction for C.

This is a bit hard to follow.
I think it's two separate sentences, and the second is something like
"If this condition is false, typo correction must be performed eagerly rather 
than delayed in many places, as it makes use of dependent types".



Comment at: clang/lib/Sema/SemaExpr.cpp:14365
+   "Should only occur in error-recovery path.");
+if (BinaryOperator::isCompoundAssignmentOp(Opc))
+  return CompoundAssignOperator::Create(

isAssignmentOp instead? including = itself



Comment at: clang/lib/Sema/SemaExpr.cpp:14383
+Context.IntTy, VK_RValue, OK_Ordinary,
+OpLoc, CurFPFeatureOverrides());
+default:

comma could get the RHS :-)
May get some mileage for comma operators in macros, not sure though.

(Other aspect is value category: fortunately *in C* comma is an rvalue)



Comment at: clang/test/AST/ast-dump-recovery.c:45
   int* ptr;
-  // FIXME: the top-level expr should be a binary operator.
-  // CHECK:  ImplicitCastExpr {{.*}} contains-errors 
-  // CHECK-NEXT: `-RecoveryExpr {{.*}} contains-errors lvalue
-  // CHECK-NEXT:   |-DeclRefExpr {{.*}} 'ptr' 'int *'
-  // CHECK-NEXT:   `-RecoveryExpr {{.*}}
-  // CHECK-NEXT: `-DeclRefExpr {{.*}} 'some_func'
+  // CHECK: BinaryOperator {{.*}} contains-errors '='
+  // CHECK-NEXT: |-DeclRefExpr {{.*}} 'ptr' 'int *'

`'int *' lvalue contains-errors '='`



Comment at: clang/test/AST/ast-dump-recovery.c:52
+  int compoundOp;
+  // CHECK: CompoundAssignOperator {{.*}} '+='
+  // CHECK-NEXT: |-DeclRefExpr {{.*}} 'compoundOp'

`'int' lvalue contains-errors '+='`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84226

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

[PATCH] D88810: [clangd] Add isKeyword function.

2020-10-05 Thread Haojian Wu via Phabricator via cfe-commits

hokein added inline comments.



Comment at: clang-tools-extra/clangd/SourceCode.cpp:638
+  // Keywords are initialized in constructor.
+  clang::IdentifierTable KeywordsTable(LangOpts);
+  return KeywordsTable.find(NewName) != KeywordsTable.end();

sammccall wrote:
> this builds a pretty decent sized hashtable for each call, because LangOpts 
> isn't constant.
> 
> We could consider exposing getKeywordStatus from IdentifierTable.h, then we 
> could build a StringMap (once, statically) and then call 
> getKeywordStatus to apply the per-langopts logic.
> In fact that StringMap might also be generally nice to expose in 
> TokenKinds.h... as long as there are no conflicts.
> 
> Or we could have cache map from LangOpts to IdentifierTable, since we won't 
> see many sets in practice.
> 
> Or maybe it doesn't matter, I suppose!
> Or maybe it doesn't matter, I suppose!
this is my impression as well, there are ~120 keywords for C++, I think the 
number is not too big.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88810

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

[PATCH] D88810: [clangd] Add isKeyword function.

2020-10-05 Thread Haojian Wu via Phabricator via cfe-commits

hokein updated this revision to Diff 296168.
hokein marked an inline comment as done.
hokein added a comment.

address code comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88810

Files:
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/unittests/SourceCodeTests.cpp


Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -789,6 +789,19 @@
   EXPECT_TRUE(isHeaderFile("header.h", LangOpts));
 }
 
+TEST(SourceCodeTests, isKeywords) {
+  LangOptions LangOpts;
+  LangOpts.CPlusPlus20 = true;
+  EXPECT_TRUE(isKeyword("int", LangOpts));
+  EXPECT_TRUE(isKeyword("return", LangOpts));
+  EXPECT_TRUE(isKeyword("co_await", LangOpts));
+
+  // these are identifiers (not keywords!) with special meaning in some
+  // contexts.
+  EXPECT_FALSE(isKeyword("final", LangOpts));
+  EXPECT_FALSE(isKeyword("override", LangOpts));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/SourceCode.h
===
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -248,6 +248,10 @@
   const LangOptions &LangOpts);
 };
 
+/// Return true if the \p TokenName is in the list of reversed keywords of the
+/// language.
+bool isKeyword(llvm::StringRef TokenName, const LangOptions &LangOpts);
+
 /// Heuristically determine namespaces visible at a point, without parsing 
Code.
 /// This considers using-directives and enclosing namespace-declarations that
 /// are visible (and not obfuscated) in the file itself (not headers).
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -633,6 +633,12 @@
   return Ranges;
 }
 
+bool isKeyword(llvm::StringRef NewName, const LangOptions &LangOpts) {
+  // Keywords are initialized in constructor.
+  clang::IdentifierTable KeywordsTable(LangOpts);
+  return KeywordsTable.find(NewName) != KeywordsTable.end();
+}
+
 namespace {
 struct NamespaceEvent {
   enum {


Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -789,6 +789,19 @@
   EXPECT_TRUE(isHeaderFile("header.h", LangOpts));
 }
 
+TEST(SourceCodeTests, isKeywords) {
+  LangOptions LangOpts;
+  LangOpts.CPlusPlus20 = true;
+  EXPECT_TRUE(isKeyword("int", LangOpts));
+  EXPECT_TRUE(isKeyword("return", LangOpts));
+  EXPECT_TRUE(isKeyword("co_await", LangOpts));
+
+  // these are identifiers (not keywords!) with special meaning in some
+  // contexts.
+  EXPECT_FALSE(isKeyword("final", LangOpts));
+  EXPECT_FALSE(isKeyword("override", LangOpts));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/SourceCode.h
===
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -248,6 +248,10 @@
   const LangOptions &LangOpts);
 };
 
+/// Return true if the \p TokenName is in the list of reversed keywords of the
+/// language.
+bool isKeyword(llvm::StringRef TokenName, const LangOptions &LangOpts);
+
 /// Heuristically determine namespaces visible at a point, without parsing Code.
 /// This considers using-directives and enclosing namespace-declarations that
 /// are visible (and not obfuscated) in the file itself (not headers).
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -633,6 +633,12 @@
   return Ranges;
 }
 
+bool isKeyword(llvm::StringRef NewName, const LangOptions &LangOpts) {
+  // Keywords are initialized in constructor.
+  clang::IdentifierTable KeywordsTable(LangOpts);
+  return KeywordsTable.find(NewName) != KeywordsTable.end();
+}
+
 namespace {
 struct NamespaceEvent {
   enum {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 1425c72 - [clangd] Add isKeyword function.

2020-10-05 Thread Haojian Wu via cfe-commits


Author: Haojian Wu
Date: 2020-10-05T15:11:24+02:00
New Revision: 1425c72236766ad9107d86cb645ee8c6a3ee0eb1

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

LOG: [clangd] Add isKeyword function.

This will be used in rename for doing basic name validation.

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

Added: 


Modified: 
clang-tools-extra/clangd/SourceCode.cpp
clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/unittests/SourceCodeTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/SourceCode.cpp 
b/clang-tools-extra/clangd/SourceCode.cpp
index 0432097b4348..c6279177eba9 100644
--- a/clang-tools-extra/clangd/SourceCode.cpp
+++ b/clang-tools-extra/clangd/SourceCode.cpp
@@ -633,6 +633,12 @@ std::vector collectIdentifierRanges(llvm::StringRef 
Identifier,
   return Ranges;
 }
 
+bool isKeyword(llvm::StringRef NewName, const LangOptions &LangOpts) {
+  // Keywords are initialized in constructor.
+  clang::IdentifierTable KeywordsTable(LangOpts);
+  return KeywordsTable.find(NewName) != KeywordsTable.end();
+}
+
 namespace {
 struct NamespaceEvent {
   enum {

diff  --git a/clang-tools-extra/clangd/SourceCode.h 
b/clang-tools-extra/clangd/SourceCode.h
index 128f985a5266..be78e2f86436 100644
--- a/clang-tools-extra/clangd/SourceCode.h
+++ b/clang-tools-extra/clangd/SourceCode.h
@@ -248,6 +248,10 @@ struct SpelledWord {
   const LangOptions &LangOpts);
 };
 
+/// Return true if the \p TokenName is in the list of reversed keywords of the
+/// language.
+bool isKeyword(llvm::StringRef TokenName, const LangOptions &LangOpts);
+
 /// Heuristically determine namespaces visible at a point, without parsing 
Code.
 /// This considers using-directives and enclosing namespace-declarations that
 /// are visible (and not obfuscated) in the file itself (not headers).

diff  --git a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp 
b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
index 9c3ae4df51ff..c05515f2c094 100644
--- a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -789,6 +789,19 @@ TEST(SourceCodeTests, isHeaderFile) {
   EXPECT_TRUE(isHeaderFile("header.h", LangOpts));
 }
 
+TEST(SourceCodeTests, isKeywords) {
+  LangOptions LangOpts;
+  LangOpts.CPlusPlus20 = true;
+  EXPECT_TRUE(isKeyword("int", LangOpts));
+  EXPECT_TRUE(isKeyword("return", LangOpts));
+  EXPECT_TRUE(isKeyword("co_await", LangOpts));
+
+  // these are identifiers (not keywords!) with special meaning in some
+  // contexts.
+  EXPECT_FALSE(isKeyword("final", LangOpts));
+  EXPECT_FALSE(isKeyword("override", LangOpts));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang



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

[PATCH] D88810: [clangd] Add isKeyword function.

2020-10-05 Thread Haojian Wu via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1425c7223676: [clangd] Add isKeyword function. (authored by 
hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88810

Files:
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/unittests/SourceCodeTests.cpp


Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -789,6 +789,19 @@
   EXPECT_TRUE(isHeaderFile("header.h", LangOpts));
 }
 
+TEST(SourceCodeTests, isKeywords) {
+  LangOptions LangOpts;
+  LangOpts.CPlusPlus20 = true;
+  EXPECT_TRUE(isKeyword("int", LangOpts));
+  EXPECT_TRUE(isKeyword("return", LangOpts));
+  EXPECT_TRUE(isKeyword("co_await", LangOpts));
+
+  // these are identifiers (not keywords!) with special meaning in some
+  // contexts.
+  EXPECT_FALSE(isKeyword("final", LangOpts));
+  EXPECT_FALSE(isKeyword("override", LangOpts));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/SourceCode.h
===
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -248,6 +248,10 @@
   const LangOptions &LangOpts);
 };
 
+/// Return true if the \p TokenName is in the list of reversed keywords of the
+/// language.
+bool isKeyword(llvm::StringRef TokenName, const LangOptions &LangOpts);
+
 /// Heuristically determine namespaces visible at a point, without parsing 
Code.
 /// This considers using-directives and enclosing namespace-declarations that
 /// are visible (and not obfuscated) in the file itself (not headers).
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -633,6 +633,12 @@
   return Ranges;
 }
 
+bool isKeyword(llvm::StringRef NewName, const LangOptions &LangOpts) {
+  // Keywords are initialized in constructor.
+  clang::IdentifierTable KeywordsTable(LangOpts);
+  return KeywordsTable.find(NewName) != KeywordsTable.end();
+}
+
 namespace {
 struct NamespaceEvent {
   enum {


Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -789,6 +789,19 @@
   EXPECT_TRUE(isHeaderFile("header.h", LangOpts));
 }
 
+TEST(SourceCodeTests, isKeywords) {
+  LangOptions LangOpts;
+  LangOpts.CPlusPlus20 = true;
+  EXPECT_TRUE(isKeyword("int", LangOpts));
+  EXPECT_TRUE(isKeyword("return", LangOpts));
+  EXPECT_TRUE(isKeyword("co_await", LangOpts));
+
+  // these are identifiers (not keywords!) with special meaning in some
+  // contexts.
+  EXPECT_FALSE(isKeyword("final", LangOpts));
+  EXPECT_FALSE(isKeyword("override", LangOpts));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/SourceCode.h
===
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -248,6 +248,10 @@
   const LangOptions &LangOpts);
 };
 
+/// Return true if the \p TokenName is in the list of reversed keywords of the
+/// language.
+bool isKeyword(llvm::StringRef TokenName, const LangOptions &LangOpts);
+
 /// Heuristically determine namespaces visible at a point, without parsing Code.
 /// This considers using-directives and enclosing namespace-declarations that
 /// are visible (and not obfuscated) in the file itself (not headers).
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -633,6 +633,12 @@
   return Ranges;
 }
 
+bool isKeyword(llvm::StringRef NewName, const LangOptions &LangOpts) {
+  // Keywords are initialized in constructor.
+  clang::IdentifierTable KeywordsTable(LangOpts);
+  return KeywordsTable.find(NewName) != KeywordsTable.end();
+}
+
 namespace {
 struct NamespaceEvent {
   enum {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 7a932f4 - [Parser] ParseMicrosoftAsmStatement - Replace bit '|' operator with logical '||' operator. (PR47071)

2020-10-05 Thread Simon Pilgrim via cfe-commits


Author: Simon Pilgrim
Date: 2020-10-05T14:23:28+01:00
New Revision: 7a932f4f4ccbc0c4294c6911d404f74529f3259b

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

LOG: [Parser] ParseMicrosoftAsmStatement - Replace bit '|' operator with 
logical '||' operator. (PR47071)

Fixes static analysis warning.

Added: 


Modified: 
clang/lib/Parse/ParseStmtAsm.cpp

Removed: 




diff  --git a/clang/lib/Parse/ParseStmtAsm.cpp 
b/clang/lib/Parse/ParseStmtAsm.cpp
index 7d0818840a4f..bdf40c291cb6 100644
--- a/clang/lib/Parse/ParseStmtAsm.cpp
+++ b/clang/lib/Parse/ParseStmtAsm.cpp
@@ -581,7 +581,7 @@ StmtResult 
Parser::ParseMicrosoftAsmStatement(SourceLocation AsmLoc) {
   std::unique_ptr STI(
   TheTarget->createMCSubtargetInfo(TT, TO.CPU, FeaturesStr));
   // Target MCTargetDesc may not be linked in clang-based tools.
-  if (!MAI || !MII | !MOFI || !STI) {
+  if (!MAI || !MII || !MOFI || !STI) {
 Diag(AsmLoc, diag::err_msasm_unable_to_create_target)
 << "target MC unavailable";
 return EmptyStmt();



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

[PATCH] D88831: [clang-tidy] Remove obsolete checker google-runtime-references

2020-10-05 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: alexfh, gribozavr2, aaron.ballman.
baloghadamsoftware added a project: clang-tools-extra.
Herald added subscribers: martong, Charusso, gamesh411, Szelethus, dkrupp, 
rnkovacs, xazax.hun, whisperity, mgorny.
Herald added a project: clang.
baloghadamsoftware requested review of this revision.

The rules which is the base of this checker is removed from the //Google C++ 
Style Guide// in May: Update C++ styleguide 
. Now this checker became 
obsolete.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88831

Files:
  clang-tools-extra/clang-tidy/google/CMakeLists.txt
  clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
  clang-tools-extra/clang-tidy/google/NonConstReferences.cpp
  clang-tools-extra/clang-tidy/google/NonConstReferences.h
  clang-tools-extra/docs/clang-tidy/checks/google-runtime-references.rst
  clang-tools-extra/test/clang-tidy/checkers/google-runtime-references.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/google-runtime-references.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/google-runtime-references.cpp
+++ /dev/null
@@ -1,155 +0,0 @@
-// RUN: %check_clang_tidy %s google-runtime-references %t -- \
-// RUN:   -config="{CheckOptions: \
-// RUN: [{key: google-runtime-references.IncludedTypes, \
-// RUN:   value: 'included::A; included::B'}]}"
-
-int a;
-int &b = a;
-int *c;
-void f1(int a);
-void f2(int *b);
-void f3(const int &c);
-void f4(int const &d);
-
-// Don't warn on implicit operator= in c++11 mode.
-class A {
-  virtual void f() {}
-};
-// Don't warn on rvalue-references.
-struct A2 {
-  A2(A2&&) = default;
-  void f(A2&&) {}
-};
-
-// Don't warn on iostream parameters.
-namespace xxx {
-class istream { };
-class ostringstream { };
-}
-void g1(xxx::istream &istr);
-void g1(xxx::ostringstream &istr);
-
-void g1(int &a);
-// CHECK-MESSAGES: [[@LINE-1]]:14: warning: non-const reference parameter 'a', make it const or use a pointer [google-runtime-references]
-
-struct s {};
-void g2(int a, int b, s c, s &d);
-// CHECK-MESSAGES: [[@LINE-1]]:31: warning: non-const reference parameter 'd', {{.*}}
-
-typedef int &ref;
-void g3(ref a);
-// CHECK-MESSAGES: [[@LINE-1]]:13: warning: non-const reference {{.*}}
-
-void g4(int &a, int &b, int &);
-// CHECK-MESSAGES: [[@LINE-1]]:14: warning: non-const reference parameter 'a', {{.*}}
-// CHECK-MESSAGES: [[@LINE-2]]:22: warning: non-const reference parameter 'b', {{.*}}
-// CHECK-MESSAGES: [[@LINE-3]]:30: warning: non-const reference parameter at index 2, {{.*}}
-
-class B {
-  B(B& a) {}
-// CHECK-MESSAGES: [[@LINE-1]]:8: warning: non-const reference {{.*}}
-  virtual void f(int &a) {}
-// CHECK-MESSAGES: [[@LINE-1]]:23: warning: non-const reference {{.*}}
-  void g(int &b);
-// CHECK-MESSAGES: [[@LINE-1]]:15: warning: non-const reference {{.*}}
-
-  // Don't warn on the parameter of stream extractors defined as members.
-  B& operator>>(int& val) { return *this; }
-};
-
-// Only warn on the first declaration of each function to reduce duplicate
-// warnings.
-void B::g(int &b) {}
-
-// Don't warn on the first parameter of stream inserters.
-A& operator<<(A& s, int&) { return s; }
-// CHECK-MESSAGES: [[@LINE-1]]:25: warning: non-const reference parameter at index 1, {{.*}}
-
-// Don't warn on either parameter of stream extractors. Both need to be
-// non-const references by convention.
-A& operator>>(A& input, int& val) { return input; }
-
-// Don't warn on lambdas.
-auto lambda = [] (int&) {};
-
-// Don't warn on typedefs, as we'll warn on the function itself.
-typedef int (*fp)(int &);
-
-// Don't warn on function references.
-typedef void F();
-void g5(const F& func) {}
-void g6(F& func) {}
-
-template
-void g7(const T& t) {}
-
-template
-void g8(T t) {}
-
-void f5() {
-  g5(f5);
-  g6(f5);
-  g7(f5);
-  g7(f5);
-  g8(f5);
-  g8(f5);
-}
-
-// Don't warn on dependent types.
-template
-void g9(T& t) {}
-template
-void g10(T t) {}
-
-void f6() {
-  int i;
-  float f;
-  g9(i);
-  g9(i);
-  g9(i);
-  g10(i);
-  g10(f);
-}
-
-// Warn only on the overridden methods from the base class, as the child class
-// only implements the interface.
-class C : public B {
-  C();
-  virtual void f(int &a) {}
-};
-
-// Don't warn on operator<< with streams-like interface.
-A& operator<<(A& s, int) { return s; }
-
-// Don't warn on swap().
-void swap(C& c1, C& c2) {}
-
-// Don't warn on standalone operator++, operator--, operator+=, operator-=,
-// operator*=, etc. that all need non-const references to be functional.
-A& operator++(A& a) { return a; }
-A operator++(A& a, int) { return a; }
-A& operator--(A& a) { return a; }
-A operator--(A& a, int) { return a; }
-A& operator+=(A& a, const A& b) { return a; }
-A& operator-=(A& a, const A& b) { return a; }
-A& operator*=(A& a, const A& b) { return a; }
-A& operator/=(A

[PATCH] D88829: [OpenMP][RTL] Remove dead code

2020-10-05 Thread Jon Chesterfield via Phabricator via cfe-commits

JonChesterfield added a comment.

Change looks great to me.

Rolling the reduction in leading whitespace in 
nvptx_target_parallel_reduction_codegen.cpp in with the patch might be 
contentious, added a couple more reviewers to see if other people would prefer 
that part split out. I'll accept in a day or so if there are no comments on the 
whitespace.

Thanks!




Comment at: openmp/libomptarget/deviceRTLs/common/src/omptarget.cu:135
 "%d threads\n",
 (int)newTaskDescr->ThreadId(), (int)ThreadLimit);
 }

This is interesting. This turns out to be the only call to RootS(), and that 
cascades through a bunch of other code removed in this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88829

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

[PATCH] D88829: [OpenMP][RTL] Remove dead code

2020-10-05 Thread George Rokos via Phabricator via cfe-commits

grokos added a comment.

In D88829#2311768 , @JonChesterfield 
wrote:

> Rolling the reduction in leading whitespace in 
> nvptx_target_parallel_reduction_codegen.cpp in with the patch might be 
> contentious, added a couple more reviewers to see if other people would 
> prefer that part split out. I'll accept in a day or so if there are no 
> comments on the whitespace.

Fine by me, I don't think it's worth uploading an extra patch just for this 
minor formatting detail.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88829

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

[PATCH] D88833: Do not warn on pointer decays in system macros

2020-10-05 Thread fiesh via Phabricator via cfe-commits

fiesh created this revision.
fiesh added a reviewer: aaron.ballman.
fiesh added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, kbarton, nemanjai.
Herald added a project: clang.
fiesh requested review of this revision.

As system headers are in general out of reach, it makes no sense to warn on 
pointer decays happening in them.

This is a reboot of https://reviews.llvm.org/D31130
It is essentially unchanged, only updated to the new repository as well as 
minor API changes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88833

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/cppcoreguidelines-pro-bounds-array-to-pointer-decay/macro.h
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-pro-bounds-array-to-pointer-decay %t
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-bounds-array-to-pointer-decay %t -- -- -isystem%S/Inputs/cppcoreguidelines-pro-bounds-array-to-pointer-decay
+
+#include 
 #include 
 
 namespace gsl {
@@ -8,7 +10,7 @@
   template 
   array_view(U (&arr)[N]);
 };
-}
+} // namespace gsl
 
 void pointerfun(int *p);
 void arrayfun(int p[]);
@@ -29,9 +31,9 @@
   gsl::array_view av(a);
   arrayviewfun(av); // OK
 
-  int i = a[0];  // OK
-  int j = a[(1 + 2)];// OK
-  pointerfun(&a[0]); // OK
+  int i = a[0];   // OK
+  int j = a[(1 + 2)]; // OK
+  pointerfun(&a[0]);  // OK
 
   for (auto &e : a) // OK, iteration internally decays array to pointer
 e = 1;
@@ -41,11 +43,53 @@
   return "clang"; // OK, decay string literal to pointer
 }
 const char *g2() {
-return ("clang"); // OK, ParenExpr hides the literal-pointer decay
+  return ("clang"); // OK, ParenExpr hides the literal-pointer decay
 }
 
 void f2(void *const *);
 void bug25362() {
   void *a[2];
-  f2(static_cast(a)); // OK, explicit cast
+  f2(static_cast(a)); // OK, explicit cast
+}
+
+void user_fn_decay_str(const char *);
+void user_fn_decay_int_array(const int *);
+void bug32239() {
+  sys_macro_with_pretty_function_string_decay; // Ok
+  sys_macro_with_sys_string_decay; // Ok
+  sys_macro_with_sys_int_array_decay;  // Ok
+
+  sys_fn_decay_str(__PRETTY_FUNCTION__); // Ok
+
+  sys_fn_decay_str(SYS_STRING); // Not Ok - should it be ok?
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
+
+  sys_fn_decay_int_array(SYS_INT_ARRAY); // Not Ok
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
+
+  user_code_in_sys_macro(sys_fn_decay_str(__PRETTY_FUNCTION__)); // Ok
+
+  user_code_in_sys_macro(sys_fn_decay_str(SYS_STRING)); // Not Ok - should it be ok?
+  // CHECK-MESSAGES: :[[@LINE-1]]:43: warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
+
+  user_code_in_sys_macro(sys_fn_decay_int_array(SYS_INT_ARRAY)); // Not Ok
+  // CHECK-MESSAGES: :[[@LINE-1]]:49: warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
+
+  const char UserString[1] = "";
+  decay_to_char_pointer_in_macro(UserString); // Not Ok
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
+
+  decay_to_char_pointer_in_macro(__PRETTY_FUNCTION__); // Ok
+
+  int a[5];
+  decay_to_int_pointer_in_macro(a); // Not Ok
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
+
+  user_fn_decay_str(__PRETTY_FUNCTION__); // Ok
+
+  user_fn_decay_str(SYS_STRING); // Not Ok - should it be ok?
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
+
+  user_fn_decay_int_array(SYS_INT_ARRAY); // Not ok
+  // CHECK-MESSAGES: :[[@LI

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

2020-10-05 Thread fiesh via Phabricator via cfe-commits

fiesh added a comment.

Continued in https://reviews.llvm.org/D88833


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

https://reviews.llvm.org/D31130

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

[PATCH] D88789: [InstCombine] Revert rL226781 "Teach InstCombine to canonicalize loads which are only ever stored to always use a legal integer type if one is available." (PR47592)

2020-10-05 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri updated this revision to Diff 296173.
lebedev.ri added a comment.

NFC


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88789

Files:
  clang/test/CodeGen/attr-arm-sve-vector-bits-bitcast.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-globals.c
  llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
  llvm/test/Transforms/InstCombine/atomic.ll
  llvm/test/Transforms/InstCombine/load.ll
  llvm/test/Transforms/InstCombine/loadstore-metadata.ll
  llvm/test/Transforms/InstCombine/non-integral-pointers.ll
  llvm/test/Transforms/PhaseOrdering/instcombine-sroa-inttoptr.ll

Index: llvm/test/Transforms/PhaseOrdering/instcombine-sroa-inttoptr.ll
===
--- llvm/test/Transforms/PhaseOrdering/instcombine-sroa-inttoptr.ll
+++ llvm/test/Transforms/PhaseOrdering/instcombine-sroa-inttoptr.ll
@@ -50,10 +50,10 @@
 define dso_local void @_Z3gen1S(%0* noalias sret align 8 %arg, %0* byval(%0) align 8 %arg1) {
 ; CHECK-LABEL: @_Z3gen1S(
 ; CHECK-NEXT:  bb:
-; CHECK-NEXT:[[TMP0:%.*]] = bitcast %0* [[ARG1:%.*]] to i64*
-; CHECK-NEXT:[[I21:%.*]] = load i64, i64* [[TMP0]], align 8
-; CHECK-NEXT:[[TMP1:%.*]] = bitcast %0* [[ARG:%.*]] to i64*
-; CHECK-NEXT:store i64 [[I21]], i64* [[TMP1]], align 8
+; CHECK-NEXT:[[I:%.*]] = getelementptr inbounds [[TMP0:%.*]], %0* [[ARG1:%.*]], i64 0, i32 0
+; CHECK-NEXT:[[I2:%.*]] = load i32*, i32** [[I]], align 8
+; CHECK-NEXT:[[I3:%.*]] = getelementptr inbounds [[TMP0]], %0* [[ARG:%.*]], i64 0, i32 0
+; CHECK-NEXT:store i32* [[I2]], i32** [[I3]], align 8
 ; CHECK-NEXT:ret void
 ;
 bb:
@@ -68,13 +68,12 @@
 ; CHECK-LABEL: @_Z3foo1S(
 ; CHECK-NEXT:  bb:
 ; CHECK-NEXT:[[I2:%.*]] = alloca [[TMP0:%.*]], align 8
-; CHECK-NEXT:[[I1_SROA_0_0_I5_SROA_CAST:%.*]] = bitcast %0* [[ARG:%.*]] to i64*
-; CHECK-NEXT:[[I1_SROA_0_0_COPYLOAD:%.*]] = load i64, i64* [[I1_SROA_0_0_I5_SROA_CAST]], align 8
-; CHECK-NEXT:[[I_SROA_0_0_I6_SROA_CAST:%.*]] = bitcast %0* [[I2]] to i64*
-; CHECK-NEXT:store i64 [[I1_SROA_0_0_COPYLOAD]], i64* [[I_SROA_0_0_I6_SROA_CAST]], align 8
+; CHECK-NEXT:[[I1_SROA_0_0_I5_SROA_IDX:%.*]] = getelementptr inbounds [[TMP0]], %0* [[ARG:%.*]], i64 0, i32 0
+; CHECK-NEXT:[[I1_SROA_0_0_COPYLOAD:%.*]] = load i32*, i32** [[I1_SROA_0_0_I5_SROA_IDX]], align 8
+; CHECK-NEXT:[[I_SROA_0_0_I6_SROA_IDX:%.*]] = getelementptr inbounds [[TMP0]], %0* [[I2]], i64 0, i32 0
+; CHECK-NEXT:store i32* [[I1_SROA_0_0_COPYLOAD]], i32** [[I_SROA_0_0_I6_SROA_IDX]], align 8
 ; CHECK-NEXT:tail call void @_Z7escape01S(%0* nonnull byval(%0) align 8 [[I2]])
-; CHECK-NEXT:[[TMP0]] = inttoptr i64 [[I1_SROA_0_0_COPYLOAD]] to i32*
-; CHECK-NEXT:ret i32* [[TMP0]]
+; CHECK-NEXT:ret i32* [[I1_SROA_0_0_COPYLOAD]]
 ;
 bb:
   %i = alloca %0, align 8
@@ -108,24 +107,21 @@
 define dso_local i32* @_Z3bar1S(%0* byval(%0) align 8 %arg) {
 ; CHECK-LABEL: @_Z3bar1S(
 ; CHECK-NEXT:  bb:
-; CHECK-NEXT:[[I1_SROA_0_0_I4_SROA_CAST:%.*]] = bitcast %0* [[ARG:%.*]] to i64*
-; CHECK-NEXT:[[I1_SROA_0_0_COPYLOAD:%.*]] = load i64, i64* [[I1_SROA_0_0_I4_SROA_CAST]], align 8
+; CHECK-NEXT:[[I1_SROA_0_0_I4_SROA_IDX:%.*]] = getelementptr inbounds [[TMP0:%.*]], %0* [[ARG:%.*]], i64 0, i32 0
+; CHECK-NEXT:[[I1_SROA_0_0_COPYLOAD:%.*]] = load i32*, i32** [[I1_SROA_0_0_I4_SROA_IDX]], align 8
 ; CHECK-NEXT:[[I5:%.*]] = tail call i32 @_Z4condv()
 ; CHECK-NEXT:[[I6_NOT:%.*]] = icmp eq i32 [[I5]], 0
 ; CHECK-NEXT:br i1 [[I6_NOT]], label [[BB10:%.*]], label [[BB7:%.*]]
 ; CHECK:   bb7:
 ; CHECK-NEXT:tail call void @_Z5sync0v()
-; CHECK-NEXT:[[TMP0:%.*]] = inttoptr i64 [[I1_SROA_0_0_COPYLOAD]] to i32*
-; CHECK-NEXT:tail call void @_Z7escape0Pi(i32* [[TMP0]])
+; CHECK-NEXT:tail call void @_Z7escape0Pi(i32* [[I1_SROA_0_0_COPYLOAD]])
 ; CHECK-NEXT:br label [[BB13:%.*]]
 ; CHECK:   bb10:
 ; CHECK-NEXT:tail call void @_Z5sync1v()
-; CHECK-NEXT:[[TMP1:%.*]] = inttoptr i64 [[I1_SROA_0_0_COPYLOAD]] to i32*
-; CHECK-NEXT:tail call void @_Z7escape1Pi(i32* [[TMP1]])
+; CHECK-NEXT:tail call void @_Z7escape1Pi(i32* [[I1_SROA_0_0_COPYLOAD]])
 ; CHECK-NEXT:br label [[BB13]]
 ; CHECK:   bb13:
-; CHECK-NEXT:[[DOTPRE_PHI:%.*]] = phi i32* [ [[TMP1]], [[BB10]] ], [ [[TMP0]], [[BB7]] ]
-; CHECK-NEXT:ret i32* [[DOTPRE_PHI]]
+; CHECK-NEXT:ret i32* [[I1_SROA_0_0_COPYLOAD]]
 ;
 bb:
   %i = alloca %0, align 8
Index: llvm/test/Transforms/InstCombine/non-integral-pointers.ll
===
--- llvm/test/Transforms/InstCombine/non-integral-pointers.ll
+++ llvm/test/Transforms/InstCombine/non-integral-pointers.ll
@@ -41,10 +41,8 @@
 ; integers, since pointers in address space 3 are integral.
 ; CHECK-LABEL: @

[PATCH] D88831: [clang-tidy] Remove obsolete checker google-runtime-references

2020-10-05 Thread Eugene Zelenko via Phabricator via cfe-commits

Eugene.Zelenko added a comment.

Does this check make sense in content of other style guides?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88831

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

[PATCH] D88831: [clang-tidy] Remove obsolete checker google-runtime-references

2020-10-05 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

In D88831#2311800 , @Eugene.Zelenko 
wrote:

> Does this check make sense in content of other style guides?

I'd +1 to moving it to `readability`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88831

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

[PATCH] D88829: [OpenMP][RTL] Remove dead code

2020-10-05 Thread Johannes Doerfert via Phabricator via cfe-commits

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

LGTM, thanks for removing dead code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88829

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

[PATCH] D88829: [OpenMP][RTL] Remove dead code

2020-10-05 Thread Jon Chesterfield via Phabricator via cfe-commits

JonChesterfield accepted this revision.
JonChesterfield added a comment.

Thanks guys, will remember that as the local convention on rolling whitespace 
changes into other stuff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88829

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

[PATCH] D88594: [OpenMP] Add Error Handling for Conflicting Pointer Sizes for Target Offload

2020-10-05 Thread Johannes Doerfert via Phabricator via cfe-commits

jdoerfert accepted this revision.
jdoerfert added a comment.

LGTM, a new triple makes sense here, we don't support arbitrary combinations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88594

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

[PATCH] D88831: [clang-tidy] Remove obsolete checker google-runtime-references

2020-10-05 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware added a comment.

In D88831#2311802 , @lebedev.ri wrote:

> In D88831#2311800 , @Eugene.Zelenko 
> wrote:
>
>> Does this check make sense in content of other style guides?
>
> I'd +1 to moving it to `readability`.

IMHO this check never made any sense. There **was** a //Google//-specific rule 
for this, that is why it was placed in `google` not in `readability`. 
Generally, forcing programmers not to use non-const reference parameters and 
force them the old //C// way (pointers) does not make any sense. I wonder why 
this rule was there in //Google//, but outside of it I never saw such a rule. I 
would surely not place such check into `readability` because following such 
rule reduces the readability instead of improving it. However, the main point 
is that even for //Google// there is no such rule anymore thus I see no point 
to keep this check.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88831

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

[PATCH] D87774: [flang] Introduce DiagnosticConsumer classes in libflangFrontend

2020-10-05 Thread sameeran joshi via Phabricator via cfe-commits

sameeranjoshi accepted this revision.
sameeranjoshi added a comment.

Thanks for patch.
Looks good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87774

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

[PATCH] D86841: [clang] Add mustprogress and llvm.loop.mustprogress attribute deduction

2020-10-05 Thread Johannes Doerfert via Phabricator via cfe-commits

jdoerfert added a subscriber: jyknight.
jdoerfert added a comment.

One minor nit from me.

@jyknight @aaron.ballman @rjmccall any more thoughts?




Comment at: clang/lib/CodeGen/CGStmt.cpp:801
+ getLangOpts().CPlusPlus11 || getLangOpts().CPlusPlus14 ||
+ getLangOpts().CPlusPlus17 || getLangOpts().C2x) {
+LoopMustProgress = true;

atmnpatel wrote:
> aqjune wrote:
> > A silly question: does old C/C++ not guarantee that loops should make 
> > forward progress?
> Nope, it was introduced explicitly simultaneously in C11 (6.8.5p6) and C++11 
> has it implicitly through [intro.progress].
Move this condition into a helper function somewhere with documentation. If we 
use another (similar) condition the same. This has to be updated over time and 
that way makes it much easier (also to read).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86841

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

[PATCH] D88814: [clangd] Enable partial namespace matches for workspace symbols

2020-10-05 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:74
 
   auto Names = splitQualifiedName(Query);
 

Add a comment here (or elsewhere, I guess) about how qualified names are 
handled.

- exact namespace: boosted on the index side
- approximate namespace (e.g. substring): included using "all scopes" logic
- non-matching namespace (no subtring match): filtered out from index-provided 
results



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:83
+  // Boost symbols from desired namespace.
+  if (!Req.AnyScope || !Names.first.empty())
 Req.Scopes = {std::string(Names.first)};

This expression doesn't make sense (except in context of the above code). 
Variables are cheap!

```
LeadingColons = consume_front("::");
Req.AnyScope = !LeadingColons
if (LeadingColons || !Names.first.empty())
  ...
```



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:86
   if (Limit)
 Req.Limit = Limit;
   TopN Top(

Given the dynamic filter, we should request a greater multiple here (this time 
if anyscope && Names.first.empty is the right logic!)

This gives us a second class of regression :-) but we can tune the multiple to 
control it. I'd suggest 5 or so



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:93
+llvm::StringRef ScopeRef = Scope;
+// Fuzzfind might return symbols from irrelevant namespaces if query was 
not
+// fully-qualified, drop those.

nit: fuzzyfind



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:95
+// fully-qualified, drop those.
+if (!ScopeRef.contains(Names.first))
+  return;

Pull out a `approximateScopeMatch(scope, query)` or so function?

Substring isn't quite right - fine for now with a fixme, but might as well pull 
out the function now so we don't make a mess when the code grows.



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:111
 Quality.merge(Sym);
 SymbolRelevanceSignals Relevance;
 Relevance.Name = Sym.Name;

Would be nice to incorporate exact vs approximate scope match here: people 
complain a lot when an exact string match ranks below an approximate one.

I don't think ScopeDistance works as-is, because it assumes the reference point 
(query) is absolute.
You could consider NeedsFixIts or applying a multiplier to NameMatch (0.3?) or 
InBaseClass (all of these are a stretch I guess)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88814

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

[PATCH] D88737: [AIX] Turn -fdata-sections on by default in Clang

2020-10-05 Thread Digger via Phabricator via cfe-commits

DiggerLin added inline comments.



Comment at: clang/include/clang/Basic/CodeGenOptions.def:46
  ///< aliases to base ctors when possible.
-CODEGENOPT(DataSections  , 1, 0) ///< Set when -fdata-sections is enabled.
+CODEGENOPT(DataSections  , 1, 0) ///< Set by default, or when 
-f[no-]data-sections.
+CODEGENOPT(HasExplicitDataSections, 1, 0) ///< Set when -f[no-]data-sections 
is set.

turn on by default , it should be changed to CODEGENOPT(DataSections  , 1, 
1) ?



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

https://reviews.llvm.org/D88737

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

[PATCH] D87029: [AIX] Implement AIX special bitfield related alignment rules

2020-10-05 Thread Sean Fertile via Phabricator via cfe-commits

sfertile added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:16447
+
+bool AIXBitfieldViolation = false;
+if (const BuiltinType *BTy = FieldTy.getTypePtr()->getAs()) {

Can  this change can be split out into its own patch? If it can i would suggest 
doing so.



Comment at: clang/test/Layout/aix-oversized-bitfield.cpp:1
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fsyntax-only -verify %s
+

Is it possible to run this test as both C and C++?


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

https://reviews.llvm.org/D87029

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

[clang] eaf7329 - [OpenMP] Add Error Handling for Conflicting Pointer Sizes for Target Offload

2020-10-05 Thread Joseph Huber via cfe-commits


Author: Joseph Huber
Date: 2020-10-05T11:02:13-04:00
New Revision: eaf73293cb6b8d45dd85ffced57aea7ad4177754

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

LOG: [OpenMP] Add Error Handling for Conflicting Pointer Sizes for Target 
Offload

Summary:
This patch adds an error to Clang that detects if OpenMP offloading is
used between two architectures with incompatible pointer sizes. This
ensures that the data mapping can be done correctly and solves an issue
in code generation generating the wrong size pointer. This patch adds a
new lit substitution, %omp_powerpc_triple that, if the system is 32-bit or
64-bit, sets the powerpc triple accordingly. This was required to fix
some OpenMP tests that automatically populated the target architecture.

Reviewers: jdoerfert

Subscribers: cfe-commits guansong sstefan1 yaxunl delcypher

Tags: OpenMP clang LLVM

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

Added: 
clang/test/OpenMP/target_incompatible_architecture_messages.cpp

Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
clang/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp
clang/test/OpenMP/distribute_parallel_for_simd_if_codegen.cpp
clang/test/OpenMP/distribute_parallel_for_simd_num_threads_codegen.cpp
clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
clang/test/OpenMP/target_teams_distribute_parallel_for_if_codegen.cpp
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_if_codegen.cpp
clang/test/OpenMP/teams_distribute_parallel_for_if_codegen.cpp
clang/test/OpenMP/teams_distribute_parallel_for_simd_if_codegen.cpp
llvm/utils/lit/lit/llvm/config.py

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 3bf1bb19b7ae..29bc19e5a84e 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -253,6 +253,7 @@ def err_drv_optimization_remark_format : Error<
   "unknown remark serializer format: '%0'">;
 def err_drv_no_neon_modifier : Error<"[no]neon is not accepted as modifier, 
please use [no]simd instead">;
 def err_drv_invalid_omp_target : Error<"OpenMP target is invalid: '%0'">;
+def err_drv_incompatible_omp_arch : Error<"OpenMP target architecture '%0' 
pointer size is incompatible with host '%1'">;
 def err_drv_omp_host_ir_file_not_found : Error<
   "The provided host compiler IR file '%0' is required to generate code for 
OpenMP target regions but cannot be found.">;
 def err_drv_omp_host_target_not_supported : Error<

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index b402f53cc765..bbdf0e3be7ae 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3206,6 +3206,14 @@ static void ParseLangArgs(LangOptions &Opts, ArgList 
&Args, InputKind IK,
 TT.getArch() == llvm::Triple::x86 ||
 TT.getArch() == llvm::Triple::x86_64))
 Diags.Report(diag::err_drv_invalid_omp_target) << A->getValue(i);
+  else if ((T.isArch64Bit() && TT.isArch32Bit()) ||
+   (T.isArch64Bit() && TT.isArch16Bit()) ||
+   (T.isArch32Bit() && TT.isArch64Bit()) ||
+   (T.isArch32Bit() && TT.isArch16Bit()) ||
+   (T.isArch16Bit() && TT.isArch32Bit()) ||
+   (T.isArch16Bit() && TT.isArch64Bit()))
+Diags.Report(diag::err_drv_incompatible_omp_arch)
+<< A->getValue(i) << T.str();
   else
 Opts.OMPTargetTriples.push_back(TT);
 }

diff  --git a/clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp 
b/clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
index c62832a2705f..2b766f136d1d 100644
--- a/clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
+++ b/clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
@@ -1,19 +1,19 @@
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 
-fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple %itanium_abi_triple 
-emit-llvm %s -o - | FileCheck %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 
-fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple 
%itanium_abi_triple -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 
-fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple %itanium_abi_triple 
-std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 
-fopenmp-targets=%omp_powerpc_triple -x c++ -triple %itanium_abi_triple 
-emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fo

[PATCH] D88594: [OpenMP] Add Error Handling for Conflicting Pointer Sizes for Target Offload

2020-10-05 Thread Joseph Huber 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 rGeaf73293cb6b: [OpenMP] Add Error Handling for Conflicting 
Pointer Sizes for Target Offload (authored by jhuber6).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88594

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_num_threads_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
  clang/test/OpenMP/target_incompatible_architecture_messages.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_simd_if_codegen.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_simd_if_codegen.cpp
  llvm/utils/lit/lit/llvm/config.py

Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -456,6 +456,8 @@
   self.make_itanium_abi_triple(self.config.target_triple)))
 self.config.substitutions.append(('%ms_abi_triple',
   self.make_msabi_triple(self.config.target_triple)))
+self.config.substitutions.append(('%omp_powerpc_triple',
+  'powerpc' + str(sys.hash_info.width) + 'le-ibm-linux-gnu'))
 self.config.substitutions.append(
 ('%resource_dir', builtin_include_dir))
 
Index: clang/test/OpenMP/teams_distribute_parallel_for_simd_if_codegen.cpp
===
--- clang/test/OpenMP/teams_distribute_parallel_for_simd_if_codegen.cpp
+++ clang/test/OpenMP/teams_distribute_parallel_for_simd_if_codegen.cpp
@@ -1,16 +1,16 @@
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix OMP45
-// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple %itanium_abi_triple -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple %itanium_abi_triple -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefix CHECK --check-prefix OMP45
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=50 -DOMP5 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix OMP50
-// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -DOMP5 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple %itanium_abi_triple -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -DOMP5 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple %itanium_abi_triple -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefix CHECK --check-prefix OMP50
-
-// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=45 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=45 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple %itanium_abi_triple -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=45 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple %itanium_abi_triple -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=50 -DOMP5 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -DOMP5 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple %itanium_abi_triple -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -DOMP5 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple %itanium_abi_triple -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 -fopenmp-targets=%omp_powerpc_triple -x c++ -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix OMP45
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -fopenmp-targets=%omp_powerpc_triple -x c+

Re: [PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

2020-10-05 Thread Breno Guimarães via cfe-commits

Nice! I'm glad to see the patch being revived.
Thanks for that :)

Em seg, 5 de out de 2020 10:48, fiesh via Phabricator <
revi...@reviews.llvm.org> escreveu:

> fiesh added a comment.
>
> Continued in https://reviews.llvm.org/D88833
>
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D31130/new/
>
> https://reviews.llvm.org/D31130
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

2020-10-05 Thread Melanie Blower via Phabricator via cfe-commits

mibintc added a comment.

I neglected to hit the "submit" button last week, these are the inline 
questions I have.

Also in addition to the comments above, check-clang is showing 1 LIT test 
failing AST/const-fpfeatures.cpp
I believe the clang constant folder is returning False for the expression, and 
the constant folding is being done by the LLVM constant folder
llvm-project/clang/test/AST/const-fpfeatures.cpp:21:11: error: CHECK: expected 
string not found in input
// CHECK: @V1 = {{.*}} float 1.00e+00

  ^

:1:1: note: scanning from here
; ModuleID = 
'/export/iusers/mblower/sandbox/llorg/llvm-project/clang/test/AST/const-fpfeatures.cpp'
^
:24:1: note: possible intended match here
@V1 = global float 0.00e+00, align 4

The reason i think clang constant folder/my patch/ isn't to blame for faulty 
logic is because if I change the test to make V1 constexpr, there is an error 
message "not constant". So i'm not sure if the bug is in clang or the test case 
needs to be fixed?




Comment at: clang/lib/AST/ExprConstant.cpp:2734
   if (LHS.isNaN()) {
-Info.CCEDiag(E, diag::note_constexpr_float_arithmetic) << LHS.isNaN();
+Info.FFDiag(E, diag::note_constexpr_float_arithmetic) << LHS.isNaN();
 return Info.noteUndefinedBehavior();

This should be FFDiag?



Comment at: clang/lib/AST/ExprConstant.cpp:12302
+E->getFPFeaturesInEffect(Info.Ctx.getLangOpts()).isFPConstrained()) {
+  // Note: Compares may raise invalid in some cases involving NaN or sNaN.
+  Info.FFDiag(E, diag::note_constexpr_float_arithmetic_strict);

My scribbled notes say "ISO 10967 LIA-1 
Equality returns invalid if either operand is signaling NaN
Other comparisons < <= >= > return invalid if either operand is NaN".  I'm not 
putting my hands on the exact reference. 



Comment at: clang/test/CodeGen/pragma-fenv_access.c:9
+// CHECK-LABEL: @func_01
+// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, 
float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+

sepavloff wrote:
> Shall the rounding mode be `dynamic` rather than `tonearest`? If `#pragma 
> STDC FENV_ACCESS ON` is specified, it means FP environment may be changed in 
> arbitrary way and we cannot expect any particular rounding mode. Probably the 
> environment was changed in the caller of `func_01`.
Yes thanks @sepavloff !  i made this change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

2020-10-05 Thread Melanie Blower via Phabricator via cfe-commits

mibintc added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.h:410-411
 setRoundingMode(LO.getFPRoundingMode());
+// FENV access is true if rounding math is enabled.
+setAllowFEnvAccess((getRoundingMode() == llvm::RoundingMode::Dynamic));
 setFPExceptionMode(LO.getFPExceptionMode());

sepavloff wrote:
> 
> I don't think this deduction is correct.
> 
> `AllowFEnvAccess` implies that user can access FP environment in a way 
> unknown to the compiler, for example by calling external functions or by 
> using inline assembler. So if `AllowFEnvAccess` is set, the compiler must 
> assume that rounding mode is dynamic, as user can set it to any value unknown 
> to the compiler. Similarly, `FPExceptionMode` must be set to strict as user 
> may read/write exception bits in any way or may associate them with traps. 
> These deductions are valid and compiler must apply them.
> 
> The reverse is not valid. `AllowFEnvAccess` is a very coarse instrument to 
> manipulate FP environment, it prevents from many optimization techniques. In 
> contrast, setting `RoundingMode` to dynamic means only that rounding mode may 
> be different from the default value. If a user changes rounding mode by calls 
> to external functions like `fesetmode` or by using some intrinsic, the 
> compiler still can, for example, reorder FP operations, if `FPExceptionMode` 
> is `ignore`. Impact of performance in this case can be minimized.
> 
> 
Thanks @sepavloff I'll make the change you recommend


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

[PATCH] D88831: [clang-tidy] Remove obsolete checker google-runtime-references

2020-10-05 Thread Dmitri Gribenko via Phabricator via cfe-commits

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

Google C++ style guide does not have this rule anymore. Thanks for the cleanup!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88831

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

[clang] 1dce692 - Revert "[OpenMP] Add Error Handling for Conflicting Pointer Sizes for Target Offload"

2020-10-05 Thread Joseph Huber via cfe-commits


Author: Joseph Huber
Date: 2020-10-05T12:35:39-04:00
New Revision: 1dce692de1896412693f25a3afb4818883a611e7

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

LOG: Revert "[OpenMP] Add Error Handling for Conflicting Pointer Sizes for 
Target Offload"

Reverting because detecting architecture size doesn't work on all
platforms.

This reverts commit eaf73293cb6b8d45dd85ffced57aea7ad4177754.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
clang/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp
clang/test/OpenMP/distribute_parallel_for_simd_if_codegen.cpp
clang/test/OpenMP/distribute_parallel_for_simd_num_threads_codegen.cpp
clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
clang/test/OpenMP/target_teams_distribute_parallel_for_if_codegen.cpp
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_if_codegen.cpp
clang/test/OpenMP/teams_distribute_parallel_for_if_codegen.cpp
clang/test/OpenMP/teams_distribute_parallel_for_simd_if_codegen.cpp
llvm/utils/lit/lit/llvm/config.py

Removed: 
clang/test/OpenMP/target_incompatible_architecture_messages.cpp



diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 29bc19e5a84e..3bf1bb19b7ae 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -253,7 +253,6 @@ def err_drv_optimization_remark_format : Error<
   "unknown remark serializer format: '%0'">;
 def err_drv_no_neon_modifier : Error<"[no]neon is not accepted as modifier, 
please use [no]simd instead">;
 def err_drv_invalid_omp_target : Error<"OpenMP target is invalid: '%0'">;
-def err_drv_incompatible_omp_arch : Error<"OpenMP target architecture '%0' 
pointer size is incompatible with host '%1'">;
 def err_drv_omp_host_ir_file_not_found : Error<
   "The provided host compiler IR file '%0' is required to generate code for 
OpenMP target regions but cannot be found.">;
 def err_drv_omp_host_target_not_supported : Error<

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index bbdf0e3be7ae..b402f53cc765 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3206,14 +3206,6 @@ static void ParseLangArgs(LangOptions &Opts, ArgList 
&Args, InputKind IK,
 TT.getArch() == llvm::Triple::x86 ||
 TT.getArch() == llvm::Triple::x86_64))
 Diags.Report(diag::err_drv_invalid_omp_target) << A->getValue(i);
-  else if ((T.isArch64Bit() && TT.isArch32Bit()) ||
-   (T.isArch64Bit() && TT.isArch16Bit()) ||
-   (T.isArch32Bit() && TT.isArch64Bit()) ||
-   (T.isArch32Bit() && TT.isArch16Bit()) ||
-   (T.isArch16Bit() && TT.isArch32Bit()) ||
-   (T.isArch16Bit() && TT.isArch64Bit()))
-Diags.Report(diag::err_drv_incompatible_omp_arch)
-<< A->getValue(i) << T.str();
   else
 Opts.OMPTargetTriples.push_back(TT);
 }

diff  --git a/clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp 
b/clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
index 2b766f136d1d..c62832a2705f 100644
--- a/clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
+++ b/clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
@@ -1,19 +1,19 @@
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 
-fopenmp-targets=%omp_powerpc_triple -x c++ -triple %itanium_abi_triple 
-emit-llvm %s -o - | FileCheck %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 
-fopenmp-targets=%omp_powerpc_triple -x c++ -std=c++11 -triple 
%itanium_abi_triple -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 
-fopenmp-targets=%omp_powerpc_triple -x c++ -triple %itanium_abi_triple 
-std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 
-fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple %itanium_abi_triple 
-emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 
-fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple 
%itanium_abi_triple -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 
-fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple %itanium_abi_triple 
-std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=45 
-fopenmp-targets=%omp_powerpc_triple -x c++ -triple %itanium_abi_triple 
-emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
-// R

[PATCH] D88754: [clang] Add a test for CGDebugInfo treatment of blocks

2020-10-05 Thread Adrian Prantl via Phabricator via cfe-commits

aprantl added a comment.

I think that this should either hardcode a 64-bit architecture or it shouldn't 
test for explicit offsets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88754

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

[PATCH] D88594: [OpenMP] Add Error Handling for Conflicting Pointer Sizes for Target Offload

2020-10-05 Thread Joseph Huber via Phabricator via cfe-commits

jhuber6 updated this revision to Diff 296204.
jhuber6 added a comment.

Changing method of determining architecture.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88594

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_num_threads_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
  clang/test/OpenMP/target_incompatible_architecture_messages.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_simd_if_codegen.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_simd_if_codegen.cpp
  llvm/utils/lit/lit/llvm/config.py

Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -456,6 +456,8 @@
   self.make_itanium_abi_triple(self.config.target_triple)))
 self.config.substitutions.append(('%ms_abi_triple',
   self.make_msabi_triple(self.config.target_triple)))
+self.config.substitutions.append(('%omp_powerpc_triple',
+  'powerpc' + str(64 if sys.maxsize > 2**32 else 32) + 'le-ibm-linux-gnu'))
 self.config.substitutions.append(
 ('%resource_dir', builtin_include_dir))
 
Index: clang/test/OpenMP/teams_distribute_parallel_for_simd_if_codegen.cpp
===
--- clang/test/OpenMP/teams_distribute_parallel_for_simd_if_codegen.cpp
+++ clang/test/OpenMP/teams_distribute_parallel_for_simd_if_codegen.cpp
@@ -1,16 +1,16 @@
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix OMP45
-// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple %itanium_abi_triple -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple %itanium_abi_triple -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefix CHECK --check-prefix OMP45
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=50 -DOMP5 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix OMP50
-// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -DOMP5 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple %itanium_abi_triple -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -DOMP5 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple %itanium_abi_triple -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefix CHECK --check-prefix OMP50
-
-// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=45 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=45 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple %itanium_abi_triple -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=45 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple %itanium_abi_triple -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=50 -DOMP5 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -DOMP5 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple %itanium_abi_triple -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -DOMP5 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple %itanium_abi_triple -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 -fopenmp-targets=%omp_powerpc_triple -x c++ -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix OMP45
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -fopenmp-targets=%omp_powerpc_triple -x c++ -std=c++11 -triple %itanium_abi_triple -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -fopenmp-targets=%omp_p

[PATCH] D87774: [flang] Introduce DiagnosticConsumer classes in libflangFrontend

2020-10-05 Thread Andrzej Warzynski 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 rG8d51d37e0628: [flang] Introduce DiagnosticConsumer classes 
in libflangFrontend (authored by awarzynski).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87774

Files:
  clang/include/clang/Driver/Options.td
  flang/include/flang/Frontend/CompilerInvocation.h
  flang/include/flang/Frontend/TextDiagnostic.h
  flang/include/flang/Frontend/TextDiagnosticBuffer.h
  flang/include/flang/Frontend/TextDiagnosticPrinter.h
  flang/lib/Frontend/CMakeLists.txt
  flang/lib/Frontend/CompilerInstance.cpp
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/TextDiagnostic.cpp
  flang/lib/Frontend/TextDiagnosticBuffer.cpp
  flang/lib/Frontend/TextDiagnosticPrinter.cpp
  flang/test/Flang-Driver/driver-error-cc1.c
  flang/test/Flang-Driver/driver-error-cc1.cpp
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Flang-Driver/driver-version.f90
  flang/test/Flang-Driver/missing-input.f90
  flang/tools/flang-driver/driver.cpp
  flang/tools/flang-driver/fc1_main.cpp
  flang/unittests/Frontend/CompilerInstanceTest.cpp

Index: flang/unittests/Frontend/CompilerInstanceTest.cpp
===
--- flang/unittests/Frontend/CompilerInstanceTest.cpp
+++ flang/unittests/Frontend/CompilerInstanceTest.cpp
@@ -7,8 +7,8 @@
 //===--===//
 
 #include "flang/Frontend/CompilerInstance.h"
+#include "flang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Basic/DiagnosticOptions.h"
-#include "clang/Frontend/TextDiagnosticPrinter.h"
 
 #include "gtest/gtest.h"
 
@@ -21,7 +21,7 @@
   // 1. Set-up a basic DiagnosticConsumer
   std::string diagnosticOutput;
   llvm::raw_string_ostream diagnosticsOS(diagnosticOutput);
-  auto diagPrinter = std::make_unique(
+  auto diagPrinter = std::make_unique(
   diagnosticsOS, new clang::DiagnosticOptions());
 
   // 2. Create a CompilerInstance (to manage a DiagnosticEngine)
Index: flang/tools/flang-driver/fc1_main.cpp
===
--- flang/tools/flang-driver/fc1_main.cpp
+++ flang/tools/flang-driver/fc1_main.cpp
@@ -14,9 +14,9 @@
 
 #include "flang/Frontend/CompilerInstance.h"
 #include "flang/Frontend/CompilerInvocation.h"
+#include "flang/Frontend/TextDiagnosticBuffer.h"
 #include "flang/FrontendTool/Utils.h"
 #include "clang/Driver/DriverDiagnostic.h"
-#include "clang/Frontend/TextDiagnosticBuffer.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/OptTable.h"
@@ -34,18 +34,22 @@
   if (!flang->HasDiagnostics())
 return 1;
 
+  // We will buffer diagnostics from argument parsing so that we can output
+  // them using a well formed diagnostic object.
+  TextDiagnosticBuffer *diagsBuffer = new TextDiagnosticBuffer;
+
   // Create CompilerInvocation - use a dedicated instance of DiagnosticsEngine
   // for parsing the arguments
   llvm::IntrusiveRefCntPtr diagID(
   new clang::DiagnosticIDs());
   llvm::IntrusiveRefCntPtr diagOpts =
   new clang::DiagnosticOptions();
-  clang::TextDiagnosticBuffer *diagsBuffer = new clang::TextDiagnosticBuffer;
   clang::DiagnosticsEngine diags(diagID, &*diagOpts, diagsBuffer);
   bool success =
   CompilerInvocation::CreateFromArgs(flang->GetInvocation(), argv, diags);
 
   diagsBuffer->FlushDiagnostics(flang->getDiagnostics());
+
   if (!success)
 return 1;
 
Index: flang/tools/flang-driver/driver.cpp
===
--- flang/tools/flang-driver/driver.cpp
+++ flang/tools/flang-driver/driver.cpp
@@ -11,11 +11,12 @@
 //
 //===--===//
 #include "clang/Driver/Driver.h"
+#include "flang/Frontend/CompilerInvocation.h"
+#include "flang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Driver/Compilation.h"
-#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/Option/ArgList.h"
@@ -23,6 +24,8 @@
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/VirtualFileSystem.h"
 
+using llvm::StringRef;
+
 // main frontend method. Lives inside fc1_main.cpp
 extern int fc1_main(llvm::ArrayRef argv, const char *argv0);
 
@@ -37,6 +40,17 @@
 static clang::DiagnosticOptions *CreateAndPopulateDiagOpts(
 llvm::ArrayRef argv) {
   auto *diagOpts = new clang::DiagnosticOptions;
+
+  // Ignore missingArgCount and the return value of ParseDiagnosticArgs.
+  // Any errors that would be diagnosed here will also be diagnosed later,
+  // when the DiagnosticsEngine actually exists.
+  unsigned mi

[clang] 8d51d37 - [flang] Introduce DiagnosticConsumer classes in libflangFrontend

2020-10-05 Thread Andrzej Warzynski via cfe-commits


Author: Andrzej Warzynski
Date: 2020-10-05T17:46:44+01:00
New Revision: 8d51d37e0628bde3eb5a3200507ba7135dfc2751

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

LOG: [flang] Introduce DiagnosticConsumer classes in libflangFrontend

Currently Flang uses TextDiagnostic, TextDiagnosticPrinter &
TestDiagnosticBuffer classes from Clang (more specifically, from
libclangFrontend). This patch introduces simplified equivalents of these
classes in Flang (i.e. it removes the dependency on libclangFrontend).

Flang only needs these diagnostics classes for the compiler driver
diagnostics. This is unlike in Clang in which similar diagnostic classes
are used for e.g. Lexing/Parsing/Sema diagnostics. For this reason, the
implementations introduced here are relatively basic. We can extend them
in the future if this is required.

This patch also enhances how the diagnostics are printed. In particular,
this is the diagnostic that you'd get _before_  the changes introduced here
(no text formatting):

```
$ bin/flang-new
error: no input files
```

This is the diagnostic that you get _after_ the changes introduced here
(in terminals that support it, the text is formatted - bold + red):

```
$ bin/flang-new
flang-new: error: no input files
```

Tests are updated accordingly and options related to enabling/disabling
color diagnostics are flagged as supported by Flang.

Reviewed By: sameeranjoshi, CarolineConcatto

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

Added: 
flang/include/flang/Frontend/TextDiagnostic.h
flang/include/flang/Frontend/TextDiagnosticBuffer.h
flang/include/flang/Frontend/TextDiagnosticPrinter.h
flang/lib/Frontend/TextDiagnostic.cpp
flang/lib/Frontend/TextDiagnosticBuffer.cpp
flang/lib/Frontend/TextDiagnosticPrinter.cpp

Modified: 
clang/include/clang/Driver/Options.td
flang/include/flang/Frontend/CompilerInvocation.h
flang/lib/Frontend/CMakeLists.txt
flang/lib/Frontend/CompilerInstance.cpp
flang/lib/Frontend/CompilerInvocation.cpp
flang/test/Flang-Driver/driver-error-cc1.c
flang/test/Flang-Driver/driver-error-cc1.cpp
flang/test/Flang-Driver/driver-help.f90
flang/test/Flang-Driver/driver-version.f90
flang/test/Flang-Driver/missing-input.f90
flang/tools/flang-driver/driver.cpp
flang/tools/flang-driver/fc1_main.cpp
flang/unittests/Frontend/CompilerInstanceTest.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 18a123476253..e65a68c0deaa 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -876,7 +876,8 @@ def fclang_abi_compat_EQ : Joined<["-"], 
"fclang-abi-compat=">, Group, MetaVarName<"">, 
Values<".,latest">,
   HelpText<"Attempt to match the ABI of Clang ">;
 def fclasspath_EQ : Joined<["-"], "fclasspath=">, Group;
-defm color_diagnostics : OptInFFlag<"color-diagnostics", "Enable", "Disable", 
" colors in diagnostics", [CoreOption]>;
+defm color_diagnostics : OptInFFlag<"color-diagnostics", "Enable", "Disable", 
" colors in diagnostics",
+  [CoreOption, FlangOption]>;
 def fdiagnostics_color : Flag<["-"], "fdiagnostics-color">, Group,
   Flags<[CoreOption, DriverOption]>;
 def fdiagnostics_color_EQ : Joined<["-"], "fdiagnostics-color=">, 
Group;

diff  --git a/flang/include/flang/Frontend/CompilerInvocation.h 
b/flang/include/flang/Frontend/CompilerInvocation.h
index 0fa169fd1620..05f93293d0a5 100644
--- a/flang/include/flang/Frontend/CompilerInvocation.h
+++ b/flang/include/flang/Frontend/CompilerInvocation.h
@@ -11,8 +11,17 @@
 #include "flang/Frontend/FrontendOptions.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
+#include "llvm/Option/ArgList.h"
 
 namespace Fortran::frontend {
+
+/// Fill out Opts based on the options given in Args.
+///
+/// When errors are encountered, return false and, if Diags is non-null,
+/// report the error(s).
+bool ParseDiagnosticArgs(clang::DiagnosticOptions &opts,
+llvm::opt::ArgList &args, bool defaultDiagColor = true);
+
 class CompilerInvocationBase {
 public:
   /// Options controlling the diagnostic engine.$

diff  --git a/flang/include/flang/Frontend/TextDiagnostic.h 
b/flang/include/flang/Frontend/TextDiagnostic.h
new file mode 100644
index ..f803058c88c5
--- /dev/null
+++ b/flang/include/flang/Frontend/TextDiagnostic.h
@@ -0,0 +1,70 @@
+//===--- TextDiagnostic.h - Text Diagnostic Pretty-Printing -*- 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
+//
+//===--

[PATCH] D88363: [CodeGen] Improve likelihood attribute branch weights

2020-10-05 Thread Jeremy Morse via Phabricator via cfe-commits

jmorse added a comment.

Hi -- We (Sony) are running into a bit of difficulty with the test for this 
change, as it relies on the configuration of the -O1 optimisation pipeline. 
Would it be possible to reduce down to a frontend test, and then tests for 
whatever passes are to interpret the IR produced by clang?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88363

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

[PATCH] D84226: [AST][RecoveryExpr] Part1: Support dependent binary operator in C for error recovery.

2020-10-05 Thread Haojian Wu via Phabricator via cfe-commits

hokein updated this revision to Diff 296218.
hokein marked 2 inline comments as done.
hokein added a comment.

address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84226

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/Sema/SemaExpr.cpp
  clang/test/AST/ast-dump-recovery.c
  clang/test/Sema/error-dependence.c

Index: clang/test/Sema/error-dependence.c
===
--- /dev/null
+++ clang/test/Sema/error-dependence.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -frecovery-ast -fno-recovery-ast-type %s
+
+int call(int); // expected-note {{'call' declared here}}
+
+void test1(int s) {
+  // verify "assigning to 'int' from incompatible type ''" is
+  // not emitted.
+  s = call(); // expected-error {{too few arguments to function call}}
+}
Index: clang/test/AST/ast-dump-recovery.c
===
--- clang/test/AST/ast-dump-recovery.c
+++ clang/test/AST/ast-dump-recovery.c
@@ -42,11 +42,33 @@
 
 void test2() {
   int* ptr;
-  // FIXME: the top-level expr should be a binary operator.
-  // CHECK:  ImplicitCastExpr {{.*}} contains-errors 
-  // CHECK-NEXT: `-RecoveryExpr {{.*}} contains-errors lvalue
-  // CHECK-NEXT:   |-DeclRefExpr {{.*}} 'ptr' 'int *'
-  // CHECK-NEXT:   `-RecoveryExpr {{.*}}
-  // CHECK-NEXT: `-DeclRefExpr {{.*}} 'some_func'
+  // CHECK: BinaryOperator {{.*}} 'int *' contains-errors '='
+  // CHECK-NEXT: |-DeclRefExpr {{.*}} 'ptr' 'int *'
+  // CHECK-NEXT: `-RecoveryExpr {{.*}}
+  // CHECK-NEXT:   `-DeclRefExpr {{.*}} 'some_func'
   ptr = some_func(); // should not crash
+
+  int compoundOp;
+  // CHECK: CompoundAssignOperator {{.*}} 'int' contains-errors '+='
+  // CHECK-NEXT: |-DeclRefExpr {{.*}} 'compoundOp'
+  // CHECK-NEXT: `-RecoveryExpr {{.*}} contains-errors
+  // CHECK-NEXT:   `-DeclRefExpr {{.*}} 'some_func'
+  compoundOp += some_func();
+
+  // CHECK: BinaryOperator {{.*}} 'int' contains-errors '||'
+  // CHECK-NEXT: |-RecoveryExpr {{.*}}
+  // CHECK-NEXT: | `-DeclRefExpr {{.*}} 'some_func'
+  // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
+  some_func() || 1;
+
+  // CHECK: BinaryOperator {{.*}} '' contains-errors ','
+  // CHECK-NEXT: |-IntegerLiteral {{.*}} 'int' 1
+  // CHECK-NEXT: `-RecoveryExpr {{.*}}
+  // CHECK-NEXT:   `-DeclRefExpr {{.*}} 'some_func'
+  1, some_func();
+  // CHECK: BinaryOperator {{.*}} 'int' contains-errors ','
+  // CHECK-NEXT: |-RecoveryExpr {{.*}} ''
+  // CHECK-NEXT: | `-DeclRefExpr {{.*}} 'some_func'
+  // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
+  some_func(), 1;
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -24,6 +24,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ExprOpenMP.h"
+#include "clang/AST/OperationKinds.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/Builtins.h"
@@ -13683,7 +13684,7 @@
 CorrectDelayedTyposInBinOp(Sema &S, BinaryOperatorKind Opc, Expr *LHSExpr,
Expr *RHSExpr) {
   ExprResult LHS = LHSExpr, RHS = RHSExpr;
-  if (!S.getLangOpts().CPlusPlus) {
+  if (!S.Context.isDependenceAllowed()) {
 // C cannot handle TypoExpr nodes on either side of a binop because it
 // doesn't handle dependent types properly, so make sure any TypoExprs have
 // been dealt with before checking the operands.
@@ -14364,6 +14365,49 @@
   return BuildOverloadedBinOp(*this, S, OpLoc, Opc, LHSExpr, RHSExpr);
   }
 
+  if (getLangOpts().RecoveryAST &&
+  (LHSExpr->isTypeDependent() || RHSExpr->isTypeDependent())) {
+assert(!getLangOpts().CPlusPlus);
+assert((LHSExpr->containsErrors() || RHSExpr->containsErrors()) &&
+   "Should only occur in error-recovery path.");
+if (BinaryOperator::isCompoundAssignmentOp(Opc))
+  // C [6.15.16] p3:
+  // An assignment expression has the value of the left operand after the
+  // assignment, but is not an lvalue.
+  return CompoundAssignOperator::Create(
+  Context, LHSExpr, RHSExpr, Opc,
+  LHSExpr->getType().getUnqualifiedType(), VK_RValue, OK_Ordinary,
+  OpLoc, CurFPFeatureOverrides());
+switch (Opc) {
+case BO_Assign:
+  return BinaryOperator::Create(Context, LHSExpr, RHSExpr, Opc,
+LHSExpr->getType().getUnqualifiedType(),
+VK_RValue, OK_Ordinary, OpLoc,
+CurFPFeatureOverrides());
+case BO_LT:
+case BO_GT:
+case BO_LE:
+case BO_GE:
+case BO_EQ:
+case BO_NE:
+
+case BO_LAnd:
+case BO_LOr:
+  // These operators have a fixed result type regardless of operands.
+  return BinaryOperato

[PATCH] D84226: [AST][RecoveryExpr] Part1: Support dependent binary operator in C for error recovery.

2020-10-05 Thread Haojian Wu via Phabricator via cfe-commits

hokein added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:14365
+   "Should only occur in error-recovery path.");
+if (BinaryOperator::isCompoundAssignmentOp(Opc))
+  return CompoundAssignOperator::Create(

sammccall wrote:
> isAssignmentOp instead? including = itself
Simple assignment `=` doesn't belong to `CompoundAssignOperator`, it should be 
`BinaryOperator`.

Added the handling logic in the switch-case below.



Comment at: clang/test/AST/ast-dump-recovery.c:45
   int* ptr;
-  // FIXME: the top-level expr should be a binary operator.
-  // CHECK:  ImplicitCastExpr {{.*}} contains-errors 
-  // CHECK-NEXT: `-RecoveryExpr {{.*}} contains-errors lvalue
-  // CHECK-NEXT:   |-DeclRefExpr {{.*}} 'ptr' 'int *'
-  // CHECK-NEXT:   `-RecoveryExpr {{.*}}
-  // CHECK-NEXT: `-DeclRefExpr {{.*}} 'some_func'
+  // CHECK: BinaryOperator {{.*}} contains-errors '='
+  // CHECK-NEXT: |-DeclRefExpr {{.*}} 'ptr' 'int *'

sammccall wrote:
> `'int *' lvalue contains-errors '='`
oh, this spots a bug in our code -- unlike C++, it should not be a lvalue.

From  C [6.15.16] p3:
> An assignment expression has the value of the left operand after the 
> assignment, but is not an lvalue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84226

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

[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

2020-10-05 Thread Melanie Blower via Phabricator via cfe-commits

mibintc updated this revision to Diff 296221.
mibintc added a comment.

In the previous version of this patch, I enabled FENV_ACCESS if frounding-math, 
but @sepavloff commented that this deduction is not correct.  I changed the 
logic to check for the "ffp-model=strict" settings, and if those settings are 
in effect from the command line, then I also enable fenv_access.  To repeat 
what I've said elsewhere, this is the documented behavior of the "/fp:strict" 
Microsoft command line option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

Files:
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/AST/const-fpfeatures-diag.c
  clang/test/CXX/expr/expr.const/p2-0x.cpp
  clang/test/CodeGen/fp-floatcontrol-pragma.cpp
  clang/test/CodeGen/pragma-fenv_access.c
  clang/test/Parser/fp-floatcontrol-syntax.cpp
  clang/test/Parser/pragma-fenv_access.c
  clang/test/Preprocessor/pragma_unknown.c

Index: clang/test/Preprocessor/pragma_unknown.c
===
--- clang/test/Preprocessor/pragma_unknown.c
+++ clang/test/Preprocessor/pragma_unknown.c
@@ -16,15 +16,6 @@
 // CHECK: {{^}}#pragma STDC FP_CONTRACT DEFAULT{{$}}
 // CHECK: {{^}}#pragma STDC FP_CONTRACT IN_BETWEEN{{$}}
 
-#pragma STDC FENV_ACCESS ON  // expected-warning {{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS OFF
-#pragma STDC FENV_ACCESS DEFAULT
-#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS ON{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS OFF{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS DEFAULT{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS IN_BETWEEN{{$}}
-
 #pragma STDC CX_LIMITED_RANGE ON
 #pragma STDC CX_LIMITED_RANGE OFF
 #pragma STDC CX_LIMITED_RANGE DEFAULT 
Index: clang/test/Parser/pragma-fenv_access.c
===
--- /dev/null
+++ clang/test/Parser/pragma-fenv_access.c
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -ffp-exception-behavior=strict -DSTRICT -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -DCPP -DSTRICT -ffp-exception-behavior=strict -fsyntax-only -verify %s
+#ifdef CPP
+#define CONST constexpr
+#else
+#define CONST const
+#endif
+
+#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
+
+#pragma STDC FENV_ACCESS OFF
+
+float func_04(int x, float y) {
+  if (x)
+return y + 2;
+  #pragma STDC FENV_ACCESS ON // expected-error{{'#pragma STDC FENV_ACCESS' can only appear at file scope or at the start of a compound statement}}
+  return x + y;
+}
+
+#pragma STDC FENV_ACCESS ON
+int main() {
+  CONST float one = 1.0F ;
+  CONST float three = 3.0F ;
+  CONST float four = 4.0F ;
+  CONST float frac_ok = one/four;
+#if defined(CPP) & defined(STRICT)
+//expected-error@+3 {{constexpr variable 'frac' must be initialized by a constant expression}}
+//expected-note@+2 {{compile time floating point arithmetic suppressed in strict evaluation modes}}
+#endif
+  CONST float frac = one/three; // rounding
+  CONST double d = one;
+  CONST int not_too_big = 255;
+  CONST float fnot_too_big = not_too_big;
+  CONST int too_big = 0x7ff0;
+#if defined(CPP) & defined(STRICT)
+//expected-error@+6 {{constexpr variable 'fbig' must be initialized by a constant expression}}
+//expected-note@+5 {{compile time floating point arithmetic suppressed in strict evaluation modes}}
+#endif
+#if defined(CPP)
+//expected-warning@+2{{implicit conversion}}
+#endif
+  CONST float fbig = too_big; // inexact
+  if (one <= four)  return 0;
+  return -1;
+}
Index: clang/test/Parser/fp-floatcontrol-syntax.cpp
===
--- clang/test/Parser/fp-floatcontrol-syntax.cpp
+++ clang/test/Parser/fp-floatcontrol-syntax.cpp
@@ -26,19 +26,13 @@
 double a = 0.0;
 double b = 1.0;
 
-//FIXME At some point this warning will be removed, until then
-//  document the warning
-#ifdef FAST
-// expected-warning@+1{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS ON
-#else
-#pragma STDC FENV_ACCESS ON // expected-warning{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#endif
 #ifdef STRICT
 #pragma float_control(precise, off) // expected-error {{'#pragma float_control(precise, off)' is illegal when except is enabled}}
 #else
-// Currently FENV_ACCESS cannot be enabled by pr

[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

2020-10-05 Thread Melanie Blower via Phabricator via cfe-commits

mibintc added a comment.

For the LIT test clang/test/AST/const-fpfeatures.cpp, currently failing in this 
patch, the variables V1 and others are initialized via call to "global var 
init" which performs the rounding at execution time, I think that's correct not 
certain.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

[PATCH] D88659: [FE]Split SuitableAlign into two parts

2020-10-05 Thread Xiangling Liao via Phabricator via cfe-commits

Xiangling_L added a comment.

In D88659#2306403 , @jyknight wrote:

> Hm, to start with, the current state of this confuses me.
>
> In GCC, the preprocessor macro `__BIGGEST_ALIGNMENT__` was supposed to expose 
> the alignment used by `__attribute__((aligned))` (no arg specified), as well 
> the alignment used for alloca. However, this is no longer the case on x86: 
> `BIGGEST_ALIGNMENT` is 512bits with avx-512 enabled, 256bits with avx 
> enabled, and otherwise 128bits. Alloca follows this too. But, 
> `__attribute__((aligned))` was fixed at 128bit alignment, regardless of AVX 
> being enabled, in order to not break ABI compatibility with structs using 
> that. On other architectures, the 3 values seem to be always the same.
>
> In clang, we similarly have (before this patch) both 
> DefaultAlignForAttributeAligned (used for ``attribute((aligned))`), and 
> SuitableAlign (used for the predefined `__BIGGEST_ALIGNMENT__` and alignment 
> for alloca). But these values are different on very many 
> architectures...which I think is probably wrong. Furthermore, SuitableAlign 
> isn't being adjusted to be suitable for vectors, like it is in gcc, which 
> _also_ seems wrong. Looks like there's actually an earlier patch to fix that 
> which was never merged: https://reviews.llvm.org/D39313
>
> So, anyways -- back to this patch: On AIX PPC, you want alloca to align to 
> 128bits, `__attribute__((aligned))` to align to 128bits (aka 8 bytes), but 
> `__BIGGEST_ALIGNMENT__` to only be 4?
>
> That seems pretty weird, and probably wrong?

As you mentioned, without this patch, `SuitableAlign` is used for the 
predefined `__BIGGEST_ALIGNMENT__` and alignment for alloca. But on AIX, the 
__BIGGEST_ALIGNMENT__ should be 8bytes,  alloca and `__attribute__((aligned))`  
should align to 16bytes considering vector types which have to be aligned to 
16bytes, that's why we want to split `SuitableAlign`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88659

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

[PATCH] D88013: [AArch64] Correct parameter type for unsigned Neon scalar shift intrinsics

2020-10-05 Thread Eli Friedman via Phabricator via cfe-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88013

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

[PATCH] D82485: Add tests for sequences of callbacks that RecursiveASTVisitor produces

2020-10-05 Thread Nemanja Ivanovic via Phabricator via cfe-commits

nemanjai added a comment.

Hi @gribozavr do you think we can do something about this test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82485

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

[PATCH] D88786: [CUDA] Don't call __cudaRegisterVariable on C++17 inline variables

2020-10-05 Thread Artem Belevich via Phabricator via cfe-commits

tra added a comment.

> If such a variable (which has a comdat group) is discarded (a copy from 
> another
> translation unit is prevailing and selected), accessing the variable from
> outside the section group (__cuda_register_globals) is a violation of the ELF
> specification and will be rejected by linkers:

Every TU  is the whole program on the GPU side, provided we compile w/o 
`-frdc`, so there's no other TU to prevail.
I don't have a good idea yet what's the best way to handle this in CUDA, but 
not registering the variables will likely to create other issues, only visible 
at runtime. E.g. some host-side code will attempt to use cudaMemcpy() on the 
symbol and will fail, because it's not been registered, even though we do have 
all other glue in place.

Could you provide an example where this is causing an issue?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88786

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

[clang] 85d5064 - docs: add documentation describing API Notes

2020-10-05 Thread Saleem Abdulrasool via cfe-commits


Author: Saleem Abdulrasool
Date: 2020-10-05T18:29:13Z
New Revision: 85d506400081e121ef876a4c66fb337b050dcae6

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

LOG: docs: add documentation describing API Notes

API Notes are a feature which allows annotation of headers by an
auxiliary file that contains metadata for declarations pertaining to the
associated module.  This enables adding attributes to declarations
without requiring modification of the headers, enabling finer grained
control for library headers for consumers without having to modify
external headers.

Differential Revision: https://reviews.llvm.org/D88446
Reviewed By: Richard Smith, Marcel Hlopko

Added: 
clang/docs/APINotes.rst

clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.apinotes
clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h

clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKitExplicitNullability.h

clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit_private.apinotes

Modified: 


Removed: 




diff  --git a/clang/docs/APINotes.rst b/clang/docs/APINotes.rst
new file mode 100644
index ..4ac4c01cdefb
--- /dev/null
+++ b/clang/docs/APINotes.rst
@@ -0,0 +1,363 @@
+
+API Notes: Annotations Without Modifying Headers
+
+
+**The Problem:** You have headers you want to use, but you also want to add
+extra information to the API. You don't want to put that information in the
+headers themselves --- perhaps because you want to keep them clean for other
+clients, or perhaps because they're from some open source project and you don't
+want to modify them at all.
+
+**Incomplete solution:** Redeclare all the interesting parts of the API in your
+own header and add the attributes you want. Unfortunately, this:
+
+* doesn't work with attributes that must be present on a definition
+* doesn't allow changing the definition in other ways
+* requires your header to be included in any client code to take effect
+
+**Better solution:** Provide a "sidecar" file with the information you want to
+add, and have that automatically get picked up by the module-building logic in
+the compiler.
+
+That's API notes.
+
+API notes use a YAML-based file format. YAML is a format best explained by
+example, so here is a `small example`__ from the compiler test suite of API
+notes for a hypothetical "SomeKit" framework.
+
+__ test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.apinotes
+
+
+Usage
+=
+
+API notes files are found relative to the module map that defines a module,
+under the name "SomeKit.apinotes" for a module named "SomeKit". Additionally, a
+file named "SomeKit_private.apinotes" will also be picked up to go with a
+private module map. For bare modules these two files will be in the same
+directory as the corresponding module map; for framework modules, they should
+be placed in the Headers and PrivateHeaders directories, respectively. The
+module map for a private top-level framework module should be placed in the
+PrivateHeaders directory as well, though it does not need an additional
+"_private" suffix on its name.
+
+Clang will search for API notes files next to module maps only when passed the
+``-fapi-notes-modules`` option.
+
+
+Limitations
+===
+
+- Since they're identified by module name, API notes cannot be used to modify
+  arbitrary textual headers.
+
+
+"Versioned" API Notes
+=
+
+Many API notes affect how a C API is imported into Swift. In order to change
+that behavior while still remaining backwards-compatible, API notes can be
+selectively applied based on the Swift compatibility version provided to the
+compiler (e.g. ``-fapi-notes-swift-version=5``). The rule is that an
+explicitly-versioned API note applies to that version *and all earlier
+versions,* and any applicable explicitly-versioned API note takes precedence
+over an unversioned API note.
+
+
+Reference
+=
+
+An API notes file contains a YAML dictionary with the following top-level
+entries:
+
+:Name:
+
+  The name of the module (the framework name, for frameworks). Note that this
+  is always the name of a top-level module, even within a private API notes
+  file.
+
+  ::
+
+Name: MyFramework
+
+:Classes, Protocols, Tags, Typedefs, Globals, Enumerators, Functions:
+
+  Arrays of top-level declarations. Each entry in the array must have a
+  'Name' key with its Objective-C name. "Tags" refers to structs, enums, and
+  unions; "Enumerators" refers to enum cases.
+
+  ::
+
+Classes:
+- Name: MyController
+  …
+- Name: MyView
+  …
+
+:SwiftVersions:
+
+  Contains explic

[PATCH] D88786: [CUDA] Don't call __cudaRegisterVariable on C++17 inline variables

2020-10-05 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D88786#2312329 , @tra wrote:

>> If such a variable (which has a comdat group) is discarded (a copy from 
>> another
>> translation unit is prevailing and selected), accessing the variable from
>> outside the section group (__cuda_register_globals) is a violation of the ELF
>> specification and will be rejected by linkers:
>
> Every TU  is the whole program on the GPU side, provided we compile w/o 
> `-frdc`, so there's no other TU to prevail.
> I don't have a good idea yet what's the best way to handle this in CUDA, but 
> not registering the variables will likely to create other issues, only 
> visible at runtime. E.g. some host-side code will attempt to use cudaMemcpy() 
> on the symbol and will fail, because it's not been registered, even though we 
> do have all other glue in place.
>
> Could you provide an example where this is causing an issue?

If the C++17 inline variable appears in two TUs. They have the same comdat 
group. The first comdat group is prevailing and the second one is disarded. 
`__cudaRegisterVar(...)` in the second TU references a local symbol in a 
discarded section.

The previous revision (https://reviews.llvm.org/D88786?id=295997 ) drops the 
comdat, but I think it is inferior to this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88786

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

[PATCH] D88446: docs: add documentation describing API Notes

2020-10-05 Thread Saleem Abdulrasool 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 rG85d506400081: docs: add documentation describing API Notes 
(authored by compnerd).

Changed prior to commit:
  https://reviews.llvm.org/D88446?vs=295924&id=296243#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88446

Files:
  clang/docs/APINotes.rst
  
clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.apinotes
  clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h
  
clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKitExplicitNullability.h
  
clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit_private.apinotes

Index: clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit_private.apinotes
===
--- /dev/null
+++ clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit_private.apinotes
@@ -0,0 +1,15 @@
+Name: SomeKit
+Classes:
+  - Name: A
+Methods:
+  - Selector: "privateTransform:input:"
+MethodKind:  Instance
+NullabilityOfRet: N
+Nullability:  [ N, S ]
+Properties:
+  - Name: internalProperty
+Nullability: N
+Protocols:
+  - Name: InternalProtocol
+Availability: none
+AvailabilityMsg: "not for you"
Index: clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKitExplicitNullability.h
===
--- /dev/null
+++ clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKitExplicitNullability.h
@@ -0,0 +1,4 @@
+@interface A(ExplicitNullabilityProperties)
+@property (nonatomic, readwrite, retain, nonnull) A *explicitNonnullInstance;
+@property (nonatomic, readwrite, retain, nullable) A *explicitNullableInstance;
+@end
Index: clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h
===
--- /dev/null
+++ clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h
@@ -0,0 +1,60 @@
+#ifndef SOMEKIT_H
+#define SOMEKIT_H
+
+__attribute__((objc_root_class))
+@interface A
+-(A*)transform:(A*)input;
+-(A*)transform:(A*)input integer:(int)integer;
+
+@property (nonatomic, readonly, retain) A* someA;
+@property (nonatomic, retain) A* someOtherA;
+
+@property (nonatomic) int intValue;
+@end
+
+@interface B : A
+@end
+
+@interface C : A
+- (instancetype)init;
+- (instancetype)initWithA:(A*)a;
+@end
+
+@interface ProcessInfo : A
++(instancetype)processInfo;
+@end
+
+@interface A(NonNullProperties)
+@property (nonatomic, readwrite, retain) A *nonnullAInstance;
+@property (class, nonatomic, readwrite, retain) A *nonnullAInstance;
+
+@property (nonatomic, readwrite, retain) A *nonnullAClass;
+@property (class, nonatomic, readwrite, retain) A *nonnullAClass;
+
+@property (nonatomic, readwrite, retain) A *nonnullABoth;
+@property (class, nonatomic, readwrite, retain) A *nonnullABoth;
+@end
+
+#import 
+
+extern int *global_int_ptr;
+
+int *global_int_fun(int *ptr, int *ptr2);
+
+#define SOMEKIT_DOUBLE double
+
+__attribute__((objc_root_class))
+@interface OverriddenTypes
+-(int *)methodToMangle:(int *)ptr1 second:(int *)ptr2;
+@property int *intPropertyToMangle;
+@end
+
+@interface A(ImplicitGetterSetters)
+@property (nonatomic, readonly, retain) A *implicitGetOnlyInstance;
+@property (class, nonatomic, readonly, retain) A *implicitGetOnlyClass;
+
+@property (nonatomic, readwrite, retain) A *implicitGetSetInstance;
+@property (class, nonatomic, readwrite, retain) A *implicitGetSetClass;
+@end
+
+#endif
Index: clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.apinotes
===
--- /dev/null
+++ clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.apinotes
@@ -0,0 +1,98 @@
+Name: SomeKit
+Classes:
+  - Name: A
+Methods:
+  - Selector:"transform:"
+MethodKind:  Instance
+Availability:none
+AvailabilityMsg: "anything but this"
+  - Selector: "transform:integer:"
+MethodKind:  Instance
+NullabilityOfRet: N
+Nullability:  [ N, S ]
+  - Selector: "implicitGetOnlyInstance"
+MethodKind:  Instance
+Availability:none
+AvailabilityMsg: "getter gone"
+  - Selector: "implicitGetOnlyClass"
+MethodKind:  Class
+Availability:none
+AvailabilityMsg: "getter gone"
+  - Selector: "implicitGetSetInstance"
+MethodKind:  Instance
+Availability:none
+AvailabilityMsg: "getter gone"
+  - Selector: "implicitGetSetClass"
+MethodKind:  Class
+Availability:none
+AvailabilityMsg: "getter gone

[PATCH] D87029: [AIX] Implement AIX special bitfield related alignment rules

2020-10-05 Thread Sean Fertile via Phabricator via cfe-commits

sfertile added inline comments.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1643
+  } else {
+// Handle AIX oversized Long Long bitfield under 32 bit compile mode.
+if (StorageUnitSize > 32 &&

I get different results for using a long long bitfield, and using an enum with 
an underlying long long type:


```
struct S {
unsigned long long  field : 32;
};

extern "C" int printf(const char*, ...);

int main(void) {
printf("%lu %lu\n", sizeof(struct S), alignof(struct S));
}

```
```
*** Dumping AST Record Layout
 0 | struct S
0:0-31 |   unsigned long long field
   | [sizeof=4, dsize=4, align=4, preferredalign=4,
   |  nvsize=4, nvalign=4, preferrednvalign=4]
```
__

```
enum e : unsigned long long { E = 1 };

struct S {
enum e field : 32;
};

extern "C" int printf(const char*, ...);

int main(void) {
printf("%lu %lu\n", sizeof(struct S), alignof(struct S));
}
```

```
*** Dumping AST Record Layout
 0 | struct S
0:0-31 |   enum e field
   | [sizeof=8, dsize=8, align=8, preferredalign=8,
   |  nvsize=8, nvalign=8, preferrednvalign=8]
```
Shouldn't we be truncating the size/align of the enum typed bitfield as well?


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

https://reviews.llvm.org/D87029

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

[PATCH] D84226: [AST][RecoveryExpr] Part1: Support dependent binary operator in C for error recovery.

2020-10-05 Thread Sam McCall via Phabricator via cfe-commits

sammccall accepted this revision.
sammccall added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:14381
+  OpLoc, CurFPFeatureOverrides());
+switch (Opc) {
+case BO_Assign:

now the only thing that differs between the cases is the type. You could 
consider putting the type in a variable, and factoring the Create after the 
switch.



Comment at: clang/lib/Sema/SemaExpr.cpp:14393
+case BO_NE:
+
+case BO_LAnd:

(why the line break here)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84226

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

[PATCH] D88822: [clangd] Describe non-handling of most IWYU pragmas. NFC

2020-10-05 Thread Sam McCall via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG95262ee2be75: [clangd] Describe non-handling of most IWYU 
pragmas. NFC (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88822

Files:
  clang-tools-extra/clangd/index/CanonicalIncludes.h


Index: clang-tools-extra/clangd/index/CanonicalIncludes.h
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.h
+++ clang-tools-extra/clangd/index/CanonicalIncludes.h
@@ -71,6 +71,21 @@
 ///
 /// Currently it only supports IWYU private pragma:
 /// 
https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/IWYUPragmas.md#iwyu-pragma-private
+///
+/// We ignore other pragmas:
+/// - keep: this is common but irrelevant: we do not currently remove includes
+/// - export: this is common and potentially interesting, there are three 
cases:
+///* Points to a public header (common): we can suppress include2 if you
+///  already have include1. Only marginally useful.
+///* Points to a private header annotated with `private` (somewhat 
commmon):
+///  Not incrementally useful as we support private.
+///* Points to a private header without pragmas (rare). This is a reversed
+///  private pragma, and is valuable but too rare to be worthwhile.
+/// - no_include: this is about as common as private, but only affects the
+///   current file, so the value is smaller. We could add support.
+/// - friend: this is less common than private, has implementation 
difficulties,
+///   and affects behavior in a limited scope.
+/// - associated: extremely rare
 std::unique_ptr
 collectIWYUHeaderMaps(CanonicalIncludes *Includes);
 


Index: clang-tools-extra/clangd/index/CanonicalIncludes.h
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.h
+++ clang-tools-extra/clangd/index/CanonicalIncludes.h
@@ -71,6 +71,21 @@
 ///
 /// Currently it only supports IWYU private pragma:
 /// https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/IWYUPragmas.md#iwyu-pragma-private
+///
+/// We ignore other pragmas:
+/// - keep: this is common but irrelevant: we do not currently remove includes
+/// - export: this is common and potentially interesting, there are three cases:
+///* Points to a public header (common): we can suppress include2 if you
+///  already have include1. Only marginally useful.
+///* Points to a private header annotated with `private` (somewhat commmon):
+///  Not incrementally useful as we support private.
+///* Points to a private header without pragmas (rare). This is a reversed
+///  private pragma, and is valuable but too rare to be worthwhile.
+/// - no_include: this is about as common as private, but only affects the
+///   current file, so the value is smaller. We could add support.
+/// - friend: this is less common than private, has implementation difficulties,
+///   and affects behavior in a limited scope.
+/// - associated: extremely rare
 std::unique_ptr
 collectIWYUHeaderMaps(CanonicalIncludes *Includes);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 95262ee - [clangd] Describe non-handling of most IWYU pragmas. NFC

2020-10-05 Thread Sam McCall via cfe-commits


Author: Sam McCall
Date: 2020-10-05T20:50:26+02:00
New Revision: 95262ee2be75daffd05e9a8a985ca2c8e34c9af7

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

LOG: [clangd] Describe non-handling of most IWYU pragmas. NFC

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

Added: 


Modified: 
clang-tools-extra/clangd/index/CanonicalIncludes.h

Removed: 




diff  --git a/clang-tools-extra/clangd/index/CanonicalIncludes.h 
b/clang-tools-extra/clangd/index/CanonicalIncludes.h
index aabfabc75368..da7dc46db778 100644
--- a/clang-tools-extra/clangd/index/CanonicalIncludes.h
+++ b/clang-tools-extra/clangd/index/CanonicalIncludes.h
@@ -71,6 +71,21 @@ class CanonicalIncludes {
 ///
 /// Currently it only supports IWYU private pragma:
 /// 
https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/IWYUPragmas.md#iwyu-pragma-private
+///
+/// We ignore other pragmas:
+/// - keep: this is common but irrelevant: we do not currently remove includes
+/// - export: this is common and potentially interesting, there are three 
cases:
+///* Points to a public header (common): we can suppress include2 if you
+///  already have include1. Only marginally useful.
+///* Points to a private header annotated with `private` (somewhat 
commmon):
+///  Not incrementally useful as we support private.
+///* Points to a private header without pragmas (rare). This is a reversed
+///  private pragma, and is valuable but too rare to be worthwhile.
+/// - no_include: this is about as common as private, but only affects the
+///   current file, so the value is smaller. We could add support.
+/// - friend: this is less common than private, has implementation 
diff iculties,
+///   and affects behavior in a limited scope.
+/// - associated: extremely rare
 std::unique_ptr
 collectIWYUHeaderMaps(CanonicalIncludes *Includes);
 



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

[PATCH] D88786: [CUDA] Don't call __cudaRegisterVariable on C++17 inline variables

2020-10-05 Thread Artem Belevich via Phabricator via cfe-commits

tra added a comment.

In D88786#2312365 , @MaskRay wrote:

>> Could you provide an example where this is causing an issue?
>
> If the C++17 inline variable appears in two TUs. They have the same comdat 
> group. The first comdat group is prevailing and the second one is disarded. 
> `__cudaRegisterVar(...)` in the second TU references a local symbol in a 
> discarded section.

So, if I understand you correctly, it's the *host* side which ends up dropping 
it in one of TUs. It is a bit of a problem, considering that both of those TUs 
will need their own register call for their own GPU-side counterpart of the 
variable.

  a.h:
__device__ inline int foo;
  a1.cu: #inlcude "a.h"
a1.o/host : inline int foo; // 'shadow' variable. 
register(foo, gpu-side-foo) // tell runtime that when we use 
host-side foo we want to access device-side foo.
a1/GPU: int foo; // the only device-side instance. It's always there.
  a2.cu: #inlcude "a.h"
a2.o/host : inline int foo; // 'shadow' variable. 
register(foo, gpu-side-foo) // tell runtime that when we use 
host-side foo we want to access device-side foo.
a2/GPU: int foo; // the only device-side instance. It's always there.

  host_exe: a1.o, a2.o
only one instance of inline int foo survives and we lose ability to tell 
which GPU-side `foo` we want to access when we use host-side foo shadow.

Not allowing inline/constexpr variables seems to be the only choice here. 
Otherwise we's have to keep multiple instances of the shadow and that would 
break the C++ semantics for `inline` and `constexpr`

> The previous revision (https://reviews.llvm.org/D88786?id=295997 ) drops the 
> comdat, but I think it is inferior to this one.

Silently dropping variable registration shifts the problem from link time to 
runtime. It may be OK as a temporary workaround for the build issues and only 
fraction of those will run into it at runtime, so it's technically an 
improvement, but we will need to catch it in Sema ASAP.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88786

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

[PATCH] D88789: [InstCombine] Revert rL226781 "Teach InstCombine to canonicalize loads which are only ever stored to always use a legal integer type if one is available." (PR47592)

2020-10-05 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added a comment.

> Keeping loads and stores of pointers as pointers to the extent possible 
> doesn't seem like a bad idea, but I'm worried people will feel like this 
> gives a *semantic* guarantee that isn't really there. Fundamentally, LLVM 
> still doesn't currently have typed memory. All of the optimizer is built upon 
> this assumption.

LLVM currently isn't internally consistent.  See 
https://bugs.llvm.org/show_bug.cgi?id=34548 .  I should probably make a LangRef 
patch so the "pointer aliasing" section indicates there's an issue here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88789

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

[PATCH] D88789: [InstCombine] Revert rL226781 "Teach InstCombine to canonicalize loads which are only ever stored to always use a legal integer type if one is available." (PR47592)

2020-10-05 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

So, we can't really teach SCEV about this: D88788 
 (not without the 
https://bugs.llvm.org/show_bug.cgi?id=47592 at lease)
And we can't recover the situation post-inlining in instcombine: D88842 
.

It really does look like this fold is actively breaking
otherwise-good IR, in a way that is not recoverable.
And that means, this fold isn't helpful in exposing the passes
that are otherwise unaware of these patterns it produces.

I'm proceeding with this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88789

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

[PATCH] D88789: [InstCombine] Revert rL226781 "Teach InstCombine to canonicalize loads which are only ever stored to always use a legal integer type if one is available." (PR47592)

2020-10-05 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

In D88789#2312510 , @lebedev.ri wrote:

> So, we can't really teach SCEV about this: D88788 
>  (not without the 
> https://bugs.llvm.org/show_bug.cgi?id=47592 at least)
> And we can't recover the situation post-inlining in instcombine: D88842 
> .
>
> It really does look like this fold is actively breaking
> otherwise-good IR, in a way that is not recoverable.
> And that means, this fold isn't helpful in exposing the passes
> that are otherwise unaware of these patterns it produces.
>
> I'm proceeding with this patch.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88789

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

[PATCH] D87029: [AIX] Implement AIX special bitfield related alignment rules

2020-10-05 Thread Sean Fertile via Phabricator via cfe-commits

sfertile added inline comments.



Comment at: clang/test/Layout/aix-bitfield-alignment.cpp:118
+
+typedef __attribute__((aligned(32))) short mySHORT;
+struct D {

We should have a similar test for an overaligned long or long long.


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

https://reviews.llvm.org/D87029

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

[PATCH] D88844: [clangd] Add `score` extension to workspace/symbol response.

2020-10-05 Thread Sam McCall via Phabricator via cfe-commits

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

The protocol doesn't really incorporate ranking.
As with code completion, most clients respect what the server sends, but
VSCode re-ranks items, with predictable results.
See https://github.com/clangd/vscode-clangd/issues/81

There's no filterText field so we may be unable to construct a good workaround.
But expose the score so we may be able to do this on the client in future.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88844

Files:
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/test/symbols.test

Index: clang-tools-extra/clangd/test/symbols.test
===
--- clang-tools-extra/clangd/test/symbols.test
+++ clang-tools-extra/clangd/test/symbols.test
@@ -23,7 +23,8 @@
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "uri": "file://{{.*}}/vector.h"
 # CHECK-NEXT:},
-# CHECK-NEXT:"name": "vector"
+# CHECK-NEXT:"name": "vector",
+# CHECK-NEXT:"score": {{.*}}
 # CHECK-NEXT:  }
 # CHECK-NEXT:]
 # CHECK-NEXT:}
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -1015,6 +1015,14 @@
 
   /// The name of the symbol containing this symbol.
   std::string containerName;
+
+  /// The score that clangd calculates to rank the returned symbols.
+  /// This excludes the fuzzy-matching score between `name` and the query.
+  /// (Specifically, the last ::-separated component).
+  /// This can be used to re-rank results as the user types, using client-side
+  /// fuzzy-matching (that score should be multiplied with this one).
+  /// This is a clangd extension, set only for workspace/symbol responses.
+  llvm::Optional score;
 };
 llvm::json::Value toJSON(const SymbolInformation &);
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const SymbolInformation &);
@@ -1175,11 +1183,11 @@
   /// Indicates if this item is deprecated.
   bool deprecated = false;
 
-  /// This is Clangd extension.
-  /// The score that Clangd calculates to rank completion items. This score can
-  /// be used to adjust the ranking on the client side.
-  /// NOTE: This excludes fuzzy matching score which is typically calculated on
-  /// the client side.
+  /// The score that clangd calculates to rank the returned completions.
+  /// This excludes the fuzzy-match between `filterText` and the partial word.
+  /// This can be used to re-rank results as the user types, using client-side
+  /// fuzzy-matching (that score should be multiplied with this one).
+  /// This is a clangd extension.
   float score = 0.f;
 
   // TODO: Add custom commitCharacters for some of the completion items. For
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -662,12 +662,15 @@
 }
 
 llvm::json::Value toJSON(const SymbolInformation &P) {
-  return llvm::json::Object{
+  llvm::json::Object O{
   {"name", P.name},
   {"kind", static_cast(P.kind)},
   {"location", P.location},
   {"containerName", P.containerName},
   };
+  if (P.score)
+O["score"] = *P.score;
+  return std::move(O);
 }
 
 llvm::raw_ostream &operator<<(llvm::raw_ostream &O,
Index: clang-tools-extra/clangd/FindSymbols.cpp
===
--- clang-tools-extra/clangd/FindSymbols.cpp
+++ clang-tools-extra/clangd/FindSymbols.cpp
@@ -96,12 +96,13 @@
   return;
 }
 
-SymbolKind SK = indexSymbolKindToSymbolKind(Sym.SymInfo.Kind);
-std::string Scope = std::string(Sym.Scope);
-llvm::StringRef ScopeRef = Scope;
-ScopeRef.consume_back("::");
-SymbolInformation Info = {(Sym.Name + Sym.TemplateSpecializationArgs).str(),
-  SK, *Loc, std::string(ScopeRef)};
+llvm::StringRef Scope = Sym.Scope;
+Scope.consume_back("::");
+SymbolInformation Info;
+Info.name = (Sym.Name + Sym.TemplateSpecializationArgs).str();
+Info.kind = indexSymbolKindToSymbolKind(Sym.SymInfo.Kind);
+Info.location = *Loc;
+Info.containerName = Scope.str();
 
 SymbolQualitySignals Quality;
 Quality.merge(Sym);
@@ -121,6 +122,7 @@
 dlog("FindSymbols: {0}{1} = {2}\n{3}{4}\n", Sym.Scope, Sym.Name, Score,
  Quality, Relevance);
 
+Info.score = Score / Relevance.NameMatch;
 Top.push({Score, std::move(Info)});
   });
   for (auto &R : std::move(Top).items())
___
cfe-commits mailing

1 2 >

1 - 100 of 139 matches

Mail list logo