Re: C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v2)

[Martin Sebor <msebor@gmail.com>]

On 6/16/19 10:10 AM, Marek Polacek wrote:
> On Sat, Jun 15, 2019 at 10:39:13AM -0400, Marek Polacek wrote:
>> On Sat, Jun 15, 2019 at 04:33:26PM +0200, Jakub Jelinek wrote:
>>> On Sat, Jun 15, 2019 at 10:29:17AM -0400, Marek Polacek wrote:
>>>> [dcl.attr.noreturn] says "The first declaration of a function shall specify the
>>>> noreturn attribute if any declaration of that function specifies the noreturn
>>>> attribute" meaning that we should diagnose
>>>>
>>>>    void func ();
>>>>    void func [[noreturn]] ();
>>>>
>>>> but we do not.  I'd been meaning to issue a hard error for [[noreturn]] and
>>>> only a warning for __attribute__((noreturn)) but then I found out that we
>>>> treat [[noreturn]] exactly as the GNU attribute, and so cxx11_attribute_p
>>>> returns false for it, so I decided to make it a pedwarn for all the cases.
>>>> A pedwarn counts as a diagnostic, so we'd be conforming still.
>>>>
>>>> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>>>
>>> IMHO we should treat __attribute__((noreturn)) as before without any
>>> warnings, just [[noreturn]] that way.  There is nothing wrong on declaring
>>> it just on second or following declaration, it is an optimization attribute.
>>
>> That's a complication then; currently [[noreturn]], [[gnu::noreturn]], and
>> __attribute__((noreturn)) are not distinguishable.  :(
>>
>> Guess I will really have to make the changes to treat [[noreturn]] similarly
>> to e.g. [[nodiscard]], so that cxx11_attribute_p works.
> 
> Thus.  Changes I've made:
> * don't treat [[noreturn]] as an equivalent to __attribute__((noreturn));
> * for that I had to adjust decl_attributes, it wasn't preserving the
>    C++11 form (a list in another list); fix shadowing while at it;
> * the above turned up two spots that were wrongly accessing TREE_PURPOSE
>    directly instead of using get_attribute_name;
> * give error only for [[noreturn]] but not for __attribute__((noreturn))
>    or [[gnu::noreturn]].
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
> 
> 2019-06-16  Marek Polacek  <polacek@redhat.com>
> 
> 	PR c++/60364 - noreturn after first decl not diagnosed.
> 	* attribs.c (get_attribute_namespace): No longer static.
> 	(decl_attributes): Avoid shadowing.  Preserve the C++11 form for C++11
> 	attributes.
> 	* attribs.h (get_attribute_namespace): Declare.
> 	* tree-inline.c (function_attribute_inlinable_p): Use
> 	get_attribute_name.
> 
> 	* c-attribs.c (handle_noreturn_attribute): No longer static.
> 	* c-common.h (handle_noreturn_attribute): Declare.
> 	* c-format.c (check_function_format): Use get_attribute_name.
> 
> 	* decl.c (duplicate_decls): Give an error when a function is
> 	declared [[noreturn]] after its first declaration.
> 	* parser.c (cp_parser_std_attribute): Don't treat C++11 noreturn
> 	attribute as equivalent to GNU's.
> 	* tree.c (std_attribute_table): Add noreturn.
> 
> 	* g++.dg/warn/noreturn-8.C: New test.
...
> diff --git gcc/cp/tree.c gcc/cp/tree.c
> index cd021b7f594..bb695e14e73 100644
> --- gcc/cp/tree.c
> +++ gcc/cp/tree.c
> @@ -4453,6 +4453,8 @@ const struct attribute_spec std_attribute_table[] =
>       handle_likeliness_attribute, attr_cold_hot_exclusions },
>     { "unlikely", 0, 0, false, false, false, false,
>       handle_likeliness_attribute, attr_cold_hot_exclusions },
> +  { "noreturn", 0, 0, true, false, false, false,
> +    handle_noreturn_attribute, NULL },
                                   ^^^^

The GNU attribute is made mutually exclusive with a bunch of other
attributes (e.g., malloc or warn_unused_result) by setting the last
member to the array of exclusive attribute.  Does the change preserve
this relationship some other way?

Martin