Re: [C++ PATCH] Fix regressions introduced by my PR34094 fix (PR c++/34238, c++/34340)

[Mark Mitchell <mark at codesourcery dot com>]

Jakub Jelinek wrote:
> On Fri, Dec 07, 2007 at 12:52:31PM -0800, Mark Mitchell wrote:
>> Jakub Jelinek wrote:
>>
>>>>   namespace { struct A { static int i; }; }
>>>>   static int foo() { return A::i; }
>>>>
>>>> the program is still erroneous, even though the compiler may likely
>>>> optimize away foo, and the reference to A::i.
>>> That's true, but the standard explicitly says the diagnostic is not
>>> required.
>> It does?  Where?  (I'm not disagreeing, I'm just surprised, and I
>> couldn't find a pointer.)
> 
> [class.static.data]/5:
> "There shall be exactly one definition of a static data member that is used in a program; no diagnostic is required; see 3.2."

Thanks.

However, I would suspect that there is also something that says that if
you use the value of a variable (whether a static data member or not),
the variable must be defined in the program.  I'm sure that you're
right, though, thinking a bit more about it; that probably says that no
diagnostic is required too.

> The patch I posted last Wednesday diagnosed the errors basically for the
> same stuff that ld would diagnose it, if anon namespace was not involved.
> When all uses are optimized out, etc., ld will not error nor warn either.
> 
>>> that should be warned about.  If we have some bit which we can really rely
>>> on to be set reliably for these purposes, then the warning can be precise,
>>> but TREE_USED is unfortunately not it.
>> TREE_USED should be accurate.  It should tell us whether or not a thing
>> was used by the input.  If TREE_USED is not set when it should be, or
>> set when it shouldn't be, we have a problem.
> 
> But it is set e.g. when seen in sizeof as shown on that boost snippet.

Hmph, I see.  I suspect that the standard itself isn't consistent on
this point.  For example, there is language that says that if you use
the name of a member in a class definition, the member name must refer
to the same thing after the definition.  For example:

  const int i = 7;
  struct S {
     int a[i];
     static const int i = 3;
  };

is erroneous.  But, surely:

  int i;
  struct S {
     int a[sizeof(i)];
     double i;
  };

is also erroneous.  So, in this context "use" must mean what TREE_USED
is meaning.

Frankly, I think the Boost code is trying too hard to be clever.
Defining the variable is not a big deal, and if the compiler can pull
out all references to the variable, it can pull out the variable too!
But, what I think doesn't necessarily matter...

>>> I'm afraid I don't know the FE enough to further improve this warning
>>> without breaking valid programs, so the other possibility for me is just to
>>> back out the original patch and leave this can of worms to you/Jason.
>> All right; let's revert your original patch, then.  AIUI, that patch
>> fixed an accepts-invalid problem; that's better than rejects-valid problems.
> 
> If you prefer that, here is a patch I've bootstrapped/regtested on
> x86_64-linux.

OK, please apply that.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713