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

[Zachary Turner via lldb-commits]

Actually you're right, i read the test backwards and thought the string was
the argument. I shouldn't do this 30 seconds after waking up :)
On Thu, Sep 1, 2016 at 7:26 AM Pavel Labath <labath at google.com> wrote:

> I don't think that even makes sense. :)
>
> Scalar class has constructors overrides for each of the primitive
> types. I cannot construct it with a value which is larger than it will
> fit in the primitive type. I suppose I could try the boundary
> conditions like UINT32_MAX, but that's about it.
>
> Or I am misunderstanding the question..
>
> pl
>
> On 1 September 2016 at 15:15, Zachary Turner <zturner at google.com> wrote:
> > 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/36654a14/attachment.html>