[clang-tools-extra] r360115 - [clangd] add CLANG_ENABLE_CLANGD option to build clangd. Require threads.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Tue May  7 00:05:47 2019
New Revision: 360115

URL: http://llvm.org/viewvc/llvm-project?rev=360115&view=rev
Log:
[clangd] add CLANG_ENABLE_CLANGD option to build clangd. Require threads.

Reviewers: gribozavr

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

Tags: #clang

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

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

Modified: clang-tools-extra/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/CMakeLists.txt?rev=360115&r1=360114&r2=360115&view=diff
==
--- clang-tools-extra/trunk/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/CMakeLists.txt Tue May  7 00:05:47 2019
@@ -1,3 +1,5 @@
+include(CMakeDependentOption)
+
 add_subdirectory(clang-apply-replacements)
 add_subdirectory(clang-reorder-fields)
 add_subdirectory(modularize)
@@ -9,7 +11,6 @@ add_subdirectory(clang-doc)
 add_subdirectory(clang-include-fixer)
 add_subdirectory(clang-move)
 add_subdirectory(clang-query)
-add_subdirectory(clangd)
 add_subdirectory(pp-trace)
 add_subdirectory(tool-template)
 
@@ -25,3 +26,9 @@ if( CLANG_TOOLS_EXTRA_INCLUDE_DOCS )
   add_subdirectory(docs)
 endif()
 
+# clangd has its own CMake tree. It requires threads.
+CMAKE_DEPENDENT_OPTION(CLANG_ENABLE_CLANGD "Build clangd language server" ON
+   "LLVM_ENABLE_THREADS" OFF)
+if (CLANG_ENABLE_CLANGD)
+  add_subdirectory(clangd)
+endif()


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

[PATCH] D61365: [libcxx] [test] Suppress float->int narrowing warning in vector range-construction test.

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter added inline comments.



Comment at: 
test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:159
 float array[3] = {0.0f, 1.0f, 2.0f};
+#pragma warning(suppress: 4244) // narrowing float to int
 std::vector v(array, array + 3);

BillyONeal wrote:
> CaseyCarter wrote:
> > This will blow up non-MSVC-ish when running the test suite with `-Wall -W 
> > -Werror` (which is typical). I suggest wrapping in `#ifdef _MSC_VER`.
> Why didn't it blow up on Contest then? clang-cl is happy with it?
clang-cl is the "ish" in my "MSVC-ish" (MSVC and compilers emulating MSVC). GCC 
and clang-without-`-fms-compatibility` when compiling with `-Wall` warn about 
unrecognized pragmas, just as does cl in default mode 
(https://godbolt.org/z/Chue0L).


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

https://reviews.llvm.org/D61365



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

[PATCH] D61518: [clangd] add CLANG_ENABLE_CLANGD option to build clangd. Require threads.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE360115: [clangd] add CLANG_ENABLE_CLANGD option to build 
clangd. Require threads. (authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61518?vs=198026&id=198400#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61518

Files:
  CMakeLists.txt


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -1,3 +1,5 @@
+include(CMakeDependentOption)
+
 add_subdirectory(clang-apply-replacements)
 add_subdirectory(clang-reorder-fields)
 add_subdirectory(modularize)
@@ -9,7 +11,6 @@
 add_subdirectory(clang-include-fixer)
 add_subdirectory(clang-move)
 add_subdirectory(clang-query)
-add_subdirectory(clangd)
 add_subdirectory(pp-trace)
 add_subdirectory(tool-template)
 
@@ -25,3 +26,9 @@
   add_subdirectory(docs)
 endif()
 
+# clangd has its own CMake tree. It requires threads.
+CMAKE_DEPENDENT_OPTION(CLANG_ENABLE_CLANGD "Build clangd language server" ON
+   "LLVM_ENABLE_THREADS" OFF)
+if (CLANG_ENABLE_CLANGD)
+  add_subdirectory(clangd)
+endif()


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -1,3 +1,5 @@
+include(CMakeDependentOption)
+
 add_subdirectory(clang-apply-replacements)
 add_subdirectory(clang-reorder-fields)
 add_subdirectory(modularize)
@@ -9,7 +11,6 @@
 add_subdirectory(clang-include-fixer)
 add_subdirectory(clang-move)
 add_subdirectory(clang-query)
-add_subdirectory(clangd)
 add_subdirectory(pp-trace)
 add_subdirectory(tool-template)
 
@@ -25,3 +26,9 @@
   add_subdirectory(docs)
 endif()
 
+# clangd has its own CMake tree. It requires threads.
+CMAKE_DEPENDENT_OPTION(CLANG_ENABLE_CLANGD "Build clangd language server" ON
+   "LLVM_ENABLE_THREADS" OFF)
+if (CLANG_ENABLE_CLANGD)
+  add_subdirectory(clangd)
+endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61122: [clangd] Don't build clangd or run its tests when LLVM_ENABLE_THREADS is off, unless specifically directed to do so

[Sam McCall via Phabricator via cfe-commits]

sammccall abandoned this revision.
sammccall added a comment.

Superseded by rL360115 


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61122



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

[clang-tools-extra] r360116 - [clangd] Move Rename into its own file, and add unit test. NFC

[Sam McCall via cfe-commits]

Author: sammccall
Date: Tue May  7 00:11:56 2019
New Revision: 360116

URL: http://llvm.org/viewvc/llvm-project?rev=360116&view=rev
Log:
[clangd] Move Rename into its own file, and add unit test. NFC

Reviewers: kadircet

Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, arphaman, jfb, cfe-commits

Tags: #clang

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

Added:
clang-tools-extra/trunk/clangd/refactor/Rename.cpp
clang-tools-extra/trunk/clangd/refactor/Rename.h
clang-tools-extra/trunk/clangd/unittests/RenameTests.cpp
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/unittests/CMakeLists.txt

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=360116&r1=360115&r2=360116&view=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Tue May  7 00:11:56 2019
@@ -89,6 +89,7 @@ add_clang_library(clangDaemon
   index/dex/PostingList.cpp
   index/dex/Trigram.cpp
 
+  refactor/Rename.cpp
   refactor/Tweak.cpp
 
   LINK_LIBS

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=360116&r1=360115&r2=360116&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue May  7 00:11:56 2019
@@ -18,6 +18,7 @@
 #include "index/CanonicalIncludes.h"
 #include "index/FileIndex.h"
 #include "index/Merge.h"
+#include "refactor/Rename.h"
 #include "refactor/Tweak.h"
 #include "clang/Format/Format.h"
 #include "clang/Frontend/CompilerInstance.h"
@@ -25,8 +26,6 @@
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Core/Replacement.h"
-#include "clang/Tooling/Refactoring/RefactoringResultConsumer.h"
-#include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/ScopeExit.h"
@@ -44,38 +43,6 @@ namespace clang {
 namespace clangd {
 namespace {
 
-// Expand a DiagnosticError to make it print-friendly (print the detailed
-// message, rather than "clang diagnostic").
-llvm::Error expandDiagnostics(llvm::Error Err, DiagnosticsEngine &DE) {
-  if (auto Diag = DiagnosticError::take(Err)) {
-llvm::cantFail(std::move(Err));
-SmallVector DiagMessage;
-Diag->second.EmitToString(DE, DiagMessage);
-return llvm::make_error(DiagMessage,
-   llvm::inconvertibleErrorCode());
-  }
-  return Err;
-}
-
-class RefactoringResultCollector final
-: public tooling::RefactoringResultConsumer {
-public:
-  void handleError(llvm::Error Err) override {
-assert(!Result.hasValue());
-Result = std::move(Err);
-  }
-
-  // Using the handle(SymbolOccurrences) from parent class.
-  using tooling::RefactoringResultConsumer::handle;
-
-  void handle(tooling::AtomicChanges SourceReplacements) override {
-assert(!Result.hasValue());
-Result = std::move(SourceReplacements);
-  }
-
-  llvm::Optional> Result;
-};
-
 // Update the FileIndex with new ASTs and plumb the diagnostics responses.
 struct UpdateIndexCallbacks : public ParsingCallbacks {
   UpdateIndexCallbacks(FileIndex *FIndex, DiagnosticsConsumer &DiagConsumer)
@@ -299,47 +266,13 @@ void ClangdServer::rename(PathRef File,
   llvm::Expected InpAST) {
 if (!InpAST)
   return CB(InpAST.takeError());
-auto &AST = InpAST->AST;
-
-RefactoringResultCollector ResultCollector;
-const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
-SourceLocation SourceLocationBeg =
-clangd::getBeginningOfIdentifier(AST, Pos, SourceMgr.getMainFileID());
-tooling::RefactoringRuleContext Context(
-AST.getASTContext().getSourceManager());
-Context.setASTContext(AST.getASTContext());
-auto Rename = clang::tooling::RenameOccurrences::initiate(
-Context, SourceRange(SourceLocationBeg), NewName);
-if (!Rename)
-  return CB(expandDiagnostics(Rename.takeError(),
-  AST.getASTContext().getDiagnostics()));
-
-Rename->invoke(ResultCollector, Context);
-
-assert(ResultCollector.Result.hasValue());
-if (!ResultCollector.Result.getValue())
-  return CB(expandDiagnostics(ResultCollector.Result->takeError(),
-  AST.getASTContext().getDiagnostics()));
-
-std::vector Replacements;
-for (const tooling::AtomicChange &Change : ResultCollector.Result->get()) {
-  

[PATCH] D61596: [clangd] Move Rename into its own file, and add unit test. NFC

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rCTE360116: [clangd] Move Rename into its own file, and add 
unit test. NFC (authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61596?vs=198269&id=198401#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61596

Files:
  clangd/CMakeLists.txt
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/refactor/Rename.cpp
  clangd/refactor/Rename.h
  clangd/unittests/CMakeLists.txt
  clangd/unittests/RenameTests.cpp

Index: clangd/unittests/CMakeLists.txt
===
--- clangd/unittests/CMakeLists.txt
+++ clangd/unittests/CMakeLists.txt
@@ -48,6 +48,7 @@
   JSONTransportTests.cpp
   PrintASTTests.cpp
   QualityTests.cpp
+  RenameTests.cpp
   RIFFTests.cpp
   SelectionTests.cpp
   SerializationTests.cpp
Index: clangd/unittests/RenameTests.cpp
===
--- clangd/unittests/RenameTests.cpp
+++ clangd/unittests/RenameTests.cpp
@@ -0,0 +1,47 @@
+//===-- RenameTests.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 "Annotations.h"
+#include "TestFS.h"
+#include "TestTU.h"
+#include "refactor/Rename.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(RenameTest, SingleFile) {
+  Annotations Code(R"cpp(
+void foo() {
+  fo^o();
+}
+  )cpp");
+  auto TU = TestTU::withCode(Code.code());
+  TU.HeaderCode = "void foo();"; // outside main file, will not be touched.
+
+  auto AST = TU.build();
+  auto RenameResult =
+  renameWithinFile(AST, testPath(TU.Filename), Code.point(), "abcde");
+  ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError();
+  auto ApplyResult = tooling::applyAllReplacements(Code.code(), *RenameResult);
+  ASSERT_TRUE(bool(ApplyResult)) << ApplyResult.takeError();
+
+  const char *Want = R"cpp(
+void abcde() {
+  abcde();
+}
+  )cpp";
+  EXPECT_EQ(Want, *ApplyResult);
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -18,6 +18,7 @@
 #include "index/CanonicalIncludes.h"
 #include "index/FileIndex.h"
 #include "index/Merge.h"
+#include "refactor/Rename.h"
 #include "refactor/Tweak.h"
 #include "clang/Format/Format.h"
 #include "clang/Frontend/CompilerInstance.h"
@@ -25,8 +26,6 @@
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Core/Replacement.h"
-#include "clang/Tooling/Refactoring/RefactoringResultConsumer.h"
-#include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/ScopeExit.h"
@@ -44,38 +43,6 @@
 namespace clangd {
 namespace {
 
-// Expand a DiagnosticError to make it print-friendly (print the detailed
-// message, rather than "clang diagnostic").
-llvm::Error expandDiagnostics(llvm::Error Err, DiagnosticsEngine &DE) {
-  if (auto Diag = DiagnosticError::take(Err)) {
-llvm::cantFail(std::move(Err));
-SmallVector DiagMessage;
-Diag->second.EmitToString(DE, DiagMessage);
-return llvm::make_error(DiagMessage,
-   llvm::inconvertibleErrorCode());
-  }
-  return Err;
-}
-
-class RefactoringResultCollector final
-: public tooling::RefactoringResultConsumer {
-public:
-  void handleError(llvm::Error Err) override {
-assert(!Result.hasValue());
-Result = std::move(Err);
-  }
-
-  // Using the handle(SymbolOccurrences) from parent class.
-  using tooling::RefactoringResultConsumer::handle;
-
-  void handle(tooling::AtomicChanges SourceReplacements) override {
-assert(!Result.hasValue());
-Result = std::move(SourceReplacements);
-  }
-
-  llvm::Optional> Result;
-};
-
 // Update the FileIndex with new ASTs and plumb the diagnostics responses.
 struct UpdateIndexCallbacks : public ParsingCallbacks {
   UpdateIndexCallbacks(FileIndex *FIndex, DiagnosticsConsumer &DiagConsumer)
@@ -299,47 +266,13 @@
   llvm::Expected InpAST) {
 if (!InpAST)
   return CB(InpAST.takeError());
-auto &AST = InpAST->AST;
-
-RefactoringResultCollector ResultCollector;
-const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
-SourceLoca

[PATCH] D61596: [clangd] Move Rename into its own file, and add unit test. NFC

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clangd/ClangdServer.cpp:264
   Callback> CB) {
-  auto Action = [Pos](Path File, std::string NewName,
+  auto Action = [Pos](std::string File, std::string NewName,
   Callback> CB,

kadircet wrote:
> Why change Path to std::string ?
Oops, removed this capture and then re-added it.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61596



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

r360117 - Improve function / variable disambiguation.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Tue May  7 00:36:07 2019
New Revision: 360117

URL: http://llvm.org/viewvc/llvm-project?rev=360117&view=rev
Log:
Improve function / variable disambiguation.

Keep looking for decl-specifiers after an unknown identifier. Don't
issue diagnostics about an error type specifier conflicting with later
type specifiers.

Modified:
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/Parse/ParseTentative.cpp
cfe/trunk/lib/Sema/DeclSpec.cpp
cfe/trunk/test/CodeGen/builtins-ppc-altivec.c
cfe/trunk/test/Parser/cxx-ambig-decl-expr.cpp

Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=360117&r1=360116&r2=360117&view=diff
==
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Tue May  7 00:36:07 2019
@@ -2743,7 +2743,7 @@ bool Parser::ParseImplicitInt(DeclSpec &
   // TODO: Could inject an invalid typedef decl in an enclosing scope to
   // avoid rippling error messages on subsequent uses of the same type,
   // could be useful if #include was forgotten.
-  return false;
+  return true;
 }
 
 /// Determine the declaration specifier context from the declarator

Modified: cfe/trunk/lib/Parse/ParseTentative.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseTentative.cpp?rev=360117&r1=360116&r2=360117&view=diff
==
--- cfe/trunk/lib/Parse/ParseTentative.cpp (original)
+++ cfe/trunk/lib/Parse/ParseTentative.cpp Tue May  7 00:36:07 2019
@@ -1884,31 +1884,31 @@ Parser::TryParseParameterDeclarationClau
 // decl-specifier-seq '{' is not a parameter in C++11.
 TPResult TPR = isCXXDeclarationSpecifier(TPResult::False,
  InvalidAsDeclaration);
+// A declaration-specifier (not followed by '(' or '{') means this can't be
+// an expression, but it could still be a template argument.
+if (TPR != TPResult::Ambiguous &&
+!(VersusTemplateArgument && TPR == TPResult::True))
+  return TPR;
 
-if (VersusTemplateArgument && TPR == TPResult::True) {
-  // Consume the decl-specifier-seq. We have to look past it, since a
-  // type-id might appear here in a template argument.
-  bool SeenType = false;
-  do {
-SeenType |= isCXXDeclarationSpecifierAType();
-if (TryConsumeDeclarationSpecifier() == TPResult::Error)
-  return TPResult::Error;
-
-// If we see a parameter name, this can't be a template argument.
-if (SeenType && Tok.is(tok::identifier))
-  return TPResult::True;
-
-TPR = isCXXDeclarationSpecifier(TPResult::False,
-InvalidAsDeclaration);
-if (TPR == TPResult::Error)
-  return TPR;
-  } while (TPR != TPResult::False);
-} else if (TPR == TPResult::Ambiguous) {
-  // Disambiguate what follows the decl-specifier.
+bool SeenType = false;
+do {
+  SeenType |= isCXXDeclarationSpecifierAType();
   if (TryConsumeDeclarationSpecifier() == TPResult::Error)
 return TPResult::Error;
-} else
-  return TPR;
+
+  // If we see a parameter name, this can't be a template argument.
+  if (SeenType && Tok.is(tok::identifier))
+return TPResult::True;
+
+  TPR = isCXXDeclarationSpecifier(TPResult::False,
+  InvalidAsDeclaration);
+  if (TPR == TPResult::Error)
+return TPR;
+
+  // Two declaration-specifiers means this can't be an expression.
+  if (TPR == TPResult::True && !VersusTemplateArgument)
+return TPR;
+} while (TPR != TPResult::False);
 
 // declarator
 // abstract-declarator[opt]

Modified: cfe/trunk/lib/Sema/DeclSpec.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/DeclSpec.cpp?rev=360117&r1=360116&r2=360117&view=diff
==
--- cfe/trunk/lib/Sema/DeclSpec.cpp (original)
+++ cfe/trunk/lib/Sema/DeclSpec.cpp Tue May  7 00:36:07 2019
@@ -706,6 +706,8 @@ bool DeclSpec::SetTypeSpecType(TST T, So
const PrintingPolicy &Policy) {
   assert(isTypeRep(T) && "T does not store a type");
   assert(Rep && "no type provided!");
+  if (TypeSpecType == TST_error)
+return false;
   if (TypeSpecType != TST_unspecified) {
 PrevSpec = DeclSpec::getSpecifierName((TST) TypeSpecType, Policy);
 DiagID = diag::err_invalid_decl_spec_combination;
@@ -726,6 +728,8 @@ bool DeclSpec::SetTypeSpecType(TST T, So
const PrintingPolicy &Policy) {
   assert(isExprRep(T) && "T does not store an expr");
   assert(Rep && "no expression provided!");
+  if (TypeSpecType == TST_error)
+return false;
   if (TypeSpecType != TST_unspecified) {
 PrevSpec = DeclSpec::getSpeci

[clang-tools-extra] r360118 - [clangd] Add test that r360116 accidentally fixed a duplicate-edits bug in rename. NFC

[Sam McCall via cfe-commits]

Author: sammccall
Date: Tue May  7 00:45:41 2019
New Revision: 360118

URL: http://llvm.org/viewvc/llvm-project?rev=360118&view=rev
Log:
[clangd] Add test that r360116 accidentally fixed a duplicate-edits bug in 
rename. NFC

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

Modified: clang-tools-extra/trunk/clangd/refactor/Rename.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/Rename.cpp?rev=360118&r1=360117&r2=360118&view=diff
==
--- clang-tools-extra/trunk/clangd/refactor/Rename.cpp (original)
+++ clang-tools-extra/trunk/clangd/refactor/Rename.cpp Tue May  7 00:45:41 2019
@@ -66,6 +66,8 @@ renameWithinFile(ParsedAST &AST, llvm::S
   // Right now we only support renaming the main file, so we
   // drop replacements not for the main file. In the future, we might
   // also support rename with wider scope.
+  // Rename sometimes returns duplicate edits (which is a bug). A side-effect 
of 
+  // adding them to a single Replacements object is these are deduplicated.
   for (const tooling::AtomicChange &Change : ResultCollector.Result->get()) {
 for (const auto &Rep : Change.getReplacements()) {
   if (Rep.getFilePath() == File)

Modified: clang-tools-extra/trunk/clangd/unittests/RenameTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/RenameTests.cpp?rev=360118&r1=360117&r2=360118&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/RenameTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/RenameTests.cpp Tue May  7 
00:45:41 2019
@@ -19,27 +19,72 @@ namespace clangd {
 namespace {
 
 TEST(RenameTest, SingleFile) {
-  Annotations Code(R"cpp(
-void foo() {
-  fo^o();
-}
-  )cpp");
-  auto TU = TestTU::withCode(Code.code());
-  TU.HeaderCode = "void foo();"; // outside main file, will not be touched.
+  struct Test {
+const char* Before;
+const char* After;
+  } Tests[] = {
+  // Rename function.
+  {
+  R"cpp(
+void foo() {
+  fo^o();
+}
+  )cpp",
+  R"cpp(
+void abcde() {
+  abcde();
+}
+  )cpp",
+  },
+  // Rename type.
+  {
+  R"cpp(
+struct foo{};
+foo test() {
+   f^oo x;
+   return x;
+}
+  )cpp",
+  R"cpp(
+struct abcde{};
+abcde test() {
+   abcde x;
+   return x;
+}
+  )cpp",
+  },
+  // Rename variable.
+  {
+  R"cpp(
+void bar() {
+  if (auto ^foo = 5) {
+foo = 3;
+  }
+}
+  )cpp",
+  R"cpp(
+void bar() {
+  if (auto abcde = 5) {
+abcde = 3;
+  }
+}
+  )cpp",
+  },
+  };
+  for (const Test &T : Tests) {
+Annotations Code(T.Before);
+auto TU = TestTU::withCode(Code.code());
+TU.HeaderCode = "void foo();"; // outside main file, will not be touched.
+auto AST = TU.build();
+auto RenameResult =
+renameWithinFile(AST, testPath(TU.Filename), Code.point(), "abcde");
+ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError();
+auto ApplyResult =
+tooling::applyAllReplacements(Code.code(), *RenameResult);
+ASSERT_TRUE(bool(ApplyResult)) << ApplyResult.takeError();
 
-  auto AST = TU.build();
-  auto RenameResult =
-  renameWithinFile(AST, testPath(TU.Filename), Code.point(), "abcde");
-  ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError();
-  auto ApplyResult = tooling::applyAllReplacements(Code.code(), *RenameResult);
-  ASSERT_TRUE(bool(ApplyResult)) << ApplyResult.takeError();
-
-  const char *Want = R"cpp(
-void abcde() {
-  abcde();
-}
-  )cpp";
-  EXPECT_EQ(Want, *ApplyResult);
+EXPECT_EQ(T.After, *ApplyResult) << T.Before;
+  }
 }
 
 } // namespace


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

[PATCH] D61596: [clangd] Move Rename into its own file, and add unit test. NFC

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Incidentally, this was not NFC - it deduplicates edits and therefore works 
around a bug in the visitor!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61596



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

[PATCH] D61508: [clang-tidy] bugprone-header-guard : a simple version of llvm-header-guard

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

> My understanding of the directory layout guidance in the docs is that 
> different styles get different directories.

Yes, but for "the same check" we use aliases instead. When aliasing checks in 
different categories it is possible to change default configuration values, so 
we keep the code in one check and do the rest with options.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61508



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

[PATCH] D60678: [libclang] Forward isInline for NamespaceDecl to libclang

[Nikolai Kosjar via Phabricator via cfe-commits]

nik added a comment.

...and increased  CINDEX_VERSION_MINOR.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60678



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

[PATCH] D61601: [clangd] Represent Hover result using FormattedString

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

I think it makes more sense to expose semantical information in HoverInfo(like 
Name, Scope, Definition, etc), rather than formatted strings, and let this be 
serialized by the users of ClangdServer (as in D61497 
). This is what I'll perform with that patch 
anyway, since it aims to provide consumers of ClangdServer with sementical 
information. So it is up to you whether to change the layering or keep it as it 
is in this patch.




Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:853
+  Hover R;
+  switch (HoverContentFormat) {
+  case MarkupKind::Plaintext:

Why not move `R.contents.kind` and `R.range` assignment out of the switch 
statement. and perform return after the switch. That way you can get rid of the 
`llvm_unreachable`(we would still get warnings for that switch-statement if 
someone adds a new `MarkupKind` but doesn't update that statement.

And hopefully we should fallback to `PlainText` if editor doesn't support any 
markupkinds known to us.



Comment at: clang-tools-extra/clangd/Protocol.cpp:707
+  else
+return false;
+  return true;

Maybe also vlog/elog the unknown kind? So that we can easily catch new 
additions to spec.



Comment at: clang-tools-extra/clangd/Protocol.h:359
+enum class MarkupKind {
+  Plaintext,
+  Markdown,

This is mentioned as `PlainText` in the specs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61601



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

[PATCH] D60678: [libclang] Forward isInline for NamespaceDecl to libclang

[Nikolai Kosjar via Phabricator via cfe-commits]

nik updated this revision to Diff 198403.
nik added a comment.
Herald added a project: clang.

Adapted c-index-test.c and added function to libclang.exports.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60678

Files:
  include/clang-c/Index.h
  test/Index/print-type.cpp
  tools/c-index-test/c-index-test.c
  tools/libclang/CXType.cpp
  tools/libclang/libclang.exports


Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -43,6 +43,7 @@
 clang_Cursor_isBitField
 clang_Cursor_isDynamicCall
 clang_Cursor_isExternalSymbol
+clang_Cursor_isInlineNamespace
 clang_Cursor_isNull
 clang_Cursor_isObjCOptional
 clang_Cursor_isVariadic
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -1263,6 +1263,14 @@
   return 0;
 }
 
+unsigned clang_Cursor_isInlineNamespace(CXCursor C) {
+  if (!clang_isDeclaration(C.kind))
+return 0;
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const NamespaceDecl *ND = dyn_cast_or_null(D);
+  return ND ? ND->isInline() : 0;
+}
+
 CXType clang_Type_getNamedType(CXType CT){
   QualType T = GetQualType(CT);
   const Type *TP = T.getTypePtrOrNull();
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -1671,6 +1671,13 @@
   printf(" [isAnonRecDecl=%d]", isAnonRecDecl);
 }
 
+/* Print if it is an inline namespace decl */
+{
+  unsigned isInlineNamespace = clang_Cursor_isInlineNamespace(cursor);
+  if (isInlineNamespace != 0)
+printf(" [isInlineNamespace=%d]", isInlineNamespace);
+}
+
 printf("\n");
   }
   return CXChildVisit_Recurse;
Index: test/Index/print-type.cpp
===
--- test/Index/print-type.cpp
+++ test/Index/print-type.cpp
@@ -90,6 +90,8 @@
 namespace {
   int a;
 }
+
+inline namespace InlineNS {}
 // RUN: c-index-test -test-print-type %s -std=c++14 | FileCheck %s
 // CHECK: Namespace=outer:1:11 (Definition) [type=] [typekind=Invalid] 
[isPOD=0]
 // CHECK: ClassTemplate=Foo:4:8 (Definition) [type=] [typekind=Invalid] 
[isPOD=0]
@@ -204,3 +206,4 @@
 // CHECK: UnionDecl=:86:3 (Definition) [type=X::(anonymous union at 
{{.*}}print-type.cpp:86:3)] [typekind=Record] [isPOD=1] [nbFields=2] [isAnon=1]
 // CHECK: EnumDecl=:87:3 (Definition) [type=X::(anonymous enum at 
{{.*}}print-type.cpp:87:3)] [typekind=Enum] [isPOD=1] [isAnon=1]
 // CHECK: Namespace=:90:11 (Definition) [type=] [typekind=Invalid] [isPOD=0] 
[isAnon=1]
+// CHECK: Namespace=InlineNS:94:18 (Definition) [type=] [typekind=Invalid] 
[isPOD=0] [isAnonRecDecl=0] [isInlineNamespace=1]
Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -32,7 +32,7 @@
  * compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable.
  */
 #define CINDEX_VERSION_MAJOR 0
-#define CINDEX_VERSION_MINOR 56
+#define CINDEX_VERSION_MINOR 57
 
 #define CINDEX_VERSION_ENCODE(major, minor) ( \
   ((major) * 1)   \
@@ -3932,6 +3932,12 @@
  */
 CINDEX_LINKAGE unsigned clang_Cursor_isAnonymousRecordDecl(CXCursor C);
 
+/**
+ * Determine whether the given cursor represents an inline namespace
+ * declaration.
+ */
+CINDEX_LINKAGE unsigned clang_Cursor_isInlineNamespace(CXCursor C);
+
 enum CXRefQualifierKind {
   /** No ref-qualifier was provided. */
   CXRefQualifier_None = 0,


Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -43,6 +43,7 @@
 clang_Cursor_isBitField
 clang_Cursor_isDynamicCall
 clang_Cursor_isExternalSymbol
+clang_Cursor_isInlineNamespace
 clang_Cursor_isNull
 clang_Cursor_isObjCOptional
 clang_Cursor_isVariadic
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -1263,6 +1263,14 @@
   return 0;
 }
 
+unsigned clang_Cursor_isInlineNamespace(CXCursor C) {
+  if (!clang_isDeclaration(C.kind))
+return 0;
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const NamespaceDecl *ND = dyn_cast_or_null(D);
+  return ND ? ND->isInline() : 0;
+}
+
 CXType clang_Type_getNamedType(CXType CT){
   QualType T = GetQualType(CT);
   const Type *TP = T.getTypePtrOrNull();
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -1671,6 +1671,13 @@
   printf(" [isAnonRecDecl=%d]", isAnonRecDecl);
 }
 
+  

[PATCH] D60678: [libclang] Forward isInline for NamespaceDecl to libclang

[Nikolai Kosjar via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D60678



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

[clang-tools-extra] r360119 - [clangd] switchSourceHeader uses null not empty string as sentinel.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Tue May  7 00:55:35 2019
New Revision: 360119

URL: http://llvm.org/viewvc/llvm-project?rev=360119&view=rev
Log:
[clangd] switchSourceHeader uses null not empty string as sentinel.

As far as I can see, only theia actually implements this, and it expects null.

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=360119&r1=360118&r2=360119&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Tue May  7 00:55:35 2019
@@ -823,10 +823,13 @@ void ClangdLSPServer::onGoToDeclaration(
   std::move(Reply)));
 }
 
-void ClangdLSPServer::onSwitchSourceHeader(const TextDocumentIdentifier 
&Params,
-   Callback Reply) {
-  llvm::Optional Result = Server->switchSourceHeader(Params.uri.file());
-  Reply(Result ? URI::createFile(*Result).toString() : "");
+void ClangdLSPServer::onSwitchSourceHeader(
+const TextDocumentIdentifier &Params,
+Callback> Reply) {
+  if (auto Result = Server->switchSourceHeader(Params.uri.file()))
+Reply(URI::resolvePath(*Result, Params.uri.file()));
+  else
+Reply(llvm::None);
 }
 
 void ClangdLSPServer::onDocumentHighlight(

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=360119&r1=360118&r2=360119&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Tue May  7 00:55:35 2019
@@ -85,7 +85,7 @@ private:
 Callback>);
   void onReference(const ReferenceParams &, Callback>);
   void onSwitchSourceHeader(const TextDocumentIdentifier &,
-Callback);
+Callback>);
   void onDocumentHighlight(const TextDocumentPositionParams &,
Callback>);
   void onFileEvent(const DidChangeWatchedFilesParams &);


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

[PATCH] D61628: Fix a bug that reports UTF16 (LE) files as UTF32 (LE) ones

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added a reviewer: sammccall.
owenpan added a project: clang.
Herald added a subscriber: cfe-commits.

Also fix a typo for the SCSU byte order mark.


Repository:
  rC Clang

https://reviews.llvm.org/D61628

Files:
  clang/lib/Basic/SourceManager.cpp


Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -167,16 +167,16 @@
   // http://en.wikipedia.org/wiki/Byte_order_mark for more information.
   StringRef BufStr = Buffer.getPointer()->getBuffer();
   const char *InvalidBOM = llvm::StringSwitch(BufStr)
-.StartsWith("\xFE\xFF", "UTF-16 (BE)")
-.StartsWith("\xFF\xFE", "UTF-16 (LE)")
 .StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
   "UTF-32 (BE)")
 .StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
   "UTF-32 (LE)")
+.StartsWith("\xFE\xFF", "UTF-16 (BE)")
+.StartsWith("\xFF\xFE", "UTF-16 (LE)")
 .StartsWith("\x2B\x2F\x76", "UTF-7")
 .StartsWith("\xF7\x64\x4C", "UTF-1")
 .StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
-.StartsWith("\x0E\xFE\xFF", "SDSU")
+.StartsWith("\x0E\xFE\xFF", "SCSU")
 .StartsWith("\xFB\xEE\x28", "BOCU-1")
 .StartsWith("\x84\x31\x95\x33", "GB-18030")
 .Default(nullptr);


Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -167,16 +167,16 @@
   // http://en.wikipedia.org/wiki/Byte_order_mark for more information.
   StringRef BufStr = Buffer.getPointer()->getBuffer();
   const char *InvalidBOM = llvm::StringSwitch(BufStr)
-.StartsWith("\xFE\xFF", "UTF-16 (BE)")
-.StartsWith("\xFF\xFE", "UTF-16 (LE)")
 .StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
   "UTF-32 (BE)")
 .StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
   "UTF-32 (LE)")
+.StartsWith("\xFE\xFF", "UTF-16 (BE)")
+.StartsWith("\xFF\xFE", "UTF-16 (LE)")
 .StartsWith("\x2B\x2F\x76", "UTF-7")
 .StartsWith("\xF7\x64\x4C", "UTF-1")
 .StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
-.StartsWith("\x0E\xFE\xFF", "SDSU")
+.StartsWith("\x0E\xFE\xFF", "SCSU")
 .StartsWith("\xFB\xEE\x28", "BOCU-1")
 .StartsWith("\x84\x31\x95\x33", "GB-18030")
 .Default(nullptr);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r360120 - [Sema] Add missing VisitMacroQualifiedTypeLoc to TypeSpecLocFiller

[Leonard Chan via cfe-commits]

Author: leonardchan
Date: Tue May  7 01:12:28 2019
New Revision: 360120

URL: http://llvm.org/viewvc/llvm-project?rev=360120&view=rev
Log:
[Sema] Add missing VisitMacroQualifiedTypeLoc to TypeSpecLocFiller

To hopefully fix greenbot failures

Modified:
cfe/trunk/lib/Sema/SemaType.cpp

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=360120&r1=360119&r2=360120&view=diff
==
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Tue May  7 01:12:28 2019
@@ -5388,6 +5388,11 @@ namespace {
   Visit(TL.getModifiedLoc());
   fillAttributedTypeLoc(TL, State);
 }
+void VisitMacroQualifiedTypeLoc(MacroQualifiedTypeLoc TL) {
+  Visit(TL.getInnerLoc());
+  TL.setExpansionLoc(
+  State.getExpansionLocForMacroQualifiedType(TL.getTypePtr()));
+}
 void VisitQualifiedTypeLoc(QualifiedTypeLoc TL) {
   Visit(TL.getUnqualifiedLoc());
 }


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

[PATCH] D61630: [clangd] Oops, switchSourceHeader still needs to return a URI.

[Sam McCall via Phabricator via cfe-commits]

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

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61630

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h


Index: clangd/ClangdLSPServer.h
===
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -85,7 +85,7 @@
 Callback>);
   void onReference(const ReferenceParams &, Callback>);
   void onSwitchSourceHeader(const TextDocumentIdentifier &,
-Callback>);
+Callback>);
   void onDocumentHighlight(const TextDocumentPositionParams &,
Callback>);
   void onFileEvent(const DidChangeWatchedFilesParams &);
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -825,9 +825,9 @@
 
 void ClangdLSPServer::onSwitchSourceHeader(
 const TextDocumentIdentifier &Params,
-Callback> Reply) {
+Callback> Reply) {
   if (auto Result = Server->switchSourceHeader(Params.uri.file()))
-Reply(URI::resolvePath(*Result, Params.uri.file()));
+Reply(URIForFile::canonicalize(*Result, Params.uri.file()));
   else
 Reply(llvm::None);
 }


Index: clangd/ClangdLSPServer.h
===
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -85,7 +85,7 @@
 Callback>);
   void onReference(const ReferenceParams &, Callback>);
   void onSwitchSourceHeader(const TextDocumentIdentifier &,
-Callback>);
+Callback>);
   void onDocumentHighlight(const TextDocumentPositionParams &,
Callback>);
   void onFileEvent(const DidChangeWatchedFilesParams &);
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -825,9 +825,9 @@
 
 void ClangdLSPServer::onSwitchSourceHeader(
 const TextDocumentIdentifier &Params,
-Callback> Reply) {
+Callback> Reply) {
   if (auto Result = Server->switchSourceHeader(Params.uri.file()))
-Reply(URI::resolvePath(*Result, Params.uri.file()));
+Reply(URIForFile::canonicalize(*Result, Params.uri.file()));
   else
 Reply(llvm::None);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61631: [Tooling] Don't mmap the JSONCompilationDatabase, it's not correct for long-lived processes.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D61631

Files:
  lib/Tooling/JSONCompilationDatabase.cpp


Index: lib/Tooling/JSONCompilationDatabase.cpp
===
--- lib/Tooling/JSONCompilationDatabase.cpp
+++ lib/Tooling/JSONCompilationDatabase.cpp
@@ -190,8 +190,11 @@
 JSONCompilationDatabase::loadFromFile(StringRef FilePath,
   std::string &ErrorMessage,
   JSONCommandLineSyntax Syntax) {
+  // Don't mmap: if we're a long-lived process, the build system may overwrite.
   llvm::ErrorOr> DatabaseBuffer =
-  llvm::MemoryBuffer::getFile(FilePath);
+  llvm::MemoryBuffer::getFile(FilePath, /*FileSize=*/-1,
+  /*RequiresNullTerminator=*/true,
+  /*IsVolatile=*/true);
   if (std::error_code Result = DatabaseBuffer.getError()) {
 ErrorMessage = "Error while opening JSON database: " + Result.message();
 return nullptr;


Index: lib/Tooling/JSONCompilationDatabase.cpp
===
--- lib/Tooling/JSONCompilationDatabase.cpp
+++ lib/Tooling/JSONCompilationDatabase.cpp
@@ -190,8 +190,11 @@
 JSONCompilationDatabase::loadFromFile(StringRef FilePath,
   std::string &ErrorMessage,
   JSONCommandLineSyntax Syntax) {
+  // Don't mmap: if we're a long-lived process, the build system may overwrite.
   llvm::ErrorOr> DatabaseBuffer =
-  llvm::MemoryBuffer::getFile(FilePath);
+  llvm::MemoryBuffer::getFile(FilePath, /*FileSize=*/-1,
+  /*RequiresNullTerminator=*/true,
+  /*IsVolatile=*/true);
   if (std::error_code Result = DatabaseBuffer.getError()) {
 ErrorMessage = "Error while opening JSON database: " + Result.message();
 return nullptr;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61601: [clangd] Represent Hover result using FormattedString

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D61601#1492979 , @kadircet wrote:

> I think it makes more sense to expose semantical information in 
> HoverInfo(like Name, Scope, Definition, etc), rather than formatted strings, 
> and let this be serialized by the users of ClangdServer (as in D61497 
> ). This is what I'll perform with that patch 
> anyway, since it aims to provide consumers of ClangdServer with sementical 
> information. So it is up to you whether to change the layering or keep it as 
> it is in this patch.


Having structured information instead of `FormattedString` looks good, this 
change mostly aims to add markdown support for resulting hover when the LSP 
clients have support for it.
I'm happy to restructure the `HoverInfo` to your liking if the current one 
seems problematic for any reason.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61601



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

[PATCH] D61630: [clangd] Oops, switchSourceHeader still needs to return a URI.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE360121: [clangd] Oops, switchSourceHeader still needs to 
return a URI. (authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61630?vs=198407&id=198409#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61630

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h


Index: clangd/ClangdLSPServer.h
===
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -85,7 +85,7 @@
 Callback>);
   void onReference(const ReferenceParams &, Callback>);
   void onSwitchSourceHeader(const TextDocumentIdentifier &,
-Callback>);
+Callback>);
   void onDocumentHighlight(const TextDocumentPositionParams &,
Callback>);
   void onFileEvent(const DidChangeWatchedFilesParams &);
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -825,9 +825,9 @@
 
 void ClangdLSPServer::onSwitchSourceHeader(
 const TextDocumentIdentifier &Params,
-Callback> Reply) {
+Callback> Reply) {
   if (auto Result = Server->switchSourceHeader(Params.uri.file()))
-Reply(URI::resolvePath(*Result, Params.uri.file()));
+Reply(URIForFile::canonicalize(*Result, Params.uri.file()));
   else
 Reply(llvm::None);
 }


Index: clangd/ClangdLSPServer.h
===
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -85,7 +85,7 @@
 Callback>);
   void onReference(const ReferenceParams &, Callback>);
   void onSwitchSourceHeader(const TextDocumentIdentifier &,
-Callback>);
+Callback>);
   void onDocumentHighlight(const TextDocumentPositionParams &,
Callback>);
   void onFileEvent(const DidChangeWatchedFilesParams &);
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -825,9 +825,9 @@
 
 void ClangdLSPServer::onSwitchSourceHeader(
 const TextDocumentIdentifier &Params,
-Callback> Reply) {
+Callback> Reply) {
   if (auto Result = Server->switchSourceHeader(Params.uri.file()))
-Reply(URI::resolvePath(*Result, Params.uri.file()));
+Reply(URIForFile::canonicalize(*Result, Params.uri.file()));
   else
 Reply(llvm::None);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r360121 - [clangd] Oops, switchSourceHeader still needs to return a URI.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Tue May  7 01:30:32 2019
New Revision: 360121

URL: http://llvm.org/viewvc/llvm-project?rev=360121&view=rev
Log:
[clangd] Oops, switchSourceHeader still needs to return a URI.

Reviewers: kadircet

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=360121&r1=360120&r2=360121&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Tue May  7 01:30:32 2019
@@ -825,9 +825,9 @@ void ClangdLSPServer::onGoToDeclaration(
 
 void ClangdLSPServer::onSwitchSourceHeader(
 const TextDocumentIdentifier &Params,
-Callback> Reply) {
+Callback> Reply) {
   if (auto Result = Server->switchSourceHeader(Params.uri.file()))
-Reply(URI::resolvePath(*Result, Params.uri.file()));
+Reply(URIForFile::canonicalize(*Result, Params.uri.file()));
   else
 Reply(llvm::None);
 }

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=360121&r1=360120&r2=360121&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Tue May  7 01:30:32 2019
@@ -85,7 +85,7 @@ private:
 Callback>);
   void onReference(const ReferenceParams &, Callback>);
   void onSwitchSourceHeader(const TextDocumentIdentifier &,
-Callback>);
+Callback>);
   void onDocumentHighlight(const TextDocumentPositionParams &,
Callback>);
   void onFileEvent(const DidChangeWatchedFilesParams &);


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

[PATCH] D61628: Fix a bug that reports UTF16 (LE) files as UTF32 (LE) ones

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This looks good, but did you intend to make a new diff? It looks like D61559 
 didn't land yet.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61628



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

[PATCH] D61601: [clangd] Represent Hover result using FormattedString

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Move range and kind assignment out of the loop.
- Log error on failed deserialization of markup kind.
- s/Plaintext/PlainText


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61601

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/FormattedString.cpp
  clang-tools-extra/clangd/FormattedString.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -580,7 +580,10 @@
   int test1 = bonjour;
 }
   )cpp",
-  "Declared in function main\n\nint bonjour",
+  "text[Declared in]code[function main]\n"
+  "codeblock(cpp) [\n"
+  "int bonjour\n"
+  "]",
   },
   {
   R"cpp(// Local variable in method
@@ -591,7 +594,10 @@
   }
 };
   )cpp",
-  "Declared in function s::method\n\nint bonjour",
+  "text[Declared in]code[function s::method]\n"
+  "codeblock(cpp) [\n"
+  "int bonjour\n"
+  "]",
   },
   {
   R"cpp(// Struct
@@ -602,7 +608,10 @@
   ns1::My^Class* Params;
 }
   )cpp",
-  "Declared in namespace ns1\n\nstruct MyClass {}",
+  "text[Declared in]code[namespace ns1]\n"
+  "codeblock(cpp) [\n"
+  "struct MyClass {}\n"
+  "]",
   },
   {
   R"cpp(// Class
@@ -613,7 +622,10 @@
   ns1::My^Class* Params;
 }
   )cpp",
-  "Declared in namespace ns1\n\nclass MyClass {}",
+  "text[Declared in]code[namespace ns1]\n"
+  "codeblock(cpp) [\n"
+  "class MyClass {}\n"
+  "]",
   },
   {
   R"cpp(// Union
@@ -624,7 +636,10 @@
   ns1::My^Union Params;
 }
   )cpp",
-  "Declared in namespace ns1\n\nunion MyUnion {}",
+  "text[Declared in]code[namespace ns1]\n"
+  "codeblock(cpp) [\n"
+  "union MyUnion {}\n"
+  "]",
   },
   {
   R"cpp(// Function definition via pointer
@@ -633,7 +648,10 @@
   auto *X = &^foo;
 }
   )cpp",
-  "Declared in global namespace\n\nint foo(int)",
+  "text[Declared in]code[global namespace]\n"
+  "codeblock(cpp) [\n"
+  "int foo(int)\n"
+  "]",
   },
   {
   R"cpp(// Function declaration via call
@@ -642,7 +660,10 @@
   return ^foo(42);
 }
   )cpp",
-  "Declared in global namespace\n\nint foo(int)",
+  "text[Declared in]code[global namespace]\n"
+  "codeblock(cpp) [\n"
+  "int foo(int)\n"
+  "]",
   },
   {
   R"cpp(// Field
@@ -652,7 +673,10 @@
   bar.^x;
 }
   )cpp",
-  "Declared in struct Foo\n\nint x",
+  "text[Declared in]code[struct Foo]\n"
+  "codeblock(cpp) [\n"
+  "int x\n"
+  "]",
   },
   {
   R"cpp(// Field with initialization
@@ -662,7 +686,10 @@
   bar.^x;
 }
   )cpp",
-  "Declared in struct Foo\n\nint x = 5",
+  "text[Declared in]code[struct Foo]\n"
+  "codeblock(cpp) [\n"
+  "int x = 5\n"
+  "]",
   },
   {
   R"cpp(// Static field
@@ -671,7 +698,10 @@
   Foo::^x;
 }
   )cpp",
-  "Declared in struct Foo\n\nstatic int x",
+  "text[Declared in]code[struct Foo]\n"
+  "codeblock(cpp) [\n"
+  "static int x\n"
+  "]",
   },
   {
   R"cpp(// Field, member initializer
@@ -680,7 +710,10 @@
   Foo() : ^x(0) {}
 };
   )cpp",
-  "Declared in struct Foo\n\nint x",
+  "text[Declared in]code[struct Foo]\n"
+  "codeblock(cpp) [\n"
+  "int x\n"
+  "]",
   },
   {
   R"cpp(// Field, GNU old-style field designator
@@ -689,7 +722,10 @@
   Foo bar = { ^x : 1 };
 }
   )cpp",
-  "Declared in struct Foo\n\nint x",
+  "text[Declared in]code[struct Foo]\n"
+  "codeblock(cpp) [\n"
+  "int x\n"
+  "]",
   },
   {
   R"cpp(// Field, field designator
@@ -698,7 +734,10 @@
 

[PATCH] D61497: [clangd] Introduce a structured hover response

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Was passing by, just a small clarifying question...




Comment at: clang-tools-extra/clangd/XRefs.h:73
+  llvm::Optional> Parameters;
+  llvm::Optional> TemplateParameters;
+

What does `Type` mean for non-type and template template parameters?
Could you add a comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61497



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

[PATCH] D61601: [clangd] Represent Hover result using FormattedString

[Ilya Biryukov via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:853
+  Hover R;
+  switch (HoverContentFormat) {
+  case MarkupKind::Plaintext:

kadircet wrote:
> Why not move `R.contents.kind` and `R.range` assignment out of the switch 
> statement. and perform return after the switch. That way you can get rid of 
> the `llvm_unreachable`(we would still get warnings for that switch-statement 
> if someone adds a new `MarkupKind` but doesn't update that statement.
> 
> And hopefully we should fallback to `PlainText` if editor doesn't support any 
> markupkinds known to us.
Done, we do want `llvm_unreachable` at the end, just in case a new `MarkupKind` 
gets added.

> And hopefully we should fallback to PlainText if editor doesn't support any 
> markupkinds known to us.
This is handled by the code parsing initialization options and it actually 
falls back to plaintext.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61601



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

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

[Tyker via Phabricator via cfe-commits]

Tyker added a comment.

could you commit this for me ?


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

https://reviews.llvm.org/D60934



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

[PATCH] D61631: [Tooling] Don't mmap the JSONCompilationDatabase, it's not correct for long-lived processes.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC360122: [Tooling] Don't mmap the 
JSONCompilationDatabase, it's not correct for long… (authored by 
sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61631?vs=198408&id=198414#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D61631

Files:
  lib/Tooling/JSONCompilationDatabase.cpp


Index: lib/Tooling/JSONCompilationDatabase.cpp
===
--- lib/Tooling/JSONCompilationDatabase.cpp
+++ lib/Tooling/JSONCompilationDatabase.cpp
@@ -190,8 +190,11 @@
 JSONCompilationDatabase::loadFromFile(StringRef FilePath,
   std::string &ErrorMessage,
   JSONCommandLineSyntax Syntax) {
+  // Don't mmap: if we're a long-lived process, the build system may overwrite.
   llvm::ErrorOr> DatabaseBuffer =
-  llvm::MemoryBuffer::getFile(FilePath);
+  llvm::MemoryBuffer::getFile(FilePath, /*FileSize=*/-1,
+  /*RequiresNullTerminator=*/true,
+  /*IsVolatile=*/true);
   if (std::error_code Result = DatabaseBuffer.getError()) {
 ErrorMessage = "Error while opening JSON database: " + Result.message();
 return nullptr;


Index: lib/Tooling/JSONCompilationDatabase.cpp
===
--- lib/Tooling/JSONCompilationDatabase.cpp
+++ lib/Tooling/JSONCompilationDatabase.cpp
@@ -190,8 +190,11 @@
 JSONCompilationDatabase::loadFromFile(StringRef FilePath,
   std::string &ErrorMessage,
   JSONCommandLineSyntax Syntax) {
+  // Don't mmap: if we're a long-lived process, the build system may overwrite.
   llvm::ErrorOr> DatabaseBuffer =
-  llvm::MemoryBuffer::getFile(FilePath);
+  llvm::MemoryBuffer::getFile(FilePath, /*FileSize=*/-1,
+  /*RequiresNullTerminator=*/true,
+  /*IsVolatile=*/true);
   if (std::error_code Result = DatabaseBuffer.getError()) {
 ErrorMessage = "Error while opening JSON database: " + Result.message();
 return nullptr;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61633: [Tooling] Add -x flags when inferring compile commands for files with no/invalid extension.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We treat them as headers, as the motivating case is C++ standard library.


Repository:
  rC Clang

https://reviews.llvm.org/D61633

Files:
  lib/Tooling/InterpolatingCompilationDatabase.cpp
  unittests/Tooling/CompilationDatabaseTest.cpp


Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -723,14 +723,17 @@
   // .h is ambiguous, so we add explicit language flags
   EXPECT_EQ(getCommand("foo.h"),
 "clang -D dir/foo.cpp -x c++-header -std=c++17");
+  // Same thing if we have no extension. (again, we treat as header).
+  EXPECT_EQ(getCommand("foo"), "clang -D dir/foo.cpp -x c++-header 
-std=c++17");
+  // and invalid extensions.
+  EXPECT_EQ(getCommand("foo.cce"),
+"clang -D dir/foo.cpp -x c++-header -std=c++17");
   // and don't add -x if the inferred language is correct.
   EXPECT_EQ(getCommand("foo.hpp"), "clang -D dir/foo.cpp -std=c++17");
   // respect -x if it's already there.
   EXPECT_EQ(getCommand("baz.h"), "clang -D dir/baz.cee -x c-header");
   // prefer a worse match with the right extension.
   EXPECT_EQ(getCommand("foo.c"), "clang -D dir/bar.c");
-  // make sure we don't crash on queries with invalid extensions.
-  EXPECT_EQ(getCommand("foo.cce"), "clang -D dir/foo.cpp");
   Entries.erase(path(StringRef("dir/bar.c")));
   // Now we transfer across languages, so drop -std too.
   EXPECT_EQ(getCommand("foo.c"), "clang -D dir/foo.cpp");
Index: lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -205,10 +205,13 @@
 bool TypeCertain;
 auto TargetType = guessType(Filename, &TypeCertain);
 // If the filename doesn't determine the language (.h), transfer with -x.
-if (TargetType != types::TY_INVALID && !TypeCertain && Type) {
-  TargetType = types::onlyPrecompileType(TargetType) // header?
-   ? types::lookupHeaderTypeForSourceType(*Type)
-   : *Type;
+if ((!TargetType || !TypeCertain) && Type) {
+  // Use *Type, or its header variant if the file is a header.
+  // Treat no/invalid extension as header (e.g. C++ standard library).
+  TargetType =
+  (!TargetType || types::onlyPrecompileType(TargetType)) // header?
+  ? types::lookupHeaderTypeForSourceType(*Type)
+  : *Type;
   if (ClangCLMode) {
 const StringRef Flag = toCLFlag(TargetType);
 if (!Flag.empty())


Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -723,14 +723,17 @@
   // .h is ambiguous, so we add explicit language flags
   EXPECT_EQ(getCommand("foo.h"),
 "clang -D dir/foo.cpp -x c++-header -std=c++17");
+  // Same thing if we have no extension. (again, we treat as header).
+  EXPECT_EQ(getCommand("foo"), "clang -D dir/foo.cpp -x c++-header -std=c++17");
+  // and invalid extensions.
+  EXPECT_EQ(getCommand("foo.cce"),
+"clang -D dir/foo.cpp -x c++-header -std=c++17");
   // and don't add -x if the inferred language is correct.
   EXPECT_EQ(getCommand("foo.hpp"), "clang -D dir/foo.cpp -std=c++17");
   // respect -x if it's already there.
   EXPECT_EQ(getCommand("baz.h"), "clang -D dir/baz.cee -x c-header");
   // prefer a worse match with the right extension.
   EXPECT_EQ(getCommand("foo.c"), "clang -D dir/bar.c");
-  // make sure we don't crash on queries with invalid extensions.
-  EXPECT_EQ(getCommand("foo.cce"), "clang -D dir/foo.cpp");
   Entries.erase(path(StringRef("dir/bar.c")));
   // Now we transfer across languages, so drop -std too.
   EXPECT_EQ(getCommand("foo.c"), "clang -D dir/foo.cpp");
Index: lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -205,10 +205,13 @@
 bool TypeCertain;
 auto TargetType = guessType(Filename, &TypeCertain);
 // If the filename doesn't determine the language (.h), transfer with -x.
-if (TargetType != types::TY_INVALID && !TypeCertain && Type) {
-  TargetType = types::onlyPrecompileType(TargetType) // header?
-   ? types::lookupHeaderTypeForSourceType(*Type)
-   : *Type;
+if ((!TargetType || !TypeCertain) && Type) {
+  // Use *Type, or its header variant if the file is a header.
+  // Treat no/invalid extension as header (e.g. C++

r360122 - [Tooling] Don't mmap the JSONCompilationDatabase, it's not correct for long-lived processes.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Tue May  7 02:05:15 2019
New Revision: 360122

URL: http://llvm.org/viewvc/llvm-project?rev=360122&view=rev
Log:
[Tooling] Don't mmap the JSONCompilationDatabase, it's not correct for 
long-lived processes.

Reviewers: ilya-biryukov

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Tooling/JSONCompilationDatabase.cpp

Modified: cfe/trunk/lib/Tooling/JSONCompilationDatabase.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/JSONCompilationDatabase.cpp?rev=360122&r1=360121&r2=360122&view=diff
==
--- cfe/trunk/lib/Tooling/JSONCompilationDatabase.cpp (original)
+++ cfe/trunk/lib/Tooling/JSONCompilationDatabase.cpp Tue May  7 02:05:15 2019
@@ -190,8 +190,11 @@ std::unique_ptr
 JSONCompilationDatabase::loadFromFile(StringRef FilePath,
   std::string &ErrorMessage,
   JSONCommandLineSyntax Syntax) {
+  // Don't mmap: if we're a long-lived process, the build system may overwrite.
   llvm::ErrorOr> DatabaseBuffer =
-  llvm::MemoryBuffer::getFile(FilePath);
+  llvm::MemoryBuffer::getFile(FilePath, /*FileSize=*/-1,
+  /*RequiresNullTerminator=*/true,
+  /*IsVolatile=*/true);
   if (std::error_code Result = DatabaseBuffer.getError()) {
 ErrorMessage = "Error while opening JSON database: " + Result.message();
 return nullptr;


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

[PATCH] D61601: [clangd] Represent Hover result using FormattedString

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

> I'm happy to restructure the `HoverInfo` to your liking if the current one 
> seems problematic for any reason.

It will just cause more code move on my side, no problems apart from that




Comment at: clang-tools-extra/clangd/Protocol.cpp:707
+  else
+return false;
+  return true;

kadircet wrote:
> Maybe also vlog/elog the unknown kind? So that we can easily catch new 
> additions to spec.
I was referring to the `else` case down below :D


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61601



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

[PATCH] D61601: [clangd] Represent Hover result using FormattedString

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/Protocol.cpp:707
+  else
+return false;
+  return true;

kadircet wrote:
> kadircet wrote:
> > Maybe also vlog/elog the unknown kind? So that we can easily catch new 
> > additions to spec.
> I was referring to the `else` case down below :D
Thanks for catching that!
My mistake, I made the change but did not save the file.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61601



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

[PATCH] D61601: [clangd] Represent Hover result using FormattedString

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 198415.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.

- Forgotten change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61601

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/FormattedString.cpp
  clang-tools-extra/clangd/FormattedString.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -580,7 +580,10 @@
   int test1 = bonjour;
 }
   )cpp",
-  "Declared in function main\n\nint bonjour",
+  "text[Declared in]code[function main]\n"
+  "codeblock(cpp) [\n"
+  "int bonjour\n"
+  "]",
   },
   {
   R"cpp(// Local variable in method
@@ -591,7 +594,10 @@
   }
 };
   )cpp",
-  "Declared in function s::method\n\nint bonjour",
+  "text[Declared in]code[function s::method]\n"
+  "codeblock(cpp) [\n"
+  "int bonjour\n"
+  "]",
   },
   {
   R"cpp(// Struct
@@ -602,7 +608,10 @@
   ns1::My^Class* Params;
 }
   )cpp",
-  "Declared in namespace ns1\n\nstruct MyClass {}",
+  "text[Declared in]code[namespace ns1]\n"
+  "codeblock(cpp) [\n"
+  "struct MyClass {}\n"
+  "]",
   },
   {
   R"cpp(// Class
@@ -613,7 +622,10 @@
   ns1::My^Class* Params;
 }
   )cpp",
-  "Declared in namespace ns1\n\nclass MyClass {}",
+  "text[Declared in]code[namespace ns1]\n"
+  "codeblock(cpp) [\n"
+  "class MyClass {}\n"
+  "]",
   },
   {
   R"cpp(// Union
@@ -624,7 +636,10 @@
   ns1::My^Union Params;
 }
   )cpp",
-  "Declared in namespace ns1\n\nunion MyUnion {}",
+  "text[Declared in]code[namespace ns1]\n"
+  "codeblock(cpp) [\n"
+  "union MyUnion {}\n"
+  "]",
   },
   {
   R"cpp(// Function definition via pointer
@@ -633,7 +648,10 @@
   auto *X = &^foo;
 }
   )cpp",
-  "Declared in global namespace\n\nint foo(int)",
+  "text[Declared in]code[global namespace]\n"
+  "codeblock(cpp) [\n"
+  "int foo(int)\n"
+  "]",
   },
   {
   R"cpp(// Function declaration via call
@@ -642,7 +660,10 @@
   return ^foo(42);
 }
   )cpp",
-  "Declared in global namespace\n\nint foo(int)",
+  "text[Declared in]code[global namespace]\n"
+  "codeblock(cpp) [\n"
+  "int foo(int)\n"
+  "]",
   },
   {
   R"cpp(// Field
@@ -652,7 +673,10 @@
   bar.^x;
 }
   )cpp",
-  "Declared in struct Foo\n\nint x",
+  "text[Declared in]code[struct Foo]\n"
+  "codeblock(cpp) [\n"
+  "int x\n"
+  "]",
   },
   {
   R"cpp(// Field with initialization
@@ -662,7 +686,10 @@
   bar.^x;
 }
   )cpp",
-  "Declared in struct Foo\n\nint x = 5",
+  "text[Declared in]code[struct Foo]\n"
+  "codeblock(cpp) [\n"
+  "int x = 5\n"
+  "]",
   },
   {
   R"cpp(// Static field
@@ -671,7 +698,10 @@
   Foo::^x;
 }
   )cpp",
-  "Declared in struct Foo\n\nstatic int x",
+  "text[Declared in]code[struct Foo]\n"
+  "codeblock(cpp) [\n"
+  "static int x\n"
+  "]",
   },
   {
   R"cpp(// Field, member initializer
@@ -680,7 +710,10 @@
   Foo() : ^x(0) {}
 };
   )cpp",
-  "Declared in struct Foo\n\nint x",
+  "text[Declared in]code[struct Foo]\n"
+  "codeblock(cpp) [\n"
+  "int x\n"
+  "]",
   },
   {
   R"cpp(// Field, GNU old-style field designator
@@ -689,7 +722,10 @@
   Foo bar = { ^x : 1 };
 }
   )cpp",
-  "Declared in struct Foo\n\nint x",
+  "text[Declared in]code[struct Foo]\n"
+  "codeblock(cpp) [\n"
+  "int x\n"
+  "]",
   },
   {
   R"cpp(// Field, field designator
@@ -698,7 +734,10 @@
   Foo bar = { .^x = 2 };
 }
   )cpp",
-  

[PATCH] D61601: [clangd] Represent Hover result using FormattedString

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov planned changes to this revision.
ilya-biryukov added a comment.

Planning to rebase this on top of D61497  and 
land afterwards in order to minimize merge conflicts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61601



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

[PATCH] D61497: [clangd] Introduce a structured hover response

[Kadir Cetinkaya via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/XRefs.h:73
+  llvm::Optional> Parameters;
+  llvm::Optional> TemplateParameters;
+

ilya-biryukov wrote:
> What does `Type` mean for non-type and template template parameters?
> Could you add a comment?
added comment for template template parameters.

For non-type template params, isn't it clear that this will hold the type of 
that param? e.g `template ... X>` -> `C...`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61497



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

[PATCH] D61497: [clangd] Introduce a structured hover response

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Add comments for non-type params


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61497

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

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1185,7 +1185,9 @@
 auto AST = TU.build();
 if (auto H = getHover(AST, T.point())) {
   EXPECT_NE("", Test.ExpectedHover) << Test.Input;
-  EXPECT_EQ(H->contents.value, Test.ExpectedHover.str()) << Test.Input;
+  EXPECT_EQ(H->render().contents.value, Test.ExpectedHover.str())
+  << Test.Input << '\n'
+  << H;
 } else
   EXPECT_EQ("", Test.ExpectedHover.str()) << Test.Input;
   }
Index: clang-tools-extra/clangd/XRefs.h
===
--- clang-tools-extra/clangd/XRefs.h
+++ clang-tools-extra/clangd/XRefs.h
@@ -16,7 +16,10 @@
 #include "ClangdUnit.h"
 #include "Protocol.h"
 #include "index/Index.h"
+#include "index/SymbolLocation.h"
+#include "clang/Index/IndexSymbol.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/Support/raw_ostream.h"
 #include 
 
 namespace clang {
@@ -46,8 +49,39 @@
 std::vector findDocumentHighlights(ParsedAST &AST,
   Position Pos);
 
+struct HoverInfo {
+  using Type = std::string;
+  struct Param {
+// For template template params it holds the whole template decl, e.g,
+// template class.
+Type T;
+std::string Name;
+std::string Default;
+  };
+
+  LocatedSymbol Sym;
+  /// Fully qualiffied name for the scope containing the Sym.
+  std::string Scope;
+  std::string ParentScope;
+  SymbolKind Kind;
+  std::string Documentation;
+  /// Line containing the definition of the symbol.
+  std::string Definition;
+
+  /// T and ReturnType are mutually exclusive.
+  llvm::Optional T;
+  llvm::Optional ReturnType;
+  llvm::Optional> Parameters;
+  llvm::Optional> TemplateParameters;
+
+  /// Lower to LSP struct.
+  Hover render() const;
+};
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, const HoverInfo::Param &);
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, const HoverInfo &);
+
 /// Get the hover information when hovering at \p Pos.
-llvm::Optional getHover(ParsedAST &AST, Position Pos);
+llvm::Optional getHover(ParsedAST &AST, Position Pos);
 
 /// Returns reference locations of the symbol at a specified \p Pos.
 /// \p Limit limits the number of results returned (0 means no limit).
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -7,21 +7,32 @@
 //===--===//
 #include "XRefs.h"
 #include "AST.h"
+#include "CodeCompletionStrings.h"
 #include "FindSymbols.h"
 #include "Logger.h"
+#include "Protocol.h"
 #include "SourceCode.h"
 #include "URI.h"
 #include "index/Merge.h"
 #include "index/SymbolCollector.h"
 #include "index/SymbolLocation.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Index/IndexDataConsumer.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Index/USRGeneration.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
 
 namespace clang {
 namespace clangd {
@@ -241,17 +252,17 @@
   return {DeclMacrosFinder.getFoundDecls(), DeclMacrosFinder.takeMacroInfos()};
 }
 
-Range getTokenRange(ParsedAST &AST, SourceLocation TokLoc) {
-  const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
-  SourceLocation LocEnd = Lexer::getLocForEndOfToken(
-  TokLoc, 0, SourceMgr, AST.getASTContext().getLangOpts());
+Range getTokenRange(ASTContext &AST, SourceLocation TokLoc) {
+  const SourceManager &SourceMgr = AST.getSourceManager();
+  SourceLocation LocEnd =
+  Lexer::getLocForEndOfToken(TokLoc, 0, SourceMgr, AST.getLangOpts());
   return {sourceLocToPosition(SourceMgr, TokLoc),
   sourceLocToPosition(SourceMgr, LocEnd)};
 }
 
-llvm::Optional makeLocation(ParsedAST &AST, Sou

[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet created this revision.
gchatelet added a reviewer: courbet.
Herald added subscribers: llvm-commits, cfe-commits, mgrang, hiraditya.
Herald added projects: clang, LLVM.

POC for the rfc http://lists.llvm.org/pipermail/llvm-dev/2019-April/131973.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61634

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/IRBuilder.cpp
  llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
  llvm/test/CodeGen/X86/memcpy.ll

Index: llvm/test/CodeGen/X86/memcpy.ll
===
--- llvm/test/CodeGen/X86/memcpy.ll
+++ llvm/test/CodeGen/X86/memcpy.ll
@@ -7,7 +7,7 @@
 
 
 ; Variable memcpy's should lower to calls.
-define i8* @test1(i8* %a, i8* %b, i64 %n) nounwind {
+define void @test1(i8* %a, i8* %b, i64 %n) nounwind {
 ; LINUX-LABEL: test1:
 ; LINUX:   # %bb.0: # %entry
 ; LINUX-NEXT:jmp memcpy # TAILCALL
@@ -17,11 +17,11 @@
 ; DARWIN-NEXT:jmp _memcpy ## TAILCALL
 entry:
 	tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %a, i8* %b, i64 %n, i1 0 )
-	ret i8* %a
+  ret void
 }
 
 ; Variable memcpy's should lower to calls.
-define i8* @test2(i64* %a, i64* %b, i64 %n) nounwind {
+define void @test2(i64* %a, i64* %b, i64 %n) nounwind {
 ; LINUX-LABEL: test2:
 ; LINUX:   # %bb.0: # %entry
 ; LINUX-NEXT:jmp memcpy # TAILCALL
@@ -33,7 +33,25 @@
 	%tmp14 = bitcast i64* %a to i8*
 	%tmp25 = bitcast i64* %b to i8*
 	tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %tmp14, i8* align 8 %tmp25, i64 %n, i1 0 )
-	ret i8* %tmp14
+  ret void
+}
+
+; Variable length memcpy's with disabled runtime should lower to repmovsb.
+define void @memcpy_no_runtime(i8* %a, i8* %b, i64 %n) nounwind {
+; LINUX-LABEL: memcpy_no_runtime:
+; LINUX:   # %bb.0: # %entry
+; LINUX-NEXT:movq %rdx, %rcx
+; LINUX-NEXT:rep;movsb (%rsi), %es:(%rdi)
+; LINUX-NEXT:retq
+;
+; DARWIN-LABEL: memcpy_no_runtime:
+; DARWIN:   ## %bb.0: ## %entry
+; DARWIN-NEXT:movq %rdx, %rcx
+; DARWIN-NEXT:rep;movsb (%rsi), %es:(%rdi)
+; DARWIN-NEXT:retq
+entry:
+	tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %a, i8* %b, i64 %n, i1 0 ) "no_runtime_for" = "memcpy"
+  ret void
 }
 
 ; Large constant memcpy's should lower to a call when optimizing for size.
Index: llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
===
--- llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
+++ llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
@@ -314,5 +314,9 @@
   Size.getValueType(), Align, isVolatile,
   AlwaysInline, DstPtrInfo, SrcPtrInfo);
 
+  /// Handle runtime sizes through repmovsb when we AlwaysInline.
+  if (AlwaysInline)
+return emitRepmovs(Subtarget, DAG, dl, Chain, Dst, Src, Size, MVT::i8);
+
   return SDValue();
 }
Index: llvm/lib/IR/IRBuilder.cpp
===
--- llvm/lib/IR/IRBuilder.cpp
+++ llvm/lib/IR/IRBuilder.cpp
@@ -96,6 +96,17 @@
   return II;
 }
 
+static void ForwardNoRuntimeAttribute(const Function *F,
+ StringRef FunctionName,
+ CallInst *CI) {
+  if (F->hasFnAttribute("no_runtime_for")) {
+const Attribute A = F->getFnAttribute("no_runtime_for");
+if (A.getValueAsString().contains(FunctionName)) {
+  CI->addAttribute(AttributeList::FunctionIndex, A);
+}
+  }
+}
+
 CallInst *IRBuilderBase::
 CreateMemSet(Value *Ptr, Value *Val, Value *Size, unsigned Align,
  bool isVolatile, MDNode *TBAATag, MDNode *ScopeTag,
@@ -103,7 +114,8 @@
   Ptr = getCastedInt8PtrValue(Ptr);
   Value *Ops[] = {Ptr, Val, Size, getInt1(isVolatile)};
   Type *Tys[] = { Ptr->getType(), Size->getType() };
-  Module *M = BB->getParent()->getParent();
+  Function *F = BB->getParent();
+  Module *M = F->getParent();
   Function *TheFn = Intrinsic::getDeclaration(M, Intrinsic::memset, Tys);
 
   CallInst *CI = createCallHelper(TheFn, Ops, this);
@@ -121,6 +133,8 @@
   if (NoAliasTag)
 CI->setMetadata(LLVMContext::MD_noalias, NoAliasTag);
 
+  ForwardNoRuntimeAttribute(F, "memset", CI);
+
   return CI;
 }
 
@@ -165,7 +179,8 @@
 
   Value *Ops[] = {Dst, Src, Size, getInt1(isVolatile)};
   Type *Tys[] = { Dst->getType(), Src->getType(), Size->getType() };
-  Module *M = BB->getParent()->getParent();
+  Function *F = BB->getParent();
+  Module *M = F->getParent();
   Function *TheFn = Intrinsic::getDeclaration(M, Intrinsic::memcpy, Tys);
 
   CallInst *CI = createCallHelper(TheFn, Ops, this);
@@ -190,6 +205,8 @@
   if (NoAliasTag)
 CI->setMetadata(LLVMContext::MD_noalias, NoAliasTag);
 
+  ForwardNoRuntimeAttribute(F, "memcpy", CI);
+
   return CI;
 }
 
@@ -245,7 +262,8 @@
 
   Value *Ops[] = {Dst, Src, Size, getInt1(isVolatile)};
   T

[PATCH] D61506: [OpenCL] Switch to C++17

[Anastasia Stulova via Phabricator via cfe-commits]

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

In D61506#1490555 , @rsmith wrote:

> Per the OpenCL C++ 1.0 specification, section 2:
>
> > The OpenCL C++ programming language is based on the ISO/IEC JTC1 SC22 WG21 
> > N 3690 language specification (a.k.a. C++14 specification).
>
> I think it would be reasonable to permit changing the base C++ standard in 
> OpenCL C++ mode, but we'd need a good reason to deviate from the behavior 
> specified in the OpenCL C++ specification by default.


I am in discussion with Khronos about changing the directions for this spec and 
I believe some announcements will appear hopefully soon.




Comment at: include/clang/Frontend/LangStandards.def:162
  OpenCL, "OpenCL C++ 1.0",
- LineComment | CPlusPlus | CPlusPlus11 | CPlusPlus14 | Digraphs | 
OpenCL)
+ LineComment | CPlusPlus | CPlusPlus11 | CPlusPlus14 | CPlusPlus17 
| Digraphs | OpenCL)
 

keryell wrote:
> kpet wrote:
> > Suggest you add `HexFloat` as well. It is part of c++17 and all OpenCL 
> > versions.
> Why only C++17?
> I would love to have `CPlusPlus2a` here too...
I would love to have `C++2a` too but it's currently an experimental feature 
only and I can't see any timeline about when this will become production 
quality. As we are planning to release C++ for OpenCL in production quality 
during this year I am afraid it won't be possible to include `C++2a` at this 
point .

Perhaps this is something we can do in the future.


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

https://reviews.llvm.org/D61506



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

[PATCH] D61637: [Syntax] Introduce syntax trees

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added a subscriber: mgorny.
Herald added a project: clang.
ilya-biryukov added a parent revision: D59887: [Syntax] Introduce TokenBuffer, 
start clangToolingSyntax library.

An tooling-focused alternative to the AST. This commit focuses on the
memory-management strategy and the structure of the AST.

More to follow later:

- Operations to mutate the syntax trees and corresponding textual replacements.
- Mapping between clang AST nodes and syntax tree nodes.
- More node types corresponding to the language constructs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61637

Files:
  clang/include/clang/Tooling/Syntax/Corpus.h
  clang/include/clang/Tooling/Syntax/Tree.h
  clang/include/clang/Tooling/Syntax/Tree/Cascade.h
  clang/include/clang/Tooling/Syntax/Tree/NodeKind.h
  clang/include/clang/Tooling/Syntax/Tree/NodeList.h
  clang/include/clang/Tooling/Syntax/Tree/Nodes.h
  clang/lib/Tooling/Syntax/BuildFromAST.cpp
  clang/lib/Tooling/Syntax/CMakeLists.txt
  clang/lib/Tooling/Syntax/Cascade.cpp
  clang/lib/Tooling/Syntax/Corpus.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/lib/Tooling/Syntax/Tree.cpp
  clang/tools/CMakeLists.txt
  clang/unittests/Tooling/Syntax/CMakeLists.txt
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -0,0 +1,155 @@
+//===- TreeTest.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/Tree.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/Decl.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Tooling/Syntax/Corpus.h"
+#include "clang/Tooling/Syntax/Tokens.h"
+#include "clang/Tooling/Syntax/Tree/Nodes.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace clang;
+
+namespace {
+class SyntaxTreeTest : public ::testing::Test {
+protected:
+  // Build a syntax tree for the code.
+  syntax::TranslationUnit *buildTree(llvm::StringRef Code) {
+// FIXME: this code is almost the identical to the one in TokensTest. Share
+//it.
+class BuildSyntaxTree : public ASTConsumer {
+public:
+  BuildSyntaxTree(syntax::TranslationUnit *&Root,
+  std::unique_ptr &Corpus,
+  std::unique_ptr Tokens)
+  : Root(Root), Corpus(Corpus), Tokens(std::move(Tokens)) {
+assert(this->Tokens);
+  }
+
+  void HandleTranslationUnit(ASTContext &Ctx) override {
+Corpus = llvm::make_unique(
+Ctx.getSourceManager(), Ctx.getLangOpts(),
+std::move(*Tokens).consume());
+Tokens = nullptr; // make sure we fail if this gets called twice.
+Root = syntax::buildSyntaxTree(*Corpus, *Ctx.getTranslationUnitDecl());
+  }
+
+private:
+  syntax::TranslationUnit *&Root;
+  std::unique_ptr &Corpus;
+  std::unique_ptr Tokens;
+};
+
+class BuildSyntaxTreeAction : public ASTFrontendAction {
+public:
+  BuildSyntaxTreeAction(syntax::TranslationUnit *&Root,
+std::unique_ptr &Corpus)
+  : Root(Root), Corpus(Corpus) {}
+
+  std::unique_ptr
+  CreateASTConsumer(CompilerInstance &CI, StringRef InFile) override {
+// We start recording the tokens, ast consumer will take on the result.
+auto Tokens =
+llvm::make_unique(CI.getPreprocessor());
+return llvm::make_unique(Root, Corpus,
+  std::move(Tokens));
+  }
+
+private:
+  syntax::TranslationUnit *&Root;
+  std::unique_ptr &Corpus;
+};
+
+constexpr const char *FileName = "./input.cpp";
+FS->addFile(FileName, time_t(), llvm::MemoryBuffer::getMemBufferCopy(""));
+// Prepare to run a compiler.
+std::vector Args = {"tok-test", "-std=c++03", "-fsyntax-only",
+  FileName};
+auto CI = createInvocationFromCommandLine(Args, Diags, FS);
+assert(CI);
+CI->getFrontendOpts().DisableFree = false;
+CI->getPreprocessorOpts().addRemappedFile(
+FileName, llvm::MemoryBuffer::getMemBufferCopy(Code).release());
+CompilerInstance Compiler;
+Compiler.setInvocation(std::move(CI));
+if (!Diags->getClient())
+  Diags->setClient(new IgnoringDiagConsumer);
+Com

r360132 - [ASTImporter] Import TemplateParameterLists in function templates.

[Balazs Keri via cfe-commits]

Author: balazske
Date: Tue May  7 03:55:11 2019
New Revision: 360132

URL: http://llvm.org/viewvc/llvm-project?rev=360132&view=rev
Log:
[ASTImporter] Import TemplateParameterLists in function templates.

Summary: Correct missing import of TemplateParameterList in function decl.

Reviewers: martong, a.sidorin, shafik

Reviewed By: martong

Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=360132&r1=360131&r2=360132&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Tue May  7 03:55:11 2019
@@ -419,6 +419,8 @@ namespace clang {
 Expected
 ImportFunctionTemplateWithTemplateArgsFromSpecialization(
 FunctionDecl *FromFD);
+Error ImportTemplateParameterLists(const DeclaratorDecl *FromD,
+   DeclaratorDecl *ToD);
 
 Error ImportTemplateInformation(FunctionDecl *FromFD, FunctionDecl *ToFD);
 
@@ -2780,6 +2782,22 @@ ExpectedDecl ASTNodeImporter::VisitEnumC
   return ToEnumerator;
 }
 
+Error ASTNodeImporter::ImportTemplateParameterLists(const DeclaratorDecl 
*FromD,
+DeclaratorDecl *ToD) {
+  unsigned int Num = FromD->getNumTemplateParameterLists();
+  if (Num == 0)
+return Error::success();
+  SmallVector ToTPLists(Num);
+  for (unsigned int I = 0; I < Num; ++I)
+if (Expected ToTPListOrErr =
+import(FromD->getTemplateParameterList(I)))
+  ToTPLists[I] = *ToTPListOrErr;
+else
+  return ToTPListOrErr.takeError();
+  ToD->setTemplateParameterListsInfo(Importer.ToContext, ToTPLists);
+  return Error::success();
+}
+
 Error ASTNodeImporter::ImportTemplateInformation(
 FunctionDecl *FromFD, FunctionDecl *ToFD) {
   switch (FromFD->getTemplatedKind()) {
@@ -2826,6 +2844,9 @@ Error ASTNodeImporter::ImportTemplateInf
 if (!POIOrErr)
   return POIOrErr.takeError();
 
+if (Error Err = ImportTemplateParameterLists(FromFD, ToFD))
+  return Err;
+
 TemplateSpecializationKind TSK = FTSInfo->getTemplateSpecializationKind();
 ToFD->setFunctionTemplateSpecialization(
 std::get<0>(*FunctionAndArgsOrErr), ToTAList, /* InsertPos= */ nullptr,

Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=360132&r1=360131&r2=360132&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Tue May  7 03:55:11 2019
@@ -5083,6 +5083,24 @@ TEST_P(ASTImporterOptionSpecificTestBase
   EXPECT_FALSE(ImportedD->getUnderlyingType()->isIncompleteType());
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportTemplateParameterLists) {
+  auto Code =
+  R"(
+  template
+  int f() { return 0; }
+  template <> int f() { return 4; }
+  )";
+
+  Decl *FromTU = getTuDecl(Code, Lang_CXX);
+  auto *FromD = FirstDeclMatcher().match(FromTU,
+  functionDecl(hasName("f"), isExplicitTemplateSpecialization()));
+  ASSERT_EQ(FromD->getNumTemplateParameterLists(), 1);
+
+  auto *ToD = Import(FromD, Lang_CXX);
+  // The template parameter list should exist.
+  EXPECT_EQ(ToD->getNumTemplateParameterLists(), 1);
+}
+
 struct ASTImporterLookupTableTest : ASTImporterOptionSpecificTestBase {};
 
 TEST_P(ASTImporterLookupTableTest, OneDecl) {


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

[PATCH] D61639: Add Triple::isSPIR() to simplify code

[Kévin Petit via Phabricator via cfe-commits]

kpet created this revision.
kpet added a reviewer: Anastasia.
Herald added subscribers: llvm-commits, cfe-commits, dexonsmith.
Herald added projects: clang, LLVM.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61639

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  llvm/include/llvm/ADT/Triple.h


Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -671,6 +671,11 @@
getEnvironment() == Triple::MuslEABIHF;
   }
 
+  /// Tests whether the target is SPIR (32- or 64-bit).
+  bool isSPIR() const {
+return getArch() == Triple::spir || getArch() == Triple::spir64;
+  }
+
   /// Tests whether the target is NVPTX (32- or 64-bit).
   bool isNVPTX() const {
 return getArch() == Triple::nvptx || getArch() == Triple::nvptx64;
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -1070,8 +1070,7 @@
 Builder.defineMacro(#Ext);
 #include "clang/Basic/OpenCLExtensions.def"
 
-auto Arch = TI.getTriple().getArch();
-if (Arch == llvm::Triple::spir || Arch == llvm::Triple::spir64)
+if (TI.getTriple().isSPIR())
   Builder.defineMacro("__IMAGE_SUPPORT__");
   }
 
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3349,10 +3349,8 @@
   Res.getFrontendOpts().ProgramAction);
 
   // Turn on -Wspir-compat for SPIR target.
-  auto Arch = T.getArch();
-  if (Arch == llvm::Triple::spir || Arch == llvm::Triple::spir64) {
+  if (T.isSPIR())
 Res.getDiagnosticOpts().Warnings.push_back("spir-compat");
-  }
 
   // If sanitizer is enabled, disable OPT_ffine_grained_bitfield_accesses.
   if (Res.getCodeGenOpts().FineGrainedBitfieldAccesses &&
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -539,8 +539,7 @@
   if (LangOpts.OpenCL) {
 EmitOpenCLMetadata();
 // Emit SPIR version.
-if (getTriple().getArch() == llvm::Triple::spir ||
-getTriple().getArch() == llvm::Triple::spir64) {
+if (getTriple().isSPIR()) {
   // SPIR v2.0 s2.12 - The SPIR version used by the module is stored in the
   // opencl.spir.version named metadata.
   llvm::Metadata *SPIRVerElts[] = {


Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -671,6 +671,11 @@
getEnvironment() == Triple::MuslEABIHF;
   }
 
+  /// Tests whether the target is SPIR (32- or 64-bit).
+  bool isSPIR() const {
+return getArch() == Triple::spir || getArch() == Triple::spir64;
+  }
+
   /// Tests whether the target is NVPTX (32- or 64-bit).
   bool isNVPTX() const {
 return getArch() == Triple::nvptx || getArch() == Triple::nvptx64;
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -1070,8 +1070,7 @@
 Builder.defineMacro(#Ext);
 #include "clang/Basic/OpenCLExtensions.def"
 
-auto Arch = TI.getTriple().getArch();
-if (Arch == llvm::Triple::spir || Arch == llvm::Triple::spir64)
+if (TI.getTriple().isSPIR())
   Builder.defineMacro("__IMAGE_SUPPORT__");
   }
 
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3349,10 +3349,8 @@
   Res.getFrontendOpts().ProgramAction);
 
   // Turn on -Wspir-compat for SPIR target.
-  auto Arch = T.getArch();
-  if (Arch == llvm::Triple::spir || Arch == llvm::Triple::spir64) {
+  if (T.isSPIR())
 Res.getDiagnosticOpts().Warnings.push_back("spir-compat");
-  }
 
   // If sanitizer is enabled, disable OPT_ffine_grained_bitfield_accesses.
   if (Res.getCodeGenOpts().FineGrainedBitfieldAccesses &&
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -539,8 +539,7 @@
   if (LangOpts.OpenCL) {
 EmitOpenCLMetadata();
 // Emit SPIR version.
-if (getTriple().getArch() == llvm::Triple::spir ||
-getTriple().getArch() == llvm::Triple::spir64) {
+if (getTriple().isSPIR()) {
   // SPIR v2.0 s2.12 - The SPIR version used by the 

[PATCH] D60461: [ASTImporter] Import TemplateParameterLists in function templates.

[Balázs Kéri via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC360132: [ASTImporter] Import TemplateParameterLists in 
function templates. (authored by balazske, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60461?vs=194310&id=198431#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D60461

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -419,6 +419,8 @@
 Expected
 ImportFunctionTemplateWithTemplateArgsFromSpecialization(
 FunctionDecl *FromFD);
+Error ImportTemplateParameterLists(const DeclaratorDecl *FromD,
+   DeclaratorDecl *ToD);
 
 Error ImportTemplateInformation(FunctionDecl *FromFD, FunctionDecl *ToFD);
 
@@ -2780,6 +2782,22 @@
   return ToEnumerator;
 }
 
+Error ASTNodeImporter::ImportTemplateParameterLists(const DeclaratorDecl 
*FromD,
+DeclaratorDecl *ToD) {
+  unsigned int Num = FromD->getNumTemplateParameterLists();
+  if (Num == 0)
+return Error::success();
+  SmallVector ToTPLists(Num);
+  for (unsigned int I = 0; I < Num; ++I)
+if (Expected ToTPListOrErr =
+import(FromD->getTemplateParameterList(I)))
+  ToTPLists[I] = *ToTPListOrErr;
+else
+  return ToTPListOrErr.takeError();
+  ToD->setTemplateParameterListsInfo(Importer.ToContext, ToTPLists);
+  return Error::success();
+}
+
 Error ASTNodeImporter::ImportTemplateInformation(
 FunctionDecl *FromFD, FunctionDecl *ToFD) {
   switch (FromFD->getTemplatedKind()) {
@@ -2826,6 +2844,9 @@
 if (!POIOrErr)
   return POIOrErr.takeError();
 
+if (Error Err = ImportTemplateParameterLists(FromFD, ToFD))
+  return Err;
+
 TemplateSpecializationKind TSK = FTSInfo->getTemplateSpecializationKind();
 ToFD->setFunctionTemplateSpecialization(
 std::get<0>(*FunctionAndArgsOrErr), ToTAList, /* InsertPos= */ nullptr,
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -5083,6 +5083,24 @@
   EXPECT_FALSE(ImportedD->getUnderlyingType()->isIncompleteType());
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportTemplateParameterLists) {
+  auto Code =
+  R"(
+  template
+  int f() { return 0; }
+  template <> int f() { return 4; }
+  )";
+
+  Decl *FromTU = getTuDecl(Code, Lang_CXX);
+  auto *FromD = FirstDeclMatcher().match(FromTU,
+  functionDecl(hasName("f"), isExplicitTemplateSpecialization()));
+  ASSERT_EQ(FromD->getNumTemplateParameterLists(), 1);
+
+  auto *ToD = Import(FromD, Lang_CXX);
+  // The template parameter list should exist.
+  EXPECT_EQ(ToD->getNumTemplateParameterLists(), 1);
+}
+
 struct ASTImporterLookupTableTest : ASTImporterOptionSpecificTestBase {};
 
 TEST_P(ASTImporterLookupTableTest, OneDecl) {


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -419,6 +419,8 @@
 Expected
 ImportFunctionTemplateWithTemplateArgsFromSpecialization(
 FunctionDecl *FromFD);
+Error ImportTemplateParameterLists(const DeclaratorDecl *FromD,
+   DeclaratorDecl *ToD);
 
 Error ImportTemplateInformation(FunctionDecl *FromFD, FunctionDecl *ToFD);
 
@@ -2780,6 +2782,22 @@
   return ToEnumerator;
 }
 
+Error ASTNodeImporter::ImportTemplateParameterLists(const DeclaratorDecl *FromD,
+DeclaratorDecl *ToD) {
+  unsigned int Num = FromD->getNumTemplateParameterLists();
+  if (Num == 0)
+return Error::success();
+  SmallVector ToTPLists(Num);
+  for (unsigned int I = 0; I < Num; ++I)
+if (Expected ToTPListOrErr =
+import(FromD->getTemplateParameterList(I)))
+  ToTPLists[I] = *ToTPListOrErr;
+else
+  return ToTPListOrErr.takeError();
+  ToD->setTemplateParameterListsInfo(Importer.ToContext, ToTPLists);
+  return Error::success();
+}
+
 Error ASTNodeImporter::ImportTemplateInformation(
 FunctionDecl *FromFD, FunctionDecl *ToFD) {
   switch (FromFD->getTemplatedKind()) {
@@ -2826,6 +2844,9 @@
 if (!POIOrErr)
   return POIOrErr.takeError();
 
+if (Error Err = ImportTemplateParameterLists(FromFD, ToFD))
+  return Err;
+
 TemplateSpecializationKind TSK = FTSInfo->getTemplateSpecializationKind();
 ToFD->setFunctionTemplateSpecialization(
 std::get<0>(*FunctionAndArgsOrErr), ToTAList, /* InsertPos= */ nullptr,
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unitte

[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet updated this revision to Diff 198436.
gchatelet added a comment.
Herald added a subscriber: jdoerfert.

- Add documentation.
- Fix permissive HasNoRuntimeAttribute


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61634

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/IRBuilder.cpp
  llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
  llvm/test/CodeGen/X86/memcpy.ll

Index: llvm/test/CodeGen/X86/memcpy.ll
===
--- llvm/test/CodeGen/X86/memcpy.ll
+++ llvm/test/CodeGen/X86/memcpy.ll
@@ -7,7 +7,7 @@
 
 
 ; Variable memcpy's should lower to calls.
-define i8* @test1(i8* %a, i8* %b, i64 %n) nounwind {
+define void @test1(i8* %a, i8* %b, i64 %n) nounwind {
 ; LINUX-LABEL: test1:
 ; LINUX:   # %bb.0: # %entry
 ; LINUX-NEXT:jmp memcpy # TAILCALL
@@ -17,11 +17,11 @@
 ; DARWIN-NEXT:jmp _memcpy ## TAILCALL
 entry:
 	tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %a, i8* %b, i64 %n, i1 0 )
-	ret i8* %a
+  ret void
 }
 
 ; Variable memcpy's should lower to calls.
-define i8* @test2(i64* %a, i64* %b, i64 %n) nounwind {
+define void @test2(i64* %a, i64* %b, i64 %n) nounwind {
 ; LINUX-LABEL: test2:
 ; LINUX:   # %bb.0: # %entry
 ; LINUX-NEXT:jmp memcpy # TAILCALL
@@ -33,7 +33,25 @@
 	%tmp14 = bitcast i64* %a to i8*
 	%tmp25 = bitcast i64* %b to i8*
 	tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %tmp14, i8* align 8 %tmp25, i64 %n, i1 0 )
-	ret i8* %tmp14
+  ret void
+}
+
+; Variable length memcpy's with disabled runtime should lower to repmovsb.
+define void @memcpy_no_runtime(i8* %a, i8* %b, i64 %n) nounwind {
+; LINUX-LABEL: memcpy_no_runtime:
+; LINUX:   # %bb.0: # %entry
+; LINUX-NEXT:movq %rdx, %rcx
+; LINUX-NEXT:rep;movsb (%rsi), %es:(%rdi)
+; LINUX-NEXT:retq
+;
+; DARWIN-LABEL: memcpy_no_runtime:
+; DARWIN:   ## %bb.0: ## %entry
+; DARWIN-NEXT:movq %rdx, %rcx
+; DARWIN-NEXT:rep;movsb (%rsi), %es:(%rdi)
+; DARWIN-NEXT:retq
+entry:
+	tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %a, i8* %b, i64 %n, i1 0 ) "no_runtime_for" = "memcpy"
+  ret void
 }
 
 ; Large constant memcpy's should lower to a call when optimizing for size.
Index: llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
===
--- llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
+++ llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
@@ -314,5 +314,9 @@
   Size.getValueType(), Align, isVolatile,
   AlwaysInline, DstPtrInfo, SrcPtrInfo);
 
+  /// Handle runtime sizes through repmovsb when we AlwaysInline.
+  if (AlwaysInline)
+return emitRepmovs(Subtarget, DAG, dl, Chain, Dst, Src, Size, MVT::i8);
+
   return SDValue();
 }
Index: llvm/lib/IR/IRBuilder.cpp
===
--- llvm/lib/IR/IRBuilder.cpp
+++ llvm/lib/IR/IRBuilder.cpp
@@ -96,6 +96,17 @@
   return II;
 }
 
+static void ForwardNoRuntimeAttribute(const Function *F,
+ StringRef FunctionName,
+ CallInst *CI) {
+  if (F->hasFnAttribute("no_runtime_for")) {
+const Attribute A = F->getFnAttribute("no_runtime_for");
+if (A.getValueAsString().contains(FunctionName)) {
+  CI->addAttribute(AttributeList::FunctionIndex, A);
+}
+  }
+}
+
 CallInst *IRBuilderBase::
 CreateMemSet(Value *Ptr, Value *Val, Value *Size, unsigned Align,
  bool isVolatile, MDNode *TBAATag, MDNode *ScopeTag,
@@ -103,7 +114,8 @@
   Ptr = getCastedInt8PtrValue(Ptr);
   Value *Ops[] = {Ptr, Val, Size, getInt1(isVolatile)};
   Type *Tys[] = { Ptr->getType(), Size->getType() };
-  Module *M = BB->getParent()->getParent();
+  Function *F = BB->getParent();
+  Module *M = F->getParent();
   Function *TheFn = Intrinsic::getDeclaration(M, Intrinsic::memset, Tys);
 
   CallInst *CI = createCallHelper(TheFn, Ops, this);
@@ -121,6 +133,8 @@
   if (NoAliasTag)
 CI->setMetadata(LLVMContext::MD_noalias, NoAliasTag);
 
+  ForwardNoRuntimeAttribute(F, "memset", CI);
+
   return CI;
 }
 
@@ -165,7 +179,8 @@
 
   Value *Ops[] = {Dst, Src, Size, getInt1(isVolatile)};
   Type *Tys[] = { Dst->getType(), Src->getType(), Size->getType() };
-  Module *M = BB->getParent()->getParent();
+  Function *F = BB->getParent();
+  Module *M = F->getParent();
   Function *TheFn = Intrinsic::getDeclaration(M, Intrinsic::memcpy, Tys);
 
   CallInst *CI = createCallHelper(TheFn, Ops, this);
@@ -190,6 +205,8 @@
   if (NoAliasTag)
 CI->setMetadata(LLVMContext::MD_noalias, NoAliasTag);
 
+  ForwardNoRuntimeAttribute(F, "memcpy", CI);
+
   return CI;
 }
 
@@ -245,7 +262,8 @@
 
   Value *Ops[] = {Dst, Src, Size, getIn

[PATCH] D61396: [hip] Fix ambiguity from `>>>` of CUDA.

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

@rsmith Do you have the chance to review this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61396



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

[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

I wonder if a list of comma-separated names is the right approach.  Would it 
make more sense to add a new attribute for each of the helpers, such as 
`#no-runtime-for-memcpy`? That should make querying simpler (one lookup in the 
table, rather than a lookup and a string scan) and also make it easier to add 
and remove attributes (merging is now just a matter of trying to add all of 
them, with the existing logic to deduplicate repeated attributes working).

We probably need to also carefully audit all of the transform passes to find 
ones that insert calls.  Last time I looked, there were four places in LLVM 
where memcpy could be expanded, I wonder if there are a similar number where it 
can be synthesised...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61634



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

[PATCH] D55562: Atomics: support min/max orthogonally

[Tim Northover via Phabricator via cfe-commits]

t.p.northover updated this revision to Diff 198452.
t.p.northover added a comment.

Sorry, I managed to forget about this one somehow. I hadn't changed the 32-bit 
requirement, but I agree it shouldn't be there so this diff removes it and adds 
tests for the newly legal cases.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55562

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/Expr.cpp
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/atomic-ops.c
  clang/test/Sema/atomic-ops.c
  clang/test/SemaOpenCL/atomic-ops.cl

Index: clang/test/SemaOpenCL/atomic-ops.cl
===
--- clang/test/SemaOpenCL/atomic-ops.cl
+++ clang/test/SemaOpenCL/atomic-ops.cl
@@ -73,7 +73,7 @@
   __opencl_atomic_fetch_add(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to atomic integer or pointer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
   __opencl_atomic_fetch_and(i, 1, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_and(p, 1, memory_order_seq_cst, memory_scope_work_group);
-  __opencl_atomic_fetch_and(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to bitwise atomic operation must be a pointer to atomic integer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
+  __opencl_atomic_fetch_and(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to atomic integer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
 
   __opencl_atomic_fetch_min(i, 1, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_max(i, 1, memory_order_seq_cst, memory_scope_work_group);
Index: clang/test/Sema/atomic-ops.c
===
--- clang/test/Sema/atomic-ops.c
+++ clang/test/Sema/atomic-ops.c
@@ -173,8 +173,8 @@
   __atomic_fetch_sub(P, 3, memory_order_seq_cst);
   __atomic_fetch_sub(D, 3, memory_order_seq_cst); // expected-error {{must be a pointer to integer or pointer}}
   __atomic_fetch_sub(s1, 3, memory_order_seq_cst); // expected-error {{must be a pointer to integer or pointer}}
-  __atomic_fetch_min(D, 3, memory_order_seq_cst); // expected-error {{must be a pointer to signed or unsigned 32-bit integer}}
-  __atomic_fetch_max(P, 3, memory_order_seq_cst); // expected-error {{must be a pointer to signed or unsigned 32-bit integer}}
+  __atomic_fetch_min(D, 3, memory_order_seq_cst); // expected-error {{must be a pointer to integer}}
+  __atomic_fetch_max(P, 3, memory_order_seq_cst); // expected-error {{must be a pointer to integer}}
   __atomic_fetch_max(p, 3);   // expected-error {{too few arguments to function call, expected 3, have 2}}
 
   __c11_atomic_fetch_and(i, 1, memory_order_seq_cst);
@@ -354,6 +354,20 @@
   (void)__c11_atomic_fetch_xor(Ap, val, memory_order_acq_rel);
   (void)__c11_atomic_fetch_xor(Ap, val, memory_order_seq_cst);
 
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_relaxed);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_acquire);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_consume);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_release);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_acq_rel);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_seq_cst);
+
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_relaxed);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_acquire);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_consume);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_release);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_acq_rel);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_seq_cst);
+
   (void)__c11_atomic_exchange(Ap, val, memory_order_relaxed);
   (void)__c11_atomic_exchange(Ap, val, memory_order_acquire);
   (void)__c11_atomic_exchange(Ap, val, memory_order_consume);
@@ -501,6 +515,20 @@
   (void)__atomic_nand_fetch(p, val, memory_order_acq_rel);
   (void)__atomic_nand_fetch(p, val, memory_order_seq_cst);
 
+  (void)__atomic_max_fetch(p, val, memory_order_relaxed);
+  (void)__atomic_max_fetch(p, val, memory_order_acquire);
+  (void)__atomic_max_fetch(p, val, memory_order_consume);
+  (void)__atomic_max_fetch(p, val, memory_order_release);
+  (void)__atomic_max_fetch(p, val, memory_order_acq_rel);
+  (void)__atomic_max_fetch(p, val, memory_order_seq_cst);
+
+  (void)__atomic_min_fetch(p, val, memory_order_relaxed);
+  (void)__atomic_min_fetch(p, val, memory_order_acquire);
+  (void)__atomic_min_fetch(p, val, memory_order_consume);
+  (void)__atomic_min_fetch(p, val, memory_order_rel

[PATCH] D61497: [clangd] Introduce a structured hover response

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.h:57
+// template class.
+Type T;
+std::string Name;

Same about type template parameters.
Probably just holds `class` or `typename`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61497



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

[PATCH] D61497: [clangd] Introduce a structured hover response

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.h:73
+  llvm::Optional> Parameters;
+  llvm::Optional> TemplateParameters;
+

kadircet wrote:
> ilya-biryukov wrote:
> > What does `Type` mean for non-type and template template parameters?
> > Could you add a comment?
> added comment for template template parameters.
> 
> For non-type template params, isn't it clear that this will hold the type of 
> that param? e.g `template ... X>` -> `C...`
Ah, sure, sorry, I meant the type parameters :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61497



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

[PATCH] D33841: [clang-tidy] redundant keyword check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: test/clang-tidy/readability-redundant-extern.cpp:37
+
+void another_file_scope(int _extern);

koldaniel wrote:
> aaron.ballman wrote:
> > koldaniel wrote:
> > > aaron.ballman wrote:
> > > > More tests that I figured out:
> > > > ```
> > > > namespace {
> > > > extern void f(); // 'extern' is not redundant
> > > > }
> > > > 
> > > > namespace a {
> > > > namespace {
> > > > namespace b {
> > > > extern void f(); // 'extern' is not redundant
> > > > }
> > > > }
> > > > }
> > > > 
> > > > // Note, the above are consequences of 
> > > > http://eel.is/c++draft/basic.link#6
> > > > 
> > > > #define FOO_EXTERN extern
> > > > typedef int extern_int;
> > > > 
> > > > extern_int FOO_EXTERN foo(); // 'extern' is redundant, but hopefully we 
> > > > don't try to fixit this to be '_int FOO_EXTERN foo();'
> > > > 
> > > > // The above is a weird consequence of how specifiers are parsed in C 
> > > > and C++
> > > > ```
> > > In the first two examples extern is redundant:
> > > 
> > > "An unnamed namespace or a namespace declared directly or indirectly 
> > > within an unnamed namespace has internal linkage. All other namespaces 
> > > have external linkage."
> > > 
> > > Also, based on the examples in http://eel.is/c++draft/basic.link#8 , the 
> > > extern keyword has no effect in case of unnamed namespaces. In case of 
> > > 'C' linkage defined by an extern keyword the checker does not warn.
> > > 
> > > In the first two examples extern is redundant:
> > 
> > The `extern` keyword there isn't redundant, it's useless. When I hear 
> > "redundant", I read it as "you can remove this keyword because the 
> > declaration is already `extern`" but that's not the case there.
> > 
> > You are correct that the declarations retain internal linkage -- that's why 
> > I think the `extern` keyword isn't redundant so much as it is confusing. We 
> > already issue a diagnostic for this in the frontend, so I think we may just 
> > want to silence the diagnostic here entirely. https://godbolt.org/z/WE6Fkd
> > 
> > WDYT?
> I see, you are right, calling it redundant is not the best way to describe 
> the situation. What I wanted to achieve here  is that I wanted this checker 
> to warn on every occurrence of the keyword extern when it seems to be useless 
> (redundant, unnecessary, etc.). If there are other checkers which warn in 
> situations like that (i.e. when extern has no effect), we could silence these 
> warnings (or we could handle this checker as a more generic one which warns 
> not on redundant but on unnecessary uses of extern).
I think it is okay to keep the check as-is but change the wording of the 
diagnostic slightly. Maybe `"unnecessary 'extern' keyword; %select{the 
declaration already has external linkage|the 'extern' specifier has no 
effect}0"` to really make it clear what's going on?


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

https://reviews.llvm.org/D33841



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

[PATCH] D61497: [clangd] Introduce a structured hover response

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

I will start adding test cases(and most likely change the way I populate the
struct at some places) if everyone is happy with current representation for
`HoverInfo`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61497

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

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1185,7 +1185,9 @@
 auto AST = TU.build();
 if (auto H = getHover(AST, T.point())) {
   EXPECT_NE("", Test.ExpectedHover) << Test.Input;
-  EXPECT_EQ(H->contents.value, Test.ExpectedHover.str()) << Test.Input;
+  EXPECT_EQ(H->render().contents.value, Test.ExpectedHover.str())
+  << Test.Input << '\n'
+  << H;
 } else
   EXPECT_EQ("", Test.ExpectedHover.str()) << Test.Input;
   }
Index: clang-tools-extra/clangd/XRefs.h
===
--- clang-tools-extra/clangd/XRefs.h
+++ clang-tools-extra/clangd/XRefs.h
@@ -16,7 +16,10 @@
 #include "ClangdUnit.h"
 #include "Protocol.h"
 #include "index/Index.h"
+#include "index/SymbolLocation.h"
+#include "clang/Index/IndexSymbol.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/Support/raw_ostream.h"
 #include 
 
 namespace clang {
@@ -46,8 +49,40 @@
 std::vector findDocumentHighlights(ParsedAST &AST,
   Position Pos);
 
+struct HoverInfo {
+  using Type = std::string;
+  struct Param {
+// For template template params it holds the whole template decl, e.g,
+// "template class".
+// For type-template params will contain "typename" or "class".
+Type T;
+std::string Name;
+std::string Default;
+  };
+
+  LocatedSymbol Sym;
+  /// Fully qualiffied name for the scope containing the Sym.
+  std::string Scope;
+  std::string ParentScope;
+  SymbolKind Kind;
+  std::string Documentation;
+  /// Line containing the definition of the symbol.
+  std::string Definition;
+
+  /// T and ReturnType are mutually exclusive.
+  llvm::Optional T;
+  llvm::Optional ReturnType;
+  llvm::Optional> Parameters;
+  llvm::Optional> TemplateParameters;
+
+  /// Lower to LSP struct.
+  Hover render() const;
+};
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, const HoverInfo::Param &);
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, const HoverInfo &);
+
 /// Get the hover information when hovering at \p Pos.
-llvm::Optional getHover(ParsedAST &AST, Position Pos);
+llvm::Optional getHover(ParsedAST &AST, Position Pos);
 
 /// Returns reference locations of the symbol at a specified \p Pos.
 /// \p Limit limits the number of results returned (0 means no limit).
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -7,21 +7,32 @@
 //===--===//
 #include "XRefs.h"
 #include "AST.h"
+#include "CodeCompletionStrings.h"
 #include "FindSymbols.h"
 #include "Logger.h"
+#include "Protocol.h"
 #include "SourceCode.h"
 #include "URI.h"
 #include "index/Merge.h"
 #include "index/SymbolCollector.h"
 #include "index/SymbolLocation.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Index/IndexDataConsumer.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Index/USRGeneration.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
 
 namespace clang {
 namespace clangd {
@@ -241,17 +252,17 @@
   return {DeclMacrosFinder.getFoundDecls(), DeclMacrosFinder.takeMacroInfos()};
 }
 
-Range getTokenRange(ParsedAST &AST, SourceLocation TokLoc) {
-  const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
-  SourceLocation LocEnd = Lexer::getLocForEndOfToken(
-  TokLoc, 0, SourceMgr, AST.getASTContext().getLangOpts());
+Range getTokenRange(ASTContext &AST, SourceLocation TokLoc) {
+  const SourceManager &SourceMgr = AST.getSourceManager();
+  SourceLocation LocEnd =
+  Lexer::getLocForEndOfTo

[PATCH] D61497: [clangd] Introduce a structured hover response

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.h:57
+// template class.
+Type T;
+std::string Name;

ilya-biryukov wrote:
> Same about type template parameters.
> Probably just holds `class` or `typename`?
Exactly, adding comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61497



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

[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/FormattedString.h:34
+  /// markdown.
+  /// EXPECTS: Code does not contain newlines.
+  void appendInlineCode(std::string Code);

newlines are allowed now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58547



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

[PATCH] D61350: [clang-tidy] New check calling out uses of +new in Objective-C code

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp:73
+  // of -init.
+  const StringRef Receiver =
+  getReceiverString(Expr->getReceiverRange(), SM, LangOpts);

We don't typically make local value types `const` (only pointers or 
references); here and elsewhere in the patch.



Comment at: clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp:76-77
+  // Some classes should use standard factory methods instead of alloc/init.
+  const std::map ClassToFactoryMethodMap = {
+  {"NSDate", "date"}, {"NSNull", "null"}};
+  const auto FoundClassFactory = ClassToFactoryMethodMap.find(Receiver);

Should this be configurable, or will users not need to control the behavior 
here?


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

https://reviews.llvm.org/D61350



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

r360147 - Allow field offset lookups in types with incomplete arrays within libclang.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue May  7 07:00:49 2019
New Revision: 360147

URL: http://llvm.org/viewvc/llvm-project?rev=360147&view=rev
Log:
Allow field offset lookups in types with incomplete arrays within libclang.

Patch thanks to Jorn Vernee

Added:
cfe/trunk/test/Index/print-type-size.c
Modified:
cfe/trunk/tools/libclang/CXType.cpp

Added: cfe/trunk/test/Index/print-type-size.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/print-type-size.c?rev=360147&view=auto
==
--- cfe/trunk/test/Index/print-type-size.c (added)
+++ cfe/trunk/test/Index/print-type-size.c Tue May  7 07:00:49 2019
@@ -0,0 +1,31 @@
+// RUN: c-index-test -test-print-type-size %s | FileCheck %s
+
+struct Foo {
+  int size;
+  // CHECK: FieldDecl=size:4:7 (Definition) [type=int] [typekind=Int] 
[sizeof=4] [alignof=4] [offsetof=0]
+  void *data[];
+  // CHECK: FieldDecl=data:6:9 (Definition) [type=void *[]] 
[typekind=IncompleteArray] [sizeof=-2] [alignof=8] [offsetof=64]
+};
+
+struct Bar {
+  int size;
+  // CHECK: FieldDecl=size:11:7 (Definition) [type=int] [typekind=Int] 
[sizeof=4] [alignof=4] [offsetof=0]
+  struct {
+int dummy;
+// CHECK: FieldDecl=dummy:14:9 (Definition) [type=int] [typekind=Int] 
[sizeof=4] [alignof=4] [offsetof=64/0]
+void *data[];
+// CHECK: FieldDecl=data:16:11 (Definition) [type=void *[]] 
[typekind=IncompleteArray] [sizeof=-2] [alignof=8] [offsetof=128/64]
+  };
+};
+
+struct Baz {
+  int size;
+  // CHECK: FieldDecl=size:22:7 (Definition) [type=int] [typekind=Int] 
[sizeof=4] [alignof=4] [offsetof=0]
+  union {
+void *data1[];
+// CHECK: FieldDecl=data1:25:11 (Definition) [type=void *[]] 
[typekind=IncompleteArray] [sizeof=-2] [alignof=8] [offsetof=64/0]
+void *data2[];
+// CHECK: FieldDecl=data2:27:11 (Definition) [type=void *[]] 
[typekind=IncompleteArray] [sizeof=-2] [alignof=8] [offsetof=64/0]
+  };
+};
+

Modified: cfe/trunk/tools/libclang/CXType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CXType.cpp?rev=360147&r1=360146&r2=360147&view=diff
==
--- cfe/trunk/tools/libclang/CXType.cpp (original)
+++ cfe/trunk/tools/libclang/CXType.cpp Tue May  7 07:00:49 2019
@@ -885,6 +885,10 @@ long long clang_getArraySize(CXType CT)
   return result;
 }
 
+static bool isIncompleteTypeWithAlignment(QualType QT) {
+  return QT->isIncompleteArrayType() || !QT->isIncompleteType();
+}
+
 long long clang_Type_getAlignOf(CXType T) {
   if (T.kind == CXType_Invalid)
 return CXTypeLayoutError_Invalid;
@@ -895,7 +899,7 @@ long long clang_Type_getAlignOf(CXType T
   // [expr.alignof] p3: if reference type, return size of referenced type
   if (QT->isReferenceType())
 QT = QT.getNonReferenceType();
-  if (QT->isIncompleteType())
+  if (!isIncompleteTypeWithAlignment(QT))
 return CXTypeLayoutError_Incomplete;
   if (QT->isDependentType())
 return CXTypeLayoutError_Dependent;
@@ -950,10 +954,14 @@ long long clang_Type_getSizeOf(CXType T)
   return Ctx.getTypeSizeInChars(QT).getQuantity();
 }
 
+static bool isTypeIncompleteForLayout(QualType QT) {
+  return QT->isIncompleteType() && !QT->isIncompleteArrayType();
+}
+
 static long long visitRecordForValidation(const RecordDecl *RD) {
   for (const auto *I : RD->fields()){
 QualType FQT = I->getType();
-if (FQT->isIncompleteType())
+if (isTypeIncompleteForLayout(FQT))
   return CXTypeLayoutError_Incomplete;
 if (FQT->isDependentType())
   return CXTypeLayoutError_Dependent;


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

[PATCH] D60956: [Sema] Fix the lookup for a declaration conflicting with an enumerator

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I found a few nits, but generally think this LG. However, I think @rsmith 
should sign off on it just in case I've misinterpreted something along the way.




Comment at: lib/Sema/SemaDecl.cpp:16537
+//   instead would cause us to miss using-declarations.
+// - it behaves as no declaration was found when the lookup result
+//   is not LookupResult::Found. This would cause us to miss

behaves as no -> behaves as if no



Comment at: lib/Sema/SemaDecl.cpp:16551
 
+llvm_unreachable("Unexpected LookupResultKind");
+  }

Missing a `default` label in front of this?



Comment at: lib/Sema/SemaDecl.cpp:16599
+  } else {
+
+if (isa(PrevDecl))

Spurious newline



Comment at: lib/Sema/SemaDecl.cpp:16602
+  Diag(IdLoc, diag::err_redefinition_of_enumerator) << Id;
+
+else

Spurious newline


Repository:
  rC Clang

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

https://reviews.llvm.org/D60956



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

[PATCH] D61239: [libclang] Allow field offset lookups in types with incomplete arrays.

[Aaron Ballman via Phabricator via cfe-commits]

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

I've commit in r360147, thank you for the patch!


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

https://reviews.llvm.org/D61239



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

[clang-tools-extra] r360151 - [clangd] Introduce intermediate representation of formatted text

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Tue May  7 07:18:18 2019
New Revision: 360151

URL: http://llvm.org/viewvc/llvm-project?rev=360151&view=rev
Log:
[clangd] Introduce intermediate representation of formatted text

Summary: That can render to markdown or plain text. Used for findHover requests.

Reviewers: malaperle, sammccall, kadircet

Reviewed By: sammccall

Subscribers: mgorny, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Added:
clang-tools-extra/trunk/clangd/FormattedString.cpp
clang-tools-extra/trunk/clangd/FormattedString.h
clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/unittests/CMakeLists.txt

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=360151&r1=360150&r2=360151&view=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Tue May  7 07:18:18 2019
@@ -50,6 +50,7 @@ add_clang_library(clangDaemon
   FileDistance.cpp
   FS.cpp
   FSProvider.cpp
+  FormattedString.cpp
   FuzzyMatch.cpp
   GlobalCompilationDatabase.cpp
   Headers.cpp

Added: clang-tools-extra/trunk/clangd/FormattedString.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FormattedString.cpp?rev=360151&view=auto
==
--- clang-tools-extra/trunk/clangd/FormattedString.cpp (added)
+++ clang-tools-extra/trunk/clangd/FormattedString.cpp Tue May  7 07:18:18 2019
@@ -0,0 +1,173 @@
+//===--- FormattedString.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 "FormattedString.h"
+#include "clang/Basic/CharInfo.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ErrorHandling.h"
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+
+namespace {
+/// Escape a markdown text block. Ensures the punctuation will not introduce
+/// any of the markdown constructs.
+static std::string renderText(llvm::StringRef Input) {
+  // Escaping ASCII punctiation ensures we can't start a markdown construct.
+  constexpr llvm::StringLiteral Punctuation =
+  R"txt(!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~)txt";
+
+  std::string R;
+  for (size_t From = 0; From < Input.size();) {
+size_t Next = Input.find_first_of(Punctuation, From);
+R += Input.substr(From, Next - From);
+if (Next == llvm::StringRef::npos)
+  break;
+R += "\\";
+R += Input[Next];
+
+From = Next + 1;
+  }
+  return R;
+}
+
+/// Renders \p Input as an inline block of code in markdown. The returned value
+/// is surrounded by backticks and the inner contents are properly escaped.
+static std::string renderInlineBlock(llvm::StringRef Input) {
+  std::string R;
+  // Double all backticks to make sure we don't close the inline block early.
+  for (size_t From = 0; From < Input.size();) {
+size_t Next = Input.find("`", From);
+R += Input.substr(From, Next - From);
+if (Next == llvm::StringRef::npos)
+  break;
+R += "``"; // double the found backtick.
+
+From = Next + 1;
+  }
+  // If results starts with a backtick, add spaces on both sides. The spaces
+  // are ignored by markdown renderers.
+  if (llvm::StringRef(R).startswith("`") || llvm::StringRef(R).endswith("`"))
+return "` " + std::move(R) + " `";
+  // Markdown render should ignore first and last space if both are there. We
+  // add an extra pair of spaces in that case to make sure we render what the
+  // user intended.
+  if (llvm::StringRef(R).startswith(" ") && llvm::StringRef(R).endswith(" "))
+return "` " + std::move(R) + " `";
+  return "`" + std::move(R) + "`";
+}
+/// Render \p Input as markdown code block with a specified \p Language. The
+/// result is surrounded by >= 3 backticks. Although markdown also allows to 
use
+/// '~' for code blocks, they are never used.
+static std::string renderCodeBlock(llvm::StringRef Input,
+   llvm::StringRef Language) {
+  // Count the maximum number of consecutive backticks in \p Input. We need to
+  // start and end the code block with more.
+  unsigned MaxBackticks = 0;
+  unsigned Backticks = 0;
+  for (char C : Input) {
+if (C == '`') {
+  ++Backticks;
+  continue;
+}
+MaxBackticks = std::max(MaxBackticks, Backticks);
+Backticks = 0;
+  }
+  MaxBackticks = std::max(Backticks, MaxBackticks);
+  // Use the corresponding number of backticks to st

[PATCH] D61497: [clangd] Introduce a structured hover response

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.h:53
+struct HoverInfo {
+  using Type = std::string;
+  struct Param {

NIT: maybe go with `std::string Type` at a use-site instead?
The scope of the name is large enough to make single-letter names a bit 
confusing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61497



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

[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 198463.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.

- Update a comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58547

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/FormattedString.cpp
  clang-tools-extra/clangd/FormattedString.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/FormattedStringTests.cpp

Index: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -0,0 +1,156 @@
+//===-- FormattedStringTests.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 "FormattedString.h"
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/StringRef.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(FormattedString, Basic) {
+  FormattedString S;
+  EXPECT_EQ(S.renderAsPlainText(), "");
+  EXPECT_EQ(S.renderAsMarkdown(), "");
+
+  S.appendText("foobar");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar");
+  EXPECT_EQ(S.renderAsMarkdown(), "foobar");
+
+  S = FormattedString();
+  S.appendInlineCode("foobar");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foobar`");
+
+  S = FormattedString();
+  S.appendCodeBlock("foobar");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar");
+  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
+  "foobar\n"
+  "```\n");
+}
+
+TEST(FormattedString, CodeBlocks) {
+  FormattedString S;
+  S.appendCodeBlock("foobar");
+  S.appendCodeBlock("bazqux", "javascript");
+
+  EXPECT_EQ(S.renderAsPlainText(), "foobar\n\n\nbazqux");
+  std::string ExpectedMarkdown = R"md(```cpp
+foobar
+```
+```javascript
+bazqux
+```
+)md";
+  EXPECT_EQ(S.renderAsMarkdown(), ExpectedMarkdown);
+
+  S = FormattedString();
+  S.appendInlineCode("foobar");
+  S.appendInlineCode("bazqux");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar bazqux");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foobar` `bazqux`");
+
+  S = FormattedString();
+  S.appendText("foo");
+  S.appendInlineCode("bar");
+  S.appendText("baz");
+
+  EXPECT_EQ(S.renderAsPlainText(), "foo bar baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo`bar`baz");
+}
+
+TEST(FormattedString, Escaping) {
+  // Check some ASCII punctuation
+  FormattedString S;
+  S.appendText("*!`");
+  EXPECT_EQ(S.renderAsMarkdown(), "\\*\\!\\`");
+
+  // Check all ASCII punctuation.
+  S = FormattedString();
+  std::string Punctuation = R"txt(!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~)txt";
+  // Same text, with each character escaped.
+  std::string EscapedPunctuation;
+  EscapedPunctuation.reserve(2 * Punctuation.size());
+  for (char C : Punctuation)
+EscapedPunctuation += std::string("\\") + C;
+  S.appendText(Punctuation);
+  EXPECT_EQ(S.renderAsMarkdown(), EscapedPunctuation);
+
+  // In code blocks we don't need to escape ASCII punctuation.
+  S = FormattedString();
+  S.appendInlineCode("* foo !+ bar * baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "`* foo !+ bar * baz`");
+  S = FormattedString();
+  S.appendCodeBlock("#define FOO\n* foo !+ bar * baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
+  "#define FOO\n* foo !+ bar * baz\n"
+  "```\n");
+
+  // But we have to escape the backticks.
+  S = FormattedString();
+  S.appendInlineCode("foo`bar`baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foo``bar``baz`");
+
+  S = FormattedString();
+  S.appendCodeBlock("foo`bar`baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
+  "foo`bar`baz\n"
+  "```\n");
+
+  // Inline code blocks starting or ending with backticks should add spaces.
+  S = FormattedString();
+  S.appendInlineCode("`foo");
+  EXPECT_EQ(S.renderAsMarkdown(), "` ``foo `");
+  S = FormattedString();
+  S.appendInlineCode("foo`");
+  EXPECT_EQ(S.renderAsMarkdown(), "` foo`` `");
+  S = FormattedString();
+  S.appendInlineCode("`foo`");
+  EXPECT_EQ(S.renderAsMarkdown(), "` ``foo`` `");
+
+  // Should also add extra spaces if the block stars and ends with spaces.
+  S = FormattedString();
+  S.appendInlineCode(" foo ");
+  EXPECT_EQ(S.renderAsMarkdown(), "`  foo  `");
+  S = FormattedString();
+  S.appendInlineCode("foo ");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foo `");
+  S = FormattedString();
+  S.appendInlineCode(" foo");
+  EXPECT_EQ(S.renderAsMarkdown(), "` foo`");
+
+

[PATCH] D58547: [clangd] Introduce intermediate representation of formatted text

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL360151: [clangd] Introduce intermediate representation of 
formatted text (authored by ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58547?vs=198463&id=198466#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58547

Files:
  clang-tools-extra/trunk/clangd/CMakeLists.txt
  clang-tools-extra/trunk/clangd/FormattedString.cpp
  clang-tools-extra/trunk/clangd/FormattedString.h
  clang-tools-extra/trunk/clangd/unittests/CMakeLists.txt
  clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp

Index: clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp
@@ -0,0 +1,156 @@
+//===-- FormattedStringTests.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 "FormattedString.h"
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/StringRef.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(FormattedString, Basic) {
+  FormattedString S;
+  EXPECT_EQ(S.renderAsPlainText(), "");
+  EXPECT_EQ(S.renderAsMarkdown(), "");
+
+  S.appendText("foobar");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar");
+  EXPECT_EQ(S.renderAsMarkdown(), "foobar");
+
+  S = FormattedString();
+  S.appendInlineCode("foobar");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foobar`");
+
+  S = FormattedString();
+  S.appendCodeBlock("foobar");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar");
+  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
+  "foobar\n"
+  "```\n");
+}
+
+TEST(FormattedString, CodeBlocks) {
+  FormattedString S;
+  S.appendCodeBlock("foobar");
+  S.appendCodeBlock("bazqux", "javascript");
+
+  EXPECT_EQ(S.renderAsPlainText(), "foobar\n\n\nbazqux");
+  std::string ExpectedMarkdown = R"md(```cpp
+foobar
+```
+```javascript
+bazqux
+```
+)md";
+  EXPECT_EQ(S.renderAsMarkdown(), ExpectedMarkdown);
+
+  S = FormattedString();
+  S.appendInlineCode("foobar");
+  S.appendInlineCode("bazqux");
+  EXPECT_EQ(S.renderAsPlainText(), "foobar bazqux");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foobar` `bazqux`");
+
+  S = FormattedString();
+  S.appendText("foo");
+  S.appendInlineCode("bar");
+  S.appendText("baz");
+
+  EXPECT_EQ(S.renderAsPlainText(), "foo bar baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo`bar`baz");
+}
+
+TEST(FormattedString, Escaping) {
+  // Check some ASCII punctuation
+  FormattedString S;
+  S.appendText("*!`");
+  EXPECT_EQ(S.renderAsMarkdown(), "\\*\\!\\`");
+
+  // Check all ASCII punctuation.
+  S = FormattedString();
+  std::string Punctuation = R"txt(!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~)txt";
+  // Same text, with each character escaped.
+  std::string EscapedPunctuation;
+  EscapedPunctuation.reserve(2 * Punctuation.size());
+  for (char C : Punctuation)
+EscapedPunctuation += std::string("\\") + C;
+  S.appendText(Punctuation);
+  EXPECT_EQ(S.renderAsMarkdown(), EscapedPunctuation);
+
+  // In code blocks we don't need to escape ASCII punctuation.
+  S = FormattedString();
+  S.appendInlineCode("* foo !+ bar * baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "`* foo !+ bar * baz`");
+  S = FormattedString();
+  S.appendCodeBlock("#define FOO\n* foo !+ bar * baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
+  "#define FOO\n* foo !+ bar * baz\n"
+  "```\n");
+
+  // But we have to escape the backticks.
+  S = FormattedString();
+  S.appendInlineCode("foo`bar`baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foo``bar``baz`");
+
+  S = FormattedString();
+  S.appendCodeBlock("foo`bar`baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
+  "foo`bar`baz\n"
+  "```\n");
+
+  // Inline code blocks starting or ending with backticks should add spaces.
+  S = FormattedString();
+  S.appendInlineCode("`foo");
+  EXPECT_EQ(S.renderAsMarkdown(), "` ``foo `");
+  S = FormattedString();
+  S.appendInlineCode("foo`");
+  EXPECT_EQ(S.renderAsMarkdown(), "` foo`` `");
+  S = FormattedString();
+  S.appendInlineCode("`foo`");
+  EXPECT_EQ(S.renderAsMarkdown(), "` ``foo`` `");
+
+  // Should also add extra spaces if the block stars and ends with spaces.
+  S = FormattedString()

r360152 - [OpenCL] Prevent mangling kernel functions.

[Anastasia Stulova via cfe-commits]

Author: stulova
Date: Tue May  7 07:22:34 2019
New Revision: 360152

URL: http://llvm.org/viewvc/llvm-project?rev=360152&view=rev
Log:
[OpenCL] Prevent mangling kernel functions.

Kernel function names have to be preserved as in the original
source to be able to access them from the host API side. 

This commit also adds restriction to kernels that prevents them
from being used in overloading, templates, etc.

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


Added:
cfe/trunk/test/SemaOpenCLCXX/kernel_invalid.cl
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/test/CodeGenOpenCLCXX/addrspace-of-this.cl
cfe/trunk/test/CodeGenOpenCLCXX/local_addrspace_init.cl

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=360152&r1=360151&r2=360152&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue May  7 07:22:34 
2019
@@ -8582,6 +8582,10 @@ def err_invalid_astype_of_different_size
   "invalid reinterpretation: sizes of %0 and %1 must match">;
 def err_static_kernel : Error<
   "kernel functions cannot be declared static">;
+def err_method_kernel : Error<
+  "kernel functions cannot be class members">;
+def err_template_kernel : Error<
+  "kernel functions cannot be used in a template declaration, instantiation or 
specialization">;
 def err_opencl_ptrptr_kernel_param : Error<
   "kernel parameter cannot be declared as a pointer to a pointer">;
 def err_kernel_arg_address_space : Error<

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=360152&r1=360151&r2=360152&view=diff
==
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Tue May  7 07:22:34 2019
@@ -2961,6 +2961,8 @@ bool FunctionDecl::isExternC() const {
 }
 
 bool FunctionDecl::isInExternCContext() const {
+  if (hasAttr())
+return true;
   return getLexicalDeclContext()->isExternCContext();
 }
 

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=360152&r1=360151&r2=360152&view=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue May  7 07:22:34 2019
@@ -9214,18 +9214,9 @@ Sema::ActOnFunctionDeclarator(Scope *S,
 
   MarkUnusedFileScopedDecl(NewFD);
 
-  if (getLangOpts().CPlusPlus) {
-if (FunctionTemplate) {
-  if (NewFD->isInvalidDecl())
-FunctionTemplate->setInvalidDecl();
-  return FunctionTemplate;
-}
 
-if (isMemberSpecialization && !NewFD->isInvalidDecl())
-  CompleteMemberSpecialization(NewFD, Previous);
-  }
 
-  if (NewFD->hasAttr()) {
+  if (getLangOpts().OpenCL && NewFD->hasAttr()) {
 // OpenCL v1.2 s6.8 static is invalid for kernel functions.
 if ((getLangOpts().OpenCLVersion >= 120)
 && (SC == SC_Static)) {
@@ -9245,7 +9236,30 @@ Sema::ActOnFunctionDeclarator(Scope *S,
 llvm::SmallPtrSet ValidTypes;
 for (auto Param : NewFD->parameters())
   checkIsValidOpenCLKernelParameter(*this, D, Param, ValidTypes);
+
+if (getLangOpts().OpenCLCPlusPlus) {
+  if (DC->isRecord()) {
+Diag(D.getIdentifierLoc(), diag::err_method_kernel);
+D.setInvalidType();
+  }
+  if (FunctionTemplate) {
+Diag(D.getIdentifierLoc(), diag::err_template_kernel);
+D.setInvalidType();
+  }
+}
   }
+
+  if (getLangOpts().CPlusPlus) {
+if (FunctionTemplate) {
+  if (NewFD->isInvalidDecl())
+FunctionTemplate->setInvalidDecl();
+  return FunctionTemplate;
+}
+
+if (isMemberSpecialization && !NewFD->isInvalidDecl())
+  CompleteMemberSpecialization(NewFD, Previous);
+  }
+
   for (const ParmVarDecl *Param : NewFD->parameters()) {
 QualType PT = Param->getType();
 

Modified: cfe/trunk/test/CodeGenOpenCLCXX/addrspace-of-this.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCLCXX/addrspace-of-this.cl?rev=360152&r1=360151&r2=360152&view=diff
==
--- cfe/trunk/test/CodeGenOpenCLCXX/addrspace-of-this.cl (original)
+++ cfe/trunk/test/CodeGenOpenCLCXX/addrspace-of-this.cl Tue May  7 07:22:34 
2019
@@ -83,7 +83,7 @@ __kernel void test__global() {
 // EXPL-LABEL: @__cxx_global_var_init()
 // EXPL: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast 
(%class.C addrspace(1)* @c to %class.C addrspace(4)*))
 
-// COMMON-LABEL: @_Z12test__globalv()
+// COMMON-LABEL: @test__global()
 
 // Test the address space of 'this' w

[PATCH] D60454: [OpenCL] Prevent mangling kernel functions

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC360152: [OpenCL] Prevent mangling kernel functions. 
(authored by stulova, committed by ).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D60454?vs=197934&id=198467#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D60454

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/Decl.cpp
  lib/Sema/SemaDecl.cpp
  test/CodeGenOpenCLCXX/addrspace-of-this.cl
  test/CodeGenOpenCLCXX/local_addrspace_init.cl
  test/SemaOpenCLCXX/kernel_invalid.cl

Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -2961,6 +2961,8 @@
 }
 
 bool FunctionDecl::isInExternCContext() const {
+  if (hasAttr())
+return true;
   return getLexicalDeclContext()->isExternCContext();
 }
 
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -9214,18 +9214,9 @@
 
   MarkUnusedFileScopedDecl(NewFD);
 
-  if (getLangOpts().CPlusPlus) {
-if (FunctionTemplate) {
-  if (NewFD->isInvalidDecl())
-FunctionTemplate->setInvalidDecl();
-  return FunctionTemplate;
-}
 
-if (isMemberSpecialization && !NewFD->isInvalidDecl())
-  CompleteMemberSpecialization(NewFD, Previous);
-  }
 
-  if (NewFD->hasAttr()) {
+  if (getLangOpts().OpenCL && NewFD->hasAttr()) {
 // OpenCL v1.2 s6.8 static is invalid for kernel functions.
 if ((getLangOpts().OpenCLVersion >= 120)
 && (SC == SC_Static)) {
@@ -9245,7 +9236,30 @@
 llvm::SmallPtrSet ValidTypes;
 for (auto Param : NewFD->parameters())
   checkIsValidOpenCLKernelParameter(*this, D, Param, ValidTypes);
+
+if (getLangOpts().OpenCLCPlusPlus) {
+  if (DC->isRecord()) {
+Diag(D.getIdentifierLoc(), diag::err_method_kernel);
+D.setInvalidType();
+  }
+  if (FunctionTemplate) {
+Diag(D.getIdentifierLoc(), diag::err_template_kernel);
+D.setInvalidType();
+  }
+}
+  }
+
+  if (getLangOpts().CPlusPlus) {
+if (FunctionTemplate) {
+  if (NewFD->isInvalidDecl())
+FunctionTemplate->setInvalidDecl();
+  return FunctionTemplate;
+}
+
+if (isMemberSpecialization && !NewFD->isInvalidDecl())
+  CompleteMemberSpecialization(NewFD, Previous);
   }
+
   for (const ParmVarDecl *Param : NewFD->parameters()) {
 QualType PT = Param->getType();
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -8582,6 +8582,10 @@
   "invalid reinterpretation: sizes of %0 and %1 must match">;
 def err_static_kernel : Error<
   "kernel functions cannot be declared static">;
+def err_method_kernel : Error<
+  "kernel functions cannot be class members">;
+def err_template_kernel : Error<
+  "kernel functions cannot be used in a template declaration, instantiation or specialization">;
 def err_opencl_ptrptr_kernel_param : Error<
   "kernel parameter cannot be declared as a pointer to a pointer">;
 def err_kernel_arg_address_space : Error<
Index: test/SemaOpenCLCXX/kernel_invalid.cl
===
--- test/SemaOpenCLCXX/kernel_invalid.cl
+++ test/SemaOpenCLCXX/kernel_invalid.cl
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -cl-std=c++ -pedantic -verify -fsyntax-only
+
+struct C {
+  kernel void m(); //expected-error{{kernel functions cannot be class members}}
+};
+
+template 
+kernel void templ(T par) { //expected-error{{kernel functions cannot be used in a template declaration, instantiation or specialization}}
+}
+
+template 
+kernel void bar(int par) { //expected-error{{kernel functions cannot be used in a template declaration, instantiation or specialization}}
+}
+
+kernel void foo(int); //expected-note{{previous declaration is here}}
+
+kernel void foo(float); //expected-error{{conflicting types for 'foo'}}
Index: test/CodeGenOpenCLCXX/addrspace-of-this.cl
===
--- test/CodeGenOpenCLCXX/addrspace-of-this.cl
+++ test/CodeGenOpenCLCXX/addrspace-of-this.cl
@@ -83,7 +83,7 @@
 // EXPL-LABEL: @__cxx_global_var_init()
 // EXPL: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast (%class.C addrspace(1)* @c to %class.C addrspace(4)*))
 
-// COMMON-LABEL: @_Z12test__globalv()
+// COMMON-LABEL: @test__global()
 
 // Test the address space of 'this' when invoking a method.
 // COMMON: call i32 @_ZNU3AS41C3getEv(%class.C addrspace(4)* addrspacecast (%class.C addrspace(1)* @c to %class.C addrspace(4)*))
@@ -152,19 +152,19 @@
 
 TEST(__local)
 
-// COMMON-LABEL: _Z11test__localv
+// COMMON-LABEL: @test__local
 
 // Test that we don't initialize an object in 

[PATCH] D61639: Add Triple::isSPIR() to simplify code

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61639



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

[PATCH] D61497: [clangd] Introduce a structured hover response

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

(I only got about halfway through the implementation - it's missing a lot of 
comments I think ;-))




Comment at: clang-tools-extra/clangd/XRefs.cpp:539
 
-/// Generate a \p Hover object given the declaration \p D.
-static Hover getHoverContents(const Decl *D) {
-  Hover H;
-  llvm::Optional NamedScope = getScopeName(D);
+static QualType getDeclType(const Decl *D) {
+  if (const TypedefNameDecl *TDD = dyn_cast(D))

I think this is basically ASTContext::getTypeDeclType()?



Comment at: clang-tools-extra/clangd/XRefs.cpp:549
 
-  // Generate the "Declared in" section.
-  if (NamedScope) {
-assert(!NamedScope->empty());
+static llvm::Optional getDeclLoc(const SourceLocation &SL,
+   ASTContext &AST) {

what does this have to do with decls?



Comment at: clang-tools-extra/clangd/XRefs.cpp:563
 
-H.contents.value += "Declared in ";
-H.contents.value += *NamedScope;
-H.contents.value += "\n\n";
+static std::string getDefinitionLine(const Decl *D) {
+  std::string Definition;

printDefinition?



Comment at: clang-tools-extra/clangd/XRefs.cpp:574
 
-  // We want to include the template in the Hover.
-  if (TemplateDecl *TD = D->getDescribedTemplate())
-D = TD;
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
+  const std::vector &Params) {

I don't think it's reasonable to define this private helper as an overload of 
operator<<.
Make it a function, or inline it? (I think used only once)



Comment at: clang-tools-extra/clangd/XRefs.cpp:1207
+  const HoverInfo::Param &P) {
+  OS << P.T << ' ' << P.Name;
+  if (!P.Default.empty())

avoid emitting the space if T/name are empty?



Comment at: clang-tools-extra/clangd/XRefs.cpp:1214
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const HoverInfo &HI) {
+  OS << HI.Definition << '\n';
+  OS << HI.Documentation << '\n';

This doesn't seem sufficiently self-documenting.



Comment at: clang-tools-extra/clangd/XRefs.h:52
+struct HoverInfo {
+  LocatedSymbol Sym;
+  /// Name of the context containing the symbol.

kadircet wrote:
> sammccall wrote:
> > I'm not sure about reuse of LocatedSymbol - do we want to commit to 
> > returning decl/def locations?
> > Name might be enough here.
> It might be nice to provide editors with enough info to jump to definition(it 
> was brought up during last meeting). But happy to reduce it to just name.
(this is not done I think - LocatedSymbol is still here)



Comment at: clang-tools-extra/clangd/XRefs.h:73
+  llvm::Optional> Parameters;
+  llvm::Optional> TemplateParameters;
+

ilya-biryukov wrote:
> kadircet wrote:
> > ilya-biryukov wrote:
> > > What does `Type` mean for non-type and template template parameters?
> > > Could you add a comment?
> > added comment for template template parameters.
> > 
> > For non-type template params, isn't it clear that this will hold the type 
> > of that param? e.g `template ... X>` -> `C...`
> Ah, sure, sorry, I meant the type parameters :-)
I get the curiosity, but this is too much text, and only covers template 
parameters which are not the most important case.

Can we just say something like `The pretty-printed parameter type, e.g. "int", 
or "typename" (in TemplateParameters)`?

We should do something sensible for template template parameters, but it's not 
important or non-obvious enough to document.



Comment at: clang-tools-extra/clangd/XRefs.h:52
 
+struct HoverInfo {
+  using Type = std::string;

This needs docs :-)



Comment at: clang-tools-extra/clangd/XRefs.h:54
+  using Type = std::string;
+  struct Param {
+// For template template params it holds the whole template decl, e.g,

one line doc for this struct?



Comment at: clang-tools-extra/clangd/XRefs.h:58
+// For type-template params will contain "typename" or "class".
+Type T;
+std::string Name;

needs a real name
mention the no-type case (macros)



Comment at: clang-tools-extra/clangd/XRefs.h:59
+Type T;
+std::string Name;
+std::string Default;

mention the unnamed case



Comment at: clang-tools-extra/clangd/XRefs.h:60
+std::string Name;
+std::string Default;
+  };

mention the no-default case



Comment at: clang-tools-extra/clangd/XRefs.h:64
+  LocatedSymbol Sym;
+  /// Fully qualiffied name for the scope containing the Sym.
+  std::string Scope;

qualiffied -> qualified



Comment at: clang-tools-extra/clangd/XRefs.h:65
+  /// Fully qualiffied name for the scope containing the 

r360153 - [Tooling] Add -x flags when inferring compile commands for files with no/invalid extension.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Tue May  7 07:34:06 2019
New Revision: 360153

URL: http://llvm.org/viewvc/llvm-project?rev=360153&view=rev
Log:
[Tooling] Add -x flags when inferring compile commands for files with 
no/invalid extension.

Summary: We treat them as headers, as the motivating case is C++ standard 
library.

Reviewers: kadircet

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp

Modified: cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp?rev=360153&r1=360152&r2=360153&view=diff
==
--- cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp (original)
+++ cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp Tue May  7 
07:34:06 2019
@@ -205,10 +205,13 @@ struct TransferableCommand {
 bool TypeCertain;
 auto TargetType = guessType(Filename, &TypeCertain);
 // If the filename doesn't determine the language (.h), transfer with -x.
-if (TargetType != types::TY_INVALID && !TypeCertain && Type) {
-  TargetType = types::onlyPrecompileType(TargetType) // header?
-   ? types::lookupHeaderTypeForSourceType(*Type)
-   : *Type;
+if ((!TargetType || !TypeCertain) && Type) {
+  // Use *Type, or its header variant if the file is a header.
+  // Treat no/invalid extension as header (e.g. C++ standard library).
+  TargetType =
+  (!TargetType || types::onlyPrecompileType(TargetType)) // header?
+  ? types::lookupHeaderTypeForSourceType(*Type)
+  : *Type;
   if (ClangCLMode) {
 const StringRef Flag = toCLFlag(TargetType);
 if (!Flag.empty())

Modified: cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp?rev=360153&r1=360152&r2=360153&view=diff
==
--- cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp Tue May  7 07:34:06 
2019
@@ -723,14 +723,17 @@ TEST_F(InterpolateTest, Language) {
   // .h is ambiguous, so we add explicit language flags
   EXPECT_EQ(getCommand("foo.h"),
 "clang -D dir/foo.cpp -x c++-header -std=c++17");
+  // Same thing if we have no extension. (again, we treat as header).
+  EXPECT_EQ(getCommand("foo"), "clang -D dir/foo.cpp -x c++-header 
-std=c++17");
+  // and invalid extensions.
+  EXPECT_EQ(getCommand("foo.cce"),
+"clang -D dir/foo.cpp -x c++-header -std=c++17");
   // and don't add -x if the inferred language is correct.
   EXPECT_EQ(getCommand("foo.hpp"), "clang -D dir/foo.cpp -std=c++17");
   // respect -x if it's already there.
   EXPECT_EQ(getCommand("baz.h"), "clang -D dir/baz.cee -x c-header");
   // prefer a worse match with the right extension.
   EXPECT_EQ(getCommand("foo.c"), "clang -D dir/bar.c");
-  // make sure we don't crash on queries with invalid extensions.
-  EXPECT_EQ(getCommand("foo.cce"), "clang -D dir/foo.cpp");
   Entries.erase(path(StringRef("dir/bar.c")));
   // Now we transfer across languages, so drop -std too.
   EXPECT_EQ(getCommand("foo.c"), "clang -D dir/foo.cpp");


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

[PATCH] D61497: [clangd] Introduce a structured hover response

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.h:53
+struct HoverInfo {
+  using Type = std::string;
+  struct Param {

ilya-biryukov wrote:
> NIT: maybe go with `std::string Type` at a use-site instead?
> The scope of the name is large enough to make single-letter names a bit 
> confusing
We had discussed making Type a struct, in case we want to add links etc to it 
later in a back-compatible way.

The typedef doesn't achieve that, I agree with just inlining std::string if 
we're not going to have the struct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61497



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

[PATCH] D61633: [Tooling] Add -x flags when inferring compile commands for files with no/invalid extension.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC360153: [Tooling] Add -x flags when inferring compile 
commands for files with… (authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61633?vs=198413&id=198469#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D61633

Files:
  lib/Tooling/InterpolatingCompilationDatabase.cpp
  unittests/Tooling/CompilationDatabaseTest.cpp


Index: lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -205,10 +205,13 @@
 bool TypeCertain;
 auto TargetType = guessType(Filename, &TypeCertain);
 // If the filename doesn't determine the language (.h), transfer with -x.
-if (TargetType != types::TY_INVALID && !TypeCertain && Type) {
-  TargetType = types::onlyPrecompileType(TargetType) // header?
-   ? types::lookupHeaderTypeForSourceType(*Type)
-   : *Type;
+if ((!TargetType || !TypeCertain) && Type) {
+  // Use *Type, or its header variant if the file is a header.
+  // Treat no/invalid extension as header (e.g. C++ standard library).
+  TargetType =
+  (!TargetType || types::onlyPrecompileType(TargetType)) // header?
+  ? types::lookupHeaderTypeForSourceType(*Type)
+  : *Type;
   if (ClangCLMode) {
 const StringRef Flag = toCLFlag(TargetType);
 if (!Flag.empty())
Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -723,14 +723,17 @@
   // .h is ambiguous, so we add explicit language flags
   EXPECT_EQ(getCommand("foo.h"),
 "clang -D dir/foo.cpp -x c++-header -std=c++17");
+  // Same thing if we have no extension. (again, we treat as header).
+  EXPECT_EQ(getCommand("foo"), "clang -D dir/foo.cpp -x c++-header 
-std=c++17");
+  // and invalid extensions.
+  EXPECT_EQ(getCommand("foo.cce"),
+"clang -D dir/foo.cpp -x c++-header -std=c++17");
   // and don't add -x if the inferred language is correct.
   EXPECT_EQ(getCommand("foo.hpp"), "clang -D dir/foo.cpp -std=c++17");
   // respect -x if it's already there.
   EXPECT_EQ(getCommand("baz.h"), "clang -D dir/baz.cee -x c-header");
   // prefer a worse match with the right extension.
   EXPECT_EQ(getCommand("foo.c"), "clang -D dir/bar.c");
-  // make sure we don't crash on queries with invalid extensions.
-  EXPECT_EQ(getCommand("foo.cce"), "clang -D dir/foo.cpp");
   Entries.erase(path(StringRef("dir/bar.c")));
   // Now we transfer across languages, so drop -std too.
   EXPECT_EQ(getCommand("foo.c"), "clang -D dir/foo.cpp");


Index: lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -205,10 +205,13 @@
 bool TypeCertain;
 auto TargetType = guessType(Filename, &TypeCertain);
 // If the filename doesn't determine the language (.h), transfer with -x.
-if (TargetType != types::TY_INVALID && !TypeCertain && Type) {
-  TargetType = types::onlyPrecompileType(TargetType) // header?
-   ? types::lookupHeaderTypeForSourceType(*Type)
-   : *Type;
+if ((!TargetType || !TypeCertain) && Type) {
+  // Use *Type, or its header variant if the file is a header.
+  // Treat no/invalid extension as header (e.g. C++ standard library).
+  TargetType =
+  (!TargetType || types::onlyPrecompileType(TargetType)) // header?
+  ? types::lookupHeaderTypeForSourceType(*Type)
+  : *Type;
   if (ClangCLMode) {
 const StringRef Flag = toCLFlag(TargetType);
 if (!Flag.empty())
Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -723,14 +723,17 @@
   // .h is ambiguous, so we add explicit language flags
   EXPECT_EQ(getCommand("foo.h"),
 "clang -D dir/foo.cpp -x c++-header -std=c++17");
+  // Same thing if we have no extension. (again, we treat as header).
+  EXPECT_EQ(getCommand("foo"), "clang -D dir/foo.cpp -x c++-header -std=c++17");
+  // and invalid extensions.
+  EXPECT_EQ(getCommand("foo.cce"),
+"clang -D dir/foo.cpp -x c++-header -std=c++17");
   // and don't add -x if the inferred language is correct.
   EXPECT_EQ(getCommand("foo.hpp"), "clang -D dir/foo.cpp -std=c++17");
   // respect -x if it's already there.
   EXPECT_EQ(getCom

r360155 - Add an explicit triple to this test to hopefully appease the build bots.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue May  7 07:40:37 2019
New Revision: 360155

URL: http://llvm.org/viewvc/llvm-project?rev=360155&view=rev
Log:
Add an explicit triple to this test to hopefully appease the build bots.

Modified:
cfe/trunk/test/Index/print-type-size.c

Modified: cfe/trunk/test/Index/print-type-size.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/print-type-size.c?rev=360155&r1=360154&r2=360155&view=diff
==
--- cfe/trunk/test/Index/print-type-size.c (original)
+++ cfe/trunk/test/Index/print-type-size.c Tue May  7 07:40:37 2019
@@ -1,4 +1,4 @@
-// RUN: c-index-test -test-print-type-size %s | FileCheck %s
+// RUN: c-index-test -test-print-type-size %s -target x86_64-pc-linux-gnu | 
FileCheck %s
 
 struct Foo {
   int size;


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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

@jakehehrlich @compnerd ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D59988: [PR41276] Generate address space cast of 'this' for objects attributed by an address space in C++

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: test/CodeGenCXX/address-space-of-this.cpp:9
+//CHECK: call void @_ZN6MyTypeC1Ei(%struct.MyType* addrspacecast 
(%struct.MyType addrspace(10)* @m to %struct.MyType*), i32 123)
+MyType __attribute__((address_space(10))) m = 123;

rjmccall wrote:
> Anastasia wrote:
> > rjmccall wrote:
> > > Sorry I didn't catch this before, but I don't see why this test is 
> > > expected to work.  We can't actually pass a pointer in address space 10 
> > > to that constructor.
> > Ok, I have created a bug to follow up on this issues:
> > https://bugs.llvm.org/show_bug.cgi?id=41730
> > 
> > It seems that the check is missing here for allowing the address space 
> > conversions implicitly, but I am afraid if I add it now addr spaces will 
> > become less usable because implicit conversions can't be setup by the 
> > target yet. And everyone is using no address space as some sort of 
> > `__generic` but unofficially. :(
> > 
> > I have some thoughts about adding something like `__generic` address space 
> > to C++ that can either be resolved by the compiler or supported by HW. I 
> > think it might help to implement those cases correctly without modifying 
> > too much of code base. I just struggled to find enough bandwidth to send an 
> > RFC but I will try to progress on this asap.
> > Ok, I have created a bug to follow up on this issues:
> 
> Thanks.
> 
> > It seems that the check is missing here for allowing the address space 
> > conversions implicitly, but I am afraid if I add it now addr spaces will 
> > become less usable because implicit conversions can't be setup by the 
> > target yet. And everyone is using no address space as some sort of 
> > __generic but unofficially. :(
> 
> As far as I'm concerned, address-space support in the C++ feature set is all 
> still extremely experimental and there are no real users that we have to 
> worry about making things less useful for.  The right thing for the basic 
> model is for constructors to only work when the object is being constructed 
> in an address space that's convertible to the address space of the 
> constructor.  Languages with a `__generic` superspace (whether implemented 
> with dynamic selection or cloning or anything else) can consider making it 
> the default address space of constructors, but that's not something we should 
> be pushing in the basic model.
> As far as I'm concerned, address-space support in the C++ feature set is all 
> still extremely experimental and there are no real users that we have to 
> worry about making things less useful for. 

Ok, then I can try to fix that generically. And at least we can test it using 
`__constant` in OpenCL.


> The right thing for the basic model is for constructors to only work when the 
> object is being constructed in an address space that's convertible to the 
> address space of the constructor.

Just to understand a bit more. Would the constructor address space be given in 
the source code or would it be up to the target to set it up? Also would it be 
applied to all other methods too?



> Languages with a __generic superspace (whether implemented with dynamic 
> selection or cloning or anything else) can consider making it the default 
> address space of constructors, but that's not something we should be pushing 
> in the basic model.

Ok, I have drafted an RFC around this topic that I was planning to share with 
Clang dev community at some point.
https://docs.google.com/document/d/1YybeeRgGrckMjxjtLdRUf0V5f9Psq97cWxBN4npDoqk/edit?usp=sharing

Just in short the idea of this proposal is to make something like `__generic` 
address space as a feature configurable by the targets. It is extending the 
original idea described in 
http://lists.llvm.org/pipermail/cfe-dev/2019-March/061541.html

I am also discussing briefly some extra ideas of language based solutions to 
provide information on address spaces for class instantiations at the source 
level. Your feedback would be highly appreciated even at this very early stage 
(of course if you have any extra bandwidth). It would be much easier to fix 
remaining issues once we agree on the future directions. Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D59988



___
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

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:216
+llvm::dbgs()
+<< "$[collect-tokens]: "
+<< syntax::Token(T).dumpForTests(SourceMgr) << "\n"

what's "$[collect-tokens]"?
Maybe just "Token: "?



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:224
+
+TokenBuffer TokenCollector::consume() && {
+  TokenBuffer B(SourceMgr);

this function is now >100 lines long. Can we split it up?




Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:225
+TokenBuffer TokenCollector::consume() && {
+  TokenBuffer B(SourceMgr);
+  for (unsigned I = 0; I < Expanded.size(); ++I) {

this could be a member, which would help splitting up



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:226
+  TokenBuffer B(SourceMgr);
+  for (unsigned I = 0; I < Expanded.size(); ++I) {
+auto FID =

Is there a reason we do this at the end instead of as tokens are collected?



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:243
+  llvm::DenseMap NextSpelled;
+  auto ConsumeSpelledUntil = [&](TokenBuffer::MarkedFile &File,
+ SourceLocation L) {

consumespelleduntil and fillgapuntil could be methods, I think



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:277
+  for (unsigned I = 0; I < Expanded.size() - 1; ++I) {
+auto L = Expanded[I].location();
+if (L.isFileID()) {

the body here could be a method too, I think
i.e. for each (expanded token), process it?



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:322
+
+  // Some files might have unaccounted spelled tokens at the end.
+  for (auto &F : B.Files) {

and similarly this section



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:130
+  OS << llvm::formatv(
+  "['{0}'_{1}, '{2}'_{3}) => ['{4}'_{5}, '{6}'_{7})\n",
+  PrintToken(File.SpelledTokens[M.BeginSpelled]), M.BeginSpelled,

sammccall wrote:
> As predicted :-) I think these `_` suffixes are a maintenance hazard.
> 
> In practice, the assertions are likely to add them by copy-pasting the test 
> output.
> 
> They guard against a class of error that doesn't seem very likely, and in 
> practice they don't even really prevent it (because of the copy/paste issue).
> 
> I'd suggest just dropping them, and living with the test assertions being 
> slightly ambiguous.
> 
> Failing that, some slightly trickier formatting could give more context:
> 
> `A B C D E F` --> `A B  ... E F`
> 
> With special cases for smaller numbers of tokens. I don't like the 
> irregularity of that approach though.
(this one is still open)



Comment at: clang/unittests/Tooling/Syntax/TokensTest.cpp:622
+
+// FIXME: add tests for mapping spelled tokens into offsets.
+

sammccall wrote:
> please fix :-)
(still missing?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59887



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

r360156 - [ASTImporter] Corrected type of integer constant in a test.

[Balazs Keri via cfe-commits]

Author: balazske
Date: Tue May  7 07:53:04 2019
New Revision: 360156

URL: http://llvm.org/viewvc/llvm-project?rev=360156&view=rev
Log:
[ASTImporter] Corrected type of integer constant in a test.

Modified:
cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=360156&r1=360155&r2=360156&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Tue May  7 07:53:04 2019
@@ -5094,11 +5094,11 @@ TEST_P(ASTImporterOptionSpecificTestBase
   Decl *FromTU = getTuDecl(Code, Lang_CXX);
   auto *FromD = FirstDeclMatcher().match(FromTU,
   functionDecl(hasName("f"), isExplicitTemplateSpecialization()));
-  ASSERT_EQ(FromD->getNumTemplateParameterLists(), 1);
+  ASSERT_EQ(FromD->getNumTemplateParameterLists(), 1u);
 
   auto *ToD = Import(FromD, Lang_CXX);
   // The template parameter list should exist.
-  EXPECT_EQ(ToD->getNumTemplateParameterLists(), 1);
+  EXPECT_EQ(ToD->getNumTemplateParameterLists(), 1u);
 }
 
 struct ASTImporterLookupTableTest : ASTImporterOptionSpecificTestBase {};


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

[PATCH] D61642: [clang-tidy] Do not show incorrect fix in modernize-make-unique

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: aaron.ballman.
Herald added a subscriber: xazax.hun.
Herald added a project: clang.

The case when initialize_list hides behind an implicit case was not
handled before.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61642

Files:
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp


Index: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
===
--- clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
+++ clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
@@ -273,6 +273,12 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead
   // CHECK-FIXES: std::make_unique(APair{T, 1});
 
+  // Check aggregate init with intermediate temporaries.
+  std::unique_ptr PAggrTemp = std::unique_ptr(new APair({T, 1}));
+  // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: use std::make_unique instead
+  PAggrTemp.reset(new APair({T, 1}));
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
+
   // Test different kinds of initialization of the pointee, when the unique_ptr
   // is initialized with braces.
 
Index: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -292,6 +292,8 @@
   //   Foo{1} => false
   auto HasListIntializedArgument = [](const CXXConstructExpr *CE) {
 for (const auto *Arg : CE->arguments()) {
+  Arg = Arg->IgnoreImplicit();
+
   if (isa(Arg) || isa(Arg))
 return true;
   // Check whether we implicitly construct a class from a


Index: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
===
--- clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
+++ clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
@@ -273,6 +273,12 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead
   // CHECK-FIXES: std::make_unique(APair{T, 1});
 
+  // Check aggregate init with intermediate temporaries.
+  std::unique_ptr PAggrTemp = std::unique_ptr(new APair({T, 1}));
+  // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: use std::make_unique instead
+  PAggrTemp.reset(new APair({T, 1}));
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
+
   // Test different kinds of initialization of the pointee, when the unique_ptr
   // is initialized with braces.
 
Index: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -292,6 +292,8 @@
   //   Foo{1} => false
   auto HasListIntializedArgument = [](const CXXConstructExpr *CE) {
 for (const auto *Arg : CE->arguments()) {
+  Arg = Arg->IgnoreImplicit();
+
   if (isa(Arg) || isa(Arg))
 return true;
   // Check whether we implicitly construct a class from a
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61566: Fix for bug 41747: AST Printer doesn't print nested name specifier for out of scope record definitions

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added reviewers: rsmith, aaron.ballman.
jdenny accepted this revision.
jdenny added a comment.
This revision is now accepted and ready to land.

LGTM except for nits in the tests.  I'm not close to C++ support in Clang, so 
please give other reviewers a few days to comment just in case.  I've added a 
couple who have reviewed my patches in this area in the past.

Thanks for the fix!




Comment at: clang/test/AST/ast-print-record-decl.c:303
+  };
+}
+#endif

To make this easier to read (especially in editors without highlighting), I 
suggest putting all FileCheck directives for the definition of each of `struct 
DeclEnclosing` and `struct DeclEnclosing::DeclMember` before that definition.



Comment at: clang/test/AST/ast-print-record-decl.c:307
 // A tag decl group in the tag decl's own member list is exercised in
 // defSelfRef above.

This comment should stay next to `DeclGroupInMemberList` because they're both 
about tag decl groups in member lists.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61566



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

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

Hi @VelocityRa, just FYI - it's considered fine to ping your reviewers once per 
week here if you've addressed their comments and there's no activity in the 
review. Sometimes people just get distracted by other things.


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

https://reviews.llvm.org/D28462



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

[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet added a comment.

In D61634#1493350 , @theraven wrote:

> I wonder if a list of comma-separated names is the right approach.  Would it 
> make more sense to add a new attribute for each of the helpers, such as 
> `#no-runtime-for-memcpy`? That should make querying simpler (one lookup in 
> the table, rather than a lookup and a string scan) and also make it easier to 
> add and remove attributes (merging is now just a matter of trying to add all 
> of them, with the existing logic to deduplicate repeated attributes working).


So I decided to go that route for two reasons:

- The `no_runtime_for` attribute will be used almost exclusively by runtime 
implementers, on average lookup will return false and the parsing part should 
be marginal (famous last words?)
- We need to support every function in TargetLibraryInfo 

 (I count 434 of them) and I'm not sure adding one attribute per function will 
scale or can stay in sync.

Now I haven't thought about merging indeed, we may be able to reuse or clone 
the approach used for `target-features`?
For instance some backend disable specific functions and we may warn the user 
if it happens. e.g. `setLibcallName(RTLIB::SHL_I128, nullptr);` in 
X86ISelLowering.cpp 


I'm not saying one attribute per helper is not feasible but I'd like to put it 
into perspective with other constraints.

> We probably need to also carefully audit all of the transform passes to find 
> ones that insert calls.  Last time I looked, there were four places in LLVM 
> where memcpy could be expanded, I wonder if there are a similar number where 
> it can be synthesised...

Yes indeed, it's going to be long and painful, here are the functions calling 
`CallLoweringInfo::setLibCallee` :

**llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp**

- ExpandLibCall(LC, Node, isSigned)
- ExpandChainLibCall(LC, Node, isSigned)
- ExpandDivRemLibCall(Node, Results)
- ExpandSinCosLibCall(Node, Results)
- ConvertNodeToLibcall(Node)
- ConvertNodeToLibcall(Node)

**llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp**

- ExpandIntRes_XMULO(N, Lo, Hi)

**llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp**

- ExpandChainLibCall(LC, Node, isSigned)

**llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp**

- getMemcpy(Chain, dl, Dst, Src, Size, Align, isVol, AlwaysInline, isTailCall, 
DstPtrInfo, SrcPtrInfo)
- getAtomicMemcpy(Chain, dl, Dst, DstAlign, Src, SrcAlign, Size, SizeTy, 
ElemSz, isTailCall, DstPtrInfo, SrcPtrInfo)
- getMemmove(Chain, dl, Dst, Src, Size, Align, isVol, isTailCall, DstPtrInfo, 
SrcPtrInfo)
- getAtomicMemmove(Chain, dl, Dst, DstAlign, Src, SrcAlign, Size, SizeTy, 
ElemSz, isTailCall, DstPtrInfo, SrcPtrInfo)
- getMemset(Chain, dl, Dst, Src, Size, Align, isVol, isTailCall, DstPtrInfo)
- getAtomicMemset(Chain, dl, Dst, DstAlign, Value, Size, SizeTy, ElemSz, 
isTailCall, DstPtrInfo)

**llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp**

- visitIntrinsicCall(I, Intrinsic)

**llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp**

- makeLibCall(DAG, LC, RetVT, Ops, isSigned, dl, doesNotReturn, 
isReturnValueUsed, isPostTypeLegalization)
- LowerToTLSEmulatedModel(GA, DAG)

**llvm/lib/Target/AArch64/AArch64ISelLowering.cpp**

- LowerFSINCOS(Op, DAG)

**llvm/lib/Target/AArch64/AArch64SelectionDAGInfo.cpp**

- EmitTargetCodeForMemset(DAG, dl, Chain, Dst, Src, Size, Align, isVolatile, 
DstPtrInfo)

**llvm/lib/Target/ARM/ARMISelLowering.cpp**

- LowerToTLSGeneralDynamicModel(GA, DAG)

**llvm/lib/Target/ARM/ARMSelectionDAGInfo.cpp**

- EmitSpecializedLibcall(DAG, dl, Chain, Dst, Src, Size, Align, LC)

**llvm/lib/Target/Hexagon/HexagonSelectionDAGInfo.cpp**

- EmitTargetCodeForMemcpy(DAG, dl, Chain, Dst, Src, Size, Align, isVolatile, 
AlwaysInline, DstPtrInfo, SrcPtrInfo)

**llvm/lib/Target/PowerPC/PPCISelLowering.cpp**

- LowerINIT_TRAMPOLINE(Op, DAG)

**llvm/lib/Target/X86/X86ISelLowering.cpp**

- LowerWin64_i128OP(Op, DAG)
- LowerFSINCOS(Op, Subtarget, DAG)

**llvm/lib/Target/X86/X86SelectionDAGInfo.cpp**

- EmitTargetCodeForMemset(DAG, dl, Chain, Dst, Val, Size, Align, isVolatile, 
DstPtrInfo)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61634



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

[PATCH] D61642: [clang-tidy] Do not show incorrect fix in modernize-make-unique

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.

BTW, for a common use-case we can do the same trick that's being done for 
aggregate init:

  new X({1,2,3}, 123, {a});

into

  make_unique(X({1,2,3}, 123, {a}));

I can try fixing this, but would want to land this first to fix the issue at 
hand.




Comment at: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp:278
+  std::unique_ptr PAggrTemp = std::unique_ptr(new APair({T, 1}));
+  // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: use std::make_unique instead
+  PAggrTemp.reset(new APair({T, 1}));

I'm wondering how to properly check that no fixes were produced here. 
Do we have a common way to express this?
Would `CHECK-FIXES-NOT: ...` mentioning the line numbers somehow work?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61642



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

[PATCH] D49587: [CMake] Support exporting runtimes using the CMake export

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In D49587#1493585 , @ldionne wrote:

> I think the right way to do this was to use `install(export)`?


Oh, and you are doing this, but in `llvm/runtimes`. Why don't we handle this 
per project? I don't think we should assume that the rest of LLVM is available 
in order to export those targets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49587



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

[PATCH] D49587: [CMake] Support exporting runtimes using the CMake export

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

I think the right way to do this was to use `install(export)`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49587



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

[PATCH] D60910: [WIP] Dumping the AST to JSON

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/AST/DeclBase.h:1139
+  void dump(raw_ostream &Out, bool Deserialize = false,
+ASTDumpOutputFormat OutputFormat = ADOF_Default) const;
 

steveire wrote:
> I think we've talked about this before, but I don't think growing interfaces 
> like this is the best way forward. An enum is a less-good replacement for an 
> object (ie making the user of the API responsible for creating the dumper 
> they want to use).
> 
> I think that could be made more convenient in the future. What do you think?
I think that may be a good future improvement, yes.

One thing to take on balance when considering this for the future is that 
`dump()` can be called from within a debugger and that's a bit harder to 
accomplish with an object interface. I'm not certain that will be a big issue 
for my use case, but it may matter to some folks.



Comment at: include/clang/AST/JSONNodeDumper.h:55
+  }
+
+  /// Add a child of the current node with an optional label.

riccibruno wrote:
> Perhaps you should perfect-forward `DoAddChild` ?
Hmm, if I was doing that, I'd probably prefer to use `std::invoke()` to call 
the function, but we can't use that because it's C++17. Do you have concerns if 
I don't make that change just yet (we don't do it from `TextNodeDumper` either).



Comment at: include/clang/AST/JSONNodeDumper.h:53
+  template  void AddChild(Fn DoAddChild) {
+return AddChild("inner", DoAddChild);
+  }

steveire wrote:
> You have 
> 
> !Label.empty() ? Label : "inner";
> 
> below. This looks needlessly non-DRY?
There are calls to `AddChild()` from the AST traverser that pass in an empty 
string, which is how I got to the state I'm in (see 
`ASTNodeTraverser::Visit(const Stmt *, StringRef)`). I can pull the explicit 
label from this call and just pass an empty string.



Comment at: include/clang/AST/JSONNodeDumper.h:130
+// JSON and then dump to a file because the AST is an ordered collection of
+// nodes but our JSON objects are modelled as an unordered container. We can
+// use JSON facilities for constructing the terminal fields in a node because

steveire wrote:
> I'm no expert in JSON, so I'm not certain if this has implications.
> 
> Do entries in a JSON object (ie not an Array-type) have order? Are you 
> 'giving' them order here which is not part of how JSON is usually parsed (eg 
> by third party tools)?
> 
> Can you create ordered JSON (arrays) where it is needed? I see that `inner` 
> is already an array, so I'm probably missing the part that is 'modelled as an 
> unordered container'. Can you point that out to me?
Oh, good catch! I need to update those comments because they got stale.

> Can you create ordered JSON (arrays) where it is needed? I see that inner is 
> already an array, so I'm probably missing the part that is 'modelled as an 
> unordered container'. Can you point that out to me?

It's the key/value pair bits that are unordered, and that turns out to not 
matter here. I wrote that comment during a very early pass when I was passing 
around JSON objects more liberally and I had run into a situation where I was 
using kv pairs but order mattered (I don't even recall what the case was off 
the top of my head now). With the new design, these comments are stale -- I've 
corrected that.



Comment at: include/clang/AST/JSONNodeDumper.h:161
+  OS << ",\n";
+OS << Indentation << "\"" << Key << "\": {\n";
+Indentation += "  ";

steveire wrote:
> Is it possible this 'textual' JSON generation could produce invalid JSON? One 
> of the things a library solution ensures is balanced braces and valid syntax 
> etc. I'm not familiar enough with JSON to know if it has dark corners.
I don't believe this will produce invalid JSON, short of aborting mid-stream.


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

https://reviews.llvm.org/D60910



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

[PATCH] D60910: [WIP] Dumping the AST to JSON

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 198477.
aaron.ballman marked 14 inline comments as done.
aaron.ballman added a comment.

Updating based on review feedback.


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

https://reviews.llvm.org/D60910

Files:
  include/clang/AST/ASTDumperUtils.h
  include/clang/AST/DeclBase.h
  include/clang/AST/JSONNodeDumper.h
  include/clang/Driver/CC1Options.td
  include/clang/Frontend/ASTConsumers.h
  include/clang/Frontend/FrontendOptions.h
  lib/AST/ASTDumper.cpp
  lib/AST/CMakeLists.txt
  lib/AST/JSONNodeDumper.cpp
  lib/Frontend/ASTConsumers.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/FrontendActions.cpp
  test/AST/ast-dump-enum-json.cpp
  test/AST/ast-dump-if-json.cpp
  tools/clang-check/ClangCheck.cpp
  tools/clang-import-test/clang-import-test.cpp

Index: tools/clang-import-test/clang-import-test.cpp
===
--- tools/clang-import-test/clang-import-test.cpp
+++ tools/clang-import-test/clang-import-test.cpp
@@ -316,8 +316,9 @@
   auto &CG = *static_cast(ASTConsumers.back().get());
 
   if (ShouldDumpAST)
-ASTConsumers.push_back(CreateASTDumper(nullptr /*Dump to stdout.*/,
-   "", true, false, false));
+ASTConsumers.push_back(
+CreateASTDumper(nullptr /*Dump to stdout.*/, "", true, false, false,
+clang::ADOF_Default));
 
   CI.getDiagnosticClient().BeginSourceFile(
   CI.getCompilerInstance().getLangOpts(),
Index: tools/clang-check/ClangCheck.cpp
===
--- tools/clang-check/ClangCheck.cpp
+++ tools/clang-check/ClangCheck.cpp
@@ -134,11 +134,11 @@
 if (ASTList)
   return clang::CreateASTDeclNodeLister();
 if (ASTDump)
-  return clang::CreateASTDumper(nullptr /*Dump to stdout.*/,
-ASTDumpFilter,
+  return clang::CreateASTDumper(nullptr /*Dump to stdout.*/, ASTDumpFilter,
 /*DumpDecls=*/true,
 /*Deserialize=*/false,
-/*DumpLookups=*/false);
+/*DumpLookups=*/false,
+clang::ADOF_Default);
 if (ASTPrint)
   return clang::CreateASTPrinter(nullptr, ASTDumpFilter);
 return llvm::make_unique();
Index: test/AST/ast-dump-if-json.cpp
===
--- test/AST/ast-dump-if-json.cpp
+++ test/AST/ast-dump-if-json.cpp
@@ -0,0 +1,360 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux -std=c++17 -ast-dump=json %s | FileCheck %s
+
+void func(int val) {
+  if (val)
+;
+
+// CHECK: "kind": "IfStmt",
+// CHECK-NEXT: "range": {"begin":{"col":3,"file":"{{.*}}","line":4},"end":{"col":5,"file":"{{.*}}","line":5}},
+// CHECK-NEXT: "inner": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "id": "0x{{.*}}",
+// CHECK-NEXT: "kind": "ImplicitCastExpr",
+// CHECK-NEXT: "range": {"begin":{"col":7,"file":"{{.*}}","line":4},"end":{"col":7,"file":"{{.*}}","line":4}},
+// CHECK-NEXT: "type": {"qualType":"bool"},
+// CHECK-NEXT: "valueCategory": "rvalue",
+// CHECK-NEXT: "inner": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "id": "0x{{.*}}",
+// CHECK-NEXT: "kind": "ImplicitCastExpr",
+// CHECK-NEXT: "range": {"begin":{"col":7,"file":"{{.*}}","line":4},"end":{"col":7,"file":"{{.*}}","line":4}},
+// CHECK-NEXT: "type": {"qualType":"int"},
+// CHECK-NEXT: "valueCategory": "rvalue",
+// CHECK-NEXT: "inner": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "id": "0x{{.*}}",
+// CHECK-NEXT: "kind": "DeclRefExpr",
+// CHECK-NEXT: "range": {"begin":{"col":7,"file":"{{.*}}","line":4},"end":{"col":7,"file":"{{.*}}","line":4}},
+// CHECK-NEXT: "type": {"qualType":"int"},
+// CHECK-NEXT: "valueCategory": "lvalue"
+// CHECK-NEXT: }
+// CHECK-NEXT: ]
+// CHECK-NEXT: }
+// CHECK-NEXT: ]
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT: "id": "0x{{.*}}",
+// CHECK-NEXT: "kind": "NullStmt",
+// CHECK-NEXT: "range": {"begin":{"col":5,"file":"{{.*}}","line":5},"end":{"col":5,"file":"{{.*}}","line":5}}
+// CHECK-NEXT: }
+// CHECK-NEXT: ]
+// CHECK-NEXT: },
+
+  if (val)
+;
+  else
+;
+
+// CHECK: "kind": "IfStmt",
+// CHECK-NEXT: "range": {"begin":{"col":3,"file":"{{.*}}","line":43},"end":{"col":5,"file":"{{.*}}","line":46}},
+// CHECK-NEXT: "hasElse": true,
+// CHECK-NEXT: "inner": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "id": "0x{{.*}}",
+// CHECK-NEXT: "kind": "ImplicitCastExpr",
+// CHECK-NEXT: "range": {"begin":{"col":7,"file":"{{.*}}","line":43},"end":{"col":7,"file":"{{.*}}","line":43}},
+// CHECK-NEXT: "type": {"qualType":"bool"},
+// CHECK-NEXT: "valueCategory": "rvalue",
+// CHECK-NEXT: "inner": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "id": "0x{{.*}}",
+// CHECK-NEXT: "kind": "ImplicitCastExpr",
+// CHECK-NEXT: "range": {"begin":{"col":7,"file":"{{.*}}","line":43},"end":{"col":7,"file":"{{.*}}","line":43}},
+// CHE

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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:130
+  OS << llvm::formatv(
+  "['{0}'_{1}, '{2}'_{3}) => ['{4}'_{5}, '{6}'_{7})\n",
+  PrintToken(File.SpelledTokens[M.BeginSpelled]), M.BeginSpelled,

sammccall wrote:
> sammccall wrote:
> > As predicted :-) I think these `_` suffixes are a maintenance hazard.
> > 
> > In practice, the assertions are likely to add them by copy-pasting the test 
> > output.
> > 
> > They guard against a class of error that doesn't seem very likely, and in 
> > practice they don't even really prevent it (because of the copy/paste 
> > issue).
> > 
> > I'd suggest just dropping them, and living with the test assertions being 
> > slightly ambiguous.
> > 
> > Failing that, some slightly trickier formatting could give more context:
> > 
> > `A B C D E F` --> `A B  ... E F`
> > 
> > With special cases for smaller numbers of tokens. I don't like the 
> > irregularity of that approach though.
> (this one is still open)
Will make sure to do something about it before submitting



Comment at: clang/unittests/Tooling/Syntax/TokensTest.cpp:622
+
+// FIXME: add tests for mapping spelled tokens into offsets.
+

sammccall wrote:
> sammccall wrote:
> > please fix :-)
> (still missing?)
Will make sure to land this before submitting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59887



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

[PATCH] D61637: [Syntax] Introduce syntax trees

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/Corpus.h:23
+/// token buffers, source manager, etc.
+class Corpus {
+public:

I think plain SyntaxArena might be a better name here :-/
Corpus refers to texts (the use in dex is by analogy, as we call symbols 
"documents" from search).




Comment at: clang/include/clang/Tooling/Syntax/Corpus.h:26
+  Corpus(SourceManager &SourceMgr, const LangOptions &LangOpts,
+ TokenBuffer MainFile);
+

MainFile is presumably the whole TU, name might need a tweak.
Can it be empty?
The relationship between Corpus and TokenBuffer seems a little weird. Why is it 
needed?



Comment at: clang/include/clang/Tooling/Syntax/Corpus.h:38
+  std::pair>
+  tokenizeBuffer(std::unique_ptr Buffer);
+

Are you planning to have a way to add tokens directly? Having to turn them into 
text and re-lex them seems like it might be inconvenient.



Comment at: clang/include/clang/Tooling/Syntax/Corpus.h:40
+
+  /// Construct a new syntax node of a specified kind. The memory for a node is
+  /// owned by the corpus and will be freed when the corpus is destroyed.

Now there's two ways to do this: `new (C.allocator()) T(...)` or 
`C.construct(...)`. Do we need both?

(If we do, is the syntax `new (C) T(...)` more natural?)



Comment at: clang/include/clang/Tooling/Syntax/Tree.h:34
+
+/// Build a syntax tree for the main file.
+TranslationUnit *buildSyntaxTree(Corpus &C,

for a translation unit? or for only decls within the main file?



Comment at: clang/include/clang/Tooling/Syntax/Tree.h:40
+/// node.
+void traverse(Node *N, llvm::function_ref Visit);
+void traverse(const Node *N, llvm::function_ref Visit);

I've been burned with adding these APIs without use cases.

It seems likely you want a way to:
 - skip traversal of children
 - abort the traversal entirely



Comment at: clang/include/clang/Tooling/Syntax/Tree/Cascade.h:1
+//===- Tree.h - cascade of the syntax tree *- C++ 
-*-=//
+//

this is Cascade.h, not tree.h



Comment at: clang/include/clang/Tooling/Syntax/Tree/Cascade.h:1
+//===- Tree.h - cascade of the syntax tree *- C++ 
-*-=//
+//

sammccall wrote:
> this is Cascade.h, not tree.h
why "cascade"?



Comment at: clang/include/clang/Tooling/Syntax/Tree/Cascade.h:1
+//===- Tree.h - cascade of the syntax tree *- C++ 
-*-=//
+//

sammccall wrote:
> sammccall wrote:
> > this is Cascade.h, not tree.h
> why "cascade"?
The Tree/ subdirectory seems superfluous - why are these separate from Syntax/?



Comment at: clang/include/clang/Tooling/Syntax/Tree/Cascade.h:74
+/// A composite tree node that has children.
+class TreeNode : public Node {
+public:

This use of "tree node" to mean specifically internal node seems confusing - is 
it common?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61637



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

[PATCH] D61475: Update an information about ReSharper C++ and clang-tidy custom binary integration

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added a comment.

Can anyone merge it? I have no commit access


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61475



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

[PATCH] D61627: [clang driver] Allow -fembed-bitcode combined with -mno-red-zone

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

The main concern for adding this `blacklist` was because of long term 
maintainability since the option is going to be embedded into bitcode. Looking 
at this specific option, I have no reason against because it doesn't affect 
embedded compiler flags.

I added Tim to see if allowing disable redzone can affect ABI and if it can be 
supported.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61627



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

[PATCH] D60974: Clang IFSO driver action.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.



Comment at: clang/include/clang/Driver/Options.td:626
   HelpText<"Use the LLVM representation for assembler and object files">;
+def emit_ifso : Flag<["-"], "emit-interface-stubs">, Flags<[CC1Option]>, 
Group,
+  HelpText<"Generate Inteface Stub Files.">;

I thought that we were going to add `experimental` to this for the time being?



Comment at: clang/include/clang/Driver/Options.td:628
+  HelpText<"Generate Inteface Stub Files.">;
+def ifso_version_EQ : Joined<["-"], "interface-stubs-version=">, 
Flags<[CC1Option]>;
 def exported__symbols__list : Separate<["-"], "exported_symbols_list">;

Personally, I have a slight preference for the separate version rather than the 
joined.



Comment at: clang/include/clang/Driver/Types.def:91
 TYPE("ast",  AST,  INVALID, "ast",   "u")
+TYPE("ifs",  IFS,  INVALID, "ifs",   "u")
 TYPE("pcm",  ModuleFile,   INVALID, "pcm",   "u")

What about `ifo` instead of `ifs`?



Comment at: clang/include/clang/Frontend/FrontendActions.h:128
+};
+// Support different ifso formats this way:
+class GenerateIfsoObjYamlExpV1Action : public GenerateIFSOAction {

Not sure if the comment adds anything.  But a newline before these would be 
nice.



Comment at: clang/include/clang/Frontend/FrontendActions.h:134
+};
+class GenerateIfsoTbeExpV1Action : public GenerateIFSOAction {
+protected:

A newline between the classes would be nice.



Comment at: clang/include/clang/Frontend/FrontendOptions.h:91
+  /// Generate Interface Stub Files.
+  GenerateIfsoObjYamlExpV1,
+  GenerateIfsoTbeExpV1,

As per LLVM style, initialisms are not downcased.  This should be 
`GenerateIfSoObjYAMLExpV1`.  I think I prefer `GenerateInterfaceYAMLExpV1`



Comment at: clang/include/clang/Frontend/FrontendOptions.h:92
+  GenerateIfsoObjYamlExpV1,
+  GenerateIfsoTbeExpV1,
+

Similar.  I think I prefer `GenerateInterfaceTBEExpV1`



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3590
+Twine("-interface-stubs-version=") +
+Args.getLastArgValue(options::OPT_ifso_version_EQ)));
+  }

Please ensure that the value being passed is valid or emit a diagnostic.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1649
+case OPT_emit_ifso:
+  Opts.ProgramAction = frontend::GenerateIfsoTbeExpV1;
+  if (Args.hasArg(OPT_ifso_version_EQ))

Can we make this required to be specified instead of defaulting please?  I 
think that it is more clear.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1651
+  if (Args.hasArg(OPT_ifso_version_EQ))
+if(Args.getLastArgValue(OPT_ifso_version_EQ) == 
"experimental-ifo-elf-v1")
+  Opts.ProgramAction = frontend::GenerateIfsoObjYamlExpV1;

Please use a covered `StringSwitch` here.  Since this is in the frontend,  I 
think that we should have a test that the driver diagnoses invalid values.



Comment at: clang/lib/Frontend/FrontendActions.cpp:164
 
+class IFSOFunctionsConsumer : public ASTConsumer {
+  CompilerInstance &Instance;

This does not belong here.  Please extract it into a separate file, much like 
the serialization is extracted into a separate file.



Comment at: clang/lib/Frontend/FrontendActions.cpp:179
+  };
+  using MangledSymbols = std::map;
+

Hmm, I wonder if `DenseSet` is more appropriate here.  Is there some reason 
that I am overlooking for supporting duplicate entries?  In fact, since the key 
is a pointer, you could even do a `DenseMap`.



Comment at: clang/lib/Frontend/FrontendActions.cpp:182
+  template 
+  bool WriteNamedDecl(const NamedDecl *ND, MangledSymbols &Symbols, int RDO) {
+if (!isa(ND))

I'm not sure I like this very much.  You are templating over a type a function 
that needs to walk the hierarchy for `NamedDecl`.  This is going to re-emit the 
function and without LTO cause a large `.text` contribution for something that 
should really just be a jump table.  Would it not be possible to use a covered 
switch on the `kindof()` in the `NamedDecl`.  The covering should ensure that 
future changes would allow us to find this site.



Comment at: clang/lib/Frontend/FrontendActions.cpp:189
+  return true;
+if (ND->getVisibility() != DefaultVisibility)
+  return true;

I understand this, but, it may be nice to have a comment explaining why we only 
consider default visibility (hidden visibility interfaces cannot be seen by 
consumers, and protected mode visibility in ELF does not participate in 
linking,

[PATCH] D61627: [clang driver] Allow -fembed-bitcode combined with -mno-red-zone

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

@steven_wu - yeah, `-mred-zone`, `-mno-red-zone`  does impact the ABI, at least 
on x86_64.  It changes the way that the arguments are spilled and the stack 
layout.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61627



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

[PATCH] D61509: [PragmaHandler] Expose `#pragma` location

[Joel E. Denny via Phabricator via cfe-commits]

jdenny updated this revision to Diff 198488.
jdenny retitled this revision from "[PragmaHandler][OpenMP] Expose `#pragma` 
location" to "[PragmaHandler] Expose `#pragma` location".
jdenny edited the summary of this revision.
jdenny set the repository for this revision to rG LLVM Github Monorepo.
jdenny added a comment.

As suggested, I've created a `struct PragmaIntroducer` and I've reduced this 
patch not to include the OpenMP changes.  I'll add a new patch with the OpenMP 
changes soon.

There are a few additional changes here reviewers might want to check out:

- This time I noticed that `clang/docs/ClangPlugins.rst` and 
`clang/examples/AnnotateFunctions` need to be updated for the change to 
`PragmaHandler` parameters, so I did that.  Does that represent an important 
break in backward compatibility?

- I've also used `PragmaIntroducer` to simplify 
`clang::Preprocessor::HandlePragmaDirective`, which already accepted a 
`SourceLocation` and `PragmaIntroducerKind` before any of my patches.

- I didn't do the same for `clang::PPCallbacks::PragmaDirective` because that 
changes the API documented in `clang-tools-extra/docs/pp-trace.rst`, and I'm 
not sure about backward compatibility guarantees there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61509

Files:
  clang/docs/ClangPlugins.rst
  clang/examples/AnnotateFunctions/AnnotateFunctions.cpp
  clang/include/clang/Lex/Pragma.h
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Frontend/PrintPreprocessedOutput.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/Pragma.cpp
  clang/lib/Parse/ParsePragma.cpp

Index: clang/lib/Parse/ParsePragma.cpp
===
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -26,71 +26,72 @@
 
 struct PragmaAlignHandler : public PragmaHandler {
   explicit PragmaAlignHandler() : PragmaHandler("align") {}
-  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+  void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
 Token &FirstToken) override;
 };
 
 struct PragmaGCCVisibilityHandler : public PragmaHandler {
   explicit PragmaGCCVisibilityHandler() : PragmaHandler("visibility") {}
-  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+  void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
 Token &FirstToken) override;
 };
 
 struct PragmaOptionsHandler : public PragmaHandler {
   explicit PragmaOptionsHandler() : PragmaHandler("options") {}
-  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+  void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
 Token &FirstToken) override;
 };
 
 struct PragmaPackHandler : public PragmaHandler {
   explicit PragmaPackHandler() : PragmaHandler("pack") {}
-  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+  void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
 Token &FirstToken) override;
 };
 
 struct PragmaClangSectionHandler : public PragmaHandler {
   explicit PragmaClangSectionHandler(Sema &S)
  : PragmaHandler("section"), Actions(S) {}
-  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+  void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
 Token &FirstToken) override;
+
 private:
   Sema &Actions;
 };
 
 struct PragmaMSStructHandler : public PragmaHandler {
   explicit PragmaMSStructHandler() : PragmaHandler("ms_struct") {}
-  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+  void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
 Token &FirstToken) override;
 };
 
 struct PragmaUnusedHandler : public PragmaHandler {
   PragmaUnusedHandler() : PragmaHandler("unused") {}
-  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+  void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
 Token &FirstToken) override;
 };
 
 struct PragmaWeakHandler : public PragmaHandler {
   explicit PragmaWeakHandler() : PragmaHandler("weak") {}
-  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+  void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
 Token &FirstToken) override;
 };
 
 struct PragmaRedefineExtnameHandler : public PragmaHandler {
   explicit PragmaRedefineExtnameHandler() : PragmaHandler("redefine_extname") {}
-  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+  void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
 Token &FirstToken) override;
 };
 
 struct PragmaOpenCLExtensionHandler : public PragmaHandler {
   PragmaOpenCLExtensionHandler() : PragmaHandler("EXTENSION") {}
-  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+  vo

[PATCH] D61642: [clang-tidy] Do not show incorrect fix in modernize-make-unique

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked an inline comment as done.
aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:302
   if (const auto *ImplicitCE =
   dyn_cast(Arg->IgnoreImplicit())) {
 if (ImplicitCE->isStdInitListInitialization())

We can now remove the `IgnoreImplicit()` call here, can't we?



Comment at: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp:278
+  std::unique_ptr PAggrTemp = std::unique_ptr(new APair({T, 1}));
+  // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: use std::make_unique instead
+  PAggrTemp.reset(new APair({T, 1}));

ilya-biryukov wrote:
> I'm wondering how to properly check that no fixes were produced here. 
> Do we have a common way to express this?
> Would `CHECK-FIXES-NOT: ...` mentioning the line numbers somehow work?
I believe we typically use `CHECK-FIXES: ` to ensure 
that the fix does not change any code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61642



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

[PATCH] D61566: Fix for bug 41747: AST Printer doesn't print nested name specifier for out of scope record definitions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/DeclPrinter.cpp:958
+
+if (auto *Q = D->getQualifier())
+  Q->print(Out, Policy);

Don't use `auto` here as the type is not explicitly spelled out in the 
initialization. If possible, `const`-qualify the pointer type as well.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61566



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

[PATCH] D61627: [clang driver] Allow -fembed-bitcode combined with -mno-red-zone

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

In D61627#1493674 , @compnerd wrote:

> @steven_wu - yeah, `-mred-zone`, `-mno-red-zone`  does impact the ABI, at 
> least on x86_64.  It changes the way that the arguments are spilled and the 
> stack layout.


Well, I am not concern about ABI in that sense and not x86_64. I am concerning 
about ABI re-targeting from armv7k to arm64_32. It might be just fine but I 
can't tell for sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61627



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi marked 6 inline comments as done.
plotfi added inline comments.



Comment at: clang/include/clang/Driver/Options.td:626
   HelpText<"Use the LLVM representation for assembler and object files">;
+def emit_ifso : Flag<["-"], "emit-interface-stubs">, Flags<[CC1Option]>, 
Group,
+  HelpText<"Generate Inteface Stub Files.">;

compnerd wrote:
> I thought that we were going to add `experimental` to this for the time being?
Oh, I was specifying experimental in the 
-interface-stubs-version=experimental-ifo-elf-v1. Do you want the main flag to 
also be -emit-interface-stubs-experimental??



Comment at: clang/include/clang/Driver/Types.def:91
 TYPE("ast",  AST,  INVALID, "ast",   "u")
+TYPE("ifs",  IFS,  INVALID, "ifs",   "u")
 TYPE("pcm",  ModuleFile,   INVALID, "pcm",   "u")

compnerd wrote:
> What about `ifo` instead of `ifs`?
I went with ifs because ifo implies the analog of a .o file and ifso implies 
the analog of a .so file. I want the tbe/ifs text files to just be thought of 
as something a little different. Like symbol listings that another tool can 
assemble into whatever format.  



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1649
+case OPT_emit_ifso:
+  Opts.ProgramAction = frontend::GenerateIfsoTbeExpV1;
+  if (Args.hasArg(OPT_ifso_version_EQ))

compnerd wrote:
> Can we make this required to be specified instead of defaulting please?  I 
> think that it is more clear.
I think that makes sense. 



Comment at: clang/lib/Frontend/FrontendActions.cpp:179
+  };
+  using MangledSymbols = std::map;
+

compnerd wrote:
> Hmm, I wonder if `DenseSet` is more appropriate here.  Is there some reason 
> that I am overlooking for supporting duplicate entries?  In fact, since the 
> key is a pointer, you could even do a `DenseMap`.
Can we do data-structure improvements post landing the first version? 



Comment at: clang/lib/Frontend/FrontendActions.cpp:214
+  ND->dump();
+}
+return true;

compnerd wrote:
> debugging left over?
Ah yeah I need to replace this with an llvm_unreachable or something. 



Comment at: clang/lib/Frontend/FrontendActions.cpp:297
+sema.LateTemplateParser(sema.OpaqueParser, LPT);
+HandleNamedDecl(FD, Symbols, (FromTU | IsLate));
+  }

compnerd wrote:
> Typo of `||`?  The field is boolean not a mask.
It is a bitmask. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D61509: [PragmaHandler] Expose `#pragma` location

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

It would have been better to submit this refactor as a new patch..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61509



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

[PATCH] D61370: Add a C2x mode and allow attributes in it

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Ping.


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

https://reviews.llvm.org/D61370



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

[PATCH] D60974: Clang IFSO driver action.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.



Comment at: clang/include/clang/Driver/Options.td:626
   HelpText<"Use the LLVM representation for assembler and object files">;
+def emit_ifso : Flag<["-"], "emit-interface-stubs">, Flags<[CC1Option]>, 
Group,
+  HelpText<"Generate Inteface Stub Files.">;

plotfi wrote:
> compnerd wrote:
> > I thought that we were going to add `experimental` to this for the time 
> > being?
> Oh, I was specifying experimental in the 
> -interface-stubs-version=experimental-ifo-elf-v1. Do you want the main flag 
> to also be -emit-interface-stubs-experimental??
Okay, I can live with that.  I guess that was just unclear to me.  Doing that 
is nicer in that there is less churn in the actual flags.



Comment at: clang/include/clang/Driver/Types.def:91
 TYPE("ast",  AST,  INVALID, "ast",   "u")
+TYPE("ifs",  IFS,  INVALID, "ifs",   "u")
 TYPE("pcm",  ModuleFile,   INVALID, "pcm",   "u")

plotfi wrote:
> compnerd wrote:
> > What about `ifo` instead of `ifs`?
> I went with ifs because ifo implies the analog of a .o file and ifso implies 
> the analog of a .so file. I want the tbe/ifs text files to just be thought of 
> as something a little different. Like symbol listings that another tool can 
> assemble into whatever format.  
Oh, I see.  Yeah  torn on that tbh.  There are aspects of it as being 
analogs to `.o` or `.obj` as well.  But, having them be confused for one 
another is worse.  If this makes sense to you and @rupprecht, WFM.



Comment at: clang/lib/Frontend/FrontendActions.cpp:297
+sema.LateTemplateParser(sema.OpaqueParser, LPT);
+HandleNamedDecl(FD, Symbols, (FromTU | IsLate));
+  }

plotfi wrote:
> compnerd wrote:
> > Typo of `||`?  The field is boolean not a mask.
> It is a bitmask. 
Oh!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D61509: [PragmaHandler] Expose `#pragma` location

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D61509#1493703 , @lebedev.ri wrote:

> It would have been better to submit this refactor as a new patch..


Sorry, I didn't realize that was the norm.  I can do that now if it would help. 
 I can also revert changes to this patch if the goal is to make it easier to 
reference the old version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61509



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

[PATCH] D61509: [PragmaHandler] Expose `#pragma` location

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D61509#1493714 , @jdenny wrote:

> In D61509#1493703 , @lebedev.ri 
> wrote:
>
> > It would have been better to submit this refactor as a new patch..
>
>
> Sorry, I didn't realize that was the norm.  I can do that now if it would 
> help.  I can also revert changes to this patch if the goal is to make it 
> easier to reference the old version.


I think that would be good. This current diff would be a simple NFC cleanup, i 
think i will signoff it even.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61509



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

[PATCH] D61509: [PragmaHandler] Expose `#pragma` location

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D61509#1493718 , @lebedev.ri wrote:

> In D61509#1493714 , @jdenny wrote:
>
> > In D61509#1493703 , @lebedev.ri 
> > wrote:
> >
> > > It would have been better to submit this refactor as a new patch..
> >
> >
> > Sorry, I didn't realize that was the norm.  I can do that now if it would 
> > help.  I can also revert changes to this patch if the goal is to make it 
> > easier to reference the old version.
>
>
> I think that would be good. This current diff would be a simple NFC cleanup, 
> i think i will signoff it even.


Thanks, I'll do that.

For future reference, I assume abandoning and starting a new review is better 
here because I'm splitting the patch.  Is there a more general rule of thumb on 
this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61509



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

Re: r360109 - Recommit r359859 "[Attribute/Diagnostics] Print macro if definition is an attribute declaration"

[Jonas Devlieghere via cfe-commits]

Hi Leonard,

It appears that your patch is still triggering an assertion on GreenDragon:
http://green.lab.llvm.org/green/job/clang-stage1-configure-RA/56255/consoleFull#312501878d489585b-5106-414a-ac11-3ff90657619c

Can you please have a look?

Thanks,
Jonas


On Mon, May 6, 2019 at 8:17 PM Leonard Chan via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: leonardchan
> Date: Mon May  6 20:20:17 2019
> New Revision: 360109
>
> URL: http://llvm.org/viewvc/llvm-project?rev=360109&view=rev
> Log:
> Recommit r359859 "[Attribute/Diagnostics] Print macro if definition is an
> attribute declaration"
>
> Updated with fix for read of uninitialized memory.
>
> Added:
> cfe/trunk/test/Frontend/macro_defined_type.cpp
> cfe/trunk/test/Sema/address_space_print_macro.c
> Modified:
> cfe/trunk/include/clang/AST/ASTContext.h
> cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
> cfe/trunk/include/clang/AST/Type.h
> cfe/trunk/include/clang/AST/TypeLoc.h
> cfe/trunk/include/clang/AST/TypeNodes.def
> cfe/trunk/include/clang/Parse/Parser.h
> cfe/trunk/include/clang/Sema/ParsedAttr.h
> cfe/trunk/include/clang/Sema/Sema.h
> cfe/trunk/include/clang/Serialization/ASTBitCodes.h
> cfe/trunk/lib/ARCMigrate/TransGCAttrs.cpp
> cfe/trunk/lib/AST/ASTContext.cpp
> cfe/trunk/lib/AST/ASTDiagnostic.cpp
> cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp
> cfe/trunk/lib/AST/ItaniumMangle.cpp
> cfe/trunk/lib/AST/Type.cpp
> cfe/trunk/lib/AST/TypePrinter.cpp
> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
> cfe/trunk/lib/Parse/ParseDecl.cpp
> cfe/trunk/lib/Sema/SemaExpr.cpp
> cfe/trunk/lib/Sema/SemaStmt.cpp
> cfe/trunk/lib/Sema/SemaType.cpp
> cfe/trunk/lib/Sema/TreeTransform.h
> cfe/trunk/lib/Serialization/ASTReader.cpp
> cfe/trunk/lib/Serialization/ASTWriter.cpp
> cfe/trunk/test/Sema/address_spaces.c
> cfe/trunk/test/SemaObjC/externally-retained.m
> cfe/trunk/test/SemaObjC/gc-attributes.m
> cfe/trunk/test/SemaObjC/mrc-weak.m
> cfe/trunk/test/SemaObjCXX/gc-attributes.mm
> cfe/trunk/tools/libclang/CIndex.cpp
>
> Modified: cfe/trunk/include/clang/AST/ASTContext.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=360109&r1=360108&r2=360109&view=diff
>
> ==
> --- cfe/trunk/include/clang/AST/ASTContext.h (original)
> +++ cfe/trunk/include/clang/AST/ASTContext.h Mon May  6 20:20:17 2019
> @@ -1441,6 +1441,9 @@ public:
>
>QualType getParenType(QualType NamedType) const;
>
> +  QualType getMacroQualifiedType(QualType UnderlyingTy,
> + const IdentifierInfo *MacroII) const;
> +
>QualType getElaboratedType(ElaboratedTypeKeyword Keyword,
>   NestedNameSpecifier *NNS, QualType NamedType,
>   TagDecl *OwnedTagDecl = nullptr) const;
>
> Modified: cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/RecursiveASTVisitor.h?rev=360109&r1=360108&r2=360109&view=diff
>
> ==
> --- cfe/trunk/include/clang/AST/RecursiveASTVisitor.h (original)
> +++ cfe/trunk/include/clang/AST/RecursiveASTVisitor.h Mon May  6 20:20:17
> 2019
> @@ -1065,6 +1065,9 @@ DEF_TRAVERSE_TYPE(AttributedType,
>
>  DEF_TRAVERSE_TYPE(ParenType, { TRY_TO(TraverseType(T->getInnerType())); })
>
> +DEF_TRAVERSE_TYPE(MacroQualifiedType,
> +  { TRY_TO(TraverseType(T->getUnderlyingType())); })
> +
>  DEF_TRAVERSE_TYPE(ElaboratedType, {
>if (T->getQualifier()) {
>  TRY_TO(TraverseNestedNameSpecifier(T->getQualifier()));
> @@ -1308,6 +1311,9 @@ DEF_TRAVERSE_TYPELOC(InjectedClassNameTy
>
>  DEF_TRAVERSE_TYPELOC(ParenType, {
> TRY_TO(TraverseTypeLoc(TL.getInnerLoc())); })
>
> +DEF_TRAVERSE_TYPELOC(MacroQualifiedType,
> + { TRY_TO(TraverseTypeLoc(TL.getInnerLoc())); })
> +
>  DEF_TRAVERSE_TYPELOC(AttributedType,
>   { TRY_TO(TraverseTypeLoc(TL.getModifiedLoc())); })
>
>
> Modified: cfe/trunk/include/clang/AST/Type.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=360109&r1=360108&r2=360109&view=diff
>
> ==
> --- cfe/trunk/include/clang/AST/Type.h (original)
> +++ cfe/trunk/include/clang/AST/Type.h Mon May  6 20:20:17 2019
> @@ -4184,6 +4184,41 @@ public:
>static bool classof(const Type *T) { return T->getTypeClass() ==
> Typedef; }
>  };
>
> +/// Sugar type that represents a type that was qualified by a qualifier
> written
> +/// as a macro invocation.
> +class MacroQualifiedType : public Type {
> +  friend class ASTContext; // ASTContext creates these.
> +
> +  QualType UnderlyingTy;
> +  const IdentifierInfo *Ma