[PATCH] D126664: Expand definition deprecation warning to include constexpr statements.

[Luke Nihlen via Phabricator via cfe-commits]

luken-google created this revision.
Herald added a project: All.
luken-google requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Clang currently warns on definitions downgraded to declarations
with a const modifier, but not for a constexpr modifier. This patch
updates the warning logic to warn on both inputs, and adds a test to
check the additional case as well.

See also: https://bugs.chromium.org/p/chromium/issues/detail?id=1284718


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126664

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CXX/basic/basic.def/p2.cpp


Index: clang/test/CXX/basic/basic.def/p2.cpp
===================================================================
--- clang/test/CXX/basic/basic.def/p2.cpp
+++ clang/test/CXX/basic/basic.def/p2.cpp
@@ -5,4 +5,9 @@
     static constexpr int n = 0;
   };
   const int A::n; // expected-warning {{deprecated}}
+
+  struct B {
+    static constexpr int m = 0;
+  };
+  constexpr int B::m; // expected-warning {{deprecated}}
 }
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -4508,15 +4508,15 @@
   }
 
   // C++ doesn't have tentative definitions, so go right ahead and check here.
-  if (getLangOpts().CPlusPlus &&
-      New->isThisDeclarationADefinition() == VarDecl::Definition) {
+  if (getLangOpts().CPlusPlus) {
     if (Old->isStaticDataMember() && Old->getCanonicalDecl()->isInline() &&
         Old->getCanonicalDecl()->isConstexpr()) {
       // This definition won't be a definition any more once it's been merged.
       Diag(New->getLocation(),
            diag::warn_deprecated_redundant_constexpr_static_def);
-    } else if (VarDecl *Def = Old->getDefinition()) {
-      if (checkVarDeclRedefinition(Def, New))
+    } else if (New->isThisDeclarationADefinition() == VarDecl::Definition) {
+      VarDecl *Def = Old->getDefinition();
+      if (Def && checkVarDeclRedefinition(Def, New))
         return;
     }
   }



Index: clang/test/CXX/basic/basic.def/p2.cpp
===================================================================
--- clang/test/CXX/basic/basic.def/p2.cpp
+++ clang/test/CXX/basic/basic.def/p2.cpp
@@ -5,4 +5,9 @@
     static constexpr int n = 0;
   };
   const int A::n; // expected-warning {{deprecated}}
+
+  struct B {
+    static constexpr int m = 0;
+  };
+  constexpr int B::m; // expected-warning {{deprecated}}
 }
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -4508,15 +4508,15 @@
   }
 
   // C++ doesn't have tentative definitions, so go right ahead and check here.
-  if (getLangOpts().CPlusPlus &&
-      New->isThisDeclarationADefinition() == VarDecl::Definition) {
+  if (getLangOpts().CPlusPlus) {
     if (Old->isStaticDataMember() && Old->getCanonicalDecl()->isInline() &&
         Old->getCanonicalDecl()->isConstexpr()) {
       // This definition won't be a definition any more once it's been merged.
       Diag(New->getLocation(),
            diag::warn_deprecated_redundant_constexpr_static_def);
-    } else if (VarDecl *Def = Old->getDefinition()) {
-      if (checkVarDeclRedefinition(Def, New))
+    } else if (New->isThisDeclarationADefinition() == VarDecl::Definition) {
+      VarDecl *Def = Old->getDefinition();
+      if (Def && checkVarDeclRedefinition(Def, New))
         return;
     }
   }

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits