<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Dec 8, 2013, at 4:24 AM, Alp Toker <<a href="mailto:alp@nuanti.com">alp@nuanti.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
<div bgcolor="#FFFFFF" text="#000000">
<br>
<div class="moz-cite-prefix">On 08/12/2013 11:22, Chandler Carruth
wrote:<br>
</div>
<blockquote cite="mid:CAGCO0Kid8scvO6hgnRH+KdgfaaMXiN_MrXCn5NcOotZM06V1sg@mail.gmail.com" type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">On Sun, Dec 8, 2013 at 2:11 AM, Alp
Toker <span dir="ltr"><<a moz-do-not-send="true" href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"> I'd like to propose
deprecating and shortly thereafter removing the lit test
runner feature that concatenates RUN lines ending in a
trailing \<br>
</div>
</blockquote>
<div><br>
</div>
<div>I'm really opposed to this. Especially for the Clang
test suite where run lines are often *very* long and hard
to organize, read, and edit without this feature.</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"> <br>
<b>Rationale:</b><b><br>
</b><br>
<ul>
<li>Trailing \ has a special meaning in various
language standards that we support nowadays. In the
C preprocessor, for example, it's handled _before_
comments. Various compilers handle this differently
and it introduces needless incompatibilities.</li>
</ul>
</div>
</blockquote>
<div>What incompatibilities? I've never had this be an
issue.</div>
</div>
</div>
</div>
</blockquote>
<br>
It's an issue if you try to run the clang tests against other
compilers, say to check compatibility with MSVC.<br>
<br>
The problem is that "the trailing backslash on a continued line is
commonly referred to as a backslash-newline" -- ie. it's handled by
the preprocessor, so has significance rather than being part of the
comment.<br>
<br></div></blockquote><div><br></div><div>It’s translation phase 1. If you’re seeing differences in behavior about this between compilers, that’s a *huge* bug in those compilers. Can you cite a specific example?</div><br><blockquote type="cite"><div bgcolor="#FFFFFF" text="#000000">
That causes dissonance between what the compiler sees and what
lit.py sees for no particularly good reason. One of the nice
properties of lit tests is that they're also valid compiler inputs,
so trailing slash is a bit unfortunate.<br>
<br></div></blockquote><div><br></div><div>How does the backslash break this in any way?</div><br><blockquote type="cite"><div bgcolor="#FFFFFF" text="#000000">
It less of a problem if you're just consuming the suite with make
check-all, more of a problem for authoring.<br>
<br>
<br>
<blockquote cite="mid:CAGCO0Kid8scvO6hgnRH+KdgfaaMXiN_MrXCn5NcOotZM06V1sg@mail.gmail.com" type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<ul>
<li>Forgetting the trailing \ between two RUN lines
causes the lines to be run individually. People have
checked in tests which they believed were getting
run whereas the features being tested were actually
silently broken. I've been committing fixes for some
of these but it's exceptionally time-consuming to
hunt them down after the fact.<br>
</li>
</ul>
</div>
</blockquote>
<div>I'd like to understand the rate at which this happens
(per RUN-line? per test-file?). It's never been a problem
for me, but that is in part because I check that my tests
fail without my change in addition to passing with my
change. <br>
</div>
</div>
</div>
</div>
</blockquote>
<br>
If only everyone did check their changes.<br>
<br>
See r194071 where trailing backslash caused a test to always
succeed, and r194073 for the kind of long-term a broken test has on
code quality.<br>
<br>
Looking at the SVN log for test/ in clang, and to a lesser extent
LLVM core, I've been doing nearly all the no-op test fixes in the
last few months. Not all of them are related to trailing \ but those
are the most pernicious kinds.<br>
<br>
It's a bad idea to gamble that I or someone else will always be
around taking the time to manually verify old tests to see if they
do what they're meant to do.<br>
<br>
<br>
<blockquote cite="mid:CAGCO0Kid8scvO6hgnRH+KdgfaaMXiN_MrXCn5NcOotZM06V1sg@mail.gmail.com" type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<ul>
<li>Removing trailing \ will introduce the neat
property that one RUN line corresponds precisely to
one command that's executed. This is good for humans
and will enable simplifications in the test runner.</li>
</ul>
</div>
</blockquote>
<div>FWIW, I've never really had a problem that needed this.
The RUN: forms a prefix of a shell script in my head, and
I know how to read shell scripts including multiple lines.</div>
</div>
</div>
</div>
</blockquote>
<br>
The transformations lit does are really too complex and there's at
least one known bug to do with closed pipes that's contributing to
no-op tests (think the discussion thread was on cfe-dev).<br>
<br>
In a nutshell, the script output lit forms right now is not likely
not the pipeline you had in your head ;-)<br>
<br>
We need to simplify this stuff to fix no-op test issues, and also to
achieve improved source line information.<br>
<br>
<blockquote cite="mid:CAGCO0Kid8scvO6hgnRH+KdgfaaMXiN_MrXCn5NcOotZM06V1sg@mail.gmail.com" type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div> <br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<ul>
<li>Eliminating the trailing \ syntax will unblock
work on improved failure source locations in lit.
Right now, when the builders encounter a test RUN
failure it's a matter of guesswork as to which RUN
line is failing, and the cycle of
commit-fix-and-watch-buildbots is laborious. We've
all wasted time with this at some point and can
totally do better.<br>
</li>
</ul>
</div>
</blockquote>
<div>While I would very much enjoy better failure reporting,
I don't really understand why it needs this. We have a
in-python parser for the RUN: lines which understands what
lines a "command" spans?<br>
</div>
<div><br>
</div>
<div>Anyways, even though I would *really* like better
failure reporting from lit, not at the cost of less
readable tests. That's the tail wagging the dog IMO.</div>
</div>
</div>
</div>
</blockquote>
<br>
So, my contention is that the \ is not making the long lines more
readable, just pasting over the complexity and hiding bugs.<br>
<br>
After all, long pipelines aren't how people use the LLVM tools in
the real world and they totally miss out on testing file IO, losing
stdout/stderr distinctions etc.<br>
<br>
Another option is to use a different break marker and require
RUN-NEXT: on continuation lines. But my view is that long RUN lines
could do with simplification anyway, so removing the feature is a
better way forward.<br>
<br>
I'll throw the ball in your court to see if you have a better
solution going forward?<br>
<br>
Alp.<br>
<br>
<br>
<br>
<pre class="moz-signature" cols="72">--
<a class="moz-txt-link-freetext" href="http://www.nuanti.com/">http://www.nuanti.com</a>
the browser experts
</pre>
</div>
_______________________________________________<br>LLVM Developers mailing list<br><a href="mailto:LLVMdev@cs.uiuc.edu">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu">http://llvm.cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br></blockquote></div><br></body></html>