New issue
Advanced search Search tips

Issue 704361 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Add presubmit check when using NULL instead of nullptr

Project Member Reported by thomasanderson@chromium.org, Mar 23 2017

Issue description

It seems we're currently allowed to commit new code containing NULL.  This has bitten me in the past where I landed a commit with NULL, causing a Wsentinel error on 32-bit linux builders which are not on the CQ (32-bit linux headers define NULL to 0 instead of nullptr for some reason).

It should be simple enough to add a check alongside the printf check here https://cs.chromium.org/chromium/src/PRESUBMIT.py?rcl=434cad11c529808f9aa4c4898c3ddc57376709c9&l=1236
 
Cc: dpranke@chromium.org thakis@chromium.org
+Nico
Should we add this?
Cc: dcheng@chromium.org
+dcheng also ...

Comment 4 by dcheng@chromium.org, Mar 23 2017

Seems reasonable to me: we should be using nullptr instead of NULL in all new code.

Comment 5 by thakis@chromium.org, Mar 23 2017

I feel that we shouldn't parse c++ from Python, and during presubmit time. If we want a check for this, I think it should probably be in the clang plugin, or maybe in clang itself.

I'm not sure it's a very useful thing to have, but I don't have a strong opinion.

Comment 6 by dcheng@chromium.org, Mar 23 2017

We shouldn't, but we're just hooking into what's already there. So it doesn't strike me as that bad.

I imagine that once Tricium is up, we'd be able to remove the presubmit and just enable the clang-tidy check: https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-nullptr.html
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e7caaa9b45386f0061c6d5e33ec893a951b12c5b

commit e7caaa9b45386f0061c6d5e33ec893a951b12c5b
Author: thomasanderson <thomasanderson@google.com>
Date: Wed Mar 29 19:22:53 2017

Add presubmit check to warn against using NULL

BUG= 704361 
R=dpranke@chromium.org

Review-Url: https://codereview.chromium.org/2765423003
Cr-Commit-Position: refs/heads/master@{#460485}

[modify] https://crrev.com/e7caaa9b45386f0061c6d5e33ec893a951b12c5b/PRESUBMIT.py

Status: fixed (was: Untriaged)

Sign in to add a comment