Content-Length: 348566 | pFad | https://github.com/purescript/purescript/pull/4492

22 Fix VTAs wildcard inferred warning by JordanMartinez · Pull Request #4492 · purescript/purescript · GitHub
Skip to content

Fix VTAs wildcard inferred warning #4492

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

Merged
merged 8 commits into from
Jul 25, 2023
Merged

Conversation

JordanMartinez
Copy link
Contributor

Description of the change

Fixes #4488.

The problem initially arises when we convert the ExprVisibleApp (CST value) into VisibleTypeApp (AST value). Using convertType, whenever we come across a wildcard, we always convert the resulting value into an UnnamedWildcard.

Later, right before we typecheck a module's declarations, we update all UnnamedWildcard to IgnoreWildcard if they appear in a specific context via ignoreWildcardsUnderCompleteTypeSignatures. Because Visible Type Applications are a separate concept, they aren't converted here. Presumably, while typechecking, the compiler emits a warning for each UnnamedWildcard found, which produces the issue.

Thus, there are two ways to resolve this:

  1. (this PR) we update convertType to take another arg indicating whether it was called while converting an ExprVisibleApp into a VisibleTypeApp. If it was, then convert any wildcards into IgnoreWildcard in the first place. This solves the root of the problem but any other usages of convertType don't have this special rule in place.
  2. (not this PR) we update the ignoreWildcards* function to also account for VisibleTypeApp. This is a smaller change but comes at the cost of another traversal through the AST.

AFAICT, convertType is only used in two other places, both of which I think aren't affected by this change:


Checklist:

  • Added a file to CHANGELOG.d for this PR (see CHANGELOG.d/README.md)
  • Added myself to CONTRIBUTORS.md (if this is my first contribution)
  • Linked any existing issues or proposals that this pull request should close
  • Updated or added relevant documentation
  • Added a test for the contribution (if applicable)

@rhendric
Copy link
Member

Don't all the functions that use convertType in this file need to be parameterized? What happens if there's a wildcard in a constraint in a type application, for example?

@JordanMartinez
Copy link
Contributor Author

JordanMartinez commented Jul 23, 2023

Don't all the functions that use convertType in this file need to be parameterized? What happens if there's a wildcard in a constraint in a type application, for example?

Could you give an example of what you mean? Meaning, what would this look like in syntax?

What happens if there's a wildcard in a constraint in a type application, for example?

@rhendric
Copy link
Member

This is a highly contrived example, but I imagine something like this:

class Foo a b c | a -> b c where
  fooMember :: a -> b

f :: forall @a. Array a -> Array (Array a)
f as = [as]

x :: forall c. Array (Foo Int Boolean c => Int -> Boolean)
x = [fooMember]

y :: forall c. Array (Array (Foo Int Boolean c => Int -> Boolean))
y = f @(Foo Int Boolean _ => _) x -- neither wildcard should warn IMO

@JordanMartinez
Copy link
Contributor Author

Don't all the functions that use convertType in this file need to be parameterized? What happens if there's a wildcard in a constraint in a type application, for example?

Using your contrived example, no wildcard is emitted.
But to answer your question, no I don't think they need to parameterized. If there's a wildcard anywhere in the type expression passed to the VTA, it'll be parsed as an IgnoreWildcard rather than an UnnamedWildcard.

@rhendric
Copy link
Member

rhendric commented Jul 24, 2023

Tests that no warnings are issued need to be in tests/purs/warning. If after making that change the test works then I have no further objections, but I believe it won't—in which case, I think parameterizing convertConstraint is sufficient, as it looks like the only function that convertType' uses to call back into itself (though if you could double-check me on this, I'd appreciate it).

@JordanMartinez
Copy link
Contributor Author

Ah.... No, you're right. I see the issue now.

@JordanMartinez
Copy link
Contributor Author

it looks like [convertConstraint is] the only function that convertType' uses to call back into itself (though if you could double-check me on this, I'd appreciate it).

I can confirm that convertConstraint is the only other convert* function used in convertType whose implementation calls back into convertType.

@JordanMartinez JordanMartinez merged commit 4afea2f into master Jul 25, 2023
@JordanMartinez JordanMartinez deleted the fix-vtas-wildcard-warning branch July 25, 2023 14:01
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.

WildcardInferredType warning when using wildcards in a visible type application
2 participants








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: https://github.com/purescript/purescript/pull/4492

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy