Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incr.comp.: Current serialization logic for Spans messes with fingerprint stability #46059

Closed
michaelwoerister opened this issue Nov 17, 2017 · 4 comments
Labels
A-incr-comp Area: Incremental compilation C-enhancement Category: An issue proposing an enhancement or a PR with one.

Comments

@michaelwoerister
Copy link
Member

At the moment, when we serialize a Span, we drop any macro expansion information, that is, we just keep the lo and hi fields and drop the ctxt field. Consequently, when we load something with a Span from disk (like a piece of MIR, for example), and that Span in there had a non-zero ctxt before being stored to disk, then we get a different Fingerprint because we now have an empty ctxt.

I'm not sure whether this is harmless and could be ignored but it certainly messes with the otherwise very handy -Zincremental-verify-ich feature. I'd prefer if we found a way of making this completely accurate (e.g. by somehow serializing and then restoring Span expansion contexts).

cc @nikomatsakis

@michaelwoerister michaelwoerister added the A-incr-comp Area: Incremental compilation label Nov 17, 2017
@nikomatsakis
Copy link
Contributor

A thought: we could maybe erase the ctxt from spans out of MIR sometime during the optimization process, so that the initial hash also excludes the ctxt.

@michaelwoerister
Copy link
Member Author

Since the same serialization routines are also used when serializing Spans in cached diagnostics -- a case where we don't want to lose any information -- we need to think of a way of restoring the expansion information.

@TimNN TimNN added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Nov 21, 2017
@michaelwoerister
Copy link
Member Author

This is partly addressed by #46301.

@michaelwoerister
Copy link
Member Author

This has been solved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-incr-comp Area: Incremental compilation C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Development

No branches or pull requests

3 participants