Skip to content

Commit 72d1f23

Browse files
authored
improve line number merging for reflection tools (#42247)
We were representing some frames as discontiguous that were clearly the same frame, but the detection logic for it was off-by-one.
1 parent f5ce7c4 commit 72d1f23

File tree

2 files changed

+19
-8
lines changed

2 files changed

+19
-8
lines changed

base/compiler/ssair/show.jl

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ function DILineInfoPrinter(linetable::Vector, showtypes::Bool=false)
382382
# if so, drop all existing calls to it from the top of the context
383383
# AND check if instead the context was previously printed that way
384384
# but now has removed the recursive frames
385-
let method = method_name(context[nctx])
385+
let method = method_name(context[nctx]) # last matching frame
386386
if (nctx < nframes && method_name(DI[nframes - nctx]) === method) ||
387387
(nctx < length(context) && method_name(context[nctx + 1]) === method)
388388
update_line_only = true
@@ -391,8 +391,15 @@ function DILineInfoPrinter(linetable::Vector, showtypes::Bool=false)
391391
end
392392
end
393393
end
394-
elseif length(context) > 0
395-
update_line_only = true
394+
end
395+
# look at the first non-matching element to see if we are only changing the line number
396+
if !update_line_only && nctx < length(context) && nctx < nframes
397+
let CtxLine = context[nctx + 1],
398+
FrameLine = DI[nframes - nctx]
399+
if method_name(CtxLine) === method_name(FrameLine)
400+
update_line_only = true
401+
end
402+
end
396403
end
397404
elseif nctx < length(context) && nctx < nframes
398405
# look at the first non-matching element to see if we are only changing the line number

src/disasm.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -229,17 +229,21 @@ void DILineInfoPrinter::emit_lineinfo(raw_ostream &Out, std::vector<DILineInfo>
229229
// if so, drop all existing calls to it from the top of the context
230230
// AND check if instead the context was previously printed that way
231231
// but now has removed the recursive frames
232-
StringRef method = StringRef(context.at(nctx - 1).FunctionName).rtrim(';');
232+
StringRef method = StringRef(context.at(nctx - 1).FunctionName).rtrim(';'); // last matching frame
233233
if ((nctx < nframes && StringRef(DI.at(nframes - nctx - 1).FunctionName).rtrim(';') == method) ||
234234
(nctx < context.size() && StringRef(context.at(nctx).FunctionName).rtrim(';') == method)) {
235235
update_line_only = true;
236-
while (nctx > 0 && StringRef(context.at(nctx - 1).FunctionName).rtrim(';') == method) {
236+
// transform nctx to exclude the combined frames
237+
while (nctx > 0 && StringRef(context.at(nctx - 1).FunctionName).rtrim(';') == method)
237238
nctx -= 1;
238-
}
239239
}
240240
}
241-
else if (context.size() > 0) {
242-
update_line_only = true;
241+
if (!update_line_only && nctx < context.size() && nctx < nframes) {
242+
// look at the first non-matching element to see if we are only changing the line number
243+
const DILineInfo &CtxLine = context.at(nctx);
244+
const DILineInfo &FrameLine = DI.at(nframes - 1 - nctx);
245+
if (StringRef(CtxLine.FunctionName).rtrim(';') == StringRef(FrameLine.FunctionName).rtrim(';'))
246+
update_line_only = true;
243247
}
244248
}
245249
else if (nctx < context.size() && nctx < nframes) {

0 commit comments

Comments
 (0)