Sign in to follow this  
larspensjo

c++11 iteration and warnings

Recommended Posts

larspensjo    1561

I want to iterate through an object in c++11, and like the new syntax. If I have the following code:

for (auto x : object) {
    ...
}

 

I have a case where I don't want to use 'x'. That will produce a warning, as expected. How do I best suppress this warning?

 

I suppose I can add a line (void)x; but maybe there is some better way?

 

If there is no better way, can I be sure that the void reference to 'x' generates no code?

 

There is of course the old way of using iterators, but it looks much better with the new way of doing it. Or tell the compiler to ignore these warnings, which I do not want to do.

Share this post


Link to post
Share on other sites
Cornstalks    7030

There is of course the old way of using iterators, but it looks much better with the new way of doing it. Or tell the compiler to ignore these warnings, which I do not want to do.

I don't know why you're iterating without using the reference at all, but I'd do the following:

 

for (auto i = object.cbegin(); i != object.cend(); ++i) {
    ...
}

 

It shouldn't be any worse in performance than a range based for-each loop (I imagine it'd be exactly the same, with optimizations turned on).

 

As C++11 currently is, there's no way to do a range-based for loop without a range declaration.

Edited by Cornstalks

Share this post


Link to post
Share on other sites
Hiwas    5807

Err, I realize I missread the initial question...  (void)x; will do the trick and it generates no code, what could it possibly generate?  The downside is still in the case that not using x means you "are" going to have a copy taking place into "auto x" though as part of the loop.

 

using the new modified syntax, I'd go with the following:

 

for( auto ibegin = object.cbegin(), iend = object.cend(); ibegin!=iend; ++ibegin )
...

 

this avoids the useless copy construction of the 'x' value.

 

I want to iterate through an object in c++11, and like the new syntax. If I have the following code:

for (auto x : object) {
    ...
}

 

I have a case where I don't want to use 'x'. That will produce a warning, as expected. How do I best suppress this warning?

 

I suppose I can add a line (void)x; but maybe there is some better way?

 

If there is no better way, can I be sure that the void reference to 'x' generates no code?

 

There is of course the old way of using iterators, but it looks much better with the new way of doing it. Or tell the compiler to ignore these warnings, which I do not want to do.

Edited by AllEightUp

Share this post


Link to post
Share on other sites
ApochPiQ    23004
Hiding side effects behind iteration seems... gross to me.

It's more verbose, but I'd rather see your code look like this, just for sake of clarity:
struct MovementSystem : public System<MovementSystem> {
  void update(EntityManager &es, EventManager &events, double dt) override {
    for (auto& entity : es.entities_with_components<Position, Direction>()) {
      std::shared_ptr<Position> position(entity.get_component<Position>());
      std::shared_ptr<Direction> direction(entity.get_component<Direction>());

      position->x += direction->x * dt;
      position->y += direction->y * dt;
    }
  }
};
It took me several read-throughs of your code to figure out that it's equivalent to this. IMHO that's bad, because it means your system is not intuitive enough, or idiomatic in terms of how iteration typically works in C++.

It's also clearer that this code will optimize cleanly. Moreover, if you know you don't want to affect an object's lifetime during the update, you can opt for raw pointers instead of shared_ptr which gives you a little less overhead, assuming that that sort of micro-optimization is relevant.

That's just my opinion, though :-)

Share this post


Link to post
Share on other sites
Ravyne    14300

If you're not using x, its probably more efficient to iterate over an integer count with size of the number of objects.

Share this post


Link to post
Share on other sites

If there is no better way, can I be sure that the void reference to 'x' generates no code?

There is of course the old way of using iterators, but it looks much better with the new way of doing it. Or tell the compiler to ignore these warnings, which I do not want to do.

If you use "auto& x" or "const auto& x" as the loop variable, there are no copies.
And if I understand correctly, the standard guarantees (void)x; does nothing with x either:
http://stackoverflow.com/questions/689677/casting-unused-return-values-to-void

Share this post


Link to post
Share on other sites
Hiwas    5807

If there is no better way, can I be sure that the void reference to 'x' generates no code?

There is of course the old way of using iterators, but it looks much better with the new way of doing it. Or tell the compiler to ignore these warnings, which I do not want to do.

If you use "auto& x" or "const auto& x" as the loop variable, there are no copies.
And if I understand correctly, the standard guarantees (void)x; does nothing with x either:
http://stackoverflow.com/questions/689677/casting-unused-return-values-to-void

 

While all true, "auto& x" still may cause something to be done in the iteration which may not be needed/desired.  I.e. de-referencing the iterator in order to get the reference initialized. At this point you have to look at assembly to figure out if the compiler is doing what you intend (simply dropping the item and optimizing it away) or if it is actually doing unused work behind your back.  As this is a relatively new language feature, who knows if the compilers are really optimizing this specific case correctly, I don't trust compilers that much and prefer to be explicit.

Share this post


Link to post
Share on other sites

While all true, "auto& x" still may cause something to be done in the iteration which may not be needed/desired. I.e. de-referencing the iterator in order to get the reference initialized. At this point you have to look at assembly to figure out if the compiler is doing what you intend (simply dropping the item and optimizing it away) or if it is actually doing unused work behind your back. As this is a relatively new language feature, who knows if the compilers are really optimizing this specific case correctly, I don't trust compilers that much and prefer to be explicit.

I'd prefer to always write the code as nicely as the language allows and if it matters, verify the compiler is doing the right thing. That may be more of an idealist approach while yours is a more practical one. Considering that range-for and auto are probably the most-used features of the new standard, I think support will be pretty good. Lars' library isn't exactly shy of using C++11 to begin with.

Share this post


Link to post
Share on other sites
Bregma    9202

always write the code as nicely as the language allows

I would assert that using conventional constructs to invoke hidden secret indirect subtle Klever nonmanifest behaviour is allowed by the language but could in no charitable way be described as 'nice'.

Share this post


Link to post
Share on other sites
larspensjo    1561

It took me several read-throughs of your code to figure out that it's equivalent to this. IMHO that's bad, because it means your system is not intuitive enough, or idiomatic in terms of how iteration typically works in C++.

 

Thanks for the analysis, I can see you spent some effort doing it. And I agree your proposal is more readable and intuitive.

 

The link I used was to my fork, but it is not my system, though, just a library I am using.

Edited by larspensjo

Share this post


Link to post
Share on other sites
Ravyne    14300

Don't use auto in place of using regular type declarations, but other than that I don't think that the sky is falling quite so hard as Matias is worried about.

 

In general, use auto to:

  • avoid repeating yourself.
  • refer to an un-uterable type.
  • store something that really is dependent on the result of an expression.

You get yourself in trouble when you use auto to simply avoid thinking about types and their consequences.

 

Used appropriately, though, auto has worthwhile benefits, including a reduction of maintainance effort, simpifying template code, and reducing visual clutter.

Edited by Ravyne

Share this post


Link to post
Share on other sites
swiftcoder    18432

avoid repeating yourself.
refer to an un-uterable type.

Why not typedef it, like everyone else does? Same short declarations, no magic sauce to make reading difficult.

store something that really is dependent on the result of an expression.

 This is really what [tt]auto[/tt] is designed for. There are certain things that are very hard to efficiently compile without [tt]auto[/tt] (for example, [tt]boost::spirit[/tt] used to have awful trouble with recursive declarations).

Share this post


Link to post
Share on other sites
Ravyne    14300

avoid repeating yourself.
refer to an un-uterable type.

Why not typedef it, like everyone else does? Same short declarations, no magic sauce to make reading difficult.

 

typedefs don't avoid the repetition in the cases I'm speaking of, if the type changes, you've got to change it in multiple places. auto solves these cases. typedef is great where it works, which is most cases. I'm thinking of stuff like 'std::pair<int, int> p = std::make_pair..." -- not literal repetition, but just as useless. One could argue that this falls under the dependent type bucket, though.

 

And you can't typedef an un-uterable type, of course.

Share this post


Link to post
Share on other sites
swiftcoder    18432

I'm thinking of stuff like 'std::pair<int, int> p = std::make_pair..." -- not literal repetition, but just as useless.

But [tt]std::pair<int, int> p[/tt] is almost as bad as [tt]auto[/tt]. It conveys structure, sure, but it conveys nothing in the way of semantics. An explicit [tt]typedef std::pair<int, int> Point[/tt] is infinitely preferable to either (though a [tt]struct Point {int x, int y;}[/tt] would be better still).

And while I'm sure that was merely a contrived example, I think the same holds for many common uses of [tt]auto[/tt].

Share this post


Link to post
Share on other sites
Hiwas    5807

While all true, "auto& x" still may cause something to be done in the iteration which may not be needed/desired. I.e. de-referencing the iterator in order to get the reference initialized. At this point you have to look at assembly to figure out if the compiler is doing what you intend (simply dropping the item and optimizing it away) or if it is actually doing unused work behind your back. As this is a relatively new language feature, who knows if the compilers are really optimizing this specific case correctly, I don't trust compilers that much and prefer to be explicit.

I'd prefer to always write the code as nicely as the language allows and if it matters, verify the compiler is doing the right thing. That may be more of an idealist approach while yours is a more practical one. Considering that range-for and auto are probably the most-used features of the new standard, I think support will be pretty good. Lars' library isn't exactly shy of using C++11 to begin with.

But the given item is "not" writing the most clean code in this case.  The new "foreach" functionality is not intended for this usage, hence compilers bitch if you use it this way.  I'm a clean code (I think of it as self documenting) purist in terms that I always try to write the most descriptive code possible without writing it dumb just to make it readable.  My code is "usually" straight forward and where it gets complicated I add a couple comments.  Using a syntactic sugar item such as the new for( : ) variation when you don't intend to use the actual form it implies, isn't clean, that's at odds with clean.

 

I'm sorry but I don't see how else you can argue this.  You are using something for something is it not intended for, end of subject?

Share this post


Link to post
Share on other sites
larspensjo    1561

The presence of 'auto' is becoming wide spread and is a really really bad practice.

This is a most interesting topic, and I think we are going to see all kinds of practices of this (when to use and when not to use).

 

There is the recommendation not to repeat yourself, but then there is the problem for the reader of the source code to understand what is going on.

 

I used to favor the second view (and explictely use the type). But then I programmed Go a lot, and there is a similar mechanism. Assignments can be done as usual with "int a = f()" just as in C/C++, but there is also the syntax "a := f()" where you don't have to declare the type of a. This is used consequently everywhere, and it is recognized as a strength of Go (while still having static type testing by the compiler).

 

So my personal opinion is that I like to use "auto" for all but the primitive types. And I want the IDE to help me see what the type is.

 

Edit: spelling and wording.

Edited by larspensjo

Share this post


Link to post
Share on other sites
blutzeit    1650

The presence of 'auto' is becoming wide spread and is a really really bad practice.  [...]
 
In regular code, there is no benefit in using it (other than lazyness) and inherits all the problems from weakly typed languages: [...]

 

I used to program a lot of C# and was of the same opinion regarding its auto-equivalent, the "var" keyword. Then I played around a bit with F# which completely changed my mind. F# is a strongly typed language but encourages you to not specify any types anywhere. It makes you have to write readable code without the help of type "tags" sprinkled thru out the code. It's a very good exercise. Going back to C# a completely refactored the codebase I worked with to use "var" for all variables except the "built in" types (in C# int, float, decimal, string are language specific aliases to System.Int32 etc). The code got shorter and much more readable.

 

Now, the type system in C++ is much more complex than C#, so I'm not advocating the same principle here. But I still think that liberal but purposeful use of "auto", when done right in the right context, can be a good coding style.

Share this post


Link to post
Share on other sites
Hiwas    5807

The presence of 'auto' is becoming wide spread and is a really really bad practice.

This is a most interesting topic, and I think we are going to see all kinds of practices of this (when to use and when not to use).

 

There is the recommendation not to repeat yourself, but then there is the problem for the reader of the source code to understand what is going on.

 

I used to favor the second view (and explictely use the type). But then I programmed Go a lot, and there is a similar mechanism. Assignments can be done as usual with "int a = f()" just as in C/C++, but there is also the syntax "a := f()" where you don't have to declare the type of a. This is used consequently everywhere, and it is recognized as a strength of Go (while still having static type testing by the compiler).

 

So my personal opinion is that I like to use "auto" for all but the primitive types. And I want the IDE to help me see what the type is.

 

Edit: spelling and wording.

 

I tend to agree that this is going to be interesting and I'm sure we'll see a new Herb book about 10-20 ways to write better C++ in the next year related to all the fun things Cxx11 does to the language.

 

In the case of auto though, I agree overuse could be a problem but in general I don't agree with typedef everything.  In code I was working on tonight I had something like the following:

 

std::vector< Item* > removals;
... do a loop adding to "removals" those items which need to be removed after iteration processing ...

 

In no other case do I need a vector of that type, it is purely local and very descriptive between what it is defined as and the name of the local it is stored in.  Hmm, what could that mean other than, these are things to be removed..  So the end of the function is:

 

for( auto it : removals )
  something.erase( it ); // It's a set, so no find required.

 

I find this to be both extremely readable and easily understood by others.  Yes, it is a simple case, but the point is can you think of many ways to interpret a well named (ok, descriptively named, "well" named is questionable) variable with a locally given type that could be used in in a different context?  If you can use it in a different context, does it still apply to the overall descriptive naming?

 

In the same code, I have a type "mPairs" which is a "PairMap_t", hopefully you already know what the typedef is.. :)  So, using auto in this case:

 

auto it = mPairs.find( Pair( lhs, rhs ) );

 

Is still pretty readable?  "Pair" in this case is a local class used to contain a bit more than the "lhs/rhs" items but the intention is still clear, find "that specific one" and give me an iterator to it.  I "could" add the following: "typedef PairMap_t const_iterator PairConstIterator_t" or some such but what good does that do for readability?  It doesn't do anything useful except waste typing really, if you bothered to look at the definition of "mPairs", you know exactly what 'it' is defined as.  Seems pretty clean and descriptive to me.

Share this post


Link to post
Share on other sites
wintertime    4108

What I find weird about the new range for:

- There is the new cbegin and cend you should use on read access as to avoid the container preparing for a write, like for example a string making a private copy of the shared internal buffer.

- The range for seems to always use begin and end, but never cbegin or cend?

Edited by wintertime

Share this post


Link to post
Share on other sites
EWClay    659

What I find weird about the new range for:
- There is the new cbegin and cend you should use on read access as to avoid the container preparing for a write, like for example a string making a private copy of the shared internal buffer.
- The range for seems to always use begin and end, but never cbegin or cend?


cbegin and cend are used to ensure that const iterators are obtained even if the container is not const. It isn't because non-const begin might do extra work; in fact string cannot use copy-on-write anymore in C++11. With range for, the iterator is hidden but you can declare the element const, which is sufficient.

Share this post


Link to post
Share on other sites
Ravyne    14300

I'm thinking of stuff like 'std::pair<int, int> p = std::make_pair..." -- not literal repetition, but just as useless.

But [tt]std::pair<int, int> p[/tt] is almost as bad as [tt]auto[/tt]. It conveys structure, sure, but it conveys nothing in the way of semantics. An explicit [tt]typedef std::pair<int, int> Point[/tt] is infinitely preferable to either (though a [tt]struct Point {int x, int y;}[/tt] would be better still).

And while I'm sure that was merely a contrived example, I think the same holds for many common uses of [tt]auto[/tt].

 

I agree that a class, struct, or typedef is going to be available in many cases, but the semantics of my example would change along with it. Imagine instead:

 

Point p = Player.GetPos();

 

Is it terrible to have to write this? No. Is using auto instead going to lose tons of semantic value? No again, I argue. Each approach has its pros and cons, they just have to be weighed out.

 

Other times you find yourself needing "throw-away" or "temporal" types -- types that exist maybe in one place, and so don't need to be shared around. You can typedef those too, but I don't think that usually buys any more semantic understanding either. Most times that you use an iterator is a good example of this, lambdas are another. In functional languages, the pattern is to create these kinds of temporal types all the time wherever they're needed, and you hardly even think about it that way.

 

I do agree though, that over the next probably 2-5 years we'll see some real guidance about when its best to use auto or not. Right now we're at a level of "this is what they're designed for, this is what they can do.", which doesn't always turn out to be the same thing as "this is what you should do."

Share this post


Link to post
Share on other sites
sam_hughes    132

Other times you find yourself needing "throw-away" or "temporal" types -- types that exist maybe in one place, and so don't need to be shared around. You can typedef those too, but I don't think that usually buys any more semantic understanding either. Most times that you use an iterator is a good example of this, lambdas are another. In functional languages, the pattern is to create these kinds of temporal types all the time wherever they're needed, and you hardly even think about it that way.

 

You don't create "temporal" types in functional languages.  What are you talking about?

Edited by sam_hughes

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this