[clang] [Clang] skip warnings for constructors marked with the [[noreturn]] attribute (PR #115558)

Oleksandr T. via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 21 00:24:27 PST 2024


https://github.com/a-tarasyuk updated https://github.com/llvm/llvm-project/pull/115558

>From f63263a1aa4873a63918649ea92352eb5cfe66eb Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Sat, 9 Nov 2024 00:41:13 +0200
Subject: [PATCH 1/4] [Clang] skip warnings for constructors marked with the
 [[noreturn]] attribute

---
 clang/docs/ReleaseNotes.rst                       |  2 ++
 clang/lib/Analysis/CFG.cpp                        | 15 ++++++++++-----
 .../CXX/dcl.dcl/dcl.attr/dcl.attr.noreturn/p1.cpp | 12 ++++++++++++
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c3424e0e6f34c9..11d0e35d12a786 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -527,6 +527,8 @@ Improvements to Clang's diagnostics
 
 - Clang now diagnoses ``[[deprecated]]`` attribute usage on local variables (#GH90073).
 
+- Clang now omits warnings for constructors marked with the ``[[noreturn]]`` attribute (#GH63009).
+
 Improvements to Clang's time-trace
 ----------------------------------
 
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index f678ac6f2ff36a..4d83f04c23060a 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -4889,16 +4889,21 @@ CFGBlock *CFGBuilder::VisitCXXFunctionalCastExpr(CXXFunctionalCastExpr *E,
   return Visit(E->getSubExpr(), asc);
 }
 
-CFGBlock *CFGBuilder::VisitCXXTemporaryObjectExpr(CXXTemporaryObjectExpr *C,
+CFGBlock *CFGBuilder::VisitCXXTemporaryObjectExpr(CXXTemporaryObjectExpr *E,
                                                   AddStmtChoice asc) {
   // If the constructor takes objects as arguments by value, we need to properly
   // construct these objects. Construction contexts we find here aren't for the
   // constructor C, they're for its arguments only.
-  findConstructionContextsForArguments(C);
+  findConstructionContextsForArguments(E);
 
-  autoCreateBlock();
-  appendConstructor(Block, C);
-  return VisitChildren(C);
+  CXXConstructorDecl *C = E->getConstructor();
+  if (C && C->isNoReturn())
+    Block = createNoReturnBlock();
+  else
+    autoCreateBlock();
+
+  appendConstructor(Block, E);
+  return VisitChildren(E);
 }
 
 CFGBlock *CFGBuilder::VisitImplicitCastExpr(ImplicitCastExpr *E,
diff --git a/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.noreturn/p1.cpp b/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.noreturn/p1.cpp
index 56920ea8e8cf20..a96224dd03e360 100644
--- a/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.noreturn/p1.cpp
+++ b/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.noreturn/p1.cpp
@@ -49,3 +49,15 @@ void check() {
   test_type(g);
   test_type(h); // expected-note {{instantiation}}
 }
+
+namespace GH63009 {
+struct S {
+  [[noreturn]] S() { throw int {}; }
+};
+
+int test_no_return_constructor() { S(); } // ok
+
+int main() {
+  test_no_return_constructor();
+}
+}

>From 99582e7e30048b07dba57d0c284acf209f53b83e Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Thu, 21 Nov 2024 01:25:43 +0200
Subject: [PATCH 2/4] suppress warnings for constructor expressions with the
 [[noreturn]] attribute

---
 clang/lib/Analysis/CFG.cpp                    | 16 ++++++++------
 .../dcl.dcl/dcl.attr/dcl.attr.noreturn/p1.cpp | 11 +++++++---
 clang/test/SemaCXX/warn-missing-noreturn.cpp  | 22 +++++++++++++++++++
 3 files changed, 39 insertions(+), 10 deletions(-)

diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 4d83f04c23060a..35cf8c49bd27eb 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -760,6 +760,12 @@ class CFGBuilder {
   void cleanupConstructionContext(Expr *E);
 
   void autoCreateBlock() { if (!Block) Block = createBlock(); }
+  void createConstructorBlock(CXXConstructorDecl *C) {
+    if (C && C->isNoReturn())
+      Block = createNoReturnBlock();
+    else
+      autoCreateBlock();
+  };
   CFGBlock *createBlock(bool add_successor = true);
   CFGBlock *createNoReturnBlock();
 
@@ -4830,7 +4836,7 @@ CFGBlock *CFGBuilder::VisitCXXConstructExpr(CXXConstructExpr *C,
   // constructor C, they're for its arguments only.
   findConstructionContextsForArguments(C);
 
-  autoCreateBlock();
+  createConstructorBlock(C->getConstructor());
   appendConstructor(Block, C);
 
   return VisitChildren(C);
@@ -4896,13 +4902,9 @@ CFGBlock *CFGBuilder::VisitCXXTemporaryObjectExpr(CXXTemporaryObjectExpr *E,
   // constructor C, they're for its arguments only.
   findConstructionContextsForArguments(E);
 
-  CXXConstructorDecl *C = E->getConstructor();
-  if (C && C->isNoReturn())
-    Block = createNoReturnBlock();
-  else
-    autoCreateBlock();
-
+  createConstructorBlock(E->getConstructor());
   appendConstructor(Block, E);
+
   return VisitChildren(E);
 }
 
diff --git a/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.noreturn/p1.cpp b/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.noreturn/p1.cpp
index a96224dd03e360..afcb133e48a1a8 100644
--- a/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.noreturn/p1.cpp
+++ b/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.noreturn/p1.cpp
@@ -51,13 +51,18 @@ void check() {
 }
 
 namespace GH63009 {
-struct S {
-  [[noreturn]] S() { throw int {}; }
+struct S1 {
+  [[noreturn]] S1() { throw int {}; }
+};
+struct S2 {
+  [[noreturn]] ~S2() { throw int {}; }
 };
 
-int test_no_return_constructor() { S(); } // ok
+int test_no_return_constructor() { S1(); } // ok
+int test_no_return_destructor() { S2(); } // ok
 
 int main() {
   test_no_return_constructor();
+  test_no_return_destructor();
 }
 }
diff --git a/clang/test/SemaCXX/warn-missing-noreturn.cpp b/clang/test/SemaCXX/warn-missing-noreturn.cpp
index 400b471600e027..32b49e0a325f26 100644
--- a/clang/test/SemaCXX/warn-missing-noreturn.cpp
+++ b/clang/test/SemaCXX/warn-missing-noreturn.cpp
@@ -122,3 +122,25 @@ namespace PR10801 {
     thingy(b);
   }
 }
+
+namespace GH63009 {
+struct S1 {
+  [[noreturn]] S1();
+};
+
+struct S2 {
+  [[noreturn]] ~S2();
+};
+
+int foo();
+
+int test_1() {
+  S1 s1;
+  foo();
+}
+
+int test_2() {
+  S2 s2;
+  foo();
+}
+}

>From f7e9401c3607d3c4892660464f605e9504d68829 Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Thu, 21 Nov 2024 01:29:17 +0200
Subject: [PATCH 3/4] update release notes

---
 clang/docs/ReleaseNotes.rst | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8834a8236cce1c..4887594af9ead7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -555,7 +555,8 @@ Improvements to Clang's diagnostics
       getS(); // Now diagnoses "Reason 2", previously diagnoses "Reason 1"
     }
 
-- Clang now omits warnings for constructors marked with the ``[[noreturn]]`` attribute (#GH63009).
+- Clang now correctly recognises code after a call to a ``[[noreturn]]`` constructor
+  as unreachable (#GH63009).
 
 Improvements to Clang's time-trace
 ----------------------------------

>From 76e4c090371f743f1b76c54a4059313b8706ffef Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Thu, 21 Nov 2024 10:24:12 +0200
Subject: [PATCH 4/4] move duplicate logic into appendConstructor

---
 clang/lib/Analysis/CFG.cpp | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index fd3cd1676276cb..304bbb2b422c61 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -760,12 +760,7 @@ class CFGBuilder {
   void cleanupConstructionContext(Expr *E);
 
   void autoCreateBlock() { if (!Block) Block = createBlock(); }
-  void createConstructorBlock(CXXConstructorDecl *C) {
-    if (C && C->isNoReturn())
-      Block = createNoReturnBlock();
-    else
-      autoCreateBlock();
-  };
+
   CFGBlock *createBlock(bool add_successor = true);
   CFGBlock *createNoReturnBlock();
 
@@ -824,15 +819,21 @@ class CFGBuilder {
     B->appendStmt(const_cast<Stmt*>(S), cfg->getBumpVectorContext());
   }
 
-  void appendConstructor(CFGBlock *B, CXXConstructExpr *CE) {
+  void appendConstructor(CXXConstructExpr *CE) {
+    CXXConstructorDecl *C = CE->getConstructor();
+    if (C && C->isNoReturn())
+      Block = createNoReturnBlock();
+    else
+      autoCreateBlock();
+
     if (const ConstructionContext *CC =
             retrieveAndCleanupConstructionContext(CE)) {
-      B->appendConstructor(CE, CC, cfg->getBumpVectorContext());
+      Block->appendConstructor(CE, CC, cfg->getBumpVectorContext());
       return;
     }
 
     // No valid construction context found. Fall back to statement.
-    B->appendStmt(CE, cfg->getBumpVectorContext());
+    Block->appendStmt(CE, cfg->getBumpVectorContext());
   }
 
   void appendCall(CFGBlock *B, CallExpr *CE) {
@@ -4838,9 +4839,7 @@ CFGBlock *CFGBuilder::VisitCXXConstructExpr(CXXConstructExpr *C,
   // construct these objects. Construction contexts we find here aren't for the
   // constructor C, they're for its arguments only.
   findConstructionContextsForArguments(C);
-
-  createConstructorBlock(C->getConstructor());
-  appendConstructor(Block, C);
+  appendConstructor(C);
 
   return VisitChildren(C);
 }
@@ -4904,9 +4903,7 @@ CFGBlock *CFGBuilder::VisitCXXTemporaryObjectExpr(CXXTemporaryObjectExpr *E,
   // construct these objects. Construction contexts we find here aren't for the
   // constructor C, they're for its arguments only.
   findConstructionContextsForArguments(E);
-
-  createConstructorBlock(E->getConstructor());
-  appendConstructor(Block, E);
+  appendConstructor(E);
 
   return VisitChildren(E);
 }



More information about the cfe-commits mailing list