don't warn on using Foo: _

[ericphanson]

closes #58667

re- #58666, embarrassingly enough I did use cursor to help with this 1-line change because I don't know C.

[github-actions]

Hello! I am a bot.

Thank you for your pull request!

I have assigned @LilithHafner to this pull request.

@LilithHafner can either choose to review this pull request themselves, or they can choose to find someone else to review this pull request.

Note: If you are a Julia committer, please make sure that your organization membership is public.

[LilithHafner]

@Keno or @adienes, are you familiar with this code path and willing to review this?

[Keno]

I don't know how I feel about this. I do think import Foo as _ is cleaner in its intent to load Foo. The loading side effect of the colon-full version of using is a bit of an incidental thing almost. As for the implementation, if we make this work, it just do nothing at that point rather than actually assigning var"_".

[Keno]

We have @test_warn and @test_nowarn.

[ericphanson]

thanks, fixed. I copied https://github.com/JuliaLang/julia/blob/master/test/misc.jl#L81-L101 after searching for WARNING in the tests, maybe that one needs an update

[ericphanson]

As for the implementation, if we make this work, it just do nothing at that point rather than actually assigning var"_".

Yeah, to me that seems correct, matching import Foo as _. Currently I get

julia> var"_"
ERROR: syntax: all-underscore identifiers are write-only and their values cannot be used in expressions
Stacktrace:
 [1] top-level scope
   @ REPL[14]:1

which is what I wanted, but if I re-run the @test_nowarn test I get

julia> @test_nowarn using .Mod58667: _
WARNING: ignoring conflicting import of Mod58667._ into Main
Test Failed at /Users/eph/julia-native/usr/share/julia/stdlib/v1.13/Test/src/Test.jl:984
  Expression: contains_warn(read(fname, String), $(Expr(:escape, Test.var"#@test_nowarn##0#@test_nowarn##1"())))
   Evaluated: contains_warn("WARNING: ignoring conflicting import of Mod58667._ into Main\n", Test.var"#@test_nowarn##0#@test_nowarn##1"())
ERROR: There was an error during testing

so I guess it does think it imported a binding _ which is not ideal.

edit: fixed in 858c9ca

[ericphanson]

I do think import Foo as _ is cleaner in its intent to load Foo. The loading side effect of the colon-full version of using is a bit of an incidental thing almost

blue style and yas style say "never use import" (well, "prefer using" for blue, the stronger language is from yas) to simplify the import vs using situation, since we want users to qualify when overriding methods for local clarity, and we don't want to say "sometimes use import, sometimes use using" since then folks will do it wrong. So to me this provides an elegant way to load a package with using without bringing in any names.

[ericphanson]

ok, I think I fixed the implementation issue just by returning early like in the next branch, and I strengthened the test to verify we can do using Foo: _ twice without warnings, and to verify that the module is loaded and side-effects triggered. This way the "incidental" loading of Foo will trigger a test failure if it stops happening, hopefully preventing that from changing. Therefore if we accept this PR we should commit to using Foo: _ loading Foo.

[Keno]

The dropping of _ is a syntactic feature, so this should happen at lowering, not in the runtime, but I do think we need to finish discussing whether this is a good idea first. I'm sympathetic to wanting to avoid import, but I don't know that the existence of a style guide is really strong enough reason to decide semantics. Like, a priori using Foo: _ doesn't make sense, because it's not just an assignment to _ - it's also a read of the binding _ in Foo. I think it's very odd to drop that read - we don't do that anywhere else. Of course, I don't know what else this would mean, but I'm very hesitant to invent a special meaning for _ when a perfectly good alternative does already exist.

[ericphanson]

Yeah, it does seem like a weird special case, I agree. I think the syntax would be handy but agree that like all new syntax it deserves scrutiny and a high bar.