Re: [Bug-zile] astr_cstr

[Gary V. Vaughan]

Hi Reuben,

On 7 Sep 2011, at 18:44, Reuben Thomas wrote:

> Re. a recent commit:
> 
> commit 5171aa758f7354e8db522f341a2dc55563e82a62
> Author: Gary V. Vaughan <address@hidden>
> Date:   Thu Sep 1 21:57:27 2011 +0700
> 
>    compiler warning: assignment to as->text discards const
> 
>    remove the constness of the string passed to castr_new_nstr, because
>    once its address is held in as->text, the rest of the code will not
>    treat it as const anyway; thus, we also have to remove the constness of
>    return type of astr_cstr() which is simply giving the address in the
>    non-const as->text pointer too.
> 
> I am unhappy with this, because it removes compiler warnings about two
> sorts of potential bug:
> 
> 1. Code overwrites the return value of astr_cstr and the compiler
> doesn't complain. (astr_cstr returns a read-only copy of an astr as a
> char *; it's pure coincidence that it happens to be the actual
> representation too.)
> 
> 2. Code passes a non-const string to castr_new_nstr. The whole point
> of this constructor is that the string really is const! (Look at the
> code for castr_new_nstr). The underlying problem here is trying to
> have two structured types, astr and castr, which are compatible but
> one of which is immutable.
> 
> This is a good example of where trying to keep the compiler happy
> actually makes things worse (and why the GCS does not insist on
> warning-free compilation, even with the GNU compiler).

Ugh.  True.  My analysis was not quite right then :(  Sorry again.

Feel free to revert.

Maybe this manywarnings macro is really far more trouble than it's worth?
It's certainly cost us quite a lot of time already, that would have been
better spent on fixing real bugs or doing more testing... instead we have
made the compiler noisier, and either have to pick through evermore
warnings and look hard at the code to decide whether they are important
or not to make sense of them (and again every time we use --enable-gcc-warnings
if we haven't looked at the code for a few weeks and can't remember which
warnings are new and worth investigating, and which are useless noise), or
else fiddle with the code like this to make the unimportant ones go away
after poring over the relevant parts of the code once so that the remaining
warnings are actually useful.

I think, like you say elsewhere, if there's no way to silence the warnings
on a case by case basis when the proper analysis of each warning is complete,
then they become less and less useful overall as we more and more of them.

Cheers,
-- 
Gary V. Vaughan (gary AT gnu DOT org)