[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