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

[refactor] typing/predef: introduce a variant of predefined type constructors #13460

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

gasche
Copy link
Member

@gasche gasche commented Sep 20, 2024

predef.ml contains the definition of all "predefined" type constructors, that are defined in the compiler rather than the standard library. Some type analyses need special cases for those predefined types, whose property often cannot be defined from their definition.

Currently the check for these special cases is done with an or-cascade of boolean checks, for example Typeopt.classify (for compilation of lazy thunks):

ocaml/typing/typeopt.ml

Lines 92 to 101 in d753ea1

| Tconstr (p, _args, _abbrev) ->
if Path.same p Predef.path_float then Float
else if Path.same p Predef.path_lazy_t then Lazy
else if Path.same p Predef.path_string
|| Path.same p Predef.path_bytes
|| Path.same p Predef.path_array
|| Path.same p Predef.path_nativeint
|| Path.same p Predef.path_int32
|| Path.same p Predef.path_int64 then Addr
else begin

This is not very robust, because no one remembers to update those analyses when new predefined types are introduced. And of course, the code I quoted above does not contain the logic one could expect for the predefined types floatarray, eff and continuation.

To make this code more robust, we add in typing/Predef a variant type of all predefined type constructors (it is split in two classes, the "abstract" type constructors and the "data" type constructors that have a datatype definition, because analyses typically only need to special-case the abstract ones). They are used just enough in the module to force people to add a new variant each time they add a predefined type.

The PR also includes a find_type_constr function to decide whether am arbitrary type constructor is predefined, and uses it to update the definition in Typeopt.classify -- which revealed the missing cases.

It is useful for some type-directed analyses to special case
predefined type constructors -- see for example
{!Typeopt.classify}. Currently the only way to do it is to list all
special cases by hand, with an explicit path check. If we add more
predefined types, we typically forget to update those lists of cases,
and the new types are not properly taken into account by the analyses.

In this PR we introduce a variant type of all predefined types, so that
analyses can be defined by pattern-matching on this variant type, and will
get an exhaustivity warning if a new case is added.

(We make sure to also use the variant type internally -- in the module
itself -- so that people don't forget to extend it when they add a new
predefined type.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant