-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Cleanup GCCAsmStmt children iterations #131358
Comments
@llvm/issue-subscribers-clang-frontend Author: cor3ntin (cor3ntin)
`GCCAsmStmt` stores its arguments in separately allocated array
llvm-project/clang/include/clang/AST/Stmt.h Lines 3305 to 3311 in b936ef1
And is not considered to have any children. Instead, we should store all these statements (contiguously) within the llvm-project/clang/include/clang/AST/Stmt.h Line 3160 in b936ef1
However, we should also make sure that the order of the children - and therefore the order of elements ie Constructors, Getter, Setters, etc of GCCAsmStmt needs to be adapted to understand this order |
Note that we should wait #131003 to be merged before working on this |
This is perhaps a 'medium first issue' as it requires significantly modifying the AST node, but is on the easier side. Any such patch should also refactor the Basically, we need to spend time evaluating the three types ( |
GCCAsmStmt
stores its arguments in separately allocated arrayllvm-project/clang/include/clang/AST/Stmt.h
Lines 3305 to 3311 in b936ef1
And is not considered to have any children.
Instead, we should store all these statements (contiguously) within the
Exprs
ofASmStmt
and make sure that's returned fromchildren()
llvm-project/clang/include/clang/AST/Stmt.h
Line 3160 in b936ef1
However, we should also make sure that the order of the children - and therefore the order of elements
in Expr makes sense,
ie
Template, OutputConstraint0, OutputExpr0, ... OutputConstraintN, OutputExprN, InputConstraint0, InputExpr0, ... InputConstraintN, InputExprN, Label0, ...
Constructors, Getter, Setters, etc of GCCAsmStmt needs to be adapted to understand this order
The text was updated successfully, but these errors were encountered: