Ping?

Il giorno 24/feb/2012, alle ore 12:09, Nicola Gigante ha scritto:

> 
> Il giorno 24/feb/2012, alle ore 03:28, Richard Smith ha scritto:
> 
>> Hi,
>> 
> Hello!
> 
>> On to the patch. I think it's really very close now:
>> [..]
> I'm going to fix all those issues, off course.
> Just a couple of things:
> 
>> -void CastOperation::CheckStaticCast() {
>> +void CastOperation::CheckStaticCast(bool &CastNodesCreated,
>> +                                    Sema::CheckedConversionInfo CCI) {
>> 
>> Rather than taking a bool argument by reference, it might be nicer to return 
>> a bool from here to indicate whether a CXXStaticCast node was created.
>> 
> I've chosen to use a by-reference argument for clarity. A bool returned by a 
> function named CheckStaticCast() can be very easily misinterpreted,
> in my opinion, to mean "checking done" vs "checking failed", even adding a 
> comment that says otherwise. Don't you think so?
> I'll change it anyway, if you wish.
> 
> 
>> +  CastNodesCreated = (SrcExpr.get() != SrcExprOrig &&
>> +                      Kind != CK_ConstructorConversion);
>> 
>> This approach makes me nervous: it seems too easy for us to accidentally 
>> change SrcExpr without building a cast node (checkNonOverloadPlaceholders 
>> could do this, for instance). Can we ensure that TryStaticCast returns 
>> TC_Success exactly when it's created a CXXStaticCast node, then use that to 
>> determine whether we need to build one?
>> 
> 
> You're right. I think we can be sure that TryStaticImplicitCast() has created 
> the node if it returns TC_Success and Kind != CK_ConstructorConversion.
> But it depends on how InitializationSequence::Perform() behaves. Do you think 
> this is the case?
> 
>> In the ElType == ToType case, it looks like this could still produce a no-op 
>> static cast containing an implicit cast.
> 
> You're right. Fixed it.
> 
> I've also contextually fixed a warning that appears in 
> addFixitForObjCARCConversion() that doesn't include
> the recently added CCK_StaticCast enum value in a switch statement.
> 
> Attached is the patch to the last revision with all the issues fixed 
> excepting the two questions I asked here.
> 
>> 
>> Thanks!
>> Richard
> 
> Thanks,
> Nicola
> 


_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to