bison-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RFC: propagate the indentation of the actions


From: Akim Demaille
Subject: RFC: propagate the indentation of the actions
Date: Thu, 13 Jun 2019 07:48:06 +0200

I'm very ambivalent about this change, I would appreciate feedback.


When compiling a generated parser, gcc recently told me something like:

> parse.y: In function 'yy_symbol_value_print':
> parse.y:69:18: error: 'yyou' undeclared (first use in this function); did you 
> mean 'yyo'?
>  %printer { fprintf (yyou, "\"%s\"", $$); } <char*>
>                   ^~~~
>                   yyo
> parse.y:69:18: note: each undeclared identifier is reported only once for 
> each function it appears in
> parse.y: In function 'yyparse':
> parse.y:103:15: warning: passing argument 1 of 'printf' from incompatible 
> pointer type [-Wincompatible-pointer-types]
>    NUM           { printf (stdout, "Hello, world!\n"); $$ = $1; }
>                ^~~~~~
> parse.y:104:23: warning: assignment to 'int' from 'const char *' makes 
> integer from pointer without a cast [-Wint-conversion]
>  | exp "+" exp   { $$ = $1 + "$3"; }
>                        ^

As you can see, there's an offset on the columns, and therefore the carets are 
badly placed.  So far, so we don't pay attention to the columns in which we 
output the user's code.  The appended patch tries to reproduce the indentation, 
which results in this:

> parse.y: In function 'parse.y: In function 'yy_symbol_value_print':
> parse.y:69:21: error: 'yyou' undeclared (first use in this function); did you 
> mean 'yyo'?
>  %printer { fprintf (yyou, "\"%s\"", $$); } <char*>
>                      ^~~~
>                      yyo
> parse.y:69:21: note: each undeclared identifier is reported only once for 
> each function it appears in
> parse.y: In function 'yyparse':
> parse.y:103:27: warning: passing argument 1 of 'printf' from incompatible 
> pointer type [-Wincompatible-pointer-types]
>    NUM           { printf (stdout, "Hello, world!\n"); $$ = $1; }
>                            ^~~~~~
> In file included from parse.y:35:
> /opt/local/lib/gcc8/gcc/x86_64-apple-darwin18/8.3.0/include-fixed/stdio.h:184:13:
>  note: expected 'const char * restrict' but argument is of type 'FILE *' {aka 
> 'struct __sFILE *'}
>  int  printf(const char * __restrict, ...) __printflike(1, 2);
>              ^
> parse.y:104:35: warning: assignment to 'int' from 'const char *' makes 
> integer from pointer without a cast [-Wint-conversion]
>  | exp "+" exp   { $$ = $1 + "$3"; }
>                                    ^

The first two diagnostics are much better, the last one is still wrong.  The 
generated code is:

>   case  case 9:
> #line 104 "/Users/akim/src/gnu/bison/examples/c/reccalc/parse.y"
>                 { (yyval.TOK_exp) = (yyvsp[-2].TOK_exp) + "$3"; }
> #line 1334 "examples/c/reccalc/parse.c"
>     break;

Indentation is correct, but the output does not have the same width, so, of 
course, alignment is not preserved.


On the one hand, this patch does improve the situation: some compiler-generated 
errors will be accurate.  On the other hand, the result is still not guaranteed 
to be correct, and I do not see how I could help the compiler produce accurate 
diagnostics: there's no #column (or better, _Pragma) that I know of, that we 
could use to insert accurate location information within the line.

So is this change worth it?  If it does, we should probably not indent when 
--no-line is used, since in that case the location of the diagnostics are of 
course always correct.

WDYT?


commit 05eba230bc39c2e88e61cf2a6cd6829cb141a541
Author: Akim Demaille <address@hidden>
Date:   Sun Jun 9 09:11:54 2019 +0200

    WIP: try to match the columns in the ouput
    
    * #: .

diff --git a/data/skeletons/bison.m4 b/data/skeletons/bison.m4
index ff769410..2c01ac0f 100644
--- a/data/skeletons/bison.m4
+++ b/data/skeletons/bison.m4
@@ -449,7 +449,7 @@ m4_define([b4_symbol_action],
                    [(*yylocationp)])dnl
     _b4_symbol_case([$1])[]dnl
 b4_syncline([b4_symbol([$1], [$2_line])], [b4_symbol([$1], [$2_file])])dnl
-      b4_symbol([$1], [$2])
+b4_symbol([$1], [$2])
 b4_syncline([@oline@], [@ofile@])dnl
         break;
 
diff --git a/src/output.c b/src/output.c
index 1f519d0c..97882069 100644
--- a/src/output.c
+++ b/src/output.c
@@ -374,7 +374,9 @@ user_actions_output (FILE *out)
                  rules[r].is_predicate ? "b4_predicate_case" : "b4_case",
                  r + 1, rules[r].action_loc.start.line);
         string_output (out, rules[r].action_loc.start.file);
-        fprintf (out, ")dnl\n[    %s]])\n\n", rules[r].action);
+        fprintf (out, ")dnl\n[%*s%s]])\n\n",
+                 rules[r].action_loc.start.column - 1, "",
+                 rules[r].action);
       }
   fputs ("])\n\n", out);
 }
@@ -482,7 +484,9 @@ prepare_symbol_definitions (void)
               muscle_location_grow (key, p->location);
 
               SET_KEY (pname);
-              MUSCLE_INSERT_STRING_RAW (key, p->code);
+              obstack_printf (&muscle_obstack,
+                              "%*s%s", p->location.start.column - 1, "", 
p->code);
+              muscle_insert (key, obstack_finish0 (&muscle_obstack));
             }
         }
 #undef SET_KEY2




reply via email to

[Prev in Thread] Current Thread [Next in Thread]