[lldb-dev] Was this an unintended consequence of the Args switch to StringRef's?

[Zachary Turner via lldb-dev]

Honestly i think it seems reasonable for getAsInteger to just work in this
case, so I'll try to add it to llvm
On Thu, Apr 6, 2017 at 10:15 AM Jim Ingham <jingham at apple.com> wrote:

> BTW, Jason says that sometimes the gdb remote protocol does send 0x
> prefaced numbers.  Apparently not in the cases which have been changed to
> use getAsInteger with a radix of 16, but that is something we'll have to
> keep in mind.
>
> Jim
>
> > On Apr 5, 2017, at 6:55 PM, Jim Ingham <jingham at apple.com> wrote:
> >
> > base of 16 was only used in a few places.  It's used frequently in the
> gdb remote protocol code, but I don't think the gdb remote protocol ever
> sends hex numbers with the 0x prefix, it assumes you know the radix, so
> that should be okay.  Other than that this was the only consequential
> usage.  So I went with detecting the 0x by hand and treating that
> separately (r299609).
> >
> > Why does getAsInteger return false if it succeeds in getting an
> integer?  That just seems weird to me.
> >
> > Jim
> >
> >
> >> On Apr 5, 2017, at 4:36 PM, Jim Ingham via lldb-dev <
> lldb-dev at lists.llvm.org> wrote:
> >>
> >> It does seem unfortunate that getAsInteger works differently from
> strtol's documented behavior in this little way...  Makes conversions like
> this one trickier than necessary.  But presumably somebody had a reason for
> it to be different?
> >>
> >> Anyway, I have to go see if this feature of strtol & Co. is used in
> more places than CommandObjectMemoryWrite.  If it was a common idiom then
> it might be nice to have direct support for it.  But if it's just in one
> place, then hacking around it like this seems fine.
> >>
> >> Jim
> >>
> >>> On Apr 5, 2017, at 4:18 PM, Zachary Turner <zturner at google.com> wrote:
> >>>
> >>> What if you write this:
> >>>
> >>> // Explicitly try hex.  This will catch the case where there is no 0x
> prefix.
> >>> if (entry.ref.getAsInteger(16, uval64)) {
> >>> // If that fails, let the algorithm auto-detect the radix.  This will
> catch the case where there is a 0x prefix.
> >>> if (entry.ref.getAsInteger(0, uval64)) {
> >>>
> >>> Would that work?  Alternatively, we could probably update the
> implementation of getAsInteger in LLVM to you to manually hint the radix
> AND prefix the number, as long as the prefix matches the hint.
> >>>
> >>> On Wed, Apr 5, 2017 at 4:10 PM Jim Ingham <jingham at apple.com> wrote:
> >>>>
> >>>
> >>> I think somebody was being too clever.  The real purpose for
> specifying the format is to set the byte size of the item written down:
> >>>
> >>>   OptionValueUInt64 &byte_size_value =
> m_format_options.GetByteSizeValue();
> >>>   size_t item_byte_size = byte_size_value.GetCurrentValue();
> >>>
> >>> and to do some special magic for c-strings and the like.  Somebody
> (not me) must have thought overloading "hex" format to mean "you can drop
> the 0x" was a tasty little side benefit.
> >>>
> >>> But it's been that way forever, and I don't know who's relying on that
> behavior, so I'd rather not change it (intentionally) if we don't have to.
> >>>
> >>> Jim
> >>>
> >>>> On Wed, Apr 5, 2017 at 3:37 PM Jim Ingham via lldb-dev <
> lldb-dev at lists.llvm.org> wrote:
> >>>> memory write's argument ingestion was changed as part of the
> StringRefifying of Args so that we get:
> >>>>
> >>>> (lldb) memory write &buffer 0x62
> >>>> error: '0x62' is not a valid hex string value.
> >>>>
> >>>> That seems unexpected and not desirable.  What's going on is that the
> default format is hex, and if the format is hex, the command also supports:
> >>>>
> >>>> (lldb) memory write -f x &buffer 62
> >>>> (lldb) fr v/x buffer[0]
> >>>> (char) buffer[0] = 0x62
> >>>>
> >>>> The StringRef version of the args parsing is:
> >>>>
> >>>>     case eFormatDefault:
> >>>>     case eFormatBytes:
> >>>>     case eFormatHex:
> >>>>     case eFormatHexUppercase:
> >>>>     case eFormatPointer:
> >>>>       // Decode hex bytes
> >>>>       if (entry.ref.getAsInteger(16, uval64)) {
> >>>>
> >>>> The problem is that passing "0x62" to getAsInteger with a radix of 16
> rejects "0x62".
> >>>>
> >>>> We do want to hint the radix.  But it seems weird to reject an
> explicit indicator.  Is there some clever way to use the StringRef
> functions to get the desired effect, or do I have to hack around this by
> manually stripping the 0x if I see it?
> >>>>
> >>>> Jim
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> lldb-dev mailing list
> >>>> lldb-dev at lists.llvm.org
> >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
> >>>
> >>
> >> _______________________________________________
> >> lldb-dev mailing list
> >> lldb-dev at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20170406/ab7aae5c/attachment-0001.html>