Page MenuHomeWildfire Games

Fix some warning for vs2015 in Atlas
ClosedPublic

Authored by Stan on Jan 12 2019, 1:27 PM.

Details

Reviewers
Angen
Commits
rP22161: Fixes
Summary

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

Still remaining

Severity Code Description File Line Column Suppression State
Warning LNK4221 This object file does not define any previously undefined public symbols, so it will not be used by any link operation that consumes this library D:\Dev\0-A-D\ps\trunk\build\workspaces\vc2015\ws2_32.lib(WS2_32.dll) 1 1
Warning C4459 declaration of 'iter_policy_t' hides global declaration (compiling source file ..\..\..\source\tools\atlas\AtlasObject\AtlasObjectJS.cpp) D:\Dev\0-A-D\ps\trunk\libraries\win32\boost\include\boost\spirit\home\classic\core\scanner\impl\skipper.ipp 101 1
Warning C4459 declaration of 'scanner_policies_t' hides global declaration (compiling source file ..\..\..\source\tools\atlas\AtlasObject\AtlasObjectJS.cpp) D:\Dev\0-A-D\ps\trunk\libraries\win32\boost\include\boost\spirit\home\classic\core\scanner\impl\skipper.ipp 102 1
Warning LNK4006 NULL_IMPORT_DESCRIPTOR already defined in winmm.lib(WINMM.dll); second definition ignored D:\Dev\0-A-D\ps\trunk\build\workspaces\vc2015\comctl32.lib(COMCTL32.dll) 1 1
Warning LNK4221 This object file does not define any previously undefined public symbols, so it will not be used by any link operation that consumes this library D:\Dev\0-A-D\ps\trunk\build\workspaces\vc2015\comctl32.lib(COMCTL32.dll) 1 1
Warning LNK4006
NULL_IMPORT_DESCRIPTOR already defined in winmm.lib(WINMM.dll); second definition ignored D:\Dev\0-A-D\ps\trunk\build\workspaces\vc2015\rpcrt4.lib(RPCRT4.dll) 1 1
Warning LNK4221 This object file does not define any previously undefined public symbols, so it will not be used by any link operation that consumes this library D:\Dev\0-A-D\ps\trunk\build\workspaces\vc2015\rpcrt4.lib(RPCRT4.dll) 1 1
Warning LNK4006 __NULL_IMPORT_DESCRIPTOR already defined in winmm.lib(WINMM.dll); second definition ignored D:\Dev\0-A-D\ps\trunk\build\workspaces\vc2015\ws2_32.lib(WS2_32.dll) 1 1

REFS: D1678

Test Plan

Test it doesn'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.Jan 12 2019, 1:27 PM
Vulcan added a subscriber: Vulcan.Jan 12 2019, 1:28 PM

Build failure - The Moirai have given mortals hearts that can endure.

Linter detected issues:
Executing section Source...

source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Object/Object.cpp
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2017"

source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Object/Object.h
|   1| /*·Copyright·(C)·2011·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2011"

source/tools/atlas/AtlasUI/ScenarioEditor/ScenarioEditor.cpp
|   1| /*·Copyright·(C)·2018·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2018"

source/tools/atlas/AtlasObject/AtlasObject.h
|   1| /*·Copyright·(C)·2015·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2015"

source/tools/atlas/AtlasObject/AtlasObjectImpl.cpp
|   1| /*·Copyright·(C)·2015·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2015"

source/tools/atlas/AtlasObject/AtlasObjectXML.cpp
|   1| /*·Copyright·(C)·2009·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2009"

source/tools/atlas/AtlasObject/AtlasObjectText.cpp
|   1| /*·Copyright·(C)·2009·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2009"

source/tools/atlas/AtlasObject/AtlasObjectImpl.h
|   1| /*·Copyright·(C)·2009·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2009"

source/tools/atlas/AtlasObject/AtlasObjectJS.cpp
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2017"
Executing section JS...
Executing section cli...

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

Angen added inline comments.Jan 12 2019, 1:46 PM
source/tools/atlas/AtlasObject/AtlasObjectImpl.cpp
81 ↗(On Diff #7316)

m_P, also years pls

Itms added a comment.Jan 12 2019, 1:56 PM

I think _p or m_p would be acceptable names since it's one letter.
This might conflict with D1395, which I should prioritize.
The other warnings shall be fixed by upgrading Boost and/or deleting old unneeded dlls.

Stan updated this revision to Diff 7318.Jan 12 2019, 5:17 PM

Made this little powershell script to build compile and extract stuff from wxwidgets for atlas.

This update should fix the build, the macro wasn't checked at compile time for some reason.

Will update dates a bit later after committing D1678

Build failure - The Moirai have given mortals hearts that can endure.

Linter detected issues:
Executing section Source...

source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Object/Object.h
|   1| /*·Copyright·(C)·2011·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2011"

source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Object/Object.cpp
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2017"

source/tools/atlas/AtlasObject/AtlasObjectXML.cpp
|   1| /*·Copyright·(C)·2009·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2009"

source/tools/atlas/AtlasObject/AtlasObjectImpl.h
|   1| /*·Copyright·(C)·2009·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2009"

source/tools/atlas/AtlasUI/ScenarioEditor/ScenarioEditor.cpp
|   1| /*·Copyright·(C)·2018·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2018"

source/tools/atlas/AtlasObject/AtlasObjectText.cpp
|   1| /*·Copyright·(C)·2009·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2009"

source/tools/atlas/AtlasObject/AtlasObjectJS.cpp
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2017"

source/tools/atlas/AtlasObject/AtlasObject.h
|   1| /*·Copyright·(C)·2015·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2015"

source/tools/atlas/AtlasObject/AtlasObjectImpl.cpp
|   1| /*·Copyright·(C)·2015·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2015"
Executing section JS...
Executing section cli...

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

Angen added inline comments.Jan 12 2019, 5:26 PM
source/tools/atlas/AtlasObject/AtlasObjectXML.cpp
203 ↗(On Diff #7318)

m_Children I think, did you build it ?

Angen requested changes to this revision.Jan 12 2019, 5:27 PM
This revision now requires changes to proceed.Jan 12 2019, 5:27 PM
Stan marked an inline comment as done.Jan 12 2019, 5:32 PM
Stan added inline comments.
source/tools/atlas/AtlasObject/AtlasObjectXML.cpp
203 ↗(On Diff #7318)

Yes it builds somehow, that's really weird. I guess it cached some stuff.

Stan updated this revision to Diff 7320.Jan 12 2019, 6:26 PM

Update dates fix build. Will rename the members in the next diff.

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

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

Stan updated this revision to Diff 7323.Jan 12 2019, 7:38 PM

@Itms should be good to go.

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

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

vladislavbelov added inline comments.
source/tools/atlas/AtlasObject/AtlasObject.h
126 ↗(On Diff #7323)

Members names after the m_ prefix starts with a capital letter. I'd call it m_Node, because m_P tells nothing that it's AtNode.

source/tools/atlas/AtlasObject/AtlasObjectImpl.cpp
285 ↗(On Diff #7323)

const child_maptype::value_type& child.

source/tools/atlas/AtlasObject/AtlasObjectText.cpp
35 ↗(On Diff #7323)

I suppose (obj->m_Value.length() != 0) > !obj->m_Value.empty() (without parenthesis too). The same below.

source/tools/atlas/AtlasObject/AtlasObjectXML.cpp
198 ↗(On Diff #7323)

static_cast?

Stan marked an inline comment as done.Jan 13 2019, 3:15 PM
Stan added inline comments.
source/tools/atlas/AtlasObject/AtlasObjectXML.cpp
198 ↗(On Diff #7323)

I can't make it work it won't cast. Only reinterpret works but I'm worried it will break the code.

vladislavbelov added inline comments.Jan 13 2019, 3:22 PM
source/tools/atlas/AtlasObject/AtlasObjectXML.cpp
198 ↗(On Diff #7323)

Use reinterpret_cast then, C-style cast isn't safer here. It just tries all types of casts. So if the only reinterpret_cast can be used here, then the C-style cast will you use it.

Stan updated this revision to Diff 7336.Jan 13 2019, 6:40 PM
Stan marked 5 inline comments as done.

Fix @vladislavbelov comments.

Build failure - The Moirai have given mortals hearts that can endure.

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

Stan updated this revision to Diff 7343.Jan 13 2019, 10:48 PM

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

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

Stan added a reviewer: Restricted Owners Package.Feb 4 2019, 5:55 PM
Angen accepted this revision.Mar 17 2019, 7:48 PM

C4458 and C4456 not displayed while building project with --atlas. Project builds without errors. It is good for me.

This revision is now accepted and ready to land.Mar 17 2019, 7:48 PM
Stan added a comment.Mar 24 2019, 12:41 PM

@Itms any concerns with this patch ?

Itms removed reviewers: Itms, Restricted Owners Package.Apr 2 2019, 11:50 PM
In D1741#73409, @Stan wrote:

@Itms any concerns with this patch ?

No! I trust Angen's review and the few remarks from Vlad 🙂 You can commit it.

vladislavbelov added inline comments.Apr 3 2019, 12:53 AM
source/tools/atlas/AtlasObject/AtlasObjectImpl.h
44 ↗(On Diff #7343)

I suppose we can rename the refcount too.

Stan added inline comments.Apr 3 2019, 8:10 AM
source/tools/atlas/AtlasObject/AtlasObjectImpl.h
44 ↗(On Diff #7343)

Will do then upload here for autobuild, then commit :)

Stan updated this revision to Diff 7651.Apr 3 2019, 11:28 AM

Rename variable.

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

Link to build: https://jenkins.wildfiregames.com/job/differential/1154/display/redirect

vladislavbelov added inline comments.Apr 3 2019, 1:49 PM
source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Object/Object.cpp
547 ↗(On Diff #7651)

m_Node isn't a member here.

source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Object/Object.h
40 ↗(On Diff #7651)

m_Impl? Like above.

Stan updated this revision to Diff 7653.Apr 3 2019, 3:38 PM

Rename m_Sidebar to m_Impl
Fix iterator name

Vulcan added a comment.Apr 3 2019, 3:43 PM

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

Link to build: https://jenkins.wildfiregames.com/job/differential/1155/display/redirect

Closed by commit rP22161: Fixes (authored by Stan). · Explain WhyApr 5 2019, 2:37 PM
This revision was automatically updated to reflect the committed changes.
Owners added a subscriber: Restricted Owners Package.Apr 5 2019, 2:37 PM