Skip to content

Add progress reports for bridging header compilation. #8103

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

Merged
merged 3 commits into from
Feb 2, 2024
Merged

Add progress reports for bridging header compilation. #8103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dce951c
TypeSystemSwiftTypeRef: Fall through if TypeSystemClang fails
adrian-prantl Feb 1, 2024
2e38b48
[lldb][progress] Correctly check total for deterministic progress (#7…
chelcassanova Jan 30, 2024
50da0b5
Add progress reports for bridging header compilation.
adrian-prantl Feb 1, 2024
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
3 changes: 2 additions & 1 deletion lldb/include/lldb/Core/DebuggerEvents.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//

#include "lldb/Core/ModuleSpec.h"
#include "lldb/Core/Progress.h"
#include "lldb/Utility/Event.h"
#include "lldb/Utility/StructuredData.h"

Expand Down Expand Up @@ -39,7 +40,7 @@ class ProgressEventData : public EventData {
GetAsStructuredData(const Event *event_ptr);

uint64_t GetID() const { return m_id; }
bool IsFinite() const { return m_total != UINT64_MAX; }
bool IsFinite() const { return m_total != Progress::kNonDeterministicTotal; }
uint64_t GetCompleted() const { return m_completed; }
uint64_t GetTotal() const { return m_total; }
std::string GetMessage() const {
Expand Down
3 changes: 3 additions & 0 deletions lldb/include/lldb/Core/Progress.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ class Progress {
void Increment(uint64_t amount = 1,
std::optional<std::string> updated_detail = {});

/// Used to indicate a non-deterministic progress report
static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;

private:
void ReportProgress();
static std::atomic<uint64_t> g_id;
Expand Down
3 changes: 2 additions & 1 deletion lldb/source/Core/DebuggerEvents.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "lldb/Core/DebuggerEvents.h"
#include "lldb/Core/Debugger.h"
#include "lldb/Core/Module.h"
#include "lldb/Core/Progress.h"
#include "llvm/Support/WithColor.h"

using namespace lldb_private;
Expand Down Expand Up @@ -41,7 +42,7 @@ void ProgressEventData::Dump(Stream *s) const {
s->PutCString(", type = update");
// If m_total is UINT64_MAX, there is no progress to report, just "start"
// and "end". If it isn't we will show the completed and total amounts.
if (m_total != UINT64_MAX)
if (m_total != Progress::kNonDeterministicTotal)
s->Printf(", progress = %" PRIu64 " of %" PRIu64, m_completed, m_total);
}

Expand Down
3 changes: 1 addition & 2 deletions lldb/source/Core/Progress.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ Progress::Progress(std::string title, std::string details,
std::optional<uint64_t> total,
lldb_private::Debugger *debugger)
: m_title(title), m_details(details), m_id(++g_id), m_completed(0),
m_total(1) {
assert(total == std::nullopt || total > 0);
m_total(Progress::kNonDeterministicTotal) {
if (total)
m_total = *total;

Expand Down
52 changes: 35 additions & 17 deletions lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2060,16 +2060,19 @@ SwiftASTContext::CreateInstance(lldb::LanguageType language, Module &module,
// swift::ASTContext
Progress progress("Importing Swift standard library");
swift_ast_sp->m_ast_context_ap->SetPreModuleImportCallback(
[&progress](llvm::StringRef module_name, bool is_overlay) {
progress.Increment(1, (is_overlay ? module_name.str() + " (overlay)"
: module_name.str()));
[&progress](llvm::StringRef module_name,
swift::ASTContext::ModuleImportKind kind) {
progress.Increment(1, (kind == swift::ASTContext::Overlay
Copy link

Choose a reason for hiding this comment

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

Should we have a switch here now that we have 3 kinds of module imports ?

Copy link
Author

Choose a reason for hiding this comment

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

Honestly I think even the overlay case makes no sense here since the stdlib is always a pure Swift module. I just left it in because it was there before.

Copy link

Choose a reason for hiding this comment

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

Fair

? module_name.str() + " (overlay)"
: module_name.str()));
});

// Clear the callback function on scope exit to prevent an out-of-scope
// access of the progress local variable
auto on_exit = llvm::make_scope_exit([&]() {
swift_ast_sp->m_ast_context_ap->SetPreModuleImportCallback(
[](llvm::StringRef module_name, bool is_overlay) {
[](llvm::StringRef module_name,
swift::ASTContext::ModuleImportKind kind) {
Progress("Importing Swift modules");
});
});
Expand Down Expand Up @@ -2569,19 +2572,22 @@ lldb::TypeSystemSP SwiftASTContext::CreateInstance(
const bool can_create = true;

// Report progress on module importing by using a callback function in
// swift::ASTContext
// swift::ASTContext.
Progress progress("Importing Swift standard library");
swift_ast_sp->m_ast_context_ap->SetPreModuleImportCallback(
[&progress](llvm::StringRef module_name, bool is_overlay) {
progress.Increment(1, (is_overlay ? module_name.str() + " (overlay)"
: module_name.str()));
[&progress](llvm::StringRef module_name,
swift::ASTContext::ModuleImportKind kind) {
progress.Increment(1, (kind == swift::ASTContext::Overlay
Copy link

Choose a reason for hiding this comment

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

ditto

? module_name.str() + " (overlay)"
: module_name.str()));
});

// Clear the callback function on scope exit to prevent an out-of-scope
// access of the progress local variable
// access of the progress local variable.
auto on_exit = llvm::make_scope_exit([&]() {
swift_ast_sp->m_ast_context_ap->SetPreModuleImportCallback(
[](llvm::StringRef module_name, bool is_overlay) {
[](llvm::StringRef module_name,
swift::ASTContext::ModuleImportKind kind) {
Progress("Importing Swift modules");
});
});
Expand Down Expand Up @@ -3627,19 +3633,31 @@ swift::ModuleDecl *SwiftASTContext::GetModule(const SourceModule &module,
auto import_diags = getScopedDiagnosticConsumer();

// Report progress on module importing by using a callback function in
// swift::ASTContext
// swift::ASTContext.
Progress progress("Importing Swift modules");
ast->SetPreModuleImportCallback([&progress](llvm::StringRef module_name,
bool is_overlay) {
progress.Increment(
1, (is_overlay ? module_name.str() + " (overlay)" : module_name.str()));
});
ast->SetPreModuleImportCallback(
[&progress](llvm::StringRef module_name,
swift::ASTContext::ModuleImportKind kind) {
switch (kind) {
case swift::ASTContext::Module:
progress.Increment(1, module_name.str());
break;
case swift::ASTContext::Overlay:
progress.Increment(1, module_name.str() + " (overlay)");
break;
case swift::ASTContext::BridgingHeader:
progress.Increment(1,
"Compiling bridging header: " + module_name.str());
break;
Comment on lines +3642 to +3651
Copy link

@medismailben medismailben Feb 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

In the swift::ASTContext::Module case, we only put the name of the module, if it's an overlay, we show both the name and (overlay) annotation, but if it's a bridging header, we now show Compiling bridging header: with the module name.

I don't have any preference but let's stay consistent across all the Increment updated_detail string.

}
});

// Clear the callback function on scope exit to prevent an out-of-scope access
// of the progress local variable
auto on_exit = llvm::make_scope_exit([&]() {
ast->SetPreModuleImportCallback(
[](llvm::StringRef module_name, bool is_overlay) {
[](llvm::StringRef module_name,
swift::ASTContext::ModuleImportKind kind) {
Progress("Importing Swift modules");
});
});
Expand Down
9 changes: 6 additions & 3 deletions lldb/source/Plugins/TypeSystem/Swift/TypeSystemSwiftTypeRef.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2830,7 +2830,8 @@ TypeSystemSwiftTypeRef::GetBitSize(opaque_compiler_type_t type,
// pointer instead of the underlying object.
if (Flags(clang_type.GetTypeInfo()).AllSet(eTypeIsObjC | eTypeIsClass))
return GetPointerByteSize() * 8;
return clang_type.GetBitSize(exe_scope);
if (auto clang_size = clang_type.GetBitSize(exe_scope))
return clang_size;
}
if (!exe_scope) {
LLDB_LOGF(GetLog(LLDBLog::Types),
Expand Down Expand Up @@ -4253,7 +4254,8 @@ TypeSystemSwiftTypeRef::GetTypeBitAlign(opaque_compiler_type_t type,
// object pointer instead of the underlying object.
if (Flags(clang_type.GetTypeInfo()).AllSet(eTypeIsObjC | eTypeIsClass))
return GetPointerByteSize() * 8;
return clang_type.GetTypeBitAlign(exe_scope);
if (auto clang_align = clang_type.GetTypeBitAlign(exe_scope))
return clang_align;
}
if (!exe_scope) {
LLDB_LOGF(GetLog(LLDBLog::Types),
Expand All @@ -4264,7 +4266,8 @@ TypeSystemSwiftTypeRef::GetTypeBitAlign(opaque_compiler_type_t type,
}
if (auto *runtime =
SwiftLanguageRuntime::Get(exe_scope->CalculateProcess())) {
if (auto result = runtime->GetBitAlignment({weak_from_this(), type}, exe_scope))
if (auto result =
runtime->GetBitAlignment({weak_from_this(), type}, exe_scope))
return result;
// If this is an expression context, perhaps the type was
// defined in the expression. In that case we don't have debug
Expand Down
3 changes: 2 additions & 1 deletion lldb/test/API/functionalities/progress_reporting/swift_progress_reporting/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
SWIFT_SOURCES := main.swift

SWIFT_BRIDGING_HEADER := bridging.h
SWIFT_PRECOMPILE_BRIDGING_HEADER := YES
LD_EXTRAS := -L. -linvisible
SWIFTFLAGS_EXTRAS := -I.

Expand Down
1 change: 1 addition & 0 deletions ...functionalities/progress_reporting/swift_progress_reporting/TestSwiftProgressReporting.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ def test_swift_progress_report(self):

beacons = [
"Loading Swift module",
"Compiling bridging header",
"Importing modules used in expression",
"Setting up Swift reflection",
"Importing Swift module dependencies for main.swift",
Expand Down
1 change: 1 addition & 0 deletions lldb/test/API/functionalities/progress_reporting/swift_progress_reporting/bridging.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
struct FromBridgingHeader {};
1 change: 1 addition & 0 deletions lldb/test/API/functionalities/progress_reporting/swift_progress_reporting/main.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import Foundation
func main() {
let boo = Invisible.👻()
let s = NSAttributedString(string: "Hello")
let b = FromBridgingHeader()
print("break here")
}

Expand Down