public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Reverting your patch from 2004-06-24
@ 2004-07-06  7:32 Mark Mitchell
  2004-07-06 22:33 ` Bug 16115, C++ invisible references (was: Reverting your patch from 2004-06-24) Jason Merrill
  0 siblings, 1 reply; 18+ messages in thread
From: Mark Mitchell @ 2004-07-06  7:32 UTC (permalink / raw)
  To: gcc-patches, jason


Jason --

I've reverted this patch:

	2004-06-24  Jason Merrill  <jason@redhat.com>
	PR c++/16115
	* decl.c (grokparms): Give the PARM_DECL reference type if the
	parameter is passed by invisible reference.

because (a) it's slightly buggy, and (b) I think it's the wrong
approach.

I recognize that I've exceeded my mandate here, in that I've just
reverted the patch without the usual 48-hour deal, but I'm counting on
the fact that you and I have always resolved differences effectively
to allow me to avoid massive wrath from the direction of greater
Boston.  If you're not happy, well, revert my reversion, and we'll
find another way to work it out.

Here's what I put into bugzilla:

==

This patch is incorrect, on several levels.

First, it changes the type of the PARM_DECL to be different from the
type that C++ assigns that declaration.  That is incorrect; the C++
front end's assignment of types should match the language.  As we've
discussed previously, for example, it's a bug that we have expressions
with REFERENCE_TYPE; the standard says that no expressions have that
type.  (Declarations can, of course, have REFERENCE_TYPE -- but these
parameters do not.)  We should be trying to minimize the extent to
which ABI details appear in the front end.

Furthermore, this test case:

  template <typename T>
  struct S {
    ~S();
  };
  struct T {
    T(S<int>);
    S<int> s_;
  };
  T::T(S<int> s) : s_(s) {
  }

now ICEs the compiler on (at least) powerpc-apple-darwin7.4.0 due to
some problem with cloned functions.

Also, this change is potentially pessimizing code in that it will no
longer be obvious to the optimizers that the static type of the
parameter must in fact be the declared type.  So, we may not be able
to resolve virtual function invoked on the parameter.

Finally, if we were going to make this change in the front end, the
right place would be in grokdeclarator (where the PARM_DECL is
created) rather than in grokparms, where we must then wastefully
create a second PARM_DECL.

If the optimizers needs these parameters to have reference type, then
that change needs to happen after the front end processing is
complete, in the course of generating code for this function.  I'm not
sure what the tradeoffs are vis a vis (re)teaching the optimizers
about TREE_ADDRESSABLE on types.

I've reverted this patch.  

I'll be happy to help with an alternate solution in whatever way I
can.

(Parenthetically, I'm not sure the call1.C test is checking anything
useful, since it does not fail even after I reverted the patch.)

==

--
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com

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

end of thread, other threads:[~2004-07-14 22:46 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-07-06  7:32 Reverting your patch from 2004-06-24 Mark Mitchell
2004-07-06 22:33 ` Bug 16115, C++ invisible references (was: Reverting your patch from 2004-06-24) Jason Merrill
2004-07-06 22:42   ` Bug 16115, C++ invisible references Mark Mitchell
2004-07-06 22:54     ` Phil Edwards
2004-07-06 23:05       ` Mark Mitchell
2004-07-06 23:15     ` Steven Bosscher
2004-07-07  1:32       ` Mark Mitchell
2004-07-07 17:53     ` Jason Merrill
2004-07-07 18:22       ` Mark Mitchell
2004-07-07 19:40         ` Richard Henderson
2004-07-07 20:29           ` Mark Mitchell
2004-07-07 21:16             ` Richard Henderson
2004-07-07 21:17               ` Mark Mitchell
2004-07-07 21:35                 ` Richard Henderson
2004-07-07 21:41                   ` Mark Mitchell
2004-07-07 21:58                     ` Gabriel Dos Reis
2004-07-15  7:27                     ` Jason Merrill
2004-07-15  7:28                       ` Mark Mitchell

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