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

feat: have IR checker suggest noncomputable #4729

Merged
merged 1 commit into from
Sep 7, 2024

Conversation

kmill
Copy link
Collaborator

@kmill kmill commented 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 #1785

@kmill kmill requested a review from leodemoura as a code owner July 11, 2024 19:28
@kmill
Copy link
Collaborator Author

kmill commented Jul 11, 2024

@leodemoura Keven Buzzard was telling me today that he has to frequently help people with this IR compiler check issue. I'm not sure if this is the correct fix, but putting this error message is this particular place seemed potentially appropriate after studying ll_infer_type, checkDecls, and some other bits of the compiler.

If anything, this PR contains a simpler version of the test case at #1785. Note that removing the typeclass argument makes ll_infer_type catch the missing constant instead.

@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Jul 11, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Jul 11, 2024

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase f0eab4b7b1d2e18d5f7f480743448bd1bf182520 --onto 5f70c1ca64a2c05a5866c47b9eb80a92034433ec. (2024-07-11 19:46:24)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase e5e577865fe74ad17182346a225f8f209fa6e2e4 --onto 3ec55d3d498ce361f5886afee516d16df5cbd6fc. (2024-09-07 22:03:07)

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
@kmill kmill force-pushed the compiler_check_noncomputable branch from fb07ecf to 0820de5 Compare September 7, 2024 21:45
@kmill kmill enabled auto-merge September 7, 2024 21:45
@kmill kmill added this pull request to the merge queue Sep 7, 2024
Merged via the queue into leanprover:master with commit 7a7440f Sep 7, 2024
13 checks passed
tobiasgrosser pushed a commit to opencompl/lean4 that referenced this pull request 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
@kmill kmill deleted the compiler_check_noncomputable branch September 20, 2024 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-medium We may work on this issue if we find the time toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IR check failed in presence of noncomputable
3 participants