[PATCH] D112707: [clangd] IncludeCleaner: Be more conservative in marking RecordDecl usage

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:51
+      Result.insert(Definition ? Definition->getLocation()
+                               : RD->getMostRecentDecl()->getLocation());
+      return true;
----------------
i think this might turn a direct dependency into a transitive one, e.g. you got forward declarations of `struct Foo;` in a.h and b.h, then c.h includes b.h. In the main file you might have includes for a.h and c.h. Now the most recent redecl happens through c.h hence a.h will be marked as unused, even though it's the one header providing the forward decl directly.

what about just rolling with the definition when it's visible and handling the forward-decl in main file case inside the `add` ? i suppose that's something we'd want for all decls and not just records? it implies passing in main file id and a sourcemanager into the crawler, and inside the `add` before going over all redecls, we just check if most recent decl falls into the main file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112707