stephanemoore requested changes to this revision.
stephanemoore marked an inline comment as done.
stephanemoore added inline comments.
This revision now requires changes to proceed.
================
Comment at: clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp:112
+ Result.Nodes.getNodeAs<ObjCMessageExpr>("new_call")) {
+ // Don't warn if the call expression originates from a macro expansion.
+ if (isMessageExpressionInsideMacro(CallExpr))
----------------
mwyman wrote:
> stephanemoore wrote:
> > If the message expression is within a macro expansion, maybe we should emit
> > the diagnostic without the fixit?
> I'm leery of emitting a warning in a case where the code may not originate
> from the code in question. Maybe if the macro is defined in the same source
> file, but I think perhaps that could be a follow-up.
I think that it's reasonable to surface the diagnostic. If the user determines
that the diagnostic is not relevant in this situation then they can suppress
it. That is, I believe it's preferable to surface issues and have the user
determine relevance rather than suppress diagnostics without the knowledge of
the user.
(with that said, I do not consider this a hard blocker and am comfortable
deferring this for future consideration)
================
Comment at:
clang-tools-extra/test/clang-tidy/google-objc-avoid-nsobject-new.m:53
+
+@interface ProxyFoo : NSProxy
+@end
----------------
Shouldn't you declare `+[ProxyFoo new]` so that you can call it on line 67?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61350/new/
https://reviews.llvm.org/D61350
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
