Skip to content

Conversation

@KaganCanSit
Copy link
Contributor

Hello,

This pull request addresses CppCheck's Function Arguments Names Different warnings. Additional information on this topic was provided in the discussion section here.

Here is the list of warnings:

include/spdlog/details/log_msg-inl.h:38:style:funcArgNamesDifferent -> Function 'log_msg' argument 1 names different: declaration 'logger_name' definition 'a_logger_name'.
include/spdlog/spdlog-inl.h:42:style:funcArgNamesDifferent -> Function 'should_log' argument 1 names different: declaration 'lvl' definition 'log_level'.
include/spdlog/async_logger-inl.h:60:style:funcArgNamesDifferent -> Function 'backend_sink_it_' argument 1 names different: declaration 'incoming_log_msg' definition 'msg'.
include/spdlog/cfg/helpers-inl.h:73:style:funcArgNamesDifferent -> Function 'load_levels' argument 1 names different: declaration 'txt' definition 'input'.

While fixing the warnings, I ended up making more changes than I initially anticipated. Therefore, to make it easier for you to review and understand, I created a separate commit for each warning and will summarize my actions. After the review, I can squash them into a single commit.

Looking at the commits in the branch from the end to the beginning;

  • helpers-inl.h - load_levels -> ‘txt’ -> ‘input’

    Although there are many different names used for the load_levels function and its parameter, such as txt, input, and levels_string, it serves multiple purposes. Its primary purpose is to parse a string in the format off,logger1=debug,logger2=info and set the log levels.

    To resolve the naming confusion here, I updated the name to levels_spec. It is a specification that defines how levels are configured. This term is commonly used in technical literature for format strings and DSL-like structures.

    This does not affect API compatibility.

  • async_logger-inl.h - backend_sink_it_ -> ‘incoming_log_msg’ -> 'msg'

    There is a deliberate semantic distinction between the frontend (sink_it_) and backend (backend_sink_it_) functions in the async_logger class. In the frontend, the message is processed in the context of the thread that called it immediately, while in the backend, the message has been queued, is time-delayed, and has changed ownership. Please correct me if I'm misunderstanding the design intent.

    While the name incoming_log_msg was used in the declaration to reflect this distinction, the definition simply used msg. The definition was corrected to incoming_log_msg to match the declaration. This both resolved the warning and preserved the semantic distinction in the design.

    Does not affect API compatibility.

  • spdlog-inl.h - should_log -> ‘lvl’ -> 'log_level'

    lvl was used in the declaration, while log_level was used in the definition. Since the set_level and flush_on functions in the same file pair (spdlog.h / spdlog-inl.h) also use the log_level parameter, the declaration was updated to log_level for consistency.

    This does not affect API compatibility.


Unimplemented Change:

  • log_msg-inl.h - log_msg -> ‘logger_name’ - 'a_logger_name'

    In this case, the easy solution would be to change the content of the logger_name parameter in the second constructor function to a_logger_name. However, this would create a significant inconsistency by affecting the readability and understanding of the header file. The question is likely to arise: why is there no a_ prefix in the other parameters?

    I believe the most appropriate approach is to add the m_ (member) prefix to the log_msg struct member variables and adjust the signatures accordingly. However, this would require modifying all member variables in their accessible areas (which would be quite extensive!). Therefore, I decided not to intervene on this point as it would also affect API compatibility.

    Perhaps we can address this in the future or in a separate PR if you wish, and I can assist you. FYI @tt4g


I may always have mistakes and omissions, so just leave a comment. I will look into it when I have time.

Best regards.

@gabime gabime requested a review from Copilot January 11, 2026 19:55
@gabime
Copy link
Owner

gabime commented Jan 11, 2026

member names end with “_”

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request addresses CppCheck warnings about inconsistent parameter names between function declarations and definitions. The changes standardize naming conventions to improve code clarity and eliminate static analysis warnings.

Changes:

  • Renamed should_log parameter from lvl to log_level for consistency with other functions
  • Renamed load_levels parameter from txt/input to levels_spec to better describe its purpose
  • Renamed backend_sink_it_ parameter from msg to incoming_log_msg to preserve semantic distinction
  • Added const qualifiers to improve code quality in helper functions

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
include/spdlog/spdlog.h Updated parameter name in should_log declaration for consistency
include/spdlog/cfg/helpers.h Renamed load_levels parameter to better reflect its purpose as a levels specification
include/spdlog/cfg/helpers-inl.h Updated load_levels implementation with new parameter name and added const qualifier
include/spdlog/cfg/env.h Updated variable name to match new load_levels parameter convention
include/spdlog/cfg/argv.h Updated variable name to match new load_levels parameter convention
include/spdlog/async_logger-inl.h Renamed parameter to preserve semantic distinction between frontend and backend message handling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@gabime gabime merged commit f2a9dec into gabime:v1.x Jan 11, 2026
13 of 14 checks passed
@gabime
Copy link
Owner

gabime commented Jan 11, 2026

Thanks @KaganCanSit

@KaganCanSit KaganCanSit deleted the fix-function-arguments-names-different-warnings branch January 12, 2026 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants