Skip to content
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

IR check failed in presence of noncomputable #1785

Closed
1 task done
alexjbest opened this issue Oct 27, 2022 · 11 comments · Fixed by #4729
Closed
1 task done

IR check failed in presence of noncomputable #1785

alexjbest opened this issue Oct 27, 2022 · 11 comments · Fixed by #4729
Labels
new-user-papercuts Issue likely to confuse or otherwise negatively affect new users, even if only a little

Comments

@alexjbest
Copy link
Contributor

Prerequisites

  • Put an X between the brackets on this line if you have done all of the following:
    • Checked that your issue isn't already filed.
    • Reduced the issue to a self-contained, reproducible test case.

Description

Code generator sometimes doesn't correctly flag noncomputaility

Steps to Reproduce

In this (somewhat) minimal example I believe def bug should be marked noncomputable, but the IR generator doesn't pick up on this for some reason.

namespace WellFounded

variable {α : Sort u} {C : α → Sort v} {r : α → α → Prop}

unsafe def fix'.impl (hwf : WellFounded r) (F : ∀ x, (∀ y, r y x → C y) → C x) (x : α) : C x :=
  F x fun y _ => impl hwf F y

@[implemented_by fix'.impl]
def fix' (hwf : WellFounded r) (F : ∀ x, (∀ y, r y x → C y) → C x) (x : α) : C x := hwf.fix F x

end WellFounded

namespace Nat

theorem lt_or_eq_of_le {a b : Nat} : a ≤ b → a < b ∨ a = b := sorry
section find
variable (p : Nat → Prop)

private def lbp (m n : Nat) : Prop := m = n + 1 ∧ ∀ k, k ≤ n → ¬p k

variable [DecidablePred p] (H : ∃ n, p n)

private def wf_lbp : WellFounded (lbp p) := by
  refine ⟨let ⟨n, pn⟩ := H; ?_⟩
  suffices ∀ m k, n ≤ k + m → Acc (lbp p) k from fun a => this _ _ (Nat.le_add_left _ _)
  intro m
  induction m with refine fun k kn => ⟨_, fun | _, ⟨rfl, a⟩ => ?_⟩
  | zero => exact absurd pn (a _ kn)
  | succ m IH => exact IH _ (by rw [Nat.add_right_comm]; exact kn)

protected def find_x : {n // p n ∧ ∀ m, m < n → ¬p m} :=
(wf_lbp p H).fix' (C := fun k => (∀n, n < k → ¬p n) → {n // p n ∧ ∀ m, m < n → ¬p m})
  (fun m IH al => if pm : p m then ⟨m, pm, al⟩ else
      have this : ∀ n, n ≤ m → ¬p n := fun n h =>
        (lt_or_eq_of_le h).elim (al n) fun e => by rw [e]; exact pm
      IH _ ⟨rfl, this⟩ fun n h => this n $ Nat.le_of_succ_le_succ h)
  0 fun n h => absurd h (Nat.not_lt_zero _)

protected def find : Nat := (Nat.find_x p H).1
end find
end Nat

variable (R : Type) [∀ n, OfNat R n]

open Classical

noncomputable
def char : Nat := Nat.find (fun n => (1 : R) = 1) ⟨1, rfl⟩

def bug : R := -- IR check failed at 'bug._rarg', error: unknown declaration 'char'
  match char R with
  | 0 => 1
  | _ => 0

Expected behavior: message saying def bug must be marked noncomputable

Actual behavior: IR check failed

Reproduces how often: 100% but seems to need a fairly complicated example

Versions

nightly-10-25

@digama0
Copy link
Collaborator

digama0 commented Oct 27, 2022

minimized:

noncomputable def char (x : α) : Bool := true
def bug (x : α) : Bool := not (char x)
-- IR check failed at 'bug._rarg', error: unknown declaration 'char'

@leodemoura
Copy link
Member

@digama0 Thanks for minimizing the problem. The minimized version makes it clear what the issue is.

@alexjbest We will improve the error message and make it clear code generation failed because char is marked as noncomputable.

@alexjbest
Copy link
Contributor Author

Thanks, and sorry for being so bad at minimizing! All modifications I tried gave the correct error message, so I assumed this problem was somehow more complicated than it appears to actually be :)

@kim-em
Copy link
Collaborator

kim-em commented Sep 20, 2023

Here's another example of the same issue, discovered on zulip. (Original by @kbuzzard, minimized by @digama0.)

class More (R : Type) extends Mul R, Add R

def Foo (R : Type) [Sub R] := R

noncomputable instance bar {R} [Sub R] : More (Foo R) := sorry

instance foo {R} [Sub R] : Add (Foo R) := inferInstance
-- compiler IR check failed at 'foo._rarg', error: unknown declaration 'bar'

@alexjbest alexjbest changed the title IR check failed for intricate noncomputable def involving find IR check failed in presence of noncomputable Oct 31, 2023
@BoltonBailey
Copy link
Contributor

Here are two more examples from Zulip.

@kbuzzard
Copy link
Contributor

Here's another one.

@digama0 digama0 added the new-user-papercuts Issue likely to confuse or otherwise negatively affect new users, even if only a little label Jun 25, 2024
@LibertasSpZ
Copy link

LibertasSpZ commented Jul 9, 2024

Yet another example is here. (And indeed a new-user papercut :) )

@kbuzzard
Copy link
Contributor

kbuzzard commented Jul 11, 2024

Here's another. The user initially asked completely the wrong question, led astray by error: unknown declaration 'CategoryTheory.Limits.pullback after the IR message, when the declaration was known.

kmill added a commit to kmill/lean4 that referenced this issue Jul 11, 2024
Currently, `ll_infer_type` is responsible for telling the user about `noncomputable` when a definition depends on one without executable code. However, this is imperfect because type inference does not check every subexpression. This leads to errors later on that users find to be hard to interpret.

Now, `Lean.IR.checkDecls` has a friendlier error message when it encounters constants without compiled definitions, suggesting to consider using `noncomputable`. While this function is an internal IR consistency check, it is also reasonable to have it give an informative error message in this particular case. The suggestion to use `noncomputable` is limited to just unknown constants.

Some alternatives would be to either (1) create another checker just for missing constants, (2) change `ll_infer_type` to always visit every subexpression no matter if they are necessary for inferring the type, or (3) investigate whether `tests/lean/run/1785.lean` is due to a deeper issue.

Closes leanprover#1785
@kbuzzard
Copy link
Contributor

Here's another. "This is giving weird errors, I'm not sure what's the problem here."

@sgraf812
Copy link

sgraf812 commented Aug 24, 2024

Here is another rather small example:

axiom Later : TypeType
axiom gfix {α : Type} (f : Later α → α) : Later α
def gfix2 {α : Type} (f : Later α → α) : α := f (gfix f)

It certainly triggers a related error message (4.10-rc2):

compiler IR check failed at 'IGDTT.gfix2._rarg', error: unknown declaration 'IGDTT.gfix'

It took me a bit, but of course the error is that gfix indeed has no code and thus I shall add noncomputable to the definitions that use it.

@kbuzzard
Copy link
Contributor

kbuzzard commented Aug 24, 2024

Two people caught out by this in one day today: here's the other one.

kmill added a commit to kmill/lean4 that referenced this issue Sep 7, 2024
Currently, `ll_infer_type` is responsible for telling the user about `noncomputable` when a definition depends on one without executable code. However, this is imperfect because type inference does not check every subexpression. This leads to errors later on that users find to be hard to interpret.

Now, `Lean.IR.checkDecls` has a friendlier error message when it encounters constants without compiled definitions, suggesting to consider using `noncomputable`. While this function is an internal IR consistency check, it is also reasonable to have it give an informative error message in this particular case. The suggestion to use `noncomputable` is limited to just unknown constants.

Some alternatives would be to either (1) create another checker just for missing constants, (2) change `ll_infer_type` to always visit every subexpression no matter if they are necessary for inferring the type, or (3) investigate whether `tests/lean/run/1785.lean` is due to a deeper issue.

Closes leanprover#1785
github-merge-queue bot pushed a commit that referenced this issue Sep 7, 2024
Currently, `ll_infer_type` is responsible for telling the user about
`noncomputable` when a definition depends on one without executable
code. However, this is imperfect because type inference does not check
every subexpression. This leads to errors later on that users find to be
hard to interpret.

Now, `Lean.IR.checkDecls` has a friendlier error message when it
encounters constants without compiled definitions, suggesting to
consider using `noncomputable`. While this function is an internal IR
consistency check, it is also reasonable to have it give an informative
error message in this particular case. The suggestion to use
`noncomputable` is limited to just unknown constants.

Some alternatives would be to either (1) create another checker just for
missing constants, (2) change `ll_infer_type` to always visit every
subexpression no matter if they are necessary for inferring the type, or
(3) investigate whether `tests/lean/run/1785.lean` is due to a deeper
issue.

Closes #1785
tobiasgrosser pushed a commit to opencompl/lean4 that referenced this issue Sep 16, 2024
Currently, `ll_infer_type` is responsible for telling the user about
`noncomputable` when a definition depends on one without executable
code. However, this is imperfect because type inference does not check
every subexpression. This leads to errors later on that users find to be
hard to interpret.

Now, `Lean.IR.checkDecls` has a friendlier error message when it
encounters constants without compiled definitions, suggesting to
consider using `noncomputable`. While this function is an internal IR
consistency check, it is also reasonable to have it give an informative
error message in this particular case. The suggestion to use
`noncomputable` is limited to just unknown constants.

Some alternatives would be to either (1) create another checker just for
missing constants, (2) change `ll_infer_type` to always visit every
subexpression no matter if they are necessary for inferring the type, or
(3) investigate whether `tests/lean/run/1785.lean` is due to a deeper
issue.

Closes leanprover#1785
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-user-papercuts Issue likely to confuse or otherwise negatively affect new users, even if only a little
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants