Page MenuHomePhabricator

DR330: when determining whether a cast casts away constness, consider qualifiers from all levels matching a multidimensional array
ClosedPublic

Authored by rsmith on Jul 17 2018, 4:42 PM.

Details

Summary

DR330: when determining whether a cast casts away constness, consider
qualifiers from all levels matching a multidimensional array.

For example, this allows casting from

pointer to       array of            array of   const volatile int

to

pointer to const pointer to volatile pointer to                int

because the multidimensional array part of the source type corresponds
to a part of the destination type that contains both 'const' and
'volatile'.

Fixes the bug reported here: https://github.com/apple/swift/pull/17882#discussion_r203196368

Diff Detail

Repository
rC Clang

Event Timeline

rsmith created this revision.Jul 17 2018, 4:42 PM
rjmccall added inline comments.Jul 17 2018, 6:50 PM
lib/Sema/SemaCast.cpp
535

Hmm. Just CVR? I understand that we can have problems here with the enumerated qualifiers, so maybe we shouldn't blindly merge them, but is there some way to propagate them if not conflicting?

rsmith added inline comments.Jul 17 2018, 7:24 PM
lib/Sema/SemaCast.cpp
535

When diagnosing casting away constness, we intentionally only look at CVR qualifiers rather than the full gamut of qualifiers, so that's all I propagated here. (The restriction on casting away qualifiers in reinterpret_cast is arbitrary nannying, and I don't think we should extend its scope. In any case, a const_cast rightly can't change non-CVR qualifiers, so if we diagnosed anything else there'd be no way to perform certain casts.)

That said, the caller (below) can also check for ObjC lifetime qualifier mismatches, so we should figure out what the right rule is for that case. Fortunately, if I understand correctly a lifetime qualifier can only appear at the innermost level in a cv-decomposition (we either have a pointer to a lifetime type, which can't be further decomposed, or a block pointer whose pointee must be a function type and therefore can't be further decomposed), so I don't think we need any special handling for that case.

If I'm wrong about that and we do actually support lifetime qualifiers anywhere other than the lowest level in a cv-decomposition, we'll need to figure out what to do when we have multiple inconsistent qualifiers. (But given that's our extension anyway, perhaps we could pick a rule that's more sane than the C++ standard's rule, such as only decomposing similar levels of types, and skipping all "array of" qualifiers on both sides regardless of how many there are on each, so the question doesn't arise.)

rjmccall added inline comments.Jul 17 2018, 7:51 PM
lib/Sema/SemaCast.cpp
535

No, that all makes sense to me, and you're right about the ARC ownership qualifiers. This seems reasonable to me.

...well, I do wonder if we're really getting much from UnwrapSimilarTypes here, and especially the way it pays attention to array sizes makes me anxious. But we don't need to change that now.

Hi @rsmith – I've verified this patch fixes the original issue. Thanks for the quick fix!

This revision was not accepted when it landed; it landed in state Needs Review.Jul 18 2018, 1:18 PM
This revision was automatically updated to reflect the committed changes.