Second Lit tests C++11 compatibility patch: using preprocessor to filter expected-error

Mon Aug 17 17:40:31 PDT 2015

On Mon, Aug 17, 2015 at 5:15 PM, Li, Charles via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Hi Richard and Justin,
>
>
>
>
>
> > What's the upside to this approach? AFAICT it makes the test harder to
> read and errors less informative due to pointing at the wrong lines, but
> (at least in switch-1.c) it doesn't actually reduce any code duplication or
> anything like that. What is this gaining us apart from not having to create
> one more file?
>
>
>
> Thank you Justin.
>
> Our original intention was to get the Lit tests to run at any default C++
> dialect.
>
>
>
> We first discovered that FileCheck does not respect #ifdef since it does
> not know about pre-defined macros.
>
> So we figured if we preprocess the source first, the preprocessor will
> filter the #ifdef sections and the output will feed nicely into FileCheck.
>
> The upside is the test can run at the default dialect in addition to
> explicitly specified dialect.
>
> The downside is, as you mentioned, the errors diagnostics would point to
> the wrong lines.
>
>
>
>
>
> > The only thing novel about this approach is using the preprocessor to
> achieve it. -verify *does* respect #ifdef, and we have a lot of tests that
> rely on that.
>
>
>
> Thank you Richard.
>
> We erroneously assumed that “// CHECK:” and “// expected-error” work the
> same way.
>
> But now we realized that assumption was wrong.
>
>
>
> In light this discussion, I have removed the preprocessing to temporary
> step for all tests.
>
> The attached patch (Lit_24.patch) revised 2 test fixes relative to the
> previous patch (Lit_22.patch)
>
>
>
>   test/Analysis/temp-obj-dtors-cfg-output.cpp
>
>     This test uses FileCheck to check for CFG dump.
>
>     Instead of using #ifdef for the dialect specific “// CHECK:” lines,
>
>     I have created 2 check-prefixes “CXX11” and “CXX98”.
>
>     The pre-process step have been removed.
>
>
>
>   test/Sema/switch-1.c
>
>     This test uses –verify to check for integer overflows diagnostics.
>
>     The pre-process step have been removed.
>
>     The #ifdef is kept since it works with -verify.
>

Instead of duplicating the code, you can do this:

case blah:
#if __cplusplus >= 201103L
// expected-error at -2 {{error 2 lines above}}
#else
// expected-error at -4 {{error 4 lines above}}
#endif

> Please let me know how you feel about this patch.
>
>
>
> Sincerely,
>
> Charles
>
>
>
>
>
> *From:* metafoo at gmail.com [mailto:metafoo at gmail.com] *On Behalf Of *Richard
> Smith
> *Sent:* Monday, August 17, 2015 1:07 PM
> *To:* Li, Charles
> *Cc:* cfe-commits at lists.llvm.org
> *Subject:* Re: Second Lit tests C++11 compatibility patch: using
> preprocessor to filter expected-error
>
>
>
> On Mon, Aug 17, 2015 at 9:56 AM, Li, Charles via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
> Hi Clang developers,
>
>
>
> We here at Sony are continuing to update the Lit tests for C++ dialects
> compatibility.
>
> Attached is the second patch. (As a reference, here is the link to the
> discussion on the previous Lit patch.
> http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20150727/134667.html
> )
>
> In this second patch, there is 1 complicated change and 3 simple changes.
>
>
>
> Here is the complicated change first.
>
>
>
> test/Sema/switch-1.c
>
>   This test verifies the diagnostics for integer overflows.
>
>   Due to C++11’s more strict requirements on constant-expressions in
> 5.19p2 [expr.const],
>
>   The diagnostics have changed from “overflow in expression” to “not a
> constant expression”.
>
>
>
>   Usually we would create a C++11 version of the switch-1.c file.
>
>   But here we propose a novel approach to “#ifdef” the expected
> diagnostics. (We hope to use this approach for all similar cases in the
> future.)
>
>   Normally ‘// expected-error’ does not honor any ‘#ifdef’.
>
>   But if we first preprocess the source into a temporary file, only the
> valid ‘#ifdef’ sections remain.
>
>   We then run the preprocessed file at the desired dialect.
>
>   The main downside to this approach is If the test fails, the errors are
> reported on the temporary file, not on the original file, and the line
> numbers of these two files  do not match
>
>
>
> The only thing novel about this approach is using the preprocessor to
> achieve it. -verify *does* respect #ifdef, and we have a lot of tests that
> rely on that.
>
>
>
> Here are the simple changes.
>
>
>
> test/Analysis/temp-obj-dtors-cfg-output.cpp
>
>   This test verifies CFG dump for temporary destructors
>
>   C++11 no longer has the following implicit cast.
>
>     (ImplicitCastExpr, NoOp, const struct D)
>
>   We modified the test using the #ifdef approach to have the preprocessor
> generate the desired CHECK lines.
>
>
>
> test/CodeCompletion/ordinary-name.cpp
>
>   This test verifies for code completion patterns.
>
>   Since C++11 has more keywords than C++98,
>
>   We made this test to be C++98 specific, and create a separate C++11
> version.
>
>
>
> test/CodeCompletion/ordinary-name-cxx11.cpp
>
>   This is the C++11 specific version of the code completion.
>
>   This test added patterns for the following keywords:
>
>     char16, char32, noexcept, nullptr, sizeof...,
>
>     auto, decltype, char16_t, char32_t
>
>
>
> test/Sema/thread-specifier.c
>
>   Tests for __thread specifier at various C++ dialects
>
>   We made the default RUN line explicit to be at –std=c++98
>
>
>
>
>
> If there is anything that seems confusing to you, please let me know.
>
> I would be more than happy to expand on the reasons for the these changes.
>
>
>
>
>
> Thanks,
>
> Charles
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150817/0b0167f0/attachment-0001.html>