[clang-tools-extra] 0dc2589 - [clangd] Attempt at fixing ExternalIndex tests on windows

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-11-23T09:16:06+01:00
New Revision: 0dc2589d4a72474f3956d4472ad25a1085dda260

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

LOG: [clangd] Attempt at fixing ExternalIndex tests on windows

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp 
b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
index 9e7fc5a49868..2b4605eb97e2 100644
--- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -229,11 +229,13 @@ TEST_F(ConfigCompileTests, ExternalBlockErrOnNoSource) {
 }
 
 TEST_F(ConfigCompileTests, ExternalBlockDisablesBackgroundIndex) {
-  Parm.Path = "/foo/bar/baz.h";
+  auto BazPath = testPath("foo/bar/baz.h", llvm::sys::path::Style::posix);
+  Parm.Path = BazPath;
   Frag.Index.Background.emplace("Build");
   Fragment::IndexBlock::ExternalBlock External;
-  External.File.emplace("/foo");
-  External.MountPoint.emplace("/foo/bar");
+  External.File.emplace(testPath("foo"));
+  External.MountPoint.emplace(
+  testPath("foo/bar", llvm::sys::path::Style::posix));
   Frag.Index.External = std::move(External);
   compileAndApply();
   EXPECT_EQ(Conf.Index.Background, Config::BackgroundPolicy::Skip);
@@ -245,14 +247,15 @@ TEST_F(ConfigCompileTests, ExternalBlockMountPoint) {
 Fragment Frag;
 Frag.Source.Directory = Directory.str();
 Fragment::IndexBlock::ExternalBlock External;
-External.File.emplace("/foo");
+External.File.emplace(testPath("foo"));
 if (MountPoint)
   External.MountPoint.emplace(*MountPoint);
 Frag.Index.External = std::move(External);
 return Frag;
   };
 
-  Parm.Path = "/foo/bar.h";
+  auto BarPath = testPath("foo/bar.h", llvm::sys::path::Style::posix);
+  Parm.Path = BarPath;
   // Non-absolute MountPoint without a directory raises an error.
   Frag = GetFrag("", "foo");
   compileAndApply();
@@ -264,41 +267,44 @@ TEST_F(ConfigCompileTests, ExternalBlockMountPoint) {
 DiagKind(llvm::SourceMgr::DK_Error;
   ASSERT_FALSE(Conf.Index.External);
 
+  auto FooPath = testPath("foo/", llvm::sys::path::Style::posix);
   // Ok when relative.
-  Frag = GetFrag("/", "foo");
+  Frag = GetFrag(testRoot(), "foo/");
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
   ASSERT_TRUE(Conf.Index.External);
-  EXPECT_THAT(Conf.Index.External->MountPoint, "/foo");
+  EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
 
   // None defaults to ".".
-  Frag = GetFrag("/", llvm::None);
+  Frag = GetFrag(FooPath, llvm::None);
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
   ASSERT_TRUE(Conf.Index.External);
-  EXPECT_THAT(Conf.Index.External->MountPoint, "/");
+  EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
 
   // Without a file, external index is empty.
   Parm.Path = "";
-  Frag = GetFrag("", "/foo");
+  Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
   ASSERT_FALSE(Conf.Index.External);
 
   // File outside MountPoint, no index.
-  Parm.Path = "/bar/baz.h";
-  Frag = GetFrag("", "/foo");
+  auto BazPath = testPath("bar/baz.h", llvm::sys::path::Style::posix);
+  Parm.Path = BazPath;
+  Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
   ASSERT_FALSE(Conf.Index.External);
 
   // File under MountPoint, index should be set.
-  Parm.Path = "/foo/baz.h";
-  Frag = GetFrag("", "/foo");
+  BazPath = testPath("foo/baz.h", llvm::sys::path::Style::posix);
+  Parm.Path = BazPath;
+  Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
   ASSERT_TRUE(Conf.Index.External);
-  EXPECT_THAT(Conf.Index.External->MountPoint, "/foo");
+  EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
 }
 } // namespace
 } // namespace config



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

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Thanks, this looks nicer now.




Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:247
+  if (const auto *Destructor = dyn_cast(D))
+return canonicalRenameDecl(Destructor->getParent());
+  if (const auto *VarTemplate = dyn_cast(D))

ctor & dtor is one of the kinds of `CXXMethodDecl`, I think we can merge the 
ctor & dtor cases into the `if (const auto *Method = 
dyn_cast(D)) {`. 



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:257
+->getTemplatedDecl();
+  if (const auto *Method = dyn_cast(D)) {
+if (const FunctionDecl *InstantiatedMethod =

using `else if` would be more clear  -- as now we modify `D` is the preceding 
`if`, if the D is assigned to a CXXMethodDecl in the `preceding` if, then it 
may fall into this branch.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:262
+while (Method->isVirtual() && Method->size_overridden_methods())
+  Method = *Method->overridden_methods().begin();
+D = Method;

oh, it is a tricky case where a method overrides > 1 virtual methods. Looks 
like we will regress this case in this patch, e.g.

```
class Foo {
  virtual void foo();
};

class Bar {
  virtual void foo();
};

class C : public Foo, Bar {
  void foo() override; // -> rename foo => bar
};
```

prior to the patch, both `Foo::foo` and `Bar::foo` will be renamed, after this 
patch, only one of them will be renamed (with within-file rename)? I think it 
is ok, but probably need some comments here.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:265
+  }
+  if (const auto *Function = dyn_cast(D))
+if (const FunctionTemplateDecl *Template = Function->getPrimaryTemplate())

the same, `else`

IIUC, `CXXMethodDecl` must be processed before `FunctionDecl`, I think we can 
add an assertion (the FunctionDecl must not be `CXXMethodDecl`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71880

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

[PATCH] D91721: [clangd] textDocument/implementation (LSP layer)

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 marked 3 inline comments as done.
usaxena95 added a comment.

> Looks like the patch is based on the old revision (pre-merging tests are 
> failing), I assume you have fixed the failure tests last week?

Yes. That was fixed last week. Rebased.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91721

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

[PATCH] D91721: [clangd] textDocument/implementation (LSP layer)

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 306969.
usaxena95 added a comment.

Addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91721

Files:
  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/test/implementations.test
  clang-tools-extra/clangd/test/initialize-params.test

Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -67,7 +67,8 @@
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "hoverProvider": true,
-# CHECK-NEXT:  "memoryUsageProvider": true
+# CHECK-NEXT:  "implementationProvider": true,
+# CHECK-NEXT:  "memoryUsageProvider": true,
 # CHECK-NEXT:  "referencesProvider": true,
 # CHECK-NEXT:  "renameProvider": true,
 # CHECK-NEXT:  "selectionRangeProvider": true,
Index: clang-tools-extra/clangd/test/implementations.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/implementations.test
@@ -0,0 +1,39 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"struct Parent { virtual void Foo(); };\nstruct Child1 : Parent { void Foo() override(); };\nstruct Child2 : Parent { void Foo() override(); };"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/implementation","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":32}}}
+#  CHECK:  "id": 1
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 33,
+# CHECK-NEXT:  "line": 1
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 30,
+# CHECK-NEXT:  "line": 1
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "uri": "file://{{.*}}/clangd-test/main.cpp"
+# CHECK-NEXT:},
+# CHECK-NEXT:{
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 33,
+# CHECK-NEXT:  "line": 2
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 30,
+# CHECK-NEXT:  "line": 2
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:"uri": "file://{{.*}}/clangd-test/main.cpp"
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -253,6 +253,10 @@
   /// Retrieve ranges that can be used to fold code within the specified file.
   void foldingRanges(StringRef File, Callback> CB);
 
+  /// Retrieve implementations for virtual method.
+  void findImplementations(PathRef File, Position Pos,
+   Callback> CB);
+
   /// Retrieve locations for symbol references.
   void findReferences(PathRef File, Position Pos, uint32_t Limit,
   Callback CB);
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -718,6 +718,18 @@
TUScheduler::InvalidateOnUpdate);
 }
 
+void ClangdServer::findImplementations(
+PathRef File, Position Pos, Callback> CB) {
+  auto Action = [Pos, CB = std::move(CB),
+ this](llvm::Expected InpAST) mutable {
+if (!InpAST)
+  return CB(InpAST.takeError());
+CB(clangd::findImplementations(InpAST->AST, Pos, Index));
+  };
+
+  WorkScheduler.runWithAST("Implementations", File, std::move(Action));
+}
+
 void ClangdServer::findReferences(PathRef File, Position Pos, uint32_t Limit,
   Callback CB) {
   auto Action = [Pos, Limit, CB = std::move(CB),
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -115,6 +115,8 @@
  Callback>);
   void onGoToDefinition(const TextDocumentPositionParams &,
 Callback>);
+  void onGoToImplementation(const TextDocumentPosi

[PATCH] D91721: [clangd] textDocument/implementation (LSP layer)

[Haojian Wu via Phabricator via cfe-commits]

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

thanks, looks good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91721

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

[clang-tools-extra] 66ace4d - [clang-tidy] Fix a nullptr-access crash in unused-raii-check.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-11-23T09:44:19+01:00
New Revision: 66ace4dc0275c8d7740bc5ff57c20e85e6660371

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

LOG: [clang-tidy] Fix a nullptr-access crash in unused-raii-check.

I saw this crash in our internal production, but unfortunately didn't get
reproduced testcase, we likely hit this crash when the AST is ill-formed
(e.g. broken code).

Reviewed By: gribozavr2

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

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
index 70ce413d20ff..5e4a0ba6d569 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
@@ -79,12 +79,11 @@ void UnusedRaiiCheck::check(const MatchFinder::MatchResult 
&Result) {
   // written type.
   auto Matches =
   match(expr(hasDescendant(typeLoc().bind("t"))), *E, *Result.Context);
-  const auto *TL = selectFirst("t", Matches);
-  assert(TL);
-  D << FixItHint::CreateInsertion(
-  Lexer::getLocForEndOfToken(TL->getEndLoc(), 0, *Result.SourceManager,
- getLangOpts()),
-  Replacement);
+  if (const auto *TL = selectFirst("t", Matches))
+D << FixItHint::CreateInsertion(
+Lexer::getLocForEndOfToken(TL->getEndLoc(), 0, *Result.SourceManager,
+   getLangOpts()),
+Replacement);
 }
 
 } // namespace bugprone



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

[PATCH] D91614: [clang-tidy] Fix a nullptr-access crash in unused-raii-check.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG66ace4dc0275: [clang-tidy] Fix a nullptr-access crash in 
unused-raii-check. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91614

Files:
  clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp


Index: clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
@@ -79,12 +79,11 @@
   // written type.
   auto Matches =
   match(expr(hasDescendant(typeLoc().bind("t"))), *E, *Result.Context);
-  const auto *TL = selectFirst("t", Matches);
-  assert(TL);
-  D << FixItHint::CreateInsertion(
-  Lexer::getLocForEndOfToken(TL->getEndLoc(), 0, *Result.SourceManager,
- getLangOpts()),
-  Replacement);
+  if (const auto *TL = selectFirst("t", Matches))
+D << FixItHint::CreateInsertion(
+Lexer::getLocForEndOfToken(TL->getEndLoc(), 0, *Result.SourceManager,
+   getLangOpts()),
+Replacement);
 }
 
 } // namespace bugprone


Index: clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
@@ -79,12 +79,11 @@
   // written type.
   auto Matches =
   match(expr(hasDescendant(typeLoc().bind("t"))), *E, *Result.Context);
-  const auto *TL = selectFirst("t", Matches);
-  assert(TL);
-  D << FixItHint::CreateInsertion(
-  Lexer::getLocForEndOfToken(TL->getEndLoc(), 0, *Result.SourceManager,
- getLangOpts()),
-  Replacement);
+  if (const auto *TL = selectFirst("t", Matches))
+D << FixItHint::CreateInsertion(
+Lexer::getLocForEndOfToken(TL->getEndLoc(), 0, *Result.SourceManager,
+   getLangOpts()),
+Replacement);
 }
 
 } // namespace bugprone
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91898: [attributes] Add a facility for defining and enforcing a Trusted Computing Base.

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

LGTM!  But I'd like other folks to take a look at it first.


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

https://reviews.llvm.org/D91898

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

[PATCH] D91941: [clangd] Use WorkScheduler.run() in ClangdServer::resolveTypeHierarchy()

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91941

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

[PATCH] D91944: OpenMP 5.0 metadirective

[Alok Mishra via Phabricator via cfe-commits]

alokmishra.besu created this revision.
alokmishra.besu added a project: OpenMP.
Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, martong, 
arphaman, guansong, hiraditya, yaxunl.
Herald added projects: clang, LLVM.
alokmishra.besu requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

This patch supports OpenMP 5.0 metadirective features.
It is implemented keeping the OpenMP 5.1 features like dynamic user condition 
in mind.

A new function, getBestWhenMatchForContext, is defined in 
llvm/Frontend/OpenMP/OMPContext.h

Currently this function return the index of the when clause with the highest 
score from the ones applicable in the Context.
But this function is declared with an array which can be used in OpenMP 5.1 
implementation to select all the valid when clauses which can be resolved in 
runtime. Currently this array is set to null by default and its implementation 
is left for future.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91944

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/StmtOpenMP.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/StmtNodes.td
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/OpenMP/metadirective_ast_print.cpp
  clang/test/OpenMP/metadirective_codegen.cpp
  clang/test/OpenMP/metadirective_construct.cpp
  clang/test/OpenMP/metadirective_empty.cpp
  clang/test/OpenMP/metadirective_implementation.cpp
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/CXCursor.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td
  llvm/include/llvm/Frontend/OpenMP/OMPContext.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPContext.cpp

Index: llvm/lib/Frontend/OpenMP/OMPContext.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPContext.cpp
+++ llvm/lib/Frontend/OpenMP/OMPContext.cpp
@@ -332,6 +332,73 @@
   return Score;
 }
 
+int llvm::omp::getBestWhenMatchForContext(
+const SmallVectorImpl &VMIs, const OMPContext &Ctx,
+SmallVectorImpl *OrderedMatch) {
+
+  APInt BestScore(64, 0);
+  int BestVMIIdx = -1;
+  const VariantMatchInfo *BestVMI = nullptr;
+
+  for (unsigned u = 0, e = VMIs.size(); u < e; ++u) {
+const VariantMatchInfo &VMI = VMIs[u];
+
+SmallVector ConstructMatches;
+// Check if its clearly not the best.
+APInt Score = getVariantMatchScore(VMI, Ctx, ConstructMatches);
+if (Score.ult(BestScore))
+  continue;
+// Equal score need subset checks.
+if (Score.eq(BestScore)) {
+  // Strict subset are never best.
+  if (isStrictSubset(VMI, *BestVMI))
+continue;
+  // Same score and the current best is no strict subset so we keep it.
+  if (!isStrictSubset(*BestVMI, VMI))
+continue;
+}
+// New best found.
+BestVMI = &VMI;
+BestVMIIdx = u;
+BestScore = Score;
+  }
+
+  return BestVMIIdx;
+}
+
+/*int llvm::omp::getBestWhenMatchForContext(
+const SmallVectorImpl &VMIs, const OMPContext &Ctx) {
+
+  APInt BestScore(64, 0);
+  int BestVMIIdx = -1;
+  const VariantMatchInfo *BestVMI = nullptr;
+
+  for (unsigned u = 0, e = VMIs.size(); u < e; ++u) {
+const VariantMatchInfo &VMI = VMIs[u];
+
+SmallVector ConstructMatches;
+// Check if its clearly not the best.
+APInt Score = getVariantMatchScore(VMI, Ctx, ConstructMatches);
+if (Score.ult(BestScore))
+  continue;
+// Equal score need subset checks.
+if (Score.eq(BestScore)) {
+  // Strict subset are never best.
+  if (isStrictSubset(VMI, *BestVMI))
+continue;
+  // Same score and the current best is no strict subset so we keep it.
+  if (!isStrictSubset(*BestVMI, VMI))
+continue;
+}
+// New best found.
+BestVMI = &VMI;
+BestVMIIdx = u;
+BestScore = Score;
+  }
+
+  return BestVMIIdx;
+}*/
+
 int llvm::omp::getBestVariantMatchForContext(
 const SmallVectorImpl &VMIs, const OMPContext &Ctx) {
 
Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -115,6 +115,7 @@
 __OMP_CL

[clang-tools-extra] fee78fb - [clangd] Second attempt at fixing windows buildbots

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-11-23T10:06:48+01:00
New Revision: fee78fb0049ae2556c99768a06421d7cdbb9d016

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

LOG: [clangd] Second attempt at fixing windows buildbots

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp 
b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
index 2b4605eb97e2..a2423094b17a 100644
--- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -255,6 +255,7 @@ TEST_F(ConfigCompileTests, ExternalBlockMountPoint) {
   };
 
   auto BarPath = testPath("foo/bar.h", llvm::sys::path::Style::posix);
+  BarPath = llvm::sys::path::convert_to_slash(BarPath);
   Parm.Path = BarPath;
   // Non-absolute MountPoint without a directory raises an error.
   Frag = GetFrag("", "foo");
@@ -268,6 +269,7 @@ TEST_F(ConfigCompileTests, ExternalBlockMountPoint) {
   ASSERT_FALSE(Conf.Index.External);
 
   auto FooPath = testPath("foo/", llvm::sys::path::Style::posix);
+  FooPath = llvm::sys::path::convert_to_slash(FooPath);
   // Ok when relative.
   Frag = GetFrag(testRoot(), "foo/");
   compileAndApply();
@@ -291,6 +293,7 @@ TEST_F(ConfigCompileTests, ExternalBlockMountPoint) {
 
   // File outside MountPoint, no index.
   auto BazPath = testPath("bar/baz.h", llvm::sys::path::Style::posix);
+  BazPath = llvm::sys::path::convert_to_slash(BazPath);
   Parm.Path = BazPath;
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
@@ -299,6 +302,7 @@ TEST_F(ConfigCompileTests, ExternalBlockMountPoint) {
 
   // File under MountPoint, index should be set.
   BazPath = testPath("foo/baz.h", llvm::sys::path::Style::posix);
+  BazPath = llvm::sys::path::convert_to_slash(BazPath);
   Parm.Path = BazPath;
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();



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

[PATCH] D91947: [clangd] testPath's final result agrees with the passed in Style

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

This was confusing, as testRoot on windows results in C:\\clangd-test
and testPath generated with posix explicitly still contained backslashes.

This patch ensures not only the relative part, but the whole final result
respects passed in Style.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91947

Files:
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/TestFS.cpp


Index: clang-tools-extra/clangd/unittests/TestFS.cpp
===
--- clang-tools-extra/clangd/unittests/TestFS.cpp
+++ clang-tools-extra/clangd/unittests/TestFS.cpp
@@ -80,12 +80,10 @@
 }
 
 std::string testPath(PathRef File, llvm::sys::path::Style Style) {
-  assert(llvm::sys::path::is_relative(File) && "FileName should be relative");
-
-  llvm::SmallString<32> NativeFile = File;
-  llvm::sys::path::native(NativeFile, Style);
+  assert(llvm::sys::path::is_relative(File, Style));
   llvm::SmallString<32> Path;
-  llvm::sys::path::append(Path, Style, testRoot(), NativeFile);
+  llvm::sys::path::append(Path, testRoot(), File);
+  llvm::sys::path::native(Path, Style);
   return std::string(Path.str());
 }
 
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -255,7 +255,6 @@
   };
 
   auto BarPath = testPath("foo/bar.h", llvm::sys::path::Style::posix);
-  BarPath = llvm::sys::path::convert_to_slash(BarPath);
   Parm.Path = BarPath;
   // Non-absolute MountPoint without a directory raises an error.
   Frag = GetFrag("", "foo");
@@ -269,7 +268,6 @@
   ASSERT_FALSE(Conf.Index.External);
 
   auto FooPath = testPath("foo/", llvm::sys::path::Style::posix);
-  FooPath = llvm::sys::path::convert_to_slash(FooPath);
   // Ok when relative.
   Frag = GetFrag(testRoot(), "foo/");
   compileAndApply();
@@ -293,7 +291,6 @@
 
   // File outside MountPoint, no index.
   auto BazPath = testPath("bar/baz.h", llvm::sys::path::Style::posix);
-  BazPath = llvm::sys::path::convert_to_slash(BazPath);
   Parm.Path = BazPath;
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
@@ -302,7 +299,6 @@
 
   // File under MountPoint, index should be set.
   BazPath = testPath("foo/baz.h", llvm::sys::path::Style::posix);
-  BazPath = llvm::sys::path::convert_to_slash(BazPath);
   Parm.Path = BazPath;
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();


Index: clang-tools-extra/clangd/unittests/TestFS.cpp
===
--- clang-tools-extra/clangd/unittests/TestFS.cpp
+++ clang-tools-extra/clangd/unittests/TestFS.cpp
@@ -80,12 +80,10 @@
 }
 
 std::string testPath(PathRef File, llvm::sys::path::Style Style) {
-  assert(llvm::sys::path::is_relative(File) && "FileName should be relative");
-
-  llvm::SmallString<32> NativeFile = File;
-  llvm::sys::path::native(NativeFile, Style);
+  assert(llvm::sys::path::is_relative(File, Style));
   llvm::SmallString<32> Path;
-  llvm::sys::path::append(Path, Style, testRoot(), NativeFile);
+  llvm::sys::path::append(Path, testRoot(), File);
+  llvm::sys::path::native(Path, Style);
   return std::string(Path.str());
 }
 
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -255,7 +255,6 @@
   };
 
   auto BarPath = testPath("foo/bar.h", llvm::sys::path::Style::posix);
-  BarPath = llvm::sys::path::convert_to_slash(BarPath);
   Parm.Path = BarPath;
   // Non-absolute MountPoint without a directory raises an error.
   Frag = GetFrag("", "foo");
@@ -269,7 +268,6 @@
   ASSERT_FALSE(Conf.Index.External);
 
   auto FooPath = testPath("foo/", llvm::sys::path::Style::posix);
-  FooPath = llvm::sys::path::convert_to_slash(FooPath);
   // Ok when relative.
   Frag = GetFrag(testRoot(), "foo/");
   compileAndApply();
@@ -293,7 +291,6 @@
 
   // File outside MountPoint, no index.
   auto BazPath = testPath("bar/baz.h", llvm::sys::path::Style::posix);
-  BazPath = llvm::sys::path::convert_to_slash(BazPath);
   Parm.Path = BazPath;
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
@@ -302,7 +299,6 @@
 
   // File under MountPoint, index should be set.
   BazPath = testPath("foo/baz.h", llvm::sys::path::Style::posix);
-  BazPath = llvm::sys::path::convert_to_slash(BazPath);
   Parm.Path = BazPath;
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
__

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 306987.
kbobyrev marked 4 inline comments as done.
kbobyrev added a comment.

Resolve comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71880

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

Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -18,7 +18,6 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/SourceLocation.h"
-#include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/STLExtras.h"
@@ -76,6 +75,8 @@
   return OtherFile;
 }
 
+const NamedDecl *canonicalRenameDecl(const NamedDecl *D);
+
 llvm::DenseSet locateDeclAt(ParsedAST &AST,
SourceLocation TokenStartLoc) {
   unsigned Offset =
@@ -91,9 +92,7 @@
   for (const NamedDecl *D :
targetDecl(SelectedNode->ASTNode,
   DeclRelation::Alias | DeclRelation::TemplatePattern)) {
-// Get to CXXRecordDecl from constructor or destructor.
-D = tooling::getCanonicalSymbolDeclaration(D);
-Result.insert(D);
+Result.insert(canonicalRenameDecl(D));
   }
   return Result;
 }
@@ -222,23 +221,64 @@
   return error("Cannot rename symbol: {0}", Message(Reason));
 }
 
+// Canonical declarations help simplify the process of renaming. Examples:
+// - Template's canonical decl is the templated declaration (i.e.
+//   ClassTemplateDecl is canonicalized to its child CXXRecordDecl,
+//   FunctionTemplateDecl - to child FunctionDecl)
+// - Given a constructor/destructor, canonical declaration is the parent
+//   CXXRecordDecl because we want to rename both type name and its ctor/dtor.
+// - All specializations are canonicalized to the primary template. For example:
+//
+//template 
+//bool Foo = true; (1)
+//
+//template 
+//bool Foo = true; (2)
+//
+//template <>
+//bool Foo = true; (3)
+//
+// Here, both partial (2) and full (3) specializations are canonicalized to (1)
+// which ensures all three of them are renamed.
+const NamedDecl *canonicalRenameDecl(const NamedDecl *D) {
+  if (const auto *VarTemplate = dyn_cast(D))
+return canonicalRenameDecl(
+VarTemplate->getSpecializedTemplate()->getTemplatedDecl());
+  if (const auto *Template = dyn_cast(D))
+if (const NamedDecl *TemplatedDecl = Template->getTemplatedDecl())
+  return canonicalRenameDecl(Template->getTemplatedDecl());
+  if (const auto *ClassTemplateSpecialization =
+  dyn_cast(D))
+return canonicalRenameDecl(
+ClassTemplateSpecialization->getSpecializedTemplate()
+->getTemplatedDecl());
+  if (const auto *Method = dyn_cast(D)) {
+if (Method->getDeclKind() == Decl::Kind::CXXConstructor ||
+Method->getDeclKind() == Decl::Kind::CXXDestructor)
+  return canonicalRenameDecl(Method->getParent());
+if (const FunctionDecl *InstantiatedMethod =
+Method->getInstantiatedFromMemberFunction())
+  Method = cast(InstantiatedMethod);
+// FIXME(kirillbobyrev): For virtual methods with
+// size_overridden_methods() > 1, this will not rename all functions it
+// overrides, because this code assumes there is a single canonical
+// declaration.
+while (Method->isVirtual() && Method->size_overridden_methods())
+  Method = *Method->overridden_methods().begin();
+return dyn_cast(Method->getCanonicalDecl());
+  }
+  if (const auto *Function = dyn_cast(D))
+if (const FunctionTemplateDecl *Template = Function->getPrimaryTemplate())
+  return canonicalRenameDecl(Template);
+  return dyn_cast(D->getCanonicalDecl());
+}
+
 // Return all rename occurrences in the main file.
 std::vector findOccurrencesWithinFile(ParsedAST &AST,
   const NamedDecl &ND) {
   trace::Span Tracer("FindOccurrencesWithinFile");
-  // If the cursor is at the underlying CXXRecordDecl of the
-  // ClassTemplateDecl, ND will be the CXXRecordDecl. In this case, we need to
-  // get the primary template manually.
-  // getUSRsForDeclaration will find other related symbols, e.g. virtual and its
-  // overriddens, primary template and all explicit specializations.
-  // FIXME: Get rid of the remaining tooling APIs.
-  const auto *RenameDecl =
-  ND.getDescribedTemplate() ? ND.getDescribedTemplate() : &ND;
-  std::vector RenameUSRs =
-  tooling::getUSRsForDeclaration(RenameDecl, AST.getASTContext());
-  llvm::DenseSet TargetIDs;
-  for (auto &USR : RenameUSRs)
-TargetIDs.insert(SymbolID(USR));
+  assert(canonicalRenameDecl(&ND) == &ND &&
+ "ND should be already canonicalized.");
 
   std::vector Results;
   for (Decl *TopLevelDecl : AST.

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Make use of TemplatedDecl.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71880

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

Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -18,7 +18,6 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/SourceLocation.h"
-#include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/STLExtras.h"
@@ -76,6 +75,8 @@
   return OtherFile;
 }
 
+const NamedDecl *canonicalRenameDecl(const NamedDecl *D);
+
 llvm::DenseSet locateDeclAt(ParsedAST &AST,
SourceLocation TokenStartLoc) {
   unsigned Offset =
@@ -91,9 +92,7 @@
   for (const NamedDecl *D :
targetDecl(SelectedNode->ASTNode,
   DeclRelation::Alias | DeclRelation::TemplatePattern)) {
-// Get to CXXRecordDecl from constructor or destructor.
-D = tooling::getCanonicalSymbolDeclaration(D);
-Result.insert(D);
+Result.insert(canonicalRenameDecl(D));
   }
   return Result;
 }
@@ -222,23 +221,64 @@
   return error("Cannot rename symbol: {0}", Message(Reason));
 }
 
+// Canonical declarations help simplify the process of renaming. Examples:
+// - Template's canonical decl is the templated declaration (i.e.
+//   ClassTemplateDecl is canonicalized to its child CXXRecordDecl,
+//   FunctionTemplateDecl - to child FunctionDecl)
+// - Given a constructor/destructor, canonical declaration is the parent
+//   CXXRecordDecl because we want to rename both type name and its ctor/dtor.
+// - All specializations are canonicalized to the primary template. For example:
+//
+//template 
+//bool Foo = true; (1)
+//
+//template 
+//bool Foo = true; (2)
+//
+//template <>
+//bool Foo = true; (3)
+//
+// Here, both partial (2) and full (3) specializations are canonicalized to (1)
+// which ensures all three of them are renamed.
+const NamedDecl *canonicalRenameDecl(const NamedDecl *D) {
+  if (const auto *VarTemplate = dyn_cast(D))
+return canonicalRenameDecl(
+VarTemplate->getSpecializedTemplate()->getTemplatedDecl());
+  if (const auto *Template = dyn_cast(D))
+if (const NamedDecl *TemplatedDecl = Template->getTemplatedDecl())
+  return canonicalRenameDecl(TemplatedDecl);
+  if (const auto *ClassTemplateSpecialization =
+  dyn_cast(D))
+return canonicalRenameDecl(
+ClassTemplateSpecialization->getSpecializedTemplate()
+->getTemplatedDecl());
+  if (const auto *Method = dyn_cast(D)) {
+if (Method->getDeclKind() == Decl::Kind::CXXConstructor ||
+Method->getDeclKind() == Decl::Kind::CXXDestructor)
+  return canonicalRenameDecl(Method->getParent());
+if (const FunctionDecl *InstantiatedMethod =
+Method->getInstantiatedFromMemberFunction())
+  Method = cast(InstantiatedMethod);
+// FIXME(kirillbobyrev): For virtual methods with
+// size_overridden_methods() > 1, this will not rename all functions it
+// overrides, because this code assumes there is a single canonical
+// declaration.
+while (Method->isVirtual() && Method->size_overridden_methods())
+  Method = *Method->overridden_methods().begin();
+return dyn_cast(Method->getCanonicalDecl());
+  }
+  if (const auto *Function = dyn_cast(D))
+if (const FunctionTemplateDecl *Template = Function->getPrimaryTemplate())
+  return canonicalRenameDecl(Template);
+  return dyn_cast(D->getCanonicalDecl());
+}
+
 // Return all rename occurrences in the main file.
 std::vector findOccurrencesWithinFile(ParsedAST &AST,
   const NamedDecl &ND) {
   trace::Span Tracer("FindOccurrencesWithinFile");
-  // If the cursor is at the underlying CXXRecordDecl of the
-  // ClassTemplateDecl, ND will be the CXXRecordDecl. In this case, we need to
-  // get the primary template manually.
-  // getUSRsForDeclaration will find other related symbols, e.g. virtual and its
-  // overriddens, primary template and all explicit specializations.
-  // FIXME: Get rid of the remaining tooling APIs.
-  const auto *RenameDecl =
-  ND.getDescribedTemplate() ? ND.getDescribedTemplate() : &ND;
-  std::vector RenameUSRs =
-  tooling::getUSRsForDeclaration(RenameDecl, AST.getASTContext());
-  llvm::DenseSet TargetIDs;
-  for (auto &USR : RenameUSRs)
-TargetIDs.insert(SymbolID(USR));
+  assert(canonicalRenameDecl(&ND) == &ND &&
+ "ND should be already canonicalized.");
 
   std::vector Results;
   for (Decl *TopLevelDecl : AST.getLocalTopLevelDecls()) {
@@ -246,11 +286,11 @@

[PATCH] D91948: [analyzer][doc] Add Container- and IteratorModeling developer docs

[Endre Fülöp via Phabricator via cfe-commits]

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

Container and IteratorModeling is a feature in ClangSA which handles the 
containers and their 
iterators. Containers include arrays and STL containers, and their iterators 
implemented as either 
pointers or class instances.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91948

Files:
  clang/docs/analyzer/developer-docs.rst
  clang/docs/analyzer/developer-docs/ContainerModeling.rst

Index: clang/docs/analyzer/developer-docs/ContainerModeling.rst
===
--- /dev/null
+++ clang/docs/analyzer/developer-docs/ContainerModeling.rst
@@ -0,0 +1,386 @@
+===
+Container and Iterator Modeling
+===
+
+The goal of checker ``alpha.cplusplus.ContainerModeling`` is to provide a
+symbolic abstraction for containers to the Clang Static Analyzer. There are
+various concepts regarding containers that help formulate static analysis
+problems more concisely. The size of the container, whether is empty or not are
+the most trivial motivating examples. Standard containers can be iterated, and
+this idiom is well-adopted in case of non-standard container implementations as
+well, because it can be used to provide a compatible interface to algorithms.
+Therefore iterator modeling is closely related to containers. Iterators extend
+the range of useful properties when it comes to finding bugs, for example, which
+container an iterator belongs to, what position inside the container it is in,
+and whether it is a valid or invalid state (see rules for `iterator invalidation
+`_).
+Iterator modeling is implemented in checker ``alpha.cplusplus.IteratorModeling``.
+
+There are also various checkers which make use if the information provided by
+the modeling checkers mentioned:
+  * :ref:`alpha-cplusplus-InvalidatedIterator`
+  * :ref:`alpha-cplusplus-IteratorRange`
+  * :ref:`alpha-cplusplus-MismatchedIterator`
+
+
+Definition of a container
+-
+
+According to ContainerModeling, a value ``c`` with type ``C`` is considered a
+container if either of the following holds:
+  * The expression ``c.begin()`` and ``c.end()`` are both valid expressions, and
+return an `iterator
+`_.
+This should be detected by type C having member functions ``T C::begin()``
+and ``T C::end()``, where T is an `iterator
+`_.
+  * The expression ``begin(c)`` and ``end(c)`` are both valid expressions in a
+given scope, and return an `iterator
+`_.
+This should be detected by checking the existence functions with the
+corresponding names, and can either be user-defined free functions or
+template specialization of the standard-defined ``template
+constexpr auto std::begin(T& t) -> decltype(t.begin())`` and
+``template constexpr auto std::end(T& t) ->
+decltype(t.end())`` function templates (see `std::begin
+`_ and `std::end
+`_).
+
+Example containers in STL (with different `invalidation properties
+`_)
+ - std::array
+ - std::vector
+ - std::deque
+ - std::list
+ - std::forward_list
+
+Example of a custom container with member functions.
+
+.. code-block:: cpp
+
+  class C {
+std::vector v;
+  public:
+using iterator = typename std::vector::iterator;
+
+iterator begin() { return v.begin(); }
+iterator end() { return v.end(); }
+  };
+
+
+Example of a custom container with free functions.
+
+.. code-block:: cpp
+
+  class C {
+std::vector v;
+  public:
+using iterator = typename std::vector::iterator;
+  
+friend iterator begin(C&);
+friend iterator end(C&);
+  };
+
+  C::iterator begin(C& c) { return c.v.begin(); }
+  C::iterator end(C& c) { return c.v.end(); }
+
+Example of a custom container with std template specialization.
+
+.. code-block:: cpp
+
+  class C {
+std::vector v;
+auto begin() { return v.begin(); }
+auto end() { return v.end(); }
+  public:
+template
+friend constexpr auto std::begin(T& t) -> decltype(t.begin());
+template
+friend constexpr auto std::end(T& t) -> decltype(t.end());
+  };
+
+Modeling of a container
+-
+
+A container is modeled if it has an associated ``MemRegion``, and this ``MemRegion``,
+or rather the ``const MemReg

[PATCH] D91122: [clangd] Call hierarchy (XRefs layer, incoming calls)

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

This looks great! Thanks a lot for bearing with me and doing all of this!




Comment at: clang-tools-extra/clangd/XRefs.cpp:1613
+if (auto CHI = declToCallHierarchyItem(*Decl))
+  Result.emplace_back(*std::move(CHI));
+  }

please put the deref inside `std::move`. as move semantics of `Optional` are 
optional (no pun intended :P), i.e. depends on `LLVM_HAS_RVALUE_REFERENCE_THIS`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91122

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

[PATCH] D89046: [AST] Build recovery expression by default for all language.

[Haojian Wu via Phabricator via cfe-commits]

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

rebase and update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89046

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/LangOptions.def
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector-error.c
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector2-error.c
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector3-error.c
  clang/test/CodeGen/builtins-ppc-error.c
  clang/test/Index/complete-switch.c
  clang/test/OpenMP/begin_declare_variant_messages.c
  clang/test/OpenMP/declare_variant_messages.c
  clang/test/Parser/objc-foreach-syntax.m
  clang/test/Sema/__try.c
  clang/test/Sema/enum.c
  clang/test/Sema/typo-correction.c

Index: clang/test/Sema/typo-correction.c
===
--- clang/test/Sema/typo-correction.c
+++ clang/test/Sema/typo-correction.c
@@ -14,9 +14,9 @@
   // expected-error {{use of undeclared identifier 'b'}}
 
 int foobar;  // expected-note {{'foobar' declared here}}
-a = goobar ?: 4;  // expected-warning {{type specifier missing, defaults to 'int'}} \
-  // expected-error {{use of undeclared identifier 'goobar'; did you mean 'foobar'?}} \
-  // expected-error {{initializer element is not a compile-time constant}}
+new_a = goobar ?: 4; // expected-warning {{type specifier missing, defaults to 'int'}} \
+  // expected-error {{use of undeclared identifier 'goobar'; did you mean 'foobar'?}} \
+  // expected-error {{initializer element is not a compile-time constant}}
 
 struct ContainerStuct {
   enum { SOME_ENUM }; // expected-note {{'SOME_ENUM' declared here}}
@@ -50,10 +50,10 @@
   cabs(errij);  // expected-error {{use of undeclared identifier 'errij'}}
 }
 
-extern long afunction(int); // expected-note {{'afunction' declared here}}
+extern long afunction(int);
 void fn2() {
-  f(THIS_IS_AN_ERROR, // expected-error {{use of undeclared identifier 'THIS_IS_AN_ERROR'}}
-afunction(afunction_));  // expected-error {{use of undeclared identifier 'afunction_'; did you mean 'afunction'?}}
+  f(THIS_IS_AN_ERROR,   // expected-error {{use of undeclared identifier 'THIS_IS_AN_ERROR'}}
+afunction(afunction_)); // expected-error {{use of undeclared identifier 'afunction_'}}
 }
 
 int d = X ? d : L; // expected-error 2 {{use of undeclared identifier}}
Index: clang/test/Sema/enum.c
===
--- clang/test/Sema/enum.c
+++ clang/test/Sema/enum.c
@@ -100,7 +100,8 @@
 // PR7911
 extern enum PR7911T PR7911V; // expected-warning{{ISO C forbids forward references to 'enum' types}}
 void PR7911F() {
-  switch (PR7911V); // expected-error {{statement requires expression of integer type}}
+  switch (PR7911V) // expected-error {{statement requires expression of integer type}}
+;
 }
 
 char test5[__has_feature(enumerator_attributes) ? 1 : -1];
Index: clang/test/Sema/__try.c
===
--- clang/test/Sema/__try.c
+++ clang/test/Sema/__try.c
@@ -50,9 +50,9 @@
 }  // expected-error{{expected '__except' or '__finally' block}}
 
 void TEST() {
-  __except ( FilterExpression() ) { // expected-warning{{implicit declaration of function '__except' is invalid in C99}} \
-// expected-error{{too few arguments to function call, expected 1, have 0}}
-
+  __except (FilterExpression()) { // expected-warning{{implicit declaration of function '__except' is invalid in C99}} \
+// expected-error{{too few arguments to function call, expected 1, have 0}} \
+// expected-error{{expected ';' after expression}}
   }
 }
 
Index: clang/test/Parser/objc-foreach-syntax.m
===
--- clang/test/Parser/objc-foreach-syntax.m
+++ clang/test/Parser/objc-foreach-syntax.m
@@ -21,5 +21,8 @@
 
 
 static int test7(id keys) {
-  for (id key; in keys) ;  // expected-error {{use of undeclared identifier 'in'}}
+  // FIXME: would be nice to suppress the secondary diagnostics.
+  for (id key; in keys) ;  // expected-error {{use of undeclared identifier 'in'}} \
+   // expected-error {{expected ';' in 'for' statement specifier}} \
+   // expected-warning {{expression result unused}}
 }
Index: clang/test/OpenMP/declare_variant_messages.c
===
--- clang/test/OpenMP/declare_variant_messages.c
+++ clang/test/OpenMP/declare_variant_messages.c
@@ -10,7 +10,7 @@
 #pragma omp declare variant // expected-error {{expected '(' after 'declare variant'}}
 #pragma omp declare variant( // expected-error {{expected expression}} expected-error {{expected ')'}} expected-note {{to match this '('}}
 #pragma omp declar

[clang] b1444ed - [AST] Build recovery expression by default for all language.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-11-23T11:08:28+01:00
New Revision: b1444edbf41c1fe9f7e676df6e873e9c9318283e

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

LOG: [AST] Build recovery expression by default for all language.

The dependency mechanism for C has been implemented, and we have rolled out
this to all internal users, didn't see crashy issues, we consider it is stable
enough.

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

Added: 


Modified: 
clang/include/clang/AST/Expr.h
clang/include/clang/Basic/LangOptions.def
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/SystemZ/builtins-systemz-zvector-error.c
clang/test/CodeGen/SystemZ/builtins-systemz-zvector2-error.c
clang/test/CodeGen/SystemZ/builtins-systemz-zvector3-error.c
clang/test/CodeGen/builtins-ppc-error.c
clang/test/Index/complete-switch.c
clang/test/OpenMP/begin_declare_variant_messages.c
clang/test/OpenMP/declare_variant_messages.c
clang/test/Parser/objc-foreach-syntax.m
clang/test/Sema/__try.c
clang/test/Sema/enum.c
clang/test/Sema/typo-correction.c

Removed: 




diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 11b05ce7a5f5e..3ea2817f1926c 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -6330,9 +6330,6 @@ class TypoExpr : public Expr {
 ///
 /// One can also reliably suppress all bogus errors on expressions containing
 /// recovery expressions by examining results of Expr::containsErrors().
-///
-/// FIXME: RecoveryExpr is currently generated by default in C++ mode only, as
-/// dependence isn't handled properly on several C-only codepaths.
 class RecoveryExpr final : public Expr,
private llvm::TrailingObjects 
{
 public:

diff  --git a/clang/include/clang/Basic/LangOptions.def 
b/clang/include/clang/Basic/LangOptions.def
index 9c573b43049c7..e4113789f07ce 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -148,8 +148,8 @@ LANGOPT(RelaxedTemplateTemplateArgs, 1, 0, "C++17 relaxed 
matching of template t
 
 LANGOPT(DoubleSquareBracketAttributes, 1, 0, "'[[]]' attributes extension for 
all language standard modes")
 
-COMPATIBLE_LANGOPT(RecoveryAST, 1, 0, "Preserve expressions in AST when 
encountering errors")
-COMPATIBLE_LANGOPT(RecoveryASTType, 1, 0, "Preserve the type in recovery 
expressions")
+COMPATIBLE_LANGOPT(RecoveryAST, 1, 1, "Preserve expressions in AST when 
encountering errors")
+COMPATIBLE_LANGOPT(RecoveryASTType, 1, 1, "Preserve the type in recovery 
expressions")
 
 BENIGN_LANGOPT(ThreadsafeStatics , 1, 1, "thread-safe static initializers")
 LANGOPT(POSIXThreads  , 1, 0, "POSIX thread support")

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index f6753b5b91a03..35025a9829ef8 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3020,11 +3020,9 @@ static void ParseLangArgs(LangOptions &Opts, ArgList 
&Args, InputKind IK,
   !Args.hasArg(OPT_fno_concept_satisfaction_caching);
   if (Args.hasArg(OPT_fconcepts_ts))
 Diags.Report(diag::warn_fe_concepts_ts_flag);
-  // Recovery AST still heavily relies on dependent-type machinery.
-  Opts.RecoveryAST =
-  Args.hasFlag(OPT_frecovery_ast, OPT_fno_recovery_ast, Opts.CPlusPlus);
-  Opts.RecoveryASTType = Args.hasFlag(
-  OPT_frecovery_ast_type, OPT_fno_recovery_ast_type, Opts.CPlusPlus);
+  Opts.RecoveryAST = Args.hasFlag(OPT_frecovery_ast, OPT_fno_recovery_ast);
+  Opts.RecoveryASTType =
+  Args.hasFlag(OPT_frecovery_ast_type, OPT_fno_recovery_ast_type);
   Opts.HeinousExtensions = Args.hasArg(OPT_fheinous_gnu_extensions);
   Opts.AccessControl = !Args.hasArg(OPT_fno_access_control);
   Opts.ElideConstructors = !Args.hasArg(OPT_fno_elide_constructors);

diff  --git a/clang/test/CodeGen/SystemZ/builtins-systemz-zvector-error.c 
b/clang/test/CodeGen/SystemZ/builtins-systemz-zvector-error.c
index 5fdcb4061850f..77e90b5ad4b84 100644
--- a/clang/test/CodeGen/SystemZ/builtins-systemz-zvector-error.c
+++ b/clang/test/CodeGen/SystemZ/builtins-systemz-zvector-error.c
@@ -67,7 +67,7 @@ void test_core(void) {
   len = __lcbb(cptr, 8192);  // expected-error {{no matching function}}
  // expected-note@vecintrin.h:* {{must be a 
constant power of 2 from 64 to 4096}}
 
-  vsl = vec_permi(vsl, vsl, idx); // expected-error {{no matching function}}
+  vsl = vec_permi(vsl, vsl, idx); // expected-error {{no matching function}} 
expected-error {{argument to '__builtin_s390_vpdi' must be a constant integer}}
   // expected-note@vecintrin.h:* 3 {{candidate 
function

[PATCH] D89046: [AST] Build recovery expression by default for all language.

[Haojian Wu via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb1444edbf41c: [AST] Build recovery expression by default for 
all language. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89046

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/LangOptions.def
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector-error.c
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector2-error.c
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector3-error.c
  clang/test/CodeGen/builtins-ppc-error.c
  clang/test/Index/complete-switch.c
  clang/test/OpenMP/begin_declare_variant_messages.c
  clang/test/OpenMP/declare_variant_messages.c
  clang/test/Parser/objc-foreach-syntax.m
  clang/test/Sema/__try.c
  clang/test/Sema/enum.c
  clang/test/Sema/typo-correction.c

Index: clang/test/Sema/typo-correction.c
===
--- clang/test/Sema/typo-correction.c
+++ clang/test/Sema/typo-correction.c
@@ -14,9 +14,9 @@
   // expected-error {{use of undeclared identifier 'b'}}
 
 int foobar;  // expected-note {{'foobar' declared here}}
-a = goobar ?: 4;  // expected-warning {{type specifier missing, defaults to 'int'}} \
-  // expected-error {{use of undeclared identifier 'goobar'; did you mean 'foobar'?}} \
-  // expected-error {{initializer element is not a compile-time constant}}
+new_a = goobar ?: 4; // expected-warning {{type specifier missing, defaults to 'int'}} \
+  // expected-error {{use of undeclared identifier 'goobar'; did you mean 'foobar'?}} \
+  // expected-error {{initializer element is not a compile-time constant}}
 
 struct ContainerStuct {
   enum { SOME_ENUM }; // expected-note {{'SOME_ENUM' declared here}}
@@ -50,10 +50,10 @@
   cabs(errij);  // expected-error {{use of undeclared identifier 'errij'}}
 }
 
-extern long afunction(int); // expected-note {{'afunction' declared here}}
+extern long afunction(int);
 void fn2() {
-  f(THIS_IS_AN_ERROR, // expected-error {{use of undeclared identifier 'THIS_IS_AN_ERROR'}}
-afunction(afunction_));  // expected-error {{use of undeclared identifier 'afunction_'; did you mean 'afunction'?}}
+  f(THIS_IS_AN_ERROR,   // expected-error {{use of undeclared identifier 'THIS_IS_AN_ERROR'}}
+afunction(afunction_)); // expected-error {{use of undeclared identifier 'afunction_'}}
 }
 
 int d = X ? d : L; // expected-error 2 {{use of undeclared identifier}}
Index: clang/test/Sema/enum.c
===
--- clang/test/Sema/enum.c
+++ clang/test/Sema/enum.c
@@ -100,7 +100,8 @@
 // PR7911
 extern enum PR7911T PR7911V; // expected-warning{{ISO C forbids forward references to 'enum' types}}
 void PR7911F() {
-  switch (PR7911V); // expected-error {{statement requires expression of integer type}}
+  switch (PR7911V) // expected-error {{statement requires expression of integer type}}
+;
 }
 
 char test5[__has_feature(enumerator_attributes) ? 1 : -1];
Index: clang/test/Sema/__try.c
===
--- clang/test/Sema/__try.c
+++ clang/test/Sema/__try.c
@@ -50,9 +50,9 @@
 }  // expected-error{{expected '__except' or '__finally' block}}
 
 void TEST() {
-  __except ( FilterExpression() ) { // expected-warning{{implicit declaration of function '__except' is invalid in C99}} \
-// expected-error{{too few arguments to function call, expected 1, have 0}}
-
+  __except (FilterExpression()) { // expected-warning{{implicit declaration of function '__except' is invalid in C99}} \
+// expected-error{{too few arguments to function call, expected 1, have 0}} \
+// expected-error{{expected ';' after expression}}
   }
 }
 
Index: clang/test/Parser/objc-foreach-syntax.m
===
--- clang/test/Parser/objc-foreach-syntax.m
+++ clang/test/Parser/objc-foreach-syntax.m
@@ -21,5 +21,8 @@
 
 
 static int test7(id keys) {
-  for (id key; in keys) ;  // expected-error {{use of undeclared identifier 'in'}}
+  // FIXME: would be nice to suppress the secondary diagnostics.
+  for (id key; in keys) ;  // expected-error {{use of undeclared identifier 'in'}} \
+   // expected-error {{expected ';' in 'for' statement specifier}} \
+   // expected-warning {{expression result unused}}
 }
Index: clang/test/OpenMP/declare_variant_messages.c
===
--- clang/test/OpenMP/declare_variant_messages.c
+++ clang/test/OpenMP/declare_variant_messages.c
@@ -10,7 +10,7 @@
 #pragma omp declare variant // expected-error {{expected '(' after 'declare variant'}}
 

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

[Haojian Wu via Phabricator via cfe-commits]

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

Thanks, this looks great now.

I think we can get rid of the `clangToolingRefactoring` dependence from clangd 
CMake files, clangd now should have no use of any functions there, could you 
take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71880

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

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:78
 
+const NamedDecl *canonicalRenameDecl(const NamedDecl *D);
+

nit: maybe we can just move the function definition to here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71880

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

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 307013.
kbobyrev marked an inline comment as done.
kbobyrev added a comment.

Get rid of forward declaration.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71880

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

Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -18,7 +18,6 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/SourceLocation.h"
-#include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/STLExtras.h"
@@ -76,6 +75,58 @@
   return OtherFile;
 }
 
+// Canonical declarations help simplify the process of renaming. Examples:
+// - Template's canonical decl is the templated declaration (i.e.
+//   ClassTemplateDecl is canonicalized to its child CXXRecordDecl,
+//   FunctionTemplateDecl - to child FunctionDecl)
+// - Given a constructor/destructor, canonical declaration is the parent
+//   CXXRecordDecl because we want to rename both type name and its ctor/dtor.
+// - All specializations are canonicalized to the primary template. For example:
+//
+//template 
+//bool Foo = true; (1)
+//
+//template 
+//bool Foo = true; (2)
+//
+//template <>
+//bool Foo = true; (3)
+//
+// Here, both partial (2) and full (3) specializations are canonicalized to (1)
+// which ensures all three of them are renamed.
+const NamedDecl *canonicalRenameDecl(const NamedDecl *D) {
+  if (const auto *VarTemplate = dyn_cast(D))
+return canonicalRenameDecl(
+VarTemplate->getSpecializedTemplate()->getTemplatedDecl());
+  if (const auto *Template = dyn_cast(D))
+if (const NamedDecl *TemplatedDecl = Template->getTemplatedDecl())
+  return canonicalRenameDecl(TemplatedDecl);
+  if (const auto *ClassTemplateSpecialization =
+  dyn_cast(D))
+return canonicalRenameDecl(
+ClassTemplateSpecialization->getSpecializedTemplate()
+->getTemplatedDecl());
+  if (const auto *Method = dyn_cast(D)) {
+if (Method->getDeclKind() == Decl::Kind::CXXConstructor ||
+Method->getDeclKind() == Decl::Kind::CXXDestructor)
+  return canonicalRenameDecl(Method->getParent());
+if (const FunctionDecl *InstantiatedMethod =
+Method->getInstantiatedFromMemberFunction())
+  Method = cast(InstantiatedMethod);
+// FIXME(kirillbobyrev): For virtual methods with
+// size_overridden_methods() > 1, this will not rename all functions it
+// overrides, because this code assumes there is a single canonical
+// declaration.
+while (Method->isVirtual() && Method->size_overridden_methods())
+  Method = *Method->overridden_methods().begin();
+return dyn_cast(Method->getCanonicalDecl());
+  }
+  if (const auto *Function = dyn_cast(D))
+if (const FunctionTemplateDecl *Template = Function->getPrimaryTemplate())
+  return canonicalRenameDecl(Template);
+  return dyn_cast(D->getCanonicalDecl());
+}
+
 llvm::DenseSet locateDeclAt(ParsedAST &AST,
SourceLocation TokenStartLoc) {
   unsigned Offset =
@@ -91,9 +142,7 @@
   for (const NamedDecl *D :
targetDecl(SelectedNode->ASTNode,
   DeclRelation::Alias | DeclRelation::TemplatePattern)) {
-// Get to CXXRecordDecl from constructor or destructor.
-D = tooling::getCanonicalSymbolDeclaration(D);
-Result.insert(D);
+Result.insert(canonicalRenameDecl(D));
   }
   return Result;
 }
@@ -226,19 +275,8 @@
 std::vector findOccurrencesWithinFile(ParsedAST &AST,
   const NamedDecl &ND) {
   trace::Span Tracer("FindOccurrencesWithinFile");
-  // If the cursor is at the underlying CXXRecordDecl of the
-  // ClassTemplateDecl, ND will be the CXXRecordDecl. In this case, we need to
-  // get the primary template manually.
-  // getUSRsForDeclaration will find other related symbols, e.g. virtual and its
-  // overriddens, primary template and all explicit specializations.
-  // FIXME: Get rid of the remaining tooling APIs.
-  const auto *RenameDecl =
-  ND.getDescribedTemplate() ? ND.getDescribedTemplate() : &ND;
-  std::vector RenameUSRs =
-  tooling::getUSRsForDeclaration(RenameDecl, AST.getASTContext());
-  llvm::DenseSet TargetIDs;
-  for (auto &USR : RenameUSRs)
-TargetIDs.insert(SymbolID(USR));
+  assert(canonicalRenameDecl(&ND) == &ND &&
+ "ND should be already canonicalized.");
 
   std::vector Results;
   for (Decl *TopLevelDecl : AST.getLocalTopLevelDecls()) {
@@ -246,11 +284,11 @@
   if (Ref.Targets.empty())
 return;
   for (const auto *Target : Ref.Targets) {
-auto ID = getSymbolID(Targ

[clang-tools-extra] cf39bdb - [clangd] Implement Decl canonicalization rules for rename

[Kirill Bobyrev via cfe-commits]

Author: Kirill Bobyrev
Date: 2020-11-23T11:42:56+01:00
New Revision: cf39bdb49086350e7178a0a058273907d180e809

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

LOG: [clangd] Implement Decl canonicalization rules for rename

This patch introduces new canonicalization rules which are used for AST-based
rename in Clangd. By comparing two canonical declarations of inspected nodes,
Clangd determines whether both of them belong to the same entity user would
like to rename. Such functionality is relatively concise compared to the
Clang-Rename API that is used right now. It also helps to overcome the
limitations that Clang-Rename originally had and helps to eliminate several
classes of bugs.

Clangd AST-based rename currently relies on Clang-Rename which has design
limitations and also lacks some features. This patch breaks this dependency and
significantly reduces the amount of code to maintain (Clang-Rename is ~2000 LOC,
this patch is just <30 LOC of replacement code).

We eliminate technical debt by simultaneously

* Maintaining feature parity and ensuring no regressions
* Opening a straightforward path to improving existing rename bugs
* Making it possible to add more capabilities to rename feature which would not
  be possible with Clang-Rename

Reviewed By: hokein

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

Added: 


Modified: 
clang-tools-extra/clangd/refactor/Rename.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/Rename.cpp 
b/clang-tools-extra/clangd/refactor/Rename.cpp
index 33a9c845beaa..d10a7a4574ba 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -18,7 +18,6 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/SourceLocation.h"
-#include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/STLExtras.h"
@@ -76,6 +75,58 @@ llvm::Optional getOtherRefFile(const Decl &D, 
StringRef MainFile,
   return OtherFile;
 }
 
+// Canonical declarations help simplify the process of renaming. Examples:
+// - Template's canonical decl is the templated declaration (i.e.
+//   ClassTemplateDecl is canonicalized to its child CXXRecordDecl,
+//   FunctionTemplateDecl - to child FunctionDecl)
+// - Given a constructor/destructor, canonical declaration is the parent
+//   CXXRecordDecl because we want to rename both type name and its ctor/dtor.
+// - All specializations are canonicalized to the primary template. For 
example:
+//
+//template 
+//bool Foo = true; (1)
+//
+//template 
+//bool Foo = true; (2)
+//
+//template <>
+//bool Foo = true; (3)
+//
+// Here, both partial (2) and full (3) specializations are canonicalized to (1)
+// which ensures all three of them are renamed.
+const NamedDecl *canonicalRenameDecl(const NamedDecl *D) {
+  if (const auto *VarTemplate = dyn_cast(D))
+return canonicalRenameDecl(
+VarTemplate->getSpecializedTemplate()->getTemplatedDecl());
+  if (const auto *Template = dyn_cast(D))
+if (const NamedDecl *TemplatedDecl = Template->getTemplatedDecl())
+  return canonicalRenameDecl(TemplatedDecl);
+  if (const auto *ClassTemplateSpecialization =
+  dyn_cast(D))
+return canonicalRenameDecl(
+ClassTemplateSpecialization->getSpecializedTemplate()
+->getTemplatedDecl());
+  if (const auto *Method = dyn_cast(D)) {
+if (Method->getDeclKind() == Decl::Kind::CXXConstructor ||
+Method->getDeclKind() == Decl::Kind::CXXDestructor)
+  return canonicalRenameDecl(Method->getParent());
+if (const FunctionDecl *InstantiatedMethod =
+Method->getInstantiatedFromMemberFunction())
+  Method = cast(InstantiatedMethod);
+// FIXME(kirillbobyrev): For virtual methods with
+// size_overridden_methods() > 1, this will not rename all functions it
+// overrides, because this code assumes there is a single canonical
+// declaration.
+while (Method->isVirtual() && Method->size_overridden_methods())
+  Method = *Method->overridden_methods().begin();
+return dyn_cast(Method->getCanonicalDecl());
+  }
+  if (const auto *Function = dyn_cast(D))
+if (const FunctionTemplateDecl *Template = Function->getPrimaryTemplate())
+  return canonicalRenameDecl(Template);
+  return dyn_cast(D->getCanonicalDecl());
+}
+
 llvm::DenseSet locateDeclAt(ParsedAST &AST,
SourceLocation TokenStartLoc) {
   unsigned Offset =
@@ -91,9 +142,7 @@ llvm::DenseSet locateDeclAt(ParsedAST 
&AST,
   for (const NamedDecl *D :
targetDecl(SelectedNode->ASTNode,
   DeclRelation::Alias | DeclRelat

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcf39bdb49086: [clangd] Implement Decl canonicalization rules 
for rename (authored by kbobyrev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71880

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

Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -18,7 +18,6 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/SourceLocation.h"
-#include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/STLExtras.h"
@@ -76,6 +75,58 @@
   return OtherFile;
 }
 
+// Canonical declarations help simplify the process of renaming. Examples:
+// - Template's canonical decl is the templated declaration (i.e.
+//   ClassTemplateDecl is canonicalized to its child CXXRecordDecl,
+//   FunctionTemplateDecl - to child FunctionDecl)
+// - Given a constructor/destructor, canonical declaration is the parent
+//   CXXRecordDecl because we want to rename both type name and its ctor/dtor.
+// - All specializations are canonicalized to the primary template. For example:
+//
+//template 
+//bool Foo = true; (1)
+//
+//template 
+//bool Foo = true; (2)
+//
+//template <>
+//bool Foo = true; (3)
+//
+// Here, both partial (2) and full (3) specializations are canonicalized to (1)
+// which ensures all three of them are renamed.
+const NamedDecl *canonicalRenameDecl(const NamedDecl *D) {
+  if (const auto *VarTemplate = dyn_cast(D))
+return canonicalRenameDecl(
+VarTemplate->getSpecializedTemplate()->getTemplatedDecl());
+  if (const auto *Template = dyn_cast(D))
+if (const NamedDecl *TemplatedDecl = Template->getTemplatedDecl())
+  return canonicalRenameDecl(TemplatedDecl);
+  if (const auto *ClassTemplateSpecialization =
+  dyn_cast(D))
+return canonicalRenameDecl(
+ClassTemplateSpecialization->getSpecializedTemplate()
+->getTemplatedDecl());
+  if (const auto *Method = dyn_cast(D)) {
+if (Method->getDeclKind() == Decl::Kind::CXXConstructor ||
+Method->getDeclKind() == Decl::Kind::CXXDestructor)
+  return canonicalRenameDecl(Method->getParent());
+if (const FunctionDecl *InstantiatedMethod =
+Method->getInstantiatedFromMemberFunction())
+  Method = cast(InstantiatedMethod);
+// FIXME(kirillbobyrev): For virtual methods with
+// size_overridden_methods() > 1, this will not rename all functions it
+// overrides, because this code assumes there is a single canonical
+// declaration.
+while (Method->isVirtual() && Method->size_overridden_methods())
+  Method = *Method->overridden_methods().begin();
+return dyn_cast(Method->getCanonicalDecl());
+  }
+  if (const auto *Function = dyn_cast(D))
+if (const FunctionTemplateDecl *Template = Function->getPrimaryTemplate())
+  return canonicalRenameDecl(Template);
+  return dyn_cast(D->getCanonicalDecl());
+}
+
 llvm::DenseSet locateDeclAt(ParsedAST &AST,
SourceLocation TokenStartLoc) {
   unsigned Offset =
@@ -91,9 +142,7 @@
   for (const NamedDecl *D :
targetDecl(SelectedNode->ASTNode,
   DeclRelation::Alias | DeclRelation::TemplatePattern)) {
-// Get to CXXRecordDecl from constructor or destructor.
-D = tooling::getCanonicalSymbolDeclaration(D);
-Result.insert(D);
+Result.insert(canonicalRenameDecl(D));
   }
   return Result;
 }
@@ -226,19 +275,8 @@
 std::vector findOccurrencesWithinFile(ParsedAST &AST,
   const NamedDecl &ND) {
   trace::Span Tracer("FindOccurrencesWithinFile");
-  // If the cursor is at the underlying CXXRecordDecl of the
-  // ClassTemplateDecl, ND will be the CXXRecordDecl. In this case, we need to
-  // get the primary template manually.
-  // getUSRsForDeclaration will find other related symbols, e.g. virtual and its
-  // overriddens, primary template and all explicit specializations.
-  // FIXME: Get rid of the remaining tooling APIs.
-  const auto *RenameDecl =
-  ND.getDescribedTemplate() ? ND.getDescribedTemplate() : &ND;
-  std::vector RenameUSRs =
-  tooling::getUSRsForDeclaration(RenameDecl, AST.getASTContext());
-  llvm::DenseSet TargetIDs;
-  for (auto &USR : RenameUSRs)
-TargetIDs.insert(SymbolID(USR));
+  assert(canonicalRenameDecl(&ND) == &ND &&
+ "ND should be already canonicalized.");
 
   std::vector Results;
   for (Decl *TopLevelDecl : AST.getLocalTopLevelDecls()) {
@@ -246,11 +284,11 @@
   if (Ref.Targets.empty())

[PATCH] D91951: [clangd] Get rid of clangToolingRefactoring dependency

[Kirill Bobyrev via Phabricator via cfe-commits]

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

D71880  makes this dependency redundant and we 
can safely remove it. Tested for
both shared lib build and static lib build.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91951

Files:
  clang-tools-extra/clangd/CMakeLists.txt


Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -140,7 +140,6 @@
   clangTooling
   clangToolingCore
   clangToolingInclusions
-  clangToolingRefactoring
   clangToolingSyntax
   )
 


Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -140,7 +140,6 @@
   clangTooling
   clangToolingCore
   clangToolingInclusions
-  clangToolingRefactoring
   clangToolingSyntax
   )
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 3c696a2 - [AArch64][SVE] Allow lax conversion between VLATs and GNU vectors

[Joe Ellis via cfe-commits]

Author: Joe Ellis
Date: 2020-11-23T10:47:17Z
New Revision: 3c696a212ba4328e4f8f92136bc4d728a6490ef7

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

LOG: [AArch64][SVE] Allow lax conversion between VLATs and GNU vectors

Previously, lax conversions were only allowed between SVE vector-length
agnostic types and vector-length specific types. This meant that code
such as the following:

#include 
#define N __ARM_FEATURE_SVE_BITS
#define FIXED_ATTR __attribute__ ((vector_size (N/8)))
typedef float fixed_float32_t FIXED_ATTR;

void foo() {
fixed_float32_t fs32;
svfloat64_t s64;
fs32 = s64;
}

was not allowed.

This patch makes a minor change to areLaxCompatibleSveTypes to allow for
lax conversions to be performed between SVE vector-length agnostic types
and GNU vectors.

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

Added: 


Modified: 
clang/lib/AST/ASTContext.cpp
clang/test/Sema/aarch64-sve-lax-vector-conversions.c
clang/test/Sema/attr-arm-sve-vector-bits.c
clang/test/SemaCXX/aarch64-sve-lax-vector-conversions.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index f54916babed7..67ee8c0956d6 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -8586,10 +8586,20 @@ bool ASTContext::areLaxCompatibleSveTypes(QualType 
FirstType,
 
 const auto *VecTy = SecondType->getAs();
 if (VecTy &&
-VecTy->getVectorKind() == VectorType::SveFixedLengthDataVector) {
+(VecTy->getVectorKind() == VectorType::SveFixedLengthDataVector ||
+ VecTy->getVectorKind() == VectorType::GenericVector)) {
   const LangOptions::LaxVectorConversionKind LVCKind =
   getLangOpts().getLaxVectorConversions();
 
+  // If __ARM_FEATURE_SVE_BITS != N do not allow GNU vector lax conversion.
+  // "Whenever __ARM_FEATURE_SVE_BITS==N, GNUT implicitly
+  // converts to VLAT and VLAT implicitly converts to GNUT."
+  // ACLE Spec Version 00bet6, 3.7.3.2. Behavior common to vectors and
+  // predicates.
+  if (VecTy->getVectorKind() == VectorType::GenericVector &&
+  getTypeSize(SecondType) != getLangOpts().ArmSveVectorBits)
+return false;
+
   // If -flax-vector-conversions=all is specified, the types are
   // certainly compatible.
   if (LVCKind == LangOptions::LaxVectorConversionKind::All)

diff  --git a/clang/test/Sema/aarch64-sve-lax-vector-conversions.c 
b/clang/test/Sema/aarch64-sve-lax-vector-conversions.c
index e2fe87f7dd20..1a1addcf1c1b 100644
--- a/clang/test/Sema/aarch64-sve-lax-vector-conversions.c
+++ b/clang/test/Sema/aarch64-sve-lax-vector-conversions.c
@@ -7,32 +7,61 @@
 #include 
 
 #define N __ARM_FEATURE_SVE_BITS
-#define FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
+#define SVE_FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
+#define GNU_FIXED_ATTR __attribute__((vector_size(N / 8)))
 
-typedef svfloat32_t fixed_float32_t FIXED_ATTR;
-typedef svint32_t fixed_int32_t FIXED_ATTR;
+typedef svfloat32_t sve_fixed_float32_t SVE_FIXED_ATTR;
+typedef svint32_t sve_fixed_int32_t SVE_FIXED_ATTR;
+typedef float gnu_fixed_float32_t GNU_FIXED_ATTR;
+typedef int gnu_fixed_int32_t GNU_FIXED_ATTR;
 
-void allowed_with_integer_lax_conversions() {
-  fixed_int32_t fi32;
+void sve_allowed_with_integer_lax_conversions() {
+  sve_fixed_int32_t fi32;
   svint64_t si64;
 
   // The implicit cast here should fail if -flax-vector-conversions=none, but 
pass if
   // -flax-vector-conversions={integer,all}.
   fi32 = si64;
-  // lax-vector-none-error@-1 {{assigning to 'fixed_int32_t' (vector of 16 
'int' values) from incompatible type}}
+  // lax-vector-none-error@-1 {{assigning to 'sve_fixed_int32_t' (vector of 16 
'int' values) from incompatible type}}
   si64 = fi32;
   // lax-vector-none-error@-1 {{assigning to 'svint64_t' (aka '__SVInt64_t') 
from incompatible type}}
 }
 
-void allowed_with_all_lax_conversions() {
-  fixed_float32_t ff32;
+void sve_allowed_with_all_lax_conversions() {
+  sve_fixed_float32_t ff32;
   svfloat64_t sf64;
 
   // The implicit cast here should fail if 
-flax-vector-conversions={none,integer}, but pass if
   // -flax-vector-conversions=all.
   ff32 = sf64;
-  // lax-vector-none-error@-1 {{assigning to 'fixed_float32_t' (vector of 16 
'float' values) from incompatible type}}
-  // lax-vector-integer-error@-2 {{assigning to 'fixed_float32_t' (vector of 
16 'float' values) from incompatible type}}
+  // lax-vector-none-error@-1 {{assigning to 'sve_fixed_float32_t' (vector of 
16 'float' values) from incompatible type}}
+  // lax-vector-integer-error@-2 {{assigning to 'sve_fixed_float32_t' (vector 
of 16 'float' values) from incompatible type}}
+  sf64 = ff32;
+  //

[PATCH] D91696: [AArch64][SVE] Allow lax conversion between VLATs and GNU vectors

[Joe Ellis via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3c696a212ba4: [AArch64][SVE] Allow lax conversion between 
VLATs and GNU vectors (authored by joechrisellis).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91696

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/Sema/aarch64-sve-lax-vector-conversions.c
  clang/test/Sema/attr-arm-sve-vector-bits.c
  clang/test/SemaCXX/aarch64-sve-lax-vector-conversions.cpp

Index: clang/test/SemaCXX/aarch64-sve-lax-vector-conversions.cpp
===
--- clang/test/SemaCXX/aarch64-sve-lax-vector-conversions.cpp
+++ clang/test/SemaCXX/aarch64-sve-lax-vector-conversions.cpp
@@ -7,32 +7,61 @@
 #include 
 
 #define N __ARM_FEATURE_SVE_BITS
-#define FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
+#define SVE_FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
+#define GNU_FIXED_ATTR __attribute__((vector_size(N / 8)))
 
-typedef svfloat32_t fixed_float32_t FIXED_ATTR;
-typedef svint32_t fixed_int32_t FIXED_ATTR;
+typedef svfloat32_t sve_fixed_float32_t SVE_FIXED_ATTR;
+typedef svint32_t sve_fixed_int32_t SVE_FIXED_ATTR;
+typedef float gnu_fixed_float32_t GNU_FIXED_ATTR;
+typedef int gnu_fixed_int32_t GNU_FIXED_ATTR;
 
-void allowed_with_integer_lax_conversions() {
-  fixed_int32_t fi32;
+void sve_allowed_with_integer_lax_conversions() {
+  sve_fixed_int32_t fi32;
   svint64_t si64;
 
   // The implicit cast here should fail if -flax-vector-conversions=none, but pass if
   // -flax-vector-conversions={integer,all}.
   fi32 = si64;
-  // lax-vector-none-error@-1 {{assigning to 'fixed_int32_t' (vector of 16 'int' values) from incompatible type}}
+  // lax-vector-none-error@-1 {{assigning to 'sve_fixed_int32_t' (vector of 16 'int' values) from incompatible type}}
   si64 = fi32;
   // lax-vector-none-error@-1 {{assigning to 'svint64_t' (aka '__SVInt64_t') from incompatible type}}
 }
 
-void allowed_with_all_lax_conversions() {
-  fixed_float32_t ff32;
+void sve_allowed_with_all_lax_conversions() {
+  sve_fixed_float32_t ff32;
   svfloat64_t sf64;
 
   // The implicit cast here should fail if -flax-vector-conversions={none,integer}, but pass if
   // -flax-vector-conversions=all.
   ff32 = sf64;
-  // lax-vector-none-error@-1 {{assigning to 'fixed_float32_t' (vector of 16 'float' values) from incompatible type}}
-  // lax-vector-integer-error@-2 {{assigning to 'fixed_float32_t' (vector of 16 'float' values) from incompatible type}}
+  // lax-vector-none-error@-1 {{assigning to 'sve_fixed_float32_t' (vector of 16 'float' values) from incompatible type}}
+  // lax-vector-integer-error@-2 {{assigning to 'sve_fixed_float32_t' (vector of 16 'float' values) from incompatible type}}
+  sf64 = ff32;
+  // lax-vector-none-error@-1 {{assigning to 'svfloat64_t' (aka '__SVFloat64_t') from incompatible type}}
+  // lax-vector-integer-error@-2 {{assigning to 'svfloat64_t' (aka '__SVFloat64_t') from incompatible type}}
+}
+
+void gnu_allowed_with_integer_lax_conversions() {
+  gnu_fixed_int32_t fi32;
+  svint64_t si64;
+
+  // The implicit cast here should fail if -flax-vector-conversions=none, but pass if
+  // -flax-vector-conversions={integer,all}.
+  fi32 = si64;
+  // lax-vector-none-error@-1 {{assigning to 'gnu_fixed_int32_t' (vector of 16 'int' values) from incompatible type}}
+  si64 = fi32;
+  // lax-vector-none-error@-1 {{assigning to 'svint64_t' (aka '__SVInt64_t') from incompatible type}}
+}
+
+void gnu_allowed_with_all_lax_conversions() {
+  gnu_fixed_float32_t ff32;
+  svfloat64_t sf64;
+
+  // The implicit cast here should fail if -flax-vector-conversions={none,integer}, but pass if
+  // -flax-vector-conversions=all.
+  ff32 = sf64;
+  // lax-vector-none-error@-1 {{assigning to 'gnu_fixed_float32_t' (vector of 16 'float' values) from incompatible type}}
+  // lax-vector-integer-error@-2 {{assigning to 'gnu_fixed_float32_t' (vector of 16 'float' values) from incompatible type}}
   sf64 = ff32;
   // lax-vector-none-error@-1 {{assigning to 'svfloat64_t' (aka '__SVFloat64_t') from incompatible type}}
   // lax-vector-integer-error@-2 {{assigning to 'svfloat64_t' (aka '__SVFloat64_t') from incompatible type}}
Index: clang/test/Sema/attr-arm-sve-vector-bits.c
===
--- clang/test/Sema/attr-arm-sve-vector-bits.c
+++ clang/test/Sema/attr-arm-sve-vector-bits.c
@@ -272,9 +272,6 @@
 // Test the implicit conversion only applies to valid types
 fixed_bool_t to_fixed_bool_t__from_svint32_t(svint32_t x) { return x; } // expected-error-re {{returning 'svint32_t' (aka '__SVInt32_t') from a function with incompatible result type 'fixed_bool_t' (vector of {{[0-9]+}} 'unsigned char' values)}}
 
-svint64_t to_svint64_t__from_gnu_int32_t(gnu_int32_t x) { return x; } // expected-error-re {{returning 'gn

[PATCH] D91952: [clangd] Add support for within-file rename of complicated fields

[Kirill Bobyrev via Phabricator via cfe-commits]

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

This was originally a part of D71880  but is 
separated for simplicity and ease
of reviewing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91952

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  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
@@ -540,6 +540,81 @@
 }
   )cpp",
 
+  // Fields in classes & partial and full specialiations.
+  R"cpp(
+class Foo {
+public:
+  Foo(int Variable) : [[Variabl^e]](Variable) {}
+
+  int [[Va^riable]] = 42;
+
+private:
+  void foo() { ++[[Vari^able]]; }
+};
+
+void bar() {
+  Foo f(9000);
+  f.[[Variable^]] = -1;
+}
+  )cpp",
+  R"cpp(
+template
+struct Foo {
+  T [[Vari^able]] = 42;
+};
+
+void foo() {
+  Foo f;
+  f.[[Varia^ble]] = 9000;
+}
+  )cpp",
+  R"cpp(
+template
+struct Foo {
+  T Variable[42];
+  U Another;
+
+  void bar() {}
+};
+
+template
+struct Foo {
+  T [[Var^iable]];
+  void bar() { ++[[Var^iable]]; }
+};
+
+void foo() {
+  Foo f;
+  f.[[Var^iable]] = 9000;
+}
+  )cpp",
+  R"cpp(
+template
+struct Foo {
+  T Variable[42];
+  U Another;
+
+  void bar() {}
+};
+
+template
+struct Foo {
+  T Variable;
+  void bar() { ++Variable; }
+};
+
+template<>
+struct Foo {
+  unsigned [[Var^iable]];
+  void bar() { ++[[Var^iable]]; }
+};
+
+void foo() {
+  Foo f;
+  f.[[Var^iable]] = 9000;
+}
+  )cpp",
+
   // Template parameters.
   R"cpp(
 template 
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -119,11 +119,35 @@
 // declaration.
 while (Method->isVirtual() && Method->size_overridden_methods())
   Method = *Method->overridden_methods().begin();
-return dyn_cast(Method->getCanonicalDecl());
+return Method->getCanonicalDecl();
   }
   if (const auto *Function = dyn_cast(D))
 if (const FunctionTemplateDecl *Template = Function->getPrimaryTemplate())
   return canonicalRenameDecl(Template);
+  if (const auto *Field = dyn_cast(D)) {
+// This is a hacky way to do something like
+// CXXMethodDecl::getInstantiatedFromMemberFunction for the field because
+// Clang AST does not store relevant information about the field that is
+// instantiated.
+const auto *TemplateSpec =
+dyn_cast(Field->getParent());
+if (!TemplateSpec)
+  return Field->getCanonicalDecl();
+const CXXRecordDecl *FieldParent =
+TemplateSpec->getTemplateInstantiationPattern();
+// Field is not instantiation.
+if (!FieldParent || Field->getParent() == FieldParent)
+  return Field->getCanonicalDecl();
+for (const FieldDecl *Candidate : FieldParent->fields()) {
+  if (Field->getFieldIndex() == Candidate->getFieldIndex()) {
+assert(Field->getLocation() == Candidate->getLocation() &&
+   "Field should be generated from Candidate so it has the same "
+   "location.");
+return Candidate->getCanonicalDecl();
+  }
+}
+llvm_unreachable("FieldParent should have field with same index as Field.");
+  }
   return dyn_cast(D->getCanonicalDecl());
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 1319c66 - [clangd] Get rid of clangToolingRefactoring dependency

[Kirill Bobyrev via cfe-commits]

Author: Kirill Bobyrev
Date: 2020-11-23T11:59:38+01:00
New Revision: 1319c6624ed6b0a6fb48d975bbf278263a85fcef

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

LOG: [clangd] Get rid of clangToolingRefactoring dependency

D71880 makes this dependency redundant and we can safely remove it. Tested for
both shared lib build and static lib build.

Reviewed By: hokein

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

Added: 


Modified: 
clang-tools-extra/clangd/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/clangd/CMakeLists.txt 
b/clang-tools-extra/clangd/CMakeLists.txt
index 72b232b92c08..b8300ddaf548 100644
--- a/clang-tools-extra/clangd/CMakeLists.txt
+++ b/clang-tools-extra/clangd/CMakeLists.txt
@@ -140,7 +140,6 @@ clang_target_link_libraries(clangDaemon
   clangTooling
   clangToolingCore
   clangToolingInclusions
-  clangToolingRefactoring
   clangToolingSyntax
   )
 



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

[PATCH] D91951: [clangd] Get rid of clangToolingRefactoring dependency

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1319c6624ed6: [clangd] Get rid of clangToolingRefactoring 
dependency (authored by kbobyrev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91951

Files:
  clang-tools-extra/clangd/CMakeLists.txt


Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -140,7 +140,6 @@
   clangTooling
   clangToolingCore
   clangToolingInclusions
-  clangToolingRefactoring
   clangToolingSyntax
   )
 


Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -140,7 +140,6 @@
   clangTooling
   clangToolingCore
   clangToolingInclusions
-  clangToolingRefactoring
   clangToolingSyntax
   )
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91861: [clang][cli] Split DefaultAnyOf into a default value and ImpliedByAnyOf

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

Thanks for the feedback. I left some comments inline and will update the patch 
accordingly.




Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3762-3764
+this->KEYPATH = MERGER(this->KEYPATH, DEFAULT_VALUE);  
\
+if (IMPLIED_CHECK) 
\
+  this->KEYPATH = MERGER(this->KEYPATH, IMPLIED_VALUE);
\

dexonsmith wrote:
> This flip to the logic is interesting. I see it now matches the non-flag 
> case; I can't remember if there was a subtle reason it was different before 
> though.
This makes more sense to me personally and I saw similar change in a later 
patch D84674. Let me know if you remember why it wasn't like that already.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4027-4031
+  if (((FLAGS)&options::CC1Option) &&  
\
+  (ALWAYS_EMIT ||  
\
+   (EXTRACTOR(this->KEYPATH) != DEFAULT_VALUE && !(IMPLIED_CHECK { 
\
 DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, EXTRACTOR(this->KEYPATH));   
\
   }

dexonsmith wrote:
> I'm not sure this logic is quite right. It looks to me like if an option can 
> very be implied, it will never be seriazed to `-cc1`, even if its current 
> value is not an implied one.
> 
> Or have I understood the conditions under which `IMPLIED_CHECK` returns 
> `true`?
> 
> IIUC, then this logic seems closer:
> ```
> if (((FLAGS)&options::CC1Option) &&  \
> (ALWAYS_EMIT ||  \
>  (EXTRACTOR(this->KEYPATH) !=\
>   (IMPLIED_CHECK ? (DEFAULT_VALUE)   \
>  : (MERGER(DEFAULT_VALUE, IMPLIED_VALUE)) {  \
>   DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, EXTRACTOR(this->KEYPATH));   \
> }
> ```
> 
> It would be great to see tests in particular for a bitset (or similar) option 
> where the merger does a union.
`IMPLIED_CHECK` is a logical disjunction of the implying option keypaths. It 
evaluates to `true` whenever at least one of the implying keypaths evaluates to 
`true`.

I think I know what you're concerned about. Let me paraphrase it and check if 
my understanding is correct:
Suppose option `a` has default value of `x`, and flag `b` can imply the value 
of `a` to be `y`. If we have a command line `-b -a=z`, then `-a=z` would not be 
generated with the current logic: `EXTRACTOR(this->KEYPATH) != DEFAULT_VALUE` 
evaluates to true, but `!(IMPLIED_CHECK)` to `false`.

Your conditions gets close, but I think the ternary branches should be the 
other way around.

Here's a table exploring all cases:

```
IMPLIED_CHECK | EXTRACTOR(this->KEYPATH) == | SHOULD_EMIT
--+-+-
true  | IMPLIED_VALUE   | NO - emitting only the implying 
option is enough
true  | DEFAULT_VALUE   | YES - value explicitly specified 
(and it's DEFAULT_VALUE just by chance)
true  | ??? | YES - value explicitly specified
false | IMPLIED_VALUE   | YES - value explicitly specified 
(and it's IMPLIED_VALUE just by chance)
false | DEFAULT_VALUE   | NO - default value handles this 
automatically
false | ??? | YES - value explicitly specified
```

I think this logic is what we're looking for:

```
  if (((FLAGS)&options::CC1Option) &&  \
  (ALWAYS_EMIT ||  \
   (EXTRACTOR(this->KEYPATH) !=\
((IMPLIED_CHECK) ? (IMPLIED_VALUE) : (DEFAULT_VALUE) { \
DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, EXTRACTOR(this->KEYPATH));   \
  }
```



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4027-4031
+  if (((FLAGS)&options::CC1Option) &&  
\
+  (ALWAYS_EMIT ||  
\
+   (EXTRACTOR(this->KEYPATH) != DEFAULT_VALUE && !(IMPLIED_CHECK { 
\
 DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, EXTRACTOR(this->KEYPATH));   
\
   }

jansvoboda11 wrote:
> dexonsmith wrote:
> > I'm not sure this logic is quite right. It looks to me like if an option 
> > can very be implied, it will never be seriazed to `-cc1`, even if its 
> > current value is not an implied one.
> > 
> > Or have I understood the conditions under which `IMPLIED_CHECK` returns 
> > `true`?
> > 
> > IIUC, then this logic seems closer:
> > ```
> > if (((FLAGS)&options::CC1Option) &&

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

[Chirag Khandelwal via Phabricator via cfe-commits]

AMDChirag updated this revision to Diff 307022.
AMDChirag added a comment.

Fixed usage of BodyGenCallbackTy
Removed ArrayRef variable


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91054

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp


Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -3546,12 +3546,65 @@
 }
 
 void CodeGenFunction::EmitOMPSectionsDirective(const OMPSectionsDirective &S) {
-  {
-auto LPCRegion =
-CGOpenMPRuntime::LastprivateConditionalRAII::disable(*this, S);
-OMPLexicalScope Scope(*this, S, OMPD_unknown);
-EmitSections(S);
+  if (CGM.getLangOpts().OpenMPIRBuilder) {
+llvm::OpenMPIRBuilder &OMPBuilder = CGM.getOpenMPRuntime().getOMPBuilder();
+using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
+using BodyGenCallbackTy = llvm::OpenMPIRBuilder::BodyGenCallbackTy;
+
+auto FiniCB = [this](InsertPointTy IP) {
+  OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
+};
+
+const Stmt *CapturedStmt = S.getInnermostCapturedStmt()->getCapturedStmt();
+const auto *CS = dyn_cast(CapturedStmt);
+llvm::SmallVector SectionCBVector;
+if (CS) {
+  for (const Stmt *SubStmt : CS->children()) {
+auto SectionCB = [this, SubStmt](InsertPointTy AllocaIP,
+ InsertPointTy CodeGenIP,
+ llvm::BasicBlock &FiniBB) {
+  OMPBuilderCBHelpers::InlinedRegionBodyRAII IRB(*this, AllocaIP,
+ FiniBB);
+  OMPBuilderCBHelpers::EmitOMPRegionBody(*this, SubStmt, CodeGenIP,
+ FiniBB);
+};
+SectionCBVector.push_back(SectionCB);
+  }
+} else {
+  auto SectionCB = [this, CS](InsertPointTy AllocaIP,
+  InsertPointTy CodeGenIP,
+  llvm::BasicBlock &FiniBB) {
+OMPBuilderCBHelpers::InlinedRegionBodyRAII IRB(*this, AllocaIP, 
FiniBB);
+OMPBuilderCBHelpers::EmitOMPRegionBody(*this, CS, CodeGenIP, FiniBB);
+  };
+  SectionCBVector.push_back(SectionCB);
+}
+
+// Privatization callback that performs appropriate action for
+// shared/private/firstprivate/lastprivate/copyin/... variables.
+//
+// TODO: This defaults to shared right now.
+auto PrivCB = [](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
+ llvm::Value &Val, llvm::Value *&ReplVal) {
+  // The next line is appropriate only for variables (Val) with the
+  // data-sharing attribute "shared".
+  ReplVal = &Val;
+
+  return CodeGenIP;
+};
+
+llvm::OpenMPIRBuilder::InsertPointTy AllocaIP(
+AllocaInsertPt->getParent(), AllocaInsertPt->getIterator());
+Builder.restoreIP(OMPBuilder.CreateSections(
+Builder, AllocaIP, SectionCBVector, PrivCB, FiniCB, S.hasCancel(),
+S.getSingleClause()));
+
+return;
   }
+  auto LPCRegion =
+  CGOpenMPRuntime::LastprivateConditionalRAII::disable(*this, S);
+  OMPLexicalScope Scope(*this, S, OMPD_unknown);
+  EmitSections(S);
   // Emit an implicit barrier at the end.
   if (!S.getSingleClause()) {
 CGM.getOpenMPRuntime().emitBarrierCall(*this, S.getBeginLoc(),


Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -3546,12 +3546,65 @@
 }
 
 void CodeGenFunction::EmitOMPSectionsDirective(const OMPSectionsDirective &S) {
-  {
-auto LPCRegion =
-CGOpenMPRuntime::LastprivateConditionalRAII::disable(*this, S);
-OMPLexicalScope Scope(*this, S, OMPD_unknown);
-EmitSections(S);
+  if (CGM.getLangOpts().OpenMPIRBuilder) {
+llvm::OpenMPIRBuilder &OMPBuilder = CGM.getOpenMPRuntime().getOMPBuilder();
+using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
+using BodyGenCallbackTy = llvm::OpenMPIRBuilder::BodyGenCallbackTy;
+
+auto FiniCB = [this](InsertPointTy IP) {
+  OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
+};
+
+const Stmt *CapturedStmt = S.getInnermostCapturedStmt()->getCapturedStmt();
+const auto *CS = dyn_cast(CapturedStmt);
+llvm::SmallVector SectionCBVector;
+if (CS) {
+  for (const Stmt *SubStmt : CS->children()) {
+auto SectionCB = [this, SubStmt](InsertPointTy AllocaIP,
+ InsertPointTy CodeGenIP,
+ llvm::BasicBlock &FiniBB) {
+  OMPBuilderCBHelpers::InlinedRegionBodyRAII IRB(*this, AllocaIP,
+ FiniBB);
+  OMPBuilderCBHelpers::EmitOMPRegionBody(*this, SubStmt, CodeGenIP,
+  

[PATCH] D91861: [clang][cli] Split DefaultAnyOf into a default value and ImpliedByAnyOf

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 307028.
jansvoboda11 added a comment.

Add tests, fix condition in generator


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91861

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/unittests/Option/OptionMarshallingTest.cpp
  llvm/unittests/Option/Opts.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -71,6 +71,8 @@
   StringRef KeyPath;
   StringRef DefaultValue;
   StringRef NormalizedValuesScope;
+  StringRef ImpliedCheck;
+  StringRef ImpliedValue;
   StringRef Normalizer;
   StringRef Denormalizer;
   StringRef ValueMerger;
@@ -113,6 +115,10 @@
 OS << ", ";
 emitScopedNormalizedValue(OS, DefaultValue);
 OS << ", ";
+OS << ImpliedCheck;
+OS << ", ";
+emitScopedNormalizedValue(OS, ImpliedValue);
+OS << ", ";
 OS << Normalizer;
 OS << ", ";
 OS << Denormalizer;
@@ -188,6 +194,9 @@
   Ret->KeyPath = R.getValueAsString("KeyPath");
   Ret->DefaultValue = R.getValueAsString("DefaultValue");
   Ret->NormalizedValuesScope = R.getValueAsString("NormalizedValuesScope");
+  Ret->ImpliedCheck = R.getValueAsString("ImpliedCheck");
+  Ret->ImpliedValue =
+  R.getValueAsOptionalString("ImpliedValue").getValueOr(Ret->DefaultValue);
 
   Ret->Normalizer = R.getValueAsString("Normalizer");
   Ret->Denormalizer = R.getValueAsString("Denormalizer");
Index: llvm/unittests/Option/Opts.td
===
--- llvm/unittests/Option/Opts.td
+++ llvm/unittests/Option/Opts.td
@@ -46,10 +46,13 @@
 def DashDash : Option<["--"], "", KIND_REMAINING_ARGS>;
 
 def marshalled_flag_d : Flag<["-"], "marshalled-flag-d">,
-  MarshallingInfoFlag<"MarshalledFlagD", DefaultAnyOf<[]>>;
+  MarshallingInfoFlag<"MarshalledFlagD">;
 def marshalled_flag_c : Flag<["-"], "marshalled-flag-c">,
-  MarshallingInfoFlag<"MarshalledFlagC", DefaultAnyOf<[marshalled_flag_d]>>;
+  MarshallingInfoFlag<"MarshalledFlagC">,
+  ImpliedByAnyOf<[marshalled_flag_d], "true">;
 def marshalled_flag_b : Flag<["-"], "marshalled-flag-b">,
-  MarshallingInfoFlag<"MarshalledFlagB", DefaultAnyOf<[marshalled_flag_d]>>;
+  MarshallingInfoFlag<"MarshalledFlagB">,
+  ImpliedByAnyOf<[marshalled_flag_d], "true">;
 def marshalled_flag_a : Flag<["-"], "marshalled-flag-a">,
-  MarshallingInfoFlag<"MarshalledFlagA", DefaultAnyOf<[marshalled_flag_c, marshalled_flag_b]>>;
+  MarshallingInfoFlag<"MarshalledFlagA">,
+  ImpliedByAnyOf<[marshalled_flag_c, marshalled_flag_b]>;
Index: llvm/unittests/Option/OptionMarshallingTest.cpp
===
--- llvm/unittests/Option/OptionMarshallingTest.cpp
+++ llvm/unittests/Option/OptionMarshallingTest.cpp
@@ -11,15 +11,17 @@
 struct OptionWithMarshallingInfo {
   const char *Name;
   const char *KeyPath;
-  const char *DefaultValue;
+  const char *ImpliedCheck;
+  const char *ImpliedValue;
 };
 
 static const OptionWithMarshallingInfo MarshallingTable[] = {
 #define OPTION_WITH_MARSHALLING(   \
 PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,\
 HELPTEXT, METAVAR, VALUES, SPELLING, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE,  \
-NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, TABLE_INDEX)  \
-  {NAME, #KEYPATH, #DEFAULT_VALUE},
+IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, \
+TABLE_INDEX)   \
+  {NAME, #KEYPATH, #IMPLIED_CHECK, #IMPLIED_VALUE},
 #include "Opts.inc"
 #undef OPTION_WITH_MARSHALLING
 };
@@ -38,10 +40,16 @@
   ASSERT_STREQ(MarshallingTable[3].KeyPath, "MarshalledFlagA");
 }
 
-TEST(OptionMarshalling, DefaultAnyOfConstructedDisjunctionOfKeypaths) {
-  ASSERT_STREQ(MarshallingTable[0].DefaultValue, "false");
-  ASSERT_STREQ(MarshallingTable[1].DefaultValue, "false || MarshalledFlagD");
-  ASSERT_STREQ(MarshallingTable[2].DefaultValue, "false || MarshalledFlagD");
-  ASSERT_STREQ(MarshallingTable[3].DefaultValue,
-"false || MarshalledFlagC || MarshalledFlagB");
+TEST(OptionMarshalling, ImpliedCheckContainsDisjunctionOfKeypaths) {
+  ASSERT_STREQ(MarshallingTable[0].ImpliedCheck, "false");
+
+  ASSERT_STREQ(MarshallingTable[1].ImpliedCheck, "false || MarshalledFlagD");
+  ASSERT_STREQ(MarshallingTable[1].ImpliedValue, "true");
+
+  ASSERT_STREQ(MarshallingTable[2].ImpliedCheck, "false || MarshalledFlagD");
+  ASSERT_STREQ(MarshallingTable[2].ImpliedValue, "true");
+
+  ASSERT_STREQ(MarshallingTable[3].ImpliedCheck,
+   "f

[clang-tools-extra] 8cec8de - [clangd] testPath's final result agrees with the passed in Style

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-11-23T12:45:06+01:00
New Revision: 8cec8de2a4e6692da6226bb02cf417eb0e50adde

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

LOG: [clangd] testPath's final result agrees with the passed in Style

This was confusing, as testRoot on windows results in C:\\clangd-test
and testPath generated with posix explicitly still contained backslashes.

This patch ensures not only the relative part, but the whole final result
respects passed in Style.

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

Added: 


Modified: 
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/TestFS.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp 
b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
index a2423094b17a..2b4605eb97e2 100644
--- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -255,7 +255,6 @@ TEST_F(ConfigCompileTests, ExternalBlockMountPoint) {
   };
 
   auto BarPath = testPath("foo/bar.h", llvm::sys::path::Style::posix);
-  BarPath = llvm::sys::path::convert_to_slash(BarPath);
   Parm.Path = BarPath;
   // Non-absolute MountPoint without a directory raises an error.
   Frag = GetFrag("", "foo");
@@ -269,7 +268,6 @@ TEST_F(ConfigCompileTests, ExternalBlockMountPoint) {
   ASSERT_FALSE(Conf.Index.External);
 
   auto FooPath = testPath("foo/", llvm::sys::path::Style::posix);
-  FooPath = llvm::sys::path::convert_to_slash(FooPath);
   // Ok when relative.
   Frag = GetFrag(testRoot(), "foo/");
   compileAndApply();
@@ -293,7 +291,6 @@ TEST_F(ConfigCompileTests, ExternalBlockMountPoint) {
 
   // File outside MountPoint, no index.
   auto BazPath = testPath("bar/baz.h", llvm::sys::path::Style::posix);
-  BazPath = llvm::sys::path::convert_to_slash(BazPath);
   Parm.Path = BazPath;
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
@@ -302,7 +299,6 @@ TEST_F(ConfigCompileTests, ExternalBlockMountPoint) {
 
   // File under MountPoint, index should be set.
   BazPath = testPath("foo/baz.h", llvm::sys::path::Style::posix);
-  BazPath = llvm::sys::path::convert_to_slash(BazPath);
   Parm.Path = BazPath;
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();

diff  --git a/clang-tools-extra/clangd/unittests/TestFS.cpp 
b/clang-tools-extra/clangd/unittests/TestFS.cpp
index ba4010cb4581..f343a85331d0 100644
--- a/clang-tools-extra/clangd/unittests/TestFS.cpp
+++ b/clang-tools-extra/clangd/unittests/TestFS.cpp
@@ -80,12 +80,10 @@ const char *testRoot() {
 }
 
 std::string testPath(PathRef File, llvm::sys::path::Style Style) {
-  assert(llvm::sys::path::is_relative(File) && "FileName should be relative");
-
-  llvm::SmallString<32> NativeFile = File;
-  llvm::sys::path::native(NativeFile, Style);
+  assert(llvm::sys::path::is_relative(File, Style));
   llvm::SmallString<32> Path;
-  llvm::sys::path::append(Path, Style, testRoot(), NativeFile);
+  llvm::sys::path::append(Path, testRoot(), File);
+  llvm::sys::path::native(Path, Style);
   return std::string(Path.str());
 }
 



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

[PATCH] D91947: [clangd] testPath's final result agrees with the passed in Style

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8cec8de2a4e6: [clangd] testPath's final result agrees 
with the passed in Style (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91947

Files:
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/TestFS.cpp


Index: clang-tools-extra/clangd/unittests/TestFS.cpp
===
--- clang-tools-extra/clangd/unittests/TestFS.cpp
+++ clang-tools-extra/clangd/unittests/TestFS.cpp
@@ -80,12 +80,10 @@
 }
 
 std::string testPath(PathRef File, llvm::sys::path::Style Style) {
-  assert(llvm::sys::path::is_relative(File) && "FileName should be relative");
-
-  llvm::SmallString<32> NativeFile = File;
-  llvm::sys::path::native(NativeFile, Style);
+  assert(llvm::sys::path::is_relative(File, Style));
   llvm::SmallString<32> Path;
-  llvm::sys::path::append(Path, Style, testRoot(), NativeFile);
+  llvm::sys::path::append(Path, testRoot(), File);
+  llvm::sys::path::native(Path, Style);
   return std::string(Path.str());
 }
 
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -255,7 +255,6 @@
   };
 
   auto BarPath = testPath("foo/bar.h", llvm::sys::path::Style::posix);
-  BarPath = llvm::sys::path::convert_to_slash(BarPath);
   Parm.Path = BarPath;
   // Non-absolute MountPoint without a directory raises an error.
   Frag = GetFrag("", "foo");
@@ -269,7 +268,6 @@
   ASSERT_FALSE(Conf.Index.External);
 
   auto FooPath = testPath("foo/", llvm::sys::path::Style::posix);
-  FooPath = llvm::sys::path::convert_to_slash(FooPath);
   // Ok when relative.
   Frag = GetFrag(testRoot(), "foo/");
   compileAndApply();
@@ -293,7 +291,6 @@
 
   // File outside MountPoint, no index.
   auto BazPath = testPath("bar/baz.h", llvm::sys::path::Style::posix);
-  BazPath = llvm::sys::path::convert_to_slash(BazPath);
   Parm.Path = BazPath;
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
@@ -302,7 +299,6 @@
 
   // File under MountPoint, index should be set.
   BazPath = testPath("foo/baz.h", llvm::sys::path::Style::posix);
-  BazPath = llvm::sys::path::convert_to_slash(BazPath);
   Parm.Path = BazPath;
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();


Index: clang-tools-extra/clangd/unittests/TestFS.cpp
===
--- clang-tools-extra/clangd/unittests/TestFS.cpp
+++ clang-tools-extra/clangd/unittests/TestFS.cpp
@@ -80,12 +80,10 @@
 }
 
 std::string testPath(PathRef File, llvm::sys::path::Style Style) {
-  assert(llvm::sys::path::is_relative(File) && "FileName should be relative");
-
-  llvm::SmallString<32> NativeFile = File;
-  llvm::sys::path::native(NativeFile, Style);
+  assert(llvm::sys::path::is_relative(File, Style));
   llvm::SmallString<32> Path;
-  llvm::sys::path::append(Path, Style, testRoot(), NativeFile);
+  llvm::sys::path::append(Path, testRoot(), File);
+  llvm::sys::path::native(Path, Style);
   return std::string(Path.str());
 }
 
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -255,7 +255,6 @@
   };
 
   auto BarPath = testPath("foo/bar.h", llvm::sys::path::Style::posix);
-  BarPath = llvm::sys::path::convert_to_slash(BarPath);
   Parm.Path = BarPath;
   // Non-absolute MountPoint without a directory raises an error.
   Frag = GetFrag("", "foo");
@@ -269,7 +268,6 @@
   ASSERT_FALSE(Conf.Index.External);
 
   auto FooPath = testPath("foo/", llvm::sys::path::Style::posix);
-  FooPath = llvm::sys::path::convert_to_slash(FooPath);
   // Ok when relative.
   Frag = GetFrag(testRoot(), "foo/");
   compileAndApply();
@@ -293,7 +291,6 @@
 
   // File outside MountPoint, no index.
   auto BazPath = testPath("bar/baz.h", llvm::sys::path::Style::posix);
-  BazPath = llvm::sys::path::convert_to_slash(BazPath);
   Parm.Path = BazPath;
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
@@ -302,7 +299,6 @@
 
   // File under MountPoint, index should be set.
   BazPath = testPath("foo/baz.h", llvm::sys::path::Style::posix);
-  BazPath = llvm::sys::path::convert_to_slash(BazPath);
   Parm.Path = BazPath;
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91948: [WIP][analyzer][doc] Add Container- and IteratorModeling developer docs

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

In general, perhaps you should go over the rendered text once again and make 
use of //emph// and **bold** some more.
Explanation looks okay from my part, although I'm really not knowledgeable 
about CSA internals. But there are plenty of "typesetting" issues.

Do you have to stick to 80-col in the documentation file? It is customary (at 
least in LaTeX) that prose is organised in a "one line of code per sentence" 
fashion. This also makes handling the diff easier... In the latter part towards 
the discussion about how iterators are targeted, the "80-col" is broken anyways.




Comment at: clang/docs/analyzer/developer-docs.rst:14
developer-docs/RegionStore
-   
\ No newline at end of file
+   developer-docs/ContainerModeling.rst
+   

How come this new addition is a `.rst` but the rest isn't. Does the rendering 
work without it? We should keep a style here, either way. So turn all into 
proper `.rst` links, or drop the `.rst` here.



Comment at: clang/docs/analyzer/developer-docs/ContainerModeling.rst:5
+
+The goal of checker ``alpha.cplusplus.ContainerModeling`` is to provide a
+symbolic abstraction for containers to the Clang Static Analyzer. There are

Can the checker's name be turned into a link, or am I mixing up individual 
checker documentations' availability with Tidy?

Either way, show the check's name in bold face.



Comment at: clang/docs/analyzer/developer-docs/ContainerModeling.rst:6
+The goal of checker ``alpha.cplusplus.ContainerModeling`` is to provide a
+symbolic abstraction for containers to the Clang Static Analyzer. There are
+various concepts regarding containers that help formulate static analysis





Comment at: clang/docs/analyzer/developer-docs/ContainerModeling.rst:8
+various concepts regarding containers that help formulate static analysis
+problems more concisely. The size of the container, whether is empty or not are
+the most trivial motivating examples. Standard containers can be iterated, and





Comment at: clang/docs/analyzer/developer-docs/ContainerModeling.rst:11
+this idiom is well-adopted in case of non-standard container implementations as
+well, because it can be used to provide a compatible interface to algorithms.
+Therefore iterator modeling is closely related to containers. Iterators extend

what does `it` refer to here?



Comment at: clang/docs/analyzer/developer-docs/ContainerModeling.rst:21
+the modeling checkers mentioned:
+  * :ref:`alpha-cplusplus-InvalidatedIterator`
+  * :ref:`alpha-cplusplus-IteratorRange`

Will this appear to the reader as `alpha-` or `alpha.`?



Comment at: clang/docs/analyzer/developer-docs/ContainerModeling.rst:29
+
+According to ContainerModeling, a value ``c`` with type ``C`` is considered a
+container if either of the following holds:





Comment at: clang/docs/analyzer/developer-docs/ContainerModeling.rst:35
+This should be detected by type C having member functions ``T C::begin()``
+and ``T C::end()``, where T is an `iterator
+`_.





Comment at: clang/docs/analyzer/developer-docs/ContainerModeling.rst:35-36
+This should be detected by type C having member functions ``T C::begin()``
+and ``T C::end()``, where T is an `iterator
+`_.
+  * The expression ``begin(c)`` and ``end(c)`` are both valid expressions in a

whisperity wrote:
> 
Iterator was linked already, I don't think this needs to repeat.



Comment at: clang/docs/analyzer/developer-docs/ContainerModeling.rst:37-47
+  * The expression ``begin(c)`` and ``end(c)`` are both valid expressions in a
+given scope, and return an `iterator
+`_.
+This should be detected by checking the existence functions with the
+corresponding names, and can either be user-defined free functions or
+template specialization of the standard-defined ``template
+constexpr auto std::begin(T& t) -> decltype(t.begin())`` and

What about [[ https://en.cppreference.com/w/cpp/algorithm/ranges/for_each | 
neibloids (see right before Parameters) ]]  (callables that are explicitly 
disabling ADL at a call site even though they look like an ADL call, with 
tricks, usually found in the `ranges` library).



Comment at: clang/docs/analyzer/developer-docs/ContainerModeling.rst:40
+`_.
+This should be detected by checking the existence functions with the
+corresponding names, and can either be user-defined free functions or
-

[PATCH] D91948: [WIP][analyzer][doc] Add Container- and IteratorModeling developer docs

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

Actually, while the explanation is understandable for me with additional 
knowledge about the representation... I think it would be useful to add the 
most simple example from the iterator checkers to the end of the document, how 
this whole thing ties together and how it is useful in a checker.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91948

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

[PATCH] D91656: [clang-tidy] add concurrency module

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added a comment.

jdoerfert, alexfh, hokein, aaron.ballman,

Any comments on the patch?


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

https://reviews.llvm.org/D91656

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

[PATCH] D83694: [clang][cli] Port DependencyOutput option flags to new option parsing system

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 307034.
jansvoboda11 added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Reduce template instantiations, remove names of unused parameters


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83694

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/Option/OptParser.td

Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -170,7 +170,7 @@
 
 class MarshallingInfoBitfieldFlag
   : MarshallingInfoFlag {
-  code Normalizer = "(normalizeFlagToValue)";
+  code Normalizer = "makeFlagToValueNormalizer("#value#")";
   code ValueMerger = "mergeMaskValue";
   code ValueExtractor = "(extractMaskValue)";
 }
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -134,20 +134,45 @@
   return None;
 }
 
-void denormalizeSimpleFlag(SmallVectorImpl &Args,
-   const char *Spelling,
-   CompilerInvocation::StringAllocator SA,
-   unsigned TableIndex, unsigned Value) {
+/// The tblgen-erated code passes in a fifth parameter of an arbitrary type, but
+/// denormalizeSimpleFlags never looks at it. Avoid bloating compile-time with
+/// unnecessary template instantiations and just ignore it with a variadic
+/// argument.
+static void denormalizeSimpleFlag(SmallVectorImpl &Args,
+  const char *Spelling,
+  CompilerInvocation::StringAllocator, unsigned,
+  /*T*/...) {
   Args.push_back(Spelling);
 }
 
-template 
-static llvm::Optional
-normalizeFlagToValue(OptSpecifier Opt, unsigned TableIndex, const ArgList &Args,
- DiagnosticsEngine &Diags) {
-  if (Args.hasArg(Opt))
-return Value;
-  return None;
+namespace {
+template  struct FlagToValueNormalizer {
+  T Value;
+
+  Optional operator()(OptSpecifier Opt, unsigned, const ArgList &Args,
+ DiagnosticsEngine &) {
+if (Args.hasArg(Opt))
+  return Value;
+return None;
+  }
+};
+} // namespace
+
+template 
+static constexpr bool is_int_convertible() {
+  return sizeof(T) <= sizeof(std::uint64_t) &&
+ std::is_trivially_constructible::value &&
+ std::is_trivially_constructible::value;
+}
+
+template (), bool> = false>
+static FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
+  return FlagToValueNormalizer{Value};
+}
+
+template (), bool> = false>
+static FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
+  return FlagToValueNormalizer{std::move(Value)};
 }
 
 static Optional normalizeBooleanFlag(OptSpecifier PosOpt,
@@ -1552,13 +1577,8 @@
   ArgList &Args) {
   Opts.OutputFile = std::string(Args.getLastArgValue(OPT_dependency_file));
   Opts.Targets = Args.getAllArgValues(OPT_MT);
-  Opts.IncludeSystemHeaders = Args.hasArg(OPT_sys_header_deps);
-  Opts.IncludeModuleFiles = Args.hasArg(OPT_module_file_deps);
-  Opts.UsePhonyTargets = Args.hasArg(OPT_MP);
-  Opts.ShowHeaderIncludes = Args.hasArg(OPT_H);
   Opts.HeaderIncludeOutputFile =
   std::string(Args.getLastArgValue(OPT_header_include_file));
-  Opts.AddMissingHeaderDeps = Args.hasArg(OPT_MG);
   if (Args.hasArg(OPT_show_includes)) {
 // Writing both /showIncludes and preprocessor output to stdout
 // would produce interleaved output, so use stderr for /showIncludes.
@@ -1573,8 +1593,6 @@
   Opts.DOTOutputFile = std::string(Args.getLastArgValue(OPT_dependency_dot));
   Opts.ModuleDependencyOutputDir =
   std::string(Args.getLastArgValue(OPT_module_dependency_dir));
-  if (Args.hasArg(OPT_MV))
-Opts.OutputFormat = DependencyOutputFormat::NMake;
   // Add sanitizer blacklists as extra dependencies.
   // They won't be discovered by the regular preprocessor, so
   // we let make / ninja to know about this implicit dependency.
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -429,7 +429,8 @@
 "into small data section (MIPS / Hexagon)">;
 def G_EQ : Joined<["-"], "G=">, Flags<[NoXarchOption]>, Group, Alias;
 def H : Flag<["-"], "H">, Flags<[CC1Option]>, Group,
-HelpText<"Show header includes and nesting depth">;
+HelpText<"Show header includes and nesting depth">,
+MarshallingInfoFlag<"DependencyOutputOpts.ShowHeaderIncludes">;
 def I_ : Flag<["-"], "I-">, Group,
 HelpText<"Restrict all prior -I flags to double-quoted inclusion and "
  

[PATCH] D83694: [clang][cli] Port DependencyOutput option flags to new option parsing system

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

Addressed review feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83694

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

[PATCH] D91656: [clang-tidy] add concurrency module

[Roman Lebedev via Phabricator via cfe-commits]

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

LGTM




Comment at: clang-tools-extra/clang-tidy/CMakeLists.txt:57-58
 add_subdirectory(bugprone)
 add_subdirectory(cert)
 add_subdirectory(cppcoreguidelines)
 add_subdirectory(darwin)





Comment at: clang-tools-extra/clang-tidy/CMakeLists.txt:75-77
 add_subdirectory(readability)
+add_subdirectory(concurrency)
 add_subdirectory(zircon)





Comment at: clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt:2
+set(LLVM_LINK_COMPONENTS
+  FrontendOpenMP
+  Support

Is this really nessesary?


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

https://reviews.llvm.org/D91656

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

[clang-tools-extra] 61e538b - Revert "[clangd] testPath's final result agrees with the passed in Style"

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-11-23T13:12:59+01:00
New Revision: 61e538b15ddb2de9250277d151f0f655c2220d9b

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

LOG: Revert "[clangd] testPath's final result agrees with the passed in Style"

This reverts commit 8cec8de2a4e6692da6226bb02cf417eb0e50adde as it
breaks windows buildbots.

Added: 


Modified: 
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/TestFS.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp 
b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
index 2b4605eb97e2..a2423094b17a 100644
--- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -255,6 +255,7 @@ TEST_F(ConfigCompileTests, ExternalBlockMountPoint) {
   };
 
   auto BarPath = testPath("foo/bar.h", llvm::sys::path::Style::posix);
+  BarPath = llvm::sys::path::convert_to_slash(BarPath);
   Parm.Path = BarPath;
   // Non-absolute MountPoint without a directory raises an error.
   Frag = GetFrag("", "foo");
@@ -268,6 +269,7 @@ TEST_F(ConfigCompileTests, ExternalBlockMountPoint) {
   ASSERT_FALSE(Conf.Index.External);
 
   auto FooPath = testPath("foo/", llvm::sys::path::Style::posix);
+  FooPath = llvm::sys::path::convert_to_slash(FooPath);
   // Ok when relative.
   Frag = GetFrag(testRoot(), "foo/");
   compileAndApply();
@@ -291,6 +293,7 @@ TEST_F(ConfigCompileTests, ExternalBlockMountPoint) {
 
   // File outside MountPoint, no index.
   auto BazPath = testPath("bar/baz.h", llvm::sys::path::Style::posix);
+  BazPath = llvm::sys::path::convert_to_slash(BazPath);
   Parm.Path = BazPath;
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
@@ -299,6 +302,7 @@ TEST_F(ConfigCompileTests, ExternalBlockMountPoint) {
 
   // File under MountPoint, index should be set.
   BazPath = testPath("foo/baz.h", llvm::sys::path::Style::posix);
+  BazPath = llvm::sys::path::convert_to_slash(BazPath);
   Parm.Path = BazPath;
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();

diff  --git a/clang-tools-extra/clangd/unittests/TestFS.cpp 
b/clang-tools-extra/clangd/unittests/TestFS.cpp
index f343a85331d0..ba4010cb4581 100644
--- a/clang-tools-extra/clangd/unittests/TestFS.cpp
+++ b/clang-tools-extra/clangd/unittests/TestFS.cpp
@@ -80,10 +80,12 @@ const char *testRoot() {
 }
 
 std::string testPath(PathRef File, llvm::sys::path::Style Style) {
-  assert(llvm::sys::path::is_relative(File, Style));
+  assert(llvm::sys::path::is_relative(File) && "FileName should be relative");
+
+  llvm::SmallString<32> NativeFile = File;
+  llvm::sys::path::native(NativeFile, Style);
   llvm::SmallString<32> Path;
-  llvm::sys::path::append(Path, testRoot(), File);
-  llvm::sys::path::native(Path, Style);
+  llvm::sys::path::append(Path, Style, testRoot(), NativeFile);
   return std::string(Path.str());
 }
 



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

[PATCH] D83694: [clang][cli] Port DependencyOutput option flags to new option parsing system

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 307037.
jansvoboda11 added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83694

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/Option/OptParser.td

Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -170,7 +170,7 @@
 
 class MarshallingInfoBitfieldFlag
   : MarshallingInfoFlag {
-  code Normalizer = "(normalizeFlagToValue)";
+  code Normalizer = "makeFlagToValueNormalizer("#value#")";
   code ValueMerger = "mergeMaskValue";
   code ValueExtractor = "(extractMaskValue)";
 }
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -134,20 +134,45 @@
   return None;
 }
 
-void denormalizeSimpleFlag(SmallVectorImpl &Args,
-   const char *Spelling,
-   CompilerInvocation::StringAllocator SA,
-   unsigned TableIndex, unsigned Value) {
+/// The tblgen-erated code passes in a fifth parameter of an arbitrary type, but
+/// denormalizeSimpleFlags never looks at it. Avoid bloating compile-time with
+/// unnecessary template instantiations and just ignore it with a variadic
+/// argument.
+static void denormalizeSimpleFlag(SmallVectorImpl &Args,
+  const char *Spelling,
+  CompilerInvocation::StringAllocator, unsigned,
+  /*T*/...) {
   Args.push_back(Spelling);
 }
 
-template 
-static llvm::Optional
-normalizeFlagToValue(OptSpecifier Opt, unsigned TableIndex, const ArgList &Args,
- DiagnosticsEngine &Diags) {
-  if (Args.hasArg(Opt))
-return Value;
-  return None;
+namespace {
+template  struct FlagToValueNormalizer {
+  T Value;
+
+  Optional operator()(OptSpecifier Opt, unsigned, const ArgList &Args,
+ DiagnosticsEngine &) {
+if (Args.hasArg(Opt))
+  return Value;
+return None;
+  }
+};
+} // namespace
+
+template 
+static constexpr bool is_int_convertible() {
+  return sizeof(T) <= sizeof(std::uint64_t) &&
+ std::is_trivially_constructible::value &&
+ std::is_trivially_constructible::value;
+}
+
+template (), bool> = false>
+static FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
+  return FlagToValueNormalizer{Value};
+}
+
+template (), bool> = false>
+static FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
+  return FlagToValueNormalizer{std::move(Value)};
 }
 
 static Optional normalizeBooleanFlag(OptSpecifier PosOpt,
@@ -1552,13 +1577,8 @@
   ArgList &Args) {
   Opts.OutputFile = std::string(Args.getLastArgValue(OPT_dependency_file));
   Opts.Targets = Args.getAllArgValues(OPT_MT);
-  Opts.IncludeSystemHeaders = Args.hasArg(OPT_sys_header_deps);
-  Opts.IncludeModuleFiles = Args.hasArg(OPT_module_file_deps);
-  Opts.UsePhonyTargets = Args.hasArg(OPT_MP);
-  Opts.ShowHeaderIncludes = Args.hasArg(OPT_H);
   Opts.HeaderIncludeOutputFile =
   std::string(Args.getLastArgValue(OPT_header_include_file));
-  Opts.AddMissingHeaderDeps = Args.hasArg(OPT_MG);
   if (Args.hasArg(OPT_show_includes)) {
 // Writing both /showIncludes and preprocessor output to stdout
 // would produce interleaved output, so use stderr for /showIncludes.
@@ -1573,8 +1593,6 @@
   Opts.DOTOutputFile = std::string(Args.getLastArgValue(OPT_dependency_dot));
   Opts.ModuleDependencyOutputDir =
   std::string(Args.getLastArgValue(OPT_module_dependency_dir));
-  if (Args.hasArg(OPT_MV))
-Opts.OutputFormat = DependencyOutputFormat::NMake;
   // Add sanitizer blacklists as extra dependencies.
   // They won't be discovered by the regular preprocessor, so
   // we let make / ninja to know about this implicit dependency.
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -429,7 +429,8 @@
 "into small data section (MIPS / Hexagon)">;
 def G_EQ : Joined<["-"], "G=">, Flags<[NoXarchOption]>, Group, Alias;
 def H : Flag<["-"], "H">, Flags<[CC1Option]>, Group,
-HelpText<"Show header includes and nesting depth">;
+HelpText<"Show header includes and nesting depth">,
+MarshallingInfoFlag<"DependencyOutputOpts.ShowHeaderIncludes">;
 def I_ : Flag<["-"], "I-">, Group,
 HelpText<"Restrict all prior -I flags to double-quoted inclusion and "
  "remove current directory from include path">;
@@ -455,17 +456,21 @@
 HelpText<"Write depfile output from -MMD, -MD, -MM, 

[PATCH] D91656: [clang-tidy] add concurrency module

[Vasily Kulikov via Phabricator via cfe-commits]

segoon updated this revision to Diff 307039.
segoon added a comment.

- sort order
- do not link with FrontendOpenMP


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

https://reviews.llvm.org/D91656

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt
  clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/index.rst

Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -64,6 +64,8 @@
 ``bugprone-``  Checks that target bugprone code constructs.
 ``cert-``  Checks related to CERT Secure Coding Guidelines.
 ``clang-analyzer-``Clang Static Analyzer checks.
+``concurrency-``   Checks related to concurrent programming (including
+   threads, fibers, coroutines, etc.).
 ``cppcoreguidelines-`` Checks related to C++ Core Guidelines.
 ``darwin-``Checks related to Darwin coding conventions.
 ``fuchsia-``   Checks related to Fuchsia coding conventions.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -82,6 +82,11 @@
   `Altera SDK for OpenCL: Best Practices Guide
   `_.
 
+- New ``concurrency`` module.
+
+  Includes checks related to concurrent programming (e.g. threads, fibers,
+  coroutines, etc.).
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/concurrency/ConcurrencyTidyModule.cpp
@@ -0,0 +1,33 @@
+//===--- ConcurrencyTidyModule.cpp - clang-tidy ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "../ClangTidy.h"
+#include "../ClangTidyModule.h"
+#include "../ClangTidyModuleRegistry.h"
+
+namespace clang {
+namespace tidy {
+namespace concurrency {
+
+class ConcurrencyModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {}
+};
+
+} // namespace concurrency
+
+// Register the ConcurrencyTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add
+X("concurrency-module", "Adds concurrency checks.");
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the ConcurrencyModule.
+volatile int ConcurrencyModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/concurrency/CMakeLists.txt
@@ -0,0 +1,22 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_clang_library(clangTidyConcurrencyModule
+  ConcurrencyTidyModule.cpp
+
+  LINK_LIBS
+  clangTidy
+  clangTidyUtils
+  )
+
+clang_target_link_libraries(clangTidyConcurrencyModule
+  PRIVATE
+  clangAnalysis
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangLex
+  clangSerialization
+  clangTooling
+  )
Index: clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
===
--- clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
+++ clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
@@ -45,6 +45,11 @@
 static int LLVM_ATTRIBUTE_UNUSED CERTModuleAnchorDestination =
 CERTModuleAnchorSource;
 
+// This anchor is used to force the linker to link the ConcurrencyModule.
+extern volatile int ConcurrencyModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED ConcurrencyModuleAnchorDestination =
+ConcurrencyModuleAnchorSource;
+
 // This anchor is used to force the linker to link the CppCoreGuidelinesModule.
 extern volatile int CppCoreGuidelinesModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED CppCoreGuidelinesModuleAnchorDestination =
Index: clang-tools-extra/clang-tidy/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/CMakeLists.txt
@@ -53,6 +53,7 @@
 add_subdirectory(abseil)
 add_subdirectory(altera)
 add_subdirectory(boost)
+add_subdirectory(concurrency)
 add_subdirectory(bugprone)
 add_subdirectory(cert)
 add_subdirectory(cppco

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

[Chirag Khandelwal via Phabricator via cfe-commits]

AMDChirag added inline comments.



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:3576
+} else {
+  auto SectionCB = [this, CS](InsertPointTy AllocaIP,
+  InsertPointTy CodeGenIP,

Meinersbur wrote:
> In what situation would there be no `#pragma omp section` children?
You are correct - I could not find a scenario where `CS` would be a `nullptr`.
However, the original implementation `EmitSections()` handles the `CS == 
nullptr` case as well. Also, if `CS` is `nullptr`, the software will exit with 
an assertion failure anyway (`CodeGenFunction::EmitStmt()`).
Should I just remove the `else` block altogether? I think an `assert` statement 
instead, for checking `CS == nullptr` should suffice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91054

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

[PATCH] D83697: [clang][cli] Port Frontend option flags to new option parsing system

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 307045.
jansvoboda11 added a comment.

Drop namespace specifier & names of unused args, add tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83697

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td

Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -187,7 +187,7 @@
 // Mixins for additional marshalling attributes.
 
 class IsNegative {
-  // todo: create & apply a normalizer for negative flags
+  code Normalizer = "normalizeSimpleNegativeFlag";
 }
 class AlwaysEmit { bit ShouldAlwaysEmit = true; }
 class Normalizer { code Normalizer = normalizer; }
Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -22,6 +22,13 @@
 using ::testing::StrNe;
 
 namespace {
+struct OptsPopulationTest : public ::testing::Test {
+  IntrusiveRefCntPtr Diags;
+  CompilerInvocation CInvok;
+
+  OptsPopulationTest()
+  : Diags(CompilerInstance::createDiagnostics(new DiagnosticOptions())) {}
+};
 
 class CC1CommandLineGenerationTest : public ::testing::Test {
 public:
@@ -37,23 +44,36 @@
   : Diags(CompilerInstance::createDiagnostics(new DiagnosticOptions())) {}
 };
 
-TEST(OptsPopulationTest, CanPopulateOptsWithImpliedFlags) {
-  const char *Args[] = {"clang", "-xc++", "-cl-unsafe-math-optimizations"};
+TEST_F(OptsPopulationTest, OptIsInitializedWithCustomDefaultValue) {
+  const char *Args[] = {"clang", "-xc++"};
 
-  auto Diags = CompilerInstance::createDiagnostics(new DiagnosticOptions());
+  CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
+
+  ASSERT_TRUE(CInvok.getFrontendOpts().UseTemporary);
+}
+
+TEST_F(OptsPopulationTest, OptOfNegativeFlagIsPopulatedWithFalse) {
+  const char *Args[] = {"clang", "-xc++", "-fno-temp-file"};
+
+  CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
+
+  ASSERT_FALSE(CInvok.getFrontendOpts().UseTemporary);
+}
+
+TEST_F(OptsPopulationTest, OptsOfImpliedPositiveFlagArePopulatedWithTrue) {
+  const char *Args[] = {"clang", "-xc++", "-cl-unsafe-math-optimizations"};
 
-  CompilerInvocation CInvok;
   CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
 
   // Explicitly provided flag.
-  ASSERT_EQ(CInvok.getLangOpts()->CLUnsafeMath, true);
+  ASSERT_TRUE(CInvok.getLangOpts()->CLUnsafeMath);
 
   // Flags directly implied by explicitly provided flag.
-  ASSERT_EQ(CInvok.getCodeGenOpts().LessPreciseFPMAD, true);
-  ASSERT_EQ(CInvok.getLangOpts()->UnsafeFPMath, true);
+  ASSERT_TRUE(CInvok.getCodeGenOpts().LessPreciseFPMAD);
+  ASSERT_TRUE(CInvok.getLangOpts()->UnsafeFPMath);
 
   // Flag transitively implied by explicitly provided flag.
-  ASSERT_EQ(CInvok.getLangOpts()->AllowRecip, true);
+  ASSERT_TRUE(CInvok.getLangOpts()->AllowRecip);
 }
 
 TEST_F(CC1CommandLineGenerationTest, CanGenerateCC1CommandLineFlag) {
@@ -134,6 +154,28 @@
   ASSERT_THAT(GeneratedArgs, Each(StrNe(RelocationModelCStr)));
 }
 
+TEST_F(CC1CommandLineGenerationTest, NotPresentNegativeFlagNotGenerated) {
+  const char *Args[] = {"clang", "-xc++"};
+
+  CompilerInvocation CInvok;
+  CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
+
+  CInvok.generateCC1CommandLine(GeneratedArgs, *this);
+
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fno-temp-file";
+}
+
+TEST_F(CC1CommandLineGenerationTest, PresentNegativeFlagGenerated) {
+  const char *Args[] = {"clang", "-xc++", "-fno-temp-file"};
+
+  CompilerInvocation CInvok;
+  CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
+
+  CInvok.generateCC1CommandLine(GeneratedArgs, *this);
+
+  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fno-temp-file")));
+}
+
 TEST_F(CC1CommandLineGenerationTest, NotPresentAndNotImpliedNotGenerated) {
   const char *Args[] = {"clang", "-xc++"};
 
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -134,6 +134,14 @@
   return None;
 }
 
+static Optional normalizeSimpleNegativeFlag(OptSpecifier Opt, unsigned,
+  const ArgList &Args,
+  DiagnosticsEngine &) {
+  if (Args.hasArg(Opt))
+return false;
+  return None;
+}
+
 void denormalizeSimpleFlag(SmallVectorImpl &Args,
const char *Spelling,
CompilerInvocation::StringAllocator SA,
@@ -276,9 +284,11 @@
   LangOptions &LangOpts = *Invocation.getLangOpt

[PATCH] D91656: [clang-tidy] add concurrency module

[Vasily Kulikov via Phabricator via cfe-commits]

segoon added a comment.

In D91656#2411072 , @lebedev.ri wrote:

> LGTM

thanks for comments, fixed


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

https://reviews.llvm.org/D91656

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

[clang-tools-extra] b31486a - [clangd] textDocument/implementation (LSP layer)

[Utkarsh Saxena via cfe-commits]

Author: Utkarsh Saxena
Date: 2020-11-23T13:50:44+01:00
New Revision: b31486ad971774c859e3e031fc0d8d9b77e3b083

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

LOG: [clangd] textDocument/implementation (LSP layer)

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

Added: 
clang-tools-extra/clangd/test/implementations.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/test/initialize-params.test

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index e726271fe7cbe..335a6fc9ad94e 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -604,6 +604,7 @@ void ClangdLSPServer::onInitialize(const InitializeParams 
&Params,
  }},
 {"declarationProvider", true},
 {"definitionProvider", true},
+{"implementationProvider", true},
 {"documentHighlightProvider", true},
 {"documentLinkProvider",
  llvm::json::Object{
@@ -1291,6 +1292,22 @@ void ClangdLSPServer::onReference(const ReferenceParams 
&Params,
  });
 }
 
+void ClangdLSPServer::onGoToImplementation(
+const TextDocumentPositionParams &Params,
+Callback> Reply) {
+  Server->findImplementations(
+  Params.textDocument.uri.file(), Params.position,
+  [Reply = std::move(Reply)](
+  llvm::Expected> Overrides) mutable {
+if (!Overrides)
+  return Reply(Overrides.takeError());
+std::vector Impls;
+for (const LocatedSymbol &Sym : *Overrides)
+  Impls.push_back(Sym.PreferredDeclaration);
+return Reply(std::move(Impls));
+  });
+}
+
 void ClangdLSPServer::onSymbolInfo(const TextDocumentPositionParams &Params,
Callback> Reply) 
{
   Server->symbolInfo(Params.textDocument.uri.file(), Params.position,
@@ -1431,6 +1448,7 @@ ClangdLSPServer::ClangdLSPServer(class Transport &Transp,
   MsgHandler->bind("textDocument/signatureHelp", 
&ClangdLSPServer::onSignatureHelp);
   MsgHandler->bind("textDocument/definition", 
&ClangdLSPServer::onGoToDefinition);
   MsgHandler->bind("textDocument/declaration", 
&ClangdLSPServer::onGoToDeclaration);
+  MsgHandler->bind("textDocument/implementation", 
&ClangdLSPServer::onGoToImplementation);
   MsgHandler->bind("textDocument/references", &ClangdLSPServer::onReference);
   MsgHandler->bind("textDocument/switchSourceHeader", 
&ClangdLSPServer::onSwitchSourceHeader);
   MsgHandler->bind("textDocument/prepareRename", 
&ClangdLSPServer::onPrepareRename);

diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.h 
b/clang-tools-extra/clangd/ClangdLSPServer.h
index b9200f6a2e1b8..4d568bc13d8bf 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.h
+++ b/clang-tools-extra/clangd/ClangdLSPServer.h
@@ -115,6 +115,8 @@ class ClangdLSPServer : private ClangdServer::Callbacks {
  Callback>);
   void onGoToDefinition(const TextDocumentPositionParams &,
 Callback>);
+  void onGoToImplementation(const TextDocumentPositionParams &,
+Callback>);
   void onReference(const ReferenceParams &, Callback>);
   void onSwitchSourceHeader(const TextDocumentIdentifier &,
 Callback>);

diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index b6f9fcfd23da8..889d2cbcf2807 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -718,6 +718,18 @@ void ClangdServer::foldingRanges(llvm::StringRef File,
TUScheduler::InvalidateOnUpdate);
 }
 
+void ClangdServer::findImplementations(
+PathRef File, Position Pos, Callback> CB) {
+  auto Action = [Pos, CB = std::move(CB),
+ this](llvm::Expected InpAST) mutable {
+if (!InpAST)
+  return CB(InpAST.takeError());
+CB(clangd::findImplementations(InpAST->AST, Pos, Index));
+  };
+
+  WorkScheduler.runWithAST("Implementations", File, std::move(Action));
+}
+
 void ClangdServer::findReferences(PathRef File, Position Pos, uint32_t Limit,
   Callback CB) {
   auto Action = [Pos, Limit, CB = std::move(CB),

diff  --git a/clang-tools-extra/clangd/ClangdServer.h 
b/clang-tools-extra/clangd/ClangdServer.h
index 0056f5072cca3..1ccb4c5899f81 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -253,6 +253,10 @@ class ClangdServer {
   /// Retrieve ranges that

[PATCH] D91721: [clangd] textDocument/implementation (LSP layer)

[Utkarsh Saxena via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb31486ad9717: [clangd] textDocument/implementation (LSP 
layer) (authored by usaxena95).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91721

Files:
  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/test/implementations.test
  clang-tools-extra/clangd/test/initialize-params.test

Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -67,7 +67,8 @@
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "hoverProvider": true,
-# CHECK-NEXT:  "memoryUsageProvider": true
+# CHECK-NEXT:  "implementationProvider": true,
+# CHECK-NEXT:  "memoryUsageProvider": true,
 # CHECK-NEXT:  "referencesProvider": true,
 # CHECK-NEXT:  "renameProvider": true,
 # CHECK-NEXT:  "selectionRangeProvider": true,
Index: clang-tools-extra/clangd/test/implementations.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/implementations.test
@@ -0,0 +1,39 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"struct Parent { virtual void Foo(); };\nstruct Child1 : Parent { void Foo() override(); };\nstruct Child2 : Parent { void Foo() override(); };"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/implementation","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":32}}}
+#  CHECK:  "id": 1
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 33,
+# CHECK-NEXT:  "line": 1
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 30,
+# CHECK-NEXT:  "line": 1
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "uri": "file://{{.*}}/clangd-test/main.cpp"
+# CHECK-NEXT:},
+# CHECK-NEXT:{
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 33,
+# CHECK-NEXT:  "line": 2
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 30,
+# CHECK-NEXT:  "line": 2
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:"uri": "file://{{.*}}/clangd-test/main.cpp"
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -253,6 +253,10 @@
   /// Retrieve ranges that can be used to fold code within the specified file.
   void foldingRanges(StringRef File, Callback> CB);
 
+  /// Retrieve implementations for virtual method.
+  void findImplementations(PathRef File, Position Pos,
+   Callback> CB);
+
   /// Retrieve locations for symbol references.
   void findReferences(PathRef File, Position Pos, uint32_t Limit,
   Callback CB);
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -718,6 +718,18 @@
TUScheduler::InvalidateOnUpdate);
 }
 
+void ClangdServer::findImplementations(
+PathRef File, Position Pos, Callback> CB) {
+  auto Action = [Pos, CB = std::move(CB),
+ this](llvm::Expected InpAST) mutable {
+if (!InpAST)
+  return CB(InpAST.takeError());
+CB(clangd::findImplementations(InpAST->AST, Pos, Index));
+  };
+
+  WorkScheduler.runWithAST("Implementations", File, std::move(Action));
+}
+
 void ClangdServer::findReferences(PathRef File, Position Pos, uint32_t Limit,
   Callback CB) {
   auto Action = [Pos, Limit, CB = std::move(CB),
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -115,6 +115,8 @@
  Callback>);
   void 

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91828

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

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

[Jann Horn via Phabricator via cfe-commits]

thejh added a comment.

@aaron.ballman Can you land it for me? I don't have commit access.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91828

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

[PATCH] D91948: [WIP][analyzer][doc] Add Container- and IteratorModeling developer docs

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

Szelethus added a comment.

I don't have the time to comb through this doc, unfortunately, but I want to 
applaud this effort to make the iterator checker family more accessible. Its 
certainly a forerunner in modeling tricky C++ constructs, and I can't wait to 
be a more valuable reviewer after being a bit more knowledgeable about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91948

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

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D91828#2411203 , @thejh wrote:

> @aaron.ballman Can you land it for me? I don't have commit access.

Happy to do so -- are you okay with "Jann Horn " for author 
attribution?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91828

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

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

[Jann Horn via Phabricator via cfe-commits]

thejh added a comment.

In D91828#2411207 , @aaron.ballman 
wrote:

> In D91828#2411203 , @thejh wrote:
>
>> @aaron.ballman Can you land it for me? I don't have commit access.
>
> Happy to do so -- are you okay with "Jann Horn " for author 
> attribution?

Yes, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91828

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

[clang] 00dad9d - Ignore noderef attribute in unevaluated context

[Aaron Ballman via cfe-commits]

Author: Jann Horn
Date: 2020-11-23T08:10:35-05:00
New Revision: 00dad9d028ce31739b992a3ce2df5de054a9fa3c

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

LOG: Ignore noderef attribute in unevaluated context

The noderef attribute is for catching code that accesses pointers in
a different address space. Unevaluated code is always safe in that regard.

Added: 


Modified: 
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprMember.cpp
clang/test/Frontend/noderef.c
clang/test/Frontend/noderef.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 60a685bfdf15..5580cdf13691 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -4751,6 +4751,9 @@ void Sema::CheckAddressOfNoDeref(const Expr *E) {
 }
 
 void Sema::CheckSubscriptAccessOfNoDeref(const ArraySubscriptExpr *E) {
+  if (isUnevaluatedContext())
+return;
+
   QualType ResultTy = E->getType();
   ExpressionEvaluationContextRecord &LastRecord = ExprEvalContexts.back();
 
@@ -14666,7 +14669,8 @@ ExprResult Sema::CreateBuiltinUnaryOp(SourceLocation 
OpLoc,
 OpLoc, CanOverflow, CurFPFeatureOverrides());
 
   if (Opc == UO_Deref && UO->getType()->hasAttr(attr::NoDeref) &&
-  !isa(UO->getType().getDesugaredType(Context)))
+  !isa(UO->getType().getDesugaredType(Context)) &&
+  !isUnevaluatedContext())
 ExprEvalContexts.back().PossibleDerefs.insert(UO);
 
   // Convert the result back to a half vector.

diff  --git a/clang/lib/Sema/SemaExprMember.cpp 
b/clang/lib/Sema/SemaExprMember.cpp
index 93ed756e084b..23cfae81df46 100644
--- a/clang/lib/Sema/SemaExprMember.cpp
+++ b/clang/lib/Sema/SemaExprMember.cpp
@@ -1734,6 +1734,9 @@ ExprResult Sema::ActOnMemberAccessExpr(Scope *S, Expr 
*Base,
 }
 
 void Sema::CheckMemberAccessOfNoDeref(const MemberExpr *E) {
+  if (isUnevaluatedContext())
+return;
+
   QualType ResultTy = E->getType();
 
   // Do not warn on member accesses to arrays since this returns an array

diff  --git a/clang/test/Frontend/noderef.c b/clang/test/Frontend/noderef.c
index b072b995fcf6..3388f2a39992 100644
--- a/clang/test/Frontend/noderef.c
+++ b/clang/test/Frontend/noderef.c
@@ -57,12 +57,19 @@ int test() {
   p = &*(p + 1);
 
   // Struct member access
-  struct S NODEREF *s;  // expected-note 2 {{s declared here}}
+  struct S NODEREF *s; // expected-note 3 {{s declared here}}
   x = s->a;   // expected-warning{{dereferencing s; was declared with a 
'noderef' type}}
   x = (*s).b; // expected-warning{{dereferencing s; was declared with a 
'noderef' type}}
   p = &s->a;
   p = &(*s).b;
 
+  // Most things in sizeof() can't actually access memory
+  x = sizeof(s->a);  // ok
+  x = sizeof(*s);// ok
+  x = sizeof(s[0]);  // ok
+  x = sizeof(s->a + (s->b)); // ok
+  x = sizeof(int[++s->a]);   // expected-warning{{dereferencing s; was 
declared with a 'noderef' type}}
+
   // Nested struct access
   struct S2 NODEREF *s2_noderef;// expected-note 5 {{s2_noderef declared 
here}}
   p = s2_noderef->a;  // ok since result is an array in a struct

diff  --git a/clang/test/Frontend/noderef.cpp b/clang/test/Frontend/noderef.cpp
index 32d5ca34d1b1..68342a8e6467 100644
--- a/clang/test/Frontend/noderef.cpp
+++ b/clang/test/Frontend/noderef.cpp
@@ -6,6 +6,11 @@
 
 #define NODEREF __attribute__((noderef))
 
+// Stub out types for 'typeid' to work.
+namespace std {
+class type_info {};
+} // namespace std
+
 void Normal() {
   int NODEREF i;// expected-warning{{'noderef' can only be used on an 
array or pointer type}}
   int NODEREF *i_ptr;   // expected-note 2 {{i_ptr declared here}}
@@ -102,6 +107,18 @@ int ChildCall(NODEREF Child *child) { // 
expected-note{{child declared here}}
   return child->func();   // expected-warning{{dereferencing 
child; was declared with a 'noderef' type}}
 }
 
+std::type_info TypeIdPolymorphic(NODEREF A *a) { // expected-note{{a declared 
here}}
+  return typeid(*a); // 
expected-warning{{dereferencing a; was declared with a 'noderef' type}}
+}
+
+class SimpleClass {
+  int a;
+};
+
+std::type_info TypeIdNonPolymorphic(NODEREF SimpleClass *simple) {
+  return typeid(*simple);
+}
+
 template 
 class B {
   Ty NODEREF *member;



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

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D91828#2411210 , @thejh wrote:

> In D91828#2411207 , @aaron.ballman 
> wrote:
>
>> In D91828#2411203 , @thejh wrote:
>>
>>> @aaron.ballman Can you land it for me? I don't have commit access.
>>
>> Happy to do so -- are you okay with "Jann Horn " for 
>> author attribution?
>
> Yes, thanks.

Thank you for the patch! I've committed the changes for you in 
00dad9d028ce31739b992a3ce2df5de054a9fa3c 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91828

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

[PATCH] D91948: [WIP][analyzer][doc] Add Container- and IteratorModeling developer docs

[Daniel Krupp via Phabricator via cfe-commits]

dkrupp added a comment.

In D91948#2411058 , @whisperity wrote:

> Actually, while the explanation is understandable for me with additional 
> knowledge about the representation... I think it would be useful to add the 
> most simple example from the iterator checkers to the end of the document, 
> how this whole thing ties together and how it is useful in a checker.

I cannot agree more. Please add examples (with indicators where we want 
warning) and explain how they would be handled according to the different 
implementation options. Also please highlight, if possible, what are the 
limitations of the different solutions: potential false positives that we 
cannot filter out, or bugs we will not be able to find.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91948

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

[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman edited subscribers, added: ymandel; removed: aaron.ballman.
aaron.ballman added a comment.

In D89743#2409115 , @sammccall wrote:

> In D89743#2409001 , @sammccall wrote:
>
>> We didn't talk about overloading isImplicit to apply to attrs too, but it 
>> doesn't seem like that was controversial (and it does help with the tests).
>
> I spoke too soon about this...
> This prevents `hasAncestor(isImplicit())` from compiling, because 
> `hasAncestor` needs to deduce the node type from its argument to call 
> `ASTMatchFinder::matchesAncestorOf()`.
> This occurs in a few places in tree and many places in our private codebase...
> The workaround is `hasAncestor(decl(isImplicit()))` which is reasonable, 
> except that "is contained in *any* implicit node" is probably actually the 
> intent. Well, at least it's not a regression.

Users can always traverse in `IgnoreUnlessSpelledInSource` mode for that 
situation though, so at least there's a reasonable path forward.

> In addition, while digging into this, I realized Attrs are not traversed in 
> the parent map, and not supported by the parent/child/ancestor/descendant 
> traversals.
> So I'm fixing that... and adding some tests.

Good catch!

> I'll need to send this for another round, even without the name matcher.

Thank you!




Comment at: clang/lib/AST/ASTTypeTraits.cpp:138
+return ASTNodeKind(NKI_##A##Attr);
+#include "clang/Basic/AttrList.inc"
+  }

Oye, this brings up an interesting point. Plugin-based attributes currently 
cannot create their own semantic attribute, but will often instead reuse an 
existing semantic attribute like `annotate`. This means code like 
`[[clang::plugin_attr]] int x;` may or may not be possible to match. Further, 
some builtin attributes have no semantic attribute associated with them 
whatsoever: 
https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Basic/Attr.td#L2740

I think the `switch` statement logic here is correct in these weird cases and 
we won't hit the `llvm_unreachable`. For attributes with no AST representation, 
there's no `Attr` object that could be passed in the first place. Unknown 
attributes similarly won't get here because there's no way to get an AST node 
for them. Plugin-based attributes are still going to be similarly surprising, 
but... I don't know that we can solve that here given there's no way to create 
a plugin-based semantic attribute yet.

Pining @ymandel to raise awareness of these sorts of issues that stencil may 
run into. For the AST matchers, I think it's reasonable for us to say "if 
there's no AST node, we can't match on it", but IIRC, stencil was looking to 
stay a bit closer to the user's source code rather than be strongly tied to the 
AST.



Comment at: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp:1887
+  // On windows, some nodes have an implicit visibility attribute.
+  EXPECT_TRUE(
+  notMatches("struct F{}; int x(int *);", attr(unless(isImplicit();

Can you add an expects false test for an unknown attribute and another one for 
an attribute with no AST node associated with it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89743

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

[PATCH] D91917: Update mode used in traverse() examples

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91917

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

[PATCH] D83211: Factor out call to EXTRACTOR in generateCC1CommandLine

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4062
+  if ((FLAGS)&options::CC1Option) {
\
+const auto &Extracted = EXTRACTOR(this->KEYPATH);  
\
+if (ALWAYS_EMIT || Extracted != (DEFAULT_VALUE))   
\

dexonsmith wrote:
> IIUC, then we can do this more simply as:
> ```
> bool Extracted = EXTRACTOR(this->KEYPATH);
> ```
> That might clarify why we don't have lifetime extension concerns here.
Yes, we could simplify this for booleans.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3938
+  if ((FLAGS)&options::CC1Option) {
\
+const auto &Extracted = EXTRACTOR(this->KEYPATH);  
\
+if (ALWAYS_EMIT || Extracted != DEFAULT_VALUE) 
\

dexonsmith wrote:
> dexonsmith wrote:
> > jansvoboda11 wrote:
> > > Bigcheese wrote:
> > > > Will this ever have an issue with lifetime? I can see various values 
> > > > for `EXTRACTOR` causing issues here. https://abseil.io/tips/107
> > > > 
> > > > 
> > > > It would be good to at least document somewhere the restrictions on 
> > > > `EXTRACTOR`.
> > > Mentioned the reference lifetime extension in a comment near extractor 
> > > definitions.
> > It might be safer to refactor as:
> > ```
> > // Capture the extracted value as a lambda argument to avoid potential
> > // lifetime extension issues.
> > [&](const auto &Extracted) {
> >   if (ALWAYS_EMIT || Extracted != (DEFAULT_VALUE))
> > DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, Extracted);
> > }(EXTRACTOR(this->KEYPATH));
> > ```
> > 
> Might be even better to avoid the generic lambda:
> ```
> // Capture the extracted value as a lambda argument to avoid potential
> // lifetime extension issues.
> using ExtractedType =
> std::remove_const_t decltype(EXTRACTOR(this->KEYPATH))>>
> [&](const ExtractedType &Extracted) {
>   if (ALWAYS_EMIT || Extracted != (DEFAULT_VALUE))
> DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, Extracted);
> }(EXTRACTOR(this->KEYPATH));
> ```
> (since generic vs. non-generic could affect compile-time of 
> CompilerInvocation.cpp given how many instances there will be).
Thanks for the suggestions @dexonsmith. I'm having trouble writing a test case 
where the lambda workaround produces a different result than `const auto &` 
variable.
@Bigcheese, could you show a concrete example of an extractor that causes 
issues so I can test it out?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83211

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

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Jakub Jelínek via Phabricator via cfe-commits]

jakubjelinek added a comment.

In D87974#2409829 , @zoecarver wrote:

> @jwakely It looks like `UnsizedTail` causes a crash.

Filed https://gcc.gnu.org/PR97943 for that.  Avoiding the crash is trivial, 
deciding what we want to do exactly for flexible array members (which are 
beyond the C++ standard) is harder.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974

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

[PATCH] D91916: Remove automatic traversal from forEach matcher

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91916

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

[PATCH] D91918: Remove the IgnoreImplicitCastsAndParentheses traversal kind

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a documentation request, thank you for this cleanup!




Comment at: clang/docs/ReleaseNotes.rst:237
 
+- The TK_IgnoreImplicitCastsAndParentheses traversal kind was removed. It
+  is recommended to use TK_IgnoreUnlessSpelledInSource instead.

Can you add a similar note in clang-tools-extra/docs/ReleaseNotes.rst for the 
change in clang-query?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91918

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

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:616-619
+  if (CRD->isStruct() && 
!isHungarianNotationOptionEnabled("TreatStructAsClass",
+   HNOption.General)) {
+return "";
+  }





Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:671-672
+
+  if (clang::Decl::Kind::EnumConstant == ND->getKind() ||
+  clang::Decl::Kind::Function == ND->getKind()) {
+return "";





Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:740
+// Remove redundant tailing.
+const static std::list TailsOfMultiWordType = {
+" int", " char", " double", " long", " short"};

Similar to what was suggested above.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:718
+
+const static std::list Keywords = {
+// Constexpr specifiers

njames93 wrote:
> 
This comment doesn't appear to have been handled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

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

[PATCH] D91915: [clang-tidy] Fix RenamerClangTidy checks trying to emit a fix that isnt a valid identifier

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91915

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

[PATCH] D91930: [clangd] Implement textDocument/codeLens

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Thanks a lot for working on improving clangd!

Can you also give a high-level overview of what kind of functionality you are 
providing here? Looks like there's a lot going on here, and it would be nice to 
know what you are attempting to do, rather than inferring that from the code. 
It would also help future travellers, as they can just read the description 
rather than going through the whole patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91930

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

[PATCH] D90282: [clang-tidy] Add IgnoreShortNames config to identifier naming checks

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, but please wait for @njames93 in case they have additional feedback.




Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h:64
+// Store both compiled and non-compiled forms so original value can be
+// serialised
+llvm::Regex IgnoredRegexp;




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

https://reviews.llvm.org/D90282

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

[clang] 72a9f36 - Remove automatic traversal from forEach matcher

[Stephen Kelly via cfe-commits]

Author: Stephen Kelly
Date: 2020-11-23T14:27:47Z
New Revision: 72a9f365e9933d68645f796592932a27d11bbfd0

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

LOG: Remove automatic traversal from forEach matcher

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/ASTMatchers/ASTMatchersInternal.h
clang/lib/ASTMatchers/ASTMatchFinder.cpp
clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8da9490f7b6f..737f165c80cb 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -234,6 +234,9 @@ AST Matchers
   has been changed to no longer match on template instantiations or on
   implicit nodes which are not spelled in the source.
 
+- The behavior of the forEach() matcher was changed to not internally ignore
+  implicit and parenthesis nodes.
+
 clang-format
 
 

diff  --git a/clang/include/clang/ASTMatchers/ASTMatchersInternal.h 
b/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
index 480a12b0dafb..fdc4f66f5d9c 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -679,8 +679,7 @@ class ASTMatchFinder {
 
   template 
   bool matchesChildOf(const T &Node, const DynTypedMatcher &Matcher,
-  BoundNodesTreeBuilder *Builder, TraversalKind Traverse,
-  BindKind Bind) {
+  BoundNodesTreeBuilder *Builder, BindKind Bind) {
 static_assert(std::is_base_of::value ||
   std::is_base_of::value ||
   std::is_base_of::value ||
@@ -689,7 +688,7 @@ class ASTMatchFinder {
   std::is_base_of::value,
   "unsupported type for recursive matching");
 return matchesChildOf(DynTypedNode::create(Node), getASTContext(), Matcher,
-  Builder, Traverse, Bind);
+  Builder, Bind);
   }
 
   template 
@@ -730,7 +729,7 @@ class ASTMatchFinder {
   virtual bool matchesChildOf(const DynTypedNode &Node, ASTContext &Ctx,
   const DynTypedMatcher &Matcher,
   BoundNodesTreeBuilder *Builder,
-  TraversalKind Traverse, BindKind Bind) = 0;
+  BindKind Bind) = 0;
 
   virtual bool matchesDescendantOf(const DynTypedNode &Node, ASTContext &Ctx,
const DynTypedMatcher &Matcher,
@@ -1367,7 +1366,6 @@ class HasMatcher : public MatcherInterface {
   bool matches(const T &Node, ASTMatchFinder *Finder,
BoundNodesTreeBuilder *Builder) const override {
 return Finder->matchesChildOf(Node, this->InnerMatcher, Builder,
-  TraversalKind::TK_AsIs,
   ASTMatchFinder::BK_First);
   }
 };
@@ -1392,7 +1390,6 @@ class ForEachMatcher : public MatcherInterface {
BoundNodesTreeBuilder *Builder) const override {
 return Finder->matchesChildOf(
 Node, this->InnerMatcher, Builder,
-TraversalKind::TK_IgnoreImplicitCastsAndParentheses,
 ASTMatchFinder::BK_All);
   }
 };

diff  --git a/clang/lib/ASTMatchers/ASTMatchFinder.cpp 
b/clang/lib/ASTMatchers/ASTMatchFinder.cpp
index e43778b4fe8f..cc9537144524 100644
--- a/clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -95,12 +95,11 @@ class MatchChildASTVisitor
   // matching the descendants.
   MatchChildASTVisitor(const DynTypedMatcher *Matcher, ASTMatchFinder *Finder,
BoundNodesTreeBuilder *Builder, int MaxDepth,
-   TraversalKind Traversal, bool IgnoreImplicitChildren,
+   bool IgnoreImplicitChildren,
ASTMatchFinder::BindKind Bind)
   : Matcher(Matcher), Finder(Finder), Builder(Builder), CurrentDepth(0),
-MaxDepth(MaxDepth), Traversal(Traversal),
-IgnoreImplicitChildren(IgnoreImplicitChildren), Bind(Bind),
-Matches(false) {}
+MaxDepth(MaxDepth), IgnoreImplicitChildren(IgnoreImplicitChildren),
+Bind(Bind), Matches(false) {}
 
   // Returns true if a match is found in the subtree rooted at the
   // given AST node. This is done via a set of mutually recursive
@@ -168,10 +167,6 @@ class MatchChildASTVisitor
 Finder->getASTContext().getParentMapContext().traverseIgnored(
 ExprNode);
 }
-if (Traversal == TraversalKind::TK_IgnoreImplicitCastsAndParentheses) {
-  if (Expr *ExprNode = dyn_cast_or_

[clang] f052cf4 - Update mode used in traverse() examples

[Stephen Kelly via cfe-commits]

Author: Stephen Kelly
Date: 2020-11-23T14:27:48Z
New Revision: f052cf494f07a33af5aa7c680cfe0bfcca24beae

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

LOG: Update mode used in traverse() examples

traverse() predates the IgnoreUnlessSpelledInSource mode. Update example
and test code to use the newer mode.

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

Added: 


Modified: 
clang/docs/LibASTMatchersReference.html
clang/include/clang/ASTMatchers/ASTMatchers.h
clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Removed: 




diff  --git a/clang/docs/LibASTMatchersReference.html 
b/clang/docs/LibASTMatchersReference.html
index fbc53ed743a2..9f14ab9f4c5b 100644
--- a/clang/docs/LibASTMatchersReference.html
+++ b/clang/docs/LibASTMatchersReference.html
@@ -5567,7 +5567,7 @@ AST Traversal Matchers
   int i = 3.0;
   }
 The matcher
-  traverse(TK_IgnoreImplicitCastsAndParentheses,
+  traverse(TK_IgnoreUnlessSpelledInSource,
 varDecl(hasInitializer(floatLiteral().bind("init")))
   )
 matches the variable declaration with "init" bound to the "3.0".

diff  --git a/clang/include/clang/ASTMatchers/ASTMatchers.h 
b/clang/include/clang/ASTMatchers/ASTMatchers.h
index d8b049f45341..0da469ea0f78 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -782,7 +782,7 @@ AST_POLYMORPHIC_MATCHER_P(
 /// \endcode
 /// The matcher
 /// \code
-///   traverse(TK_IgnoreImplicitCastsAndParentheses,
+///   traverse(TK_IgnoreUnlessSpelledInSource,
 /// varDecl(hasInitializer(floatLiteral().bind("init")))
 ///   )
 /// \endcode

diff  --git a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp 
b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
index 004c667f053d..b0ec1719daaf 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -1873,8 +1873,8 @@ void foo()
   auto Matcher = varDecl(hasInitializer(floatLiteral()));
 
   EXPECT_TRUE(notMatches(VarDeclCode, traverse(TK_AsIs, Matcher)));
-  EXPECT_TRUE(matches(VarDeclCode,
-  traverse(TK_IgnoreImplicitCastsAndParentheses, 
Matcher)));
+  EXPECT_TRUE(
+  matches(VarDeclCode, traverse(TK_IgnoreUnlessSpelledInSource, Matcher)));
 
   auto ParentMatcher = floatLiteral(hasParent(varDecl(hasName("i";
 
@@ -2715,14 +2715,14 @@ void foo()
 )cpp";
 
   EXPECT_TRUE(
-  matches(Code, traverse(TK_IgnoreImplicitCastsAndParentheses,
+  matches(Code, traverse(TK_IgnoreUnlessSpelledInSource,
  callExpr(has(callExpr(traverse(
  TK_AsIs, callExpr(has(implicitCastExpr(
   has(floatLiteral(;
 
   EXPECT_TRUE(matches(
   Code,
-  traverse(TK_IgnoreImplicitCastsAndParentheses,
+  traverse(TK_IgnoreUnlessSpelledInSource,
traverse(TK_AsIs, implicitCastExpr(has(floatLiteral()));
 }
 
@@ -2738,8 +2738,7 @@ void constructImplicit() {
 }
   )cpp";
 
-  auto Matcher =
-  traverse(TK_IgnoreImplicitCastsAndParentheses, implicitCastExpr());
+  auto Matcher = traverse(TK_IgnoreUnlessSpelledInSource, implicitCastExpr());
 
   // Verfiy that it does not segfault
   EXPECT_FALSE(matches(Code, Matcher));
@@ -2766,7 +2765,7 @@ void foo()
   EXPECT_TRUE(matches(
   Code,
   functionDecl(anyOf(hasDescendant(Matcher),
- traverse(TK_IgnoreImplicitCastsAndParentheses,
+ traverse(TK_IgnoreUnlessSpelledInSource,
   functionDecl(hasDescendant(Matcher)));
 }
 



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

[clang-tools-extra] 5e18018 - Remove the IgnoreImplicitCastsAndParentheses traversal kind

[Stephen Kelly via cfe-commits]

Author: Stephen Kelly
Date: 2020-11-23T14:27:48Z
New Revision: 5e1801813d93210acae84ff3c68a01512c2df9bc

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

LOG: Remove the IgnoreImplicitCastsAndParentheses traversal kind

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

Added: 


Modified: 
clang-tools-extra/clang-query/QueryParser.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/ASTNodeTraverser.h
clang/include/clang/AST/ASTTypeTraits.h
clang/lib/AST/ParentMapContext.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-query/QueryParser.cpp 
b/clang-tools-extra/clang-query/QueryParser.cpp
index 2f1965e77ab4..45a0d425b7c2 100644
--- a/clang-tools-extra/clang-query/QueryParser.cpp
+++ b/clang-tools-extra/clang-query/QueryParser.cpp
@@ -134,8 +134,6 @@ QueryRef QueryParser::parseSetTraversalKind(
   unsigned Value =
   LexOrCompleteWord(this, ValStr)
   .Case("AsIs", ast_type_traits::TK_AsIs)
-  .Case("IgnoreImplicitCastsAndParentheses",
-ast_type_traits::TK_IgnoreImplicitCastsAndParentheses)
   .Case("IgnoreUnlessSpelledInSource",
 ast_type_traits::TK_IgnoreUnlessSpelledInSource)
   .Default(~0u);

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 5e78de2b0edc..c99a589b1212 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -57,7 +57,7 @@ The improvements are...
 Improvements to clang-query
 ---
 
-The improvements are...
+- The IgnoreImplicitCastsAndParentheses traversal mode has been removed.
 
 Improvements to clang-rename
 

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 737f165c80cb..d62c62dad3d2 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -234,6 +234,9 @@ AST Matchers
   has been changed to no longer match on template instantiations or on
   implicit nodes which are not spelled in the source.
 
+- The TK_IgnoreImplicitCastsAndParentheses traversal kind was removed. It
+  is recommended to use TK_IgnoreUnlessSpelledInSource instead.
+
 - The behavior of the forEach() matcher was changed to not internally ignore
   implicit and parenthesis nodes.
 

diff  --git a/clang/include/clang/AST/ASTNodeTraverser.h 
b/clang/include/clang/AST/ASTNodeTraverser.h
index 4f33b0c67e94..6f7affe66273 100644
--- a/clang/include/clang/AST/ASTNodeTraverser.h
+++ b/clang/include/clang/AST/ASTNodeTraverser.h
@@ -126,9 +126,6 @@ class ASTNodeTraverser
 switch (Traversal) {
 case TK_AsIs:
   break;
-case TK_IgnoreImplicitCastsAndParentheses:
-  S = E->IgnoreParenImpCasts();
-  break;
 case TK_IgnoreUnlessSpelledInSource:
   S = E->IgnoreUnlessSpelledInSource();
   break;

diff  --git a/clang/include/clang/AST/ASTTypeTraits.h 
b/clang/include/clang/AST/ASTTypeTraits.h
index bd817b75bb84..034260cc8ed3 100644
--- a/clang/include/clang/AST/ASTTypeTraits.h
+++ b/clang/include/clang/AST/ASTTypeTraits.h
@@ -40,10 +40,6 @@ enum TraversalKind {
   /// Will traverse all child nodes.
   TK_AsIs,
 
-  /// Will not traverse implicit casts and parentheses.
-  /// Corresponds to Expr::IgnoreParenImpCasts()
-  TK_IgnoreImplicitCastsAndParentheses,
-
   /// Ignore AST nodes not written in the source
   TK_IgnoreUnlessSpelledInSource
 };
@@ -542,8 +538,6 @@ using ASTNodeKind = ::clang::ASTNodeKind;
 using TraversalKind = ::clang::TraversalKind;
 
 constexpr TraversalKind TK_AsIs = ::clang::TK_AsIs;
-constexpr TraversalKind TK_IgnoreImplicitCastsAndParentheses =
-::clang::TK_IgnoreImplicitCastsAndParentheses;
 constexpr TraversalKind TK_IgnoreUnlessSpelledInSource =
 ::clang::TK_IgnoreUnlessSpelledInSource;
 } // namespace ast_type_traits

diff  --git a/clang/lib/AST/ParentMapContext.cpp 
b/clang/lib/AST/ParentMapContext.cpp
index c80c8bc23e00..cb4995312efa 100644
--- a/clang/lib/AST/ParentMapContext.cpp
+++ b/clang/lib/AST/ParentMapContext.cpp
@@ -36,8 +36,6 @@ Expr *ParentMapContext::traverseIgnored(Expr *E) const {
   switch (Traversal) {
   case TK_AsIs:
 return E;
-  case TK_IgnoreImplicitCastsAndParentheses:
-return E->IgnoreParenImpCasts();
   case TK_IgnoreUnlessSpelledInSource:
 return E->IgnoreUnlessSpelledInSource();
   }



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

[PATCH] D91916: Remove automatic traversal from forEach matcher

[Stephen Kelly via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG72a9f365e993: Remove automatic traversal from forEach 
matcher (authored by stephenkelly).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91916

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -3370,6 +3370,38 @@
 std::make_unique>("f", 4)));
 }
 
+TEST(ForEach, DoesNotIgnoreImplicit) {
+  StringRef Code = R"cpp(
+void foo()
+{
+int i = 0;
+int b = 4;
+i < b;
+}
+)cpp";
+  EXPECT_TRUE(matchAndVerifyResultFalse(
+  Code, binaryOperator(forEach(declRefExpr().bind("dre"))),
+  std::make_unique>("dre", 0)));
+
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  Code,
+  binaryOperator(forEach(
+  implicitCastExpr(hasSourceExpression(declRefExpr().bind("dre"),
+  std::make_unique>("dre", 2)));
+
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  Code,
+  binaryOperator(
+  forEach(expr(ignoringImplicit(declRefExpr().bind("dre"),
+  std::make_unique>("dre", 2)));
+
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  Code,
+  traverse(TK_IgnoreUnlessSpelledInSource,
+   binaryOperator(forEach(declRefExpr().bind("dre",
+  std::make_unique>("dre", 2)));
+}
+
 TEST(ForEachDescendant, BindsOneNode) {
   EXPECT_TRUE(matchAndVerifyResultTrue("class C { class D { int x; }; };",
recordDecl(hasName("C"),
Index: clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
@@ -70,7 +70,6 @@
   internal::Matcher, AMatcher) {
   return Finder->matchesChildOf(
   Node, AMatcher, Builder,
-  TraversalKind::TK_IgnoreImplicitCastsAndParentheses,
   ASTMatchFinder::BK_First);
 }
 
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -95,12 +95,11 @@
   // matching the descendants.
   MatchChildASTVisitor(const DynTypedMatcher *Matcher, ASTMatchFinder *Finder,
BoundNodesTreeBuilder *Builder, int MaxDepth,
-   TraversalKind Traversal, bool IgnoreImplicitChildren,
+   bool IgnoreImplicitChildren,
ASTMatchFinder::BindKind Bind)
   : Matcher(Matcher), Finder(Finder), Builder(Builder), CurrentDepth(0),
-MaxDepth(MaxDepth), Traversal(Traversal),
-IgnoreImplicitChildren(IgnoreImplicitChildren), Bind(Bind),
-Matches(false) {}
+MaxDepth(MaxDepth), IgnoreImplicitChildren(IgnoreImplicitChildren),
+Bind(Bind), Matches(false) {}
 
   // Returns true if a match is found in the subtree rooted at the
   // given AST node. This is done via a set of mutually recursive
@@ -168,10 +167,6 @@
 Finder->getASTContext().getParentMapContext().traverseIgnored(
 ExprNode);
 }
-if (Traversal == TraversalKind::TK_IgnoreImplicitCastsAndParentheses) {
-  if (Expr *ExprNode = dyn_cast_or_null(StmtNode))
-StmtToTraverse = ExprNode->IgnoreParenImpCasts();
-}
 return StmtToTraverse;
   }
 
@@ -371,7 +366,6 @@
   BoundNodesTreeBuilder ResultBindings;
   int CurrentDepth;
   const int MaxDepth;
-  const TraversalKind Traversal;
   const bool IgnoreImplicitChildren;
   const ASTMatchFinder::BindKind Bind;
   bool Matches;
@@ -473,11 +467,10 @@
   bool memoizedMatchesRecursively(const DynTypedNode &Node, ASTContext &Ctx,
   const DynTypedMatcher &Matcher,
   BoundNodesTreeBuilder *Builder, int MaxDepth,
-  TraversalKind Traversal, BindKind Bind) {
+  BindKind Bind) {
 // For AST-nodes that don't have an identity, we can't memoize.
 if (!Node.getMemoizationData() || !Builder->isComparable())
-  return matchesRecursively(Node, Matcher, Builder, MaxDepth, Traversal,
-Bind);
+  return matchesRecursively(Node, Matcher, Builder, MaxDepth, Bind);
 
 MatchKey Key;
 Key.MatcherID = Matcher.getID();
@@ -495,8 +488,8 @@
 
 MemoizedMatchResult Resu

[PATCH] D91917: Update mode used in traverse() examples

[Stephen Kelly via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf052cf494f07: Update mode used in traverse() examples 
(authored by stephenkelly).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91917

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp


Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -1873,8 +1873,8 @@
   auto Matcher = varDecl(hasInitializer(floatLiteral()));
 
   EXPECT_TRUE(notMatches(VarDeclCode, traverse(TK_AsIs, Matcher)));
-  EXPECT_TRUE(matches(VarDeclCode,
-  traverse(TK_IgnoreImplicitCastsAndParentheses, 
Matcher)));
+  EXPECT_TRUE(
+  matches(VarDeclCode, traverse(TK_IgnoreUnlessSpelledInSource, Matcher)));
 
   auto ParentMatcher = floatLiteral(hasParent(varDecl(hasName("i";
 
@@ -2715,14 +2715,14 @@
 )cpp";
 
   EXPECT_TRUE(
-  matches(Code, traverse(TK_IgnoreImplicitCastsAndParentheses,
+  matches(Code, traverse(TK_IgnoreUnlessSpelledInSource,
  callExpr(has(callExpr(traverse(
  TK_AsIs, callExpr(has(implicitCastExpr(
   has(floatLiteral(;
 
   EXPECT_TRUE(matches(
   Code,
-  traverse(TK_IgnoreImplicitCastsAndParentheses,
+  traverse(TK_IgnoreUnlessSpelledInSource,
traverse(TK_AsIs, implicitCastExpr(has(floatLiteral()));
 }
 
@@ -2738,8 +2738,7 @@
 }
   )cpp";
 
-  auto Matcher =
-  traverse(TK_IgnoreImplicitCastsAndParentheses, implicitCastExpr());
+  auto Matcher = traverse(TK_IgnoreUnlessSpelledInSource, implicitCastExpr());
 
   // Verfiy that it does not segfault
   EXPECT_FALSE(matches(Code, Matcher));
@@ -2766,7 +2765,7 @@
   EXPECT_TRUE(matches(
   Code,
   functionDecl(anyOf(hasDescendant(Matcher),
- traverse(TK_IgnoreImplicitCastsAndParentheses,
+ traverse(TK_IgnoreUnlessSpelledInSource,
   functionDecl(hasDescendant(Matcher)));
 }
 
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -782,7 +782,7 @@
 /// \endcode
 /// The matcher
 /// \code
-///   traverse(TK_IgnoreImplicitCastsAndParentheses,
+///   traverse(TK_IgnoreUnlessSpelledInSource,
 /// varDecl(hasInitializer(floatLiteral().bind("init")))
 ///   )
 /// \endcode
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -5567,7 +5567,7 @@
   int i = 3.0;
   }
 The matcher
-  traverse(TK_IgnoreImplicitCastsAndParentheses,
+  traverse(TK_IgnoreUnlessSpelledInSource,
 varDecl(hasInitializer(floatLiteral().bind("init")))
   )
 matches the variable declaration with "init" bound to the "3.0".


Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -1873,8 +1873,8 @@
   auto Matcher = varDecl(hasInitializer(floatLiteral()));
 
   EXPECT_TRUE(notMatches(VarDeclCode, traverse(TK_AsIs, Matcher)));
-  EXPECT_TRUE(matches(VarDeclCode,
-  traverse(TK_IgnoreImplicitCastsAndParentheses, Matcher)));
+  EXPECT_TRUE(
+  matches(VarDeclCode, traverse(TK_IgnoreUnlessSpelledInSource, Matcher)));
 
   auto ParentMatcher = floatLiteral(hasParent(varDecl(hasName("i";
 
@@ -2715,14 +2715,14 @@
 )cpp";
 
   EXPECT_TRUE(
-  matches(Code, traverse(TK_IgnoreImplicitCastsAndParentheses,
+  matches(Code, traverse(TK_IgnoreUnlessSpelledInSource,
  callExpr(has(callExpr(traverse(
  TK_AsIs, callExpr(has(implicitCastExpr(
   has(floatLiteral(;
 
   EXPECT_TRUE(matches(
   Code,
-  traverse(TK_IgnoreImplicitCastsAndParentheses,
+  traverse(TK_IgnoreUnlessSpelledInSource,
traverse(TK_AsIs, implicitCastExpr(has(floatLiteral()));
 }
 
@@ -2738,8 +2738,7 @@
 }
   )cpp";
 
-  auto Matcher =
-  traverse(TK_IgnoreImplicitCastsAndParentheses, implicitCastExpr());
+  auto Matcher = traverse(TK_IgnoreUnlessSpelledInSource, implicitCastExpr());
 
   // Verfiy that it does not segfault
   EXPEC

[PATCH] D91918: Remove the IgnoreImplicitCastsAndParentheses traversal kind

[Stephen Kelly via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5e1801813d93: Remove the IgnoreImplicitCastsAndParentheses 
traversal kind (authored by stephenkelly).

Changed prior to commit:
  https://reviews.llvm.org/D91918?vs=306859&id=307065#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91918

Files:
  clang-tools-extra/clang-query/QueryParser.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/ASTTypeTraits.h
  clang/lib/AST/ParentMapContext.cpp


Index: clang/lib/AST/ParentMapContext.cpp
===
--- clang/lib/AST/ParentMapContext.cpp
+++ clang/lib/AST/ParentMapContext.cpp
@@ -36,8 +36,6 @@
   switch (Traversal) {
   case TK_AsIs:
 return E;
-  case TK_IgnoreImplicitCastsAndParentheses:
-return E->IgnoreParenImpCasts();
   case TK_IgnoreUnlessSpelledInSource:
 return E->IgnoreUnlessSpelledInSource();
   }
Index: clang/include/clang/AST/ASTTypeTraits.h
===
--- clang/include/clang/AST/ASTTypeTraits.h
+++ clang/include/clang/AST/ASTTypeTraits.h
@@ -40,10 +40,6 @@
   /// Will traverse all child nodes.
   TK_AsIs,
 
-  /// Will not traverse implicit casts and parentheses.
-  /// Corresponds to Expr::IgnoreParenImpCasts()
-  TK_IgnoreImplicitCastsAndParentheses,
-
   /// Ignore AST nodes not written in the source
   TK_IgnoreUnlessSpelledInSource
 };
@@ -542,8 +538,6 @@
 using TraversalKind = ::clang::TraversalKind;
 
 constexpr TraversalKind TK_AsIs = ::clang::TK_AsIs;
-constexpr TraversalKind TK_IgnoreImplicitCastsAndParentheses =
-::clang::TK_IgnoreImplicitCastsAndParentheses;
 constexpr TraversalKind TK_IgnoreUnlessSpelledInSource =
 ::clang::TK_IgnoreUnlessSpelledInSource;
 } // namespace ast_type_traits
Index: clang/include/clang/AST/ASTNodeTraverser.h
===
--- clang/include/clang/AST/ASTNodeTraverser.h
+++ clang/include/clang/AST/ASTNodeTraverser.h
@@ -126,9 +126,6 @@
 switch (Traversal) {
 case TK_AsIs:
   break;
-case TK_IgnoreImplicitCastsAndParentheses:
-  S = E->IgnoreParenImpCasts();
-  break;
 case TK_IgnoreUnlessSpelledInSource:
   S = E->IgnoreUnlessSpelledInSource();
   break;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -234,6 +234,9 @@
   has been changed to no longer match on template instantiations or on
   implicit nodes which are not spelled in the source.
 
+- The TK_IgnoreImplicitCastsAndParentheses traversal kind was removed. It
+  is recommended to use TK_IgnoreUnlessSpelledInSource instead.
+
 - The behavior of the forEach() matcher was changed to not internally ignore
   implicit and parenthesis nodes.
 
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -57,7 +57,7 @@
 Improvements to clang-query
 ---
 
-The improvements are...
+- The IgnoreImplicitCastsAndParentheses traversal mode has been removed.
 
 Improvements to clang-rename
 
Index: clang-tools-extra/clang-query/QueryParser.cpp
===
--- clang-tools-extra/clang-query/QueryParser.cpp
+++ clang-tools-extra/clang-query/QueryParser.cpp
@@ -134,8 +134,6 @@
   unsigned Value =
   LexOrCompleteWord(this, ValStr)
   .Case("AsIs", ast_type_traits::TK_AsIs)
-  .Case("IgnoreImplicitCastsAndParentheses",
-ast_type_traits::TK_IgnoreImplicitCastsAndParentheses)
   .Case("IgnoreUnlessSpelledInSource",
 ast_type_traits::TK_IgnoreUnlessSpelledInSource)
   .Default(~0u);


Index: clang/lib/AST/ParentMapContext.cpp
===
--- clang/lib/AST/ParentMapContext.cpp
+++ clang/lib/AST/ParentMapContext.cpp
@@ -36,8 +36,6 @@
   switch (Traversal) {
   case TK_AsIs:
 return E;
-  case TK_IgnoreImplicitCastsAndParentheses:
-return E->IgnoreParenImpCasts();
   case TK_IgnoreUnlessSpelledInSource:
 return E->IgnoreUnlessSpelledInSource();
   }
Index: clang/include/clang/AST/ASTTypeTraits.h
===
--- clang/include/clang/AST/ASTTypeTraits.h
+++ clang/include/clang/AST/ASTTypeTraits.h
@@ -40,10 +40,6 @@
   /// Will traverse all child nodes.
   TK_AsIs,
 
-  /// Will not traverse implicit casts and parentheses.
-  /// Corresponds 

[PATCH] D91605: [sanitizers] Implement GetTls on Solaris

[Rainer Orth via Phabricator via cfe-commits]

ro added inline comments.



Comment at: clang/tools/driver/CMakeLists.txt:123
+
+check_linker_flag("-Wl,-z,relax=transtls" LINKER_SUPPORTS_Z_RELAX_TRANSTLS)

MaskRay wrote:
> ro wrote:
> > MaskRay wrote:
> > > ro wrote:
> > > > MaskRay wrote:
> > > > > GNU ld reports a warning instead of an error when an unknown `-z` is 
> > > > > seen. The warning remains a warning even with `--fatal-warnings`.
> > > > > GNU ld reports a warning instead of an error when an unknown `-z` is 
> > > > > seen. The warning remains a warning even with `--fatal-warnings`.
> > > > 
> > > > Thanks for reminding me about that misfeature of GNU `ld`.  I guess 
> > > > `check_linker_flags` needs to be updated to handle that.
> > > > In the case at hand, it won't matter either way: the flag is only 
> > > > passed to `ld`, which on Solaris is guaranteed to be the native linker. 
> > > >  Once (if at all) I get around to completing D85309, I can deal with 
> > > > that.  For now, other targets won't see linker warnings about this 
> > > > flag, other than when the flag is used at build time.
> > > OK. Then I guess you can condition this when the OS is Solaris?
> > > OK. Then I guess you can condition this when the OS is Solaris?
> > 
> > I fear not: `LINKER_SUPPORTS_Z_RELAX_TRANSTLS` is tested inside an `if` in 
> > `Solaris.cpp`: this code is also compiled on non-Solaris hosts.  Why are 
> > you worried about the definition being always present?
> It is not suitable if LINKER_SUPPORTS_Z_RELAX_TRANSTLS returns a wrong result 
> for GNU ld, even if it is not used for non-Solaris. We should make the value 
> correct in other configurations.
> It is not suitable if LINKER_SUPPORTS_Z_RELAX_TRANSTLS returns a wrong result 
> for GNU ld, even if it is not used for non-Solaris. We should make the value 
> correct in other configurations.

Tell the binutils maintainers that ;-)  While I'm still unconcerned about this 
particular case (it's only used on a Solaris host where `clang` hardcodes the 
use of `/usr/bin/ld`), I continue to be annoyed by GNU `ld`'s nonsensical (or 
even outright dangerous) behaviour of accepting every `-z` option.

It took me some wrestling with `cmake` , but now `check_linker_flag` correctly 
rejects `-z ` flags where GNU `ld` produces the warning.

Some caveats about the implementation:
- `check_cxx_compiler_flag` doesn't support the `FAIL_REGEX` arg, so I had to 
switch to `check_cxx_source_compiles` instead.
- While it would be more appropriate to add the linker flag under test to 
`CMAKE_REQUIRED_LINK_OPTIONS`, that is only present since `cmake` 3.14 while 
LLVM still only requires 3.13.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91605

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

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:614
+  if (CRD->isUnion())
+return "";
+

Returning {} as default initializer will be better. Same in other places.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

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

[PATCH] D91605: [sanitizers] Implement GetTls on Solaris

[Rainer Orth via Phabricator via cfe-commits]

ro updated this revision to Diff 307067.
ro added a comment.
Herald added a project: LLVM.

- Move declarations to new `sanitizer_solaris.h`.
- Augment `check_linker_flag` do reject unknown `-z` options that GNU ld 
noisily accepts.

Tested on `amd64-pc-solaris2.11`, `sparcv9-sun-solaris2.11`, and 
`x86_64-pc-linux-gnu`.  `CMakeCache.txt` was unchanged on Solaris, while 
`LINKER_SUPPORTS_Z_RELAX_TRANSTLS` is now false on Linux, both for stage 1 and 
2.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91605

Files:
  clang/include/clang/Config/config.h.cmake
  clang/lib/Driver/ToolChains/Solaris.cpp
  clang/tools/driver/CMakeLists.txt
  compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
  compiler-rt/lib/sanitizer_common/sanitizer_solaris.h
  llvm/cmake/modules/CheckLinkerFlag.cmake

Index: llvm/cmake/modules/CheckLinkerFlag.cmake
===
--- llvm/cmake/modules/CheckLinkerFlag.cmake
+++ llvm/cmake/modules/CheckLinkerFlag.cmake
@@ -1,6 +1,10 @@
-include(CheckCXXCompilerFlag)
+include(CheckCXXSourceCompiles)
 
 function(check_linker_flag flag out_var)
-  set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${flag}")
-  check_cxx_compiler_flag("" ${out_var})
+  set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
+  set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} ${flag}")
+  # GNU ld accepts every -z option, warning if it isn't supported, e.g.
+  # /usr/bin/ld: warning: -z relax=transtls ignored
+  check_cxx_source_compiles("int main() { return 0; }" ${out_var} FAIL_REGEX "warning: -z.* ignored")
+  set(CMAKE_REQUIRED_FLAGS ${OLD_CMAKE_REQUIRED_FLAGS})
 endfunction()
Index: compiler-rt/lib/sanitizer_common/sanitizer_solaris.h
===
--- /dev/null
+++ compiler-rt/lib/sanitizer_common/sanitizer_solaris.h
@@ -0,0 +1,63 @@
+//===-- sanitizer_solaris.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file is a part of Sanitizer runtime. It contains Solaris-specific
+// definitions.
+//
+//===--===//
+
+#ifndef SANITIZER_SOLARIS_H
+#define SANITIZER_SOLARIS_H
+
+#include "sanitizer_internal_defs.h"
+
+#if SANITIZER_SOLARIS
+
+#include 
+
+namespace __sanitizer {
+
+// Beginning of declaration from OpenSolaris/Illumos
+// $SRC/cmd/sgs/include/rtld.h.
+struct Rt_map {
+  Link_map rt_public;
+  const char *rt_pathname;
+  ulong_t rt_padstart;
+  ulong_t rt_padimlen;
+  ulong_t rt_msize;
+  uint_t rt_flags;
+  uint_t rt_flags1;
+  ulong_t rt_tlsmodid;
+};
+
+// Structure matching the Solaris 11.4 struct dl_phdr_info used to determine
+// presence of dlpi_tls_modid field at runtime.  Cf. Solaris 11.4
+// dl_iterate_phdr(3C), Example 2.
+struct dl_phdr_info_test {
+  ElfW(Addr) dlpi_addr;
+  const char *dlpi_name;
+  const ElfW(Phdr) * dlpi_phdr;
+  ElfW(Half) dlpi_phnum;
+  u_longlong_t dlpi_adds;
+  u_longlong_t dlpi_subs;
+  size_t dlpi_tls_modid;
+  void *dlpi_tls_data;
+};
+
+struct TLS_index {
+  unsigned long ti_moduleid;
+  unsigned long ti_tlsoffset;
+};
+
+extern "C" void *__tls_get_addr(TLS_index *);
+
+}  // namespace __sanitizer
+
+#endif  // SANITIZER_SOLARIS
+
+#endif  // SANITIZER_SOLARIS_H
Index: compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
===
--- compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
+++ compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
@@ -27,6 +27,7 @@
 #include "sanitizer_linux.h"
 #include "sanitizer_placement_new.h"
 #include "sanitizer_procmaps.h"
+#include "sanitizer_solaris.h"
 
 #if SANITIZER_NETBSD
 #define _RTLD_SOURCE  // for __lwp_gettcb_fast() / __lwp_getprivate_fast()
@@ -57,6 +58,7 @@
 #endif
 
 #if SANITIZER_SOLARIS
+#include 
 #include 
 #include 
 #endif
@@ -451,6 +453,39 @@
   void **);
 #endif
 
+#if SANITIZER_SOLARIS
+// dlpi_tls_modid is only available since Solaris 11.4 SRU 10.  Use
+// dlinfo(RTLD_DI_LINKMAP) instead which works on both Solaris 11.3 and Illumos.
+
+static size_t main_tls_modid;
+
+int GetSizeFromHdr(struct dl_phdr_info *info, size_t size, void *data) {
+  const ElfW(Phdr) *hdr = info->dlpi_phdr;
+  const ElfW(Phdr) *last_hdr = hdr + info->dlpi_phnum;
+
+  // With the introduction of dlpi_tls_modid, the tlsmodid of the executable
+  // was changed to 1 to match other implementations.
+  if (size >= offsetof(dl_phdr_info_test, dlpi_tls_modid))
+main_tls_modid = 1;
+  else
+main_tls_modid = 

[PATCH] D91916: Remove automatic traversal from forEach matcher

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Looks like this broke clang-query tests: 
http://45.33.8.238/linux/33556/step_8.txt

Please take a look, and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91916

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

[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D91055#2410447 , @lebedev.ri wrote:

> Ping.
> If there's no pushback within next 24h i will commit this and wait for 
> post-commit review (if any).
> Thanks.

FWIW, that's not the way we usually do things, so please don't push and hope 
for post-commit review on something people have expressed questions/concerns 
over.




Comment at: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp:40
 "misc-new-delete-overloads");
+CheckFactories.registerCheck("misc-no-inttoptr");
 CheckFactories.registerCheck("misc-no-recursion");

Is `misc` really the right place for this? If this is related to optimizer 
behavior, perhaps it should live in `performance`?



Comment at: clang-tools-extra/clang-tidy/misc/NoIntToPtrCheck.cpp:26
+  const auto *MatchedCast = Result.Nodes.getNodeAs("x");
+  diag(MatchedCast->getBeginLoc(), "avoid integer to pointer casts");
+}

This doesn't really tell the user what's wrong with their code or how to fix 
it. There are valid use cases for casting an integer into a pointer, after all. 
For instance, if I'm doing this cast because I'm trying to read a register bank:
```
volatile int *registers = (volatile int *)0xFEEDFACE;
```
what is wrong with my code and what should I do to silence this diagnostic?

Intuitively, one would think that use of `intptr_t` or `uintptr_t` would have 
some impact on this given the standard's requirements for that type in C17 
7.20.1.4p1. Could there be a certain cast path that can be used to silence the 
diagnostic because it's safe enough for the optimizer's needs?



Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-no-inttoptr.rst:6
+
+Diagnoses every integer to pointer cast.
+

```
char buffer[16];
sprintf(buffer, "%p", ptr);

intptr_t val;
sscanf(buffer, "%" SCNxPTR, &val);
```
Do you think this check should catch such constructs under the same reasoning 
as other conversions that hide provenance?



Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-no-inttoptr.rst:12
+if you got that integral value via a pointer-to-integer cast originally,
+the new pointer will lack the provenance information from the original pointer.
+

Does this run afoul of the C++ standard's requirements for pointer 
traceability: http://eel.is/c++draft/basic.stc.dynamic.safety ?



Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-no-inttoptr.cpp:3
+
+// can not implicitly cast int to a pointer.
+// can not use static_cast<>() to cast integer to a pointer.

can not -> cannot
(same below)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91055

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

[PATCH] D91898: [attributes] Add a facility for defining and enforcing a Trusted Computing Base.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: aaronpuchert, delesley.
aaron.ballman added a comment.

This feels an awful lot like a set of attributes we already have -- can 
capability attributes be used for this instead? 
https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#basic-concepts-capabilities
 The documentation is largely centered around thread safety, but the gist of 
the idea seems similar to what you're proposing here -- certain parts of the 
program have different roles and you want to control how functions in one role 
can interact with functions in another role.

(I'll give a more thorough review to the proposed patch here but I wanted to 
pose that question first as it may obviate the need for this patch.)


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

https://reviews.llvm.org/D91898

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

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

1. this needs a full context diff  (diff -U 99)
2. this needs unit tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91950

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

[PATCH] D91949: [clang-format] Add BreakBeforeStructInitialization configuration

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Could you make you patch with a full context diff, plus we really want unit 
tests for all changes

if you change Format.h you need to regenerate the ClangFormatStyleOptions.rst 
by running dump_style.py in clang/docs/tools


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91949

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

[PATCH] D91966: [clangd] AddUsing: Used spelled text instead of type name.

[Adam Czachorowski via Phabricator via cfe-commits]

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

This improves the behavior related to type aliases, as well as cases of
typo correction.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91966

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

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2520,6 +2520,9 @@
   EXPECT_UNAVAILABLE(Header + "void fun() { ::ban::fo^o(); }");
   EXPECT_AVAILABLE(Header + "void fun() { banana::fo^o(); }");
 
+  // Do not offer code action on typo-corrections.
+  EXPECT_UNAVAILABLE(Header + "/*error-ok*/c^c C;");
+
   // Check that we do not trigger in header files.
   FileName = "test.h";
   ExtraArgs.push_back("-xc++-header"); // .h file is treated a C by default.
@@ -2793,6 +2796,21 @@
 
 void fun() {
   ff();
+})cpp"},
+// using alias; insert using for the spelled name.
+{R"cpp(
+#include "test.hpp"
+
+void fun() {
+  one::u^u u;
+})cpp",
+ R"cpp(
+#include "test.hpp"
+
+using one::uu;
+
+void fun() {
+  uu u;
 })cpp"}};
   llvm::StringMap EditedFiles;
   for (const auto &Case : Cases) {
@@ -2809,6 +2827,7 @@
   static void mm() {}
 };
 }
+using uu = two::cc;
 })cpp";
   EXPECT_EQ(apply(SubCase, &EditedFiles), Case.ExpectedSource);
 }
Index: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -43,10 +43,11 @@
   }
 
 private:
-  // The qualifier to remove. Set by prepare().
+  // All of the following are set by prepare().
+  // The qualifier to remove.
   NestedNameSpecifierLoc QualifierToRemove;
-  // The name following QualifierToRemove. Set by prepare().
-  llvm::StringRef Name;
+  // The name following QualifierToRemove.
+  std::string Name;
 };
 REGISTER_TWEAK(AddUsing)
 
@@ -206,8 +207,17 @@
   return false;
 }
 
+std::string GetNNSLAsString(NestedNameSpecifierLoc &NNSL,
+const PrintingPolicy &Policy) {
+  std::string Out;
+  llvm::raw_string_ostream OutStream(Out);
+  NNSL.getNestedNameSpecifier()->print(OutStream, Policy);
+  return OutStream.str();
+}
+
 bool AddUsing::prepare(const Selection &Inputs) {
   auto &SM = Inputs.AST->getSourceManager();
+  auto &TB = Inputs.AST->getTokens();
 
   // Do not suggest "using" in header files. That way madness lies.
   if (isHeaderFile(SM.getFileEntryForID(SM.getMainFileID())->getName(),
@@ -243,15 +253,25 @@
   if (auto *D = Node->ASTNode.get()) {
 if (auto *II = D->getDecl()->getIdentifier()) {
   QualifierToRemove = D->getQualifierLoc();
-  Name = II->getName();
+  Name = II->getName().str();
 }
   } else if (auto *T = Node->ASTNode.get()) {
 if (auto E = T->getAs()) {
-  if (auto *BaseTypeIdentifier =
-  E.getType().getUnqualifiedType().getBaseTypeIdentifier()) {
-Name = BaseTypeIdentifier->getName();
-QualifierToRemove = E.getQualifierLoc();
-  }
+  QualifierToRemove = E.getQualifierLoc();
+
+  auto SpelledTokens =
+  TB.spelledForExpanded(TB.expandedTokens(E.getSourceRange()));
+  if (!SpelledTokens)
+return false;
+  auto SpelledRange = syntax::Token::range(SM, SpelledTokens->front(),
+   SpelledTokens->back());
+  llvm::StringRef NameRef = SpelledRange.text(SM);
+
+  std::string QualifierToRemoveStr = GetNNSLAsString(
+  QualifierToRemove, Inputs.AST->getASTContext().getPrintingPolicy());
+  if (!NameRef.consume_front(QualifierToRemoveStr))
+return false; // What's spelled doesn't match the qualifier.
+  Name = NameRef.str();
 }
   }
 
@@ -283,20 +303,13 @@
 
 Expected AddUsing::apply(const Selection &Inputs) {
   auto &SM = Inputs.AST->getSourceManager();
-  auto &TB = Inputs.AST->getTokens();
 
-  // Determine the length of the qualifier under the cursor, then remove it.
-  auto SpelledTokens = TB.spelledForExpanded(
-  TB.expandedTokens(QualifierToRemove.getSourceRange()));
-  if (!SpelledTokens) {
-return error("Could not determine length of the qualifier");
-  }
-  unsigned Length =
-  syntax::Token::range(SM, SpelledTokens->front(), SpelledTokens->back())
-  .length();
+  std::string QualifierToRemoveStr = GetNNSLAsString(
+  QualifierToRemove, Inputs.AST->getASTContext().getPrintingPolicy());
   tooling::Replacements R;
   if (auto Err = R.add(tooling::Replacement(
-  SM, SpelledTokens->front().location(), Length, "

[PATCH] D72184: [BPF] support atomic instructions

[Brendan Jackman via Phabricator via cfe-commits]

jackmanb added a comment.

I thought a little more about something I was saying in the office hours.

I'm pretty sure GCC's `__atomic_store(&x, &y, order)` should fail to compile 
for anything other than `order=__ATOMIC_RELAXED`, since with the current kernel 
patchset we have `BPF_SET` (which is called `BPF_XCHG` in this LLVM patch) 
implemented with the kernel's `atomic_store`, which _doesn't imply a barrier_.

One alternative solution might be just to scrap `BPF_SET` without `BPF_FETCH` 
(`BPF_SET | BPF_FETCH` is implemented with `atomic_xchg` which _does_ imply a 
barrier IIUC).  As we discussed in the office hours, `BPF_CMPSET` (called 
`BPF_CMPXCHG` in this LLVM patch) won't be supported without `BPF_FETCH`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72184

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

[PATCH] D72184: [BPF] support atomic instructions

[Brendan Jackman via Phabricator via cfe-commits]

jackmanb added a comment.

BTW to investigate my previous comment I tried compiling this code:

  // SPDX-License-Identifier: GPL-2.0
  #include 
  #include 
  #include 
  
  __u64 add64_value = 1;
  __u64 add64_result;
  __u32 add32_value = 1;
  __u32 add32_result;
  __u64 add_stack_value_copy;
  __u64 add_stack_result;
  SEC("fentry/bpf_fentry_test1")
  int BPF_PROG(add, int a)
  {
  __u64 add_stack_value = 1;
  
  add64_result = __sync_fetch_and_add(&add64_value, 2);
  add32_result = __sync_fetch_and_add(&add32_value, 2);
  add_stack_result = __sync_fetch_and_add(&add_stack_value, 2);
  add_stack_value_copy = add_stack_value;
  
  return 0;
  }
  
  __u64 cmpxchg64_value = 1;
  __u64 cmpxchg64_result_fail;
  __u64 cmpxchg64_result_succeed;
  __u32 cmpxchg32_value = 1;
  __u32 cmpxchg32_result_fail;
  __u32 cmpxchg32_result_succeed;
  SEC("fentry/bpf_fentry_test1")
  int BPF_PROG(cmpxchg, int a)
  {
  cmpxchg64_result_fail = __sync_val_compare_and_swap(
  &cmpxchg64_value, 0, 3);
  cmpxchg64_result_succeed = __sync_val_compare_and_swap(
  &cmpxchg64_value, 1, 2);
  
  cmpxchg32_result_fail = __sync_val_compare_and_swap(
  &cmpxchg32_value, 0, 3);
  cmpxchg32_result_succeed = __sync_val_compare_and_swap(
  &cmpxchg32_value, 1, 2);
  
  return 0;
  }
  
  __u64 xchg64_value = 1;
  __u64 xchg64_result;
  __u32 xchg32_value = 1;
  __u32 xchg32_result;
  SEC("fentry/bpf_fentry_test1")
  int BPF_PROG(xchg, int a)
  {
  __u64 val64 = 2;
  __u32 val32 = 2;
  
  __atomic_exchange(&xchg64_value, &val64, &xchg64_result, 
__ATOMIC_RELAXED);
  __atomic_exchange(&xchg32_value, &val32, &xchg32_result, 
__ATOMIC_RELAXED);
  __atomic_store(&xchg64_result, &val64, __ATOMIC_SEQ_CST);
  
  return 0;
  }

(By modifying  this code 

 to add the `__atomic_store` and then running `make -C 
~/src/linux/linux/tools/testing/selftests/bpf -j test_progs` from the base of 
the kernel tree)

And got a crash:

  $ make -C ~/src/linux/linux/tools/testing/selftests/bpf -j test_progs  
  make: Entering directory 
'/usr/local/google/home/jackmanb/src/linux/linux/tools/testing/selftests/bpf'
CLNG-LLC [test_maps] atomics_test.o
  LLVM ERROR: Cannot select: 0x5638e9444528: ch = AtomicStore<(store seq_cst 8 
into @xchg64_result)> 0x5638e94445f8, 0x5638e9444660, Constant:i64<2>, 
progs/atomics_test.c:59:2 @[ progs/atomics_test.c:52:5 ]
0x5638e9444660: i64 = BPFISD::Wrapper TargetGlobalAddress:i64 0, progs/atomics_test.c:57:2 @[ progs/atomics_test.c:52:5 ]
  0x5638e9444b40: i64 = TargetGlobalAddress 0, 
progs/atomics_test.c:57:2 @[ progs/atomics_test.c:52:5 ]
0x5638e94628c0: i64 = Constant<2>
  In function: xchg
  PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace.
  Stack dump:
  0.  Program arguments: llc -mattr=dwarfris -march=bpf -mcpu=v4 
-mattr=+alu32 -filetype=obj -o 
/usr/local/google/home/jackmanb/src/linux/linux/tools/testing/selftests/bpf/atomics_test.o
  1.  Running pass 'Function Pass Manager' on module ''.
  2.  Running pass 'BPF DAG->DAG Pattern Instruction Selection' on function 
'@xchg'
   #0 0x5638e543ae1d llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
(/usr/local/bin/llc+0x26f4e1d)
   #1 0x5638e5438c14 llvm::sys::RunSignalHandlers() 
(/usr/local/bin/llc+0x26f2c14)
   #2 0x5638e5438d70 SignalHandler(int) (/usr/local/bin/llc+0x26f2d70)
   #3 0x7ff91703b140 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x14140)
   #4 0x7ff916b1edb1 raise ./signal/../sysdeps/unix/sysv/linux/raise.c:51:1
   #5 0x7ff916b08537 abort ./stdlib/abort.c:81:7
   #6 0x5638e53b393c llvm::report_fatal_error(llvm::Twine const&, bool) 
(/usr/local/bin/llc+0x266d93c)
   #7 0x5638e53b3a6e (/usr/local/bin/llc+0x266da6e)
   #8 0x5638e527bac0 llvm::SelectionDAGISel::CannotYetSelect(llvm::SDNode*) 
(/usr/local/bin/llc+0x2535ac0)
   #9 0x5638e527c8b1 
llvm::SelectionDAGISel::SelectCodeCommon(llvm::SDNode*, unsigned char const*, 
unsigned int) (/usr/local/bin/llc+0x25368b1)
  #10 0x5638e527a5d7 llvm::SelectionDAGISel::DoInstructionSelection() 
(/usr/local/bin/llc+0x25345d7)
  #11 0x5638e52833f5 llvm::SelectionDAGISel::CodeGenAndEmitDAG() 
(/usr/local/bin/llc+0x253d3f5)
  #12 0x5638e5287471 
llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&) 
(/usr/local/bin/llc+0x2541471)
  #13 0x5638e5289dbc 
llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&) (.part.0) 
(/usr/local/bin/llc+0x2543dbc)
  #14 0x5638e491a5d4 
llvm::MachineFunctionPass::runOnFunction(llvm::Function&) 
(/usr/local/bin/llc+0x1bd45d4)
  #15 0x5638e4d0a050 llvm::FPPassManager::runOnFunct

[clang-tools-extra] 76bd444 - Fix tests for clang-query completion

[Stephen Kelly via cfe-commits]

Author: Stephen Kelly
Date: 2020-11-23T15:23:13Z
New Revision: 76bde36197465f1c72f4b6f1d59721012a59

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

LOG: Fix tests for clang-query completion

Added: 


Modified: 
clang-tools-extra/unittests/clang-query/QueryParserTest.cpp

Removed: 




diff  --git a/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp 
b/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
index 4a0a80146af4..78d6f593777d 100644
--- a/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
+++ b/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
@@ -232,14 +232,12 @@ TEST_F(QueryParserTest, Complete) {
   EXPECT_EQ("dump", Comps[3].DisplayText);
 
   Comps = QueryParser::complete("set traversal ", 14, QS);
-  ASSERT_EQ(3u, Comps.size());
+  ASSERT_EQ(2u, Comps.size());
 
   EXPECT_EQ("AsIs ", Comps[0].TypedText);
   EXPECT_EQ("AsIs", Comps[0].DisplayText);
-  EXPECT_EQ("IgnoreImplicitCastsAndParentheses ", Comps[1].TypedText);
-  EXPECT_EQ("IgnoreImplicitCastsAndParentheses", Comps[1].DisplayText);
-  EXPECT_EQ("IgnoreUnlessSpelledInSource ", Comps[2].TypedText);
-  EXPECT_EQ("IgnoreUnlessSpelledInSource", Comps[2].DisplayText);
+  EXPECT_EQ("IgnoreUnlessSpelledInSource ", Comps[1].TypedText);
+  EXPECT_EQ("IgnoreUnlessSpelledInSource", Comps[1].DisplayText);
 
   Comps = QueryParser::complete("match while", 11, QS);
   ASSERT_EQ(1u, Comps.size());



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

[PATCH] D90188: Add support for attribute 'using_if_exists'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:442
+// Make the selection of the recovery decl deterministic.
+(RealRes)->getLocation().getRawEncoding() <
   IIDecl->getLocation().getRawEncoding())





Comment at: clang/lib/Sema/SemaDecl.cpp:1173
+  if (auto *EmptyD = dyn_cast(FirstDecl)) {
+DiagnoseUseOfDecl(EmptyD, NameLoc);
+return NameClassification::Error();





Comment at: clang/lib/Sema/SemaExpr.cpp:3175
 
+  ValueDecl *VD = cast(D);
+

`auto *VD = cast(D);`



Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:1704-1707
+void ASTDeclReader::VisitUnresolvedUsingIfExistsDecl(
+UnresolvedUsingIfExistsDecl *D) {
+  VisitNamedDecl(D);
+}

I'm not super familiar with this code, but is this change actually necessary? I 
would have expected this to be handled via `VisitNamedDecl` by virtue of 
`UnresolvedUsingIfExistsDecl` inheriting from `NamedDecl`.


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

https://reviews.llvm.org/D90188

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

[PATCH] D91315: [RISCV] Handle zfh in the arch string.

[Luís Marques via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91315

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

Re: [clang-tools-extra] 76bd444 - Fix tests for clang-query completion

[Nico Weber via cfe-commits]

Thanks for fixing the tests, but didn't the tests point out a true
regression in clang-query functionality here?

On Mon, Nov 23, 2020 at 10:23 AM Stephen Kelly via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Stephen Kelly
> Date: 2020-11-23T15:23:13Z
> New Revision: 76bde36197465f1c72f4b6f1d59721012a59
>
> URL:
> https://github.com/llvm/llvm-project/commit/76bde36197465f1c72f4b6f1d59721012a59
> DIFF:
> https://github.com/llvm/llvm-project/commit/76bde36197465f1c72f4b6f1d59721012a59.diff
>
> LOG: Fix tests for clang-query completion
>
> Added:
>
>
> Modified:
> clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
> b/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
> index 4a0a80146af4..78d6f593777d 100644
> --- a/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
> +++ b/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
> @@ -232,14 +232,12 @@ TEST_F(QueryParserTest, Complete) {
>EXPECT_EQ("dump", Comps[3].DisplayText);
>
>Comps = QueryParser::complete("set traversal ", 14, QS);
> -  ASSERT_EQ(3u, Comps.size());
> +  ASSERT_EQ(2u, Comps.size());
>
>EXPECT_EQ("AsIs ", Comps[0].TypedText);
>EXPECT_EQ("AsIs", Comps[0].DisplayText);
> -  EXPECT_EQ("IgnoreImplicitCastsAndParentheses ", Comps[1].TypedText);
> -  EXPECT_EQ("IgnoreImplicitCastsAndParentheses", Comps[1].DisplayText);
> -  EXPECT_EQ("IgnoreUnlessSpelledInSource ", Comps[2].TypedText);
> -  EXPECT_EQ("IgnoreUnlessSpelledInSource", Comps[2].DisplayText);
> +  EXPECT_EQ("IgnoreUnlessSpelledInSource ", Comps[1].TypedText);
> +  EXPECT_EQ("IgnoreUnlessSpelledInSource", Comps[1].DisplayText);
>
>Comps = QueryParser::complete("match while", 11, QS);
>ASSERT_EQ(1u, Comps.size());
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91695: [ARM][AArch64] Adding Neoverse N2 CPU support

[Mark Murray via Phabricator via cfe-commits]

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

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91695

Files:
  clang/test/Driver/aarch64-cpus.c
  clang/test/Driver/arm-cortex-cpus.c
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/include/llvm/Support/ARMTargetParser.def
  llvm/lib/Support/Host.cpp
  llvm/lib/Target/AArch64/AArch64.td
  llvm/lib/Target/AArch64/AArch64Subtarget.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMSubtarget.cpp
  llvm/lib/Target/ARM/ARMSubtarget.h
  llvm/test/CodeGen/AArch64/cpus.ll
  llvm/test/CodeGen/AArch64/neon-dot-product.ll
  llvm/test/CodeGen/AArch64/remat.ll
  llvm/test/MC/AArch64/armv8.2a-dotprod.s
  llvm/test/MC/AArch64/armv8.3a-rcpc.s
  llvm/test/MC/AArch64/armv8.5a-ssbs.s
  llvm/test/MC/ARM/armv8.2a-dotprod-a32.s
  llvm/test/MC/ARM/armv8.2a-dotprod-t32.s
  llvm/test/MC/Disassembler/AArch64/armv8.3a-rcpc.txt
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -280,6 +280,12 @@
 ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP | ARM::AEK_FP16 |
 ARM::AEK_RAS | ARM::AEK_DOTPROD,
 "8.2-A"));
+  EXPECT_TRUE(testARMCPU("neoverse-n2", "armv8.5-a", "crypto-neon-fp-armv8",
+ ARM::AEK_CRC | ARM::AEK_HWDIVTHUMB | ARM::AEK_HWDIVARM |
+ ARM::AEK_MP | ARM::AEK_SEC | ARM::AEK_VIRT |
+ ARM::AEK_DSP | ARM::AEK_BF16 | ARM::AEK_DOTPROD |
+ ARM::AEK_RAS | ARM::AEK_I8MM | ARM::AEK_SB,
+"8.5-A"));
   EXPECT_TRUE(testARMCPU("neoverse-v1", "armv8.4-a", "crypto-neon-fp-armv8",
  ARM::AEK_SEC | ARM::AEK_MP | ARM::AEK_VIRT |
  ARM::AEK_HWDIVARM | ARM::AEK_HWDIVTHUMB |
@@ -328,7 +334,7 @@
  "7-S"));
 }
 
-static constexpr unsigned NumARMCPUArchs = 90;
+static constexpr unsigned NumARMCPUArchs = 91;
 
 TEST(TargetParserTest, testARMCPUArchList) {
   SmallVector List;
@@ -996,6 +1002,15 @@
   AArch64::AEK_PROFILE | AArch64::AEK_RAS | AArch64::AEK_RCPC |
   AArch64::AEK_RDM | AArch64::AEK_SIMD | AArch64::AEK_SSBS,
  "8.2-A"));
+  EXPECT_TRUE(testAArch64CPU(
+ "neoverse-n2", "armv8.5-a", "crypto-neon-fp-armv8",
+  AArch64::AEK_CRC | AArch64::AEK_CRYPTO | AArch64::AEK_FP |
+  AArch64::AEK_SIMD | AArch64::AEK_FP16 | AArch64::AEK_RAS |
+  AArch64::AEK_LSE | AArch64::AEK_SVE | AArch64::AEK_DOTPROD |
+  AArch64::AEK_RCPC | AArch64::AEK_RDM | AArch64::AEK_MTE |
+  AArch64::AEK_SSBS | AArch64::AEK_SB | AArch64::AEK_SVE2 |
+  AArch64::AEK_SVE2BITPERM | AArch64::AEK_BF16 | AArch64::AEK_I8MM,
+ "8.5-A"));
   EXPECT_TRUE(testAArch64CPU(
   "thunderx2t99", "armv8.1-a", "crypto-neon-fp-armv8",
   AArch64::AEK_CRC | AArch64::AEK_CRYPTO | AArch64::AEK_LSE |
@@ -1048,7 +1063,7 @@
   "8.2-A"));
 }
 
-static constexpr unsigned NumAArch64CPUArchs = 44;
+static constexpr unsigned NumAArch64CPUArchs = 45;
 
 TEST(TargetParserTest, testAArch64CPUArchList) {
   SmallVector List;
Index: llvm/test/MC/Disassembler/AArch64/armv8.3a-rcpc.txt
===
--- llvm/test/MC/Disassembler/AArch64/armv8.3a-rcpc.txt
+++ llvm/test/MC/Disassembler/AArch64/armv8.3a-rcpc.txt
@@ -9,6 +9,7 @@
 # RUN: llvm-mc -triple aarch64-none-linux-gnu -mcpu=cortex-x1 --disassemble < %s | FileCheck %s
 # RUN: llvm-mc -triple aarch64-none-linux-gnu -mcpu=neoverse-e1 --disassemble < %s | FileCheck %s
 # RUN: llvm-mc -triple aarch64-none-linux-gnu -mcpu=neoverse-n1 --disassemble < %s | FileCheck %s
+# RUN: llvm-mc -triple aarch64-none-linux-gnu -mcpu=neoverse-n2 --disassemble < %s | FileCheck %s
 
 # CHECK: ldaprb w0, [x0]
 # CHECK: ldaprh w0, [x0]
Index: llvm/test/MC/ARM/armv8.2a-dotprod-t32.s
===
--- llvm/test/MC/ARM/armv8.2a-dotprod-t32.s
+++ llvm/test/MC/ARM/armv8.2a-dotprod-t32.s
@@ -6,6 +6,7 @@
 // RUN: llvm-mc -triple thumb -mcpu=cortex-a78 -show-encoding < %s | FileCheck %s --check-prefix=CHECK
 // RUN: llvm-mc -triple thumb -mcpu=cortex-x1 -show-encoding < %s | FileCheck %s --check-prefix=CHECK
 // RUN: llvm-mc -triple thumb -mcpu=neoverse-n1 -show-encoding < %s | FileCheck %s --check-prefix=CHECK
+// RUN: llvm-mc -triple thumb -mcpu=neoverse-n2 -show-encoding < %s | FileCheck %s --check-prefix=CHECK
 
 // RUN: not llvm-mc -triple thumb -mattr=-dotprod -show-encoding < %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-ERROR < %t %s
Index: llvm/test/MC/ARM/armv8.2a-dotprod-a32.s
==

Re: [clang-tools-extra] 76bd444 - Fix tests for clang-query completion

[Aaron Ballman via cfe-commits]

On Mon, Nov 23, 2020 at 10:34 AM Nico Weber via cfe-commits
 wrote:
>
> Thanks for fixing the tests, but didn't the tests point out a true regression 
> in clang-query functionality here?

We removed the IgnoreImplicitCastsAndParentheses traversal mode (it
was poorly supported and obviated by IgnoreUnlessSpelledInSource), so
these test changes look correct to me.

~Aaron

>
> On Mon, Nov 23, 2020 at 10:23 AM Stephen Kelly via cfe-commits 
>  wrote:
>>
>>
>> Author: Stephen Kelly
>> Date: 2020-11-23T15:23:13Z
>> New Revision: 76bde36197465f1c72f4b6f1d59721012a59
>>
>> URL: 
>> https://github.com/llvm/llvm-project/commit/76bde36197465f1c72f4b6f1d59721012a59
>> DIFF: 
>> https://github.com/llvm/llvm-project/commit/76bde36197465f1c72f4b6f1d59721012a59.diff
>>
>> LOG: Fix tests for clang-query completion
>>
>> Added:
>>
>>
>> Modified:
>> clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
>>
>> Removed:
>>
>>
>>
>> 
>> diff  --git a/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp 
>> b/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
>> index 4a0a80146af4..78d6f593777d 100644
>> --- a/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
>> +++ b/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
>> @@ -232,14 +232,12 @@ TEST_F(QueryParserTest, Complete) {
>>EXPECT_EQ("dump", Comps[3].DisplayText);
>>
>>Comps = QueryParser::complete("set traversal ", 14, QS);
>> -  ASSERT_EQ(3u, Comps.size());
>> +  ASSERT_EQ(2u, Comps.size());
>>
>>EXPECT_EQ("AsIs ", Comps[0].TypedText);
>>EXPECT_EQ("AsIs", Comps[0].DisplayText);
>> -  EXPECT_EQ("IgnoreImplicitCastsAndParentheses ", Comps[1].TypedText);
>> -  EXPECT_EQ("IgnoreImplicitCastsAndParentheses", Comps[1].DisplayText);
>> -  EXPECT_EQ("IgnoreUnlessSpelledInSource ", Comps[2].TypedText);
>> -  EXPECT_EQ("IgnoreUnlessSpelledInSource", Comps[2].DisplayText);
>> +  EXPECT_EQ("IgnoreUnlessSpelledInSource ", Comps[1].TypedText);
>> +  EXPECT_EQ("IgnoreUnlessSpelledInSource", Comps[1].DisplayText);
>>
>>Comps = QueryParser::complete("match while", 11, QS);
>>ASSERT_EQ(1u, Comps.size());
>>
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91695: [ARM][AArch64] Adding Neoverse N2 CPU support

[Dave Green via Phabricator via cfe-commits]

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

Thanks for the changes. From what I can tell, this LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91695

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

[PATCH] D90928: [OpenCL] Check for extension string extension lookup

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D90928#2408249 , @erik2020 wrote:

> In D90928#2405796 , @Anastasia wrote:
>
>> Do you think we could improve testing? I presume there is something that 
>> triggers a failure without your change...
>
> I'm not really sure how to test this code. Best I can tell, there's no way 
> for the `clang` executable to call these functions with invalid strings. I 
> only ran into the seg faults when I was programmatically setting options 
> using the clang API.

Oh, I see. We could also create a g-test for this but it's probably not worth 
enough. Perhaps if you just update a comment that the API behavior becomes 
clear it should be good enough.




Comment at: clang/include/clang/Basic/OpenCLOptions.h:47
   bool isSupported(llvm::StringRef Ext, const LangOptions &LO) const {
+auto E = OptMap.find(Ext);
+if (E == OptMap.end()) {

Btw how about we use `isKnown` instead because it does exactly that? Also, I 
think we should update the comment to explain this change in the API behavior 
and add a comment for `isKnown`. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90928

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

[PATCH] D72184: [BPF] support atomic instructions

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

Ya, the above llvm crash is expected as bpf backend does not handle AtomicStore.

For kernel code, I can see:

  kvm/x86.c:  vcpu->arch.nmi_pending += atomic_xchg(&vcpu->arch.nmi_queued, 
0);

...

  kvm/x86.c:  atomic_set(&kvm_guest_has_master_clock, 1);

So for atomic_set we do not return a value, right?

I did not see kernel has atomic_store, do you mean atomic_set?

Do you suggest we also implement atomic_set? There is no need for 64-bit 
architecture like x64, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72184

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

[PATCH] D88220: [C++20] P1825R0: More implicit moves

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:4626
 CES_AllowExceptionVariables = 4,
-CES_FormerDefault = (CES_AllowParameters),
-CES_Default = (CES_AllowParameters | CES_AllowDifferentTypes),
-CES_AsIfByStdMove = (CES_AllowParameters | CES_AllowDifferentTypes |
- CES_AllowExceptionVariables),
+CES_AllowRValueReferenceType = 8,
+CES_ImplicitlyMovableCXX11CXX14CXX17 =

I believe `RValue` should be spelled `Rvalue`, throughout.



Comment at: clang/include/clang/Sema/Sema.h:4634
+(CES_AllowParameters | CES_AllowDifferentTypes |
+ CES_AllowExceptionVariables | CES_AllowRValueReferenceType),
   };

Unless I'm mistaken, `CES_AsIfByStdMove` is now a synonym for 
`CES_ImplicitlyMovableCXX20`. Could we discuss the pros and cons of simply 
removing `CES_AsIfByStdMove`? Is there some difference in connotation here, 
like maybe we are expecting them to diverge again in C++23 or beyond?



Comment at: clang/lib/Sema/SemaStmt.cpp:3141
 
 FunctionDecl *FD = Step.Function.Function;
 if (ConvertingConstructorsOnly) {

This loop is hard to understand before your patch. I don't think you made it 
any worse. But, consider whether you could pull out the condition so that this 
part became something like

FunctionDecl *FD = Step.Function.Function;
if (!IsSuitableConversionForImplicitMove(FD, NRVOCandidate, 
ConvertingConstructorsOnly)) {
  break;
}
// Promote "AsRvalue" to the heap...

Actually, I don't even understand why `continue;` is being used in the old 
code. Doesn't that mean "skip this step and go on to the next step"?



Comment at: clang/test/CXX/class/class.init/class.copy.elision/p3.cpp:59
+  } catch (C c_move) {
+throw c_move; // expected-error {{calling a private constructor of class 
'test_throw_parameter::C'}}
+  }

(1) I would prefer to [additionally?] see a test case using `=delete`, rather 
than having the member exist but be private. We expect the same behavior in 
both cases, right? but `=delete` is more like the code we hope people are 
writing in practice.

(2) Is there any simpler way to write the "pre-C++20" and "C++20" versions of 
this test? It's ugly to repeat the entire class body in both branches of the 
`#if`, when the only difference is whether the "expected-note" is present or 
not.



Comment at: clang/test/CXX/class/class.init/class.copy.elision/p3.cpp:92
+  NeedRvalueRef() {}
+  ~NeedRvalueRef() {}
+  NeedRvalueRef(B &&);

In all of these tests, I don't see any reason to have `{}` when `;` would 
suffice; and I don't think you need the destructor to be explicitly declared at 
all; and in fact for `NeedRvalueRef` and `NeedValue`, I think just

struct NeedRvalueRef {
NeedRvalueRef(B&&);
};

would suffice, right?



Comment at: clang/test/SemaCXX/warn-return-std-move.cpp:87
 // expected-note@-2{{to avoid copying}}
 // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d4)"
 }

I would like to see a copy of this file, titled something like 
`warn-return-std-move-cxx20.cpp`, updated to run with `-std=c++20` instead of 
`-std=c++14`. (Initially I thought that the lines above indicated a flaw in 
your patch; it took me a while to realize that this test file is compiled only 
with `-std=c++14`. In C++20, we expect that `ConstructFromBase(Base&&)` will be 
called and the expected-warning will not be printed, right?)

I would also appreciate either seeing an actual test file with all the test 
cases from http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1155r2.html 
, or at least hearing (in reply to this comment) that you've tested them all 
and you believe their behavior is correct.

It looks to me as if every P1155r2 case is handled correctly **except** for 
this one:

struct To {
operator Widget() const &;  // "copy"
operator Widget() &&;  // "move"
};
Widget nine() {
To t;
return t;  // C++17 copies; C++20 should move (but this patch still 
copies)
}



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

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

[PATCH] D72184: [BPF] support atomic instructions

[Brendan Jackman via Phabricator via cfe-commits]

jackmanb added a comment.

> I did not see kernel has atomic_store, do you mean atomic_set?

Sorry yep I meant `atomic_set`

> Do you suggest we also implement atomic_set? There is no need for 64-bit 
> architecture like x64, right?

Yeah actually now I think about it, `atomic_set` is pretty pointless, I think 
it's only there in the kernel to support strong type-checking of `atomic_t`; It 
doesn't imply any barriers.

I think we can do without it, it makes more sense to implement alongside 
barrier instructions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72184

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

[PATCH] D91531: [RFC][OpenCL] Provide mechanisms for defining extension macros

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D91531#2406390 , @azabaznov wrote:

> Yes, in general this approach looks good to me conceptually. I have two 
> suggestions:
>
> 1. As we discussed, the term //core functionality// should be revisited here. 
> There's no clear meaning about that in the spec and I think interpreting it 
> as //supported by default// is a little dangerous. So //core// (AFAIK) means 
> that it was just promoted to a core specification thus is still remains 
> optional by targets.

Btw I think there is a core and an optional core extension too? I believe that 
the definition that you are providing applies to the optional core extension 
but not core extension. However I have created this issue as I think the spec 
should be explicit about all of these: 
https://github.com/KhronosGroup/OpenCL-Docs/issues/500.

> 1. Sort of a implementation suggestion. I understand that double-scored 
> identifiers are reserved for any use, but still, can defining such macro as 
> `__undef_cl_khr_depth_images ` be avoided? We could use `Preproceccor` class 
> for the things that you are proposing to do. I was trying to do something 
> similar when implementing features and I tried something like 
> (`Preprocessor::appendDefMacroDirective` already exists):
>
>
>
>   UndefMacroDirective *Preprocessor::appendUndefMacroDirective(IdentifierInfo 
> *II,
>  SourceLocation Loc) {
>  if (!II->hasMacroDefinition())
>return nullptr;
>  UndefMacroDirective *UD = AllocateUndefMacroDirective(Loc);
>  appendMacroDirective(II, UD);
>  return UD;
>   }
>
> I tried to handle some special pragma in this way and it worked. So maybe 
> this can be reused without binding to any specific `SourceLocation`? But 
> maybe there is an other strong concern why 
> `Preprocessor::appendUndefMacroDirective` still doesn't exist...

As far as I can see `Preprocessor::appendDefMacroDirective` is mainly used for 
the extension 
https://gcc.gnu.org/onlinedocs/gcc/Push_002fPop-Macro-Pragmas.html. It seems 
interesting but I am not sure yet if it can help.  So what do you plan to do 
with `Preprocessor::appendUndefMacroDirective`? Perhaps if you give me an 
example it would help to understand.

I agree that `__undef` macro is a bit messy. We could of course make it a bit 
prettier. For example,

(1.) we could add a helper macro

  #define DEFINE_EXTENSION_MACRO(NAME) \
  #if !defined(NAME) && !defined(__undef_ ## NAME)\
  #define NAME \
  #endif

Then we could make a definition of `cl_khr_depth_images` simpler

  #if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ == 
CL_VERSION_2_0) || \
  (__OPENCL_C_VERSION__ >= CL_VERSION_1_2  && defined(__SPIR__) )
  DEFINE_EXTENSION_MACRO(cl_khr_depth_images)
  #endif

or (2.) just do something like the following at the end of all 
extension/feature macro setting code.

  #if defined(__undef_cl_khr_depth_images)
  #undef cl_khr_depth_images
  #endif


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

https://reviews.llvm.org/D91531

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

[PATCH] D90282: [clang-tidy] Add IgnoreShortNames config to identifier naming checks

[Nathan James via Phabricator via cfe-commits]

njames93 accepted this revision.
njames93 added a comment.

LGTM, before you push this, can you edit the summary and title as its no longer 
about just the length of the name. Otherwise the commit message will likely 
lead to confusion on whats being added.


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

https://reviews.llvm.org/D90282

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

[PATCH] D91898: [attributes] Add a facility for defining and enforcing a Trusted Computing Base.

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

Thread safety attributes want **callers** of a function to have the same 
attribute, while this change wants **callees** to have the same attribute. So 
the attributes propagate in different directions.

By contraposition  the absence of 
an attribute propagates the other way around as the attribute itself, so you 
could have a role "untrusted", and callers of untrusted functions would have to 
be untrusted as well.

I guess it depends on how many functions need to be annotated one way or the 
other, if the TCB-based functions are a small subset of the code then this 
attribute is better, if most functions are based on the TCB and only some are 
not, the capability-based approach would be better.


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

https://reviews.llvm.org/D91898

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

[PATCH] D91974: [PowerPC] Rename the pair intrinsics and builtins to replace the _mma_ prefix by _vsx_

[Baptiste Saleil via Phabricator via cfe-commits]

bsaleil created this revision.
bsaleil added reviewers: nemanjai, amyk, saghir, lei, PowerPC.
bsaleil added projects: PowerPC, LLVM.
Herald added subscribers: llvm-commits, cfe-commits, shchenz, kbarton, 
hiraditya.
Herald added a project: clang.
bsaleil requested review of this revision.

On PPC, the vector pair instructions are independent from MMA.
This patch renames the vector pair LLVM intrinsics and Clang builtins to 
replace the `_mma_` prefix by `_vsx_` in their names.
We also move the vector pair type/intrinsic/builtin tests to their own files.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91974

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-ppc-mma.c
  clang/test/CodeGen/builtins-ppc-pair-mma.c
  clang/test/Sema/ppc-mma-types.c
  clang/test/Sema/ppc-pair-mma-types.c
  clang/test/SemaCXX/ppc-mma-types.cpp
  clang/test/SemaCXX/ppc-pair-mma-types.cpp
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/lib/Target/PowerPC/PPCLoopInstrFormPrep.cpp
  llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
  llvm/test/CodeGen/PowerPC/dform-pair-load-store.ll
  llvm/test/CodeGen/PowerPC/loop-p10-pair-prepare.ll
  llvm/test/CodeGen/PowerPC/mma-intrinsics.ll
  llvm/test/CodeGen/PowerPC/mma-outer-product.ll
  llvm/test/CodeGen/PowerPC/paired-vector-intrinsics-without-mma.ll
  llvm/test/CodeGen/PowerPC/paired-vector-intrinsics.ll

Index: llvm/test/CodeGen/PowerPC/paired-vector-intrinsics.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/paired-vector-intrinsics.ll
@@ -0,0 +1,356 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -O3 \
+; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr \
+; RUN:   < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -O3 \
+; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr -mattr=-mma \
+; RUN:   < %s | FileCheck %s --check-prefix=CHECK-NOMMA
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu -O3 \
+; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr \
+; RUN:   < %s | FileCheck %s --check-prefix=CHECK-BE
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu -O3 \
+; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr -mattr=-mma \
+; RUN:   < %s | FileCheck %s --check-prefix=CHECK-BE-NOMMA
+
+; This test is to check that the paired vector intrinsics are available even
+
+; assemble_pair
+declare <256 x i1> @llvm.ppc.vsx.assemble.pair(<16 x i8>, <16 x i8>)
+define void @ass_pair(<256 x i1>* %ptr, <16 x i8> %vc) {
+; CHECK-LABEL: ass_pair:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vmr v3, v2
+; CHECK-NEXT:stxv v2, 16(r3)
+; CHECK-NEXT:stxv v3, 0(r3)
+; CHECK-NEXT:blr
+;
+; CHECK-NOMMA-LABEL: ass_pair:
+; CHECK-NOMMA:   # %bb.0: # %entry
+; CHECK-NOMMA-NEXT:vmr v3, v2
+; CHECK-NOMMA-NEXT:stxv v2, 16(r3)
+; CHECK-NOMMA-NEXT:stxv v3, 0(r3)
+; CHECK-NOMMA-NEXT:blr
+;
+; CHECK-BE-LABEL: ass_pair:
+; CHECK-BE:   # %bb.0: # %entry
+; CHECK-BE-NEXT:vmr v3, v2
+; CHECK-BE-NEXT:stxv v2, 16(r3)
+; CHECK-BE-NEXT:stxv v2, 0(r3)
+; CHECK-BE-NEXT:blr
+;
+; CHECK-BE-NOMMA-LABEL: ass_pair:
+; CHECK-BE-NOMMA:   # %bb.0: # %entry
+; CHECK-BE-NOMMA-NEXT:vmr v3, v2
+; CHECK-BE-NOMMA-NEXT:stxv v2, 16(r3)
+; CHECK-BE-NOMMA-NEXT:stxv v2, 0(r3)
+; CHECK-BE-NOMMA-NEXT:blr
+entry:
+  %0 = tail call <256 x i1> @llvm.ppc.vsx.assemble.pair(<16 x i8> %vc, <16 x i8> %vc)
+  store <256 x i1> %0, <256 x i1>* %ptr, align 32
+  ret void
+}
+
+; disassemble_pair
+declare { <16 x i8>, <16 x i8> } @llvm.ppc.vsx.disassemble.pair(<256 x i1>)
+define void @disass_pair(<256 x i1>* %ptr1, <16 x i8>* %ptr2, <16 x i8>* %ptr3) {
+; CHECK-LABEL: disass_pair:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:lxv vs1, 0(r3)
+; CHECK-NEXT:lxv vs0, 16(r3)
+; CHECK-NEXT:stxv vs1, 0(r4)
+; CHECK-NEXT:stxv vs0, 0(r5)
+; CHECK-NEXT:blr
+;
+; CHECK-NOMMA-LABEL: disass_pair:
+; CHECK-NOMMA:   # %bb.0: # %entry
+; CHECK-NOMMA-NEXT:lxv vs1, 0(r3)
+; CHECK-NOMMA-NEXT:lxv vs0, 16(r3)
+; CHECK-NOMMA-NEXT:stxv vs1, 0(r4)
+; CHECK-NOMMA-NEXT:stxv vs0, 0(r5)
+; CHECK-NOMMA-NEXT:blr
+;
+; CHECK-BE-LABEL: disass_pair:
+; CHECK-BE:   # %bb.0: # %entry
+; CHECK-BE-NEXT:lxv vs1, 16(r3)
+; CHECK-BE-NEXT:lxv vs0, 0(r3)
+; CHECK-BE-NEXT:stxv vs0, 0(r4)
+; CHECK-BE-NEXT:stxv vs1, 0(r5)
+; CHECK-BE-NEXT:blr
+;
+; CHECK-BE-NOMMA-LABEL: disass_pair:
+; CHECK-BE-NOMMA:   # %bb.0: # %entry
+; CHECK-BE-NOMMA-NEXT:lxv vs1, 16(r3)
+; CHECK-BE-NOMMA-NEXT:lxv vs0, 0(r3)
+; CHECK-BE-NOMMA-NEXT:stxv vs0, 

[PATCH] D91531: [RFC][OpenCL] Provide mechanisms for defining extension macros

[Anton Zabaznov via Phabricator via cfe-commits]

azabaznov added a comment.

> Perhaps if you give me an example it would help to understand

I was meant that this can potentially be used to undefine macros inside clang 
directly. In this case there will no need to add a lot of conditional 
preprocessor directives in the header, also the existing interface 
(`-cl-ext=-cl_khr_depth_images`) will be preserved. So for example in the 
header there was specified an extension definition. Can undef directive be 
allocated and bound to a specific source location right after extension 
definition if `-cl-ext=-cl_khr_depth_images` was specifed:

  #if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ == 
CL_VERSION_2_0) || \
  (__OPENCL_C_VERSION__ >= CL_VERSION_1_2  && defined(__SPIR__) )
  #define cl_khr_depth_images 1
  #endif
  
  // Bind undef directive here

I understand that this sounds tricky, but preserving interface sound reasonable 
for me.


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

https://reviews.llvm.org/D91531

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