On Mon, 2018-12-03 at 15:10 -0700, Jeff Law wrote: > On 11/19/18 9:51 AM, David Malcolm wrote: > > Ping, for these patches: > > > > [PATCH 1/2] C++: more location wrapper nodes (PR c++/43064, PR > > c++/43486) > > https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00304.html > > > > [PATCH 2/2] C++: improvements to binary operator diagnostics (PR > > c++/87504) > > https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00303.html > > > > > > Thanks > > Dave > > > > > > > > [1] I've split them up for ease of review; they could be reworked > > > to > > > be > > > fully independent, but there's some churn in the results for > > > -Wtautological-compare introduced by the 1st patch which the 2nd > > > patch addresses. > > > > > > gcc/ChangeLog: > > > PR c++/43064 > > > PR c++/43486 > > > * convert.c: Include "selftest.h". > > > (preserve_any_location_wrapper): New function. > > > (convert_to_pointer_maybe_fold): Update to handle location > > > wrappers. > > > (convert_to_real_maybe_fold): Likewise. > > > (convert_to_integer_1): Handle location wrappers when checking > > > for > > > INTEGER_CST. > > > (convert_to_integer_maybe_fold): Update to handle location > > > wrappers. > > > (convert_to_complex_maybe_fold): Likewise. > > > (selftest::test_convert_to_integer_maybe_fold): New functions. > > > (selftest::convert_c_tests): New function. > > > * fold-const.c (operand_equal_p): Strip any location wrappers. > > > * selftest-run-tests.c (selftest::run_tests): Call > > > selftest::convert_c_tests. > > > * selftest.h (selftest::convert_c_tests): New decl. > > > * tree.c (tree_int_cst_equal): Strip any location wrappers. > > > (maybe_wrap_with_location): Don't create wrappers if any > > > auto_suppress_location_wrappers are active. > > > (suppress_location_wrappers): New variable. > > > * tree.h (CONSTANT_CLASS_OR_WRAPPER_P): New macro. > > > (suppress_location_wrappers): New decl. > > > (class auto_suppress_location_wrappers): New class. > > > > > > gcc/c-family/ChangeLog: > > > PR c++/43064 > > > PR c++/43486 > > > * c-common.c (unsafe_conversion_p): Strip any location wrapper. > > > (verify_tree): Handle location wrappers. > > > (c_common_truthvalue_conversion): Strip any location wrapper. > > > Handle CONST_DECL. > > > (fold_offsetof): Strip any location wrapper. > > > (complete_array_type): Likewise for initial_value. > > > (convert_vector_to_array_for_subscript): Call fold_for_warn on > > > the > > > index before checking for INTEGER_CST. > > > * c-pretty-print.c (c_pretty_printer::primary_expression): > > > Don't > > > print parentheses around location wrappers. > > > * c-warn.c (warn_logical_operator): Call fold_for_warn on > > > op_right > > > before checking for INTEGER_CST. > > > (warn_tautological_bitwise_comparison): Call > > > tree_strip_any_location_wrapper on lhs, rhs, and bitop's > > > operand > > > before checking for INTEGER_CST. > > > (readonly_error): Strip any location wrapper. > > > (warn_array_subscript_with_type_char): Strip location wrappers > > > before checking for INTEGER_CST. Use the location of the index > > > if > > > available. > > > > > > gcc/cp/ChangeLog: > > > PR c++/43064 > > > PR c++/43486 > > > * call.c (build_conditional_expr_1): Strip location wrappers > > > when > > > checking for CONST_DECL. > > > (conversion_null_warnings): Use location of "expr" if > > > available. > > > * class.c (fixed_type_or_null): Handle location wrappers. > > > * constexpr.c (potential_constant_expression_1): Likewise. > > > * cvt.c (ignore_overflows): Strip location wrappers when > > > checking for INTEGER_CST, and re-wrap the result if present. > > > (ocp_convert): Call fold_for_warn before checking for > > > INTEGER_CST. > > > * decl.c (reshape_init_r): Strip any location wrapper. > > > (undeduced_auto_decl): Likewise. > > > * decl2.c (grokbitfield): Likewise for width. > > > * expr.c (mark_discarded_use): Likewise for expr. > > > * init.c (build_aggr_init): Likewise before checking init for > > > DECL_P. > > > (warn_placement_new_too_small): Call fold_for_warn on adj > > > before > > > checking for CONSTANT_CLASS_P, and on nelts. Strip any > > > location > > > wrapper from op0 and on oper before checking for VAR_P. > > > * lambda.c (add_capture): Strip any location from initializer. > > > * name-lookup.c (handle_namespace_attrs): Strip any location > > > from > > > x before checking for STRING_CST. > > > * parser.c (cp_parser_primary_expression): Call > > > maybe_add_location_wrapper on numeric and string literals. > > > (cp_parser_postfix_expression): Strip any location wrapper when > > > checking for DECL_IS_BUILTIN_CONSTANT_P. > > > (cp_parser_binary_expression): Strip any location wrapper when > > > checking for DECL_P on the lhs. > > > (cp_parser_decltype_expr): Suppress location wrappers in the > > > id-expression. > > > (cp_parser_mem_initializer): Add location wrappers to the > > > parenthesized expression list. > > > (cp_parser_template_parameter_list): Don't create wrapper nodes > > > within a template-parameter-list. > > > (cp_parser_template_argument_list): Don't create wrapper nodes > > > within a template-argument-list. > > > (cp_parser_parameter_declaration): Strip location wrappers from > > > default arguments. > > > (cp_parser_gnu_attribute_list): Don't create wrapper nodes > > > within > > > an attribute. > > > (cp_parser_late_parsing_default_args): Strip location wrappers > > > from default arguments. > > > (cp_parser_omp_all_clauses): Don't create wrapper nodes within > > > OpenMP clauses. > > > (cp_parser_omp_for_loop): Likewise. > > > (cp_parser_omp_declare_reduction_exprs): Likewise. > > > * pt.c (convert_nontype_argument_function): Strip location > > > wrappers from fn_no_ptr before checking for FUNCTION_DECL. > > > (do_auto_deduction): Likewise from init before checking for > > > DECL_P. > > > * semantics.c (force_paren_expr): Likewise from expr before > > > checking for DECL_P. > > > (finish_parenthesized_expr): Likewise from expr before > > > checking for STRING_CST. > > > (perform_koenig_lookup): Likewise from fn. > > > (finish_call_expr): Likewise. > > > (finish_id_expression): Rename to... > > > (finish_id_expression_1): ...this, calling > > > maybe_add_location_wrapper on the result. > > > * tree.c (cp_stabilize_reference): Strip any location wrapper. > > > (builtin_valid_in_constant_expr_p): Likewise. > > > (is_overloaded_fn): Likewise. > > > (maybe_get_fns): Likewise. > > > (selftest::test_lvalue_kind): Verify lvalue_p. > > > * typeck.c (cxx_sizeof_expr): Strip any location wrapper. > > > (cxx_alignof_expr): Likewise. > > > (is_bitfield_expr_with_lowered_type): Handle location wrappers. > > > (cp_build_array_ref): Strip location wrappers from idx before > > > checking for INTEGER_CST. > > > (cp_build_binary_op): Strip location wrapper from first_arg > > > before > > > checking for PARM_DECL. Likewise for op1 before checking for > > > INTEGER_CST in two places. Likewise for orig_op0 and orig_op1 > > > when checking for STRING_CST. > > > (cp_build_addr_expr_1): Likewise for arg when checking for > > > FUNCTION_DECL. > > > (cp_build_modify_expr): Likewise for newrhs when checking for > > > STRING_CST. > > > (convert_for_assignment): Don't strip location wrappers when > > > stripping NON_LVALUE_EXPR. > > > (maybe_warn_about_returning_address_of_local): Strip location > > > wrapper from whats_returned before checking for DECL_P. > > > (can_do_nrvo_p): Strip location wrapper from retval. > > > (treat_lvalue_as_rvalue_p): Likewise. > > > (check_return_expr): Likewise. > > > * typeck2.c (cxx_incomplete_type_diagnostic): Strip location > > > wrapper from value before checking for VAR_P or PARM_DECL. > > > (digest_init_r): Strip location wrapper from init. When > > > copying "init", also copy the wrapped node. > > > > > > gcc/objc/ChangeLog: > > > PR c++/43064 > > > PR c++/43486 > > > * objc-act.c (objc_maybe_build_component_ref): Strip any > > > location > > > wrapper before checking for UOBJC_SUPER_decl and self_decl. > > > (objc_finish_message_expr): Strip any location wrapper. > > > > > > gcc/testsuite/ChangeLog: > > > PR c++/43064 > > > PR c++/43486 > > > * c-c++-common/pr51712.c (valid2): Mark xfail as passing on > > > C++. > > > * g++.dg/cpp1z/decomp48.C: Update expected location of warning > > > for named local variables to use that of the local variable. > > > * g++.dg/init/array43.C: Update expected column to be that of > > > the > > > initializer. > > > * g++.dg/init/initializer-string-too-long.C: New test. > > > * g++.dg/init/pr43064-1.C: New test. > > > * g++.dg/init/pr43064-2.C: New test. > > > * g++.dg/init/pr43064-3.C: New test. > > > * g++.dg/wrappers/Wparentheses.C: New test. > > Well, you probably know my biggest worry with this -- the unwrappign > wack-a-mole problem. It looks like additional memory usage is > measurable, but tiny. > > Have you received any feedback from Jason on the C++ side? That's > the > bulk of this patch AFAICT.
Not yet. > I don't see anything objectionable in the c-family/ or generic bits. > But again I worry more about the need to sprinkle unwrapping all over > the place. Thanks for looking at this. FWIW, the patches have bit-rotted somewhat over the last month; I'm working on fixing them. Dave
