rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land.
Only minor comments remain (other than the `-Wc++17-compat` warning). In the interest of incremental progress, let's leave the `-Wc++17-compat` warning for a later patch; feel free to commit this after fixing up the other things. (If you don't have commit access yet, now would be a good time to request it; please read http://llvm.org/docs/DeveloperPolicy.html for details.) Thanks again! ================ Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:1598 +def err_consteval_non_function : Error< + "'virtual' can only appear on functions and constructors">; def err_virtual_non_function : Error< ---------------- Should this say `consteval` rather than `virtual`? Actually... looks like this new diagnostic is not used, and could just be removed? ================ Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7860 + "defaulted declaration of %sub{select_special_member_kind}0 " + "cannot be consteval because implicit definition is not be constexpr">; def warn_defaulted_method_deleted : Warning< ---------------- Remove stray "be" here. ================ Comment at: clang/lib/AST/ExprConstant.cpp:4561 // Can we evaluate this function call? - if (Definition && Definition->isConstexpr() && Body) + if (Definition && Definition->isConstexpr() && !Definition->isInvalidDecl() && + Body) ---------------- The `Definition && [...] Definition->isInvalidDecl()` case was handled a few lines above; this change appears to have no effect. ================ Comment at: clang/lib/Parse/ParseExprCXX.cpp:1152-1154 P.Diag(ConstexprLoc, !P.getLangOpts().CPlusPlus17 ? diag::ext_constexpr_on_lambda_cxx17 : diag::warn_cxx14_compat_constexpr_on_lambda); ---------------- Tyker wrote: > rsmith wrote: > > We should produce a `-Wc++17-compat` diagnostic similar to this for uses of > > `consteval`. > a consteval keyword can only be lexed when we are in C++2a because it is a > C++2a keyword so the warning would never fire. The purpose of this warning is to warn about code parsed in C++2a mode that would not be valid in C++17. Try (eg) enabling `-Wc++98-compat` and building some C++11 code to see what's supposed to happen. ================ Comment at: clang/lib/Sema/DeclSpec.cpp:1296-1297 << (TypeSpecType == TST_char16 ? "char16_t" : "char32_t"); - if (Constexpr_specified) + if (hasConstexprSpecifier()) S.Diag(ConstexprLoc, diag::warn_cxx98_compat_constexpr); ---------------- Tyker wrote: > rsmith wrote: > > For `consteval` we should produce an "incompatible with C++ standards > > before C++2a" diagnostic, not an "incompatible with C++98" diagnostic. > same as previous comment. the consteval keyword cannot be lexed unless we are > in c++2a mode so the warning would never fire. (See previous reply.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61790/new/ https://reviews.llvm.org/D61790 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits