From 439e68c1e5889a01116ba6eec1c03c9fe11bfaa0 Mon Sep 17 00:00:00 2001 From: Georgi Gerganov Date: Sun, 12 Jan 2025 15:29:33 +0200 Subject: [PATCH] cmake : re-enable GCC -Wshadow ggml-ci --- cmake/common.cmake | 7 ++++-- common/arg.h | 34 ++++++++++++++-------------- examples/export-lora/export-lora.cpp | 8 +++---- examples/gguf-split/gguf-split.cpp | 16 ++++++------- examples/run/CMakeLists.txt | 9 ++++++-- examples/server/server.cpp | 2 +- src/llama-adapter.h | 2 +- src/llama-arch.cpp | 2 +- src/llama-arch.h | 2 +- src/llama-context.h | 4 ++-- src/llama-impl.cpp | 2 +- src/llama-model-loader.h | 2 +- src/llama-model.cpp | 2 +- src/llama-quant.cpp | 6 ++--- src/llama-vocab.cpp | 14 ++++++------ src/llama.cpp | 26 ++++++++++----------- 16 files changed, 73 insertions(+), 65 deletions(-) diff --git a/cmake/common.cmake b/cmake/common.cmake index 5dee785c3..45bac7af8 100644 --- a/cmake/common.cmake +++ b/cmake/common.cmake @@ -15,9 +15,12 @@ function(llama_add_compile_flags) list(APPEND CXX_FLAGS -Wmissing-declarations -Wmissing-noreturn) - # GCC -Wshadow is way too agressive - if (CMAKE_CXX_COMPILER_ID MATCHES "Clang") + 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 -Wshadow-field-in-constructor) + endif() endif() list(APPEND WARNING_FLAGS -Wall -Wextra -Wpedantic -Wcast-qual -Wno-unused-function) diff --git a/common/arg.h b/common/arg.h index d88efa462..eff9e6e1f 100644 --- a/common/arg.h +++ b/common/arg.h @@ -25,33 +25,33 @@ struct common_arg { void (*handler_int) (common_params & params, int) = nullptr; common_arg( - const std::initializer_list & args, - const char * value_hint, - const std::string & help, + const std::initializer_list & 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 & args, - const char * value_hint, - const std::string & help, + const std::initializer_list & 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 & args, - const std::string & help, + const std::initializer_list & 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 & args, - const char * value_hint, - const char * value_hint_2, - const std::string & help, + const std::initializer_list & 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 vals); common_arg & set_excludes(std::initializer_list vals); @@ -69,7 +69,7 @@ struct common_params_context { common_params & params; std::vector 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 diff --git a/examples/export-lora/export-lora.cpp b/examples/export-lora/export-lora.cpp index 99063b5d5..592cffbf4 100644 --- a/examples/export-lora/export-lora.cpp +++ b/examples/export-lora/export-lora.cpp @@ -66,7 +66,7 @@ struct file_input { float alpha; float scale; - file_input(std::string & fname, float scale): f_in(fname, std::ios::binary), scale(scale) { + file_input(std::string & fname, float scale_): f_in(fname, std::ios::binary), scale(scale_) { if (!f_in.is_open()) { throw std::runtime_error("failed to open input gguf from " + fname); } @@ -131,7 +131,7 @@ struct lora_merge_ctx { std::string & base_fname, std::vector & lora_files, std::string & outfile, - int n_threads) : base_model(base_fname, 0), n_threads(n_threads), fout(outfile, std::ios::binary) { + int n_threads_) : base_model(base_fname, 0), n_threads(n_threads_), fout(outfile, std::ios::binary) { fout.exceptions(std::ofstream::failbit); // fail fast on write errors if (gguf_find_key(base_model.ctx_gguf, LLM_KV_SPLIT_COUNT) >= 0) { @@ -157,7 +157,7 @@ struct lora_merge_ctx { allocr = ggml_gallocr_new(ggml_backend_get_default_buffer_type(backend)); } - void check_metadata_lora(file_input * adapter) { + void check_metadata_lora(const file_input * adapter) const { auto general_type = get_kv_str(adapter->ctx_gguf, "general.type"); if (general_type != "adapter") { throw std::runtime_error("expect general.type to be 'adapter', but got: " + general_type); @@ -175,7 +175,7 @@ struct lora_merge_ctx { } } - ggml_type get_out_tensor_type(struct ggml_tensor * t) { + static ggml_type get_out_tensor_type(struct ggml_tensor * t) { if (t->type == GGML_TYPE_F32) { return GGML_TYPE_F32; } else { diff --git a/examples/gguf-split/gguf-split.cpp b/examples/gguf-split/gguf-split.cpp index ef3ceb686..3b9ae6a58 100644 --- a/examples/gguf-split/gguf-split.cpp +++ b/examples/gguf-split/gguf-split.cpp @@ -204,14 +204,14 @@ struct split_strategy { // temporary buffer for reading in tensor data std::vector read_buf; - split_strategy(const split_params & params, - std::ifstream & f_input, - struct gguf_context * ctx_gguf, - struct ggml_context * ctx_meta) : - params(params), - f_input(f_input), - ctx_gguf(ctx_gguf), - ctx_meta(ctx_meta), + split_strategy(const split_params & params_, + std::ifstream & f_input_, + struct gguf_context * ctx_gguf_, + struct ggml_context * ctx_meta_) : + params(params_), + f_input(f_input_), + ctx_gguf(ctx_gguf_), + ctx_meta(ctx_meta_), n_tensors(gguf_get_n_tensors(ctx_gguf)) { // because we need to know list of tensors for each file in advance, we will build all the ctx_out for all output splits diff --git a/examples/run/CMakeLists.txt b/examples/run/CMakeLists.txt index 5e9c57bbc..8735c9dc2 100644 --- a/examples/run/CMakeLists.txt +++ b/examples/run/CMakeLists.txt @@ -4,6 +4,11 @@ install(TARGETS ${TARGET} RUNTIME) target_link_libraries(${TARGET} PRIVATE common llama ${CMAKE_THREAD_LIBS_INIT}) target_compile_features(${TARGET} PRIVATE cxx_std_17) -if (CMAKE_CXX_COMPILER_ID MATCHES "Clang") - target_compile_options(${TARGET} PRIVATE -Wno-shadow) # TMP +# TMP +if (CMAKE_CXX_COMPILER_ID MATCHES "GNU" OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") + target_compile_options(${TARGET} PRIVATE -Wno-shadow) + + if (CMAKE_CXX_COMPILER_ID MATCHES "Clang") + target_compile_options(${TARGET} PRIVATE -Wno-shadow-field-in-constructor) + endif() endif() diff --git a/examples/server/server.cpp b/examples/server/server.cpp index aa8b54680..0c0f066ca 100644 --- a/examples/server/server.cpp +++ b/examples/server/server.cpp @@ -200,7 +200,7 @@ struct server_task { // used by SERVER_TASK_TYPE_SET_LORA std::vector set_lora; - server_task(server_task_type type) : type(type) {} + server_task(server_task_type type_) : type(type_) {} static slot_params params_from_json_cmpl( const llama_context * ctx, diff --git a/src/llama-adapter.h b/src/llama-adapter.h index 603fa08f6..7cfc49689 100644 --- a/src/llama-adapter.h +++ b/src/llama-adapter.h @@ -55,7 +55,7 @@ struct llama_adapter_lora_weight { } llama_adapter_lora_weight() = default; - llama_adapter_lora_weight(struct ggml_tensor * a, struct ggml_tensor * b) : a(a), b(b) {} + llama_adapter_lora_weight(struct ggml_tensor * a_, struct ggml_tensor * b_) : a(a_), b(b_) {} }; struct llama_adapter_lora { diff --git a/src/llama-arch.cpp b/src/llama-arch.cpp index 5c1f14cfd..17d7939af 100644 --- a/src/llama-arch.cpp +++ b/src/llama-arch.cpp @@ -1443,7 +1443,7 @@ static const std::map LLM_TENSOR_INFOS = { {LLM_TENSOR_CONVNEXT_GAMMA, {LLM_TENSOR_LAYER_REPEATING, GGML_OP_MUL}}, }; -LLM_KV::LLM_KV(llm_arch arch) : arch(arch) {} +LLM_KV::LLM_KV(llm_arch arch_) : arch(arch_) {} std::string LLM_KV::operator()(llm_kv kv) const { return ::format(LLM_KV_NAMES.at(kv), LLM_ARCH_NAMES.at(arch)); diff --git a/src/llama-arch.h b/src/llama-arch.h index 349844790..d6a79db1e 100644 --- a/src/llama-arch.h +++ b/src/llama-arch.h @@ -374,7 +374,7 @@ struct LLM_TN_IMPL { }; struct LLM_TN { - LLM_TN(llm_arch arch) : arch(arch) {} + LLM_TN(llm_arch arch_) : arch(arch_) {} llm_arch arch; diff --git a/src/llama-context.h b/src/llama-context.h index a9268b292..70c3d0ad7 100644 --- a/src/llama-context.h +++ b/src/llama-context.h @@ -15,8 +15,8 @@ #include struct llama_context { - llama_context(const llama_model & model) - : model(model) + llama_context(const llama_model & model_) + : model(model_) , t_start_us(model.t_start_us) , t_load_us(model.t_load_us) {} diff --git a/src/llama-impl.cpp b/src/llama-impl.cpp index 6ec709dd3..37cf7cdb7 100644 --- a/src/llama-impl.cpp +++ b/src/llama-impl.cpp @@ -17,7 +17,7 @@ struct llama_logger_state { static llama_logger_state g_logger_state; -time_meas::time_meas(int64_t & t_acc, bool disable) : t_start_us(disable ? -1 : ggml_time_us()), t_acc(t_acc) {} +time_meas::time_meas(int64_t & t_acc_, bool disable) : t_start_us(disable ? -1 : ggml_time_us()), t_acc(t_acc_) {} time_meas::~time_meas() { if (t_start_us >= 0) { diff --git a/src/llama-model-loader.h b/src/llama-model-loader.h index 4814bbdc9..2164da710 100644 --- a/src/llama-model-loader.h +++ b/src/llama-model-loader.h @@ -31,7 +31,7 @@ struct llama_model_loader { ggml_tensor * tensor; - llama_tensor_weight(const llama_file * file, uint16_t idx, const struct gguf_context * gguf_ctx, ggml_tensor * tensor) : idx(idx), tensor(tensor) { + llama_tensor_weight(const llama_file * file, uint16_t idx_, const struct gguf_context * gguf_ctx, ggml_tensor * tensor_) : idx(idx_), tensor(tensor_) { const int tensor_idx = gguf_find_tensor(gguf_ctx, ggml_get_name(tensor)); if (tensor_idx < 0) { throw std::runtime_error(format("tensor '%s' not found in the model", ggml_get_name(tensor))); diff --git a/src/llama-model.cpp b/src/llama-model.cpp index 1229d8738..01a3afa40 100644 --- a/src/llama-model.cpp +++ b/src/llama-model.cpp @@ -369,7 +369,7 @@ struct llama_model::impl { std::vector dev_layer; }; -llama_model::llama_model(const struct llama_model_params & params) : params(params), pimpl(std::make_unique()) { +llama_model::llama_model(const struct llama_model_params & params_) : params(params_), pimpl(std::make_unique()) { } llama_model::~llama_model() {} diff --git a/src/llama-quant.cpp b/src/llama-quant.cpp index 6c59e1730..75899d142 100644 --- a/src/llama-quant.cpp +++ b/src/llama-quant.cpp @@ -41,9 +41,9 @@ struct quantize_state_impl { // used to figure out if a model shares tok_embd with the output weight bool has_output = false; - quantize_state_impl(const llama_model & model, const llama_model_quantize_params * params) - : model(model) - , params(params) + quantize_state_impl(const llama_model & model_, const llama_model_quantize_params * params_) + : model(model_) + , params(params_) {} }; diff --git a/src/llama-vocab.cpp b/src/llama-vocab.cpp index df6bcdf6a..ef108b991 100644 --- a/src/llama-vocab.cpp +++ b/src/llama-vocab.cpp @@ -115,7 +115,7 @@ struct llm_tokenizer_spm : llm_tokenizer { }; struct llm_tokenizer_spm_session { - llm_tokenizer_spm_session(const llama_vocab & vocab) : vocab(vocab) {} + llm_tokenizer_spm_session(const llama_vocab & vocab_) : vocab(vocab_) {} void tokenize(const std::string & text, std::vector & output) { // split string into utf8 chars @@ -415,7 +415,7 @@ struct llm_tokenizer_bpe : llm_tokenizer { }; struct llm_tokenizer_bpe_session { - llm_tokenizer_bpe_session(const llama_vocab & vocab, const llm_tokenizer_bpe & tokenizer) : vocab(vocab), tokenizer(tokenizer) {} + llm_tokenizer_bpe_session(const llama_vocab & vocab_, const llm_tokenizer_bpe & tokenizer_) : vocab(vocab_), tokenizer(tokenizer_) {} static void append(const llama_token token_id, std::vector & output) { output.push_back(token_id); @@ -603,7 +603,7 @@ struct llm_tokenizer_wpm : llm_tokenizer { }; struct llm_tokenizer_wpm_session { - llm_tokenizer_wpm_session(const llama_vocab & vocab) : vocab(vocab) {} + llm_tokenizer_wpm_session(const llama_vocab & vocab_) : vocab(vocab_) {} void tokenize(const std::string & text, std::vector & output) { // normalize and split by whitespace @@ -782,7 +782,7 @@ struct llm_tokenizer_ugm : llm_tokenizer { }; struct llm_tokenizer_ugm_session { - llm_tokenizer_ugm_session(const llama_vocab & vocab, const llm_tokenizer_ugm & tokenizer) : vocab(vocab), tokenizer(tokenizer) {} + llm_tokenizer_ugm_session(const llama_vocab & vocab_, const llm_tokenizer_ugm & tokenizer_) : vocab(vocab_), tokenizer(tokenizer_) {} /* This implementation is based on SentencePiece optimized Viterbi algorithm for * unigram language models. The general idea is to: @@ -949,7 +949,7 @@ private: */ struct xcda_array_view { public: - xcda_array_view(const uint32_t * xcda_array, size_t xcda_array_size) : xcda_array(xcda_array), xcda_array_size(xcda_array_size) { + xcda_array_view(const uint32_t * xcda_array_, size_t xcda_array_size_) : xcda_array(xcda_array_), xcda_array_size(xcda_array_size_) { } uint32_t get_base(size_t index) { uint32_t packed_node = get_node(index); @@ -1135,7 +1135,7 @@ struct llm_tokenizer_rwkv : llm_tokenizer { }; struct llm_tokenizer_rwkv_session { - llm_tokenizer_rwkv_session(const llama_vocab & vocab, const llm_tokenizer_rwkv & tokenizer) : vocab(vocab), tokenizer(tokenizer) {} + llm_tokenizer_rwkv_session(const llama_vocab & vocab_, const llm_tokenizer_rwkv & tokenizer_) : vocab(vocab_), tokenizer(tokenizer_) {} void tokenize(const std::string & text, std::vector & output) { uint32_t position = 0; @@ -1262,7 +1262,7 @@ struct llama_vocab::impl { std::vector precompiled_charsmap; - impl(const llama_vocab & vocab) : vocab(vocab) { + impl(const llama_vocab & vocab_) : vocab(vocab_) { } ~impl() = default; diff --git a/src/llama.cpp b/src/llama.cpp index d907c2d6e..094ed0024 100644 --- a/src/llama.cpp +++ b/src/llama.cpp @@ -1089,16 +1089,16 @@ struct llm_build_context { // TODO: consider making the entire interface noexcept llm_build_context( - llama_context & lctx, - const llama_ubatch & ubatch, - const llm_build_cb & cb, + llama_context & lctx_, + const llama_ubatch & ubatch_, + const llm_build_cb & cb_, bool worst_case) : - model (lctx.model), - lctx (lctx), + model (lctx_.model), + lctx (lctx_), hparams (model.hparams), - cparams (lctx.cparams), - ubatch (ubatch), - kv_self (lctx.kv_self), + cparams (lctx_.cparams), + ubatch (ubatch_), + kv_self (lctx_.kv_self), n_embd (hparams.n_embd), n_layer (hparams.n_layer), n_rot (hparams.n_rot), @@ -1119,17 +1119,17 @@ struct llm_build_context { beta_slow (cparams.yarn_beta_slow), norm_eps (hparams.f_norm_eps), norm_rms_eps (hparams.f_norm_rms_eps), - n_tokens (ubatch.n_tokens), + n_tokens (ubatch_.n_tokens), n_kv (worst_case ? kv_self.size : kv_self.n), - n_outputs (worst_case ? n_tokens : lctx.n_outputs), - n_outputs_enc (worst_case ? n_tokens : lctx.embd_enc.size() / hparams.n_embd), + n_outputs (worst_case ? n_tokens : lctx_.n_outputs), + n_outputs_enc (worst_case ? n_tokens : lctx_.embd_enc.size() / hparams.n_embd), kv_head (worst_case ? (kv_self.recurrent ? 0 : kv_self.size - n_tokens) : kv_self.head), n_ctx_orig (cparams.n_ctx_orig_yarn), flash_attn (cparams.flash_attn), pooling_type (cparams.pooling_type), rope_type (hparams.rope_type), - cb (cb), - buf_compute_meta (lctx.buf_compute_meta) { + cb (cb_), + buf_compute_meta (lctx_.buf_compute_meta) { // all initializations should be done in init() }