aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land.
This LGTM but I'd appreciate it if @dblaikie could also give it a once over so we have more than one sets of eyes on the changes in case I've missed something. ================ 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]; ---------------- MaskRay wrote: > aaron.ballman wrote: > > 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)? > The proposal is probably clean if we the majority of tests work with C++98, > but I think we have accrued many tests which don't work with C++98 so we need > directives like `%stdcxx_11-`. > > Since C++98 is actually uncommon now. I prefer explicit `%stdcxx_98-` to > indicate a test works with C++98. > Since C++98 is actually uncommon now. I prefer explicit %stdcxx_98- to > indicate a test works with C++98. Heheh, there's still a *ton* of C++98 code out there, but I'm okay being explicit just the same (it makes the tests easier to read). ================ 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 ---------------- MaskRay wrote: > aaron.ballman wrote: > > Pre 14? Post 17? > Unfortunately, C++17 and C++20 have different behaviors. I haven't > investigated why it is the case. Ah, that's fine to do in a follow-up then. Thanks! ================ 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 \ ---------------- MaskRay wrote: > aaron.ballman wrote: > > Is this test specific to C++14? > This is similar to the previous -fdump-record-layouts test that the dump > order is different across 14, 17, 20. I do now know whether there is > something which should be improved to add the coverage. > > For now I assume it is not this patch's responsibility to address it. Agreed, thanks for the explanation! 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
