Merge pull request swiftlang#23730 from slavapestov/assorted-bug-fixes

Assorted bug fixes

Author: slavapestov

include/swift/AST/ASTWalker.h

@@ -211,6 +211,14 @@ class ASTWalker {
   /// However, ASTWalker does not walk into LazyInitializerExprs on its own.
   virtual bool shouldWalkIntoLazyInitializers() { return true; }
 
+  /// This method configures whether the walker should visit the body of a
+  /// non-single expression closure.
+  ///
+  /// For work that is performed for every top-level expression, this should
+  /// be overridden to return false, to avoid duplicating work or visiting
+  /// bodies of closures that have not yet been type checked.
+  virtual bool shouldWalkIntoNonSingleExpressionClosure() { return true; }
+
   /// walkToParameterListPre - This method is called when first visiting a
   /// ParameterList, before walking into its parameters.  If it returns false,
   /// the subtree is skipped.

include/swift/AST/DiagnosticsSema.def

@@ -2268,14 +2268,10 @@ ERROR(duplicate_inheritance,none,
   "duplicate inheritance from %0", (Type))
 WARNING(duplicate_anyobject_class_inheritance,none,
   "redundant inheritance from 'AnyObject' and Swift 3 'class' keyword", ())
-ERROR(objc_protocol_with_superclass,none,
-  "protocol %0 is '@objc' and cannot have a superclass constraint", (Identifier))
+ERROR(inheritance_from_protocol_with_superclass,none,
+  "inheritance from class-constrained protocol composition type %0", (Type))
 ERROR(multiple_inheritance,none,
   "multiple inheritance from classes %0 and %1", (Type, Type))
-ERROR(non_class_inheritance,none,
-  "non-class type %0 cannot inherit from class %1", (Type, Type))
-ERROR(extension_class_inheritance,none,
-  "extension of type %0 cannot inherit from class %1", (Type, Type))
 ERROR(inheritance_from_non_protocol_or_class,none,
   "inheritance from non-protocol, non-class type %0", (Type))
 ERROR(inheritance_from_non_protocol,none,

lib/AST/ASTWalker.cpp

@@ -755,6 +755,9 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
       return nullptr;
     }
 
+    if (!Walker.shouldWalkIntoNonSingleExpressionClosure())
+      return expr;
+
     // Handle other closures.
     if (BraceStmt *body = cast_or_null<BraceStmt>(doIt(expr->getBody()))) {
       expr->setBody(body, false);
@@ -1065,12 +1068,14 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
     if (auto oldSubExpr = E->getSubExpr()) {
       if (auto subExpr = doIt(oldSubExpr)) {
         E->setSubExpr(subExpr);
-      }
-      else {
+      } else {
         return nullptr;
       }
     }
 
+    if (!Walker.shouldWalkIntoNonSingleExpressionClosure())
+      return E;
+
     if (auto oldBody = E->getBody()) {
       if (auto body = doIt(oldBody)) {
         E->setBody(dyn_cast<BraceStmt>(body));

lib/IRGen/GenProto.cpp

@@ -1100,7 +1100,8 @@ class AccessorConformanceInfo : public ConformanceInfo {
   llvm::Value *getTable(IRGenFunction &IGF,
                         llvm::Value **typeMetadataCache) const override {
     // If we're looking up a dependent type, we can't cache the result.
-    if (Conformance->getType()->hasArchetype()) {
+    if (Conformance->getType()->hasArchetype() ||
+        Conformance->getType()->hasDynamicSelfType()) {
       return emitWitnessTableAccessorCall(IGF, Conformance,
                                           typeMetadataCache);
     }

lib/SILGen/SILGenConstructor.cpp

@@ -152,9 +152,7 @@ static void emitImplicitValueConstructor(SILGenFunction &SGF,
       if (!field->isStatic() && field->isLet() &&
           field->getParentInitializer()) {
 #ifndef NDEBUG
-        auto fieldTy = decl->getDeclContext()->mapTypeIntoContext(
-            field->getInterfaceType());
-        assert(fieldTy->isEqual(field->getParentInitializer()->getType())
+        assert(field->getType()->isEqual(field->getParentInitializer()->getType())
                && "Checked by sema");
 #endif
 

lib/Sema/CSApply.cpp

@@ -7315,8 +7315,7 @@ namespace {
         // Coerce the pattern, in case we resolved something.
         auto fnType = cs.getType(closure)->castTo<FunctionType>();
         auto *params = closure->getParameters();
-        if (tc.coerceParameterListToType(params, closure, fnType))
-          return { false, nullptr };
+        tc.coerceParameterListToType(params, closure, fnType);
 
         // Require layout of dependent types that could be used to materialize
         // metadata types/conformances during IRGen.

lib/Sema/CSDiag.cpp

@@ -5741,8 +5741,7 @@ bool FailureDiagnosis::diagnoseClosureExpr(
     }
 
     // Coerce parameter types here only if there are no unresolved
-    if (CS.TC.coerceParameterListToType(params, CE, fnType))
-      return true;
+    CS.TC.coerceParameterListToType(params, CE, fnType);
     expectedResultType = fnType->getResult();
   }
 

lib/Sema/CodeSynthesis.cpp

@@ -1690,6 +1690,25 @@ static void maybeAddMemberwiseDefaultArg(ParamDecl *arg, VarDecl *var,
   arg->setDefaultArgumentKind(DefaultArgumentKind::StoredProperty);
 }
 
+bool swift::isMemberwiseInitialized(VarDecl *var) {
+  // Implicit, computed, and static properties are not initialized.
+  // The exception is lazy properties, which due to batch mode we may or
+  // may not have yet finalized, so they may currently be "stored" or
+  // "computed" in the current AST state.
+  if (var->isImplicit() || var->isStatic())
+    return false;
+
+  if (!var->hasStorage() && !var->getAttrs().hasAttribute<LazyAttr>())
+    return false;
+
+  // Initialized 'let' properties have storage, but don't get an argument
+  // to the memberwise initializer since they already have an initial
+  // value that cannot be overridden.
+  if (var->isLet() && var->getParentInitializer())
+    return false;
+
+  return true;
+}
 /// Create an implicit struct or class constructor.
 ///
 /// \param decl The struct or class for which a constructor will be created.
@@ -1716,23 +1735,10 @@ ConstructorDecl *swift::createImplicitConstructor(TypeChecker &tc,
       auto var = dyn_cast<VarDecl>(member);
       if (!var)
         continue;
-      
-      // Implicit, computed, and static properties are not initialized.
-      // The exception is lazy properties, which due to batch mode we may or
-      // may not have yet finalized, so they may currently be "stored" or
-      // "computed" in the current AST state.
-      if (var->isImplicit() || var->isStatic())
-        continue;
 
-      if (!var->hasStorage() && !var->getAttrs().hasAttribute<LazyAttr>())
+      if (!isMemberwiseInitialized(var))
         continue;
 
-      // Initialized 'let' properties have storage, but don't get an argument
-      // to the memberwise initializer since they already have an initial
-      // value that cannot be overridden.
-      if (var->isLet() && var->getParentInitializer())
-        continue;
-      
       accessLevel = std::min(accessLevel, var->getFormalAccess());
 
       tc.validateDecl(var);
@@ -1765,9 +1771,9 @@ ConstructorDecl *swift::createImplicitConstructor(TypeChecker &tc,
   DeclName name(ctx, DeclBaseName::createConstructor(), paramList);
   auto *ctor =
     new (ctx) ConstructorDecl(name, Loc,
-                                  OTK_None, /*FailabilityLoc=*/SourceLoc(),
-                                  /*Throws=*/false, /*ThrowsLoc=*/SourceLoc(),
-                                  paramList, /*GenericParams=*/nullptr, decl);
+                              OTK_None, /*FailabilityLoc=*/SourceLoc(),
+                              /*Throws=*/false, /*ThrowsLoc=*/SourceLoc(),
+                              paramList, /*GenericParams=*/nullptr, decl);
 
   // Mark implicit.
   ctor->setImplicit();

lib/Sema/CodeSynthesis.h

@@ -69,6 +69,8 @@ enum class ImplicitConstructorKind {
   Memberwise
 };
 
+bool isMemberwiseInitialized(VarDecl *var);
+
 /// Create an implicit struct or class constructor.
 ///
 /// \param decl The struct or class for which a constructor will be created.

lib/Sema/MiscDiagnostics.cpp

@@ -111,6 +111,8 @@ static void diagSyntacticUseRestrictions(TypeChecker &TC, const Expr *E,
     bool walkToDeclPre(Decl *D) override { return false; }
     bool walkToTypeReprPre(TypeRepr *T) override { return true; }
 
+    bool shouldWalkIntoNonSingleExpressionClosure() override { return false; }
+
     std::pair<bool, Expr *> walkToExprPre(Expr *E) override {
       // See through implicit conversions of the expression.  We want to be able
       // to associate the parent of this expression with the ultimate callee.
@@ -582,7 +584,7 @@ static void diagSyntacticUseRestrictions(TypeChecker &TC, const Expr *E,
     void checkNoEscapeParameterUse(DeclRefExpr *DRE, Expr *parent,
                                    OperandKind useKind) {
       // This only cares about declarations of noescape function type.
-      auto AFT = DRE->getDecl()->getInterfaceType()->getAs<AnyFunctionType>();
+      auto AFT = DRE->getType()->getAs<FunctionType>();
       if (!AFT || !AFT->isNoEscape())
         return;
 
@@ -1399,6 +1401,8 @@ static void diagRecursivePropertyAccess(TypeChecker &TC, const Expr *E,
              cast<VarDecl>(DRE->getDecl())->isSelfParameter();
     }
 
+    bool shouldWalkIntoNonSingleExpressionClosure() override { return false; }
+
     std::pair<bool, Expr *> walkToExprPre(Expr *E) override {
       Expr *subExpr;
       bool isStore = false;
@@ -1547,11 +1551,10 @@ static void diagnoseImplicitSelfUseInClosure(TypeChecker &TC, const Expr *E,
       return false;
     }
 
+    bool shouldWalkIntoNonSingleExpressionClosure() override { return false; }
+
     std::pair<bool, Expr *> walkToExprPre(Expr *E) override {
       if (auto *CE = dyn_cast<AbstractClosureExpr>(E)) {
-        if (!CE->hasSingleExpressionBody())
-          return { false, E };
-
         // If this is a potentially-escaping closure expression, start looking
         // for references to self if we aren't already.
         if (isClosureRequiringSelfQualification(CE))
@@ -2348,7 +2351,7 @@ class VarDeclUsageChecker : public ASTWalker {
     // other things going on in the initializer expressions.
     return true;
   }
-  
+
   /// The heavy lifting happens when visiting expressions.
   std::pair<bool, Expr *> walkToExprPre(Expr *E) override;
 
@@ -2733,8 +2736,6 @@ void VarDeclUsageChecker::markStoredOrInOutExpr(Expr *E, unsigned Flags) {
   E->walk(*this);
 }
 
-   
-
 /// The heavy lifting happens when visiting expressions.
 std::pair<bool, Expr *> VarDeclUsageChecker::walkToExprPre(Expr *E) {
   // Sema leaves some subexpressions null, which seems really unfortunate.  It
@@ -2975,6 +2976,8 @@ static void checkStmtConditionTrailingClosure(TypeChecker &TC, const Expr *E) {
   public:
     DiagnoseWalker(TypeChecker &tc) : TC(tc) { }
 
+    bool shouldWalkIntoNonSingleExpressionClosure() override { return false; }
+
     std::pair<bool, Expr *> walkToExprPre(Expr *E) override {
       // Dig into implicit expression.
       if (E->isImplicit()) return { true, E };
@@ -3106,6 +3109,8 @@ class ObjCSelectorWalker : public ASTWalker {
   ObjCSelectorWalker(TypeChecker &tc, const DeclContext *dc, Type selectorTy)
     : TC(tc), DC(dc), SelectorTy(selectorTy) { }
 
+  bool shouldWalkIntoNonSingleExpressionClosure() override { return false; }
+
   std::pair<bool, Expr *> walkToExprPre(Expr *expr) override {
     auto *stringLiteral = dyn_cast<StringLiteralExpr>(expr);
     bool fromStringLiteral = false;
@@ -3777,14 +3782,12 @@ static void diagnoseUnintendedOptionalBehavior(TypeChecker &TC, const Expr *E,
       }
     }
 
+    bool shouldWalkIntoNonSingleExpressionClosure() override { return false; }
+
     std::pair<bool, Expr *> walkToExprPre(Expr *E) override {
       if (!E || isa<ErrorExpr>(E) || !E->getType())
         return { false, E };
 
-      if (auto *CE = dyn_cast<AbstractClosureExpr>(E))
-        if (!CE->hasSingleExpressionBody())
-          return { false, E };
-
       if (IgnoredExprs.count(E))
         return { true, E };
 
@@ -3851,6 +3854,8 @@ static void diagnoseDeprecatedWritableKeyPath(TypeChecker &TC, const Expr *E,
       }
     }
 
+    bool shouldWalkIntoNonSingleExpressionClosure() override { return false; }
+
     std::pair<bool, Expr *> walkToExprPre(Expr *E) override {
       if (!E || isa<ErrorExpr>(E) || !E->getType())
         return {false, E};