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

Dyno: add workaround for finding 'serialize' methods in ChapelIO #26691

Merged
merged 1 commit into from
Feb 12, 2025

Conversation

DanilaFe
Copy link
Contributor

@DanilaFe DanilaFe commented Feb 11, 2025

Closes https://github.com/Cray/chapel-private/issues/7090.
Probably closes https://github.com/Cray/chapel-private/issues/7098 (the assertion is during canPass for compiler-generated tuple functions, which this PR ought to disable).

In production, we generate serialize functions for types if no serialize function is present anywhere in the program. Dyno, however, is an incremental compiler, and as a result, it cannot use "anywhere in the program" properties (not to mention, due to various scoping rules, the "anywhere in the program" rule can get you into hot water). Instead, Dyno uses a type's definition point to search for serialize etc.

This would be nice, except that we define some serialization functions for builtin types in ChapelIO, away from the types' definition points. This PR works around this in Dyno, by deliberately including ChapelIO as a place where we search for existing serialize fns.

In the ideal world, we would like for this to be solved using interfaces. In this world, serialize would not be automatically generated except when auto-satisfying the writeSerializable interface, and only using functions available in the current scope. This would solve the problem with checking "anywhere in the program", as well as other non-dyno-related problems ("serialize should only be special if the interface is present, but right now it's special anyway"). However, we are not yet in the ideal world, and production cannot handle that sort of implementation today.

Reviewed by @benharsh -- thanks!

Testing

  • dyno tests
  • paratest

@DanilaFe DanilaFe merged commit 636f2ae into chapel-lang:main Feb 12, 2025
9 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