[libcxx-commits] [PATCH] D99515: [libc++] Build and test with -Wundef warning. NFC.

Marek Kurdej via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon Mar 29 12:39:56 PDT 2021


curdeius added inline comments.


================
Comment at: libcxx/include/future:505
+#if _LIBCPP_STD_VER > 14
+    // explicit future_error(future_errc _Ev) : logic_error(), __ec_(make_error_code(_Ev)) {}
 #endif
----------------
Quuxplusone wrote:
> curdeius wrote:
> > This was already dead code, so just commented it out in this patch.
> > I'll create another patch to fix this if that's wanted. However, the `future_error(error_code __ec);` gets called with arguments of type `future_errc`, so adding `future_error(future_errc)` ctor as `future_error(future_errc _Ev) : future_error(make_error_code(_Ev) {}` will not add anything.
> > We might however want to do something more standard conformant (mind that `future(error_code)` ctor is "exposition only").
> I recommend just deleting lines 504–506. That clearly preserves the existing behavior (which is not buggy AFAWCT, right?). If the existing behavior //is// buggy, then you should add a test that detects the bug.
> I would not check in commented-out code.
The existing behaviour is correct but not very strict (it accepts what standard prescribes, but also more).
Precisely:
* the ctor is not explicit
* the ctor accepts not only `future_errc` (which is indirect through `error_code`), but also `error_code`
That's seems acceptable though (because changing that will break some code depending on this).
Anyway, it's out of scope of this patch IMO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99515



More information about the libcxx-commits mailing list