Page MenuHomePhabricator

[llvm-size] Reject unknown radix values
ClosedPublic

Authored by eush on Oct 28 2018, 9:21 AM.

Details

Summary

This addresses https://bugs.llvm.org/show_bug.cgi?id=39403 by making
-radix en enumeration option with 8, 10, and 16 as the only accepted
values.

Diff Detail

Repository
rL LLVM

Event Timeline

eush created this revision.Oct 28 2018, 9:21 AM
MaskRay accepted this revision.Oct 28 2018, 10:21 AM
This revision is now accepted and ready to land.Oct 28 2018, 10:21 AM
jhenderson requested changes to this revision.Oct 29 2018, 3:55 AM

Code change is fine, but please add a test to show that an unknown value is rejected with a useful error message.

This revision now requires changes to proceed.Oct 29 2018, 3:55 AM
eush updated this revision to Diff 171510.Oct 29 2018, 8:39 AM

Added a test.

jhenderson accepted this revision.Oct 29 2018, 9:48 AM

LGTM with the suggested changes in the CHECK command names.

test/tools/llvm-size/unknown-radix.test
2 ↗(On Diff #171510)

Don't use a CHECK prefix with "NOT" in it, to avoid confusion with the "CHECK-NOT:" FileCheck directive. You don't even need "CHECK" in fact, so you could just label them "NON-NUMERIC" and "NUMERIC" or similar.

This revision is now accepted and ready to land.Oct 29 2018, 9:48 AM
eush updated this revision to Diff 171656.Oct 30 2018, 3:33 AM

Renamed CHECK command names as suggested.

Great, thanks. Do you need me to commit it for you?

This revision was automatically updated to reflect the committed changes.