Skip to content

Conversation

@bettio
Copy link
Collaborator

@bettio bettio commented Feb 5, 2026

This will prevent bugs such as #2086.

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

`default` keyword silences "warning: enumeration value ‘FOO’ not handled in
switch", making some bugs hard to spot.

This caused issue atomvm#2086: `term_compare()` wasn't handling funs due to an
unnoticed missing case.

However `default` is still useful for telling compiler than any other case is
unreachable, but we don't want to silence warnings.

So the right strategy is using `-Wswitch-enum`, while doing fall through to the
default, for each enum value that we explicitly don't want to handle.

Signed-off-by: Davide Bettio <davide@uninstall.it>
Those missing cases were legit, still not obvious and they were causing
a warning now.

Signed-off-by: Davide Bettio <davide@uninstall.it>
@bettio bettio marked this pull request as draft February 5, 2026 12:02
// `type_t == type_other`,
// but we do `t == other` as the first thing, making this case unreachable.
case TERM_TYPE_INDEX_INVALID:
default:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
default:

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default: UNREACHABLE(); tells the compiler that any other value is undefined behavior, so it can perform additional optimizations. Otherwise it stills emit code for handling values outside the cases we defined.
Variables having an enum type are glorified integers, so the compiler by default doesn't infer that only allowed values are the enum values.

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