[PATCH] D125468: [clangd] Include Cleaner: ignore headers with IWYU export pragmas

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Great! A few more nits



================
Comment at: clang-tools-extra/clangd/Headers.cpp:151
+      // will know that the next inclusion is behind the IWYU pragma.
+      if (!Text.consume_front(IWYUPragmaExport) &&
+          !Text.consume_front(IWYUPragmaKeep))
----------------
you never use Text again, so consume_front is unneccesary and confusing here, we can just use starts_with


================
Comment at: clang-tools-extra/clangd/Headers.cpp:151
+      // will know that the next inclusion is behind the IWYU pragma.
+      if (!Text.consume_front(IWYUPragmaExport) &&
+          !Text.consume_front(IWYUPragmaKeep))
----------------
sammccall wrote:
> you never use Text again, so consume_front is unneccesary and confusing here, we can just use starts_with
add a FIXME that begin_export is not handled here (now that the other branch supports it)


================
Comment at: clang-tools-extra/clangd/Headers.h:148
 
+  bool hasIWYUPragmas(HeaderID ID) const {
+    return HasIWYUPragmas.contains(ID);
----------------
hasIWYUExport?
(if the file contains non-export pragmas we need to return false)


================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:274
+  // FIXME: Ignore the headers with IWYU export pragmas for now, remove this
+  // check when we have actual support for export pragmas.
+  if (AST.getIncludeStructure().hasIWYUPragmas(HID))
----------------
actual support for export pragmas => more precise tracking of exported headers?

(current wording doesn't really say what's missing)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125468