On 02/12/2018 10:11 AM, Jason Merrill wrote:
On Mon, Feb 12, 2018 at 11:59 AM, Martin Sebor <mse...@gmail.com> wrote:
On 02/12/2018 09:30 AM, Jason Merrill wrote:
On Fri, Feb 9, 2018 at 6:57 PM, Martin Sebor <mse...@gmail.com> wrote:
On 02/09/2018 12:52 PM, Jason Merrill wrote:
On 02/08/2018 04:52 PM, Martin Sebor wrote:
I took me a while to find DECL_TEMPLATE_RESULT. Hopefully
that's the right way to get the primary from a TEMPLATE_DECL.
Yes.
Attached is an updated patch. It hasn't gone through full
testing yet but please let me know if you'd like me to make
some changes.
+ const char* const whitelist[] = {
+ "error", "noreturn", "warning"
+ };
Why whitelist noreturn? I would expect to want that to be consistent.
I expect noreturn to be used on a primary whose definition
is provided but that's not meant to be used the way the API
is otherwise expected to be. As in:
template <class T>
T [[noreturn]] foo () { throw "not implemented"; }
template <> int foo<int>(); // implemented elsewhere
Marking that template as noreturn seems pointless, and possibly harmful;
the deprecated, warning, or error attributes would be better for this
situation.
I meant either:
template <class T>
T __attribute__ ((noreturn)) foo () { throw "not implemented"; }
template <> int foo<int>(); // implemented elsewhere
or (sigh)
template <class T>
[[noreturn]] T foo () { throw "not implemented"; }
template <> int foo<int>(); // implemented elsewhere
It lets code like this
int bar ()
{
return foo<char>();
}
be diagnosed because it's likely a bug (as Clang does with
-Wunreachable-code). It doesn't stop code like the following
from compiling (which is good) but it instead lets them throw
at runtime which is what foo's author wants.
void bar ()
{
foo<char>();
}
It's the same as having an "unimplemented" base virtual function
throw an exception when it's called rather than making it pure
and having calls to it abort. Declaring the base virtual function
noreturn is useful for the same reason (and also diagnosed by
Clang). I should remember to add the same warning in GCC 9.
Yes, I understood the patterns you had in mind, but I disagree with
them. My point about harmful is that declaring a function noreturn
because it's unimplemented could be a problem for when the function is
later implemented, and callers were optimized inappropriately. This
seems like a rather roundabout way to get a warning about calling an
unimplemented function, and not worth overriding the normal behavior.
Removing noreturn from the whitelist means having to prevent
the attribute from causing conflicts with the attributes on
the blacklist. E.g., in this:
template <class T> [[malloc]] void* allocate (int);
template <> [[noreturn]] void* allocate<void> (int);
-Wmissing-attributes would warn for the missing malloc but
-Wattributes will warn once malloc is added. Ditto for all
other attributes noreturn is considered to conflict with such
as alloc_size and warn_unused_result.
This example seems rather unlikely, and the solution is to remove
[[noreturn]]. I don't think this is worth worrying about for GCC 8.
Removing [[noreturn]] is not a solution because (as I said)
-Wmissing-attributes will warn that the specialization is
missing the malloc attribute. The only solutions to avoid
both warnings are to either a) remove the malloc attribute
(and all others on the blacklist) from the primary or b) add
all of them to the specialization and remove the noreturn.
I.e., have them match. That makes sense except when either
the primary or the specialization in fact does not return.
I really don't think it's helpful to try to force noreturn
to match between the primary and its specializations.
The use case you are concerned about (a noreturn function
returning) is already diagnosed:
warning: function declared ‘noreturn’ has a ‘return’ statement
Martin
