[PATCH] D63640: [clang] Improve Serialization/Imporing of APValues

Thu Oct 15 02:20:15 PDT 2020

martong added a comment.

In D63640#2331779 <https://reviews.llvm.org/D63640#2331779>, @Tyker wrote:

> In D63640#2331734 <https://reviews.llvm.org/D63640#2331734>, @martong wrote:
>
>> In D63640#2331410 <https://reviews.llvm.org/D63640#2331410>, @rsmith wrote:
>>
>>> Reverse ping: I have a patch implementing class type non-type template parameters
>
> that's blocked on this landing. If you won't have time to address @martong's comments soon, do you mind if I take this over and land it?
>
> nice to see this coming.
>
>> It is okay for me to commit this patch in its current state. The changes I suggested could result in a cleaner code, but I can do those changes after we land this.
>
> i couldn't apply martong's suggestion completely because importChecked is part of ASTNodeImporter not ASTImporter, but cleaned up some code.

This indicates that the newly added `Import(APValue *)` function should be part of the `ASTNodeImporter`. I came up with a draft patch on top of your patch:
https://github.com/llvm/llvm-project/commit/ac738cf854bdafa83a23c400bd5b2a90520566f9
You can see, this way we can eliminate some redundant `if`s and `casts`.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63640/new/

https://reviews.llvm.org/D63640