Skip to content

Commit 7800a04

Browse files
xedinxedin
committed
[Diagnostics] Adjust "missing arguments" diagnostic to store parameter indices directly
This helps to avoid allocating new type variables (which shouldn't be done regardless) to store parameter indices when `missing arguments` diagnostic is used by other diagnostics.
1 parent 3ab8710 commit 7800a04

File tree

5 files changed

+59
-60
lines changed

5 files changed

+59
-60
lines changed

‎lib/Sema/CSDiagnostics.cpp

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3850,15 +3850,13 @@ bool MissingArgumentsFailure::diagnoseAsError() {
38503850

38513851
interleave(
38523852
SynthesizedArgs,
3853-
[&](const AnyFunctionType::Param &arg) {
3853+
[&](const std::pair<unsigned, AnyFunctionType::Param> &e) {
3854+
const auto paramIdx = e.first;
3855+
const auto &arg = e.second;
3856+
38543857
if (arg.hasLabel()) {
38553858
arguments << "'" << arg.getLabel().str() << "'";
38563859
} else {
3857-
auto *typeVar = arg.getPlainType()->castTo<TypeVariableType>();
3858-
auto *locator = typeVar->getImpl().getLocator();
3859-
auto paramIdx = locator->findLast<LocatorPathElt::ApplyArgToParam>()
3860-
->getParamIdx();
3861-
38623860
arguments << "#" << (paramIdx + 1);
38633861
}
38643862
},
@@ -3882,7 +3880,9 @@ bool MissingArgumentsFailure::diagnoseAsError() {
38823880
llvm::raw_svector_ostream fixIt(scratch);
38833881
interleave(
38843882
SynthesizedArgs,
3885-
[&](const AnyFunctionType::Param &arg) { forFixIt(fixIt, arg); },
3883+
[&](const std::pair<unsigned, AnyFunctionType::Param> &arg) {
3884+
forFixIt(fixIt, arg.second);
3885+
},
38863886
[&] { fixIt << ", "; });
38873887

38883888
auto *tuple = cast<TupleExpr>(argExpr);
@@ -3927,19 +3927,16 @@ bool MissingArgumentsFailure::diagnoseSingleMissingArgument() const {
39273927
return false;
39283928

39293929
const auto &argument = SynthesizedArgs.front();
3930-
auto *argType = argument.getPlainType()->castTo<TypeVariableType>();
3931-
auto *argLocator = argType->getImpl().getLocator();
3932-
auto position =
3933-
argLocator->findLast<LocatorPathElt::ApplyArgToParam>()->getParamIdx();
3934-
auto label = argument.getLabel();
3930+
auto position = argument.first;
3931+
auto label = argument.second.getLabel();
39353932

39363933
SmallString<32> insertBuf;
39373934
llvm::raw_svector_ostream insertText(insertBuf);
39383935

39393936
if (position != 0)
39403937
insertText << ", ";
39413938

3942-
forFixIt(insertText, argument);
3939+
forFixIt(insertText, argument.second);
39433940

39443941
Expr *fnExpr = nullptr;
39453942
Expr *argExpr = nullptr;
@@ -5599,26 +5596,20 @@ bool ArgumentMismatchFailure::diagnoseArchetypeMismatch() const {
55995596
}
56005597

56015598
bool ArgumentMismatchFailure::diagnoseMisplacedMissingArgument() const {
5602-
auto &cs = getConstraintSystem();
5599+
const auto &solution = getSolution();
56035600
auto *locator = getLocator();
56045601

5605-
if (!MissingArgumentsFailure::isMisplacedMissingArgument(getSolution(),
5606-
locator))
5602+
if (!MissingArgumentsFailure::isMisplacedMissingArgument(solution, locator))
56075603
return false;
56085604

5609-
auto *argType = cs.createTypeVariable(
5610-
cs.getConstraintLocator(locator, LocatorPathElt::SynthesizedArgument(1)),
5611-
/*flags=*/0);
5612-
56135605
// Assign new type variable to a type of a parameter.
56145606
auto *fnType = getFnType();
56155607
const auto &param = fnType->getParams()[0];
5616-
cs.assignFixedType(argType, param.getOldType());
56175608

56185609
auto *anchor = getRawAnchor();
56195610

56205611
MissingArgumentsFailure failure(
5621-
getSolution(), {param.withType(argType)},
5612+
solution, {std::make_pair(0, param)},
56225613
getConstraintLocator(anchor, ConstraintLocator::ApplyArgument));
56235614

56245615
return failure.diagnoseSingleMissingArgument();

‎lib/Sema/CSDiagnostics.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1223,13 +1223,13 @@ class ImplicitInitOnNonConstMetatypeFailure final
12231223
};
12241224

12251225
class MissingArgumentsFailure final : public FailureDiagnostic {
1226-
using Param = AnyFunctionType::Param;
1226+
using SynthesizedParam = std::pair<unsigned, AnyFunctionType::Param>;
12271227

1228-
SmallVector<Param, 4> SynthesizedArgs;
1228+
SmallVector<SynthesizedParam, 4> SynthesizedArgs;
12291229

12301230
public:
12311231
MissingArgumentsFailure(const Solution &solution,
1232-
ArrayRef<Param> synthesizedArgs,
1232+
ArrayRef<SynthesizedParam> synthesizedArgs,
12331233
ConstraintLocator *locator)
12341234
: FailureDiagnostic(solution, locator),
12351235
SynthesizedArgs(synthesizedArgs.begin(), synthesizedArgs.end()) {

‎lib/Sema/CSFix.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -636,9 +636,9 @@ bool AddMissingArguments::diagnose(const Solution &solution,
636636

637637
AddMissingArguments *
638638
AddMissingArguments::create(ConstraintSystem &cs,
639-
llvm::ArrayRef<Param> synthesizedArgs,
639+
ArrayRef<SynthesizedParam> synthesizedArgs,
640640
ConstraintLocator *locator) {
641-
unsigned size = totalSizeToAlloc<Param>(synthesizedArgs.size());
641+
unsigned size = totalSizeToAlloc<SynthesizedParam>(synthesizedArgs.size());
642642
void *mem = cs.getAllocator().Allocate(size, alignof(AddMissingArguments));
643643
return new (mem) AddMissingArguments(cs, synthesizedArgs, locator);
644644
}

‎lib/Sema/CSFix.h

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1091,16 +1091,16 @@ class AllowClosureParamDestructuring final : public ConstraintFix {
10911091

10921092
class AddMissingArguments final
10931093
: public ConstraintFix,
1094-
private llvm::TrailingObjects<AddMissingArguments,
1095-
AnyFunctionType::Param> {
1094+
private llvm::TrailingObjects<
1095+
AddMissingArguments, std::pair<unsigned, AnyFunctionType::Param>> {
10961096
friend TrailingObjects;
10971097

1098-
using Param = AnyFunctionType::Param;
1098+
using SynthesizedParam = std::pair<unsigned, AnyFunctionType::Param>;
10991099

11001100
unsigned NumSynthesized;
11011101

11021102
AddMissingArguments(ConstraintSystem &cs,
1103-
llvm::ArrayRef<Param> synthesizedArgs,
1103+
ArrayRef<SynthesizedParam> synthesizedArgs,
11041104
ConstraintLocator *locator)
11051105
: ConstraintFix(cs, FixKind::AddMissingArguments, locator),
11061106
NumSynthesized(synthesizedArgs.size()) {
@@ -1111,8 +1111,8 @@ class AddMissingArguments final
11111111
public:
11121112
std::string getName() const override { return "synthesize missing argument(s)"; }
11131113

1114-
ArrayRef<Param> getSynthesizedArguments() const {
1115-
return {getTrailingObjects<Param>(), NumSynthesized};
1114+
ArrayRef<SynthesizedParam> getSynthesizedArguments() const {
1115+
return {getTrailingObjects<SynthesizedParam>(), NumSynthesized};
11161116
}
11171117

11181118
bool diagnose(const Solution &solution, bool asNote = false) const override;
@@ -1122,12 +1122,12 @@ class AddMissingArguments final
11221122
}
11231123

11241124
static AddMissingArguments *create(ConstraintSystem &cs,
1125-
llvm::ArrayRef<Param> synthesizedArgs,
1125+
ArrayRef<SynthesizedParam> synthesizedArgs,
11261126
ConstraintLocator *locator);
11271127

11281128
private:
1129-
MutableArrayRef<Param> getSynthesizedArgumentsBuf() {
1130-
return {getTrailingObjects<Param>(), NumSynthesized};
1129+
MutableArrayRef<SynthesizedParam> getSynthesizedArgumentsBuf() {
1130+
return {getTrailingObjects<SynthesizedParam>(), NumSynthesized};
11311131
}
11321132
};
11331133

‎lib/Sema/CSSimplify.cpp

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -773,7 +773,7 @@ class ArgumentFailureTracker : public MatchCallArgumentListener {
773773
SmallVectorImpl<ParamBinding> &Bindings;
774774
ConstraintLocatorBuilder Locator;
775775

776-
unsigned NumSynthesizedArgs = 0;
776+
SmallVector<std::pair<unsigned, AnyFunctionType::Param>, 4> MissingArguments;
777777
SmallVector<std::pair<unsigned, AnyFunctionType::Param>, 4> ExtraArguments;
778778

779779
public:
@@ -786,15 +786,12 @@ class ArgumentFailureTracker : public MatchCallArgumentListener {
786786
Locator(locator) {}
787787

788788
~ArgumentFailureTracker() override {
789-
if (NumSynthesizedArgs > 0) {
790-
ArrayRef<AnyFunctionType::Param> argRef(Arguments);
791-
792-
auto *fix =
793-
AddMissingArguments::create(CS, argRef.take_back(NumSynthesizedArgs),
794-
CS.getConstraintLocator(Locator));
789+
if (!MissingArguments.empty()) {
790+
auto *fix = AddMissingArguments::create(CS, MissingArguments,
791+
CS.getConstraintLocator(Locator));
795792

796793
// Not having an argument is the same impact as having a type mismatch.
797-
(void)CS.recordFix(fix, /*impact=*/NumSynthesizedArgs * 2);
794+
(void)CS.recordFix(fix, /*impact=*/MissingArguments.size() * 2);
798795
}
799796
}
800797

@@ -814,8 +811,10 @@ class ArgumentFailureTracker : public MatchCallArgumentListener {
814811
CS.createTypeVariable(argLoc, TVO_CanBindToInOut | TVO_CanBindToLValue |
815812
TVO_CanBindToNoEscape | TVO_CanBindToHole);
816813

817-
Arguments.push_back(param.withType(argType));
818-
++NumSynthesizedArgs;
814+
auto synthesizedArg = param.withType(argType);
815+
816+
MissingArguments.push_back(std::make_pair(paramIdx, synthesizedArg));
817+
Arguments.push_back(synthesizedArg);
819818

820819
return newArgIdx;
821820
}
@@ -846,7 +845,7 @@ class ArgumentFailureTracker : public MatchCallArgumentListener {
846845
// record a fix for this, increase the score so there is a way
847846
// to identify that there is something going on besides just missing
848847
// arguments.
849-
if (NumSynthesizedArgs || !ExtraArguments.empty()) {
848+
if (!MissingArguments.empty() || !ExtraArguments.empty()) {
850849
CS.increaseScore(SK_Fix);
851850
return false;
852851
}
@@ -915,7 +914,7 @@ class ArgumentFailureTracker : public MatchCallArgumentListener {
915914
// If there are not only labeling problems but also some of the
916915
// arguments are missing, let's account of that in the impact.
917916
auto impact = 1 + numOutOfOrder + numExtraneous * 2 + numRenames * 3 +
918-
NumSynthesizedArgs * 2;
917+
MissingArguments.size() * 2;
919918
return CS.recordFix(fix, impact);
920919
}
921920

@@ -1008,15 +1007,19 @@ ConstraintSystem::TypeMatchResult constraints::matchCallArguments(
10081007
argsWithLabels.pop_back();
10091008
// Let's make sure that labels associated with tuple elements
10101009
// line up with what is expected by argument list.
1011-
for (const auto &arg : argTuple->getElements()) {
1012-
argsWithLabels.push_back(
1013-
AnyFunctionType::Param(arg.getType(), arg.getName()));
1010+
SmallVector<std::pair<unsigned, AnyFunctionType::Param>, 4>
1011+
synthesizedArgs;
1012+
for (unsigned i = 0, n = argTuple->getNumElements(); i != n; ++i) {
1013+
const auto &elt = argTuple->getElement(i);
1014+
AnyFunctionType::Param argument(elt.getType(), elt.getName());
1015+
synthesizedArgs.push_back(std::make_pair(i, argument));
1016+
argsWithLabels.push_back(argument);
10141017
}
10151018

10161019
(void)cs.recordFix(
1017-
AddMissingArguments::create(cs, argsWithLabels,
1020+
AddMissingArguments::create(cs, synthesizedArgs,
10181021
cs.getConstraintLocator(locator)),
1019-
/*impact=*/argsWithLabels.size() * 2);
1022+
/*impact=*/synthesizedArgs.size() * 2);
10201023
}
10211024
}
10221025

@@ -1476,12 +1479,17 @@ static bool fixMissingArguments(ConstraintSystem &cs, Expr *anchor,
14761479
for (unsigned i = args.size(), n = params.size(); i != n; ++i) {
14771480
auto *argLoc = cs.getConstraintLocator(
14781481
anchor, LocatorPathElt::SynthesizedArgument(i));
1479-
args.push_back(params[i].withType(cs.createTypeVariable(argLoc,
1480-
TVO_CanBindToNoEscape)));
1482+
args.push_back(params[i].withType(
1483+
cs.createTypeVariable(argLoc, TVO_CanBindToNoEscape)));
14811484
}
14821485

1483-
ArrayRef<AnyFunctionType::Param> argsRef(args);
1484-
auto *fix = AddMissingArguments::create(cs, argsRef.take_back(numMissing),
1486+
SmallVector<std::pair<unsigned, AnyFunctionType::Param>, 4> synthesizedArgs;
1487+
synthesizedArgs.reserve(numMissing);
1488+
for (unsigned i = args.size() - numMissing, n = args.size(); i != n; ++i) {
1489+
synthesizedArgs.push_back(std::make_pair(i, args[i]));
1490+
}
1491+
1492+
auto *fix = AddMissingArguments::create(cs, synthesizedArgs,
14851493
cs.getConstraintLocator(locator));
14861494

14871495
if (cs.recordFix(fix))
@@ -3564,9 +3572,9 @@ bool ConstraintSystem::repairFailures(
35643572
// But if `T.Element` didn't get resolved to `Void` we'd like
35653573
// to diagnose this as a missing argument which can't be ignored.
35663574
if (arg != getTypeVariables().end()) {
3567-
conversionsOrFixes.push_back(
3568-
AddMissingArguments::create(*this, {FunctionType::Param(*arg)},
3569-
getConstraintLocator(anchor, path)));
3575+
conversionsOrFixes.push_back(AddMissingArguments::create(
3576+
*this, {std::make_pair(0, AnyFunctionType::Param(*arg))},
3577+
getConstraintLocator(anchor, path)));
35703578
break;
35713579
}
35723580

0 commit comments

Comments
 (0)