Ident_env: Assume unknown types are core types #1073
Conversation
Types that couldn't be found in the environment are looked up in the list of predefined core types. This lookup is not expected to fail, as indicated by the presence of an `assert false`. This removes the core type lookup and assumes all unknown types are core types.
Predefined identifiers, paths, references and exceptions are not used.
Core type are no longer predefined and are instead assumed to be resolved and constructed on the fly. This replaces the pre-definition of all the core type with the `type_of_core_type` function, which construct core types when needed and only hardcode the representation of known core types. It doesn't fail.
|
The third commit removes the core type lookup in With this change, Odoc no longer crash when encountering unknown core types (eg. using a patched compiler). |
|
The only way to add custom core types that I know of is to patch the compiler, so no test is possible. But I believe this is already tested. For example, |
| (** Lookup a type in the environment. If it isn't found, it's assumed to be a | ||
| core type. *) |
There was a problem hiding this comment.
That means we are confident there is no error in our building of the environment.
Would it make sense to include a simple check that Ident.name id is part of a list of predefined types? Kind of similar to what was before, but much more lightweight and easily removable.
There was a problem hiding this comment.
This patch is intended to be used with patched compilers and not in prevision of new core types. Not having a check is intended.
To more easily debug wrongly built environments, we'd just need to differentiate core types from types that didn't resolve in the generated HTML. We could wrap core types in a span with a class (they don't need to be styled differently for this purpose but could be) but I think we can already debug this case without this.
There was a problem hiding this comment.
I agree that the check is somehow redundant with OCaml having successfully compiled the file, and that it will be annoying for patched compilers.
That might be better than the "easier debugging" of odoc wrongly assigning types to core types.
There was a problem hiding this comment.
Thanks for reviewing :) Now that we agree, I'll merge!
Types that couldn't be found in the environment are looked up in the list of predefined core types.
This lookup is not expected to fail, as indicated by the presence of an
assert false.This removes the core type lookup and assumes all unknown types are core types.
The second commit removes unused Predefined identifiers, paths, references and exceptions. This code required some maintenance in the past.