On Fri, 2018-01-05 at 15:29 -0500, Jason Merrill wrote:
> On 12/29/2017 12:06 PM, David Malcolm wrote:
> > One issue I ran into was that fold_for_warn doesn't eliminate
> > location wrappers when processing_template_decl, leading to
> > failures of the template-based cases in
> > g++.dg/warn/Wmemset-transposed-args-1.C.
> >
> > This is due to the early bailout when processing_template_decl
> > within cp_fold:
> >
> > 2078 if (processing_template_decl
> > 2079 || (EXPR_P (x) && (!TREE_TYPE (x) || TREE_TYPE
> > (x) == error_mark_node)))
> > 2080 return x;
> >
> > which dates back to the merger of the C++ delayed folding branch.
> >
> > I've fixed that in this version of the patch by removing that
> > "processing_template_decl ||" condition from that cp_fold early
> > bailout.
>
> Hmm, that makes me nervous. We might want to fold in templates when
> called from fold_for_warn, but not for other occurrences. But I see
> that we check processing_template_decl in cp_fully_fold and in the
> call
> to cp_fold_function, so I guess this is fine.
(I wondered if it would be possible to add a new flag to the various
fold* calls to request folding in templates, but given that the API is
partially shared with C doing so doesn't seem to make sense)
> > + case VIEW_CONVERT_EXPR:
> > + case NON_LVALUE_EXPR:
> > case CAST_EXPR:
> > case REINTERPRET_CAST_EXPR:
> > case CONST_CAST_EXPR:
> > @@ -14937,6 +14940,15 @@ tsubst_copy (tree t, tree args,
> > tsubst_flags_t complain, tree in_decl)
> > case CONVERT_EXPR:
> > case NOP_EXPR:
> > {
> > + if (location_wrapper_p (t))
> > + {
> > + /* Handle location wrappers by substituting the
> > wrapped node
> > + first, *then* reusing the resulting type. Doing
> > the type
> > + first ensures that we handle template parameters
> > and
> > + parameter pack expansions. */
> > + tree op0 = tsubst_copy (TREE_OPERAND (t, 0), args,
> > complain, in_decl);
> > + return build1 (code, TREE_TYPE (op0), op0);
> > + }
>
> I'd rather handle location wrappers separately, and abort if
> VIEW_CONVERT_EXPR or NON_LVALUE_EXPR appear other than as wrappers.
OK. I'm testing an updated version which does so.
> > @@ -24998,6 +25018,8 @@ build_non_dependent_expr (tree expr)
> > && !expanding_concept ())
> > fold_non_dependent_expr (expr);
> >
> > + STRIP_ANY_LOCATION_WRAPPER (expr);
>
> Why is this needed?
I added this to fix a failure:
template argument deduction/substitution failed:
note: cannot convert ‘0’ (type ‘int’) to type ‘char**’
seen building the precompiled header, where build_non_dependent_expr
adds a NON_DEPENDENT_EXPR around a location wrapper around an
INTEGER_CST, whereas without location wrappers we'd just have an
INTEGER_CST.
build_non_dependent_expr performs various tests for exact TREE_CODEs,
special-casing constants; without the stripping these TREE_CODE tests
don't match when the constants have location wrappers, leading to the
failure.
Here are the gory details:
In file included from ./src/libstdc++-
v3/include/precompiled/extc++.h:82:
./build/x86_64-pc-linux-gnu/libstdc++-
v3/include/ext/codecvt_specializations.h: In member function ‘virtual
std::codecvt_base::result std::codecvt<_InternT, _ExternT,
__gnu_cxx::__cxx11::encoding_state>::do_unshift(std::codecvt<_InternT,
_ExternT, __gnu_cxx::__cxx11::encoding_state>::state_type&,
std::codecvt<_InternT, _ExternT,
__gnu_cxx::__cxx11::encoding_state>::extern_type*,
std::codecvt<_InternT, _ExternT,
__gnu_cxx::__cxx11::encoding_state>::extern_type*,
std::codecvt<_InternT, _ExternT,
__gnu_cxx::__cxx11::encoding_state>::extern_type*&) const’:
./build/x86_64-pc-linux-gnu/libstdc++-
v3/include/ext/codecvt_specializations.h:392:58: error: no matching
function for call to ‘__iconv_adaptor(size_t (&)(iconv_t, char**,
size_t*, char**, size_t*), void* const&, int, int, char**,
std::size_t*)’
&__cto, &__tlen);
^
./build/x86_64-pc-linux-gnu/libstdc++-
v3/include/ext/codecvt_specializations.h:301:5: note: candidate:
‘template<class _Tp> std::size_t std::__iconv_adaptor(std::size_t
(*)(iconv_t, _Tp, std::size_t*, char**, std::size_t*), iconv_t, char**,
std::size_t*, char**, std::size_t*)’
__iconv_adaptor(size_t(*__func)(iconv_t, _Tp, size_t*, char**,
size_t*),
^~~~~~~~~~~~~~~
./build/x86_64-pc-linux-gnu/libstdc++-
v3/include/ext/codecvt_specializations.h:301:5: note: template
argument deduction/substitution failed:
./build/x86_64-pc-linux-gnu/libstdc++-
v3/include/ext/codecvt_specializations.h:392:58: note: cannot convert
‘0’ (type ‘int’) to type ‘char**’
&__cto, &__tlen);
^
(gdb) call debug_tree (arg)
<non_dependent_expr 0x7fffe7402820
type <integer_type 0x7ffff0e4f5e8 int asm_written public type_6 SI
size <integer_cst 0x7ffff0e520c0 constant 32>
unit-size <integer_cst 0x7ffff0e520d8 constant 4>
align:32 warn_if_not_align:0 symtab:-291372496 alias-set -1
canonical-type 0x7ffff0e4f5e8 precision:32 min <integer_cst
0x7ffff0e52078 -2147483648> max <integer_cst 0x7ffff0e52090 2147483647>
pointer_to_this <pointer_type 0x7ffff0e57a80> reference_to_this
<reference_type 0x7fffee916e70>>
arg:0 <non_lvalue_expr 0x7fffe7402720 type <integer_type
0x7ffff0e4f5e8 int>
constant
arg:0 <integer_cst 0x7ffff0e52210 constant 0>
./build/x86_64-pc-linux-gnu/libstdc++-
v3/include/ext/codecvt_specializations.h:391:50 start: ./build/x86_64-
pc-linux-gnu/libstdc++-v3/include/ext/codecvt_specializations.h:391:50
finish: ./build/x86_64-pc-linux-gnu/libstdc++-
v3/include/ext/codecvt_specializations.h:391:50>>
...when for a unpatched build we'd just have an INTEGER_CST.
Given the various TREE_CODE comparisons, using the
STRIP_ANY_LOCATION_WRAPPER macro seemed most appropriate; I put it
before the first TREE_CODE test.
I also added selftest::test_build_non_dependent_expr for this, to
verify that location wrappers don't affect the result of
build_non_dependent_expr (for integer constants and on string
literals).
Thanks
Dave
