Skip to content

Commit fd8e349

Browse files
ahoppenahoppen
authored
Merge pull request #36551 from ahoppen/pr/internal-labels-in-closures
[CodeComplete] Default parameter names of completed closure to internal names
2 parents 6ca5b32 + 865e80f commit fd8e349

18 files changed

+161
-37
lines changed

‎include/swift/AST/PrintOptions.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,12 @@ struct PrintOptions {
371371
/// Whether to print the extensions from conforming protocols.
372372
bool PrintExtensionFromConformingProtocols = false;
373373

374+
/// Whether to always try and print parameter labels. If present, print the
375+
/// external parameter name. Otherwise try printing the internal name as
376+
/// `_ <internalName>`, if an internal name exists. If neither an external nor
377+
/// an internal name exists, only print the parameter's type.
378+
bool AlwaysTryPrintParameterLabels = false;
379+
374380
std::shared_ptr<ShouldPrintChecker> CurrentPrintabilityChecker =
375381
std::make_shared<ShouldPrintChecker>();
376382

‎include/swift/AST/Types.h

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2785,8 +2785,9 @@ class AnyFunctionType : public TypeBase {
27852785
public:
27862786
explicit Param(Type t,
27872787
Identifier l = Identifier(),
2788-
ParameterTypeFlags f = ParameterTypeFlags())
2789-
: Ty(t), Label(l), Flags(f) {
2788+
ParameterTypeFlags f = ParameterTypeFlags(),
2789+
Identifier internalLabel = Identifier())
2790+
: Ty(t), Label(l), InternalLabel(internalLabel), Flags(f) {
27902791
assert(t && "param type must be non-null");
27912792
assert(!t->is<InOutType>() && "set flags instead");
27922793
}
@@ -2796,8 +2797,18 @@ class AnyFunctionType : public TypeBase {
27962797
/// element type.
27972798
Type Ty;
27982799

2799-
// The label associated with the parameter, if any.
2800+
/// The label associated with the parameter, if any.
28002801
Identifier Label;
2802+
2803+
/// The internal label of the parameter, if explicitly specified, otherwise
2804+
/// empty. The internal label is considered syntactic sugar. It is not
2805+
/// considered part of the canonical type and is thus also ignored in \c
2806+
/// operator==.
2807+
/// E.g.
2808+
/// - `name name2: Int` has internal label `name2`
2809+
/// - `_ name2: Int` has internal label `name2`
2810+
/// - `name: Int` has no internal label
2811+
Identifier InternalLabel;
28012812

28022813
/// Parameter specific flags.
28032814
ParameterTypeFlags Flags = {};
@@ -2823,6 +2834,9 @@ class AnyFunctionType : public TypeBase {
28232834

28242835
bool hasLabel() const { return !Label.empty(); }
28252836
Identifier getLabel() const { return Label; }
2837+
2838+
bool hasInternalLabel() const { return !InternalLabel.empty(); }
2839+
Identifier getInternalLabel() const { return InternalLabel; }
28262840

28272841
ParameterTypeFlags getParameterFlags() const { return Flags; }
28282842

@@ -2851,23 +2865,34 @@ class AnyFunctionType : public TypeBase {
28512865
return Flags.getValueOwnership();
28522866
}
28532867

2868+
/// Returns \c true if the two \c Params are equal in their canonicalized
2869+
/// form.
2870+
/// Two \c Params are equal if their external label, flags and
2871+
/// *canonicalized* types match. The internal label and sugar types are
2872+
/// *not* considered for type equality.
28542873
bool operator==(Param const &b) const {
28552874
return (Label == b.Label &&
28562875
getPlainType()->isEqual(b.getPlainType()) &&
28572876
Flags == b.Flags);
28582877
}
28592878
bool operator!=(Param const &b) const { return !(*this == b); }
28602879

2861-
Param getWithoutLabel() const { return Param(Ty, Identifier(), Flags); }
2880+
/// Return the parameter without external and internal labels.
2881+
Param getWithoutLabels() const {
2882+
return Param(Ty, /*Label=*/Identifier(), Flags,
2883+
/*InternalLabel=*/Identifier());
2884+
}
28622885

28632886
Param withLabel(Identifier newLabel) const {
2864-
return Param(Ty, newLabel, Flags);
2887+
return Param(Ty, newLabel, Flags, InternalLabel);
28652888
}
28662889

2867-
Param withType(Type newType) const { return Param(newType, Label, Flags); }
2890+
Param withType(Type newType) const {
2891+
return Param(newType, Label, Flags, InternalLabel);
2892+
}
28682893

28692894
Param withFlags(ParameterTypeFlags flags) const {
2870-
return Param(Ty, Label, flags);
2895+
return Param(Ty, Label, flags, InternalLabel);
28712896
}
28722897
};
28732898

@@ -2988,14 +3013,19 @@ class AnyFunctionType : public TypeBase {
29883013
return composeInput(ctx, params.getOriginalArray(), canonicalVararg);
29893014
}
29903015

2991-
/// Given two arrays of parameters determine if they are equal.
3016+
/// Given two arrays of parameters determine if they are equal in their
3017+
/// canonicalized form. Internal labels and type sugar is *not* taken into
3018+
/// account.
29923019
static bool equalParams(ArrayRef<Param> a, ArrayRef<Param> b);
29933020

2994-
/// Given two arrays of parameters determine if they are equal.
3021+
/// Given two arrays of parameters determine if they are equal in their
3022+
/// canonicalized form. Internal labels and type sugar is *not* taken into
3023+
/// account.
29953024
static bool equalParams(CanParamArrayRef a, CanParamArrayRef b);
29963025

29973026
/// Given an array of parameters and an array of labels of the
29983027
/// same length, update each parameter to have the corresponding label.
3028+
/// The internal parameter labels remain the same.
29993029
static void relabelParams(MutableArrayRef<Param> params,
30003030
ArrayRef<Identifier> labels);
30013031

‎lib/AST/ASTContext.cpp

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3085,11 +3085,15 @@ getFunctionRecursiveProperties(ArrayRef<AnyFunctionType::Param> params,
30853085
}
30863086

30873087
static bool
3088-
isFunctionTypeCanonical(ArrayRef<AnyFunctionType::Param> params,
3088+
isAnyFunctionTypeCanonical(ArrayRef<AnyFunctionType::Param> params,
30893089
Type result) {
30903090
for (auto param : params) {
30913091
if (!param.getPlainType()->isCanonical())
30923092
return false;
3093+
if (!param.getInternalLabel().empty()) {
3094+
// Canonical types don't have internal labels
3095+
return false;
3096+
}
30933097
}
30943098

30953099
return result->isCanonical();
@@ -3128,6 +3132,10 @@ isGenericFunctionTypeCanonical(GenericSignature sig,
31283132
for (auto param : params) {
31293133
if (!sig->isCanonicalTypeInContext(param.getPlainType()))
31303134
return false;
3135+
if (!param.getInternalLabel().empty()) {
3136+
// Canonical types don't have internal labels
3137+
return false;
3138+
}
31313139
}
31323140

31333141
return sig->isCanonicalTypeInContext(result);
@@ -3231,17 +3239,21 @@ void AnyFunctionType::relabelParams(MutableArrayRef<Param> params,
32313239
assert(params.size() == labels.size());
32323240
for (auto i : indices(params)) {
32333241
auto &param = params[i];
3234-
param = AnyFunctionType::Param(param.getPlainType(),
3235-
labels[i],
3236-
param.getParameterFlags());
3242+
param = AnyFunctionType::Param(param.getPlainType(), labels[i],
3243+
param.getParameterFlags(),
3244+
param.getInternalLabel());
32373245
}
32383246
}
32393247

3248+
/// Profile \p params into \p ID. In contrast to \c == on \c Param, the profile
3249+
/// *does* take the internal label into account and *does not* canonicalize
3250+
/// the param's type.
32403251
static void profileParams(llvm::FoldingSetNodeID &ID,
32413252
ArrayRef<AnyFunctionType::Param> params) {
32423253
ID.AddInteger(params.size());
32433254
for (auto param : params) {
32443255
ID.AddPointer(param.getLabel().get());
3256+
ID.AddPointer(param.getInternalLabel().get());
32453257
ID.AddPointer(param.getPlainType().getPointer());
32463258
ID.AddInteger(param.getParameterFlags().toRaw());
32473259
}
@@ -3293,7 +3305,7 @@ FunctionType *FunctionType::get(ArrayRef<AnyFunctionType::Param> params,
32933305
>(params.size(), hasClangInfo ? 1 : 0, globalActor ? 1 : 0);
32943306
void *mem = ctx.Allocate(allocSize, alignof(FunctionType), arena);
32953307

3296-
bool isCanonical = isFunctionTypeCanonical(params, result);
3308+
bool isCanonical = isAnyFunctionTypeCanonical(params, result);
32973309
if (!clangTypeInfo.empty()) {
32983310
if (ctx.LangOpts.UseClangFunctionTypes)
32993311
isCanonical &= clangTypeInfo.getType()->isCanonicalUnqualified();

‎lib/AST/ASTDumper.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3808,6 +3808,8 @@ namespace {
38083808
PrintWithColorRAII(OS, TypeFieldColor) << "param";
38093809
if (param.hasLabel())
38103810
printField("name", param.getLabel().str());
3811+
if (param.hasInternalLabel())
3812+
printField("internal_name", param.getInternalLabel().str());
38113813
dumpParameterFlags(param.getParameterFlags());
38123814
printRec(param.getPlainType());
38133815
OS << ")";

‎lib/AST/ASTPrinter.cpp

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4801,10 +4801,23 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
48014801
Printer.printStructurePost(PrintStructureKind::FunctionParameter);
48024802
};
48034803

4804-
if (printLabels && Param.hasLabel()) {
4804+
if ((Options.AlwaysTryPrintParameterLabels || printLabels) &&
4805+
Param.hasLabel()) {
4806+
// Label printing was requested and we have an external label. Print it
4807+
// and omit the internal label.
48054808
Printer.printName(Param.getLabel(),
48064809
PrintNameContext::FunctionParameterExternal);
48074810
Printer << ": ";
4811+
} else if (Options.AlwaysTryPrintParameterLabels &&
4812+
Param.hasInternalLabel()) {
4813+
// We didn't have an external parameter label but were requested to
4814+
// always try and print parameter labels. Print The internal label.
4815+
// If we have neither an external nor an internal label, only print the
4816+
// type.
4817+
Printer << "_ ";
4818+
Printer.printName(Param.getInternalLabel(),
4819+
PrintNameContext::FunctionParameterLocal);
4820+
Printer << ": ";
48084821
}
48094822

48104823
auto type = Param.getPlainType();

‎lib/AST/Decl.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2622,7 +2622,8 @@ static Type mapSignatureFunctionType(ASTContext &ctx, Type type,
26222622
newFlags = newFlags.withInOut(false);
26232623
}
26242624

2625-
AnyFunctionType::Param newParam(newParamType, param.getLabel(), newFlags);
2625+
AnyFunctionType::Param newParam(newParamType, param.getLabel(), newFlags,
2626+
param.getInternalLabel());
26262627
newParams.push_back(newParam);
26272628
}
26282629

@@ -6335,11 +6336,12 @@ AnyFunctionType::Param ParamDecl::toFunctionParam(Type type) const {
63356336
type = ParamDecl::getVarargBaseTy(type);
63366337

63376338
auto label = getArgumentName();
6339+
auto internalLabel = getParameterName();
63386340
auto flags = ParameterTypeFlags::fromParameterType(
63396341
type, isVariadic(), isAutoClosure(), isNonEphemeral(),
63406342
getValueOwnership(),
63416343
/*isNoDerivative*/ false);
6342-
return AnyFunctionType::Param(type, label, flags);
6344+
return AnyFunctionType::Param(type, label, flags, internalLabel);
63436345
}
63446346

63456347
Optional<Initializer *> ParamDecl::getCachedDefaultArgumentInitContext() const {

‎lib/AST/Type.cpp

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -856,7 +856,7 @@ Type TypeBase::removeArgumentLabels(unsigned numArgumentLabels) {
856856
llvm::SmallVector<AnyFunctionType::Param, 8> unlabeledParams;
857857
unlabeledParams.reserve(fnType->getNumParams());
858858
for (const auto &param : fnType->getParams())
859-
unlabeledParams.push_back(param.getWithoutLabel());
859+
unlabeledParams.push_back(param.getWithoutLabels());
860860

861861
auto result = fnType->getResult()
862862
->removeArgumentLabels(numArgumentLabels - 1);
@@ -1081,9 +1081,7 @@ rebuildSelfTypeWithObjectType(AnyFunctionType::Param selfParam,
10811081
if (selfParam.getPlainType()->getAs<MetatypeType>())
10821082
objectTy = MetatypeType::get(objectTy);
10831083

1084-
return AnyFunctionType::Param(objectTy,
1085-
selfParam.getLabel(),
1086-
selfParam.getParameterFlags());
1084+
return selfParam.withType(objectTy);
10871085
}
10881086

10891087
/// Returns a new function type exactly like this one but with the self
@@ -1258,9 +1256,11 @@ getCanonicalParams(AnyFunctionType *funcType,
12581256
SmallVectorImpl<AnyFunctionType::Param> &canParams) {
12591257
auto origParams = funcType->getParams();
12601258
for (auto param : origParams) {
1259+
// Canonicalize the type and drop the internal label to canonicalize the
1260+
// Param.
12611261
canParams.emplace_back(param.getPlainType()->getCanonicalType(genericSig),
1262-
param.getLabel(),
1263-
param.getParameterFlags());
1262+
param.getLabel(), param.getParameterFlags(),
1263+
/*InternalLabel=*/Identifier());
12641264
}
12651265
}
12661266

@@ -4849,6 +4849,7 @@ case TypeKind::Id:
48494849
auto type = param.getPlainType();
48504850
auto label = param.getLabel();
48514851
auto flags = param.getParameterFlags();
4852+
auto internalLabel = param.getInternalLabel();
48524853

48534854
auto substType = type.transformRec(fn);
48544855
if (!substType)
@@ -4867,7 +4868,7 @@ case TypeKind::Id:
48674868
flags = flags.withInOut(true);
48684869
}
48694870

4870-
substParams.emplace_back(substType, label, flags);
4871+
substParams.emplace_back(substType, label, flags, internalLabel);
48714872
}
48724873

48734874
// Transform result type.

‎lib/IDE/CodeCompletion.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,6 +1001,7 @@ void CodeCompletionResultBuilder::addCallParameter(Identifier Name,
10011001
PO.SkipAttributes = true;
10021002
PO.OpaqueReturnTypePrinting =
10031003
PrintOptions::OpaqueReturnTypePrintingMode::WithoutOpaqueKeyword;
1004+
PO.AlwaysTryPrintParameterLabels = true;
10041005
if (ContextTy)
10051006
PO.setBaseType(ContextTy);
10061007

@@ -1017,6 +1018,8 @@ void CodeCompletionResultBuilder::addCallParameter(Identifier Name,
10171018

10181019
if (param.hasLabel()) {
10191020
OS << param.getLabel();
1021+
} else if (param.hasInternalLabel()) {
1022+
OS << param.getInternalLabel();
10201023
} else {
10211024
OS << "<#";
10221025
if (param.isInOut())
@@ -2337,8 +2340,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
23372340
SmallVector<AnyFunctionType::Param, 8> erasedParams;
23382341
for (const auto &param : genericFuncType->getParams()) {
23392342
auto erasedTy = eraseArchetypes(param.getPlainType(), genericSig);
2340-
erasedParams.emplace_back(erasedTy, param.getLabel(),
2341-
param.getParameterFlags());
2343+
erasedParams.emplace_back(param.withType(erasedTy));
23422344
}
23432345
return GenericFunctionType::get(genericSig,
23442346
erasedParams,

‎lib/SIL/IR/Bridging.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,7 @@ TypeConverter::getBridgedParam(SILFunctionTypeRepresentation rep,
5353
llvm::report_fatal_error("unable to set up the ObjC bridge!");
5454
}
5555

56-
return AnyFunctionType::Param(bridged->getCanonicalType(),
57-
param.getLabel(),
58-
param.getParameterFlags());
56+
return param.withType(bridged->getCanonicalType());
5957
}
6058

6159
void TypeConverter::

‎lib/Sema/CSApply.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,8 +1040,9 @@ namespace {
10401040

10411041
// SILGen knows how to emit property-wrapped parameters, but the
10421042
// function type needs the backing wrapper type in its param list.
1043-
innerParamTypes.push_back(AnyFunctionType::Param(paramRef->getType(),
1044-
innerParam->getArgumentName()));
1043+
innerParamTypes.push_back(AnyFunctionType::Param(
1044+
paramRef->getType(), innerParam->getArgumentName(),
1045+
ParameterTypeFlags(), innerParam->getParameterName()));
10451046
} else {
10461047
// Rewrite the parameter ref if necessary.
10471048
if (outerParam->isInOut()) {

‎lib/Sema/CSDiagnostics.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4471,7 +4471,15 @@ bool MissingArgumentsFailure::diagnoseClosure(const ClosureExpr *closure) {
44714471
fixText += " ";
44724472
interleave(
44734473
funcType->getParams(),
4474-
[&fixText](const AnyFunctionType::Param &param) { fixText += '_'; },
4474+
[&fixText](const AnyFunctionType::Param &param) {
4475+
if (param.hasLabel()) {
4476+
fixText += param.getLabel().str();
4477+
} else if (param.hasInternalLabel()) {
4478+
fixText += param.getInternalLabel().str();
4479+
} else {
4480+
fixText += '_';
4481+
}
4482+
},
44754483
[&fixText] { fixText += ','; });
44764484
fixText += " in ";
44774485
}

‎lib/Sema/CSGen.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2468,7 +2468,7 @@ namespace {
24682468
// Remove parameter labels; they aren't used when matching cases,
24692469
// but outright conflicts will be checked during coercion.
24702470
for (auto &param : params) {
2471-
param = param.getWithoutLabel();
2471+
param = param.getWithoutLabels();
24722472
}
24732473

24742474
Type outputType = CS.createTypeVariable(

‎lib/Sema/ConstraintSystem.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1222,7 +1222,9 @@ unwrapPropertyWrapperParameterTypes(ConstraintSystem &cs, AbstractFunctionDecl *
12221222
auto *wrappedType = cs.createTypeVariable(cs.getConstraintLocator(locator), 0);
12231223
auto paramType = paramTypes[i].getParameterType();
12241224
auto paramLabel = paramTypes[i].getLabel();
1225-
adjustedParamTypes.push_back(AnyFunctionType::Param(wrappedType, paramLabel));
1225+
auto paramInternalLabel = paramTypes[i].getInternalLabel();
1226+
adjustedParamTypes.push_back(AnyFunctionType::Param(
1227+
wrappedType, paramLabel, ParameterTypeFlags(), paramInternalLabel));
12261228
cs.applyPropertyWrapperToParameter(paramType, wrappedType, paramDecl, argLabel,
12271229
ConstraintKind::Equal, locator);
12281230
}

‎lib/Sema/TypeCheckType.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2690,7 +2690,8 @@ TypeResolver::resolveASTFunctionTypeParams(TupleTypeRepr *inputRepr,
26902690
auto paramFlags = ParameterTypeFlags::fromParameterType(
26912691
ty, variadic, autoclosure, /*isNonEphemeral*/ false, ownership,
26922692
noDerivative);
2693-
elements.emplace_back(ty, Identifier(), paramFlags);
2693+
elements.emplace_back(ty, Identifier(), paramFlags,
2694+
inputRepr->getElementName(i));
26942695
}
26952696

26962697
// All non-`@noDerivative` parameters of `@differentiable` function types must

0 commit comments

Comments
 (0)