usd: Simplify CrateFile::_ValueHandler type hierarchy using C++17's 'if
authorgitamohr <gitamohr@users.noreply.github.com>
Sun, 10 Dec 2023 18:53:01 +0000 (10:53 -0800)
committerpixar-oss <pixar-oss@users.noreply.github.com>
Sun, 10 Dec 2023 18:53:01 +0000 (10:53 -0800)
constexpr()'.  This type hierarchy existed to let us simultaneously share
common code among all the kinds of value types (inlined-vs-not,
array-supporting-vs-not) while not instantiating code that wouldn't be valid
for one type or another.

Happily, 'if constexpr()' will totally discard the appropriate 'if' or 'else'
branch, giving us a cleaner way to achieve these goals.  This change collapses
the old type hierarchy:

_ValueHandlerBase <- _ScalarValueHandlerBase<T> <-
    _ArrayValueHandlerBase<T> <- _ValueHandler<T>

To simply:

_ValueHandlerBase <- _ValueHandler<T>

And the type-based special cases are handled locally within the methods of
_ValueHandler<T> via 'if constexpr()' rather than spread across the different
subclasses.

(Internal change: 2308134)

pxr/usd/usd/crateFile.cpp
pxr/usd/usd/crateFile.h

index ffc89be1a0923e1d3bed51f7b9e80aec4a594c5f..70b3996adacf85f1cc7dd9e0edc7c90c683dc0ad 100644 (file)
@@ -245,50 +245,40 @@ constexpr _SectionName _KnownSections[] = {
     _FieldSetsSectionName, _PathsSectionName, _SpecsSectionName
 };
 
-constexpr bool _IsInlinedImpl(string *) { return true; }
-constexpr bool _IsInlinedImpl(TfToken *) { return true; }
-constexpr bool _IsInlinedImpl(SdfPath *) { return true; }
-constexpr bool _IsInlinedImpl(SdfAssetPath *) { return true; }
 template <class T>
-constexpr bool _IsInlinedImpl(T *) {
-    return sizeof(T) <= sizeof(uint32_t) && _IsBitwiseReadWrite<T>::value;
-}
-template <class T>
-constexpr bool _IsInlinedType() {
-    return _IsInlinedImpl(static_cast<T *>(nullptr));
-}
+struct _IsAlwaysInlined : std::integral_constant<
+    bool, sizeof(T) <= sizeof(uint32_t) && _IsBitwiseReadWrite<T>::value> {};
+
+template <> struct _IsAlwaysInlined<string> : std::true_type {};
+template <> struct _IsAlwaysInlined<TfToken> : std::true_type {};
+template <> struct _IsAlwaysInlined<SdfPath> : std::true_type {};
+template <> struct _IsAlwaysInlined<SdfAssetPath> : std::true_type {};
 
-#define xx(ENUMNAME, _unused1, CPPTYPE, _unused2)               \
-    constexpr TypeEnum _TypeEnumForImpl(CPPTYPE *) {            \
-        return TypeEnum::ENUMNAME;                              \
-    }
-#include "crateDataTypes.h"
-#undef xx
 template <class T>
-constexpr Usd_CrateFile::TypeEnum TypeEnumFor() {
-    return _TypeEnumForImpl(static_cast<T *>(nullptr));
-}
+struct _TypeEnumFor {};
 
-template <class T> struct ValueTypeTraits {};
-// Generate value type traits providing enum value, array support, and whether
-// or not the value may be inlined.
-#define xx(ENUMNAME, _unused, CPPTYPE, SUPPORTSARRAY)                          \
-    template <> struct ValueTypeTraits<CPPTYPE> {                              \
-        static constexpr bool supportsArray = SUPPORTSARRAY;                   \
-        static constexpr bool isInlined = _IsInlinedType<CPPTYPE>();           \
-    };
+template <class T>
+struct _SupportsArray {};
+
+#define xx(ENUMNAME, _unused1, CPPTYPE, SUPPORTSARRAY)                         \
+template <> struct _TypeEnumFor<CPPTYPE> {                                     \
+    static const TypeEnum value = TypeEnum::ENUMNAME;                          \
+};                                                                             \
+template <> struct _SupportsArray<CPPTYPE> {                                   \
+    static constexpr bool value = SUPPORTSARRAY;                               \
+};
 #include "crateDataTypes.h"
 #undef xx
 
 template <class T>
 static constexpr ValueRep ValueRepFor(uint64_t payload = 0) {
-    return ValueRep(TypeEnumFor<T>(),
-                    ValueTypeTraits<T>::isInlined, /*isArray=*/false, payload);
+    return ValueRep(_TypeEnumFor<T>::value,
+                    _IsAlwaysInlined<T>::value, /*isArray=*/false, payload);
 }
 
 template <class T>
 static constexpr ValueRep ValueRepForArray(uint64_t payload = 0) {
-    return ValueRep(TypeEnumFor<T>(),
+    return ValueRep(_TypeEnumFor<T>::value,
                     /*isInlined=*/false, /*isArray=*/true, payload);
 }
 
@@ -1055,55 +1045,10 @@ struct CrateFile::_PackingContext
 };
 
 /////////////////////////////////////////////////////////////////////////
-// Readers
-class CrateFile::_ReaderBase
-{
-public:
-    _ReaderBase(CrateFile const *crate) : crate(crate) {}
-
-    template <class T>
-    T GetUninlinedValue(uint32_t x, T *) const {
-        static_assert(sizeof(T) <= sizeof(x), "");
-        T r;
-        char const *srcBytes = reinterpret_cast<char const *>(&x);
-        char *dstBytes = reinterpret_cast<char *>(&r);
-        memcpy(dstBytes, srcBytes, sizeof(r));
-        return r;
-    }
-
-    string const & GetUninlinedValue(uint32_t i, string *) const {
-        return crate->GetString(StringIndex(i));
-    }
-
-    TfToken const &GetUninlinedValue(uint32_t i, TfToken *) const {
-        return crate->GetToken(TokenIndex(i));
-    }
-
-    SdfPath const &GetUninlinedValue(uint32_t i, SdfPath *) const {
-        return crate->GetPath(PathIndex(i));
-    }
-
-    SdfAssetPath GetUninlinedValue(uint32_t i, SdfAssetPath *) const {
-        return SdfAssetPath(crate->GetToken(TokenIndex(i)));
-    }
-
-    SdfVariability GetUninlinedValue(uint32_t i, SdfVariability *) const {
-        // Explicitly convert legacy SdfVariabilityConfig value to
-        // SdfVariabilityUniform. This "config" variability was never used
-        // in USD but clients may have written this value out so we need
-        // to handle it to maintain backwards compatibility.
-        static const uint32_t LEGACY_CONFIG_VARIABILITY = 2;
-        if (i == LEGACY_CONFIG_VARIABILITY) {
-            return SdfVariabilityUniform;
-        }
-        return static_cast<SdfVariability>(i);
-    }
-
-    CrateFile const *crate;
-};
+// Reader
 
 template <class ByteStream>
-class CrateFile::_Reader : public _ReaderBase
+class CrateFile::_Reader
 {
     void _RecursiveRead() {
         auto start = src.Tell();
@@ -1118,25 +1063,78 @@ class CrateFile::_Reader : public _ReaderBase
         src.Seek(start + offset);
     }
 
+    // Read implementations that need partial specialization.
+    template <class T>
+    SdfListOp<T> _Read(SdfListOp<T> *) {
+        SdfListOp<T> listOp;
+        auto h = Read<_ListOpHeader>();
+        if (h.IsExplicit()) { listOp.ClearAndMakeExplicit(); }
+        if (h.HasExplicitItems()) {
+            listOp.SetExplicitItems(Read<vector<T>>()); }
+        if (h.HasAddedItems()) { listOp.SetAddedItems(Read<vector<T>>()); }
+        if (h.HasPrependedItems()) {
+            listOp.SetPrependedItems(Read<vector<T>>()); }
+        if (h.HasAppendedItems()) {
+            listOp.SetAppendedItems(Read<vector<T>>()); }
+        if (h.HasDeletedItems()) { listOp.SetDeletedItems(Read<vector<T>>()); }
+        if (h.HasOrderedItems()) { listOp.SetOrderedItems(Read<vector<T>>()); }
+        return listOp;
+    }
+    
+    template <class T>
+    vector<T> _Read(vector<T> *) {
+        auto sz = Read<uint64_t>();
+        vector<T> vec(sz);
+        ReadContiguous(vec.data(), sz);
+        return vec;
+    }
+
 public:
     static constexpr bool StreamSupportsZeroCopy = ByteStream::SupportsZeroCopy;
     
     _Reader(CrateFile const *crate, ByteStream &src)
-        : _ReaderBase(crate)
+        : crate(crate)
         , src(src) {}
 
-    template <class T>
-    static typename std::enable_if<_IsBitwiseReadWrite<T>::value, T>::type
-    StaticRead(ByteStream &src, T *) {
-        T bits;
-        src.Read(&bits, sizeof(bits));
-        return bits;
-    }
-
     void Prefetch(int64_t offset, int64_t size) { src.Prefetch(offset, size); }
 
     void Seek(uint64_t offset) { src.Seek(offset); }
 
+    template <class T>
+    T GetUninlinedValue(uint32_t x) const {
+        if constexpr (std::is_same_v<T, string>) {
+            return crate->GetString(StringIndex(x));
+        }
+        else if constexpr (std::is_same_v<T, TfToken>) {
+            return crate->GetToken(TokenIndex(x));
+        }
+        else if constexpr (std::is_same_v<T, SdfPath>) {
+            return crate->GetPath(PathIndex(x));
+        }
+        else if constexpr (std::is_same_v<T, SdfAssetPath>) {
+            return SdfAssetPath(crate->GetToken(TokenIndex(x)));
+        }
+        else if constexpr (std::is_same_v<T, SdfVariability>) {
+            // Explicitly convert legacy SdfVariabilityConfig value to
+            // SdfVariabilityUniform. This "config" variability was never used
+            // in USD but clients may have written this value out so we need to
+            // handle it to maintain backwards compatibility.
+            static const uint32_t LEGACY_CONFIG_VARIABILITY = 2;
+            if (x == LEGACY_CONFIG_VARIABILITY) {
+                return SdfVariabilityUniform;
+            }
+            return static_cast<SdfVariability>(x);
+        }
+        else {
+            static_assert(sizeof(T) <= sizeof(x), "");
+            T r;
+            char const *srcBytes = reinterpret_cast<char const *>(&x);
+            char *dstBytes = reinterpret_cast<char *>(&r);
+            memcpy(dstBytes, srcBytes, sizeof(r));
+            return r;
+        }
+    }
+
     // Map helper.
     template <class Map>
     Map ReadMap() {
@@ -1153,193 +1151,177 @@ public:
     }
 
     ////////////////////////////////////////////////////////////////////////
-    // Base template for Read.  It dispatches to the overloads that take a
-    // dummy pointer argument to allow overloading/enable_if.
+    // Main entry point Read() to read an object from the stream.
     template <class T>
     inline T Read() {
-        return this->Read(static_cast<T *>(nullptr));
-    }
-
-    // read bitwise.
-    template <class T>
-    typename std::enable_if<_IsBitwiseReadWrite<T>::value, T>::type
-    Read(T *) { return StaticRead(src, static_cast<T *>(nullptr)); }
-
-    _TableOfContents Read(_TableOfContents *) {
-        _TableOfContents ret;
-        ret.sections = Read<decltype(ret.sections)>();
-        return ret;
-    }
-    string Read(string *) { return crate->GetString(Read<StringIndex>()); }
-    TfToken Read(TfToken *) { return crate->GetToken(Read<TokenIndex>()); }
-    SdfPath Read(SdfPath *) { return crate->GetPath(Read<PathIndex>()); }
-    VtDictionary Read(VtDictionary *) { return ReadMap<VtDictionary>(); }
-    SdfAssetPath Read(SdfAssetPath *) {
-        return SdfAssetPath(Read<string>());
-    }
-    SdfTimeCode Read(SdfTimeCode *) { return SdfTimeCode(Read<double>()); }
-    SdfPathExpression Read(SdfPathExpression *) {
-        return SdfPathExpression(Read<std::string>());
-    }
-    SdfUnregisteredValue Read(SdfUnregisteredValue *) {
-        VtValue val = Read<VtValue>();
-        if (val.IsHolding<string>())
-            return SdfUnregisteredValue(val.UncheckedGet<string>());
-        if (val.IsHolding<VtDictionary>())
-            return SdfUnregisteredValue(val.UncheckedGet<VtDictionary>());
-        if (val.IsHolding<SdfUnregisteredValueListOp>())
-            return SdfUnregisteredValue(
-                val.UncheckedGet<SdfUnregisteredValueListOp>());
-        TF_CODING_ERROR("SdfUnregisteredValue in crate file contains invalid "
-                        "type '%s' = '%s'; expected string, VtDictionary or "
-                        "SdfUnregisteredValueListOp; returning empty",
-                        val.GetTypeName().c_str(), TfStringify(val).c_str());
-        return SdfUnregisteredValue();
-    }
-    SdfVariantSelectionMap Read(SdfVariantSelectionMap *) {
-        return ReadMap<SdfVariantSelectionMap>();
-    }
-    SdfLayerOffset Read(SdfLayerOffset *) {
-        // Do not combine the following into one statement.  It must be separate
-        // because the two modifications to 'src' must be correctly sequenced.
-        auto offset = Read<double>();
-        auto scale = Read<double>();
-        return SdfLayerOffset(offset, scale);
-    }
-    SdfReference Read(SdfReference *) {
-        // Do not combine the following into one statement.  It must be separate
-        // because the two modifications to 'src' must be correctly sequenced.
-        auto assetPath = Read<std::string>();
-        auto primPath = Read<SdfPath>();
-        auto layerOffset = Read<SdfLayerOffset>();
-        auto customData = Read<VtDictionary>();
-        return SdfReference(std::move(assetPath), std::move(primPath),
-                            std::move(layerOffset), std::move(customData));
-    }
-    SdfPayload Read(SdfPayload *) {
-        // Do not combine the following into one statement.  It must be separate
-        // because the two modifications to 'src' must be correctly sequenced.
-        auto assetPath = Read<string>();
-        auto primPath = Read<SdfPath>();
-
-        // Layer offsets were added to SdfPayload starting in 0.8.0. Files 
-        // before that cannot have them.
-        const bool canReadLayerOffset = 
-            (Version(crate->_boot) >= Version(0, 8, 0));
-        if (canReadLayerOffset) {
-            auto layerOffset = Read<SdfLayerOffset>();
-            return SdfPayload(assetPath, primPath, layerOffset);
-        } else {
-            return SdfPayload(assetPath, primPath);
+        if constexpr (_IsBitwiseReadWrite<T>::value) {
+            T bits;
+            src.Read(&bits, sizeof(bits));
+            return bits;
         }
-    }
-    template <class T>
-    SdfListOp<T> Read(SdfListOp<T> *) {
-        SdfListOp<T> listOp;
-        auto h = Read<_ListOpHeader>();
-        if (h.IsExplicit()) { listOp.ClearAndMakeExplicit(); }
-        if (h.HasExplicitItems()) {
-            listOp.SetExplicitItems(Read<vector<T>>()); }
-        if (h.HasAddedItems()) { listOp.SetAddedItems(Read<vector<T>>()); }
-        if (h.HasPrependedItems()) {
-            listOp.SetPrependedItems(Read<vector<T>>()); }
-        if (h.HasAppendedItems()) {
-            listOp.SetAppendedItems(Read<vector<T>>()); }
-        if (h.HasDeletedItems()) { listOp.SetDeletedItems(Read<vector<T>>()); }
-        if (h.HasOrderedItems()) { listOp.SetOrderedItems(Read<vector<T>>()); }
-        return listOp;
-    }
-    VtValue Read(VtValue *) {
-        _RecursiveReadAndPrefetch();
-        auto rep = Read<ValueRep>();
-        // Guard against recursion here -- a bad file can cause infinite
-        // recursion via VtValues that claim to contain themselves.
-        auto &recursionGuard = _LocalUnpackRecursionGuard::Get();
-        VtValue result;
-        if (!recursionGuard.insert(rep).second) {
-            TF_RUNTIME_ERROR("Corrupt asset <%s>: a VtValue claims to "
-                             "recursively contain itself -- returning "
-                             "an empty VtValue instead",
-                             crate->GetAssetPath().c_str());
+        else if constexpr (std::is_same_v<T, _TableOfContents>) {
+            _TableOfContents ret;
+            ret.sections = Read<decltype(ret.sections)>();
+            return ret;
         }
-        else {
-            result = crate->UnpackValue(rep);
+        else if constexpr (std::is_same_v<T, string>) {
+            return crate->GetString(Read<StringIndex>());
         }
-        recursionGuard.erase(rep);
-        return result;
-    }
-
-    TimeSamples Read(TimeSamples *) {
-
-        TimeSamples ret;
-
-        // Reconstitute a rep for this very location in the file to be retained
-        // in the TimeSamples result.
-        ret.valueRep = ValueRepFor<TimeSamples>(src.Tell());
-
-        _RecursiveRead();
-        auto timesRep = Read<ValueRep>();
-
-        // Deduplicate times in-memory by ValueRep.
-        // Optimistically take the read lock and see if we already have times.
-        tbb::spin_rw_mutex::scoped_lock
-            lock(crate->_sharedTimesMutex, /*write=*/false);
-        auto sharedTimesIter = crate->_sharedTimes.find(timesRep);
-        if (sharedTimesIter != crate->_sharedTimes.end()) {
-            // Yes, reuse existing times.
-            ret.times = sharedTimesIter->second;
-        } else {
-            // The lock upgrade here may or may not be atomic.  This means
-            // someone else may have populated the table while we were
-            // upgrading.
-            lock.upgrade_to_writer();
-            auto iresult =
-                crate->_sharedTimes.emplace(timesRep, Usd_EmptySharedTag);
-            if (iresult.second) {
-                // We get to do the population.
-                auto sharedTimes = TimeSamples::SharedTimes();
-                crate->_UnpackValue(timesRep, &sharedTimes.GetMutable());
-                iresult.first->second.swap(sharedTimes);
+        else if constexpr (std::is_same_v<T, TfToken>) {
+            return crate->GetToken(Read<TokenIndex>());
+        }
+        else if constexpr (std::is_same_v<T, SdfPath>) {
+            return crate->GetPath(Read<PathIndex>());
+        }
+        else if constexpr (std::is_same_v<T, VtDictionary> ||
+                           std::is_same_v<T, SdfVariantSelectionMap>) {
+            return ReadMap<T>();
+        }
+        else if constexpr (std::is_same_v<T, SdfAssetPath> ||
+                           std::is_same_v<T, SdfPathExpression>) {
+            return T(Read<string>());
+        }
+        else if constexpr (std::is_same_v<T, SdfTimeCode>) {
+            return SdfTimeCode(Read<double>());
+        }
+        else if constexpr (std::is_same_v<T, SdfUnregisteredValue>) {
+            VtValue val = Read<VtValue>();
+            if (val.IsHolding<string>())
+                return SdfUnregisteredValue(val.UncheckedGet<string>());
+            if (val.IsHolding<VtDictionary>())
+                return SdfUnregisteredValue(val.UncheckedGet<VtDictionary>());
+            if (val.IsHolding<SdfUnregisteredValueListOp>())
+                return SdfUnregisteredValue(
+                    val.UncheckedGet<SdfUnregisteredValueListOp>());
+            TF_CODING_ERROR("SdfUnregisteredValue in crate file contains "
+                            "invalid type '%s' = '%s'; expected string, "
+                            "VtDictionary or SdfUnregisteredValueListOp; "
+                            "returning empty", val.GetTypeName().c_str(),
+                            TfStringify(val).c_str());
+            return SdfUnregisteredValue();
+        }
+        else if constexpr (std::is_same_v<T, SdfLayerOffset>) {
+            // Do not combine the following into one statement.  They must be
+            // separate because the two modifications to the 'src' member must
+            // be correctly sequenced.
+            auto offset = Read<double>();
+            auto scale = Read<double>();
+            return SdfLayerOffset(offset, scale);
+        }
+        else if constexpr (std::is_same_v<T, SdfReference>) {
+            // Do not combine the following into one statement.  They must be
+            // separate because the two modifications to the 'src' member must
+            // be correctly sequenced.
+            auto assetPath = Read<string>();
+            auto primPath = Read<SdfPath>();
+            auto layerOffset = Read<SdfLayerOffset>();
+            auto customData = Read<VtDictionary>();
+            return SdfReference(std::move(assetPath), std::move(primPath),
+                                std::move(layerOffset), std::move(customData));
+        }
+        else if constexpr (std::is_same_v<T, SdfPayload>) {
+            // Do not combine the following into one statement.  They must be
+            // separate because the two modifications to the 'src' member must
+            // be correctly sequenced.
+            auto assetPath = Read<string>();
+            auto primPath = Read<SdfPath>();
+            
+            // Layer offsets were added to SdfPayload starting in 0.8.0. Files 
+            // before that cannot have them.
+            const bool canReadLayerOffset = 
+                (Version(crate->_boot) >= Version(0, 8, 0));
+            if (canReadLayerOffset) {
+                auto layerOffset = Read<SdfLayerOffset>();
+                return SdfPayload(assetPath, primPath, layerOffset);
+            } else {
+                return SdfPayload(assetPath, primPath);
             }
-            ret.times = iresult.first->second;
         }
-        lock.release();
-
-        _RecursiveRead();
-
-        // Store the offset to the value reps in the file.  The values are
-        // encoded as a uint64_t size followed by contiguous reps.  So we jump
-        // over that uint64_t and store the start of the reps.  Then we seek
-        // forward past the reps to continue.
-        auto numValues = Read<uint64_t>();
-        ret.valuesFileOffset = src.Tell();
-
-        // Now move past the reps to continue.
-        src.Seek(ret.valuesFileOffset + numValues * sizeof(ValueRep));
-
-        return ret;
-    }
-
-    template <class T>
-    vector<T> Read(vector<T> *) {
-        auto sz = Read<uint64_t>();
-        vector<T> vec(sz);
-        ReadContiguous(vec.data(), sz);
-        return vec;
-    }
+        else if constexpr (std::is_same_v<T, VtValue>) {
+            _RecursiveReadAndPrefetch();
+            auto rep = Read<ValueRep>();
+            // Guard against recursion here -- a bad file can cause infinite
+            // recursion via VtValues that claim to contain themselves.
+            auto &recursionGuard = _LocalUnpackRecursionGuard::Get();
+            VtValue result;
+            if (!recursionGuard.insert(rep).second) {
+                TF_RUNTIME_ERROR("Corrupt asset <%s>: a VtValue claims to "
+                                 "recursively contain itself -- returning "
+                                 "an empty VtValue instead",
+                                 crate->GetAssetPath().c_str());
+            }
+            else {
+                result = crate->UnpackValue(rep);
+            }
+            recursionGuard.erase(rep);
+            return result;
+        }
+        else if constexpr (std::is_same_v<T, TimeSamples>) {
+            TimeSamples ret;
+            
+            // Reconstitute a rep for this very location in the file to be
+            // retained in the TimeSamples result.
+            ret.valueRep = ValueRepFor<TimeSamples>(src.Tell());
+            
+            _RecursiveRead();
+            auto timesRep = Read<ValueRep>();
+            
+            // Deduplicate times in-memory by ValueRep.  Optimistically take the
+            // read lock and see if we already have times.
+            tbb::spin_rw_mutex::scoped_lock
+                lock(crate->_sharedTimesMutex, /*write=*/false);
+            auto sharedTimesIter = crate->_sharedTimes.find(timesRep);
+            if (sharedTimesIter != crate->_sharedTimes.end()) {
+                // Yes, reuse existing times.
+                ret.times = sharedTimesIter->second;
+            } else {
+                // The lock upgrade here may or may not be atomic.  This means
+                // someone else may have populated the table while we were
+                // upgrading.
+                lock.upgrade_to_writer();
+                auto iresult =
+                    crate->_sharedTimes.emplace(timesRep, Usd_EmptySharedTag);
+                if (iresult.second) {
+                    // We get to do the population.
+                    auto sharedTimes = TimeSamples::SharedTimes();
+                    crate->_UnpackValue(timesRep, &sharedTimes.GetMutable());
+                    iresult.first->second.swap(sharedTimes);
+                }
+                ret.times = iresult.first->second;
+            }
+            lock.release();
+            
+            _RecursiveRead();
+            
+            // Store the offset to the value reps in the file.  The values are
+            // encoded as a uint64_t size followed by contiguous reps.  So we
+            // jump over that uint64_t and store the start of the reps.  Then we
+            // seek forward past the reps to continue.
+            auto numValues = Read<uint64_t>();
+            ret.valuesFileOffset = src.Tell();
+            
+            // Now move past the reps to continue.
+            src.Seek(ret.valuesFileOffset + numValues * sizeof(ValueRep));
 
-    template <class T>
-    typename std::enable_if<_IsBitwiseReadWrite<T>::value>::type
-    ReadContiguous(T *values, size_t sz) {
-        src.Read(static_cast<void *>(values), sz * sizeof(*values));
+            return ret;
+        }
+        else {
+            // Otherwise read partial-specialized stuff.
+            return this->_Read(static_cast<T *>(nullptr));
+        }
     }
 
     template <class T>
-    typename std::enable_if<!_IsBitwiseReadWrite<T>::value>::type
-    ReadContiguous(T *values, size_t sz) {
-        std::for_each(values, values + sz, [this](T &v) { v = Read<T>(); });
+    void ReadContiguous(T *values, size_t sz) {
+        if constexpr (_IsBitwiseReadWrite<T>::value) {
+            src.Read(static_cast<void *>(values), sz * sizeof(*values));
+        }
+        else {
+            std::for_each(values, values + sz, [this](T &v) { v = Read<T>(); });
+        }
     }
-
+    
+    CrateFile const *crate;
     ByteStream src;
 };
 
@@ -1540,15 +1522,14 @@ public:
     }
 
     template <class T>
-    typename std::enable_if<_IsBitwiseReadWrite<T>::value>::type
-    WriteContiguous(T const *values, size_t sz) {
-        sink->Write(values, sizeof(*values) * sz);
-    }
-
-    template <class T>
-    typename std::enable_if<!_IsBitwiseReadWrite<T>::value>::type
-    WriteContiguous(T const *values, size_t sz) {
-        std::for_each(values, values + sz, [this](T const &v) { Write(v); });
+    void WriteContiguous(T const *values, size_t sz) {
+        if constexpr (_IsBitwiseReadWrite<T>::value) {
+            sink->Write(values, sizeof(*values) * sz);
+        }
+        else {
+            std::for_each(values, values + sz,
+                          [this](T const &v) { Write(v); });
+        }            
     }
 
     CrateFile *crate;
@@ -1557,95 +1538,152 @@ public:
 
 
 ////////////////////////////////////////////////////////////////////////
-// ValueHandler class hierarchy.  See comment for _ValueHandler itself for more
-// information.
+// ValueHandler class template -- supports top-level value pack/unpack.
 
-struct CrateFile::_ValueHandlerBase {
-    // Base Clear() does nothing.
-    void Clear() {}
-};
-
-// Scalar handler for non-inlined types -- does deduplication.
-template <class T, class Enable>
-struct CrateFile::_ScalarValueHandlerBase : _ValueHandlerBase
+template <class T>
+struct CrateFile::_ValueHandler : _ValueHandlerBase
 {
     inline ValueRep Pack(_Writer writer, T const &val) {
-        // See if we can inline the value -- we might be able to if there's some
-        // encoding that can exactly represent it in 4 bytes.
-        uint32_t ival = 0;
-        if (_EncodeInline(val, &ival)) {
-            auto ret = ValueRepFor<T>(ival);
-            ret.SetIsInlined();
-            return ret;
+        if constexpr (_IsAlwaysInlined<T>::value) {
+            // Inline it into the rep.
+            return ValueRepFor<T>(writer.GetInlinedValue(val));
+        }
+        else {
+            // The type is not always inlined, but some values of the type might
+            // be if they can be encoded in 4 bytes.
+            uint32_t ival = 0;
+            if (_EncodeInline(val, &ival)) {
+                auto ret = ValueRepFor<T>(ival);
+                ret.SetIsInlined();
+                return ret;
+            }
+            
+            // Otherwise dedup and/or write...
+            if (!_valueDedup) {
+                _valueDedup.reset(
+                    new typename decltype(_valueDedup)::element_type);
+            }
+            
+            auto iresult = _valueDedup->emplace(val, ValueRep());
+            ValueRep &target = iresult.first->second;
+            if (iresult.second) {
+                // Not yet present.  Invoke the write function.
+                target = ValueRepFor<T>(writer.Tell());
+                writer.Write(val);
+            }
+            return target;
         }
+    }
+    
+    template <class Reader>
+    inline void Unpack(Reader reader, ValueRep rep, T *out) const {
 
-        // Otherwise dedup and/or write...
-        if (!_valueDedup) {
-            _valueDedup.reset(
-                new typename decltype(_valueDedup)::element_type);
+        if constexpr (_IsAlwaysInlined<T>::value) {
+            // Value is directly in payload data.
+            uint32_t tmp =
+                (rep.GetPayload() & ((1ull << (sizeof(uint32_t) * 8))-1));
+            *out = reader.template GetUninlinedValue<T>(tmp);
         }
+        else {
+            // If the value is inlined, just decode it.
+            if (rep.IsInlined()) {
+                uint32_t tmp = (rep.GetPayload() &
+                                ((1ull << (sizeof(uint32_t) * 8))-1));
+                _DecodeInline(out, tmp);
+                return;
+            }
+            // Otherwise we have to read it from the file.
+            reader.Seek(rep.GetPayload());
+            *out = reader.template Read<T>();
+        }
+    }
 
-        auto iresult = _valueDedup->emplace(val, ValueRep());
+    ValueRep PackArray(_Writer w, VtArray<T> const &array) {
+        auto result = ValueRepForArray<T>(0);
+
+        // If this is an empty array we inline it.
+        if (array.empty())
+            return result;
+
+        if (!_arrayDedup) {
+            _arrayDedup.reset(
+                new typename decltype(_arrayDedup)::element_type);
+        }
+
+        auto iresult = _arrayDedup->emplace(array, result);
         ValueRep &target = iresult.first->second;
         if (iresult.second) {
-            // Not yet present.  Invoke the write function.
-            target = ValueRepFor<T>(writer.Tell());
-            writer.Write(val);
+            // Not yet present.
+            if (w.crate->_packCtx->writeVersion < Version(0,5,0)) {
+                target.SetPayload(w.Align(sizeof(uint64_t)));
+                w.WriteAs<uint32_t>(1);
+                w.WriteAs<uint32_t>(array.size());
+                w.WriteContiguous(array.cdata(), array.size());
+            } else {
+                // If we're writing 0.5.0 or greater, see if we can possibly
+                // compress this array.
+                target = _WritePossiblyCompressedArray(
+                    w, array, w.crate->_packCtx->writeVersion, 0);
+            }
         }
         return target;
     }
+
     template <class Reader>
-    inline void Unpack(Reader reader, ValueRep rep, T *out) const {
-        // If the value is inlined, just decode it.
-        if (rep.IsInlined()) {
-            uint32_t tmp = (rep.GetPayload() &
-                            ((1ull << (sizeof(uint32_t) * 8))-1));
-            _DecodeInline(out, tmp);
+    void UnpackArray(Reader reader, ValueRep rep, VtArray<T> *out) const {
+        // If payload is 0, it's an empty array.
+        if (rep.GetPayload() == 0) {
+            *out = VtArray<T>();
             return;
         }
-        // Otherwise we have to read it from the file.
         reader.Seek(rep.GetPayload());
-        *out = reader.template Read<T>();
-    }
-    void Clear() {
-        _valueDedup.reset();
-    }
-    std::unique_ptr<std::unordered_map<T, ValueRep, _Hasher>> _valueDedup;
-};
 
-// Scalar handler for inlined types -- no deduplication.
-template <class T>
-struct CrateFile::_ScalarValueHandlerBase<
-    T, typename std::enable_if<ValueTypeTraits<T>::isInlined>::type>
-: _ValueHandlerBase
-{
-    inline ValueRep Pack(_Writer writer, T val) {
-        // Inline it into the rep.
-        return ValueRepFor<T>(writer.GetInlinedValue(val));
-    }
-    template <class Reader>
-    inline void Unpack(Reader reader, ValueRep rep, T *out) const {
-        // Value is directly in payload data.
-        uint32_t tmp =
-            (rep.GetPayload() & ((1ull << (sizeof(uint32_t) * 8))-1));
-        *out = reader.GetUninlinedValue(tmp, static_cast<T *>(nullptr));
+        // Check version
+        Version fileVer(reader.crate->_boot);
+        if (fileVer < Version(0,5,0)) {
+            // Read and discard shape size.
+            reader.template Read<uint32_t>();
+        }
+        _ReadPossiblyCompressedArray(reader, rep, out, fileVer, 0);
     }
-};
 
-// Array handler for types that don't support arrays.
-template <class T, class Enable>
-struct CrateFile::_ArrayValueHandlerBase : _ScalarValueHandlerBase<T>
-{
     ValueRep PackVtValue(_Writer w, VtValue const &v) {
+        if constexpr (_SupportsArray<T>::value) {
+            if (v.IsArrayValued()) {
+                return this->PackArray(w, v.UncheckedGet<VtArray<T>>());
+            }
+        }
         return this->Pack(w, v.UncheckedGet<T>());
     }
 
     template <class Reader>
     void UnpackVtValue(Reader r, ValueRep rep, VtValue *out) {
+        if constexpr (_SupportsArray<T>::value) {
+            if (rep.IsArray()) {
+                VtArray<T> array;
+                this->UnpackArray(r, rep, &array);
+                out->Swap(array);
+                return;
+            }
+        }
         T obj;
         this->Unpack(r, rep, &obj);
         out->Swap(obj);
     }
+    
+    void Clear() {
+        if constexpr (!_IsAlwaysInlined<T>::value) {
+            _valueDedup.reset();
+        }
+        if constexpr (_SupportsArray<T>::value) {
+            _arrayDedup.reset();
+        }                
+    }
+    
+    std::unique_ptr<std::unordered_map<T, ValueRep, _Hasher>> _valueDedup;
+    std::unique_ptr<
+        std::unordered_map<VtArray<T>, ValueRep, _Hasher>> _arrayDedup;
+
 };
 
 // Don't compress arrays smaller than this.
@@ -2012,98 +2050,6 @@ _ReadPossiblyCompressedArray(
     }
 }
 
-// Array handler for types that support arrays -- does deduplication.
-template <class T>
-struct CrateFile::_ArrayValueHandlerBase<
-    T, typename std::enable_if<ValueTypeTraits<T>::supportsArray>::type>
-: _ScalarValueHandlerBase<T>
-{
-    ValueRep PackArray(_Writer w, VtArray<T> const &array) {
-        auto result = ValueRepForArray<T>(0);
-
-        // If this is an empty array we inline it.
-        if (array.empty())
-            return result;
-
-        if (!_arrayDedup) {
-            _arrayDedup.reset(
-                new typename decltype(_arrayDedup)::element_type);
-        }
-
-        auto iresult = _arrayDedup->emplace(array, result);
-        ValueRep &target = iresult.first->second;
-        if (iresult.second) {
-            // Not yet present.
-            if (w.crate->_packCtx->writeVersion < Version(0,5,0)) {
-                target.SetPayload(w.Align(sizeof(uint64_t)));
-                w.WriteAs<uint32_t>(1);
-                w.WriteAs<uint32_t>(array.size());
-                w.WriteContiguous(array.cdata(), array.size());
-            } else {
-                // If we're writing 0.5.0 or greater, see if we can possibly
-                // compress this array.
-                target = _WritePossiblyCompressedArray(
-                    w, array, w.crate->_packCtx->writeVersion, 0);
-            }
-        }
-        return target;
-    }
-
-    template <class Reader>
-    void UnpackArray(Reader reader, ValueRep rep, VtArray<T> *out) const {
-        // If payload is 0, it's an empty array.
-        if (rep.GetPayload() == 0) {
-            *out = VtArray<T>();
-            return;
-        }
-        reader.Seek(rep.GetPayload());
-
-        // Check version
-        Version fileVer(reader.crate->_boot);
-        if (fileVer < Version(0,5,0)) {
-            // Read and discard shape size.
-            reader.template Read<uint32_t>();
-        }
-        _ReadPossiblyCompressedArray(reader, rep, out, fileVer, 0);
-    }
-
-    ValueRep PackVtValue(_Writer w, VtValue const &v) {
-        return v.IsArrayValued() ?
-            this->PackArray(w, v.UncheckedGet<VtArray<T>>()) :
-            this->Pack(w, v.UncheckedGet<T>());
-    }
-
-    template <class Reader>
-    void UnpackVtValue(Reader r, ValueRep rep, VtValue *out) {
-        if (rep.IsArray()) {
-            VtArray<T> array;
-            this->UnpackArray(r, rep, &array);
-            out->Swap(array);
-        } else {
-            T obj;
-            this->Unpack(r, rep, &obj);
-            out->Swap(obj);
-        }
-    }
-
-    void Clear() {
-        // Invoke base implementation to clear scalar table.
-        _ScalarValueHandlerBase<T>::Clear();
-        _arrayDedup.reset();
-    }
-    
-    std::unique_ptr<
-        std::unordered_map<VtArray<T>, ValueRep, _Hasher>> _arrayDedup;
-};
-
-// _ValueHandler derives _ArrayValueHandlerBase, which in turn derives
-// _ScalarValueHandlerBase.  Those templates are specialized to handle types
-// that support or do not support arrays and types that are inlined or not
-// inlined.
-template <class T>
-struct CrateFile::_ValueHandler : public _ArrayValueHandlerBase<T> {};
-
-
 ////////////////////////////////////////////////////////////////////////
 // CrateFile
 
@@ -3967,14 +3913,14 @@ template <class T>
 CrateFile::_ValueHandler<T> &
 CrateFile::_GetValueHandler() {
     return *static_cast<_ValueHandler<T> *>(
-        _valueHandlers[static_cast<int>(TypeEnumFor<T>())]);
+        _valueHandlers[static_cast<int>(_TypeEnumFor<T>::value)]);
 }
 
 template <class T>
 CrateFile::_ValueHandler<T> const &
 CrateFile::_GetValueHandler() const {
     return *static_cast<_ValueHandler<T> const *>(
-        _valueHandlers[static_cast<int>(TypeEnumFor<T>())]);
+        _valueHandlers[static_cast<int>(_TypeEnumFor<T>::value)]);
 }
 
 template <class T>
@@ -4040,8 +3986,10 @@ CrateFile::_UnpackValue(ValueRep rep, T *out) const
     try {
         auto const &h = _GetValueHandler<T>();
         if (_useMmap) {
-            h.Unpack(_MakeReader(_MakeMmapStream(
-                                     &_mmapSrc, _debugPageMap.get())), rep, out);
+            h.Unpack(
+                _MakeReader(
+                    _MakeMmapStream(
+                        &_mmapSrc, _debugPageMap.get())), rep, out);
         } else if (_preadSrc) {
             h.Unpack(_MakeReader(_PreadStream(_preadSrc)), rep, out);
         } else {
@@ -4049,7 +3997,7 @@ CrateFile::_UnpackValue(ValueRep rep, T *out) const
         }
     }
     catch (...) {
-        TF_RUNTIME_ERROR("Corrupt asset <%s>: exception raised unpacking a "
+        TF_RUNTIME_ERROR("Corrupt asset <%s>: exception thrown unpacking a "
                          "%s, returning a value-initialized object",
                          GetAssetPath().c_str(),
                          ArchGetDemangled<T>().c_str());
@@ -4073,7 +4021,7 @@ CrateFile::_UnpackValue(ValueRep rep, VtArray<T> *out) const {
         }
     }
     catch (...) {
-        TF_RUNTIME_ERROR("Corrupt asset <%s>: exception raised unpacking a "
+        TF_RUNTIME_ERROR("Corrupt asset <%s>: exception thrown unpacking a "
                          "VtArray<%s>, returning an empty array",
                          GetAssetPath().c_str(),
                          ArchGetDemangled<T>().c_str());
@@ -4101,44 +4049,29 @@ CrateFile::_UnpackValue(ValueRep rep, VtValue *result) const {
         }
     }
     catch (...) {
-        TF_RUNTIME_ERROR("Corrupt asset <%s>: exception raised unpacking a "
+        TF_RUNTIME_ERROR("Corrupt asset <%s>: exception thrown unpacking a "
                          "value, returning an empty VtValue",
                          GetAssetPath().c_str());
         *result = VtValue();
     }
 }
 
-struct _GetTypeidForArrayTypes {
-    template <class T>
-    static std::type_info const &Get(bool array) {
-        return array ? typeid(VtArray<T>) : typeid(T);
-    }
-};
-
-struct _GetTypeidForNonArrayTypes {
-    template <class T>
-    static std::type_info const &Get(bool) {
-        return typeid(T);
-    }
-};
-
-template <bool SupportsArray>
-using _GetTypeid = typename std::conditional<SupportsArray,
-                                             _GetTypeidForArrayTypes,
-                                             _GetTypeidForNonArrayTypes>::type;
-
 std::type_info const &
 CrateFile::GetTypeid(ValueRep rep) const
 {
     switch (rep.GetType()) {
 #define xx(ENUMNAME, _unused, T, SUPPORTSARRAY)                                \
         case TypeEnum::ENUMNAME:                                               \
-            return _GetTypeid<SUPPORTSARRAY>::Get<T>(rep.IsArray());
+            if constexpr (SUPPORTSARRAY) {                                     \
+                return rep.IsArray() ? typeid(VtArray<T>) : typeid(T);         \
+            }                                                                  \
+            else {                                                             \
+                return typeid(T);                                              \
+            }
 
 #include "crateDataTypes.h"
 
 #undef xx
-
     default:
         return typeid(void);
     };
@@ -4147,7 +4080,7 @@ CrateFile::GetTypeid(ValueRep rep) const
 template <class T>
 void
 CrateFile::_DoTypeRegistration() {
-    auto typeEnumIndex = static_cast<int>(TypeEnumFor<T>());
+    auto typeEnumIndex = static_cast<int>(_TypeEnumFor<T>::value);
     auto valueHandler = new _ValueHandler<T>();
     _valueHandlers[typeEnumIndex] = valueHandler;
 
@@ -4193,7 +4126,7 @@ void
 CrateFile::_DeleteValueHandlers() {
 #define xx(_unused1, _unused2, T, _unused3)                                    \
     delete static_cast<_ValueHandler<T> *>(                                    \
-        _valueHandlers[static_cast<int>(TypeEnumFor<T>())]);
+        _valueHandlers[static_cast<int>(_TypeEnumFor<T>::value)]);
 
 #include "crateDataTypes.h"
 
@@ -4204,7 +4137,7 @@ void
 CrateFile::_ClearValueHandlerDedupTables() {
 #define xx(_unused1, _unused2, T, _unused3)                                    \
     static_cast<_ValueHandler<T> *>(                                           \
-        _valueHandlers[static_cast<int>(TypeEnumFor<T>())])->Clear();
+        _valueHandlers[static_cast<int>(_TypeEnumFor<T>::value)])->Clear();
 
 #include "crateDataTypes.h"
 
index 67b1aa385f4aa85bbc316d9327fcb394d4d5c41c..828fc60173eeaaa40229f8d220e894897e3b2f6c 100644 (file)
@@ -949,15 +949,10 @@ private:
 
     ////////////////////////////////////////////////////////////////////////
 
-    // Base class, to have a pointer type.
-    struct _ValueHandlerBase;
-    template <class, class=void> struct _ScalarValueHandlerBase;
-    template <class, class=void> struct _ArrayValueHandlerBase;
+    // Type-specific _ValueHandler<T> derives _ValueHandlerBase, just so we can
+    // have common pointers to specific instances.
+    struct _ValueHandlerBase {};
     template <class> struct _ValueHandler;
-
-    friend struct _ValueHandlerBase;
-    template <class, class> friend struct _ScalarValueHandlerBase;
-    template <class, class> friend struct _ArrayValueHandlerBase;
     template <class> friend struct _ValueHandler;
 
     template <class T> inline _ValueHandler<T> &_GetValueHandler();