[clang-tools-extra] r358157 - [clangd] Include compile command heuristic in logs

[Sam McCall via cfe-commits]

Author: sammccall
Date: Thu Apr 11 01:17:15 2019
New Revision: 358157

URL: http://llvm.org/viewvc/llvm-project?rev=358157&view=rev
Log:
[clangd] Include compile command heuristic in logs

Modified:
clang-tools-extra/trunk/clangd/TUScheduler.cpp
clang-tools-extra/trunk/test/clangd/did-change-configuration-params.test

Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.cpp?rev=358157&r1=358156&r2=358157&view=diff
==
--- clang-tools-extra/trunk/clangd/TUScheduler.cpp (original)
+++ clang-tools-extra/trunk/clangd/TUScheduler.cpp Thu Apr 11 01:17:15 2019
@@ -355,7 +355,8 @@ void ASTWorker::update(ParseInputs Input
 FileInputs = Inputs;
 DiagsWereReported = false;
 emitTUStatus({TUAction::BuildingPreamble, TaskName});
-log("Updating file {0} with command [{1}] {2}", FileName,
+log("Updating file {0} with command {1}\n[{2}]\n{3}", FileName,
+Inputs.CompileCommand.Heuristic,
 Inputs.CompileCommand.Directory,
 llvm::join(Inputs.CompileCommand.CommandLine, " "));
 // Rebuild the preamble and the AST.

Modified: 
clang-tools-extra/trunk/test/clangd/did-change-configuration-params.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/did-change-configuration-params.test?rev=358157&r1=358156&r2=358157&view=diff
==
--- clang-tools-extra/trunk/test/clangd/did-change-configuration-params.test 
(original)
+++ clang-tools-extra/trunk/test/clangd/did-change-configuration-params.test 
Thu Apr 11 01:17:15 2019
@@ -41,7 +41,9 @@
 # CHECK-NEXT:"uri": "file://{{.*}}/foo.c"
 # CHECK-NEXT:  }
 #
-# ERR: Updating file {{.*}}foo.c with command [{{.*}}clangd-test2] clang -c 
foo.c -Wall -Werror
+# ERR: Updating file {{.*}}foo.c with command
+# ERR: [{{.*}}clangd-test2]
+# ERR: clang -c foo.c -Wall -Werror
 # Don't reparse the second file:
 # ERR: Skipping rebuild of the AST for {{.*}}bar.c
 ---


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

[PATCH] D60126: [clangd] Use identifiers in file as completion candidates when build is not ready.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 194647.
ioeric marked 14 inline comments as done.
ioeric added a comment.

- address review comments


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60126

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/Headers.cpp
  clangd/Headers.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/index/SymbolOrigin.cpp
  clangd/index/SymbolOrigin.h
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/HeadersTests.cpp
  unittests/clangd/SourceCodeTests.cpp

Index: unittests/clangd/SourceCodeTests.cpp
===
--- unittests/clangd/SourceCodeTests.cpp
+++ unittests/clangd/SourceCodeTests.cpp
@@ -9,6 +9,7 @@
 #include "Context.h"
 #include "Protocol.h"
 #include "SourceCode.h"
+#include "clang/Format/Format.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/raw_os_ostream.h"
 #include "llvm/Testing/Support/Error.h"
@@ -304,6 +305,23 @@
   }
 }
 
+TEST(SourceCodeTests, CollectIdentifiers) {
+  auto Style = format::getLLVMStyle();
+  auto IDs = collectIdentifiers(R"cpp(
+  #include "a.h"
+  void foo() { int xyz; int abc = xyz; return foo(); }
+  )cpp",
+Style);
+  EXPECT_EQ(IDs.size(), 7u);
+  EXPECT_EQ(IDs["include"], 1u);
+  EXPECT_EQ(IDs["void"], 1u);
+  EXPECT_EQ(IDs["int"], 2u);
+  EXPECT_EQ(IDs["xyz"], 2u);
+  EXPECT_EQ(IDs["abc"], 1u);
+  EXPECT_EQ(IDs["return"], 1u);
+  EXPECT_EQ(IDs["foo"], 2u);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/HeadersTests.cpp
===
--- unittests/clangd/HeadersTests.cpp
+++ unittests/clangd/HeadersTests.cpp
@@ -90,7 +90,7 @@
 
 IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
  CDB.getCompileCommand(MainFile)->Directory,
- Clang->getPreprocessor().getHeaderSearchInfo());
+ &Clang->getPreprocessor().getHeaderSearchInfo());
 for (const auto &Inc : Inclusions)
   Inserter.addExisting(Inc);
 auto Declaring = ToHeaderFile(Original);
@@ -110,7 +110,7 @@
 
 IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
  CDB.getCompileCommand(MainFile)->Directory,
- Clang->getPreprocessor().getHeaderSearchInfo());
+ &Clang->getPreprocessor().getHeaderSearchInfo());
 auto Edit = Inserter.insert(VerbatimHeader);
 Action.EndSourceFile();
 return Edit;
@@ -252,6 +252,24 @@
   EXPECT_TRUE(StringRef(Edit->newText).contains(""));
 }
 
+TEST(Headers, NoHeaderSearchInfo) {
+  std::string MainFile = testPath("main.cpp");
+  IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
+   /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr);
+
+  auto HeaderPath = testPath("sub/bar.h");
+  auto Declaring = HeaderFile{HeaderPath, /*Verbatim=*/false};
+  auto Inserting = HeaderFile{HeaderPath, /*Verbatim=*/false};
+  auto Verbatim = HeaderFile{"", /*Verbatim=*/true};
+
+  EXPECT_EQ(Inserter.calculateIncludePath(Declaring, Inserting),
+"\"" + HeaderPath + "\"");
+  EXPECT_EQ(Inserter.shouldInsertInclude(Declaring, Inserting), false);
+
+  EXPECT_EQ(Inserter.calculateIncludePath(Declaring, Verbatim), "");
+  EXPECT_EQ(Inserter.shouldInsertInclude(Declaring, Verbatim), true);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -20,6 +20,7 @@
 #include "TestTU.h"
 #include "index/MemIndex.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
@@ -138,6 +139,25 @@
  FilePath);
 }
 
+// Builds a server and runs code completion.
+// If IndexSymbols is non-empty, an index will be built and passed to opts.
+CodeCompleteResult completionsNoCompile(llvm::StringRef Text,
+std::vector IndexSymbols = {},
+clangd::CodeCompleteOptions Opts = {},
+PathRef FilePath = "foo.cpp") {
+  std::unique_ptr OverrideIndex;
+  if (!IndexSymbols.empty()) {
+assert(!Opts.Index && "both Index and IndexSymbols given!");
+OverrideIndex = memIndex(std::move(IndexSymbols));
+Opts.Index = OverrideIndex.get();
+  }
+
+  MockFSProvider FS;
+  Annotations Test(Text);
+  return codeComplete(FilePath, tooling::CompileCommand(), /*Preamble=*/nul

[PATCH] D60126: [clangd] Use identifiers in file as completion candidates when build is not ready.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/CodeComplete.cpp:1360
 getQueryScopes(Recorder->CCContext, *Recorder->CCSema, Opts);
 if (!QueryScopes.empty())
   ScopeProximity.emplace(QueryScopes);

sammccall wrote:
> add this to the non-sema case too (though the if is always false for now), or 
> add a fixme?
> Worried about forgetting this.
Added a FIXME.



Comment at: clangd/Headers.cpp:196
+  if (!HeaderSearchInfo)
+return "\"" + InsertedHeader.File + "\"";
+  std::string Suggested = HeaderSearchInfo->suggestPathToFileForDiagnostics(

sammccall wrote:
> Do we expect this code path to ever be called?
> We may want to assert or elog here, depending on how this can be hit.
This can be hit when we start serving index symbols in fallback mode 
completion. `shouldInsertInclude` will return false, but the calculation will 
be attempted regardless. We can probably elog this, but the number of log 
messages would be O(# of index symbols), which I worry might be too spammy.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60126



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

[PATCH] D57464: Generalize method overloading on addr spaces to C++

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

In D57464#1438213 , @Anastasia wrote:

> > I think I would lean towards the latter since it means less fudging around 
> > with a whole bunch of unrelated methods. Do @rjmccall or @rsmith have any 
> > further opinions on this?
>
> Ok, I can change the patch to prototype this approach. I might need some 
> example test cases though.


Alright!

Just to make sure of something here - are you waiting for me to provide some 
example cases? There hasn't been activity here in a while and I was just 
wondering if it was because you were waiting for this.


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

https://reviews.llvm.org/D57464



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

[PATCH] D59641: [clangd] Show template argument list in workspacesymbols and documentsymbols responses

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

LG, just have a simple clarifying question.




Comment at: clang-tools-extra/clangd/AST.cpp:112
   if (!Out.str().empty()) {
-// FIXME(ibiryukov): do not show args not explicitly written by the user.
-if (auto *ArgList = getTemplateSpecializationArgs(ND))

Where did this FIXME go? Is this now fixed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59641



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

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[James Henderson via Phabricator via cfe-commits]

jhenderson added a comment.

I've not looked at the implementation in depth, but cl::DefaultOption seems 
like a nice way of handling this. Please make sure that there is testing in 
llvm-readobj and llvm-objdump that test their own short alias interpretation 
somewhere though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

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

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

I have one question, once it is resolved I am fine with committing this.




Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:352
+return true;
+} else if (VD->isStaticDataMember()) {
+  // Only import if const.

Do we actually need this branch? I would expect `cross_tu::containsConst(VD, 
*Ctx)` to return true for all the cases where we would return true here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D46421



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

[PATCH] D60126: [clangd] Use identifiers in file as completion candidates when build is not ready.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clangd/CodeComplete.cpp:345
+  Completion.Origin |= SymbolOrigin::Identifier;
+  Completion.Name = C.IdentifierResult->Name;
+}

Completion.Kind = CompletionItemKind::Text



Comment at: clangd/CodeComplete.cpp:1317
+  ID.References = IDAndCount.second;
+  if (ID.Name == CCPrefix.Name) // Decrement typed filter count.
+--ID.References;

comment should explain why, rather than what :-)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60126



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

[PATCH] D60126: [clangd] Use identifiers in file as completion candidates when build is not ready.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 194652.
ioeric marked 2 inline comments as done.
ioeric added a comment.

- address comments


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60126

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/Headers.cpp
  clangd/Headers.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/index/SymbolOrigin.cpp
  clangd/index/SymbolOrigin.h
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/HeadersTests.cpp
  unittests/clangd/SourceCodeTests.cpp

Index: unittests/clangd/SourceCodeTests.cpp
===
--- unittests/clangd/SourceCodeTests.cpp
+++ unittests/clangd/SourceCodeTests.cpp
@@ -9,6 +9,7 @@
 #include "Context.h"
 #include "Protocol.h"
 #include "SourceCode.h"
+#include "clang/Format/Format.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/raw_os_ostream.h"
 #include "llvm/Testing/Support/Error.h"
@@ -304,6 +305,23 @@
   }
 }
 
+TEST(SourceCodeTests, CollectIdentifiers) {
+  auto Style = format::getLLVMStyle();
+  auto IDs = collectIdentifiers(R"cpp(
+  #include "a.h"
+  void foo() { int xyz; int abc = xyz; return foo(); }
+  )cpp",
+Style);
+  EXPECT_EQ(IDs.size(), 7u);
+  EXPECT_EQ(IDs["include"], 1u);
+  EXPECT_EQ(IDs["void"], 1u);
+  EXPECT_EQ(IDs["int"], 2u);
+  EXPECT_EQ(IDs["xyz"], 2u);
+  EXPECT_EQ(IDs["abc"], 1u);
+  EXPECT_EQ(IDs["return"], 1u);
+  EXPECT_EQ(IDs["foo"], 2u);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/HeadersTests.cpp
===
--- unittests/clangd/HeadersTests.cpp
+++ unittests/clangd/HeadersTests.cpp
@@ -90,7 +90,7 @@
 
 IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
  CDB.getCompileCommand(MainFile)->Directory,
- Clang->getPreprocessor().getHeaderSearchInfo());
+ &Clang->getPreprocessor().getHeaderSearchInfo());
 for (const auto &Inc : Inclusions)
   Inserter.addExisting(Inc);
 auto Declaring = ToHeaderFile(Original);
@@ -110,7 +110,7 @@
 
 IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
  CDB.getCompileCommand(MainFile)->Directory,
- Clang->getPreprocessor().getHeaderSearchInfo());
+ &Clang->getPreprocessor().getHeaderSearchInfo());
 auto Edit = Inserter.insert(VerbatimHeader);
 Action.EndSourceFile();
 return Edit;
@@ -252,6 +252,24 @@
   EXPECT_TRUE(StringRef(Edit->newText).contains(""));
 }
 
+TEST(Headers, NoHeaderSearchInfo) {
+  std::string MainFile = testPath("main.cpp");
+  IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
+   /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr);
+
+  auto HeaderPath = testPath("sub/bar.h");
+  auto Declaring = HeaderFile{HeaderPath, /*Verbatim=*/false};
+  auto Inserting = HeaderFile{HeaderPath, /*Verbatim=*/false};
+  auto Verbatim = HeaderFile{"", /*Verbatim=*/true};
+
+  EXPECT_EQ(Inserter.calculateIncludePath(Declaring, Inserting),
+"\"" + HeaderPath + "\"");
+  EXPECT_EQ(Inserter.shouldInsertInclude(Declaring, Inserting), false);
+
+  EXPECT_EQ(Inserter.calculateIncludePath(Declaring, Verbatim), "");
+  EXPECT_EQ(Inserter.shouldInsertInclude(Declaring, Verbatim), true);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -20,6 +20,7 @@
 #include "TestTU.h"
 #include "index/MemIndex.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
@@ -138,6 +139,25 @@
  FilePath);
 }
 
+// Builds a server and runs code completion.
+// If IndexSymbols is non-empty, an index will be built and passed to opts.
+CodeCompleteResult completionsNoCompile(llvm::StringRef Text,
+std::vector IndexSymbols = {},
+clangd::CodeCompleteOptions Opts = {},
+PathRef FilePath = "foo.cpp") {
+  std::unique_ptr OverrideIndex;
+  if (!IndexSymbols.empty()) {
+assert(!Opts.Index && "both Index and IndexSymbols given!");
+OverrideIndex = memIndex(std::move(IndexSymbols));
+Opts.Index = OverrideIndex.get();
+  }
+
+  MockFSProvider FS;
+  Annotations Test(Text);
+  return codeComplete(FilePath, tooling::CompileCommand(), /*Preamble=*/nullptr,
+ 

[clang-tools-extra] r358159 - [clangd] Use identifiers in file as completion candidates when build is not ready.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Thu Apr 11 02:36:36 2019
New Revision: 358159

URL: http://llvm.org/viewvc/llvm-project?rev=358159&view=rev
Log:
[clangd] Use identifiers in file as completion candidates when build is not 
ready.

Summary:
o Lex the code to get the identifiers and put them into a "symbol" index.
o Adds a new completion mode without compilation/sema into code completion 
workflow.
o Make IncludeInserter work even when no compile command is present, by avoiding
inserting non-verbatim headers.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, jdoerfert, 
cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/CodeComplete.h
clang-tools-extra/trunk/clangd/Headers.cpp
clang-tools-extra/trunk/clangd/Headers.h
clang-tools-extra/trunk/clangd/SourceCode.cpp
clang-tools-extra/trunk/clangd/SourceCode.h
clang-tools-extra/trunk/clangd/index/SymbolOrigin.cpp
clang-tools-extra/trunk/clangd/index/SymbolOrigin.h
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=358159&r1=358158&r2=358159&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Thu Apr 11 02:36:36 2019
@@ -23,7 +23,6 @@
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Lex/Preprocessor.h"
-#include "clang/Sema/CodeCompleteConsumer.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "clang/Tooling/Refactoring/RefactoringResultConsumer.h"
@@ -187,28 +186,23 @@ void ClangdServer::codeComplete(PathRef
   return CB(IP.takeError());
 if (isCancelled())
   return CB(llvm::make_error());
-if (!IP->Preamble) {
-  vlog("File {0} is not ready for code completion. Enter fallback mode.",
-   File);
-  CodeCompleteResult CCR;
-  CCR.Context = CodeCompletionContext::CCC_Recovery;
-
-  // FIXME: perform simple completion e.g. using identifiers in the current
-  // file and symbols in the index.
-  // FIXME: let clients know that we've entered fallback mode.
-
-  return CB(std::move(CCR));
-}
 
 llvm::Optional SpecFuzzyFind;
-if (CodeCompleteOpts.Index && CodeCompleteOpts.SpeculativeIndexRequest) {
-  SpecFuzzyFind.emplace();
-  {
-std::lock_guard 
Lock(CachedCompletionFuzzyFindRequestMutex);
-SpecFuzzyFind->CachedReq = 
CachedCompletionFuzzyFindRequestByFile[File];
+if (!IP->Preamble) {
+  // No speculation in Fallback mode, as it's supposed to be much faster
+  // without compiling.
+  vlog("Build for file {0} is not ready. Enter fallback mode.", File);
+} else {
+  if (CodeCompleteOpts.Index && CodeCompleteOpts.SpeculativeIndexRequest) {
+SpecFuzzyFind.emplace();
+{
+  std::lock_guard Lock(
+  CachedCompletionFuzzyFindRequestMutex);
+  SpecFuzzyFind->CachedReq =
+  CachedCompletionFuzzyFindRequestByFile[File];
+}
   }
 }
-
 // FIXME(ibiryukov): even if Preamble is non-null, we may want to check
 // both the old and the new version in case only one of them matches.
 CodeCompleteResult Result = clangd::codeComplete(

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=358159&r1=358158&r2=358159&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Thu Apr 11 02:36:36 2019
@@ -311,7 +311,7 @@ ParsedAST::build(std::unique_ptr(
 MainInput.getFile(), Content, Style, BuildDir.get(),
-Clang->getPreprocessor().getHeaderSearchInfo());
+&Clang->getPreprocessor().getHeaderSearchInfo());
 if (Preamble) {
   for (const auto &Inc : Preamble->Includes.MainFileIncludes)
 Inserter->addExisting(Inc);

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=358159&r1=358158&r2=358159&view=diff
==
--- clang-tools-extra/trunk/clangd/Co

[PATCH] D60126: [clangd] Use identifiers in file as completion candidates when build is not ready.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE358159: [clangd] Use identifiers in file as completion 
candidates when build is not… (authored by ioeric, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60126?vs=194652&id=194653#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60126

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/Headers.cpp
  clangd/Headers.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/index/SymbolOrigin.cpp
  clangd/index/SymbolOrigin.h
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/HeadersTests.cpp
  unittests/clangd/SourceCodeTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -20,6 +20,7 @@
 #include "TestTU.h"
 #include "index/MemIndex.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
@@ -138,6 +139,25 @@
  FilePath);
 }
 
+// Builds a server and runs code completion.
+// If IndexSymbols is non-empty, an index will be built and passed to opts.
+CodeCompleteResult completionsNoCompile(llvm::StringRef Text,
+std::vector IndexSymbols = {},
+clangd::CodeCompleteOptions Opts = {},
+PathRef FilePath = "foo.cpp") {
+  std::unique_ptr OverrideIndex;
+  if (!IndexSymbols.empty()) {
+assert(!Opts.Index && "both Index and IndexSymbols given!");
+OverrideIndex = memIndex(std::move(IndexSymbols));
+Opts.Index = OverrideIndex.get();
+  }
+
+  MockFSProvider FS;
+  Annotations Test(Text);
+  return codeComplete(FilePath, tooling::CompileCommand(), /*Preamble=*/nullptr,
+  Test.code(), Test.point(), FS.getFileSystem(), Opts);
+}
+
 Symbol withReferences(int N, Symbol S) {
   S.References = N;
   return S;
@@ -2401,6 +2421,33 @@
   UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE";
 }
 
+TEST(NoCompileCompletionTest, Basic) {
+  auto Results = completionsNoCompile(R"cpp(
+void func() {
+  int xyz;
+  int abc;
+  ^
+}
+  )cpp");
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(Named("void"), Named("func"), Named("int"),
+   Named("xyz"), Named("abc")));
+}
+
+TEST(NoCompileCompletionTest, WithFilter) {
+  auto Results = completionsNoCompile(R"cpp(
+void func() {
+  int sym1;
+  int sym2;
+  int xyz1;
+  int xyz2;
+  sy^
+}
+  )cpp");
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(Named("sym1"), Named("sym2")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -535,12 +535,12 @@
   EXPECT_ERROR(runLocateSymbolAt(Server, FooCpp, Position()));
   EXPECT_ERROR(runFindDocumentHighlights(Server, FooCpp, Position()));
   EXPECT_ERROR(runRename(Server, FooCpp, Position(), "new_name"));
-  // FIXME: codeComplete and signatureHelp should also return errors when they
-  // can't parse the file.
+  // Identifier-based fallback completion.
   EXPECT_THAT(cantFail(runCodeComplete(Server, FooCpp, Position(),
clangd::CodeCompleteOptions()))
   .Completions,
-  IsEmpty());
+  ElementsAre(Field(&CodeCompletion::Name, "int"),
+  Field(&CodeCompletion::Name, "main")));
   auto SigHelp = runSignatureHelp(Server, FooCpp, Position());
   ASSERT_TRUE(bool(SigHelp)) << "signatureHelp returned an error";
   EXPECT_THAT(SigHelp->signatures, IsEmpty());
@@ -1066,10 +1066,11 @@
   ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
   auto FooCpp = testPath("foo.cpp");
-  Annotations Code(R"cpp(
+   Annotations Code(R"cpp(
+namespace ns { int xyz; }
+using namespace ns;
 int main() {
-  int xyz;
-  xy^
+   xy^
 })cpp");
   FS.Files[FooCpp] = FooCpp;
 
@@ -1081,17 +1082,21 @@
   Server.addDocument(FooCpp, Code.code());
   ASSERT_TRUE(Server.blockUntilIdleForTest());
   auto Res = cantFail(runCodeComplete(Server, FooCpp, Code.point(), Opts));
-  EXPECT_THAT(Res.Completions, IsEmpty());
   EXPECT_EQ(Res.Context, CodeCompletionContext::CCC_Recovery);
+  // Identifier-based fallback completion doesn't know about "symbol" scope.
+  EXPECT_THAT(Res.Completions,
+  ElementsAre(Al

[PATCH] D60558: [clang-format] Fix indent of trailing raw string param after newline

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Currently clang-format uses ContinuationIndent to indent the contents of a raw
string literal that is the last parameter of the function call. This is to
achieve formatting similar to trailing:

  f(1, 2, R"pb(
  x: y)pb");

However this had the unfortunate consequence of producing format like this:

  fff(1, 2,
  R"pb(
  a: b
  )pb");

This patch makes clang-format consider indenting a trailing raw string param
after a newline based off the start of the format delimiter, producing:

  fff(1, 2,
  R"pb(
a: b
  )pb");


Repository:
  rC Clang

https://reviews.llvm.org/D60558

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/ContinuationIndenter.h
  unittests/Format/FormatTestRawStrings.cpp

Index: unittests/Format/FormatTestRawStrings.cpp
===
--- unittests/Format/FormatTestRawStrings.cpp
+++ unittests/Format/FormatTestRawStrings.cpp
@@ -981,6 +981,20 @@
 })test", Style));
 }
 
+TEST_F(FormatTestRawStrings, IndentsLastParamAfterNewline) {
+  FormatStyle Style = getRawStringPbStyleWithColumns(60);
+  expect_eq(R"test(
+f("aa",
+  R"pb(
+b: c
+  )pb");)test",
+format(R"test(
+f("aa",
+  R"pb(
+  b: c
+  )pb");)test",
+   Style));
+}
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/ContinuationIndenter.h
===
--- lib/Format/ContinuationIndenter.h
+++ lib/Format/ContinuationIndenter.h
@@ -111,12 +111,12 @@
   unsigned reformatRawStringLiteral(const FormatToken &Current,
 LineState &State,
 const FormatStyle &RawStringStyle,
-bool DryRun);
+bool DryRun, bool Newline);
 
   /// If the current token is at the end of the current line, handle
   /// the transition to the next line.
   unsigned handleEndOfLine(const FormatToken &Current, LineState &State,
-   bool DryRun, bool AllowBreak);
+   bool DryRun, bool AllowBreak, bool Newline);
 
   /// If \p Current is a raw string that is configured to be reformatted,
   /// return the style to be used.
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1196,7 +1196,8 @@
   State.Column += Current.ColumnWidth;
   State.NextToken = State.NextToken->Next;
 
-  unsigned Penalty = handleEndOfLine(Current, State, DryRun, AllowBreak);
+  unsigned Penalty =
+  handleEndOfLine(Current, State, DryRun, AllowBreak, Newline);
 
   if (Current.Role)
 Current.Role->formatFromToken(State, this, DryRun);
@@ -1490,7 +1491,7 @@
 
 unsigned ContinuationIndenter::reformatRawStringLiteral(
 const FormatToken &Current, LineState &State,
-const FormatStyle &RawStringStyle, bool DryRun) {
+const FormatStyle &RawStringStyle, bool DryRun, bool Newline) {
   unsigned StartColumn = State.Column - Current.ColumnWidth;
   StringRef OldDelimiter = *getRawStringDelimiter(Current.TokenText);
   StringRef NewDelimiter =
@@ -1530,8 +1531,10 @@
   // source.
   bool ContentStartsOnNewline = Current.TokenText[OldPrefixSize] == '\n';
   // If this token is the last parameter (checked by looking if it's followed by
-  // `)`, the base the indent off the line's nested block indent. Otherwise,
-  // base the indent off the arguments indent, so we can achieve:
+  // `)` and is not on a newline, the base the indent off the line's nested
+  // block indent. Otherwise, base the indent off the arguments indent, so we
+  // can achieve:
+  //
   // fff(1, 2, 3, R"pb(
   // key1: 1  #
   // key2: 2)pb");
@@ -1540,11 +1543,18 @@
   // R"pb(
   //   key1: 1  #
   //   key2: 2
+  // )pb");
+  //
+  // fff(1, 2, 3,
+  // R"pb(
+  //   key1: 1  #
+  //   key2: 2
   // )pb",
   // 5);
-  unsigned CurrentIndent = (Current.Next && Current.Next->is(tok::r_paren))
-   ? State.Stack.back().NestedBlockIndent
-   : State.Stack.back().Indent;
+  unsigned CurrentIndent =
+  (!Newline && Current.Next && Current.Next->is(tok::r_paren))
+  ? State.Stack.back().NestedBlockIndent
+  : State.Stack.back().Indent;
   unsigned NextStartColumn = ContentStartsOnNewline
  ? C

[PATCH] D60272: [Aarch64] Add v8.2-a half precision element extract intrinsics

[Diogo N. Sampaio via Phabricator via cfe-commits]

dnsampaio added a comment.

Ping


Repository:
  rC Clang

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

https://reviews.llvm.org/D60272



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

[PATCH] D59641: [clangd] Show template argument list in workspacesymbols and documentsymbols responses

[Kadir Cetinkaya via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/AST.cpp:112
   if (!Out.str().empty()) {
-// FIXME(ibiryukov): do not show args not explicitly written by the user.
-if (auto *ArgList = getTemplateSpecializationArgs(ND))

ilya-biryukov wrote:
> Where did this FIXME go? Is this now fixed?
Yes, since `printTemplateArgsAsWritten` prints the args as written in the 
code(see changes in file 
clang-tools-extra/unittests/clangd/FindSymbolsTests.cpp, especially the tests 
with a FIXME).

Of course it still has the caveat of frienddecls, but I think the fixme inside 
`printTemplateArgsAswritten` is the one covering that case, not this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59641



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

[PATCH] D59639: [clangd] Print template arguments helper

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/AST.cpp:133
+printTemplateArgumentList(OS, *Args, Policy);
+  else if (auto *Cls = llvm::dyn_cast(&ND)) {
+if (const TypeSourceInfo *TSI = Cls->getTypeAsWritten()) {

ioeric wrote:
> kadircet wrote:
> > ioeric wrote:
> > > why isn't this handled in `getTemplateSpecializationArgLocs`? Add a 
> > > comment?
> > it is mentioned in `getTemplateSpecializationArgLocs`, would you rather 
> > move the comment here?
> I think it would be clearer if we do something like:
> 
> ```
> if (auto *Cls = llvm::dyn_cast(&ND)) {
>   // handle this specially because ...
> } else {
>   // use getTemplateSpecializationArgLocs to handle rest of cases.
> }
> 
> ```
I am not performing this in the order you mentioned since 
`ClassTemplatePartialSpecializationDecl` is also a 
`ClassTemplateSpecializationDecl`, but if you insist I can change the ordering 
by adding another condition(which just looks ugly).



Comment at: clang-tools-extra/clangd/AST.cpp:149
+} else {
+  // FIXME: Fix cases when getTypeAsWritten returns null, e.g. friend 
decls.
+  printTemplateArgumentList(OS, Cls->getTemplateArgs().asArray(), Policy);

ioeric wrote:
> I'm not quite sure if I understand this FIXME. In this `else` branch, we are 
> handling this case. Do you mean this is not a proper fix and future work is 
> needed here? Could you elaborate?
> 
> One thing that's worth commenting is why we could use 
> `Cls->getTemplateArgs().asArray()` when `Cls->getTypeAsWritten` is null. It's 
> not trivial from the code.
Yes, this should rather be fixed in AST itself. Updating comment to explain the 
fallback strategy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59639



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

[PATCH] D59639: [clangd] Print template arguments helper

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59639

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/unittests/clangd/CMakeLists.txt
  clang-tools-extra/unittests/clangd/FindSymbolsTests.cpp
  clang-tools-extra/unittests/clangd/PrintASTTests.cpp
  clang/lib/AST/TypePrinter.cpp

Index: clang/lib/AST/TypePrinter.cpp
===
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -1632,6 +1632,19 @@
   return A.getArgument();
 }
 
+static void printArgument(const TemplateArgument &A, const PrintingPolicy &PP,
+  llvm::raw_ostream &OS) {
+  A.print(PP, OS);
+}
+
+static void printArgument(const TemplateArgumentLoc &A,
+  const PrintingPolicy &PP, llvm::raw_ostream &OS) {
+  const TemplateArgument::ArgKind &Kind = A.getArgument().getKind();
+  if (Kind == TemplateArgument::ArgKind::Type)
+return A.getTypeSourceInfo()->getType().print(OS, PP);
+  return A.getArgument().print(PP, OS);
+}
+
 template
 static void printTo(raw_ostream &OS, ArrayRef Args,
 const PrintingPolicy &Policy, bool SkipBrackets) {
@@ -1653,7 +1666,8 @@
 } else {
   if (!FirstArg)
 OS << Comma;
-  Argument.print(Policy, ArgOS);
+  // Tries to print the argument with location info if exists.
+  printArgument(Arg, Policy, ArgOS);
 }
 StringRef ArgString = ArgOS.str();
 
Index: clang-tools-extra/unittests/clangd/PrintASTTests.cpp
===
--- /dev/null
+++ clang-tools-extra/unittests/clangd/PrintASTTests.cpp
@@ -0,0 +1,94 @@
+//===--- ASTUtilsTests.cpp - C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AST.h"
+#include "Annotations.h"
+#include "Protocol.h"
+#include "SourceCode.h"
+#include "TestTU.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest-param-test.h"
+#include "gtest/gtest.h"
+#include "gtest/internal/gtest-param-util-generated.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using testing::ElementsAreArray;
+
+struct Case {
+  const char *AnnotatedCode;
+  std::vector Expected;
+};
+class ASTUtils : public testing::Test,
+ public ::testing::WithParamInterface {};
+
+TEST_P(ASTUtils, PrintTemplateArgs) {
+  auto Pair = GetParam();
+  Annotations Test(Pair.AnnotatedCode);
+  auto AST = TestTU::withCode(Test.code()).build();
+  struct Visitor : RecursiveASTVisitor {
+Visitor(std::vector Points) : Points(std::move(Points)) {}
+bool VisitNamedDecl(const NamedDecl *ND) {
+  auto Pos = sourceLocToPosition(ND->getASTContext().getSourceManager(),
+ ND->getLocation());
+  if (Pos != Points[TemplateArgs.size()])
+return true;
+  TemplateArgs.push_back(printTemplateSpecializationArgs(*ND));
+  return true;
+}
+std::vector TemplateArgs;
+const std::vector Points;
+  };
+  Visitor V(Test.points());
+  V.TraverseDecl(AST.getASTContext().getTranslationUnitDecl());
+  EXPECT_THAT(V.TemplateArgs, ElementsAreArray(Pair.Expected));
+}
+
+INSTANTIATE_TEST_CASE_P(ASTUtilsTests, ASTUtils,
+testing::ValuesIn(std::vector({
+{
+R"cpp(
+  template  class Bar {};
+  template <> class ^Bar {};)cpp",
+{""}},
+{
+R"cpp(
+  template  class Bar {};
+  template  class Z, int Q>
+  struct Foo {};
+  template struct ^Foo;
+  template 
+  struct ^Foo {};)cpp",
+{"", ""}},
+{
+R"cpp(
+  template  void Foz() {};
+  template <> void ^Foz<3, 5, 8>() {};)cpp",
+{"<3, 5, 8>"}},
+{
+R"cpp(
+  template  class Bar {};
+  template  class ...>
+  class Aux {};
+ 

[PATCH] D59639: [clangd] Print template arguments helper

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Update file comment for PrintASTTests.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59639

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/unittests/clangd/CMakeLists.txt
  clang-tools-extra/unittests/clangd/FindSymbolsTests.cpp
  clang-tools-extra/unittests/clangd/PrintASTTests.cpp
  clang/lib/AST/TypePrinter.cpp

Index: clang/lib/AST/TypePrinter.cpp
===
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -1632,6 +1632,19 @@
   return A.getArgument();
 }
 
+static void printArgument(const TemplateArgument &A, const PrintingPolicy &PP,
+  llvm::raw_ostream &OS) {
+  A.print(PP, OS);
+}
+
+static void printArgument(const TemplateArgumentLoc &A,
+  const PrintingPolicy &PP, llvm::raw_ostream &OS) {
+  const TemplateArgument::ArgKind &Kind = A.getArgument().getKind();
+  if (Kind == TemplateArgument::ArgKind::Type)
+return A.getTypeSourceInfo()->getType().print(OS, PP);
+  return A.getArgument().print(PP, OS);
+}
+
 template
 static void printTo(raw_ostream &OS, ArrayRef Args,
 const PrintingPolicy &Policy, bool SkipBrackets) {
@@ -1653,7 +1666,8 @@
 } else {
   if (!FirstArg)
 OS << Comma;
-  Argument.print(Policy, ArgOS);
+  // Tries to print the argument with location info if exists.
+  printArgument(Arg, Policy, ArgOS);
 }
 StringRef ArgString = ArgOS.str();
 
Index: clang-tools-extra/unittests/clangd/PrintASTTests.cpp
===
--- /dev/null
+++ clang-tools-extra/unittests/clangd/PrintASTTests.cpp
@@ -0,0 +1,94 @@
+//===--- PrintASTTests.cpp - C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AST.h"
+#include "Annotations.h"
+#include "Protocol.h"
+#include "SourceCode.h"
+#include "TestTU.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest-param-test.h"
+#include "gtest/gtest.h"
+#include "gtest/internal/gtest-param-util-generated.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using testing::ElementsAreArray;
+
+struct Case {
+  const char *AnnotatedCode;
+  std::vector Expected;
+};
+class ASTUtils : public testing::Test,
+ public ::testing::WithParamInterface {};
+
+TEST_P(ASTUtils, PrintTemplateArgs) {
+  auto Pair = GetParam();
+  Annotations Test(Pair.AnnotatedCode);
+  auto AST = TestTU::withCode(Test.code()).build();
+  struct Visitor : RecursiveASTVisitor {
+Visitor(std::vector Points) : Points(std::move(Points)) {}
+bool VisitNamedDecl(const NamedDecl *ND) {
+  auto Pos = sourceLocToPosition(ND->getASTContext().getSourceManager(),
+ ND->getLocation());
+  if (Pos != Points[TemplateArgs.size()])
+return true;
+  TemplateArgs.push_back(printTemplateSpecializationArgs(*ND));
+  return true;
+}
+std::vector TemplateArgs;
+const std::vector Points;
+  };
+  Visitor V(Test.points());
+  V.TraverseDecl(AST.getASTContext().getTranslationUnitDecl());
+  EXPECT_THAT(V.TemplateArgs, ElementsAreArray(Pair.Expected));
+}
+
+INSTANTIATE_TEST_CASE_P(ASTUtilsTests, ASTUtils,
+testing::ValuesIn(std::vector({
+{
+R"cpp(
+  template  class Bar {};
+  template <> class ^Bar {};)cpp",
+{""}},
+{
+R"cpp(
+  template  class Bar {};
+  template  class Z, int Q>
+  struct Foo {};
+  template struct ^Foo;
+  template 
+  struct ^Foo {};)cpp",
+{"", ""}},
+{
+R"cpp(
+  template  void Foz() {};
+  template <> void ^Foz<3, 5, 8>() {};)cpp",
+{"<3, 5, 8>"}},
+{
+R"cpp(
+  template  class Bar {};
+  template  class ...>
+  class Aux {};
+   

[PATCH] D60272: [Aarch64] Add v8.2-a half precision element extract intrinsics

[Luke Geeson via Phabricator via cfe-commits]

LukeGeeson added a comment.

couple of nits, besides from that LGTM




Comment at: lib/CodeGen/CGBuiltin.cpp:7813
   }
+  case NEON::BI__builtin_neon_vduph_lane_f16:{
+return Builder.CreateExtractElement(Ops[0], EmitScalarExpr(E->getArg(1)),

nit: spacing between : and { 



Comment at: lib/CodeGen/CGBuiltin.cpp:7817
+  }
+  case NEON::BI__builtin_neon_vduph_laneq_f16:{
+return Builder.CreateExtractElement(Ops[0], EmitScalarExpr(E->getArg(1)),

nit: spacing again


Repository:
  rC Clang

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

https://reviews.llvm.org/D60272



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

[PATCH] D60560: [clangd] Enable clang-tidy by default.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, 
ilya-biryukov.
Herald added a project: clang.

We have turned on the flag internally for a while, and we don't receive 
complains.
Should be good to turn it on now.

If the projects doesn't have .clang-tidy files, no clang-tidy check will
be run.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60560

Files:
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -228,7 +228,7 @@
 static llvm::cl::opt EnableClangTidy(
 "clang-tidy",
 llvm::cl::desc("Enable clang-tidy diagnostics."),
-llvm::cl::init(false));
+llvm::cl::init(true));
 
 static llvm::cl::opt SuggestMissingIncludes(
 "suggest-missing-includes",


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -228,7 +228,7 @@
 static llvm::cl::opt EnableClangTidy(
 "clang-tidy",
 llvm::cl::desc("Enable clang-tidy diagnostics."),
-llvm::cl::init(false));
+llvm::cl::init(true));
 
 static llvm::cl::opt SuggestMissingIncludes(
 "suggest-missing-includes",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

[Gauthier via Phabricator via cfe-commits]

Tyker created this revision.
Tyker added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

this is a bugfix for this 

added a constant copy of argument to CallStackFrame.
and use it in describeCall instead of the modifiable version od arguments.
added test for the diagnostic


Repository:
  rC Clang

https://reviews.llvm.org/D60561

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx1y.cpp

Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1159,3 +1159,25 @@
 enum class InEnum3 {
   THREE = indirect_builtin_constant_p("abc")
 };
+
+constexpr int f1(int i) {
+  i -= 1;
+  return 1 << i; // expected-note {{negative shift count -8}}
+}
+
+constexpr int f2(int i) {
+  i -= 1;
+  if (i < -6)
+return f1(i); // expected-note {{'f1(-7)}}
+  // expected-note@+6 {{'f2(-6)}}
+  // expected-note@+5 {{'f2(-5)}}
+  // expected-note@+4 {{'f2(-4)}}
+  // expected-note@+3 {{'f2(-3)}}
+  // expected-note@+2 {{'f2(-2)}}
+  // expected-note@+1 {{'f2(-1)}}
+  return f2(i);
+}
+
+// expected-error@+2 {{constant expression}}
+// expected-note@+1 {{'f2(0)}}
+constexpr int a = f2(0);
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -476,9 +476,15 @@
 const LValue *This;
 
 /// Arguments - Parameter bindings for this function call, indexed by
-/// parameters' function scope indices.
+/// parameters' function scope indices. may be modified by called function
 APValue *Arguments;
 
+using ConstArgs = APValue const *const;
+/// ConstantArgs - Parameter bindings for this function call, indexed by
+/// parameters' function scope indices. can't be modified. used for
+/// diagnostics
+ConstArgs ConstantArgs;
+
 // Note that we intentionally use std::map here so that references to
 // values are stable.
 typedef std::pair MapKeyTy;
@@ -519,7 +525,7 @@
 
 CallStackFrame(EvalInfo &Info, SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
-   APValue *Arguments);
+   APValue *Arguments, ConstArgs ConstantArgs);
 ~CallStackFrame();
 
 // Return the temporary for Key whose version number is Version.
@@ -789,14 +795,15 @@
 bool checkingForOverflow() { return EvalMode == EM_EvaluateForOverflow; }
 
 EvalInfo(const ASTContext &C, Expr::EvalStatus &S, EvaluationMode Mode)
-  : Ctx(const_cast(C)), EvalStatus(S), CurrentCall(nullptr),
-CallStackDepth(0), NextCallIndex(1),
-StepsLeft(getLangOpts().ConstexprStepLimit),
-BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),
-EvaluatingDecl((const ValueDecl *)nullptr),
-EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
-HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
-InConstantContext(false), EvalMode(Mode) {}
+: Ctx(const_cast(C)), EvalStatus(S), CurrentCall(nullptr),
+  CallStackDepth(0), NextCallIndex(1),
+  StepsLeft(getLangOpts().ConstexprStepLimit),
+  BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr,
+  nullptr),
+  EvaluatingDecl((const ValueDecl *)nullptr),
+  EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
+  HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
+  InConstantContext(false), EvalMode(Mode) {}
 
 void setEvaluatingDecl(APValue::LValueBase Base, APValue &Value) {
   EvaluatingDecl = Base;
@@ -1235,9 +1242,10 @@
 
 CallStackFrame::CallStackFrame(EvalInfo &Info, SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
-   APValue *Arguments)
+   APValue *Arguments, ConstArgs CArgs)
 : Info(Info), Caller(Info.CurrentCall), Callee(Callee), This(This),
-  Arguments(Arguments), CallLoc(CallLoc), Index(Info.NextCallIndex++) {
+  Arguments(Arguments), ConstantArgs(CArgs), CallLoc(CallLoc),
+  Index(Info.NextCallIndex++) {
   Info.CurrentCall = this;
   ++Info.CallStackDepth;
 }
@@ -1679,7 +1687,7 @@
   Out << ", ";
 
 const ParmVarDecl *Param = *I;
-const APValue &Arg = Frame->Arguments[ArgIndex];
+const APValue &Arg = Frame->ConstantArgs[ArgIndex];
 Arg.printPretty(Out, Frame->Info.Ctx, Param->getType());
 
 if (ArgIndex == 0 && IsMemberCall)
@@ -4455,7 +4463,11 @@
   if (!Info.CheckCallLimit(CallLoc))
 return false;
 
-  CallStackFrame Frame(Info, CallL

[PATCH] D60541: [clang-format] Use SpacesBeforeTrailingComments for "option" directive

[Krasimir Georgiev via Phabricator via cfe-commits]

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

Thank you!


Repository:
  rC Clang

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

https://reviews.llvm.org/D60541



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

[PATCH] D59639: [clangd] Print template arguments helper

[Eric Liu via Phabricator via cfe-commits]

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

lgtm




Comment at: clang-tools-extra/clangd/AST.cpp:139
+  // location information.
+  printTemplateArgumentList(OS, Cls->getTemplateArgs().asArray(), Policy);
+}

Could you also add a test case for this with the current behavior and a FIXME?



Comment at: clang-tools-extra/unittests/clangd/PrintASTTests.cpp:47
+}
+std::vector TemplateArgs;
+const std::vector Points;

maybe `s/TemplateArgs/TemplateArgsAtPoints/` for clarity


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59639



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

[PATCH] D57860: [analyzer] Validate checker option names and values

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

Szelethus added a comment.

Ping.


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

https://reviews.llvm.org/D57860



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

[PATCH] D57858: [analyzer] Add a new frontend flag to display all checker options

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

Szelethus added a comment.

Ping.


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

https://reviews.llvm.org/D57858



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

[PATCH] D59465: [analyzer] Add example plugin for checker option handling

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

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

I'll move this, and the already existing plugin to test/ or unittest/.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59465



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

[PATCH] D60408: [LibTooling] Extend Transformer to support multiple simultaneous changes.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

gentle ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60408



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

[PATCH] D58573: [analyzer] Move UninitializedObjectChecker out of alpha

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

Szelethus updated this revision to Diff 194667.
Szelethus retitled this revision from "[analyzer] Move UninitializedObject out 
of alpha" to "[analyzer] Move UninitializedObjectChecker out of alpha".
Szelethus added a comment.

Moved the checker to `optin.cplusplus`.


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

https://reviews.llvm.org/D58573

Files:
  docs/analyzer/checkers.rst
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
  test/Analysis/cxx-uninitialized-object-inheritance.cpp
  test/Analysis/cxx-uninitialized-object-no-dereference.cpp
  test/Analysis/cxx-uninitialized-object-notes-as-warnings.cpp
  test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
  test/Analysis/cxx-uninitialized-object-unguarded-access.cpp
  test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp
  test/Analysis/cxx-uninitialized-object.cpp
  test/Analysis/objcpp-uninitialized-object.mm
  www/analyzer/alpha_checks.html
  www/analyzer/available_checks.html

Index: www/analyzer/available_checks.html
===
--- www/analyzer/available_checks.html
+++ www/analyzer/available_checks.html
@@ -543,6 +543,119 @@
 
 Name, DescriptionExample
 
+
+cplusplus.UninitializedObject
+(C++)
+This checker reports uninitialized fields in objects created after a constructor
+call. It doesn't only find direct uninitialized fields, but rather makes a deep
+inspection of the object, analyzing all of it's fields subfields. 
+The checker regards inherited fields as direct fields, so one will recieve
+warnings for uninitialized inherited data members as well. 
+
+It has several options:
+
+  
+"Pedantic" (boolean). If its not set or is set to false, the
+checker won't emit warnings for objects that don't have at least one
+initialized field. This may be set with 
+-analyzer-config cplusplus.UninitializedObject:Pedantic=true.
+  
+  
+"NotesAsWarnings" (boolean). If set to true, the checker will
+emit a warning for each uninitalized field, as opposed to emitting one
+warning per constructor call, and listing the uninitialized fields that
+belongs to it in notes. Defaults to false. 
+-analyzer-config cplusplus.UninitializedObject:NotesAsWarnings=true.
+  
+  
+"CheckPointeeInitialization" (boolean). If set to false, the
+checker will not analyze the pointee of pointer/reference fields, and will
+only check whether the object itself is initialized. Defaults to false. 
+-analyzer-config cplusplus.UninitializedObject:CheckPointeeInitialization=true.
+  
+  
+"IgnoreRecordsWithField" (string). If supplied, the checker
+will not analyze structures that have a field with a name or type name that
+matches the given pattern. Defaults to "".
+
+-analyzer-config cplusplus.UninitializedObject:IgnoreRecordsWithField="[Tt]ag|[Kk]ind".
+  
+
+
+
+// With Pedantic and CheckPointeeInitialization set to true
+
+struct A {
+  struct B {
+int x; // note: uninitialized field 'this->b.x'
+   // note: uninitialized field 'this->bptr->x'
+int y; // note: uninitialized field 'this->b.y'
+   // note: uninitialized field 'this->bptr->y'
+  };
+  int *iptr; // note: uninitialized pointer 'this->iptr'
+  B b;
+  B *bptr;
+  char *cptr; // note: uninitialized pointee 'this->cptr'
+
+  A (B *bptr, char *cptr) : bptr(bptr), cptr(cptr) {}
+};
+
+void f() {
+  A::B b;
+  char c;
+  A a(&b, &c); // warning: 6 uninitialized fields
+   //  after the constructor call
+}
+
+
+// With Pedantic set to false and
+// CheckPointeeInitialization set to true
+// (every field is uninitialized)
+
+struct A {
+  struct B {
+int x;
+int y;
+  };
+  int *iptr;
+  B b;
+  B *bptr;
+  char *cptr;
+
+  A (B *bptr, char *cptr) : bptr(bptr), cptr(cptr) {}
+};
+
+void f() {
+  A::B b;
+  char c;
+  A a(&b, &c); // no warning
+}
+
+
+// With Pedantic and CheckPointeeInitialization set to false
+// (pointees are regarded as initialized)
+
+struct A {
+  struct B {
+int x; // note: uninitialized field 'this->b.x'
+int y; // note: uninitialized field 'this->b.y'
+  };
+  int *iptr; // note: uninitialized pointer 'this->iptr'
+  B b;
+  B *bptr;
+  char *cptr;
+
+  A (B *bptr, char *cptr) : bptr(bptr), cptr(cptr) {}
+};
+
+void f() {
+  A::B b;
+  char c;
+  A a(&b, &c); // warning: 3 uninitialized fields
+   //  after the constructor call
+}
+
+
 
 
 
Index: www/analyzer/alpha_checks.html
===
--- www/analyzer/alpha_checks.html
+++ www/analyzer/alpha_checks.html
@@ -445,120 +445,6 @@
 
 
 
-
-alpha.cplusplus.UninitializedObject
-(C++)
-This checker reports uninitialized fields in objects created after a constructor
-call. It doesn't only find direct uninitialized fields, but rather makes a deep
-inspection of the object, analyzing all of it's field

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 194669.
hintonda added a comment.

- Delay actually adding default options until ParseCommandLineOptions which 
simplifies handling and makes it easy to only add them to the correct 
SubCommand.

  Add simple tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746

Files:
  clang/lib/Tooling/CommonOptionsParser.cpp
  llvm/docs/CommandLine.rst
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/test/Support/check-default-options.txt
  llvm/tools/llvm-opt-report/OptReport.cpp

Index: llvm/tools/llvm-opt-report/OptReport.cpp
===
--- llvm/tools/llvm-opt-report/OptReport.cpp
+++ llvm/tools/llvm-opt-report/OptReport.cpp
@@ -36,8 +36,6 @@
 using namespace llvm;
 using namespace llvm::yaml;
 
-static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
-
 // Mark all our options with this category, everything else (except for -version
 // and -help) will be hidden.
 static cl::OptionCategory
@@ -440,11 +438,6 @@
   "A tool to generate an optimization report from YAML optimization"
   " record files.\n");
 
-  if (Help) {
-cl::PrintHelpMessage();
-return 0;
-  }
-
   LocationInfoTy LocationInfo;
   if (!readLocationInfo(LocationInfo))
 return 1;
Index: llvm/test/Support/check-default-options.txt
===
--- /dev/null
+++ llvm/test/Support/check-default-options.txt
@@ -0,0 +1,18 @@
+# RUN: llvm-objdump -help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
+# RUN: llvm-readobj -help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
+# RUN: llvm-tblgen -help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
+# RUN: llvm-opt-report -help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
+# RUN: llvm-dwarfdump -help-hidden %t | FileCheck --check-prefix=CHECK-DWARF %s
+# RUN: llvm-dwarfdump -h %t | FileCheck --check-prefix=CHECK-DWARF-H %s
+
+
+# CHECK-OBJDUMP: -h  - Alias for --section-headers
+# CHECK-READOBJ: -h  - Alias for --file-headers
+# CHECK-TBLGEN:  -h  - Alias for -help
+# CHECK-OPT-RPT: -h  - Alias for -help
+# CHECK-DWARF:   -h  - Alias for -help
+
+# llvm-dwarfdump declares `-h` option and prints special help in that case,
+# which is weird, but makes for a good test, i.e., shows the default `-h`
+# wasn't used.
+# CHECK-DWARF-H-NOT: -help-list  - Display list of available options (-help-list-hidden for more)
Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -98,6 +98,8 @@
   // This collects additional help to be printed.
   std::vector MoreHelp;
 
+  SmallVector DefaultOptions;
+
   // This collects the different option categories that have been registered.
   SmallPtrSet RegisteredOptionCategories;
 
@@ -146,6 +148,11 @@
   void addOption(Option *O, SubCommand *SC) {
 bool HadErrors = false;
 if (O->hasArgStr()) {
+  // If it's a DefaultOption, check to make sure it isn't already there.
+  if (O->getMiscFlags() & cl::DefaultOption &&
+  SC->OptionsMap.find(O->ArgStr) != SC->OptionsMap.end())
+return;
+
   // Add argument to the argument map!
   if (!SC->OptionsMap.insert(std::make_pair(O->ArgStr, O)).second) {
 errs() << ProgramName << ": CommandLine Error: Option '" << O->ArgStr
@@ -185,7 +192,13 @@
 }
   }
 
-  void addOption(Option *O) {
+  void addOption(Option *O, bool ProcessDefaultOption = false) {
+// Handle DefaultOptions
+if (!ProcessDefaultOption && O->getMiscFlags() & cl::DefaultOption) {
+  DefaultOptions.push_back(O);
+  return;
+}
+
 if (O->Subs.empty()) {
   addOption(O, &*TopLevelSubCommand);
 } else {
@@ -266,8 +279,13 @@
 if (O->Subs.empty())
   updateArgStr(O, NewName, &*TopLevelSubCommand);
 else {
-  for (auto SC : O->Subs)
-updateArgStr(O, NewName, SC);
+  if (O->isInAllSubCommands()) {
+for (auto SC : RegisteredSubCommands)
+  updateArgStr(O, NewName, SC);
+  } else {
+for (auto SC : O->Subs)
+  updateArgStr(O, NewName, SC);
+  }
 }
   }
 
@@ -1167,6 +1185,11 @@
   auto &SinkOpts = ChosenSubCommand->SinkOpts;
   auto &OptionsMap = ChosenSubCommand->OptionsMap;
 
+  // Handle DefaultOptions.
+  for (auto O: DefaultOptions) {
+addOption(O, true);
+  }
+
   if (ConsumeAfterOpt) {
 assert(PositionalOpts.size() > 0 &&
"Cannot specify cl::ConsumeAfter without a positional argument!");
@@ -2146,6 +2169,9 @@
 cl::location(WrappedNormalPrinter), cl::ValueDisallowed,
 cl::cat(GenericCategory), cl::sub(*AllSubCommands));
 
+static cl::alias HOpA("h", cl::desc("Alias for -help"), cl::aliasopt(HOp),
+  cl::DefaultOption);
+
 stat

[libunwind] r358164 - [libunwind] Fix the typo in unw_save_vfp_as_X alias

[Petr Hosek via cfe-commits]

Author: phosek
Date: Thu Apr 11 06:08:44 2019
New Revision: 358164

URL: http://llvm.org/viewvc/llvm-project?rev=358164&view=rev
Log:
[libunwind] Fix the typo in unw_save_vfp_as_X alias

This was accidentaly introduced in r357640.

Modified:
libunwind/trunk/src/libunwind.cpp

Modified: libunwind/trunk/src/libunwind.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/libunwind.cpp?rev=358164&r1=358163&r2=358164&view=diff
==
--- libunwind/trunk/src/libunwind.cpp (original)
+++ libunwind/trunk/src/libunwind.cpp Thu Apr 11 06:08:44 2019
@@ -236,7 +236,7 @@ _LIBUNWIND_HIDDEN void __unw_save_vfp_as
   AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
   return co->saveVFPAsX();
 }
-_LIBUNWIND_WEAK_ALIAS(__unw_save_vfp_as_X, unw_save_cfp_as_X)
+_LIBUNWIND_WEAK_ALIAS(__unw_save_vfp_as_X, unw_save_vfp_as_X)
 #endif
 
 


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

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda marked 2 inline comments as done.
hintonda added inline comments.



Comment at: llvm/lib/Support/CommandLine.cpp:282
 else {
-  for (auto SC : O->Subs)
-updateArgStr(O, NewName, SC);
+  if (O->isInAllSubCommands()) {
+for (auto SC : RegisteredSubCommands)

Will move this bug fix to its own patch and add test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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

[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

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

Quuxplusone added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:486
+/// diagnostics
+ConstArgs ConstantArgs;
+

Please don't use a typedef for this. Follow the style of the surrounding lines.

const APValue *ConstantArgs;

(The pointer member itself should not be `const`-qualified. Compare to `const 
LValue *This;` two declarations above.)



Comment at: clang/lib/AST/ExprConstant.cpp:4520
   APValue *ArgValues,
+  APValue const *const ConstantArgs,
   const CXXConstructorDecl *Definition,

`const APValue *ConstantArgs,`



Comment at: clang/lib/AST/ExprConstant.cpp:4684
 
-  return HandleConstructorCall(E, This, ArgValues.data(), Definition,
-   Info, Result);
+  ArgVector ConstantArg(ArgValues.begin(), ArgValues.end());
+  return HandleConstructorCall(E, This, ArgValues.data(), ConstantArg.data(),

Surely you meant `ConstantArgs` (plural) here.



Comment at: clang/test/SemaCXX/constant-expression-cxx1y.cpp:1183
+// expected-note@+1 {{'f2(0)}}
+constexpr int a = f2(0);

Personally, I would prefer to see something as simple as my original example; I 
don't think the extra 6 levels of recursion here makes the test any easier to 
understand.
```
constexpr int f(int i) {
i = -i;
return 1 << i;  // expected-note{{negative shift count -1}}
}
static_assert(f(1));
// expected-error@-1{{static_assert expression is not an integral constant 
expression}}
// expected-note@-2{{in call to 'f(1)'}}
```

Also, procedural note: I expect you'll be asked to reupload this diff with full 
context (`git diff -U999 ...`).


Repository:
  rC Clang

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

https://reviews.llvm.org/D60561



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

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

[Oliver Stannard (Linaro) via Phabricator via cfe-commits]

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

The document you linked in the LLVM change 
(https://docs.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions?view=vs-2019#return-values)
 says that small POD types are returned directly in X0 or X0 and X1, but this 
looks like it will always return them indirectly. I think we also need to check 
the size of the type, and fall back the the plain C ABI for small types.




Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1060
+  // Note: The "inreg" attribute is used to signal that the struct return
+  // should be in X0.
+  bool sretInX0 = (CGM.getTarget().getTriple().getArch() ==

Nit: this will use X1 for functions with a this parameter, not X0.



Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1062
+  bool sretInX0 = (CGM.getTarget().getTriple().getArch() ==
+   llvm::Triple::aarch64) && !RD->isPOD();
+

This should also check aarch64_be.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60349



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

[PATCH] D59923: [Driver] Simplify -g level computation and its interaction with -gsplit-dwarf

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 194672.
MaskRay added a comment.

Update the description


Repository:
  rC Clang

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

https://reviews.llvm.org/D59923

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/split-debug.c

Index: test/Driver/split-debug.c
===
--- test/Driver/split-debug.c
+++ test/Driver/split-debug.c
@@ -71,7 +71,7 @@
 // RUN: FileCheck -check-prefix=CHECK-SPLIT-WITH-GMLT < %t %s
 //
 // CHECK-SPLIT-WITH-GMLT: "-enable-split-dwarf"
-// CHECK-SPLIT-WITH-GMLT: "-debug-info-kind=line-tables-only"
+// CHECK-SPLIT-WITH-GMLT: "-debug-info-kind=limited"
 // CHECK-SPLIT-WITH-GMLT: "-split-dwarf-file"
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -fno-split-dwarf-inlining -S -### %s 2> %t
@@ -103,6 +103,8 @@
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -g0 -S -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-G0-OVER-SPLIT < %t %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf=split -g0 -S -### %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-G0-OVER-SPLIT < %t %s
 //
 // CHECK-G0-OVER-SPLIT-NOT: "-enable-split-dwarf"
 // CHECK-G0-OVER-SPLIT-NOT: "-debug-info-kind
@@ -110,6 +112,8 @@
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -g0 -gsplit-dwarf -S -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-SPLIT-OVER-G0 < %t %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -g0 -gsplit-dwarf=split -S -### %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-SPLIT-OVER-G0 < %t %s
 //
 // CHECK-SPLIT-OVER-G0: "-enable-split-dwarf" "-debug-info-kind=limited"
 // CHECK-SPLIT-OVER-G0: "-split-dwarf-file"
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3158,35 +3158,24 @@
 SplitDWARFInlining = false;
   }
 
-  if (const Arg *A = Args.getLastArg(options::OPT_g_Group)) {
-if (checkDebugInfoOption(A, Args, D, TC)) {
-  // If the last option explicitly specified a debug-info level, use it.
-  if (A->getOption().matches(options::OPT_gN_Group)) {
-DebugInfoKind = DebugLevelToInfoKind(*A);
-// If you say "-gsplit-dwarf -gline-tables-only", -gsplit-dwarf loses.
-// But -gsplit-dwarf is not a g_group option, hence we have to check the
-// order explicitly. If -gsplit-dwarf wins, we fix DebugInfoKind later.
-// This gets a bit more complicated if you've disabled inline info in
-// the skeleton CUs (SplitDWARFInlining) - then there's value in
-// composing split-dwarf and line-tables-only, so let those compose
-// naturally in that case. And if you just turned off debug info,
-// (-gsplit-dwarf -g0) - do that.
-if (DwarfFission != DwarfFissionKind::None) {
-  if (A->getIndex() > SplitDWARFArg->getIndex()) {
-if (DebugInfoKind == codegenoptions::NoDebugInfo ||
-DebugInfoKind == codegenoptions::DebugDirectivesOnly ||
-(DebugInfoKind == codegenoptions::DebugLineTablesOnly &&
- SplitDWARFInlining))
-  DwarfFission = DwarfFissionKind::None;
-  } else if (SplitDWARFInlining)
-DebugInfoKind = codegenoptions::NoDebugInfo;
-}
-  } else {
-// For any other 'g' option, use Limited.
-DebugInfoKind = codegenoptions::LimitedDebugInfo;
-  }
-} else {
-  DebugInfoKind = codegenoptions::LimitedDebugInfo;
+  if (const Arg *A =
+  Args.getLastArg(options::OPT_g_Group, options::OPT_gsplit_dwarf,
+  options::OPT_gsplit_dwarf_EQ)) {
+DebugInfoKind = codegenoptions::LimitedDebugInfo;
+
+// If the last option explicitly specified a debug-info level, use it.
+if (checkDebugInfoOption(A, Args, D, TC) &&
+A->getOption().matches(options::OPT_gN_Group)) {
+  DebugInfoKind = DebugLevelToInfoKind(*A);
+  // For -g0 or -gline-tables-only, drop -gsplit-dwarf. This gets a bit more
+  // complicated if you've disabled inline info in the skeleton CUs
+  // (SplitDWARFInlining) - then there's value in composing split-dwarf and
+  // line-tables-only, so let those compose naturally in that case.
+  if (DebugInfoKind == codegenoptions::NoDebugInfo ||
+  DebugInfoKind == codegenoptions::DebugDirectivesOnly ||
+  (DebugInfoKind == codegenoptions::DebugLineTablesOnly &&
+   SplitDWARFInlining))
+DwarfFission = DwarfFissionKind::None;
 }
   }
 
@@ -3261,16 +3250,12 @@
   }
 }
 
-  // -gsplit-dwarf should turn on -g and enable the backend dwarf
-  // splitting and extraction.
+  // -gsplit-dwarf enables the backend dwarf splitting and extraction.
   if (T.isOSBinFormatELF()) {
 if (!SplitDWARFInlining)
   CmdArgs.push_back("-fno-split-dwarf-inlining");
 
 if (DwarfFission != DwarfFis

[PATCH] D59923: [Driver] Simplify -g level computation and its interaction with -gsplit-dwarf

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

> -gsplit-dwarf -gmlt -fno-split-dwarf-inlining => 1
> 
> This last one currently produces split-dwarf (if there's any DWARF worth 
> splitting - if there are any subprogram descriptions, etc, otherwise it saves 
> the indirection and produces an empty .dwo file).

Thanks for catching this. It should be 1 + split instead of 1. I've updated the 
description. This patch doesn't change the behavior of this rule.

> Seems there's going to be confusion either way, though - either the 
> presence/absence of -fno-split-dwarf-inlining changes whether -gsplit-dwarf 
> is respected/ignored (in the presence of -gmlt), or changes whethe -gmlt 
> composes with -gsplit-dwarf or overrides it? Seems these are both a bit 
> confusing, no?

See the updated description. The lower half of the rules can be summaries as 
`-g* -gsplit-dwarf => 2 + split` with this patch. I think that is how this 
patch makes the rules less confusing.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59923



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

[PATCH] D59923: [Driver] Simplify -g level computation and its interaction with -gsplit-dwarf

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 194673.
MaskRay edited the summary of this revision.
MaskRay added a comment.

Update description


Repository:
  rC Clang

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

https://reviews.llvm.org/D59923

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/split-debug.c

Index: test/Driver/split-debug.c
===
--- test/Driver/split-debug.c
+++ test/Driver/split-debug.c
@@ -71,7 +71,7 @@
 // RUN: FileCheck -check-prefix=CHECK-SPLIT-WITH-GMLT < %t %s
 //
 // CHECK-SPLIT-WITH-GMLT: "-enable-split-dwarf"
-// CHECK-SPLIT-WITH-GMLT: "-debug-info-kind=line-tables-only"
+// CHECK-SPLIT-WITH-GMLT: "-debug-info-kind=limited"
 // CHECK-SPLIT-WITH-GMLT: "-split-dwarf-file"
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -fno-split-dwarf-inlining -S -### %s 2> %t
@@ -103,6 +103,8 @@
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -g0 -S -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-G0-OVER-SPLIT < %t %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf=split -g0 -S -### %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-G0-OVER-SPLIT < %t %s
 //
 // CHECK-G0-OVER-SPLIT-NOT: "-enable-split-dwarf"
 // CHECK-G0-OVER-SPLIT-NOT: "-debug-info-kind
@@ -110,6 +112,8 @@
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -g0 -gsplit-dwarf -S -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-SPLIT-OVER-G0 < %t %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -g0 -gsplit-dwarf=split -S -### %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-SPLIT-OVER-G0 < %t %s
 //
 // CHECK-SPLIT-OVER-G0: "-enable-split-dwarf" "-debug-info-kind=limited"
 // CHECK-SPLIT-OVER-G0: "-split-dwarf-file"
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3158,35 +3158,24 @@
 SplitDWARFInlining = false;
   }
 
-  if (const Arg *A = Args.getLastArg(options::OPT_g_Group)) {
-if (checkDebugInfoOption(A, Args, D, TC)) {
-  // If the last option explicitly specified a debug-info level, use it.
-  if (A->getOption().matches(options::OPT_gN_Group)) {
-DebugInfoKind = DebugLevelToInfoKind(*A);
-// If you say "-gsplit-dwarf -gline-tables-only", -gsplit-dwarf loses.
-// But -gsplit-dwarf is not a g_group option, hence we have to check the
-// order explicitly. If -gsplit-dwarf wins, we fix DebugInfoKind later.
-// This gets a bit more complicated if you've disabled inline info in
-// the skeleton CUs (SplitDWARFInlining) - then there's value in
-// composing split-dwarf and line-tables-only, so let those compose
-// naturally in that case. And if you just turned off debug info,
-// (-gsplit-dwarf -g0) - do that.
-if (DwarfFission != DwarfFissionKind::None) {
-  if (A->getIndex() > SplitDWARFArg->getIndex()) {
-if (DebugInfoKind == codegenoptions::NoDebugInfo ||
-DebugInfoKind == codegenoptions::DebugDirectivesOnly ||
-(DebugInfoKind == codegenoptions::DebugLineTablesOnly &&
- SplitDWARFInlining))
-  DwarfFission = DwarfFissionKind::None;
-  } else if (SplitDWARFInlining)
-DebugInfoKind = codegenoptions::NoDebugInfo;
-}
-  } else {
-// For any other 'g' option, use Limited.
-DebugInfoKind = codegenoptions::LimitedDebugInfo;
-  }
-} else {
-  DebugInfoKind = codegenoptions::LimitedDebugInfo;
+  if (const Arg *A =
+  Args.getLastArg(options::OPT_g_Group, options::OPT_gsplit_dwarf,
+  options::OPT_gsplit_dwarf_EQ)) {
+DebugInfoKind = codegenoptions::LimitedDebugInfo;
+
+// If the last option explicitly specified a debug-info level, use it.
+if (checkDebugInfoOption(A, Args, D, TC) &&
+A->getOption().matches(options::OPT_gN_Group)) {
+  DebugInfoKind = DebugLevelToInfoKind(*A);
+  // For -g0 or -gline-tables-only, drop -gsplit-dwarf. This gets a bit more
+  // complicated if you've disabled inline info in the skeleton CUs
+  // (SplitDWARFInlining) - then there's value in composing split-dwarf and
+  // line-tables-only, so let those compose naturally in that case.
+  if (DebugInfoKind == codegenoptions::NoDebugInfo ||
+  DebugInfoKind == codegenoptions::DebugDirectivesOnly ||
+  (DebugInfoKind == codegenoptions::DebugLineTablesOnly &&
+   SplitDWARFInlining))
+DwarfFission = DwarfFissionKind::None;
 }
   }
 
@@ -3261,16 +3250,12 @@
   }
 }
 
-  // -gsplit-dwarf should turn on -g and enable the backend dwarf
-  // splitting and extraction.
+  // -gsplit-dwarf enables the backend dwarf splitting and extraction.
   if (T.isOSBinFormatELF()) {
 if (!SplitDWARFInlining)
   CmdArgs.push_back("-fno-split-dwarf-inlini

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59746#1462352 , @jhenderson wrote:

> I've not looked at the implementation in depth, but cl::DefaultOption seems 
> like a nice way of handling this. Please make sure that there is testing in 
> llvm-readobj and llvm-objdump that test their own short alias interpretation 
> somewhere though.


While I'm still working on unittests, I have added a few tests that should 
cover this.  Please let me know if you have any concerns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 194680.
ilya-biryukov marked 19 inline comments as done.
ilya-biryukov added a comment.

- Remove a spamy debug tracing output.
- Less debug output, move stuff around, more comments.
- Add methods that map expanded tokens to raw tokens.
- Rename toOffsetsRange.
- Remove default ctor of Token.
- Introduce a struct for storing FileID and a pair of offsets.
- Update comments.
- Add a test for macro expansion at the end of the file.
- Misc fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59887

Files:
  clang/include/clang/Tooling/Syntax/Tokens.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/Syntax/CMakeLists.txt
  clang/lib/Tooling/Syntax/Tokens.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/Syntax/CMakeLists.txt
  clang/unittests/Tooling/Syntax/TokensTest.cpp

Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -0,0 +1,631 @@
+//===- TokensTest.cpp -===//
+//
+// 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 "clang/Tooling/Syntax/Tokens.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/Expr.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.def"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/Utils.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Lex/Token.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Support/raw_os_ostream.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Testing/Support/Annotations.h"
+#include "gmock/gmock-more-matchers.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace clang;
+using namespace clang::syntax;
+
+using ::testing::AllOf;
+using ::testing::Contains;
+using ::testing::ElementsAre;
+using ::testing::Matcher;
+using ::testing::Pointwise;
+
+namespace {
+// Matchers for syntax::Token.
+MATCHER_P(Kind, K, "") { return arg.kind() == K; }
+MATCHER_P2(HasText, Text, SourceMgr, "") {
+  return arg.text(*SourceMgr) == Text;
+}
+MATCHER_P2(IsIdent, Text, SourceMgr, "") {
+  return arg.kind() == tok::identifier && arg.text(*SourceMgr) == Text;
+}
+/// Checks the start and end location of a token are equal to SourceRng.
+MATCHER_P(RangeIs, SourceRng, "") {
+  return arg.location() == SourceRng.first &&
+ arg.endLocation() == SourceRng.second;
+}
+/// Checks the passed tuple has two similar tokens, i.e. both are of the same
+/// kind and have the same text if they are identifiers.
+/// Ignores differences in kind between the raw and non-raw mode.
+MATCHER_P(IsSameToken, SourceMgr, "") {
+  auto ToEquivalenceClass = [](tok::TokenKind Kind) {
+if (Kind == tok::identifier || Kind == tok::raw_identifier ||
+tok::getKeywordSpelling(Kind) != nullptr)
+  return tok::identifier;
+if (Kind == tok::string_literal || Kind == tok::header_name)
+  return tok::string_literal;
+return Kind;
+  };
+
+  auto &L = std::get<0>(arg);
+  auto &R = std::get<1>(arg);
+  if (ToEquivalenceClass(L.kind()) != ToEquivalenceClass(R.kind()))
+return false;
+  return L.text(*SourceMgr) == L.text(*SourceMgr);
+}
+} // namespace
+
+// Actual test fixture lives in the syntax namespace as it's a friend of
+// TokenBuffer.
+class syntax::TokensTest : public ::testing::Test {
+public:
+  /// Run the clang frontend, collect the preprocessed tokens from the frontend
+  /// invocation and store them in this->Buffer.
+  /// This also clears SourceManager before running the compiler.
+  void recordTokens(llvm::StringRef Code) {
+class RecordTokens : public ASTFrontendAction {
+public:
+  explicit RecordTokens(TokenBuffer &Result) : Result(Result) {}
+
+  bool BeginSourceFileAction(CompilerInstance &CI) override {
+assert(!Co

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Fix header comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59887

Files:
  clang/include/clang/Tooling/Syntax/Tokens.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/Syntax/CMakeLists.txt
  clang/lib/Tooling/Syntax/Tokens.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/Syntax/CMakeLists.txt
  clang/unittests/Tooling/Syntax/TokensTest.cpp

Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -0,0 +1,631 @@
+//===- TokensTest.cpp -===//
+//
+// 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 "clang/Tooling/Syntax/Tokens.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/Expr.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.def"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/Utils.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Lex/Token.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Support/raw_os_ostream.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Testing/Support/Annotations.h"
+#include "gmock/gmock-more-matchers.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace clang;
+using namespace clang::syntax;
+
+using ::testing::AllOf;
+using ::testing::Contains;
+using ::testing::ElementsAre;
+using ::testing::Matcher;
+using ::testing::Pointwise;
+
+namespace {
+// Matchers for syntax::Token.
+MATCHER_P(Kind, K, "") { return arg.kind() == K; }
+MATCHER_P2(HasText, Text, SourceMgr, "") {
+  return arg.text(*SourceMgr) == Text;
+}
+MATCHER_P2(IsIdent, Text, SourceMgr, "") {
+  return arg.kind() == tok::identifier && arg.text(*SourceMgr) == Text;
+}
+/// Checks the start and end location of a token are equal to SourceRng.
+MATCHER_P(RangeIs, SourceRng, "") {
+  return arg.location() == SourceRng.first &&
+ arg.endLocation() == SourceRng.second;
+}
+/// Checks the passed tuple has two similar tokens, i.e. both are of the same
+/// kind and have the same text if they are identifiers.
+/// Ignores differences in kind between the raw and non-raw mode.
+MATCHER_P(IsSameToken, SourceMgr, "") {
+  auto ToEquivalenceClass = [](tok::TokenKind Kind) {
+if (Kind == tok::identifier || Kind == tok::raw_identifier ||
+tok::getKeywordSpelling(Kind) != nullptr)
+  return tok::identifier;
+if (Kind == tok::string_literal || Kind == tok::header_name)
+  return tok::string_literal;
+return Kind;
+  };
+
+  auto &L = std::get<0>(arg);
+  auto &R = std::get<1>(arg);
+  if (ToEquivalenceClass(L.kind()) != ToEquivalenceClass(R.kind()))
+return false;
+  return L.text(*SourceMgr) == L.text(*SourceMgr);
+}
+} // namespace
+
+// Actual test fixture lives in the syntax namespace as it's a friend of
+// TokenBuffer.
+class syntax::TokensTest : public ::testing::Test {
+public:
+  /// Run the clang frontend, collect the preprocessed tokens from the frontend
+  /// invocation and store them in this->Buffer.
+  /// This also clears SourceManager before running the compiler.
+  void recordTokens(llvm::StringRef Code) {
+class RecordTokens : public ASTFrontendAction {
+public:
+  explicit RecordTokens(TokenBuffer &Result) : Result(Result) {}
+
+  bool BeginSourceFileAction(CompilerInstance &CI) override {
+assert(!Collector && "expected only a single call to BeginSourceFile");
+Collector.emplace(CI.getPreprocessor());
+return true;
+  }
+  void EndSourceFileAction() override {
+assert(Collector && "BeginSourceFileAction was never called");
+Result = std::move(*Collector).consume();
+  }
+
+  std::unique_ptr
+  CreateASTConsumer(CompilerInstan

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

The new version address most of the comments, there are just a few left in the 
code doing the mapping from Dmitri, I'll look into simplifying and removing the 
possibly redundant checks.
Please take a look at this version, this is very close to the final state.




Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:72
+  /// macro or a name, arguments and parentheses of a function-like macro.
+  llvm::ArrayRef macroTokens(const TokenBuffer &B) const;
+  /// The range covering macroTokens().

ilya-biryukov wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > gribozavr wrote:
> > > > `invocationTokens` or `macroInvocationTokens`
> > > The plan is to eventually include the macro directives tokens, hence the 
> > > name.
> > > `invocationTokens` are somewhat inaccurate in that case, can't come up 
> > > with a better name.
> > I think your reply applies to TokenBuffer::macroTokens(), not 
> > MacroInvocation::macroTokens().
> > 
> > +1 to invocationTokens here.
> Yeah, sorry, I have mixed up these two functions with the same name. Will 
> change to `invocationTokens`.
The Mapping is now internal and only stores the indicies.
The names for two kinds of those are "raw" and "expanded", happy to consider 
alternatives for both.



Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:8
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLING_SYNTAX_TOKEN_BUFFER_H

sammccall wrote:
> file comment?
> sometimes they're covered by the class comments, but I think there's a bit to 
> say here.
> in particular the logical model (how we model the preprocessor, the two token 
> streams and types of mappings between them) might go here.
Added a file comment explaining the basic concepts inside the file.



Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:32
+public:
+  Token() = default;
+  Token(SourceLocation Location, unsigned Length, tok::TokenKind Kind)

sammccall wrote:
> what's the default state (and why have one?) do you need a way to query 
> whether a token is "valid"?
> (I'd avoid just relying on length() == 0 or location().isInvalid() because 
> it's not obvious to callers this can happen)
Got rid of default state altogether. I haven' seen the use-cases where it's 
important so far.
Using `Optional` for invalid tokens seems like a cleaner design anyway 
(we did not need it so far).



Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:60
+
+/// A top-level macro invocation inside a file, e.g.
+///   #define FOO 1+2

sammccall wrote:
> If you're going to say "invocation" in the name, say "use" in the comment (or 
> vice versa!)
The class is now internal to `TokenBuffer` and is called `Mapping`.



Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:66
+///macroTokens = {'BAR', '(', '1', ')'}.
+class MacroInvocation {
+public:

sammccall wrote:
> I'd suggest a more natural order is token -> tokenbuffer -> macroinvocation. 
> Forward declare?
> 
> This is because the token buffer exposes token streams, which are probably 
> the second-most important concept in the file (after tokens)
Done. I forgot that you could have members of type `vector`  where `T` is 
incomplete.



Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:74
+  /// The range covering macroTokens().
+  std::pair macroRange(const TokenBuffer &B,
+   const SourceManager &SM) const;

sammccall wrote:
> It seems weirdly non-orthogonal to be able to get the range but not the file 
> ID.
> 
> I'd suggest either adding another accessor for that, or returning a `struct 
> BufferRange { FileID File; unsigned Begin, End; }` (with a better name.)
> 
> I favor the latter, because `.first` and `.second` don't offer the reader 
> semantic hints at the callsite, and because that gives a nice place to 
> document the half-open nature of the interval.
> 
> Actually, I'd suggest adding such a struct accessor to Token too.
Done. I've used the name `FileRange` instead (the idea is that it's pointing to 
a substring in a file).
Let me know what you think about the name.




Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:87
+  /// to the identifier token corresponding to a name of the expanded macro.
+  unsigned BeginMacroToken = 0;
+  /// End index in TokenBuffer::macroTokens().

sammccall wrote:
> please rename along with macroTokens() function
This is now `BeginRawToken` and `EndRawToken`.
As usually, open to suggestions wrt to naming.



Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:92
+
+/// A list of tokens obtained by lexing and preprocessing a text buffer and a
+/// set of helpers to a

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Store a source manager in a token buffer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59887

Files:
  clang/include/clang/Tooling/Syntax/Tokens.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/Syntax/CMakeLists.txt
  clang/lib/Tooling/Syntax/Tokens.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/Syntax/CMakeLists.txt
  clang/unittests/Tooling/Syntax/TokensTest.cpp

Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -0,0 +1,629 @@
+//===- TokensTest.cpp -===//
+//
+// 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 "clang/Tooling/Syntax/Tokens.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/Expr.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.def"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/Utils.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Lex/Token.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Support/raw_os_ostream.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Testing/Support/Annotations.h"
+#include "gmock/gmock-more-matchers.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace clang;
+using namespace clang::syntax;
+
+using ::testing::AllOf;
+using ::testing::Contains;
+using ::testing::ElementsAre;
+using ::testing::Matcher;
+using ::testing::Pointwise;
+
+namespace {
+// Matchers for syntax::Token.
+MATCHER_P(Kind, K, "") { return arg.kind() == K; }
+MATCHER_P2(HasText, Text, SourceMgr, "") {
+  return arg.text(*SourceMgr) == Text;
+}
+MATCHER_P2(IsIdent, Text, SourceMgr, "") {
+  return arg.kind() == tok::identifier && arg.text(*SourceMgr) == Text;
+}
+/// Checks the start and end location of a token are equal to SourceRng.
+MATCHER_P(RangeIs, SourceRng, "") {
+  return arg.location() == SourceRng.first &&
+ arg.endLocation() == SourceRng.second;
+}
+/// Checks the passed tuple has two similar tokens, i.e. both are of the same
+/// kind and have the same text if they are identifiers.
+/// Ignores differences in kind between the raw and non-raw mode.
+MATCHER_P(IsSameToken, SourceMgr, "") {
+  auto ToEquivalenceClass = [](tok::TokenKind Kind) {
+if (Kind == tok::identifier || Kind == tok::raw_identifier ||
+tok::getKeywordSpelling(Kind) != nullptr)
+  return tok::identifier;
+if (Kind == tok::string_literal || Kind == tok::header_name)
+  return tok::string_literal;
+return Kind;
+  };
+
+  auto &L = std::get<0>(arg);
+  auto &R = std::get<1>(arg);
+  if (ToEquivalenceClass(L.kind()) != ToEquivalenceClass(R.kind()))
+return false;
+  return L.text(*SourceMgr) == L.text(*SourceMgr);
+}
+} // namespace
+
+// Actual test fixture lives in the syntax namespace as it's a friend of
+// TokenBuffer.
+class syntax::TokensTest : public ::testing::Test {
+public:
+  /// Run the clang frontend, collect the preprocessed tokens from the frontend
+  /// invocation and store them in this->Buffer.
+  /// This also clears SourceManager before running the compiler.
+  void recordTokens(llvm::StringRef Code) {
+class RecordTokens : public ASTFrontendAction {
+public:
+  explicit RecordTokens(TokenBuffer &Result) : Result(Result) {}
+
+  bool BeginSourceFileAction(CompilerInstance &CI) override {
+assert(!Collector && "expected only a single call to BeginSourceFile");
+Collector.emplace(CI.getPreprocessor());
+return true;
+  }
+  void EndSourceFileAction() override {
+assert(Collector && "BeginSourceFileAction was never called");
+Result = std::move(*Collector).consume();
+  }
+
+  std::unique_ptr
+  CreateASTCon

[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 194684.
Tyker added a comment.

@Quuxplusone  edited based of feedback

i simplified the test but didn't put the original because this one test 
arguments for variable and literal and there may be corner case differences


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

https://reviews.llvm.org/D60561

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx1y.cpp

Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1159,3 +1159,18 @@
 enum class InEnum3 {
   THREE = indirect_builtin_constant_p("abc")
 };
+
+constexpr int f1(int i) {
+  i -= 1;
+  return 1 << i; // expected-note {{negative shift count -2}}
+}
+
+constexpr int f2(int i) {
+  i -= 1;
+  // expected-note@+1 {{'f1(-1)}}
+  return f1(i);
+}
+
+// expected-error@+2 {{constant expression}}
+// expected-note@+1 {{'f2(0)}}
+constexpr int a = f2(0);
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -476,9 +476,14 @@
 const LValue *This;
 
 /// Arguments - Parameter bindings for this function call, indexed by
-/// parameters' function scope indices.
+/// parameters' function scope indices. may be modified by called function
 APValue *Arguments;
 
+/// ConstantArgs - Parameter bindings for this function call, indexed by
+/// parameters' function scope indices. can't be modified. used for
+/// diagnostics
+const APValue *ConstantArgs;
+
 // Note that we intentionally use std::map here so that references to
 // values are stable.
 typedef std::pair MapKeyTy;
@@ -519,7 +524,7 @@
 
 CallStackFrame(EvalInfo &Info, SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
-   APValue *Arguments);
+   APValue *Arguments, const APValue *ConstantArgs);
 ~CallStackFrame();
 
 // Return the temporary for Key whose version number is Version.
@@ -789,14 +794,15 @@
 bool checkingForOverflow() { return EvalMode == EM_EvaluateForOverflow; }
 
 EvalInfo(const ASTContext &C, Expr::EvalStatus &S, EvaluationMode Mode)
-  : Ctx(const_cast(C)), EvalStatus(S), CurrentCall(nullptr),
-CallStackDepth(0), NextCallIndex(1),
-StepsLeft(getLangOpts().ConstexprStepLimit),
-BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),
-EvaluatingDecl((const ValueDecl *)nullptr),
-EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
-HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
-InConstantContext(false), EvalMode(Mode) {}
+: Ctx(const_cast(C)), EvalStatus(S), CurrentCall(nullptr),
+  CallStackDepth(0), NextCallIndex(1),
+  StepsLeft(getLangOpts().ConstexprStepLimit),
+  BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr,
+  nullptr),
+  EvaluatingDecl((const ValueDecl *)nullptr),
+  EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
+  HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
+  InConstantContext(false), EvalMode(Mode) {}
 
 void setEvaluatingDecl(APValue::LValueBase Base, APValue &Value) {
   EvaluatingDecl = Base;
@@ -1235,9 +1241,10 @@
 
 CallStackFrame::CallStackFrame(EvalInfo &Info, SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
-   APValue *Arguments)
+   APValue *Arguments, const APValue *CArgs)
 : Info(Info), Caller(Info.CurrentCall), Callee(Callee), This(This),
-  Arguments(Arguments), CallLoc(CallLoc), Index(Info.NextCallIndex++) {
+  Arguments(Arguments), ConstantArgs(CArgs), CallLoc(CallLoc),
+  Index(Info.NextCallIndex++) {
   Info.CurrentCall = this;
   ++Info.CallStackDepth;
 }
@@ -1679,7 +1686,7 @@
   Out << ", ";
 
 const ParmVarDecl *Param = *I;
-const APValue &Arg = Frame->Arguments[ArgIndex];
+const APValue &Arg = Frame->ConstantArgs[ArgIndex];
 Arg.printPretty(Out, Frame->Info.Ctx, Param->getType());
 
 if (ArgIndex == 0 && IsMemberCall)
@@ -4455,7 +4462,11 @@
   if (!Info.CheckCallLimit(CallLoc))
 return false;
 
-  CallStackFrame Frame(Info, CallLoc, Callee, This, ArgValues.data());
+  // ArgValues - may be modified by the called function.
+  // ConstantArgs - can't be modified by the called function
+  const ArgVector ConstantArgs(ArgValues.begin(), ArgValues.end());
+  CallStackFrame Frame(Info, CallLoc, Callee, This, ArgValues.data(),
+   ConstantArgs.data());
 
   // For a trivial copy or move assignment, perform an APValue copy. This is
   // 

[PATCH] D59639: [clangd] Print template arguments helper

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 194686.
kadircet marked 6 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59639

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/unittests/clangd/CMakeLists.txt
  clang-tools-extra/unittests/clangd/FindSymbolsTests.cpp
  clang-tools-extra/unittests/clangd/PrintASTTests.cpp
  clang/lib/AST/TypePrinter.cpp

Index: clang/lib/AST/TypePrinter.cpp
===
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -1632,6 +1632,19 @@
   return A.getArgument();
 }
 
+static void printArgument(const TemplateArgument &A, const PrintingPolicy &PP,
+  llvm::raw_ostream &OS) {
+  A.print(PP, OS);
+}
+
+static void printArgument(const TemplateArgumentLoc &A,
+  const PrintingPolicy &PP, llvm::raw_ostream &OS) {
+  const TemplateArgument::ArgKind &Kind = A.getArgument().getKind();
+  if (Kind == TemplateArgument::ArgKind::Type)
+return A.getTypeSourceInfo()->getType().print(OS, PP);
+  return A.getArgument().print(PP, OS);
+}
+
 template
 static void printTo(raw_ostream &OS, ArrayRef Args,
 const PrintingPolicy &Policy, bool SkipBrackets) {
@@ -1653,7 +1666,8 @@
 } else {
   if (!FirstArg)
 OS << Comma;
-  Argument.print(Policy, ArgOS);
+  // Tries to print the argument with location info if exists.
+  printArgument(Arg, Policy, ArgOS);
 }
 StringRef ArgString = ArgOS.str();
 
Index: clang-tools-extra/unittests/clangd/PrintASTTests.cpp
===
--- /dev/null
+++ clang-tools-extra/unittests/clangd/PrintASTTests.cpp
@@ -0,0 +1,100 @@
+//===--- PrintASTTests.cpp - C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AST.h"
+#include "Annotations.h"
+#include "Protocol.h"
+#include "SourceCode.h"
+#include "TestTU.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest-param-test.h"
+#include "gtest/gtest.h"
+#include "gtest/internal/gtest-param-util-generated.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using testing::ElementsAreArray;
+
+struct Case {
+  const char *AnnotatedCode;
+  std::vector Expected;
+};
+class ASTUtils : public testing::Test,
+ public ::testing::WithParamInterface {};
+
+TEST_P(ASTUtils, PrintTemplateArgs) {
+  auto Pair = GetParam();
+  Annotations Test(Pair.AnnotatedCode);
+  auto AST = TestTU::withCode(Test.code()).build();
+  struct Visitor : RecursiveASTVisitor {
+Visitor(std::vector Points) : Points(std::move(Points)) {}
+bool VisitNamedDecl(const NamedDecl *ND) {
+  auto Pos = sourceLocToPosition(ND->getASTContext().getSourceManager(),
+ ND->getLocation());
+  if (Pos != Points[TemplateArgsAtPoints.size()])
+return true;
+  TemplateArgsAtPoints.push_back(printTemplateSpecializationArgs(*ND));
+  return true;
+}
+std::vector TemplateArgsAtPoints;
+const std::vector Points;
+  };
+  Visitor V(Test.points());
+  V.TraverseDecl(AST.getASTContext().getTranslationUnitDecl());
+  EXPECT_THAT(V.TemplateArgsAtPoints, ElementsAreArray(Pair.Expected));
+}
+
+INSTANTIATE_TEST_CASE_P(ASTUtilsTests, ASTUtils,
+testing::ValuesIn(std::vector({
+{
+R"cpp(
+  template  class Bar {};
+  template <> class ^Bar {};)cpp",
+{""}},
+{
+R"cpp(
+  template  class Bar {};
+  template  class Z, int Q>
+  struct Foo {};
+  template struct ^Foo;
+  template 
+  struct ^Foo {};)cpp",
+{"", ""}},
+{
+R"cpp(
+  template  void Foz() {};
+  template <> void ^Foz<3, 5, 8>() {};)cpp",
+{"<3, 5, 8>"}},
+{
+R"cpp(
+  template  class Bar {};
+  template  class ...>
+ 

[PATCH] D59639: [clangd] Print template arguments helper

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/AST.cpp:139
+  // location information.
+  printTemplateArgumentList(OS, Cls->getTemplateArgs().asArray(), Policy);
+}

ioeric wrote:
> Could you also add a test case for this with the current behavior and a FIXME?
Adding the test case, but the problem is there wouldn't be a change in 
behavior(at least not in the cases that I can think of) since a full 
specialization doesn't have any dependent types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59639



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

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

[Russell McClellan via Phabricator via cfe-commits]

russellmcc added a comment.

Thanks for the explanation!  I do understand your philosophy on this, and agree 
with the desired behavior case you brought up where you have put in new braces.

After thinking about this more, the thing I really care about is that 
clang-format is idempotent with a line filter - i.e., running it twice should 
always have the same effect as running it once.

So, either this fix, or your proposed fix of fixing all lines until the next 
correct indentation would meet that idempotence criteria.

However, I think in this particular case I still prefer my fix - to me, line 
filter is meant to limit the effect of clang-format to just fix a particular 
change and the fallout from that.  However, if the lines _after_ a change were 
wrong before, this feels very unrelated to the change that was made - why is 
now the time to fix these unrelated lines?


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

https://reviews.llvm.org/D54881



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

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D60151#1454802 , @hintonda wrote:

> In D60151#1454741 , @alexfh wrote:
>
> > In D60151#1454105 , @hintonda 
> > wrote:
> >
> > > - Rename llvm directory to llvm_project.
> > > - Change llvm- to llvm-project-.
> > > - Rename files.
> >
> >
> > Awesome! Thanks for doing this. Could you ensure that the add_new_check.py 
> > script still works?
>
>
> Thanks for mentioning this.  Found a missing rename discovered by 
> add_new_check.py which I'll checkin shortly.  However, I'm not sure how best 
> to solve a rename issue.
>
> In rename_check.py, module names can't be hyphenated, e.g.:
>
>   190:  old_module = args.old_check_name.split('-')[0]
>   191:  new_module = args.new_check_name.split('-')[0]
>   
>
> If we keep this pattern, then "llvm-project" won't work.  It would need to be 
> something like "llvmproject" or something else.  Please let me know your 
> preference.


Hmm, that's unfortunate behavior. I guess my preference would be to go with 
`llvm_project`, but it's not a strong preference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



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

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In D54881#1462804 , @russellmcc wrote:

> Thanks for the explanation!  I do understand your philosophy on this, and 
> agree with the desired behavior case you brought up where you have put in new 
> braces.
>
> After thinking about this more, the thing I really care about is that 
> clang-format is idempotent with a line filter - i.e., running it twice should 
> always have the same effect as running it once.
>
> So, either this fix, or your proposed fix of fixing all lines until the next 
> correct indentation would meet that idempotence criteria.
>
> However, I think in this particular case I still prefer my fix - to me, line 
> filter is meant to limit the effect of clang-format to just fix a particular 
> change and the fallout from that.  However, if the lines _after_ a change 
> were wrong before, this feels very unrelated to the change that was made - 
> why is now the time to fix these unrelated lines?


I agree and would be happy with the change if it would only change the 
line-filtered workflow, but this afaict (unless I'm missing something :) will 
also affect the workflow where the provided range is 0-length range, which has 
an implicit "format stuff around this" request from the user inside it. I'd be 
happy with a patch that differentiates these two sides.


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

https://reviews.llvm.org/D54881



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

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea created this revision.
gtbercea added reviewers: ABataev, AlexEichenberger, caomhin.
Herald added subscribers: cfe-commits, jdoerfert, jfb, guansong, jholewinski.
Herald added a project: clang.

This patch adds support for the registration of the requires directives with 
the runtime.

Each requires directive clause will enable a particular flag to be set.

The set of flags is passed to the runtime to be checked for compatibility with 
other such flags coming from other object files.

The registration function is called whenever OpenMP is present even if a 
requires directive is not present. This helps detect cases in which requires 
directives are used inconsistently.


Repository:
  rC Clang

https://reviews.llvm.org/D60568

Files:
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  test/OpenMP/openmp_offload_registration.cpp

Index: test/OpenMP/openmp_offload_registration.cpp
===
--- test/OpenMP/openmp_offload_registration.cpp
+++ test/OpenMP/openmp_offload_registration.cpp
@@ -26,7 +26,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 2, [[DEVTY]]* getelementptr inbounds ([2 x [[DEVTY]]], [2 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 // Check presence of foo() and the outlined target region
 // CHECK: define void [[FOO:@.+]]()
@@ -34,6 +34,11 @@
 
 // Check registration and unregistration code.
 
+// CHECK: define internal void @.omp_offloading.requires_reg()
+// CHECK: call i32 @__tgt_register_requires(i64 0)
+// CHECK: ret void
+// CHECK: declare i32 @__tgt_register_requires(i64)
+
 // CHECK: define internal void @[[UNREGFN:.+]](i8*)
 // CHECK-SAME: comdat($[[REGFN]]) {
 // CHECK: call i32 @__tgt_unregister_lib([[DSCTY]]* [[DESC]])
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -291,6 +291,8 @@
 
   typedef std::vector CtorList;
 
+  bool HasRequiresUnifiedSharedMemory = false;
+
 private:
   ASTContext &Context;
   const LangOptions &LangOpts;
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -410,6 +410,10 @@
   AddGlobalCtor(CudaCtorFunction);
   }
   if (OpenMPRuntime) {
+if (llvm::Function *OpenMPRequiresDirectiveRegFun =
+OpenMPRuntime->emitRequiresDirectiveRegFun()) {
+  AddGlobalCtor(OpenMPRequiresDirectiveRegFun, 0, nullptr);
+}
 if (llvm::Function *OpenMPRegistrationFunction =
 OpenMPRuntime->emitRegistrationFunction()) {
   auto ComdatKey = OpenMPRegistrationFunction->hasComdat() ?
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.h
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.h
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.h
@@ -383,7 +383,8 @@
 
   /// Perform check on requires decl to ensure that target architecture
   /// supports unified addressing
-  void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const override;
+  void checkArchForUnifiedAddressing(CodeGenModule &CGM,
+ const OMPRequiresDecl *D) const override;
 
   /// Returns default address space for the constant firstprivates, __constant__
   /// address space by default.
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4942,7 +4942,8 @@
 /// Check to see if target architecture supports unified addressing which is
 /// a restriction for OpenMP requires clause "unified_shared_memory".
 void CGOpenMPRuntimeNVPTX::checkArchForUnifiedAddressing(
-const OMPRequiresDecl *D) const {
+CodeGenModule &CGM, const OMPRequiresDecl *D) const {
+  CGOpenMPRuntime::checkArchForUnifiedAddressing(CGM, D);
   for (const OMPClause *Clause : D->clauselists()) {
 if (Clause->getClauseKind() == OMPC_unified_shared_memory) {
   switch (getCudaArch(CGM)) {
Index: lib/CodeGen/CGOpenMPRuntime.h
===
--- lib/CodeGen/CGOpenMPRuntime.h
+++ lib/CodeGen/CGOpen

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

are we supporting "-llvm-*" in existing .clang-tidy files?

If people selectively turn checkers off, won't all of a sudden everyone start 
getting llvm-project checks and fixes turned back on?

https://github.com/search?q=-llvm-%2A&type=Code

maybe we need to add something like:

  llvm-header-guard (redirects to llvm-project-header-guard) 
  llvm-header-include-order (redirects to llvm-project-include-order) 

  llvm-header-namespace-comment(redirects to llvm-project-namespace-comment) 

  llvm-header-twine-local(redirects to llvm-project-twine-local) 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



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

[PATCH] D60542: Add support for attributes on @implementations in Objective-C

[John McCall via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60542



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

[PATCH] D60139: [clang-tidy] Add bugprone-placement-new-target-type-mismatch check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D60139#1461269 , @DennisL wrote:

> In D60139#1460233 , @JonasToth wrote:
>
> > Hey Dennis,
> >
> > my 2cents on the check. I think it is very good to have! Did you check 
> > coding guidelines if they say something to this issue? (e.g. 
> > cppcoreguidelines, hicpp, cert) As we have modules for them it would be 
> > great to make aliases to this check if they demand this to be checked.
>
>
> Thanks for the great suggestions. Updated the diff according to the feedback. 
> Also checked with cppcoreguidelines, hicpp as well as cert. Only cert has a 
> related, yet different rule 
> 
>  stating that calls to placement new shall be provided with properly aligned 
> pointers. I'd say this should be a distinct check. Happy to work on it after 
> this one.


What is the rationale for separating those checks? It sort of feels like there 
is some overlap between alignment, size, and buffer type.

I'm not certain I agree with the way this check operates -- what problem is it 
trying to solve? For instance, this code would trigger on this check, but is 
perfectly valid code:

  static_assert(sizeof(int) == sizeof(float));
  static_assert(alignof(int) == alignof(float));
  int buffer;
  float *fp = new (&buffer) float;

The situations I can think of where the type mismatch is an issue would be for 
types with nontrivial special members, so there are times when this check makes 
sense, but perhaps it's too restrictive currently, but you may have a different 
problem in mind?




Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:20
+namespace {
+AST_MATCHER(Expr, isPlacementNewExpr) {
+  const auto *NewExpr = dyn_cast(&Node);

This should match on `CXXNewExpr` instead of `Expr`, then you won't need the 
`dyn_cast` below.



Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:29
+  Finder->addMatcher(
+  expr(isPlacementNewExpr(), unless(hasDescendant(unresolvedLookupExpr(
+  .bind("NewExpr"),

This should probably use `cxxNewExpr()` instead of `expr()` so you match on 
something more specific.



Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:38
+  assert(NewExpr && "Matched node bound by 'NewExpr' shoud be a 'CXXNewExpr'");
+  assert(NewExpr->getNumPlacementArgs() != 0 && "");
+

This assert message looks a bit incomplete. ;-)



Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:43
+  assert(PlacementExpr != nullptr && "PlacementExpr should not be null");
+  const CastExpr *Cast = dyn_cast(PlacementExpr);
+  if (Cast == nullptr)

You can use `const auto *` here.



Comment at: test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp:56
+  new (ptr) float[13];
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and 
allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}

While this code definitely has a bug from the buffer overflow, I don't think 
the diagnostic is valuable here. This is a very common pattern and I suspect 
this will yield a lot of false positives, unless the check starts taking 
alignment and buffer size into consideration.


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

https://reviews.llvm.org/D60139



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

[PATCH] D60544: Support objc_nonlazy_class attribute on Objective-C implementations

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Should we have a special `has_feature` test to check that this attribute is 
allowed on implementations?  We've done that in the past when expanding the set 
of subjects for an attribute.  But that's not necessary if we haven't made this 
attribute part of a public Xcode release yet, because then the attribute is 
just understood to always apply to implementations; I can't remember when 
exactly this was added and what releases it's made it into.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60544



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

[PATCH] D59605: [clangd] Introduce background-indexer

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Looking at the current state of `BackgroundIndex`, it has the following 
implementation details:

- Loading of shards from storage
- Storing of shards to storage
- Collecting symbols from a TU
- Sharding of symbol information collected from one translation unit
- Performing all these tasks in a thread pool that can change scheduling 
priority of the thread depending on the task running.

Our requirements from the tool that currently is not possible through 
`BackgroundIndex`:

- Running tasks without lowering scheduling priority
- Blocking until all of the indexing actions finishes

I believe we can achive the first one by adding some options. The second one is 
already possible but it is using a test method we implemented for tests, as you 
mentioned we can just change the name.
Of course the above mentioned solutions would imply exposing that functionality 
to every user of `BackgroundIndex`.

I feel like it is better than exposing rest of the implementation details I 
mentioned in the first section(and also would be a lot faster to implement). 
WDYT?




Comment at: clang-tools-extra/clangd/background-indexer/BackgroundIndexer.cpp:65
+  llvm::sys::path::append(DummyFile, "dummy.cpp");
+  CDB.getCompileCommand(DummyFile);
+

ilya-biryukov wrote:
> We seem to be fighting with the interface we defined here.
> If we need to support an operation of re-building the index for the 
> underlying CDB, let's add it explicitly.
I should've rather performed this through `enqueue`, will change to use that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59605



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

[PATCH] D60542: Add support for attributes on @implementations in Objective-C

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a question.




Comment at: clang/test/FixIt/fixit-pragma-attribute.cpp:19-20
 #pragma clang attribute push 
(__attribute__((annotate("subRuleContradictions"))), apply_to = any(variable, 
variable(is_parameter), function(is_member), variable(is_global)))
-// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:108-[[@LINE-1]]:132}:""
-// CHECK: fix-it:{{.*}}:{[[@LINE-2]]:153-[[@LINE-2]]:172}:""
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:153-[[@LINE-1]]:172}:""
+// CHECK: fix-it:{{.*}}:{[[@LINE-2]]:108-[[@LINE-2]]:132}:""
 

Any idea why this swapped?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60542



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

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.



> I agree and would be happy with the change if it would only change the 
> line-filtered workflow, but this afaict (unless I'm missing something :) will 
> also affect the workflow where the provided range is 0-length range, which 
> has an implicit "format stuff around this" request from the user inside it. 
> I'd be happy with a patch that differentiates these two sides.

This use case needs capturing in a unit test..it isn't obvious


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

https://reviews.llvm.org/D54881



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

[PATCH] D60544: Support objc_nonlazy_class attribute on Objective-C implementations

[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, modulo the `__has_feature` question.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60544



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

[PATCH] D59605: [clangd] Introduce background-indexer

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a subscriber: sammccall.
ilya-biryukov added a comment.

To reiterate the offline discussion: the tool seems useful to me, but it would 
be best to keep the client code simpler, it's currently fighting with 
`BackgroundIndex` because it's trying to hijack some of its functionality.
More specifically, I propose to add a function to `BackgroundIndex.h` that 
would be a one-shot update of the index, the client code would become 
significantly simpler and we would have more flexibility in how we move code 
around in `BackgroundIndex.cpp` to actually make this happen.

Adding @sammccall, who might also be interested in this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59605



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

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 194708.
hintonda added a comment.

- Add unittest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746

Files:
  clang/lib/Tooling/CommonOptionsParser.cpp
  llvm/docs/CommandLine.rst
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/test/Support/check-default-options.txt
  llvm/tools/llvm-opt-report/OptReport.cpp
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -620,6 +620,74 @@
   }
 }
 
+TEST(CommandLineTest, DefaultOptions) {
+  cl::ResetCommandLineParser();
+
+  StackOption Bar("bar", cl::sub(*cl::AllSubCommands),
+   cl::DefaultOption);
+  StackOption Bar_Alias(
+  "b", cl::desc("Alias for -bar"), cl::aliasopt(Bar), cl::DefaultOption);
+
+  StackOption Foo("foo", cl::init(false), cl::sub(*cl::AllSubCommands),
+cl::DefaultOption);
+  StackOption Foo_Alias("f", cl::desc("Alias for -foo"),
+ cl::aliasopt(Foo), cl::DefaultOption);
+
+  StackSubCommand SC1("sc1", "First Subcommand");
+  // Override "-b" and change type in sc1 SubCommand.
+  StackOption SC1_B("b", cl::sub(SC1), cl::init(false));
+  StackSubCommand SC2("sc2", "Second subcommand");
+  // Override "-foo" and change type in sc2 SubCommand.  Note that this does not
+  // affect "-f" alias, which continues to work correctly.
+  StackOption SC2_Foo("foo", cl::sub(SC2));
+
+  const char *args0[] = {"prog", "-b", "args0 bar string", "-f"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args0) / sizeof(char *), args0,
+  StringRef(), &llvm::nulls()));
+  EXPECT_TRUE(Bar == "args0 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_FALSE(SC1_B);
+  EXPECT_TRUE(SC2_Foo.empty());
+
+  Bar = "";
+  Foo = false;
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *args1[] = {"prog", "sc1", "-b", "-bar", "args1 bar string", "-f"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args1) / sizeof(char *), args1,
+  StringRef(), &llvm::nulls()));
+  EXPECT_TRUE(Bar == "args1 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_TRUE(SC1_B);
+  EXPECT_TRUE(SC2_Foo.empty());
+  for (auto *S : cl::getRegisteredSubcommands()) {
+if (*S) {
+  EXPECT_EQ("sc1", S->getName());
+}
+  }
+
+  Bar = "";
+  Foo = false;
+  SC1_B = false;
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *args2[] = {"prog", "sc2", "-b", "args2 bar string",
+ "-f", "-foo", "foo string"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args2) / sizeof(char *), args2,
+  StringRef(), &llvm::nulls()));
+  EXPECT_TRUE(Bar == "args2 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_FALSE(SC1_B);
+  EXPECT_TRUE(SC2_Foo == "foo string");
+  for (auto *S : cl::getRegisteredSubcommands()) {
+if (*S) {
+  EXPECT_EQ("sc2", S->getName());
+}
+  }
+}
+
 TEST(CommandLineTest, ArgumentLimit) {
   std::string args(32 * 4096, 'a');
   EXPECT_FALSE(llvm::sys::commandLineFitsWithinSystemLimits("cl", args.data()));
Index: llvm/tools/llvm-opt-report/OptReport.cpp
===
--- llvm/tools/llvm-opt-report/OptReport.cpp
+++ llvm/tools/llvm-opt-report/OptReport.cpp
@@ -36,8 +36,6 @@
 using namespace llvm;
 using namespace llvm::yaml;
 
-static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
-
 // Mark all our options with this category, everything else (except for -version
 // and -help) will be hidden.
 static cl::OptionCategory
@@ -440,11 +438,6 @@
   "A tool to generate an optimization report from YAML optimization"
   " record files.\n");
 
-  if (Help) {
-cl::PrintHelpMessage();
-return 0;
-  }
-
   LocationInfoTy LocationInfo;
   if (!readLocationInfo(LocationInfo))
 return 1;
Index: llvm/test/Support/check-default-options.txt
===
--- /dev/null
+++ llvm/test/Support/check-default-options.txt
@@ -0,0 +1,18 @@
+# RUN: llvm-objdump -help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
+# RUN: llvm-readobj -help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
+# RUN: llvm-tblgen -help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
+# RUN: llvm-opt-report -help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
+# RUN: llvm-dwarfdump -help-hidden %t | FileCheck --check-prefix=CHECK-DWARF %s
+# RUN: llvm-dwarfdump -h %t | FileCheck --check-prefix=CHECK-DWARF-H %s
+
+
+# CHECK-OBJDUMP: -h  - Alias for --section-headers
+# CHECK-READOBJ: -h  - Alias for --file-headers
+# CHECK-TBLGEN:  -h  - Alias for -help
+# CHECK-OPT-RPT: -h  -

[PATCH] D59605: [clangd] Introduce background-indexer

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

To make it clear, I think the question is not just "which part of functionality 
is missing in BackgroundIndex", it's rather "which part of BackgroundIndex we 
**don't** need".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59605



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

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGDecl.cpp:2576
 void CodeGenModule::EmitOMPRequiresDecl(const OMPRequiresDecl *D) {
-  getOpenMPRuntime().checkArchForUnifiedAddressing(D);
 }

You don't need to pass the reference to CodeGenModule to CGOpenMPRuntime class, 
it handles a reference to the CodeGenMode already.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:726
 
+enum OpenMPOffloadingRequiresDirFlags : int64_t {
+  /// no requires directive present.

You must use `LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();` and 
`LLVM_MARK_AS_BITMASK_ENUM(/* LargestFlag = */)` from 
`clang/Basic/BitmaskEnum.h` to enable bit operations on your new bit-arithmetic 
based enumeric.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:726
 
+enum OpenMPOffloadingRequiresDirFlags : int64_t {
+  /// no requires directive present.

ABataev wrote:
> You must use `LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();` and 
> `LLVM_MARK_AS_BITMASK_ENUM(/* LargestFlag = */)` from 
> `clang/Basic/BitmaskEnum.h` to enable bit operations on your new 
> bit-arithmetic based enumeric.
Add a comment for the whole new type



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:728
+  /// no requires directive present.
+  OMP_REQ_NONE= 0x000,
+  /// reverse_offload clause.

You should not handle all the possible flags for the requires directives, since 
you try to support only target-specific flags.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:2322
+auto *FnTy =
+llvm::FunctionType::get(CGM.Int32Ty, TypeParams, /*isVarArg*/ false);
+RTLFn = CGM.CreateRuntimeFunction(FnTy, "__tgt_register_requires");

Function return type must be `void`



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:3870
 llvm::Function *
+CGOpenMPRuntime::createRequiresDirectiveRegistration() {
+  // If we don't have entries or if we are emitting code for the device, we

Why do you need a new member function? Can you make a static local function?



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:3882
+const auto &FI =
+CGM.getTypes().arrangeBuiltinFunctionDeclaration(C.VoidTy, {});
+llvm::FunctionType *FTy = CGM.getTypes().GetFunctionType(FI);

Use `getTypes().arrangeNullaryFunction()`



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:3887
+CGF.StartFunction(GlobalDecl(), C.VoidTy, RequiresRegFn, FI, {});
+int64_t Flags = OMP_REQ_NONE;
+//TODO: check for other requires clauses.

Use `OpenMPOffloadingRequiresDirFlags` instead of `int64_t`



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7983
 MapFlagsArrayTy &Types) const {
+// If using unified memory, no need to do the mappings.
+if (CGF.CGM.HasRequiresUnifiedSharedMemory)

Seems to me, this must be in another patch, has nothing to do with this patch



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9069
 
+llvm::Function *CGOpenMPRuntime::emitRequiresDirectiveRegFun() {
+  // Create and register the function that handles the requires directives.

Why do you need the second function?



Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:4946
+CodeGenModule &CGM, const OMPRequiresDecl *D) const {
+  CGOpenMPRuntime::checkArchForUnifiedAddressing(CGM, D);
   for (const OMPClause *Clause : D->clauselists()) {

Call this function after the target-specific processing.



Comment at: lib/CodeGen/CodeGenModule.h:294
 
+  bool HasRequiresUnifiedSharedMemory = false;
+

Why public? Must be private. Also, add comments for this new data member.
Also, I don't think this must be in CGM. It must be a member of CGOpenMPRuntime 
class.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60568



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

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In D54881#1462893 , @MyDeveloperDay 
wrote:

> > I agree and would be happy with the change if it would only change the 
> > line-filtered workflow, but this afaict (unless I'm missing something :) 
> > will also affect the workflow where the provided range is 0-length range, 
> > which has an implicit "format stuff around this" request from the user 
> > inside it. I'd be happy with a patch that differentiates these two sides.
>
> This use case needs capturing in a unit test..it isn't obvious


Agreed!


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

https://reviews.llvm.org/D54881



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp:125
+diag(MatchedDecl->getBeginLoc(),
+ "isa_and_nonnull<> is cleaner and more efficient")
+<< FixItHint::CreateReplacement(SourceRange(MatchedDecl->getBeginLoc(),

How about `'isa_and_nonnull<>' is preferred over an explicit test for null 
followed by calling 'isa<>'`?



Comment at: 
clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h:37-39
+///  if (var && isa(var)) {}
+///  // is replaced by:
+///  if (dyn_cast_or_null(var.foo())) {}

I think this comment is now out of date.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1000
+def SYCLDevice : InheritableAttr {
+  let Spellings = [GNU<"sycl_device">];
+  let Subjects = SubjectList<[Function, Var]>;

keryell wrote:
> Fznamznon wrote:
> > aaron.ballman wrote:
> > > Is there a reason to not also introduce a C++11 and C2x style spelling in 
> > > the `clang` namespace? e.g., `[[clang::sycl_device]]`
> > I don't think that it makes sense because these attributes not for public 
> > consumption. These attributes is needed to separate code which is supposed 
> > to be offloaded from regular host code. I think SYCLDevice attribute 
> > actually doesn't need a spelling because it will be added only implicitly 
> > by compiler.
> 
> If we go towards this direction, `[[clang::sycl::device]]` or 
> `[[clang::sycl::kernel]]` look more compatible with the concept of name space.
> While not a public interface, if we have a kind of "standard" outlining in 
> Clang/LLVM, some people might want to use it in some other contexts too.
If these are only being added implicitly by the compiler, then they should not 
be given any `Spelling`. See `AlignMac68k` for an example.



Comment at: clang/include/clang/Basic/Attr.td:1003
+  let LangOpts = [SYCL];
+  let Documentation = [Undocumented];
+}

keryell wrote:
> Fznamznon wrote:
> > aaron.ballman wrote:
> > > No new, undocumented attributes, please.
> > As I said, these attributes are not for public consumption. Should I add 
> > documentation in this case too?
> Yes, documentation and comments are always appreciated.
If the attribute is only ever added implicitly and the user cannot spell it in 
any way, then I think it's reasonable to elide the documentation. It's an 
implementation detail at that point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[libunwind] r358191 - [NFC] Correct outdated links to the Itanium C++ ABI documentation

[Louis Dionne via cfe-commits]

Author: ldionne
Date: Thu Apr 11 09:37:07 2019
New Revision: 358191

URL: http://llvm.org/viewvc/llvm-project?rev=358191&view=rev
Log:
[NFC] Correct outdated links to the Itanium C++ ABI documentation

Those are now hosted on GitHub.

rdar://problem/36557462

Modified:
libunwind/trunk/include/unwind.h
libunwind/trunk/src/UnwindLevel1.c

Modified: libunwind/trunk/include/unwind.h
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/include/unwind.h?rev=358191&r1=358190&r2=358191&view=diff
==
--- libunwind/trunk/include/unwind.h (original)
+++ libunwind/trunk/include/unwind.h Thu Apr 11 09:37:07 2019
@@ -6,7 +6,7 @@
 //
 //
 // C++ ABI Level 1 ABI documented at:
-//   http://mentorembedded.github.io/cxx-abi/abi-eh.html
+//   https://itanium-cxx-abi.github.io/cxx-abi/abi-eh.html
 //
 
//===--===//
 

Modified: libunwind/trunk/src/UnwindLevel1.c
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/UnwindLevel1.c?rev=358191&r1=358190&r2=358191&view=diff
==
--- libunwind/trunk/src/UnwindLevel1.c (original)
+++ libunwind/trunk/src/UnwindLevel1.c Thu Apr 11 09:37:07 2019
@@ -6,7 +6,7 @@
 //
 //
 // Implements C++ ABI Exception Handling Level 1 as documented at:
-//  http://mentorembedded.github.io/cxx-abi/abi-eh.html
+//  https://itanium-cxx-abi.github.io/cxx-abi/abi-eh.html
 // using libunwind
 //
 
//===--===//


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

[PATCH] D60543: [clang] Add matcher for subclasses of Objective-C interfaces 🔍

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:1479
+/// \endcode
+AST_MATCHER_P(ObjCInterfaceDecl, isSubclassOfInterface,
+  internal::Matcher,

stephanemoore wrote:
> I am still uncertain about the naming.
> 
> `isSubclassOf` seemed too generic as that could apply to C++ classes.
> `objcIsSubclassOf` seemed unconventional as a matcher name.
> `isSubclassOfObjCInterface` and `isSubclassOfObjCClass` seemed awkwardly 
> lengthy.
> Creating a new namespace `clang::ast_matchers::objc` seemed unprecedented.
> 
> I am happy to change the name if you think another name would be more 
> appropriate.
Does ObjC use the term "derived" by any chance? We already have 
`isDerivedFrom`, so I'm wondering if we can use that to also match on an 
`ObjCInterfaceDecl`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60543



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

[PATCH] D55409: [clang-tidy] check for using declarations not in an anonymous namespace when there exists one

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/abseil/AnonymousEnclosedAliasesCheck.cpp:41
+// to the vector containing all candidate using declarations.
+if (AnonymousNamespaceDecl) {
+   diag(MatchedUsingDecl->getLocation(),

Naysh wrote:
> aaron.ballman wrote:
> > I don't think this logic works in practice because there's no way to 
> > determine that the anonymous namespace is even a candidate for putting the 
> > using declaration into it. Consider a common scenario where there's an 
> > anonymous namespace declared in a header file (like an STL header outside 
> > of the user's control), and a using declaration inside of an implementation 
> > file. Just because the STL declared an anonymous namespace doesn't mean 
> > that the user could have put their using declaration in it.
> If we altered the check to only apply to anonymous namespaces and using 
> declarations at namespace scope (that is, we only suggest aliases be moved to 
> anonymous namespaces when the unnamed namespace and alias are themselves 
> inside some other namespace), would this issue be resolved? 
If they're inside the same namespace, then I think that could work, but if the 
namespaces are different I don't know that you can be sure the alias can be 
moved into the anonymous namespace.


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

https://reviews.llvm.org/D55409



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

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst:10
+
+This check corresponds to the CERT C++ Coding Standard rule
+`OOP54-CPP. Gracefully handle self-copy assignment

Eugene.Zelenko wrote:
> alexfh wrote:
> > JonasToth wrote:
> > > It is worth an alias into the cert module. Please add this with this 
> > > revision already.
> > Please add a corresponding alias into the cert module.
> See also other aliased checks for documentation examples.
I kind of wonder whether we want it as an alias in the CERT module or just move 
it into the CERT module directly (and maybe provide an alias to bugprone, if we 
think the fp rate is reasonable enough).


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60507



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

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D60151#1462850 , @MyDeveloperDay 
wrote:

> are we supporting "-llvm-*" in existing .clang-tidy files?
>
> If people selectively turn checkers off, won't all of a sudden everyone start 
> getting llvm-project checks and fixes turned back on?
>
> https://github.com/search?q=-llvm-%2A&type=Code
>
> maybe we need to add something like:
>
>   llvm-header-guard (redirects to llvm-project-header-guard) 
> 
>   llvm-header-include-order (redirects to llvm-project-include-order) 
> 
>   llvm-header-namespace-comment(redirects to llvm-project-namespace-comment) 
> 
>   llvm-header-twine-local(redirects to llvm-project-twine-local) 
> 
>  
>


I suppose we could keep the names and directory structure and just change the 
namespace.  That would just be a special case in the scripts.  Haven't looked 
into it yet, but will do so as soon as I can.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



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

[PATCH] D60542: Add support for attributes on @implementations in Objective-C

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington marked an inline comment as done.
erik.pilkington added inline comments.



Comment at: clang/test/FixIt/fixit-pragma-attribute.cpp:19-20
 #pragma clang attribute push 
(__attribute__((annotate("subRuleContradictions"))), apply_to = any(variable, 
variable(is_parameter), function(is_member), variable(is_global)))
-// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:108-[[@LINE-1]]:132}:""
-// CHECK: fix-it:{{.*}}:{[[@LINE-2]]:153-[[@LINE-2]]:172}:""
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:153-[[@LINE-1]]:172}:""
+// CHECK: fix-it:{{.*}}:{[[@LINE-2]]:108-[[@LINE-2]]:132}:""
 

aaron.ballman wrote:
> Any idea why this swapped?
Its because I added a new subject match rule in Attr.td, and the match rules 
enumerators are being stored in a DenseMap in 
Sema::ActOnPragmaAttributeAttribute. I think we should make that a std::map or 
something so this test is less fragile.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60542



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

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D60151#1462976 , @hintonda wrote:

> In D60151#1462850 , @MyDeveloperDay 
> wrote:
>
> > are we supporting "-llvm-*" in existing .clang-tidy files?
> >
> > If people selectively turn checkers off, won't all of a sudden everyone 
> > start getting llvm-project checks and fixes turned back on?
> >
> > https://github.com/search?q=-llvm-%2A&type=Code
> >
> > maybe we need to add something like:
> >
> >   llvm-header-guard (redirects to llvm-project-header-guard) 
> > 
> >   llvm-header-include-order (redirects to llvm-project-include-order) 
> > 
> >   llvm-header-namespace-comment(redirects to 
> > llvm-project-namespace-comment) 
> >   llvm-header-twine-local(redirects to llvm-project-twine-local) 
> > 
> >  
> >
>
>
> I suppose we could keep the names and directory structure and just change the 
> namespace.  That would just be a special case in the scripts.  Haven't looked 
> into it yet, but will do so as soon as I can.


Isn't that matching done on strings? I.e. is there difference between `-llvm-*` 
and `-ll*` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



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

[PATCH] D55411: [clang-tidy] check for flagging using declarations not in the inner most namespace

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/abseil/SafelyScopedCheck.cpp:37
+  diag(MatchedDecl->getLocation(),
+   "using declaration %0 not declared in the innermost namespace.")
+  << MatchedDecl;

You should remove the full stop at the end of the diagnostic.



Comment at: test/clang-tidy/abseil-safely-scoped.cpp:15-16
+} // namespace foo1
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: using declaration 'A' not
+// declared in the innermost namespace. [abseil-safely-scoped]
+

This should be moved closer to where the actual warning will show up.

However, the diagnostic doesn't help me to understand what is wrong with the 
code or how to fix it. I think it might be telling me that the using 
declaration is supposed to be inside of the anonymous namespace, but moving the 
using declaration there will break the declaration of `f()`.


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

https://reviews.llvm.org/D55411



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

[PATCH] D60544: Support objc_nonlazy_class attribute on Objective-C implementations

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a comment.

In D60544#1462869 , @rjmccall wrote:

> Should we have a special `has_feature` test to check that this attribute is 
> allowed on implementations?  We've done that in the past when expanding the 
> set of subjects for an attribute.  But that's not necessary if we haven't 
> made this attribute part of a public Xcode release yet, because then the 
> attribute is just understood to always apply to implementations; I can't 
> remember when exactly this was added and what releases it's made it into.


I think we can sneak by without a `__has_feature` for this, D56555 
 isn't in LLVM 8 nor Xcode 10.2.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60544



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

[PATCH] D60542: Add support for attributes on @implementations in Objective-C

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/FixIt/fixit-pragma-attribute.cpp:19-20
 #pragma clang attribute push 
(__attribute__((annotate("subRuleContradictions"))), apply_to = any(variable, 
variable(is_parameter), function(is_member), variable(is_global)))
-// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:108-[[@LINE-1]]:132}:""
-// CHECK: fix-it:{{.*}}:{[[@LINE-2]]:153-[[@LINE-2]]:172}:""
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:153-[[@LINE-1]]:172}:""
+// CHECK: fix-it:{{.*}}:{[[@LINE-2]]:108-[[@LINE-2]]:132}:""
 

erik.pilkington wrote:
> aaron.ballman wrote:
> > Any idea why this swapped?
> Its because I added a new subject match rule in Attr.td, and the match rules 
> enumerators are being stored in a DenseMap in 
> Sema::ActOnPragmaAttributeAttribute. I think we should make that a std::map 
> or something so this test is less fragile.
Ah, that makes sense, thank you. Yeah, using an ordered container may be a 
better approach to reduce fragility.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60542



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

[PATCH] D60548: Variable auto-init: also auto-init alloca

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

I probably wouldn't do anything about suppressing init on alloca for now, but 
if we did do something I think I'd be in favour of the separate builtin for 
uninitialized alloca. I also considered the alternative of allowing 
`__attribute__((uninitialized))` to appear on a function or block statement, 
but it seemed clearer for the uninitialized-ness to be as close to the alloca 
as possible.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60548



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

[PATCH] D60544: Support objc_nonlazy_class attribute on Objective-C implementations

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall added a comment.

Great, SGTM.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60544



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

r358201 - Support objc_nonlazy_class attribute on Objective-C implementations

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Thu Apr 11 10:55:34 2019
New Revision: 358201

URL: http://llvm.org/viewvc/llvm-project?rev=358201&view=rev
Log:
Support objc_nonlazy_class attribute on Objective-C implementations

Fixes rdar://49523079

Differential revision: https://reviews.llvm.org/D60544

Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/lib/CodeGen/CGObjCMac.cpp
cfe/trunk/test/CodeGenObjC/non-lazy-classes.m
cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
cfe/trunk/test/SemaObjC/attr-objc-non-lazy.m

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=358201&r1=358200&r2=358201&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Thu Apr 11 10:55:34 2019
@@ -1767,7 +1767,7 @@ def ObjCRootClass : InheritableAttr {
 
 def ObjCNonLazyClass : Attr {
   let Spellings = [Clang<"objc_nonlazy_class">];
-  let Subjects = SubjectList<[ObjCInterface], ErrorDiag>;
+  let Subjects = SubjectList<[ObjCInterface, ObjCImpl], ErrorDiag>;
   let LangOpts = [ObjC];
   let Documentation = [ObjCNonLazyClassDocs];
 }

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=358201&r1=358200&r2=358201&view=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Thu Apr 11 10:55:34 2019
@@ -3711,14 +3711,14 @@ ensure that this class cannot be subclas
 def ObjCNonLazyClassDocs : Documentation {
   let Category = DocCatDecl;
   let Content = [{
-This attribute can be added to an Objective-C ``@interface`` declaration to
-add the class to the list of non-lazily initialized classes. A non-lazy class
-will be initialized eagerly when the Objective-C runtime is loaded.  This is
-required for certain system classes which have instances allocated in
-non-standard ways, such as the classes for blocks and constant strings.  Adding
-this attribute is essentially equivalent to providing a trivial `+load` method 
-but avoids the (fairly small) load-time overheads associated with defining and
-calling such a method.
+This attribute can be added to an Objective-C ``@interface`` or
+``@implementation`` declaration to add the class to the list of non-lazily
+initialized classes. A non-lazy class will be initialized eagerly when the
+Objective-C runtime is loaded. This is required for certain system classes 
which
+have instances allocated in non-standard ways, such as the classes for blocks
+and constant strings. Adding this attribute is essentially equivalent to
+providing a trivial `+load` method but avoids the (fairly small) load-time
+overheads associated with defining and calling such a method.
   }];
 }
 

Modified: cfe/trunk/lib/CodeGen/CGObjCMac.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjCMac.cpp?rev=358201&r1=358200&r2=358201&view=diff
==
--- cfe/trunk/lib/CodeGen/CGObjCMac.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjCMac.cpp Thu Apr 11 10:55:34 2019
@@ -6262,7 +6262,8 @@ CGObjCNonFragileABIMac::BuildClassObject
 bool CGObjCNonFragileABIMac::ImplementationIsNonLazy(
 const ObjCImplDecl *OD) const {
   return OD->getClassMethod(GetNullarySelector("load")) != nullptr ||
- OD->getClassInterface()->hasAttr();
+ OD->getClassInterface()->hasAttr() ||
+ OD->hasAttr();
 }
 
 void CGObjCNonFragileABIMac::GetClassSizeInfo(const ObjCImplementationDecl 
*OID,

Modified: cfe/trunk/test/CodeGenObjC/non-lazy-classes.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/non-lazy-classes.m?rev=358201&r1=358200&r2=358201&view=diff
==
--- cfe/trunk/test/CodeGenObjC/non-lazy-classes.m (original)
+++ cfe/trunk/test/CodeGenObjC/non-lazy-classes.m Thu Apr 11 10:55:34 2019
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -Wno-objc-root-class 
-emit-llvm -o - %s | \
-// RUN: FileCheck %s
-// CHECK: @"OBJC_LABEL_NONLAZY_CLASS_$" = private global [2 x 
{{.*}}]{{.*}}@"OBJC_CLASS_$_A"{{.*}},{{.*}}@"OBJC_CLASS_$_D"{{.*}} section 
"__DATA,__objc_nlclslist,regular,no_dead_strip", align 8
-// CHECK: @"OBJC_LABEL_NONLAZY_CATEGORY_$" = private global [1 x {{.*}}] 
{{.*}}@"\01l_OBJC_$_CATEGORY_A_$_Cat"{{.*}}, section 
"__DATA,__objc_nlcatlist,regular,no_dead_strip", align 8
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -Wno-objc-root-class 
-emit-llvm -o - %s | FileCheck %s
+
+// CHECK: @"OBJC_LABEL_NONLAZY_CLASS_$" = private global [3 x 
{{.*}}]{{.*}}@"OBJC_CLASS_$_A"{{.*}},{{.*}}@"OBJC_CLASS_$_D"{{.*}},{{.*}}"OBJC_CLASS_$_E"{{

r358200 - Add support for attributes on @implementations in Objective-C

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Thu Apr 11 10:55:30 2019
New Revision: 358200

URL: http://llvm.org/viewvc/llvm-project?rev=358200&view=rev
Log:
Add support for attributes on @implementations in Objective-C

We want to make objc_nonlazy_class apply to implementations, but ran into this.
There doesn't seem to be any reason that this isn't supported.

Differential revision: https://reviews.llvm.org/D60542

Added:
cfe/trunk/test/Parser/objc-implementation-attrs.m
Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Parse/ParseObjc.cpp
cfe/trunk/lib/Parse/Parser.cpp
cfe/trunk/lib/Sema/SemaDeclObjC.cpp
cfe/trunk/test/FixIt/fixit-pragma-attribute.cpp
cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
cfe/trunk/test/Parser/attributes.mm
cfe/trunk/test/Parser/placeholder-recovery.m
cfe/trunk/test/Sema/pragma-attribute-strict-subjects.c
cfe/trunk/test/SemaObjC/attr-objc-non-lazy.m
cfe/trunk/test/SemaObjC/objc-asm-attribute-neg-test.m

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=358200&r1=358199&r2=358200&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Thu Apr 11 10:55:30 2019
@@ -419,6 +419,10 @@ def SubjectMatcherForObjCCategory : Attr
[ObjCCategory]> {
   let LangOpts = [ObjC];
 }
+def SubjectMatcherForObjCImplementation :
+AttrSubjectMatcherRule<"objc_implementation", [ObjCImpl]> {
+  let LangOpts = [ObjC];
+}
 def SubjectMatcherForObjCMethod : AttrSubjectMatcherRule<"objc_method",
  [ObjCMethod], [
   AttrSubjectMatcherSubRule<"is_instance", [ObjCInstanceMethod]>

Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=358200&r1=358199&r2=358200&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Thu Apr 11 10:55:30 
2019
@@ -433,7 +433,7 @@ def err_objc_expected_property_attr : Er
 def err_objc_properties_require_objc2 : Error<
   "properties are an Objective-C 2 feature">;
 def err_objc_unexpected_attr : Error<
-  "prefix attribute must be followed by an interface or protocol">;
+  "prefix attribute must be followed by an interface, protocol, or 
implementation">;
 def err_objc_postfix_attribute : Error <
   "postfix attributes are not allowed on Objective-C directives">;
 def err_objc_postfix_attribute_hint : Error <

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=358200&r1=358199&r2=358200&view=diff
==
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Thu Apr 11 10:55:30 2019
@@ -1573,7 +1573,8 @@ private:
   ObjCImplParsingDataRAII *CurParsedObjCImpl;
   void StashAwayMethodOrFunctionBodyTokens(Decl *MDecl);
 
-  DeclGroupPtrTy ParseObjCAtImplementationDeclaration(SourceLocation AtLoc);
+  DeclGroupPtrTy ParseObjCAtImplementationDeclaration(SourceLocation AtLoc,
+  ParsedAttributes &Attrs);
   DeclGroupPtrTy ParseObjCAtEndDeclaration(SourceRange atEnd);
   Decl *ParseObjCAtAliasDeclaration(SourceLocation atLoc);
   Decl *ParseObjCPropertySynthesize(SourceLocation atLoc);

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=358200&r1=358199&r2=358200&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Apr 11 10:55:30 2019
@@ -8105,17 +8105,19 @@ public:
   const SourceLocation *ProtoLocs, SourceLocation EndProtoLoc,
   const ParsedAttributesView &AttrList);
 
-  Decl *ActOnStartClassImplementation(
-SourceLocation AtClassImplLoc,
-IdentifierInfo *ClassName, SourceLocation ClassLoc,
-IdentifierInfo *SuperClassname,
-SourceLocation SuperClassLoc);
+  Decl *ActOnStartClassImplementation(SourceLocation AtClassImplLoc,
+  IdentifierInfo *ClassName,
+  SourceLocation ClassLoc,
+  IdentifierInfo *SuperClassname,
+  SourceL

[PATCH] D60542: Add support for attributes on @implementations in Objective-C

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC358200: Add support for attributes on @implementations in 
Objective-C (authored by epilk, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60542?vs=194594&id=194716#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D60542

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseObjc.cpp
  lib/Parse/Parser.cpp
  lib/Sema/SemaDeclObjC.cpp
  test/FixIt/fixit-pragma-attribute.cpp
  test/Misc/pragma-attribute-supported-attributes-list.test
  test/Parser/attributes.mm
  test/Parser/objc-implementation-attrs.m
  test/Parser/placeholder-recovery.m
  test/Sema/pragma-attribute-strict-subjects.c
  test/SemaObjC/attr-objc-non-lazy.m
  test/SemaObjC/objc-asm-attribute-neg-test.m

Index: lib/Sema/SemaDeclObjC.cpp
===
--- lib/Sema/SemaDeclObjC.cpp
+++ lib/Sema/SemaDeclObjC.cpp
@@ -1892,7 +1892,8 @@
 Decl *Sema::ActOnStartCategoryImplementation(
   SourceLocation AtCatImplLoc,
   IdentifierInfo *ClassName, SourceLocation ClassLoc,
-  IdentifierInfo *CatName, SourceLocation CatLoc) {
+  IdentifierInfo *CatName, SourceLocation CatLoc,
+  const ParsedAttributesView &Attrs) {
   ObjCInterfaceDecl *IDecl = getObjCInterfaceDecl(ClassName, ClassLoc, true);
   ObjCCategoryDecl *CatIDecl = nullptr;
   if (IDecl && IDecl->hasDefinition()) {
@@ -1920,6 +1921,9 @@
 CDecl->setInvalidDecl();
   }
 
+  ProcessDeclAttributeList(TUScope, CDecl, Attrs);
+  AddPragmaAttributes(TUScope, CDecl);
+
   // FIXME: PushOnScopeChains?
   CurContext->addDecl(CDecl);
 
@@ -1955,7 +1959,8 @@
   SourceLocation AtClassImplLoc,
   IdentifierInfo *ClassName, SourceLocation ClassLoc,
   IdentifierInfo *SuperClassname,
-  SourceLocation SuperClassLoc) {
+  SourceLocation SuperClassLoc,
+  const ParsedAttributesView &Attrs) {
   ObjCInterfaceDecl *IDecl = nullptr;
   // Check for another declaration kind with the same name.
   NamedDecl *PrevDecl
@@ -2049,6 +2054,9 @@
 ObjCImplementationDecl::Create(Context, CurContext, IDecl, SDecl,
ClassLoc, AtClassImplLoc, SuperClassLoc);
 
+  ProcessDeclAttributeList(TUScope, IMPDecl, Attrs);
+  AddPragmaAttributes(TUScope, IMPDecl);
+
   if (CheckObjCDeclScope(IMPDecl))
 return ActOnObjCContainerStartDefinition(IMPDecl);
 
Index: lib/Parse/ParseObjc.cpp
===
--- lib/Parse/ParseObjc.cpp
+++ lib/Parse/ParseObjc.cpp
@@ -64,7 +64,7 @@
   case tok::objc_protocol:
 return ParseObjCAtProtocolDeclaration(AtLoc, Attrs);
   case tok::objc_implementation:
-return ParseObjCAtImplementationDeclaration(AtLoc);
+return ParseObjCAtImplementationDeclaration(AtLoc, Attrs);
   case tok::objc_end:
 return ParseObjCAtEndDeclaration(AtLoc);
   case tok::objc_compatibility_alias:
@@ -2097,7 +2097,8 @@
 ///   objc-category-implementation-prologue:
 /// @implementation identifier ( identifier )
 Parser::DeclGroupPtrTy
-Parser::ParseObjCAtImplementationDeclaration(SourceLocation AtLoc) {
+Parser::ParseObjCAtImplementationDeclaration(SourceLocation AtLoc,
+ ParsedAttributes &Attrs) {
   assert(Tok.isObjCAtKeyword(tok::objc_implementation) &&
  "ParseObjCAtImplementationDeclaration(): Expected @implementation");
   CheckNestedObjCContexts(AtLoc);
@@ -2174,8 +2175,7 @@
 /*consumeLastToken=*/true);
 }
 ObjCImpDecl = Actions.ActOnStartCategoryImplementation(
-AtLoc, nameId, nameLoc, categoryId,
-categoryLoc);
+AtLoc, nameId, nameLoc, categoryId, categoryLoc, Attrs);
 
   } else {
 // We have a class implementation
@@ -2189,8 +2189,7 @@
   superClassLoc = ConsumeToken(); // Consume super class name
 }
 ObjCImpDecl = Actions.ActOnStartClassImplementation(
-AtLoc, nameId, nameLoc,
-superClassId, superClassLoc);
+AtLoc, nameId, nameLoc, superClassId, superClassLoc, Attrs);
 
 if (Tok.is(tok::l_brace)) // we have ivars
   ParseObjCClassInstanceVariables(ObjCImpDecl, tok::objc_private, AtLoc);
Index: lib/Parse/Parser.cpp
===
--- lib/Parse/Parser.cpp
+++ lib/Parse/Parser.cpp
@@ -980,9 +980,10 @@
   if (getLangOpts().ObjC && Tok.is(tok::at)) {
 SourceLocation AtLoc = ConsumeToken(); // the "@"
 if (!Tok.isObjCAtKeyword

[PATCH] D60544: Support objc_nonlazy_class attribute on Objective-C implementations

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL358201: Support objc_nonlazy_class attribute on Objective-C 
implementations (authored by epilk, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60544?vs=194596&id=194717#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60544

Files:
  cfe/trunk/include/clang/Basic/Attr.td
  cfe/trunk/include/clang/Basic/AttrDocs.td
  cfe/trunk/lib/CodeGen/CGObjCMac.cpp
  cfe/trunk/test/CodeGenObjC/non-lazy-classes.m
  cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
  cfe/trunk/test/SemaObjC/attr-objc-non-lazy.m

Index: cfe/trunk/lib/CodeGen/CGObjCMac.cpp
===
--- cfe/trunk/lib/CodeGen/CGObjCMac.cpp
+++ cfe/trunk/lib/CodeGen/CGObjCMac.cpp
@@ -6262,7 +6262,8 @@
 bool CGObjCNonFragileABIMac::ImplementationIsNonLazy(
 const ObjCImplDecl *OD) const {
   return OD->getClassMethod(GetNullarySelector("load")) != nullptr ||
- OD->getClassInterface()->hasAttr();
+ OD->getClassInterface()->hasAttr() ||
+ OD->hasAttr();
 }
 
 void CGObjCNonFragileABIMac::GetClassSizeInfo(const ObjCImplementationDecl *OID,
Index: cfe/trunk/include/clang/Basic/AttrDocs.td
===
--- cfe/trunk/include/clang/Basic/AttrDocs.td
+++ cfe/trunk/include/clang/Basic/AttrDocs.td
@@ -3711,14 +3711,14 @@
 def ObjCNonLazyClassDocs : Documentation {
   let Category = DocCatDecl;
   let Content = [{
-This attribute can be added to an Objective-C ``@interface`` declaration to
-add the class to the list of non-lazily initialized classes. A non-lazy class
-will be initialized eagerly when the Objective-C runtime is loaded.  This is
-required for certain system classes which have instances allocated in
-non-standard ways, such as the classes for blocks and constant strings.  Adding
-this attribute is essentially equivalent to providing a trivial `+load` method 
-but avoids the (fairly small) load-time overheads associated with defining and
-calling such a method.
+This attribute can be added to an Objective-C ``@interface`` or
+``@implementation`` declaration to add the class to the list of non-lazily
+initialized classes. A non-lazy class will be initialized eagerly when the
+Objective-C runtime is loaded. This is required for certain system classes which
+have instances allocated in non-standard ways, such as the classes for blocks
+and constant strings. Adding this attribute is essentially equivalent to
+providing a trivial `+load` method but avoids the (fairly small) load-time
+overheads associated with defining and calling such a method.
   }];
 }
 
Index: cfe/trunk/include/clang/Basic/Attr.td
===
--- cfe/trunk/include/clang/Basic/Attr.td
+++ cfe/trunk/include/clang/Basic/Attr.td
@@ -1767,7 +1767,7 @@
 
 def ObjCNonLazyClass : Attr {
   let Spellings = [Clang<"objc_nonlazy_class">];
-  let Subjects = SubjectList<[ObjCInterface], ErrorDiag>;
+  let Subjects = SubjectList<[ObjCInterface, ObjCImpl], ErrorDiag>;
   let LangOpts = [ObjC];
   let Documentation = [ObjCNonLazyClassDocs];
 }
Index: cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
+++ cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -102,7 +102,7 @@
 // CHECK-NEXT: ObjCExplicitProtocolImpl (SubjectMatchRule_objc_protocol)
 // CHECK-NEXT: ObjCExternallyRetained (SubjectMatchRule_variable_not_is_parameter, SubjectMatchRule_function, SubjectMatchRule_block, SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCMethodFamily (SubjectMatchRule_objc_method)
-// CHECK-NEXT: ObjCNonLazyClass (SubjectMatchRule_objc_interface)
+// CHECK-NEXT: ObjCNonLazyClass (SubjectMatchRule_objc_interface, SubjectMatchRule_objc_implementation)
 // CHECK-NEXT: ObjCPreciseLifetime (SubjectMatchRule_variable)
 // CHECK-NEXT: ObjCRequiresPropertyDefs (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCRequiresSuper (SubjectMatchRule_objc_method)
Index: cfe/trunk/test/SemaObjC/attr-objc-non-lazy.m
===
--- cfe/trunk/test/SemaObjC/attr-objc-non-lazy.m
+++ cfe/trunk/test/SemaObjC/attr-objc-non-lazy.m
@@ -29,7 +29,11 @@
 
 @interface E
 @end
-// expected-error@+1 {{'objc_nonlazy_class' attribute only applies to Objective-C interfaces}}
+
 __attribute__((objc_nonlazy_class))
 @implementation E
 @end
+
+__attribute__((objc_nonlazy_class))
+@implementation E (MyCat)
+@end
Index: cfe/trunk/test/CodeGenObjC/non-lazy-classes.m
===
--- cfe/trunk/test/CodeGenObjC/no

Re: [PATCH] D59605: [clangd] Introduce background-indexer

[Sam McCall via cfe-commits]

Thanks - I think there's quite a lot of background missing here: what/who
is this for, why a separate binary from clangd-indexer (or and how do they
relate, how much of the BackgroundIndex code structure makes sense with
multiple users.

I assume there's been some offline discussion about this, but i missed it
:-)

On Thu, Apr 11, 2019, 18:16 Ilya Biryukov via Phabricator <
revi...@reviews.llvm.org> wrote:

> ilya-biryukov added a comment.
>
> To make it clear, I think the question is not just "which part of
> functionality is missing in BackgroundIndex", it's rather "which part of
> BackgroundIndex we **don't** need".
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D59605/new/
>
> https://reviews.llvm.org/D59605
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60570: [Sema] Add more tests for the behavior of argument-dependent name lookup

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno created this revision.
riccibruno added reviewers: rjmccall, Quuxplusone.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.

The goal here is to exercise each rule in [basic.lookup.argdep] at least once. 
These new tests expose what I believe are 2 issues:

1. CWG 1691 needs to be implemented (`p2:  [...] Its associated namespaces are 
the innermost enclosing namespaces of its associated classes [...]`) The 
corresponding tests are `adl_class_type::X2` and `adl_class_type::X5`.

2. The end of paragraph 2 (`[...] Additionally, if the aforementioned set of 
overloaded functions is named with a template-id, its associated classes and 
namespaces also include those of its type template-arguments and its template 
template-arguments.`) is not implemented. Closely related, the restriction on 
non-dependent parameter types in this same paragraph needs to be removed. The 
corresponding tests are in `adl_overload_set`. (both issues are from CWG 997).


Repository:
  rC Clang

https://reviews.llvm.org/D60570

Files:
  
test/CXX/basic/basic.lookup/basic.lookup.argdep/p2-associated-namespaces-classes.cpp
  test/CXX/basic/basic.lookup/basic.lookup.argdep/p3.cpp
  test/CXX/basic/basic.lookup/basic.lookup.argdep/p4.cpp

Index: test/CXX/basic/basic.lookup/basic.lookup.argdep/p4.cpp
===
--- test/CXX/basic/basic.lookup/basic.lookup.argdep/p4.cpp
+++ test/CXX/basic/basic.lookup/basic.lookup.argdep/p4.cpp
@@ -67,3 +67,96 @@
 foo(a, 10); // expected-error {{no matching function for call to 'foo'}}
   }
 }
+
+
+// Check the rules described in p4:
+//  When considering an associated namespace, the lookup is the same as the lookup
+//  performed when the associated namespace is used as a qualifier (6.4.3.2) except that:
+
+//  - Any using-directives in the associated namespace are ignored.
+namespace test_using_directives {
+  namespace M { struct S; }
+  namespace N {
+void f(M::S); // expected-note {{declared here}}
+  }
+  namespace M {
+using namespace N;
+struct S {};
+  }
+  void test() {
+M::S s;
+f(s); // expected-error {{use of undeclared}}
+M::f(s); // ok
+  }
+}
+
+//  - Any namespace-scope friend functions or friend function templates declared in
+//associated classes are visible within their respective namespaces even if
+//they are not visible during an ordinary lookup
+// (Note: For the friend declaration to be visible, the corresponding class must be
+//  included in the set of associated classes. Merely including the namespace in
+//  the set of associated namespaces is not enough.)
+namespace test_friend1 {
+  namespace N {
+struct S;
+struct T {
+  friend void f(S); // #1
+};
+struct S { S(); S(T); };
+  }
+
+  void test() {
+N::S s;
+N::T t;
+f(s); // expected-error {{use of undeclared}}
+f(t); // ok, #1
+  }
+}
+
+// credit: Arthur O’Dwyer
+namespace test_friend2 {
+  struct A {
+struct B {
+struct C {};
+};
+friend void foo(...); // #1
+  };
+
+  struct D {
+friend void foo(...); // #2
+  };
+  template struct E {
+struct F {};
+  };
+
+  template struct G {};
+  template struct H {};
+  template struct I {};
+  struct J { friend void foo(...) {} }; // #3
+
+  void test() {
+A::B::C c;
+foo(c); // #1 is not visible since A is not an associated class
+// expected-error@-1 {{use of undeclared}}
+E::F f;
+foo(f); // #2 is not visible since D is not an associated class
+// expected-error@-1 {{use of undeclared}}
+G > > j;
+foo(j);  // ok, #3.
+  }
+}
+
+//  - All names except those of (possibly overloaded) functions and
+//function templates are ignored.
+namespace test_other_names {
+  namespace N {
+struct S {};
+struct Callable { void operator()(S); };
+static struct Callable Callable;
+  }
+
+  void test() {
+N::S s;
+Callable(s); // expected-error {{use of undeclared}}
+  }
+}
Index: test/CXX/basic/basic.lookup/basic.lookup.argdep/p3.cpp
===
--- test/CXX/basic/basic.lookup/basic.lookup.argdep/p3.cpp
+++ test/CXX/basic/basic.lookup/basic.lookup.argdep/p3.cpp
@@ -18,3 +18,48 @@
 }
   };
 }
+
+// If X contains [...] then Y is empty.
+// - a declaration of a class member
+namespace test_adl_suppression_by_class_member {
+  namespace N {
+struct T {};
+void f(T); // expected-note {{declared here}}
+  }
+  struct S {
+void f();
+void test() {
+  N::T t;
+  f(t); // expected-error {{too many arguments}}
+}
+  };
+}
+
+// - a block-scope function declaration that is not a using-declaration
+namespace test_adl_suppression_by_block_scope {
+  namespace N {
+struct S {};
+void f(S);
+  }
+  namespace M { void f(int); } // expected-note {{candidate}}
+  void test() {
+N::S s;
+using M::f;
+f(s); // ok
+extern void f(char); // expected

[PATCH] D60572: creduce-clang-crash: Use `!` instead of `not`

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: akhuang.

The run script runs in bash, so no need to depend on llvm's `not` binary.


https://reviews.llvm.org/D60572

Files:
  clang/utils/creduce-clang-crash.py


Index: clang/utils/creduce-clang-crash.py
===
--- clang/utils/creduce-clang-crash.py
+++ clang/utils/creduce-clang-crash.py
@@ -23,7 +23,6 @@
 verbose = False
 creduce_cmd = None
 clang_cmd = None
-not_cmd = None
 
 def verbose_print(*args, **kwargs):
   if verbose:
@@ -178,8 +177,8 @@
 
 crash_flag = "--crash" if self.is_crash else ""
 
-output = "#!/bin/bash\n%s %s %s >& t.log || exit 1\n" % \
-(pipes.quote(not_cmd), crash_flag, quote_cmd(self.get_crash_cmd()))
+output = "#!/bin/bash\n! %s %s >& t.log || exit 1\n" % \
+(crash_flag, quote_cmd(self.get_crash_cmd()))
 
 for msg in self.expected_output:
   output += 'grep %s t.log || exit 1\n' % pipes.quote(msg)
@@ -360,7 +359,6 @@
   global verbose
   global creduce_cmd
   global clang_cmd
-  global not_cmd
 
   parser = ArgumentParser(description=__doc__,
   formatter_class=RawTextHelpFormatter)
@@ -370,9 +368,6 @@
   help="Name of the file to be reduced.")
   parser.add_argument('--llvm-bin', dest='llvm_bin', type=str,
   help="Path to the LLVM bin directory.")
-  parser.add_argument('--llvm-not', dest='llvm_not', type=str,
-  help="The path to the `not` executable. "
-  "By default uses the llvm-bin directory.")
   parser.add_argument('--clang', dest='clang', type=str,
   help="The path to the `clang` executable. "
   "By default uses the llvm-bin directory.")
@@ -386,7 +381,6 @@
   llvm_bin = os.path.abspath(args.llvm_bin) if args.llvm_bin else None
   creduce_cmd = check_cmd('creduce', None, args.creduce)
   clang_cmd = check_cmd('clang', llvm_bin, args.clang)
-  not_cmd = check_cmd('not', llvm_bin, args.llvm_not)
 
   crash_script = check_file(args.crash_script[0])
   file_to_reduce = check_file(args.file_to_reduce[0])


Index: clang/utils/creduce-clang-crash.py
===
--- clang/utils/creduce-clang-crash.py
+++ clang/utils/creduce-clang-crash.py
@@ -23,7 +23,6 @@
 verbose = False
 creduce_cmd = None
 clang_cmd = None
-not_cmd = None
 
 def verbose_print(*args, **kwargs):
   if verbose:
@@ -178,8 +177,8 @@
 
 crash_flag = "--crash" if self.is_crash else ""
 
-output = "#!/bin/bash\n%s %s %s >& t.log || exit 1\n" % \
-(pipes.quote(not_cmd), crash_flag, quote_cmd(self.get_crash_cmd()))
+output = "#!/bin/bash\n! %s %s >& t.log || exit 1\n" % \
+(crash_flag, quote_cmd(self.get_crash_cmd()))
 
 for msg in self.expected_output:
   output += 'grep %s t.log || exit 1\n' % pipes.quote(msg)
@@ -360,7 +359,6 @@
   global verbose
   global creduce_cmd
   global clang_cmd
-  global not_cmd
 
   parser = ArgumentParser(description=__doc__,
   formatter_class=RawTextHelpFormatter)
@@ -370,9 +368,6 @@
   help="Name of the file to be reduced.")
   parser.add_argument('--llvm-bin', dest='llvm_bin', type=str,
   help="Path to the LLVM bin directory.")
-  parser.add_argument('--llvm-not', dest='llvm_not', type=str,
-  help="The path to the `not` executable. "
-  "By default uses the llvm-bin directory.")
   parser.add_argument('--clang', dest='clang', type=str,
   help="The path to the `clang` executable. "
   "By default uses the llvm-bin directory.")
@@ -386,7 +381,6 @@
   llvm_bin = os.path.abspath(args.llvm_bin) if args.llvm_bin else None
   creduce_cmd = check_cmd('creduce', None, args.creduce)
   clang_cmd = check_cmd('clang', llvm_bin, args.clang)
-  not_cmd = check_cmd('not', llvm_bin, args.llvm_not)
 
   crash_script = check_file(args.crash_script[0])
   file_to_reduce = check_file(args.file_to_reduce[0])
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60572: creduce-clang-crash: Use `!` instead of `not`

[Nico Weber via Phabricator via cfe-commits]

thakis abandoned this revision.
thakis added a comment.

This seems to break the script, nevermind!


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

https://reviews.llvm.org/D60572



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

[PATCH] D60573: [Sema] ADL: Associated namespaces for class types and enumeration types (CWG 1691)

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno created this revision.
riccibruno added reviewers: rjmccall, Quuxplusone.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.
riccibruno added a parent revision: D60570: [Sema] Add more tests for the 
behavior of argument-dependent name lookup.

CWG 1691 changed the definition of the namespaces associated with a class type 
or enumeration type.

For a class type, the associated namespaces are the innermost enclosing 
namespaces of the associated classes.
For an enumeration type, the associated namespace is the innermost enclosing 
namespace of its declaration.

This also fixes CWG 1690 and CWG 1692.

I have applied the fix to all language versions, even though I think that the 
DR only applies to C++14. The reason for this is that it is possible to return 
a local type from a lambda in C++11, and I don't think that this will affect 
anyone (although  this can totally be a failure of my imagination...)


Repository:
  rC Clang

https://reviews.llvm.org/D60573

Files:
  lib/Sema/SemaLookup.cpp
  
test/CXX/basic/basic.lookup/basic.lookup.argdep/p2-associated-namespaces-classes.cpp
  test/CXX/drs/dr16xx.cpp

Index: test/CXX/drs/dr16xx.cpp
===
--- test/CXX/drs/dr16xx.cpp
+++ test/CXX/drs/dr16xx.cpp
@@ -284,6 +284,54 @@
 #endif
 }
 
+namespace dr1690 { // dr1690: 9
+  // See also the various tests in "CXX/basic/basic.lookup/basic.lookup.argdep".
+#if __cplusplus >= 201103L
+  namespace N {
+static auto lambda = []() { struct S {} s; return s; };
+void f(decltype(lambda()));
+  }
+
+  void test() {
+auto s = N::lambda();
+f(s); // ok
+  }
+#endif
+}
+
+namespace dr1691 { // dr1691: 9
+#if __cplusplus >= 201103L
+  namespace N {
+namespace M {
+  enum E : int;
+  void f(E);
+}
+enum M::E : int {};
+void g(M::E); // expected-note {{declared here}}
+  }
+  void test() {
+N::M::E e;
+f(e); // ok
+g(e); // expected-error {{use of undeclared}}
+  }
+#endif
+}
+
+namespace dr1692 { // dr1692: 9
+  namespace N {
+struct A {
+  struct B {
+struct C {};
+  };
+};
+void f(A::B::C);
+  }
+  void test() {
+N::A::B::C c;
+f(c); // ok
+  }
+}
+
 namespace dr1696 { // dr1696: 7
   namespace std_examples {
 #if __cplusplus >= 201402L
Index: test/CXX/basic/basic.lookup/basic.lookup.argdep/p2-associated-namespaces-classes.cpp
===
--- test/CXX/basic/basic.lookup/basic.lookup.argdep/p2-associated-namespaces-classes.cpp
+++ test/CXX/basic/basic.lookup/basic.lookup.argdep/p2-associated-namespaces-classes.cpp
@@ -38,12 +38,11 @@
   return s;
 }
 using S = decltype(foo());
-void f(S); // expected-note {{'X2::f' declared here}}
+void f(S); // #1
   }
   void test2() {
-f(X2::S{}); // FIXME: This is well-formed; X2 is the innermost enclosing namespace
-// of the local struct S.
-// expected-error@-2 {{use of undeclared identifier 'f'}}
+f(X2::S{}); // This is well-formed; X2 is the innermost enclosing namespace
+// of the local struct S. Calls #1.
   }
 
   // associated class: the parent class
@@ -83,7 +82,7 @@
 
 void test5() {
   auto lambda = N::get_lambda();
-  f(lambda); // FIXME: This is well-formed. expected-error {{use of undeclared}}
+  f(lambda); // ok
 }
   }
 } // namespace adl_class_type
@@ -172,6 +171,12 @@
   enum F : int;
   friend void g(F);
 };
+auto foo() {
+  enum G {} g;
+  return g;
+}
+using G = decltype(foo());
+void h(G);
   }
 
   void test() {
@@ -179,6 +184,9 @@
 f(e); // ok
 N::S::F f;
 g(f); // ok
+N::G g;
+h(g); // ok
+
   }
 }
 
Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -2458,30 +2458,39 @@
 static void
 addAssociatedClassesAndNamespaces(AssociatedLookup &Result, QualType T);
 
+// Given the declaration context \param Ctx of a class, class template or
+// enumeration, add the associated namespaces to \param Namespaces as described
+// in [basic.lookup.argdep]p2.
 static void CollectEnclosingNamespace(Sema::AssociatedNamespaceSet &Namespaces,
   DeclContext *Ctx) {
-  // Add the associated namespace for this class.
-
-  // We don't use DeclContext::getEnclosingNamespaceContext() as this may
-  // be a locally scoped record.
+  // The exact wording has been changed in C++14 as a result of
+  // CWG 1691 (see also CWG 1690 and CWG 1692). We apply it unconditionally
+  // to all language versions since it is possible to return a local type
+  // from a lambda in C++11.
+  //
+  // C++14 [basic.lookup.argdep]p2:
+  //   If T is a class type [...]. Its associated namespaces are the innermost
+  //   enclosing namespaces of its associated classes. [...]
+  //
+  //   If 

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 194729.
hintonda added a comment.

Enhance Option::reset to reset the default value and remove the option
if it's a cl::DefaultOption.

Also, make sure removeOption only removes an option if both the name
and the Option matches, not just the name -- different Options with
the same name in different SubCommands have always been possible, but
are more common with DefaultOptions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746

Files:
  clang/lib/Tooling/CommonOptionsParser.cpp
  llvm/docs/CommandLine.rst
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/test/Support/check-default-options.txt
  llvm/tools/llvm-opt-report/OptReport.cpp
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -620,6 +620,67 @@
   }
 }
 
+TEST(CommandLineTest, DefaultOptions) {
+  cl::ResetCommandLineParser();
+
+  StackOption Bar("bar", cl::sub(*cl::AllSubCommands),
+   cl::DefaultOption);
+  StackOption Bar_Alias(
+  "b", cl::desc("Alias for -bar"), cl::aliasopt(Bar), cl::DefaultOption);
+
+  StackOption Foo("foo", cl::init(false), cl::sub(*cl::AllSubCommands),
+cl::DefaultOption);
+  StackOption Foo_Alias("f", cl::desc("Alias for -foo"),
+ cl::aliasopt(Foo), cl::DefaultOption);
+
+  StackSubCommand SC1("sc1", "First Subcommand");
+  // Override "-b" and change type in sc1 SubCommand.
+  StackOption SC1_B("b", cl::sub(SC1), cl::init(false));
+  StackSubCommand SC2("sc2", "Second subcommand");
+  // Override "-foo" and change type in sc2 SubCommand.  Note that this does not
+  // affect "-f" alias, which continues to work correctly.
+  StackOption SC2_Foo("foo", cl::sub(SC2));
+
+  const char *args0[] = {"prog", "-b", "args0 bar string", "-f"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args0) / sizeof(char *), args0,
+  StringRef(), &llvm::nulls()));
+  EXPECT_TRUE(Bar == "args0 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_FALSE(SC1_B);
+  EXPECT_TRUE(SC2_Foo.empty());
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *args1[] = {"prog", "sc1", "-b", "-bar", "args1 bar string", "-f"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args1) / sizeof(char *), args1,
+  StringRef(), &llvm::nulls()));
+  EXPECT_TRUE(Bar == "args1 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_TRUE(SC1_B);
+  EXPECT_TRUE(SC2_Foo.empty());
+  for (auto *S : cl::getRegisteredSubcommands()) {
+if (*S) {
+  EXPECT_EQ("sc1", S->getName());
+}
+  }
+
+  cl::ResetAllOptionOccurrences();
+
+  const char *args2[] = {"prog", "sc2", "-b", "args2 bar string",
+ "-f", "-foo", "foo string"};
+  EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args2) / sizeof(char *), args2,
+  StringRef(), &llvm::nulls()));
+  EXPECT_TRUE(Bar == "args2 bar string");
+  EXPECT_TRUE(Foo);
+  EXPECT_FALSE(SC1_B);
+  EXPECT_TRUE(SC2_Foo == "foo string");
+  for (auto *S : cl::getRegisteredSubcommands()) {
+if (*S) {
+  EXPECT_EQ("sc2", S->getName());
+}
+  }
+}
+
 TEST(CommandLineTest, ArgumentLimit) {
   std::string args(32 * 4096, 'a');
   EXPECT_FALSE(llvm::sys::commandLineFitsWithinSystemLimits("cl", args.data()));
Index: llvm/tools/llvm-opt-report/OptReport.cpp
===
--- llvm/tools/llvm-opt-report/OptReport.cpp
+++ llvm/tools/llvm-opt-report/OptReport.cpp
@@ -36,8 +36,6 @@
 using namespace llvm;
 using namespace llvm::yaml;
 
-static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
-
 // Mark all our options with this category, everything else (except for -version
 // and -help) will be hidden.
 static cl::OptionCategory
@@ -440,11 +438,6 @@
   "A tool to generate an optimization report from YAML optimization"
   " record files.\n");
 
-  if (Help) {
-cl::PrintHelpMessage();
-return 0;
-  }
-
   LocationInfoTy LocationInfo;
   if (!readLocationInfo(LocationInfo))
 return 1;
Index: llvm/test/Support/check-default-options.txt
===
--- /dev/null
+++ llvm/test/Support/check-default-options.txt
@@ -0,0 +1,18 @@
+# RUN: llvm-objdump -help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
+# RUN: llvm-readobj -help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
+# RUN: llvm-tblgen -help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
+# RUN: llvm-opt-report -help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
+# RUN: llvm-dwarfdump -help-hidden %t | FileCheck --check-prefix=CHECK-D

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

@klimek, this patch is now ready for review.  Thanks...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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

[PATCH] D60572: creduce-clang-crash: Use `!` instead of `not`

[Amy Huang via Phabricator via cfe-commits]

akhuang added a comment.

Looks good- if we remove `not` there are a few other things that should be 
removed




Comment at: clang/utils/creduce-clang-crash.py:138
 if "fatal error:" in msg_re:
   self.is_crash = False
 break

also wouldn't need `self.is_crash`



Comment at: clang/utils/creduce-clang-crash.py:178
 
 crash_flag = "--crash" if self.is_crash else ""
 

We should remove `crash_flag` as well


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

https://reviews.llvm.org/D60572



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

[PATCH] D60548: Variable auto-init: also auto-init alloca

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D60548#1462124 , @jfb wrote:

> One downside to alloca is that we can's use `__attribute__((uninitialized))` 
> because it's a builtin. Maybe we need a separate uninitialized alloca? What 
> do you all think?


Actually I'm wondering if we should just implement:

  #pragma clang attribute push (__attribute__((uninitialized)), apply_to = 
variable(unless(is_global)))

And lean on that for `alloca`, instead of having a new uninitialized `alloca` 
builtin. The pragma is useful for debugging regardless, so I think overall it's 
better.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60548



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

[PATCH] D60548: Variable auto-init: also auto-init alloca

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D60548#1463181 , @jfb wrote:

> In D60548#1462124 , @jfb wrote:
>
> > One downside to alloca is that we can's use 
> > `__attribute__((uninitialized))` because it's a builtin. Maybe we need a 
> > separate uninitialized alloca? What do you all think?
>
>
> Actually I'm wondering if we should just implement:
>
>   #pragma clang attribute push (__attribute__((uninitialized)), apply_to = 
> variable(unless(is_global)))
>
>
> And lean on that for `alloca`, instead of having a new uninitialized `alloca` 
> builtin. The pragma is useful for debugging regardless, so I think overall 
> it's better.


Although the pragma syntax 

 doesn't seem to support builtins, only variables, so it's still; weird for 
alloca... I'd really want the pragma to apply to everything in its scope 
including alloca.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60548



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

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas added a comment.

I'll update the patch based on the comments.
Just a note about the false positive ratio. In LibreOffice we run other static 
analyzers too, that's why there were less positive catches there than false 
positives. For example, PVS studio also catches unprotected copy assignment 
operators. Some of these issues were already fixed before I run this new check 
on the code:
https://cgit.freedesktop.org/libreoffice/core/commit/?id=e5e0cc68f70d35e1849aeaf21c0ce68afd6a1f59


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60507



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

[PATCH] D60548: Variable auto-init: also auto-init alloca

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:57
+  case LangOptions::TrivialAutoVarInitKind::Pattern:
+Byte = CGF.Builder.getInt8(0xAA);
+break;

Can this value be taken from some common source with the existing code?  I 
mean, even if it's just a constant in some header, that'd be a start.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60548



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

[PATCH] D60548: Variable auto-init: also auto-init alloca

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 194736.
jfb added a comment.
Herald added a subscriber: mgorny.

- Move patternFor to a shared file as requested by @rjmccall


Repository:
  rC Clang

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

https://reviews.llvm.org/D60548

Files:
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CMakeLists.txt
  lib/CodeGen/PatternInit.cpp
  lib/CodeGen/PatternInit.h
  test/CodeGenCXX/trivial-auto-var-init.cpp

Index: test/CodeGenCXX/trivial-auto-var-init.cpp
===
--- test/CodeGenCXX/trivial-auto-var-init.cpp
+++ test/CodeGenCXX/trivial-auto-var-init.cpp
@@ -173,6 +173,34 @@
   used(ptr);
 }
 
+// UNINIT-LABEL:  test_alloca(
+// ZERO-LABEL:test_alloca(
+// ZERO:  %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// ZERO-NEXT: %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align [[ALIGN:[0-9]+]]
+// ZERO-NEXT: call void @llvm.memset{{.*}}(i8* align [[ALIGN]] %[[ALLOCA]], i8 0, i64 %[[SIZE]], i1 false)
+// PATTERN-LABEL: test_alloca(
+// PATTERN:   %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// PATTERN-NEXT:  %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align [[ALIGN:[0-9]+]]
+// PATTERN-NEXT:  call void @llvm.memset{{.*}}(i8* align [[ALIGN]] %[[ALLOCA]], i8 -86, i64 %[[SIZE]], i1 false)
+void test_alloca(int size) {
+  void *ptr = __builtin_alloca(size);
+  used(ptr);
+}
+
+// UNINIT-LABEL:  test_alloca_with_align(
+// ZERO-LABEL:test_alloca_with_align(
+// ZERO:  %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// ZERO-NEXT: %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align 128
+// ZERO-NEXT: call void @llvm.memset{{.*}}(i8* align 128 %[[ALLOCA]], i8 0, i64 %[[SIZE]], i1 false)
+// PATTERN-LABEL: test_alloca_with_align(
+// PATTERN:   %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// PATTERN-NEXT:  %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align 128
+// PATTERN-NEXT:  call void @llvm.memset{{.*}}(i8* align 128 %[[ALLOCA]], i8 -86, i64 %[[SIZE]], i1 false)
+void test_alloca_with_align(int size) {
+  void *ptr = __builtin_alloca_with_align(size, 1024);
+  used(ptr);
+}
+
 // UNINIT-LABEL:  test_struct_vla(
 // ZERO-LABEL:test_struct_vla(
 // ZERO:  %[[SIZE:[0-9]+]] = mul nuw i64 %{{.*}}, 16
Index: lib/CodeGen/PatternInit.h
===
--- /dev/null
+++ lib/CodeGen/PatternInit.h
@@ -0,0 +1,27 @@
+//===- PatternInit - Pattern initialization -*- 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
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_CODEGEN_PATTERNINIT_H
+#define LLVM_CLANG_LIB_CODEGEN_PATTERNINIT_H
+
+namespace llvm {
+class Constant;
+class Type;
+}
+
+namespace clang {
+namespace CodeGen {
+
+class CodeGenModule;
+
+llvm::Constant *patternFor(CodeGenModule&, llvm::Type*);
+
+}  // end namespace CodeGen
+}  // end namespace clang
+
+#endif
Index: lib/CodeGen/PatternInit.cpp
===
--- /dev/null
+++ lib/CodeGen/PatternInit.cpp
@@ -0,0 +1,97 @@
+//===--- CGDecl.cpp - Emit LLVM Code for declarations -===//
+//
+// 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 "CodeGenModule.h"
+#include "PatternInit.h"
+#include "llvm/IR/Constant.h"
+#include "llvm/IR/Type.h"
+
+namespace clang {
+namespace CodeGen {
+
+llvm::Constant *patternFor(CodeGenModule &CGM, llvm::Type *Ty) {
+  // The following value is a guaranteed unmappable pointer value and has a
+  // repeated byte-pattern which makes it easier to synthesize. We use it for
+  // pointers as well as integers so that aggregates are likely to be
+  // initialized with this repeated value.
+  constexpr uint64_t LargeValue = 0xull;
+  // For 32-bit platforms it's a bit trickier because, across systems, only the
+  // zero page can reasonably be expected to be unmapped, and even then we need
+  // a very low address. We use a smaller value, and that value sadly doesn't
+  // have a repeated byte-pattern. We don't use it for integers.
+  constexpr uint32_t SmallValue = 0x00AA;
+  // Floating-point values are initialized as NaNs because they propagate. Using
+  // a repeated byte pattern means that it will be easier to initialize
+  // all-floating-point aggregates and arrays with memset. Further, aggregates
+  // which mix integral and a few floats might also initialize with memset
+  // followed by a handful of stores

[PATCH] D60548: Variable auto-init: also auto-init alloca

[JF Bastien via Phabricator via cfe-commits]

jfb marked 2 inline comments as done.
jfb added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:57
+  case LangOptions::TrivialAutoVarInitKind::Pattern:
+Byte = CGF.Builder.getInt8(0xAA);
+break;

rjmccall wrote:
> Can this value be taken from some common source with the existing code?  I 
> mean, even if it's just a constant in some header, that'd be a start.
Done.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60548



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

[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

This seems liable to be moderately expensive, especially for large argument 
values, and it's a cost that is unnecessary in the common case where evaluation 
succeeds.

I wonder if we'd be better off speculatively trying the entire evaluation 
without storing any such values, and rerunning the evaluation to collect 
diagnostics only when we find that evaluation would fail (and we're in a 
context where the caller actually wants the diagnostics).


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

https://reviews.llvm.org/D60561



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

r358219 - [DebugInfo] Combine Trivial and NonTrivial flags

[Aaron Smith via cfe-commits]

Author: asmith
Date: Thu Apr 11 13:24:54 2019
New Revision: 358219

URL: http://llvm.org/viewvc/llvm-project?rev=358219&view=rev
Log:
[DebugInfo] Combine Trivial and NonTrivial flags

Summary:
These flags are used when emitting debug info and needed to initialize 
subprogram and member function attributes (function options) for Codeview. 
These function options are used to create an accurate compiler type for UDT 
symbols (class/struct/union) from PDBs.

The Trivial flag was introduced in https://reviews.llvm.org/D45122

It's been pointed out that Trivial and NonTrivial may imply each other and that 
seems to be the case in the current tests. This change combines them into a 
single flag -- NonTrivial -- and updates the corresponding unit tests. There is 
an additional change to llvm to update the flags.

Reviewers: rnk, zturner, dblaikie, probinson, Hui

Reviewed By: dblaikie

Subscribers: aprantl, jdoerfert, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/test/CodeGenCXX/debug-info-composite-triviality.cpp

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=358219&r1=358218&r2=358219&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Thu Apr 11 13:24:54 2019
@@ -3034,10 +3034,8 @@ llvm::DICompositeType *CGDebugInfo::Crea
 else
   Flags |= llvm::DINode::FlagTypePassByValue;
 
-// Record if a C++ record is trivial type.
-if (CXXRD->isTrivial())
-  Flags |= llvm::DINode::FlagTrivial;
-else
+// Record if a C++ record is non-trivial type.
+if (!CXXRD->isTrivial())
   Flags |= llvm::DINode::FlagNonTrivial;
   }
 

Modified: cfe/trunk/test/CodeGenCXX/debug-info-composite-triviality.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-composite-triviality.cpp?rev=358219&r1=358218&r2=358219&view=diff
==
--- cfe/trunk/test/CodeGenCXX/debug-info-composite-triviality.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/debug-info-composite-triviality.cpp Thu Apr 11 
13:24:54 2019
@@ -25,34 +25,40 @@ int GlobalVar = 0;
 
 // Cases to test composite type's triviality
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "Union",{{.*}}flags: 
{{.*}}DIFlagTrivial
+// CHECK-DAG:  {{.*}}!DIGlobalVariable(name: "Union",
+// CHECK-DAG-NEXT: {{^((?!\bDIFlagNonTrivial\b).)*$}}
 union Union {
   int a;
 } Union;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "Trivial",{{.*}}flags: 
{{.*}}DIFlagTrivial
+// CHECK-DAG:  {{.*}}!DIGlobalVariable(name: "Trivial",
+// CHECK-DAG-NEXT: {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct Trivial {
   int i;
 } Trivial;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialA",{{.*}}flags: 
{{.*}}DIFlagTrivial
+// CHECK-DAG:   {{.*}}!DIGlobalVariable(name: "TrivialA",
+// CHECK-DAG-NEXT:  {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct TrivialA {
   TrivialA() = default;
 } TrivialA;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialB",{{.*}}flags: 
{{.*}}DIFlagTrivial
+// CHECK-DAG:   {{.*}}!DIGlobalVariable(name: "TrivialB",
+// CHECK-DAG-NEXT:  {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct TrivialB {
   int m;
   TrivialB(int x) { m = x; }
   TrivialB() = default;
 } TrivialB;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialC",{{.*}}flags: 
{{.*}}DIFlagTrivial
+// CHECK-DAG:   {{.*}}!DIGlobalVariable(name: "TrivialC",
+// CHECK-DAG-NEXT:  {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct TrivialC {
   struct Trivial x;
 } TrivialC;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialD",{{.*}}flags: 
{{.*}}DIFlagTrivial
+// CHECK-DAG:   {{.*}}!DIGlobalVariable(name: "TrivialD",
+// CHECK-DAG-NEXT:  {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct NT {
   NT() {};
 };


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

[PATCH] D59347: [DebugInfo] Combine Trivial and NonTrivial flags

[Aaron Smith via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL358219: [DebugInfo] Combine Trivial and NonTrivial flags 
(authored by asmith, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59347?vs=194249&id=194737#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59347

Files:
  cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
  cfe/trunk/test/CodeGenCXX/debug-info-composite-triviality.cpp


Index: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
@@ -3034,10 +3034,8 @@
 else
   Flags |= llvm::DINode::FlagTypePassByValue;
 
-// Record if a C++ record is trivial type.
-if (CXXRD->isTrivial())
-  Flags |= llvm::DINode::FlagTrivial;
-else
+// Record if a C++ record is non-trivial type.
+if (!CXXRD->isTrivial())
   Flags |= llvm::DINode::FlagNonTrivial;
   }
 
Index: cfe/trunk/test/CodeGenCXX/debug-info-composite-triviality.cpp
===
--- cfe/trunk/test/CodeGenCXX/debug-info-composite-triviality.cpp
+++ cfe/trunk/test/CodeGenCXX/debug-info-composite-triviality.cpp
@@ -25,34 +25,40 @@
 
 // Cases to test composite type's triviality
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "Union",{{.*}}flags: 
{{.*}}DIFlagTrivial
+// CHECK-DAG:  {{.*}}!DIGlobalVariable(name: "Union",
+// CHECK-DAG-NEXT: {{^((?!\bDIFlagNonTrivial\b).)*$}}
 union Union {
   int a;
 } Union;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "Trivial",{{.*}}flags: 
{{.*}}DIFlagTrivial
+// CHECK-DAG:  {{.*}}!DIGlobalVariable(name: "Trivial",
+// CHECK-DAG-NEXT: {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct Trivial {
   int i;
 } Trivial;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialA",{{.*}}flags: 
{{.*}}DIFlagTrivial
+// CHECK-DAG:   {{.*}}!DIGlobalVariable(name: "TrivialA",
+// CHECK-DAG-NEXT:  {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct TrivialA {
   TrivialA() = default;
 } TrivialA;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialB",{{.*}}flags: 
{{.*}}DIFlagTrivial
+// CHECK-DAG:   {{.*}}!DIGlobalVariable(name: "TrivialB",
+// CHECK-DAG-NEXT:  {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct TrivialB {
   int m;
   TrivialB(int x) { m = x; }
   TrivialB() = default;
 } TrivialB;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialC",{{.*}}flags: 
{{.*}}DIFlagTrivial
+// CHECK-DAG:   {{.*}}!DIGlobalVariable(name: "TrivialC",
+// CHECK-DAG-NEXT:  {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct TrivialC {
   struct Trivial x;
 } TrivialC;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialD",{{.*}}flags: 
{{.*}}DIFlagTrivial
+// CHECK-DAG:   {{.*}}!DIGlobalVariable(name: "TrivialD",
+// CHECK-DAG-NEXT:  {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct NT {
   NT() {};
 };


Index: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
@@ -3034,10 +3034,8 @@
 else
   Flags |= llvm::DINode::FlagTypePassByValue;
 
-// Record if a C++ record is trivial type.
-if (CXXRD->isTrivial())
-  Flags |= llvm::DINode::FlagTrivial;
-else
+// Record if a C++ record is non-trivial type.
+if (!CXXRD->isTrivial())
   Flags |= llvm::DINode::FlagNonTrivial;
   }
 
Index: cfe/trunk/test/CodeGenCXX/debug-info-composite-triviality.cpp
===
--- cfe/trunk/test/CodeGenCXX/debug-info-composite-triviality.cpp
+++ cfe/trunk/test/CodeGenCXX/debug-info-composite-triviality.cpp
@@ -25,34 +25,40 @@
 
 // Cases to test composite type's triviality
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "Union",{{.*}}flags: {{.*}}DIFlagTrivial
+// CHECK-DAG:  {{.*}}!DIGlobalVariable(name: "Union",
+// CHECK-DAG-NEXT: {{^((?!\bDIFlagNonTrivial\b).)*$}}
 union Union {
   int a;
 } Union;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "Trivial",{{.*}}flags: {{.*}}DIFlagTrivial
+// CHECK-DAG:  {{.*}}!DIGlobalVariable(name: "Trivial",
+// CHECK-DAG-NEXT: {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct Trivial {
   int i;
 } Trivial;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialA",{{.*}}flags: {{.*}}DIFlagTrivial
+// CHECK-DAG:   {{.*}}!DIGlobalVariable(name: "TrivialA",
+// CHECK-DAG-NEXT:  {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct TrivialA {
   TrivialA() = default;
 } TrivialA;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialB",{{.*}}flags: {{.*}}DIFlagTrivial
+// CHECK-DAG:   {{.*}}!DIGlobalVariable(name: "TrivialB",
+// CHECK-DAG-NEXT:  {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct TrivialB {
   int m;
   TrivialB(int x) { m = x; }
   TrivialB() = default;
 } TrivialB;
 
-// CHECK-DAG:

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

Friendly ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57660



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

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 194741.
gtbercea added a comment.

- Address comments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60568

Files:
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/openmp_offload_registration.cpp

Index: test/OpenMP/openmp_offload_registration.cpp
===
--- test/OpenMP/openmp_offload_registration.cpp
+++ test/OpenMP/openmp_offload_registration.cpp
@@ -26,7 +26,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 2, [[DEVTY]]* getelementptr inbounds ([2 x [[DEVTY]]], [2 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 // Check presence of foo() and the outlined target region
 // CHECK: define void [[FOO:@.+]]()
@@ -34,6 +34,11 @@
 
 // Check registration and unregistration code.
 
+// CHECK: define internal void @.omp_offloading.requires_reg()
+// CHECK: call i32 @__tgt_register_requires(i64 0)
+// CHECK: ret void
+// CHECK: declare i32 @__tgt_register_requires(i64)
+
 // CHECK: define internal void @[[UNREGFN:.+]](i8*)
 // CHECK-SAME: comdat($[[REGFN]]) {
 // CHECK: call i32 @__tgt_unregister_lib([[DSCTY]]* [[DESC]])
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -410,6 +410,10 @@
   AddGlobalCtor(CudaCtorFunction);
   }
   if (OpenMPRuntime) {
+if (llvm::Function *OpenMPRequiresDirectiveRegFun =
+OpenMPRuntime->emitRequiresDirectiveRegFun()) {
+  AddGlobalCtor(OpenMPRequiresDirectiveRegFun, 0, nullptr);
+}
 if (llvm::Function *OpenMPRegistrationFunction =
 OpenMPRuntime->emitRegistrationFunction()) {
   auto ComdatKey = OpenMPRegistrationFunction->hasComdat() ?
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4987,6 +4987,7 @@
   }
 }
   }
+  CGOpenMPRuntime::checkArchForUnifiedAddressing(D);
 }
 
 /// Get number of SMs and number of blocks per SM.
Index: lib/CodeGen/CGOpenMPRuntime.h
===
--- lib/CodeGen/CGOpenMPRuntime.h
+++ lib/CodeGen/CGOpenMPRuntime.h
@@ -636,6 +636,13 @@
   /// must be emitted.
   llvm::SmallDenseSet DeferredGlobalVariables;
 
+  /// Flag for keeping track of weather a requires unified_shared_memory
+  /// directive is present.
+  bool HasRequiresUnifiedSharedMemory = false;
+
+  /// Creates and registers requires directives.
+  llvm::Function *createRequiresDirectiveRegistration();
+
   /// Creates and registers offloading binary descriptor for the current
   /// compilation unit. The function that does the registration is returned.
   llvm::Function *createOffloadingBinaryDescriptorRegistration();
@@ -1429,6 +1436,10 @@
   /// \param GD Global to scan.
   virtual bool emitTargetGlobal(GlobalDecl GD);
 
+  /// Creates and returns a registration function for when at least one
+  /// requires directives was used in the current module.
+  virtual llvm::Function *emitRequiresDirectiveRegFun();
+
   /// Creates the offloading descriptor in the event any target region
   /// was emitted in the current module and return the function that registers
   /// it.
@@ -1597,7 +1608,10 @@
 
   /// Perform check on requires decl to ensure that target architecture
   /// supports unified addressing
-  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const {}
+  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const;
+
+  /// Return true if unified addressing is supported by the architecture.
+  virtual bool hasUnifiedAddressingSupport() const;
 
   /// Checks if the variable has associated OMPAllocateDeclAttr attribute with
   /// the predefined allocator and translates it into the corresponding address
Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -457,6 +457,26 @@
   LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/OMP_IDENT_WORK_DISTRIBUTE)
 };
 
+/// Values for bit flags for marking which requires clauses have been used.
+enum O

[PATCH] D59221: [asan] Add gcc 8's driver option -fsanitize=pointer-compare and -fsanitize=pointer-substract.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno resigned from this revision.
riccibruno added a comment.

I am not the best person to review this, sorry!


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

https://reviews.llvm.org/D59221



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

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 194743.
gtbercea marked 6 inline comments as done.
gtbercea added a comment.

- Address comments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60568

Files:
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/openmp_offload_registration.cpp

Index: test/OpenMP/openmp_offload_registration.cpp
===
--- test/OpenMP/openmp_offload_registration.cpp
+++ test/OpenMP/openmp_offload_registration.cpp
@@ -26,7 +26,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 2, [[DEVTY]]* getelementptr inbounds ([2 x [[DEVTY]]], [2 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 // Check presence of foo() and the outlined target region
 // CHECK: define void [[FOO:@.+]]()
@@ -34,6 +34,11 @@
 
 // Check registration and unregistration code.
 
+// CHECK: define internal void @.omp_offloading.requires_reg()
+// CHECK: call i32 @__tgt_register_requires(i64 0)
+// CHECK: ret void
+// CHECK: declare i32 @__tgt_register_requires(i64)
+
 // CHECK: define internal void @[[UNREGFN:.+]](i8*)
 // CHECK-SAME: comdat($[[REGFN]]) {
 // CHECK: call i32 @__tgt_unregister_lib([[DSCTY]]* [[DESC]])
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -410,6 +410,10 @@
   AddGlobalCtor(CudaCtorFunction);
   }
   if (OpenMPRuntime) {
+if (llvm::Function *OpenMPRequiresDirectiveRegFun =
+OpenMPRuntime->emitRequiresDirectiveRegFun()) {
+  AddGlobalCtor(OpenMPRequiresDirectiveRegFun, 0, nullptr);
+}
 if (llvm::Function *OpenMPRegistrationFunction =
 OpenMPRuntime->emitRegistrationFunction()) {
   auto ComdatKey = OpenMPRegistrationFunction->hasComdat() ?
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4987,6 +4987,7 @@
   }
 }
   }
+  CGOpenMPRuntime::checkArchForUnifiedAddressing(D);
 }
 
 /// Get number of SMs and number of blocks per SM.
Index: lib/CodeGen/CGOpenMPRuntime.h
===
--- lib/CodeGen/CGOpenMPRuntime.h
+++ lib/CodeGen/CGOpenMPRuntime.h
@@ -636,6 +636,10 @@
   /// must be emitted.
   llvm::SmallDenseSet DeferredGlobalVariables;
 
+  /// Flag for keeping track of weather a requires unified_shared_memory
+  /// directive is present.
+  bool HasRequiresUnifiedSharedMemory = false;
+
   /// Creates and registers offloading binary descriptor for the current
   /// compilation unit. The function that does the registration is returned.
   llvm::Function *createOffloadingBinaryDescriptorRegistration();
@@ -1429,6 +1433,10 @@
   /// \param GD Global to scan.
   virtual bool emitTargetGlobal(GlobalDecl GD);
 
+  /// Creates and returns a registration function for when at least one
+  /// requires directives was used in the current module.
+  virtual llvm::Function *emitRequiresDirectiveRegFun();
+
   /// Creates the offloading descriptor in the event any target region
   /// was emitted in the current module and return the function that registers
   /// it.
@@ -1597,7 +1605,10 @@
 
   /// Perform check on requires decl to ensure that target architecture
   /// supports unified addressing
-  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const {}
+  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const;
+
+  /// Return true if unified addressing is supported by the architecture.
+  virtual bool hasUnifiedAddressingSupport() const;
 
   /// Checks if the variable has associated OMPAllocateDeclAttr attribute with
   /// the predefined allocator and translates it into the corresponding address
Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -457,6 +457,26 @@
   LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/OMP_IDENT_WORK_DISTRIBUTE)
 };
 
+/// Values for bit flags for marking which requires clauses have been used.
+enum OpenMPOffloadingRequiresDirFlags : int64_t {
+  /// no requires dire

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea marked 4 inline comments as done.
gtbercea added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:3870
 llvm::Function *
+CGOpenMPRuntime::createRequiresDirectiveRegistration() {
+  // If we don't have entries or if we are emitting code for the device, we

ABataev wrote:
> Why do you need a new member function? Can you make a static local function?
Same as for register lib.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:3887
+CGF.StartFunction(GlobalDecl(), C.VoidTy, RequiresRegFn, FI, {});
+int64_t Flags = OMP_REQ_NONE;
+//TODO: check for other requires clauses.

ABataev wrote:
> Use `OpenMPOffloadingRequiresDirFlags` instead of `int64_t`
Leads to error if I do that. This enum behaves like OpenMPLocationFlags.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7983
 MapFlagsArrayTy &Types) const {
+// If using unified memory, no need to do the mappings.
+if (CGF.CGM.HasRequiresUnifiedSharedMemory)

ABataev wrote:
> Seems to me, this must be in another patch, has nothing to do with this patch
I will split it.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9069
 
+llvm::Function *CGOpenMPRuntime::emitRequiresDirectiveRegFun() {
+  // Create and register the function that handles the requires directives.

ABataev wrote:
> Why do you need the second function?
Second function eliminated.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60568



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