[PATCH] Fix 'leading zeros' caveat of StringRef::compare_numeric()

Sanne Wouda snnw at gruttepier.net
Mon Mar 30 06:26:55 PDT 2015


Thanks for the review.

I'm not addressing a specific problem here; merely going off on the
comment mentioning that prefixed zeroes didn't work very well.  I
gather that you'd argue against merging in this case, even if the code
were more readable. (I'd be happy to work on the readability, though.)

Regarding octal numbers: I believe behaviour is unchanged.  Comparison
between two octal numbers (with or without leading zeroes) should work
as expected.  Exactly which behaviour are you worried about?

On Mon, Mar 30, 2015 at 1:47 AM, Chandler Carruth <chandlerc at gmail.com> wrote:
> This adds lots of code and makes the code harder to read in order to handle this case.
>
> Is there a specific problem you're addressing here? Can you work to make the code more readable rather than less readable if we need to support this use case?
>
> Also, this ignores the common usage of a leading zero to indicate octal. I'm somewhat worried about StringRef having divergent behavior there.
>
>
> REPOSITORY
>   rL LLVM
>
> http://reviews.llvm.org/D7388
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>



More information about the llvm-commits mailing list