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

[Dmitri Gribenko]

On Tue, Oct 2, 2012 at 6:46 PM, Alexander Kornienko <alexfh at google.com> wrote:
> On Tue, Oct 2, 2012 at 5:36 PM, Dmitri Gribenko <gribozavr at gmail.com> wrote:
>>
>> On Tue, Oct 2, 2012 at 6:28 PM, Alexander Kornienko <alexfh at google.com>
>> wrote:
>> > On Tue, Oct 2, 2012 at 5:15 PM, Dmitri Gribenko <gribozavr at gmail.com>
>> > wrote:
>> >>
>> >> On Tue, Oct 2, 2012 at 5:59 PM, Alexander Kornienko <alexfh at google.com>
>> >> wrote:
>> >> > So I'm definitely for using FileCheck in this case, but it's only me,
>> >> > others
>> >> > may disagree.
>> >>
>> >> I'm in favor of ASTMatchers-based tests for anything dumping related,
>> >> based on my experience of maintaining
>> >> test/Index/annotate-comments.cpp.  Any small change to the testcases
>> >> forces a change of line numbers for everything that follows it (half
>> >> of tests, on average).
>> >
>> > FileCheck has variables now, maybe we can add a special __LINE__
>> > variable
>> > (possibly with a way to specify offset)? In this case we could put CHECK
>> > lines near the actual test code and this would not be bound to absolute
>> > line
>> > numbers. What do you think?
>>
>> That would help somewhat.  But nevertheless, annotate-comments.cpp is
>> the slowest testcase in the testsuite (for some reason, FileCheck
>> takes up to 10s to match it in debug version -- spending most of the
>> time in regex matching).
>
>
> I'll look into adding special variables to FileCheck.
>
> As for debug build, can we somehow use release FileCheck even when testing a
> debug configuration?
>
>>
>> >> With ASTMatchers-based tests we can keep all
>> >> logically related tests in a single file and keep the test case close
>> >> to the expected output while ensuring that different tests are
>> >> isolated from each other.
>> >
>> > Isolation is possible now using separate RUN lines with specific -D
>> > defines
>> > and #ifdef blocks.
>>
>> That's more like putting the isolation into a tool that didn't have
>> it, versus having an approach where testcases are intrinsically
>> isolated.
>
> My problem with ASTMatchers-based tests for dumping is that there are lots
> of dependencies, that can be avoided. And the test infrastructure (a custom
> test infrastructure, used only for one test) is an additional possible point
> of failure, and definitely requires some support. In my opinion it makes
> sense only in case this infrastructure is generalized, moved elsewhere and
> gets its own tests. But still this seems to be overkill.

The infrastructure is rather simple in my opinion, and can (should!)
be shared between (Decl|Stmt)(Printer|Dumper) tests.

Dmitri

-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/