[cfe-dev] Should dump methods be LLVM_ATTRIBUTE_USED only in debug builds?

[Nico Weber]

On Fri, Jan 3, 2014 at 11:41 AM, Chandler Carruth <chandlerc at google.com>wrote:

> On Fri, Jan 3, 2014 at 2:03 PM, Nico Weber <thakis at chromium.org> wrote:
>
>> On Fri, Jan 3, 2014 at 10:15 AM, Nico Weber <thakis at chromium.org> wrote:
>>
>>> Ok, sounds like it's not clear if folks want to have dump methods behind
>>> #ifdef !NDEBUG. Maybe we don't have to resolve that part in this thread
>>>
>>> Is there any opposition to replacing the attributes on just the dump
>>> methods with LLVM_DUMP_METHOD? That's a subset of Chandler's change and I
>>> think also what Alp's diff is.
>>>
>>> …which is effectively Alp's patch I suppose. So maybe we could land that
>> for now and then discuss whether to put dump() methods in !NDEBUG at some
>> other point?
>>
>
> I'd like to actually see the patch rather than talk in the hypothetical.
> Maybe I misunderstood, but I thought it also moved the attribute to the
> definition? I don't understand why that's relevant yet (which is why maybe
> I've misunderstood what was happening). I've asked what the bug was that
> this addressed. My reading of the spec for the attribute seems to indicate
> that there is no difference....
>

I'm guessing that's to have an identical .h file in both debug and release
builds. If you have an issue with this (it seemed harmless to me), that
part can be discussed later.

If you don't like landing both of Alp's patches as-is, what about:
1. Land Alp's llvm patch as-is (maybe with
s/LLVM_DEBUG_HELPER/LLVM_DUMP_METHOD/, both seem fine)
2. Replace LLVM_ATTRIBUTE_USED in front of the dump() methods in clang with
LLVM_DEBUG_HELPER. The diff for this would look like

-  LLVM_ATTRIBUTE_USED void dumpStack() const;
+  LLVM_DEBUG_HELPER void dumpStack() const;

in a bunch of .h files.

Is this ok?

I don't care about the color, I just would like to put on a coat of paint
:-)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140103/0f6b79e4/attachment.html>