Bug 28355

Summary: Replace MAX()/MIN() macros with type-safe std::max()/min() templates
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: mitz
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch v1 mitz: review+

David Kilzer (:ddkilzer)
Reported 2009-08-16 05:25:24 PDT
Created attachment 34925 [details] Patch v1 Reviewed by NOBODY (OOPS!). WebCore: * accessibility/mac/AccessibilityObjectWrapper.mm: (AXAttributeStringSetSpelling): Changed MIN() to min(). * platform/graphics/mac/FontMacATSUI.mm: (WebCore::Font::selectionRectForComplexText): Changed MAX() to max() and MIN() to min(). (WebCore::Font::floatWidthForComplexText): Ditto. * platform/graphics/mac/SimpleFontDataMac.mm: Added using std::max statment. (WebCore::SimpleFontData::platformInit): Changed MAX() to max(). * platform/text/mac/TextCodecMac.cpp: (WebCore::TextCodecMac::decode): Changed MIN() to min(). WebKit/mac: * Plugins/WebBaseNetscapePluginStream.mm: Added using std::min statement. (WebNetscapePluginStream::deliverData): Changed MIN() to min(). Changed C-style cast to a static_cast. * Plugins/WebNetscapePluginView.mm: Added using std::min statement. (-[WebNetscapePluginView _postURL:target:len:buf:file:notifyData:sendNotification:allowHeaders:]): Changed MIN() to min(). Changed C-style cast to a static_cast. * WebView/WebHTMLView.mm: Added using std::max statement. (-[WebHTMLView _dragImageForURL:withLabel:]): Changed MAX() to max(). (-[WebHTMLView _scaleFactorForPrintOperation:]): Ditto. * WebView/WebTextCompletionController.mm: Added using std::max and using std::min statements. (-[WebTextCompletionController _placePopupWindow:]): Changed type of maxWidth variable from float to CGFloat to prevent a type mismatch on x86_64. Changed MAX() to max() and MIN() to min(). Added static_cast for a constant value since CGFloat is defined as a float on i386 and as a double on x86_64. --- 10 files changed, 70 insertions(+), 16 deletions(-)
Attachments
Patch v1 (13.17 KB, patch)
2009-08-16 05:25 PDT, David Kilzer (:ddkilzer) 		mitz: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
mitz
Comment 1 2009-08-16 10:10:15 PDT
Comment on attachment 34925 [details] Patch v1 > + <http://webkit.org/b/00000> Replace MAX()/MIN() macros with type-safe std::max()/min() templates Presumably this will transform into this bug’s URL? > + * platform/graphics/mac/SimpleFontDataMac.mm: Added using > + std::max statment. Typo: statment. In new files, it is common to just add “using namespace std;”. > + <http://webkit.org/b/00000> Replace MAX()/MIN() macros with type-safe std::max()/min() templates URL… > + deliveryBytes = static_cast<int32>(min(static_cast<NSUInteger>(deliveryBytes), [subdata length])); This is an odd combination of casts. You can do “min<int32>(deliveryBytes, [subdata length])” instead. > - dataLength = MIN((unsigned)[contentLength intValue], dataLength); > + dataLength = min(static_cast<unsigned>([contentLength intValue]), dataLength); I prefer min<unsigned>(…). > - imageSize.width = MAX(MAX_DRAG_LABEL_WIDTH + DRAG_LABEL_BORDER_X * 2.0f, MIN_DRAG_LABEL_WIDTH_BEFORE_CLIP); > + imageSize.width = max(MAX_DRAG_LABEL_WIDTH + DRAG_LABEL_BORDER_X * 2.0f, MIN_DRAG_LABEL_WIDTH_BEFORE_CLIP); While you’re here, you may just drop the “.0f”. > - imageSize.width = MAX(labelSize.width + DRAG_LABEL_BORDER_X * 2.0f, urlStringSize.width + DRAG_LABEL_BORDER_X * 2.0f); > + imageSize.width = max(labelSize.width + DRAG_LABEL_BORDER_X * 2.0f, urlStringSize.width + DRAG_LABEL_BORDER_X * 2.0f); Ditto. > + maxWidth = min(static_cast<CGFloat>(400), windowFrame.size.width); Can be done as min<CGFloat>(…). r=me but please consider at least changing to “using namespace std;”.
David Kilzer (:ddkilzer)
Comment 2 2009-08-16 15:53:06 PDT
(In reply to comment #1) > (From update of attachment 34925 [details]) > > + <http://webkit.org/b/00000> Replace MAX()/MIN() macros with type-safe std::max()/min() templates > > Presumably this will transform into this bug’s URL? Yes. I'm currently doing it manually, but one day bugzilla-tool will do the replacement automatically. I already updated the ChangeLogs after creating this bug. > > + * platform/graphics/mac/SimpleFontDataMac.mm: Added using > > + std::max statment. > > Typo: statment. Fixed. > In new files, it is common to just add “using namespace std;”. Fixed. I filed: <http://webkit.org/b/28370>. > > + <http://webkit.org/b/00000> Replace MAX()/MIN() macros with type-safe std::max()/min() templates > > URL… Already fixed. > > + deliveryBytes = static_cast<int32>(min(static_cast<NSUInteger>(deliveryBytes), [subdata length])); > > This is an odd combination of casts. You can do “min<int32>(deliveryBytes, > [subdata length])” instead. I was keeping the original behavior of the statement. Fixed. > > - dataLength = MIN((unsigned)[contentLength intValue], dataLength); > > + dataLength = min(static_cast<unsigned>([contentLength intValue]), dataLength); > > I prefer min<unsigned>(…). Fixed. > > - imageSize.width = MAX(MAX_DRAG_LABEL_WIDTH + DRAG_LABEL_BORDER_X * 2.0f, MIN_DRAG_LABEL_WIDTH_BEFORE_CLIP); > > + imageSize.width = max(MAX_DRAG_LABEL_WIDTH + DRAG_LABEL_BORDER_X * 2.0f, MIN_DRAG_LABEL_WIDTH_BEFORE_CLIP); > > While you’re here, you may just drop the “.0f”. Fixed. > > - imageSize.width = MAX(labelSize.width + DRAG_LABEL_BORDER_X * 2.0f, urlStringSize.width + DRAG_LABEL_BORDER_X * 2.0f); > > + imageSize.width = max(labelSize.width + DRAG_LABEL_BORDER_X * 2.0f, urlStringSize.width + DRAG_LABEL_BORDER_X * 2.0f); > > Ditto. Fixed. > > + maxWidth = min(static_cast<CGFloat>(400), windowFrame.size.width); > > Can be done as min<CGFloat>(…). Fixed. > r=me but please consider at least changing to “using namespace std;”. Thanks for the review!
David Kilzer (:ddkilzer)
Comment 3 2009-08-16 15:57:58 PDT
$ git svn dcommit Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/accessibility/mac/AccessibilityObjectWrapper.mm M WebCore/platform/graphics/mac/FontMacATSUI.mm M WebCore/platform/graphics/mac/SimpleFontDataMac.mm M WebCore/platform/text/mac/TextCodecMac.cpp M WebKit/mac/ChangeLog M WebKit/mac/Plugins/WebBaseNetscapePluginStream.mm M WebKit/mac/Plugins/WebNetscapePluginView.mm M WebKit/mac/WebView/WebHTMLView.mm M WebKit/mac/WebView/WebTextCompletionController.mm Committed r47341 <http://trac.webkit.org/changeset/47341>
Note You need to log in before you can comment on or make changes to this bug.