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

Handle `[@inlined]` attributes under a module constraint #1808

Merged
merged 6 commits into from Jun 1, 2018

Conversation

Projects
None yet
3 participants
@xclerc
Copy link
Contributor

xclerc commented May 31, 2018

In 4.07/trunk, the following code:

module M = Set.Make [@inlined] (Int32)

triggers a puzzling warning "attribute cannot appear in this context".
This is quite puzzling because the following code does not trigger
the warning:

module M = Set_make [@inlined] (Int32)

@lpw25 helped me understand that an existing bug was indeed revealed
by the introduction of the Stdlib module. The code triggering the warning
goes through a module alias and hence a module constraint, so that the
[@inlined] attribute is not on the outer module expression as expected
but on the inner one.

xclerc added some commits May 31, 2018

@lpw25

lpw25 approved these changes May 31, 2018

Copy link
Contributor

lpw25 left a comment

Looks good to me and fixes an annoying bug. @damiendoligez Can I still backport to 4.07? Seems like the release is incoming and I don't want to get in the way, but I think this probably qualifies as a regression even though really 4.07 just uncovers an existing bug.

@xclerc

This comment has been minimized.

Copy link
Contributor Author

xclerc commented Jun 1, 2018

I have just updated the PR number in the reproduction
case; the PR can be merged (I am unsure whether this
patch deserves an entry in the change log).

@lpw25

This comment has been minimized.

Copy link
Contributor

lpw25 commented Jun 1, 2018

It is possible to hit this bug in 4.06, so there should be a changes entry.

@xclerc

This comment has been minimized.

Copy link
Contributor Author

xclerc commented Jun 1, 2018

Here it is.

@lpw25

This comment has been minimized.

Copy link
Contributor

lpw25 commented Jun 1, 2018

I see other people are still cherry-picking bug fixes into 4.07, so if you move the changelog entry to the 4.07 section then I'll do that now.

@damiendoligez

This comment has been minimized.

Copy link
Member

damiendoligez commented Jun 1, 2018

@lpw25 OK for cherry-picking.

@damiendoligez damiendoligez added the bug label Jun 1, 2018

@damiendoligez damiendoligez added this to the 4.07 milestone Jun 1, 2018

xclerc and others added some commits Jun 1, 2018

@lpw25 lpw25 merged commit 44a5e50 into ocaml:trunk Jun 1, 2018

0 of 2 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

lpw25 added a commit that referenced this pull request Jun 1, 2018

Handle `[@inlined]` attributes under a module constraint (#1808)
Handle `[@inlined]` attributes under a module constraint.
@lpw25

This comment has been minimized.

Copy link
Contributor

lpw25 commented Jun 1, 2018

Merged and cherry-picked onto 4.07 as d0cbd11

@xclerc xclerc deleted the xclerc:fix-module-inlined-attribute branch Jun 1, 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.