<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Fri, Feb 5, 2016 at 2:11 PM David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Feb 5, 2016 at 11:30 AM, Robinson, Paul <span dir="ltr"><<a href="mailto:Paul_Robinson@playstation.sony.com" target="_blank">Paul_Robinson@playstation.sony.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div lang="EN-US" link="blue" vlink="purple">
<div><span>
<p class="MsoNormal" style="margin-left:.5in">what's the motivation not to use the LLVM versions of these enums<span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
</span><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I originally implemented the Clang bits for debugger tuning using the LLVM enum (r256063). However this caused build failures in clang-tools-extra (which I
didn't have checked out locally) because having CodeGenOptions.h pull in the extra LLVM header caused confusion in some unqualified name references.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Rather than introduce the extra qualifiers in clang-tools-extra (I looked, it would have taken a bit to sort everything out) I undid the dependency on the LLVM
headers and just replicated the enum in Clang. The only place in Clang that still "really" needed the LLVM enum is BackendUtil.cpp (natch) and it is careful not to assume the two enums are defined the same way. </span></p></div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Still seems a bit subtle - are there reasons we should avoid using the LLVM enum apart from the unknown issue with clang-tools-extra? (if we generally keep these types/enums separate, that's OK - but don't want to propagate a workaround if we don't fully understand what we're working around)</div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div></div></div></div></blockquote><div><br></div><div>We've kept a bunch of the target stuff separate, but I don't see any reason to not use the general dwarf enums. I think we should just fix the clang-tools-extra stuff.</div><div><br></div><div>-eric</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-US" link="blue" vlink="purple"><div><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">This got committed in r256078.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">However, I wasn't thorough enough about digging out the LLVM dependencies, and Driver is still using the LLVM enum.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">If I'd gotten the Driver part right the first time around, Benny would've had to move both enums in his patch.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">This Clang-versus-LLVM consideration applies only to the tuning enum, btw; LLVM doesn't have an analog of DebugInfoKind.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">--paulr<u></u><u></u></span></p>
<p class="MsoNormal"><a name="msg-f:1525374269360767556_-774682790__MailEndCompose"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></a></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #b5c4df 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> David Blaikie [mailto:<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>]
<br>
<b>Sent:</b> Friday, February 05, 2016 11:06 AM<br>
<b>To:</b> Robinson, Paul; Adrian Prantl; Eric Christopher; Benjamin Kramer<br>
<b>Cc:</b> <a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<b>Subject:</b> Re: [cfe-dev] Cleanup on aisle DebugInfo enums<u></u><u></u></span></p>
</div>
</div><div><div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">Sounds plausible.<br>
<br>
(just to check: what's the motivation not to use the LLVM versions of these enums if they match the desired semantics? (I imagine there are many good reasons not to use them, just figure it's worth writing down in this thread so it's clear for historic purposes))<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">On Fri, Feb 5, 2016 at 10:57 AM, Robinson, Paul via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<u></u><u></u></p>
<p class="MsoNormal">In r259489, Benny Kramer moved the DebugInfoKind enum into its own<br>
header, include/clang/Driver/DebugInfoKind.h. This was to prevent<br>
Driver from pulling in headers from Frontend. However, this leaves<br>
the enum in kind of a weird place, I think Basic would be better (and<br>
not re-introduce the problem Benny was solving).<br>
<br>
Wondering why the tuning enum didn't cause the same problem, I found<br>
that when I created the Clang enum, I didn't make Driver use it; it's<br>
still using the LLVM version of the enum. While this is not<br>
technically wrong (you could rearrange the values and it would do no<br>
harm) it's an inconsistency within Clang.<br>
<br>
But, if I make Driver use the tuning enum from Clang's CodeGenOptions,<br>
then we'll have the same problem that prompted r259489 in the first<br>
place--Driver depending on Frontend. Rather than create Yet Another<br>
Header, it would be convenient to put the tuning enum in the same<br>
place as DebugInfoKind, as they are topically related and commonly<br>
used in the same places.<br>
<br>
I propose to do the following:<br>
<br>
Patch #1 (totally mechanical)<br>
- move DebugInfoKind.h from Driver to Basic<br>
- rename it to DebugInfoOptions.h<br>
This will take care of all the #include renaming in one go.<br>
<br>
Patch #2 (basically mechanical)<br>
- move the DebuggerKind enum from CodeGenOptions.h to DebugInfoOptions.h<br>
- fix up Driver to use this enum instead of the LLVM equivalent.<br>
<br>
Or I can do it all at once, the combined patch would still be<br>
reasonable I think.<br>
<br>
Sound okay?<br>
--paulr<br>
<br>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><u></u><u></u></p>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div></div></div>
</div>
</div>
</blockquote></div></div></div></blockquote></div></div>