[cfe-commits] [PATCH] Implement AST dumper for Decls

[Alexander Kornienko]

On Tue, Oct 2, 2012 at 2:49 AM, Philip Craig <
reviews at llvm-reviews.chandlerc.com> wrote:

>
>   Which part of the unit tests do you think is bulky? The initial
> infrastructure is mostly a copy from StmtPrinterTest.cpp. This shouldn't
> need to be changed when adding tests (and probably should be moved to some
> common code), so the size of the actual tests is similar to what is needed
> for FileCheck.
>

I think the code except actual tests is not needed if your task can be
solved using something simpler.

  I used FileCheck for the tests for my standalone dumper, and had a couple
> of problems. It:
>   * uses the same compiler options and language for the whole test
>
You can use multiple RUN: lines with different -D defines, and #ifdef
blocks in the code.


>   * currently ignores leading whitespace so can't test indentation
>
You can use --strict-whitespace option and regex syntax for CHECK: lines.
For example, as in this .sh script:

#!/bin/bash
echo "   test" | FileCheck --strict-whitespace $0

# CHECK: {{^  test$}}

It detects wrong leading whitespace:

./test.sh:4:10: error: expected string not found in input
# CHECK: {{^  test$}}
         ^
<stdin>:1:4: note: scanning from here
   test
   ^


  * is fragile if you want to test the SourceRange dumping
>

Yes, these tests are fragile to some extent, if you use them to test source
locations. When your add or remove test code, you need to change CHECKs
after these lines. But these problems are easy to detect and fix (if you're
not going to write thousands of lines of tests in one file). And they are
relatively easy to avoid, if you add new tests only to the end of the file.


>   The ability to use an AST matcher for unit tests also makes them
> slightly more convenient (I realise there'll be a command line option for
> this eventually).
>

I'm not sure what convenience do you mean, I see mostly increased
complexity.


>   The main advantage I can see for FileCheck is that it provides more
> flexible matching.
>

The next one is that it's quite simple and you can expect a certain level
of correctness and stability from it. And you probably want your test code
to be as simple, as it can be, so anyone can easily read, understand and
change it. And you probably don't want too many dependencies in your tests,
so you don't have to catch bugs in tests when any library you rely on,
changes.

So I'm definitely for using FileCheck in this case, but it's only me,
others may disagree.

-- 
Regards,
Alex
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121002/5740994e/attachment.html>