Make @warn_unqualified_access applicable to all accessible declarations

@warn_unqualified_access is a useful tool for preventing global namespace pollution: without it, it is all too easy to import a module and break existing code.

For reasons that are unclear (due to the attribute predating Swift Evolution entirely), @warn_unqualified_access cannot be applied to global variables or constants. That’s a shame, as they can suffer from a similar issue to methods. The danger isn’t quite as pronounced without the potential for overloading, but it is still important. Many programmers emulate the behavior by declaring a global caseless enumeration with the same name as the module, but I don’t see why that should remain necessary.

8 Likes

+1. I think this is a great idea. This is a small (an hopefully uncontroversial) change that would have an outsized positive effect on how we package our software and interact with other libraries.

3 Likes

Going through the list of declarations, I think it should be possible to apply the attribute to:

  • import
    • Everything added to the top-level namespace through an import declaration implicitly gets @warn_unqualified_access applied
    • This can be superseded by more specific import declarations
  • let
  • var
    • Possibly for get and set individually, such that it applies based on which one is being used? That might be too confusing, though, and I can’t conceive of a use case
  • typealias
    • Like access control modifiers, this would apply separately from the type being referred to
    • Unlike access control modifiers, it would not inherit @warn_unqualified_access from the type it is referring to
  • func
  • enum
  • struct
  • class
    • This would not be inherited by subclasses
  • protocol
    • This would not be inherited by inheriting protocols
2 Likes

I think you have a fundamental misunderstanding as to how this attribute is used. Sequence.max is labeled @warn_unqualified_access so that users don't invoke it within an extension of Sequence when they mean to use Swift.max.

Thanks, I didn’t catch that nuance.

I think it's reasonable to extend @warn_unqualified_access to all members and global variables.

Namespacing, though, is a separate issue, and I wouldn't be interested in contorting this attribute--whose purpose is to prevent confusable overloads from being accidentally invoked--to be an ersatz namespace.

I think you are confusing function with original intent. This is an “ersatz namespace”, and the purpose was originally to prevent confusing overloads from being accidentally invoked.

Though honestly, it isn’t really “ersatz”. Modules are always namespaces.

I'm not sure what you're getting at here. @warn_unqualified_access has nothing to do with namespaces. For example, inside an extension of Sequence, writing min() causes a warning because of @warn_unqualified_access, but self.min() is fine. That's how this attribute is used.

Yes, you can apply it to a top-level function in order to require prompt users to prefix any function call with the module name. But that's just a generalization of the rule, not its raison d'être.

It makes sense to generalize the attribute to apply to members that aren't functions, yes. (Although--it will be an interesting thought as to how to apply this to enum cases. I assume we wouldn't want to extend this to initializers and subscripts as they're essentially always "qualified"--but I wonder how things work today with callAsFunction...)

But all this stuff about import rules, etc., is a stretch, in my view. The standard library has standardized on caseless enums for namespaces while a fully built-out namespace feature isn't available. I don't doubt that it is certainly possible to craft a set of rules such that @warn_unqualified_access can replace caseless enums, but I don't see how that's a more satisfactory solution, particularly since (as the attribute says) it's only going to be a warning. Nor is it necessary in order to accomplish what you outline as your motivation.

For the particular example of max (and min), a more robust solution is possible.

Name lookup should recognize that Sequence.max takes 0 arguments as an instance method (or 1 argument in a type-level context).

So a call-site with 2 arguments should not accept Sequence.max as a candidate, because Sequence.max cannot match max(_:_:).

I’m not sure why it doesn’t already work that way. I thought that was one of the biggest benefits made possible by SE–0111 “Remove type system significance of function argument labels”.

I think it's reasonable to consider that this clarification is useful for human readers and not merely machines; @warn_unqualified_access is a warning and not an error. There are certainly a number of situations where we can devise rules that make code unambiguous for the compiler, but that doesn't mean that the rule alone is usable.

I’m not sure what you’re getting at here.

I find it blazingly obvious to human readers that, within an extension of Sequence, writing max() means self.max(), while max(x, y) means Swift.max(x, y).

I have never met a human who had any doubts whatsoever about that distinction.

It is only the compiler which has a shortcoming here.

I am confident humans will be greatly relieved when the compiler becomes smart enough to recognize that argument labels are part of the function name (as mandated by SE–0111), so it will match functions to call-sites accordingly.

Then people can write max() when they mean max(), and they can write max(x, y) when they mean max(x, y), and the compiler will understand what it means just like any human reader would.

After all, argument labels are part of function names, so name-lookup should behave accordingly.

1 Like

Huh? The compiler doesn't have any shortcoming here; it understands perfectly. And, moreover, there are no argument labels involved. I am confused by what you're saying.

extension Sequence where Element : Comparable {
  func f() -> Element? { max() }
  // warning: use of 'max' treated as a reference to instance method in protocol 'Sequence'
}

[1, 2, 3].f() // 3

If the compiler itself were confused, it'd be unable to compile the code, and it wouldn't be just a warning.

The sole purpose of @warn_unqualified_access here is to give the human a heads-up in case they meant to use one when they wrote the other. It is wonderful that you've never met a human who has made that mistake; I'm sorry to ruin that perfect streak by telling you that I've made that error inadvertently myself.

The top level of Swift namespaces is modules. Below them is the global namespace, which includes all other declarations.

The module portion of a declaration in Swift is always inferred implicitly, but may be specified explicitly.

@warn_unqualified_access causes the compiler to emit a warning if a declaration is accessed without qualifying it with the parent namespace. That’s the current behavior, and this pitch is not seeking to change that. It is merely removing limitations on where it can be used.

1 Like

To the extent that your pitch is about allowing the use of @warn_unqualified_access with variables, I think that's certainly reasonable. I do think there's some design work to be done with respect to enum cases and perhaps other situations that haven't been considered.

Off the bat, I'm not certain about how it's going to work with type aliases, types, and protocols. I can see where it might be useful (earlier, when designing the SIMD types, we rejected SIMD4.Int as a spelling because, in extensions of SIMD4, that type would be called Int, which is strange). So that's perhaps an interesting point to consider, but I could go either way on that.

What you discuss regarding import isn't about "removing limitations," however, as it then extends the attribute to the consumer of the API rather than the vendor. It is true that Swift lacks a first-class concept of namespaces and submodules, but I do not think building that on a foundation of @warn_unqualified_access is the way to go.

I think it is fair to separate the import idea from everything else. I literally made that list by going through the declarations one-by-one and asking myself whether it would make sense for this to apply to them.

As for the remainder, could you give an example of an uncertainty? Especially with enumeration cases: why would this be problematic for them?

I’m not proposing that this attribute be applied to individual enumeration cases, if that wasn’t clear.

Why not? I think you should! Semantically, these are exactly like properties (when they don't have an associated value) or like functions (when they do). It would be strange to extend the attribute to both properties and functions, but not enum cases.

I assume we agree that it shouldn't apply to initializers and subscripts. Are we missing any other kinds of declarations? It would be important to work through all of them.

You have a point, though I think you are slightly off: enumeration cases with raw values are properties, and all other cases (including those without associated values) are static functions that return Self.

When approached from that angle, it doesn’t seem to be problematic. It is, however, inconsistent: enumeration cases cannot have specific modifiers (like those for access control) attached to them in most contexts. Allowing it for this should not be done without a careful understanding of why that is.

1 Like

I literally worked through all of them to make that list. I linked to each one’s entry in the Reference Guide.

Sure they can: indirect.