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

camlp4: quotation issue with strings #5442

Closed
vicuna opened this Issue Dec 23, 2011 · 3 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link
Collaborator

vicuna commented Dec 23, 2011

Original bug ID: 5442
Reporter: @damiendoligez
Assigned to: @diml
Status: closed (set by @xavierleroy on 2015-12-11T18:07:03Z)
Resolution: fixed
Priority: normal
Severity: minor
Platform: all
Version: 3.12.1
Fixed in version: 3.12.1+dev
Category: -for Camlp4 use https://github.com/ocaml/camlp4/issues
Related to: #5633 #5646
Monitored by: @hcarty

Bug description

This was reported on 2011-07-11 by Petter Urkedal on caml-list under the subject "Quotation issue with camlp4 printer".

The attached program prints out

let x = """;;

when compiled with ocamlc and camlp4 version 3.12.0. As you can see,
the string literal lacks an escape character. I haven't tried version
3.12.1, but I didn't find anything about it in the release notes.

Steps to reproduce

ocamlc.opt -c -I +camlp4 -pp camlp4orf -o quotation_bug.cmo
quotation_bug.ml
ocamlc.opt dynlink.cma -I +camlp4 camlp4lib.cma quotation_bug.cmo
-o quotation_bug.byte
./quotation_bug.byte

Additional information

A tentative fix was done in commit 11119, but it breaks findlib (among others) because it double-escapes everything in string constants when preprocessing normal code with "camlp4 pa_o.cmo pr_o.cmo".

It looks like the problem is in the parsing of quotations, not in the printing code.

File attachments

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Dec 23, 2011

Comment author: @diml

Fixed. Commits 11947 and 11948.

The problem was due to the fact that strings are escaped in the camlp4 ast, so they must be escaped again when the ast is meta-expansed, but it was not done.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Dec 30, 2011

Comment author: @damiendoligez

There is a problem with the fix: it breaks type-conv 2.3.0.

To reproduce:
Install type-conv version 2.3.0 (I haven't tried other version) from Jane Street web site.
---------- foo.ml
module Foo = struct end;; module Bar = struct end;;

camlp4o /usr/local/ocaml/3.12/lib/ocaml/site-lib/type-conv/pa_type_conv.cma foo.ml

With text output, you get: Failure: "Cannot print "\\$:i" this string contains more than one token"
With AST output, you get a compiler error because both identifiers Foo and Bar got replaced by $:i

I've narrowed it down to your commit. If I replace "String.escaped" with "safe_string_escaped" at line 474 of boot/Camlp4Ast.ml, it seems to fix the problem, but I'll let you decide what to do because I don't really understand camlp4.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Dec 30, 2011

Comment author: @diml

It is indeed safe_string_escaped that must be used instead of String.escaped.

Fixed by commits 11983 and 11984.

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.