[cfe-commits] [PATCH][Review Request] Handle CXXForRangeStmt

[Jim Goodnow II]

Hi Richard,

Excuse my density, :-) , but I'm not seeing the white-space issues other 
than the two extra lines that could be removed. Being pedantic is 
completely reasonable in a code base this large worked on by so many 
people. I'm just missing something, so any hints would be appreciated. 
Thanks!

  - jim

On 10/6/2011 8:01 PM, Richard Smith wrote:
> Hi Jim,
>
> On Thu, October 6, 2011 11:19, Jim Goodnow II wrote:
>> I made the changes you suggested and am resubmitting it. Thanks for your
>> help.
> Thanks. There are a few remaining whitespace cleanups to be made (sorry, I
> know this is pedantic):
>
> Index: lib/Sema/SemaStmt.cpp
> ===================================================================
> --- lib/Sema/SemaStmt.cpp        (revision 141174)
> +++ lib/Sema/SemaStmt.cpp        (working copy)
> @@ -1337,12 +1337,19 @@
>     if (!BeginEndDecl.get()&&  !RangeVarType->isDependentType()) {
>       SourceLocation RangeLoc = RangeVar->getLocation();
>
> -    ExprResult RangeRef = BuildDeclRefExpr(RangeVar,
> -                                           RangeVarType.getNonReferenceType(),
> -                                           VK_LValue, ColonLoc);
> -    if (RangeRef.isInvalid())
> +    const QualType RangeVarNonRefType = RangeVarType.getNonReferenceType();
> +
> +    ExprResult BeginRangeRef = BuildDeclRefExpr(RangeVar, RangeVarNonRefType,
> +                                             VK_LValue, ColonLoc);
>
> Here.
>
> +    if (BeginRangeRef.isInvalid())
>         return StmtError();
>
> +    ExprResult EndRangeRef = BuildDeclRefExpr(RangeVar, RangeVarNonRefType,
> +                                             VK_LValue, ColonLoc);
>
> Here.
>
> +
> +    if (EndRangeRef.isInvalid())
> +      return StmtError();
> +
>       QualType AutoType = Context.getAutoDeductType();
>       Expr *Range = RangeVar->getInit();
>       if (!Range)
> @@ -1368,8 +1375,8 @@
>         //   the program is ill-formed;
>
>         // begin-expr is __range.
> -      BeginExpr = RangeRef;
> -      if (FinishForRangeVarDecl(*this, BeginVar, RangeRef.get(), ColonLoc,
> +      BeginExpr = BeginRangeRef;
> +      if (FinishForRangeVarDecl(*this, BeginVar, BeginRangeRef.get(), ColonLoc,
>                                   diag::err_for_range_iter_deduction_failure)) {
>           NoteForRangeBeginEndFunction(*this, BeginExpr.get(), BEF_begin);
>           return StmtError();
> @@ -1391,7 +1398,7 @@
>         }
>
>         // end-expr is __range + __bound.
> -      EndExpr = ActOnBinOp(S, ColonLoc, tok::plus, RangeRef.get(),
> +      EndExpr = ActOnBinOp(S, ColonLoc, tok::plus, EndRangeRef.get(),
>                              BoundExpr.get());
>         if (EndExpr.isInvalid())
>           return StmtError();
> @@ -1431,14 +1438,14 @@
>         }
>
>         BeginExpr = BuildForRangeBeginEndCall(*this, S, ColonLoc, BeginVar,
> -                                            BEF_begin, BeginNameInfo,
> -                                            BeginMemberLookup, RangeRef.get());
> +                                      BEF_begin, BeginNameInfo,
> +                                      BeginMemberLookup, BeginRangeRef.get());
>
> Here.
>
>         if (BeginExpr.isInvalid())
>           return StmtError();
>
>         EndExpr = BuildForRangeBeginEndCall(*this, S, ColonLoc, EndVar,
>                                             BEF_end, EndNameInfo,
> -                                          EndMemberLookup, RangeRef.get());
> +                                          EndMemberLookup, EndRangeRef.get());
>         if (EndExpr.isInvalid())
>           return StmtError();
>       }
> @@ -1458,12 +1465,18 @@
>         BuildDeclaratorGroup(BeginEndDecls, 2, /*TypeMayContainAuto=*/false);
>       BeginEndDecl = ActOnDeclStmt(BeginEndGroup, ColonLoc, ColonLoc);
>
> -    ExprResult BeginRef = BuildDeclRefExpr(BeginVar,
> -                                           BeginType.getNonReferenceType(),
> +    const QualType BeginRefNonRefType = BeginType.getNonReferenceType();
> +    ExprResult BeginRef = BuildDeclRefExpr(BeginVar, BeginRefNonRefType,
>                                              VK_LValue, ColonLoc);
> +    if (BeginRef.isInvalid())
> +      return StmtError();
> +
>       ExprResult EndRef = BuildDeclRefExpr(EndVar, EndType.getNonReferenceType(),
>                                            VK_LValue, ColonLoc);
> +    if (EndRef.isInvalid())
> +      return StmtError();
>
> +
>
> No need for this extra line.
>
>       // Build and check __begin != __end expression.
>       NotEqExpr = ActOnBinOp(S, ColonLoc, tok::exclaimequal,
>                              BeginRef.get(), EndRef.get());
> @@ -1477,6 +1490,11 @@
>       }
>
>       // Build and check ++__begin expression.
> +    BeginRef = BuildDeclRefExpr(BeginVar, BeginRefNonRefType,
> +                                           VK_LValue, ColonLoc);
>
> Here.
>
> +    if (BeginRef.isInvalid())
> +      return StmtError();
> +
>       IncrExpr = ActOnUnaryOp(S, ColonLoc, tok::plusplus, BeginRef.get());
>       IncrExpr = ActOnFinishFullExpr(IncrExpr.get());
>       if (IncrExpr.isInvalid()) {
> @@ -1485,6 +1503,11 @@
>       }
>
>       // Build and check *__begin  expression.
> +    BeginRef = BuildDeclRefExpr(BeginVar, BeginRefNonRefType,
> +                                           VK_LValue, ColonLoc);
>
> Here.
>
> +    if (BeginRef.isInvalid())
> +      return StmtError();
> +
>       ExprResult DerefExpr = ActOnUnaryOp(S, ColonLoc, tok::star, BeginRef.get());
>       if (DerefExpr.isInvalid()) {
>         NoteForRangeBeginEndFunction(*this, BeginExpr.get(), BEF_begin);
>
>
>
>