Skip to content

[cpyrt] Check has_value before forwarding optional attributes - #120

Merged
aaronj0 merged 1 commit into
compiler-research:mainfrom
aaronj0:optional-has-value-getattr
Sep 30, 2026
Merged

aaronj0 merged 1 commit into
compiler-research:mainfrom
aaronj0:optional-has-value-getattr

Conversation

@aaronj0

@aaronj0 aaronj0 commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

Attribute lookups on std::optional and std::expected went through operator* and dereferenced a disengaged value, which trigger on empty optionals. Ask has_value() first: an empty or error state raises AttributeError, an engaged value forwards as before. Based on the patch submitted to ROOT: root-project/root#23460

@aaronj0
aaronj0 force-pushed the optional-has-value-getattr branch from 9bb9952 to 11fc3c4 Compare September 29, 2026 15:12
@aaronj0
aaronj0 requested a review from guitargeek September 29, 2026 15:14

@guitargeek guitargeek 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! Given the CI failures are fixed or understood.

@aaronj0

aaronj0 commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator Author

LGTM! Given the CI failures are fixed or understood.

unrelated, just the wheels that are still trying to pull in LLVM21: #122 fixed here

Attribute lookups on std::optional and std::expected went through
operator* and dereferenced a disengaged value, which IPython's
rich-display probes (_repr_html_ and friends) trigger on every empty
optional. Ask has_value() first: an empty or error state raises
AttributeError, an engaged value forwards as before.

Co-authored-by: Juan Miguel Carceller <jmcarcell@users.noreply.github.com>
@aaronj0
aaronj0 force-pushed the optional-has-value-getattr branch from 11fc3c4 to 06c820b Compare September 29, 2026 16:46
@aaronj0
aaronj0 merged commit 5cb4ec8 into compiler-research:main Sep 30, 2026
21 of 22 checks passed
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