Repository navigation
Allow function inlining in quantifiers #1225
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: main2
Are you sure you want to change the base?
Changes from all commits
ebb695a
c67bd85
1fd413f
58f0f61
c4c7c93
1029b88
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 |
|---|---|---|
|
|
@@ -20,6 +20,9 @@ open Std (ToFormat Format format) | |
| namespace CmdEval | ||
|
|
||
| def eval (E : Env) (e : Expression.Expr) : Expression.Expr := | ||
| -- Pre-pass: expand inline factory calls under quantifier/lambda binders, | ||
| -- because `LExpr.eval` does not descend into closed binder bodies. | ||
| let e := LExpr.expandInlineCalls E.exprEnv.config.factory E.exprEnv.config.fuel e | ||
|
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. Wiring only here means the bug persists for every other caller of
As written, two callers will silently diverge in behaviour for the same input — confusing for anyone debugging. |
||
| LExpr.eval E.exprEnv.config.fuel E.exprEnv e | ||
|
|
||
| def updateError (E : Env) (e : EvalError Expression) : Env := | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| /- | ||
| Copyright Strata Contributors | ||
|
|
||
| SPDX-License-Identifier: Apache-2.0 OR MIT | ||
| -/ | ||
|
|
||
| /- | ||
| Reproducer + verification: inline functions inside quantifier bodies. | ||
|
|
||
| - Confirms the original bug: `LExpr.eval` does not expand an `inline` call | ||
| inside a `forall` body. | ||
| - Confirms the fix: `LExpr.expandInlineCalls` correctly expands the call | ||
| under the binder, and then `LExpr.eval` produces the fully reduced form. | ||
| -/ | ||
| module | ||
|
|
||
| meta import all Strata.DL.Lambda.LExprEval | ||
|
|
||
| meta section | ||
|
|
||
| namespace Lambda | ||
| open LExpr LTy.Syntax LExpr.SyntaxMono | ||
|
|
||
| private abbrev TP : LExprParams := ⟨Unit, Unit⟩ | ||
| private instance : Coe String TP.Identifier where coe s := Identifier.mk s () | ||
|
|
||
| -- inline polyEq<a>(x, y) := ∀ (z : a), z == z | ||
| private def polyEqFns : Array (LFunc TP) := | ||
| #[{ name := "polyEq", | ||
| typeArgs := ["a"], | ||
| attr := #[.inline], | ||
| inputs := [("x", mty[%a]), ("y", mty[%a])], | ||
| output := mty[bool], | ||
| body := some esM[∀ (%a): (%0 == %0)] }] | ||
|
|
||
| private def polyState : LState TP := | ||
| { (LState.init : LState TP) with | ||
| config := { (LState.init : LState TP).config with factory := .ofArray polyEqFns } } | ||
|
|
||
| -- Top-level call expands as expected. | ||
| private def topCall : LExpr TP.mono := esM[(((~polyEq : int → int → bool) #1) #2)] | ||
| private def topExpect : LExpr TP.mono := esM[∀ (int): (%0 == %0)] | ||
| /-- info: true -/ | ||
| #guard_msgs in | ||
| #eval (LExpr.eval 100 polyState topCall) == topExpect | ||
|
|
||
| -- Same call wrapped inside a forall: original bug — eval leaves it untouched. | ||
| private def quantCall : LExpr TP.mono := | ||
| esM[∀ (int): (((~polyEq : int → int → bool) %0) %0)] | ||
| private def quantExpect : LExpr TP.mono := | ||
| esM[∀ (int): (∀ (int): (%0 == %0))] | ||
|
|
||
| /-- info: true -/ | ||
| #guard_msgs in | ||
| #eval (LExpr.eval 100 polyState quantCall) == quantCall | ||
|
|
||
| /-- info: false -/ | ||
| #guard_msgs in | ||
| #eval (LExpr.eval 100 polyState quantCall) == quantExpect | ||
|
|
||
| -- Fix: expandInlineCalls descends through the forall and rewrites the call. | ||
| /-- info: true -/ | ||
| #guard_msgs in | ||
| #eval (LExpr.expandInlineCalls polyState.config.factory 100 quantCall) == quantExpect | ||
|
|
||
| -- And piping the pre-pass before eval also yields the expected form. | ||
| /-- info: true -/ | ||
| #guard_msgs in | ||
| #eval | ||
| (LExpr.eval 100 polyState | ||
| (LExpr.expandInlineCalls polyState.config.factory 100 quantCall)) == quantExpect | ||
|
|
||
| end Lambda | ||
|
Comment on lines
+47
to
+73
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. Coverage is thin for the change's blast radius. Please add
|
||
| 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.
Two issues with this expansion arm:
eval.eval(line ~223) inlines onattr.contains .inline || constrArgAt findInlineIfConstr || (hasInlineIfAllCanonical && args.all isCanonicalValue). This pre-pass only checks.inline. So aninlineIfConstrfactory whose argument inside a binder would be a constructor at runtime will silently not get expanded by the pre-pass while it would outside the binder. If this asymmetry is intentional (because we can't determine canonicality ofbvars), please add a one-line comment saying so; otherwise mirroreval's logic.computeTypeSubst/applySubst/substFvarsLiftingchain is now a verbatim copy ofeval's inline branch. Please factor a private helpertryInlineExpand : Factory T → LExpr T.mono → Option (LExpr T.mono)(returningnonewhen the call doesn't apply) and route both call sites through it. That also makes the proof obligations in the top-level review stateable in one place.Minor:
Bool.and_eq_true_iff.mp h |>.1is more readable than the current(Bool.and_eq_true_iff.mp h).1once the helper exists, and matches the idiom used inSemantics.lean:2539.