From 2d184b6754bd4adabc7223e51e8d49f3f1d07632 Mon Sep 17 00:00:00 2001 From: Stephen Anthony Date: Sun, 6 Oct 2024 20:27:25 -0230 Subject: [PATCH] Fixes from clang-tidy-19 that didn't make it for the last release. --- src/common/HighScoresManager.cxx | 3 +- src/common/JoyMap.cxx | 41 ++++++------ src/common/JoyMap.hxx | 2 +- src/common/KeyMap.cxx | 13 ++-- src/common/PaletteHandler.cxx | 6 +- src/common/PhysicalJoystick.cxx | 16 ++--- src/common/SoundSDL2.cxx | 4 +- src/common/Version.hxx | 2 +- src/common/ZipHandler.cxx | 3 +- src/common/bspf.hxx | 9 +-- src/common/main.cxx | 66 ++++++++----------- .../repository/KeyValueRepositoryFile.hxx | 6 +- src/common/tv_filters/NTSCFilter.cxx | 6 +- src/debugger/Debugger.cxx | 15 ++--- src/debugger/DebuggerParser.cxx | 5 +- src/debugger/gui/CartDebugWidget.cxx | 3 +- src/debugger/gui/CartRamWidget.cxx | 5 +- src/debugger/gui/DataGridWidget.cxx | 6 +- src/debugger/gui/PromptWidget.cxx | 42 +++++------- src/debugger/gui/RomListWidget.cxx | 20 ++---- src/debugger/gui/ToggleBitWidget.cxx | 4 +- src/emucore/CartELF.cxx | 6 +- src/emucore/EventHandler.cxx | 8 +-- src/emucore/EventHandler.hxx | 2 +- src/emucore/FSNode.cxx | 7 +- src/emucore/PlusROM.cxx | 2 +- src/emucore/Settings.cxx | 8 +-- src/emucore/elf/VcsLib.cxx | 2 +- src/gui/Dialog.cxx | 13 ++-- src/gui/EditableWidget.cxx | 13 ++-- src/gui/FavoritesManager.cxx | 11 ++-- src/gui/ListWidget.cxx | 27 ++------ src/gui/RomImageWidget.cxx | 10 +-- src/gui/ScrollBarWidget.cxx | 30 ++++----- src/gui/TabWidget.cxx | 6 +- src/gui/ToolTip.cxx | 2 +- src/tools/TIDY | 7 +- 37 files changed, 185 insertions(+), 246 deletions(-) diff --git a/src/common/HighScoresManager.cxx b/src/common/HighScoresManager.cxx index 1e645be1f..6890474bc 100644 --- a/src/common/HighScoresManager.cxx +++ b/src/common/HighScoresManager.cxx @@ -381,8 +381,7 @@ string HighScoresManager::formattedScore(Int32 score, Int32 width) const if(scoreBCD(jprops)) { - if(width > digits) - digits = width; + digits = std::max(width, digits); buf << std::setw(digits) << std::setfill(' ') << score; } else { diff --git a/src/common/JoyMap.cxx b/src/common/JoyMap.cxx index cab67348d..6510f83b6 100644 --- a/src/common/JoyMap.cxx +++ b/src/common/JoyMap.cxx @@ -188,27 +188,26 @@ json JoyMap::saveMapping(EventMode mode) const using MapType = std::pair; std::vector sortedMap(myMap.begin(), myMap.end()); - std::sort(sortedMap.begin(), sortedMap.end(), - [](const MapType& a, const MapType& b) - { - // Event::Type first - if(a.first.button != b.first.button) - return a.first.button < b.first.button; + std::ranges::sort(sortedMap, [](const MapType& a, const MapType& b) + { + // Event::Type first + if(a.first.button != b.first.button) + return a.first.button < b.first.button; - if(a.first.axis != b.first.axis) - return a.first.axis < b.first.axis; + if(a.first.axis != b.first.axis) + return a.first.axis < b.first.axis; - if(a.first.adir != b.first.adir) - return a.first.adir < b.first.adir; + if(a.first.adir != b.first.adir) + return a.first.adir < b.first.adir; - if(a.first.hat != b.first.hat) - return a.first.hat < b.first.hat; + if(a.first.hat != b.first.hat) + return a.first.hat < b.first.hat; - if(a.first.hdir != b.first.hdir) - return a.first.hdir < b.first.hdir; + if(a.first.hdir != b.first.hdir) + return a.first.hdir < b.first.hdir; - return a.second < b.second; - } + return a.second < b.second; + } ); json eventMappings = json::array(); @@ -285,17 +284,17 @@ int JoyMap::loadMapping(const json& eventMappings, EventMode mode) } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -json JoyMap::convertLegacyMapping(string list) +json JoyMap::convertLegacyMapping(string lst) { json eventMappings = json::array(); // Since istringstream swallows whitespace, we have to make the // delimiters be spaces - std::replace(list.begin(), list.end(), '|', ' '); - std::replace(list.begin(), list.end(), ':', ' '); - std::replace(list.begin(), list.end(), ',', ' '); + std::ranges::replace(lst, '|', ' '); + std::ranges::replace(lst, ':', ' '); + std::ranges::replace(lst, ',', ' '); - istringstream buf(list); + istringstream buf(lst); int event = 0, button = 0, axis = 0, adir = 0, hat = 0, hdir = 0; while(buf >> event && buf >> button diff --git a/src/common/JoyMap.hxx b/src/common/JoyMap.hxx index cdb9bc0f0..693ace4c8 100644 --- a/src/common/JoyMap.hxx +++ b/src/common/JoyMap.hxx @@ -112,7 +112,7 @@ class JoyMap nlohmann::json saveMapping(EventMode mode) const; int loadMapping(const nlohmann::json& eventMappings, EventMode mode); - static nlohmann::json convertLegacyMapping(string list); + static nlohmann::json convertLegacyMapping(string lst); /** Erase all mappings for given mode */ void eraseMode(EventMode mode); diff --git a/src/common/KeyMap.cxx b/src/common/KeyMap.cxx index 460087e79..0d169d17d 100644 --- a/src/common/KeyMap.cxx +++ b/src/common/KeyMap.cxx @@ -222,8 +222,7 @@ json KeyMap::saveMapping(EventMode mode) const using MapType = std::pair; std::vector sortedMap(myMap.begin(), myMap.end()); - std::sort(sortedMap.begin(), sortedMap.end(), - [](const MapType& a, const MapType& b) + std::ranges::sort(sortedMap, [](const MapType& a, const MapType& b) { // Event::Type first if(a.first.key != b.first.key) @@ -290,11 +289,11 @@ json KeyMap::convertLegacyMapping(string_view lm) // Since istringstream swallows whitespace, we have to make the // delimiters be spaces - string list{lm}; - std::replace(list.begin(), list.end(), '|', ' '); - std::replace(list.begin(), list.end(), ':', ' '); - std::replace(list.begin(), list.end(), ',', ' '); - istringstream buf(list); + string lst{lm}; + std::ranges::replace(lst, '|', ' '); + std::ranges::replace(lst, ':', ' '); + std::ranges::replace(lst, ',', ' '); + istringstream buf(lst); int event = 0, key = 0, mod = 0; while(buf >> event && buf >> key && buf >> mod) diff --git a/src/common/PaletteHandler.cxx b/src/common/PaletteHandler.cxx index 6e39ce590..40a7e0123 100644 --- a/src/common/PaletteHandler.cxx +++ b/src/common/PaletteHandler.cxx @@ -478,9 +478,9 @@ void PaletteHandler::generateCustomPalette(ConsoleTiming timing) const float G = Y + dotProduct(IQ[chroma], IQG); float B = Y + dotProduct(IQ[chroma], IQB); - if(R < 0) R = 0; - if(G < 0) G = 0; - if(B < 0) B = 0; + R = std::max(R, 0.F); + G = std::max(G, 0.F); + B = std::max(B, 0.F); R = powf(R, 0.9F); G = powf(G, 0.9F); diff --git a/src/common/PhysicalJoystick.cxx b/src/common/PhysicalJoystick.cxx index 12771c051..384b03ab8 100644 --- a/src/common/PhysicalJoystick.cxx +++ b/src/common/PhysicalJoystick.cxx @@ -112,26 +112,26 @@ json PhysicalJoystick::convertLegacyMapping(string_view mapping, string_view nam { istringstream buf(string{mapping}); // TODO: fixed in C++23 json convertedMapping = json::object(); - string map; + string lmap; // Skip joystick name - getline(buf, map, MODE_DELIM); + getline(buf, lmap, MODE_DELIM); - while (getline(buf, map, MODE_DELIM)) + while (getline(buf, lmap, MODE_DELIM)) { int mode{0}; // Get event mode - std::replace(map.begin(), map.end(), '|', ' '); - istringstream modeBuf(map); + std::ranges::replace(lmap, '|', ' '); + istringstream modeBuf(lmap); modeBuf >> mode; // Remove leading "|" string - map.erase(0, 2); + lmap.erase(0, 2); - const json mappingForMode = JoyMap::convertLegacyMapping(map); + const json lmappingForMode = JoyMap::convertLegacyMapping(lmap); - convertedMapping[jsonName(static_cast(mode))] = mappingForMode; + convertedMapping[jsonName(static_cast(mode))] = lmappingForMode; } convertedMapping["name"] = name; diff --git a/src/common/SoundSDL2.cxx b/src/common/SoundSDL2.cxx index 7f8edeb02..582e2b6a2 100644 --- a/src/common/SoundSDL2.cxx +++ b/src/common/SoundSDL2.cxx @@ -479,7 +479,7 @@ void SoundSDL2::WavHandlerSDL2::processWav(uInt8* stream, uInt32 len) const int newFreq = std::round(static_cast(mySpec.freq) * origLen / len); - if(len > myRemaining) + if(len > myRemaining) // NOLINT(readability-use-std-min-max) len = myRemaining; SDL_AudioCVT cvt; @@ -505,7 +505,7 @@ void SoundSDL2::WavHandlerSDL2::processWav(uInt8* stream, uInt32 len) } else { - if(len > myRemaining) + if(len > myRemaining) // NOLINT(readability-use-std-min-max) len = myRemaining; // Mix volume adjusted WAV data into silent buffer diff --git a/src/common/Version.hxx b/src/common/Version.hxx index 35e139b84..38aa7095e 100644 --- a/src/common/Version.hxx +++ b/src/common/Version.hxx @@ -18,7 +18,7 @@ #ifndef VERSION_HXX #define VERSION_HXX -#define STELLA_VERSION "7.0" +#define STELLA_VERSION "7.1_pre" #define STELLA_BUILD "8005" #endif diff --git a/src/common/ZipHandler.cxx b/src/common/ZipHandler.cxx index e4c7a4aa7..4f99d1366 100644 --- a/src/common/ZipHandler.cxx +++ b/src/common/ZipHandler.cxx @@ -266,8 +266,7 @@ void ZipHandler::ZipFile::readEcd() uInt64 read_length = 0; // Max out the buf length at the size of the file - if(buflen > myLength) - buflen = myLength; + buflen = std::min(buflen, myLength); // Allocate buffer const ByteBuffer buffer = make_unique(buflen + 1); diff --git a/src/common/bspf.hxx b/src/common/bspf.hxx index d61f8e497..545a8e48f 100644 --- a/src/common/bspf.hxx +++ b/src/common/bspf.hxx @@ -56,6 +56,7 @@ using uInt64 = uint64_t; #include #include #include +#include #include #include #include @@ -189,12 +190,12 @@ namespace BSPF // Convert string to given case inline const string& toUpperCase(string& s) { - transform(s.begin(), s.end(), s.begin(), ::toupper); + std::ranges::transform(s, s.begin(), ::toupper); return s; } inline const string& toLowerCase(string& s) { - transform(s.begin(), s.end(), s.begin(), ::tolower); + std::ranges::transform(s, s.begin(), ::tolower); return s; } @@ -426,9 +427,9 @@ namespace BSPF const auto currtime = std::time(nullptr); std::tm tm_snapshot{}; #if (defined BSPF_WINDOWS || defined __WIN32__) && (!defined __GNUG__ || defined __MINGW32__) - localtime_s(&tm_snapshot, &currtime); + std::ignore = localtime_s(&tm_snapshot, &currtime); #else - localtime_r(&currtime, &tm_snapshot); + std::ignore = localtime_r(&currtime, &tm_snapshot); #endif return tm_snapshot; } diff --git a/src/common/main.cxx b/src/common/main.cxx index 701ade014..372d61162 100644 --- a/src/common/main.cxx +++ b/src/common/main.cxx @@ -40,6 +40,8 @@ #include "CheatManager.hxx" #endif +namespace { + /** Parse the commandline arguments and store into the appropriate hashmap. @@ -47,39 +49,6 @@ Some keys are used only by the main function; these are placed in localOpts. The rest are needed globally, and are placed in globalOpts. */ -void parseCommandLine(int ac, const char* const av[], - Settings::Options& globalOpts, Settings::Options& localOpts); - -/** - Checks the commandline for special settings that are used by various ports - to use a specific 'base directory'. - - This needs to be done separately, before either an OSystem or Settings - object can be created, since they both depend on each other, and a - variable basedir implies a different location for the settings file. - - This function will call OSystem::overrideBaseDir() when either of the - applicable arguments are found, and then remove them from the argument - list. -*/ -void checkForCustomBaseDir(Settings::Options& options); - -/** - Checks whether the commandline contains an argument corresponding to - starting a profile session. -*/ -bool isProfilingRun(int ac, char* av[]); - -/** - In Windows, attach console to allow command line output (e.g. for -help). - This is needed since by default Windows doesn't set up stdout/stderr - correctly. -*/ -void attachConsole(); -void freeConsole(); - - -// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void parseCommandLine(int ac, const char* const av[], Settings::Options& globalOpts, Settings::Options& localOpts) { @@ -136,7 +105,18 @@ void parseCommandLine(int ac, const char* const av[], #endif } -// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +/** + Checks the commandline for special settings that are used by various ports + to use a specific 'base directory'. + + This needs to be done separately, before either an OSystem or Settings + object can be created, since they both depend on each other, and a + variable basedir implies a different location for the settings file. + + This function will call OSystem::overrideBaseDir() when either of the + applicable arguments are found, and then remove them from the argument + list. +*/ void checkForCustomBaseDir(Settings::Options& options) { // If both of these are activated, the 'base in app dir' takes precedence @@ -150,14 +130,22 @@ void checkForCustomBaseDir(Settings::Options& options) } } -// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -bool isProfilingRun(int ac, char* av[]) { +/** + Checks whether the commandline contains an argument corresponding to + starting a profile session. +*/ +bool isProfilingRun(int ac, char* av[]) +{ if (ac <= 1) return false; return string(av[1]) == "-profile"; } -// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +/** + In Windows, attach console to allow command line output (e.g. for -help). + This is needed since by default Windows doesn't set up stdout/stderr + correctly. +*/ void attachConsole() { #if defined(BSPF_WINDOWS) @@ -183,7 +171,6 @@ void attachConsole() #endif } -// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void freeConsole() { #if defined(BSPF_WINDOWS) @@ -192,6 +179,9 @@ void freeConsole() #endif } +} // namespace + + // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - #if defined(BSPF_MACOS) int stellaMain(int ac, char* av[]) diff --git a/src/common/repository/KeyValueRepositoryFile.hxx b/src/common/repository/KeyValueRepositoryFile.hxx index ad1e917a8..81fce5e55 100644 --- a/src/common/repository/KeyValueRepositoryFile.hxx +++ b/src/common/repository/KeyValueRepositoryFile.hxx @@ -28,12 +28,14 @@ template class KeyValueRepositoryFile : public KeyValueRepository { public: - explicit KeyValueRepositoryFile(const FSNode& node); - KVRMap load() override; bool save(const KVRMap& values) override; + private: + explicit KeyValueRepositoryFile(const FSNode& node); + friend T; + protected: const FSNode& myNode; // NOLINT: we want a reference here diff --git a/src/common/tv_filters/NTSCFilter.cxx b/src/common/tv_filters/NTSCFilter.cxx index dc509efc3..189487506 100644 --- a/src/common/tv_filters/NTSCFilter.cxx +++ b/src/common/tv_filters/NTSCFilter.cxx @@ -19,8 +19,10 @@ #include "NTSCFilter.hxx" -constexpr float scaleFrom100(float x) { return (x / 50.F) - 1.F; } -constexpr uInt32 scaleTo100(float x) { return static_cast(50.0001F * (x + 1.F)); } +namespace { + constexpr float scaleFrom100(float x) { return (x / 50.F) - 1.F; } + constexpr uInt32 scaleTo100(float x) { return static_cast(50.0001F * (x + 1.F)); } +} // namespace // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - string NTSCFilter::setPreset(Preset preset) diff --git a/src/debugger/Debugger.cxx b/src/debugger/Debugger.cxx index 3095e0acc..0df47bcd0 100644 --- a/src/debugger/Debugger.cxx +++ b/src/debugger/Debugger.cxx @@ -793,8 +793,8 @@ bool Debugger::addFunction(string_view name, string_view definition, // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - bool Debugger::isBuiltinFunction(string_view name) { - return std::any_of(ourBuiltinFunctions.cbegin(), ourBuiltinFunctions.cend(), - [&](const auto& func) { return name == func.name; }); + return std::ranges::any_of(ourBuiltinFunctions, + [&name](const auto& func) { return name == func.name; }); } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -845,10 +845,8 @@ string Debugger::builtinHelp() // Get column widths for aligned output (functions) for(const auto& func: ourBuiltinFunctions) { - size_t len = func.name.size(); - if(len > c_maxlen) c_maxlen = len; - len = func.defn.size(); - if(len > i_maxlen) i_maxlen = len; + c_maxlen = std::max(func.name.size(), c_maxlen); + i_maxlen = std::max(func.defn.size(), i_maxlen); } buf << std::setfill(' ') << "\nBuilt-in functions:\n"; @@ -865,10 +863,7 @@ string Debugger::builtinHelp() // Get column widths for aligned output (pseudo-registers) c_maxlen = 0; for(const auto& reg: ourPseudoRegisters) - { - const size_t len = reg.name.size(); - if(len > c_maxlen) c_maxlen = len; - } + c_maxlen = std::max(reg.name.size(), c_maxlen); buf << "\nPseudo-registers:\n"; for(const auto& reg: ourPseudoRegisters) diff --git a/src/debugger/DebuggerParser.cxx b/src/debugger/DebuggerParser.cxx index 42ac0eafe..b90999b86 100644 --- a/src/debugger/DebuggerParser.cxx +++ b/src/debugger/DebuggerParser.cxx @@ -1481,10 +1481,7 @@ void DebuggerParser::executeHelp() // Find length of longest command size_t clen = 0; for(const auto& c: commands) - { - const size_t len = c.cmdString.length(); - if(len > clen) clen = len; - } + clen = std::max(clen, c.cmdString.length()); commandResult << setfill(' '); for(const auto& c: commands) diff --git a/src/debugger/gui/CartDebugWidget.cxx b/src/debugger/gui/CartDebugWidget.cxx index 85a1b477b..60bd5fb86 100644 --- a/src/debugger/gui/CartDebugWidget.cxx +++ b/src/debugger/gui/CartDebugWidget.cxx @@ -69,8 +69,7 @@ int CartDebugWidget::addBaseInformation(size_t bytes, string_view manufacturer, const StringParser bs(desc, (fwidth - ScrollBarWidget::scrollBarWidth(_font)) / myFontWidth); const StringList& sl = bs.stringList(); - size_t lines = sl.size(); - if(lines < 3) lines = 3; + size_t lines = std::max(sl.size(), 3); bool useScrollbar = false; if(lines > maxlines) { diff --git a/src/debugger/gui/CartRamWidget.cxx b/src/debugger/gui/CartRamWidget.cxx index e68529479..139a649ca 100644 --- a/src/debugger/gui/CartRamWidget.cxx +++ b/src/debugger/gui/CartRamWidget.cxx @@ -64,10 +64,9 @@ CartRamWidget::CartRamWidget( constexpr uInt16 maxlines = 6; const StringParser bs(desc, (fwidth - ScrollBarWidget::scrollBarWidth(_font)) / myFontWidth); const StringList& sl = bs.stringList(); - auto lines = static_cast(sl.size()); - bool useScrollbar = false; - if(lines < 2) lines = 2; + bool useScrollbar = false; + auto lines = std::max(static_cast(sl.size()), 2); if(lines > maxlines) { lines = maxlines; diff --git a/src/debugger/gui/DataGridWidget.cxx b/src/debugger/gui/DataGridWidget.cxx index 84e54155a..f6730a6b4 100644 --- a/src/debugger/gui/DataGridWidget.cxx +++ b/src/debugger/gui/DataGridWidget.cxx @@ -118,10 +118,8 @@ void DataGridWidget::setList(const IntArray& alist, const IntArray& vlist, const size_t size = vlist.size(); // assume the alist is the same size const bool dirty = _editMode - || !std::equal(_valueList.begin(), _valueList.end(), - vlist.begin(), vlist.end()) - || !std::equal(_changedList.begin(), _changedList.end(), - changed.begin(), changed.end()); + || !std::ranges::equal(_valueList, vlist) + || !std::ranges::equal(_changedList, changed); _addrList.clear(); _valueList.clear(); diff --git a/src/debugger/gui/PromptWidget.cxx b/src/debugger/gui/PromptWidget.cxx index 7b6bb5214..bf4f91042 100644 --- a/src/debugger/gui/PromptWidget.cxx +++ b/src/debugger/gui/PromptWidget.cxx @@ -170,9 +170,8 @@ bool PromptWidget::handleKeyDown(StellaKey key, StellaMod mod) if(_scrollLine < _currentPos / _lineWidth) { // Scroll page by page when not at cursor position: - _scrollLine += _linesPerPage; - if(_scrollLine > _promptEndPos / _lineWidth) - _scrollLine = _promptEndPos / _lineWidth; + _scrollLine = std::min(_scrollLine + _linesPerPage, + _promptEndPos / _lineWidth); updateScrollBuffer(); break; } @@ -293,9 +292,8 @@ bool PromptWidget::handleKeyDown(StellaKey key, StellaMod mod) if(_scrollLine < _linesPerPage) break; - _scrollLine -= _linesPerPage - 1; - if(_scrollLine < _firstLineInBuffer + _linesPerPage - 1) - _scrollLine = _firstLineInBuffer + _linesPerPage - 1; + _scrollLine = std::max(_scrollLine - (_linesPerPage - 1), + _firstLineInBuffer + _linesPerPage - 1); updateScrollBuffer(); break; @@ -304,9 +302,8 @@ bool PromptWidget::handleKeyDown(StellaKey key, StellaMod mod) if(_scrollLine >= _promptEndPos / _lineWidth) break; - _scrollLine += _linesPerPage - 1; - if(_scrollLine > _promptEndPos / _lineWidth) - _scrollLine = _promptEndPos / _lineWidth; + _scrollLine = std::min(_scrollLine + (_linesPerPage - 1), + _promptEndPos / _lineWidth); updateScrollBuffer(); break; @@ -316,9 +313,7 @@ bool PromptWidget::handleKeyDown(StellaKey key, StellaMod mod) break; case Event::UIEnd: - _scrollLine = _promptEndPos / _lineWidth; - if(_scrollLine < _linesPerPage - 1) - _scrollLine = _linesPerPage - 1; + _scrollLine = std::max(_promptEndPos / _lineWidth, _linesPerPage - 1); updateScrollBuffer(); break; @@ -727,8 +722,7 @@ bool PromptWidget::autoComplete(int direction) if(_tabCount != -1) len = static_cast(strlen(_inputStr)); - if(len > kLineBufferSize - 1) - len = kLineBufferSize - 1; + len = std::min(len, kLineBufferSize - 1); int lastDelimPos = -1; char delimiter = '\0'; @@ -748,11 +742,11 @@ bool PromptWidget::autoComplete(int direction) if(_tabCount == -1) _inputStr[len] = '\0'; - StringList list; + StringList lst; if(lastDelimPos == -1) // no delimiters, do only command completion: - DebuggerParser::getCompletions(_inputStr, list); + DebuggerParser::getCompletions(_inputStr, lst); else { const size_t strLen = len - lastDelimPos - 1; @@ -761,29 +755,29 @@ bool PromptWidget::autoComplete(int direction) { // Special case for 'help' command if(BSPF::startsWithIgnoreCase(_inputStr, "help")) - DebuggerParser::getCompletions(_inputStr + lastDelimPos + 1, list); + DebuggerParser::getCompletions(_inputStr + lastDelimPos + 1, lst); else { // we got a delimiter, so this must be a label or a function const Debugger& dbg = instance().debugger(); - dbg.cartDebug().getCompletions(_inputStr + lastDelimPos + 1, list); - dbg.getCompletions(_inputStr + lastDelimPos + 1, list); + dbg.cartDebug().getCompletions(_inputStr + lastDelimPos + 1, lst); + dbg.getCompletions(_inputStr + lastDelimPos + 1, lst); } } } - if(list.empty()) + if(lst.empty()) return false; - sort(list.begin(), list.end()); + std::ranges::sort(lst); if(direction < 0) { if(--_tabCount < 0) - _tabCount = static_cast(list.size()) - 1; + _tabCount = static_cast(lst.size()) - 1; } else - _tabCount = (_tabCount + 1) % list.size(); + _tabCount = (_tabCount + 1) % lst.size(); nextLine(); _currentPos = _promptStartPos; @@ -796,7 +790,7 @@ bool PromptWidget::autoComplete(int direction) putcharIntern(delimiter); // ...and add current autocompletion string - print(list[_tabCount]); + print(lst[_tabCount]); putcharIntern(' '); _promptEndPos = _currentPos; diff --git a/src/debugger/gui/RomListWidget.cxx b/src/debugger/gui/RomListWidget.cxx index 26da5bd2d..8e5c76311 100644 --- a/src/debugger/gui/RomListWidget.cxx +++ b/src/debugger/gui/RomListWidget.cxx @@ -164,11 +164,7 @@ void RomListWidget::setHighlighted(int item) // Only scroll the list if we're about to pass the page boundary if (_highlightedItem < _currentPos) - { - _currentPos -= _rows; - if (_currentPos < 0) - _currentPos = 0; - } + _currentPos = std::max(_currentPos - _rows, 0); else if(_highlightedItem == _currentPos + _rows) _currentPos += _rows; @@ -187,10 +183,7 @@ void RomListWidget::recalc() { const int size = static_cast(myDisasm->list.size()); - if (_currentPos >= size) - _currentPos = size - 1; - if (_currentPos < 0) - _currentPos = 0; + _currentPos = BSPF::clamp(_currentPos, 0, size - 1); if(_selectedItem < 0 || _selectedItem >= size) _selectedItem = 0; @@ -378,15 +371,12 @@ bool RomListWidget::handleEvent(Event::Type e) break; case Event::UIPgUp: - _selectedItem -= _rows - 1; - if (_selectedItem < 0) - _selectedItem = 0; + _selectedItem = std::max(_selectedItem - (_rows - 1), 0); break; case Event::UIPgDown: - _selectedItem += _rows - 1; - if (_selectedItem >= static_cast(myDisasm->list.size())) - _selectedItem = static_cast(myDisasm->list.size()) - 1; + _selectedItem = std::min(_selectedItem + (_rows - 1), + static_cast(myDisasm->list.size()) - 1); break; case Event::UIHome: diff --git a/src/debugger/gui/ToggleBitWidget.cxx b/src/debugger/gui/ToggleBitWidget.cxx index 85035e81b..d4effbea8 100644 --- a/src/debugger/gui/ToggleBitWidget.cxx +++ b/src/debugger/gui/ToggleBitWidget.cxx @@ -68,8 +68,7 @@ void ToggleBitWidget::setList(const StringList& off, const StringList& on) // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void ToggleBitWidget::setState(const BoolArray& state, const BoolArray& changed) { - if(!std::equal(_changedList.begin(), _changedList.end(), - changed.begin(), changed.end())) + if(!std::ranges::equal(_changedList, changed)) setDirty(); _stateList.clear(); @@ -78,7 +77,6 @@ void ToggleBitWidget::setState(const BoolArray& state, const BoolArray& changed) _changedList = changed; } - // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - string ToggleBitWidget::getToolTip(const Common::Point& pos) const { diff --git a/src/emucore/CartELF.cxx b/src/emucore/CartELF.cxx index f2dc344d0..2ee2e0d53 100644 --- a/src/emucore/CartELF.cxx +++ b/src/emucore/CartELF.cxx @@ -33,6 +33,8 @@ #include "CartELF.hxx" +// NOLINTBEGIN(bugprone-unchecked-optional-access) + using namespace elfEnvironment; namespace { @@ -178,7 +180,7 @@ namespace { { if (!props) return SystemType::ntsc; - const string displayFormat = props->get(PropType::Display_Format); + const string& displayFormat = props->get(PropType::Display_Format); if(displayFormat == "PAL" || displayFormat == "SECAM") return SystemType::pal; if(displayFormat == "PAL60") return SystemType::pal60; @@ -767,3 +769,5 @@ void CartridgeELF::resetWithConfig() switchExecutionStage(); } + +// NOLINTEND(bugprone-unchecked-optional-access) diff --git a/src/emucore/EventHandler.cxx b/src/emucore/EventHandler.cxx index 2a9551085..204973100 100644 --- a/src/emucore/EventHandler.cxx +++ b/src/emucore/EventHandler.cxx @@ -2051,15 +2051,15 @@ void EventHandler::setComboMap() } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -json EventHandler::convertLegacyComboMapping(string list) +json EventHandler::convertLegacyComboMapping(string lst) { json convertedMapping = json::array(); // Since istringstream swallows whitespace, we have to make the // delimiters be spaces - std::replace(list.begin(), list.end(), ':', ' '); - std::replace(list.begin(), list.end(), ',', ' '); - istringstream buf(list); + std::ranges::replace(lst, ':', ' '); + std::ranges::replace(lst, ',', ' '); + istringstream buf(lst); try { diff --git a/src/emucore/EventHandler.hxx b/src/emucore/EventHandler.hxx index cbf4d91c8..ced9c659e 100644 --- a/src/emucore/EventHandler.hxx +++ b/src/emucore/EventHandler.hxx @@ -475,7 +475,7 @@ class EventHandler void setActionMappings(EventMode mode); void setDefaultKeymap(Event::Type, EventMode mode); void setDefaultJoymap(Event::Type, EventMode mode); - static nlohmann::json convertLegacyComboMapping(string list); + static nlohmann::json convertLegacyComboMapping(string lst); void saveComboMapping(); static StringList getActionList(EventMode mode); diff --git a/src/emucore/FSNode.cxx b/src/emucore/FSNode.cxx index 8bec7acf7..2e90e0b09 100644 --- a/src/emucore/FSNode.cxx +++ b/src/emucore/FSNode.cxx @@ -89,8 +89,7 @@ bool FSNode::getAllChildren(FSList& fslist, ListMode mode, } #endif - std::sort(fslist.begin(), fslist.end(), - [](const FSNode& node1, const FSNode& node2) + std::ranges::sort(fslist, [](const FSNode& node1, const FSNode& node2) { if(node1.isDirectory() != node2.isDirectory()) return node1.isDirectory(); @@ -153,8 +152,8 @@ bool FSNode::getChildren(FSList& fslist, ListMode mode, } #endif - std::sort(tmp.begin(), tmp.end(), - [](const AbstractFSNodePtr& node1, const AbstractFSNodePtr& node2) + std::ranges::sort(tmp, + [](const AbstractFSNodePtr& node1, const AbstractFSNodePtr& node2) { if(node1->isDirectory() != node2->isDirectory()) return node1->isDirectory(); diff --git a/src/emucore/PlusROM.cxx b/src/emucore/PlusROM.cxx index 6c2bf62ff..8810bb6dc 100644 --- a/src/emucore/PlusROM.cxx +++ b/src/emucore/PlusROM.cxx @@ -478,7 +478,7 @@ void PlusROM::receive() // and start over const auto [responseSize, response] = (*iter)->getResponse(); - for(uInt8 i = 0; i < responseSize; i++) + for(size_t i = 0; i < responseSize; ++i) myRxBuffer[myRxWritePos++] = response[i]; myPendingRequests.erase(iter); diff --git a/src/emucore/Settings.cxx b/src/emucore/Settings.cxx index 7be24c464..f0a61eb8c 100644 --- a/src/emucore/Settings.cxx +++ b/src/emucore/Settings.cxx @@ -38,9 +38,9 @@ //#endif #if defined(BSPF_UNIX) || defined(BSPF_MACOS) -#include -#include -#include + #include + #include + #include #endif #include "Settings.hxx" @@ -366,7 +366,7 @@ void Settings::validate() if(i < -5 || i > 5) setValue("tia.vsizeadjust", 0); string s = getString("tia.dbgcolors"); - sort(s.begin(), s.end()); + std::ranges::sort(s); if(s != "bgopry") setValue("tia.dbgcolors", "roygpb"); if(PhosphorHandler::toPhosphorMode(getString(PhosphorHandler::SETTING_MODE)) == PhosphorHandler::ByRom) diff --git a/src/emucore/elf/VcsLib.cxx b/src/emucore/elf/VcsLib.cxx index 6798d27a7..d08406890 100644 --- a/src/emucore/elf/VcsLib.cxx +++ b/src/emucore/elf/VcsLib.cxx @@ -181,7 +181,7 @@ void VcsLib::vcsWrite5(uInt8 zpAddress, uInt8 value) // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void VcsLib::vcsCopyOverblankToRiotRam() { - for (uInt8 i = 0; i < OVERBLANK_PROGRAM_SIZE; i++) + for (uInt32 i = 0; i < OVERBLANK_PROGRAM_SIZE; ++i) vcsWrite5(0x80 + i, OVERBLANK_PROGRAM[i]); } diff --git a/src/gui/Dialog.cxx b/src/gui/Dialog.cxx index a7238e703..8dffd59ef 100644 --- a/src/gui/Dialog.cxx +++ b/src/gui/Dialog.cxx @@ -987,9 +987,8 @@ void Dialog::addOKBGroup(WidgetArray& wid, const GUI::Font& font, VBORDER = Dialog::vBorder(), HBORDER = Dialog::hBorder(); - buttonWidth = std::max(buttonWidth, - std::max(Dialog::buttonWidth(okText), - Dialog::buttonWidth("Cancel"))); + buttonWidth = std::max({buttonWidth, Dialog::buttonWidth(okText), + Dialog::buttonWidth("Cancel")}); _w = std::max(HBORDER * 2 + buttonWidth * 2 + BUTTON_GAP, _w); addOKWidget(new ButtonWidget(this, font, (_w - buttonWidth) / 2, @@ -1007,10 +1006,10 @@ void Dialog::addOKCancelBGroup(WidgetArray& wid, const GUI::Font& font, VBORDER = Dialog::vBorder(), HBORDER = Dialog::hBorder(); - buttonWidth = std::max(buttonWidth, - std::max(Dialog::buttonWidth("Defaults"), - std::max(Dialog::buttonWidth(okText), - Dialog::buttonWidth(cancelText)))); + buttonWidth = std::max({buttonWidth, + Dialog::buttonWidth("Defaults"), + Dialog::buttonWidth(okText), + Dialog::buttonWidth(cancelText)}); _w = std::max(HBORDER * 2 + buttonWidth * 2 + BUTTON_GAP, _w); diff --git a/src/gui/EditableWidget.cxx b/src/gui/EditableWidget.cxx index c857a490f..0c38e549c 100644 --- a/src/gui/EditableWidget.cxx +++ b/src/gui/EditableWidget.cxx @@ -64,9 +64,8 @@ void EditableWidget::setText(string_view str, bool changed) _caretPos = static_cast(_editString.size()); _selectSize = 0; - _editScrollOffset = (_font.getStringWidth(_editString) - (getEditRect().w())); - if (_editScrollOffset < 0) - _editScrollOffset = 0; + _editScrollOffset = std::max(0, + _font.getStringWidth(_editString) - getEditRect().w()); } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -424,8 +423,8 @@ bool EditableWidget::handleKeyDown(StellaKey key, StellaMod mod) if(handled) { // Put caret at last difference - myUndoHandler->lastDiff(_editString, oldString); - setCaretPos(myUndoHandler->lastDiff(_editString, oldString)); + UndoHandler::lastDiff(_editString, oldString); + setCaretPos(UndoHandler::lastDiff(_editString, oldString)); _selectSize = 0; sendCommand(EditableWidget::kChangedCmd, key, _id); } @@ -583,9 +582,7 @@ bool EditableWidget::adjustOffset() if (strWidth - _editScrollOffset < editWidth) { // scroll right - _editScrollOffset = (strWidth - editWidth); - if (_editScrollOffset < 0) - _editScrollOffset = 0; + _editScrollOffset = std::max(strWidth - editWidth, 0); } } diff --git a/src/gui/FavoritesManager.cxx b/src/gui/FavoritesManager.cxx index 66fffbcb1..352cbd0ad 100644 --- a/src/gui/FavoritesManager.cxx +++ b/src/gui/FavoritesManager.cxx @@ -182,8 +182,7 @@ const FavoritesManager::UserList& FavoritesManager::userList() const sortedList.assign(myUserSet.begin(), myUserSet.end()); if(!mySettings.getBool("altsorting")) - std::sort(sortedList.begin(), sortedList.end(), - [](string_view a, string_view b) + std::ranges::sort(sortedList, [](string_view a, string_view b) { // Sort without path const FSNode aNode(a); @@ -221,7 +220,7 @@ void FavoritesManager::addRecent(string_view path) // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - bool FavoritesManager::removeRecent(string_view path) { - auto it = std::find(myRecentList.begin(), myRecentList.end(), path); + auto it = std::ranges::find(myRecentList, path); if(it != myRecentList.end()) myRecentList.erase(it); @@ -246,8 +245,7 @@ const FavoritesManager::RecentList& FavoritesManager::recentList() const { sortedList.assign(myRecentList.begin(), myRecentList.end()); - std::sort(sortedList.begin(), sortedList.end(), - [](string_view a, string_view b) + std::ranges::sort(sortedList, [](string_view a, string_view b) { // Sort alphabetical, without path const FSNode aNode(a); @@ -324,8 +322,7 @@ FavoritesManager::sortedPopularList(bool sortByName) const sortedList.clear(); sortedList.assign(myPopularMap.begin(), myPopularMap.end()); - std::sort(sortedList.begin(), sortedList.end(), - [sortByName](const PopularType& a, const PopularType& b) + std::ranges::sort(sortedList, [sortByName](const PopularType& a, const PopularType& b) { // 1. sort by most popular if(!sortByName && a.second != b.second) diff --git a/src/gui/ListWidget.cxx b/src/gui/ListWidget.cxx index 3705754f7..38b34bde6 100644 --- a/src/gui/ListWidget.cxx +++ b/src/gui/ListWidget.cxx @@ -141,17 +141,14 @@ void ListWidget::setHighlighted(int item) const string& ListWidget::getSelectedString() const { return (_selectedItem >= 0 && _selectedItem < static_cast(_list.size())) - ? _list[_selectedItem] : EmptyString; + ? _list[_selectedItem] + : EmptyString; } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void ListWidget::scrollTo(int item) { - const int size = static_cast(_list.size()); - if (item >= size) - item = size - 1; - if (item < 0) - item = 0; + item = BSPF::clamp(item, 0, static_cast(_list.size() - 1)); if(_currentPos != item) { @@ -178,14 +175,8 @@ void ListWidget::recalc() else _currentPos = size - _rows; } - if (_currentPos < 0) - _currentPos = 0; - - if(_selectedItem >= size) - _selectedItem = size - 1; - if(_selectedItem < 0) - _selectedItem = 0; - + _currentPos = std::max(_currentPos, 0); + _selectedItem = BSPF::clamp(_selectedItem, 0, size - 1); _editMode = false; if(_useScrollbar) @@ -351,16 +342,12 @@ bool ListWidget::handleEvent(Event::Type e) case Event::UIPgUp: case Event::UILeft: - _selectedItem -= _rows - 1; - if (_selectedItem < 0) - _selectedItem = 0; + _selectedItem = std::max(_selectedItem - (_rows - 1), 0); break; case Event::UIPgDown: case Event::UIRight: - _selectedItem += _rows - 1; - if (_selectedItem >= size) - _selectedItem = size - 1; + _selectedItem = std::min(_selectedItem + (_rows - 1), size - 1); break; case Event::UIHome: diff --git a/src/gui/RomImageWidget.cxx b/src/gui/RomImageWidget.cxx index d3f19ba8f..cf5c3f99e 100644 --- a/src/gui/RomImageWidget.cxx +++ b/src/gui/RomImageWidget.cxx @@ -244,8 +244,8 @@ bool RomImageWidget::getImageList(const string& propName, const string& romName, // Sort again, not considering extensions, else would be at // the end of the list - std::sort(myImageList.begin(), myImageList.end(), - [oldFileName](const FSNode& node1, const FSNode& node2) + std::ranges::sort(myImageList, [oldFileName] + (const FSNode& node1, const FSNode& node2) { const int compare = BSPF::compareIgnoreCase( node1.getNameWithExt(), node2.getNameWithExt()); @@ -395,9 +395,9 @@ void RomImageWidget::zoomSurfaces(bool zoomed, bool force) const Int32 lh = maxSize.h - b * 2; const Int32 iw = mySrcRect.w() * scaleDpi; const Int32 ih = mySrcRect.h() * scaleDpi; - const float zoom = std::min(1.F, // do not zoom beyond original size - std::min(static_cast(lw) / iw, - static_cast(lh) / ih)); + const float zoom = std::min({1.F, // do not zoom beyond original size + static_cast(lw) / iw, + static_cast(lh) / ih}); const Int32 w = iw * zoom; const Int32 h = ih * zoom; diff --git a/src/gui/ScrollBarWidget.cxx b/src/gui/ScrollBarWidget.cxx index 623f70fe0..b9c0cdf45 100644 --- a/src/gui/ScrollBarWidget.cxx +++ b/src/gui/ScrollBarWidget.cxx @@ -184,15 +184,10 @@ void ScrollBarWidget::handleMouseMoved(int x, int y) if(_draggingPart == Part::Slider) { + _sliderPos = BSPF::clamp(y - _sliderDeltaMouseDownPos, + _upDownBoxHeight, _h - _upDownBoxHeight - _sliderHeight); + const int old_pos = _currentPos; - _sliderPos = y - _sliderDeltaMouseDownPos; - - if(_sliderPos < _upDownBoxHeight) - _sliderPos = _upDownBoxHeight; - - if(_sliderPos > _h - _upDownBoxHeight - _sliderHeight) - _sliderPos = _h - _upDownBoxHeight - _sliderHeight; - _currentPos = (_sliderPos - _upDownBoxHeight) * (_numEntries - _entriesPerPage) / (_h - 2 * _upDownBoxHeight - _sliderHeight); checkBounds(old_pos); @@ -256,14 +251,12 @@ void ScrollBarWidget::recalc() if(_numEntries > _entriesPerPage) { - _sliderHeight = (_h - 2 * _upDownBoxHeight) * _entriesPerPage / _numEntries; - if(_sliderHeight < _upDownBoxHeight) - _sliderHeight = _upDownBoxHeight; + _sliderHeight = std::max(_upDownBoxHeight, + (_h - 2 * _upDownBoxHeight) * _entriesPerPage / _numEntries); - _sliderPos = _upDownBoxHeight + (_h - 2 * _upDownBoxHeight - _sliderHeight) * - _currentPos / (_numEntries - _entriesPerPage); - if(_sliderPos < 0) - _sliderPos = 0; + _sliderPos = std::max(0, + _upDownBoxHeight + (_h - 2 * _upDownBoxHeight - _sliderHeight) * + _currentPos / (_numEntries - _entriesPerPage)); } else { @@ -309,10 +302,9 @@ void ScrollBarWidget::drawWidget(bool hilite) if(!isSinglePage) { // align slider to scroll intervals - int alignedPos = _upDownBoxHeight + (_h - 2 * _upDownBoxHeight - _sliderHeight) * - _currentPos / (_numEntries - _entriesPerPage); - if(alignedPos < 0) - alignedPos = 0; + const int alignedPos = std::max(0, + _upDownBoxHeight + (_h - 2 * _upDownBoxHeight - _sliderHeight) * + _currentPos / (_numEntries - _entriesPerPage)); s.fillRect(_x + 1, _y + alignedPos - 1, _w - 2, _sliderHeight + 2, (hilite && _part == Part::Slider) ? kScrollColorHi : kScrollColor); diff --git a/src/gui/TabWidget.cxx b/src/gui/TabWidget.cxx index a0d0a47b6..7fcf724a6 100644 --- a/src/gui/TabWidget.cxx +++ b/src/gui/TabWidget.cxx @@ -81,14 +81,12 @@ int TabWidget::addTab(string_view title, int tabWidth) } if(tabWidth == NO_WIDTH) - if(_tabWidth < newWidth) - _tabWidth = newWidth; + _tabWidth = std::max(_tabWidth, newWidth); if(numTabs - fixedTabs) { const int maxWidth = (_w - kTabLeftOffset - fixedWidth) / (numTabs - fixedTabs) - kTabLeftOffset; - if(_tabWidth > maxWidth) - _tabWidth = maxWidth; + _tabWidth = std::min(_tabWidth, maxWidth); } // Activate the new tab diff --git a/src/gui/ToolTip.cxx b/src/gui/ToolTip.cxx index 740397f43..d7fa458ba 100644 --- a/src/gui/ToolTip.cxx +++ b/src/gui/ToolTip.cxx @@ -157,7 +157,7 @@ void ToolTip::show(string_view tip) { string leftStr, rightStr; - surface()->splitString(*myFont, inStr, maxWidth, leftStr, rightStr); + FBSurface::splitString(*myFont, inStr, maxWidth, leftStr, rightStr); width = std::max(width, static_cast(myFont->getStringWidth(leftStr))); inStr = rightStr; } diff --git a/src/tools/TIDY b/src/tools/TIDY index 87221cd23..e004cfaff 100755 --- a/src/tools/TIDY +++ b/src/tools/TIDY @@ -1,12 +1,13 @@ #!/usr/bin/env bash -run-clang-tidy-18 -header-filter=\(.*\.hxx\) \ +run-clang-tidy-19 -header-filter=\(.*\.hxx\) \ -checks=*,\ -abseil*,\ -altera*,\ -android*,\ -fuchsia*,\ -llvmlibc-inline-function-decl,\ +-boost-use-ranges,\ -bugprone-assignment-in-if-condition,\ -bugprone-branch-clone,\ -bugprone-easily-swappable-parameters,\ @@ -15,6 +16,7 @@ run-clang-tidy-18 -header-filter=\(.*\.hxx\) \ -cert-dcl37-c,\ -cert-dcl51-cpp,\ -cert-err58-cpp,\ +-cert-int09-c,\ -clang-analyzer-cplusplus.NewDeleteLeaks,\ -clang-analyzer-optin.performance.Padding,\ -cppcoreguidelines-avoid-c-arrays,\ @@ -60,6 +62,7 @@ run-clang-tidy-18 -header-filter=\(.*\.hxx\) \ -modernize-avoid-bind,\ -modernize-avoid-c-arrays,\ -modernize-pass-by-value,\ +-modernize-use-designated-initializers,\ -modernize-use-equals-delete,\ -modernize-use-nodiscard,\ -modernize-use-trailing-return-type,\ @@ -67,11 +70,13 @@ run-clang-tidy-18 -header-filter=\(.*\.hxx\) \ -readability-avoid-unconditional-preprocessor-if,\ -readability-braces-around-statements,\ -readability-else-after-return,\ +-readability-enum-initial-value,\ -readability-function-cognitive-complexity,\ -readability-identifier-length,\ -readability-implicit-bool-conversion,\ -readability-isolate-declaration,\ -readability-magic-numbers,\ +-readability-math-missing-parentheses,\ -readability-named-parameter,\ -readability-redundant-access-specifiers,\ -readability-simplify-boolean-expr,\