From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from blyat.fensystems.co.uk (blyat.fensystems.co.uk [54.246.183.96])
 by mx.groups.io with SMTP id smtpd.web12.10164.1637667636443305482
 for <devel@edk2.groups.io>;
 Tue, 23 Nov 2021 03:40:37 -0800
Authentication-Results: mx.groups.io;
 dkim=missing; spf=pass (domain: ipxe.org, ip: 54.246.183.96, mailfrom: mcb30@ipxe.org)
Received: from pudding.home (unknown [188.94.42.109])
	by blyat.fensystems.co.uk (Postfix) with ESMTPSA id 313A044140;
	Tue, 23 Nov 2021 11:40:32 +0000 (UTC)
Subject: Re: [edk2-devel] [PATCH] MdePkg: DebugLib: Compilation fix for clang-13.
To: =?UTF-8?Q?Marvin_H=c3=a4user?= <mhaeuser@posteo.de>, devel@edk2.groups.io
Cc: michael.d.kinney@intel.com, krichanov@ispras.ru,
 gaoliming@byosoft.com.cn, "Liu, Zhiguang" <zhiguang.liu@intel.com>,
 vit9696@protonmail.com
References: <20211119090529.2865-1-krichanov@ispras.ru>
 <CO1PR11MB492995ACADD5A2B008627C44D29C9@CO1PR11MB4929.namprd11.prod.outlook.com>
 <db6fe4e5b31c28a6bcdb1f189cfcd5b1@ispras.ru>
 <CO1PR11MB49298A07BFBA2125E1D26B86D29F9@CO1PR11MB4929.namprd11.prod.outlook.com>
 <8d9101ac-fbda-eecf-950e-692fa375c3f9@ipxe.org>
 <b3510e57-b9df-4e32-b7f7-30c557ff9a28@posteo.de>
From: "Michael Brown" <mcb30@ipxe.org>
Message-ID: <5084a725-5ee1-d7cf-c662-1e4a7128e0f1@ipxe.org>
Date: Tue, 23 Nov 2021 11:40:31 +0000
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <b3510e57-b9df-4e32-b7f7-30c557ff9a28@posteo.de>
X-Spam-Status: No, score=-2.9 required=5.0 tests=ALL_TRUSTED,BAYES_00,
	URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2
X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on
	blyat.fensystems.co.uk
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable

On 23/11/2021 06:41, Marvin H=C3=A4user wrote:
> 23.11.2021 00:17:30 Michael Brown <mcb30@ipxe.org>:
>> I would very strongly recommend having the non-debug version of the ma=
cro use something like:
>>
>> #define ASSERT(Expression) do {=C2=A0=C2=A0 \
>>  =C2=A0=C2=A0=C2=A0=C2=A0 if (FALSE) {=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 \
>>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 (VOID) (Expression);=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 \
>>  =C2=A0=C2=A0=C2=A0=C2=A0 }=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 \
>>  =C2=A0=C2=A0 } while (FALSE)
>>
>> Without the "if (FALSE)", you will find that an expression that may ha=
ve side-effects (e.g. by calling an external function) will result in exe=
cutable code being generated.
>=20
> In theory +1, but don't some compilers generate "unreachable code" warn=
ings for this? I unfortunately cannot check them all right now. Maybe gua=
rds push/pop'ing the warning for that need to be defined, that are only a=
llowed to be used with explicit documentation why they are necessary?

A quick experiment shows that gcc won't generate a warning but clang=20
will.  Can be fixed by adding extra parentheses around FALSE:

#define ASSERT(Expression) do {   \
     if ((FALSE)) {                \
       (VOID) (Expression);        \
     }                             \
   } while (FALSE)

which is still clean and relatively elegant.

Thanks,

Michael