dexonsmith added a comment. In https://reviews.llvm.org/D32724#750074, @vsk wrote:
> 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.) Ah, interesting. That does seem ideal. > Should we diagnose sanitizer feature mismatches in ASTReader > (checkLanguageOptions) as warnings, errors, or not at all? Either warning or error seems fine with me; only compiler engineers are going to see this anyway (because implicit users are protected by the hash). As long as it's overridable... there's a -cc1 flag to allow configuration mismatches, right? Once the flag can be changed after the fact (i.e., in a post-Richard's-plan world), we should remove the error/warning, and possibly do something like: always build the module without sanitizers. https://reviews.llvm.org/D32724 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
