-
Notifications
You must be signed in to change notification settings - Fork 14.8k
Description
It was brought up here, that the initial implementation was over-zealous in doing some Sema
work in the Parser
library. The Sema
work is described below.
For context, when we parse and create RootSignatureDecl
, we want to allow ourselves to only parse identical root signatures once. To do so, when we create a new RootSignatureDecl
, we will place this implicit decl on the current scope. Then before we invoke the parser we will check to see if a corresponding decl was already created and instead just point to that decl.
So then, the Sema
work being done is:
- the storing of the
Decl
onto the current scope - the subsequent look-ups from the scope
- creation of the actual declaration
This issue tracks to move the above Sema
work to be in Sema
.
Proposed Solution:
Create two new functions ActOnStartRootSignatureDecl
and ActOnFinishRootSignatureDecl
.
The former will contain the creation of the unique DeclIdent
from the root signature string and then try to lookup if the Decl
has been previously parsed. It will return the DeclIdent
and a bool
if it was found.
The latter will contain the logic to take the parsed root elements (if parsed) and construct a RootSignatureDecl
and then put this onto the scope with PushOnScopeChains
.
The updated flow would look something like:
auto [DeclIdent, Found] = Actions.ActOnStartRootSignatureDecl(Signature);
if (!Found) {
// Invoke RootSignatureParser
...
Actions.ActOnFinishRootSignatureDecl(RootElements);
}
// Create ParsedArg
...
It is currently defined here:
llvm-project/clang/lib/Parse/ParseDeclCXX.cpp
Line 4943 in 599b2a3
// Construct our identifier |
AC:
- Any
Sema
logic currently contained inParser::ParseMicrosoftRootSignatureAttributeArgs
should be moved out and encapsulated withSema
- From the perspective of test-cases, this is expected to be a nfc
Metadata
Metadata
Assignees
Labels
Type
Projects
Status