llvm dev - Sep 2017 - Improve ScopedPrinter::printNumber? (was: [llvm] r313816 - [llvm-readobj] Fix 'Teach readobj to dump .res files'.)

Seeing the below commit, I wondered whether that's a genuine fix or 
rather a workaround for a shortcoming in llvm::ScopedPrinter::printNumber.

The latter has overloads for [u]int{8,16,32,64}_t, presumably so that it 
can take arguments of arbitrary integer types.  However, at least on 
recent macOS uint64_t is 'unsigned long long' (and uint32_t is
'unsigned
int') while size_t is 'unsigned long', so there's no best
matching overload.

Would it thus be better to change the ScopedPrinter::printNumber to be 
overloaded on all the "raw" integer types, instead of the stdint.h
typedefs?


On 09/20/2017 11:03 PM, Marek Sokolowski via llvm-commits
wrote:
> Author: mnbvmar
> Date: Wed Sep 20 14:03:37 2017
> New Revision: 313816
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=313816&view=rev
> Log:
> [llvm-readobj] Fix 'Teach readobj to dump .res files'.
> 
> Fix-up for r313790. Some buildbots couldn't convert size_t to
> uint{}_t; do it manually.
> 
> Modified:
>      llvm/trunk/tools/llvm-readobj/WindowsResourceDumper.cpp
> 
> Modified: llvm/trunk/tools/llvm-readobj/WindowsResourceDumper.cpp
> URL:
http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-readobj/WindowsResourceDumper.cpp?rev=313816&r1=313815&r2=313816&view=diff
>
=============================================================================>
--- llvm/trunk/tools/llvm-readobj/WindowsResourceDumper.cpp (original)
> +++ llvm/trunk/tools/llvm-readobj/WindowsResourceDumper.cpp Wed Sep 20
14:03:37 2017
> @@ -69,7 +69,7 @@ void Dumper::printEntry(const ResourceEn
>     SW.printNumber("Version (major)", Ref.getMajorVersion());
>     SW.printNumber("Version (minor)", Ref.getMinorVersion());
>     SW.printNumber("Characteristics", Ref.getCharacteristics());
> -  SW.printNumber("Data size", Ref.getData().size());
> +  SW.printNumber("Data size", (uint64_t)Ref.getData().size());
>     SW.printBinary("Data:", Ref.getData());
>     SW.startLine() << "\n";
>   }

I don't have access to the source at the moment, but it could also just be
templated. Not sure which approach is best
On Thu, Sep 21, 2017 at 1:41 AM Stephan Bergmann via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Seeing the below commit, I wondered whether that's a genuine fix or
> rather a workaround for a shortcoming in llvm::ScopedPrinter::printNumber.
>
> The latter has overloads for [u]int{8,16,32,64}_t, presumably so that it
> can take arguments of arbitrary integer types.  However, at least on
> recent macOS uint64_t is 'unsigned long long' (and uint32_t is
'unsigned
> int') while size_t is 'unsigned long', so there's no best
matching
> overload.
>
> Would it thus be better to change the ScopedPrinter::printNumber to be
> overloaded on all the "raw" integer types, instead of the
stdint.h
> typedefs?
>
>
> On 09/20/2017 11:03 PM, Marek Sokolowski via llvm-commits wrote:
> > Author: mnbvmar
> > Date: Wed Sep 20 14:03:37 2017
> > New Revision: 313816
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=313816&view=rev
> > Log:
> > [llvm-readobj] Fix 'Teach readobj to dump .res files'.
> >
> > Fix-up for r313790. Some buildbots couldn't convert size_t to
> > uint{}_t; do it manually.
> >
> > Modified:
> >      llvm/trunk/tools/llvm-readobj/WindowsResourceDumper.cpp
> >
> > Modified: llvm/trunk/tools/llvm-readobj/WindowsResourceDumper.cpp
> > URL:
>
http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-readobj/WindowsResourceDumper.cpp?rev=313816&r1=313815&r2=313816&view=diff
> >
>
=============================================================================>
> --- llvm/trunk/tools/llvm-readobj/WindowsResourceDumper.cpp (original)
> > +++ llvm/trunk/tools/llvm-readobj/WindowsResourceDumper.cpp Wed Sep 20
> 14:03:37 2017
> > @@ -69,7 +69,7 @@ void Dumper::printEntry(const ResourceEn
> >     SW.printNumber("Version (major)",
Ref.getMajorVersion());
> >     SW.printNumber("Version (minor)",
Ref.getMinorVersion());
> >     SW.printNumber("Characteristics",
Ref.getCharacteristics());
> > -  SW.printNumber("Data size", Ref.getData().size());
> > +  SW.printNumber("Data size",
(uint64_t)Ref.getData().size());
> >     SW.printBinary("Data:", Ref.getData());
> >     SW.startLine() << "\n";
> >   }
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170921/04f2d9ef/attachment.html>