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

Weird “P6M Merging GLOBAL symbols failed” issue #3011

Open
AlexDaniel opened this issue Jun 21, 2019 · 6 comments
Open

Weird “P6M Merging GLOBAL symbols failed” issue #3011

AlexDaniel opened this issue Jun 21, 2019 · 6 comments
Labels

Comments

@AlexDaniel
Copy link
Contributor

AlexDaniel commented Jun 21, 2019

Basically, the module has:

class X::IO is Exception {

At the same time there's an X::IO role in core.

After 84b0e38 (bisected) there's now this error:

# P6M Merging GLOBAL symbols failed: duplicate definition of symbol IO

Previously the module worked fine.

IRC discussion: https://colabti.org/irclogger/irclogger_log/perl6-dev?date=2019-06-21#l143

<jnthn> AlexDaniel: Huh, that's very odd
<jnthn> Does getting rid of the X:: in the name of the exception that I introduced help matters?
<jnthn> It's lexical so it doesn't matter what it's called
<jnthn> But it still feels off

See also #2498.

@AlexDaniel AlexDaniel added BLOCKER Preventing the next release of rakudo, or just needing attention before the release weird labels Jun 21, 2019
@AlexDaniel AlexDaniel mentioned this issue Jun 21, 2019
17 tasks
@jonathanstowe
Copy link
Contributor

Hi,
As I said in the #2498 I've used this pattern with the inner exception like that all over the place (56 instances in 29 distributions,) and overall there are likely to be quite a few obvious names like X::IO (I've just find that I have used X::NoFile in two places.) So there is always a risk that this is going to happen, if not with exceptions in the core but also in other modules.

Regardless of the outcome of this particular issue I think I'll rename the exceptions such that there is less risk of a name collision (e.g. I'll make the offending class here X::SMBus::IO,) also I'll see if the documentation can be improved to make it clearer that there is a risk of collision if obvious names are used for exceptions. Also it might be worth pointing out that there is no hard reason to use X:: for user exceptions if you don't need or want the "exporting" behaviour.

An additional thought was that perhaps the special casing of the X:: package could be amended slightly such that if a name is found that clashes in this way when it is merged into X:: rather than blowing up it just skips the attempt to attach it there (possibly with a warning,) and just introduces the fully qualified name (so in this case as RPi::Device::SMBus::X::IO.) Though that only moves the problem down to user code as the old name will suddenly not be "exported" if the name clashes like this.

@niner
Copy link
Collaborator

niner commented Jun 22, 2019

It's been years since I implemented this, but I think we're just running into the limits of lexical symbol importing.

In Perl 6, packages are actually nested. X::IO consists of 2 parts: X and IO. For both we create Stashes and The IO Stash get's put into X's Stash in the IO key. These stashes form a global hierarchy. While new top level stashes are indeed put into lexical symbols, we cannot do the same for nested ones. Instead they are still put into the existing namespace and thus may collide with symbols that are already there.

jonathanstowe added a commit to jonathanstowe/RPi-Device-SMBus that referenced this issue Jun 22, 2019
The X::IO class was causing a namespace collision with the similarly
named class in the rakudo core.

This came up in rakudo/rakudo#3011
@AlexDaniel AlexDaniel removed the BLOCKER Preventing the next release of rakudo, or just needing attention before the release label Jun 27, 2019
@AlexDaniel
Copy link
Contributor Author

AlexDaniel commented Jun 27, 2019

Blocker label removed because it was fixed in the module and I think did have a similar problem in my code before. It's a weird issue, and it's interesting that the commit in question causes this. But I think I've seen the recommendation at some point to name your exceptions as My::Module::Name::X::Foo, so I wonder if starting your own classes/roles with X:: is common at all.

Anyway, not a blocker for now unless someone has a good reason for it to be, but please feel free to discuss the issue further.

@jonathanstowe
Copy link
Contributor

I think it's the

class Foo {
     class X::IO is Exception {
     }
}

That is causing the problem, the naive expectation being that it is Foo::X::IO except the 'X' namespace being special it gets merged into the top level X namespace. I'm beginning to thing that it's not encourage the naming of the user defined exceptions like that unless there is a compelling reason.

@melezhik
Copy link

still an issue for me:

Julias-MacBook-Pro:sparrowdo alex$ raku -v
Welcome to Rakudo™ v2021.10-134-gd1bda7b56.
Implementing the Raku® Programming Language v6.d.
Built on MoarVM version 2021.10-124-g6fd623291.

@melezhik
Copy link

Julias-MacBook-Pro:sparky alex$ raku -c -MSparrow6::DSL -MHash::Merge -e 1
===SORRY!=== Error while compiling -e
P6M Merging GLOBAL symbols failed: duplicate definition of symbol Merge
at -e:1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants