Co-authored-by: David Pedersen <limero@me.com>
Co-authored-by: Gregory Anders <greg@gpanders.com>
Co-authored-by: Leo Schlosser <Leo.Schlosser@Student.HTW-Berlin.de>
Co-authored-by: zeertzjq <zeertzjq@outlook.com>
FUNC_ATTR_* should only be used in .c files with generated headers.
Defining FUNC_ATTR_* as empty in headers causes misuses of them to be
silently ignored. Instead don't define them by default, and only define
them as empty after a .c file has included its generated header.
Problem: Certain compilers (primarily GCC) do not recognize an exhaustive enum switch statement as being exhaustive. This manifests in the form of compiler errors in exhaustive switch statements where each case has a return statement but there isn't a catch-all return statements. These compiler errors are spurious in the context of the Neovim codebase. So #25533 added the `UNREACHABLE` macro to denote apart of the code that's unreachable, which was used after every such switch statement to tell the compiler to treat the switch statement as exhaustive. However, the macro is mentioned nowhere in the style guide,and new contributors would not have any natural way of learning about it as it stands now. This would lead to confusion when they inevitably encounter one of these compiler errors.
Solution: Add a style guideline which shows how to use the `UNREACHABLE` macro to fix these compiler errors.
Problem: The style guide states that all switch statements that are not conditional on an enum must have a `default` case, but does not give any explicit guideline for switch statements that are conditional on enums. As a result, a `default` case is added in many enum switch statements, even when the switch statement is exhaustive. This is not ideal because it removes the ability to have compiler errors to easily detect unchanged switch statements when a new possible value for an enum is added.
Solution: Add explicit guidelines for switch statements that are conditional on an enum, clarifying that a `default` case is not necessary if the switch statement is exhaustive. Also refactor pre-existing code with unnecessary `default` cases.
Problem: The style guide currently recommends having a `default:` case for switch statements that are not conditional on an enumerated value. Additionally, it recommends using `assert(false)` if `default:` is unreachable. This is problematic because `assert()` only runs on debug builds, which may lead to confusing breakages in release builds. Moreover, this suggestion is followed nowhere in the C code and `abort()` is used everywhere instead.
Solution: Suggest using `abort()` instead of `assert(false)`, that way the program always terminates if a logically unreachable case is reached.
Uncrustify and clang-format are already both excellent at ordering
includes; this isn't something we need to check for ourselves. Also
remove the section on include order in the dev-style documentation.
