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 the -dunique-ids and -dno-unique-ids options to the compilers #1618

Merged
merged 1 commit into from Feb 20, 2018

Conversation

Projects
None yet
4 participants
@shindere
Copy link
Contributor

shindere commented Feb 19, 2018

These options allow to control whether identifiers are made unique by
appending a stamp to them when dumping intermediate representations or not.

The default is to print the stamp, as is done currently.

The "-no-unique-ids" option is useful e.g. to simplify the comparison
between a produced intermediate reprsentation (-dlambda, say) and the
expected one, in the context of the testsuite, for instance.

This is a follow-up to a discussion started on caml-devel. On the list,
@damiendoligez suggested "making it a variant of -dlambda (for example,
-dlambda-unnumbered)" (because the original motivation is to simplify the
tests on the lambda code), but then it seemed to me the implementation would
be both simpler and more straightforward if the feature of making
identifiers unique would be kept orthogonal to the intermediate
reprresentation.

@lpw25

This comment has been minimized.

Copy link
Contributor

lpw25 commented Feb 19, 2018

Since this only affects -d... options might I suggest -dunique-ids and -dno-unique-ids to make it more obvious that it is not for normal users.

@shindere shindere force-pushed the shindere:unique-ids branch from 4563b75 to 2ef85b2 Feb 19, 2018

@shindere

This comment has been minimized.

Copy link
Contributor Author

shindere commented Feb 19, 2018

@shindere shindere force-pushed the shindere:unique-ids branch from 2ef85b2 to 2526ce5 Feb 19, 2018

@shindere shindere changed the title Add the -unique-ids and -no-unique-ids options to the compilers Add the -dunique-ids and -dno-unique-ids options to the compilers Feb 19, 2018

@shindere

This comment has been minimized.

Copy link
Contributor Author

shindere commented Feb 19, 2018

@lpw25 I updated the code to rename the options according to your suggestion.

@damiendoligez
Copy link
Member

damiendoligez left a comment

Looks good except for an indentation change that I suggested directly to Sebastien.

Add the -dunique-ids and -dno-unique-ids options to the compilers
These options allow to control whether identifiers are made unique by
appending a stamp to them when dumping intermediate representations or not.

The default is to print the stamp, as is done currently.

The "-dno-unique-ids" option is useful e.g. to simplify the comparison
between a produced intermediate reprsentation (-dlambda, say) and the
expected one, in the context of the testsuite, for instance.

@shindere shindere force-pushed the shindere:unique-ids branch from 2526ce5 to fa70194 Feb 20, 2018

@shindere shindere merged commit fa70194 into ocaml:trunk Feb 20, 2018

0 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details

@shindere shindere deleted the shindere:unique-ids branch Feb 20, 2018

@shindere

This comment has been minimized.

Copy link
Contributor Author

shindere commented Feb 20, 2018

Rebased, fixed indentation and merged.

Thanks a lot for the review @damiendoligez

@@ -104,6 +104,9 @@ Working version
- GPR#1554: warnings 52 and 57: fix reference to manual detailed explanation
(Florian Angeletti, review by Thomas Refis and Gabriel Scherer)

- GPR#1618: add the -dno-unique-ids and -dunique-ids compiler flags
(Sébastien Hinderer, review by xxx)

This comment has been minimized.

@gasche

gasche Feb 20, 2018

Member

"review by xxx" would be Leo White and Damien Doligez?

shindere added a commit that referenced this pull request Feb 20, 2018

@shindere

This comment has been minimized.

Copy link
Contributor Author

shindere commented Feb 20, 2018

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.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.