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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
