Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(37)

Issue 565383003: Fix Cocoa accessibility callbacks type mismatch (Closed)

Created:
6 years, 3 months ago by Jiang Jiang
Modified:
6 years, 3 months ago
Reviewers:
Avi (use Gerrit)
CC:
chromium-reviews, plundblad+watch_chromium.org, mkwst+moarreviews-content_chromium.org, aboxhall+watch_chromium.org, jam, yuzo+watch_chromium.org, darin-cc_chromium.org, dmazzoni+watch_chromium.org, dtseng+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Fix Cocoa accessibility callbacks type mismatch If you compile without preprocessing (first clang -E then clang, like ccache does by default without CCACHE_CPP2), you will see warnings like: you will see warnings like: ../../content/browser/accessibility/browser_accessibility_cocoa.mm:1155:12:error: implicit conversion of NULL constant to 'NSUInteger' (aka 'unsigned long') [-Werror,-Wnull-conversion] return __null; ~~~~~~ ^~~~~~ 0 It's because we return nil when 0 or NO should be returned instead, normal clang compile won't catch that for some reason, perhaps because its checking is less strict for preprocessed code. Committed: https://crrev.com/8cd33c00d45afc7879d352d471f857c9faab59db Cr-Commit-Position: refs/heads/master@{#294848}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -3 lines) Patch
M content/browser/accessibility/browser_accessibility_cocoa.mm View 3 chunks +3 lines, -3 lines 0 comments Download

Messages

Total messages: 8 (2 generated)
Jiang Jiang
Avi and Nico: Any of you know why regular clang compile won't catch this? I ...
6 years, 3 months ago (2014-09-15 13:53:36 UTC) #2
Jiang Jiang
On 2014/09/15 13:53:36, Jiang Jiang wrote: > Avi and Nico: Any of you know why ...
6 years, 3 months ago (2014-09-15 13:55:41 UTC) #3
Avi (use Gerrit)
Looks like someone was way too aggressive with the copy/paste. LGTM
6 years, 3 months ago (2014-09-15 15:47:04 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patchset/565383003/1
6 years, 3 months ago (2014-09-15 15:50:35 UTC) #6
commit-bot: I haz the power
Committed patchset #1 (id:1) as 5a00cb7069d93529a4702e48ddae3b0c295223f5
6 years, 3 months ago (2014-09-15 17:51:33 UTC) #7
commit-bot: I haz the power
6 years, 3 months ago (2014-09-15 18:03:51 UTC) #8
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/8cd33c00d45afc7879d352d471f857c9faab59db
Cr-Commit-Position: refs/heads/master@{#294848}

Powered by Google App Engine
This is Rietveld 408576698