Built "make format" target format target using clang-format based on arrow-cpp and clean up existing codebase

Change-Id: I06131ffe683e5d126cd7d95bf9525bcf818341bc

Author: wesm

CMakeLists.txt

@@ -22,12 +22,26 @@ set(THIRDPARTY_DIR ${CMAKE_CURRENT_SOURCE_DIR}/thirdparty)
 # Must be declared in the top-level CMakeLists.txt.
 set(CMAKE_SKIP_INSTALL_ALL_DEPENDENCY true)
 
-# For thirdparty libs: googletest
-# include(toolchain)
-
 set(CMAKE_MACOSX_RPATH 1)
 set(CMAKE_OSX_DEPLOYMENT_TARGET 10.9)
 
+find_package(ClangTools)
+if ("$ENV{CMAKE_EXPORT_COMPILE_COMMANDS}" STREQUAL "1" OR CLANG_TIDY_FOUND)
+  # Generate a Clang compile_commands.json "compilation database" file for use
+  # with various development tools, such as Vim's YouCompleteMe plugin.
+  # See http://clang.llvm.org/docs/JSONCompilationDatabase.html
+  set(CMAKE_EXPORT_COMPILE_COMMANDS 1)
+endif()
+
+find_program(CCACHE_FOUND ccache)
+if(CCACHE_FOUND)
+  set_property(GLOBAL PROPERTY RULE_LAUNCH_COMPILE ccache)
+  set_property(GLOBAL PROPERTY RULE_LAUNCH_LINK ccache)
+endif(CCACHE_FOUND)
+
+# ----------------------------------------------------------------------
+# CMake options
+
 # Top level cmake dir
 if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
   option(PANDAS_BUILD_TESTS
@@ -578,6 +592,19 @@ if (UNIX)
     `find ${CMAKE_CURRENT_SOURCE_DIR}/src -name \\*.cc -or -name \\*.h`)
 endif (UNIX)
 
+############################################################
+# "make format" and "make check-format" targets
+############################################################
+if (${CLANG_FORMAT_FOUND})
+  # runs clang format and updates files in place.
+  add_custom_target(format ${BUILD_SUPPORT_DIR}/run-clang-format.sh ${CMAKE_CURRENT_SOURCE_DIR} ${CLANG_FORMAT_BIN} 1
+  `find ${CMAKE_CURRENT_SOURCE_DIR}/src -name \\*.cc -or -name \\*.h | sed -e '/_generated/g'`)
+
+  # runs clang format and exits with a non-zero exit code if any files need to be reformatted
+  add_custom_target(check-format ${BUILD_SUPPORT_DIR}/run-clang-format.sh ${CMAKE_CURRENT_SOURCE_DIR} ${CLANG_FORMAT_BIN} 0
+  `find ${CMAKE_CURRENT_SOURCE_DIR}/src -name \\*.cc -or -name \\*.h | sed -e '/_generated/g'`)
+endif()
+
 #############################################################
 # Test linking
 

build-support/run-clang-format.sh

@@ -0,0 +1,42 @@
+#!/bin/bash
+#
+# Licensed 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.
+#
+# Runs clang format in the given directory
+# Arguments:
+#   $1 - Path to the source tree
+#   $2 - Path to the clang format binary
+#   $3 - Apply fixes (will raise an error if false and not there where changes)
+#   $ARGN - Files to run clang format on
+#
+SOURCE_DIR=$1
+shift
+CLANG_FORMAT=$1
+shift
+APPLY_FIXES=$1
+shift
+
+# clang format will only find its configuration if we are in
+# the source tree or in a path relative to the source tree
+pushd $SOURCE_DIR
+if [ "$APPLY_FIXES" == "1" ]; then
+  $CLANG_FORMAT -i $@
+else
+
+  NUM_CORRECTIONS=`$CLANG_FORMAT -output-replacements-xml  $@ | grep offset | wc -l`
+  if [ "$NUM_CORRECTIONS" -gt "0" ]; then
+    echo "clang-format suggested changes, please run 'make format'!!!!"
+    exit 1
+  fi
+fi
+popd

cmake_modules/FindClangTools.cmake

@@ -0,0 +1,59 @@
+#
+# Licensed 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.
+#
+# Tries to find the clang-tidy and clang-format modules
+#
+# Usage of this module as follows:
+#
+#  find_package(ClangTools)
+#
+# Variables used by this module, they can change the default behaviour and need
+# to be set before calling find_package:
+#
+#  ClangToolsBin_HOME -
+#   When set, this path is inspected instead of standard library binary locations
+#   to find clang-tidy and clang-format
+#
+# This module defines
+#  CLANG_TIDY_BIN, The  path to the clang tidy binary
+#  CLANG_TIDY_FOUND, Whether clang tidy was found
+#  CLANG_FORMAT_BIN, The path to the clang format binary
+#  CLANG_TIDY_FOUND, Whether clang format was found
+
+find_program(CLANG_TIDY_BIN
+  NAMES clang-tidy-3.8 clang-tidy-3.7 clang-tidy-3.6  clang-tidy
+  PATHS ${ClangTools_PATH} $ENV{CLANG_TOOLS_PATH} /usr/local/bin /usr/bin
+        NO_DEFAULT_PATH
+)
+
+if ( "${CLANG_TIDY_BIN}" STREQUAL "CLANG_TIDY_BIN-NOTFOUND" )
+  set(CLANG_TIDY_FOUND 0)
+  message("clang-tidy not found")
+else()
+  set(CLANG_TIDY_FOUND 1)
+  message("clang-tidy found at ${CLANG_TIDY_BIN}")
+endif()
+
+find_program(CLANG_FORMAT_BIN
+  NAMES clang-format-3.8 clang-format-3.7 clang-format-3.6  clang-format
+  PATHS ${ClangTools_PATH} $ENV{CLANG_TOOLS_PATH} /usr/local/bin /usr/bin
+        NO_DEFAULT_PATH
+)
+
+if ( "${CLANG_FORMAT_BIN}" STREQUAL "CLANG_FORMAT_BIN-NOTFOUND" )
+  set(CLANG_FORMAT_FOUND 0)
+  message("clang-format not found")
+else()
+  set(CLANG_FORMAT_FOUND 1)
+  message("clang-format found at ${CLANG_FORMAT_BIN}")
+endif()

src/.clang-format

@@ -0,0 +1,65 @@
+---
+Language:        Cpp
+# BasedOnStyle:  Google
+AccessModifierOffset: -1
+AlignAfterOpenBracket: false
+AlignConsecutiveAssignments: false
+AlignEscapedNewlinesLeft: true
+AlignOperands:   true
+AlignTrailingComments: true
+AllowAllParametersOfDeclarationOnNextLine: true
+AllowShortBlocksOnASingleLine: true
+AllowShortCaseLabelsOnASingleLine: false
+AllowShortFunctionsOnASingleLine: Inline
+AllowShortIfStatementsOnASingleLine: true
+AllowShortLoopsOnASingleLine: false
+AlwaysBreakAfterDefinitionReturnType: None
+AlwaysBreakBeforeMultilineStrings: true
+AlwaysBreakTemplateDeclarations: true
+BinPackArguments: true
+BinPackParameters: true
+BreakBeforeBinaryOperators: None
+BreakBeforeBraces: Attach
+BreakBeforeTernaryOperators: true
+BreakConstructorInitializersBeforeComma: false
+ColumnLimit: 90
+CommentPragmas:  '^ IWYU pragma:'
+ConstructorInitializerAllOnOneLineOrOnePerLine: true
+ConstructorInitializerIndentWidth: 4
+ContinuationIndentWidth: 4
+Cpp11BracedListStyle: true
+DerivePointerAlignment: false
+DisableFormat:   false
+ExperimentalAutoDetectBinPacking: false
+ForEachMacros:   [ foreach, Q_FOREACH, BOOST_FOREACH ]
+IndentCaseLabels: true
+IndentWidth:     2
+IndentWrappedFunctionNames: false
+KeepEmptyLinesAtTheStartOfBlocks: false
+MacroBlockBegin: ''
+MacroBlockEnd:   ''
+MaxEmptyLinesToKeep: 1
+NamespaceIndentation: None
+ObjCBlockIndentWidth: 2
+ObjCSpaceAfterProperty: false
+ObjCSpaceBeforeProtocolList: false
+PenaltyBreakBeforeFirstCallParameter: 1000
+PenaltyBreakComment: 300
+PenaltyBreakFirstLessLess: 120
+PenaltyBreakString: 1000
+PenaltyExcessCharacter: 1000000
+PenaltyReturnTypeOnItsOwnLine: 200
+PointerAlignment: Left
+SpaceAfterCStyleCast: false
+SpaceBeforeAssignmentOperators: true
+SpaceBeforeParens: ControlStatements
+SpaceInEmptyParentheses: false
+SpacesBeforeTrailingComments: 2
+SpacesInAngles:  false
+SpacesInContainerLiterals: true
+SpacesInCStyleCastParentheses: false
+SpacesInParentheses: false
+SpacesInSquareBrackets: false
+Standard:        Cpp11
+TabWidth:        8
+UseTab:          Never

src/pandas/api.h

@@ -13,4 +13,4 @@
 #include "pandas/types/category.h"
 #include "pandas/types/numeric.h"
 
-#endif // PANDAS_API_H
+#endif  // PANDAS_API_H

src/pandas/array-test.cc

@@ -26,8 +26,9 @@ class TestArray : public ::testing::Test {
   void SetUp() {
     values_ = {0, 1, 2, 3, 4, 5, 6, 7};
 
-    auto buffer = std::make_shared<Buffer>(reinterpret_cast<const uint8_t*>(values_.data()),
-        values_.size() * sizeof(double));
+    auto buffer =
+        std::make_shared<Buffer>(reinterpret_cast<const uint8_t*>(values_.data()),
+            values_.size() * sizeof(double));
 
     array_ = std::make_shared<DoubleArray>(values_.size(), buffer);
   }
@@ -55,8 +56,9 @@ class TestArrayView : public ::testing::Test {
   void SetUp() {
     values_ = {0, 1, 2, 3, 4, 5, 6, 7};
 
-    auto buffer = std::make_shared<Buffer>(reinterpret_cast<const uint8_t*>(values_.data()),
-        values_.size() * sizeof(value_t));
+    auto buffer =
+        std::make_shared<Buffer>(reinterpret_cast<const uint8_t*>(values_.data()),
+            values_.size() * sizeof(value_t));
 
     auto arr = std::make_shared<DoubleArray>(values_.size(), buffer);
     view_ = ArrayView(arr);

src/pandas/array.cc

@@ -12,8 +12,7 @@ namespace pandas {
 // Array
 
 Array::Array(const std::shared_ptr<DataType>& type, int64_t length)
-    : type_(type),
-      length_(length) {}
+    : type_(type), length_(length) {}
 
 Status Array::Copy(std::shared_ptr<Array>* out) const {
   return Copy(0, length_, out);
@@ -23,23 +22,17 @@ Status Array::Copy(std::shared_ptr<Array>* out) const {
 // ArrayView
 
 ArrayView::ArrayView(const std::shared_ptr<Array>& data)
-    : data_(data),
-      offset_(0),
-      length_(data->length()) {}
+    : data_(data), offset_(0), length_(data->length()) {}
 
 ArrayView::ArrayView(const std::shared_ptr<Array>& data, int64_t offset)
-    : data_(data),
-      offset_(offset),
-      length_(data->length() - offset) {
+    : data_(data), offset_(offset), length_(data->length() - offset) {
   // Debugging sanity checks
   PANDAS_DCHECK_GE(offset, 0);
   PANDAS_DCHECK_LT(offset, data->length());
 }
 
 ArrayView::ArrayView(const std::shared_ptr<Array>& data, int64_t offset, int64_t length)
-    : data_(data),
-      offset_(offset),
-      length_(length) {
+    : data_(data), offset_(offset), length_(length) {
   // Debugging sanity checks
   PANDAS_DCHECK_GE(offset, 0);
   PANDAS_DCHECK_LT(offset, data->length());
@@ -49,15 +42,11 @@ ArrayView::ArrayView(const std::shared_ptr<Array>& data, int64_t offset, int64_t
 
 // Copy ctor
 ArrayView::ArrayView(const ArrayView& other)
-    : data_(other.data_),
-      offset_(other.offset_),
-      length_(other.length_) {}
+    : data_(other.data_), offset_(other.offset_), length_(other.length_) {}
 
 // Move ctor
 ArrayView::ArrayView(ArrayView&& other)
-    : data_(std::move(other.data_)),
-      offset_(other.offset_),
-      length_(other.length_) {}
+    : data_(std::move(other.data_)), offset_(other.offset_), length_(other.length_) {}
 
 // Copy assignment
 ArrayView& ArrayView::operator=(const ArrayView& other) {
@@ -97,4 +86,4 @@ int64_t ArrayView::ref_count() const {
   return data_.use_count();
 }
 
-} // namespace pandas
+}  // namespace pandas

src/pandas/array.h

@@ -22,12 +22,13 @@ class Array {
  public:
   virtual ~Array() {}
 
-  int64_t length() const { return length_;}
-  std::shared_ptr<DataType> type() const { return type_;}
-  DataType::TypeId type_id() const { return type_->type();}
+  int64_t length() const { return length_; }
+  std::shared_ptr<DataType> type() const { return type_; }
+  DataType::TypeId type_id() const { return type_->type(); }
 
   // Copy a section of the array into a new output array
-  virtual Status Copy(int64_t offset, int64_t length, std::shared_ptr<Array>* out) const = 0;
+  virtual Status Copy(
+      int64_t offset, int64_t length, std::shared_ptr<Array>* out) const = 0;
 
   // Copy the entire array (using the virtual Copy function)
   Status Copy(std::shared_ptr<Array>* out) const;
@@ -93,4 +94,4 @@ class ArrayView {
   int64_t length_;
 };
 
-} // namespace pandas
+}  // namespace pandas

src/pandas/buffer-test.cc

@@ -9,10 +9,10 @@
 
 #include "gtest/gtest.h"
 
-#include "pandas/test-util.h"
 #include "pandas/buffer.h"
 #include "pandas/memory.h"
 #include "pandas/status.h"
+#include "pandas/test-util.h"
 
 using std::string;
 

src/pandas/buffer.cc

@@ -38,8 +38,8 @@ Status Buffer::Copy(int64_t start, int64_t nbytes, std::shared_ptr<Buffer>* out)
   return Status::OK();
 }
 
-std::shared_ptr<Buffer> SliceBuffer(const std::shared_ptr<Buffer>& buffer, int64_t offset,
-    int64_t length) {
+std::shared_ptr<Buffer> SliceBuffer(
+    const std::shared_ptr<Buffer>& buffer, int64_t offset, int64_t length) {
   PANDAS_DCHECK_LT(offset, buffer->size());
   PANDAS_DCHECK_LE(length, buffer->size() - offset);
   return std::make_shared<Buffer>(buffer, offset, length);