Skip to content

[lldb] Fix --source-regexp-function for Swift static functions #12873

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kastiglione wants to merge 2 commits into
base: swift/release/6.4.x
Choose a base branch
from
Open

[lldb] Fix --source-regexp-function for Swift static functions #12873

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8ff00c8
[lldb] Fix --source-regexp-function for Swift static functions
kastiglione May 1, 2026
469522d
[lldb] Replace non-portable grep in Swift.rules
kastiglione May 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion lldb/packages/Python/lldbsuite/test/make/Swift.rules
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,11 @@ ifeq "$(USESWIFTDRIVER)" "1"
ifeq "$(DYLIB_NAME)" ""
MODULENAME?=$(shell basename $(EXE) .out)
# Compile with -parse-as-library when main.swift contains "@main".
ifneq "$(shell grep -w '@main' $(VPATH)/main.swift 2>/dev/null)" ""
ifneq "$(wildcard $(VPATH)/main.swift)" ""
Copy link
Copy Markdown

@adrian-prantl adrian-prantl May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be much more straightforward to grep in $(SWIFT_SOURCES)? Or is that not set here?

Copy link
Copy Markdown

@adrian-prantl adrian-prantl May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or $(VPATH)/$(SWIFT_SOURCES)

Copy link
Copy Markdown
Author

@kastiglione kastiglione May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote it as main.swift to follow/enforce convention. Currently all 46 uses of @main are in files named main.swift.

Copy link
Copy Markdown

@adrian-prantl adrian-prantl May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but we have a variable that makes the file name configurable, so IMHO we should be using it.

ifneq "$(shell git -C $(VPATH) grep --no-index -w '@main' main.swift)" ""
Copy link
Copy Markdown

@adrian-prantl adrian-prantl May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think invoking git during the build is a great idea. Some build pipelines copy the sources somewhere else for building.

Copy link
Copy Markdown
Author

@kastiglione kastiglione May 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some build pipelines copy the sources somewhere else for building.

Meaning it won't work if the sources aren't part of git? That's a case that --no-index handles:

--no-index
    Search files in the current directory that is not managed by Git, or by
    ignoring that the current directory is managed by Git. This is rather similar
    to running the regular grep(1) utility with its -r option specified, but with
    some additional benefits, such as ...

Copy link
Copy Markdown
Author

@kastiglione kastiglione May 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw since I find git-diff much better than original diff, I use the following alias:

alias diff='git diff --no-index'

In my experience git grep --no-index and git diff --no-index are capable replacements for grep and diff respectively.

Copy link
Copy Markdown

@adrian-prantl adrian-prantl May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we already know the file in which to look at, what added value does going through git add? For example on macOS, grep is part of the base operating system, but git is a shimmed tool that may need to be found via a slow indirection through xcrun — this just seems to be more fragile?

@charles-zablit given all the tool helpers that are in Makefile.rules for Windows — which one is expected to work better there?

Copy link
Copy Markdown

@charles-zablit charles-zablit May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coincidentally, I looked into this recently: #12947

I have not tried git however.

Copy link
Copy Markdown
Author

@kastiglione kastiglione May 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had also considered the same as #12947 – that works for me. I went with this approach because I prefer a single method, vs one for windows and another for everything else.

I think git is the opposite of fragile, it's ubiquitous and battled tested. Given how many tools are invoked via xcrun, I didn't think that would be an issue.

Copy link
Copy Markdown

@charles-zablit charles-zablit May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the git command works on Windows, I agree that it's a better approach, as it eliminates the different codepaths in the findstring approach.

Copy link
Copy Markdown
Author

@kastiglione kastiglione May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on this PR it does work on windows.

Copy link
Copy Markdown

@charles-zablit charles-zablit May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Swift lldb tests are not enabled on PR testing yet. They only run on https://ci-external.swift.org/job/oss-lldb-windows-x86_64/ for now, until we confirm they are stable.

Copy link
Copy Markdown
Author

@kastiglione kastiglione May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR has one test which is not disabled, and was failing. With the fix to Swift.rules, it passes. I'm basing my "it does work" statement on this.

PARSE_AS_LIBRARY = -parse-as-library
endif
endif
else
EXE = $(DYLIB_FILENAME)
MODULENAME?=$(DYLIB_NAME)
Expand Down
5 changes: 5 additions & 0 deletions lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeNames.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1069,6 +1069,11 @@ static bool ParseGlobal(const swift::Demangle::NodePointer &node,
for (swift::Demangle::Node::iterator pos = node->begin(); pos != end; ++pos) {
swift::Demangle::NodePointer child = *pos;
if (child) {
// Peel off static node.
if (child->getKind() == swift::Demangle::Node::Kind::Static &&
child->hasChildren())
child = child->getFirstChild();

kind = child->getKind();
switch (child->getKind()) {
case swift::Demangle::Node::Kind::Allocator:
Expand Down
3 changes: 3 additions & 0 deletions lldb/test/API/lang/swift/break_source_regex_static_function/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
SWIFT_SOURCES := main.swift

include Makefile.rules
30 changes: 30 additions & 0 deletions ...t/API/lang/swift/break_source_regex_static_function/TestBreakSourceRegexStaticFunction.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
"""
Tests source regex breakpoints scoped to Swift static functions (-X option).
"""

import lldb
from lldbsuite.test.lldbtest import *
from lldbsuite.test.decorators import *
import lldbsuite.test.lldbutil as lldbutil


class TestCase(TestBase):
@swiftTest
def test_source_regex_with_static_function_filter(self):
self.build()
target, _, _, _ = lldbutil.run_to_name_breakpoint(self, "main")
self.expect("breakpoint set -p 'break here' -X first")
bp = target.breakpoint[-1]
self.assertTrue(bp.IsEnabled())
self.assertEqual(bp.GetNumResolvedLocations(), 1)
self.assertIn("Scope.first", str(bp.location[0]))

@swiftTest
def test_source_regex_with_static_function_filter_and_file(self):
self.build()
target, _, _, _ = lldbutil.run_to_name_breakpoint(self, "main")
self.expect("breakpoint set -f main.swift -p 'break here' -X second")
bp = target.breakpoint[-1]
self.assertTrue(bp.IsEnabled())
self.assertEqual(bp.GetNumResolvedLocations(), 1)
self.assertIn("Scope.second", str(bp.location[0]))
16 changes: 16 additions & 0 deletions lldb/test/API/lang/swift/break_source_regex_static_function/main.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
enum Scope {
static func first() {
_ = "break here"
}

static func second() {
_ = "break here"
}
}

@main enum Entry {
static func main() {
Scope.first()
Scope.second()
}
}