Fixes for warnings when compiling with GCC 9

[ceeac]

This patch fixes the following warnings (where possible) when compiling with GCC 9:

• Wparentheses
• Wimplicit-fallthrough
• Wdeprecated-copy

[Dwachs]

What is the reason to implement assignment operators?

[Ters]

Although I haven't tested, I assume it is because the lack of explicit definition give warnings with GCC 9. Since the classes have one or more constructors, generation of automatic assignment operators are deprecated in some version of the C++ standard, perhaps one that is default with GCC 9. Whether the assignment operators are actually used, is not something I know how to check without reading through the code.

It think it is possible to say that you actually do want the compiler to generate the default assignment operators, although that will require using a relatively new keyword. Since the copy constructor is hand-written, I think it is best to hand write the assignment operator as well.

[prissi]

The copy contructor is only there, because otherwise one could not convert a scr_coord into a scr_size.

Personally, I think that having the copy constructor doing something else than the assignment operator is a really bad idea that should rather generate lots of warnings. I.e. defining both should generate warnings ...

As such, I would rather not define the assignment operator. I seems rather dangerous to me.

[Ters]

Personally, I think that having the copy constructor doing something else than the assignment operator is a really bad idea that should rather generate lots of warnings. I.e. defining both should generate warnings ...

The fact that the standard is on its way to disabling automatic generation of copy assignment operators when copy constructors are defined, likely means that experience has shown that whenever custom copy constructors are around, the default copy assignment operator ends up doing something wrong. The programmer may mess up when having to define both manually, but then at least the language has forced the programmer to think through what copying such an object means, rather than lulling the programmer into thinking it will just work.

There is also the rule of three, which includes destructors as well.

The copy contructor is only there, because otherwise one could not convert a scr_coord into a scr_size.

Copy constructors do not convert types. C++ has a special class of functions that can be defined for converting between types. Unfortunately, is seems these have to be declared on the from type, which is some cases may cause dependencies that are backwards compared to what may feel natural.

[ceeac]

Update: Instead of defining assignment operators, I opted to remove the user-defined copy constructors since the compiler-generated ones already do the correct thing.

Although I haven't tested, I assume it is because the lack of explicit definition give warnings with GCC 9. Since the classes have one or more constructors, generation of automatic assignment operators are deprecated in some version of the C++ standard, perhaps one that is default with GCC 9. Whether the assignment operators are actually used, is not something I know how to check without reading through the code.

The behaviour is deprecated since C++11, however only GCC 9 warns about it by default. This is not specifc to the default C++ version GCC uses since GCC enables C++14 with GNU extensions by default since version 5 (I think) and also because Simutrans enables a pre-C++11 standard explicitly anyway.

[Ters]

Simutrans enables a pre-C++11 standard explicitly anyway

I can't see that it does. And if Simutrans is asking for pre-C++11, GCC should not give these warnings (according for the manual).

Otherwise, the patch looks fine. Unless there are same misbehaving compilers that don't generate default copy constructor if there are any custom constructors.

[prissi]

Someone added -Wextra to the Makefile at one point. Hence, there are such suspicious warning. The "-Wdeprecated-copy" has been apparently added becasue older GCC sometime generated broken code (silently). The new compiler fixes this but adds that warning. However, no other compiler complains, so I will just remove this warning from apprearringat all.