Merge pull request #4042 from slavapestov/sundry-sema-fixes

Author: swift-ci

include/swift/AST/Decl.h

@@ -288,11 +288,6 @@ class alignas(1 << DeclAlignInBits) Decl {
     /// once (either in its declaration, or once later), making it immutable.
     unsigned IsLet : 1;
 
-    /// \brief Whether this is an 'inout' parameter; this is preferable
-    /// to checking if the parameter type is an InOutType, because invalid
-    /// inout parameters have an error type.
-    unsigned IsInOut : 1;
-
     /// \brief Whether this vardecl has an initial value bound to it in a way
     /// that isn't represented in the AST with an initializer in the pattern
     /// binding.  This happens in cases like "for i in ...", switch cases, etc.
@@ -306,7 +301,7 @@ class alignas(1 << DeclAlignInBits) Decl {
     /// a.storage for lazy var a is a decl that cannot be accessed.
     unsigned IsUserAccessible : 1;
   };
-  enum { NumVarDeclBits = NumAbstractStorageDeclBits + 6 };
+  enum { NumVarDeclBits = NumAbstractStorageDeclBits + 5 };
   static_assert(NumVarDeclBits <= 32, "fits in an unsigned");
 
   class EnumElementDeclBitfields {
@@ -4250,7 +4245,6 @@ class VarDecl : public AbstractStorageDecl {
     VarDeclBits.IsUserAccessible = true;
     VarDeclBits.IsStatic = IsStatic;
     VarDeclBits.IsLet = IsLet;
-    VarDeclBits.IsInOut = false;
     VarDeclBits.IsDebuggerVar = false;
     VarDeclBits.HasNonPatternBindingInit = false;
     setType(Ty);
@@ -4348,10 +4342,6 @@ class VarDecl : public AbstractStorageDecl {
   bool isLet() const { return VarDeclBits.IsLet; }
   void setLet(bool IsLet) { VarDeclBits.IsLet = IsLet; }
 
-  /// Is this an 'inout' parameter?
-  bool isInOut() const { return VarDeclBits.IsInOut; }
-  void setInOut(bool InOut) { VarDeclBits.IsInOut = InOut; }
-
   /// Return true if this vardecl has an initial value bound to it in a way
   /// that isn't represented in the AST with an initializer in the pattern
   /// binding.  This happens in cases like "for i in ...", switch cases, etc.

include/swift/AST/DiagnosticsSema.def

@@ -96,7 +96,7 @@ ERROR(could_not_use_type_member_on_instance,none,
 ERROR(could_not_use_enum_element_on_instance,none,
       "enum element %0 cannot be referenced as an instance member",
       (DeclName))
-ERROR(could_not_use_type_member_on_existential,none,
+ERROR(could_not_use_type_member_on_protocol_metatype,none,
       "static member %1 cannot be used on protocol metatype %0",
       (Type, DeclName))
 ERROR(could_not_use_instance_member_on_type,none,

lib/AST/NameLookup.cpp

@@ -556,15 +556,8 @@ UnqualifiedLookup::UnqualifiedLookup(DeclName Name, DeclContext *DC,
 
         SmallVector<ValueDecl *, 4> Lookup;
         DC->lookupQualified(ExtendedType, Name, options, TypeResolver, Lookup);
-        bool isMetatypeType = ExtendedType->is<AnyMetatypeType>();
         bool FoundAny = false;
         for (auto Result : Lookup) {
-          // If we're looking into an instance, skip static functions.
-          if (!isMetatypeType &&
-              isa<FuncDecl>(Result) &&
-              cast<FuncDecl>(Result)->isStatic())
-            continue;
-
           // Classify this declaration.
           FoundAny = true;
 
@@ -575,12 +568,6 @@ UnqualifiedLookup::UnqualifiedLookup(DeclName Name, DeclContext *DC,
             else
               Results.push_back(UnqualifiedLookupResult(MetaBaseDecl, Result));
             continue;
-          } else if (auto FD = dyn_cast<FuncDecl>(Result)) {
-            if (FD->isStatic() && !isMetatypeType)
-              continue;
-          } else if (isa<EnumElementDecl>(Result)) {
-            Results.push_back(UnqualifiedLookupResult(BaseDecl, Result));
-            continue;
           }
 
           Results.push_back(UnqualifiedLookupResult(BaseDecl, Result));

lib/Parse/ParsePattern.cpp

@@ -387,10 +387,8 @@ mapParsedParameters(Parser &parser,
     // If a type was provided, create the type for the parameter.
     if (auto type = paramInfo.Type) {
       // If 'inout' was specified, turn the type into an in-out type.
-      if (specifierKind == Parser::ParsedParameter::InOut) {
+      if (specifierKind == Parser::ParsedParameter::InOut)
         type = new (ctx) InOutTypeRepr(type, paramInfo.LetVarInOutLoc);
-        param->setInOut(true);
-      }
 
       param->getTypeLoc() = TypeLoc(type);
     } else if (paramContext != Parser::ParameterContextKind::Closure) {

lib/Sema/CSApply.cpp

@@ -3388,7 +3388,13 @@ namespace {
     }
 
     Expr *visitUnresolvedPatternExpr(UnresolvedPatternExpr *expr) {
-      llvm_unreachable("should have been eliminated during name binding");
+      // If we end up here, we should have diagnosed somewhere else
+      // already.
+      if (!SuppressDiagnostics) {
+        cs.TC.diagnose(expr->getLoc(), diag::pattern_in_expr,
+                       expr->getSubPattern()->getKind());
+      }
+      return expr;
     }
 
     Expr *visitBindOptionalExpr(BindOptionalExpr *expr) {

lib/Sema/CSDiag.cpp

@@ -1948,10 +1948,13 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
   /// the exact expression kind).
   bool diagnoseGeneralMemberFailure(Constraint *constraint);
 
-  /// Diagnose the lookup of an enum element as instance member where only a
-  /// static member is allowed
-  void diagnoseEnumInstanceMemberLookup(EnumElementDecl *enumElementDecl,
-                                        SourceLoc loc);
+  /// Diagnose the lookup of an static member or enum element as instance member.
+  void diagnoseTypeMemberOnInstanceLookup(Type baseObjTy,
+                                          Expr *baseExpr,
+                                          DeclName memberName,
+                                          DeclNameLoc nameLoc,
+                                          ValueDecl *member,
+                                          SourceLoc loc);
 
   /// Given a result of name lookup that had no viable results, diagnose the
   /// unviable ones.
@@ -2338,12 +2341,54 @@ bool FailureDiagnosis::diagnoseGeneralMemberFailure(Constraint *constraint) {
 }
 
 void FailureDiagnosis::
-diagnoseEnumInstanceMemberLookup(EnumElementDecl *enumElementDecl,
-                                 SourceLoc loc) {
-  auto diag = diagnose(loc, diag::could_not_use_enum_element_on_instance,
-                       enumElementDecl->getName());
-  auto parentEnum = enumElementDecl->getParentEnum();
-  auto enumMetatype = parentEnum->getType()->castTo<AnyMetatypeType>();
+diagnoseTypeMemberOnInstanceLookup(Type baseObjTy,
+                                   Expr *baseExpr,
+                                   DeclName memberName,
+                                   DeclNameLoc nameLoc,
+                                   ValueDecl *member,
+                                   SourceLoc loc) {
+  SourceRange baseRange = baseExpr ? baseExpr->getSourceRange() : SourceRange();
+
+  // If the base of the lookup is a protocol metatype, suggest
+  // to replace the metatype with 'Self'
+  // error saying the lookup cannot be on a protocol metatype
+  if (auto metatypeTy = baseObjTy->getAs<MetatypeType>()) {
+    auto Diag = diagnose(loc,
+                         diag::could_not_use_type_member_on_protocol_metatype,
+                         baseObjTy, memberName);
+    Diag.highlight(baseRange).highlight(nameLoc.getSourceRange());
+
+    // See through function decl context
+    if (auto parent = CS->DC->getInnermostTypeContext()) {
+      // If we are in a protocol extension of 'Proto' and we see
+      // 'Proto.static', suggest 'Self.static'
+      if (auto extensionContext = parent->getAsProtocolExtensionContext()) {
+        if (extensionContext->getDeclaredType()->getCanonicalType()
+            == metatypeTy->getInstanceType()->getCanonicalType()) {
+          Diag.fixItReplace(baseRange, "Self");
+        }
+      }
+    }
+
+    return;
+  }
+
+  // Otherwise the static member lookup was invalid because it was
+  // called on an instance
+  Optional<InFlightDiagnostic> Diag;
+
+  if (isa<EnumElementDecl>(member))
+    Diag.emplace(diagnose(loc, diag::could_not_use_enum_element_on_instance,
+                          memberName));
+  else
+    Diag.emplace(diagnose(loc, diag::could_not_use_type_member_on_instance,
+                          baseObjTy, memberName));
+
+  Diag->highlight(nameLoc.getSourceRange());
+
+  // No fix-it if the lookup was qualified
+  if (baseExpr && !baseExpr->isImplicit())
+    return;
 
   // Determine the contextual type of the expression
   Type contextualType;
@@ -2355,8 +2400,8 @@ diagnoseEnumInstanceMemberLookup(EnumElementDecl *enumElementDecl,
 
   // Try to provide a fix-it that only contains a '.'
   if (contextualType) {
-    if (enumMetatype->getInstanceType()->isEqual(contextualType)) {
-      diag.fixItInsert(loc, ".");
+    if (baseObjTy->isEqual(contextualType)) {
+      Diag->fixItInsert(loc, ".");
       return;
     }
   }
@@ -2382,8 +2427,8 @@ diagnoseEnumInstanceMemberLookup(EnumElementDecl *enumElementDecl,
           // If the rhs of '~=' is the enum type, a single dot suffixes
           // since the type can be inferred
           Type secondArgType = binaryExpr->getArg()->getElement(1)->getType();
-          if (secondArgType->isEqual(enumMetatype->getInstanceType())) {
-            diag.fixItInsert(loc, ".");
+          if (secondArgType->isEqual(baseObjTy)) {
+            Diag->fixItInsert(loc, ".");
             return;
           }
         }
@@ -2392,12 +2437,14 @@ diagnoseEnumInstanceMemberLookup(EnumElementDecl *enumElementDecl,
   }
 
   // Fall back to a fix-it with a full type qualifier
-  SmallString<32> enumTypeName;
-  llvm::raw_svector_ostream typeNameStream(enumTypeName);
-  typeNameStream << parentEnum->getName();
-  typeNameStream << ".";
-
-  diag.fixItInsert(loc, typeNameStream.str());
+  auto nominal =
+      member->getDeclContext()
+      ->getAsNominalTypeOrNominalTypeExtensionContext();
+  SmallString<32> typeName;
+  llvm::raw_svector_ostream typeNameStream(typeName);
+  typeNameStream << nominal->getName() << ".";
+
+  Diag->fixItInsert(loc, typeNameStream.str());
   return;
 }
 
@@ -2461,8 +2508,12 @@ diagnoseUnviableLookupResults(MemberLookupResult &result, Type baseObjTy,
   // because there is exactly one candidate!) diagnose this.
   bool sameProblem = true;
   auto firstProblem = result.UnviableCandidates[0].second;
-  for (auto cand : result.UnviableCandidates)
+  ValueDecl *member = nullptr;
+  for (auto cand : result.UnviableCandidates) {
+    if (member == nullptr)
+      member = cand.first;
     sameProblem &= cand.second == firstProblem;
+  }
 
   auto instanceTy = baseObjTy;
   if (auto *MTT = instanceTy->getAs<AnyMetatypeType>())
@@ -2527,48 +2578,11 @@ diagnoseUnviableLookupResults(MemberLookupResult &result, Type baseObjTy,
                instanceTy, memberName)
         .highlight(baseRange).highlight(nameLoc.getSourceRange());
       return;
-            
-    case MemberLookupResult::UR_TypeMemberOnInstance:
-      if (instanceTy->isExistentialType() && baseObjTy->is<AnyMetatypeType>()) {
-        // If the base of the lookup is an existential metatype, emit an
-        // error saying the lookup cannot be on a protocol metatype
-        auto Diag = diagnose(loc, diag::could_not_use_type_member_on_existential,
-                             baseObjTy, memberName);
-        Diag.highlight(baseRange).highlight(nameLoc.getSourceRange());
-
-        // See through function decl context
-        if (auto parent = CS->DC->getParent())
-        // If we are in a protocol extension of 'Proto' and we see
-        // 'Proto.static', suggest 'Self.static'
-        if (auto extensionContext = parent->getAsProtocolExtensionContext()) {
-          if (extensionContext->getDeclaredType()->getCanonicalType()
-              == instanceTy->getCanonicalType()) {
-            Diag.fixItReplace(baseRange, "Self");
-          }
-        }
 
-      } else {
-        // Otherwise the static member lookup was invalid because it was
-        // called on an instance
-        
-        // Handle enum element lookup on instance type
-        auto lookThroughBaseObjTy = baseObjTy->lookThroughAllAnyOptionalTypes();
-        if (lookThroughBaseObjTy->is<EnumType>()
-            || lookThroughBaseObjTy->is<BoundGenericEnumType>()) {
-          for (auto cand : result.UnviableCandidates) {
-            ValueDecl *decl = cand.first;
-            if (auto enumElementDecl = dyn_cast<EnumElementDecl>(decl)) {
-              diagnoseEnumInstanceMemberLookup(enumElementDecl, loc);
-              return;
-            }
-          }
-        }
-        
-        // Provide diagnostic other static member lookups on instance type
-        diagnose(loc, diag::could_not_use_type_member_on_instance,
-                 baseObjTy, memberName)
-          .highlight(baseRange).highlight(nameLoc.getSourceRange());
-      }
+    case MemberLookupResult::UR_TypeMemberOnInstance:
+      diagnoseTypeMemberOnInstanceLookup(baseObjTy, baseExpr,
+                                         memberName, nameLoc,
+                                         member, loc);
       return;
 
     case MemberLookupResult::UR_MutatingMemberOnRValue:

lib/Sema/CSGen.cpp

@@ -2627,9 +2627,7 @@ namespace {
     Type visitUnresolvedPatternExpr(UnresolvedPatternExpr *expr) {
       // If there are UnresolvedPatterns floating around after name binding,
       // they are pattern productions in invalid positions.
-      CS.TC.diagnose(expr->getLoc(), diag::pattern_in_expr,
-                     expr->getSubPattern()->getKind());
-      return Type();
+      return ErrorType::get(CS.getASTContext());
     }
 
     /// Get the type T?

lib/Sema/TypeCheckDecl.cpp

@@ -1121,7 +1121,6 @@ Type swift::configureImplicitSelf(TypeChecker &tc,
   // 'self' is 'let' for reference types (i.e., classes) or when 'self' is
   // neither inout.
   selfDecl->setLet(!selfTy->is<InOutType>());
-  selfDecl->setInOut(selfTy->is<InOutType>());
   selfDecl->overwriteType(selfTy);
 
   // Install the self type on the Parameter that contains it.  This ensures that