[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
- RFC: propagate the indentation of the actions,
Akim Demaille <=