[prev in list] [next in list] [prev in thread] [next in thread]
List: gcc-patches
Subject: [RFC/PATCH] Fix-it hints
From: Manuel_López-Ibáñez <lopezibanez () gmail ! com>
Date: 2014-08-31 21:57:25
Message-ID: CAESRpQCLTvV5K=D7fBpmvZJGE+iavpRBGpcsnVxHh=EhLFjsPQ () mail ! gmail ! com
[Download RAW message or body]
This patch implements fix-it hints. See https://gcc.gnu.org/PR62314
When the caret line is active (which is the default), this adds an
additional source-line indicating how to fix the code:
gcc/testsuite/g++.dg/template/crash83.C:5:21: error: an explicit
specialization must be preceded by 'template <>'
template<typename = class A<0>: > struct B {}; // { dg-error
"explicit specialization|expected" }
^
template<>
When the caret line is disabled with -fno-diagnostics-show-caret, the
fix-it hint is printed as:
gcc/testsuite/g++.dg/template/crash83.C:5:21: error: an explicit
specialization must be preceded by 'template <>'
gcc/testsuite/g++.dg/template/crash83.C:5:21: fixit: template<>
The latter form may allow an IDE (such as emacs) to automatically apply the fix.
Currently, fix-it hints are limited to insertions at one single
location, whereas Clang allows insertions, deletions, and replacements
at arbitrary location ranges.
Opinions? Is the proposed interface/implementation acceptable?
Any other diagnostics that could use a fix-it hint? In principle, we
should only give them when we are sure that the proposed fix will fix
the error or silence a warning. For example, the C++ parser often
says 'x' expected before 'y' but adding x before y rarely fixes
anything.
gcc/ChangeLog:
2014-08-31 Manuel López-Ibáñez <manu@gcc.gnu.org>
PR c++/62314
* diagnostic.def (DK_FIXIT): New.
* diagnostic.c (adjust_column): New.
(adjust_line): Factor out adjust_column.
(get_source_line_and_column): New.
(diagnostic_show_locus): Call get_source_line_and_column.
(diagnostic_action_after_output): Ignore DK_FIXIT.
(fixit_hint): New.
* diagnostic-core.h (fixit_hint): Declare.
gcc/cp/ChangeLog:
2014-08-31 Manuel López-Ibáñez <manu@gcc.gnu.org>
PR c++/62314
* parser.c (cp_parser_diagnose_invalid_type_name): Call fixit_hint.
(cp_parser_expression_statement): Likewise.
(cp_parser_class_head): Likewise. Update location.
gcc/testsuite/ChangeLog:
2014-08-31 Manuel López-Ibáñez <manu@gcc.gnu.org>
PR c++/62314
* g++.old-deja/g++.oliva/typename1.C: Handle fixit hint.
* g++.old-deja/g++.oliva/typename2.C: Likewise.
* g++.old-deja/g++.other/typename1.C: Likewise.
* g++.old-deja/g++.pt/typename6.C: Likewise.
* g++.old-deja/g++.pt/typename3.C: Likewise.
* g++.old-deja/g++.pt/typename4.C: Likewise.
* g++.dg/parse/error36.C: Likewise.
* g++.dg/parse/typedef2.C: Likewise.
* g++.dg/template/error6.C: Likewise.
* g++.dg/template/dependent-name5.C: Likewise.
* g++.dg/template/crash83.C: Likewise.
* g++.dg/template/typename3.C: Likewise.
["fixit-hints.diff" (text/plain)]
Index: gcc/diagnostic.def
===================================================================
--- gcc/diagnostic.def (revision 214756)
+++ gcc/diagnostic.def (working copy)
@@ -35,10 +35,11 @@ DEFINE_DIAGNOSTIC_KIND (DK_ICE, "interna
DEFINE_DIAGNOSTIC_KIND (DK_ERROR, "error: ", "error")
DEFINE_DIAGNOSTIC_KIND (DK_SORRY, "sorry, unimplemented: ", "error")
DEFINE_DIAGNOSTIC_KIND (DK_WARNING, "warning: ", "warning")
DEFINE_DIAGNOSTIC_KIND (DK_ANACHRONISM, "anachronism: ", "warning")
DEFINE_DIAGNOSTIC_KIND (DK_NOTE, "note: ", "note")
+DEFINE_DIAGNOSTIC_KIND (DK_FIXIT, "fixit: ", "note")
DEFINE_DIAGNOSTIC_KIND (DK_DEBUG, "debug: ", "note")
/* These two would be re-classified as DK_WARNING or DK_ERROR, so the
prefix does not matter. */
DEFINE_DIAGNOSTIC_KIND (DK_PEDWARN, "pedwarn: ", NULL)
DEFINE_DIAGNOSTIC_KIND (DK_PERMERROR, "permerror: ", NULL)
Index: gcc/diagnostic.c
===================================================================
--- gcc/diagnostic.c (revision 214756)
+++ gcc/diagnostic.c (working copy)
@@ -254,61 +254,81 @@ diagnostic_build_prefix (diagnostic_cont
s.column, locus_ce, text_cs, text, text_ce)
: build_message_string ("%s%s:%d:%s %s%s%s", locus_cs, s.file, s.line, locus_ce,
text_cs, text, text_ce));
}
+
+static int
+adjust_column (int line_width, int max_width, int column)
+
+{
+ int right_margin = 10;
+ gcc_checking_assert (line_width >= column);
+ if (line_width >= max_width)
+ {
+ right_margin = MIN (line_width - column, right_margin);
+ right_margin = max_width - right_margin;
+ if (column > right_margin)
+ return right_margin;
+ }
+ return column;
+}
+
/* If LINE is longer than MAX_WIDTH, and COLUMN is not smaller than
MAX_WIDTH by some margin, then adjust the start of the line such
that the COLUMN is smaller than MAX_WIDTH minus the margin. The
margin is either 10 characters or the difference between the column
and the length of the line, whatever is smaller. The length of
LINE is given by LINE_WIDTH. */
static const char *
adjust_line (const char *line, int line_width,
int max_width, int *column_p)
{
- int right_margin = 10;
- int column = *column_p;
+ int old_column = *column_p;
- gcc_checking_assert (line_width >= column);
- right_margin = MIN (line_width - column, right_margin);
- right_margin = max_width - right_margin;
- if (line_width >= max_width && column > right_margin)
- {
- line += column - right_margin;
- *column_p = right_margin;
- }
+ *column_p = adjust_column (line_width, max_width, old_column);
+ line += old_column - *column_p;
return line;
}
+static const char *
+get_source_line_and_column (location_t loc, int *line_width, int *column)
+{
+ expanded_location s = expand_location_to_spelling_point (loc);
+ const char * line = location_get_source_line (s, line_width);
+ *column = s.column;
+ if (s.column > *line_width)
+ return NULL;
+
+ return line;
+}
/* Print the physical source line corresponding to the location of
this diagnostic, and a caret indicating the precise column. */
void
diagnostic_show_locus (diagnostic_context * context,
const diagnostic_info *diagnostic)
{
const char *line;
- int line_width;
+ int line_width, column;
char *buffer;
- expanded_location s;
int max_width;
const char *saved_prefix;
const char *caret_cs, *caret_ce;
if (!context->show_caret
|| diagnostic->location <= BUILTINS_LOCATION
|| diagnostic->location == context->last_location)
return;
context->last_location = diagnostic->location;
- s = expand_location_to_spelling_point (diagnostic->location);
- line = location_get_source_line (s, &line_width);
- if (line == NULL || s.column > line_width)
+ line = get_source_line_and_column (diagnostic->location,
+ &line_width, &column);
+ if (line == NULL)
return;
max_width = context->caret_max_width;
- line = adjust_line (line, line_width, max_width, &(s.column));
+ line = adjust_line (line, line_width, max_width, &column);
pp_newline (context->printer);
saved_prefix = pp_get_prefix (context->printer);
pp_set_prefix (context->printer, NULL);
pp_space (context->printer);
@@ -325,13 +345,13 @@ diagnostic_show_locus (diagnostic_contex
pp_newline (context->printer);
caret_cs = colorize_start (pp_show_color (context->printer), "caret");
caret_ce = colorize_stop (pp_show_color (context->printer));
/* pp_printf does not implement %*c. */
- size_t len = s.column + 3 + strlen (caret_cs) + strlen (caret_ce);
+ size_t len = column + 3 + strlen (caret_cs) + strlen (caret_ce);
buffer = XALLOCAVEC (char, len);
- snprintf (buffer, len, "%s %*c%s", caret_cs, s.column, context->caret_char,
+ snprintf (buffer, len, "%s %*c%s", caret_cs, column, context->caret_char,
caret_ce);
pp_string (context->printer, buffer);
pp_set_prefix (context->printer, saved_prefix);
pp_needs_newline (context->printer) = true;
}
@@ -439,10 +459,11 @@ diagnostic_action_after_output (diagnost
{
case DK_DEBUG:
case DK_NOTE:
case DK_ANACHRONISM:
case DK_WARNING:
+ case DK_FIXIT:
break;
case DK_ERROR:
case DK_SORRY:
if (context->abort_on_error)
@@ -940,10 +961,55 @@ inform (location_t location, const char
diagnostic_set_info (&diagnostic, gmsgid, &ap, location, DK_NOTE);
report_diagnostic (&diagnostic);
va_end (ap);
}
+/* A fix-it hint at LOCATION. Use this recommended textual
+ replacements after another diagnostic message. */
+void
+fixit_hint (location_t location, const char *msg, ...)
+{
+ diagnostic_info diagnostic;
+ va_list ap;
+
+ va_start (ap, msg);
+ diagnostic_set_info (&diagnostic, msg, &ap, location, DK_FIXIT);
+ if (!global_dc->show_caret)
+ {
+ report_diagnostic (&diagnostic);
+ }
+ else
+ {
+ diagnostic_context * context = global_dc;
+ const char *line;
+ int line_width, column;
+ line = get_source_line_and_column (location, &line_width, &column);
+ if (line == NULL)
+ {
+ va_end (ap);
+ return;
+ }
+ column = adjust_column (line_width, context->caret_max_width, column);
+
+ const char *saved_prefix = pp_get_prefix (context->printer);
+ pp_set_prefix (context->printer, NULL);
+
+ const char *caret_cs, *caret_ce;
+ caret_cs = colorize_start (pp_show_color (context->printer), "caret");
+ caret_ce = colorize_stop (pp_show_color (context->printer));
+
+ /* pp_printf does not implement %*c. */
+ size_t len = strlen (caret_cs) + column + 3 + strlen(msg) + strlen (caret_ce);
+ char * buffer = XALLOCAVEC (char, len);
+ snprintf (buffer, len, "%s%*c%s%s", caret_cs, column, ' ', msg, caret_ce);
+ pp_string (context->printer, buffer);
+ pp_newline_and_flush (context->printer);
+ pp_set_prefix (context->printer, saved_prefix);
+ }
+ va_end (ap);
+}
+
/* An informative note at LOCATION. Use this for additional details on an
error message. */
void
inform_n (location_t location, int n, const char *singular_gmsgid,
const char *plural_gmsgid, ...)
Index: gcc/diagnostic-core.h
===================================================================
--- gcc/diagnostic-core.h (revision 214756)
+++ gcc/diagnostic-core.h (working copy)
@@ -70,10 +70,11 @@ extern void fatal_error (const char *, .
extern bool pedwarn (location_t, int, const char *, ...)
ATTRIBUTE_GCC_DIAG(3,4);
extern bool permerror (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
extern void sorry (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2);
extern void inform (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
+extern void fixit_hint (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
extern void inform_n (location_t, int, const char *, const char *, ...)
ATTRIBUTE_GCC_DIAG(3,5) ATTRIBUTE_GCC_DIAG(4,5);
extern void verbatim (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2);
extern bool emit_diagnostic (diagnostic_t, location_t, int,
const char *, ...) ATTRIBUTE_GCC_DIAG(4,5);
Index: gcc/testsuite/g++.old-deja/g++.oliva/typename1.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.oliva/typename1.C (revision 214756)
+++ gcc/testsuite/g++.old-deja/g++.oliva/typename1.C (working copy)
@@ -10,7 +10,8 @@ template <class T> struct foo;
template <class T> struct bar {
typedef int foo;
};
template <class T> struct baz {
- typedef bar<T>::foo foo; // { dg-error "" } missing typename
+ typedef bar<T>::foo foo; // { dg-error "typename" "missing typename" }
+ // { dg-message "11:fixit: typename" "fixit" { target *-*-*} 15 }
};
Index: gcc/testsuite/g++.old-deja/g++.oliva/typename2.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.oliva/typename2.C (revision 214756)
+++ gcc/testsuite/g++.old-deja/g++.oliva/typename2.C (working copy)
@@ -21,8 +21,9 @@ struct foo;
template <class T> struct bar {
typedef int foo;
};
template <class T> struct baz {
- typedef bar<T>::foo foo; // { dg-error "" } implicit typename
+ typedef bar<T>::foo foo; // { dg-error "typename" "implicit typename" }
+ // { dg-message "11:fixit: typename" "fixit" { target *-*-*} 26 }
void m(foo);
};
Index: gcc/testsuite/g++.old-deja/g++.other/typename1.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.other/typename1.C (revision 214756)
+++ gcc/testsuite/g++.old-deja/g++.other/typename1.C (working copy)
@@ -12,6 +12,7 @@ public:
template<class T>
void f()
{
Vector<T>::iterator i = 0; // { dg-error "typename" "typename" } missing typename
-} // { dg-error "expected" "expected" { target *-*-* } 16 }
+ // { dg-message "3:fixit: typename" "fixit" { target *-*-*} 16 }
+} // { dg-bogus "expected" "expected" { xfail *-*-* } 16 }
Index: gcc/testsuite/g++.old-deja/g++.pt/typename6.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.pt/typename6.C (revision 214756)
+++ gcc/testsuite/g++.old-deja/g++.pt/typename6.C (working copy)
@@ -14,7 +14,8 @@ struct B : public A<U>
// { dg-message "note" "note" { target *-*-* } 13 }
};
template <class U>
A<U>::A_Type B<U>::Func() // { dg-error "typename" } function
+// { dg-message "1:fixit: typename" "fixit" { target *-*-*} 18 }
{
}
Index: gcc/testsuite/g++.old-deja/g++.pt/typename3.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.pt/typename3.C (revision 214756)
+++ gcc/testsuite/g++.old-deja/g++.pt/typename3.C (working copy)
@@ -16,6 +16,7 @@ struct B : public A<U>
};
template <class U>
B<U>::A_Type B<U>::Func() { // { dg-error "typename" } implicit typename
+ // { dg-message "1:fixit: typename" "fixit" { target *-*-*} 20 }
}
Index: gcc/testsuite/g++.old-deja/g++.pt/typename4.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.pt/typename4.C (revision 214756)
+++ gcc/testsuite/g++.old-deja/g++.pt/typename4.C (working copy)
@@ -21,6 +21,7 @@ struct C : public B<U>
};
template <class U>
C<U>::A_Type C<U>::Func() { // { dg-error "typename" } implicit typename
+// { dg-message "1:fixit: typename" "fixit" { target *-*-*} 25 }
}
Index: gcc/testsuite/g++.dg/parse/error36.C
===================================================================
--- gcc/testsuite/g++.dg/parse/error36.C (revision 214756)
+++ gcc/testsuite/g++.dg/parse/error36.C (working copy)
@@ -9,23 +9,28 @@ template <class T> struct A
template <class T>
void f(T t)
{
typedef A<T>::foo type; // { dg-error "typename" }
+ // { dg-message "11:fixit: typename" "fixit" { target *-*-* } 13 }
A<T>::bar b; // { dg-error "typename" "typename" }
-} // { dg-error "expected ';'" "expected" { target *-*-* } 14 }
+ // { dg-message "3:fixit: typename" "fixit" { target *-*-* } 15 }
+} // { dg-bogus "expected ';'" "expected" { xfail *-*-* } 15 }
// PR c++/36353
template <class T> struct B
{
void f()
{
A<T>::baz z; // { dg-error "typename" "typename" }
- } // { dg-error "expected ';'" "expected" { target *-*-* } 22 }
+ // { dg-message "5:fixit: typename" "fixit" { target *-*-* } 24 }
+ } // { dg-bogus "expected ';'" "expected" { xfail *-*-* } 24 }
};
// PR c++/40738
template <class T>
void g(const A<T>::type &t); // { dg-error "typename" "typename" }
+// { dg-message "14:fixit: typename" "fixit" { target *-*-* } 31 }
// PR c++/18451
template <class T> A<T>::B A<T>::b; // { dg-error "typename" }
+// { dg-message "20:fixit: typename" "fixit" { target *-*-* } 35 }
Index: gcc/testsuite/g++.dg/parse/typedef2.C
===================================================================
--- gcc/testsuite/g++.dg/parse/typedef2.C (revision 214756)
+++ gcc/testsuite/g++.dg/parse/typedef2.C (working copy)
@@ -1,3 +1,4 @@
template <typename T> struct B { typedef typename T::X X; };
-template <typename T> struct A { typedef B<T>::X::Y Z; }; // { dg-error "" }
-
+template <typename T> struct A { typedef B<T>::X::Y Z; }; // { dg-error "typename" }
+// { dg-message "42:fixit: typename" "fixit" { target *-*-* } 2 }
+
Index: gcc/testsuite/g++.dg/template/error6.C
===================================================================
--- gcc/testsuite/g++.dg/template/error6.C (revision 214756)
+++ gcc/testsuite/g++.dg/template/error6.C (working copy)
@@ -2,10 +2,11 @@ template<int n>
struct tento {
enum {value = 10*tento<n-1>::value};
};
struct tento<0> { // { dg-error "" }
+ // { dg-message "1:fixit: template<>" "fixit" { target *-*-*} 6 }
enum {value=1};
};
int main() {
if (tento<4>::value != 10000) return -1;
Index: gcc/testsuite/g++.dg/template/dependent-name5.C
===================================================================
--- gcc/testsuite/g++.dg/template/dependent-name5.C (revision 214756)
+++ gcc/testsuite/g++.dg/template/dependent-name5.C (working copy)
@@ -15,17 +15,19 @@ struct A
struct N {};
typedef Bar type1;
typedef A::Bar type2;
typedef A<T>::Bar type3;
- typedef A<T*>::Bar type4; // { dg-error "" }
+ typedef A<T*>::Bar type4; // { dg-error "typename" }
+ // { dg-message "11:fixit: typename" "fixit" { target *-*-*} 20 }
typedef typename A<T*>::Bar type5;
typedef N<int> type6;
typedef A::N<int> type7;
typedef A<T>::N<int> type8;
- typedef A<T*>::template N<int> type9; // { dg-error "" }
+ typedef A<T*>::template N<int> type9; // { dg-error "typename" }
+ // { dg-message "11:fixit: typename" "fixit" { target *-*-*} 27 }
typedef typename A<T*>::template N<int> type10;
typedef D Bar2;
struct N2 { typedef int K; };
@@ -34,11 +36,12 @@ struct A
typedef A::Bar2 type11;
typedef type11::K k3;
typedef A::N2 type12;
typedef typename type12::K k2;
- typedef type12::K k1; // { dg-error "" }
+ typedef type12::K k1; // { dg-error "typename" }
+ // { dg-message "11:fixit: typename" "fixit" { target *-*-*} 41 }
// Check that A::Bar2 is not considered dependent even if we use
// the typename keyword.
typedef typename A::Bar2 type13;
typedef type13::K k4;
Index: gcc/testsuite/g++.dg/template/crash83.C
===================================================================
--- gcc/testsuite/g++.dg/template/crash83.C (revision 214756)
+++ gcc/testsuite/g++.dg/template/crash83.C (working copy)
@@ -1,5 +1,6 @@
// PR c++/37650
template<int> struct A {};
template<typename = class A<0>: > struct B {}; // { dg-error "explicit specialization|expected" }
+// { dg-message "21:fixit: template<>" "fixit" { target *-*-* } 5 }
Index: gcc/testsuite/g++.dg/template/typename3.C
===================================================================
--- gcc/testsuite/g++.dg/template/typename3.C (revision 214756)
+++ gcc/testsuite/g++.dg/template/typename3.C (working copy)
@@ -2,6 +2,7 @@
// crash test - PR 7266
template <class A>
struct B {
typedef A::C::D E; // { dg-error "" }
+ // { dg-message "10:fixit: typename" "fixit" { target *-*-*} 6 }
};
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c (revision 214756)
+++ gcc/cp/parser.c (working copy)
@@ -2992,13 +2992,16 @@ cp_parser_diagnose_invalid_type_name (cp
error_at (location, "and %qT has no template constructors",
parser->scope);
}
else if (TYPE_P (parser->scope)
&& dependent_scope_p (parser->scope))
- error_at (location, "need %<typename%> before %<%T::%E%> because "
- "%qT is a dependent scope",
- parser->scope, id, parser->scope);
+ {
+ error_at (location, "%<typename%> is needed before %<%T::%E%> because "
+ "%qT is a dependent scope",
+ parser->scope, id, parser->scope);
+ fixit_hint (location, "typename ");
+ }
else if (TYPE_P (parser->scope))
{
if (cp_lexer_next_token_is (parser->lexer, CPP_LESS))
error_at (location_of (id),
"%qE in %q#T does not name a template type",
@@ -9813,13 +9816,16 @@ cp_parser_expression_statement (cp_parse
/* Give a helpful message for "A<T>::type t;" and the like. */
if (cp_lexer_next_token_is_not (parser->lexer, CPP_SEMICOLON)
&& !cp_parser_uncommitted_to_tentative_parse_p (parser))
{
if (TREE_CODE (statement) == SCOPE_REF)
- error_at (token->location, "need %<typename%> before %qE because "
- "%qT is a dependent scope",
- statement, TREE_OPERAND (statement, 0));
+ {
+ error_at (token->location, "%<typename%> is needed before %qE because "
+ "%qT is a dependent scope",
+ statement, TREE_OPERAND (statement, 0));
+ fixit_hint (token->location, "typename ");
+ }
else if (is_overloaded_fn (statement)
&& DECL_CONSTRUCTOR_P (get_first_fn (statement)))
{
/* A::A a; */
tree fn = get_first_fn (statement);
@@ -19820,10 +19826,12 @@ cp_parser_class_head (cp_parser* parser,
/* Look for the class-key. */
class_key = cp_parser_class_key (parser);
if (class_key == none_type)
return error_mark_node;
+ location_t class_head_start_location = input_location;
+
/* Parse the attributes. */
attributes = cp_parser_attributes_opt (parser);
/* If the next token is `::', that is invalid -- but sometimes
people do try to write:
@@ -20036,12 +20044,14 @@ cp_parser_class_head (cp_parser* parser,
it is not, try to recover gracefully. */
if (at_namespace_scope_p ()
&& parser->num_template_parameter_lists == 0
&& template_id_p)
{
- error_at (type_start_token->location,
+ error_at (class_head_start_location,
"an explicit specialization must be preceded by %<template <>%>");
+ fixit_hint (class_head_start_location, "template<> ");
+
invalid_explicit_specialization_p = true;
/* Take the same action that would have been taken by
cp_parser_explicit_specialization. */
++parser->num_template_parameter_lists;
begin_specialization ();
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic