MaskRay added a comment.
It will take some time to fix all tests properly. Let's have a guideline how to
fix them properly. I tried fixing some using several patterns in the last
revision. I didn't fix all, because it would likely lead to an unsatisfactory
result and I would spend more time :)
A construct which causes an error with C++17 onwards
----------------------------------------------------
I keep the existing `RUN:` lines and use something like
#if __cplusplus < 201703L
void* operator new[](std::size_t) throw(std::bad_alloc);
#endif
it the construct appears tested elsewhere or
register int ro; // expected-error {{illegal storage class on file-scoped
variable}}
#if __cplusplus >= 201703L
// expected-error@-2 {{ISO C++17 does not allow 'register' storage class
specifier}}
#elif __cplusplus >= 201103L
// expected-warning@-4 {{'register' storage class specifier is deprecated}}
#endif
if the test file appears the proper place to test the construct.
Pre-C++17 and C++17 have different results
------------------------------------------
I use something like
// RUN: %clang_cc1 %s -fsyntax-only -verify=expected,precxx17 -std=c++14
-fdata-sections -fcolor-diagnostics
// RUN: %clang_cc1 %s -fsyntax-only -verify -std=c++17 -fdata-sections
-fcolor-diagnostics
...
TypedefAligned4 TA8c = TA8a + TA8b; // expected-warning {{passing 4-byte
aligned argument to 8-byte aligned parameter 'this' of 'operator+' may result
in an unaligned pointer access}}
// expected-warning@-1 {{passing
4-byte aligned argument to 8-byte aligned parameter 1 of 'operator+' may result
in an unaligned pointer access}}
// precxx17-warning@-2 {{passing
4-byte aligned argument to 8-byte aligned parameter 'this' of 'StructAligned8'
may result in an unaligned pointer access}}
Since every RUN line uses -std=.
Pros: using `-verify` avoids `#if` directives which make the result look better.
Cons: when we upgrade to C++20, 23, ..., the tests will become less relevant.
Discussion
----------
Do we want to prefer `#if` in all cases? They work well with `-verify` tests
but not so well with `FileCheck` since `FileCheck` does not ignore preprocessor
skipped ranges.
Do all `CodeGenCXX` tests have to be fixed?
================
Comment at: clang/test/AST/ast-dump-undeduced-expr.cpp:1
-// RUN: not %clang_cc1 -triple x86_64-unknown-unknown -ast-dump %s | FileCheck
%s
+// RUN: not %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -ast-dump %s
| FileCheck %s
----------------
aaron.ballman wrote:
> What from this test requires C++14?
The C++17 diagnostics uses `static inline constexpr` while pre-C++17 uses
`static constexpr`. Used a regex instead.
================
Comment at: clang/test/AST/sourceranges.cpp:1-2
-// RUN: %clang_cc1 -triple i686-mingw32 -ast-dump %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++14 -triple i686-mingw32 -ast-dump %s | FileCheck %s
// RUN: %clang_cc1 -triple i686-mingw32 -std=c++1z -ast-dump %s | FileCheck %s
-check-prefix=CHECK-1Z
----------------
aaron.ballman wrote:
> I assume this change is because we're testing c++17 on the next RUN line and
> you want to be sure we still get the old test coverage?
>
> The trouble is, when we bump from C++17 to C++20, we'll lose the test
> coverage for the latest standard.
>
> (Not your problem per se, but I continue to think we have a lit deficiency
> where we need some facilities to say "run this test in C++N and later", "run
> this test in C89 through CN", or "run this test in all C++ language modes",
> etc.)
`FileCheck` does not ignore preprocessor skipped ranges, so it is very
difficult to work with both C++14/C++17 defaults, if our intention is to not
touch such tests in the default changing patch.
I think the best strategy is to do another pass to clean up the tests after the
transition, not trying to address everything in this patch.
================
Comment at: clang/test/Analysis/blocks.m:2
// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10
-analyzer-checker=core -fblocks -verify -Wno-strict-prototypes %s
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10
-analyzer-checker=core -fblocks -verify -x objective-c++ %s
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10
-analyzer-checker=core -fblocks -verify -x objective-c++ -std=c++14 %s
----------------
aaron.ballman wrote:
> What from this test is invalid in C++17?
Added `__attribute__((__nothrow__))` in C++17 mode.
================
Comment at: clang/test/CXX/except/except.spec/p2-dynamic-types.cpp:1
-// RUN: %clang_cc1 -fexceptions -fcxx-exceptions -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fexceptions -fcxx-exceptions -fsyntax-only -verify
-std=c++14 %s
----------------
aaron.ballman wrote:
> Why not pass `-Wno-dynamic-exception-spec` instead of limiting the test (we
> support dynamic exception specifications in newer language modes and it would
> be good to not lose that test coverage there).
TIL `-Wno-dynamic-exception-spec`. Adopted.
================
Comment at: clang/test/CXX/except/except.spec/p9-dynamic.cpp:1
-// RUN: %clang_cc1 -no-opaque-pointers %s -triple=x86_64-apple-darwin10
-emit-llvm -o - -fcxx-exceptions -fexceptions | FileCheck %s
--check-prefixes=CHECK,CHECK-PRE17
+// RUN: %clang_cc1 -std=c++14 -no-opaque-pointers %s
-triple=x86_64-apple-darwin10 -emit-llvm -o - -fcxx-exceptions -fexceptions |
FileCheck %s --check-prefixes=CHECK,CHECK-PRE17
// RUN: %clang_cc1 -no-opaque-pointers %s -triple=x86_64-apple-darwin10
-std=c++17 -Wno-dynamic-exception-spec -emit-llvm -o - -fcxx-exceptions
-fexceptions | FileCheck %s --check-prefixes=CHECK,CHECK-17
----------------
aaron.ballman wrote:
> Same question here as above
The check prefixes assume that this is for C++14.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131464/new/
https://reviews.llvm.org/D131464
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
