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

Tyker via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 1 10:56:55 PDT 2020


Tyker added inline comments.


================
Comment at: clang/lib/AST/ASTImporter.cpp:9010
+      ToPath[Idx] =
+          cast<const CXXRecordDecl>(const_cast<Decl *>(ImpDecl.get()));
+    }
----------------
rsmith wrote:
> We want the path in an `APValue` to be canonical, but importing a canonical decl might result in a non-canonical decl.
> but importing a canonical decl might result in a non-canonical decl.
this is a quite surprising behavior.


================
Comment at: clang/lib/AST/ASTImporter.cpp:9030-9031
+        } else {
+          FromElemTy =
+              FromValue.getLValueBase().get<const ValueDecl *>()->getType();
+          llvm::Expected<const Decl *> ImpDecl =
----------------
rsmith wrote:
> If you're intentionally not handling `DynamicAllocLValue` here (because those should always be transient), a comment to that effect would be useful.
i added an asserts with a message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63640



More information about the cfe-commits mailing list