Skip to content

Commit a61fe87

Browse files
committed
Add proper source attribution for inlined require failures / console.logs
1 parent 6cce2e5 commit a61fe87

14 files changed

Lines changed: 293 additions & 124 deletions

File tree

packages/cashc/src/compiler.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
computeBytecodeFingerprintWithConstructorArgs,
66
generateSourceMap,
77
generateSourceTags,
8+
generateInlineRanges,
89
optimiseBytecode,
910
optimiseBytecodeOld,
1011
scriptToAsm,
@@ -141,6 +142,7 @@ function compileCode(
141142
traversal.consoleLogs,
142143
traversal.requires,
143144
traversal.sourceTags,
145+
traversal.inlineRanges,
144146
constructorParamLength,
145147
);
146148

@@ -159,6 +161,7 @@ function compileCode(
159161
requires: optimisationResult.requires,
160162
sourceTags: generateSourceTags(optimisationResult.sourceTags) || undefined,
161163
functions: traversal.frames.length > 0 ? traversal.frames : undefined,
164+
inlineRanges: generateInlineRanges(optimisationResult.inlineRanges) || undefined,
162165
};
163166

164167
const fingerprint = computeBytecodeFingerprintWithConstructorArgs(optimisationResult.script, constructorParamLength);

packages/cashc/src/generation/GenerateTargetTraversal.ts

Lines changed: 9 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
OptimiseBytecodeResult,
1515
generateSourceMap,
1616
generateSourceTags,
17+
generateInlineRanges,
1718
parseSourceTags,
1819
FullLocationData,
1920
DebugFrame,
@@ -72,11 +73,12 @@ import {
7273
compileUnaryOp,
7374
} from './utils.js';
7475
import { isNumericType } from '../utils.js';
75-
import { Symbol } from '../ast/SymbolTable.js';
76+
import { collectFunctionCalls, isRecursive, shouldInline } from './inlining.js';
7677
import type { InternalCompilerOptions } from '../compiler.js';
7778

7879
interface InlineRange {
7980
startIp: number;
81+
endIp: number;
8082
frame: DebugFrame;
8183
line: number;
8284
}
@@ -95,7 +97,7 @@ export default class GenerateTargetTraversal extends AstTraversal {
9597
private scopeDepth = 0;
9698
private currentFunction: FunctionDefinitionNode;
9799
private constructorParameterCount: number;
98-
private inlineRanges: InlineRange[] = [];
100+
inlineRanges: InlineRange[] = [];
99101

100102
constructor(private compilerOptions: InternalCompilerOptions) {
101103
super();
@@ -261,6 +263,7 @@ export default class GenerateTargetTraversal extends AstTraversal {
261263
bodyTraversal.consoleLogs,
262264
bodyTraversal.requires,
263265
bodyTraversal.sourceTags,
266+
bodyTraversal.inlineRanges,
264267
0,
265268
);
266269

@@ -284,6 +287,7 @@ export default class GenerateTargetTraversal extends AstTraversal {
284287
sourceFile: node.sourceFile,
285288
logs: optimised.logs,
286289
requires: optimised.requires,
290+
inlineRanges: generateInlineRanges(optimised.inlineRanges) || undefined,
287291
};
288292
}
289293

@@ -748,10 +752,12 @@ export default class GenerateTargetTraversal extends AstTraversal {
748752
node.parameters = this.visitList(node.parameters);
749753

750754
const startIp = this.output.length + this.constructorParameterCount;
755+
const endIp = startIp + symbol.bytecode!.length - 1;
756+
751757
this.emit(symbol.bytecode!, { location: node.location, positionHint: PositionHint.END });
752758

753759
if (symbol.inlinedFrame) {
754-
this.inlineRanges.push({ startIp, frame: symbol.inlinedFrame, line: node.location.start.line });
760+
this.inlineRanges.push({ startIp, endIp, frame: symbol.inlinedFrame, line: node.location.start.line });
755761
}
756762

757763
this.popFromStack(node.parameters.length);
@@ -986,71 +992,3 @@ export default class GenerateTargetTraversal extends AstTraversal {
986992
}
987993
}
988994

989-
const shouldInline = (
990-
symbol: Symbol,
991-
optimisedResult: OptimiseBytecodeResult,
992-
reachableCalls: FunctionCallNode[],
993-
nextFunctionId: number,
994-
compilerOptions: InternalCompilerOptions,
995-
): boolean => {
996-
if (compilerOptions.disableInlining) return false;
997-
if (symbol.functionId !== undefined) return false;
998-
999-
const callCount = reachableCalls.filter((call) => call.identifier.symbol === symbol).length;
1000-
return isWorthInlining(nextFunctionId, optimisedResult.script, callCount);
1001-
};
1002-
1003-
function isWorthInlining(candidateFunctionId: number, bodyScript: Script, callCount: number): boolean {
1004-
const bodyBytes = scriptToBytecode(bodyScript).length;
1005-
const idBytes = scriptToBytecode([encodeInt(BigInt(candidateFunctionId))]).length;
1006-
1007-
const bytesWhenDefined = bodyBytes + idBytes + 1 + callCount * (idBytes + 1);
1008-
const bytesWhenInlined = callCount * bodyBytes;
1009-
1010-
return bytesWhenInlined <= bytesWhenDefined;
1011-
}
1012-
1013-
class FunctionCallCollector extends AstTraversal {
1014-
functionCalls: FunctionCallNode[] = [];
1015-
1016-
visitFunctionCall(node: FunctionCallNode): Node {
1017-
this.functionCalls.push(node);
1018-
node.parameters = this.visitList(node.parameters);
1019-
return node;
1020-
}
1021-
}
1022-
1023-
function collectFunctionCalls(node: Node): FunctionCallNode[] {
1024-
const collector = new FunctionCallCollector();
1025-
collector.visit(node);
1026-
return collector.functionCalls;
1027-
}
1028-
1029-
// The global functions a function's body calls directly (deduplicated). A call targets a global
1030-
// function when its resolved symbol carries a definition node — builtin functions have none.
1031-
function calledFunctions(func: FunctionDefinitionNode): FunctionDefinitionNode[] {
1032-
return collectFunctionCalls(func.body)
1033-
.map((call) => call.identifier.symbol?.definition)
1034-
.filter((definition): definition is FunctionDefinitionNode => definition instanceof FunctionDefinitionNode)
1035-
.filter((definition, index, definitions) => definitions.indexOf(definition) === index);
1036-
}
1037-
1038-
// A function is recursive when it can transitively call itself. Recursive functions can never be
1039-
// inlined — expanding them would never terminate — so they always keep a shared definition.
1040-
function isRecursive(func: FunctionDefinitionNode): boolean {
1041-
return transitiveCalledFunctions(func).includes(func);
1042-
}
1043-
1044-
function transitiveCalledFunctions(func: FunctionDefinitionNode): FunctionDefinitionNode[] {
1045-
const callees: FunctionDefinitionNode[] = [];
1046-
1047-
const visit = (current: FunctionDefinitionNode): void => calledFunctions(current).forEach((callee) => {
1048-
if (callees.includes(callee)) return;
1049-
callees.push(callee);
1050-
visit(callee);
1051-
});
1052-
1053-
visit(func);
1054-
return callees;
1055-
}
1056-
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import {
2+
encodeInt,
3+
OptimiseBytecodeResult,
4+
Script,
5+
scriptToBytecode,
6+
} from '@cashscript/utils';
7+
import { FunctionCallNode, FunctionDefinitionNode, Node } from '../ast/AST.js';
8+
import AstTraversal from '../ast/AstTraversal.js';
9+
import { Symbol } from '../ast/SymbolTable.js';
10+
import type { InternalCompilerOptions } from '../compiler.js';
11+
12+
export const shouldInline = (
13+
symbol: Symbol,
14+
optimisedResult: OptimiseBytecodeResult,
15+
reachableCalls: FunctionCallNode[],
16+
nextFunctionId: number,
17+
compilerOptions: InternalCompilerOptions,
18+
): boolean => {
19+
if (compilerOptions.disableInlining) return false;
20+
if (symbol.functionId !== undefined) return false;
21+
22+
const callCount = reachableCalls.filter((call) => call.identifier.symbol === symbol).length;
23+
return isWorthInlining(nextFunctionId, optimisedResult.script, callCount);
24+
};
25+
26+
function isWorthInlining(candidateFunctionId: number, bodyScript: Script, callCount: number): boolean {
27+
const bodyBytes = scriptToBytecode(bodyScript).length;
28+
const idBytes = scriptToBytecode([encodeInt(BigInt(candidateFunctionId))]).length;
29+
30+
const bytesWhenDefined = bodyBytes + idBytes + 1 + callCount * (idBytes + 1);
31+
const bytesWhenInlined = callCount * bodyBytes;
32+
33+
return bytesWhenInlined <= bytesWhenDefined;
34+
}
35+
36+
class FunctionCallCollector extends AstTraversal {
37+
functionCalls: FunctionCallNode[] = [];
38+
39+
visitFunctionCall(node: FunctionCallNode): Node {
40+
this.functionCalls.push(node);
41+
node.parameters = this.visitList(node.parameters);
42+
return node;
43+
}
44+
}
45+
46+
export function collectFunctionCalls(node: Node): FunctionCallNode[] {
47+
const collector = new FunctionCallCollector();
48+
collector.visit(node);
49+
return collector.functionCalls;
50+
}
51+
52+
export function isRecursive(func: FunctionDefinitionNode): boolean {
53+
return transitiveCalledFunctions(func).includes(func);
54+
}
55+
56+
function transitiveCalledFunctions(func: FunctionDefinitionNode): FunctionDefinitionNode[] {
57+
const callees: FunctionDefinitionNode[] = [];
58+
59+
const visit = (current: FunctionDefinitionNode): void => calledFunctions(current).forEach((callee) => {
60+
if (callees.includes(callee)) return;
61+
callees.push(callee);
62+
visit(callee);
63+
});
64+
65+
visit(func);
66+
return callees;
67+
}
68+
69+
function calledFunctions(func: FunctionDefinitionNode): FunctionDefinitionNode[] {
70+
return collectFunctionCalls(func.body)
71+
.map((call) => call.identifier.symbol?.definition)
72+
.filter((definition): definition is FunctionDefinitionNode => definition instanceof FunctionDefinitionNode)
73+
.filter((definition, index, definitions) => definitions.indexOf(definition) === index);
74+
}

packages/cashc/test/generation/fixtures.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1639,6 +1639,8 @@ export const fixtures: Fixture[] = [
16391639
{ ip: 5, line: 5 },
16401640
],
16411641
sourceMap: '5:16:5:27:1;:::33;:37::38:0;:8::40:1',
1642+
// Both literal pushes were fused into the OP_1ADDs during optimisation; the ranges track them
1643+
inlineRanges: '1:1:ONE;2:2:ONE',
16421644
functions: [
16431645
{
16441646
// The inlined constant is documented as an id-less frame; both of its literal pushes
@@ -1735,8 +1737,9 @@ export const fixtures: Fixture[] = [
17351737
{ ip: 6, line: 9 },
17361738
],
17371739
// The emitted body ops (ips 1-4) and the merged require/log entries above all map to the
1738-
// call site; the function's own lines live on its frame below
1740+
// call site; the function's own lines live on its frame below, tied together by the range
17391741
sourceMap: '9:24:9:25;:16::26:1;;;;:8::33',
1742+
inlineRanges: '1:4:checked',
17401743
functions: [
17411744
{
17421745
// The inlined function is documented as an id-less frame carrying its compiled body

packages/cashc/test/global-definitions.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,7 @@ describe('Inlining and shared definitions', () => {
295295
requires: [expect.objectContaining({ line: 2, message: 'must be positive' })],
296296
}));
297297
expect(artifact.debug?.functions?.[0].id).toBeUndefined();
298+
expect(artifact.debug?.inlineRanges).toMatch(/^\d+:\d+:assertPositive$/);
298299
});
299300

300301
it('attributes debug info spliced into a defined body to the call site within that body', () => {
@@ -321,6 +322,7 @@ describe('Inlining and shared definitions', () => {
321322
line: 7,
322323
message: 'value too large',
323324
}));
325+
expect(artifact.debug?.functions?.[0].inlineRanges).toMatch(/^\d+:\d+:assertSmall$/);
324326
});
325327

326328
it('can disable inlining for callers that need the shared-definition form', () => {

packages/cashscript/src/Errors.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Artifact, RequireStatement, sourceMapToLocationData, Type } from '@cashscript/utils';
2-
import { ResolvedFrame, rootFrame } from './debug-frame.js';
2+
import { ResolvedFrame, resolveInlineAttribution, rootFrame } from './debug-frame.js';
33

44
export class TypeError extends Error {
55
constructor(actual: string, expected: Type) {
@@ -161,10 +161,15 @@ export class FailedRequireError extends FailedTransactionError {
161161
frame?: ResolvedFrame,
162162
) {
163163
const resolvedFrame = frame ?? rootFrame(artifact);
164-
const { statement, lineNumber } = getLocationDataForFrame(resolvedFrame, failingInstructionPointer);
165-
const context = formatFrameContext(resolvedFrame, artifact.contractName, lineNumber);
166164

167-
const baseMessage = `${resolvedFrame.sourceName}:${lineNumber} Require statement failed at input ${inputIndex} ${context}`;
165+
const inline = resolveInlineAttribution(artifact, resolvedFrame, requireStatement, 'requires');
166+
const attributedFrame = inline?.frame ?? resolvedFrame;
167+
const attributedIp = inline?.entry.ip ?? failingInstructionPointer;
168+
169+
const { statement, lineNumber } = getLocationDataForFrame(attributedFrame, attributedIp);
170+
const context = formatFrameContext(attributedFrame, artifact.contractName, lineNumber);
171+
172+
const baseMessage = `${attributedFrame.sourceName}:${lineNumber} Require statement failed at input ${inputIndex} ${context}`;
168173
const baseMessageWithRequireMessage = `${baseMessage} with the following message: ${requireStatement.message}`;
169174
const headline = `${requireStatement.message ? baseMessageWithRequireMessage : baseMessage}.`;
170175

packages/cashscript/src/debug-frame.ts

Lines changed: 70 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { AuthenticationProgramStateCommon, binToHex, encodeAuthenticationInstructions } from '@bitauth/libauth';
2-
import { Artifact, LogEntry, RequireStatement } from '@cashscript/utils';
2+
import { Artifact, DebugEntry, DebugFrame, LogEntry, RequireStatement, parseInlineRanges } from '@cashscript/utils';
33

44
export interface ResolvedFrame {
55
sourceMap: string;
@@ -8,6 +8,7 @@ export interface ResolvedFrame {
88
ipOffset: number;
99
requires: readonly RequireStatement[];
1010
logs: readonly LogEntry[];
11+
inlineRanges?: string;
1112
functionName?: string;
1213
}
1314

@@ -18,6 +19,7 @@ export const rootFrame = (artifact: Artifact): ResolvedFrame => ({
1819
ipOffset: artifact.constructorInputs.length,
1920
requires: artifact.debug?.requires ?? [],
2021
logs: artifact.debug?.logs ?? [],
22+
inlineRanges: artifact.debug?.inlineRanges,
2123
});
2224

2325
export const getActiveBytecode = (step: AuthenticationProgramStateCommon): string =>
@@ -35,13 +37,73 @@ export const resolveFrame = (
3537

3638
if (!frame) return rootFrame(artifact);
3739

40+
return resolveDebugFrame(artifact, frame);
41+
};
42+
43+
const resolveDebugFrame = (artifact: Artifact, frame: DebugFrame): ResolvedFrame => ({
44+
sourceMap: frame.sourceMap,
45+
source: frame.source ?? artifact.source,
46+
sourceName: frame.sourceFile ?? `${artifact.contractName}.cash`,
47+
ipOffset: 0, // function bodies have no constructor-arg prefix; their ips start at 0
48+
requires: frame.requires,
49+
logs: frame.logs,
50+
inlineRanges: frame.inlineRanges,
51+
functionName: frame.sourceFile ? frame.name : undefined,
52+
});
53+
54+
export interface InlineAttribution {
55+
frame: ResolvedFrame; // the inlined callable, resolved like any other frame
56+
entry: DebugEntry; // the callable's own entry (frame-local ip and line)
57+
}
58+
59+
export const resolveInlineAttribution = (
60+
artifact: Artifact,
61+
containerFrame: ResolvedFrame,
62+
entry: DebugEntry,
63+
kind: 'requires' | 'logs',
64+
): InlineAttribution | undefined => {
65+
const range = parseInlineRanges(containerFrame.inlineRanges ?? '')
66+
.find((candidate) => entry.ip >= candidate.startIp && entry.ip <= candidate.endIp);
67+
if (!range) return undefined;
68+
69+
const inlinedFrame = artifact.debug?.functions?.find((candidate) => candidate.name === range.frameName);
70+
if (!inlinedFrame) return undefined;
71+
72+
const frameEntry = findMatchingFrameEntry(containerFrame[kind], inlinedFrame[kind], range, entry);
73+
if (!frameEntry) return undefined;
74+
75+
const frame = resolveDebugFrame(artifact, inlinedFrame);
76+
77+
// The callable may itself contain deeper inlined callables: attribute to the innermost one
78+
return resolveInlineAttribution(artifact, frame, frameEntry, kind) ?? { frame, entry: frameEntry };
79+
};
80+
81+
// A log merged from an inlined callable is attributed to the callable's own source
82+
export const attributeLogEntry = (
83+
artifact: Artifact,
84+
frame: ResolvedFrame,
85+
logEntry: LogEntry,
86+
): { logEntry: LogEntry, sourceName: string } => {
87+
const inline = resolveInlineAttribution(artifact, frame, logEntry, 'logs');
88+
if (!inline) return { logEntry, sourceName: frame.sourceName };
89+
3890
return {
39-
sourceMap: frame.sourceMap,
40-
source: frame.source ?? artifact.source,
41-
sourceName: frame.sourceFile ?? `${artifact.contractName}.cash`,
42-
ipOffset: 0, // function bodies have no constructor-arg prefix; their ips start at 0
43-
requires: frame.requires,
44-
logs: frame.logs,
45-
...(frame.sourceFile !== undefined ? { functionName: frame.name } : {}),
91+
logEntry: { ...logEntry, line: inline.entry.line },
92+
sourceName: inline.frame.sourceName,
4693
};
4794
};
95+
96+
const findMatchingFrameEntry = (
97+
containerEntries: readonly DebugEntry[],
98+
frameEntries: readonly DebugEntry[],
99+
range: { startIp: number, endIp: number },
100+
entry: DebugEntry,
101+
): DebugEntry | undefined => {
102+
const entriesInRange = containerEntries.filter((candidate) => (
103+
candidate.ip >= range.startIp && candidate.ip <= range.endIp
104+
));
105+
106+
const position = entriesInRange.indexOf(entry);
107+
if (position === -1) return undefined;
108+
return frameEntries[position];
109+
};

0 commit comments

Comments
 (0)