public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH,c++] use XALLOCAVEC in the C++ front-end
@ 2010-06-07  0:52 Nathan Froyd
  2010-06-08 14:06 ` Jason Merrill
  0 siblings, 1 reply; 14+ messages in thread
From: Nathan Froyd @ 2010-06-07  0:52 UTC (permalink / raw)
  To: gcc-patches

Just a minor cleanup noticed while browsing around.

There's still one call that seems like it ought to be replaceable: in
semantics.c:finish_asm_stmt, we have:

      oconstraints = (const char **) alloca (noutputs * sizeof (char *));

But changing this to:

      oconstraints = (const char **) XALLOCAVEC (char *, noutputs);

gives the error:

semantics.c: In function ‘finish_asm_stmt’:
semantics.c:1223:7: error: new ‘const’ qualifier in middle of multi-level non-const cast is unsafe [-Werror=cast-qual]

which I don't understand, because the cast is not in the "middle" of
anything.  Is this a bug in the cast checking?

Tested on x86_64-unknown-linux-gnu.  OK to commit?

	* call.c (build_call_n): Call XALLOCAVEC instead of alloca.
	(build_op_delete_call): Likewise.
	(build_over_call): Likewise.
	* cp-gimplify.c (cxx_omp_clause_apply_fn): Likewise.
	* pt.c (process_partial_specialization): Likewise.
	(tsubst_template_args): Likewise.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 03d188b..9162a1d 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -282,7 +282,7 @@ build_call_n (tree function, int n, ...)
     return build_call_a (function, 0, NULL);
   else
     {
-      tree *argarray = (tree *) alloca (n * sizeof (tree));
+      tree *argarray = XALLOCAVEC (tree, n);
       va_list ap;
       int i;
 
@@ -4756,7 +4756,7 @@ build_op_delete_call (enum tree_code code, tree addr, tree size,
 	  /* The placement args might not be suitable for overload
 	     resolution at this point, so build the call directly.  */
 	  int nargs = call_expr_nargs (placement);
-	  tree *argarray = (tree *) alloca (nargs * sizeof (tree));
+	  tree *argarray = XALLOCAVEC (tree, nargs);
 	  int i;
 	  argarray[0] = addr;
 	  for (i = 1; i < nargs; i++)
@@ -5624,7 +5624,7 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
   nargs = VEC_length (tree, args) + (first_arg != NULL_TREE ? 1 : 0);
   if (parmlen > nargs)
     nargs = parmlen;
-  argarray = (tree *) alloca (nargs * sizeof (tree));
+  argarray = XALLOCAVEC (tree, nargs);
 
   /* The implicit parameters to a constructor are not considered by overload
      resolution, and must be of the proper type.  */
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index f45d714..fb7daeb 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -981,7 +981,7 @@ cxx_omp_clause_apply_fn (tree fn, tree arg1, tree arg2)
     return NULL;
 
   nargs = list_length (DECL_ARGUMENTS (fn));
-  argarray = (tree *) alloca (nargs * sizeof (tree));
+  argarray = XALLOCAVEC (tree, nargs);
 
   defparm = TREE_CHAIN (TYPE_ARG_TYPES (TREE_TYPE (fn)));
   if (arg2)
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index b95fdf7..69e4816 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -3875,10 +3875,10 @@ process_partial_specialization (tree decl)
 
      or some such would have been OK.  */
   tpd.level = TMPL_PARMS_DEPTH (current_template_parms);
-  tpd.parms = (int *) alloca (sizeof (int) * ntparms);
+  tpd.parms = XALLOCAVEC (int, ntparms);
   memset (tpd.parms, 0, sizeof (int) * ntparms);
 
-  tpd.arg_uses_template_parms = (int *) alloca (sizeof (int) * nargs);
+  tpd.arg_uses_template_parms = XALLOCAVEC (int, nargs);
   memset (tpd.arg_uses_template_parms, 0, sizeof (int) * nargs);
   for (i = 0; i < nargs; ++i)
     {
@@ -3993,12 +3993,11 @@ process_partial_specialization (tree decl)
                   if (!tpd2.parms)
                     {
                       /* We haven't yet initialized TPD2.  Do so now.  */
-                      tpd2.arg_uses_template_parms 
-                        = (int *) alloca (sizeof (int) * nargs);
+                      tpd2.arg_uses_template_parms = XALLOCAVEC (int, nargs);
                       /* The number of parameters here is the number in the
                          main template, which, as checked in the assertion
                          above, is NARGS.  */
-                      tpd2.parms = (int *) alloca (sizeof (int) * nargs);
+                      tpd2.parms = XALLOCAVEC (int, nargs);
                       tpd2.level = 
                         TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (maintmpl));
                     }
@@ -8527,7 +8526,7 @@ tsubst_template_args (tree t, tree args, tsubst_flags_t complain, tree in_decl)
   tree orig_t = t;
   int len = TREE_VEC_LENGTH (t);
   int need_new = 0, i, expanded_len_adjust = 0, out;
-  tree *elts = (tree *) alloca (len * sizeof (tree));
+  tree *elts = XALLOCAVEC (tree, len);
 
   for (i = 0; i < len; i++)
     {

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH,c++] use XALLOCAVEC in the C++ front-end
  2010-06-07  0:52 [PATCH,c++] use XALLOCAVEC in the C++ front-end Nathan Froyd
@ 2010-06-08 14:06 ` Jason Merrill
  2010-06-08 14:43   ` Manuel López-Ibáñez
  0 siblings, 1 reply; 14+ messages in thread
From: Jason Merrill @ 2010-06-08 14:06 UTC (permalink / raw)
  To: Nathan Froyd; +Cc: gcc-patches

On 06/06/2010 08:52 PM, Nathan Froyd wrote:
> Just a minor cleanup noticed while browsing around.
>
> There's still one call that seems like it ought to be replaceable: in
> semantics.c:finish_asm_stmt, we have:
>
>        oconstraints = (const char **) alloca (noutputs * sizeof (char *));
>
> But changing this to:
>
>        oconstraints = (const char **) XALLOCAVEC (char *, noutputs);
>
> gives the error:
>
> semantics.c: In function ‘finish_asm_stmt’:
> semantics.c:1223:7: error: new ‘const’ qualifier in middle of multi-level non-const cast is unsafe [-Werror=cast-qual]
>
> which I don't understand, because the cast is not in the "middle" of
> anything.  Is this a bug in the cast checking?

The error message is unclear, but the issue is that converting from 
char** to char const** is unsafe, while converting to char const*const* 
is safe.

Just use XALLOCAVEC(const char *, noutputs) with no extra cast.

Jason

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH,c++] use XALLOCAVEC in the C++ front-end
  2010-06-08 14:06 ` Jason Merrill
@ 2010-06-08 14:43   ` Manuel López-Ibáñez
  2010-06-08 16:17     ` Jason Merrill
  0 siblings, 1 reply; 14+ messages in thread
From: Manuel López-Ibáñez @ 2010-06-08 14:43 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Nathan Froyd, gcc-patches

On 8 June 2010 16:06, Jason Merrill <jason@redhat.com> wrote:
> On 06/06/2010 08:52 PM, Nathan Froyd wrote:
>>
>> Just a minor cleanup noticed while browsing around.
>>
>> There's still one call that seems like it ought to be replaceable: in
>> semantics.c:finish_asm_stmt, we have:
>>
>>       oconstraints = (const char **) alloca (noutputs * sizeof (char *));
>>
>> But changing this to:
>>
>>       oconstraints = (const char **) XALLOCAVEC (char *, noutputs);
>>
>> gives the error:
>>
>> semantics.c: In function ‘finish_asm_stmt’:
>> semantics.c:1223:7: error: new ‘const’ qualifier in middle of multi-level
>> non-const cast is unsafe [-Werror=cast-qual]
>>
>> which I don't understand, because the cast is not in the "middle" of
>> anything.  Is this a bug in the cast checking?
>
> The error message is unclear, but the issue is that converting from char**
> to char const** is unsafe, while converting to char const*const* is safe.

What does it mean unsafe in this context? I could produce a patch that
changes the message but I would like to be sure what to say.

semantics.c:1223:7: error: new ‘const’ qualifier in cast from 'char
**' to 'const char **' is unsafe [-Werror=cast-qual]

Cheers,

Manuel.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH,c++] use XALLOCAVEC in the C++ front-end
  2010-06-08 14:43   ` Manuel López-Ibáñez
@ 2010-06-08 16:17     ` Jason Merrill
  2010-06-09  9:58       ` Manuel López-Ibáñez
  0 siblings, 1 reply; 14+ messages in thread
From: Jason Merrill @ 2010-06-08 16:17 UTC (permalink / raw)
  To: Manuel López-Ibáñez; +Cc: Nathan Froyd, gcc-patches

On 06/08/2010 10:43 AM, Manuel López-Ibáñez wrote:
> On 8 June 2010 16:06, Jason Merrill<jason@redhat.com>  wrote:

>> The error message is unclear, but the issue is that converting from char**
>> to char const** is unsafe, while converting to char const*const* is safe.
>
> What does it mean unsafe in this context? I could produce a patch that
> changes the message but I would like to be sure what to say.
>
> semantics.c:1223:7: error: new ‘const’ qualifier in cast from 'char
> **' to 'const char **' is unsafe [-Werror=cast-qual]

Explanation from the C++ standard (4.4 [conv.qual]):

[ Note: if a program could assign a pointer of type T** to a pointer of 
type const T** (that is, if line #1 below were allowed), a program could 
inadvertently modify a const object (as it is done on line #2). For example,

   int main() {
     const char c = Â’cÂ’;
     char* pc;
     const char** pcc = &pc;// #1: not allowed
     *pcc = &c;
     *pc = Â’CÂ’; // #2: modifies a const object
   }
--end note ]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH,c++] use XALLOCAVEC in the C++ front-end
  2010-06-08 16:17     ` Jason Merrill
@ 2010-06-09  9:58       ` Manuel López-Ibáñez
  2010-06-09 11:40         ` Gabriel Dos Reis
  2010-06-09 21:00         ` Joseph S. Myers
  0 siblings, 2 replies; 14+ messages in thread
From: Manuel López-Ibáñez @ 2010-06-09  9:58 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Nathan Froyd, gcc-patches

On 8 June 2010 18:14, Jason Merrill <jason@redhat.com> wrote:
> On 06/08/2010 10:43 AM, Manuel López-Ibáñez wrote:
>>
>> On 8 June 2010 16:06, Jason Merrill<jason@redhat.com>  wrote:
>
>>> The error message is unclear, but the issue is that converting from
>>> char**
>>> to char const** is unsafe, while converting to char const*const* is safe.
>>
>> What does it mean unsafe in this context? I could produce a patch that
>> changes the message but I would like to be sure what to say.
>>
>> semantics.c:1223:7: error: new ‘const’ qualifier in cast from 'char
>> **' to 'const char **' is unsafe [-Werror=cast-qual]
> Explanation from the C++ standard (4.4 [conv.qual]):
>
> [ Note: if a program could assign a pointer of type T** to a pointer of type
> const T** (that is, if line #1 below were allowed), a program could
> inadvertently modify a const object (as it is done on line #2). For example,
>
>  int main() {
>    const char c = ’c’;
>    char* pc;
>    const char** pcc = &pc;// #1: not allowed
>    *pcc = &c;
>    *pc = ’C’; // #2: modifies a const object
>  }
> --end note ]

So what do you think about the following message:

error: new ‘const’ qualifier in cast from 'char **' to 'const char **'
is unsafe [-Werror=cast-qual]
note: to be safe all intermediate pointers must be equally qualified

Cheers,

Manuel.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH,c++] use XALLOCAVEC in the C++ front-end
  2010-06-09  9:58       ` Manuel López-Ibáñez
@ 2010-06-09 11:40         ` Gabriel Dos Reis
  2010-06-09 12:08           ` Manuel López-Ibáñez
  2010-06-09 21:00         ` Joseph S. Myers
  1 sibling, 1 reply; 14+ messages in thread
From: Gabriel Dos Reis @ 2010-06-09 11:40 UTC (permalink / raw)
  To: Manuel López-Ibáñez
  Cc: Jason Merrill, Nathan Froyd, gcc-patches

On Wed, Jun 9, 2010 at 4:53 AM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> On 8 June 2010 18:14, Jason Merrill <jason@redhat.com> wrote:
>> On 06/08/2010 10:43 AM, Manuel López-Ibáñez wrote:
>>>
>>> On 8 June 2010 16:06, Jason Merrill<jason@redhat.com>  wrote:
>>
>>>> The error message is unclear, but the issue is that converting from
>>>> char**
>>>> to char const** is unsafe, while converting to char const*const* is safe.
>>>
>>> What does it mean unsafe in this context? I could produce a patch that
>>> changes the message but I would like to be sure what to say.
>>>
>>> semantics.c:1223:7: error: new ‘const’ qualifier in cast from 'char
>>> **' to 'const char **' is unsafe [-Werror=cast-qual]
>> Explanation from the C++ standard (4.4 [conv.qual]):
>>
>> [ Note: if a program could assign a pointer of type T** to a pointer of type
>> const T** (that is, if line #1 below were allowed), a program could
>> inadvertently modify a const object (as it is done on line #2). For example,
>>
>>  int main() {
>>    const char c = ’c’;
>>    char* pc;
>>    const char** pcc = &pc;// #1: not allowed
>>    *pcc = &c;
>>    *pc = ’C’; // #2: modifies a const object
>>  }
>> --end note ]
>
> So what do you think about the following message:
>
> error: new ‘const’ qualifier in cast from 'char **' to 'const char **'
> is unsafe [-Werror=cast-qual]
> note: to be safe all intermediate pointers must be equally qualified

that is OK; but I find the leading 'new' hard to parse.   Please, just
remove it.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH,c++] use XALLOCAVEC in the C++ front-end
  2010-06-09 11:40         ` Gabriel Dos Reis
@ 2010-06-09 12:08           ` Manuel López-Ibáñez
  2010-06-09 12:24             ` Gabriel Dos Reis
  0 siblings, 1 reply; 14+ messages in thread
From: Manuel López-Ibáñez @ 2010-06-09 12:08 UTC (permalink / raw)
  To: Gabriel Dos Reis
  Cc: Jason Merrill, Nathan Froyd, gcc-patches, Joseph S. Myers

On 9 June 2010 13:33, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
> On Wed, Jun 9, 2010 at 4:53 AM, Manuel López-Ibáñez
> <lopezibanez@gmail.com> wrote:
>> On 8 June 2010 18:14, Jason Merrill <jason@redhat.com> wrote:
>>> On 06/08/2010 10:43 AM, Manuel López-Ibáñez wrote:
>>>>
>>>> On 8 June 2010 16:06, Jason Merrill<jason@redhat.com>  wrote:
>>>
>>>>> The error message is unclear, but the issue is that converting from
>>>>> char**
>>>>> to char const** is unsafe, while converting to char const*const* is safe.
>>>>
>>>> What does it mean unsafe in this context? I could produce a patch that
>>>> changes the message but I would like to be sure what to say.
>>>>
>>>> semantics.c:1223:7: error: new ‘const’ qualifier in cast from 'char
>>>> **' to 'const char **' is unsafe [-Werror=cast-qual]
>>> Explanation from the C++ standard (4.4 [conv.qual]):
>>>
>>> [ Note: if a program could assign a pointer of type T** to a pointer of type
>>> const T** (that is, if line #1 below were allowed), a program could
>>> inadvertently modify a const object (as it is done on line #2). For example,
>>>
>>>  int main() {
>>>    const char c = ’c’;
>>>    char* pc;
>>>    const char** pcc = &pc;// #1: not allowed
>>>    *pcc = &c;
>>>    *pc = ’C’; // #2: modifies a const object
>>>  }
>>> --end note ]
>>
>> So what do you think about the following message:
>>
>> error: new ‘const’ qualifier in cast from 'char **' to 'const char **'
>> is unsafe [-Werror=cast-qual]
>> note: to be safe all intermediate pointers must be equally qualified
>
> that is OK; but I find the leading 'new' hard to parse.   Please, just
> remove it.
>

Anyway, this is the C FE. The C++ FE gives the following:

test.C:4:23: warning: invalid conversion from ‘char**’ to ‘const
char**’ [-fpermissive]

but the code is too intricate to give a clearer error in the case of
C++ or to factor out the common check.

Cheers,

Manuel.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH,c++] use XALLOCAVEC in the C++ front-end
  2010-06-09 12:08           ` Manuel López-Ibáñez
@ 2010-06-09 12:24             ` Gabriel Dos Reis
  2010-06-09 12:35               ` Manuel López-Ibáñez
  0 siblings, 1 reply; 14+ messages in thread
From: Gabriel Dos Reis @ 2010-06-09 12:24 UTC (permalink / raw)
  To: Manuel López-Ibáñez
  Cc: Jason Merrill, Nathan Froyd, gcc-patches, Joseph S. Myers

On Wed, Jun 9, 2010 at 6:53 AM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> On 9 June 2010 13:33, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
>> On Wed, Jun 9, 2010 at 4:53 AM, Manuel López-Ibáñez
>> <lopezibanez@gmail.com> wrote:
>>> On 8 June 2010 18:14, Jason Merrill <jason@redhat.com> wrote:
>>>> On 06/08/2010 10:43 AM, Manuel López-Ibáñez wrote:
>>>>>
>>>>> On 8 June 2010 16:06, Jason Merrill<jason@redhat.com>  wrote:
>>>>
>>>>>> The error message is unclear, but the issue is that converting from
>>>>>> char**
>>>>>> to char const** is unsafe, while converting to char const*const* is safe.
>>>>>
>>>>> What does it mean unsafe in this context? I could produce a patch that
>>>>> changes the message but I would like to be sure what to say.
>>>>>
>>>>> semantics.c:1223:7: error: new ‘const’ qualifier in cast from 'char
>>>>> **' to 'const char **' is unsafe [-Werror=cast-qual]
>>>> Explanation from the C++ standard (4.4 [conv.qual]):
>>>>
>>>> [ Note: if a program could assign a pointer of type T** to a pointer of type
>>>> const T** (that is, if line #1 below were allowed), a program could
>>>> inadvertently modify a const object (as it is done on line #2). For example,
>>>>
>>>>  int main() {
>>>>    const char c = ’c’;
>>>>    char* pc;
>>>>    const char** pcc = &pc;// #1: not allowed
>>>>    *pcc = &c;
>>>>    *pc = ’C’; // #2: modifies a const object
>>>>  }
>>>> --end note ]
>>>
>>> So what do you think about the following message:
>>>
>>> error: new ‘const’ qualifier in cast from 'char **' to 'const char **'
>>> is unsafe [-Werror=cast-qual]
>>> note: to be safe all intermediate pointers must be equally qualified
>>
>> that is OK; but I find the leading 'new' hard to parse.   Please, just
>> remove it.
>>
>
> Anyway, this is the C FE.

Yes, I know.

> The C++ FE gives the following:
>
> test.C:4:23: warning: invalid conversion from ‘char**’ to ‘const
> char**’ [-fpermissive]
>
> but the code is too intricate to give a clearer error in the case of
> C++ or to factor out the common check.

Removing the leading 'new' does not require any of that.

>
> Cheers,
>
> Manuel.
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH,c++] use XALLOCAVEC in the C++ front-end
  2010-06-09 12:35               ` Manuel López-Ibáñez
@ 2010-06-09 12:33                 ` Gabriel Dos Reis
  0 siblings, 0 replies; 14+ messages in thread
From: Gabriel Dos Reis @ 2010-06-09 12:33 UTC (permalink / raw)
  To: Manuel López-Ibáñez
  Cc: Jason Merrill, Nathan Froyd, gcc-patches, Joseph S. Myers

On Wed, Jun 9, 2010 at 7:11 AM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> On 9 June 2010 14:08, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
>> On Wed, Jun 9, 2010 at 6:53 AM, Manuel López-Ibáñez
>> <lopezibanez@gmail.com> wrote:
>>> On 9 June 2010 13:33, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
>>>> On Wed, Jun 9, 2010 at 4:53 AM, Manuel López-Ibáñez
>>>> <lopezibanez@gmail.com> wrote:
>>>>> On 8 June 2010 18:14, Jason Merrill <jason@redhat.com> wrote:
>>>>>> On 06/08/2010 10:43 AM, Manuel López-Ibáñez wrote:
>>>>>>>
>>>>>>> On 8 June 2010 16:06, Jason Merrill<jason@redhat.com>  wrote:
>>>>>>
>>>>>>>> The error message is unclear, but the issue is that converting from
>>>>>>>> char**
>>>>>>>> to char const** is unsafe, while converting to char const*const* is safe.
>>>>>>>
>>>>>>> What does it mean unsafe in this context? I could produce a patch that
>>>>>>> changes the message but I would like to be sure what to say.
>>>>>>>
>>>>>>> semantics.c:1223:7: error: new ‘const’ qualifier in cast from 'char
>>>>>>> **' to 'const char **' is unsafe [-Werror=cast-qual]
>>>>>> Explanation from the C++ standard (4.4 [conv.qual]):
>>>>>>
>>>>>> [ Note: if a program could assign a pointer of type T** to a pointer of type
>>>>>> const T** (that is, if line #1 below were allowed), a program could
>>>>>> inadvertently modify a const object (as it is done on line #2). For example,
>>>>>>
>>>>>>  int main() {
>>>>>>    const char c = ’c’;
>>>>>>    char* pc;
>>>>>>    const char** pcc = &pc;// #1: not allowed
>>>>>>    *pcc = &c;
>>>>>>    *pc = ’C’; // #2: modifies a const object
>>>>>>  }
>>>>>> --end note ]
>>>>>
>>>>> So what do you think about the following message:
>>>>>
>>>>> error: new ‘const’ qualifier in cast from 'char **' to 'const char **'
>>>>> is unsafe [-Werror=cast-qual]
>>>>> note: to be safe all intermediate pointers must be equally qualified
>>>>
>>>> that is OK; but I find the leading 'new' hard to parse.   Please, just
>>>> remove it.
>>>>
>>>
>>> Anyway, this is the C FE.
>>
>> Yes, I know.
>>
>>> The C++ FE gives the following:
>>>
>>> test.C:4:23: warning: invalid conversion from ‘char**’ to ‘const
>>> char**’ [-fpermissive]
>>>
>>> but the code is too intricate to give a clearer error in the case of
>>> C++ or to factor out the common check.
>>
>> Removing the leading 'new' does not require any of that.
>
> Of course, it was a just a comment since consistency between C and C++
> is a goal, so we should always check for low hanging-fruit. Not this
> case unfortunately.
>
> I am bootstrapping this:
>
> Index: gcc/c-typeck.c
> ===================================================================
> --- gcc/c-typeck.c      (revision 160384)
> +++ gcc/c-typeck.c      (working copy)
> @@ -4493,9 +4493,12 @@
>          && !is_const)
>        {
>          int added = TYPE_QUALS (in_type) &~ TYPE_QUALS (in_otype);
> -         warning (OPT_Wcast_qual,
> -                  ("new %qv qualifier in middle of multi-level non-const cast "
> -                   "is unsafe"), added);
> +         if (warning_at (input_location, OPT_Wcast_qual,
> +                          "%qv qualifier in cast from %qT to %qT is unsafe",
> +                          added, in_type, in_otype))
> +              inform (input_location,
> +                      G_("to be safe all intermediate pointers"
> +                         " must be equally qualified"));
>          break;
>        }
>       if (is_const)
>
> There will be some testcases that need adjusting.
>

OK, thanks!

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH,c++] use XALLOCAVEC in the C++ front-end
  2010-06-09 12:24             ` Gabriel Dos Reis
@ 2010-06-09 12:35               ` Manuel López-Ibáñez
  2010-06-09 12:33                 ` Gabriel Dos Reis
  0 siblings, 1 reply; 14+ messages in thread
From: Manuel López-Ibáñez @ 2010-06-09 12:35 UTC (permalink / raw)
  To: Gabriel Dos Reis
  Cc: Jason Merrill, Nathan Froyd, gcc-patches, Joseph S. Myers

On 9 June 2010 14:08, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
> On Wed, Jun 9, 2010 at 6:53 AM, Manuel López-Ibáñez
> <lopezibanez@gmail.com> wrote:
>> On 9 June 2010 13:33, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
>>> On Wed, Jun 9, 2010 at 4:53 AM, Manuel López-Ibáñez
>>> <lopezibanez@gmail.com> wrote:
>>>> On 8 June 2010 18:14, Jason Merrill <jason@redhat.com> wrote:
>>>>> On 06/08/2010 10:43 AM, Manuel López-Ibáñez wrote:
>>>>>>
>>>>>> On 8 June 2010 16:06, Jason Merrill<jason@redhat.com>  wrote:
>>>>>
>>>>>>> The error message is unclear, but the issue is that converting from
>>>>>>> char**
>>>>>>> to char const** is unsafe, while converting to char const*const* is safe.
>>>>>>
>>>>>> What does it mean unsafe in this context? I could produce a patch that
>>>>>> changes the message but I would like to be sure what to say.
>>>>>>
>>>>>> semantics.c:1223:7: error: new ‘const’ qualifier in cast from 'char
>>>>>> **' to 'const char **' is unsafe [-Werror=cast-qual]
>>>>> Explanation from the C++ standard (4.4 [conv.qual]):
>>>>>
>>>>> [ Note: if a program could assign a pointer of type T** to a pointer of type
>>>>> const T** (that is, if line #1 below were allowed), a program could
>>>>> inadvertently modify a const object (as it is done on line #2). For example,
>>>>>
>>>>>  int main() {
>>>>>    const char c = ’c’;
>>>>>    char* pc;
>>>>>    const char** pcc = &pc;// #1: not allowed
>>>>>    *pcc = &c;
>>>>>    *pc = ’C’; // #2: modifies a const object
>>>>>  }
>>>>> --end note ]
>>>>
>>>> So what do you think about the following message:
>>>>
>>>> error: new ‘const’ qualifier in cast from 'char **' to 'const char **'
>>>> is unsafe [-Werror=cast-qual]
>>>> note: to be safe all intermediate pointers must be equally qualified
>>>
>>> that is OK; but I find the leading 'new' hard to parse.   Please, just
>>> remove it.
>>>
>>
>> Anyway, this is the C FE.
>
> Yes, I know.
>
>> The C++ FE gives the following:
>>
>> test.C:4:23: warning: invalid conversion from ‘char**’ to ‘const
>> char**’ [-fpermissive]
>>
>> but the code is too intricate to give a clearer error in the case of
>> C++ or to factor out the common check.
>
> Removing the leading 'new' does not require any of that.

Of course, it was a just a comment since consistency between C and C++
is a goal, so we should always check for low hanging-fruit. Not this
case unfortunately.

I am bootstrapping this:

Index: gcc/c-typeck.c
===================================================================
--- gcc/c-typeck.c      (revision 160384)
+++ gcc/c-typeck.c      (working copy)
@@ -4493,9 +4493,12 @@
          && !is_const)
        {
          int added = TYPE_QUALS (in_type) &~ TYPE_QUALS (in_otype);
-         warning (OPT_Wcast_qual,
-                  ("new %qv qualifier in middle of multi-level non-const cast "
-                   "is unsafe"), added);
+         if (warning_at (input_location, OPT_Wcast_qual,
+                          "%qv qualifier in cast from %qT to %qT is unsafe",
+                          added, in_type, in_otype))
+              inform (input_location,
+                      G_("to be safe all intermediate pointers"
+                         " must be equally qualified"));
          break;
        }
       if (is_const)

There will be some testcases that need adjusting.

Cheers,

Manuel.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH,c++] use XALLOCAVEC in the C++ front-end
  2010-06-09  9:58       ` Manuel López-Ibáñez
  2010-06-09 11:40         ` Gabriel Dos Reis
@ 2010-06-09 21:00         ` Joseph S. Myers
  2010-06-11  1:43           ` Manuel López-Ibáñez
  1 sibling, 1 reply; 14+ messages in thread
From: Joseph S. Myers @ 2010-06-09 21:00 UTC (permalink / raw)
  To: Manuel López-Ibáñez
  Cc: Jason Merrill, Nathan Froyd, gcc-patches

[-- Attachment #1: Type: TEXT/PLAIN, Size: 457 bytes --]

On Wed, 9 Jun 2010, Manuel López-Ibáñez wrote:

> note: to be safe all intermediate pointers must be equally qualified

The requirement is not that they be equally qualified, it's that they be 
const-qualified.  (That is, if you are adding "volatile" qualifiers, 
"const" is still needed at intermediate levels; you can convert char ** to 
volatile char *const * but not to volatile char *volatile *.)

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH,c++] use XALLOCAVEC in the C++ front-end
  2010-06-09 21:00         ` Joseph S. Myers
@ 2010-06-11  1:43           ` Manuel López-Ibáñez
  2010-06-11  9:52             ` Joseph S. Myers
  0 siblings, 1 reply; 14+ messages in thread
From: Manuel López-Ibáñez @ 2010-06-11  1:43 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: Jason Merrill, Nathan Froyd, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 766 bytes --]

On 9 June 2010 22:35, Joseph S. Myers <joseph@codesourcery.com> wrote:
> On Wed, 9 Jun 2010, Manuel López-Ibáñez wrote:
>
>> note: to be safe all intermediate pointers must be equally qualified
>
> The requirement is not that they be equally qualified, it's that they be
> const-qualified.  (That is, if you are adding "volatile" qualifiers,
> "const" is still needed at intermediate levels; you can convert char ** to
> volatile char *const * but not to volatile char *volatile *.)

Bootstrapped and regression tested on x86_64-linux-gnu. OK for trunk?

2010-06-11  Manuel López-Ibáñez  <manu@gcc.gnu.org>

	* c-typeck.c (handle_warn_cast_qual): Add loc
	parameter. Improve warning message.
	(build_c_cast): Pass location to handle_warn_cast_qual.

[-- Attachment #2: wcast-qual.diff --]
[-- Type: text/plain, Size: 3028 bytes --]

Index: gcc/c-typeck.c
===================================================================
--- gcc/c-typeck.c	(revision 160464)
+++ gcc/c-typeck.c	(working copy)
@@ -4411,16 +4411,17 @@ build_compound_expr (location_t loc, tre
   return ret;
 }
 
 /* Issue -Wcast-qual warnings when appropriate.  TYPE is the type to
    which we are casting.  OTYPE is the type of the expression being
-   cast.  Both TYPE and OTYPE are pointer types.  -Wcast-qual appeared
-   on the command line.  Named address space qualifiers are not handled
-   here, because they result in different warnings.  */
+   cast.  Both TYPE and OTYPE are pointer types.  LOC is the location
+   of the cast.  -Wcast-qual appeared on the command line.  Named
+   address space qualifiers are not handled here, because they result
+   in different warnings.  */
 
 static void
-handle_warn_cast_qual (tree type, tree otype)
+handle_warn_cast_qual (location_t loc, tree type, tree otype)
 {
   tree in_type = type;
   tree in_otype = otype;
   int added = 0;
   int discarded = 0;
@@ -4449,19 +4450,19 @@ handle_warn_cast_qual (tree type, tree o
     }
   while (TREE_CODE (in_type) == POINTER_TYPE
 	 && TREE_CODE (in_otype) == POINTER_TYPE);
 
   if (added)
-    warning (OPT_Wcast_qual, "cast adds %q#v qualifier to function type",
-	     added);
+    warning_at (loc, OPT_Wcast_qual,
+		"cast adds %q#v qualifier to function type", added);
 
   if (discarded)
     /* There are qualifiers present in IN_OTYPE that are not present
        in IN_TYPE.  */
-    warning (OPT_Wcast_qual,
-	     "cast discards %q#v qualifier from pointer target type",
-	     discarded);
+    warning_at (loc, OPT_Wcast_qual,
+		"cast discards %q#v qualifier from pointer target type",
+		discarded);
 
   if (added || discarded)
     return;
 
   /* A cast from **T to const **T is unsafe, because it can cause a
@@ -4490,14 +4491,14 @@ handle_warn_cast_qual (tree type, tree o
       in_type = TREE_TYPE (in_type);
       in_otype = TREE_TYPE (in_otype);
       if ((TYPE_QUALS (in_type) &~ TYPE_QUALS (in_otype)) != 0
 	  && !is_const)
 	{
-	  int added = TYPE_QUALS (in_type) &~ TYPE_QUALS (in_otype);
-	  warning (OPT_Wcast_qual,
-		   ("new %qv qualifier in middle of multi-level non-const cast "
-		    "is unsafe"), added);
+	  warning_at (loc, OPT_Wcast_qual,
+		      G_("to be safe all intermediate pointers in cast from "
+			 "%qT to %qT must be %<const%> qualified"),
+		      otype, type);
 	  break;
 	}
       if (is_const)
 	is_const = TYPE_READONLY (in_type);
     }
@@ -4597,11 +4598,11 @@ build_c_cast (location_t loc, tree type,
 
       /* Optionally warn about potentially worrisome casts.  */
       if (warn_cast_qual
 	  && TREE_CODE (type) == POINTER_TYPE
 	  && TREE_CODE (otype) == POINTER_TYPE)
-	handle_warn_cast_qual (type, otype);
+	handle_warn_cast_qual (loc, type, otype);
 
       /* Warn about conversions between pointers to disjoint
 	 address spaces.  */
       if (TREE_CODE (type) == POINTER_TYPE
 	  && TREE_CODE (otype) == POINTER_TYPE

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH,c++] use XALLOCAVEC in the C++ front-end
  2010-06-11  1:43           ` Manuel López-Ibáñez
@ 2010-06-11  9:52             ` Joseph S. Myers
  2010-06-11 10:50               ` Manuel López-Ibáñez
  0 siblings, 1 reply; 14+ messages in thread
From: Joseph S. Myers @ 2010-06-11  9:52 UTC (permalink / raw)
  To: Manuel López-Ibáñez
  Cc: Jason Merrill, Nathan Froyd, gcc-patches

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=X-UNKNOWN, Size: 995 bytes --]

On Fri, 11 Jun 2010, Manuel López-Ibáñez wrote:

> On 9 June 2010 22:35, Joseph S. Myers <joseph@codesourcery.com> wrote:
> > On Wed, 9 Jun 2010, Manuel López-Ibáñez wrote:
> >
> >> note: to be safe all intermediate pointers must be equally qualified
> >
> > The requirement is not that they be equally qualified, it's that they be
> > const-qualified.  (That is, if you are adding "volatile" qualifiers,
> > "const" is still needed at intermediate levels; you can convert char ** to
> > volatile char *const * but not to volatile char *volatile *.)
> 
> Bootstrapped and regression tested on x86_64-linux-gnu. OK for trunk?
> 
> 2010-06-11  Manuel López-Ibáñez  <manu@gcc.gnu.org>
> 
> 	* c-typeck.c (handle_warn_cast_qual): Add loc
> 	parameter. Improve warning message.
> 	(build_c_cast): Pass location to handle_warn_cast_qual.

Remove the explicit G_(), which isn't needed in a call to warning_at.  OK 
with that change.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH,c++] use XALLOCAVEC in the C++ front-end
  2010-06-11  9:52             ` Joseph S. Myers
@ 2010-06-11 10:50               ` Manuel López-Ibáñez
  0 siblings, 0 replies; 14+ messages in thread
From: Manuel López-Ibáñez @ 2010-06-11 10:50 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: Jason Merrill, Nathan Froyd, gcc-patches

On 11 June 2010 11:33, Joseph S. Myers <joseph@codesourcery.com> wrote:
> On Fri, 11 Jun 2010, Manuel López-Ibáñez wrote:
>
>> On 9 June 2010 22:35, Joseph S. Myers <joseph@codesourcery.com> wrote:
>> > On Wed, 9 Jun 2010, Manuel López-Ibáñez wrote:
>> >
>> >> note: to be safe all intermediate pointers must be equally qualified
>> >
>> > The requirement is not that they be equally qualified, it's that they be
>> > const-qualified.  (That is, if you are adding "volatile" qualifiers,
>> > "const" is still needed at intermediate levels; you can convert char ** to
>> > volatile char *const * but not to volatile char *volatile *.)
>>
>> Bootstrapped and regression tested on x86_64-linux-gnu. OK for trunk?
>>
>> 2010-06-11  Manuel López-Ibáñez  <manu@gcc.gnu.org>
>>
>>       * c-typeck.c (handle_warn_cast_qual): Add loc
>>       parameter. Improve warning message.
>>       (build_c_cast): Pass location to handle_warn_cast_qual.
>
> Remove the explicit G_(), which isn't needed in a call to warning_at.  OK
> with that change.

Committed revision 160601.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2010-06-11  9:53 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-07  0:52 [PATCH,c++] use XALLOCAVEC in the C++ front-end Nathan Froyd
2010-06-08 14:06 ` Jason Merrill
2010-06-08 14:43   ` Manuel López-Ibáñez
2010-06-08 16:17     ` Jason Merrill
2010-06-09  9:58       ` Manuel López-Ibáñez
2010-06-09 11:40         ` Gabriel Dos Reis
2010-06-09 12:08           ` Manuel López-Ibáñez
2010-06-09 12:24             ` Gabriel Dos Reis
2010-06-09 12:35               ` Manuel López-Ibáñez
2010-06-09 12:33                 ` Gabriel Dos Reis
2010-06-09 21:00         ` Joseph S. Myers
2010-06-11  1:43           ` Manuel López-Ibáñez
2010-06-11  9:52             ` Joseph S. Myers
2010-06-11 10:50               ` Manuel López-Ibáñez

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).