Handles failing driver tests of clang

[Purva-Chaudhari]

Summary

Added support for incremental mode 8 and 28 ie. frontend::EmitBC: and frontend::PrintPreprocessedInput:
Added supporting clang tests to test in clang-repl mode

[v.g.vassilev]

@Purva-Chaudhari, this looks reasonable to me. I am adding more reviewers to discuss the way we refer other tests from clang-repl.

clang/test/Interpreter/clangtests.cpp

1 ↗

(On Diff #430570)

@rsmith, would it be acceptable to have a test that refers other tests from the repo in that manner?

[dblaikie]

clang/test/Interpreter/clangtests.cpp

1 ↗

(On Diff #430570)

Generally that's not done - and the inputs should be moved into an "Inputs" subdirectory and shared from there. Tests that are in different subdirectories - not sure if there's a good way to share those, maybe with an "Inputs" directory in a parent directory of both tests? We might not have precednt for that

[dblaikie]

clang/test/Interpreter/clangtests.cpp

1 ↗

(On Diff #430570)

But more broadly, could you explain what the goal of these tests are? Generally I would discourage what I think of as "shotgun" testing - taking some existing comprehensive test for a particular feature and using it to test a mostly orthogonal feature. The orthogonal feature should have more targeted tests/it shouldn't be using such broad testing in the regression suite here.

[v.g.vassilev]

clang/test/Interpreter/clangtests.cpp

1 ↗

(On Diff #430570)

My take is that clang-repl is basically clang that takes inputs incrementally. Being that, means that we should be in a position to process whatever clang processes and thus we run against all of the existing tests. We planned to add the ones which we did not support as regression tests.

We can add more targeted tests but they would be copies or simplifications of already existing ones. Hence there is my hesitation - reuse or duplication...

[dblaikie]

clang/test/Interpreter/clangtests.cpp

1 ↗

(On Diff #430570)

My take is that clang-repl is basically clang that takes inputs incrementally. Being that, means that we should be in a position to process whatever clang processes and thus we run against all of the existing tests.

Yeah, that's sort of the "proof by absurdity" - we wouldn't want every clang test running in both ahead of time and incremental mode in the usual "check-clang" regression suite (I wouldn't mind having a separate mode for testing - more of an integration test that some buildbots or those working on more comprehensive clang-repl support could run, but most people/especially fast bots would not). So then the question for me is which tests should we have running all the time in "check-clang" - and my general answer is: Situations that have motivated code changes/support in clang-repl: If no code was added/changed/etc to clang-repl, then no test should be added to "check-clang" for that test case.

If that "run everything under check-clang run under clang-repl to find missing functionality" found some clang test that didn't work with clang-repl, yeah, I'd generally be in favour of not reusing an input or duplicating in its entirety - but reducing the test case to test only the specific clang-repl functionality issue, and testing that in particular.

Like we shouldn't test every feature of static-assert with clang-repl and clang in every "clang-check" if most of those features aren't distinctly interesting in both cases. Just enough in clang-repl to test what makes static-assert interesting in clang-repl.

[v.g.vassilev]

clang/test/Interpreter/clangtests.cpp

1 ↗

(On Diff #430570)

@dblaikie, that makes sense to me.

@Purva-Chaudhari, can you address it by having just something like
// RUN: clang-repl -Xcc -E to exercise the -E and likewise for the EmitBC case?

[Purva-Chaudhari]

modify tests

[v.g.vassilev]

LGTM!

clang/lib/Interpreter/IncrementalParser.cpp

             LLVM_FALLTHROUGH;
           case frontend::EmitAssembly:
             LLVM_FALLTHROUGH;
+          case frontend::EmitBC:
+            LLVM_FALLTHROUGH;
           case frontend::EmitObj:
             LLVM_FALLTHROUGH;
+          case frontend::PrintPreprocessedInput:
+            LLVM_FALLTHROUGH;
           case frontend::EmitLLVMOnly:
             Act.reset(new EmitLLVMOnlyAction(&LLVMCtx));
             break;

clang/test/Interpreter/incremental-mode.cpp

+// RUN: clang-repl -Xcc -E
+// RUN: clang-repl -Xcc -emit-llvm 
+// expected-no-diagnostics