Revisiting this patch from 2018 that didn't quite make it; earlier versions were: v1: https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00802.html v2: https://gcc.gnu.org/legacy-ml/gcc-patches/2018-11/msg01408.html v3: https://gcc.gnu.org/legacy-ml/gcc-patches/2018-11/msg01658.html v4: https://gcc.gnu.org/legacy-ml/gcc-patches/2018-11/msg02617.html
I believe the remaining point of discussion was about enum vs int: https://gcc.gnu.org/legacy-ml/gcc-patches/2018-12/msg00293.html and that none of us had strong opinions on the matter. I've added some test coverage for that, and rebased it (e.g. for the .c to .cc renaming of our sources). Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. OK for trunk? This patch adds a note with a fix-it hint to various pointer-vs-non-pointer diagnostics, suggesting the addition of a leading '&' or '*'. For example, note the ampersand fix-it hint in the following: demo.c: In function 'int main()': demo.c:5:22: error: invalid conversion from 'pthread_key_t' {aka 'unsigned int'} to 'pthread_key_t*' {aka 'unsigned int*'} [-fpermissive] 5 | pthread_key_create(key, NULL); | ^~~ | | | pthread_key_t {aka unsigned int} demo.c:5:22: note: possible fix: take the address with '&' 5 | pthread_key_create(key, NULL); | ^~~ | & In file included from demo.c:1: /usr/include/pthread.h:1122:47: note: initializing argument 1 of 'int pthread_key_create(pthread_key_t*, void (*)(void*))' 1122 | extern int pthread_key_create (pthread_key_t *__key, | ~~~~~~~~~~~~~~~^~~~~ gcc/c-family/ChangeLog: PR c++/87850 * c-common.cc: Include "gcc-rich-location.h". (maybe_emit_indirection_note): New function. * c-common.h (maybe_emit_indirection_note): New decl. (compatible_types_for_indirection_note_p): New decl. gcc/c/ChangeLog: PR c++/87850 * c-typeck.cc (compatible_types_for_indirection_note_p): New function. (convert_for_assignment): Call maybe_emit_indirection_note for pointer vs non-pointer diagnostics. gcc/cp/ChangeLog: PR c++/87850 * call.cc (convert_like_real): Call maybe_emit_indirection_note for "invalid conversion" diagnostic. * typeck.cc (compatible_types_for_indirection_note_p): New function. gcc/testsuite/ChangeLog: PR c++/87850 * c-c++-common/indirection-fixits.c: New test. * g++.dg/template/error60.C: Add fix-it hint to expected output. * g++.dg/template/error60a.C: Likewise. Signed-off-by: David Malcolm <dmalc...@redhat.com> --- gcc/c-family/c-common.cc | 35 ++ gcc/c-family/c-common.h | 4 + gcc/c/c-typeck.cc | 18 +- gcc/cp/call.cc | 2 + gcc/cp/typeck.cc | 8 + .../c-c++-common/indirection-fixits.c | 317 ++++++++++++++++++ gcc/testsuite/g++.dg/template/error60.C | 5 + gcc/testsuite/g++.dg/template/error60a.C | 5 + 8 files changed, 392 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/indirection-fixits.c diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc index 1ffc63afbd3a..8cae3d1feb1f 100644 --- a/gcc/c-family/c-common.cc +++ b/gcc/c-family/c-common.cc @@ -56,6 +56,7 @@ along with GCC; see the file COPYING3. If not see #include "vec-perm-indices.h" #include "tree-pretty-print-markup.h" #include "gcc-urlifier.h" +#include "gcc-rich-location.h" cpp_reader *parse_in; /* Declared in c-pragma.h. */ @@ -9640,6 +9641,40 @@ maybe_suggest_missing_token_insertion (rich_location *richloc, } } +/* Potentially emit a note about likely missing '&' or '*', + depending on EXPR and EXPECTED_TYPE. */ + +void +maybe_emit_indirection_note (location_t loc, + tree expr, tree expected_type) +{ + gcc_assert (expr); + gcc_assert (expected_type); + + tree actual_type = TREE_TYPE (expr); + + /* Missing '&'. */ + if (TREE_CODE (expected_type) == POINTER_TYPE + && compatible_types_for_indirection_note_p (actual_type, + TREE_TYPE (expected_type)) + && lvalue_p (expr)) + { + gcc_rich_location richloc (loc); + richloc.add_fixit_insert_before ("&"); + inform (&richloc, "possible fix: take the address with %qs", "&"); + } + + /* Missing '*'. */ + if (TREE_CODE (actual_type) == POINTER_TYPE + && compatible_types_for_indirection_note_p (TREE_TYPE (actual_type), + expected_type)) + { + gcc_rich_location richloc (loc); + richloc.add_fixit_insert_before ("*"); + inform (&richloc, "possible fix: dereference with %qs", "*"); + } +} + #if CHECKING_P namespace selftest { diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h index a74530bafff6..a4c925278bb6 100644 --- a/gcc/c-family/c-common.h +++ b/gcc/c-family/c-common.h @@ -1694,6 +1694,10 @@ extern void maybe_add_include_fixit (rich_location *, const char *, bool); extern void maybe_suggest_missing_token_insertion (rich_location *richloc, enum cpp_ttype token_type, location_t prev_token_loc); +extern void maybe_emit_indirection_note (location_t loc, + tree expr, tree expected_type); +extern bool compatible_types_for_indirection_note_p (tree type1, tree type2); + extern tree braced_lists_to_strings (tree, tree); #if CHECKING_P diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc index 8960c55aef3b..1624d3f96918 100644 --- a/gcc/c/c-typeck.cc +++ b/gcc/c/c-typeck.cc @@ -1440,6 +1440,14 @@ struct comptypes_data { const struct tagged_tu_seen_cache* cache; }; +/* C implementation of compatible_types_for_indirection_note_p. */ + +bool +compatible_types_for_indirection_note_p (tree type1, tree type2) +{ + return comptypes (type1, type2) == 1; +} + /* Return 1 if TYPE1 and TYPE2 are compatible types for assignment or various other operations. Return 2 if they are compatible but a warning may be needed if you use them together. */ @@ -8487,7 +8495,10 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type, if (permerror_opt (&richloc, OPT_Wint_conversion, "passing argument %d of %qE makes pointer " "from integer without a cast", parmnum, rname)) - inform_for_arg (fundecl, expr_loc, parmnum, type, rhstype); + { + maybe_emit_indirection_note (expr_loc, rhs, type); + inform_for_arg (fundecl, expr_loc, parmnum, type, rhstype); + } } break; case ic_assign: @@ -8527,7 +8538,10 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type, if (permerror_opt (&richloc, OPT_Wint_conversion, "passing argument %d of %qE makes integer from " "pointer without a cast", parmnum, rname)) - inform_for_arg (fundecl, expr_loc, parmnum, type, rhstype); + { + maybe_emit_indirection_note (expr_loc, rhs, type); + inform_for_arg (fundecl, expr_loc, parmnum, type, rhstype); + } } break; case ic_assign: diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc index 3033be47c7d7..cc9761d68b40 100644 --- a/gcc/cp/call.cc +++ b/gcc/cp/call.cc @@ -8585,6 +8585,8 @@ convert_like_internal (conversion *convs, tree expr, tree fn, int argnum, complained = permerror (&richloc, "invalid conversion from %qH to %qI", TREE_TYPE (expr), totype); + if (complained) + maybe_emit_indirection_note (loc, expr, totype); } if (convs->kind == ck_ref_bind) expr = convert_to_reference (totype, expr, CONV_IMPLICIT, diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc index 4c15e26f692a..cf694555aac5 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -1687,6 +1687,14 @@ structural_comptypes (tree t1, tree t2, int strict) return true; } +/* C++ implementation of compatible_types_for_indirection_note_p. */ + +bool +compatible_types_for_indirection_note_p (tree type1, tree type2) +{ + return same_type_p (type1, type2); +} + /* Return true if T1 and T2 are related as allowed by STRICT. STRICT is a bitwise-or of the COMPARE_* flags. */ diff --git a/gcc/testsuite/c-c++-common/indirection-fixits.c b/gcc/testsuite/c-c++-common/indirection-fixits.c new file mode 100644 index 000000000000..0a658edd238d --- /dev/null +++ b/gcc/testsuite/c-c++-common/indirection-fixits.c @@ -0,0 +1,317 @@ +/* { dg-options "-fdiagnostics-show-caret" } */ + +void takes_int_ptr(int*); +void takes_char_ptr(char*); +void takes_int(int); +int returns_int(void); + +int ivar; +char cvar; +int *int_ptr; +char *char_ptr; + +void test_1 (void) +{ + takes_int_ptr(&ivar); + takes_int_ptr(int_ptr); + takes_char_ptr(&cvar); + takes_char_ptr(char_ptr); + + ivar = 42; + cvar = 'b'; + int_ptr = &ivar; + char_ptr = &cvar; +} + +void test_2 (void) +{ + takes_int_ptr(ivar); /* { dg-error "" "" } */ + /* { dg-message "possible fix: take the address with '&'" "" { target *-*-* } .-1 } */ + + /* Expect an '&' fix-it hint. */ + /* { dg-begin-multiline-output "" } + takes_int_ptr(ivar); + ^~~~ + | + int + { dg-end-multiline-output "" } */ + /* { dg-begin-multiline-output "" } + takes_int_ptr(ivar); + ^~~~ + & + { dg-end-multiline-output "" } */ + /* { dg-begin-multiline-output "" } + void takes_int_ptr(int*); + ^~~~ + { dg-end-multiline-output "" } */ +} + +void test_3 (void) +{ + takes_int_ptr(cvar); /* { dg-error "" } */ + + /* Don't expect an '&' fix-it hint. */ + /* { dg-begin-multiline-output "" } + takes_int_ptr(cvar); + ^~~~ + | + char + { dg-end-multiline-output "" } */ + /* { dg-begin-multiline-output "" } + void takes_int_ptr(int*); + ^~~~ + { dg-end-multiline-output "" } */ +} + +void test_4 (void) +{ + takes_char_ptr(ivar); /* { dg-error "" } */ + + /* Don't expect an '&' fix-it hint. */ + /* { dg-begin-multiline-output "" } + takes_char_ptr(ivar); + ^~~~ + | + int + { dg-end-multiline-output "" } */ + /* { dg-begin-multiline-output "" } + void takes_char_ptr(char*); + ^~~~~ + { dg-end-multiline-output "" } */ +} + +void test_5 (void) +{ + takes_char_ptr(cvar); /* { dg-error "" } */ + + /* Expect an '&' fix-it hint. */ + /* { dg-begin-multiline-output "" } + takes_char_ptr(cvar); + ^~~~ + | + char + { dg-end-multiline-output "" } */ + /* { dg-begin-multiline-output "" } + takes_char_ptr(cvar); + ^~~~ + & + { dg-end-multiline-output "" } */ + /* { dg-begin-multiline-output "" } + void takes_char_ptr(char*); + ^~~~~ + { dg-end-multiline-output "" } */ +} + +void test_6 (void) +{ + takes_int(int_ptr); /* { dg-error "" } */ + /* { dg-message "possible fix: dereference with '*'" "" { target *-*-* } .-1 } */ + + /* Expect a '*' fix-it hint. */ + /* { dg-begin-multiline-output "" } + takes_int(int_ptr); + ^~~~~~~ + | + int * + { dg-end-multiline-output "" { target c } } */ + /* { dg-begin-multiline-output "" } + takes_int(int_ptr); + ^~~~~~~ + | + int* + { dg-end-multiline-output "" { target c++ } } */ + /* { dg-begin-multiline-output "" } + takes_int(int_ptr); + ^~~~~~~ + * + { dg-end-multiline-output "" } */ + /* { dg-begin-multiline-output "" } + void takes_int(int); + ^~~ + { dg-end-multiline-output "" } */ +} + +void test_7 (void) +{ + takes_int(char_ptr); /* { dg-error "" } */ + + /* Don't expect a '*' fix-it hint. */ + /* { dg-begin-multiline-output "" } + takes_int(char_ptr); + ^~~~~~~~ + | + char * + { dg-end-multiline-output "" { target c } } */ + /* { dg-begin-multiline-output "" } + takes_int(char_ptr); + ^~~~~~~~ + | + char* + { dg-end-multiline-output "" { target c++ } } */ + /* { dg-begin-multiline-output "" } + void takes_int(int); + ^~~ + { dg-end-multiline-output "" } */ +} + +void test_8 (void) +{ + ivar = int_ptr; /* { dg-error "" } */ + + /* Expect a fix-it hint from the C++ FE, but not from C (due to missing + location). */ + /* { dg-begin-multiline-output "" } + ivar = int_ptr; + ^~~~~~~ + | + int* + { dg-end-multiline-output "" { target c++ } } */ + /* { dg-begin-multiline-output "" } + ivar = int_ptr; + ^~~~~~~ + * + { dg-end-multiline-output "" { target c++ } } */ + /* { dg-begin-multiline-output "" } + ivar = int_ptr; + ^ + { dg-end-multiline-output "" { target c } } */ +} + +void test_9 (void) +{ + cvar = int_ptr; /* { dg-error "" } */ + + /* Don't expect a '*' fix-it hint. */ + /* { dg-begin-multiline-output "" } + cvar = int_ptr; + ^~~~~~~ + | + int* + { dg-end-multiline-output "" { target c++ } } */ + /* { dg-begin-multiline-output "" } + cvar = int_ptr; + ^ + { dg-end-multiline-output "" { target c } } */ +} + +void test_10 (void) +{ + int_ptr = ivar; /* { dg-error "" } */ + + /* Expect a fix-it hint from the C++ FE, but not from C (due to missing + location). */ + /* { dg-begin-multiline-output "" } + int_ptr = ivar; + ^~~~ + | + int + { dg-end-multiline-output "" { target c++ } } */ + /* { dg-begin-multiline-output "" } + int_ptr = ivar; + ^~~~ + & + { dg-end-multiline-output "" { target c++ } } */ + /* { dg-begin-multiline-output "" } + int_ptr = ivar; + ^ + { dg-end-multiline-output "" { target c } } */ +} + +void test_11 (void) +{ + char_ptr = ivar; /* { dg-error "" } */ + + /* Don't expect a fix-it hint, due to mismatching types. */ + /* { dg-begin-multiline-output "" } + char_ptr = ivar; + ^~~~ + | + int + { dg-end-multiline-output "" { target c++ } } */ + /* { dg-begin-multiline-output "" } + char_ptr = ivar; + ^ + { dg-end-multiline-output "" { target c } } */ +} + +/* We shouldn't offer '&' fix-it hints for non-lvalues. */ + +void test_12 (void) +{ + takes_int_ptr (returns_int ()); /* { dg-error "" } */ + + /* { dg-begin-multiline-output "" } + takes_int_ptr (returns_int ()); + ^~~~~~~~~~~~~~ + | + int + { dg-end-multiline-output "" { target c } } */ + /* { dg-begin-multiline-output "" } + takes_int_ptr (returns_int ()); + ~~~~~~~~~~~~^~ + | + int + { dg-end-multiline-output "" { target c++ } } */ + /* { dg-begin-multiline-output "" } + void takes_int_ptr(int*); + ^~~~ + { dg-end-multiline-output "" } */ +} + +/* Ignore typedefs when offering fix-it hints. */ + +typedef int typedef_int_t; +typedef_int_t typedef_int_t_var; + +void test_13 (void) +{ + takes_int_ptr (typedef_int_t_var); /* { dg-error "" } */ + /* { dg-message "possible fix: take the address with '&'" "" { target *-*-* } .-1 } */ + + /* Expect an '&' fix-it hint. */ + /* { dg-begin-multiline-output "" } + takes_int_ptr (typedef_int_t_var); + ^~~~~~~~~~~~~~~~~ + | + typedef_int_t {aka int} + { dg-end-multiline-output "" } */ + /* { dg-begin-multiline-output "" } + takes_int_ptr (typedef_int_t_var); + ^~~~~~~~~~~~~~~~~ + & + { dg-end-multiline-output "" } */ + /* { dg-begin-multiline-output "" } + void takes_int_ptr(int*); + ^~~~ + { dg-end-multiline-output "" } */ +} + +enum foo +{ + FOO_0, + FOO_1, + FOO_2 +}; + +void test_14 (void) +{ + enum foo f; + takes_int_ptr (f); /* { dg-error "" } */ + /* We don't expect a fix-it hint here. */ + /* { dg-begin-multiline-output "" } + takes_int_ptr (f); + ^ + | + enum foo + { dg-end-multiline-output "" { target c } } */ + /* { dg-begin-multiline-output "" } + takes_int_ptr (f); + ^ + | + foo + { dg-end-multiline-output "" { target c++ } } */ + /* { dg-begin-multiline-output "" } + void takes_int_ptr(int*); + ^~~~ + { dg-end-multiline-output "" } */ +} diff --git a/gcc/testsuite/g++.dg/template/error60.C b/gcc/testsuite/g++.dg/template/error60.C index 8c2139b207c4..7d578b166cb3 100644 --- a/gcc/testsuite/g++.dg/template/error60.C +++ b/gcc/testsuite/g++.dg/template/error60.C @@ -35,3 +35,8 @@ void usage () | int { dg-end-multiline-output "" } */ + /* { dg-begin-multiline-output "" } + my_pointer<Foo> ptr (val); + ^~~ + & + { dg-end-multiline-output "" } */ diff --git a/gcc/testsuite/g++.dg/template/error60a.C b/gcc/testsuite/g++.dg/template/error60a.C index 9d0f170c18ba..41af458d3f07 100644 --- a/gcc/testsuite/g++.dg/template/error60a.C +++ b/gcc/testsuite/g++.dg/template/error60a.C @@ -44,3 +44,8 @@ void usage () | int { dg-end-multiline-output "" } */ + /* { dg-begin-multiline-output "" } + my_pointer<Foo> ptr (val); + ^~~ + & + { dg-end-multiline-output "" } */ -- 2.26.3
