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

[Richard Smith]

Hi Jim,

On Fri, October 7, 2011 12:28, Jim Goodnow II wrote:
> 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!

Sorry, I should have been more explicit. When a function call's arguments are
wrapped onto multiple lines, the second and subsequent lines are usually
indented to the column after the open paren of the function call.

Thanks!
Richard

> 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);
>>
>>
>>
>>
>>
>