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

Add --alpha flag to α-normalize output #855

Merged
merged 2 commits into from
Mar 16, 2019
Merged

Add --alpha flag to α-normalize output #855

merged 2 commits into from
Mar 16, 2019

Conversation

Gabriella439
Copy link
Collaborator

Fixes #853

... as suggested by @singpolyma

@singpolyma
Copy link
Contributor

Hmm, so maybe I'm not understanding but I'm seeing this:

$ dhall normalize --alpha < dhall-lang/tests/normalization/success/simple/optionalBuildA.dhall | dhall encode | sha256sum
f0852efdbd6263099a8feb981c1b245b0109bf930bece3099c69ef1f71ade582  -

$ dhall normalize --alpha < dhall-lang/tests/normalization/success/simple/optionalBuildA.dhall | dhall hash
sha256:57937e066a166baf3d712e744d9aec91894f14d7b25138a6f60c7b7577cc986b

Am I missing something? Should not normalize --alpha | encode be exactly the bytes to hash to make the semantic integrity check?

@singpolyma
Copy link
Contributor

Ok, so, using some Debug.Trace I've found the discrepancy. The encoding call used by dhall hash produces a binary encoding that contains this:

(_ (Optional Bool) (Some true))

That is, the binary encoding is a function _ applied to two arguments, vs the dhall encode output where the same expression is set up like this:

((_ (Optional Bool)) (Some true))

The former seems like a bug to me, since nothing in the spec indicates a normalization to a function application with multiple arguments.

@Gabriella439
Copy link
Collaborator Author

@singpolyma: The binary encoding does specify that a function applied to multiple arguments should be uncurried and encoded as a single array:

https://github.com/dhall-lang/dhall-lang/blob/master/standard/binary.md#function-application

However, you were right that the two different commands should have matched and I have another pull request up with the fix:

#859

The short answer is that the output from dhall hash was the correct one

@Gabriella439
Copy link
Collaborator Author

I'll also go ahead and merge this since the --alpha flag is working correctly

@Gabriella439 Gabriella439 merged commit dce46fc into master Mar 16, 2019
@Gabriella439 Gabriella439 deleted the gabriel/alpha branch March 16, 2019 18:20
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.

None yet

2 participants