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

RFC: abstract struct and abstract enum #10

Closed
wants to merge 2 commits into from

Conversation

glaebhoerl
Copy link
Contributor

No description provided.


##### Bikeshedding the `abstract` keyword

The word `abstract` has a different meaning in some other languages, where it's closer to our `trait`, but it's also commonly used conversationally in the same sense as in this proposal. I haven't, even with the assistance of thesaurus.com, been able to find a better word.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'opaque'?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is a word I did find but didn't think is better. Perhaps other people have different opinions?

@chris-morgan
Copy link
Member

Just a note on formatting—you don't appear to be fond of second-level headings?

Going from h1 to h3 doesn't make sense semantically.

@glaebhoerl
Copy link
Contributor Author

@chris-morgan The semantic content of document markup is not something I've been thinking about. I couldn't tell you why I did it this way because I literally didn't have a single conscious thought about it. For some reason I felt like it. If I want to rationalize, then maybe I wanted to better emphasize the separation between the levels (I also went from h3 to h5). In any case, noted for the future.

(Aside: going from h1 to h3 to h5 is perfectly sensible if you're a rook.)

@alexcrichton
Copy link
Member

As a side note, can you format the document to an 80-character width?


### Specification

Remove `pub` and `priv` as modifiers on struct fields and enum variants. Remove `priv` as a keyword. Introduce the `abstract` keyword as a modifier on `struct` and `enum` declarations.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disallowing fine-grained field privacy seems quite unfortunate. While rare, it has some crucial use-cases (such as the fmt::Formatter struct).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't an expressiveness question, just a convenience question. You can still accomplish the same thing by making a separate abstract struct with the private fields and including it as a field of the non-abstract struct. As I commented elsewhere, I'm also perfectly fine with adding back pub fields if people think it's used frequently enough to be worth having.

@glaebhoerl
Copy link
Contributor Author

OK, will do.

@huonw
Copy link
Member

huonw commented Mar 17, 2014

(If we were to do sentence-per-line, or at least, new sentences start a new line; then the diffs might be nicer as we edit as they won't include epic rewrapping due to a 2 word addition at the start of a paragraph?)

@glaebhoerl
Copy link
Contributor Author

I'm more concerned by the fact that after I pushed the updated version, @alexcrichton's earlier comments got hidden as "commented on an outdated diff". Given that the most important part of these RFC PRs is supposed to be the discussion, this seems unfortunate.

@nikomatsakis
Copy link
Contributor

On Tue, Mar 18, 2014 at 01:45:18AM -0700, Gábor Lehel wrote:

I'm more concerned by the fact that after I pushed the updated version, @alexcrichton's earlier comments got hidden as "commented on an outdated diff". Given that the most important part of these RFC PRs is supposed to be the discussion, this seems unfortunate.

I think best practice is not to force push RFCs (that is, do not use
commit --amend, do not rebase, etc). We should probably mention that
somewhere.

@pnkfelix
Copy link
Member

Filed as issue #14 against RFC 0001.

@glaebhoerl
Copy link
Contributor Author

@nikomatsakis I deliberately avoided touching the existing commit (force pushing), and instead added a new one. The issue is that even so, GitHub now hides diff comments on the previous commit by default. They are still there and can be expanded by clicking on them, but they're collapsed by default, even though their content is no less relevant than it was.

@pnkfelix
Copy link
Member

There are bookmarklets one can use to dynamically expand collapsed comments. See e.g. https://coderwall.com/p/akdgoq

(But I guess you might have to install such a bookmarklet locally on your own browser, since it seems like github's markdown renderer strips out javascript: links. Also, the bookmarklet linked there does not seem to be working out-of-the-box for me.)

@nikomatsakis
Copy link
Contributor

On Tue, Mar 18, 2014 at 04:26:04AM -0700, Gábor Lehel wrote:

@nikomatsakis I deliberately avoided touching the existing commit (force pushing), and instead added a new one. The issue is that even so, GitHub now hides diff comments on the previous commit by default. They are still there and can be expanded by clicking on them, but they're collapsed by default, even though their content is no less relevant than it was.

Oh. That is certainly something to be aware of.

@brson
Copy link
Contributor

brson commented Mar 25, 2014

This was discussed along with #1 in today's meeting: https://github.com/rust-lang/meeting-minutes/blob/master/weekly-meetings/2014-03-25.md#private-fields-by-default

The consensus was that this approach has a broader and less understood impact than simply flipping the defaults per #1; we'll continue with that approach and not this one.

Thank you.

@brson brson closed this Mar 25, 2014
withoutboats pushed a commit to withoutboats/rfcs that referenced this pull request Jan 15, 2017
more fixed typos in the documentation
wycats pushed a commit to wycats/rust-rfcs that referenced this pull request Mar 5, 2019
traviscross pushed a commit that referenced this pull request Jul 12, 2024
[RTN] Fix typo: "h" -> "hc" & "ds" -> "s"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants