Skip to content

Commit bb4f61e

Browse files
authored
Merge pull request #83141 from slavapestov/fix-rdar82992151
Sema: Improve the infinite opaque return type check
2 parents 5d98ca6 + 4eaa7e3 commit bb4f61e

File tree

15 files changed

+468
-276
lines changed

15 files changed

+468
-276
lines changed

include/swift/AST/InFlightSubstitution.h

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,21 +23,36 @@
2323
#define SWIFT_AST_INFLIGHTSUBSTITUTION_H
2424

2525
#include "swift/AST/SubstitutionMap.h"
26+
#include "llvm/ADT/DenseMap.h"
2627

2728
namespace swift {
2829
class SubstitutionMap;
2930

3031
class InFlightSubstitution {
31-
SubstOptions Options;
32+
friend class SubstitutionMap;
33+
3234
TypeSubstitutionFn BaselineSubstType;
3335
LookupConformanceFn BaselineLookupConformance;
36+
SubstOptions Options;
3437
RecursiveTypeProperties Props;
38+
unsigned RemainingCount : 15;
39+
unsigned InitLimit : 1;
40+
unsigned RemainingDepth : 15;
41+
unsigned LimitReached : 1;
3542

3643
struct ActivePackExpansion {
3744
bool isSubstExpansion = false;
3845
unsigned expansionIndex = 0;
3946
};
40-
SmallVector<ActivePackExpansion, 4> ActivePackExpansions;
47+
llvm::SmallVector<ActivePackExpansion, 4> ActivePackExpansions;
48+
llvm::SmallDenseMap<SubstitutionMap, SubstitutionMap, 2> SubMaps;
49+
50+
Type projectLaneFromPackType(
51+
Type substType, unsigned level);
52+
ProtocolConformanceRef projectLaneFromPackConformance(
53+
PackConformance *substPackConf, unsigned level);
54+
55+
bool checkLimits(Type ty);
4156

4257
public:
4358
InFlightSubstitution(TypeSubstitutionFn substType,
@@ -145,6 +160,10 @@ class InFlightSubstitution {
145160

146161
/// Is the given type invariant to substitution?
147162
bool isInvariant(Type type) const;
163+
164+
bool wasLimitReached() const {
165+
return LimitReached;
166+
}
148167
};
149168

150169
/// A helper classes that provides stable storage for the query

include/swift/AST/SubstitutionMap.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ class SubstitutionMap {
217217

218218
/// Whether to dump the full substitution map, or just a minimal useful subset
219219
/// (on a single line).
220-
enum class DumpStyle { Minimal, Full };
220+
enum class DumpStyle { Minimal, NoConformances, Full };
221221
/// Dump the contents of this substitution map for debugging purposes.
222222
void dump(llvm::raw_ostream &out, DumpStyle style = DumpStyle::Full,
223223
unsigned indent = 0) const;

include/swift/AST/Types.h

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7077,24 +7077,16 @@ enum class OpaqueSubstitutionKind {
70777077
/// archetypes with underlying types visible at a given resilience expansion
70787078
/// to their underlying types.
70797079
class ReplaceOpaqueTypesWithUnderlyingTypes {
7080-
public:
7081-
using SeenDecl = std::pair<OpaqueTypeDecl *, SubstitutionMap>;
70827080
private:
70837081
ResilienceExpansion contextExpansion;
70847082
llvm::PointerIntPair<const DeclContext *, 1, bool> inContextAndIsWholeModule;
7085-
llvm::DenseSet<SeenDecl> *seenDecls;
70867083

70877084
public:
70887085
ReplaceOpaqueTypesWithUnderlyingTypes(const DeclContext *inContext,
70897086
ResilienceExpansion contextExpansion,
70907087
bool isWholeModuleContext)
70917088
: contextExpansion(contextExpansion),
7092-
inContextAndIsWholeModule(inContext, isWholeModuleContext),
7093-
seenDecls(nullptr) {}
7094-
7095-
ReplaceOpaqueTypesWithUnderlyingTypes(
7096-
const DeclContext *inContext, ResilienceExpansion contextExpansion,
7097-
bool isWholeModuleContext, llvm::DenseSet<SeenDecl> &seen);
7089+
inContextAndIsWholeModule(inContext, isWholeModuleContext) {}
70987090

70997091
/// TypeSubstitutionFn
71007092
Type operator()(SubstitutableType *maybeOpaqueType) const;

include/swift/Basic/LangOptions.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,14 @@ namespace swift {
612612
/// rewrite system.
613613
bool EnableRequirementMachineOpaqueArchetypes = false;
614614

615+
/// Maximum nesting depth for type substitution operations, to prevent
616+
/// runaway recursion.
617+
unsigned MaxSubstitutionDepth = 50;
618+
619+
/// Maximum step count for type substitution operations, to prevent
620+
/// runaway recursion.
621+
unsigned MaxSubstitutionCount = 2000;
622+
615623
/// Enable implicit lifetime dependence for ~Escapable return types.
616624
bool EnableExperimentalLifetimeDependenceInference = false;
617625

include/swift/Option/FrontendOptions.td

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,14 @@ def disable_requirement_machine_reuse : Flag<["-"], "disable-requirement-machine
474474
def enable_requirement_machine_opaque_archetypes : Flag<["-"], "enable-requirement-machine-opaque-archetypes">,
475475
HelpText<"Enable more correct opaque archetype support, which is off by default because it might fail to produce a convergent rewrite system">;
476476

477+
def max_substitution_depth : Joined<["-"], "max-substitution-depth=">,
478+
Flags<[FrontendOption, HelpHidden, DoesNotAffectIncrementalBuild]>,
479+
HelpText<"Set the maximum nesting depth for type substitution operations">;
480+
481+
def max_substitution_count : Joined<["-"], "max-substitution-count=">,
482+
Flags<[FrontendOption, HelpHidden, DoesNotAffectIncrementalBuild]>,
483+
HelpText<"Set the maximum step count for type substitution operations">;
484+
477485
def dump_type_witness_systems : Flag<["-"], "dump-type-witness-systems">,
478486
HelpText<"Enables dumping type witness systems from associated type inference">;
479487

lib/AST/ASTDumper.cpp

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1255,13 +1255,19 @@ namespace {
12551255

12561256
/// Print a substitution map as a child node.
12571257
void printRec(SubstitutionMap map, Label label) {
1258+
printRec(map, SubstitutionMap::DumpStyle::Full, label);
1259+
}
1260+
1261+
/// Print a substitution map as a child node.
1262+
void printRec(SubstitutionMap map, SubstitutionMap::DumpStyle style,
1263+
Label label) {
12581264
SmallPtrSet<const ProtocolConformance *, 4> Dumped;
1259-
printRec(map, Dumped, label);
1265+
printRec(map, style, Dumped, label);
12601266
}
12611267

12621268
/// Print a substitution map as a child node.
1263-
void printRec(SubstitutionMap map, VisitedConformances &visited,
1264-
Label label);
1269+
void printRec(SubstitutionMap map, SubstitutionMap::DumpStyle style,
1270+
VisitedConformances &visited, Label label);
12651271

12661272
/// Print a substitution map as a child node.
12671273
void printRec(const ProtocolConformanceRef &conf,
@@ -5807,7 +5813,8 @@ class PrintConformance : public PrintBase {
58075813
if (!shouldPrintDetails)
58085814
break;
58095815

5810-
printRec(conf->getSubstitutionMap(), visited,
5816+
printRec(conf->getSubstitutionMap(),
5817+
SubstitutionMap::DumpStyle::Full, visited,
58115818
Label::optional("substitutions"));
58125819
if (auto condReqs = conf->getConditionalRequirementsIfAvailableOrCached(/*computeIfPossible=*/false)) {
58135820
printList(*condReqs, [&](auto subReq, Label label) {
@@ -5906,7 +5913,7 @@ class PrintConformance : public PrintBase {
59065913

59075914
// A minimal dump doesn't need the details about the conformances, a lot of
59085915
// that info can be inferred from the signature.
5909-
if (style == SubstitutionMap::DumpStyle::Minimal)
5916+
if (style != SubstitutionMap::DumpStyle::Full)
59105917
return;
59115918

59125919
auto conformances = map.getConformances();
@@ -5925,13 +5932,12 @@ class PrintConformance : public PrintBase {
59255932
}
59265933
};
59275934

5928-
void PrintBase::printRec(SubstitutionMap map, VisitedConformances &visited,
5929-
Label label) {
5935+
void PrintBase::printRec(SubstitutionMap map, SubstitutionMap::DumpStyle style,
5936+
VisitedConformances &visited, Label label) {
59305937
printRecArbitrary(
59315938
[&](Label label) {
59325939
PrintConformance(Writer, MemberLoading)
5933-
.visitSubstitutionMap(map, SubstitutionMap::DumpStyle::Full,
5934-
visited, label);
5940+
.visitSubstitutionMap(map, style, visited, label);
59355941
},
59365942
label);
59375943
}
@@ -6354,7 +6360,9 @@ namespace {
63546360

63556361
printArchetypeCommonRec(T);
63566362
if (!T->getSubstitutions().empty()) {
6357-
printRec(T->getSubstitutions(), Label::optional("substitutions"));
6363+
printRec(T->getSubstitutions(),
6364+
SubstitutionMap::DumpStyle::NoConformances,
6365+
Label::optional("substitutions"));
63586366
}
63596367

63606368
printFoot();

lib/AST/SubstitutionMap.cpp

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,13 @@ SubstitutionMap SubstitutionMap::subst(TypeSubstitutionFn subs,
325325
SubstitutionMap SubstitutionMap::subst(InFlightSubstitution &IFS) const {
326326
if (empty()) return SubstitutionMap();
327327

328+
// FIXME: Get this caching working with pack expansions as well.
329+
if (IFS.ActivePackExpansions.empty()) {
330+
auto found = IFS.SubMaps.find(*this);
331+
if (found != IFS.SubMaps.end())
332+
return found->second;
333+
}
334+
328335
SmallVector<Type, 4> newSubs;
329336
for (Type type : getReplacementTypes()) {
330337
newSubs.push_back(type.subst(IFS));
@@ -345,7 +352,12 @@ SubstitutionMap SubstitutionMap::subst(InFlightSubstitution &IFS) const {
345352
}
346353

347354
assert(oldConformances.empty());
348-
return SubstitutionMap(genericSig, newSubs, newConformances);
355+
auto result = SubstitutionMap(genericSig, newSubs, newConformances);
356+
357+
if (IFS.ActivePackExpansions.empty())
358+
(void) IFS.SubMaps.insert(std::make_pair(*this, result));
359+
360+
return result;
349361
}
350362

351363
SubstitutionMap
@@ -624,9 +636,23 @@ SubstitutionMap swift::substOpaqueTypesWithUnderlyingTypes(
624636
ReplaceOpaqueTypesWithUnderlyingTypes replacer(
625637
context.getContext(), context.getResilienceExpansion(),
626638
context.isWholeModuleContext());
627-
return subs.subst(replacer, replacer,
628-
SubstFlags::SubstituteOpaqueArchetypes |
629-
SubstFlags::PreservePackExpansionLevel);
639+
InFlightSubstitution IFS(replacer, replacer,
640+
SubstFlags::SubstituteOpaqueArchetypes |
641+
SubstFlags::PreservePackExpansionLevel);
642+
643+
auto substSubs = subs.subst(IFS);
644+
645+
if (IFS.wasLimitReached()) {
646+
ABORT([&](auto &out) {
647+
out << "Possible non-terminating type substitution detected\n\n";
648+
out << "Original substitution map:\n";
649+
subs.dump(out, SubstitutionMap::DumpStyle::NoConformances);
650+
out << "Substituted substitution map:\n";
651+
substSubs.dump(out, SubstitutionMap::DumpStyle::NoConformances);
652+
});
653+
}
654+
655+
return substSubs;
630656
}
631657

632658
Type OuterSubstitutions::operator()(SubstitutableType *type) const {

0 commit comments

Comments
 (0)