alexfh added inline comments.
================
Comment at: clang-tidy/misc/NewDeleteOverloadsCheck.cpp:168
@@ +167,3 @@
+ SmallVector<const FunctionDecl *, 4> Diagnose;
+ for (const auto *O : Overloads) {
+ const auto &OI = std::find_if(
----------------
aaron.ballman wrote:
> alexfh wrote:
> > Please don't use "O", "l", "I" as variable names.
> I thought this was the correct style for identifiers that do not require
> descriptive names (we use it *everywhere* in Clang)? I'm not opposed, but I
> am wondering if clang-tidy has different style guides?
I'm not opposed to single-character identifiers, as long as they don't use
characters that are indistinguishable from some other characters in some fonts.
E.g. I don't want ever to be confused about `map[O]` vs `map[0]` (same for "l",
"I", and sometimes even "1").
================
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:
> > 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 ;)
http://reviews.llvm.org/D13071
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
