From 8ec1ae3319ccc6d73897cb82bc6d1c500902a625 Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Thu, 15 Jun 2023 10:55:38 -0500 Subject: [PATCH 1/3] App/Toponaming: ComplexGeoData realthunder original Minor modifications to make it compile with previous refactorings. The only substantial change to the original is moving the getElementHistory function from ComplexGeoData to MappedName so that the dehash function can remain private. --- src/App/ComplexGeoData.cpp | 253 ++++++++++++++++++++++++++++++++++--- src/App/ComplexGeoData.h | 214 ++++++++++++++++++++++++++++--- src/App/ElementMap.cpp | 53 ++++++++ src/App/ElementMap.h | 6 +- src/App/MappedElement.h | 1 - src/App/MappedName.h | 7 +- 6 files changed, 491 insertions(+), 43 deletions(-) diff --git a/src/App/ComplexGeoData.cpp b/src/App/ComplexGeoData.cpp index df0c78265b..fe5d07ece6 100644 --- a/src/App/ComplexGeoData.cpp +++ b/src/App/ComplexGeoData.cpp @@ -1,23 +1,26 @@ -/*************************************************************************** - * Copyright (c) 2002 Jürgen Riegel * - * * - * This file is part of the FreeCAD CAx development system. * - * * - * This library is free software; you can redistribute it and/or * - * modify it under the terms of the GNU Library General Public * - * License as published by the Free Software Foundation; either * - * version 2 of the License, or (at your option) any later version. * - * * - * This library is distributed in the hope that it will be useful, * - * but WITHOUT ANY WARRANTY; without even the implied warranty of * - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * - * GNU Library General Public License for more details. * - * * - * You should have received a copy of the GNU Library General Public * - * License along with this library; see the file COPYING.LIB. If not, * - * write to the Free Software Foundation, Inc., 59 Temple Place, * - * Suite 330, Boston, MA 02111-1307, USA * - * * +// SPDX-License-Identifier: LGPL-2.1-or-later +/**************************************************************************** + * * + * Copyright (c) 2002 Jürgen Riegel * + * Copyright (c) 2022 Zheng, Lei * + * Copyright (c) 2023 FreeCAD Project Association * + * * + * This file is part of FreeCAD. * + * * + * FreeCAD is free software: you can redistribute it and/or modify it * + * under the terms of the GNU Lesser General Public License as * + * published by the Free Software Foundation, either version 2.1 of the * + * License, or (at your option) any later version. * + * * + * FreeCAD is distributed in the hope that it will be useful, but * + * WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * + * Lesser General Public License for more details. * + * * + * You should have received a copy of the GNU Lesser General Public * + * License along with FreeCAD. If not, see * + * . * + * * ***************************************************************************/ @@ -30,11 +33,16 @@ #include #include "ComplexGeoData.h" +#include "ElementMap.h" +#include "ElementNamingUtils.h" #include #include #include +#include +#include + using namespace Data; @@ -43,6 +51,11 @@ TYPESYSTEM_SOURCE_ABSTRACT(Data::Segment , Base::BaseClass) TYPESYSTEM_SOURCE_ABSTRACT(Data::ComplexGeoData , Base::Persistence) +FC_LOG_LEVEL_INIT("ComplexGeoData", true,true) + +namespace bio = boost::iostreams; +using namespace Data; + ComplexGeoData::ComplexGeoData() :Tag(0) @@ -166,3 +179,203 @@ bool ComplexGeoData::getCenterOfGravity(Base::Vector3d&) const return false; } +size_t ComplexGeoData::getElementMapSize(bool flush) const { + if (flush) { + flushElementMap(); +#ifdef _FC_MEM_TRACE + FC_MSG("memory size " << (_MemSize/1024/1024) << "MB, " << (_MemMaxSize/1024/1024)); + for (auto &unit : _MemUnits) + FC_MSG("unit " << unit.first << ": " << unit.second.count << ", " << unit.second.maxcount); +#endif + } + return _elementMap ? _elementMap->size():0; +} + +MappedName ComplexGeoData::getMappedName(const IndexedName & element, + bool allowUnmapped, + ElementIDRefs *sid) const +{ + if (!element) + return MappedName(); + flushElementMap(); + if(!_elementMap) { + if (allowUnmapped) + return MappedName(element); + return MappedName(); + } + + MappedName name = _elementMap->find(element, sid); + if (allowUnmapped && !name) + return MappedName(element); + return name; +} + +IndexedName ComplexGeoData::getIndexedName(const MappedName & name, + ElementIDRefs *sid) const +{ + flushElementMap(); + if (!name) + return IndexedName(); + if (!_elementMap) { + std::string s; + return IndexedName(name.appendToBuffer(s), getElementTypes()); + } + return _elementMap->find(name, sid); +} + +Data::MappedElement +ComplexGeoData::getElementName(const char *name, + ElementIDRefs *sid, + bool copy) const +{ + IndexedName element(name, getElementTypes()); + if (element) + return MappedElement(getMappedName(element, false, sid), element); + + const char * mapped = isMappedElement(name); + if (mapped) + name = mapped; + + MappedElement res; + // Strip out the trailing '.XXXX' if any + const char *dot = strchr(name,'.'); + if(dot) + res.name = MappedName(name, dot-name); + else if (copy) + res.name = name; + else + res.name = MappedName(name); + res.index = getIndexedName(res.name, sid); + return res; +} + +std::vector > +ComplexGeoData::getElementMappedNames(const IndexedName & element, bool needUnmapped) const { + flushElementMap(); + if(_elementMap) { + auto res = _elementMap->findAll(element); + if (!res.empty()) + return res; + } + + if (!needUnmapped) + return {}; + return {std::make_pair(MappedName(element), ElementIDRefs())}; +} + +std::vector +ComplexGeoData::getElementNamesWithPrefix(const char *prefix) const { +#if 0 + std::vector names; + flushElementMap(); + if(!prefix || !prefix[0] || !_elementMap) + return names; + const auto &p = elementMapPrefix(); + if(boost::starts_with(prefix,p)) + prefix += p.size(); + names = _elementMap->findAllStartsWith(prefix); + return names; +#else + (void)prefix; + return {}; +#endif +} + +std::vector ComplexGeoData::getElementMap() const { + flushElementMap(); + if(!_elementMap) + return {}; + return _elementMap->getAll(); +} + +ElementMapPtr ComplexGeoData::elementMap(bool flush) const +{ + if (flush) + flushElementMap(); + return _elementMap; +} + +void ComplexGeoData::flushElementMap() const +{ +} + +void ComplexGeoData::setElementMap(const std::vector &map) { + resetElementMap(); + for(auto &v : map) + _elementMap->setElementName(v.index, v.name, Tag); +} + +char ComplexGeoData::elementType(const Data::MappedName &name) const +{ + if(!name) + return 0; + auto indexedName = getIndexedName(name); + if (indexedName) + return elementType(indexedName); + char element_type=0; + if (name.findTagInElementName(0,0,0,&element_type) < 0) + return elementType(name.toIndexedName()); + return element_type; +} + +char ComplexGeoData::elementType(const Data::IndexedName &element) const +{ + if(!element) + return 0; + for(auto &type : getElementTypes()) { + if(boost::equals(element.getType(), type)) + return type[0]; + } + return 0; +} + +char ComplexGeoData::elementType(const char *name) const { + if(!name) + return 0; + + const char *type = nullptr; + IndexedName element(name, getElementTypes()); + if (element) + type = element.getType(); + else { + const char * mapped = isMappedElement(name); + if (mapped) + name = mapped; + + MappedName n; + const char *dot = strchr(name,'.'); + if(dot) { + n = MappedName(name, dot-name); + type = dot+1; + } + else + n = MappedName::fromRawData(name); + char res = elementType(n); + if (res) + return res; + } + + if(type && type[0]) { + for(auto &t : getElementTypes()) { + if(boost::starts_with(type, t)) + return type[0]; + } + } + return 0; +} + +MappedName ComplexGeoData::renameDuplicateElement(int index, + const IndexedName & element, + const IndexedName & element2, + const MappedName & name, + ElementIDRefs &sids) +{ + std::ostringstream ss; + ss << ELEMENT_MAP_PREFIX << 'D' << std::hex << index; + MappedName renamed(name); + this->elementMap()->encodeElementName(element.getType()[0],renamed,ss,&sids,Tag); + if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) + FC_WARN("duplicate element mapping '" << name << " -> " << renamed << ' ' + << element << '/' << element2); + return renamed; +} diff --git a/src/App/ComplexGeoData.h b/src/App/ComplexGeoData.h index ae59fde241..e7aa240e94 100644 --- a/src/App/ComplexGeoData.h +++ b/src/App/ComplexGeoData.h @@ -1,23 +1,26 @@ -/*************************************************************************** - * Copyright (c) 2002 Jürgen Riegel * - * * - * This file is part of the FreeCAD CAx development system. * - * * - * This library is free software; you can redistribute it and/or * - * modify it under the terms of the GNU Library General Public * - * License as published by the Free Software Foundation; either * - * version 2 of the License, or (at your option) any later version. * - * * - * This library is distributed in the hope that it will be useful, * - * but WITHOUT ANY WARRANTY; without even the implied warranty of * - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * - * GNU Library General Public License for more details. * - * * - * You should have received a copy of the GNU Library General Public * - * License along with this library; see the file COPYING.LIB. If not, * - * write to the Free Software Foundation, Inc., 59 Temple Place, * - * Suite 330, Boston, MA 02111-1307, USA * - * * +// SPDX-License-Identifier: LGPL-2.1-or-later +/**************************************************************************** + * * + * Copyright (c) 2002 Jürgen Riegel * + * Copyright (c) 2022 Zheng, Lei * + * Copyright (c) 2023 FreeCAD Project Association * + * * + * This file is part of FreeCAD. * + * * + * FreeCAD is free software: you can redistribute it and/or modify it * + * under the terms of the GNU Lesser General Public License as * + * published by the Free Software Foundation, either version 2.1 of the * + * License, or (at your option) any later version. * + * * + * FreeCAD is distributed in the hope that it will be useful, but * + * WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * + * Lesser General Public License for more details. * + * * + * You should have received a copy of the GNU Lesser General Public * + * License along with FreeCAD. If not, see * + * . * + * * ***************************************************************************/ @@ -28,6 +31,10 @@ #include #include #include +#include "MappedName.h" +#include "MappedElement.h" +#include "ElementMap.h" +#include "StringHasher.h" #ifdef __GNUC__ # include @@ -45,6 +52,8 @@ using BoundBox3d = BoundBox3; namespace Data { +struct MappedChildElements; + /** Segments * Subelement type of the ComplexGeoData type * It is used to split an object in further sub-parts. @@ -164,6 +173,135 @@ public: virtual bool getCenterOfGravity(Base::Vector3d& center) const; //@} + + /** @name Element name mapping */ + //@{ + + /** Get element indexed name + * + * @param name: the input name + * @param sid: optional output of and App::StringID involved forming this mapped name + * + * @return Returns an indexed name. + */ + IndexedName getIndexedName(const MappedName & name, + ElementIDRefs *sid = nullptr) const; + + /** Get element mapped name + * + * @param name: the input name + * @param allowUnmapped: If the queried element is not mapped, then return + * an empty name if \c allowUnmapped is false, or + * else, return the indexed name. + * @param sid: optional output of and App::StringID involved forming this mapped name + * @return Returns the mapped name. + */ + MappedName getMappedName(const IndexedName & element, + bool allowUnmapped = false, + ElementIDRefs *sid = nullptr) const; + + /** Return a pair of indexed name and mapped name + * + * @param name: the input name. + * @param sid: optional output of and App::StringID involved forming this + * mapped name + * @param copy: if true, copy the name string, or else use it as constant + * string, and caller must make sure the memory is not freed. + * + * @return Returns the MappedElement which contains both the indexed and + * mapped name. + * + * This function guesses whether the input name is an indexed name or + * mapped, and perform a lookup and return the names found. If the input + * name contains only alphabets and underscore followed by optional digits, + * it will be treated as indexed name. Or else, it will be treated as + * mapped name. + */ + MappedElement getElementName(const char * name, + ElementIDRefs *sid = nullptr, + bool copy = false) const; + + /** Get mapped element with a given prefix */ + std::vector getElementNamesWithPrefix(const char *prefix) const; + + /** Get mapped element names + * + * @param element: original element name with \c Type + \c Index + * @param needUnmapped: if true, return the original element name if no + * mapping is found + * + * @return a list of mapped names of the give element along with their + * associated string ID references + */ + std::vector > + getElementMappedNames(const IndexedName & element, bool needUnmapped=false) const; + + /// Append the Tag (if and only if it is non zero) into the element map + virtual void reTagElementMap(long tag, App::StringHasherRef hasher, const char *postfix=0) { + (void)tag; + (void)hasher; + (void)postfix; + } + + // NOTE: getElementHistory is now in ElementMap + + char elementType(const Data::MappedName &) const; + char elementType(const Data::IndexedName &) const; + char elementType(const char *name) const; + + /** Reset/swap the element map + * + * @param elementMap: optional new element map + * + * @return Returns the existing element map. + */ + virtual ElementMapPtr resetElementMap(ElementMapPtr elementMap=ElementMapPtr()) { + _elementMap.swap(elementMap); + return elementMap; + } + + /// Get the entire element map + std::vector getElementMap() const; + + /// Set the entire element map + void setElementMap(const std::vector &elements); + + /// Get the current element map size + size_t getElementMapSize(bool flush=true) const; + + /// Check if the given subname only contains an element name + static bool isElementName(const char *subname) { + return subname && *subname && findElementName(subname)==subname; + } + + /** Element trace callback + * + * The callback has the following call signature + * (const std::string &name, size_t offset, long encodedTag, long tag) -> bool + * + * @param name: the current element name. + * @param offset: the offset skipping the encoded element name for the next iteration. + * @param encodedTag: the tag encoded inside the current element, which is usually the tag + * of the previous step in the shape history. + * @param tag: the tag of the current shape element. + * + * @sa traceElement() + */ + using TraceCallback = std::function; + + /** Iterate through the history of the give element name with a given callback + * + * @param name: the input element name + * @param cb: trace callback with call signature. + * @sa TraceCallback + */ + void traceElement(const MappedName &name, TraceCallback cb) const; + + /** Flush an internal buffering for element mapping */ + virtual void flushElementMap() const; + virtual unsigned long getElementMapReserve() const { return 0; } + //@} + protected: /// from local to outside @@ -223,6 +361,42 @@ protected: } public: mutable long Tag; + + +public: + /// String hasher for element name shortening + mutable App::StringHasherRef Hasher; + +protected: + virtual MappedName renameDuplicateElement(int index, + const IndexedName & element, + const IndexedName & element2, + const MappedName & name, + ElementIDRefs &sids); + + /// from local to outside + inline Base::Vector3d transformToOutside(const Base::Vector3f& vec) const + { + return getTransform() * Base::Vector3d(static_cast(vec.x), + static_cast(vec.y), + static_cast(vec.z)); + } + /// from local to inside + inline Base::Vector3f transformToInside(const Base::Vector3d& vec) const + { + Base::Matrix4D tmpM(getTransform()); + tmpM.inverse(); + Base::Vector3d tmp = tmpM * vec; + return Base::Vector3f(static_cast(tmp.x), + static_cast(tmp.y), + static_cast(tmp.z)); + } + +protected: + ElementMapPtr elementMap(bool flush=true) const; + +private: + ElementMapPtr _elementMap; }; } //namespace App diff --git a/src/App/ElementMap.cpp b/src/App/ElementMap.cpp index 9f8a98fdd9..cc22f6b569 100644 --- a/src/App/ElementMap.cpp +++ b/src/App/ElementMap.cpp @@ -1155,5 +1155,58 @@ std::vector ElementMap::getAll() const return ret; } +long ElementMap::getElementHistory(const MappedName & name, + long masterTag, + MappedName *original, + std::vector *history) const +{ + long tag = 0; + int len = 0; + int pos = name.findTagInElementName(&tag,&len,nullptr,nullptr,true); + if(pos < 0) { + if(original) + *original = name; + return tag; + } + if(!original && !history) + return tag; + + MappedName tmp; + MappedName &ret = original?*original:tmp; + if(name.startsWith(ELEMENT_MAP_PREFIX)) { + unsigned offset = ELEMENT_MAP_PREFIX_SIZE; + ret = MappedName::fromRawData(name, offset); + } else + ret = name; + + while(1) { + if(!len || len>pos) { + FC_WARN("invalid name length " << name); + return 0; + } + bool dehashed = false; + if (ret.startsWith(MAPPED_CHILD_ELEMENTS_PREFIX, len)) { + int offset = (int)POSTFIX_TAG_SIZE; + MappedName tmp = MappedName::fromRawData(ret, len+offset, pos-len-offset); + MappedName postfix = dehashElementName(tmp); + if (postfix != tmp) { + dehashed = true; + ret = MappedName::fromRawData(ret, 0, len) + postfix; + } + } + if (!dehashed) + ret = dehashElementName(MappedName::fromRawData(ret, 0, len)); + + long tag2 = 0; + pos = ret.findTagInElementName(&tag2,&len,nullptr,nullptr,true); + if(pos < 0 || (tag2!=tag && tag2!=-tag && tag!=masterTag && -tag!=masterTag)) + return tag; + tag = tag2; + if(history) + history->push_back(ret.copy()); + } +} + + }// Namespace Data diff --git a/src/App/ElementMap.h b/src/App/ElementMap.h index ff17a1bccb..aea3e9ff22 100644 --- a/src/App/ElementMap.h +++ b/src/App/ElementMap.h @@ -170,7 +170,7 @@ public: QByteArray postfix; ElementIDRefs sids; - // prefix() has been moved to PostfixStringReferences.h + // prefix() has been moved to ElementNamingUtils.h }; /* Note: the original addChildElements passed `ComplexGeoData& master` for getting the `Tag`, @@ -181,6 +181,10 @@ public: std::vector getAll() const; + long getElementHistory(const MappedName & name, + long masterTag, + MappedName *original=0, std::vector *history=0) const; + private: /** Serialize this map * @param s: serialized stream diff --git a/src/App/MappedElement.h b/src/App/MappedElement.h index ef5784d7d9..7f6ee68f01 100644 --- a/src/App/MappedElement.h +++ b/src/App/MappedElement.h @@ -23,7 +23,6 @@ #ifndef APP_MAPPED_ELEMENT_H #define APP_MAPPED_ELEMENT_H -#include "ComplexGeoData.h" #include "IndexedName.h" #include "MappedName.h" diff --git a/src/App/MappedName.h b/src/App/MappedName.h index 184a763b87..54a58ea19f 100644 --- a/src/App/MappedName.h +++ b/src/App/MappedName.h @@ -35,7 +35,6 @@ #include #include -#include "ComplexGeoData.h" #include "IndexedName.h" #include "StringHasher.h" #include "ElementNamingUtils.h" @@ -102,6 +101,12 @@ public: } } + explicit MappedName(const App::StringIDRef & sid) + :raw(false) + { + sid.toBytes(this->data); + } + MappedName() : raw(false) {} From 66f6350fd33977d41b3e0f1891de2293026db438 Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Thu, 6 Jul 2023 10:12:55 -0500 Subject: [PATCH 2/3] App/Toponaming: Basic tests and linter issues --- src/App/ComplexGeoData.cpp | 187 +++---- src/App/ComplexGeoData.h | 68 +-- src/App/ElementMap.cpp | 875 ++++++++++++++++++------------- src/App/ElementMap.h | 34 +- src/App/MappedName.h | 96 ++-- src/App/StringHasher.h | 3 +- tests/src/App/CMakeLists.txt | 1 + tests/src/App/ComplexGeoData.cpp | 417 +++++++++++++++ tests/src/App/ElementMap.cpp | 20 +- tests/src/App/MappedName.cpp | 17 +- 10 files changed, 1138 insertions(+), 580 deletions(-) create mode 100644 tests/src/App/ComplexGeoData.cpp diff --git a/src/App/ComplexGeoData.cpp b/src/App/ComplexGeoData.cpp index fe5d07ece6..5c6fd3291a 100644 --- a/src/App/ComplexGeoData.cpp +++ b/src/App/ComplexGeoData.cpp @@ -24,7 +24,7 @@ ***************************************************************************/ -#include "PreCompiled.h" +#include "PreCompiled.h"// NOLINT #ifndef _PreComp_ # include @@ -41,21 +41,19 @@ #include #include -#include using namespace Data; -TYPESYSTEM_SOURCE_ABSTRACT(Data::Segment , Base::BaseClass) +TYPESYSTEM_SOURCE_ABSTRACT(Data::Segment , Base::BaseClass)// NOLINT +TYPESYSTEM_SOURCE_ABSTRACT(Data::ComplexGeoData , Base::Persistence)// NOLINT - -TYPESYSTEM_SOURCE_ABSTRACT(Data::ComplexGeoData , Base::Persistence) - -FC_LOG_LEVEL_INIT("ComplexGeoData", true,true) +FC_LOG_LEVEL_INIT("ComplexGeoData", true,true)// NOLINT namespace bio = boost::iostreams; using namespace Data; +// NOLINTBEGIN(cppcoreguidelines-pro-bounds-pointer-arithmetic) ComplexGeoData::ComplexGeoData() :Tag(0) @@ -107,10 +105,10 @@ Base::Placement ComplexGeoData::getPlacement() const { Base::Matrix4D mat = getTransform(); - return Base::Placement(Base::Vector3d(mat[0][3], - mat[1][3], - mat[2][3]), - Base::Rotation(mat)); + return {Base::Vector3d(mat[0][3], + mat[1][3], + mat[2][3]), + Base::Rotation(mat)}; } double ComplexGeoData::getAccuracy() const @@ -118,19 +116,21 @@ double ComplexGeoData::getAccuracy() const return 0.0; } -void ComplexGeoData::getLinesFromSubElement(const Segment*, +void ComplexGeoData::getLinesFromSubElement(const Segment* segment, std::vector &Points, std::vector &lines) const { + (void)segment; (void)Points; (void)lines; } -void ComplexGeoData::getFacesFromSubElement(const Segment*, +void ComplexGeoData::getFacesFromSubElement(const Segment* segment, std::vector &Points, std::vector &PointNormals, std::vector &faces) const { + (void)segment; (void)Points; (void)PointNormals; (void)faces; @@ -174,8 +174,9 @@ void ComplexGeoData::getFaces(std::vector &Points, (void)flags; } -bool ComplexGeoData::getCenterOfGravity(Base::Vector3d&) const +bool ComplexGeoData::getCenterOfGravity(Base::Vector3d& unused) const { + (void)unused; return false; } @@ -195,18 +196,21 @@ MappedName ComplexGeoData::getMappedName(const IndexedName & element, bool allowUnmapped, ElementIDRefs *sid) const { - if (!element) - return MappedName(); + if (!element) { + return {}; + } flushElementMap(); if(!_elementMap) { - if (allowUnmapped) + if (allowUnmapped) { return MappedName(element); - return MappedName(); + } + return {}; } MappedName name = _elementMap->find(element, sid); - if (allowUnmapped && !name) + if (allowUnmapped && !name) { return MappedName(element); + } return name; } @@ -214,11 +218,12 @@ IndexedName ComplexGeoData::getIndexedName(const MappedName & name, ElementIDRefs *sid) const { flushElementMap(); - if (!name) + if (!name) { return IndexedName(); + } if (!_elementMap) { - std::string s; - return IndexedName(name.appendToBuffer(s), getElementTypes()); + std::string str; + return {name.appendToBuffer(str), getElementTypes()}; } return _elementMap->find(name, sid); } @@ -229,24 +234,29 @@ ComplexGeoData::getElementName(const char *name, bool copy) const { IndexedName element(name, getElementTypes()); - if (element) - return MappedElement(getMappedName(element, false, sid), element); + if (element) { + return {getMappedName(element, false, sid), element}; + } const char * mapped = isMappedElement(name); - if (mapped) + if (mapped) { name = mapped; + } - MappedElement res; + MappedElement result; // Strip out the trailing '.XXXX' if any const char *dot = strchr(name,'.'); - if(dot) - res.name = MappedName(name, dot-name); - else if (copy) - res.name = name; - else - res.name = MappedName(name); - res.index = getIndexedName(res.name, sid); - return res; + if(dot) { + result.name = MappedName(name, dot - name); + } + else if (copy) { + result.name = name; + } + else { + result.name = MappedName(name); + } + result.index = getIndexedName(result.name, sid); + return result; } std::vector > @@ -254,44 +264,30 @@ ComplexGeoData::getElementMappedNames(const IndexedName & element, bool needUnma flushElementMap(); if(_elementMap) { auto res = _elementMap->findAll(element); - if (!res.empty()) + if (!res.empty()) { return res; + } } - if (!needUnmapped) + if (!needUnmapped) { return {}; + } return {std::make_pair(MappedName(element), ElementIDRefs())}; } -std::vector -ComplexGeoData::getElementNamesWithPrefix(const char *prefix) const { -#if 0 - std::vector names; - flushElementMap(); - if(!prefix || !prefix[0] || !_elementMap) - return names; - const auto &p = elementMapPrefix(); - if(boost::starts_with(prefix,p)) - prefix += p.size(); - names = _elementMap->findAllStartsWith(prefix); - return names; -#else - (void)prefix; - return {}; -#endif -} - std::vector ComplexGeoData::getElementMap() const { flushElementMap(); - if(!_elementMap) + if(!_elementMap) { return {}; + } return _elementMap->getAll(); } ElementMapPtr ComplexGeoData::elementMap(bool flush) const { - if (flush) + if (flush) { flushElementMap(); + } return _elementMap; } @@ -300,82 +296,93 @@ void ComplexGeoData::flushElementMap() const } void ComplexGeoData::setElementMap(const std::vector &map) { - resetElementMap(); - for(auto &v : map) - _elementMap->setElementName(v.index, v.name, Tag); + _elementMap = std::make_shared(); // Get rid of the old one, if any, but make + // sure the memory exists for the new data. + for(auto &element : map) { + _elementMap->setElementName(element.index, element.name, Tag); + } } char ComplexGeoData::elementType(const Data::MappedName &name) const { - if(!name) + if(!name) { return 0; + } auto indexedName = getIndexedName(name); - if (indexedName) + if (indexedName) { return elementType(indexedName); + } char element_type=0; - if (name.findTagInElementName(0,0,0,&element_type) < 0) + if (name.findTagInElementName(nullptr,nullptr,nullptr,&element_type) < 0) { return elementType(name.toIndexedName()); + } return element_type; } char ComplexGeoData::elementType(const Data::IndexedName &element) const { - if(!element) + if(!element) { return 0; + } for(auto &type : getElementTypes()) { - if(boost::equals(element.getType(), type)) + if(boost::equals(element.getType(), type)) { return type[0]; + } } return 0; } +// The elementType function can take a char *, in which case it tries a sequence of checks to +// see what it got. +// 1) Check to see if it is an indexedName, and if so directly look up the type +// 2) If not: +// a) Remove any element map prefix that is present +// b) See if the name contains a dot: +// i) If yes, create a MappedName from the part before the dot, and set the type to the +// part after the dot +// ii) If no, create a MappedName from the whole name +// c) Try to get the elementType based on the MappedName. Return it if found +// 3) Check to make sure the discovered type is in the list of types, and return its first +// character if so. char ComplexGeoData::elementType(const char *name) const { - if(!name) + if(!name) { return 0; + } const char *type = nullptr; IndexedName element(name, getElementTypes()); - if (element) + if (element) { type = element.getType(); + } else { const char * mapped = isMappedElement(name); - if (mapped) + if (mapped) { name = mapped; + } - MappedName n; + MappedName mappedName; const char *dot = strchr(name,'.'); if(dot) { - n = MappedName(name, dot-name); + mappedName = MappedName(name, dot-name); type = dot+1; } - else - n = MappedName::fromRawData(name); - char res = elementType(n); - if (res) + else { + mappedName = MappedName::fromRawData(name); + } + char res = elementType(mappedName); + if (res != 0) { return res; + } } if(type && type[0]) { - for(auto &t : getElementTypes()) { - if(boost::starts_with(type, t)) + for(auto &elementTypes : getElementTypes()) { + if(boost::starts_with(type, elementTypes)) { return type[0]; + } } } return 0; } -MappedName ComplexGeoData::renameDuplicateElement(int index, - const IndexedName & element, - const IndexedName & element2, - const MappedName & name, - ElementIDRefs &sids) -{ - std::ostringstream ss; - ss << ELEMENT_MAP_PREFIX << 'D' << std::hex << index; - MappedName renamed(name); - this->elementMap()->encodeElementName(element.getType()[0],renamed,ss,&sids,Tag); - if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) - FC_WARN("duplicate element mapping '" << name << " -> " << renamed << ' ' - << element << '/' << element2); - return renamed; -} +// NOLINTEND(cppcoreguidelines-pro-bounds-pointer-arithmetic) diff --git a/src/App/ComplexGeoData.h b/src/App/ComplexGeoData.h index e7aa240e94..dd1b81ff62 100644 --- a/src/App/ComplexGeoData.h +++ b/src/App/ComplexGeoData.h @@ -24,8 +24,8 @@ ***************************************************************************/ -#ifndef _AppComplexGeoData_h_ -#define _AppComplexGeoData_h_ +#ifndef APP_COMPLEX_GEO_DATA_H +#define APP_COMPLEX_GEO_DATA_H #include #include @@ -45,7 +45,7 @@ namespace Base { class Placement; class Rotation; -template class BoundBox3; +template class BoundBox3;// NOLINT using BoundBox3d = BoundBox3; } @@ -55,12 +55,12 @@ namespace Data struct MappedChildElements; /** Segments - * Subelement type of the ComplexGeoData type + * Sub-element type of the ComplexGeoData type * It is used to split an object in further sub-parts. */ class AppExport Segment: public Base::BaseClass { - TYPESYSTEM_HEADER_WITH_OVERRIDE(); + TYPESYSTEM_HEADER_WITH_OVERRIDE();// NOLINT public: ~Segment() override = default; @@ -72,7 +72,7 @@ public: */ class AppExport ComplexGeoData: public Base::Persistence, public Base::Handled { - TYPESYSTEM_HEADER_WITH_OVERRIDE(); + TYPESYSTEM_HEADER_WITH_OVERRIDE();// NOLINT public: struct Line {uint32_t I1; uint32_t I2;}; @@ -87,17 +87,17 @@ public: /// Destructor ~ComplexGeoData() override; - /** @name Subelement management */ + /** @name Sub-element management */ //@{ /** Sub type list - * List of different subelement types - * its NOT a list of the subelements itself + * List of different sub-element types + * its NOT a list of the sub-elements itself */ virtual std::vector getElementTypes() const=0; virtual unsigned long countSubElements(const char* Type) const=0; - /// get the subelement by type and number + /// get the sub-element by type and number virtual Segment* getSubElement(const char* Type, unsigned long) const=0; - /// get subelement by combined name + /// get sub-element by combined name virtual Segment* getSubElementByName(const char* Name) const; /** Get lines from segment */ virtual void getLinesFromSubElement( @@ -203,8 +203,8 @@ public: /** Return a pair of indexed name and mapped name * * @param name: the input name. - * @param sid: optional output of and App::StringID involved forming this - * mapped name + * @param sid: optional output of any App::StringID involved in forming + * this mapped name * @param copy: if true, copy the name string, or else use it as constant * string, and caller must make sure the memory is not freed. * @@ -221,9 +221,6 @@ public: ElementIDRefs *sid = nullptr, bool copy = false) const; - /** Get mapped element with a given prefix */ - std::vector getElementNamesWithPrefix(const char *prefix) const; - /** Get mapped element names * * @param element: original element name with \c Type + \c Index @@ -237,7 +234,9 @@ public: getElementMappedNames(const IndexedName & element, bool needUnmapped=false) const; /// Append the Tag (if and only if it is non zero) into the element map - virtual void reTagElementMap(long tag, App::StringHasherRef hasher, const char *postfix=0) { + virtual void reTagElementMap(long tag, + App::StringHasherRef hasher, + const char *postfix=nullptr) { (void)tag; (void)hasher; (void)postfix; @@ -269,37 +268,13 @@ public: /// Get the current element map size size_t getElementMapSize(bool flush=true) const; - /// Check if the given subname only contains an element name - static bool isElementName(const char *subname) { - return subname && *subname && findElementName(subname)==subname; + /// Check if the given sub-name only contains an element name + static bool isElementName(const char *subName) { + return (subName != nullptr) && (*subName != 0) && findElementName(subName)==subName; } - /** Element trace callback - * - * The callback has the following call signature - * (const std::string &name, size_t offset, long encodedTag, long tag) -> bool - * - * @param name: the current element name. - * @param offset: the offset skipping the encoded element name for the next iteration. - * @param encodedTag: the tag encoded inside the current element, which is usually the tag - * of the previous step in the shape history. - * @param tag: the tag of the current shape element. - * - * @sa traceElement() - */ - using TraceCallback = std::function; - - /** Iterate through the history of the give element name with a given callback - * - * @param name: the input element name - * @param cb: trace callback with call signature. - * @sa TraceCallback - */ - void traceElement(const MappedName &name, TraceCallback cb) const; - /** Flush an internal buffering for element mapping */ virtual void flushElementMap() const; - virtual unsigned long getElementMapReserve() const { return 0; } //@} protected: @@ -368,11 +343,6 @@ public: mutable App::StringHasherRef Hasher; protected: - virtual MappedName renameDuplicateElement(int index, - const IndexedName & element, - const IndexedName & element2, - const MappedName & name, - ElementIDRefs &sids); /// from local to outside inline Base::Vector3d transformToOutside(const Base::Vector3f& vec) const diff --git a/src/App/ElementMap.cpp b/src/App/ElementMap.cpp index cc22f6b569..3be76b9973 100644 --- a/src/App/ElementMap.cpp +++ b/src/App/ElementMap.cpp @@ -1,7 +1,9 @@ #include "PreCompiled.h" #ifndef _PreComp_ -# include -# include +#include +#ifndef FC_DEBUG +#include +#endif #endif #include "ElementMap.h" @@ -10,11 +12,11 @@ #include "App/Application.h" #include "Base/Console.h" -# include -# include +#include +#include -FC_LOG_LEVEL_INIT("ElementMap", true, 2); +FC_LOG_LEVEL_INIT("ElementMap", true, 2);// NOLINT namespace Data { @@ -37,27 +39,28 @@ namespace Data // any shape sharing is lost. But again, we do want to keep separate shape files // because of partial loading. The same technique used here can be applied to // restore shape sharing. -static std::unordered_map _ElementMapToId; -static std::unordered_map _IdToElementMap; +static std::unordered_map _elementMapToId; +static std::unordered_map _idToElementMap; -void ElementMap::init() { +void ElementMap::init() +{ static bool inited; if (!inited) { inited = true; ::App::GetApplication().signalStartSaveDocument.connect( [](const ::App::Document&, const std::string&) { - _ElementMapToId.clear(); + _elementMapToId.clear(); }); ::App::GetApplication().signalFinishSaveDocument.connect( [](const ::App::Document&, const std::string&) { - _ElementMapToId.clear(); + _elementMapToId.clear(); }); ::App::GetApplication().signalStartRestoreDocument.connect([](const ::App::Document&) { - _IdToElementMap.clear(); + _idToElementMap.clear(); }); ::App::GetApplication().signalFinishRestoreDocument.connect([](const ::App::Document&) { - _IdToElementMap.clear(); + _idToElementMap.clear(); }); } } @@ -68,131 +71,145 @@ ElementMap::ElementMap() } -void ElementMap::beforeSave(const ::App::StringHasherRef& hasher) const +void ElementMap::beforeSave(const ::App::StringHasherRef& hasherRef) const { - unsigned& id = _ElementMapToId[this]; - if (!id) - id = _ElementMapToId.size(); + unsigned& id = _elementMapToId[this]; + if (id == 0U) { + id = _elementMapToId.size(); + } this->_id = id; - for (auto& v : this->indexedNames) { - for (const MappedNameRef& ref : v.second.names) { - for (const MappedNameRef* r = &ref; r; r = r->next.get()) { - for (const ::App::StringIDRef& sid : r->sids) { - if (sid.isFromSameHasher(hasher)) + for (auto& indexedName : this->indexedNames) { + for (const MappedNameRef& mappedName : indexedName.second.names) { + for (const MappedNameRef* ref = &mappedName; ref; ref = ref->next.get()) { + for (const ::App::StringIDRef& sid : ref->sids) { + if (sid.isFromSameHasher(hasherRef)) { sid.mark(); + } } } } - for (auto& vv : v.second.children) { - if (vv.second.elementMap) - vv.second.elementMap->beforeSave(hasher); - for (auto& sid : vv.second.sids) { - if (sid.isFromSameHasher(hasher)) + for (auto& childPair : indexedName.second.children) { + if (childPair.second.elementMap) { + childPair.second.elementMap->beforeSave(hasherRef); + } + for (auto& sid : childPair.second.sids) { + if (sid.isFromSameHasher(hasherRef)) { sid.mark(); + } } } } } -void ElementMap::save(std::ostream& s, int index, +void ElementMap::save(std::ostream& stream, int index, const std::map& childMapSet, const std::map& postfixMap) const { - s << "\nElementMap " << index << ' ' << this->_id << ' ' << this->indexedNames.size() << '\n'; + stream << "\nElementMap " << index << ' ' << this->_id << ' ' << this->indexedNames.size() + << '\n'; - for (auto& v : this->indexedNames) { - s << '\n' << v.first << '\n'; + for (auto& indexedName : this->indexedNames) { + stream << '\n' << indexedName.first << '\n'; - s << "\nChildCount " << v.second.children.size() << '\n'; - for (auto& vv : v.second.children) { + stream << "\nChildCount " << indexedName.second.children.size() << '\n'; + for (auto& vv : indexedName.second.children) { auto& child = vv.second; int mapIndex = 0; if (child.elementMap) { auto it = childMapSet.find(child.elementMap.get()); - if (it == childMapSet.end() || it->second == 0) - FC_ERR("Invalid child element map"); - else + if (it == childMapSet.end() || it->second == 0) { + FC_ERR("Invalid child element map");// NOLINT + } + else { mapIndex = it->second; + } } - s << child.indexedName.getIndex() << ' ' << child.offset << ' ' << child.count << ' ' - << child.tag << ' ' << mapIndex << ' '; - s.write(child.postfix.constData(), child.postfix.size()); - s << ' ' << '0'; + stream << child.indexedName.getIndex() << ' ' << child.offset << ' ' << child.count + << ' ' << child.tag << ' ' << mapIndex << ' '; + stream.write(child.postfix.constData(), child.postfix.size()); + stream << ' ' << '0'; for (auto& sid : child.sids) { - if (sid.isMarked()) - s << '.' << sid.value(); + if (sid.isMarked()) { + stream << '.' << sid.value(); + } } - s << '\n'; + stream << '\n'; } - s << "\nNameCount " << v.second.names.size() << '\n'; - if (v.second.names.empty()) + stream << "\nNameCount " << indexedName.second.names.size() << '\n'; + if (indexedName.second.names.empty()) { continue; + } - boost::io::ios_flags_saver ifs(s); - s << std::hex; + boost::io::ios_flags_saver ifs(stream); + stream << std::hex; - for (auto& ref : v.second.names) { - for (auto r = &ref; r; r = r->next.get()) { - if (!r->name) + for (auto& dequeueOfMappedNameRef : indexedName.second.names) { + for (auto ref = &dequeueOfMappedNameRef; ref; ref = ref->next.get()) { + if (!ref->name) { break; + } - ::App::StringID::IndexID prefixid; - prefixid.id = 0; - IndexedName idx(r->name.dataBytes()); + ::App::StringID::IndexID prefixID {}; + prefixID.id = 0; + IndexedName idx(ref->name.dataBytes()); bool printName = true; if (idx) { - auto key = QByteArray::fromRawData(idx.getType(), qstrlen(idx.getType())); + auto key = QByteArray::fromRawData(idx.getType(), + static_cast(qstrlen(idx.getType()))); auto it = postfixMap.find(key); if (it != postfixMap.end()) { - s << ':' << it->second << '.' << idx.getIndex(); + stream << ':' << it->second << '.' << idx.getIndex(); printName = false; } } else { - prefixid = ::App::StringID::fromString(r->name.dataBytes()); - if (prefixid.id) { - for (auto& sid : r->sids) { - if (sid.isMarked() && sid.value() == prefixid.id) { - s << '$'; - s.write(r->name.dataBytes().constData(), - r->name.dataBytes().size()); + prefixID = ::App::StringID::fromString(ref->name.dataBytes()); + if (prefixID.id != 0) { + for (auto& sid : ref->sids) { + if (sid.isMarked() && sid.value() == prefixID.id) { + stream << '$'; + stream.write(ref->name.dataBytes().constData(), + ref->name.dataBytes().size()); printName = false; break; } } - if (printName) - prefixid.id = 0; + if (printName) { + prefixID.id = 0; + } } } if (printName) { - s << ';'; - s.write(r->name.dataBytes().constData(), r->name.dataBytes().size()); + stream << ';'; + stream.write(ref->name.dataBytes().constData(), ref->name.dataBytes().size()); } - const QByteArray& postfix = r->name.postfixBytes(); - if (postfix.isEmpty()) - s << ".0"; + const QByteArray& postfix = ref->name.postfixBytes(); + if (postfix.isEmpty()) { + stream << ".0"; + } else { auto it = postfixMap.find(postfix); assert(it != postfixMap.end()); - s << '.' << it->second; + stream << '.' << it->second; } - for (auto& sid : r->sids) { - if (sid.isMarked() && sid.value() != prefixid.id) - s << '.' << sid.value(); + for (auto& sid : ref->sids) { + if (sid.isMarked() && sid.value() != prefixID.id) { + stream << '.' << sid.value(); + } } - s << ' '; + stream << ' '; } - s << "0\n"; + stream << "0\n"; } } - s << "\nEndMap\n"; + stream << "\nEndMap\n"; } -void ElementMap::save(std::ostream& s) const +void ElementMap::save(std::ostream& stream) const { std::map childMapSet; std::vector childMaps; @@ -201,72 +218,79 @@ void ElementMap::save(std::ostream& s) const collectChildMaps(childMapSet, childMaps, postfixMap, postfixes); - s << this->_id << " PostfixCount " << postfixes.size() << '\n'; - for (auto& p : postfixes) { - s.write(p.constData(), p.size()); - s << '\n'; + stream << this->_id << " PostfixCount " << postfixes.size() << '\n'; + for (auto& postfix : postfixes) { + stream.write(postfix.constData(), postfix.size()); + stream << '\n'; } int index = 0; - s << "\nMapCount " << childMaps.size() << '\n'; - for (auto& elementMap : childMaps) - elementMap->save(s, ++index, childMapSet, postfixMap); + stream << "\nMapCount " << childMaps.size() << '\n'; + for (auto& elementMap : childMaps) { + elementMap->save(stream, ++index, childMapSet, postfixMap); + } } -ElementMapPtr ElementMap::restore(::App::StringHasherRef hasher, std::istream& s) +ElementMapPtr ElementMap::restore(::App::StringHasherRef hasherRef, std::istream& stream) { const char* msg = "Invalid element map"; - unsigned id; + unsigned id = 0; int count = 0; std::string tmp; - if (!(s >> id >> tmp >> count) || tmp != "PostfixCount") - FC_THROWM(Base::RuntimeError, msg); + if (!(stream >> id >> tmp >> count) || tmp != "PostfixCount") { + FC_THROWM(Base::RuntimeError, msg);// NOLINT + } - auto& map = _IdToElementMap[id]; - if (map) + auto& map = _idToElementMap[id]; + if (map) { return map; + } std::vector postfixes; postfixes.reserve(count); for (int i = 0; i < count; ++i) { postfixes.emplace_back(); - s >> postfixes.back(); + stream >> postfixes.back(); } std::vector childMaps; count = 0; - if (!(s >> tmp >> count) || tmp != "MapCount" || count == 0) - FC_THROWM(Base::RuntimeError, msg); + if (!(stream >> tmp >> count) || tmp != "MapCount" || count == 0) { + FC_THROWM(Base::RuntimeError, msg);// NOLINT + } childMaps.reserve(count - 1); for (int i = 0; i < count - 1; ++i) { childMaps.push_back( - std::make_shared()->restore(hasher, s, childMaps, postfixes)); + std::make_shared()->restore(hasherRef, stream, childMaps, postfixes)); } - return restore(hasher, s, childMaps, postfixes); + return restore(hasherRef, stream, childMaps, postfixes); } -ElementMapPtr ElementMap::restore(::App::StringHasherRef hasher, std::istream& s, +ElementMapPtr ElementMap::restore(::App::StringHasherRef hasherRef, std::istream& stream, std::vector& childMaps, const std::vector& postfixes) { const char* msg = "Invalid element map"; + const int hexBase {16}; + const int decBase {10}; std::string tmp; int index = 0; int typeCount = 0; unsigned id = 0; - if (!(s >> tmp >> index >> id >> typeCount) || tmp != "ElementMap") - FC_THROWM(Base::RuntimeError, msg); + if (!(stream >> tmp >> index >> id >> typeCount) || tmp != "ElementMap") { + FC_THROWM(Base::RuntimeError, msg);// NOLINT + } - auto& map = _IdToElementMap[id]; + auto& map = _idToElementMap[id]; if (map) { - do { - if (!std::getline(s, tmp)) - FC_THROWM(Base::RuntimeError, "unexpected end of child element map"); - } while (tmp != "EndMap"); + while (tmp != "EndMap") { + if (!std::getline(stream, tmp)) { + FC_THROWM(Base::RuntimeError, "unexpected end of child element map");// NOLINT + } + } return map; } - map = shared_from_this(); //FIXME does nothing? const char* hasherWarn = nullptr; const char* hasherIDWarn = nullptr; @@ -275,164 +299,193 @@ ElementMapPtr ElementMap::restore(::App::StringHasherRef hasher, std::istream& s std::vector tokens; for (int i = 0; i < typeCount; ++i) { - int count; - if (!(s >> tmp)) - FC_THROWM(Base::RuntimeError, "missing element type"); + int outerCount = 0; + if (!(stream >> tmp)) { + FC_THROWM(Base::RuntimeError, "missing element type");// NOLINT + } IndexedName idx(tmp.c_str(), 1); - if (!(s >> tmp >> count) || tmp != "ChildCount") - FC_THROWM(Base::RuntimeError, "missing element child count"); + if (!(stream >> tmp >> outerCount) || tmp != "ChildCount") { + FC_THROWM(Base::RuntimeError, "missing element child count");// NOLINT + } auto& indices = this->indexedNames[idx.getType()]; - for (int j = 0; j < count; ++j) { - int cindex; - int offset; - int count; - long tag; - int mapIndex; - if (!(s >> cindex >> offset >> count >> tag >> mapIndex >> tmp)) - FC_THROWM(Base::RuntimeError, "Invalid element child"); - if (cindex < 0) - FC_THROWM(Base::RuntimeError, "Invalid element child index"); - if (offset < 0) - FC_THROWM(Base::RuntimeError, "Invalid element child offset"); - if (mapIndex >= index || mapIndex < 0 || mapIndex > (int)childMaps.size()) - FC_THROWM(Base::RuntimeError, "Invalid element child map index"); - auto& child = indices.children[cindex + offset + count]; - child.indexedName = IndexedName::fromConst(idx.getType(), cindex); + for (int j = 0; j < outerCount; ++j) { + int cIndex = 0; + int offset = 0; + int count = 0; + long tag = 0; + int mapIndex = 0; + if (!(stream >> cIndex >> offset >> count >> tag >> mapIndex >> tmp)) { + FC_THROWM(Base::RuntimeError, "Invalid element child");// NOLINT + } + if (cIndex < 0) { + FC_THROWM(Base::RuntimeError, "Invalid element child index");// NOLINT + } + if (offset < 0) { + FC_THROWM(Base::RuntimeError, "Invalid element child offset");// NOLINT + } + if (mapIndex >= index || mapIndex < 0 || mapIndex > (int)childMaps.size()) { + FC_THROWM(Base::RuntimeError, "Invalid element child map index");// NOLINT + } + auto& child = indices.children[cIndex + offset + count]; + child.indexedName = IndexedName::fromConst(idx.getType(), cIndex); child.offset = offset; child.count = count; child.tag = tag; - if (mapIndex > 0) + if (mapIndex > 0) { child.elementMap = childMaps[mapIndex - 1]; - else + } + else { child.elementMap = nullptr; + } child.postfix = tmp.c_str(); this->childElements[child.postfix].childMap = &child; this->childElementSize += child.count; - if (!(s >> tmp)) - FC_THROWM(Base::RuntimeError, "Invalid element child string id"); + if (!(stream >> tmp)) { + FC_THROWM(Base::RuntimeError, "Invalid element child string id");// NOLINT + } tokens.clear(); boost::split(tokens, tmp, boost::is_any_of(".")); if (tokens.size() > 1) { - child.sids.reserve(tokens.size() - 1); + child.sids.reserve(static_cast(tokens.size()) - 1); for (unsigned k = 1; k < tokens.size(); ++k) { // The element child string ID is saved as decimal // instead of hex by accident. To simplify maintenance // of backward compatibility, it is not corrected, and // just restored as decimal here. - // - // long n = strtol(tokens[k].c_str(), nullptr, 16); - long n = strtol(tokens[k].c_str(), nullptr, 10); - auto sid = hasher->getID(n); - if (!sid) + long childID = strtol(tokens[k].c_str(), nullptr, decBase); + auto sid = hasherRef->getID(childID); + if (!sid) { childSIDWarn = "Missing element child string id"; - else + } + else { child.sids.push_back(sid); + } } } } - if (!(s >> tmp >> count) || tmp != "NameCount") - FC_THROWM(Base::RuntimeError, "missing element name count"); + if (!(stream >> tmp >> outerCount) || tmp != "NameCount") { + FC_THROWM(Base::RuntimeError, "missing element name outerCount");// NOLINT + } - boost::io::ios_flags_saver ifs(s); - s >> std::hex; + boost::io::ios_flags_saver ifs(stream); + stream >> std::hex; - indices.names.resize(count); - for (int j = 0; j < count; ++j) { + indices.names.resize(outerCount); + for (int j = 0; j < outerCount; ++j) { idx.setIndex(j); auto* ref = &indices.names[j]; - int k = 0; - while (1) { - if (!(s >> tmp)) - FC_THROWM(Base::RuntimeError, "Failed to read element name"); - if (tmp == "0") + int innerCount = 0; + while (true) { + if (!(stream >> tmp)) { + FC_THROWM(Base::RuntimeError, "Failed to read element name");// NOLINT + } + if (tmp == "0") { break; - if (k++ != 0) { - ref->next.reset(new MappedNameRef); + } + if (innerCount++ != 0) { + ref->next = std::make_unique(); ref = ref->next.get(); } tokens.clear(); boost::split(tokens, tmp, boost::is_any_of(".")); - if (tokens.size() < 2) - FC_THROWM(Base::RuntimeError, "Invalid element entry"); + if (tokens.size() < 2) { + FC_THROWM(Base::RuntimeError, "Invalid element entry");// NOLINT + } int offset = 1; - ::App::StringID::IndexID prefixid; - prefixid.id = 0; + ::App::StringID::IndexID prefixID {}; + prefixID.id = 0; switch (tokens[0][0]) { case ':': { - if (tokens.size() < 3) - FC_THROWM(Base::RuntimeError, "Invalid element entry"); + if (tokens.size() < 3) { + FC_THROWM(Base::RuntimeError, "Invalid element entry");// NOLINT + } ++offset; - long n = strtol(tokens[0].c_str() + 1, nullptr, 16); - if (n <= 0 || n > (int)postfixes.size()) - FC_THROWM(Base::RuntimeError, "Invalid element name index"); - long m = strtol(tokens[1].c_str(), nullptr, 16); - ref->name = MappedName(IndexedName::fromConst(postfixes[n - 1].c_str(), m)); + long elementNameIndex = strtol(tokens[0].c_str() + 1, nullptr, hexBase); + if (elementNameIndex <= 0 || elementNameIndex > (int)postfixes.size()) { + FC_THROWM(Base::RuntimeError, "Invalid element name index");// NOLINT + } + long elementIndex = strtol(tokens[1].c_str(), nullptr, hexBase); + ref->name = MappedName( + IndexedName::fromConst(postfixes[elementNameIndex - 1].c_str(), + static_cast(elementIndex))); break; } case '$': ref->name = MappedName(tokens[0].c_str() + 1); - prefixid = ::App::StringID::fromString(ref->name.dataBytes()); + prefixID = ::App::StringID::fromString(ref->name.dataBytes()); break; case ';': ref->name = MappedName(tokens[0].c_str() + 1); break; default: - FC_THROWM(Base::RuntimeError, "Invalid element name marker"); + FC_THROWM(Base::RuntimeError, "Invalid element name marker");// NOLINT } if (tokens[offset] != "0") { - long n = strtol(tokens[offset].c_str(), nullptr, 16); - if (n <= 0 || n > (int)postfixes.size()) + long postfixIndex = strtol(tokens[offset].c_str(), nullptr, hexBase); + if (postfixIndex <= 0 || postfixIndex > (int)postfixes.size()) { postfixWarn = "Invalid element postfix index"; - else - ref->name += postfixes[n - 1]; + } + else { + ref->name += postfixes[postfixIndex - 1]; + } } this->mappedNames.emplace(ref->name, idx); - if (!hasher) { - if (offset + 1 < (int)tokens.size()) - hasherWarn = "No hasher"; + if (!hasherRef) { + if (offset + 1 < (int)tokens.size()) { + hasherWarn = "No hasherRef"; + } continue; } - ref->sids.reserve(tokens.size() - offset - 1 + prefixid.id ? 1 : 0); - if (prefixid.id) { - auto sid = hasher->getID(prefixid.id); - if (!sid) + ref->sids.reserve((tokens.size() - offset - 1 + prefixID.id) != 0U ? 1 : 0); + if (prefixID.id != 0) { + auto sid = hasherRef->getID(prefixID.id); + if (!sid) { hasherIDWarn = "Missing element name prefix id"; - else + } + else { ref->sids.push_back(sid); + } } for (int l = offset + 1; l < (int)tokens.size(); ++l) { - long id = strtol(tokens[l].c_str(), nullptr, 16); - auto sid = hasher->getID(id); - if (!sid) + long readID = strtol(tokens[l].c_str(), nullptr, hexBase); + auto sid = hasherRef->getID(readID); + if (!sid) { hasherIDWarn = "Invalid element name string id"; - else + } + else { ref->sids.push_back(sid); + } } } } } - if (hasherWarn) - FC_WARN(hasherWarn); - if (hasherIDWarn) - FC_WARN(hasherIDWarn); - if (postfixWarn) - FC_WARN(postfixWarn); - if (childSIDWarn) - FC_WARN(childSIDWarn); + if (hasherWarn) { + FC_WARN(hasherWarn);// NOLINT + } + if (hasherIDWarn) { + FC_WARN(hasherIDWarn);// NOLINT + } + if (postfixWarn) { + FC_WARN(postfixWarn);// NOLINT + } + if (childSIDWarn) { + FC_WARN(childSIDWarn);// NOLINT + } - if (!(s >> tmp) || tmp != "EndMap") - FC_THROWM(Base::RuntimeError, "unexpected end of child element map"); + if (!(stream >> tmp) || tmp != "EndMap") { + FC_THROWM(Base::RuntimeError, "unexpected end of child element map");// NOLINT + } return shared_from_this(); } @@ -442,38 +495,41 @@ MappedName ElementMap::addName(MappedName& name, const IndexedName& idx, const E { if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) { if (name.find("#") >= 0 && name.findTagInElementName() < 0) { - FC_ERR("missing tag postfix " << name); + FC_ERR("missing tag postfix " << name);// NOLINT } } - do { - if (overwrite) + while (true) { + if (overwrite) { erase(idx); + } auto ret = mappedNames.insert(std::make_pair(name, idx)); if (ret.second) { // element just inserted did not exist yet in the map ret.first->first.compact();// FIXME see MappedName.cpp mappedRef(idx).append(ret.first->first, sids); - FC_TRACE(idx << " -> " << name); + FC_TRACE(idx << " -> " << name);// NOLINT return ret.first->first; } if (ret.first->second == idx) { - FC_TRACE("duplicate " << idx << " -> " << name); + FC_TRACE("duplicate " << idx << " -> " << name);// NOLINT return ret.first->first; } if (!overwrite) { - if (existing) + if (existing) { *existing = ret.first->second; - return MappedName(); + } + return {}; } erase(ret.first->first); - } while (true); + }; } void ElementMap::addPostfix(const QByteArray& postfix, std::map& postfixMap, std::vector& postfixes) { - if (postfix.isEmpty()) + if (postfix.isEmpty()) { return; + } auto res = postfixMap.insert(std::make_pair(postfix, 0)); if (res.second) { postfixes.push_back(postfix); @@ -481,51 +537,57 @@ void ElementMap::addPostfix(const QByteArray& postfix, std::map } } -MappedName ElementMap::setElementName(const IndexedName& element, - const MappedName& name, - long masterTag, - const ElementIDRefs* sid, - bool overwrite) +MappedName ElementMap::setElementName(const IndexedName& element, const MappedName& name, + long masterTag, const ElementIDRefs* sid, bool overwrite) { - if (!element) + if (!element) { throw Base::ValueError("Invalid input"); + } if (!name) { erase(element); - return MappedName(); + return {}; } for (int i = 0, count = name.size(); i < count; ++i) { - char c = name[i]; - if (c == '.' || std::isspace((int)c)) - FC_THROWM(Base::RuntimeError, "Illegal character in mapped name: " << name); + char check = name[i]; + if (check == '.' || (std::isspace((int)check) != 0)) { + FC_THROWM(Base::RuntimeError, "Illegal character in mapped name: " << name);// NOLINT + } } - for (const char* s = element.getType(); *s; ++s) { - char c = *s; - if (c == '.' || std::isspace((int)c)) - FC_THROWM(Base::RuntimeError, "Illegal character in element name: " << element); + for (const char* readChar = element.getType(); *readChar != 0; ++readChar) { + char check = *readChar; + if (check == '.' || (std::isspace((int)check) != 0)) { + FC_THROWM(Base::RuntimeError,// NOLINT + "Illegal character in element name: " << element); + } } ElementIDRefs _sid; - if (!sid) + if (!sid) { sid = &_sid; + } std::ostringstream ss; - Data::MappedName n(name); + Data::MappedName mappedName(name); for (int i = 0;;) { IndexedName existing; - MappedName res = this->addName(n, element, *sid, overwrite, &existing); - if (res) + MappedName res = this->addName(mappedName, element, *sid, overwrite, &existing); + if (res) { return res; - if (++i == 100) { - FC_ERR("unresolved duplicate element mapping '" << name << ' ' << element << '/' - << existing); + } + const int maxAttempts {100}; + if (++i == maxAttempts) { + FC_ERR("unresolved duplicate element mapping '"// NOLINT + << name << ' ' << element << '/' << existing); return name; } - if (sid != &_sid) + if (sid != &_sid) { _sid = *sid; - n = renameDuplicateElement(i, element, existing, name, _sid, masterTag); - if (!n) + } + mappedName = renameDuplicateElement(i, element, existing, name, _sid, masterTag); + if (!mappedName) { return name; + } sid = &_sid; } } @@ -535,60 +597,68 @@ void ElementMap::encodeElementName(char element_type, MappedName& name, std::ost ElementIDRefs* sids, long masterTag, const char* postfix, long tag, bool forceTag) const { - if (postfix && postfix[0]) { - if (!boost::starts_with(postfix, ELEMENT_MAP_PREFIX)) + if (postfix && (postfix[0] != 0)) { + if (!boost::starts_with(postfix, ELEMENT_MAP_PREFIX)) { ss << ELEMENT_MAP_PREFIX; + } ss << postfix; } long inputTag = 0; - if (!forceTag && !ss.tellp()) { - if (!tag || tag == masterTag) + if (!forceTag && (ss.tellp() == 0)) { + if ((tag == 0) || tag == masterTag) { return; + } name.findTagInElementName(&inputTag, nullptr, nullptr, nullptr, true); - if (inputTag == tag) + if (inputTag == tag) { return; + } } - else if (!tag || (!forceTag && tag == masterTag)) { + else if ((tag == 0) || (!forceTag && tag == masterTag)) { int pos = name.findTagInElementName(&inputTag, nullptr, nullptr, nullptr, true); - if (inputTag) { + if (inputTag != 0) { tag = inputTag; // About to encode the same tag used last time. This usually means - // the owner object is doing multi step modeling. Let's not - // recursively encode the same tag too many time. It will be a + // the owner object is doing multistep modeling. Let's not + // recursively encode the same tag too many times. It will be a // waste of memory, because the intermediate shapes has no // corresponding objects, so no real value for history tracing. // // On the other hand, we still need to distinguish the original name // from the input object from the element name of the intermediate // shapes. So we limit ourselves to encode only one extra level - // using the same tag. In order to do that, we need to dehash the + // using the same tag. In order to do that, we need to de-hash the // previous level name, and check for its tag. - Data::MappedName n(name, 0, pos); - Data::MappedName prev = dehashElementName(n); + Data::MappedName mappedName(name, 0, pos); + Data::MappedName prev = dehashElementName(mappedName); long prevTag = 0; prev.findTagInElementName(&prevTag, nullptr, nullptr, nullptr, true); - if (prevTag == inputTag || prevTag == -inputTag) - name = n; + if (prevTag == inputTag || prevTag == -inputTag) { + name = mappedName; + } } } if (sids && this->hasher) { name = hashElementName(name, *sids); - if (!forceTag && !tag && ss.tellp()) + if (!forceTag && (tag == 0) && (ss.tellp() != 0)) { forceTag = true; + } } - if (forceTag || tag) { + if (forceTag || (tag != 0)) { assert(element_type); - int pos = ss.tellp(); + auto pos = ss.tellp(); boost::io::ios_flags_saver ifs(ss); ss << POSTFIX_TAG << std::hex; - if (tag < 0) + if (tag < 0) { ss << '-' << -tag; - else if (tag) + } + else if (tag != 0) { ss << tag; + } assert(pos >= 0); - if (pos != 0) + if (pos != 0) { ss << ':' << pos; + } ss << ',' << element_type; } name += ss.str(); @@ -596,10 +666,12 @@ void ElementMap::encodeElementName(char element_type, MappedName& name, std::ost MappedName ElementMap::hashElementName(const MappedName& name, ElementIDRefs& sids) const { - if (!this->hasher || !name) + if (!this->hasher || !name) { return name; - if (name.find(ELEMENT_MAP_PREFIX) < 0) + } + if (name.find(ELEMENT_MAP_PREFIX) < 0) { return name; + } App::StringIDRef sid = this->hasher->getID(name, sids); const auto& related = sid.relatedIDs(); if (related == sids) { @@ -609,9 +681,10 @@ MappedName ElementMap::hashElementName(const MappedName& name, ElementIDRefs& si else { ElementIDRefs tmp; tmp.push_back(sid); - for (auto& s : sids) { - if (related.indexOf(s) < 0) - tmp.push_back(s); + for (auto& checkSID : sids) { + if (related.indexOf(checkSID) < 0) { + tmp.push_back(checkSID); + } } sids = tmp; } @@ -620,42 +693,47 @@ MappedName ElementMap::hashElementName(const MappedName& name, ElementIDRefs& si MappedName ElementMap::dehashElementName(const MappedName& name) const { - if (name.empty()) + if (name.empty()) { return name; - if (!this->hasher) + } + if (!this->hasher) { return name; + } auto id = App::StringID::fromString(name.toRawBytes()); - if (!id) + if (!id) { return name; + } auto sid = this->hasher->getID(id); if (!sid) { - if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_TRACE)) - FC_WARN("failed to find hash id " << id); - else - FC_LOG("failed to find hash id " << id); + if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_TRACE)) { + FC_WARN("failed to find hash id " << id);// NOLINT + } + else { + FC_LOG("failed to find hash id " << id);// NOLINT + } return name; } if (sid.isHashed()) { - FC_LOG("cannot dehash id " << id); + FC_LOG("cannot de-hash id " << id);// NOLINT return name; } MappedName ret( sid.toString());// FIXME .toString() was missing in original function. is this correct? - FC_TRACE("dehash " << name << " -> " << ret); + FC_TRACE("de-hash " << name << " -> " << ret);// NOLINT return ret; } MappedName ElementMap::renameDuplicateElement(int index, const IndexedName& element, const IndexedName& element2, const MappedName& name, - ElementIDRefs& sids, long masterTag) + ElementIDRefs& sids, long masterTag) const { - int idx; + int idx {0}; #ifdef FC_DEBUG idx = index; #else static std::random_device _RD; static std::mt19937 _RGEN(_RD()); - static std::uniform_int_distribution<> _RDIST(1,10000); + static std::uniform_int_distribution<> _RDIST(1, 10000); (void)index; idx = _RDIST(_RGEN); #endif @@ -663,38 +741,42 @@ MappedName ElementMap::renameDuplicateElement(int index, const IndexedName& elem ss << ELEMENT_MAP_PREFIX << 'D' << std::hex << idx; MappedName renamed(name); encodeElementName(element.getType()[0], renamed, ss, &sids, masterTag); - if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) - FC_WARN("duplicate element mapping '" << name << " -> " << renamed << ' ' << element << '/' - << element2); + if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) { + FC_WARN("duplicate element mapping '"// NOLINT + << name << " -> " << renamed << ' ' << element << '/' << element2); + } return renamed; } void ElementMap::erase(const MappedName& name) { auto it = this->mappedNames.find(name); - if (it == this->mappedNames.end()) + if (it == this->mappedNames.end()) { return; + } MappedNameRef* ref = findMappedRef(it->second); - if (!ref) + if (!ref) { return; + } ref->erase(name); this->mappedNames.erase(it); - return; } void ElementMap::erase(const IndexedName& idx) { auto iter = this->indexedNames.find(idx.getType()); - if (iter == this->indexedNames.end()) + if (iter == this->indexedNames.end()) { return; + } auto& indices = iter->second; - if (idx.getIndex() >= (int)indices.names.size()) + if (idx.getIndex() >= (int)indices.names.size()) { return; + } auto& ref = indices.names[idx.getIndex()]; - for (auto* r = &ref; r; r = r->next.get()) - this->mappedNames.erase(r->name); + for (auto* nameRef = &ref; nameRef; nameRef = nameRef->next.get()) { + this->mappedNames.erase(nameRef->name); + } ref.clear(); - return; } unsigned long ElementMap::size() const @@ -709,27 +791,32 @@ bool ElementMap::empty() const IndexedName ElementMap::find(const MappedName& name, ElementIDRefs* sids) const { - auto it = mappedNames.find(name); - if (it == mappedNames.end()) { - if (childElements.isEmpty()) + auto nameIter = mappedNames.find(name); + if (nameIter == mappedNames.end()) { + if (childElements.isEmpty()) { return IndexedName(); + } int len = 0; - if (name.findTagInElementName(nullptr, &len, nullptr, nullptr, false, false) < 0) + if (name.findTagInElementName(nullptr, &len, nullptr, nullptr, false, false) < 0) { return IndexedName(); + } QByteArray key = name.toRawBytes(len); auto it = this->childElements.find(key); - if (it == this->childElements.end()) + if (it == this->childElements.end()) { return IndexedName(); + } const auto& child = *it.value().childMap; IndexedName res; MappedName childName = MappedName::fromRawData(name, 0, len); - if (child.elementMap) + if (child.elementMap) { res = child.elementMap->find(childName, sids); - else + } + else { res = childName.toIndexedName(); + } if (res && boost::equals(res.getType(), child.indexedName.getType()) && child.indexedName.getIndex() <= res.getIndex() @@ -742,38 +829,44 @@ IndexedName ElementMap::find(const MappedName& name, ElementIDRefs* sids) const } if (sids) { - const MappedNameRef* ref = findMappedRef(it->second); + const MappedNameRef* ref = findMappedRef(nameIter->second); for (; ref; ref = ref->next.get()) { if (ref->name == name) { - if (!sids->size()) + if (sids->empty()) { *sids = ref->sids; - else + } + else { *sids += ref->sids; + } break; } } } - return it->second; + return nameIter->second; } MappedName ElementMap::find(const IndexedName& idx, ElementIDRefs* sids) const { - if (!idx) + if (!idx) { return MappedName(); + } auto iter = this->indexedNames.find(idx.getType()); - if (iter == this->indexedNames.end()) + if (iter == this->indexedNames.end()) { return MappedName(); + } auto& indices = iter->second; if (idx.getIndex() < (int)indices.names.size()) { const MappedNameRef& ref = indices.names[idx.getIndex()]; if (ref.name) { if (sids) { - if (!sids->size()) + if (!sids->size()) { *sids = ref.sids; - else + } + else { *sids += ref.sids; + } } return ref.name; } @@ -785,41 +878,47 @@ MappedName ElementMap::find(const IndexedName& idx, ElementIDRefs* sids) const auto& child = it->second; MappedName name; IndexedName childIdx(idx.getType(), idx.getIndex() - child.offset); - if (child.elementMap) + if (child.elementMap) { name = child.elementMap->find(childIdx, sids); - else + } + else { name = MappedName(childIdx); + } if (name) { name += child.postfix; return name; } } - return MappedName(); + return {}; } std::vector> ElementMap::findAll(const IndexedName& idx) const { std::vector> res; - if (!idx) + if (!idx) { return res; + } auto iter = this->indexedNames.find(idx.getType()); - if (iter == this->indexedNames.end()) + if (iter == this->indexedNames.end()) { return res; + } auto& indices = iter->second; if (idx.getIndex() < (int)indices.names.size()) { const MappedNameRef& ref = indices.names[idx.getIndex()]; int count = 0; - for (auto r = &ref; r; r = r->next.get()) { - if (r->name) + for (auto nameRef = &ref; nameRef; nameRef = nameRef->next.get()) { + if (nameRef->name) { ++count; + } } - if (count) { + if (count != 0) { res.reserve(count); - for (auto r = &ref; r; r = r->next.get()) { - if (r->name) - res.emplace_back(r->name, r->sids); + for (auto nameRef = &ref; nameRef; nameRef = nameRef->next.get()) { + if (nameRef->name) { + res.emplace_back(nameRef->name, nameRef->sids); + } } return res; } @@ -832,11 +931,13 @@ std::vector> ElementMap::findAll(const Inde IndexedName childIdx(idx.getType(), idx.getIndex() - child.offset); if (child.elementMap) { res = child.elementMap->findAll(childIdx); - for (auto& v : res) + for (auto& v : res) { v.first += child.postfix; + } } - else + else { res.emplace_back(MappedName(childIdx) + child.postfix, ElementIDRefs()); + } } return res; @@ -845,22 +946,26 @@ std::vector> ElementMap::findAll(const Inde const MappedNameRef* ElementMap::findMappedRef(const IndexedName& idx) const { auto iter = this->indexedNames.find(idx.getType()); - if (iter == this->indexedNames.end()) + if (iter == this->indexedNames.end()) { return nullptr; + } auto& indices = iter->second; - if (idx.getIndex() >= (int)indices.names.size()) + if (idx.getIndex() >= (int)indices.names.size()) { return nullptr; + } return &indices.names[idx.getIndex()]; } MappedNameRef* ElementMap::findMappedRef(const IndexedName& idx) { auto iter = this->indexedNames.find(idx.getType()); - if (iter == this->indexedNames.end()) + if (iter == this->indexedNames.end()) { return nullptr; + } auto& indices = iter->second; - if (idx.getIndex() >= (int)indices.names.size()) + if (idx.getIndex() >= (int)indices.names.size()) { return nullptr; + } return &indices.names[idx.getIndex()]; } @@ -868,8 +973,9 @@ MappedNameRef& ElementMap::mappedRef(const IndexedName& idx) { assert(idx); auto& indices = this->indexedNames[idx.getType()]; - if (idx.getIndex() >= (int)indices.names.size()) + if (idx.getIndex() >= (int)indices.names.size()) { indices.names.resize(idx.getIndex() + 1); + } return indices.names[idx.getIndex()]; } @@ -880,16 +986,18 @@ bool ElementMap::hasChildElementMap() const void ElementMap::hashChildMaps(long masterTag) { - if (childElements.empty() || !this->hasher) + if (childElements.empty() || !this->hasher) { return; + } std::ostringstream ss; for (auto& indexedNameIndexedElements : this->indexedNames) { for (auto& indexedChild : indexedNameIndexedElements.second.children) { auto& child = indexedChild.second; int len = 0; - long tag; + long tag = 0; int pos = MappedName::fromRawData(child.postfix) .findTagInElementName(&tag, &len, nullptr, nullptr, false, false); + // TODO: What is this 10? if (pos > 10) { MappedName postfix = hashElementName( MappedName::fromRawData(child.postfix.constData(), pos), child.sids); @@ -912,21 +1020,27 @@ void ElementMap::collectChildMaps(std::map& childMapSet, std::vector& postfixes) const { auto res = childMapSet.insert(std::make_pair(this, 0)); - if (!res.second) + if (!res.second) { return; + } - for (auto& v : this->indexedNames) { - addPostfix(QByteArray::fromRawData(v.first, qstrlen(v.first)), postfixMap, postfixes); + for (auto& indexedName : this->indexedNames) { + addPostfix(QByteArray::fromRawData(indexedName.first, + static_cast(qstrlen(indexedName.first))), + postfixMap, + postfixes); - for (auto& vv : v.second.children) { - auto& child = vv.second; - if (child.elementMap) + for (auto& childPair : indexedName.second.children) { + auto& child = childPair.second; + if (child.elementMap) { child.elementMap->collectChildMaps(childMapSet, childMaps, postfixMap, postfixes); + } } } - for (auto& v : this->mappedNames) - addPostfix(v.first.constPostfix(), postfixMap, postfixes); + for (auto& mappedName : this->mappedNames) { + addPostfix(mappedName.first.constPostfix(), postfixMap, postfixes); + } childMaps.push_back(this); res.first->second = (int)childMaps.size(); @@ -943,18 +1057,20 @@ void ElementMap::addChildElements(long masterTag, const std::vectorchildElements.empty()) { - if (expansion.size()) + if (!expansion.empty()) { expansion.push_back(child); + } continue; } auto& indices = child.elementMap->indexedNames[child.indexedName.getType()]; if (indices.children.empty()) { - if (expansion.size()) + if (!expansion.empty()) { expansion.push_back(child); + } continue; } - // Note that it is allow to have both mapped names and child map. We + // Note that it is allowable to have both mapped names and child map. We // may have to split the current child mapping into pieces. int start = child.indexedName.getIndex(); @@ -964,10 +1080,11 @@ void ElementMap::addChildElements(long masterTag, const std::vectorsecond; int istart = grandchild.indexedName.getIndex() + grandchild.offset; int iend = istart + grandchild.count; - if (end <= istart) + if (end <= istart) { break; + } if (istart >= end) { - if (expansion.size()) { + if (!expansion.empty()) { expansion.push_back(child); expansion.back().indexedName.setIndex(start); expansion.back().count = end - start; @@ -975,7 +1092,8 @@ void ElementMap::addChildElements(long masterTag, const std::vector end) + if (iend > end) { iend = end; + } entry->indexedName.setIndex(istart - grandchild.offset); entry->count = iend - istart; entry->offset += grandchild.offset; entry->elementMap = grandchild.elementMap; entry->sids += grandchild.sids; - if (grandchild.postfix.size()) { - if (entry->postfix.size() + if (grandchild.postfix.size() != 0) { + if ((entry->postfix.size() != 0) && !entry->postfix.startsWith(ELEMENT_MAP_PREFIX)) { - entry->postfix = - grandchild.postfix + ELEMENT_MAP_PREFIX + entry->postfix; + entry->postfix = grandchild.postfix + ELEMENT_MAP_PREFIX + entry->postfix; } - else + else { entry->postfix = grandchild.postfix + entry->postfix; + } } start = iend; - if (start >= end) + if (start >= end) { break; + } } - if (expansion.size() && start < end) { + if (!expansion.empty() && start < end) { expansion.push_back(child); expansion.back().indexedName.setIndex(start); expansion.back().count = end - start; } } - for (auto& child : expansion.size() ? expansion : children) { - if (!child.indexedName || !child.count) { - if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) - FC_ERR("invalid mapped child element"); + for (auto& child : expansion.empty() ? children : expansion) { + if (!child.indexedName || (child.count == 0)) { + if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) { + FC_ERR("invalid mapped child element");// NOLINT + } continue; } @@ -1032,7 +1154,8 @@ void ElementMap::addChildElements(long masterTag, const std::vector= 5 - if (child.count >= 5 || !child.elementMap) { + const int threshold {5}; + if (child.count >= threshold || !child.elementMap) { encodeElementName(child.indexedName[0], tmp, ss, @@ -1061,9 +1184,10 @@ void ElementMap::addChildElements(long masterTag, const std::vectorfind(childIdx, &sids); if (!name) { - if (!child.tag || child.tag == masterTag) { - if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) - FC_WARN("unmapped element"); + if ((child.tag == 0) || child.tag == masterTag) { + if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) { + FC_WARN("unmapped element");// NOLINT + } continue; } name = MappedName(childIdx); @@ -1079,7 +1203,7 @@ void ElementMap::addChildElements(long masterTag, const std::vectorindex != 1) { // There is some ambiguity in child mapping. We need some // additional postfix for disambiguation. NOTE: We are not - // using ComplexGeoData::indexPostfix() so as to not confuse + // using ComplexGeoData::indexPostfix() so we don't confuse // other code that actually uses this postfix for indexing // purposes. Here, we just need some postfix for // disambiguation. We don't need to extract the index. @@ -1098,7 +1222,7 @@ void ElementMap::addChildElements(long masterTag, const std::vectorchildMap) { - FC_ERR("duplicate mapped child element"); + FC_ERR("duplicate mapped child element");// NOLINT continue; } } @@ -1107,9 +1231,10 @@ void ElementMap::addChildElements(long masterTag, const std::vectorchildMap) + if (!entry->childMap) { this->childElements.remove(tmp.toBytes()); - FC_ERR("duplicate mapped child element"); + } + FC_ERR("duplicate mapped child element");// NOLINT continue; } @@ -1124,8 +1249,9 @@ std::vector ElementMap::getChildElements() cons { std::vector res; res.reserve(this->childElements.size()); - for (auto& v : this->childElements) - res.push_back(*v.childMap); + for (auto& childElement : this->childElements) { + res.push_back(*childElement.childMap); + } return res; } @@ -1133,19 +1259,22 @@ std::vector ElementMap::getAll() const { std::vector ret; ret.reserve(size()); - for (auto& v : this->mappedNames) - ret.emplace_back(v.first, v.second); - for (auto& v : this->childElements) { - auto& child = *v.childMap; + for (auto& mappedName : this->mappedNames) { + ret.emplace_back(mappedName.first, mappedName.second); + } + for (auto& childElement : this->childElements) { + auto& child = *childElement.childMap; IndexedName idx(child.indexedName); idx.setIndex(idx.getIndex() + child.offset); IndexedName childIdx(child.indexedName); for (int i = 0; i < child.count; ++i, ++idx, ++childIdx) { MappedName name; - if (child.elementMap) + if (child.elementMap) { name = child.elementMap->find(childIdx); - else + } + else { name = MappedName(childIdx); + } if (name) { name += child.postfix; ret.emplace_back(name, idx); @@ -1155,58 +1284,62 @@ std::vector ElementMap::getAll() const return ret; } -long ElementMap::getElementHistory(const MappedName & name, - long masterTag, - MappedName *original, - std::vector *history) const +long ElementMap::getElementHistory(const MappedName& name, long masterTag, MappedName* original, + std::vector* history) const { long tag = 0; int len = 0; - int pos = name.findTagInElementName(&tag,&len,nullptr,nullptr,true); - if(pos < 0) { - if(original) + int pos = name.findTagInElementName(&tag, &len, nullptr, nullptr, true); + if (pos < 0) { + if (original) { *original = name; + } return tag; } - if(!original && !history) + if (!original && !history) { return tag; + } MappedName tmp; - MappedName &ret = original?*original:tmp; - if(name.startsWith(ELEMENT_MAP_PREFIX)) { + MappedName& ret = original ? *original : tmp; + if (name.startsWith(ELEMENT_MAP_PREFIX)) { unsigned offset = ELEMENT_MAP_PREFIX_SIZE; - ret = MappedName::fromRawData(name, offset); - } else + ret = MappedName::fromRawData(name, static_cast(offset)); + } + else { ret = name; + } - while(1) { - if(!len || len>pos) { - FC_WARN("invalid name length " << name); + while (true) { + if ((len == 0) || len > pos) { + FC_WARN("invalid name length " << name);// NOLINT return 0; } - bool dehashed = false; + bool deHashed = false; if (ret.startsWith(MAPPED_CHILD_ELEMENTS_PREFIX, len)) { int offset = (int)POSTFIX_TAG_SIZE; - MappedName tmp = MappedName::fromRawData(ret, len+offset, pos-len-offset); - MappedName postfix = dehashElementName(tmp); - if (postfix != tmp) { - dehashed = true; + MappedName tmp2 = MappedName::fromRawData(ret, len + offset, pos - len - offset); + MappedName postfix = dehashElementName(tmp2); + if (postfix != tmp2) { + deHashed = true; ret = MappedName::fromRawData(ret, 0, len) + postfix; } } - if (!dehashed) + if (!deHashed) { ret = dehashElementName(MappedName::fromRawData(ret, 0, len)); + } long tag2 = 0; - pos = ret.findTagInElementName(&tag2,&len,nullptr,nullptr,true); - if(pos < 0 || (tag2!=tag && tag2!=-tag && tag!=masterTag && -tag!=masterTag)) + pos = ret.findTagInElementName(&tag2, &len, nullptr, nullptr, true); + if (pos < 0 || (tag2 != tag && tag2 != -tag && tag != masterTag && -tag != masterTag)) { return tag; + } tag = tag2; - if(history) + if (history) { history->push_back(ret.copy()); + } } } - }// Namespace Data diff --git a/src/App/ElementMap.h b/src/App/ElementMap.h index aea3e9ff22..fd598fecd1 100644 --- a/src/App/ElementMap.h +++ b/src/App/ElementMap.h @@ -63,27 +63,27 @@ public: ElementMap(); /** Ensures that naming is properly assigned. It then marks as "used" all the StringID - * that are used to make up this particular map and are stored in the hasher passed + * that are used to make up this particular map and are stored in the hasherRef passed * as a parameter. Finally do this recursively for all childEelementMaps as well. * - * @param hasher where all the StringID needed to build the map are stored. + * @param hasherRef where all the StringID needed to build the map are stored. */ // FIXME this should be made part of \c save, to achieve symmetry with the restore method - void beforeSave(const ::App::StringHasherRef& hasher) const; + void beforeSave(const ::App::StringHasherRef& hasherRef) const; /** Serialize this map. Calls \c collectChildMaps to get \c childMapSet and * \c postfixMap, then calls the other (private) save function with those parameters. - * @param s: serialized stream + * @param stream: serialized stream */ - void save(std::ostream& s) const; + void save(std::ostream& stream) const; - /** Deserialize and restore this map. This function restores \c childMaps and + /** Deserialize and restore this map. This function restores \c childMaps and * \c postfixes from the stream, then calls the other (private) restore function with those * parameters. - * @param hasher: where all the StringIDs are stored - * @param s: stream to deserialize + * @param hasherRef: where all the StringIDs are stored + * @param stream: stream to deserialize */ - ElementMapPtr restore(::App::StringHasherRef hasher, std::istream& s); + ElementMapPtr restore(::App::StringHasherRef hasherRef, std::istream& stream); /** Add a sub-element name mapping. @@ -187,20 +187,20 @@ public: private: /** Serialize this map - * @param s: serialized stream + * @param stream: serialized stream * @param childMapSet: where all child element maps are stored * @param postfixMap. where all postfixes are stored */ - void save(std::ostream& s, int index, const std::map& childMapSet, + void save(std::ostream& stream, int index, const std::map& childMapSet, const std::map& postfixMap) const; /** Deserialize and restore this map. - * @param hasher: where all the StringIDs are stored - * @param s: stream to deserialize + * @param hasherRef: where all the StringIDs are stored + * @param stream: stream to deserialize * @param childMaps: where all child element maps are stored * @param postfixes. where all postfixes are stored */ - ElementMapPtr restore(::App::StringHasherRef hasher, std::istream& s, + ElementMapPtr restore(::App::StringHasherRef hasherRef, std::istream& stream, std::vector& childMaps, const std::vector& postfixes); @@ -224,9 +224,9 @@ private: /* Note: the original proc passed `ComplexGeoData& master` for getting the `Tag`, * now it just passes `long masterTag`.*/ - virtual MappedName renameDuplicateElement(int index, const IndexedName& element, - const IndexedName& element2, const MappedName& name, - ElementIDRefs& sids, long masterTag); + MappedName renameDuplicateElement(int index, const IndexedName& element, + const IndexedName& element2, const MappedName& name, + ElementIDRefs& sids, long masterTag) const; /** Convenience method to hash the main element name * diff --git a/src/App/MappedName.h b/src/App/MappedName.h index 54a58ea19f..9921e4280e 100644 --- a/src/App/MappedName.h +++ b/src/App/MappedName.h @@ -34,10 +34,11 @@ #include #include #include +#include +#include "ElementNamingUtils.h" #include "IndexedName.h" #include "StringHasher.h" -#include "ElementNamingUtils.h" namespace Data @@ -92,8 +93,9 @@ public: /// is appended as text to the MappedName. In that case the memory is *not* shared between the /// original IndexedName and the MappedName. explicit MappedName(const IndexedName& element) - : data(QByteArray::fromRawData(element.getType(), qstrlen(element.getType()))), - raw(true) + : data(QByteArray::fromRawData(element.getType(), + static_cast(qstrlen(element.getType())))) + , raw(true) { if (element.getIndex() > 0) { this->data += QByteArray::number(element.getIndex()); @@ -101,8 +103,8 @@ public: } } - explicit MappedName(const App::StringIDRef & sid) - :raw(false) + explicit MappedName(const App::StringIDRef& sid) + : raw(false) { sid.toBytes(this->data); } @@ -130,16 +132,16 @@ public: /// \param other The mapped name to copy. Its data and postfix become the new MappedName's data /// \param postfix The postfix for the new MappedName MappedName(const MappedName& other, const char* postfix) - : data(other.data + other.postfix), - postfix(postfix), - raw(false) + : data(other.data + other.postfix) + , postfix(postfix) + , raw(false) {} /// Move constructor MappedName(MappedName&& other) noexcept - : data(std::move(other.data)), - postfix(std::move(other.postfix)), - raw(other.raw) + : data(std::move(other.data)) + , postfix(std::move(other.postfix)) + , raw(other.raw) {} ~MappedName() = default; @@ -899,7 +901,7 @@ public: /// /// \param tag: optional pointer to receive the extracted tag /// \param len: optional pointer to receive the length field after the tag field. - /// This gives the length of the previous hashsed element name starting + /// This gives the length of the previous hashed element name starting /// from the beginning of the give element name. /// \param postfix: optional pointer to receive the postfix starting at the found tag field. /// \param type: optional pointer to receive the element type character @@ -908,8 +910,9 @@ public: /// \param recursive: recursively find the last non-zero tag /// /// \return Return the end position of the tag field, or return -1 if not found. - int findTagInElementName(long* tag = 0, int* len = 0, const char* postfix = 0, char* type = 0, - bool negative = false, bool recursive = true) const; + int findTagInElementName(long* tag = nullptr, int* len = nullptr, const char* postfix = nullptr, + char* type = nullptr, bool negative = false, + bool recursive = true) const; /// Get a hash for this MappedName std::size_t hash() const @@ -924,7 +927,7 @@ private: }; -typedef QVector<::App::StringIDRef> ElementIDRefs; +using ElementIDRefs = QVector<::App::StringIDRef>; struct MappedNameRef { @@ -934,25 +937,34 @@ struct MappedNameRef MappedNameRef() = default; - MappedNameRef(const MappedName& name, const ElementIDRefs& sids = ElementIDRefs()) - : name(name), - sids(sids) + ~MappedNameRef() = default; + + MappedNameRef(MappedName name, ElementIDRefs sids = ElementIDRefs()) + : name(std::move(name)) + , sids(std::move(sids)) { compact(); } MappedNameRef(const MappedNameRef& other) - : name(other.name), - sids(other.sids) + : name(other.name) + , sids(other.sids) {} - MappedNameRef(MappedNameRef&& other) - : name(std::move(other.name)), - sids(std::move(other.sids)), - next(std::move(other.next)) + MappedNameRef(MappedNameRef&& other) noexcept + : name(std::move(other.name)) + , sids(std::move(other.sids)) + , next(std::move(other.next)) {} - MappedNameRef& operator=(MappedNameRef&& other) + MappedNameRef& operator=(const MappedNameRef& other) noexcept + { + name = other.name; + sids = other.sids; + return *this; + } + + MappedNameRef& operator=(MappedNameRef&& other) noexcept { name = std::move(other.name); sids = std::move(other.sids); @@ -965,22 +977,24 @@ struct MappedNameRef return !name.empty(); } - void append(const MappedName& name, const ElementIDRefs sids = ElementIDRefs()) + void append(const MappedName& _name, const ElementIDRefs _sids = ElementIDRefs()) { - if (!name) + if (!_name) { return; + } if (!this->name) { - this->name = name; - this->sids = sids; + this->name = _name; + this->sids = _sids; compact(); return; } - std::unique_ptr n(new MappedNameRef(name, sids)); - if (!this->next) - this->next = std::move(n); + std::unique_ptr mappedName(new MappedNameRef(_name, _sids)); + if (!this->next) { + this->next = std::move(mappedName); + } else { - this->next.swap(n); - this->next->next = std::move(n); + this->next.swap(mappedName); + this->next->next = std::move(mappedName); } } @@ -992,9 +1006,9 @@ struct MappedNameRef } } - bool erase(const MappedName& name) + bool erase(const MappedName& _name) { - if (this->name == name) { + if (this->name == _name) { this->name.clear(); this->sids.clear(); if (this->next) { @@ -1007,11 +1021,11 @@ struct MappedNameRef return true; } - for (std::unique_ptr* p = &this->next; *p; p = &(*p)->next) { - if ((*p)->name == name) { + for (std::unique_ptr* ptr = &this->next; *ptr; ptr = &(*ptr)->next) { + if ((*ptr)->name == _name) { std::unique_ptr tmp; - tmp.swap(*p); - *p = std::move(tmp->next); + tmp.swap(*ptr); + *ptr = std::move(tmp->next); return true; } } @@ -1033,4 +1047,4 @@ struct MappedNameRef }// namespace Data -#endif// APP_MAPPED_NAME_H \ No newline at end of file +#endif// APP_MAPPED_NAME_H diff --git a/src/App/StringHasher.h b/src/App/StringHasher.h index 7c8f4f5dcf..e56ba9d9ac 100644 --- a/src/App/StringHasher.h +++ b/src/App/StringHasher.h @@ -173,7 +173,7 @@ public: } /// Returns the data (prefix) - QByteArray data() const + const QByteArray& data() const { return _data; } @@ -556,6 +556,7 @@ public: void toBytes(QByteArray& bytes) const { + // TODO: return the QByteArray instead of passing in by reference if (_sid) { bytes = _sid->dataToBytes(_index); } diff --git a/tests/src/App/CMakeLists.txt b/tests/src/App/CMakeLists.txt index e14c01958f..8a98989dc9 100644 --- a/tests/src/App/CMakeLists.txt +++ b/tests/src/App/CMakeLists.txt @@ -3,6 +3,7 @@ target_sources( PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/Application.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Branding.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/ComplexGeoData.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Expression.cpp ${CMAKE_CURRENT_SOURCE_DIR}/ElementMap.cpp ${CMAKE_CURRENT_SOURCE_DIR}/IndexedName.cpp diff --git a/tests/src/App/ComplexGeoData.cpp b/tests/src/App/ComplexGeoData.cpp new file mode 100644 index 0000000000..2447c6209c --- /dev/null +++ b/tests/src/App/ComplexGeoData.cpp @@ -0,0 +1,417 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later + +#include "gtest/gtest.h" + +#include +#include + +#include +#include +#include + + +class ConcreteComplexGeoDataForTesting: public Data::ComplexGeoData +{ +public: + std::vector getElementTypes() const override + { + return {"EDGE"}; + } + + unsigned long countSubElements(const char* Type) const override + { + (void)Type; + return 0; + } + + Data::Segment* getSubElement(const char* Type, unsigned long number) const override + { + (void)number; + (void)Type; + return nullptr; + } + + unsigned int getMemSize() const override + { + return 0; + } + + void Save(Base::Writer& writer) const override + { + (void)writer; + } + + void Restore(Base::XMLReader& reader) override + { + (void)reader; + } + + void setTransform(const Base::Matrix4D& rclTrf) override + { + (void)rclTrf; + } + + Base::Matrix4D getTransform() const override + { + return {}; + } + + void transformGeometry(const Base::Matrix4D& rclMat) override + { + (void)rclMat; + } + + Base::BoundBox3d getBoundBox() const override + { + return Base::BoundBox3d(); + } +}; + +class ComplexGeoDataTest: public ::testing::Test +{ +protected: + static void SetUpTestSuite() + { + if (App::Application::GetARGC() == 0) { + constexpr int argc = 1; + std::array argv {"FreeCAD"}; + App::Application::Config()["ExeName"] = "FreeCAD"; + App::Application::init(argc, argv.data()); + } + } + + void SetUp() override + { + _docName = App::GetApplication().getUniqueDocumentName("test"); + App::GetApplication().newDocument(_docName.c_str(), "testUser"); + } + + void TearDown() override + { + App::GetApplication().closeDocument(_docName.c_str()); + } + + ConcreteComplexGeoDataForTesting& cgd() + { + return _complexGeoData; + } + + std::tuple createMappedName(const std::string& name) + { + auto elementMap = std::make_shared(); + cgd().resetElementMap(elementMap); + auto mappedName = Data::MappedName(name.c_str()); + auto indexedName = Data::IndexedName(_complexGeoData.getElementTypes().front(), 1); + elementMap->setElementName(indexedName, mappedName, 0); + return std::make_tuple(indexedName, mappedName); + } + +private: + ConcreteComplexGeoDataForTesting _complexGeoData; + std::string _docName; +}; + +// NOLINTBEGIN(readability-magic-numbers) + +TEST_F(ComplexGeoDataTest, getIndexedNameNoName)// NOLINT +{ + // Arrange & Act + auto result = cgd().getIndexedName(Data::MappedName()); + + // Assert + EXPECT_FALSE(result); +} + +TEST_F(ComplexGeoDataTest, getIndexedNameNoElementMap)// NOLINT +{ + // Arrange & Act + auto result = cgd().getIndexedName(Data::MappedName("TestName")); + + // Assert + EXPECT_TRUE(result); +} + +TEST_F(ComplexGeoDataTest, getMappedNameNoElement)// NOLINT +{ + // Arrange & Act + auto result = cgd().getMappedName(Data::IndexedName {}); + + // Assert + EXPECT_FALSE(result); +} + +TEST_F(ComplexGeoDataTest, getMappedNameDisallowUnmappedNoMap)// NOLINT +{ + // Arrange & Act + auto result = cgd().getMappedName(Data::IndexedName {"TestName"}, false); + + // Assert + EXPECT_FALSE(result); +} + +TEST_F(ComplexGeoDataTest, getMappedNameDisallowUnmappedWithMap)// NOLINT +{ + // Arrange + auto elementMap = std::make_shared(); + cgd().resetElementMap(elementMap); + auto mappedName = Data::MappedName("TestMappedName"); + auto indexedName = Data::IndexedName("EDGE", 1); + elementMap->setElementName(indexedName, mappedName, 0); + + // Act + auto result = cgd().getMappedName(indexedName, false); + + // Assert + EXPECT_TRUE(result); + EXPECT_EQ(mappedName, result); +} + +TEST_F(ComplexGeoDataTest, getMappedNameDisallowUnmappedMissingFromMap)// NOLINT +{ + // Arrange + auto mappedName = Data::MappedName("NotTheTestName"); + cgd().getIndexedName(mappedName);// Put it in the map + + // Act + auto result = cgd().getMappedName(Data::IndexedName {"TestName"}, false); + + // Assert + EXPECT_FALSE(result); +} + +TEST_F(ComplexGeoDataTest, getMappedNameAllowUnmapped)// NOLINT +{ + // Arrange & Act + auto result = cgd().getMappedName(Data::IndexedName {"TestName"}, true); + + // Assert + EXPECT_TRUE(result); +} + +TEST_F(ComplexGeoDataTest, getElementNameGivenIndexedName)// NOLINT +{ + // Arrange + const char* name {"EDGE123"}; + auto indexedName = cgd().getIndexedName(Data::MappedName(name)); + + // Act + auto result = cgd().getElementName(name); + + // Assert + EXPECT_EQ(result.index, indexedName); +} + +TEST_F(ComplexGeoDataTest, getElementNameGivenMappedName)// NOLINT +{ + // Arrange + const char* name {"EDGE123"}; + auto mappedName = cgd().getMappedName(Data::IndexedName(name)); + + // Act + auto result = cgd().getElementName(name); + + // Assert + EXPECT_EQ(result.name, mappedName); +} + +TEST_F(ComplexGeoDataTest, getElementMappedNamesNoMapNoUnmapped)// NOLINT +{ + // Arrange + // Do not create an element map + auto indexedName = Data::IndexedName("EDGE1"); + + // Act + auto result = cgd().getElementMappedNames(indexedName, false); + + // Assert + EXPECT_TRUE(result.empty()); +} + +TEST_F(ComplexGeoDataTest, getElementMappedNamesWithMapNoUnmapped)// NOLINT +{ + // Arrange + auto elementMap = std::make_shared(); + cgd().resetElementMap(elementMap); + auto mappedName = Data::MappedName("TestMappedName"); + auto indexedName = Data::IndexedName("EDGE", 1); + elementMap->setElementName(indexedName, mappedName, 0); + + // Act + auto result = cgd().getElementMappedNames(indexedName, false); + + // Assert + EXPECT_FALSE(result.empty()); + EXPECT_EQ(result[0].first, mappedName); +} + +TEST_F(ComplexGeoDataTest, getElementMappedNamesNoMapWithUnmapped)// NOLINT +{ + // Do not create an element map + auto indexedName = Data::IndexedName("EDGE1"); + + // Act + auto result = cgd().getElementMappedNames(indexedName, true); + + // Assert + EXPECT_FALSE(result.empty()); +} + + +TEST_F(ComplexGeoDataTest, getElementMappedNamesWithMapWithUnmapped)// NOLINT +{ + // Arrange + Data::MappedName mappedName; + Data::IndexedName indexedName; + std::tie(indexedName, mappedName) = createMappedName("TestMappedName"); + auto unmappedName = Data::IndexedName("EDGE", 2); + + // Act + auto result = cgd().getElementMappedNames(unmappedName, true); + + // Assert + EXPECT_FALSE(result.empty()); + EXPECT_NE(result[0].first, mappedName); +} + +TEST_F(ComplexGeoDataTest, elementTypeValidIndexName)// NOLINT +{ + // Arrange + auto indexedName = Data::IndexedName("EDGE", 1); + + // Act + char elementType = cgd().elementType(indexedName); + + // Assert + EXPECT_EQ(elementType, 'E'); +} + +TEST_F(ComplexGeoDataTest, elementTypeInvalidIndexedName)// NOLINT +{ + // Arrange + auto indexedName = Data::IndexedName("INVALID", 1);// Not in the element type list + + // Act + char elementType = cgd().elementType(indexedName); + + // Assert + EXPECT_EQ(elementType, 0); +} + +TEST_F(ComplexGeoDataTest, elementTypeCharEmptyName)// NOLINT +{ + // Arrange & Act + char elementType = cgd().elementType(nullptr); + + // Assert + EXPECT_EQ(elementType, 0); +} + +TEST_F(ComplexGeoDataTest, elementTypeCharIndexedName)// NOLINT +{ + // Arrange & Act + char elementType = cgd().elementType("EDGE1"); + + // Assert + EXPECT_EQ(elementType, 'E'); +} + +TEST_F(ComplexGeoDataTest, elementTypeCharMappedNameNoPrefix)// NOLINT +{ + // Arrange + int size {0}; + Data::MappedName mappedName; + Data::IndexedName indexedName; + std::tie(indexedName, mappedName) = createMappedName("TestMappedName:;"); + + // Act + char elementType = cgd().elementType(mappedName.toConstString(0, size)); + + // Assert + EXPECT_EQ(elementType, 'E'); +} + +TEST_F(ComplexGeoDataTest, elementTypeCharMappedNameWithPrefix)// NOLINT +{ + // Arrange + int size {0}; + Data::MappedName mappedName; + Data::IndexedName indexedName; + auto name = fmt::format("{}TestMappedElement:;", Data::ELEMENT_MAP_PREFIX); + std::tie(indexedName, mappedName) = createMappedName(name); + + // Act + char elementType = cgd().elementType(mappedName.toConstString(0, size)); + + // Assert + EXPECT_EQ(elementType, 'E'); +} + +TEST_F(ComplexGeoDataTest, resetElementMapNoArgument)// NOLINT +{ + // Arrange & Act + cgd().resetElementMap(); + + // Assert + EXPECT_EQ(cgd().getElementMapSize(), 0); +} + +TEST_F(ComplexGeoDataTest, resetElementMapWithArgument)// NOLINT +{ + // Arrange + auto elementMap = std::make_shared(); + auto mappedName = Data::MappedName("TestMappedName"); + auto indexedName = Data::IndexedName("EDGE", 1); + elementMap->setElementName(indexedName, mappedName, 0); + + // Act + cgd().resetElementMap(elementMap); + + // Assert + EXPECT_EQ(cgd().getElementMapSize(), 1); +} + +TEST_F(ComplexGeoDataTest, setAndGetElementMap)// NOLINT +{ + // Arrange + auto elementMap = std::make_shared(); + cgd().resetElementMap(elementMap); + std::vector vecMappedElements; + auto mappedNameA = Data::MappedName("TestMappedNameA"); + auto indexedNameA = Data::IndexedName("EDGE", 1); + vecMappedElements.emplace_back(indexedNameA, mappedNameA); + auto mappedNameB = Data::MappedName("TestMappedNameB"); + auto indexedNameB = Data::IndexedName("EDGE", 2); + vecMappedElements.emplace_back(indexedNameB, mappedNameB); + + // Act + auto emptyElementMap = cgd().getElementMap(); + cgd().setElementMap(vecMappedElements); + auto resultingElementMap = cgd().getElementMap(); + + // Assert + EXPECT_TRUE(emptyElementMap.empty()); + EXPECT_EQ(resultingElementMap.size(), vecMappedElements.size()); +} + +TEST_F(ComplexGeoDataTest, getElementMapSize)// NOLINT +{ + // Arrange + auto elementMap = std::make_shared(); + auto mappedName = Data::MappedName("TestMappedName"); + auto indexedName = Data::IndexedName("EDGE", 1); + elementMap->setElementName(indexedName, mappedName, 0); + cgd().resetElementMap(elementMap); + + // Act + auto result = cgd().getElementMapSize(); + + // Assert + EXPECT_EQ(result, 1); +} + +TEST_F(ComplexGeoDataTest, flushElementMap)// NOLINT +{ + // Does nothing in the base class +} + +// NOLINTEND(readability-magic-numbers) diff --git a/tests/src/App/ElementMap.cpp b/tests/src/App/ElementMap.cpp index 13819bd723..3e103be85a 100644 --- a/tests/src/App/ElementMap.cpp +++ b/tests/src/App/ElementMap.cpp @@ -45,21 +45,29 @@ class ElementMapTest: public ::testing::Test protected: static void SetUpTestSuite() { - int argc = 1; - char* argv[] = {"FreeCAD"}; - App::Application::Config()["ExeName"] = "FreeCAD"; - App::Application::init(argc, argv); + if (App::Application::GetARGC() == 0) { + int argc = 1; + char* argv[] = {"FreeCAD"}; + App::Application::Config()["ExeName"] = "FreeCAD"; + App::Application::init(argc, argv); + } } void SetUp() override { - App::GetApplication().newDocument("test", "testUser"); + _docName = App::GetApplication().getUniqueDocumentName("test"); + App::GetApplication().newDocument(_docName.c_str(), "testUser"); _sids = &_sid; _hasher = Base::Reference(new App::StringHasher); + ASSERT_EQ(_hasher.getRefCount(), 1); } - // void TearDown() override {} + void TearDown() override + { + App::GetApplication().closeDocument(_docName.c_str()); + } + std::string _docName; Data::ElementIDRefs _sid; QVector* _sids; App::StringHasherRef _hasher; diff --git a/tests/src/App/MappedName.cpp b/tests/src/App/MappedName.cpp index 4b11ec5f6e..77b6b297f0 100644 --- a/tests/src/App/MappedName.cpp +++ b/tests/src/App/MappedName.cpp @@ -225,8 +225,11 @@ TEST(MappedName, fromRawData) TEST(MappedName, fromRawDataQByteArray) { + // Arrange + QByteArray testByteArray("TESTTEST", 10); + // Act - Data::MappedName mappedName = Data::MappedName::fromRawData(QByteArray("TESTTEST", 10)); + Data::MappedName mappedName = Data::MappedName::fromRawData(testByteArray); // Assert EXPECT_EQ(mappedName.isRaw(), true); @@ -239,7 +242,8 @@ TEST(MappedName, fromRawDataQByteArray) TEST(MappedName, fromRawDataCopy) { // Arrange - Data::MappedName temp = Data::MappedName::fromRawData(QByteArray("TESTTEST", 10)); + QByteArray testByteArray("TESTTEST", 10); + Data::MappedName temp = Data::MappedName::fromRawData(testByteArray); temp.append("TESTPOSTFIX"); temp.compact();// Always call compact before accessing data! @@ -257,7 +261,8 @@ TEST(MappedName, fromRawDataCopy) TEST(MappedName, fromRawDataCopyStartposAndSize) { // Arrange - Data::MappedName temp = Data::MappedName::fromRawData(QByteArray("TESTTEST", 8)); + QByteArray testByteArray("TESTTEST", 8); + Data::MappedName temp = Data::MappedName::fromRawData(testByteArray); temp.append("ABCDEFGHIJKLM");// postfix temp.compact(); // Always call compact before accessing data! @@ -373,11 +378,12 @@ TEST(MappedName, additionOperators) { // Arrange Data::MappedName mappedName(Data::MappedName("TEST"), "POSTFIXTEST"); + QByteArray post3("POST3"); // Act mappedName += "POST1"; mappedName += std::string("POST2"); - mappedName += QByteArray("POST3"); + mappedName += post3; mappedName += Data::MappedName("POST4"); // Assert @@ -389,12 +395,13 @@ TEST(MappedName, additionOperators) // Arrange mappedName = Data::MappedName(Data::MappedName("TEST"), "POSTFIXTEST"); + QByteArray post8("POST8"); // Act mappedName = mappedName + Data::MappedName("POST5"); mappedName = mappedName + "POST6"; mappedName = mappedName + std::string("POST7"); - mappedName = mappedName + QByteArray("POST8"); + mappedName = mappedName + post8; // Assert EXPECT_EQ(mappedName.isRaw(), false); From aae9cc84d95bd699c42e799031f557609e9fcb00 Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Fri, 14 Jul 2023 23:35:16 -0500 Subject: [PATCH 3/3] PD: prevent MSVC from defining min/max --- src/App/MappedName.h | 1 - src/Mod/PartDesign/Gui/PreCompiled.h | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/App/MappedName.h b/src/App/MappedName.h index 9921e4280e..8e3ef863db 100644 --- a/src/App/MappedName.h +++ b/src/App/MappedName.h @@ -25,7 +25,6 @@ #ifndef APP_MAPPED_NAME_H #define APP_MAPPED_NAME_H - #include #include diff --git a/src/Mod/PartDesign/Gui/PreCompiled.h b/src/Mod/PartDesign/Gui/PreCompiled.h index 4065daab5c..29ac54ca4a 100644 --- a/src/Mod/PartDesign/Gui/PreCompiled.h +++ b/src/Mod/PartDesign/Gui/PreCompiled.h @@ -32,6 +32,8 @@ #ifdef _PreComp_ #ifdef FC_OS_WIN32 +# undef NOMINMAX +# define NOMINMAX # include #endif