[lldb] Fix swift exception breakpoint command type filtering #4027
Conversation
|
@swift-ci test |
|
@swift-ci test Linux |
| @@ -216,12 +216,6 @@ let Command = "breakpoint set" in { | |||
| def breakpoint_set_file_colon_line : Option<"joint-specifier", "y">, Group<12>, Arg<"FileLineColumn">, | |||
| Required, Completion<"SourceFile">, | |||
There was a problem hiding this comment.
I'm a bit confused about this change — does that mean the option doesn't (and didn't) show up in the online help for the breakpoint command? Is that what we want?
There was a problem hiding this comment.
I updated the PR description to point out that this commented-out definition is a duplicate. The actual definition of breakpoint_set_file_colon_line is a few lines up. Maybe this duplicate came from a merge?
There was a problem hiding this comment.
This looks fine. I would prefer all the tests that set breakpoints do so through facilities in lldbutil so we can centralize checking and getting the result out. Here guessing that the breakpoint you make is the first one in the target seems like laying a little landmine for people coming to revise the test later.
| cmd = "breakpoint set -E swift" | ||
| if typename: | ||
| cmd += " --exception-typename " + typename | ||
| self.runCmd(cmd) |
There was a problem hiding this comment.
Might be better to make a lldbutil.run_break_for_exception along the lines of "run_break_set_by_name, etc. Then you can use the check_breakpoint_result to pull the breakpoint out and make sure that the setting actually worked.
There was a problem hiding this comment.
Should we be adding those on the Swift branch? (no strong opinion either way)
There was a problem hiding this comment.
Break for exception works for C++ & ObjC, so there's no reason not to. The Object type would be an extra argument, so you don't need to bring along any explicit swiftness.
There was a problem hiding this comment.
I have added lldbutil.run_break_set_by_exception. I will open a diff to upstream llvm too.
|
@swift-ci test |
kastiglione
deleted the
lldb-Fix-swift-exception-breakpoint-command-type-filtering
branch
Specifying a typename filter for Swift exception breakpoints has been broken when creating breakpoints from the command line. In other words, this command: ``` (lldb) breakpoint set -E swift --exception-typename MyError ``` would create a breakpoint that stopped for **any** Swift exception. The fix is a one-line fix, changing the way `m_exception_extra_args` is constructed in `CommandObjectBreakpoint`. These args are consumed in [`SwiftExceptionPrecondition::ConfigurePrecondition`](https://github.com/apple/llvm-project/blob/9509716734619d7299d296cb175e5d50f1d0a857/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp#L1524-L1531). Most of the changes included in this PR are to refactor the associated unit tests. The tests had been creating the exception breakpoint via SB API, not via command line which is why this bug wasn't caught by the tests. The tests have been updated to create exception breakpoints via both API and CLI. While editing the tests, I cleaned them up too. I also deleted a duplicate and commented-out definition of `breakpoint_set_exception_typename` in `Commands/Options.td`. rdar://75959015 (cherry picked from commit 0a70434)
Specifying a typename filter for Swift exception breakpoints has been broken when creating breakpoints from the command line.
In other words, this command:
would create a breakpoint that stopped for any Swift exception.
The fix is a one-line fix, changing the way
m_exception_extra_argsis constructed inCommandObjectBreakpoint. These args are consumed inSwiftExceptionPrecondition::ConfigurePrecondition.Most of the changes included in this PR are to refactor the associated unit tests. The tests had been creating the exception breakpoint via SB API, not via command line which is why this bug wasn't caught by the tests. The tests have been updated to create exception breakpoints via both API and CLI. While editing the tests, I cleaned them up too.
I also deleted a duplicate and commented-out definition of
breakpoint_set_exception_typenameinCommands/Options.td.rdar://75959015