Merge pull request #5321 from mxswd/tuple-expr-sourcerange

Updated TupleExpr's getSourceRange to inspect child elements source ranges

Author: shahmishal

lib/AST/Expr.cpp

@@ -1322,14 +1322,26 @@ SourceRange TupleExpr::getSourceRange() const {
   } else if (getNumElements() == 0) {
     return { SourceLoc(), SourceLoc() };
   } else {
-    start = getElement(0)->getStartLoc();
+    // Scan forward for the first valid source loc.
+    for (Expr *expr : getElements()) {
+      start = expr->getStartLoc();
+      if (start.isValid()) {
+        break;
+      }
+    }
   }
 
   if (hasTrailingClosure() || RParenLoc.isInvalid()) {
     if (getNumElements() == 0) {
       return { SourceLoc(), SourceLoc() };
     } else {
-      end = getElements().back()->getEndLoc();
+      // Scan backwards for a valid source loc.
+      for (Expr *expr : reversed(getElements())) {
+        end = expr->getEndLoc();
+        if (end.isValid()) {
+          break;
+        }
+      }
     }
   } else {
     end = RParenLoc;

unittests/AST/SourceLocTests.cpp

@@ -199,22 +199,24 @@ TEST(SourceLoc, TupleExpr) {
   TestContext C;
 
   // In a TupleExpr, if the parens are both invalid, then you can only have a
-  // valid range iff both the first element and last element have valid ranges.
+  // valid range if there exists at least one expr with a valid source range.
+  // The tuple's source range will be the upper bound of the inner source
+  // ranges.
   // Source ranges also have the property:
   //   Start.isValid() == End.isValid()
   // For example, given the buffer "one", of the form:
   // (tuple_expr
   //   (declref_expr range=[test.swift:1:0 - line:1:2] ...)
   //   (declref_expr range=invalid ...))
-  // the range of this TupleExpr is SourceLoc() (invalid).
+  // the range of this TupleExpr is 1:0 - 1:2.
   //       v invalid                v invalid
   //       (     one,         two   )
   //       valid ^    invalid ^
   // COL:  xxxxxx012xxxxxxxxxxxxxxxxx
-  // but the SourceRange of 'one' is 1:0 - 1:2.
+  // and the SourceRange of 'one' is 1:0 - 1:2.
 
-  //                                                012
-  auto bufferID = C.Ctx.SourceMgr.addMemBufferCopy("one");
+  //                                                01234567
+  auto bufferID = C.Ctx.SourceMgr.addMemBufferCopy("one four");
   SourceLoc start = C.Ctx.SourceMgr.getLocForBufferStart(bufferID);
 
   auto one = new (C.Ctx) UnresolvedDeclRefExpr(
@@ -227,22 +229,61 @@ TEST(SourceLoc, TupleExpr) {
       DeclRefKind::Ordinary,
       DeclNameLoc());
 
+  auto three = new (C.Ctx) UnresolvedDeclRefExpr(
+      C.Ctx.getIdentifier("three"),
+      DeclRefKind::Ordinary,
+      DeclNameLoc());
+  
+  auto four = new (C.Ctx) UnresolvedDeclRefExpr(
+      C.Ctx.getIdentifier("four"),
+      DeclRefKind::Ordinary,
+      DeclNameLoc(start.getAdvancedLoc(4)));
+  
+  EXPECT_EQ(start, one->getStartLoc());
+  EXPECT_EQ(SourceLoc(), two->getStartLoc());
+  
+  // a tuple with only invalid elements
+  SmallVector<Expr *, 2> subExprsInvalid({ two, three });
+  SmallVector<Identifier, 2> subExprNamesInvalid(2, Identifier());
+  auto allInvalid = TupleExpr::createImplicit(C.Ctx, subExprsInvalid, subExprNamesInvalid);
+  
+  EXPECT_EQ(SourceLoc(), allInvalid->getStartLoc());
+  EXPECT_EQ(SourceLoc(), allInvalid->getEndLoc());
+  EXPECT_EQ(SourceRange(), allInvalid->getSourceRange());
+  
   // the tuple from the example
   SmallVector<Expr *, 2> subExprsRight({ one, two });
   SmallVector<Identifier, 2> subExprNamesRight(2, Identifier());
   auto rightInvalidTuple = TupleExpr::createImplicit(C.Ctx, subExprsRight, subExprNamesRight);
 
-  EXPECT_EQ(start, one->getStartLoc());
-  EXPECT_EQ(SourceLoc(), rightInvalidTuple->getStartLoc());
-  EXPECT_EQ(SourceLoc(), rightInvalidTuple->getEndLoc());
-  EXPECT_EQ(SourceRange(), rightInvalidTuple->getSourceRange());
+  EXPECT_EQ(start, rightInvalidTuple->getStartLoc());
+  EXPECT_EQ(start, rightInvalidTuple->getEndLoc());
+  EXPECT_EQ(SourceRange(start, start), rightInvalidTuple->getSourceRange());
 
   SmallVector<Expr *, 2> subExprsLeft({ two, one });
   SmallVector<Identifier, 2> subExprNamesLeft(2, Identifier());
   auto leftInvalidTuple = TupleExpr::createImplicit(C.Ctx, subExprsLeft, subExprNamesLeft);
 
-  EXPECT_EQ(start, one->getStartLoc());
-  EXPECT_EQ(SourceLoc(), leftInvalidTuple->getStartLoc());
-  EXPECT_EQ(SourceLoc(), leftInvalidTuple->getEndLoc());
-  EXPECT_EQ(SourceRange(), leftInvalidTuple->getSourceRange());
+  EXPECT_EQ(start, leftInvalidTuple->getStartLoc());
+  EXPECT_EQ(start, leftInvalidTuple->getEndLoc());
+  EXPECT_EQ(SourceRange(start, start), leftInvalidTuple->getSourceRange());
+  
+  // Some TupleExprs are triples. If only the middle expr has a valid SourceLoc
+  // then the TupleExpr's SourceLoc should point at that.
+  SmallVector<Expr *, 3> subExprsTriple({ two, one, two });
+  SmallVector<Identifier, 3> subExprNamesTriple(3, Identifier());
+  auto tripleValidMid = TupleExpr::createImplicit(C.Ctx, subExprsTriple, subExprNamesTriple);
+  EXPECT_EQ(start, tripleValidMid->getStartLoc());
+  EXPECT_EQ(start, tripleValidMid->getEndLoc());
+  EXPECT_EQ(SourceRange(start, start), tripleValidMid->getSourceRange());
+  
+  // Some TupleExprs are quadruples. Quadruples should point at the range from
+  // the first to the last valid exprs.
+  SmallVector<Expr *, 4> subExprsQuad({ one, two, four, three });
+  SmallVector<Identifier, 4> subExprNamesQuad(4, Identifier());
+  auto quadValidMids = TupleExpr::createImplicit(C.Ctx, subExprsQuad, subExprNamesQuad);
+  EXPECT_EQ(start, quadValidMids->getStartLoc());
+  EXPECT_EQ(start.getAdvancedLoc(4), quadValidMids->getEndLoc());
+  EXPECT_EQ(SourceRange(start, start.getAdvancedLoc(4)), quadValidMids->getSourceRange());
+  
 }