Derived from bug 1308897. I don't see any code in JS_smprintf or dosprintf that explicitly handles PRI* string, like PRIu64. on OSX, PRIu64 is "llu", and it matches to current implementation, but I'm not sure if it's so on all env, since I see different definition in tree, like "I64u".

Best case scenario we would remove the custom format code and just standard function, maybe with some wrapper to do dynamic memory allocation (bug 1300320).

Bug 553032 included some JS_smprintf tests that try to ensure that the PRI* formats are handled there.

nice :D so, they're guaranteed to be supported on all tested env now. might be nice to static_assert that PRI* is supported value, so that not-supported case will be caught on compile time. also, adding PRI* to jsprf.h comment will be helpful.

(In reply to Tooru Fujisawa [:arai] from comment #3) > might be nice to static_assert that PRI* is supported value, > so that not-supported case will be caught on compile time. I think the new testPrintf.cpp test should cause an assertion failure (in jsprf.cpp) if one of them doesn't work. Though, I haven't actually tried this... > also, adding PRI* to jsprf.h comment will be helpful. The comment currently mentions just: ** %zd, %zo, %zu, %zx, %zX - size_t versions of above ** %Id, %Io, %Iu, %Ix, %IX - size_t versions of above (for Windows compat) ** You should use PRI*SIZE macros instead I'm not sure if that's sufficient.

(In reply to Tom Tromey :tromey from comment #4) > (In reply to Tooru Fujisawa [:arai] from comment #3) > > > might be nice to static_assert that PRI* is supported value, > > so that not-supported case will be caught on compile time. > > I think the new testPrintf.cpp test should cause an assertion failure > (in jsprf.cpp) if one of them doesn't work. Though, I haven't actually > tried this... I see. we could check this again once we found any not-supported case. > > also, adding PRI* to jsprf.h comment will be helpful. > > The comment currently mentions just: > > ** %zd, %zo, %zu, %zx, %zX - size_t versions of above > ** %Id, %Io, %Iu, %Ix, %IX - size_t versions of above (for Windows > compat) > ** You should use PRI*SIZE macros instead > > I'm not sure if that's sufficient. Adding comment for PRI*64 would be nice, if it's preferred over ll*

(In reply to Tooru Fujisawa [:arai] from comment #5) > Adding comment for PRI*64 would be nice, if it's preferred over ll* The story here is that you have to use PRI*64 if the type is int64_t or uint64_t. If the type is "long long" then ll* is correct. So after thinking about it I think no more comment is needed. Or maybe a pointer to cppreference's page on the topic? What do you think? One nice thing is that, now, if one writes the wrong thing, the build will fail on some platform.

sorry, I haven't understood the change there. with fixed comment and impl, it's now clear that uint64_t doesn't match "%llu". I agree that comment is not needed. maybe we could just close this.