bruno added a comment.
Hi Nathan, thanks for implementing this. Besides formatting nitpicks, few other
comments/questions:
- Update needed in cxx_status.html
- Should we support this as an extension for earlier C++ versions? This is a
very handy feature. In clang terms,
`warn_cxx17_compat_constexpr_body_invalid_stmt` and
`ext_constexpr_body_invalid_stmt_cxx20` would be examples for that.
> Perhaps deferring the scope check until after construction of the
> shadow-decls in the parsing case would be preferred?
Doing it as part of `BuildUsingDeclaration` seem reasonable, anything that
might not work because of that?
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:12353
+ ED = R->getAsSingle<EnumConstantDecl>();
+ else if (UD && UD->shadow_size () == 1)
+ ED = dyn_cast<EnumConstantDecl>(UD->shadow_begin()->getTargetDecl());
----------------
-> `UD->shadow_size()`
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:12370
// If we weren't able to compute a valid scope, it might validly be a
// dependent class scope or a dependent enumeration unscoped scope. If
// we have a 'typename' keyword, the scope must resolve to a class type.
----------------
Does this comment needs update?
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:12452
+ // The current scope is a record.
+
if (!NamedContext->isRecord()) {
----------------
trim off newline
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:12538
+
Decl *Sema::ActOnAliasDeclaration(Scope *S, AccessSpecifier AS,
----------------
same here.
================
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:3079
+ // recheck the inheritance
+
if (NameInfo.getName().getNameKind() == DeclarationName::CXXConstructorName)
----------------
here too
================
Comment at: clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p3.cpp:20
class C {
+public:
int g();
----------------
> The change to clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p3.cpp
> is to silence an uninteresting access error that the above change causes.
The fact that the access check changed for previous version of the language
(not necessarily related to p1099) indicates that this specific change deserves
its own patch.
================
Comment at:
clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p7-cxx20.cpp:1
+// RUN: %clang_cc1 -fsyntax-only -std=c++14 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify %s
----------------
Why not c++17?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100276/new/
https://reviews.llvm.org/D100276
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
