Skip to content

Commit 1260658

Browse files
authored
Merge pull request swiftlang#30348 from omochi/fix-arg-reorder
[ConstraintSystem] Fix bug of argument reordering in matchCallArguments
2 parents 719002b + 484606d commit 1260658

File tree

2 files changed

+247
-43
lines changed

2 files changed

+247
-43
lines changed

lib/Sema/CSSimplify.cpp

Lines changed: 43 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -651,58 +651,67 @@ matchCallArguments(SmallVectorImpl<AnyFunctionType::Param> &args,
651651
// parameter (or even worse - OoO parameter with label re-naming),
652652
// we most likely have no idea what would be the best
653653
// diagnostic for this situation, so let's just try to re-label.
654-
auto isOutOfOrderArgument = [&](bool hadLabelMismatch, unsigned argIdx,
655-
unsigned prevArgIdx) {
656-
if (hadLabelMismatch)
654+
auto isOutOfOrderArgument = [&](unsigned toParamIdx, unsigned fromArgIdx,
655+
unsigned toArgIdx) {
656+
if (fromArgIdx <= toArgIdx) {
657657
return false;
658+
}
659+
660+
auto newLabel = args[fromArgIdx].getLabel();
661+
auto oldLabel = args[toArgIdx].getLabel();
658662

659-
auto newLabel = args[argIdx].getLabel();
660-
auto oldLabel = args[prevArgIdx].getLabel();
663+
if (newLabel != params[toParamIdx].getLabel()) {
664+
return false;
665+
}
661666

662-
unsigned actualIndex = prevArgIdx;
663-
for (; actualIndex != argIdx; ++actualIndex) {
667+
auto paramIdx = toParamIdx + 1;
668+
for (; paramIdx < params.size(); ++paramIdx) {
664669
// Looks like new position (excluding defaulted parameters),
665670
// has a valid label.
666-
if (newLabel == params[actualIndex].getLabel())
671+
if (oldLabel == params[paramIdx].getLabel())
667672
break;
668673

669674
// If we are moving the the position with a different label
670675
// and there is no default value for it, can't diagnose the
671676
// problem as a simple re-ordering.
672-
if (!paramInfo.hasDefaultArgument(actualIndex))
677+
if (!paramInfo.hasDefaultArgument(paramIdx))
673678
return false;
674679
}
675680

676-
for (unsigned i = actualIndex + 1, n = params.size(); i != n; ++i) {
677-
if (oldLabel == params[i].getLabel())
678-
break;
679-
680-
if (!paramInfo.hasDefaultArgument(i))
681-
return false;
681+
// label was not found
682+
if (paramIdx == params.size()) {
683+
return false;
682684
}
683685

684686
return true;
685687
};
686688

687-
unsigned argIdx = 0;
689+
SmallVector<unsigned, 4> paramToArgMap;
690+
paramToArgMap.reserve(params.size());
691+
{
692+
unsigned argIdx = 0;
693+
for (const auto &binding : parameterBindings) {
694+
paramToArgMap.push_back(argIdx);
695+
argIdx += binding.size();
696+
}
697+
}
698+
688699
// Enumerate the parameters and their bindings to see if any arguments are
689700
// our of order
690701
bool hadLabelMismatch = false;
691-
for (auto binding : parameterBindings) {
692-
for (auto boundArgIdx : binding) {
702+
for (const auto paramIdx : indices(params)) {
703+
const auto toArgIdx = paramToArgMap[paramIdx];
704+
const auto &binding = parameterBindings[paramIdx];
705+
for (const auto paramBindIdx : indices(binding)) {
693706
// We've found the parameter that has an out of order
694707
// argument, and know the indices of the argument that
695708
// needs to move (fromArgIdx) and the argument location
696709
// it should move to (toArgIdx).
697-
auto fromArgIdx = boundArgIdx;
698-
auto toArgIdx = argIdx;
710+
const auto fromArgIdx = binding[paramBindIdx];
699711

700-
// If there is no re-ordering going on, and index is past
701-
// the number of parameters, it could only mean that this
702-
// is variadic parameter, so let's just move on.
703-
if (fromArgIdx == toArgIdx && toArgIdx >= params.size()) {
712+
// Does nothing for variadic tail.
713+
if (params[paramIdx].isVariadic() && paramBindIdx > 0) {
704714
assert(args[fromArgIdx].getLabel().empty());
705-
argIdx++;
706715
continue;
707716
}
708717

@@ -711,40 +720,40 @@ matchCallArguments(SmallVectorImpl<AnyFunctionType::Param> &args,
711720
// one argument requires label and another one doesn't, but caller
712721
// doesn't provide either, problem is going to be identified as
713722
// out-of-order argument instead of label mismatch.
714-
auto expectedLabel = params[toArgIdx].getLabel();
715-
auto argumentLabel = args[fromArgIdx].getLabel();
723+
const auto expectedLabel = params[paramIdx].getLabel();
724+
const auto argumentLabel = args[fromArgIdx].getLabel();
716725

717726
if (argumentLabel != expectedLabel) {
718727
// - The parameter is unnamed, in which case we try to fix the
719728
// problem by removing the name.
720729
if (expectedLabel.empty()) {
721730
hadLabelMismatch = true;
722-
if (listener.extraneousLabel(toArgIdx))
731+
if (listener.extraneousLabel(paramIdx))
723732
return true;
724733
// - The argument is unnamed, in which case we try to fix the
725734
// problem by adding the name.
726735
} else if (argumentLabel.empty()) {
727736
hadLabelMismatch = true;
728-
if (listener.missingLabel(toArgIdx))
737+
if (listener.missingLabel(paramIdx))
729738
return true;
730739
// - The argument label has a typo at the same position.
731740
} else if (fromArgIdx == toArgIdx) {
732741
hadLabelMismatch = true;
733-
if (listener.incorrectLabel(toArgIdx))
734-
return true;
742+
if (listener.incorrectLabel(paramIdx))
743+
return true;
735744
}
736745
}
737746

738-
if (boundArgIdx == argIdx) {
747+
if (fromArgIdx == toArgIdx) {
739748
// If the argument is in the right location, just continue
740-
argIdx++;
741749
continue;
742750
}
743751

744752
// This situation looks like out-of-order argument but it's hard
745753
// to say exactly without considering other factors, because it
746754
// could be invalid labeling too.
747-
if (isOutOfOrderArgument(hadLabelMismatch, fromArgIdx, toArgIdx))
755+
if (!hadLabelMismatch &&
756+
isOutOfOrderArgument(paramIdx, fromArgIdx, toArgIdx))
748757
return listener.outOfOrderArgument(fromArgIdx, toArgIdx);
749758

750759
SmallVector<Identifier, 8> expectedLabels;

0 commit comments

Comments
 (0)