Page MenuHomeWildfire Games

Fix Warnings of VS2015
ClosedPublic

Authored by Stan on Nov 22 2018, 10:39 PM.

Details

Summary

This patch fixes warnings when building the tests and atlas about a variable being redeclared, by getting rid of the macro

Test Plan

Test I didn't break anything.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Stan created this revision.Nov 22 2018, 10:39 PM
Vulcan added a subscriber: Vulcan.Nov 22 2018, 10:43 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Default...
Executing section Source...

source/tools/atlas/GameInterface/Handlers/ObjectHandlers.cpp
|  33| #include·"graphics/Terrain.h"
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character ({) when these macros are defined: ''.

source/tools/atlas/GameInterface/Handlers/ObjectHandlers.cpp
|  33| #include·"graphics/Terrain.h"
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character ({) when these macros are defined: 'MESSAGES_SKIP_STRUCTS'.

source/tools/atlas/GameInterface/Handlers/ObjectHandlers.cpp
|  33| #include·"graphics/Terrain.h"
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character ({) when these macros are defined: '_MSC_VER'.
Executing section JS...

Link to build: https://jenkins.wildfiregames.com/job/differential/794/

Stan planned changes to this revision.Nov 23 2018, 9:08 AM

I will see if I can fix those warnings in terrain.h

The problem isn't in macros, because we use scoped block to solve the redeclaration. Probably it was missed somewhere. If you still remove macros, you need to remove scoped blocks too.

Stan added a comment.EditedNov 23 2018, 9:52 AM

You can't remove braces see the comment I added : "scoping braces are important to indicate where an element ends. If you don't put them the tag won't be closed until the object's destructor is called, usually when it goes out of scope."

There was no way to update the macros to work because you couldn't know what users would name them. Also some of the macros were hacky.

Stan requested review of this revision.Nov 23 2018, 12:57 PM

Can't fix those warnings, because they have no reasons to be.

https://github.com/0ad/0ad/search?q=MESSAGES_SKIP_STRUCTS&unscoped_q=MESSAGES_SKIP_STRUCTS

Angen added a comment.Nov 23 2018, 2:34 PM

So far I can tell that warnings are solved. I did not try to run game with this patch yet.

Stan added a reviewer: Itms.Dec 28 2018, 1:43 PM
Angen added a comment.Jan 6 2019, 10:42 PM

Atlas loads, generates, saves maps -> ok
no error or crash in game starting different maps

Angen accepted this revision.Jan 7 2019, 9:12 PM
This revision is now accepted and ready to land.Jan 7 2019, 9:12 PM
Itms requested changes to this revision.Jan 12 2019, 12:37 AM

The patch looks good and it works for me too! However, I tested building Atlas and got the following warnings:

  • C4458 in AtlasObjectImpl.cpp line 292 and in AtlasUI/Object.cpp:547
  • C4456 in MapDialog.cpp:173 and in ScenarioEditor.cpp:742

I know that this patch fixes warnings in the non-Atlas-DLL projects, but since there are few warnings left, we could include them in here for completeness 🙂

This revision now requires changes to proceed.Jan 12 2019, 12:37 AM

@Itms can I commit this separately after fixing the headers ? The other diff is gonna be quite big.

Itms accepted this revision.Jan 12 2019, 1:48 PM

Okay, you're right. Go ahead and thanks for the patch!

This revision is now accepted and ready to land.Jan 12 2019, 1:48 PM
This revision was automatically updated to reflect the committed changes.
Owners added subscribers: Restricted Owners Package, Restricted Owners Package.Jan 12 2019, 5:26 PM