dblaikie added a comment.
> 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.
Yeah, that's the main/a major concern as @aaron.ballman has mentioned.
I think in this case a regex over either would be acceptable, since the test
probably wasn't intending to test the particular wording for a particular
version (presumably if this is the place where this separate wording is
uniquely used (rather than some generic diagnostic infrastructure change) then
it's got a C++17 test already)
> 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.
I think where `#if` works well, it seems good to prefer it, yeah.
> Do all `CodeGenCXX` tests have to be fixed?
I think so.
================
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
----------------
MaskRay wrote:
> 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.
> 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.
Not everything needs to be addressed in this patch - these cleanups can be in
several preliminary patches that are isolated and not dependent on the switch
to C++17.
It looks like in this case there's one `CHECK-1Z` that's out of place (line
111, test seems to pass OK even if that's a plain `CHECK` so I guess it
probably should be) - and the rest are all at the end, which could be moved to
a separate file and done with a hardcoded C++17 target. (but yeah, after the
default switch we might end up wanting to remove the hardcoded 17 (allowing the
test to become "17 and above"), move more test coverage over to here and leave
whatever's 14 specific in the old test)
(maybe as a separate pass, though, to your point - we might be able to go
through and remove `-std=c++17` from tests once that's the default, so the
tests are forwards compatible to the next language version, most likely - but,
again, a broader LIT feature that @aaron.ballman is referring to, where tests
could specify which configurations (beyond language versions, this could also
help cover things like... oh, right here: https://reviews.llvm.org/D125946,
tests could be assumed to be correct for clang-repl too, and some mode where
clang-repl's used instead of clang for broad-scale testing of clang-repl)
================
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
----------------
MaskRay wrote:
> aaron.ballman wrote:
> > Same question here as above
> The check prefixes assume that this is for C++14.
so in this case the 17 test could become language-agnostic after the version
default change?
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
