vsk added a comment. Offline, @aprantl mentioned a concern about module hashes being required for correctness. I'm not sure whether this is OK or not (@bruno, any thoughts?). AFAICT there are items included in the module hash that, were they removed, would break implicit module builds (e.g '-DFOO'-style preprocessor defs).
================ Comment at: lib/Basic/LangOptions.cpp:32 - // FIXME: This should not be reset; modules can be different with different - // sanitizer options (this affects __has_feature(address_sanitizer) etc). - Sanitize.clear(); + // These options do not affect AST generation. SanitizerBlacklistFiles.clear(); ---------------- spyffe wrote: > I'd replace this with > ``` > Sanitize.clear(SanitizerKind::CFI | SanitizerKind::Integer | > SanitizerKind::Nullability | SanitizerKind::Undefined); > ``` > We know those options don't affect modules, as demonstrated by you clearing > them anyway in CompilerInvocation... I don't think this would work, and don't quite see a way to make it work. The problem is that when importing a module into a CU, the CU's hash is required to match the to-be-imported module's hash [1]. If we clear some sanitizer options in resetNonModularOptions(), then the "matching hashes" check would break, because you can't reset the non-modular options in a CU that you're importing a module into. You'd end up disabling the sanitizers for the CU you're building. [1] CompilerInstance.cpp ``` 1095 assert(ImportingInstance.getInvocation().getModuleHash() == 1096 Invocation->getModuleHash() && "Module hash mismatch!"); ``` ================ Comment at: test/Modules/check-for-sanitizer-feature.cpp:25 +// +// First, built without any sanitization. +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t.2 \ ---------------- spyffe wrote: > Typo: //build// Will fix, pending resolution of the issue you raised above. Same for your next comment about the #error messages. https://reviews.llvm.org/D32724 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits