On Wed, Dec 16, 2015 at 5:58 AM, Alexander Kornienko via cfe-commits <cfe-commits@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@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
