On 11/9/18, David Malcolm <[email protected]> wrote:
> This patch adds 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: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}
> | &
Having both the type and the fixit underneath the caret looks kind of confusing
> 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,
> | ~~~~~~~~~~~~~~~^~~~~
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
>
> gcc/c-family/ChangeLog:
> PR c++/87850
> * c-common.c (maybe_add_indirection_token): New function.
> * c-common.h (maybe_add_indirection_token): New decl.
>
> gcc/c/ChangeLog:
> PR c++/87850
> * c-typeck.c (convert_for_assignment): Call
> maybe_add_indirection_token for pointer vs non-pointer
> diagnostics.
>
> gcc/cp/ChangeLog:
> PR c++/87850
> * call.c (convert_like_real): Call maybe_add_indirection_token
> for "invalid conversion" diagnostic.
>
> gcc/testsuite/ChangeLog:
> PR c++/87850
> * c-c++-common/indirection-fixits.c: New test.
> ---
> gcc/c-family/c-common.c | 25 +++
> gcc/c-family/c-common.h | 2 +
> gcc/c/c-typeck.c | 2 +
> gcc/cp/call.c | 1 +
> gcc/testsuite/c-c++-common/indirection-fixits.c | 250
> ++++++++++++++++++++++++
> 5 files changed, 280 insertions(+)
> create mode 100644 gcc/testsuite/c-c++-common/indirection-fixits.c
>
> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> index 534d928..3756e6d 100644
> --- a/gcc/c-family/c-common.c
> +++ b/gcc/c-family/c-common.c
> @@ -8382,6 +8382,31 @@ maybe_suggest_missing_token_insertion (rich_location
> *richloc,
> }
> }
>
> +/* Potentially add a fix-it hint for a missing '&' or '*' to RICHLOC,
> + depending on EXPR and EXPECTED_TYPE. */
> +
> +void
> +maybe_add_indirection_token (rich_location *richloc,
> + tree expr, tree expected_type)
> +{
> + gcc_assert (richloc);
> + gcc_assert (expr);
> + gcc_assert (expected_type);
> +
> + tree actual_type = TREE_TYPE (expr);
> +
> + /* Fix-it hint for missing '&'. */
> + if (TREE_CODE (expected_type) == POINTER_TYPE
> + && actual_type == TREE_TYPE (expected_type)
> + && lvalue_p (expr))
> + richloc->add_fixit_insert_before ("&");
> +
> + /* Fix-it hint for missing '*'. */
> + if (TREE_CODE (actual_type) == POINTER_TYPE
> + && TREE_TYPE (actual_type) == expected_type)
> + richloc->add_fixit_insert_before ("*");
> +}
> +
> #if CHECKING_P
>
> namespace selftest {
> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> index a218432..e362137 100644
> --- a/gcc/c-family/c-common.h
> +++ b/gcc/c-family/c-common.h
> @@ -1340,6 +1340,8 @@ 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_add_indirection_token (rich_location *richloc,
> + tree expr, tree expected_type);
> extern tree braced_list_to_string (tree, tree);
>
> #if CHECKING_P
> diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> index 144977e..f407025 100644
> --- a/gcc/c/c-typeck.c
> +++ b/gcc/c/c-typeck.c
> @@ -7058,6 +7058,7 @@ convert_for_assignment (location_t location,
> location_t expr_loc, tree type,
> auto_diagnostic_group d;
> range_label_for_type_mismatch rhs_label (rhstype, type);
> gcc_rich_location richloc (expr_loc, &rhs_label);
> + maybe_add_indirection_token (&richloc, rhs, type);
> if (pedwarn (&richloc, OPT_Wint_conversion,
> "passing argument %d of %qE makes pointer from "
> "integer without a cast", parmnum, rname))
> @@ -7094,6 +7095,7 @@ convert_for_assignment (location_t location,
> location_t expr_loc, tree type,
> auto_diagnostic_group d;
> range_label_for_type_mismatch rhs_label (rhstype, type);
> gcc_rich_location richloc (expr_loc, &rhs_label);
> + maybe_add_indirection_token (&richloc, rhs, type);
> if (pedwarn (&richloc, OPT_Wint_conversion,
> "passing argument %d of %qE makes integer from "
> "pointer without a cast", parmnum, rname))
> diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> index 6f40156..85d722c 100644
> --- a/gcc/cp/call.c
> +++ b/gcc/cp/call.c
> @@ -6814,6 +6814,7 @@ convert_like_real (conversion *convs, tree expr, tree
> fn, int argnum,
> {
> range_label_for_type_mismatch label (TREE_TYPE (expr), totype);
> gcc_rich_location richloc (loc, &label);
> + maybe_add_indirection_token (&richloc, expr, totype);
> complained = permerror (&richloc,
> "invalid conversion from %qH to %qI",
> TREE_TYPE (expr), totype);
> 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 0000000..2dbd8b2
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/indirection-fixits.c
> @@ -0,0 +1,250 @@
> +/* { 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-warning "" "" { target c } } */
> + /* { dg-error "" "" { target c++ } .-1 } */
> +
> + /* Expect an '&' fix-it hint. */
> + /* { dg-begin-multiline-output "" }
> + takes_int_ptr(ivar);
> + ^~~~
> + |
> + int
> + &
> + { 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-warning "" "" { target c } } */
> + /* { dg-error "" "" { target c++ } .-1 } */
> +
> + /* 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-warning "" "" { target c } } */
> + /* { dg-error "" "" { target c++ } .-1 } */
> +
> + /* 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-warning "" "" { target c } } */
> + /* { dg-error "" "" { target c++ } .-1 } */
> +
> + /* Expect an '&' fix-it hint. */
> + /* { dg-begin-multiline-output "" }
> + takes_char_ptr(cvar);
> + ^~~~
> + |
> + char
> + &
> + { 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-warning "" "" { target c } } */
> + /* { dg-error "" "" { target c++ } .-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 "" }
> + void takes_int(int);
> + ^~~
> + { dg-end-multiline-output "" } */
> +}
> +
> +void test_7 (void)
> +{
> + takes_int(char_ptr); /* { dg-warning "" "" { target c } } */
> + /* { dg-error "" "" { target c++ } .-1 } */
> +
> + /* 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-warning "" "" { target c } } */
> + /* { dg-error "" "" { target c++ } .-1 } */
> +
> + /* 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 } } */
> +}
> +
> +void test_9 (void)
> +{
> + cvar = int_ptr; /* { dg-warning "" "" { target c } } */
> + /* { dg-error "" "" { target c++ } .-1 } */
> +
> + /* Expect a fix-it hint from the C++ FE, but not from C (due to missing
> + location). */
> + /* { 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-warning "" "" { target c } } */
> + /* { dg-error "" "" { target c++ } .-1 } */
> +
> + /* 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 } } */
> +}
> +
> +void test_11 (void)
> +{
> + char_ptr = ivar; /* { dg-warning "" "" { target c } } */
> + /* { dg-error "" "" { target c++ } .-1 } */
> +
> + /* 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-warning "" "" { target c } } */
> + /* { dg-error "" "" { target c++ } .-1 } */
> +
> + /* { 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 "" } */
> +}
> --
> 1.8.5.3
>
>
