r341910 - [Tooling] Restore working dir in ClangTool.

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Tue Sep 11 00:29:09 2018
New Revision: 341910

URL: http://llvm.org/viewvc/llvm-project?rev=341910&view=rev
Log:
[Tooling] Restore working dir in ClangTool.

Summary:
And add an option to disable this behavior. The option is only used in
AllTUsExecutor to avoid races when running concurrently on multiple
threads.

This fixes PR38869 introduced by r340937.

Reviewers: ioeric, steveire

Reviewed By: ioeric

Subscribers: cfe-commits

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

Modified:
cfe/trunk/include/clang/Tooling/Tooling.h
cfe/trunk/lib/Tooling/AllTUsExecution.cpp
cfe/trunk/lib/Tooling/Tooling.cpp

Modified: cfe/trunk/include/clang/Tooling/Tooling.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Tooling.h?rev=341910&r1=341909&r2=341910&view=diff
==
--- cfe/trunk/include/clang/Tooling/Tooling.h (original)
+++ cfe/trunk/include/clang/Tooling/Tooling.h Tue Sep 11 00:29:09 2018
@@ -356,6 +356,11 @@ public:
   /// append them to ASTs.
   int buildASTs(std::vector> &ASTs);
 
+  /// Sets whether working directory should be restored after calling run(). By
+  /// default, working directory is restored. However, it could be useful to
+  /// turn this off when running on multiple threads to avoid the raciness.
+  void setRestoreWorkingDir(bool RestoreCWD);
+
   /// Returns the file manager used in the tool.
   ///
   /// The file manager is shared between all translation units.
@@ -380,6 +385,8 @@ private:
   ArgumentsAdjuster ArgsAdjuster;
 
   DiagnosticConsumer *DiagConsumer = nullptr;
+
+  bool RestoreCWD = true;
 };
 
 template 

Modified: cfe/trunk/lib/Tooling/AllTUsExecution.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/AllTUsExecution.cpp?rev=341910&r1=341909&r2=341910&view=diff
==
--- cfe/trunk/lib/Tooling/AllTUsExecution.cpp (original)
+++ cfe/trunk/lib/Tooling/AllTUsExecution.cpp Tue Sep 11 00:29:09 2018
@@ -104,7 +104,12 @@ llvm::Error AllTUsToolExecutor::execute(
   {
 llvm::ThreadPool Pool(ThreadCount == 0 ? llvm::hardware_concurrency()
: ThreadCount);
-
+llvm::SmallString<128> InitialWorkingDir;
+if (auto EC = llvm::sys::fs::current_path(InitialWorkingDir)) {
+  InitialWorkingDir = "";
+  llvm::errs() << "Error while getting current working directory: "
+   << EC.message() << "\n";
+}
 for (std::string File : Files) {
   Pool.async(
   [&](std::string Path) {
@@ -116,12 +121,19 @@ llvm::Error AllTUsToolExecutor::execute(
 for (const auto &FileAndContent : OverlayFiles)
   Tool.mapVirtualFile(FileAndContent.first(),
   FileAndContent.second);
+// Do not restore working dir from multiple threads to avoid races.
+Tool.setRestoreWorkingDir(false);
 if (Tool.run(Action.first.get()))
   AppendError(llvm::Twine("Failed to run action on ") + Path +
   "\n");
   },
   File);
 }
+if (!InitialWorkingDir.empty()) {
+  if (auto EC = llvm::sys::fs::set_current_path(InitialWorkingDir))
+llvm::errs() << "Error while restoring working directory: "
+ << EC.message() << "\n";
+}
   }
 
   if (!ErrorMsg.empty())

Modified: cfe/trunk/lib/Tooling/Tooling.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Tooling.cpp?rev=341910&r1=341909&r2=341910&view=diff
==
--- cfe/trunk/lib/Tooling/Tooling.cpp (original)
+++ cfe/trunk/lib/Tooling/Tooling.cpp Tue Sep 11 00:29:09 2018
@@ -441,6 +441,17 @@ int ClangTool::run(ToolAction *Action) {
 AbsolutePaths.push_back(std::move(*AbsPath));
   }
 
+  // Remember the working directory in case we need to restore it.
+  std::string InitialWorkingDir;
+  if (RestoreCWD) {
+if (auto CWD = OverlayFileSystem->getCurrentWorkingDirectory()) {
+  InitialWorkingDir = std::move(*CWD);
+} else {
+  llvm::errs() << "Could not get working directory: "
+   << CWD.getError().message() << "\n";
+}
+  }
+
   for (llvm::StringRef File : AbsolutePaths) {
 // Currently implementations of CompilationDatabase::getCompileCommands can
 // change the state of the file system (e.g.  prepare generated headers), 
so
@@ -508,6 +519,13 @@ int ClangTool::run(ToolAction *Action) {
   }
 }
   }
+
+  if (!InitialWorkingDir.empty()) {
+if (auto EC =
+OverlayFileSystem->setCurrentWorkingDirectory(InitialWorkingDir))
+  llvm::errs() << "Error when trying to restore working dir: "
+   << EC.message() << "\n";
+  }
   return ProcessingFailed ? 1 : (FileSkipped ? 2 : 0);
 }
 
@@ -544,6 +562,10 @@ int ClangTool::buildAS

[PATCH] D51864: [Tooling] Restore working dir in ClangTool.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC341910: [Tooling] Restore working dir in ClangTool. 
(authored by ibiryukov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51864?vs=164686&id=164816#toc

Repository:
  rC Clang

https://reviews.llvm.org/D51864

Files:
  include/clang/Tooling/Tooling.h
  lib/Tooling/AllTUsExecution.cpp
  lib/Tooling/Tooling.cpp

Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -441,6 +441,17 @@
 AbsolutePaths.push_back(std::move(*AbsPath));
   }
 
+  // Remember the working directory in case we need to restore it.
+  std::string InitialWorkingDir;
+  if (RestoreCWD) {
+if (auto CWD = OverlayFileSystem->getCurrentWorkingDirectory()) {
+  InitialWorkingDir = std::move(*CWD);
+} else {
+  llvm::errs() << "Could not get working directory: "
+   << CWD.getError().message() << "\n";
+}
+  }
+
   for (llvm::StringRef File : AbsolutePaths) {
 // Currently implementations of CompilationDatabase::getCompileCommands can
 // change the state of the file system (e.g.  prepare generated headers), so
@@ -508,6 +519,13 @@
   }
 }
   }
+
+  if (!InitialWorkingDir.empty()) {
+if (auto EC =
+OverlayFileSystem->setCurrentWorkingDirectory(InitialWorkingDir))
+  llvm::errs() << "Error when trying to restore working dir: "
+   << EC.message() << "\n";
+  }
   return ProcessingFailed ? 1 : (FileSkipped ? 2 : 0);
 }
 
@@ -544,6 +562,10 @@
   return run(&Action);
 }
 
+void ClangTool::setRestoreWorkingDir(bool RestoreCWD) {
+  this->RestoreCWD = RestoreCWD;
+}
+
 namespace clang {
 namespace tooling {
 
Index: lib/Tooling/AllTUsExecution.cpp
===
--- lib/Tooling/AllTUsExecution.cpp
+++ lib/Tooling/AllTUsExecution.cpp
@@ -104,7 +104,12 @@
   {
 llvm::ThreadPool Pool(ThreadCount == 0 ? llvm::hardware_concurrency()
: ThreadCount);
-
+llvm::SmallString<128> InitialWorkingDir;
+if (auto EC = llvm::sys::fs::current_path(InitialWorkingDir)) {
+  InitialWorkingDir = "";
+  llvm::errs() << "Error while getting current working directory: "
+   << EC.message() << "\n";
+}
 for (std::string File : Files) {
   Pool.async(
   [&](std::string Path) {
@@ -116,12 +121,19 @@
 for (const auto &FileAndContent : OverlayFiles)
   Tool.mapVirtualFile(FileAndContent.first(),
   FileAndContent.second);
+// Do not restore working dir from multiple threads to avoid races.
+Tool.setRestoreWorkingDir(false);
 if (Tool.run(Action.first.get()))
   AppendError(llvm::Twine("Failed to run action on ") + Path +
   "\n");
   },
   File);
 }
+if (!InitialWorkingDir.empty()) {
+  if (auto EC = llvm::sys::fs::set_current_path(InitialWorkingDir))
+llvm::errs() << "Error while restoring working directory: "
+ << EC.message() << "\n";
+}
   }
 
   if (!ErrorMsg.empty())
Index: include/clang/Tooling/Tooling.h
===
--- include/clang/Tooling/Tooling.h
+++ include/clang/Tooling/Tooling.h
@@ -356,6 +356,11 @@
   /// append them to ASTs.
   int buildASTs(std::vector> &ASTs);
 
+  /// Sets whether working directory should be restored after calling run(). By
+  /// default, working directory is restored. However, it could be useful to
+  /// turn this off when running on multiple threads to avoid the raciness.
+  void setRestoreWorkingDir(bool RestoreCWD);
+
   /// Returns the file manager used in the tool.
   ///
   /// The file manager is shared between all translation units.
@@ -380,6 +385,8 @@
   ArgumentsAdjuster ArgsAdjuster;
 
   DiagnosticConsumer *DiagConsumer = nullptr;
+
+  bool RestoreCWD = true;
 };
 
 template 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50171: [python] [tests] Update test_code_completion

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

Ping.


Repository:
  rC Clang

https://reviews.llvm.org/D50171



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

[PATCH] D51847: Print correctly dependency paths on Windows

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

xbolva00 added a comment.

In https://reviews.llvm.org/D51847#1228927, @zturner wrote:

> What prints this? How do you exercise this codepath?


DFGImpl::OutputDependencyFile() -> for (StringRef File : Files)


https://reviews.llvm.org/D51847



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

[PATCH] D51860: [clangd] NFC: Use uint32_t for FuzzyFindRequest limits

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 164821.
kbobyrev marked an inline comment as done.

https://reviews.llvm.org/D51860

Files:
  clang-tools-extra/clangd/index/Index.cpp
  clang-tools-extra/clangd/index/Index.h


Index: clang-tools-extra/clangd/index/Index.h
===
--- clang-tools-extra/clangd/index/Index.h
+++ clang-tools-extra/clangd/index/Index.h
@@ -437,7 +437,7 @@
   std::vector Scopes;
   /// \brief The number of top candidates to return. The index may choose to
   /// return more than this, e.g. if it doesn't know which candidates are best.
-  size_t MaxCandidateCount = std::numeric_limits::max();
+  uint32_t MaxCandidateCount = std::numeric_limits::max();
   /// If set to true, only symbols for completion support will be considered.
   bool RestrictForCodeCompletion = false;
   /// Contextually relevant files (e.g. the file we're code-completing in).
Index: clang-tools-extra/clangd/index/Index.cpp
===
--- clang-tools-extra/clangd/index/Index.cpp
+++ clang-tools-extra/clangd/index/Index.cpp
@@ -177,29 +177,25 @@
 
 bool fromJSON(const llvm::json::Value &Parameters, FuzzyFindRequest &Request) {
   json::ObjectMapper O(Parameters);
-  llvm::Optional MaxCandidateCount;
+  int64_t MaxCandidateCount;
   bool OK =
   O && O.map("Query", Request.Query) && O.map("Scopes", Request.Scopes) &&
+  O.map("MaxCandidateCount", MaxCandidateCount) &&
   O.map("RestrictForCodeCompletion", Request.RestrictForCodeCompletion) &&
-  O.map("ProximityPaths", Request.ProximityPaths) &&
-  O.map("MaxCandidateCount", MaxCandidateCount);
-  if (MaxCandidateCount)
-Request.MaxCandidateCount = MaxCandidateCount.getValue();
+  O.map("ProximityPaths", Request.ProximityPaths);
+  if (OK && MaxCandidateCount <= std::numeric_limits::max())
+Request.MaxCandidateCount = MaxCandidateCount;
   return OK;
 }
 
 llvm::json::Value toJSON(const FuzzyFindRequest &Request) {
-  auto Result = json::Object{
+  return json::Object{
   {"Query", Request.Query},
   {"Scopes", json::Array{Request.Scopes}},
+  {"MaxCandidateCount", Request.MaxCandidateCount},
   {"RestrictForCodeCompletion", Request.RestrictForCodeCompletion},
   {"ProximityPaths", json::Array{Request.ProximityPaths}},
   };
-  // A huge limit means no limit, leave it out.
-  if (Request.MaxCandidateCount <= std::numeric_limits::max())
-Result["MaxCandidateCount"] =
-static_cast(Request.MaxCandidateCount);
-  return std::move(Result);
 }
 
 bool SwapIndex::fuzzyFind(const FuzzyFindRequest &R,


Index: clang-tools-extra/clangd/index/Index.h
===
--- clang-tools-extra/clangd/index/Index.h
+++ clang-tools-extra/clangd/index/Index.h
@@ -437,7 +437,7 @@
   std::vector Scopes;
   /// \brief The number of top candidates to return. The index may choose to
   /// return more than this, e.g. if it doesn't know which candidates are best.
-  size_t MaxCandidateCount = std::numeric_limits::max();
+  uint32_t MaxCandidateCount = std::numeric_limits::max();
   /// If set to true, only symbols for completion support will be considered.
   bool RestrictForCodeCompletion = false;
   /// Contextually relevant files (e.g. the file we're code-completing in).
Index: clang-tools-extra/clangd/index/Index.cpp
===
--- clang-tools-extra/clangd/index/Index.cpp
+++ clang-tools-extra/clangd/index/Index.cpp
@@ -177,29 +177,25 @@
 
 bool fromJSON(const llvm::json::Value &Parameters, FuzzyFindRequest &Request) {
   json::ObjectMapper O(Parameters);
-  llvm::Optional MaxCandidateCount;
+  int64_t MaxCandidateCount;
   bool OK =
   O && O.map("Query", Request.Query) && O.map("Scopes", Request.Scopes) &&
+  O.map("MaxCandidateCount", MaxCandidateCount) &&
   O.map("RestrictForCodeCompletion", Request.RestrictForCodeCompletion) &&
-  O.map("ProximityPaths", Request.ProximityPaths) &&
-  O.map("MaxCandidateCount", MaxCandidateCount);
-  if (MaxCandidateCount)
-Request.MaxCandidateCount = MaxCandidateCount.getValue();
+  O.map("ProximityPaths", Request.ProximityPaths);
+  if (OK && MaxCandidateCount <= std::numeric_limits::max())
+Request.MaxCandidateCount = MaxCandidateCount;
   return OK;
 }
 
 llvm::json::Value toJSON(const FuzzyFindRequest &Request) {
-  auto Result = json::Object{
+  return json::Object{
   {"Query", Request.Query},
   {"Scopes", json::Array{Request.Scopes}},
+  {"MaxCandidateCount", Request.MaxCandidateCount},
   {"RestrictForCodeCompletion", Request.RestrictForCodeCompletion},
   {"ProximityPaths", json::Array{Request.ProximityPaths}},
   };
-  // A huge limit means no limit, leave it out.
-  if (Request.MaxCandidateCount <= std::numeric_limits::max())
-Result["MaxCandidateCount"] =
-static_cast(Request.MaxCandidateCount)

[PATCH] D51860: [clangd] NFC: Use uint32_t for FuzzyFindRequest limits

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/index/Index.h:440
   /// return more than this, e.g. if it doesn't know which candidates are best.
-  size_t MaxCandidateCount = std::numeric_limits::max();
+  uint32_t MaxCandidateCount = std::numeric_limits::max();
   /// If set to true, only symbols for completion support will be considered.

ioeric wrote:
> Or use `unsigned`?
`unsigned` would have different size on different platforms, I'm not really 
sure we want that; could you elaborate on why you think that would be better?


https://reviews.llvm.org/D51860



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

[PATCH] D51860: [clangd] NFC: Use uint32_t for FuzzyFindRequest limits

[Eric Liu via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/index/Index.h:440
   /// return more than this, e.g. if it doesn't know which candidates are best.
-  size_t MaxCandidateCount = std::numeric_limits::max();
+  uint32_t MaxCandidateCount = std::numeric_limits::max();
   /// If set to true, only symbols for completion support will be considered.

kbobyrev wrote:
> ioeric wrote:
> > Or use `unsigned`?
> `unsigned` would have different size on different platforms, I'm not really 
> sure we want that; could you elaborate on why you think that would be better?
I thought it's (almost) always 4 bytes? But it should always have a smaller 
size than `uint64_t` in json serialization, so it should work for us. In 
general, I would prefer `unsigned` to `uint32_t` when possible. For most of the 
platforms, they are the same. But up to you :) I don't really feel strong about 
this.


https://reviews.llvm.org/D51860



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

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

[Takuto Ikuta via Phabricator via cfe-commits]

takuto.ikuta updated this revision to Diff 164822.
takuto.ikuta added a comment.

I'm trying to handle local static var correctly.


https://reviews.llvm.org/D51340

Files:
  clang/include/clang/Driver/CLCompatOptions.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/CodeGenCXX/dllexport-import-fvisibility-inlines-hidden.cpp
  clang/test/CodeGenCXX/hidden-dllimport.cpp

Index: clang/test/CodeGenCXX/hidden-dllimport.cpp
===
--- clang/test/CodeGenCXX/hidden-dllimport.cpp
+++ clang/test/CodeGenCXX/hidden-dllimport.cpp
@@ -1,8 +1,8 @@
 // RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -emit-llvm -fvisibility-inlines-hidden -o - %s | FileCheck %s
 
-// We used to declare this hidden dllimport, which is contradictory.
+// We don't declare this hidden dllimport.
 
-// CHECK: declare dllimport void @"?bar@foo@@QEAAXXZ"(%struct.foo*)
+// CHECK-NOT: declare dllimport void @"?bar@foo@@QEAAXXZ"(%struct.foo*)
 
 struct __attribute__((dllimport)) foo {
   void bar() {}
Index: clang/test/CodeGenCXX/dllexport-import-fvisibility-inlines-hidden.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-import-fvisibility-inlines-hidden.cpp
@@ -0,0 +1,162 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc   \
+// RUN: -fvisibility-inlines-hidden -emit-llvm -O0 -o - |   \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc   \
+// RUN: -emit-llvm -O0 -o - |   \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// NOINLINE-DAG: define weak_odr hidden dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+// INLINE-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+  static int static_variable = 0;
+  ++static_variable;
+  return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+  static int static_variable = 0;
+  ++static_variable;
+  return static_variable;
+}
+
+int ImportedFunctionUser() {
+  return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// DEFAULT-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+// DEFAULT-DAG: @"?static_variable@?1??InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// local static var in ImportedClass
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+// INLINE-NOT: static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+  // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+  NoTemplateExportedClass() = default;
+
+  // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+  // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+  void InclassDefFunc() {}
+
+  int f();
+
+  // INLINE-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+  // NOINLINE-DAG: define weak_odr hidden dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+  int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+  }
+
+  // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+  __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+  // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+  __forceinline void InlineOutclassDefFunc();
+
+  // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+  __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+  // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+  void OutclassDefFunc();
+};
+
+vo

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

[Takuto Ikuta via Phabricator via cfe-commits]

takuto.ikuta added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5244
+   false))
+CmdArgs.push_back("-fvisibility-inlines-hidden");
+

hans wrote:
> takuto.ikuta wrote:
> > hans wrote:
> > > Huh, does this actually affect whether functions get dllexported or not?
> > Sorry, what you want to ask?
> > 
> > This will used to not add dllexport attr in L5690 of SemaDeclCXX.cpp.
> > 
> Oops, I didn't see that. I'm glad to see this is looking so simple :-)
> 
> Actually, I don't think we should the same flag name for this, since "hidden" 
> is an ELF concept, not a COFF one, just that we should match the behaviour of 
> the flag.
> 
> Hmm, or do people use -fvisibility-inlines-hidden on MinGW or something? 
> Where does the hidden-dllimport.cpp file come from?
> 
> Also, is it the case that -fvisibility-inlines-hidden just ignores the 
> problem of static local variables? If that's the case we can probably do it 
> too, we just have to be sure, and document it eventually.
> 
I confirmed that -fvisibility-inlines-hidden treats local static var correctly 
in linux.
So I'm trying to export inline functions if it has local static variables.


https://reviews.llvm.org/D51340



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

[PATCH] D51898: Revert "[clang-tidy] Handle unresolved expressions in ExprMutationAnalyzer"

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

i had an issue with msvc before, where `-fno-delayed-template-parsing` had to 
be added to the compilation, because MSVC did not have uninstantiated 
templates. Maybe this could be similar? But i am not sure how to resolve the 
issue here, as you probably can't pass in flags.
Maybe calling the template functions to instantiate them would be a possibility.


Repository:
  rL LLVM

https://reviews.llvm.org/D51898



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

[PATCH] D50171: [python] [tests] Update test_code_completion

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Also can't explain why `const` and `volatile` have a different priority now.
The `P::` and `Q::` seem to be completely different completion items from `P` 
and `Q` (wildly different priorities suggest they're not the same), can't 
explain what caused the first ones not being added anymore.

Happy to LGTM to unbreak tests, but won't have time to dig up the commits that 
introduced these changes.

PS Could you upload the diff with full context?


Repository:
  rC Clang

https://reviews.llvm.org/D50171



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

[PATCH] D51860: [clangd] NFC: Use uint32_t for FuzzyFindRequest limits

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/index/Index.h:440
   /// return more than this, e.g. if it doesn't know which candidates are best.
-  size_t MaxCandidateCount = std::numeric_limits::max();
+  uint32_t MaxCandidateCount = std::numeric_limits::max();
   /// If set to true, only symbols for completion support will be considered.

ioeric wrote:
> kbobyrev wrote:
> > ioeric wrote:
> > > Or use `unsigned`?
> > `unsigned` would have different size on different platforms, I'm not really 
> > sure we want that; could you elaborate on why you think that would be 
> > better?
> I thought it's (almost) always 4 bytes? But it should always have a smaller 
> size than `uint64_t` in json serialization, so it should work for us. In 
> general, I would prefer `unsigned` to `uint32_t` when possible. For most of 
> the platforms, they are the same. But up to you :) I don't really feel strong 
> about this.
BTW, many people think using unsigned ints just because inputs can't be 
negative is a bad idea.
See https://stackoverflow.com/a/18796234


https://reviews.llvm.org/D51860



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

[PATCH] D50953: [clang-tidy] Handle sugared reference types in ExprMutationAnalyzer

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In https://reviews.llvm.org/D50953#1230003, @shuaiwang wrote:

> In https://reviews.llvm.org/D50953#1229287, @JonasToth wrote:
>
> > What happens to pointers in a typedef (in the sense of `*` instead of `&`)?
>
>
> I checked around and I believe reference type is the only type we're 
> explicitly matching right now. We'll need to handle carefully when handling 
> pointer types in the future.


We match on pointers as values. So figure this out `int * >const< ptr = 
nullptr`.
And the constness transformation is especially intersting for pointer typedefs, 
because in `typedef int * IntPtr; const IntPtr p1; IntPtr const p2;` p1 and p2 
are  different things. It would be nice if you could check, that the value 
semantic of the pointer is detected through the typedef as well.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50953



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

[PATCH] D51917: [CodeCompletion] Enable signature help when initializing class/struct/union members.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added reviewers: sammccall, ilya-biryukov, ioeric.
Herald added a subscriber: cfe-commits.

Factors out member decleration gathering and uses it in parsing to call 
signature
help. Doesn't support signature help for base class constructors, the code was 
too
coupled with diagnostic handling, but still can be factored out but just needs
more afford.


Repository:
  rC Clang

https://reviews.llvm.org/D51917

Files:
  include/clang/Sema/Sema.h
  lib/Parse/ParseDeclCXX.cpp
  lib/Sema/SemaDeclCXX.cpp
  test/CodeCompletion/ctor-initializer.cpp

Index: test/CodeCompletion/ctor-initializer.cpp
===
--- test/CodeCompletion/ctor-initializer.cpp
+++ test/CodeCompletion/ctor-initializer.cpp
@@ -64,3 +64,26 @@
   // CHECK-CC8: COMPLETION: Pattern : member2(<#args#>
   int member1, member2;
 };
+
+struct Base2 {
+  Base2(int);
+};
+
+struct Composition1 {
+  Composition1() : b2_elem() {}
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:73:28 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:73:28 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // CHECK-CC9: OVERLOAD: Base2(<#int#>)
+  // CHECK-CC9: OVERLOAD: Base2(<#const Base2 &#>)
+  Composition1(Base2);
+  Base2 b2_elem;
+};
+
+struct Composition2 {
+  Composition2() : c1_elem(Base2(1)) {}
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:83:34 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:83:34 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:83:35 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:83:35 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  Composition1 c1_elem;
+};
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -3777,6 +3777,23 @@
 
 }
 
+ValueDecl *clang::tryGetMember(CXXRecordDecl *ClassDecl, CXXScopeSpec &SS,
+ParsedType TemplateTypeTy,
+IdentifierInfo *MemberOrBase) {
+  if (!SS.getScopeRep() && !TemplateTypeTy) {
+// Look for a member, first.
+DeclContext::lookup_result Result = ClassDecl->lookup(MemberOrBase);
+if (!Result.empty()) {
+  ValueDecl *Member;
+  if ((Member = dyn_cast(Result.front())) ||
+  (Member = dyn_cast(Result.front( {
+return Member;
+  }
+}
+  }
+  return nullptr;
+}
+
 /// Handle a C++ member initializer.
 MemInitResult
 Sema::BuildMemInitializer(Decl *ConstructorD,
@@ -3820,21 +3837,14 @@
   //   of a single identifier refers to the class member. A
   //   mem-initializer-id for the hidden base class may be specified
   //   using a qualified name. ]
-  if (!SS.getScopeRep() && !TemplateTypeTy) {
-// Look for a member, first.
-DeclContext::lookup_result Result = ClassDecl->lookup(MemberOrBase);
-if (!Result.empty()) {
-  ValueDecl *Member;
-  if ((Member = dyn_cast(Result.front())) ||
-  (Member = dyn_cast(Result.front( {
-if (EllipsisLoc.isValid())
-  Diag(EllipsisLoc, diag::err_pack_expansion_member_init)
-<< MemberOrBase
-<< SourceRange(IdLoc, Init->getSourceRange().getEnd());
-
-return BuildMemberInitializer(Member, Init, IdLoc);
-  }
-}
+  if (ValueDecl *Member =
+  tryGetMember(ClassDecl, SS, TemplateTypeTy, MemberOrBase)) {
+if (EllipsisLoc.isValid())
+  Diag(EllipsisLoc, diag::err_pack_expansion_member_init)
+  << MemberOrBase
+  << SourceRange(IdLoc, Init->getSourceRange().getEnd());
+
+return BuildMemberInitializer(Member, Init, IdLoc);
   }
   // It didn't name a member, so see if it names a class.
   QualType BaseType;
Index: lib/Parse/ParseDeclCXX.cpp
===
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -3449,6 +3449,7 @@
   if (getLangOpts().CPlusPlus11 && Tok.is(tok::l_brace)) {
 Diag(Tok, diag::warn_cxx98_compat_generalized_initializer_lists);
 
+// FIXME: Add support for signature help inside initializer lists.
 ExprResult InitList = ParseBraceInitializer();
 if (InitList.isInvalid())
   return true;
@@ -3466,7 +3467,26 @@
 // Parse the optional expression-list.
 ExprVector ArgExprs;
 CommaLocsTy CommaLocs;
-if (Tok.isNot(tok::r_paren) && ParseExpressionList(ArgExprs, CommaLocs)) {
+auto SignatureHelpCaller = [&] {
+  if (CalledSignatureHelp)
+return;
+  CXXConstructorDecl *Constructor =
+  dyn_cast(ConstructorDecl);
+  if (!Constructor)
+return;
+  // FIXME: Add support for Base class constructors as well.
+   

[PATCH] D51860: [clangd] NFC: Use uint32_t for FuzzyFindRequest limits

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/index/Index.h:440
   /// return more than this, e.g. if it doesn't know which candidates are best.
-  size_t MaxCandidateCount = std::numeric_limits::max();
+  uint32_t MaxCandidateCount = std::numeric_limits::max();
   /// If set to true, only symbols for completion support will be considered.

ilya-biryukov wrote:
> ioeric wrote:
> > kbobyrev wrote:
> > > ioeric wrote:
> > > > Or use `unsigned`?
> > > `unsigned` would have different size on different platforms, I'm not 
> > > really sure we want that; could you elaborate on why you think that would 
> > > be better?
> > I thought it's (almost) always 4 bytes? But it should always have a smaller 
> > size than `uint64_t` in json serialization, so it should work for us. In 
> > general, I would prefer `unsigned` to `uint32_t` when possible. For most of 
> > the platforms, they are the same. But up to you :) I don't really feel 
> > strong about this.
> BTW, many people think using unsigned ints just because inputs can't be 
> negative is a bad idea.
> See https://stackoverflow.com/a/18796234
I mostly agree with that, but LLVM uses unsigned types pervasively, and 
-Wsign-compare, so they're hard to avoid.

(FWIW, I still think that this case has become complicated enough that we 
should use the most explicit option, which seems like Optional here)


https://reviews.llvm.org/D51860



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

[PATCH] D50883: [clang-tidy] Handle unique owning smart pointers in ExprMutationAnalyzer

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp:658
+ "void f() { UniquePtr x; x->mf(); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));

shuaiwang wrote:
> JonasToth wrote:
> > Template testcases i miss:
> > 
> > ```
> > // modifying
> > template 
> > void f() { UnqiuePtr x; x->mf(); }
> > 
> > // constant
> > template 
> > void f2() { UnqiuePtr x; x->cmf(); }
> > 
> > // indecidable for the template itself, but only the instantiations
> > template 
> > void f3() { T x; x->cmf(); }
> > 
> > struct const_class { void cmf() const; }
> > struct modifying_class { void cmf(); };
> > 
> > void call_template() {
> > // don't trigger
> > f3>();
> > // trigger modification
> > f3();
> > }
> > 
> > // again not decidable by the template itself
> > template 
> > void f4() { T t; *t; }
> > 
> > struct very_weird {
> > int& operator*() { return *new int(42); }
> > };
> > void call_template_deref() {
> >   // no modification
> >   f4();
> >   // modification, because deref is not const
> >   f4>():
> > }
> > ```
> Added a case with template. However I don't think we can do much whenever 
> template appears: only the AST of **uninstantiated** template worth 
> analyzing, because whatever analyze result (diag or fixit) would have to be 
> applied to the template itself not the instantiations.
> So the fact that the template argument of `f3` or `f4` could happen to be 
> `UniquePtr` doesn't really matter when we analyze `f3` and `f4`, we have to 
> analyze the way assuming the "worst" anyway.
Yes, whenever there is something type-dependent we have to bail, thats why i 
would like to see the testcases to show that we actually bail, even when one 
instantiation would not modify.

I believe we do analyze all versions of the templated functions (uninstantiated 
and every instantiation), don't we? With that there can be conflicting results.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50883



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

[PATCH] D51297: [docs] Create a guide for Vim users on how to set up Clangd

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In https://reviews.llvm.org/D51297#1228725, @ioeric wrote:

> I also support having some instructions/pointers on editor integration. That 
> said, I think we should have a section "Editor integration" with a list of 
> editor clients that are known to work with clangd, instead of having a 
> section just for vim. Something like:
>
>   #Editor (or client?) integration
>  
>   ##Vim
>   Some LSP clients that are known to work with clangd:
>- nvim, LanguageClient-neovim ..
>- ...
>  
>   ## vscode
>  
>   ## emacs?
>  
>
>
> What do you think?


Sorry I am trespassing, just wanted to say +1 to this.


https://reviews.llvm.org/D51297



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

[PATCH] D51391: [clang-cl,PCH] Add support for #pragma hdrstop

[Hans Wennborg via Phabricator via cfe-commits]

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

lgtm!

Please add a note to docs/ReleaseNotes.rst when landing.


https://reviews.llvm.org/D51391



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

[PATCH] D51860: [clangd] NFC: Use uint32_t for FuzzyFindRequest limits

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clang-tools-extra/clangd/index/Index.h:440
   /// return more than this, e.g. if it doesn't know which candidates are best.
-  size_t MaxCandidateCount = std::numeric_limits::max();
+  uint32_t MaxCandidateCount = std::numeric_limits::max();
   /// If set to true, only symbols for completion support will be considered.

sammccall wrote:
> ilya-biryukov wrote:
> > ioeric wrote:
> > > kbobyrev wrote:
> > > > ioeric wrote:
> > > > > Or use `unsigned`?
> > > > `unsigned` would have different size on different platforms, I'm not 
> > > > really sure we want that; could you elaborate on why you think that 
> > > > would be better?
> > > I thought it's (almost) always 4 bytes? But it should always have a 
> > > smaller size than `uint64_t` in json serialization, so it should work for 
> > > us. In general, I would prefer `unsigned` to `uint32_t` when possible. 
> > > For most of the platforms, they are the same. But up to you :) I don't 
> > > really feel strong about this.
> > BTW, many people think using unsigned ints just because inputs can't be 
> > negative is a bad idea.
> > See https://stackoverflow.com/a/18796234
> I mostly agree with that, but LLVM uses unsigned types pervasively, and 
> -Wsign-compare, so they're hard to avoid.
> 
> (FWIW, I still think that this case has become complicated enough that we 
> should use the most explicit option, which seems like Optional here)
The complication seems to be mostly in json serialization of the field, which 
is more of an implementation detail I think. If we could get away with using 
max unsigned as no limit, which seems to have worked well so far, I would still 
prefer it over `Optional`, as it is less confusing on the reference site and 
involves less refactoring.


https://reviews.llvm.org/D51860



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

[PATCH] D51917: [CodeCompletion] Enable signature help when initializing class/struct/union members.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: include/clang/Sema/Sema.h:10798
 
+ValueDecl *tryGetMember(CXXRecordDecl *ClassDecl, CXXScopeSpec &SS,
+ParsedType TemplateTypeTy,

The name is very generic, but the helper is only applicable to our specific 
case (looking up ctor member initializer). Maybe choose a different name?
Something like `tryLookupCtorInitMemberDecl` would be closer to what it 
actually does.



Comment at: lib/Parse/ParseDeclCXX.cpp:3471
+auto SignatureHelpCaller = [&] {
+  if (CalledSignatureHelp)
+return;

We're calling completion in other instances, this gives us valuable information 
(preferred type).
Why not do the same in this case?



Comment at: lib/Parse/ParseDeclCXX.cpp:3473
+return;
+  CXXConstructorDecl *Constructor =
+  dyn_cast(ConstructorDecl);

That's a lot of code in parser.

We could probably extract this as a helper in Sema (similar to 
`ProduceConstructorSignatureHelp`) to follow the same pattern that we have for 
all the other signature help calls.
Would keep the parser simpler and, as an added benefit, would allow to make 
`tryGetMember` helper private (as it should be).
WDYT?


Repository:
  rC Clang

https://reviews.llvm.org/D51917



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

[PATCH] D51090: [clangd] Add index benchmarks

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Nice and simple :-) Looks good, just some details.




Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:1
+//===--- DexBenchmark.cpp - DexIndex benchmarks -*- C++ 
-*-===//
+//

rename? (it's not just dex now, right?)



Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:21
+
+std::string IndexFilename;
+std::string LogFilename;

const char* (no globals with nontrivial constructors/destructors)



Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:25
+
+std::unique_ptr buildMem() {
+  return clang::clangd::loadIndex(IndexFilename, {}, false);

these should be in ns clang::clangd:: I think?



Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:26
+std::unique_ptr buildMem() {
+  return clang::clangd::loadIndex(IndexFilename, {}, false);
+}

This includes reading the file contents, which I'm not sure is a useful part of 
the BuildDex() benchmark.

You could consider splitting into loadIndex that takes content and 
loadIndexFromFile that takes a filename. The former would be more appropriate 
to time.



Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:33
+
+std::vector extractQueriesFromLogs() {
+  llvm::Regex RequestMatcher("fuzzyFind\\(\"([a-zA-Z]*)\", scopes=\\[(.*)\\]");

comment?



Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:65
+std::vector generateArtificialRequests() {
+  std::vector Requests;
+  // FXIME(kbobyrev): Add more requests.

this seems a little weird - the extractQueriesFromLogs/buildDex seem to be 
agnostic to the index contents, but here we assume knowledge of it being LLVM.

The former model is nicer, and we can extend it by using the JSON 
representation to add more query attributes. Can we drop this function, and 
`ArtificialQueries->Queries`?



Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:104
+namespace clangd {
+namespace dex {
+

remove dex if this isn't dex-specific?



Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:132
+  for (auto _ : State)
+buildDex();
+}

BTW this also counts *destruction* time. I think that's fine, though.


https://reviews.llvm.org/D51090



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

[PATCH] D51860: [clangd] NFC: Use uint32_t for FuzzyFindRequest limits

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/index/Index.h:440
   /// return more than this, e.g. if it doesn't know which candidates are best.
-  size_t MaxCandidateCount = std::numeric_limits::max();
+  uint32_t MaxCandidateCount = std::numeric_limits::max();
   /// If set to true, only symbols for completion support will be considered.

ioeric wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > ioeric wrote:
> > > > kbobyrev wrote:
> > > > > ioeric wrote:
> > > > > > Or use `unsigned`?
> > > > > `unsigned` would have different size on different platforms, I'm not 
> > > > > really sure we want that; could you elaborate on why you think that 
> > > > > would be better?
> > > > I thought it's (almost) always 4 bytes? But it should always have a 
> > > > smaller size than `uint64_t` in json serialization, so it should work 
> > > > for us. In general, I would prefer `unsigned` to `uint32_t` when 
> > > > possible. For most of the platforms, they are the same. But up to you 
> > > > :) I don't really feel strong about this.
> > > BTW, many people think using unsigned ints just because inputs can't be 
> > > negative is a bad idea.
> > > See https://stackoverflow.com/a/18796234
> > I mostly agree with that, but LLVM uses unsigned types pervasively, and 
> > -Wsign-compare, so they're hard to avoid.
> > 
> > (FWIW, I still think that this case has become complicated enough that we 
> > should use the most explicit option, which seems like Optional here)
> The complication seems to be mostly in json serialization of the field, which 
> is more of an implementation detail I think. If we could get away with using 
> max unsigned as no limit, which seems to have worked well so far, I would 
> still prefer it over `Optional`, as it is less confusing on the reference 
> site and involves less refactoring.
+1 to using Optional, this states the contract better and actually gives a 
clear model of how to should be serialized and deserialized.

Can see the logic in using large values for the limits too, but optional seems 
to avoid various corner cases, which outweighs the refactoring costs IMO.

Feel free to ignore, though, just a drive-by-comment.


https://reviews.llvm.org/D51860



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

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 164830.
kadircet marked 5 inline comments as done.
kadircet added a comment.

- Change flag's name and rebase.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51214

Files:
  clangd/tool/ClangdMain.cpp


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -168,6 +168,19 @@
 "'compile_commands.json' files")),
 llvm::cl::init(FilesystemCompileArgs), llvm::cl::Hidden);
 
+static llvm::cl::opt IncludeFixIts(
+"completions-with-fixes",
+llvm::cl::desc(
+"Enables suggestion of completion items that needs additional changes. 
"
+"Like changing an arrow(->) member access to dot(.) member access."),
+llvm::cl::init(clangd::CodeCompleteOptions().IncludeFixIts));
+
+static llvm::cl::opt EnableFunctionArgSnippets(
+"enable-function-arg-snippets",
+llvm::cl::desc("Gives snippets for function arguments, when disabled only "
+   "gives parantheses for the call."),
+llvm::cl::init(clangd::CodeCompleteOptions().EnableFunctionArgSnippets));
+
 int main(int argc, char *argv[]) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   llvm::cl::SetVersionPrinter([](llvm::raw_ostream &OS) {
@@ -295,6 +308,8 @@
 CCOpts.IncludeIndicator.NoInsert.clear();
   }
   CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
+  CCOpts.IncludeFixIts = IncludeFixIts;
+  CCOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
 
   // Initialize and run ClangdLSPServer.
   ClangdLSPServer LSPServer(


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -168,6 +168,19 @@
 "'compile_commands.json' files")),
 llvm::cl::init(FilesystemCompileArgs), llvm::cl::Hidden);
 
+static llvm::cl::opt IncludeFixIts(
+"completions-with-fixes",
+llvm::cl::desc(
+"Enables suggestion of completion items that needs additional changes. "
+"Like changing an arrow(->) member access to dot(.) member access."),
+llvm::cl::init(clangd::CodeCompleteOptions().IncludeFixIts));
+
+static llvm::cl::opt EnableFunctionArgSnippets(
+"enable-function-arg-snippets",
+llvm::cl::desc("Gives snippets for function arguments, when disabled only "
+   "gives parantheses for the call."),
+llvm::cl::init(clangd::CodeCompleteOptions().EnableFunctionArgSnippets));
+
 int main(int argc, char *argv[]) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   llvm::cl::SetVersionPrinter([](llvm::raw_ostream &OS) {
@@ -295,6 +308,8 @@
 CCOpts.IncludeIndicator.NoInsert.clear();
   }
   CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
+  CCOpts.IncludeFixIts = IncludeFixIts;
+  CCOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
 
   // Initialize and run ClangdLSPServer.
   ClangdLSPServer LSPServer(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

LG from my side, @sammccall is not a big fan of options so please wait for his 
approval too




Comment at: clangd/tool/ClangdMain.cpp:174
+llvm::cl::desc(
+"Enables suggestion of completion items that needs additional changes. 
"
+"Like changing an arrow(->) member access to dot(.) member access."),

s/needs/need



Comment at: clangd/tool/ClangdMain.cpp:180
+"enable-function-arg-snippets",
+llvm::cl::desc("Gives snippets for function arguments, when disabled only "
+   "gives parantheses for the call."),

Maybe mention code completion in the comment?
Something like:
> When enabled, completion snippets insert full function signatures.
> When disabled, completion snippets insert only parentheses for the call.



Comment at: clangd/tool/ClangdMain.cpp:181
+llvm::cl::desc("Gives snippets for function arguments, when disabled only "
+   "gives parantheses for the call."),
+llvm::cl::init(clangd::CodeCompleteOptions().EnableFunctionArgSnippets));

s/parantheses/parentheses


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51214



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

[PATCH] D51628: [clangd] Implement a Proof-of-Concept tool for symbol index exploration

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 164834.
kbobyrev marked 8 inline comments as done.
kbobyrev added a comment.

Address a round of comments; implement a dummy ad-hoc subcommand parser.


https://reviews.llvm.org/D51628

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/dexplorer/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp

Index: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp
@@ -0,0 +1,186 @@
+//===--- DExplorer.cpp - Dex Exploration tool ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file implements a simple interactive tool which can be used to manually
+// evaluate symbol search quality of Clangd index.
+//
+//===--===//
+
+#include "../../../index/SymbolYAML.h"
+#include "../Dex.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSwitch.h"
+#include "llvm/LineEditor/LineEditor.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Signals.h"
+#include "llvm/Support/YAMLTraits.h"
+#include 
+
+using clang::clangd::FuzzyFindRequest;
+using clang::clangd::loadIndex;
+using clang::clangd::Symbol;
+using clang::clangd::SymbolIndex;
+using llvm::StringRef;
+
+namespace {
+
+llvm::cl::opt
+SymbolCollection("symbol-collection-file",
+ llvm::cl::desc("Path to the file with symbol collection"),
+ llvm::cl::Positional, llvm::cl::Required);
+
+static const std::string Overview = R"(
+This is an **experimental** interactive tool to process user-provided search
+queries over given symbol collection obtained via global-symbol-builder. The
+tool can be used to evaluate search quality of existing index implementations
+and manually construct non-trivial test cases.
+
+Type use "help" request to get information about the details.
+)";
+
+template  std::string unitToString() { return ""; }
+
+template <> std::string unitToString() {
+  return "ms";
+}
+
+template <> std::string unitToString() { return "s"; }
+
+template 
+void reportTime(StringRef Name, Function F) {
+  const auto TimerStart = std::chrono::high_resolution_clock::now();
+  F();
+  const auto TimerStop = std::chrono::high_resolution_clock::now();
+  llvm::outs()
+  << Name << " took "
+  << std::chrono::duration_cast(TimerStop - TimerStart).count()
+  << ' ' << unitToString() << ".\n";
+}
+
+void fuzzyFindRequest(const std::unique_ptr &Index,
+  const FuzzyFindRequest &Request) {
+  std::vector Symbols;
+  Index->fuzzyFind(Request, [&](const Symbol &S) { Symbols.push_back(S); });
+  // FIXME(kbobyrev): Print symbol final scores to see the distribution.
+  llvm::outs() << "\nRetrieved Symbols:\n";
+  llvm::outs() << "Symbol Name\n";
+  for (const auto &Symbol : Symbols)
+llvm::outs() << Symbol.Name << '\n';
+  llvm::outs() << '\n';
+}
+
+static const std::string HelpMessage = R"(
+DExplorer commands:
+
+> fuzzy-find Name
+
+Constructs fuzzy find request given unqualified symbol name and returns top 10
+symbols retrieved from index.
+
+> lookup-id SymbolID
+
+Retrieves symbol names given USR.
+
+> help
+
+Shows this message.
+
+Press Ctrl-D to exit.
+)";
+
+void helpRequest() { llvm::outs() << HelpMessage; }
+
+void lookupRequest(const std::unique_ptr &Index,
+   clang::clangd::LookupRequest &Request) {
+  std::vector Symbols;
+  Index->lookup(Request, [&](const Symbol &S) { Symbols.push_back(S); });
+  // FIXME(kbobyrev): Print symbol final scores to see the distribution.
+  llvm::outs() << "\nRetrieved Symbols:\n";
+  llvm::outs() << "Rank. Symbol Name | Symbol ID\n";
+  for (size_t Rank = 0; Rank < Symbols.size(); ++Rank)
+llvm::outs() << Rank << ". " << Symbols[Rank].Name << " | "
+ << Symbols[Rank].ID.str() << '\n';
+  llvm::outs() << '\n';
+}
+
+// FIXME(kbobyrev): Make this an actual REPL: probably use LLVM Command Line
+// library for parsing flags and arguments.
+// FIXME(kbobyrev): Ideas for commands:
+// * symbol lookup: print out symbol in YAML format given SymbolID
+// * find symbol references: print set of reference locations
+// * load/swap/reload index: this would make it possible to get rid of llvm::cl
+//   usages in the tool driver and actually use llvm::cl library in the REPL.
+// * show posting list density histogram (our dump data somewhere so that user
+//   could build one)
+// * show number of tokens of each kind
+// * print out tokens with the most dense posting lists
+// * print out tokens with least dense 

[PATCH] D51860: [clangd] NFC: Use uint32_t for FuzzyFindRequest limits

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 164836.

https://reviews.llvm.org/D51860

Files:
  clang-tools-extra/clangd/index/Index.cpp
  clang-tools-extra/clangd/index/Index.h


Index: clang-tools-extra/clangd/index/Index.h
===
--- clang-tools-extra/clangd/index/Index.h
+++ clang-tools-extra/clangd/index/Index.h
@@ -437,7 +437,9 @@
   std::vector Scopes;
   /// \brief The number of top candidates to return. The index may choose to
   /// return more than this, e.g. if it doesn't know which candidates are best.
-  size_t MaxCandidateCount = std::numeric_limits::max();
+  // FIXME: Use llvm::Optional; semantically, the absence of MaxCandidateCount
+  // is equivalent to setting this field to default value as below.
+  uint32_t MaxCandidateCount = std::numeric_limits::max();
   /// If set to true, only symbols for completion support will be considered.
   bool RestrictForCodeCompletion = false;
   /// Contextually relevant files (e.g. the file we're code-completing in).
Index: clang-tools-extra/clangd/index/Index.cpp
===
--- clang-tools-extra/clangd/index/Index.cpp
+++ clang-tools-extra/clangd/index/Index.cpp
@@ -177,29 +177,25 @@
 
 bool fromJSON(const llvm::json::Value &Parameters, FuzzyFindRequest &Request) {
   json::ObjectMapper O(Parameters);
-  llvm::Optional MaxCandidateCount;
+  int64_t MaxCandidateCount;
   bool OK =
   O && O.map("Query", Request.Query) && O.map("Scopes", Request.Scopes) &&
+  O.map("MaxCandidateCount", MaxCandidateCount) &&
   O.map("RestrictForCodeCompletion", Request.RestrictForCodeCompletion) &&
-  O.map("ProximityPaths", Request.ProximityPaths) &&
-  O.map("MaxCandidateCount", MaxCandidateCount);
-  if (MaxCandidateCount)
-Request.MaxCandidateCount = MaxCandidateCount.getValue();
+  O.map("ProximityPaths", Request.ProximityPaths);
+  if (OK && MaxCandidateCount <= std::numeric_limits::max())
+Request.MaxCandidateCount = MaxCandidateCount;
   return OK;
 }
 
 llvm::json::Value toJSON(const FuzzyFindRequest &Request) {
-  auto Result = json::Object{
+  return json::Object{
   {"Query", Request.Query},
   {"Scopes", json::Array{Request.Scopes}},
+  {"MaxCandidateCount", Request.MaxCandidateCount},
   {"RestrictForCodeCompletion", Request.RestrictForCodeCompletion},
   {"ProximityPaths", json::Array{Request.ProximityPaths}},
   };
-  // A huge limit means no limit, leave it out.
-  if (Request.MaxCandidateCount <= std::numeric_limits::max())
-Result["MaxCandidateCount"] =
-static_cast(Request.MaxCandidateCount);
-  return std::move(Result);
 }
 
 bool SwapIndex::fuzzyFind(const FuzzyFindRequest &R,


Index: clang-tools-extra/clangd/index/Index.h
===
--- clang-tools-extra/clangd/index/Index.h
+++ clang-tools-extra/clangd/index/Index.h
@@ -437,7 +437,9 @@
   std::vector Scopes;
   /// \brief The number of top candidates to return. The index may choose to
   /// return more than this, e.g. if it doesn't know which candidates are best.
-  size_t MaxCandidateCount = std::numeric_limits::max();
+  // FIXME: Use llvm::Optional; semantically, the absence of MaxCandidateCount
+  // is equivalent to setting this field to default value as below.
+  uint32_t MaxCandidateCount = std::numeric_limits::max();
   /// If set to true, only symbols for completion support will be considered.
   bool RestrictForCodeCompletion = false;
   /// Contextually relevant files (e.g. the file we're code-completing in).
Index: clang-tools-extra/clangd/index/Index.cpp
===
--- clang-tools-extra/clangd/index/Index.cpp
+++ clang-tools-extra/clangd/index/Index.cpp
@@ -177,29 +177,25 @@
 
 bool fromJSON(const llvm::json::Value &Parameters, FuzzyFindRequest &Request) {
   json::ObjectMapper O(Parameters);
-  llvm::Optional MaxCandidateCount;
+  int64_t MaxCandidateCount;
   bool OK =
   O && O.map("Query", Request.Query) && O.map("Scopes", Request.Scopes) &&
+  O.map("MaxCandidateCount", MaxCandidateCount) &&
   O.map("RestrictForCodeCompletion", Request.RestrictForCodeCompletion) &&
-  O.map("ProximityPaths", Request.ProximityPaths) &&
-  O.map("MaxCandidateCount", MaxCandidateCount);
-  if (MaxCandidateCount)
-Request.MaxCandidateCount = MaxCandidateCount.getValue();
+  O.map("ProximityPaths", Request.ProximityPaths);
+  if (OK && MaxCandidateCount <= std::numeric_limits::max())
+Request.MaxCandidateCount = MaxCandidateCount;
   return OK;
 }
 
 llvm::json::Value toJSON(const FuzzyFindRequest &Request) {
-  auto Result = json::Object{
+  return json::Object{
   {"Query", Request.Query},
   {"Scopes", json::Array{Request.Scopes}},
+  {"MaxCandidateCount", Request.MaxCandidateCount},
   {"RestrictForCodeCompletion", Request.RestrictForCodeCompletion},
   {"P

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clangd/tool/ClangdMain.cpp:197
 
+static llvm::cl::opt IncludeFixIts(
+"include-fixits",

ilya-biryukov wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > I wonder if we should make the `IncludeFixIts` option hidden?
> > > It currently only works for our YCM integration, VSCode and other clients 
> > > break.
> > why would a user want to turn this on or off?
> Ideally, we want to have it always on.
> However, all editors interpret the results we return in different ways. This 
> is a temporary option until we can define how text edits are handled by LSP.
> We filed the bugs, will dig them up on Monday.
Do we have any more details here? I'm still skeptical that exposing this to end 
users will help at all, this seems likely that it should be a capability if we 
do need it.



Comment at: clangd/tool/ClangdMain.cpp:205
+static llvm::cl::opt EnableFunctionArgSnippets(
+"enable-function-arg-snippets",
+llvm::cl::desc("Gives snippets for function arguments, when disabled only "

ilya-biryukov wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > I wonder if we can infer this setting from the `-completion-style' 
> > > (`=bundled` implies `enable-function-arg-snippets == false`)
> > > @sammccall, WDYT?
> > They're not inextricably linked, the main connection is "these options both 
> > need good signaturehelp support to be really useful".
> > But we might want to link them just to cut down on number of options.
> > 
> > I don't have a strong opinion, I don't use `-completion-style=bundled`. Can 
> > we find a few people who do and ask if they like this setting?
> I would (obviously) want these two options to be packaged together whenever I 
> use `=bundled`.
> But I also use VSCode, which has signatureHelp.
> 
> I think @ioeric wanted to try out the `=bundled` completion style. @ioeric, 
> WDYT?
This seems reasonable to have as a preference, I'm also fine combining it with 
bundled.

If you keep it, naming nits:
 - drop "enable" prefix (our other bool flags don't have this)
 - snippets -> placeholders (snippets is both jargon and technically incorrect 
- we still emit snippets like `foo({$0})`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51214



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

[PATCH] D51860: [clangd] NFC: Use uint32_t for FuzzyFindRequest limits

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/index/Index.h:440
   /// return more than this, e.g. if it doesn't know which candidates are best.
-  size_t MaxCandidateCount = std::numeric_limits::max();
+  uint32_t MaxCandidateCount = std::numeric_limits::max();
   /// If set to true, only symbols for completion support will be considered.

sammccall wrote:
> ilya-biryukov wrote:
> > ioeric wrote:
> > > kbobyrev wrote:
> > > > ioeric wrote:
> > > > > Or use `unsigned`?
> > > > `unsigned` would have different size on different platforms, I'm not 
> > > > really sure we want that; could you elaborate on why you think that 
> > > > would be better?
> > > I thought it's (almost) always 4 bytes? But it should always have a 
> > > smaller size than `uint64_t` in json serialization, so it should work for 
> > > us. In general, I would prefer `unsigned` to `uint32_t` when possible. 
> > > For most of the platforms, they are the same. But up to you :) I don't 
> > > really feel strong about this.
> > BTW, many people think using unsigned ints just because inputs can't be 
> > negative is a bad idea.
> > See https://stackoverflow.com/a/18796234
> I mostly agree with that, but LLVM uses unsigned types pervasively, and 
> -Wsign-compare, so they're hard to avoid.
> 
> (FWIW, I still think that this case has become complicated enough that we 
> should use the most explicit option, which seems like Optional here)
Yes, AFAIK Google Coding Guidelines prohibit usage of unsigned (e.g. in `for` 
loops), but I don't know whether I feel good about that.

As for the `llvm::Optional`, I can understand arguments for and against it, but 
it might be OK to use the proposed approach for now as it doesn't require any 
refactoring and add optional later if I (or someone else) has time to make sure 
the user code doesn't make anything unexpected. I'll put a `FIXME` on that, 
will probably fix later if I'll get enough time. Also, it might make sense to 
rename it to `Limit` or something similar.


https://reviews.llvm.org/D51860



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

[PATCH] D51411: [OpenCL] Improve diagnostic of argument in address space conversion builtins

[Alistair Davies via Phabricator via cfe-commits]

AlistairD updated this revision to Diff 164835.

https://reviews.llvm.org/D51411

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/SemaOpenCL/to_addr_builtin.cl


Index: test/SemaOpenCL/to_addr_builtin.cl
===
--- test/SemaOpenCL/to_addr_builtin.cl
+++ test/SemaOpenCL/to_addr_builtin.cl
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -verify -fsyntax-only %s
-// RUN: %clang_cc1 -verify -fsyntax-only -cl-std=CL2.0 %s
+// RUN: %clang_cc1 -Wconversion -verify -fsyntax-only -cl-std=CL2.0 %s
 
 void test(void) {
   global int *glob;
@@ -43,13 +43,15 @@
   // expected-warning@-2{{incompatible integer to pointer conversion assigning 
to '__local int *' from 'int'}}
 #else
   // expected-error@-4{{assigning '__global int *' to '__local int *' changes 
address space of pointer}}
+  // expected-warning@-5{{passing non-generic address space pointer to 
to_global may cause dynamic conversion affecting performance}}
 #endif
 
   global char *glob_c = to_global(loc);
 #if __OPENCL_C_VERSION__ < CL_VERSION_2_0
   // expected-warning@-2{{incompatible integer to pointer conversion 
initializing '__global char *' with an expression of type 'int'}}
 #else
   // expected-warning@-4{{incompatible pointer types initializing '__global 
char *' with an expression of type '__global int *'}}
+  // expected-warning@-5{{passing non-generic address space pointer to 
to_global may cause dynamic conversion affecting performance}}
 #endif
 
 }
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -849,6 +849,13 @@
 return true;
   }
 
+  if (RT->getPointeeType().getAddressSpace() != LangAS::opencl_generic) {
+S.Diag(Call->getArg(0)->getLocStart(),
+   diag::warn_opencl_generic_address_space_arg)
+<< Call->getDirectCallee()->getNameInfo().getAsString()
+<< Call->getArg(0)->getSourceRange();
+  }
+
   RT = RT->getPointeeType();
   auto Qual = RT.getQualifiers();
   switch (BuiltinID) {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -8606,6 +8606,10 @@
   "invalid prototype, variadic arguments are not allowed in OpenCL">;
 def err_opencl_requires_extension : Error<
   "use of %select{type|declaration}0 %1 requires %2 extension to be enabled">;
+def warn_opencl_generic_address_space_arg : Warning<
+  "passing non-generic address space pointer to %0"
+  " may cause dynamic conversion affecting performance">,
+  InGroup, DefaultIgnore;
 
 // OpenCL v2.0 s6.13.6 -- Builtin Pipe Functions
 def err_opencl_builtin_pipe_first_arg : Error<


Index: test/SemaOpenCL/to_addr_builtin.cl
===
--- test/SemaOpenCL/to_addr_builtin.cl
+++ test/SemaOpenCL/to_addr_builtin.cl
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -verify -fsyntax-only %s
-// RUN: %clang_cc1 -verify -fsyntax-only -cl-std=CL2.0 %s
+// RUN: %clang_cc1 -Wconversion -verify -fsyntax-only -cl-std=CL2.0 %s
 
 void test(void) {
   global int *glob;
@@ -43,13 +43,15 @@
   // expected-warning@-2{{incompatible integer to pointer conversion assigning to '__local int *' from 'int'}}
 #else
   // expected-error@-4{{assigning '__global int *' to '__local int *' changes address space of pointer}}
+  // expected-warning@-5{{passing non-generic address space pointer to to_global may cause dynamic conversion affecting performance}}
 #endif
 
   global char *glob_c = to_global(loc);
 #if __OPENCL_C_VERSION__ < CL_VERSION_2_0
   // expected-warning@-2{{incompatible integer to pointer conversion initializing '__global char *' with an expression of type 'int'}}
 #else
   // expected-warning@-4{{incompatible pointer types initializing '__global char *' with an expression of type '__global int *'}}
+  // expected-warning@-5{{passing non-generic address space pointer to to_global may cause dynamic conversion affecting performance}}
 #endif
 
 }
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -849,6 +849,13 @@
 return true;
   }
 
+  if (RT->getPointeeType().getAddressSpace() != LangAS::opencl_generic) {
+S.Diag(Call->getArg(0)->getLocStart(),
+   diag::warn_opencl_generic_address_space_arg)
+<< Call->getDirectCallee()->getNameInfo().getAsString()
+<< Call->getArg(0)->getSourceRange();
+  }
+
   RT = RT->getPointeeType();
   auto Qual = RT.getQualifiers();
   switch (BuiltinID) {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -8606,6 +8606,10 @@
   "invalid pr

[PATCH] D51880: [ASTMatchers] add two matchers for dependent expressions

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In https://reviews.llvm.org/D51880#1229513, @aaron.ballman wrote:

> Missing tests and changes to Registry.cpp for dynamic matchers.
>
> Also, do you want to add `isInstantiationDependent()` at the same time, given 
> the relationship with the other two matchers?


Do you mean a matcher that does `return Node.isValueDependent() || 
Node.isTypeDependent()` or `hasAncestor(expr(anyOf(isValueDependent(), 
isTypeDependent(`?


Repository:
  rC Clang

https://reviews.llvm.org/D51880



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

[PATCH] D51411: [OpenCL] Improve diagnostic of argument in address space conversion builtins

[Alistair Davies via Phabricator via cfe-commits]

AlistairD added a comment.

Reworded warning message, switched warning off by default, and added it to 
-Wconversion group


https://reviews.llvm.org/D51411



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

[clang-tools-extra] r341921 - [clangd] NFC: Use uint32_t for FuzzyFindRequest limits

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Tue Sep 11 03:31:38 2018
New Revision: 341921

URL: http://llvm.org/viewvc/llvm-project?rev=341921&view=rev
Log:
[clangd] NFC: Use uint32_t for FuzzyFindRequest limits

Reviewed By: ioeric

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

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

Modified: clang-tools-extra/trunk/clangd/index/Index.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.cpp?rev=341921&r1=341920&r2=341921&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Index.cpp Tue Sep 11 03:31:38 2018
@@ -177,29 +177,25 @@ std::shared_ptr SwapIndex::
 
 bool fromJSON(const llvm::json::Value &Parameters, FuzzyFindRequest &Request) {
   json::ObjectMapper O(Parameters);
-  llvm::Optional MaxCandidateCount;
+  int64_t MaxCandidateCount;
   bool OK =
   O && O.map("Query", Request.Query) && O.map("Scopes", Request.Scopes) &&
+  O.map("MaxCandidateCount", MaxCandidateCount) &&
   O.map("RestrictForCodeCompletion", Request.RestrictForCodeCompletion) &&
-  O.map("ProximityPaths", Request.ProximityPaths) &&
-  O.map("MaxCandidateCount", MaxCandidateCount);
-  if (MaxCandidateCount)
-Request.MaxCandidateCount = MaxCandidateCount.getValue();
+  O.map("ProximityPaths", Request.ProximityPaths);
+  if (OK && MaxCandidateCount <= std::numeric_limits::max())
+Request.MaxCandidateCount = MaxCandidateCount;
   return OK;
 }
 
 llvm::json::Value toJSON(const FuzzyFindRequest &Request) {
-  auto Result = json::Object{
+  return json::Object{
   {"Query", Request.Query},
   {"Scopes", json::Array{Request.Scopes}},
+  {"MaxCandidateCount", Request.MaxCandidateCount},
   {"RestrictForCodeCompletion", Request.RestrictForCodeCompletion},
   {"ProximityPaths", json::Array{Request.ProximityPaths}},
   };
-  // A huge limit means no limit, leave it out.
-  if (Request.MaxCandidateCount <= std::numeric_limits::max())
-Result["MaxCandidateCount"] =
-static_cast(Request.MaxCandidateCount);
-  return std::move(Result);
 }
 
 bool SwapIndex::fuzzyFind(const FuzzyFindRequest &R,

Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=341921&r1=341920&r2=341921&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Tue Sep 11 03:31:38 2018
@@ -437,7 +437,9 @@ struct FuzzyFindRequest {
   std::vector Scopes;
   /// \brief The number of top candidates to return. The index may choose to
   /// return more than this, e.g. if it doesn't know which candidates are best.
-  size_t MaxCandidateCount = std::numeric_limits::max();
+  // FIXME: Use llvm::Optional; semantically, the absence of MaxCandidateCount
+  // is equivalent to setting this field to default value as below.
+  uint32_t MaxCandidateCount = std::numeric_limits::max();
   /// If set to true, only symbols for completion support will be considered.
   bool RestrictForCodeCompletion = false;
   /// Contextually relevant files (e.g. the file we're code-completing in).


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

[PATCH] D51628: [clangd] Implement a Proof-of-Concept tool for symbol index exploration

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/dexplorer/Dexplorer.cpp:39
+
+// FIXME(kbobyrev): Make this an actual REPL: probably use LLVM Command Line
+// library for parsing flags and arguments.

ilya-biryukov wrote:
> kbobyrev wrote:
> > ilya-biryukov wrote:
> > > Maybe we could expose `CommandLineParser` from 
> > > `llvm/lib/Support/CommandLine.cpp` as a public API and use it here?
> > > Not sure if there are any obstacles to doing so or how much work is it, 
> > > though.
> > > E.g. `cl::opt` seem to rely on being registered in the global parser and 
> > > I'm not sure if there's an easy way out of it.
> > Yes, that might be tricky. I have thought about a few options, e.g. 
> > consuming the first token from the input string as the command and 
> > dispatching arguments parsed via `CommandLineParser` manually (discarding 
> > those which are not accepted by provided command, etc). There are still few 
> > complications: help wouldn't be separate and easily accessible (unless 
> > hardcoded, which is suboptimal).
> > 
> > Another approach I could think of would be simplifying the interface of 
> > each command so that it would be easily parsed:
> > 
> > * `> fuzzy-find request.json`: this would require `FuzzyFindRequest` JSON 
> > (de)serialization implementation, but that would be useful for the 
> > benchmark tool, too
> > * `> lookup-id symbolid`
> > * `> load symbol.yaml`/`swap symbol.yaml`
> > * `> density-hist`
> > * `> tokens-distribution`
> > * `> dense-tokens`
> > * `> sparse-tokens`
> > 
> > And also a generic `> help` for the short reference of each command. Out of 
> > all these commands, only Fuzzy Find request would benefit a lot from a 
> > proper parsing of arguments, having JSON file representation might not be 
> > ideal, but it looks OK for the first iteration for me. Fuzzy Find request 
> > would presumably be one of the most used commands, though, but then again, 
> > we could iterate if that is not really convinient.
> The single-argument and no-arg commands certainly look nice. However, 
> creating a separate file for each fuzzy-find request feels like a pain.
> 
> Have you tried prototyping parsing with `CommandLineParser `? What are the 
> complications to exposing it from LLVM? 
I didn't, but I looked closely at `clang-query` which decided not to do that 
IIUC due to the complications with the `CommandLineParser` and also the 
specifics of the tool and `clang-refactor` which utilizes 
`CommandLineRefactoringOptionVisitor` and `RefactoringActionSubcommand` 
internal implementations, but overall I don't think it looks very simple.

We had a discussion with Eric and Sam, the consensus was that we should start 
with something very simple and iterate on that if we decide that more 
complicated interface is necessary.


https://reviews.llvm.org/D51628



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

[PATCH] D51860: [clangd] NFC: Use uint32_t for FuzzyFindRequest limits

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL341921: [clangd] NFC: Use uint32_t for FuzzyFindRequest 
limits (authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51860?vs=164836&id=164837#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51860

Files:
  clang-tools-extra/trunk/clangd/index/Index.cpp
  clang-tools-extra/trunk/clangd/index/Index.h


Index: clang-tools-extra/trunk/clangd/index/Index.h
===
--- clang-tools-extra/trunk/clangd/index/Index.h
+++ clang-tools-extra/trunk/clangd/index/Index.h
@@ -437,7 +437,9 @@
   std::vector Scopes;
   /// \brief The number of top candidates to return. The index may choose to
   /// return more than this, e.g. if it doesn't know which candidates are best.
-  size_t MaxCandidateCount = std::numeric_limits::max();
+  // FIXME: Use llvm::Optional; semantically, the absence of MaxCandidateCount
+  // is equivalent to setting this field to default value as below.
+  uint32_t MaxCandidateCount = std::numeric_limits::max();
   /// If set to true, only symbols for completion support will be considered.
   bool RestrictForCodeCompletion = false;
   /// Contextually relevant files (e.g. the file we're code-completing in).
Index: clang-tools-extra/trunk/clangd/index/Index.cpp
===
--- clang-tools-extra/trunk/clangd/index/Index.cpp
+++ clang-tools-extra/trunk/clangd/index/Index.cpp
@@ -177,29 +177,25 @@
 
 bool fromJSON(const llvm::json::Value &Parameters, FuzzyFindRequest &Request) {
   json::ObjectMapper O(Parameters);
-  llvm::Optional MaxCandidateCount;
+  int64_t MaxCandidateCount;
   bool OK =
   O && O.map("Query", Request.Query) && O.map("Scopes", Request.Scopes) &&
+  O.map("MaxCandidateCount", MaxCandidateCount) &&
   O.map("RestrictForCodeCompletion", Request.RestrictForCodeCompletion) &&
-  O.map("ProximityPaths", Request.ProximityPaths) &&
-  O.map("MaxCandidateCount", MaxCandidateCount);
-  if (MaxCandidateCount)
-Request.MaxCandidateCount = MaxCandidateCount.getValue();
+  O.map("ProximityPaths", Request.ProximityPaths);
+  if (OK && MaxCandidateCount <= std::numeric_limits::max())
+Request.MaxCandidateCount = MaxCandidateCount;
   return OK;
 }
 
 llvm::json::Value toJSON(const FuzzyFindRequest &Request) {
-  auto Result = json::Object{
+  return json::Object{
   {"Query", Request.Query},
   {"Scopes", json::Array{Request.Scopes}},
+  {"MaxCandidateCount", Request.MaxCandidateCount},
   {"RestrictForCodeCompletion", Request.RestrictForCodeCompletion},
   {"ProximityPaths", json::Array{Request.ProximityPaths}},
   };
-  // A huge limit means no limit, leave it out.
-  if (Request.MaxCandidateCount <= std::numeric_limits::max())
-Result["MaxCandidateCount"] =
-static_cast(Request.MaxCandidateCount);
-  return std::move(Result);
 }
 
 bool SwapIndex::fuzzyFind(const FuzzyFindRequest &R,


Index: clang-tools-extra/trunk/clangd/index/Index.h
===
--- clang-tools-extra/trunk/clangd/index/Index.h
+++ clang-tools-extra/trunk/clangd/index/Index.h
@@ -437,7 +437,9 @@
   std::vector Scopes;
   /// \brief The number of top candidates to return. The index may choose to
   /// return more than this, e.g. if it doesn't know which candidates are best.
-  size_t MaxCandidateCount = std::numeric_limits::max();
+  // FIXME: Use llvm::Optional; semantically, the absence of MaxCandidateCount
+  // is equivalent to setting this field to default value as below.
+  uint32_t MaxCandidateCount = std::numeric_limits::max();
   /// If set to true, only symbols for completion support will be considered.
   bool RestrictForCodeCompletion = false;
   /// Contextually relevant files (e.g. the file we're code-completing in).
Index: clang-tools-extra/trunk/clangd/index/Index.cpp
===
--- clang-tools-extra/trunk/clangd/index/Index.cpp
+++ clang-tools-extra/trunk/clangd/index/Index.cpp
@@ -177,29 +177,25 @@
 
 bool fromJSON(const llvm::json::Value &Parameters, FuzzyFindRequest &Request) {
   json::ObjectMapper O(Parameters);
-  llvm::Optional MaxCandidateCount;
+  int64_t MaxCandidateCount;
   bool OK =
   O && O.map("Query", Request.Query) && O.map("Scopes", Request.Scopes) &&
+  O.map("MaxCandidateCount", MaxCandidateCount) &&
   O.map("RestrictForCodeCompletion", Request.RestrictForCodeCompletion) &&
-  O.map("ProximityPaths", Request.ProximityPaths) &&
-  O.map("MaxCandidateCount", MaxCandidateCount);
-  if (MaxCandidateCount)
-Request.MaxCandidateCount = MaxCandidateCount.getValue();
+  O.map("ProximityPaths", Request.ProximityPaths);
+  if (OK && MaxCandidateCount <= std::numeric_limits::max())
+Request.MaxCandidateCou

[PATCH] D51921: [VFS] vfs::directory_iterator yields path and file type instead of full Status

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: bkramer.
Herald added subscribers: cfe-commits, fedor.sergeev.

Most callers I can find are using only `getName()`. Type is used by the
recursive iterator.

Now we don't have to call stat() on every listed file (on most platforms).
Exceptions are e.g. Solaris where readdir() doesn't include type information.
On those platforms we'll still stat() - see https://reviews.llvm.org/D51918.

The result is significantly faster (stat() can be slow).
My motivation: this may allow us to improve clang IO on large TUs with long
include search paths. Caching readdir() results may allow us to skip many stat()
and open() operations on nonexistent files.


Repository:
  rC Clang

https://reviews.llvm.org/D51921

Files:
  include/clang/Basic/VirtualFileSystem.h
  lib/Basic/VirtualFileSystem.cpp
  unittests/Basic/VirtualFileSystemTest.cpp

Index: unittests/Basic/VirtualFileSystemTest.cpp
===
--- unittests/Basic/VirtualFileSystemTest.cpp
+++ unittests/Basic/VirtualFileSystemTest.cpp
@@ -104,21 +104,23 @@
   Path(_Path.str()) {
   for ( ; I != FilesAndDirs.end(); ++I) {
 if (isInPath(I->first)) {
-  CurrentEntry = I->second;
+  CurrentEntry.Path = I->second.getName();
+  CurrentEntry.Type = I->second.getType();
   break;
 }
   }
 }
 std::error_code increment() override {
   ++I;
   for ( ; I != FilesAndDirs.end(); ++I) {
 if (isInPath(I->first)) {
-  CurrentEntry = I->second;
+  CurrentEntry.Path = I->second.getName();
+  CurrentEntry.Type = I->second.getType();
   break;
 }
   }
   if (I == FilesAndDirs.end())
-CurrentEntry = vfs::Status();
+CurrentEntry = vfs::directory_entry();
   return std::error_code();
 }
   };
@@ -656,14 +658,14 @@
   O->pushOverlay(Upper);
 
   std::error_code EC;
-  Lower->addRegularFile("/onlyInLow", sys::fs::owner_read);
-  Lower->addRegularFile("/hiddenByMid", sys::fs::owner_read);
-  Lower->addRegularFile("/hiddenByUp", sys::fs::owner_read);
-  Middle->addRegularFile("/onlyInMid", sys::fs::owner_write);
-  Middle->addRegularFile("/hiddenByMid", sys::fs::owner_write);
-  Middle->addRegularFile("/hiddenByUp", sys::fs::owner_write);
-  Upper->addRegularFile("/onlyInUp", sys::fs::owner_all);
-  Upper->addRegularFile("/hiddenByUp", sys::fs::owner_all);
+  Lower->addRegularFile("/onlyInLow");
+  Lower->addDirectory("/hiddenByMid");
+  Lower->addDirectory("/hiddenByUp");
+  Middle->addRegularFile("/onlyInMid");
+  Middle->addRegularFile("/hiddenByMid");
+  Middle->addDirectory("/hiddenByUp");
+  Upper->addRegularFile("/onlyInUp");
+  Upper->addRegularFile("/hiddenByUp");
   checkContents(
   O->dir_begin("/", EC),
   {"/hiddenByUp", "/onlyInUp", "/hiddenByMid", "/onlyInMid", "/onlyInLow"});
@@ -676,16 +678,16 @@
   if (I->getName() == "/hiddenByUp")
 break;
 ASSERT_NE(E, I);
-EXPECT_EQ(sys::fs::owner_all, I->getPermissions());
+EXPECT_EQ(sys::fs::file_type::regular_file, I->Type);
   }
   {
 std::error_code EC;
 vfs::directory_iterator I = O->dir_begin("/", EC), E;
 for ( ; !EC && I != E; I.increment(EC))
   if (I->getName() == "/hiddenByMid")
 break;
 ASSERT_NE(E, I);
-EXPECT_EQ(sys::fs::owner_write, I->getPermissions());
+EXPECT_EQ(sys::fs::file_type::regular_file, I->Type);
   }
 }
 
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -316,25 +316,19 @@
 public:
   RealFSDirIter(const Twine &Path, std::error_code &EC) : Iter(Path, EC) {
 if (Iter != llvm::sys::fs::directory_iterator()) {
-  llvm::sys::fs::file_status S;
-  std::error_code ErrorCode = llvm::sys::fs::status(Iter->path(), S, true);
-  CurrentEntry = Status::copyWithNewName(S, Iter->path());
-  if (!EC)
-EC = ErrorCode;
+  CurrentEntry.Path = Iter->path();
+  CurrentEntry.Type = Iter->type();
 }
   }
 
   std::error_code increment() override {
 std::error_code EC;
 Iter.increment(EC);
 if (Iter == llvm::sys::fs::directory_iterator()) {
-  CurrentEntry = Status();
+  CurrentEntry = directory_entry();
 } else {
-  llvm::sys::fs::file_status S;
-  std::error_code ErrorCode = llvm::sys::fs::status(Iter->path(), S, true);
-  CurrentEntry = Status::copyWithNewName(S, Iter->path());
-  if (!EC)
-EC = ErrorCode;
+  CurrentEntry.Path = Iter->path();
+  CurrentEntry.Type = Iter->type();
 }
 return EC;
   }
@@ -446,11 +440,11 @@
 while (true) {
   std::error_code EC = incrementDirIter(IsFirstTime);
   if (EC || CurrentDirIter == directory_iterator()) {
-CurrentEntry = Status();
+CurrentEntry = directory_entry();
 return EC;
   }

[PATCH] D51297: [docs] Create a guide for Vim users on how to set up Clangd

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Outline the structure for "Editor Integration" section which is to be filled 
with other options later.


https://reviews.llvm.org/D51297

Files:
  clang-tools-extra/docs/clangd.rst


Index: clang-tools-extra/docs/clangd.rst
===
--- clang-tools-extra/docs/clangd.rst
+++ clang-tools-extra/docs/clangd.rst
@@ -108,6 +108,22 @@
 | Gen. Getters/Setters| No |   No |
 +-++--+
 
+Editor Integration
+==
+
+Vim Integration
+---
+
+LanguageClient-neovim
+~
+
+One of the options of using :program:`Clangd` in :program:`vim` (or
+:program:`nvim`) is to utilize `LanguageClient-neovim
+`_ plugin. Please see the
+`Clangd Wiki page
+`_ for
+instructions.
+
 Getting Involved
 ==
 


Index: clang-tools-extra/docs/clangd.rst
===
--- clang-tools-extra/docs/clangd.rst
+++ clang-tools-extra/docs/clangd.rst
@@ -108,6 +108,22 @@
 | Gen. Getters/Setters| No |   No |
 +-++--+
 
+Editor Integration
+==
+
+Vim Integration
+---
+
+LanguageClient-neovim
+~
+
+One of the options of using :program:`Clangd` in :program:`vim` (or
+:program:`nvim`) is to utilize `LanguageClient-neovim
+`_ plugin. Please see the
+`Clangd Wiki page
+`_ for
+instructions.
+
 Getting Involved
 ==
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r341925 - [clang-tidy] Add a missing comma after "flags"

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Tue Sep 11 03:37:08 2018
New Revision: 341925

URL: http://llvm.org/viewvc/llvm-project?rev=341925&view=rev
Log:
[clang-tidy] Add a missing comma after "flags"

Added:
clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/flags/
clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/flags/internal-file.h
Modified:
clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h
clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/strings/internal-file.h
clang-tools-extra/trunk/test/clang-tidy/abseil-no-internal-dependencies.cpp

Modified: clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h?rev=341925&r1=341924&r2=341925&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h Tue Sep 11 
03:37:08 2018
@@ -43,24 +43,15 @@ AST_POLYMORPHIC_MATCHER(
   // Determine whether filepath contains "absl/[absl-library]" substring, where
   // [absl-library] is AbseilLibraries list entry.
   StringRef Path = FileEntry->getName();
-  const static llvm::SmallString<5> AbslPrefix("absl/");
+  static constexpr llvm::StringLiteral AbslPrefix("absl/");
   size_t PrefixPosition = Path.find(AbslPrefix);
   if (PrefixPosition == StringRef::npos)
 return false;
   Path = Path.drop_front(PrefixPosition + AbslPrefix.size());
-  static const char *AbseilLibraries[] = {"algorithm",
-  "base",
-  "container",
-  "debugging",
-  "flags"
-  "memory",
-  "meta",
-  "numeric",
-  "strings",
-  "synchronization",
-  "time",
-  "types",
-  "utility"};
+  static const char *AbseilLibraries[] = {
+  "algorithm", "base",  "container", "debugging", "flags",
+  "memory","meta",  "numeric",   "strings",   "synchronization",
+  "time",  "types", "utility"};
   return std::any_of(
   std::begin(AbseilLibraries), std::end(AbseilLibraries),
   [&](const char *Library) { return Path.startswith(Library); });

Added: clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/flags/internal-file.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/flags/internal-file.h?rev=341925&view=auto
==
--- clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/flags/internal-file.h 
(added)
+++ clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/flags/internal-file.h 
Tue Sep 11 03:37:08 2018
@@ -0,0 +1 @@
+#define USE_INTERNAL(x) absl::strings_internal::Internal##x()

Modified: 
clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/strings/internal-file.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/strings/internal-file.h?rev=341925&r1=341924&r2=341925&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/strings/internal-file.h 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/strings/internal-file.h 
Tue Sep 11 03:37:08 2018
@@ -31,5 +31,3 @@ class FriendUsageInternal {
 namespace absl {
 void OpeningNamespaceInternally() { strings_internal::InternalFunction(); }
 } // namespace absl
-
-#define USE_INTERNAL(x) absl::strings_internal::Internal##x()

Modified: 
clang-tools-extra/trunk/test/clang-tidy/abseil-no-internal-dependencies.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/abseil-no-internal-dependencies.cpp?rev=341925&r1=341924&r2=341925&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/abseil-no-internal-dependencies.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/abseil-no-internal-dependencies.cpp 
Tue Sep 11 03:37:08 2018
@@ -2,6 +2,7 @@
 // RUN: clang-tidy -checks='-*, abseil-no-internal-dependencies' 
-header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s
 
 #include "absl/strings/internal-file.h"
+#include "absl/flags/internal-file.h"
 // CHECK-NOT: warning:
 
 #include "absl/external-file.h"


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

[PATCH] D51292: [docs] Update clang-rename documentation

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

@ioeric does it look better now?


https://reviews.llvm.org/D51292



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

[PATCH] D51628: [clangd] Implement a Proof-of-Concept tool for symbol index exploration

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Dexplorer is a long name. How about shortening it to `dexp`?




Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:56
+
+template 
+void reportTime(StringRef Name, Function F) {

Why do we want `TimeUnit`?
It adds complexity, if we want to make large values more readable we have other 
alternatives:
- printing adaptive units, e.g. "when it's larger than 5000ms, start printing 
seconds", "when it's larger than 5minutes, start printing minutes"
- provide separators (`5ms`  is pretty much unreadable, `500 000 000ms` 
is much better)




Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:125
+// * print out tokens with least dense posting lists
+void dispatchRequest(const std::unique_ptr &Index,
+ StringRef Request) {

This function does too many things:
- parses the input.
- dispatches on different kinds of requests.
- handles user input errors.
- actually runs the commands.

This turns out somewhat unreadable at the end, we might want to separate those 
into multiple functions. Will need a bit of time to think on how to actually do 
it best. Will come back after I have concrete suggestions.



Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:139
+  "symbol name.\n";
+  helpRequest();
+  return;

This produces a ton of output.
Could we simply produce a message to run 'help' to get a list of supported 
commands instead?

It could be really annoying to see large help messages on every typo.


https://reviews.llvm.org/D51628



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

[PATCH] D51921: [VFS] vfs::directory_iterator yields path and file type instead of full Status

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: include/clang/Basic/VirtualFileSystem.h:135
+  // For compatibility with old Status-based API. Prefer using Path directly.
+  StringRef getName() const { return Path; }
+};

Backwards-compatibility notes:

 - Almost all users of `directory_iterator` require no source changes (with 
this change)
 - Implementations of VFS require changes if they support directory iteration 
and do not merely wrap other VFSes. Anecdotally, most do not require changes. 

So this weird API seems worth having to make out-of-tree breakages less painful.
Happy to update the internal uses though if that seems worthwhile.


Repository:
  rC Clang

https://reviews.llvm.org/D51921



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

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/tool/ClangdMain.cpp:197
 
+static llvm::cl::opt IncludeFixIts(
+"include-fixits",

sammccall wrote:
> ilya-biryukov wrote:
> > sammccall wrote:
> > > ilya-biryukov wrote:
> > > > I wonder if we should make the `IncludeFixIts` option hidden?
> > > > It currently only works for our YCM integration, VSCode and other 
> > > > clients break.
> > > why would a user want to turn this on or off?
> > Ideally, we want to have it always on.
> > However, all editors interpret the results we return in different ways. 
> > This is a temporary option until we can define how text edits are handled 
> > by LSP.
> > We filed the bugs, will dig them up on Monday.
> Do we have any more details here? I'm still skeptical that exposing this to 
> end users will help at all, this seems likely that it should be a capability 
> if we do need it.
No updates  on the issue. Here it is:
https://github.com/Microsoft/language-server-protocol/issues/543

Not sure capability is the right thing there, the problem is that 
additionalTextEdits are underspecified and implemented differently in every 
client. What we need is a fix in the protocol and fixes in all the clients.

Sadly, this only works in YCM-based completer for now (of all we tested)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51214



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

[PATCH] D51544: [OpenCL] Split opencl-c.h header

[Andrew Savonichev via Phabricator via cfe-commits]

asavonic added a comment.

In https://reviews.llvm.org/D51544#1229105, @Anastasia wrote:

> > With this setup, we can compile opencl-c-common.h, opencl-c-fp16.h and
> >  opencl-c-fp64.h into PCHs with one set of extensions/OpenCL version,
> >  and use them for any other set of extensions/OpenCL version. Clang
> >  will detect this and throw out an error, which can be safely disabled
> >  by -fno-validate-pch option.
>
> However, keeping this as a permanent solution is unsafe. Because this way can 
> result in unexpected errors to be silent out and allow erroneous 
> configurations to be accepted successfully without any notification.


Agree. LIT test in `test/Headers/opencl-c-header-split.cl` is supposed
to verify that common/fp16/fp64 headers do not use preprocessor macros
and it should catch most of the issues, but this is definitely not the
most robust solution.

> So I am wondering if there is any plan to put a proper solution in place at 
> some point?

This solution can be improved if we implement `convert` and `shuffle`
as clang builtins with custom typechecking: these two builtins (with
all their overloadings) take ~90% of `opencl-c-common.h` and ~50% of
fp16/fp64 headers.

However, this can be a non-trivial change: it is difficult to do a
proper mangling for clang builtins and rounding modes must be handled
as well.

I'll take a few days to prototype this. If it turns out to be good in
terms of performance/footprint, we can drop this patch.


Repository:
  rC Clang

https://reviews.llvm.org/D51544



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

[PATCH] D51917: [CodeCompletion] Enable signature help when initializing class/struct/union members.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 164844.
kadircet marked 3 inline comments as done.
kadircet added a comment.

- Move ValueDecl extraction to a helper.
- Call completion handlers as well.


Repository:
  rC Clang

https://reviews.llvm.org/D51917

Files:
  include/clang/Sema/Sema.h
  lib/Parse/ParseDeclCXX.cpp
  lib/Sema/SemaCodeComplete.cpp
  lib/Sema/SemaDeclCXX.cpp
  test/CodeCompletion/ctor-initializer.cpp

Index: test/CodeCompletion/ctor-initializer.cpp
===
--- test/CodeCompletion/ctor-initializer.cpp
+++ test/CodeCompletion/ctor-initializer.cpp
@@ -64,3 +64,26 @@
   // CHECK-CC8: COMPLETION: Pattern : member2(<#args#>
   int member1, member2;
 };
+
+struct Base2 {
+  Base2(int);
+};
+
+struct Composition1 {
+  Composition1() : b2_elem() {}
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:73:28 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:73:28 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // CHECK-CC9: OVERLOAD: Base2(<#int#>)
+  // CHECK-CC9: OVERLOAD: Base2(<#const Base2 &#>)
+  Composition1(Base2);
+  Base2 b2_elem;
+};
+
+struct Composition2 {
+  Composition2() : c1_elem(Base2(1)) {}
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:83:34 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:83:34 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:83:35 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:83:35 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  Composition1 c1_elem;
+};
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -3777,6 +3777,24 @@
 
 }
 
+ValueDecl *Sema::tryLookupCtorInitMemberDecl(CXXRecordDecl *ClassDecl,
+ CXXScopeSpec &SS,
+ ParsedType TemplateTypeTy,
+ IdentifierInfo *MemberOrBase) {
+  if (!SS.getScopeRep() && !TemplateTypeTy) {
+// Look for a member, first.
+DeclContext::lookup_result Result = ClassDecl->lookup(MemberOrBase);
+if (!Result.empty()) {
+  ValueDecl *Member;
+  if ((Member = dyn_cast(Result.front())) ||
+  (Member = dyn_cast(Result.front( {
+return Member;
+  }
+}
+  }
+  return nullptr;
+}
+
 /// Handle a C++ member initializer.
 MemInitResult
 Sema::BuildMemInitializer(Decl *ConstructorD,
@@ -3820,21 +3838,14 @@
   //   of a single identifier refers to the class member. A
   //   mem-initializer-id for the hidden base class may be specified
   //   using a qualified name. ]
-  if (!SS.getScopeRep() && !TemplateTypeTy) {
-// Look for a member, first.
-DeclContext::lookup_result Result = ClassDecl->lookup(MemberOrBase);
-if (!Result.empty()) {
-  ValueDecl *Member;
-  if ((Member = dyn_cast(Result.front())) ||
-  (Member = dyn_cast(Result.front( {
-if (EllipsisLoc.isValid())
-  Diag(EllipsisLoc, diag::err_pack_expansion_member_init)
-<< MemberOrBase
-<< SourceRange(IdLoc, Init->getSourceRange().getEnd());
-
-return BuildMemberInitializer(Member, Init, IdLoc);
-  }
-}
+  if (ValueDecl *Member = tryLookupCtorInitMemberDecl(
+  ClassDecl, SS, TemplateTypeTy, MemberOrBase)) {
+if (EllipsisLoc.isValid())
+  Diag(EllipsisLoc, diag::err_pack_expansion_member_init)
+  << MemberOrBase
+  << SourceRange(IdLoc, Init->getSourceRange().getEnd());
+
+return BuildMemberInitializer(Member, Init, IdLoc);
   }
   // It didn't name a member, so see if it names a class.
   QualType BaseType;
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -4586,6 +4586,25 @@
   return ProduceSignatureHelp(*this, S, Results, Args.size(), OpenParLoc);
 }
 
+QualType Sema::ProduceCtorInitMemberSignatureHelp(
+Scope *S, Decl *ConstructorDecl, CXXScopeSpec SS, ParsedType TemplateTypeTy,
+ArrayRef ArgExprs, IdentifierInfo *II, SourceLocation OpenParLoc) {
+  if (!CodeCompleter)
+return QualType();
+
+  CXXConstructorDecl *Constructor =
+  dyn_cast(ConstructorDecl);
+  if (!Constructor)
+return QualType();
+  // FIXME: Add support for Base class constructors as well.
+  if (ValueDecl *MemberDecl = tryLookupCtorInitMemberDecl(
+  Constructor->getParent(), SS, TemplateTypeTy, II))
+return ProduceConstructorSignatureHelp(getCurScope(), MemberDecl->getType(),
+   MemberDecl->getLocation(), ArgExprs,
+

[PATCH] D51924: [clangd] Add unittests for D51917

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51924

Files:
  unittests/clangd/CodeCompleteTests.cpp


Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1917,6 +1917,45 @@
AllOf(Named("TestClangc"), Deprecated(;
 }
 
+TEST(SignatureHelpTest, ConstructorInitializeFields) {
+  {
+const auto Results = signatures(R"cpp(
+  struct A {
+A(int);
+  };
+  struct B {
+B() : a_elem(^) {}
+A a_elem;
+  };
+)cpp");
+EXPECT_THAT(Results.signatures, UnorderedElementsAre(
+Sig("A(int)", {"int"}),
+Sig("A(A &&)", {"A &&"}),
+Sig("A(const A &)", {"const A &"})
+));
+  }
+  {
+const auto Results = signatures(R"cpp(
+  struct A {
+A(int);
+  };
+  struct C {
+C(int);
+C(A);
+  };
+  struct B {
+B() : c_elem(A(1^)) {}
+C c_elem;
+  };
+)cpp");
+EXPECT_THAT(Results.signatures, UnorderedElementsAre(
+Sig("A(int)", {"int"}),
+Sig("A(A &&)", {"A &&"}),
+Sig("A(const A &)", {"const A &"})
+));
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang


Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1917,6 +1917,45 @@
AllOf(Named("TestClangc"), Deprecated(;
 }
 
+TEST(SignatureHelpTest, ConstructorInitializeFields) {
+  {
+const auto Results = signatures(R"cpp(
+  struct A {
+A(int);
+  };
+  struct B {
+B() : a_elem(^) {}
+A a_elem;
+  };
+)cpp");
+EXPECT_THAT(Results.signatures, UnorderedElementsAre(
+Sig("A(int)", {"int"}),
+Sig("A(A &&)", {"A &&"}),
+Sig("A(const A &)", {"const A &"})
+));
+  }
+  {
+const auto Results = signatures(R"cpp(
+  struct A {
+A(int);
+  };
+  struct C {
+C(int);
+C(A);
+  };
+  struct B {
+B() : c_elem(A(1^)) {}
+C c_elem;
+  };
+)cpp");
+EXPECT_THAT(Results.signatures, UnorderedElementsAre(
+Sig("A(int)", {"int"}),
+Sig("A(A &&)", {"A &&"}),
+Sig("A(const A &)", {"const A &"})
+));
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51917: [CodeCompletion] Enable signature help when initializing class/struct/union members.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: lib/Parse/ParseDeclCXX.cpp:3472
+ParseExpressionList(ArgExprs, CommaLocs, [&] {
+  if (CalledSignatureHelp)
+return;

Let's always call signature help and code completion here to be consistent with 
other cases.
We don't want to ever block completion, even in obscure cases when it's called 
twice.



Comment at: lib/Sema/SemaDeclCXX.cpp:3784
+ IdentifierInfo *MemberOrBase) {
+  if (!SS.getScopeRep() && !TemplateTypeTy) {
+// Look for a member, first.

NIT: invert condition to reduce nesting and simplify the code, i.e.
```
if (SS.getScopeRep() || TemplateTypeTy)
  return nullptr;
// rest of the code
```




Comment at: lib/Sema/SemaDeclCXX.cpp:3785
+  if (!SS.getScopeRep() && !TemplateTypeTy) {
+// Look for a member, first.
+DeclContext::lookup_result Result = ClassDecl->lookup(MemberOrBase);

This comment does not look useful without a context, maybe move it to the 
previous call site in sema?


Repository:
  rC Clang

https://reviews.llvm.org/D51917



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

[PATCH] D51924: [clangd] Add unittests for D51917

[Ilya Biryukov via Phabricator via cfe-commits]

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

LGTM


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51924



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

[PATCH] D51628: [clangd] Implement a Proof-of-Concept tool for symbol index exploration

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:23
+#include "llvm/Support/Signals.h"
+#include "llvm/Support/YAMLTraits.h"
+#include 

why?



Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:56
+
+template 
+void reportTime(StringRef Name, Function F) {

ilya-biryukov wrote:
> Why do we want `TimeUnit`?
> It adds complexity, if we want to make large values more readable we have 
> other alternatives:
> - printing adaptive units, e.g. "when it's larger than 5000ms, start printing 
> seconds", "when it's larger than 5minutes, start printing minutes"
> - provide separators (`5ms`  is pretty much unreadable, `500 000 
> 000ms` is much better)
> 
+1 just pick ms or us for now

You can use `formatv({0:ms}, Duration)` to print neatly, no need for the traits.



Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:79
+
+static const std::string HelpMessage = R"(
+DExplorer commands:

nit: this starts with a newline



Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:82
+
+> fuzzy-find Name
+

nit: just "find" for convenience?



Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:87
+
+> lookup-id SymbolID
+

examples use `lookup-id`, code says `lookup`.
(I prefer `lookup` FWIW)



Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:90
+Retrieves symbol names given USR.
+
+> help

(Not sure the "help" doc or "press ctrl-d to exit" are particularly useful)



Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:100
+
+void lookupRequest(const std::unique_ptr &Index,
+   clang::clangd::LookupRequest &Request) {

SymbolIndex&, you don't need the unique_ptr



Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:100
+
+void lookupRequest(const std::unique_ptr &Index,
+   clang::clangd::LookupRequest &Request) {

sammccall wrote:
> SymbolIndex&, you don't need the unique_ptr
nit: lookup (the "request" is the arg)



Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:103
+  std::vector Symbols;
+  Index->lookup(Request, [&](const Symbol &S) { Symbols.push_back(S); });
+  // FIXME(kbobyrev): Print symbol final scores to see the distribution.

why not print them on-the-fly?
(this isn't safe, the data backing the symbols may not live long enough)



Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:106
+  llvm::outs() << "\nRetrieved Symbols:\n";
+  llvm::outs() << "Rank. Symbol Name | Symbol ID\n";
+  for (size_t Rank = 0; Rank < Symbols.size(); ++Rank)

please be consistent in using . or | as separator



Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:106
+  llvm::outs() << "\nRetrieved Symbols:\n";
+  llvm::outs() << "Rank. Symbol Name | Symbol ID\n";
+  for (size_t Rank = 0; Rank < Symbols.size(); ++Rank)

sammccall wrote:
> please be consistent in using . or | as separator
I'd suggest putting the ID first, as it's fixed-width so the columns will 
actually be columns



Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:125
+// * print out tokens with least dense posting lists
+void dispatchRequest(const std::unique_ptr &Index,
+ StringRef Request) {

ilya-biryukov wrote:
> This function does too many things:
> - parses the input.
> - dispatches on different kinds of requests.
> - handles user input errors.
> - actually runs the commands.
> 
> This turns out somewhat unreadable at the end, we might want to separate 
> those into multiple functions. Will need a bit of time to think on how to 
> actually do it best. Will come back after I have concrete suggestions.
For now can we keep it simple:
 - this function can split the command from the rest, and pass the rest of the 
args to `fuzzyFindRequest` etc
 - if the command is unknown, the dispatcher reports the error
 - if the command is known, the command handler reports input errors (this 
could be cleaner/more declarative, but we don't want to build a big framework 
in this patch)
 - this function could do the timing - the request construction is negligible 
and that keeps the command handlers focused on their job



Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:130
+  if (Arguments.empty()) {
+llvm::outs() << "ERROR: Request can not be empty.\n";
+helpRequest();

REPLs don't usually make this an error, just return



Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DEx

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 164847.
kadircet marked 4 inline comments as done.
kadircet added a comment.

- Update descriptions and change parameter name.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51214

Files:
  clangd/tool/ClangdMain.cpp


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -168,6 +168,20 @@
 "'compile_commands.json' files")),
 llvm::cl::init(FilesystemCompileArgs), llvm::cl::Hidden);
 
+static llvm::cl::opt IncludeFixIts(
+"completions-with-fixes",
+llvm::cl::desc(
+"Enables suggestion of completion items that need additional changes. "
+"Like changing an arrow(->) member access to dot(.) member access."),
+llvm::cl::init(clangd::CodeCompleteOptions().IncludeFixIts));
+
+static llvm::cl::opt EnableFunctionArgSnippets(
+"function-arg-placeholders",
+llvm::cl::desc("When disabled, completions contain only parentheses for "
+   "function calls. When enabled, completions also contain "
+   "placeholders for method parameters."),
+llvm::cl::init(clangd::CodeCompleteOptions().EnableFunctionArgSnippets));
+
 int main(int argc, char *argv[]) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   llvm::cl::SetVersionPrinter([](llvm::raw_ostream &OS) {
@@ -295,6 +309,8 @@
 CCOpts.IncludeIndicator.NoInsert.clear();
   }
   CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
+  CCOpts.IncludeFixIts = IncludeFixIts;
+  CCOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
 
   // Initialize and run ClangdLSPServer.
   ClangdLSPServer LSPServer(


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -168,6 +168,20 @@
 "'compile_commands.json' files")),
 llvm::cl::init(FilesystemCompileArgs), llvm::cl::Hidden);
 
+static llvm::cl::opt IncludeFixIts(
+"completions-with-fixes",
+llvm::cl::desc(
+"Enables suggestion of completion items that need additional changes. "
+"Like changing an arrow(->) member access to dot(.) member access."),
+llvm::cl::init(clangd::CodeCompleteOptions().IncludeFixIts));
+
+static llvm::cl::opt EnableFunctionArgSnippets(
+"function-arg-placeholders",
+llvm::cl::desc("When disabled, completions contain only parentheses for "
+   "function calls. When enabled, completions also contain "
+   "placeholders for method parameters."),
+llvm::cl::init(clangd::CodeCompleteOptions().EnableFunctionArgSnippets));
+
 int main(int argc, char *argv[]) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   llvm::cl::SetVersionPrinter([](llvm::raw_ostream &OS) {
@@ -295,6 +309,8 @@
 CCOpts.IncludeIndicator.NoInsert.clear();
   }
   CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
+  CCOpts.IncludeFixIts = IncludeFixIts;
+  CCOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
 
   // Initialize and run ClangdLSPServer.
   ClangdLSPServer LSPServer(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

I think I'm still where I was a few weeks ago - option to drop args makes 
sense, completions with fixes isn't something users should care about.




Comment at: clangd/tool/ClangdMain.cpp:197
 
+static llvm::cl::opt IncludeFixIts(
+"include-fixits",

ilya-biryukov wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > sammccall wrote:
> > > > ilya-biryukov wrote:
> > > > > I wonder if we should make the `IncludeFixIts` option hidden?
> > > > > It currently only works for our YCM integration, VSCode and other 
> > > > > clients break.
> > > > why would a user want to turn this on or off?
> > > Ideally, we want to have it always on.
> > > However, all editors interpret the results we return in different ways. 
> > > This is a temporary option until we can define how text edits are handled 
> > > by LSP.
> > > We filed the bugs, will dig them up on Monday.
> > Do we have any more details here? I'm still skeptical that exposing this to 
> > end users will help at all, this seems likely that it should be a 
> > capability if we do need it.
> No updates  on the issue. Here it is:
> https://github.com/Microsoft/language-server-protocol/issues/543
> 
> Not sure capability is the right thing there, the problem is that 
> additionalTextEdits are underspecified and implemented differently in every 
> client. What we need is a fix in the protocol and fixes in all the clients.
> 
> Sadly, this only works in YCM-based completer for now (of all we tested)
Sure, sounds like protocol fix is the long-term answer. I don't think adding 
user-facing options are better than nothing. If YCM does the right thing and we 
want to disable it for everyone not on YCM, we can add a 
`textEditsAreAppliedInOrder` capability to the YCM completer and treat that as 
an opt-in. It's not clear what the advantage of a user-facing flag over an 
editor-facing capability is for this purpose.

Mostly given LSP is unclear here it seems this feature isn't ready for 
prime-time.
Could we fix it on our side by coalescing multiple edits into a single one?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51214



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

[PATCH] D51090: [clangd] Add index benchmarks

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 164848.
kbobyrev marked 6 inline comments as done.
kbobyrev added a comment.

Address few comments (not all of them for now, though).


https://reviews.llvm.org/D51090

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/benchmarks/CMakeLists.txt
  clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp

Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
@@ -0,0 +1,124 @@
+//===--- IndexBenchmark.cpp - Clangd index benchmarks ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "../index/SymbolYAML.h"
+#include "../index/dex/Dex.h"
+#include "benchmark/benchmark.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Regex.h"
+#include 
+#include 
+#include 
+
+const char *IndexFilename;
+const char *LogFilename;
+const char *LLVMRootPath;
+
+namespace clang {
+namespace clangd {
+namespace {
+
+std::unique_ptr buildMem() {
+  return clang::clangd::loadIndex(IndexFilename, {}, false);
+}
+
+std::unique_ptr buildDex() {
+  return clang::clangd::loadIndex(IndexFilename, {}, true);
+}
+
+// This function processes user-provided Log file with fuzzy find requests in
+// the following format:
+//
+// fuzzyFind("UnqualifiedName", scopes=["clang::", "clang::clangd::"])
+//
+// It constructs vector of FuzzyFindRequests which is later used for the
+// benchmarks.
+std::vector extractQueriesFromLogs() {
+  llvm::Regex RequestMatcher("fuzzyFind\\(\"([a-zA-Z]*)\", scopes=\\[(.*)\\]");
+  llvm::SmallVector Matches;
+  std::ifstream InputStream(LogFilename);
+  std::string Log((std::istreambuf_iterator(InputStream)),
+  std::istreambuf_iterator());
+  llvm::StringRef Temporary(Log);
+  llvm::SmallVector Strings;
+  Temporary.split(Strings, '\n');
+
+  clang::clangd::FuzzyFindRequest R;
+  R.MaxCandidateCount = 100;
+
+  llvm::SmallVector CommaSeparatedValues;
+
+  std::vector RealRequests;
+  for (auto Line : Strings) {
+if (RequestMatcher.match(Line, &Matches)) {
+  R.Query = Matches[1];
+  CommaSeparatedValues.clear();
+  Line.split(CommaSeparatedValues, ',');
+  R.Scopes.clear();
+  for (auto C : CommaSeparatedValues) {
+R.Scopes.push_back(C);
+  }
+  RealRequests.push_back(R);
+}
+  }
+  return RealRequests;
+}
+
+static void BuildMem(benchmark::State &State) {
+  for (auto _ : State)
+buildMem();
+}
+BENCHMARK(BuildMem);
+
+static void MemQueries(benchmark::State &State) {
+  const auto Mem = buildMem();
+  const auto Requests = extractQueriesFromLogs();
+  for (auto _ : State)
+for (const auto &Request : Requests)
+  Mem->fuzzyFind(Request, [](const Symbol &S) {});
+}
+BENCHMARK(MemQueries);
+
+static void BuildDex(benchmark::State &State) {
+  for (auto _ : State)
+buildDex();
+}
+BENCHMARK(BuildDex);
+
+static void DexQueries(benchmark::State &State) {
+  const auto Dex = buildDex();
+  const auto Requests = extractQueriesFromLogs();
+  for (auto _ : State)
+for (const auto &Request : Requests)
+  Dex->fuzzyFind(Request, [](const Symbol &S) {});
+}
+BENCHMARK(DexQueries);
+
+} // namespace
+} // namespace clangd
+} // namespace clang
+
+int main(int argc, char *argv[]) {
+  if (argc < 4) {
+llvm::errs() << "Usage: " << argv[0]
+ << " global-symbol-index.yaml fuzzy-find-requests.log "
+"/path/to/llvm BENCHMARK_OPTIONS...\n";
+return -1;
+  }
+  IndexFilename = argv[1];
+  LogFilename = argv[2];
+  LLVMRootPath = argv[3];
+  // Trim first two arguments of the benchmark invocation.
+  argv += 3;
+  argc -= 3;
+  ::benchmark::Initialize(&argc, argv);
+  ::benchmark::RunSpecifiedBenchmarks();
+}
Index: clang-tools-extra/clangd/benchmarks/CMakeLists.txt
===
--- /dev/null
+++ clang-tools-extra/clangd/benchmarks/CMakeLists.txt
@@ -0,0 +1,9 @@
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../)
+
+add_benchmark(IndexBenchmark IndexBenchmark.cpp)
+
+target_link_libraries(IndexBenchmark
+  PRIVATE
+  clangDaemon
+  LLVMSupport
+  )
Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -74,3 +74,7 @@
 endif()
 add_subdirectory(tool)
 add_subdirectory(global-symbol-builder)
+
+if (LLVM_INCLUDE_BENCHMARKS)
+  add_subdirectory(benchmarks)
+endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.ll

[PATCH] D49916: [CodeGen] Add to emitted DebugLoc information about coverage when it's required

[calixte via Phabricator via cfe-commits]

calixte updated this revision to Diff 164851.
calixte added a comment.

- Add some ImplicitCode when there is some stuff at the end of a function
- Fix the tests


Repository:
  rC Clang

https://reviews.llvm.org/D49916

Files:
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  test/CodeGen/debug-info-scope-file.c
  test/CodeGenCXX/debug-info-inheriting-constructor.cpp
  test/CodeGenCXX/debug-info-nested-exprs.cpp
  test/CodeGenCXX/linetable-virtual-variadic.cpp
  test/CodeGenObjC/arc-linetable.m
  test/CodeGenObjC/debug-info-blocks.m

Index: test/CodeGenObjC/debug-info-blocks.m
===
--- test/CodeGenObjC/debug-info-blocks.m
+++ test/CodeGenObjC/debug-info-blocks.m
@@ -18,11 +18,9 @@
 // CHECK: call {{.*}}, !dbg ![[DBG_LINE:[0-9]+]]
 // CHECK-NOT: ret
 // CHECK: load {{.*}}, !dbg ![[COPY_LINE:[0-9]+]]
-// CHECK: ret void, !dbg ![[COPY_LINE]]
 // CHECK: define {{.*}} @__destroy_helper_block_{{.*}}(i8*)
 // CHECK-NOT: ret
 // CHECK: load {{.*}}, !dbg ![[DESTROY_LINE:[0-9]+]]
-// CHECK: ret void, !dbg ![[DESTROY_LINE]]
 
 typedef unsigned int NSUInteger;
 
Index: test/CodeGenObjC/arc-linetable.m
===
--- test/CodeGenObjC/arc-linetable.m
+++ test/CodeGenObjC/arc-linetable.m
@@ -60,7 +60,8 @@
 - (int)testNoSideEffect:(NSString *)foo {
   int x = 1;
   return 1; // Return expression
-  // CHECK: ![[RET1]] = !DILocation(line: [[@LINE+1]], scope: ![[TESTNOSIDEEFFECT]])
+  // CHECK: ![[RET1]] = !DILocation(line: [[@LINE+1]], scope:
+  // ![[TESTNOSIDEEFFECT]], isImplicitCode: true)
 }   // Cleanup + Ret
 
 - (int)testNoCleanup {
Index: test/CodeGenCXX/linetable-virtual-variadic.cpp
===
--- test/CodeGenCXX/linetable-virtual-variadic.cpp
+++ test/CodeGenCXX/linetable-virtual-variadic.cpp
@@ -11,12 +11,12 @@
 
 void Derived::VariadicFunction(...) { }
 
-// CHECK: define void @_ZN7Derived16VariadicFunctionEz({{.*}} !dbg ![[SP:[0-9]+]]
-// CHECK: ret void, !dbg ![[LOC:[0-9]+]]
-// CHECK: define void @_ZT{{.+}}N7Derived16VariadicFunctionEz({{.*}} !dbg ![[SP_I:[0-9]+]]
-// CHECK: ret void, !dbg ![[LOC_I:[0-9]+]]
+// CHECK: define void @_ZN7Derived16VariadicFunctionEz({{.*}} !dbg
+// ![[SP:[0-9]+]] CHECK: ret void, !dbg ![[LOC:[0-9]+]] CHECK: define void
+// @_ZT{{.+}}N7Derived16VariadicFunctionEz({{.*}} !dbg ![[SP_I:[0-9]+]] CHECK:
+// ret void, !dbg ![[LOC_I:[0-9]+]]
 //
 // CHECK: ![[SP]] = distinct !DISubprogram(name: "VariadicFunction"
-// CHECK: ![[LOC]] = !DILocation({{.*}}scope: ![[SP]])
+// CHECK: ![[LOC]] = !DILocation({{.*}}scope: ![[SP]], isImplicitCode: true)
 // CHECK: ![[SP_I]] = distinct !DISubprogram(name: "VariadicFunction"
-// CHECK: ![[LOC_I]] = !DILocation({{.*}}scope: ![[SP_I]])
+// CHECK: ![[LOC_I]] = !DILocation({{.*}}scope: ![[SP_I]], isImplicitCode: true)
Index: test/CodeGenCXX/debug-info-nested-exprs.cpp
===
--- test/CodeGenCXX/debug-info-nested-exprs.cpp
+++ test/CodeGenCXX/debug-info-nested-exprs.cpp
@@ -37,7 +37,6 @@
   // NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
   // NESTED: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
   // NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
-  // NESTED: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[BAR]]
   // COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
   // COLUMNS: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
   // COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
@@ -98,7 +97,6 @@
   // NONEST: store i32 %{{[^,]+}}, i32* %d,{{.*}} !dbg ![[DECLD]]
   // NESTED: call i32 @{{.*}}noargs{{.*}}, !dbg ![[DNOARGS:[0-9]+]]
   // NESTED: call i32 @{{.*}}onearg{{.*}}, !dbg ![[DECLD:[0-9]+]]
-  // NESTED: store i32 %{{[^,]+}}, i32* %d,{{.*}} !dbg ![[DECLD]]
   // COLUMNS: call i32 @{{.*}}noargs{{.*}}, !dbg ![[DNOARGS:[0-9]+]]
   // COLUMNS: call i32 @{{.*}}onearg{{.*}}, !dbg ![[DONEARG:[0-9]+]]
   // COLUMNS: store i32 %{{[^,]+}}, i32* %d,{{.*}} !dbg ![[DECLD:[0-9]+]]
@@ -178,16 +176,16 @@
 // NESTED: ![[RETSUB]] = !DILocation(
 // NESTED: ![[RETMUL]] = !DILocation(
 
-// COLUMNS: ![[DECLA]] = !DILocation(
 // COLUMNS: ![[BAR]] = !DILocation(
 // COLUMNS: ![[BAZ]] = !DILocation(
 // COLUMNS: ![[QUX]] = !DILocation(
+// COLUMNS: ![[DECLA]] = !DILocation(
 // COLUMNS: ![[ILOC]] = !DILocation(
 // COLUMNS: ![[BLOC]] = !DILocation(
 // COLUMNS: ![[CLOC]] = !DILocation(
-// COLUMNS: ![[DECLD]] = !DILocation(
 // COLUMNS: ![[DNOARGS]] = !DILocation(
 // COLUMNS: ![[DONEARG]] = !DILocation(
+// COLUMNS: ![[DECLD]] = !DILocation(
 // COLUMNS: ![[SETDNOARGS]] = !DILocation(
 // COLUMNS: ![[SETDONEARG]] = !DILocation(
 // COLUMNS: ![[SETD]] = !DILocation(
Index: test/CodeGenCXX/debug-info-inheriting-constructor.cpp
===
--- test/CodeGenCXX/de

[PATCH] D51880: [ASTMatchers] add two matchers for dependent expressions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D51880#1230221, @JonasToth wrote:

> In https://reviews.llvm.org/D51880#1229513, @aaron.ballman wrote:
>
> > Missing tests and changes to Registry.cpp for dynamic matchers.
> >
> > Also, do you want to add `isInstantiationDependent()` at the same time, 
> > given the relationship with the other two matchers?
>
>
> Do you mean a matcher that does `return Node.isValueDependent() || 
> Node.isTypeDependent()` or `hasAncestor(expr(anyOf(isValueDependent(), 
> isTypeDependent(`?


I mean a matcher that does `return Node.isInstantiationDependent();` over 
`Expr`.


Repository:
  rC Clang

https://reviews.llvm.org/D51880



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

[clang-tools-extra] r341929 - [clang-tidy] Insert absl::StrAppend when replacing StrCat.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Tue Sep 11 05:19:45 2018
New Revision: 341929

URL: http://llvm.org/viewvc/llvm-project?rev=341929&view=rev
Log:
[clang-tidy] Insert absl::StrAppend when replacing StrCat.

There might be no using decl for StrAppend around, inserting the
qualified name is less likely to break things.

Modified:
clang-tools-extra/trunk/clang-tidy/abseil/StrCatAppendCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/abseil-str-cat-append.cpp

Modified: clang-tools-extra/trunk/clang-tidy/abseil/StrCatAppendCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/StrCatAppendCheck.cpp?rev=341929&r1=341928&r2=341929&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/StrCatAppendCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/StrCatAppendCheck.cpp Tue Sep 11 
05:19:45 2018
@@ -93,7 +93,7 @@ void StrCatAppendCheck::check(const Matc
   << FixItHint::CreateReplacement(
  CharSourceRange::getTokenRange(Op->getBeginLoc(),
 Call->getCallee()->getEndLoc()),
- "StrAppend")
+ "absl::StrAppend")
   << FixItHint::CreateInsertion(Call->getArg(0)->getBeginLoc(), "&");
 }
 

Modified: clang-tools-extra/trunk/test/clang-tidy/abseil-str-cat-append.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/abseil-str-cat-append.cpp?rev=341929&r1=341928&r2=341929&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/abseil-str-cat-append.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/abseil-str-cat-append.cpp Tue Sep 
11 05:19:45 2018
@@ -97,7 +97,7 @@ void Bar() {
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: call to 'absl::StrCat' has no 
effect
   A = StrCat(A, B);
 // CHECK-MESSAGES: [[@LINE-1]]:3: warning: call 'absl::StrAppend' instead of 
'absl::StrCat' when appending to a string to avoid a performance penalty
-// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+// CHECK-FIXES: {{^}}  absl::StrAppend(&A, B);
   B = StrCat(A, B);
 
 #define M(X) X = StrCat(X, A)
@@ -117,13 +117,13 @@ void OutsideAbsl() {
   std::string A, B;
   A = absl::StrCat(A, B);
 // CHECK-MESSAGES: [[@LINE-1]]:3: warning: call 'absl::StrAppend' instead of 
'absl::StrCat' when appending to a string to avoid a performance penalty
-// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+// CHECK-FIXES: {{^}}  absl::StrAppend(&A, B);
 }
 
-void OutisdeUsingAbsl() {
+void OutsideUsingAbsl() {
   std::string A, B;
   using absl::StrCat;
   A = StrCat(A, B);
 // CHECK-MESSAGES: [[@LINE-1]]:3: warning: call 'absl::StrAppend' instead of 
'absl::StrCat' when appending to a string to avoid a performance penalty
-// CHECK-FIXES: {{^}}  StrAppend(&A, B);
+// CHECK-FIXES: {{^}}  absl::StrAppend(&A, B);
 }


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

[PATCH] D51917: [CodeCompletion] Enable signature help when initializing class/struct/union members.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: include/clang/Sema/Sema.h:4571
 
+  /// Tries to get decleration for a member field.
+  ValueDecl *tryLookupCtorInitMemberDecl(CXXRecordDecl *ClassDecl,

s/decleration/declaration.

Maybe even remove the comment? the name is self-descriptive


Repository:
  rC Clang

https://reviews.llvm.org/D51917



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

r341930 - [python bindings] Expose getNumTemplateArguments

[Jonathan Coe via cfe-commits]

Author: jbcoe
Date: Tue Sep 11 05:44:52 2018
New Revision: 341930

URL: http://llvm.org/viewvc/llvm-project?rev=341930&view=rev
Log:
[python bindings] Expose getNumTemplateArguments

Expose the C bindings for clang_Type_getNumTemplateArguments() and
clang_Type_getTemplateArgumentAsType() in the python API.

Patch by kjteske (Kyle Teske).

Reviewed By: jbcoe

Subscribers: cfe-commits

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

Modified:
cfe/trunk/bindings/python/clang/cindex.py
cfe/trunk/bindings/python/tests/cindex/test_type.py

Modified: cfe/trunk/bindings/python/clang/cindex.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/clang/cindex.py?rev=341930&r1=341929&r2=341930&view=diff
==
--- cfe/trunk/bindings/python/clang/cindex.py (original)
+++ cfe/trunk/bindings/python/clang/cindex.py Tue Sep 11 05:44:52 2018
@@ -2254,6 +2254,12 @@ class Type(Structure):
 
 return res
 
+def get_num_template_arguments(self):
+return conf.lib.clang_Type_getNumTemplateArguments(self)
+
+def get_template_argument_type(self, num):
+return conf.lib.clang_Type_getTemplateArgumentAsType(self, num)
+
 def get_canonical(self):
 """
 Return the canonical type for a Type.
@@ -3999,6 +4005,15 @@ functionList = [
Type,
Type.from_result),
 
+  ("clang_Type_getNumTemplateArguments",
+   [Type],
+   c_int),
+
+  ("clang_Type_getTemplateArgumentAsType",
+   [Type, c_uint],
+   Type,
+   Type.from_result),
+
   ("clang_Type_getOffsetOf",
[Type, c_interop_string],
c_longlong),

Modified: cfe/trunk/bindings/python/tests/cindex/test_type.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/tests/cindex/test_type.py?rev=341930&r1=341929&r2=341930&view=diff
==
--- cfe/trunk/bindings/python/tests/cindex/test_type.py (original)
+++ cfe/trunk/bindings/python/tests/cindex/test_type.py Tue Sep 11 05:44:52 2018
@@ -436,3 +436,28 @@ class TestType(unittest.TestCase):
 
 self.assertIsNotNone(testInteger, "Could not find testInteger.")
 self.assertEqual(testInteger.type.get_address_space(), 2)
+
+def test_template_arguments(self):
+source = """
+class Foo {
+};
+template 
+class Template {
+};
+Template instance;
+int bar;
+"""
+tu = get_tu(source, lang='cpp')
+
+# Varible with a template argument.
+cursor = get_cursor(tu, 'instance')
+cursor_type = cursor.type
+self.assertEqual(cursor.kind, CursorKind.VAR_DECL)
+self.assertEqual(cursor_type.spelling, 'Template')
+self.assertEqual(cursor_type.get_num_template_arguments(), 1)
+template_type = cursor_type.get_template_argument_type(0)
+self.assertEqual(template_type.spelling, 'Foo')
+
+# Variable without a template argument.
+cursor = get_cursor(tu, 'bar')
+self.assertEqual(cursor.get_num_template_arguments(), -1)


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

[PATCH] D51299: [python bindings] Expose template argument API for Type

[Jonathan B Coe via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC341930: [python bindings] Expose getNumTemplateArguments 
(authored by jbcoe, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51299?vs=162667&id=164854#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51299

Files:
  bindings/python/clang/cindex.py
  bindings/python/tests/cindex/test_type.py


Index: bindings/python/clang/cindex.py
===
--- bindings/python/clang/cindex.py
+++ bindings/python/clang/cindex.py
@@ -2254,6 +2254,12 @@
 
 return res
 
+def get_num_template_arguments(self):
+return conf.lib.clang_Type_getNumTemplateArguments(self)
+
+def get_template_argument_type(self, num):
+return conf.lib.clang_Type_getTemplateArgumentAsType(self, num)
+
 def get_canonical(self):
 """
 Return the canonical type for a Type.
@@ -3999,6 +4005,15 @@
Type,
Type.from_result),
 
+  ("clang_Type_getNumTemplateArguments",
+   [Type],
+   c_int),
+
+  ("clang_Type_getTemplateArgumentAsType",
+   [Type, c_uint],
+   Type,
+   Type.from_result),
+
   ("clang_Type_getOffsetOf",
[Type, c_interop_string],
c_longlong),
Index: bindings/python/tests/cindex/test_type.py
===
--- bindings/python/tests/cindex/test_type.py
+++ bindings/python/tests/cindex/test_type.py
@@ -436,3 +436,28 @@
 
 self.assertIsNotNone(testInteger, "Could not find testInteger.")
 self.assertEqual(testInteger.type.get_address_space(), 2)
+
+def test_template_arguments(self):
+source = """
+class Foo {
+};
+template 
+class Template {
+};
+Template instance;
+int bar;
+"""
+tu = get_tu(source, lang='cpp')
+
+# Varible with a template argument.
+cursor = get_cursor(tu, 'instance')
+cursor_type = cursor.type
+self.assertEqual(cursor.kind, CursorKind.VAR_DECL)
+self.assertEqual(cursor_type.spelling, 'Template')
+self.assertEqual(cursor_type.get_num_template_arguments(), 1)
+template_type = cursor_type.get_template_argument_type(0)
+self.assertEqual(template_type.spelling, 'Foo')
+
+# Variable without a template argument.
+cursor = get_cursor(tu, 'bar')
+self.assertEqual(cursor.get_num_template_arguments(), -1)


Index: bindings/python/clang/cindex.py
===
--- bindings/python/clang/cindex.py
+++ bindings/python/clang/cindex.py
@@ -2254,6 +2254,12 @@
 
 return res
 
+def get_num_template_arguments(self):
+return conf.lib.clang_Type_getNumTemplateArguments(self)
+
+def get_template_argument_type(self, num):
+return conf.lib.clang_Type_getTemplateArgumentAsType(self, num)
+
 def get_canonical(self):
 """
 Return the canonical type for a Type.
@@ -3999,6 +4005,15 @@
Type,
Type.from_result),
 
+  ("clang_Type_getNumTemplateArguments",
+   [Type],
+   c_int),
+
+  ("clang_Type_getTemplateArgumentAsType",
+   [Type, c_uint],
+   Type,
+   Type.from_result),
+
   ("clang_Type_getOffsetOf",
[Type, c_interop_string],
c_longlong),
Index: bindings/python/tests/cindex/test_type.py
===
--- bindings/python/tests/cindex/test_type.py
+++ bindings/python/tests/cindex/test_type.py
@@ -436,3 +436,28 @@
 
 self.assertIsNotNone(testInteger, "Could not find testInteger.")
 self.assertEqual(testInteger.type.get_address_space(), 2)
+
+def test_template_arguments(self):
+source = """
+class Foo {
+};
+template 
+class Template {
+};
+Template instance;
+int bar;
+"""
+tu = get_tu(source, lang='cpp')
+
+# Varible with a template argument.
+cursor = get_cursor(tu, 'instance')
+cursor_type = cursor.type
+self.assertEqual(cursor.kind, CursorKind.VAR_DECL)
+self.assertEqual(cursor_type.spelling, 'Template')
+self.assertEqual(cursor_type.get_num_template_arguments(), 1)
+template_type = cursor_type.get_template_argument_type(0)
+self.assertEqual(template_type.spelling, 'Foo')
+
+# Variable without a template argument.
+cursor = get_cursor(tu, 'bar')
+self.assertEqual(cursor.get_num_template_arguments(), -1)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51299: [python bindings] Expose template argument API for Type

[Jonathan B Coe via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL341930: [python bindings] Expose getNumTemplateArguments 
(authored by jbcoe, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D51299

Files:
  cfe/trunk/bindings/python/clang/cindex.py
  cfe/trunk/bindings/python/tests/cindex/test_type.py


Index: cfe/trunk/bindings/python/clang/cindex.py
===
--- cfe/trunk/bindings/python/clang/cindex.py
+++ cfe/trunk/bindings/python/clang/cindex.py
@@ -2254,6 +2254,12 @@
 
 return res
 
+def get_num_template_arguments(self):
+return conf.lib.clang_Type_getNumTemplateArguments(self)
+
+def get_template_argument_type(self, num):
+return conf.lib.clang_Type_getTemplateArgumentAsType(self, num)
+
 def get_canonical(self):
 """
 Return the canonical type for a Type.
@@ -3999,6 +4005,15 @@
Type,
Type.from_result),
 
+  ("clang_Type_getNumTemplateArguments",
+   [Type],
+   c_int),
+
+  ("clang_Type_getTemplateArgumentAsType",
+   [Type, c_uint],
+   Type,
+   Type.from_result),
+
   ("clang_Type_getOffsetOf",
[Type, c_interop_string],
c_longlong),
Index: cfe/trunk/bindings/python/tests/cindex/test_type.py
===
--- cfe/trunk/bindings/python/tests/cindex/test_type.py
+++ cfe/trunk/bindings/python/tests/cindex/test_type.py
@@ -436,3 +436,28 @@
 
 self.assertIsNotNone(testInteger, "Could not find testInteger.")
 self.assertEqual(testInteger.type.get_address_space(), 2)
+
+def test_template_arguments(self):
+source = """
+class Foo {
+};
+template 
+class Template {
+};
+Template instance;
+int bar;
+"""
+tu = get_tu(source, lang='cpp')
+
+# Varible with a template argument.
+cursor = get_cursor(tu, 'instance')
+cursor_type = cursor.type
+self.assertEqual(cursor.kind, CursorKind.VAR_DECL)
+self.assertEqual(cursor_type.spelling, 'Template')
+self.assertEqual(cursor_type.get_num_template_arguments(), 1)
+template_type = cursor_type.get_template_argument_type(0)
+self.assertEqual(template_type.spelling, 'Foo')
+
+# Variable without a template argument.
+cursor = get_cursor(tu, 'bar')
+self.assertEqual(cursor.get_num_template_arguments(), -1)


Index: cfe/trunk/bindings/python/clang/cindex.py
===
--- cfe/trunk/bindings/python/clang/cindex.py
+++ cfe/trunk/bindings/python/clang/cindex.py
@@ -2254,6 +2254,12 @@
 
 return res
 
+def get_num_template_arguments(self):
+return conf.lib.clang_Type_getNumTemplateArguments(self)
+
+def get_template_argument_type(self, num):
+return conf.lib.clang_Type_getTemplateArgumentAsType(self, num)
+
 def get_canonical(self):
 """
 Return the canonical type for a Type.
@@ -3999,6 +4005,15 @@
Type,
Type.from_result),
 
+  ("clang_Type_getNumTemplateArguments",
+   [Type],
+   c_int),
+
+  ("clang_Type_getTemplateArgumentAsType",
+   [Type, c_uint],
+   Type,
+   Type.from_result),
+
   ("clang_Type_getOffsetOf",
[Type, c_interop_string],
c_longlong),
Index: cfe/trunk/bindings/python/tests/cindex/test_type.py
===
--- cfe/trunk/bindings/python/tests/cindex/test_type.py
+++ cfe/trunk/bindings/python/tests/cindex/test_type.py
@@ -436,3 +436,28 @@
 
 self.assertIsNotNone(testInteger, "Could not find testInteger.")
 self.assertEqual(testInteger.type.get_address_space(), 2)
+
+def test_template_arguments(self):
+source = """
+class Foo {
+};
+template 
+class Template {
+};
+Template instance;
+int bar;
+"""
+tu = get_tu(source, lang='cpp')
+
+# Varible with a template argument.
+cursor = get_cursor(tu, 'instance')
+cursor_type = cursor.type
+self.assertEqual(cursor.kind, CursorKind.VAR_DECL)
+self.assertEqual(cursor_type.spelling, 'Template')
+self.assertEqual(cursor_type.get_num_template_arguments(), 1)
+template_type = cursor_type.get_template_argument_type(0)
+self.assertEqual(template_type.spelling, 'Foo')
+
+# Variable without a template argument.
+cursor = get_cursor(tu, 'bar')
+self.assertEqual(cursor.get_num_template_arguments(), -1)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51926: [scan-build-py] Prevent crashes of CTU analysis by suppressing warnings

[Lorinc Balog via Phabricator via cfe-commits]

lorincbalog created this revision.
lorincbalog added reviewers: rizsotto.mailinglist, dcoughlin.
Herald added subscribers: cfe-commits, whisperity.

During the mapping of functions for cross-translation unit analysis, the 
functions' names and the containing files are collected by clang-func-mapping. 
Warnings are stored in the collection too, which later makes the parsing of the 
collection fail. Proposed solution is to suppress all warnings during the 
function mapping.


Repository:
  rC Clang

https://reviews.llvm.org/D51926

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


Index: tools/scan-build-py/libscanbuild/analyze.py
===
--- tools/scan-build-py/libscanbuild/analyze.py
+++ tools/scan-build-py/libscanbuild/analyze.py
@@ -598,6 +598,7 @@
 funcmap_command.append(opts['file'])
 funcmap_command.append('--')
 funcmap_command.extend(args)
+funcmap_command.append('-w')
 logging.debug("Generating function map using '%s'", funcmap_command)
 func_src_list = run_command(funcmap_command, cwd=opts['directory'])
 func_ast_list = func_map_list_src_to_ast(func_src_list)


Index: tools/scan-build-py/libscanbuild/analyze.py
===
--- tools/scan-build-py/libscanbuild/analyze.py
+++ tools/scan-build-py/libscanbuild/analyze.py
@@ -598,6 +598,7 @@
 funcmap_command.append(opts['file'])
 funcmap_command.append('--')
 funcmap_command.extend(args)
+funcmap_command.append('-w')
 logging.debug("Generating function map using '%s'", funcmap_command)
 func_src_list = run_command(funcmap_command, cwd=opts['directory'])
 func_ast_list = func_map_list_src_to_ast(func_src_list)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r341933 - [NFC][clangd] fix warning for extra semicolon

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Tue Sep 11 06:01:49 2018
New Revision: 341933

URL: http://llvm.org/viewvc/llvm-project?rev=341933&view=rev
Log:
[NFC][clangd] fix warning for extra semicolon

Modified:
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp

Modified: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp?rev=341933&r1=341932&r2=341933&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp Tue Sep 11 06:01:49 
2018
@@ -1210,7 +1210,7 @@ TEST(FindReferences, NeedsIndex) {
   TU.Code = ("\n\n" + Main.code()).str();
   EXPECT_THAT(findReferences(AST, Main.point(), TU.index().get()),
   ElementsAre(RangeIs(Main.range(;
-};
+}
 
 TEST(FindReferences, NoQueryForLocalSymbols) {
   struct RecordingIndex : public MemIndex {
@@ -1244,7 +1244,7 @@ TEST(FindReferences, NoQueryForLocalSymb
 else
   EXPECT_EQ(Rec.RefIDs, llvm::None) << T.AnnotatedCode;
   }
-};
+}
 
 } // namespace
 } // namespace clangd


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

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

+1 to adding an option to drop arguments from snippets and removing the option 
for the fixes.




Comment at: clangd/tool/ClangdMain.cpp:197
 
+static llvm::cl::opt IncludeFixIts(
+"include-fixits",

sammccall wrote:
> ilya-biryukov wrote:
> > sammccall wrote:
> > > ilya-biryukov wrote:
> > > > sammccall wrote:
> > > > > ilya-biryukov wrote:
> > > > > > I wonder if we should make the `IncludeFixIts` option hidden?
> > > > > > It currently only works for our YCM integration, VSCode and other 
> > > > > > clients break.
> > > > > why would a user want to turn this on or off?
> > > > Ideally, we want to have it always on.
> > > > However, all editors interpret the results we return in different ways. 
> > > > This is a temporary option until we can define how text edits are 
> > > > handled by LSP.
> > > > We filed the bugs, will dig them up on Monday.
> > > Do we have any more details here? I'm still skeptical that exposing this 
> > > to end users will help at all, this seems likely that it should be a 
> > > capability if we do need it.
> > No updates  on the issue. Here it is:
> > https://github.com/Microsoft/language-server-protocol/issues/543
> > 
> > Not sure capability is the right thing there, the problem is that 
> > additionalTextEdits are underspecified and implemented differently in every 
> > client. What we need is a fix in the protocol and fixes in all the clients.
> > 
> > Sadly, this only works in YCM-based completer for now (of all we tested)
> Sure, sounds like protocol fix is the long-term answer. I don't think adding 
> user-facing options are better than nothing. If YCM does the right thing and 
> we want to disable it for everyone not on YCM, we can add a 
> `textEditsAreAppliedInOrder` capability to the YCM completer and treat that 
> as an opt-in. It's not clear what the advantage of a user-facing flag over an 
> editor-facing capability is for this purpose.
> 
> Mostly given LSP is unclear here it seems this feature isn't ready for 
> prime-time.
> Could we fix it on our side by coalescing multiple edits into a single one?
I agree, the feature is not very useful if it breaks everywhere. Removing the 
option and exploring other ways to do it LG.

> Could we fix it on our side by coalescing multiple edits into a single one?
We tried to combine additionalTextEdits into the main textEdit, that's what 
works in YCM.
However, it did not help in other editors, they misinterpret a main textedit 
(each in a different way) if it affects anything before the start of the 
completion identifier, which is exactly the case for the only fix we have at 
the time, that is `. to ->`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51214



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

[PATCH] D51747: [clangd] Implement deprecation diagnostics with lower severity.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In https://reviews.llvm.org/D51747#1229066, @sammccall wrote:

> In https://reviews.llvm.org/D51747#1228919, @ilya-biryukov wrote:
>
> > > Most of the value of adding an option is: if someone complains, we can 
> > > tell them to go away :-) One possible corollary is: we shouldn't add the 
> > > option until someone complains. I'm not 100% sure about that, though - 
> > > not totally opposed to an option here.
> >
> > Any thoughts on tampering with provided compile args in the first place? Is 
> > it fine for clangd to be opinionated about the warnings we choose to always 
> > show to the users?
> >  E.g. I'm a big fan of various uninitialized warnings, but nevertheless 
> > don't think clangd should force them on everyone.
>
>
> A few thoughts here:
>
> - does CDB describe user or project preferences? unclear.
> - "show this warning for code I build" is a higher bar than "show this 
> warning for code I edit". So CDB probably enables too few warnings.
> - Some warnings play well with -Werror (like uninit warnings), some don't 
> (like deprecated). -Werror projects often disable interesting warnings.
>
>   I'm not sure we should strictly follow the CDB, but the bar to override it 
> should probably be high. Maybe the (default) behavior here should be "add 
> -Wdeprecated -Wno-error=deprecated if -Werror is set"?


I agree with checking for -Werror and then providing -Wno-error as well, if 
-Wdeprecated was not added already.
Then keeping it as warnings shouldn't be too much of a problem except the case 
you mentioned, if user wants to see all diagnostics as a list suddenly they 
will get deprecations in that list as well :(.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51747



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

[PATCH] D51747: [clangd] Implement deprecation diagnostics with lower severity.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In https://reviews.llvm.org/D51747#1230420, @kadircet wrote:

> In https://reviews.llvm.org/D51747#1229066, @sammccall wrote:
>
> > In https://reviews.llvm.org/D51747#1228919, @ilya-biryukov wrote:
> >
> > > > Most of the value of adding an option is: if someone complains, we can 
> > > > tell them to go away :-) One possible corollary is: we shouldn't add 
> > > > the option until someone complains. I'm not 100% sure about that, 
> > > > though - not totally opposed to an option here.
> > >
> > > Any thoughts on tampering with provided compile args in the first place? 
> > > Is it fine for clangd to be opinionated about the warnings we choose to 
> > > always show to the users?
> > >  E.g. I'm a big fan of various uninitialized warnings, but nevertheless 
> > > don't think clangd should force them on everyone.
> >
> >
> > A few thoughts here:
> >
> > - does CDB describe user or project preferences? unclear.
> > - "show this warning for code I build" is a higher bar than "show this 
> > warning for code I edit". So CDB probably enables too few warnings.
> > - Some warnings play well with -Werror (like uninit warnings), some don't 
> > (like deprecated). -Werror projects often disable interesting warnings.
> >
> >   I'm not sure we should strictly follow the CDB, but the bar to override 
> > it should probably be high. Maybe the (default) behavior here should be 
> > "add -Wdeprecated -Wno-error=deprecated if -Werror is set"?
>
>
> I agree with checking for -Werror and then providing -Wno-error as well, if 
> -Wdeprecated was not added already.


I'm nervous enough about trying to ad-hoc parse the flags that I'd be tempted 
just to ad -Wno-error=deprecated without checking to see if -Werror is set - 
it's a no-op if not. But up to you.

Ilya's suggestion of just respecting the compilation database also seems OK. It 
seems a bit sad for the users who won't see deprecation warnings because their 
project doesn't want to show them at build time. I'd lean towards adding this 
to see if anyone complains, but happy with whatever you two can agree on.

> Then keeping it as warnings shouldn't be too much of a problem except the 
> case you mentioned, if user wants to see all diagnostics as a list suddenly 
> they will get deprecations in that list as well :(.

I don't think that making them notes will avoid this though :-/


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51747



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

[PATCH] D49916: [CodeGen] Add to emitted DebugLoc information about coverage when it's required

[calixte via Phabricator via cfe-commits]

calixte updated this revision to Diff 164860.
calixte added a comment.

- only put an ImplicitCode for EmitLandingPad
- fix tests (were broken with clanf-format-diff)


Repository:
  rC Clang

https://reviews.llvm.org/D49916

Files:
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  lib/CodeGen/CGException.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  test/CodeGen/debug-info-scope-file.c
  test/CodeGenCXX/debug-info-inheriting-constructor.cpp
  test/CodeGenCXX/linetable-virtual-variadic.cpp
  test/CodeGenObjC/arc-linetable.m
  test/CodeGenObjC/debug-info-blocks.m

Index: test/CodeGenObjC/debug-info-blocks.m
===
--- test/CodeGenObjC/debug-info-blocks.m
+++ test/CodeGenObjC/debug-info-blocks.m
@@ -18,11 +18,9 @@
 // CHECK: call {{.*}}, !dbg ![[DBG_LINE:[0-9]+]]
 // CHECK-NOT: ret
 // CHECK: load {{.*}}, !dbg ![[COPY_LINE:[0-9]+]]
-// CHECK: ret void, !dbg ![[COPY_LINE]]
 // CHECK: define {{.*}} @__destroy_helper_block_{{.*}}(i8*)
 // CHECK-NOT: ret
 // CHECK: load {{.*}}, !dbg ![[DESTROY_LINE:[0-9]+]]
-// CHECK: ret void, !dbg ![[DESTROY_LINE]]
 
 typedef unsigned int NSUInteger;
 
Index: test/CodeGenObjC/arc-linetable.m
===
--- test/CodeGenObjC/arc-linetable.m
+++ test/CodeGenObjC/arc-linetable.m
@@ -60,7 +60,7 @@
 - (int)testNoSideEffect:(NSString *)foo {
   int x = 1;
   return 1; // Return expression
-  // CHECK: ![[RET1]] = !DILocation(line: [[@LINE+1]], scope: ![[TESTNOSIDEEFFECT]])
+  // CHECK: ![[RET1]] = !DILocation(line: [[@LINE+1]], scope: ![[TESTNOSIDEEFFECT]], isImplicitCode: true)
 }   // Cleanup + Ret
 
 - (int)testNoCleanup {
Index: test/CodeGenCXX/linetable-virtual-variadic.cpp
===
--- test/CodeGenCXX/linetable-virtual-variadic.cpp
+++ test/CodeGenCXX/linetable-virtual-variadic.cpp
@@ -17,6 +17,6 @@
 // CHECK: ret void, !dbg ![[LOC_I:[0-9]+]]
 //
 // CHECK: ![[SP]] = distinct !DISubprogram(name: "VariadicFunction"
-// CHECK: ![[LOC]] = !DILocation({{.*}}scope: ![[SP]])
+// CHECK: ![[LOC]] = !DILocation({{.*}}scope: ![[SP]], isImplicitCode: true)
 // CHECK: ![[SP_I]] = distinct !DISubprogram(name: "VariadicFunction"
-// CHECK: ![[LOC_I]] = !DILocation({{.*}}scope: ![[SP_I]])
+// CHECK: ![[LOC_I]] = !DILocation({{.*}}scope: ![[SP_I]], isImplicitCode: true)
Index: test/CodeGenCXX/debug-info-inheriting-constructor.cpp
===
--- test/CodeGenCXX/debug-info-inheriting-constructor.cpp
+++ test/CodeGenCXX/debug-info-inheriting-constructor.cpp
@@ -21,5 +21,5 @@
 // CHECK-DAG: ![[LOC]] = !DILocation(line: 0, scope: ![[A]], inlinedAt: ![[INL:[0-9]+]])
 // CHECK-DAG: ![[INL]] = !DILocation(line: [[@LINE+1]], scope: ![[FOO]])
   B b(0);
-// CHECK: ![[NOINL]] = !DILocation(line: [[@LINE+1]], scope: !{{[0-9]+}})
+  // CHECK: ![[NOINL]] = !DILocation(line: [[@LINE+1]], scope: !{{[0-9]+}}, isImplicitCode: true)
 }
Index: test/CodeGen/debug-info-scope-file.c
===
--- test/CodeGen/debug-info-scope-file.c
+++ test/CodeGen/debug-info-scope-file.c
@@ -5,10 +5,11 @@
 
 // CHECK: ret void, !dbg [[F1_LINE:![0-9]*]]
 // CHECK: ret void, !dbg [[F2_LINE:![0-9]*]]
-// CHECK: [[F1:![0-9]*]] = distinct !DISubprogram(name: "f1",{{.*}} isDefinition: true
-// CHECK: [[F1_LINE]] = !DILocation({{.*}}, scope: [[F1]])
-// CHECK: [[F2:![0-9]*]] = distinct !DISubprogram(name: "f2",{{.*}} isDefinition: true
-// CHECK: [[F2_LINE]] = !DILocation({{.*}}, scope: [[F2]])
+// CHECK: [[F1:![0-9]*]] = distinct !DISubprogram(name: "f1",{{.*}}
+// isDefinition: true CHECK: [[F1_LINE]] = !DILocation({{.*}}, scope: [[F1]],
+// isImplicitCode: true) CHECK: [[F2:![0-9]*]] = distinct !DISubprogram(name:
+// "f2",{{.*}} isDefinition: true CHECK: [[F2_LINE]] = !DILocation({{.*}},
+// scope: [[F2]], isImplicitCode: true)
 
 void f1() {
 }
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -786,7 +786,7 @@
   // If we should perform a cleanup, force them now.  Note that
   // this ends the cleanup scope before rescoping any labels.
   if (PerformCleanup) {
-ApplyDebugLocation DL(CGF, Range.getEnd());
+ApplyDebugLocation DL(CGF, Range.getEnd(), true /* ImplicitCode */);
 ForceCleanup();
   }
 }
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -317,7 +317,7 @@
 if (OnlySimpleReturnStmts)
   DI->EmitLocation(Builder, LastStopPoint);
 else
-  DI->EmitLocation(Builder, EndLoc);
+  DI->EmitLocation(Builder, EndLoc, true /* ImplicitCode */);
   }
 
   // Pop any cleanups that might have be

[clang-tools-extra] r341938 - Remove unnecessary semicolon to silence -Wpedantic warning. NFCI.

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Tue Sep 11 06:42:15 2018
New Revision: 341938

URL: http://llvm.org/viewvc/llvm-project?rev=341938&view=rev
Log:
Remove unnecessary semicolon to silence -Wpedantic warning. NFCI.

Modified:
clang-tools-extra/trunk/clangd/RIFF.cpp

Modified: clang-tools-extra/trunk/clangd/RIFF.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/RIFF.cpp?rev=341938&r1=341937&r2=341938&view=diff
==
--- clang-tools-extra/trunk/clangd/RIFF.cpp (original)
+++ clang-tools-extra/trunk/clangd/RIFF.cpp Tue Sep 11 06:42:15 2018
@@ -37,7 +37,7 @@ Expected readChunk(StringRef &Str
 Stream = Stream.drop_front();
   }
   return std::move(C);
-};
+}
 
 raw_ostream &operator<<(raw_ostream &OS, const Chunk &C) {
   OS.write(C.ID.data(), C.ID.size());


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

[PATCH] D51747: [clangd] Implement deprecation diagnostics with lower severity.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In https://reviews.llvm.org/D51747#1229066, @sammccall wrote:

> A few thoughts here:
>
> - does CDB describe user or project preferences? unclear.


Agree, it's a mix, defaults are from the project but users can add extra flags.

> - "show this warning for code I build" is a higher bar than "show this 
> warning for code I edit". So CDB probably enables too few warnings.
> - Some warnings play well with -Werror (like uninit warnings), some don't 
> (like deprecated). -Werror projects often disable interesting warnings.

Agreed, editors are different from build.

> I'm not sure we should strictly follow the CDB, but the bar to override it 
> should probably be high.

WDYT in the long term about a more general mechanism (to allow users override 
compiler or warning flags at the clangd level?
So that even if clangd is opinionated about the default warnings it enables, 
users have an option to override according to their preferences.

In https://reviews.llvm.org/D51747#1230420, @kadircet wrote:

> if user wants to see all diagnostics as a list suddenly they will get 
> deprecations in that list as well :(.


Yeah, some level of noise is probably inevitable.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51747



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

[PATCH] D51880: [ASTMatchers] add two matchers for dependent expressions

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 164863.
JonasToth added a comment.

- add isInstantiationDependent matcher as well
- add unit tests for new matchers


Repository:
  rC Clang

https://reviews.llvm.org/D51880

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1768,6 +1768,54 @@
 Matcher));
 }
 
+TEST(IsInstantiationDependent, MatchesNonValueTypeDependent) {
+  EXPECT_TRUE(
+  matches("template void f() { sizeof(sizeof(T() + T())); }",
+  expr(isInstantiationDependent(;
+}
+
+TEST(IsInstantiationDependent, MatchesValueDependent) {
+  EXPECT_TRUE(matches("template int f() { return T; }",
+  expr(isInstantiationDependent(;
+}
+
+TEST(IsInstantiationDependent, MatchesTypeDependent) {
+  EXPECT_TRUE(matches("template typename T f() { return T(); }",
+  expr(isInstantiationDependent(;
+}
+
+TEST(IsTypeDependent, MatchesTypeDependent) {
+  EXPECT_TRUE(matches("template typename T f() { return T(); }",
+  expr(isTypeDependent(;
+}
+
+TEST(IsTypeDependent, NotMatchesValueDependent) {
+  EXPECT_TRUE(notMatches("template int f() { return T; }",
+ expr(isTypeDependent(;
+}
+
+TEST(IsTypeDependent, NotMatchesInstantiationDependent) {
+  EXPECT_TRUE(
+  notMatches("template void f() { sizeof(sizeof(T() + T())); }",
+ expr(isTypeDependent(;
+}
+
+TEST(IsValueDependent, MatchesValueDependent) {
+  EXPECT_TRUE(matches("template int f() { return T; }",
+  expr(isValueDependent(;
+}
+
+TEST(IsValueDependent, NotMatchesTypeDependent) {
+  EXPECT_TRUE(notMatches("template typename T f() { return T(); }",
+ expr(isValueDependent(;
+}
+
+TEST(IsValueDependent, NotMatchesInstantiationDependent) {
+  EXPECT_TRUE(
+  notMatches("template void f() { sizeof(sizeof(T() + T())); }",
+ expr(isValueDependent(;
+}
+
 TEST(IsExplicitTemplateSpecialization,
  DoesNotMatchPrimaryTemplate) {
   EXPECT_TRUE(notMatches(
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -357,6 +357,7 @@
   REGISTER_MATCHER(isExpansionInSystemHeader);
   REGISTER_MATCHER(isInteger);
   REGISTER_MATCHER(isIntegral);
+  REGISTER_MATCHER(isInstantiationDependent);
   REGISTER_MATCHER(isInTemplateInstantiation);
   REGISTER_MATCHER(isLambda);
   REGISTER_MATCHER(isListInitialization);
@@ -376,8 +377,10 @@
   REGISTER_MATCHER(isStaticStorageClass);
   REGISTER_MATCHER(isStruct);
   REGISTER_MATCHER(isTemplateInstantiation);
+  REGISTER_MATCHER(isTypeDependent);
   REGISTER_MATCHER(isUnion);
   REGISTER_MATCHER(isUnsignedInteger);
+  REGISTER_MATCHER(isValueDependent);
   REGISTER_MATCHER(isVariadic);
   REGISTER_MATCHER(isVirtual);
   REGISTER_MATCHER(isVirtualAsWritten);
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -816,6 +816,45 @@
   return InnerMatcher.matches(Node.IgnoreParens(), Finder, Builder);
 }
 
+/// Matches expressions that are instantiation-dependent even if it is
+/// neither type- nor value-dependent.
+///
+/// In the following example, the expression sizeof(sizeof(T() + T()))
+/// is instantiation-dependent (since it involves a template parameter T),
+/// but is neither type- nor value-dependent, since the type of the inner
+/// sizeof is known (std::size_t) and therefore the size of the outer
+/// sizeof is known.
+/// \code
+///   template
+///   void f(T x, T y) { sizeof(sizeof(T() + T()); }
+/// \endcode
+AST_MATCHER(Expr, isInstantiationDependent) {
+  return Node.isInstantiationDependent();
+}
+
+/// Matches expressions that are type-dependant because the template type
+/// is not yet instantiated.
+///
+/// For example, the expressions "x" and "x + y" are type-dependent in
+/// the following code, but "y" is not type-dependent:
+/// \code
+///   template
+///   void add(T x, int y) {
+/// x + y;
+///   }
+/// \endcode
+AST_MATCHER(Expr, isTypeDependent) { return Node.isTypeDependent(); }
+
+/// Matches expression that are value-dependant because they contain a
+/// non-type template parameter.
+///
+/// For example, the array bound of "Chars" in the following example is
+/// value-dependent.
+/// \code
+///   template struct meta_string;
+/// \endcode
+AST_MATCHER(Expr, isValueDependent) { return Node.isValueDependent(); }
+
 /// Matches classTempl

[PATCH] D51917: [CodeCompletion] Enable signature help when initializing class/struct/union members.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 164864.
kadircet marked 3 inline comments as done.
kadircet added a comment.

- Resolve discussions.


Repository:
  rC Clang

https://reviews.llvm.org/D51917

Files:
  include/clang/Sema/Sema.h
  lib/Parse/ParseDeclCXX.cpp
  lib/Sema/SemaCodeComplete.cpp
  lib/Sema/SemaDeclCXX.cpp
  test/CodeCompletion/ctor-initializer.cpp

Index: test/CodeCompletion/ctor-initializer.cpp
===
--- test/CodeCompletion/ctor-initializer.cpp
+++ test/CodeCompletion/ctor-initializer.cpp
@@ -64,3 +64,26 @@
   // CHECK-CC8: COMPLETION: Pattern : member2(<#args#>
   int member1, member2;
 };
+
+struct Base2 {
+  Base2(int);
+};
+
+struct Composition1 {
+  Composition1() : b2_elem() {}
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:73:28 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:73:28 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // CHECK-CC9: OVERLOAD: Base2(<#int#>)
+  // CHECK-CC9: OVERLOAD: Base2(<#const Base2 &#>)
+  Composition1(Base2);
+  Base2 b2_elem;
+};
+
+struct Composition2 {
+  Composition2() : c1_elem(Base2(1)) {}
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:83:34 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:83:34 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:83:35 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:83:35 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  Composition1 c1_elem;
+};
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -3777,6 +3777,22 @@
 
 }
 
+ValueDecl *Sema::tryLookupCtorInitMemberDecl(CXXRecordDecl *ClassDecl,
+ CXXScopeSpec &SS,
+ ParsedType TemplateTypeTy,
+ IdentifierInfo *MemberOrBase) {
+  if (SS.getScopeRep() || TemplateTypeTy)
+return nullptr;
+  DeclContext::lookup_result Result = ClassDecl->lookup(MemberOrBase);
+  if (Result.empty())
+return nullptr;
+  ValueDecl *Member;
+  if ((Member = dyn_cast(Result.front())) ||
+  (Member = dyn_cast(Result.front(
+return Member;
+  return nullptr;
+}
+
 /// Handle a C++ member initializer.
 MemInitResult
 Sema::BuildMemInitializer(Decl *ConstructorD,
@@ -3820,21 +3836,16 @@
   //   of a single identifier refers to the class member. A
   //   mem-initializer-id for the hidden base class may be specified
   //   using a qualified name. ]
-  if (!SS.getScopeRep() && !TemplateTypeTy) {
-// Look for a member, first.
-DeclContext::lookup_result Result = ClassDecl->lookup(MemberOrBase);
-if (!Result.empty()) {
-  ValueDecl *Member;
-  if ((Member = dyn_cast(Result.front())) ||
-  (Member = dyn_cast(Result.front( {
-if (EllipsisLoc.isValid())
-  Diag(EllipsisLoc, diag::err_pack_expansion_member_init)
-<< MemberOrBase
-<< SourceRange(IdLoc, Init->getSourceRange().getEnd());
-
-return BuildMemberInitializer(Member, Init, IdLoc);
-  }
-}
+
+  // Look for a member, first.
+  if (ValueDecl *Member = tryLookupCtorInitMemberDecl(
+  ClassDecl, SS, TemplateTypeTy, MemberOrBase)) {
+if (EllipsisLoc.isValid())
+  Diag(EllipsisLoc, diag::err_pack_expansion_member_init)
+  << MemberOrBase
+  << SourceRange(IdLoc, Init->getSourceRange().getEnd());
+
+return BuildMemberInitializer(Member, Init, IdLoc);
   }
   // It didn't name a member, so see if it names a class.
   QualType BaseType;
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -4586,6 +4586,25 @@
   return ProduceSignatureHelp(*this, S, Results, Args.size(), OpenParLoc);
 }
 
+QualType Sema::ProduceCtorInitMemberSignatureHelp(
+Scope *S, Decl *ConstructorDecl, CXXScopeSpec SS, ParsedType TemplateTypeTy,
+ArrayRef ArgExprs, IdentifierInfo *II, SourceLocation OpenParLoc) {
+  if (!CodeCompleter)
+return QualType();
+
+  CXXConstructorDecl *Constructor =
+  dyn_cast(ConstructorDecl);
+  if (!Constructor)
+return QualType();
+  // FIXME: Add support for Base class constructors as well.
+  if (ValueDecl *MemberDecl = tryLookupCtorInitMemberDecl(
+  Constructor->getParent(), SS, TemplateTypeTy, II))
+return ProduceConstructorSignatureHelp(getCurScope(), MemberDecl->getType(),
+   MemberDecl->getLocation(), ArgExprs,
+   OpenParLoc);
+  return QualType();
+}
+
 vo

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clangd/tool/ClangdMain.cpp:197
 
+static llvm::cl::opt IncludeFixIts(
+"include-fixits",

ilya-biryukov wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > I wonder if we should make the `IncludeFixIts` option hidden?
> > > It currently only works for our YCM integration, VSCode and other clients 
> > > break.
> > why would a user want to turn this on or off?
> Ideally, we want to have it always on.
> However, all editors interpret the results we return in different ways. This 
> is a temporary option until we can define how text edits are handled by LSP.
> We filed the bugs, will dig them up on Monday.
+1 to @ilya-biryukov . We might need to wait for a while until all editors 
supports additionalTextEdits in a sane way. 



Comment at: clangd/tool/ClangdMain.cpp:202
+"Like changing an arrow(->) member access to dot(.) member access."),
+llvm::cl::init(clangd::CodeCompleteOptions().IncludeFixIts));
+

ilya-biryukov wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > Maybe specify the value here explicitly?
> > > The defaults for users of the clangd binary (what the option is for) is 
> > > not necessarily the best default for the `ClangdServer` clients (what the 
> > > default in the struct is for).
> > > More importantly, it's useful to be more straightforward about the 
> > > defaults we have for the options.
> > Not sure I agree here, this is consistent with the other options above. 
> > It's the other `ClangdServer` clients that are the weirdos, they should 
> > have to be explicit :-)
> > 
> > The defaults are clear when running with `-help`, which is how users will 
> > see them.
> > The defaults are clear when running with -help, which is how users will see 
> > them.
> Sure, but I would still argue reading the code is simpler without extra 
> indirection and defaults of the binary are not necessarily the defaults we 
> use in the APIs.
> 
> But also feel free to keep the code as is, don't think it's terribly 
> important.
Keeping it as it is.



Comment at: clangd/tool/ClangdMain.cpp:205
+static llvm::cl::opt EnableFunctionArgSnippets(
+"enable-function-arg-snippets",
+llvm::cl::desc("Gives snippets for function arguments, when disabled only "

ilya-biryukov wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > I wonder if we can infer this setting from the `-completion-style' 
> > > (`=bundled` implies `enable-function-arg-snippets == false`)
> > > @sammccall, WDYT?
> > They're not inextricably linked, the main connection is "these options both 
> > need good signaturehelp support to be really useful".
> > But we might want to link them just to cut down on number of options.
> > 
> > I don't have a strong opinion, I don't use `-completion-style=bundled`. Can 
> > we find a few people who do and ask if they like this setting?
> I would (obviously) want these two options to be packaged together whenever I 
> use `=bundled`.
> But I also use VSCode, which has signatureHelp.
> 
> I think @ioeric wanted to try out the `=bundled` completion style. @ioeric, 
> WDYT?
ping on this discussion to @ioeric , but I think linking seems like a good 
idea. Because it wouldn't be very useful if one already selects a specific 
overload from completion list.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51214



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

[PATCH] D51628: [clangd] Implement a Proof-of-Concept tool for symbol index exploration

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 164865.
kbobyrev marked 19 inline comments as done.
kbobyrev added a comment.

Address comments.


https://reviews.llvm.org/D51628

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/dexp/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp

Index: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
@@ -0,0 +1,167 @@
+//===--- Dexp.cpp - Dex EXPloration tool *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file implements a simple interactive tool which can be used to manually
+// evaluate symbol search quality of Clangd index.
+//
+//===--===//
+
+#include "../../../index/SymbolYAML.h"
+#include "../Dex.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSwitch.h"
+#include "llvm/LineEditor/LineEditor.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Signals.h"
+
+using clang::clangd::FuzzyFindRequest;
+using clang::clangd::loadIndex;
+using clang::clangd::Symbol;
+using clang::clangd::SymbolIndex;
+using llvm::StringRef;
+
+namespace {
+
+llvm::cl::opt
+SymbolCollection("symbol-collection-file",
+ llvm::cl::desc("Path to the file with symbol collection"),
+ llvm::cl::Positional, llvm::cl::Required);
+
+static const std::string Overview = R"(
+This is an **experimental** interactive tool to process user-provided search
+queries over given symbol collection obtained via global-symbol-builder. The
+tool can be used to evaluate search quality of existing index implementations
+and manually construct non-trivial test cases.
+
+Type use "help" request to get information about the details.
+)";
+
+template  void reportTime(StringRef Name, Function F) {
+  const auto TimerStart = std::chrono::high_resolution_clock::now();
+  F();
+  const auto TimerStop = std::chrono::high_resolution_clock::now();
+  const auto Duration = std::chrono::duration_cast(
+  TimerStop - TimerStart);
+  llvm::outs() << llvm::formatv("{0} took {1:ms+n}.\n", Name, Duration);
+}
+
+// llvm::formatv string pattern for pretty-printing symbols.
+static const auto OutputFormat = "{0,-4} | {1,-40} | {2,-25}\n";
+
+void fuzzyFind(llvm::StringRef UnqualifiedName, const SymbolIndex &Index) {
+  FuzzyFindRequest Request;
+  Request.MaxCandidateCount = 10;
+  Request.Query = UnqualifiedName;
+  std::vector Symbols;
+  llvm::outs() << '\n';
+  // FIXME(kbobyrev): Print symbol final scores to see the distribution.
+  llvm::outs() << llvm::formatv(OutputFormat, "Rank", "Symbol ID",
+"Symbol Name");
+  // FIXME(kbobyrev): Print symbol final scores to see the distribution.
+  size_t Rank = 0;
+  Index.fuzzyFind(Request, [&](const Symbol &Sym) {
+llvm::outs() << llvm::formatv(OutputFormat, Rank++, Sym.ID.str(), Sym.Name);
+  });
+  llvm::outs() << '\n';
+}
+
+static const std::string HelpMessage = R"(dexp commands:
+
+> find Name
+
+Constructs fuzzy find request given unqualified symbol name and returns top 10
+symbols retrieved from index.
+
+> lookup SymbolID
+
+Retrieves symbol names given USR.
+)";
+
+void help() { llvm::outs() << HelpMessage; }
+
+void lookup(StringRef USR, const SymbolIndex &Index) {
+  llvm::DenseSet IDs{clang::clangd::SymbolID{USR}};
+  clang::clangd::LookupRequest Request{IDs};
+  llvm::outs() << '\n';
+  llvm::outs() << llvm::formatv(OutputFormat, "Rank", "Symbol ID",
+"Symbol Name");
+  size_t Rank = 0;
+  Index.lookup(Request, [&](const Symbol &Sym) {
+llvm::outs() << llvm::formatv(OutputFormat, Rank++, Sym.ID.str(), Sym.Name);
+  });
+  llvm::outs() << '\n';
+}
+
+// FIXME(kbobyrev): Make this an actual REPL: probably use LLVM Command Line
+// library for parsing flags and arguments.
+// FIXME(kbobyrev): Ideas for commands:
+// * symbol lookup: print out symbol in YAML format given SymbolID
+// * find symbol references: print set of reference locations
+// * load/swap/reload index: this would make it possible to get rid of llvm::cl
+//   usages in the tool driver and actually use llvm::cl library in the REPL.
+// * show posting list density histogram (our dump data somewhere so that user
+//   could build one)
+// * show number of tokens of each kind
+// * print out tokens with the most dense posting lists
+// * print out tokens with least dense posting lists
+void dispatch(StringRef Request, const SymbolIndex &Index) {
+  llvm::SmallVector Arguments;
+  Request.split(Arguments, ' ');
+  if (Argument

r341939 - [OPENMP] Simplified checks for declarations in declare target regions.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Tue Sep 11 06:59:10 2018
New Revision: 341939

URL: http://llvm.org/viewvc/llvm-project?rev=341939&view=rev
Log:
[OPENMP] Simplified checks for declarations in declare target regions.

Sema analysis should not mark functions as an implicit declare target,
it may break codegen. Simplified semantic analysis and removed extra
code for implicit declare target functions.

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/declare_target_messages.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=341939&r1=341938&r2=341939&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Tue Sep 11 06:59:10 2018
@@ -12998,16 +12998,20 @@ void Sema::ActOnOpenMPDeclareTargetName(
   }
 
   NamedDecl *ND = Lookup.getAsSingle();
-  if (isa(ND) || isa(ND)) {
+  if (isa(ND) || isa(ND) ||
+  isa(ND)) {
 if (!SameDirectiveDecls.insert(cast(ND->getCanonicalDecl(
   Diag(Id.getLoc(), diag::err_omp_declare_target_multiple) << Id.getName();
-if (!ND->hasAttr()) {
+llvm::Optional Res =
+OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(
+cast(ND));
+if (!Res) {
   auto *A = OMPDeclareTargetDeclAttr::CreateImplicit(Context, MT);
   ND->addAttr(A);
   if (ASTMutationListener *ML = Context.getASTMutationListener())
 ML->DeclarationMarkedOpenMPDeclareTarget(ND, A);
   checkDeclIsAllowedInOpenMPTarget(nullptr, ND, Id.getLoc());
-} else if (ND->getAttr()->getMapType() != MT) {
+} else if (*Res != MT) {
   Diag(Id.getLoc(), diag::err_omp_declare_target_to_and_link)
   << Id.getName();
 }
@@ -13018,79 +13022,13 @@ void Sema::ActOnOpenMPDeclareTargetName(
 
 static void checkDeclInTargetContext(SourceLocation SL, SourceRange SR,
  Sema &SemaRef, Decl *D) {
-  if (!D)
+  if (!D || !isa(D))
 return;
-  const Decl *LD = nullptr;
-  if (isa(D)) {
-LD = cast(D)->getDefinition();
-  } else if (isa(D)) {
-LD = cast(D)->getDefinition();
-
-// If this is an implicit variable that is legal and we do not need to do
-// anything.
-if (cast(D)->isImplicit()) {
-  auto *A = OMPDeclareTargetDeclAttr::CreateImplicit(
-  SemaRef.Context, OMPDeclareTargetDeclAttr::MT_To);
-  D->addAttr(A);
-  if (ASTMutationListener *ML = SemaRef.Context.getASTMutationListener())
-ML->DeclarationMarkedOpenMPDeclareTarget(D, A);
-  return;
-}
-  } else if (const auto *F = dyn_cast(D)) {
-const FunctionDecl *FD = nullptr;
-if (cast(D)->hasBody(FD)) {
-  LD = FD;
-  // If the definition is associated with the current declaration in the
-  // target region (it can be e.g. a lambda) that is legal and we do not
-  // need to do anything else.
-  if (LD == D) {
-auto *A = OMPDeclareTargetDeclAttr::CreateImplicit(
-SemaRef.Context, OMPDeclareTargetDeclAttr::MT_To);
-D->addAttr(A);
-if (ASTMutationListener *ML = SemaRef.Context.getASTMutationListener())
-  ML->DeclarationMarkedOpenMPDeclareTarget(D, A);
-return;
-  }
-} else if (F->isFunctionTemplateSpecialization() &&
-   F->getTemplateSpecializationKind() ==
-   TSK_ImplicitInstantiation) {
-  // Check if the function is implicitly instantiated from the template
-  // defined in the declare target region.
-  const FunctionTemplateDecl *FTD = F->getPrimaryTemplate();
-  if (FTD && FTD->hasAttr())
-return;
-}
-  }
-  if (!LD)
-LD = D;
-  if (LD && !LD->hasAttr() &&
-  ((isa(LD) && !isa(LD)) || isa(LD))) {
-// Outlined declaration is not declared target.
-if (!isa(LD)) {
-  if (LD->isOutOfLine()) {
-SemaRef.Diag(LD->getLocation(), diag::warn_omp_not_in_target_context);
-SemaRef.Diag(SL, diag::note_used_here) << SR;
-  } else {
-const DeclContext *DC = LD->getDeclContext();
-while (DC &&
-   (!isa(DC) ||
-!cast(DC)->hasAttr()))
-  DC = DC->getParent();
-if (DC)
-  return;
-
-// Is not declared in target context.
-SemaRef.Diag(LD->getLocation(), diag::warn_omp_not_in_target_context);
-SemaRef.Diag(SL, diag::note_used_here) << SR;
-  }
-}
-// Mark decl as declared target to prevent further diagnostic.
-auto *A = OMPDeclareTargetDeclAttr::CreateImplicit(
-SemaRef.Context, OMPDeclareTargetDeclAttr::MT_To);
-D->addAttr(A);
-if (ASTMutationListener *ML = SemaRef.Context.getASTMutationListener())
-  ML->DeclarationMarkedOpenMPDeclareTarget(D, A);
-  }
+  auto *VD = cast(D);
+  if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(VD))
+return;
+  SemaRef.Diag(VD->getLocation(), diag::warn_omp_not_in_tar

[PATCH] D51917: [CodeCompletion] Enable signature help when initializing class/struct/union members.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Update tests.


Repository:
  rC Clang

https://reviews.llvm.org/D51917

Files:
  include/clang/Sema/Sema.h
  lib/Parse/ParseDeclCXX.cpp
  lib/Sema/SemaCodeComplete.cpp
  lib/Sema/SemaDeclCXX.cpp
  test/CodeCompletion/ctor-initializer.cpp

Index: test/CodeCompletion/ctor-initializer.cpp
===
--- test/CodeCompletion/ctor-initializer.cpp
+++ test/CodeCompletion/ctor-initializer.cpp
@@ -64,3 +64,27 @@
   // CHECK-CC8: COMPLETION: Pattern : member2(<#args#>
   int member1, member2;
 };
+
+struct Base2 {
+  Base2(int);
+};
+
+struct Composition1 {
+  Composition1() : b2_elem() {}
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:73:28 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:73:28 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // CHECK-CC9: OVERLOAD: Base2(<#int#>)
+  // CHECK-CC9: OVERLOAD: Base2(<#const Base2 &#>)
+  // CHECK-CC9-NOT: OVERLOAD: Composition1
+  Composition1(Base2);
+  Base2 b2_elem;
+};
+
+struct Composition2 {
+  Composition2() : c1_elem(Base2(1)) {}
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:84:34 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:84:34 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:84:35 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:84:35 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  Composition1 c1_elem;
+};
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -3777,6 +3777,22 @@
 
 }
 
+ValueDecl *Sema::tryLookupCtorInitMemberDecl(CXXRecordDecl *ClassDecl,
+ CXXScopeSpec &SS,
+ ParsedType TemplateTypeTy,
+ IdentifierInfo *MemberOrBase) {
+  if (SS.getScopeRep() || TemplateTypeTy)
+return nullptr;
+  DeclContext::lookup_result Result = ClassDecl->lookup(MemberOrBase);
+  if (Result.empty())
+return nullptr;
+  ValueDecl *Member;
+  if ((Member = dyn_cast(Result.front())) ||
+  (Member = dyn_cast(Result.front(
+return Member;
+  return nullptr;
+}
+
 /// Handle a C++ member initializer.
 MemInitResult
 Sema::BuildMemInitializer(Decl *ConstructorD,
@@ -3820,21 +3836,16 @@
   //   of a single identifier refers to the class member. A
   //   mem-initializer-id for the hidden base class may be specified
   //   using a qualified name. ]
-  if (!SS.getScopeRep() && !TemplateTypeTy) {
-// Look for a member, first.
-DeclContext::lookup_result Result = ClassDecl->lookup(MemberOrBase);
-if (!Result.empty()) {
-  ValueDecl *Member;
-  if ((Member = dyn_cast(Result.front())) ||
-  (Member = dyn_cast(Result.front( {
-if (EllipsisLoc.isValid())
-  Diag(EllipsisLoc, diag::err_pack_expansion_member_init)
-<< MemberOrBase
-<< SourceRange(IdLoc, Init->getSourceRange().getEnd());
-
-return BuildMemberInitializer(Member, Init, IdLoc);
-  }
-}
+
+  // Look for a member, first.
+  if (ValueDecl *Member = tryLookupCtorInitMemberDecl(
+  ClassDecl, SS, TemplateTypeTy, MemberOrBase)) {
+if (EllipsisLoc.isValid())
+  Diag(EllipsisLoc, diag::err_pack_expansion_member_init)
+  << MemberOrBase
+  << SourceRange(IdLoc, Init->getSourceRange().getEnd());
+
+return BuildMemberInitializer(Member, Init, IdLoc);
   }
   // It didn't name a member, so see if it names a class.
   QualType BaseType;
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -4586,6 +4586,25 @@
   return ProduceSignatureHelp(*this, S, Results, Args.size(), OpenParLoc);
 }
 
+QualType Sema::ProduceCtorInitMemberSignatureHelp(
+Scope *S, Decl *ConstructorDecl, CXXScopeSpec SS, ParsedType TemplateTypeTy,
+ArrayRef ArgExprs, IdentifierInfo *II, SourceLocation OpenParLoc) {
+  if (!CodeCompleter)
+return QualType();
+
+  CXXConstructorDecl *Constructor =
+  dyn_cast(ConstructorDecl);
+  if (!Constructor)
+return QualType();
+  // FIXME: Add support for Base class constructors as well.
+  if (ValueDecl *MemberDecl = tryLookupCtorInitMemberDecl(
+  Constructor->getParent(), SS, TemplateTypeTy, II))
+return ProduceConstructorSignatureHelp(getCurScope(), MemberDecl->getType(),
+   MemberDecl->getLocation(), ArgExprs,
+   OpenParLoc);
+  return QualType();
+}
+
 void Sem

[PATCH] D50171: [python] [tests] Update test_code_completion

[Michał Górny via Phabricator via cfe-commits]

mgorny updated this revision to Diff 164871.
mgorny added a comment.

Ai, sorry about that. Uploaded the proper diff now. I suppose it's not going to 
make it for 7.0.0 anymore, so it's not a priority. I'll try to bisect it today 
once I finish testing RC3.


https://reviews.llvm.org/D50171

Files:
  bindings/python/tests/cindex/test_code_completion.py


Index: bindings/python/tests/cindex/test_code_completion.py
===
--- bindings/python/tests/cindex/test_code_completion.py
+++ bindings/python/tests/cindex/test_code_completion.py
@@ -61,11 +61,11 @@
 cr = tu.codeComplete('fake.cpp', 12, 5, unsaved_files=files)
 
 expected = [
-  "{'const', TypedText} || Priority: 40 || Availability: Available || 
Brief comment: None",
-  "{'volatile', TypedText} || Priority: 40 || Availability: Available 
|| Brief comment: None",
+  "{'const', TypedText} || Priority: 50 || Availability: Available || 
Brief comment: None",
+  "{'volatile', TypedText} || Priority: 50 || Availability: Available 
|| Brief comment: None",
   "{'operator', TypedText} || Priority: 40 || Availability: Available 
|| Brief comment: None",
-  "{'P', TypedText} | {'::', Text} || Priority: 75 || Availability: 
Available || Brief comment: None",
-  "{'Q', TypedText} | {'::', Text} || Priority: 75 || Availability: 
Available || Brief comment: None"
+  "{'P', TypedText} || Priority: 50 || Availability: Available || 
Brief comment: None",
+  "{'Q', TypedText} || Priority: 50 || Availability: Available || 
Brief comment: None"
 ]
 self.check_completion_results(cr, expected)
 


Index: bindings/python/tests/cindex/test_code_completion.py
===
--- bindings/python/tests/cindex/test_code_completion.py
+++ bindings/python/tests/cindex/test_code_completion.py
@@ -61,11 +61,11 @@
 cr = tu.codeComplete('fake.cpp', 12, 5, unsaved_files=files)
 
 expected = [
-  "{'const', TypedText} || Priority: 40 || Availability: Available || Brief comment: None",
-  "{'volatile', TypedText} || Priority: 40 || Availability: Available || Brief comment: None",
+  "{'const', TypedText} || Priority: 50 || Availability: Available || Brief comment: None",
+  "{'volatile', TypedText} || Priority: 50 || Availability: Available || Brief comment: None",
   "{'operator', TypedText} || Priority: 40 || Availability: Available || Brief comment: None",
-  "{'P', TypedText} | {'::', Text} || Priority: 75 || Availability: Available || Brief comment: None",
-  "{'Q', TypedText} | {'::', Text} || Priority: 75 || Availability: Available || Brief comment: None"
+  "{'P', TypedText} || Priority: 50 || Availability: Available || Brief comment: None",
+  "{'Q', TypedText} || Priority: 50 || Availability: Available || Brief comment: None"
 ]
 self.check_completion_results(cr, expected)
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51880: [ASTMatchers] add two matchers for dependent expressions

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 164873.
JonasToth added a comment.

- fix typos, include example in doc and adjust tests


Repository:
  rC Clang

https://reviews.llvm.org/D51880

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1768,6 +1768,48 @@
 Matcher));
 }
 
+TEST(IsInstantiationDependent, MatchesNonValueTypeDependent) {
+  EXPECT_TRUE(matches(
+  "template void f() { (void) sizeof(sizeof(T() + T())); }",
+  expr(isInstantiationDependent(;
+}
+
+TEST(IsInstantiationDependent, MatchesValueDependent) {
+  EXPECT_TRUE(matches("template int f() { return T; }",
+  expr(isInstantiationDependent(;
+}
+
+TEST(IsInstantiationDependent, MatchesTypeDependent) {
+  EXPECT_TRUE(matches("template T f() { return T(); }",
+  expr(isInstantiationDependent(;
+}
+
+TEST(IsTypeDependent, MatchesTypeDependent) {
+  EXPECT_TRUE(matches("template T f() { return T(); }",
+  expr(isTypeDependent(;
+}
+
+TEST(IsTypeDependent, NotMatchesValueDependent) {
+  EXPECT_TRUE(notMatches("template int f() { return T; }",
+ expr(isTypeDependent(;
+}
+
+TEST(IsValueDependent, MatchesValueDependent) {
+  EXPECT_TRUE(matches("template int f() { return T; }",
+  expr(isValueDependent(;
+}
+
+TEST(IsValueDependent, MatchesTypeDependent) {
+  EXPECT_TRUE(matches("template T f() { return T(); }",
+  expr(isValueDependent(;
+}
+
+TEST(IsValueDependent, MatchesInstantiationDependent) {
+  EXPECT_TRUE(matches(
+  "template void f() { (void) sizeof(sizeof(T() + T())); }",
+  expr(isValueDependent(;
+}
+
 TEST(IsExplicitTemplateSpecialization,
  DoesNotMatchPrimaryTemplate) {
   EXPECT_TRUE(notMatches(
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -357,6 +357,7 @@
   REGISTER_MATCHER(isExpansionInSystemHeader);
   REGISTER_MATCHER(isInteger);
   REGISTER_MATCHER(isIntegral);
+  REGISTER_MATCHER(isInstantiationDependent);
   REGISTER_MATCHER(isInTemplateInstantiation);
   REGISTER_MATCHER(isLambda);
   REGISTER_MATCHER(isListInitialization);
@@ -376,8 +377,10 @@
   REGISTER_MATCHER(isStaticStorageClass);
   REGISTER_MATCHER(isStruct);
   REGISTER_MATCHER(isTemplateInstantiation);
+  REGISTER_MATCHER(isTypeDependent);
   REGISTER_MATCHER(isUnion);
   REGISTER_MATCHER(isUnsignedInteger);
+  REGISTER_MATCHER(isValueDependent);
   REGISTER_MATCHER(isVariadic);
   REGISTER_MATCHER(isVirtual);
   REGISTER_MATCHER(isVirtualAsWritten);
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -816,6 +816,48 @@
   return InnerMatcher.matches(Node.IgnoreParens(), Finder, Builder);
 }
 
+/// Matches expressions that are instantiation-dependent even if it is
+/// neither type- nor value-dependent.
+///
+/// In the following example, the expression sizeof(sizeof(T() + T()))
+/// is instantiation-dependent (since it involves a template parameter T),
+/// but is neither type- nor value-dependent, since the type of the inner
+/// sizeof is known (std::size_t) and therefore the size of the outer
+/// sizeof is known.
+/// \code
+///   template
+///   void f(T x, T y) { sizeof(sizeof(T() + T()); }
+/// \endcode
+/// expr(isInstantiationDependent()) matches sizeof(sizeof(T() + T())
+AST_MATCHER(Expr, isInstantiationDependent) {
+  return Node.isInstantiationDependent();
+}
+
+/// Matches expressions that are type-dependent because the template type
+/// is not yet instantiated.
+///
+/// For example, the expressions "x" and "x + y" are type-dependent in
+/// the following code, but "y" is not type-dependent:
+/// \code
+///   template
+///   void add(T x, int y) {
+/// x + y;
+///   }
+/// \endcode
+/// expr(isTypeDependent()) matches x + y
+AST_MATCHER(Expr, isTypeDependent) { return Node.isTypeDependent(); }
+
+/// Matches expression that are value-dependent because they contain a
+/// non-type template parameter.
+///
+/// For example, the array bound of "Chars" in the following example is
+/// value-dependent.
+/// \code
+///   template int f() { return Size; }
+/// \endcode
+/// expr(isValueDependent()) matches return Size
+AST_MATCHER(Expr, isValueDependent) { return Node.isValueDependent(); }
+
 /// Matches classTemplateSpecializations, templateSpecializationType and
 /// functionDecl where the n'th TemplateArgument 

[PATCH] D51880: [ASTMatchers] add two matchers for dependent expressions

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 164876.
JonasToth added a comment.

rebase to master


Repository:
  rC Clang

https://reviews.llvm.org/D51880

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1768,6 +1768,48 @@
 Matcher));
 }
 
+TEST(IsInstantiationDependent, MatchesNonValueTypeDependent) {
+  EXPECT_TRUE(matches(
+  "template void f() { (void) sizeof(sizeof(T() + T())); }",
+  expr(isInstantiationDependent(;
+}
+
+TEST(IsInstantiationDependent, MatchesValueDependent) {
+  EXPECT_TRUE(matches("template int f() { return T; }",
+  expr(isInstantiationDependent(;
+}
+
+TEST(IsInstantiationDependent, MatchesTypeDependent) {
+  EXPECT_TRUE(matches("template T f() { return T(); }",
+  expr(isInstantiationDependent(;
+}
+
+TEST(IsTypeDependent, MatchesTypeDependent) {
+  EXPECT_TRUE(matches("template T f() { return T(); }",
+  expr(isTypeDependent(;
+}
+
+TEST(IsTypeDependent, NotMatchesValueDependent) {
+  EXPECT_TRUE(notMatches("template int f() { return T; }",
+ expr(isTypeDependent(;
+}
+
+TEST(IsValueDependent, MatchesValueDependent) {
+  EXPECT_TRUE(matches("template int f() { return T; }",
+  expr(isValueDependent(;
+}
+
+TEST(IsValueDependent, MatchesTypeDependent) {
+  EXPECT_TRUE(matches("template T f() { return T(); }",
+  expr(isValueDependent(;
+}
+
+TEST(IsValueDependent, MatchesInstantiationDependent) {
+  EXPECT_TRUE(matches(
+  "template void f() { (void) sizeof(sizeof(T() + T())); }",
+  expr(isValueDependent(;
+}
+
 TEST(IsExplicitTemplateSpecialization,
  DoesNotMatchPrimaryTemplate) {
   EXPECT_TRUE(notMatches(
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -357,6 +357,7 @@
   REGISTER_MATCHER(isExpansionInSystemHeader);
   REGISTER_MATCHER(isInteger);
   REGISTER_MATCHER(isIntegral);
+  REGISTER_MATCHER(isInstantiationDependent);
   REGISTER_MATCHER(isInTemplateInstantiation);
   REGISTER_MATCHER(isLambda);
   REGISTER_MATCHER(isListInitialization);
@@ -376,8 +377,10 @@
   REGISTER_MATCHER(isStaticStorageClass);
   REGISTER_MATCHER(isStruct);
   REGISTER_MATCHER(isTemplateInstantiation);
+  REGISTER_MATCHER(isTypeDependent);
   REGISTER_MATCHER(isUnion);
   REGISTER_MATCHER(isUnsignedInteger);
+  REGISTER_MATCHER(isValueDependent);
   REGISTER_MATCHER(isVariadic);
   REGISTER_MATCHER(isVirtual);
   REGISTER_MATCHER(isVirtualAsWritten);
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -816,6 +816,48 @@
   return InnerMatcher.matches(Node.IgnoreParens(), Finder, Builder);
 }
 
+/// Matches expressions that are instantiation-dependent even if it is
+/// neither type- nor value-dependent.
+///
+/// In the following example, the expression sizeof(sizeof(T() + T()))
+/// is instantiation-dependent (since it involves a template parameter T),
+/// but is neither type- nor value-dependent, since the type of the inner
+/// sizeof is known (std::size_t) and therefore the size of the outer
+/// sizeof is known.
+/// \code
+///   template
+///   void f(T x, T y) { sizeof(sizeof(T() + T()); }
+/// \endcode
+/// expr(isInstantiationDependent()) matches sizeof(sizeof(T() + T())
+AST_MATCHER(Expr, isInstantiationDependent) {
+  return Node.isInstantiationDependent();
+}
+
+/// Matches expressions that are type-dependent because the template type
+/// is not yet instantiated.
+///
+/// For example, the expressions "x" and "x + y" are type-dependent in
+/// the following code, but "y" is not type-dependent:
+/// \code
+///   template
+///   void add(T x, int y) {
+/// x + y;
+///   }
+/// \endcode
+/// expr(isTypeDependent()) matches x + y
+AST_MATCHER(Expr, isTypeDependent) { return Node.isTypeDependent(); }
+
+/// Matches expression that are value-dependent because they contain a
+/// non-type template parameter.
+///
+/// For example, the array bound of "Chars" in the following example is
+/// value-dependent.
+/// \code
+///   template int f() { return Size; }
+/// \endcode
+/// expr(isValueDependent()) matches return Size
+AST_MATCHER(Expr, isValueDependent) { return Node.isValueDependent(); }
+
 /// Matches classTemplateSpecializations, templateSpecializationType and
 /// functionDecl where the n'th TemplateArgument matches the given InnerMatcher.
 ///

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/tool/ClangdMain.cpp:205
+static llvm::cl::opt EnableFunctionArgSnippets(
+"enable-function-arg-snippets",
+llvm::cl::desc("Gives snippets for function arguments, when disabled only "

sammccall wrote:
> kadircet wrote:
> > ilya-biryukov wrote:
> > > sammccall wrote:
> > > > ilya-biryukov wrote:
> > > > > I wonder if we can infer this setting from the `-completion-style' 
> > > > > (`=bundled` implies `enable-function-arg-snippets == false`)
> > > > > @sammccall, WDYT?
> > > > They're not inextricably linked, the main connection is "these options 
> > > > both need good signaturehelp support to be really useful".
> > > > But we might want to link them just to cut down on number of options.
> > > > 
> > > > I don't have a strong opinion, I don't use `-completion-style=bundled`. 
> > > > Can we find a few people who do and ask if they like this setting?
> > > I would (obviously) want these two options to be packaged together 
> > > whenever I use `=bundled`.
> > > But I also use VSCode, which has signatureHelp.
> > > 
> > > I think @ioeric wanted to try out the `=bundled` completion style. 
> > > @ioeric, WDYT?
> > ping on this discussion to @ioeric , but I think linking seems like a good 
> > idea. Because it wouldn't be very useful if one already selects a specific 
> > overload from completion list.
> This seems reasonable to have as a preference, I'm also fine combining it 
> with bundled.
> 
> If you keep it, naming nits:
>  - drop "enable" prefix (our other bool flags don't have this)
>  - snippets -> placeholders (snippets is both jargon and technically 
> incorrect - we still emit snippets like `foo({$0})`.
I don't think YCM completes the snippets currently, so Eric won't notice the 
change for bundled vs normal completions.

Anyhow, keeping it as a separate option is a safe bet. 

And some personal experience: have been using bundled completions for a few 
weeks now and will probably switch this one on immediately when it lands.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51214



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

[PATCH] D51628: [clangd] Implement a Proof-of-Concept tool for symbol index exploration

[Sam McCall via Phabricator via cfe-commits]

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

Really just details now.




Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:56
+// llvm::formatv string pattern for pretty-printing symbols.
+static const auto OutputFormat = "{0,-4} | {1,-40} | {2,-25}\n";
+

(I don't think you want to share this between fuzzyfind and lookup, see below)



Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:62
+  Request.Query = UnqualifiedName;
+  std::vector Symbols;
+  llvm::outs() << '\n';

unused



Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:63
+  std::vector Symbols;
+  llvm::outs() << '\n';
+  // FIXME(kbobyrev): Print symbol final scores to see the distribution.

why? (and below)

If you want all commands to be separated by a  blank line, the dispatcher can 
do it



Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:64
+  llvm::outs() << '\n';
+  // FIXME(kbobyrev): Print symbol final scores to see the distribution.
+  llvm::outs() << llvm::formatv(OutputFormat, "Rank", "Symbol ID",

you have this comment twice



Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:90
+void lookup(StringRef USR, const SymbolIndex &Index) {
+  llvm::DenseSet IDs{clang::clangd::SymbolID{USR}};
+  clang::clangd::LookupRequest Request{IDs};

note that you're looking up one ID so you'll always get 0 or 1 result



Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:95
+"Symbol Name");
+  size_t Rank = 0;
+  Index.lookup(Request, [&](const Symbol &Sym) {

Rank is never meaningful for lookup, especially when only looking up one id



Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:97
+  Index.lookup(Request, [&](const Symbol &Sym) {
+llvm::outs() << llvm::formatv(OutputFormat, Rank++, Sym.ID.str(), 
Sym.Name);
+  });

this isn't enough detail to be really useful. I'd suggest dumping the whole 
symbol as YAML, or printing 'not found' if no result.



Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:125
+// * print out tokens with least dense posting lists
+void dispatchRequest(const std::unique_ptr &Index,
+ StringRef Request) {

sammccall wrote:
> ilya-biryukov wrote:
> > This function does too many things:
> > - parses the input.
> > - dispatches on different kinds of requests.
> > - handles user input errors.
> > - actually runs the commands.
> > 
> > This turns out somewhat unreadable at the end, we might want to separate 
> > those into multiple functions. Will need a bit of time to think on how to 
> > actually do it best. Will come back after I have concrete suggestions.
> For now can we keep it simple:
>  - this function can split the command from the rest, and pass the rest of 
> the args to `fuzzyFindRequest` etc
>  - if the command is unknown, the dispatcher reports the error
>  - if the command is known, the command handler reports input errors (this 
> could be cleaner/more declarative, but we don't want to build a big framework 
> in this patch)
>  - this function could do the timing - the request construction is negligible 
> and that keeps the command handlers focused on their job
this is not done (dispatchRequest is still doing validation for each command)



Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:130
+  if (Arguments.empty()) {
+llvm::outs() << "ERROR: Request can not be empty.\n";
+helpRequest();

sammccall wrote:
> REPLs don't usually make this an error, just return
this is not done



Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:180
+
+  llvm::LineEditor LE("dexplorer");
+

dexp?


https://reviews.llvm.org/D51628



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

[PATCH] D51917: [CodeCompletion] Enable signature help when initializing class/struct/union members.

[Ilya Biryukov via Phabricator via cfe-commits]

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

LGTM. Thanks for the fix!


Repository:
  rC Clang

https://reviews.llvm.org/D51917



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

[PATCH] D51628: [clangd] Implement a Proof-of-Concept tool for symbol index exploration

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/dexp/CMakeLists.txt:10
+  Dexp.cpp
+  )
+

Should we indent closing parens to the opening one or keep at the start of the 
line?
Let's pick one style and be consistent (the best option is being consistent 
with the rest of LLVM/clangd)



Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:46
+
+template  void reportTime(StringRef Name, Function F) {
+  const auto TimerStart = std::chrono::high_resolution_clock::now();

NIT: Now that we ignore the return value, we could even remove templates:
```
void reportTime(StringRef Name, llvm::function_ref F);
```


https://reviews.llvm.org/D51628



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

[PATCH] D51090: [clangd] Add index benchmarks

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 164881.
kbobyrev marked 2 inline comments as done.
kbobyrev added a comment.

The only problem left is that I'm not sure how to run binary which is not under 
bin (`IndexBenchmark`) using llvm-lit.


https://reviews.llvm.org/D51090

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/benchmarks/CMakeLists.txt
  clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
  clang-tools-extra/test/clangd/Inputs/BenchmarkHeader.h
  clang-tools-extra/test/clangd/Inputs/BenchmarkSource.cpp
  clang-tools-extra/test/clangd/Inputs/requests.log
  clang-tools-extra/test/clangd/benchmark-dummy.test

Index: clang-tools-extra/test/clangd/benchmark-dummy.test
===
--- /dev/null
+++ clang-tools-extra/test/clangd/benchmark-dummy.test
@@ -0,0 +1,2 @@
+# RUN: global-symbol-builder %p/Inputs/BenchmarkSource.cpp -- -I%p/Inputs > %t.index
+# RUN: IndexBenchmark %t.index %p/Inputs/requests.log
Index: clang-tools-extra/test/clangd/Inputs/requests.log
===
--- /dev/null
+++ clang-tools-extra/test/clangd/Inputs/requests.log
@@ -0,0 +1,5 @@
+V[09:08:34.301] Code complete: fuzzyFind("s", scopes=[llvm::])
+V[09:08:34.368] Code complete: fuzzyFind("sy", scopes=[llvm::])
+V[09:08:34.442] Code complete: fuzzyFind("sys", scopes=[llvm::])
+V[09:09:12.075] Code complete: fuzzyFind("Dex", scopes=[clang::clangd::, clang::, clang::clangd::dex::])
+V[09:09:12.075] Code complete: fuzzyFind("Variable", scopes=[])
Index: clang-tools-extra/test/clangd/Inputs/BenchmarkSource.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clangd/Inputs/BenchmarkSource.cpp
@@ -0,0 +1 @@
+#include "BenchmarkHeader.h"
Index: clang-tools-extra/test/clangd/Inputs/BenchmarkHeader.h
===
--- /dev/null
+++ clang-tools-extra/test/clangd/Inputs/BenchmarkHeader.h
@@ -0,0 +1,19 @@
+namespace clang {
+namespace clangd {
+namespace dex {
+class Dex;
+} // namespace dex
+} // namespace clangd
+} // namespace clang
+
+namespace llvm {
+namespace sys {
+
+int getHostNumPhysicalCores();
+
+} // namespace sys
+} // namespace llvm
+
+namespace {
+int Variable;
+} // namespace
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
@@ -0,0 +1,113 @@
+//===--- IndexBenchmark.cpp - Clangd index benchmarks ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "../index/SymbolYAML.h"
+#include "../index/dex/Dex.h"
+#include "benchmark/benchmark.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Regex.h"
+#include 
+#include 
+#include 
+
+const char *IndexFilename;
+const char *LogFilename;
+
+namespace clang {
+namespace clangd {
+namespace {
+
+std::unique_ptr buildMem() {
+  return clang::clangd::loadIndex(IndexFilename, {}, false);
+}
+
+std::unique_ptr buildDex() {
+  return clang::clangd::loadIndex(IndexFilename, {}, true);
+}
+
+// This function processes user-provided Log file with fuzzy find requests in
+// the following format:
+//
+// fuzzyFind("UnqualifiedName", scopes=["clang::", "clang::clangd::"])
+//
+// It constructs vector of FuzzyFindRequests which is later used for the
+// benchmarks.
+std::vector extractQueriesFromLogs() {
+  llvm::Regex RequestMatcher("fuzzyFind\\(\"([a-zA-Z]*)\", scopes=\\[(.*)\\]");
+  llvm::SmallVector Matches;
+  std::ifstream InputStream(LogFilename);
+  std::string Log((std::istreambuf_iterator(InputStream)),
+  std::istreambuf_iterator());
+  llvm::StringRef Temporary(Log);
+  llvm::SmallVector Strings;
+  Temporary.split(Strings, '\n');
+
+  clang::clangd::FuzzyFindRequest R;
+  R.MaxCandidateCount = 100;
+
+  llvm::SmallVector CommaSeparatedValues;
+
+  std::vector RealRequests;
+  for (auto Line : Strings) {
+if (RequestMatcher.match(Line, &Matches)) {
+  R.Query = Matches[1];
+  CommaSeparatedValues.clear();
+  Line.split(CommaSeparatedValues, ',');
+  R.Scopes.clear();
+  for (auto C : CommaSeparatedValues) {
+R.Scopes.push_back(C);
+  }
+  RealRequests.push_back(R);
+}
+  }
+  return RealRequests;
+}
+
+static void MemQueries(benchmark::State &State) {
+  const auto Mem = buildMem();
+  const auto Requests = extractQueriesFromLogs();
+  for (auto _ : State)
+for (const auto &Request : Requests)
+  Mem->fuzzyFind(Request, [](const Symbol &S) {});
+}
+BENCHMARK(MemQueries);
+
+static void DexQueries(ben

r341949 - [CodeCompletion] Enable signature help when initializing class/struct/union members.

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Tue Sep 11 08:02:18 2018
New Revision: 341949

URL: http://llvm.org/viewvc/llvm-project?rev=341949&view=rev
Log:
[CodeCompletion] Enable signature help when initializing class/struct/union 
members.

Summary:
Factors out member decleration gathering and uses it in parsing to call 
signature
help. Doesn't support signature help for base class constructors, the code was 
too
coupled with diagnostic handling, but still can be factored out but just needs
more afford.

Reviewers: sammccall, ilya-biryukov, ioeric

Reviewed By: ilya-biryukov

Subscribers: cfe-commits

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

Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Parse/ParseDeclCXX.cpp
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/CodeCompletion/ctor-initializer.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=341949&r1=341948&r2=341949&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Tue Sep 11 08:02:18 2018
@@ -4568,6 +4568,11 @@ private:
   // of a ComparisonCategoryType enumerator.
   llvm::SmallBitVector FullyCheckedComparisonCategories;
 
+  ValueDecl *tryLookupCtorInitMemberDecl(CXXRecordDecl *ClassDecl,
+ CXXScopeSpec &SS,
+ ParsedType TemplateTypeTy,
+ IdentifierInfo *MemberOrBase);
+
 public:
   /// Lookup the specified comparison category types in the standard
   ///   library, an check the VarDecls possibly returned by the operator<=>
@@ -10254,6 +10259,12 @@ public:
SourceLocation Loc,
ArrayRef Args,
SourceLocation OpenParLoc);
+  QualType ProduceCtorInitMemberSignatureHelp(Scope *S, Decl *ConstructorDecl,
+  CXXScopeSpec SS,
+  ParsedType TemplateTypeTy,
+  ArrayRef ArgExprs,
+  IdentifierInfo *II,
+  SourceLocation OpenParLoc);
   void CodeCompleteInitializer(Scope *S, Decl *D);
   void CodeCompleteReturn(Scope *S);
   void CodeCompleteAfterIf(Scope *S);
@@ -10794,7 +10805,6 @@ struct LateParsedTemplate {
   /// The template function declaration to be late parsed.
   Decl *D;
 };
-
 } // end namespace clang
 
 namespace llvm {

Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=341949&r1=341948&r2=341949&view=diff
==
--- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Tue Sep 11 08:02:18 2018
@@ -3449,6 +3449,7 @@ MemInitResult Parser::ParseMemInitialize
   if (getLangOpts().CPlusPlus11 && Tok.is(tok::l_brace)) {
 Diag(Tok, diag::warn_cxx98_compat_generalized_initializer_lists);
 
+// FIXME: Add support for signature help inside initializer lists.
 ExprResult InitList = ParseBraceInitializer();
 if (InitList.isInvalid())
   return true;
@@ -3466,7 +3467,20 @@ MemInitResult Parser::ParseMemInitialize
 // Parse the optional expression-list.
 ExprVector ArgExprs;
 CommaLocsTy CommaLocs;
-if (Tok.isNot(tok::r_paren) && ParseExpressionList(ArgExprs, CommaLocs)) {
+if (Tok.isNot(tok::r_paren) &&
+ParseExpressionList(ArgExprs, CommaLocs, [&] {
+  QualType PreferredType = Actions.ProduceCtorInitMemberSignatureHelp(
+  getCurScope(), ConstructorDecl, SS, TemplateTypeTy, ArgExprs, II,
+  T.getOpenLocation());
+  CalledSignatureHelp = true;
+  Actions.CodeCompleteExpression(getCurScope(), PreferredType);
+})) {
+  if (PP.isCodeCompletionReached() && !CalledSignatureHelp) {
+Actions.ProduceCtorInitMemberSignatureHelp(
+getCurScope(), ConstructorDecl, SS, TemplateTypeTy, ArgExprs, II,
+T.getOpenLocation());
+CalledSignatureHelp = true;
+  }
   SkipUntil(tok::r_paren, StopAtSemi);
   return true;
 }

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=341949&r1=341948&r2=341949&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Tue Sep 11 08:02:18 2018
@@ -4586,6 +4586,25 @@ QualType Sema::ProduceConstructorSignatu
   return ProduceSignatureHelp(*this, S, Results, 

[PATCH] D51090: [clangd] Add index benchmarks

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D51090#1230579, @kbobyrev wrote:

> The only problem left is that I'm not sure how to run binary which is not 
> under bin (`IndexBenchmark`) using llvm-lit.


But it's a //benchmark//.. What use could be from running it as part of the 
_tests_?
For test-suite (lnt) integration, i'm not sure.


https://reviews.llvm.org/D51090



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

[PATCH] D51917: [CodeCompletion] Enable signature help when initializing class/struct/union members.

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC341949: [CodeCompletion] Enable signature help when 
initializing class/struct/union… (authored by kadircet, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51917?vs=164866&id=164883#toc

Repository:
  rC Clang

https://reviews.llvm.org/D51917

Files:
  include/clang/Sema/Sema.h
  lib/Parse/ParseDeclCXX.cpp
  lib/Sema/SemaCodeComplete.cpp
  lib/Sema/SemaDeclCXX.cpp
  test/CodeCompletion/ctor-initializer.cpp

Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -4568,6 +4568,11 @@
   // of a ComparisonCategoryType enumerator.
   llvm::SmallBitVector FullyCheckedComparisonCategories;
 
+  ValueDecl *tryLookupCtorInitMemberDecl(CXXRecordDecl *ClassDecl,
+ CXXScopeSpec &SS,
+ ParsedType TemplateTypeTy,
+ IdentifierInfo *MemberOrBase);
+
 public:
   /// Lookup the specified comparison category types in the standard
   ///   library, an check the VarDecls possibly returned by the operator<=>
@@ -10254,6 +10259,12 @@
SourceLocation Loc,
ArrayRef Args,
SourceLocation OpenParLoc);
+  QualType ProduceCtorInitMemberSignatureHelp(Scope *S, Decl *ConstructorDecl,
+  CXXScopeSpec SS,
+  ParsedType TemplateTypeTy,
+  ArrayRef ArgExprs,
+  IdentifierInfo *II,
+  SourceLocation OpenParLoc);
   void CodeCompleteInitializer(Scope *S, Decl *D);
   void CodeCompleteReturn(Scope *S);
   void CodeCompleteAfterIf(Scope *S);
@@ -10794,7 +10805,6 @@
   /// The template function declaration to be late parsed.
   Decl *D;
 };
-
 } // end namespace clang
 
 namespace llvm {
Index: test/CodeCompletion/ctor-initializer.cpp
===
--- test/CodeCompletion/ctor-initializer.cpp
+++ test/CodeCompletion/ctor-initializer.cpp
@@ -64,3 +64,27 @@
   // CHECK-CC8: COMPLETION: Pattern : member2(<#args#>
   int member1, member2;
 };
+
+struct Base2 {
+  Base2(int);
+};
+
+struct Composition1 {
+  Composition1() : b2_elem() {}
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:73:28 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:73:28 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // CHECK-CC9: OVERLOAD: Base2(<#int#>)
+  // CHECK-CC9: OVERLOAD: Base2(<#const Base2 &#>)
+  // CHECK-CC9-NOT: OVERLOAD: Composition1
+  Composition1(Base2);
+  Base2 b2_elem;
+};
+
+struct Composition2 {
+  Composition2() : c1_elem(Base2(1)) {}
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:84:34 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:84:34 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:84:35 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:84:35 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  Composition1 c1_elem;
+};
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -3777,6 +3777,22 @@
 
 }
 
+ValueDecl *Sema::tryLookupCtorInitMemberDecl(CXXRecordDecl *ClassDecl,
+ CXXScopeSpec &SS,
+ ParsedType TemplateTypeTy,
+ IdentifierInfo *MemberOrBase) {
+  if (SS.getScopeRep() || TemplateTypeTy)
+return nullptr;
+  DeclContext::lookup_result Result = ClassDecl->lookup(MemberOrBase);
+  if (Result.empty())
+return nullptr;
+  ValueDecl *Member;
+  if ((Member = dyn_cast(Result.front())) ||
+  (Member = dyn_cast(Result.front(
+return Member;
+  return nullptr;
+}
+
 /// Handle a C++ member initializer.
 MemInitResult
 Sema::BuildMemInitializer(Decl *ConstructorD,
@@ -3820,21 +3836,16 @@
   //   of a single identifier refers to the class member. A
   //   mem-initializer-id for the hidden base class may be specified
   //   using a qualified name. ]
-  if (!SS.getScopeRep() && !TemplateTypeTy) {
-// Look for a member, first.
-DeclContext::lookup_result Result = ClassDecl->lookup(MemberOrBase);
-if (!Result.empty()) {
-  ValueDecl *Member;
-  if ((Member = dyn_cast(Result.front())) ||
-  (Member = dyn_cast(Result.front( {

[PATCH] D51917: [CodeCompletion] Enable signature help when initializing class/struct/union members.

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL341949: [CodeCompletion] Enable signature help when 
initializing class/struct/union… (authored by kadircet, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D51917

Files:
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/Parse/ParseDeclCXX.cpp
  cfe/trunk/lib/Sema/SemaCodeComplete.cpp
  cfe/trunk/lib/Sema/SemaDeclCXX.cpp
  cfe/trunk/test/CodeCompletion/ctor-initializer.cpp

Index: cfe/trunk/include/clang/Sema/Sema.h
===
--- cfe/trunk/include/clang/Sema/Sema.h
+++ cfe/trunk/include/clang/Sema/Sema.h
@@ -4568,6 +4568,11 @@
   // of a ComparisonCategoryType enumerator.
   llvm::SmallBitVector FullyCheckedComparisonCategories;
 
+  ValueDecl *tryLookupCtorInitMemberDecl(CXXRecordDecl *ClassDecl,
+ CXXScopeSpec &SS,
+ ParsedType TemplateTypeTy,
+ IdentifierInfo *MemberOrBase);
+
 public:
   /// Lookup the specified comparison category types in the standard
   ///   library, an check the VarDecls possibly returned by the operator<=>
@@ -10254,6 +10259,12 @@
SourceLocation Loc,
ArrayRef Args,
SourceLocation OpenParLoc);
+  QualType ProduceCtorInitMemberSignatureHelp(Scope *S, Decl *ConstructorDecl,
+  CXXScopeSpec SS,
+  ParsedType TemplateTypeTy,
+  ArrayRef ArgExprs,
+  IdentifierInfo *II,
+  SourceLocation OpenParLoc);
   void CodeCompleteInitializer(Scope *S, Decl *D);
   void CodeCompleteReturn(Scope *S);
   void CodeCompleteAfterIf(Scope *S);
@@ -10794,7 +10805,6 @@
   /// The template function declaration to be late parsed.
   Decl *D;
 };
-
 } // end namespace clang
 
 namespace llvm {
Index: cfe/trunk/test/CodeCompletion/ctor-initializer.cpp
===
--- cfe/trunk/test/CodeCompletion/ctor-initializer.cpp
+++ cfe/trunk/test/CodeCompletion/ctor-initializer.cpp
@@ -64,3 +64,27 @@
   // CHECK-CC8: COMPLETION: Pattern : member2(<#args#>
   int member1, member2;
 };
+
+struct Base2 {
+  Base2(int);
+};
+
+struct Composition1 {
+  Composition1() : b2_elem() {}
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:73:28 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:73:28 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // CHECK-CC9: OVERLOAD: Base2(<#int#>)
+  // CHECK-CC9: OVERLOAD: Base2(<#const Base2 &#>)
+  // CHECK-CC9-NOT: OVERLOAD: Composition1
+  Composition1(Base2);
+  Base2 b2_elem;
+};
+
+struct Composition2 {
+  Composition2() : c1_elem(Base2(1)) {}
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:84:34 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:84:34 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:84:35 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:84:35 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  Composition1 c1_elem;
+};
Index: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
===
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp
@@ -4586,6 +4586,25 @@
   return ProduceSignatureHelp(*this, S, Results, Args.size(), OpenParLoc);
 }
 
+QualType Sema::ProduceCtorInitMemberSignatureHelp(
+Scope *S, Decl *ConstructorDecl, CXXScopeSpec SS, ParsedType TemplateTypeTy,
+ArrayRef ArgExprs, IdentifierInfo *II, SourceLocation OpenParLoc) {
+  if (!CodeCompleter)
+return QualType();
+
+  CXXConstructorDecl *Constructor =
+  dyn_cast(ConstructorDecl);
+  if (!Constructor)
+return QualType();
+  // FIXME: Add support for Base class constructors as well.
+  if (ValueDecl *MemberDecl = tryLookupCtorInitMemberDecl(
+  Constructor->getParent(), SS, TemplateTypeTy, II))
+return ProduceConstructorSignatureHelp(getCurScope(), MemberDecl->getType(),
+   MemberDecl->getLocation(), ArgExprs,
+   OpenParLoc);
+  return QualType();
+}
+
 void Sema::CodeCompleteInitializer(Scope *S, Decl *D) {
   ValueDecl *VD = dyn_cast_or_null(D);
   if (!VD) {
Index: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
===
--- cfe/trun

[PATCH] D43871: [modules] No longer include stdlib.h from mm_malloc.h.

[Raphael Isemann via Phabricator via cfe-commits]

teemperor updated this revision to Diff 164885.
teemperor marked 3 inline comments as done.
teemperor added a comment.

- Removed comment about redeclaring free in the test. That's wasn't correctly 
formulated and is anyway no longer true now that the test case including this 
file got bigger.
- Fixed formatting in the free declaration.

(Thanks, Richard).


https://reviews.llvm.org/D43871

Files:
  lib/Headers/mm_malloc.h
  lib/Sema/SemaExceptionSpec.cpp
  test/CXX/except/except.spec/Inputs/clang/mm_malloc.h
  test/CXX/except/except.spec/Inputs/clang/module.modulemap
  test/CXX/except/except.spec/Inputs/libc/module.modulemap
  test/CXX/except/except.spec/Inputs/libc/stdlib.h
  test/CXX/except/except.spec/libc-empty-except.cpp

Index: test/CXX/except/except.spec/libc-empty-except.cpp
===
--- /dev/null
+++ test/CXX/except/except.spec/libc-empty-except.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -std=c++11 -isystem %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -isystem %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify -DREVERSE %s
+
+// One of the headers is in a user include, so our redeclaration should fail.
+// RUN: not %clang_cc1 -std=c++11 -I %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -std=c++11 -isystem %S/Inputs/clang -I %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify -DREVERSE %s
+
+// The same test cases again with enabled modules.
+// The modules cases *all* pass because we marked both as [system].
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN:-std=c++11 -isystem %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify %s
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN:-std=c++11 -isystem %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify -DREVERSE %s
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN:-std=c++11 -I %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify %s
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN:-std=c++11 -isystem %S/Inputs/clang -I %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify -DREVERSE %s
+
+// expected-no-diagnostics
+#ifdef REVERSE
+#include "mm_malloc.h"
+#include "stdlib.h"
+#else
+#include "mm_malloc.h"
+#include "stdlib.h"
+#endif
+
+void f() {
+  free(nullptr);
+}
Index: test/CXX/except/except.spec/Inputs/libc/stdlib.h
===
--- /dev/null
+++ test/CXX/except/except.spec/Inputs/libc/stdlib.h
@@ -0,0 +1,2 @@
+// Declare 'free' like glibc with a empty exception specifier.
+extern "C" void free(void *ptr) throw();
Index: test/CXX/except/except.spec/Inputs/libc/module.modulemap
===
--- /dev/null
+++ test/CXX/except/except.spec/Inputs/libc/module.modulemap
@@ -0,0 +1,4 @@
+module libc [system] {
+  header "stdlib.h"
+  export *
+}
Index: test/CXX/except/except.spec/Inputs/clang/module.modulemap
===
--- /dev/null
+++ test/CXX/except/except.spec/Inputs/clang/module.modulemap
@@ -0,0 +1,4 @@
+module builtin [system] {
+  header "mm_malloc.h"
+  export *
+}
Index: test/CXX/except/except.spec/Inputs/clang/mm_malloc.h
===
--- /dev/null
+++ test/CXX/except/except.spec/Inputs/clang/mm_malloc.h
@@ -0,0 +1,2 @@
+// Missing throw() is allowed in this case as we are in a system header.
+extern "C" void free(void *ptr);
Index: lib/Sema/SemaExceptionSpec.cpp
===
--- lib/Sema/SemaExceptionSpec.cpp
+++ lib/Sema/SemaExceptionSpec.cpp
@@ -246,6 +246,7 @@
 const FunctionProtoType *New, SourceLocation NewLoc,
 bool *MissingExceptionSpecification = nullptr,
 bool *MissingEmptyExceptionSpecification = nullptr,
+bool *ExtraEmptyExceptionSpecification = nullptr,
 bool AllowNoexceptAllMatchWithNoSpec = false, bool IsOperatorNew = false);
 
 /// Determine whether a function has an implicitly-generated exception
@@ -269,6 +270,15 @@
   return !Ty->hasExceptionSpec();
 }
 
+/// Returns true if the given function is a function/builtin with C lin

[clang-tools-extra] r341950 - [clangd] Add unittests for D51917

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Tue Sep 11 08:12:10 2018
New Revision: 341950

URL: http://llvm.org/viewvc/llvm-project?rev=341950&view=rev
Log:
[clangd] Add unittests for D51917

Reviewers: ilya-biryukov

Reviewed By: ilya-biryukov

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

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

Modified:
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=341950&r1=341949&r2=341950&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Tue Sep 11 
08:12:10 2018
@@ -1967,6 +1967,45 @@ TEST(SignatureHelpTest, InsideArgument)
   }
 }
 
+TEST(SignatureHelpTest, ConstructorInitializeFields) {
+  {
+const auto Results = signatures(R"cpp(
+  struct A {
+A(int);
+  };
+  struct B {
+B() : a_elem(^) {}
+A a_elem;
+  };
+)cpp");
+EXPECT_THAT(Results.signatures, UnorderedElementsAre(
+Sig("A(int)", {"int"}),
+Sig("A(A &&)", {"A &&"}),
+Sig("A(const A &)", {"const A &"})
+));
+  }
+  {
+const auto Results = signatures(R"cpp(
+  struct A {
+A(int);
+  };
+  struct C {
+C(int);
+C(A);
+  };
+  struct B {
+B() : c_elem(A(1^)) {}
+C c_elem;
+  };
+)cpp");
+EXPECT_THAT(Results.signatures, UnorderedElementsAre(
+Sig("A(int)", {"int"}),
+Sig("A(A &&)", {"A &&"}),
+Sig("A(const A &)", {"const A &"})
+));
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang


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

[PATCH] D51924: [clangd] Add unittests for D51917

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL341950: [clangd] Add unittests for D51917 (authored by 
kadircet, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51924?vs=164845&id=164886#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51924

Files:
  clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp


Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
@@ -1967,6 +1967,45 @@
   }
 }
 
+TEST(SignatureHelpTest, ConstructorInitializeFields) {
+  {
+const auto Results = signatures(R"cpp(
+  struct A {
+A(int);
+  };
+  struct B {
+B() : a_elem(^) {}
+A a_elem;
+  };
+)cpp");
+EXPECT_THAT(Results.signatures, UnorderedElementsAre(
+Sig("A(int)", {"int"}),
+Sig("A(A &&)", {"A &&"}),
+Sig("A(const A &)", {"const A &"})
+));
+  }
+  {
+const auto Results = signatures(R"cpp(
+  struct A {
+A(int);
+  };
+  struct C {
+C(int);
+C(A);
+  };
+  struct B {
+B() : c_elem(A(1^)) {}
+C c_elem;
+  };
+)cpp");
+EXPECT_THAT(Results.signatures, UnorderedElementsAre(
+Sig("A(int)", {"int"}),
+Sig("A(A &&)", {"A &&"}),
+Sig("A(const A &)", {"const A &"})
+));
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang


Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
@@ -1967,6 +1967,45 @@
   }
 }
 
+TEST(SignatureHelpTest, ConstructorInitializeFields) {
+  {
+const auto Results = signatures(R"cpp(
+  struct A {
+A(int);
+  };
+  struct B {
+B() : a_elem(^) {}
+A a_elem;
+  };
+)cpp");
+EXPECT_THAT(Results.signatures, UnorderedElementsAre(
+Sig("A(int)", {"int"}),
+Sig("A(A &&)", {"A &&"}),
+Sig("A(const A &)", {"const A &"})
+));
+  }
+  {
+const auto Results = signatures(R"cpp(
+  struct A {
+A(int);
+  };
+  struct C {
+C(int);
+C(A);
+  };
+  struct B {
+B() : c_elem(A(1^)) {}
+C c_elem;
+  };
+)cpp");
+EXPECT_THAT(Results.signatures, UnorderedElementsAre(
+Sig("A(int)", {"int"}),
+Sig("A(A &&)", {"A &&"}),
+Sig("A(const A &)", {"const A &"})
+));
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D51847: Print correctly dependency paths on Windows

[Zachary Turner via cfe-commits]

I mean in practice. What command do i run to hit this and what will the
output look like? Can you paste some terminal output showing the command
and output?
On Tue, Sep 11, 2018 at 12:55 AM Dávid Bolvanský via Phabricator <
revi...@reviews.llvm.org> wrote:

> xbolva00 added a comment.
>
> In https://reviews.llvm.org/D51847#1228927, @zturner wrote:
>
> > What prints this? How do you exercise this codepath?
>
>
> DFGImpl::OutputDependencyFile() -> for (StringRef File : Files)
>
>
> https://reviews.llvm.org/D51847
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51910: [Modules] Add platform feature to requires clause

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: docs/Modules.rst:470
+*platform variant*
+  A specific os/platform variant (e.g. ``ios``, ``macos``, ``android``, 
``win32``, ``linux``, etc) is available.
 

Does this work with platforms+environment combinations, such as the ios 
simulator on Darwin?


Repository:
  rC Clang

https://reviews.llvm.org/D51910



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

[PATCH] D51847: Print correctly dependency paths on Windows

[Zachary Turner via Phabricator via cfe-commits]

zturner added a comment.

I mean in practice. What command do i run to hit this and what will the
output look like? Can you paste some terminal output showing the command
and output?


https://reviews.llvm.org/D51847



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

[PATCH] D51090: [clangd] Add index benchmarks

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Find a hacky workaround to call `IndexBenchmark` binary.


https://reviews.llvm.org/D51090

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/benchmarks/CMakeLists.txt
  clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
  clang-tools-extra/test/clangd/Inputs/BenchmarkHeader.h
  clang-tools-extra/test/clangd/Inputs/BenchmarkSource.cpp
  clang-tools-extra/test/clangd/Inputs/requests.log
  clang-tools-extra/test/clangd/benchmark-dummy.test
  clang-tools-extra/test/lit.cfg

Index: clang-tools-extra/test/lit.cfg
===
--- clang-tools-extra/test/lit.cfg
+++ clang-tools-extra/test/lit.cfg
@@ -137,3 +137,9 @@
 else:
 # exclude the clang-tidy test directory
 config.excludes.append('clang-tidy')
+
+clangd_benchmarks_dir = os.path.join(os.path.dirname(config.clang_tools_dir),
+ "tools", "clang", "tools", "extra",
+ "clangd", "benchmarks")
+config.substitutions.append(('%clangd-benchmark-dir',
+ '%s' % (clangd_benchmarks_dir)))
Index: clang-tools-extra/test/clangd/benchmark-dummy.test
===
--- /dev/null
+++ clang-tools-extra/test/clangd/benchmark-dummy.test
@@ -0,0 +1,2 @@
+# RUN: global-symbol-builder %p/Inputs/BenchmarkSource.cpp -- -I%p/Inputs > %t.index
+# RUN: %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.log
Index: clang-tools-extra/test/clangd/Inputs/requests.log
===
--- /dev/null
+++ clang-tools-extra/test/clangd/Inputs/requests.log
@@ -0,0 +1,5 @@
+V[09:08:34.301] Code complete: fuzzyFind("s", scopes=[llvm::])
+V[09:08:34.368] Code complete: fuzzyFind("sy", scopes=[llvm::])
+V[09:08:34.442] Code complete: fuzzyFind("sys", scopes=[llvm::])
+V[09:09:12.075] Code complete: fuzzyFind("Dex", scopes=[clang::clangd::, clang::, clang::clangd::dex::])
+V[09:09:12.075] Code complete: fuzzyFind("Variable", scopes=[])
Index: clang-tools-extra/test/clangd/Inputs/BenchmarkSource.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clangd/Inputs/BenchmarkSource.cpp
@@ -0,0 +1 @@
+#include "BenchmarkHeader.h"
Index: clang-tools-extra/test/clangd/Inputs/BenchmarkHeader.h
===
--- /dev/null
+++ clang-tools-extra/test/clangd/Inputs/BenchmarkHeader.h
@@ -0,0 +1,19 @@
+namespace clang {
+namespace clangd {
+namespace dex {
+class Dex;
+} // namespace dex
+} // namespace clangd
+} // namespace clang
+
+namespace llvm {
+namespace sys {
+
+int getHostNumPhysicalCores();
+
+} // namespace sys
+} // namespace llvm
+
+namespace {
+int Variable;
+} // namespace
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
@@ -0,0 +1,113 @@
+//===--- IndexBenchmark.cpp - Clangd index benchmarks ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "../index/SymbolYAML.h"
+#include "../index/dex/Dex.h"
+#include "benchmark/benchmark.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Regex.h"
+#include 
+#include 
+#include 
+
+const char *IndexFilename;
+const char *LogFilename;
+
+namespace clang {
+namespace clangd {
+namespace {
+
+std::unique_ptr buildMem() {
+  return clang::clangd::loadIndex(IndexFilename, {}, false);
+}
+
+std::unique_ptr buildDex() {
+  return clang::clangd::loadIndex(IndexFilename, {}, true);
+}
+
+// This function processes user-provided Log file with fuzzy find requests in
+// the following format:
+//
+// fuzzyFind("UnqualifiedName", scopes=["clang::", "clang::clangd::"])
+//
+// It constructs vector of FuzzyFindRequests which is later used for the
+// benchmarks.
+std::vector extractQueriesFromLogs() {
+  llvm::Regex RequestMatcher("fuzzyFind\\(\"([a-zA-Z]*)\", scopes=\\[(.*)\\]");
+  llvm::SmallVector Matches;
+  std::ifstream InputStream(LogFilename);
+  std::string Log((std::istreambuf_iterator(InputStream)),
+  std::istreambuf_iterator());
+  llvm::StringRef Temporary(Log);
+  llvm::SmallVector Strings;
+  Temporary.split(Strings, '\n');
+
+  clang::clangd::FuzzyFindRequest R;
+  R.MaxCandidateCount = 100;
+
+  llvm::SmallVector CommaSeparatedValues;
+
+  std::vector RealRequests;
+  for (auto Line : Strings) {
+if (RequestMatcher.match(Line, &Matches)) {
+  R.Query = Matches[1];
+  

[PATCH] D51090: [clangd] Add index benchmarks

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D51090#1230629, @sammccall wrote:

> In https://reviews.llvm.org/D51090#1230582, @lebedev.ri wrote:
>
> > In https://reviews.llvm.org/D51090#1230579, @kbobyrev wrote:
> >
> > > The only problem left is that I'm not sure how to run binary which is not 
> > > under bin (`IndexBenchmark`) using llvm-lit.
> >
> >
> > But it's a //benchmark//.. What use could be from running it as part of the 
> > _tests_?
> >  For test-suite (lnt) integration, i'm not sure.
>
>
> Same as any other binary that isn't itself a test - make sure it works :-)
>  (Mostly just that it doesn't crash)


Then you don't actually care about the measurements, do you?
It would be a great idea to add `--benchmark_min_time=0.01` so it does not 
actually try to measure anything good.


https://reviews.llvm.org/D51090



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

[PATCH] D51090: [clangd] Add index benchmarks

[Sam McCall via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D51090#1230582, @lebedev.ri wrote:

> In https://reviews.llvm.org/D51090#1230579, @kbobyrev wrote:
>
> > The only problem left is that I'm not sure how to run binary which is not 
> > under bin (`IndexBenchmark`) using llvm-lit.
>
>
> But it's a //benchmark//.. What use could be from running it as part of the 
> _tests_?
>  For test-suite (lnt) integration, i'm not sure.


Same as any other binary that isn't itself a test - make sure it works :-)
(Mostly just that it doesn't crash)




Comment at: clang-tools-extra/test/clangd/benchmark-dummy.test:1
+# RUN: global-symbol-builder %p/Inputs/BenchmarkSource.cpp -- -I%p/Inputs > 
%t.index
+# RUN: %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.log

i'd call this `index-tools.test` - it's a good integration test for 
global-symbol-builder, better when we add dexp to the test to verify the output.


https://reviews.llvm.org/D51090



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

[PATCH] D51926: [scan-build-py] Prevent crashes of CTU analysis by suppressing warnings

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

Will this properly synergise across compilers with user-specified warning 
options, such as `-Wall -Werror`?


Repository:
  rC Clang

https://reviews.llvm.org/D51926



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

[PATCH] D51628: [clangd] Implement a Proof-of-Concept tool for symbol index exploration

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 164893.
kbobyrev marked 9 inline comments as done.
kbobyrev added a comment.

Address comments


https://reviews.llvm.org/D51628

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/dexp/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp

Index: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
@@ -0,0 +1,162 @@
+//===--- Dexp.cpp - Dex EXPloration tool *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file implements a simple interactive tool which can be used to manually
+// evaluate symbol search quality of Clangd index.
+//
+//===--===//
+
+#include "../../../index/SymbolYAML.h"
+#include "../Dex.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSwitch.h"
+#include "llvm/LineEditor/LineEditor.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Signals.h"
+
+using clang::clangd::FuzzyFindRequest;
+using clang::clangd::loadIndex;
+using clang::clangd::Symbol;
+using clang::clangd::SymbolIndex;
+using llvm::StringRef;
+
+namespace {
+
+llvm::cl::opt
+SymbolCollection("symbol-collection-file",
+ llvm::cl::desc("Path to the file with symbol collection"),
+ llvm::cl::Positional, llvm::cl::Required);
+
+static const std::string Overview = R"(
+This is an **experimental** interactive tool to process user-provided search
+queries over given symbol collection obtained via global-symbol-builder. The
+tool can be used to evaluate search quality of existing index implementations
+and manually construct non-trivial test cases.
+
+Type use "help" request to get information about the details.
+)";
+
+void reportTime(StringRef Name, llvm::function_ref F) {
+  const auto TimerStart = std::chrono::high_resolution_clock::now();
+  F();
+  const auto TimerStop = std::chrono::high_resolution_clock::now();
+  const auto Duration = std::chrono::duration_cast(
+  TimerStop - TimerStart);
+  llvm::outs() << llvm::formatv("{0} took {1:ms+n}.\n", Name, Duration);
+}
+
+// llvm::formatv string pattern for pretty-printing symbols.
+void fuzzyFind(llvm::StringRef UnqualifiedName, const SymbolIndex &Index) {
+  FuzzyFindRequest Request;
+  Request.MaxCandidateCount = 10;
+  Request.Query = UnqualifiedName;
+  // FIXME(kbobyrev): Print symbol final scores to see the distribution.
+  static const auto OutputFormat = "{0,-4} | {1,-40} | {2,-25}\n";
+  llvm::outs() << llvm::formatv(OutputFormat, "Rank", "Symbol ID",
+"Symbol Name");
+  size_t Rank = 0;
+  Index.fuzzyFind(Request, [&](const Symbol &Sym) {
+llvm::outs() << llvm::formatv(OutputFormat, Rank++, Sym.ID.str(), Sym.Name);
+  });
+}
+
+static const std::string HelpMessage = R"(dexp commands:
+
+> find Name
+
+Constructs fuzzy find request given unqualified symbol name and returns top 10
+symbols retrieved from index.
+
+> lookup SymbolID
+
+Retrieves symbol names given USR.
+)";
+
+void help() { llvm::outs() << HelpMessage; }
+
+void lookup(StringRef USR, const SymbolIndex &Index) {
+  llvm::DenseSet IDs{clang::clangd::SymbolID{USR}};
+  clang::clangd::LookupRequest Request{IDs};
+  bool FoundSymbol = false;
+  Index.lookup(Request, [&](const Symbol &Sym) {
+if (!FoundSymbol)
+  FoundSymbol = true;
+llvm::outs() << SymbolToYAML(Sym);
+  });
+  if (!FoundSymbol)
+llvm::outs() << "not found\n";
+}
+
+// FIXME(kbobyrev): Make this an actual REPL: probably use LLVM Command Line
+// library for parsing flags and arguments.
+// FIXME(kbobyrev): Ideas for commands:
+// * symbol lookup: print out symbol in YAML format given SymbolID
+// * find symbol references: print set of reference locations
+// * load/swap/reload index: this would make it possible to get rid of llvm::cl
+//   usages in the tool driver and actually use llvm::cl library in the REPL.
+// * show posting list density histogram (our dump data somewhere so that user
+//   could build one)
+// * show number of tokens of each kind
+// * print out tokens with the most dense posting lists
+// * print out tokens with least dense posting lists
+void dispatch(StringRef Request, const SymbolIndex &Index) {
+  llvm::SmallVector Arguments;
+  Request.split(Arguments, ' ');
+  if (Arguments.empty()) {
+llvm::outs() << "Request can not be empty.\n";
+help();
+return;
+  }
+
+  if (Arguments.front() == "find") {
+if (Arguments.size() != 2) {
+  llvm::outs() << "find request must specify unqualified symbol name

[PATCH] D51090: [clangd] Add index benchmarks

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Add `--benchmark_min_time=0.01` to prevent testing time increase.


https://reviews.llvm.org/D51090

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/benchmarks/CMakeLists.txt
  clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
  clang-tools-extra/test/clangd/Inputs/BenchmarkHeader.h
  clang-tools-extra/test/clangd/Inputs/BenchmarkSource.cpp
  clang-tools-extra/test/clangd/Inputs/requests.log
  clang-tools-extra/test/clangd/benchmark-dummy.test
  clang-tools-extra/test/lit.cfg

Index: clang-tools-extra/test/lit.cfg
===
--- clang-tools-extra/test/lit.cfg
+++ clang-tools-extra/test/lit.cfg
@@ -137,3 +137,9 @@
 else:
 # exclude the clang-tidy test directory
 config.excludes.append('clang-tidy')
+
+clangd_benchmarks_dir = os.path.join(os.path.dirname(config.clang_tools_dir),
+ "tools", "clang", "tools", "extra",
+ "clangd", "benchmarks")
+config.substitutions.append(('%clangd-benchmark-dir',
+ '%s' % (clangd_benchmarks_dir)))
Index: clang-tools-extra/test/clangd/benchmark-dummy.test
===
--- /dev/null
+++ clang-tools-extra/test/clangd/benchmark-dummy.test
@@ -0,0 +1,2 @@
+# RUN: global-symbol-builder %p/Inputs/BenchmarkSource.cpp -- -I%p/Inputs > %t.index
+# RUN: %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.log --benchmark_min_time=0.01
Index: clang-tools-extra/test/clangd/Inputs/requests.log
===
--- /dev/null
+++ clang-tools-extra/test/clangd/Inputs/requests.log
@@ -0,0 +1,5 @@
+V[09:08:34.301] Code complete: fuzzyFind("s", scopes=[llvm::])
+V[09:08:34.368] Code complete: fuzzyFind("sy", scopes=[llvm::])
+V[09:08:34.442] Code complete: fuzzyFind("sys", scopes=[llvm::])
+V[09:09:12.075] Code complete: fuzzyFind("Dex", scopes=[clang::clangd::, clang::, clang::clangd::dex::])
+V[09:09:12.075] Code complete: fuzzyFind("Variable", scopes=[])
Index: clang-tools-extra/test/clangd/Inputs/BenchmarkSource.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clangd/Inputs/BenchmarkSource.cpp
@@ -0,0 +1 @@
+#include "BenchmarkHeader.h"
Index: clang-tools-extra/test/clangd/Inputs/BenchmarkHeader.h
===
--- /dev/null
+++ clang-tools-extra/test/clangd/Inputs/BenchmarkHeader.h
@@ -0,0 +1,19 @@
+namespace clang {
+namespace clangd {
+namespace dex {
+class Dex;
+} // namespace dex
+} // namespace clangd
+} // namespace clang
+
+namespace llvm {
+namespace sys {
+
+int getHostNumPhysicalCores();
+
+} // namespace sys
+} // namespace llvm
+
+namespace {
+int Variable;
+} // namespace
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
@@ -0,0 +1,113 @@
+//===--- IndexBenchmark.cpp - Clangd index benchmarks ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "../index/SymbolYAML.h"
+#include "../index/dex/Dex.h"
+#include "benchmark/benchmark.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Regex.h"
+#include 
+#include 
+#include 
+
+const char *IndexFilename;
+const char *LogFilename;
+
+namespace clang {
+namespace clangd {
+namespace {
+
+std::unique_ptr buildMem() {
+  return clang::clangd::loadIndex(IndexFilename, {}, false);
+}
+
+std::unique_ptr buildDex() {
+  return clang::clangd::loadIndex(IndexFilename, {}, true);
+}
+
+// This function processes user-provided Log file with fuzzy find requests in
+// the following format:
+//
+// fuzzyFind("UnqualifiedName", scopes=["clang::", "clang::clangd::"])
+//
+// It constructs vector of FuzzyFindRequests which is later used for the
+// benchmarks.
+std::vector extractQueriesFromLogs() {
+  llvm::Regex RequestMatcher("fuzzyFind\\(\"([a-zA-Z]*)\", scopes=\\[(.*)\\]");
+  llvm::SmallVector Matches;
+  std::ifstream InputStream(LogFilename);
+  std::string Log((std::istreambuf_iterator(InputStream)),
+  std::istreambuf_iterator());
+  llvm::StringRef Temporary(Log);
+  llvm::SmallVector Strings;
+  Temporary.split(Strings, '\n');
+
+  clang::clangd::FuzzyFindRequest R;
+  R.MaxCandidateCount = 100;
+
+  llvm::SmallVector CommaSeparatedValues;
+
+  std::vector RealRequests;
+  for (auto Line : Strings) {
+if (RequestMatcher.match(Line, &Matches)) {
+

[PATCH] D51628: [clangd] Implement a Proof-of-Concept tool for symbol index exploration

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:55
+
+// llvm::formatv string pattern for pretty-printing symbols.
+void fuzzyFind(llvm::StringRef UnqualifiedName, const SymbolIndex &Index) {

Is this a leftover from previous changes?


https://reviews.llvm.org/D51628



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

[PATCH] D51544: [OpenCL] Split opencl-c.h header

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In https://reviews.llvm.org/D51544#1230264, @asavonic wrote:

> In https://reviews.llvm.org/D51544#1229105, @Anastasia wrote:
>
> > > With this setup, we can compile opencl-c-common.h, opencl-c-fp16.h and
> > >  opencl-c-fp64.h into PCHs with one set of extensions/OpenCL version,
> > >  and use them for any other set of extensions/OpenCL version. Clang
> > >  will detect this and throw out an error, which can be safely disabled
> > >  by -fno-validate-pch option.
> >
> > However, keeping this as a permanent solution is unsafe. Because this way 
> > can result in unexpected errors to be silent out and allow erroneous 
> > configurations to be accepted successfully without any notification.
>
>
> Agree. LIT test in `test/Headers/opencl-c-header-split.cl` is supposed
>  to verify that common/fp16/fp64 headers do not use preprocessor macros
>  and it should catch most of the issues, but this is definitely not the
>  most robust solution.
>
> > So I am wondering if there is any plan to put a proper solution in place at 
> > some point?
>
> This solution can be improved if we implement `convert` and `shuffle`
>  as clang builtins with custom typechecking: these two builtins (with
>  all their overloadings) take ~90% of `opencl-c-common.h` and ~50% of
>  fp16/fp64 headers.
>
> However, this can be a non-trivial change: it is difficult to do a
>  proper mangling for clang builtins and rounding modes must be handled
>  as well.
>
> I'll take a few days to prototype this. If it turns out to be good in
>  terms of performance/footprint, we can drop this patch.


Btw, I was wondering if a target agnostic PCH is a viable solution to move 
forward too. Something that we discussed/prototyped during the HackerLab in 
EuroLLVM?


Repository:
  rC Clang

https://reviews.llvm.org/D51544



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

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 164898.
JonasToth added a comment.

- ignore lambdas properly


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp
  clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-const-correctness.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-const-correctness-pointer-as-values.cpp
  test/clang-tidy/cppcoreguidelines-const-correctness-values.cpp

Index: test/clang-tidy/cppcoreguidelines-const-correctness-values.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-const-correctness-values.cpp
@@ -0,0 +1,563 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-const-correctness %t
+
+// --- Provide test samples for primitive builtins -
+// - every 'p_*' variable is a 'potential_const_*' variable
+// - every 'np_*' variable is a 'non_potential_const_*' variable
+
+bool global;
+char np_global = 0; // globals can't be known to be const
+
+namespace foo {
+int scoped;
+float np_scoped = 1; // namespace variables are like globals
+} // namespace foo
+
+// Lambdas should be ignored, because they do not follow the normal variable
+// semantic (e.g. the type is only known to the compiler).
+void lambdas() {
+  auto Lambda = [](int i) { return i < 0; };
+}
+
+void some_function(double, wchar_t);
+
+void some_function(double np_arg0, wchar_t np_arg1) {
+  int p_local0 = 2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'
+
+  int np_local0;
+  const int np_local1 = 42;
+
+  unsigned int np_local2 = 3;
+  np_local2 <<= 4;
+
+  int np_local3 = 4;
+  ++np_local3;
+  int np_local4 = 4;
+  np_local4++;
+
+  int np_local5 = 4;
+  --np_local5;
+  int np_local6 = 4;
+  np_local6--;
+}
+
+void nested_scopes() {
+  int p_local0 = 2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'
+  int np_local0 = 42;
+
+  {
+int p_local1 = 42;
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: variable 'p_local1' of type 'int' can be declared 'const'
+np_local0 *= 2;
+  }
+}
+
+void some_lambda_environment_capture_all_by_reference(double np_arg0) {
+  int np_local0 = 0;
+  int p_local0 = 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'
+
+  int np_local2;
+  const int np_local3 = 2;
+
+  // Capturing all variables by reference prohibits making them const.
+  [&]() { ++np_local0; };
+
+  int p_local1 = 0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared 'const'
+}
+
+void some_lambda_environment_capture_all_by_value(double np_arg0) {
+  int np_local0 = 0;
+  int p_local0 = 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'
+
+  int np_local1;
+  const int np_local2 = 2;
+
+  // Capturing by value has no influence on them.
+  [=]() { (void)p_local0; };
+
+  np_local0 += 10;
+}
+
+void function_inout_pointer(int *inout);
+void function_in_pointer(const int *in);
+
+void some_pointer_taking(int *out) {
+  int np_local0 = 42;
+  const int *const p0_np_local0 = &np_local0;
+  int *const p1_np_local0 = &np_local0;
+
+  int np_local1 = 42;
+  const int *const p0_np_local1 = &np_local1;
+  int *const p1_np_local1 = &np_local1;
+  *p1_np_local0 = 43;
+
+  int np_local2 = 42;
+  function_inout_pointer(&np_local2);
+
+  // Prevents const.
+  int np_local3 = 42;
+  out = &np_local3; // This returns and invalid address, its just about the AST
+
+  int p_local1 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared 'const'
+  const int *const p0_p_local1 = &p_local1;
+
+  int p_local2 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'int' can be declared 'const'
+  function_in_pointer(&p_local2);
+}
+
+void function_inout_ref(int &inout);
+void function_in_ref(const int &in);
+
+void some_reference_taking() {
+  int np_local0 = 42;
+  const int &r0_np_local0 = np_local0;
+  int &r1_np_local0 = np_local0;
+  r1_np_local0 = 43;
+  const int &r2_np_local0 = r1_np_local0;
+
+  int np_local1 = 42;
+  function_inout_ref(np_local1);
+
+  int p_local0 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'
+  const int &r0_p_local0 = p_local0;
+
+  int p_local1 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared 'const'
+  function_in_ref(p_local1);
+}
+
+double *non_const_pointer_return() {
+  double p_local0 = 0.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double' can be declared 'const'

[PATCH] D51880: [ASTMatchers] add two matchers for dependent expressions

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D51880



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

[clang-tools-extra] r341955 - Reland "Implement a (simple) Markdown generator"

[Julie Hockett via cfe-commits]

Author: juliehockett
Date: Tue Sep 11 08:56:55 2018
New Revision: 341955

URL: http://llvm.org/viewvc/llvm-project?rev=341955&view=rev
Log:
Reland "Implement a (simple) Markdown generator"

Relanding with fixes to tests for the failing bots.

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

Added:
clang-tools-extra/trunk/clang-doc/MDGenerator.cpp
clang-tools-extra/trunk/test/clang-doc/md-comment.cpp
clang-tools-extra/trunk/test/clang-doc/md-linkage.cpp
clang-tools-extra/trunk/test/clang-doc/md-module.cpp
clang-tools-extra/trunk/test/clang-doc/md-namespace.cpp
clang-tools-extra/trunk/test/clang-doc/md-record.cpp
Modified:
clang-tools-extra/trunk/clang-doc/CMakeLists.txt
clang-tools-extra/trunk/clang-doc/Generators.cpp
clang-tools-extra/trunk/clang-doc/Generators.h
clang-tools-extra/trunk/clang-doc/Representation.h
clang-tools-extra/trunk/clang-doc/YAMLGenerator.cpp
clang-tools-extra/trunk/clang-doc/gen_tests.py
clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp

Modified: clang-tools-extra/trunk/clang-doc/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/CMakeLists.txt?rev=341955&r1=341954&r2=341955&view=diff
==
--- clang-tools-extra/trunk/clang-doc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-doc/CMakeLists.txt Tue Sep 11 08:56:55 2018
@@ -10,6 +10,7 @@ add_clang_library(clangDoc
   ClangDoc.cpp
   Generators.cpp
   Mapper.cpp
+  MDGenerator.cpp
   Representation.cpp
   Serialize.cpp
   YAMLGenerator.cpp

Modified: clang-tools-extra/trunk/clang-doc/Generators.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/Generators.cpp?rev=341955&r1=341954&r2=341955&view=diff
==
--- clang-tools-extra/trunk/clang-doc/Generators.cpp (original)
+++ clang-tools-extra/trunk/clang-doc/Generators.cpp Tue Sep 11 08:56:55 2018
@@ -29,8 +29,11 @@ findGeneratorByName(llvm::StringRef Form
 // This anchor is used to force the linker to link in the generated object file
 // and thus register the generators.
 extern volatile int YAMLGeneratorAnchorSource;
+extern volatile int MDGeneratorAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED YAMLGeneratorAnchorDest =
 YAMLGeneratorAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED MDGeneratorAnchorDest =
+MDGeneratorAnchorSource;
 
 } // namespace doc
 } // namespace clang

Modified: clang-tools-extra/trunk/clang-doc/Generators.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/Generators.h?rev=341955&r1=341954&r2=341955&view=diff
==
--- clang-tools-extra/trunk/clang-doc/Generators.h (original)
+++ clang-tools-extra/trunk/clang-doc/Generators.h Tue Sep 11 08:56:55 2018
@@ -27,7 +27,7 @@ public:
   virtual ~Generator() = default;
 
   // Write out the decl info in the specified format.
-  virtual bool generateDocForInfo(Info *I, llvm::raw_ostream &OS) = 0;
+  virtual llvm::Error generateDocForInfo(Info *I, llvm::raw_ostream &OS) = 0;
 };
 
 typedef llvm::Registry GeneratorRegistry;

Added: clang-tools-extra/trunk/clang-doc/MDGenerator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/MDGenerator.cpp?rev=341955&view=auto
==
--- clang-tools-extra/trunk/clang-doc/MDGenerator.cpp (added)
+++ clang-tools-extra/trunk/clang-doc/MDGenerator.cpp Tue Sep 11 08:56:55 2018
@@ -0,0 +1,312 @@
+//===-- MDGenerator.cpp - Markdown Generator *- C++ 
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "Generators.h"
+#include "Representation.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+#include 
+
+using namespace llvm;
+
+namespace clang {
+namespace doc {
+
+// Enum conversion
+
+std::string getAccess(AccessSpecifier AS) {
+  switch (AS) {
+  case AccessSpecifier::AS_public:
+return "public";
+  case AccessSpecifier::AS_protected:
+return "protected";
+  case AccessSpecifier::AS_private:
+return "private";
+  case AccessSpecifier::AS_none:
+return {};
+  }
+}
+
+std::string getTagType(TagTypeKind AS) {
+  switch (AS) {
+  case TagTypeKind::TTK_Class:
+return "class";
+  case TagTypeKind::TTK_Union:
+return "union";
+  case TagTypeKind::TTK_Interface:
+return "interface";
+  case TagTypeKind::TTK_Struct:
+return "struct";
+  case TagTypeKind::TTK_Enum:
+return "enum";
+  }
+}
+
+// Markdown generation
+
+std::string genItalic(const Twine &Text) { return "*"