<div dir="ltr">On Fri, Jan 3, 2014 at 11:41 AM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div class="im">On Fri, Jan 3, 2014 at 2:03 PM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:<br>
</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="im"><div>On Fri, Jan 3, 2014 at 10:15 AM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:<br>

</div></div><div class="gmail_extra"><div class="gmail_quote"><div class="im"><div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">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<div>


<br></div><div>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.</div>
<div><br></div></div></blockquote></div></div><div class="im"><div>…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?</div>
</div></div>
</div></div></blockquote><div><br></div><div>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....</div>
</div></div></div>
</blockquote></div><br></div><div class="gmail_extra">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.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">If you don't like landing both of Alp's patches as-is, what about:</div><div class="gmail_extra">1. Land Alp's llvm patch as-is (maybe with s/LLVM_DEBUG_HELPER/LLVM_DUMP_METHOD/, both seem fine)</div>
<div class="gmail_extra">2. Replace LLVM_ATTRIBUTE_USED in front of the dump() methods in clang with LLVM_DEBUG_HELPER. The diff for this would look like</div><div class="gmail_extra"><div class="gmail_extra"><br></div><div class="gmail_extra">
-  LLVM_ATTRIBUTE_USED void dumpStack() const;</div><div class="gmail_extra">+  LLVM_DEBUG_HELPER void dumpStack() const;</div><div><br></div><div>in a bunch of .h files.</div><div><br></div><div>Is this ok?</div><div><br>
</div><div>I don't care about the color, I just would like to put on a coat of paint :-)</div></div></div>