[C++, concepts] Two slightly obscure pt.c functions?

[C++, concepts] Two slightly obscure pt.c functions?

[Paolo Carlini]

Hi,

in 2013 (2013-09-16) Adam added two slightly obscure functions and I 
can't find much around in terms of rationale, etc:

/* Returns true iff TYPE is a TEMPLATE_TYPE_PARM representing 'auto',
    'decltype(auto)' or a concept.  */

bool
is_auto_or_concept (const_tree type)
{
   return is_auto (type); // or concept
}

/* Returns the TEMPLATE_TYPE_PARM in TYPE representing a generic type 
(`auto' or
    a concept identifier) iff TYPE contains a use of a generic type.  
Returns
    NULL_TREE otherwise.  */

tree
type_uses_auto_or_concept (tree type)
{
   return find_type_usage (type, is_auto_or_concept);
}

The latter seems completely unused (it's meant for debugging purposes?); 
the former evidently simply forwards to is_auto, and we end up in the 
front-end with uses of both, which in fact are equivalent, which seems 
weird: IMHO, if they are actually equivalent in our implementation we 
should clearly explain that in the comment and have only one. Or what?

Thanks for any feedback,

Paolo.

[C++ Patch] Remove is_auto_or_concept, etc (Was: Re: [C++, concepts] Two slightly obscure pt.c functions?)

[Paolo Carlini]

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

Hi again,

On 26/04/2017 12:32, Paolo Carlini wrote:
> Hi,
>
> in 2013 (2013-09-16) Adam added two slightly obscure functions and I 
> can't find much around in terms of rationale, etc:
>
> /* Returns true iff TYPE is a TEMPLATE_TYPE_PARM representing 'auto',
>    'decltype(auto)' or a concept.  */
>
> bool
> is_auto_or_concept (const_tree type)
> {
>   return is_auto (type); // or concept
> }
>
> /* Returns the TEMPLATE_TYPE_PARM in TYPE representing a generic type 
> (`auto' or
>    a concept identifier) iff TYPE contains a use of a generic type.  
> Returns
>    NULL_TREE otherwise.  */
>
> tree
> type_uses_auto_or_concept (tree type)
> {
>   return find_type_usage (type, is_auto_or_concept);
> }
>
> The latter seems completely unused (it's meant for debugging 
> purposes?); the former evidently simply forwards to is_auto, and we 
> end up in the front-end with uses of both, which in fact are 
> equivalent, which seems weird: IMHO, if they are actually equivalent 
> in our implementation we should clearly explain that in the comment 
> and have only one. Or what?

... replying to myself, in practice we could do the below, which 
certainly passes testing, and in fact now seems to me even more obvious 
than I thought a couple of days ago...

Thanks,
Paolo.

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

[-- Attachment #2: CL_CU2 --]
[-- Type: text/plain, Size: 305 bytes --]

2017-04-27  Paolo Carlini  <paolo.carlini@oracle.com>

	* pt.c (is_auto_or_concept): Remove.
	(type_uses_auto_or_concept): Remove, unused.
	(find_parameter_packs_r, extract_autos_r, is_auto_r): Adjust.
	* parser.c (tree_type_is_auto_or_concept): Remove, unused.
	* cp-tree.h (is_auto_or_concept): Remove.

[-- Attachment #3: patch_CU2 --]
[-- Type: text/plain, Size: 3275 bytes --]

Index: cp-tree.h
===================================================================
--- cp-tree.h	(revision 247347)
+++ cp-tree.h	(working copy)
@@ -6161,7 +6161,6 @@ extern void append_type_to_template_for_access_che
 extern tree convert_generic_types_to_packs	(tree, int, int);
 extern tree splice_late_return_type		(tree, tree);
 extern bool is_auto				(const_tree);
-extern bool is_auto_or_concept			(const_tree);
 extern tree process_template_parm		(tree, location_t, tree, 
 						 bool, bool);
 extern tree end_template_parm_list		(tree);
Index: parser.c
===================================================================
--- parser.c	(revision 247347)
+++ parser.c	(working copy)
@@ -38791,16 +38791,6 @@ make_generic_type_name ()
   return get_identifier (buf);
 }
 
-/* Predicate that behaves as is_auto_or_concept but matches the parent
-   node of the generic type rather than the generic type itself.  This
-   allows for type transformation in add_implicit_template_parms.  */
-
-static inline bool
-tree_type_is_auto_or_concept (const_tree t)
-{
-  return TREE_TYPE (t) && is_auto_or_concept (TREE_TYPE (t));
-}
-
 /* Add an implicit template type parameter to the CURRENT_TEMPLATE_PARMS
    (creating a new template parameter list if necessary).  Returns the newly
    created template type parm.  */
Index: pt.c
===================================================================
--- pt.c	(revision 247347)
+++ pt.c	(working copy)
@@ -3489,7 +3489,7 @@ find_parameter_packs_r (tree *tp, int *walk_subtre
 	 parameter pack (14.6.3), or the type-specifier-seq of a type-id that
 	 is a pack expansion, the invented template parameter is a template
 	 parameter pack.  */
-      if (ppd->type_pack_expansion_p && is_auto_or_concept (t))
+      if (ppd->type_pack_expansion_p && is_auto (t))
 	TEMPLATE_TYPE_PARAMETER_PACK (t) = true;
       if (TEMPLATE_TYPE_PARAMETER_PACK (t))
         parameter_pack_p = true;
@@ -24806,7 +24806,7 @@ static int
 extract_autos_r (tree t, void *data)
 {
   hash_table<auto_hash> &hash = *(hash_table<auto_hash>*)data;
-  if (is_auto_or_concept (t))
+  if (is_auto (t))
     {
       /* All the autos were built with index 0; fix that up now.  */
       tree *p = hash.find_slot (t, INSERT);
@@ -25530,7 +25530,7 @@ is_auto (const_tree type)
 int
 is_auto_r (tree tp, void */*data*/)
 {
-  return is_auto_or_concept (tp);
+  return is_auto (tp);
 }
 
 /* Returns the TEMPLATE_TYPE_PARM in TYPE representing `auto' iff TYPE contains
@@ -25556,26 +25556,6 @@ type_uses_auto (tree type)
     return find_type_usage (type, is_auto);
 }
 
-/* Returns true iff TYPE is a TEMPLATE_TYPE_PARM representing 'auto',
-   'decltype(auto)' or a concept.  */
-
-bool
-is_auto_or_concept (const_tree type)
-{
-  return is_auto (type); // or concept
-}
-
-/* Returns the TEMPLATE_TYPE_PARM in TYPE representing a generic type (`auto' or
-   a concept identifier) iff TYPE contains a use of a generic type.  Returns
-   NULL_TREE otherwise.  */
-
-tree
-type_uses_auto_or_concept (tree type)
-{
-  return find_type_usage (type, is_auto_or_concept);
-}
-
-
 /* For a given template T, return the vector of typedefs referenced
    in T for which access check is needed at T instantiation time.
    T is either  a FUNCTION_DECL or a RECORD_TYPE.

Re: [C++ Patch] Remove is_auto_or_concept, etc (Was: Re: [C++, concepts] Two slightly obscure pt.c functions?)

[Nathan Sidwell]

On 04/27/2017 02:02 PM, Paolo Carlini wrote:

> ... replying to myself, in practice we could do the below, which 
> certainly passes testing, and in fact now seems to me even more obvious 
> than I thought a couple of days ago...


I'm insufficiently conceptified to know whether this is safe (but lack 
of test failure is promising).

nathan

-- 
Nathan Sidwell

Re: [C++ Patch] Remove is_auto_or_concept, etc (Was: Re: [C++, concepts] Two slightly obscure pt.c functions?)

[Jason Merrill]

OK.

On Thu, Apr 27, 2017 at 2:02 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi again,
>
> On 26/04/2017 12:32, Paolo Carlini wrote:
>>
>> Hi,
>>
>> in 2013 (2013-09-16) Adam added two slightly obscure functions and I can't
>> find much around in terms of rationale, etc:
>>
>> /* Returns true iff TYPE is a TEMPLATE_TYPE_PARM representing 'auto',
>>    'decltype(auto)' or a concept.  */
>>
>> bool
>> is_auto_or_concept (const_tree type)
>> {
>>   return is_auto (type); // or concept
>> }
>>
>> /* Returns the TEMPLATE_TYPE_PARM in TYPE representing a generic type
>> (`auto' or
>>    a concept identifier) iff TYPE contains a use of a generic type.
>> Returns
>>    NULL_TREE otherwise.  */
>>
>> tree
>> type_uses_auto_or_concept (tree type)
>> {
>>   return find_type_usage (type, is_auto_or_concept);
>> }
>>
>> The latter seems completely unused (it's meant for debugging purposes?);
>> the former evidently simply forwards to is_auto, and we end up in the
>> front-end with uses of both, which in fact are equivalent, which seems
>> weird: IMHO, if they are actually equivalent in our implementation we should
>> clearly explain that in the comment and have only one. Or what?
>
>
> ... replying to myself, in practice we could do the below, which certainly
> passes testing, and in fact now seems to me even more obvious than I thought
> a couple of days ago...
>
> Thanks,
> Paolo.
>
> /////////////////////

Re: [C++ Patch] Remove is_auto_or_concept, etc

[Adam Butcher]

Sorry for not getting back to your original post Paolo.  I haven't been 
picking up mails for a while.

On 2017-05-01 16:56, Jason Merrill wrote:
> On Thu, Apr 27, 2017 at 2:02 PM, Paolo Carlini 
> <paolo.carlini@oracle.com> wrote:
>> On 26/04/2017 12:32, Paolo Carlini wrote:
>>> 
>>> in 2013 (2013-09-16) Adam added two slightly obscure functions and I 
>>> can't
>>> find much around in terms of rationale, etc:
>>> 
>>> is_auto_or_concept (const_tree type)
>>> 
>>> type_uses_auto_or_concept (tree type)
>>> 

I don't think they were there in the original 2009 version on the old 
lambdas branch -- but it stagnated somewhat and when I remade it against 
mainline (4.something) it was around the time that concepts were showing 
similar semantics (in terms of implicit/abbrievated templates).  I made 
a cardinal sin and introduced an overly-generic function name expecting 
that a future concepts implementation would need to trigger at the same 
point of the parse too.  I.e. if a concept name or 'auto' is seen we 
inject an implicit template parameter (or make the "plain" function into 
a template at that point).

That intent was not well documented or published (other than in the API 
name) and, since -fconcepts is now working without any calls to this 
function, it's clearly not been necessary or has been more naturally 
done in a different way.


>>> The latter seems completely unused (it's meant for debugging 
>>> purposes?);
>>> 

Quite possibly for debugging though maybe some refactoring got rid of 
the need for it and neglected to bin it.


>>> the former evidently simply forwards to is_auto, and we end up in the
>>> front-end with uses of both, which in fact are equivalent, which 
>>> seems
>>> weird: IMHO, if they are actually equivalent in our implementation we 
>>> should
>>> clearly explain that in the comment and have only one. Or what?
>> 
>> 
>> ... replying to myself, in practice we could do the below, which 
>> certainly
>> passes testing, and in fact now seems to me even more obvious than I 
>> thought
>> a couple of days ago...
>> 

Definitely OK to bin.  No point in having dead or confusing code; it's 
complicated enough as it is.  :)