ChuanqiXu added inline comments.
================ Comment at: clang/test/SemaCXX/coroutines-exp-namespace.cpp:2 +// This file is the same with coroutines.cpp except the coroutine components are defined in std::experimental namespace. +// This intention of this test is to make sure the legacy imeplementation in std::experimental namespace could work. +// TODO: Remove this test once we didn't support ---------------- ChuanqiXu wrote: > Quuxplusone wrote: > > ldionne wrote: > > > > > ...and not just "could work," but "works." :) > > ``` > > // This file is the same as coroutines.cpp, except the components are > > defined in namespace std::experimental. > > // The intent of this test is to make sure the std::experimental > > implementation still works. > > // TODO: Remove this test once we drop support for <experimental/coroutine>. > > ``` > > > > Also, it occurs to me that you should probably be testing both > > `<coroutine>` and `<experimental/coroutine>` in all the other tests, as > > well; e.g. `coroutine_handle-address-return-type.cpp` should have a > > matching `coroutine_handle-address-return-type-exp-namespace.cpp` and so > > on. Otherwise, aren't you removing a lot of regression tests for > > `<experimental/coroutine>`, despite that we still claim to support > > `<experimental/coroutine>` in Clang 14.x? > > > > In many cases, it should be possible to do something like > > ``` > > // RUN: %clang_cc1 -std=c++14 -fcoroutines-ts -fsyntax-only -verify %s > > -DINCLUDE_EXPERIMENTAL=1 > > // RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s > > -DINCLUDE_EXPERIMENTAL=0 > > > > #if INCLUDE_EXPERIMENTAL > > #include <experimental/coroutine> > > namespace coro = std::experimental; > > #else > > #include <coroutine> > > namespace coro = std; > > #endif > > > > [...] > > ``` > Thanks for reporting this. I would try to test both by duplicating the tests. > The method to use macro may not be good due to it needs to mangled. (Ignore this) I just noticed that we could use the macro based method for Sema tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108696/new/ https://reviews.llvm.org/D108696 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
