llvm dev - Oct 2017 - Question: Should we consider valid a variable defined #ifndef NDEBUG scope and used in assert?

Good point, I forgot to check the standard.
Given the clang was failing I assumed the code was wrong x).

I am guessing there is something weird with the project, because indeed,
paragraph 7.2 of the standard says:
The assert macro is redefined according to the current state of NDEBUG each time
that

<assert.h> is included. 

> On Oct 4, 2017, at 2:53 PM, Craig Topper <craig.topper at gmail.com>
wrote:
> 
> I thought NDEBUG was what the assert macro also used to decide whether to
be an assert or not.
> 
> ~Craig
> 
> On Wed, Oct 4, 2017 at 2:41 PM, Quentin Colombet via llvm-dev <llvm-dev
at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
> Hi,
> 
> While audit our code, we found out a strange pattern in one of the LLVM
headers and we were wondering if that was expected or if it should be fixed.
> 
> Namely the problem looks like this:
> #ifndef NDEBUG
>   // Define some variable.
> #endif
> 
> assert(/*use this variable, i.e., outside of the ifndef NDEBUG scope*/);
> 
> This happens in include/llvm/IR/ValueHandle.h for the variable Poisoned
line 494
> 
> This works because when we build LLVM with assert we explicitly disable
NDEBUG:
> if( LLVM_ENABLE_ASSERTIONS )
>   […]
>   # On non-Debug builds cmake automatically defines NDEBUG, so we
>   # explicitly undefine it:
>   if( NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" )
>     add_definitions( -UNDEBUG )
> 
> If we use this header in a different project and thus, with potentially
different rules for macro definitions, the compiler complains with
> error:   use of undeclared identifier ‘Poisoned'
> 
> I think the right thing to do is to fix the code to work without the
special way of setting the macros, but I’d like people opinions first.
> 
> Thanks,
> -Quentin
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
> 
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20171004/ae479957/attachment.html>

Yep they are messing up with the DEBUG macros…

Sorry for the noise!
> On Oct 4, 2017, at 2:58 PM, Quentin Colombet via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> 
> Good point, I forgot to check the standard.
> Given the clang was failing I assumed the code was wrong x).
> 
> I am guessing there is something weird with the project, because indeed,
paragraph 7.2 of the standard says:
> The assert macro is redefined according to the current state of NDEBUG each
time that
> 
> <assert.h> is included. 
> 
> 
>> On Oct 4, 2017, at 2:53 PM, Craig Topper <craig.topper at gmail.com
<mailto:craig.topper at gmail.com>> wrote:
>> 
>> I thought NDEBUG was what the assert macro also used to decide whether
to be an assert or not.
>> 
>> ~Craig
>> 
>> On Wed, Oct 4, 2017 at 2:41 PM, Quentin Colombet via llvm-dev
<llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>>
wrote:
>> Hi,
>> 
>> While audit our code, we found out a strange pattern in one of the LLVM
headers and we were wondering if that was expected or if it should be fixed.
>> 
>> Namely the problem looks like this:
>> #ifndef NDEBUG
>>   // Define some variable.
>> #endif
>> 
>> assert(/*use this variable, i.e., outside of the ifndef NDEBUG
scope*/);
>> 
>> This happens in include/llvm/IR/ValueHandle.h for the variable Poisoned
line 494
>> 
>> This works because when we build LLVM with assert we explicitly disable
NDEBUG:
>> if( LLVM_ENABLE_ASSERTIONS )
>>   […]
>>   # On non-Debug builds cmake automatically defines NDEBUG, so we
>>   # explicitly undefine it:
>>   if( NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" )
>>     add_definitions( -UNDEBUG )
>> 
>> If we use this header in a different project and thus, with potentially
different rules for macro definitions, the compiler complains with
>> error:   use of undeclared identifier ‘Poisoned'
>> 
>> I think the right thing to do is to fix the code to work without the
special way of setting the macros, but I’d like people opinions first.
>> 
>> Thanks,
>> -Quentin
>> 
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>> 
>> 
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20171004/412ceac4/attachment.html>

On the topic of NDEBUG usage in public headers; this may be interesting:
https://reviews.llvm.org/D38511
> On Oct 4, 2017, at 3:02 PM, Quentin Colombet via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> Yep they are messing up with the DEBUG macros…
> 
> Sorry for the noise!
> 
>> On Oct 4, 2017, at 2:58 PM, Quentin Colombet via llvm-dev <llvm-dev
at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>> 
>> 
>> Good point, I forgot to check the standard.
>> Given the clang was failing I assumed the code was wrong x).
>> 
>> I am guessing there is something weird with the project, because
indeed, paragraph 7.2 of the standard says:
>> The assert macro is redefined according to the current state of NDEBUG
each time that
>> 
>> <assert.h> is included. 
>> 
>> 
>>> On Oct 4, 2017, at 2:53 PM, Craig Topper <craig.topper at
gmail.com <mailto:craig.topper at gmail.com>> wrote:
>>> 
>>> I thought NDEBUG was what the assert macro also used to decide
whether to be an assert or not.
>>> 
>>> ~Craig
>>> 
>>> On Wed, Oct 4, 2017 at 2:41 PM, Quentin Colombet via llvm-dev
<llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>>
wrote:
>>> Hi,
>>> 
>>> While audit our code, we found out a strange pattern in one of the
LLVM headers and we were wondering if that was expected or if it should be
fixed.
>>> 
>>> Namely the problem looks like this:
>>> #ifndef NDEBUG
>>>   // Define some variable.
>>> #endif
>>> 
>>> assert(/*use this variable, i.e., outside of the ifndef NDEBUG
scope*/);
>>> 
>>> This happens in include/llvm/IR/ValueHandle.h for the variable
Poisoned line 494
>>> 
>>> This works because when we build LLVM with assert we explicitly
disable NDEBUG:
>>> if( LLVM_ENABLE_ASSERTIONS )
>>>   […]
>>>   # On non-Debug builds cmake automatically defines NDEBUG, so we
>>>   # explicitly undefine it:
>>>   if( NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" )
>>>     add_definitions( -UNDEBUG )
>>> 
>>> If we use this header in a different project and thus, with
potentially different rules for macro definitions, the compiler complains with
>>> error:   use of undeclared identifier ‘Poisoned'
>>> 
>>> I think the right thing to do is to fix the code to work without
the special way of setting the macros, but I’d like people opinions first.
>>> 
>>> Thanks,
>>> -Quentin
>>> 
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>>> 
>>> 
>> 
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20171004/e46a0584/attachment.html>