Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Explain how to name rule identifiers #1609
Explain how to name rule identifiers #1609
Changes from 1 commit
c17f60c
06c1d98
8912726
d628425
d032fa8
d78728d
3c7523f
0ab2181
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ehuss and I talked about this part. We're a but uncertain about the strategy here. It feels like there are going to be a ton of
restriction-this
andconstraint-that
rules, and we're not sure that it carries its weight.We're also probably unconvinced on this means of separating syntactic and type checking requirements. I.e., it doesn't immediately speak to us that "restriction" would refer to parsing and "constraint" would refer to type checking and that these should be separated in this manner.
This probably also applies to some of the others, such as "preconditions".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't seen ambiguous keyword rules too often in my work, though that might change once I start my second pass.
As for the specific terms, I'd like to discuss that tomorrow at the T-spec meeting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is true everywhere. Plural often makes sense. I'd expect chapter titles like "Items" (rather than "Item") or "Lints" (rather than "Lint"), and I'd expect the rule names to follow suit here.
Often, what's going to come up, as it did with the diagnostic namespace, is not the difference between plural and singular, but the difference between nouns and adjectives. As a noun, it makes sense for the chapter title (and rules that follow that) to be "Diagnostics", but when used with "diagnostic namespace", it's an adjective, and so it doesn't have the "s".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
item.fn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we think of any examples of where this would come up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AssertUnwindSafe
is probably close (assert-unwind-safe
if we ever refer to it in the reference). I can't think of any specific examples, but that doesn't mean it won't happen (especially if we expand the list of keywords).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In discussion we were a bit unclear about the distinction being drawn here. Perhaps more examples would help.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually did this a couple times working on #1618.
r[type.trait-object.syntax-edition2021]
- it refers to Edition 2021 and modifies the syntax rule accordingly, so we use a suffix here.r[type.text.char-precondition]
is a precondition that applies tochar
, rather than modifying a general precondition because of the use ofchar
.As another example,
r[items.fn.params.self-constraint]
, is talking about the functions that can use aself
param... which probably should be a suffix here as well, thinking about it.self-constraint
probably should be for talking about what types aself
param could have.