On Sep 11, 2012, at 10:16 PM, Rafael Espíndola <[email protected]>
wrote:
>> The logic of this routine is fairly broken. It's trying to find an object
>> construction (which is doesn't do correctly, even after your patch) and
>> using that to blacklist all of the ill-formed patterns, rather than
>> detecting based on the type and then white-listing the few exceptions.
>>
>> It should really check for the valid cases directly (scalar/vector types are
>> okay, class types are okay when we're using the trivial default ctor and the
>> dtor is trivial) and fail with a generic error if our type is not one of
>> those valid cases. That way, the restricted form of the checking for
>> CXXConstructExpr that's currently being used will suffice for detecting the
>> "constructed with trivial constructor" case.
>
> I think we still need to find object construction to decide if we need
> an OutDiag or not.
Ah, yes, that's correct. There's also a terrible case that your patch doesn't
get right, which is something like:
struct X { operator &int() const; }
const int &i = X();
The problem here, which I hadn't been thinking about before, is lifetime
extension for temporaries. We actually have to dig into the initializer
expression to find the CXXConstructExpr, even when the variable type is a
reference to a scalar type.
> The attached patch improves the existing logic by
> getting the CXXRecordDecl from the type, computing OutDiag early and
> having a white list for InDiag as you suggested.
+ // If the variable is not of record type (or array thereof) the
+ // initializer cannot be a trivial ctor and there is no destructor
+ // to worry about.
+ const Type *T = Init->getType().getTypePtr();
+ if (T->isArrayType())
+ T = T->getBaseElementTypeUnsafe();
+ const CXXRecordDecl *Record = T->getAsCXXRecordDecl();
+ if (!Record)
+ return ScopePair(diag::note_protected_by_variable_init, 0);
+
This early exit seems not to work, per my example above.
+ // If we need to call a non trivial destructor for this variable,
+ // record an out diagnostic.
+ unsigned OutDiag = 0;
+ if (!Record->hasTrivialDestructor() && !isa<DeclRefExpr>(Init))
+ OutDiag = diag::note_exits_dtor;
I don't see what the DeclRefExpr check is meant to do.
FWIW, here's my test from above in a form that you can add to your patch:
namespace test13 {
struct C {
C(int x);
~C();
operator int&() const;
};
void f(void **ip) {
static void *ips[] = { &&l0 };
l0: // expected-note {{possible target of indirect goto}}
const int &c1 = C(1); // expected-note {{jump exits scope of variable with
non-trivial destructor}}
goto *ip; // expected-error {{indirect goto might cross protected scopes}}
}
}
- Doug
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
