-
Notifications
You must be signed in to change notification settings - Fork 23
Changes to enable communication with JVerify #308
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
base: main
Are you sure you want to change the base?
Changes from all commits
613fec6
e18f450
f99ea87
780c78e
e0b2bb5
da67291
67c170d
8731075
98c3a4a
b36929a
1404ab0
4f9e815
be0acbf
775600c
22d07ed
cfc4a3a
705cfb4
28c581c
b738175
9e5a509
52e1f3f
e2f5f47
ac9400b
8d10e11
e2715f1
9e7994d
c3aec77
a3e9ec9
46e0e58
44155c0
7400e34
f0068d6
9c06af7
3948293
fbe3a2c
122f63d
8ed03a4
7abbc3e
6197e3c
6a865f0
98ca32b
14957b2
a4d6089
2104a31
d19710f
285ffc8
212226d
5da0ff4
e332bad
5d30ca5
d15ab9a
9d40949
6d7fc13
a223c5d
84d86e7
8da23f6
2797281
49bac76
4b86292
bafb7d2
79ee155
8e20c05
a5b1e4b
39d7d71
da8413c
9022bf6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -294,7 +294,7 @@ private def deserializeValue {α} (bs : ByteArray) (act : Ion SymbolId → FromI | |
| throw s!"Error reading Ion: {msg} (offset = {off})" | ||
| | .ok a => pure a | ||
| let .isTrue p := inferInstanceAs (Decidable (a.size = 1)) | ||
| | throw s!"Expected single Ion value." | ||
| | throw s!"Expected single Ion value, but got {a.size} values." | ||
| let entries := a[0] | ||
| let .isTrue p := inferInstanceAs (Decidable (entries.size = 2)) | ||
| | throw s!"Expected symbol table and value in dialect." | ||
|
|
@@ -1411,6 +1411,11 @@ private instance : FromIon Dialect where | |
|
|
||
| end Dialect | ||
|
|
||
| structure StrataFile where | ||
| filePath : String | ||
| program : Program | ||
| deriving Inhabited | ||
|
|
||
| namespace Program | ||
|
|
||
| private instance : CachedToIon Program where | ||
|
|
@@ -1436,7 +1441,7 @@ def fromIonFragment (f : Ion.Fragment) | |
| commands := ← fromIonFragmentCommands f | ||
| } | ||
|
|
||
| def fromIon (dialects : DialectMap) (dialect : DialectName) (bytes : ByteArray) : Except String Strata.Program := do | ||
| def fileFromIon (dialects : DialectMap) (dialect : DialectName) (bytes : ByteArray) : Except String Strata.Program := do | ||
| let (hdr, frag) ← | ||
| match Strata.Ion.Header.parse bytes with | ||
| | .error msg => | ||
|
|
@@ -1451,5 +1456,60 @@ def fromIon (dialects : DialectMap) (dialect : DialectName) (bytes : ByteArray) | |
| throw s!"{name} program found when {dialect} expected." | ||
| fromIonFragment frag dialects dialect | ||
|
|
||
| def filesFromIon (dialects : DialectMap) (bytes : ByteArray) : Except String (List StrataFile) := do | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems like it shouldn't live in the Program namespace. Maybe move up one level and rename
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this could be StrataFile.filesFromIon and appear below Program things.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I notice I have 0 intuition for what should be in what namespace.
When do you decide to put things in a namespace? Can you give me any guidelines for how to decide to put things in a particular file or in a particular namespace? Without understanding the context very well, my inclination would be to have a single namespace per file, so you wouldn't be able to have multiple definitions with the same name in a single file, but that seems fine to me. Why not put everything in this file in a In any case, right now I'm happy to name/move things in whatever way you prefer, but it wasn't entirely clear to me what you meant with "appear below Program things". If you're willing to move/name the things yourself, that could help speed-up the PR, since then I can approve your commit and @aqjune-aws can be the second approver and code-owner.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For me, I mostly keep things in the same top-level namespace except for three reasons:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm kind of slow at task switching and want to prioritize my regular work commitments. I think I am fine with leaving this as is also; it doesn't matter that much if I don't see it as idiomatic. |
||
| let ctx ← | ||
| match Ion.deserialize bytes with | ||
| | .error (off, msg) => throw s!"Error reading Ion: {msg} (offset = {off})" | ||
| | .ok a => | ||
| if h : a.size = 1 then | ||
| pure a[0] | ||
| else | ||
| throw s!"Expected single Ion value" | ||
|
|
||
| let .isTrue p := inferInstanceAs (Decidable (ctx.size = 2)) | ||
| | throw "Expected symbol table and value" | ||
|
|
||
| let symbols ← | ||
| match SymbolTable.ofLocalSymbolTable ctx[0] with | ||
| | .error (p, msg) => throw s!"Error at {p}: {msg}" | ||
| | .ok symbols => pure symbols | ||
|
|
||
| let ionCtx : FromIonContext := ⟨symbols⟩ | ||
|
|
||
| let ⟨filesList, _⟩ ← FromIonM.asList ctx[1]! ionCtx | ||
|
|
||
| let tbl := symbols | ||
| let filePathId := tbl.symbolId! "filePath" | ||
| let programId := tbl.symbolId! "program" | ||
|
|
||
| filesList.toList.mapM fun fileEntry => do | ||
| let fields ← FromIonM.asStruct0 fileEntry ionCtx | ||
|
|
||
| let some (_, filePathData) := fields.find? (·.fst == filePathId) | ||
| | throw "Could not find 'filePath' field" | ||
|
|
||
| let some (_, programData) := fields.find? (·.fst == programId) | ||
| | throw "Could not find 'program' field" | ||
|
|
||
| let filePath ← FromIonM.asString "filePath" filePathData ionCtx | ||
|
|
||
| let ⟨programValues, _⟩ ← FromIonM.asList programData ionCtx | ||
| let .isTrue ne := inferInstanceAs (Decidable (programValues.size ≥ 1)) | ||
| | throw "Expected program header" | ||
|
|
||
| let hdr ← Ion.Header.fromIon programValues[0] ionCtx | ||
| let dialect ← match hdr with | ||
| | .program name => pure name | ||
| | .dialect _ => throw "Expected program, not dialect" | ||
|
|
||
| let frag : Ion.Fragment := { | ||
| symbols := symbols, | ||
| values := programValues, | ||
| offset := 1 | ||
| } | ||
|
|
||
| let program ← fromIonFragment frag dialects dialect | ||
|
|
||
| pure { filePath := filePath, program := program } | ||
|
|
||
| end Strata.Program | ||
| end | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a style preference, I put instances inside the namespace so this has the name
SourceRange.instToFormat.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. How would I refer to the
instance : ToFormat SourceRange wheredefinition in the location where it's currently at? Also, what are situations where you want to reference an instance definition?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not quite sure of the naming, but probably something like
instToFormatSourceRange. This is a pretty minor style comment honestly.Instance names mostly show up when trying to debug why a rewrite or simp rule isn't going through, and turning on something like
pp.all. Sometimes there is a mismatch on instances in a simp theorem and the current goal (usually with polymorphic instances and coercisions).This instance is so simple and unlikely to show up in proofs, so it doesn't matter. You could leave it as is.