[PATCH] D55415: Revert removal of tidy plugin support from libclang

[Ivan Donchevskii via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348764: [libclang] Revert removal of tidy plugin support 
from libclang introduced in… (authored by yvvan, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55415?vs=177134=177515#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55415

Files:
  cfe/trunk/tools/libclang/CIndex.cpp
  cfe/trunk/tools/libclang/CMakeLists.txt


Index: cfe/trunk/tools/libclang/CIndex.cpp
===
--- cfe/trunk/tools/libclang/CIndex.cpp
+++ cfe/trunk/tools/libclang/CIndex.cpp
@@ -8938,3 +8938,16 @@
 OS << "--\n";
   }
 }
+
+#ifdef CLANG_TOOL_EXTRA_BUILD
+// This anchor is used to force the linker to link the clang-tidy plugin.
+extern volatile int ClangTidyPluginAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED ClangTidyPluginAnchorDestination =
+ClangTidyPluginAnchorSource;
+
+// This anchor is used to force the linker to link the clang-include-fixer
+// plugin.
+extern volatile int ClangIncludeFixerPluginAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED ClangIncludeFixerPluginAnchorDestination =
+ClangIncludeFixerPluginAnchorSource;
+#endif
Index: cfe/trunk/tools/libclang/CMakeLists.txt
===
--- cfe/trunk/tools/libclang/CMakeLists.txt
+++ cfe/trunk/tools/libclang/CMakeLists.txt
@@ -47,6 +47,15 @@
   list(APPEND LIBS clangARCMigrate)
 endif ()
 
+if (TARGET clangTidyPlugin)
+  add_definitions(-DCLANG_TOOL_EXTRA_BUILD)
+  list(APPEND LIBS clangTidyPlugin)
+  list(APPEND LIBS clangIncludeFixerPlugin)
+  if(LLVM_ENABLE_MODULES)
+list(APPEND LLVM_COMPILE_FLAGS 
"-fmodules-ignore-macro=CLANG_TOOL_EXTRA_BUILD")
+  endif()
+endif ()
+
 find_library(DL_LIBRARY_PATH dl)
 if (DL_LIBRARY_PATH)
   list(APPEND LIBS dl)


Index: cfe/trunk/tools/libclang/CIndex.cpp
===
--- cfe/trunk/tools/libclang/CIndex.cpp
+++ cfe/trunk/tools/libclang/CIndex.cpp
@@ -8938,3 +8938,16 @@
 OS << "--\n";
   }
 }
+
+#ifdef CLANG_TOOL_EXTRA_BUILD
+// This anchor is used to force the linker to link the clang-tidy plugin.
+extern volatile int ClangTidyPluginAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED ClangTidyPluginAnchorDestination =
+ClangTidyPluginAnchorSource;
+
+// This anchor is used to force the linker to link the clang-include-fixer
+// plugin.
+extern volatile int ClangIncludeFixerPluginAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED ClangIncludeFixerPluginAnchorDestination =
+ClangIncludeFixerPluginAnchorSource;
+#endif
Index: cfe/trunk/tools/libclang/CMakeLists.txt
===
--- cfe/trunk/tools/libclang/CMakeLists.txt
+++ cfe/trunk/tools/libclang/CMakeLists.txt
@@ -47,6 +47,15 @@
   list(APPEND LIBS clangARCMigrate)
 endif ()
 
+if (TARGET clangTidyPlugin)
+  add_definitions(-DCLANG_TOOL_EXTRA_BUILD)
+  list(APPEND LIBS clangTidyPlugin)
+  list(APPEND LIBS clangIncludeFixerPlugin)
+  if(LLVM_ENABLE_MODULES)
+list(APPEND LLVM_COMPILE_FLAGS "-fmodules-ignore-macro=CLANG_TOOL_EXTRA_BUILD")
+  endif()
+endif ()
+
 find_library(DL_LIBRARY_PATH dl)
 if (DL_LIBRARY_PATH)
   list(APPEND LIBS dl)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55415: Revert removal of tidy plugin support from libclang

[Benjamin Kramer via Phabricator via cfe-commits]

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

I think that's a fair point for bringing it back for now. It's not supported 
though and we will get rid of it eventually.


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

https://reviews.llvm.org/D55415



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

[PATCH] D55415: Revert removal of tidy plugin support from libclang

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

In D55415#1323120 , @bkramer wrote:

> I'd be interested in hearing how this is used. I added this feature as an 
> experiment a while back but it simply didn't work as I envisioned it to. Some 
> checks do work but the overall latency makes it unusable in an IDE setting. 
> People repeatedly asked me to remove it because it slows down builds while 
> not adding value.


We use it with Qt Creator and you can see that we make additions/fixes in 
libclang from time to time.
With tidy checks selected it is slow indeed but there's always a choice whether 
to enable checks or not. And if you have very few checks enabled it performs 
quite fine. And we are still using libclang because we already have 
infrastructure on top of it which can't be just replaced with clangd (at least 
it requires quite some time).


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

https://reviews.llvm.org/D55415



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

[PATCH] D55415: Revert removal of tidy plugin support from libclang

[Benjamin Kramer via Phabricator via cfe-commits]

bkramer added a comment.

I'd be interested in hearing how this is used. I added this feature as an 
experiment a while back but it simply didn't work as I envisioned it to. Some 
checks do work but the overall latency makes it unusable in an IDE setting. 
People repeatedly asked me to remove it because it slows down builds while not 
adding value.


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

https://reviews.llvm.org/D55415



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

[PATCH] D55415: Revert removal of tidy plugin support from libclang

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

We can also add an extra variable for it if you care about build time


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

https://reviews.llvm.org/D55415



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

[PATCH] D55415: Revert removal of tidy plugin support from libclang

[Nikolai Kosjar via Phabricator via cfe-commits]

nik added a comment.

+1 as this seems to fix the regression.


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

https://reviews.llvm.org/D55415



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

[PATCH] D55415: Revert removal of tidy plugin support from libclang

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan updated this revision to Diff 177134.
yvvan added a comment.
Herald added a subscriber: mgorny.

I generated the wrong diff. This is the proper one.


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

https://reviews.llvm.org/D55415

Files:
  tools/libclang/CIndex.cpp
  tools/libclang/CMakeLists.txt


Index: tools/libclang/CMakeLists.txt
===
--- tools/libclang/CMakeLists.txt
+++ tools/libclang/CMakeLists.txt
@@ -47,6 +47,15 @@
   list(APPEND LIBS clangARCMigrate)
 endif ()
 
+if (TARGET clangTidyPlugin)
+  add_definitions(-DCLANG_TOOL_EXTRA_BUILD)
+  list(APPEND LIBS clangTidyPlugin)
+  list(APPEND LIBS clangIncludeFixerPlugin)
+  if(LLVM_ENABLE_MODULES)
+list(APPEND LLVM_COMPILE_FLAGS 
"-fmodules-ignore-macro=CLANG_TOOL_EXTRA_BUILD")
+  endif()
+endif ()
+
 find_library(DL_LIBRARY_PATH dl)
 if (DL_LIBRARY_PATH)
   list(APPEND LIBS dl)
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -8938,3 +8938,16 @@
 OS << "--\n";
   }
 }
+
+#ifdef CLANG_TOOL_EXTRA_BUILD
+// This anchor is used to force the linker to link the clang-tidy plugin.
+extern volatile int ClangTidyPluginAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED ClangTidyPluginAnchorDestination =
+ClangTidyPluginAnchorSource;
+
+// This anchor is used to force the linker to link the clang-include-fixer
+// plugin.
+extern volatile int ClangIncludeFixerPluginAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED ClangIncludeFixerPluginAnchorDestination =
+ClangIncludeFixerPluginAnchorSource;
+#endif


Index: tools/libclang/CMakeLists.txt
===
--- tools/libclang/CMakeLists.txt
+++ tools/libclang/CMakeLists.txt
@@ -47,6 +47,15 @@
   list(APPEND LIBS clangARCMigrate)
 endif ()
 
+if (TARGET clangTidyPlugin)
+  add_definitions(-DCLANG_TOOL_EXTRA_BUILD)
+  list(APPEND LIBS clangTidyPlugin)
+  list(APPEND LIBS clangIncludeFixerPlugin)
+  if(LLVM_ENABLE_MODULES)
+list(APPEND LLVM_COMPILE_FLAGS "-fmodules-ignore-macro=CLANG_TOOL_EXTRA_BUILD")
+  endif()
+endif ()
+
 find_library(DL_LIBRARY_PATH dl)
 if (DL_LIBRARY_PATH)
   list(APPEND LIBS dl)
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -8938,3 +8938,16 @@
 OS << "--\n";
   }
 }
+
+#ifdef CLANG_TOOL_EXTRA_BUILD
+// This anchor is used to force the linker to link the clang-tidy plugin.
+extern volatile int ClangTidyPluginAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED ClangTidyPluginAnchorDestination =
+ClangTidyPluginAnchorSource;
+
+// This anchor is used to force the linker to link the clang-include-fixer
+// plugin.
+extern volatile int ClangIncludeFixerPluginAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED ClangIncludeFixerPluginAnchorDestination =
+ClangIncludeFixerPluginAnchorSource;
+#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55415: Revert removal of tidy plugin support from libclang

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan created this revision.
yvvan added reviewers: bkramer, arphaman, nik.
Herald added subscribers: kadircet, jkorous, ioeric, javed.absar, ilya-biryukov.

libclang has nothing to do with clangd so I don't see why having the last one 
is the reason to remove features from libclang. Especially without reviews.


https://reviews.llvm.org/D55415

Files:
  unittests/clangd/TUSchedulerTests.cpp


Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -709,29 +709,6 @@
   TUState(TUAction::Idle, /*No action*/ "")));
 }
 
-TEST_F(TUSchedulerTests, NoTUStatusEmittedForRemovedFile) {
-  class CaptureTUStatus : public DiagnosticsConsumer {
-  public:
-void onDiagnosticsReady(PathRef File,
-std::vector Diagnostics) override {}
-
-void onFileUpdated(PathRef File, const TUStatus ) override {
-  // Block the worker thread until the document is removed.
-  Removed.wait();
-}
-Notification Removed;
-  } CaptureTUStatus;
-  MockFSProvider FS;
-  MockCompilationDatabase CDB;
-  ClangdServer Server(CDB, FS, CaptureTUStatus, ClangdServer::optsForTest());
-
-  Server.addDocument(testPath("foo.cpp"), "int main() {}",
- WantDiagnostics::Yes);
-  Server.removeDocument(testPath("foo.cpp"));
-  CaptureTUStatus.Removed.notify();
-  ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for finishing";
-}
-
 } // namespace
 } // namespace clangd
 } // namespace clang


Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -709,29 +709,6 @@
   TUState(TUAction::Idle, /*No action*/ "")));
 }
 
-TEST_F(TUSchedulerTests, NoTUStatusEmittedForRemovedFile) {
-  class CaptureTUStatus : public DiagnosticsConsumer {
-  public:
-void onDiagnosticsReady(PathRef File,
-std::vector Diagnostics) override {}
-
-void onFileUpdated(PathRef File, const TUStatus ) override {
-  // Block the worker thread until the document is removed.
-  Removed.wait();
-}
-Notification Removed;
-  } CaptureTUStatus;
-  MockFSProvider FS;
-  MockCompilationDatabase CDB;
-  ClangdServer Server(CDB, FS, CaptureTUStatus, ClangdServer::optsForTest());
-
-  Server.addDocument(testPath("foo.cpp"), "int main() {}",
- WantDiagnostics::Yes);
-  Server.removeDocument(testPath("foo.cpp"));
-  CaptureTUStatus.Removed.notify();
-  ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for finishing";
-}
-
 } // namespace
 } // namespace clangd
 } // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits