This is an archive of the discontinued LLVM Phabricator instance.

Make LLDB -Werror clean under clang
ClosedPublic

Authored by zturner on Oct 4 2016, 11:34 AM.

Details

Summary

Some of this is in Windows specific code, but some of it is not. In a few places I think I fixed real bugs in LLDB, but not sure how to really test this.

Diff Detail

Repository
rL LLVM

Event Timeline

zturner updated this revision to Diff 73515.Oct 4 2016, 11:34 AM
zturner retitled this revision from to Make LLDB -Werror clean under clang.
zturner updated this object.
zturner added reviewers: amccarth, labath.
zturner added a subscriber: lldb-commits.
labath accepted this revision.Oct 4 2016, 11:46 AM
labath edited edge metadata.

Seems reasonable. For testing we'll yell at you if the buildbots break.

I take it we can now freely use the %z printf modifier.

This revision is now accepted and ready to land.Oct 4 2016, 11:46 AM

Thankfully, yes. Technically it's not supported by MSVC 2013, which is still a supported LLVM compiler. But we said long ago that we require MSVC 2015 for running the test suite on Windows. And in any case, LLVM is bumping to MSVC 2015 within the next 2 weeks. So I'm going to go ahead and start using %z now.

Eugene.Zelenko added inline comments.
tools/lldb-mi/Platform.h
16 ↗(On Diff #73515)

This is application header. Should be in quotes. Same below.

zturner marked an inline comment as done.Oct 4 2016, 11:59 AM
zturner added inline comments.
tools/lldb-mi/Platform.h
16 ↗(On Diff #73515)

Thanks for pointing this out. Fixed locally

amccarth accepted this revision.Oct 4 2016, 2:22 PM
amccarth edited edge metadata.

lgtm

source/Host/common/UDPSocket.cpp
106 ↗(On Diff #73515)

Yuck. Given that this is going to get reduced from UTF-16 to MBCS, it might be cleaner to leave the format string alone and call gai_strerrorA explicitly on Windows. I guess it would be just as ugly that way.

source/Utility/SelectHelper.cpp
122 ↗(On Diff #73515)

I find this logic less clear than the original version, since it's not obvious that updateMaxFd uses an in-out parameter until you go look up to see what it does. It also seems weird to have an output parameter come before an input-only parameter.

Perhaps if updateMaxFd returned the value instead of updating the parameter, it would be more obvious:

max_fd = updateMaxFd(max_fd, fd);
This revision was automatically updated to reflect the committed changes.
zturner marked an inline comment as done.