On 10/18/2017 02:52 PM, Marek Polacek wrote:
> On Wed, Oct 18, 2017 at 02:46:23PM +0200, Martin Liška wrote:
>> On 10/12/2017 10:48 AM, Jakub Jelinek wrote:
>>> On Thu, Oct 12, 2017 at 10:40:42AM +0200, Martin Liška wrote:
>>>> --- a/gcc/cp/constexpr.c
>>>> +++ b/gcc/cp/constexpr.c
>>>> @@ -1175,7 +1175,12 @@ cxx_eval_builtin_function_call (const constexpr_ctx *ctx, tree t, tree fun,
>>>>  	{
>>>>  	  new_call = build_call_array_loc (EXPR_LOCATION (t), TREE_TYPE (t),
>>>>  					   CALL_EXPR_FN (t), nargs, args);
>>>> -	  error ("%q+E is not a constant expression", new_call);
>>>> +
>>>> +	  /* Do not allow__builtin_unreachable in constexpr function.  */
>>>> +	  if (DECL_FUNCTION_CODE (fun) == BUILT_IN_UNREACHABLE)
>>>
>>> As I said earlier, I think it would be better to differentiate between
>>> explicit __builtin_unreachable and the implicitly added one from the patch.
>>> So this could be done as
>>> if (DECL_FUNCTION_CODE (fun) == BUILT_IN_UNREACHABLE
>>>     && EXPR_LOCATION (t) == BUILTINS_LOCATION)
>>>
>>>> +  location_t loc = DECL_SOURCE_LOCATION (fndecl);
>>>> +  if (sanitize_flags_p (SANITIZE_RETURN, fndecl))
>>>> +    t = ubsan_instrument_return (loc);
>>>> +  else
>>>> +    t = build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_UNREACHABLE),
>>>
>>> and here use BUILTINS_LOCATION instead of loc.
>>> The code might be more readable by doing:
>>>     {
>>>       tree fndecl = builtin_decl_explicit (BUILT_IN_UNREACHABLE);
>>>       t = build_call_expr_loc (BUILTINS_LOCATION, fndecl, 0);
>>>     }
>>>
>>>> +			     0);
>>>> +
>>>
>>> 	Jakub
>>>
>>
>> Hi.
>>
>> I'm sending updated version of the patch that should address it.
>>
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>
>> Ready to be installed?
>> Martin
> 
>> From 36f3f45d9fa42344261faf60bb3cfbe22ed262ac Mon Sep 17 00:00:00 2001
>> From: marxin <mliska@suse.cz>
>> Date: Thu, 12 Oct 2017 10:14:59 +0200
>> Subject: [PATCH 1/3] Instrument function exit with __builtin_unreachable in
>>  C++
>>
>> gcc/c-family/ChangeLog:
>>
>> 2017-10-12  Martin Liska  <mliska@suse.cz>
>>
>> 	PR middle-end/82404
>> 	* c-opts.c (c_common_post_options): Set -Wreturn-type for C++
>> 	FE.
>> 	* c.opt: Set default value of warn_return_type.
>>
>> gcc/cp/ChangeLog:
>>
>> 2017-10-12  Martin Liska  <mliska@suse.cz>
>>
>> 	PR middle-end/82404
>> 	* constexpr.c (cxx_eval_builtin_function_call): Handle
>> 	__builtin_unreachable call.
>> 	* cp-gimplify.c (cp_ubsan_maybe_instrument_return): Rename to
>> 	...
>> 	(cp_maybe_instrument_return): ... this.
>> 	(cp_genericize): Call the function unconditionally.
>>
>> gcc/fortran/ChangeLog:
>>
>> 2017-10-12  Martin Liska  <mliska@suse.cz>
>>
>> 	PR middle-end/82404
>> 	* options.c (gfc_post_options): Set default value of
>> 	-Wreturn-type to false.
>> ---
>>  gcc/c-family/c-opts.c |  3 +++
>>  gcc/c-family/c.opt    |  2 +-
>>  gcc/cp/constexpr.c    |  8 +++++++-
>>  gcc/cp/cp-gimplify.c  | 20 ++++++++++++++------
>>  gcc/fortran/options.c |  3 +++
>>  5 files changed, 28 insertions(+), 8 deletions(-)
>>
>> diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
>> index 6bd535532d3..682d7a83ec5 100644
>> --- a/gcc/c-family/c-opts.c
>> +++ b/gcc/c-family/c-opts.c
>> @@ -978,6 +978,9 @@ c_common_post_options (const char **pfilename)
>>  	flag_extern_tls_init = 1;
>>      }
>>  
>> +  if (warn_return_type == -1)
>> +    warn_return_type = c_dialect_cxx () ? 1 : 0;
> 
> Here you can simply
> 
>   warn_return_type = c_dialect_cxx ();
> 
> no?
> 
> 	Marek
> 

Yes, thanks for the nit.

Martin