rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land.
In D113517#3121455 <https://reviews.llvm.org/D113517#3121455>, @jyknight wrote: > This change allows those future optimizations to apply to throw() as well, in > C++17 mode, which is the desirable outcome. I see. It seems inconsistent that `throw(X)` would still call `unexpected` but that `throw()` would call `terminate`, but I suppose in the `throw()` case there is really not much interesting that an `unexpected_handler` can do other than (take some logging action and) terminate the program -- in particular, it can't do exception translation. So maybe the inconsistency is not a big deal, and it's more important to get the better code generation, especially given how rare `throw(X)` is compared to `throw()`. OK, I think I'm convinced that this is the best direction. ================ Comment at: clang/lib/CodeGen/CGException.cpp:480-487 + // In C++17 and later, 'throw()' aka EST_DynamicNone is treated the same way + // as noexcept. In earlier standards, it is handled separately, below. + if ((getLangOpts().CPlusPlus17 || EST != EST_DynamicNone) && + Proto->canThrow() == CT_Cannot) { // noexcept functions are simple terminate scopes. if (!getLangOpts().EHAsynch) // -EHa: HW exception still can occur EHStack.pushTerminate(); ---------------- Maybe the logic would be clearer if we swap the terminate and unexpected cases: ``` if (EST == EST_Dynamic || (!getLangOpts().CPlusPlus17 && EST == EST_DynamicNone)) { // Prepare to call unexpected } else if (Proto->canThrow() == CT_Cannot) { // Prepare to call terminate } ``` This would keep the syntactic checks of `EST` separated from the semantic checks of `canThrow`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113517/new/ https://reviews.llvm.org/D113517 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
