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

Jun Zhang via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 9 16:08:49 PDT 2022


junaire added inline comments.


================
Comment at: clang/test/AST/ast-dump-openmp-begin-declare-variant_11.c:2
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -verify=c_mode   -ast-dump %s       | FileCheck %s --check-prefix=C
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -verify=cxx_mode -ast-dump %s -x c++| FileCheck %s --check-prefix=CXX
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -verify=cxx_mode -ast-dump %s -x c++ -std=c++14 | FileCheck %s --check-prefix=CXX
 
----------------
aaron.ballman wrote:
> MaskRay wrote:
> > aaron.ballman wrote:
> > > I'm not really keen on this sort of change because it loses test coverage for other language standard versions. We usually try to write our tests to be standard version agnostic and only specify a specific language mode only when absolutely necessary, which doesn't seem to be the case for a lot of the tests in this patch (like this one).
> > I think we can identify such issues (tests which want to test the latest mature standard) and fix them post-transition. This way the transition patch feels more isolated and can more easily be reverted.
> > 
> > Not sure whether @junaire wants to work on this...
> > I think we can identify such issues (tests which want to test the latest mature standard) and fix them post-transition. This way the transition patch feels more isolated and can more easily be reverted.
> 
> That feels backwards to me, but maybe I'm misunderstanding. If there are tests that are specifically testing C++14 behavior (that did not carry forward into C++17 or later) but don't have a language standard specified on the RUN line, I think we should fix those *before* transitioning the default language standard version because those are NFC fixes that improve the test clarity even if we never make the transition. The transition patch should remain isolated and easily revertible with that approach, but there's less risk that nobody goes back and fixes the tests post-transition.
> Not sure whether @junaire wants to work on this...
I don't think I have enough knowledge and experience to do this work, So I would like to abandon my previous patch to hope you can pick it up!


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