AST: Don't attach trailing where clause requirements to the GenericParamList

slavapestov · slavapestov · commit c46eb22fcd78 · 2020-07-28T02:07:16.000-04:00
Previously we had two representations for the 'where' clause of a
parsed declaration; if the declaration had generic parameters of
its own, we would store them in the GenericParamList, otherwise
we would store them separately in a TrailingWhereClause instance.

Since the latter is more general and also used for protocols and
extensions, let's just use it for everything and simplify
GenericParamList in the process.
diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h
@@ -1190,9 +1190,6 @@ class GenericParamList final :
 
   GenericParamList *OuterParameters;
 
-  SourceLoc TrailingWhereLoc;
-  unsigned FirstTrailingWhereArg;
-
   GenericParamList(SourceLoc LAngleLoc,
                    ArrayRef<GenericTypeParamDecl *> Params,
                    SourceLoc WhereLoc,
@@ -1272,31 +1269,6 @@ class GenericParamList final :
   /// implicitly-generated requirements, and may be non-empty even if no
   /// 'where' keyword is present.
   ArrayRef<RequirementRepr> getRequirements() const { return Requirements; }
-
-  /// Retrieve only those requirements that are written within the brackets,
-  /// which does not include any requirements written in a trailing where
-  /// clause.
-  ArrayRef<RequirementRepr> getNonTrailingRequirements() const {
-    return Requirements.slice(0, FirstTrailingWhereArg);
-  }
-
-  /// Retrieve only those requirements written in a trailing where
-  /// clause.
-  ArrayRef<RequirementRepr> getTrailingRequirements() const {
-    return Requirements.slice(FirstTrailingWhereArg);
-  }
-
-  /// Determine whether the generic parameters have a trailing where clause.
-  bool hasTrailingWhereClause() const {
-    return FirstTrailingWhereArg < Requirements.size();
-  }
-
-  /// Add a trailing 'where' clause to the list of requirements.
-  ///
-  /// Trailing where clauses are written outside the angle brackets, after the
-  /// main part of a declaration's signature.
-  void addTrailingWhereClause(ASTContext &ctx, SourceLoc trailingWhereLoc,
-                              ArrayRef<RequirementRepr> trailingRequirements);
   
   /// Retrieve the outer generic parameter list.
   ///
@@ -1321,19 +1293,10 @@ class GenericParamList final :
     if (WhereLoc.isInvalid())
       return SourceRange();
 
-    auto endLoc = Requirements[FirstTrailingWhereArg-1].getSourceRange().End;
+    auto endLoc = Requirements.back().getSourceRange().End;
     return SourceRange(WhereLoc, endLoc);
   }
 
-  /// Retrieve the source range covering the trailing where clause.
-  SourceRange getTrailingWhereClauseSourceRange() const {
-    if (!hasTrailingWhereClause())
-      return SourceRange();
-
-    return SourceRange(TrailingWhereLoc,
-                       Requirements.back().getSourceRange().End);
-  }
-
   /// Configure the depth of the generic parameters in this list.
   void setDepth(unsigned depth);
 
diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h
@@ -396,6 +396,9 @@ struct WhereClauseOwner {
   WhereClauseOwner(DeclContext *dc, GenericParamList *genericParams)
       : dc(dc), source(genericParams) {}
 
+  WhereClauseOwner(DeclContext *dc, TrailingWhereClause *where)
+      : dc(dc), source(where) {}
+
   WhereClauseOwner(DeclContext *dc, SpecializeAttr *attr)
       : dc(dc), source(attr) {}
 
diff --git a/include/swift/Parse/Parser.h b/include/swift/Parse/Parser.h
@@ -1647,9 +1647,7 @@ class Parser {
   diagnoseWhereClauseInGenericParamList(const GenericParamList *GenericParams);
 
   ParserStatus
-  parseFreestandingGenericWhereClause(GenericContext *genCtx,
-                                      GenericParamList *&GPList,
-                                      ParseDeclOptions flags);
+  parseFreestandingGenericWhereClause(GenericContext *genCtx);
 
   ParserStatus parseGenericWhereClause(
       SourceLoc &WhereLoc, SmallVectorImpl<RequirementRepr> &Requirements,
diff --git a/lib/AST/ASTDumper.cpp b/lib/AST/ASTDumper.cpp
@@ -561,8 +561,7 @@ namespace {
       };
 
       if (const auto GC = Owner.dyn_cast<const GenericContext *>()) {
-        if (GC->getParsedGenericParams() == nullptr)
-          printWhere(GC->getTrailingWhereClause());
+        printWhere(GC->getTrailingWhereClause());
       } else {
         const auto ATD = Owner.get<const AssociatedTypeDecl *>();
         printWhere(ATD->getTrailingWhereClause());
diff --git a/lib/AST/ASTWalker.cpp b/lib/AST/ASTWalker.cpp
@@ -142,10 +142,6 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
       for (auto &Req: Where->getRequirements())
         if (doIt(Req))
           return true;
-    } else if (const auto GP = GC->getParsedGenericParams()) {
-      for (auto Req: GP->getTrailingRequirements())
-        if (doIt(Req))
-          return true;
     }
     return false;
   }
@@ -445,7 +441,7 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
     }
 
     // Visit param conformance
-    for (auto Req : GPL->getNonTrailingRequirements()) {
+    for (auto Req : GPL->getRequirements()) {
       if (doIt(Req))
         return true;
     }
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
@@ -888,8 +888,7 @@ GenericParamList::GenericParamList(SourceLoc LAngleLoc,
                                    SourceLoc RAngleLoc)
   : Brackets(LAngleLoc, RAngleLoc), NumParams(Params.size()),
     WhereLoc(WhereLoc), Requirements(Requirements),
-    OuterParameters(nullptr),
-    FirstTrailingWhereArg(Requirements.size())
+    OuterParameters(nullptr)
 {
   std::uninitialized_copy(Params.begin(), Params.end(),
                           getTrailingObjects<GenericTypeParamDecl *>());
@@ -938,27 +937,6 @@ GenericParamList::clone(DeclContext *dc) const {
   return GenericParamList::create(ctx, SourceLoc(), params, SourceLoc());
 }
 
-void GenericParamList::addTrailingWhereClause(
-       ASTContext &ctx,
-       SourceLoc trailingWhereLoc,
-       ArrayRef<RequirementRepr> trailingRequirements) {
-  assert(TrailingWhereLoc.isInvalid() &&
-         "Already have a trailing where clause?");
-  TrailingWhereLoc = trailingWhereLoc;
-  FirstTrailingWhereArg = Requirements.size();
-
-  // Create a unified set of requirements.
-  auto newRequirements = ctx.AllocateUninitialized<RequirementRepr>(
-                           Requirements.size() + trailingRequirements.size());
-  std::memcpy(newRequirements.data(), Requirements.data(),
-              Requirements.size() * sizeof(RequirementRepr));
-  std::memcpy(newRequirements.data() + Requirements.size(),
-              trailingRequirements.data(),
-              trailingRequirements.size() * sizeof(RequirementRepr));
-
-  Requirements = newRequirements;
-}
-
 void GenericParamList::setDepth(unsigned depth) {
   for (auto param : *this)
     param->setDepth(depth);
@@ -1054,9 +1032,7 @@ void GenericContext::setGenericSignature(GenericSignature genericSig) {
 }
 
 SourceRange GenericContext::getGenericTrailingWhereClauseSourceRange() const {
-  if (auto *params = getParsedGenericParams())
-    return params->getTrailingWhereClauseSourceRange();
-  else if (const auto *where = getTrailingWhereClause())
+  if (const auto *where = getTrailingWhereClause())
     return where->getSourceRange();
 
   return SourceRange();
diff --git a/lib/AST/GenericSignatureBuilder.cpp b/lib/AST/GenericSignatureBuilder.cpp
@@ -7591,17 +7591,17 @@ InferredGenericSignatureRequest::evaluate(
         .visitRequirements(TypeResolutionStage::Structural,
                            visitRequirement);
     }
-  } else {
-    // The declaration has a where clause, but no generic parameters of its own.
-    const auto ctx = paramSource.get<GenericContext *>();
-
-    assert(ctx->getTrailingWhereClause() && "No params or where clause");
+  }
 
-    // Determine where and how to perform name lookup.
-    lookupDC = ctx;
+  if (auto *ctx = paramSource.dyn_cast<GenericContext *>()) {
+    // The declaration might have a trailing where clause.
+    if (auto *where = ctx->getTrailingWhereClause()) {
+      // Determine where and how to perform name lookup.
+      lookupDC = ctx;
 
-    WhereClauseOwner(ctx).visitRequirements(
-      TypeResolutionStage::Structural, visitRequirement);
+      WhereClauseOwner(lookupDC, where).visitRequirements(
+        TypeResolutionStage::Structural, visitRequirement);
+    }
   }
       
   /// Perform any remaining requirement inference.
diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp
@@ -898,21 +898,24 @@ TypeDeclsFromWhereClauseRequest::evaluate(Evaluator &evaluator,
     auto decls = directReferencesForTypeRepr(evaluator, ctx, typeRepr, ext);
     result.insert(result.end(), decls.begin(), decls.end());
   };
-  for (const auto &req : ext->getGenericParams()->getTrailingRequirements()) {
-    switch (req.getKind()) {
-    case RequirementReprKind::TypeConstraint:
-      resolve(req.getSubjectRepr());
-      resolve(req.getConstraintRepr());
-      break;
 
-    case RequirementReprKind::SameType:
-      resolve(req.getFirstTypeRepr());
-      resolve(req.getSecondTypeRepr());
-      break;
+  if (auto *whereClause = ext->getTrailingWhereClause()) {
+    for (const auto &req : whereClause->getRequirements()) {
+      switch (req.getKind()) {
+      case RequirementReprKind::TypeConstraint:
+        resolve(req.getSubjectRepr());
+        resolve(req.getConstraintRepr());
+        break;
 
-    case RequirementReprKind::LayoutConstraint:
-      resolve(req.getSubjectRepr());
-      break;
+      case RequirementReprKind::SameType:
+        resolve(req.getFirstTypeRepr());
+        resolve(req.getSecondTypeRepr());
+        break;
+
+      case RequirementReprKind::LayoutConstraint:
+        resolve(req.getSubjectRepr());
+        break;
+      }
     }
   }
 
@@ -2333,23 +2336,6 @@ GenericParamListRequest::evaluate(Evaluator &evaluator, GenericContext *value) c
          outerParams = outerParams->getOuterParameters())
       outerParams->setDepth(depth--);
 
-    // If we have a trailing where clause, deal with it now.
-    // For now, trailing where clauses are only permitted on protocol extensions.
-    if (auto trailingWhereClause = ext->getTrailingWhereClause()) {
-      if (genericParams) {
-        // Merge the trailing where clause into the generic parameter list.
-        // FIXME: Long-term, we'd like clients to deal with the trailing where
-        // clause explicitly, but for now it's far more direct to represent
-        // the trailing where clause as part of the requirements.
-        genericParams->addTrailingWhereClause(
-          ext->getASTContext(),
-          trailingWhereClause->getWhereLoc(),
-          trailingWhereClause->getRequirements());
-      }
-
-      // If there's no generic parameter list, the where clause is diagnosed
-      // in typeCheckDecl().
-    }
     return genericParams;
   } else if (auto *proto = dyn_cast<ProtocolDecl>(value)) {
     // The generic parameter 'Self'.
diff --git a/lib/IDE/Formatting.cpp b/lib/IDE/Formatting.cpp
@@ -1585,6 +1585,8 @@ class FormatWalker : public ASTWalker {
         return Ctx;
       if (auto Ctx = getIndentContextFrom(AFD->getParsedGenericParams(), ContextLoc, D))
         return Ctx;
+      if (auto Ctx = getIndentContextFrom(AFD->getTrailingWhereClause(), ContextLoc, D))
+        return Ctx;
 
       if (TrailingTarget)
         return None;
@@ -1658,6 +1660,8 @@ class FormatWalker : public ASTWalker {
         return Ctx;
       if (auto Ctx = getIndentContextFrom(SD->getParsedGenericParams(), ContextLoc, D))
         return Ctx;
+      if (auto Ctx = getIndentContextFrom(SD->getTrailingWhereClause(), ContextLoc, D))
+        return Ctx;
 
       if (TrailingTarget)
         return None;
@@ -1756,6 +1760,10 @@ class FormatWalker : public ASTWalker {
                                           D)) {
         return Ctx;
       }
+      if (auto Ctx = getIndentContextFrom(TAD->getTrailingWhereClause(), ContextLoc,
+                                          D)) {
+        return Ctx;
+      }
 
       return IndentContext {ContextLoc, !OutdentChecker::hasOutdent(SM, D)};
     }
@@ -1874,11 +1882,6 @@ class FormatWalker : public ASTWalker {
         return Ctx;
     }
 
-    SourceRange TrailingRange = GP->getTrailingWhereClauseSourceRange();
-    if (auto Ctx = getIndentContextFromWhereClause(GP->getRequirements(),
-                                                   TrailingRange, ContextLoc,
-                                                   WalkableParent))
-      return Ctx;
     return None;
   }
 
diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
@@ -5052,10 +5052,10 @@ parseDeclTypeAlias(Parser::ParseDeclOptions Flags, DeclAttributes &Attributes) {
   TAD->setUnderlyingTypeRepr(UnderlyingTy.getPtrOrNull());
   TAD->getAttrs() = Attributes;
 
-  // Parse a 'where' clause if present, adding it to our GenericParamList.
+  // Parse a 'where' clause if present.
   if (Tok.is(tok::kw_where)) {
     ContextChange CC(*this, TAD);
-    Status |= parseFreestandingGenericWhereClause(TAD, genericParams, Flags);
+    Status |= parseFreestandingGenericWhereClause(TAD);
   }
 
   if (UnderlyingTy.isNull()) {
@@ -6393,11 +6393,11 @@ ParserResult<FuncDecl> Parser::parseDeclFunc(SourceLoc StaticLoc,
     }
   }
   
-  // Parse a 'where' clause if present, adding it to our GenericParamList.
+  // Parse a 'where' clause if present.
   if (Tok.is(tok::kw_where)) {
     ContextChange CC(*this, FD);
 
-    Status |= parseFreestandingGenericWhereClause(FD, GenericParams, Flags);
+    Status |= parseFreestandingGenericWhereClause(FD);
     if (Status.hasCodeCompletion() && !CodeCompletion) {
       // Trigger delayed parsing, no need to continue.
       return Status;
@@ -6650,10 +6650,9 @@ ParserResult<EnumDecl> Parser::parseDeclEnum(ParseDeclOptions Flags,
 
   diagnoseWhereClauseInGenericParamList(GenericParams);
   
-  // Parse a 'where' clause if present, adding it to our GenericParamList.
+  // Parse a 'where' clause if present.
   if (Tok.is(tok::kw_where)) {
-    auto whereStatus =
-        parseFreestandingGenericWhereClause(ED, GenericParams, Flags);
+    auto whereStatus = parseFreestandingGenericWhereClause(ED);
     if (whereStatus.hasCodeCompletion() && !CodeCompletion) {
       // Trigger delayed parsing, no need to continue.
       return whereStatus;
@@ -6931,10 +6930,9 @@ ParserResult<StructDecl> Parser::parseDeclStruct(ParseDeclOptions Flags,
 
   diagnoseWhereClauseInGenericParamList(GenericParams);
 
-  // Parse a 'where' clause if present, adding it to our GenericParamList.
+  // Parse a 'where' clause if present.
   if (Tok.is(tok::kw_where)) {
-    auto whereStatus =
-        parseFreestandingGenericWhereClause(SD, GenericParams, Flags);
+    auto whereStatus = parseFreestandingGenericWhereClause(SD);
     if (whereStatus.hasCodeCompletion() && !CodeCompletion) {
       // Trigger delayed parsing, no need to continue.
       return whereStatus;
@@ -7044,10 +7042,9 @@ ParserResult<ClassDecl> Parser::parseDeclClass(ParseDeclOptions Flags,
 
   diagnoseWhereClauseInGenericParamList(GenericParams);
 
-  // Parse a 'where' clause if present, adding it to our GenericParamList.
+  // Parse a 'where' clause if present.
   if (Tok.is(tok::kw_where)) {
-    auto whereStatus =
-        parseFreestandingGenericWhereClause(CD, GenericParams, Flags);
+    auto whereStatus = parseFreestandingGenericWhereClause(CD);
     if (whereStatus.hasCodeCompletion() && !CodeCompletion) {
       // Trigger delayed parsing, no need to continue.
       return whereStatus;
@@ -7287,12 +7284,11 @@ Parser::parseDeclSubscript(SourceLoc StaticLoc,
 
   DefaultArgs.setFunctionContext(Subscript, Subscript->getIndices());
 
-  // Parse a 'where' clause if present, adding it to our GenericParamList.
+  // Parse a 'where' clause if present.
   if (Tok.is(tok::kw_where)) {
     ContextChange CC(*this, Subscript);
 
-    Status |= parseFreestandingGenericWhereClause(Subscript, GenericParams,
-                                                  Flags);
+    Status |= parseFreestandingGenericWhereClause(Subscript);
     if (Status.hasCodeCompletion() && !CodeCompletion) {
       // Trigger delayed parsing, no need to continue.
       return Status;
@@ -7430,11 +7426,11 @@ Parser::parseDeclInit(ParseDeclOptions Flags, DeclAttributes &Attributes) {
   CD->setImplicitlyUnwrappedOptional(IUO);
   CD->getAttrs() = Attributes;
 
-  // Parse a 'where' clause if present, adding it to our GenericParamList.
+  // Parse a 'where' clause if present.
   if (Tok.is(tok::kw_where)) {
     ContextChange(*this, CD);
 
-    Status |= parseFreestandingGenericWhereClause(CD, GenericParams, Flags);
+    Status |= parseFreestandingGenericWhereClause(CD);
     if (Status.hasCodeCompletion() && !CodeCompletion) {
       // Trigger delayed parsing, no need to continue.
       return Status;
diff --git a/lib/Parse/ParseGeneric.cpp b/lib/Parse/ParseGeneric.cpp
diff --git a/lib/Sema/TypeCheckGeneric.cpp b/lib/Sema/TypeCheckGeneric.cpp
diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp
diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp

Original file line number	Diff line number	Diff line change
`@@ -142,10 +142,6 @@ class Traversal : public ASTVisitor<Traversal, Expr, Stmt,`
`142`	`142`	`for (auto &Req: Where->getRequirements())`
`143`	`143`	`if (doIt(Req))`
`144`	`144`	`return true;`
`145`		`- } else if (const auto GP = GC->getParsedGenericParams()) {`
`146`		`- for (auto Req: GP->getTrailingRequirements())`
`147`		`- if (doIt(Req))`
`148`		`- return true;`
`149`	`145`	`}`
`150`	`146`	`return false;`
`151`	`147`	`}`
`@@ -445,7 +441,7 @@ class Traversal : public ASTVisitor<Traversal, Expr, Stmt,`
`445`	`441`	`}`
`446`	`442`
`447`	`443`	`// Visit param conformance`
`448`		`- for (auto Req : GPL->getNonTrailingRequirements()) {`
	`444`	`+ for (auto Req : GPL->getRequirements()) {`
`449`	`445`	`if (doIt(Req))`
`450`	`446`	`return true;`
`451`	`447`	`}`