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

bug: issue-4625 repro #4659

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft

bug: issue-4625 repro #4659

wants to merge 8 commits into from

Conversation

crusso
Copy link
Contributor

@crusso crusso commented Aug 15, 2024

Non-dfx repro for issue #4625

test/run-drun/issue-4625.mo Outdated Show resolved Hide resolved
ggreif
ggreif previously approved these changes Aug 15, 2024
Copy link
Contributor

@ggreif ggreif left a comment

Choose a reason for hiding this comment

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

LGTM, maybe you can add the 3 missing newlines too.

crusso and others added 2 commits August 15, 2024 12:07
Co-authored-by: Gabor Greif <gabor@dfinity.org>
Co-authored-by: Gabor Greif <gabor@dfinity.org>
test/run-drun/issue-4625.mo Outdated Show resolved Hide resolved
@ggreif
Copy link
Contributor

ggreif commented Aug 15, 2024

Maybe this should be under test/fail, then?

@crusso
Copy link
Contributor Author

crusso commented Aug 15, 2024

I'm not sure how fixable this is. I wonder if design/IDL-Motoko.md is underspecified on how to translate Motoko/Candid references to type definitions, i.e. translate without or after expansion and that affects the translation of argument sequences.

In addition, both Motoko Null and () map to the Candid null.

so type X = () translates to type X = Null
but
func f(x : X) : async X has Candid type () -> ()
(I think), not, null -> null.

Copy link

github-actions bot commented Aug 15, 2024

Comparing from 9b67c0c to 79710b1:
The produced WebAssembly code seems to be completely unchanged.

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