On 16.02.2022 10:25, Bertrand Marquis wrote: > Hi Jan, Julien, > >> On 15 Feb 2022, at 21:00, Julien Grall <[email protected]> wrote: >> >> (+ Bertrand) >> >> Hi Jan, >> >> On 27/01/2022 14:34, Jan Beulich wrote: >>> The increasing amount of constructs along the lines of >>> if ( !condition ) >>> { >>> ASSERT_UNREACHABLE(); >>> return; >>> } >>> is not only longer than necessary, but also doesn't produce incident >>> specific console output (except for file name and line number). >> >> So I agree that this construct will always result to a minimum 5 lines. >> Which is not nice. But the proposed change is... >> >>> Allow >>> the intended effect to be achieved with ASSERT(), by giving it a second >>> parameter allowing specification of the action to take in release builds >>> in case an assertion would have triggered in a debug one. The example >>> above then becomes >>> ASSERT(condition, return); >>> Make sure the condition will continue to not get evaluated when just a >>> single argument gets passed to ASSERT(). >>> Signed-off-by: Jan Beulich <[email protected]> >>> --- >>> v2: Rename new macro parameter. >>> --- >>> RFC: The use of a control flow construct as a macro argument may be >>> controversial. >> >> indeed controversial. I find this quite confusing and not something I would >> request a user to switch to if they use the longer version. >> >> That said, this is mainly a matter of taste. So I am interested to hear >> others view. >> >> I have also CCed Bertrand to have an opinions from the Fusa Group (I suspect >> this will go backward for them). > > Thanks and here is my feedback in regards to Fusa here. > > Most certification standards are forbidding completely macros including > conditions (and quite a number are forbidding static inline with conditions). > The main reason for that is MCDC coverage (condition/decisions and not only > code line) is not possible to do anymore down to the source code and has to be > done down to the pre-processed code. > > Out of Fusa considerations, one thing I do not like in this solution is the > fact that > you put some code as parameter of the macro (the return). > > To make this a bit better you could put the return code as parameter > instead of having “return CODE” as parameter.
Except that it's not always "return" what you want, and hence it can't be made implicit. > An other thing is that Xen ASSERT after this change will be quite different > from > any ASSERT found in other projects which could make it misleading for > developers. > Maybe we could introduce an ASSERT_RETURN macros instead of modifying the > behaviour of the standard ASSERT ? Along the lines of the above, this would then mean a multitude of new macros. Jan
