514071 - warning: format ‘%llu’ expects type ‘long long unsigned int’, but argument 4 has type ‘long unsigned int’

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Blake Kaplan (:mrbkap) (inactive)]

Normally these are pretty easy to fix, but I'm really not sure about this one the full g++ output is:

jscntxt.cpp: In function ‘void DumpEvalCacheMeter(JSContext*)’:
jscntxt.cpp:559: warning: format ‘%llu’ expects type ‘long long unsigned int’, but argument 4 has type ‘long unsigned int’

I think the problem is that I'm on a 64-bit machine. The actual type in question is uint64 so on a 64 bit Linux system only wants %lu and really wants %llu on a 32 bit system. This is one of those cases where I really miss C++ iostreams.

Comment 1

[Brendan Eich [:brendan]]

Iostreams, bah -- isn't there a new and portable %-escaped format specifier for ptrdiff_t and other standard int types?

/be

Comment 2

[(not currently active) Ted Mielczarek]

You can use the horrid macros defined in inttypes, although we don't have that on Windows, I'm sure:
http://www.opengroup.org/onlinepubs/9699919799/basedefs/inttypes.h.html

Comment 3

[Jeff Walden [:Waldo]]

What Ted said!  Shouldn't be hard, just more of the same of jsstdint.h.

Comment 4

[Graydon Hoare :graydon]

Yeah. You want the string-pasted incantation "%" PRIu64. Or PRIuPTR if you're doing pointer-sized, as Brendan suggested. Either way, I'll take this, it's just janitorial.

We may wind up having to duplicate platform-neutral definitions a little in nanojit's VMPI layer.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

This has gotten a lot worse now that jstracer.cpp is built on x64. I now get 28 of these in that file alone.

Comment 6

[Nicholas Nethercote [inactive]]

Casting the args to 'long long unsigned' is also a possibility, one that's arguably no uglier than PRIuWhatever.

Comment 7

[Brendan Eich [:brendan]]

(In reply to comment #6)
> Casting the args to 'long long unsigned' is also a possibility, one that's
> arguably no uglier than PRIuWhatever.

That's been the traditional solution in js/src.

Someone do something, my warning greps are non-zero which makes me itchy.

/be

Comment 8

[Julian Seward [:jseward]]

Attachment: stops gcc complaining on 32/64-bit opensuse 11

Comment 9

[Julian Seward [:jseward]]

(In reply to comment #8)
> stops gcc complaining on 32/64-bit opensuse 11

Might be a bit overkill, but .. in every place where the 
difference between two pointers is printed, I widened it
to long long int and used %lld.  This seems simpler than
figuring out a type and format specifier which reliably
means "machine word sized signed int" on all platforms,
particularly Win64.

Comment 11

[Andreas Gal :gal]

Luke, what do you think? Do we have to do this? Any better ideas?

Comment 12

[Graydon Hoare :graydon]

The better option is to use PRIuWhatever in our format strings, IMO. But to get those definitions right involves autoconfery as well as some fallback stuff in jsinttypes or jsstdint. I keep starting on that and then getting distracted by other work. It requires a lot of tedious fiddling across platforms.

Comment 13

[Luke Wagner [:luke]]

(In reply to comment #11)
> Luke, what do you think? Do we have to do this? Any better ideas?

I say we let template argument deduction do the work instead.  IOStreams isn't the only way to get this; assuming we don't mind converting a single "A%dC%p" formatted ::fprintf call into 4 separate ::fprintf calls (this is for debug output, right?), it would be pretty easy to throw in a few hollow templates to bring us type-inferred goodness:

namespace js {
 template <class T1> void fprint(FILE *f, const T1 &t1) {
  ::fprintf(f, tl::TypeToFormatString<T1>::result, t1);
 }
 template <class T1,class T2> void fprint(FILE *f, const T1 &t1, const T2 &t2) {
  ::fprintf(f, tl::TypeToFormatString<T1>::result, t1);
  ::fprintf(f, tl::TypeToFormatString<T2>::result, t2);
 }
 ...
}

uint64_t l;
js::fprint(f, "got: ", l);

Getting formatting etc in on the game would involve further trickery,
but it seems eminently doable.

Comment 14

[David Mandelin [:dmandelin]]

Pushed to TM as 09e12b174aca.

Comment 15

[Brendan Eich [:brendan]]

That should be (length + 1) * sizeof(jschar), right? rs=me on followup.

/be

Comment 16

[Brendan Eich [:brendan]]

Comment 15 was meant for bug 516458 but I dropped it here based on a commit message typo ;-).

/be

Comment 17

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/9131d01c2d2a

Comment 18

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/18d5079f0127