<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Tue, Jan 16, 2018 at 11:35 AM Philip Reames <<a href="mailto:listmail@philipreames.com">listmail@philipreames.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div text="#000000" bgcolor="#FFFFFF">
<p><br>
</p>
<br>
<div class="m_2745718309214774179moz-cite-prefix">On 01/16/2018 09:21 AM, David Blaikie
via llvm-dev wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Context: I've been looking at experimenting with
using Modular Code Generation (My talk at last year's LLVM dev
meeting <a href="https://www.youtube.com/watch?v=lYYxDXgbUZ0" target="_blank">https://www.youtube.com/watch?v=lYYxDXgbUZ0</a> is
about the best reference at the moment) when building the LLVM
project, as a good experiment for the feature. This can/does
enforce a stronger layering invariant than LLVM has historically
been enforced. So I'm curious to get buy-in and maybe document
this if it's something people like the idea of.<br>
<br>
I'm starting this discussion here rather than in an actual code
review on llvm-commits since it seems like it could do with a
bit of a wider discussion, but once/if the general direction is
agreed on, I'll send a patch for review of specific wording for
the LLVM Coding Standards.<br>
<br>
<br>
Currently the <a href="https://llvm.org/docs/CodingStandards.html" target="_blank">LLVM Coding Standards</a> doesn't say
much/anything about layering. <a href="https://llvm.org/docs/CodingStandards.html#a-public-header-file-is-a-module" target="_blank">'A Public Header File <b>is</b> a
Module'</a> section talks about modules of functionality,
mostly trying to describe why a header file should be self
contained - but uses anachronistic language about modules that
doesn't line up with the implicit or explicit modules concepts
in use today, I think.<br>
<br>
I propose making this wording a bit more explicit, including:<br>
<br>
1) Headers should be standalone (include all their dependencies
- this is mentioned in the "is a Module" piece, by way of a
technique to help ensure this, but not explicit as a goal
itself).<br>
<br>
2) Files intended to be included in a particular context (that
aren't safe/benign to include multiple times, in multiple .cpp
files, etc) should use a '.inc' or '.def' (.def specifically for
those "define a macro, include the header which will reference
that macro" style setups we have in a few places).<br>
</div>
</blockquote></div><div text="#000000" bgcolor="#FFFFFF">
Everything up to here seems non-controversial. We should document
this and ideally identify tooling suitable to enforce it.</div><div text="#000000" bgcolor="#FFFFFF"><br>
<blockquote type="cite">
<div dir="ltr"><br>
And the actual layering issue:<br>
3) Each library should only include headers or otherwise
reference entities from libraries it depends on. Including in
headers and inline functions. A simple/explicit way to put this:
every inline function should be able to be moved into a .cpp
file and the build (with a unix linker - one that cannot handle
circular library dependencies) should still succeed.<br>
<br>
<br>
This last point is the most interesting - and I hope one that
people generally find desirable, so it might not be immediately
obvious why it may be contentious or difficult:<br>
<br>
LLVM violates this constraint by using inline functions in
headers to avoid certain layering constraints that might
otherwise cause the build to fail. A couple of major examples
I've hit are:<br>
<br>
<a href="http://lists.llvm.org/pipermail/llvm-dev/2017-December/119494.html" target="_blank">TargetSelect.h </a>and similar: This
one's especially tricky - the header is part of libSupport, but
each function in here depends on a different subset of targets
(creating a circular dependency) - to call the given function
the programmer needs to choose the right dependencies to link to
or the program will not link.<br>
<a href="https://reviews.llvm.org/D41357" target="_blank">Clang
Diagnostics</a> (work in progress): The diagnostics for each
component are in their own component directories, but are then
all included from libClangBasic, a library none of those
components depends on. (so this isn't so much an inlining case
as #include based circular dependency)<br>
<br>
<br>
Generally I'd like to get buy-in that stricter layering is
desirable, and that these few cases are at least sub-optimal, if
accepted for now.<br>
</div>
</blockquote></div><div text="#000000" bgcolor="#FFFFFF">
I have no strong opinion on this topic. My experience has been that
it's often far harder to unwind these types of inline dependencies
than it first seems and that the value in doing so is often
unclear. I'm not opposed, but I'm also not signing up to help. :)<br></div></blockquote><div><br>Oh, yeah - mostly I'm looking for community agreement (enough for me to change the Coding Standards and to push for adherence when these issues come up in future changes) about the general principle.<br><br>For existing violations - I'm not expecting people to sign up to help, and I'm not sure how many I'll fix/get through before I get tired and just whitelist them in as "old quirky LLVM" with a note that if someone gets deep into any of that code for other reasons, they might want to keep in mind how these issues could be fixed while they're there.<br><br>- Dave<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div text="#000000" bgcolor="#FFFFFF">
<blockquote type="cite"></blockquote></div><div text="#000000" bgcolor="#FFFFFF"><blockquote type="cite">
<div dir="ltr"><br>
Happy to go into more details about any of this, examples, etc,
but I realize this is already a bit long.<br>
- Dave</div>
<br>
<fieldset class="m_2745718309214774179mimeAttachmentHeader"></fieldset>
<br>
<pre>_______________________________________________
LLVM Developers mailing list
<a class="m_2745718309214774179moz-txt-link-abbreviated" href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>
<a class="m_2745718309214774179moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a>
</pre>
</blockquote></div></blockquote></div></div>