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

Explain how to name rule identifiers #1609

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

chorman0773
Copy link
Contributor

This adds guidelines for using spec identifier syntax in the reference.

It is also intended to aid review in the PRs for adding spec identifier syntax.

docs/authoring.md Outdated Show resolved Hide resolved
docs/authoring.md Outdated Show resolved Hide resolved
docs/authoring.md Outdated Show resolved Hide resolved
docs/authoring.md Outdated Show resolved Hide resolved
Comment on lines +90 to +91
* `restriction`: Syntactic (parsing) requirements on the construct
* `constraint`: Semantic (type checking) requirements on the construct
Copy link
Contributor

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 and constraint-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".

Copy link
Contributor Author

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.

docs/authoring.md Outdated Show resolved Hide resolved
* If the rule is naming a specific Rust language construct (IE. an attribute, standard library type/function, or keyword-introduced concept), use the construct as named in the language, appropriately case-adjusted (but do not replace `_`s with `-`s)
* Other than rust language concepts with `_`s in the name, use `-` characters to separate words within a "subrule"
* Whenever possible, do not repeat previous components of the rule
* Prefer using singular forms of words over plural unless the rule applies to a list or the construct is named as plural in the language (e.g. `r[attribute.diagnostic.lint.group])
Copy link
Contributor

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".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

item.fn

* Other than rust language concepts with `_`s in the name, use `-` characters to separate words within a "subrule"
* Whenever possible, do not repeat previous components of the rule
* Prefer using singular forms of words over plural unless the rule applies to a list or the construct is named as plural in the language (e.g. `r[attribute.diagnostic.lint.group])
* Whenever possible, don't use a name that conflicts with one of the above keywords, even if this violates the first bullet.
Copy link
Contributor

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?

Copy link
Contributor Author

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).

Comment on lines +104 to +106
6. When a keyword applies, but multiple different rules in the same section would use the same keyword, prefix or suffix the rule with a descriptive id given above, separated with a `-`
* When the paragraph modifies a specific named construct or applies to a specific named construct only, prefix the rule with the name of the construct (e.g. `r[items.fn.params.self-constraint]`).
* When the paragraph refers to a specific named construct that applies the particular keyword behaviour, suffix the rule with the name of the construct
Copy link
Contributor

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.

Copy link
Contributor Author

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 to char, rather than modifying a general precondition because of the use of char.

As another example, r[items.fn.params.self-constraint], is talking about the functions that can use a self param... which probably should be a suffix here as well, thinking about it. self-constraint probably should be for talking about what types a self param could have.

Co-authored-by: Travis Cross <tc@traviscross.com>
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.

2 participants