From 4edbb0db2ed00d86434f8fba79f78b526f39aee6 Mon Sep 17 00:00:00 2001 From: Vladimir Dimic Date: Fri, 13 May 2022 11:40:55 +0200 Subject: [PATCH 1/5] Fix line break in bullet list --- docs/Development.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Development.md b/docs/Development.md index 5bdb5af28..8a250b5b6 100644 --- a/docs/Development.md +++ b/docs/Development.md @@ -95,7 +95,7 @@ well: * files should end with an empty line * no `if`, `for`, `while`, or any other control structure without curly -* brackets, even if what follows is a single statement +brackets, even if what follows is a single statement * OpenMP pragmas (or any pragma) are indented as regular code * nested `ifdef`s etc. in close proximity of one another are indented by spaces From b7f6bdb4f0f4925d311074cd5d7462f268540f1d Mon Sep 17 00:00:00 2001 From: Vladimir Dimic Date: Fri, 13 May 2022 14:42:08 +0200 Subject: [PATCH 2/5] Expand the definition of coding style guidelines --- docs/Development.md | 119 ++++++++++++++++++-------------------------- 1 file changed, 48 insertions(+), 71 deletions(-) diff --git a/docs/Development.md b/docs/Development.md index 8a250b5b6..7f5b031ad 100644 --- a/docs/Development.md +++ b/docs/Development.md @@ -31,76 +31,36 @@ abstractions (typically encoded as template parameters: see the ALP/GraphBLAS follows certain code style rules in order to ensure readability and uniformity. -To apply these rules, the directory `tools` contains the script -`clang-format-linter.sh` to format (*lint*, in Unix jargon) the code -accordingly, based on the `clang-format` tool. -Version 11 or higher is requested for the settings to be applied; if you want to -use a different version, you can alias it in Bash before invoking -`tools/clang-format-linter.sh`, which directly calls the command -`clang-format-11`. -This tools is available in the standard repositories of the main Linux -distributions: for example, in Ubuntu you can install it with -`apt-get install clang-format-11`. - -To list the script parameters, simply type - -```bash -tools/clang-format-linter.sh -h -``` -For example, to lint the file `tests/add15d.cpp` and see the lint'ed code on the -standard output, type - -```bash -tools/clang-format-linter.sh tests/add15d.cpp -``` - -while to change the file in-place, add the `-i` option - -```bash -tools/clang-format-linter.sh -i tests/add15d.cpp -``` - -Instead, to lint the whole ALP/GraphBLAS code-base in-place, type - -```bash -tools/clang-format-linter.sh -i --lint-whole-grb -``` - -The style rules enforced by the tool are - -- [x] lines are max 200 characters long, which means the line size is pretty -liberal to avoid weird re-flows -- [x] indents should be *tabs*, not spaces -- [x] alignment should be done using spaces, not tabs -- [x] essentially any line that ends in `{`, `(`, or whatever increases the -current number of indents by one and vice versa -- [x] argument lists (including template arguments) longer than 80 chars should -be broken over multiple lines -- [x] `if( `, not `if (` (also for `for`, etc.) -- [x] no lines with indents and curly brackets only: put curly brackets on the -same line as what starts that code block instead (only exception: code blocks -that are not started by standard C++ key words, but e.g. required pragmas -instead) -- [x] no lines ending with spaces -- [x] `#ifdef`, `#else`, `#endif` etc are never indented. -- [x] comment blocks are capped at 80 chars per line -- [x] include lines primarily ordered by - 1. standard includes - 2. external libraries - 3. internal headers/files +### General guidelines -The following rules are also mandated, but cannot currently be applied via -`clang-format`; however, developers should abide by the following guidelines as -well: +* all functions should have `doxygen`-friendly documentation +* minimise the use of pre-processor macros (use C++11 `constexpr` instead) +* when closing a block (either `#endif` or `}`) and the block was long (whatever +long may be), add a comment on what it is that is being closed +### Naming +* classes: UpperCamelCase (e.g., MatrixContainer) +* class methods: lowerCamelCase (e.g., getElement) +* global functions: lower case, multi-word names separated by underscores +* variables: lower-case, underscore-separated +* template parameter type names: UpperCamelCase +* template parameter variables: lower-case, underscore-separated +* types (defined with typedef or using): lower case, separated by underscores, +terminating in `_type` + + +### Code formatting +* maximum line length is 200 characters, with the following exceptions +* comment blocks are capped at 80 chars per line +* argument lists (including template arguments) are capped at 80 characters per line + +* indents should be *tabs*, not spaces +* alignment should be done using spaces, not tabs +* essentially any line that ends in `{`, `(`, or similar increases the +current number of indents by one and vice versa +* write `if( `, not `if (` (also for `for`, etc.) +* lines cannot have trailing whitespace * files should end with an empty line -* no `if`, `for`, `while`, or any other control structure without curly -brackets, even if what follows is a single statement -* OpenMP pragmas (or any pragma) are indented as regular code -* nested `ifdef`s etc. in close proximity of one another are indented by spaces - -The following guidelines are not strictly requested nor enforced, but are -suggested to ensure readability and uniformity: * be gratuitous with spaces and parenthesis: anything that could possibly be construed as confusing or ambiguous should be clarified with spaces and @@ -109,10 +69,27 @@ parentheses if that removes (some of the) possible confusion or ambiguity (e.g., `if( x == 5 )` instead of `if( x==5 )`) * in particular, only write `<<` or `>>` when doing bit shifts, not when performing template magic -* when closing a block (either `#endif` or `}`) and the block was long (whatever -long may be), add a comment on what it is that is being closed -* all functions should have `doxygen`-friendly documentation -* minimise the use of pre-processor macros (use C++11 `constexpr` instead) +* exceptions from above rules: + * negation of a condition (write `!value` instead of `! value`) + * pointer or reference (write `&variable` instead of `& variable`) + +* `#ifdef`, `#else`, `#endif` etc are never indented. +* OpenMP pragmas (or any pragma) are indented as regular code +* nested `ifdef`s etc. in close proximity of one another are indented by spaces + +* no `if`, `for`, `while`, or any other control structure without curly +brackets, even if what follows is a single statement + +- no lines with indents and curly brackets only: put curly brackets on the +same line as what starts that code block instead (only exception: code blocks +that are not started by standard C++ key words, but e.g. required pragmas +instead) + +- include lines primarily ordered by + 1. standard includes + 2. external libraries + 3. internal headers/files + ## Building and Testing infrastructure From d5e76508ebfc2d3de9138cf6a125f776c964078c Mon Sep 17 00:00:00 2001 From: Vladimir Dimic Date: Fri, 13 May 2022 14:47:23 +0200 Subject: [PATCH 3/5] Remove clang formatting tool --- .clang-format | 178 ----------------------------------- tools/clang-format-linter.sh | 120 ----------------------- 2 files changed, 298 deletions(-) delete mode 100644 .clang-format delete mode 100755 tools/clang-format-linter.sh diff --git a/.clang-format b/.clang-format deleted file mode 100644 index 0353287f4..000000000 --- a/.clang-format +++ /dev/null @@ -1,178 +0,0 @@ - -# -# Copyright 2021 Huawei Technologies Co., Ltd. -# -# 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. -# - -Language: Cpp - -# assumes clang-format 11 https://releases.llvm.org/11.0.0/tools/clang/docs/ClangFormatStyleOptions.html -# which is available in Ubuntu 20.04 as clang-format-11 - -BasedOnStyle: Mozilla -AccessModifierOffset: -4 -AlignAfterOpenBracket: DontAlign -AlignConsecutiveMacros: false -AlignConsecutiveAssignments: false -#AlignConsecutiveBitFields: false -AlignConsecutiveDeclarations: false -AlignEscapedNewlines: Left -AlignOperands: false -AlignTrailingComments: true -AllowAllArgumentsOnNextLine: true -AllowAllConstructorInitializersOnNextLine: true -AllowAllParametersOfDeclarationOnNextLine: false -#AllowShortEnumsOnASingleLine: false -AllowShortBlocksOnASingleLine: Empty -AllowShortCaseLabelsOnASingleLine: false -AllowShortFunctionsOnASingleLine: Empty -AllowShortIfStatementsOnASingleLine: Never -AllowShortLambdasOnASingleLine: Empty -AllowShortLoopsOnASingleLine: false -AlwaysBreakAfterDefinitionReturnType: None -AlwaysBreakAfterReturnType: None -AlwaysBreakBeforeMultilineStrings: false -AlwaysBreakTemplateDeclarations: Yes -BinPackArguments: true -BinPackParameters: false -BraceWrapping: -BreakBeforeBinaryOperators: None - -BreakBeforeBraces: Attach -# AfterCaseLabel: true -# AfterClass: false -# AfterControlStatement: Never -# AfterEnum: false -# AfterFunction: false -# AfterNamespace: false -# AfterObjCDeclaration: false -# AfterStruct: false -# AfterUnion: false -# AfterExternBlock: false -# BeforeCatch: false -# BeforeElse: false -## BeforeLambdaBody: false -## BeforeWhile: false -# IndentBraces: false -# SplitEmptyFunction: true -# SplitEmptyRecord: true -# SplitEmptyNamespace: true - -BreakBeforeTernaryOperators: false -BreakBeforeInheritanceComma: false -#BreakConstructorInitializersBeforeComma: false -BreakConstructorInitializers: AfterColon -BreakInheritanceList: AfterColon -#BreakAfterJavaFieldAnnotations: false -BreakStringLiterals: true -ColumnLimit: 200 -#CommentPragmas: '^ IWYU pragma:' -CompactNamespaces: false -ConstructorInitializerAllOnOneLineOrOnePerLine: false -ConstructorInitializerIndentWidth: 4 -# ATTENTION: -# setting this option to a value other than 0 may cause the indentation -# to be unstable: multiple runs of the linter would keep changing -# the same file back and forth and loop between two different text layouts -ContinuationIndentWidth: 4 -Cpp11BracedListStyle: false -DeriveLineEnding: true -DerivePointerAlignment: false -DisableFormat: false -#EmptyLineBeforeAccessModifier: Always -ExperimentalAutoDetectBinPacking: false -FixNamespaceComments: true -ForEachMacros: - - foreach -IncludeBlocks: Regroup -IncludeCategories: -# -# Extended Regular Expressions: -# https://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap09.html -# -# first match full path graphblas includes, but put them third - - Regex: '^([<|"]graphblas/)' - Priority: 1 - SortPriority: 3 -# then match standard includes (i.e. headers in <> without extension) and put them first - - Regex: '^<([A-Za-z0-9\Q/-_\E]+)>' - Priority: 2 - SortPriority: 1 -# then match standard headers starting with "std" and with extension and put them also in the first group - - Regex: '^<(std[A-Za-z0-9\Q/-_\E]*\.h)>' - Priority: 3 - SortPriority: 1 -# then match other headers in <> with extensions and put in the second group - - Regex: '^<([A-Za-z0-9.\Q/-_\E]+\.h[h|pp|xx]?)>' - Priority: 4 - SortPriority: 2 -# finally includes within "" and put them last - - Regex: '".*"' - Priority: 5 - SortPriority: 4 -IncludeIsMainRegex: '(Test)?$' -IncludeIsMainSourceRegex: '' -#IndentCaseBlocks: false -IndentCaseLabels: true -#IndentExternBlock: AfterExternBlock -IndentGotoLabels: true -IndentPPDirectives: None -IndentWidth: 4 -IndentWrappedFunctionNames: false -#InsertTrailingCommas: None -JavaScriptQuotes: Leave -JavaScriptWrapImports: true -KeepEmptyLinesAtTheStartOfBlocks: true -MacroBlockBegin: '' -MacroBlockEnd: '' -MaxEmptyLinesToKeep: 1 -NamespaceIndentation: All -PenaltyBreakAssignment: 100 -PenaltyBreakBeforeFirstCallParameter: 5 -PenaltyBreakComment: 1 -PenaltyBreakFirstLessLess: 5 -PenaltyBreakString: 1 -PenaltyBreakTemplateDeclaration: 1 -PenaltyExcessCharacter: 1000 -PenaltyReturnTypeOnItsOwnLine: 20 -PointerAlignment: Middle -ReflowComments: true -SortIncludes: true -SortUsingDeclarations: true -SpaceAfterCStyleCast: false -SpaceAfterLogicalNot: true -SpaceAfterTemplateKeyword: false -SpaceBeforeAssignmentOperators: true -SpaceBeforeCpp11BracedList: true -SpaceBeforeCtorInitializerColon: true -SpaceBeforeInheritanceColon: true -SpaceBeforeParens: Never -SpaceBeforeRangeBasedForLoopColon: true -SpaceInEmptyBlock: false -SpaceInEmptyParentheses: false -SpacesBeforeTrailingComments: 1 -SpacesInAngles: true -SpacesInConditionalStatement: true -SpacesInContainerLiterals: true -SpacesInCStyleCastParentheses: false -SpacesInParentheses: true -SpacesInSquareBrackets: true -SpaceBeforeSquareBrackets: false -Standard: c++11 -TabWidth: 4 -UseCRLF: false -UseTab: AlignWithSpaces -#WhitespaceSensitiveMacros: -# - STRINGIZE -# - PP_STRINGIZE diff --git a/tools/clang-format-linter.sh b/tools/clang-format-linter.sh deleted file mode 100755 index 0a7d1cada..000000000 --- a/tools/clang-format-linter.sh +++ /dev/null @@ -1,120 +0,0 @@ -#!/bin/bash - -# -# Copyright 2021 Huawei Technologies Co., Ltd. -# -# 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. -# - -SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )" -# assuming we are in a subdirectory of the code (currently 'tools') -REPO_ROOT="$(dirname ${SCRIPT_DIR})" -CURDIR="$(pwd)" -SCRIPT_NAME=`basename "$0"` -CXX_EXT='.*\.\(cpp\|hpp\|cc\|cxx\|hh\|hxx\)' -MIN_VER=11 -CF_COMMAND="clang-format-${MIN_VER}" -LINTER="${CF_COMMAND} -style=file" -INPLACE=no -TARGET="file" - -function print_synopsis() { - echo "SYNOPSIS: ${SCRIPT_NAME} [OPTIONS] " - echo " OPTIONS:" - echo " --help, -h prints this help" - echo " --in-place, -i to lint the file(s) in place" - echo " --lint-whole-grb lints the whole GraphBLAS codebase" - echo " --tree, -t lints all the files in the given directory" - echo " lints the given file(s)" -} - -(which ${CF_COMMAND} &> /dev/null) -FOUND=$? -if [[ "${FOUND}" -ne "0" ]]; then - echo -e "Cannot find the command '${CF_COMMAND}'" - exit -1 -fi - -if [[ $# -eq 0 ]]; then - echo -e "No argument given!" - print_synopsis - exit -1 -fi - -VERSION="$(${CF_COMMAND} --version | sed -r 's/.*version\s+([0-9]+).*/\1/g')" -if [[ "${VERSION}" -lt "${MIN_VER}" ]]; then - echo -e "Detected ${CF_COMMAND} version ${VERSION}, while version ${MIN_VER} or greater is expected." - echo -e "The applied format may not be as expected or the tool may unexpectedly terminate: cannot proceed!" - exit -1 -fi - -ALL_ARGS=("$@") -while test -n "$1" -do - case "$1" in - --help|-h) - print_synopsis - exit 0 - ;; - --lint-whole-grb) - TARGET="tree" - shift - ;; - --in-place|-i) - INPLACE=yes - shift - ;; - --tree|-t) - TARGET="tree" - REPO_ROOT=$(cd "$2" && pwd) - if [[ "$?" != "0" ]]; then - echo -e "'$2' is not a valid directory" - exit -1 - fi - shift 2 - ;; - -*) - echo -e "unknown option $1" - print_synopsis - exit -1 - ;; - *) - break - ;; - esac -done - -files="$@" -if [[ "${TARGET}" = "tree" ]]; then - echo "Linting all source files inside ${REPO_ROOT}" - files="$(find "${REPO_ROOT}" -regex "${CXX_EXT}" )" -fi - -# to amortize for format parsing, chunk the list of files -# into chunks of g length and pass each list to clang-format, -# which ignores too long lists -g=10 -files=( ${files} ) -for (( i=0; i < ${#files[@]}; i+=g )) -do - part=( "${files[@]:i:g}" ) - if [ "x${INPLACE}" = "xyes" ]; then - sed -i 's/#pragma omp/\/\/#pragma omp/' ${part[*]} - ${LINTER} -i ${part[*]} - sed -i 's/\/\/#pragma omp/#pragma omp/' ${part[*]} - else - sed 's/#pragma omp/\/\/#pragma omp/' ${part[*]} | ${LINTER} | sed 's/\/\/#pragma omp/#pragma omp/' - fi -done -echo "=> To check all files are properly linted until convergence, you should run the same command again <=" >&2 - From 89e9eb785a8d64bc9e4144442a641ed1826530bc Mon Sep 17 00:00:00 2001 From: Vladimir Dimic Date: Fri, 13 May 2022 15:25:11 +0200 Subject: [PATCH 4/5] Fix indentation --- docs/Development.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/Development.md b/docs/Development.md index 7f5b031ad..a5cfe0bd5 100644 --- a/docs/Development.md +++ b/docs/Development.md @@ -51,8 +51,9 @@ terminating in `_type` ### Code formatting * maximum line length is 200 characters, with the following exceptions -* comment blocks are capped at 80 chars per line -* argument lists (including template arguments) are capped at 80 characters per line + * comment blocks are capped at 80 chars per line + * argument lists (including template arguments) are capped at 80 characters per line + * indents should be *tabs*, not spaces * alignment should be done using spaces, not tabs From 78a75f0b4dba077dc6ea377684641cf047ba1178 Mon Sep 17 00:00:00 2001 From: Vladimir Dimic Date: Fri, 13 May 2022 15:25:29 +0200 Subject: [PATCH 5/5] Fix formatting and other minor improvements --- docs/Development.md | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/Development.md b/docs/Development.md index a5cfe0bd5..5890825c4 100644 --- a/docs/Development.md +++ b/docs/Development.md @@ -63,6 +63,7 @@ current number of indents by one and vice versa * lines cannot have trailing whitespace * files should end with an empty line + * be gratuitous with spaces and parenthesis: anything that could possibly be construed as confusing or ambiguous should be clarified with spaces and parentheses if that removes (some of the) possible confusion or ambiguity @@ -71,22 +72,26 @@ parentheses if that removes (some of the) possible confusion or ambiguity * in particular, only write `<<` or `>>` when doing bit shifts, not when performing template magic * exceptions from above rules: - * negation of a condition (write `!value` instead of `! value`) + * negation of a condition (write `!condition` instead of `! condition`) * pointer or reference (write `&variable` instead of `& variable`) + * `#ifdef`, `#else`, `#endif` etc are never indented. * OpenMP pragmas (or any pragma) are indented as regular code * nested `ifdef`s etc. in close proximity of one another are indented by spaces + * no `if`, `for`, `while`, or any other control structure without curly brackets, even if what follows is a single statement -- no lines with indents and curly brackets only: put curly brackets on the + +* no lines with indents and curly brackets only: put curly brackets on the same line as what starts that code block instead (only exception: code blocks that are not started by standard C++ key words, but e.g. required pragmas instead) -- include lines primarily ordered by + +* include lines primarily ordered by 1. standard includes 2. external libraries 3. internal headers/files