* [PATCH] C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
@ 2018-11-09 22:36 David Malcolm
2018-11-10 7:01 ` Eric Gallager
0 siblings, 1 reply; 20+ messages in thread
From: David Malcolm @ 2018-11-09 22:36 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
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}
| &
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
2018-11-09 22:36 [PATCH] C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850) David Malcolm
@ 2018-11-10 7:01 ` Eric Gallager
2018-11-11 18:01 ` Martin Sebor
0 siblings, 1 reply; 20+ messages in thread
From: Eric Gallager @ 2018-11-10 7:01 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches
On 11/9/18, David Malcolm <dmalcolm@redhat.com> 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
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
2018-11-10 7:01 ` Eric Gallager
@ 2018-11-11 18:01 ` Martin Sebor
2018-11-11 21:02 ` David Malcolm
0 siblings, 1 reply; 20+ messages in thread
From: Martin Sebor @ 2018-11-11 18:01 UTC (permalink / raw)
To: Eric Gallager, David Malcolm; +Cc: gcc-patches
On 11/10/2018 12:01 AM, Eric Gallager wrote:
> On 11/9/18, David Malcolm <dmalcolm@redhat.com> 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
I agree it's rather subtle. Keeping the diagnostics separate from
the suggested fix should avoid the confusion.
Martin
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
2018-11-11 18:01 ` Martin Sebor
@ 2018-11-11 21:02 ` David Malcolm
2018-11-12 21:32 ` Martin Sebor
0 siblings, 1 reply; 20+ messages in thread
From: David Malcolm @ 2018-11-11 21:02 UTC (permalink / raw)
To: Martin Sebor, Eric Gallager; +Cc: gcc-patches
On Sun, 2018-11-11 at 11:01 -0700, Martin Sebor wrote:
> On 11/10/2018 12:01 AM, Eric Gallager wrote:
> > On 11/9/18, David Malcolm <dmalcolm@redhat.com> 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
>
> I agree it's rather subtle. Keeping the diagnostics separate from
> the suggested fix should avoid the confusion.
FWIW, the fix-it hint is in a different color (assuming that gcc is
invoked in an environment that prints that...)
Dave
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
2018-11-11 21:02 ` David Malcolm
@ 2018-11-12 21:32 ` Martin Sebor
2018-11-13 21:34 ` Jason Merrill
0 siblings, 1 reply; 20+ messages in thread
From: Martin Sebor @ 2018-11-12 21:32 UTC (permalink / raw)
To: David Malcolm, Eric Gallager; +Cc: gcc-patches
On 11/11/2018 02:02 PM, David Malcolm wrote:
> On Sun, 2018-11-11 at 11:01 -0700, Martin Sebor wrote:
>> On 11/10/2018 12:01 AM, Eric Gallager wrote:
>>> On 11/9/18, David Malcolm <dmalcolm@redhat.com> 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
>>
>> I agree it's rather subtle. Keeping the diagnostics separate from
>> the suggested fix should avoid the confusion.
>
> FWIW, the fix-it hint is in a different color (assuming that gcc is
> invoked in an environment that prints that...)
I figured it would be, but I'm still not sure it's good design
to be relying on color alone to distinguish between the problem
and the suggested fix. Especially when they are so close to one
another and the fix is just a single character with no obvious
relationship to the rest of the text on the screen. In other
warnings there's at least the "did you forget the '@'?" part
to give a clue, even though even there the connection between
the "did you forget" and the & several lines down wouldn't
necessarily be immediately apparent.
I'm not an expert on these things so I'm going strictly by
intuition and my personal bias. Are there any user interface
guidelines that you like to refer to that speak to this? (Other
than our own or the blog post you referenced in one of your posts.)
Back in my GUI days, I remember reading articles and even whole
books on how to design good interfaces, including layouts and
the use of color (e.g., for the color-blind). It seems like
there should be something relevant for the command line as well,
at least some basic principles that we could apply.
Martin
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
2018-11-12 21:32 ` Martin Sebor
@ 2018-11-13 21:34 ` Jason Merrill
2018-11-15 21:48 ` [PATCH] v2: " David Malcolm
0 siblings, 1 reply; 20+ messages in thread
From: Jason Merrill @ 2018-11-13 21:34 UTC (permalink / raw)
To: Martin Sebor; +Cc: David Malcolm, Eric Gallager, gcc-patches List
On Mon, Nov 12, 2018 at 4:32 PM Martin Sebor <msebor@gmail.com> wrote:
> On 11/11/2018 02:02 PM, David Malcolm wrote:
> > On Sun, 2018-11-11 at 11:01 -0700, Martin Sebor wrote:
> >> On 11/10/2018 12:01 AM, Eric Gallager wrote:
> >>> On 11/9/18, David Malcolm <dmalcolm@redhat.com> 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
> >>
> >> I agree it's rather subtle. Keeping the diagnostics separate from
> >> the suggested fix should avoid the confusion.
> >
> > FWIW, the fix-it hint is in a different color (assuming that gcc is
> > invoked in an environment that prints that...)
>
> I figured it would be, but I'm still not sure it's good design
> to be relying on color alone to distinguish between the problem
> and the suggested fix. Especially when they are so close to one
> another and the fix is just a single character with no obvious
> relationship to the rest of the text on the screen. In other
> warnings there's at least the "did you forget the '@'?" part
> to give a clue, even though even there the connection between
> the "did you forget" and the & several lines down wouldn't
> necessarily be immediately apparent.
Agreed, something along those lines would help to understand why the
compiler is throwing a random & into the diagnostic.
Jason
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] v2: C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
2018-11-13 21:34 ` Jason Merrill
@ 2018-11-15 21:48 ` David Malcolm
2018-11-16 18:13 ` Jason Merrill
0 siblings, 1 reply; 20+ messages in thread
From: David Malcolm @ 2018-11-15 21:48 UTC (permalink / raw)
To: Jason Merrill
Cc: Martin Sebor, Eric Gallager, gcc-patches List, David Malcolm
On Tue, 2018-11-13 at 16:34 -0500, Jason Merrill wrote:
> On Mon, Nov 12, 2018 at 4:32 PM Martin Sebor <msebor@gmail.com>
> wrote:
> > On 11/11/2018 02:02 PM, David Malcolm wrote:
> > > On Sun, 2018-11-11 at 11:01 -0700, Martin Sebor wrote:
> > > > On 11/10/2018 12:01 AM, Eric Gallager wrote:
> > > > > On 11/9/18, David Malcolm <dmalcolm@redhat.com> 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
> > > >
> > > > I agree it's rather subtle. Keeping the diagnostics separate
> > > > from
> > > > the suggested fix should avoid the confusion.
> > >
> > > FWIW, the fix-it hint is in a different color (assuming that gcc
> > > is
> > > invoked in an environment that prints that...)
> >
> > I figured it would be, but I'm still not sure it's good design
> > to be relying on color alone to distinguish between the problem
> > and the suggested fix. Especially when they are so close to one
> > another and the fix is just a single character with no obvious
> > relationship to the rest of the text on the screen. In other
> > warnings there's at least the "did you forget the '@'?" part
> > to give a clue, even though even there the connection between
> > the "did you forget" and the & several lines down wouldn't
> > necessarily be immediately apparent.
>
> Agreed, something along those lines would help to understand why the
> compiler is throwing a random & into the diagnostic.
>
> Jason
Here's an updated version which adds a note, putting the fix-it hint
on that instead (I attempted adding the text to the initial error,
but there was something of a combinatorial explosion of messages).
The above example becomes:
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,
| ~~~~~~~~~~~~~~~^~~~~
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for trunk?
gcc/c-family/ChangeLog:
PR c++/87850
* c-common.c: Include "gcc-rich-location.h".
(maybe_emit_indirection_note): New function.
* c-common.h (maybe_emit_indirection_note): New decl.
gcc/c/ChangeLog:
PR c++/87850
* c-typeck.c (convert_for_assignment): Call
maybe_emit_indirection_note for pointer vs non-pointer
diagnostics.
gcc/cp/ChangeLog:
PR c++/87850
* call.c (convert_like_real): Call
maybe_emit_indirection_note for "invalid conversion" diagnostic.
gcc/testsuite/ChangeLog:
PR c++/87850
* c-c++-common/indirection-fixits.c: New test.
---
gcc/c-family/c-common.c | 33 +++
gcc/c-family/c-common.h | 2 +
gcc/c/c-typeck.c | 10 +-
gcc/cp/call.c | 2 +
gcc/testsuite/c-c++-common/indirection-fixits.c | 270 ++++++++++++++++++++++++
5 files changed, 315 insertions(+), 2 deletions(-)
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 cd88f3a..d5c7c7f 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -48,6 +48,7 @@ along with GCC; see the file COPYING3. If not see
#include "gimplify.h"
#include "substring-locations.h"
#include "spellcheck.h"
+#include "gcc-rich-location.h"
#include "selftest.h"
cpp_reader *parse_in; /* Declared in c-pragma.h. */
@@ -8405,6 +8406,38 @@ 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
+ && 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
+ && 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 31cc273..e765d3c 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_emit_indirection_note (location_t loc,
+ 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 5d4e973..efa836d 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -7061,7 +7061,10 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
if (pedwarn (&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:
@@ -7097,7 +7100,10 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
if (pedwarn (&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.c b/gcc/cp/call.c
index ee099cc..a25d109 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6862,6 +6862,8 @@ convert_like_real (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 (complained && fn)
inform (get_fndecl_argument_location (fn, argnum),
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..4e45d66
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/indirection-fixits.c
@@ -0,0 +1,270 @@
+/* { 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 } */
+ /* { dg-message "possible fix: take the address with '&'" "" { target *-*-* } .-2 } */
+
+ /* 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-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 "" }
+ 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-warning "" "" { target c } } */
+ /* { dg-error "" "" { target c++ } .-1 } */
+ /* { dg-message "possible fix: dereference with '*'" "" { target *-*-* } .-2 } */
+
+ /* 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-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++ } } */
+ /* { 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 } */
+
+ /* 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-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++ } } */
+ /* { 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] v2: C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
2018-11-15 21:48 ` [PATCH] v2: " David Malcolm
@ 2018-11-16 18:13 ` Jason Merrill
2018-11-19 21:23 ` [PATCH] v3: " David Malcolm
0 siblings, 1 reply; 20+ messages in thread
From: Jason Merrill @ 2018-11-16 18:13 UTC (permalink / raw)
To: David Malcolm; +Cc: Martin Sebor, Eric Gallager, gcc-patches List
On Thu, Nov 15, 2018 at 4:48 PM David Malcolm <dmalcolm@redhat.com> wrote:
> On Tue, 2018-11-13 at 16:34 -0500, Jason Merrill wrote:
> > On Mon, Nov 12, 2018 at 4:32 PM Martin Sebor <msebor@gmail.com>
> > wrote:
> > > On 11/11/2018 02:02 PM, David Malcolm wrote:
> > > > On Sun, 2018-11-11 at 11:01 -0700, Martin Sebor wrote:
> > > > > On 11/10/2018 12:01 AM, Eric Gallager wrote:
> > > > > > On 11/9/18, David Malcolm <dmalcolm@redhat.com> 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
> > > > >
> > > > > I agree it's rather subtle. Keeping the diagnostics separate
> > > > > from
> > > > > the suggested fix should avoid the confusion.
> > > >
> > > > FWIW, the fix-it hint is in a different color (assuming that gcc
> > > > is
> > > > invoked in an environment that prints that...)
> > >
> > > I figured it would be, but I'm still not sure it's good design
> > > to be relying on color alone to distinguish between the problem
> > > and the suggested fix. Especially when they are so close to one
> > > another and the fix is just a single character with no obvious
> > > relationship to the rest of the text on the screen. In other
> > > warnings there's at least the "did you forget the '@'?" part
> > > to give a clue, even though even there the connection between
> > > the "did you forget" and the & several lines down wouldn't
> > > necessarily be immediately apparent.
> >
> > Agreed, something along those lines would help to understand why the
> > compiler is throwing a random & into the diagnostic.
> >
> > Jason
>
> Here's an updated version which adds a note, putting the fix-it hint
> on that instead (I attempted adding the text to the initial error,
> but there was something of a combinatorial explosion of messages).
>
> The above example becomes:
>
> 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,
> | ~~~~~~~~~~~~~~~^~~~~
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
>
> gcc/c-family/ChangeLog:
> PR c++/87850
> * c-common.c: Include "gcc-rich-location.h".
> (maybe_emit_indirection_note): New function.
> * c-common.h (maybe_emit_indirection_note): New decl.
>
> gcc/c/ChangeLog:
> PR c++/87850
> * c-typeck.c (convert_for_assignment): Call
> maybe_emit_indirection_note for pointer vs non-pointer
> diagnostics.
>
> gcc/cp/ChangeLog:
> PR c++/87850
> * call.c (convert_like_real): Call
> maybe_emit_indirection_note for "invalid conversion" diagnostic.
>
> gcc/testsuite/ChangeLog:
> PR c++/87850
> * c-c++-common/indirection-fixits.c: New test.
> ---
> gcc/c-family/c-common.c | 33 +++
> gcc/c-family/c-common.h | 2 +
> gcc/c/c-typeck.c | 10 +-
> gcc/cp/call.c | 2 +
> gcc/testsuite/c-c++-common/indirection-fixits.c | 270 ++++++++++++++++++++++++
> 5 files changed, 315 insertions(+), 2 deletions(-)
> 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 cd88f3a..d5c7c7f 100644
> --- a/gcc/c-family/c-common.c
> +++ b/gcc/c-family/c-common.c
> @@ -48,6 +48,7 @@ along with GCC; see the file COPYING3. If not see
> #include "gimplify.h"
> #include "substring-locations.h"
> #include "spellcheck.h"
> +#include "gcc-rich-location.h"
> #include "selftest.h"
>
> cpp_reader *parse_in; /* Declared in c-pragma.h. */
> @@ -8405,6 +8406,38 @@ 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
> + && actual_type == TREE_TYPE (expected_type)
Type comparison should use same_type_p, not ==, so that we deal
properly with typedef variants. OK with that change.
Jason
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] v3: C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
2018-11-16 18:13 ` Jason Merrill
@ 2018-11-19 21:23 ` David Malcolm
2018-11-20 2:46 ` Joseph Myers
` (2 more replies)
0 siblings, 3 replies; 20+ messages in thread
From: David Malcolm @ 2018-11-19 21:23 UTC (permalink / raw)
To: Jason Merrill
Cc: Martin Sebor, Eric Gallager, gcc-patches List, David Malcolm
On Fri, 2018-11-16 at 13:13 -0500, Jason Merrill wrote:
> On Thu, Nov 15, 2018 at 4:48 PM David Malcolm <dmalcolm@redhat.com>
> wrote:
> > On Tue, 2018-11-13 at 16:34 -0500, Jason Merrill wrote:
> > > On Mon, Nov 12, 2018 at 4:32 PM Martin Sebor <msebor@gmail.com>
> > > wrote:
> > > > On 11/11/2018 02:02 PM, David Malcolm wrote:
> > > > > On Sun, 2018-11-11 at 11:01 -0700, Martin Sebor wrote:
> > > > > > On 11/10/2018 12:01 AM, Eric Gallager wrote:
> > > > > > > On 11/9/18, David Malcolm <dmalcolm@redhat.com> 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
> > > > > >
> > > > > > I agree it's rather subtle. Keeping the diagnostics
> > > > > > separate
> > > > > > from
> > > > > > the suggested fix should avoid the confusion.
> > > > >
> > > > > FWIW, the fix-it hint is in a different color (assuming that
> > > > > gcc
> > > > > is
> > > > > invoked in an environment that prints that...)
> > > >
> > > > I figured it would be, but I'm still not sure it's good design
> > > > to be relying on color alone to distinguish between the problem
> > > > and the suggested fix. Especially when they are so close to
> > > > one
> > > > another and the fix is just a single character with no obvious
> > > > relationship to the rest of the text on the screen. In other
> > > > warnings there's at least the "did you forget the '@'?" part
> > > > to give a clue, even though even there the connection between
> > > > the "did you forget" and the & several lines down wouldn't
> > > > necessarily be immediately apparent.
> > >
> > > Agreed, something along those lines would help to understand why
> > > the
> > > compiler is throwing a random & into the diagnostic.
> > >
> > > Jason
> >
> > Here's an updated version which adds a note, putting the fix-it
> > hint
> > on that instead (I attempted adding the text to the initial error,
> > but there was something of a combinatorial explosion of messages).
> >
> > The above example becomes:
> >
> > 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,
> > | ~~~~~~~~~~~~~~~^~~~~
> >
> > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
> >
> > OK for trunk?
> >
> > gcc/c-family/ChangeLog:
> > PR c++/87850
> > * c-common.c: Include "gcc-rich-location.h".
> > (maybe_emit_indirection_note): New function.
> > * c-common.h (maybe_emit_indirection_note): New decl.
> >
> > gcc/c/ChangeLog:
> > PR c++/87850
> > * c-typeck.c (convert_for_assignment): Call
> > maybe_emit_indirection_note for pointer vs non-pointer
> > diagnostics.
> >
> > gcc/cp/ChangeLog:
> > PR c++/87850
> > * call.c (convert_like_real): Call
> > maybe_emit_indirection_note for "invalid conversion"
> > diagnostic.
> >
> > gcc/testsuite/ChangeLog:
> > PR c++/87850
> > * c-c++-common/indirection-fixits.c: New test.
> > ---
> > gcc/c-family/c-common.c | 33 +++
> > gcc/c-family/c-common.h | 2 +
> > gcc/c/c-typeck.c | 10 +-
> > gcc/cp/call.c | 2 +
> > gcc/testsuite/c-c++-common/indirection-fixits.c | 270
> > ++++++++++++++++++++++++
> > 5 files changed, 315 insertions(+), 2 deletions(-)
> > 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 cd88f3a..d5c7c7f 100644
> > --- a/gcc/c-family/c-common.c
> > +++ b/gcc/c-family/c-common.c
> > @@ -48,6 +48,7 @@ along with GCC; see the file COPYING3. If not
> > see
> > #include "gimplify.h"
> > #include "substring-locations.h"
> > #include "spellcheck.h"
> > +#include "gcc-rich-location.h"
> > #include "selftest.h"
> >
> > cpp_reader *parse_in; /* Declared in c-pragma.h. */
> > @@ -8405,6 +8406,38 @@ 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
> > + && actual_type == TREE_TYPE (expected_type)
>
> Type comparison should use same_type_p, not ==, so that we deal
> properly with typedef variants. OK with that change.
>
> Jason
The above code is in c-family, but same_type_p is specific to C++,
so the change is not quite trivial.
Here's a v3 of the patch which moves same_type_p from cp/cp-tree.h
to c-family/c-common.h, converting it from a macro to an extern decl,
with implementations for C and for C++. I used:
comptypes (type1, type2) == 1
for the C implementation of same_type_p.
The v3 patch uses same_type_p rather than pointer equality, and I added
a test case for a typedef of int vs a int.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Is this OK for trunk?
Thanks
Dave
gcc/c-family/ChangeLog:
PR c++/87850
* c-common.c: Include "gcc-rich-location.h".
(maybe_emit_indirection_note): New function.
* c-common.h (maybe_emit_indirection_note): New decl.
(same_type_p): New decl
gcc/c/ChangeLog:
PR c++/87850
* c-typeck.c (same_type_p): New function.
(convert_for_assignment): Call maybe_emit_indirection_note for
pointer vs non-pointer diagnostics.
gcc/cp/ChangeLog:
PR c++/87850
* call.c (convert_like_real): Call
maybe_emit_indirection_note for "invalid conversion" diagnostic.
* cp-tree.h (same_type_p): Delete macro, moving to...
* typeck.c (same_type_p): New function.
gcc/testsuite/ChangeLog:
PR c++/87850
* c-c++-common/indirection-fixits.c: New test.
---
gcc/c-family/c-common.c | 33 +++
gcc/c-family/c-common.h | 4 +
gcc/c/c-typeck.c | 20 +-
gcc/cp/call.c | 2 +
gcc/cp/cp-tree.h | 5 -
gcc/cp/typeck.c | 10 +
gcc/testsuite/c-c++-common/indirection-fixits.c | 299 ++++++++++++++++++++++++
7 files changed, 366 insertions(+), 7 deletions(-)
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 cd88f3a..babe329 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -48,6 +48,7 @@ along with GCC; see the file COPYING3. If not see
#include "gimplify.h"
#include "substring-locations.h"
#include "spellcheck.h"
+#include "gcc-rich-location.h"
#include "selftest.h"
cpp_reader *parse_in; /* Declared in c-pragma.h. */
@@ -8405,6 +8406,38 @@ 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
+ && same_type_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
+ && same_type_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 31cc273..e0be104 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1340,8 +1340,12 @@ 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 tree braced_list_to_string (tree, tree);
+extern bool same_type_p (tree type1, tree type2);
+
#if CHECKING_P
namespace selftest {
/* Declarations for specific families of tests within c-family,
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 5d4e973..1ddcf71 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -1003,6 +1003,16 @@ common_type (tree t1, tree t2)
return c_common_type (t1, t2);
}
+/* C implementation of same_type_p.
+ Returns true iff TYPE1 and TYPE2 are the same type, in the usual
+ sense of `same'. */
+
+bool
+same_type_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. */
@@ -7061,7 +7071,10 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
if (pedwarn (&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:
@@ -7097,7 +7110,10 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
if (pedwarn (&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.c b/gcc/cp/call.c
index ee099cc..a25d109 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6862,6 +6862,8 @@ convert_like_real (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 (complained && fn)
inform (get_fndecl_argument_location (fn, argnum),
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index f2e6709..57d9eed 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -671,11 +671,6 @@ typedef struct ptrmem_cst * ptrmem_cst_t;
#define REINTERPRET_CAST_P(NODE) \
TREE_LANG_FLAG_0 (NOP_EXPR_CHECK (NODE))
-/* Returns nonzero iff TYPE1 and TYPE2 are the same type, in the usual
- sense of `same'. */
-#define same_type_p(TYPE1, TYPE2) \
- comptypes ((TYPE1), (TYPE2), COMPARE_STRICT)
-
/* Returns nonzero iff NODE is a declaration for the global function
`main'. */
#define DECL_MAIN_P(NODE) \
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 7b42d53..eeae9f1 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -1448,6 +1448,16 @@ structural_comptypes (tree t1, tree t2, int strict)
return comp_type_attributes (t1, t2);
}
+/* C++ implementation of same_type_p.
+ Returns true iff TYPE1 and TYPE2 are the same type, in the usual
+ sense of `same'. */
+
+bool
+same_type_p (tree type1, tree type2)
+{
+ return comptypes (type1, type2, COMPARE_STRICT);
+}
+
/* 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 0000000..8987ee9
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/indirection-fixits.c
@@ -0,0 +1,299 @@
+/* { 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 } */
+ /* { dg-message "possible fix: take the address with '&'" "" { target *-*-* } .-2 } */
+
+ /* 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-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 "" }
+ 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-warning "" "" { target c } } */
+ /* { dg-error "" "" { target c++ } .-1 } */
+ /* { dg-message "possible fix: dereference with '*'" "" { target *-*-* } .-2 } */
+
+ /* 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-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++ } } */
+ /* { 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 } */
+
+ /* 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-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++ } } */
+ /* { 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 "" } */
+}
+
+/* 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-warning "" "" { target c } } */
+ /* { dg-error "" "" { target c++ } .-1 } */
+ /* { dg-message "possible fix: take the address with '&'" "" { target *-*-* } .-2 } */
+
+ /* 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 "" } */
+}
--
1.8.5.3
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] v3: C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
2018-11-19 21:23 ` [PATCH] v3: " David Malcolm
@ 2018-11-20 2:46 ` Joseph Myers
2018-11-20 22:05 ` David Malcolm
2018-11-21 0:36 ` [PATCH] v3: " Jeff Law
2018-11-21 0:39 ` Martin Sebor
2 siblings, 1 reply; 20+ messages in thread
From: Joseph Myers @ 2018-11-20 2:46 UTC (permalink / raw)
To: David Malcolm
Cc: Jason Merrill, Martin Sebor, Eric Gallager, gcc-patches List
On Mon, 19 Nov 2018, David Malcolm wrote:
> +/* C implementation of same_type_p.
> + Returns true iff TYPE1 and TYPE2 are the same type, in the usual
> + sense of `same'. */
> +
> +bool
> +same_type_p (tree type1, tree type2)
> +{
> + return comptypes (type1, type2) == 1;
> +}
I don't think "compatible" and "same" are the same concept. Normally in C
you'd be concerned with compatibility; "same type" is only used for the
rule on duplicate typedefs, which uses comptypes_check_different_types.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] v3: C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
2018-11-20 2:46 ` Joseph Myers
@ 2018-11-20 22:05 ` David Malcolm
2018-11-20 22:23 ` Joseph Myers
0 siblings, 1 reply; 20+ messages in thread
From: David Malcolm @ 2018-11-20 22:05 UTC (permalink / raw)
To: Joseph Myers; +Cc: Jason Merrill, Martin Sebor, Eric Gallager, gcc-patches List
On Tue, 2018-11-20 at 02:46 +0000, Joseph Myers wrote:
> On Mon, 19 Nov 2018, David Malcolm wrote:
>
> > +/* C implementation of same_type_p.
> > + Returns true iff TYPE1 and TYPE2 are the same type, in the
> > usual
> > + sense of `same'. */
> > +
> > +bool
> > +same_type_p (tree type1, tree type2)
> > +{
> > + return comptypes (type1, type2) == 1;
> > +}
>
> I don't think "compatible" and "same" are the same concept. Normally
> in C
> you'd be concerned with compatibility; "same type" is only used for
> the
> rule on duplicate typedefs, which uses
> comptypes_check_different_types.
The purpose here is to be able to offer fix-it hints for bogus code
that's missing an '&' or a '*' prefix, and have that code live in c-
common.c
Jason wanted to avoid a pointer-equality test for types by using
same_type_p to look through enums - but same_type_p is C++-specific.
Should I do:
(a) something like this for C:
/* C implementation of same_type_p.
Returns true iff TYPE1 and TYPE2 are the same type, or are
compatible enough to be permitted in C11 typedef redeclarations. */
bool
same_type_p (tree type1, tree type2)
{
bool different_types_p = false;
int result = comptypes_check_different_types (type1, type2,
&different_types_p);
if (result == 1 && !different_types_p)
return true;
return false;
}
(b) provide a same_type_p for C that e.g. simply does pointer equality,
(d) add a newly named function (e.g. "compatible_types_p", as C++ has a
comptypes, but it has a 3rd param), or
(d) fall back to simply doing pointer equality.
Thanks
Dave
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] v3: C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
2018-11-20 22:05 ` David Malcolm
@ 2018-11-20 22:23 ` Joseph Myers
2018-11-30 23:01 ` [PATCH] v4: " David Malcolm
0 siblings, 1 reply; 20+ messages in thread
From: Joseph Myers @ 2018-11-20 22:23 UTC (permalink / raw)
To: David Malcolm
Cc: Jason Merrill, Martin Sebor, Eric Gallager, gcc-patches List
On Tue, 20 Nov 2018, David Malcolm wrote:
> Should I do:
You should do whatever is appropriate for the warning in question. But if
what's appropriate for the warning in question includes types that are
compatible but not the same, the comments need to avoid saying it's about
the types being the same.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] v3: C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
2018-11-19 21:23 ` [PATCH] v3: " David Malcolm
2018-11-20 2:46 ` Joseph Myers
@ 2018-11-21 0:36 ` Jeff Law
2018-11-21 0:39 ` Martin Sebor
2 siblings, 0 replies; 20+ messages in thread
From: Jeff Law @ 2018-11-21 0:36 UTC (permalink / raw)
To: David Malcolm, Jason Merrill
Cc: Martin Sebor, Eric Gallager, gcc-patches List
On 11/19/18 2:23 PM, David Malcolm wrote:
[ Snip ]
>
> The above code is in c-family, but same_type_p is specific to C++,
> so the change is not quite trivial.
>
> Here's a v3 of the patch which moves same_type_p from cp/cp-tree.h
> to c-family/c-common.h, converting it from a macro to an extern decl,
> with implementations for C and for C++. I used:
> comptypes (type1, type2) == 1
> for the C implementation of same_type_p.
>
> The v3 patch uses same_type_p rather than pointer equality, and I added
> a test case for a typedef of int vs a int.
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>
> Is this OK for trunk?
>
> Thanks
> Dave
>
> gcc/c-family/ChangeLog:
> PR c++/87850
> * c-common.c: Include "gcc-rich-location.h".
> (maybe_emit_indirection_note): New function.
> * c-common.h (maybe_emit_indirection_note): New decl.
> (same_type_p): New decl
>
> gcc/c/ChangeLog:
> PR c++/87850
> * c-typeck.c (same_type_p): New function.
> (convert_for_assignment): Call maybe_emit_indirection_note for
> pointer vs non-pointer diagnostics.
>
> gcc/cp/ChangeLog:
> PR c++/87850
> * call.c (convert_like_real): Call
> maybe_emit_indirection_note for "invalid conversion" diagnostic.
> * cp-tree.h (same_type_p): Delete macro, moving to...
> * typeck.c (same_type_p): New function.
>
> gcc/testsuite/ChangeLog:
> PR c++/87850
> * c-c++-common/indirection-fixits.c: New test.
OK.
jeff
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] v3: C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
2018-11-19 21:23 ` [PATCH] v3: " David Malcolm
2018-11-20 2:46 ` Joseph Myers
2018-11-21 0:36 ` [PATCH] v3: " Jeff Law
@ 2018-11-21 0:39 ` Martin Sebor
2018-11-30 15:27 ` David Malcolm
2 siblings, 1 reply; 20+ messages in thread
From: Martin Sebor @ 2018-11-21 0:39 UTC (permalink / raw)
To: David Malcolm, Jason Merrill; +Cc: Eric Gallager, gcc-patches List
> +void test_2 (void)
> +{
> + takes_int_ptr(ivar); /* { dg-warning "" "" { target c } } */
> + /* { dg-error "" "" { target c++ } .-1 } */
> + /* { dg-message "possible fix: take the address with '&'" "" { target *-*-* } .-2 } */
> +
> + /* 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);
> + ^~~~
> + &
I experimented with adding hints to sizeof_pointer_memaccess_warning
over the weekend and ran into a location difference between the two
front-ends. In an attempt to resolve it, rather than using
an "insertion hint" like I think you did above, I used a replacement
hint. And although it started out as a workaround I ended up liking
the result better. What I think the same approach would result in
here is something like:
takes_int_ptr(ivar);
note: possible fix: take the address with '&'
note: takes_int_ptr(ivar);
^~~~
|
int
note: takes_int_ptr(ivar);
^~~~
&ivar
Have you considered this style, i.e., printing valid expressions
in the hints when possible rather than just operators? It solves
the problem of the lone operators looking a little too terse and
cryptic.
I realize it's not always possible (not every fix-it hint is for
a bad expression) but I'm wondering if it would be worth using
when it is.
Martin
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] v3: C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
2018-11-21 0:39 ` Martin Sebor
@ 2018-11-30 15:27 ` David Malcolm
0 siblings, 0 replies; 20+ messages in thread
From: David Malcolm @ 2018-11-30 15:27 UTC (permalink / raw)
To: Martin Sebor, Jason Merrill; +Cc: Eric Gallager, gcc-patches List
On Tue, 2018-11-20 at 17:39 -0700, Martin Sebor wrote:
> > +void test_2 (void)
> > +{
> > + takes_int_ptr(ivar); /* { dg-warning "" "" { target c } } */
> > + /* { dg-error "" "" { target c++ } .-1 } */
> > + /* { dg-message "possible fix: take the address with '&'" "" {
> > target *-*-* } .-2 } */
> > +
> > + /* 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);
> > + ^~~~
> > + &
>
> I experimented with adding hints to sizeof_pointer_memaccess_warning
> over the weekend and ran into a location difference between the two
> front-ends. In an attempt to resolve it, rather than using
> an "insertion hint" like I think you did above, I used a replacement
> hint. And although it started out as a workaround I ended up liking
> the result better. What I think the same approach would result in
> here is something like:
>
> takes_int_ptr(ivar);
> note: possible fix: take the address with '&'
> note: takes_int_ptr(ivar);
> ^~~~
> |
> int
> note: takes_int_ptr(ivar);
> ^~~~
> &ivar
>
> Have you considered this style, i.e., printing valid expressions
> in the hints when possible rather than just operators? It solves
> the problem of the lone operators looking a little too terse and
> cryptic.
>
> I realize it's not always possible (not every fix-it hint is for
> a bad expression) but I'm wondering if it would be worth using
> when it is.
I definitely prefer the style of the output in your example, but I
think there's a "data model vs presentation" thing going on here.
I'd prefer to avoid the underlying rich_location using replacement when
we mean insertion, for the same reasons as for deletion discussed at
[1]
I think the issue here is with the presentation of insertion fix-it
hints in diagnostic-show-locus.c: there's currently no way for the user
to tell if this:
takes_int_ptr(ivar);
^~~~
&
means:
(a) replace "ivar" with "&", or
(b) insert "&" in front of "ivar" (the intended behavior)
and this alternative presentation would definitely be clearer:
takes_int_ptr(ivar);
^~~~
&ivar
I can have a look at reimplementing how we present insertions in
diagnostic-show-locus.c.
Dave
[1] https://gcc.gnu.org/onlinedocs/gccint/Guidelines-for-Diagnostics.html#Express-deletion-in-terms-of-deletion_002c-not-replacement
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] v4: C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
2018-11-20 22:23 ` Joseph Myers
@ 2018-11-30 23:01 ` David Malcolm
2018-12-01 18:38 ` Jason Merrill
0 siblings, 1 reply; 20+ messages in thread
From: David Malcolm @ 2018-11-30 23:01 UTC (permalink / raw)
To: Joseph Myers, Jason Merrill, law
Cc: Martin Sebor, Eric Gallager, gcc-patches List, David Malcolm
On Tue, 2018-11-20 at 22:23 +0000, Joseph Myers wrote:
> On Tue, 20 Nov 2018, David Malcolm wrote:
>
> > Should I do:
>
> You should do whatever is appropriate for the warning in
> question. But if
> what's appropriate for the warning in question includes types that
> are
> compatible but not the same, the comments need to avoid saying it's
> about
> the types being the same.
Thanks.
Here's an updated version of the patch. I added a new
compatible_types_for_indirection_note_p
specifically for use by maybe_emit_indirection_note, and provided
implementations for C and C++.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for trunk?
Changed in v4: introduce a compatible_types_for_indirection_note_p
function, and use it in place of same_type_p.
Changed in v3: use same_type_p rather than pointer equality.
Provide implementation of same_type_p for C.
Changed in v2: add a note, and put the fix-it hint on that instead
Blurb follows:
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.c: 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.c (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.c (convert_like_real): Call
maybe_emit_indirection_note for "invalid conversion" diagnostic.
* typeck.c (compatible_types_for_indirection_note_p): New
function.
gcc/testsuite/ChangeLog:
PR c++/87850
* c-c++-common/indirection-fixits.c: New test.
---
gcc/c-family/c-common.c | 35 +++
gcc/c-family/c-common.h | 4 +
gcc/c/c-typeck.c | 18 +-
gcc/cp/call.c | 2 +
gcc/cp/typeck.c | 8 +
gcc/testsuite/c-c++-common/indirection-fixits.c | 299 ++++++++++++++++++++++++
6 files changed, 364 insertions(+), 2 deletions(-)
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 9d51815..f235c9d 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -48,6 +48,7 @@ along with GCC; see the file COPYING3. If not see
#include "gimplify.h"
#include "substring-locations.h"
#include "spellcheck.h"
+#include "gcc-rich-location.h"
#include "selftest.h"
cpp_reader *parse_in; /* Declared in c-pragma.h. */
@@ -8420,6 +8421,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 4187343..0b1fdd7 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1355,6 +1355,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_list_to_string (tree, tree);
extern bool has_attribute (location_t, tree, tree, tree (*)(tree));
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 81c520a..cb7184b 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -1004,6 +1004,14 @@ common_type (tree t1, tree t2)
return c_common_type (t1, t2);
}
+/* 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. */
@@ -7293,7 +7301,10 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
if (pedwarn (&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:
@@ -7329,7 +7340,10 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
if (pedwarn (&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.c b/gcc/cp/call.c
index ee099cc..a25d109 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6862,6 +6862,8 @@ convert_like_real (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 (complained && fn)
inform (get_fndecl_argument_location (fn, argnum),
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 81cb405..2730f77 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -1448,6 +1448,14 @@ structural_comptypes (tree t1, tree t2, int strict)
return comp_type_attributes (t1, t2);
}
+/* 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 0000000..8987ee9
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/indirection-fixits.c
@@ -0,0 +1,299 @@
+/* { 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 } */
+ /* { dg-message "possible fix: take the address with '&'" "" { target *-*-* } .-2 } */
+
+ /* 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-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 "" }
+ 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-warning "" "" { target c } } */
+ /* { dg-error "" "" { target c++ } .-1 } */
+ /* { dg-message "possible fix: dereference with '*'" "" { target *-*-* } .-2 } */
+
+ /* 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-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++ } } */
+ /* { 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 } */
+
+ /* 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-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++ } } */
+ /* { 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 "" } */
+}
+
+/* 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-warning "" "" { target c } } */
+ /* { dg-error "" "" { target c++ } .-1 } */
+ /* { dg-message "possible fix: take the address with '&'" "" { target *-*-* } .-2 } */
+
+ /* 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 "" } */
+}
--
1.8.5.3
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] v4: C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
2018-11-30 23:01 ` [PATCH] v4: " David Malcolm
@ 2018-12-01 18:38 ` Jason Merrill
2018-12-03 22:14 ` Joseph Myers
0 siblings, 1 reply; 20+ messages in thread
From: Jason Merrill @ 2018-12-01 18:38 UTC (permalink / raw)
To: David Malcolm, Joseph Myers, law
Cc: Martin Sebor, Eric Gallager, gcc-patches List
On 11/30/18 6:48 PM, David Malcolm wrote:
> On Tue, 2018-11-20 at 22:23 +0000, Joseph Myers wrote:
>> On Tue, 20 Nov 2018, David Malcolm wrote:
>>
>>> Should I do:
>>
>> You should do whatever is appropriate for the warning in question. But if
>> what's appropriate for the warning in question includes types that are
>> compatible but not the same, the comments need to avoid saying it's about
>> the types being the same.
> Thanks.
>
> Here's an updated version of the patch. I added a new
> compatible_types_for_indirection_note_p
> specifically for use by maybe_emit_indirection_note, and provided
> implementations for C and C++.
> +/* 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;
> +}
Hmm, it looks like the C front-end comptypes will return 1 for e.g. enum
and int. It seems to me that what you want for this warning is actually
to check for the same type. Perhaps you want to use
comptypes_check_different_types? Joseph would know better what's
correct for the C front-end.
Jason
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] v4: C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
2018-12-01 18:38 ` Jason Merrill
@ 2018-12-03 22:14 ` Joseph Myers
2018-12-05 16:03 ` Jason Merrill
0 siblings, 1 reply; 20+ messages in thread
From: Joseph Myers @ 2018-12-03 22:14 UTC (permalink / raw)
To: Jason Merrill
Cc: David Malcolm, law, Martin Sebor, Eric Gallager, gcc-patches List
On Sat, 1 Dec 2018, Jason Merrill wrote:
> Hmm, it looks like the C front-end comptypes will return 1 for e.g. enum and
> int. It seems to me that what you want for this warning is actually to check
> for the same type. Perhaps you want to use comptypes_check_different_types?
> Joseph would know better what's correct for the C front-end.
Well, it's valid to pass a pointer to enum where a pointer to the
compatible integer type is required, or vice versa, but I don't have
advice on which cases you want to accept for this particular fix-it.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] v4: C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
2018-12-03 22:14 ` Joseph Myers
@ 2018-12-05 16:03 ` Jason Merrill
2018-12-05 16:18 ` David Malcolm
0 siblings, 1 reply; 20+ messages in thread
From: Jason Merrill @ 2018-12-05 16:03 UTC (permalink / raw)
To: Joseph S. Myers
Cc: David Malcolm, Jeff Law, Martin Sebor, Eric Gallager, gcc-patches List
On Mon, Dec 3, 2018 at 5:14 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Sat, 1 Dec 2018, Jason Merrill wrote:
>
> > Hmm, it looks like the C front-end comptypes will return 1 for e.g. enum and
> > int. It seems to me that what you want for this warning is actually to check
> > for the same type. Perhaps you want to use comptypes_check_different_types?
> > Joseph would know better what's correct for the C front-end.
>
> Well, it's valid to pass a pointer to enum where a pointer to the
> compatible integer type is required, or vice versa, but I don't have
> advice on which cases you want to accept for this particular fix-it.
Since the diagnostic is about returning, e.g. an int when an int* is
needed, or vice versa, I don't think considering that kind of
conversion is helpful.
Jason
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] v4: C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
2018-12-05 16:03 ` Jason Merrill
@ 2018-12-05 16:18 ` David Malcolm
0 siblings, 0 replies; 20+ messages in thread
From: David Malcolm @ 2018-12-05 16:18 UTC (permalink / raw)
To: Jason Merrill, Joseph S. Myers
Cc: Jeff Law, Martin Sebor, Eric Gallager, gcc-patches List
On Wed, 2018-12-05 at 11:03 -0500, Jason Merrill wrote:
> On Mon, Dec 3, 2018 at 5:14 PM Joseph Myers <joseph@codesourcery.com>
> wrote:
> >
> > On Sat, 1 Dec 2018, Jason Merrill wrote:
> >
> > > Hmm, it looks like the C front-end comptypes will return 1 for
> > > e.g. enum and
> > > int. It seems to me that what you want for this warning is
> > > actually to check
> > > for the same type. Perhaps you want to use
> > > comptypes_check_different_types?
> > > Joseph would know better what's correct for the C front-end.
> >
> > Well, it's valid to pass a pointer to enum where a pointer to the
> > compatible integer type is required, or vice versa, but I don't
> > have
> > advice on which cases you want to accept for this particular fix-
> > it.
>
> Since the diagnostic is about returning, e.g. an int when an int* is
> needed, or vice versa, I don't think considering that kind of
> conversion is helpful.
FWIW I'd be happy to use simple pointer equality of the types for the C
FE, so that we only emit the suggestion for an exact match (better to
fail to offer a suggestion than to offer a bogus one, I think).
Dave
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2018-12-05 16:18 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-09 22:36 [PATCH] C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850) David Malcolm
2018-11-10 7:01 ` Eric Gallager
2018-11-11 18:01 ` Martin Sebor
2018-11-11 21:02 ` David Malcolm
2018-11-12 21:32 ` Martin Sebor
2018-11-13 21:34 ` Jason Merrill
2018-11-15 21:48 ` [PATCH] v2: " David Malcolm
2018-11-16 18:13 ` Jason Merrill
2018-11-19 21:23 ` [PATCH] v3: " David Malcolm
2018-11-20 2:46 ` Joseph Myers
2018-11-20 22:05 ` David Malcolm
2018-11-20 22:23 ` Joseph Myers
2018-11-30 23:01 ` [PATCH] v4: " David Malcolm
2018-12-01 18:38 ` Jason Merrill
2018-12-03 22:14 ` Joseph Myers
2018-12-05 16:03 ` Jason Merrill
2018-12-05 16:18 ` David Malcolm
2018-11-21 0:36 ` [PATCH] v3: " Jeff Law
2018-11-21 0:39 ` Martin Sebor
2018-11-30 15:27 ` David Malcolm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).