Warning fix (I moved C++17 and CI to another branch) (#739)

* clang warning fixes
* use CMake 3.10 due to MXE CI does not have 3.13 yet

Co-authored-by: Konstantin <rilian@niisi.ras.ru>

AI/VCAI/ArmyManager.cpp

@@ -113,7 +113,7 @@ ui64 ArmyManager::howManyReinforcementsCanBuy(const CCreatureSet * h, const CGDw
 	TResources availableRes = cb->getResourceAmount();
 	int freeHeroSlots = GameConstants::ARMY_SIZE - h->stacksCount();
 
-	for(auto const dc : t->creatures)
+	for(auto const & dc : t->creatures)
 	{
 		creInfo ci = infoFromDC(dc);
 

AI/VCAI/Pathfinding/Actions/ISpecialAction.h

@@ -18,6 +18,7 @@ struct AIPathNode;
 class ISpecialAction
 {
 public:
+	virtual ~ISpecialAction() = default;
 	virtual Goals::TSubgoal whatToDo(const HeroPtr & hero) const = 0;
 
 	virtual void applyOnDestination(

CMakeLists.txt

@@ -1,7 +1,7 @@
 # Minimum required version greatly affect CMake behavior
 # So cmake_minimum_required must be called before the project()
-# 2.8.12 is used since it's present in Ubuntu 14.04 and Cotire require it
-cmake_minimum_required(VERSION 2.8.12)
+# 3.10.0 is used since it's minimal in MXE dependencies for now
+cmake_minimum_required(VERSION 3.10.0)
 
 project(VCMI)
 # TODO
@@ -118,6 +118,12 @@ include(UseDoxygen OPTIONAL)
 #        Compile and linking options       #
 ############################################
 
+#Enable C++14 Globally
+set (CMAKE_CXX_STANDARD 14)
+#General visibility options
+set(CMAKE_CXX_VISIBILITY_PRESET hidden)
+set(CMAKE_VISIBILITY_INLINES_HIDDEN 1)
+
 if(APPLE)
 	set(CMAKE_MACOSX_RPATH 0)
 endif(APPLE)
@@ -180,7 +186,7 @@ if(MINGW OR MSVC)
 endif(MINGW OR MSVC)
 
 if(CMAKE_COMPILER_IS_GNUCXX OR NOT WIN32) #so far all *nix compilers support such parameters
-	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++0x -Wall -Wextra -Wpointer-arith -Wuninitialized")
+	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Wpointer-arith -Wuninitialized")
 	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-strict-aliasing -Wno-switch -Wno-sign-compare -Wno-unused-local-typedefs")
 	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-parameter -Wno-overloaded-virtual -Wno-type-limits -Wno-unknown-pragmas")
 	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-reorder")
@@ -190,7 +196,6 @@ if(CMAKE_COMPILER_IS_GNUCXX OR NOT WIN32) #so far all *nix compilers support suc
 	endif()
 
 	if(UNIX)
-		set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fvisibility=hidden")
 		set(SYSTEM_LIBS ${SYSTEM_LIBS} ${CMAKE_DL_LIBS})
 	endif()
 endif()

client/gui/Geometries.h

@@ -119,6 +119,8 @@ struct Rect : public SDL_Rect
 		w = r.w;
 		h = r.h;
 	}
+	Rect(const Rect& r) : Rect(static_cast<const SDL_Rect&>(r))
+	{}
 	explicit Rect(const SDL_Surface * const &surf)
 	{
 		x = y = 0;

client/windows/GUIClasses.cpp

@@ -1167,7 +1167,6 @@ void CExchangeController::moveArtifact(
 {
 	auto artifact = source->getArt(srcPosition);
 	auto srcLocation = ArtifactLocation(source, srcPosition);
-	bool changeMade = false;
 
 	for(auto slot : artifact->artType->possibleSlots.at(target->bearerType()))
 	{

lib/CGameState.h

@@ -185,7 +185,7 @@ public:
 	bool checkForVisitableDir(const int3 & src, const int3 & dst) const; //check if src tile is visitable from dst tile
 	void calculatePaths(const CGHeroInstance *hero, CPathsInfo &out); //calculates possible paths for hero, by default uses current hero position and movement left; returns pointer to newly allocated CPath or nullptr if path does not exists
 	void calculatePaths(std::shared_ptr<PathfinderConfig> config) override;
-	int3 guardingCreaturePosition (int3 pos) const;
+	int3 guardingCreaturePosition (int3 pos) const override;
 	std::vector<CGObjectInstance*> guardingCreatures (int3 pos) const;
 	void updateRumor();
 

lib/CGameStateFwd.h

@@ -181,13 +181,15 @@ struct DLL_LINKAGE QuestInfo //universal interface for human and AI
 	QuestInfo (const CQuest * Quest, const CGObjectInstance * Obj, int3 Tile) :
 		quest (Quest), obj (Obj), tile (Tile){};
 
-	//FIXME: assignment operator should return QuestInfo &
-	bool operator= (const QuestInfo &qi)
+	QuestInfo (const QuestInfo &qi) : quest(qi.quest), obj(qi.obj), tile(qi.tile)
+	{};
+
+	const QuestInfo& operator= (const QuestInfo &qi)
 	{
 		quest = qi.quest;
 		obj = qi.obj;
 		tile = qi.tile;
-		return true;
+		return *this;
 	}
 
 	bool operator== (const QuestInfo & qi) const

lib/CPathfinder.h

@@ -232,6 +232,7 @@ struct DLL_LINKAGE CDestinationNodeInfo : public PathNodeInfo
 class IPathfindingRule
 {
 public:
+	virtual ~IPathfindingRule() = default;
 	virtual void process(
 		const PathNodeInfo & source,
 		CDestinationNodeInfo & destination,
@@ -419,6 +420,7 @@ public:
 	}
 
 	void initialize(const PathfinderOptions & options, const CGameState * gs) override;
+	virtual ~NodeStorage() = default;
 
 	virtual std::vector<CGPathNode *> getInitialNodes() override;
 
@@ -456,6 +458,7 @@ private:
 
 public:
 	SingleHeroPathfinderConfig(CPathsInfo & out, CGameState * gs, const CGHeroInstance * hero);
+	virtual ~SingleHeroPathfinderConfig() = default;
 
 	virtual CPathfinderHelper * getOrCreatePathfinderHelper(const PathNodeInfo & source, CGameState * gs) override;
 
@@ -545,7 +548,7 @@ public:
 	const PathfinderOptions & options;
 
 	CPathfinderHelper(CGameState * gs, const CGHeroInstance * Hero, const PathfinderOptions & Options);
-	~CPathfinderHelper();
+	virtual ~CPathfinderHelper();
 	void initializePatrol();
 	bool isHeroPatrolLocked() const;
 	bool isPatrolMovementAllowed(const int3 & dst) const;

lib/HeroBonus.h

@@ -128,6 +128,7 @@ class DLL_LINKAGE CCheckProxy
 public:
 	CCheckProxy(const IBonusBearer * Target, CSelector Selector);
 	CCheckProxy(const CCheckProxy & other);
+	CCheckProxy& operator= (const CCheckProxy & other) = default;
 
 	bool getHasBonus() const;
 
@@ -702,6 +703,7 @@ public:
 	// * root is node on which call was made (nullptr will be replaced with this)
 	//interface
 	IBonusBearer();
+	virtual ~IBonusBearer() = default;
 	virtual TConstBonusListPtr getAllBonuses(const CSelector &selector, const CSelector &limit, const CBonusSystemNode *root = nullptr, const std::string &cachingStr = "") const = 0;
 	int valOfBonuses(const CSelector &selector, const std::string &cachingStr = "") const;
 	bool hasBonus(const CSelector &selector, const std::string &cachingStr = "") const;

lib/NetPacksLib.cpp

@@ -709,6 +709,7 @@ DLL_LINKAGE void NewObject::applyGs(CGameState *gs)
 
 		const int3 previousXAxisTile = int3(pos.x - 1, pos.y, pos.z);
 		assert(gs->isInTheMap(previousXAxisTile) && (testObject.visitablePos() == previousXAxisTile));
+		UNUSED(previousXAxisTile);
 	}
 	else
 	{

lib/battle/CBattleInfoCallback.cpp

@@ -1782,7 +1782,7 @@ SpellID CBattleInfoCallback::getRandomBeneficialSpell(CRandomGenerator & rand, c
 			return stacks.front();
 	};
 
-	for(const SpellID spellID : allPossibleSpells)
+	for(const SpellID& spellID : allPossibleSpells)
 	{
 		std::stringstream cachingStr;
 		cachingStr << "source_" << Bonus::SPELL_EFFECT << "id_" << spellID.num;

lib/events/ObjectVisitEnded.h

@@ -28,7 +28,6 @@ public:
 private:
 	PlayerColor player;
 	ObjectInstanceID heroId;
-	bool enabled;
 };
 
 }

lib/mapObjects/CGTownInstance.cpp

@@ -1285,8 +1285,7 @@ void CGTownInstance::recreateBuildingsBonuses()
 	for(auto b : bl)
 		removeBonus(b);
 
-	auto owner = this->getOwner();
-	for(const auto bid : builtBuildings)
+	for(const auto & bid : builtBuildings)
 	{
 		if(vstd::contains(overriddenBuildings, bid)) //tricky! -> checks tavern only if no bratherhood of sword
 			continue;
@@ -1586,13 +1585,13 @@ void CGTownInstance::serializeJsonOptions(JsonSerializeFormat & handler)
 
 			boost::logic::tribool hasFort(false);
 
-			for(const BuildingID id : forbiddenBuildings)
+			for(const BuildingID & id : forbiddenBuildings)
 			{
 				buildingsLIC.none.insert(id);
 				customBuildings = true;
 			}
 
-			for(const BuildingID id : builtBuildings)
+			for(const BuildingID & id : builtBuildings)
 			{
 				if(id == BuildingID::DEFAULT)
 					continue;

lib/rmg/CRmgTemplateZone.cpp

@@ -419,7 +419,7 @@ void CRmgTemplateZone::fractalize()
 
 void CRmgTemplateZone::connectLater()
 {
-	for (const int3 node : tilesToConnectLater)
+	for (const int3& node : tilesToConnectLater)
 	{
 		if (!connectWithCenter(node, true))
 			logGlobal->error("Failed to connect node %s with center of the zone", node.toString());

lib/serializer/BinaryDeserializer.h

@@ -521,7 +521,7 @@ public:
 	std::unique_ptr<FileStream> sfile;
 
 	CLoadFile(const boost::filesystem::path & fname, int minimalVersion = SERIALIZATION_VERSION); //throws!
-	~CLoadFile();
+	virtual ~CLoadFile();
 	int read(void * data, unsigned size) override; //throws!
 
 	void openNextFile(const boost::filesystem::path & fname, int minimalVersion); //throws!

lib/serializer/CTypeList.h

@@ -17,6 +17,7 @@ struct IPointerCaster
 	virtual boost::any castSharedPtr(const boost::any &ptr) const = 0; // takes std::shared_ptr<From>, performs dynamic cast, returns std::shared_ptr<To>
 	virtual boost::any castWeakPtr(const boost::any &ptr) const = 0; // takes std::weak_ptr<From>, performs dynamic cast, returns std::weak_ptr<To>. The object under poitner must live.
 	//virtual boost::any castUniquePtr(const boost::any &ptr) const = 0; // takes std::unique_ptr<From>, performs dynamic cast, returns std::unique_ptr<To>
+	virtual ~IPointerCaster() = default;
 };
 
 template <typename From, typename To>

lib/serializer/JsonDeserializer.cpp

@@ -63,7 +63,7 @@ void JsonDeserializer::serializeInternal(const std::string & fieldName, std::vec
 	value.clear();
 	value.reserve(data.size());
 
-	for(const JsonNode elem : data)
+	for(const JsonNode& elem : data)
 	{
 		si32 rawId = decoder(elem.String());
 

lib/spells/CSpellHandler.cpp

@@ -372,7 +372,7 @@ void CSpell::getEffects(std::vector<Bonus> & lst, const int level, const bool cu
 
 	lst.reserve(lst.size() + effects.size());
 
-	for(const auto b : effects)
+	for(const auto& b : effects)
 	{
 		Bonus nb(*b);
 

lib/spells/TargetCondition.h

@@ -43,6 +43,7 @@ public:
 	using Object = std::shared_ptr<TargetConditionItem>;
 
 	static const TargetConditionItemFactory * getDefault();
+	virtual ~TargetConditionItemFactory() = default;
 
 	virtual Object createAbsoluteLevel() const = 0;
 	virtual Object createAbsoluteSpell() const = 0;

server/CGameHandler.h

@@ -242,7 +242,7 @@ public:
 	//void lootArtifacts (TArtHolder source, TArtHolder dest, std::vector<ui32> &arts); //after battle - move al arts to winer
 	bool buySecSkill( const IMarket *m, const CGHeroInstance *h, SecondarySkill skill);
 	bool garrisonSwap(ObjectInstanceID tid);
-	bool swapGarrisonOnSiege(ObjectInstanceID tid);
+	bool swapGarrisonOnSiege(ObjectInstanceID tid) override;
 	bool upgradeCreature( ObjectInstanceID objid, SlotID pos, CreatureID upgID );
 	bool recruitCreatures(ObjectInstanceID objid, ObjectInstanceID dst, CreatureID crid, ui32 cram, si32 level);
 	bool buildStructure(ObjectInstanceID tid, BuildingID bid, bool force=false);//force - for events: no cost, no checkings