[clang-tools-extra] r255758 - [clang-tidy] Fix a crash in misc-new-delete-overloads

[Aaron Ballman via cfe-commits]

On Wed, Dec 16, 2015 at 5:58 AM, Alexander Kornienko via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
> Author: alexfh
> Date: Wed Dec 16 04:58:14 2015
> New Revision: 255758
>
> URL: http://llvm.org/viewvc/llvm-project?rev=255758&view=rev
> Log:
> [clang-tidy] Fix a crash in misc-new-delete-overloads

Good catch, thanks!

~Aaron

>
> Modified:
>     clang-tools-extra/trunk/clang-tidy/misc/NewDeleteOverloadsCheck.cpp
>     clang-tools-extra/trunk/test/clang-tidy/misc-new-delete-overloads.cpp
>
> Modified: clang-tools-extra/trunk/clang-tidy/misc/NewDeleteOverloadsCheck.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/NewDeleteOverloadsCheck.cpp?rev=255758&r1=255757&r2=255758&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/misc/NewDeleteOverloadsCheck.cpp (original)
> +++ clang-tools-extra/trunk/clang-tidy/misc/NewDeleteOverloadsCheck.cpp Wed Dec 16 04:58:14 2015
> @@ -98,32 +98,30 @@ bool areCorrespondingOverloads(const Fun
>    return RHS->getOverloadedOperator() == getCorrespondingOverload(LHS);
>  }
>
> -bool hasCorrespondingOverloadInOneClass(const CXXRecordDecl *RD,
> -                                        const CXXMethodDecl *MD) {
> -  // Check the methods in the given class and accessible to derived classes.
> -  for (const auto *BMD : RD->methods())
> -    if (BMD->isOverloadedOperator() && BMD->getAccess() != AS_private &&
> -        areCorrespondingOverloads(MD, BMD))
> -      return true;
> -
> -  // Check base classes.
> -  for (const auto &BS : RD->bases())
> -    if (hasCorrespondingOverloadInOneClass(BS.getType()->getAsCXXRecordDecl(),
> -                                           MD))
> -      return true;
> +bool hasCorrespondingOverloadInBaseClass(const CXXMethodDecl *MD,
> +                                         const CXXRecordDecl *RD = nullptr) {
> +  if (RD) {
> +    // Check the methods in the given class and accessible to derived classes.
> +    for (const auto *BMD : RD->methods())
> +      if (BMD->isOverloadedOperator() && BMD->getAccess() != AS_private &&
> +          areCorrespondingOverloads(MD, BMD))
> +        return true;
> +  } else {
> +    // Get the parent class of the method; we do not need to care about checking
> +    // the methods in this class as the caller has already done that by looking
> +    // at the declaration contexts.
> +    RD = MD->getParent();
> +  }
>
> -  return false;
> -}
> -bool hasCorrespondingOverloadInBaseClass(const CXXMethodDecl *MD) {
> -  // Get the parent class of the method; we do not need to care about checking
> -  // the methods in this class as the caller has already done that by looking
> -  // at the declaration contexts.
> -  const CXXRecordDecl *RD = MD->getParent();
> -
> -  for (const auto &BS : RD->bases())
> -    if (hasCorrespondingOverloadInOneClass(BS.getType()->getAsCXXRecordDecl(),
> -                                           MD))
> +  for (const auto &BS : RD->bases()) {
> +    // We can't say much about a dependent base class, but to avoid false
> +    // positives assume it can have a corresponding overload.
> +    if (BS.getType()->isDependentType())
>        return true;
> +    if (const auto *BaseRD = BS.getType()->getAsCXXRecordDecl())
> +      if (hasCorrespondingOverloadInBaseClass(MD, BaseRD))
> +        return true;
> +  }
>
>    return false;
>  }
> @@ -174,24 +172,23 @@ void NewDeleteOverloadsCheck::onEndOfTra
>      // to shard the overloads by declaration context to reduce the algorithmic
>      // complexity when searching for corresponding free store functions.
>      for (const auto *Overload : RP.second) {
> -      const auto *Match = std::find_if(
> -          RP.second.begin(), RP.second.end(), [&](const FunctionDecl *FD) {
> -            if (FD == Overload)
> -              return false;
> -            // If the declaration contexts don't match, we don't
> -            // need to check
> -            // any further.
> -            if (FD->getDeclContext() != Overload->getDeclContext())
> -              return false;
> -
> -            // Since the declaration contexts match, see whether
> -            // the current
> -            // element is the corresponding operator.
> -            if (!areCorrespondingOverloads(Overload, FD))
> -              return false;
> +      const auto *Match =
> +          std::find_if(RP.second.begin(), RP.second.end(),
> +                       [&Overload](const FunctionDecl *FD) {
> +                         if (FD == Overload)
> +                           return false;
> +                         // If the declaration contexts don't match, we don't
> +                         // need to check any further.
> +                         if (FD->getDeclContext() != Overload->getDeclContext())
> +                           return false;
> +
> +                         // Since the declaration contexts match, see whether
> +                         // the current element is the corresponding operator.
> +                         if (!areCorrespondingOverloads(Overload, FD))
> +                           return false;
>
> -            return true;
> -          });
> +                         return true;
> +                       });
>
>        if (Match == RP.second.end()) {
>          // Check to see if there is a corresponding overload in a base class
>
> Modified: clang-tools-extra/trunk/test/clang-tidy/misc-new-delete-overloads.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-new-delete-overloads.cpp?rev=255758&r1=255757&r2=255758&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/test/clang-tidy/misc-new-delete-overloads.cpp (original)
> +++ clang-tools-extra/trunk/test/clang-tidy/misc-new-delete-overloads.cpp Wed Dec 16 04:58:14 2015
> @@ -75,3 +75,7 @@ struct H : G {
>    // CHECK-MESSAGES: :[[@LINE+1]]:9: warning: declaration of 'operator new' has no matching declaration of 'operator delete' at the same scope
>    void *operator new(size_t) noexcept; // base class operator is inaccessible
>  };
> +
> +template <typename Base> struct Derived : Base {
> +  void operator delete(void *);
> +};
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits