[PATCH] D32724: [Modules] Include the enabled sanitizers in the module hash

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

In https://reviews.llvm.org/D32724#749868, @dexonsmith wrote:

> In https://reviews.llvm.org/D32724#747728, @aprantl wrote:
>
> > Is it the right solution to use the module hash for correctness, or should the mismatch of the serialized langopts trigger a module rebuild and the module hash is only there to tune the performance/disk size tradeoff?
>
>
> I'm not sure if there is (or should be) a hard-and-fast rule, but certainly in this case changing the hash SGTM.  Otherwise, users toggling back and forth between build configurations would have to rebuild the modules each time.


Great, it looks like changing the module hash is acceptable. However, I'm not sure whether it's OK to make sanitizer feature mismatches errors for explicit modules. @aprantl suggests checking for language option mismatches early on instead of just relying on the module hash, but @rsmith mentioned:

> I would expect this [sanitizer features] to be permitted to differ between an explicit module build and its use. (Ideally we would apply the sanitization settings from the module build to the code generated for its inline functions in that case, but that can wait.)

Should we diagnose sanitizer feature mismatches in ASTReader (checkLanguageOptions) as warnings, errors, or not at all?


https://reviews.llvm.org/D32724