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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
