rdar://85917212
Created attachment 445911 [details] Patch
Comment on attachment 445911 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445911&action=review r=me > Tools/TestWebKitAPI/Tests/WebKitCocoa/ImageAnalysisTests.mm:87 > +static CGPoint gSwizzledLocationInView = CGPoint { 100, 100 }; > static CGPoint swizzledLocationInView(id, SEL, UIView *) > { > - return CGPointMake(100, 100); > + return gSwizzledLocationInView; > } Nit: The most efficient thing to do here is "return CGPoint { 100, 100 }". On all modern hardware, that will just fill two registers with constants and return. Maybe that's the best thing to do. If we do have a specific motivation to use a constant, I'd suggest putting the static inside the function and marking it const, to reduce its scope / potential for incorrect usage.
(In reply to Geoffrey Garen from comment #2) > Comment on attachment 445911 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=445911&action=review > > r=me > > > Tools/TestWebKitAPI/Tests/WebKitCocoa/ImageAnalysisTests.mm:87 > > +static CGPoint gSwizzledLocationInView = CGPoint { 100, 100 }; > > static CGPoint swizzledLocationInView(id, SEL, UIView *) > > { > > - return CGPointMake(100, 100); > > + return gSwizzledLocationInView; > > } > > Nit: > > The most efficient thing to do here is "return CGPoint { 100, 100 }". On all > modern hardware, that will just fill two registers with constants and > return. Maybe that's the best thing to do. > > If we do have a specific motivation to use a constant, I'd suggest putting > the static inside the function and marking it const, to reduce its scope / > potential for incorrect usage. Ah, so I pulled this out into a variable because I needed to be able to change this to another value (500, 500) for the new test below.
Comment on attachment 445911 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445911&action=review >>> Tools/TestWebKitAPI/Tests/WebKitCocoa/ImageAnalysisTests.mm:87 >>> } >> >> Nit: >> >> The most efficient thing to do here is "return CGPoint { 100, 100 }". On all modern hardware, that will just fill two registers with constants and return. Maybe that's the best thing to do. >> >> If we do have a specific motivation to use a constant, I'd suggest putting the static inside the function and marking it const, to reduce its scope / potential for incorrect usage. > > Ah, so I pulled this out into a variable because I needed to be able to change this to another value (500, 500) for the new test below. Oh, lol, I totally missed that! Seems fine as-is.
Comment on attachment 445911 [details] Patch Thanks for the review!
Committed r286522 (244856@main): <https://commits.webkit.org/244856@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 445911 [details].