Skip to content

Fix -Wsign-conversion error on Android by using the right function#2245

Merged
gennadiycivil merged 1 commit intogoogle:masterfrom
daquexian:fix_wsign_conversion
May 3, 2019
Merged

Fix -Wsign-conversion error on Android by using the right function#2245
gennadiycivil merged 1 commit intogoogle:masterfrom
daquexian:fix_wsign_conversion

Conversation

@daquexian
Copy link
Copy Markdown
Contributor

@daquexian daquexian commented May 2, 2019

Google Test build is broken on Android by the following error:

~/googletest/googletest/src/gtest-printers.cc:240:43: error:
      implicit conversion changes signedness: 'wchar_t' to 'int' [-Werror,-Wsign-conversion]
    *os << ", 0x" << String::FormatHexInt(static_cast<UnsignedChar>(c));
                     ~~~~~~               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
~/googletest/googletest/src/gtest-printers.cc:255:3: note: in
      instantiation of function template specialization 'testing::internal::PrintCharAndCodeTo<wchar_t,
      wchar_t>' requested here
  PrintCharAndCodeTo<wchar_t>(wc, os);
  ^
1 error generated.

I fixed it by using FormatHexUInt32 instead of FormatHexInt. After all, UnsignedChar is intended to be an unsigned type.

@gennadiycivil
Copy link
Copy Markdown
Contributor

gennadiycivil commented May 2, 2019

CI fails

@daquexian
Copy link
Copy Markdown
Contributor Author

daquexian commented May 2, 2019 •
edited
Loading

Hmm.. wchar_t is unsigned on some platforms and is signed on others.

I force pushed and just added a static_cast now

@daquexian daquexian force-pushed the fix_wsign_conversion branch from aac65ec to ca642a9 Compare May 2, 2019 16:24
@gennadiycivil
Copy link
Copy Markdown
Contributor

gennadiycivil commented May 3, 2019

@ngie-eign please take a look. Will this work for you ?

ngie-eign
ngie-eign approved these changes May 3, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@ngie-eign ngie-eign left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@gennadiycivil gennadiycivil merged commit ca642a9 into google:master May 3, 2019
gennadiycivil added a commit that referenced this pull request May 3, 2019
@gennadiycivil
Merge pull request #2245 from daquexian:fix_wsign_conversion
bf3ef5c 
PiperOrigin-RevId: 246550729
@daquexian daquexian deleted the fix_wsign_conversion branch May 4, 2019 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@ngie-eign ngie-eign ngie-eign approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@daquexian @gennadiycivil @ngie-eign @googlebot