search

REGoth-project / REGoth

OpenSource-Reimplementation of the zEngine, used by the game "Gothic"
GNU General Public License v3.0
630 stars 63 forks source link

Clang tidy #362

Open ghost opened 5 years ago

ghost commented 5 years ago

Small cleanup problems found by clang-tidy. ;)

ataulien commented 5 years ago

About time this was being done. Thanks!

I checked all replacements and found one instance of where a comment said that iteratiors should not be used there (see in-line comment). Otherwise they should be all good.

ghost commented 5 years ago

Should be ok now. ;)

mdrost commented 5 years ago

Removing empty destructors from cpp files and using default in h files could increase executable size because of inlining and that could cause performance drop because of instruction cache miss.

ghost commented 5 years ago

Because struct A {}; should give almost the same result as struct A{ ~A() = default;}; it would be weird if "default" approach (without defining dtor) gives nasty results.

Furthermore inlining is just clue for compiler, and if compiler generates nasty code, I would say it's a bug.

frabert commented 5 years ago

I'd tend to agree with @ShFil119, there is no guarantee that the compiler is going to inline anything here; in fact, it may well decide that if the ctor/dtor is used often around the program it is not a good fit for inline. On the contrary, it might improve performance in the case where a ctor/dtor is used sparingly (once or twice) around the codebase and it could not have been inlined before because the source was not available.