From f85b9a62f562f9b1f79cc83653439baed93f58fd Mon Sep 17 00:00:00 2001 From: Jared Van Bortel Date: Mon, 7 Oct 2024 18:35:08 -0400 Subject: [PATCH 1/7] cmake: establish a basic set of warnings to be be used in C++ projects Signed-off-by: Jared Van Bortel --- common/common.cmake | 46 ++++++++++++++++++++++++++++++++++ gpt4all-backend/CMakeLists.txt | 5 ++++ gpt4all-chat/CMakeLists.txt | 3 +++ 3 files changed, 54 insertions(+) create mode 100644 common/common.cmake diff --git a/common/common.cmake b/common/common.cmake new file mode 100644 index 000000000000..d25e171931ab --- /dev/null +++ b/common/common.cmake @@ -0,0 +1,46 @@ +function(gpt4all_add_warning_options target) + if (MSVC) + return() + endif() + target_compile_options("${target}" PRIVATE + # base options + -Wall + -Wextra + # extra options + -Wcast-align + -Wformat=2 + -Wmissing-include-dirs + -Wnull-dereference + -Wstrict-overflow=2 + -Wvla + # errors + -Werror=format-security + -Werror=init-self + -Werror=pointer-arith + -Werror=undef + # disabled warnings + -Wno-sign-compare + -Wno-unused-parameter + -Wno-missing-braces + -Wno-unused-function + -Wno-unused-variable + ) + if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + target_compile_options("${target}" PRIVATE + -Wduplicated-branches + -Wduplicated-cond + -Wlogical-op + -Wno-reorder + -Wno-null-dereference + ) + elseif (CMAKE_CXX_COMPILER_ID MATCHES "^(Apple)?Clang$") + target_compile_options("${target}" PRIVATE + -Wunreachable-code-break + -Wunreachable-code-return + -Werror=pointer-integer-compare + -Wno-reorder-ctor + -Wno-unused-lambda-capture + -Wno-unused-private-field + ) + endif() +endfunction() diff --git a/gpt4all-backend/CMakeLists.txt b/gpt4all-backend/CMakeLists.txt index 4605b139fd22..3e99c535f999 100644 --- a/gpt4all-backend/CMakeLists.txt +++ b/gpt4all-backend/CMakeLists.txt @@ -1,4 +1,7 @@ cmake_minimum_required(VERSION 3.23) # for FILE_SET + +include(../common/common.cmake) + set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON) set(CMAKE_EXPORT_COMPILE_COMMANDS ON) @@ -151,6 +154,7 @@ foreach(BUILD_VARIANT IN LISTS BUILD_VARIANTS) # Add each individual implementations add_library(llamamodel-mainline-${BUILD_VARIANT} SHARED src/llamamodel.cpp src/llmodel_shared.cpp) + gpt4all_add_warning_options(llamamodel-mainline-${BUILD_VARIANT}) target_compile_definitions(llamamodel-mainline-${BUILD_VARIANT} PRIVATE LLAMA_VERSIONS=>=3 LLAMA_DATE=999999) target_include_directories(llamamodel-mainline-${BUILD_VARIANT} PRIVATE @@ -169,6 +173,7 @@ add_library(llmodel src/llmodel_c.cpp src/llmodel_shared.cpp ) +gpt4all_add_warning_options(llmodel) target_sources(llmodel PUBLIC FILE_SET public_headers TYPE HEADERS BASE_DIRS include FILES include/gpt4all-backend/llmodel.h diff --git a/gpt4all-chat/CMakeLists.txt b/gpt4all-chat/CMakeLists.txt index efd414591a7b..1c357ffd2b16 100644 --- a/gpt4all-chat/CMakeLists.txt +++ b/gpt4all-chat/CMakeLists.txt @@ -1,5 +1,7 @@ cmake_minimum_required(VERSION 3.25) # for try_compile SOURCE_FROM_VAR +include(../common/common.cmake) + set(APP_VERSION_MAJOR 3) set(APP_VERSION_MINOR 4) set(APP_VERSION_PATCH 0) @@ -157,6 +159,7 @@ qt_add_executable(chat src/xlsxtomd.cpp src/xlsxtomd.h ${CHAT_EXE_RESOURCES} ) +gpt4all_add_warning_options(chat) qt_add_qml_module(chat URI gpt4all From 9db13a0bb9b706468cb9dc52b651d5b300a9791f Mon Sep 17 00:00:00 2001 From: Jared Van Bortel Date: Mon, 7 Oct 2024 18:45:55 -0400 Subject: [PATCH 2/7] backend(build): be less verbose This will make it easier to see compiler warnings in output, should they occur. Signed-off-by: Jared Van Bortel --- gpt4all-backend/CMakeLists.txt | 2 -- 1 file changed, 2 deletions(-) diff --git a/gpt4all-backend/CMakeLists.txt b/gpt4all-backend/CMakeLists.txt index 3e99c535f999..4bb7035477ea 100644 --- a/gpt4all-backend/CMakeLists.txt +++ b/gpt4all-backend/CMakeLists.txt @@ -97,8 +97,6 @@ if (LLMODEL_ROCM) list(APPEND BUILD_VARIANTS rocm rocm-avxonly) endif() -set(CMAKE_VERBOSE_MAKEFILE ON) - # Go through each build variant foreach(BUILD_VARIANT IN LISTS BUILD_VARIANTS) # Determine flags From 93c93957bda2a578b47eb2c2b2a3036d1b401ef1 Mon Sep 17 00:00:00 2001 From: Jared Van Bortel Date: Mon, 7 Oct 2024 18:52:23 -0400 Subject: [PATCH 3/7] build: add -Wextra-semi and fix warnings This is an example of a commit that introduces a new warning flag to the build, and fixes any existing problems that the new warning detects. Signed-off-by: Jared Van Bortel --- common/common.cmake | 1 + gpt4all-backend/include/gpt4all-backend/llmodel.h | 2 +- gpt4all-chat/src/chat.h | 2 +- gpt4all-chat/src/chatllm.cpp | 2 +- gpt4all-chat/src/chatmodel.h | 4 ++-- gpt4all-chat/src/modellist.cpp | 4 ++-- 6 files changed, 8 insertions(+), 7 deletions(-) diff --git a/common/common.cmake b/common/common.cmake index d25e171931ab..13605827cdff 100644 --- a/common/common.cmake +++ b/common/common.cmake @@ -8,6 +8,7 @@ function(gpt4all_add_warning_options target) -Wextra # extra options -Wcast-align + -Wextra-semi -Wformat=2 -Wmissing-include-dirs -Wnull-dereference diff --git a/gpt4all-backend/include/gpt4all-backend/llmodel.h b/gpt4all-backend/include/gpt4all-backend/llmodel.h index 9ea14092149f..1b60dae6faa2 100644 --- a/gpt4all-backend/include/gpt4all-backend/llmodel.h +++ b/gpt4all-backend/include/gpt4all-backend/llmodel.h @@ -146,7 +146,7 @@ class LLModel { virtual bool supportsEmbedding() const = 0; virtual bool supportsCompletion() const = 0; virtual bool loadModel(const std::string &modelPath, int n_ctx, int ngl) = 0; - virtual bool isModelBlacklisted(const std::string &modelPath) const { (void)modelPath; return false; }; + virtual bool isModelBlacklisted(const std::string &modelPath) const { (void)modelPath; return false; } virtual bool isEmbeddingModel(const std::string &modelPath) const { (void)modelPath; return false; } virtual bool isModelLoaded() const = 0; virtual size_t requiredMem(const std::string &modelPath, int n_ctx, int ngl) = 0; diff --git a/gpt4all-chat/src/chat.h b/gpt4all-chat/src/chat.h index da644c89e7e1..2d98322fef8d 100644 --- a/gpt4all-chat/src/chat.h +++ b/gpt4all-chat/src/chat.h @@ -33,7 +33,7 @@ class Chat : public QObject Q_PROPERTY(ResponseState responseState READ responseState NOTIFY responseStateChanged) Q_PROPERTY(QList collectionList READ collectionList NOTIFY collectionListChanged) Q_PROPERTY(QString modelLoadingError READ modelLoadingError NOTIFY modelLoadingErrorChanged) - Q_PROPERTY(QString tokenSpeed READ tokenSpeed NOTIFY tokenSpeedChanged); + Q_PROPERTY(QString tokenSpeed READ tokenSpeed NOTIFY tokenSpeedChanged) Q_PROPERTY(QString deviceBackend READ deviceBackend NOTIFY loadedModelInfoChanged) Q_PROPERTY(QString device READ device NOTIFY loadedModelInfoChanged) Q_PROPERTY(QString fallbackReason READ fallbackReason NOTIFY loadedModelInfoChanged) diff --git a/gpt4all-chat/src/chatllm.cpp b/gpt4all-chat/src/chatllm.cpp index 36c3a2694307..2b133ce760bf 100644 --- a/gpt4all-chat/src/chatllm.cpp +++ b/gpt4all-chat/src/chatllm.cpp @@ -585,7 +585,7 @@ bool ChatLLM::loadNewModel(const ModelInfo &modelInfo, QVariantMap &modelLoadPro modelLoadProps.insert("$duration", modelLoadTimer.elapsed() / 1000.); return true; -}; +} bool ChatLLM::isModelLoaded() const { diff --git a/gpt4all-chat/src/chatmodel.h b/gpt4all-chat/src/chatmodel.h index d50103151499..43b96246252b 100644 --- a/gpt4all-chat/src/chatmodel.h +++ b/gpt4all-chat/src/chatmodel.h @@ -65,8 +65,8 @@ struct ChatItem Q_PROPERTY(bool thumbsDownState MEMBER thumbsDownState) Q_PROPERTY(QList sources MEMBER sources) Q_PROPERTY(QList consolidatedSources MEMBER consolidatedSources) - Q_PROPERTY(QList promptAttachments MEMBER promptAttachments); - Q_PROPERTY(QString promptPlusAttachments READ promptPlusAttachments); + Q_PROPERTY(QList promptAttachments MEMBER promptAttachments) + Q_PROPERTY(QString promptPlusAttachments READ promptPlusAttachments) public: QString promptPlusAttachments() const diff --git a/gpt4all-chat/src/modellist.cpp b/gpt4all-chat/src/modellist.cpp index d53c8fbfa316..35d8358b8e52 100644 --- a/gpt4all-chat/src/modellist.cpp +++ b/gpt4all-chat/src/modellist.cpp @@ -518,12 +518,12 @@ QString ModelList::compatibleModelNameHash(QUrl baseUrl, QString modelName) { QCryptographicHash sha256(QCryptographicHash::Sha256); sha256.addData((baseUrl.toString() + "_" + modelName).toUtf8()); return sha256.result().toHex(); -}; +} QString ModelList::compatibleModelFilename(QUrl baseUrl, QString modelName) { QString hash(compatibleModelNameHash(baseUrl, modelName)); return QString(u"gpt4all-%1-capi.rmodel"_s).arg(hash); -}; +} bool ModelList::eventFilter(QObject *obj, QEvent *ev) { From 77788fa9446133baea27025aea5e62b39e3fdae1 Mon Sep 17 00:00:00 2001 From: Jared Van Bortel Date: Mon, 7 Oct 2024 18:54:23 -0400 Subject: [PATCH 4/7] fix doubled semicolons Signed-off-by: Jared Van Bortel --- gpt4all-chat/src/database.cpp | 2 +- gpt4all-chat/src/modellist.cpp | 6 +++--- gpt4all-chat/src/mysettings.cpp | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/gpt4all-chat/src/database.cpp b/gpt4all-chat/src/database.cpp index 9b1e9ecdb624..f89d3378d33e 100644 --- a/gpt4all-chat/src/database.cpp +++ b/gpt4all-chat/src/database.cpp @@ -1659,7 +1659,7 @@ void Database::scanQueue() if (info.isPdf()) { QPdfDocument doc; if (doc.load(document_path) != QPdfDocument::Error::None) { - qWarning() << "ERROR: Could not load pdf" << document_id << document_path;; + qWarning() << "ERROR: Could not load pdf" << document_id << document_path; return updateFolderToIndex(folder_id, countForFolder); } title = doc.metaData(QPdfDocument::MetaDataField::Title).toString(); diff --git a/gpt4all-chat/src/modellist.cpp b/gpt4all-chat/src/modellist.cpp index 35d8358b8e52..bdc571fe1097 100644 --- a/gpt4all-chat/src/modellist.cpp +++ b/gpt4all-chat/src/modellist.cpp @@ -502,7 +502,7 @@ ModelList::ModelList() connect(MySettings::globalInstance(), &MySettings::contextLengthChanged, this, &ModelList::updateDataForSettings); connect(MySettings::globalInstance(), &MySettings::gpuLayersChanged, this, &ModelList::updateDataForSettings); connect(MySettings::globalInstance(), &MySettings::repeatPenaltyChanged, this, &ModelList::updateDataForSettings); - connect(MySettings::globalInstance(), &MySettings::repeatPenaltyTokensChanged, this, &ModelList::updateDataForSettings);; + connect(MySettings::globalInstance(), &MySettings::repeatPenaltyTokensChanged, this, &ModelList::updateDataForSettings); connect(MySettings::globalInstance(), &MySettings::promptTemplateChanged, this, &ModelList::updateDataForSettings); connect(MySettings::globalInstance(), &MySettings::systemPromptChanged, this, &ModelList::updateDataForSettings); connect(&m_networkManager, &QNetworkAccessManager::sslErrors, this, &ModelList::handleSslErrors); @@ -2100,7 +2100,7 @@ void ModelList::parseDiscoveryJsonFile(const QByteArray &jsonData) emit discoverProgressChanged(); if (!m_discoverNumberOfResults) { m_discoverInProgress = false; - emit discoverInProgressChanged();; + emit discoverInProgressChanged(); } } @@ -2178,7 +2178,7 @@ void ModelList::handleDiscoveryItemFinished() if (discoverProgress() >= 1.0) { emit layoutChanged(); m_discoverInProgress = false; - emit discoverInProgressChanged();; + emit discoverInProgressChanged(); } reply->deleteLater(); diff --git a/gpt4all-chat/src/mysettings.cpp b/gpt4all-chat/src/mysettings.cpp index 97af196fa4ca..38c8ab6821f5 100644 --- a/gpt4all-chat/src/mysettings.cpp +++ b/gpt4all-chat/src/mysettings.cpp @@ -186,7 +186,7 @@ void MySettings::restoreModelDefaults(const ModelInfo &info) setModelTemperature(info, info.m_temperature); setModelTopP(info, info.m_topP); setModelMinP(info, info.m_minP); - setModelTopK(info, info.m_topK);; + setModelTopK(info, info.m_topK); setModelMaxLength(info, info.m_maxLength); setModelPromptBatchSize(info, info.m_promptBatchSize); setModelContextLength(info, info.m_contextLength); From 29b667e07c66ca655b2dfc8daefe7805857a7772 Mon Sep 17 00:00:00 2001 From: Jared Van Bortel Date: Mon, 7 Oct 2024 18:59:02 -0400 Subject: [PATCH 5/7] build: enable -Wunused-lambda-capture and fix warning Signed-off-by: Jared Van Bortel --- common/common.cmake | 1 - gpt4all-backend/src/llmodel_shared.cpp | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/common/common.cmake b/common/common.cmake index 13605827cdff..264c2ad8169f 100644 --- a/common/common.cmake +++ b/common/common.cmake @@ -40,7 +40,6 @@ function(gpt4all_add_warning_options target) -Wunreachable-code-return -Werror=pointer-integer-compare -Wno-reorder-ctor - -Wno-unused-lambda-capture -Wno-unused-private-field ) endif() diff --git a/gpt4all-backend/src/llmodel_shared.cpp b/gpt4all-backend/src/llmodel_shared.cpp index b0c31d11e78b..c1e969d4b940 100644 --- a/gpt4all-backend/src/llmodel_shared.cpp +++ b/gpt4all-backend/src/llmodel_shared.cpp @@ -260,7 +260,7 @@ void LLModel::generateResponse(std::function cachedTokens.push_back(new_tok.value()); cachedResponse += new_piece; - auto accept = [this, &promptCtx, &cachedTokens, &new_tok, allowContextShift]() -> bool { + auto accept = [this, &promptCtx, &new_tok, allowContextShift]() -> bool { // Shift context if out of space if (promptCtx.n_past >= promptCtx.n_ctx) { (void)allowContextShift; From 0e7b13cb81b1d9d8cd030fe1fd7737aaa594d531 Mon Sep 17 00:00:00 2001 From: Jared Van Bortel Date: Mon, 7 Oct 2024 19:00:47 -0400 Subject: [PATCH 6/7] build: enable -Wunused-private-field and fix warning Signed-off-by: Jared Van Bortel --- common/common.cmake | 1 - gpt4all-chat/src/database.h | 1 - 2 files changed, 2 deletions(-) diff --git a/common/common.cmake b/common/common.cmake index 264c2ad8169f..ce3a91155f26 100644 --- a/common/common.cmake +++ b/common/common.cmake @@ -40,7 +40,6 @@ function(gpt4all_add_warning_options target) -Wunreachable-code-return -Werror=pointer-integer-compare -Wno-reorder-ctor - -Wno-unused-private-field ) endif() endfunction() diff --git a/gpt4all-chat/src/database.h b/gpt4all-chat/src/database.h index 113a007667a4..1a93e3b137bc 100644 --- a/gpt4all-chat/src/database.h +++ b/gpt4all-chat/src/database.h @@ -176,7 +176,6 @@ class ChunkStreamer { QString m_author; QString m_subject; QString m_keywords; - bool m_atStart; // working state QString m_chunk; // has a trailing space for convenience From fc7efd97fc21baa0b003d19eef48d1a1f5610661 Mon Sep 17 00:00:00 2001 From: Jared Van Bortel Date: Mon, 7 Oct 2024 19:08:55 -0400 Subject: [PATCH 7/7] build: enable -Wmissing-braces and fix warning Signed-off-by: Jared Van Bortel --- common/common.cmake | 1 - gpt4all-chat/src/database.cpp | 10 ++++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/common/common.cmake b/common/common.cmake index ce3a91155f26..a60eb33af851 100644 --- a/common/common.cmake +++ b/common/common.cmake @@ -22,7 +22,6 @@ function(gpt4all_add_warning_options target) # disabled warnings -Wno-sign-compare -Wno-unused-parameter - -Wno-missing-braces -Wno-unused-function -Wno-unused-variable ) diff --git a/gpt4all-chat/src/database.cpp b/gpt4all-chat/src/database.cpp index f89d3378d33e..02261cb4c92c 100644 --- a/gpt4all-chat/src/database.cpp +++ b/gpt4all-chat/src/database.cpp @@ -296,10 +296,12 @@ static bool selectAllUncompletedChunks(QSqlQuery &q, QHash