rjmccall added inline comments.
================ Comment at: lib/Sema/SemaCast.cpp:535 + T1 = Unwrap(T1); + T2 = Unwrap(T2).withCVRQualifiers(T2.getCVRQualifiers()); + } ---------------- rsmith wrote: > rjmccall wrote: > > 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? > 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.) 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. Repository: rC Clang https://reviews.llvm.org/D49457 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
