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

Drop the --infer flag #573

Merged
merged 4 commits into from
Oct 11, 2024
Merged

Drop the --infer flag #573

merged 4 commits into from
Oct 11, 2024

Conversation

erszcz
Copy link
Collaborator

@erszcz erszcz commented Oct 11, 2024

This removes the --infer CLI and #env{} option. The Erlang VM does dynamic type checking based on type tags embedded in values. This means that it's safe to infer type information from literals and operators according to the same rules that the VM enforces at runtime. Actually, these are one of the few completely credible sources of information, unlike type specs, in which there's room for human error. Removing the option along with the branching in the typechecker should somewhat minimise the amount of code to maintain. Moreover, requiring users to know the available options might lead to a worse impression of what the typechecker is capable of, since if they're not used, the tool is not showing it's full potential.

@erszcz erszcz requested review from xxdavid and zuiderkwast October 11, 2024 14:26
Copy link
Collaborator

@xxdavid xxdavid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I never fully understood why infer wasn't the default (and only) mode. If I recall the discussions correctly, it was added by @zuiderkwast but never fully supported by @josefs, as @josefs wanted to rely solely on typespecs as part of the gradual typing paradigm ("without typespecs, no checking happens, it is up to the programmer to enable checking by adding typespecs, the programmer is in control of what is checked and what is not"). I don't really share this perspective on gradual typing (but I do respect it!) and I agree with you that literals are a trustworthy source of type information.

The pull request itself seems legit to me and I personally approve it, but that's only my take on it.

%% Although there is no spec for f/1 - inferred arity does not match
?_assertNot(type_check_forms(["-spec g() -> fun(() -> integer()).",
"g() -> fun f/1.",
"f(_) -> ok."],
[infer]))
"f(_) -> ok."]))
].

infer_types_test_() ->
%% Checking type_check_expr with inference enabled
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think comment can be now reworded.

Comment on lines 490 to 489
%% infer exact type of record index
?_assertMatch("2",
type_check_expr(_Env = "-record(r, {f}).",
_Expr = "#r.f",
[infer]))
_Expr = "#r.f"))
].
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now checked in the propagate_types_test_ as well.

This removes the --infer CLI and #env{} option.
The Erlang VM does dynamic type checking based on type tags embedded in values.
This means that it's safe to infer type information from literals and operators
according to the same rules that the VM enforces at runtime.
Actually, these are one of the few completely credible sources of information,
unlike type specs, in which there's room for human error.
Removing the option along with the branching in the typechecker should somewhat
minimise the amount of code to maintain.
@erszcz erszcz merged commit 112646d into josefs:master Oct 11, 2024
5 checks passed
@erszcz erszcz deleted the drop-infer-flag branch October 11, 2024 15:30
@josefs
Copy link
Owner

josefs commented Oct 11, 2024

Since I'm no longer active in this project, I don't have an opinion on this PR. But since I was pinged, I thought I'd like to clarify my position here.

When I set out build gradualizer, an important goal for me was to be able to start with an existing Erlang code base and run gradualizer without any errors (or perhaps, if there was already some erroneous type specs, they would be highlighted, but nothing more). Being able to apply gradualizer with minimal cost to an existing code base without being overwhelmed with type errors immediately would lower the threshold to adoption. Then, one could progressively add more and more type specs and get gradually more benefit from gradualizer. The goal was to provide a pay-as-you-go experience. Enabling inference could be added later, as it could potentially generate a lot of type errors up front. It certainly would have for the code base I used to work on.

I had planned, but never implemented, a strict mode which would require type specs everywhere in a file, no any() types anywhere, inference, and all the imports to also be completely typed. The goal for any code base would be to eventually be checked in strict mode everywhere.

@zuiderkwast
Copy link
Collaborator

Hey Josef! I'm not that active either anymore.

Eqwalizer has a strict and a non-strict mode, I believe. They seem to provide some real guarantees by not supporting the detailed type hierarchy (no subtyping?), but it may be a more useful tool in practice? (I don't know.)

David, thanks for reminding me of the discussions around this. I recall that Josef's idea initially was to rely on typespecs only and nothing else, and I supported that. The more we implemented and tested it, though, the more it became apparent that this approach had some problems, like all the OTP library functions having type specs, while arithmetic operators and literals not having any, was a bit problematic. I argued that we should consider other sources of type information too. (What should be used as type information and why?)

The graduality guarantee ("gradual soundness") vs eliminating false positives was another balance between theoretical correctness and practical usefulness, and I believe still is. I found it hard to accept that we can't fully achieve both. (I still haven't accepted it. 😁)

xxdavid added a commit to xxdavid/Gradualizer that referenced this pull request Oct 17, 2024
… return type

Even when a function does not have a spec, we still know that it is a function
and that is of certain arity.

This is probably related to making the infer flag default in josefs#573, as without
infer, we always assumed any() when there wasn't a spec for something.
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.

4 participants