[CS] Add specific accessors for path element info

This commit replaces the `getValue()` and `getValue2()` members on
`ConstraintLocator::PathElement` with specific accessors for each
expected path component kind. IMO this adds some clarity to the call
sites, especially for `getArgIdx()` and `getParamIdx()`.

In addition, this commit adds a private `getValue` member that can
access a value at a given index, which will make it easier to add a
third value in the future.

Author: hamishknight

lib/Sema/CSApply.cpp

@@ -1549,7 +1549,7 @@ namespace {
           [](ConstraintLocator *locator) -> unsigned {
         for (const auto &elt : locator->getPath()) {
           if (elt.getKind() == ConstraintLocator::KeyPathComponent)
-            return elt.getValue();
+            return elt.getKeyPathComponentIdx();
         }
         llvm_unreachable("no keypath component node");
       };

lib/Sema/CSDiag.cpp

@@ -175,7 +175,7 @@ void constraints::simplifyLocator(Expr *&anchor,
     case ConstraintLocator::NamedTupleElement:
     case ConstraintLocator::TupleElement: {
       // Extract tuple element.
-      unsigned index = path[0].getValue();
+      unsigned index = path[0].getTupleElementIdx();
       if (auto tupleExpr = dyn_cast<TupleExpr>(anchor)) {
         if (index < tupleExpr->getNumElements()) {
           anchor = tupleExpr->getElement(index);
@@ -197,7 +197,7 @@ void constraints::simplifyLocator(Expr *&anchor,
     case ConstraintLocator::ApplyArgToParam:
       // Extract tuple element.
       if (auto tupleExpr = dyn_cast<TupleExpr>(anchor)) {
-        unsigned index = path[0].getValue();
+        unsigned index = path[0].getArgIdx();
         if (index < tupleExpr->getNumElements()) {
           anchor = tupleExpr->getElement(index);
           path = path.slice(1);
@@ -207,7 +207,7 @@ void constraints::simplifyLocator(Expr *&anchor,
 
       // Extract subexpression in parentheses.
       if (auto parenExpr = dyn_cast<ParenExpr>(anchor)) {
-        assert(path[0].getValue() == 0);
+        assert(path[0].getArgIdx() == 0);
 
         anchor = parenExpr->getSubExpr();
         path = path.slice(1);
@@ -901,7 +901,7 @@ diagnoseUnresolvedDotExprTypeRequirementFailure(ConstraintSystem &cs,
 
   auto req = member->getAsGenericContext()
                  ->getGenericSignature()
-                 ->getRequirements()[last.getValue()];
+                 ->getRequirements()[last.getRequirementIdx()];
 
   Diag<Type, Type, Type, Type, StringRef> note;
   switch (req.getKind()) {

lib/Sema/CSDiagnostics.cpp

@@ -271,8 +271,8 @@ FailureDiagnostic::getFunctionArgApplyInfo(ConstraintLocator *locator) const {
     fnInterfaceType = resolveInterfaceType(rawFnType);
   }
 
-  auto argIdx = applyArgElt->getValue();
-  auto paramIdx = applyArgElt->getValue2();
+  auto argIdx = applyArgElt->getArgIdx();
+  auto paramIdx = applyArgElt->getParamIdx();
 
   return FunctionArgApplyInfo(argExpr, argIdx, getType(argExpr), paramIdx,
                               fnInterfaceType, fnType, callee);
@@ -1263,7 +1263,7 @@ bool RValueTreatedAsLValueFailure::diagnoseAsError() {
       subElementDiagID = diag::cannot_pass_rvalue_inout_subelement;
       rvalueDiagID = diag::cannot_pass_rvalue_inout;
       if (auto argTuple = dyn_cast<TupleExpr>(argExpr))
-        diagExpr = argTuple->getElement(lastPathElement.getValue());
+        diagExpr = argTuple->getElement(lastPathElement.getArgIdx());
       else if (auto parens = dyn_cast<ParenExpr>(argExpr))
         diagExpr = parens->getSubExpr();
     } else {
@@ -2417,7 +2417,7 @@ bool AutoClosureForwardingFailure::diagnoseAsError() {
 
   // We need a raw anchor here because `getAnchor()` is simplified
   // to the argument expression.
-  auto *argExpr = getArgumentExpr(getRawAnchor(), last.getValue());
+  auto *argExpr = getArgumentExpr(getRawAnchor(), last.getArgIdx());
   emitDiagnostic(argExpr->getLoc(), diag::invalid_autoclosure_forwarding)
       .highlight(argExpr->getSourceRange())
       .fixItInsertAfter(argExpr->getEndLoc(), "()");
@@ -3578,7 +3578,7 @@ bool KeyPathSubscriptIndexHashableFailure::diagnoseAsError() {
     auto *KPE = cast<KeyPathExpr>(anchor);
     for (auto &elt : locator->getPath()) {
       if (elt.isKeyPathComponent()) {
-        loc = KPE->getComponents()[elt.getValue()].getLoc();
+        loc = KPE->getComponents()[elt.getKeyPathComponentIdx()].getLoc();
         break;
       }
     }
@@ -3600,7 +3600,7 @@ SourceLoc InvalidMemberRefInKeyPath::getLoc() const {
         });
 
     assert(component != locator->getPath().end());
-    return KPE->getComponents()[component->getValue()].getLoc();
+    return KPE->getComponents()[component->getKeyPathComponentIdx()].getLoc();
   }
 
   return anchor->getLoc();
@@ -3689,7 +3689,7 @@ bool CollectionElementContextualFailure::diagnoseAsError() {
   if (isa<DictionaryExpr>(getRawAnchor())) {
     const auto &eltLoc = locator->getPath().back();
 
-    switch (eltLoc.getValue()) {
+    switch (eltLoc.getTupleElementIdx()) {
     case 0: // key
       diagnostic.emplace(emitDiagnostic(anchor->getLoc(),
                                         diag::cannot_convert_dict_key, eltType,

lib/Sema/CSDiagnostics.h

@@ -243,7 +243,7 @@ class RequirementFailure : public FailureDiagnostic {
     auto &last = path.back();
     assert(last.isTypeParameterRequirement() ||
            last.isConditionalRequirement());
-    assert(static_cast<RequirementKind>(last.getValue2()) == kind);
+    assert(last.getRequirementKind() == kind);
 
     // It's possible sometimes not to have no base expression.
     if (!expr)
@@ -258,9 +258,7 @@ class RequirementFailure : public FailureDiagnostic {
     assert(!path.empty());
 
     auto &requirementLoc = path.back();
-    assert(requirementLoc.isTypeParameterRequirement() ||
-           requirementLoc.isConditionalRequirement());
-    return requirementLoc.getValue();
+    return requirementLoc.getRequirementIdx();
   }
 
   /// The generic base type where failing requirement comes from.

lib/Sema/CSSimplify.cpp

@@ -769,7 +769,7 @@ getCalleeDeclAndArgs(ConstraintSystem &cs,
       return std::make_tuple(nullptr, /*hasAppliedSelf=*/false, argLabels,
                              hasTrailingClosure, nullptr);
 
-    auto componentIndex = path[0].getValue();
+    auto componentIndex = path[0].getKeyPathComponentIdx();
     if (componentIndex >= keyPath->getComponents().size())
       return std::make_tuple(nullptr, /*hasAppliedSelf=*/false, argLabels,
                              hasTrailingClosure, nullptr);
@@ -2106,8 +2106,7 @@ static ConstraintFix *fixRequirementFailure(ConstraintSystem &cs, Type type1,
 
   auto *reqLoc = cs.getConstraintLocator(anchor, path);
 
-  auto reqKind = static_cast<RequirementKind>(req.getValue2());
-  switch (reqKind) {
+  switch (req.getRequirementKind()) {
   case RequirementKind::SameType: {
     return SkipSameTypeRequirement::create(cs, type1, type2, reqLoc);
   }
@@ -5692,7 +5691,8 @@ ConstraintSystem::simplifyKeyPathConstraint(Type keyPathTy,
     if (locator->getAnchor() == keyPath
         && locator->getPath().size() <= 2
         && locator->getPath()[0].getKind() == ConstraintLocator::KeyPathComponent) {
-      choices[locator->getPath()[0].getValue()] = resolvedItem->Choice;
+      auto idx = locator->getPath()[0].getKeyPathComponentIdx();
+      choices[idx] = resolvedItem->Choice;
     }
   }
 

lib/Sema/ConstraintLocator.cpp

@@ -84,11 +84,9 @@ void ConstraintLocator::Profile(llvm::FoldingSetNodeID &id, Expr *anchor,
     case KeyPathRoot:
     case KeyPathValue:
     case KeyPathComponentResult:
-      if (unsigned numValues = numNumericValuesInPathElement(elt.getKind())) {
-        id.AddInteger(elt.getValue());
-        if (numValues > 1)
-          id.AddInteger(elt.getValue2());
-      }
+      auto numValues = numNumericValuesInPathElement(elt.getKind());
+      for (unsigned i = 0; i < numValues; ++i)
+        id.AddInteger(elt.getValue(i));
       break;
     }
   }
@@ -152,7 +150,7 @@ bool ConstraintLocator::isKeyPathSubscriptComponent() const {
     if (!elt.isKeyPathComponent())
       return false;
 
-    auto index = elt.getValue();
+    auto index = elt.getKeyPathComponentIdx();
     auto &component = KPE->getComponents()[index];
     return component.getKind() == ComponentKind::Subscript ||
            component.getKind() == ComponentKind::UnresolvedSubscript;
@@ -255,8 +253,8 @@ void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) {
       break;
 
     case ApplyArgToParam:
-      out << "comparing call argument #" << llvm::utostr(elt.getValue())
-          << " to parameter #" << llvm::utostr(elt.getValue2());
+      out << "comparing call argument #" << llvm::utostr(elt.getArgIdx())
+          << " to parameter #" << llvm::utostr(elt.getParamIdx());
       break;
 
     case ClosureResult:
@@ -284,7 +282,7 @@ void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) {
       break;
 
     case GenericArgument:
-      out << "generic argument #" << llvm::utostr(elt.getValue());
+      out << "generic argument #" << llvm::utostr(elt.getGenericArgIdx());
       break;
 
     case InstanceType:
@@ -304,7 +302,7 @@ void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) {
       break;
 
     case NamedTupleElement:
-      out << "named tuple element #" << llvm::utostr(elt.getValue());
+      out << "named tuple element #" << llvm::utostr(elt.getTupleElementIdx());
       break;
 
     case UnresolvedMember:
@@ -332,11 +330,12 @@ void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) {
       break;
 
     case TupleElement:
-      out << "tuple element #" << llvm::utostr(elt.getValue());
+      out << "tuple element #" << llvm::utostr(elt.getTupleElementIdx());
       break;
 
     case KeyPathComponent:
-      out << "key path component #" << llvm::utostr(elt.getValue());
+      out << "key path component #"
+          << llvm::utostr(elt.getKeyPathComponentIdx());
       break;
 
     case Requirement:
@@ -354,13 +353,15 @@ void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) {
       break;
 
     case ConditionalRequirement:
-      out << "conditional requirement #" << llvm::utostr(elt.getValue());
-      dumpReqKind(static_cast<RequirementKind>(elt.getValue2()));
+      out << "conditional requirement #"
+          << llvm::utostr(elt.getRequirementIdx());
+      dumpReqKind(elt.getRequirementKind());
       break;
 
     case TypeParameterRequirement: {
-      out << "type parameter requirement #" << llvm::utostr(elt.getValue());
-      dumpReqKind(static_cast<RequirementKind>(elt.getValue2()));
+      out << "type parameter requirement #"
+          << llvm::utostr(elt.getRequirementIdx());
+      dumpReqKind(elt.getRequirementKind());
       break;
     }
 
@@ -380,7 +381,7 @@ void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) {
       break;
 
     case SynthesizedArgument:
-      out << "synthesized argument #" << llvm::utostr(elt.getValue());
+      out << "synthesized argument #" << llvm::utostr(elt.getArgIdx());
       break;
 
     case KeyPathDynamicMember:

lib/Sema/ConstraintLocator.h

@@ -296,6 +296,19 @@ class ConstraintLocator : public llvm::FoldingSetNode {
       return { (PathElementKind)((unsigned)storage & 0xFF), storage >> 8 };
     }
 
+    /// Retrieve a value associated with the path element.
+    unsigned getValue(unsigned index) const {
+      unsigned numValues = numNumericValuesInPathElement(getKind());
+      assert(index < numValues && "Index out of range for path element value");
+
+      // We pack values into 16 bit components of the storage, with value0
+      // being stored in the upper bits, valueN in the lower bits. Therefore we
+      // need to shift out any extra values in the lower bits.
+      auto extraValues = numValues - index - 1;
+      auto value = decodeStorage(storage).second >> (extraValues * 16);
+      return value & 0xFFFF;
+    }
+
     PathElement(PathElementKind kind, unsigned value)
       : storage(encodeStorage(kind, value)), storedKind(StoredKindAndValue)
     {
@@ -431,29 +444,50 @@ class ConstraintLocator : public llvm::FoldingSetNode {
       llvm_unreachable("Unhandled StoredKind in switch.");
     }
 
-    /// Retrieve the value associated with this path element,
-    /// if it has one.
-    unsigned getValue() const {
-      unsigned numValues = numNumericValuesInPathElement(getKind());
-      assert(numValues > 0 && "No value in path element!");
+    /// Retrieve the index for a function argument path element.
+    unsigned getArgIdx() const {
+      assert((getKind() == ApplyArgToParam || isSynthesizedArgument()) &&
+             "Not an argument path element");
+      return getValue(0);
+    }
 
-      auto value = decodeStorage(storage).second;
-      if (numValues == 1) {
-        return value;
-      }
+    /// Retrieve the parameter index for an apply-arg-to-param path element.
+    unsigned getParamIdx() const {
+      assert(getKind() == ApplyArgToParam && "Not a parameter application");
+      return getValue(1);
+    }
 
-      return value >> 16;
+    /// Retrieve the index for a tuple element.
+    unsigned getTupleElementIdx() const {
+      assert((getKind() == TupleElement || getKind() == NamedTupleElement) &&
+             "Not a tuple element");
+      return getValue(0);
     }
 
-    /// Retrieve the second value associated with this path element,
-    /// if it has one.
-    unsigned getValue2() const {
-      unsigned numValues = numNumericValuesInPathElement(getKind());
-      (void)numValues;
-      assert(numValues == 2 && "No second value in path element!");
+    /// Retrieve the index for a key path component path element.
+    unsigned getKeyPathComponentIdx() const {
+      assert(isKeyPathComponent() && "Not a key path component");
+      return getValue(0);
+    }
+
+    /// Retrieve the index for a generic argument path element.
+    unsigned getGenericArgIdx() const {
+      assert(getKind() == GenericArgument && "Not a generic argument");
+      return getValue(0);
+    }
+
+    /// Retrieve the index for a generic requirement path element.
+    unsigned getRequirementIdx() const {
+      assert((isTypeParameterRequirement() || isConditionalRequirement()) &&
+             "Not a requirement");
+      return getValue(0);
+    }
 
-      auto value = decodeStorage(storage).second;
-      return value & 0x00FFFF;
+    /// Retrieve the kind of generic requirement path element.
+    RequirementKind getRequirementKind() const {
+      assert((isTypeParameterRequirement() || isConditionalRequirement()) &&
+             "Not a requirement");
+      return static_cast<RequirementKind>(getValue(1));
     }
 
     /// Retrieve the declaration for a witness path element.
@@ -522,7 +556,7 @@ class ConstraintLocator : public llvm::FoldingSetNode {
     /// Determine whether this element points to the contextual type
     /// associated with result of a single expression function.
     bool isResultOfSingleExprFunction() const {
-      return getKind() == PathElementKind::ContextualType ? bool(getValue())
+      return getKind() == PathElementKind::ContextualType ? bool(getValue(0))
                                                           : false;
     }
   };