This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Fix sizeof() in Scalar::operator=()
AbandonedPublic

Authored by miko on Jun 2 2020, 4:03 AM.

Details

Reviewers
teemperor
Summary

In Scalar::operator=(long long) the size of the result was calculated using sizeof(long) instead of sizeof(long long). On Windows sizeof(long)==4 but sizeof(long long)==8.

Diff Detail

Event Timeline

miko created this revision.Jun 2 2020, 4:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 2 2020, 4:03 AM
labath added a subscriber: labath.Jun 2 2020, 4:36 AM

The fix looks obviously correct, but could you please also add a unit test for this. I guess this would manifest itself as the inability to roundtrip e.g. std::numeric_limits<long long>::max() to a Scalar and back.

It's not required, but if you're interested, it would be nice to change all of these assignment operators to do return *this = Scalar(v);. That would remove this class of bugs (constructor/operator= inconsistencies) completely.

teemperor requested changes to this revision.Jun 2 2020, 4:39 AM
teemperor added a subscriber: teemperor.

Could you add a unit test for this? See the`ScalarTest.cpp` for the other Scalar tests.
Beside that this LGTM.

This revision now requires changes to proceed.Jun 2 2020, 4:39 AM