Details
- Reviewers
bollu JDevlieghere andreadb alexander-shaposhnikov rupprecht jdoerfert jhenderson nicolasvasilache herhut aartbik MaskRay awarzynski bondhugula rafauler Amir maksfb ThomasRaoux NoQ ributzka dcaballe - Group Reviewers
Restricted Project
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
Please expand the description and include how you figured this out and how do we know it is actually correct.
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?
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.