[PATCH] Improve -Winvalid-noreturn
Aaron Ballman
aaron at aaronballman.com
Thu May 21 13:11:22 PDT 2015
On Thu, May 21, 2015 at 4:02 PM, Richard Trieu <rtrieu at google.com> wrote:
> Ping.
One question below.
>
>
> http://reviews.llvm.org/D9454
>
> EMAIL PREFERENCES
> http://reviews.llvm.org/settings/panel/emailpreferences/
> Index: include/clang/AST/DeclCXX.h
> ===================================================================
> --- include/clang/AST/DeclCXX.h
> +++ include/clang/AST/DeclCXX.h
> @@ -2380,6 +2380,10 @@
> return cast<CXXDestructorDecl>(getFirstDecl())->OperatorDelete;
> }
>
> + // Returns true if this destructor, or any implicitly invoked destructors,
> + // are marked noreturn.
> + bool isAnyDestructorNoReturn() const;
> +
> // Implement isa/cast/dyncast/etc.
> static bool classof(const Decl *D) { return classofKind(D->getKind()); }
> static bool classofKind(Kind K) { return K == CXXDestructor; }
> Index: lib/AST/DeclCXX.cpp
> ===================================================================
> --- lib/AST/DeclCXX.cpp
> +++ lib/AST/DeclCXX.cpp
> @@ -1899,6 +1899,31 @@
> }
> }
>
> +bool CXXDestructorDecl::isAnyDestructorNoReturn() const {
> + // Destructor is noreturn
> + if (isNoReturn())
> + return true;
> +
> + // Check base classes destructor for noreturn
> + const CXXRecordDecl *Record = getParent();
> + for (const auto &Base : Record->bases())
> + if (Base.getType()
> + ->getAsCXXRecordDecl()
> + ->getDestructor()
> + ->isAnyDestructorNoReturn())
Is this safe -- getDestructor() appears to be able to return nullptr?
Other places check for null, such as
CXXMethodDecl::getCorrespondingMethodInClass()
> + return true;
> +
> + // Check fields for noreturn
> + for (const auto *Field : Record->fields())
> + if (const CXXRecordDecl *RD =
> + Field->getType()->getBaseElementTypeUnsafe()->getAsCXXRecordDecl())
> + if (RD->getDestructor()->isAnyDestructorNoReturn())
Same here, and elsewhere.
> + return true;
> +
> + // All destructors are not noreturn
> + return false;
> +}
> +
> void CXXConversionDecl::anchor() { }
>
> CXXConversionDecl *
> Index: lib/Analysis/CFG.cpp
> ===================================================================
> --- lib/Analysis/CFG.cpp
> +++ lib/Analysis/CFG.cpp
> @@ -1180,7 +1180,7 @@
> Ty = Context->getBaseElementType(Ty);
>
> const CXXDestructorDecl *Dtor = Ty->getAsCXXRecordDecl()->getDestructor();
> - if (Dtor->isNoReturn())
> + if (Dtor->isAnyDestructorNoReturn())
> Block = createNoReturnBlock();
> else
> autoCreateBlock();
> @@ -3682,7 +3682,7 @@
>
> const CXXDestructorDecl *Dtor = E->getTemporary()->getDestructor();
>
> - if (Dtor->isNoReturn()) {
> + if (Dtor->isAnyDestructorNoReturn()) {
> // If the destructor is marked as a no-return destructor, we need to
> // create a new block for the destructor which does not have as a
> // successor anything built thus far. Control won't flow out of this
> Index: test/SemaCXX/attr-noreturn.cpp
> ===================================================================
> --- test/SemaCXX/attr-noreturn.cpp
> +++ test/SemaCXX/attr-noreturn.cpp
> @@ -17,6 +17,132 @@
> }
> }
>
> +namespace destructor_tests {
> + __attribute__((noreturn)) void fail();
> +
> + struct A {
> + ~A() __attribute__((noreturn)) { fail(); }
> + };
> + struct B {
> + B() {}
> + ~B() __attribute__((noreturn)) { fail(); }
> + };
> + struct C : A {};
> + struct D : B {};
> + struct E : virtual A {};
> + struct F : A, virtual B {};
> + struct G : E {};
> + struct H : virtual D {};
> + struct I : A {};
> + struct J : I {};
> + struct K : virtual A {};
> + struct L : K {};
> + struct M : virtual C {};
> + struct N : M {};
> + struct O { N n; };
> +
> + __attribute__((noreturn)) void test_1() { A a; }
> + __attribute__((noreturn)) void test_2() { B b; }
> + __attribute__((noreturn)) void test_3() { C c; }
> + __attribute__((noreturn)) void test_4() { D d; }
> + __attribute__((noreturn)) void test_5() { E e; }
> + __attribute__((noreturn)) void test_6() { F f; }
> + __attribute__((noreturn)) void test_7() { G g; }
> + __attribute__((noreturn)) void test_8() { H h; }
> + __attribute__((noreturn)) void test_9() { I i; }
> + __attribute__((noreturn)) void test_10() { J j; }
> + __attribute__((noreturn)) void test_11() { K k; }
> + __attribute__((noreturn)) void test_12() { L l; }
> + __attribute__((noreturn)) void test_13() { M m; }
> + __attribute__((noreturn)) void test_14() { N n; }
> + __attribute__((noreturn)) void test_15() { O o; }
> +
> + __attribute__((noreturn)) void test_16() { const A& a = A(); }
> + __attribute__((noreturn)) void test_17() { const B& b = B(); }
> + __attribute__((noreturn)) void test_18() { const C& c = C(); }
> + __attribute__((noreturn)) void test_19() { const D& d = D(); }
> + __attribute__((noreturn)) void test_20() { const E& e = E(); }
> + __attribute__((noreturn)) void test_21() { const F& f = F(); }
> + __attribute__((noreturn)) void test_22() { const G& g = G(); }
> + __attribute__((noreturn)) void test_23() { const H& h = H(); }
> + __attribute__((noreturn)) void test_24() { const I& i = I(); }
> + __attribute__((noreturn)) void test_25() { const J& j = J(); }
> + __attribute__((noreturn)) void test_26() { const K& k = K(); }
> + __attribute__((noreturn)) void test_27() { const L& l = L(); }
> + __attribute__((noreturn)) void test_28() { const M& m = M(); }
> + __attribute__((noreturn)) void test_29() { const N& n = N(); }
> + __attribute__((noreturn)) void test_30() { const O& o = O(); }
> +
> + struct AA {};
> + struct BB { BB() {} ~BB() {} };
> + struct CC : AA {};
> + struct DD : BB {};
> + struct EE : virtual AA {};
> + struct FF : AA, virtual BB {};
> + struct GG : EE {};
> + struct HH : virtual DD {};
> + struct II : AA {};
> + struct JJ : II {};
> + struct KK : virtual AA {};
> + struct LL : KK {};
> + struct MM : virtual CC {};
> + struct NN : MM {};
> + struct OO { NN n; };
> +
> + __attribute__((noreturn)) void test_31() {
> + AA a;
> + BB b;
> + CC c;
> + DD d;
> + EE e;
> + FF f;
> + GG g;
> + HH h;
> + II i;
> + JJ j;
> + KK k;
> + LL l;
> + MM m;
> + NN n;
> + OO o;
> +
> + const AA& aa = AA();
> + const BB& bb = BB();
> + const CC& cc = CC();
> + const DD& dd = DD();
> + const EE& ee = EE();
> + const FF& ff = FF();
> + const GG& gg = GG();
> + const HH& hh = HH();
> + const II& ii = II();
> + const JJ& jj = JJ();
> + const KK& kk = KK();
> + const LL& ll = LL();
> + const MM& mm = MM();
> + const NN& nn = NN();
> + const OO& oo = OO();
> + } // expected-warning {{function declared 'noreturn' should not return}}
> +
> + struct P {
> + ~P() __attribute__((noreturn)) { fail(); }
> + void foo() {}
> + };
> + struct Q : P { };
> + __attribute__((noreturn)) void test31() {
> + P().foo();
> + }
> + __attribute__((noreturn)) void test32() {
> + Q().foo();
> + }
> +
> + struct R {
> + A a[5];
> + };
> + __attribute__((noreturn)) void test33() {
> + R r;
> + }
> +}
> +
> // PR5620
> void f0() __attribute__((__noreturn__));
> void f1(void (*)());
>
~Aaron
More information about the cfe-commits
mailing list