Skip to content

[CSDiagnostics] Add diagnostics for holes in generic parameter packs #71701

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Mar 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -4460,6 +4460,13 @@ ERROR(cannot_force_unwrap_nil_literal,none,
ERROR(could_not_infer_placeholder,none,
"could not infer type for placeholder", ())

ERROR(could_not_infer_pack_element,none,
"could not infer pack element #%0 from context", (unsigned))

NOTE(note_in_opening_pack_element,none,
"in inferring pack element #%0 of '%1'", (unsigned,StringRef))


ERROR(type_of_expression_is_ambiguous,none,
"type of expression is ambiguous without a type annotation", ())

Expand Down
23 changes: 23 additions & 0 deletions include/swift/Sema/CSFix.h
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,10 @@ enum class FixKind : uint8_t {
/// is declared was not imported.
SpecifyObjectLiteralTypeImport,

/// Type of the element in generic pack couldn't be inferred and has to be
/// specified explicitly.
SpecifyPackElementType,

/// Allow any type (and not just class or class-constrained type) to
/// be convertible to AnyObject.
AllowNonClassTypeToConvertToAnyObject,
Expand Down Expand Up @@ -2737,6 +2741,25 @@ class SpecifyObjectLiteralTypeImport final : public ConstraintFix {
}
};

class SpecifyPackElementType final : public ConstraintFix {
SpecifyPackElementType(ConstraintSystem &cs, ConstraintLocator *locator)
: ConstraintFix(cs, FixKind::SpecifyPackElementType, locator) {}
public:
std::string getName() const override {
return "specify pack element type";
}

bool diagnose(const Solution &solution, bool asNote) const override;

static SpecifyPackElementType *create(ConstraintSystem &cs,
ConstraintLocator *locator);

static bool classof(const ConstraintFix *fix) {
return fix->getKind() == FixKind::SpecifyPackElementType;
}
};


class AddQualifierToAccessTopLevelName final : public ConstraintFix {
AddQualifierToAccessTopLevelName(ConstraintSystem &cs,
ConstraintLocator *locator)
Expand Down
6 changes: 6 additions & 0 deletions lib/Sema/CSBindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2549,6 +2549,12 @@ TypeVariableBinding::fixForHole(ConstraintSystem &cs) const {
return std::make_pair(fix, defaultImpact);
}

if (dstLocator->isLastElement<LocatorPathElt::PackElement>()) {
// A hole appears as an element of generic pack params
ConstraintFix *Fix = SpecifyPackElementType::create(cs, dstLocator);
return std::make_pair(Fix, defaultImpact);
}

return std::nullopt;
}

Expand Down
39 changes: 39 additions & 0 deletions lib/Sema/CSDiagnostics.cpp
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to 0-based indexing since I found other sema diagnostics are doing so.

Original file line number Diff line number Diff line change
Expand Up @@ -8199,6 +8199,45 @@ bool UnableToInferClosureReturnType::diagnoseAsError() {
return true;
}

bool UnableToInferGenericPackElementType::diagnoseAsError() {
auto *locator = getLocator();
auto path = locator->getPath();

auto packElementElt = locator->getLastElementAs<LocatorPathElt::PackElement>();
assert(packElementElt && "Expected path to end with a pack element locator");

if (isExpr<NilLiteralExpr>(getAnchor())) {
// `nil` appears as an element of generic pack params, let's record a
// specify contextual type for nil fix.
emitDiagnostic(diag::unresolved_nil_literal);
} else {
// unable to infer the type of an element of generic pack params
emitDiagnostic(diag::could_not_infer_pack_element,
packElementElt->getIndex());
}

if (isExpr<ApplyExpr>(locator->getAnchor())) {
// emit callee side diagnostics
if (auto *calleeLocator = getSolution().getCalleeLocator(locator)) {
if (const auto choice = getOverloadChoiceIfAvailable(calleeLocator)) {
if (auto *decl = choice->choice.getDeclOrNull()) {
if (auto applyArgToParamElt =
locator->findFirst<LocatorPathElt::ApplyArgToParam>()) {
if (auto paramDecl =
getParameterAt(decl, applyArgToParamElt->getParamIdx())) {
emitDiagnosticAt(
paramDecl->getLoc(), diag::note_in_opening_pack_element,
packElementElt->getIndex(), paramDecl->getNameStr());
}
}
}
}
}
}

return true;
}

static std::pair<StringRef, StringRef>
getImportModuleAndDefaultType(const ASTContext &ctx,
const ObjectLiteralExpr *expr) {
Expand Down
9 changes: 9 additions & 0 deletions lib/Sema/CSDiagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -2343,6 +2343,15 @@ class UnableToInferClosureReturnType final : public FailureDiagnostic {
bool diagnoseAsError() override;
};

class UnableToInferGenericPackElementType final : public FailureDiagnostic {
public:
UnableToInferGenericPackElementType(const Solution &solution,
ConstraintLocator *locator)
: FailureDiagnostic(solution, locator) {}

bool diagnoseAsError() override;
};

class UnableToInferProtocolLiteralType final : public FailureDiagnostic {
public:
UnableToInferProtocolLiteralType(const Solution &solution,
Expand Down
14 changes: 14 additions & 0 deletions lib/Sema/CSFix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1972,6 +1972,20 @@ AllowNonClassTypeToConvertToAnyObject::create(ConstraintSystem &cs, Type type,
AllowNonClassTypeToConvertToAnyObject(cs, type, locator);
}


bool SpecifyPackElementType::diagnose(const Solution &solution,
bool asNote) const {
UnableToInferGenericPackElementType failure(solution, getLocator());
return failure.diagnose(asNote);
}

SpecifyPackElementType *
SpecifyPackElementType::create(ConstraintSystem &cs,
ConstraintLocator *locator) {
return new (cs.getAllocator()) SpecifyPackElementType(cs, locator);
}


bool AddQualifierToAccessTopLevelName::diagnose(const Solution &solution,
bool asNote) const {
MissingQualifierInMemberRefFailure failure(solution, getLocator());
Expand Down
1 change: 1 addition & 0 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15105,6 +15105,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
case FixKind::SpecifyContextualTypeForNil:
case FixKind::AllowRefToInvalidDecl:
case FixKind::SpecifyBaseTypeForOptionalUnresolvedMember:
case FixKind::SpecifyPackElementType:
case FixKind::AllowCheckedCastCoercibleOptionalType:
case FixKind::AllowNoopCheckedCast:
case FixKind::AllowNoopExistentialToCFTypeCheckedCast:
Expand Down
17 changes: 16 additions & 1 deletion lib/Sema/ConstraintSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5883,9 +5883,24 @@ void constraints::simplifyLocator(ASTNode &anchor,
if (!elt)
break;

// If the 3rd element is an PackElement, add the index of pack element
// within packs to locate the correct element.
std::optional<unsigned> eltPackIdx;
if (path.size() > 2) {
if (auto eltPack = path[2].getAs<LocatorPathElt::PackElement>()) {
eltPackIdx = eltPack->getIndex();
}
}

// Extract application argument.
if (auto *args = anchorExpr->getArgs()) {
if (elt->getArgIdx() < args->size()) {
if (eltPackIdx.has_value()) {
if (elt->getArgIdx() + eltPackIdx.value() < args->size()) {
anchor = args->getExpr(elt->getArgIdx() + eltPackIdx.value());
path = path.slice(3);
continue;
}
} else if (elt->getArgIdx() < args->size()) {
anchor = args->getExpr(elt->getArgIdx());
path = path.slice(2);
continue;
Expand Down
3 changes: 3 additions & 0 deletions test/Constraints/pack_expansion_types.swift
Original file line number Diff line number Diff line change
Expand Up @@ -243,8 +243,10 @@ func patternInstantiationConcreteValid() {
func patternInstantiationConcreteInvalid() {
let _: Set<Int> = patternInstantiationTupleTest1()
// expected-error@-1 {{cannot convert value of type '(repeat Array<_>)' to specified type 'Set<Int>'}}
// expected-error@-2 {{could not infer pack element #0 from context}}

let _: (Array<Int>, Set<String>) = patternInstantiationTupleTest1() // expected-error {{'(repeat Array<Int, _>)' is not convertible to '(Array<Int>, Set<String>)', tuples have a different number of elements}}
// expected-error@-1 {{could not infer pack element #1 from context}}
}

func patternInstantiationGenericValid<each T, each U>(t: repeat each T, u: repeat each U)
Expand Down Expand Up @@ -275,6 +277,7 @@ func patternInstantiationGenericInvalid<each T: Hashable>(t: repeat each T) {
// expected-error@-1 {{generic parameter 'each T' could not be inferred}}

let _: (repeat Array<each T>, Set<String>) = patternInstantiationTupleTest1() // expected-error {{'(repeat Array<repeat each T, _>)' is not convertible to '(repeat Array<each T>, Set<String>)', tuples have a different number of elements}}
// expected-error@-1 {{could not infer pack element #1 from context}}
}

// rdar://107996926 - Vanishing metatype of tuple not supported
Expand Down
26 changes: 26 additions & 0 deletions test/Constraints/variadic_generic_functions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,29 @@ do {

func bar<each T>() -> (repeat each T) {}
}


// apple/swift#69432 - Passing nil to a parameter pack fails to produce diagnostic for expression
do {
func foo<each T>(_ value: repeat each T) {} // expected-note {{in inferring pack element #0 of 'value'}}
// expected-note@-1 {{in inferring pack element #0 of 'value'}}
// expected-note@-2 {{in inferring pack element #1 of 'value'}}

foo(nil) // expected-error {{'nil' requires a contextual type}}
foo(nil, 1) // expected-error {{'nil' requires a contextual type}}
foo(2, nil) // expected-error {{'nil' requires a contextual type}}

func bar<each T, U, each W>(_ t: repeat each T, u: U, w: repeat each W) {} // expected-note {{in inferring pack element #2 of 'w'}}
// expected-note@-1 {{in inferring pack element #3 of 't'}}

bar(1, 2, 3, nil, "Hello", u: 3, w: 4, 8, nil) // expected-error {{'nil' requires a contextual type}}
// expected-error@-1 {{'nil' requires a contextual type}}


func fooWithOverload(_ value: Int) {}
func fooWithOverload<each T>(_ value: repeat each T) {}
// expected-note@-1 {{in inferring pack element #4 of 'value'}}

fooWithOverload(0, 1, 2, 3, nil) // expected-error {{'nil' requires a contextual type}}

}
12 changes: 12 additions & 0 deletions test/Constraints/variadic_generic_types.swift
Original file line number Diff line number Diff line change
Expand Up @@ -106,3 +106,15 @@ do {
}
}
}

// apple/swift#69432 - Passing nil to a parameter pack fails to produce diagnostic for expression
do {
struct Foo<each T> {
init(_ value: repeat each T) {}
// expected-note@-1 {{in inferring pack element #0 of 'value'}}
// expected-note@-2 {{in inferring pack element #0 of 'value'}}
}

_ = Foo(nil) // expected-error {{'nil' requires a contextual type}}
_ = Foo(nil, 1) // expected-error {{'nil' requires a contextual type}}
}