[PATCH] D128204: [clangd] Add fix-it for inserting IWYU pragma: keep

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 21 06:54:32 PDT 2022


sammccall added a comment.

This is a bit tricky - we'd like to offer more fixes in several scenarios (e.g. replace a header with the transitive includes you're relying on), but some of those require more complex implementation.

On one hand, presenting only one automated fix will bias users towards that option.
On the other hand, it's not clear when the more complex fixes will be ready and this solves a real problem.

I'm inclined toward landing it, but will leave this up to @kadircet



================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:502
     // including them directly instead.
     // FIXME(kirillbobyrev): Add fix suggestion for adding IWYU pragmas
     // (keep/export) remove the warning once we support IWYU pragmas.
----------------
this FIXME is addressed by this change


================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:511
+    D.Fixes.emplace_back();
+    D.Fixes.back().Message = "add pragma keep directive";
+    D.Fixes.back().Edits.emplace_back();
----------------
"directive" isn't accurate here I think

What about just: "add IWYU pragma: keep"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128204



More information about the cfe-commits mailing list