Skip to content

Commit b0c2dd8

Browse files
xedinxedin
authored
Merge pull request #65950 from xedin/more-variadic-sema-improvements-5.9
[5.9][Sema] Improvements to variadic generics inference and diagnostics
2 parents 57cc0c1 + 2eff1cd commit b0c2dd8

19 files changed

+390
-137
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5435,6 +5435,9 @@ ERROR(pack_reference_must_be_in_expansion,none,
54355435
ERROR(pack_type_requires_keyword_each,none,
54365436
"type pack %0 must be referenced with 'each'",
54375437
(TypeRepr*))
5438+
ERROR(value_pack_requires_keyword_each,none,
5439+
"value pack %0 must be referenced with 'each'",
5440+
(Type))
54385441
ERROR(tuple_duplicate_label,none,
54395442
"cannot create a tuple with a duplicate element label", ())
54405443
ERROR(multiple_ellipsis_in_tuple,none,

‎include/swift/AST/Expr.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3627,8 +3627,6 @@ class PackExpansionExpr final : public Expr {
36273627
PatternExpr = patternExpr;
36283628
}
36293629

3630-
void getExpandedPacks(SmallVectorImpl<ASTNode> &packs);
3631-
36323630
GenericEnvironment *getGenericEnvironment() {
36333631
return Environment;
36343632
}

‎include/swift/Sema/CSFix.h

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,9 @@ enum class FixKind : uint8_t {
441441

442442
/// Allow value pack expansion without pack references.
443443
AllowValueExpansionWithoutPackReferences,
444+
445+
/// Ignore missing 'each' keyword before value pack reference.
446+
IgnoreMissingEachKeyword,
444447
};
445448

446449
class ConstraintFix {
@@ -3481,6 +3484,33 @@ class AllowValueExpansionWithoutPackReferences final : public ConstraintFix {
34813484
}
34823485
};
34833486

3487+
class IgnoreMissingEachKeyword final : public ConstraintFix {
3488+
Type ValuePackType;
3489+
3490+
IgnoreMissingEachKeyword(ConstraintSystem &cs, Type valuePackTy,
3491+
ConstraintLocator *locator)
3492+
: ConstraintFix(cs, FixKind::IgnoreMissingEachKeyword, locator),
3493+
ValuePackType(valuePackTy) {}
3494+
3495+
public:
3496+
std::string getName() const override {
3497+
return "allow value pack reference without 'each'";
3498+
}
3499+
3500+
bool diagnose(const Solution &solution, bool asNote = false) const override;
3501+
3502+
bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override {
3503+
return diagnose(*commonFixes.front().first);
3504+
}
3505+
3506+
static IgnoreMissingEachKeyword *
3507+
create(ConstraintSystem &cs, Type valuePackTy, ConstraintLocator *locator);
3508+
3509+
static bool classof(const ConstraintFix *fix) {
3510+
return fix->getKind() == FixKind::IgnoreMissingEachKeyword;
3511+
}
3512+
};
3513+
34843514
} // end namespace constraints
34853515
} // end namespace swift
34863516

‎lib/AST/Expr.cpp

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1279,46 +1279,6 @@ PackExpansionExpr::create(ASTContext &ctx, SourceLoc repeatLoc,
12791279
implicit, type);
12801280
}
12811281

1282-
void PackExpansionExpr::getExpandedPacks(SmallVectorImpl<ASTNode> &packs) {
1283-
struct PackCollector : public ASTWalker {
1284-
llvm::SmallVector<ASTNode, 2> packs;
1285-
1286-
/// Walk everything that's available.
1287-
MacroWalking getMacroWalkingBehavior() const override {
1288-
return MacroWalking::ArgumentsAndExpansion;
1289-
}
1290-
1291-
virtual PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
1292-
// Don't walk into nested pack expansions
1293-
if (isa<PackExpansionExpr>(E)) {
1294-
return Action::SkipChildren(E);
1295-
}
1296-
1297-
if (isa<PackElementExpr>(E)) {
1298-
packs.push_back(E);
1299-
}
1300-
1301-
return Action::Continue(E);
1302-
}
1303-
1304-
virtual PreWalkAction walkToTypeReprPre(TypeRepr *T) override {
1305-
// Don't walk into nested pack expansions
1306-
if (isa<PackExpansionTypeRepr>(T)) {
1307-
return Action::SkipChildren();
1308-
}
1309-
1310-
if (isa<PackElementTypeRepr>(T)) {
1311-
packs.push_back(T);
1312-
}
1313-
1314-
return Action::Continue();
1315-
}
1316-
} packCollector;
1317-
1318-
getPatternExpr()->walk(packCollector);
1319-
packs.append(packCollector.packs.begin(), packCollector.packs.end());
1320-
}
1321-
13221282
PackElementExpr *
13231283
PackElementExpr::create(ASTContext &ctx, SourceLoc eachLoc, Expr *packRefExpr,
13241284
bool implicit, Type type) {

‎lib/Sema/CSDiagnostics.cpp

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "swift/AST/DiagnosticsClangImporter.h"
2525
#include "swift/AST/ExistentialLayout.h"
2626
#include "swift/AST/Expr.h"
27+
#include "swift/AST/GenericEnvironment.h"
2728
#include "swift/AST/GenericSignature.h"
2829
#include "swift/AST/ImportCache.h"
2930
#include "swift/AST/Initializer.h"
@@ -88,6 +89,54 @@ Type FailureDiagnostic::getRawType(ASTNode node) const {
8889
return S.getType(node);
8990
}
9091

92+
Type FailureDiagnostic::resolveType(Type rawType, bool reconstituteSugar,
93+
bool wantRValue) const {
94+
rawType = rawType.transform([&](Type type) -> Type {
95+
if (auto *typeVar = type->getAs<TypeVariableType>()) {
96+
auto resolvedType = S.simplifyType(typeVar);
97+
98+
if (!resolvedType->hasUnresolvedType())
99+
return resolvedType;
100+
101+
// If type variable was simplified to an unresolved pack expansion
102+
// type, let's examine its original pattern type because it could
103+
// contain type variables replaceable with their generic parameter
104+
// types.
105+
if (auto *expansion = resolvedType->getAs<PackExpansionType>()) {
106+
auto *locator = typeVar->getImpl().getLocator();
107+
auto *openedExpansionTy =
108+
locator->castLastElementTo<LocatorPathElt::PackExpansionType>()
109+
.getOpenedType();
110+
auto patternType = resolveType(openedExpansionTy->getPatternType());
111+
return PackExpansionType::get(patternType, expansion->getCountType());
112+
}
113+
114+
Type GP = typeVar->getImpl().getGenericParameter();
115+
return resolvedType->is<UnresolvedType>() && GP ? GP : resolvedType;
116+
}
117+
118+
if (type->hasElementArchetype()) {
119+
auto *env = getDC()->getGenericEnvironmentOfContext();
120+
return env->mapElementTypeIntoPackContext(type);
121+
}
122+
123+
if (auto *packType = type->getAs<PackType>()) {
124+
if (packType->getNumElements() == 1) {
125+
auto eltType = resolveType(packType->getElementType(0));
126+
if (auto expansion = eltType->getAs<PackExpansionType>())
127+
return expansion->getPatternType();
128+
}
129+
}
130+
131+
return type->isPlaceholder() ? Type(type->getASTContext().TheUnresolvedType)
132+
: type;
133+
});
134+
135+
if (reconstituteSugar)
136+
rawType = rawType->reconstituteSugar(/*recursive*/ true);
137+
return wantRValue ? rawType->getRValueType() : rawType;
138+
}
139+
91140
template <typename... ArgTypes>
92141
InFlightDiagnostic
93142
FailureDiagnostic::emitDiagnostic(ArgTypes &&... Args) const {
@@ -8904,3 +8953,25 @@ bool ValuePackExpansionWithoutPackReferences::diagnoseAsError() {
89048953
emitDiagnostic(diag::value_expansion_not_variadic);
89058954
return true;
89068955
}
8956+
8957+
bool MissingEachForValuePackReference::diagnoseAsError() {
8958+
bool fixItNeedsParens = false;
8959+
// If 'each' is missing form a base of a member reference
8960+
// it has to be wrapped in parens.
8961+
if (auto anchor = getAsExpr(getAnchor())) {
8962+
fixItNeedsParens = isExpr<UnresolvedDotExpr>(findParentExpr(anchor));
8963+
}
8964+
8965+
{
8966+
auto diagnostic = emitDiagnostic(diag::value_pack_requires_keyword_each, ValuePackType);
8967+
if (fixItNeedsParens) {
8968+
auto range = getSourceRange();
8969+
diagnostic.fixItInsert(range.Start, "(each ")
8970+
.fixItInsertAfter(range.End, ")");
8971+
} else {
8972+
diagnostic.fixItInsert(getLoc(), "each ");
8973+
}
8974+
}
8975+
8976+
return true;
8977+
}

‎lib/Sema/CSDiagnostics.h

Lines changed: 22 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -98,48 +98,7 @@ class FailureDiagnostic {
9898

9999
/// Resolve type variables present in the raw type, if any.
100100
Type resolveType(Type rawType, bool reconstituteSugar = false,
101-
bool wantRValue = true) const {
102-
rawType = rawType.transform([&](Type type) -> Type {
103-
if (auto *typeVar = type->getAs<TypeVariableType>()) {
104-
auto resolvedType = S.simplifyType(typeVar);
105-
106-
if (!resolvedType->hasUnresolvedType())
107-
return resolvedType;
108-
109-
// If type variable was simplified to an unresolved pack expansion
110-
// type, let's examine its original pattern type because it could
111-
// contain type variables replaceable with their generic parameter
112-
// types.
113-
if (auto *expansion = resolvedType->getAs<PackExpansionType>()) {
114-
auto *locator = typeVar->getImpl().getLocator();
115-
auto *openedExpansionTy =
116-
locator->castLastElementTo<LocatorPathElt::PackExpansionType>()
117-
.getOpenedType();
118-
auto patternType = resolveType(openedExpansionTy->getPatternType());
119-
return PackExpansionType::get(patternType, expansion->getCountType());
120-
}
121-
122-
Type GP = typeVar->getImpl().getGenericParameter();
123-
return resolvedType->is<UnresolvedType>() && GP ? GP : resolvedType;
124-
}
125-
126-
if (auto *packType = type->getAs<PackType>()) {
127-
if (packType->getNumElements() == 1) {
128-
auto eltType = resolveType(packType->getElementType(0));
129-
if (auto expansion = eltType->getAs<PackExpansionType>())
130-
return expansion->getPatternType();
131-
}
132-
}
133-
134-
return type->isPlaceholder()
135-
? Type(type->getASTContext().TheUnresolvedType)
136-
: type;
137-
});
138-
139-
if (reconstituteSugar)
140-
rawType = rawType->reconstituteSugar(/*recursive*/ true);
141-
return wantRValue ? rawType->getRValueType() : rawType;
142-
}
101+
bool wantRValue = true) const;
143102

144103
template <typename... ArgTypes>
145104
InFlightDiagnostic emitDiagnostic(ArgTypes &&... Args) const;
@@ -3017,6 +2976,27 @@ class ValuePackExpansionWithoutPackReferences final : public FailureDiagnostic {
30172976
bool diagnoseAsError() override;
30182977
};
30192978

2979+
/// Diagnose situations where value pack is referenced without explicit 'each':
2980+
///
2981+
/// \code
2982+
/// func compute<each T>(_: repeat each T) {}
2983+
///
2984+
/// func test<each T>(v: repeat each T) {
2985+
/// repeat compute(v) // should be `repeat compute(each v)`
2986+
/// }
2987+
/// \endcode
2988+
class MissingEachForValuePackReference final : public FailureDiagnostic {
2989+
Type ValuePackType;
2990+
2991+
public:
2992+
MissingEachForValuePackReference(const Solution &solution, Type valuePackTy,
2993+
ConstraintLocator *locator)
2994+
: FailureDiagnostic(solution, locator),
2995+
ValuePackType(resolveType(valuePackTy)) {}
2996+
2997+
bool diagnoseAsError() override;
2998+
};
2999+
30203000
} // end namespace constraints
30213001
} // end namespace swift
30223002

‎lib/Sema/CSFix.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2769,3 +2769,17 @@ AllowValueExpansionWithoutPackReferences::create(ConstraintSystem &cs,
27692769
return new (cs.getAllocator())
27702770
AllowValueExpansionWithoutPackReferences(cs, locator);
27712771
}
2772+
2773+
bool IgnoreMissingEachKeyword::diagnose(const Solution &solution,
2774+
bool asNote) const {
2775+
MissingEachForValuePackReference failure(solution, ValuePackType,
2776+
getLocator());
2777+
return failure.diagnose(asNote);
2778+
}
2779+
2780+
IgnoreMissingEachKeyword *
2781+
IgnoreMissingEachKeyword::create(ConstraintSystem &cs, Type valuePackTy,
2782+
ConstraintLocator *locator) {
2783+
return new (cs.getAllocator())
2784+
IgnoreMissingEachKeyword(cs, valuePackTy, locator);
2785+
}

0 commit comments

Comments
 (0)