Explicitly cast int -> char to prevent conversion warnings

[Manu343726]

This prevents integer conversion warnings so the library can be compiled with GCC and -Wconversion enabled (also -Werror -Wall -Wextra -pedantic, but I think the offender was -Wconversion) as "discussed" in #552.

Note this change makes possible to build fmt with these flags, but the unit tests cannot be compiled, googletest triggers a lot of conversion warnings.

[vitaut]

Thanks for the PR. One of the casts makes total sense but the one in visit_int seems misplaced and makes tests to fail. Do you get warnings there too?

[vitaut]

I don't think this is correct.

[Manu343726]

Hi,
Sorry for the delay,

You were right, that change was wrong. I've changed the commit ton only include an explicit cast in visit_char case. But it seems that the test that is failing:

4: [ RUN      ] FormatterTest.Examples
4: /home/manu343726/Documentos/fmt/test/format-test.cc:1494: Failure
4: Value of: format(L"Cyrillic letter {}", L'\x42e')
4:   Actual: L"Cyrillic letter ."
4: Expected: L"Cyrillic letter \x42e"
4: Which is: L"Cyrillic letter \x42E"
4: [  FAILED  ] FormatterTest.Examples (0 ms

is related to the visit_char cast instead of the visit_int one (I've pushed the commit to fire your build again). So, from the multiple visit_char functions in your codebase:

$ ack visit_char
test/util-test.cc
614:  Result visit_char(int value) { return static_cast<char>(value); }

fmt/printf.h
67:  char visit_char(int) { return 'c'; }
113:  void visit_char(char value) {
251:  void visit_char(int value) {

fmt/format.h
1608:  Result visit_char(int value) {
1684:      return FMT_DISPATCH(visit_char(static_cast<char>(arg.int_value)));
2005:  void visit_char(int value) {

I see only one non-void candidate that could fit in format.h:1684 is Result visit_char(int value) in format.h:1608, but that function takes an int so a conversion should not be necessary. But since a warning is issued there if no static_cast<char>() is done, I'm starting to think that this is a warning triggered by overload resolution candidates, and not by the final call.

[vitaut]

The reason why the test is failing is that the character type can be wchar_t in which case it shouldn't be cast to char. I think a proper fix is to change

void visit_char(char value)

to

void visit_char(int value)

in line 113 of printf.h. Could you check if it fixes the warning? (I wasn't able to repro it for some reason.)

[Manu343726]

Great, that fixes the warnings, all tests passing. Thanks a lot for your time.
UPDATE: No, not all on my CI.... Time for one more commit...

[Manu343726]

Ooook fixed https://travis-ci.org/foonathan/cppast/builds/266489096

[vitaut]

Merged, thanks!