[GIT] Cann't compile simutrans exp.

[O01eg]

When I try I get error:

Code Select Expand


===> CXX simhalt.cc
In file included from boden/wege/weg.h:14:0,
                 from boden/grund.h:18,
                 from simplan.h:12,
                 from simworld.h:34,
                 from path_explorer.h:14,
                 from simhalt.cc:18:
boden/wege/../../besch/weg_besch.h: In member function 'uint32 weg_besch_t::get_max_axle_load() const':
boden/wege/../../besch/weg_besch.h:100:56: warning: comparison is always true due to limited range of data type [-Wtype-limits]
simhalt.cc: In static member function 'static halthandle_t haltestelle_t::get_halt(koord3d, const spieler_t*)':
simhalt.cc:289:144: warning: suggest parentheses around '&&' within '||' [-Wparentheses]
simhalt.cc: In member function 'void haltestelle_t::step()':
simhalt.cc:1188:46: warning: suggest parentheses around '&&' within '||' [-Wparentheses]
simhalt.cc:1211:19: warning: variable 'h' set but not used [-Wunused-but-set-variable]
simhalt.cc: In member function 'uint32 haltestelle_t::liefere_an(ware_t, uint8)':
simhalt.cc:2480:34: warning: suggest parentheses around '&&' within '||' [-Wparentheses]
simhalt.cc:2556:2: error: jump to label 'walking' [-fpermissive]
simhalt.cc:2502:9: error:   from here [-fpermissive]
simhalt.cc:2518:14: error:   crosses initialization of 'const bool destination_is_within_coverage'
simhalt.cc:2517:16: error:   crosses initialization of 'const uint16 straight_line_distance_destination'
simhalt.cc: At global scope:
simhalt.cc:2643:6: warning: unused parameter 'dummy' [-Wunused-parameter]
simhalt.cc: In member function 'void haltestelle_t::rdwr(loadsave_t*)':
simhalt.cc:3674:23: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
simhalt.cc:3681:23: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
simhalt.cc: In member function 'uint32 haltestelle_t::deposit_ware_at_destination(ware_t)':
simhalt.cc:2641:1: warning: control reaches end of non-void function [-Wreturn-type]
make: *** [build/default/simhalt.o] Error 1


commit: 0f9b2a08b4705ca229fedfc12dd7fd71766c2ded
branch: way-improvements

[jamespetts]

What compiler are you using?

[DrSuperGood]

It compiles with VS10 fine and that is old now.

That said the code is throwing far more warnings that it really should. Warnings should not be ignored and most people consider it bad practice to have code throwing any warnings at all.

I believe he is using G++ as a Google shows that error being commonly related to it.

This is an actual C++ error here, just it seems VS is very lenient. The C++ standard states (according to some post somewhere...)

It is possible to transfer into a block, but not in a way that bypasses declarations with initialization. A program that jumps from a point where a local variable with automatic storage duration is not in scope to a point where it is in scope is ill-formed unless the variable has POD type (3.9) and is declared without an initializer.

The problem is the two variables...

Code Select Expand


const uint16 straight_line_distance_destination = shortest_distance(get_init_pos(), ware.get_zielpos());
const bool destination_is_within_coverage = straight_line_distance_destination <= (welt->get_settings().get_station_coverage() / 2);

Are still in scope after jumping to "walking". This is not allowed as you have skipped their initializers. VS seems to ignore this error (you do not use them after all) but G++ is more strict and takes this as a full blown compile error.

The solution could be something as dumb as...

Code Select Expand


const uint16 straight_line_distance_destination;
const bool destination_is_within_coverage;
                straight_line_distance_destination = shortest_distance(get_init_pos(), ware.get_zielpos());
                destination_is_within_coverage = straight_line_distance_destination <= (welt->get_settings().get_station_coverage() / 2);

Which should compile in G++. This is because the variables are declared without an initializer so the jump will not violate the C++ specification. Hopefully it optimizes to the same results.

That said, jumping is generally considered very bad programming practice. Maybe it would be best to remove the jump completely? Not sure how one would do that in this case due to the spaghetti nature of what the function does without introducing procedural coupling. Maybe the extra procedural coupling would be worth it just to remove that jump. Since walking causes a return, maybe some sort of conditional test at a function termination stage would also work.

[jamespetts]

001eg - are you able manually to test whether this change allows it to compile in G++? That is not something that I can test easily myself in a Windows environment.

Dr. SuperGood - thank you very much for that analysis: that is most helpful.

[O01eg]

I use gcc-4.7.3

[O01eg]

With separate declaration and initialization it cause next error due `const`:

Code Select Expand


simhalt.cc: In member function 'uint32 haltestelle_t::liefere_an(ware_t, uint8)':
simhalt.cc:2480:34: warning: suggest parentheses around '&&' within '||' [-Wparentheses]
simhalt.cc:2517:16: error: uninitialized const 'straight_line_distance_destination' [-fpermissive]
simhalt.cc:2518:14: error: uninitialized const 'destination_is_within_coverage' [-fpermissive]
simhalt.cc:2519:92: error: assignment of read-only variable 'straight_line_distance_destination'
simhalt.cc:2520:122: error: assignment of read-only variable 'destination_is_within_coverage'

If I remove const all compiles.

[DrSuperGood]

With separate declaration and initialization it cause next error due `const`:

Yeh sorry, not too used to C++ programming.

Removing the const from the locals should have little to no impact on performance after optimization. They are mostly used for programmers (easier understanding) and compile time safety.

The other solution would be a rewrite of that method to remove the jump (recommended solution, jumps are generally considered bad practice nowadays).

[jamespetts]

Now fixed - thank you very much for the report.