JDevlieghere marked 2 inline comments as done. JDevlieghere added inline comments.
================ Comment at: lldb/include/lldb/Interpreter/OptionValue.h:325-344 + template <typename T, + std::enable_if_t<!std::is_pointer<T>::value, bool> = true> + std::optional<T> GetValueAs() const { + if constexpr (std::is_same_v<T, uint64_t>) + return GetUInt64Value(); + if constexpr (std::is_same_v<T, int64_t>) + return GetSInt64Value(); ---------------- mib wrote: > nit: In the template argument, you use `std::is_pointer<T>::value` instead of > `std::is_pointer_v<T>` and the in the if statement you do the opposite > (`std::is_same_v<T>` vs `std::is_same<T>::value`). I personally not a fan of > the `_v` alias but what I'm saying here is it would be good to stay > consistent. Good point, I've unified everything to use the `_v` variant. ================ Comment at: lldb/source/Interpreter/Property.cpp:229 } + assert(m_value_sp && "invalid property definition"); } ---------------- mib wrote: > May be we should print a warning or error to the user ? Properties are all tablegen'd so if this fires we did something wrong. There's nothing actionable here for a user. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149774/new/ https://reviews.llvm.org/D149774 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits