[PATCH] D131464: [test] Make tests pass regardless of gnu++14/gnu++17 default

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I didn't do a full review because I spotted enough concerns that we might want to talk about the approach more first. I think the goal should be to avoid adding a `-std=` to any RUN line unless the test really requires it. I spotted a lot of instances where we went from testing the current language standard mode to losing testing for all language modes later than 14 or 17, which will give us problems when we go to update to C++20 or beyond. I think we want to use more `#if __cplusplus` or disable diagnostics when possible.

I made another lamentation about the lack of lit facilities that I think would solve these problems for us, but that's a big ask and is not something I insist upon for this. But if you felt like it was a good approach and wanted to work on it, I certainly wouldn't be sad about it. :-D



================
Comment at: clang/lib/Basic/LangStandards.cpp:81
     else
-      return LangStandard::lang_gnucxx14;
+      return LangStandard::lang_gnucxx17;
   case Language::RenderScript:
----------------
Unintended change used to find the tests that needed updating?


================
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
 
----------------
What from this test requires C++14?


================
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
 
----------------
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.)


================
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
 
----------------
What from this test is invalid in C++17?


================
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
 
----------------
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).


================
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
----------------
Same question here as above


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131464/new/

https://reviews.llvm.org/D131464