minimal profile produces a lot of warnings (-Wunused-function)

[teetak01]

Description

• Type: Bug

---

Bug

mbed-client-cli version b97cbfb3db70058ed66579b7496200f851d386a2

/home/teetak01/devel/mbed-client-testapp/mbed-client-cli/source/ns_cmdline.c:368:25: warning: ‘cmd_history_save’ declared ‘static’ but never defined [-Wunused-function]
 static void             cmd_history_save(int16_t index);
                         ^~~~~~~~~~~~~~~~
/home/teetak01/devel/mbed-client-testapp/mbed-client-cli/source/ns_cmdline.c:369:25: warning: ‘cmd_history_get’ declared ‘static’ but never defined [-Wunused-function]
 static void             cmd_history_get(uint16_t index);
                         ^~~~~~~~~~~~~~~
/home/teetak01/devel/mbed-client-testapp/mbed-client-cli/source/ns_cmdline.c:370:25: warning: ‘cmd_history_clean_overflow’ declared ‘static’ but never defined [-Wunused-function]
 static void             cmd_history_clean_overflow(void);
                         ^~~~~~~~~~~~~~~~~~~~~~~~~~
/home/teetak01/devel/mbed-client-testapp/mbed-client-cli/source/ns_cmdline.c:371:25: warning: ‘cmd_history_clean’ declared ‘static’ but never defined [-Wunused-function]
 static void             cmd_history_clean(void);
                         ^~~~~~~~~~~~~~~~~
/home/teetak01/devel/mbed-client-testapp/mbed-client-cli/source/ns_cmdline.c:372:25: warning: ‘cmd_history_find’ declared ‘static’ but never defined [-Wunused-function]
 static cmd_history_t   *cmd_history_find(int16_t index);
                         ^~~~~~~~~~~~~~~~
/home/teetak01/devel/mbed-client-testapp/mbed-client-cli/source/ns_cmdline.c:387:25: warning: ‘goto_end_of_history’ declared ‘static’ but never defined [-Wunused-function]
 static void             goto_end_of_history(void);
                         ^~~~~~~~~~~~~~~~~~~
/home/teetak01/devel/mbed-client-testapp/mbed-client-cli/source/ns_cmdline.c:388:25: warning: ‘goto_beginning_of_history’ declared ‘static’ but never defined [-Wunused-function]
 static void             goto_beginning_of_history(void);
                         ^~~~~~~~~~~~~~~~~~~~~~~~~
/home/teetak01/devel/mbed-client-testapp/mbed-client-cli/source/ns_cmdline.c:1872:13: warning: ‘cmd_variable_print_all’ defined but not used [-Wunused-function]
 static void cmd_variable_print_all(void)
             ^~~~~~~~~~~~~~~~~~~~~~
/home/teetak01/devel/mbed-client-testapp/mbed-client-cli/source/ns_cmdline.c:1820:24: warning: ‘variable_find’ defined but not used [-Wunused-function]
 static cmd_variable_t *variable_find(char *variable)
                        ^~~~~~~~~~~~~
/home/teetak01/devel/mbed-client-testapp/mbed-client-cli/source/ns_cmdline.c:1796:21: warning: ‘alias_find_n’ defined but not used [-Wunused-function]
 static cmd_alias_t *alias_find_n(char *alias, int aliaslength, int n)
                     ^~~~~~~~~~~~
/home/teetak01/devel/mbed-client-testapp/mbed-client-cli/source/ns_cmdline.c:1774:21: warning: ‘alias_find’ defined but not used [-Wunused-function]
 static cmd_alias_t *alias_find(const char *alias)
                     ^~~~~~~~~~
/home/teetak01/devel/mbed-client-testapp/mbed-client-cli/source/ns_cmdline.c:1648:13: warning: ‘cmd_replace_variables’ defined but not used [-Wunused-function]
 static void cmd_replace_variables(char *input)
             ^~~~~~~~~~~~~~~~~~~~~
/home/teetak01/devel/mbed-client-testapp/mbed-client-cli/source/ns_cmdline.c:1626:13: warning: ‘replace_variable’ defined but not used [-Wunused-function]
 static void replace_variable(char *str, cmd_variable_t *variable_ptr)
             ^~~~~~~~~~~~~~~~
/home/teetak01/devel/mbed-client-testapp/mbed-client-cli/source/ns_cmdline.c:1615:13: warning: ‘cmd_replace_alias’ defined but not used [-Wunused-function]
 static void cmd_replace_alias(char *input)
             ^~~~~~~~~~~~~~~~~
/home/teetak01/devel/mbed-client-testapp/mbed-client-cli/source/ns_cmdline.c:1559:13: warning: ‘cmd_clear_last_word’ defined but not used [-Wunused-function]
 static void cmd_clear_last_word()
             ^~~~~~~~~~~~~~~~~~~
/home/teetak01/devel/mbed-client-testapp/mbed-client-cli/source/ns_cmdline.c:1547:13: warning: ‘cmd_move_cursor_to_next_space’ defined but not used [-Wunused-function]
 static void cmd_move_cursor_to_next_space(void)
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/teetak01/devel/mbed-client-testapp/mbed-client-cli/source/ns_cmdline.c:1533:13: warning: ‘cmd_move_cursor_to_last_space’ defined but not used [-Wunused-function]
 static void cmd_move_cursor_to_last_space(void)
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/teetak01/devel/mbed-client-testapp/mbed-client-cli/source/ns_cmdline.c:1507:6: warning: ‘cmd_tab_lookup’ defined but not used [-Wunused-function]
 bool cmd_tab_lookup(void)
      ^~~~~~~~~~~~~~
/home/teetak01/devel/mbed-client-testapp/mbed-client-cli/source/ns_cmdline.c:1184:13: warning: ‘cmd_arrow_left’ defined but not used [-Wunused-function]
 static void cmd_arrow_left()
             ^~~~~~~~~~~~~~
/home/teetak01/devel/mbed-client-testapp/mbed-client-cli/source/ns_cmdline.c:1163:13: warning: ‘cmd_arrow_right’ defined but not used [-Wunused-function]
 static void cmd_arrow_right()

[jupe]

Originally I decided to use #ifdef and linker combination to reduce foot print of the library. #ifdef reduce internal static function calls that is not needed for selected configuration and linker do rest of reduction jobs (require some optimization flag, like -O1). Reason for this was that number of #ifdef start growing and makes harder to read the code. It is true that -Wunused-function causes ~20 errors because of unused function, but using -Wno-unused-function silents those errors.

Questions:

• Does current approach works for all compilers we use ?
• Which compiler option is default in mbed-os (-Wunused-function or -Wnounused-function) ?
• Is there some optimization enabled by default ? (-O1)
• How mbed-os implementation generally does this thing ? (only #ifdef's or rely on linker functionality to drop unused functions)
• Should we encapsulate also "unused functions" with #ifdef even it makes code less readable ?

@seppo @kjbracey-arm please check this out..

[kjbracey]

This warning is on by default in most compilers - seems you're doing something which really does tend to provoke it.

We tend to use ifdefs where this arises in Mbed OS, but it doesn't occur that often. Normally stuff that isn't used is not marked static, which is what prompts the complaint.

I think marking the functions static inline should shut that warning up in all compilers, with no downside that I can see if it's called exactly once. Or you could stop them being static.

It's possible the MBED_UNUSED attribute in mbed_toolchain.h might work, but it's only documented for use on parameters.

[jupe]

most of those functions that causes compiler warning are called only once so inline is just fine for them. It also seems to work as expected so I'll create PR with inline.

[jupe]

seems that ARM compiler was not happy with inline:

[Error] ns_cmdline.c@1740,0:  #1059-D: an entity with internal linkage cannot be referenced within an inline function with external linkage

Hmm have to re-think how to fix this.

It's possible the MBED_UNUSED attribute in mbed_toolchain.h might work, but it's only documented for use on parameters.

Would not like to use mbed-os specific attributes since library is generic and used with other OS's as well...

[kjbracey]

They should have been set static inline, not just inline. They're still private to that C file, right?

[jupe]

seems that static inline does not silent these warnings:

../../source/ns_cmdline.c:388:39: error: unused function 'goto_beginning_of_history' [-Werror,-Wunused-function]
static inline void                    goto_beginning_of_history(void);

Are you happy if I just bring them "public" - without static/inline keywords, and ensure they have enough unique names to avoid conflicts with applications.. ?

[kjbracey]

Okay, that's a bit weird. It must be distinguishing between headers and C files, because it wouldn't complain if there was an unused "static inline" in a header.

Making them publicly visible isn't ideal - it makes the code worse to shut up a compiler warning, which I'm not a fan of.

How about sticking pragma(s) at the top of the file to silence the warnings? When the code isn't wrong, and any change would make it worse, I'd rather shut the compiler up:

#pragma GCC diagnostic ignored "-Wunused-function"

will work for Clang (ARMC6) and GCC. IAR and ARMC5 would need something else.

[jupe]

that's fine for me also. Does anyone have an objection? @SeppoTakalo @teetak01