[PATCH] Improve -Winvalid-noreturn

[Aaron Ballman]

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