jingham accepted this revision. jingham added a comment. In D50304#3592849 <https://reviews.llvm.org/D50304#3592849>, @RamNalamothu wrote:
> In D50304#3586710 <https://reviews.llvm.org/D50304#3586710>, @jingham wrote: > >> For some reason I'm not getting mail notifications for review changes, sorry >> about that. >> >> This is certainly better than the original implementation. Among other >> things, if we find an exact match, we really shouldn't be doing any more >> inexact matches, so after the first exact hit it should have switched exact >> to true. >> >> But if you had line tables laid out like (this is in increasing order of >> address: >> >> 10 >> 20 >> 30 >> 10 >> 16 >> >> and you did "thread until 15", this would find the inexact match at 20, >> switch to an exact match for line 20 and find no other matches. But the gap >> between 10 & 16 in the line table is maybe an even more plausible place to >> put the line 15 until breakpoint, so maybe we did want to throw a breakpoint >> there as well? > > @jingham > Nope, with the current patch, we would find the inexact match at 16. Ah, my bad. FindLineEntry does end up in FindLineEntryByFileIndexImpl which does the closest match. >> Regular breakpoint setting has to move inexact breakpoints in much the same >> way. The code in the BreakpointResolverFileLine::SearchCallback ends up >> calling CompileUnit::ResolveSymbolContext to get the "best" inexact match. >> Maybe it would be better to not do this by hand here in the Until command, >> but reuse the code that we use to move break points more generally? > > Be it CompileUnit::ResolveSymbolContext or CompileUnit::FindLineEntry, we end > up calling LineTable::FindLineEntryIndexByFileIndex which finds "Exact match > always wins. Otherwise try to find the closest line > the desired line." > when we pass `exact = false` to it. > Given that and we anyway have to extract address list, from what we get out > of CompileUnit::ResolveSymbolContext, for the thread until thread plan to > work with, probably the additional overhead to use > CompileUnit::ResolveSymbolContext here does not make sense. Or am I missing > something? Finding these addresses happens once in response to a user command, so I highly doubt that the overhead matters one way or the other. Both end up doing the closest match (I misremembered where that was done) which is the main thing. ResolveSymbolContext does more work to handle inlined files, but there's no way to do "thread until" from a main CU into inlined source so that doesn't matter. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D50304/new/ https://reviews.llvm.org/D50304 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
