Re: [PATCH v2 1/4] gdb: Add typesafe getter/setter for cmd_list_element.var

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

>>> diff --git a/gdb/command.h b/gdb/command.h
>>> index baf34401a07..644812c4d46 100644
>>> --- a/gdb/command.h
>>> +++ b/gdb/command.h
>>> @@ -125,6 +125,164 @@ typedef enum var_types
>>>    }
>>>  var_types;
>>>  
>>> +/* Return true if a setting of type VAR_TYPE is backed with type T.
>>> +
>>> +   This  function is left without definition intentionally.  This template
>>> +   is specialized for all valid types that are used to back var_types.
>>> +   Therefore if one tries to instantiate this un-specialized template it
>>> +   means the T parameter is not a type used to back a var_type and it is most
>>> +   likely a programming error.  */
>>> +template<typename T>
>>> +inline bool var_type_uses (var_types t);
>>
>> So what will happen if you try to use var_type_uses with an invalid
>> template type, link error?  Tony suggested using a static_assert to have
>> a compile-time error instead, with an explanation.  I suppose you would
>> define a body for this function and put a static_assert (false) in
>> there?
> 
> With a 'standard' (development) build, this translate to compile-time
> error.  I did not pay enough attention, but I implicitly relied on the
> fact that the configure script adds '-Werror' by default.  Something
> should be in place to fail as early as possible (i.e. compile-time)
> whatever compiler options are in use.
> 
> We cannot use a plain 'static_assert (false)' here, it will fail even if
> this template is never instantiated.  All versions of g++/clang++ I just
> tried do trigger consistently:
> 
> 	$CXX -c -x c++ - <<EOT
> 	template<typename T>
> 	void foo ()
> 	{  static_assert (false, "Invalid instantiation."); }
> 	EOT
> 
> 	<stdin>:3:4: error: static_assert failed "Invalid instantiation."
> 	{  static_assert (false, "Invalid instantiation."); }
> 	   ^              ~~~~~
> 	   1 error generated.
> 
> I could use something similar to this:
> 
> 	template<typename T>
> 	inline bool var_type_uses (var_types var_type ATTRIBUTE_UNUSED)
> 	{
> 	  static_assert (sizeof(T) == 0,
> 			 "Invalid type used to instantiate 'var_type_uses'. "
> 			 "Use one of the types associated with enum var_types.");
> 	  return false;
> 	}
> 
> 
> It is not the prettiest way of writing an assertion, but it is the only
> one I am aware of that can work in this situation.
> 
> Would this be OK?

I think what you have already is fine.  It produces a diagnostic, which
is typically error in development builds.  And it's not like
var_type_uses is meant to be used everywhere, it's used in the internal
command code and that's it.  I think it's sufficiently clear like that.

Simon