On Mon, Sep 28, 2015 at 12:36 PM, Alexander Kornienko via cfe-commits
<cfe-commits@lists.llvm.org> wrote:
> 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<,>>`.
I wound up going with std::map and llvm::SmallVector. The values seem
too expensive for a DenseMap, but the lists should definitely be
small.
~Aaron
>
>
> http://reviews.llvm.org/D13071
>
>
>
> _______________________________________________
> 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
