widelands-dev team mailing list archive

[Klaus Halfmann <klaus.halfmann@xxxxxxxx>]

Review: Needs Information codereview, compile

I now get some (new?) clang warning, see inline comments.
Shall I commit the change?

Diff comments:

> === modified file 'src/logic/queue_cmd_factory.cc'
> --- src/logic/queue_cmd_factory.cc	2016-01-18 05:12:51 +0000
> +++ src/logic/queue_cmd_factory.cc	2016-05-01 10:05:57 +0000
> @@ -112,7 +112,7 @@
>  	case QueueCommandTypes::kReplaySyncRead:
>  	case QueueCommandTypes::kReplayEnd:
>  	case QueueCommandTypes::kNone:
> -		throw wexception("Unknown Queue_Cmd_Id in file: %u", id);
> +		throw wexception("Unknown Queue_Cmd_Id in file: %u", static_cast<unsigned int>(id));
>  	}

Mhh, there only really good way to ourput an enum is some kind of toString() method,
But I do not know if C++ has some defaults here :-(. But this is an exceptional case :-)

>  	NEVER_HERE();
>  }
> 
> === modified file 'src/map_io/map_players_view_packet.cc'
> --- src/map_io/map_players_view_packet.cc	2016-02-16 10:27:23 +0000
> +++ src/map_io/map_players_view_packet.cc	2016-05-01 10:05:57 +0000
> @@ -991,12 +991,12 @@
>  						}
>  
>  						//  edges
> -						if (!bl_seen & (f_everseen | bl_everseen))
> +						if (!(bl_seen) & (f_everseen | bl_everseen))
>  							roads_file.unsigned_8(f_player_field.road_sw());
> -						if (!br_seen & (f_everseen | br_everseen))
> +						if (!(br_seen) & (f_everseen | br_everseen))
>  							roads_file.unsigned_8(f_player_field.road_se());
> -						if (!r_seen & (f_everseen |  r_everseen))
> -							roads_file.unsigned_8(f_player_field.road_e ());
> +						if (!(r_seen) & (f_everseen | r_everseen))
> +							roads_file.unsigned_8(f_player_field.road_e());

clang now complains:
src/map_io/map_players_view_packet.cc:994: Old C-Style cast. Change to static_cast<Type>(var) or similar!
src/map_io/map_players_view_packet.cc:996: Old C-Style cast. Change to static_cast<Type>(var) or similar!
src/map_io/map_players_view_packet.cc:998: Old C-Style cast. Change to static_cast<Type>(var) or similar!

Mhh, what does gcc complain about? As all these are boolean this code might be better with && and ||.
Shall I check in the change?

>  					}
>  
>  					//  geologic survey


-- 
https://code.launchpad.net/~widelands-dev/widelands/gcc531/+merge/293478
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/gcc531.