[LLVMdev] lit: deprecating trailing \ in RUN lines

[Daniel Dunbar]

Ok, that makes sense. I don't see this as a good enough argument to remove
backslash support though.

For problems like the clang_cc1 substitution mistakes, it would be much
better to just improve the substitution support so that those cause
immediate test failures. I would definitely support a move to make lit's
substitution machinery more strict. For problems like your first search, I
can appreciate how wrapped lines make this less trivial, but this isn't
enough of a use case to motivate removing the feature in my opinion.

My opinion can be summarized as:

1. If you are worried about bogus or broken tests, but you have an
automatic way to detect the brokenness, then the right way to attack this
is to build the check into the test runner so no one ever can write the
test to pass again.

2. If you are trying to do more fuzzy searches that are going to no be
fully automatic, the individual developer can deal with the existence of
wrapped lines. Or we could add a way to have lit run a regexp over all of
the (collapsed) test script lines if this is a really important use case
for you. Either way, its not a good enough argument to remove a useful
feature.

 - Daniel



On Sun, Dec 8, 2013 at 11:47 AM, Alp Toker <alp at nuanti.com> wrote:

>
> On 08/12/2013 16:43, Daniel Dunbar wrote:
>
> With one-to-one mapping, it becomes possible to use simple tools like grep
>> to validate common mistakes like %clang / %clang_cc1 mixups, a missing -o
>> flag and so on.
>>
>> Right now there's no obvious way to do those checks and we've ended up
>> without an easy way to lint for broken tests as a result. Each broken test
>> has a high cost so we need to continually look at ways to improve the
>> situation.
>
>
>  Can you elaborate on exactly what kind of checks you want to do?
>
>
> Basically, it's for asking questions about what's currently tested..
>
> e.g. *Find me all tests for c++11 exceptions*:
>
> git grep 'RUN:.*c++11' -- test/ | grep exceptions
>
>
> .. experimenting with new features or refactoring existing tests..
>
> e.g. *How many tests could we add -fsyntax-only to?*
>
> giteach perl -pi -e 's/((RUN:.*%clang_cc1)(.*))$/$2 -fsyntax-only$3/g
> unless
> m/(-fsyntax-only|-analyze|-arcmt-check|-arcmt-migrate|-o|-emit-llvm|-E|-ast-print|-ast-dump|[|>]|\\$)/'
>
> Answer, 405. Then git diff and page through to see if any are actionable.
>
>
> and fixing broken tests..
>
> eg. *Let's find and fix %clang_cc1 substitution mistakes..*
>
> giteach perl -pi -e 's/(RUN:.*%clang_cc1)[^\s]+/$1/g'
>
>
> I committed the results of this run just a few minutes ago in r196729 and
> r196730.
>
> One was a test that had been disabled since 2007.
>
> Losing 80 columns is a small price to pay if it'll help make tests easier
> to write, understand and validate in my opinion.
>
> (The two editors I use, vim and XCode, both do a good job of virtual line
> wrapping so it turns out long RUN lines aren't that much of a big deal in
> practice. But I can see why it may be a bigger problem with editors that
> don't support wrapping. Is this still a problem in 2013?)
>
>
> Alp.
>
> -- http://www.nuanti.com
> the browser experts
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20131209/78ca5afa/attachment.html>