This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Thu, Dec 06, 2007 at 08:31:17PM -0800, Mark Mitchell wrote:
> Jakub Jelinek wrote:
> 
> > The following patch (which obsoletes my earlier 34238 patch) fixes all of
> > this, but letting cgraph_optimize actually emit all the vars into assembly
> > and only checking afterwards if we emitted something we shouldn't, which
> > seems to be much more reliable.
> 
> The diagnostic is not about what the compiler happens to end up spitting
> out; it's about the user's program.  If you do:
> 
>   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.  With
struct A { static int i; }; static int foo() { return A::i; } int main () {}
as whole program we wouldn't diagnose this either.  
To me it is more important that we don't error on valid programs (especially
a lot of boost using code is broken), and when all references are optimized
out, no real harm is done, compiler doesn't make up something unexpected.
When compiler puts it into bss, it can e.g. do so even for objects that need
constructing, without running any constructors on it etc.  That's something
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.

BTW, is the boost testcase valid?
template<typename T, typename U> struct C
{
  static T ca;
  static const int value = sizeof (D<U>::foo (ca, 0)) == sizeof (int);
};
is instantiated with T in anon namespace (so ca is also limited to current
CU), but it is (intentionally) never defined and only used in sizeof
expression.  [class.static.data]/5 says
"There shall be exactly one definition of a static data member that is used in a
program"
but it is not clear (at least to me) what exactly counts as use.
Boost certainly wouldn't like to pollute bss of programs with many
(sometimes big) variables that nobody ever uses.

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.
Though, waiting for that in a state which at least errors out when real
harm is done rather than being silent even for those cases is IMHO
preferrable.

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]