[PATCH] D146406: [IncludeCleaner][clangd] Mark umbrella headers as users of private

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 20 06:15:45 PDT 2023


hokein added inline comments.


================
Comment at: clang-tools-extra/include-cleaner/lib/Analysis.cpp:100
+      // Check if main file is the public interface for a private header. If so
+      // we shouldn't diagnose it as unused.
+      if (auto PHeader = PI->getPublic(I.Resolved); !PHeader.empty()) {
----------------
The beahvior (no showing the diagnostic) seems reasonable to me as we can infer that the included header is supposed to be exported by the main file. Just explore this a bit more.

The sad bit is that we start diverging from the classical IWYU tool (I have check it, for this case, it gives an unused-include unless you add an export pragma).

I think the main cause is that we're missing the `// IWYU pragma: export`, we should still want to recommend users to add the pragma export, right?

My only concern is that without the `export` pragma, whether the include is used can't be reason about by just "reading" the main-file content by human, e.g. for the case below, there is no usage of the `private.h` (previously the trailing `// IWYU pragma: export` comment is considered a usage), we have to open the private.h and find the private mapping to verify the usage.

```
// public.h

#include "private.h"
```

It seems better to provide an `adding //IWYU pragma: export` FIXIT.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146406



More information about the cfe-commits mailing list