Skip to content

Commit 80b2cc2

Browse files
committed
Address review feedback: simplify parser, Java codegen, and formatting
- Parser.lean: use Id.run do + early return for comment parsing, replace startsWith with utf8ByteSize check (comment 6) - Java/Gen.lean: match on DDM category name (Init.Num/Init.Decimal) instead of Java type string for builder parameter generation (comment 7) - Format.lean: collapse comma/space/newline seq formatting into single sepBy arm, eliminating duplication (comment 8) - Java/Gen.lean: consolidate getSeparator with SepFormat.toIonName by adding fromCategoryName? and moving Ion-specific functions to Strata.DDM.Ion (comment 9)
1 parent f530354 commit 80b2cc2

5 files changed

Lines changed: 63 additions & 60 deletions

File tree

Strata/DDM/AST.lean

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -200,25 +200,14 @@ def toString : SepFormat → String
200200
| .spacePrefix => "spacePrefixSepBy"
201201
| .newline => "newlineSepBy"
202202

203-
def toIonName : SepFormat → String
204-
| .none => "seq"
205-
| .comma => "commaSepList"
206-
| .space => "spaceSepList"
207-
| .spacePrefix => "spacePrefixedList"
208-
| .newline => "newlineSepList"
209-
210-
def fromIonName? : String → Option SepFormat
211-
| "seq" => some .none
212-
| "commaSepList" => some .comma
213-
| "spaceSepList" => some .space
214-
| "spacePrefixedList" => some .spacePrefix
215-
| "newlineSepList" => some .newline
203+
def fromCategoryName? : QualifiedIdent → Option SepFormat
204+
| q`Init.Seq => some .none
205+
| q`Init.CommaSepBy => some .comma
206+
| q`Init.SpaceSepBy => some .space
207+
| q`Init.SpacePrefixSepBy => some .spacePrefix
208+
| q`Init.NewlineSepBy => some .newline
216209
| _ => none
217210

218-
theorem fromIonName_toIonName_roundtrip (sep : SepFormat) :
219-
fromIonName? (toIonName sep) = some sep := by
220-
cases sep <;> rfl
221-
222211
instance : ToString SepFormat where
223212
toString := SepFormat.toString
224213

Strata/DDM/Format.lean

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -382,26 +382,16 @@ private partial def ArgF.mformatM {α} : ArgF α → FormatM PrecFormat
382382
| .none =>
383383
.atom <$> entries.foldlM (init := .nil) fun p a =>
384384
return (p ++ (← a.mformatM).format)
385-
| .comma =>
385+
| .comma | .space | .newline =>
386+
let sepStr := match sep with | .comma => ", " | .space => " " | .newline => "\n" | _ => ""
386387
if z : entries.size = 0 then
387388
pure (.atom .nil)
388389
else do
389-
let f i q s := return s ++ ", " ++ (← entries[i].mformatM).format
390390
let a := (← entries[0].mformatM).format
391-
.atom <$> entries.size.foldlM f (start := 1) a
392-
| .space =>
393-
if z : entries.size = 0 then
394-
pure (.atom .nil)
395-
else do
396-
let f i q s := return s ++ " " ++ (← entries[i].mformatM).format
397-
let a := (← entries[0].mformatM).format
398-
.atom <$> entries.size.foldlM f (start := 1) a
391+
.atom <$> entries.size.foldlM (fun i q s => return s ++ sepStr ++ (← entries[i].mformatM).format) (start := 1) a
399392
| .spacePrefix =>
400393
.atom <$> entries.foldlM (init := .nil) fun p a =>
401394
return (p ++ " " ++ (← a.mformatM).format)
402-
| .newline =>
403-
.atom <$> entries.foldlM (init := .nil) fun p a =>
404-
return (if p.isEmpty then p else p ++ "\n") ++ (← a.mformatM).format
405395

406396
private partial def ppArgs (f : StrataFormat) (rargs : Array Arg) : FormatM PrecFormat :=
407397
if rargs.isEmpty then

Strata/DDM/Integration/Java/Gen.lean

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
module
77

88
public import Strata.DDM.AST
9+
import Strata.DDM.Ion
910
import Strata.DDM.Integration.Categories
1011

1112
namespace Strata.Java
@@ -137,13 +138,7 @@ def argDeclKindToJavaType : ArgDeclKind → JavaType
137138

138139
/-- Get Ion separator name for a list category, or none if not a list. -/
139140
def getSeparator (c : SyntaxCat) : Option String :=
140-
match c.name with
141-
| q`Init.Seq => some "seq"
142-
| q`Init.CommaSepBy => some "commaSepList"
143-
| q`Init.NewlineSepBy => some "newlineSepList"
144-
| q`Init.SpaceSepBy => some "spaceSepList"
145-
| q`Init.SpacePrefixSepBy => some "spacePrefixedList"
146-
| _ => none
141+
SepFormat.fromCategoryName? c.name |>.map SepFormat.toIonName
147142

148143
/-- Extract the QualifiedIdent for categories that need Java interfaces, or none for primitives. -/
149144
partial def syntaxCatToQualifiedName (cat : SyntaxCat) : Option QualifiedIdent :=
@@ -322,15 +317,22 @@ def opDeclToJavaRecord (dialectName : String) (names : NameAssignments) (op : Op
322317
323318
def generateBuilders (package : String) (dialectName : String) (d : Dialect) (names : NameAssignments) : String :=
324319
let methods (op : OpDecl) :=
325-
let fields := op.argDecls.toArray.map argDeclToJavaField
326-
let (ps, as, checks) := fields.foldl (init := (#[], #[], #[])) fun (ps, as, checks) f =>
327-
match f.type with
328-
| .simple "java.math.BigInteger" _ =>
329-
(ps.push s!"long {f.name}",
330-
as.push s!"java.math.BigInteger.valueOf({f.name})",
331-
checks.push s!"if ({f.name} < 0) throw new IllegalArgumentException(\"{f.name} must be non-negative\");")
332-
| .simple "java.math.BigDecimal" _ => (ps.push s!"double {f.name}", as.push s!"java.math.BigDecimal.valueOf({f.name})", checks)
333-
| t => (ps.push s!"{t.toJava} {f.name}", as.push f.name, checks)
320+
let (ps, as, checks) := op.argDecls.toArray
321+
|>.foldl (init := (#[], #[], #[])) fun (ps, as, checks) decl =>
322+
let name := escapeJavaName decl.ident
323+
let cat := decl.kind.categoryOf.name
324+
if cat == q`Init.Num then
325+
-- Long parameter must be non-negative.
326+
(ps.push s!"long {name}",
327+
as.push s!"java.math.BigInteger.valueOf({name})",
328+
checks.push s!"if ({name} < 0) throw new IllegalArgumentException(\"{name} must be non-negative\");")
329+
else if cat == q`Init.Decimal then
330+
(ps.push s!"double {name}",
331+
as.push s!"java.math.BigDecimal.valueOf({name})",
332+
checks)
333+
else
334+
let t := (argDeclKindToJavaType decl.kind).toJava
335+
(ps.push s!"{t} {name}", as.push name, checks)
334336
let methodName := escapeJavaName op.name
335337
let returnType := names.categories[op.category]!
336338
let recordName := names.operators[(op.category, op.name)]!

Strata/DDM/Ion.lean

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,29 @@ end Ion.Ion
8282

8383
namespace Strata
8484

85+
namespace SepFormat
86+
87+
def toIonName : SepFormat → String
88+
| .none => "seq"
89+
| .comma => "commaSepList"
90+
| .space => "spaceSepList"
91+
| .spacePrefix => "spacePrefixedList"
92+
| .newline => "newlineSepList"
93+
94+
def fromIonName? : String → Option SepFormat
95+
| "seq" => some .none
96+
| "commaSepList" => some .comma
97+
| "spaceSepList" => some .space
98+
| "spacePrefixedList" => some .spacePrefix
99+
| "newlineSepList" => some .newline
100+
| _ => none
101+
102+
theorem fromIonName_toIonName_roundtrip (sep : SepFormat) :
103+
fromIonName? (toIonName sep) = some sep := by
104+
cases sep <;> rfl
105+
106+
end SepFormat
107+
85108
/--
86109
Represents an Ion value that is either a symbol/string or a non-empty
87110
s-expression. The size proof ensures termination in recursive deserialization.

Strata/DDM/Parser.lean

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -227,20 +227,19 @@ private partial def whitespace : ParserFn := fun c s =>
227227
let j := c.next' i h
228228
let curr := c.get j
229229
match curr with
230-
| '/' =>
231-
-- Treat as comment unless a token starting with "//" exists (e.g., //@pre)
232-
match c.tokens.matchPrefix c.inputString i with
233-
| some tk => if tk.startsWith "//" then s
234-
else andthenFn (takeUntilFn (fun c => c = '\n')) whitespace c (s.next c j)
235-
| none =>
236-
andthenFn (takeUntilFn (fun c => c = '\n')) whitespace c (s.next c j)
237-
| '*' =>
238-
-- Treat as comment unless a token starting with "/*" exists
239-
match c.tokens.matchPrefix c.inputString i with
240-
| some tk => if tk.startsWith "/*" then s
241-
else andthenFn (finishCommentBlock (pushMissingOnError := false)) whitespace c (s.next c (c.next j))
242-
| none =>
243-
andthenFn (finishCommentBlock (pushMissingOnError := false)) whitespace c (s.next c (c.next j))
230+
| '/' => Id.run do
231+
-- Treat // as comment unless a token covering both chars exists (e.g., //@pre)
232+
if let some tk := c.tokens.matchPrefix c.inputString i then
233+
if tk.utf8ByteSize >= 2 then
234+
return s
235+
andthenFn (takeUntilFn (fun c => c = '\n')) whitespace c (s.next c j)
236+
| '*' => Id.run do
237+
-- Treat /* as comment unless a token covering both chars exists
238+
if let some tk := c.tokens.matchPrefix c.inputString i then
239+
if tk.utf8ByteSize >= 2 then
240+
return s
241+
andthenFn (finishCommentBlock (pushMissingOnError := false))
242+
whitespace c (s.next c (c.next j))
244243
| _ =>
245244
s
246245
else s

0 commit comments

Comments
 (0)