[PATCH] D12047: test/SemaObjC: Remove cruft in unused getter test

Brian Gesiak via cfe-commits cfe-commits at lists.llvm.org
Sat Aug 15 05:18:07 PDT 2015


modocache added a comment.

> Why do you think it's a cruft? Seems it's a bit more verbose than it should be


The verbosity is the cruft, in my opinion. Why spend thirty lines of code to demonstrate behavior that could be demonstrated in just five?

> but what is missing in your test is inheritance, which is important.


Is it, though? I haven't read the source code completely, but there doesn't seem to be anything in there written specifically to handle inheritence as deep as the test had it. I believe the relevant code is in `Expr::isUnusedResultAWarning()`, which has case statements for `case ObjCMessageExprClass` and `case ObjCPropertyRefExprClass`. The behavior doesn't change based on where those are defined, whether on a base class or, as in this change, on a protocol.

> I think the code for the initial test was just extracted from a real project.


Yep, based on the header prefix `XC` I'm guessing it was something in Xcode. And while I appreciate the fact that it's a "real-world" example, I think it's valuable to reduce this sort of thing to the minimum amount of code necessary to demonstrate the behavior.


http://reviews.llvm.org/D12047





More information about the cfe-commits mailing list