llvm dev - Nov 2014 - [LLVMdev] [RFC] New ToolsSupport library for stuff that only tools need

> On Nov 6, 2014, at 3:00 PM, Reid Kleckner <rnk at google.com> wrote:
> 
> On Thu, Nov 6, 2014 at 2:42 PM, Juergen Ributzka <juergen at apple.com
<mailto:juergen at apple.com>> wrote:
> I would have preferred the library to be moved out of the “Support” folder
and have its own top-level library and include folder.
> 
> +1, you will need to do this if you want to support the autoconf build
system.
This should be a fairly small change to make.
> 
> ----
> 
> Looking at the diff, I now remember why you want to do this. I seem to
recall you want to do this because the Mac-specific code in Support defines
raise(), __assert_rtn(), and abort(), and that's hostile to embedders like
WebKit. Makes sense, it's a gross hack, and I'd be in favor of fixing it
if possible. If it weren't for this hack, I would recommend that you just
avoid calling PrintStackTraceOnErrorSignal from WebKit.
That hack is the primary motivator, but that can be solved other ways. One
option would be for us to move the nasty Mac-specific hack out of the cpp file
and into a header that tools could optionally include.
> 
> Is there some way we can prevent this Mac-specific hack from affecting our
library organization? It's not clear to me that the distinction between
Support and ToolsSupport is really worth it.
> 
> Most LLVM tools are single threaded and don't need to redirect signals
to the current thread. I think it's only the crash recovery logic that needs
the signal to be delivered to the current thread. Maybe we should factor out a
CrashRecovery library, given that XCode via libclang is basically the only
client of this stuff? Does WebKit want to try to recover from JIT assertion
failures?
The other thought I had which motivated this solution was that if we could strip
all the functionality that is only really used by tools out into a separate
library it would offer cleaner organization of code. Support seems to often get
used as a dumping ground for stuff that just doesn’t fit anywhere else.

Based on your feedback and Chandler’s maybe this just isn’t the right
separation. I can look into a solution to address our hackiness without creating
a separate library.

-Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20141106/92cff3aa/attachment.html>

On Thu, Nov 6, 2014 at 3:22 PM, Chris Bieneman <beanz at apple.com>
wrote:
>
> The other thought I had which motivated this solution was that if we could
> strip all the functionality that is only really used by tools out into a
> separate library it would offer cleaner organization of code. Support seems
> to often get used as a dumping ground for stuff that just doesn’t fit
> anywhere else.
>
> Based on your feedback and Chandler’s maybe this just isn’t the right
> separation. I can look into a solution to address our hackiness without
> creating a separate library.
>
What other stuff do you think belongs in ToolsSupport that doesn't belong
in Support? Looking back at the initial email, you have command line
parsing and ToolOutputFile.

We could split out command line parsing, but it doesn't seem worth it,
given that we're still carrying regex support, Unicode conversion, dynamic
library support, and other things that probably aren't absolutely necessary.

What about splitting out a CrashRecovery library instead? That seems a lot
more targeted and meaningful. We'd probably put ToolOutputFile.cpp in there.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20141106/9ed05a11/attachment.html>

I think for the main goal of cleaning up the Mac-specific hack, a CrashRecovery
library would work equally well. Juergen is more familiar with the WebKit side
of things, so he may be aware of something I’m not thinking of.

Chandler, does splitting out a CrashRecovery library instead seem sane?

Other than code organization and naming, the general idea of splitting out a
CrashRecovery library would be the same as the other patches I sent out. I was
thinking of taking the approach of moving one symbol, fixing everything, then
repeat.

Does that seem like the right approach?

-Chris
> On Nov 6, 2014, at 3:43 PM, Reid Kleckner <rnk at google.com> wrote:
> 
> On Thu, Nov 6, 2014 at 3:22 PM, Chris Bieneman <beanz at apple.com
<mailto:beanz at apple.com>> wrote:
> The other thought I had which motivated this solution was that if we could
strip all the functionality that is only really used by tools out into a
separate library it would offer cleaner organization of code. Support seems to
often get used as a dumping ground for stuff that just doesn’t fit anywhere
else.
>  
> Based on your feedback and Chandler’s maybe this just isn’t the right
separation. I can look into a solution to address our hackiness without creating
a separate library.
> 
> What other stuff do you think belongs in ToolsSupport that doesn't
belong in Support? Looking back at the initial email, you have command line
parsing and ToolOutputFile.
> 
> We could split out command line parsing, but it doesn't seem worth it,
given that we're still carrying regex support, Unicode conversion, dynamic
library support, and other things that probably aren't absolutely necessary.
> 
> What about splitting out a CrashRecovery library instead? That seems a lot
more targeted and meaningful. We'd probably put ToolOutputFile.cpp in there.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20141106/934366e4/attachment.html>