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.

Sign up for GitHub

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

Jump to bottom

Imported name shadowed by local type not reported #8868

Open
2 tasks
radeusgd opened this issue Jan 25, 2024 · 5 comments
Open
2 tasks

Imported name shadowed by local type not reported #8868

radeusgd opened this issue Jan 25, 2024 · 5 comments
Assignees
@Akirathan
Labels
-compiler s-research-needed Status: the task will require heavy research to complete x-on-hold

Comments

@radeusgd
Copy link
Member

radeusgd commented Jan 25, 2024 •
edited by Akirathan

Within a project with 2 files:

Mod.enso

type A
   Ctor1

make_a = A.Ctor1

and Main.enso

from Standard.Base import all

import project.Mod.A 
from project.Mod import make_a

type A
    Ctor2

foo (a : A) -> A = a

main =
    IO.println (foo (A.Ctor2))
    IO.println (foo make_a)

running Main.enso will result in:

Ctor2
Execution finished with an error: Type error: expected `a` to be A, but got A.
        at <enso> Main.foo(src\Main.enso:9:1-20)
        at <enso> Main.main<arg-1>(src\Main.enso:13:17-26)
        at <enso> Main.main(src\Main.enso:13:5-27)

There's 2 issues here:

  1. The imported name A is shadowed by the type definition type A but this is not reported. If not error, at least a warning should be reported to the user about the shadowing.
  2. The Type_Error message is confusing: expected A but got A - ???. The problem is that the qualified name of the types differs. We should record the qualified name of each type and if the short-names are equal, we should display the qualified names to allow the user to disambiguate types with same short names.

Tasks
Beta Give feedback
@radeusgd
Copy link
Member Author

Moreover, adding make_a = A.Ctor2 to Main.enso makes the program run:

Ctor2
Ctor2

because it also shadows the imported make_a.

This shadowing also does not issue any error nor warning.

I think shadowed symbols (methods and types) should result in a warning.

@Akirathan
Copy link
Member

According to @4e6 and @JaroslavTulach, type shadowing is perfectly fine. Imagine you would like to define your Boolean type. In that case, you would not be able to import all symbols from Standard.Base. Nevertheless, it should at least report a warning, as shadowing might not always be the desired behavior.

The type error message is indeed confusing. I will try to improve it.

@Akirathan Akirathan mentioned this issue May 3, 2024
Closed
4 tasks
@enso-bot
Copy link

enso-bot bot commented May 3, 2024

Pavel Marek reports a new STANDUP for today (2024-05-03):

Progress: - Writing tests and test utilities to create a whole temporary project structure. It should be finished by 2024-05-07.

@radeusgd
Copy link
Member Author

radeusgd commented May 6, 2024

According to @4e6 and @JaroslavTulach, type shadowing is perfectly fine. Imagine you would like to define your Boolean type. In that case, you would not be able to import all symbols from Standard.Base.

That's exactly the rationale behind the hiding construct.

If you want to define your own Boolean - that's perfectly fine, you need to use:

from Standard.Base import all hiding Boolean

@Akirathan
Copy link
Member

Marking this issue as x-on-hold, as it would be very useful to have more specific information in BindingsMap.resolvedImports. For example, for these two modules:

local.Proj.Mod:

type T

local.Proj.Main:

from local.Proj.Mod import T

if we look into the bindings map of local.Proj.Main, there is a ResolvedImport with the target of ResolvedModule("local.Proj.Mod"), but it should rather be ResolvedType("local.Proj.Mod.T").

Adding this kind of information to the BindingsMap is tracked in #6729

Without this additional information, implementing a static linting compiler pass to detect shadowing of imported symbols is not worth the effort at the moment.

Closing the associated PR #9863 with tests and a skeleton of the linting pass. Let's re-open it once we have a better BindingMap.

@Akirathan Akirathan added s-research-needed Status: the task will require heavy research to complete x-on-hold labels Jun 11, 2024
@Akirathan Akirathan mentioned this issue Jun 11, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Akirathan Akirathan

Labels
-compiler s-research-needed Status: the task will require heavy research to complete x-on-hold
Projects
Issues Board
Status: 🗄️ Archived
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Shadowing identifiers produces warnings enso-org/enso
2 participants
@radeusgd @Akirathan