[clang] [Serialization] Downgrade error to warning for inconsistent language flags (PR #117840)
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 27 18:27:50 PST 2024
ChuanqiXu9 wrote:
> > I don't think the discussion here has run its course
> > #115416
>
> Yeah, I had explicitly asked for a review from @Bigcheese before that landed, so I was surprised to see that get merged.
I misread it. I thought it wasn't message to me. It would be clear if you ask me for waiting more time for @Bigcheese
>
> > I don't think we should force users for our too conservative decisions. As I said there are many false positive error messages which doesn't affect the process actually.
>
> False positive _error_ diagnostics? Warnings can have false positives; errors are not allowed to and that suggests we have some serious issues elsewhere to address first.
Yes. This was my point. Maybe I didn't make it clear. All the `LANGOPT` in https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/LangOptions.def now are not allowed to be inconsistent in different TUs when importing. But actually some of these flags won't affect things. We changed some of them, e.g., VisibilityMode (https://github.com/llvm/llvm-project/blob/700d9ac9ef82fa5aa6b2972e8656ab5055a90d15/clang/include/clang/Basic/LangOptions.def#L380-L381). But giving the amount of options, there must be a lot of too-conservative `LANGOPT`.
>
> > I feel it is really wishful thinking to ban this and hope we can be in a more safe world. I think it may only stop more people using modules and change nothing else. Again, please give users a chance to choose what they like.
>
> Maybe I'm misunderstanding something but that sounds like the exact opposite of how we usually operate. We typically do aim for improved safety and if that means people don't use modules as much, that's fine -- it's better for us to be too restrictive and then relax those restrictions in the future than to be too permissive and have to support that forever due to users relying on it.
Maybe the above paragraph can explain it better. The point is false-positive diagnostics. And I fully agree safe is good and ODR violation is bad. I just thought we can transfer the right to the users. People who want it can have `-Werror` or `-Werror=`. People who don't want it can have `-Wno-error=`. And if people use `-Wno-`. They get what they get. I don't think we need to teach users on this topic. And I feel it is not good to drag other users leg by saying "we're considering the safety of the last kind of people".
https://github.com/llvm/llvm-project/pull/117840
More information about the cfe-commits
mailing list