test/SemaObjC: Remove cruft in unused getter test

[modocache]

Summary

The tests that verify that accessing a property without using the result
emits a warning were needlessly complicated. Remove several layers of
abstraction to make the tests much simpler to read and reason about.

[AlexDenisov]

Why do you think it's a cruft? Seems it's a bit more verbose than it should be, but what is missing in your test is inheritance, which is important.

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

[modocache]

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.

[AlexDenisov]

s/which is important/which is might be important'/
;)

IMHO, the purpose of the test is not just prove that functionality is working as expected, but also prevent from regression, I completely agree that this test can be cleaned up a bit, but I'd cover at least two cases: a property of a class itself and a property of a superclass.
Though, it's better to ask somebody with a proper understanding of internals :)

[rjmccall]

Sure, fine to me.

[AlexDenisov]

Committed, r245731.

test/SemaObjC/access-property-getter.m

 // RUN: %clang_cc1 -verify %s
 
-@protocol NSObject
-- (oneway void)release;
+@protocol Protocol
+- (oneway void) method;
 @end
 
-@protocol XCOutputStreams <NSObject>
-@end
-
-
-@interface XCWorkQueueCommandInvocation 
-{
-    id <XCOutputStreams> _outputStream;
-}
-@end
-
-@interface XCWorkQueueCommandSubprocessInvocation : XCWorkQueueCommandInvocation
-@end
-
-@interface XCWorkQueueCommandLocalSubprocessInvocation : XCWorkQueueCommandSubprocessInvocation
-@end
-
-@interface XCWorkQueueCommandDistributedSubprocessInvocation : XCWorkQueueCommandSubprocessInvocation
-@end
-
-@interface XCWorkQueueCommandCacheFetchInvocation : XCWorkQueueCommandSubprocessInvocation
-
-@end
-
-@implementation XCWorkQueueCommandCacheFetchInvocation
-- (id)harvestPredictivelyProcessedOutputFiles
-{
-     _outputStream.release; // expected-warning {{property access result unused - getters should not be used for side effects}}
-     return 0;
+void accessMethodViaPropertySyntaxAndTriggerWarning(id<Protocol> object) {
+    object.method; // expected-warning {{property access result unused - getters should not be used for side effects}}
 }
-@end
 
 // rdar://19137815
 #pragma clang diagnostic ignored "-Wunused-getter-return-value"
 
-@interface NSObject @end
-
-@interface I : NSObject
-@property (copy) id window;
-@end
-
-@implementation I
-- (void) Meth {
-  [self window];
-  self.window;
+void accessMethodViaPropertySyntaxWhenWarningIsIgnoredDoesNotTriggerWarning(id<Protocol> object) {
+    object.method;
 }
-@end