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

[Nico Weber]

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.

(I don't understand the "This is a problem with 3.5's goal of not using
NDEBUG in headers" – I read the thread about NDEBUG [1], and it seems to be
mostly about abi compat – and a) clang itself isn't deadstripped itself
because plugins b) if stuff in a core binary itself calls a dump method it
won't be stripped anyhows c) the thread at [1] sounded more about abi stuff
like mvars and virtual functions, there's no clear support for the assert
stuff as far as I can tell. So just varying the presence or absence of
attribute((used)) based on NDEBUG sounds fine to me.)

[1]: https://groups.google.com/forum/#!msg/llvm-dev/OvcDArJjrSw/teSzTMugoGcJ


On Fri, Jan 3, 2014 at 9:43 AM, Argyrios Kyrtzidis <akyrtzi at gmail.com>wrote:

> On Jan 3, 2014, at 2:08 AM, Chandler Carruth <chandlerc at google.com> wrote:
>
> >
> > That said, I dislike this approach. I prefer actually *removing* the
> functions in non-asserts builds because then we will get early warnings if
> we miss some, and we don't have to rely on linker dead stripping to remove
> all of them.
>
> The benefit of relying on the linker is that a project can easily use any
> particular dump/print function from the llvm/clang libraries that it sees
> fit (e.g. for logging purposes) and since the linker has the ultimate
> knowledge of what gets used or not, it "just works" without unnecessary
> code-bloat.
> For example, a project may just want to use "Decl::dump(raw_ostream &OS)"
> for logging; with your scheme it'd need to configure & build specially for
> this and accept the code bloat from _all_ dump functions across llvm/clang,
> just to use this one function.
>
> But we don't need to hypothesize, we can see a similar example in clang
> with '-ast-dump'.
> Do you really want to disable this option on release builds ? I would much
> prefer not, I've used it frequently enough and I'd like to continue to be
> able to use it from our release builds.
>
> >
> > It also ensures that we don't grow accidental dependencies on functions
> that were only ever intended to be used inside of assert() and in dump()
> methods. Historically, this has happened quite a few times, and has taken
> someone some time to track down a compile time performance regression and
> separate out the functionality.
>
> Could you elaborate with a specific example ? Was someone ever printing
> something by accident ?
> I think this could apply to invariant-checking functions, but for
> dump/print functionality I honestly don't think that there is much "danger"
> in practice.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140103/94ee39ee/attachment.html>