[cfe-dev] const_cast and p0388 (array boundloss becomes const_cast)

[Richard Smith via cfe-dev]

On Wed, 14 Apr 2021 at 12:13, Nathan Sidwell <nathan at acm.org> wrote:

> On 4/14/21 2:39 PM, Richard Smith via cfe-dev wrote:
> > On Wed, 14 Apr 2021 at 08:49, Nathan Sidwell via cfe-dev
> > <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
> >
> >     P0388
> >     http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p0388r4.html
> >     <
> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p0388r4.html>
> >     Makes conversions between bounded-array and unbounded-array look like
> >     qualification conversions.  This means you can use const-cast to add
> >     array bounds (and implicitly lose bounds)
> >
> >
> > I think we changed the const_cast rules to disallow that. Here:
> >
> > Modify 7.6.1.10 [expr.const.cast] ¶3:
> >
> > "For two similar types T1 and T2, a prvalue of type T1 may be explicitly
> > converted to the type T2 using a const_cast <ins>if, considering the
> > cv-decompositions ([conv.qual]) of both types, all Pi1 are the same as
> > Pi2</ins>."
> >
> > A change in array bound would result in the Pi's differing between T1
> > and T2, so a const_cast can't do that.
>
> Ah, I'd misunderstood that change, /and/ paid too much attention to
> 'modelled like a cv qualifier'.
>
> > Regardless of the above observation about const_cast, I think we do need
> > to think about the cast kind for array bound conversions, because
> > implicit conversions can now remove an array bound, and static_cast can
> > now add one. Seems like the choices are CK_NoOp, CK_BitCast, or
> > something new. I don't like using CK_BitCast for this, since that
> > usually implies something more violent is happening (and would cause
> > problems for constant evaluation, which generally allows pointer
> bitcasts).
>
> Agreed. (CK_LValueBitCast also looked applicable)
>
> > CK_NoOp is really a misnomer, and should probably be renamed to
> > something that indicates what it actually models: a change in
> > qualifiers. I think that changing array bounds is notionally  the same
> > kind of type change as changing type qualifiers or 'noexcept' on a
> > function type (even though in IR it will result in a type change until
> > the work to remove pointee types is completed), so my inclination would
> > be to call that a CK_NoOp too, unless we have a reason not to -- and
> > that's probably also easier because it means we don't need to work out
> > how to decompose an arbitrary qualification conversion into a change of
> > cv-qualifiers and a separate change of array bounds. I think it should
> > be relatively straightforward to get CodeGen to insert bitcasts for
> > CK_NoOp casts where necessary.
>
> The fly in that ointment is, IIUC, that the array bounds changes are
> visible in IR, but the cv qualifier and noexcept changes are not?  So we
> have to emit some type conversion IR to avoid mismatches there -- that
> is how I ran into this problem after all.  I will go poke some more.
> Perhaps the cast emission for CK_NoOp (line 4674 of CGExpr.cpp)
>      return EmitLValue(E->getSubExpr());
> needs to do more, sometimes.
>

Yes, I think we should create an IR-level bitcast where necessary here. At
least one path through IR generation already does
(EmitPointerWithAlignment), but it looks like we mostly assume that CK_NoOp
is, well, a no-op for IR generation. (There's ongoing work to replace the
IR-level "pointer to T" type family with a single "pointer" type, which
would remove the need for a bitcast, but I think that's probably still
months or years from being complete and I wouldn't imagine we want to wait
for that.)


> > On the subject of constant evaluation, do we have clear rules on whether
> > things like this work:
> >
> > constexpr int a[3][4] = {};
> > constexpr int (*p)[] = a;
> > constexpr int (*q)[12] = static_cast<int (*)[12]>(p);
> > constexpr int r = (*q)[5];
> >
> > ? I think that's probably valid up until the final line, which we should
> > reject for out-of-bounds array indexing because q points to an array of
> > 4 ints. (Though it might be nicer if we could reject the incorrect array
> > bound in the static_cast.) I think that's the behavior we'll get by
> > modeling this as a CK_NoOp.
> >
> >     nathan
> >
> >     --
> >     Nathan Sidwell
> >     _______________________________________________
> >     cfe-dev mailing list
> >     cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
> >     https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> >     <https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev>
> >
> >
> > _______________________________________________
> > cfe-dev mailing list
> > cfe-dev at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> >
>
>
> --
> Nathan Sidwell
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20210414/0f86a5cf/attachment.html>