Move unsupported super use diagnosis form Parse to Sema

Author: AnthonyLatsis

include/swift/AST/Decl.h

@@ -6689,6 +6689,13 @@ class ParamDecl : public VarDecl {
                  DeclContext *Parent,
                  ParamSpecifier specifier = ParamSpecifier::Default);
 
+  static ParamDecl *createParsed(ASTContext &Context, SourceLoc specifierLoc,
+                                 SourceLoc argumentNameLoc,
+                                 Identifier argumentName,
+                                 SourceLoc parameterNameLoc,
+                                 Identifier parameterName, Expr *defaultValue,
+                                 DeclContext *dc);
+
   /// Retrieve the argument (API) name for this function parameter.
   Identifier getArgumentName() const {
     return ArgumentNameAndFlags.getPointer();

include/swift/AST/DefaultArgumentKind.h

@@ -27,8 +27,6 @@ class StringRef;
 
 namespace swift {
 
-class Expr;
-
 /// Describes the kind of default argument a tuple pattern element has.
 enum class DefaultArgumentKind : uint8_t {
   /// No default argument.
@@ -56,11 +54,6 @@ enum class DefaultArgumentKind : uint8_t {
 };
 enum { NumDefaultArgumentKindBits = 4 };
 
-/// Determine the kind of a default argument given a parsed expression that has
-/// not yet been type-checked.
-/// FIXME: Requestify/internalize the computation of the default arg expr and its kind (given a parsed expr) once the old parser no longer needs this.
-DefaultArgumentKind getDefaultArgKind(Expr *init);
-
 struct ArgumentAttrs {
   DefaultArgumentKind argumentKind;
   bool isUnavailableInSwift = false;

include/swift/AST/DiagnosticsParse.def

@@ -1344,8 +1344,6 @@ ERROR(expected_identifier_after_dot_expr,none,
 ERROR(expected_identifier_after_super_dot_expr,
       PointsToFirstBadToken,
       "expected identifier or 'init' after super '.' expression", ())
-ERROR(expected_dot_or_subscript_after_super,PointsToFirstBadToken,
-      "expected '.' or '[' after 'super'", ())
 
 WARNING(await_before_try,none, "'try' must precede 'await'", ())
 

include/swift/AST/DiagnosticsSema.def

@@ -4317,6 +4317,9 @@ ERROR(no_member_of_module,none,
 
 // 'super'.
 
+ERROR(super_invalid_parent_expr,none,
+      "'super' may be used only to access a superclass member, subscript, or "
+      "initializer", ())
 ERROR(super_no_superclass,none,
       "'super' cannot be used in %select{|extension of }0class %1 because it "
       "has no superclass",

include/swift/Parse/Parser.h

@@ -1593,9 +1593,6 @@ class Parser {
     /// The default argument for this parameter.
     Expr *DefaultArg = nullptr;
 
-    /// True if this parameter inherits a default argument via '= super'
-    bool hasInheritedDefaultArg = false;
-    
     /// True if we emitted a parse error about this parameter.
     bool isInvalid = false;
 

lib/AST/ASTBridging.cpp

@@ -840,22 +840,10 @@ BridgedParamDecl BridgedParamDecl_createParsed(
     BridgedSourceLoc cArgNameLoc, BridgedIdentifier cParamName,
     BridgedSourceLoc cParamNameLoc, BridgedNullableTypeRepr opaqueType,
     BridgedNullableExpr opaqueDefaultValue) {
-  auto *declContext = cDeclContext.unbridged();
-
-  auto *defaultValue = opaqueDefaultValue.unbridged();
-  DefaultArgumentKind defaultArgumentKind;
-
-  if (declContext->getParentSourceFile()->Kind == SourceFileKind::Interface &&
-      isa<SuperRefExpr>(defaultValue)) {
-    defaultValue = nullptr;
-    defaultArgumentKind = DefaultArgumentKind::Inherited;
-  } else {
-    defaultArgumentKind = getDefaultArgKind(defaultValue);
-  }
-
-  auto *paramDecl = new (cContext.unbridged()) ParamDecl(
-      cSpecifierLoc.unbridged(), cArgNameLoc.unbridged(), cArgName.unbridged(),
-      cParamNameLoc.unbridged(), cParamName.unbridged(), declContext);
+  auto *paramDecl = ParamDecl::createParsed(
+      cContext.unbridged(), cSpecifierLoc.unbridged(), cArgNameLoc.unbridged(),
+      cArgName.unbridged(), cParamNameLoc.unbridged(), cParamName.unbridged(),
+      opaqueDefaultValue.unbridged(), cDeclContext.unbridged());
 
   if (auto type = opaqueType.unbridged()) {
     paramDecl->setTypeRepr(type);
@@ -899,9 +887,6 @@ BridgedParamDecl BridgedParamDecl_createParsed(
     }
   }
 
-  paramDecl->setDefaultExpr(defaultValue, /*isTypeChecked*/ false);
-  paramDecl->setDefaultArgumentKind(defaultArgumentKind);
-
   return paramDecl;
 }
 

lib/AST/Decl.cpp

@@ -8301,7 +8301,9 @@ ParamDecl *ParamDecl::createImplicit(ASTContext &Context,
                                    interfaceType, Parent, specifier);
 }
 
-DefaultArgumentKind swift::getDefaultArgKind(Expr *init) {
+/// Determine the kind of a default argument for the given expression.
+static DefaultArgumentKind computeDefaultArgumentKind(DeclContext *dc,
+                                                      Expr *init) {
   if (!init)
     return DefaultArgumentKind::None;
 
@@ -8314,6 +8316,19 @@ DefaultArgumentKind swift::getDefaultArgKind(Expr *init) {
   if (isa<MacroExpansionExpr>(init))
     return DefaultArgumentKind::ExpressionMacro;
 
+  if (isa<SuperRefExpr>(init)) {
+    // The compiler does not synthesize inherited initializers when
+    // type-checking Swift module interfaces. Instead, module interfaces are
+    // expected to include them explicitly in subclasses. A default argument of
+    // '= super' in a parameter of such initializer indicates that the default
+    // argument is inherited.
+    if (dc->getParentSourceFile()->Kind == SourceFileKind::Interface) {
+      return DefaultArgumentKind::Inherited;
+    } else {
+      return DefaultArgumentKind::Normal;
+    }
+  }
+
   auto magic = dyn_cast<MagicIdentifierLiteralExpr>(init);
   if (!magic)
     return DefaultArgumentKind::Normal;
@@ -8328,6 +8343,30 @@ DefaultArgumentKind swift::getDefaultArgKind(Expr *init) {
   llvm_unreachable("Unhandled MagicIdentifierLiteralExpr in switch.");
 }
 
+ParamDecl *ParamDecl::createParsed(ASTContext &Context, SourceLoc specifierLoc,
+                                   SourceLoc argumentNameLoc,
+                                   Identifier argumentName,
+                                   SourceLoc parameterNameLoc,
+                                   Identifier parameterName, Expr *defaultValue,
+                                   DeclContext *dc) {
+  auto *decl =
+      new (Context) ParamDecl(specifierLoc, argumentNameLoc, argumentName,
+                              parameterNameLoc, parameterName, dc);
+
+  const auto kind = computeDefaultArgumentKind(dc, defaultValue);
+  if (kind == DefaultArgumentKind::Inherited) {
+    // The 'super' in inherited default arguments is a specifier rather than an
+    // expression.
+    // TODO: However, we may want to retain its location for diagnostics.
+    defaultValue = nullptr;
+  }
+
+  decl->setDefaultExpr(defaultValue, false);
+  decl->setDefaultArgumentKind(kind);
+
+  return decl;
+}
+
 /// Retrieve the type of 'self' for the given context.
 Type DeclContext::getSelfTypeInContext() const {
   return mapTypeIntoContext(getSelfInterfaceType());

lib/Parse/ParseExpr.cpp

@@ -1031,28 +1031,11 @@ void Parser::tryLexRegexLiteral(bool forUnappliedOperator) {
 /// parseExprSuper
 ///
 ///   expr-super:
-///     expr-super-member
-///     expr-super-init
-///     expr-super-subscript
-///   expr-super-member:
-///     'super' '.' identifier
-///   expr-super-init:
-///     'super' '.' 'init'
-///   expr-super-subscript:
-///     'super' '[' expr ']'
+///     'super'
 ParserResult<Expr> Parser::parseExprSuper() {
   // Parse the 'super' reference.
   SourceLoc superLoc = consumeToken(tok::kw_super);
 
-  // 'super.' must be followed by a member ref, explicit initializer ref, or
-  // subscript call.
-  if (!Tok.isAny(tok::period, tok::period_prefix, tok::code_complete) &&
-      !Tok.isFollowingLSquare()) {
-    if (!consumeIf(tok::unknown))
-      diagnose(Tok, diag::expected_dot_or_subscript_after_super);
-    return nullptr;
-  }
-
   return makeParserResult(new (Context) SuperRefExpr(/*selfDecl=*/nullptr,
                                                      superLoc,
                                                      /*Implicit=*/false));

lib/Parse/ParsePattern.cpp

@@ -37,31 +37,14 @@ void Parser::DefaultArgumentInfo::setFunctionContext(
   }
 }
 
-static ParserStatus parseDefaultArgument(
-    Parser &P, Parser::DefaultArgumentInfo *defaultArgs, unsigned argIndex,
-    Expr *&init, bool &hasInheritedDefaultArg,
-    Parser::ParameterContextKind paramContext) {
+static ParserStatus
+parseDefaultArgument(Parser &P, Parser::DefaultArgumentInfo *defaultArgs,
+                     unsigned argIndex, Expr *&init,
+                     Parser::ParameterContextKind paramContext) {
   assert(P.Tok.is(tok::equal) ||
        (P.Tok.isBinaryOperator() && P.Tok.getText() == "=="));
   SourceLoc equalLoc = P.consumeToken();
 
-  if (P.SF.Kind == SourceFileKind::Interface) {
-    // Swift module interfaces don't synthesize inherited initializers and
-    // instead include them explicitly in subclasses. Since the
-    // \c DefaultArgumentKind of these initializers is \c Inherited, this is
-    // represented textually as `= super` in the interface.
-
-    // If we're in a module interface and the default argument is exactly
-    // `super` (i.e. the token after that is `,` or `)` which end a parameter)
-    // report an inherited default argument to the caller and return.
-    if (P.Tok.is(tok::kw_super) && P.peekToken().isAny(tok::comma, tok::r_paren)) {
-      hasInheritedDefaultArg = true;
-      P.consumeToken(tok::kw_super);
-      defaultArgs->HasDefaultArgument = true;
-      return ParserStatus();
-    }
-  }
-
   // Enter a fresh default-argument context with a meaningless parent.
   // We'll change the parent to the function later after we've created
   // that declaration.
@@ -483,9 +466,8 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
             .fixItReplace(EqualLoc, "=");
       }
 
-      status |= parseDefaultArgument(
-          *this, defaultArgs, defaultArgIndex, param.DefaultArg,
-          param.hasInheritedDefaultArg, paramContext);
+      status |= parseDefaultArgument(*this, defaultArgs, defaultArgIndex,
+                                     param.DefaultArg, paramContext);
     }
 
     // If we haven't made progress, don't add the parameter.
@@ -547,10 +529,9 @@ mapParsedParameters(Parser &parser,
                          Identifier argName, SourceLoc argNameLoc,
                          Identifier paramName, SourceLoc paramNameLoc)
   -> ParamDecl * {
-    auto param = new (ctx) ParamDecl(paramInfo.SpecifierLoc,
-                                     argNameLoc, argName,
-                                     paramNameLoc, paramName,
-                                     parser.CurDeclContext);
+    auto param = ParamDecl::createParsed(
+        ctx, paramInfo.SpecifierLoc, argNameLoc, argName, paramNameLoc,
+        paramName, paramInfo.DefaultArg, parser.CurDeclContext);
     param->getAttrs() = paramInfo.Attrs;
 
     bool parsingEnumElt
@@ -713,8 +694,7 @@ mapParsedParameters(Parser &parser,
                            param.FirstName, param.FirstNameLoc);
     }
 
-    assert (((!param.DefaultArg &&
-              !param.hasInheritedDefaultArg) ||
+    assert ((!param.DefaultArg ||
              paramContext == Parser::ParameterContextKind::Function ||
              paramContext == Parser::ParameterContextKind::Operator ||
              paramContext == Parser::ParameterContextKind::Initializer ||
@@ -723,14 +703,6 @@ mapParsedParameters(Parser &parser,
              paramContext == Parser::ParameterContextKind::Macro) &&
             "Default arguments are only permitted on the first param clause");
 
-    if (param.DefaultArg) {
-      DefaultArgumentKind kind = getDefaultArgKind(param.DefaultArg);
-      result->setDefaultArgumentKind(kind);
-      result->setDefaultExpr(param.DefaultArg, /*isTypeChecked*/ false);
-    } else if (param.hasInheritedDefaultArg) {
-      result->setDefaultArgumentKind(DefaultArgumentKind::Inherited);
-    }
-
     elements.push_back(result);
 
     if (argNames)

lib/Sema/PreCheckExpr.cpp

@@ -1106,6 +1106,8 @@ namespace {
     VarDecl *getImplicitSelfDeclForSuperContext(SourceLoc Loc) ;
 
     PreWalkResult<Expr *> walkToExprPre(Expr *expr) override {
+      auto &diags = Ctx.Diags;
+
       // FIXME(diagnostics): `InOutType` could appear here as a result
       // of successful re-typecheck of the one of the sub-expressions e.g.
       // `let _: Int = { (s: inout S) in s.bar() }`. On the first
@@ -1148,11 +1150,39 @@ namespace {
       // Resolve 'super' references.
       if (auto *superRef = dyn_cast<SuperRefExpr>(expr)) {
         auto loc = superRef->getLoc();
+
         auto *selfDecl = getImplicitSelfDeclForSuperContext(loc);
         if (selfDecl == nullptr)
-          return finish(true, new (Ctx) ErrorExpr(loc));
+          return finish(false, new (Ctx) ErrorExpr(loc));
 
         superRef->setSelf(selfDecl);
+
+        const bool isValidSuper = [&]() -> bool {
+          auto *parentExpr = Parent.getAsExpr();
+          if (!parentExpr) {
+            return false;
+          }
+
+          if (isa<UnresolvedDotExpr>(parentExpr) ||
+              isa<MemberRefExpr>(parentExpr)) {
+            return true;
+          } else if (auto *SE = dyn_cast<SubscriptExpr>(parentExpr)) {
+            // 'super[]' is valid, but 'x[super]' is not.
+            return superRef == SE->getBase();
+          }
+
+          return false;
+        }();
+
+        // NB: This is done along the happy path because presenting this error
+        // in a context where 'super' is not legal to begin with is not helpful.
+        if (!isValidSuper) {
+          // Diagnose and keep going. It is important for source tooling such
+          // as code completion that Sema is able to provide type information
+          // for 'super' in arbitrary positions inside expressions.
+          diags.diagnose(loc, diag::super_invalid_parent_expr);
+        }
+
         return finish(true, superRef);
       }
 
@@ -1193,8 +1223,8 @@ namespace {
             auto *DRE = cast<DeclRefExpr>(refExpr);
             if (accessor->getImplicitSelfDecl() == DRE->getDecl() &&
                 !isa_and_nonnull<UnresolvedDotExpr>(Parent.getAsExpr())) {
-              Ctx.Diags.diagnose(unresolved->getLoc(),
-                                 diag::invalid_use_of_self_in_init_accessor);
+              diags.diagnose(unresolved->getLoc(),
+                             diag::invalid_use_of_self_in_init_accessor);
               refExpr = new (Ctx) ErrorExpr(unresolved->getSourceRange());
             }
           }
@@ -1240,24 +1270,21 @@ namespace {
             // a member reference, it might be valid to have `&`
             // before all of the parens.
             if (lastInnerParenLoc.isValid()) {
-              auto &DE = getASTContext().Diags;
-              auto diag = DE.diagnose(expr->getStartLoc(),
-                                      diag::extraneous_address_of);
+              auto diag = diags.diagnose(expr->getStartLoc(),
+                                         diag::extraneous_address_of);
               diag.fixItExchange(expr->getLoc(), lastInnerParenLoc);
             }
             return finish(true, expr);
           }
 
           if (isa<SubscriptExpr>(parent)) {
-            getASTContext().Diags.diagnose(
-                expr->getStartLoc(),
-                diag::cannot_pass_inout_arg_to_subscript);
+            diags.diagnose(expr->getStartLoc(),
+                           diag::cannot_pass_inout_arg_to_subscript);
             return finish(false, nullptr);
           }
         }
 
-        getASTContext().Diags.diagnose(expr->getStartLoc(),
-                                       diag::extraneous_address_of);
+        diags.diagnose(expr->getStartLoc(), diag::extraneous_address_of);
         return finish(false, nullptr);
       }