lichray / nvi2

A multibyte fork of the nvi editor for BSD
Other
144 stars 34 forks source link

Fix "add explicit braces to avoid dangling else" warnings. #87

Closed leres closed 3 years ago

leres commented 3 years ago

While building 04c163d under FreeBSD 12.1, clang 8.0.1 produces a lot of warnings, e.g:

CMake Warning at CMakeLists.txt:9 (message):
../common/mark.c:223:7: warning: add explicit braces to avoid dangling else [-Wdangling-else]
1 warning generated.

One cause are multi-statement macros wrapped with just braces. The usual way is to wrap multi-statements with do {} while (0) as described here: Swallowing the Semicolon

The other cause are nested if statements, e.g:

        if (lmp->lno >= lno)
                if (lmp->lno == lno) {
                        F_SET(lmp, MARK_DELETED);
                        (void)log_mark(sp, lmp);
                } else
                        --lmp->lno;

Add do {} while (0) and explicit braces where needed.

leres commented 3 years ago

I reverted the unrelated change. There was a bug reported against the FreeBSD base version of vi that is also in the 2.2.0 release of nvi2 that I started to debug but while compiling I ran into the dangling else warnings. By the time I fixed those I retested for the bug ("nvi -w 10" was broken) I found the current version had already fixed it.

I apologize for the lapse.

lichray commented 3 years ago

No worries. Thanks again for the fix, especially the added semicolons.

leres commented 3 years ago

I'm a big fan of letting the compiler do as much work as possible and I believe my end of the bargain is eliminate any and all compiler warnings.

lichray commented 3 years ago

I'm a big fan of letting the compiler do as much work as possible and I believe my end of the bargain is eliminate any and all compiler warnings.

Me too. On that topic, I had a thought: what if we compile the project with a C++ compiler? A few templates may replace the macros, and the type conversions/checkings are more restricted. This practice has precedent. Microsoft's Universal C runtime (ucrt, C standard library) is written in C++).

leres commented 3 years ago

Conversion to C++ is not something I feel strongly about.