Skip to content

4673: roll back AtVoid#4677

Closed
R0bari wants to merge 2 commits into
objectionary:masterfrom
R0bari:4673-rolling-back-attr
Closed

4673: roll back AtVoid#4677
R0bari wants to merge 2 commits into
objectionary:masterfrom
R0bari:4673-rolling-back-attr

Conversation

@R0bari

@R0bari R0bari commented Nov 20, 2025

Copy link
Copy Markdown
Contributor

#4673

In this PR I rolled back Attr, AtVoid and created a missed test for AtVoid

Summary by CodeRabbit

  • New Features

    • Introduced Attr interface and AtVoid class for improved attribute lifecycle management with single-assignment semantics.
    • Added copy(Phi self) method to support enhanced object copying with explicit self-reference.
  • Bug Fixes

    • Improved RHO and void attribute handling in object delegation patterns.
  • Refactor

    • Streamlined internal wrapper architecture by removing unused abstraction layers.
    • Transitioned from lazy-composition to single-evaluation semantics for improved predictability.
    • Reorganized attribute access and mutation validation flows.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Nov 20, 2025

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

Walkthrough

This PR refactors the EO runtime's attribute and caching infrastructure by introducing a new Attr interface, replacing the composition-based wrapping (PhComposite) with expression-based lazy evaluation (PhOnce), removing several wrapper classes (PhCached, PhComposite, PhRho, PhWithRho), and substantially refactoring PhDefault, PhVoid, and related classes to support writable-once semantics and improved attribute lifecycle management. The XSL template also switches to PhOnce for code generation.

Changes

Cohort / File(s) Change Summary
New abstraction layer
eo-runtime/src/main/java/org/eolang/Attr.java, eo-runtime/src/main/java/org/eolang/AtVoid.java
Introduces Attr interface defining copy(Phi self), get(), put(Phi) contract with three string constants (LAMBDA, PHI, RHO). Implements AtVoid as a writable-once attribute wrapper with lifecycle methods (set(), isSet(), validation).
Removed wrapper classes
eo-runtime/src/main/java/org/eolang/PhCached.java, eo-runtime/src/main/java/org/eolang/PhComposite.java, eo-runtime/src/main/java/org/eolang/PhRho.java, eo-runtime/src/main/java/org/eolang/PhWithRho.java
Deletes caching and composition wrapper infrastructure; functionality integrated into remaining classes.
PhOnce refactoring
eo-runtime/src/main/java/org/eolang/PhOnce.java
Replaces Supplier-based storage with Function<Phi, Phi> expression evaluation; adds new constructor PhOnce(Phi obj, Function<Phi, Phi> func) and copy(Phi self) method; evaluates expression lazily via expr.apply(arg) for all Phi operations.
PhVoid lifecycle & delegation
eo-runtime/src/main/java/org/eolang/PhVoid.java
Adds validation (validate(), isSet(), set() methods); updates copy(Phi self) to delegate inner copy; adds take(String) and put(String, Phi) delegation to inner object; modifies delta(), hasRho(), locator() to support new semantics.
PhDefault attribute handling
eo-runtime/src/main/java/org/eolang/PhDefault.java
Adds copy(Phi self) overload; refactors take(String) to support RHO copying without wrapper; updates put(int/String) with validation and ExUnset/ExReadOnly errors; replaces RHO initialization from PhRho to PhVoid; updates delta() logic.
Phi interface & logging
eo-runtime/src/main/java/org/eolang/Phi.java, eo-runtime/src/main/java/org/eolang/PhLogged.java
Adds copy(Phi self) method to Phi interface; introduces copy(Phi self) override in PhLogged with logging and delta() method delegation.
Code generation XSL
eo-maven-plugin/src/main/resources/org/eolang/maven/transpile/to-java.xsl
Replaces inner wrapping type from PhComposite to PhOnce in atom, abstract, and bound paths for PhCached-wrapped functions.
Test suite updates
eo-runtime/src/test/java/org/eolang/AtVoidTest.java, eo-runtime/src/test/java/org/eolang/PhDefaultTest.java, eo-runtime/src/test/java/org/eolang/PhMethodTest.java, eo-runtime/src/test/java/org/eolang/PhSimple.java, eo-runtime/src/test/java/org/eolang/PhWithTest.java, eo-runtime/src/test/java/EOorg/EOeolang/...
Adds AtVoidTest suite (5 tests for writable-once semantics); replaces PhComposite with PhOnce across multiple test files (EOerrorTest, EOmallocTest, EOtryTest, HeapsTest, PhDefaultTest, PhMethodTest, PhSimple, PhWithTest); removes PhCompositeTest.

Sequence Diagram(s)

sequenceDiagram
    participant Old as Old Flow (PhComposite)
    participant New as New Flow (PhOnce)
    participant Caller as Caller
    
    Caller->>Old: take(name) / put(pos) call
    Old->>Old: re-evaluate expression each time
    Old-->>Caller: result (recomputed)
    
    Caller->>New: take(name) / put(pos) call
    New->>New: expr.apply(arg) once, cache in AtomicRef
    New->>New: compareAndSet checks cache
    New-->>Caller: cached result
Loading
sequenceDiagram
    participant PhDefault as PhDefault
    participant PhVoid as PhVoid (Attr impl)
    participant Inner as Inner Phi
    
    rect rgb(220, 240, 255)
    Note over PhDefault,Inner: Writable-Once Lifecycle
    end
    
    PhDefault->>PhDefault: add(name, PhVoid)
    PhVoid->>PhVoid: initialize with name, null object
    
    Caller->>PhVoid: put(value)
    PhVoid->>PhVoid: validate() check
    PhVoid->>PhVoid: set(value) if not already set
    PhVoid-->>Caller: success or throw ExReadOnly
    
    Caller->>PhVoid: get()
    PhVoid->>PhVoid: validate() or throw ExUnset
    PhVoid-->>Caller: inner Phi object
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

  • PhOnce refactoring: The fundamental shift from Supplier to Function<Phi, Phi> with expression-based lazy evaluation affects all Phi method delegates; requires careful review of expression caching semantics and interaction with the new arg parameter.
  • PhDefault attribute handling: Multiple interacting changes to take(String), put(int/String), and RHO initialization; the transition from PhRho/PhWithRho wrappers to inline PhVoid requires understanding the new single-assignment semantics.
  • PhVoid lifecycle management: New validation, state-checking, and delegation patterns require verification that the writable-once contract is enforced correctly across all paths.
  • Cascading effects across tests: Eight test files update from PhComposite to PhOnce; each needs verification that the new lazy-evaluation semantics maintain intended behavior.
  • Removal of four classes: Need to verify all usages are correctly replaced and no orphaned dependencies remain.

Areas requiring extra attention:

  • PhOnce.copy(Phi self) semantics and interaction with expr.apply(arg) vs. direct argument passing
  • PhDefault.take(String) and RHO handling without PhWithRho wrapper
  • PhVoid.set(Phi) enforcement of single-assignment for Phi.RHO specifically
  • PhDefault.put(String/int) validation logic and ExUnset vs. ExReadOnly error path correctness
  • Test changes to EOmallocTest, EOtryTest, and HeapsTest for semantic correctness

Possibly related PRs

  • objectionary/eo#4333: Modifies PhDefault attribute-access logic and take(String) control flow; overlaps in the same class/method area.
  • objectionary/eo#4223: Modifies the EO attribute/caching layer including PhComposite, PhOnce, PhCached, AtVoid, and PhVoid classes.
  • objectionary/eo#4389: Modifies PhCompositeTest (changes visibility, removes constants); related through test file changes.

Suggested labels

core, refactoring

Suggested reviewers

  • h1alexbel
  • yegor256
  • maxonfjvipon

Poem

🐰 Whiskers twitching with glee

Old Composite hops away,
PhOnce takes the stage today—
Single-shot, cache-true, hooray!
RHO finds its void, so bright,
Attributes locked—once, just right! 🔐✨

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bd32543 and 26644ff.

📒 Files selected for processing (22)
  • eo-maven-plugin/src/main/resources/org/eolang/maven/transpile/to-java.xsl (3 hunks)
  • eo-runtime/src/main/java/org/eolang/AtVoid.java (1 hunks)
  • eo-runtime/src/main/java/org/eolang/Attr.java (1 hunks)
  • eo-runtime/src/main/java/org/eolang/PhCached.java (0 hunks)
  • eo-runtime/src/main/java/org/eolang/PhComposite.java (0 hunks)
  • eo-runtime/src/main/java/org/eolang/PhDefault.java (10 hunks)
  • eo-runtime/src/main/java/org/eolang/PhLogged.java (2 hunks)
  • eo-runtime/src/main/java/org/eolang/PhOnce.java (1 hunks)
  • eo-runtime/src/main/java/org/eolang/PhRho.java (0 hunks)
  • eo-runtime/src/main/java/org/eolang/PhVoid.java (2 hunks)
  • eo-runtime/src/main/java/org/eolang/PhWithRho.java (0 hunks)
  • eo-runtime/src/main/java/org/eolang/Phi.java (2 hunks)
  • eo-runtime/src/test/java/EOorg/EOeolang/EOerrorTest.java (1 hunks)
  • eo-runtime/src/test/java/EOorg/EOeolang/EOmallocTest.java (2 hunks)
  • eo-runtime/src/test/java/EOorg/EOeolang/EOtryTest.java (4 hunks)
  • eo-runtime/src/test/java/EOorg/EOeolang/HeapsTest.java (1 hunks)
  • eo-runtime/src/test/java/org/eolang/AtVoidTest.java (1 hunks)
  • eo-runtime/src/test/java/org/eolang/PhCompositeTest.java (0 hunks)
  • eo-runtime/src/test/java/org/eolang/PhDefaultTest.java (6 hunks)
  • eo-runtime/src/test/java/org/eolang/PhMethodTest.java (1 hunks)
  • eo-runtime/src/test/java/org/eolang/PhSimple.java (1 hunks)
  • eo-runtime/src/test/java/org/eolang/PhWithTest.java (1 hunks)

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@R0bari R0bari closed this Nov 20, 2025
@github-actions

Copy link
Copy Markdown
Contributor

🚀 Performance Analysis

All benchmarks are within the acceptable range. No critical degradation detected (threshold is 100%). Please refer to the detailed report for more information.

Click to see the detailed report
Test Base Score PR Score Change % Change Unit Mode
benchmarks.XmirBench.xmirToEO 182.705 189.368 6.663 3.65% ms/op Average Time

⚠️ Performance loss: benchmarks.XmirBench.xmirToEO is slower by 6.663 ms/op (3.65%)

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.

1 participant