[PATCH] D70535: [clangd] Define out-of-line qualify return value

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Move TargetContext generation logic to a separate function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70535

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.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
@@ -1775,6 +1775,56 @@
   }
 }
 
+TEST_F(DefineOutlineTest, QualifyReturnValue) {
+  FileName = "Test.hpp";
+  ExtraFiles["Test.cpp"] = "";
+
+  struct {
+llvm::StringRef Test;
+llvm::StringRef ExpectedHeader;
+llvm::StringRef ExpectedSource;
+  } Cases[] = {
+  {R"cpp(
+namespace a { class Foo; }
+using namespace a;
+Foo fo^o() { return; })cpp",
+   R"cpp(
+namespace a { class Foo; }
+using namespace a;
+Foo foo() ;)cpp",
+   "a::Foo foo() { return; }"},
+  {R"cpp(
+namespace a {
+  class Foo {
+class Bar {};
+Bar fo^o() { return {}; }
+  }
+})cpp",
+   R"cpp(
+namespace a {
+  class Foo {
+class Bar {};
+Bar foo() ;
+  }
+})cpp",
+   "a::Foo::Bar foo() { return {}; }\n"},
+  {R"cpp(
+class Foo;
+Foo fo^o() { return; })cpp",
+   R"cpp(
+class Foo;
+Foo foo() ;)cpp",
+   "Foo foo() { return; }"},
+  };
+  llvm::StringMap EditedFiles;
+  for (auto &Case : Cases) {
+apply(Case.Test, &EditedFiles);
+EXPECT_EQ(apply(Case.Test, &EditedFiles), Case.ExpectedHeader);
+EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+ testPath("Test.cpp"), Case.ExpectedSource)));
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -6,13 +6,17 @@
 //
 //===--===//
 
+#include "AST.h"
+#include "FindTarget.h"
 #include "HeaderSourceSwitch.h"
+#include "Logger.h"
 #include "Path.h"
 #include "Selection.h"
 #include "SourceCode.h"
 #include "refactor/Tweak.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Stmt.h"
 #include "clang/Basic/SourceLocation.h"
@@ -20,10 +24,13 @@
 #include "clang/Driver/Types.h"
 #include "clang/Format/Format.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/Error.h"
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -68,29 +75,125 @@
   return getCorrespondingHeaderOrSource(FileName, Sel.AST, Sel.Index);
 }
 
+// Synthesize a DeclContext for TargetNS from CurContext.
+llvm::Optional
+synthesizeContextForNS(llvm::StringRef TargetNS,
+   const DeclContext *CurContext) {
+  assert(TargetNS.empty() || TargetNS.endswith("::"));
+  // Skip any non-namespace contexts, e.g. TagDecls, functions/methods.
+  CurContext = CurContext->getEnclosingNamespaceContext();
+  if (TargetNS.empty()) {
+// If TargetNS is empty, it means global ns, which is translation unit.
+while (!CurContext->isTranslationUnit())
+  CurContext = CurContext->getParent();
+  } else {
+// Otherwise we need to drop any trailing namespaces from CurContext until
+// we reach TargetNS.
+std::string TargetContextNS =
+CurContext->isNamespace()
+? llvm::cast(CurContext)->getQualifiedNameAsString()
+: "";
+TargetContextNS.append("::");
+
+llvm::StringRef CurrentContextNS(TargetContextNS);
+// If TargetNS is not a prefix of CurrentContext, there's no way to reach
+// it.
+if (!CurrentContextNS.startswith(TargetNS))
+  return llvm::None;
+
+while (CurrentContextNS != TargetNS) {
+  CurContext = CurContext->getParent();
+  // These colons always exists since TargetNS is a prefix of
+  // CurrentContextNS, it ends with "::" and they are not equal.
+  CurrentContextNS = CurrentContextNS.take_front(
+  CurrentContextNS.drop_back(2).rfind("::") + 2);
+}
+  }
+  return CurContext;
+}
+
 // Creates a modified version of function definition that can be inserted at a
 // different location. Contains both function signature and body.
-llvm::Optional getFunctionSourceCode(const FunctionDecl *FD) {
+llvm::Expected

[PATCH] D70633: clang-format-vs : Fix Unicode formatting

[Hans Wennborg via Phabricator via cfe-commits]

hans added a reviewer: klimek.
hans added a comment.

klimek, What do you think?

empty2fill: Do you have an example input that I could use to hit the "Specified 
argument was out of the range of valid values." error?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70633



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

[PATCH] D70535: [clangd] Define out-of-line qualify return value

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: FAILURE - Could not check out parent git hash 
"627221d6588eb650759f39e80858abb2a3848ccb". It was not found in the repository. 
Did you configure the "Parent Revision" in Phabricator properly? Trying to 
apply the patch to the master branch instead...

ERROR: arc patch failed with error code 1. Check build log for details.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70535



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

[PATCH] D70656: [clangd] Define out-of-line qualify function name

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Get rid of debug output


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70656

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.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
@@ -1807,7 +1807,7 @@
 Bar foo() ;
   }
 })cpp",
-   "a::Foo::Bar foo() { return {}; }\n"},
+   "a::Foo::Bar a::Foo::foo() { return {}; }\n"},
   {R"cpp(
 class Foo;
 Foo fo^o() { return; })cpp",
@@ -1825,6 +1825,50 @@
   }
 }
 
+TEST_F(DefineOutlineTest, QualifyFunctionName) {
+  FileName = "Test.hpp";
+  struct {
+llvm::StringRef Test;
+llvm::StringRef SourceFile;
+llvm::StringRef ExpectedHeader;
+llvm::StringRef ExpectedSource;
+  } Cases[] = {
+  {R"cpp(
+namespace a {
+  namespace b {
+class Foo {
+  void fo^o() {}
+};
+  }
+})cpp",
+   "",
+   R"cpp(
+namespace a {
+  namespace b {
+class Foo {
+  void foo() ;
+};
+  }
+})cpp",
+   "void a::b::Foo::foo() {}\n"},
+  {"namespace a { namespace b { void f^oo() {} } }", "namespace a{}",
+   "namespace a { namespace b { void foo() ; } }",
+   "namespace a{void b::foo() {} }"},
+  {"namespace a { namespace b { void f^oo() {} } }", "using namespace a;",
+   "namespace a { namespace b { void foo() ; } }",
+   // FIXME: Take using namespace directives in the source file into
+   // account. This can be spelled as b::foo instead.
+   "using namespace a;void a::b::foo() {} "},
+  };
+  llvm::StringMap EditedFiles;
+  for (auto &Case : Cases) {
+ExtraFiles["Test.cpp"] = Case.SourceFile;
+EXPECT_EQ(apply(Case.Test, &EditedFiles), Case.ExpectedHeader);
+EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+ testPath("Test.cpp"), Case.ExpectedSource)))
+<< Case.Test;
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -132,8 +132,9 @@
 // inside a macro body.
 if (Ref.Qualifier || Ref.Targets.empty() || Ref.NameLoc.isMacroID())
   return;
-// Qualify return type
-if (Ref.NameLoc != FD->getReturnTypeSourceRange().getBegin())
+// Only qualify return type and function name.
+if (Ref.NameLoc != FD->getReturnTypeSourceRange().getBegin() &&
+Ref.NameLoc != FD->getLocation())
   return;
 
 for (const NamedDecl *ND : Ref.Targets) {
@@ -178,8 +179,6 @@
   if (!QualifiedFunc)
 return QualifiedFunc.takeError();
   return QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1);
-
-  // FIXME: Qualify function name depending on the target context.
 }
 
 struct InsertionPoint {
@@ -285,9 +284,7 @@
 auto FuncDef =
 getFunctionSourceCode(Source, InsertionPoint->EnclosingNamespace);
 if (!FuncDef)
-  return llvm::createStringError(
-  llvm::inconvertibleErrorCode(),
-  "Couldn't get full source for function definition.");
+  return FuncDef.takeError();
 
 SourceManagerForFile SMFF(*CCFile, Contents);
 const tooling::Replacement InsertFunctionDef(


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1807,7 +1807,7 @@
 Bar foo() ;
   }
 })cpp",
-   "a::Foo::Bar foo() { return {}; }\n"},
+   "a::Foo::Bar a::Foo::foo() { return {}; }\n"},
   {R"cpp(
 class Foo;
 Foo fo^o() { return; })cpp",
@@ -1825,6 +1825,50 @@
   }
 }
 
+TEST_F(DefineOutlineTest, QualifyFunctionName) {
+  FileName = "Test.hpp";
+  struct {
+llvm::StringRef Test;
+llvm::StringRef SourceFile;
+llvm::StringRef ExpectedHeader;
+llvm::StringRef ExpectedSource;
+  } Cases[] = {
+  {R"cpp(
+namespace a {
+  namespace b {
+class Foo {
+  void fo^o() {}
+};
+  }
+})cpp",
+   "",
+   R"cpp(
+namespace a {
+  namespace b {
+class Foo {
+  void foo() ;
+};
+  }
+})cpp",
+   "void a::b::Foo::foo() {}\n"},
+  {"namespace a { namespace b { void f^oo() {} } }"

[PATCH] D70656: [clangd] Define out-of-line qualify function name

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: hokein.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.
kadircet updated this revision to Diff 230843.
kadircet added a comment.

- Get rid of debug output


When moving function definitions to a different context, the function
name might need a different spelling, for example in the header it might be:

  namespace a {
void foo() {}
  }

And we might want to move it into a context which doesn't have `namespace a` as
a parent, then we must re-spell the function name, i.e:

  void a::foo() {}

This patch implements a version of this which ignores using namespace
declarations in the source file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70656

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.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
@@ -1807,7 +1807,7 @@
 Bar foo() ;
   }
 })cpp",
-   "a::Foo::Bar foo() { return {}; }\n"},
+   "a::Foo::Bar a::Foo::foo() { return {}; }\n"},
   {R"cpp(
 class Foo;
 Foo fo^o() { return; })cpp",
@@ -1825,6 +1825,50 @@
   }
 }
 
+TEST_F(DefineOutlineTest, QualifyFunctionName) {
+  FileName = "Test.hpp";
+  struct {
+llvm::StringRef Test;
+llvm::StringRef SourceFile;
+llvm::StringRef ExpectedHeader;
+llvm::StringRef ExpectedSource;
+  } Cases[] = {
+  {R"cpp(
+namespace a {
+  namespace b {
+class Foo {
+  void fo^o() {}
+};
+  }
+})cpp",
+   "",
+   R"cpp(
+namespace a {
+  namespace b {
+class Foo {
+  void foo() ;
+};
+  }
+})cpp",
+   "void a::b::Foo::foo() {}\n"},
+  {"namespace a { namespace b { void f^oo() {} } }", "namespace a{}",
+   "namespace a { namespace b { void foo() ; } }",
+   "namespace a{void b::foo() {} }"},
+  {"namespace a { namespace b { void f^oo() {} } }", "using namespace a;",
+   "namespace a { namespace b { void foo() ; } }",
+   // FIXME: Take using namespace directives in the source file into
+   // account. This can be spelled as b::foo instead.
+   "using namespace a;void a::b::foo() {} "},
+  };
+  llvm::StringMap EditedFiles;
+  for (auto &Case : Cases) {
+ExtraFiles["Test.cpp"] = Case.SourceFile;
+EXPECT_EQ(apply(Case.Test, &EditedFiles), Case.ExpectedHeader);
+EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+ testPath("Test.cpp"), Case.ExpectedSource)))
+<< Case.Test;
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -132,8 +132,9 @@
 // inside a macro body.
 if (Ref.Qualifier || Ref.Targets.empty() || Ref.NameLoc.isMacroID())
   return;
-// Qualify return type
-if (Ref.NameLoc != FD->getReturnTypeSourceRange().getBegin())
+// Only qualify return type and function name.
+if (Ref.NameLoc != FD->getReturnTypeSourceRange().getBegin() &&
+Ref.NameLoc != FD->getLocation())
   return;
 
 for (const NamedDecl *ND : Ref.Targets) {
@@ -178,8 +179,6 @@
   if (!QualifiedFunc)
 return QualifiedFunc.takeError();
   return QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1);
-
-  // FIXME: Qualify function name depending on the target context.
 }
 
 struct InsertionPoint {
@@ -285,9 +284,7 @@
 auto FuncDef =
 getFunctionSourceCode(Source, InsertionPoint->EnclosingNamespace);
 if (!FuncDef)
-  return llvm::createStringError(
-  llvm::inconvertibleErrorCode(),
-  "Couldn't get full source for function definition.");
+  return FuncDef.takeError();
 
 SourceManagerForFile SMFF(*CCFile, Contents);
 const tooling::Replacement InsertFunctionDef(


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1807,7 +1807,7 @@
 Bar foo() ;
   }
 })cpp",
-   "a::Foo::Bar foo() { return {}; }\n"},
+   "a::Foo::Bar a::Foo::foo() { return {}; }\n"},
   {R"cpp(
 class Foo;
 Foo fo^o() { return; })cpp",
@@ -1825,6 +1825,50 @@
   }
 }
 
+TEST_F(DefineOutlineTest, QualifyFunctionName) {
+  FileName = "Test.hpp";
+  struct {
+llvm

[PATCH] D70656: [clangd] Define out-of-line qualify function name

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: FAILURE - Could not check out parent git hash 
"0a2c8c14d3aa16ca964a8e643f9a5bfdd5459de3". It was not found in the repository. 
Did you configure the "Parent Revision" in Phabricator properly? Trying to 
apply the patch to the master branch instead...

ERROR: arc patch failed with error code 1. Check build log for details.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70656



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

[PATCH] D70656: [clangd] Define out-of-line qualify function name

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: FAILURE - Could not check out parent git hash 
"0a2c8c14d3aa16ca964a8e643f9a5bfdd5459de3". It was not found in the repository. 
Did you configure the "Parent Revision" in Phabricator properly? Trying to 
apply the patch to the master branch instead...

ERROR: arc patch failed with error code 1. Check build log for details.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70656



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

[PATCH] D70494: [clangd] Fix diagnostic location for macro expansions

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:961
 
 TEST(IgnoreDiags, FromNonWrittenInclude) {
   TestTU TU = TestTU::withCode("");

I'd move this to the end of the file, so that all `DiagsInHeaders` are grouped 
together.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70494



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

[PATCH] D70633: clang-format-vs : Fix Unicode formatting

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

generally makes sense


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70633



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

[clang-tools-extra] 97d6e8e - [clangd] Helper for getting nested namespace qualification

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2019-11-25T10:42:13+01:00
New Revision: 97d6e8e0f3748e6fe2b27983803e2df75874507e

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

LOG: [clangd] Helper for getting nested namespace qualification

Summary:
Introduce a new helper for getting minimally required qualifiers
necessary to spell a name at a point in a given DeclContext. Currently takes
using directives and nested namespecifier of DeclContext itself into account.

Initially will be used in define inline and outline actions.

Reviewers: ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/AST.cpp
clang-tools-extra/clangd/AST.h
clang-tools-extra/clangd/unittests/ASTTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/AST.cpp 
b/clang-tools-extra/clangd/AST.cpp
index 6c69367633bf..cd6a682007f2 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -11,6 +11,7 @@
 #include "SourceCode.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/NestedNameSpecifier.h"
@@ -22,10 +23,15 @@
 #include "clang/Basic/Specifiers.h"
 #include "clang/Index/USRGeneration.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -67,6 +73,74 @@ bool isTemplateSpecializationKind(const NamedDecl *D,
  isTemplateSpecializationKind(D, Kind);
 }
 
+// Store all UsingDirectiveDecls in parent contexts of DestContext, that were
+// introduced before InsertionPoint.
+llvm::DenseSet
+getUsingNamespaceDirectives(const DeclContext *DestContext,
+SourceLocation Until) {
+  const auto &SM = DestContext->getParentASTContext().getSourceManager();
+  llvm::DenseSet VisibleNamespaceDecls;
+  for (const auto *DC = DestContext; DC; DC = DC->getLookupParent()) {
+for (const auto *D : DC->decls()) {
+  if (!SM.isWrittenInSameFile(D->getLocation(), Until) ||
+  !SM.isBeforeInTranslationUnit(D->getLocation(), Until))
+continue;
+  if (auto *UDD = llvm::dyn_cast(D))
+VisibleNamespaceDecls.insert(
+UDD->getNominatedNamespace()->getCanonicalDecl());
+}
+  }
+  return VisibleNamespaceDecls;
+}
+
+// Goes over all parents of SourceContext until we find a comman ancestor for
+// DestContext and SourceContext. Any qualifier including and above common
+// ancestor is redundant, therefore we stop at lowest common ancestor.
+// In addition to that stops early whenever IsVisible returns true. This can be
+// used to implement support for "using namespace" decls.
+std::string
+getQualification(ASTContext &Context, const DeclContext *DestContext,
+ const DeclContext *SourceContext,
+ llvm::function_ref IsVisible) {
+  std::vector Parents;
+  bool ReachedNS = false;
+  for (const DeclContext *CurContext = SourceContext; CurContext;
+   CurContext = CurContext->getLookupParent()) {
+// Stop once we reach a common ancestor.
+if (CurContext->Encloses(DestContext))
+  break;
+
+NestedNameSpecifier *NNS = nullptr;
+if (auto *TD = llvm::dyn_cast(CurContext)) {
+  // There can't be any more tag parents after hitting a namespace.
+  assert(!ReachedNS);
+  NNS = NestedNameSpecifier::Create(Context, nullptr, false,
+TD->getTypeForDecl());
+} else {
+  ReachedNS = true;
+  auto *NSD = llvm::cast(CurContext);
+  NNS = NestedNameSpecifier::Create(Context, nullptr, NSD);
+  // Anonymous and inline namespace names are not spelled while qualifying 
a
+  // name, so skip those.
+  if (NSD->isAnonymousNamespace() || NSD->isInlineNamespace())
+continue;
+}
+// Stop if this namespace is already visible at DestContext.
+if (IsVisible(NNS))
+  break;
+
+Parents.push_back(NNS);
+  }
+
+  // Go over name-specifiers in reverse order to create necessary 
qualification,
+  // since we stored inner-most parent first.
+  std::string Result;
+  llvm::raw_string_ostream OS(Result);
+  for (const auto *Parent : llvm::reverse(Parents))
+Parent->print(OS, Context.getPrintingPolicy());
+  return OS.str();
+}
+
 } // namespace
 
 bool isImplicitTemplateInstantiation(const NamedDecl *D) {
@@ -82,9 +156,7 @@ bool isImplement

[clang-tools-extra] 5075c68 - [clangd] Improve symbol qualification in DefineInline code action

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2019-11-25T10:42:13+01:00
New Revision: 5075c68219826a199c67f7450c7cf60a55a71c0b

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

LOG: [clangd] Improve symbol qualification in DefineInline code action

Summary:
Currently define inline action fully qualifies any names in the
function body, which is not optimal and definitely natural.

This patch tries to improve the situation by dropping any name
specifiers shared by function and names spelled in the body. For example
if you are moving definition of a function in namespace clang::clangd,
and body has any decl's coming from clang or clang::clangd namespace,
those qualifications won't be added since they are redundant.

It also drops any qualifiers that are visible in target context.

Reviewers: ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/AST.h
clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h
index bea67a41005a..bc38f19c6553 100644
--- a/clang-tools-extra/clangd/AST.h
+++ b/clang-tools-extra/clangd/AST.h
@@ -137,6 +137,8 @@ llvm::Optional getDeducedType(ASTContext &, 
SourceLocation Loc);
 /// InsertionPoint. i.e, if you have `using namespace
 /// clang::clangd::bar`, this function will return an empty string for the
 /// example above since no qualification is necessary in that case.
+/// FIXME: Also take using directives and namespace aliases inside function 
body
+/// into account.
 std::string getQualification(ASTContext &Context,
  const DeclContext *DestContext,
  SourceLocation InsertionPoint,

diff  --git a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp 
b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
index 6d0599e8821c..3bc5df0edbfd 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -135,8 +135,10 @@ bool checkDeclsAreVisible(const llvm::DenseSet &DeclRefs,
   return true;
 }
 
-// Rewrites body of FD to fully-qualify all of the decls inside.
-llvm::Expected qualifyAllDecls(const FunctionDecl *FD) {
+// Rewrites body of FD by re-spelling all of the names to make sure they are
+// still valid in context of Target.
+llvm::Expected qualifyAllDecls(const FunctionDecl *FD,
+const FunctionDecl *Target) {
   // There are three types of spellings that needs to be qualified in a 
function
   // body:
   // - Types:   Foo -> ns::Foo
@@ -147,16 +149,16 @@ llvm::Expected qualifyAllDecls(const 
FunctionDecl *FD) {
   //using ns3 = ns2 -> using ns3 = ns1::ns2
   //
   // Go over all references inside a function body to generate replacements 
that
-  // will fully qualify those. So that body can be moved into an arbitrary 
file.
+  // will qualify those. So that body can be moved into an arbitrary file.
   // We perform the qualification by qualyfying the first type/decl in a
   // (un)qualified name. e.g:
   //namespace a { namespace b { class Bar{}; void foo(); } }
   //b::Bar x; -> a::b::Bar x;
   //foo(); -> a::b::foo();
-  // FIXME: Instead of fully qualyfying we should try deducing visible scopes 
at
-  // target location and generate minimal edits.
 
+  auto *TargetContext = Target->getLexicalDeclContext();
   const SourceManager &SM = FD->getASTContext().getSourceManager();
+
   tooling::Replacements Replacements;
   bool HadErrors = false;
   findExplicitReferences(FD->getBody(), [&](ReferenceLoc Ref) {
@@ -193,7 +195,8 @@ llvm::Expected qualifyAllDecls(const 
FunctionDecl *FD) {
 if (!ND->getDeclContext()->isNamespace())
   return;
 
-std::string Qualifier = printNamespaceScope(*ND->getDeclContext());
+const std::string Qualifier = getQualification(
+FD->getASTContext(), TargetContext, Target->getBeginLoc(), ND);
 if (auto Err = Replacements.add(
 tooling::Replacement(SM, Ref.NameLoc, 0, Qualifier))) {
   HadErrors = true;
@@ -437,7 +440,7 @@ class DefineInline : public Tweak {
 if (!ParamReplacements)
   return ParamReplacements.takeError();
 
-auto QualifiedBody = qualifyAllDecls(Source);
+auto QualifiedBody = qualifyAllDecls(Source, Target);
 if (!QualifiedBody)
   return QualifiedBody.takeError();
 

diff  --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp 
b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index ab2808835832..4e481241acd8 100644
--- a/clang-tools-ex

[PATCH] D68261: [clangd] Add "inline" keyword to prevent ODR-violations in DefineInline

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68261

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.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
@@ -1809,6 +1809,66 @@
   EXPECT_EQ(apply(Test), Expected) << Test;
 }
 
+TEST_F(DefineInlineTest, AddInline) {
+  llvm::StringMap EditedFiles;
+  ExtraFiles["a.h"] = "void foo();";
+  apply(R"cpp(#include "a.h"
+  void fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+   testPath("a.h"), "inline void foo(){}")));
+
+  // Check we put inline before cv-qualifiers.
+  ExtraFiles["a.h"] = "const int foo();";
+  apply(R"cpp(#include "a.h"
+  const int fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+   testPath("a.h"), "inline const int foo(){}")));
+
+  // No double inline.
+  ExtraFiles["a.h"] = "inline void foo();";
+  apply(R"cpp(#include "a.h"
+  inline void fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+   testPath("a.h"), "inline void foo(){}")));
+
+  // Constexprs don't need "inline".
+  ExtraFiles["a.h"] = "constexpr void foo();";
+  apply(R"cpp(#include "a.h"
+  constexpr void fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+   testPath("a.h"), "constexpr void foo(){}")));
+
+  // Class members don't need "inline".
+  ExtraFiles["a.h"] = "struct Foo { void foo(); }";
+  apply(R"cpp(#include "a.h"
+  void Foo::fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles,
+  testing::ElementsAre(FileWithContents(
+  testPath("a.h"), "struct Foo { void foo(){} }")));
+
+  // Function template doesn't need to be "inline"d.
+  ExtraFiles["a.h"] = "template  void foo();";
+  apply(R"cpp(#include "a.h"
+  template 
+  void fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles,
+  testing::ElementsAre(FileWithContents(
+  testPath("a.h"), "template  void foo(){}")));
+
+  // Specializations needs to be marked "inline".
+  ExtraFiles["a.h"] = R"cpp(
+template  void foo();
+template <> void foo();)cpp";
+  apply(R"cpp(#include "a.h"
+  template <>
+  void fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles,
+  testing::ElementsAre(FileWithContents(testPath("a.h"),
+R"cpp(
+template  void foo();
+template <> inline void foo(){})cpp")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/TweakTesting.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTesting.cpp
+++ clang-tools-extra/clangd/unittests/TweakTesting.cpp
@@ -124,7 +124,7 @@
 ADD_FAILURE() << "There were changes to additional files, but client "
  "provided a nullptr for EditedFiles.";
   else
-EditedFiles->try_emplace(It.first(), Unwrapped.str());
+EditedFiles->insert_or_assign(It.first(), Unwrapped.str());
 }
   }
   return EditedMainFile;
Index: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -32,6 +32,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TokenKinds.h"
+#include "clang/Driver/Types.h"
 #include "clang/Index/IndexDataConsumer.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Index/IndexingAction.h"
@@ -360,6 +361,30 @@
   return FD->getBeginLoc();
 }
 
+llvm::Optional
+addInlineIfInHeader(const FunctionDecl *FD) {
+  // This includes inline functions and constexpr functions.
+  if (FD->isInlined() || llvm::isa(FD))
+return llvm::None;
+  // Primary template doesn't need inline.
+  if (FD->isTemplated() && !FD->isFunctionTemplateSpecialization())
+return llvm::None;
+
+  const SourceManager &SM = FD->getASTContext().getSourceManager();
+  llvm::StringRef FileName = SM.getFilename(FD->getLocation());
+
+  auto Type = drive

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

[fiesh via Phabricator via cfe-commits]

fiesh added a comment.

In D31130#1711841 , @lebedev.ri wrote:

> The description only says what the patch does, not why this is the desired 
> behavior.
>  Also, this is now under a wrong license.


What are the options in case the original author has abandonded this?  Can 
anybody re-create the MR under a new license with the commit message fixed, or 
is that an insurmountable license issue?


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] D70494: [clangd] Fix diagnostic location for macro expansions

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 230849.
kadircet marked an inline comment as done.
kadircet added a comment.

- Group similar tests together


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70494

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


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -941,7 +941,7 @@
   WithNote(Diag(Header.range(), "error occurred here");
 }
 
-TEST(IgnoreDiags, FromNonWrittenSources) {
+TEST(DiagsInHeaders, FromNonWrittenSources) {
   Annotations Main(R"cpp(
 #include [["a.h"]]
 void foo() {})cpp");
@@ -951,11 +951,49 @@
   TestTU TU = TestTU::withCode(Main.code());
   TU.AdditionalFiles = {{"a.h", Header.code()}};
   TU.ExtraArgs = {"-DFOO=NOOO"};
-  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(AllOf(
+  Diag(Main.range(),
+   "in included file: use of undeclared identifier 
'NOOO'"),
+  WithNote(Diag(Header.range(), "error occurred here");
+}
+
+TEST(DiagsInHeaders, ErrorFromMacroExpansion) {
+  Annotations Main(R"cpp(
+  void bar() {
+int fo;
+#include [["a.h"]]
+  })cpp");
+  Annotations Header(R"cpp(
+  #define X foo
+  X;)cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.AdditionalFiles = {{"a.h", Header.code()}};
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range(), "in included file: use of undeclared "
+ "identifier 'foo'; did you mean 'fo'?")));
+}
+
+TEST(DiagsInHeaders, ErrorFromMacroArgument) {
+  Annotations Main(R"cpp(
+  void bar() {
+int fo;
+#include [["a.h"]]
+  })cpp");
+  Annotations Header(R"cpp(
+  #define X(arg) arg
+  X(foo);)cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.AdditionalFiles = {{"a.h", Header.code()}};
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range(), "in included file: use of undeclared "
+ "identifier 'foo'; did you mean 'fo'?")));
 }
 
 TEST(IgnoreDiags, FromNonWrittenInclude) {
-  TestTU TU = TestTU::withCode("");
+  TestTU TU;
   TU.ExtraArgs.push_back("--include=a.h");
   TU.AdditionalFiles = {{"a.h", "void main();"}};
   // The diagnostic "main must return int" is from the header, we don't attempt
@@ -964,6 +1002,5 @@
 }
 
 } // namespace
-
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -117,8 +117,8 @@
   if (D.Severity < DiagnosticsEngine::Level::Error)
 return false;
 
-  const SourceLocation &DiagLoc = Info.getLocation();
   const SourceManager &SM = Info.getSourceManager();
+  const SourceLocation &DiagLoc = SM.getExpansionLoc(Info.getLocation());
   SourceLocation IncludeInMainFile;
   auto GetIncludeLoc = [&SM](SourceLocation SLoc) {
 return SM.getIncludeLoc(SM.getFileID(SLoc));


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -941,7 +941,7 @@
   WithNote(Diag(Header.range(), "error occurred here");
 }
 
-TEST(IgnoreDiags, FromNonWrittenSources) {
+TEST(DiagsInHeaders, FromNonWrittenSources) {
   Annotations Main(R"cpp(
 #include [["a.h"]]
 void foo() {})cpp");
@@ -951,11 +951,49 @@
   TestTU TU = TestTU::withCode(Main.code());
   TU.AdditionalFiles = {{"a.h", Header.code()}};
   TU.ExtraArgs = {"-DFOO=NOOO"};
-  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(AllOf(
+  Diag(Main.range(),
+   "in included file: use of undeclared identifier 'NOOO'"),
+  WithNote(Diag(Header.range(), "error occurred here");
+}
+
+TEST(DiagsInHeaders, ErrorFromMacroExpansion) {
+  Annotations Main(R"cpp(
+  void bar() {
+int fo;
+#include [["a.h"]]
+  })cpp");
+  Annotations Header(R"cpp(
+  #define X foo
+  X;)cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.AdditionalFiles = {{"a.h", Header.code()}};
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range(), "in included file: use of undeclared "
+

[PATCH] D69608: [clangd] Helper for getting nested namespace qualification

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
kadircet marked an inline comment as done.
Closed by commit rG97d6e8e0f374: [clangd] Helper for getting nested namespace 
qualification (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69608

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/unittests/ASTTests.cpp

Index: clang-tools-extra/clangd/unittests/ASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ASTTests.cpp
@@ -9,9 +9,17 @@
 #include "AST.h"
 
 #include "Annotations.h"
+#include "ParsedAST.h"
 #include "TestTU.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
 #include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/StringRef.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -64,6 +72,108 @@
   }
 }
 
+TEST(ClangdAST, GetQualification) {
+  // Tries to insert the decl `Foo` into position of each decl named `insert`.
+  // This is done to get an appropriate DeclContext for the insertion location.
+  // Qualifications are the required nested name specifier to spell `Foo` at the
+  // `insert`ion location.
+  // VisibleNamespaces are assumed to be visible at every insertion location.
+  const struct {
+llvm::StringRef Test;
+std::vector Qualifications;
+std::vector VisibleNamespaces;
+  } Cases[] = {
+  {
+  R"cpp(
+namespace ns1 { namespace ns2 { class Foo {}; } }
+void insert(); // ns1::ns2::Foo
+namespace ns1 {
+  void insert(); // ns2::Foo
+  namespace ns2 {
+void insert(); // Foo
+  }
+  using namespace ns2;
+  void insert(); // Foo
+}
+using namespace ns1;
+void insert(); // ns2::Foo
+using namespace ns2;
+void insert(); // Foo
+  )cpp",
+  {"ns1::ns2::", "ns2::", "", "", "ns2::", ""},
+  {},
+  },
+  {
+  R"cpp(
+namespace ns1 { namespace ns2 { class Bar { void Foo(); }; } }
+void insert(); // ns1::ns2::Bar::Foo
+namespace ns1 {
+  void insert(); // ns2::Bar::Foo
+  namespace ns2 {
+void insert(); // Bar::Foo
+  }
+  using namespace ns2;
+  void insert(); // Bar::Foo
+}
+using namespace ns1;
+void insert(); // ns2::Bar::Foo
+using namespace ns2;
+void insert(); // Bar::Foo
+  )cpp",
+  {"ns1::ns2::Bar::", "ns2::Bar::", "Bar::", "Bar::", "ns2::Bar::",
+   "Bar::"},
+  {},
+  },
+  {
+  R"cpp(
+namespace ns1 { namespace ns2 { void Foo(); } }
+void insert(); // ns2::Foo
+namespace ns1 {
+  void insert(); // ns2::Foo
+  namespace ns2 {
+void insert(); // Foo
+  }
+}
+  )cpp",
+  {"ns2::", "ns2::", ""},
+  {"ns1::"},
+  },
+  };
+  for (const auto &Case : Cases) {
+Annotations Test(Case.Test);
+TestTU TU = TestTU::withCode(Test.code());
+ParsedAST AST = TU.build();
+std::vector InsertionPoints;
+const NamedDecl *TargetDecl;
+findDecl(AST, [&](const NamedDecl &ND) {
+  if (ND.getNameAsString() == "Foo") {
+TargetDecl = &ND;
+return true;
+  }
+
+  if (ND.getNameAsString() == "insert")
+InsertionPoints.push_back(&ND);
+  return false;
+});
+
+ASSERT_EQ(InsertionPoints.size(), Case.Qualifications.size());
+for (size_t I = 0, E = InsertionPoints.size(); I != E; ++I) {
+  const Decl *D = InsertionPoints[I];
+  if (Case.VisibleNamespaces.empty()) {
+EXPECT_EQ(getQualification(AST.getASTContext(),
+   D->getLexicalDeclContext(), D->getBeginLoc(),
+   TargetDecl),
+  Case.Qualifications[I]);
+  } else {
+EXPECT_EQ(getQualification(AST.getASTContext(),
+   D->getLexicalDeclContext(), D->getBeginLoc(),
+   TargetDecl, Case.VisibleNamespaces),
+  Case.Qualifications[I]);
+  }
+}
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/AST.h
===
--- clang-tools-extra/clangd/AST.h
+++ clang-tools-extra/clangd/AST.h
@@ -15,9 +15,13 @@
 
 #include "index/SymbolID.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/NestedNameSpecifier.h"
 #include "clang/Basic/SourceLo

[clang-tools-extra] e841029 - [clangd] Fix diagnostic location for macro expansions

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2019-11-25T10:45:14+01:00
New Revision: e841029aef74d99d1cb9443edd4a7b761d84ff45

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

LOG: [clangd] Fix diagnostic location for macro expansions

Summary:
Diagnostic locations were broken when it was result of a macro
expansion. This patch fixes it by using expansion location instead of location
inside macro body.

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

Reviewers: hokein

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/Diagnostics.cpp 
b/clang-tools-extra/clangd/Diagnostics.cpp
index c9e1ed6bc687..cd95807162bc 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -117,8 +117,8 @@ bool adjustDiagFromHeader(Diag &D, const clang::Diagnostic 
&Info,
   if (D.Severity < DiagnosticsEngine::Level::Error)
 return false;
 
-  const SourceLocation &DiagLoc = Info.getLocation();
   const SourceManager &SM = Info.getSourceManager();
+  const SourceLocation &DiagLoc = SM.getExpansionLoc(Info.getLocation());
   SourceLocation IncludeInMainFile;
   auto GetIncludeLoc = [&SM](SourceLocation SLoc) {
 return SM.getIncludeLoc(SM.getFileID(SLoc));

diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp 
b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index 374dae073e0b..fe7a8898c5de 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -941,7 +941,7 @@ TEST(DiagsInHeaders, OnlyErrorOrFatal) {
   WithNote(Diag(Header.range(), "error occurred here");
 }
 
-TEST(IgnoreDiags, FromNonWrittenSources) {
+TEST(DiagsInHeaders, FromNonWrittenSources) {
   Annotations Main(R"cpp(
 #include [["a.h"]]
 void foo() {})cpp");
@@ -951,11 +951,49 @@ TEST(IgnoreDiags, FromNonWrittenSources) {
   TestTU TU = TestTU::withCode(Main.code());
   TU.AdditionalFiles = {{"a.h", Header.code()}};
   TU.ExtraArgs = {"-DFOO=NOOO"};
-  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(AllOf(
+  Diag(Main.range(),
+   "in included file: use of undeclared identifier 
'NOOO'"),
+  WithNote(Diag(Header.range(), "error occurred here");
+}
+
+TEST(DiagsInHeaders, ErrorFromMacroExpansion) {
+  Annotations Main(R"cpp(
+  void bar() {
+int fo;
+#include [["a.h"]]
+  })cpp");
+  Annotations Header(R"cpp(
+  #define X foo
+  X;)cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.AdditionalFiles = {{"a.h", Header.code()}};
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range(), "in included file: use of undeclared "
+ "identifier 'foo'; did you mean 'fo'?")));
+}
+
+TEST(DiagsInHeaders, ErrorFromMacroArgument) {
+  Annotations Main(R"cpp(
+  void bar() {
+int fo;
+#include [["a.h"]]
+  })cpp");
+  Annotations Header(R"cpp(
+  #define X(arg) arg
+  X(foo);)cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.AdditionalFiles = {{"a.h", Header.code()}};
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range(), "in included file: use of undeclared "
+ "identifier 'foo'; did you mean 'fo'?")));
 }
 
 TEST(IgnoreDiags, FromNonWrittenInclude) {
-  TestTU TU = TestTU::withCode("");
+  TestTU TU;
   TU.ExtraArgs.push_back("--include=a.h");
   TU.AdditionalFiles = {{"a.h", "void main();"}};
   // The diagnostic "main must return int" is from the header, we don't attempt
@@ -964,6 +1002,5 @@ TEST(IgnoreDiags, FromNonWrittenInclude) {
 }
 
 } // 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] D69033: [clangd] Improve symbol qualification in DefineInline code action

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5075c6821982: [clangd] Improve symbol qualification in 
DefineInline code action (authored by kadircet).

Changed prior to commit:
  https://reviews.llvm.org/D69033?vs=230424&id=230851#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69033

Files:
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.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
@@ -1667,6 +1667,148 @@
 EXPECT_EQ(apply(Case.first), Case.second) << Case.first;
 }
 
+TEST_F(DefineInlineTest, DropCommonNameSpecifiers) {
+  struct {
+llvm::StringRef Test;
+llvm::StringRef Expected;
+  } Cases[] = {
+  {R"cpp(
+namespace a { namespace b { void aux(); } }
+namespace ns1 {
+  void foo();
+  namespace qq { void test(); }
+  namespace ns2 {
+void bar();
+namespace ns3 { void baz(); }
+  }
+}
+
+using namespace a;
+using namespace a::b;
+using namespace ns1::qq;
+void ns1::ns2::ns3::b^az() {
+  foo();
+  bar();
+  baz();
+  ns1::ns2::ns3::baz();
+  aux();
+  test();
+})cpp",
+   R"cpp(
+namespace a { namespace b { void aux(); } }
+namespace ns1 {
+  void foo();
+  namespace qq { void test(); }
+  namespace ns2 {
+void bar();
+namespace ns3 { void baz(){
+  foo();
+  bar();
+  baz();
+  ns1::ns2::ns3::baz();
+  a::b::aux();
+  qq::test();
+} }
+  }
+}
+
+using namespace a;
+using namespace a::b;
+using namespace ns1::qq;
+)cpp"},
+  {R"cpp(
+namespace ns1 {
+  namespace qq { struct Foo { struct Bar {}; }; using B = Foo::Bar; }
+  namespace ns2 { void baz(); }
+}
+
+using namespace ns1::qq;
+void ns1::ns2::b^az() { Foo f; B b; })cpp",
+   R"cpp(
+namespace ns1 {
+  namespace qq { struct Foo { struct Bar {}; }; using B = Foo::Bar; }
+  namespace ns2 { void baz(){ qq::Foo f; qq::B b; } }
+}
+
+using namespace ns1::qq;
+)cpp"},
+  {R"cpp(
+namespace ns1 {
+  namespace qq {
+template struct Foo { template  struct Bar {}; };
+template
+using B = typename Foo::template Bar;
+  }
+  namespace ns2 { void baz(); }
+}
+
+using namespace ns1::qq;
+void ns1::ns2::b^az() { B b; })cpp",
+   R"cpp(
+namespace ns1 {
+  namespace qq {
+template struct Foo { template  struct Bar {}; };
+template
+using B = typename Foo::template Bar;
+  }
+  namespace ns2 { void baz(){ qq::B b; } }
+}
+
+using namespace ns1::qq;
+)cpp"},
+  };
+  for (const auto &Case : Cases)
+EXPECT_EQ(apply(Case.Test), Case.Expected) << Case.Test;
+}
+
+TEST_F(DefineInlineTest, QualifyWithUsingDirectives) {
+  llvm::StringRef Test = R"cpp(
+namespace a {
+  void bar();
+  namespace b { struct Foo{}; void aux(); }
+  namespace c { void cux(); }
+}
+using namespace a;
+using X = b::Foo;
+void foo();
+
+using namespace b;
+using namespace c;
+void ^foo() {
+  cux();
+  bar();
+  X x;
+  aux();
+  using namespace c;
+  // FIXME: The last reference to cux() in body of foo should not be
+  // qualified, since there is a using directive inside the function body.
+  cux();
+})cpp";
+  llvm::StringRef Expected = R"cpp(
+namespace a {
+  void bar();
+  namespace b { struct Foo{}; void aux(); }
+  namespace c { void cux(); }
+}
+using namespace a;
+using X = b::Foo;
+void foo(){
+  c::cux();
+  bar();
+  X x;
+  b::aux();
+  using namespace c;
+  // FIXME: The last reference to cux() in body of foo should not be
+  // qualified, since there is a using directive inside the function body.
+  c::cux();
+}
+
+using namespace b;
+using namespace c;
+)cpp";
+  EXPECT_EQ(apply(Test), Expected) << Test;
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -135,8 +135,10 @@
   return true;
 }
 
-// Re

[PATCH] D70494: [clangd] Fix diagnostic location for macro expansions

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe841029aef74: [clangd] Fix diagnostic location for macro 
expansions (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70494

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


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -941,7 +941,7 @@
   WithNote(Diag(Header.range(), "error occurred here");
 }
 
-TEST(IgnoreDiags, FromNonWrittenSources) {
+TEST(DiagsInHeaders, FromNonWrittenSources) {
   Annotations Main(R"cpp(
 #include [["a.h"]]
 void foo() {})cpp");
@@ -951,11 +951,49 @@
   TestTU TU = TestTU::withCode(Main.code());
   TU.AdditionalFiles = {{"a.h", Header.code()}};
   TU.ExtraArgs = {"-DFOO=NOOO"};
-  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(AllOf(
+  Diag(Main.range(),
+   "in included file: use of undeclared identifier 
'NOOO'"),
+  WithNote(Diag(Header.range(), "error occurred here");
+}
+
+TEST(DiagsInHeaders, ErrorFromMacroExpansion) {
+  Annotations Main(R"cpp(
+  void bar() {
+int fo;
+#include [["a.h"]]
+  })cpp");
+  Annotations Header(R"cpp(
+  #define X foo
+  X;)cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.AdditionalFiles = {{"a.h", Header.code()}};
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range(), "in included file: use of undeclared "
+ "identifier 'foo'; did you mean 'fo'?")));
+}
+
+TEST(DiagsInHeaders, ErrorFromMacroArgument) {
+  Annotations Main(R"cpp(
+  void bar() {
+int fo;
+#include [["a.h"]]
+  })cpp");
+  Annotations Header(R"cpp(
+  #define X(arg) arg
+  X(foo);)cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.AdditionalFiles = {{"a.h", Header.code()}};
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range(), "in included file: use of undeclared "
+ "identifier 'foo'; did you mean 'fo'?")));
 }
 
 TEST(IgnoreDiags, FromNonWrittenInclude) {
-  TestTU TU = TestTU::withCode("");
+  TestTU TU;
   TU.ExtraArgs.push_back("--include=a.h");
   TU.AdditionalFiles = {{"a.h", "void main();"}};
   // The diagnostic "main must return int" is from the header, we don't attempt
@@ -964,6 +1002,5 @@
 }
 
 } // namespace
-
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -117,8 +117,8 @@
   if (D.Severity < DiagnosticsEngine::Level::Error)
 return false;
 
-  const SourceLocation &DiagLoc = Info.getLocation();
   const SourceManager &SM = Info.getSourceManager();
+  const SourceLocation &DiagLoc = SM.getExpansionLoc(Info.getLocation());
   SourceLocation IncludeInMainFile;
   auto GetIncludeLoc = [&SM](SourceLocation SLoc) {
 return SM.getIncludeLoc(SM.getFileID(SLoc));


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -941,7 +941,7 @@
   WithNote(Diag(Header.range(), "error occurred here");
 }
 
-TEST(IgnoreDiags, FromNonWrittenSources) {
+TEST(DiagsInHeaders, FromNonWrittenSources) {
   Annotations Main(R"cpp(
 #include [["a.h"]]
 void foo() {})cpp");
@@ -951,11 +951,49 @@
   TestTU TU = TestTU::withCode(Main.code());
   TU.AdditionalFiles = {{"a.h", Header.code()}};
   TU.ExtraArgs = {"-DFOO=NOOO"};
-  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(AllOf(
+  Diag(Main.range(),
+   "in included file: use of undeclared identifier 'NOOO'"),
+  WithNote(Diag(Header.range(), "error occurred here");
+}
+
+TEST(DiagsInHeaders, ErrorFromMacroExpansion) {
+  Annotations Main(R"cpp(
+  void bar() {
+int fo;
+#include [["a.h"]]
+  })cpp");
+  Annotations Header(R"cpp(
+  #define X foo
+  X;)cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.AdditionalFiles = {{"a.h", Header.code()}};
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range(), "in include

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

In D68720#1755546 , @xbolva00 wrote:

> https://reviews.llvm.org/rG397fa687691876de9ff0fbaaf0def3ac5a48899c
>
> Commited?


No, still waiting for @rnk feedback. This commit is just me using the wrong 
remote when pushing for validation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

For the record : this validates fine on OSX, Linux and Windows, using the 
github actions setup by @tstellar

https://github.com/serge-sans-paille/llvm-project/pull/3


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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

[PATCH] D70546: [ARM][MVE][Intrinsics] Add MVE VMUL intrinsics.

[Dave Green via Phabricator via cfe-commits]

dmgreen added inline comments.



Comment at: clang/test/CodeGen/arm-mve-intrinsics/vmulq.c:12
+//
+uint32x4_t test_vmulq_u32(uint32x4_t a, uint32x4_t b)
+{

I think its worth adding some extra tests types. The way I like to think of it 
is that if someone went and completely changed the arm_mve.td in the future, 
there should be enough tests for them to tell they didn't miss anything. Even 
if they are not someone from Arm, that knows a lot about MVE intrinsics.



Comment at: llvm/test/CodeGen/Thumb2/LowOverheadLoops/wlstp.mir:219
   ; CHECK:   renamable $r3, dead $cpsr = tSUBi8 killed renamable $r3, 16, 14, 
$noreg
-  ; CHECK:   renamable $q0 = MVE_VMULt1i8 killed renamable $q1, killed 
renamable $q0, 0, $noreg, undef renamable $q0
+  ; CHECK:   renamable $q0 = MVE_VMULi8 killed renamable $q1, killed renamable 
$q0, 0, $noreg, undef renamable $q0
   ; CHECK:   MVE_VSTRBU8 killed renamable $q0, killed renamable $r4, 0, 0, 
killed $noreg :: (store 16 into %ir.scevgep1, align 1)

This I like


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70546



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

[PATCH] D68261: [clangd] Add "inline" keyword to prevent ODR-violations in DefineInline

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 60290 tests passed, 0 failed and 732 were skipped.

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68261



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

[PATCH] D70545: [ARM][MVE][Intrinsics] Add MVE VABD intrinsics.

[Dave Green via Phabricator via cfe-commits]

dmgreen added inline comments.



Comment at: clang/include/clang/Basic/arm_mve.td:33
 let params = T.Int in {
+def vabdq: Intrinsic $a, $b)>;
 def vaddq: Intrinsic;

Can this and vadbqf below be combined into one using T.Usual?

I believe the differences only usually come from "fadd" being different to 
"add". If they are both intrinsics (which sounds good to me for abd), then they 
can more happily live together.



Comment at: clang/test/CodeGen/arm-mve-intrinsics/vabdq.c:13
+uint32x4_t test_vabdq_u32(uint32x4_t a, uint32x4_t b)
+{
+#ifdef POLYMORPHIC

More tests please.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70545



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

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

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In D31130#1758453 , @fiesh wrote:

> In D31130#1711841 , @lebedev.ri 
> wrote:
>
> > The description only says what the patch does, not why this is the desired 
> > behavior.
> >  Also, this is now under a wrong license.
>
>
> What are the options in case the original author has abandonded this?  Can 
> anybody re-create the MR under a new license with the commit message fixed, 
> or is that an insurmountable license issue?


I guess we need specific permission from the author. maybe writing a mail to 
him/her would work? Then he/she can give permission (or decline it, which is 
unexpected i guess).


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] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

[fiesh via Phabricator via cfe-commits]

fiesh added a comment.

> I guess we need specific permission from the author. maybe writing a mail to 
> him/her would work? Then he/she can give permission (or decline it, which is 
> unexpected i guess).

I already wrote an email a couple weeks ago but haven't received a reply so 
far.  Let's hope they come back to this thread eventually.


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] D70632: clang-format-vs : Fix typo NUGET_EXE_DIR on README

[Hans Wennborg via Phabricator via cfe-commits]

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

Thanks!

Do you have commit access, or if not would you like me to commit on your behalf?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70632



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

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/Selection.cpp:49
+Result = New;
+  else if (Result != New)
+Result = SelectionTree::Partial;

I might be missing something, I don't understand why we set Partial if `Result 
!= New`.



Comment at: clang-tools-extra/clangd/Selection.cpp:446
+// Consider the macro expansion FLAG(x) -> int x = 0;
+// Neither S.getBegin() nor S.getEnd() are arg expansions, but x is.
+// The selection FLAG([[x]]) must partially select the VarDecl.

what's S for this case?



Comment at: clang-tools-extra/clangd/Selection.cpp:464
+  // represent one AST construct, but a macro invocation can represent 
many.
+  if (FID == SelFile) {
+// Tokens written directly in the main file.

nit: maybe use 
[`early-exist`](https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code)
 here?



Comment at: clang-tools-extra/clangd/Selection.cpp:496
+  // Check if the macro name is selected, don't claim it exclusively.
+  auto Expansion = SM.getDecomposedExpansionLoc(S.getBegin());
+  if (Expansion.first == SelFile)

not sure what's the rational behavior, but I think for the following case, we 
just have the TUDecl in the selection tree, maybe use the whole macro range?

```
#define M() 123

int d = M(^); // now we only have the TUDecl in the selection tree.
```



Comment at: clang-tools-extra/clangd/unittests/SelectionTests.cpp:139
+  R"cpp(
+int x(int);
+#define M(foo) x(foo)

could we have a  testcase to cover the "tokens expanded from another #include 
file" code path?




Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:125
+return {};
+  const Token *Begin =
+  llvm::partition_point(expandedTokens(), [&](const syntax::Token &T) {

nit: for code readability, I'd use `llvm::ArrayRef::iterator` 
type here.



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:126
+  const Token *Begin =
+  llvm::partition_point(expandedTokens(), [&](const syntax::Token &T) {
+return SourceMgr->isBeforeInTranslationUnit(T.location(), 
R.getBegin());

I think the parition_point requires the `ExpandedTokens` is partitioned, but I 
didn't found any documentation about this guarantee in the code, would be nice 
to have this in the comment (probably around the `ExpandedTokens`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70512



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

[PATCH] D70664: update string comparison in clang-format.py

[Paul Seyfert via Phabricator via cfe-commits]

pseyfert created this revision.
pseyfert added reviewers: MyDeveloperDay, klimek, llvm-commits, cfe-commits.
pseyfert added a project: clang-format.
Herald added a project: clang.

Python 3.8 introduces a SyntaxWarning about string comparisons with 'is'. This 
commit updates the string comparison in clang-format.py that is done with 'is 
not' to '!='. This should not break compatibility with older python versions 
(tested 3.4.9, 2.7.17, 2.7.5, 3.8.0).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70664

Files:
  clang/tools/clang-format/clang-format.py


Index: clang/tools/clang-format/clang-format.py
===
--- clang/tools/clang-format/clang-format.py
+++ clang/tools/clang-format/clang-format.py
@@ -132,7 +132,7 @@
 lines = lines[1:]
 sequence = difflib.SequenceMatcher(None, buf, lines)
 for op in reversed(sequence.get_opcodes()):
-  if op[0] is not 'equal':
+  if op[0] != 'equal':
 vim.current.buffer[op[1]:op[2]] = lines[op[3]:op[4]]
 if output.get('IncompleteFormat'):
   print('clang-format: incomplete (syntax errors)')


Index: clang/tools/clang-format/clang-format.py
===
--- clang/tools/clang-format/clang-format.py
+++ clang/tools/clang-format/clang-format.py
@@ -132,7 +132,7 @@
 lines = lines[1:]
 sequence = difflib.SequenceMatcher(None, buf, lines)
 for op in reversed(sequence.get_opcodes()):
-  if op[0] is not 'equal':
+  if op[0] != 'equal':
 vim.current.buffer[op[1]:op[2]] = lines[op[3]:op[4]]
 if output.get('IncompleteFormat'):
   print('clang-format: incomplete (syntax errors)')
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69266: [clangd] Define out-of-line availability checks

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

still lgtm.




Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:72
+// definition even if we are inside a source file.
+if (!Sel.AST.getASTContext().getLangOpts().IsHeaderFile)
+  return false;

nit: we have the `isHeaderFile` helper in the `SourceCode.h`, please use it (it 
is more precise).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69266



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

[PATCH] D70632: clang-format-vs : Fix typo NUGET_EXE_DIR on README

[empty2fill via Phabricator via cfe-commits]

empty2fill added a comment.

In D70632#1758572 , @hans wrote:

> Thanks!
>
> Do you have commit access, or if not would you like me to commit on your 
> behalf?


Commit on my behalf please. Thank you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70632



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

[PATCH] D70633: clang-format-vs : Fix Unicode formatting

[empty2fill via Phabricator via cfe-commits]

empty2fill added a comment.

In D70633#1758372 , @hans wrote:

> klimek, What do you think?
>
> empty2fill: Do you have an example input that I could use to hit the 
> "Specified argument was out of the range of valid values." error?


F10867396: Main.cpp 

Encoded Windows code page 949 for the Korean language.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70633



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

[PATCH] D70664: update string comparison in clang-format.py

[Manuel Klimek via Phabricator via cfe-commits]

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

lg


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70664



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

[PATCH] D69298: [clangd] Define out-of-line initial apply logic

[Haojian Wu via Phabricator via cfe-commits]

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

looks good.




Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:53
+// template keyword for templated functions.
+// FIXME: This is shared with define inline, move them to a common header once
+// we have a place for such.

this function seems trivial, I'd inline them into call sides for both define 
inline and outline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69298



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

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

[Gabor Bencze via Phabricator via cfe-commits]

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

Mark comments as Done.


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

https://reviews.llvm.org/D70052



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

[PATCH] D70440: [Driver] Use VFS to check if sanitizer blacklists exist

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D70440#1756262 , @thakis wrote:

> Do we need to add a dep on Frontend to DriverTests here? That's a heavy 
> dependency (it pulls in Sema etc). If it is needed, maybe the test is in the 
> wrong binary?


We need it solely for `TextDiagnosticPrinter`. Very handy to see the errors in 
case of failures.
There are probably no reasons it cannot live outside frontend, should be a 
manageable refactoring. (I haven't checked though, might be wrong)

Logically, the tests seem to be in the right place.

I agree it's not perfect, but there are probably more useful things one can 
spend their time on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70440



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

[PATCH] D70664: update string comparison in clang-format.py

[Paul Seyfert via Phabricator via cfe-commits]

pseyfert added a comment.

PS: I do not have commit access, so someone with the rights would need to land 
this for me. Thanks in advance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70664



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

[PATCH] D70545: [ARM][MVE][Intrinsics] Add MVE VABD intrinsics.

[Mark Murray via Phabricator via cfe-commits]

MarkMurrayARM updated this revision to Diff 230890.
MarkMurrayARM added a comment.

Merge all VABD intrinis types under T.Usual instead of doing the floats
separately.

Add more tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70545

Files:
  clang/include/clang/Basic/arm_mve.td
  clang/test/CodeGen/arm-mve-intrinsics/vabdq.c
  llvm/include/llvm/IR/IntrinsicsARM.td
  llvm/lib/Target/ARM/ARMInstrMVE.td
  llvm/test/CodeGen/Thumb2/mve-intrinsics/vabdq.ll

Index: llvm/test/CodeGen/Thumb2/mve-intrinsics/vabdq.ll
===
--- /dev/null
+++ llvm/test/CodeGen/Thumb2/mve-intrinsics/vabdq.ll
@@ -0,0 +1,62 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=thumbv8.1m.main -mattr=+mve.fp -verify-machineinstrs -o - %s | FileCheck %s
+
+define arm_aapcs_vfpcc <4 x i32> @test_vabdq_u32(<4 x i32> %a, <4 x i32> %b) {
+; CHECK-LABEL: test_vabdq_u32:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vabd.s32 q0, q0, q1
+; CHECK-NEXT:bx lr
+entry:
+  %0 = tail call <4 x i32> @llvm.arm.mve.vabd.v4i32(<4 x i32>%a, <4 x i32>%b)
+  ret <4 x i32> %0
+}
+
+declare <4 x i32> @llvm.arm.mve.vabd.v4i32(<4 x i32>, <4 x i32>)
+
+define arm_aapcs_vfpcc <4 x float> @test_vabdq_f32(<4 x float> %a, <4 x float> %b) {
+; CHECK-LABEL: test_vabdq_f32:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vabd.f32 q0, q0, q1
+; CHECK-NEXT:bx lr
+entry:
+  %0 = tail call <4 x float> @llvm.arm.mve.vabd.v4f32(<4 x float>%a, <4 x float>%b)
+  ret <4 x float> %0
+}
+
+declare <4 x float> @llvm.arm.mve.vabd.v4f32(<4 x float>, <4 x float>)
+
+define arm_aapcs_vfpcc <16 x i8> @test_vabdq_m_s8(<16 x i8> %inactive, <16 x i8> %a, <16 x i8> %b, i16 zeroext %p) {
+; CHECK-LABEL: test_vabdq_m_s8:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vmsr p0, r0
+; CHECK-NEXT:vpst
+; CHECK-NEXT:vabdt.s8 q0, q1, q2
+; CHECK-NEXT:bx lr
+entry:
+  %0 = zext i16 %p to i32
+  %1 = tail call <16 x i1> @llvm.arm.mve.pred.i2v.v16i1(i32 %0)
+  %2 = tail call <16 x i8> @llvm.arm.mve.abd.predicated.v16i8.v16i1(<16 x i8> %a, <16 x i8> %b, <16 x i1> %1, <16 x i8> %inactive)
+  ret <16 x i8> %2
+}
+
+declare <16 x i1> @llvm.arm.mve.pred.i2v.v16i1(i32)
+
+declare <16 x i8> @llvm.arm.mve.abd.predicated.v16i8.v16i1(<16 x i8>, <16 x i8>, <16 x i1>, <16 x i8>)
+
+define arm_aapcs_vfpcc <8 x half> @test_vabdq_m_f16(<8 x half> %inactive, <8 x half> %a, <8 x half> %b, i16 zeroext %p) {
+; CHECK-LABEL: test_vabdq_m_f16:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vmsr p0, r0
+; CHECK-NEXT:vpst
+; CHECK-NEXT:vabdt.f16 q0, q1, q2
+; CHECK-NEXT:bx lr
+entry:
+  %0 = zext i16 %p to i32
+  %1 = tail call <8 x i1> @llvm.arm.mve.pred.i2v.v8i1(i32 %0)
+  %2 = tail call <8 x half> @llvm.arm.mve.abd.predicated.v8f16.v8i1(<8 x half> %a, <8 x half> %b, <8 x i1> %1, <8 x half> %inactive)
+  ret <8 x half> %2
+}
+
+declare <8 x i1> @llvm.arm.mve.pred.i2v.v8i1(i32)
+
+declare <8 x half> @llvm.arm.mve.abd.predicated.v8f16.v8i1(<8 x half>, <8 x half>, <8 x i1>, <8 x half>)
Index: llvm/lib/Target/ARM/ARMInstrMVE.td
===
--- llvm/lib/Target/ARM/ARMInstrMVE.td
+++ llvm/lib/Target/ARM/ARMInstrMVE.td
@@ -1664,8 +1664,9 @@
 }
 
 
-class MVE_VABD_int size, list pattern=[]>
-  : MVE_int<"vabd", suffix, size, pattern> {
+class MVE_VABD_int size,
+ list pattern=[]>
+  : MVE_int {
 
   let Inst{28} = U;
   let Inst{25-23} = 0b110;
@@ -1676,12 +1677,35 @@
   let validForTailPredication = 1;
 }
 
-def MVE_VABDs8  : MVE_VABD_int<"s8", 0b0, 0b00>;
-def MVE_VABDs16 : MVE_VABD_int<"s16", 0b0, 0b01>;
-def MVE_VABDs32 : MVE_VABD_int<"s32", 0b0, 0b10>;
-def MVE_VABDu8  : MVE_VABD_int<"u8", 0b1, 0b00>;
-def MVE_VABDu16 : MVE_VABD_int<"u16", 0b1, 0b01>;
-def MVE_VABDu32 : MVE_VABD_int<"u32", 0b1, 0b10>;
+multiclass MVE_VABD_m {
+  def "" : MVE_VABD_int;
+
+  let Predicates = [HasMVEInt] in {
+// Unpredicated absolute difference
+def : Pat<(VTI.Vec (unpred_int (VTI.Vec MQPR:$Qm), (VTI.Vec MQPR:$Qn))),
+  (VTI.Vec (!cast(NAME)
+(VTI.Vec MQPR:$Qm), (VTI.Vec MQPR:$Qn)))>;
+
+// Predicated absolute difference
+def : Pat<(VTI.Vec (pred_int (VTI.Vec MQPR:$Qm), (VTI.Vec MQPR:$Qn),
+(VTI.Pred VCCR:$mask), (VTI.Vec MQPR:$inactive))),
+  (VTI.Vec (!cast(NAME)
+(VTI.Vec MQPR:$Qm), (VTI.Vec MQPR:$Qn),
+(i32 1), (VTI.Pred VCCR:$mask),
+(VTI.Vec MQPR:$inactive)))>;
+  }
+}
+
+multiclass MVE_VABD
+  : MVE_VABD_m<"vabd", VTI, int_arm_mve_vabd, int_arm_mve_abd_predicated>;
+
+defm MVE_VABDs8  : MVE_VABD;
+defm MVE_VABDs16 : MVE_VABD;
+defm MVE_VABDs32 : MVE_VABD;
+defm MVE_VABDu8  : MVE_VABD;
+defm MVE_VABDu16 : MVE_VABD;
+defm MVE_VABDu32 : MVE_

[PATCH] D70547: [ARM][MVE][Intrinsics] Add MVE VAND/VORR/VORN/VEOR/VBIC intrinsics.

[Mark Murray via Phabricator via cfe-commits]

MarkMurrayARM updated this revision to Diff 230891.
MarkMurrayARM added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70547

Files:
  clang/include/clang/Basic/arm_mve.td
  clang/include/clang/Basic/arm_mve_defs.td
  clang/test/CodeGen/arm-mve-intrinsics/vandq.c
  clang/test/CodeGen/arm-mve-intrinsics/vbicq.c
  clang/test/CodeGen/arm-mve-intrinsics/veorq.c
  clang/test/CodeGen/arm-mve-intrinsics/vornq.c
  clang/test/CodeGen/arm-mve-intrinsics/vorrq.c
  llvm/include/llvm/IR/IntrinsicsARM.td
  llvm/lib/Target/ARM/ARMInstrMVE.td
  llvm/test/CodeGen/Thumb2/mve-intrinsics/vandq.ll
  llvm/test/CodeGen/Thumb2/mve-intrinsics/vbicq.ll
  llvm/test/CodeGen/Thumb2/mve-intrinsics/veorq.ll
  llvm/test/CodeGen/Thumb2/mve-intrinsics/vornq.ll
  llvm/test/CodeGen/Thumb2/mve-intrinsics/vorrq.ll

Index: llvm/test/CodeGen/Thumb2/mve-intrinsics/vorrq.ll
===
--- /dev/null
+++ llvm/test/CodeGen/Thumb2/mve-intrinsics/vorrq.ll
@@ -0,0 +1,104 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=thumbv8.1m.main -mattr=+mve.fp -verify-machineinstrs -o - %s | FileCheck %s
+
+define arm_aapcs_vfpcc <16 x i8> @test_vorrq_u8(<16 x i8> %a, <16 x i8> %b) local_unnamed_addr #0 {
+; CHECK-LABEL: test_vorrq_u8:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vorr q0, q1, q0
+; CHECK-NEXT:bx lr
+entry:
+  %0 = or <16 x i8> %b, %a
+  ret <16 x i8> %0
+}
+
+define arm_aapcs_vfpcc <4 x i32> @test_vorrq_u32(<4 x i32> %a, <4 x i32> %b) local_unnamed_addr #0 {
+; CHECK-LABEL: test_vorrq_u32:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vorr q0, q1, q0
+; CHECK-NEXT:bx lr
+entry:
+  %0 = or <4 x i32> %b, %a
+  ret <4 x i32> %0
+}
+
+define arm_aapcs_vfpcc <8 x i16> @test_vorrq_s16(<8 x i16> %a, <8 x i16> %b) local_unnamed_addr #0 {
+; CHECK-LABEL: test_vorrq_s16:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vorr q0, q1, q0
+; CHECK-NEXT:bx lr
+entry:
+  %0 = or <8 x i16> %b, %a
+  ret <8 x i16> %0
+}
+
+define arm_aapcs_vfpcc <4 x float> @test_vorrq_f32(<4 x float> %a, <4 x float> %b) local_unnamed_addr #0 {
+; CHECK-LABEL: test_vorrq_f32:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vorr q0, q1, q0
+; CHECK-NEXT:bx lr
+entry:
+  %0 = bitcast <4 x float> %a to <4 x i32>
+  %1 = bitcast <4 x float> %b to <4 x i32>
+  %2 = or <4 x i32> %1, %0
+  %3 = bitcast <4 x i32> %2 to <4 x float>
+  ret <4 x float> %3
+}
+
+define arm_aapcs_vfpcc <16 x i8> @test_vorrq_m_s8(<16 x i8> %inactive, <16 x i8> %a, <16 x i8> %b, i16 zeroext %p) local_unnamed_addr #1 {
+; CHECK-LABEL: test_vorrq_m_s8:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vmsr p0, r0
+; CHECK-NEXT:vpst
+; CHECK-NEXT:vorrt q0, q1, q2
+; CHECK-NEXT:bx lr
+entry:
+  %0 = zext i16 %p to i32
+  %1 = tail call <16 x i1> @llvm.arm.mve.pred.i2v.v16i1(i32 %0)
+  %2 = tail call <16 x i8> @llvm.arm.mve.orr.predicated.v16i8.v16i1(<16 x i8> %a, <16 x i8> %b, <16 x i1> %1, <16 x i8> %inactive)
+  ret <16 x i8> %2
+}
+
+declare <16 x i1> @llvm.arm.mve.pred.i2v.v16i1(i32) #2
+
+declare <16 x i8> @llvm.arm.mve.orr.predicated.v16i8.v16i1(<16 x i8>, <16 x i8>, <16 x i1>, <16 x i8>) #2
+
+define arm_aapcs_vfpcc <8 x i16> @test_vorrq_m_u16(<8 x i16> %inactive, <8 x i16> %a, <8 x i16> %b, i16 zeroext %p) local_unnamed_addr #1 {
+; CHECK-LABEL: test_vorrq_m_u16:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vmsr p0, r0
+; CHECK-NEXT:vpst
+; CHECK-NEXT:vorrt q0, q1, q2
+; CHECK-NEXT:bx lr
+entry:
+  %0 = zext i16 %p to i32
+  %1 = tail call <8 x i1> @llvm.arm.mve.pred.i2v.v8i1(i32 %0)
+  %2 = tail call <8 x i16> @llvm.arm.mve.orr.predicated.v8i16.v8i1(<8 x i16> %a, <8 x i16> %b, <8 x i1> %1, <8 x i16> %inactive)
+  ret <8 x i16> %2
+}
+
+declare <8 x i1> @llvm.arm.mve.pred.i2v.v8i1(i32) #2
+
+declare <8 x i16> @llvm.arm.mve.orr.predicated.v8i16.v8i1(<8 x i16>, <8 x i16>, <8 x i1>, <8 x i16>) #2
+
+; Function Attrs: nounwind readnone
+define arm_aapcs_vfpcc <8 x half> @test_vorrq_m_f32(<4 x float> %inactive, <4 x float> %a, <4 x float> %b, i16 zeroext %p) local_unnamed_addr #1 {
+; CHECK-LABEL: test_vorrq_m_f32:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vmsr p0, r0
+; CHECK-NEXT:vpst
+; CHECK-NEXT:vorrt q0, q1, q2
+; CHECK-NEXT:bx lr
+entry:
+  %0 = bitcast <4 x float> %a to <4 x i32>
+  %1 = bitcast <4 x float> %b to <4 x i32>
+  %2 = zext i16 %p to i32
+  %3 = tail call <4 x i1> @llvm.arm.mve.pred.i2v.v4i1(i32 %2)
+  %4 = bitcast <4 x float> %inactive to <4 x i32>
+  %5 = tail call <4 x i32> @llvm.arm.mve.orr.predicated.v4i32.v4i1(<4 x i32> %0, <4 x i32> %1, <4 x i1> %3, <4 x i32> %4)
+  %6 = bitcast <4 x i32> %5 to <8 x half>
+  ret <8 x half> %6
+}
+
+declare <4 x i1> @llvm.arm.mve.pred.i2v.v4i1(i32) #2
+
+declare <4 x i32> @llvm.arm.mve.orr.predicated.v4i32.v4i1(<4 x i32>, <4 x i32>, <4 x i1>, <4 x i32>)

[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

We're now getting a ton of warnings about this in Chromium. 
(https://crbug.com/1028110)

And I'm not really sure what the warning wants us to do?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70342



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

[PATCH] D70547: [ARM][MVE][Intrinsics] Add MVE VAND/VORR/VORN/VEOR/VBIC intrinsics.

[Mark Murray via Phabricator via cfe-commits]

MarkMurrayARM updated this revision to Diff 230895.
MarkMurrayARM added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70547

Files:
  clang/include/clang/Basic/arm_mve.td
  clang/include/clang/Basic/arm_mve_defs.td
  clang/test/CodeGen/arm-mve-intrinsics/vandq.c
  clang/test/CodeGen/arm-mve-intrinsics/vbicq.c
  clang/test/CodeGen/arm-mve-intrinsics/veorq.c
  clang/test/CodeGen/arm-mve-intrinsics/vornq.c
  clang/test/CodeGen/arm-mve-intrinsics/vorrq.c
  llvm/include/llvm/IR/IntrinsicsARM.td
  llvm/lib/Target/ARM/ARMInstrMVE.td
  llvm/test/CodeGen/Thumb2/mve-intrinsics/vandq.ll
  llvm/test/CodeGen/Thumb2/mve-intrinsics/vbicq.ll
  llvm/test/CodeGen/Thumb2/mve-intrinsics/veorq.ll
  llvm/test/CodeGen/Thumb2/mve-intrinsics/vornq.ll
  llvm/test/CodeGen/Thumb2/mve-intrinsics/vorrq.ll

Index: llvm/test/CodeGen/Thumb2/mve-intrinsics/vorrq.ll
===
--- /dev/null
+++ llvm/test/CodeGen/Thumb2/mve-intrinsics/vorrq.ll
@@ -0,0 +1,104 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=thumbv8.1m.main -mattr=+mve.fp -verify-machineinstrs -o - %s | FileCheck %s
+
+define arm_aapcs_vfpcc <16 x i8> @test_vorrq_u8(<16 x i8> %a, <16 x i8> %b) local_unnamed_addr #0 {
+; CHECK-LABEL: test_vorrq_u8:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vorr q0, q1, q0
+; CHECK-NEXT:bx lr
+entry:
+  %0 = or <16 x i8> %b, %a
+  ret <16 x i8> %0
+}
+
+define arm_aapcs_vfpcc <4 x i32> @test_vorrq_u32(<4 x i32> %a, <4 x i32> %b) local_unnamed_addr #0 {
+; CHECK-LABEL: test_vorrq_u32:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vorr q0, q1, q0
+; CHECK-NEXT:bx lr
+entry:
+  %0 = or <4 x i32> %b, %a
+  ret <4 x i32> %0
+}
+
+define arm_aapcs_vfpcc <8 x i16> @test_vorrq_s16(<8 x i16> %a, <8 x i16> %b) local_unnamed_addr #0 {
+; CHECK-LABEL: test_vorrq_s16:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vorr q0, q1, q0
+; CHECK-NEXT:bx lr
+entry:
+  %0 = or <8 x i16> %b, %a
+  ret <8 x i16> %0
+}
+
+define arm_aapcs_vfpcc <4 x float> @test_vorrq_f32(<4 x float> %a, <4 x float> %b) local_unnamed_addr #0 {
+; CHECK-LABEL: test_vorrq_f32:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vorr q0, q1, q0
+; CHECK-NEXT:bx lr
+entry:
+  %0 = bitcast <4 x float> %a to <4 x i32>
+  %1 = bitcast <4 x float> %b to <4 x i32>
+  %2 = or <4 x i32> %1, %0
+  %3 = bitcast <4 x i32> %2 to <4 x float>
+  ret <4 x float> %3
+}
+
+define arm_aapcs_vfpcc <16 x i8> @test_vorrq_m_s8(<16 x i8> %inactive, <16 x i8> %a, <16 x i8> %b, i16 zeroext %p) local_unnamed_addr #1 {
+; CHECK-LABEL: test_vorrq_m_s8:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vmsr p0, r0
+; CHECK-NEXT:vpst
+; CHECK-NEXT:vorrt q0, q1, q2
+; CHECK-NEXT:bx lr
+entry:
+  %0 = zext i16 %p to i32
+  %1 = tail call <16 x i1> @llvm.arm.mve.pred.i2v.v16i1(i32 %0)
+  %2 = tail call <16 x i8> @llvm.arm.mve.orr.predicated.v16i8.v16i1(<16 x i8> %a, <16 x i8> %b, <16 x i1> %1, <16 x i8> %inactive)
+  ret <16 x i8> %2
+}
+
+declare <16 x i1> @llvm.arm.mve.pred.i2v.v16i1(i32) #2
+
+declare <16 x i8> @llvm.arm.mve.orr.predicated.v16i8.v16i1(<16 x i8>, <16 x i8>, <16 x i1>, <16 x i8>) #2
+
+define arm_aapcs_vfpcc <8 x i16> @test_vorrq_m_u16(<8 x i16> %inactive, <8 x i16> %a, <8 x i16> %b, i16 zeroext %p) local_unnamed_addr #1 {
+; CHECK-LABEL: test_vorrq_m_u16:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vmsr p0, r0
+; CHECK-NEXT:vpst
+; CHECK-NEXT:vorrt q0, q1, q2
+; CHECK-NEXT:bx lr
+entry:
+  %0 = zext i16 %p to i32
+  %1 = tail call <8 x i1> @llvm.arm.mve.pred.i2v.v8i1(i32 %0)
+  %2 = tail call <8 x i16> @llvm.arm.mve.orr.predicated.v8i16.v8i1(<8 x i16> %a, <8 x i16> %b, <8 x i1> %1, <8 x i16> %inactive)
+  ret <8 x i16> %2
+}
+
+declare <8 x i1> @llvm.arm.mve.pred.i2v.v8i1(i32) #2
+
+declare <8 x i16> @llvm.arm.mve.orr.predicated.v8i16.v8i1(<8 x i16>, <8 x i16>, <8 x i1>, <8 x i16>) #2
+
+; Function Attrs: nounwind readnone
+define arm_aapcs_vfpcc <8 x half> @test_vorrq_m_f32(<4 x float> %inactive, <4 x float> %a, <4 x float> %b, i16 zeroext %p) local_unnamed_addr #1 {
+; CHECK-LABEL: test_vorrq_m_f32:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vmsr p0, r0
+; CHECK-NEXT:vpst
+; CHECK-NEXT:vorrt q0, q1, q2
+; CHECK-NEXT:bx lr
+entry:
+  %0 = bitcast <4 x float> %a to <4 x i32>
+  %1 = bitcast <4 x float> %b to <4 x i32>
+  %2 = zext i16 %p to i32
+  %3 = tail call <4 x i1> @llvm.arm.mve.pred.i2v.v4i1(i32 %2)
+  %4 = bitcast <4 x float> %inactive to <4 x i32>
+  %5 = tail call <4 x i32> @llvm.arm.mve.orr.predicated.v4i32.v4i1(<4 x i32> %0, <4 x i32> %1, <4 x i1> %3, <4 x i32> %4)
+  %6 = bitcast <4 x i32> %5 to <8 x half>
+  ret <8 x half> %6
+}
+
+declare <4 x i1> @llvm.arm.mve.pred.i2v.v4i1(i32) #2
+
+declare <4 x i32> @llvm.arm.mve.orr.predicated.v4i32.v4i1(<4 x i32>, <4 x i32>, <4 x i1>, <4 x i32>)

[PATCH] D69092: [CodeGen] #pragma clang transform

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69092



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

[PATCH] D70547: [ARM][MVE][Intrinsics] Add MVE VAND/VORR/VORN/VEOR/VBIC intrinsics.

[Mark Murray via Phabricator via cfe-commits]

MarkMurrayARM updated this revision to Diff 230896.
MarkMurrayARM added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70547

Files:
  clang/include/clang/Basic/arm_mve.td
  clang/include/clang/Basic/arm_mve_defs.td
  clang/test/CodeGen/arm-mve-intrinsics/vandq.c
  clang/test/CodeGen/arm-mve-intrinsics/vbicq.c
  clang/test/CodeGen/arm-mve-intrinsics/veorq.c
  clang/test/CodeGen/arm-mve-intrinsics/vornq.c
  clang/test/CodeGen/arm-mve-intrinsics/vorrq.c
  llvm/include/llvm/IR/IntrinsicsARM.td
  llvm/lib/Target/ARM/ARMInstrMVE.td
  llvm/test/CodeGen/Thumb2/mve-intrinsics/vandq.ll
  llvm/test/CodeGen/Thumb2/mve-intrinsics/vbicq.ll
  llvm/test/CodeGen/Thumb2/mve-intrinsics/veorq.ll
  llvm/test/CodeGen/Thumb2/mve-intrinsics/vornq.ll
  llvm/test/CodeGen/Thumb2/mve-intrinsics/vorrq.ll

Index: llvm/test/CodeGen/Thumb2/mve-intrinsics/vorrq.ll
===
--- /dev/null
+++ llvm/test/CodeGen/Thumb2/mve-intrinsics/vorrq.ll
@@ -0,0 +1,104 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=thumbv8.1m.main -mattr=+mve.fp -verify-machineinstrs -o - %s | FileCheck %s
+
+define arm_aapcs_vfpcc <16 x i8> @test_vorrq_u8(<16 x i8> %a, <16 x i8> %b) local_unnamed_addr #0 {
+; CHECK-LABEL: test_vorrq_u8:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vorr q0, q1, q0
+; CHECK-NEXT:bx lr
+entry:
+  %0 = or <16 x i8> %b, %a
+  ret <16 x i8> %0
+}
+
+define arm_aapcs_vfpcc <4 x i32> @test_vorrq_u32(<4 x i32> %a, <4 x i32> %b) local_unnamed_addr #0 {
+; CHECK-LABEL: test_vorrq_u32:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vorr q0, q1, q0
+; CHECK-NEXT:bx lr
+entry:
+  %0 = or <4 x i32> %b, %a
+  ret <4 x i32> %0
+}
+
+define arm_aapcs_vfpcc <8 x i16> @test_vorrq_s16(<8 x i16> %a, <8 x i16> %b) local_unnamed_addr #0 {
+; CHECK-LABEL: test_vorrq_s16:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vorr q0, q1, q0
+; CHECK-NEXT:bx lr
+entry:
+  %0 = or <8 x i16> %b, %a
+  ret <8 x i16> %0
+}
+
+define arm_aapcs_vfpcc <4 x float> @test_vorrq_f32(<4 x float> %a, <4 x float> %b) local_unnamed_addr #0 {
+; CHECK-LABEL: test_vorrq_f32:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vorr q0, q1, q0
+; CHECK-NEXT:bx lr
+entry:
+  %0 = bitcast <4 x float> %a to <4 x i32>
+  %1 = bitcast <4 x float> %b to <4 x i32>
+  %2 = or <4 x i32> %1, %0
+  %3 = bitcast <4 x i32> %2 to <4 x float>
+  ret <4 x float> %3
+}
+
+define arm_aapcs_vfpcc <16 x i8> @test_vorrq_m_s8(<16 x i8> %inactive, <16 x i8> %a, <16 x i8> %b, i16 zeroext %p) local_unnamed_addr #1 {
+; CHECK-LABEL: test_vorrq_m_s8:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vmsr p0, r0
+; CHECK-NEXT:vpst
+; CHECK-NEXT:vorrt q0, q1, q2
+; CHECK-NEXT:bx lr
+entry:
+  %0 = zext i16 %p to i32
+  %1 = tail call <16 x i1> @llvm.arm.mve.pred.i2v.v16i1(i32 %0)
+  %2 = tail call <16 x i8> @llvm.arm.mve.orr.predicated.v16i8.v16i1(<16 x i8> %a, <16 x i8> %b, <16 x i1> %1, <16 x i8> %inactive)
+  ret <16 x i8> %2
+}
+
+declare <16 x i1> @llvm.arm.mve.pred.i2v.v16i1(i32) #2
+
+declare <16 x i8> @llvm.arm.mve.orr.predicated.v16i8.v16i1(<16 x i8>, <16 x i8>, <16 x i1>, <16 x i8>) #2
+
+define arm_aapcs_vfpcc <8 x i16> @test_vorrq_m_u16(<8 x i16> %inactive, <8 x i16> %a, <8 x i16> %b, i16 zeroext %p) local_unnamed_addr #1 {
+; CHECK-LABEL: test_vorrq_m_u16:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vmsr p0, r0
+; CHECK-NEXT:vpst
+; CHECK-NEXT:vorrt q0, q1, q2
+; CHECK-NEXT:bx lr
+entry:
+  %0 = zext i16 %p to i32
+  %1 = tail call <8 x i1> @llvm.arm.mve.pred.i2v.v8i1(i32 %0)
+  %2 = tail call <8 x i16> @llvm.arm.mve.orr.predicated.v8i16.v8i1(<8 x i16> %a, <8 x i16> %b, <8 x i1> %1, <8 x i16> %inactive)
+  ret <8 x i16> %2
+}
+
+declare <8 x i1> @llvm.arm.mve.pred.i2v.v8i1(i32) #2
+
+declare <8 x i16> @llvm.arm.mve.orr.predicated.v8i16.v8i1(<8 x i16>, <8 x i16>, <8 x i1>, <8 x i16>) #2
+
+; Function Attrs: nounwind readnone
+define arm_aapcs_vfpcc <8 x half> @test_vorrq_m_f32(<4 x float> %inactive, <4 x float> %a, <4 x float> %b, i16 zeroext %p) local_unnamed_addr #1 {
+; CHECK-LABEL: test_vorrq_m_f32:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vmsr p0, r0
+; CHECK-NEXT:vpst
+; CHECK-NEXT:vorrt q0, q1, q2
+; CHECK-NEXT:bx lr
+entry:
+  %0 = bitcast <4 x float> %a to <4 x i32>
+  %1 = bitcast <4 x float> %b to <4 x i32>
+  %2 = zext i16 %p to i32
+  %3 = tail call <4 x i1> @llvm.arm.mve.pred.i2v.v4i1(i32 %2)
+  %4 = bitcast <4 x float> %inactive to <4 x i32>
+  %5 = tail call <4 x i32> @llvm.arm.mve.orr.predicated.v4i32.v4i1(<4 x i32> %0, <4 x i32> %1, <4 x i1> %3, <4 x i32> %4)
+  %6 = bitcast <4 x i32> %5 to <8 x half>
+  ret <8 x half> %6
+}
+
+declare <4 x i1> @llvm.arm.mve.pred.i2v.v4i1(i32) #2
+
+declare <4 x i32> @llvm.arm.mve.orr.predicated.v4i32.v4i1(<4 x i32>, <4 x i32>, <4 x i1>, <4 x i32>)

[PATCH] D70671: [clang][CodeGen] Fix wrong memcpy size of no_unique_address in FieldMemcpyizer

[Senran Zhang via Phabricator via cfe-commits]

zsrkmyn created this revision.
zsrkmyn added reviewers: erichkeane, aaron.ballman, MaskRay, rjmccall.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When generating ctor, FieldMemcpyizer wrongly treated zero-sized class members
as what should be copied, and generated wrong memcpy size under some special
circumstances. This patch tries to fix it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70671

Files:
  clang/lib/CodeGen/CGClass.cpp
  clang/test/CodeGenCXX/no-unique-address-2.cpp


Index: clang/test/CodeGenCXX/no-unique-address-2.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/no-unique-address-2.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -std=c++2a %s -emit-llvm -o - -triple x86_64-linux-gnu | 
FileCheck %s
+
+struct TriviallyCopyable {};
+
+struct NonTriviallyCopyable {
+  NonTriviallyCopyable() = default;
+  NonTriviallyCopyable(const NonTriviallyCopyable&) = default;
+  NonTriviallyCopyable(NonTriviallyCopyable &&) {}
+};
+
+struct Foo {
+  int i;
+  [[no_unique_address]] TriviallyCopyable m;
+  [[no_unique_address]] NonTriviallyCopyable n;
+};
+
+void call()
+{
+  Foo foo;
+  Foo foo2(static_cast(foo));
+}
+
+// The memcpy call should copy exact 4 bytes for member 'int i'
+// CHECK: define {{.*}} void @_ZN3FooC2EOS_
+// CHECK:  call void @llvm.memcpy.p0i8.p0i8.i64(i8* {{.+}}, i8* {{.+}}, i64 4, 
i1 false)
+// CHECK:  call void @_ZN20NonTriviallyCopyableC2EOS_
Index: clang/lib/CodeGen/CGClass.cpp
===
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -914,6 +914,8 @@
 }
 
 void addMemcpyableField(FieldDecl *F) {
+  if (F->isZeroSize(CGF.getContext()))
+return;
   if (!FirstField)
 addInitialField(F);
   else


Index: clang/test/CodeGenCXX/no-unique-address-2.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/no-unique-address-2.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -std=c++2a %s -emit-llvm -o - -triple x86_64-linux-gnu | FileCheck %s
+
+struct TriviallyCopyable {};
+
+struct NonTriviallyCopyable {
+  NonTriviallyCopyable() = default;
+  NonTriviallyCopyable(const NonTriviallyCopyable&) = default;
+  NonTriviallyCopyable(NonTriviallyCopyable &&) {}
+};
+
+struct Foo {
+  int i;
+  [[no_unique_address]] TriviallyCopyable m;
+  [[no_unique_address]] NonTriviallyCopyable n;
+};
+
+void call()
+{
+  Foo foo;
+  Foo foo2(static_cast(foo));
+}
+
+// The memcpy call should copy exact 4 bytes for member 'int i'
+// CHECK: define {{.*}} void @_ZN3FooC2EOS_
+// CHECK:  call void @llvm.memcpy.p0i8.p0i8.i64(i8* {{.+}}, i8* {{.+}}, i64 4, i1 false)
+// CHECK:  call void @_ZN20NonTriviallyCopyableC2EOS_
Index: clang/lib/CodeGen/CGClass.cpp
===
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -914,6 +914,8 @@
 }
 
 void addMemcpyableField(FieldDecl *F) {
+  if (F->isZeroSize(CGF.getContext()))
+return;
   if (!FirstField)
 addInitialField(F);
   else
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@sylvestre.ledru backport for  release/9.x branch: 
https://sergesanspaille.fedorapeople.org/0001-Stack-clash-mir-attempt.patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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

[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Add copy ctor/op= manually.

If you have a lot of warnings, you can use -Wno-deprecated-copy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70342



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

Re: [clang] 21f2647 - Revert 3f91705ca54 "ARM-NEON: make type modifiers orthogonal and allow multiple modifiers."

[Hans Wennborg via cfe-commits]

+Tim, I've reverted this since it seems to have broken the vector
initialization intrinsics.

(I would have replied to the llvm-commits email for the original
change, but it seems it didn't reach the list?)

On Mon, Nov 25, 2019 at 4:35 PM Hans Wennborg via cfe-commits
 wrote:
>
>
> Author: Hans Wennborg
> Date: 2019-11-25T16:27:53+01:00
> New Revision: 21f26470e9747c472d3c18654e676cbea8393635
>
> URL: 
> https://github.com/llvm/llvm-project/commit/21f26470e9747c472d3c18654e676cbea8393635
> DIFF: 
> https://github.com/llvm/llvm-project/commit/21f26470e9747c472d3c18654e676cbea8393635.diff
>
> LOG: Revert 3f91705ca54 "ARM-NEON: make type modifiers orthogonal and allow 
> multiple modifiers."
>
> This broke the vcreate_u64 intrinsic. Example:
>
>   $ cat /tmp/a.cc
>   #include 
>
>   void g() {
> auto v = vcreate_u64(0);
>   }
>   $ bin/clang -c /tmp/a.cc --target=arm-linux-androideabi16 -march=armv7-a
>   /tmp/a.cc:4:12: error: C-style cast from scalar 'int' to vector 
> 'uint64x1_t' (vector of 1 'uint64_t' value) of different size
> auto v = vcreate_u64(0);
>  ^~
>   
> /work/llvm.monorepo/build.release/lib/clang/10.0.0/include/arm_neon.h:4144:11:
>  note: expanded from macro 'vcreate_u64'
> __ret = (uint64x1_t)(__p0); \
> ^~
>
> Reverting until this can be investigated.
>
> > The modifier system used to mutate types on NEON intrinsic definitions had a
> > separate letter for all kinds of transformations that might be needed, and 
> > we
> > were quite quickly running out of letters to use. This patch converts to a 
> > much
> > smaller set of orthogonal modifiers that can be applied together to achieve 
> > the
> > desired effect.
> >
> > When merging with downstream it is likely to cause a conflict with any local
> > modifications to the .td files. There is a new script in
> > utils/convert_arm_neon.py that was used to convert all .td definitions and I
> > would suggest running it on the last downstream version of those files 
> > before
> > this commit rather than resolving conflicts manually.
>
> Added:
>
>
> Modified:
> clang/include/clang/Basic/arm_fp16.td
> clang/include/clang/Basic/arm_neon.td
> clang/include/clang/Basic/arm_neon_incl.td
> clang/test/CodeGen/aarch64-neon-intrinsics.c
> clang/test/CodeGen/aarch64-neon-scalar-x-indexed-elem.c
> clang/utils/TableGen/NeonEmitter.cpp
>
> Removed:
> clang/utils/convert_arm_neon.py
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69618: NeonEmitter: clean up prototype modifiers

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

Sorry about the delay investigating this, your e-mail bypassed my inbox for 
some reason and I only noticed the issue when Hans reverted the change this 
afternoon. I'm looking into it now.


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

https://reviews.llvm.org/D69618



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

The test also break in our internal integrate.
Relying on having programs in path and ignoring the `-B` and sysroot is 
definitely a bad idea.

I would suggest reverting this patch and re-landing after doing another round 
of review and fixing those issues.
@thakis, @sunfish, any objections or thoughts on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70539: [clang][CodeGen] Implicit Conversion Sanitizer: handle increment/derement (PR44054)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70539



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

[PATCH] D70480: [clangd] Use expansion location when the ref is inside macros.

[Haojian Wu via Phabricator via cfe-commits]

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

address comments

- add related test for ast-based xrefs
- add comments to clarify using different location for symbols/references.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70480

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


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -796,6 +796,19 @@
 } // namespace ns
 int main() { [[^ns]]::Foo foo; }
   )cpp",
+
+  R"cpp(// Macros
+#define TYPE(X) X
+#define FOO Foo
+#define CAT(X, Y) X##Y
+class [[Fo^o]] {};
+void test() {
+  TYPE([[Foo]]) foo;
+  [[FOO]] foo2;
+  TYPE(TYPE([[Foo]])) foo3;
+  [[CAT]](Fo, o) foo4;
+}
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -644,6 +644,29 @@
   HaveRanges(Header.ranges();
 }
 
+TEST_F(SymbolCollectorTest, RefsOnMacros) {
+  // Refs collected from SymbolCollector behave in the same way as
+  // AST-based xrefs.
+  CollectorOpts.RefFilter = RefKind::All;
+  CollectorOpts.RefsInHeaders = true;
+  Annotations Header(R"(
+#define TYPE(X) X
+#define FOO Foo
+#define CAT(X, Y) X##Y
+class [[Foo]] {};
+void test() {
+  TYPE([[Foo]]) foo;
+  [[FOO]] foo2;
+  TYPE(TYPE([[Foo]])) foo3;
+  [[CAT]](Fo, o) foo4;
+}
+  )");
+  CollectorOpts.RefFilter = RefKind::All;
+  runSymbolCollector(Header.code(), "");
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
+  HaveRanges(Header.ranges();
+}
+
 TEST_F(SymbolCollectorTest, HeaderAsMainFile) {
   CollectorOpts.RefFilter = RefKind::All;
   Annotations Header(R"(
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -275,10 +275,9 @@
   // Mark D as referenced if this is a reference coming from the main file.
   // D may not be an interesting symbol, but it's cheaper to check at the end.
   auto &SM = ASTCtx->getSourceManager();
-  auto SpellingLoc = SM.getSpellingLoc(Loc);
   if (Opts.CountReferences &&
   (Roles & static_cast(index::SymbolRole::Reference)) &&
-  SM.getFileID(SpellingLoc) == SM.getMainFileID())
+  SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
 ReferencedDecls.insert(ND);
 
   auto ID = getSymbolID(ND);
@@ -311,9 +310,13 @@
   !shouldCollectSymbol(*ND, *ASTCtx, Opts, IsMainFileOnly))
 return true;
   // Do not store references to main-file symbols.
+  // Unlike other fields, e.g. Symbols (which use spelling locations), we use
+  // expansion locations for references comming from macros (as it aligns the
+  // behavior of clangd's AST-based xref).
   if (CollectRef && !IsMainFileOnly && !isa(ND) &&
-  (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))
-DeclRefs[ND].emplace_back(SpellingLoc, Roles);
+  (Opts.RefsInHeaders ||
+   SM.getFileID(SM.getFileLoc(Loc)) == SM.getMainFileID()))
+DeclRefs[ND].emplace_back(SM.getFileLoc(Loc), Roles);
   // Don't continue indexing if this is a mere reference.
   if (IsOnlyRef)
 return true;


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -796,6 +796,19 @@
 } // namespace ns
 int main() { [[^ns]]::Foo foo; }
   )cpp",
+
+  R"cpp(// Macros
+#define TYPE(X) X
+#define FOO Foo
+#define CAT(X, Y) X##Y
+class [[Fo^o]] {};
+void test() {
+  TYPE([[Foo]]) foo;
+  [[FOO]] foo2;
+  TYPE(TYPE([[Foo]])) foo3;
+  [[CAT]](Fo, o) foo4;
+}
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests

[PATCH] D70480: [clangd] Use expansion location when the ref is inside macros.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:280
   (Roles & static_cast(index::SymbolRole::Reference)) &&
-  SM.getFileID(SpellingLoc) == SM.getMainFileID())
+  SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
 ReferencedDecls.insert(ND);

ilya-biryukov wrote:
> hokein wrote:
> > ilya-biryukov wrote:
> > > We're using `getSpellingLoc` here and `getFileLoc` later. Why not use 
> > > `getFileLoc` everywhere?
> > > 
> > > Having a variable (similar to the `SpellingLoc` we had before) and 
> > > calling `getFileLoc` only once also seems preferable.
> > > We're using getSpellingLoc here and getFileLoc later. Why not use 
> > > getFileLoc everywhere?
> > 
> > There are two things in SymbolCollector:
> > - symbols & ranking signals, we use spelling location for them, the code is 
> > part of this, `ReferencedDecls` is used to calculate the ranking
> > - references
> > 
> > this patch only targets the reference part (changing the loc here would 
> > break many assumptions I think, and there was a failure test).
> - What are the assumptions that it will break?
> - What is rationale for using spelling locations for ranking and file 
> location for references?
> 
> It would be nice to have this spelled out somewhere in the code, too. 
> Currently this looks like an accidental inconsistency. Especially given that 
> `getFileLoc` and `getSpellingLoc` are often the same.
Added comments to clarify the difference between references and other fields. 



Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:659
+  TYPE(TYPE([[Foo]])) foo3;
+  [[CAT]](Fo, o) foo4;
+}

ilya-biryukov wrote:
> Previously we would not report any location at all in that case, right?
> Not sure how common this is, hope this won't blow up our index size too much. 
> No need to change anything now, but we should be ready to revert if needed.
> 
> Worth putting a comment here that AST-based XRefs behave in the same way. 
> (And maybe adding a test there, if there isn't one already)
Yes, I measure the memory usage before vs after, it increased ~5% memory usage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70480



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

[PATCH] D70480: [clangd] Use expansion location when the ref is inside macros.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 60296 tests passed, 0 failed and 732 were skipped.

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70480



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

[PATCH] D70424: clang/AMDGPU: Fix default for frame-pointer attribute

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

LGTM. But I am wondering how it affects -g. Do we need to keep frame pointer 
when -g is specified? Should we add a test for -O3 -g?


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

https://reviews.llvm.org/D70424



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

[PATCH] D70539: [clang][CodeGen] Implicit Conversion Sanitizer: handle increment/derement (PR44054)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Thank you for the review!
I'll hold this off for a bit in case anyone else wants to comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70539



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

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

Thanks for the feedback Russell!

Can you possibly try again with `/MT`? (ie. `-DLLVM_USE_CRT_RELEASE=MT`)

In the `abba_test.ps1` script, `ninja check-all` is only used for preparing 
clang.exe with the patch. The A/B loop //does not// use `check-all`.

I've modified the `abba_test.ps1` script to only build `llvm/`, with the same 
options as you do. There was also a bug in the timespan calculation, the test 
would last a lot more than you specified.

I also found out that on some of our multi-socket servers, Ninja defaulted to 
only the # threads from the first socket. If you had a 2x 18-core system, Ninja 
would default to 38 threads (18 x 2 + 2) instead of 74 threads (2 x 18 x 2 + 
2). This behavior seems to be random,  depending on the system. This has been 
fixed by Ninja PR 1674 , I've 
tested all our systems with this patch and they now all default to the proper # 
of threads.

I'm doing a new round of tests, I'll get back with updated figures.


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

https://reviews.llvm.org/D69825



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

[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D70342#1758832 , @xbolva00 wrote:

> Add copy ctor/op= manually.


Does the warning mean that the implicitly defined copy ctor is not going to 
work in some later version of the language?

I'm just trying to understand what it's warning about. If it's pointing out 
bugs in our code, we should fix them obviously.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70342



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

[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0619r4.html#3.2

So C++20 or C++23.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70342



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

[PATCH] D70675: [AIX] Disable clang python binding tests

[David Tenty via Phabricator via cfe-commits]

daltenty created this revision.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.

The Python ctypes FFI interface is broken on AIX, it cannot properly pass
structures containing  arrays ( https://bugs.python.org/issue38628). So
disable the clang python binding tests on AIX till this is resolved.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70675

Files:
  clang/bindings/python/tests/CMakeLists.txt


Index: clang/bindings/python/tests/CMakeLists.txt
===
--- clang/bindings/python/tests/CMakeLists.txt
+++ clang/bindings/python/tests/CMakeLists.txt
@@ -32,6 +32,11 @@
   set(RUN_PYTHON_TESTS FALSE)
 endif()
 
+# The Python FFI interface is broken on AIX: 
https://bugs.python.org/issue38628.
+if(${CMAKE_SYSTEM_NAME} MATCHES "AIX")
+  set(RUN_PYTHON_TESTS FALSE)
+endif()
+
 # AArch64, Hexagon, and Sparc have known test failures that need to be
 # addressed.
 # SystemZ has broken Python/FFI interface:


Index: clang/bindings/python/tests/CMakeLists.txt
===
--- clang/bindings/python/tests/CMakeLists.txt
+++ clang/bindings/python/tests/CMakeLists.txt
@@ -32,6 +32,11 @@
   set(RUN_PYTHON_TESTS FALSE)
 endif()
 
+# The Python FFI interface is broken on AIX: https://bugs.python.org/issue38628.
+if(${CMAKE_SYSTEM_NAME} MATCHES "AIX")
+  set(RUN_PYTHON_TESTS FALSE)
+endif()
+
 # AArch64, Hexagon, and Sparc have known test failures that need to be
 # addressed.
 # SystemZ has broken Python/FFI interface:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish marked an inline comment as done.
sunfish added a comment.

In D70500#1757735 , @thakis wrote:

> Please don't add code to the driver that runs programs off PATH. Nothing else 
> does this.


Clang's normal `GetProgramPath` does do this: 
https://github.com/llvm/llvm-project/blob/master/clang/lib/Driver/Driver.cpp#L4719

> If you need to run an external program, look for it next to the compiler, 
> like we do for gas with -fno-integrated-as, linker, etc. People can use the 
> existing -B flag to make clang look elsewhere if they need that.

Unfortunately, wasm-opt isn't typically installed alongside the compiler. It's 
also not a typical system tool like an assembler, which is why it didn't seem 
right to search the -B paths.

The first version of my patch used a dedicated environment variable, WASM_OPT, 
rather than PATH, but I changed it to PATH after review feedback. If people 
feel using -B paths, and COMPILER_PATH, are appropriate, we can use 
`GetProgramPath` itself, though note that that still does have a PATH fallback.




Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:137
+  getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple 
+
+   "/llvm-lto");
+}

thakis wrote:
> sunfish wrote:
> > sbc100 wrote:
> > > Is there any kind of president here?   i.e. do any other platforms have 
> > > support for this kind thing?  Seems like good idea I'd just like to be 
> > > sure we follow existing practices.
> > I looked around, but didn't see anything similar.
> One immediate problem of this approach is that if HAVE_VCS_VERSION_INC is not 
> set, then the test fails, but if it is set, `clang --version` reports a 
> current git hash, which is either out of date or requires a pretty big 
> rebuild on every single git commit (i.e. not just after `git pull` but also 
> after local commits). Neither's great.
> 
> Do you expected that sysroots will ship 3-4 LTO folders, to support ~2 years 
> of clang releases? Or do you expect that a sysroot will always support a 
> single clang/llvm revision? If so, maybe the LLVM version is enough?
Yes, I think we can switch from the VCS string to the LLVM_VERSION string. The 
documentation says LLVM guarantees bitcode backwards compatibility between 
minor versions, so this should be sufficient. I'll post a new patch for that, 
which should also fix the test when HAVE_VCS_VERSION_INC is false.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70677: [WebAssembly] Change the llvm-lto dir to use the LLVM Version

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
sunfish added reviewers: sbc100, dschuff.
Herald added subscribers: cfe-commits, dexonsmith, aheejin, jgravelle-google, 
inglorion.
Herald added a project: clang.

Using the version instead of the VCS revision, which isn't available when 
LLVM_APPEND_VC_REV is set. The bitcode format should be backwards-compatible at 
least within a minor version, so the version string should be sufficient.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70677

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


Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -119,14 +119,11 @@
 }
 
 /// Given a base library directory, append path components to form the
-/// LTO directory. The LLVMRevision allows the path to be keyed to the
-/// specific version of LLVM in used, as the bitcode format is not stable.
-static std::string AppendLTOLibDir(const std::string &Dir,
-   const std::string &LLVMRevision) {
-if (LLVMRevision.empty()) {
-return Dir;
-}
-return Dir + "/llvm-lto/" + LLVMRevision;
+/// LTO directory.
+static std::string AppendLTOLibDir(const std::string &Dir) {
+// The version allows the path to be keyed to the specific version of
+// LLVM in used, as the bitcode format is not stable.
+return Dir + "/llvm-lto/" LLVM_VERSION_STRING;
 }
 
 WebAssembly::WebAssembly(const Driver &D, const llvm::Triple &Triple,
@@ -148,15 +145,11 @@
 getMultiarchTriple(getDriver(), Triple, getDriver().SysRoot);
 auto Sysroot = getDriver().SysRoot;
 if (D.isUsingLTO()) {
-  auto LLVMRevision = getLLVMRevision();
-  if (!LLVMRevision.empty()) {
-// For LTO, enable use of lto-enabled sysroot libraries too, if 
available.
-// Note that the directory is keyed to the LLVM revision, as LLVM's
-// bitcode format is not stable.
-auto Dir = AppendLTOLibDir(Sysroot + "/lib/" + MultiarchTriple,
-   LLVMRevision);
-getFilePaths().push_back(Dir);
-  }
+  // For LTO, enable use of lto-enabled sysroot libraries too, if 
available.
+  // Note that the directory is keyed to the LLVM revision, as LLVM's
+  // bitcode format is not stable.
+  auto Dir = AppendLTOLibDir(Sysroot + "/lib/" + MultiarchTriple);
+  getFilePaths().push_back(Dir);
 }
 getFilePaths().push_back(Sysroot + "/lib/" + MultiarchTriple);
   }
@@ -166,12 +159,9 @@
   // If we're doing LTO and we have an LTO library available, use it.
   const auto &D = getDriver();
   if (D.isUsingLTO()) {
-auto LLVMRevision = getLLVMRevision();
-if (!LLVMRevision.empty()) {
-  auto Dir = AppendLTOLibDir(ToolChain::getCompilerRTPath(), LLVMRevision);
-  if (llvm::sys::fs::exists(Dir))
-return Dir;
-}
+auto Dir = AppendLTOLibDir(ToolChain::getCompilerRTPath());
+if (llvm::sys::fs::exists(Dir))
+  return Dir;
   }
 
   // Otherwise just use the default.


Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -119,14 +119,11 @@
 }
 
 /// Given a base library directory, append path components to form the
-/// LTO directory. The LLVMRevision allows the path to be keyed to the
-/// specific version of LLVM in used, as the bitcode format is not stable.
-static std::string AppendLTOLibDir(const std::string &Dir,
-   const std::string &LLVMRevision) {
-if (LLVMRevision.empty()) {
-return Dir;
-}
-return Dir + "/llvm-lto/" + LLVMRevision;
+/// LTO directory.
+static std::string AppendLTOLibDir(const std::string &Dir) {
+// The version allows the path to be keyed to the specific version of
+// LLVM in used, as the bitcode format is not stable.
+return Dir + "/llvm-lto/" LLVM_VERSION_STRING;
 }
 
 WebAssembly::WebAssembly(const Driver &D, const llvm::Triple &Triple,
@@ -148,15 +145,11 @@
 getMultiarchTriple(getDriver(), Triple, getDriver().SysRoot);
 auto Sysroot = getDriver().SysRoot;
 if (D.isUsingLTO()) {
-  auto LLVMRevision = getLLVMRevision();
-  if (!LLVMRevision.empty()) {
-// For LTO, enable use of lto-enabled sysroot libraries too, if available.
-// Note that the directory is keyed to the LLVM revision, as LLVM's
-// bitcode format is not stable.
-auto Dir = AppendLTOLibDir(Sysroot + "/lib/" + MultiarchTriple,
-   LLVMRevision);
-getFilePaths().push_back(Dir);
-  }
+  // For LTO, enable use of lto-enabled sysroot libraries too, if available.
+  // Note that the directory is keyed to the LLVM revision, as LLVM's
+  // bitcode format is 

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

I've now posted https://reviews.llvm.org/D70677 which should fix the test 
failure when `LLVM_APPEND_VC_REV=NO` is set.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70549: [OPENMP]Fix PR41826: symbols visibility in device code.

[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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70549



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

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

I still have to try out the patch.

In D61446#1756211 , @serge-sans-paille 
wrote:

> @Meinersbur to make your reviewer job easier, I've setup validation for that 
> patch, see https://github.com/serge-sans-paille/llvm-project/pull/2/checks
>  It build and validates fine.


Thanks!

[suggestion] Also add `-DPOLLY_ENABLE_GPGPU_CODEGEN=ON` and 
`-DLLVM_ENABLE_ASSERTIONS=ON`, and run `ninja check-polly`. It currently does 
not run any polly code, just compiles it.




Comment at: llvm/docs/WritingAnLLVMPass.rst:1200
+
+- ``LLVM_${NAME}_LINK_INTO_TOOLS``, when sets to ``ON``, turns the project into
+  a statically linked extension

[typo] when set~~s~~ to



Comment at: llvm/test/Feature/load_extension.ll:1-2
+; RUN: opt %s %loadbye -goodbye -wave-goodbye -disable-output > %t.log
+; RUN: FileCheck %s < %t.log
+; REQUIRES: plugins, examples

[suggestion] The established pattern is to pipe the output to FileCheck without 
using a temporary file.



Comment at: llvm/tools/bugpoint/bugpoint.cpp:230-232
+// Needed to pull in symbols from statically linked extensions, including 
static
+// registration. It is unused otherwise because bugpoint has no support for
+// NewPM.

Nice!



Comment at: polly/include/polly/RegisterPasses.h:26
 void initializePollyPasses(llvm::PassRegistry &Registry);
-void registerPollyPasses(llvm::legacy::PassManagerBase &PM);
+void RegisterPollyPasses(llvm::PassBuilder &PB);
 } // namespace polly

[style] [[ 
https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
 | Start function names with lower case letters ]]



Comment at: polly/lib/Plugin/Polly.cpp:17-30
+/// Initialize Polly passes when library is loaded.
+///
+/// We use the constructor of a statically declared object to initialize the
+/// different Polly passes right after the Polly library is loaded. This 
ensures
+/// that the Polly passes are available e.g. in the 'opt' tool.
+class StaticInitializer {
+public:

[serious] Since `opt`, ... etc. don't call `initializePollyPasses` directly 
anymore, I think the static initializer has to moved to `RegisterPasses.cpp`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert updated this revision to Diff 230928.
jdoerfert marked 4 inline comments as done.
jdoerfert added a comment.

use streams, remove caching


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69785

Files:
  llvm/include/llvm/Frontend/OpenMPConstants.h
  llvm/include/llvm/Frontend/OpenMPIRBuilder.h
  llvm/include/llvm/Frontend/OpenMPKinds.def
  llvm/lib/Frontend/CMakeLists.txt
  llvm/lib/Frontend/OpenMPConstants.cpp
  llvm/lib/Frontend/OpenMPIRBuilder.cpp
  llvm/unittests/CMakeLists.txt
  llvm/unittests/Frontend/CMakeLists.txt
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- /dev/null
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -0,0 +1,178 @@
+//===- llvm/unittest/IR/OpenMPIRBuilderTest.cpp - OpenMPIRBuilder tests ---===//
+//
+// 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 "llvm/Frontend/OpenMPIRBuilder.h"
+#include "llvm/IR/BasicBlock.h"
+#include "llvm/IR/DIBuilder.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Module.h"
+#include "llvm/Frontend/OpenMPConstants.h"
+#include "llvm/IR/Verifier.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace omp;
+using namespace types;
+
+namespace {
+
+class OpenMPIRBuilderTest : public testing::Test {
+protected:
+  void SetUp() override {
+M.reset(new Module("MyModule", Ctx));
+FunctionType *FTy =
+FunctionType::get(Type::getVoidTy(Ctx), {Type::getInt32Ty(Ctx)},
+  /*isVarArg=*/false);
+F = Function::Create(FTy, Function::ExternalLinkage, "", M.get());
+BB = BasicBlock::Create(Ctx, "", F);
+
+DIBuilder DIB(*M);
+auto File = DIB.createFile("test.dbg", "/");
+auto CU =
+DIB.createCompileUnit(dwarf::DW_LANG_C, File, "llvm-C", true, "", 0);
+auto Type = DIB.createSubroutineType(DIB.getOrCreateTypeArray(None));
+auto SP = DIB.createFunction(
+CU, "foo", "", File, 1, Type, 1, DINode::FlagZero,
+DISubprogram::SPFlagDefinition | DISubprogram::SPFlagOptimized);
+F->setSubprogram(SP);
+auto Scope = DIB.createLexicalBlockFile(SP, File, 0);
+DIB.finalize();
+DL = DebugLoc::get(3, 7, Scope);
+  }
+
+  void TearDown() override {
+BB = nullptr;
+M.reset();
+uninitializeTypes();
+  }
+
+  LLVMContext Ctx;
+  std::unique_ptr M;
+  Function *F;
+  BasicBlock *BB;
+  DebugLoc DL;
+};
+
+TEST_F(OpenMPIRBuilderTest, CreateBarrier) {
+  OpenMPIRBuilder OMPBuilder(*M);
+  OMPBuilder.initialize();
+
+  IRBuilder<> Builder(BB);
+
+  OMPBuilder.CreateBarrier({IRBuilder<>::InsertPoint()}, OMPD_for);
+  EXPECT_TRUE(M->global_empty());
+  EXPECT_EQ(M->size(), 1U);
+  EXPECT_EQ(F->size(), 1U);
+  EXPECT_EQ(BB->size(), 0U);
+
+  OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP()});
+  OMPBuilder.CreateBarrier(Loc, OMPD_for);
+  EXPECT_FALSE(M->global_empty());
+  EXPECT_EQ(M->size(), 3U);
+  EXPECT_EQ(F->size(), 1U);
+  EXPECT_EQ(BB->size(), 2U);
+
+  CallInst *GTID = dyn_cast(&BB->front());
+  EXPECT_NE(GTID, nullptr);
+  EXPECT_EQ(GTID->getNumArgOperands(), 1U);
+  EXPECT_EQ(GTID->getCalledFunction()->getName(), "__kmpc_global_thread_num");
+  EXPECT_FALSE(GTID->getCalledFunction()->doesNotAccessMemory());
+  EXPECT_FALSE(GTID->getCalledFunction()->doesNotFreeMemory());
+
+  CallInst *Barrier = dyn_cast(GTID->getNextNode());
+  EXPECT_NE(Barrier, nullptr);
+  EXPECT_EQ(Barrier->getNumArgOperands(), 2U);
+  EXPECT_EQ(Barrier->getCalledFunction()->getName(), "__kmpc_barrier");
+  EXPECT_FALSE(Barrier->getCalledFunction()->doesNotAccessMemory());
+  EXPECT_FALSE(Barrier->getCalledFunction()->doesNotFreeMemory());
+
+  EXPECT_EQ(cast(Barrier)->getArgOperand(1), GTID);
+
+  Builder.CreateUnreachable();
+  EXPECT_FALSE(verifyModule(*M));
+}
+
+TEST_F(OpenMPIRBuilderTest, CreateCancelBarrier) {
+  OpenMPIRBuilder OMPBuilder(*M);
+  OMPBuilder.initialize();
+
+  BasicBlock *CBB = BasicBlock::Create(Ctx, "", F);
+  new UnreachableInst(Ctx, CBB);
+  OMPBuilder.setCancellationBlock(CBB);
+
+  IRBuilder<> Builder(BB);
+
+  OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP()});
+  auto NewIP = OMPBuilder.CreateBarrier(Loc, OMPD_for);
+  Builder.restoreIP(NewIP);
+  EXPECT_FALSE(M->global_empty());
+  EXPECT_EQ(M->size(), 3U);
+  EXPECT_EQ(F->size(), 3U);
+  EXPECT_EQ(BB->size(), 4U);
+
+  CallInst *GTID = dyn_cast(&BB->front());
+  EXPECT_NE(GTID, nullptr);
+  EXPECT_EQ(GTID->getNumArgOperands(), 1U);
+  EXPECT_EQ(GTID->getCalledFunction()->getName(), "__kmpc_global_thread_num");
+  EXPECT_FALSE(GTID->getCalledFunction()->doesNotAccessMemory(

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

@ABataev  @JonChesterfield  anything else blocking this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69785



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

[clang] 872a53e - [WebAssembly] Change the llvm-lto dir to use the LLVM Version

[Dan Gohman via cfe-commits]

Author: Dan Gohman
Date: 2019-11-25T10:29:51-08:00
New Revision: 872a53ef9489fcfbb48c6f8dd30bd9a9a026934f

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

LOG: [WebAssembly] Change the llvm-lto dir to use the LLVM Version

Using the version instead of the VCS revision, which isn't available
when LLVM_APPEND_VC_REV is set. The bitcode format should be
backwards-compatible at least within a minor version, so the version
string should be sufficient.

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/WebAssembly.cpp 
b/clang/lib/Driver/ToolChains/WebAssembly.cpp
index a2a9dff79e52..55b82592c09f 100644
--- a/clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ b/clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -118,6 +118,14 @@ void wasm::Linker::ConstructJob(Compilation &C, const 
JobAction &JA,
   }
 }
 
+/// Given a base library directory, append path components to form the
+/// LTO directory.
+static std::string AppendLTOLibDir(const std::string &Dir) {
+// The version allows the path to be keyed to the specific version of
+// LLVM in used, as the bitcode format is not stable.
+return Dir + "/llvm-lto/" LLVM_VERSION_STRING;
+}
+
 WebAssembly::WebAssembly(const Driver &D, const llvm::Triple &Triple,
  const llvm::opt::ArgList &Args)
 : ToolChain(D, Triple, Args) {
@@ -126,26 +134,24 @@ WebAssembly::WebAssembly(const Driver &D, const 
llvm::Triple &Triple,
 
   getProgramPaths().push_back(getDriver().getInstalledDir());
 
+  auto SysRoot = getDriver().SysRoot;
   if (getTriple().getOS() == llvm::Triple::UnknownOS) {
 // Theoretically an "unknown" OS should mean no standard libraries, however
 // it could also mean that a custom set of libraries is in use, so just add
 // /lib to the search path. Disable multiarch in this case, to discourage
 // paths containing "unknown" from acquiring meanings.
-getFilePaths().push_back(getDriver().SysRoot + "/lib");
+getFilePaths().push_back(SysRoot + "/lib");
   } else {
 const std::string MultiarchTriple =
-getMultiarchTriple(getDriver(), Triple, getDriver().SysRoot);
+getMultiarchTriple(getDriver(), Triple, SysRoot);
 if (D.isUsingLTO()) {
-  auto LLVMRevision = getLLVMRevision();
-  if (!LLVMRevision.empty()) {
-// For LTO, enable use of lto-enabled sysroot libraries too, if 
available.
-// Note that the directory is keyed to the LLVM revision, as LLVM's
-// bitcode format is not stable.
-getFilePaths().push_back(getDriver().SysRoot + "/lib/" + 
MultiarchTriple +
- "/llvm-lto/" + LLVMRevision);
-  }
+  // For LTO, enable use of lto-enabled sysroot libraries too, if 
available.
+  // Note that the directory is keyed to the LLVM revision, as LLVM's
+  // bitcode format is not stable.
+  auto Dir = AppendLTOLibDir(SysRoot + "/lib/" + MultiarchTriple);
+  getFilePaths().push_back(Dir);
 }
-getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple);
+getFilePaths().push_back(SysRoot + "/lib/" + MultiarchTriple);
   }
 }
 



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

[PATCH] D69628: [Clang] Pragma vectorize_width() implies vectorize(enable), take 3

[Michael Kruse via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D69628



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

[PATCH] D70677: [WebAssembly] Change the llvm-lto dir to use the LLVM Version

[sunfishcode via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG872a53ef9489: [WebAssembly] Change the llvm-lto dir to use 
the LLVM Version (authored by sunfishcode).

Changed prior to commit:
  https://reviews.llvm.org/D70677?vs=230919&id=230934#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70677

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


Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -118,6 +118,14 @@
   }
 }
 
+/// Given a base library directory, append path components to form the
+/// LTO directory.
+static std::string AppendLTOLibDir(const std::string &Dir) {
+// The version allows the path to be keyed to the specific version of
+// LLVM in used, as the bitcode format is not stable.
+return Dir + "/llvm-lto/" LLVM_VERSION_STRING;
+}
+
 WebAssembly::WebAssembly(const Driver &D, const llvm::Triple &Triple,
  const llvm::opt::ArgList &Args)
 : ToolChain(D, Triple, Args) {
@@ -126,26 +134,24 @@
 
   getProgramPaths().push_back(getDriver().getInstalledDir());
 
+  auto SysRoot = getDriver().SysRoot;
   if (getTriple().getOS() == llvm::Triple::UnknownOS) {
 // Theoretically an "unknown" OS should mean no standard libraries, however
 // it could also mean that a custom set of libraries is in use, so just add
 // /lib to the search path. Disable multiarch in this case, to discourage
 // paths containing "unknown" from acquiring meanings.
-getFilePaths().push_back(getDriver().SysRoot + "/lib");
+getFilePaths().push_back(SysRoot + "/lib");
   } else {
 const std::string MultiarchTriple =
-getMultiarchTriple(getDriver(), Triple, getDriver().SysRoot);
+getMultiarchTriple(getDriver(), Triple, SysRoot);
 if (D.isUsingLTO()) {
-  auto LLVMRevision = getLLVMRevision();
-  if (!LLVMRevision.empty()) {
-// For LTO, enable use of lto-enabled sysroot libraries too, if 
available.
-// Note that the directory is keyed to the LLVM revision, as LLVM's
-// bitcode format is not stable.
-getFilePaths().push_back(getDriver().SysRoot + "/lib/" + 
MultiarchTriple +
- "/llvm-lto/" + LLVMRevision);
-  }
+  // For LTO, enable use of lto-enabled sysroot libraries too, if 
available.
+  // Note that the directory is keyed to the LLVM revision, as LLVM's
+  // bitcode format is not stable.
+  auto Dir = AppendLTOLibDir(SysRoot + "/lib/" + MultiarchTriple);
+  getFilePaths().push_back(Dir);
 }
-getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple);
+getFilePaths().push_back(SysRoot + "/lib/" + MultiarchTriple);
   }
 }
 


Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -118,6 +118,14 @@
   }
 }
 
+/// Given a base library directory, append path components to form the
+/// LTO directory.
+static std::string AppendLTOLibDir(const std::string &Dir) {
+// The version allows the path to be keyed to the specific version of
+// LLVM in used, as the bitcode format is not stable.
+return Dir + "/llvm-lto/" LLVM_VERSION_STRING;
+}
+
 WebAssembly::WebAssembly(const Driver &D, const llvm::Triple &Triple,
  const llvm::opt::ArgList &Args)
 : ToolChain(D, Triple, Args) {
@@ -126,26 +134,24 @@
 
   getProgramPaths().push_back(getDriver().getInstalledDir());
 
+  auto SysRoot = getDriver().SysRoot;
   if (getTriple().getOS() == llvm::Triple::UnknownOS) {
 // Theoretically an "unknown" OS should mean no standard libraries, however
 // it could also mean that a custom set of libraries is in use, so just add
 // /lib to the search path. Disable multiarch in this case, to discourage
 // paths containing "unknown" from acquiring meanings.
-getFilePaths().push_back(getDriver().SysRoot + "/lib");
+getFilePaths().push_back(SysRoot + "/lib");
   } else {
 const std::string MultiarchTriple =
-getMultiarchTriple(getDriver(), Triple, getDriver().SysRoot);
+getMultiarchTriple(getDriver(), Triple, SysRoot);
 if (D.isUsingLTO()) {
-  auto LLVMRevision = getLLVMRevision();
-  if (!LLVMRevision.empty()) {
-// For LTO, enable use of lto-enabled sysroot libraries too, if available.
-// Note that the directory is keyed to the LLVM revision, as LLVM's
-// bitcode format is not stable.
-getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple +
- "/llvm-lto/" + LLVMRevision);
-  }
+  // For LTO, enable use of lto-en

[PATCH] D70684: [clangd] Shutdown cleanly on signals.

[Sam McCall via Phabricator via cfe-commits]

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

This avoids leaking PCH files if editors don't use the LSP shutdown protocol.

This is one fix for https://github.com/clangd/clangd/issues/209
(Though I think we should *also* be unlinking the files)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70684

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/JSONTransport.cpp
  clang-tools-extra/clangd/Shutdown.cpp
  clang-tools-extra/clangd/Shutdown.h
  clang-tools-extra/clangd/test/exit-eof.test
  clang-tools-extra/clangd/test/exit-signal.test
  clang-tools-extra/clangd/tool/ClangdMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -11,6 +11,7 @@
 #include "Features.inc"
 #include "Path.h"
 #include "Protocol.h"
+#include "Shutdown.h"
 #include "Trace.h"
 #include "Transport.h"
 #include "index/Background.h"
@@ -35,6 +36,10 @@
 #include 
 #include 
 
+#ifndef _WIN32
+#include 
+#endif
+
 namespace clang {
 namespace clangd {
 namespace {
@@ -435,6 +440,7 @@
 
   llvm::InitializeAllTargetInfos();
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
+  llvm::sys::SetInterruptFunction(&requestShutdown);
   llvm::cl::SetVersionPrinter([](llvm::raw_ostream &OS) {
 OS << clang::getClangToolFullVersion("clangd") << "\n";
   });
@@ -531,6 +537,10 @@
   LoggingSession LoggingSession(Logger);
   // Write some initial logs before we start doing any real work.
   log("{0}", clang::getClangToolFullVersion("clangd"));
+// FIXME: abstract this better, and print PID on windows too.
+#ifndef _WIN32
+  log("PID: {0}", getpid());
+#endif
   {
 SmallString<128> CWD;
 if (auto Err = llvm::sys::fs::current_path(CWD))
@@ -683,12 +693,7 @@
   // However if a bug causes them to run forever, we want to ensure the process
   // eventually exits. As clangd isn't directly user-facing, an editor can
   // "leak" clangd processes. Crashing in this case contains the damage.
-  //
-  // This is more portable than sys::WatchDog, and yields a stack trace.
-  std::thread([] {
-std::this_thread::sleep_for(std::chrono::minutes(5));
-std::abort();
-  }).detach();
+  abortAfterTimeout(std::chrono::minutes(5));
 
   return ExitCode;
 }
Index: clang-tools-extra/clangd/test/exit-signal.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/exit-signal.test
@@ -0,0 +1,31 @@
+# This is a fiddly signal test, we need POSIX and a real shell.
+UNSUPPORTED: win32
+REQUIRES: shell
+
+# Our goal is:
+#  1) spawn clangd
+#  2) wait for it to start up (install signal handlers)
+#  3) send SIGTERM
+#  4) wait for clangd to shut down (nonzero exit for a signal)
+#  4) verify the shutdown was clean
+
+RUN: rm %t.err
+ # To keep clangd running, we need to hold its input stream open.
+ # We redirect its input to a subshell that waits for it to start up.
+RUN: not clangd 2> %t.err < <( \
+   # Loop waiting for clangd to start up, so signal handlers are installed.
+   # Reading the PID line ensures this, and lets us send a signal.
+RUN:   while true; do \
+ # Relevant log line is I[timestamp] PID: 
+RUN: CLANGD_PID=$(grep -a -m 1 "PID:" %t.err | cut -d' ' -f 3); \
+RUN: [ ! -z "$CLANGD_PID" ] && break; \
+RUN:   done; \
+RUN:   kill $CLANGD_PID \
+ # Now wait for clangd to exit.
+RUN: )
+
+# Check that clangd caught the signal and shut down cleanly.
+RUN: FileCheck %s < %t.err
+CHECK: Transport error: Got signal
+CHECK: LSP finished
+
Index: clang-tools-extra/clangd/test/exit-eof.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/exit-eof.test
@@ -0,0 +1,7 @@
+# RUN: not clangd -sync < %s 2> %t.err
+# RUN: FileCheck %s < %t.err
+#
+# No LSP messages here, just let clangd see the end-of-file
+# CHECK: Transport error:
+# (Typically "Transport error: Input/output error" but platform-dependent).
+
Index: clang-tools-extra/clangd/Shutdown.h
===
--- /dev/null
+++ clang-tools-extra/clangd/Shutdown.h
@@ -0,0 +1,84 @@
+//===--- Shutdown.h - Unclean exit scenarios *- 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
+//
+//===--===//
+//
+// LSP specifies a protocol for shutting down: a `shutdown` request followed
+// by an `exit` notification. If this protocol is fol

[PATCH] D70687: [WebAssembly] Add an llvm-lto path for compiler-rt.

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
Herald added subscribers: cfe-commits, dexonsmith, aheejin, jgravelle-google, 
inglorion, sbc100, mehdi_amini, dberris, dschuff.
Herald added a project: clang.

This allows LTO-enabled builds of compiler-rt to be used.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70687

Files:
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/lib/Driver/ToolChains/WebAssembly.h


Index: clang/lib/Driver/ToolChains/WebAssembly.h
===
--- clang/lib/Driver/ToolChains/WebAssembly.h
+++ clang/lib/Driver/ToolChains/WebAssembly.h
@@ -71,6 +71,7 @@
   const char *getDefaultLinker() const override { return "wasm-ld"; }
 
   Tool *buildLinker() const override;
+  std::string getCompilerRTPath() const;
 };
 
 } // end namespace toolchains
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -155,6 +155,19 @@
   }
 }
 
+std::string WebAssembly::getCompilerRTPath() const {
+  // If we're doing LTO and we have an LTO library available, use it.
+  const auto &D = getDriver();
+  if (D.isUsingLTO()) {
+auto Dir = AppendLTOLibDir(ToolChain::getCompilerRTPath());
+if (llvm::sys::fs::exists(Dir))
+  return Dir;
+  }
+
+  // Otherwise just use the default.
+  return ToolChain::getCompilerRTPath();
+}
+
 bool WebAssembly::IsMathErrnoDefault() const { return false; }
 
 bool WebAssembly::IsObjCNonFragileABIDefault() const { return true; }


Index: clang/lib/Driver/ToolChains/WebAssembly.h
===
--- clang/lib/Driver/ToolChains/WebAssembly.h
+++ clang/lib/Driver/ToolChains/WebAssembly.h
@@ -71,6 +71,7 @@
   const char *getDefaultLinker() const override { return "wasm-ld"; }
 
   Tool *buildLinker() const override;
+  std::string getCompilerRTPath() const;
 };
 
 } // end namespace toolchains
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -155,6 +155,19 @@
   }
 }
 
+std::string WebAssembly::getCompilerRTPath() const {
+  // If we're doing LTO and we have an LTO library available, use it.
+  const auto &D = getDriver();
+  if (D.isUsingLTO()) {
+auto Dir = AppendLTOLibDir(ToolChain::getCompilerRTPath());
+if (llvm::sys::fs::exists(Dir))
+  return Dir;
+  }
+
+  // Otherwise just use the default.
+  return ToolChain::getCompilerRTPath();
+}
+
 bool WebAssembly::IsMathErrnoDefault() const { return false; }
 
 bool WebAssembly::IsObjCNonFragileABIDefault() const { return true; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70684: [clangd] Shutdown cleanly on signals.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.
Herald added a subscriber: dexonsmith.

Build result: fail - 60239 tests passed, 1 failed and 732 were skipped.

  failed: Clangd.Clangd/exit-signal.test

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70684



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

[PATCH] D69938: [OpenCL] Use __generic addr space when generating internal representation of lambda

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D69938#1755709 , @rjmccall wrote:

> In D69938#1754894 , @Anastasia wrote:
>
> > In D69938#1752024 , @rjmccall 
> > wrote:
> >
> > > Yes, in that case copy-elision into the global variable is guaranteed.  
> > > You can write arbitrary expressions in global initializers, however, and 
> > > those can use temporary lambdas.
> >
> >
> > I guess you mean something like this?
> >
> >   auto glambda = []() { return 1; }();
> >   
>
>
> Yes, or just passing it as an argument to something.
>
> > I don't see a way to change the address space of a lambda object however. 
> > It would only be possible to specify addr space qualifiers for a call 
> > operator of such lambdas.
>
> Yes, I think the language has to say what address space the lambda temporary 
> is in.  Among other things, this can affect how captures are initialized, 
> since some constructors are only usable in certain address spaces.  (Lambdas 
> in global contexts can't capture surrounding local variables, but they can 
> still have explicitly-initialized captures, like `[x=foo()] { ... }`.)  That 
> language rule could be that lambda temporaries are always in the private 
> address space, or it could be that lambdas in global contexts are in the 
> global address space.  The latter might be easier because it's compatible 
> with lifetime-extension: we don't necessarily know when processing the lambda 
> whether its temporary will be lifetime-extended, and if it is, it needs to be 
> in the global address space.  The only disadvantage of that is that we'd have 
> to use global memory even for non-extended temporaries in global initializers.
>
> Richard might have thoughts about this.  I don't know if there's been any 
> language work around copy-elision and/or lifetime-extension that might be 
> stretchable to allow us to delay the address-space decision until later.


Ok, I see. Thanks! I agree for OpenCL `__global` might make more sense for the 
program scope lambdas. I will look into this. However, would it be ok to 
proceed with this patch adding `__generic` address space as a qualifier to 
lambda call operators by default. Is there anything that should be added to 
complete it?


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

https://reviews.llvm.org/D69938



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

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 230948.
sammccall marked 10 inline comments as done.
sammccall added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70512

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/Selection.h
  clang-tools-extra/clangd/unittests/SelectionTests.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp
  clang/include/clang/Tooling/Syntax/Tokens.h
  clang/lib/Tooling/Syntax/Tokens.cpp
  clang/unittests/Tooling/Syntax/TokensTest.cpp

Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -40,6 +40,7 @@
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Testing/Support/Annotations.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
+#include "gmock/gmock.h"
 #include 
 #include 
 #include 
@@ -663,6 +664,20 @@
   ValueIs(SameRange(findSpelled("not_mapped";
 }
 
+TEST_F(TokenBufferTest, ExpandedTokensForRange) {
+  recordTokens(R"cpp(
+#define SIGN(X) X##_washere
+A SIGN(B) C SIGN(D) E SIGN(F) G
+  )cpp");
+
+  SourceRange R(findExpanded("C").front().location(),
+findExpanded("F_washere").front().location());
+  // Sanity check: expanded and spelled tokens are stored separately.
+  EXPECT_THAT(Buffer.expandedTokens(R),
+  SameRange(findExpanded("C D_washere E F_washere")));
+  EXPECT_THAT(Buffer.expandedTokens(SourceRange()), testing::IsEmpty());
+}
+
 TEST_F(TokenBufferTest, ExpansionStartingAt) {
   // Object-like macro expansions.
   recordTokens(R"cpp(
Index: clang/lib/Tooling/Syntax/Tokens.cpp
===
--- clang/lib/Tooling/Syntax/Tokens.cpp
+++ clang/lib/Tooling/Syntax/Tokens.cpp
@@ -119,6 +119,22 @@
   return Text.substr(Begin, length());
 }
 
+llvm::ArrayRef TokenBuffer::expandedTokens(SourceRange R) const {
+  if (R.isInvalid())
+return {};
+  const Token *Begin =
+  llvm::partition_point(expandedTokens(), [&](const syntax::Token &T) {
+return SourceMgr->isBeforeInTranslationUnit(T.location(), R.getBegin());
+  });
+  const Token *End =
+  llvm::partition_point(expandedTokens(), [&](const syntax::Token &T) {
+return !SourceMgr->isBeforeInTranslationUnit(R.getEnd(), T.location());
+  });
+  if (Begin > End)
+return {};
+  return {Begin, End};
+}
+
 std::pair
 TokenBuffer::spelledForExpandedToken(const syntax::Token *Expanded) const {
   assert(Expanded);
Index: clang/include/clang/Tooling/Syntax/Tokens.h
===
--- clang/include/clang/Tooling/Syntax/Tokens.h
+++ clang/include/clang/Tooling/Syntax/Tokens.h
@@ -175,6 +175,7 @@
   /// All tokens produced by the preprocessor after all macro replacements,
   /// directives, etc. Source locations found in the clang AST will always
   /// point to one of these tokens.
+  /// Tokens are in TU order (per SourceManager::isBeforeInTranslationUnit()).
   /// FIXME: figure out how to handle token splitting, e.g. '>>' can be split
   ///into two '>' tokens by the parser. However, TokenBuffer currently
   ///keeps it as a single '>>' token.
@@ -182,6 +183,10 @@
 return ExpandedTokens;
   }
 
+  /// Returns the subrange of expandedTokens() corresponding to the closed
+  /// token range R.
+  llvm::ArrayRef expandedTokens(SourceRange R) const;
+
   /// Find the subrange of spelled tokens that produced the corresponding \p
   /// Expanded tokens.
   ///
Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -269,7 +269,7 @@
   EXPECT_UNAVAILABLE(UnavailableCases);
 
   // vector of pairs of input and output strings
-  const std::vector>
+  const std::vector>
   InputOutputs = {
   // extraction from variable declaration/assignment
   {R"cpp(void varDecl() {
@@ -321,17 +321,10 @@
if(1)
 LOOP(5 + [[3]])
  })cpp",
-   /*FIXME: It should be extracted like this. SelectionTree needs to be
- * fixed for macros.
 R"cpp(#define LOOP(x) while (1) {a = x;}
-void f(int a) {
-  auto dummy = 3; if(1)
-   LOOP(5 + dummy)
-})cpp"},*/
-   R"cpp(#define LOOP(x) while (1) {a = x;}
  void f(int a) {
-   auto dummy = LOOP(5 + 3); if(1)
-dummy
+   auto dummy = 3; if(1)
+LOOP(5 + dummy)
  })cpp"},
   {R"cpp(#define LOOP(x) do {x;} while(1);
   

[PATCH] D70689: [analyzer] Fix SARIF column locations

[Joe Ranieri via Phabricator via cfe-commits]

jranieri-grammatech created this revision.
jranieri-grammatech added a reviewer: aaron.ballman.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

This fixes SARIF column locations to be end-exclusive and be counted in terms 
of Unicode characters.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70689

Files:
  clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
  
clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
  
clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
  clang/test/Analysis/diagnostics/sarif-multi-diagnostic-test.c

Index: clang/test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
===
--- clang/test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
+++ clang/test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
@@ -30,11 +30,18 @@
   return 0;
 }
 
+int unicode()
+{
+  int løçål = 0;
+  /* ☃ */ return 1 / løçål; // expected-warning {{Division by zero}}
+}
+
 int main(void) {
   f();
   g();
   h(0);
   leak(0);
+  unicode();
   return 0;
 }
 
Index: clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
===
--- clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
+++ clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
@@ -4,7 +4,7 @@
 {
   "artifacts": [
 {
-  "length": 951,
+  "length": 1077,
   "location": {
   },
   "mimeType": "text/plain",
@@ -13,6 +13,7 @@
   ]
 }
   ],
+  "columnKind": "unicodeCodePoints",
   "results": [
 {
   "codeFlows": [
@@ -32,9 +33,9 @@
   },
   "region": {
 "endColumn": 6,
-"endLine": 34,
+"endLine": 40,
 "startColumn": 3,
-"startLine": 34
+"startLine": 40
   }
 }
   }
@@ -102,9 +103,9 @@
   },
   "region": {
 "endColumn": 6,
-"endLine": 35,
+"endLine": 41,
 "startColumn": 3,
-"startLine": 35
+"startLine": 41
   }
 }
   }
@@ -120,7 +121,7 @@
 "index": 0,
   },
   "region": {
-"endColumn": 11,
+"endColumn": 12,
 "endLine": 15,
 "startColumn": 3,
 "startLine": 15
@@ -363,6 +364,74 @@
   },
   "ruleId": "unix.Malloc",
   "ruleIndex": 3
+},
+{
+  "codeFlows": [
+{
+  "threadFlows": [
+{
+  "locations": [
+{
+  "importance": "essential",
+  "location": {
+"message": {
+  "text": "'løçål' initialized to 0"
+},
+"physicalLocation": {
+  "artifactLocation": {
+"index": 0,
+  },
+  "region": {
+"endColumn": 12,
+"endLine": 35,
+"startColumn": 3,
+"startLine": 35
+  }
+}
+  }
+},
+{
+  "importance": "essential",
+  "location": {
+"message": {
+  "text": "Division by zero"
+},
+"physicalLocation": {
+  "artifactLocation": {
+"index": 0,
+  },
+  "region": {
+"endColumn": 20,
+"startColumn": 20,
+"startLine": 36
+  }
+}
+  }
+}
+  ]
+}
+  ]
+}
+  ],
+  "locations": [
+{
+  "physic

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: fail - 60299 tests passed, 1 failed and 732 were skipped.

  failed: Clangd Unit Tests._/ClangdTests/SelectionTest.PathologicalPreprocessor

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70512



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

[clang] 7b86188 - [Diagnostic] add a warning which warns about misleading indentation

[via cfe-commits]

Author: Tyker
Date: 2019-11-25T20:46:32+01:00
New Revision: 7b86188b50bf6e537fe98b326f258fbd23108b83

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

LOG: [Diagnostic] add a warning which warns about misleading indentation

Summary: Add a warning for misleading indentation similar to GCC's 
-Wmisleading-indentation

Reviewers: aaron.ballman, xbolva00

Reviewed By: aaron.ballman, xbolva00

Subscribers: arphaman, Ka-Ka, thakis

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

Added: 
clang/test/SemaCXX/warn-misleading-indentation.cpp

Modified: 
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Parse/Parser.h
clang/lib/Parse/ParseStmt.cpp
clang/test/Index/pragma-diag-reparse.c
clang/test/Misc/warning-wall.c
clang/test/Preprocessor/pragma_diagnostic_sections.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 5bfb3de86a47..666193e074f0 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -693,6 +693,7 @@ def ZeroLengthArray : DiagGroup<"zero-length-array">;
 def GNUZeroLineDirective : DiagGroup<"gnu-zero-line-directive">;
 def GNUZeroVariadicMacroArguments : 
DiagGroup<"gnu-zero-variadic-macro-arguments">;
 def Fallback : DiagGroup<"fallback">;
+def MisleadingIndentation : DiagGroup<"misleading-indentation">;
 
 // This covers both the deprecated case (in C++98)
 // and the extension case (in C++11 onwards).
@@ -884,7 +885,7 @@ def Consumed   : DiagGroup<"consumed">;
 // Note that putting warnings in -Wall will not disable them by default. If a
 // warning should be active _only_ when -Wall is passed in, mark it as
 // DefaultIgnore in addition to putting it here.
-def All : DiagGroup<"all", [Most, Parentheses, Switch, SwitchBool]>;
+def All : DiagGroup<"all", [Most, Parentheses, Switch, SwitchBool, 
MisleadingIndentation]>;
 
 // Warnings that should be in clang-cl /w4.
 def : DiagGroup<"CL4", [All, Extra]>;

diff  --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index c94d1b99d0e8..e6aa92eddef7 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -61,6 +61,13 @@ def warn_null_statement : Warning<
   "remove unnecessary ';' to silence this warning">,
   InGroup, DefaultIgnore;
 
+def warn_misleading_indentation : Warning<
+  "misleading indentation; %select{statement|declaration}0 is not part of "
+  "the previous '%select{if|else|for|while}1'">,
+  InGroup, DefaultIgnore;
+def note_previous_statement : Note<
+  "previous statement is here">;
+
 def ext_thread_before : Extension<"'__thread' before '%0'">;
 def ext_keyword_as_ident : ExtWarn<
   "keyword '%0' will be made available as an identifier "

diff  --git a/clang/include/clang/Parse/Parser.h 
b/clang/include/clang/Parse/Parser.h
index ea1116ff7a23..edd31e3ff7e8 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -2266,11 +2266,13 @@ class Parser : public CodeCompletionHandler {
 return isTypeSpecifierQualifier();
   }
 
+public:
   /// isCXXDeclarationStatement - C++-specialized function that disambiguates
   /// between a declaration or an expression statement, when parsing function
   /// bodies. Returns true for declaration, false for expression.
   bool isCXXDeclarationStatement();
 
+private:
   /// isCXXSimpleDeclaration - C++-specialized function that disambiguates
   /// between a simple-declaration or an expression-statement.
   /// If during the disambiguation process a parsing error is encountered,

diff  --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index 727ab75adae8..ce8aa7574b9b 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -1191,6 +1191,27 @@ bool Parser::ParseParenExprOrCondition(StmtResult 
*InitStmt,
   return false;
 }
 
+enum MisleadingStatementKind { MSK_if, MSK_else, MSK_for, MSK_while };
+
+static void
+MaybeDiagnoseMisleadingIndentation(Parser &P, SourceLocation PrevLoc,
+   SourceLocation StmtLoc,
+   MisleadingStatementKind StmtKind) {
+  Token Tok = P.getCurToken();
+  if (Tok.is(tok::semi))
+return;
+  SourceManager &SM = P.getPreprocessor().getSourceManager();
+  unsigned PrevColNum = SM.getSpellingColumnNumber(PrevLoc);
+  unsigned CurColNum = SM.getSpellingColumnNumber(Tok.getLocation());
+  unsigned StmtColNum = SM.getSpellingColumnNumber(StmtLoc);
+  if (!Tok.isAtStartOfLine() ||
+  (PrevColNum != 0 && CurColNum != 0 && StmtColNum != 0 &&
+   P

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

[Tyker via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7b86188b50bf: [Diagnostic] add a warning which warns about 
misleading indentation (authored by Tyker).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70638

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseStmt.cpp
  clang/test/Index/pragma-diag-reparse.c
  clang/test/Misc/warning-wall.c
  clang/test/Preprocessor/pragma_diagnostic_sections.cpp
  clang/test/SemaCXX/warn-misleading-indentation.cpp

Index: clang/test/SemaCXX/warn-misleading-indentation.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-misleading-indentation.cpp
@@ -0,0 +1,184 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wmisleading-indentation -DWITH_WARN %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify -Wall -Wno-unused -DWITH_WARN -DCXX17 %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify -Wall -Wno-unused -Wno-misleading-indentation -DCXX17 %s
+
+#ifndef WITH_WARN
+// expected-no-diagnostics
+#endif
+
+void f0(int i) {
+  if (i)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i = i + 1;
+int x = 0;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; declaration is not part of the previous 'if'}}
+#endif
+  return;
+#ifdef CXX17
+  if constexpr (false)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i = 0;
+i += 1;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}}
+#endif
+#endif
+}
+
+void f1(int i) {
+  for (;i;)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i = i + 1;
+i *= 2;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'for'}}
+#endif
+  return;
+}
+
+void f2(int i) {
+  while (i)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i = i + 1; i *= 2;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'while'}}
+#endif
+  return;
+}
+
+void f3(int i) {
+  if (i)
+i = i + 1;
+  else
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i *= 2;
+const int x = 0;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; declaration is not part of the previous 'else'}}
+#endif
+}
+
+#ifdef CXX17
+struct Range {
+  int *begin() {return nullptr;}
+  int *end() {return nullptr;}
+};
+#endif
+
+void f4(int i) {
+  if (i)
+  i *= 2;
+  return;
+  if (i)
+i *= 2;
+;
+  if (i)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i *= 2;
+typedef int Int;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; declaration is not part of the previous 'if'}}
+#endif
+#ifdef CXX17
+  Range R;
+  for (auto e : R)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i *= 2;
+using Int2 = int;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; declaration is not part of the previous 'for'}}
+#endif
+#endif
+}
+
+int bar(void);
+
+int foo(int* dst)
+{   
+if (dst)
+   return
+bar();
+  if (dst)
+dst = dst + \
+bar();
+  return 0;
+}
+
+// No diagnostics from GCC on this
+void g(int i) {
+  if (1)
+i = 2;
+  else
+ if (i == 3)
+i = 4;
+i = 5;
+}
+
+// Or this
+#define TEST i = 5
+void g0(int i) {
+  if (1)
+i = 2;
+  else
+i = 5;
+TEST;
+}
+
+void g1(int i) {
+  if (1)
+i = 2;
+  else
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+if (i == 3)
+i = 4;
+i = 5;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'else'}}
+#endif
+}
+
+void g2(int i) {
+  if (1)
+i = 2;
+  else
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+if (i == 3)
+{i = 4;}
+i = 5;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'else'}}
+#endif
+}
+
+void g6(int i) {
+if (1)
+  if (i == 3)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i = 4;
+i = 5;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}}
+#endif
+}
Index: clang/test/Preprocessor/pragma_diagnostic_sections.cpp
===
--- clang/test/Preprocessor/pragma_diagnostic_sections.cpp
+++ clang/test/Preprocessor/pragma_diagnostic_sections.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -Wall -Wunused-macros -Wunused-parameter -Wno-uninitialized -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wall

[clang] bbc328c - [OPENMP]Fix PR41826: symbols visibility in device code.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2019-11-25T15:01:28-05:00
New Revision: bbc328c62430dd3e2e72973ca85c5c6fb550b227

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

LOG: [OPENMP]Fix PR41826: symbols visibility in device code.

Summary:
Currently, we ignore all locality attributes/info when building for
the device and thus all symblos are externally visible and can be
preemted at the runtime. It may lead to incorrect results. We need to
follow the same logic, compiler uses for static/pie builds. But in some
cases changing of dso locality may lead to problems with codegen, so
instead mark external symbols as hidden instead in the device code.

Reviewers: jdoerfert

Subscribers: guansong, caomhin, kkwli0, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/AST/Decl.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/OpenMP/declare_target_codegen.cpp
clang/test/OpenMP/nvptx_allocate_codegen.cpp
clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp
clang/test/OpenMP/nvptx_target_codegen.cpp
clang/test/OpenMP/nvptx_unsupported_type_codegen.cpp

Removed: 




diff  --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 66446626c2eb..3723c868004f 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -901,6 +901,10 @@ LinkageComputer::getLVForNamespaceScopeDecl(const 
NamedDecl *D,
   if (!isExternallyVisible(LV.getLinkage()))
 return LinkageInfo(LV.getLinkage(), DefaultVisibility, false);
 
+  // Mark the symbols as hidden when compiling for the device.
+  if (Context.getLangOpts().OpenMP && Context.getLangOpts().OpenMPIsDevice)
+LV.mergeVisibility(HiddenVisibility, /*newExplicit=*/false);
+
   return LV;
 }
 

diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 25268efff61b..9793a5ef4729 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -815,7 +815,7 @@ static bool shouldAssumeDSOLocal(const CodeGenModule &CGM,
   const auto &CGOpts = CGM.getCodeGenOpts();
   llvm::Reloc::Model RM = CGOpts.RelocationModel;
   const auto &LOpts = CGM.getLangOpts();
-  if (RM != llvm::Reloc::Static && !LOpts.PIE && !LOpts.OpenMPIsDevice)
+  if (RM != llvm::Reloc::Static && !LOpts.PIE)
 return false;
 
   // A definition cannot be preempted from an executable.

diff  --git a/clang/test/OpenMP/declare_target_codegen.cpp 
b/clang/test/OpenMP/declare_target_codegen.cpp
index f1074ee5b294..165ba097b95e 100644
--- a/clang/test/OpenMP/declare_target_codegen.cpp
+++ b/clang/test/OpenMP/declare_target_codegen.cpp
@@ -27,22 +27,22 @@
 // CHECK-DAG: Bake
 // CHECK-NOT: @{{hhh|ggg|fff|eee}} =
 // CHECK-DAG: @aaa = external global i32,
-// CHECK-DAG: @bbb ={{ dso_local | }}global i32 0,
+// CHECK-DAG: @bbb ={{ hidden | }}global i32 0,
 // CHECK-DAG: weak constant %struct.__tgt_offload_entry { i8* bitcast (i32* 
@bbb to i8*),
 // CHECK-DAG: @ccc = external global i32,
-// CHECK-DAG: @ddd ={{ dso_local | }}global i32 0,
+// CHECK-DAG: @ddd ={{ hidden | }}global i32 0,
 // CHECK-DAG: @hhh_decl_tgt_ref_ptr = weak global i32* null
 // CHECK-DAG: @ggg_decl_tgt_ref_ptr = weak global i32* null
 // CHECK-DAG: @fff_decl_tgt_ref_ptr = weak global i32* null
 // CHECK-DAG: @eee_decl_tgt_ref_ptr = weak global i32* null
 // CHECK-DAG: @{{.*}}maini1{{.*}}aaa = internal global i64 23,
-// CHECK-DAG: @b ={{ dso_local | }}global i32 15,
-// CHECK-DAG: @d ={{ dso_local | }}global i32 0,
+// CHECK-DAG: @b ={{ hidden | }}global i32 15,
+// CHECK-DAG: @d ={{ hidden | }}global i32 0,
 // CHECK-DAG: @c = external global i32,
-// CHECK-DAG: @globals ={{ dso_local | }}global %struct.S zeroinitializer,
+// CHECK-DAG: @globals ={{ hidden | }}global %struct.S zeroinitializer,
 // CHECK-DAG: [[STAT:@.+stat]] = internal global %struct.S zeroinitializer,
 // CHECK-DAG: [[STAT_REF:@.+]] = internal constant %struct.S* [[STAT]]
-// CHECK-DAG: @out_decl_target ={{ dso_local | }}global i32 0,
+// CHECK-DAG: @out_decl_target ={{ hidden | }}global i32 0,
 // CHECK-DAG: @llvm.used = appending global [2 x i8*] [i8* bitcast (void ()* 
@__omp_offloading__{{.+}}_globals_l[[@LINE+84]]_ctor to i8*), i8* bitcast (void 
()* @__omp_offloading__{{.+}}_stat_l[[@LINE+85]]_ctor to i8*)],
 // CHECK-DAG: @llvm.compiler.used = appending global [1 x i8*] [i8* bitcast 
(%struct.S** [[STAT_REF]] to i8*)],
 

diff  --git a/clang/test/OpenMP/nvptx_allocate_codegen.cpp 
b/clang/test/OpenMP/nvptx_allocate_codegen.cpp
index 647bc1d96efc..e8352b85fbcd 100644
--- a/clang/test/OpenMP/nvptx_allocate_codegen.cpp
+++ b/clang/test/OpenMP/nvptx_allocate_codegen.cpp
@@ -17,11 +17,11 @@ extern const omp_allocator_handle_t omp_pteam_mem_alloc;
 extern const omp_allocator_handle_t omp_

[PATCH] D70549: [OPENMP]Fix PR41826: symbols visibility in device code.

[Alexey Bataev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbbc328c62430: [OPENMP]Fix PR41826: symbols visibility in 
device code. (authored by ABataev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70549

Files:
  clang/lib/AST/Decl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/OpenMP/declare_target_codegen.cpp
  clang/test/OpenMP/nvptx_allocate_codegen.cpp
  clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp
  clang/test/OpenMP/nvptx_target_codegen.cpp
  clang/test/OpenMP/nvptx_unsupported_type_codegen.cpp

Index: clang/test/OpenMP/nvptx_unsupported_type_codegen.cpp
===
--- clang/test/OpenMP/nvptx_unsupported_type_codegen.cpp
+++ clang/test/OpenMP/nvptx_unsupported_type_codegen.cpp
@@ -34,18 +34,18 @@
 #pragma omp declare target
 T a = T();
 T f = a;
-// CHECK: define{{ dso_local | }}void @{{.+}}foo{{.+}}([[T]]* byval([[T]]) align {{.+}})
+// CHECK: define{{ hidden | }}void @{{.+}}foo{{.+}}([[T]]* byval([[T]]) align {{.+}})
 void foo(T a = T()) {
   return;
 }
-// CHECK: define{{ dso_local | }}[6 x i64] @{{.+}}bar{{.+}}()
+// CHECK: define{{ hidden | }}[6 x i64] @{{.+}}bar{{.+}}()
 T bar() {
 // CHECK:  bitcast [[T]]* %{{.+}} to [6 x i64]*
 // CHECK-NEXT: load [6 x i64], [6 x i64]* %{{.+}},
 // CHECK-NEXT: ret [6 x i64]
   return T();
 }
-// CHECK: define{{ dso_local | }}void @{{.+}}baz{{.+}}()
+// CHECK: define{{ hidden | }}void @{{.+}}baz{{.+}}()
 void baz() {
 // CHECK:  call [6 x i64] @{{.+}}bar{{.+}}()
 // CHECK-NEXT: bitcast [[T]]* %{{.+}} to [6 x i64]*
@@ -54,17 +54,17 @@
 }
 T1 a1 = T1();
 T1 f1 = a1;
-// CHECK: define{{ dso_local | }}void @{{.+}}foo1{{.+}}([[T1]]* byval([[T1]]) align {{.+}})
+// CHECK: define{{ hidden | }}void @{{.+}}foo1{{.+}}([[T1]]* byval([[T1]]) align {{.+}})
 void foo1(T1 a = T1()) {
   return;
 }
-// CHECK: define{{ dso_local | }}[[T1]] @{{.+}}bar1{{.+}}()
+// CHECK: define{{ hidden | }}[[T1]] @{{.+}}bar1{{.+}}()
 T1 bar1() {
 // CHECK:  load [[T1]], [[T1]]*
 // CHECK-NEXT: ret [[T1]]
   return T1();
 }
-// CHECK: define{{ dso_local | }}void @{{.+}}baz1{{.+}}()
+// CHECK: define{{ hidden | }}void @{{.+}}baz1{{.+}}()
 void baz1() {
 // CHECK: call [[T1]] @{{.+}}bar1{{.+}}()
   T1 t = bar1();
Index: clang/test/OpenMP/nvptx_target_codegen.cpp
===
--- clang/test/OpenMP/nvptx_target_codegen.cpp
+++ clang/test/OpenMP/nvptx_target_codegen.cpp
@@ -573,7 +573,7 @@
   // CHECK: [[EXIT]]
   // CHECK: ret void
 
-  // CHECK: define{{ dso_local | }}i32 [[BAZ]](i32 [[F:%.*]], double* dereferenceable{{.*}})
+  // CHECK: define{{ hidden | }}i32 [[BAZ]](i32 [[F:%.*]], double* dereferenceable{{.*}})
   // CHECK: alloca i32,
   // CHECK: [[LOCAL_F_PTR:%.+]] = alloca i32,
   // CHECK: [[ZERO_ADDR:%.+]] = alloca i32,
Index: clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp
===
--- clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp
+++ clang/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp
@@ -16,9 +16,9 @@
 // SIMD-ONLY-NOT: {{__kmpc|__tgt}}
 
 // DEVICE-DAG: [[C_ADDR:.+]] = internal global i32 0,
-// DEVICE-DAG: [[CD_ADDR:@.+]] ={{ dso_local | }}global %struct.S zeroinitializer,
+// DEVICE-DAG: [[CD_ADDR:@.+]] ={{ hidden | }}global %struct.S zeroinitializer,
 // HOST-DAG: @[[C_ADDR:.+]] = internal global i32 0,
-// HOST-DAG: @[[CD_ADDR:.+]] ={{ dso_local | }}global %struct.S zeroinitializer,
+// HOST-DAG: @[[CD_ADDR:.+]] ={{ hidden | }}global %struct.S zeroinitializer,
 
 #pragma omp declare target
 int foo() { return 0; }
@@ -34,12 +34,12 @@
 #pragma omp declare target (bar)
 int caz() { return 0; }
 
-// DEVICE-DAG: define{{ dso_local | }}i32 [[FOO:@.*foo.*]]()
-// DEVICE-DAG: define{{ dso_local | }}i32 [[BAR:@.*bar.*]]()
-// DEVICE-DAG: define{{ dso_local | }}i32 [[BAZ:@.*baz.*]]()
-// DEVICE-DAG: define{{ dso_local | }}i32 [[DOO:@.*doo.*]]()
-// DEVICE-DAG: define{{ dso_local | }}i32 [[CAR:@.*car.*]]()
-// DEVICE-DAG: define{{ dso_local | }}i32 [[CAZ:@.*caz.*]]()
+// DEVICE-DAG: define{{ hidden | }}i32 [[FOO:@.*foo.*]]()
+// DEVICE-DAG: define{{ hidden | }}i32 [[BAR:@.*bar.*]]()
+// DEVICE-DAG: define{{ hidden | }}i32 [[BAZ:@.*baz.*]]()
+// DEVICE-DAG: define{{ hidden | }}i32 [[DOO:@.*doo.*]]()
+// DEVICE-DAG: define{{ hidden | }}i32 [[CAR:@.*car.*]]()
+// DEVICE-DAG: define{{ hidden | }}i32 [[CAZ:@.*caz.*]]()
 
 static int c = foo() + bar() + baz();
 #pragma omp declare target (c)
Index: clang/test/OpenMP/nvptx_allocate_codegen.cpp
===
--- clang/test/OpenMP/nvptx_allocate_codegen.cpp
+++ clang/test/OpenMP/nvptx_allocate_codegen.cpp
@@ -17,11 +17,11 @@
 extern const omp_allocator_handle_t omp_thread_mem_alloc;
 
 // CHECK-DAG: @{{.+}}St1{{.+}}b{{.

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

[Tyker via Phabricator via cfe-commits]

Tyker added a comment.

Thanks for the review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70638



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

[PATCH] D70692: IRGen: Call SetLLVMFunctionAttributes{, ForDefinition} on __cfi_check_fail.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc created this revision.
pcc added a reviewer: eugenis.
Herald added a project: clang.

This has the main effect of causing target-cpu and target-features to be set
on __cfi_check_fail, causing the function to become ABI-compatible with other
functions in the case where these attributes affect ABI (e.g. reserve-x18).

Technically we only need to call SetLLVMFunctionAttributes to get the target-*
attributes set, but since we're creating a definition we probably ought to
call the ForDefinition function as well.

Fixes PR44094.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70692

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/CodeGen/cfi-check-fail-attrs.c


Index: clang/test/CodeGen/cfi-check-fail-attrs.c
===
--- /dev/null
+++ clang/test/CodeGen/cfi-check-fail-attrs.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -triple aarch64-unknown-linux -fsanitize-cfi-cross-dso 
-target-feature +reserve-x18 -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: define weak_odr hidden void @__cfi_check_fail{{.*}} [[ATTR:#[0-9]*]]
+
+// CHECK: attributes [[ATTR]] = {{.*}} "target-features"="+reserve-x18"
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -3200,6 +3200,9 @@
   llvm::Function *F = llvm::Function::Create(
   llvm::FunctionType::get(VoidTy, {VoidPtrTy, VoidPtrTy}, false),
   llvm::GlobalValue::WeakODRLinkage, "__cfi_check_fail", &CGM.getModule());
+
+  CGM.SetLLVMFunctionAttributes(GlobalDecl(), FI, F);
+  CGM.SetLLVMFunctionAttributesForDefinition(nullptr, F);
   F->setVisibility(llvm::GlobalValue::HiddenVisibility);
 
   StartFunction(GlobalDecl(), CGM.getContext().VoidTy, F, FI, Args,


Index: clang/test/CodeGen/cfi-check-fail-attrs.c
===
--- /dev/null
+++ clang/test/CodeGen/cfi-check-fail-attrs.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -triple aarch64-unknown-linux -fsanitize-cfi-cross-dso -target-feature +reserve-x18 -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: define weak_odr hidden void @__cfi_check_fail{{.*}} [[ATTR:#[0-9]*]]
+
+// CHECK: attributes [[ATTR]] = {{.*}} "target-features"="+reserve-x18"
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -3200,6 +3200,9 @@
   llvm::Function *F = llvm::Function::Create(
   llvm::FunctionType::get(VoidTy, {VoidPtrTy, VoidPtrTy}, false),
   llvm::GlobalValue::WeakODRLinkage, "__cfi_check_fail", &CGM.getModule());
+
+  CGM.SetLLVMFunctionAttributes(GlobalDecl(), FI, F);
+  CGM.SetLLVMFunctionAttributesForDefinition(nullptr, F);
   F->setVisibility(llvm::GlobalValue::HiddenVisibility);
 
   StartFunction(GlobalDecl(), CGM.getContext().VoidTy, F, FI, Args,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Github build status reporting

[Galina Kistanova via cfe-commits]

Hello everyone,

I have configured buildbot to report build statuses to github. It is
running in a pilot mode. Only the following 4 builders annotate revisions
they build for now:

* llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast
* llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast
* llvm-clang-x86_64-expensive-checks-ubuntu
* llvm-clang-x86_64-win-fast

Please let me know if you have any questions, concerns, or see any issues.

Thanks

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

[PATCH] D70693: [scan-build-py] Set of small fixes

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun created this revision.
xazax.hun added reviewers: NoQ, haowei, rizsotto.mailinglist.
Herald added subscribers: Charusso, gamesh411, Szelethus, dkrupp, rnkovacs, 
whisperity.
Herald added a project: clang.
xazax.hun added a subscriber: phosek.

This patch fix some small errors in scan-build-py, including:

- When invoking clang with `-###` and parsing error add 
`-fno-color-diagnostics` to the invocation to avoid problems parsing potential 
errors. This might be sub-optimal for users expecting to see the results in the 
command line and expecting to have colors. If we really want this to work I can 
either make the regex more forgiving or try to restore the 
`-fcolor-diagnostics`flag to its original state after the `-###` invocation.
- If a file was missing from a compilation database the whole analysis was 
stopped due to an unhandled exception.
- Fix two typos: analyzis -> analysis


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70693

Files:
  clang/tools/scan-build-py/libscanbuild/analyze.py
  clang/tools/scan-build-py/libscanbuild/clang.py


Index: clang/tools/scan-build-py/libscanbuild/clang.py
===
--- clang/tools/scan-build-py/libscanbuild/clang.py
+++ clang/tools/scan-build-py/libscanbuild/clang.py
@@ -19,6 +19,11 @@
 ACTIVE_CHECKER_PATTERN = re.compile(r'^-analyzer-checker=(.*)$')
 
 
+class ClangErrorException(Exception):
+def __init__(self, error):
+self.error = error
+
+
 def get_version(clang):
 """ Returns the compiler version as string.
 
@@ -39,13 +44,14 @@
 
 cmd = command[:]
 cmd.insert(1, '-###')
+cmd.append('-fno-color-diagnostics')
 
 output = run_command(cmd, cwd=cwd)
 # The relevant information is in the last line of the output.
 # Don't check if finding last line fails, would throw exception anyway.
 last_line = output[-1]
 if re.search(r'clang(.*): error:', last_line):
-raise Exception(last_line)
+raise ClangErrorException(last_line)
 return decode(last_line)
 
 
Index: clang/tools/scan-build-py/libscanbuild/analyze.py
===
--- clang/tools/scan-build-py/libscanbuild/analyze.py
+++ clang/tools/scan-build-py/libscanbuild/analyze.py
@@ -33,7 +33,8 @@
 from libscanbuild.report import document
 from libscanbuild.compilation import split_command, classify_source, \
 compiler_language
-from libscanbuild.clang import get_version, get_arguments, get_triple_arch
+from libscanbuild.clang import get_version, get_arguments, get_triple_arch, \
+ClangErrorException
 from libscanbuild.shell import decode
 
 __all__ = ['scan_build', 'analyze_build', 'analyze_compiler_wrapper']
@@ -435,7 +436,7 @@
 of the compilation database.
 
 This complex task is decomposed into smaller methods which are calling
-each other in chain. If the analyzis is not possible the given method
+each other in chain. If the analysis is not possible the given method
 just return and break the chain.
 
 The passed parameter is a python dictionary. Each method first check
@@ -451,7 +452,7 @@
 
 return arch_check(opts)
 except Exception:
-logging.error("Problem occurred during analyzis.", exc_info=1)
+logging.error("Problem occurred during analysis.", exc_info=1)
 return None
 
 
@@ -490,10 +491,13 @@
 os.close(handle)
 # Execute Clang again, but run the syntax check only.
 cwd = opts['directory']
-cmd = get_arguments(
-[opts['clang'], '-fsyntax-only', '-E'
- ] + opts['flags'] + [opts['file'], '-o', name], cwd)
-run_command(cmd, cwd=cwd)
+cmd = [opts['clang'], '-fsyntax-only', '-E'] + opts['flags'] + \
+[opts['file'], '-o', name]
+try:
+cmd = get_arguments(cmd, cwd)
+run_command(cmd, cwd=cwd)
+except ClangErrorException:
+pass
 # write general information about the crash
 with open(name + '.info.txt', 'w') as handle:
 handle.write(opts['file'] + os.linesep)
@@ -542,6 +546,12 @@
 opts.update(result)
 continuation(opts)
 return result
+except ClangErrorException as ex:
+result = {'error_output': ex.error, 'exit_code': 0}
+if opts.get('output_failures', False):
+opts.update(result)
+continuation(opts)
+return result
 
 
 def extdef_map_list_src_to_ast(extdef_src_list):


Index: clang/tools/scan-build-py/libscanbuild/clang.py
===
--- clang/tools/scan-build-py/libscanbuild/clang.py
+++ clang/tools/scan-build-py/libscanbuild/clang.py
@@ -19,6 +19,11 @@
 ACTIVE_CHECKER_PATTERN = re.compile(r'^-analyzer-checker=(.*)$')
 
 
+class ClangErrorException(Exception):
+def __init__(self, error):
+self.error = error
+
+
 def get_version(clang):
 """ Returns the compiler version as string.
 
@@ -39,13 +44,

[PATCH] D70627: [AST] Split out Attrs.h for table generated *Attr classes

[Reid Kleckner via Phabricator via cfe-commits]

rnk marked an inline comment as done.
rnk added a comment.

Thanks!




Comment at: clang/include/clang/AST/Attrs.h:1
+//===--- Attr.h - Classes for representing attributes --*- C++ -*-===//
+//

aaron.ballman wrote:
> This should read `Attrs.h` instead, but I find the naming distinction to be a 
> bit too subtle. How about `AttrSubclasses.h` or `ConcreteAttrs.h` (other 
> suggestions welcome too).
WDYT about following Decl.h / DeclBase.h? So, rename Attr.h to AttrBase.h, 
update all current includers of Attr.h to AttrBase.h, then add in new includes 
of Attr.h. Perhaps staged as two commits, one to do the rename, one to 
introduce the new header.

Otherwise, I guess I like AttrSubclasses.h from your list, or AttrClasses.h as 
an alternative.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70627



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

[PATCH] D70424: clang/AMDGPU: Fix default for frame-pointer attribute

[Tony Tye via Phabricator via cfe-commits]

t-tye added a comment.

@scott.linder can answer about the -g question, but I would expect that the CFI 
is capable of describing the address of the CFA regardless of whether there is 
a frame pointer by simply knowing the constant offset from the stack pointer.

For AMDGPU it seems to me what we really have is an FP and we optimize away the 
SP since the stack grows low address to high address, and S32 points to the 
base of the frame, and not the top of the stack.


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

https://reviews.llvm.org/D70424



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

[PATCH] D70627: [AST] Split out Attrs.h for table generated *Attr classes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/AST/Attrs.h:1
+//===--- Attr.h - Classes for representing attributes --*- C++ -*-===//
+//

rnk wrote:
> aaron.ballman wrote:
> > This should read `Attrs.h` instead, but I find the naming distinction to be 
> > a bit too subtle. How about `AttrSubclasses.h` or `ConcreteAttrs.h` (other 
> > suggestions welcome too).
> WDYT about following Decl.h / DeclBase.h? So, rename Attr.h to AttrBase.h, 
> update all current includers of Attr.h to AttrBase.h, then add in new 
> includes of Attr.h. Perhaps staged as two commits, one to do the rename, one 
> to introduce the new header.
> 
> Otherwise, I guess I like AttrSubclasses.h from your list, or AttrClasses.h 
> as an alternative.
I think AttrBase.h for the `Attr` base class makes a lot of sense -- it makes 
it more clear that you only get the base class. WDYT about `Attrs.h` for the 
subclasses instead of `Attr.h` -- might make it a bit more clear that you're 
getting all of the attributes, not just the base class?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70627



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

[clang] 0e12815 - Revert "[Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy"

[Tom Stellard via cfe-commits]

Author: Tom Stellard
Date: 2019-11-25T13:19:57-08:00
New Revision: 0e12815566b2f8dd2d3bfe2319e55b3ffb9767ae

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

LOG: Revert "[Diagnostics] Put "deprecated copy" warnings into 
-Wdeprecated-copy"

This reverts commit 9353c5dd0664ea444236e527bf93566e11dc34df.

This commit introduced bot falures for multi-stage bots with -Werror.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDeclCXX.cpp

Removed: 
clang/test/SemaCXX/deprecated-copy.cpp



diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 666193e074f0..086e57778051 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -128,8 +128,6 @@ def CXX11CompatDeprecatedWritableStr :
 
 def DeprecatedAttributes : DiagGroup<"deprecated-attributes">;
 def DeprecatedCommaSubscript : DiagGroup<"deprecated-comma-subscript">;
-def DeprecatedCopy : DiagGroup<"deprecated-copy">;
-def DeprecatedCopyDtor : DiagGroup<"deprecated-copy-dtor">;
 def DeprecatedDeclarations : DiagGroup<"deprecated-declarations">;
 def UnavailableDeclarations : DiagGroup<"unavailable-declarations">;
 def UnguardedAvailabilityNew : DiagGroup<"unguarded-availability-new">;
@@ -149,8 +147,6 @@ def DeprecatedWritableStr : 
DiagGroup<"deprecated-writable-strings",
 // FIXME: Why is DeprecatedImplementations not in this group?
 def Deprecated : DiagGroup<"deprecated", [DeprecatedAttributes,
   DeprecatedCommaSubscript,
-  DeprecatedCopy,
-  DeprecatedCopyDtor,
   DeprecatedDeclarations,
   DeprecatedDynamicExceptionSpec,
   DeprecatedIncrementBool,
@@ -817,7 +813,6 @@ def Move : DiagGroup<"move", [
   ]>;
 
 def Extra : DiagGroup<"extra", [
-DeprecatedCopy,
 MissingFieldInitializers,
 IgnoredQualifiers,
 InitializerOverrides,

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c19862addec9..799b3ed2ea92 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -551,13 +551,9 @@ def err_access_decl : Error<
   "use using declarations instead">;
 def warn_deprecated_copy_operation : Warning<
   "definition of implicit copy %select{constructor|assignment operator}1 "
-  "for %0 is deprecated because it has a user-declared copy "
-  "%select{assignment operator|constructor}1">,
-  InGroup, DefaultIgnore;
-def warn_deprecated_copy_dtor_operation : Warning<
-  "definition of implicit copy %select{constructor|assignment operator}1 "
-  "for %0 is deprecated because it has a user-declared destructor">,
-  InGroup, DefaultIgnore;
+  "for %0 is deprecated because it has a user-declared "
+  "%select{copy %select{assignment operator|constructor}1|destructor}2">,
+  InGroup, DefaultIgnore;
 def warn_cxx17_compat_exception_spec_in_signature : Warning<
   "mangled name of %0 will change in C++17 due to non-throwing exception "
   "specification in function signature">, InGroup;

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 9c220efebe60..e3ea9788ad5d 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -12435,10 +12435,9 @@ static void diagnoseDeprecatedCopyOperation(Sema &S, 
CXXMethodDecl *CopyOp) {
 
   if (UserDeclaredOperation && UserDeclaredOperation->isUserProvided()) {
 S.Diag(UserDeclaredOperation->getLocation(),
-   isa(UserDeclaredOperation)
-   ? diag::warn_deprecated_copy_dtor_operation
-   : diag::warn_deprecated_copy_operation)
-<< RD << /*copy assignment*/ !isa(CopyOp);
+ diag::warn_deprecated_copy_operation)
+  << RD << /*copy assignment*/!isa(CopyOp)
+  << /*destructor*/isa(UserDeclaredOperation);
   }
 }
 

diff  --git a/clang/test/SemaCXX/deprecated-copy.cpp 
b/clang/test/SemaCXX/deprecated-copy.cpp
deleted file mode 100644
index 4d3e798d912b..
--- a/clang/test/SemaCXX/deprecated-copy.cpp
+++ /dev/null
@@ -1,23 +0,0 @@
-// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated-copy -verify
-// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated-copy-dtor -DDEPRECATED_COPY_DTOR 
-verify
-// RUN: %clang_cc1 -std=c++11 %s -Wextra -verify
-
-#ifdef DEPRECATED_COPY_DTOR
-struct A {
-  int *ptr;
-  ~A() { delete ptr; } // expected-warning {{definition of implicit copy 
constructor for 'A' is deprecated because it has a 

[clang] 3c51425 - Revert "[Diagnostic] add a warning which warns about misleading indentation"

[Tom Stellard via cfe-commits]

Author: Tom Stellard
Date: 2019-11-25T13:19:57-08:00
New Revision: 3c5142597a451a03db21c2ffe8f6520c7eacce59

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

LOG: Revert "[Diagnostic] add a warning which warns about misleading 
indentation"

This reverts commit 7b86188b50bf6e537fe98b326f258fbd23108b83.

This commit introduced bot falures for multi-stage bots with -Werror.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Parse/Parser.h
clang/lib/Parse/ParseStmt.cpp
clang/test/Index/pragma-diag-reparse.c
clang/test/Misc/warning-wall.c
clang/test/Preprocessor/pragma_diagnostic_sections.cpp

Removed: 
clang/test/SemaCXX/warn-misleading-indentation.cpp



diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 086e57778051..6b83bf59ea89 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -689,7 +689,6 @@ def ZeroLengthArray : DiagGroup<"zero-length-array">;
 def GNUZeroLineDirective : DiagGroup<"gnu-zero-line-directive">;
 def GNUZeroVariadicMacroArguments : 
DiagGroup<"gnu-zero-variadic-macro-arguments">;
 def Fallback : DiagGroup<"fallback">;
-def MisleadingIndentation : DiagGroup<"misleading-indentation">;
 
 // This covers both the deprecated case (in C++98)
 // and the extension case (in C++11 onwards).
@@ -880,7 +879,7 @@ def Consumed   : DiagGroup<"consumed">;
 // Note that putting warnings in -Wall will not disable them by default. If a
 // warning should be active _only_ when -Wall is passed in, mark it as
 // DefaultIgnore in addition to putting it here.
-def All : DiagGroup<"all", [Most, Parentheses, Switch, SwitchBool, 
MisleadingIndentation]>;
+def All : DiagGroup<"all", [Most, Parentheses, Switch, SwitchBool]>;
 
 // Warnings that should be in clang-cl /w4.
 def : DiagGroup<"CL4", [All, Extra]>;

diff  --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index e6aa92eddef7..c94d1b99d0e8 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -61,13 +61,6 @@ def warn_null_statement : Warning<
   "remove unnecessary ';' to silence this warning">,
   InGroup, DefaultIgnore;
 
-def warn_misleading_indentation : Warning<
-  "misleading indentation; %select{statement|declaration}0 is not part of "
-  "the previous '%select{if|else|for|while}1'">,
-  InGroup, DefaultIgnore;
-def note_previous_statement : Note<
-  "previous statement is here">;
-
 def ext_thread_before : Extension<"'__thread' before '%0'">;
 def ext_keyword_as_ident : ExtWarn<
   "keyword '%0' will be made available as an identifier "

diff  --git a/clang/include/clang/Parse/Parser.h 
b/clang/include/clang/Parse/Parser.h
index edd31e3ff7e8..ea1116ff7a23 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -2266,13 +2266,11 @@ class Parser : public CodeCompletionHandler {
 return isTypeSpecifierQualifier();
   }
 
-public:
   /// isCXXDeclarationStatement - C++-specialized function that disambiguates
   /// between a declaration or an expression statement, when parsing function
   /// bodies. Returns true for declaration, false for expression.
   bool isCXXDeclarationStatement();
 
-private:
   /// isCXXSimpleDeclaration - C++-specialized function that disambiguates
   /// between a simple-declaration or an expression-statement.
   /// If during the disambiguation process a parsing error is encountered,

diff  --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index ce8aa7574b9b..727ab75adae8 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -1191,27 +1191,6 @@ bool Parser::ParseParenExprOrCondition(StmtResult 
*InitStmt,
   return false;
 }
 
-enum MisleadingStatementKind { MSK_if, MSK_else, MSK_for, MSK_while };
-
-static void
-MaybeDiagnoseMisleadingIndentation(Parser &P, SourceLocation PrevLoc,
-   SourceLocation StmtLoc,
-   MisleadingStatementKind StmtKind) {
-  Token Tok = P.getCurToken();
-  if (Tok.is(tok::semi))
-return;
-  SourceManager &SM = P.getPreprocessor().getSourceManager();
-  unsigned PrevColNum = SM.getSpellingColumnNumber(PrevLoc);
-  unsigned CurColNum = SM.getSpellingColumnNumber(Tok.getLocation());
-  unsigned StmtColNum = SM.getSpellingColumnNumber(StmtLoc);
-  if (!Tok.isAtStartOfLine() ||
-  (PrevColNum != 0 && CurColNum != 0 && StmtColNum != 0 &&
-   PrevColNum > StmtColNum && PrevColNum == CurColNum)) {
-P.Diag(Tok.getLocation(), diag::warn_misleading_inde

[PATCH] D70692: IRGen: Call SetLLVMFunctionAttributes{, ForDefinition} on __cfi_check_fail.

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis accepted this revision.
eugenis 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/D70692/new/

https://reviews.llvm.org/D70692



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

[PATCH] D69938: [OpenCL] Use __generic addr space when generating internal representation of lambda

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Sema/SemaLambda.cpp:891
+if (getLangOpts().OpenCL)
+  EPI.TypeQuals.addAddressSpace(LangAS::opencl_generic);
+

This should probably check that there isn't already an address space, right?


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

https://reviews.llvm.org/D69938



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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song created this revision.
yonghong-song added reviewers: dblaikie, aprantl, RKSimon.
yonghong-song added a project: debug-info.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.

Extern variable usage in BPF is different from traditional
pure user space application. Recent discussion in linux bpf 
mailing list has two use cases where debug info types are 
required to use extern variables:

- extern types are required to have a suitable interface in libbpf (bpf loader) 
to provide kernel config parameters to bpf programs. 
https://lore.kernel.org/bpf/CAEf4BzYCNo5GeVGMhp3fhysQ=_axAf=23ptwazs-yayafmx...@mail.gmail.com/T/#t
- extern types are required so kernel bpf verifier can verify program which 
uses external functions more precisely. This will make later link with actual 
external function no need to reverify. 
https://lore.kernel.org/bpf/87eez4odqp@toke.dk/T/#m8d5c3e87ffe7f2764e02d722cb0d8cbc136880ed

This patch added clang support to emit debuginfo for extern variables
with a TargetInfo hook to enable it. Currently, only BPF target enables
to generate debug info for extern variables. The emission of
such debuginfo is disabled for C++ at this moment since BPF only
supports a subset of C language. Emission with C++ can be enabled
later if an appropriate use case is identified.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70696

Files:
  clang/include/clang/AST/ASTConsumer.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Basic/Targets/BPF.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ModuleBuilder.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGenCXX/debug-info-extern-duplicate-2.c
  clang/test/CodeGenCXX/debug-info-extern-duplicate.c
  clang/test/CodeGenCXX/debug-info-extern-unused.c
  clang/test/CodeGenCXX/debug-info-extern-var-char-2.c
  clang/test/CodeGenCXX/debug-info-extern-var-char.c
  clang/test/CodeGenCXX/debug-info-extern-var-func.c
  clang/test/CodeGenCXX/debug-info-extern-var-multi.c
  llvm/include/llvm/IR/DIBuilder.h
  llvm/lib/IR/DIBuilder.cpp
  llvm/lib/IR/DebugInfo.cpp
  llvm/unittests/Transforms/Utils/CloningTest.cpp

Index: llvm/unittests/Transforms/Utils/CloningTest.cpp
===
--- llvm/unittests/Transforms/Utils/CloningTest.cpp
+++ llvm/unittests/Transforms/Utils/CloningTest.cpp
@@ -764,7 +764,7 @@
 
 DBuilder.createGlobalVariableExpression(
 Subprogram, "unattached", "unattached", File, 1,
-DBuilder.createNullPtrType(), false, Expr);
+DBuilder.createNullPtrType(), false, true, Expr);
 
 auto *Entry = BasicBlock::Create(C, "", F);
 IBuilder.SetInsertPoint(Entry);
Index: llvm/lib/IR/DebugInfo.cpp
===
--- llvm/lib/IR/DebugInfo.cpp
+++ llvm/lib/IR/DebugInfo.cpp
@@ -1290,7 +1290,7 @@
   return wrap(unwrap(Builder)->createGlobalVariableExpression(
   unwrapDI(Scope), {Name, NameLen}, {Linkage, LinkLen},
   unwrapDI(File), LineNo, unwrapDI(Ty), LocalToUnit,
-  unwrap(Expr), unwrapDI(Decl),
+  true, unwrap(Expr), unwrapDI(Decl),
   nullptr, AlignInBits));
 }
 
Index: llvm/lib/IR/DIBuilder.cpp
===
--- llvm/lib/IR/DIBuilder.cpp
+++ llvm/lib/IR/DIBuilder.cpp
@@ -639,13 +639,14 @@
 
 DIGlobalVariableExpression *DIBuilder::createGlobalVariableExpression(
 DIScope *Context, StringRef Name, StringRef LinkageName, DIFile *F,
-unsigned LineNumber, DIType *Ty, bool isLocalToUnit, DIExpression *Expr,
+unsigned LineNumber, DIType *Ty, bool isLocalToUnit,
+bool isDefined, DIExpression *Expr,
 MDNode *Decl, MDTuple *templateParams, uint32_t AlignInBits) {
   checkGlobalVariableScope(Context);
 
   auto *GV = DIGlobalVariable::getDistinct(
   VMContext, cast_or_null(Context), Name, LinkageName, F,
-  LineNumber, Ty, isLocalToUnit, true, cast_or_null(Decl),
+  LineNumber, Ty, isLocalToUnit, isDefined, cast_or_null(Decl),
   templateParams, AlignInBits);
   if (!Expr)
 Expr = createExpression();
Index: llvm/include/llvm/IR/DIBuilder.h
===
--- llvm/include/llvm/IR/DIBuilder.h
+++ llvm/include/llvm/IR/DIBuilder.h
@@ -581,7 +581,7 @@
 ///specified)
 DIGlobalVariableExpression *createGlobalVariableExpression(
 DIScope *Context, StringRef Name, StringRef LinkageName, DIFile *File,
-unsigned LineNo, DIType *Ty, bool isLocalToUnit,
+unsigned LineNo, DIType *Ty, bool isLocalToUnit, bool isDefined = true,
 DIExpression *Expr = nullptr, MDNode *Decl = nullptr,
 MDTuple *templateParams = nullptr, uint32_

[PATCH] D70627: [AST] Split out Attrs.h for table generated *Attr classes

[Reid Kleckner via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/AST/Attrs.h:1
+//===--- Attr.h - Classes for representing attributes --*- C++ -*-===//
+//

aaron.ballman wrote:
> rnk wrote:
> > aaron.ballman wrote:
> > > This should read `Attrs.h` instead, but I find the naming distinction to 
> > > be a bit too subtle. How about `AttrSubclasses.h` or `ConcreteAttrs.h` 
> > > (other suggestions welcome too).
> > WDYT about following Decl.h / DeclBase.h? So, rename Attr.h to AttrBase.h, 
> > update all current includers of Attr.h to AttrBase.h, then add in new 
> > includes of Attr.h. Perhaps staged as two commits, one to do the rename, 
> > one to introduce the new header.
> > 
> > Otherwise, I guess I like AttrSubclasses.h from your list, or AttrClasses.h 
> > as an alternative.
> I think AttrBase.h for the `Attr` base class makes a lot of sense -- it makes 
> it more clear that you only get the base class. WDYT about `Attrs.h` for the 
> subclasses instead of `Attr.h` -- might make it a bit more clear that you're 
> getting all of the attributes, not just the base class?
That makes sense. Do you mind if I land this first, and then do the Attr.h -> 
AttrBase.h rename? That way I don't have to make a new patch, put it up for 
review, and rebase this patch over it. I'll send a review assuming the answer 
is yes and then reorder them if you have concerns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70627



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

[PATCH] D70627: [AST] Split out Attrs.h for table generated *Attr classes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/AST/Attrs.h:1
+//===--- Attr.h - Classes for representing attributes --*- C++ -*-===//
+//

rnk wrote:
> aaron.ballman wrote:
> > rnk wrote:
> > > aaron.ballman wrote:
> > > > This should read `Attrs.h` instead, but I find the naming distinction 
> > > > to be a bit too subtle. How about `AttrSubclasses.h` or 
> > > > `ConcreteAttrs.h` (other suggestions welcome too).
> > > WDYT about following Decl.h / DeclBase.h? So, rename Attr.h to 
> > > AttrBase.h, update all current includers of Attr.h to AttrBase.h, then 
> > > add in new includes of Attr.h. Perhaps staged as two commits, one to do 
> > > the rename, one to introduce the new header.
> > > 
> > > Otherwise, I guess I like AttrSubclasses.h from your list, or 
> > > AttrClasses.h as an alternative.
> > I think AttrBase.h for the `Attr` base class makes a lot of sense -- it 
> > makes it more clear that you only get the base class. WDYT about `Attrs.h` 
> > for the subclasses instead of `Attr.h` -- might make it a bit more clear 
> > that you're getting all of the attributes, not just the base class?
> That makes sense. Do you mind if I land this first, and then do the Attr.h -> 
> AttrBase.h rename? That way I don't have to make a new patch, put it up for 
> review, and rebase this patch over it. I'll send a review assuming the answer 
> is yes and then reorder them if you have concerns.
Totally fine with that approach, thanks for checking!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70627



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

[PATCH] D70625: [DebugInfo][BPF] Support to emit debugInfo for extern variables

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song abandoned this revision.
yonghong-song added a comment.

@dblaikie As suggested, just submitted two separate patches:

- https://reviews.llvm.org/D70696 for clang/llvm change to generate debuginfo 
for extern variables
- https://reviews.llvm.org/D70697 for BPF backend to consume such debuginfo.

Please help take a look. Thanks!

I will abandon this patch to avoid confusion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70625



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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

The corresponding BPF patch to consume the debuginfo 
https://reviews.llvm.org/D70697.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696



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

[PATCH] D70627: [AST] Split out Attrs.h for table generated *Attr classes

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM with the follow-up plan.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70627



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

[PATCH] D70627: [AST] Split out Attrs.h for table generated *Attr classes

[Reid Kleckner via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/AST/Attrs.h:1
+//===--- Attr.h - Classes for representing attributes --*- C++ -*-===//
+//

aaron.ballman wrote:
> rnk wrote:
> > aaron.ballman wrote:
> > > rnk wrote:
> > > > aaron.ballman wrote:
> > > > > This should read `Attrs.h` instead, but I find the naming distinction 
> > > > > to be a bit too subtle. How about `AttrSubclasses.h` or 
> > > > > `ConcreteAttrs.h` (other suggestions welcome too).
> > > > WDYT about following Decl.h / DeclBase.h? So, rename Attr.h to 
> > > > AttrBase.h, update all current includers of Attr.h to AttrBase.h, then 
> > > > add in new includes of Attr.h. Perhaps staged as two commits, one to do 
> > > > the rename, one to introduce the new header.
> > > > 
> > > > Otherwise, I guess I like AttrSubclasses.h from your list, or 
> > > > AttrClasses.h as an alternative.
> > > I think AttrBase.h for the `Attr` base class makes a lot of sense -- it 
> > > makes it more clear that you only get the base class. WDYT about 
> > > `Attrs.h` for the subclasses instead of `Attr.h` -- might make it a bit 
> > > more clear that you're getting all of the attributes, not just the base 
> > > class?
> > That makes sense. Do you mind if I land this first, and then do the Attr.h 
> > -> AttrBase.h rename? That way I don't have to make a new patch, put it up 
> > for review, and rebase this patch over it. I'll send a review assuming the 
> > answer is yes and then reorder them if you have concerns.
> Totally fine with that approach, thanks for checking!
Hm, I just realized that this will create a lot of churn for out of tree 
includers of Attr.h. Chrome's plugins have three hits:
https://cs.chromium.org/search/?q=include.*AST/Attr%5C.h&sq=package:chromium&type=cs
Other users will have more.

Clang doesn't promise API stability, so we can certainly do this, but it 
creates a fair volume of work.

Now I'm considering splitting out AttrBase.h on its own and updating as many 
includes of Attr.h to use that as possible. That'll end up being a much less 
disruptive and much smaller change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70627



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

[clang] d930ed1 - Disallow use of __has_c_attribute in C++ mode.

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2019-11-25T17:35:12-05:00
New Revision: d930ed1acc0ea49d4b3aae7e95b4c6d9cd310578

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

LOG: Disallow use of __has_c_attribute in C++ mode.

__has_cpp_attribute is not available in C mode, and __has_c_attribute
should not be available in C++ mode. This also adds a test to
demonstrate that we properly handle scoped attribute tokens even in C
mode.

Added: 
clang/test/Preprocessor/has_c_attribute.cpp

Modified: 
clang/lib/Lex/PPMacroExpansion.cpp
clang/test/Preprocessor/has_c_attribute.c

Removed: 




diff  --git a/clang/lib/Lex/PPMacroExpansion.cpp 
b/clang/lib/Lex/PPMacroExpansion.cpp
index e6e00b1c1700..a69c4dbb3a2a 100644
--- a/clang/lib/Lex/PPMacroExpansion.cpp
+++ b/clang/lib/Lex/PPMacroExpansion.cpp
@@ -370,7 +370,11 @@ void Preprocessor::RegisterBuiltinMacros() {
   Ident__has_extension= RegisterBuiltinMacro(*this, "__has_extension");
   Ident__has_builtin  = RegisterBuiltinMacro(*this, "__has_builtin");
   Ident__has_attribute= RegisterBuiltinMacro(*this, "__has_attribute");
-  Ident__has_c_attribute  = RegisterBuiltinMacro(*this, "__has_c_attribute");
+  if (!LangOpts.CPlusPlus)
+Ident__has_c_attribute = RegisterBuiltinMacro(*this, "__has_c_attribute");
+  else
+Ident__has_c_attribute = nullptr;
+
   Ident__has_declspec = RegisterBuiltinMacro(*this, 
"__has_declspec_attribute");
   Ident__has_include  = RegisterBuiltinMacro(*this, "__has_include");
   Ident__has_include_next = RegisterBuiltinMacro(*this, "__has_include_next");

diff  --git a/clang/test/Preprocessor/has_c_attribute.c 
b/clang/test/Preprocessor/has_c_attribute.c
index 843a67a2646c..f8b0b364faa5 100644
--- a/clang/test/Preprocessor/has_c_attribute.c
+++ b/clang/test/Preprocessor/has_c_attribute.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fdouble-square-bracket-attributes -std=c11 -E %s -o - | 
FileCheck %s
+// RUN: %clang_cc1 -std=c2x -E %s -o - | FileCheck %s
 
 // CHECK: has_fallthrough
 #if __has_c_attribute(fallthrough)
@@ -14,3 +15,8 @@
 #if __has_c_attribute(__nodiscard__)
   int has_nodiscard_underscore();
 #endif
+
+// CHECK: has_clang_annotate
+#if __has_c_attribute(clang::annotate)
+  int has_clang_annotate();
+#endif

diff  --git a/clang/test/Preprocessor/has_c_attribute.cpp 
b/clang/test/Preprocessor/has_c_attribute.cpp
new file mode 100644
index ..0bde73067178
--- /dev/null
+++ b/clang/test/Preprocessor/has_c_attribute.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -std=c++11 %s -verify
+
+#if __has_c_attribute(fallthrough) // expected-error {{function-like macro 
'__has_c_attribute' is not defined}}
+#endif
+
+#if __has_c_attribute(gnu::transparent_union) // expected-error 
{{function-like macro '__has_c_attribute' is not defined}}
+#endif
+



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

Re: [clang] d930ed1 - Disallow use of __has_c_attribute in C++ mode.

[James Y Knight via cfe-commits]

Isn't this unnecessarily annoying to users? You have the same syntax to use
the attributes, and the attributes are expected to be compatible when named
the same way, but you can't use the same #if conditional to check for
availability, when writing a header intended to work in both modes?

On Mon, Nov 25, 2019 at 5:35 PM Aaron Ballman via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Aaron Ballman
> Date: 2019-11-25T17:35:12-05:00
> New Revision: d930ed1acc0ea49d4b3aae7e95b4c6d9cd310578
>
> URL:
> https://github.com/llvm/llvm-project/commit/d930ed1acc0ea49d4b3aae7e95b4c6d9cd310578
> DIFF:
> https://github.com/llvm/llvm-project/commit/d930ed1acc0ea49d4b3aae7e95b4c6d9cd310578.diff
>
> LOG: Disallow use of __has_c_attribute in C++ mode.
>
> __has_cpp_attribute is not available in C mode, and __has_c_attribute
> should not be available in C++ mode. This also adds a test to
> demonstrate that we properly handle scoped attribute tokens even in C
> mode.
>
> Added:
> clang/test/Preprocessor/has_c_attribute.cpp
>
> Modified:
> clang/lib/Lex/PPMacroExpansion.cpp
> clang/test/Preprocessor/has_c_attribute.c
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang/lib/Lex/PPMacroExpansion.cpp
> b/clang/lib/Lex/PPMacroExpansion.cpp
> index e6e00b1c1700..a69c4dbb3a2a 100644
> --- a/clang/lib/Lex/PPMacroExpansion.cpp
> +++ b/clang/lib/Lex/PPMacroExpansion.cpp
> @@ -370,7 +370,11 @@ void Preprocessor::RegisterBuiltinMacros() {
>Ident__has_extension= RegisterBuiltinMacro(*this,
> "__has_extension");
>Ident__has_builtin  = RegisterBuiltinMacro(*this, "__has_builtin");
>Ident__has_attribute= RegisterBuiltinMacro(*this,
> "__has_attribute");
> -  Ident__has_c_attribute  = RegisterBuiltinMacro(*this,
> "__has_c_attribute");
> +  if (!LangOpts.CPlusPlus)
> +Ident__has_c_attribute = RegisterBuiltinMacro(*this,
> "__has_c_attribute");
> +  else
> +Ident__has_c_attribute = nullptr;
> +
>Ident__has_declspec = RegisterBuiltinMacro(*this,
> "__has_declspec_attribute");
>Ident__has_include  = RegisterBuiltinMacro(*this, "__has_include");
>Ident__has_include_next = RegisterBuiltinMacro(*this,
> "__has_include_next");
>
> diff  --git a/clang/test/Preprocessor/has_c_attribute.c
> b/clang/test/Preprocessor/has_c_attribute.c
> index 843a67a2646c..f8b0b364faa5 100644
> --- a/clang/test/Preprocessor/has_c_attribute.c
> +++ b/clang/test/Preprocessor/has_c_attribute.c
> @@ -1,4 +1,5 @@
>  // RUN: %clang_cc1 -fdouble-square-bracket-attributes -std=c11 -E %s -o -
> | FileCheck %s
> +// RUN: %clang_cc1 -std=c2x -E %s -o - | FileCheck %s
>
>  // CHECK: has_fallthrough
>  #if __has_c_attribute(fallthrough)
> @@ -14,3 +15,8 @@
>  #if __has_c_attribute(__nodiscard__)
>int has_nodiscard_underscore();
>  #endif
> +
> +// CHECK: has_clang_annotate
> +#if __has_c_attribute(clang::annotate)
> +  int has_clang_annotate();
> +#endif
>
> diff  --git a/clang/test/Preprocessor/has_c_attribute.cpp
> b/clang/test/Preprocessor/has_c_attribute.cpp
> new file mode 100644
> index ..0bde73067178
> --- /dev/null
> +++ b/clang/test/Preprocessor/has_c_attribute.cpp
> @@ -0,0 +1,8 @@
> +// RUN: %clang_cc1 -std=c++11 %s -verify
> +
> +#if __has_c_attribute(fallthrough) // expected-error {{function-like
> macro '__has_c_attribute' is not defined}}
> +#endif
> +
> +#if __has_c_attribute(gnu::transparent_union) // expected-error
> {{function-like macro '__has_c_attribute' is not defined}}
> +#endif
> +
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 90b8bc0 - IRGen: Call SetLLVMFunctionAttributes{, ForDefinition} on __cfi_check_fail.

[Peter Collingbourne via cfe-commits]

Author: Peter Collingbourne
Date: 2019-11-25T15:16:43-08:00
New Revision: 90b8bc003caacd165dedbb9cafc32de10d610ea7

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

LOG: IRGen: Call SetLLVMFunctionAttributes{,ForDefinition} on __cfi_check_fail.

This has the main effect of causing target-cpu and target-features to be set
on __cfi_check_fail, causing the function to become ABI-compatible with other
functions in the case where these attributes affect ABI (e.g. reserve-x18).

Technically we only need to call SetLLVMFunctionAttributes to get the target-*
attributes set, but since we're creating a definition we probably ought to
call the ForDefinition function as well.

Fixes PR44094.

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

Added: 
clang/test/CodeGen/cfi-check-fail-attrs.c

Modified: 
clang/lib/CodeGen/CGExpr.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 214378a966f0..04c6504910b8 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -3200,6 +3200,9 @@ void CodeGenFunction::EmitCfiCheckFail() {
   llvm::Function *F = llvm::Function::Create(
   llvm::FunctionType::get(VoidTy, {VoidPtrTy, VoidPtrTy}, false),
   llvm::GlobalValue::WeakODRLinkage, "__cfi_check_fail", &CGM.getModule());
+
+  CGM.SetLLVMFunctionAttributes(GlobalDecl(), FI, F);
+  CGM.SetLLVMFunctionAttributesForDefinition(nullptr, F);
   F->setVisibility(llvm::GlobalValue::HiddenVisibility);
 
   StartFunction(GlobalDecl(), CGM.getContext().VoidTy, F, FI, Args,

diff  --git a/clang/test/CodeGen/cfi-check-fail-attrs.c 
b/clang/test/CodeGen/cfi-check-fail-attrs.c
new file mode 100644
index ..77ba29a5581c
--- /dev/null
+++ b/clang/test/CodeGen/cfi-check-fail-attrs.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -triple aarch64-unknown-linux -fsanitize-cfi-cross-dso 
-target-feature +reserve-x18 -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: define weak_odr hidden void @__cfi_check_fail{{.*}} [[ATTR:#[0-9]*]]
+
+// CHECK: attributes [[ATTR]] = {{.*}} "target-features"="+reserve-x18"



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