aaron.ballman added a comment.
Thank you for the review!
================
Comment at: clang-tidy/misc/NewDeleteOverloadsCheck.cpp:66
@@ +65,3 @@
+namespace {
+OverloadedOperatorKind GetCorrespondingOverload(const FunctionDecl *FD) {
+ switch (FD->getOverloadedOperator()) {
----------------
alexfh wrote:
> http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
>
> > Function names [...] should be camel case, and start with a lower case
> > letter (e.g. openFile() or isFoo()).
Ugh, I always get this backwards because of other parts of the code base that
didn't follow the convention. I'll fix.
================
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(
----------------
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?
================
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)
----------------
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?
================
Comment at: test/clang-tidy/misc-new-delete-overloads-sized-dealloc.cpp:6
@@ +5,3 @@
+struct S {
+ // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: declaration of 'operator delete'
has no matching declaration of 'operator new' at the same scope
+ void operator delete(void *ptr, size_t) noexcept; // not a placement delete
----------------
alexfh wrote:
> nit: Let's include the check name in brackets to the check pattern once.
Can do!
http://reviews.llvm.org/D13071
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
