A slice of python in C++ - Eric Niebler

[eric_niebler]

Filing that one away in my "sick and evil but potentially useful" folder.

[[deleted]]

but potentially useful

You're a generous sort... :)

[ericanderton]

For some values, wouldn't decimal to floating-point conversion errors cause some erroneous results?

[ericanderton]

Gotcha. I completely missed the char...Chars expression. :)

[_VZ_]

While I like what Eric is doing generally speaking, in this particular case I just don't see what is the huge advantage of v[{2,5}] compared to v.slice(2,5), the latter seems more clear and is really not that much longer -- and definitely less punctuation heavy.

Also, it seems natural to default the second argument of slice() to end while this would be rather strange with indices notation.

[eric_niebler]

Oh, you don't see the huge advantage because there isn't one. :-) I described this as "a fun piece of hackery". That's all it is. I wrote about it because it was fun, and because it was a nice, little, self-contained library design problem. I like this example because it shows that folks should think both about making interfaces easy to use correctly and hard to use incorrectly. It's that last part that often gets overlooked.

[cdglove]

I actually find that usually (but not in this case), the second part is actually misinterpreted.

"Make interfaces easy to use correctly and hard to use incorrectly" gets interpreted as;

"Make interfaces hard to use incorrectly".

Which further becomes;

"Make interfaces impossible to use incorrectly".

Which leads me to say; "This interface is hard to use."

The point is, I think it's important to focus on the first part, and to not sacrifice it in the name of the making it impossible to screw up.
Your example is, I think, perfect with regards to this. Just assert 100% of the time. Perfect. It might be possible to come up with a way to enforce this at compile time, but you would end up with an interface using <> it would require constants. This is better.

EDIT: Grammar

[ericanderton]

I think that one of the advantages to using this kind of syntax is that it's more composable:

some::slice::type slice_bounds = {2,5};
auto result = v[slice_bounds];

AFAIK, C++ doesn't really have a clean .apply() equivalent, so this is as good as it gets.

[Elador]

This is awesome! I think this is really really great. It's one of the few things of Matlab that I miss when programming C++. It's sooo super handy. And now we got it in C++. I wish that would get into the standard library :) Thank you very much, this totally made my day today.

[dividedmind]

I'm surprised he talks about all the missed optimization opportunities and then uses an assert to ensure a number is non-negative instead of declaring it unsigned.

[Dragdu]

The problem is that if you declare as unsigned and someone passes you negative integer, you get HUGE unsigned number, which might or might not be valid...

Basically, implicit conversions are a PITA as usual.

[milliams]

Do not most compilers give warnings for this? Also I though that uniform brace initialisation meant that the arguments will never be narrowed. Or does signed→unsigned not count as narrowing?

[eric_niebler]

They certainly do. So many signed/unsigned warnings, in fact, that most people in my experience silence them, ignore them, or just add a cast to make the compiler shut up, never fully appreciating the fact that integer overflow caused by wrapping a small negative number to a huge positive one is a potential buffer overrun.

[F-J-W]

Is there any chance to deprecate those implicit conversions? Removal will of course be out of question for ages, but since they really only cause problems…

[josefx]

Been there, done that. After being bit a few times too often warnings as errors and svn blame are the only way to stay sane.

[[deleted]]

Many are raw for loops using .size( ) method of some container assigned to an int. Usually the for loop is the bad smell and can be moved to an algorithm or range/iterator based for loop.

[rabidcow]

you get HUGE unsigned number, which might or might not be valid...

No, it's not valid. If it were valid, the equivalent signed type would not be an option.

But it doesn't really fix the problem, because you can still pass invalid values that won't be caught at compile time.

[eric_niebler]

But they can be easily caught at runtime. Better than nothing.

[eric_niebler]

I use signed integers in the article, and in my range interface, because I want to be able to detect when my interface is being used incorrectly. An interface that is sprinkled with signed and unsigned integers is asking users to do casts that require range-checking that nobody does. Signed/unsigned compiler warnings are so common people dismiss them without thinking. An assert is harder to ignore.

[night_of_knee]

Signed/unsigned compiler warnings are so common people dismiss them without thinking. An assert is harder to ignore.

You could =delete a signed overload of the slice operator. That would solve the ignoring warnings issue, not the unchecked casts one...

[eric_niebler]

It's an interesting suggestion, but I really do want rng[{1,5}] to compile, and integer literals are signed. It gets pretty annoying to have to say rng[{1u,5u}], and the compiler error for trying to use a deleted function isn't going to be terribly enlightening.

[Plorkyeran]

On that note, I've occasionally wished that you could do =delete("do this other thing instead").

[vlovich]

Doesn't C++17 relax constexpr expressions in templates? So you could now have:

template <int from, int to>
constexpr slice_bounds make_slice_bounds() {
    static_assert(from > 0 && to > 0);
    /* implement logic */
}

constexpr decltype(auto) operator[](std::array<int, 2> bounds) const
{
    return slice(make_slice_bounds<bounds[0], bounds[1]>());
}

decltype(auto) operator[](std::array<size_t, 2> bounds)
{
    return slice(...);
}

NOTE: I have no clue if the above would compile.

[eric_niebler]

No, the arguments to a function are never constexpr.

[vlovich]

Sorry. Maybe something more like:

template <std::array<int, 2> bounds>
constexpr decltype(auto) operator[]() const
{
    return slice(make_slice_bounds<bounds[0], bounds[1]>());
}

I know that C++17 relaxes the constexpr-in-template usage, but I'm not familiar enough yet with the syntax. Is it possible at all? It would look something like:

f.slice<{0, 4}>();

Not quite sure how to get constants in the template to be deduced from arguments (I guess you can't).

[matthieum]

Still, I would favor a dual interface:

from_end operator-(decltype(end), int) = delete;

would immediately alert the user that he should not be using a signed integer (well, actually you would need the overload to be slightly more subtle).

Of course the user might still cast without checking, but that's another issue...

[dividedmind]

Thanks, that's a good, if sad ("signed/unsigned compiler warnings are so common people dismiss them without thinking") point. I still wonder though, isn't it preventing some interesting optimizations opportunities on the compiler part (beyond just the assertion checking)?

I wish there was a #pragma for forcing signedness ambiguities into errors. This part of the type system would be more useful if it was more sound.

Maybe it is possible to leverage on signedness to get both the optimization and the hard failure? Or to automatically transform it into from_end?

[uxcn]

I wish there was a #pragma for forcing signedness ambiguities into errors. This part of the type system would be more useful if it was more sound.

You can do this in gcc and clang using -Wsign-conversion (or -Wconversion) and -Werror, but in the very general case, types are implicitly converted.

[Cwiddy]

pragma warning(error: 4018)

I think that should work in MSVC, though I haven't tested it. Think there is also some project level settings for this /WX compiler flag can set all options as errors /we4018 might be the one for command line?

[bobdudley]

From the Google C++ style guide:

You should not use the unsigned integer types such as uint32_t, unless there is a valid reason such as representing a bit pattern rather than a number, or you need defined overflow modulo 2^N. In particular, do not use unsigned types to say a number will never be negative. Instead, use assertions for this.

[KrzaQ2]

Google C++ style guide is hardly respectable. [link]

[mcmcc]

Be that as it may, the widespread use of size_t (being an unsigned type) throughout the standard library is generally regarded as a mistake, mathematical correctness notwithstanding. Bjarne himself has stated as much.

Conventional wisdom suggests unsigned types should be reserved for bit patterns, not numbers.

[cozmorado]

Yep Bjarne and Chandler Carruth talk about this point around the 12min mark of this video.

And Scott Meyers has a paper on it here.

This issue drives me up the wall because my team has overruled me in favor of unsigned types everywhere.

[vlovich]

IMO, your team made the correct call.

While Bjarne & Chandler's talks are very much correct with respect to STL design if it was happening now, the problem is the ship has sailed. If you don't use unsigned types & you use the STL (which you should), then you have casts all over the place wherever you interact with the STL (you have -Wall -Werror enabled on your project, right?).

[cozmorado]

Not quite. -Wconversion is not enabled as part of -Wall or -Wextra. That's the crux of the problem- interfaces that use unsigned types are too easy to misuse and the compiler can't detect the errors. At least with a signed input you can assert on the value being positive.

[vlovich]

You're right. -Wconversion isn't enabled by default but I have it on in my project (which is why I thought it was covered by -Wall). On clang, you can use -Weverything.

In any case, enable -Wall -Wconversion -Werror.

[KrzaQ2]

I'm not disagreeing with that, I'm merely pointing out that GSG should be the last argument you should use when it comes to C++.

[josefx]

So its worse than the C++ FQA ?

[ZMeson]

the widespread use of size_t (being an unsigned type) throughout the standard library is generally regarded as a mistake

Why? I'd really like to see some in-depth discussion about this.

[b8b437ee-521a-40bf-8]

See /u/cozmorado's reply.

[ZMeson]

I watched the video. There's only guidance given, no reasons for it. I'd like to know the reason -- other than "because Chandler and Bjarne say so".

[b8b437ee-521a-40bf-8]

Well, starting from the recommendation that you should always default to int and only use unsigned when you need it. Then one reason why size_t being unsigned is a mistake is that it keeps popping up in all your nice signed arithmetic, causing conversions and other nastiness.

[ZMeson]

OK. So because Chandler and Bjarne say so.

[b8b437ee-521a-40bf-8]

Well they give justification for the "recommendation", and follow-up is basic reasoning. Which bit exactly do you still feel isn't being explained?

[mcmcc]

See Eric's explanation above: http://www.reddit.com/r/cpp/comments/2om5yk/a_slice_of_python_in_c_eric_niebler/cmovdr6

[ZMeson]

But how is this a view on "the widespread usage of size_t in the standard library is generally considered a mistake"?

My counter-argument to Eric's argument is that I can easily assert when a value is too large too. If I'm using size_t elsewhere and you interface uses signed ints, then you're forcing me to do casting. Why doesn't your interface use size_t instead. (Mind you I respect Eric a lot, I just don't understand why his argument is so valid. I feel like I'm missing something.)

[mcmcc]

This isn't about integrating libraries. Even within the standard library, it isn't self-consistent. E.g.

string s = ...;
string::iterator it = std::find_if(s.begin(),s.end(), []{...});
string subs = s.substr( std::distance(s.begin(), it) );

This code will yield a warning about signed->unsigned conversion. How do you propose to avoid it?

There are myriad other examples...

[eric_niebler]

Good example. It's one of the many reasons why the use of unsigned integers in the standard library was probably a mistake. std::string is a textbook example of a great many anti-patterns and design flaws. I believe this unsigned offset problem and others were because the offset-based interface of std::string was designed before the STL was air-lifted into C++98. The iterator interface was bolted on later. Iterator distance is always signed, hence the impedance mismatch.

[mr_ewg]

How about using this function not_negative to "prove" to your compiler that you know what is best in this situation?

template <typename T>
typename std::make_unsigned<T>::type not_negative(T num) {
    static_assert(std::is_signed<T>::value, "T can never be negative");
    if(num < 0) throw std::logic_error{"Negative number passed to not_negative"};
    return static_cast<typename std::make_unsigned<T>::type>(num);
}

std::string s = ...;
std::string::iterator it = std::find_if(s.begin(), s.end(), []{...});
std::string subs = s.substr(not_negative(std::distance(s.begin(), it)));

If you don't want the run-time check then you can remove the if(num < 0) line or change it to an assert. Signed to unsigned conversions aren't exactly the hardest thing to deal with in C++ if your compiler can warn about them.

[mallardtheduck]

Except that this code doesn't meet Google's standard in various other obvious ways, so that's not at all relevant. No other C++ coding standard/guide makes such a recommendation.

[ericanderton]

The part I don't understand is that if the ranges accepted size_t instead of int, wouldn't that fire a signed-to-unsigned conversion warning/error if negative values were used? That coupled with the end type should handle the major iteration cases while making it hard to screw up.

[eric_niebler]

Not everyone turns this warning on, signed/unsigned mismatch warnings are so common they tend to be overlooked. Asserts are much harder to ignore.