From 0e4fed1829b1afce17cde1d69f28b1147e49ca24 Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Tue, 23 Jul 2019 14:57:46 -0700 Subject: [PATCH 1/2] deepCopy also copies type information of lists Differential Revision: [D16449220](https://our.internmc.facebook.com/intern/diff/D16449220/) --- aten/src/ATen/core/List.h | 4 ++++ aten/src/ATen/core/List_inl.h | 5 +++++ .../passes/utils/check_alias_annotation.cpp | 20 ++++++++++++++----- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/aten/src/ATen/core/List.h b/aten/src/ATen/core/List.h index e2af5c7621630..c8cde840f97cf 100644 --- a/aten/src/ATen/core/List.h +++ b/aten/src/ATen/core/List.h @@ -435,6 +435,10 @@ class List final { // TODO Test use_count size_t use_count() const; + // private API for now because the return type will change to TypePtr + // instead of optional once types are mandatory. + optional _elementType() const; + private: explicit List(c10::intrusive_ptr>&& elements); friend struct IValue; diff --git a/aten/src/ATen/core/List_inl.h b/aten/src/ATen/core/List_inl.h index 62c61ff7ea1fa..9b1a66cb35f77 100644 --- a/aten/src/ATen/core/List_inl.h +++ b/aten/src/ATen/core/List_inl.h @@ -289,4 +289,9 @@ size_t List::use_count() const { return impl_.use_count(); } +template +optional List::_elementType() const { + return impl_->elementType; +} + } diff --git a/torch/csrc/jit/passes/utils/check_alias_annotation.cpp b/torch/csrc/jit/passes/utils/check_alias_annotation.cpp index e3cfc6fa19928..45c6370057ae7 100644 --- a/torch/csrc/jit/passes/utils/check_alias_annotation.cpp +++ b/torch/csrc/jit/passes/utils/check_alias_annotation.cpp @@ -25,12 +25,22 @@ IValue deepCopy(const IValue& self) { // Lists of ivalues should recursively deep copy their contents if (self.isGenericList()) { - auto newList = c10::impl::GenericList(c10::impl::deprecatedUntypedList()); - newList.reserve(self.toGenericListRef().size()); - for (const IValue& value : self.toGenericListRef()) { - newList.push_back(deepCopy(value)); + auto source = std::move(self).toGenericList(); + auto deepCopyGenericList = [&] (c10::impl::GenericList& dest) { + dest.reserve(source.size()); + for (const IValue& value : source) { + dest.push_back(deepCopy(value)); + } + }; + if (source._elementType().has_value()) { + auto newList = c10::impl::GenericList(*source._elementType()); + deepCopyGenericList(newList); + return newList; + } else { + auto newList = c10::impl::GenericList(c10::impl::deprecatedUntypedList()); + deepCopyGenericList(newList); + return newList; } - return newList; } // Regular lists can copy assign From f661681fe1d95d01db35d20d3abb4383fc2fbbb7 Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Wed, 24 Jul 2019 10:30:01 -0700 Subject: [PATCH 2/2] Update on "deepCopy also copies type information of lists" Differential Revision: [D16449220](https://our.internmc.facebook.com/intern/diff/D16449220/) --- .../passes/utils/check_alias_annotation.cpp | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/torch/csrc/jit/passes/utils/check_alias_annotation.cpp b/torch/csrc/jit/passes/utils/check_alias_annotation.cpp index 45c6370057ae7..b0a9afaa96853 100644 --- a/torch/csrc/jit/passes/utils/check_alias_annotation.cpp +++ b/torch/csrc/jit/passes/utils/check_alias_annotation.cpp @@ -26,21 +26,14 @@ IValue deepCopy(const IValue& self) { // Lists of ivalues should recursively deep copy their contents if (self.isGenericList()) { auto source = std::move(self).toGenericList(); - auto deepCopyGenericList = [&] (c10::impl::GenericList& dest) { - dest.reserve(source.size()); - for (const IValue& value : source) { - dest.push_back(deepCopy(value)); - } - }; - if (source._elementType().has_value()) { - auto newList = c10::impl::GenericList(*source._elementType()); - deepCopyGenericList(newList); - return newList; - } else { - auto newList = c10::impl::GenericList(c10::impl::deprecatedUntypedList()); - deepCopyGenericList(newList); - return newList; + auto newList = source._elementType().has_value() + ? c10::impl::GenericList(*source._elementType()) + : c10::impl::GenericList(c10::impl::deprecatedUntypedList()); + newList.reserve(source.size()); + for (const IValue& value : source) { + newList.push_back(deepCopy(value)); } + return newList; } // Regular lists can copy assign