Re: [RFC PATCH] c++: Minimal handling of carries_dependency attribute

[Jason Merrill <jason@redhat.com>]

On 11/8/22 04:42, Jakub Jelinek wrote:
> Hi!
> 
> A comment in D2552R1:
> "The only questionable (but still conforming) case we found was
> [[carries_dependency(some_argument)]] on GCC, where the emitted diagnostic said that the
> carries_dependency attribute is not supported, but did not specifically call out the syntax error
> in the argument clause."
> made me try the following patch, where we'll error at least
> for arguments to the attribute and for some uses of the attribute
> appertaining to something not mentioned in the standard warn
> with different diagnostics (or should that be an error?; clang++
> does that, but I think we never do for any attribute, standard or not).
> The diagnostics on toplevel attribute declaration is still an
> attribute ignored warning and on empty statement different wording.
> 
> The paper additionally mentions
> struct X { [[nodiscard]]; }; // no diagnostic on GCC
> and 2 cases of missing diagnostics on [[fallthrough]] (guess I should
> file a PR about those; one problem is that do { ... } while (0); there
> is replaced during genericization just by ... and another that
> [[fallthrough]] there is followed by a label, but not user/case/default
> label, but an artificial one created from while loop genericization.
> 
> Thoughts on this?

LGTM.

> 2022-11-08  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* tree.cc (handle_carries_dependency_attribute): New function.
> 	(std_attribute_table): Add carries_dependency attribute.
> 	* parser.cc (cp_parser_check_std_attribute): Add carries_dependency
> 	attribute.
> 
> 	* g++.dg/cpp0x/attr-carries_dependency1.C: New test.
> 
> --- gcc/cp/tree.cc.jj	2022-11-07 10:30:42.758629740 +0100
> +++ gcc/cp/tree.cc	2022-11-08 14:45:08.853864684 +0100
> @@ -4923,6 +4923,32 @@ structural_type_p (tree t, bool explain)
>     return true;
>   }
>   
> +/* Partially handle the C++11 [[carries_dependency]] attribute.
> +   Just emit a different diagnostics when it is used on something the
> +   spec doesn't allow vs. where it allows and we just choose to ignore
> +   it.  */
> +
> +static tree
> +handle_carries_dependency_attribute (tree *node, tree name,
> +				     tree ARG_UNUSED (args),
> +				     int ARG_UNUSED (flags),
> +				     bool *no_add_attrs)
> +{
> +  if (TREE_CODE (*node) != FUNCTION_DECL
> +      && TREE_CODE (*node) != PARM_DECL)
> +    {
> +      warning (OPT_Wattributes, "%qE attribute can only be applied to "
> +	       "functions or parameters", name);
> +      *no_add_attrs = true;
> +    }
> +  else
> +    {
> +      warning (OPT_Wattributes, "%qE attribute ignored", name);
> +      *no_add_attrs = true;
> +    }
> +  return NULL_TREE;
> +}
> +
>   /* Handle the C++17 [[nodiscard]] attribute, which is similar to the GNU
>      warn_unused_result attribute.  */
>   
> @@ -5036,6 +5062,8 @@ const struct attribute_spec std_attribut
>       handle_likeliness_attribute, attr_cold_hot_exclusions },
>     { "noreturn", 0, 0, true, false, false, false,
>       handle_noreturn_attribute, attr_noreturn_exclusions },
> +  { "carries_dependency", 0, 0, true, false, false, false,
> +    handle_carries_dependency_attribute, NULL },
>     { NULL, 0, 0, false, false, false, false, NULL, NULL }
>   };
>   
> --- gcc/cp/parser.cc.jj	2022-11-04 18:11:41.523945997 +0100
> +++ gcc/cp/parser.cc	2022-11-08 13:41:35.075135139 +0100
> @@ -29239,8 +29239,7 @@ cp_parser_std_attribute (cp_parser *pars
>   
>   /* Warn if the attribute ATTRIBUTE appears more than once in the
>      attribute-list ATTRIBUTES.  This used to be enforced for certain
> -   attributes, but the restriction was removed in P2156.  Note that
> -   carries_dependency ([dcl.attr.depend]) isn't implemented yet in GCC.
> +   attributes, but the restriction was removed in P2156.
>      LOC is the location of ATTRIBUTE.  Returns true if ATTRIBUTE was not
>      found in ATTRIBUTES.  */
>   
> @@ -29249,7 +29248,7 @@ cp_parser_check_std_attribute (location_
>   {
>     static auto alist = { "noreturn", "deprecated", "nodiscard", "maybe_unused",
>   			"likely", "unlikely", "fallthrough",
> -			"no_unique_address" };
> +			"no_unique_address", "carries_dependency" };
>     if (attributes)
>       for (const auto &a : alist)
>         if (is_attribute_p (a, get_attribute_name (attribute))
> --- gcc/testsuite/g++.dg/cpp0x/attr-carries_dependency1.C.jj	2022-11-08 15:17:43.168238390 +0100
> +++ gcc/testsuite/g++.dg/cpp0x/attr-carries_dependency1.C	2022-11-08 15:16:39.695104787 +0100
> @@ -0,0 +1,17 @@
> +// { dg-do compile { target c++11 } }
> +
> +[[carries_dependency]] int *f1 ();		// { dg-warning "attribute ignored" }
> +int f2 (int *x [[carries_dependency]]);		// { dg-warning "attribute ignored" }
> +[[carries_dependency]] int f3 ();		// { dg-warning "attribute ignored" }
> +int f4 (int x [[carries_dependency]]);		// { dg-warning "attribute ignored" }
> +[[carries_dependency(1)]] int f5 ();		// { dg-error "'carries_dependency' attribute does not take any arguments" }
> +[[carries_dependency]] int v;			// { dg-warning "'carries_dependency' attribute can only be applied to functions or parameters" }
> +[[carries_dependency]];				// { dg-warning "attribute ignored" }
> +void
> +f6 ()
> +{
> +  [[carries_dependency]];			// { dg-warning "attributes at the beginning of statement are ignored" }
> +}
> +#if __has_cpp_attribute(carries_dependency)
> +#error carries_dependency attribute is not actually implemented
> +#endif
> 
> 	Jakub
>