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

Add [Clambda.usymbol_provenance] #2088

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
3 participants
@mshinwell
Copy link
Contributor

commented Oct 3, 2018

This pull request adds a new type and two record fields to be used for the tracking of provenance information for symbols, such that they can be referenced (e.g. for printing) by their proper module paths. In my previous working branch these were not filled in for Closure mode but I have sketched an implementation here, which I shall test in due course. The Flambda patch which @chambart is working on fills these fields in properly for Flambda.

The DWARF emitter reads directly from the Clambda constant definition structures, which is why the provenances are not transmitted further. (In the future, particularly with the plan for Flambda to bypass Clambda entirely, we will probably have to change this.)

@mshinwell mshinwell added this to the 4.08 milestone Oct 3, 2018

@damiendoligez

This comment has been minimized.

Copy link
Member

commented Oct 17, 2018

This is only a skeleton, right? The original_idents field is always empty.

@lthls

This comment has been minimized.

Copy link
Contributor

commented Oct 17, 2018

As far as I understand, the non-flambda pass never generates symbols for named constructions: there is the global module, and the constants lifted to symbols during Cmm conversion.
I guess you could use a default ident name for the global module, and try to see if the constants were let-bound, but otherwise this field is mostly meant to be filled by the flambda side.

@damiendoligez
Copy link
Member

left a comment

It is evident that this patch won't break the compiler.

@damiendoligez

This comment has been minimized.

Copy link
Member

commented Jan 7, 2019

@mshinwell can you rebase and merge?

@damiendoligez damiendoligez removed this from the 4.08 milestone Jan 10, 2019

damiendoligez added a commit that referenced this pull request Jan 15, 2019

Merge pull request #2088 from mshinwell/symbol_provenance
Add Clambda.usymbol_provenance
@damiendoligez

This comment has been minimized.

Copy link
Member

commented Jan 15, 2019

rebased and merged by hand (commit 0f86fe2)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.