Someone internally's been dabbling with turning on some of Google's build system's stricter header checking modes - and they caught these (you can check the internal code review 184305506 for the original where I'm pulling things out from). & yeah, fair question about the modules builds - I don't fully understand what they catch and don't catch. I suspect it's a matter of whether the headers themselves form a cycle - whereas the Google build system's a bit more structured - knowing about libraries and explicit dependencies between them. & I didn't look closely to see if, as I said, MC could depend on DebugInfoCodeView - probably weird/awkward/not what we want (I'm guessing?) but the modules buildbots don't have explicit dependencies, they just fail when they find a cycle. So they can implicitly support all sorts of weird dependencies we might not've expected/planned, so long as it's not an actual cycle (& only within the headers - so the modules system might be like "oh, sure, MC can depend on DebugInfoCodeView" but the library dependency might be the exact opposite (some MC .cpp file could call into DebugInfoCodeView & the modules build wouldn't fail - it knows nothing about .cpp modularity/grouping, just headers)) On Wed, Mar 21, 2018 at 11:42 AM Zachary Turner <zturner at google.com> wrote:> Yes, some of the headers and stuff that are just raw structure definitions > and enums could probably be sunk into BinaryFormat.. > > How'd you find this? Curious why it hasn't been breaking in modules > builds for a long time. > > On Wed, Mar 21, 2018 at 11:31 AM David Blaikie <dblaikie at gmail.com> wrote: > >> I'm looking at fixing some layering violations in LLVM & came across a >> few in the CodeView handling, specifically: >> >> lib/MC/MCCodeView includes several llvm/DebugInfo/CodeView headers >> I guess MC could be made dependent on DebugInfoCodeView? But probably >> these things should be sunk into BinaryFormat as is the case for DWARF >> features used by MC? >> >> include/llvm/Object/COFF.h includes >> include/llvm/DebugInfo/CodeView/CVDebugRecord.h >> Also seems like this could/should/needs to be sunk into BinaryFormat? >> >> I'm open to ideas & happy to do the work, or help in any way that might >> be useful. >> >> Thanks, >> - Dave >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180321/4a29f1f5/attachment.html>
Intuitively I'd think MC and DebugInfo should be independent. MC is a producer, DebugInfo is a consumer. What's common is the definition of the structures they operate on, which doesn't properly belong to either one. --paulr From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of David Blaikie via llvm-dev Sent: Wednesday, March 21, 2018 11:52 AM To: Zachary Turner Cc: llvm-dev Subject: Re: [llvm-dev] CodeView layering Someone internally's been dabbling with turning on some of Google's build system's stricter header checking modes - and they caught these (you can check the internal code review 184305506 for the original where I'm pulling things out from). & yeah, fair question about the modules builds - I don't fully understand what they catch and don't catch. I suspect it's a matter of whether the headers themselves form a cycle - whereas the Google build system's a bit more structured - knowing about libraries and explicit dependencies between them. & I didn't look closely to see if, as I said, MC could depend on DebugInfoCodeView - probably weird/awkward/not what we want (I'm guessing?) but the modules buildbots don't have explicit dependencies, they just fail when they find a cycle. So they can implicitly support all sorts of weird dependencies we might not've expected/planned, so long as it's not an actual cycle (& only within the headers - so the modules system might be like "oh, sure, MC can depend on DebugInfoCodeView" but the library dependency might be the exact opposite (some MC .cpp file could call into DebugInfoCodeView & the modules build wouldn't fail - it knows nothing about .cpp modularity/grouping, just headers)) On Wed, Mar 21, 2018 at 11:42 AM Zachary Turner <zturner at google.com<mailto:zturner at google.com>> wrote: Yes, some of the headers and stuff that are just raw structure definitions and enums could probably be sunk into BinaryFormat.. How'd you find this? Curious why it hasn't been breaking in modules builds for a long time. On Wed, Mar 21, 2018 at 11:31 AM David Blaikie <dblaikie at gmail.com<mailto:dblaikie at gmail.com>> wrote: I'm looking at fixing some layering violations in LLVM & came across a few in the CodeView handling, specifically: lib/MC/MCCodeView includes several llvm/DebugInfo/CodeView headers I guess MC could be made dependent on DebugInfoCodeView? But probably these things should be sunk into BinaryFormat as is the case for DWARF features used by MC? include/llvm/Object/COFF.h includes include/llvm/DebugInfo/CodeView/CVDebugRecord.h Also seems like this could/should/needs to be sunk into BinaryFormat? I'm open to ideas & happy to do the work, or help in any way that might be useful. Thanks, - Dave -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180321/2fd84ecd/attachment.html>
DebugInfoCodeView is nto strictly a consumer, it is also a producer. On Wed, Mar 21, 2018 at 1:29 PM <paul.robinson at sony.com> wrote:> Intuitively I'd think MC and DebugInfo should be independent. MC is a > producer, DebugInfo is a consumer. What's common is the definition of the > structures they operate on, which doesn't properly belong to either one. > > --paulr > > > > *From:* llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] *On Behalf Of *David > Blaikie via llvm-dev > *Sent:* Wednesday, March 21, 2018 11:52 AM > *To:* Zachary Turner > *Cc:* llvm-dev > *Subject:* Re: [llvm-dev] CodeView layering > > > > Someone internally's been dabbling with turning on some of Google's build > system's stricter header checking modes - and they caught these (you can > check the internal code review 184305506 for the original where I'm pulling > things out from). > > & yeah, fair question about the modules builds - I don't fully understand > what they catch and don't catch. I suspect it's a matter of whether the > headers themselves form a cycle - whereas the Google build system's a bit > more structured - knowing about libraries and explicit dependencies between > them. > > & I didn't look closely to see if, as I said, MC could depend on > DebugInfoCodeView - probably weird/awkward/not what we want (I'm guessing?) > but the modules buildbots don't have explicit dependencies, they just fail > when they find a cycle. So they can implicitly support all sorts of weird > dependencies we might not've expected/planned, so long as it's not an > actual cycle (& only within the headers - so the modules system might be > like "oh, sure, MC can depend on DebugInfoCodeView" but the library > dependency might be the exact opposite (some MC .cpp file could call into > DebugInfoCodeView & the modules build wouldn't fail - it knows nothing > about .cpp modularity/grouping, just headers)) > > > > On Wed, Mar 21, 2018 at 11:42 AM Zachary Turner <zturner at google.com> > wrote: > > Yes, some of the headers and stuff that are just raw structure definitions > and enums could probably be sunk into BinaryFormat.. > > > > How'd you find this? Curious why it hasn't been breaking in modules > builds for a long time. > > > > On Wed, Mar 21, 2018 at 11:31 AM David Blaikie <dblaikie at gmail.com> wrote: > > I'm looking at fixing some layering violations in LLVM & came across a few > in the CodeView handling, specifically: > > lib/MC/MCCodeView includes several llvm/DebugInfo/CodeView headers > I guess MC could be made dependent on DebugInfoCodeView? But probably > these things should be sunk into BinaryFormat as is the case for DWARF > features used by MC? > > include/llvm/Object/COFF.h includes > include/llvm/DebugInfo/CodeView/CVDebugRecord.h > Also seems like this could/should/needs to be sunk into BinaryFormat? > > I'm open to ideas & happy to do the work, or help in any way that might be > useful. > > Thanks, > - Dave > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180321/c83583a9/attachment.html>