Reflection: use std::strtol instead of std::atoi
#3129
Conversation
|
We don't care about hex and octal digits here, the code above makes sure the digits are only 0...9. This code is parsing mangled names. |
|
Thats what I thought to (which is why I opted to use atoi). @harlanhaskins asked for this change though. |
|
I suggested it before seeing the validation beforehand. I suppose atoi is alright, but the lack of failure reporting in atoi raises a red flag |
|
@harlanhaskins okay, so whats the plan on this? Are we happy with the code as is and this can be abandoned or do we want to pursue some changes here? |
|
I'm hesitantly okay with this now. Sorry for stirring the pot. |
|
I see the benefit in using |
Address post-commit review comments from Harlan. Use `std::strtol` as it can deal with hex and octal inputs.
compnerd
force-pushed
the
integer-parsing
branch
from
40fbe69 to
f29e614
Compare
|
@gribozavr updated with the error handling, but Im really not sure how much it buys us. I did also remove an allocation, which can be nice, but, really, Im not convinced that it really is an improvement. Note that it previously didnt have any error handling either. |
|
@swift-ci Please test and merge |
What's in this pull request?
Resolved bug number: (SR-)
Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
Validation Testing
Lint Testing
Note: Only members of the Apple organization can trigger swift-ci.
Address post-commit review comments from Harlan. Use
std::strtolas it candeal with hex and octal inputs.