Private or Public headers for non-exported symbols?

We’ve recently completed a long-term project to annotate all the publicly exported symbols in the libLLVM.so shared object with explicit visibility attributes. I would like to take the next step in this project and start removing public symbols from the shared object that aren’t really useful to library consumers (e.g. GlobalISel, SelectionDAG, etc.)

This leads me to the question of how to approach this. Should we leave all the headers with non-exported symbols in the public include directory (include/llvm) or should we move them somewhere else? Either the same directory as their .cpp implementation files or a shared include/llvm-private directory in the source tree?

The only reason I see to have non-public headers in include/llvm is just for build system convenience, but I could be missing something. Having them in another directory will make it easier for tooling we have that checks for missing visibility attributes on symbols, because we won’t need to have it scan these other directories to make sure that new symbols have the correct visibility attributes.

Having a single include/llvm-private directory would simplify the build system, since we could set this include directory as a global property, but it would mean we have 3 locations for headers depending on how they are used:

  • include/llvm for headers with public symobls
  • include/llvm-private for headers with non-public symbols that are used by multiple different llvm components.
  • $CPP_DIRECTORY for headers with non-public symbols that aren’t used outside of their own component.

What’s the best approach here?

cc @Steelskin

I strongly feel like symbols that are not meant to be exported should not be in the public headers under include/llvm, since they “pollute” the headers and become available for every consumer.

This is specifically about LLVM modules that are meant to be used by other LLVM modules, but not by “external” LLVM consumers. I favor using the same $CPP_DIRECTORY approach that we have been using for module-internal headers, but I do not feel very strongly on the matter. Either way, I think the important part is to not have them include/llvm.

On your main point, I don’t have a strong opinion, but:

Before removing SDAG/GISel headers, we should clarify whether building out-of-tree targets is supported with a distributed LLVM and/or the dylib (again something I don’t have an opinion on); moving these headers into lib/ will make building out-of-tree targets more difficult. Generally, I think removing a component from the public API deserves an RFC, if only to raise awareness and confirm consensus.

I don’t think any out of tree targets are building as an out of tree project. I assume they all just fork the whole project and insert the backend alongside the upstream targets.