[PATCH] D120375: Trim unnecessary component/library dependencies.

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

Relying on implicit dependencies doesn't often work very well - please can you 
confirm what kind of builds you have tested this with? Which projects? shared 
libs? build types?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120375

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

[PATCH] D120375: Trim unnecessary component/library dependencies.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Mostly a review of the clangd changes, I am not familiar with all the other 
parts (and not necessarily everyone will be) hence some explicit testing 
results would be great.
Also please upload the patch with full context.




Comment at: clang-tools-extra/clangd/CMakeLists.txt:41
-  Support
-  AllTargetsInfos
-  FrontendOpenMP

clangd actually requires `AllTargetsInfos` so that it can display detailed 
semantic information like bitwidth of certain types. not just on the final 
binary (as I see you've included this on the tool/CMakeLists.txt), there are 
also tests that depend on it.



Comment at: clang-tools-extra/clangd/indexer/CMakeLists.txt:11
   PRIVATE
-  clangAST
   clangBasic

these are all required dependencies of clangd-indexer. i suppose you're getting 
rid of these as they're transitively linked in by clangDeamon, but I remember 
some build configurations (shared lib builds) still requiring these to be 
present at these levels as well. i might be wrong, but it would be better to 
show some explicit testing results claiming these are not causing breakages (as 
Mehdi pointed out).



Comment at: clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt:35
   clangdSupport
-  clangFormat
   clangLex

this might require extra cleanups on the headers of some sources.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120375

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

[PATCH] D120375: Trim unnecessary component/library dependencies.

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

Please expand the description and include how you figured this out and how do 
we know it is actually correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120375

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