Skip to content

Commit 2f2249c

Browse files
committed
RequirementMachine: Fix various RewriteSteps to work when re-contextualized
When a rewrite rule is replaced with a path containing ::Adjust, ::Decompose, ::ConcreteConformance or ::SuperclassConformance rewrite steps, the steps will get a non-zero EndOffset if the original rule appears in a step with a non-zero EndOffset. For this reason, these steps must work with a non-zero EndOffset, which primarily means computing correct offsets into the term being manipulated.
1 parent 044611d commit 2f2249c

File tree

5 files changed

+64
-27
lines changed

5 files changed

+64
-27
lines changed

lib/AST/RequirementMachine/GeneratingConformances.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -549,9 +549,9 @@ void RewriteSystem::verifyGeneratingConformanceEquations(
549549

550550
if (baseTerm != otherTerm) {
551551
llvm::errs() << "Invalid equation: ";
552-
llvm::errs() << "\n";
553552
dumpGeneratingConformanceEquation(llvm::errs(),
554553
pair.first, pair.second);
554+
llvm::errs() << "\n";
555555
llvm::errs() << "Invalid conformance path:\n";
556556
llvm::errs() << "Expected: " << baseTerm << "\n";
557557
llvm::errs() << "Got: " << otherTerm << "\n\n";

lib/AST/RequirementMachine/KnuthBendix.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,8 @@ RewriteSystem::computeCriticalPair(ArrayRef<Symbol>::const_iterator from,
454454
if (xv.back().hasSubstitutions() &&
455455
!xv.back().getSubstitutions().empty() &&
456456
t.size() > 0) {
457-
path.add(RewriteStep::forAdjustment(t.size(), /*inverse=*/true));
457+
path.add(RewriteStep::forAdjustment(t.size(), /*endOffset=*/0,
458+
/*inverse=*/true));
458459

459460
xv.back() = xv.back().prependPrefixToConcreteSubstitutions(
460461
t, Context);

lib/AST/RequirementMachine/PropertyUnification.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -762,8 +762,8 @@ void PropertyMap::recordConcreteConformanceRule(
762762
unsigned adjustment = rhs.size() - concreteRule.getRHS().size();
763763
if (adjustment > 0 &&
764764
!concreteSymbol.getSubstitutions().empty()) {
765-
// FIXME: This needs an endOffset!
766-
path.add(RewriteStep::forAdjustment(adjustment, /*inverse=*/false));
765+
path.add(RewriteStep::forAdjustment(adjustment, /*endOffset=*/1,
766+
/*inverse=*/false));
767767

768768
concreteSymbol = concreteSymbol.prependPrefixToConcreteSubstitutions(
769769
MutableTerm(rhs.begin(), rhs.begin() + adjustment),

lib/AST/RequirementMachine/RewriteLoop.cpp

Lines changed: 53 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,14 @@ void RewriteStep::dump(llvm::raw_ostream &out,
4040
break;
4141
}
4242
case AdjustConcreteType: {
43-
auto result = evaluator.applyAdjustment(*this, system);
43+
auto pair = evaluator.applyAdjustment(*this, system);
4444

4545
out << "";
4646
out << (Inverse ? " - " : " + ");
47-
out << result << ")";
47+
out << pair.first << ")";
48+
49+
if (!pair.second.empty())
50+
out << "." << pair.second;
4851

4952
break;
5053
}
@@ -186,7 +189,7 @@ RewritePathEvaluator::applyRewriteRule(const RewriteStep &step,
186189
return {lhs, rhs, prefix, suffix};
187190
}
188191

189-
MutableTerm
192+
std::pair<MutableTerm, MutableTerm>
190193
RewritePathEvaluator::applyAdjustment(const RewriteStep &step,
191194
const RewriteSystem &system) {
192195
auto &term = getCurrentTerm();
@@ -196,9 +199,19 @@ RewritePathEvaluator::applyAdjustment(const RewriteStep &step,
196199
auto &ctx = system.getRewriteContext();
197200
MutableTerm prefix(term.begin() + step.StartOffset,
198201
term.begin() + step.StartOffset + step.RuleID);
202+
MutableTerm suffix(term.end() - step.EndOffset - 1, term.end());
199203

200204
// We're either adding or removing the prefix to each concrete substitution.
201-
term.back() = term.back().transformConcreteSubstitutions(
205+
Symbol &last = *(term.end() - step.EndOffset - 1);
206+
if (!last.hasSubstitutions()) {
207+
llvm::errs() << "Invalid rewrite path\n";
208+
llvm::errs() << "- Term: " << term << "\n";
209+
llvm::errs() << "- Start offset: " << step.StartOffset << "\n";
210+
llvm::errs() << "- End offset: " << step.EndOffset << "\n";
211+
abort();
212+
}
213+
214+
last = last.transformConcreteSubstitutions(
202215
[&](Term t) -> Term {
203216
if (step.Inverse) {
204217
if (!std::equal(t.begin(),
@@ -208,6 +221,7 @@ RewritePathEvaluator::applyAdjustment(const RewriteStep &step,
208221
llvm::errs() << "- Term: " << term << "\n";
209222
llvm::errs() << "- Substitution: " << t << "\n";
210223
llvm::errs() << "- Start offset: " << step.StartOffset << "\n";
224+
llvm::errs() << "- End offset: " << step.EndOffset << "\n";
211225
llvm::errs() << "- Expected subterm: " << prefix << "\n";
212226
abort();
213227
}
@@ -221,7 +235,7 @@ RewritePathEvaluator::applyAdjustment(const RewriteStep &step,
221235
}
222236
}, ctx);
223237

224-
return prefix;
238+
return std::make_pair(prefix, suffix);
225239
}
226240

227241
void RewritePathEvaluator::applyShift(const RewriteStep &step,
@@ -247,16 +261,15 @@ void RewritePathEvaluator::applyShift(const RewriteStep &step,
247261
void RewritePathEvaluator::applyDecompose(const RewriteStep &step,
248262
const RewriteSystem &system) {
249263
assert(step.Kind == RewriteStep::Decompose);
250-
assert(step.EndOffset == 0);
251264

252265
auto &ctx = system.getRewriteContext();
253266
unsigned numSubstitutions = step.RuleID;
254267

255268
if (!step.Inverse) {
256-
// The top of the A stack must be a term ending with a superclass or
257-
// concrete type symbol.
269+
// The input term takes the form U.[concrete: C].V or U.[superclass: C].V,
270+
// where |V| == EndOffset.
258271
const auto &term = getCurrentTerm();
259-
auto symbol = term.back();
272+
auto symbol = *(term.end() - step.EndOffset - 1);
260273
if (!symbol.hasSubstitutions()) {
261274
llvm::errs() << "Expected term with superclass or concrete type symbol"
262275
<< " on A stack\n";
@@ -277,7 +290,8 @@ void RewritePathEvaluator::applyDecompose(const RewriteStep &step,
277290
}
278291
} else {
279292
// The A stack must store the number of substitutions, together with a
280-
// term ending with a superclass or concrete type symbol.
293+
// term that takes the form U.[concrete: C].V or U.[superclass: C].V,
294+
// where |V| == EndOffset.
281295
if (A.size() < numSubstitutions + 1) {
282296
llvm::errs() << "Not enough terms on A stack\n";
283297
dump(llvm::errs());
@@ -287,7 +301,8 @@ void RewritePathEvaluator::applyDecompose(const RewriteStep &step,
287301
// The term immediately underneath the substitutions is the one we're
288302
// updating with new substitutions.
289303
auto &term = *(A.end() - numSubstitutions - 1);
290-
auto symbol = term.back();
304+
305+
auto &symbol = *(term.end() - step.EndOffset - 1);
291306
if (!symbol.hasSubstitutions()) {
292307
llvm::errs() << "Expected term with superclass or concrete type symbol"
293308
<< " on A stack\n";
@@ -312,7 +327,7 @@ void RewritePathEvaluator::applyDecompose(const RewriteStep &step,
312327
}
313328

314329
// Build the new symbol with the new substitutions.
315-
term.back() = symbol.withConcreteSubstitutions(substitutions, ctx);
330+
symbol = symbol.withConcreteSubstitutions(substitutions, ctx);
316331

317332
// Pop the substitutions from the A stack.
318333
A.resize(A.size() - numSubstitutions);
@@ -324,16 +339,23 @@ RewritePathEvaluator::applyConcreteConformance(const RewriteStep &step,
324339
const RewriteSystem &system) {
325340
checkA();
326341
auto &term = A.back();
342+
Symbol *last = term.end() - step.EndOffset;
327343

328344
auto &ctx = system.getRewriteContext();
329345

330346
if (!step.Inverse) {
331-
assert(term.size() > 2);
332-
auto concreteType = *(term.end() - 2);
333-
auto proto = *(term.end() - 1);
347+
// The input term takes one of the following forms, where |V| == EndOffset:
348+
// - U.[concrete: C].[P].V
349+
// - U.[superclass: C].[P].V
350+
assert(term.size() > step.EndOffset + 2);
351+
auto concreteType = *(last - 2);
352+
auto proto = *(last - 1);
334353
assert(proto.getKind() == Symbol::Kind::Protocol);
335354

336-
MutableTerm newTerm(term.begin(), term.end() - 2);
355+
// Get the prefix U.
356+
MutableTerm newTerm(term.begin(), last - 2);
357+
358+
// Build the term U.[concrete: C : P].
337359
if (step.Kind == RewriteStep::ConcreteConformance) {
338360
assert(concreteType.getKind() == Symbol::Kind::ConcreteType);
339361

@@ -353,14 +375,21 @@ RewritePathEvaluator::applyConcreteConformance(const RewriteStep &step,
353375
ctx));
354376
}
355377

378+
// Add the suffix V to get the final term U.[concrete: C : P].V.
379+
newTerm.append(last, term.end());
356380
term = newTerm;
357381
} else {
358-
assert(term.size() > 1);
359-
auto concreteConformance = term.back();
382+
// The input term takes the form U.[concrete: C : P].V, where
383+
// |V| == EndOffset.
384+
assert(term.size() > step.EndOffset + 1);
385+
auto concreteConformance = *(last - 1);
360386
assert(concreteConformance.getKind() == Symbol::Kind::ConcreteConformance);
361387

362-
MutableTerm newTerm(term.begin(), term.end() - 1);
388+
// Build the term U.
389+
MutableTerm newTerm(term.begin(), last - 1);
363390

391+
// Add the symbol [concrete: C] or [superclass: C] to get the term
392+
// U.[concrete: C] or U.[superclass: C].
364393
if (step.Kind == RewriteStep::ConcreteConformance) {
365394
newTerm.add(Symbol::forConcreteType(
366395
concreteConformance.getConcreteType(),
@@ -374,10 +403,15 @@ RewritePathEvaluator::applyConcreteConformance(const RewriteStep &step,
374403
ctx));
375404
}
376405

406+
// Add the symbol [P] to get the term U.[concrete: C].[P] or
407+
// U.[superclass: C].[P].
377408
newTerm.add(Symbol::forProtocol(
378409
concreteConformance.getProtocol(),
379410
ctx));
380411

412+
// Add the suffix V to get the final term U.[concrete: C].[P].V or
413+
// U.[superclass: C].[P].V.
414+
newTerm.append(last, term.end());
381415
term = newTerm;
382416
}
383417
}

lib/AST/RequirementMachine/RewriteLoop.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,9 @@ struct RewriteStep {
139139
return RewriteStep(ApplyRewriteRule, startOffset, endOffset, ruleID, inverse);
140140
}
141141

142-
static RewriteStep forAdjustment(unsigned offset, bool inverse) {
143-
return RewriteStep(AdjustConcreteType, /*startOffset=*/0, /*endOffset=*/0,
142+
static RewriteStep forAdjustment(unsigned offset, unsigned endOffset,
143+
bool inverse) {
144+
return RewriteStep(AdjustConcreteType, /*startOffset=*/0, endOffset,
144145
/*ruleID=*/offset, inverse);
145146
}
146147

@@ -317,8 +318,9 @@ struct RewritePathEvaluator {
317318
AppliedRewriteStep applyRewriteRule(const RewriteStep &step,
318319
const RewriteSystem &system);
319320

320-
MutableTerm applyAdjustment(const RewriteStep &step,
321-
const RewriteSystem &system);
321+
std::pair<MutableTerm, MutableTerm>
322+
applyAdjustment(const RewriteStep &step,
323+
const RewriteSystem &system);
322324

323325
void applyShift(const RewriteStep &step,
324326
const RewriteSystem &system);

0 commit comments

Comments
 (0)