On Mon, Aug 17, 2015 at 6:27 PM, Li, Charles < charles...@playstation.sony.com> wrote:
> Hi Richard, > > > > I have modified the “expected-“ lines as you requested. > +#if (!defined(__cplusplus)) || (__cplusplus <= 199711L) // C or C++03 or earlier modes You can simplify that to: #if __cplusplus <= 199711L __cplusplus will expand to 0 inside a #if in C (as will any un#defined identifier). Otherwise, this LGTM. Cheers, > > Charles > > > > > > *From:* meta...@gmail.com [mailto:meta...@gmail.com] *On Behalf Of *Richard > Smith > *Sent:* Monday, August 17, 2015 5:41 PM > *To:* Li, Charles > *Cc:* Justin Bogner; cfe-commits@lists.llvm.org > > *Subject:* Re: Second Lit tests C++11 compatibility patch: using > preprocessor to filter expected-error > > > > On Mon, Aug 17, 2015 at 5:15 PM, Li, Charles via cfe-commits < > cfe-commits@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@-2 {{error 2 lines above}} > > #else > > // expected-error@-4 {{error 4 lines above}} > > #endif > > > > Please let me know how you feel about this patch. > > > > Sincerely, > > Charles > > > > > > *From:* meta...@gmail.com [mailto:meta...@gmail.com] *On Behalf Of *Richard > Smith > *Sent:* Monday, August 17, 2015 1:07 PM > *To:* Li, Charles > *Cc:* cfe-commits@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@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@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
