Attention is currently required from: Arthur Heymans, Cliff Huang, Elyes Haouas, Hung-Te Lin, Julius Werner, Jérémy Compostella, Lance Zhao, Tim Wawrzynczak, Yidi Lin.
Yu-Ping Wu has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84208?usp=email )
Change subject: soc/mt6366: Work around GCC LTO build ......................................................................
Patch Set 8:
(1 comment)
File src/soc/mediatek/mt8186/mt6366.c:
https://review.coreboot.org/c/coreboot/+/84208/comment/4f27016a_4644548c?usp... : PS8, Line 10: #define NO_BUILDTIME_ASSERT
just because one assert statement in a file had this issue the other statements in that file are somehow more likely to also have it
Well, that's exactly the argument I made in my previous comment. I still think this statement is true, at least for this file. By "robust" I meant a workaround solution which will continue to work despite future changes of other code (until GCC fixes the bug). Therefore I actually prefer PS5 the most (PS5 > per-file > per-statement).
Consider the situation where someone adds a new regulator type for mt6366, which breaks some of the existing `assert()` calls in this file. They probably wouldn't notice that until uploading the patch and then seeing the Jenkins build failure. Then, they will see the error message, probably spend some time trying to fix the problem, and then eventually figure out they'd need to just change those calls to `assertX()`. My question is, why not changing all of them to `assertX()` *right now*, so that all of these potential hassles could be avoided?
-
Yu-Ping Wu (Code Review)