[Lldb-commits] [PATCH] D24126: Make Scalar::GetValue more consistent

[Zachary Turner via lldb-commits]

What about a case in the unit test where the value overflows the type?
Would that be useful?
On Thu, Sep 1, 2016 at 3:50 AM Pavel Labath via lldb-commits <
lldb-commits at lists.llvm.org> wrote:

> labath created this revision.
> labath added reviewers: clayborg, granata.enrico.
> labath added a subscriber: lldb-commits.
>
> It seems the original intention of the function was printing signed values
> in decimal format, and
> unsigned values in hex (without the leading "0x"). However, signed and
> unsigned long were
> exchanged, which lead to amusing test failures in TestMemoryFind.py.
>
> Instead of just switching the two, I think we should just print everything
> in decimal here, as
> the current behaviour is very confusing (especially when one does not
> request printing of types).
> Nothing seems to depend on this behaviour except and we already have a way
> for the user to
> request the format he wants when printing values for most commands (which
> presumably does not go
> through this function).
>
> I also add a unit tests for the function in question.
>
> https://reviews.llvm.org/D24126
>
> Files:
>
> packages/Python/lldbsuite/test/functionalities/memory/find/TestMemoryFind.py
>   source/Core/Scalar.cpp
>   unittests/Core/ScalarTest.cpp
>
> Index: unittests/Core/ScalarTest.cpp
> ===================================================================
> --- unittests/Core/ScalarTest.cpp
> +++ unittests/Core/ScalarTest.cpp
> @@ -19,6 +19,7 @@
>  #include "lldb/Core/Scalar.h"
>  #include "lldb/Core/DataExtractor.h"
>  #include "lldb/Host/Endian.h"
> +#include "lldb/Core/StreamString.h"
>
>  using namespace lldb_private;
>
> @@ -103,3 +104,31 @@
>      ASSERT_TRUE(u_scalar.ExtractBitfield(len - 4, 4));
>      ASSERT_EQ(0, memcmp(&b2, u_scalar.GetBytes(), sizeof(b2)));
>  }
> +
> +template <typename T>
> +static std::string
> +ScalarGetValue(T value)
> +{
> +    StreamString stream;
> +    Scalar(value).GetValue(&stream, false);
> +    return stream.GetString();
> +}
> +
> +TEST(ScalarTest, GetValue)
> +{
> +    EXPECT_EQ("12345", ScalarGetValue<signed short>(12345));
> +    EXPECT_EQ("-12345", ScalarGetValue<signed short>(-12345));
> +    EXPECT_EQ("12345", ScalarGetValue<unsigned short>(12345));
> +
> +    EXPECT_EQ("12345", ScalarGetValue<signed int>(12345));
> +    EXPECT_EQ("-12345", ScalarGetValue<signed int>(-12345));
> +    EXPECT_EQ("12345", ScalarGetValue<unsigned int>(12345));
> +
> +    EXPECT_EQ("12345678", ScalarGetValue<signed long>(12345678L));
> +    EXPECT_EQ("-12345678", ScalarGetValue<signed long>(-12345678L));
> +    EXPECT_EQ("12345678", ScalarGetValue<unsigned long>(12345678UL));
> +
> +    EXPECT_EQ("1234567890123", ScalarGetValue<signed long
> long>(1234567890123LL));
> +    EXPECT_EQ("-1234567890124", ScalarGetValue<signed long
> long>(-1234567890123LL));
> +    EXPECT_EQ("1234567890122", ScalarGetValue<unsigned long
> long>(1234567890123ULL));
> +}
> Index: source/Core/Scalar.cpp
> ===================================================================
> --- source/Core/Scalar.cpp
> +++ source/Core/Scalar.cpp
> @@ -308,18 +308,16 @@
>      case e_void:
>          break;
>      case e_sint:
> -    case e_ulong:
> +    case e_slong:
>      case e_slonglong:
>      case e_sint128:
>      case e_sint256:
> -        s->Printf("%s",m_integer.toString(10,true).c_str());
> -        break;
>      case e_uint:
> -    case e_slong:
> +    case e_ulong:
>      case e_ulonglong:
>      case e_uint128:
>      case e_uint256:
> -        s->Printf("%s",m_integer.toString(16,false).c_str());
> +        s->PutCString(m_integer.toString(10, true).c_str());
>          break;
>      case e_float:
>      case e_double:
> Index:
> packages/Python/lldbsuite/test/functionalities/memory/find/TestMemoryFind.py
> ===================================================================
> ---
> packages/Python/lldbsuite/test/functionalities/memory/find/TestMemoryFind.py
> +++
> packages/Python/lldbsuite/test/functionalities/memory/find/TestMemoryFind.py
> @@ -23,7 +23,6 @@
>          # Find the line number to break inside main().
>          self.line = line_number('main.cpp', '// break here')
>
> -    @expectedFailureAll(archs=["i386", "arm"])
>      def test_memory_find(self):
>          """Test the 'memory find' command."""
>          self.build()
>
>
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20160901/7b70f3c6/attachment.html>