Fix LLVM_ENABLE_MODULES=ON + BUILD_SHARED_LIBS=ON build

[arichardson]

Summary

Using the same approach as d0782a69ff4c04dc4f1dc1c11cb2fb43c6b638f5 and
91cef98d2c41eff0dea474a760d2dce5ba873209.

[merge_guards_bot]

Build result: pass - 60466 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt, CMakeCache.txt

[arphaman]

Is the Tooling/Transformer library used by clang-format? The problem might be solved if the Tooling/Transformer library was its own module (just like Clang_ToolingInclusions).

[arichardson]

In D71026#1769340, @arphaman wrote:

Is the Tooling/Transformer library used by clang-format? The problem might be solved if the Tooling/Transformer library was its own module (just like Clang_ToolingInclusions).

I don't think it's clang-format, I get the failure when looking the ASTDiff library and also for many others.

I tried making it a separate module but it gave me an error due to recursive dependencies from clangtooling.

[arichardson]

In D71026#1769340, @arphaman wrote:

Is the Tooling/Transformer library used by clang-format? The problem might be solved if the Tooling/Transformer library was its own module (just like Clang_ToolingInclusions).

I just tried to create a separate module again. I managed to fix the recursive dep error. However, it still doesn't fix the linker error. There is certainly a better solution than this, but this is all I can come up with since I don't really understand the semantics of modules.

[rsmith]

Please can you describe the problem that this is fixing? Without that, it's hard to know whether this is the appropriate fix. Is there a modules buildbot that's failing, that we could look at the output from?

[arichardson]

If I build on macOS with cmake -GNinja -DBUILD_SHARED_LIBS=ON -DCMAKE_BUILD_TYPE=Debug -DLLVM_ENABLE_PROJECTS=llvm;clang;lld;compiler-rt;libcxx;libcxxabi -DBUILD_SHARED_LIBS=ON -DLLVM_ENABLE_MODULES=ON, I get lots of linker errors building e.g. libclangASTDiff.dylib due to using the internal ASTMatchers header inside the Tooling/Transformer headers.

This is probably due to https://bugs.llvm.org/show_bug.cgi?id=23521 mentioned in the previous commits that added the exclude directives.

[dblaikie]

In D71026#1785025, @arichardson wrote:

If I build on macOS with cmake -GNinja -DBUILD_SHARED_LIBS=ON -DCMAKE_BUILD_TYPE=Debug -DLLVM_ENABLE_PROJECTS=llvm;clang;lld;compiler-rt;libcxx;libcxxabi -DBUILD_SHARED_LIBS=ON -DLLVM_ENABLE_MODULES=ON, I get lots of linker errors building e.g. libclangASTDiff.dylib due to using the internal ASTMatchers header inside the Tooling/Transformer headers.

This is probably due to https://bugs.llvm.org/show_bug.cgi?id=23521 mentioned in the previous commits that added the exclude directives.

Could we instead workaround this by adhering to the LLVM style guide's "requirement" (it's clearly not universally applied) have strong vtables? https://llvm.org/docs/CodingStandards.html#provide-a-virtual-method-anchor-for-classes-in-headers that way this would come up less often, by the sounds of it?

[arichardson]

I believe this is no longer necessary.

clang/include/clang/module.modulemap

 
 module Clang_Tooling {
   requires cplusplus umbrella "Tooling" module * { export * }
   // FIXME: Exclude these headers to avoid pulling all of the AST matchers
   // library into clang-format. Due to inline key functions in the headers,
   // importing the AST matchers library gives a link dependency on the AST
   // matchers (and thus the AST), which clang-format should not have.
   exclude header "Tooling/RefactoringCallbacks.h"
+  exclude header "Tooling/Transformer/MatchConsumer.h"
+  exclude header "Tooling/Transformer/RewriteRule.h"
+  exclude header "Tooling/Transformer/RangeSelector.h"
+  exclude header "Tooling/Transformer/Stencil.h"
+  exclude header "Tooling/Transformer/Transformer.h"
 }
 
 module Clang_ToolingCore {
   requires cplusplus
   umbrella "Tooling/Core" module * { export * }
 }
 
 module Clang_ToolingInclusions {
   requires cplusplus
   umbrella "Tooling/Inclusions" module * { export * }
 }