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
Cannot re-bind "exception False" #5778
Comments
Comment author: @diml
This part is fixed. Commits 12994 and 12995. I think there are other places where the conversion is not done correctly: for example in "module False = struct end" the space at the beginning of False is kept.
It is another problem in the conversion from the Camlp4 Ast to the OCaml Ast. |
Comment author: @bobzhang can you pinpoint which printer is buggy? probably it's the toplevel printer (Camlp4Top/Rprint.ml) |
Comment author: @bobzhang camlp4>camlp4r -str 'exception False;' -printer o |
Comment author: @diml Actually it is not a printing problem. "False" can be used anywhere in revised syntax and Camlp4Ast2OCaml converts "False" to "false" (because the camlp4 ast uses "True" and "False" instead of "true" and "false", as in revised syntax): $ echo 'exception False;' > exn.ml |
Comment author: @bobzhang I see, the compiler does not check in 'Pstr_exception str' str is an uidentifier? In my branch I keep the original syntax and revised syntax the same, they both use true, false, other syntax also the same, like 'a:=!b ' instead of 'a.val:=b.val', how do you think of it? |
Comment author: furuse Today I found the same problem at type t = [ which When fixing, could you check they are also resolved? |
Comment author: @bobzhang This might be a design mistake, in Fan, there is no such translation, like the original syntax, 'true', 'false' are keywords instead. Given that Camlp4 is already deprecated internally, I am not sure such bugs might be fixed in time |
Comment author: @alainfrisch
I think this is an over-statement. While there are efforts to move away from Camlp4 (Fan; -ppx and attributes; runtime types), it is fair to believe that Camlp4 will remain alive and used in large code bases for some more months/years. |
Comment author: @diml I already tried to fix this issue, and it is just a nightmare. There are lot of places where the conversion from one way to the other is missing, and we have to be careful not to double convert True and False, i.e. not to end up with: "module false". I'll try to find the courage to give it another try. I think the best solution would be to remove these kind of conversions but there is a good chance it would break many syntax extensions, which often use revised syntax inside quotations. The best workaround for now is to avoid using True and False with camlp4... |
Comment author: @bobzhang Yes, it's quite tricky, especially in the meta level, and make sure to know that Camlp4 support anti-quotation by two ways: 1. the normal Ant node (this is fine) 2. string mangling via "\$" prefix ( a place to introduce number of potential bugs) |
Comment author: furuse Everyone hates P4 but it is also true that it survives few more years. If fixing is too hard, how about providing better error message? Is it also too hard to reject creating malformed idents/paths at rebuilding OCaml Parsetree? Currently, the error we get around True and False are so cryptic and need to run camlp4o to see what is happening. This is very cumbersome. |
Comment author: @diml Avoiding " True" and " False" should be feasible. This still means that sometimes we'll get "true" instead of "True" or the opposite. BTW the printer and the camlp4 ast to ocaml parsetree converter are both mapping true and false independently so the output of camlp4o may be different to what is given to the compiler. [-dparsetree] or [-dsource] should be more helpful. |
Comment author: @damiendoligez What is the status of this PR? The status field says "assigned" (which is a flavour of "open"), but the "Fixed in Version" says "4.00.2+dev" (which implies that it is fixed. |
Comment author: @diml It is still open, the "Fixed in Version" was an error. Actually if nobody objects or want to reassign the issue to himself I'll set the resolution to "won't fix". I don't see any simple fix and since camlp4 is going away I don't think it is worth spending too much time on this. |
Comment author: @bobzhang I would suggest a fundamental solution : no conversion table. What do you think? |
Comment author: @diml Well, that would indeed be much simpler, but that would break extensions written using revised syntax quotations, which are quite common. |
Original bug ID: 5778
Reporter: beckerb
Assigned to: @diml
Status: closed (set by @xavierleroy on 2015-12-11T18:21:18Z)
Resolution: won't fix
Priority: normal
Severity: minor
Version: 4.00.1
Target version: later
Category: -for Camlp4 use https://github.com/ocaml/camlp4/issues
Bug description
Re-binding an exception [font=courier]False[/font] causes an error "Unbound exception constructor" when using Camlp4 with original syntax.
[code]
module M = struct exception False end;;
module M : sig exception False end
exception False = M.False;;
Error: Unbound exception constructor
M. False
[/code]
Note the space before [font=courier]False[/font] in the error message.
BTW, there is a possibly related glitch when pretty-printing that exception in revised syntax:
[code]
exception False;
exception false
[/code]
The text was updated successfully, but these errors were encountered: