[cfe-dev] Should dump methods be LLVM_ATTRIBUTE_USED only in debug builds?
thakis at chromium.org
Fri Jan 3 10:15:32 PST 2014
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 , 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  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.)
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
> 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...
More information about the cfe-dev