cmake : enable -Wshadow for C++ code

ci/run.sh

@@ -13,6 +13,9 @@
 # # with SYCL support
 # GG_BUILD_SYCL=1 bash ./ci/run.sh ./tmp/results ./tmp/mnt
 #
+# # with METAL support
+# GG_BUILD_METAL=1 bash ./ci/run.sh ./tmp/results ./tmp/mnt
+#
 # # with VULKAN support
 # GG_BUILD_VULKAN=1 bash ./ci/run.sh ./tmp/results ./tmp/mnt
 #

cmake/common.cmake

@@ -15,6 +15,14 @@ function(llama_add_compile_flags)
 
             list(APPEND CXX_FLAGS -Wmissing-declarations -Wmissing-noreturn)
 
+            if (CMAKE_CXX_COMPILER_ID MATCHES "GNU" OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+                list(APPEND CXX_FLAGS -Wshadow)
+
+                if (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+                    list(APPEND CXX_FLAGS -Wshadow-field-in-constructor)
+                endif()
+            endif()
+
             list(APPEND WARNING_FLAGS -Wall -Wextra -Wpedantic -Wcast-qual -Wno-unused-function)
 
             list(APPEND C_FLAGS   ${WARNING_FLAGS})

common/arg.h

@@ -25,51 +25,51 @@ struct common_arg {
     void (*handler_int)    (common_params & params, int) = nullptr;
 
     common_arg(
-        const std::initializer_list<const char *> & args,
-        const char * value_hint,
-        const std::string & help,
+        const std::initializer_list<const char *> & args_,
+        const char * value_hint_,
+        const std::string & help_,
         void (*handler)(common_params & params, const std::string &)
-    ) : args(args), value_hint(value_hint), help(help), handler_string(handler) {}
+    ) : args(args_), value_hint(value_hint_), help(help_), handler_string(handler) {}
 
     common_arg(
-        const std::initializer_list<const char *> & args,
-        const char * value_hint,
-        const std::string & help,
+        const std::initializer_list<const char *> & args_,
+        const char * value_hint_,
+        const std::string & help_,
         void (*handler)(common_params & params, int)
-    ) : args(args), value_hint(value_hint), help(help), handler_int(handler) {}
+    ) : args(args_), value_hint(value_hint_), help(help_), handler_int(handler) {}
 
     common_arg(
-        const std::initializer_list<const char *> & args,
-        const std::string & help,
+        const std::initializer_list<const char *> & args_,
+        const std::string & help_,
         void (*handler)(common_params & params)
-    ) : args(args), help(help), handler_void(handler) {}
+    ) : args(args_), help(help_), handler_void(handler) {}
 
     // support 2 values for arg
     common_arg(
-        const std::initializer_list<const char *> & args,
-        const char * value_hint,
-        const char * value_hint_2,
-        const std::string & help,
+        const std::initializer_list<const char *> & args_,
+        const char * value_hint_,
+        const char * value_hint_2_,
+        const std::string & help_,
         void (*handler)(common_params & params, const std::string &, const std::string &)
-    ) : args(args), value_hint(value_hint), value_hint_2(value_hint_2), help(help), handler_str_str(handler) {}
+    ) : args(args_), value_hint(value_hint_), value_hint_2(value_hint_2_), help(help_), handler_str_str(handler) {}
 
-    common_arg & set_examples(std::initializer_list<enum llama_example> examples);
-    common_arg & set_excludes(std::initializer_list<enum llama_example> excludes);
-    common_arg & set_env(const char * env);
+    common_arg & set_examples(std::initializer_list<enum llama_example> vals);
+    common_arg & set_excludes(std::initializer_list<enum llama_example> vals);
+    common_arg & set_env(const char * val);
     common_arg & set_sparam();
     bool in_example(enum llama_example ex);
     bool is_exclude(enum llama_example ex);
-    bool get_value_from_env(std::string & output);
-    bool has_value_from_env();
-    std::string to_string();
+    bool get_value_from_env(std::string & output) const;
+    bool has_value_from_env() const;
+    std::string to_string() const;
 };
 
 struct common_params_context {
     enum llama_example ex = LLAMA_EXAMPLE_COMMON;
     common_params & params;
     std::vector<common_arg> options;
     void(*print_usage)(int, char **) = nullptr;
-    common_params_context(common_params & params) : params(params) {}
+    common_params_context(common_params & params_) : params(params_) {}
 };
 
 // parse input arguments from CLI

common/common.cpp

@@ -763,9 +763,11 @@ bool fs_create_directory_with_parents(const std::string & path) {
     return true;
 #else
     // if the path already exists, check whether it's a directory
-    struct stat info;
-    if (stat(path.c_str(), &info) == 0) {
-        return S_ISDIR(info.st_mode);
+    {
+        struct stat info;
+        if (stat(path.c_str(), &info) == 0) {
+            return S_ISDIR(info.st_mode);
+        }
     }
 
     size_t pos_slash = 1; // skip leading slashes for directory creation
@@ -796,7 +798,7 @@ bool fs_create_directory_with_parents(const std::string & path) {
 }
 
 std::string fs_get_cache_directory() {
-    std::string cache_directory = "";
+    std::string cache_directory;
     auto ensure_trailing_slash = [](std::string p) {
         // Make sure to add trailing slash
         if (p.back() != DIRECTORY_SEPARATOR) {
@@ -1206,7 +1208,7 @@ static bool common_download_file(const std::string & url, const std::string & pa
     {
         typedef size_t(*CURLOPT_HEADERFUNCTION_PTR)(char *, size_t, size_t, void *);
         auto header_callback = [](char * buffer, size_t /*size*/, size_t n_items, void * userdata) -> size_t {
-            common_load_model_from_url_headers * headers = (common_load_model_from_url_headers *) userdata;
+            common_load_model_from_url_headers * cur = (common_load_model_from_url_headers *) userdata;
 
             static std::regex header_regex("([^:]+): (.*)\r\n");
             static std::regex etag_regex("ETag", std::regex_constants::icase);
@@ -1218,9 +1220,9 @@ static bool common_download_file(const std::string & url, const std::string & pa
                 const std::string & key = match[1];
                 const std::string & value = match[2];
                 if (std::regex_match(key, match, etag_regex)) {
-                    headers->etag = value;
+                    cur->etag = value;
                 } else if (std::regex_match(key, match, last_modified_regex)) {
-                    headers->last_modified = value;
+                    cur->last_modified = value;
                 }
             }
             return n_items;
@@ -1292,18 +1294,18 @@ static bool common_download_file(const std::string & url, const std::string & pa
         curl_easy_setopt(curl.get(), CURLOPT_NOPROGRESS, 0L);
 
         // helper function to hide password in URL
-        auto llama_download_hide_password_in_url = [](const std::string & url) -> std::string {
-            std::size_t protocol_pos = url.find("://");
+        auto llama_download_hide_password_in_url = [](const std::string & url_full) -> std::string {
+            std::size_t protocol_pos = url_full.find("://");
             if (protocol_pos == std::string::npos) {
-                return url;  // Malformed URL
+                return url_full;  // Malformed URL
             }
 
-            std::size_t at_pos = url.find('@', protocol_pos + 3);
+            std::size_t at_pos = url_full.find('@', protocol_pos + 3);
             if (at_pos == std::string::npos) {
-                return url;  // No password in URL
+                return url_full;  // No password in URL
             }
 
-            return url.substr(0, protocol_pos + 3) + "********" + url.substr(at_pos);
+            return url_full.substr(0, protocol_pos + 3) + "********" + url_full.substr(at_pos);
         };
 
         // start the download

common/console.cpp

@@ -43,7 +43,7 @@ namespace console {
     static bool      simple_io        = true;
     static display_t current_display  = reset;
 
-    static FILE*     out              = stdout;
+    static FILE*     fout             = stdout;
 
 #if defined (_WIN32)
     static void*     hConsole;
@@ -110,7 +110,7 @@ namespace console {
 
             tty = fopen("/dev/tty", "w+");
             if (tty != nullptr) {
-                out = tty;
+                fout = tty;
             }
         }
 
@@ -126,7 +126,7 @@ namespace console {
         // Restore settings on POSIX systems
         if (!simple_io) {
             if (tty != nullptr) {
-                out = stdout;
+                fout = stdout;
                 fclose(tty);
                 tty = nullptr;
             }
@@ -145,19 +145,19 @@ namespace console {
             fflush(stdout);
             switch(display) {
                 case reset:
-                    fprintf(out, ANSI_COLOR_RESET);
+                    fprintf(fout, ANSI_COLOR_RESET);
                     break;
                 case prompt:
-                    fprintf(out, ANSI_COLOR_YELLOW);
+                    fprintf(fout, ANSI_COLOR_YELLOW);
                     break;
                 case user_input:
-                    fprintf(out, ANSI_BOLD ANSI_COLOR_GREEN);
+                    fprintf(fout, ANSI_BOLD ANSI_COLOR_GREEN);
                     break;
                 case error:
-                    fprintf(out, ANSI_BOLD ANSI_COLOR_RED);
+                    fprintf(fout, ANSI_BOLD ANSI_COLOR_RED);
             }
             current_display = display;
-            fflush(out);
+            fflush(fout);
         }
     }
 
@@ -233,7 +233,7 @@ namespace console {
             return;
         }
 #endif
-        putc('\b', out);
+        putc('\b', fout);
     }
 
     static int estimateWidth(char32_t codepoint) {
@@ -274,7 +274,7 @@ namespace console {
 #else
         // We can trust expectedWidth if we've got one
         if (expectedWidth >= 0 || tty == nullptr) {
-            fwrite(utf8_codepoint, length, 1, out);
+            fwrite(utf8_codepoint, length, 1, fout);
             return expectedWidth;
         }
 
@@ -311,7 +311,7 @@ namespace console {
         pop_cursor();
         put_codepoint(&ch, 1, 1);
 #else
-        fprintf(out, "\b%c", ch);
+        fprintf(fout, "\b%c", ch);
 #endif
     }
 
@@ -353,7 +353,7 @@ namespace console {
     }
 
     static bool readline_advanced(std::string & line, bool multiline_input) {
-        if (out != stdout) {
+        if (fout != stdout) {
             fflush(stdout);
         }
 
@@ -364,7 +364,7 @@ namespace console {
 
         char32_t input_char;
         while (true) {
-            fflush(out); // Ensure all output is displayed before waiting for input
+            fflush(fout); // Ensure all output is displayed before waiting for input
             input_char = getchar32();
 
             if (input_char == '\r' || input_char == '\n') {
@@ -432,7 +432,7 @@ namespace console {
             line.pop_back();
             if (last == '\\') {
                 line += '\n';
-                fputc('\n', out);
+                fputc('\n', fout);
                 has_more = !has_more;
             } else {
                 // llama will just eat the single space, it won't act as a space
@@ -447,11 +447,11 @@ namespace console {
                 has_more = false;
             } else {
                 line += '\n';
-                fputc('\n', out);
+                fputc('\n', fout);
             }
         }
 
-        fflush(out);
+        fflush(fout);
         return has_more;
     }
 

common/json-schema-to-grammar.cpp

@@ -579,8 +579,8 @@ class SchemaConverter {
                     seq.back().second = false;
                 } else {
                     std::string literal;
-                    auto is_non_literal = [&](char c) {
-                        return NON_LITERAL_SET.find(c) != NON_LITERAL_SET.end();
+                    auto is_non_literal = [&](char ch) {
+                        return NON_LITERAL_SET.find(ch) != NON_LITERAL_SET.end();
                     };
                     while (i < length) {
                         if (sub_pattern[i] == '\\' && i < length - 1) {

common/log.cpp

@@ -255,8 +255,8 @@ struct common_log {
         thrd = std::thread([this]() {
             while (true) {
                 {
-                    std::unique_lock<std::mutex> lock(mtx);
-                    cv.wait(lock, [this]() { return head != tail; });
+                    std::unique_lock<std::mutex> lock_thrd(mtx);
+                    cv.wait(lock_thrd, [this]() { return head != tail; });
 
                     cur = entries[head];
 
@@ -338,16 +338,16 @@ struct common_log {
         resume();
     }
 
-    void set_prefix(bool prefix) {
+    void set_prefix(bool val) {
         std::lock_guard<std::mutex> lock(mtx);
 
-        this->prefix = prefix;
+        prefix = val;
     }
 
-    void set_timestamps(bool timestamps) {
+    void set_timestamps(bool val) {
         std::lock_guard<std::mutex> lock(mtx);
 
-        this->timestamps = timestamps;
+        timestamps = val;
     }
 };
 

examples/batched-bench/batched-bench.cpp

@@ -62,7 +62,7 @@ int main(int argc, char ** argv) {
     llama_batch batch = llama_batch_init(n_kv_max, 0, 1);
 
     // decode in batches of ctx_params.n_batch tokens
-    auto decode_helper = [](llama_context * ctx, llama_batch & batch, int32_t n_batch) {
+    auto decode_helper = [&ctx, &batch](int32_t n_batch) {
         for (int32_t i = 0; i < (int32_t) batch.n_tokens; i += n_batch) {
             const int32_t n_tokens = std::min(n_batch, (int32_t) (batch.n_tokens - i));
 
@@ -94,7 +94,7 @@ int main(int argc, char ** argv) {
             common_batch_add(batch, 0, i, { 0 }, false);
         }
 
-        if (!decode_helper(ctx, batch, ctx_params.n_batch)) {
+        if (!decode_helper(ctx_params.n_batch)) {
             LOG_ERR("%s: llama_decode() failed\n", __func__);
             return 1;
         }
@@ -134,7 +134,7 @@ int main(int argc, char ** argv) {
 
                 llama_kv_cache_clear(ctx);
 
-                if (!decode_helper(ctx, batch, ctx_params.n_batch)) {
+                if (!decode_helper(ctx_params.n_batch)) {
                     LOG_ERR("%s: llama_decode() failed\n", __func__);
                     return 1;
                 }
@@ -156,7 +156,7 @@ int main(int argc, char ** argv) {
                         common_batch_add(batch, 0, pp + i, { j }, true);
                     }
 
-                    if (!decode_helper(ctx, batch, ctx_params.n_batch)) {
+                    if (!decode_helper(ctx_params.n_batch)) {
                         LOG_ERR("%s: llama_decode() failed\n", __func__);
                         return 1;
                     }

examples/convert-llama2c-to-ggml/convert-llama2c-to-ggml.cpp

@@ -471,12 +471,12 @@ struct my_llama_file {
         GGML_ASSERT(ret == 0); // same
     }
 
-    void read_raw(void * ptr, size_t size) {
-        if (size == 0) {
+    void read_raw(void * raw_addr, size_t raw_size) {
+        if (raw_size == 0) {
             return;
         }
         errno = 0;
-        std::size_t ret = std::fread(ptr, size, 1, fp);
+        std::size_t ret = std::fread(raw_addr, raw_size, 1, fp);
         if (ferror(fp)) {
             die_fmt("fread failed: %s", strerror(errno));
         }