Bug 28355 – Replace MAX()/MIN() macros with type-safe std::max()/min() templates

• Component: Platform
• Status: RESOLVED FIXED

[David Kilzer (:ddkilzer)]

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(-)

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)]

(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)]

$ 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>