[clang] [Modules][Diagnostic] Mention which AST file's options differ from the current TU options. (PR #101413)
Volodymyr Sapsai via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 2 08:24:14 PDT 2024
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
More information about the cfe-commits
mailing list