Sema: New opaque return type circularity check that doesn't trigger lazy type checking of bodies

Commit b70f8a82b1 introduced a usage of
ReplaceOpaqueTypesWithUnderlyingTypes in Sema. Previously this was only
called from SILGen.

The problem was that ReplaceOpaqueTypesWithUnderlyingTypes would call
getUniqueUnderlyingTypeSubstitutions(), which triggers a request to
type check the body of the referenced function.

While this didn't result in unnecessary type checking work, because
UniqueUnderlyingTypeSubstitutionsRequest::evaluate() would skip bodies
in secondary files, it did change declaration checking order.

The specific issue we saw was a bad interaction with associated type
inference and unqualified lookup in a WMO build, and a complete test
case is hard to reduce here.

However, no behavior change is intended with this change, modulo bugs
elsewhere related to declaration checking order, so I feel OK not adding
a test case.

I'll hopefully address the unqualified lookup issue exposed in the
radar soon; it has a reproducer independent of opaque return types.

Fixes rdar://157329046.
This commit is contained in:
Slava Pestov
2025-08-14 16:34:45 -04:00
parent 3dcadf8bb0
commit e26034b7ac
5 changed files with 76 additions and 72 deletions

View File

@@ -27,7 +27,6 @@
#include "swift/AST/DiagnosticsSema.h"
#include "swift/AST/ExistentialLayout.h"
#include "swift/AST/Expr.h"
#include "swift/AST/InFlightSubstitution.h"
#include "swift/AST/NameLookup.h"
#include "swift/AST/NameLookupRequests.h"
#include "swift/AST/Pattern.h"
@@ -3666,50 +3665,6 @@ public:
}
}
bool isSelfReferencing(const Candidate &candidate) {
auto substitutions = std::get<1>(candidate);
// The underlying type can't be defined recursively
// in terms of the opaque type itself.
for (auto genericParam : OpaqueDecl->getOpaqueGenericParams()) {
auto underlyingType = Type(genericParam).subst(substitutions);
// Look through underlying types of other opaque archetypes known to
// us. This is not something the type checker is allowed to do in
// general, since the intent is that the underlying type is completely
// hidden from view at the type system level. However, here we're
// trying to catch recursive underlying types before we proceed to
// SIL, so we specifically want to erase opaque archetypes just
// for the purpose of this check.
ReplaceOpaqueTypesWithUnderlyingTypes replacer(
OpaqueDecl->getDeclContext(),
ResilienceExpansion::Maximal,
/*isWholeModuleContext=*/false);
InFlightSubstitution IFS(replacer, replacer,
SubstFlags::SubstituteOpaqueArchetypes |
SubstFlags::PreservePackExpansionLevel);
auto simplifiedUnderlyingType = underlyingType.subst(IFS);
auto isSelfReferencing =
(IFS.wasLimitReached() ||
simplifiedUnderlyingType.findIf([&](Type t) -> bool {
if (auto *other = t->getAs<OpaqueTypeArchetypeType>()) {
return other->getDecl() == OpaqueDecl;
}
return false;
}));
if (isSelfReferencing) {
Ctx.Diags.diagnose(std::get<0>(candidate)->getLoc(),
diag::opaque_type_self_referential_underlying_type,
underlyingType);
return true;
}
}
return false;
}
// A single unique underlying substitution.
void finalizeUnique(const Candidate &candidate) {
// If we have one successful candidate, then save it as the underlying
@@ -3808,11 +3763,6 @@ public:
auto candidate =
std::make_tuple(underlyingToOpaque->getSubExpr(), subMap, isUnique);
if (isSelfReferencing(candidate)) {
HasInvalidReturn = true;
return Action::Stop();
}
if (subMap.getRecursiveProperties().hasDynamicSelf()) {
Ctx.Diags.diagnose(E->getLoc(),
diag::opaque_type_cannot_contain_dynamic_self);