Skip to content

Conversation

@JaroslavTulach
Copy link
Member

@JaroslavTulach JaroslavTulach commented Dec 20, 2025

Pull Request Description

Important Notes

  • two tests in test/Base_Tests had to be updated
  • to reflect the new memoizing semantic of suspended blocks
  • originally I called the suspended blocks lazy blocks
    • after a bit of constructive critique I changed the PR to stick to the existing suspended block terminology

Checklist

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

  • The documentation has been updated, if necessary.
  • All code follows the
    Scala,
    Java,
  • Unit tests have been written where possible.
    • Test the computation is done just once

Copy link
Member

@Akirathan Akirathan left a comment

Choose a reason for hiding this comment

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

compute =
    ref.put (ref.get+1)
    "Value"

is not a lazy block. It is a method. It should be executed every time is is called. That is the correct and expected semantics. There is no such thing as a lazy block, unless you specifically define it.

I don't believe that "Snowflake tests is slow, because I am running it via enso --run test/Snowflake_Tests" is a sufficient justification for changing the parameterless method semantics in this way.

To make Snowflake tests faster:

  • Run just a single test file, ano not the entire project
  • Or start introducing lazy atom fields, just like we have them in benchmarks.

@github-project-automation github-project-automation bot moved this to 🔴 Changes requested in Issues Board Dec 20, 2025
* @return the truffle nodes corresponding to `block`
*/
private def processBlock(block: Expression.Block): RuntimeExpression = {
if (block.suspended) {
Copy link
Member Author

Choose a reason for hiding this comment

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

  • block.suspended check

@JaroslavTulach
Copy link
Member Author

JaroslavTulach commented Dec 20, 2025

is not a lazy block

  • If you look into the code, you'll find method processBlock with block.suspended check
  • in the IR terms there can be (and is) suspended block
  • I often use suspended/lazy interchangeably - but I can change the title to use suspended

It is a method.

  • it not easy to differentiate between argument-less method and variable in lambda calculus
  • but I think we can agree that x = 42 is a binding
  • and then we can say x =\n 42 is also a binding
    • this is time is is a suspended binding
  • all this PR is proposing is to make sure such suspended bindings are evaluated at most once
  • but yeah, it is a change in semantics and for example Fetch_Spec tests are failing because of that
  • there might be a value in being compatible

@JaroslavTulach JaroslavTulach changed the title Make sure lazy blocks are evaluated only once Make sure suspended blocks are evaluated only once Dec 20, 2025
@jdunkerley
Copy link
Member

jdunkerley commented Dec 21, 2025

What's the implication for?

type Date_Time
     now =
          Date_Time.now_builtin

I do agree that a = 42 vs a = \n 42 is confusing, and I had Radek teach me a while ago to be aware.

What would:

    a = if Date_Time.now.seconds > 30 then 30 else
         0

do currently? Does it eval as an assignment or become a function?

@JaroslavTulach
Copy link
Member Author

type Date_Time
     now =
          Date_Time.now_builtin

This remains unchanged. It is a definition of static method on Date_Time. This PR touched only code working with local variables...

    a = if Date_Time.now.seconds > 30 then 30 else
         0

It is an assignment. One would need to write:

     a = 
          if Date_Time.now.seconds > 30 then 30 else
             0   

to make it a function/thunk (prior to this PR) or suspended block (with changes in this PR).

@jdunkerley
Copy link
Member

type Date_Time
     now =
          Date_Time.now_builtin

This remains unchanged. It is a definition of static method on Date_Time. This PR touched only code working with local variables...

    a = if Date_Time.now.seconds > 30 then 30 else
         0

It is an assignment. One would need to write:

     a = 
          if Date_Time.now.seconds > 30 then 30 else
             0   

to make it a function/thunk (prior to this PR) or suspended block (with changes in this PR).

Thanks - then I think I am happy with this.
I do think most (read all) of us lib devs forget that splitting a line means it is a function, not a variable.

I do worry it is a little subtle and something like ... in the syntax to indicate it is lazy might be clearer. However, I think the memorized behaviour is probably more what we expect when we write the above code as a local variable.

ccing @AdRiley and @GregoryTravis as they may not have seen this

@JaroslavTulach
Copy link
Member Author

  • there might be a value in being compatible
  • if we agree deferred, suspended lazily assigned variables are useful
  • and we decide to keep the current semantics of suspended blocks
  • then we need new syntax, the ideas we discussed so far:
    • suspended execution sign - e.g. ... at the end of the line x = (a b c 42 f)...
    • different assignment operator than = for example x ~= a b c 42 f
    • or prefixing such variables like ~x = a b c 42 f
  • any other syntax idea?

@Akirathan
Copy link
Member

Although suspended lazily assigned variables can be modeled with lazy atom fields, it may be worth to invest some time in this feature. Because rewriting simple variables to lazy fields is hard - I was personally doing that in test/Benchmarks. And it is also difficult to read that code.

Syntax: if my vote counts for something, I am voting for the third option - prefixing with tilde - ~x = a b c 42 f because:

  • Three dots at the end of the line are simple to miss
  • The ~= operator looks weird.

in the IR terms there can be (and is) suspended block

Until now, I had no idea that Expression.Block has boolean suspended field. Turns out, that for example

resolved_local_file =
parts = Path_Helpers.split_path path
# Special case - if there are no parts, it means path was "", so we use that as base.
base_part = parts.get 0 if_missing=""
# Handle special case when starts with ~
base = if is_home.not then File_Helpers.get_file base_part else
if Enso_File.cloud_project_parent_directory == Nothing then File.home else Enso_File.home
rest = parts.drop 1
rest.fold base acc-> part-> acc.resolve_single_part part
is Method.Explicit with suspended block as the body. Interesting.

Still, I would not touch that and rather introduce a special syntax for suspended lazily assigned variables.

@GregoryTravis
Copy link
Contributor

I'm happy with either syntax, ~= or ~x.

Would this change mean that this is fixed? #9814 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: 🔴 Changes requested

Development

Successfully merging this pull request may close these issues.

5 participants