-
Notifications
You must be signed in to change notification settings - Fork 39
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
[0002] Specify the grammar formulations for attributes #65
base: main
Are you sure you want to change the base?
[0002] Specify the grammar formulations for attributes #65
Conversation
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.
Looks good. I have some minor editing notes that you can take or leave as you please.
all shared grammar elements between HLSL and C++. This spec attempts to detail | ||
some of the grammar and parsing implications and will specify the process by | ||
which existing attributes will convert to C++ attributes. | ||
### Attribute Parsing |
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.
Best to add a blank line before the section header for the benefit of those reading the markdown directly (I don't think it makes a difference in the rendered doc)
proposals/0002-cxx-attributes.md
Outdated
which existing attributes will convert to C++ attributes. | ||
### Attribute Parsing | ||
|
||
This proposal introduces new grammar formulations for parsing attributes |
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'd probably drop the word "new" - "introduces grammar formulations" is clear
proposals/0002-cxx-attributes.md
Outdated
``` | ||
|
||
In contrast to existing HLSL annotations and Microsoft-style attributes, these | ||
attribute identifiers are case-sensitive `identifier` tokens. |
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.
This is probably clear enough from context and I'm just being pedantic, but "attribute identifiers are case-sensitive identifier
tokens" isn't strictly true (there's also the attribute-scoped-token
formulation).
Maybe:
In contrast to existing HLSL annotations and Microsoft-style attributes, these formulations use case-sensitive
identifier
tokens
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.
Sorry for all the markups, if I'm understanding the grammar's grammar correctly I think it has some details wrong
proposals/0002-cxx-attributes.md
Outdated
```ebnf | ||
attribute-specifier-seq: [ attribute-specifier-seq ], attribute-specifier | ||
attribute-specifier: "[" , "[" , attribute-list , "]" , "]" | ||
attribute-list: [ attribute ] || |
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.
If one of the options for attribute-list
is an optional attribute
that means [[]]
will parse as attribute-specifier(attribute-list(optional-attribute = none))
is that an intended spelling?
This does not exhaustively express all the places C++ attributes are valid, but it should cover enough detail to be useful. Fixes microsoft#63.
This updates the grammar specification to align with how C++ specifies parsing grammar, and the language spec latex is included. I've also added images containing the rendered grammar for ease of reading.
8b194ea
to
059b06f
Compare
\textit{simple-template-id}\br | ||
|
||
\define{class-specifier}\br | ||
\textit{class-head} \terminal{\}} \opt{member-specification} \terminal{\}}\br |
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.
typo?
\textit{class-head} \terminal{\}} \opt{member-specification} \terminal{\}}\br | |
\textit{class-head} \terminal{\{} \opt{member-specification} \terminal{\}}\br | |
This does not exhaustively express all the places C++ attributes are valid, but it should cover enough detail to be useful.
Fixes #63.