Merge pull request #23724 from brentdax/static-start

[NFC] Correct assumptions about static AbstractStorageDecls

Author: beccadax

include/swift/AST/Decl.h

@@ -334,7 +334,7 @@ class alignas(1 << DeclAlignInBits) Decl {
     IsUserAccessible : 1
   );
 
-  SWIFT_INLINE_BITFIELD(AbstractStorageDecl, ValueDecl, 1+1+1+1+2+1+1,
+  SWIFT_INLINE_BITFIELD(AbstractStorageDecl, ValueDecl, 1+1+1+1+2+1+1+1,
     /// Whether the getter is mutating.
     IsGetterMutating : 1,
 
@@ -353,14 +353,14 @@ class alignas(1 << DeclAlignInBits) Decl {
     /// Whether a keypath component can directly reference this storage,
     /// or if it must use the overridden declaration instead.
     HasComputedValidKeyPathComponent : 1,
-    ValidKeyPathComponent : 1
-  );
-
-  SWIFT_INLINE_BITFIELD(VarDecl, AbstractStorageDecl, 1+4+1+1+1+1,
+    ValidKeyPathComponent : 1,
+    
     /// Whether this property is a type property (currently unfortunately
     /// called 'static').
-    IsStatic : 1,
+    IsStatic : 1
+  );
 
+  SWIFT_INLINE_BITFIELD(VarDecl, AbstractStorageDecl, 4+1+1+1+1,
     /// The specifier associated with this variable or parameter.  This
     /// determines the storage semantics of the value e.g. mutability.
     Specifier : 4,
@@ -393,6 +393,10 @@ class alignas(1 << DeclAlignInBits) Decl {
     /// Information about a symbolic default argument, like #file.
     defaultArgumentKind : NumDefaultArgumentKindBits
   );
+  
+  SWIFT_INLINE_BITFIELD(SubscriptDecl, VarDecl, 2,
+    StaticSpelling : 2
+  );
 
   SWIFT_INLINE_BITFIELD(EnumElementDecl, ValueDecl, 1,
     /// The ResilienceExpansion to use for default arguments.
@@ -4283,15 +4287,17 @@ class AbstractStorageDecl : public ValueDecl {
   }
 
 protected:
-  AbstractStorageDecl(DeclKind Kind, DeclContext *DC, DeclName Name,
-                      SourceLoc NameLoc, StorageIsMutable_t supportsMutation)
+  AbstractStorageDecl(DeclKind Kind, bool IsStatic, DeclContext *DC,
+                      DeclName Name, SourceLoc NameLoc,
+                      StorageIsMutable_t supportsMutation)
     : ValueDecl(Kind, DC, Name, NameLoc) {
     Bits.AbstractStorageDecl.HasStorage = true;
     Bits.AbstractStorageDecl.SupportsMutation = supportsMutation;
     Bits.AbstractStorageDecl.IsGetterMutating = false;
     Bits.AbstractStorageDecl.IsSetterMutating = true;
     Bits.AbstractStorageDecl.OpaqueReadOwnership =
       unsigned(OpaqueReadOwnership::Owned);
+    Bits.AbstractStorageDecl.IsStatic = IsStatic;
   }
 
   void setSupportsMutationIfStillStored(StorageIsMutable_t supportsMutation) {
@@ -4312,9 +4318,16 @@ class AbstractStorageDecl : public ValueDecl {
   /// attribute.
   bool isTransparent() const;
 
-  /// Determine whether this storage is a static member, if it
-  /// is a member.  Currently only variables can be static.
-  inline bool isStatic() const; // defined in this header
+  /// Is this a type ('static') variable?
+  bool isStatic() const {
+    return Bits.AbstractStorageDecl.IsStatic;
+  }
+  void setStatic(bool IsStatic) {
+    Bits.AbstractStorageDecl.IsStatic = IsStatic;
+  }
+
+  /// \returns the way 'static'/'class' should be spelled for this declaration.
+  StaticSpellingKind getCorrectStaticSpelling() const;
 
   /// Return the interface type of the stored value.
   Type getValueInterfaceType() const;
@@ -4612,10 +4625,9 @@ class VarDecl : public AbstractStorageDecl {
 
   VarDecl(DeclKind Kind, bool IsStatic, Specifier Sp, bool IsCaptureList,
           SourceLoc NameLoc, Identifier Name, DeclContext *DC)
-    : AbstractStorageDecl(Kind, DC, Name, NameLoc,
+    : AbstractStorageDecl(Kind, IsStatic, DC, Name, NameLoc,
                           StorageIsMutable_t(!isImmutableSpecifier(Sp)))
   {
-    Bits.VarDecl.IsStatic = IsStatic;
     Bits.VarDecl.Specifier = static_cast<unsigned>(Sp);
     Bits.VarDecl.IsCaptureList = IsCaptureList;
     Bits.VarDecl.IsDebuggerVar = false;
@@ -4794,14 +4806,6 @@ class VarDecl : public AbstractStorageDecl {
     return getSpecifier() == Specifier::InOut;
   }
 
-  
-  /// Is this a type ('static') variable?
-  bool isStatic() const { return Bits.VarDecl.IsStatic; }
-  void setStatic(bool IsStatic) { Bits.VarDecl.IsStatic = IsStatic; }
-
-  /// \returns the way 'static'/'class' should be spelled for this declaration.
-  StaticSpellingKind getCorrectStaticSpelling() const;
-
   bool isImmutable() const {
     return isImmutableSpecifier(getSpecifier());
   }
@@ -5110,24 +5114,40 @@ enum class ObjCSubscriptKind {
 /// signatures (indices and element type) are distinct.
 ///
 class SubscriptDecl : public GenericContext, public AbstractStorageDecl {
+  SourceLoc StaticLoc;
   SourceLoc ArrowLoc;
   ParameterList *Indices;
   TypeLoc ElementTy;
 
 public:
-  SubscriptDecl(DeclName Name, SourceLoc SubscriptLoc, ParameterList *Indices,
+  SubscriptDecl(DeclName Name,
+                SourceLoc StaticLoc, StaticSpellingKind StaticSpelling,
+                SourceLoc SubscriptLoc, ParameterList *Indices,
                 SourceLoc ArrowLoc, TypeLoc ElementTy, DeclContext *Parent,
                 GenericParamList *GenericParams)
     : GenericContext(DeclContextKind::SubscriptDecl, Parent),
-      AbstractStorageDecl(DeclKind::Subscript, Parent, Name, SubscriptLoc,
+      AbstractStorageDecl(DeclKind::Subscript,
+                          StaticSpelling != StaticSpellingKind::None,
+                          Parent, Name, SubscriptLoc,
                           /*will be overwritten*/ StorageIsNotMutable),
-      ArrowLoc(ArrowLoc), Indices(nullptr), ElementTy(ElementTy) {
+      StaticLoc(StaticLoc), ArrowLoc(ArrowLoc),
+      Indices(nullptr), ElementTy(ElementTy) {
+    Bits.SubscriptDecl.StaticSpelling = static_cast<unsigned>(StaticSpelling);
     setIndices(Indices);
     setGenericParams(GenericParams);
   }
 
+  /// \returns the way 'static'/'class' was spelled in the source.
+  StaticSpellingKind getStaticSpelling() const {
+    return static_cast<StaticSpellingKind>(Bits.SubscriptDecl.StaticSpelling);
+  }
+  
+  SourceLoc getStaticLoc() const { return StaticLoc; }
   SourceLoc getSubscriptLoc() const { return getNameLoc(); }
-  SourceLoc getStartLoc() const { return getSubscriptLoc(); }
+  
+  SourceLoc getStartLoc() const {
+    return getStaticLoc().isValid() ? getStaticLoc() : getSubscriptLoc();
+  }
   SourceRange getSourceRange() const;
   SourceRange getSignatureSourceRange() const;
 
@@ -6825,15 +6845,6 @@ AbstractStorageDecl::overwriteSetterAccess(AccessLevel accessLevel) {
     mutableAddressor->overwriteAccess(accessLevel);
 }
 
-inline bool AbstractStorageDecl::isStatic() const {
-  if (auto var = dyn_cast<VarDecl>(this)) {
-    return var->isStatic();
-  }
-
-  // Currently, subscripts are never static.
-  return false;
-}
-
 /// Constructors and destructors always have a 'self' parameter,
 /// which is stored in an instance member. Functions only have a
 /// 'self' if they are declared inside of a nominal type or extension,

include/swift/Serialization/ModuleFormat.h

@@ -52,7 +52,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0;
 /// describe what change you made. The content of this comment isn't important;
 /// it just ensures a conflict if two people change the module format.
 /// Don't worry about adhering to the 80-column limit for this line.
-const uint16_t SWIFTMODULE_VERSION_MINOR = 481; // Last change: custom attrs
+const uint16_t SWIFTMODULE_VERSION_MINOR = 482; // static subscripts
 
 using DeclIDField = BCFixed<31>;
 
@@ -1183,6 +1183,7 @@ namespace decls_block {
     DeclIDField, // overridden decl
     AccessLevelField, // access level
     AccessLevelField, // setter access, if applicable
+    StaticSpellingKindField,    // is subscript static?
     BCVBR<5>,    // number of parameter name components
     BCArray<IdentifierIDField> // name components,
                                // followed by DeclID accessors,

lib/AST/ASTDumper.cpp

@@ -797,8 +797,6 @@ namespace {
 
     void visitVarDecl(VarDecl *VD) {
       printCommon(VD, "var_decl");
-      if (VD->isStatic())
-        PrintWithColorRAII(OS, DeclModifierColor) << " type";
       if (VD->isLet())
         PrintWithColorRAII(OS, DeclModifierColor) << " let";
       if (VD->hasNonPatternBindingInit())
@@ -811,6 +809,9 @@ namespace {
     }
 
     void printStorageImpl(AbstractStorageDecl *D) {
+      if (D->isStatic())
+        PrintWithColorRAII(OS, DeclModifierColor) << " type";
+
       auto impl = D->getImplInfo();
       PrintWithColorRAII(OS, DeclModifierColor)
         << " readImpl="

lib/AST/ASTPrinter.cpp

@@ -893,8 +893,8 @@ class PrintAST : public ASTVisitor<PrintAST> {
 } // unnamed namespace
 
 static StaticSpellingKind getCorrectStaticSpelling(const Decl *D) {
-  if (auto *VD = dyn_cast<VarDecl>(D)) {
-    return VD->getCorrectStaticSpelling();
+  if (auto *ASD = dyn_cast<AbstractStorageDecl>(D)) {
+    return ASD->getCorrectStaticSpelling();
   } else if (auto *PBD = dyn_cast<PatternBindingDecl>(D)) {
     return PBD->getCorrectStaticSpelling();
   } else if (auto *FD = dyn_cast<FuncDecl>(D)) {

lib/AST/Decl.cpp

@@ -2019,12 +2019,9 @@ bool ValueDecl::isInstanceMember() const {
     return false;
 
   case DeclKind::Subscript:
-    // Subscripts are always instance members.
-    return true;
-
   case DeclKind::Var:
-    // Non-static variables are instance members.
-    return !cast<VarDecl>(this)->isStatic();
+    // Non-static variables and subscripts are instance members.
+    return !cast<AbstractStorageDecl>(this)->isStatic();
 
   case DeclKind::Module:
     // Modules are never instance members.
@@ -5098,12 +5095,16 @@ bool VarDecl::isAnonClosureParam() const {
   return nameStr[0] == '$';
 }
 
-StaticSpellingKind VarDecl::getCorrectStaticSpelling() const {
+StaticSpellingKind AbstractStorageDecl::getCorrectStaticSpelling() const {
   if (!isStatic())
     return StaticSpellingKind::None;
-  if (auto *PBD = getParentPatternBinding()) {
-    if (PBD->getStaticSpelling() != StaticSpellingKind::None)
-      return PBD->getStaticSpelling();
+  if (auto *VD = dyn_cast<VarDecl>(this)) {
+    if (auto *PBD = VD->getParentPatternBinding()) {
+      if (PBD->getStaticSpelling() != StaticSpellingKind::None)
+        return PBD->getStaticSpelling();
+    }
+  } else if (auto *SD = dyn_cast<SubscriptDecl>(this)) {
+    return SD->getStaticSpelling();
   }
 
   return getCorrectStaticSpellingForDecl(this);

lib/ClangImporter/ImportDecl.cpp

@@ -1565,7 +1565,7 @@ buildSubscriptGetterDecl(ClangImporter::Implementation &Impl,
                      AccessorKind::Get,
                      subscript,
                      /*StaticLoc=*/SourceLoc(),
-                     StaticSpellingKind::None,
+                     subscript->getStaticSpelling(),
                      /*Throws=*/false,
                      /*ThrowsLoc=*/SourceLoc(),
                      /*GenericParams=*/nullptr,
@@ -1621,7 +1621,7 @@ buildSubscriptSetterDecl(ClangImporter::Implementation &Impl,
                      AccessorKind::Set,
                      subscript,
                      /*StaticLoc=*/SourceLoc(),
-                     StaticSpellingKind::None,
+                     subscript->getStaticSpelling(),
                      /*Throws=*/false,
                      /*ThrowsLoc=*/SourceLoc(),
                      /*GenericParams=*/nullptr,
@@ -6570,6 +6570,7 @@ SwiftDeclConverter::importSubscript(Decl *decl,
   DeclName name(C, DeclBaseName::createSubscript(), {Identifier()});
   auto subscript = Impl.createDeclWithClangNode<SubscriptDecl>(
       getter->getClangNode(), getOverridableAccessLevel(dc), name,
+      /*StaticLoc=*/SourceLoc(), StaticSpellingKind::None,
       decl->getLoc(), bodyParams, decl->getLoc(),
       TypeLoc::withoutLoc(elementTy), dc,
       /*GenericParams=*/nullptr);

lib/Index/Index.cpp

@@ -927,8 +927,7 @@ bool IndexSwiftASTWalker::reportPseudoAccessor(AbstractStorageDecl *D,
     Info.symInfo.Kind = SymbolKind::Function;
     if (D->getDeclContext()->isTypeContext()) {
       if (D->isStatic()) {
-        if (isa<VarDecl>(D) &&
-            cast<VarDecl>(D)->getCorrectStaticSpelling() == StaticSpellingKind::KeywordClass)
+        if (D->getCorrectStaticSpelling() == StaticSpellingKind::KeywordClass)
           Info.symInfo.Kind = SymbolKind::ClassMethod;
         else
           Info.symInfo.Kind = SymbolKind::StaticMethod;

lib/Parse/ParseDecl.cpp

@@ -6412,6 +6412,7 @@ Parser::parseDeclSubscript(ParseDeclOptions Flags,
   DeclName name = DeclName(Context, DeclBaseName::createSubscript(),
                            argumentNames);
   auto *Subscript = new (Context) SubscriptDecl(name,
+                                                SourceLoc(), StaticSpellingKind::None,
                                                 SubscriptLoc, Indices.get(),
                                                 ArrowLoc, ElementTy.get(),
                                                 CurDeclContext,

lib/Sema/CodeSynthesis.cpp

@@ -184,10 +184,8 @@ static AccessorDecl *createGetterPrototype(AbstractStorageDecl *storage,
   auto *getterParams = buildIndexForwardingParamList(storage, {}, ctx);
 
   SourceLoc staticLoc;
-  if (auto var = dyn_cast<VarDecl>(storage)) {
-    if (var->isStatic())
-      staticLoc = var->getLoc();
-  }
+  if (storage->isStatic())
+    staticLoc = storage->getLoc();
 
   auto storageInterfaceType = storage->getValueInterfaceType();
 

lib/Sema/ConstraintSystem.cpp

@@ -1287,7 +1287,8 @@ ConstraintSystem::getTypeOfMemberReference(
     // If self is a value type and the base type is an lvalue, wrap it in an
     // inout type.
     auto selfFlags = ParameterTypeFlags();
-    if (!outerDC->getDeclaredInterfaceType()->hasReferenceSemantics() &&
+    if (isInstance &&
+        !outerDC->getDeclaredInterfaceType()->hasReferenceSemantics() &&
         baseTy->is<LValueType>() &&
         !selfTy->hasError())
       selfFlags = selfFlags.withInOut(true);

lib/Sema/TypeCheckDeclOverride.cpp

@@ -234,30 +234,21 @@ static bool areOverrideCompatibleSimple(ValueDecl *decl,
   if (parentDecl->isInvalid())
     return false;
 
-  if (auto func = dyn_cast<FuncDecl>(decl)) {
-    // Specific checking for methods.
-    auto parentFunc = cast<FuncDecl>(parentDecl);
-    if (func->isStatic() != parentFunc->isStatic())
-      return false;
-    if (func->isGeneric() != parentFunc->isGeneric())
-      return false;
-  } else if (auto ctor = dyn_cast<ConstructorDecl>(decl)) {
-    auto parentCtor = cast<ConstructorDecl>(parentDecl);
-    if (ctor->isGeneric() != parentCtor->isGeneric())
+  // If their staticness is different, they aren't compatible.
+  if (decl->isStatic() != parentDecl->isStatic())
+    return false;
+
+  // If their genericity is different, they aren't compatible.
+  if (auto genDecl = decl->getAsGenericContext()) {
+    auto genParentDecl = parentDecl->getAsGenericContext();
+    if (genDecl->isGeneric() != genParentDecl->isGeneric())
       return false;
+  }
 
-    // Factory initializers cannot be overridden.
+  // Factory initializers cannot be overridden.
+  if (auto parentCtor = dyn_cast<ConstructorDecl>(parentDecl))
     if (parentCtor->isFactoryInit())
       return false;
-  } else if (auto var = dyn_cast<VarDecl>(decl)) {
-    auto parentVar = cast<VarDecl>(parentDecl);
-    if (var->isStatic() != parentVar->isStatic())
-      return false;
-  } else if (auto subscript = dyn_cast<SubscriptDecl>(decl)) {
-    auto parentSubscript = cast<SubscriptDecl>(parentDecl);
-    if (subscript->isGeneric() != parentSubscript->isGeneric())
-      return false;
-  }
 
   return true;
 }

lib/Sema/TypeCheckProtocol.cpp

@@ -490,12 +490,10 @@ swift::matchWitness(
   } else if (auto *witnessASD = dyn_cast<AbstractStorageDecl>(witness)) {
     auto *reqASD = cast<AbstractStorageDecl>(req);
 
-    // If this is a property requirement, check that the static-ness matches.
-    if (auto *vdWitness = dyn_cast<VarDecl>(witness)) {
-      if (cast<VarDecl>(req)->isStatic() != vdWitness->isStatic())
-        return RequirementMatch(witness, MatchKind::StaticNonStaticConflict);
-    }
-    
+    // Check that the static-ness matches.
+    if (reqASD->isStatic() != witnessASD->isStatic())
+      return RequirementMatch(witness, MatchKind::StaticNonStaticConflict);
+
     // If the requirement is settable and the witness is not, reject it.
     if (req->isSettable(req->getDeclContext()) &&
         !witness->isSettable(witness->getDeclContext()))