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

A type with a built-in type's name can co-exist. #5537

Open
0rir opened this issue Mar 21, 2024 · 12 comments
Open

A type with a built-in type's name can co-exist. #5537

0rir opened this issue Mar 21, 2024 · 12 comments
Labels
detrap Improving warnings, tweaking the language, etc. (anything to prevent WATs)

Comments

@0rir
Copy link
Contributor

0rir commented Mar 21, 2024

say 'version: ', $*RAKU.version;            # v6.d

class Quadruped { has $.legs = 4; }
class Rat is Quadruped { has $.head = 1 }

# class Rat is Quadruped {}     These variations have parallel behavior.
# class Rat { has $.head = 1}

my $num = 1.1;                                                             
say '$num.raku: ', $num.raku;               # 1.1                               
say '$num.WHAT: ', $num.WHAT;               # (Rat)                             
                                                                                
my $rat = Rat.new();                                                            
say '$rat.WHAT:  ', $rat.WHAT;              # (Rat)                             
say '$rat.^parents:     ', $rat.^parents;   # ((Quadruped))·                    
                                                                                
my Rat $r = 2.2;                            #  see below                         
# Type check failed in assignment to $r; expected Rat but got Rat (2.2)         
# in block <unit> at c.raku line 18

@lizmat
Copy link
Contributor

lizmat commented Mar 21, 2024

Indeed.

But only within that lexical scope.

If you put the classes into a separate scope, there is no issue:

say 'version: ', $*RAKU.version;            # v6.d

{
    class Quadruped { has $.legs = 4; }
    class Rat is Quadruped { has $.head = 1 }

    my $num = 1.1;
    say '$num.raku: ', $num.raku;               # 1.1
    say '$num.WHAT: ', $num.WHAT;               # (Rat)

    my $rat = Rat.new();
    say '$rat.WHAT:  ', $rat.WHAT;              # (Rat)
    say '$rat.^parents:     ', $rat.^parents;   # ((Quadruped))·
}

my Rat $r = 2.2;                            #  see below
dd $r;
version: v6.d
$num.raku: 1.1
$num.WHAT: (Rat)
$rat.WHAT:  (Rat)
$rat.^parents:     ((Quadruped))
Rat $r = 2.2

Like imports, (almost) everything in Raku is lexically scoped. So yes, you can have a Rat that's also a Quadruped.

Note that if you would have removed the Rat from my Rat $r = 2.2; you wouldn't have had an issue either, as the 2.2 would be parsed by the grammar into the system's Rat.

@lizmat
Copy link
Contributor

lizmat commented Mar 21, 2024

I would tend to close this as a non-issue.

Maybe a doc issue would need to be created instead?

@0rir
Copy link
Contributor Author

0rir commented Mar 21, 2024

My issue is not that the exception is thrown. My issue is that a X::Redeclaration is not thrown at the explicit class Rat.
What is the benefit of having two classes co-existing with the same name?

@vrurg
Copy link
Member

vrurg commented Mar 21, 2024

@0rir Say, you import a module that exports Other::Mod::Foo by default, or with a specific :select_export. You get Foo in your local namespace. But, as a matter of fact, you don't need it for your task, though you still need other exports coming with select_export. You need your local Foo. And you can have it.

@ab5tract
Copy link
Collaborator

It seems to me that we could emit a worry when a core class is masked.

But maybe I just secretly like the idea of:

{
     no worries;
     class Rat { ... };
}

@ab5tract ab5tract added the detrap Improving warnings, tweaking the language, etc. (anything to prevent WATs) label Mar 23, 2024
@alabamenhu
Copy link
Contributor

My issue is not that the exception is thrown. My issue is that a X::Redeclaration is not thrown at the explicit class Rat. What is the benefit of having two classes co-existing with the same name?

Quite a few, especially at different lexical scopes. I take advantage of this in DateTime::Timezones, so removing this would break that module. Basically, when you want a drop in replacement for a core class. It's lexically scoped, so it doesn't affect other code.

It seems to me that we could emit a worry when a core class is masked.

I get that I'm biased, but that would then require the above module to be called as no worries; use DateTime::Timezones;, at the risk of now not catching other worries (afaik we can't control which worries to ignore).

@rakudo rakudo deleted a comment from 2colours Mar 25, 2024
@0rir
Copy link
Contributor Author

0rir commented Mar 28, 2024

@ab5tract, that was the trend of my thought.

@lizmat, given that the class is created in the scope, it seems uncertain that the later container typing would be intended for the built-in type or that Any is sufficient when some Rat container was wanted.

@alabamenhu, in this situation the current limitations of no worries can be lessened:

 `constant \Lint :=  do { no worries; use Lint; Lint; }`

@alabamenhu
Copy link
Contributor

constant \Lint :=  do { no worries; use Lint; Lint; }

That's a mouthful for something that already works. I guess the question is, if someone writes a class name that clashes with a core class name, does Raku need to assume that the programmer did this accidentally, or that the programmer did this intentionally? And of those two, who is going to be more bothered by a warning being issued?

              |     Warn     | Don't warn
--------------+--------------+-------------------
Unintentional |    Thanks!   | Likely type error 
  Intentional | Shut up Raku | Runs as expected

Either way, the person who does this unintentionally will figure it out: be it by warning or error. OTOH, the person who does it intentionally will be annoyed by taking advantage of a feature of Raku: fully lexical scoping. The following is perfectly legal:

my $a = 1;
{
    my $a = 2;
}

We don't warn there as lexical scoping explicit feature of Raku, taking advantage of this is enables the not-so-common (but useful when you need 'em) let and temp declarators. We do, though, warn when something is redeclared in the same scope. That makes sense because a declaration implies a first use, but it's seen where it's not, and lexical scoping doesn't apply.

Obviously, I'm team no-warn. Besides, it's not like the core Rat doesn't work. Using 0rir's code:

class Rat { has $.head = 1 }

my           $r1 = 2.2; 
my       Rat $r2 = Rat.new: :1head;
my CORE::Rat $r3 = 2.2; 
my       Rat $r4 = 2.2; # type check fail

The literal 2.2 is equivalent to BEGIN CORE::Rat.new(22,10), and since $r1 is typed Any, there's no problem. The next one uses the MY::Rat as expected, masking the core Rat type. We can still access the core one, though, by calling it with CORE::Rat, and the assignment works in $r3. But if we mess up and type in what's seen in $r4 we'll get a typecheck fail, cluing us in to the issue.

Where I do think we could produce a better error message is if there are two identically named types in a type check to give a warning about that. For instance, instead of

Type check failed in assignment to $r; expected Rat but got Rat (2.2)

We could say

Type check failed in assignment to $r; expected Rat but got CORE::Rat (2.2).
Did you mean to have different types with the same name?

For non-core types that get shadowed, we could potentially point to where they originated if such information is still available when the type check fail occurs.

@ab5tract
Copy link
Collaborator

ab5tract commented Mar 30, 2024

@alabamenhu

First I want to say that I think your suggestion of changing the output to reference CORE:: seems quite workable to me.

But I also wanted to contribute a contrary example WRT lexical scoping and types:

> raku -e 'enum S <a b c>; { enum S <e f g>; }'
===SORRY!=== Error while compiling -e
Redeclaration of symbol 'S'.
at -e:1
------> enum S <a b c>; { enum S⏏ <e f g>; }

EDIT: I should mention that I was actually surprised to see a SORRY and not a worry here. It's related to the scope not being a named package, thus there is no place to install the second S enum but at the (currently occupied) location in the global namespace.

@lizmat
Copy link
Contributor

lizmat commented Mar 30, 2024

That's because enum are by default our. Make the inner or the outer one a my, and there's no conflict.

@ab5tract
Copy link
Collaborator

Doh! I knew that! ... but apparently just not yesterday :)

So for the resolution of this ticket, I support increasing the acuity of the thrown exception as per @alabamenhu's suggestion.

@0rir
Copy link
Contributor Author

0rir commented Apr 3, 2024

@ab5tract, although I endorse @alabamenhu's suggested action, it seems insufficient.

@alabamenhu, your unintentional may include a symbol which is exported to your code without your explicit vetting later-time, anywhere, and anywho your code runs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
detrap Improving warnings, tweaking the language, etc. (anything to prevent WATs)
Projects
None yet
Development

No branches or pull requests

5 participants