Bug 16174

Summary: [3.4/4.0 Regression] deducing top-level consts
Product: gcc Reporter: Giovanni Bajo <giovannibajo>
Component: c++Assignee: Nathan Sidwell <nathan>
Status: RESOLVED FIXED    
Severity: normal CC: gcc-bugs, mmitchel, nathan
Priority: P2 Keywords: patch, rejects-valid
Version: 4.0.0   
Target Milestone: 3.4.1   
Host: Target:
Build: Known to work:
Known to fail: Last reconfirmed:

Description Giovanni Bajo 2004-06-24 10:35:35 UTC
This comes from Rani Sharoni's implementation of auto_ptr:
http://tinyurl.com/2l2ge

--------------------------------------------------
template <class T> struct K 
{
  K();

  K(K<T> & rhs);
  K(K<T> const& rhs);
  template <class U> K(K<U> const& rhs);

private:
  template <class U> struct A;
  template <class U> struct A< K<U> const>
  {  typedef typename K<U>::compile_time_error type; };

  // This is used to reject calls to the copy constructor
  //  with objects which are top-level const. If they are
  //  const, the specialization of A is instantiated and
  //  causes a compile time error. Otherwise, the general
  //  template is picked up, it misses definition, so this
  //  ctor declaration is rejected by SFINAE and everybody
  //  is happy.
  // GCC 3.4.1pre and 3.5.0 always matches A's specialization
  //  when instantiating from foo(), and this causes the error.
  template <class U>
  K(U& rhs, typename A<U>::type = 0);
};


K<int> foo(void)
{
  return K<int>();
}
--------------------------------------------------

This code works on any released GCC since 3.0, but stopped working on 3.4 
branch and mainline lately, probably because of the work done by Nathan.

Nathan, can you check if the code is indeed invalid and we are now rightfully 
rejecting it? Latest version of EDG accepts it.
Comment 1 Giovanni Bajo 2004-06-24 11:01:48 UTC
Mark,

I think we want this targeted at 3.4.1 for stability: otherwise, the code would 
compile with 3.4.0, would be broken with 3.4.1 and compile again with 3.4.2. 
This is not desirable IMO. Plus, Nathan is looking into this so hopefully he 
can find out what it is wrong soon enough.
Comment 2 Nathan Sidwell 2004-06-24 12:39:22 UTC
This is a SFINAE failure. we deduce a call requiring a complete A<const K<int>>
in a non-deduced context (the default parameter).  We then do the final
tsubst of the function type, to make sure no bad tstubsting is happening,
but don't propagate the 'do not complain' flag to complete_type.

I suspect my recent patches have fixed a bug where we didn't get as far as we
do now.
Comment 3 Giovanni Bajo 2004-06-25 01:06:50 UTC
Nathan,

there is something I don't understand here, and with your patch. This is what 
should happen in my opinion:

- We try to instantiate the declaration of the templated copy ctor to add it to 
the overload set.
- We deduce U = K<int> (no const here!) from the first parameter
- We substitute U into the non deduced context (second parameter). This causes 
the instantiation of A<K<int> > (still no const).
- A<K<int> > matches the general template (not the specialization), which is 
incomplete. This triggers SFINAE.
- The declaration we were trying to instantiate is discarded and not added to 
the overload set.

Instead, if you have code like this:

void bar(void)
{
  const K<int> a;
  K<int> b(a);
}

the deduction will find U = K<int> const, and it will trigger the instantiation 
of A<K<int> const>. Whild doing so, we would be trying to access 
K<int>::compile_time_error which does not exist, and this is not SFINAE 
anymore! A diagnostic will be emitted and compilation is aborted.

The whoel point is that K is actually auto_ptr<>, and it is forbidden to do a 
cctor from a const auto_ptr<> (because it would effectively modify it, given 
the semantic of the pointer). This smart trick allows to detect and reject such 
situations.

If I'm reading your patch correctly, you are making so that 
instantiate_template also falls under SFINAE. This would prevent the function 
bar() above from emitting a diagnostic, and would make the whole trick totally 
useless.

I think I read a DR where John Spicer was saying that it is unfeasable to shut 
all the possible errors that could be emitted during (possibly cascaded)
template instantiations for the sake of SFINAE. In fact, this is what EDG does. 
If you try it on g++.old-deja/g++.pt/crash30.C, it will emit diagnostic also on 
the line where you removed the dg-error.
Comment 4 Giovanni Bajo 2004-06-25 01:08:54 UTC
BTW, for readers' reference, Nathan's patch was posted here:
http://gcc.gnu.org/ml/gcc-patches/2004-06/msg01957.html
Comment 5 Nathan Sidwell 2004-06-25 08:32:31 UTC
Subject: Re:  [3.4/3.5 Regression] deducing top-level consts

giovannibajo at libero dot it wrote:
> ------- Additional Comments From giovannibajo at libero dot it  2004-06-25 01:06 -------
> Nathan,
> 
> there is something I don't understand here, and with your patch. This is what 
> should happen in my opinion:
> 
> - We try to instantiate the declaration of the templated copy ctor to add it to 
> the overload set.
> - We deduce U = K<int> (no const here!) from the first parameter
> - We substitute U into the non deduced context (second parameter). This causes 
> the instantiation of A<K<int> > (still no const).
> - A<K<int> > matches the general template (not the specialization), which is 
> incomplete. This triggers SFINAE.
> - The declaration we were trying to instantiate is discarded and not added to 
> the overload set.
aah, it appear I missed the lack of const in that final templated
copy ctor with the default arg.  Why is C++ so complicated? Make it simpler.


> I think I read a DR where John Spicer was saying that it is unfeasable to shut 
> all the possible errors that could be emitted during (possibly cascaded)
> template instantiations for the sake of SFINAE.
Yes, but I thought EDG coped with this kind of case, I don't know.

> In fact, this is what EDG does. 
> If you try it on g++.old-deja/g++.pt/crash30.C, it will emit diagnostic also on 
> the line where you removed the dg-error.
I did not know that

nathan

Comment 6 Giovanni Bajo 2004-06-25 13:14:35 UTC
It looks like GCC deduction and SFINAE works correctly for the overloaded 
template function. The problem is elsewhere.

It seems that the problem is here:

      if (convs->check_copy_constructor_p)
	/* Generate a temporary copy purely to generate the required
	   diagnostics.  */
	build_temp
	  (build_dummy_object
	   (build_qualified_type (totype, TYPE_QUAL_CONST)),
	   totype, LOOKUP_NORMAL|LOOKUP_ONLYCONVERTING, &diagnostic_fn);

This is where a temporary is generated and discarded, for the purpose of access 
checking of the copy constructor (it's the new check that GCC 3.4 does).

In the testcase of this PR, GCC correctly deduces U=K<int> and rejects the 
template overload with SFINAE. Then, it decides that K(K<T> const&) is the 
right overload, but to bind the rvalue "K<int>()" to it, it needs to do create 
a temporary and use the cctor to fill it (actually, the temporary is then 
discarded, but still the cctor access check must be done).

The point is that this temporary is created with a top-level const qualifier 
(as can be seen in the code above). Thus, when reiterating cctor overload 
resolution, this time with a CONST LVALUE (such is what build_dummy_object 
returns), the K(U& rhs, ...) declaration is instantiated, and the trick works: 
it rejects initialization of the class from a const object.

Now, the question is why the dummy object is created with that top-level const 
qualifier (through build_qualified_type). This was introduced here: 
http://gcc.gnu.org/ml/gcc-patches/2004-03/msg00534.html (testcases in 
g++.dg/overload/ref1.C). Mark suggested a different resolution which looks more 
correct at first sight, but if implemented actually make GCC enter an infinite 
loop. They discussed the patch off-list and eventually it went in as-is.

It looks like putting the const there is wrong through. There must be another 
solution.
Comment 7 Giovanni Bajo 2004-06-25 15:31:37 UTC
New patch posted here:
http://gcc.gnu.org/ml/gcc-patches/2004-06/msg02056.html

This looks like the correct solution to me, thanks Nathan!
Comment 8 GCC Commits 2004-06-28 10:44:19 UTC
Subject: Bug 16174

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_4-branch
Changes by:	nathan@gcc.gnu.org	2004-06-28 10:44:15

Modified files:
	gcc/cp         : ChangeLog call.c cp-tree.h 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/template: ctor4.C 

Log message:
	cp:
	PR C++/16174
	* call.c (build_temp): Declare.
	(check_constructor_callable): New.
	(reference_binding): Only set CHECK_COPY_CONSTRUCTOR if not for
	CONSTRUCTOR_CALLABLE.
	(convert_like_real, initialize_reference): Use
	check_constructor_callable.
	* cp-tree.h (LOOKUP_CONSTRUCTOR_CALLABLE): New.
	(LOOKUP_*): Renumber.
	testsuite:
	PR C++/16174
	* g++.dg/template/ctor4.C: New.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.3892.2.128&r2=1.3892.2.129
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/call.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.452.2.18&r2=1.452.2.19
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/cp-tree.h.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.946.4.13&r2=1.946.4.14
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.3389.2.216&r2=1.3389.2.217
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/template/ctor4.C.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=NONE&r2=1.1.2.1

Comment 9 GCC Commits 2004-06-28 11:07:40 UTC
Subject: Bug 16174

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	nathan@gcc.gnu.org	2004-06-28 11:07:25

Modified files:
	gcc/cp         : ChangeLog call.c cp-tree.h 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/template: ctor4.C 

Log message:
	cp:
	PR C++/16174
	* call.c (build_temp): Declare.
	(check_constructor_callable): New.
	(reference_binding): Only set CHECK_COPY_CONSTRUCTOR if not for
	CONSTRUCTOR_CALLABLE.
	(convert_like_real, initialize_reference): Use
	check_constructor_callable.
	* cp-tree.h (LOOKUP_CONSTRUCTOR_CALLABLE): New.
	(LOOKUP_*): Renumber.
	testsuite:
	* PR C++/16174
	* g++.dg/template/ctor4.C: New.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&r1=1.4139&r2=1.4140
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/call.c.diff?cvsroot=gcc&r1=1.485&r2=1.486
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/cp-tree.h.diff?cvsroot=gcc&r1=1.991&r2=1.992
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.3908&r2=1.3909
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/template/ctor4.C.diff?cvsroot=gcc&r1=1.1&r2=1.2

Comment 10 Andrew Pinski 2004-06-28 16:21:10 UTC
Fixed.