[clang] [Serialization] Downgrade error to warning for inconsistent language flags (PR #117840)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

> > I see, so because we no longer allow inconsistent options when importing, we now issue a lot of error diagnostics. But for many of the options, differences between TUs should have no impact on modules anyway, and so those errors are false positives. Is that about correct?
> 
> Yes.
> 
> > If I'm right, then what about a different way forward: update the individual options to specify whether it is an error, a warning, or silently fine for the option to be inconsistent across module boundaries, the emit diagnostics (or not) as appropriate for each option. It's more up-front work, but it means we stop issue false positive errors without losing diagnostic fidelity. (Note, we may find we want to reorganize Options.td so it's easier for us to specify "this entire block of options are ones for which module inconsistency is an error".)
> 
> The problem may be about the amount of work. Although I am not sure how far we can make it in practice, it is technically doable.
> 
> Some other thoughts about false-positive error diagnostics. I am wondering, if we don't want false-positive error diagnostics at all, then we have to do what I proposed in this PR (to downgrade errors to warnings for inconsistent flags). Otherwise, we can always construct at least one example that it is false-positive to diagnose the inconsistency. e.g.,
> 
> ```
> /// m1.cppm
> export module m1;
> // eof
> 
> // m2.cpp
> import m1;
> ```
> 
> then is it really matter to compile `m1.cppm` with `-std=c++23` and import it to `m2.cpp` which is compiled as `-std=c++20`? I don't think so as far as I know. So is it a false-positive error diagnostic? I think yes. And `-std=` may be the most impactful options, if we can construct cases for `-std=`, then we can construct cases for other flags too. Then, if we can't tolerant false positive error diagnostics, we should emit warnings instead for all the flags.

To overcome this, we need to finer-grained analysis in Sema (or ASTReader on AST really) to emit errors to emit the true positive error message.

A simple true positive error example is,

```
/// m1.cppm
export module m1;
export template <class T>
T func() {
#if __cplusplus >= 202303L
   return T(123);
#else
   return T(456);
#endif
}

int a() {
   return func<int>();
}

// m2.cpp
import m1;
int b() {
    return func<int>();
}
```

then if we compile `m1.cppm` with `-std=c++23` but import the BMI compiled with `-std=c++23` to `m2.cpp` which is compiled with `-std=c++20`, then this is a real ODR violation, and this is the case we should emit an error for sure.

So my conclusion is, if we want to emit error without false-positive diagnostics, the current mechanism is too coarse-grained. The current mechanism only emit diagnostics based on comparing flags. But if we want to avoid false-positive diagnostics really, we must have finer-grained mechanism which can analysis the AST really. And we'd better to avoid to emit  errors in the current mechanism to avoid the false positive error diagnostics.

And my feeling is, we're too overly defensive when hearing this have may increase the ODR violation chances. But from my perspective, primarily from users, I do feel it is not comfortable that it bans too many use cases but not preventing the ODR violations after all. 

(both GCC and MSVC didn't make so strict rules as clang did. And I think this is not designed at all. It is just... what inherits from the era of PCH and we didn't change it)

CC @AaronBallman @cor3ntin @mizvekov @Bigcheese 


https://github.com/llvm/llvm-project/pull/117840