rsmith added inline comments.
================
Comment at: clang/include/clang/AST/Designator.h:87-92
+ /// Refers to the index expression.
+ Expr *IndexExpr;
+
+ /// Location of the first index expression within the designated
+ /// initializer expression's list of subexpressions.
+ unsigned Index;
----------------
This looks very odd -- these two index fields represent completely different
kinds of information, one of which is specific to an implementation detail of
`InitListExpr` / `DesignatedInitExpr`. Perhaps this class could be templated
over its representation of an expression, instead of having both cases be
possibilities here, and having the user of this class keep track of whether
they're using the kind of designator that stores an expression pointer or the
kind that stores an expression index? (We'd end up storing two different
indexes for array range designators this way, but that seems like it should not
be problematic.)
================
Comment at: clang/include/clang/AST/Designator.h:98
+ /// The location of the ']' terminating the array designator.
+ mutable SourceLocation RBracketLoc;
+
----------------
Does this really need to be mutable? The AST version shouldn't be exposing a
way to mutate this field! It seems like we should have a non-const overload of
`Designation::getDesignator` for `Sema` to use if it really needs to modify
this field.
================
Comment at: clang/include/clang/AST/Designator.h:313-319
+ /// ClearExprs - Null out any expression references, which prevents
+ /// them from being 'delete'd later.
+ void ClearExprs(Sema &Actions) {}
+
+ /// FreeExprs - Release any unclaimed memory for the expressions in
+ /// this designator.
+ void FreeExprs(Sema &Actions) {}
----------------
AST classes shouldn't reference `Sema`. But these functions do nothing; can
they be removed?
================
Comment at: clang/include/clang/AST/Designator.h:340-346
+ /// ClearExprs - Null out any expression references, which prevents them from
+ /// being 'delete'd later.
+ void ClearExprs(Sema &Actions) {}
+
+ /// FreeExprs - Release any unclaimed memory for the expressions in this
+ /// designation.
+ void FreeExprs(Sema &Actions) {}
----------------
AST classes shouldn't reference `Sema`. But these functions do nothing; can
they be removed?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140584/new/
https://reviews.llvm.org/D140584
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
