Address code review feedback and improve an error message.

This change addresses code review feedback to rename some variables and add
a comment.

It also improves the error message for bounds(a,b) when a and b are omitted.
We would get two error messages at the same location, one saying that an
expression was expected and the other saying that a ',' was expected.  The
improvement is to print the error message for the ',' only if nothing
went wrong trying to parse the lower bound.

Author: dtarditi

include/clang/Sema/DeclSpec.h

@@ -1288,7 +1288,7 @@ struct DeclaratorChunk {
     unsigned ExceptionSpecLocEnd;
 
     /// The location of the ':' for the return bounds expression in the source
-    unsigned ReturnBoundsColon;
+    unsigned ReturnBoundsColonLoc;
 
     /// Params - This is a pointer to a new[]'d array of ParamInfo objects that
     /// describe the parameters specified by this function declarator.  null if
@@ -1359,8 +1359,8 @@ struct DeclaratorChunk {
       return SourceLocation::getFromRawEncoding(RParenLoc);
     }
 
-    SourceLocation getReturnBoundsColon() const {
-      return SourceLocation::getFromRawEncoding(ReturnBoundsColon);
+    SourceLocation getReturnBoundsColonLoc() const {
+      return SourceLocation::getFromRawEncoding(ReturnBoundsColonLoc);
     }
 
     SourceLocation getExceptionSpecLocBeg() const {

lib/Parse/ParseDecl.cpp

@@ -5747,6 +5747,8 @@ void Parser::ParseFunctionDeclarator(Declarator &D,
     }
   }
 
+  // Parse optional Checked C bounds expression with the form:
+  // ':' bounds-expression.
   if (getLangOpts().CheckedC && Tok.is(tok::colon)) {
     BoundsColonLoc = Tok.getLocation();
     ConsumeToken();

lib/Parse/ParseExpr.cpp

@@ -2795,10 +2795,8 @@ ExprResult Parser::ParseBoundsExpression() {
       ExprResult LowerBound =
         Actions.CorrectDelayedTyposInExpr(ParseAssignmentExpression());
 
-     if (ExpectAndConsume(tok::comma))
-       // We didn't find a comma, so don't try to parse the upper bounds expression.
-       Result = ExprError();
-     else {
+     if (Tok.getKind() == tok::comma) {
+       ConsumeToken();
        ExprResult UpperBound =
          Actions.CorrectDelayedTyposInExpr(ParseAssignmentExpression());
        if (LowerBound.isInvalid() || UpperBound.isInvalid())
@@ -2807,6 +2805,15 @@ ExprResult Parser::ParseBoundsExpression() {
          Result = Actions.ActOnRangeBoundsExpr(BoundsKWLoc, LowerBound.get(),
                                                UpperBound.get(),
                                                Tok.getLocation());
+     } else {
+       // We didn't find a comma. Only issue an error message if the
+       // LowerBound expression is valid.  Otherwise, we already issued an
+       // error message when parsing the lower bound. Emitting an error
+       // message here could be spurious or confusing.
+       if (!LowerBound.isInvalid()) {
+         Diag(Tok, diag::err_expected) << tok::comma;
+       }
+       Result = ExprError();
      }
     } // if (!FoundNullaryOperator)
   } else {

lib/Parse/ParseExprCXX.cpp

@@ -1222,7 +1222,7 @@ ExprResult Parser::ParseLambdaExpressionAfterIntroducer(
                                              NoexceptExpr.get() : nullptr,
                                            /*ExceptionSpecTokens*/nullptr,
                                            LParenLoc, FunLocalRangeEnd,
-                                           /*ReturnBoundsColon=*/NoLoc,
+                                           /*ReturnBoundsColonLoc=*/NoLoc,
                                            /*ReturnBoundsExpr=*/nullptr,
                                            D,
                                            TrailingReturnType),
@@ -1294,7 +1294,7 @@ ExprResult Parser::ParseLambdaExpressionAfterIntroducer(
                                                /*NoexceptExpr=*/nullptr,
                                                /*ExceptionSpecTokens=*/nullptr,
                                                DeclLoc, DeclEndLoc,
-                                               /*ReturnBoundsColon=*/NoLoc,
+                                               /*ReturnBoundsColonLoc=*/NoLoc,
                                                /*ReturnBoundsExpr=*/nullptr,
                                                D,
                                                TrailingReturnType),

lib/Sema/DeclSpec.cpp

@@ -212,7 +212,7 @@ DeclaratorChunk DeclaratorChunk::getFunction(bool hasProto,
   I.Fun.HasTrailingReturnType   = TrailingReturnType.isUsable() ||
                                   TrailingReturnType.isInvalid();
   I.Fun.TrailingReturnType      = TrailingReturnType.get();
-  I.Fun.ReturnBoundsColon       = ReturnBoundsColonLoc.getRawEncoding();
+  I.Fun.ReturnBoundsColonLoc    = ReturnBoundsColonLoc.getRawEncoding();
   I.Fun.ReturnBounds            = ReturnBoundsExpr;
 
   assert(I.Fun.TypeQuals == TypeQuals && "bitfield overflow");

lib/Sema/SemaDecl.cpp

@@ -11628,7 +11628,7 @@ NamedDecl *Sema::ImplicitlyDefineFunction(SourceLocation Loc,
                                              /*NoexceptExpr=*/nullptr,
                                              /*ExceptionSpecTokens=*/nullptr,
                                              Loc, Loc,
-                                             /*ReturnBoundsColon=*/NoLoc,
+                                             /*ReturnBoundsColonLoc=*/NoLoc,
                                              /*ReturnBoundsExpr=*/nullptr, D),
                 DS.getAttributes(),
                 SourceLocation());