-
Notifications
You must be signed in to change notification settings - Fork 8
Introduce Directive Hierarchy #76
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
Introduce Directive Hierarchy #76
Conversation
to classify grammatical declarations that modify the behavior of a C++ translator
|
@microsoft-github-policy-service agree [company="Microsoft"] |
|
@microsoft-github-policy-service agree company="Microsoft" |
in favor of defined `DirSort::Using`
|
|
||
| A \type{DirIndex} value with tag \valueTag{DirSort::Using} represents an abstract reference to a \grammar{using-directive} declaration. | ||
| The \field{index} field is an index into the using-directive partition. | ||
| Each entry in that partition is a structure with the following layout |
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.
How do we semantically extract the scope which nominated the namespace?
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.
How do we semantically extract the scope which nominated the namespace?
That would be the scope whose list of declarations this directive appears
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.
Does this imply you want to remove the home_scope field from the other declaration structures? Based on the comment it sounds like all home_scope fields are redundant?
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.
Does this imply you want to remove the
home_scopefield from the other declaration structures? Based on the comment it sounds like allhome_scopefields are redundant?
No. For a semantic declaration, the home_scope is relevant - and we need to distinguish them from the lexical scope. For a directive, I am not seeing the need. Do you have an example that would need home_scope (other than other semantics decls have a home scope)?
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 looking at this from the prospective of how quickly we can read and convey information up and down the graph. If we go with what you suggest I:
- Find it wildly inconsistent with other declaration types where
home_scopeis a pretty reliable field on the other structures. Note: I'm talking specifically aboutBarrenDecl - Believe it makes reading the graph a bit harder as it is now the first case where I need to push down context from the scope I'm reading into the declaration resolver in order to figure out what the
home_scopeshould have been.
That said, I don't believe there's a reason we need to encode home_scope onto BarrenDecl, but it would be a huge convenience to authoring tooling and reduce complexity in the compiler as I would need to add more context to inner resolvers whereas before I could pass the structure X directly and have all the information I need to resolve enclosing scopes.
This is mostly a balance between ideal representation (which is what you have here) and duplicate, but useful, information.
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.
We are talking about directives here, not semantic declarations for which home_scope is an essential component. Directives always appear on the list of something, even though they don't belong to a thing.
I would like for us to proceed with the minimal representation here for the directives and come up with evidence about how inconvenient it is and add the extra info - it is harder to remove than to add.
cdacamar
left a comment
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 looking good! I appreciate the better breakdown of these semantic entities.
* Introduce Directive Hierarchy to classify grammatical declarations that modify the behavior of a C++ translator * Remove `DeclSort::UsingDirective` in favor of defined `DirSort::Using` * Add a field for the result of `DirSort::DeclUse` * Add `specifiers` and `access` to `DeclSort::Barren` * Clarify `DirExpr::Using` and `DirSort::DeclUse` * Embed directives in `StmtSort` * Update `Phases` * Introduce directive heap Update `DirSort::Tuple`
* Introduce Directive Hierarchy to classify grammatical declarations that modify the behavior of a C++ translator * Remove `DeclSort::UsingDirective` in favor of defined `DirSort::Using` * Add a field for the result of `DirSort::DeclUse` * Add `specifiers` and `access` to `DeclSort::Barren` * Clarify `DirExpr::Using` and `DirSort::DeclUse` * Embed directives in `StmtSort` * Update `Phases` * Introduce directive heap Update `DirSort::Tuple`
to classify grammatical declarations that modify the behavior of a C++ translator