This is an archive of the discontinued LLVM Phabricator instance.

Trim unnecessary component/library dependencies.
AbandonedPublic

Authored by alxu on Feb 22 2022, 5:34 PM.

Diff Detail

Event Timeline

alxu created this revision.Feb 22 2022, 5:34 PM
alxu created this object with visibility "alxu (Alex Xu (Hello71))".
alxu created this object with edit policy "alxu (Alex Xu (Hello71))".
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptFeb 22 2022, 5:34 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
alxu requested review of this revision.Feb 22 2022, 5:34 PM
Herald added projects: Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project. · View Herald TranscriptFeb 22 2022, 5:34 PM
alxu planned changes to this revision.Feb 22 2022, 5:39 PM
alxu removed a reviewer: Restricted Project.Feb 22 2022, 6:12 PM
alxu changed the visibility from "alxu (Alex Xu (Hello71))" to "Public (No Login Required)".
alxu changed the edit policy from "alxu (Alex Xu (Hello71))" to "All Users".

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

yota9 removed a subscriber: yota9.Feb 23 2022, 1:23 AM

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.

clang-tools-extra/clangd/CMakeLists.txt
41

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.

clang-tools-extra/clangd/indexer/CMakeLists.txt
11

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

clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
35

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

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?

martong removed a subscriber: martong.May 5 2022, 6:03 AM
Herald added a project: Restricted Project. · View Herald Transcript
alxu abandoned this revision.Aug 5 2022, 3:02 PM