[C++ Patch] A few more cp_expr_loc_or_input_loc and a diagnostic regression fix

Jason Merrill jason@redhat.com
Tue Dec 3 19:20:00 GMT 2019


On 12/2/19 5:22 PM, Paolo Carlini wrote:
> Hi,
> 
> On 02/12/19 19:58, Jason Merrill wrote:
>> On 11/29/19 8:08 AM, Paolo Carlini wrote:
>>> Hi,
>>>
>>> a few more rather straightforward uses for cp_expr_loc_or_input_loc.
>>>
>>> Additionally, while working on the latter, I noticed that, compared 
>>> to say, gcc-7, lately the code we have in cp_build_addr_expr_1 to 
>>> diagnose taking the address of 'main' often doesn't work anymore, 
>>> when the argument is wrapped in a location_wrapper. The below fixes 
>>> that by using tree_strip_any_location_wrapper in the DECL_MAIN_P 
>>> check, which works fine, but I can imagine various other ways to 
>>> solve the issue...
>>
>> Maybe
>>
>> location_t loc = cp_expr_loc_or_input_loc (arg);
>> STRIP_ANY_LOCATION_WRAPPER (arg);
>>
>> at the top?  In general I prefer the local variable to writing 
>> cp_expr_loc_or_input_loc in lots of places, whether or not we then 
>> strip wrappers.
> 
> Sure. In a few circumstances I hesitated because 
> cp_expr_loc_or_input_loc isn't really trivial, boils down to quite a few 
> conditionals, and adds a bit to the complexity of simple functions when 
> in fact no errors nor warnings are issued. But certainly calling it at 
> the top is often much cleaner. I changed both the functions.
> 
> However, using STRIP_ANY_LOCATION_WRAPPER at the beginning of 
> cp_build_addr_expr_1 doesn't seem straightforward, causes a few 
> regressions (wrong location for conversion/Wwrite-strings.C; even an ICE 
> for cpp1z/constexpr-lambda23.C; cpp1z/decomp48.C). The function doesn't 
> seem trivial from this point of view, there is already a localized 
> tree_strip_any_location_wrapper near the end, for 
> processing_template_decl. I would rather further investigate that kind 
> of simplification in a separate patch, beyond the regression fix. 
> (before I would like to improve the locations for all the various kinds 
> of cast, quite a few changes)

Absolutely, if it isn't simple don't bother.

> @@ -6061,6 +6061,7 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue
>  {
>    tree argtype;
>    tree val;
> +  location_t loc;
>  
>    if (!arg || error_operand_p (arg))
>      return error_mark_node;
> @@ -6070,6 +6071,7 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue
>      return error_mark_node;
>  
>    argtype = lvalue_type (arg);
> +  loc = cp_expr_loc_or_input_loc (arg);

You don't need to separately declare the variable at the top of the 
function anymore.  OK with that change.

Jason



More information about the Gcc-patches mailing list