Upgrade to C23 and adding more static check from compiler

wdhongtw commented 4 months ago

For motivation, see #206.

This PR contains commits for

Upgrade from C99 to C23
Turn on -Wextra and -Werror compiler options
Fix corresponding errors

If desired, maybe I can extract the commit that upgrade to C23 as another PR. Since that it's a standalone change to this project.

wdhongtw commented 4 months ago

Used features from C23

[[maybe_unused]]: should be ok for both GCC and Clang
empty initializer: not sure if this works on all distrubutions.. can rollback this change if there is a concern.
- Clang support this since 17, but Debian seems only provide llvm 14
- Can not find compliance page for GCC

GCC/Clang version in some Linux distributions:

FreeBSD (Latest): gcc 9-15, llvm 11-18
Arch (Latest): gcc 13-14, llvm 17-18
Fedora 40: gcc 14, llvm 18
Ubuntu 24.04: gcc 13, llvm 18
Debian 12.0: gcc 12, llvm 14

Clang and GCC compliance page:

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 75.00000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 60.07%. Comparing base (2ffbf00) to head (9485582).

Files	Patch %	Lines
src/setup/ibus-setup-chewing-window.c	20.00%	4 Missing :warning:
src/ibus-chewing-engine.c	40.00%	3 Missing :warning:
src/IBusChewingPreEdit.c	0.00%	1 Missing :warning:
src/main.c	0.00%	1 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #207 +/- ## ========================================== - Coverage 60.13% 60.07% -0.07% ========================================== Files 24 24 Lines 2757 2760 +3 ========================================== Hits 1658 1658 - Misses 1099 1102 +3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

wdhongtw commented 4 months ago

Checks failed at ...

This may take longer than expected. I'm still trying to reproduce this failure locally.

kanru commented 4 months ago

Checks failed at ...

This may take longer than expected. I'm still trying to reproduce this failure locally.

The signature is defined like https://docs.gtk.org/gtk4/callback.WidgetActionActivateFunc.html

wdhongtw commented 4 months ago

I'm confused why I can not reproduce this error in my local environment (Fedora 40, Clang version 18.1.6) It tooks the same as the state in CI container...

llvm-libs x86_64 18.1.6-2.fc40 updates 28 M

So it may trigger other compile error after I update the PR later. O_Q

kanru commented 4 months ago

I'm confused why I can not reproduce this error in my local environment (Fedora 40, Clang version 18.1.6) It tooks the same as the state in CI container...

llvm-libs x86_64 18.1.6-2.fc40 updates 28 M

So it may trigger other compile error after I update the PR later. O_Q

That's why Werror can be annoying at times : 🤭

wdhongtw commented 4 months ago

That's why Werror can be annoying at times : 🤭

Now I known that...

It's may looks more natural to use the original (signed-integer) type to do comparision. And it should be safe to cast from unsigned-int to signed-int for the value ranges in these secenarios.

Maybe I should modify the PR again?

diff --git a/src/IBusChewingLookupTable.c b/src/IBusChewingLookupTable.c
index 99cec8b..0bdd552 100644
--- a/src/IBusChewingLookupTable.c
+++ b/src/IBusChewingLookupTable.c
@@ -58,8 +58,8 @@ guint ibus_chewing_lookup_table_update(
     [[maybe_unused]] IBusChewingProperties *iProperties,
     ChewingContext *context) {
     IBusText *iText = NULL;
-    guint i;
-    guint choicePerPage = (guint)chewing_cand_ChoicePerPage(context);
+    gint i;
+    gint choicePerPage = chewing_cand_ChoicePerPage(context);
     gint totalChoice = chewing_cand_TotalChoice(context);
     gint currentPage = chewing_cand_CurrentPage(context);

diff --git a/src/ibus-chewing-engine.c b/src/ibus-chewing-engine.c
index 7307baf..091495b 100644
--- a/src/ibus-chewing-engine.c
+++ b/src/ibus-chewing-engine.c
@@ -989,7 +989,7 @@ void ibus_chewing_engine_candidate_clicked(IBusEngine *engine, guint index,

     if (is_password(self))
         return;
-    if (index >= (guint)chewing_get_candPerPage(self->icPreEdit->context)) {
+    if ((gint)index >= chewing_get_candPerPage(self->icPreEdit->context)) {
         IBUS_CHEWING_LOG(DEBUG, "candidate_clicked() index out of ranged");
         return;
     }

kanru commented 4 months ago

That's why Werror can be annoying at times : 🤭

Now I known that...

It's may looks more natural to use the original (signed-integer) type to do comparision. And it should be safe to cast from unsigned-int to signed-int for the value ranges in these secenarios.

Maybe I should modify the PR again?

Sure, there are opinions that in C/C++ you rarely need to use unsigned integer and it might be dangerous. I'm not sure if the compiler has new warnings against them. It might still worth to follow the wisdom.

https://jacobegner.blogspot.com/2019/11/unsigned-integers-are-dangerous.html

wdhongtw commented 4 months ago

Rebase and force pushed again.

Introduce an option to control whether to use -Werror
Add and use new preset *-release-ci to build the project with the option above

The default and *-release presets are not changed, to avoid potential troubles addressed by @yan12125 :smiley_cat:

kanru commented 4 months ago

LGTM

chewing / ibus-chewing

Upgrade to C23 and adding more static check from compiler #207

Codecov Report