Fix build with older avr-gcc: BL must be a constant expression - #877
Open
brubrudsi wants to merge 1 commit into
Open
Fix build with older avr-gcc: BL must be a constant expression#877brubrudsi wants to merge 1 commit into
brubrudsi wants to merge 1 commit into
Conversation
`BL` is used at file scope in the `keyball` initializer:
.pressing_keys = { BL, BL, BL, BL, BL, BL, 0 },
In C, an object with static storage duration must be initialized by a
constant expression. A `static const char` is a read-only object, not a
constant expression (unlike in C++), so this is a constraint violation.
Recent GCC folds it as an extension; older avr-gcc does not, and fails
with "error: initializer element is not constant". This affects anyone
building with distro-packaged AVR toolchains rather than the QMK
container -- e.g. Ubuntu 24.04, whose only gcc-avr candidate is
1:7.3.0+Atmel3.7.0-1.
An enum constant is a constant expression, so the initializer becomes
conforming. The value is unchanged (0xB0) and the two other uses of BL
are unaffected.
Fixes Yowkees#803
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #803.
Summary
This is not a bug in the firmware's behaviour, and it is not a broken build on your side — it is a toolchain portability issue. The code has shipped fine in v1.3.0–v1.3.2 and builds without complaint in the QMK container used by CI. The one-line change below widens support to distro-packaged AVR toolchains at no cost.
BLis used at file scope, in thekeyballinitializer:In C, an object with static storage duration must be initialized by a constant expression. A
static const charis a read-only object, not a constant expression — unlike in C++, where it would be. So this is a constraint violation that a conforming compiler must diagnose.Recent GCC folds it as an extension and stays silent. Older avr-gcc does not:
-std=c11 -pedantic-errorserror: initializer element is not constantThis is reachable for anyone building with distro AVR packages instead of the QMK container. On Ubuntu 24.04 the only
gcc-avrcandidate is1:7.3.0+Atmel3.7.0-1— there is no newer version to install — so those users hit it immediately.Since
lib/keyball/keyball.cis shared, it affects every variant (keyball39/44/46/61, one47) and every keymap.The change
An enum constant is a constant expression, so the initializer becomes conforming. I chose
enumover#defineto keepBLinside the type system rather than introduce a macro into a block ofconstdeclarations — but#define BL '\xB0'is equally correct, and it is what the reporter of #803 suggested. Happy to switch if you prefer it.Why it is safe
BLhas three uses and none change behaviour:oled_write_char((layer_state_is(i) ? to_1x(i) : BL), false)— the conditional operator already promoted both arms toint, so this is unchangedchar where = BL;/value = BL;—int→charconversion, exactly as beforeThe value is identical:
'\xB0'is anint-typed character constant that narrows tocharon assignment the same way theconst charobject did. AVR's plaincharis unsigned, so0xB0is representable either way.Verification
avr-gcc -mmcu=atmega32u4 -std=gnu11 -Os -Wall -Wextra: no errors and no new warnings.BLstill evaluates to0xB0.There is precedent for accepting build-portability fixes in this area — #816, "Fix compilation error when OLED is disabled".
概要 (Japanese)
これはファームウェアの動作不具合ではなく、またメンテナ側のビルドが壊れているという報告でもありません。ツールチェーンの互換性の問題です。現状のコードは v1.3.0〜v1.3.2 で問題なく動作しており、CI で使われている QMK コンテナでも正常にビルドできます。以下の1行の変更により、ディストリビューション標準の AVR ツールチェーンでもビルドできるようになります。
BLはkeyballの初期化子(ファイルスコープ)で使われています。C 言語では、静的記憶域期間を持つオブジェクトは定数式で初期化しなければなりません。
static const charは読み取り専用のオブジェクトであり、定数式ではありません(C++ とは異なります)。したがってこれは制約違反であり、規格に準拠したコンパイラは診断を出す必要があります。新しい GCC は拡張機能としてこれを受け入れますが、古い avr-gcc は受け入れず
error: initializer element is not constantとなります。Ubuntu 24.04 ではgcc-avrの候補が1:7.3.0+Atmel3.7.0-1のみで、これより新しい版を apt で入れることができないため、QMK コンテナを使わずにビルドする利用者はすぐにこの問題に遭遇します。lib/keyball/keyball.cは共通ファイルのため、すべての機種(keyball39/44/46/61、one47)とすべてのキーマップが対象になります。変更内容
static const char BLをenum { BL = '\xB0' };に変更しました。列挙定数は定数式なので、初期化子が規格準拠になります。マクロを増やさず型システム内に留めるためenumを選びましたが、#define BL '\xB0'でも同様に正しく、#803 の報告者が提案した方法でもあります。そちらがお好みであれば変更します。安全性
BLの使用箇所は3つあり、いずれも動作は変わりません。56行目の初期化子(本修正の対象)、496行目の条件演算子(元々両辺がintに格上げされているため変化なし)、615/619行目のcharへの代入(従来どおりのint→char変換)です。値も0xB0のまま変わりません。AVR のcharは既定で符号なしのため、いずれの場合も表現可能です。検証
Ubuntu 24.04 + avr-gcc 7.3.0 でエラーを再現し、修正後は
-Wall -Wextra付きで3箇所すべてが警告なしでコンパイルできることを確認しました。値が0xB0のままであることも確認済みです。本 PR の CI で全機種・全キーマップがビルドされるため、実際に使用されているツールチェーンでの非互換がないことも確認できるはずです。お忙しいところ恐れ入りますが、ご検討いただけますと幸いです。