vsapsai wrote:
> I'd suggest keeping this PR and the resulting commit free of any formatting
> changes and then commit the results from `clang-format` as a separate commit.
Will do that.
> After #101280 I had the impression that we're trying to be very precise about
> reporting whether an AST file is a precompiled header, module file, or other.
> Why doesn't this PR implement that? To be clear this change is already a big
> improvement as is, but I'd like to understand if further polish and
> unification is still planned.
I was precise in #101280 because it was easy to do, not because I was trying
hard. The main reason I didn't use a similar approach in this PR is because
ModuleKind isn't always available. For example, the diagnostic can be reached
through a call stack like
```
ASTReader::isAcceptableASTFile
ASTReader::readASTFileControlBlock
ASTReader::ReadOptionsBlock
ASTReader::ParseLanguageOptions
ASTReaderListener::ReadLanguageOptions
checkLanguageOptions
```
and my understanding is that `isAcceptableASTFile` doesn't know the module kind
by design. There are ways to deal with it, of course. I had in mind passing
`std::optional<ModuleKind>` alongside `StringRef Filename` everywhere but don't
think the extra complexity is worth the resulting precision.
As far as I remember, module kind is decided during a module file creation and
we don't support interpreting the same .pcm file as a different kind (cannot
use PCH as a preamble, for instance). That's why I think emitting a module kind
in a diagnostic isn't particularly valuable. Over time that can change but
right now I'm not planning a further polish.
https://github.com/llvm/llvm-project/pull/101413
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
