fix some warnings from gcc and clang-tidy (#3038)

cebtenzzre · xaedes · web-flow · commit 00d62adb79bf · 2023-09-07T13:22:29.000-04:00
Co-authored-by: xaedes &lt;xaedes@gmail.com&gt;
diff --git a/.clang-tidy b/.clang-tidy
@@ -3,6 +3,7 @@ Checks: >
     bugprone-*,
     -bugprone-easily-swappable-parameters,
     -bugprone-implicit-widening-of-multiplication-result,
+    -bugprone-misplaced-widening-cast,
     -bugprone-narrowing-conversions,
     readability-*,
     -readability-avoid-unconditional-preprocessor-if,
@@ -15,4 +16,8 @@ Checks: >
     -clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,
     performance-*,
     portability-*,
+    misc-*,
+    -misc-const-correctness,
+    -misc-non-private-member-variables-in-classes,
+    -misc-no-recursion,
 FormatStyle: none
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -426,7 +426,7 @@ if (LLAMA_ALL_WARNINGS)
         )
         if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
             # g++ only
-            set(cxx_flags ${cxx_flags} -Wno-format-truncation)
+            set(cxx_flags ${cxx_flags} -Wno-format-truncation -Wno-array-bounds)
         endif()
     else()
         # todo : msvc
diff --git a/Makefile b/Makefile
@@ -134,7 +134,7 @@ MK_CXXFLAGS  += -Wall -Wextra -Wpedantic -Wcast-qual -Wno-unused-function -Wno-m
 
 ifeq '' '$(findstring clang++,$(CXX))'
 	# g++ only
-	MK_CXXFLAGS += -Wno-format-truncation
+	MK_CXXFLAGS += -Wno-format-truncation -Wno-array-bounds
 endif
 
 # OS specific
diff --git a/common/common.cpp b/common/common.cpp
@@ -57,7 +57,7 @@ int32_t get_num_physical_cores() {
             siblings.insert(line);
         }
     }
-    if (siblings.size() > 0) {
+    if (!siblings.empty()) {
         return static_cast<int32_t>(siblings.size());
     }
 #elif defined(__APPLE__) && defined(__MACH__)
diff --git a/common/common.h b/common/common.h
@@ -20,6 +20,9 @@
 #define DIRECTORY_SEPARATOR '/'
 #endif // _WIN32
 
+#define die(msg)          do { fputs("error: " msg "\n", stderr);                  exit(1); } while (0)
+#define die_fmt(fmt, ...) do { fprintf(stderr, "error: " fmt "\n", ##__VA_ARGS__); exit(1); } while (0)
+
 //
 // CLI argument parsing
 //
diff --git a/common/grammar-parser.cpp b/common/grammar-parser.cpp
@@ -415,6 +415,7 @@ namespace grammar_parser {
 
     std::vector<const llama_grammar_element *> parse_state::c_rules() {
         std::vector<const llama_grammar_element *> ret;
+        ret.reserve(rules.size());
         for (const auto & rule : rules) {
             ret.push_back(rule.data());
         }
diff --git a/examples/convert-llama2c-to-ggml/convert-llama2c-to-ggml.cpp b/examples/convert-llama2c-to-ggml/convert-llama2c-to-ggml.cpp
@@ -1,5 +1,6 @@
 #include "ggml.h"
 #include "llama.h"
+#include "common.h"
 
 #include <unordered_map>
 #include <vector>
@@ -499,10 +500,10 @@ struct llama_file {
         errno = 0;
         std::size_t ret = std::fread(ptr, size, 1, fp);
         if (ferror(fp)) {
-            throw std::runtime_error(format("read error: %s", strerror(errno)));
+            die_fmt("fread failed: %s", strerror(errno));
         }
         if (ret != 1) {
-            throw std::runtime_error(std::string("unexpectedly reached end of file"));
+            die("unexpectedly reached end of file");
         }
     }
 
@@ -597,8 +598,7 @@ void load_vocab(const char *filename, Config *config, struct llama_vocab *vocab)
         printf("Assuming llama2.c vocabulary since %s is not a gguf file\n", filename);
         llama_file file(filename, "rb");
         if (!file.fp) {
-            fprintf(stderr, "error: %s: %s\n", strerror(errno), filename);
-            exit(1);
+            die_fmt("%s: %s", strerror(errno), filename);
         }
         const int  n_vocab = config->vocab_size;
         /* uint32_t max_token_length =  */ file.read_u32(); // unused
diff --git a/examples/embd-input/embd-input-lib.cpp b/examples/embd-input/embd-input-lib.cpp
@@ -23,7 +23,7 @@ extern "C" {
 struct MyModel* create_mymodel(int argc, char ** argv) {
     gpt_params params;
 
-    if (gpt_params_parse(argc, argv, params) == false) {
+    if (!gpt_params_parse(argc, argv, params)) {
         return nullptr;
     }
 
diff --git a/examples/embedding/embedding.cpp b/examples/embedding/embedding.cpp
@@ -11,7 +11,7 @@
 int main(int argc, char ** argv) {
     gpt_params params;
 
-    if (gpt_params_parse(argc, argv, params) == false) {
+    if (!gpt_params_parse(argc, argv, params)) {
         return 1;
     }
 
diff --git a/examples/gptneox-wip/falcon-main.cpp b/examples/gptneox-wip/falcon-main.cpp
@@ -953,7 +953,7 @@ int main(int argc, char ** argv) {
 
     gpt_params params;
 
-    if (gpt_params_parse(argc, argv, params) == false) {
+    if (!gpt_params_parse(argc, argv, params)) {
         return 1;
     }
 
diff --git a/examples/gptneox-wip/gptneox-main.cpp b/examples/gptneox-wip/gptneox-main.cpp
@@ -925,7 +925,7 @@ int main(int argc, char ** argv) {
 
     gpt_params params;
 
-    if (gpt_params_parse(argc, argv, params) == false) {
+    if (!gpt_params_parse(argc, argv, params)) {
         return 1;
     }
 
diff --git a/examples/main/main.cpp b/examples/main/main.cpp
@@ -48,8 +48,9 @@ static bool is_interacting = false;
 
 void write_logfile(
     const llama_context * ctx, const gpt_params & params, const llama_model * model,
-    const std::vector<llama_token> input_tokens, const std::string output, const std::vector<llama_token> output_tokens) {
-
+    const std::vector<llama_token> & input_tokens, const std::string & output,
+    const std::vector<llama_token> & output_tokens
+) {
     if (params.logdir.empty()) {
         return;
     }
@@ -109,7 +110,7 @@ int main(int argc, char ** argv) {
     gpt_params params;
     g_params = &params;
 
-    if (gpt_params_parse(argc, argv, params) == false) {
+    if (!gpt_params_parse(argc, argv, params)) {
         return 1;
     }
 
@@ -303,7 +304,7 @@ int main(int argc, char ** argv) {
 
     // debug message about similarity of saved session, if applicable
     size_t n_matching_session_tokens = 0;
-    if (session_tokens.size() > 0) {
+    if (!session_tokens.empty()) {
         for (llama_token id : session_tokens) {
             if (n_matching_session_tokens >= embd_inp.size() || id != embd_inp[n_matching_session_tokens]) {
                 break;
@@ -401,7 +402,7 @@ int main(int argc, char ** argv) {
 
         LOG_TEE("%s: interactive mode on.\n", __func__);
 
-        if (params.antiprompt.size()) {
+        if (!params.antiprompt.empty()) {
             for (const auto & antiprompt : params.antiprompt) {
                 LOG_TEE("Reverse prompt: '%s'\n", antiprompt.c_str());
             }
@@ -499,7 +500,7 @@ int main(int argc, char ** argv) {
 
     while ((n_remain != 0 && !is_antiprompt) || params.interactive) {
         // predict
-        if (embd.size() > 0) {
+        if (!embd.empty()) {
             // Note: n_ctx - 4 here is to match the logic for commandline prompt handling via
             // --prompt or --file which uses the same value.
             int max_embd_size = n_ctx - 4;
@@ -624,7 +625,7 @@ int main(int argc, char ** argv) {
                 LOG("n_past = %d\n", n_past);
             }
 
-            if (embd.size() > 0 && !path_session.empty()) {
+            if (!embd.empty() && !path_session.empty()) {
                 session_tokens.insert(session_tokens.end(), embd.begin(), embd.end());
                 n_session_consumed = session_tokens.size();
             }
@@ -695,7 +696,7 @@ int main(int argc, char ** argv) {
         // if not currently processing queued inputs;
         if ((int) embd_inp.size() <= n_consumed) {
             // check for reverse prompt
-            if (params.antiprompt.size()) {
+            if (!params.antiprompt.empty()) {
                 std::string last_output;
                 for (auto id : last_tokens) {
                     last_output += llama_token_to_piece(ctx, id);
@@ -732,7 +733,7 @@ int main(int argc, char ** argv) {
                 LOG("found EOS token\n");
 
                 if (params.interactive) {
-                    if (params.antiprompt.size() != 0) {
+                    if (!params.antiprompt.empty()) {
                         // tokenize and inject first reverse prompt
                         const auto first_antiprompt = ::llama_tokenize(ctx, params.antiprompt.front(), false);
                         embd_inp.insert(embd_inp.end(), first_antiprompt.begin(), first_antiprompt.end());
diff --git a/examples/perplexity/perplexity.cpp b/examples/perplexity/perplexity.cpp
@@ -655,7 +655,7 @@ int main(int argc, char ** argv) {
     gpt_params params;
 
     params.n_batch = 512;
-    if (gpt_params_parse(argc, argv, params) == false) {
+    if (!gpt_params_parse(argc, argv, params)) {
         return 1;
     }
 
diff --git a/examples/quantize-stats/quantize-stats.cpp b/examples/quantize-stats/quantize-stats.cpp
@@ -71,7 +71,7 @@ void quantize_stats_print_usage(int /*argc*/, char ** argv) {
 }
 
 // Check if a layer is included/excluded by command line
-bool layer_included(const quantize_stats_params params, const std::string & layer) {
+bool layer_included(const quantize_stats_params & params, const std::string & layer) {
     for (const auto& excluded : params.exclude_layers) {
         if (std::regex_search(layer, std::regex(excluded))) {
             return false;
diff --git a/examples/quantize/quantize.cpp b/examples/quantize/quantize.cpp
@@ -143,10 +143,9 @@ int main(int argc, char ** argv) {
         if (!try_parse_ftype(argv[arg_idx], params.ftype, ftype_str)) {
             fprintf(stderr, "%s: invalid ftype '%s'\n", __func__, argv[3]);
             return 1;
-        } else {
-            if (ftype_str == "COPY") {
-               params.only_copy = true;
-            }
+        }
+        if (ftype_str == "COPY") {
+           params.only_copy = true;
         }
         arg_idx++;
     }
diff --git a/examples/save-load-state/save-load-state.cpp b/examples/save-load-state/save-load-state.cpp
@@ -13,7 +13,7 @@ int main(int argc, char ** argv) {
     params.repeat_last_n = 64;
     params.prompt = "The quick brown fox";
 
-    if (gpt_params_parse(argc, argv, params) == false) {
+    if (!gpt_params_parse(argc, argv, params)) {
         return 1;
     }
 
@@ -44,7 +44,7 @@ int main(int argc, char ** argv) {
         llama_free_model(model);
         return 1;
     }
-    auto tokens = llama_tokenize(ctx, params.prompt.c_str(), true);
+    auto tokens = llama_tokenize(ctx, params.prompt, true);
     auto n_prompt_tokens = tokens.size();
     if (n_prompt_tokens < 1) {
         fprintf(stderr, "%s : failed to tokenize prompt\n", __func__);
diff --git a/examples/server/server.cpp b/examples/server/server.cpp
@@ -139,7 +139,7 @@ static std::string tokens_to_output_formatted_string(const llama_context *ctx, c
 }
 
 // convert a vector of completion_token_output to json
-static json probs_vector_to_json(const llama_context *ctx, const std::vector<completion_token_output> probs)
+static json probs_vector_to_json(const llama_context *ctx, const std::vector<completion_token_output> & probs)
 {
     json out = json::array();
     for (const auto &prob : probs)
@@ -271,7 +271,7 @@ struct llama_server_context
         return true;
     }
 
-    std::vector<llama_token> tokenize(json json_prompt, bool add_bos)
+    std::vector<llama_token> tokenize(const json & json_prompt, bool add_bos) const
     {
         // If `add_bos` is true, we only add BOS, when json_prompt is a string,
         // or the first element of the json_prompt array is a string.
@@ -611,7 +611,7 @@ struct llama_server_context
 
     completion_token_output doCompletion()
     {
-        const completion_token_output token_with_probs = nextToken();
+        auto token_with_probs = nextToken();
 
         const std::string token_text = token_with_probs.tok == -1 ? "" : llama_token_to_piece(ctx, token_with_probs.tok);
         generated_text += token_text;
@@ -1255,7 +1255,7 @@ void beam_search_callback(void * callback_data, llama_beams_state beams_state) {
 struct token_translator {
     llama_context * ctx;
     std::string operator()(llama_token tok) const { return llama_token_to_piece(ctx, tok); }
-    std::string operator()(completion_token_output cto) const { return (*this)(cto.tok); }
+    std::string operator()(const completion_token_output & cto) const { return (*this)(cto.tok); }
 };
 
 void append_to_generated_text_from_generated_token_probs(llama_server_context & llama) {
diff --git a/examples/train-text-from-scratch/train-text-from-scratch.cpp b/examples/train-text-from-scratch/train-text-from-scratch.cpp
diff --git a/ggml-alloc.c b/ggml-alloc.c
diff --git a/ggml.c b/ggml.c
diff --git a/llama.cpp b/llama.cpp
diff --git a/tests/test-quantize-perf.cpp b/tests/test-quantize-perf.cpp

Original file line number	Diff line number	Diff line change
`@@ -426,7 +426,7 @@ if (LLAMA_ALL_WARNINGS)`
`426`	`426`	`)`
`427`	`427`	`if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")`
`428`	`428`	`# g++ only`
`429`		`- set(cxx_flags ${cxx_flags} -Wno-format-truncation)`
	`429`	`+ set(cxx_flags ${cxx_flags} -Wno-format-truncation -Wno-array-bounds)`
`430`	`430`	`endif()`
`431`	`431`	`else()`
`432`	`432`	`# todo : msvc`
Original file line number	Diff line number	Diff line change
`@@ -57,7 +57,7 @@ int32_t get_num_physical_cores() {`
`57`	`57`	`siblings.insert(line);`
`58`	`58`	`}`
`59`	`59`	`}`
`60`		`- if (siblings.size() > 0) {`
	`60`	`+ if (!siblings.empty()) {`
`61`	`61`	`return static_cast<int32_t>(siblings.size());`
`62`	`62`	`}`
`63`	`63`	`#elif defined(__APPLE__) && defined(__MACH__)`
Original file line number	Diff line number	Diff line change
`@@ -415,6 +415,7 @@ namespace grammar_parser {`
`415`	`415`
`416`	`416`	`std::vector<const llama_grammar_element *> parse_state::c_rules() {`
`417`	`417`	`std::vector<const llama_grammar_element *> ret;`
	`418`	`+ ret.reserve(rules.size());`
`418`	`419`	`for (const auto & rule : rules) {`
`419`	`420`	`ret.push_back(rule.data());`
`420`	`421`	`}`
Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@ extern "C" {`
`23`	`23`	`struct MyModel* create_mymodel(int argc, char ** argv) {`
`24`	`24`	`gpt_params params;`
`25`	`25`
`26`		`- if (gpt_params_parse(argc, argv, params) == false) {`
	`26`	`+ if (!gpt_params_parse(argc, argv, params)) {`
`27`	`27`	`return nullptr;`
`28`	`28`	`}`
`29`	`29`
Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@`
`11`	`11`	`int main(int argc, char ** argv) {`
`12`	`12`	`gpt_params params;`
`13`	`13`
`14`		`- if (gpt_params_parse(argc, argv, params) == false) {`
	`14`	`+ if (!gpt_params_parse(argc, argv, params)) {`
`15`	`15`	`return 1;`
`16`	`16`	`}`
`17`	`17`
Original file line number	Diff line number	Diff line change
`@@ -953,7 +953,7 @@ int main(int argc, char ** argv) {`
`953`	`953`
`954`	`954`	`gpt_params params;`
`955`	`955`
`956`		`- if (gpt_params_parse(argc, argv, params) == false) {`
	`956`	`+ if (!gpt_params_parse(argc, argv, params)) {`
`957`	`957`	`return 1;`
`958`	`958`	`}`
`959`	`959`
Original file line number	Diff line number	Diff line change
`@@ -925,7 +925,7 @@ int main(int argc, char ** argv) {`
`925`	`925`
`926`	`926`	`gpt_params params;`
`927`	`927`
`928`		`- if (gpt_params_parse(argc, argv, params) == false) {`
	`928`	`+ if (!gpt_params_parse(argc, argv, params)) {`
`929`	`929`	`return 1;`
`930`	`930`	`}`
`931`	`931`
Original file line number	Diff line number	Diff line change
`@@ -655,7 +655,7 @@ int main(int argc, char ** argv) {`
`655`	`655`	`gpt_params params;`
`656`	`656`
`657`	`657`	`params.n_batch = 512;`
`658`		`- if (gpt_params_parse(argc, argv, params) == false) {`
	`658`	`+ if (!gpt_params_parse(argc, argv, params)) {`
`659`	`659`	`return 1;`
`660`	`660`	`}`
`661`	`661`
Original file line number	Diff line number	Diff line change
`@@ -71,7 +71,7 @@ void quantize_stats_print_usage(int /argc/, char ** argv) {`
`71`	`71`	`}`
`72`	`72`
`73`	`73`	`// Check if a layer is included/excluded by command line`
`74`		`-bool layer_included(const quantize_stats_params params, const std::string & layer) {`
	`74`	`+bool layer_included(const quantize_stats_params & params, const std::string & layer) {`
`75`	`75`	`for (const auto& excluded : params.exclude_layers) {`
`76`	`76`	`if (std::regex_search(layer, std::regex(excluded))) {`
`77`	`77`	`return false;`
Original file line number	Diff line number	Diff line change
`@@ -143,10 +143,9 @@ int main(int argc, char ** argv) {`
`143`	`143`	`if (!try_parse_ftype(argv[arg_idx], params.ftype, ftype_str)) {`
`144`	`144`	`fprintf(stderr, "%s: invalid ftype '%s'\n", __func__, argv[3]);`
`145`	`145`	`return 1;`
`146`		`- } else {`
`147`		`- if (ftype_str == "COPY") {`
`148`		`- params.only_copy = true;`
`149`		`- }`
	`146`	`+ }`
	`147`	`+ if (ftype_str == "COPY") {`
	`148`	`+ params.only_copy = true;`
`150`	`149`	`}`
`151`	`150`	`arg_idx++;`
`152`	`151`	`}`