hokein marked an inline comment as done.
hokein added inline comments.
================
Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:39
FileID FID = SM.getFileID(Loc.physical());
- const auto *FE = SM.getFileEntryForID(FID);
+ const auto *FE = getResponsibleHeader(FID, SM, PI);
if (!FE)
----------------
kadircet wrote:
> i am not sure if sequencing of this and rest of the IWYU pragma handling is
> the right actually.
>
> e.g. consider a scenario in which:
> private.h:
> ```
> // IWYU private, include "public.h"
> void foo();
> ```
> private-impl.h:
> ```
> #pragma once
> #include "private.h"
> ```
>
> we'll actually now use `private-impl.h` as provider for `foo` rather than
> `public.h`, a similar argument goes for export pragma. i think intent of the
> developer is a lot more explicit when we have these pragmas around, so we
> should prioritize them and then pick the self contained one, if it's still
> needed. WDYT?
It looks reasonable to me. Thanks for raising this case, this is a good point.
I mostly considered the case where the self-contained file has the IWYU private
mapping.
My current thought is:
Overall algorithm: we check the original `FE` to see whether we can get any
IWYU mapping (private, export) headers, if there is any, we use these mapping
headers as final results; if not, we use the `selfContainedHeader(FE)` and its
IWYU mapping headers;
Below are the cases I considered -- 1), 2), 4) works fine, but the 3) is not
perfect (as we prioritize the IWYU mapping headers), but probably ok.
```
// Case1: Private
// findHeaders("private1.inc") => "path/public1.h"
Inputs.ExtraFiles["private1.h"] = R"cpp(
#pragma once
#include "private1.inc"
)cpp");
Inputs.ExtraFiles["private1.inc"] = R"cpp(
// IWYU pragma: private, include "path/public1.h"
)cpp";
// Case2: Private
// findHeaders("private2.inc") => "path/public2.h"
Inputs.ExtraFiles["private2.h"] = R"cpp(
#pragma once
// IWYU pragma: private, include "path/public2.h"
#include "private2.inc"
)cpp";
Inputs.ExtraFiles["private2.inc"] = "";
// Case3: Export
// findHeaders("details1.inc") => "details1.inc", "export1.h"
Inputs.ExtraFiles["export1.h"] = R"cpp(
#include "details1.inc" // IWYU pragma: export
)cpp";
Inputs.ExtraFiles["details1.inc"] = "";
// Case4: Export
// findHeaders("details2.inc") => "export2.h", "export2_internal.h"
Inputs.ExtraFiles["export2.h"] =R"cpp(
#pragma once
#include "export2_internal.h" // IWYU pragma: export
)cpp";
Inputs.ExtraFiles["export2_internal.h"] = R"cpp(
#pragma once
#include "details2.inc"
)cpp";
Inputs.ExtraFiles["details2.inc"] = "";
```
Let me know what you think about it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137698/new/
https://reviews.llvm.org/D137698
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
