<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p.msonormal0, li.msonormal0, div.msonormal0
{mso-style-name:msonormal;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
p.m-7755242525583096278m-1817337732708028278m8143167336583448376msolistparagraph, li.m-7755242525583096278m-1817337732708028278m8143167336583448376msolistparagraph, div.m-7755242525583096278m-1817337732708028278m8143167336583448376msolistparagraph
{mso-style-name:m_-7755242525583096278m-1817337732708028278m8143167336583448376msolistparagraph;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
span.EmailStyle19
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
/* List Definitions */
@list l0
{mso-list-id:2022852973;
mso-list-template-ids:1429626356;}
ol
{margin-bottom:0in;}
ul
{margin-bottom:0in;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal">Simplifying (and making things more robust in the process) sounds great to me. I think the current iteration of how parameters are passed to the tests is quite complicated and unclear, so this will be a step in the right direction and if
there’s a need for gcc later, we can take the time to design the feature properly so that it works across the lit tests and the test suite both.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Thanks,<o:p></o:p></p>
<p class="MsoNormal">-Stella<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><b>From:</b> Zachary Turner <zturner@google.com> <br>
<b>Sent:</b> Tuesday, November 13, 2018 4:30 PM<br>
<b>To:</b> Stella Stamenova <stilis@microsoft.com><br>
<b>Cc:</b> reviews+D54009+public+0e164460da8f1d7f@reviews.llvm.org; pavel@labath.sk; chris.bieneman@me.com; dccitaliano@gmail.com; aleksandr.urakov@jetbrains.com; jdevlieghere@apple.com; abidh.haq@gmail.com; teemperor@gmail.com; ki.stfu@gmail.com; mgorny@gentoo.org;
dan@su-root.co.uk; jfbastien@apple.com; lldb-commits@lists.llvm.org; llvm@inglorion.net<br>
<b>Subject:</b> Re: [PATCH] D54009: Refactor LLDB lit configuration files<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">use_clang() already will fall back on searching the environment variable 'CLANG' to find a path to it.<o:p></o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<div>
<p class="MsoNormal"> self.config.clang = self.use_llvm_tool(<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> 'clang', search_env='CLANG', required=required)<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">But we could make this environment variable a parameter to use_clang() if we wanted to. As long as we can agree that we don't need to worry about gcc -- at least for now -- then it should all simplify down quite a bit. And AFAICT, there's
nobody using gcc with the lit tests right now, so it just adds unnecessary complexity. And if and when we do have people using it, there is even more work to be done. <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">If someone only wants a clang that isn't the just-built clang (for example a release version to make sure the tests run faster), all they need to do is set the environment variable 'CLANG' and it should be fine. <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Since the lit suite is still very new and developing, I'm not too concerned about regressing a feature (especially one with zero users), because the important thing to me is that it's designed right so that the feature can grow in organically
and not be "forced" in with a subpar implementation.<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">On Tue, Nov 13, 2018 at 4:23 PM Stella Stamenova <<a href="mailto:stilis@microsoft.com">stilis@microsoft.com</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">The plan for the lit tests sounds reasonable to me. I would also remove LLDB_TEST_C/CXX_COMPILER entirely so that we can reduce confusion since they’re only used for the lit tests,
right?<o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">My only concern is that I’ve been told that there are people who will build lldb with a different compiler than the tests – so the properties for LLDB_TEST_C/CXX_COMPILER might
actually be used especially in cases where clang is not built alongside lldb.<o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Thanks,<o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">-Stella<o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><b>From:</b> Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>>
<br>
<b>Sent:</b> Tuesday, November 13, 2018 4:16 PM<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><br>
<b>To:</b> Stella Stamenova <<a href="mailto:stilis@microsoft.com" target="_blank">stilis@microsoft.com</a>><br>
<b>Cc:</b> <a href="mailto:reviews%2BD54009%2Bpublic%2B0e164460da8f1d7f@reviews.llvm.org" target="_blank">
reviews+D54009+public+0e164460da8f1d7f@reviews.llvm.org</a>; <a href="mailto:pavel@labath.sk" target="_blank">
pavel@labath.sk</a>; <a href="mailto:chris.bieneman@me.com" target="_blank">chris.bieneman@me.com</a>;
<a href="mailto:dccitaliano@gmail.com" target="_blank">dccitaliano@gmail.com</a>;
<a href="mailto:aleksandr.urakov@jetbrains.com" target="_blank">aleksandr.urakov@jetbrains.com</a>;
<a href="mailto:jdevlieghere@apple.com" target="_blank">jdevlieghere@apple.com</a>;
<a href="mailto:abidh.haq@gmail.com" target="_blank">abidh.haq@gmail.com</a>; <a href="mailto:teemperor@gmail.com" target="_blank">
teemperor@gmail.com</a>; <a href="mailto:ki.stfu@gmail.com" target="_blank">ki.stfu@gmail.com</a>;
<a href="mailto:mgorny@gentoo.org" target="_blank">mgorny@gentoo.org</a>; <a href="mailto:dan@su-root.co.uk" target="_blank">
dan@su-root.co.uk</a>; <a href="mailto:jfbastien@apple.com" target="_blank">jfbastien@apple.com</a>;
<a href="mailto:lldb-commits@lists.llvm.org" target="_blank">lldb-commits@lists.llvm.org</a>;
<a href="mailto:llvm@inglorion.net" target="_blank">llvm@inglorion.net</a><br>
<b>Subject:</b> Re: [PATCH] D54009: Refactor LLDB lit configuration files<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Ok so for dotest, it seems to be ignoring the config.cc and config.cxx entirely. So we can theoretically do whatever we want with it, or change around the directory structure so
that it's more like:<o:p></o:p></p>
</div>
</div>
</div>
<div>
<div>
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">lldb<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">* lit<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">* * Dotest<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">* * Unit<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">* * Tests<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">and put the config.cc / config.cxx logic under Tests. That's a large change though and probably not worth making such a large change right away.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">dotest tests manually construct the command line directly in CMake via this `LLDB_DOTEST_ARGS_PROPERTY` global property, and then in lldb/lit/Suite/lit.cfg we have this line:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">dotest_cmd = [config.dotest_path, '-q']<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">dotest_cmd.extend(config.dotest_args_str.split(';'))<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">So pretty much everythign the parent lit file has done is totally ignored. <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">With that in mind, **for the lit tests only** I propose dropping support for non-clang compilers and ignoring LLDB_TEST_C_COMPILER and LLDB_TEST_CXX_COMPILER (you can still have
a custom path to clang executable via an environment variable, which is consistent with how clang's test suite works).<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Note that when you run ninja check-lldb-lit you will now get messages that tell you the exact path to the clang executable, so you can see what the PATH resolution is doing.<o:p></o:p></p>
</div>
</div>
</div>
</div>
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">On Tue, Nov 13, 2018 at 4:02 PM Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<div>
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">On Tue, Nov 13, 2018 at 3:47 PM Stella Stamenova <<a href="mailto:stilis@microsoft.com" target="_blank">stilis@microsoft.com</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">I am not sure if that’s the right solution for a couple of reasons:
<o:p></o:p></p>
<ol start="1" type="1">
<li class="m-7755242525583096278m-1817337732708028278m8143167336583448376msolistparagraph" style="mso-list:l0 level1 lfo1">
As far as I can tell only clang calls use_clang (and only lld calls use_lld), while the other projects such as lld and llvm rely on the environment to be setup correctly<o:p></o:p></li><li class="m-7755242525583096278m-1817337732708028278m8143167336583448376msolistparagraph" style="mso-list:l0 level1 lfo1">
Lld also has tests that invoke clang-cl and they pass – while the ones in LLDB do not, so the invocation of use_clang is not necessary for the tests to pass (maybe?)<o:p></o:p></li><li class="m-7755242525583096278m-1817337732708028278m8143167336583448376msolistparagraph" style="mso-list:l0 level1 lfo1">
LLDB allows us to specify whether to use gcc or clang as well as the path and it can also have a test compiler specified via LLDB_USE_TEST_*_COMPILER, so we should first decide what scenarios we want to support before trying to make this work and possibly making
it even more confusing and complicated<o:p></o:p></li></ol>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Do you know what the answer for 3) is? What compilers are valid to specify for the lit/suite/unittests via the various parameters?<o:p></o:p></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
</div>
</div>
<div>
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">For the unit tests, I don't think we ever specify a compiler, or we don't ever read the value. Because a unit test shouldn't be compiling anything, it's a different kind of test.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">For the dotest suite, specifying the compiler is important and it can definitely be gcc, but I don't think this uses the same method of going through config.cc. In fact, I'm not
sure how it determines what compiler to use at the moment, as it's been a number of years since I've looked at the dotest suite.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">For the lit tests, I'm inclined to say we should keep things simple and only support clang for now, and add support for new compilers such as gcc if and when someone actually wants
it. Otherwise YAGNI.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Definitely that time will come, but it doesn't make sense to support it immediately if nobody is using it today and nobody is planning to enable it immediately.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">So I'm tempted to say that perhaps we should just call llvm_config.use_clang() and llvm_config.use_lld() and ignore LLDB_TEST_COMPILER, which in my experience has only been a source
of unnecessary complexity that never actually gets used in practice.<o:p></o:p></p>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</body>
</html>