ARROW-1722: [C++] Add linting script to find C++/CLI incompatibilities

ci/travis_lint.sh

@@ -19,11 +19,15 @@
 
 set -ex
 
+# Disable toolchain variables in this script
+export ARROW_TRAVIS_USE_TOOLCHAIN=0
+source $TRAVIS_BUILD_DIR/ci/travis_env_common.sh
+
 # Fail fast for code linting issues
 
 if [ "$ARROW_CI_CPP_AFFECTED" != "0" ]; then
-  mkdir $TRAVIS_BUILD_DIR/cpp/lint
-  pushd $TRAVIS_BUILD_DIR/cpp/lint
+  mkdir $ARROW_CPP_DIR/lint
+  pushd $ARROW_CPP_DIR/lint
 
   cmake ..
   make lint
@@ -32,6 +36,8 @@ if [ "$ARROW_CI_CPP_AFFECTED" != "0" ]; then
     make check-format
   fi
 
+  python $ARROW_CPP_DIR/build-support/lint_cpp_cli.py $ARROW_CPP_DIR/src
+
   popd
 fi
 
@@ -41,11 +47,9 @@ fi
 if [ "$ARROW_CI_PYTHON_AFFECTED" != "0" ]; then
   sudo pip install -q flake8
 
-  PYTHON_DIR=$TRAVIS_BUILD_DIR/python
-
-  flake8 --count $PYTHON_DIR
+  flake8 --count $ARROW_PYTHON_DIR
 
   # Check Cython files with some checks turned off
-  flake8 --count --config=$PYTHON_DIR/.flake8.cython \
-         $PYTHON_DIR
+  flake8 --count --config=$ARROW_PYTHON_DIR/.flake8.cython \
+         $ARROW_PYTHON_DIR
 fi

cpp/build-support/lint_cpp_cli.py

@@ -0,0 +1,79 @@
+#!/usr/bin/env python
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import argparse
+import re
+import os
+
+parser = argparse.ArgumentParser(
+    description="Check for illegal headers for C++/CLI applications")
+parser.add_argument("source_path",
+                    help="Path to source code")
+arguments = parser.parse_args()
+
+
+_STRIP_COMMENT_REGEX = re.compile('(.+)?(?=//)')
+
+
+def _strip_comments(line):
+    m = _STRIP_COMMENT_REGEX.match(line)
+    if not m:
+        return line
+    else:
+        return m.group(0)
+
+
+def lint_file(path):
+    fail_rules = [
+        (lambda x: '<mutex>' in x, 'Uses <mutex>'),
+        (lambda x: 'nullptr' in x, 'Uses nullptr')
+    ]
+
+    with open(path) as f:
+        for i, line in enumerate(f):
+            stripped_line = _strip_comments(line)
+            for rule, why in fail_rules:
+                if rule(stripped_line):
+                    raise Exception('File {0} failed C++/CLI lint check: {1}\n'
+                                    'Line {2}: {3}'
+                                    .format(path, why, i + 1, line))
+
+
+EXCLUSIONS = [
+    'arrow/util/macros.h',
+    'arrow/util/parallel.h',
+    'arrow/io/hdfs-internal.h'
+]
+
+
+for dirpath, _, filenames in os.walk(arguments.source_path):
+    for filename in filenames:
+        full_path = os.path.join(dirpath, filename)
+
+        exclude = False
+        for exclusion in EXCLUSIONS:
+            if exclusion in full_path:
+                exclude = True
+                break
+
+        if exclude:
+            continue
+
+        # Only run on header files
+        if filename.endswith('.h'):
+            lint_file(full_path)

cpp/src/arrow/compute/kernel.h

@@ -58,7 +58,7 @@ struct ARROW_EXPORT Datum {
       value;
 
   /// \brief Empty datum, to be populated elsewhere
-  Datum() : value(nullptr) {}
+  Datum() : value(NULLPTR) {}
 
   explicit Datum(const std::shared_ptr<Scalar>& value) : value(value) {}
 
@@ -124,7 +124,7 @@ struct ARROW_EXPORT Datum {
     } else if (this->kind() == Datum::CHUNKED_ARRAY) {
       return util::get<std::shared_ptr<ChunkedArray>>(this->value)->type();
     }
-    return nullptr;
+    return NULLPTR;
   }
 };
 

cpp/src/arrow/ipc/test-common.h

@@ -171,7 +171,7 @@ Status MakeRandomBooleanArray(const int length, bool include_nulls,
     test::random_null_bytes(length, 0.1, valid_bytes.data());
     *out = std::make_shared<BooleanArray>(length, data, null_bitmap, -1);
   } else {
-    *out = std::make_shared<BooleanArray>(length, data, nullptr, 0);
+    *out = std::make_shared<BooleanArray>(length, data, NULLPTR, 0);
   }
   return Status::OK();
 }
@@ -461,7 +461,7 @@ Status MakeUnion(std::shared_ptr<RecordBatch>* out) {
   auto sparse_no_nulls =
       std::make_shared<UnionArray>(sparse_type, length, sparse_children, type_ids_buffer);
   auto sparse = std::make_shared<UnionArray>(sparse_type, length, sparse_children,
-                                             type_ids_buffer, nullptr, null_bitmask, 1);
+                                             type_ids_buffer, NULLPTR, null_bitmask, 1);
 
   auto dense =
       std::make_shared<UnionArray>(dense_type, length, dense_children, type_ids_buffer,

cpp/src/arrow/memory_pool.cc

@@ -18,10 +18,12 @@
 #include "arrow/memory_pool.h"
 
 #include <algorithm>
+#include <atomic>
 #include <cerrno>
 #include <cstdlib>
 #include <cstring>
 #include <iostream>
+#include <memory>
 #include <mutex>
 #include <sstream>  // IWYU pragma: keep
 
@@ -202,39 +204,70 @@ int64_t LoggingMemoryPool::max_memory() const {
   return mem;
 }
 
-ProxyMemoryPool::ProxyMemoryPool(MemoryPool* pool) : pool_(pool) {}
+class ProxyMemoryPool::ProxyMemoryPoolImpl {
+ public:
+  explicit ProxyMemoryPoolImpl(MemoryPool* pool) : pool_(pool) {}
 
-Status ProxyMemoryPool::Allocate(int64_t size, uint8_t** out) {
-  RETURN_NOT_OK(pool_->Allocate(size, out));
-  bytes_allocated_ += size;
-  {
-    std::lock_guard<std::mutex> guard(lock_);
-    if (bytes_allocated_ > max_memory_) {
-      max_memory_ = bytes_allocated_.load();
+  Status Allocate(int64_t size, uint8_t** out) {
+    RETURN_NOT_OK(pool_->Allocate(size, out));
+    bytes_allocated_ += size;
+    {
+      std::lock_guard<std::mutex> guard(lock_);
+      if (bytes_allocated_ > max_memory_) {
+        max_memory_ = bytes_allocated_.load();
+      }
     }
+    return Status::OK();
   }
-  return Status::OK();
-}
 
-Status ProxyMemoryPool::Reallocate(int64_t old_size, int64_t new_size, uint8_t** ptr) {
-  RETURN_NOT_OK(pool_->Reallocate(old_size, new_size, ptr));
-  bytes_allocated_ += new_size - old_size;
-  {
-    std::lock_guard<std::mutex> guard(lock_);
-    if (bytes_allocated_ > max_memory_) {
-      max_memory_ = bytes_allocated_.load();
+  Status Reallocate(int64_t old_size, int64_t new_size, uint8_t** ptr) {
+    RETURN_NOT_OK(pool_->Reallocate(old_size, new_size, ptr));
+    bytes_allocated_ += new_size - old_size;
+    {
+      std::lock_guard<std::mutex> guard(lock_);
+      if (bytes_allocated_ > max_memory_) {
+        max_memory_ = bytes_allocated_.load();
+      }
     }
+    return Status::OK();
   }
-  return Status::OK();
+
+  void Free(uint8_t* buffer, int64_t size) {
+    pool_->Free(buffer, size);
+    bytes_allocated_ -= size;
+  }
+
+  int64_t bytes_allocated() const { return bytes_allocated_.load(); }
+
+  int64_t max_memory() const { return max_memory_.load(); }
+
+ private:
+  mutable std::mutex lock_;
+  MemoryPool* pool_;
+  std::atomic<int64_t> bytes_allocated_{0};
+  std::atomic<int64_t> max_memory_{0};
+};
+
+ProxyMemoryPool::ProxyMemoryPool(MemoryPool* pool) {
+  impl_.reset(new ProxyMemoryPoolImpl(pool));
+}
+
+ProxyMemoryPool::~ProxyMemoryPool() {}
+
+Status ProxyMemoryPool::Allocate(int64_t size, uint8_t** out) {
+  return impl_->Allocate(size, out);
+}
+
+Status ProxyMemoryPool::Reallocate(int64_t old_size, int64_t new_size, uint8_t** ptr) {
+  return impl_->Reallocate(old_size, new_size, ptr);
 }
 
 void ProxyMemoryPool::Free(uint8_t* buffer, int64_t size) {
-  pool_->Free(buffer, size);
-  bytes_allocated_ -= size;
+  return impl_->Free(buffer, size);
 }
 
-int64_t ProxyMemoryPool::bytes_allocated() const { return bytes_allocated_.load(); }
+int64_t ProxyMemoryPool::bytes_allocated() const { return impl_->bytes_allocated(); }
 
-int64_t ProxyMemoryPool::max_memory() const { return max_memory_.load(); }
+int64_t ProxyMemoryPool::max_memory() const { return impl_->max_memory(); }
 
 }  // namespace arrow