[PATCH] D157395: [include-cleaner] Follow `IWYU pragma: export` links transitively.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

i think we should also have some unittests in FindHeadersTest, some sample scenarios:

foo.h

  #pragma once
  void foo();

export1.h

  #pragma once
  // IWYU pragma: private, include "public1.h"
  #include "foo.h" // IWYU pragma: export

export2.h

  #pragma once
  // IWYU pragma: private, include "public2.h"
  #include "export2.h" // IWYU pragma: export

export3.h

  #pragma once
  // IWYU pragma: private, include "public3.h"
  #include "export3.h" // IWYU pragma: export
  void foo(); // we also have a declaration here, hence this is another provider root.

user.cc

  #include "export3.h"
  void bar() { foo(); }

`headersForFoo()` should look like `foo.h, public3.h,  export1.h, export2.h, export3.h`



================
Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:195
+    std::set<const FileEntry *> Exporters;
     while (FE) {
       Results.emplace_back(FE,
----------------
what about writing this as:
```
std::queue<const FileEntry*> Exporters;
while (FE) {
     Results.emplace_back(FE,
                           isPublicHeader(FE, *PI) |
                               (IsOrigin ? Hints::OriginHeader : Hints::None));
      for (const auto *Export : PI->getExporters(FE, SM.getFileManager()))
        Exporters.push(Export);

      if (auto Verbatim = PI->getPublic(FE); !Verbatim.empty()) {
        Results.emplace_back(Verbatim,
                             Hints::PublicHeader | Hints::PreferredHeader);
        break;
      }
      if (PI->isSelfContained(FE) || FID == SM.getMainFileID())
        break;

      // Walkup the include stack for non self-contained headers.
      FID = SM.getDecomposedIncludedLoc(FID).first;
      FE = SM.getFileEntryForID(FID);
      IsOrigin = false;
}
// Now traverse provider trees rooted at exporters.
// Note that we only traverse export edges, and ignore private -> public mappings,
// as those pragmas apply to exporter, and not the main provider being exported in
// this header.
std::set<const FileEntry*> SeenExports;
while (!Exporters.empty()) {
  auto *Export = Exporters.front();
  Exporters.pop();
  if (!SeenExports.insert(Export).second) // In case of cyclic exports
     continue;
  Results.emplace_back(Export, isPublicHeader(Export, *PI));
  for (const auto *Export : PI->getExporters(FE, SM.getFileManager()))
     Exporters.push(Export);
}
```

That way we don't need to change the contracts around `getExporter` in `PragmaIncludes`


================
Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:204
 
       if (auto Verbatim = PI->getPublic(FE); !Verbatim.empty()) {
+        Hints Hint = Hints::PublicHeader;
----------------
`FE` here is always a "provider" for the symbol in question, so we should actually respect the private pragmas in them. we should only ignore those pragmas if they're discovered inside a header, that we found because we followed an export edge.


================
Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:207
+        auto FE =
+                expectedToOptional(SM.getFileManager().getFileRef(Verbatim));
+        if (!FE || std::find(Exporters.begin(), Exporters.end(), FE) ==
----------------
verbatim spellings are relative to an include search path (or even worse, relative to the file containing the include directive) so we can't rely on being able to resolve it directly through FileManager (it can only resolve either absolute paths nor paths relative to CWD).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157395