From 695b08c5462c5ea36231c4d0938fccba1e0ab1e2 Mon Sep 17 00:00:00 2001 From: Christian Heimlich Date: Wed, 2 Aug 2023 07:25:48 -0400 Subject: [PATCH] Change Index and TextPos to use enum constructors for extents The static member variable version was flawed as it suffered from SIOF, since TextPos' static END made use of Index's static LAST. Often, END would be initialized before LAST resulting in END being invalid/garbage. This approach, while slightly less terse, prevents that issue. The only other option is to use static member functions that return a type constructed to be that extent, which would enact RAII. The interface would look like: Index32::last(); instead of Index32(Index32::Last). --- lib/core/include/qx/core/qx-index.h | 54 ++++++++++++++++------------- lib/core/src/qx-index.dox | 34 ++++++++---------- lib/io/include/qx/io/qx-common-io.h | 10 +++--- lib/io/include/qx/io/qx-textpos.h | 12 +++++-- lib/io/src/qx-common-io.cpp | 26 +++++++------- lib/io/src/qx-common-io_p.cpp | 4 +-- lib/io/src/qx-common-io_p.h | 2 +- lib/io/src/qx-textpos.cpp | 40 +++++++++++++++++---- lib/io/src/qx-textquery.cpp | 4 +-- 9 files changed, 111 insertions(+), 75 deletions(-) diff --git a/lib/core/include/qx/core/qx-index.h b/lib/core/include/qx/core/qx-index.h index 492ea766..4df1054d 100644 --- a/lib/core/include/qx/core/qx-index.h +++ b/lib/core/include/qx/core/qx-index.h @@ -22,12 +22,13 @@ class Index //-Class Types---------------------------------------------------------------------------------------------------- private: enum class Type {Null, End, Value}; - struct LastKey {}; -//-Class Members---------------------------------------------------------------------------------------------------- public: - static const Index FIRST; - static const Index LAST; + enum Extent + { + First, + Last + }; //-Instance Members---------------------------------------------------------------------------------------------------- private: @@ -35,19 +36,32 @@ class Index T mValue; //-Constructor---------------------------------------------------------------------------------------------- -private: - Index(LastKey) : - mType(Type::End), - mValue(std::numeric_limits::max()) - {} - public: - Index() : + constexpr Index() : mType(Type::Null), mValue(0) {} - Index(T value) : + constexpr Index(Extent e) + { + switch(e) + { + case First: + mType = Type::Value; + mValue = 0; + break; + + case Last: + mType = Type::End; + mValue = std::numeric_limits::max(); + break; + + default: + qCritical("Invalid extent"); + } + } + + constexpr Index(T value) : mType(Type::Value), mValue(value) { @@ -93,7 +107,7 @@ class Index if(other.mType == Type::End) return 0; else if(mType == Type::End) - return LAST; + return Index(Last); else return constrainedSub(mValue, other.mValue, 0); } @@ -103,7 +117,7 @@ class Index Index operator+(const Index& other) { return (mType == Type::End || other.mType == Type::End) ? - LAST : constrainedAdd(mValue, other.mValue, 0); + Index(Last) : constrainedAdd(mValue, other.mValue, 0); } @@ -117,7 +131,7 @@ class Index if(other.mType == Type::End) return mType == Type::End ? 1 : 0; else if(mType == Type::End) - return LAST; + return Index(Last); else return constrainedDiv(mValue, other.mValue, 0); } @@ -129,7 +143,7 @@ class Index if(mValue == 0 || other.mValue == 0) return 0; else if(mType == Type::End || other.mType == Type::End) - return LAST; + return Index(Last); else return constrainedMult(mValue, other.mValue, 0); @@ -289,14 +303,6 @@ class Index // friend T& operator*=(N& integer, const Index& index) { integer *= static_cast(index.mValue); return integer; } }; -//-Outer Class Definitions---------------------------------------------------------------------------------- -template - requires std::signed_integral -const Index Index::FIRST = Index(0); -template - requires std::signed_integral -const Index Index::LAST = Index(LastKey{}); - //-Outer Class Types---------------------------------------------------------------------------------------- typedef Index Index8; typedef Index Index16; diff --git a/lib/core/src/qx-index.dox b/lib/core/src/qx-index.dox index 25ab4e58..d25f21f7 100644 --- a/lib/core/src/qx-index.dox +++ b/lib/core/src/qx-index.dox @@ -17,7 +17,7 @@ namespace Qx * use -1 to mean 'none' in the context of searches. * * The Index type is designed to remedy this issue by providing a more distinct representation of a set's final index - * via the member variable Index::LAST, which for completeness is paired with Index::FIRST. The type also + * via the constructor Index(Extent), which can also be used to more explicitly reference the first index. The type also * can be null, providing further clarity for the 'none' case. * * To minimize unintentional interactions, Index only has comparison and arithmetic operators defined for when @@ -52,20 +52,18 @@ namespace Qx * @li 2) 'last' indices are greater than everything except other 'last' indices (highest possible value) */ -//-Class Members---------------------------------------------------------------------------------------------------- +//-Class Types---------------------------------------------------------------------------------------------------- //Public: /*! - * @var Index Index::FIRST + * @enum Index::Extent * - * Used to refer to the first index of a dataset. + * Used to refer to indicies of special significance. * - * This is equivalent to @c Index(0). - */ - -/*! - * @var Index Index::LAST + * @var Index::Extent Index::First + * The first index, equivalent to @c Index(0). * - * Used to refer to the last index of a dataset. + * @var Index::Extent Index::Last + * The last index. */ //-Constructor---------------------------------------------------------------------------------------------- @@ -76,6 +74,12 @@ namespace Qx * Creates an null index. */ +/*! + * @fn Index::Index(Extent e) + * + * Creates an index at the given extent @a e. + */ + /*! * @fn Index::Index(T value) * @@ -97,15 +101,7 @@ namespace Qx * * Returns @c true if the index represents the last index of an arbitrary dataset; otherwise returns @c false. * - * This is equivalent to *this == Index::LAST. - */ - -/*! - * @fn bool Index::isLast() const - * - * Returns @c true if the index represents the last index of an arbitrary dataset; otherwise returns @c false. - * - * This is equivalent to *this == Index::LAST. + * This is equivalent to *this == Index(Last). */ //-Operators------------------------------------------------------------------------------------------------------- diff --git a/lib/io/include/qx/io/qx-common-io.h b/lib/io/include/qx/io/qx-common-io.h index f267b937..e8888c09 100644 --- a/lib/io/include/qx/io/qx-common-io.h +++ b/lib/io/include/qx/io/qx-common-io.h @@ -62,10 +62,10 @@ QX_IO_EXPORT IoOpReport textFileAbsolutePosition(TextPos& textPos, QFile& textFi QX_IO_EXPORT IoOpReport findStringInFile(QList& returnBuffer, QFile& textFile, const TextQuery& query, ReadOptions readOptions = NoReadOptions); QX_IO_EXPORT IoOpReport fileContainsString(bool& returnBuffer, QFile& textFile, const QString& query, Qt::CaseSensitivity caseSensitivity = Qt::CaseSensitive, bool allowSplit = false); QX_IO_EXPORT IoOpReport readTextFromFile(QString& returnBuffer, QFile& textFile, TextPos startPos, int count, ReadOptions readOptions = NoReadOptions); -QX_IO_EXPORT IoOpReport readTextFromFile(QString& returnBuffer, QFile& textFile, TextPos startPos = TextPos::START, TextPos endPos = TextPos::END, ReadOptions readOptions = NoReadOptions); -QX_IO_EXPORT IoOpReport readTextFromFile(QStringList& returnBuffer, QFile& textFile, Index32 startLine = 0, Index32 endLine = Index32::LAST, ReadOptions readOptions = NoReadOptions); -QX_IO_EXPORT IoOpReport writeStringToFile(QFile& textFile, const QString& text, WriteMode writeMode = Truncate, TextPos startPos = TextPos::START, WriteOptions writeOptions = NoWriteOptions); -QX_IO_EXPORT IoOpReport writeStringToFile(QSaveFile& textFile, const QString& text, WriteMode writeMode = Truncate, TextPos startPos = TextPos::START, WriteOptions writeOptions = NoWriteOptions); +QX_IO_EXPORT IoOpReport readTextFromFile(QString& returnBuffer, QFile& textFile, TextPos startPos = TextPos(TextPos::Start), TextPos endPos = TextPos(TextPos::End), ReadOptions readOptions = NoReadOptions); +QX_IO_EXPORT IoOpReport readTextFromFile(QStringList& returnBuffer, QFile& textFile, Index32 startLine = 0, Index32 endLine = Index32(Index32::Last), ReadOptions readOptions = NoReadOptions); +QX_IO_EXPORT IoOpReport writeStringToFile(QFile& textFile, const QString& text, WriteMode writeMode = Truncate, TextPos startPos = TextPos(TextPos::Start), WriteOptions writeOptions = NoWriteOptions); +QX_IO_EXPORT IoOpReport writeStringToFile(QSaveFile& textFile, const QString& text, WriteMode writeMode = Truncate, TextPos startPos = TextPos(TextPos::Start), WriteOptions writeOptions = NoWriteOptions); QX_IO_EXPORT IoOpReport deleteTextFromFile(QFile& textFile, TextPos startPos, TextPos endPos); // Directory: @@ -81,7 +81,7 @@ QX_IO_EXPORT IoOpReport calculateFileChecksum(QString& returnBuffer, QFile& file QX_IO_EXPORT IoOpReport fileMatchesChecksum(bool& returnBuffer, QFile& file, QString checksum, QCryptographicHash::Algorithm hashAlgorithm); // Binary Based -QX_IO_EXPORT IoOpReport readBytesFromFile(QByteArray& returnBuffer, QFile& file, Index64 startPos = 0, Index64 endPos = Index64::LAST); +QX_IO_EXPORT IoOpReport readBytesFromFile(QByteArray& returnBuffer, QFile& file, Index64 startPos = 0, Index64 endPos = Index64(Index64::Last)); QX_IO_EXPORT IoOpReport writeBytesToFile(QFile& file, const QByteArray& bytes, WriteMode writeMode = Truncate, Index64 startPos = 0, WriteOptions writeOptions = NoWriteOptions); QX_IO_EXPORT IoOpReport writeBytesToFile(QSaveFile& file, const QByteArray& bytes, WriteMode writeMode = Truncate, Index64 startPos = 0, WriteOptions writeOptions = NoWriteOptions); } diff --git a/lib/io/include/qx/io/qx-textpos.h b/lib/io/include/qx/io/qx-textpos.h index 93bf4e7f..dfcfefc6 100644 --- a/lib/io/include/qx/io/qx-textpos.h +++ b/lib/io/include/qx/io/qx-textpos.h @@ -7,6 +7,10 @@ // Extra-component Includes #include "qx/core/qx-index.h" +// TODO: This class should probably be constexpr construable, though this +// would mean it cant have separate implementation files which is somewhat ugly. +// probably should reconsider this once using modules. + namespace Qx { @@ -14,8 +18,11 @@ class QX_IO_EXPORT TextPos { //-Class Types------------------------------------------------------------------------------------------------------ public: - static const TextPos START; - static const TextPos END; + enum Extent + { + Start, + End + }; //-Instance Variables------------------------------------------------------------------------------------------------ private: @@ -25,6 +32,7 @@ class QX_IO_EXPORT TextPos //-Constructor------------------------------------------------------------------------------------------------------- public: TextPos(); + TextPos(Extent e); TextPos(Index32 line, Index32 character); //-Instance Functions------------------------------------------------------------------------------------------------ diff --git a/lib/io/src/qx-common-io.cpp b/lib/io/src/qx-common-io.cpp index fc2173ef..d99ad33f 100644 --- a/lib/io/src/qx-common-io.cpp +++ b/lib/io/src/qx-common-io.cpp @@ -378,9 +378,9 @@ IoOpReport textFileLineCount(int& returnBuffer, QFile& textFile, bool ignoreTrai /*! * Converts any relative component of @a textPos to an absolute one. I.e. determines the actual line - * and or/character that Index32::LAST references for the given @a textFile, if present. + * and or/character that Index32(Index32::Last) references for the given @a textFile, if present. * - * If neither the line or character component of @a textPos contain the value Index32::LAST, + * If neither the line or character component of @a textPos contain the value Index32(Index32::Last), * @a textPos is left unchanged. * * @param[in,out] textPos The text position to translate into an absolute position. @@ -459,7 +459,7 @@ IoOpReport findStringInFile(QList& returnBuffer, QFile& textFile, const // Query tracking TextPos trueStartPos = query.startPosition(); - TextPos currentPos = TextPos::START; + TextPos currentPos = TextPos(TextPos::Start); TextPos possibleMatch = TextPos(); int hitsSkipped = 0; QString::const_iterator queryIt = query.string().constBegin(); @@ -469,7 +469,7 @@ IoOpReport findStringInFile(QList& returnBuffer, QFile& textFile, const QTextStream fileTextStream(&textFile); // Translate start position to absolute position - if(trueStartPos != TextPos::START) + if(trueStartPos != TextPos(TextPos::Start)) { IoOpReport translate = textFileAbsolutePosition(trueStartPos, textFile, readOptions.testFlag(IgnoreTrailingBreak)); if(translate.isFailure()) @@ -489,7 +489,7 @@ IoOpReport findStringInFile(QList& returnBuffer, QFile& textFile, const QScopeGuard fileGuard([&textFile](){ textFile.close(); }); // Skip to start pos - if(trueStartPos != TextPos::START) + if(trueStartPos != TextPos(TextPos::Start)) { int line; // Skip to start line @@ -739,7 +739,7 @@ IoOpReport readTextFromFile(QString& returnBuffer, QFile& textFile, TextPos star TextStream fileTextStream(&textFile); // Cover each possible range type - if(startPos == TextPos::START && endPos == TextPos::END) // Whole file is desired + if(startPos == TextPos(TextPos::Start) && endPos == TextPos(TextPos::End)) // Whole file is desired { returnBuffer = fileTextStream.readAll(); @@ -999,7 +999,7 @@ namespace // Fill beforeNew TextPos beforeEnd = TextPos(startPos.line(), startPos.character() - 1); - IoOpReport readBefore = readTextFromFile(beforeNew, auxFile, TextPos::START, beforeEnd); + IoOpReport readBefore = readTextFromFile(beforeNew, auxFile, TextPos(TextPos::Start), beforeEnd); if(readBefore.isFailure()) return readBefore; @@ -1178,27 +1178,27 @@ IoOpReport deleteTextFromFile(QFile& textFile, TextPos startPos, TextPos endPos) IoOpReport transientReport; // Determine beforeDeletion - if(startPos == TextPos::START) // (0,0) + if(startPos == TextPos(TextPos::Start)) // (0,0) beforeDeletion = u""_s; else if(startPos.character().isLast()) { - transientReport = readTextFromFile(beforeDeletion, textFile, TextPos::START, startPos); + transientReport = readTextFromFile(beforeDeletion, textFile, TextPos(TextPos::Start), startPos); beforeDeletion.chop(1); } else - transientReport = readTextFromFile(beforeDeletion, textFile, TextPos::START, TextPos(startPos.line(), startPos.character() - 1)); + transientReport = readTextFromFile(beforeDeletion, textFile, TextPos(TextPos::Start), TextPos(startPos.line(), startPos.character() - 1)); // Check for transient errors if(!transientReport.isNull() && transientReport.result() != IO_SUCCESS) return IoOpReport(IO_OP_WRITE, transientReport.result(), textFile); // Determine afterDeletion - if(endPos == TextPos::END) + if(endPos == TextPos(TextPos::End)) afterDeletion = u""_s; else if(endPos.character().isLast()) - transientReport = readTextFromFile(afterDeletion, textFile, TextPos(endPos.line() + 1, 0), TextPos::END); + transientReport = readTextFromFile(afterDeletion, textFile, TextPos(endPos.line() + 1, 0), TextPos(TextPos::End)); else - transientReport = readTextFromFile(afterDeletion, textFile, TextPos(endPos.line(), endPos.character() + 1), TextPos::END); + transientReport = readTextFromFile(afterDeletion, textFile, TextPos(endPos.line(), endPos.character() + 1), TextPos(TextPos::End)); // Check for transient errors if(transientReport.result() != IO_SUCCESS) diff --git a/lib/io/src/qx-common-io_p.cpp b/lib/io/src/qx-common-io_p.cpp index 3961b2ee..b240c34e 100644 --- a/lib/io/src/qx-common-io_p.cpp +++ b/lib/io/src/qx-common-io_p.cpp @@ -132,10 +132,10 @@ IoOpReport writePrep(const QFileInfo& fileInfo, WriteOptions writeOptions) void matchAppendConditionParams(WriteMode& writeMode, TextPos& startPos) { // Match append condition parameters - if(startPos == TextPos::END) + if(startPos == TextPos(TextPos::End)) writeMode = Append; else if(writeMode == Append) - startPos = TextPos::END; + startPos = TextPos(TextPos::End); } /*! @endcond */ diff --git a/lib/io/src/qx-common-io_p.h b/lib/io/src/qx-common-io_p.h index 10a5fb35..26ddb4b2 100644 --- a/lib/io/src/qx-common-io_p.h +++ b/lib/io/src/qx-common-io_p.h @@ -37,7 +37,7 @@ void matchAppendConditionParams(WriteMode& writeMode, Index& startPos) if(startPos.isLast()) writeMode = Append; else if(writeMode == Append) - startPos = Index::LAST; + startPos = Index(Index::Last); } /*! @endcond */ } diff --git a/lib/io/src/qx-textpos.cpp b/lib/io/src/qx-textpos.cpp index b471457a..6b6198c5 100644 --- a/lib/io/src/qx-textpos.cpp +++ b/lib/io/src/qx-textpos.cpp @@ -15,22 +15,24 @@ namespace Qx * @brief The TextPos class is used to represent an offset within a text file in terms of lines and characters. */ -//-Class Variables----------------------------------------------------------------------------------------------- +//-Class Types----------------------------------------------------------------------------------------------- //Public: /*! + * @enum Index::Extent + * + * Used to refer to text positions of special significance. + * + * @var Index::Extent Index::First * A text position representing the start of a file. * * Equivalent to @c TextPos(0,0). - */ -const TextPos TextPos::START = TextPos(0,0); // Initialization of constant reference TextPos - -/*! + * + * @var Index::Extent Index::Last * A text position representing the end file. * - * Equivalent to @c TextPos(Index32::LAST,Index32::LAST). + * Equivalent to @c TextPos(Index32(Index32::Last), Index32(Index32::Last)). */ -const TextPos TextPos::END = TextPos(Index32::LAST, Index32::LAST); // Initialization of constant reference TextPos //-Constructor--------------------------------------------------------------------------------------------------- //Public: @@ -43,6 +45,30 @@ TextPos::TextPos() : mCharacter(Index32()) {} +//TODO: Consider creating Qx::Extent (with Start/End) in qx-global and just use +// that for Index and TextPos. +/*! + * Creates a text position at the given extent @a e. + */ +TextPos::TextPos(Extent e) +{ + switch(e) + { + case Start: + mLine = 0; + mCharacter = 0; + break; + + case End: + mLine = Index32(Index32::Last); + mCharacter = Index32(Index32::Last); + break; + + default: + qCritical("Invalid extent"); + } +} + /*! * Creates a text position that points to @a line and @a character. */ diff --git a/lib/io/src/qx-textquery.cpp b/lib/io/src/qx-textquery.cpp index d6348b64..ae2e8dab 100644 --- a/lib/io/src/qx-textquery.cpp +++ b/lib/io/src/qx-textquery.cpp @@ -27,7 +27,7 @@ namespace Qx TextQuery::TextQuery(const QString& string, Qt::CaseSensitivity cs) : mString(string), mCaseSensitivity(cs), - mStartPos(TextPos::START), + mStartPos(TextPos(TextPos::Start)), mHitsToSkip(0), mHitLimit(-1), mAllowSplit(false) @@ -48,7 +48,7 @@ Qt::CaseSensitivity TextQuery::caseSensitivity() const { return mCaseSensitivity /*! * Returns the position from which the search should begin. * - * The default is @c TextPos::START. + * The default is @c TextPos(TextPos::Start). */ TextPos TextQuery::startPosition() const { return mStartPos; }