Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various warnings fixes #3840

Merged
merged 8 commits into from
Nov 3, 2023
Merged

Conversation

b4n
Copy link
Member

@b4n b4n commented Oct 26, 2023

This fixes various mostly harmless warnings from GCC. This is not everything yet, and most of it is trivial, but it still gets rid of some and helps tidy code a little.

Admittedly some of them were already fine because they were the
definition, and had a proper declaration already, but fix them as well
to be clear and consistent.
GCC's -Wold-style-declaration.
This is not super useful as it's not commonly called, but it allows the
compiler to check the calls to the error printer.
Those are trailing semicolons in macros expanding to function
definitions, but ISO C doesn't like that -- although compilers usually
don't care.
This prevents GCC from warning this is an infinite recursion loop, as
well as pointing out the intent of that function.
@codecov
Copy link

codecov bot commented Oct 26, 2023

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (35abbb1) 85.05% compared to head (1504fc1) 85.05%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3840   +/-   ##
=======================================
  Coverage   85.05%   85.05%           
=======================================
  Files         228      228           
  Lines       53982    53982           
=======================================
+ Hits        45914    45915    +1     
+ Misses       8068     8067    -1     
Files Coverage Δ
dsl/sorter.c 64.82% <ø> (ø)
extra-cmds/optscript-repl.c 52.41% <ø> (ø)
extra-cmds/readtags-cmd.c 60.85% <100.00%> (ø)
main/lxpath.c 88.67% <ø> (ø)
main/mbcs.c 78.26% <100.00%> (ø)
main/mini-geany.c 80.95% <100.00%> (ø)
main/options.c 83.98% <ø> (ø)
main/read.c 95.52% <100.00%> (ø)
main/trashbox.c 83.14% <ø> (ø)
main/unwindi.c 91.25% <100.00%> (ø)
... and 11 more

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@masatake
Copy link
Member

The extra semicolons help the indentation engine of editors and source code indexes like ctags.
How about changing the definition of the macro for defining something like:

+#define END_DEF extern int ctags_nosuch_name_this_is_just_dummy_ignore_me
 #define DECLARE_ALT_VALUE_FN(N)									\
 static EsObject* alt_value_##N (EsObject *args, DSLEnv *env)
 
@@ -31,7 +32,7 @@ static EsObject* alt_value_##N (EsObject *args, DSLEnv *env)			\
 	if (!env->alt_entry)												\
 		dsl_throw (NO_ALT_ENTRY, es_symbol_intern ("&" #N)); 			\
 	return dsl_entry_##N (env->alt_entry);								\
-}
+} END_DEF

@b4n
Copy link
Member Author

b4n commented Nov 2, 2023

@masatake Hum, OK… so maybe it's easiest not to care about this specific issue? It's not like it's likely to cause an issue, and not everybody is building with -Wpedantic. But OTOH it's an actual non-ISO usage, which could (very theoretically) be an issue somewhere.

If you like your solution enough, we could indeed do it, but use #define END_DEF typedef int ctags_dummy_int_type_ignore_me, because redundant declarations would we warned about by -Wredundant-decls while redundant typedefs wouldn't.

If you do like it, I can update the patch to include this.

@masatake
Copy link
Member

masatake commented Nov 2, 2023

@b4n, your typedef version is better than mine. Could you update the patch?

They might help parsers (including uctags) not get confused by those
macros.  To still not have extra semicolons ISO C doesn't like, add a
dummy typedef at the end, that has no purpose but allow (actually,
require) a semicolon after.

The macro uses a custom suffix to have an actually unique name for each
typedef not to risk triggering a redundant declaration warning.
I am however not sure why I don't see the warning without the specific
suffix in dsl/* while I do see it in parsers/asm.c.
@b4n
Copy link
Member Author

b4n commented Nov 2, 2023

@masatake done. For now I added a new commit, but it can be merged.

I'm however a bit puzzled, in parsers/asm.c I see the warning from -Wredundant-decls even with a typedef, although I don't see it in a dummy C source, or in dsl/*… anyway, I switched to use a specific suffix which actually make the typedefs unique.

@masatake
Copy link
Member

masatake commented Nov 3, 2023

@b4n Thank you. You developed an interesting technique. I would like to merge this pull request as is because the commit log for the commit introducing the technique is written well. People in another project can refer to the commit and this pull request.

@masatake masatake merged commit 5f3415b into universal-ctags:master Nov 3, 2023
41 of 42 checks passed
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