-
Notifications
You must be signed in to change notification settings - Fork 341
[lldb] Check if C++ interop and embedded Swift are enabled on CU instead of whole module #10279
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
[lldb] Check if C++ interop and embedded Swift are enabled on CU instead of whole module #10279
Conversation
@swift-ci test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes a lot more sense than the copy&paste error that was there before.
@swift-ci test |
Checking if C++ interop is enabled can be extremely slow if the project is big enough. We have changed this to checking only the compile unit instead, but one usage of the old API slipped by. rdar://147009063
When deciding whether to format a C++ type as a Swiftified type or not, checking if Swift/C++ interop is enabled on the entire target can be very expensive for big projects, and may not swiftify types that should be. rdar://117708944 (cherry picked from commit fe06ebc)
Reading debug info of an entire module to check whether embedded Swift is enabled or not can be very slow. This patch replaces that check with only checking the compile unit instead. rdar://147009063
c2c8cdb
to
03ee42c
Compare
@swift-ci test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks reasonable to me, but I don't have the knowledge to know whether this prevents the expensive flag checks.
@swift-ci test macOS |
@swift-ci test macos |
lldb::SymbolContextItem::eSymbolContextModule); | ||
|
||
// If there is a compile unit, use that to check if C++ interop should be | ||
// enabled. If there is no compiler unit, use the module. If neither |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
compiler -> compile
@@ -2653,7 +2650,7 @@ static bool ShouldEnableCXXInterop(CompileUnit *cu) { | |||
if (unit.get() == cu) { | |||
if (cu->GetLanguage() == eLanguageTypeSwift) | |||
for (const char *arg : args.GetArgumentArrayRef()) | |||
if (strcmp(arg, "-enable-experimental-cxx-interop") == 0) | |||
if (strcmp(arg, flag) == 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we not have a DWARF attribute for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't
@@ -2663,6 +2660,15 @@ static bool ShouldEnableCXXInterop(CompileUnit *cu) { | |||
return false; | |||
} | |||
|
|||
/// Determine whether this CU was compiled with C++ interop enabled. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this comment should be in the header
Checking if C++ interop is enabled can be extremely slow if the project is big enough. We have changed this to checking only the compile unit instead, but one usage of the old API slipped by.
rdar://147009063