https://github.com/zmodem commented:

Thanks for the PR, and for providing a great test case.

The fix *sounds* plausible, but I'm not an expert on modules (or the 
diagnostics engine).

This boils down to checking both 
`Diag.GetDiagStateForLoc(Loc)->SuppressSystemWarnings` and 
`Diag.GetCurDiagState()->SuppressSystemWarnings`.

It sounds like in the non-modules case, `GetDiagStateForLoc(Loc)` and 
`GetCurDiagState()` are the same here, but in the modules case they are 
different.

Ah, e37391c4fe6d169b7c265a69d0291760dc159b4d made us serialize the diag state 
with the module. So if `Loc` is from a module, `GetDiagStateForLoc(Loc)` is the 
diag state *when the module was built*.

This change seems like it could affect more than the temporary suppression in 
SemaAvailability though.

If the module was built with `-Wno-system-headers` (the default), but then we 
compile code which uses the module with `-Wsystem-headers`, we will no longer 
suppress system-header warnings at code locations inside the module, undoing 
the effect of e37391c4fe6d169b7c265a69d0291760dc159b4d.

I think we need someone with more Clang expertise to look at this. Maybe 
@AaronBallman could look or suggest someone?

https://github.com/llvm/llvm-project/pull/180684
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to