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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 29 08:07:45 PDT 2022


aaron.ballman added a comment.

I like the direction this is going; I ran into some questions on the tests about whether we should use a range or not and other small things, but I think this is getting close.



================
Comment at: clang/test/CXX/temp/temp.res/temp.local/p3.cpp:2
+// RUN: %clang_cc1 -verify=expected,precxx17 %stdcxx_98-14 %s
+// RUN: %clang_cc1 -verify=expected,cxx17 -std=c++17 %s
 
----------------
Should this be C++17+?


================
Comment at: clang/test/CodeGen/typedef_alignment_mismatch_warning.cpp:2
+// RUN: %clang_cc1 %s -fsyntax-only -verify=expected,precxx17 %stdcxx_11-14 -fdata-sections -fcolor-diagnostics
+// RUN: %clang_cc1 %s -fsyntax-only -verify -std=c++17 -fdata-sections -fcolor-diagnostics
 
----------------
Same question here about C++17+?


================
Comment at: clang/test/CodeGenCXX/exception-spec-decay.cpp:1
-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions %s -triple=i686-unknown-linux -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %stdcxx_98- -fcxx-exceptions -fexceptions -Wno-dynamic-exception-spec %s -triple=i686-unknown-linux -emit-llvm -o - | FileCheck %s
 typedef int Array[10];
----------------
Should we drop the `%stdcxx_98-` entirely from tests and not have any `-std` flag (e.g., no such flags tells lit to run the test in all language modes, eventually)?


================
Comment at: clang/test/CodeGenCXX/override-bit-field-layout.cpp:2
+// RUN: %clang_cc1 %stdcxx_98-14 -w -triple=x86_64-pc-win32 -fms-compatibility -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-bit-field-layout.layout %s | FileCheck %s --check-prefixes=CHECK,PRE17
+// RUN: %clang_cc1 -std=c++17 -w -triple=x86_64-pc-win32 -fms-compatibility -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-bit-field-layout.layout %s | FileCheck %s --check-prefixes=CHECK,CXX17
 
----------------
17+?


================
Comment at: clang/test/CodeGenCXX/override-layout.cpp:1-9
+// RUN: %clang_cc1 -std=c++14 -w -fdump-record-layouts-simple %s > %t.layouts
+// RUN: %clang_cc1 -std=c++14 -w -fdump-record-layouts-simple %s > %t.before
+// RUN: %clang_cc1 -std=c++14 -w -DPACKED= -DALIGNED16= -fdump-record-layouts-simple -foverride-record-layout=%t.layouts %s > %t.after
 // RUN: diff -u %t.before %t.after
-// RUN: FileCheck %s < %t.after
+// RUN: FileCheck --check-prefixes=CHECK,PRE17 %s < %t.after
+
+// RUN: %clang_cc1 -std=c++17 -w -fdump-record-layouts-simple %s > %t.layouts
----------------
Pre 14? Post 17?


================
Comment at: clang/test/Layout/ms-x86-vtordisp.cpp:1-3
+// RUN: %clang_cc1 -std=c++14 -fno-rtti -fms-extensions -emit-llvm-only -triple i686-pc-win32 -fdump-record-layouts -fsyntax-only %s 2>&1 \
 // RUN:            | FileCheck %s
+// RUN: %clang_cc1 -std=c++14 -fno-rtti -fms-extensions -emit-llvm-only -triple x86_64-pc-win32 -fdump-record-layouts -fsyntax-only %s 2>/dev/null \
----------------
Is this test specific to C++14?


================
Comment at: clang/test/Parser/cxx-casting.cpp:1-3
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++14 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
----------------
LG given that lit doesn't run multiple tests  yet, so switching to a range would lose coverage.


================
Comment at: clang/test/SemaCXX/libstdcxx_is_pod_hack.cpp:1
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++14 %s
 
----------------
Is this one only for C++14 or should there be a range used instead?


================
Comment at: llvm/utils/lit/lit/llvm/config.py:573
+            if t.endswith('-'):
+                t += '2b'
+            l = clang_std_values.index(t[0:2])
----------------
aaron.ballman wrote:
> Maybe we can use `clang_std_values[-1]` so we don't have to hardcode this?
Thoughts?


================
Comment at: llvm/utils/lit/lit/llvm/config.py:579
+            l = h - clang_std_group % (h-l+1)
+            self.config.substitutions.append((s, '-std=c++' + clang_std_values[l]))
+
----------------
MaskRay wrote:
> aaron.ballman wrote:
> > One thing we should consider is whether we want to run in *all* the specified language modes instead of just the newest mode. This will make running tests slower because we'll run significantly more of them, and it might get awkward if a lot of tests change behavior in the different language modes, so I don't suggest it as part of this patch.
> This is difficult in lit. Will answer in my main comment.
It's unfortunate that it's difficult in lit. I'm fine punting on that work for now, but I think we should try to invest in it (or are you saying "difficult" as in "not worth the effort"?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131464



More information about the cfe-commits mailing list