On Mar 12, 2013, at 10:01 AM, Jordan Rose <[email protected]> wrote:
> Pinging this myself, adding Richard Smith as another reviewer. I don't quite
> feel qualified to give the final word on it, but I'm fixing an analyzer bug
> that's basically "don't crash when the user does this in exactly the right
> wrong way", and it'd be nice to have a warning so that they fix their code.
+def ReinterpretUpDownCastZeroAdjustment :
+ DiagGroup<"reinterpret-updown-zero-adjustment">;
+def ReinterpretUpDownCast : DiagGroup<"reinterpret-updown",
How about -Wreinterpret-base and -Wreinterpret-primary-base?
+def warn_reinterpret_wrong_subobject : Warning<
+ "'reinterpret_cast' might return a pointer to wrong subobject">,
How about "reinterpret_cast %select{from|to}3 class %0 %select{to|from}3
%select{virtual base|base at non-zero offset}2 %1 behaves differently from
static_cast"?
+static void DiagnoseReinterpretUpDownCast(Sema &Self, ExprResult &SrcExpr,
+ QualType DestType,
+ const SourceRange &OpRange) {
You should take SrcExpr as a const Expr * and OpRange as a SourceRange.
+ const Type *SrcUnqualType = SrcType.getTypePtr();
+ const Type *DestUnqualType = DestType.getTypePtr();
This is unnecessary and also doesn't necessarily actually give you an
unqualified type.
+ // When casting from pointer or reference, get pointee type; use original
+ // type otherwise.
+ const CXXRecordDecl *SrcPointeeRD = SrcUnqualType->getPointeeCXXRecordDecl();
+ const CXXRecordDecl *SrcRD =
+ SrcPointeeRD ? SrcPointeeRD : SrcUnqualType->getAsCXXRecordDecl();
It'd be good to remember the original non-pointer sugared types here for
diagnostic purposes.
Also, at this point in semantic analysis you can differentiate between the
pointer and reference cases by just checking SrcExpr->isRValue().
+ CXXBasePaths SrcBP, DestBP;
+
...
+ const ASTRecordLayout &SrcLayout = Self.Context.getASTRecordLayout(SrcRD);
+ const ASTRecordLayout &DestLayout = Self.Context.getASTRecordLayout(DestRD);
1. Please sink this down to the point where you need it.
2. Please only actually get the layout for the derived class.
3. You can re-use the paths object if the first lookup failed.
+ if (SrcBP.getDetectedVirtual() || DestBP.getDetectedVirtual()) {
You are ignoring the possibility that there are subobjects at multiple paths.
We should not warn if *any* of the paths is a non-virtually-derived base at
offset zero.
+ CharUnits Offset = CharUnits::Zero();
+ if (ReinterpretKind == ReinterpretUpcast)
+ Offset = SrcLayout.getBaseClassOffset(DestRD);
+ else if (ReinterpretKind == ReinterpretDowncast)
+ Offset = DestLayout.getBaseClassOffset(SrcRD);
This won't work for indirect base classes.
+ DiagnoseReinterpretUpDownCast(Self, SrcExpr, DestType, OpRange);
+
Please sink this until we've decided that the cast succeeds.
John.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
