[PATCH] D100276: [clang] p1099 using enum part 1

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

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