ufmg-smite / lean-smt

Tactics for discharging Lean goals into SMT solvers.
Apache License 2.0
84 stars 18 forks source link

Smt rw tac: Added the import to Smt.Reconstruction.Certifying #65

Closed mhk119 closed 10 months ago

tomaz1502 commented 10 months ago

This is looking good! Two comments about the last commit (please make a PR before pushing to main, so its easier to review):

tomaz1502 commented 10 months ago

Also, in smtRw you could iterate through arrays instead of indices to avoid unsafe operations, like:

  for innerArr in arr do
    let m := innerArr.size
    if m > 1 then
      for j in [: m-1] do
        let r ← mv'.rewrite (← mv'.getType) (mkAppN assoc #[innerArr[m-j-2]!]) true
        mv' ← mv'.replaceTargetEq r.eNew r.eqProof

You could also iterate through the Exprs in innerArr.reverse instead of the indices. It looks like you are always skipping the last element of each array? if j = 0 then m - j - 2 = m - 2, whereas the last element is at position m - 1

mhk119 commented 10 months ago

This is looking good! Two comments about the last commit (please make a PR before pushing to main, so its easier to review):

  • It's not necessary to do replaceMainGoal [mv] at the end of evalSMTRw. The goal is already closed when you do mv'.refl
  • The variable m inside both for-loops in smtRw does not need to be mutable

Oh thanks. This is nicer.

mhk119 commented 10 months ago

Also, in smtRw you could iterate through arrays instead of indices to avoid unsafe operations, like:

  for innerArr in arr do
    let m := innerArr.size
    if m > 1 then
      for j in [: m-1] do
        let r ← mv'.rewrite (← mv'.getType) (mkAppN assoc #[innerArr[m-j-2]!]) true
        mv' ← mv'.replaceTargetEq r.eNew r.eqProof

You could also iterate through the Exprs in innerArr.reverse instead of the indices. It looks like you are always skipping the last element of each array? if j = 0 then m - j - 2 = m - 2, whereas the last element is at position m - 1

Yes. I am skipping the last element deliberately. This is also why I iterated over indices instead. I can try iterating through arrays though.