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

[Nathan Sidwell via cfe-dev]

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.

> 
> 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