llvm dev - Mar 2018 - [RFC] Change DEBUG() macro to LLVM_DEBUG()

Hi all,

We recently stumbled upon some issues with the DEBUG() macro being 
redefined in some internal libraries and some external ones, such as 
LLVM. After trying out a few ideas to avoid the problem we thought that 
the simplest solution would be to prefix all generic macro names in 
order to avoid clashes.

A quick search showed that Mesa had a similar issue with DEBUG being 
used by LLVM and had a couple of proposed patches such as 
https://lists.freedesktop.org/archives/mesa-dev/2016-July/124111.html 
and https://lists.freedesktop.org/archives/mesa-dev/2016-June/120738.html

I noticed that others have left comments in the LLVM source about the 
same issue, so I decided to try to replace all DEBUG() uses with 
LLVM_DEBUG() and remove the old macro.
A review for the LLVM side of the patch is here: 
https://reviews.llvm.org/D43624, this was generated mostly by a 
find/replace regex.

This kind of change is quite invasive (~500 files) and might be 
disruptive to multiple parties. One possible way to avoid some of the 
disruption would be to keep arond the old macro during a transition 
period in which it would be "deprecated".

Thoughts?

Thanks,
Nicola

+1, claiming the "DEBUG" macro is pretty hostile. Debug.h is included
pretty widely from llvm/include/llvm/*, so we can't claim this is some
internal macro that users won't see.


On Fri, Mar 23, 2018 at 9:18 AM Nicola Zaghen via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Hi all,
>
> We recently stumbled upon some issues with the DEBUG() macro being
> redefined in some internal libraries and some external ones, such as
> LLVM. After trying out a few ideas to avoid the problem we thought that
> the simplest solution would be to prefix all generic macro names in
> order to avoid clashes.
>
> A quick search showed that Mesa had a similar issue with DEBUG being
> used by LLVM and had a couple of proposed patches such as
> https://lists.freedesktop.org/archives/mesa-dev/2016-July/124111.html
> and https://lists.freedesktop.org/archives/mesa-dev/2016-June/120738.html
>
> I noticed that others have left comments in the LLVM source about the
> same issue, so I decided to try to replace all DEBUG() uses with
> LLVM_DEBUG() and remove the old macro.
> A review for the LLVM side of the patch is here:
> https://reviews.llvm.org/D43624, this was generated mostly by a
> find/replace regex.
>
> This kind of change is quite invasive (~500 files) and might be
> disruptive to multiple parties. One possible way to avoid some of the
> disruption would be to keep arond the old macro during a transition
> period in which it would be "deprecated".
>
> Thoughts?
>
> Thanks,
> Nicola
>
> _______________________________________________
> 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/20180323/578a5667/attachment.html>

Sounds like a plan. I'd keep the old name around in the first commit, make
the changes to other llvm projects, and then delete the old name
immediately after that. I'd not keep it around any longer than a few days...

And while you're fixing this header, it has a few more similar problems...
I'd suggest that:
a. "DEBUG_WITH_TYPE" should be renamed
"LLVM_DEBUG_WITH_TYPE",
b. "DEBUG_TYPE" macro should be renamed "LLVM_DEBUG_TYPE",
and
c. the #defines for isCurrentDebugType (etc) should be actual functions (in
the llvm namespace!), rather than defines.

Probably those should be in a separate commit, since this is already a
giant change.


On Fri, Mar 23, 2018 at 1:58 PM Reid Kleckner via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> +1, claiming the "DEBUG" macro is pretty hostile. Debug.h is
included
> pretty widely from llvm/include/llvm/*, so we can't claim this is some
> internal macro that users won't see.
>
>
> On Fri, Mar 23, 2018 at 9:18 AM Nicola Zaghen via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> Hi all,
>>
>> We recently stumbled upon some issues with the DEBUG() macro being
>> redefined in some internal libraries and some external ones, such as
>> LLVM. After trying out a few ideas to avoid the problem we thought that
>> the simplest solution would be to prefix all generic macro names in
>> order to avoid clashes.
>>
>> A quick search showed that Mesa had a similar issue with DEBUG being
>> used by LLVM and had a couple of proposed patches such as
>> https://lists.freedesktop.org/archives/mesa-dev/2016-July/124111.html
>> and
https://lists.freedesktop.org/archives/mesa-dev/2016-June/120738.html
>>
>> I noticed that others have left comments in the LLVM source about the
>> same issue, so I decided to try to replace all DEBUG() uses with
>> LLVM_DEBUG() and remove the old macro.
>> A review for the LLVM side of the patch is here:
>> https://reviews.llvm.org/D43624, this was generated mostly by a
>> find/replace regex.
>>
>> This kind of change is quite invasive (~500 files) and might be
>> disruptive to multiple parties. One possible way to avoid some of the
>> disruption would be to keep arond the old macro during a transition
>> period in which it would be "deprecated".
>>
>> Thoughts?
>>
>> Thanks,
>> Nicola
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> 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/20180323/89e1c814/attachment.html>

> On Mar 23, 2018, at 9:18 AM, Nicola Zaghen via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> Hi all,
> 
> We recently stumbled upon some issues with the DEBUG() macro being
redefined in some internal libraries and some external ones, such as LLVM. After
trying out a few ideas to avoid the problem we thought that the simplest
solution would be to prefix all generic macro names in order to avoid clashes.
> 
> A quick search showed that Mesa had a similar issue with DEBUG being used
by LLVM and had a couple of proposed patches such as
https://lists.freedesktop.org/archives/mesa-dev/2016-July/124111.html and
https://lists.freedesktop.org/archives/mesa-dev/2016-June/120738.html
> 
> I noticed that others have left comments in the LLVM source about the same
issue, so I decided to try to replace all DEBUG() uses with LLVM_DEBUG() and
remove the old macro.
+1

-Chris