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
> > >
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 an
dexonsmith added a comment.
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
spyffe added inline comments.
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 aff
On 1 May 2017 at 16:43, Vedant Kumar via Phabricator <
revi...@reviews.llvm.org> wrote:
> vsk created this revision.
>
> When building with implicit modules it's possible to hit a scenario
> where modules are built without -fsanitize=address, and are subsequently
> imported into CUs with -fsanitiz
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 '-DF
aprantl added a comment.
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?
https://reviews.llvm.org/D32724
spyffe requested changes to this revision.
spyffe added a comment.
This revision now requires changes to proceed.
A few minor nits, but the operation of ignoring certain sanitizer flags seems
to be happening in the wrong place.
Comment at: lib/Basic/LangOptions.cpp:32
- //
vsk updated this revision to Diff 98030.
vsk edited the summary of this revision.
vsk added a comment.
- Exclude sanitizers which cannot affect AST generation from the module hash.
- Improve the test to check modules are actually rebuilt when we expect them to
be rebuilt, and not rebuilt otherwis
vsk created this revision.
When building with implicit modules it's possible to hit a scenario
where modules are built without -fsanitize=address, and are subsequently
imported into CUs with -fsanitize=address enabled. This can cause
strange failures at runtime. One case I've seen affects libcxx,
10 matches
Mail list logo