`Call` nodes: App tree roots #14303

kazcw · 2025-11-17T10:47:07Z

Pull Request Description

Add new Call node at the root of app/named-app calls; the next PR will enable the nodes at the sites of possible no-argument calls (i.e. around Ident and PropertyAccess nodes). For now node is ignored in backend and frontend; the next step is to use it to replace some analysis on both sides.

See: https://github.com/enso-org/design/pull/60

Important Notes

Updated all Rust crates to Edition 2024

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
All code follows the
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
Unit tests have been written where possible.
If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries,
or the Snowflake database integration, a run of the Extra Tests has been scheduled.
- If applicable, it is suggested to paste a link to a successful run of the Extra Tests.

kazcw · 2025-11-28T10:50:35Z

engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/TypeInferenceTest.java

  @Test
  public void callingExtensionMethodDefinedElsewhere() throws Exception {
-    final URI uriA = new URI("memory://local.Project1.modA.enso");
+    final URI uriA = new URI("memory://local.Project1.Mod_A.enso");


The original was not a valid module name

introduced by Infer method calls on known types and warn about nonexistent methods #11399

OK to change

farmaazon

Approving parser&GUI

farmaazon · 2025-11-28T14:47:21Z

lib/rust/parser/src/lib.rs

+    if let Variant::Eval(eval) = &tree.variant
+        && matches!(&eval.value.variant, Variant::Ident(_) | Variant::PropertyAccess(_))
+    {
+        let Tree { variant: Variant::Eval(mut eval), span, .. } = tree else { unreachable!() };


Isn't it possible to make this let inside if condition in place of let Variant::Eval(eval) = &tree.variant? Also, I'm not sure, but perhaps making && matches! in if a where clause in match may look better.

I think the unsafe borrow-to-owned upgrade pattern is unavoidable in this case, unless we refactor the whole thing to a mutable-borrow solution (but that tends to require allocations that wouldn't be necessary in this approach).
In order to tell if ownership should be taken, we must look inside the variant; sometimes a deep pattern can take ownership while checking the variant simultaneously, but that isn't possible here because the variant is boxed and box patterns aren't available in Rust as of our current version.

I will think about whether I can encapsulate the borrow-inspect-upgrade pattern for tree variant operations, to contain the unsafety to one function.

Oh, in this case I can take ownership unconditionally, then if the validity check fails I can return an error explicitly rather than falling through to the general error case.

All I looked for is code simplification; if it would required "unsafe" operations, then it is not worth it.

Oh, I shouldn't have said unsafe, I meant "requiring an unreachable branch", which could be unsafe but in this case is handled by a panic. Anyway, I've eliminated the panic branch with some ownership wrangling.

farmaazon · 2025-11-28T15:09:49Z

lib/rust/parser/src/syntax/expression/operand.rs

+/// Wraps a value, tracking the number of wildcards operands within it.
+#[derive(Default, Debug, PartialEq, Eq)]
+pub struct Operand<'s> {
+    pub value: Tree<'s>,
+    pub wildcards: bool,
+    pub eval: bool,
+}


Docs says about tracking number of wildcards, but field has bool only.

JaroslavTulach

the next step is to use it to replace some analysis on both sides.

May I get some explanation why this change is needed?
I don't see any reason to introduce Tree.Eval from engine perspective
- we already have EvalNode!
I don't understand why it should be needed by the IDE
may some documentation be updated to provide some high level perspective
and justify the change?

JaroslavTulach · 2025-12-03T05:40:35Z

* I don't see any reason to introduce `Tree.Eval` from engine perspective
  * we already have [EvalNode](https://github.com/enso-org/enso/blob/9a9602fe47dc92a83a1c3cbd76b31a2bb48b9e0a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/debug/EvalNode.java#L30)!

"Call node" sounds better than "Eval Node"
there are unexpected API changes https://github.com/enso-org/enso/actions/runs/19870865486/job/56949306648?pr=14303#step:14:1184
changes in TreeToIr seem to loose return type of argument-less methods in the IR

kazcw · 2025-12-03T10:57:31Z

* I don't see any reason to introduce `Tree.Eval` from engine perspective
  * we already have [EvalNode](https://github.com/enso-org/enso/blob/9a9602fe47dc92a83a1c3cbd76b31a2bb48b9e0a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/debug/EvalNode.java#L30)!

* "Call node" sounds better than "Eval Node"

* there are unexpected API changes https://github.com/enso-org/enso/actions/runs/19870865486/job/56949306648?pr=14303#step:14:1184

* changes in `TreeToIr` seem to loose return type of argument-less methods in the `IR`

Yeah, I'm debugging this. Probably a missed case in TreeToIr, but this result is strange (a few methods have the problem while other syntactically-similar methods are unaffected)

This reverts commit 41dbdd7.

kazcw · 2025-12-05T10:54:24Z

Making some changes to the treatment of PropertyAccess (thanks to @JaroslavTulach's feedback in enso-org/design/pull/60#discussion_r2587629509). I'm disabling adding the new nodes to PropertyAccess/Ident to get the bulk of the changes merged while I work on the new PropertyAccess logic.

kazcw added the CI: No changelog needed Do not require a changelog entry for this PR. label Nov 17, 2025

kazcw force-pushed the wip/kw/eval-nodes branch 2 times, most recently from 3de84c1 to 861b902 Compare November 26, 2025 19:41

kazcw added the CI: Clean build required CI runners will be cleaned before and after this PR is built. label Nov 27, 2025

Eval nodes

42096c9

kazcw force-pushed the wip/kw/eval-nodes branch from ff69db1 to 42096c9 Compare November 27, 2025 13:19

kazcw added 3 commits November 27, 2025 14:08

fmt

8bf3d77

fmt

9c7a196

Merge remote-tracking branch 'origin/develop' into wip/kw/eval-nodes

fe5583b

kazcw marked this pull request as ready for review November 28, 2025 10:45

kazcw requested review from 4e6, AdRiley, Akirathan, Frizi, JaroslavTulach, PabloBuchu, farmaazon, hubertp, indiv0, somebody1234 and vitvakatu as code owners November 28, 2025 10:45

kazcw commented Nov 28, 2025

View reviewed changes

cleanup

bd3e1ac

farmaazon approved these changes Nov 28, 2025

View reviewed changes

JaroslavTulach requested changes Nov 29, 2025

View reviewed changes

kazcw added 4 commits December 2, 2025 11:20

Review 1

635eb07

Merge remote-tracking branch 'origin/develop' into wip/kw/eval-nodes

cf40998

Review 1/integration

26cb236

update test

b848561

kazcw changed the title ~~Eval nodes~~ Call nodes Dec 2, 2025

fix

1463624

kazcw requested a review from JaroslavTulach December 2, 2025 19:29

debugging

41dbdd7

Merge remote-tracking branch 'origin/develop' into wip/kw/eval-nodes

4b52135

enso-bot bot mentioned this pull request Dec 4, 2025

Using validity bitmap as Arrow does #14409

Merged

3 tasks

kazcw added 3 commits December 4, 2025 10:14

fix

3cde728

Revert "debugging"

d254fe9

This reverts commit 41dbdd7.

Disable non-app-tree Call nodes

3fc9b7d

kazcw changed the title ~~Call nodes~~ Call nodes: App tree roots Dec 5, 2025

update test

437c9a6

Call nodes: App tree roots #14303

Are you sure you want to change the base?

Call nodes: App tree roots #14303

Uh oh!

Conversation

kazcw commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Description

Important Notes

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

farmaazon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JaroslavTulach left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JaroslavTulach commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kazcw commented Dec 3, 2025

Uh oh!

kazcw commented Dec 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

`Call` nodes: App tree roots #14303

`Call` nodes: App tree roots #14303

kazcw commented Nov 17, 2025 •

edited

Loading

JaroslavTulach left a comment •

edited

Loading

JaroslavTulach commented Dec 3, 2025 •

edited

Loading