Skip to content

[lldb] Fix embedded type summary regex handling #8121

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.

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

Merged

Conversation

kastiglione
Copy link

@kastiglione kastiglione commented Feb 4, 2024

Fixes a logic error when determining if a type is a regex. Look for a leading ^ in the type name, not the summary string.

Adds a test to verify regex handling.

Fixes a log message to help debugging.

@kastiglione
Copy link
Author

@swift-ci test

@kastiglione
Copy link
Author

@swift-ci test

@kastiglione
Copy link
Author

The new test fails in CI, but passes for me locally. I am not yet sure why.

@kastiglione
Copy link
Author

@swift-ci test macOS

@kastiglione
Copy link
Author

@swift-ci test macOS

@kastiglione
Copy link
Author

@swift-ci test macOS

@kastiglione
Copy link
Author

@swift-ci test macOS

@kastiglione
Copy link
Author

@swift-ci test

@kastiglione
Copy link
Author

@swift-ci test windows

@kastiglione kastiglione merged commit 81ae1b5 into stable/20230725 Feb 6, 2024
@kastiglione kastiglione deleted the dl/lldb-Fix-embedded-type-summary-regex-handling branch February 6, 2024 23:14
@@ -1452,7 +1452,7 @@ static void LoadTypeSummariesForModule(ModuleSP module_sp) {
return;

Log *log = GetLog(LLDBLog::DataFormatters);
const char *module_name = module_sp->GetObjectName().GetCString();
const char *module_name = module_sp->GetFileSpec().GetFilename().GetCString();

Choose a reason for hiding this comment

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

Micro-optimization: since this is only used in LLDB_LOGF, we can save the C string conversion by using LLDB_LOG instead and use a StringRef.

@@ -1488,7 +1488,7 @@ static void LoadTypeSummariesForModule(ModuleSP module_sp) {
auto summary_sp =
std::make_shared<StringSummaryFormat>(flags, summary_string.data());
FormatterMatchType match_type = eFormatterMatchExact;
if (summary_string.front() == '^' && summary_string.back() == '$')
if (type_name.front() == '^')

Choose a reason for hiding this comment

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

It just dawned on me that in Pascal, ^ is the equivalent of * in C and thus can be part of a type name. In the next revision we probably should include an isRegex bit in the header.

Choose a reason for hiding this comment

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

and AFAIK there are downstream Pascal compilers that target LLDB.

kastiglione added a commit that referenced this pull request Feb 16, 2024
Fixes a logic error when determining if a type is a regex. Look for a leading `^` in 
the type name, not the summary string.

Adds a test to verify regex handling.

Fixes a log message to help debugging.

(cherry-picked from commit 81ae1b5)
kastiglione added a commit that referenced this pull request Feb 16, 2024
Fixes a logic error when determining if a type is a regex. Look for a leading `^` in 
the type name, not the summary string.

Adds a test to verify regex handling.

Fixes a log message to help debugging.

(cherry-picked from commit 81ae1b5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants