Merge pull request #21343 from slavapestov/one-element-tuple

Ban one element tuples harder

Author: slavapestov

lib/Sema/CSApply.cpp

@@ -381,14 +381,15 @@ static Expr *buildDynamicMemberLookupIndexExpr(StringRef name, Type ty,
   // Build and type check the string literal index value to the specific
   // string type expected by the subscript.
   Expr *nameExpr = new (ctx) StringLiteralExpr(name, loc, /*implicit*/true);
+  (void)cs.TC.typeCheckExpression(nameExpr, dc);
+  cs.cacheExprTypes(nameExpr);
 
   // Build a tuple so that the argument has a label.
   Expr *tuple = TupleExpr::create(ctx, loc, nameExpr, ctx.Id_dynamicMember,
                                   loc, loc, /*hasTrailingClosure*/false,
                                   /*implicit*/true);
-  (void)cs.TC.typeCheckExpression(tuple, dc, TypeLoc::withoutLoc(ty),
-                                  CTP_CallArgument);
-  cs.cacheExprTypes(tuple);
+  cs.setType(tuple, ty);
+  tuple->setType(ty);
   return tuple;
 }
 
@@ -4410,13 +4411,17 @@ namespace {
             auto loc = origComponent.getLoc();
             auto fieldName =
                 foundDecl->choice.getName().getBaseIdentifier().str();
-            auto index = buildDynamicMemberLookupIndexExpr(fieldName, indexType,
-                                                           loc, dc, cs);
-            
+
+            Expr *nameExpr = new (ctx) StringLiteralExpr(fieldName, loc,
+                                                         /*implicit*/true);
+            (void)cs.TC.typeCheckExpression(nameExpr, dc);
+            cs.cacheExprTypes(nameExpr);
+
             origComponent = KeyPathExpr::Component::
-              forUnresolvedSubscript(ctx, loc, index, {}, loc, loc,
-                                     /*trailingClosure*/nullptr);
-            cs.setType(origComponent.getIndexExpr(), index->getType());
+              forUnresolvedSubscript(ctx, loc,
+                                     {nameExpr}, {ctx.Id_dynamicMember}, {loc},
+                                     loc, /*trailingClosure*/nullptr);
+            cs.setType(origComponent.getIndexExpr(), indexType);
           }
 
           auto subscriptType =

lib/Sema/MiscDiagnostics.cpp

@@ -203,10 +203,6 @@ static void diagSyntacticUseRestrictions(TypeChecker &TC, const Expr *E,
       while (auto Conv = dyn_cast<ImplicitConversionExpr>(Base))
         Base = Conv->getSubExpr();
 
-      // Record call arguments.
-      if (auto Call = dyn_cast<CallExpr>(Base))
-        CallArgs.insert(Call->getArg());
-
       if (auto *DRE = dyn_cast<DeclRefExpr>(Base)) {
         // Verify metatype uses.
         if (isa<TypeDecl>(DRE->getDecl())) {
@@ -235,7 +231,14 @@ static void diagSyntacticUseRestrictions(TypeChecker &TC, const Expr *E,
       if (isa<TypeExpr>(Base))
         checkUseOfMetaTypeName(Base);
 
+      if (auto *TSE = dyn_cast<TupleShuffleExpr>(E)) {
+        if (CallArgs.count(TSE))
+          CallArgs.insert(TSE->getSubExpr());
+      }
+
       if (auto *SE = dyn_cast<SubscriptExpr>(E)) {
+        CallArgs.insert(SE->getIndex());
+
         // Implicit InOutExpr's are allowed in the base of a subscript expr.
         if (auto *IOE = dyn_cast<InOutExpr>(SE->getBase()))
           if (IOE->isImplicit())
@@ -248,6 +251,13 @@ static void diagSyntacticUseRestrictions(TypeChecker &TC, const Expr *E,
         });
       }
 
+      if (auto *KPE = dyn_cast<KeyPathExpr>(E)) {
+        for (auto Comp : KPE->getComponents()) {
+          if (auto *Arg = Comp.getIndexExpr())
+            CallArgs.insert(Arg);
+        }
+      }
+
       if (auto *AE = dyn_cast<CollectionExpr>(E)) {
         visitCollectionElements(AE, [&](unsigned argIndex, Expr *arg) {
           arg = lookThroughArgument(arg);
@@ -266,6 +276,9 @@ static void diagSyntacticUseRestrictions(TypeChecker &TC, const Expr *E,
       // Check function calls, looking through implicit conversions on the
       // function and inspecting the arguments directly.
       if (auto *Call = dyn_cast<ApplyExpr>(E)) {
+        // Record call arguments.
+        CallArgs.insert(Call->getArg());
+
         // Warn about surprising implicit optional promotions.
         checkOptionalPromotions(Call);
 
@@ -381,6 +394,18 @@ static void diagSyntacticUseRestrictions(TypeChecker &TC, const Expr *E,
         }
       }
 
+      // Diagnose single-element tuple expressions.
+      if (auto *tupleExpr = dyn_cast<TupleExpr>(E)) {
+        if (!CallArgs.count(tupleExpr)) {
+          if (tupleExpr->getNumElements() == 1) {
+            TC.diagnose(tupleExpr->getElementNameLoc(0),
+                        diag::tuple_single_element)
+              .fixItRemoveChars(tupleExpr->getElementNameLoc(0),
+                                tupleExpr->getElement(0)->getStartLoc());
+          }
+        }
+      }
+
       return { true, E };
     }
 

lib/Sema/TypeCheckGeneric.cpp

@@ -710,11 +710,11 @@ GenericEnvironment *TypeChecker::checkGenericEnvironment(
     if (recursivelyVisitGenericParams) {
       visitOuterToInner(genericParams,
                         [&](GenericParamList *gpList) {
-      auto genericParamsDC = gpList->begin()[0]->getDeclContext();
-      TypeResolution structuralResolution =
-        TypeResolution::forStructural(genericParamsDC);
-        checkGenericParamList(*this, &builder, gpList, nullptr,
-                              structuralResolution);
+        auto genericParamsDC = gpList->begin()[0]->getDeclContext();
+        TypeResolution structuralResolution =
+          TypeResolution::forStructural(genericParamsDC);
+          checkGenericParamList(*this, &builder, gpList, nullptr,
+                                structuralResolution);
       });
     } else {
       auto genericParamsDC = genericParams->begin()[0]->getDeclContext();

test/Constraints/tuple.swift

@@ -56,7 +56,7 @@ f5((1,1))
 // Tuples with existentials
 var any : Any = ()
 any = (1, 2)
-any = (label: 4)
+any = (label: 4) // expected-error {{cannot create a single-element tuple with an element label}}
 
 // Scalars don't have .0/.1/etc
 i = j.0 // expected-error{{value of type 'Int' has no member '0'}}
@@ -252,3 +252,11 @@ func f(b: Bool) -> (a: Int, b: String)? {
   let y = ""
   return b ? (x, y) : nil
 }
+
+// Single element tuple expressions
+func singleElementTuple() {
+  let _ = (label: 123) // expected-error {{cannot create a single-element tuple with an element label}} {{12-19=}}
+  let _ = (label: 123).label // expected-error {{cannot create a single-element tuple with an element label}} {{12-19=}}
+  let _ = ((label: 123)) // expected-error {{cannot create a single-element tuple with an element label}} {{13-20=}}
+  let _ = ((label: 123)).label // expected-error {{cannot create a single-element tuple with an element label}} {{13-20=}}
+}

test/decl/enum/enumtest.swift

@@ -164,9 +164,9 @@ func test5(_ myorigin: CGPoint) {
   // Dot syntax.
   _ = x2.origin.x
   _ = x1.size.area()
-  _ = (r : x1.size).r.area()
+  _ = (r : x1.size).r.area() // expected-error {{cannot create a single-element tuple with an element label}}
   _ = x1.size.area()
-  _ = (r : x1.size).r.area()
+  _ = (r : x1.size).r.area() // expected-error {{cannot create a single-element tuple with an element label}}
 
   _ = x1.area
 

validation-test/Sema/type_checker_perf/fast/rdar21720888.swift.gyb

@@ -4,6 +4,6 @@
 
 _ = [
 %for i in range(0, N):
-  (label: "string"),
+  (label: "string", another: 123),
 %end
 ]