[PATCH] D135245: [clang][Tooling] Move STL recognizer to its own library

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet marked 2 inline comments as done.
kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/CMakeLists.txt:163
   clangToolingInclusions
+  clangToolingInclusionsSTL
   clangToolingSyntax
----------------
sammccall wrote:
> StandardLibrary or Stdlib?
> 
> STL isn't accurate or consistent with the names in the code.
changing to Stdlib


================
Comment at: clang/lib/Tooling/Inclusions/STL/CMakeLists.txt:2
+add_clang_library(clangToolingInclusionsSTL
+  StandardLibrary.cpp
+
----------------
sammccall wrote:
> This means the implementation files and the header files have a different directory structure, which may be confusing to people trying to work out which library to link against based on the headers they included.
> 
> On the other hand, I think the cascading effect of dependencies => libraries => directory structure => header structure is pretty unfortunate leaking of llvm's sad cmake structure. Up to you
> 
> 
right, i was also torn between moving the headers around vs not. but i finally managed to convince myself that the implementation being in a different subdirectory is actually an unfortunate detail about the way LLVM is build (I didn't want to have PARTIAL_SOURCES_INTENDED, either) and shouldn't matter for the applications that want to use it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135245