[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D71499#1800636 , @arichardson wrote:

> Address feedback: Avoid inttoptr by using ptrtoint + GEP instead.
>
> Using two GEPs for align_up seems to generate better code than select + a 
> single GEP.
>  See https://godbolt.org/z/UdPjZk


Avoiding `select` is good, true. But we can do better with a single `gep` still:
https://godbolt.org/z/u7YHKw


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D72066: [clangd] Assert that the testcases in LocateSymbol.All have no diagnostics

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a reviewer: kadircet.
kadircet added a comment.

thanks for taking a look at this!




Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:447
 struct X {
-  X& [[operator]]++() {}
+  X& [[operator]]++() { return *this; }
 };

nit: I would rather turn this into a declaration.



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:532
 
+// Disable warnings which some testcases intentionally trigger,
+// so that we can assert the testcases have no diagnostics and

could we rather move these cases into a new test case?

in order to prevent accidental reliance on these flags when adding new tests.



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:542
+for (auto &Diag : AST.getDiagnostics()) {
+  llvm::errs() << Diag << "\n";
+}

use `ADD_FAILURE` instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72066



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

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 235837.
ilya-biryukov added a comment.

- Use DependencyFlagsBits for computing NumExprBits
- Reformat


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71920

Files:
  clang-tools-extra/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
  clang/include/clang/AST/DependencyFlags.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/TemplateBase.h
  clang/include/clang/AST/Type.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/ExprObjC.cpp
  clang/lib/AST/TemplateBase.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp

Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -511,10 +511,23 @@
 void ASTStmtReader::VisitExpr(Expr *E) {
   VisitStmt(E);
   E->setType(Record.readType());
-  E->setTypeDependent(Record.readInt());
-  E->setValueDependent(Record.readInt());
-  E->setInstantiationDependent(Record.readInt());
-  E->ExprBits.ContainsUnexpandedParameterPack = Record.readInt();
+
+  // FIXME: write and read all DependentFlags with a single call.
+  bool TypeDependent = Record.readInt();
+  bool ValueDependent = Record.readInt();
+  bool InstantiationDependent = Record.readInt();
+  bool ContainsUnexpandedTemplateParameters = Record.readInt();
+  DependencyFlags Deps = DependencyFlags::None;
+  if (TypeDependent)
+Deps = Deps | DependencyFlags::Type;
+  if (ValueDependent)
+Deps = Deps | DependencyFlags::Value;
+  if (InstantiationDependent)
+Deps = Deps | DependencyFlags::Instantiation;
+  if (ContainsUnexpandedTemplateParameters)
+Deps = Deps | DependencyFlags::UnexpandedPack;
+  E->setDependencies(Deps);
+
   E->setValueKind(static_cast(Record.readInt()));
   E->setObjectKind(static_cast(Record.readInt()));
   assert(Record.getIdx() == NumExprFields &&
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -12400,9 +12400,7 @@
   // base classes.
   CallExpr *CE = CallExpr::Create(Context, Fn, Args, Context.DependentTy,
   VK_RValue, RParenLoc);
-  CE->setTypeDependent(true);
-  CE->setValueDependent(true);
-  CE->setInstantiationDependent(true);
+  CE->addDependencies(DependencyFlags::TypeValueInstantiation);
   *Result = CE;
   return true;
 }
Index: clang/lib/AST/TemplateBase.cpp
===
--- clang/lib/AST/TemplateBase.cpp
+++ clang/lib/AST/TemplateBase.cpp
@@ -111,84 +111,65 @@
   return TemplateArgument(Args.copy(Context));
 }
 
-bool TemplateArgument::isDependent() const {
+DependencyFlags TemplateArgument::getDependencies() const {
+  DependencyFlags Deps = DependencyFlags::None;
   switch (getKind()) {
   case Null:
 llvm_unreachable("Should not have a NULL template argument");
 
   case Type:
-return getAsType()->isDependentType() ||
-   isa(getAsType());
+Deps = getAsType()->getDependencies();
+if (isa(getAsType()))
+  Deps = Deps | DependencyFlags::Type;
+return Deps;
 
   case Template:
-return getAsTemplate().isDependent();
+// FIXME: add TemplateName::getDependencies.
+if (getAsTemplate().isDependent())
+  Deps = Deps | DependencyFlags::TypeValue;
+if (getAsTemplate().isInstantiationDependent())
+  Deps = Deps | DependencyFlags::Instantiation;
+if (getAsTemplate().containsUnexpandedParameterPack())
+  Deps = Deps | DependencyFlags::UnexpandedPack;
+return Deps;
 
   case TemplateExpansion:
-return true;
+return DependencyFlags::TypeValueInstantiation;
 
-  case Declaration:
-if (DeclContext *DC = dyn_cast(getAsDecl()))
-  return DC->isDependentContext();
-return getAsDecl()->getDeclContext()->isDependentContext();
+  case Declaration: {
+auto *DC = dyn_cast(getAsDecl());
+if (!DC)
+  DC = getAsDecl()->getDeclContext();
+if (DC->isDependentContext())
+  Deps = DependencyFlags::TypeValueInstantiation;
+return Deps;
+  }
 
   case NullPtr:
-return false;
-
   case Integral:
-// Never dependent
-return false;
+return DependencyFlags::None;
 
   case Expression:
-return (getAsExpr()->isTypeDependent() || getAsExpr()->isValueDependent() ||
-isa(getAsExpr()));
+Deps = getAsExpr()->getDependencies();
+if (isa(getAsExpr()))
+  Deps = Deps | DependencyFlags::TypeValueInstantiation;
+return Deps;
 
   case Pack:
 for (const auto &P : pack_elements())
-  if (P.isDependent())
-return true;
-return false;
+  Deps = Deps | P.getDependencies();
+return Dep

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked 4 inline comments as done.
ilya-biryukov added inline comments.



Comment at: clang/include/clang/AST/Expr.h:126
+if (TD)
+  D = D | DependencyFlags::Type;
+if (VD)

Mordante wrote:
> Just curious why do you prefer `D = D | DependencyFlags::Type;` over `D |= 
> DependencyFlags::Type;` ? The latter seems to be more common.
Would also prefer `D |=`, but it leads to compilation errors.

The builtin `operator |=` accepts ints and, therefore, fails on strongly-typed 
enums.
And, AFAIK, there's no way to redefine `operator |=` for non-class types.



Comment at: clang/include/clang/AST/Stmt.h:323
   };
   enum { NumExprBits = NumStmtBits + 9 };
 

Mordante wrote:
> Please use `enum { NumExprBits = NumStmtBits + 5 +  DependencyFlagsBits };` 
> to avoid bugs when the size changes.
Many thanks! Using it here was the reason I wanted to add the constant in the 
first place. Ended up forgetting about it.
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71920



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

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14318
+  }
+  // Negate the mask to only clear the lower bits.
+  llvm::Value *Result;

But this isn't what we are doing, negation != inversion.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

[Jonas Toth via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:66
+  StringRef S = "MyInt target = 0;";
+  auto Cat = [&T](StringRef S) { return (T + S).str(); };
+

aaron.ballman wrote:
> I don't think this is needed -- just add a newline between the literals and 
> let string concat work its magic? (Same elsewhere)
> ```
> StringRef S = "typedef int MyInt;"
>   "MyInt target = 0;";
> ```
My Idea was to highlight the expected code-change and create both the before 
and after-version from the same snippets.
With `S` being all the code, i would need to test for the full code.

I think with your proposal the test looks like this:

```
StringRef S = "typedef int MyInt;"
  "MyInt target = 0;";
EXPECT_EQ("typedef int MyInt;"
  "const MyInt target = 0;",
  runCheckOnCode(S));
```
I prefer the current version, especially with the bigger test later this file.



Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:83-84
+
+  EXPECT_EQ(Cat("const MyInt target = nullptr;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("const MyInt target = nullptr;"),

aaron.ballman wrote:
> This does what I would expect, but boy does it make me sad to see (because 
> `target` is not actually a `const int *` but is instead an `int * const`).
You are right, but I am not sure what the proper policy should be. I think the 
smarts to detect such potential improper const should be in the library code 
calling the transformation.
The utility should allow both transformations.



Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:1006
+}
+
+} // namespace test

aaron.ballman wrote:
> Can you also add some ObjC pointer tests?
i added the most basic test, do you think more is necessary? I don't know a lot 
about the objc languages.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54395



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

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 235838.
JonasToth marked 3 inline comments as done.
JonasToth added a comment.

- address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54395

Files:
  clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
  clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
  clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
  clang-tools-extra/clang-tidy/utils/LexerUtils.h
  clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp
  clang-tools-extra/unittests/clang-tidy/CMakeLists.txt

Index: clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
===
--- clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
+++ clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
@@ -7,6 +7,7 @@
 include_directories(${CLANG_LINT_SOURCE_DIR})
 
 add_extra_unittest(ClangTidyTests
+  AddConstTest.cpp
   ClangTidyDiagnosticConsumerTest.cpp
   ClangTidyOptionsTest.cpp
   IncludeInserterTest.cpp
Index: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp
===
--- /dev/null
+++ clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp
@@ -0,0 +1,1047 @@
+#include "../clang-tidy/utils/FixItHintUtils.h"
+#include "ClangTidyTest.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tidy {
+
+namespace {
+using namespace clang::ast_matchers;
+using namespace utils::fixit;
+
+template 
+class ConstTransform : public ClangTidyCheck {
+public:
+  ConstTransform(StringRef CheckName, ClangTidyContext *Context)
+  : ClangTidyCheck(CheckName, Context) {}
+
+  void registerMatchers(MatchFinder *Finder) override {
+Finder->addMatcher(varDecl(hasName("target")).bind("var"), this);
+  }
+
+  void check(const MatchFinder::MatchResult &Result) override {
+const auto *D = Result.Nodes.getNodeAs("var");
+using utils::fixit::addQualifierToVarDecl;
+Optional Fix = addQualifierToVarDecl(
+*D, *Result.Context, DeclSpec::TQ::TQ_const, CT, CP);
+auto Diag = diag(D->getBeginLoc(), "doing const transformation");
+if (Fix)
+  Diag << *Fix;
+  }
+};
+} // namespace
+
+namespace test {
+using PointeeLTransform =
+ConstTransform;
+using PointeeRTransform =
+ConstTransform;
+
+using ValueLTransform =
+ConstTransform;
+using ValueRTransform =
+ConstTransform;
+
+// 
+// Test Value-like types. Everything with indirection is done later.
+// 
+
+TEST(Values, Builtin) {
+  StringRef Snippet = "int target = 0;";
+
+  EXPECT_EQ("const int target = 0;", runCheckOnCode(Snippet));
+  EXPECT_EQ("const int target = 0;",
+runCheckOnCode(Snippet));
+
+  EXPECT_EQ("int const target = 0;", runCheckOnCode(Snippet));
+  EXPECT_EQ("int const target = 0;",
+runCheckOnCode(Snippet));
+}
+TEST(Values, TypedefBuiltin) {
+  StringRef T = "typedef int MyInt;";
+  StringRef S = "MyInt target = 0;";
+  auto Cat = [&T](StringRef S) { return (T + S).str(); };
+
+  EXPECT_EQ(Cat("const MyInt target = 0;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("const MyInt target = 0;"),
+runCheckOnCode(Cat(S)));
+
+  EXPECT_EQ(Cat("MyInt const target = 0;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("MyInt const target = 0;"),
+runCheckOnCode(Cat(S)));
+}
+TEST(Values, TypedefBuiltinPointer) {
+  StringRef T = "typedef int* MyInt;";
+  StringRef S = "MyInt target = nullptr;";
+  auto Cat = [&T](StringRef S) { return (T + S).str(); };
+
+  EXPECT_EQ(Cat("const MyInt target = nullptr;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("const MyInt target = nullptr;"),
+runCheckOnCode(Cat(S)));
+
+  EXPECT_EQ(Cat("MyInt const target = nullptr;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("MyInt const target = nullptr;"),
+runCheckOnCode(Cat(S)));
+}
+TEST(Values, UsingBuiltin) {
+  StringRef T = "using MyInt = int;";
+  StringRef S = "MyInt target = 0;";
+  auto Cat = [&T](StringRef S) { return (T + S).str(); };
+
+  EXPECT_EQ(Cat("const MyInt target = 0;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("const MyInt target = 0;"),
+runCheckOnCode(Cat(S)));
+
+  EXPECT_EQ(Cat("MyInt const target = 0;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("MyInt const target = 0;"),
+runCheckOnCode(Cat(S)));
+}
+TEST(Values, UsingBuiltinPointer) {
+  String

[PATCH] D72071: [clangd] Add correctness checks for index-based rename

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay.
Herald added a project: clang.

In some cases the candidate ranges for rename final stage (textual
replacements) are invalid and do not contain references to identifier being
renamed. Examples of such behavior include implicit references that are
currently not filtered out (though in the future they should be dealt with
during the references collection stage).

This patch addresses this issue by explicitly checking whether the text in each
candidate range is equivalent to the renamed identifier's name. It does not make
index-based rename absolutely correct, but it is a cheap way to filter out some
replacements that are clearly incorrect.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72071

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/refactor/Rename.h
  clang-tools-extra/clangd/unittests/RenameTests.cpp

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -795,6 +795,7 @@
 
 void func() {
   [[Foo]] foo;
+  foo.~[[Foo]];
 }
   )cpp",
   },
@@ -868,6 +869,20 @@
 }
   )cpp",
   },
+  {
+  // Macros and implicit references.
+  R"cpp(
+class [[Fo^o]] {};
+#define FooFoo Foo
+  )cpp",
+  R"cpp(
+#include "foo.h"
+void bar() {
+  [[Foo]] foo;
+  FooFoo z;
+}
+  )cpp",
+  },
   };
 
   for (const auto& T : Cases) {
@@ -920,7 +935,7 @@
   auto LSPRange = Code.range();
   llvm::StringRef FilePath = "/test/TestTU.cpp";
   llvm::StringRef NewName = "abc";
-  auto Edit = buildRenameEdit(FilePath, Code.code(), {LSPRange}, NewName);
+  auto Edit = buildRenameEdit(FilePath, Code.code(), {LSPRange}, NewName, "😂");
   ASSERT_TRUE(bool(Edit)) << Edit.takeError();
   ASSERT_EQ(1UL, Edit->Replacements.size());
   EXPECT_EQ(FilePath, Edit->Replacements.begin()->getFilePath());
@@ -928,7 +943,7 @@
 
   // Test invalid range.
   LSPRange.end = {10, 0}; // out of range
-  Edit = buildRenameEdit(FilePath, Code.code(), {LSPRange}, NewName);
+  Edit = buildRenameEdit(FilePath, Code.code(), {LSPRange}, NewName, "😂");
   EXPECT_FALSE(Edit);
   EXPECT_THAT(llvm::toString(Edit.takeError()),
   testing::HasSubstr("fail to convert"));
@@ -939,7 +954,7 @@
   [[range]]
   [[range]]
   )cpp");
-  Edit = buildRenameEdit(FilePath, T.code(), T.ranges(), NewName);
+  Edit = buildRenameEdit(FilePath, T.code(), T.ranges(), NewName, "range");
   ASSERT_TRUE(bool(Edit)) << Edit.takeError();
   EXPECT_EQ(applyEdits(FileEdits{{T.code(), std::move(*Edit)}}).front().second,
 expectedResult(T, NewName));
@@ -1013,11 +1028,6 @@
 llvm::StringRef IndexedCode;
 llvm::StringRef LexedCode;
   } Tests[] = {
-{
-  // no lexed ranges.
-  "[[]]",
-  "",
-},
 {
   // both line and column are changed, not a near miss.
   R"([[]])",
Index: clang-tools-extra/clangd/refactor/Rename.h
===
--- clang-tools-extra/clangd/refactor/Rename.h
+++ clang-tools-extra/clangd/refactor/Rename.h
@@ -55,7 +55,8 @@
 llvm::Expected buildRenameEdit(llvm::StringRef AbsFilePath,
  llvm::StringRef InitialCode,
  std::vector Occurrences,
- llvm::StringRef NewName);
+ llvm::StringRef NewName,
+ llvm::StringRef OldName);
 
 /// Adjusts indexed occurrences to match the current state of the file.
 ///
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -374,7 +374,8 @@
   llvm::inconvertibleErrorCode());
 }
 auto RenameEdit =
-buildRenameEdit(FilePath, *AffectedFileCode, *RenameRanges, NewName);
+buildRenameEdit(FilePath, *AffectedFileCode, *RenameRanges, NewName,
+RenameDecl.getNameAsString());
 if (!RenameEdit) {
   return llvm::make_error(
   llvm::formatv("fail to build rename edit for file {0}: {1}", FilePath,
@@ -522,7 +523,8 @@
 llvm::Expected buildRenameEdit(llvm::StringRef AbsFilePath,
  llvm::StringRef InitialCode,
  std::vector Occurrences,
- llvm::StringRef NewName) {
+ llvm::StringRef NewName,
+ llvm::StringRef OldName) 

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

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

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61158 tests passed, 0 failed 
and 728 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71920



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

[PATCH] D72072: [AST] Respect shouldTraversePostOrder when traversing type locs

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: gribozavr2.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72072

Files:
  clang/include/clang/AST/RecursiveASTVisitor.h


Index: clang/include/clang/AST/RecursiveASTVisitor.h
===
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -1115,10 +1115,17 @@
 #define DEF_TRAVERSE_TYPELOC(TYPE, CODE)   
\
   template   
\
   bool RecursiveASTVisitor::Traverse##TYPE##Loc(TYPE##Loc TL) {   
\
-if (getDerived().shouldWalkTypesOfTypeLocs())  
\
-  TRY_TO(WalkUpFrom##TYPE(const_cast(TL.getTypePtr(;   
\
-TRY_TO(WalkUpFrom##TYPE##Loc(TL)); 
\
+if (!getDerived().shouldTraversePostOrder()) { 
\
+  TRY_TO(WalkUpFrom##TYPE##Loc(TL));   
\
+  if (getDerived().shouldWalkTypesOfTypeLocs())
\
+TRY_TO(WalkUpFrom##TYPE(const_cast(TL.getTypePtr(; 
\
+}  
\
 { CODE; }  
\
+if (getDerived().shouldTraversePostOrder()) {  
\
+  TRY_TO(WalkUpFrom##TYPE##Loc(TL));   
\
+  if (getDerived().shouldWalkTypesOfTypeLocs())
\
+TRY_TO(WalkUpFrom##TYPE(const_cast(TL.getTypePtr(; 
\
+}  
\
 return true;   
\
   }
 
@@ -1187,22 +1194,22 @@
 
 DEF_TRAVERSE_TYPELOC(ConstantArrayType, {
   TRY_TO(TraverseTypeLoc(TL.getElementLoc()));
-  return TraverseArrayTypeLocHelper(TL);
+  TRY_TO(TraverseArrayTypeLocHelper(TL));
 })
 
 DEF_TRAVERSE_TYPELOC(IncompleteArrayType, {
   TRY_TO(TraverseTypeLoc(TL.getElementLoc()));
-  return TraverseArrayTypeLocHelper(TL);
+  TRY_TO(TraverseArrayTypeLocHelper(TL));
 })
 
 DEF_TRAVERSE_TYPELOC(VariableArrayType, {
   TRY_TO(TraverseTypeLoc(TL.getElementLoc()));
-  return TraverseArrayTypeLocHelper(TL);
+  TRY_TO(TraverseArrayTypeLocHelper(TL));
 })
 
 DEF_TRAVERSE_TYPELOC(DependentSizedArrayType, {
   TRY_TO(TraverseTypeLoc(TL.getElementLoc()));
-  return TraverseArrayTypeLocHelper(TL);
+  TRY_TO(TraverseArrayTypeLocHelper(TL));
 })
 
 DEF_TRAVERSE_TYPELOC(DependentAddressSpaceType, {


Index: clang/include/clang/AST/RecursiveASTVisitor.h
===
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -1115,10 +1115,17 @@
 #define DEF_TRAVERSE_TYPELOC(TYPE, CODE)   \
   template   \
   bool RecursiveASTVisitor::Traverse##TYPE##Loc(TYPE##Loc TL) {   \
-if (getDerived().shouldWalkTypesOfTypeLocs())  \
-  TRY_TO(WalkUpFrom##TYPE(const_cast(TL.getTypePtr(;   \
-TRY_TO(WalkUpFrom##TYPE##Loc(TL)); \
+if (!getDerived().shouldTraversePostOrder()) { \
+  TRY_TO(WalkUpFrom##TYPE##Loc(TL));   \
+  if (getDerived().shouldWalkTypesOfTypeLocs())\
+TRY_TO(WalkUpFrom##TYPE(const_cast(TL.getTypePtr(; \
+}  \
 { CODE; }  \
+if (getDerived().shouldTraversePostOrder()) {  \
+  TRY_TO(WalkUpFrom##TYPE##Loc(TL));   \
+  if (getDerived().shouldWalkTypesOfTypeLocs())\
+TRY_TO(WalkUpFrom##TYPE(const_cast(TL.getTypePtr(; \
+}  \
 return true;   \
   }
 
@@ -1187,22 +1194,22 @@
 
 DEF_TRAVERSE_TYPELOC(ConstantArrayType, {
   TRY_TO(TraverseTypeLoc(TL.getElementLoc()));
-  return TraverseArrayTypeLocHelper(TL);
+  TRY_TO(TraverseArrayTypeLocHelper(TL));
 })
 
 DEF_TRAVERSE_TYPELOC(IncompleteArrayType, {
   TRY_TO(TraverseTypeLoc(TL.getElementLoc()));
-  return TraverseArrayTypeLocHelper(TL);
+  TRY_TO(TraverseArrayTypeLocHelper(TL));
 })
 
 DEF_TRAVERSE_TYPELOC(VariableArrayType, {
   TRY_TO(TraverseTypeLoc(TL.getElementLoc()));
-  return TraverseArrayTypeLocHelper(TL);
+  TRY_TO(TraverseArrayTypeLocHelper(TL));
 }

[PATCH] D72072: [AST] Respect shouldTraversePostOrder when traversing type locs

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Would be nice to write a test too, but didn't get to it yet...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72072



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

[PATCH] D72071: [clangd] Add correctness checks for index-based rename

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Improve wording in the comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72071

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/refactor/Rename.h
  clang-tools-extra/clangd/unittests/RenameTests.cpp

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -795,6 +795,7 @@
 
 void func() {
   [[Foo]] foo;
+  foo.~[[Foo]];
 }
   )cpp",
   },
@@ -868,6 +869,20 @@
 }
   )cpp",
   },
+  {
+  // Macros and implicit references.
+  R"cpp(
+class [[Fo^o]] {};
+#define FooFoo Foo
+  )cpp",
+  R"cpp(
+#include "foo.h"
+void bar() {
+  [[Foo]] foo;
+  FooFoo z;
+}
+  )cpp",
+  },
   };
 
   for (const auto& T : Cases) {
@@ -920,7 +935,7 @@
   auto LSPRange = Code.range();
   llvm::StringRef FilePath = "/test/TestTU.cpp";
   llvm::StringRef NewName = "abc";
-  auto Edit = buildRenameEdit(FilePath, Code.code(), {LSPRange}, NewName);
+  auto Edit = buildRenameEdit(FilePath, Code.code(), {LSPRange}, NewName, "😂");
   ASSERT_TRUE(bool(Edit)) << Edit.takeError();
   ASSERT_EQ(1UL, Edit->Replacements.size());
   EXPECT_EQ(FilePath, Edit->Replacements.begin()->getFilePath());
@@ -928,7 +943,7 @@
 
   // Test invalid range.
   LSPRange.end = {10, 0}; // out of range
-  Edit = buildRenameEdit(FilePath, Code.code(), {LSPRange}, NewName);
+  Edit = buildRenameEdit(FilePath, Code.code(), {LSPRange}, NewName, "😂");
   EXPECT_FALSE(Edit);
   EXPECT_THAT(llvm::toString(Edit.takeError()),
   testing::HasSubstr("fail to convert"));
@@ -939,7 +954,7 @@
   [[range]]
   [[range]]
   )cpp");
-  Edit = buildRenameEdit(FilePath, T.code(), T.ranges(), NewName);
+  Edit = buildRenameEdit(FilePath, T.code(), T.ranges(), NewName, "range");
   ASSERT_TRUE(bool(Edit)) << Edit.takeError();
   EXPECT_EQ(applyEdits(FileEdits{{T.code(), std::move(*Edit)}}).front().second,
 expectedResult(T, NewName));
@@ -1013,11 +1028,6 @@
 llvm::StringRef IndexedCode;
 llvm::StringRef LexedCode;
   } Tests[] = {
-{
-  // no lexed ranges.
-  "[[]]",
-  "",
-},
 {
   // both line and column are changed, not a near miss.
   R"([[]])",
Index: clang-tools-extra/clangd/refactor/Rename.h
===
--- clang-tools-extra/clangd/refactor/Rename.h
+++ clang-tools-extra/clangd/refactor/Rename.h
@@ -55,7 +55,8 @@
 llvm::Expected buildRenameEdit(llvm::StringRef AbsFilePath,
  llvm::StringRef InitialCode,
  std::vector Occurrences,
- llvm::StringRef NewName);
+ llvm::StringRef NewName,
+ llvm::StringRef OldName);
 
 /// Adjusts indexed occurrences to match the current state of the file.
 ///
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -374,7 +374,8 @@
   llvm::inconvertibleErrorCode());
 }
 auto RenameEdit =
-buildRenameEdit(FilePath, *AffectedFileCode, *RenameRanges, NewName);
+buildRenameEdit(FilePath, *AffectedFileCode, *RenameRanges, NewName,
+RenameDecl.getNameAsString());
 if (!RenameEdit) {
   return llvm::make_error(
   llvm::formatv("fail to build rename edit for file {0}: {1}", FilePath,
@@ -522,7 +523,8 @@
 llvm::Expected buildRenameEdit(llvm::StringRef AbsFilePath,
  llvm::StringRef InitialCode,
  std::vector Occurrences,
- llvm::StringRef NewName) {
+ llvm::StringRef NewName,
+ llvm::StringRef OldName) {
   assert(std::is_sorted(Occurrences.begin(), Occurrences.end()));
   assert(std::unique(Occurrences.begin(), Occurrences.end()) ==
  Occurrences.end() &&
@@ -557,7 +559,12 @@
 auto EndOffset = Offset(R.end);
 if (!EndOffset)
   return EndOffset.takeError();
-OccurrencesOffsets.push_back({*StartOffset, *EndOffset});
+// FIXME: We should not generate invalid replacements in the first place,
+// but it currently happens for implicit references and in some complicated
+// cases where the might be incorrectly captured tokens.
+if (*EndOffset > *StartOffset &&
+Initi

[PATCH] D72071: [clangd] Add correctness checks for index-based rename

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

@sammccall `Indexed.size() > Lexed.size()` is one of the assumptions that I 
think might not hold in real-world scenarios. I was reading patch heuristics 
code a lot and trying to understand whether anything breaks when this check is 
not in place, but I couldn't come up with some cases, so I thought it'd be OK 
to remove it.

I have written down some thoughts on the topic in the issue I opened on GitHub:

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

We have briefly discussed range patching heuristics in D71598 
, @kadircet has more context if you are 
interested.

I'd be happy to discuss more details once I'm back, let me know if you have any 
thoughts/comments on the issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72071



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

[PATCH] D72071: [clangd] Add correctness checks for index-based rename

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

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61168 tests passed, 0 failed 
and 728 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72071



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

[PATCH] D72072: [AST] Respect shouldTraversePostOrder when traversing type locs

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

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61158 tests passed, 0 failed 
and 728 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72072



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

[PATCH] D72071: [clangd] Add correctness checks for index-based rename

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

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61168 tests passed, 0 failed 
and 728 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72071



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

[PATCH] D72073: [Sema] Fix location of star ('*') inside MemberPointerTypeLoc

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: gribozavr2.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72073

Files:
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaType.cpp


Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5691,7 +5691,7 @@
   }
 
   // Finally fill in MemberPointerLocInfo fields.
-  TL.setStarLoc(Chunk.Loc);
+  TL.setStarLoc(SourceLocation::getFromRawEncoding(Chunk.Mem.StarLoc));
   TL.setClassTInfo(ClsTInfo);
 }
 void VisitLValueReferenceTypeLoc(LValueReferenceTypeLoc TL) {
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -5552,6 +5552,7 @@
 return;
   }
 
+  SourceLocation StarLoc = Tok.getLocation();
   SourceLocation Loc = ConsumeToken();
   D.SetRangeEnd(Loc);
   DeclSpec DS(AttrFactory);
@@ -5564,7 +5565,7 @@
   // Sema will have to catch (syntactically invalid) pointers into global
   // scope. It has to catch pointers into namespace scope anyway.
   D.AddTypeInfo(DeclaratorChunk::getMemberPointer(
-SS, DS.getTypeQualifiers(), DS.getEndLoc()),
+SS, DS.getTypeQualifiers(), StarLoc, DS.getEndLoc()),
 std::move(DS.getAttributes()),
 /* Don't replace range end. */ SourceLocation());
   return;
Index: clang/include/clang/Sema/DeclSpec.h
===
--- clang/include/clang/Sema/DeclSpec.h
+++ clang/include/clang/Sema/DeclSpec.h
@@ -1503,6 +1503,8 @@
   struct MemberPointerTypeInfo {
 /// The type qualifiers: const/volatile/restrict/__unaligned/_Atomic.
 unsigned TypeQuals : 5;
+/// Location of the '*' token.
+unsigned StarLoc;
 // CXXScopeSpec has a constructor, so it can't be a direct member.
 // So we need some pointer-aligned storage and a bit of trickery.
 alignas(CXXScopeSpec) char ScopeMem[sizeof(CXXScopeSpec)];
@@ -1645,11 +1647,13 @@
 
   static DeclaratorChunk getMemberPointer(const CXXScopeSpec &SS,
   unsigned TypeQuals,
-  SourceLocation Loc) {
+  SourceLocation StarLoc,
+  SourceLocation EndLoc) {
 DeclaratorChunk I;
 I.Kind  = MemberPointer;
 I.Loc   = SS.getBeginLoc();
-I.EndLoc= Loc;
+I.EndLoc= EndLoc;
+I.Mem.StarLoc   = StarLoc.getRawEncoding();
 I.Mem.TypeQuals = TypeQuals;
 new (I.Mem.ScopeMem) CXXScopeSpec(SS);
 return I;


Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5691,7 +5691,7 @@
   }
 
   // Finally fill in MemberPointerLocInfo fields.
-  TL.setStarLoc(Chunk.Loc);
+  TL.setStarLoc(SourceLocation::getFromRawEncoding(Chunk.Mem.StarLoc));
   TL.setClassTInfo(ClsTInfo);
 }
 void VisitLValueReferenceTypeLoc(LValueReferenceTypeLoc TL) {
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -5552,6 +5552,7 @@
 return;
   }
 
+  SourceLocation StarLoc = Tok.getLocation();
   SourceLocation Loc = ConsumeToken();
   D.SetRangeEnd(Loc);
   DeclSpec DS(AttrFactory);
@@ -5564,7 +5565,7 @@
   // Sema will have to catch (syntactically invalid) pointers into global
   // scope. It has to catch pointers into namespace scope anyway.
   D.AddTypeInfo(DeclaratorChunk::getMemberPointer(
-SS, DS.getTypeQualifiers(), DS.getEndLoc()),
+SS, DS.getTypeQualifiers(), StarLoc, DS.getEndLoc()),
 std::move(DS.getAttributes()),
 /* Don't replace range end. */ SourceLocation());
   return;
Index: clang/include/clang/Sema/DeclSpec.h
===
--- clang/include/clang/Sema/DeclSpec.h
+++ clang/include/clang/Sema/DeclSpec.h
@@ -1503,6 +1503,8 @@
   struct MemberPointerTypeInfo {
 /// The type qualifiers: const/volatile/restrict/__unaligned/_Atomic.
 unsigned TypeQuals : 5;
+/// Location of the '*' token.
+unsigned StarLoc;
 // CXXScopeSpec has a constructor, so it can't be a direct member.
 // So we need some pointer-aligned storage and a bit of trickery.
 alignas(CXXScopeSpec) char ScopeMem[sizeof(CXXScopeSpec)];
@@ -1645,11 +1647,13 @@
 
   static DeclaratorChunk getMemberPointer(const 

[PATCH] D72073: [Sema] Fix location of star ('*') inside MemberPointerTypeLoc

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

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61158 tests passed, 0 failed 
and 728 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72073



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

[PATCH] D71982: [docs] Update path to clang-tools-extra

[Alexander Lanin via Phabricator via cfe-commits]

AlexanderLanin added a comment.

@Jim `git commit --amend --author="Alexander Lanin "`


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

https://reviews.llvm.org/D71982



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

[clang-tools-extra] 8188c99 - [docs] Update path to clang-tools-extra

[Jim Lin via cfe-commits]

Author: Alexander Lanin
Date: 2020-01-02T19:30:29+08:00
New Revision: 8188c998ffa4d20253444b257402907d2aa74dc2

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

LOG: [docs] Update path to clang-tools-extra

Summary:
> tools/clang/tools/extra
has become
>clang-tools-extra
which was not updated in all docs.

Reviewers: alexfh, aaron.ballman, ilya-biryukov, juliehockett

Reviewed By: aaron.ballman

Subscribers: Jim, cfe-commits

Tags: #clang-tools-extra, #clang

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

Added: 


Modified: 
clang-tools-extra/docs/clang-include-fixer.rst
clang-tools-extra/docs/clang-tidy/Contributing.rst
clang-tools-extra/docs/pp-trace.rst

Removed: 




diff  --git a/clang-tools-extra/docs/clang-include-fixer.rst 
b/clang-tools-extra/docs/clang-include-fixer.rst
index b934095d8e74..7d1fd9ed70e7 100644
--- a/clang-tools-extra/docs/clang-include-fixer.rst
+++ b/clang-tools-extra/docs/clang-include-fixer.rst
@@ -49,7 +49,7 @@ database for LLVM, any project built by CMake should follow 
similar steps.
   $ ninja clang-include-fixer // build clang-include-fixer tool.
   $ ls compile_commands.json # Make sure compile_commands.json exists.
 compile_commands.json
-  $ 
path/to/llvm/source/tools/clang/tools/extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
+  $ 
path/to/llvm/source/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
 ... wait as clang indexes the code base ...
   $ ln -s $PWD/find_all_symbols_db.yaml path/to/llvm/source/ # Link database 
into the source tree.
   $ ln -s $PWD/compile_commands.json path/to/llvm/source/ # Also link 
compilation database if it's not there already.
@@ -64,7 +64,7 @@ following key binding to your ``.vimrc``:
 
 .. code-block:: console
 
-  noremap cf :pyf 
path/to/llvm/source/tools/clang/tools/extra/clang-include-fixer/tool/clang-include-fixer.py
+  noremap cf :pyf 
path/to/llvm/source/clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.py
 
 This enables `clang-include-fixer` for NORMAL and VISUAL mode. Change
 `cf` to another binding if you need clang-include-fixer on a 
diff erent
@@ -118,7 +118,7 @@ in your ``.emacs``:
 
 .. code-block:: console
 
- (add-to-list 'load-path 
"path/to/llvm/source/tools/clang/tools/extra/clang-include-fixer/tool/"
+ (add-to-list 'load-path 
"path/to/llvm/source/clang-tools-extra/clang-include-fixer/tool/"
  (require 'clang-include-fixer)
 
 Within Emacs the tool can be invoked with the command

diff  --git a/clang-tools-extra/docs/clang-tidy/Contributing.rst 
b/clang-tools-extra/docs/clang-tidy/Contributing.rst
index 09ff1f65c2c2..3ed6dadb5e8b 100644
--- a/clang-tools-extra/docs/clang-tidy/Contributing.rst
+++ b/clang-tools-extra/docs/clang-tidy/Contributing.rst
@@ -63,7 +63,7 @@ the LLVM System`_, `Using Clang Tools`_ and `How To Setup 
Clang Tooling For
 LLVM`_ documents to check out and build LLVM, Clang and Clang Extra Tools with
 CMake.
 
-Once you are done, change to the ``llvm/tools/clang/tools/extra`` directory, 
and
+Once you are done, change to the ``llvm/clang-tools-extra`` directory, and
 let's start!
 
 .. _Getting Started with the LLVM System: 
https://llvm.org/docs/GettingStarted.html
@@ -75,7 +75,7 @@ The Directory Structure
 ---
 
 :program:`clang-tidy` source code resides in the
-``llvm/tools/clang/tools/extra`` directory and is structured as follows:
+``llvm/clang-tools-extra`` directory and is structured as follows:
 
 ::
 

diff  --git a/clang-tools-extra/docs/pp-trace.rst 
b/clang-tools-extra/docs/pp-trace.rst
index b0930070dcca..60e4de461253 100644
--- a/clang-tools-extra/docs/pp-trace.rst
+++ b/clang-tools-extra/docs/pp-trace.rst
@@ -104,16 +104,16 @@ With real data:::
 
   ---
   - Callback: FileChanged
-Loc: 
"c:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-include.cpp:1:1"
+Loc: 
"c:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-include.cpp:1:1"
 Reason: EnterFile
 FileType: C_User
 PrevFID: (invalid)
 (etc.)
   - Callback: FileChanged
-Loc: 
"D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-include.cpp:5:1"
+Loc: 
"D:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-include.cpp:5:1"
 Reason: ExitFile
 FileType: C_User
-PrevFID: 
"D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/Input/Level1B.h"
+PrevFID: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/Input/Level1B.h"
   - Callback: EndOfMainFile
   ...
 
@@ -172,7 +172,7 @@ PrevFID  ((file)|(invalid)) 
  FileID
 Example:::
 
   - Callback: FileChanged
-Loc: 
"D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-include.cpp:1:1"
+Loc: 
"D:/Clang/llvm/clang-tools-extr

[PATCH] D71982: [docs] Update path to clang-tools-extra

[Jim Lin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8188c998ffa4: [docs] Update path to clang-tools-extra 
(authored by AlexanderLanin, committed by Jim).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71982

Files:
  clang-tools-extra/docs/clang-include-fixer.rst
  clang-tools-extra/docs/clang-tidy/Contributing.rst
  clang-tools-extra/docs/pp-trace.rst

Index: clang-tools-extra/docs/pp-trace.rst
===
--- clang-tools-extra/docs/pp-trace.rst
+++ clang-tools-extra/docs/pp-trace.rst
@@ -104,16 +104,16 @@
 
   ---
   - Callback: FileChanged
-Loc: "c:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-include.cpp:1:1"
+Loc: "c:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-include.cpp:1:1"
 Reason: EnterFile
 FileType: C_User
 PrevFID: (invalid)
 (etc.)
   - Callback: FileChanged
-Loc: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-include.cpp:5:1"
+Loc: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-include.cpp:5:1"
 Reason: ExitFile
 FileType: C_User
-PrevFID: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/Input/Level1B.h"
+PrevFID: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/Input/Level1B.h"
   - Callback: EndOfMainFile
   ...
 
@@ -172,7 +172,7 @@
 Example:::
 
   - Callback: FileChanged
-Loc: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-include.cpp:1:1"
+Loc: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-include.cpp:1:1"
 Reason: EnterFile
 FileType: C_User
 PrevFID: (invalid)
@@ -248,8 +248,8 @@
 FileName: "Input/Level1B.h"
 IsAngled: false
 FilenameRange: "Input/Level1B.h"
-File: "D:/Clang/llvmnewmod/tools/clang/tools/extra/test/pp-trace/Input/Level1B.h"
-SearchPath: "D:/Clang/llvmnewmod/tools/clang/tools/extra/test/pp-trace"
+File: "D:/Clang/llvmnewmod/clang-tools-extra/test/pp-trace/Input/Level1B.h"
+SearchPath: "D:/Clang/llvmnewmod/clang-tools-extra/test/pp-trace"
 RelativePath: "Input/Level1B.h"
 Imported: (null)
 
@@ -271,8 +271,8 @@
 Example:::
 
   - Callback: moduleImport
-ImportLoc: "d:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-modules.cpp:4:2"
-Path: [{Name: Level1B, Loc: "d:/Clang/llvmnewmod/tools/clang/tools/extra/test/pp-trace/pp-trace-modules.cpp:4:9"}, {Name: Level2B, Loc: "d:/Clang/llvmnewmod/tools/clang/tools/extra/test/pp-trace/pp-trace-modules.cpp:4:17"}]
+ImportLoc: "d:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-modules.cpp:4:2"
+Path: [{Name: Level1B, Loc: "d:/Clang/llvmnewmod/clang-tools-extra/test/pp-trace/pp-trace-modules.cpp:4:9"}, {Name: Level2B, Loc: "d:/Clang/llvmnewmod/clang-tools-extra/test/pp-trace/pp-trace-modules.cpp:4:17"}]
 Imported: Level2B
 
 `EndOfMainFile `_ Callback
@@ -309,7 +309,7 @@
 Example:::
 
   - Callback: Ident
-Loc: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-ident.cpp:3:1"
+Loc: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-ident.cpp:3:1"
 str: "$Id$"
 
 `PragmaDirective `_ Callback
@@ -329,7 +329,7 @@
 Example:::
 
   - Callback: PragmaDirective
-Loc: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
+Loc: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
 Introducer: PIK_HashPragma
 
 `PragmaComment `_ Callback
@@ -350,7 +350,7 @@
 Example:::
 
   - Callback: PragmaComment
-Loc: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
+Loc: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
 Kind: library
 Str: kernel32.lib
 
@@ -372,7 +372,7 @@
 Example:::
 
   - Callback: PragmaDetectMismatch
-Loc: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
+Loc: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
 Name: name
 Value: value
 
@@ -393,7 +393,7 @@
 Example:::
 
   - Callback: PragmaDebug
-Loc: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
+Loc: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
 DebugType: warning
 
 `PragmaMessage `_ Callback
@@ -415,7 +415,7 @@
 Example:::
 
   - Callback: PragmaMessage
-Loc: "D:/Clang/llvm/tools/clang/tools/extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
+Loc: "D:/Clang/llvm/clang-tools-extra/test/pp-trace/pp-trace-pragma.cpp:3:1"
 Na

[PATCH] D72071: [clangd] Add correctness checks for index-based rename

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:378
+buildRenameEdit(FilePath, *AffectedFileCode, *RenameRanges, NewName,
+RenameDecl.getNameAsString());
 if (!RenameEdit) {

nit: move both this and invocation above(line 363) out of the loop into a 
`std::string OldName`



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:565
+// cases where the might be incorrectly captured tokens.
+if (*EndOffset > *StartOffset &&
+InitialCode.slice(*StartOffset, *EndOffset) == OldName)

maybe I am missing it but, it is unclear whether start to end is 
half-open/closed intervals.
could you add a test case with a single character identifier(you might want to 
accept equality, if this is a closed interval) and add some comments to the 
`OccurrencesOffsets`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72071



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

[PATCH] D72076: [OpenCL] Add link to C++ for OpenCL documentation

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added a reviewer: svenvh.
Herald added subscribers: ebevhan, yaxunl.

Remove description of language mode from the language extensions and add a link 
to pdf document.


https://reviews.llvm.org/D72076

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/UsersManual.rst

Index: clang/docs/UsersManual.rst
===
--- clang/docs/UsersManual.rst
+++ clang/docs/UsersManual.rst
@@ -2856,6 +2856,10 @@
 
 Declaring the same types in different vendor extensions is disallowed.
 
+Clang also supports language extensions documented in `The OpenCL C Language
+Extensions Documentation
+`_.
+
 OpenCL Metadata
 ---
 
@@ -3031,8 +3035,9 @@
 `_ and
 there is no plan to support it in clang in any new releases in the near future.
 
-For detailed information about restrictions to allowed C++ features please
-refer to :doc:`LanguageExtensions`.
+For detailed information about this language refer to `The C++ for OpenCL
+Programming Language Documentation
+`_.
 
 Since C++ features are to be used on top of OpenCL C functionality, all existing
 restrictions from OpenCL C v2.0 will inherently apply. All OpenCL C builtin types
Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1631,285 +1631,6 @@
 Query the presence of this new mangling with
 ``__has_feature(objc_protocol_qualifier_mangling)``.
 
-
-OpenCL Features
-===
-
-C++ for OpenCL
---
-
-This functionality is built on top of OpenCL C v2.0 and C++17 enabling most of
-regular C++ features in OpenCL kernel code. Most functionality from OpenCL C
-is inherited. This section describes minor differences to OpenCL C and any
-limitations related to C++ support as well as interactions between OpenCL and
-C++ features that are not documented elsewhere.
-
-Restrictions to C++17
-^
-
-The following features are not supported:
-
-- Virtual functions
-- Exceptions
-- ``dynamic_cast`` operator
-- Non-placement ``new``/``delete`` operators
-- Standard C++ libraries. Currently there is no solution for alternative C++
-  libraries provided. Future release will feature library support.
-
-
-Interplay of OpenCL and C++ features
-
-
-Address space behavior
-""
-
-Address spaces are part of the type qualifiers; many rules are just inherited
-from the qualifier behavior documented in OpenCL C v2.0 s6.5 and Embedded C
-extension ISO/IEC JTC1 SC22 WG14 N1021 s3.1. Note that since the address space
-behavior in C++ is not documented formally, Clang extends the existing concept
-from C and OpenCL. For example conversion rules are extended from qualification
-conversion but the compatibility is determined using notation of sets and
-overlapping of address spaces from Embedded C (ISO/IEC JTC1 SC22 WG14 N1021
-s3.1.3). For OpenCL it means that implicit conversions are allowed from
-a named address space (except for ``__constant``) to ``__generic`` (OpenCL C
-v2.0 6.5.5). Reverse conversion is only allowed explicitly. The ``__constant``
-address space does not overlap with any other and therefore no valid conversion
-between ``__constant`` and other address spaces exists. Most of the rules
-follow this logic.
-
-**Casts**
-
-C-style casts follow OpenCL C v2.0 rules (s6.5.5). All cast operators
-permit conversion to ``__generic`` implicitly. However converting from
-``__generic`` to named address spaces can only be done using ``addrspace_cast``.
-Note that conversions between ``__constant`` and any other address space
-are disallowed.
-
-.. _opencl_cpp_addrsp_deduction:
-
-**Deduction**
-
-Address spaces are not deduced for:
-
-- non-pointer/non-reference template parameters or any dependent types except
-  for template specializations.
-- non-pointer/non-reference class members except for static data members that are
-  deduced to ``__global`` address space.
-- non-pointer/non-reference alias declarations.
-- ``decltype`` expressions.
-
-.. code-block:: c++
-
-  template 
-  void foo() {
-T m; // address space of m will be known at template instantiation time.
-T * ptr; // ptr points to __generic address space object.
-T & ref = ...; // ref references an object in __generic address space.
-  };
-
-  template 
-  struct S {
-int i; // i has no address space
-static int ii; // ii is in global address space
-int * ptr; // ptr points to __generic address space int.
-int & ref = ...; // ref references int in __generic address space.
-  };
-
-  template 
-  void bar()
-  {
-   

[PATCH] D72076: [OpenCL] Add link to C++ for OpenCL documentation

[Sven van Haastregt via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D72076



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

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 235848.
arichardson marked an inline comment as done.
arichardson added a comment.

Improved code generation with a single GEP

The pointer case of align_up is now generates the expected add+mask assembly 
even without the llvm.ptrmask intrinsic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtin-align-array.c
  clang/test/CodeGen/builtin-align.c
  clang/test/Sema/builtin-align.c
  clang/test/SemaCXX/builtin-align-cxx.cpp

Index: clang/test/SemaCXX/builtin-align-cxx.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/builtin-align-cxx.cpp
@@ -0,0 +1,236 @@
+// C++-specific checks for the alignment builtins
+// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -std=c++11 -o - %s -fsyntax-only -verify
+
+// Check that we don't crash when using dependent types in __builtin_align:
+template 
+void *c(void *d) { // expected-note{{candidate template ignored}}
+  return __builtin_align_down(d, b);
+}
+
+struct x {};
+x foo;
+void test(void *value) {
+  c(value);
+  c(value); // expected-error{{no matching function for call to 'c'}}
+}
+
+template 
+void test_templated_arguments() {
+  T array[ArraySize];   // expected-error{{variable has incomplete type 'fwddecl'}}
+  static_assert(__is_same(decltype(__builtin_align_up(array, Alignment)), T *), // expected-error{{requested alignment is not a power of 2}}
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_align_down(array, Alignment)), T *),
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_is_aligned(array, Alignment)), bool),
+"return type should be bool");
+  T *x1 = __builtin_align_up(array, Alignment);
+  T *x2 = __builtin_align_down(array, Alignment);
+  bool x3 = __builtin_align_up(array, Alignment);
+}
+
+void test() {
+  test_templated_arguments(); // fine
+  test_templated_arguments();
+  // expected-note@-1{{in instantiation of function template specialization 'test_templated_arguments'}}
+  // expected-note@-2{{forward declaration of 'fwddecl'}}
+  test_templated_arguments(); // invalid alignment value
+  // expected-note@-1{{in instantiation of function template specialization 'test_templated_arguments'}}
+}
+
+template 
+void test_incorrect_alignment_without_instatiation(T value) {
+  int array[32];
+  static_assert(__is_same(decltype(__builtin_align_up(array, 31)), int *), // expected-error{{requested alignment is not a power of 2}}
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_align_down(array, 7)), int *), // expected-error{{requested alignment is not a power of 2}}
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_is_aligned(array, -1)), bool), // expected-error{{requested alignment must be 1 or greater}}
+"return type should be bool");
+  __builtin_align_up(array);   // expected-error{{too few arguments to function call, expected 2, have 1}}
+  __builtin_align_up(array, 31);   // expected-error{{requested alignment is not a power of 2}}
+  __builtin_align_down(array, 31); // expected-error{{requested alignment is not a power of 2}}
+  __builtin_align_up(array, 31);   // expected-error{{requested alignment is not a power of 2}}
+  __builtin_align_up(value, 31);   // This shouldn't want since the type is dependent
+  __builtin_align_up(value);   // Same here
+}
+
+// The original fix for the issue above broke some legitimate code.
+// Here is a regression test:
+typedef __SIZE_TYPE__ size_t;
+void *allocate_impl(size_t size);
+template 
+T *allocate() {
+  constexpr size_t allocation_size =
+  __builtin_align_up(sizeof(T), sizeof(void *));
+  return static_cast(
+  __builtin_assume_aligned(allocate_impl(allocation_size), sizeof(void *)));
+}
+struct Foo {
+  int value;
+};
+void *test2() {
+  return allocate();
+}
+
+// Check that pointers-to-members cannot be used:
+class MemPtr {
+public:
+  int data;
+  void func();
+  virtual void vfunc();
+};
+void test_member_ptr() {
+  __builtin_align_up(&MemPtr::data, 64);// expected-error{{operand of type 'int MemPtr::*' where arithmetic or pointer type is required}}
+  __builtin_align_down(&MemPtr::func, 64);  // expected-error{{operand of type 'void (MemPtr::*)()' where arithmetic or pointer type is required}}
+  __builtin_

[PATCH] D71554: [llvm-ranlib] Handle -D and -U command line flag

[Alexander Richardson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG535b3c6b2f1c: [llvm-ranlib] Handle -D and -U command line 
flag (authored by arichardson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71554

Files:
  llvm/test/tools/llvm-ranlib/D-flag.test
  llvm/test/tools/llvm-ranlib/help-message.test
  llvm/tools/llvm-ar/llvm-ar.cpp

Index: llvm/tools/llvm-ar/llvm-ar.cpp
===
--- llvm/tools/llvm-ar/llvm-ar.cpp
+++ llvm/tools/llvm-ar/llvm-ar.cpp
@@ -64,8 +64,10 @@
 USAGE: llvm-ranlib 
 
 OPTIONS:
-  -h --help - Display available options
-  --version - Display the version of this program
+  -h --help - Display available options
+  -v --version  - Display the version of this program
+  -D- Use zero for timestamps and uids/gids (default)
+  -U- Use actual timestamps and uids/gids
 )";
 
 const char ArHelp[] = R"(OVERVIEW: LLVM Archiver
@@ -1156,13 +1158,33 @@
 static int ranlib_main(int argc, char **argv) {
   bool ArchiveSpecified = false;
   for (int i = 1; i < argc; ++i) {
-if (handleGenericOption(argv[i])) {
+StringRef arg(argv[i]);
+if (handleGenericOption(arg)) {
   return 0;
+} else if (arg.consume_front("-")) {
+  // Handle the -D/-U flag
+  while (!arg.empty()) {
+if (arg.front() == 'D') {
+  Deterministic = true;
+} else if (arg.front() == 'U') {
+  Deterministic = false;
+} else if (arg.front() == 'h') {
+  printHelpMessage();
+  return 0;
+} else if (arg.front() == 'v') {
+  cl::PrintVersionMessage();
+  return 0;
+} else {
+  // TODO: GNU ranlib also supports a -t flag
+  fail("Invalid option: '-" + arg + "'");
+}
+arg = arg.drop_front(1);
+  }
 } else {
   if (ArchiveSpecified)
 fail("exactly one archive should be specified");
   ArchiveSpecified = true;
-  ArchiveName = argv[i];
+  ArchiveName = arg.str();
 }
   }
   if (!ArchiveSpecified) {
Index: llvm/test/tools/llvm-ranlib/help-message.test
===
--- llvm/test/tools/llvm-ranlib/help-message.test
+++ llvm/test/tools/llvm-ranlib/help-message.test
@@ -1,8 +1,17 @@
 ## Show that the help message for llvm-ranlib can be printed with either the
 ## long flag -help.
 
-# RUN: llvm-ranlib -h | FileCheck %s
-# RUN: llvm-ranlib -help | FileCheck %s
-# RUN: llvm-ranlib --help | FileCheck %s
+# RUN: llvm-ranlib -h | FileCheck %s --check-prefix=HELP
+# RUN: llvm-ranlib -help | FileCheck %s --check-prefix=HELP
+# RUN: llvm-ranlib --help | FileCheck %s --check-prefix=HELP
+# RUN: llvm-ranlib --version | FileCheck %s --check-prefix=VERSION
+# RUN: llvm-ranlib -version | FileCheck %s --check-prefix=VERSION
+# RUN: llvm-ranlib -v | FileCheck %s --check-prefix=VERSION
 
-# CHECK: USAGE: llvm-ranlib
+## Also check combined options (first -h/-v flag wins)
+# RUN: llvm-ranlib -Dh | FileCheck %s --check-prefix=HELP
+# RUN: llvm-ranlib -Dvh | FileCheck %s --check-prefix=VERSION
+# RUN: llvm-ranlib -Dhv | FileCheck %s --check-prefix=HELP
+
+# HELP: USAGE: llvm-ranlib
+# VERSION: LLVM version
Index: llvm/test/tools/llvm-ranlib/D-flag.test
===
--- /dev/null
+++ llvm/test/tools/llvm-ranlib/D-flag.test
@@ -0,0 +1,45 @@
+## Test the -D and -U flags of llvm-ranlib
+## Create an archive with timestamps but without symbol table
+## Important: all `llvm-ar tv` calls must use TZ=UTC to produce identical values
+# RUN: yaml2obj %S/../llvm-ar/Inputs/add-lib1.yaml -o %t.o
+# RUN: env TZ=UTC touch -t 21020304 %t.o
+# RUN: rm -f %t.a %t-no-index.a && llvm-ar cqSU %t-no-index.a %t.o
+
+## Check that the intial listing has real values:
+# RUN: env TZ=UTC llvm-ar tv %t-no-index.a | FileCheck %s --check-prefix=REAL-VALUES
+
+## Check that the -D flag clears the timestamps:
+# RUN: cp %t-no-index.a %t.a && llvm-ranlib -D %t.a
+# RUN: env TZ=UTC llvm-ar tv %t.a | FileCheck %s --check-prefix=DETERMINISTIC-VALUES
+
+## Check that the -U flag maintains the timestamps:
+# RUN: cp %t-no-index.a %t.a && llvm-ranlib -U %t.a
+# RUN: env TZ=UTC llvm-ar tv %t.a | FileCheck %s --check-prefix=REAL-VALUES
+
+## Check that we accept multiple values and the last one wins:
+# RUN: cp %t-no-index.a %t.a && llvm-ranlib -UDU %t.a
+# RUN: env TZ=UTC llvm-ar tv %t.a | FileCheck %s --check-prefix=REAL-VALUES
+# RUN: cp %t-no-index.a %t.a && llvm-ranlib -UUD %t.a
+# RUN: env TZ=UTC llvm-ar tv %t.a | FileCheck %s --check-prefix=DETERMINISTIC-VALUES
+
+## Check arguments can be passed before and after the file name
+# RUN: cp %t-no-index.a %t.a && llvm-ranlib -U %t.a -D -U
+# RUN: env TZ=UTC llvm-ar tv %t.a | FileCheck %s

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

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

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61172 tests passed, 0 failed 
and 728 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D72076: [OpenCL] Add link to C++ for OpenCL documentation

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 235854.
Anastasia added a comment.

Recovered lost bit of information.


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

https://reviews.llvm.org/D72076

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/UsersManual.rst

Index: clang/docs/UsersManual.rst
===
--- clang/docs/UsersManual.rst
+++ clang/docs/UsersManual.rst
@@ -2856,6 +2856,10 @@
 
 Declaring the same types in different vendor extensions is disallowed.
 
+Clang also supports language extensions documented in `The OpenCL C Language
+Extensions Documentation
+`_.
+
 OpenCL Metadata
 ---
 
@@ -3031,8 +3035,9 @@
 `_ and
 there is no plan to support it in clang in any new releases in the near future.
 
-For detailed information about restrictions to allowed C++ features please
-refer to :doc:`LanguageExtensions`.
+For detailed information about this language refer to `The C++ for OpenCL
+Programming Language Documentation
+`_.
 
 Since C++ features are to be used on top of OpenCL C functionality, all existing
 restrictions from OpenCL C v2.0 will inherently apply. All OpenCL C builtin types
@@ -3060,6 +3065,35 @@
 
  clang -cl-std=clc++ test.cl
 
+Constructing and destroying global objects
+^^
+
+Global objects must be constructed before the first kernel using the global objects
+is executed and destroyed just after the last kernel using the program objects is
+executed. In OpenCL v2.0 drivers there is no specific API for invoking global
+constructors. However, an easy workaround would be to enqueue a constructor
+initialization kernel that has a name ``@_GLOBAL__sub_I_``.
+This kernel is only present if there are any global objects to be initialized in
+the compiled binary. One way to check this is by passing ``CL_PROGRAM_KERNEL_NAMES``
+to ``clGetProgramInfo`` (OpenCL v2.0 s5.8.7).
+
+Note that if multiple files are compiled and linked into libraries, multiple kernels
+that initialize global objects for multiple modules would have to be invoked.
+
+Applications are currently required to run initialization of global objects manually
+before running any kernels in which the objects are used.
+
+   .. code-block:: console
+
+ clang -cl-std=clc++ test.cl
+
+If there are any global objects to be initialized, the final binary will contain
+the ``@_GLOBAL__sub_I_test.cl`` kernel to be enqueued.
+
+Global destructors can not be invoked in OpenCL v2.0 drivers. However, all memory used
+for program scope objects is released on ``clReleaseProgram``.
+
+
 .. _target_features:
 
 Target-Specific Features and Limitations
Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1631,285 +1631,6 @@
 Query the presence of this new mangling with
 ``__has_feature(objc_protocol_qualifier_mangling)``.
 
-
-OpenCL Features
-===
-
-C++ for OpenCL
---
-
-This functionality is built on top of OpenCL C v2.0 and C++17 enabling most of
-regular C++ features in OpenCL kernel code. Most functionality from OpenCL C
-is inherited. This section describes minor differences to OpenCL C and any
-limitations related to C++ support as well as interactions between OpenCL and
-C++ features that are not documented elsewhere.
-
-Restrictions to C++17
-^
-
-The following features are not supported:
-
-- Virtual functions
-- Exceptions
-- ``dynamic_cast`` operator
-- Non-placement ``new``/``delete`` operators
-- Standard C++ libraries. Currently there is no solution for alternative C++
-  libraries provided. Future release will feature library support.
-
-
-Interplay of OpenCL and C++ features
-
-
-Address space behavior
-""
-
-Address spaces are part of the type qualifiers; many rules are just inherited
-from the qualifier behavior documented in OpenCL C v2.0 s6.5 and Embedded C
-extension ISO/IEC JTC1 SC22 WG14 N1021 s3.1. Note that since the address space
-behavior in C++ is not documented formally, Clang extends the existing concept
-from C and OpenCL. For example conversion rules are extended from qualification
-conversion but the compatibility is determined using notation of sets and
-overlapping of address spaces from Embedded C (ISO/IEC JTC1 SC22 WG14 N1021
-s3.1.3). For OpenCL it means that implicit conversions are allowed from
-a named address space (except for ``__constant``) to ``__generic`` (OpenCL C
-v2.0 6.5.5). Reverse conversion is only allowed explicitly. The ``__constant``
-address space does not overla

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri accepted this revision.
lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Looks ok to me now in principle.
I have one more question about pointer variants though (see inline)




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14321
+  if (Args.Src->getType()->isPointerTy()) {
+/// TODO: Use ptrmask instead of ptrtoint+gep once it is optimized well.
+// Result = Builder.CreateIntrinsic(

I honestly wonder if (at least for `align up`) that would result in worse 
results,
but we'll see if/when such patch appears i guess..



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14327
+llvm::Value* Difference = Builder.CreateSub(Result, SrcAddr, "diff");
+Result = Builder.CreateGEP(EmitCastToVoidPtr(Args.Src), Difference,
+   "aligned_result");

What's the semantics of these aligning builtins for pointers?
Is this GEP supposed to comply to the C17 6.5.6p8, C++ [expr.add]?
(TLDR: both the old pointer, and the newly-aligned pointer
must both point to the **same** array (allocated memory region))

I.e. can this GEP be `inbounds`? If it can, it'd be most great for 
optimizations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In D71499#1801104 , @lebedev.ri wrote:

> Looks ok to me now in principle.
>  I have one more question about pointer variants though (see inline)


I am not sure the GEP can be inbounds since I have seen some cases where 
aligning pointers is used to get a pointer to a different object.
I most cases it should be in-bounds (even when used to implement `malloc()`), 
but I have seen some cases where aligning pointers is used to get a pointer to 
a different object.
For example, some versions WebKit align pointers down by 64k to get a pointer 
to a structure that holds metadata for all objects allocated inside that region.

I am not sure what happens for those cases if we add inbounds 
(miscompilation?), so I haven't added it here.
I guess we could add it if alignment is a constant and is less than the object 
size, but there might already be a pass to infer if a GEP is inbounds?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D72071: [clangd] Add correctness checks for index-based rename

[Sam McCall via Phabricator via cfe-commits]

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

Postfiltering non-textual references won't work for the index-based rename, as 
the whole adjust-the-ranges algorithm relies on the index references being a 
subset of textual matches. More details in 
https://github.com/clangd/clangd/issues/238

It looks like this code change only affects index-based rename, but the 
attached test is only for AST-based rename.

I thought the plan of record was to make sure the index knew whether the 
reference was textual or not, and filter them out before calling 
adjustRenameRanges?




Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:798
   [[Foo]] foo;
+  foo.~[[Foo]];
 }

this isn't valid c++



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:882
+  [[Foo]] foo;
+  FooFoo z;
+}

If you want to be sure of fixing this bug, I'd like to see a test case: 
 - where the index is out of date
 - where the macro name is unrelated to the symbol name
 - where the macro usages are interleaved with the non-macro usages
 - where the macro usages are in the file which is renamed based on the index, 
not the one where renamed based on ast


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72071



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

[PATCH] D71652: [clangd] Replace shortenNamespace with getQualification

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71652



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

[PATCH] D71652: [clangd] Replace shortenNamespace with getQualification

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang-tools-extra/clangd/AST.cpp:307
+  llvm::raw_string_ostream OS(Result);
+  auto Decls = explicitReferenceTargets(
+  ast_type_traits::DynTypedNode::create(QT), DeclRelation::Alias);

This is such a terrible hack... Could you put a FIXME mentioning this should be 
done when printing names of the underlying decls instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71652



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

[PATCH] D71698: [AArch64][SVE] Add intrinsic for non-faulting loads

[Andrzej Warzynski via Phabricator via cfe-commits]

andwar added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:9998
+
   // GLD1* instructions perform an implicit zero-extend, which makes them
   // perfect candidates for combining.

Could you replace `GLD1*` with `Load`? I believe that that will be still 
correct with the added bonus of covering the new case :)



Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:11051
+  if (ContainerVT.isInteger()) {
+switch (VT.getVectorNumElements()) {
+default: return SDValue();

You could use `getSVEContainterType` here instead. You'll need to extend it a 
wee bit.



Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12284
 
   // Gather load nodes (e.g. AArch64ISD::GLD1) are straightforward candidates
   // for DAG Combine with SIGN_EXTEND_INREG. Bail out for all other nodes.

The following `switch` statement will now cover more than just *Gather* nodes. 
Maybe `SVE load nodes` instead? 



Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12328-12331
+  Ops.push_back(Src->getOperand(0));
+  Ops.push_back(Src->getOperand(1));
+  Ops.push_back(Src->getOperand(2));
+  Ops.push_back(Src->getOperand(3));

Why not:
```
SmallVector Ops = {Src->getOperand(0), Src->getOperand(1), 
Src->getOperand(2), Src->getOperand(3), Src->getOperand(4)};
```
?



Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12332
+  Ops.push_back(Src->getOperand(3));
+  if (NewOpc != AArch64ISD::LDNF1S)
+Ops.push_back(Src->getOperand(4));

Could you add a comment explaining what the underlying difference between 
`LDNF1S` and `GLD1S` is? Otherwise it's not clear why this `if` statement is 
needed. IIUC, `GLD1S` has an extra argument for the offsets (hence 5 args vs 4).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71698



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

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added reviewers: nlopes, aqjune.
lebedev.ri added a subscriber: nlopes.
lebedev.ri added a comment.

(would be good for @nlopes to comment, maybe i'm overweighting this..)

In D71499#1801119 , @arichardson wrote:

> In D71499#1801104 , @lebedev.ri 
> wrote:
>
> > Looks ok to me now in principle.
> >  I have one more question about pointer variants though (see inline)
>


What i'm asking is:

- Are these builtins designed (as per `clang/docs/LanguageExtensions.rst`) to 
only be passed pointers in-bounds to the allocated memory chunk (`Logical 
pointer`*), or any random bag of bits casted to pointer type (`Physical 
Pointer`*)?
- If `Logical pointer`, are they designed to also produce `Logical pointer`? Or 
`Physical Pointer`?

> I am not sure the GEP can be inbounds since I have seen some cases
>  where aligning pointers is used to get a pointer to a different object.
>  I most cases it should be in-bounds (even when used to implement `malloc()`),
>  but I have seen some cases where aligning pointers is used to get a pointer
>  to a different object.

Object as in C++ object?
I'm specifically talking about memory region/chunk, as in what is returned by 
`malloc()`.

> For example, some versions WebKit align pointers down by 64k to get a pointer 
> to a structure that holds metadata for all objects allocated inside that 
> region.

But that entire region is still a single memory region/chunk,
not just some other random memory chunk that *happens* to be close nearby?

> I am not sure what happens for those cases if we add inbounds 
> (miscompilation?), so I haven't added it here.
>  I guess we could add it if alignment is a constant and is less than the 
> object size, but there might already be a pass to infer if a GEP is inbounds?

I'm pushing on this because these intrinsics are supposed to be better than 
hand-rolled variants
(and in their current form with a single non-inbounds GEP they already are 
infinitly better
than ptrtoint+inttoptr pair), but if we go with current design (return 
`Physical pointer`),
(which is less optimizer friendly than `gep inbounds` which would 
requre/produce `Logical pointer`s),
we'll be stuck with it..

Since there is no such builtin currently (certainly not in clang,
i don't see one in GCC), we **do** get to dictate it's semantics.
We don't **have** to define it to be most lax/UB-free (`ptrtoint`+`inttoptr` or 
non-`inbounds` `gep`),
but we //can// define it to be most optimizer-friendly (`gep inbounds`).

- https://web.ist.utl.pt/nuno.lopes/pubs/llvmmem-oopsla18.pdf


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D71554: [llvm-ranlib] Handle -D and -U command line flag

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

The test fails on Windows: http://45.33.8.238/win/4948/step_11.txt

Please take a look, and if takes a while to fix, revert in the meantime.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71554



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

[PATCH] D71554: [llvm-ranlib] Handle -D and -U command line flag

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In D71554#1801203 , @thakis wrote:

> The test fails on Windows: http://45.33.8.238/win/4948/step_11.txt
>
> Please take a look, and if takes a while to fix, revert in the meantime.


I'll just relax the permissions check. We only care about the timestamp in this 
test so there is no need to check the rwx values.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71554



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

[PATCH] D71554: [llvm-ranlib] Handle -D and -U command line flag

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

Should be fixed in 
https://github.com/llvm/llvm-project/commit/a4f3847f3d5742cfab7acdc614e7ca54643e0c85


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71554



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

[clang] 87a004d - [OpenMP] Fix formatting of OpenMP error message, by Wang Tianqing.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2020-01-02T10:07:04-05:00
New Revision: 87a004d0f8c2fe5c4577d81b4306c35e77f21f9a

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

LOG: [OpenMP] Fix formatting of OpenMP error message, by Wang Tianqing.

Summary: `getListOfPossibleValues()` formatted incorrectly when there is only 
one value, emitting something like `expected 'conditional' or  in OpenMP clause 
'lastprivate'`.

Reviewers: jdoerfert, ABataev

Reviewed By: jdoerfert

Subscribers: guansong, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/for_lastprivate_messages.cpp
clang/test/OpenMP/for_simd_lastprivate_messages.cpp
clang/test/OpenMP/parallel_for_lastprivate_messages.cpp
clang/test/OpenMP/parallel_for_simd_lastprivate_messages.cpp
clang/test/OpenMP/parallel_sections_lastprivate_messages.cpp
clang/test/OpenMP/sections_lastprivate_messages.cpp
clang/test/OpenMP/simd_lastprivate_messages.cpp
clang/test/OpenMP/target_parallel_for_lastprivate_messages.cpp
clang/test/OpenMP/target_parallel_for_simd_lastprivate_messages.cpp
clang/test/OpenMP/target_simd_lastprivate_messages.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index decda442fed8..23cd6f04a93d 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -11933,7 +11933,6 @@ getListOfPossibleValues(OpenMPClauseKind K, unsigned 
First, unsigned Last,
 ArrayRef Exclude = llvm::None) {
   SmallString<256> Buffer;
   llvm::raw_svector_ostream Out(Buffer);
-  unsigned Bound = Last >= 2 ? Last - 2 : 0;
   unsigned Skipped = Exclude.size();
   auto S = Exclude.begin(), E = Exclude.end();
   for (unsigned I = First; I < Last; ++I) {
@@ -11942,9 +11941,9 @@ getListOfPossibleValues(OpenMPClauseKind K, unsigned 
First, unsigned Last,
   continue;
 }
 Out << "'" << getOpenMPSimpleClauseTypeName(K, I) << "'";
-if (I == Bound - Skipped)
+if (I + Skipped + 2 == Last)
   Out << " or ";
-else if (I != Bound + 1 - Skipped)
+else if (I + Skipped + 1 != Last)
   Out << ", ";
   }
   return Out.str();

diff  --git a/clang/test/OpenMP/for_lastprivate_messages.cpp 
b/clang/test/OpenMP/for_lastprivate_messages.cpp
index 4d6d85fa6c91..5ad8552a4262 100644
--- a/clang/test/OpenMP/for_lastprivate_messages.cpp
+++ b/clang/test/OpenMP/for_lastprivate_messages.cpp
@@ -107,6 +107,10 @@ int foomain(int argc, char **argv) {
   for (int k = 0; k < argc; ++k)
 ++k;
 #pragma omp parallel
+#pragma omp for lastprivate(foo:argc) // omp50-error {{expected 'conditional' 
in OpenMP clause 'lastprivate'}} omp45-error {{expected ',' or ')' in 
'lastprivate' clause}} omp45-error {{expected ')'}} omp45-error {{expected 
variable name}} omp45-note {{to match this '('}}
+  for (int k = 0; k < argc; ++k)
+++k;
+#pragma omp parallel
 #pragma omp for lastprivate(conditional: argc,s) lastprivate(conditional: // 
omp45-error 2 {{use of undeclared identifier 'conditional'}} omp50-error 
{{expected expression}} expected-error {{expected ')'}} expected-note {{to 
match this '('}} omp45-error 2 {{calling a private constructor of class 'S6'}} 
omp50-error {{expected list item of scalar type in 'lastprivate' clause with 
'conditional' modifier}}
   for (int k = 0; k < argc; ++k)
 ++k;

diff  --git a/clang/test/OpenMP/for_simd_lastprivate_messages.cpp 
b/clang/test/OpenMP/for_simd_lastprivate_messages.cpp
index ac52f8a2fa89..023f56c1f2bd 100644
--- a/clang/test/OpenMP/for_simd_lastprivate_messages.cpp
+++ b/clang/test/OpenMP/for_simd_lastprivate_messages.cpp
@@ -111,6 +111,10 @@ int foomain(int argc, char **argv) {
   for (int k = 0; k < argc; ++k)
 ++k;
 #pragma omp parallel
+#pragma omp for simd lastprivate(foo:argc) // omp50-error {{expected 
'conditional' in OpenMP clause 'lastprivate'}} omp45-error {{expected ',' or 
')' in 'lastprivate' clause}} omp45-error {{expected ')'}} omp45-error 
{{expected variable name}} omp45-note {{to match this '('}}
+  for (int k = 0; k < argc; ++k)
+++k;
+#pragma omp parallel
 #pragma omp for simd lastprivate(z, a, b) // expected-error {{lastprivate 
variable with incomplete type 'S1'}}
   for (int k = 0; k < argc; ++k)
 ++k;

diff  --git a/clang/test/OpenMP/parallel_for_lastprivate_messages.cpp 
b/clang/test/OpenMP/parallel_for_lastprivate_messages.cpp
index ae548d88b08c..fbbf826e6c03 100644
--- a/clang/test/OpenMP/parallel_for_lastprivate_messages.cpp
+++ b/clang/test/OpenMP/parallel_for_lastprivate_messages.cpp
@@ -98,6 +98,9 @@ int foomain(int argc, char **argv) {
 #pragma omp parallel for lastprivate(conditional: argc,s) 
lastprivate(conditional: // omp50-error 

[PATCH] D72085: [clangd] Fix hover for functions inside templated classes

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72085

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1531,7 +1531,11 @@
 TEST(Hover, DocsFromAST) {
   Annotations T(R"cpp(
   // doc
-  template  class X {};
+  template  class X {
+   public:
+// doc
+void foo();
+  };
   // doc
   template  void bar() {}
   // doc
@@ -1542,6 +1546,7 @@
 b^ar();
 au^to T = ba^z>;
 ba^z = 0;
+X().f^oo();
   })cpp");
 
   TestTU TU = TestTU::withCode(T.code());
@@ -1560,15 +1565,28 @@
 TEST(Hover, DocsFromMostSpecial) {
   Annotations T(R"cpp(
   // doc1
-  template  class $doc1^X {};
+  template  struct $doc1^X {
+// doc1
+void foo();
+  };
   // doc2
-  template <> class $doc2^X {};
+  template <> struct $doc2^X {
+// doc2
+void foo();
+  };
   // doc3
-  template  class $doc3^X {};
+  template  struct $doc3^X {
+// doc3
+void foo();
+  };
   void foo() {
 X$doc1^();
 X$doc2^();
 X$doc3^();
+
+X().fo$doc1^o();
+X().fo$doc2^o();
+X().fo$doc3^o();
   })cpp");
 
   TestTU TU = TestTU::withCode(T.code());
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -195,7 +195,7 @@
   return VTSD->getTemplateInstantiationPattern();
   if (auto *FD = D->getAsFunction())
 if (FD->isTemplateInstantiation())
-  return FD->getTemplateSpecializationInfo()->getTemplate();
+  return FD->getTemplateInstantiationPattern();
   return D;
 }
 


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1531,7 +1531,11 @@
 TEST(Hover, DocsFromAST) {
   Annotations T(R"cpp(
   // doc
-  template  class X {};
+  template  class X {
+   public:
+// doc
+void foo();
+  };
   // doc
   template  void bar() {}
   // doc
@@ -1542,6 +1546,7 @@
 b^ar();
 au^to T = ba^z>;
 ba^z = 0;
+X().f^oo();
   })cpp");
 
   TestTU TU = TestTU::withCode(T.code());
@@ -1560,15 +1565,28 @@
 TEST(Hover, DocsFromMostSpecial) {
   Annotations T(R"cpp(
   // doc1
-  template  class $doc1^X {};
+  template  struct $doc1^X {
+// doc1
+void foo();
+  };
   // doc2
-  template <> class $doc2^X {};
+  template <> struct $doc2^X {
+// doc2
+void foo();
+  };
   // doc3
-  template  class $doc3^X {};
+  template  struct $doc3^X {
+// doc3
+void foo();
+  };
   void foo() {
 X$doc1^();
 X$doc2^();
 X$doc3^();
+
+X().fo$doc1^o();
+X().fo$doc2^o();
+X().fo$doc3^o();
   })cpp");
 
   TestTU TU = TestTU::withCode(T.code());
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -195,7 +195,7 @@
   return VTSD->getTemplateInstantiationPattern();
   if (auto *FD = D->getAsFunction())
 if (FD->isTemplateInstantiation())
-  return FD->getTemplateSpecializationInfo()->getTemplate();
+  return FD->getTemplateInstantiationPattern();
   return D;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71884: [OpenMP] Fix formatting of OpenMP error message.

[Alexey Bataev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG87a004d0f8c2: [OpenMP] Fix formatting of OpenMP error 
message, by Wang Tianqing. (authored by ABataev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71884

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/for_lastprivate_messages.cpp
  clang/test/OpenMP/for_simd_lastprivate_messages.cpp
  clang/test/OpenMP/parallel_for_lastprivate_messages.cpp
  clang/test/OpenMP/parallel_for_simd_lastprivate_messages.cpp
  clang/test/OpenMP/parallel_sections_lastprivate_messages.cpp
  clang/test/OpenMP/sections_lastprivate_messages.cpp
  clang/test/OpenMP/simd_lastprivate_messages.cpp
  clang/test/OpenMP/target_parallel_for_lastprivate_messages.cpp
  clang/test/OpenMP/target_parallel_for_simd_lastprivate_messages.cpp
  clang/test/OpenMP/target_simd_lastprivate_messages.cpp

Index: clang/test/OpenMP/target_simd_lastprivate_messages.cpp
===
--- clang/test/OpenMP/target_simd_lastprivate_messages.cpp
+++ clang/test/OpenMP/target_simd_lastprivate_messages.cpp
@@ -107,6 +107,9 @@
 #pragma omp target simd lastprivate(conditional: s,argc) lastprivate(conditional: // omp45-error 2 {{use of undeclared identifier 'conditional'}} omp50-error {{expected expression}} expected-error {{expected ')'}} expected-note {{to match this '('}} omp50-error {{expected list item of scalar type in 'lastprivate' clause with 'conditional' modifier}}
   for (int k = 0; k < argc; ++k)
 ++k;
+#pragma omp target simd lastprivate(foo:argc) // omp50-error {{expected 'conditional' in OpenMP clause 'lastprivate'}} omp45-error {{expected ',' or ')' in 'lastprivate' clause}} omp45-error {{expected ')'}} omp45-error {{expected variable name}} omp45-note {{to match this '('}}
+  for (int k = 0; k < argc; ++k)
+++k;
 #pragma omp target simd lastprivate(S1) // expected-error {{'S1' does not refer to a value}}
   for (int k = 0; k < argc; ++k)
 ++k;
Index: clang/test/OpenMP/target_parallel_for_simd_lastprivate_messages.cpp
===
--- clang/test/OpenMP/target_parallel_for_simd_lastprivate_messages.cpp
+++ clang/test/OpenMP/target_parallel_for_simd_lastprivate_messages.cpp
@@ -107,6 +107,9 @@
 #pragma omp target parallel for simd lastprivate(conditional: argc,s) lastprivate(conditional: // omp50-error {{expected expression}} omp45-error 2 {{use of undeclared identifier 'conditional'}} expected-error {{expected ')'}} expected-note {{to match this '('}} omp45-error 2 {{calling a private constructor of class 'S6'}} omp50-error {{expected list item of scalar type in 'lastprivate' clause with 'conditional' modifier}}
   for (int k = 0; k < argc; ++k)
 ++k;
+#pragma omp target parallel for simd lastprivate(foo:argc) // omp50-error {{expected 'conditional' in OpenMP clause 'lastprivate'}} omp45-error {{expected ',' or ')' in 'lastprivate' clause}} omp45-error {{expected ')'}} omp45-error {{expected variable name}} omp45-note {{to match this '('}}
+  for (int k = 0; k < argc; ++k)
+++k;
 #pragma omp target parallel for simd lastprivate(S1) // expected-error {{'S1' does not refer to a value}}
   for (int k = 0; k < argc; ++k)
 ++k;
Index: clang/test/OpenMP/target_parallel_for_lastprivate_messages.cpp
===
--- clang/test/OpenMP/target_parallel_for_lastprivate_messages.cpp
+++ clang/test/OpenMP/target_parallel_for_lastprivate_messages.cpp
@@ -107,6 +107,9 @@
 #pragma omp target parallel for lastprivate(conditional: s,argc) lastprivate(conditional: // omp50-error {{expected expression}} omp45-error 2 {{use of undeclared identifier 'conditional'}} expected-error {{expected ')'}} expected-note {{to match this '('}} omp50-error {{expected list item of scalar type in 'lastprivate' clause with 'conditional' modifier}}
   for (int k = 0; k < argc; ++k)
 ++k;
+#pragma omp target parallel for lastprivate(foo:argc) // omp50-error {{expected 'conditional' in OpenMP clause 'lastprivate'}} omp45-error {{expected ',' or ')' in 'lastprivate' clause}} omp45-error {{expected ')'}} omp45-error {{expected variable name}} omp45-note {{to match this '('}}
+  for (int k = 0; k < argc; ++k)
+++k;
 #pragma omp target parallel for lastprivate(S1) // expected-error {{'S1' does not refer to a value}}
   for (int k = 0; k < argc; ++k)
 ++k;
Index: clang/test/OpenMP/simd_lastprivate_messages.cpp
===
--- clang/test/OpenMP/simd_lastprivate_messages.cpp
+++ clang/test/OpenMP/simd_lastprivate_messages.cpp
@@ -96,6 +96,9 @@
 #pragma omp simd lastprivate(conditional: argc,g) lastprivate(conditional: // omp50-error {{expected expression}} omp45-error 2 {{use of undeclared identifier 'conditional'}} expected-error {{expected ')'}} expected-note 

[clang-tools-extra] 8d7ecc1 - Revert "Revert "[clangd] Implement "textDocument/documentLink" protocol support""

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-01-02T16:36:21+01:00
New Revision: 8d7ecc16291ff415da0d5bfccb6363590a1310ad

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

LOG: Revert "Revert "[clangd] Implement "textDocument/documentLink" protocol 
support""

This reverts commit 079ef783dd5530b5f87beefe624b9179547ded7e.

The revert describes a test failure without details, after offline
discussion this in in a private/unsupported build system and doesn't
seem to reflect a real upstream bug.

Added: 
clang-tools-extra/clangd/test/document-link.test

Modified: 
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/Protocol.cpp
clang-tools-extra/clangd/Protocol.h
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/XRefs.h
clang-tools-extra/clangd/test/initialize-params.test
clang-tools-extra/clangd/unittests/XRefsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index bd9c31d3844b..69b4308a1c9e 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -566,6 +566,10 @@ void ClangdLSPServer::onInitialize(const InitializeParams 
&Params,
 {"declarationProvider", true},
 {"definitionProvider", true},
 {"documentHighlightProvider", true},
+{"documentLinkProvider",
+ llvm::json::Object{
+ {"resolveProvider", false},
+ }},
 {"hoverProvider", true},
 {"renameProvider", std::move(RenameProvider)},
 {"selectionRangeProvider", true},
@@ -1200,6 +1204,25 @@ void ClangdLSPServer::onSelectionRange(
   });
 }
 
+void ClangdLSPServer::onDocumentLink(
+const DocumentLinkParams &Params,
+Callback> Reply) {
+
+  // TODO(forster): This currently resolves all targets eagerly. This is slow,
+  // because it blocks on the preamble/AST being built. We could respond to the
+  // request faster by using string matching or the lexer to find the includes
+  // and resolving the targets lazily.
+  Server->documentLinks(
+  Params.textDocument.uri.file(),
+  [Reply = std::move(Reply)](
+  llvm::Expected> Links) mutable {
+if (!Links) {
+  return Reply(Links.takeError());
+}
+return Reply(std::move(Links));
+  });
+}
+
 ClangdLSPServer::ClangdLSPServer(
 class Transport &Transp, const FileSystemProvider &FSProvider,
 const clangd::CodeCompleteOptions &CCOpts,
@@ -1243,6 +1266,7 @@ ClangdLSPServer::ClangdLSPServer(
   MsgHandler->bind("textDocument/typeHierarchy", 
&ClangdLSPServer::onTypeHierarchy);
   MsgHandler->bind("typeHierarchy/resolve", 
&ClangdLSPServer::onResolveTypeHierarchy);
   MsgHandler->bind("textDocument/selectionRange", 
&ClangdLSPServer::onSelectionRange);
+  MsgHandler->bind("textDocument/documentLink", 
&ClangdLSPServer::onDocumentLink);
   // clang-format on
 }
 

diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.h 
b/clang-tools-extra/clangd/ClangdLSPServer.h
index f1ed317f6bad..9650b6a7dbb2 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.h
+++ b/clang-tools-extra/clangd/ClangdLSPServer.h
@@ -111,6 +111,8 @@ class ClangdLSPServer : private DiagnosticsConsumer {
 Callback>);
   void onSelectionRange(const SelectionRangeParams &,
 Callback>);
+  void onDocumentLink(const DocumentLinkParams &,
+  Callback>);
 
   std::vector getFixes(StringRef File, const clangd::Diagnostic &D);
 

diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index 40fa28ae140a..c71a8a0d566b 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -611,6 +611,17 @@ void ClangdServer::semanticRanges(PathRef File, Position 
Pos,
   WorkScheduler.runWithAST("SemanticRanges", File, std::move(Action));
 }
 
+void ClangdServer::documentLinks(PathRef File,
+ Callback> CB) {
+  auto Action =
+  [CB = std::move(CB)](llvm::Expected InpAST) mutable {
+if (!InpAST)
+  return CB(InpAST.takeError());
+CB(clangd::getDocumentLinks(InpAST->AST));
+  };
+  WorkScheduler.runWithAST("DocumentLinks", File, std::move(Action));
+}
+
 std::vector>
 ClangdServer::getUsedBytesPerFile() const {
   return WorkScheduler.getUsedBytesPerFile();

diff  --git a/clang-tools-extra/clangd/ClangdServer.h 
b/clang-tools-extra/clangd/ClangdServer.h
index 499340808765..4574d6c35034 100644
--- a/clang-tools-extra/clangd/ClangdServer.

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a subscriber: brooks.
arichardson added a comment.

> What i'm asking is:
> 
> - Are these builtins designed (as per `clang/docs/LanguageExtensions.rst`) to 
> only be passed pointers in-bounds to the allocated memory chunk (`Logical 
> pointer`*), or any random bag of bits casted to pointer type (`Physical 
> Pointer`*)?
> - If `Logical pointer`, are they designed to also produce `Logical pointer`? 
> Or `Physical Pointer`?

In my view you both input and output should be a `Logical pointer`. If you want 
random values, you should be aligning `uintptr_t` instead.
For CHERI we only need to support passing and creating valid pointers (i.e. 
with the capability tag bit set and valid bounds information).
Since the source pointer contains bounds information, it will never be possible 
to use the __builtin_align_up/down result to access memory from a different 
underlying allocation.

>> I am not sure the GEP can be inbounds since I have seen some cases
>>  where aligning pointers is used to get a pointer to a different object.
>>  I most cases it should be in-bounds (even when used to implement 
>> `malloc()`),
>>  but I have seen some cases where aligning pointers is used to get a pointer
>>  to a different object.
> 
> Object as in C++ object?
>  I'm specifically talking about memory region/chunk, as in what is returned 
> by `malloc()`.
> 
>> For example, some versions WebKit align pointers down by 64k to get a 
>> pointer to a structure that holds metadata for all objects allocated inside 
>> that region.
> 
> But that entire region is still a single memory region/chunk,
>  not just some other random memory chunk that *happens* to be close nearby?

Since we can run that code on CHERI-MIPS, I realize it must be the same memory 
region.
I had look at the code again and the aligned structure is constructed in a 64K 
region that is created by a single call to `fastMalloc(blockSize)`, i.e. 
`malloc()/mmap()`.

> 
> 
>> I am not sure what happens for those cases if we add inbounds 
>> (miscompilation?), so I haven't added it here.
>>  I guess we could add it if alignment is a constant and is less than the 
>> object size, but there might already be a pass to infer if a GEP is inbounds?
> 
> I'm pushing on this because these intrinsics are supposed to be better than 
> hand-rolled variants
>  (and in their current form with a single non-inbounds GEP they already are 
> infinitly better
>  than ptrtoint+inttoptr pair), but if we go with current design (return 
> `Physical pointer`),
>  (which is less optimizer friendly than `gep inbounds` which would 
> requre/produce `Logical pointer`s),
>  we'll be stuck with it..
> 
> Since there is no such builtin currently (certainly not in clang,
>  i don't see one in GCC), we **do** get to dictate it's semantics.
>  We don't **have** to define it to be most lax/UB-free (`ptrtoint`+`inttoptr` 
> or non-`inbounds` `gep`),
>  but we //can// define it to be most optimizer-friendly (`gep inbounds`).
> 
> - https://web.ist.utl.pt/nuno.lopes/pubs/llvmmem-oopsla18.pdf

It seems like we should be able to add inbounds to the GEP when aligning 
pointer values.

However, I think this means that for the downstream CHERI codegen there is one 
more case where we need to be careful (but that shouldn't matter for upstream).
For us `uintptr_t` is represented as `i8 addrspace(200)*`.
As `uintptr_t` can hold values that are plain integers and not pointers, we 
would need to add the inbounds only if the AST type is a pointer, and use a 
non-inbounds GEP for `(u)intptr_t`.
Does that sound correct or did I misunderstand something?

Note: Creating out-of-bounds pointers (capabilities) is fine for CHERI in 
hardware, they just can't be dereferenced without trapping. You can bring them 
back in bounds and then use them again (as long as they weren't *too* far out 
of bounds, since the compression scheme can't represent all possible values).

Adding @brooks in case there is a use case in the OS kernel that may need to 
create out-of-bounds pointers that I forgot about.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added inline comments.



Comment at: clang/include/clang/AST/Expr.h:126
+if (TD)
+  D = D | DependencyFlags::Type;
+if (VD)

ilya-biryukov wrote:
> Mordante wrote:
> > Just curious why do you prefer `D = D | DependencyFlags::Type;` over `D |= 
> > DependencyFlags::Type;` ? The latter seems to be more common.
> Would also prefer `D |=`, but it leads to compilation errors.
> 
> The builtin `operator |=` accepts ints and, therefore, fails on 
> strongly-typed enums.
> And, AFAIK, there's no way to redefine `operator |=` for non-class types.
You certainly can define a compound assignment operator for an enumeration 
type. It is only non-compound-assignment that is special cased and required to 
be a member function.

Example: https://godbolt.org/z/JV5uPw



Comment at: clang/include/clang/AST/Expr.h:134
+
+ExprBits.Dependent = static_cast(D);
 ExprBits.ValueKind = VK;

You may want to add an assertion here to make sure that no truncation occurred.



Comment at: clang/lib/Serialization/ASTReaderStmt.cpp:530
+  E->setDependencies(Deps);
+
   E->setValueKind(static_cast(Record.readInt()));

I think it would be nicer to add an abbreviation for the right number of bits 
(using `DependencyFlagsBits`) and as you say just serialize/deserialize all the 
flags in one go.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71920



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

[PATCH] D72085: [clangd] Fix hover for functions inside templated classes

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

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61043 tests passed, 0 failed 
and 728 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72085



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

[clang] 24ab9b5 - Generalize the pass registration mechanism used by Polly to any third-party tool

[via cfe-commits]

Author: serge_sans_paille
Date: 2020-01-02T16:45:31+01:00
New Revision: 24ab9b537e61b3fe5e6a1019492ff6530d82a3ee

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

LOG: Generalize the pass registration mechanism used by Polly to any 
third-party tool

There's quite a lot of references to Polly in the LLVM CMake codebase. However
the registration pattern used by Polly could be useful to other external
projects: thanks to that mechanism it would be possible to develop LLVM
extension without touching the LLVM code base.

This patch has two effects:

1. Remove all code specific to Polly in the llvm/clang codebase, replaicing it
   with a generic mechanism

2. Provide a generic mechanism to register compiler extensions.

A compiler extension is similar to a pass plugin, with the notable difference
that the compiler extension can be configured to be built dynamically (like
plugins) or statically (like regular passes).

As a result, people willing to add extra passes to clang/opt can do it using a
separate code repo, but still have their pass be linked in clang/opt as built-in
passes.

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

Added: 
llvm/examples/Bye/Bye.cpp
llvm/examples/Bye/CMakeLists.txt
llvm/test/Feature/load_extension.ll
polly/lib/Plugin/Polly.cpp

Modified: 
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/CodeGen/CMakeLists.txt
clang/tools/driver/CMakeLists.txt
clang/tools/driver/cc1_main.cpp
llvm/CMakeLists.txt
llvm/cmake/modules/AddLLVM.cmake
llvm/docs/WritingAnLLVMPass.rst
llvm/examples/CMakeLists.txt
llvm/include/llvm/Config/llvm-config.h.cmake
llvm/test/Other/new-pm-defaults.ll
llvm/test/Other/new-pm-thinlto-defaults.ll
llvm/test/Other/opt-O0-pipeline.ll
llvm/test/Other/opt-O2-pipeline.ll
llvm/test/Other/opt-O3-pipeline.ll
llvm/test/Other/opt-Os-pipeline.ll
llvm/test/lit.cfg.py
llvm/test/lit.site.cfg.py.in
llvm/tools/CMakeLists.txt
llvm/tools/bugpoint/CMakeLists.txt
llvm/tools/bugpoint/bugpoint.cpp
llvm/tools/opt/CMakeLists.txt
llvm/tools/opt/NewPMDriver.cpp
llvm/tools/opt/opt.cpp
llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
polly/include/polly/RegisterPasses.h
polly/lib/CMakeLists.txt
polly/lib/Support/RegisterPasses.cpp
polly/test/Unit/lit.site.cfg.in
polly/test/lit.site.cfg.in
polly/test/update_check.py

Removed: 
polly/lib/Polly.cpp



diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index 645ef0165a53..ed881f2ddf68 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -75,6 +75,10 @@
 using namespace clang;
 using namespace llvm;
 
+#define HANDLE_EXTENSION(Ext)  
\
+  llvm::PassPluginLibraryInfo get##Ext##PluginInfo();
+#include "llvm/Support/Extension.def"
+
 namespace {
 
 // Default filename used for profile generation.
@@ -1076,6 +1080,9 @@ void EmitAssemblyHelper::EmitAssemblyWithNewPassManager(
   << PluginFN << toString(PassPlugin.takeError());
 }
   }
+#define HANDLE_EXTENSION(Ext)  
\
+  get##Ext##PluginInfo().RegisterPassBuilderCallbacks(PB);
+#include "llvm/Support/Extension.def"
 
   LoopAnalysisManager LAM(CodeGenOpts.DebugPassManager);
   FunctionAnalysisManager FAM(CodeGenOpts.DebugPassManager);

diff  --git a/clang/lib/CodeGen/CMakeLists.txt 
b/clang/lib/CodeGen/CMakeLists.txt
index a3980637c592..d8b3c234a1ef 100644
--- a/clang/lib/CodeGen/CMakeLists.txt
+++ b/clang/lib/CodeGen/CMakeLists.txt
@@ -96,6 +96,8 @@ add_clang_library(clangCodeGen
   TargetInfo.cpp
   VarBypassDetector.cpp
 
+  ENABLE_PLUGINS
+
   DEPENDS
   ${codegen_deps}
 

diff  --git a/clang/tools/driver/CMakeLists.txt 
b/clang/tools/driver/CMakeLists.txt
index 52a95e6bc6b1..2b783cff0955 100644
--- a/clang/tools/driver/CMakeLists.txt
+++ b/clang/tools/driver/CMakeLists.txt
@@ -122,7 +122,3 @@ if(CLANG_ORDER_FILE AND
 set_target_properties(clang PROPERTIES LINK_DEPENDS ${CLANG_ORDER_FILE})
   endif()
 endif()
-
-if(WITH_POLLY AND LINK_POLLY_INTO_TOOLS)
-  target_link_libraries(clang PRIVATE Polly)
-endif(WITH_POLLY AND LINK_POLLY_INTO_TOOLS)

diff  --git a/clang/tools/driver/cc1_main.cpp b/clang/tools/driver/cc1_main.cpp
index 2b82a4378111..b551e9f4cf82 100644
--- a/clang/tools/driver/cc1_main.cpp
+++ b/clang/tools/driver/cc1_main.cpp
@@ -72,12 +72,6 @@ static void LLVMErrorHandler(void *UserData, const 
std::string &Message,
   exit(GenCrashDiag ? 70 : 1);
 }
 
-#ifdef LINK_POLLY_INTO_TOOLS
-namespace polly {
-void initializePollyPasses(llvm::PassRegistry &Registry);
-}
-#endif
-
 #ifdef CLANG_HAVE_RLIMITS
 #if defined(__linux__) && defined(__PIE__)
 static 

[PATCH] D71726: Let clang atomic builtins fetch add/sub support floating point types

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: clang/lib/CodeGen/CGAtomic.cpp:597-598
   case AtomicExpr::AO__atomic_add_fetch:
-PostOp = llvm::Instruction::Add;
+PostOp = E->getValueType()->isFloatingType() ? llvm::Instruction::FAdd
+ : llvm::Instruction::Add;
 LLVM_FALLTHROUGH;

Should this really be based on the type, or should the builtin name be 
different for FP?


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

https://reviews.llvm.org/D71726



___
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

[serge via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG24ab9b537e61: Generalize the pass registration mechanism 
used by Polly to any third-party tool (authored by serge-sans-paille).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CMakeLists.txt
  clang/tools/driver/CMakeLists.txt
  clang/tools/driver/cc1_main.cpp
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/docs/WritingAnLLVMPass.rst
  llvm/examples/Bye/Bye.cpp
  llvm/examples/Bye/CMakeLists.txt
  llvm/examples/CMakeLists.txt
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/test/Feature/load_extension.ll
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/opt-O0-pipeline.ll
  llvm/test/Other/opt-O2-pipeline.ll
  llvm/test/Other/opt-O3-pipeline.ll
  llvm/test/Other/opt-Os-pipeline.ll
  llvm/test/lit.cfg.py
  llvm/test/lit.site.cfg.py.in
  llvm/tools/CMakeLists.txt
  llvm/tools/bugpoint/CMakeLists.txt
  llvm/tools/bugpoint/bugpoint.cpp
  llvm/tools/opt/CMakeLists.txt
  llvm/tools/opt/NewPMDriver.cpp
  llvm/tools/opt/opt.cpp
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  polly/include/polly/RegisterPasses.h
  polly/lib/CMakeLists.txt
  polly/lib/Plugin/Polly.cpp
  polly/lib/Polly.cpp
  polly/lib/Support/RegisterPasses.cpp
  polly/test/Unit/lit.site.cfg.in
  polly/test/lit.site.cfg.in
  polly/test/update_check.py

Index: polly/test/update_check.py
===
--- polly/test/update_check.py
+++ polly/test/update_check.py
@@ -15,7 +15,7 @@
 polly_lib_dir = '''@POLLY_LIB_DIR@'''
 shlibext = '''@LLVM_SHLIBEXT@'''
 llvm_tools_dir = '''@LLVM_TOOLS_DIR@'''
-link_polly_into_tools = not '''@LINK_POLLY_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','link_polly_into_tools-notfound'}
+llvm_polly_link_into_tools = not '''@LLVM_POLLY_LINK_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','llvm_polly_link_into_tools-notfound'}
 
 runre = re.compile(r'\s*\;\s*RUN\s*\:(?P.*)')
 filecheckre = re.compile(r'\s*(?P.*)\|\s*(?PFileCheck\s[^|]*)')
@@ -298,7 +298,7 @@
 toolarg = toolarg.replace('%s', filename)
 toolarg = toolarg.replace('%S', os.path.dirname(filename))
 if toolarg == '%loadPolly':
-if not link_polly_into_tools:
+if not llvm_polly_link_into_tools:
 newtool += ['-load',os.path.join(polly_lib_dir,'LLVMPolly' + shlibext)]
 newtool.append('-polly-process-unprofitable')
 newtool.append('-polly-remarks-minimal')
Index: polly/test/lit.site.cfg.in
===
--- polly/test/lit.site.cfg.in
+++ polly/test/lit.site.cfg.in
@@ -8,7 +8,7 @@
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.targets_to_build = "@TARGETS_TO_BUILD@"
 config.extra_paths = "@POLLY_TEST_EXTRA_PATHS@".split(";")
 
@@ -36,14 +36,14 @@
 # directories.
 config.excludes = ['Inputs']
 
-if config.link_polly_into_tools == '' or \
-   config.link_polly_into_tools.lower() == '0' or \
-   config.link_polly_into_tools.lower() == 'n' or \
-   config.link_polly_into_tools.lower() == 'no' or \
-   config.link_polly_into_tools.lower() == 'off' or \
-   config.link_polly_into_tools.lower() == 'false' or \
-   config.link_polly_into_tools.lower() == 'notfound' or \
-   config.link_polly_into_tools.lower() == 'link_polly_into_tools-notfound':
+if config.llvm_polly_link_into_tools == '' or \
+   config.llvm_polly_link_into_tools.lower() == '0' or \
+   config.llvm_polly_link_into_tools.lower() == 'n' or \
+   config.llvm_polly_link_into_tools.lower() == 'no' or \
+   config.llvm_polly_link_into_tools.lower() == 'off' or \
+   config.llvm_polly_link_into_tools.lower() == 'false' or \
+   config.llvm_polly_link_into_tools.lower() == 'notfound' or \
+   config.llvm_polly_link_into_tools.lower() == 'llvm_polly_link_into_tools-notfound':
 config.substitutions.append(('%loadPolly', '-load '
  + config.polly_lib_dir + '/LLVMPolly@LLVM_SHLIBEXT@'
  + ' -load-pass-plugin '
Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -13,7 +13,7 @@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 conf

[clang] e406cca - Revert "build: reduce CMake handling for zlib"

[James Henderson via cfe-commits]

Author: James Henderson
Date: 2020-01-02T16:02:10Z
New Revision: e406cca5f9a6477c9861717f81c156aa83feeaca

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

LOG: Revert "build: reduce CMake handling for zlib"

This reverts commit 68a235d07f9e7049c7eb0c8091f37e385327ac28.

This commit broke the clang-x64-windows-msvc build bot and a follow-up
commit did not fix it. Reverting to fix the bot.

Added: 


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

Removed: 




diff  --git a/clang/test/CMakeLists.txt b/clang/test/CMakeLists.txt
index 6f6121f06e56..16e487f07b78 100644
--- a/clang/test/CMakeLists.txt
+++ b/clang/test/CMakeLists.txt
@@ -9,13 +9,22 @@ endif ()
 
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} CLANG_TOOLS_DIR 
${LLVM_RUNTIME_OUTPUT_INTDIR})
 
+if(CLANG_BUILT_STANDALONE)
+  # Set HAVE_LIBZ according to recorded LLVM_ENABLE_ZLIB value. This
+  # value is forced to 0 if zlib was not found, so it is fine to use it
+  # instead of HAVE_LIBZ (not recorded).
+  if(LLVM_ENABLE_ZLIB)
+set(HAVE_LIBZ 1)
+  endif()
+endif()
+
 llvm_canonicalize_cmake_booleans(
   CLANG_BUILD_EXAMPLES
   CLANG_ENABLE_ARCMT
   CLANG_ENABLE_STATIC_ANALYZER
   ENABLE_BACKTRACES
   ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER
-  LLVM_ENABLE_ZLIB
+  HAVE_LIBZ
   LLVM_ENABLE_PER_TARGET_RUNTIME_DIR
   LLVM_ENABLE_PLUGINS
   LLVM_ENABLE_THREADS)

diff  --git a/clang/test/lit.site.cfg.py.in b/clang/test/lit.site.cfg.py.in
index e9b35ac01771..520afab6af82 100644
--- a/clang/test/lit.site.cfg.py.in
+++ b/clang/test/lit.site.cfg.py.in
@@ -16,7 +16,7 @@ config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.host_cxx = "@CMAKE_CXX_COMPILER@"
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @LLVM_ENABLE_ZLIB@
+config.have_zlib = @HAVE_LIBZ@
 config.clang_arcmt = @CLANG_ENABLE_ARCMT@
 config.clang_default_cxx_stdlib = "@CLANG_DEFAULT_CXX_STDLIB@"
 config.clang_staticanalyzer = @CLANG_ENABLE_STATIC_ANALYZER@

diff  --git a/compiler-rt/test/lit.common.configured.in 
b/compiler-rt/test/lit.common.configured.in
index 0fb51741783e..b4862f74cdd0 100644
--- a/compiler-rt/test/lit.common.configured.in
+++ b/compiler-rt/test/lit.common.configured.in
@@ -50,7 +50,7 @@ if config.enable_per_target_runtime_dir:
 else:
   set_default("target_suffix", "-%s" % config.target_arch)
 
-set_default("have_zlib", "@LLVM_ENABLE_ZLIB@")
+set_default("have_zlib", "@HAVE_LIBZ@")
 set_default("libcxx_used", "@LLVM_LIBCXX_USED@")
 
 # LLVM tools dir can be passed in lit parameters, so try to

diff  --git a/lld/test/CMakeLists.txt b/lld/test/CMakeLists.txt
index dc8cedf2ea09..8be42c46dd8a 100644
--- a/lld/test/CMakeLists.txt
+++ b/lld/test/CMakeLists.txt
@@ -4,8 +4,17 @@ set(LLVM_BUILD_MODE "%(build_mode)s")
 set(LLVM_TOOLS_DIR "${LLVM_TOOLS_BINARY_DIR}/%(build_config)s")
 set(LLVM_LIBS_DIR 
"${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/%(build_config)s")
 
+if(LLD_BUILT_STANDALONE)
+  # Set HAVE_LIBZ according to recorded LLVM_ENABLE_ZLIB value. This
+  # value is forced to 0 if zlib was not found, so it is fine to use it
+  # instead of HAVE_LIBZ (not recorded).
+  if(LLVM_ENABLE_ZLIB)
+set(HAVE_LIBZ 1)
+  endif()
+endif()
+
 llvm_canonicalize_cmake_booleans(
-  LLVM_ENABLE_ZLIB
+  HAVE_LIBZ
   LLVM_LIBXML2_ENABLED
   )
 

diff  --git a/lld/test/lit.site.cfg.py.in b/lld/test/lit.site.cfg.py.in
index 531fce15839d..02840f8d6a30 100644
--- a/lld/test/lit.site.cfg.py.in
+++ b/lld/test/lit.site.cfg.py.in
@@ -14,7 +14,7 @@ config.lld_libs_dir = "@LLVM_LIBRARY_OUTPUT_INTDIR@"
 config.lld_tools_dir = "@LLVM_RUNTIME_OUTPUT_INTDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.python_executable = "@PYTHON_EXECUTABLE@"
-config.have_zlib = @LLVM_ENABLE_ZLIB@
+config.have_zlib = @HAVE_LIBZ@
 config.sizeof_void_p = @CMAKE_SIZEOF_VOID_P@
 
 # Support substitution of the tools and libs dirs with user parameters. This is

diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index 7cea013eea7f..0a98f6a15d75 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCom

[PATCH] D71854: [SystemZ] Use FNeg in s390x clang builtins

[Kevin P. Neal via Phabricator via cfe-commits]

kpn updated this revision to Diff 235880.
kpn added a comment.

Update now-failing tests.


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

https://reviews.llvm.org/D71854

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-systemz-vector.c
  clang/test/CodeGen/builtins-systemz-vector2.c
  clang/test/CodeGen/builtins-systemz-zvector.c
  clang/test/CodeGen/builtins-systemz-zvector2.c

Index: clang/test/CodeGen/builtins-systemz-zvector2.c
===
--- clang/test/CodeGen/builtins-systemz-zvector2.c
+++ clang/test/CodeGen/builtins-systemz-zvector2.c
@@ -686,11 +686,11 @@
 
   vf = vec_nabs(vf);
   // CHECK: [[ABS:%[^ ]+]] = tail call <4 x float> @llvm.fabs.v4f32(<4 x float> %{{.*}})
-  // CHECK-NEXT: fsub <4 x float> , [[ABS]]
+  // CHECK-NEXT: fneg <4 x float> [[ABS]]
   // CHECK-ASM: vflnsb
   vd = vec_nabs(vd);
   // CHECK: [[ABS:%[^ ]+]] = tail call <2 x double> @llvm.fabs.v2f64(<2 x double> %{{.*}})
-  // CHECK-NEXT: fsub <2 x double> , [[ABS]]
+  // CHECK-NEXT: fneg <2 x double> [[ABS]]
   // CHECK-ASM: vflndb
 
   vf = vec_max(vf, vf);
@@ -715,32 +715,32 @@
   // CHECK-ASM: vfmadb
 
   vf = vec_msub(vf, vf, vf);
-  // CHECK: [[NEG:%[^ ]+]] = fsub <4 x float> , %{{.*}}
+  // CHECK: [[NEG:%[^ ]+]] = fneg <4 x float> %{{.*}}
   // CHECK: call <4 x float> @llvm.fma.v4f32(<4 x float> %{{.*}}, <4 x float> %{{.*}}, <4 x float> [[NEG]])
   // CHECK-ASM: vfmssb
   vd = vec_msub(vd, vd, vd);
-  // CHECK: [[NEG:%[^ ]+]] = fsub <2 x double> , %{{.*}}
+  // CHECK: [[NEG:%[^ ]+]] = fneg <2 x double> %{{.*}}
   // CHECK: call <2 x double> @llvm.fma.v2f64(<2 x double> %{{.*}}, <2 x double> %{{.*}}, <2 x double> [[NEG]])
   // CHECK-ASM: vfmsdb
 
   vf = vec_nmadd(vf, vf, vf);
   // CHECK: [[RES:%[^ ]+]] = tail call <4 x float> @llvm.fma.v4f32(<4 x float> %{{.*}}, <4 x float> %{{.*}}, <4 x float> %{{.*}})
-  // CHECK: fsub <4 x float> , [[RES]]
+  // CHECK: fneg <4 x float> [[RES]]
   // CHECK-ASM: vfnmasb
   vd = vec_nmadd(vd, vd, vd);
   // CHECK: [[RES:%[^ ]+]] = tail call <2 x double> @llvm.fma.v2f64(<2 x double> %{{.*}}, <2 x double> %{{.*}}, <2 x double> %{{.*}})
-  // CHECK: fsub <2 x double> , [[RES]]
+  // CHECK: fneg <2 x double> [[RES]]
   // CHECK-ASM: vfnmadb
 
   vf = vec_nmsub(vf, vf, vf);
-  // CHECK: [[NEG:%[^ ]+]] = fsub <4 x float> , %{{.*}}
+  // CHECK: [[NEG:%[^ ]+]] = fneg <4 x float> %{{.*}}
   // CHECK: [[RES:%[^ ]+]] = tail call <4 x float> @llvm.fma.v4f32(<4 x float> %{{.*}}, <4 x float> %{{.*}}, <4 x float> [[NEG]])
-  // CHECK: fsub <4 x float> , [[RES]]
+  // CHECK: fneg <4 x float> [[RES]]
   // CHECK-ASM: vfnmssb
   vd = vec_nmsub(vd, vd, vd);
-  // CHECK: [[NEG:%[^ ]+]] = fsub <2 x double> , %{{.*}}
+  // CHECK: [[NEG:%[^ ]+]] = fneg <2 x double> %{{.*}}
   // CHECK: [[RES:%[^ ]+]] = tail call <2 x double> @llvm.fma.v2f64(<2 x double> %{{.*}}, <2 x double> %{{.*}}, <2 x double> [[NEG]])
-  // CHECK: fsub <2 x double> , [[RES]]
+  // CHECK: fneg <2 x double> [[RES]]
   // CHECK-ASM: vfnmsdb
 
   vf = vec_sqrt(vf);
Index: clang/test/CodeGen/builtins-systemz-zvector.c
===
--- clang/test/CodeGen/builtins-systemz-zvector.c
+++ clang/test/CodeGen/builtins-systemz-zvector.c
@@ -4442,14 +4442,14 @@
 
   vd = vec_nabs(vd);
   // CHECK: [[ABS:%[^ ]+]] = tail call <2 x double> @llvm.fabs.v2f64(<2 x double> %{{.*}})
-  // CHECK-NEXT: fsub <2 x double> , [[ABS]]
+  // CHECK-NEXT: fneg <2 x double> [[ABS]]
   // CHECK-ASM: vflndb
 
   vd = vec_madd(vd, vd, vd);
   // CHECK: call <2 x double> @llvm.fma.v2f64(<2 x double> %{{.*}}, <2 x double> %{{.*}}, <2 x double> %{{.*}})
   // CHECK-ASM: vfmadb
   vd = vec_msub(vd, vd, vd);
-  // CHECK: [[NEG:%[^ ]+]] = fsub <2 x double> , %{{.*}}
+  // CHECK: [[NEG:%[^ ]+]] = fneg <2 x double> %{{.*}}
   // CHECK: call <2 x double> @llvm.fma.v2f64(<2 x double> %{{.*}}, <2 x double> %{{.*}}, <2 x double> [[NEG]])
   // CHECK-ASM: vfmsdb
   vd = vec_sqrt(vd);
Index: clang/test/CodeGen/builtins-systemz-vector2.c
===
--- clang/test/CodeGen/builtins-systemz-vector2.c
+++ clang/test/CodeGen/builtins-systemz-vector2.c
@@ -64,11 +64,11 @@
 
   vd = __builtin_s390_vfnmadb(vd, vd, vd);
   // CHECK: [[RES:%[^ ]+]] = call <2 x double> @llvm.fma.v2f64(<2 x double> %{{.*}}, <2 x double> %{{.*}}, <2 x double> %{{.*}})
-  // CHECK: fsub <2 x double> , [[RES]]
+  // CHECK: fneg <2 x double> [[RES]]
   vd = __builtin_s390_vfnmsdb(vd, vd, vd);
-  // CHECK: [[NEG:%[^ ]+]] = fsub <2 x double> , %{{.*}}
+  // CHECK: [[NEG:%[^ ]+]] = fneg <2 x double> %{{.*}}
   // CHECK: [[RES:%[^ ]+]] = call <2 x double> @llvm.fma.v2f64(<2 x double> %{{.*}}, <2 x double> %{{.*}}, <2 x double> [[NEG]])
-  // CHECK: fsub <2 x double> , [[RES]]
+  // CHECK: fneg <2 x double> [[RES]]
 
   vsi = __builtin_s390_vfcesbs(vf, vf, &cc);
   // CHECK: call { <4 x i32>, i32 } @llvm.s390.vfcesbs(<

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

[James Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:2844
+The builtins ``__builtin_align_up``, ``__builtin_align_down``, return their
+first argument aligned up/down to the next multiple of the second argument.
+The builtin ``__builtin_is_aligned`` returns whether the first argument is

This should clearly state whether an already-aligned argument is left 
unmodified or incremented/decremented by the alignment amount. As it stands it 
reads more like such arguments will be modified, but the implementation is to 
round, ie the former.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D72085: [clangd] Fix hover for functions inside templated classes

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I landed a patch that does exactly this a few days ago:
14e11005d1a6ac1fecb230c470e9011d6956b8e4 


Did you pull the latest changes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72085



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

[PATCH] D72087: NFC: Fix trivial typos in comments

[Kazuaki Ishizaki via Phabricator via cfe-commits]

kiszk created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous.
Herald added a project: clang.

"the the" -> "the"
"an" -> "a"


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72087

Files:
  clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
  clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
  clang-tools-extra/clangd/CodeComplete.h
  clang-tools-extra/clangd/unittests/TweakTests.cpp
  clang-tools-extra/docs/doxygen.cfg.in


Index: clang-tools-extra/docs/doxygen.cfg.in
===
--- clang-tools-extra/docs/doxygen.cfg.in
+++ clang-tools-extra/docs/doxygen.cfg.in
@@ -46,7 +46,7 @@
 
 PROJECT_BRIEF  =
 
-# With the PROJECT_LOGO tag one can specify an logo or icon that is included in
+# With the PROJECT_LOGO tag one can specify a logo or icon that is included in
 # the documentation. The maximum height of the logo should not exceed 55 pixels
 # and the maximum width should not exceed 200 pixels. Doxygen will copy the 
logo
 # to the output directory.
Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -127,7 +127,7 @@
   ExtraArgs.push_back("-x");
   ExtraArgs.push_back("objective-c");
 
-  // Ensure the the action can be initiated in the string literal.
+  // Ensure the action can be initiated in the string literal.
   EXPECT_AVAILABLE(R"(id x = ^[[@[[^"^t^est^";)");
 
   // Ensure that the action can't be initiated in other places.
Index: clang-tools-extra/clangd/CodeComplete.h
===
--- clang-tools-extra/clangd/CodeComplete.h
+++ clang-tools-extra/clangd/CodeComplete.h
@@ -61,7 +61,7 @@
   bool IncludeIneligibleResults = false;
 
   /// Combine overloads into a single completion item where possible.
-  /// If none, the the implementation may choose an appropriate behavior.
+  /// If none, the implementation may choose an appropriate behavior.
   /// (In practice, ClangdLSPServer enables bundling if the client claims
   /// to supports signature help).
   llvm::Optional BundleOverloads;
Index: clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
@@ -114,7 +114,7 @@
  this);
 
   // Find ownership transfers via copy construction and assignment.
-  // AutoPtrOwnershipTransferId is bound to the the part that has to be wrapped
+  // AutoPtrOwnershipTransferId is bound to the part that has to be wrapped
   // into std::move().
   //   std::auto_ptr i, j;
   //   i = j;
Index: clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
===
--- clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
+++ clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
@@ -145,7 +145,7 @@
   ///
   /// In order to avoid this, this class looks at the container expression
   /// `arr[k]` and decides whether or not it contains a sub-expression declared
-  /// within the the loop body.
+  /// within the loop body.
   bool dependsOnInsideVariable(const clang::Stmt *Body) {
 DependsOnInsideVariable = false;
 TraverseStmt(const_cast(Body));
Index: clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
===
--- clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
+++ clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
@@ -867,7 +867,7 @@
   return OldIndex->getName();
 }
 
-/// Determines whether or not the the name \a Symbol conflicts with
+/// Determines whether or not the name \a Symbol conflicts with
 /// language keywords or defined macros. Also checks if the name exists in
 /// LoopContext, any of its parent contexts, or any of its child statements.
 ///
Index: clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
@@ -393,7 +393,7 @@
 }
 
 // If the destination array is the same length as the given length we have to
-// increase the capacity by one to create space for the the null terminator.
+// increase the capacity by one to create space for the null terminator.
 static bool isDestCapacityFix(const MatchFinder::MatchResult &Result,
   DiagnosticBuilder &Diag) {

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

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: llvm/test/lit.site.cfg.py.in:6
+def cmake_bool(val):
+return val.lower() in (1, "on", "yes", "true", "y",)
+

FYI, this is a now-discouraged pattern. It's better to use 
llvm_canonicalize_cmake_booleans in the cmake file instead. See 
https://reviews.llvm.org/D56912 for an example.


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] D71726: Let clang atomic builtins fetch add/sub support floating point types

[Olivier Giroux via Phabricator via cfe-commits]

__simt__ added a comment.

In D71726#1792852 , @yaxunl wrote:

> In D71726#1791904 , @jfb wrote:
>
> > This generally seems fine. Does it work on most backends? I want to make 
> > sure it doesn't fail in backends :)
>
>
> For x86_64, amdgcn, aarch64, armv7, mips64, it is translated to cmpxchg by 
> AtomicExpandPass and backends did codegen successfully.
>
> For hexagon, riscv32, it is translated to call of `__atomic_fetch_add_4` for 
> fadd float. This is concerning. Probably we need to add 
> `__atomic_fetch_{add|sub}_{f16|f32|f64}` ?


For systems that have load-link/store-conditional architectures, like ARM / PPC 
/ base RISC-V without extension, I would imagine that using a cmpxchg loop is 
much worse than simply doing the floating-point add/sub in the middle of the 
atomic mini-transaction. I'm sure that we want back-ends to be capable of 
implementing this better than what this pass is doing, even when they don't 
have "native" fp atomics.

You listed amdgcn... what does this do on nvptx?


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

https://reviews.llvm.org/D71726



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

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked 4 inline comments as done.
ilya-biryukov added inline comments.



Comment at: clang/include/clang/AST/Expr.h:126
+if (TD)
+  D = D | DependencyFlags::Type;
+if (VD)

riccibruno wrote:
> ilya-biryukov wrote:
> > Mordante wrote:
> > > Just curious why do you prefer `D = D | DependencyFlags::Type;` over `D 
> > > |= DependencyFlags::Type;` ? The latter seems to be more common.
> > Would also prefer `D |=`, but it leads to compilation errors.
> > 
> > The builtin `operator |=` accepts ints and, therefore, fails on 
> > strongly-typed enums.
> > And, AFAIK, there's no way to redefine `operator |=` for non-class types.
> You certainly can define a compound assignment operator for an enumeration 
> type. It is only non-compound-assignment that is special cased and required 
> to be a member function.
> 
> Example: https://godbolt.org/z/JV5uPw
Ah, thanks! So it turns out I was wrong. Will update the patch.



Comment at: clang/lib/Serialization/ASTReaderStmt.cpp:530
+  E->setDependencies(Deps);
+
   E->setValueKind(static_cast(Record.readInt()));

riccibruno wrote:
> I think it would be nicer to add an abbreviation for the right number of bits 
> (using `DependencyFlagsBits`) and as you say just serialize/deserialize all 
> the flags in one go.
Will do this in a follow-up.
That would be a functional change, I'm aiming to keep this one an NFC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71920



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

[PATCH] D32199: [TySan] A Type Sanitizer (Clang)

[CJ Johnson via Phabricator via cfe-commits]

CJ-Johnson commandeered this revision.
CJ-Johnson added a reviewer: hfinkel.
CJ-Johnson added a comment.

After discussing things with Hal, I'm going to take over these diffs and try to 
update them to the new pass manager :)


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

https://reviews.llvm.org/D32199



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

[PATCH] D72087: [clang-tools-extra] NFC: Fix trivial typos in comments

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

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61171 tests passed, 0 failed 
and 729 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72087



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

[PATCH] D72089: [Syntax] Build declarator nodes

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: gribozavr2.
Herald added a project: clang.
ilya-biryukov added a parent revision: D72072: [AST] Respect 
shouldTraversePostOrder when traversing type locs.

They cover part of types and names for some declarations, including
common cases like variables, functions, etc.

See the comment of Declarator for more details.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72089

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

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -173,17 +173,21 @@
 *: TranslationUnit
 |-SimpleDeclaration
 | |-int
-| |-main
-| |-(
-| |-)
+| |-SimpleDeclarator
+| | |-main
+| | `-ParametersAndQualifiers
+| |   |-(
+| |   `-)
 | `-CompoundStatement
 |   |-{
 |   `-}
 `-SimpleDeclaration
   |-void
-  |-foo
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-foo
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 `-}
@@ -200,9 +204,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-main
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-main
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-IfStatement
@@ -245,9 +251,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ForStatement
@@ -267,18 +275,21 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
-| | |-a
-| | |-=
-| | `-UnknownExpression
-| |   `-10
+| | `-SimpleDeclarator
+| |   |-a
+| |   |-=
+| |   `-UnknownExpression
+| | `-10
 | `-;
 `-}
 )txt"},
@@ -286,9 +297,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-EmptyStatement
@@ -308,9 +321,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-SwitchStatement
@@ -344,9 +359,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-WhileStatement
@@ -374,9 +391,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ReturnStatement
@@ -397,26 +416,31 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
-| | |-a
-| | |-[
-| | |-UnknownExpression
-| | | `-3
-| | `-]
+| | `-SimpleDeclarator
+| |   |-a
+| |   `-ArraySubscript
+| | |-[
+| | |-UnknownExpression
+| | | `-3
+| | `-]
 | `-;
 |-RangeBasedForStatement
 | |-for
 | |-(
 | |-SimpleDeclaration
 | | |-int
-| | |-x
+| | |-SimpleDeclarator
+| | | `-x
 | | `-:
 | |-UnknownExpression
 | | `-a
@@ -432,9 +456,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-main
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-main
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-UnknownStatement
@@ -459,9 +485,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ExpressionStatement
@@ -499,10 +527,12 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-*
-  |-a
+  |-SimpleDeclarator
+  | |-*
+  | `-a
   |-,
-  |-b
+  |-SimpleDeclarator
+  | `-b
   `-;
   )txt"},
   {R"cpp(
@@ -513,10 +543,12 @@
 `-SimpleDeclaration
   |-typedef
   |-int
-  |-*
-  |-a
+  |-SimpleDeclarator
+  | |-*
+  | `-a
   |-,
-  |-b
+  |-SimpleDeclarator
+  | `-b
   `-;
   )txt"},
   // Multiple declarators inside a statement.
@@ -530,27 +562,33 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-foo
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-foo
+  | `-ParametersAndQualifiers
+  |   |-(

[PATCH] D69878: Consoldiate internal denormal flushing controls

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

ping


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

https://reviews.llvm.org/D69878



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

[PATCH] D71854: [SystemZ] Use FNeg in s390x clang builtins

[Ulrich Weigand via Phabricator via cfe-commits]

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

LGTM, thanks!


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

https://reviews.llvm.org/D71854



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

[PATCH] D72089: [Syntax] Build declarator nodes

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

merge_guards_bot added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon question-circle color=gray} clang-tidy: unknown.

{icon question-circle color=gray} clang-format: unknown.

Build artifacts 
: 
diff.json 
,
 console-log.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72089



___
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

[serge via Phabricator via cfe-commits]

serge-sans-paille marked an inline comment as done.
serge-sans-paille added a comment.
serge-sans-paille added inline comments.



Comment at: llvm/test/lit.site.cfg.py.in:6
+def cmake_bool(val):
+return val.lower() in (1, "on", "yes", "true", "y",)
+

thakis wrote:
> FYI, this is a now-discouraged pattern. It's better to use 
> llvm_canonicalize_cmake_booleans in the cmake file instead. See 
> https://reviews.llvm.org/D56912 for an example.
Sure! Thanks for pointing at this, I'll fix that once the builds are back to 
normal.


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] D71596: [clangd] Improve documentation for auto and implicit specs

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.h:194
+llvm::SmallVector
+explicitReferenceTargets(ast_type_traits::DynTypedNode N,
+ DeclRelationSet Mask = {});

ilya-biryukov wrote:
> No need to fix this.
> 
> The name could probably be better, but we can fix this later. Don't have any 
> good ideas.
I think as a public API this should be clearer on how/when to use and its 
relation to other things (planning to send a patch, but wanted to discuss a bit 
here first).

- This is essentially a filter of allTargetDecls (as is targetDecl), but the 
relationship between the two isn't clear. They should have closely related 
names (variant) or maybe better be more orthogonal and composed at the callsite.
- The most distinctive word in the name is `explicit`, but this function 
doesn't actually have anything to do with explicit vs non-explicit references 
(just history?)
- It's not clear to me whether we actually want to encapsulate/hide the 
preference for instantiations over templates here, or whether it should be 
expressed at the callsite because the policy is decided feature-by-feature. 
`chooseBest(...)` vs `prefer(TemplateInstantiation, TemplatePattern, ...)`. The 
latter seems safer to me, based on experience with targetDecl so far.

A couple of ideas:
 - caller invokes allTargetDecls() and then this is a helper function 
`prefer(DeclRelation, DeclRelation, Results)` that mutates Results
 - targetDecl() becomes the swiss-army filter and accepts a list of 
"prefer-X-over-Y", which it applies before returning the results with flags 
dropped


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71596



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

[PATCH] D71554: [llvm-ranlib] Handle -D and -U command line flag

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D71554#1801246 , @arichardson wrote:

> Should be fixed in 
> https://github.com/llvm/llvm-project/commit/a4f3847f3d5742cfab7acdc614e7ca54643e0c85


Thanks! Now it fails with a different error: 
http://45.33.8.238/win/4962/step_11.txt


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71554



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

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

[Roman Lebedev via Phabricator via cfe-commits]

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

(temporarily undoing my review since it looks like things may change)

In D71499#1801302 , @arichardson wrote:

> ...


Aha! :)

In D71499#1801302 , @arichardson wrote:

> However, I think this means that for the downstream CHERI codegen
>  there is one more case where we need to be careful
>  (but that shouldn't matter for upstream).
>  For us `uintptr_t` is represented as `i8 addrspace(200)*`.
>  As `uintptr_t` can hold values that are plain integers and not pointers,
>  we would need to add the inbounds only if the AST type is a pointer,
>  and use a non-inbounds GEP for `(u)intptr_t`.


I may be missing some details, but won't we get that for free,
since we only do `gep` for pointers already?

In D71499#1801302 , @arichardson wrote:

> Does that sound correct or did I misunderstand something?


To //me// this all sounds great.
Since we get to dictate^W define semantics, i'd think we can require

In D71499#1801302 , @arichardson wrote:

> both input and output should be a `Logical pointer`.
>  If you want random values, you should be aligning uintptr_t instead.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[clang-tools-extra] acc4ffb - [clangd] Reorder FindTarget.h - group targetDecl() stuff and findExplicitReferences(). NFC

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-01-02T18:01:29+01:00
New Revision: acc4ffbb4733ec716d6ca3ad4d1e4605b9a2bcea

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

LOG: [clangd] Reorder FindTarget.h - group targetDecl() stuff and 
findExplicitReferences(). NFC

Added: 


Modified: 
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/FindTarget.h

Removed: 




diff  --git a/clang-tools-extra/clangd/FindTarget.cpp 
b/clang-tools-extra/clangd/FindTarget.cpp
index 55bb8a0b70ea..f9332ee4f220 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -617,13 +617,13 @@ llvm::SmallVector refInTypeLoc(TypeLoc 
L) {
 DependentTemplateSpecializationTypeLoc L) {
   Ref = ReferenceLoc{
   L.getQualifierLoc(), L.getTemplateNameLoc(), /*IsDecl=*/false,
-  explicitReferenceTargets(DynTypedNode::create(L.getType()))};
+  explicitReferenceTargets(DynTypedNode::create(L.getType()), {})};
 }
 
 void VisitDependentNameTypeLoc(DependentNameTypeLoc L) {
   Ref = ReferenceLoc{
   L.getQualifierLoc(), L.getNameLoc(), /*IsDecl=*/false,
-  explicitReferenceTargets(DynTypedNode::create(L.getType()))};
+  explicitReferenceTargets(DynTypedNode::create(L.getType()), {})};
 }
 
 void VisitTypedefTypeLoc(TypedefTypeLoc L) {

diff  --git a/clang-tools-extra/clangd/FindTarget.h 
b/clang-tools-extra/clangd/FindTarget.h
index 39c00b0f6df0..2f6c26ca6912 100644
--- a/clang-tools-extra/clangd/FindTarget.h
+++ b/clang-tools-extra/clangd/FindTarget.h
@@ -79,6 +79,36 @@ class DeclRelationSet;
 llvm::SmallVector
 targetDecl(const ast_type_traits::DynTypedNode &, DeclRelationSet Mask);
 
+/// Similar to targetDecl(), however instead of applying a filter, all possible
+/// decls are returned along with their DeclRelationSets.
+/// This is suitable for indexing, where everything is recorded and filtering
+/// is applied later.
+llvm::SmallVector, 1>
+allTargetDecls(const ast_type_traits::DynTypedNode &);
+
+enum class DeclRelation : unsigned {
+  // Template options apply when the declaration is an instantiated template.
+  // e.g. [[vector]] vec;
+
+  /// This is the template instantiation that was referred to.
+  /// e.g. template<> class vector (the implicit specialization)
+  TemplateInstantiation,
+  /// This is the pattern the template specialization was instantiated from.
+  /// e.g. class vector (the pattern within the primary template)
+  TemplatePattern,
+
+  // Alias options apply when the declaration is an alias.
+  // e.g. namespace clang { [[StringRef]] S; }
+
+  /// This declaration is an alias that was referred to.
+  /// e.g. using llvm::StringRef (the UsingDecl directly referenced).
+  Alias,
+  /// This is the underlying declaration for an alias, decltype etc.
+  /// e.g. class llvm::StringRef (the underlying declaration referenced).
+  Underlying,
+};
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, DeclRelation);
+
 /// Information about a reference written in the source code, independent of 
the
 /// actual AST node that this reference lives in.
 /// Useful for tools that are source-aware, e.g. refactorings.
@@ -111,35 +141,20 @@ void findExplicitReferences(const Decl *D,
 void findExplicitReferences(const ASTContext &AST,
 llvm::function_ref Out);
 
-/// Similar to targetDecl(), however instead of applying a filter, all possible
-/// decls are returned along with their DeclRelationSets.
-/// This is suitable for indexing, where everything is recorded and filtering
-/// is applied later.
-llvm::SmallVector, 1>
-allTargetDecls(const ast_type_traits::DynTypedNode &);
-
-enum class DeclRelation : unsigned {
-  // Template options apply when the declaration is an instantiated template.
-  // e.g. [[vector]] vec;
-
-  /// This is the template instantiation that was referred to.
-  /// e.g. template<> class vector (the implicit specialization)
-  TemplateInstantiation,
-  /// This is the pattern the template specialization was instantiated from.
-  /// e.g. class vector (the pattern within the primary template)
-  TemplatePattern,
-
-  // Alias options apply when the declaration is an alias.
-  // e.g. namespace clang { [[StringRef]] S; }
+/// Find declarations explicitly referenced in the source code defined by \p N.
+/// For templates, will prefer to return a template instantiation whenever
+/// possible. However, can also return a template pattern if the specialization
+/// cannot be picked, e.g. in dependent code or when there is no corresponding
+/// Decl for a template instantitation, e.g. for templated using decls:
+///template  using Ptr = T*;
+///Ptr x;
+///^~~ there is no Decl for 'Ptr', so we return the template pattern.
+/

[clang] 89d6c28 - [SystemZ] Use FNeg in s390x clang builtins

[Kevin P. Neal via cfe-commits]

Author: Kevin P. Neal
Date: 2020-01-02T12:14:43-05:00
New Revision: 89d6c288ba5adb20d92142e9425f7ab79b8f159e

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

LOG: [SystemZ] Use FNeg in s390x clang builtins

The s390x builtins are still using FSub instead of FNeg. Correct that.

Added: 


Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/CodeGen/builtins-systemz-vector.c
clang/test/CodeGen/builtins-systemz-vector2.c
clang/test/CodeGen/builtins-systemz-zvector.c
clang/test/CodeGen/builtins-systemz-zvector2.c

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index be02004bf54b..12517709573a 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -13292,9 +13292,8 @@ Value *CodeGenFunction::EmitSystemZBuiltinExpr(unsigned 
BuiltinID,
 Value *X = EmitScalarExpr(E->getArg(0));
 Value *Y = EmitScalarExpr(E->getArg(1));
 Value *Z = EmitScalarExpr(E->getArg(2));
-Value *Zero = llvm::ConstantFP::getZeroValueForNegation(ResultType);
 Function *F = CGM.getIntrinsic(Intrinsic::fma, ResultType);
-return Builder.CreateCall(F, {X, Y, Builder.CreateFSub(Zero, Z, "sub")});
+return Builder.CreateCall(F, {X, Y, Builder.CreateFNeg(Z, "neg")});
   }
   case SystemZ::BI__builtin_s390_vfnmasb:
   case SystemZ::BI__builtin_s390_vfnmadb: {
@@ -13302,9 +13301,8 @@ Value *CodeGenFunction::EmitSystemZBuiltinExpr(unsigned 
BuiltinID,
 Value *X = EmitScalarExpr(E->getArg(0));
 Value *Y = EmitScalarExpr(E->getArg(1));
 Value *Z = EmitScalarExpr(E->getArg(2));
-Value *Zero = llvm::ConstantFP::getZeroValueForNegation(ResultType);
 Function *F = CGM.getIntrinsic(Intrinsic::fma, ResultType);
-return Builder.CreateFSub(Zero, Builder.CreateCall(F, {X, Y, Z}), "sub");
+return Builder.CreateFNeg(Builder.CreateCall(F, {X, Y, Z}), "neg");
   }
   case SystemZ::BI__builtin_s390_vfnmssb:
   case SystemZ::BI__builtin_s390_vfnmsdb: {
@@ -13312,10 +13310,9 @@ Value 
*CodeGenFunction::EmitSystemZBuiltinExpr(unsigned BuiltinID,
 Value *X = EmitScalarExpr(E->getArg(0));
 Value *Y = EmitScalarExpr(E->getArg(1));
 Value *Z = EmitScalarExpr(E->getArg(2));
-Value *Zero = llvm::ConstantFP::getZeroValueForNegation(ResultType);
 Function *F = CGM.getIntrinsic(Intrinsic::fma, ResultType);
-Value *NegZ = Builder.CreateFSub(Zero, Z, "sub");
-return Builder.CreateFSub(Zero, Builder.CreateCall(F, {X, Y, NegZ}));
+Value *NegZ = Builder.CreateFNeg(Z, "neg");
+return Builder.CreateFNeg(Builder.CreateCall(F, {X, Y, NegZ}));
   }
   case SystemZ::BI__builtin_s390_vflpsb:
   case SystemZ::BI__builtin_s390_vflpdb: {
@@ -13328,9 +13325,8 @@ Value *CodeGenFunction::EmitSystemZBuiltinExpr(unsigned 
BuiltinID,
   case SystemZ::BI__builtin_s390_vflndb: {
 llvm::Type *ResultType = ConvertType(E->getType());
 Value *X = EmitScalarExpr(E->getArg(0));
-Value *Zero = llvm::ConstantFP::getZeroValueForNegation(ResultType);
 Function *F = CGM.getIntrinsic(Intrinsic::fabs, ResultType);
-return Builder.CreateFSub(Zero, Builder.CreateCall(F, X), "sub");
+return Builder.CreateFNeg(Builder.CreateCall(F, X), "neg");
   }
   case SystemZ::BI__builtin_s390_vfisb:
   case SystemZ::BI__builtin_s390_vfidb: {

diff  --git a/clang/test/CodeGen/builtins-systemz-vector.c 
b/clang/test/CodeGen/builtins-systemz-vector.c
index 85cdc30aa54c..116bd8fa492a 100644
--- a/clang/test/CodeGen/builtins-systemz-vector.c
+++ b/clang/test/CodeGen/builtins-systemz-vector.c
@@ -584,14 +584,14 @@ void test_float(void) {
   vd = __builtin_s390_vfmadb(vd, vd, vd);
   // CHECK: call <2 x double> @llvm.fma.v2f64(<2 x double> %{{.*}}, <2 x 
double> %{{.*}}, <2 x double> %{{.*}})
   vd = __builtin_s390_vfmsdb(vd, vd, vd);
-  // CHECK: [[NEG:%[^ ]+]] = fsub <2 x double> , %{{.*}}
+  // CHECK: [[NEG:%[^ ]+]] = fneg <2 x double> %{{.*}}
   // CHECK: call <2 x double> @llvm.fma.v2f64(<2 x double> %{{.*}}, <2 x 
double> %{{.*}}, <2 x double> [[NEG]])
 
   vd = __builtin_s390_vflpdb(vd);
   // CHECK: call <2 x double> @llvm.fabs.v2f64(<2 x double> %{{.*}})
   vd = __builtin_s390_vflndb(vd);
   // CHECK: [[ABS:%[^ ]+]] = call <2 x double> @llvm.fabs.v2f64(<2 x double> 
%{{.*}})
-  // CHECK: fsub <2 x double> , 
[[ABS]]
+  // CHECK: fneg <2 x double> [[ABS]]
 
   vd = __builtin_s390_vfidb(vd, 0, 0);
   // CHECK: call <2 x double> @llvm.rint.v2f64(<2 x double> %{{.*}})

diff  --git a/clang/test/CodeGen/builtins-systemz-vector2.c 
b/clang/test/CodeGen/builtins-systemz-vector2.c
index a17cdf0a5ae6..0a9906002f2b 100644
--- a/clang/test/CodeGen/builtins-systemz-vector2.c
+++ b/clang/test/CodeGen/builtins-systemz-vector2.c
@@ -64,11 +64,11 @@ void test_float(void) {
 
   vd = __builtin_s390_vfnmadb(vd,

[PATCH] D71854: [SystemZ] Use FNeg in s390x clang builtins

[Kevin P. Neal via Phabricator via cfe-commits]

kpn closed this revision.
kpn added a comment.

My pleasure!

Closed with commit 89d6c288ba5adb20d92142e9425f7ab79b8f159e 
.


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

https://reviews.llvm.org/D71854



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

[PATCH] D72018: [attributes] [analyzer] Add an attribute to prevent checkers from modeling a function

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

xazax.hun added a comment.

In D72018#1800018 , @aaron.ballman 
wrote:

> This seems like a very specialized attribute for the static analyzer and I'm 
> not certain how much utility it adds, but that may be because I don't 
> understand the analyzer requirements sufficiently yet. It seems as though 
> this attribute is intended to suppress diagnostics within a certain function 
> for a certain check, much like `gsl::suppress` does for the C++ Core 
> Guidelines. Is that correct? If so, perhaps we should just reuse that 
> attribute (but with a different spelling)?


In some sense they are similar, but not entirely. When I annotate a function 
with `gsl::suppress`, I would expect to suppress diagnostics inside the 
function. When I annotate a function with `analyzer_checker_ignore`, that would 
suppress diagnostics in the callers of the function. So while the 
`gsl::suppress` is merely a way to suppress diagnostics, this attribute does 
change how the analysis is carried out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72018



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

[PATCH] D71969: [OpenMP] diagnose zero-length array section in the depend clause

[Kelvin Li via Phabricator via cfe-commits]

kkwli0 updated this revision to Diff 235894.
kkwli0 added a comment.

Update based on suggestion to simply the check and rebase.


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

https://reviews.llvm.org/D71969

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_depend_messages.cpp
  clang/test/OpenMP/target_enter_data_depend_messages.cpp
  clang/test/OpenMP/target_exit_data_depend_messages.cpp
  clang/test/OpenMP/target_parallel_depend_messages.cpp
  clang/test/OpenMP/target_parallel_for_depend_messages.cpp
  clang/test/OpenMP/target_parallel_for_simd_depend_messages.cpp
  clang/test/OpenMP/target_simd_depend_messages.cpp
  clang/test/OpenMP/target_teams_depend_messages.cpp
  clang/test/OpenMP/target_teams_distribute_depend_messages.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_depend_messages.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_depend_messages.cpp
  clang/test/OpenMP/target_teams_distribute_simd_depend_messages.cpp
  clang/test/OpenMP/target_update_depend_messages.cpp
  clang/test/OpenMP/task_depend_messages.cpp

Index: clang/test/OpenMP/task_depend_messages.cpp
===
--- clang/test/OpenMP/task_depend_messages.cpp
+++ clang/test/OpenMP/task_depend_messages.cpp
@@ -45,7 +45,7 @@
   #pragma omp task depend (in : argv[argc: // expected-error {{expected expression}} expected-error {{expected ']'}} expected-error {{expected ')'}} expected-note {{to match this '['}} expected-note {{to match this '('}}
   #pragma omp task depend (in : argv[argc:argc] // expected-error {{expected ')'}} expected-note {{to match this '('}}
   #pragma omp task depend (in : argv[0:-1]) // expected-error {{section length is evaluated to a negative value -1}}
-  #pragma omp task depend (in : argv[-1:0])
+  #pragma omp task depend (in : argv[-1:0]) // expected-error {{zero-length array section is not allowed in 'depend' clause}}
   #pragma omp task depend (in : argv[:]) // expected-error {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
   #pragma omp task depend (in : argv[3:4:1]) // expected-error {{expected ']'}} expected-note {{to match this '['}}
   #pragma omp task depend(in:a[0:1]) // expected-error {{subscripted value is not an array or pointer}}
Index: clang/test/OpenMP/target_update_depend_messages.cpp
===
--- clang/test/OpenMP/target_update_depend_messages.cpp
+++ clang/test/OpenMP/target_update_depend_messages.cpp
@@ -15,7 +15,6 @@
   public:
 int operator[](int index) { return 0; }
 };
-
 template 
 int tmain(T argc, S **argv, R *env[]) {
   vector vec;
@@ -52,7 +51,7 @@
   #pragma omp target update to(z) depend(in : argv[argc: // expected-error {{expected expression}} expected-error {{expected ']'}} expected-error {{expected ')'}} expected-note {{to match this '['}} expected-note {{to match this '('}}
   #pragma omp target update to(z) depend(in : argv[argc:argc] // expected-error {{expected ')'}} expected-note {{to match this '('}}
   #pragma omp target update to(z) depend(in : argv[0:-1]) // expected-error {{section length is evaluated to a negative value -1}}
-  #pragma omp target update to(z) depend(in : argv[-1:0])
+  #pragma omp target update to(z) depend(in : argv[-1:0]) // expected-error {{zero-length array section is not allowed in 'depend' clause}}
   #pragma omp target update to(z) depend(in : argv[:]) // expected-error {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
   #pragma omp target update to(z) depend(in : argv[3:4:1]) // expected-error {{expected ']'}} expected-note {{to match this '['}}
   #pragma omp target update to(z) depend(in:a[0:1]) // expected-error {{subscripted value is not an array or pointer}}
@@ -64,7 +63,6 @@
 
   return 0;
 }
-
 int main(int argc, char **argv, char *env[]) {
   vector vec;
   typedef float V __attribute__((vector_size(16)));
@@ -100,7 +98,7 @@
   #pragma omp target update to(z) depend(in : argv[argc: // expected-error {{expected expression}} expected-error {{expected ']'}} expected-error {{expected ')'}} expected-note {{to match this '['}} expected-note {{to match this '('}}
   #pragma omp target update to(z) depend(in : argv[argc:argc] // expected-error {{expected ')'}} expected-note {{to match this '('}}
   #pragma omp target update to(z) depend(in : argv[0:-1]) // expected-error {{section length is evaluated to a negative value -1}}
-  #pragma omp target update to(z) depend(in : argv[-1:0])
+  #pragma omp target update to(z) depend(in : argv[-1:0]) // expected-error {{zero-length array section is not allowed in 'depend' clause}}
   #pragma omp target update to(z) depend(in : argv[:]) // expected-error {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
   #pragma omp ta

[PATCH] D69878: Consoldiate internal denormal flushing controls

[Cameron McInally via Phabricator via cfe-commits]

cameron.mcinally added a comment.

This is looking pretty good to me, but I'm ignoring some of the target specific 
code that I'm not familiar with.

Is `denormal-fp-math` influenced by `-Ofast`? Or are there plans for that? 
Seems like `-Ofast` should imply DAZ and FTZ (if supported by target).

I think we discussed this before, but it's worth repeating. If 
`denormal-fp-math` isn't specified, we default to IEEE behavior, right? When 
this lands in master, there could be an unexpected performance hit for targets 
that aren't paying attention. E.g. I want to use `denormal-fp-math` to toggle 
whether a FSUB(-0.0,X) is converted to a FNEG(X) in SelectionDAGBuilder.

Apologies in advance if this has been discussed recently. I've been distracted 
with another project for the passed few months...




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2311
+  bool TrappingMath = true;
 // overriden by ffp-exception-behavior?
   bool RoundingFPMath = false;

Last line of comment was not removed.

Also, is it safe to remove `TrappingMathPresent`? Is that part of the 
work-in-progress to support `ffp-exception-behavior`?


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

https://reviews.llvm.org/D69878



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

[PATCH] D72087: [clang-tools-extra] NFC: Fix trivial typos in comments

[Kazuaki Ishizaki via Phabricator via cfe-commits]

kiszk added a comment.

I do not have commit rights to the repository. Could someone commit this change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72087



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

[PATCH] D71969: [OpenMP] diagnose zero-length array section in the depend clause

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


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

https://reviews.llvm.org/D71969



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

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:66
+  StringRef S = "MyInt target = 0;";
+  auto Cat = [&T](StringRef S) { return (T + S).str(); };
+

JonasToth wrote:
> aaron.ballman wrote:
> > I don't think this is needed -- just add a newline between the literals and 
> > let string concat work its magic? (Same elsewhere)
> > ```
> > StringRef S = "typedef int MyInt;"
> >   "MyInt target = 0;";
> > ```
> My Idea was to highlight the expected code-change and create both the before 
> and after-version from the same snippets.
> With `S` being all the code, i would need to test for the full code.
> 
> I think with your proposal the test looks like this:
> 
> ```
> StringRef S = "typedef int MyInt;"
>   "MyInt target = 0;";
> EXPECT_EQ("typedef int MyInt;"
>   "const MyInt target = 0;",
>   runCheckOnCode(S));
> ```
> I prefer the current version, especially with the bigger test later this file.
Okay, that's a good point. Also, this is testing code, so if it's a bit... 
odd... it's not an issue.



Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:83-84
+
+  EXPECT_EQ(Cat("const MyInt target = nullptr;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("const MyInt target = nullptr;"),

JonasToth wrote:
> aaron.ballman wrote:
> > This does what I would expect, but boy does it make me sad to see (because 
> > `target` is not actually a `const int *` but is instead an `int * const`).
> You are right, but I am not sure what the proper policy should be. I think 
> the smarts to detect such potential improper const should be in the library 
> code calling the transformation.
> The utility should allow both transformations.
I think the API behaves how I would expect it to and agree that the caller 
should be the one deciding what to add the qualifier to. It just hurts me to 
see the test code verifying it (but the test code is good). ;-)



Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:1006
+}
+
+} // namespace test

JonasToth wrote:
> aaron.ballman wrote:
> > Can you also add some ObjC pointer tests?
> i added the most basic test, do you think more is necessary? I don't know a 
> lot about the objc languages.
Ah, those are regular C pointers, not ObjC pointers. I was thinking more along 
the lines of:
```
@class Object;
Object *g; // Try adding const to this

@interface Inter
- (void) foo1: (int *)arg1; // Try adding const to this parameter
@end
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54395



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

[PATCH] D72057: [docs] Update dead anchor in hacking page

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/www/hacking.html:301
 
-  It is also possible to https://llvm.org/docs/GettingStarted.html#sending-patches-with-git";>use 
git to contribute to Clang.
+  It is also possible to https://llvm.org/docs/GettingStarted.html#checkout-llvm-from-git";>use git 
to contribute to Clang.
 

AlexanderLanin wrote:
> aaron.ballman wrote:
> > Should this instead point to 
> > `https://llvm.org/docs/GettingStarted.html#sending-patches` to more closely 
> > match the original target anchor?
> For a precise match yes, but I figured that one would need to start with git 
> first of all as git is not mentioned anywhere else on the page.
That's fair, but I think this whole section needs a bit more love than what's 
proposed. You cannot use svn diff for creating patches within a git repo. This 
text only makes sense when we were still doing the transition from svn to git, 
and the bit you're changing is the "oh yeah, or you can use git if you want" 
stuff. Now we need it to read "This is how you do this with git", at which 
point the checkout from git link isn't as useful as pointing out how you send a 
patch (which is the next logical step after forming a patch file).

Would you like to take a stab at updating this section rather than just the 
link?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72057



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

[PATCH] D71463: Implement VectorType conditional operator GNU extension.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Ping?


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

https://reviews.llvm.org/D71463



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

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 235899.
arichardson marked 2 inline comments as done.
arichardson added a comment.

- Generate inbounds GEP. Also mention that values must point to the same 
allocation in the documentation.
- Clarify that correctly aligned values do not change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtin-align-array.c
  clang/test/CodeGen/builtin-align.c
  clang/test/Sema/builtin-align.c
  clang/test/SemaCXX/builtin-align-cxx.cpp

Index: clang/test/SemaCXX/builtin-align-cxx.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/builtin-align-cxx.cpp
@@ -0,0 +1,236 @@
+// C++-specific checks for the alignment builtins
+// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -std=c++11 -o - %s -fsyntax-only -verify
+
+// Check that we don't crash when using dependent types in __builtin_align:
+template 
+void *c(void *d) { // expected-note{{candidate template ignored}}
+  return __builtin_align_down(d, b);
+}
+
+struct x {};
+x foo;
+void test(void *value) {
+  c(value);
+  c(value); // expected-error{{no matching function for call to 'c'}}
+}
+
+template 
+void test_templated_arguments() {
+  T array[ArraySize];   // expected-error{{variable has incomplete type 'fwddecl'}}
+  static_assert(__is_same(decltype(__builtin_align_up(array, Alignment)), T *), // expected-error{{requested alignment is not a power of 2}}
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_align_down(array, Alignment)), T *),
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_is_aligned(array, Alignment)), bool),
+"return type should be bool");
+  T *x1 = __builtin_align_up(array, Alignment);
+  T *x2 = __builtin_align_down(array, Alignment);
+  bool x3 = __builtin_align_up(array, Alignment);
+}
+
+void test() {
+  test_templated_arguments(); // fine
+  test_templated_arguments();
+  // expected-note@-1{{in instantiation of function template specialization 'test_templated_arguments'}}
+  // expected-note@-2{{forward declaration of 'fwddecl'}}
+  test_templated_arguments(); // invalid alignment value
+  // expected-note@-1{{in instantiation of function template specialization 'test_templated_arguments'}}
+}
+
+template 
+void test_incorrect_alignment_without_instatiation(T value) {
+  int array[32];
+  static_assert(__is_same(decltype(__builtin_align_up(array, 31)), int *), // expected-error{{requested alignment is not a power of 2}}
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_align_down(array, 7)), int *), // expected-error{{requested alignment is not a power of 2}}
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_is_aligned(array, -1)), bool), // expected-error{{requested alignment must be 1 or greater}}
+"return type should be bool");
+  __builtin_align_up(array);   // expected-error{{too few arguments to function call, expected 2, have 1}}
+  __builtin_align_up(array, 31);   // expected-error{{requested alignment is not a power of 2}}
+  __builtin_align_down(array, 31); // expected-error{{requested alignment is not a power of 2}}
+  __builtin_align_up(array, 31);   // expected-error{{requested alignment is not a power of 2}}
+  __builtin_align_up(value, 31);   // This shouldn't want since the type is dependent
+  __builtin_align_up(value);   // Same here
+}
+
+// The original fix for the issue above broke some legitimate code.
+// Here is a regression test:
+typedef __SIZE_TYPE__ size_t;
+void *allocate_impl(size_t size);
+template 
+T *allocate() {
+  constexpr size_t allocation_size =
+  __builtin_align_up(sizeof(T), sizeof(void *));
+  return static_cast(
+  __builtin_assume_aligned(allocate_impl(allocation_size), sizeof(void *)));
+}
+struct Foo {
+  int value;
+};
+void *test2() {
+  return allocate();
+}
+
+// Check that pointers-to-members cannot be used:
+class MemPtr {
+public:
+  int data;
+  void func();
+  virtual void vfunc();
+};
+void test_member_ptr() {
+  __builtin_align_up(&MemPtr::data, 64);// expected-error{{operand of type 'int MemPtr::*' where arithmetic or pointer type is required}}
+  __builtin_align_down(&MemPtr::func, 64);  // expected-error{{operand of type 'void (MemPtr::*)()' where arithmetic or pointer type is required}}
+  __builtin_i

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

[Gabor Bencze via Phabricator via cfe-commits]

gbencze marked 18 inline comments as done.
gbencze added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp:1
+// RUN: %check_clang_tidy %s bugprone-suspicious-memory-comparison %t
+

JonasToth wrote:
> We should explicitly test, that the check runs under C-code, either with two 
> run-lines or with separate test files (my preference).
Should I duplicate the tests that are legal C or try to split it up so that 
only c++ specific code is tested in this one? 



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp:3
+
+typedef unsigned long long size_t;
+int memcmp(const void *lhs, const void *rhs, size_t count);

JonasToth wrote:
> I think the test could be sensitive to cpu-architectures.
> If you could bring up a case that is e.g. problematic on X86, but not on 
> anything else (powerpc, or arm or anything, 64bit vs 32bit) and demonstrate 
> with tests that such cases are caught as well this would be great. I see no 
> reason it wouldn't.
> 
> That would maybe justify another alias into `portability`, as this is an 
> issue in that case.
I may have misunderstood you but the check currently only warns if you're 
comparing padding on the current compilation target. 
Or do you mean adding another test file that is compiled e.g. with -m32 and 
testing if calling memcmp on the following doesn't warn there but does on 64bit?
```
struct S {
int x;
int* y;
};
```


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

https://reviews.llvm.org/D71973



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

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

[Gabor Bencze via Phabricator via cfe-commits]

gbencze updated this revision to Diff 235900.
gbencze added a comment.

Coding guide and better diagnostic message for padding comparison


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

https://reviews.llvm.org/D71973

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.h
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-memory-comparison.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-exp42-c.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp
@@ -0,0 +1,373 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-memory-comparison %t
+
+typedef __SIZE_TYPE__ size_t;
+int memcmp(const void *lhs, const void *rhs, size_t count);
+
+namespace std {
+int memcmp(const void *lhs, const void *rhs, size_t count);
+}
+
+namespace sei_cert_example_exp42_c {
+struct s {
+  char c;
+  int i;
+  char buffer[13];
+};
+
+void noncompliant(const struct s *left, const struct s *right) {
+  if ((left && right) && (0 == memcmp(left, right, sizeof(struct s {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: comparing padding data in type
+// sei_cert_example_exp42_c::s; consider comparing the fields manually
+  }
+}
+
+void compliant(const struct s *left, const struct s *right) {
+  if ((left && right) && (left->c == right->c) && (left->i == right->i) &&
+  (0 == memcmp(left->buffer, right->buffer, 13))) {
+  }
+}
+} // namespace sei_cert_example_exp42_c
+
+namespace sei_cert_example_exp42_c_ex1 {
+#pragma pack(push, 1)
+struct s {
+  char c;
+  int i;
+  char buffer[13];
+};
+#pragma pack(pop)
+
+void compare(const struct s *left, const struct s *right) {
+  if ((left && right) && (0 == memcmp(left, right, sizeof(struct s {
+// no-warning
+  }
+}
+} // namespace sei_cert_example_exp42_c_ex1
+
+namespace sei_cert_example_oop57_cpp {
+class C {
+  int i;
+
+public:
+  virtual void f();
+};
+
+void f(C &c1, C &c2) {
+  if (!std::memcmp(&c1, &c2, sizeof(C))) {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: comparing object representation
+// of non-standard-layout type sei_cert_example_oop57_cpp::C; consider
+// using a comparison operator instead
+  }
+}
+} // namespace sei_cert_example_oop57_cpp
+
+namespace trailing_padding {
+struct S {
+  int i;
+  char c;
+};
+
+void test() {
+  S a, b;
+  memcmp(&a, &b, sizeof(S));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // trailing_padding::S; consider comparing the fields manually
+  memcmp(&a, &b, sizeof(int));
+  memcmp(&a, &b, sizeof(int) + sizeof(char));
+  memcmp(&a, &b, 2 * sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // trailing_padding::S; consider comparing the fields manually
+}
+} // namespace trailing_padding
+
+namespace inner_padding {
+struct S {
+  char c;
+  int i;
+};
+
+void test() {
+  S a, b;
+  memcmp(&a, &b, sizeof(S));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // inner_padding::S; consider comparing the fields manually
+  memcmp(&a, &b, sizeof(char) + sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // inner_padding::S; consider comparing the fields manually
+  memcmp(&a, &b, sizeof(char));
+  memcmp(&a, &b, 2 * sizeof(char));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // inner_padding::S; consider comparing the fields manually
+}
+} // namespace inner_padding
+
+namespace bitfield {
+struct S {
+  int x : 10;
+  int y : 6;
+};
+
+void test() {
+  S a, b;
+  memcmp(&a, &b, sizeof(S));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // bitfield::S; consider comparing the fields manually
+  memcmp(&a, &b, 2); // no-warning: no padding in first 2 bytes
+}
+} // namespace bitfield
+
+namespace bitfield_2 {
+struct S {
+  int x : 10;
+  int y : 7;
+};
+
+void test() {
+  S a, b;
+  memcmp(&a, &b, sizeof(S));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // bitfield_2::S; consider comparing the fields manually
+  memcmp(&a, &b, 2);
+  memcmp(&a, &b, 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // bitfield_2::S; consider comparing the fields manually
+}
+} // namespace bitfield_2
+
+namespace bitfield_3 {
+struct S {
+  int x : 2

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In D71499#1801421 , @lebedev.ri wrote:

> (temporarily undoing my review since it looks like things may change)
>
> In D71499#1801302 , @arichardson 
> wrote:
>
> > ...
>
>
> Aha! :)
>
> In D71499#1801302 , @arichardson 
> wrote:
>
> > However, I think this means that for the downstream CHERI codegen
> >  there is one more case where we need to be careful
> >  (but that shouldn't matter for upstream).
> >  For us `uintptr_t` is represented as `i8 addrspace(200)*`.
> >  As `uintptr_t` can hold values that are plain integers and not pointers,
> >  we would need to add the inbounds only if the AST type is a pointer,
> >  and use a non-inbounds GEP for `(u)intptr_t`.
>
>
> I may be missing some details, but won't we get that for free,
>  since we only do `gep` for pointers already?


I believe we would need something like this downstream instead of always 
creating an inbounds GEP:

  if (E->getType()->isPointerType()) {
// The result must point to the same underlying allocation. This means we
// can use an inbounds GEP to enable better optimization.
Result = Builder.CreateInBoundsGEP(EmitCastToVoidPtr(Args.Src),
   Difference, "aligned_result");
  } else {
// However, for when performing operations on __intcap_t (which is also
// a pointer type in LLVM IR), we cannot set the inbounds flag as the
// result could be either an arbitrary integer value or a valid pointer.
// Setting the inbounds flag for the arbitrary integer case is not safe.
assert(E->getType()->isIntCapType());
Result = Builder.CreateGEP(EmitCastToVoidPtr(Args.Src), Difference,
   "aligned_result");
  }



> In D71499#1801302 , @arichardson 
> wrote:
> 
>> Does that sound correct or did I misunderstand something?
> 
> 
> To //me// this all sounds great.
>  Since we get to dictate^W define semantics, i'd think we can require
> 
> In D71499#1801302 , @arichardson 
> wrote:
> 
>> both input and output should be a `Logical pointer`.
>>  If you want random values, you should be aligning uintptr_t instead.

Thank you very much for the useful suggestion!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

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

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61176 tests passed, 0 failed 
and 729 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 235903.
arichardson added a comment.

- Fix typos caused by dodgy n key on my keyboard.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtin-align-array.c
  clang/test/CodeGen/builtin-align.c
  clang/test/Sema/builtin-align.c
  clang/test/SemaCXX/builtin-align-cxx.cpp

Index: clang/test/SemaCXX/builtin-align-cxx.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/builtin-align-cxx.cpp
@@ -0,0 +1,236 @@
+// C++-specific checks for the alignment builtins
+// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -std=c++11 -o - %s -fsyntax-only -verify
+
+// Check that we don't crash when using dependent types in __builtin_align:
+template 
+void *c(void *d) { // expected-note{{candidate template ignored}}
+  return __builtin_align_down(d, b);
+}
+
+struct x {};
+x foo;
+void test(void *value) {
+  c(value);
+  c(value); // expected-error{{no matching function for call to 'c'}}
+}
+
+template 
+void test_templated_arguments() {
+  T array[ArraySize];   // expected-error{{variable has incomplete type 'fwddecl'}}
+  static_assert(__is_same(decltype(__builtin_align_up(array, Alignment)), T *), // expected-error{{requested alignment is not a power of 2}}
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_align_down(array, Alignment)), T *),
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_is_aligned(array, Alignment)), bool),
+"return type should be bool");
+  T *x1 = __builtin_align_up(array, Alignment);
+  T *x2 = __builtin_align_down(array, Alignment);
+  bool x3 = __builtin_align_up(array, Alignment);
+}
+
+void test() {
+  test_templated_arguments(); // fine
+  test_templated_arguments();
+  // expected-note@-1{{in instantiation of function template specialization 'test_templated_arguments'}}
+  // expected-note@-2{{forward declaration of 'fwddecl'}}
+  test_templated_arguments(); // invalid alignment value
+  // expected-note@-1{{in instantiation of function template specialization 'test_templated_arguments'}}
+}
+
+template 
+void test_incorrect_alignment_without_instatiation(T value) {
+  int array[32];
+  static_assert(__is_same(decltype(__builtin_align_up(array, 31)), int *), // expected-error{{requested alignment is not a power of 2}}
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_align_down(array, 7)), int *), // expected-error{{requested alignment is not a power of 2}}
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_is_aligned(array, -1)), bool), // expected-error{{requested alignment must be 1 or greater}}
+"return type should be bool");
+  __builtin_align_up(array);   // expected-error{{too few arguments to function call, expected 2, have 1}}
+  __builtin_align_up(array, 31);   // expected-error{{requested alignment is not a power of 2}}
+  __builtin_align_down(array, 31); // expected-error{{requested alignment is not a power of 2}}
+  __builtin_align_up(array, 31);   // expected-error{{requested alignment is not a power of 2}}
+  __builtin_align_up(value, 31);   // This shouldn't want since the type is dependent
+  __builtin_align_up(value);   // Same here
+}
+
+// The original fix for the issue above broke some legitimate code.
+// Here is a regression test:
+typedef __SIZE_TYPE__ size_t;
+void *allocate_impl(size_t size);
+template 
+T *allocate() {
+  constexpr size_t allocation_size =
+  __builtin_align_up(sizeof(T), sizeof(void *));
+  return static_cast(
+  __builtin_assume_aligned(allocate_impl(allocation_size), sizeof(void *)));
+}
+struct Foo {
+  int value;
+};
+void *test2() {
+  return allocate();
+}
+
+// Check that pointers-to-members cannot be used:
+class MemPtr {
+public:
+  int data;
+  void func();
+  virtual void vfunc();
+};
+void test_member_ptr() {
+  __builtin_align_up(&MemPtr::data, 64);// expected-error{{operand of type 'int MemPtr::*' where arithmetic or pointer type is required}}
+  __builtin_align_down(&MemPtr::func, 64);  // expected-error{{operand of type 'void (MemPtr::*)()' where arithmetic or pointer type is required}}
+  __builtin_is_aligned(&MemPtr::vfunc, 64); // expected-error{{operand of type 'void (MemPtr::*)()' where arithmetic or pointer type is required}}
+}
+
+void test_referenc

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Hmm, i keep coming up with things here, sorry :/

I think we should take one more step - explicitly call out that the result of 
`__builtin_align_*` is, well, aligned :)
For that, we need to add `__attribute__((alloc_align(2)))` attribute.
Despite the name, it does not imply anything about provenance/aliasing/etc, 
only about alignment:
https://godbolt.org/z/9bAjxK




Comment at: clang/docs/LanguageExtensions.rst:2854-2855
+qualifiers such as ``const``) with an adjusted address.
+When aligning pointers up or down, the resulting value must be within the same
+underlyig allocation (or one past the end), i.e., arbitrary integer values
+stored in pointer-type variables must not be passed to these builtins.

Might be worth explicitly calling out C17 6.5.6p8, C++ [expr.add]?



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14329-14330
+// can use an inbounds GEP to enable better optimization.
+Result = Builder.CreateInBoundsGEP(EmitCastToVoidPtr(Args.Src), Difference,
+   "aligned_result");
+Result = Builder.CreatePointerCast(Result, Args.SrcType);

I would suspect this should follow suit of the rest of clang codegen, i.e. do
```
Value* Base = EmitCastToVoidPtr(Args.Src);
if (CGF.getLangOpts().isSignedOverflowDefined())
  Result = Builder.CreateGEP(Base, Difference, "aligned_result");
else
  Result = CGF.EmitCheckedInBoundsGEP(Base, Difference,
 /*SignedIndices=*/true, /*isSubtraction*/=false,
 E->getExprLoc(), "aligned_result");
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

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

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61176 tests passed, 0 failed 
and 729 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp:1
+// RUN: %check_clang_tidy %s bugprone-suspicious-memory-comparison %t
+

gbencze wrote:
> JonasToth wrote:
> > We should explicitly test, that the check runs under C-code, either with 
> > two run-lines or with separate test files (my preference).
> Should I duplicate the tests that are legal C or try to split it up so that 
> only c++ specific code is tested in this one? 
Rather splitting. Duplication might be painfull in the future.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp:3
+
+typedef unsigned long long size_t;
+int memcmp(const void *lhs, const void *rhs, size_t count);

gbencze wrote:
> JonasToth wrote:
> > I think the test could be sensitive to cpu-architectures.
> > If you could bring up a case that is e.g. problematic on X86, but not on 
> > anything else (powerpc, or arm or anything, 64bit vs 32bit) and demonstrate 
> > with tests that such cases are caught as well this would be great. I see no 
> > reason it wouldn't.
> > 
> > That would maybe justify another alias into `portability`, as this is an 
> > issue in that case.
> I may have misunderstood you but the check currently only warns if you're 
> comparing padding on the current compilation target. 
> Or do you mean adding another test file that is compiled e.g. with -m32 and 
> testing if calling memcmp on the following doesn't warn there but does on 
> 64bit?
> ```
> struct S {
> int x;
> int* y;
> };
> ```
Yes. Because it is only tested for the current archtiecture the buildbots will 
probably fail on it, because they test many architectures.

I think it is necessary to specify the arch in the RUN-line (`%t -- -- -target 
x86_64-unknown-unknown`) at the end of it.

And yes: Another test-file would be great to demonstrate that it works as 
expected.


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

https://reviews.llvm.org/D71973



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

[clang-tools-extra] ec3d8e6 - Handle init statements in readability-else-after-return

[Aaron Ballman via cfe-commits]

Author: Nathan James
Date: 2020-01-02T13:39:27-05:00
New Revision: ec3d8e61b527c6312f77a4dab095ffc34e954927

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

LOG: Handle init statements in readability-else-after-return

Adds a new ASTMatcher condition called 'hasInitStatement()' that matches if,
switch and range-for statements with an initializer. Reworked clang-tidy
readability-else-after-return to handle variables in the if condition or init
statements in c++17 ifs. Also checks if removing the else would affect object
lifetimes in the else branch.

Fixes PR44364.

Added: 

clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return-no-warn.cpp

Modified: 
clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.h
clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
clang/docs/LibASTMatchersReference.html
clang/include/clang/ASTMatchers/ASTMatchers.h
clang/lib/ASTMatchers/Dynamic/Registry.cpp
clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
index 520586dfe25d..93b197a52a52 100644
--- a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -10,6 +10,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Tooling/FixIt.h"
+#include "llvm/ADT/SmallVector.h"
 
 using namespace clang::ast_matchers;
 
@@ -17,45 +18,226 @@ namespace clang {
 namespace tidy {
 namespace readability {
 
+namespace {
+static const char ReturnStr[] = "return";
+static const char ContinueStr[] = "continue";
+static const char BreakStr[] = "break";
+static const char ThrowStr[] = "throw";
+static const char WarningMessage[] = "do not use 'else' after '%0'";
+static const char WarnOnUnfixableStr[] = "WarnOnUnfixable";
+
+const DeclRefExpr *findUsage(const Stmt *Node, int64_t DeclIdentifier) {
+  if (const auto *DeclRef = dyn_cast(Node)) {
+if (DeclRef->getDecl()->getID() == DeclIdentifier) {
+  return DeclRef;
+}
+  } else {
+for (const Stmt *ChildNode : Node->children()) {
+  if (const DeclRefExpr *Result = findUsage(ChildNode, DeclIdentifier)) {
+return Result;
+  }
+}
+  }
+  return nullptr;
+}
+
+const DeclRefExpr *
+findUsageRange(const Stmt *Node,
+   const llvm::iterator_range &DeclIdentifiers) {
+  if (const auto *DeclRef = dyn_cast(Node)) {
+if (llvm::is_contained(DeclIdentifiers, DeclRef->getDecl()->getID())) {
+  return DeclRef;
+}
+  } else {
+for (const Stmt *ChildNode : Node->children()) {
+  if (const DeclRefExpr *Result =
+  findUsageRange(ChildNode, DeclIdentifiers)) {
+return Result;
+  }
+}
+  }
+  return nullptr;
+}
+
+const DeclRefExpr *checkInitDeclUsageInElse(const IfStmt *If) {
+  const auto *InitDeclStmt = dyn_cast_or_null(If->getInit());
+  if (!InitDeclStmt)
+return nullptr;
+  if (InitDeclStmt->isSingleDecl()) {
+const Decl *InitDecl = InitDeclStmt->getSingleDecl();
+assert(isa(InitDecl) && "SingleDecl must be a VarDecl");
+return findUsage(If->getElse(), InitDecl->getID());
+  }
+  llvm::SmallVector DeclIdentifiers;
+  for (const Decl *ChildDecl : InitDeclStmt->decls()) {
+assert(isa(ChildDecl) && "Init Decls must be a VarDecl");
+DeclIdentifiers.push_back(ChildDecl->getID());
+  }
+  return findUsageRange(If->getElse(), DeclIdentifiers);
+}
+
+const DeclRefExpr *checkConditionVarUsageInElse(const IfStmt *If) {
+  const VarDecl *CondVar = If->getConditionVariable();
+  return CondVar != nullptr ? findUsage(If->getElse(), CondVar->getID())
+: nullptr;
+}
+
+bool containsDeclInScope(const Stmt *Node) {
+  if (isa(Node)) {
+return true;
+  }
+  if (const auto *Compound = dyn_cast(Node)) {
+return llvm::any_of(Compound->body(), [](const Stmt *SubNode) {
+  return isa(SubNode);
+});
+  }
+  return false;
+}
+
+void removeElseAndBrackets(DiagnosticBuilder &Diag, ASTContext &Context,
+   const Stmt *Else, SourceLocation ElseLoc) {
+  auto Remap = [&](SourceLocation Loc) {
+return Context.getSourceManager().getExpansionLoc(Loc);
+  };
+  auto TokLen = [&](SourceLocation Loc) {
+return Lexer::MeasureTokenLength(Loc, Context.getSourceManager(),
+ Context.getLangOpts());
+  };
+
+  if (const auto *CS = dyn_cast(Else)) {
+Diag << tooling::fixit::createRemoval(ElseLoc);
+SourceLocation LBrace = CS->getLBracLoc();
+SourceL

[clang-tools-extra] 7ab9acd - Fix trivial typos in comments; NFC

[Aaron Ballman via cfe-commits]

Author: Kazuaki Ishizaki
Date: 2020-01-02T13:41:43-05:00
New Revision: 7ab9acd8f414161b784b61a1633a7c241b82be85

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

LOG: Fix trivial typos in comments; NFC

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
clang-tools-extra/clangd/CodeComplete.h
clang-tools-extra/clangd/unittests/TweakTests.cpp
clang-tools-extra/docs/doxygen.cfg.in

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
index 4c9b546e225f..e49688b96be7 100644
--- a/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
@@ -393,7 +393,7 @@ static bool isDestExprFix(const MatchFinder::MatchResult 
&Result,
 }
 
 // If the destination array is the same length as the given length we have to
-// increase the capacity by one to create space for the the null terminator.
+// increase the capacity by one to create space for the null terminator.
 static bool isDestCapacityFix(const MatchFinder::MatchResult &Result,
   DiagnosticBuilder &Diag) {
   bool IsOverflows = isDestCapacityOverflows(Result);

diff  --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp 
b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
index 0389a6148954..6cb4276ce1b8 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
@@ -867,7 +867,7 @@ std::string VariableNamer::createIndexName() {
   return OldIndex->getName();
 }
 
-/// Determines whether or not the the name \a Symbol conflicts with
+/// Determines whether or not the name \a Symbol conflicts with
 /// language keywords or defined macros. Also checks if the name exists in
 /// LoopContext, any of its parent contexts, or any of its child statements.
 ///

diff  --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h 
b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
index 9aea8f6ca57f..b2cba8c0172a 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
@@ -145,7 +145,7 @@ class DependencyFinderASTVisitor
   ///
   /// In order to avoid this, this class looks at the container expression
   /// `arr[k]` and decides whether or not it contains a sub-expression declared
-  /// within the the loop body.
+  /// within the loop body.
   bool dependsOnInsideVariable(const clang::Stmt *Body) {
 DependsOnInsideVariable = false;
 TraverseStmt(const_cast(Body));

diff  --git a/clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
index 8a2d7f028d4b..9ed13775dd72 100644
--- a/clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
@@ -114,7 +114,7 @@ void ReplaceAutoPtrCheck::registerMatchers(MatchFinder 
*Finder) {
  this);
 
   // Find ownership transfers via copy construction and assignment.
-  // AutoPtrOwnershipTransferId is bound to the the part that has to be wrapped
+  // AutoPtrOwnershipTransferId is bound to the part that has to be wrapped
   // into std::move().
   //   std::auto_ptr i, j;
   //   i = j;

diff  --git a/clang-tools-extra/clangd/CodeComplete.h 
b/clang-tools-extra/clangd/CodeComplete.h
index dda4f44590cd..fd01dfd80b0e 100644
--- a/clang-tools-extra/clangd/CodeComplete.h
+++ b/clang-tools-extra/clangd/CodeComplete.h
@@ -61,7 +61,7 @@ struct CodeCompleteOptions {
   bool IncludeIneligibleResults = false;
 
   /// Combine overloads into a single completion item where possible.
-  /// If none, the the implementation may choose an appropriate behavior.
+  /// If none, the implementation may choose an appropriate behavior.
   /// (In practice, ClangdLSPServer enables bundling if the client claims
   /// to supports signature help).
   llvm::Optional BundleOverloads;

diff  --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp 
b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index 8627439d4555..88decb6a04ef 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -127,7 +127,7 @@ TEST_F(ObjCLocalizeStringLiteralTest, Test) {
   ExtraArgs.push_back("-x");
   ExtraArgs.push_back("objective-c");
 
-  // Ensure the

[PATCH] D72087: [clang-tools-extra] NFC: Fix trivial typos in comments

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

Thank you for the patch! I've commit on your behalf in 
7ab9acd8f414161b784b61a1633a7c241b82be85 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72087



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

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

Thank you for the patch, I've committed on your behalf in 
ec3d8e61b527c6312f77a4dab095ffc34e954927 



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

https://reviews.llvm.org/D71846



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

[PATCH] D71991: Fix external-names.c test when separator is \\

[Adrian McCarthy via Phabricator via cfe-commits]

amccarth added a comment.

It would surprise me if the different test config is cause here.  The path is 
assembled entirely in the code, so it should be consistent.  If you had a 
systemic problem introduced by test config, I'd expect more of the VFS tests to 
fail.

Anyway, if you're back in business, that's great, but I can't promise future 
VFS changes won't cause similar problems for you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71991



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

[clang] abb0075 - build: reduce CMake handling for zlib

[Saleem Abdulrasool via cfe-commits]

Author: Saleem Abdulrasool
Date: 2020-01-02T11:19:12-08:00
New Revision: abb00753069554c538f3d850897373d093389945

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

LOG: build: reduce CMake handling for zlib

Rather than handling zlib handling manually, use `find_package` from CMake
to find zlib properly. Use this to normalize the `LLVM_ENABLE_ZLIB`,
`HAVE_ZLIB`, `HAVE_ZLIB_H`. Furthermore, require zlib if `LLVM_ENABLE_ZLIB` is
set to `YES`, which requires the distributor to explicitly select whether
zlib is enabled or not. This simplifies the CMake handling and usage in
the rest of the tooling.

This restores 68a235d07f9e7049c7eb0c8091f37e385327ac28,
e6c7ed6d2164a0659fd9f6ee44f1375d301e3cad.  The problem with the windows
bot is a need for clearing the cache.

Added: 


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

Removed: 




diff  --git a/clang/test/CMakeLists.txt b/clang/test/CMakeLists.txt
index 16e487f07b78..6f6121f06e56 100644
--- a/clang/test/CMakeLists.txt
+++ b/clang/test/CMakeLists.txt
@@ -9,22 +9,13 @@ endif ()
 
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} CLANG_TOOLS_DIR 
${LLVM_RUNTIME_OUTPUT_INTDIR})
 
-if(CLANG_BUILT_STANDALONE)
-  # Set HAVE_LIBZ according to recorded LLVM_ENABLE_ZLIB value. This
-  # value is forced to 0 if zlib was not found, so it is fine to use it
-  # instead of HAVE_LIBZ (not recorded).
-  if(LLVM_ENABLE_ZLIB)
-set(HAVE_LIBZ 1)
-  endif()
-endif()
-
 llvm_canonicalize_cmake_booleans(
   CLANG_BUILD_EXAMPLES
   CLANG_ENABLE_ARCMT
   CLANG_ENABLE_STATIC_ANALYZER
   ENABLE_BACKTRACES
   ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER
-  HAVE_LIBZ
+  LLVM_ENABLE_ZLIB
   LLVM_ENABLE_PER_TARGET_RUNTIME_DIR
   LLVM_ENABLE_PLUGINS
   LLVM_ENABLE_THREADS)

diff  --git a/clang/test/lit.site.cfg.py.in b/clang/test/lit.site.cfg.py.in
index 520afab6af82..e9b35ac01771 100644
--- a/clang/test/lit.site.cfg.py.in
+++ b/clang/test/lit.site.cfg.py.in
@@ -16,7 +16,7 @@ config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.host_cxx = "@CMAKE_CXX_COMPILER@"
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.clang_arcmt = @CLANG_ENABLE_ARCMT@
 config.clang_default_cxx_stdlib = "@CLANG_DEFAULT_CXX_STDLIB@"
 config.clang_staticanalyzer = @CLANG_ENABLE_STATIC_ANALYZER@

diff  --git a/compiler-rt/test/lit.common.configured.in 
b/compiler-rt/test/lit.common.configured.in
index b4862f74cdd0..0fb51741783e 100644
--- a/compiler-rt/test/lit.common.configured.in
+++ b/compiler-rt/test/lit.common.configured.in
@@ -50,7 +50,7 @@ if config.enable_per_target_runtime_dir:
 else:
   set_default("target_suffix", "-%s" % config.target_arch)
 
-set_default("have_zlib", "@HAVE_LIBZ@")
+set_default("have_zlib", "@LLVM_ENABLE_ZLIB@")
 set_default("libcxx_used", "@LLVM_LIBCXX_USED@")
 
 # LLVM tools dir can be passed in lit parameters, so try to

diff  --git a/lld/test/CMakeLists.txt b/lld/test/CMakeLists.txt
index 8be42c46dd8a..dc8cedf2ea09 100644
--- a/lld/test/CMakeLists.txt
+++ b/lld/test/CMakeLists.txt
@@ -4,17 +4,8 @@ set(LLVM_BUILD_MODE "%(build_mode)s")
 set(LLVM_TOOLS_DIR "${LLVM_TOOLS_BINARY_DIR}/%(build_config)s")
 set(LLVM_LIBS_DIR 
"${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/%(build_config)s")
 
-if(LLD_BUILT_STANDALONE)
-  # Set HAVE_LIBZ according to recorded LLVM_ENABLE_ZLIB value. This
-  # value is forced to 0 if zlib was not found, so it is fine to use it
-  # instead of HAVE_LIBZ (not recorded).
-  if(LLVM_ENABLE_ZLIB)
-set(HAVE_LIBZ 1)
-  endif()
-endif()
-
 llvm_canonicalize_cmake_booleans(
-  HAVE_LIBZ
+  LLVM_ENABLE_ZLIB
   LLVM_LIBXML2_ENABLED
   )
 

diff  --git a/lld/test/lit.site.cfg.py.in b/lld/test/lit.site.cfg.py.in
index 02840f8d6a30..531fce15839d 100644
--- a/lld/test/lit.site.cfg.py.in
+++ b/lld/test/lit.site.cfg.py.in
@@ -14,7 +14,7 @@ config.lld_libs_dir = "@LLVM_LIBRARY_OUTPUT_INTDIR@"
 config.lld_tools_dir = "@LLVM_RUNTIME_OUTPUT_INTDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.python_executable = "@PYTHON_EXECUTABLE@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.sizeof_void_p = @CMAKE_SIZEOF_VOID_P@
 

[PATCH] D71857: [NFC] Fixes -Wrange-loop-analysis warnings

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

In D71857#1800663 , @MaskRay wrote:

> However, I am afraid I don't like some of the fixes here. You can replace 
> `const auto` with `const auto &` and call that a fix... IMHO if the type is 
> not obvious, `const ConcreteType &` will be better.


I notice some parts of the code prefer `auto` and others `ConcreteType`, so 
there is no consensus on what is the best. I feel with this change I want to 
change as little as possible.

> I think there is a false positive.
> 
> https://github.com/llvm/llvm-project/tree/master/lld/ELF/Relocations.cpp#L1622
> 
>   for (const std::pair ts : isd->thunkSections)
>
> 
> Diagnostic:
> 
>   lld/ELF/Relocations.cpp:1622:14: note: use reference type 'const 
> std::pair &' (aka 'const 
> pair &') to prevent copying
>   for (const std::pair ts : 
> isd->thunkSections)
>   
> 
> 
> i.e. The diagnostic asks me to replace `const std::pair` with 
> `const std::pair &`, when it is clear that the type is cheap to 
> copy.

The code has a 'protection' for this case `clang/lib/Sema/SemaStmt.cpp:2803`:

  // TODO: Determine a maximum size that a POD type can be before a diagnostic
  // should be emitted.  Also, only ignore POD types with trivial copy
  // constructors.
  if (VariableType.isPODType(SemaRef.Context))
return;

I could change this protection to test whether the type is trivially copyable 
which will reduce the number of warnings. Unfortunately std::pair is not 
trivially copyable so that won't help here. Do you think it makes sense to 
allow trivially copyable types instead of POD types? Any suggestion how we 
could solve it for std::pair?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71857



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

[PATCH] D72018: [attributes] [analyzer] Add an attribute to prevent checkers from modeling a function

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D72018#1801450 , @xazax.hun wrote:

> In D72018#1800018 , @aaron.ballman 
> wrote:
>
> > This seems like a very specialized attribute for the static analyzer and 
> > I'm not certain how much utility it adds, but that may be because I don't 
> > understand the analyzer requirements sufficiently yet. It seems as though 
> > this attribute is intended to suppress diagnostics within a certain 
> > function for a certain check, much like `gsl::suppress` does for the C++ 
> > Core Guidelines. Is that correct? If so, perhaps we should just reuse that 
> > attribute (but with a different spelling)?
>
>
> In some sense they are similar, but not entirely. When I annotate a function 
> with `gsl::suppress`, I would expect to suppress diagnostics inside the 
> function. When I annotate a function with `analyzer_checker_ignore`, that 
> would suppress diagnostics in the callers of the function. So while the 
> `gsl::suppress` is merely a way to suppress diagnostics, this attribute does 
> change how the analysis is carried out.


Thank you for the explanation, that makes sense, but I'm still a bit 
uncomfortable. In this case, it seems like the author of the API needs to 1) 
know that users will be analyzing the code with clang's static analyzer, 2) and 
that a particular function will cause problems for a specific check. It seems 
like the API author won't be in a position to add the attribute to the correct 
APIs, and what we really need is something for a *user* to write on an existing 
declaration they may not control or have the ability to modify the declaration 
of. I am not certain how much I like this idea, but perhaps we could allow the 
attribute to be written on a redeclaration and apply it to the canonical 
declaration so that users could add the attribute instead of relying on the 
library author to do it for them?

Also, does this mean that changing the name of a check in the static analyzer 
will now have the potential to break code? e.g., if a check move from the alpha 
group to the core group, the check name changes, and thus code needs to be 
updated. I presume one of the things this attribute will do is diagnose when an 
unknown check name is given? This has the potential to introduce diagnostics 
where a library is written for a newer clang but being compiled within an older 
clang, so we'd probably need a warning flag or something else to control the 
diagnostic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72018



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

[PATCH] D71554: [llvm-ranlib] Handle -D and -U command line flag

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added a comment.

In D71554#1789360 , @arichardson wrote:

> Also handle -h/-v as short options. Does the adjusted test look okay?


Sorry, didn't have time to take a second look before the holiday break -- yep, 
looks good, I didn't anticipate so many buildbot failures. Would be nice if the 
precommit bot tested Windows too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71554



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

[PATCH] D72018: [attributes] [analyzer] Add an attribute to prevent checkers from modeling a function

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

xazax.hun added a comment.

In D72018#1801725 , @aaron.ballman 
wrote:

> Thank you for the explanation, that makes sense, but I'm still a bit 
> uncomfortable. In this case, it seems like the author of the API needs to 1) 
> know that users will be analyzing the code with clang's static analyzer, 2) 
> and that a particular function will cause problems for a specific check. It 
> seems like the API author won't be in a position to add the attribute to the 
> correct APIs, and what we really need is something for a *user* to write on 
> an existing declaration they may not control or have the ability to modify 
> the declaration of. I am not certain how much I like this idea, but perhaps 
> we could allow the attribute to be written on a redeclaration and apply it to 
> the canonical declaration so that users could add the attribute instead of 
> relying on the library author to do it for them?


This is a valid concern. Currently, this is a common practice in the static 
analyzer to exclude certain functions from the checks, but until now, we 
usually hard coded the name/signature of the function rather than relying on an 
attribute. This did make sense, since it is an implementation detail that 
belongs to the analyzer. The main reason why we was thinking about making this 
an annotation, because if there is an annotation, when the API changes, we do 
not need to update the analyzer, and do not need to make sure that the analyzer 
version and the API matches.

> Also, does this mean that changing the name of a check in the static analyzer 
> will now have the potential to break code? e.g., if a check move from the 
> alpha group to the core group, the check name changes, and thus code needs to 
> be updated. I presume one of the things this attribute will do is diagnose 
> when an unknown check name is given? This has the potential to introduce 
> diagnostics where a library is written for a newer clang but being compiled 
> within an older clang, so we'd probably need a warning flag or something else 
> to control the diagnostic.

This is correct. If we want to diagnose non-existent checker names we 
definitely would need a diagnostic flag and probably we want to have the check 
off by default. The reason other reason (apart from backward compatibility) is 
that, some users might load non-upstreamed checkers as plugins for some builds 
(but not for others).

I think if there are many problems with this concept, I will fall back to hard 
code this information in the checker instead of using an annotation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72018



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

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

[Gabor Bencze via Phabricator via cfe-commits]

gbencze updated this revision to Diff 235915.
gbencze added a comment.

Tests: Split C/C++ tests and add 32/64bit specific test.


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

https://reviews.llvm.org/D71973

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.h
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-memory-comparison.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-exp42-c.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison-32bits.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.c
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-memory-comparison %t \
+// RUN: -- -- -target x86_64-unknown-unknown
+
+namespace std {
+typedef __SIZE_TYPE__ size_t;
+int memcmp(const void *lhs, const void *rhs, size_t count);
+} // namespace std
+
+namespace sei_cert_example_oop57_cpp {
+class C {
+  int i;
+
+public:
+  virtual void f();
+};
+
+void f(C &c1, C &c2) {
+  if (!std::memcmp(&c1, &c2, sizeof(C))) {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: comparing object representation
+// of non-standard-layout type sei_cert_example_oop57_cpp::C; consider
+// using a comparison operator instead
+  }
+}
+} // namespace sei_cert_example_oop57_cpp
+
+namespace inner_padding_64bit_only {
+struct S {
+  int x;
+  int *y;
+};
+
+void test() {
+  S a, b;
+  std::memcmp(&a, &b, sizeof(S));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // inner_padding_64bit_only::S; consider comparing the fields manually
+}
+} // namespace inner_padding_64bit_only
+
+namespace padding_in_base {
+class Base {
+  char c;
+  int i;
+};
+
+class Derived : public Base {};
+
+class Derived2 : public Derived {};
+
+void testDerived() {
+  Derived a, b;
+  std::memcmp(&a, &b, sizeof(char));
+  std::memcmp(&a, &b, sizeof(Base));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // padding_in_base::Derived; consider comparing the fields manually
+  std::memcmp(&a, &b, sizeof(Derived));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // padding_in_base::Derived; consider comparing the fields manually
+}
+
+void testDerived2() {
+  Derived2 a, b;
+  std::memcmp(&a, &b, sizeof(char));
+  std::memcmp(&a, &b, sizeof(Base));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // padding_in_base::Derived2; consider comparing the fields manually
+  std::memcmp(&a, &b, sizeof(Derived2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // padding_in_base::Derived2; consider comparing the fields manually
+}
+
+} // namespace padding_in_base
+
+namespace no_padding_in_base {
+class Base {
+  int a, b;
+};
+
+class Derived : public Base {};
+
+class Derived2 : public Derived {};
+
+void testDerived() {
+  Derived a, b;
+  std::memcmp(&a, &b, sizeof(Base));
+  std::memcmp(&a, &b, sizeof(Derived));
+}
+
+void testDerived2() {
+  Derived2 a, b;
+  std::memcmp(&a, &b, sizeof(char));
+  std::memcmp(&a, &b, sizeof(Base));
+  std::memcmp(&a, &b, sizeof(Derived2));
+}
+} // namespace no_padding_in_base
+
+namespace non_standard_layout {
+class C {
+private:
+  int x;
+
+public:
+  int y;
+};
+
+void test() {
+  C a, b;
+  std::memcmp(&a, &b, sizeof(C));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation
+  // of non-standard-layout type non_standard_layout::C; consider using a
+  // comparison operator instead
+}
+
+} // namespace non_standard_layout
+
+namespace static_ignored {
+struct S {
+  static char c;
+  int i;
+};
+
+void test() {
+  S a, b;
+  std::memcmp(&a, &b, sizeof(S));
+}
+
+} // namespace static_ignored
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.c
@@ -0,0 +1,224 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-memory-comparison %t \
+// RUN: -- -- -target x86_64-unknown-unknown -std=c99
+
+typedef __SIZE_TYPE__ size_t;
+int memcmp(const void *lhs, const void *rhs, size_t count);
+
+// Examples from cert

[PATCH] D72097: [LifetimeAnalysis] Do not forbid void deref type in gsl::Pointer/gsl::Owner annotations

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

xazax.hun created this revision.
xazax.hun added reviewers: aaron.ballman, gribozavr2, mgehre.
xazax.hun added a project: clang.
Herald added subscribers: Szelethus, Charusso, gamesh411, dkrupp, rnkovacs.

It turns out it is useful to be able to define the deref type as void. In case 
we have a type erased owner, we want to express that the pointee can be 
basically any type. I think it should not be unnatural to have a void deref 
type as we already familiar with "pointers to void".


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72097

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/SemaCXX/attr-gsl-owner-pointer.cpp


Index: clang/test/SemaCXX/attr-gsl-owner-pointer.cpp
===
--- clang/test/SemaCXX/attr-gsl-owner-pointer.cpp
+++ clang/test/SemaCXX/attr-gsl-owner-pointer.cpp
@@ -31,9 +31,11 @@
 // CHECK: OwnerAttr {{.*}} int
 
 class [[gsl::Owner(void)]] OwnerVoidDerefType{};
-// expected-error@-1 {{'void' is an invalid argument to attribute 'Owner'}}
+// CHECK: CXXRecordDecl {{.*}} OwnerVoidDerefType
+// CHECK: OwnerAttr {{.*}} void
 class [[gsl::Pointer(void)]] PointerVoidDerefType{};
-// expected-error@-1 {{'void' is an invalid argument to attribute 'Pointer'}}
+// CHECK: CXXRecordDecl {{.*}} PointerVoidDerefType
+// CHECK: PointerAttr {{.*}} void
 
 class [[gsl::Pointer(int)]] AddConflictLater{};
 // CHECK: CXXRecordDecl {{.*}} AddConflictLater
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -2913,7 +2913,7 @@
   if (!ParmType->isExtVectorType() && !ParmType->isFloatingType() &&
   (ParmType->isBooleanType() ||
!ParmType->isIntegralType(S.getASTContext( {
-S.Diag(AL.getLoc(), diag::err_attribute_invalid_argument) << 3 << AL;
+S.Diag(AL.getLoc(), diag::err_attribute_invalid_argument) << 2 << AL;
 return;
   }
 
@@ -4454,12 +4454,10 @@
 ParmType = S.GetTypeFromParser(AL.getTypeArg(), &DerefTypeLoc);
 
 unsigned SelectIdx = ~0U;
-if (ParmType->isVoidType())
+if (ParmType->isReferenceType())
   SelectIdx = 0;
-else if (ParmType->isReferenceType())
-  SelectIdx = 1;
 else if (ParmType->isArrayType())
-  SelectIdx = 2;
+  SelectIdx = 1;
 
 if (SelectIdx != ~0U) {
   S.Diag(AL.getLoc(), diag::err_attribute_invalid_argument)
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2631,7 +2631,7 @@
 def err_attributes_are_not_compatible : Error<
   "%0 and %1 attributes are not compatible">;
 def err_attribute_invalid_argument : Error<
-  "%select{'void'|a reference type|an array type|a non-vector or "
+  "%select{a reference type|an array type|a non-vector or "
   "non-vectorizable scalar type}0 is an invalid argument to attribute %1">;
 def err_attribute_wrong_number_arguments : Error<
   "%0 attribute %plural{0:takes no arguments|1:takes one argument|"


Index: clang/test/SemaCXX/attr-gsl-owner-pointer.cpp
===
--- clang/test/SemaCXX/attr-gsl-owner-pointer.cpp
+++ clang/test/SemaCXX/attr-gsl-owner-pointer.cpp
@@ -31,9 +31,11 @@
 // CHECK: OwnerAttr {{.*}} int
 
 class [[gsl::Owner(void)]] OwnerVoidDerefType{};
-// expected-error@-1 {{'void' is an invalid argument to attribute 'Owner'}}
+// CHECK: CXXRecordDecl {{.*}} OwnerVoidDerefType
+// CHECK: OwnerAttr {{.*}} void
 class [[gsl::Pointer(void)]] PointerVoidDerefType{};
-// expected-error@-1 {{'void' is an invalid argument to attribute 'Pointer'}}
+// CHECK: CXXRecordDecl {{.*}} PointerVoidDerefType
+// CHECK: PointerAttr {{.*}} void
 
 class [[gsl::Pointer(int)]] AddConflictLater{};
 // CHECK: CXXRecordDecl {{.*}} AddConflictLater
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -2913,7 +2913,7 @@
   if (!ParmType->isExtVectorType() && !ParmType->isFloatingType() &&
   (ParmType->isBooleanType() ||
!ParmType->isIntegralType(S.getASTContext( {
-S.Diag(AL.getLoc(), diag::err_attribute_invalid_argument) << 3 << AL;
+S.Diag(AL.getLoc(), diag::err_attribute_invalid_argument) << 2 << AL;
 return;
   }
 
@@ -4454,12 +4454,10 @@
 ParmType = S.GetTypeFromParser(AL.getTypeArg(), &DerefTypeLoc);
 
 unsigned SelectIdx = ~0U;
-if (ParmType->isVoidType())
+if (ParmType->isReferenceType())
   SelectIdx = 0;
-else if (ParmType->isReferenceType())
-  SelectIdx = 1;
 else if (ParmType->isArrayType())
-  SelectIdx = 2;
+  SelectIdx = 1;
 
 if (SelectIdx != ~0U) {
   S.Diag(AL.getLoc(), diag

[PATCH] D71227: [cuda][hip] Fix function overload resolution in the global initiailizer.

[Michael Liao via Phabricator via cfe-commits]

hliao marked 6 inline comments as done.
hliao added a comment.

refinements are made after comments from reviewers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71227



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

[PATCH] D71227: [cuda][hip] Fix function overload resolution in the global initiailizer.

[Michael Liao via Phabricator via cfe-commits]

hliao updated this revision to Diff 235921.
hliao added a comment.

code refinement after reviewers' comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71227

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCUDA/function-overload.cu
  clang/test/SemaCUDA/global-initializers-host.cu
  clang/test/SemaCUDA/hip-pinned-shadow.cu

Index: clang/test/SemaCUDA/hip-pinned-shadow.cu
===
--- clang/test/SemaCUDA/hip-pinned-shadow.cu
+++ clang/test/SemaCUDA/hip-pinned-shadow.cu
@@ -13,13 +13,19 @@
 
 template 
 struct texture : public textureReference {
+// expected-note@-1{{candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 0 were provided}}
+// expected-note@-2{{candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 0 were provided}}
+// expected-note@-3{{candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 0 were provided}}
+// expected-note@-4{{candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 0 were provided}}
 texture() { a = 1; }
+// expected-note@-1{{candidate constructor not viable: call to __host__ function from __device__ function}}
+// expected-note@-2{{candidate constructor not viable: call to __host__ function from __device__ function}}
 };
 
 __hip_pinned_shadow__ texture tex;
 __device__ __hip_pinned_shadow__ texture tex2; // expected-error{{'hip_pinned_shadow' and 'device' attributes are not compatible}}
-// expected-error@-1{{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables}}
-// expected-note@-2{{conflicting attribute is here}}
+// expected-note@-1{{conflicting attribute is here}}
+// expected-error@-2{{no matching constructor for initialization of 'texture'}}
 __constant__ __hip_pinned_shadow__ texture tex3; // expected-error{{'hip_pinned_shadow' and 'constant' attributes are not compatible}}
-  // expected-error@-1{{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables}}
-  // expected-note@-2{{conflicting attribute is here}}
+  // expected-note@-1{{conflicting attribute is here}}
+  // expected-error@-2{{no matching constructor for initialization of 'texture'}}
Index: clang/test/SemaCUDA/global-initializers-host.cu
===
--- clang/test/SemaCUDA/global-initializers-host.cu
+++ clang/test/SemaCUDA/global-initializers-host.cu
@@ -6,12 +6,14 @@
 // module initializer.
 
 struct S {
+  // expected-note@-1 {{candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 0 were provided}}
+  // expected-note@-2 {{candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 0 were provided}}
   __device__ S() {}
-  // expected-note@-1 {{'S' declared here}}
+  // expected-note@-1 {{candidate constructor not viable: call to __device__ function from __host__ function}}
 };
 
 S s;
-// expected-error@-1 {{reference to __device__ function 'S' in global initializer}}
+// expected-error@-1 {{no matching constructor for initialization of 'S'}}
 
 struct T {
   __host__ __device__ T() {}
@@ -19,14 +21,17 @@
 T t;  // No error, this is OK.
 
 struct U {
+  // expected-note@-1 {{candidate constructor (the implicit copy constructor) not viable: no known conversion from 'int' to 'const U' for 1st argument}}
+  // expected-note@-2 {{candidate constructor (the implicit move constructor) not viable: no known conversion from 'int' to 'U' for 1st argument}}
   __host__ U() {}
+  // expected-note@-1 {{candidate constructor not viable: requires 0 arguments, but 1 was provided}}
   __device__ U(int) {}
-  // expected-note@-1 {{'U' declared here}}
+  // expected-note@-1 {{candidate constructor not viable: call to __device__ function from __host__ function}}
 };
 U u(42);
-// expected-error@-1 {{reference to __device__ function 'U' in global initializer}}
+// expected-error@-1 {{no matching constructor for initialization of 'U'}}
 
 __device__ int device_fn() { return 42; }
-// expected-note@-1 {{'device_fn' declared here}}
+// expected-note@-1 {{candidate function not viable: call