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

Uncaught exception with a typing error. #5876

Closed
vicuna opened this Issue Jan 7, 2013 · 4 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link
Collaborator

commented Jan 7, 2013

Original bug ID: 5876
Reporter: jm
Assigned to: @gasche
Status: closed (set by @xavierleroy on 2015-12-11T18:18:21Z)
Resolution: fixed
Priority: normal
Severity: minor
Fixed in version: 4.01.0+dev
Category: typing

Bug description

% ocamlc -version
4.01.0+dev10-2012-10-16
% head -n -0 *.ml
module type T = sig end
module T = functor (T:sig end) -> struct end
type t = T.t
% ocamlc -i t.ml
File "t.ml", line 3, characters 9-12:
Error: Unbound type constructor T.tFatal error: exception Not_found

This "Fatal error: exception Not_found" should not be.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 7, 2013

Comment author: @gasche

I plead guilty as a this could be a spellchecking-related bug. I'll have a look in the week.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 8, 2013

Comment author: @alainfrisch

Fixed by commit 13214. Gabriel: do you agree with the fix? I don't think that the Env.fold_* functions should ever raise Not_found.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 8, 2013

Comment author: @gasche

Thanks for tackling the issue so quickly!

My understanding of the problem is the following (sorry for repeating this to you Alain, but it may be an useful point of reference if someone else stumbles on this).
The Env.find_all function(s) has an option to refine folding, not to all the environment but to only the identifiers 'id' accessible under a given module longindent 'L' (that is directly L.id, with no search in the submodules of L). It expects that the longident is correct, in that it correctly returns a module component. In the present case, the longident is incorrect: it denotes an unapplied functor rather than a module.

Your fix effectively mutes the error in this case. Another option would be to check before calling Env.find_all (in fact the Env.fold_* functions called at spellchecking time) that the longident is valid. I think that would be nicer from an user point of view: during the narrow_unbound_lid_error step (that refines an unbound-foo error by finding the shortest wrong prefix of the component), we could check in the Ldot(mlid, _) case that mlid, when it exists, indeed denotes a module component.

This would require adding a new kind of error for this. Do you think it is acceptable? (I'm ready to try to contribute a patch if you don't wish to invest in refining this.)

However, both in this case and in the Lapply situation of #5046, I do not really understand why this is handled as a lexical error rather than a typing error. I would rather expect a Typemod error rather than a Typetexp error.

I think do not understand the typer code well enough to suggest a change there; it seems that the Typetexp.find_foo functions is called precisely when we only have a longindent rather than a Path.t (already type-checked), as in the latter case Env.find_foo (which uses no spell-checking logic) is preferred. What is the logic for using Typetexp instead of typechecking the path into a Path.t first, and why does it suffices in practice to do spell-checking on Typetexp only (are Env.find_* functions assumed never to fail?), I don't know.

PS: When I contributed the spell-checking patch I found the Env.fold_* functions and was delighted to use them; I believed that there had been there for a long time. I looked after your fix, and they apparently come from the huge bin-annot blurb patch, and are seemingly never used anywhere expect in the spellchecking logic. I was right to take care to flush the error buffer before doing the spell-checking, but for the wrong reason: the problem is not that spellchecking may take time, but that it calls non-battle-tested code and is therefore prone to be buggy.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 8, 2013

Comment author: @alainfrisch

Yes, I believe it would make sense to have a proper error message in that case.

@vicuna vicuna closed this Dec 11, 2015

@vicuna vicuna added the typing label Mar 14, 2019

@vicuna vicuna added the bug label Mar 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.