Understanding FOR()

[jamespetts]

Apologies for asking what is probably a fairly basic question, but I am trying to merge the latest changes from Standard into Experimental, and am having some trouble with the new FOR() iterators. The syntax is a little confusing - I have tried to copy the relevant syntax and make adjustments where necessary, but it would help to understand conceptually how the FOR() syntax actually works - could anybody explain it? I should be very grateful.

One particularly confusing aspect is the relevance of the use of typedefs in hashtables for iterating over them. For example, what is the purpose of doing things this way:

Code Select Expand


typedef ptrhashtable_tpl<vehikel_besch_t const*, gui_image_list_t::image_data_t*> vehicle_image_map;
vehicle_image_map vehicle_map;

and

Code Select Expand


FOR(vehicle_image_map, const& i, vehicle_map) {
vehikel_besch_t const* const    info = i.key;
gui_image_list_t::image_data_t& img  = *i.value;

?

Perhaps I am being dim, but I do not quite understand how a parameter passed to FOR can be a type rather than an actual variable/object. Also, I am not quite sure how properly to use this method when the last argument in FOR() is not an object, but a method call which returns a pointer to an object, as here (Experimental code):

Code Select Expand


// is there already one airport next to this town?
quickstone_hashtable_iterator_tpl<haltestelle_t, haltestelle_t::connexion*> iter(*start_connect_hub->get_connexions(0));
while(iter.next())
   {
        halthandle_t h = iter.get_current_key();
        if( h.is_bound() && h->get_station_type()&haltestelle_t::airstop  )
        {
             start_hub = h; break;
         }
   } 
}


I have attempted to implement it thus:

Code Select Expand


FOR(connexions_map, const& i, *end_connect_hub->get_connexions(0))
            {
                halthandle_t h = i.key;
                if( h.is_bound() && h->get_station_type()&haltestelle_t::airstop  )
                {
                    start_hub = h;
                    break;
                }
            }
        }

and in simhalt.h

Code Select Expand


typedef quickstone_hashtable_tpl<haltestelle_t, connexion*>* connexions_map;
    connexions_map get_connexions(uint8 c) { return connexions[c]; }

But I suspect that this is not correct.

Any pointers (no pun intended) would be most helpful!

[isidoro]

Quick answer without seeing the actual code:
FOR is probably a #define macro.  Arguments may be whatever because macro substitution is done before the compiler looks for the actual text to compile.

The author of this macro probably intends it as a shorthand for iteration through a collection.

Typedefs are also shorthands, so that you don't have to write ptrhashtable_tpl<vehikel_besch_t const*, gui_image_list_t::image_data_t*> each time you want to refer to this type and can write vehicle_image_map instead.  It would work without it, I guess.

To see how it works, see the FOR macro definition and do the substitution yourself, just as the compiler does, and see if the resulting code is correct.

By the way, I think the new C++11 standard has new idioms for this iterations, if I recall correctly.

[prissi]

FOR uses templates (the first type is mobed there), ytpedefs and macros. It relies that the classes in question have an STL compatible iterator. ptrhashtable is not used in standard, thus it has no iterator.

(By the way, for simutrans a hashtable of quickstones is not a good idea, much better would be a sorted array, i.e. add a < operator to quickstone and use insert_sorted/binary search for adding/retrieving elements from this. This should scale even better with very large sets.

C11 has, but I think we have to wait a little to have it available on most platforms. Even MSVC 2008 (my current system and still supported) is not able to do it.

[Dwachs]

Code Select Expand


// is there already one airport next to this town?
quickstone_hashtable_iterator_tpl<haltestelle_t, haltestelle_t::connexion*> iter(*start_connect_hub->get_connexions(0));
while(iter.next())
   {
        halthandle_t h = iter.get_current_key();
        if( h.is_bound() && h->get_station_type()&haltestelle_t::airstop  )
        {
             start_hub = h; break;
         }
   } 
}


As far as I understand, the FOR-replacement of this loop should look like

Code Select Expand


FOR(quickstone_hashtable_tpl<haltestelle_t, haltestelle_t::connexion*>, & iter, *start_connect_hub->get_connexions(0) ) {
        halthandle_t h = iter.get_current_key();
...


Can you post compile errors for conversions that failed?

[Ters]

The FOR-macro contains several nested for loops for some reasonn, but I think the code before FOR-replacement would be something like

Code Select Expand


for (quickstone_hashtable_iterator_tpl<haltestelle_t, haltestelle_t::connexion*>::iterator iter = start_connect_hub->get_connexions(0).begin(); iter != start_connect_hub->get_connexions(0).end(); ++iter) {
  halthandle_t h = iter->get_current_key();
  if( h.is_bound() && h->get_station_type()&haltestelle_t::airstop  )
  {
    start_hub = h; break;
  }
}

But as prissi writes, that will only be possible if quickstone_hashtable_iterator_tpl has STL type iterators. The code in the first post, and repeated in Dwachs' post, uses a Java-like iterator.

[jamespetts]

Thank you for your help - that is is much appreciated. However, I am still having some trouble with this. Consider the following code:

Code Select Expand


inthashtable_iterator_tpl<uint16, departure_data_t> iter(departures);
while(iter.next())
{
id = iter.get_current_key();
...


If I replace it with this:

Code Select Expand


FOR(inthashtable_tpl<uint16, departure_data_t>, const& iter, departures)
{
id = iter.key;
...


Intellisense will show an error on the word "FOR", stating "ERROR: expected a ')'". I can only use Intellisense rather than checking for actual compile errors, as I have not managed to expunge enough of the old type iterators to attempt to compile it yet. The difficulty is that, because FOR is a preprocessor macro, not an actual method, I do not get very meaningful errors from Intellisense or the compiler. Any input would be much appreciated.

[Tron]

Code Select Expand


FOR(connexions_map, const& i, *end_connect_hub->get_connexions(0))

and in simhalt.h

Code Select Expand


typedef quickstone_hashtable_tpl<haltestelle_t, connexion*>* connexions_map;
    connexions_map get_connexions(uint8 c) { return connexions[c]; }

But I suspect that this is not correct.

connexions_map is a pointer to some container, but the first argument of FOR() must be the type of the container, i.e. the typedef should leave out the * at the end. It should be left out anyway, because such a typedef is confusing. Furthe, maybe get_connexions() better should return a reference instead of a pointer.

The FOR-macro contains several nested for loops for some reasonn, [...]

It is to have a place to declare variables. The init statement part of for is the only place to start a scope and declare an arbitrary variable. The boolean flags are there to make continue; and break; do The Right Thing(TM), i.e. so from the user's perspective it looks like FOR() is really a single iteration statement.

Code Select Expand


FOR(inthashtable_tpl<uint16, departure_data_t>, const& iter, departures)
            {
               id = iter.key;
...


Intellisense will show an error on the word "FOR", stating "ERROR: expected a ')'".

For macro substitution it looks like there are for arguments, separated by the three (!) commas. Macro arguments cannot contain (not parenthesized) commas. That's part of the reason to use a typedef there (the other is that the names are really long).
If we could use auto (local type inference for variables from the type of their initialisers), then this argument could simply be removed. It would also save lots of tedious and sometimes error prone typing (pun somewhat intended) everywhere else. It is supported since GCC 4.4 and MSVC 10.

I can only use Intellisense rather than checking for actual compile errors, as I have not managed to expunge enough of the old type iterators to attempt to compile it yet.

Why do you not have both types of iterators to get a compilable state?

[Ters]

It is to have a place to declare variables. The init statement part of for is the only place to start a scope and declare an arbitrary variable. The boolean flags are there to make continue; and break; do The Right Thing(TM), i.e. so from the user's perspective it looks like FOR() is really a single iteration statement.

Doesn't C++ allow you to declare variables all over the place and create scopes that are just that?

[Tron]

Yes and no. You want a variable, whose scope is confined to the FOR-loop. So just declaring a variables is not good. Using just { ... } does not cut it either, because the FOR-macro could contain the { and not the }, so you would need a ENDFOR-macro to provide the }, which is not desirable. Alternatively you could pass the loop body as macro parameter, which is also troublesome, e.g. there must not be any not parenthesized commas in it as mentioned above. So the way to go is to declare the variable in a for-loop and ensure that it is only executed once.

[Ters]

I missed the fact that the braces were not part of the FOR macro. That explains it.

[jamespetts]

I had not had both iterators side by side because I had merged the code from Standard to provide the new types of iterators, which also removed the old types; I did not realise at the time that there would be such difficulty.

Am I right in understanding that the reason that the hashtable iterators do not work is that there is a comma in the type definition and the macro cannot cope with this? If so, the FOR system as currently implemented is completely unsuited to iterating through hashtables (apart from stringhashtables, where there is no need to define the type of the key).

I am not familiar with "auto", I am afraid - I should be most grateful if you could elaborate a little on how this might work. It would obviate the need entirely for the first parameter in the FOR loop - is that correct? Perhaps what might be helpful is to have an additional macro, called "AUTOFOR" that works with this system - does this make sense?

[prissi]

However in your case hashing ints is again not the best way of storing and retrieving. Again an indexed array (where you could retrieve stuff with binary search) will be faster for large data sets.

[isidoro]

[...]
If we could use auto (local type inference for variables from the type of their initialisers), then this argument could simply be removed. It would also save lots of tedious and sometimes error prone typing (pun somewhat intended) everywhere else. It is supported since GCC 4.4 and MSVC 10.
[...]

Very nice explanations, Tron.  Regarding auto, now is part of C++11 standard.  So it's supposed to be there to stay.

[Tron]

I had not had both iterators side by side because I had merged the code from Standard to provide the new types of iterators, which also removed the old types; I did not realise at the time that there would be such difficulty.

I explicitly made introducing FOR(), using it instead of some iterator and removing that iterator separate commits. So you can just use the point before an iterator is removed as merge point.

Am I right in understanding that the reason that the hashtable iterators do not work is that there is a comma in the type definition and the macro cannot cope with this? If so, the FOR system as currently implemented is completely unsuited to iterating through hashtables (apart from stringhashtables, where there is no need to define the type of the key).

As noted earlier (and you noticed, too), just use a typedef. It saves typing anyway.

I am not familiar with "auto", I am afraid - I should be most grateful if you could elaborate a little on how this might work. It would obviate the need entirely for the first parameter in the FOR loop - is that correct?

auto

Perhaps what might be helpful is to have an additional macro, called "AUTOFOR" that works with this system - does this make sense?

No. The only sensible way to go then is to remove the first parameter of FOR().

Regarding auto, now is part of C++11 standard.  So it's supposed to be there to stay.

There's a slight difference between something being standardized and being commonly implemented.

[jamespetts]

Ahh, now I understand what the typedefs were all about! Apologies for being dim.

Prissi - by indexed array here, do you just mean an ordinary array, declared

Code Select Expand


int foo[256]

?

If so, I do not see that that would work, since the number of elements in the hashtable are not fixed.

[prissi]

and array of a class which has the < operator defined.

The class has the entries with the hash (int) and the carrier. Then you can use the vector and insert_sorted. Retireving would be then just a bsearch. Even 1000 elements are less than 10 iterations away, whcih would be par with the 101 element hashes. At 8000 elements, it would be already 13 with binary search versus 40 with hashes and lists. (Not to mention the cache misses of list elements.)

[jamespetts]

and array of a class which has the < operator defined.

The class has the entries with the hash (int) and the carrier. Then you can use the vector and insert_sorted. Retireving would be then just a bsearch. Even 1000 elements are less than 10 iterations away, whcih would be par with the 101 element hashes. At 8000 elements, it would be already 13 with binary search versus 40 with hashes and lists. (Not to mention the cache misses of list elements.)

Hmm - do you mean a fixed sized array here? If so, how would one deal with the variability of the number of elements in the collection?

[isidoro]

[...]
There's a slight difference between something being standardized and being commonly implemented.

In this case, I think there isn't.  There are some parts of the standard not yet implemented, but I think they will be in a short time.  And even there were some implementations (concepts) that were kept outside...

[prissi]

Indeed there is. Not every OS updates everything at the same time. Some of them are lagging more than other. Otherwise simutrans would also not need its own routines for format string reordering.

[Tron]

In this case, I think there isn't.  There are some parts of the standard not yet implemented, but I think they will be in a short time.

You are wrong, just have a look at this list to see what is not going to be implemented in VC11. And even the supported stuff is only available on bleeding edge compilers. Look at GCC's list for comparison. Quite some stuff is only supported on GCC 4.6+.
auto might be feasible, if it is ok to restrict the set of compilers to GCC 4.4+ and VC10+.

[Tron]

Then you can use the vector and insert_sorted.

BTW, you pessimized several "add all vector elements, then sort them" (O(n ld n)) to "do insertion sort manually" (O(n^2)) recently.

Retireving would be then just a bsearch. Even 1000 elements are less than 10 iterations away, whcih would be par with the 101 element hashes. At 8000 elements, it would be already 13 with binary search versus 40 with hashes and lists. (Not to mention the cache misses of list elements.)

Having a hash map, which does rescale itself, when it gets fuller, would probably be the better choice.

[prissi]

OK, but the hastables this is based are not. Especial hashtable are not very much optimized in simutrans. Originally they were only for looking up strings, and were keyed by the first letter. Thus 101 Elements was ok then. Also such lookups are not very frequent in standard.

[isidoro]

You are wrong, just have a look at this list to see what is not going to be implemented in VC11.
[...]

I didn't know that the VC guys were that behind.  I don't use it any more, nor Windows...

So the score is:
         *************************************************
         * Free Software 10   -   Proprietary Software 1 *
         *************************************************

I don't think Microsoft will allow this for a long time...  Let's see.

[jamespetts]

OK, but the hastables this is based are not. Especial hashtable are not very much optimized in simutrans. Originally they were only for looking up strings, and were keyed by the first letter. Thus 101 Elements was ok then. Also such lookups are not very frequent in standard.

Hmm - might there be some benefit, then, to using STL hashtables, if there are such things?

[prissi]

@isidoro

There are much much more compilers around than just GCC and MSVC; current version exist also of portland group (http://www.pgroup.com/) which is only C99, and LLVM (http://clang.llvm.org/cxx_status.html) ... ANd at least the protland compiler produces fast code than GCC.

Thus you will need for any portable software some years.

[isidoro]

I understand...  It's a pity.  I really like the new stuff.

[jamespetts]

Hmm - I am having some trouble with a particularly complicated conversion, and I wonder whether anyone would be able to offer any suggestions. The original code, in simhalt.h and simhalt.cc, looked like this:

simhalt.h

Code Select Expand


        // Table of all direct connexions to this halt, with routing information.
// Array: one entry per goods type.
// Knightly : Change into an array of pointers to connexion hash tables
quickstone_hashtable_tpl<haltestelle_t, connexion*> **connexions;


simhalt.cc

Code Select Expand


int catg_index =  warenbauer_t::passagiere->get_catg_index();
quickstone_hashtable_iterator_tpl<haltestelle_t, connexion*> iter(*(connexions[catg_index]));
while(iter.next())
{
iter.get_current_value()->alternative_seats = 0;
}


Converting it to the following produces an error in intellisense:

simhalt.h

Code Select Expand


        // Table of all direct connexions to this halt, with routing information.
// Array: one entry per goods type.
// Knightly : Change into an array of pointers to connexion hash tables
quickstone_hashtable_tpl<haltestelle_t, connexion*> **connexions;
typedef quickstone_hashtable_tpl<haltestelle_t, connexion*> connexions_map;


Code Select Expand


FOR(connexions_map, const& iter, *(connexions[catg_index]))
{
iter.value.alternative_seats = 0;
}


The error is shown in the line, FOR(connexions_map, const& iter, *(connexions[catg_index])). It complains, "Error: no suitable conversion from 'quickstone_hashtable_tpl<haltestelle_t, connexion*>' to 'haltestelle_t::connexions_map'", which I do not understand, since, as far as I can see from the typedef directive, they are one and the same thing.

Any assistance would be most appreciated.

[jamespetts]

A further problem appears to be the use of the new keyword. The FOR() loops work with references. That means that if I declare:

Code Select Expand


inthashtable_tpl<uint16, departure_data_t> *departures;


I cannot use the FOR() syntax with "departures", because "departures" is a pointer, and FOR() requires a reference. If I change "departures" to a reference, however, thus:

Code Select Expand


/**
* Time in ticks since this convoy last departed from
* any given stop, plus accumulated distance since the last
* stop, indexed here by its handle ID.
* @author: jamespetts, August 2011. Replaces the original
* "last_departure_time" member.
* Modified October 2011 to include accumulated distance.
*/
typedef inthashtable_tpl<uint16, departure_data_t>& departure_map;
departure_map departures;


I then am unable to create a new one, as this line is no longer possible:

Code Select Expand


departures = new inthashtable_tpl<uint16, departure_data_t>;


Adding a * before the "new" I thought would help, but it appears not to help, the error being given that the "=" operator in inthashtable_tpl is inaccessible. Given that that operator would be a copy operator, it seems as if this would be doing the wrong thing even if "=" was accessible. Any tips here would be very much appreciated.

[Ters]

Can't you just pass *departures or (*departures) to the FOR() macro?

[Tron]

simhalt.h

Code Select Expand


        // Table of all direct connexions to this halt, with routing information.
   // Array: one entry per goods type.
   // Knightly : Change into an array of pointers to connexion hash tables
   quickstone_hashtable_tpl<haltestelle_t, connexion*> **connexions;
   typedef quickstone_hashtable_tpl<haltestelle_t, connexion*> connexions_map;


It does not make sense to declare a typedef after the first place, where it could be used, but it is not wrong.

Code Select Expand


FOR(connexions_map, const& iter, *(connexions[catg_index]))
   {
      iter.value.alternative_seats = 0;
   }


The error is shown in the line, FOR(connexions_map, const& iter, *(connexions[catg_index])). It complains, "Error: no suitable conversion from 'quickstone_hashtable_tpl<haltestelle_t, connexion*>' to 'haltestelle_t::connexions_map'", which I do not understand, since, as far as I can see from the typedef directive, they are one and the same thing.

This look right. It even works just fine (after I cherry picked three necessary commits to provide hashtable_t with STL-style iterators and fixed an unrelated error in Simutrans-exp). Does the compiler produce this error or just this intellinonsense?

A further problem appears to be the use of the new keyword. The FOR() loops work with references. That means that if I declare:

Code Select Expand


inthashtable_tpl<uint16, departure_data_t> *departures;


I cannot use the FOR() syntax with "departures", because "departures" is a pointer, and FOR() requires a reference. If I change "departures" to a reference, however, thus:

Huh? Why not? Just dereference the pointer. You even did this up there with connexions (even twice in a row).

Code Select Expand


   typedef inthashtable_tpl<uint16, departure_data_t>& departure_map;
   departure_map departures;


Why would you make a typedef to a reference? Why did you change the type of departures? Kill the & and re-add the * in the second line.

BTW, it does not make sense for departures and average_journey_times to be delegates instead of aggregations in the first place. There is even a memory leak in one of the constructors of convoi_t and the other has two rather pointless new-delete-new chains (the first new is in init()).

I failed to make heads or tails of the history in your repo. You do not seem to have a single systematic way to merge with normal Simutrans, but rather two unrelated ones. Further, it does not seem like you merged all changes of Simutrans, but left out many commits (and there are indicators of incorrectly resolved merge conflicts). On the other hand some changes seem to be brought in by copy&paste instead of merge or cherry-pick (for example utils/for.h). I can see why you have such a hard time syncing with normal Simutrans.
Though I had no big problems getting a compilable version where the containers mentioned above use FOR().

[jamespetts]

Tron,

thank you very much for your helpful reply, and apologies for not having reverted to you sooner - I am afraid that I have been rather preoccupied with other things lately!

As to the first point, yes, of course - I didn't spot that: I shall change it as suggested. I have in fact done this properly elsewhere.

As to the second point - it is just Intellisense, as I have not got far enough with the merge to try compiling yet, since there are still old style iterators in place.

As to the third point, I suspect that this is me being very dim about the niceties of C++ pointer dereferencing - apologies. My C++ knowledge is not as deep as many of those here as I am entirely self taught and have never programmed professionally. Simply dereferencing "departures" (i.e., "*departures") will work, then? I have a vague recollection that I tried this and got an error, although I cannot remember for sure now.

As to the fourth point - thank you for the tip: that is most helpful.

BTW, it does not make sense for departures and average_journey_times to be delegates instead of aggregations in the first place. There is even a memory leak in one of the constructors of convoi_t and the other has two rather pointless new-delete-new chains (the first new is in init()).

I am having some difficulty in understanding this fully, probably due to my abovementioned limited understanding of the niceties of programming. I did try to look up "delegates" and "aggregation", but the Wikipedia article suggested three meanings of "delegation", one of which was specific to C# and one of which was said to be also known as aggregation - I should be extremely grateful, therefore, if you could expand a little on what you mean here and how you would suggest that these things be done instead. As to the memory leak - may I ask how that arises? And which new-delete-new chains are pointless?

As to the history in the repository, apologies if I have not been as systematic as I might have been. Perhaps if I outline what system that I do use and you can let me know where I am going wrong?

I use Git for Windows for all Git related operations. What I normally do now (although it has not always been this way) is to have the "master" branch as always containing the latest release. I have a branch for the current major version (currently 10.x), to which I generally apply bug fixes or minor feature changes. Any major feature changes go into their own branches and are merged either into the current major version branch if I am planning to release a new version with significant new features without changing the major version number, or into a new major version branch (e.g., 11.x) if I am planning to release a new major version with a new major version number. Previously, I used to merge changes from Standard into the major version branch, but more recently, I have taken to merging them into a special merge branch (currently "111.1-merge", although this is intended to merge now up to 111.2.1) before merging that branch itself into the appropriate major version branch. Resolving merge conflicts with merges from Standard is very often a gargantuan and enormously difficult task, as all the merge conflicts have to be resolved manually by looking at the Standard merge histories and trying to deduce the intention of the relevant changes, and then trying to work out how to accommodate that intention in a way consistent with the way in which Experimental is intended to work. Sometimes, I purposely discard the changes in Standard (for example, if they are not relevant to Experimental, such as the recent Standard changes to the routing of goods - Experimental has its own completely different system here), and sometimes I make errors in interpreting either what is intended or how to implement the joint intention of the Standard change or the Experimental code (as with the city generation code in the merge branch, for which Richard Smith has produced a patch). I generally make sure that I have fixed the merge conflicts to a sufficient standard to allow me to compile the merge branch before committing, but on occasions (as with the current 111.1-merge branch), I have to commit part way through resolving merge conflicts as there are so many of them.

May I ask - what am I doing wrong in the above system, and what ought I be doing instead? (I do not think that I did copy and paste for.h, though: that was incorporated automatically in the merge, if I recall correctly. May I ask why you thought otherwise?) Also, if you have spotted any particular incorrectly resolved merge conflicts, I should be extremely grateful indeed if you could let me know what they are to avoid many hours of trying to track down merge related bugs in the future!

You write that you had no big problems getting a compilable version - may I ask - do you still have this code? If you do, I should be very grateful indeed if you could let me see it so that I can avoid duplication of work.

Apologies again for being dim and for not replying sooner, and thank you very much indeed for your help.

[Tron]

As to the third point, I suspect that this is me being very dim about the niceties of C++ pointer dereferencing - apologies. My C++ knowledge is not as deep as many of those here as I am entirely self taught and have never programmed professionally. Simply dereferencing "departures" (i.e., "*departures") will work, then? I have a vague recollection that I tried this and got an error, although I cannot remember for sure now.

This is basic C: if T* is the type of x, then T is the type of *x.

As to the memory leak - may I ask how that arises?

convoi_t::convoi_t(spielter_t*) first calls convoi_t::init(karte_t*, spieler_t*), which allocates average_journey_times and departures. Then it goes on and allocates two new objects overwriting the pointers set in init().

And which new-delete-new chains are pointless?

convoi_t::convoi_t() calls convoi_t::init(karte_t*, spieler_t*), which allocates average_journey_times and departures. Then deletes them and allocates two new objects. I.e. same as above, but without the memory leak.

As they are only allocated in the constructors and deleted only in the destructor, it makes no sense for them to be pointers.
Having them as member objects is simpler and would get rid of all the allocation problems.

(I do not think that I did copy and paste for.h, though: that was incorporated automatically in the merge, if I recall correctly. May I ask why you thought otherwise?)

Code Select Expand


| * | commit ffb14c24c5f594575d324439ee25accd30c0b460
| | | Author: James E. Petts <jamespetts@yahoo.com>
| | | Date:   2012-02-16 10:11:40 +0000
| | |
| | |     FIX: Add file now necessary for compiling.
| | |
| | | A utils/for.h
| | |   
| | * commit e4a2be114a47e2ebbfc462679331129abf4f7306
| |/  Author: Richard Smith <richard@ex-parrot.com>
| |   Date:   2012-02-16 09:41:56 +0000
| |   
| |       Missing file.
| |   
| |   A utils/for.h


Also, if you have spotted any particular incorrectly resolved merge conflicts, I should be extremely grateful indeed if you could let me know what they are to avoid many hours of trying to track down merge related bugs in the future!

Just compile with warnings. This shows some places, which look not entierly right.

You write that you had no big problems getting a compilable version - may I ask - do you still have this code? If you do, I should be very grateful indeed if you could let me see it so that I can avoid duplication of work.

For quick testing I just cherry picked r5314, r5315 and r5317.

[jamespetts]

Thank you for all of your help with this. I have made some progress this evening, but am still having trouble. I have pushed my latest work to the 111.1-merge branch. However, although I have now replaced all of the iterator_tpl type iterators with FOR type iterators, I am getting baffling and inexplicable compile errors. For example, I get the following output from simhalt.cc (truncated):

Code Select Expand


1>c:\users\james\documents\development\simutrans\simutrans-experimental-sources\utils/for.h(33): error C2825: 'T': must be a class or namespace when followed by '::'
1>          simhalt.cc(1056) : see reference to class template instantiation 'for_sel_iter<T,SEL>' being compiled
1>          with
1>          [
1>              T=haltestelle_t::waiting_time_map,
1>              SEL=void (const int &)
1>          ]
1>c:\users\james\documents\development\simutrans\simutrans-experimental-sources\utils/for.h(33): error C2039: 'const_iterator' : is not a member of '`global namespace''
1>c:\users\james\documents\development\simutrans\simutrans-experimental-sources\utils/for.h(33): error C2146: syntax error : missing ';' before identifier 'iter'
1>c:\users\james\documents\development\simutrans\simutrans-experimental-sources\utils/for.h(33): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
1>simhalt.cc(1056): error C2039: 'iter' : is not a member of 'for_sel_iter<T,SEL>'
1>          with
1>          [
1>              T=haltestelle_t::waiting_time_map,
1>              SEL=void (const int &)
1>          ]
1>simhalt.cc(1056): error C2065: 'iter' : undeclared identifier
1>simhalt.cc(1056): error C2146: syntax error : missing ';' before identifier 'iter__1056'
1>simhalt.cc(1056): error C2065: 'iter__1056' : undeclared identifier
1>simhalt.cc(1056): error C2065: 'end__1056' : undeclared identifier
1>simhalt.cc(1056): error C2065: 'iter__1056' : undeclared identifier
1>simhalt.cc(1056): error C2065: 'end__1056' : undeclared identifier
1>simhalt.cc(1056): error C2146: syntax error : missing ')' before identifier 'break__1056'
1>simhalt.cc(1056): error C2059: syntax error : ';'
1>simhalt.cc(1056): error C2065: 'iter__1056' : undeclared identifier
1>simhalt.cc(1056): error C2059: syntax error : ')'
1>simhalt.cc(1056): error C2143: syntax error : missing ';' before 'if'
1>simhalt.cc(1056): error C2065: 'break__1056' : undeclared identifier
1>simhalt.cc(1056): error C2039: 'iter' : is not a member of 'for_sel_iter<T,SEL>'
1>          with
1>          [
1>              T=haltestelle_t::waiting_time_map,
1>              SEL=void (const int &)
1>          ]
1>simhalt.cc(1056): error C2065: 'iter' : undeclared identifier
1>simhalt.cc(1056): error C2955: 'std::iterator_traits' : use of class template requires template argument list
1>          c:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\include\xutility(384) : see declaration of 'std::iterator_traits'
1>simhalt.cc(1056): error C2065: 'iter__1056' : undeclared identifier
1>simhalt.cc(1056): error C2065: 'break__1056' : undeclared identifier
1>simhalt.cc(1059): error C2228: left of '.value' must have class/struct/union
1>          type is 'const _Iter::value_type'
1>simhalt.cc(1059): error C2228: left of '.month' must have class/struct/union
1>simhalt.cc(1063): error C2228: left of '.value' must have class/struct/union
1>          type is 'const _Iter::value_type'


I also get some incomprehensible "undefined identifier" messages in cases where the identifier is clearly defined in a header file included in the source file. For example:

Code Select Expand


1>path_explorer.cc(1201): error C2065: 'connexions_map_single' : undeclared identifier
1>path_explorer.cc(1201): error C2955: 'for_sel_iter' : use of class template requires template argument list
1>          c:\users\james\documents\development\simutrans\simutrans-experimental-sources\utils/for.h(33) : see declaration of 'for_sel_iter'
1>path_explorer.cc(1201): error C2143: syntax error : missing ';' before 'const'
1>path_explorer.cc(1201): error C2143: syntax error : missing ';' before 'const'
1>path_explorer.cc(1201): error C2143: syntax error : missing ')' before 'const'
1>path_explorer.cc(1201): error C2143: syntax error : missing ';' before 'const'
1>path_explorer.cc(1201): error C2065: 'once1__1201' : undeclared identifier
1>path_explorer.cc(1201): error C2065: 'break__1201' : undeclared identifier
1>path_explorer.cc(1201): error C2065: 'once1__1201' : undeclared identifier
1>path_explorer.cc(1201): error C2059: syntax error : ')'
1>path_explorer.cc(1202): error C2143: syntax error : missing ';' before '{'
1>path_explorer.cc(1203): error C2065: 'connexions_iter' : undeclared identifier
1>path_explorer.cc(1203): error C2228: left of '.key' must have class/struct/union
1>          type is ''unknown-type''
1>path_explorer.cc(1211): error C2065: 'connexions_iter' : undeclared identifier
1>path_explorer.cc(1211): error C2228: left of '.value' must have class/struct/union
1>          type is ''unknown-type''
1>path_explorer.cc(1788): error C2065: 'connexions_map' : undeclared identifier
1>path_explorer.cc(1788): error C2955: 'for_sel_ref' : use of class template requires template argument list
1>          c:\users\james\documents\development\simutrans\simutrans-experimental-sources\utils/for.h(30) : see declaration of 'for_sel_ref'
1>path_explorer.cc(1788): error C2065: 'connexions_map' : undeclared identifier
1>path_explorer.cc(1788): error C2955: 'for_sel_iter' : use of class template requires template argument list
1>          c:\users\james\documents\development\simutrans\simutrans-experimental-sources\utils/for.h(33) : see declaration of 'for_sel_iter'
1>path_explorer.cc(1788): error C2228: left of '.begin' must have class/struct/union
1>          type is 'const T'
1>path_explorer.cc(1788): error C2228: left of '.end' must have class/struct/union
1>          type is 'const T'
1>path_explorer.cc(1788): error C2065: 'connexions_map' : undeclared identifier
1>path_explorer.cc(1788): error C2955: 'for_sel_iter' : use of class template requires template argument list
1>          c:\users\james\documents\development\simutrans\simutrans-experimental-sources\utils/for.h(33) : see declaration of 'for_sel_iter'
1>path_explorer.cc(1788): error C2143: syntax error : missing ';' before 'const'
1>path_explorer.cc(1788): error C2143: syntax error : missing ';' before 'const'
1>path_explorer.cc(1788): error C2143: syntax error : missing ')' before 'const'
1>path_explorer.cc(1788): error C2143: syntax error : missing ';' before 'const'
1>path_explorer.cc(1788): error C2065: 'once1__1788' : undeclared identifier
1>path_explorer.cc(1788): error C2065: 'break__1788' : undeclared identifier
1>path_explorer.cc(1788): error C2065: 'once1__1788' : undeclared identifier
1>path_explorer.cc(1788): error C2059: syntax error : ')'
1>path_explorer.cc(1789): error C2143: syntax error : missing ';' before '{'
1>path_explorer.cc(1790): error C2065: 'iter' : undeclared identifier
1>path_explorer.cc(1790): error C2228: left of '.value' must have class/struct/union
1>          type is ''unknown-type''


Any help in attempting to deal with these issues would be very much appreciated. I am rather at a loss as to why the "undefined identifier" error should appear here.

[Tron]

I am getting baffling and inexplicable compile errors. For example, I get the following output from simhalt.cc (truncated):

Code Select Expand


1>c:\users\james\documents\development\simutrans\simutrans-experimental-sources\utils/for.h(33): error C2825: 'T': must be a class or namespace when followed by '::'
1>          simhalt.cc(1056) : see reference to class template instantiation 'for_sel_iter<T,SEL>' being compiled
1>          with
1>          [
1>              T=haltestelle_t::waiting_time_map,
1>              SEL=void (const int &)
1>          ]


utils/for.h line 27

I also get some incomprehensible "undefined identifier" messages in cases where the identifier is clearly defined in a header file included in the source file. For example:

Code Select Expand


1>path_explorer.cc(1201): error C2065: 'connexions_map_single' : undeclared identifier


connexions_map_single is declared in class haltestelle_t, so the name needs to be qualified to access it from outside of haltestelle_t.
Also having the connexions_map_single at all makes little sense, given there is connexions_map already.

[jamespetts]

Thank you very much for your help here. I have eventually managed to get it to compile (see my 111.1-merge branch), but it is plagued with I am afraid seemingly inexplicable crashes of all kinds, some in the path explorer, others in totally random places such as the method for checking whether an electricity symbol should be displayed in the depot. I am rather perplexed and should be extraordinarily grateful for any assistance at all.