On Thu, Jan 14, 2021 at 08:43:54AM +0100, Richard Biener wrote:
> > But for
> > diagnostics that is what the user actually want to see IMHO.
> > So on the attached testcase, instead of printing what is in left column
> > it prints what is in right column:
> > ((int*)t) + 3 t.u.b
> > ((int*)t) + 6 t.u.e.i
> > ((int*)t) + 8 t.v
> > s + 1 s[1]
>
> so while that's "nice" in general, for TBAA diagnostics it might actually
> be misleading.
>
> I wonder whether we absolutely need to print a C expression here.
I'm afraid yes, because it is not a toplevel routine, but something called
from the c-family pretty-printers, so it can be in the middle of arbitrary
C/C++ expressions. And printing
(3 * (access to a memory object of type 'int' at offset 12 bytes from 't') +
31) * 42
would be just weird.
> We could print, instead of *((int *)t + 3), "access to a memory
> object of type 'int' at offset 12 bytes from 't'", thus explain
> in plain english.
>
> That said, *((int *)t + 3) is exactly what the access is,
*((int *)&t + 3) actually, the code I haven't touched has multiple bugs.
The user generally doesn't know the exact layout of the structures,
and especially with C++ templates it is extremely hard to figure that out,
so even when we could print verbose text it would be helpful to give a hint
(in your text something like (which falls into 't.u.b')).
I don't see how we can print both the MEM_REF type and TBAA type in a way
that would be understandable to the user.
Could we print
t.u.b
if the TBAA type is compatible with the type of the reference and perhaps
*(int*)&t.u.b
if it is incompatible?
>From the aliasing perspective that is still different, but we don't print
the TBAA type anyway.
> In the light of Martins patch this is probably reasonable but still
> the general direction is wrong (which is why I didn't approve Martins
> original patch). I'm also somewhat disappointed we're breaking this
> so late in the cycle.
I'm too.
> c_fold_indirect_ref_for_warn doesn't look like it is especially
> careful about error recovery issues (error_mark_node in random
> places of the trees). Maybe that never happens.
I've created it by copying and adjusting the C++ cxx_fold_indirect_ref_1
which had those error_mark_node checks in there (haven't verified if
they are strictly necessary or not), but as the diagnostic code isn't used
solely during middle-end, but also in the FEs and I remember several cases
where the types had error marks within the types in there.
The function seemed to be too short and after the changes too different
from cxx_fold_indirect_ref_1, which contains some very C++ specific parts,
handling of the active union member or empty bases (there is a pending PR
for it) etc.
Jakub
