Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(22)

Issue 674003: NSLocalizedString compatibility (10.8 SDK and clang trunk -Wformat-extra-args) (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years ago by Mark Mentovai
Modified:
10 years, 12 months ago
Reviewers:
mochalatte, thakis
CC:
google-breakpad-dev_googlegroups.com
Base URL:
http://google-breakpad.googlecode.com/svn/trunk/
Visibility:
Public.

Description

NSLocalizedString compatibility (10.8 SDK and clang trunk -Wformat-extra-args)

Apparently, as of the 10.8 SDK, Apple has quietly decided that the first
argument to NSLocalizedString is supposed to be usable as-is as a format
string, instead of simply being the key to obtain a usable format string.
The recent clang trunk enforces this, resulting in build breaks like

crash_report_sender.m:560:14: error: data argument not used by format string
[-Werror,-Wformat-extra-args]
             displayName];
             ^

Breaking the result of NSLocalizedString into a temporary NSString* is enough
to suppress the warning.

BUG=chromium:314109
R=thakis@chromium.org

Committed: https://code.google.com/p/google-breakpad/source/detail?r=1230

Patch Set 1 #

Total comments: 2

Patch Set 2 : #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
M src/client/mac/sender/crash_report_sender.m View 1 2 chunks +12 lines, -12 lines 0 comments Download

Messages

Total messages: 4
Mark Mentovai
https://breakpad.appspot.com/674003/diff/1/src/client/mac/sender/crash_report_sender.m File src/client/mac/sender/crash_report_sender.m (right): https://breakpad.appspot.com/674003/diff/1/src/client/mac/sender/crash_report_sender.m#newcode430 src/client/mac/sender/crash_report_sender.m:430: NSString *formatString; See, there’s precedent for doing it this ...
11 years ago #1
thakis
lgtm, thanks! https://breakpad.appspot.com/674003/diff/1/src/client/mac/sender/crash_report_sender.m File src/client/mac/sender/crash_report_sender.m (right): https://breakpad.appspot.com/674003/diff/1/src/client/mac/sender/crash_report_sender.m#newcode557 src/client/mac/sender/crash_report_sender.m:557: if ([self isOnDemand]) { Maybe add // ...
11 years ago #2
Mark Mentovai
Sure.
11 years ago #3
Mark Mentovai
10 years, 12 months ago #4
Message was sent while issue was closed.
Committed patchset #2 manually as r1230 (presubmit successful).
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1004:630ec63f810e-tainted