[PATCH] D111100: enable plugins for clang-tidy

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

I've proposed a fix for the standalone builds here D120301 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash added inline comments.



Comment at: clang-tools-extra/test/CMakeLists.txt:91
+  if(TARGET CTTestTidyModule)
+  list(APPEND CLANG_TOOLS_TEST_DEPS CTTestTidyModule LLVMHello)
+  target_include_directories(CTTestTidyModule PUBLIC BEFORE 
"${CLANG_TOOLS_SOURCE_DIR}")

tstellar wrote:
> aaron.ballman wrote:
> > tstellar wrote:
> > > Our stand-alone builds for Fedora are still not working after this patch, 
> > > even with D119199.  Why is it necesary to add LLVMHello as a test 
> > > dependency?  Which test is using it?
> > CTestTidyModule.cpp is using it:
> > ```
> > // RUN: clang-tidy -checks='-*,mytest*' --list-checks -load 
> > %llvmshlibdir/CTTestTidyModule%pluginext -load 
> > %llvmshlibdir/LLVMHello%pluginext | FileCheck --check-prefix=CHECK-LIST %s
> > ```
> > I *think* this is testing that we can load a clang-tidy plugin and an llvm 
> > plugin at the same time and not hit conflicting symbols or other issues. If 
> > I'm correct, then I think that's useful test functionality, but I wouldn't 
> > describe it as critical, so I'd be fine if we dropped it for now to get 
> > this patch in, and then added the extra testing in a subsequent patch if we 
> > think it's necessary.
> OK, that does sound like a useful test.  I will spend some time investigating 
> how to make this work with stand-alone builds.
Yes, that is the purpose. It is only a test dependency, so we could prevent it 
from running these tests instead. However, in theory, it should have figured 
out the relative path to the llvm/ source directory in the repo and re-built 
this target it if required for the test and not already available (with 
D119199):
https://github.com/llvm/llvm-project/blob/27f72eb25e366cf6fd79ea7495fec5d926a5b895/clang-tools-extra/test/CMakeLists.txt#L92

As this seemed to be the way other subprojects were implementing it already:
https://github.com/llvm/llvm-project/blob/27f72eb25e366cf6fd79ea7495fec5d926a5b895/clang-tools-extra/clangd/unittests/CMakeLists.txt#L9


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Tom Stellard via Phabricator via cfe-commits]

tstellar added inline comments.



Comment at: clang-tools-extra/test/CMakeLists.txt:91
+  if(TARGET CTTestTidyModule)
+  list(APPEND CLANG_TOOLS_TEST_DEPS CTTestTidyModule LLVMHello)
+  target_include_directories(CTTestTidyModule PUBLIC BEFORE 
"${CLANG_TOOLS_SOURCE_DIR}")

aaron.ballman wrote:
> tstellar wrote:
> > Our stand-alone builds for Fedora are still not working after this patch, 
> > even with D119199.  Why is it necesary to add LLVMHello as a test 
> > dependency?  Which test is using it?
> CTestTidyModule.cpp is using it:
> ```
> // RUN: clang-tidy -checks='-*,mytest*' --list-checks -load 
> %llvmshlibdir/CTTestTidyModule%pluginext -load 
> %llvmshlibdir/LLVMHello%pluginext | FileCheck --check-prefix=CHECK-LIST %s
> ```
> I *think* this is testing that we can load a clang-tidy plugin and an llvm 
> plugin at the same time and not hit conflicting symbols or other issues. If 
> I'm correct, then I think that's useful test functionality, but I wouldn't 
> describe it as critical, so I'd be fine if we dropped it for now to get this 
> patch in, and then added the extra testing in a subsequent patch if we think 
> it's necessary.
OK, that does sound like a useful test.  I will spend some time investigating 
how to make this work with stand-alone builds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/test/CMakeLists.txt:91
+  if(TARGET CTTestTidyModule)
+  list(APPEND CLANG_TOOLS_TEST_DEPS CTTestTidyModule LLVMHello)
+  target_include_directories(CTTestTidyModule PUBLIC BEFORE 
"${CLANG_TOOLS_SOURCE_DIR}")

tstellar wrote:
> Our stand-alone builds for Fedora are still not working after this patch, 
> even with D119199.  Why is it necesary to add LLVMHello as a test dependency? 
>  Which test is using it?
CTestTidyModule.cpp is using it:
```
// RUN: clang-tidy -checks='-*,mytest*' --list-checks -load 
%llvmshlibdir/CTTestTidyModule%pluginext -load 
%llvmshlibdir/LLVMHello%pluginext | FileCheck --check-prefix=CHECK-LIST %s
```
I *think* this is testing that we can load a clang-tidy plugin and an llvm 
plugin at the same time and not hit conflicting symbols or other issues. If I'm 
correct, then I think that's useful test functionality, but I wouldn't describe 
it as critical, so I'd be fine if we dropped it for now to get this patch in, 
and then added the extra testing in a subsequent patch if we think it's 
necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Tom Stellard via Phabricator via cfe-commits]

tstellar added inline comments.



Comment at: clang-tools-extra/test/CMakeLists.txt:91
+  if(TARGET CTTestTidyModule)
+  list(APPEND CLANG_TOOLS_TEST_DEPS CTTestTidyModule LLVMHello)
+  target_include_directories(CTTestTidyModule PUBLIC BEFORE 
"${CLANG_TOOLS_SOURCE_DIR}")

Our stand-alone builds for Fedora are still not working after this patch, even 
with D119199.  Why is it necesary to add LLVMHello as a test dependency?  Which 
test is using it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Cristian Adam via Phabricator via cfe-commits]

cristian.adam requested changes to this revision.
cristian.adam added a comment.

I had on Windows two uses cases that failed due to this patch.

1. MSVC2019 with `-D LLVM_EXPORT_SYMBOLS_FOR_PLUGINS=ON`

  FAILED: bin/CTTestTidyModule.dll 
  cmd.exe /C "cd . && C:\tools\cmake\bin\cmake.exe -E vs_link_dll 
--intdir=tools\clang\tools\extra\test\CMakeFiles\CTTestTidyModule.dir 
--rc=C:\PROGRA~2\WI3CF2~1\10\bin\100183~1.0\x64\rc.exe 
--mt=C:\PROGRA~2\WI3CF2~1\10\bin\100183~1.0\x64\mt.exe --manifests  -- 
C:\PROGRA~2\MICROS~1\2019\BUILDT~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\link.exe
 /nologo 
tools\clang\tools\extra\test\CMakeFiles\CTTestTidyModule.dir\clang-tidy\CTTestTidyModule.cpp.obj
  /out:bin\CTTestTidyModule.dll /implib:lib\CTTestTidyModule.lib 
/pdb:bin\CTTestTidyModule.pdb /dll /version:0.0 /machine:x64 /INCREMENTAL:NO  
lib\clang-tidy.lib  kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib 
ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib  && cd ."
  LINK: command 
"C:\PROGRA~2\MICROS~1\2019\BUILDT~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\link.exe
 /nologo 
tools\clang\tools\extra\test\CMakeFiles\CTTestTidyModule.dir\clang-tidy\CTTestTidyModule.cpp.obj
 /out:bin\CTTestTidyModule.dll /implib:lib\CTTestTidyModule.lib 
/pdb:bin\CTTestTidyModule.pdb /dll /version:0.0 /machine:x64 /INCREMENTAL:NO 
lib\clang-tidy.lib kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib 
ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib /MANIFEST 
/MANIFESTFILE:bin\CTTestTidyModule.dll.manifest" failed (exit code 1120) with 
the following output:
  CTTestTidyModule.cpp.obj : error LNK2001: unresolved external symbol "class 
clang::ast_matchers::internal::VariadicDynCastAllOfMatcher const 
clang::ast_matchers::translationUnitDecl" 
(?translationUnitDecl@ast_matchers@clang@@3V?$VariadicDynCastAllOfMatcher@VDecl@clang@@VTranslationUnitDecl@2@@internal@12@B)
  bin\CTTestTidyModule.dll : fatal error LNK1120: 1 unresolved externals



2. MinGW 11.2.0 with `-D LLVM_BUILD_LLVM_DYLIB=ON` `-D LLVM_LINK_LLVM_DYLIB=ON` 
`-D CLANG_LINK_CLANG_DYLIB=ON`

  FAILED: bin/CTTestTidyModule.dll 
  cmd.exe /C "cd . && C:\mingw64\bin\g++.exe -Wa,-mbig-obj -Werror=date-time 
-Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual 
-Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough 
-Wno-maybe-uninitialized -Wno-class-memaccess -Wno-redundant-move 
-Wno-pessimizing-move -Wno-noexcept-type -Wdelete-non-virtual-dtor 
-Wsuggest-override -Wno-comment -Wmisleading-indentation -ffunction-sections 
-fdata-sections -fno-common -Woverloaded-virtual -fno-strict-aliasing  -O2 
-DNDEBUG  -Wl,--gc-sections -shared -o bin\CTTestTidyModule.dll 
-Wl,--major-image-version,0,--minor-image-version,0 
tools/clang/tools/extra/test/CMakeFiles/CTTestTidyModule.dir/clang-tidy/CTTestTidyModule.cpp.obj
  lib/libLLVM-14.dll.a  -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 
-lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 && cd ."
  
C:/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/11.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe:
 
tools/clang/tools/extra/test/CMakeFiles/CTTestTidyModule.dir/clang-tidy/CTTestTidyModule.cpp.obj:CTTestTidyModule.cpp:(.text$_ZNK5clang12ast_matchers8internal30matcher_hasDecayedType0Matcher7matchesERKNS_11DecayedTypeEPNS1_14ASTMatchFinderEPNS1_21BoundNodesTreeBuilderE[_ZNK5clang12ast_matchers8internal30matcher_hasDecayedType0Matcher7matchesERKNS_11DecayedTypeEPNS1_14ASTMatchFinderEPNS1_21BoundNodesTreeBuilderE]+0x1f):
 undefined reference to 
`clang::ast_matchers::internal::DynTypedMatcher::matches(clang::DynTypedNode 
const&, clang::ast_matchers::internal::ASTMatchFinder*, 
clang::ast_matchers::internal::BoundNodesTreeBuilder*) const'
  ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash accepted this revision.
vtjnash added a comment.

Fixed by D119199 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

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

mgorny added a comment.

In D00#3302396 , @vtjnash wrote:

> It is a somewhat worthless test IMO, and might belong better in LLVM itself 
> (where this functionality is defined), but there does not appear to be any 
> other like it currently, and it was requested by a previous reviewer. 
> Comparing to the code in LLVMTestingSupport, does this fix it for you:

Yes, this patch fixes it for me. Thank you! When you push it, please also 
request a backport to 14.x.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash added a comment.

Ah, this looks annoying: there are apparently two flags, CLANG_PLUGIN_SUPPORT 
and LLVM_ENABLE_PLUGINS, but the existing build for clang uses 
CLANG_PLUGIN_SUPPORT to turn off the build support and LLVM_ENABLE_PLUGINS to 
turn off the tests (you might not have noticed this existing issue since you 
turned off CLANG_ENABLE_STATIC_ANALYZER support, and CLANG_BUILD_EXAMPLES is 
off by default, which looks like it would disable all of the existing tests for 
this functionality). This might fix this particular test?

  diff --git a/clang-tools-extra/test/CMakeLists.txt 
b/clang-tools-extra/test/CMakeLists.txt
  index 9321457ae1a3..c98ec90a179b 100644
  --- a/clang-tools-extra/test/CMakeLists.txt
  +++ b/clang-tools-extra/test/CMakeLists.txt
  @@ -17,7 +17,7 @@ string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} 
CLANG_TOOLS_DIR ${LLVM_RUN
   
   llvm_canonicalize_cmake_booleans(
 CLANG_TIDY_ENABLE_STATIC_ANALYZER
  -  LLVM_ENABLE_PLUGINS
  +  CLANG_PLUGIN_SUPPORT
 LLVM_INSTALL_TOOLCHAIN_ONLY
 )
   
  diff --git a/clang-tools-extra/test/lit.site.cfg.py.in 
b/clang-tools-extra/test/lit.site.cfg.py.in
  index e7db0e2ef2cb..d30e6664816b 100644
  --- a/clang-tools-extra/test/lit.site.cfg.py.in
  +++ b/clang-tools-extra/test/lit.site.cfg.py.in
  @@ -12,7 +12,7 @@ config.clang_libs_dir = "@SHLIBDIR@"
   config.python_executable = "@Python3_EXECUTABLE@"
   config.target_triple = "@TARGET_TRIPLE@"
   config.clang_tidy_staticanalyzer = @CLANG_TIDY_ENABLE_STATIC_ANALYZER@
  -config.has_plugins = @LLVM_ENABLE_PLUGINS@ & ~@LLVM_INSTALL_TOOLCHAIN_ONLY@
  +config.has_plugins = @CLANG_PLUGIN_SUPPORT@ & ~@LLVM_INSTALL_TOOLCHAIN_ONLY@
   
   # Support substitution of the tools and libs dirs with user parameters. This 
is
   # used when we can't determine the tool dir at configuration time.

but it means there are a few other places that need to be changed also to make 
your configuration work fully.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This is still breaking tests on our bots: 
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket/8822909450036830977/+/u/package_clang/stdout?format=raw

  FAIL: Clang Tools :: clang-tidy/CTTestTidyModule.cpp (17913 of 89667)
    TEST 'Clang Tools :: clang-tidy/CTTestTidyModule.cpp' 
FAILED 
   Script:
   --
   : 'RUN: at line 2';   clang-tidy -checks='-*,mytest*' --list-checks -load 
/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/./lib/CTTestTidyModule.so
 -load 
/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/./lib/LLVMHello.so
 | FileCheck --check-prefix=CHECK-LIST 
/b/s/w/ir/cache/builder/src/third_party/llvm/clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
   : 'RUN: at line 6';   clang-tidy 
-checks='-*,mytest*,misc-definitions-in-headers' -load 
/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/./lib/CTTestTidyModule.so
 /dev/null -- -xc 2>&1 | FileCheck 
/b/s/w/ir/cache/builder/src/third_party/llvm/clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
   --
   Exit Code: 2
   
   Command Output (stderr):
   --
   Error opening 
'/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/./lib/CTTestTidyModule.so':
 
/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/./lib/CTTestTidyModule.so:
 undefined symbol: _ZN5clang4tidy15ClangTidyModule16getModuleOptionsEv
 -load request ignored.
   Error opening 
'/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/./lib/LLVMHello.so':
 
/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/./lib/LLVMHello.so:
 undefined symbol: _ZN4llvm4PassD2Ev
 -load request ignored.
   No checks enabled.
   FileCheck error: '' is empty.
   FileCheck command line:  FileCheck --check-prefix=CHECK-LIST 
/b/s/w/ir/cache/builder/src/third_party/llvm/clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
   
   --
   
   
   Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
   
   Failed Tests (1):
 Clang Tools :: clang-tidy/CTTestTidyModule.cpp

The cmake invocation looks something like this:

  Running cmake -GNinja -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=OFF 
'-DLLVM_ENABLE_PROJECTS=clang;compiler-rt;lld;clang-tools-extra' 
'-DLLVM_TARGETS_TO_BUILD=AArch64;ARM;Mips;PowerPC;SystemZ;WebAssembly;X86' 
-DLLVM_ENABLE_PIC=ON -DLLVM_ENABLE_UNWIND_TABLES=OFF -DLLVM_ENABLE_TERMINFO=OFF 
-DLLVM_ENABLE_Z3_SOLVER=OFF -DCLANG_PLUGIN_SUPPORT=OFF 
-DCLANG_ENABLE_STATIC_ANALYZER=OFF -DCLANG_ENABLE_ARCMT=OFF 
'-DBUG_REPORT_URL=https://crbug.com and run 
tools/clang/scripts/process_crashreports.py (only works inside Google) which 
will upload a report' -DLLVM_INCLUDE_GO_TESTS=OFF 
-DENABLE_X86_RELAX_RELOCATIONS=NO -DLLVM_ENABLE_DIA_SDK=OFF 
'-DCOMPILER_RT_SANITIZERS_TO_BUILD=asan;dfsan;msan;hwasan;tsan;cfi' 
-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF -DLLVM_ENABLE_CURL=OFF 
-DLLVM_LOCAL_RPATH=/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc-10.2.0-trusty/lib64
 
'-DCOMPILER_RT_TEST_COMPILER_CFLAGS=--gcc-toolchain=/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc-10.2.0-trusty
 
-Wl,-rpath,/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc-10.2.0-trusty/lib64
 
-Wl,-rpath,/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc-10.2.0-trusty/lib32'
 -DLLVM_ENABLE_LIBXML2=FORCE_ON 
-DLIBXML2_INCLUDE_DIR=/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/libxml2-v2.9.12/build/install/include/libxml2
 
-DLIBXML2_LIBRARIES=/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/libxml2-v2.9.12/build/install/lib/libxml2.a
 -DLLVM_ENABLE_LLD=ON 
-DCMAKE_C_COMPILER=/b/s/w/ir/cache/builder/src/third_party/llvm-bootstrap-install/bin/clang
 
-DCMAKE_CXX_COMPILER=/b/s/w/ir/cache/builder/src/third_party/llvm-bootstrap-install/bin/clang++
 -DCOMPILER_RT_BUILD_CRT=ON -DCOMPILER_RT_BUILD_LIBFUZZER=OFF 
-DCOMPILER_RT_BUILD_MEMPROF=OFF -DCOMPILER_RT_BUILD_ORC=OFF 
-DCOMPILER_RT_BUILD_PROFILE=ON -DCOMPILER_RT_BUILD_SANITIZERS=ON 
-DCOMPILER_RT_BUILD_XRAY=OFF -DCOMPILER_RT_BUILD_BUILTINS=ON 
'-DCMAKE_C_FLAGS=-DLIBXML_STATIC 
--gcc-toolchain=/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc-10.2.0-trusty'
 '-DCMAKE_CXX_FLAGS=-DLIBXML_STATIC 
--gcc-toolchain=/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc-10.2.0-trusty'
 -DCMAKE_EXE_LINKER_FLAGS= -DCMAKE_SHARED_LINKER_FLAGS= 
-DCMAKE_MODULE_LINKER_FLAGS= 
-DCMAKE_INSTALL_PREFIX=/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts
 -DLLVM_EXTERNAL_PROJECTS=chrometools 
-DLLVM_EXTERNAL_CHROMETOOLS_SOURCE_DIR=/b/s/w/ir/cache/builder/src/tools/clang 
'-DCHROMIUM_TOOLS=blink_gc_plugin;plugins;translation_unit' 
-DLLVM_PROFDATA_FILE=/b/s/w/ir/cache/builder/src/third_party/llvm-instrumented/profdata.prof
 -DLLVM_ENABLE_LTO=Thin -DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-unknown-linux-gnu 
-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON 

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash added a comment.

It is a somewhat worthless test IMO, and might belong better in LLVM itself 
(where this functionality is defined), but there does not appear to be any 
other like it currently, and it was requested by a previous reviewer. Comparing 
to the code in LLVMTestingSupport, does this fix it for you:

  diff --git a/clang-tools-extra/test/CMakeLists.txt 
b/clang-tools-extra/test/CMakeLists.txt
  index 9321457ae1a3..17cc12473565 100644
  --- a/clang-tools-extra/test/CMakeLists.txt
  +++ b/clang-tools-extra/test/CMakeLists.txt
  @@ -87,6 +87,15 @@ if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
 PLUGIN_TOOL clang-tidy
 DEPENDS clang-tidy-headers)
   
  +  if(CLANG_BUILT_STANDALONE)
  +# LLVMHello library is needed below
  +if (EXISTS ${LLVM_MAIN_SRC_DIR}/lib/Transforms/Hello
  +   AND NOT TARGET LLVMHello)
  +  add_subdirectory(${LLVM_MAIN_SRC_DIR}/lib/Transforms/Hello
  +lib/Transforms/Hello)
  +endif()
  +  endif()
  +
 if(TARGET CTTestTidyModule)
 list(APPEND CLANG_TOOLS_TEST_DEPS CTTestTidyModule LLVMHello)
 target_include_directories(CTTestTidyModule PUBLIC BEFORE 
"${CLANG_TOOLS_SOURCE_DIR}")


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

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

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

This breaks build of clang against system-installed LLVM:

  CMake Error at /usr/lib/llvm/14/lib64/cmake/llvm/AddLLVM.cmake:1821 
(add_dependencies):
The dependency target "LLVMHello" of target "check-all" does not exist.
  Call Stack (most recent call first):
CMakeLists.txt:574 (add_lit_target)

`LLVMHello` isn't installed, so you can't rely on it being available. The 
customary way to resolve this kind of issue would be to check whether the 
target is present, and build it via `add_subdirectory()` against the respective 
LLVM directory (see how LLVMTestingSupport is handled in unittests). That said, 
you will also probably need to account for different build path (it will land 
in clang's shlibdir).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash closed this revision.
vtjnash added a comment.

Closed by rG84f137a590e7de25c4105303e5938c40566c2dfb 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash updated this revision to Diff 405105.
vtjnash added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

Files:
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/Contributing.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/CMakeLists.txt
  clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
  clang-tools-extra/test/lit.cfg.py
  clang-tools-extra/test/lit.site.cfg.py.in

Index: clang-tools-extra/test/lit.site.cfg.py.in
===
--- clang-tools-extra/test/lit.site.cfg.py.in
+++ clang-tools-extra/test/lit.site.cfg.py.in
@@ -4,6 +4,7 @@
 
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
 config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_plugin_ext = "@LLVM_PLUGIN_EXT@"
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.clang_tools_binary_dir = "@CLANG_TOOLS_BINARY_DIR@"
 config.clang_tools_dir = "@CLANG_TOOLS_DIR@"
@@ -11,6 +12,7 @@
 config.python_executable = "@Python3_EXECUTABLE@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.clang_tidy_staticanalyzer = @CLANG_TIDY_ENABLE_STATIC_ANALYZER@
+config.has_plugins = @LLVM_ENABLE_PLUGINS@ & ~@LLVM_INSTALL_TOOLCHAIN_ONLY@
 
 # Support substitution of the tools and libs dirs with user parameters. This is
 # used when we can't determine the tool dir at configuration time.
Index: clang-tools-extra/test/lit.cfg.py
===
--- clang-tools-extra/test/lit.cfg.py
+++ clang-tools-extra/test/lit.cfg.py
@@ -149,3 +149,9 @@
  "clangd", "benchmarks")
 config.substitutions.append(('%clangd-benchmark-dir',
  '%s' % (clangd_benchmarks_dir)))
+config.substitutions.append(('%llvmshlibdir', config.clang_libs_dir))
+config.substitutions.append(('%pluginext', config.llvm_plugin_ext))
+
+# Plugins (loadable modules)
+if config.has_plugins and config.llvm_plugin_ext:
+config.available_features.add('plugins')
Index: clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
@@ -0,0 +1,66 @@
+// REQUIRES: plugins
+// RUN: clang-tidy -checks='-*,mytest*' --list-checks -load %llvmshlibdir/CTTestTidyModule%pluginext -load %llvmshlibdir/LLVMHello%pluginext | FileCheck --check-prefix=CHECK-LIST %s
+// CHECK-LIST: Enabled checks:
+// CHECK-LIST-NEXT:mytest1
+// CHECK-LIST-NEXT:mytest2
+// RUN: clang-tidy -checks='-*,mytest*,misc-definitions-in-headers' -load %llvmshlibdir/CTTestTidyModule%pluginext /dev/null -- -xc 2>&1 | FileCheck %s
+// CHECK: 3 warnings generated.
+// CHECK-NEXT: warning: mytest success [misc-definitions-in-headers,mytest1,mytest2]
+
+#include "clang-tidy/ClangTidy.h"
+#include "clang-tidy/ClangTidyCheck.h"
+#include "clang-tidy/ClangTidyModule.h"
+#include "clang-tidy/ClangTidyModuleRegistry.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang;
+using namespace clang::tidy;
+using namespace clang::ast_matchers;
+
+namespace {
+class MyTestCheck : public ClangTidyCheck {
+
+public:
+  MyTestCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override {
+Finder->addMatcher(translationUnitDecl().bind("tu"), this);
+  }
+
+  void check(const ast_matchers::MatchFinder::MatchResult ) override {
+auto S = Result.Nodes.getNodeAs("tu");
+if (S)
+  diag("mytest success");
+  }
+
+private:
+};
+
+class CTTestModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck("mytest1");
+CheckFactories.registerCheck("mytest2");
+// intentionally collide with an existing test name, overriding it
+CheckFactories.registerCheck("misc-definitions-in-headers");
+  }
+};
+} // namespace
+
+namespace tidy1 {
+// Register the CTTestTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<::CTTestModule>
+X("mytest-module", "Adds my checks.");
+} // namespace tidy1
+
+namespace tidy2 {
+// intentionally collide with an existing test group name, merging with it
+static ClangTidyModuleRegistry::Add<::CTTestModule>
+X("misc-module", "Adds miscellaneous lint checks.");
+} // namespace tidy2
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the CTTestModule.
+volatile int CTTestModuleAnchorSource = 0;
Index: clang-tools-extra/test/CMakeLists.txt
===
--- 

[PATCH] D111100: enable plugins for clang-tidy

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, I think this approach is worth trying. I agree with you that we're in a 
bit of new territory here regarding the testing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash added a comment.

I decided it made the most sense to me to go with option 3, so this should be 
ready to land again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash updated this revision to Diff 404993.
vtjnash added a comment.

- Reland "enable plugins for clang-tidy"
- fixup! Reland "enable plugins for clang-tidy": Disable the test if the user 
has disabled support for building it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

Files:
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/Contributing.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/CMakeLists.txt
  clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
  clang-tools-extra/test/lit.cfg.py
  clang-tools-extra/test/lit.site.cfg.py.in

Index: clang-tools-extra/test/lit.site.cfg.py.in
===
--- clang-tools-extra/test/lit.site.cfg.py.in
+++ clang-tools-extra/test/lit.site.cfg.py.in
@@ -4,6 +4,7 @@
 
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
 config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_plugin_ext = "@LLVM_PLUGIN_EXT@"
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.clang_tools_binary_dir = "@CLANG_TOOLS_BINARY_DIR@"
 config.clang_tools_dir = "@CLANG_TOOLS_DIR@"
@@ -11,6 +12,7 @@
 config.python_executable = "@Python3_EXECUTABLE@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.clang_tidy_staticanalyzer = @CLANG_TIDY_ENABLE_STATIC_ANALYZER@
+config.has_plugins = @LLVM_ENABLE_PLUGINS@ & ~@LLVM_INSTALL_TOOLCHAIN_ONLY@
 
 # Support substitution of the tools and libs dirs with user parameters. This is
 # used when we can't determine the tool dir at configuration time.
Index: clang-tools-extra/test/lit.cfg.py
===
--- clang-tools-extra/test/lit.cfg.py
+++ clang-tools-extra/test/lit.cfg.py
@@ -149,3 +149,9 @@
  "clangd", "benchmarks")
 config.substitutions.append(('%clangd-benchmark-dir',
  '%s' % (clangd_benchmarks_dir)))
+config.substitutions.append(('%llvmshlibdir', config.clang_libs_dir))
+config.substitutions.append(('%pluginext', config.llvm_plugin_ext))
+
+# Plugins (loadable modules)
+if config.has_plugins and config.llvm_plugin_ext:
+config.available_features.add('plugins')
Index: clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
@@ -0,0 +1,66 @@
+// REQUIRES: plugins
+// RUN: clang-tidy -checks='-*,mytest*' --list-checks -load %llvmshlibdir/CTTestTidyModule%pluginext -load %llvmshlibdir/LLVMHello%pluginext | FileCheck --check-prefix=CHECK-LIST %s
+// CHECK-LIST: Enabled checks:
+// CHECK-LIST-NEXT:mytest1
+// CHECK-LIST-NEXT:mytest2
+// RUN: clang-tidy -checks='-*,mytest*,misc-definitions-in-headers' -load %llvmshlibdir/CTTestTidyModule%pluginext /dev/null -- -xc 2>&1 | FileCheck %s
+// CHECK: 3 warnings generated.
+// CHECK-NEXT: warning: mytest success [misc-definitions-in-headers,mytest1,mytest2]
+
+#include "clang-tidy/ClangTidy.h"
+#include "clang-tidy/ClangTidyCheck.h"
+#include "clang-tidy/ClangTidyModule.h"
+#include "clang-tidy/ClangTidyModuleRegistry.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang;
+using namespace clang::tidy;
+using namespace clang::ast_matchers;
+
+namespace {
+class MyTestCheck : public ClangTidyCheck {
+
+public:
+  MyTestCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override {
+Finder->addMatcher(translationUnitDecl().bind("tu"), this);
+  }
+
+  void check(const ast_matchers::MatchFinder::MatchResult ) override {
+auto S = Result.Nodes.getNodeAs("tu");
+if (S)
+  diag("mytest success");
+  }
+
+private:
+};
+
+class CTTestModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck("mytest1");
+CheckFactories.registerCheck("mytest2");
+// intentionally collide with an existing test name, overriding it
+CheckFactories.registerCheck("misc-definitions-in-headers");
+  }
+};
+} // namespace
+
+namespace tidy1 {
+// Register the CTTestTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<::CTTestModule>
+X("mytest-module", "Adds my checks.");
+} // namespace tidy1
+
+namespace tidy2 {
+// intentionally collide with an existing test group name, merging with it
+static ClangTidyModuleRegistry::Add<::CTTestModule>
+X("misc-module", "Adds miscellaneous lint checks.");
+} // namespace tidy2
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the CTTestModule.
+volatile int CTTestModuleAnchorSource 

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash added a comment.

It looks like this is probably the first time that a test was written for a 
PLUGIN_TOOL, outside of the docs, so we are likely in new territory here :/

There seem to be a couple of paths forward:

1. disable the PLUGIN_TOOL option in cmake if the user has disabled the headers 
needed to make that usable to cmake
2. define the headers target unconditionally (as done by 
https://github.com/llvm/llvm-project/blob/ab3b89855c5318f0009e1f016ffe5b1483507fd0/clang/lib/Headers/CMakeLists.txt#L232)
3. disable the added test if the headers required to use plugins are disabled 
by the user


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

In D00#3282760 , @vtjnash wrote:

> Yes, please push a revert so I can look later. Do you have a link to the 
> buildbot configuration, so I can reproduce that?

I did some debugging and it looks like this failure is due to 
`-DLLVM_INSTALL_TOOLCHAIN_ONLY=ON` in our toolchain build. The 
`clang-tidy-headers` is created conditionally only when 
`LLVM_INSTALL_TOOLCHAIN_ONLY` is `OFF`, see 
https://github.com/llvm/llvm-project/blob/ab3b89855c5318f0009e1f016ffe5b1483507fd0/clang-tools-extra/clang-tidy/CMakeLists.txt#L115.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash added a comment.

Yes, please push a revert so I can look later. Do you have a link to the 
buildbot configuration, so I can reproduce that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

We started seeing CMake error after this change landed:

  CMake Error at cmake/modules/AddLLVM.cmake:683 (add_dependencies):
The dependency target "clang-tidy-headers" of target "CTTestTidyModule"
does not exist.
  Call Stack (most recent call first):
/b/s/w/ir/x/w/llvm-llvm-project/clang-tools-extra/test/CMakeLists.txt:82 
(llvm_add_library)

Would it be possible to revert it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG36892727e4f1: enable plugins for clang-tidy (authored by 
vtjnash).

Changed prior to commit:
  https://reviews.llvm.org/D00?vs=401997=404294#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

Files:
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/Contributing.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/CMakeLists.txt
  clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
  clang-tools-extra/test/lit.cfg.py
  clang-tools-extra/test/lit.site.cfg.py.in

Index: clang-tools-extra/test/lit.site.cfg.py.in
===
--- clang-tools-extra/test/lit.site.cfg.py.in
+++ clang-tools-extra/test/lit.site.cfg.py.in
@@ -4,6 +4,7 @@
 
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
 config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_plugin_ext = "@LLVM_PLUGIN_EXT@"
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.clang_tools_binary_dir = "@CLANG_TOOLS_BINARY_DIR@"
 config.clang_tools_dir = "@CLANG_TOOLS_DIR@"
@@ -11,6 +12,7 @@
 config.python_executable = "@Python3_EXECUTABLE@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.clang_tidy_staticanalyzer = @CLANG_TIDY_ENABLE_STATIC_ANALYZER@
+config.has_plugins = @LLVM_ENABLE_PLUGINS@
 
 # Support substitution of the tools and libs dirs with user parameters. This is
 # used when we can't determine the tool dir at configuration time.
Index: clang-tools-extra/test/lit.cfg.py
===
--- clang-tools-extra/test/lit.cfg.py
+++ clang-tools-extra/test/lit.cfg.py
@@ -149,3 +149,9 @@
  "clangd", "benchmarks")
 config.substitutions.append(('%clangd-benchmark-dir',
  '%s' % (clangd_benchmarks_dir)))
+config.substitutions.append(('%llvmshlibdir', config.clang_libs_dir))
+config.substitutions.append(('%pluginext', config.llvm_plugin_ext))
+
+# Plugins (loadable modules)
+if config.has_plugins and config.llvm_plugin_ext:
+config.available_features.add('plugins')
Index: clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
@@ -0,0 +1,66 @@
+// REQUIRES: plugins
+// RUN: clang-tidy -checks='-*,mytest*' --list-checks -load %llvmshlibdir/CTTestTidyModule%pluginext -load %llvmshlibdir/LLVMHello%pluginext | FileCheck --check-prefix=CHECK-LIST %s
+// CHECK-LIST: Enabled checks:
+// CHECK-LIST-NEXT:mytest1
+// CHECK-LIST-NEXT:mytest2
+// RUN: clang-tidy -checks='-*,mytest*,misc-definitions-in-headers' -load %llvmshlibdir/CTTestTidyModule%pluginext /dev/null -- -xc 2>&1 | FileCheck %s
+// CHECK: 3 warnings generated.
+// CHECK-NEXT: warning: mytest success [misc-definitions-in-headers,mytest1,mytest2]
+
+#include "clang-tidy/ClangTidy.h"
+#include "clang-tidy/ClangTidyCheck.h"
+#include "clang-tidy/ClangTidyModule.h"
+#include "clang-tidy/ClangTidyModuleRegistry.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang;
+using namespace clang::tidy;
+using namespace clang::ast_matchers;
+
+namespace {
+class MyTestCheck : public ClangTidyCheck {
+
+public:
+  MyTestCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override {
+Finder->addMatcher(translationUnitDecl().bind("tu"), this);
+  }
+
+  void check(const ast_matchers::MatchFinder::MatchResult ) override {
+auto S = Result.Nodes.getNodeAs("tu");
+if (S)
+  diag("mytest success");
+  }
+
+private:
+};
+
+class CTTestModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck("mytest1");
+CheckFactories.registerCheck("mytest2");
+// intentionally collide with an existing test name, overriding it
+CheckFactories.registerCheck("misc-definitions-in-headers");
+  }
+};
+} // namespace
+
+namespace tidy1 {
+// Register the CTTestTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<::CTTestModule>
+X("mytest-module", "Adds my checks.");
+} // namespace tidy1
+
+namespace tidy2 {
+// intentionally collide with an existing test group name, merging with it
+static ClangTidyModuleRegistry::Add<::CTTestModule>
+X("misc-module", "Adds miscellaneous lint checks.");
+} // namespace tidy2
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the CTTestModule.
+volatile int CTTestModuleAnchorSource = 0;
Index: 

[PATCH] D111100: enable plugins for clang-tidy

[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.

Thanks for the fixes, this LGTM! However, if @alexfh can sign off as well as 
code owner, I'd appreciate it; this is a pretty big directional change and I 
think he should be on board for it. (If we don't hear back by Fri Jan 28, then 
I think this is okay to land.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash added a comment.

@aaron.ballman I think I incorporated all of your feedback. Is this okay for me 
to merge to main? I would like to get it in before the feature branch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash updated this revision to Diff 401997.
vtjnash added a comment.

address review feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

Files:
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/Contributing.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/CMakeLists.txt
  clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
  clang-tools-extra/test/lit.cfg.py
  clang-tools-extra/test/lit.site.cfg.py.in

Index: clang-tools-extra/test/lit.site.cfg.py.in
===
--- clang-tools-extra/test/lit.site.cfg.py.in
+++ clang-tools-extra/test/lit.site.cfg.py.in
@@ -4,6 +4,7 @@
 
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
 config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_plugin_ext = "@LLVM_PLUGIN_EXT@"
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.clang_tools_binary_dir = "@CLANG_TOOLS_BINARY_DIR@"
 config.clang_tools_dir = "@CLANG_TOOLS_DIR@"
@@ -11,6 +12,7 @@
 config.python_executable = "@Python3_EXECUTABLE@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.clang_tidy_staticanalyzer = @CLANG_TIDY_ENABLE_STATIC_ANALYZER@
+config.has_plugins = @LLVM_ENABLE_PLUGINS@
 
 # Support substitution of the tools and libs dirs with user parameters. This is
 # used when we can't determine the tool dir at configuration time.
Index: clang-tools-extra/test/lit.cfg.py
===
--- clang-tools-extra/test/lit.cfg.py
+++ clang-tools-extra/test/lit.cfg.py
@@ -149,3 +149,9 @@
  "clangd", "benchmarks")
 config.substitutions.append(('%clangd-benchmark-dir',
  '%s' % (clangd_benchmarks_dir)))
+config.substitutions.append(('%llvmshlibdir', config.clang_libs_dir))
+config.substitutions.append(('%pluginext', config.llvm_plugin_ext))
+
+# Plugins (loadable modules)
+if config.has_plugins and config.llvm_plugin_ext:
+config.available_features.add('plugins')
Index: clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
@@ -0,0 +1,66 @@
+// REQUIRES: plugins
+// RUN: clang-tidy -checks='-*,mytest*' --list-checks -load %llvmshlibdir/CTTestTidyModule%pluginext -load %llvmshlibdir/LLVMHello%pluginext | FileCheck --check-prefix=CHECK-LIST %s
+// CHECK-LIST: Enabled checks:
+// CHECK-LIST-NEXT:mytest1
+// CHECK-LIST-NEXT:mytest2
+// RUN: clang-tidy -checks='-*,mytest*,misc-definitions-in-headers' -load %llvmshlibdir/CTTestTidyModule%pluginext /dev/null -- -xc 2>&1 | FileCheck %s
+// CHECK: 3 warnings generated.
+// CHECK-NEXT: warning: mytest success [misc-definitions-in-headers,mytest1,mytest2]
+
+#include "clang-tidy/ClangTidy.h"
+#include "clang-tidy/ClangTidyCheck.h"
+#include "clang-tidy/ClangTidyModule.h"
+#include "clang-tidy/ClangTidyModuleRegistry.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang;
+using namespace clang::tidy;
+using namespace clang::ast_matchers;
+
+namespace {
+class MyTestCheck : public ClangTidyCheck {
+
+public:
+  MyTestCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override {
+Finder->addMatcher(translationUnitDecl().bind("tu"), this);
+  }
+
+  void check(const ast_matchers::MatchFinder::MatchResult ) override {
+auto S = Result.Nodes.getNodeAs("tu");
+if (S)
+  diag("mytest success");
+  }
+
+private:
+};
+
+class CTTestModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck("mytest1");
+CheckFactories.registerCheck("mytest2");
+// intentionally collide with an existing test name, overriding it
+CheckFactories.registerCheck("misc-definitions-in-headers");
+  }
+};
+} // namespace
+
+namespace tidy1 {
+// Register the CTTestTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<::CTTestModule>
+X("mytest-module", "Adds my checks.");
+} // namespace tidy1
+
+namespace tidy2 {
+// intentionally collide with an existing test group name, merging with it
+static ClangTidyModuleRegistry::Add<::CTTestModule>
+X("misc-module", "Adds miscellaneous lint checks.");
+} // namespace tidy2
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the CTTestModule.
+volatile int CTTestModuleAnchorSource = 0;
Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ 

[PATCH] D111100: enable plugins for clang-tidy

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:428
+of our new checks.
+
+.. code-block:: console

We should also document our expectations explicitly regarding things like ABI 
and API stability. e.g., remind users that there is none and so the plugin must 
be compiled against the version of clang-tidy that will be loading the plugin. 
(We should also make sure we're correct about that; I don't know if Clang 13 
and Clang 13.0.1 can share plugins or not.)

Another thing we should document is whether the plugins can or cannot use 
threads or TLS, or other details along those lines that developers need to be 
aware of.



Comment at: clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp:24-25
+
+  //void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  //void check(const ast_matchers::MatchFinder::MatchResult ) override;
+

vtjnash wrote:
> aaron.ballman wrote:
> > Shouldn't these functions be overloaded? We don't need it to be 
> > particularly functional, but the plugin should demonstrate that it works 
> > and can be run by clang-tidy (not just loaded and listed as a check).
> I figured that is guaranteed by the C++ linker, if it can successfully list 
> the check, so I didn't think it seemed essential to test for that also. I put 
> these here mostly just to help anyone copying the file to getting started 
> with adopting it to their use case.
I think it's important to demonstrate that the functionality works, and I think 
it's even more important to ensure that plugin support is maintained as new 
features are added to clang-tidy. "It links" is insufficient to tell us that.

One possible approach to this is to write and maintain a simple check as a 
plugin, then test it using the normal infrastructure (additional command line 
arg to load the plugin notwithstanding). We could either make it a functional 
check that users can optionally use (perhaps the reason it's a plugin is 
because it relies on a third-party library that may not always be available on 
all users' systems), or we could make it a toy check that only exists to test 
tidy features like plugins, plugin config options, etc.

The kinds of things I want to verify work are things like: does the plugin name 
get properly emitted as part of the diagnostics from the check? Are config 
options properly loaded for the plugin? Can a plugin alias a builtin check and 
provides a different set of config options?

We should also verify the tool gracefully handles a plugin that isn't valid 
(e.g., ask to load an arbitrary shared library as a plugin, ensure that tidy 
doesn't crash).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash added a comment.

Thanks! I will work on making those changes




Comment at: clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp:24-25
+
+  //void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  //void check(const ast_matchers::MatchFinder::MatchResult ) override;
+

aaron.ballman wrote:
> Shouldn't these functions be overloaded? We don't need it to be particularly 
> functional, but the plugin should demonstrate that it works and can be run by 
> clang-tidy (not just loaded and listed as a check).
I figured that is guaranteed by the C++ linker, if it can successfully list the 
check, so I didn't think it seemed essential to test for that also. I put these 
here mostly just to help anyone copying the file to getting started with 
adopting it to their use case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:392-394
+  cl::Option *load_opt = cl::getRegisteredOptions().lookup("load");
+  if (load_opt)
+load_opt->addCategory(ClangTidyCategory);





Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:421-424
+If you want to develop this out-of-tree, the steps above are the largely same.
+External to the clang-tidy build system, put all of the new code into a single
+shared library. Build and link it against llvm, while allowing some symbols to
+be undefined during linking, almost exactly as you would define a clang plugin.

I wordsmithed a bit and this should be similar to what you already had. I did 
remove the "while allowing some symbols to be undefined during linking" bit 
because I wasn't certain what that was about.



Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:426-427
+
+Then we can run it by passing `-load` to `clang-tidy`, in addition to the name
+of our new checks.
+





Comment at: clang-tools-extra/docs/clang-tidy/index.rst:223
+ Load the dynamic object ``plugin``. This
+ object should register new static 
analyzer or clang-tidy passes. Once loaded, the object
+ will add new command line options to run





Comment at: clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp:24-25
+
+  //void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  //void check(const ast_matchers::MatchFinder::MatchResult ) override;
+

Shouldn't these functions be overloaded? We don't need it to be particularly 
functional, but the plugin should demonstrate that it works and can be run by 
clang-tidy (not just loaded and listed as a check).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash updated this revision to Diff 392835.
vtjnash added a comment.

  fix cmake


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

Files:
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/Contributing.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/CMakeLists.txt
  clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
  clang-tools-extra/test/lit.cfg.py
  clang-tools-extra/test/lit.site.cfg.py.in

Index: clang-tools-extra/test/lit.site.cfg.py.in
===
--- clang-tools-extra/test/lit.site.cfg.py.in
+++ clang-tools-extra/test/lit.site.cfg.py.in
@@ -4,6 +4,7 @@
 
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
 config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_plugin_ext = "@LLVM_PLUGIN_EXT@"
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.clang_tools_binary_dir = "@CLANG_TOOLS_BINARY_DIR@"
 config.clang_tools_dir = "@CLANG_TOOLS_DIR@"
@@ -11,6 +12,7 @@
 config.python_executable = "@Python3_EXECUTABLE@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.clang_tidy_staticanalyzer = @CLANG_TIDY_ENABLE_STATIC_ANALYZER@
+config.has_plugins = @LLVM_ENABLE_PLUGINS@
 
 # Support substitution of the tools and libs dirs with user parameters. This is
 # used when we can't determine the tool dir at configuration time.
Index: clang-tools-extra/test/lit.cfg.py
===
--- clang-tools-extra/test/lit.cfg.py
+++ clang-tools-extra/test/lit.cfg.py
@@ -149,3 +149,9 @@
  "clangd", "benchmarks")
 config.substitutions.append(('%clangd-benchmark-dir',
  '%s' % (clangd_benchmarks_dir)))
+config.substitutions.append(('%llvmshlibdir', config.clang_libs_dir))
+config.substitutions.append(('%pluginext', config.llvm_plugin_ext))
+
+# Plugins (loadable modules)
+if config.has_plugins and config.llvm_plugin_ext:
+config.available_features.add('plugins')
Index: clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
@@ -0,0 +1,50 @@
+// REQUIRES: plugins
+// RUN: clang-tidy -checks='-*,mytest' --list-checks -load %llvmshlibdir/CTTestTidyModule%pluginext | FileCheck %s
+// CHECK: Enabled checks:
+// CHECK-NEXT:mytest
+
+#include "clang-tidy/ClangTidy.h"
+#include "clang-tidy/ClangTidyCheck.h"
+#include "clang-tidy/ClangTidyModule.h"
+#include "clang-tidy/ClangTidyModuleRegistry.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang;
+using namespace clang::tidy;
+using namespace clang::ast_matchers;
+
+namespace {
+class MyTestCheck : public ClangTidyCheck {
+
+public:
+  MyTestCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+
+  //void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  //void check(const ast_matchers::MatchFinder::MatchResult ) override;
+
+private:
+};
+
+class CTTestModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck("mytest");
+  }
+};
+} // namespace
+
+namespace clang {
+namespace tidy {
+
+// Register the CTTestTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<::CTTestModule>
+X("mytest-module", "Adds my checks.");
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the CTTestModule.
+volatile int CTTestModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -17,6 +17,7 @@
 
 llvm_canonicalize_cmake_booleans(
   CLANG_TIDY_ENABLE_STATIC_ANALYZER
+  LLVM_ENABLE_PLUGINS
   )
 
 configure_lit_site_cfg(
@@ -78,6 +79,22 @@
   endif()
 endforeach()
 
+llvm_add_library(
+CTTestTidyModule
+MODULE clang-tidy/CTTestTidyModule.cpp
+PLUGIN_TOOL clang-tidy
+DEPENDS clang-tidy-headers)
+
+if(TARGET CTTestTidyModule)
+list(APPEND CLANG_TOOLS_TEST_DEPS CTTestTidyModule)
+target_include_directories(CTTestTidyModule PUBLIC BEFORE "${CLANG_TOOLS_SOURCE_DIR}")
+if(LLVM_ENABLE_PLUGINS AND (WIN32 OR CYGWIN))
+  set(LLVM_LINK_COMPONENTS
+Support
+  )
+endif()
+endif()
+
 add_lit_testsuite(check-clang-tools "Running the Clang extra tools' regression tests"
   ${CMAKE_CURRENT_BINARY_DIR}
   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
Index: clang-tools-extra/docs/clang-tidy/index.rst

[PATCH] D111100: enable plugins for clang-tidy

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D00#3162293 , @vtjnash wrote:

> There is clearly some more work to do to get the cmake file to be correct, 
> but was hoping to check this looked like the direction you thought looked 
> right for adding this test, since there isn't an obvious example to follow.

My familiarity with plugins is pretty limited (I'm on Windows, where plugins 
don't work at all), but this looks like a reasonable direction to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash added a comment.

There is clearly some more work to do to get the cmake file to be correct, but 
was hoping to check this looked like the direction you thought looked right for 
adding this test, since there isn't an obvious example to follow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash added a comment.

This is probably just a draft, but please let me know what you think


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash updated this revision to Diff 389658.
vtjnash added a comment.

enable help


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

Files:
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/Contributing.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/CMakeLists.txt
  clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
  clang-tools-extra/test/lit.cfg.py
  clang-tools-extra/test/lit.site.cfg.py.in

Index: clang-tools-extra/test/lit.site.cfg.py.in
===
--- clang-tools-extra/test/lit.site.cfg.py.in
+++ clang-tools-extra/test/lit.site.cfg.py.in
@@ -4,6 +4,7 @@
 
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
 config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_plugin_ext = "@LLVM_PLUGIN_EXT@"
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.clang_tools_binary_dir = "@CLANG_TOOLS_BINARY_DIR@"
 config.clang_tools_dir = "@CLANG_TOOLS_DIR@"
@@ -11,6 +12,7 @@
 config.python_executable = "@Python3_EXECUTABLE@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.clang_tidy_staticanalyzer = @CLANG_TIDY_ENABLE_STATIC_ANALYZER@
+config.has_plugins = @LLVM_ENABLE_PLUGINS@
 
 # Support substitution of the tools and libs dirs with user parameters. This is
 # used when we can't determine the tool dir at configuration time.
Index: clang-tools-extra/test/lit.cfg.py
===
--- clang-tools-extra/test/lit.cfg.py
+++ clang-tools-extra/test/lit.cfg.py
@@ -149,3 +149,9 @@
  "clangd", "benchmarks")
 config.substitutions.append(('%clangd-benchmark-dir',
  '%s' % (clangd_benchmarks_dir)))
+config.substitutions.append(('%llvmshlibdir', config.clang_libs_dir))
+config.substitutions.append(('%pluginext', config.llvm_plugin_ext))
+
+# Plugins (loadable modules)
+if config.has_plugins and config.llvm_plugin_ext:
+config.available_features.add('plugins')
Index: clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
@@ -0,0 +1,50 @@
+// REQUIRES: plugins
+// RUN: clang-tidy -checks='-*,mytest' --list-checks -load %llvmshlibdir/CTTestTidyModule%pluginext | FileCheck %s
+// CHECK: Enabled checks:
+// CHECK-NEXT:mytest
+
+#include "clang-tidy/ClangTidy.h"
+#include "clang-tidy/ClangTidyCheck.h"
+#include "clang-tidy/ClangTidyModule.h"
+#include "clang-tidy/ClangTidyModuleRegistry.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang;
+using namespace clang::tidy;
+using namespace clang::ast_matchers;
+
+namespace {
+class MyTestCheck : public ClangTidyCheck {
+
+public:
+  MyTestCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+
+  //void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  //void check(const ast_matchers::MatchFinder::MatchResult ) override;
+
+private:
+};
+
+class CTTestModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck("mytest");
+  }
+};
+} // namespace
+
+namespace clang {
+namespace tidy {
+
+// Register the CTTestTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<::CTTestModule>
+X("mytest-module", "Adds my checks.");
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the CTTestModule.
+volatile int CTTestModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -17,6 +17,7 @@
 
 llvm_canonicalize_cmake_booleans(
   CLANG_TIDY_ENABLE_STATIC_ANALYZER
+  LLVM_ENABLE_PLUGINS
   )
 
 configure_lit_site_cfg(
@@ -89,3 +90,17 @@
 add_lit_testsuites(CLANG-EXTRA ${CMAKE_CURRENT_SOURCE_DIR}
   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
   )
+
+llvm_add_library(
+CTTestTidyModule
+MODULE clang-tidy/CTTestTidyModule.cpp
+PLUGIN_TOOL clang-tidy
+DEPENDS clang-tidy-headers)
+
+target_include_directories(CTTestTidyModule PUBLIC BEFORE "${CLANG_TOOLS_SOURCE_DIR}")
+
+if(LLVM_ENABLE_PLUGINS AND (WIN32 OR CYGWIN))
+  set(LLVM_LINK_COMPONENTS
+Support
+  )
+endif()
Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -218,6 +218,14 @@
 --list-checks  -
   

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash updated this revision to Diff 389656.
vtjnash added a comment.

oops


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

Files:
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/Contributing.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/CMakeLists.txt
  clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
  clang-tools-extra/test/lit.cfg.py
  clang-tools-extra/test/lit.site.cfg.py.in

Index: clang-tools-extra/test/lit.site.cfg.py.in
===
--- clang-tools-extra/test/lit.site.cfg.py.in
+++ clang-tools-extra/test/lit.site.cfg.py.in
@@ -4,6 +4,7 @@
 
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
 config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_plugin_ext = "@LLVM_PLUGIN_EXT@"
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.clang_tools_binary_dir = "@CLANG_TOOLS_BINARY_DIR@"
 config.clang_tools_dir = "@CLANG_TOOLS_DIR@"
@@ -11,6 +12,7 @@
 config.python_executable = "@Python3_EXECUTABLE@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.clang_tidy_staticanalyzer = @CLANG_TIDY_ENABLE_STATIC_ANALYZER@
+config.has_plugins = @LLVM_ENABLE_PLUGINS@
 
 # Support substitution of the tools and libs dirs with user parameters. This is
 # used when we can't determine the tool dir at configuration time.
Index: clang-tools-extra/test/lit.cfg.py
===
--- clang-tools-extra/test/lit.cfg.py
+++ clang-tools-extra/test/lit.cfg.py
@@ -149,3 +149,9 @@
  "clangd", "benchmarks")
 config.substitutions.append(('%clangd-benchmark-dir',
  '%s' % (clangd_benchmarks_dir)))
+config.substitutions.append(('%llvmshlibdir', config.clang_libs_dir))
+config.substitutions.append(('%pluginext', config.llvm_plugin_ext))
+
+# Plugins (loadable modules)
+if config.has_plugins and config.llvm_plugin_ext:
+config.available_features.add('plugins')
Index: clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
@@ -0,0 +1,50 @@
+// REQUIRES: plugins
+// RUN: clang-tidy -checks='-*,mytest' --list-checks -load %llvmshlibdir/CTTestTidyModule%pluginext | FileCheck %s
+// CHECK: Enabled checks:
+// CHECK-NEXT:mytest
+
+#include "clang-tidy/ClangTidy.h"
+#include "clang-tidy/ClangTidyCheck.h"
+#include "clang-tidy/ClangTidyModule.h"
+#include "clang-tidy/ClangTidyModuleRegistry.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang;
+using namespace clang::tidy;
+using namespace clang::ast_matchers;
+
+namespace {
+class MyTestCheck : public ClangTidyCheck {
+
+public:
+  MyTestCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+
+  //void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  //void check(const ast_matchers::MatchFinder::MatchResult ) override;
+
+private:
+};
+
+class CTTestModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck("mytest");
+  }
+};
+} // namespace
+
+namespace clang {
+namespace tidy {
+
+// Register the CTTestTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<::CTTestModule>
+X("mytest-module", "Adds my checks.");
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the CTTestModule.
+volatile int CTTestModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -17,6 +17,7 @@
 
 llvm_canonicalize_cmake_booleans(
   CLANG_TIDY_ENABLE_STATIC_ANALYZER
+  LLVM_ENABLE_PLUGINS
   )
 
 configure_lit_site_cfg(
@@ -89,3 +90,17 @@
 add_lit_testsuites(CLANG-EXTRA ${CMAKE_CURRENT_SOURCE_DIR}
   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
   )
+
+llvm_add_library(
+CTTestTidyModule
+MODULE clang-tidy/CTTestTidyModule.cpp
+PLUGIN_TOOL clang-tidy
+DEPENDS clang-tidy-headers)
+
+target_include_directories(CTTestTidyModule PUBLIC BEFORE "${CLANG_TOOLS_SOURCE_DIR}")
+
+if(LLVM_ENABLE_PLUGINS AND (WIN32 OR CYGWIN))
+  set(LLVM_LINK_COMPONENTS
+Support
+  )
+endif()
Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -218,6 +218,14 @@
 --list-checks  -
  

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash updated this revision to Diff 389655.
vtjnash added a comment.

oops


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

Files:
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/Contributing.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/CMakeLists.txt
  clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
  clang-tools-extra/test/lit.cfg.py
  clang-tools-extra/test/lit.site.cfg.py.in

Index: clang-tools-extra/test/lit.site.cfg.py.in
===
--- clang-tools-extra/test/lit.site.cfg.py.in
+++ clang-tools-extra/test/lit.site.cfg.py.in
@@ -4,6 +4,7 @@
 
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
 config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_plugin_ext = "@LLVM_PLUGIN_EXT@"
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.clang_tools_binary_dir = "@CLANG_TOOLS_BINARY_DIR@"
 config.clang_tools_dir = "@CLANG_TOOLS_DIR@"
@@ -11,6 +12,7 @@
 config.python_executable = "@Python3_EXECUTABLE@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.clang_tidy_staticanalyzer = @CLANG_TIDY_ENABLE_STATIC_ANALYZER@
+config.has_plugins = @LLVM_ENABLE_PLUGINS@
 
 # Support substitution of the tools and libs dirs with user parameters. This is
 # used when we can't determine the tool dir at configuration time.
Index: clang-tools-extra/test/lit.cfg.py
===
--- clang-tools-extra/test/lit.cfg.py
+++ clang-tools-extra/test/lit.cfg.py
@@ -149,3 +149,9 @@
  "clangd", "benchmarks")
 config.substitutions.append(('%clangd-benchmark-dir',
  '%s' % (clangd_benchmarks_dir)))
+config.substitutions.append(('%llvmshlibdir', config.clang_libs_dir))
+config.substitutions.append(('%pluginext', config.llvm_plugin_ext))
+
+# Plugins (loadable modules)
+if config.has_plugins and config.llvm_plugin_ext:
+config.available_features.add('plugins')
Index: clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
@@ -0,0 +1,50 @@
+// REQUIRES: plugins
+// RUN: clang-tidy -checks='-*,mytest' --list-checks -load %llvmshlibdir/CTTestTidyModule%pluginext | FileCheck %s
+// CHECK: Enabled checks:
+// CHECK-NEXT:mytest
+
+#include "clang-tidy/ClangTidy.h"
+#include "clang-tidy/ClangTidyCheck.h"
+#include "clang-tidy/ClangTidyModule.h"
+#include "clang-tidy/ClangTidyModuleRegistry.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang;
+using namespace clang::tidy;
+using namespace clang::ast_matchers;
+
+namespace {
+class MyTestCheck : public ClangTidyCheck {
+
+public:
+  MyTestCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+
+  //void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  //void check(const ast_matchers::MatchFinder::MatchResult ) override;
+
+private:
+};
+
+class CTTestModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck("mytest");
+  }
+};
+} // namespace
+
+namespace clang {
+namespace tidy {
+
+// Register the CTTestTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<::CTTestModule>
+X("mytest-module", "Adds my checks.");
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the CTTestModule.
+volatile int CTTestModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -17,6 +17,7 @@
 
 llvm_canonicalize_cmake_booleans(
   CLANG_TIDY_ENABLE_STATIC_ANALYZER
+  LLVM_ENABLE_PLUGINS
   )
 
 configure_lit_site_cfg(
@@ -89,3 +90,17 @@
 add_lit_testsuites(CLANG-EXTRA ${CMAKE_CURRENT_SOURCE_DIR}
   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
   )
+
+llvm_add_library(
+CTTestTidyModule
+MODULE clang-tidy/CTTestTidyModule.cpp
+PLUGIN_TOOL clang-tidy
+DEPENDS clang-tidy-headers)
+
+target_include_directories(CTTestTidyModule PUBLIC BEFORE "${CLANG_TOOLS_SOURCE_DIR}")
+
+if(LLVM_ENABLE_PLUGINS AND (WIN32 OR CYGWIN))
+  set(LLVM_LINK_COMPONENTS
+Support
+  )
+endif()
Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -218,6 +218,14 @@
 --list-checks  -
  List all enabled checks and exit. Use with
  

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash updated this revision to Diff 389653.
vtjnash added a comment.

add release notes, docs mention, and test example


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

Files:
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/Contributing.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/CMakeLists.txt
  clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
  clang-tools-extra/test/lit.cfg.py
  clang-tools-extra/test/lit.site.cfg.py.in

Index: clang-tools-extra/test/lit.site.cfg.py.in
===
--- clang-tools-extra/test/lit.site.cfg.py.in
+++ clang-tools-extra/test/lit.site.cfg.py.in
@@ -4,6 +4,7 @@
 
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
 config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_plugin_ext = "@LLVM_PLUGIN_EXT@"
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.clang_tools_binary_dir = "@CLANG_TOOLS_BINARY_DIR@"
 config.clang_tools_dir = "@CLANG_TOOLS_DIR@"
@@ -11,6 +12,7 @@
 config.python_executable = "@Python3_EXECUTABLE@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.clang_tidy_staticanalyzer = @CLANG_TIDY_ENABLE_STATIC_ANALYZER@
+config.has_plugins = @LLVM_ENABLE_PLUGINS@
 
 # Support substitution of the tools and libs dirs with user parameters. This is
 # used when we can't determine the tool dir at configuration time.
Index: clang-tools-extra/test/lit.cfg.py
===
--- clang-tools-extra/test/lit.cfg.py
+++ clang-tools-extra/test/lit.cfg.py
@@ -149,3 +149,5 @@
  "clangd", "benchmarks")
 config.substitutions.append(('%clangd-benchmark-dir',
  '%s' % (clangd_benchmarks_dir)))
+config.substitutions.append(('%llvmshlibdir', config.clang_libs_dir))
+config.substitutions.append(('%pluginext', config.llvm_plugin_ext))
Index: clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
@@ -0,0 +1,50 @@
+// REQUIRES: plugins
+// RUN: clang-tidy -checks='-*,mytest' --list-checks -load %llvmshlibdir/CTTestTidyModule%pluginext | FileCheck %s
+// CHECK: Enabled checks:
+// CHECK-NEXT:mytest
+
+#include "clang-tidy/ClangTidy.h"
+#include "clang-tidy/ClangTidyCheck.h"
+#include "clang-tidy/ClangTidyModule.h"
+#include "clang-tidy/ClangTidyModuleRegistry.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang;
+using namespace clang::tidy;
+using namespace clang::ast_matchers;
+
+namespace {
+class MyTestCheck : public ClangTidyCheck {
+
+public:
+  MyTestCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+
+  //void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  //void check(const ast_matchers::MatchFinder::MatchResult ) override;
+
+private:
+};
+
+class CTTestModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck("mytest");
+  }
+};
+} // namespace
+
+namespace clang {
+namespace tidy {
+
+// Register the CTTestTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<::CTTestModule>
+X("mytest-module", "Adds my checks.");
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the CTTestModule.
+volatile int CTTestModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -17,6 +17,7 @@
 
 llvm_canonicalize_cmake_booleans(
   CLANG_TIDY_ENABLE_STATIC_ANALYZER
+  LLVM_ENABLE_PLUGINS
   )
 
 configure_lit_site_cfg(
@@ -89,3 +90,17 @@
 add_lit_testsuites(CLANG-EXTRA ${CMAKE_CURRENT_SOURCE_DIR}
   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
   )
+
+llvm_add_library(
+CTTestTidyModule
+MODULE clang-tidy/CTTestTidyModule.cpp
+PLUGIN_TOOL clang-tidy
+DEPENDS clang-tidy-headers)
+
+target_include_directories(CTTestTidyModule PUBLIC BEFORE "${CLANG_TOOLS_SOURCE_DIR}")
+
+if(LLVM_ENABLE_PLUGINS AND (WIN32 OR CYGWIN))
+  set(LLVM_LINK_COMPONENTS
+Support
+  )
+endif()
Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -218,6 +218,14 @@
 --list-checks  -
  List all enabled checks and exit. Use with
 

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash updated this revision to Diff 389652.
vtjnash added a comment.
Herald added a subscriber: arphaman.

add release notes, docs mention, and test example


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

Files:
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/Contributing.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/CMakeLists.txt
  clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
  clang-tools-extra/test/lit.cfg.py
  clang-tools-extra/test/lit.site.cfg.py.in

Index: clang-tools-extra/test/lit.site.cfg.py.in
===
--- clang-tools-extra/test/lit.site.cfg.py.in
+++ clang-tools-extra/test/lit.site.cfg.py.in
@@ -4,6 +4,7 @@
 
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
 config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_plugin_ext = "@LLVM_PLUGIN_EXT@"
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.clang_tools_binary_dir = "@CLANG_TOOLS_BINARY_DIR@"
 config.clang_tools_dir = "@CLANG_TOOLS_DIR@"
@@ -11,6 +12,7 @@
 config.python_executable = "@Python3_EXECUTABLE@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.clang_tidy_staticanalyzer = @CLANG_TIDY_ENABLE_STATIC_ANALYZER@
+config.has_plugins = @LLVM_ENABLE_PLUGINS@
 
 # Support substitution of the tools and libs dirs with user parameters. This is
 # used when we can't determine the tool dir at configuration time.
Index: clang-tools-extra/test/lit.cfg.py
===
--- clang-tools-extra/test/lit.cfg.py
+++ clang-tools-extra/test/lit.cfg.py
@@ -149,3 +149,5 @@
  "clangd", "benchmarks")
 config.substitutions.append(('%clangd-benchmark-dir',
  '%s' % (clangd_benchmarks_dir)))
+config.substitutions.append(('%llvmshlibdir', config.clang_libs_dir))
+config.substitutions.append(('%pluginext', config.llvm_plugin_ext))
Index: clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/CTTestTidyModule.cpp
@@ -0,0 +1,50 @@
+// REQUIRES: plugins
+// RUN: clang-tidy -checks='-*,mytest' --list-checks -load %llvmshlibdir/CTTestTidyModule%pluginext | FileCheck %s
+// CHECK: Enabled checks:
+// CHECK-NEXT:mytest
+
+#include "clang-tidy/ClangTidy.h"
+#include "clang-tidy/ClangTidyCheck.h"
+#include "clang-tidy/ClangTidyModule.h"
+#include "clang-tidy/ClangTidyModuleRegistry.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang;
+using namespace clang::tidy;
+using namespace clang::ast_matchers;
+
+namespace {
+class MyTestCheck : public ClangTidyCheck {
+
+public:
+  MyTestCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+
+  //void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  //void check(const ast_matchers::MatchFinder::MatchResult ) override;
+
+private:
+};
+
+class CTTestModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck("mytest");
+  }
+};
+} // namespace
+
+namespace clang {
+namespace tidy {
+
+// Register the CTTestTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<::CTTestModule>
+X("mytest-module", "Adds my checks.");
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the CTTestModule.
+volatile int CTTestModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -17,6 +17,7 @@
 
 llvm_canonicalize_cmake_booleans(
   CLANG_TIDY_ENABLE_STATIC_ANALYZER
+  LLVM_ENABLE_PLUGINS
   )
 
 configure_lit_site_cfg(
@@ -89,3 +90,17 @@
 add_lit_testsuites(CLANG-EXTRA ${CMAKE_CURRENT_SOURCE_DIR}
   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
   )
+
+llvm_add_library(
+CTTestTidyModule
+MODULE clang-tidy/CTTestTidyModule.cpp
+PLUGIN_TOOL clang-tidy
+DEPENDS clang-tidy-headers)
+
+target_include_directories(CTTestTidyModule PUBLIC BEFORE "${CLANG_TOOLS_SOURCE_DIR}")
+
+if(LLVM_ENABLE_PLUGINS AND (WIN32 OR CYGWIN))
+  set(LLVM_LINK_COMPONENTS
+Support
+  )
+endif()
Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -218,6 +218,14 @@
 --list-checks  -
  List all enabled checks and exit. Use with
  -checks=* to list all available checks.
+

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash added a comment.

Yes, this header does everything


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

Wait... Is the diff //really// supposed to be this small? You didn't include 
code... that one header takes care of **everything**?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman edited reviewers, added: aaron.ballman, whisperity, hokein; 
removed: sergio.martins, iamsergio, chgans, nocnokneo, kfunk, alexfh_, segoon.
aaron.ballman requested changes to this revision.
aaron.ballman added a comment.
This revision now requires changes to proceed.
Herald added a subscriber: rnkovacs.

Modifying the reviewer list a bit (feel free to re-add yourself if you were 
dropped but still wanted to provide review).

Thank you for working on this! Previously, the only way to get a clang-tidy 
plugin was to use a clang static analyzer plugin and run it through clang-tidy, 
which is a lot of hoops to jump through. However, that may raise an interesting 
question -- how well does this functionality work when there are static 
analyzer and tidy plugins (I'm presuming it'll run them both)?

At a minimum, I think these changes require test coverage. Also, the release 
notes should be updated to remark on the new functionality, documentation 
should be added that explains this is supported, and we may also want to 
consider adding an in-tree example (clang-tidy doesn't have an `examples` 
folder like Clang does, but this seems like a good reason for us to add one). 
But I think this is the correct direction to do -- this is functionality I've 
wished I had several times in the past.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash added a comment.

bump? tagging some more people who seemed like possible reviewers. I realized 
my original list of candidates might have mostly been inactive people.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash added a comment.

bump?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash added a comment.

bump?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D111100: enable plugins for clang-tidy

[Jameson Nash via Phabricator via cfe-commits]

vtjnash created this revision.
Herald added a subscriber: mgorny.
vtjnash requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Fixes 32739


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D00

Files:
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp


Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -20,6 +20,7 @@
 #include "../GlobList.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/PluginLoader.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
Index: clang-tools-extra/clang-tidy/tool/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/tool/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/tool/CMakeLists.txt
@@ -29,11 +29,17 @@
   clangToolingCore
   )
 
+# Support plugins.
+if(CLANG_PLUGIN_SUPPORT)
+  set(support_plugins SUPPORT_PLUGINS)
+endif()
+
 add_clang_tool(clang-tidy
   ClangTidyToolMain.cpp
-  )
-add_dependencies(clang-tidy
+
+  DEPENDS
   clang-resource-headers
+  ${support_plugins}
   )
 clang_target_link_libraries(clang-tidy
   PRIVATE
@@ -50,6 +56,9 @@
   ${ALL_CLANG_TIDY_CHECKS}
   )
 
+if(CLANG_PLUGIN_SUPPORT)
+  export_executable_symbols_for_plugins(clang-tidy)
+endif()
 
 install(PROGRAMS clang-tidy-diff.py
   DESTINATION share/clang


Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -20,6 +20,7 @@
 #include "../GlobList.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/PluginLoader.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
Index: clang-tools-extra/clang-tidy/tool/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/tool/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/tool/CMakeLists.txt
@@ -29,11 +29,17 @@
   clangToolingCore
   )
 
+# Support plugins.
+if(CLANG_PLUGIN_SUPPORT)
+  set(support_plugins SUPPORT_PLUGINS)
+endif()
+
 add_clang_tool(clang-tidy
   ClangTidyToolMain.cpp
-  )
-add_dependencies(clang-tidy
+
+  DEPENDS
   clang-resource-headers
+  ${support_plugins}
   )
 clang_target_link_libraries(clang-tidy
   PRIVATE
@@ -50,6 +56,9 @@
   ${ALL_CLANG_TIDY_CHECKS}
   )
 
+if(CLANG_PLUGIN_SUPPORT)
+  export_executable_symbols_for_plugins(clang-tidy)
+endif()
 
 install(PROGRAMS clang-tidy-diff.py
   DESTINATION share/clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits