[PATCH] D69903: [Basic] Introduce PODSourceLocation, NFCI

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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