dexonsmith added a comment. In D69903#2342011 <https://reviews.llvm.org/D69903#2342011>, @miyuki wrote:
> In D69903#2340020 <https://reviews.llvm.org/D69903#2340020>, @dexonsmith > wrote: > >> An alternative would be to update the unions to an `AlignedCharArrayUnion` >> and use `SourceLocation` directly. WDYT? > > So, say, in `DeclarationNameLoc`, I would add `AlignedCharArrayUnion` as > follows: > > union { > llvm::AlignedCharArrayUnion<struct NT, struct CXXOpName, struct > CXXLitOpName> UninitStorage; > struct NT NamedType; > struct CXXOpName CXXOperatorName; > struct CXXLitOpName CXXLiteralOperatorName; > }; > > And change the constructor of `DeclarationNameLoc` to default-construct > `UninitStorage`, i.e.: > > DeclarationNameLoc() : UninitStorage() { > memset(UninitStorage.buffer, 0, sizeof(UninitStorage.buffer)); > } > > After that, I can use `SourceLocation` in `DeclarationNameLoc` directly. > > Do I understand your idea correctly? Not quite, I wasn't thinking of wrapping the `AlignedCharArrayUnion` in a `union`, I was thinking of using it *as* the union. Here's one way you could do it. In the first commit, you can change: union { struct NT NamedType; struct CXXOpName CXXOperatorName; struct CXXLitOpName CXXLiteralOperatorName; }; to something like: AlignedCharArrayUnion<NT, CXXOpName, CXXLitOpName> NameLoc; This will mean updating users of `NamedType`, `CXXOperatorName`, and `CXXLiteralOperatorName` to go through the new `NameLoc` field. If there are a lot of them, you might want to start with a prep commit that adds: NT &getNamedType() { return NamedType; } CXXOpName &getCXXOperatorName() { return CXXOperatorName; } CXXLitOpName &getCXXLiteralOperatorName() { return CXXLiteralOperatorName; } and change all the users to go through the function, then when you land the change to `AlignedCharArrayUnion` you just have to update those three functions. But if there aren't many users it may not be worth it. Once you've created `NameLoc` (using an `AlignedCharArrayUnion`), a follow-up commit can change `CXXOpName` and `CXXLitOpName` to use a `SourceLocation` instead of a `PODSourceLocation`, and then you can delete `PODSourceLocation` once you've handled all of its users. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69903/new/ https://reviews.llvm.org/D69903 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
