Il giorno 24/ott/2011, alle ore 17:22, Douglas Gregor ha scritto:

>  ExprResult ImpCastExprToType(Expr *E, QualType Type, CastKind CK,
>                                ExprValueKind VK = VK_RValue,
> -                               const CXXCastPath *BasePath = 0,
> -                               CheckedConversionKind CCK 
> -                                  = CCK_ImplicitConversion);
> +                               const CXXCastPath *BasePath = 0);
> 
> I'm slightly nervous about the two defaulted arguments here, since '0' can be 
> silently passed for either of them, but I guess it works since '0' is 
> CCK_ImplicitConversion anyway :)
> 
> @@ -2193,13 +2192,13 @@
>   return Owned(From);
> }
> 
> -/// PerformImplicitConversion - Perform an implicit conversion of the
> +/// PerformConversion - Perform a conversion of the
> /// expression From to the type ToType by following the standard
> /// conversion sequence SCS. Returns the converted
> /// expression. Flavor is the context in which we're performing this
> /// conversion, for use in error messages.
> ExprResult
> -Sema::PerformImplicitConversion(Expr *From, QualType ToType,
> +Sema::PerformConversion(Expr *From, QualType ToType,
>                                 const StandardConversionSequence& SCS,
>                                 AssignmentAction Action, 
>                                 CheckedConversionKind CCK) {
> @@ -2277,20 +2276,20 @@
>   case ICK_Array_To_Pointer:
>     FromType = Context.getArrayDecayedType(FromType);
>     From = ImpCastExprToType(From, FromType, CK_ArrayToPointerDecay, 
> -                             VK_RValue, /*BasePath=*/0, CCK).take();
> +                             VK_RValue, /*BasePath=*/0).take();
>     break;
> 
>   case ICK_Function_To_Pointer:
>     FromType = Context.getPointerType(FromType);
>     From = ImpCastExprToType(From, FromType, CK_FunctionToPointerDecay, 
> -                             VK_RValue, /*BasePath=*/0, CCK).take();
> +                             VK_RValue, /*BasePath=*/0).take();
>     break;
> 
>   default:
>     llvm_unreachable("Improper first standard conversion");
>   }
> 
> I think you'll want to count the # of necessary conversions a bit more 
> closely here, or maybe compute the number ahead of time. This still creates 
> an extra conversion on code like this:
> 
> struct A { };
> 
> void f(A as[]) {
>  static_cast<const A*>(as);
> }
> 
> by producing
> 
>  (CXXStaticCastExpr 0x7f823203dde8 <line:4:3, col:27> 'const struct A *' 
> static_cast<const struct A *> <NoOp>
>    (ImplicitCastExpr 0x7f823203ddd0 <col:25> 'const struct A *' <NoOp>
>      (ImplicitCastExpr 0x7f823203ddb8 <col:25> 'struct A *' <LValueToRValue>
>        (DeclRefExpr 0x7f823203dd50 <col:25> 'struct A *' lvalue ParmVar 
> 0x7f823203dbf0 'as' 'struct A *')))))
> 
> Index: lib/Sema/SemaCast.cpp
> ===================================================================
> --- lib/Sema/SemaCast.cpp     (revision 142579)
> +++ lib/Sema/SemaCast.cpp     (working copy)
> @@ -289,6 +289,18 @@
>         return ExprError();
>     }
> 
> +    // CheckStaticCast _may_ have already created the cast node. Let's check
> +    if(CXXStaticCastExpr *Cast = 
> dyn_cast<CXXStaticCastExpr>(Op.SrcExpr.get()))
> +    {
> +      if(!Cast->getTypeInfoAsWritten())
> +      {
> +        Cast->setTypeInfoAsWritten(DestTInfo);
> +        Cast->setOperatorLoc(OpLoc);
> +        Cast->setRParenLoc(Parens.getEnd());
> +        return Op.complete(Cast);
> +      }
> +    }
> +    
> 
> This feels a little shaky. I assume that the inner "if" is intended to make 
> sure we don't cannibalize the inner "if" for something like
> 
>       static_cast<int>(static_cast<float>(something))
> 
> However, I'd feel far better if we had a more direct way to signal to this 
> code that the CXXStaticCastExpr has already been created. Also, it seems like 
> some assertions comparing Op.Kind to Cast->getCastKind() and comparing the 
> resulting types would be helpful to flush out any remaining issues.
> 
> Oh, and '{'s up on the line above, rather than on their own line, please.
> 
> The changes to Initialization.h (e.g., SIK_DirectCast -> SIK_DirectStaticCast 
> and related) could go in at any time, if you'd like to tease these out from 
> the main part of the patch.
> 

Hello.
I've attached a patch that corrects what you pointed out. The issue about 
FunctionToPointer and ArrayToPointer decays persists,
because they're handled in a very different way and they'll require a future 
new patch.
I've removed the "shaky if" by setting a boolean parameter in CheckStaticCast 
and CheckCXXCStyleCast, telling the caller
if the function have created and applied a new cast node.
I believe that this patch does what we need and is ready to be committed if you 
agree.
It applies to r143190 (today) and passes the tests.

>       - Doug
> 

Thanks,
Nicola

Attachment: cast.patch
Description: Binary data

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

Reply via email to