[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