public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [RFC / RFH] Re-opened C++/51213 (access control under SFINAE)
@ 2012-08-01 11:09 Paolo Carlini
  2012-08-01 14:32 ` Jason Merrill
  0 siblings, 1 reply; 11+ messages in thread
From: Paolo Carlini @ 2012-08-01 11:09 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jason Merrill

Hi,

thus I started analyzing why we are handling incorrectly some variants 
posted by Daniel. A typical example (another is about using the sizeof 
in a template argument) is:

class C {
typedef int type; // Line 2
};

template<class T, class = typename T::type>
auto f(int) -> char;

template<class>
auto f(...) -> char (&)[2];

typedef int test[sizeof(f<C>(0)) == 2 ? 1 : -1]; // Error (line #11)

// static_assert(sizeof(f<C>(0)) == 2, ""); // OK (line #13)

the first interesting bit of analysis I can offer is a comparison vs 
4.7. This is the error message with 4.7:

51213_3.C:11:47: error: size of array ‘test’ is negative

as you can see, nothing about access control! Thus, it seems to me, the 
issues we are having with mainline ultimately boil down to 
non-conforming/missing access control in pre-existing code. In current 
mainline, for comparison, we have:

51213_3.C:11:47: error: size of array ‘test’ is negative
typedef int test[sizeof(f<C>(0)) == 2 ? 1 : -1]; // Error (line #11)
^
51213_3.C:2:15: error: ‘typedef int C::type’ is private
typedef int type; // Line 2
^
51213_3.C:5:19: error: within this context
template<class T, class = typename T::type>

thus, what is happening is that we actually do access control, but too 
late, with the SFINAE already mishandled. Thus, it seems that, 
irrespective of the access control under SFINAE bits, we are making 
progress here, we actually do access control now, only, too late, 
because it should happen earlier, and fail in time for SFINAE.

In little more detail, in mainline we reach 
perform_or_defer_access_check from perform_overload_resolution, but 
ptr->deferring_access_checks_kind == dk_deferred, thus the former has no 
chances to call enforce_access and return false. I'm adding below the 
stacktrace. On the other hand, in the case which already works 
(commented out above), ptr->deferring_access_checks_kind == 
dk_no_deferred and everything is fine.

So, it looks like we are deferring an access check which shouldn't be 
deferred, at least should happen in time for SFINAE. At the moment, I 
have no idea why it is happening only for line #11 and not for line #13.

Jason, are those first bits of analysis enough for you to figure out 
what could be possibly going wrong? Or you have suggestions about the 
next steps? If on the other hand, you feel like just taking over from 
here (eg, the issue seems interesting enough ;) just let me know...

Thanks!
Paolo.

/////////////////////////////////

Breakpoint 2, perform_or_defer_access_check (binfo=0x7ffff6e664e0, 
decl=0x7ffff6e59ac8, diag_decl=0x7ffff6e59ac8, complain=8) at 
/scratch/Gcc/svn-dirs/trunk/gcc/cp/semantics.c:327
(gdb) bt
#0 perform_or_defer_access_check (binfo=0x7ffff6e664e0, 
decl=0x7ffff6e59ac8, diag_decl=0x7ffff6e59ac8, complain=8) at 
/scratch/Gcc/svn-dirs/trunk/gcc/cp/semantics.c:327
#1 0x000000000051f8bb in make_typename_type (context=0x7ffff6e5e888, 
name=0x7ffff6e6c8f0, tag_type=typename_type, complain=8) at 
/scratch/Gcc/svn-dirs/trunk/gcc/cp/decl.c:3311
#2 0x00000000005e8c6f in tsubst (t=0x7ffff6e5ebd0, args=0x7ffff6d16b10, 
complain=0, in_decl=0x0) at /scratch/Gcc/svn-dirs/trunk/gcc/cp/pt.c:11405
#3 0x00000000005ce538 in tsubst_template_arg (t=0x7ffff6e5ebd0, 
args=0x7ffff6d16b10, complain=0, in_decl=0x0) at 
/scratch/Gcc/svn-dirs/trunk/gcc/cp/pt.c:8917
#4 0x000000000060d472 in type_unification_real (tparms=0x7ffff6d16900, 
targs=0x7ffff6d16b10, xparms=0x7ffff6e6d370, xargs=0x7fffffffc060, 
xnargs=1, subr=0, strict=DEDUCE_CALL, flags=1, explain_p=0 '\000') at 
/scratch/Gcc/svn-dirs/trunk/gcc/cp/pt.c:15125
#5 0x000000000060af3f in fn_type_unification (fn=0x7ffff6e59e60, 
explicit_targs=0x7ffff6e6d500, targs=0x7ffff6d16b10, 
args=0x7fffffffc060, nargs=1, return_type=0x0, strict=DEDUCE_CALL, 
flags=1, explain_p=0 '\000') at 
/scratch/Gcc/svn-dirs/trunk/gcc/cp/pt.c:14648
#6 0x00000000004de20a in add_template_candidate_real 
(candidates=0x7fffffffc590, tmpl=0x7ffff6e59e60, ctype=0x0, 
explicit_targs=0x7ffff6e6d500, first_arg=0x0, arglist=0x7ffff6e6d528, 
return_type=0x0, access_path=0x0, conversion_path=0x0, flags=1, obj=0x0, 
strict=DEDUCE_CALL, complain=3) at 
/scratch/Gcc/svn-dirs/trunk/gcc/cp/call.c:2921
#7 0x00000000004deb48 in add_template_candidate 
(candidates=0x7fffffffc590, tmpl=0x7ffff6e59e60, ctype=0x0, 
explicit_targs=0x7ffff6e6d500, first_arg=0x0, arglist=0x7ffff6e6d528, 
return_type=0x0, access_path=0x0, conversion_path=0x0, flags=1, 
strict=DEDUCE_CALL, complain=3) at 
/scratch/Gcc/svn-dirs/trunk/gcc/cp/call.c:3024
#8 0x00000000004e8375 in add_candidates (fns=0x7ffff6e6a640, 
first_arg=0x0, args=0x7ffff6e6d528, return_type=0x0, 
explicit_targs=0x7ffff6e6d500, template_only=1 '\001', 
conversion_path=0x0, access_path=0x0, flags=1, 
candidates=0x7fffffffc590, complain=3) at 
/scratch/Gcc/svn-dirs/trunk/gcc/cp/call.c:4939
#9 0x00000000004e2702 in perform_overload_resolution (fn=0x7ffff6e6a6c0, 
args=0x7ffff6e6d528, candidates=0x7fffffffc590, 
any_viable_p=0x7fffffffc58f "\001 \201\201\001", complain=3) at 
/scratch/Gcc/svn-dirs/trunk/gcc/cp/call.c:3823

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

* Re: [RFC / RFH] Re-opened C++/51213 (access control under SFINAE)
  2012-08-01 11:09 [RFC / RFH] Re-opened C++/51213 (access control under SFINAE) Paolo Carlini
@ 2012-08-01 14:32 ` Jason Merrill
  2012-08-01 15:49   ` Paolo Carlini
  0 siblings, 1 reply; 11+ messages in thread
From: Jason Merrill @ 2012-08-01 14:32 UTC (permalink / raw)
  To: Paolo Carlini; +Cc: gcc-patches

I think the problem is that we're deferring access control due to 
tentative parsing on line 11, and not on line 13.  I guess we need a

push_deferring_access_checks (dk_no_deferred);
pop_deferring_access_checks ();

around the substitution of default template args in type_unification_real.

Jason

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

* Re: [RFC / RFH] Re-opened C++/51213 (access control under SFINAE)
  2012-08-01 14:32 ` Jason Merrill
@ 2012-08-01 15:49   ` Paolo Carlini
  2012-08-01 18:11     ` Jason Merrill
  0 siblings, 1 reply; 11+ messages in thread
From: Paolo Carlini @ 2012-08-01 15:49 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1576 bytes --]

Hi,

On 08/01/2012 04:32 PM, Jason Merrill wrote:
> I think the problem is that we're deferring access control due to
> tentative parsing on line 11, and not on line 13.  I guess we need a
>
> push_deferring_access_checks (dk_no_deferred);
> pop_deferring_access_checks ();
>
> around the substitution of default template args in
> type_unification_real.
Great, thanks. Thus, I have been testing the attached and it definitely 
works for the testcases I discussed so far. Testsuite seems also Ok 
(lightly tested so far).

However, something weird is going on for this variant, using decltype 
(wanted to consistently extend sfinae37.C):

class C {
    typedef int type;
};

template<class T>
auto g(int) -> decltype(typename T::type(), char());

template<class>
auto g(...) -> char (&)[2];

static_assert(sizeof(g<C>(0)) == 2, "Ouch");          // line 11

typedef int testg[sizeof(g<C>(0)) == 2 ? 1 : -1];      // line 13

what happens is that line 13 is mishandled:

sfinae37_red.C:13:48: error: size of array ‘testg’ is negative

However, *if I comment out line 11*, things work for line 13! If I swap 
line 11 and line 13 then the declaration of testg is accepted and the 
static_assert triggers. In any case, only the first evaluation of the 
sizeof is correct, the next are incorrect. The issue seems so weird that 
it should be easy to fix... ;)

Final important observation: in fact, this variant with decltype is 
handled in the same way with or without the push_deferring_access_checks 
/ pop_deferring_access_checks calls.

Paolo.

////////////////////////


[-- Attachment #2: p --]
[-- Type: text/plain, Size: 660 bytes --]

Index: pt.c
===================================================================
--- pt.c	(revision 190031)
+++ pt.c	(working copy)
@@ -15122,9 +15122,11 @@ type_unification_real (tree tparms,
 	      location_t save_loc = input_location;
 	      if (DECL_P (parm))
 		input_location = DECL_SOURCE_LOCATION (parm);
+	      push_deferring_access_checks (dk_no_deferred);
 	      arg = tsubst_template_arg (arg, targs, complain, NULL_TREE);
 	      arg = convert_template_argument (parm, arg, targs, complain,
 					       i, NULL_TREE);
+	      pop_deferring_access_checks ();
 	      input_location = save_loc;
 	      if (arg == error_mark_node)
 		return 1;


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

* Re: [RFC / RFH] Re-opened C++/51213 (access control under SFINAE)
  2012-08-01 15:49   ` Paolo Carlini
@ 2012-08-01 18:11     ` Jason Merrill
  2012-08-01 18:41       ` Paolo Carlini
  0 siblings, 1 reply; 11+ messages in thread
From: Jason Merrill @ 2012-08-01 18:11 UTC (permalink / raw)
  To: Paolo Carlini; +Cc: gcc-patches

On 08/01/2012 11:49 AM, Paolo Carlini wrote:
> static_assert(sizeof(g<C>(0)) == 2, "Ouch");          // line 11
>
> typedef int testg[sizeof(g<C>(0)) == 2 ? 1 : -1];      // line 13
>
> what happens is that line 13 is mishandled:
>
> sfinae37_red.C:13:48: error: size of array ‘testg’ is negative
>
> However, *if I comment out line 11*, things work for line 13! If I swap
> line 11 and line 13 then the declaration of testg is accepted and the
> static_assert triggers.

Curious.  I guess that the second time we see the call the compiler 
thinks it already has the candidate it needs, but I don't know why that 
would be.  Are we not getting to type_unification_real from 
add_template_candidate the second time?

Jason

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

* Re: [RFC / RFH] Re-opened C++/51213 (access control under SFINAE)
  2012-08-01 18:11     ` Jason Merrill
@ 2012-08-01 18:41       ` Paolo Carlini
  2012-08-01 18:57         ` Paolo Carlini
  0 siblings, 1 reply; 11+ messages in thread
From: Paolo Carlini @ 2012-08-01 18:41 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

On 08/01/2012 07:35 PM, Jason Merrill wrote:
> On 08/01/2012 11:49 AM, Paolo Carlini wrote:
>> static_assert(sizeof(g<C>(0)) == 2, "Ouch");          // line 11
>>
>> typedef int testg[sizeof(g<C>(0)) == 2 ? 1 : -1];      // line 13
>>
>> what happens is that line 13 is mishandled:
>>
>> sfinae37_red.C:13:48: error: size of array ‘testg’ is negative
>>
>> However, *if I comment out line 11*, things work for line 13! If I swap
>> line 11 and line 13 then the declaration of testg is accepted and the
>> static_assert triggers.
>
> Curious.  I guess that the second time we see the call the compiler 
> thinks it already has the candidate it needs, but I don't know why 
> that would be.  Are we not getting to type_unification_real from 
> add_template_candidate the second time?
So, I'm in the middle of this (got distracted earlier today). I can tell 
you what I have.

For the second evaluation, the second time we call 
instantiate_template_1, thus for the interesting g(int) overload, here:

   spec = retrieve_specialization (gen_tmpl, targ_ptr, 0);

   gcc_assert (tmpl == gen_tmpl
           || ((fndecl = retrieve_specialization (tmpl, orig_args, 0))
           == spec)
           || fndecl == NULL_TREE);

   if (spec != NULL_TREE)
     {
       if (FNDECL_RECHECK_ACCESS_P (spec) && (complain & tf_error))
     recheck_decl_substitution (spec, gen_tmpl, targ_ptr);
       return spec;
     }

things are completely different, because spec != NULL_TREE and, more 
importantly, complain is tf_none, thus recheck_decl_substitution is not 
called, we just return immediately.

Compare to the first evaluation: in that case we call enforce_access 
*way* below, with the perform_deferred_access_checks call near the end 
of instantiate_template_1.

Thus, looks like the recheck_decl_substitution mechanism is not working 
by design because of complain == tf_none?!?

Note that while I'm debugging this, I see instantiate_template_1 always 
getting complain == tf_none, something seems weird about the && 
(complain & tf_error) above...

Paolo.

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

* Re: [RFC / RFH] Re-opened C++/51213 (access control under SFINAE)
  2012-08-01 18:41       ` Paolo Carlini
@ 2012-08-01 18:57         ` Paolo Carlini
  2012-08-02 15:33           ` Jason Merrill
  0 siblings, 1 reply; 11+ messages in thread
From: Paolo Carlini @ 2012-08-01 18:57 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

Hi again,

On 08/01/2012 08:40 PM, Paolo Carlini wrote:
> For the second evaluation, the second time we call 
> instantiate_template_1, thus for the interesting g(int) overload, here:
>
>   spec = retrieve_specialization (gen_tmpl, targ_ptr, 0);
>
>   gcc_assert (tmpl == gen_tmpl
>           || ((fndecl = retrieve_specialization (tmpl, orig_args, 0))
>           == spec)
>           || fndecl == NULL_TREE);
>
>   if (spec != NULL_TREE)
>     {
>       if (FNDECL_RECHECK_ACCESS_P (spec) && (complain & tf_error))
>     recheck_decl_substitution (spec, gen_tmpl, targ_ptr);
>       return spec;
>     }
>
> things are completely different, because spec != NULL_TREE and, more 
> importantly, complain is tf_none, thus recheck_decl_substitution is 
> not called, we just return immediately.
So, it is possible that when spec != NULL_TREE and we are once more in a 
SFINAE context, we have to actually call perform_deferred_access_checks 
(complain) and either return error_mark_node or the spec depending on 
the return value?

Paolo.

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

* Re: [RFC / RFH] Re-opened C++/51213 (access control under SFINAE)
  2012-08-01 18:57         ` Paolo Carlini
@ 2012-08-02 15:33           ` Jason Merrill
  2012-08-02 16:22             ` Paolo Carlini
  0 siblings, 1 reply; 11+ messages in thread
From: Jason Merrill @ 2012-08-02 15:33 UTC (permalink / raw)
  To: Paolo Carlini; +Cc: gcc-patches

On 08/01/2012 02:57 PM, Paolo Carlini wrote:
> So, it is possible that when spec != NULL_TREE and we are once more in a
> SFINAE context, we have to actually call perform_deferred_access_checks
> (complain) and either return error_mark_node or the spec depending on
> the return value?

I don't think we need to redo the access check in SFINAE context; we 
know that there's an error, so we can just return error_mark_node in 
that case.  I guess we should change the name of FNDECL_RECHECK_ACCESS_P 
to something like FNDECL_HAS_ACCESS_ERRORS to make it clearer.

Jason

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

* Re: [RFC / RFH] Re-opened C++/51213 (access control under SFINAE)
  2012-08-02 15:33           ` Jason Merrill
@ 2012-08-02 16:22             ` Paolo Carlini
  2012-08-02 16:53               ` Jason Merrill
  0 siblings, 1 reply; 11+ messages in thread
From: Paolo Carlini @ 2012-08-02 16:22 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 805 bytes --]

Hi,

On 08/02/2012 05:33 PM, Jason Merrill wrote:
> On 08/01/2012 02:57 PM, Paolo Carlini wrote:
>> So, it is possible that when spec != NULL_TREE and we are once more in a
>> SFINAE context, we have to actually call perform_deferred_access_checks
>> (complain) and either return error_mark_node or the spec depending on
>> the return value?
>
> I don't think we need to redo the access check in SFINAE context; we 
> know that there's an error, so we can just return error_mark_node in 
> that case.  I guess we should change the name of 
> FNDECL_RECHECK_ACCESS_P to something like FNDECL_HAS_ACCESS_ERRORS to 
> make it clearer.
Ah, very well, everything makes sense now. Thus I'm finishing testing 
the below (already past the C++ testsuite), Ok if it passes?

Thanks,
Paolo.

///////////////////////

[-- Attachment #2: CL_51213_again --]
[-- Type: text/plain, Size: 691 bytes --]

/cp
2012-08-02  Jason Merrill  <jason@redhat.com>
	    Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/51213 (again)
	* pt.c (type_unification_real): Call push_deferring_access_checks /
	pop_deferring_access_checks around the substitution of default
	template args.
	(instantiate_template_1): When the specialization returned by
	retrieve_specialization has FNDECL_HAS_ACCESS_ERRORS set and we
	are in a SFINAE context, simply return error_mark_node.
	* cp-tree.h (FNDECL_RECHECK_ACCESS_P): Rename FNDECL_HAS_ACCESS_ERRORS.

/testsuite
2012-08-02  Jason Merrill  <jason@redhat.com>
	    Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/51213 (again)
	* g++.dg/cpp0x/sfinae37.C: Extend.

[-- Attachment #3: patch_51213_again --]
[-- Type: text/plain, Size: 3055 bytes --]

Index: testsuite/g++.dg/cpp0x/sfinae37.C
===================================================================
--- testsuite/g++.dg/cpp0x/sfinae37.C	(revision 190071)
+++ testsuite/g++.dg/cpp0x/sfinae37.C	(working copy)
@@ -5,6 +5,12 @@ class C {
   typedef int type;
 };
 
+template<int>
+struct I;
+
+template<>
+struct I<2> { };
+
 template<class T, class = typename T::type>
 auto f(int) -> char;
 
@@ -13,6 +19,10 @@ auto f(...) -> char (&)[2];
 
 static_assert(sizeof(f<C>(0)) == 2, "Ouch");
 
+typedef int testf[sizeof(f<C>(0)) == 2 ? 1 : -1];
+
+I<sizeof(f<C>(0))> vf;
+
 template<class T>
 auto g(int) -> decltype(typename T::type(), char());
 
@@ -20,3 +30,7 @@ template<class>
 auto g(...) -> char (&)[2];
 
 static_assert(sizeof(g<C>(0)) == 2, "Ouch");
+
+typedef int testg[sizeof(g<C>(0)) == 2 ? 1 : -1];
+
+I<sizeof(g<C>(0))> vg;
Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 190071)
+++ cp/pt.c	(working copy)
@@ -14363,8 +14363,13 @@ instantiate_template_1 (tree tmpl, tree orig_args,
 
   if (spec != NULL_TREE)
     {
-      if (FNDECL_RECHECK_ACCESS_P (spec) && (complain & tf_error))
-	recheck_decl_substitution (spec, gen_tmpl, targ_ptr);
+      if (FNDECL_HAS_ACCESS_ERRORS (spec))
+	{
+	  if (complain & tf_error)
+	    recheck_decl_substitution (spec, gen_tmpl, targ_ptr);
+	  else
+	    return error_mark_node;
+	}
       return spec;
     }
 
@@ -14426,7 +14431,7 @@ instantiate_template_1 (tree tmpl, tree orig_args,
 	{
 	  /* Remember to reinstantiate when we're out of SFINAE so the user
 	     can see the errors.  */
-	  FNDECL_RECHECK_ACCESS_P (fndecl) = true;
+	  FNDECL_HAS_ACCESS_ERRORS (fndecl) = true;
 	}
       return error_mark_node;
     }
@@ -15122,9 +15127,11 @@ type_unification_real (tree tparms,
 	      location_t save_loc = input_location;
 	      if (DECL_P (parm))
 		input_location = DECL_SOURCE_LOCATION (parm);
+	      push_deferring_access_checks (dk_no_deferred);
 	      arg = tsubst_template_arg (arg, targs, complain, NULL_TREE);
 	      arg = convert_template_argument (parm, arg, targs, complain,
 					       i, NULL_TREE);
+	      pop_deferring_access_checks ();
 	      input_location = save_loc;
 	      if (arg == error_mark_node)
 		return 1;
Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h	(revision 190071)
+++ cp/cp-tree.h	(working copy)
@@ -729,10 +729,10 @@ DEF_VEC_ALLOC_O (qualified_typedef_usage_t,gc);
 /* Non-zero if this template specialization has access violations that
    should be rechecked when the function is instantiated outside argument
    deduction.  */
-#define TINFO_RECHECK_ACCESS_P(NODE) \
+#define TINFO_HAS_ACCESS_ERRORS(NODE) \
   (TREE_LANG_FLAG_0 (TEMPLATE_INFO_CHECK (NODE)))
-#define FNDECL_RECHECK_ACCESS_P(NODE) \
-  (TINFO_RECHECK_ACCESS_P (DECL_TEMPLATE_INFO (NODE)))
+#define FNDECL_HAS_ACCESS_ERRORS(NODE) \
+  (TINFO_HAS_ACCESS_ERRORS (DECL_TEMPLATE_INFO (NODE)))
 
 struct GTY(()) tree_template_info {
   struct tree_common common;

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

* Re: [RFC / RFH] Re-opened C++/51213 (access control under SFINAE)
  2012-08-02 16:22             ` Paolo Carlini
@ 2012-08-02 16:53               ` Jason Merrill
  2012-08-02 17:09                 ` Paolo Carlini
  0 siblings, 1 reply; 11+ messages in thread
From: Jason Merrill @ 2012-08-02 16:53 UTC (permalink / raw)
  To: Paolo Carlini; +Cc: gcc-patches

On 08/02/2012 12:21 PM, Paolo Carlini wrote:
> +	  if (complain & tf_error)
> +	    recheck_decl_substitution (spec, gen_tmpl, targ_ptr);
> +	  else
> +	    return error_mark_node;

I think we want to be consistent with the end of the function about 
whether we return error_mark_node or the decl after giving access errors 
in non-SFINAE context; I don't have a strong opinion either way, but 
this returns the decl, and the end of the function returns error_mark_node.

Jason

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

* Re: [RFC / RFH] Re-opened C++/51213 (access control under SFINAE)
  2012-08-02 16:53               ` Jason Merrill
@ 2012-08-02 17:09                 ` Paolo Carlini
  0 siblings, 0 replies; 11+ messages in thread
From: Paolo Carlini @ 2012-08-02 17:09 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 629 bytes --]

On 08/02/2012 06:53 PM, Jason Merrill wrote:
> On 08/02/2012 12:21 PM, Paolo Carlini wrote:
>> +      if (complain & tf_error)
>> +        recheck_decl_substitution (spec, gen_tmpl, targ_ptr);
>> +      else
>> +        return error_mark_node;
>
> I think we want to be consistent with the end of the function about 
> whether we return error_mark_node or the decl after giving access 
> errors in non-SFINAE context; I don't have a strong opinion either 
> way, but this returns the decl, and the end of the function returns 
> error_mark_node.
I see. Then, I restarted testing with this variant.

Paolo.

/////////////////////

[-- Attachment #2: patch_51213_again_2 --]
[-- Type: text/plain, Size: 3044 bytes --]

Index: testsuite/g++.dg/cpp0x/sfinae37.C
===================================================================
--- testsuite/g++.dg/cpp0x/sfinae37.C	(revision 190071)
+++ testsuite/g++.dg/cpp0x/sfinae37.C	(working copy)
@@ -5,6 +5,12 @@ class C {
   typedef int type;
 };
 
+template<int>
+struct I;
+
+template<>
+struct I<2> { };
+
 template<class T, class = typename T::type>
 auto f(int) -> char;
 
@@ -13,6 +19,10 @@ auto f(...) -> char (&)[2];
 
 static_assert(sizeof(f<C>(0)) == 2, "Ouch");
 
+typedef int testf[sizeof(f<C>(0)) == 2 ? 1 : -1];
+
+I<sizeof(f<C>(0))> vf;
+
 template<class T>
 auto g(int) -> decltype(typename T::type(), char());
 
@@ -20,3 +30,7 @@ template<class>
 auto g(...) -> char (&)[2];
 
 static_assert(sizeof(g<C>(0)) == 2, "Ouch");
+
+typedef int testg[sizeof(g<C>(0)) == 2 ? 1 : -1];
+
+I<sizeof(g<C>(0))> vg;
Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 190071)
+++ cp/pt.c	(working copy)
@@ -14363,8 +14363,12 @@ instantiate_template_1 (tree tmpl, tree orig_args,
 
   if (spec != NULL_TREE)
     {
-      if (FNDECL_RECHECK_ACCESS_P (spec) && (complain & tf_error))
-	recheck_decl_substitution (spec, gen_tmpl, targ_ptr);
+      if (FNDECL_HAS_ACCESS_ERRORS (spec))
+	{
+	  if (complain & tf_error)
+	    recheck_decl_substitution (spec, gen_tmpl, targ_ptr);
+	  return error_mark_node;
+	}
       return spec;
     }
 
@@ -14426,7 +14430,7 @@ instantiate_template_1 (tree tmpl, tree orig_args,
 	{
 	  /* Remember to reinstantiate when we're out of SFINAE so the user
 	     can see the errors.  */
-	  FNDECL_RECHECK_ACCESS_P (fndecl) = true;
+	  FNDECL_HAS_ACCESS_ERRORS (fndecl) = true;
 	}
       return error_mark_node;
     }
@@ -15122,9 +15126,11 @@ type_unification_real (tree tparms,
 	      location_t save_loc = input_location;
 	      if (DECL_P (parm))
 		input_location = DECL_SOURCE_LOCATION (parm);
+	      push_deferring_access_checks (dk_no_deferred);
 	      arg = tsubst_template_arg (arg, targs, complain, NULL_TREE);
 	      arg = convert_template_argument (parm, arg, targs, complain,
 					       i, NULL_TREE);
+	      pop_deferring_access_checks ();
 	      input_location = save_loc;
 	      if (arg == error_mark_node)
 		return 1;
Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h	(revision 190071)
+++ cp/cp-tree.h	(working copy)
@@ -729,10 +729,10 @@ DEF_VEC_ALLOC_O (qualified_typedef_usage_t,gc);
 /* Non-zero if this template specialization has access violations that
    should be rechecked when the function is instantiated outside argument
    deduction.  */
-#define TINFO_RECHECK_ACCESS_P(NODE) \
+#define TINFO_HAS_ACCESS_ERRORS(NODE) \
   (TREE_LANG_FLAG_0 (TEMPLATE_INFO_CHECK (NODE)))
-#define FNDECL_RECHECK_ACCESS_P(NODE) \
-  (TINFO_RECHECK_ACCESS_P (DECL_TEMPLATE_INFO (NODE)))
+#define FNDECL_HAS_ACCESS_ERRORS(NODE) \
+  (TINFO_HAS_ACCESS_ERRORS (DECL_TEMPLATE_INFO (NODE)))
 
 struct GTY(()) tree_template_info {
   struct tree_common common;

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

* Re: [RFC / RFH] Re-opened C++/51213 (access control under SFINAE)
@ 2012-08-02 17:57 Jason Merrill
  0 siblings, 0 replies; 11+ messages in thread
From: Jason Merrill @ 2012-08-02 17:57 UTC (permalink / raw)
  To: Paolo Carlini; +Cc: gcc-patches

Looks good.

-------- Original Message --------
 From: Paolo Carlini <paolo.carlini@oracle.com>
 Sent: Thu, Aug 2, 2012 01:09 PM
 To: Jason Merrill <jason@redhat.com>
 CC: gcc-patches@gcc.gnu.org
 Subject: Re: [RFC / RFH] Re-opened C++/51213 (access control under SFINAE)

On 08/02/2012 06:53 PM, Jason Merrill wrote:
> On 08/02/2012 12:21 PM, Paolo Carlini wrote:
>> +      if (complain & tf_error)
>> +        recheck_decl_substitution (spec, gen_tmpl, targ_ptr);
>> +      else
>> +        return error_mark_node;
>
> I think we want to be consistent with the end of the function about 
> whether we return error_mark_node or the decl after giving access 
> errors in non-SFINAE context; I don't have a strong opinion either 
> way, but this returns the decl, and the end of the function returns 
> error_mark_node.
I see. Then, I restarted testing with this variant.

Paolo.

/////////////////////

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

end of thread, other threads:[~2012-08-02 17:57 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-01 11:09 [RFC / RFH] Re-opened C++/51213 (access control under SFINAE) Paolo Carlini
2012-08-01 14:32 ` Jason Merrill
2012-08-01 15:49   ` Paolo Carlini
2012-08-01 18:11     ` Jason Merrill
2012-08-01 18:41       ` Paolo Carlini
2012-08-01 18:57         ` Paolo Carlini
2012-08-02 15:33           ` Jason Merrill
2012-08-02 16:22             ` Paolo Carlini
2012-08-02 16:53               ` Jason Merrill
2012-08-02 17:09                 ` Paolo Carlini
2012-08-02 17:57 Jason Merrill

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