Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Jonathan Wakely]

On 25/05/17 16:56 +0200, Jakub Jelinek wrote:

On Thu, May 25, 2017 at 03:52:47PM +0100, Jonathan Wakely wrote:

I'd probably write that like this instead:

void
foo (const void *p)
{
 typedef const int* ptr_type;
 ptr_type const q = (ptr_type) p;
 ptr_type const r = (ptr_type) p;
 (void) q;
 (void) r;
}

It names the type only once, not twice as in your example, and doesn't
need to use typeof to refer to that type again.


That was over-simplified, I meant cases where you actually don't know the
exact type, just use typeof to create a variable of such a type and
then want to cast something to that type.


The most obvious case I can think of where you don't know the type
would be the result of some expression (maybe involving overloaded
operators, or a call to an overloaded function).  In that case:

 typeof (expr) const q = (typeof (expr)) p;

Again, add the const to the variable definition, but not the cast.

or using a typedef again for clarity and to avoid the repetition:

 typedef typeof (expr) the_type;
 the_type const q = (the_type) p;

If the result of the expression is a scalar type then it won't be
const-qualified, so the_type isn't either, and the cast won't warn.

If the result of the expression is a class-type then it can be
const-qualified, and so the_type is also const-qualified, but the
warning isn't issued in such case.

All the realistic case I can think of expr won't be simply a variable
(because if you have a variable then it was already declared with some
type and you can refer to that type directly) so the original problem
where typeof(q) deduces a const-qualified type won't happen.

template void f (const T& t, const void* p) {
 // no need for typeof(t) because we can just use T
 T* const q = (T*)p;
 T* const r = (T*)p;
}

There's this case, which will warn if the type is a pointer or other
scalar:

 auto const q = some_function (p);
 typeof (q) r = (typeof (q)) p

But what you really care about is typeof (some_function (p)) not
typeof (q), which is just the 'expr' case above:

 typedef typeof (some_function(p)) ptr_type;
 ptr_type const q = some_function (p);
 ptr_type const r = (ptr_type)p;

The warning is avoidable by using typeof on the expression that
determines the type, not on a variable that has a const-qualified
version of the type.

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Jakub Jelinek]

On Thu, May 25, 2017 at 03:52:47PM +0100, Jonathan Wakely wrote:
> I'd probably write that like this instead:
> 
> void
> foo (const void *p)
> {
>  typedef const int* ptr_type;
>  ptr_type const q = (ptr_type) p;
>  ptr_type const r = (ptr_type) p;
>  (void) q;
>  (void) r;
> }
> 
> It names the type only once, not twice as in your example, and doesn't
> need to use typeof to refer to that type again.

That was over-simplified, I meant cases where you actually don't know the
exact type, just use typeof to create a variable of such a type and
then want to cast something to that type.

Jakub

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Jonathan Wakely]

On 25/05/17 16:11 +0200, Jakub Jelinek wrote:

On Thu, May 25, 2017 at 03:02:42PM +0100, Jonathan Wakely wrote:

On 25/05/17 11:07 +0100, Jonathan Wakely wrote:
> On 25/05/17 10:05 +0200, Andreas Schwab wrote:
> > ../../gcc/ada/gcc-interface/utils2.c: In function 'int 
compare_elmt_bitpos(const void*, const void*)':
> > ../../gcc/ada/gcc-interface/utils2.c:1937:73: error: type qualifiers 
ignored on cast result type [-Werror=ignored-qualifiers]
> >  const constructor_elt * const elmt1 = (const constructor_elt * const) rt1;
> >^~~
> > ../../gcc/ada/gcc-interface/utils2.c:1938:73: error: type qualifiers 
ignored on cast result type [-Werror=ignored-qualifiers]
> >  const constructor_elt * const elmt2 = (const constructor_elt * const) rt2;
>
> I'm testing this obvious fix.

Committed as r248458 because it gets bootstrap past the error above,
although now Ada fails for me with:

/home/jwakely/src/gcc/bootstrap/./gcc/xgcc 
-B/home/jwakely/src/gcc/bootstrap/./gcc/ -B/usr/local/x86_64-pc-linux-gnu/bin/ 
-B/usr/local/x86_64-pc-linux-gnu/lib/ -isystem 
/usr/local/x86_64-pc-linux-gnu/include -isystem 
/usr/local/x86_64-pc-linux-gnu/sys-include-c -g -O2 -m32 -fpic  -W -Wall 
-gnatpg -nostdinc -m32  s-regpat.adb -o s-regpat.o

raised STORAGE_ERROR : stack overflow or erroneous memory access
../gcc-interface/Makefile:296: recipe for target 's-regpat.o' failed


> diff --git a/gcc/ada/gcc-interface/utils2.c b/gcc/ada/gcc-interface/utils2.c
> index fc6f1b8..cd37791 100644
> --- a/gcc/ada/gcc-interface/utils2.c
> +++ b/gcc/ada/gcc-interface/utils2.c
> @@ -1934,8 +1934,8 @@ build_call_raise_range (int msg, Node_Id gnat_node, 
char kind,
> static int
> compare_elmt_bitpos (const PTR rt1, const PTR rt2)
> {
> -  const constructor_elt * const elmt1 = (const constructor_elt * const) rt1;
> -  const constructor_elt * const elmt2 = (const constructor_elt * const) rt2;
> +  const constructor_elt * const elmt1 = (const constructor_elt *) rt1;
> +  const constructor_elt * const elmt2 = (const constructor_elt *) rt2;
>   const_tree const field1 = elmt1->index;
>   const_tree const field2 = elmt2->index;
>   const int ret


So, what can one do with typeof or similar to avoid the warning?

void
foo (const void *p)
{
 const int *const q = (const int *const) p;
 typeof (q) r = (typeof (q)) p;
 (void) q;
 (void) r;
}


I'd probably write that like this instead:

void
foo (const void *p)
{
 typedef const int* ptr_type;
 ptr_type const q = (ptr_type) p;
 ptr_type const r = (ptr_type) p;
 (void) q;
 (void) r;
}

It names the type only once, not twice as in your example, and doesn't
need to use typeof to refer to that type again.

The variables p and q are defined const, which is what's wanted. The
cast is to ptr_type, not ptr_type const.

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Marek Polacek]

On Thu, May 25, 2017 at 04:11:19PM +0200, Jakub Jelinek wrote:
> On Thu, May 25, 2017 at 03:02:42PM +0100, Jonathan Wakely wrote:
> > On 25/05/17 11:07 +0100, Jonathan Wakely wrote:
> > > On 25/05/17 10:05 +0200, Andreas Schwab wrote:
> > > > ../../gcc/ada/gcc-interface/utils2.c: In function 'int 
> > > > compare_elmt_bitpos(const void*, const void*)':
> > > > ../../gcc/ada/gcc-interface/utils2.c:1937:73: error: type qualifiers 
> > > > ignored on cast result type [-Werror=ignored-qualifiers]
> > > >  const constructor_elt * const elmt1 = (const constructor_elt * const) 
> > > > rt1;
> > > >
> > > > ^~~
> > > > ../../gcc/ada/gcc-interface/utils2.c:1938:73: error: type qualifiers 
> > > > ignored on cast result type [-Werror=ignored-qualifiers]
> > > >  const constructor_elt * const elmt2 = (const constructor_elt * const) 
> > > > rt2;
> > > 
> > > I'm testing this obvious fix.
> > 
> > Committed as r248458 because it gets bootstrap past the error above,
> > although now Ada fails for me with:
> > 
> > /home/jwakely/src/gcc/bootstrap/./gcc/xgcc 
> > -B/home/jwakely/src/gcc/bootstrap/./gcc/ 
> > -B/usr/local/x86_64-pc-linux-gnu/bin/ -B/usr/local/x86_64-pc-linux-gnu/lib/ 
> > -isystem /usr/local/x86_64-pc-linux-gnu/include -isystem 
> > /usr/local/x86_64-pc-linux-gnu/sys-include-c -g -O2 -m32 -fpic  -W 
> > -Wall -gnatpg -nostdinc -m32  s-regpat.adb -o s-regpat.o
> > 
> > raised STORAGE_ERROR : stack overflow or erroneous memory access
> > ../gcc-interface/Makefile:296: recipe for target 's-regpat.o' failed
> > 
> > 
> > > diff --git a/gcc/ada/gcc-interface/utils2.c 
> > > b/gcc/ada/gcc-interface/utils2.c
> > > index fc6f1b8..cd37791 100644
> > > --- a/gcc/ada/gcc-interface/utils2.c
> > > +++ b/gcc/ada/gcc-interface/utils2.c
> > > @@ -1934,8 +1934,8 @@ build_call_raise_range (int msg, Node_Id gnat_node, 
> > > char kind,
> > > static int
> > > compare_elmt_bitpos (const PTR rt1, const PTR rt2)
> > > {
> > > -  const constructor_elt * const elmt1 = (const constructor_elt * const) 
> > > rt1;
> > > -  const constructor_elt * const elmt2 = (const constructor_elt * const) 
> > > rt2;
> > > +  const constructor_elt * const elmt1 = (const constructor_elt *) rt1;
> > > +  const constructor_elt * const elmt2 = (const constructor_elt *) rt2;
> > >   const_tree const field1 = elmt1->index;
> > >   const_tree const field2 = elmt2->index;
> > >   const int ret
> 
> So, what can one do with typeof or similar to avoid the warning?
> 
> void
> foo (const void *p)
> {
>   const int *const q = (const int *const) p;
>   typeof (q) r = (typeof (q)) p;
>   (void) q;
>   (void) r;
> }
> 
> AFAIK typeof doesn't strip the toplevel qualifiers and I see current trunk
> warns even about the cast in r initialization.  Similarly to what has been
> noted recently in another (C) PR, it would be nice if we had toplevel cv
> stripping variant of typeof or some special builtin that could wrap
> typeof or some type and could be used in places where typeof can,
>   __strip_cv (typeof (q)) = (__strip_cv (typeof (q))) p;
> or
>   typeof (q) = (__strip_cv (typeof (q))) p;
> or
>   __strip_cv (const int *const) z;
> where the last one would be effectively
>   const int *z;

I remember trying to implement the stripping version of __typeof; I even had
a prototype patch that I'm of course not finding right now, but I'd be happy to
work on this again.

Ok, I at least found the PR:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65455
and
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=39985
Also there's
https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00268.html

Let me know if I should get back to it.

Marek

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Jakub Jelinek]

On Thu, May 25, 2017 at 04:11:19PM +0200, Jakub Jelinek wrote:
> So, what can one do with typeof or similar to avoid the warning?
> 
> void
> foo (const void *p)
> {
>   const int *const q = (const int *const) p;
>   typeof (q) r = (typeof (q)) p;
>   (void) q;
>   (void) r;
> }
> 
> AFAIK typeof doesn't strip the toplevel qualifiers and I see current trunk
> warns even about the cast in r initialization.  Similarly to what has been
> noted recently in another (C) PR, it would be nice if we had toplevel cv
> stripping variant of typeof or some special builtin that could wrap
> typeof or some type and could be used in places where typeof can,
>   __strip_cv (typeof (q)) = (__strip_cv (typeof (q))) p;
> or
>   typeof (q) = (__strip_cv (typeof (q))) p;
> or
>   __strip_cv (const int *const) z;
> where the last one would be effectively
>   const int *z;

I guess in C++ one can use
  typeof (q) r = (remove_cv ::type) p;
or something similar, but in C there is nothing like that.

Jakub

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Jakub Jelinek]

On Thu, May 25, 2017 at 03:02:42PM +0100, Jonathan Wakely wrote:
> On 25/05/17 11:07 +0100, Jonathan Wakely wrote:
> > On 25/05/17 10:05 +0200, Andreas Schwab wrote:
> > > ../../gcc/ada/gcc-interface/utils2.c: In function 'int 
> > > compare_elmt_bitpos(const void*, const void*)':
> > > ../../gcc/ada/gcc-interface/utils2.c:1937:73: error: type qualifiers 
> > > ignored on cast result type [-Werror=ignored-qualifiers]
> > >  const constructor_elt * const elmt1 = (const constructor_elt * const) 
> > > rt1;
> > >^~~
> > > ../../gcc/ada/gcc-interface/utils2.c:1938:73: error: type qualifiers 
> > > ignored on cast result type [-Werror=ignored-qualifiers]
> > >  const constructor_elt * const elmt2 = (const constructor_elt * const) 
> > > rt2;
> > 
> > I'm testing this obvious fix.
> 
> Committed as r248458 because it gets bootstrap past the error above,
> although now Ada fails for me with:
> 
> /home/jwakely/src/gcc/bootstrap/./gcc/xgcc 
> -B/home/jwakely/src/gcc/bootstrap/./gcc/ 
> -B/usr/local/x86_64-pc-linux-gnu/bin/ -B/usr/local/x86_64-pc-linux-gnu/lib/ 
> -isystem /usr/local/x86_64-pc-linux-gnu/include -isystem 
> /usr/local/x86_64-pc-linux-gnu/sys-include-c -g -O2 -m32 -fpic  -W -Wall 
> -gnatpg -nostdinc -m32  s-regpat.adb -o s-regpat.o
> 
> raised STORAGE_ERROR : stack overflow or erroneous memory access
> ../gcc-interface/Makefile:296: recipe for target 's-regpat.o' failed
> 
> 
> > diff --git a/gcc/ada/gcc-interface/utils2.c b/gcc/ada/gcc-interface/utils2.c
> > index fc6f1b8..cd37791 100644
> > --- a/gcc/ada/gcc-interface/utils2.c
> > +++ b/gcc/ada/gcc-interface/utils2.c
> > @@ -1934,8 +1934,8 @@ build_call_raise_range (int msg, Node_Id gnat_node, 
> > char kind,
> > static int
> > compare_elmt_bitpos (const PTR rt1, const PTR rt2)
> > {
> > -  const constructor_elt * const elmt1 = (const constructor_elt * const) 
> > rt1;
> > -  const constructor_elt * const elmt2 = (const constructor_elt * const) 
> > rt2;
> > +  const constructor_elt * const elmt1 = (const constructor_elt *) rt1;
> > +  const constructor_elt * const elmt2 = (const constructor_elt *) rt2;
> >   const_tree const field1 = elmt1->index;
> >   const_tree const field2 = elmt2->index;
> >   const int ret

So, what can one do with typeof or similar to avoid the warning?

void
foo (const void *p)
{
  const int *const q = (const int *const) p;
  typeof (q) r = (typeof (q)) p;
  (void) q;
  (void) r;
}

AFAIK typeof doesn't strip the toplevel qualifiers and I see current trunk
warns even about the cast in r initialization.  Similarly to what has been
noted recently in another (C) PR, it would be nice if we had toplevel cv
stripping variant of typeof or some special builtin that could wrap
typeof or some type and could be used in places where typeof can,
  __strip_cv (typeof (q)) = (__strip_cv (typeof (q))) p;
or
  typeof (q) = (__strip_cv (typeof (q))) p;
or
  __strip_cv (const int *const) z;
where the last one would be effectively
  const int *z;

Jakub

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Jonathan Wakely]

On 25/05/17 11:07 +0100, Jonathan Wakely wrote:

On 25/05/17 10:05 +0200, Andreas Schwab wrote:

../../gcc/ada/gcc-interface/utils2.c: In function 'int 
compare_elmt_bitpos(const void*, const void*)':
../../gcc/ada/gcc-interface/utils2.c:1937:73: error: type qualifiers ignored on 
cast result type [-Werror=ignored-qualifiers]
 const constructor_elt * const elmt1 = (const constructor_elt * const) rt1;
   ^~~
../../gcc/ada/gcc-interface/utils2.c:1938:73: error: type qualifiers ignored on 
cast result type [-Werror=ignored-qualifiers]
 const constructor_elt * const elmt2 = (const constructor_elt * const) rt2;


I'm testing this obvious fix.


Committed as r248458 because it gets bootstrap past the error above,
although now Ada fails for me with:

/home/jwakely/src/gcc/bootstrap/./gcc/xgcc 
-B/home/jwakely/src/gcc/bootstrap/./gcc/ -B/usr/local/x86_64-pc-linux-gnu/bin/ 
-B/usr/local/x86_64-pc-linux-gnu/lib/ -isystem 
/usr/local/x86_64-pc-linux-gnu/include -isystem 
/usr/local/x86_64-pc-linux-gnu/sys-include-c -g -O2 -m32 -fpic  -W -Wall 
-gnatpg -nostdinc -m32  s-regpat.adb -o s-regpat.o

raised STORAGE_ERROR : stack overflow or erroneous memory access
../gcc-interface/Makefile:296: recipe for target 's-regpat.o' failed



diff --git a/gcc/ada/gcc-interface/utils2.c b/gcc/ada/gcc-interface/utils2.c
index fc6f1b8..cd37791 100644
--- a/gcc/ada/gcc-interface/utils2.c
+++ b/gcc/ada/gcc-interface/utils2.c
@@ -1934,8 +1934,8 @@ build_call_raise_range (int msg, Node_Id gnat_node, char 
kind,
static int
compare_elmt_bitpos (const PTR rt1, const PTR rt2)
{
-  const constructor_elt * const elmt1 = (const constructor_elt * const) rt1;
-  const constructor_elt * const elmt2 = (const constructor_elt * const) rt2;
+  const constructor_elt * const elmt1 = (const constructor_elt *) rt1;
+  const constructor_elt * const elmt2 = (const constructor_elt *) rt2;
  const_tree const field1 = elmt1->index;
  const_tree const field2 = elmt2->index;
  const int ret

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Jonathan Wakely]

On 25/05/17 10:05 +0200, Andreas Schwab wrote:

../../gcc/ada/gcc-interface/utils2.c: In function 'int 
compare_elmt_bitpos(const void*, const void*)':
../../gcc/ada/gcc-interface/utils2.c:1937:73: error: type qualifiers ignored on 
cast result type [-Werror=ignored-qualifiers]
  const constructor_elt * const elmt1 = (const constructor_elt * const) rt1;
^~~
../../gcc/ada/gcc-interface/utils2.c:1938:73: error: type qualifiers ignored on 
cast result type [-Werror=ignored-qualifiers]
  const constructor_elt * const elmt2 = (const constructor_elt * const) rt2;


I'm testing this obvious fix.


diff --git a/gcc/ada/gcc-interface/utils2.c b/gcc/ada/gcc-interface/utils2.c
index fc6f1b8..cd37791 100644
--- a/gcc/ada/gcc-interface/utils2.c
+++ b/gcc/ada/gcc-interface/utils2.c
@@ -1934,8 +1934,8 @@ build_call_raise_range (int msg, Node_Id gnat_node, char kind,
 static int
 compare_elmt_bitpos (const PTR rt1, const PTR rt2)
 {
-  const constructor_elt * const elmt1 = (const constructor_elt * const) rt1;
-  const constructor_elt * const elmt2 = (const constructor_elt * const) rt2;
+  const constructor_elt * const elmt1 = (const constructor_elt *) rt1;
+  const constructor_elt * const elmt2 = (const constructor_elt *) rt2;
   const_tree const field1 = elmt1->index;
   const_tree const field2 = elmt2->index;
   const int ret

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Jonathan Wakely]

On 24/05/17 20:09 -0700, Andrew Pinski wrote:

On Wed, May 24, 2017 at 8:07 PM, Andrew Pinski  wrote:

This change caused a bootstrap failure on aarch64-linux-gnu and
x86_64-linux-gnu:
In file included from ../../gcc/gcc/system.h:691:0,
 from ../../gcc/gcc/read-rtl.c:31:
../../gcc/gcc/read-rtl.c: In member function ‘const char*
md_reader::apply_iterator_to_string(const char*)’:
../../gcc/gcc/../include/libiberty.h:722:38: error: type qualifiers
ignored on cast result type [-Werror=ignored-qualifiers]
 # define alloca(x) __builtin_alloca(x)
  ^
../../gcc/gcc/../include/libiberty.h:727:47: note: in expansion of
macro ‘alloca’
char *const libiberty_nptr = (char *const) alloca (libiberty_len); \
   ^~
../../gcc/gcc/read-rtl.c:380:21: note: in expansion of macro ‘ASTRDUP’
   base = p = copy = ASTRDUP (string);
 ^~~

I know you did not touch libiberty.h but that is emitting an error.
Did you test your patch with a full bootstrap?  I thought that was
recorded as being required now for C++ patches; I know a few years
back when the GCC was not compiling as C++, it was not required.


Oh it looks like it was already fixed by revision 248442.  Just my
build automated build was not done for that timeframe.


I thought I did, but a --disable-bootstrap slipped in there,
copy from another build, sorry.

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Andreas Schwab]

../../gcc/ada/gcc-interface/utils2.c: In function 'int 
compare_elmt_bitpos(const void*, const void*)':
../../gcc/ada/gcc-interface/utils2.c:1937:73: error: type qualifiers ignored on 
cast result type [-Werror=ignored-qualifiers]
   const constructor_elt * const elmt1 = (const constructor_elt * const) rt1;
 ^~~
../../gcc/ada/gcc-interface/utils2.c:1938:73: error: type qualifiers ignored on 
cast result type [-Werror=ignored-qualifiers]
   const constructor_elt * const elmt2 = (const constructor_elt * const) rt2;
 ^~~

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Andrew Pinski]

On Wed, May 24, 2017 at 8:07 PM, Andrew Pinski  wrote:
> On Wed, May 24, 2017 at 12:29 PM, Jonathan Wakely  wrote:
>> On 24/05/17 14:50 -0400, Jason Merrill wrote:
>>>
>>> On Wed, May 24, 2017 at 10:20 AM, Jonathan Wakely 
>>> wrote:

 On 23/05/17 16:26 -0400, Jason Merrill wrote:
>
>
> On Tue, May 23, 2017 at 2:00 PM, Jonathan Wakely 
> wrote:
>>
>>
>> On 19/05/17 15:14 -0400, Jason Merrill wrote:
>>>
>>>
>>>
>>> On Thu, Apr 27, 2017 at 12:59 PM, Jonathan Wakely 
>>> wrote:



 I also tried to add a warning like EDG's (see the PR) but it gave a
 false positive for direct-list-init of scoped enums (P0138R2,
 r240449)
 because that code goes through build_c_cast to perform the
 conversion,
 so looks like a cast to my new warning.
>>>
>>>
>>>
>>>
>>> The enum init code could strip the cv-quals when calling build_c_cast
>>> to avoid the warning.
>>
>>
>>
>>
>> Thanks, that works. I don't think this warning fits under any existing
>> option, so I'll add a new one. -Wuseless-cast-qual maybe? Or is that
>> too close to -Wuseless-cast and -Wcast-qual and would cause confusion?
>
>
>
> -Wcast-rvalue-qual ?



 I realised that -Wignored-qualifiers is a good fit. That warns about
 ignored cv-qualifiers on return types, which is a very similar case.

 This patch adds a new function to conditionally warn and calls that
 from the build_*_cast functions after they produce a valid result. I
 originally put the warnings next to the cv_unqualified calls that
 strip the quals, but was getting duplicate warnings when build_cp_cast
 calls more than one of the build_*_cast_1 functions.

 This also removes the unnecessary check for reference types that
 Nathan pointed out.

 Tested x86_64-linux and powerpc64-linux. OK for trunk?
>>>
>>>
 +/*
 +  Warns if the cast ignores cv-qualifiers on TYPE.
 + */
>>>
>>>
>>> The GCC sources don't put /* and */ on their own line.  OK with that
>>> change, thanks!
>>
>>
>> OK, I'll also fix it on the maybe_warn_about_useless_cast function
>> just above, which I copied :-)
>
> This change caused a bootstrap failure on aarch64-linux-gnu and
> x86_64-linux-gnu:
> In file included from ../../gcc/gcc/system.h:691:0,
>  from ../../gcc/gcc/read-rtl.c:31:
> ../../gcc/gcc/read-rtl.c: In member function ‘const char*
> md_reader::apply_iterator_to_string(const char*)’:
> ../../gcc/gcc/../include/libiberty.h:722:38: error: type qualifiers
> ignored on cast result type [-Werror=ignored-qualifiers]
>  # define alloca(x) __builtin_alloca(x)
>   ^
> ../../gcc/gcc/../include/libiberty.h:727:47: note: in expansion of
> macro ‘alloca’
> char *const libiberty_nptr = (char *const) alloca (libiberty_len); \
>^~
> ../../gcc/gcc/read-rtl.c:380:21: note: in expansion of macro ‘ASTRDUP’
>base = p = copy = ASTRDUP (string);
>  ^~~
>
> I know you did not touch libiberty.h but that is emitting an error.
> Did you test your patch with a full bootstrap?  I thought that was
> recorded as being required now for C++ patches; I know a few years
> back when the GCC was not compiling as C++, it was not required.

Oh it looks like it was already fixed by revision 248442.  Just my
build automated build was not done for that timeframe.

Thanks,
Andrew

>
> Thanks,
> Andrew
>
>
>>
>>

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Andrew Pinski]

On Wed, May 24, 2017 at 12:29 PM, Jonathan Wakely  wrote:
> On 24/05/17 14:50 -0400, Jason Merrill wrote:
>>
>> On Wed, May 24, 2017 at 10:20 AM, Jonathan Wakely 
>> wrote:
>>>
>>> On 23/05/17 16:26 -0400, Jason Merrill wrote:


 On Tue, May 23, 2017 at 2:00 PM, Jonathan Wakely 
 wrote:
>
>
> On 19/05/17 15:14 -0400, Jason Merrill wrote:
>>
>>
>>
>> On Thu, Apr 27, 2017 at 12:59 PM, Jonathan Wakely 
>> wrote:
>>>
>>>
>>>
>>> I also tried to add a warning like EDG's (see the PR) but it gave a
>>> false positive for direct-list-init of scoped enums (P0138R2,
>>> r240449)
>>> because that code goes through build_c_cast to perform the
>>> conversion,
>>> so looks like a cast to my new warning.
>>
>>
>>
>>
>> The enum init code could strip the cv-quals when calling build_c_cast
>> to avoid the warning.
>
>
>
>
> Thanks, that works. I don't think this warning fits under any existing
> option, so I'll add a new one. -Wuseless-cast-qual maybe? Or is that
> too close to -Wuseless-cast and -Wcast-qual and would cause confusion?



 -Wcast-rvalue-qual ?
>>>
>>>
>>>
>>> I realised that -Wignored-qualifiers is a good fit. That warns about
>>> ignored cv-qualifiers on return types, which is a very similar case.
>>>
>>> This patch adds a new function to conditionally warn and calls that
>>> from the build_*_cast functions after they produce a valid result. I
>>> originally put the warnings next to the cv_unqualified calls that
>>> strip the quals, but was getting duplicate warnings when build_cp_cast
>>> calls more than one of the build_*_cast_1 functions.
>>>
>>> This also removes the unnecessary check for reference types that
>>> Nathan pointed out.
>>>
>>> Tested x86_64-linux and powerpc64-linux. OK for trunk?
>>
>>
>>> +/*
>>> +  Warns if the cast ignores cv-qualifiers on TYPE.
>>> + */
>>
>>
>> The GCC sources don't put /* and */ on their own line.  OK with that
>> change, thanks!
>
>
> OK, I'll also fix it on the maybe_warn_about_useless_cast function
> just above, which I copied :-)

This change caused a bootstrap failure on aarch64-linux-gnu and
x86_64-linux-gnu:
In file included from ../../gcc/gcc/system.h:691:0,
 from ../../gcc/gcc/read-rtl.c:31:
../../gcc/gcc/read-rtl.c: In member function ‘const char*
md_reader::apply_iterator_to_string(const char*)’:
../../gcc/gcc/../include/libiberty.h:722:38: error: type qualifiers
ignored on cast result type [-Werror=ignored-qualifiers]
 # define alloca(x) __builtin_alloca(x)
  ^
../../gcc/gcc/../include/libiberty.h:727:47: note: in expansion of
macro ‘alloca’
char *const libiberty_nptr = (char *const) alloca (libiberty_len); \
   ^~
../../gcc/gcc/read-rtl.c:380:21: note: in expansion of macro ‘ASTRDUP’
   base = p = copy = ASTRDUP (string);
 ^~~

I know you did not touch libiberty.h but that is emitting an error.
Did you test your patch with a full bootstrap?  I thought that was
recorded as being required now for C++ patches; I know a few years
back when the GCC was not compiling as C++, it was not required.

Thanks,
Andrew


>
>

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Jonathan Wakely]

On 24/05/17 14:50 -0400, Jason Merrill wrote:

On Wed, May 24, 2017 at 10:20 AM, Jonathan Wakely  wrote:

On 23/05/17 16:26 -0400, Jason Merrill wrote:


On Tue, May 23, 2017 at 2:00 PM, Jonathan Wakely 
wrote:


On 19/05/17 15:14 -0400, Jason Merrill wrote:



On Thu, Apr 27, 2017 at 12:59 PM, Jonathan Wakely 
wrote:



I also tried to add a warning like EDG's (see the PR) but it gave a
false positive for direct-list-init of scoped enums (P0138R2, r240449)
because that code goes through build_c_cast to perform the conversion,
so looks like a cast to my new warning.




The enum init code could strip the cv-quals when calling build_c_cast
to avoid the warning.




Thanks, that works. I don't think this warning fits under any existing
option, so I'll add a new one. -Wuseless-cast-qual maybe? Or is that
too close to -Wuseless-cast and -Wcast-qual and would cause confusion?



-Wcast-rvalue-qual ?



I realised that -Wignored-qualifiers is a good fit. That warns about
ignored cv-qualifiers on return types, which is a very similar case.

This patch adds a new function to conditionally warn and calls that
from the build_*_cast functions after they produce a valid result. I
originally put the warnings next to the cv_unqualified calls that
strip the quals, but was getting duplicate warnings when build_cp_cast
calls more than one of the build_*_cast_1 functions.

This also removes the unnecessary check for reference types that
Nathan pointed out.

Tested x86_64-linux and powerpc64-linux. OK for trunk?



+/*
+  Warns if the cast ignores cv-qualifiers on TYPE.
+ */


The GCC sources don't put /* and */ on their own line.  OK with that
change, thanks!


OK, I'll also fix it on the maybe_warn_about_useless_cast function
just above, which I copied :-)


diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index ecc7190..b81d6c8 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -6655,9 +6655,7 @@ check_for_casting_away_constness (tree src_type, tree dest_type,
 }
 }
 
-/*
-  Warns if the cast from expression EXPR to type TYPE is useless.
- */
+/* Warns if the cast from expression EXPR to type TYPE is useless.  */
 void
 maybe_warn_about_useless_cast (tree type, tree expr, tsubst_flags_t complain)
 {
@@ -6673,9 +6671,7 @@ maybe_warn_about_useless_cast (tree type, tree expr, tsubst_flags_t complain)
 }
 }
 
-/*
-  Warns if the cast ignores cv-qualifiers on TYPE.
- */
+/* Warns if the cast ignores cv-qualifiers on TYPE.  */
 void
 maybe_warn_about_cast_ignoring_quals (tree type, tsubst_flags_t complain)
 {

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Jason Merrill]

On Wed, May 24, 2017 at 10:20 AM, Jonathan Wakely  wrote:
> On 23/05/17 16:26 -0400, Jason Merrill wrote:
>>
>> On Tue, May 23, 2017 at 2:00 PM, Jonathan Wakely 
>> wrote:
>>>
>>> On 19/05/17 15:14 -0400, Jason Merrill wrote:


 On Thu, Apr 27, 2017 at 12:59 PM, Jonathan Wakely 
 wrote:
>
>
> I also tried to add a warning like EDG's (see the PR) but it gave a
> false positive for direct-list-init of scoped enums (P0138R2, r240449)
> because that code goes through build_c_cast to perform the conversion,
> so looks like a cast to my new warning.



 The enum init code could strip the cv-quals when calling build_c_cast
 to avoid the warning.
>>>
>>>
>>>
>>> Thanks, that works. I don't think this warning fits under any existing
>>> option, so I'll add a new one. -Wuseless-cast-qual maybe? Or is that
>>> too close to -Wuseless-cast and -Wcast-qual and would cause confusion?
>>
>>
>> -Wcast-rvalue-qual ?
>
>
> I realised that -Wignored-qualifiers is a good fit. That warns about
> ignored cv-qualifiers on return types, which is a very similar case.
>
> This patch adds a new function to conditionally warn and calls that
> from the build_*_cast functions after they produce a valid result. I
> originally put the warnings next to the cv_unqualified calls that
> strip the quals, but was getting duplicate warnings when build_cp_cast
> calls more than one of the build_*_cast_1 functions.
>
> This also removes the unnecessary check for reference types that
> Nathan pointed out.
>
> Tested x86_64-linux and powerpc64-linux. OK for trunk?

> +/*
> +  Warns if the cast ignores cv-qualifiers on TYPE.
> + */

The GCC sources don't put /* and */ on their own line.  OK with that
change, thanks!

Jason

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Jonathan Wakely]

On 23/05/17 16:26 -0400, Jason Merrill wrote:

On Tue, May 23, 2017 at 2:00 PM, Jonathan Wakely  wrote:

On 19/05/17 15:14 -0400, Jason Merrill wrote:


On Thu, Apr 27, 2017 at 12:59 PM, Jonathan Wakely 
wrote:


I also tried to add a warning like EDG's (see the PR) but it gave a
false positive for direct-list-init of scoped enums (P0138R2, r240449)
because that code goes through build_c_cast to perform the conversion,
so looks like a cast to my new warning.



The enum init code could strip the cv-quals when calling build_c_cast
to avoid the warning.



Thanks, that works. I don't think this warning fits under any existing
option, so I'll add a new one. -Wuseless-cast-qual maybe? Or is that
too close to -Wuseless-cast and -Wcast-qual and would cause confusion?


-Wcast-rvalue-qual ?


I realised that -Wignored-qualifiers is a good fit. That warns about
ignored cv-qualifiers on return types, which is a very similar case.

This patch adds a new function to conditionally warn and calls that
from the build_*_cast functions after they produce a valid result. I
originally put the warnings next to the cv_unqualified calls that
strip the quals, but was getting duplicate warnings when build_cp_cast
calls more than one of the build_*_cast_1 functions.

This also removes the unnecessary check for reference types that
Nathan pointed out.

Tested x86_64-linux and powerpc64-linux. OK for trunk?


commit dddbdfbdc8de6565b45a6d794148dfe077d83d49
Author: Jonathan Wakely 
Date:   Wed May 24 14:09:36 2017 +0100

PR c++/80544 strip cv-quals from cast results

gcc/cp:

	PR c++/80544
	* tree.c (reshape_init): Use unqualified type for direct enum init.
	* typeck.c (maybe_warn_about_cast_ignoring_quals): New.
	(build_static_cast_1, build_reinterpret_cast_1): Strip cv-quals from
	non-class destination types.
	(build_const_cast_1): Strip cv-quals from destination types.
	(build_static_cast, build_reinterpret_cast, build_const_cast)
	(cp_build_c_cast): Add calls to maybe_warn_about_cast_ignoring_quals.

gcc/testsuite:

	PR c++/80544
	* g++.dg/expr/cast11.C: New test.

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index afd47bb..3ff0130 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -6043,6 +6043,7 @@ reshape_init (tree type, tree init, tsubst_flags_t complain)
   if (is_direct_enum_init (type, init))
 {
   tree elt = CONSTRUCTOR_ELT (init, 0)->value;
+  type = cv_unqualified (type);
   if (check_narrowing (ENUM_UNDERLYING_TYPE (type), elt, complain))
 	return cp_build_c_cast (type, elt, tf_warning_or_error);
   else
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 13d90a6..ecc7190 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -6673,6 +6673,22 @@ maybe_warn_about_useless_cast (tree type, tree expr, tsubst_flags_t complain)
 }
 }
 
+/*
+  Warns if the cast ignores cv-qualifiers on TYPE.
+ */
+void
+maybe_warn_about_cast_ignoring_quals (tree type, tsubst_flags_t complain)
+{
+  if (warn_ignored_qualifiers
+  && complain & tf_warning
+  && !CLASS_TYPE_P (type)
+  && (cp_type_quals (type) & (TYPE_QUAL_CONST|TYPE_QUAL_VOLATILE)))
+{
+  warning (OPT_Wignored_qualifiers, "type qualifiers ignored on cast "
+	   "result type");
+}
+}
+
 /* Convert EXPR (an expression with pointer-to-member type) to TYPE
(another pointer-to-member type in the same hierarchy) and return
the converted expression.  If ALLOW_INVERSE_P is permitted, a
@@ -6746,6 +6762,10 @@ build_static_cast_1 (tree type, tree expr, bool c_cast_p,
   /* Save casted types in the function's used types hash table.  */
   used_types_insert (type);
 
+  /* A prvalue of non-class type is cv-unqualified.  */
+  if (!CLASS_TYPE_P (type))
+type = cv_unqualified (type);
+
   /* [expr.static.cast]
 
  An lvalue of type "cv1 B", where B is a class type, can be cast
@@ -7035,7 +7055,10 @@ build_static_cast (tree type, tree expr, tsubst_flags_t complain)
   if (valid_p)
 {
   if (result != error_mark_node)
-	maybe_warn_about_useless_cast (type, expr, complain);
+	{
+	  maybe_warn_about_useless_cast (type, expr, complain);
+	  maybe_warn_about_cast_ignoring_quals (type, complain);
+	}
   return result;
 }
 
@@ -7108,6 +7131,10 @@ build_reinterpret_cast_1 (tree type, tree expr, bool c_cast_p,
   /* Save casted types in the function's used types hash table.  */
   used_types_insert (type);
 
+  /* A prvalue of non-class type is cv-unqualified.  */
+  if (!CLASS_TYPE_P (type))
+type = cv_unqualified (type);
+
   /* [expr.reinterpret.cast]
  An lvalue expression of type T1 can be cast to the type
  "reference to T2" if an expression of type "pointer to T1" can be
@@ -7289,7 +7316,10 @@ build_reinterpret_cast (tree type, tree expr, tsubst_flags_t complain)
   r = build_reinterpret_cast_1 (type, expr, /*c_cast_p=*/false,
 /*valid_p=*/NULL, complain);
   if (r != 

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Jason Merrill]

On Tue, May 23, 2017 at 2:00 PM, Jonathan Wakely  wrote:
> On 19/05/17 15:14 -0400, Jason Merrill wrote:
>>
>> On Thu, Apr 27, 2017 at 12:59 PM, Jonathan Wakely 
>> wrote:
>>>
>>> I also tried to add a warning like EDG's (see the PR) but it gave a
>>> false positive for direct-list-init of scoped enums (P0138R2, r240449)
>>> because that code goes through build_c_cast to perform the conversion,
>>> so looks like a cast to my new warning.
>>
>>
>> The enum init code could strip the cv-quals when calling build_c_cast
>> to avoid the warning.
>
>
> Thanks, that works. I don't think this warning fits under any existing
> option, so I'll add a new one. -Wuseless-cast-qual maybe? Or is that
> too close to -Wuseless-cast and -Wcast-qual and would cause confusion?

-Wcast-rvalue-qual ?

Jason

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Jonathan Wakely]

On 23/05/17 13:58 -0400, Nathan Sidwell wrote:

On 05/23/2017 01:56 PM, Jonathan Wakely wrote:

On 18/05/17 13:44 -0400, Nathan Sidwell wrote:


References can't be CV qualified, so the REFERENCE_TYPE check 
seems superfluous?


True. I did it because that matches the semantics of the cast
according to the standard, but it isn't needed here. Is it worth
keeping anyway, to avoid a redundant call to cv_unqualified?


I don't think it's worth checking.


Ah yes, cp_build_qualified_type_real returns early if there's nothing
to do. OK, I'll remove those checks. Thanks.

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Jonathan Wakely]

On 19/05/17 15:14 -0400, Jason Merrill wrote:

On Thu, Apr 27, 2017 at 12:59 PM, Jonathan Wakely  wrote:

I also tried to add a warning like EDG's (see the PR) but it gave a
false positive for direct-list-init of scoped enums (P0138R2, r240449)
because that code goes through build_c_cast to perform the conversion,
so looks like a cast to my new warning.


The enum init code could strip the cv-quals when calling build_c_cast
to avoid the warning.


Thanks, that works. I don't think this warning fits under any existing
option, so I'll add a new one. -Wuseless-cast-qual maybe? Or is that
too close to -Wuseless-cast and -Wcast-qual and would cause confusion?

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Nathan Sidwell]

On 05/23/2017 01:56 PM, Jonathan Wakely wrote:

On 18/05/17 13:44 -0400, Nathan Sidwell wrote:


References can't be CV qualified, so the REFERENCE_TYPE check seems 
superfluous?


True. I did it because that matches the semantics of the cast
according to the standard, but it isn't needed here. Is it worth
keeping anyway, to avoid a redundant call to cv_unqualified?


I don't think it's worth checking.


It also occurs to me that checking for !CLASS_TYPE_P (type) isn't
needed in build_const_cast_1 because you can't const_cast to a class
type, only reference or pointer types.


true.

nathan

--
Nathan Sidwell

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Jonathan Wakely]

On 18/05/17 13:44 -0400, Nathan Sidwell wrote:

On 05/18/2017 01:40 PM, Jonathan Wakely wrote:


+  /* A prvalue of non-class type is cv-unqualified.  */
+  if (TREE_CODE (type) != REFERENCE_TYPE && !CLASS_TYPE_P (type))
+type = cv_unqualified (type);
+


References can't be CV qualified, so the REFERENCE_TYPE check seems 
superfluous?


True. I did it because that matches the semantics of the cast
according to the standard, but it isn't needed here. Is it worth
keeping anyway, to avoid a redundant call to cv_unqualified?

It also occurs to me that checking for !CLASS_TYPE_P (type) isn't
needed in build_const_cast_1 because you can't const_cast to a class
type, only reference or pointer types.

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Jason Merrill]

On Thu, Apr 27, 2017 at 12:59 PM, Jonathan Wakely  wrote:
> I also tried to add a warning like EDG's (see the PR) but it gave a
> false positive for direct-list-init of scoped enums (P0138R2, r240449)
> because that code goes through build_c_cast to perform the conversion,
> so looks like a cast to my new warning.

The enum init code could strip the cv-quals when calling build_c_cast
to avoid the warning.

Jason

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Nathan Sidwell]

On 05/18/2017 01:40 PM, Jonathan Wakely wrote:


+  /* A prvalue of non-class type is cv-unqualified.  */
+  if (TREE_CODE (type) != REFERENCE_TYPE && !CLASS_TYPE_P (type))
+type = cv_unqualified (type);
+


References can't be CV qualified, so the REFERENCE_TYPE check seems 
superfluous?


nathan
--
Nathan Sidwell

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Jonathan Wakely]

Ping for https://gcc.gnu.org/ml/gcc-patches/2017-04/msg01414.html ...

On 27/04/17 17:59 +0100, Jonathan Wakely wrote:

This is probably not the best way to do this, but it seems to work.

I also tried to add a warning like EDG's (see the PR) but it gave a
false positive for direct-list-init of scoped enums (P0138R2, r240449)
because that code goes through build_c_cast to perform the conversion,
so looks like a cast to my new warning.

Tested x86_64-linux, OK for trunk?


gcc/cp:

PR c++/80544
* typeck.c (build_static_cast_1, build_reinterpret_cast_1)
(build_const_cast_1): Strip cv-quals from non-class prvalue results.

gcc/testsuite:

PR c++/80544
* g++.dg/expr/cast11.C: New test.





commit 47763f3c84de86dd1ebbaac73e341e2de9b5be68
Author: Jonathan Wakely 
Date:   Thu Apr 27 16:49:25 2017 +0100

   PR c++/80544 strip cv-quals from cast results
   
   gcc/cp:
   
   	PR c++/80544

* typeck.c (build_static_cast_1, build_reinterpret_cast_1)
(build_const_cast_1): Strip cv-quals from non-class prvalue results.
   
   gcc/testsuite:
   
   	PR c++/80544

* g++.dg/expr/cast11.C: New test.

diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 7aee0d6..e41b335 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -6708,6 +6708,10 @@ build_static_cast_1 (tree type, tree expr, bool c_cast_p,
  /* Save casted types in the function's used types hash table.  */
  used_types_insert (type);

+  /* A prvalue of non-class type is cv-unqualified.  */
+  if (TREE_CODE (type) != REFERENCE_TYPE && !CLASS_TYPE_P (type))
+type = cv_unqualified (type);
+
  /* [expr.static.cast]

 An lvalue of type "cv1 B", where B is a class type, can be cast
@@ -7070,6 +7074,10 @@ build_reinterpret_cast_1 (tree type, tree expr, bool 
c_cast_p,
  /* Save casted types in the function's used types hash table.  */
  used_types_insert (type);

+  /* A prvalue of non-class type is cv-unqualified.  */
+  if (TREE_CODE (type) != REFERENCE_TYPE && !CLASS_TYPE_P (type))
+type = cv_unqualified (type);
+
  /* [expr.reinterpret.cast]
 An lvalue expression of type T1 can be cast to the type
 "reference to T2" if an expression of type "pointer to T1" can be
@@ -7300,6 +7308,10 @@ build_const_cast_1 (tree dst_type, tree expr, 
tsubst_flags_t complain,
  /* Save casted types in the function's used types hash table.  */
  used_types_insert (dst_type);

+  /* A prvalue of non-class type is cv-unqualified.  */
+  if (TREE_CODE (dst_type) != REFERENCE_TYPE && !CLASS_TYPE_P (dst_type))
+dst_type = cv_unqualified (dst_type);
+
  src_type = TREE_TYPE (expr);
  /* Expressions do not really have reference types.  */
  if (TREE_CODE (src_type) == REFERENCE_TYPE)
diff --git a/gcc/testsuite/g++.dg/expr/cast11.C 
b/gcc/testsuite/g++.dg/expr/cast11.C
new file mode 100644
index 000..642087e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/expr/cast11.C
@@ -0,0 +1,34 @@
+// { dg-do compile { target c++11 } }
+// c++/80544 cast expressions returned cv-qualified prvalues
+
+template void f(T&&) { }
+template void f(T const&&) = delete;
+
+template void g(T&&) = delete;
+template void g(T const&&) { }
+
+struct B { int i; } b;
+
+void f1()
+{
+  int i = 0;
+  f((long const)i);
+  f((int* const));
+  f((int const* const));
+  f((long* const));
+
+  f(static_cast(i));
+  f(reinterpret_cast());
+
+  f(static_cast());
+  f(const_cast());
+  f(reinterpret_cast());
+
+  using ptrmem = int B::*;
+  f(static_cast(::i));
+  f(const_cast(::i));
+  f(reinterpret_cast(::i));
+
+  // prvalue of class type can have cv-quals:
+  g(static_cast(b));
+}

Re: [PATCH] PR c++/80544 strip cv-quals from cast results

[Paolo Carlini]

Hi,

On 27/04/2017 18:59, Jonathan Wakely wrote:

This is probably not the best way to do this, but it seems to work.
Eventually, if this is the way to go, a small maybe_strip_* helper could 
tidy a bit the code...


Paolo.