Skip to content

Report the dlopen failure reason on stderr - #1101

Open
conrade-ctc wants to merge 1 commit into
compiler-research:mainfrom
chicagotrading:pr-e-dlopen-reason
Open

Report the dlopen failure reason on stderr#1101
conrade-ctc wants to merge 1 commit into
compiler-research:mainfrom
chicagotrading:pr-e-dlopen-reason

Conversation

@conrade-ctc

Copy link
Copy Markdown
Collaborator

DynamicLibraryManager::loadLibrary captured the dlerror() text but logged it only under LLVM_DEBUG. cppjit's load_library reads stderr to build its Python error, so it saw an empty reason. This change writes the text to std::cerr, because cppjit captures only std::cerr's rdbuf, not llvm::errs(). Paths.cpp already gets this text from ::dlerror().

@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.82%. Comparing base (9bbdbb2) to head (9a9d37d).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1101      +/-   ##
==========================================
+ Coverage   87.74%   87.82%   +0.08%     
==========================================
  Files          23       23              
  Lines        6429     6431       +2     
==========================================
+ Hits         5641     5648       +7     
+ Misses        788      783       -5     
Files with missing lines Coverage Δ
lib/CppInterOp/DynamicLibraryManager.cpp 75.00% <100.00%> (+1.16%) ⬆️

... and 2 files with indirect coverage changes

Files with missing lines Coverage Δ
lib/CppInterOp/DynamicLibraryManager.cpp 75.00% <100.00%> (+1.16%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@aaronj0 aaronj0 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM! I think updating compiler-research/cppjit#63 to make use of this directly is a more robust (and platform independent) solution

// message.
// TODO: Implement callbacks

// std::cerr, not llvm::errs(): cppyy captures only std::cerr's rdbuf.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// std::cerr, not llvm::errs(): cppyy captures only std::cerr's rdbuf.

@aaronj0
aaronj0 requested a review from vgvassilev September 2, 2026 08:08

@vgvassilev vgvassilev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

DynamicLibraryManager::loadLibrary got the dlerror() text from
platform::DLOpen but only emitted it under LLVM_DEBUG. cppjit's
load_library builds its Python error from captured stderr, so the
reason was empty. Emit the text to std::cerr; cppjit captures only
std::cerr's rdbuf, not llvm::errs().

Paths.cpp already calls ::dlerror() to build this text.

Co-developed-with-the-help-of: Claude Code (Fable 5, human in the loop)
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

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.

3 participants