Pub impl Type syntax

language design
1

We already have the ability to have multiple, separate impl blocks, and for large (non-trait) impls writing pub over and over is repetitive.

A pub impl (and pub(...) impl) syntax would allow an immediate, clear distinction between public and private implementation:

pub struct MyStruct;

pub impl MyStruct {
    // Public API here
    // behaves as if all functions were `pub fn`
}

impl MyStruct {
    // Private implementation here
}

This should (as far as I can tell) be perfectly backwards compatible, as one could still use the regular impl syntax and make individual functions pub.

This syntax would be symmetrical to pub trait.

A potential issue is that for sufficiently large impls, it would be unclear if a given function is actually public or not without looking at the top of the block.

However, with our documentation tools this should not be a problem. Furthermore, it should be no issue for IDEs to show the visibility again as part of a tooltip or similar facility.

4 Likes
2

The main problem is that this also would imply pub impl Trait for Type and more problematically, pub(crate) impl Trait for Type, which is a bit more problematic.

3

It doesn’t seem like a massive inconsistency to disallow pub impl Trait for Type given that you already can’t use pub fn inside an impl Trait for Type.

5 Likes
4

I don't see much value in this. It's not that hard to write pub once per public function.

I don't think it'd make distinction clearer, because locally fn foo() wouldn't say whether it's public or not, so the distance between implementation and nearest pub would be even greater.

For the record I don't like that impl Trait for T forbids pub fn. If anything, I'd prefer allowing pub in trait implementations, so that public functions without pub can be avoided.

6 Likes
5

This makes code harder to read: you can no longer determine if a method is public just by looking at it, you may also have to scroll up to top of the impl block.

We should optimize for the reader rather than the writer.

4 Likes
6

It's not as simple as not having this is simpler on the reader.

If an impl block could be declared pub, it would be easier for a reader of the file to load "all functions in this block are public" than to check each individual function for being public.

Now, if the reader is jumping straight to the function declaration rather than reading the whole block, visibility on the function is indeed easier to load.

I'd support $vis impl Type iff we also have $vis impl Trait for Type. That would bring the two closer together conceptually.

Here's a question for @LunarLambda :

What does a lower visibility function within a more visible impl do? Do we allow it? What about a higher visiblity function in a less visible impl?.

These aren't questions with obviously correct answers -- is the impl block itself what you're changing the visibility of? -- and I think getting it correct would be too tied to $vis impl Trait for Type to do this without knowing how we want to handle that (even if it's to say "it won't work"). Having $vis impl work differently in the two cases would be very problematic. The difference between the two impl blocks' visibility behavior is already unfortunate enough.

1 Like
7

I think this is a bad idea, not only because of the readability problem (which would be a huge problem in itself too), but because of the unintended and non-obvious interactions mentioned in the previous post. I don't think we should complicate the visibility system at all, it's good as-is.

5 Likes
8

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.