AST: Remove fromDefault and inferred from StructuralRequirement

Author: slavapestov

include/swift/AST/ASTContext.h

@@ -19,7 +19,6 @@
 
 #include "swift/AST/ASTAllocated.h"
 #include "swift/AST/Evaluator.h"
-#include "swift/AST/GenericSignature.h"
 #include "swift/AST/Identifier.h"
 #include "swift/AST/Import.h"
 #include "swift/AST/SILOptions.h"
@@ -86,6 +85,7 @@ namespace swift {
   class ForeignRepresentationInfo;
   class FuncDecl;
   class GenericContext;
+  class GenericSignature;
   class InFlightDiagnostic;
   class IterableDeclContext;
   class LazyContextData;

include/swift/AST/Attr.h

@@ -27,7 +27,6 @@
 #include "swift/AST/MacroDeclaration.h"
 #include "swift/AST/Ownership.h"
 #include "swift/AST/PlatformKind.h"
-#include "swift/AST/Requirement.h"
 #include "swift/AST/StorageImpl.h"
 #include "swift/Basic/Debug.h"
 #include "swift/Basic/EnumTraits.h"

include/swift/AST/ProtocolConformanceRef.h

@@ -17,7 +17,6 @@
 #define SWIFT_AST_PROTOCOLCONFORMANCEREF_H
 
 #include "swift/AST/ProtocolConformanceRef.h"
-#include "swift/AST/Requirement.h"
 #include "swift/AST/Type.h"
 #include "swift/AST/TypeAlignments.h"
 #include "swift/Basic/Debug.h"
@@ -36,6 +35,7 @@ class BuiltinProtocolConformance;
 class ConcreteDeclRef;
 class PackConformance;
 class ProtocolConformance;
+class Requirement;
 enum class EffectKind : uint8_t;
 
 /// A ProtocolConformanceRef is a handle to a protocol conformance which

include/swift/AST/Requirement.h

@@ -233,24 +233,11 @@ CheckRequirementsResult checkRequirements(
 /// A requirement as written in source, together with a source location. See
 /// ProtocolDecl::getStructuralRequirements().
 struct StructuralRequirement {
-  /// The actual requirement, where the types were resolved with the
-  /// 'Structural' type resolution stage.
+  /// A requirement with resolved in the structural resolution stage.
   Requirement req;
 
-  /// The source location where the requirement is written, used for redundancy
-  /// and conflict diagnostics.
+  /// The source location where the requirement is written, for diagnostics.
   SourceLoc loc;
-
-  /// A flag indicating whether the requirement was inferred from the
-  /// application of a type constructor. Also used for diagnostics, because
-  /// an inferred requirement made redundant by an explicit requirement is not
-  /// diagnosed as redundant, since we want to give users the option of
-  /// spelling out these requirements explicitly.
-  bool inferred = false;
-
-  /// A flag indicating whether this requirement was produced via the expansion
-  /// of default conformances to invertible protocols.
-  bool fromDefault = false;
 };
 
 /// An "anti-conformance" requirement `Subject: ~Protocol`.

include/swift/AST/Types.h

@@ -27,7 +27,6 @@
 #include "swift/AST/KnownProtocols.h"
 #include "swift/AST/Ownership.h"
 #include "swift/AST/ProtocolConformanceRef.h"
-#include "swift/AST/Requirement.h"
 #include "swift/AST/SubstitutionMap.h"
 #include "swift/AST/Type.h"
 #include "swift/AST/TypeAlignments.h"

lib/AST/Requirement.cpp

@@ -363,10 +363,7 @@ void InverseRequirement::expandDefaults(
 
       auto protoTy = proto->getDeclaredInterfaceType();
       result.push_back({{RequirementKind::Conformance, gp, protoTy},
-                        SourceLoc(),
-                        /*inferred=*/true,
-                        /*default=*/true,
-                       });
+                        SourceLoc()});
     }
   }
 }

lib/AST/RequirementMachine/ConcreteContraction.cpp

@@ -691,32 +691,15 @@ bool ConcreteContraction::performConcreteContraction(
     // requirement where the left hand side is not a type parameter.
     SmallVector<Requirement, 4> reqs;
     SmallVector<InverseRequirement, 4> ignoreInverses;
-    if (req.inferred) {
-      // Discard errors from desugaring a substituted requirement that
-      // was inferred. For example, if we have something like
-      //
-      //   <T, U where T == Int, U == Set<T>>
-      //
-      // The inferred requirement 'T : Hashable' from 'Set<>' will
-      // be substituted with 'T == Int' to get 'Int : Hashable'.
-      //
-      // Desugaring will diagnose a redundant conformance requirement,
-      // but we want to ignore that, since the user did not explicitly
-      // write 'Int : Hashable' (or 'T : Hashable') anywhere.
-      SmallVector<RequirementError, 4> discardErrors;
-      desugarRequirement(substReq, SourceLoc(), reqs,
-                         ignoreInverses, discardErrors);
-    } else {
-      desugarRequirement(substReq, req.loc, reqs, ignoreInverses, errors);
-    }
+    desugarRequirement(substReq, req.loc, reqs, ignoreInverses, errors);
 
     for (auto desugaredReq : reqs) {
       if (Debug) {
         llvm::dbgs() << "@@ Desugared requirement: ";
         desugaredReq.dump(llvm::dbgs());
         llvm::dbgs() << "\n";
       }
-      result.push_back({desugaredReq, req.loc, req.inferred});
+      result.push_back({desugaredReq, req.loc});
     }
 
     if (preserveSameTypeRequirement(req.req) &&
@@ -730,7 +713,7 @@ bool ConcreteContraction::performConcreteContraction(
 
       // Make the duplicated requirement 'inferred' so that we don't diagnose
       // it as redundant.
-      result.push_back({req.req, SourceLoc(), /*inferred=*/true});
+      result.push_back({req.req, SourceLoc()});
     }
   }
 

lib/AST/RequirementMachine/RequirementLowering.cpp

@@ -455,17 +455,11 @@ void swift::rewriting::desugarRequirements(
   SmallVector<StructuralRequirement, 2> result;
   for (auto req : reqs) {
     SmallVector<Requirement, 2> desugaredReqs;
-    SmallVector<RequirementError, 2> ignoredErrors;
-
-    if (req.inferred)
-      desugarRequirement(req.req, SourceLoc(), desugaredReqs,
-                         inverses, ignoredErrors);
-    else
-      desugarRequirement(req.req, req.loc, desugaredReqs,
-                         inverses, errors);
+    desugarRequirement(req.req, req.loc, desugaredReqs,
+                       inverses, errors);
 
     for (auto desugaredReq : desugaredReqs)
-      result.push_back({desugaredReq, req.loc, req.inferred});
+      result.push_back({desugaredReq, req.loc});
   }
 
   std::swap(reqs, result);
@@ -510,11 +504,11 @@ static void realizeTypeRequirement(DeclContext *dc,
   if (constraintType->isConstraintType()) {
     result.push_back({Requirement(RequirementKind::Conformance,
                                   subjectType, constraintType),
-                      loc, /*wasInferred=*/false});
+                      loc});
   } else if (constraintType->getClassOrBoundGenericClass()) {
     result.push_back({Requirement(RequirementKind::Superclass,
                                   subjectType, constraintType),
-                      loc, /*wasInferred=*/false});
+                      loc});
   } else {
     errors.push_back(
         RequirementError::forInvalidTypeRequirement(subjectType,
@@ -541,6 +535,9 @@ struct InferRequirementsWalker : public TypeWalker {
     if (ty->is<UnboundGenericType>())
       return Action::Stop;
 
+    if (!ty->hasTypeParameter())
+      return Action::SkipChildren;
+
     return Action::Continue;
   }
 
@@ -570,7 +567,7 @@ struct InferRequirementsWalker : public TypeWalker {
         if (skipRequirement(rawReq, decl))
           continue;
 
-        reqs.push_back({rawReq.subst(subMap), SourceLoc(), /*inferred=*/true});
+        reqs.push_back({rawReq.subst(subMap), SourceLoc()});
       }
 
       return Action::Continue;
@@ -586,7 +583,7 @@ struct InferRequirementsWalker : public TypeWalker {
       auto countType = packExpansion->getCountType();
       for (auto pack : packReferences)
         reqs.push_back({Requirement(RequirementKind::SameShape, countType, pack),
-                        SourceLoc(), /*inferred=*/true});
+                        SourceLoc()});
     }
 
     // Infer requirements from `@differentiable` function types.
@@ -600,7 +597,7 @@ struct InferRequirementsWalker : public TypeWalker {
       auto addConformanceConstraint = [&](Type type, ProtocolDecl *protocol) {
         reqs.push_back({Requirement(RequirementKind::Conformance, type,
                                     protocol->getDeclaredInterfaceType()),
-                        SourceLoc(), /*inferred=*/true});
+                        SourceLoc()});
       };
 
       auto &ctx = module->getASTContext();
@@ -614,7 +611,7 @@ struct InferRequirementsWalker : public TypeWalker {
               ->substBaseType(module, firstType);
           reqs.push_back({Requirement(RequirementKind::SameType,
                                       firstType, secondType),
-                          SourceLoc(), /*inferred=*/true});
+                          SourceLoc()});
         };
         auto *tangentVectorAssocType =
             differentiableProtocol->getAssociatedType(ctx.Id_TangentVector);
@@ -662,7 +659,7 @@ struct InferRequirementsWalker : public TypeWalker {
       if (skipRequirement(rawReq, decl))
         continue;
 
-      reqs.push_back({rawReq.subst(subMap), SourceLoc(), /*inferred=*/true});
+      reqs.push_back({rawReq.subst(subMap), SourceLoc()});
     }
 
     return Action::Continue;
@@ -731,7 +728,7 @@ void swift::rewriting::realizeRequirement(
       inferRequirements(firstType, firstLoc, moduleForInference, dc, result);
     }
 
-    result.push_back({req, loc, /*wasInferred=*/false});
+    result.push_back({req, loc});
     break;
   }
 
@@ -748,7 +745,7 @@ void swift::rewriting::realizeRequirement(
       inferRequirements(secondType, secondLoc, moduleForInference, dc, result);
     }
 
-    result.push_back({req, loc, /*wasInferred=*/false});
+    result.push_back({req, loc});
     break;
   }
   }
@@ -812,11 +809,7 @@ void swift::rewriting::applyInverses(
     if (req.getKind() != RequirementKind::Conformance)
       return false;
 
-    // Only consider requirements from defaults-expansion...
-    if (!structReq.fromDefault)
-      return false;
-
-    // involving an invertible protocol.
+    // Only consider requirements involving an invertible protocol.
     llvm::Optional<InvertibleProtocolKind> proto;
     if (auto kp = req.getProtocolDecl()->getKnownProtocolKind())
       if (auto ip = getInvertibleProtocolKind(*kp))
@@ -917,7 +910,7 @@ StructuralRequirementsRequest::evaluate(Evaluator &evaluator,
       result.push_back({
           Requirement(RequirementKind::Conformance,
                       selfTy, inheritedProto->getDeclaredInterfaceType()),
-          SourceLoc(), /*wasInferred=*/false});
+          SourceLoc()});
     }
   }
 
@@ -936,7 +929,7 @@ StructuralRequirementsRequest::evaluate(Evaluator &evaluator,
     auto layout = LayoutConstraint::getLayoutConstraint(
         LayoutConstraintKind::Class, ctx);
     result.push_back({Requirement(RequirementKind::Layout, selfTy, layout),
-                      proto->getLoc(), /*inferred=*/true});
+                      SourceLoc()});
 
     desugarRequirements(result, inverses, errors);
     InverseRequirement::expandDefaults(ctx, needsDefaultRequirements, result);
@@ -1000,8 +993,7 @@ StructuralRequirementsRequest::evaluate(Evaluator &evaluator,
             selfTy, typeAliasDecl->getName());
         Requirement req(RequirementKind::SameType, subjectType,
                         underlyingType);
-        result.push_back({req, typeAliasDecl->getLoc(),
-                          /*inferred=*/false});
+        result.push_back({req, typeAliasDecl->getLoc()});
       }
     }
   }

lib/AST/RequirementMachine/RequirementMachine.cpp

@@ -126,16 +126,10 @@
 // redundant rules. This is implemented in HomotopyReduction.cpp and
 // MinimalConformances.cpp.
 //
-// Minimization emits warnings about redundant rules by producing that
-// correspond to user-written requirements by producing RequirementError
-// values.
-//
 // After minimization, the remaining non-redundant rules are converted into
-// the Requirements of a minimal generic signature using the
-// RequirementBuilder.
-//
-// After minimization, the requirement machine undergoes a final state
-// transition into the immutable "frozen" state:
+// the Requirements of a minimal generic signature by the RequirementBuilder.
+// Then, the requirement machine undergoes a final state transition into the
+// immutable "frozen" state:
 //
 //   /-----------------------------\
 //  |  Complete RequirementMachine  |

lib/AST/RequirementMachine/RequirementMachineRequests.cpp

@@ -188,8 +188,8 @@ static void splitConcreteEquivalenceClasses(
                            req.getFirstType(), concreteType);
       Requirement secondReq(RequirementKind::SameType,
                             req.getSecondType(), concreteType);
-      splitRequirements.push_back({firstReq, SourceLoc(), /*inferred=*/false});
-      splitRequirements.push_back({secondReq, SourceLoc(), /*inferred=*/false});
+      splitRequirements.push_back({firstReq, SourceLoc()});
+      splitRequirements.push_back({secondReq, SourceLoc()});
 
       if (debug) {
         llvm::dbgs() << "- First split: ";
@@ -201,7 +201,7 @@ static void splitConcreteEquivalenceClasses(
       continue;
     }
 
-    splitRequirements.push_back({req, SourceLoc(), /*inferred=*/false});
+    splitRequirements.push_back({req, SourceLoc()});
 
     if (debug) {
       llvm::dbgs() << "- Not split: ";
@@ -319,7 +319,7 @@ RequirementSignatureRequest::evaluate(Evaluator &evaluator,
     for (auto req : proto->getStructuralRequirements())
       requirements.push_back(req);
     for (auto req : proto->getTypeAliasRequirements())
-      requirements.push_back({req, SourceLoc(), /*inferred=*/false});
+      requirements.push_back({req, SourceLoc()});
   }
 
   if (rewriteCtx.getDebugOptions().contains(DebugFlags::Timers)) {
@@ -638,11 +638,11 @@ AbstractGenericSignatureRequest::evaluate(
   // empty source locations.
   SmallVector<StructuralRequirement, 2> requirements;
   for (auto req : baseSignature.getRequirements())
-    requirements.push_back({req, SourceLoc(), /*wasInferred=*/false});
+    requirements.push_back({req, SourceLoc()});
 
   // Add the new requirements.
   for (auto req : addedRequirements)
-    requirements.push_back({req, SourceLoc(), /*wasInferred=*/false});
+    requirements.push_back({req, SourceLoc()});
 
   // The requirements passed to this request may have been substituted,
   // meaning the subject type might be a concrete type and not a type
@@ -780,7 +780,7 @@ InferredGenericSignatureRequest::evaluate(
   }();
 
   for (const auto &req : parentSig.getRequirements())
-    requirements.push_back({req, loc, /*wasInferred=*/false});
+    requirements.push_back({req, loc});
 
   DeclContext *lookupDC = nullptr;
 
@@ -868,7 +868,7 @@ InferredGenericSignatureRequest::evaluate(
   // inferred same-type requirements when building the generic signature of
   // an extension whose extended type is a generic typealias.
   for (const auto &req : addedRequirements)
-    requirements.push_back({req, SourceLoc(), /*inferred=*/true});
+    requirements.push_back({req, SourceLoc()});
 
   desugarRequirements(requirements, inverses, errors);
 
@@ -883,15 +883,6 @@ InferredGenericSignatureRequest::evaluate(
   InverseRequirement::expandDefaults(ctx, paramTypes, requirements);
   applyInverses(ctx, paramTypes, inverses, requirements, errors);
 
-  // Re-order requirements so that inferred requirements appear last. This
-  // ensures that if an inferred requirement is redundant with some other
-  // requirement, it is the inferred requirement that becomes redundant,
-  // which muffles the redundancy diagnostic.
-  std::stable_partition(requirements.begin(), requirements.end(),
-                        [](const StructuralRequirement &req) {
-                          return !req.inferred;
-                        });
-
   auto &rewriteCtx = ctx.getRewriteContext();
 
   if (rewriteCtx.getDebugOptions().contains(DebugFlags::Timers)) {