alexfh added inline comments.

================
Comment at: clang-tidy/misc/NewDeleteOverloadsCheck.cpp:170
@@ +169,3 @@
+    const auto &OI = std::find_if(
+        Overloads.begin(), Overloads.end(), [&](const FunctionDecl *FD) {
+          if (FD == O)
----------------
aaron.ballman wrote:
> alexfh wrote:
> > aaron.ballman wrote:
> > > alexfh wrote:
> > > > I just noticed that this will be an O(N^2) from all new/delete 
> > > > overloads in all classes in a TU. This should probably be not much 
> > > > usually, but I can imagine a corner-case, where this is going to be 
> > > > slooow. How about sharding these by the enclosing record declaration?
> > > Yes, the O(N^2) is unfortunate, sorry for not calling that out 
> > > explicitly. I figured that N should be incredibly minimal, however 
> > > (especially since we only care about *written* overloads that are not 
> > > placement overloads). So realistically, the maximum that N can be here is 
> > > 6: operator new(), operator new[](), operator delete(), operator 
> > > delete[](), and sized operator delete()/operator delete[](). I figured 
> > > that this wasn't worth complicating the code over since N is bounded.
> > > 
> > > But I suppose the worry is if you have these operators defined in a a lot 
> > > of classes in the same TU? In that case, I suppose I could replace 
> > > SmallVector<FunctionDecl *> Overloads with MapVector<CXXRecordDecl *, 
> > > FunctionDecl *> Overloads?
> > > But I suppose the worry is if you have these operators defined in a a lot 
> > > of 
> > > classes in the same TU? In that case, I suppose I could replace 
> > > SmallVector<FunctionDecl *> Overloads with MapVector<CXXRecordDecl 
> > > *, FunctionDecl *> Overloads?
> > 
> > Yes, this is what I meant. Though I didn't know about `MapVector<>` before 
> > you told me ;)
> Er, not MapVector, but perhaps a std::map<CXXRecordDecl *, 
> std::vector<FunctionDecl *>>, I guess.
Might be a `DenseMap<CXXRecordDecl *, SmallVector<FunctionDecl *, 6>>` as well. 
Or `std::map<..., SmallVector<,>>`.


http://reviews.llvm.org/D13071



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to