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

Toplevel let should take attributes. #6677

Closed
vicuna opened this issue Nov 26, 2014 · 7 comments
Closed

Toplevel let should take attributes. #6677

vicuna opened this issue Nov 26, 2014 · 7 comments
Assignees

Comments

@vicuna
Copy link

vicuna commented Nov 26, 2014

Original bug ID: 6677
Reporter: furuse
Assigned to: @alainfrisch
Status: resolved (set by @alainfrisch on 2017-02-24T15:13:02Z)
Resolution: fixed
Priority: normal
Severity: minor
Version: 4.02.1
Fixed in version: 4.03.0
Category: typing
Related to: #6586 #6714
Monitored by: @whitequark @hcarty

Bug description

In 4.02.1, the toplevel let does not accept attributes but only extensions, while the local let accepts the both. Is this asymmetry required?

This is problematic when ppx code generators produce toplevel bindings possibly recursive but not necessarily.

At the local lets we can use [@ocaml.warning "-39"] to stop the warning:

let [@ocaml.warning "-39"] rec x = 1 in x   (* nice! *)

But it seems the same technique is not usable against the toplevel let:

let [@ocaml.warning "-39"] rec x = 1;;
@vicuna
Copy link
Author

vicuna commented Nov 26, 2014

Comment author: furuse

Oh #6586 is technically the same issue.

The motivation here is to control the warning 39 at the toplevel let recs by attributes.

@vicuna
Copy link
Author

vicuna commented Nov 26, 2014

Comment author: furuse

[@@@ocaml.warning "-39"]
let rec x = 1

works but it changes the setting globally...

@vicuna
Copy link
Author

vicuna commented Apr 29, 2015

Comment author: @damiendoligez

This is not exactly the same as #6586: what you need is not a shorthand notation but any way at all of placing an attribute on the whole let.

@vicuna
Copy link
Author

vicuna commented Nov 27, 2015

Comment author: @alainfrisch

Toplevel lets accept attributes. As for any other structure/signature item, the syntax is [@@...] following the item:

let rec x = 1 in x
[@@ocaml.warning "-39"]

Now I don't think that ocaml.warning is interpreted in such context, but this is another story. If this is about generated code, a workaround is to generate:

include struct
[@@@ocaml.warning "-39"]
let rec x = 1 in x
end

@vicuna
Copy link
Author

vicuna commented Mar 12, 2016

Comment author: @whitequark

I think this was fixed in 4.03.

@vicuna
Copy link
Author

vicuna commented Mar 14, 2016

Comment author: @alainfrisch

let [@ocaml.warnings "-39"] rec x = 1;;

This is now allowed syntactically; the attribute is attached to the "value_binding".

But the ocaml.warning attribute is not interpreted on value bindings.
I've pushed a fix ( ddb2826 ) to the specific problem reported here; warning 39 requires a specific treatment anyway since it is a global property of the "let rec" declaration, not local to one of the (potentially) multiple value bindings in it.

It could make sense to control warnings while type-checking each binding, but this would require a different fix anyway. I suggest to keep this ticket open for this part.

@vicuna
Copy link
Author

vicuna commented Feb 24, 2017

Comment author: @alainfrisch

The original issue (being able to add attributes on toplevel lets) has been fixed. Marking as fixed for now. I don't know how critical is the remaining part (interpreting the ocaml.warning attribute). Feel free to open another ticket for this part.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants