| Anonymous | Login | Signup for a new account | 2013-05-26 04:13 CEST | ![]() |
| Main | My View | View Issues | Change Log | Roadmap |
| View Issue Details [ Jump to Notes ] | [ Issue History ] [ Print ] | |||||||||||
| ID | Project | Category | View Status | Date Submitted | Last Update | |||||||
| 0005801 | OCaml | Misc | public | 2012-10-27 14:38 | 2012-11-05 16:30 | |||||||
| Reporter | turpin | |||||||||||
| Assigned To | hongboz | |||||||||||
| Priority | low | Severity | trivial | Reproducibility | always | |||||||
| Status | resolved | Resolution | no change required | |||||||||
| Platform | OS | OS Version | ||||||||||
| Product Version | 4.00.1 | |||||||||||
| Target Version | Fixed in Version | |||||||||||
| Summary | 0005801: two minor bugs in pprintast: (*) -> ( * ), {... _} -> {...; _} | |||||||||||
| Description | The attached patch corrects the above two syntax errors, and treats "as" in the same way as variables (imagine "match ... with _ as ( * ) -> ...") | |||||||||||
| Tags | No tags attached. | |||||||||||
| Attached Files | ||||||||||||
Notes |
|
|
(0008350) hongboz (developer) 2012-10-28 03:14 |
IIRC, this bug was already fixed in trunk. The tools/pprintast.ml was quite primitive(a lot of corner cases are not covered, and the indentation was not good). please don't use it for production, I did a rewrite for this file, when it's ready I will commit it , thanks |
|
(0008351) turpin (reporter) 2012-10-28 09:40 |
Indeed, it is fixed: no pore pprintast.ml in the trunk ;-) |
|
(0008364) frisch (developer) 2012-10-30 09:52 |
Tiphaine: pprintast.ml is now in parsing/, no longer in tools/. |
|
(0008365) hongboz (developer) 2012-10-30 12:29 |
Alain, actually I am ready, but found some problems so that I was waiting damien to help, but you can rename AstPrint.ml into pprintast.ml and commit if you like. Now, the output is quite good, bug report is welcome ;-) |
|
(0008366) frisch (developer) 2012-10-30 12:46 |
Hongbo: feel free to commit your patch directly. |
|
(0008391) hongboz (developer) 2012-10-31 19:24 |
turpin, I've committed a new parsing/pprintast.ml in the trunk, bug report and feature request is welcome |
|
(0008392) turpin (reporter) 2012-10-31 19:29 |
I will. |
|
(0008393) turpin (reporter) 2012-10-31 21:51 |
I have tried your new version. It seems less naïve than the previous code, but there are a number of bugs. Here is a patch which corrects 5 of them. The next one is about optional labels: assert (is_predef_option txt) fails. |
|
(0008395) hongboz (developer) 2012-10-31 21:59 |
turpin, I did not see your patch and error case |
|
(0008397) turpin (reporter) 2012-10-31 23:29 |
pprintast.patch, I have uploaded it on this page. |
|
(0008398) hongboz (developer) 2012-10-31 23:35 |
you did not update my version revision 13054 in trunk |
|
(0008399) hongboz (developer) 2012-10-31 23:36 |
btw, the new version is written using objects, so you can do hot-patch without touching the source, but bug report is still welcome |
|
(0008400) turpin (reporter) 2012-11-01 00:01 |
The patch I uploaded is against version 13054, unless my svn is lying to me. If you already fixed the bugs ( * ), ( or ), ( -1 ), etc. then you didn't commit the right file. Currently I'm reverting the 4.00.1 version with the patch I submitted, because it seems to produce syntactically correct code, and I don't care too much about readability. But I will re-test the new version once it's fixed. |
|
(0008401) hongboz (developer) 2012-11-01 00:59 |
your patch is not against my version, and sorry I can not reproduce your case. It would be helpful that you upload your error case. http://caml.inria.fr/cgi-bin/viewvc.cgi/ocaml/trunk/parsing/pprintast.ml?view=markup&pathrev=13055 [^] |
|
(0008406) turpin (reporter) 2012-11-01 18:28 |
I'm sorry, I didn't upload the right patch file. I thougt I did because I had named it pprintast.patch as the old patch that I did fot the 4.00.1 version. I must have forgotten to click the upload button or something. I just uploaded it, as pprintast-13054.patch (I cannot adapt it for 13055 right now but it should mostly work). |
|
(0008407) hongboz (developer) 2012-11-01 20:03 |
would you upload your error cases? add parens everywhere could eliminate the bugs, but it's not the optimal solution, thanks |
|
(0008413) turpin (reporter) 2012-11-04 18:26 |
anything like "assert (f x)" triggers the parenthesis bug. It is clear that in "assert e", "e" must be parenthesized (unless you do a precedence-aware unparsing, which I didn't see in your code). |
|
(0008414) hongboz (developer) 2012-11-04 21:33 edited on: 2012-11-04 21:34 |
it is precedence-aware. it passes all the parsetree in trunk even including camlp4. If you have bugs to report, plz file a reproducible case with respect to the trunk version, thx. |
|
(0008415) turpin (reporter) 2012-11-04 22:06 |
I had not seen the methods "simple_expr" ... which implement precedence. The assert bug is fixed in the current svn version (#expression replaced by #simple_expr). As for the other bugs, I didn't check everything, but: - "( ~+ )" is printed "+" - "let _ as f = fun x ->..." is printed incorrectly too. |
|
(0008420) hongboz (developer) 2012-11-05 15:37 |
the second is fixed. The first let a = (~+) 3 --> let a = 3 this is intended behavior, will it cause a bug in your code? |
|
(0008421) turpin (reporter) 2012-11-05 16:30 |
Yes: I generate code like "let ( ~+ ) = ( ~+ )". So ~+ may be used without argument, in this corner case. Also, as anyone can rebind ~+, it seems dangerous to assume that it means identity. |
Issue History |
|||
| Date Modified | Username | Field | Change |
| 2012-10-27 14:38 | turpin | New Issue | |
| 2012-10-27 14:38 | turpin | File Added: pprintast.patch | |
| 2012-10-28 03:14 | hongboz | Note Added: 0008350 | |
| 2012-10-28 03:15 | hongboz | Assigned To | => hongboz |
| 2012-10-28 03:15 | hongboz | Status | new => assigned |
| 2012-10-28 09:40 | turpin | Note Added: 0008351 | |
| 2012-10-28 14:52 | hongboz | Status | assigned => resolved |
| 2012-10-28 14:52 | hongboz | Resolution | open => no change required |
| 2012-10-30 09:52 | frisch | Note Added: 0008364 | |
| 2012-10-30 12:26 | hongboz | File Added: AstPrint.ml | |
| 2012-10-30 12:26 | hongboz | File Added: AstPrint.mli | |
| 2012-10-30 12:29 | hongboz | Note Added: 0008365 | |
| 2012-10-30 12:46 | frisch | Note Added: 0008366 | |
| 2012-10-31 19:24 | hongboz | Note Added: 0008391 | |
| 2012-10-31 19:29 | turpin | Note Added: 0008392 | |
| 2012-10-31 21:51 | turpin | Note Added: 0008393 | |
| 2012-10-31 21:59 | hongboz | Note Added: 0008395 | |
| 2012-10-31 22:00 | hongboz | File Deleted: AstPrint.ml | |
| 2012-10-31 23:29 | turpin | Note Added: 0008397 | |
| 2012-10-31 23:35 | hongboz | Note Added: 0008398 | |
| 2012-10-31 23:36 | hongboz | Note Added: 0008399 | |
| 2012-11-01 00:01 | turpin | Note Added: 0008400 | |
| 2012-11-01 00:59 | hongboz | Note Added: 0008401 | |
| 2012-11-01 18:22 | turpin | File Added: pprintast-13054.patch | |
| 2012-11-01 18:28 | turpin | Note Added: 0008406 | |
| 2012-11-01 20:03 | hongboz | Note Added: 0008407 | |
| 2012-11-04 18:26 | turpin | Note Added: 0008413 | |
| 2012-11-04 21:33 | hongboz | Note Added: 0008414 | |
| 2012-11-04 21:34 | hongboz | Note Edited: 0008414 | View Revisions |
| 2012-11-04 22:06 | turpin | Note Added: 0008415 | |
| 2012-11-05 15:37 | hongboz | Note Added: 0008420 | |
| 2012-11-05 16:30 | turpin | Note Added: 0008421 | |
| Copyright © 2000 - 2011 MantisBT Group |