Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rich atoms don't cache suspended fields #8710

Closed
JaroslavTulach opened this issue Jan 9, 2024 · 6 comments · Fixed by #8712
Closed

Rich atoms don't cache suspended fields #8710

JaroslavTulach opened this issue Jan 9, 2024 · 6 comments · Fixed by #8712
Assignees
Labels

Comments

@JaroslavTulach
Copy link
Member

Suspended atom fields, as implemented by

may not be cached properly, when the amount of atom fields is higher than four. Following program shall cache the value generated by the java.util.Random generator, but it doesn't:

polyglot java import java.util.Random as R

type Num
    Holder a b c d ~num

    new  = Num.Holder "a" "b" "c" "d" (R.new.nextInt)

main =
   fbl = Num.new
   f = fbl.num
   n = fbl.num
   [ f, n, f == n ]

It is able to print [712182466, -522235373, False].

@JaroslavTulach
Copy link
Member Author

Removing any of the a, b, c or d dummy atom fields fixes the problem: [-1299288440, -1299288440, True]

@JaroslavTulach
Copy link
Member Author

Allocating the Num.Holder with primitive types like Num.Holder 1 2 3 4 (R.new.nextInt) doesn't fix the problem.

@enso-bot
Copy link

enso-bot bot commented Jan 13, 2024

Jaroslav Tulach reports a new STANDUP for yesterday (2024-01-12):

Progress: - TCK PR integrated: #8685

Next Day: Integrate Atoms, look towards BigDecimal support

@enso-bot
Copy link

enso-bot bot commented Jan 15, 2024

Jaroslav Tulach reports a new STANDUP for yesterday (2024-01-14):

Progress: - Fixing atom NI failures: 6d890de

  • merging to resolve conflicts:
  • refactoring AtomNewInstanceNode & co.: 85c396f
  • trying to write getUncached version
  • AtomConstructorTest: d433218
  • uncached variant: 8675747
  • unboxlingLayouts optimized in uncached variant: f8d8a98 It should be finished by 2024-01-16.

Next Day: Integrate Atoms, look towards BigDecimal support

@enso-bot
Copy link

enso-bot bot commented Jan 16, 2024

Jaroslav Tulach reports a new STANDUP for yesterday (2024-01-15):

Progress: - "Atom PR" is ready: #8712

Next Day: Review Test API, look towards BigDecimal support

@JaroslavTulach JaroslavTulach moved this from 🔧 Implementation to 👁️ Code review in Issues Board Jan 16, 2024
@mergify mergify bot closed this as completed in #8712 Jan 16, 2024
mergify bot pushed a commit that referenced this issue Jan 16, 2024
Fixes #8710 by making sure suspended atom fields support works also for "normal" `Atom` instances without any special `Layout`. Moves all _atom related_ classes into single package and hides as much of classes as possible by making them _package private_.
@github-project-automation github-project-automation bot moved this from 👁️ Code review to 🟢 Accepted in Issues Board Jan 16, 2024
@enso-bot
Copy link

enso-bot bot commented Jan 17, 2024

Jaroslav Tulach reports a new STANDUP for yesterday (2024-01-16):

Progress: - "Atom PR" integrated: #8712

Next Day: BigDecimal (via Complex number) support

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant