-
Notifications
You must be signed in to change notification settings - Fork 45
Struct/record type declarations with named field access in Boole #1128
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
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 |
|---|---|---|
|
|
@@ -942,6 +942,33 @@ def evalBindingSpec | |
| | .error e => | ||
| logError loc e | ||
| pure <| .empty gctx | ||
| | .record b => | ||
| let nameInfo := args[b.nameIndex.toLevel].info | ||
| let (_, ident) ← | ||
| match nameInfo with | ||
| | .ofIdentInfo i => | ||
| pure (i.loc, i.val) | ||
| | _ => | ||
| logInternalError nameInfo.loc s!"Expected ident" | ||
| return tctx | ||
| assert! tctx.bindings.size = 0 | ||
| let gctx := tctx.globalContext | ||
| let gctx := gctx.ensureDefined ident (.type [] none) | ||
| let dialects := (← read).dialects | ||
| let fieldsArg := args[b.fieldsIndex.toLevel] | ||
| match extractFieldsFromBindings dialects fieldsArg.arg with | ||
| | .ok fields => | ||
| let recordIndex := gctx.findIndex? ident |>.getD (gctx.vars.size - 1) | ||
| let recordType := mkDatatypeTypeRef loc recordIndex #[] | ||
| let ctorInfo : Array ConstructorInfo := #[{ name := ident ++ "_mk", fields }] | ||
| let (gctx, errors) := gctx.expandFunctionTemplates | ||
| dialectName loc ident recordType ctorInfo | ||
| b.functionTemplates | ||
| errors.forM (logError loc) | ||
| pure <| .empty gctx | ||
| | .error e => | ||
| logError loc e | ||
| pure <| .empty gctx | ||
| | .tvar b => | ||
| let ident := evalBindingNameIndex args b.nameIndex | ||
| pure <| tctx.push { ident, kind := .tvar loc ident } | ||
|
|
@@ -1666,7 +1693,29 @@ partial def elabExpr (tctx : TypingContext) (stx : Syntax) : ElabM Tree := do | |
| let einfo : ElabInfo := { loc := loc, inputCtx := tctx } | ||
| let name := elabIdent stx[0] | ||
| let some binding := tctx.lookupVar name | ||
| | logError loc s!"Unknown expr identifier {name}" | ||
| | -- Desugar dot notation: "base.field" → T..field(base) | ||
| -- A single dot (not "..") separates the variable from the field name. | ||
| let dotParts := name.splitOn ".." | ||
| let parts := name.splitOn "." | ||
| if parts.length == 2 && dotParts.length == 1 then | ||
| let baseName := parts[0]! | ||
| let fieldName := parts[1]! | ||
| if let some baseBinding := tctx.lookupVar baseName then | ||
| let typeExprOpt : Option TypeExpr := match baseBinding with | ||
| | .bvar _ (.expr tp) => some tp | ||
| | .fvar _ (.expr tp) => some tp | ||
| | _ => none | ||
| if let some (.fvar _ typeIdx _) := typeExprOpt then | ||
| if let some typeName := tctx.globalContext.nameOf? typeIdx then | ||
| let selectorName := typeName ++ ".." ++ fieldName | ||
| if let some selectorIdx := tctx.globalContext.findIndex? selectorName then | ||
| let baseExpr : Expr := match baseBinding with | ||
| | .bvar idx _ => .bvar loc idx | ||
| | .fvar idx _ => .fvar loc idx | ||
| let appExpr : Expr := .app loc (.fvar loc selectorIdx) (.expr baseExpr) | ||
|
Comment on lines
+1696
to
+1715
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. The dot-notation desugaring here is the most delicate part of the PR. Three observations: (a) Chained access is silently unsupported.
(b) String-split identifier parsing is fragile. Today it relies on Boole's grammar parsing (c) No direct unit tests. The only coverage is
See also the main review body's suggested proof: a theorem (or informal comment) that |
||
| let info : ExprInfo := { toElabInfo := einfo, expr := appExpr } | ||
| return .node (.ofExprInfo info) #[] | ||
| logError loc s!"Unknown expr identifier {name}" | ||
| return default | ||
| match binding with | ||
| | .bvar idx k => do | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -233,18 +233,6 @@ private def toCoreMonoBind (b : BooleDDM.MonoBind SourceRange) : TranslateM (Cor | |
| match b with | ||
| | .mono_bind_mk _ ⟨_, n⟩ ty => return (mkIdent n, ← toCoreMonoType ty) | ||
|
|
||
| def toCoreTypedUn (m : SourceRange) (ty : Boole.Type) (op : String) (a : Core.Expression.Expr) : TranslateM Core.Expression.Expr := do | ||
| let .int _ := ty | ||
| | throwAt m s!"Unsupported typed operator type: {repr ty}" | ||
| let iop : Core.Expression.Expr := .op () ⟨s!"Int.{op}", ()⟩ none | ||
| return .app () iop a | ||
|
|
||
| def toCoreTypedBin (m : SourceRange) (ty : Boole.Type) (op : String) (a b : Core.Expression.Expr) : TranslateM Core.Expression.Expr := do | ||
| let .int _ := ty | ||
| | throwAt m s!"Unsupported typed operator type: {repr ty}" | ||
| let iop : Core.Expression.Expr := .op () ⟨s!"Int.{op}", ()⟩ none | ||
| return mkCoreApp iop [a, b] | ||
|
|
||
| private def bvWidth (m : SourceRange) (ty : Boole.Type) : TranslateM Nat := | ||
| match ty with | ||
| | .bv1 _ => return 1 | ||
|
|
@@ -262,6 +250,20 @@ private def toCoreBvBin (m : SourceRange) (ty : Boole.Type) (op : String) (a b : | |
| let n ← bvWidth m ty | ||
| return mkCoreApp (.op () ⟨s!"Bv{n}.{op}", ()⟩ none) [a, b] | ||
|
|
||
| def toCoreTypedUn (m : SourceRange) (ty : Boole.Type) (op : String) (a : Core.Expression.Expr) : TranslateM Core.Expression.Expr := do | ||
| match ty with | ||
| | .int _ => | ||
| let iop : Core.Expression.Expr := .op () ⟨s!"Int.{op}", ()⟩ none | ||
| return .app () iop a | ||
| | _ => toCoreBvUn m ty op a | ||
|
|
||
| def toCoreTypedBin (m : SourceRange) (ty : Boole.Type) (op : String) (a b : Core.Expression.Expr) : TranslateM Core.Expression.Expr := do | ||
| match ty with | ||
| | .int _ => | ||
| let iop : Core.Expression.Expr := .op () ⟨s!"Int.{op}", ()⟩ none | ||
| return mkCoreApp iop [a, b] | ||
| | _ => toCoreBvBin m ty op a b | ||
|
|
||
| private def toCoreExtensionalEq | ||
| (m : SourceRange) | ||
| (ty : Boole.Type) | ||
|
|
@@ -373,9 +375,21 @@ def toCoreExpr (e : Boole.Expr) : TranslateM Core.Expression.Expr := do | |
| | .bvand m ty a b => toCoreBvBin m ty "And" (← toCoreExpr a) (← toCoreExpr b) | ||
| | .bvor m ty a b => toCoreBvBin m ty "Or" (← toCoreExpr a) (← toCoreExpr b) | ||
| | .bvxor m ty a b => toCoreBvBin m ty "Xor" (← toCoreExpr a) (← toCoreExpr b) | ||
| | .bvshl m ty a b => toCoreBvBin m ty "Shl" (← toCoreExpr a) (← toCoreExpr b) | ||
| | .bvshl m ty a b => | ||
| match ty with | ||
| | .int _ => do | ||
| -- Integer << n encoded as multiplication by 2^n; n must be a literal. | ||
| let n ← match b with | ||
| | .natToInt _ ⟨_, n⟩ => pure n | ||
| | _ => throwAt m "Integer << requires a constant (literal) shift amount" | ||
| toCoreTypedBin m ty "Mul" (← toCoreExpr a) (.intConst () (Int.ofNat (1 <<< n))) | ||
| | _ => toCoreBvBin m ty "Shl" (← toCoreExpr a) (← toCoreExpr b) | ||
|
Comment on lines
+378
to
+386
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. Two small things on the integer-left-shift encoding: (a) Error message could be more specific. (b) Unbounded shift amount. (c) Test coverage is benchmark-only. |
||
| | .bvushr m ty a b => toCoreBvBin m ty "UShr" (← toCoreExpr a) (← toCoreExpr b) | ||
| | .bvsshr m ty a b => toCoreBvBin m ty "SShr" (← toCoreExpr a) (← toCoreExpr b) | ||
| | .bvult m ty a b => toCoreBvBin m ty "ULt" (← toCoreExpr a) (← toCoreExpr b) | ||
| | .bvule m ty a b => toCoreBvBin m ty "ULe" (← toCoreExpr a) (← toCoreExpr b) | ||
| | .bvugt m ty a b => toCoreBvBin m ty "UGt" (← toCoreExpr a) (← toCoreExpr b) | ||
| | .bvuge m ty a b => toCoreBvBin m ty "UGe" (← toCoreExpr a) (← toCoreExpr b) | ||
| | .old _ _ a => | ||
| return oldifyExpr (← get).currentInoutNames (← toCoreExpr a) | ||
| | _ => throw (.fromMessage s!"Unsupported expression: {repr e}") | ||
|
|
@@ -747,6 +761,7 @@ private def registerCommandSymbols (cmd : BooleDDM.Command SourceRange) : List B | |
| -- Procedure names are referenced by call statements directly and are not Expr.fvar symbols. | ||
| | .boole_procedure _ _ _ _ _ _ _ | .command_procedure _ _ _ _ _ _ => [] | ||
| | .command_datatypes _ ⟨_, decls⟩ => decls.toList.map (fun _ => false) | ||
| | .struct_decl _ _ _ => [false] | ||
| | .command_block _ _ => [] | ||
| | .command_axiom _ _ _ => [] | ||
| | .command_distinct _ _ _ => [] | ||
|
|
@@ -886,6 +901,20 @@ def toCoreDecls (cmd : BooleDDM.Command SourceRange) : TranslateM (List Core.Dec | |
| } .empty] | ||
| | .command_datatypes _ ⟨_, decls⟩ => | ||
| return [.type (.data (← decls.toList.mapM toCoreDatatypeDecl)) .empty] | ||
| | .struct_decl m ⟨_, tname⟩ ⟨_, fields⟩ => | ||
| -- Desugar `type T := { f1: A, f2: B }` to a single-constructor datatype. | ||
| -- DDM registration (type + selectors) is handled by @[declareRecord]. | ||
| let args ← fields.toList.mapM toCoreBinding | ||
| let ctorName := tname ++ "_mk" | ||
| let constr : Lambda.LConstr Unit := { | ||
| name := mkIdent ctorName | ||
| args := args | ||
| testerName := s!"{tname}..is{ctorName}" | ||
| } | ||
| return [.type (.data [{ name := tname | ||
| typeArgs := [] | ||
| constrs := [constr] | ||
| constrs_ne := by simp }]) .empty] | ||
|
|
||
| /-- Render a `Boole.Program` to a format object using the provided `GlobalContext` and | ||
| `DialectMap`. These should come from the originating `Strata.Program` (i.e. `env.globalContext` | ||
|
|
||
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.
CI blocker:
Run lint checks (stable)fails on the three newpanic!uses here and at line 2194. The Strata lint only flags newly added panics without-- nopanic:okannotations (pre-existing ones in the same file, e.g.addDatatypeBindingsat 1134–1138, are grandfathered), so this is strictly a new-code policy issue.Two ways to fix:
-- nopanic:okafter eachpanic!line, matching the grandfathered sibling sites. Minimal change; keeps behaviour identical. Suggested if you want to keep the parallel structure with the existing datatype code.panic!with error returns through theExcept StringchannelparseNewBindingsalready uses. Line 1148–1155 already handles similar malformed-metadata cases vianewBindingErr "..."; return none; thedecideProp … | return panic!branches at lines 1156 and 1158 could do the same.addDatatypeBindings's existing panics would still be there but are out of scope.Same choice applies to line 2194:
| a => panic! s!"Expected ident for record name {repr a}"which could become an
Except Stringerror like the rest ofaddRecordBindings, or gain a-- nopanic:ok. Note that the symmetric site inaddDatatypeBindings(line 2134 area for the datatype name) also panics without annotation; if you take the quick-fix route here, consider tagging those too as a small consistency pass.Either way, CI stays red until this is resolved.