From 3e7c2aa6c6fad7113ba6fab5c471da4515d30e8f Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 5 Mar 2020 18:13:11 -0800 Subject: [PATCH 01/55] initial version of clang-tidy wrapper script --- cpp/scripts/run-clang-tidy.py | 163 ++++++++++++++++++++++++++++++++++ 1 file changed, 163 insertions(+) create mode 100755 cpp/scripts/run-clang-tidy.py diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py new file mode 100755 index 0000000000..757e409aa7 --- /dev/null +++ b/cpp/scripts/run-clang-tidy.py @@ -0,0 +1,163 @@ +# Copyright (c) 2020, NVIDIA CORPORATION. +# +# 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. +# + +from __future__ import print_function +import sys +import re +import os +import subprocess +import argparse +import json + + +VERSION_REGEX = re.compile(r"LLVM version ([0-9.]+)") +GPU_ARCH_REGEX = re.compile(r"sm_(\d+)") +SPACES = re.compile(r"\s+") +SEPARATOR = "-" * 16 + + +def parse_args(): + argparser = argparse.ArgumentParser("Runs clang-tidy on a project") + argparser.add_argument("-cdb", type=str, default="compile_commands.json", + help="Path to cmake-generated compilation database") + argparser.add_argument("-exe", type=str, default="clang-tidy", + help="Path to clang-tidy exe") + argparser.add_argument("-ignore", type=str, default=None, + help="Regex used to ignore files from checking") + args = argparser.parse_args() + args.ignore_compiled = re.compile(args.ignore) if args.ignore else None + ret = subprocess.check_output("%s --version" % args.exe, shell=True) + ret = ret.decode("utf-8") + version = VERSION_REGEX.match(ret) + if version is None: + raise Exception("Failed to figure out clang-tidy version!") + version = version.group(1) + if version != "8.0.0": + raise Exception("clang-tidy exe must be v8.0.0 found '%s'" % version) + if not os.path.exists(args.cdb): + raise Exception("Compilation database '%s' missing" % args.cdb) + return args + + +def list_all_cmds(cdb): + with open(cdb, "r") as fp: + return json.load(fp) + + +def get_gpu_archs(command): + archs = [] + for loc in range(len(command)): + if command[loc] != "-gencode": + continue + arch_flag = command[loc + 1] + match = GPU_ARCH_REGEX.search(arch_flag) + if match is not None: + archs.append("--cuda-gpu-arch") + archs.append("sm_%s" % match.group(1)) + return archs + + +def get_index(arr, item): + try: + return arr.index(item) + except: + return -1 + +def remove_item_plus_one(arr, item): + loc = get_index(arr, item) + if loc >= 0: + del arr[loc + 1] + del arr[loc] + return loc + + +def get_tidy_args(cmd): + command, file = cmd["command"], cmd["file"] + is_cuda = file.endswith(".cu") + command = re.split(SPACES, command) + # compiler is always clang++! + command[0] = "clang++" + # remove compilation and output targets from the original command + remove_item_plus_one(command, "-c") + remove_item_plus_one(command, "-o") + if is_cuda: + # replace nvcc's "-gencode ..." with clang's "--cuda-gpu-arch ..." + archs = get_gpu_archs(command) + command.extend(archs) + while True: + loc = remove_item_plus_one(command, "-gencode") + if loc < 0: + break + # "-x cuda" is the right usage in clang + loc = get_index(command, "-x") + if loc >= 0: + command[loc + 1] = "cuda" + remove_item_plus_one(command, "-ccbin") + return command, is_cuda + + +def run_clang_tidy_command(tidy_cmd): + try: + subprocess.check_output(tidy_cmd, shell=True) + return True + except: + return False + + +def run_clang_tidy(cmd, args): + command, is_cuda = get_tidy_args(cmd) + tidy_cmd = [args.exe, cmd["file"], "--", ] + tidy_cmd.extend(command) + status = True + if is_cuda: + tidy_cmd.append("--cuda-device-only") + ret = run_clang_tidy_command(tidy_cmd) + if not ret: + status = ret + tidy_cmd[-1] = "--cuda-host-only" + ret = run_clang_tidy_command(tidy_cmd) + if not ret: + status = ret + else: + ret = run_clang_tidy_command(tidy_cmd) + if not ret: + status = ret + return status + + +def main(): + args = parse_args() + # Attempt to making sure that we run this script from root of repo always + if not os.path.exists(".git"): + raise Exception("This needs to always be run from the root of repo") + all_files = list_all_cmds(args.cdb) + # actual tidy checker + status = True + for cmd in all_files: + # skip files that we don't want to look at + if args.ignore_compiled is not None and \ + re.search(args.ignore_compiled, cmd["file"]) is not None: + continue + ret = run_clang_tidy(cmd, args) + if not ret: + status = ret + print("%s ^^^ %s ^^^ %s" % (SEPARATOR, cmd["file"], SEPARATOR)) + if not status: + raise Exception("clang-tidy failed! Refer to the errors above.") + return + + +if __name__ == "__main__": + main() From 5492263f686cba920154202d3afe99add6faeb0a Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 5 Mar 2020 19:20:36 -0800 Subject: [PATCH 02/55] minor fixes/hacks to make clang-tidy command run --- cpp/scripts/run-clang-tidy.py | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index 757e409aa7..a1f01745f1 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -22,7 +22,7 @@ import json -VERSION_REGEX = re.compile(r"LLVM version ([0-9.]+)") +VERSION_REGEX = re.compile(r" LLVM version ([0-9.]+)") GPU_ARCH_REGEX = re.compile(r"sm_(\d+)") SPACES = re.compile(r"\s+") SEPARATOR = "-" * 16 @@ -40,7 +40,7 @@ def parse_args(): args.ignore_compiled = re.compile(args.ignore) if args.ignore else None ret = subprocess.check_output("%s --version" % args.exe, shell=True) ret = ret.decode("utf-8") - version = VERSION_REGEX.match(ret) + version = VERSION_REGEX.search(ret) if version is None: raise Exception("Failed to figure out clang-tidy version!") version = version.group(1) @@ -83,6 +83,17 @@ def remove_item_plus_one(arr, item): return loc +# currently assumes g++ as the compiler +def get_system_includes(): + ret = subprocess.check_output("echo | g++ -E -Wp,-v - 2>&1", shell=True) + ret = ret.decode("utf-8") + incs = [] + for line in ret.split(os.linesep): + if len(line) > 0 and line[0] == " ": + incs.extend(["-I", line[1:]]) + return incs + + def get_tidy_args(cmd): command, file = cmd["command"], cmd["file"] is_cuda = file.endswith(".cu") @@ -105,12 +116,14 @@ def get_tidy_args(cmd): if loc >= 0: command[loc + 1] = "cuda" remove_item_plus_one(command, "-ccbin") + command.extend(get_system_includes()) return command, is_cuda def run_clang_tidy_command(tidy_cmd): try: - subprocess.check_output(tidy_cmd, shell=True) + cmd = " ".join(tidy_cmd) + subprocess.check_call(cmd, shell=True) return True except: return False @@ -123,14 +136,16 @@ def run_clang_tidy(cmd, args): status = True if is_cuda: tidy_cmd.append("--cuda-device-only") + tidy_cmd.append(cmd["file"]) ret = run_clang_tidy_command(tidy_cmd) if not ret: status = ret - tidy_cmd[-1] = "--cuda-host-only" + tidy_cmd[-2] = "--cuda-host-only" ret = run_clang_tidy_command(tidy_cmd) if not ret: status = ret else: + tidy_cmd.append(cmd["file"]) ret = run_clang_tidy_command(tidy_cmd) if not ret: status = ret @@ -154,6 +169,7 @@ def main(): if not ret: status = ret print("%s ^^^ %s ^^^ %s" % (SEPARATOR, cmd["file"], SEPARATOR)) + break if not status: raise Exception("clang-tidy failed! Refer to the errors above.") return From 0fa695c0854db5e728210de727010bc8daf6fe7c Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 5 Mar 2020 19:21:24 -0800 Subject: [PATCH 03/55] making PEP8 format fixes --- cpp/scripts/run-clang-tidy.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index a1f01745f1..251349ff50 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -75,6 +75,7 @@ def get_index(arr, item): except: return -1 + def remove_item_plus_one(arr, item): loc = get_index(arr, item) if loc >= 0: From 3ef66169fe10a59d063090f04ab3ce88a59ce47a Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 5 Mar 2020 19:26:46 -0800 Subject: [PATCH 04/55] removed an unnecessary break statement + added initial .clang-tidy --- cpp/.clang-tidy | 176 ++++++++++++++++++++++++++++++++++ cpp/scripts/run-clang-tidy.py | 1 - 2 files changed, 176 insertions(+), 1 deletion(-) create mode 100644 cpp/.clang-tidy diff --git a/cpp/.clang-tidy b/cpp/.clang-tidy new file mode 100644 index 0000000000..5a1f76451e --- /dev/null +++ b/cpp/.clang-tidy @@ -0,0 +1,176 @@ +--- +Checks: 'clang-diagnostic-*,clang-analyzer-*,clang-diagnostic-*,clang-analyzer-*,modernize-*,-modernize-make-*,-modernize-raw-string-literal,google-*,-google-default-arguments,-clang-diagnostic-#pragma-messages,readability-identifier-naming,-*,modernize-*,-modernize-make-*,-modernize-raw-string-literal,google-*,-google-default-arguments,-clang-diagnostic-#pragma-messages,readability-identifier-naming' +WarningsAsErrors: '' +HeaderFilterRegex: '' +AnalyzeTemporaryDtors: false +FormatStyle: google +User: snanditale +CheckOptions: + - key: cert-dcl16-c.NewSuffixes + value: 'L;LL;LU;LLU' + - key: cppcoreguidelines-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic + value: '1' + - key: google-build-namespaces.HeaderFileExtensions + value: ',h,hh,hpp,hxx' + - key: google-global-names-in-headers.HeaderFileExtensions + value: ',h,hh,hpp,hxx' + - key: google-readability-braces-around-statements.ShortStatementLines + value: '1' + - key: google-readability-function-size.BranchThreshold + value: '4294967295' + - key: google-readability-function-size.LineThreshold + value: '4294967295' + - key: google-readability-function-size.NestingThreshold + value: '4294967295' + - key: google-readability-function-size.ParameterThreshold + value: '4294967295' + - key: google-readability-function-size.StatementThreshold + value: '800' + - key: google-readability-function-size.VariableThreshold + value: '4294967295' + - key: google-readability-namespace-comments.ShortNamespaceLines + value: '10' + - key: google-readability-namespace-comments.SpacesBeforeComments + value: '2' + - key: google-runtime-int.SignedTypePrefix + value: int + - key: google-runtime-int.TypeSuffix + value: '' + - key: google-runtime-int.UnsignedTypePrefix + value: uint + - key: google-runtime-references.WhiteListTypes + value: '' + - key: modernize-loop-convert.MaxCopySize + value: '16' + - key: modernize-loop-convert.MinConfidence + value: reasonable + - key: modernize-loop-convert.NamingStyle + value: CamelCase + - key: modernize-pass-by-value.IncludeStyle + value: llvm + - key: modernize-pass-by-value.ValuesOnly + value: '0' + - key: modernize-replace-auto-ptr.IncludeStyle + value: llvm + - key: modernize-replace-random-shuffle.IncludeStyle + value: llvm + - key: modernize-use-auto.MinTypeNameLength + value: '5' + - key: modernize-use-auto.RemoveStars + value: '0' + - key: modernize-use-default-member-init.IgnoreMacros + value: '1' + - key: modernize-use-default-member-init.UseAssignment + value: '0' + - key: modernize-use-emplace.ContainersWithPushBack + value: '::std::vector;::std::list;::std::deque' + - key: modernize-use-emplace.SmartPointers + value: '::std::shared_ptr;::std::unique_ptr;::std::auto_ptr;::std::weak_ptr' + - key: modernize-use-emplace.TupleMakeFunctions + value: '::std::make_pair;::std::make_tuple' + - key: modernize-use-emplace.TupleTypes + value: '::std::pair;::std::tuple' + - key: modernize-use-equals-default.IgnoreMacros + value: '1' + - key: modernize-use-equals-delete.IgnoreMacros + value: '1' + - key: modernize-use-nodiscard.ReplacementString + value: '[[nodiscard]]' + - key: modernize-use-noexcept.ReplacementString + value: '' + - key: modernize-use-noexcept.UseNoexceptFalse + value: '1' + - key: modernize-use-nullptr.NullMacros + value: 'NULL' + - key: modernize-use-transparent-functors.SafeMode + value: '0' + - key: modernize-use-using.IgnoreMacros + value: '1' + - key: readability-identifier-naming.ClassCase + value: CamelCase + - key: readability-identifier-naming.ClassPrefix + value: '' + - key: readability-identifier-naming.ClassSuffix + value: '' + - key: readability-identifier-naming.ConstexprVariableCase + value: CamelCase + - key: readability-identifier-naming.ConstexprVariablePrefix + value: k + - key: readability-identifier-naming.ConstexprVariableSuffix + value: '' + - key: readability-identifier-naming.EnumCase + value: CamelCase + - key: readability-identifier-naming.EnumConstantPrefix + value: k + - key: readability-identifier-naming.EnumConstantSuffix + value: '' + - key: readability-identifier-naming.EnumPrefix + value: '' + - key: readability-identifier-naming.EnumSuffix + value: '' + - key: readability-identifier-naming.FunctionCase + value: CamelCase + - key: readability-identifier-naming.FunctionPrefix + value: '' + - key: readability-identifier-naming.FunctionSuffix + value: '' + - key: readability-identifier-naming.GlobalConstantCase + value: CamelCase + - key: readability-identifier-naming.GlobalConstantPrefix + value: k + - key: readability-identifier-naming.GlobalConstantSuffix + value: '' + - key: readability-identifier-naming.IgnoreFailedSplit + value: '0' + - key: readability-identifier-naming.MemberCase + value: lower_case + - key: readability-identifier-naming.MemberPrefix + value: '' + - key: readability-identifier-naming.MemberSuffix + value: '' + - key: readability-identifier-naming.NamespaceCase + value: lower_case + - key: readability-identifier-naming.NamespacePrefix + value: '' + - key: readability-identifier-naming.NamespaceSuffix + value: '' + - key: readability-identifier-naming.PrivateMemberPrefix + value: '' + - key: readability-identifier-naming.PrivateMemberSuffix + value: _ + - key: readability-identifier-naming.ProtectedMemberPrefix + value: '' + - key: readability-identifier-naming.ProtectedMemberSuffix + value: _ + - key: readability-identifier-naming.StaticConstantCase + value: CamelCase + - key: readability-identifier-naming.StaticConstantPrefix + value: k + - key: readability-identifier-naming.StaticConstantSuffix + value: '' + - key: readability-identifier-naming.StructCase + value: CamelCase + - key: readability-identifier-naming.StructPrefix + value: '' + - key: readability-identifier-naming.StructSuffix + value: '' + - key: readability-identifier-naming.TypeAliasCase + value: CamelCase + - key: readability-identifier-naming.TypeAliasPrefix + value: '' + - key: readability-identifier-naming.TypeAliasSuffix + value: '' + - key: readability-identifier-naming.TypeTemplateParameterCase + value: CamelCase + - key: readability-identifier-naming.TypeTemplateParameterPrefix + value: '' + - key: readability-identifier-naming.TypeTemplateParameterSuffix + value: '' + - key: readability-identifier-naming.TypedefCase + value: CamelCase + - key: readability-identifier-naming.TypedefPrefix + value: '' + - key: readability-identifier-naming.TypedefSuffix + value: '' +... + diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index 251349ff50..576a820536 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -170,7 +170,6 @@ def main(): if not ret: status = ret print("%s ^^^ %s ^^^ %s" % (SEPARATOR, cmd["file"], SEPARATOR)) - break if not status: raise Exception("clang-tidy failed! Refer to the errors above.") return From 210f1a52d6f776b757cbe66c0e655e3953b8533e Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 5 Mar 2020 19:34:34 -0800 Subject: [PATCH 05/55] removed google-* clang tidy rules for now --- cpp/.clang-tidy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/.clang-tidy b/cpp/.clang-tidy index 5a1f76451e..b6de52b766 100644 --- a/cpp/.clang-tidy +++ b/cpp/.clang-tidy @@ -1,5 +1,5 @@ --- -Checks: 'clang-diagnostic-*,clang-analyzer-*,clang-diagnostic-*,clang-analyzer-*,modernize-*,-modernize-make-*,-modernize-raw-string-literal,google-*,-google-default-arguments,-clang-diagnostic-#pragma-messages,readability-identifier-naming,-*,modernize-*,-modernize-make-*,-modernize-raw-string-literal,google-*,-google-default-arguments,-clang-diagnostic-#pragma-messages,readability-identifier-naming' +Checks: 'clang-diagnostic-*,clang-analyzer-*,clang-diagnostic-*,clang-analyzer-*,modernize-*,-modernize-make-*,-modernize-raw-string-literal,-google-*,-clang-diagnostic-#pragma-messages,readability-identifier-naming,-*,modernize-*,-modernize-make-*,-modernize-raw-string-literal,-clang-diagnostic-#pragma-messages,readability-identifier-naming' WarningsAsErrors: '' HeaderFilterRegex: '' AnalyzeTemporaryDtors: false From 671d5d6ee1a19b12c60083da791d76cbde6792c9 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 5 Mar 2020 19:35:02 -0800 Subject: [PATCH 06/55] added a select option to only check for certain source files --- cpp/scripts/run-clang-tidy.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index 576a820536..a7d0cdf905 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -36,8 +36,11 @@ def parse_args(): help="Path to clang-tidy exe") argparser.add_argument("-ignore", type=str, default=None, help="Regex used to ignore files from checking") + argparser.add_argument("-select", type=str, default="cumlHandle.cpp", + help="Regex used to select files for checking") args = argparser.parse_args() args.ignore_compiled = re.compile(args.ignore) if args.ignore else None + args.select_compiled = re.compile(args.select) if args.select else None ret = subprocess.check_output("%s --version" % args.exe, shell=True) ret = ret.decode("utf-8") version = VERSION_REGEX.search(ret) @@ -165,7 +168,10 @@ def main(): # skip files that we don't want to look at if args.ignore_compiled is not None and \ re.search(args.ignore_compiled, cmd["file"]) is not None: - continue + continue + if args.select_compiled is not None and \ + re.search(args.select_compiled, cmd["file"]) is None: + continue ret = run_clang_tidy(cmd, args) if not ret: status = ret From 8e5176daf9ac2941ae6855bf530f857712dd75b0 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 5 Mar 2020 19:38:30 -0800 Subject: [PATCH 07/55] disabled most clang tidy checks for now --- cpp/.clang-tidy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/.clang-tidy b/cpp/.clang-tidy index b6de52b766..87f2942d48 100644 --- a/cpp/.clang-tidy +++ b/cpp/.clang-tidy @@ -1,5 +1,5 @@ --- -Checks: 'clang-diagnostic-*,clang-analyzer-*,clang-diagnostic-*,clang-analyzer-*,modernize-*,-modernize-make-*,-modernize-raw-string-literal,-google-*,-clang-diagnostic-#pragma-messages,readability-identifier-naming,-*,modernize-*,-modernize-make-*,-modernize-raw-string-literal,-clang-diagnostic-#pragma-messages,readability-identifier-naming' +Checks: 'clang-diagnostic-*,clang-analyzer-*,-modernize-*,-google-*,-clang-diagnostic-#pragma-messages,-readability-identifier-naming' WarningsAsErrors: '' HeaderFilterRegex: '' AnalyzeTemporaryDtors: false From 8519b5a0a4be901ffd5439ee4d782cc468e25d68 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 5 Mar 2020 19:42:59 -0800 Subject: [PATCH 08/55] made warnings as errors --- cpp/.clang-tidy | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cpp/.clang-tidy b/cpp/.clang-tidy index 87f2942d48..5a3fedbeff 100644 --- a/cpp/.clang-tidy +++ b/cpp/.clang-tidy @@ -1,10 +1,9 @@ --- Checks: 'clang-diagnostic-*,clang-analyzer-*,-modernize-*,-google-*,-clang-diagnostic-#pragma-messages,-readability-identifier-naming' -WarningsAsErrors: '' +WarningsAsErrors: '*' HeaderFilterRegex: '' AnalyzeTemporaryDtors: false FormatStyle: google -User: snanditale CheckOptions: - key: cert-dcl16-c.NewSuffixes value: 'L;LL;LU;LLU' From 477aae239baf869544b0bd2ca62b150f14c36c7d Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 5 Mar 2020 19:44:52 -0800 Subject: [PATCH 09/55] suppressed switch-case warning too --- cpp/.clang-tidy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/.clang-tidy b/cpp/.clang-tidy index 5a3fedbeff..5107d9189c 100644 --- a/cpp/.clang-tidy +++ b/cpp/.clang-tidy @@ -1,5 +1,5 @@ --- -Checks: 'clang-diagnostic-*,clang-analyzer-*,-modernize-*,-google-*,-clang-diagnostic-#pragma-messages,-readability-identifier-naming' +Checks: 'clang-diagnostic-*,clang-analyzer-*,-modernize-*,-google-*,-clang-diagnostic-#pragma-messages,-readability-identifier-naming,-clang-diagnostic-switch' WarningsAsErrors: '*' HeaderFilterRegex: '' AnalyzeTemporaryDtors: false From f305336bd7421856b63a1af0ed31e49e163f06b9 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 5 Mar 2020 19:56:02 -0800 Subject: [PATCH 10/55] enabled warnings/errors in the included header files too --- cpp/scripts/run-clang-tidy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index a7d0cdf905..6c2189498a 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -135,7 +135,7 @@ def run_clang_tidy_command(tidy_cmd): def run_clang_tidy(cmd, args): command, is_cuda = get_tidy_args(cmd) - tidy_cmd = [args.exe, cmd["file"], "--", ] + tidy_cmd = [args.exe, "-header-filter=.*", cmd["file"], "--", ] tidy_cmd.extend(command) status = True if is_cuda: From 8b9ef203b19515f8ddd7747028e85a9a96ca233b Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 5 Mar 2020 20:16:55 -0800 Subject: [PATCH 11/55] fixed correct option for clang gpu-arch --- cpp/scripts/run-clang-tidy.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index 6c2189498a..afdf794c29 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -36,7 +36,7 @@ def parse_args(): help="Path to clang-tidy exe") argparser.add_argument("-ignore", type=str, default=None, help="Regex used to ignore files from checking") - argparser.add_argument("-select", type=str, default="cumlHandle.cpp", + argparser.add_argument("-select", type=str, default=None, help="Regex used to select files for checking") args = argparser.parse_args() args.ignore_compiled = re.compile(args.ignore) if args.ignore else None @@ -67,8 +67,7 @@ def get_gpu_archs(command): arch_flag = command[loc + 1] match = GPU_ARCH_REGEX.search(arch_flag) if match is not None: - archs.append("--cuda-gpu-arch") - archs.append("sm_%s" % match.group(1)) + archs.append("--cuda-gpu-arch=sm_%s" % match.group(1)) return archs From f3e065d4f1f464a79c2de9a24212c8c04a379eb4 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 5 Mar 2020 20:20:06 -0800 Subject: [PATCH 12/55] optimal packing order for cumlHandle_impl members based on clang-tidy --- cpp/src/common/cumlHandle.cpp | 16 ++++++++++++---- cpp/src/common/cumlHandle.hpp | 21 ++++++++++----------- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/cpp/src/common/cumlHandle.cpp b/cpp/src/common/cumlHandle.cpp index 6514f9cc66..7111f05114 100644 --- a/cpp/src/common/cumlHandle.cpp +++ b/cpp/src/common/cumlHandle.cpp @@ -183,7 +183,18 @@ using MLCommon::defaultDeviceAllocator; using MLCommon::defaultHostAllocator; cumlHandle_impl::cumlHandle_impl(int n_streams) - : _dev_id([]() -> int { + : _cublas_handle(), + _cusolverDn_handle(), + _cusolverSp_handle(), + _cusparse_handle(), + _userStream(nullptr), + _event(), + _deviceAllocator(std::make_shared()), + _hostAllocator(std::make_shared()), + _communicator(), + _streams(), + _prop(), + _dev_id([]() -> int { int cur_dev = -1; CUDA_CHECK(cudaGetDevice(&cur_dev)); return cur_dev; @@ -193,9 +204,6 @@ cumlHandle_impl::cumlHandle_impl(int n_streams) _cusolverDnInitialized(false), _cusolverSpInitialized(false), _cusparseInitialized(false), - _deviceAllocator(std::make_shared()), - _hostAllocator(std::make_shared()), - _userStream(NULL), _devicePropInitialized(false) { createResources(); } diff --git a/cpp/src/common/cumlHandle.hpp b/cpp/src/common/cumlHandle.hpp index 63b24c960f..0b732f4ccd 100644 --- a/cpp/src/common/cumlHandle.hpp +++ b/cpp/src/common/cumlHandle.hpp @@ -74,26 +74,25 @@ class cumlHandle_impl { const cudaDeviceProp& getDeviceProperties() const; private: - const int _dev_id; - const int _num_streams; - std::vector _streams; mutable cublasHandle_t _cublas_handle; - mutable bool _cublasInitialized; mutable cusolverDnHandle_t _cusolverDn_handle; - mutable bool _cusolverDnInitialized; mutable cusolverSpHandle_t _cusolverSp_handle; - mutable bool _cusolverSpInitialized; mutable cusparseHandle_t _cusparse_handle; - mutable bool _cusparseInitialized; - std::shared_ptr _deviceAllocator; - std::shared_ptr _hostAllocator; cudaStream_t _userStream; cudaEvent_t _event; + std::shared_ptr _deviceAllocator; + std::shared_ptr _hostAllocator; + std::shared_ptr _communicator; + std::vector _streams; mutable cudaDeviceProp _prop; + const int _dev_id; + const int _num_streams; + mutable bool _cublasInitialized; + mutable bool _cusolverDnInitialized; + mutable bool _cusolverSpInitialized; + mutable bool _cusparseInitialized; mutable bool _devicePropInitialized; - std::shared_ptr _communicator; - void createResources(); void destroyResources(); }; From 59a9c0376dd4c710901897ab670bf6babed1eb80 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 5 Mar 2020 20:23:29 -0800 Subject: [PATCH 13/55] removed other unknown options to clang --- cpp/scripts/run-clang-tidy.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index afdf794c29..8fd7d7a688 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -78,6 +78,13 @@ def get_index(arr, item): return -1 +def remove_item(arr, item): + loc = get_index(arr, item) + if loc >= 0: + del arr[loc] + return loc + + def remove_item_plus_one(arr, item): loc = get_index(arr, item) if loc >= 0: @@ -119,6 +126,9 @@ def get_tidy_args(cmd): if loc >= 0: command[loc + 1] = "cuda" remove_item_plus_one(command, "-ccbin") + remove_item(command, "--expt-extended-lambda") + remove_item(command, "--diag_suppress=unrecognized_gcc_pragma") + command.append("-nocudalib") command.extend(get_system_includes()) return command, is_cuda From 3f687edce3a52bb10ee175526b6a44dc7dd34e92 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 5 Mar 2020 20:26:13 -0800 Subject: [PATCH 14/55] disabled checks on cuda files for now --- cpp/scripts/run-clang-tidy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index 8fd7d7a688..f74b9ba026 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -34,7 +34,7 @@ def parse_args(): help="Path to cmake-generated compilation database") argparser.add_argument("-exe", type=str, default="clang-tidy", help="Path to clang-tidy exe") - argparser.add_argument("-ignore", type=str, default=None, + argparser.add_argument("-ignore", type=str, default="[.]cu$", help="Regex used to ignore files from checking") argparser.add_argument("-select", type=str, default=None, help="Regex used to select files for checking") From 23cbab3cf0d392e83d05b96f7c8c8dab97c89f1c Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 5 Mar 2020 20:29:39 -0800 Subject: [PATCH 15/55] filter diagnostics for headers from cuml only --- cpp/scripts/run-clang-tidy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index f74b9ba026..2f9584ef60 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -144,7 +144,7 @@ def run_clang_tidy_command(tidy_cmd): def run_clang_tidy(cmd, args): command, is_cuda = get_tidy_args(cmd) - tidy_cmd = [args.exe, "-header-filter=.*", cmd["file"], "--", ] + tidy_cmd = [args.exe, "-header-filter=.*cuml/cpp/.*", cmd["file"], "--", ] tidy_cmd.extend(command) status = True if is_cuda: From 546946fe23207ed4a8d46d1c20ba525519e4cea7 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 5 Mar 2020 20:31:32 -0800 Subject: [PATCH 16/55] fixed clang-tidy error in holtwinters C code --- cpp/src/holtwinters/holtwinters_api.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/cpp/src/holtwinters/holtwinters_api.cpp b/cpp/src/holtwinters/holtwinters_api.cpp index cc765ee145..d9d6a5a4aa 100644 --- a/cpp/src/holtwinters/holtwinters_api.cpp +++ b/cpp/src/holtwinters/holtwinters_api.cpp @@ -25,14 +25,12 @@ cumlError_t cumlHoltWinters_buffer_size(int n, int batch_size, int frequency, int *leveltrend_coef_shift, int *season_coef_shift) { cumlError_t status; - if (status == CUML_SUCCESS) { - try { - ML::HoltWinters::buffer_size( - n, batch_size, frequency, start_leveltrend_len, start_season_len, - components_len, error_len, leveltrend_coef_shift, season_coef_shift); - } catch (...) { - status = CUML_ERROR_UNKNOWN; - } + try { + ML::HoltWinters::buffer_size( + n, batch_size, frequency, start_leveltrend_len, start_season_len, + components_len, error_len, leveltrend_coef_shift, season_coef_shift); + } catch (...) { + status = CUML_ERROR_UNKNOWN; } return status; } From 1fb02ad3ee5da3c5b9ab369651546f6baccc9633 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 5 Mar 2020 20:43:41 -0800 Subject: [PATCH 17/55] currently do not consider examples/ folder for clang tidy checks --- cpp/scripts/run-clang-tidy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index 2f9584ef60..d192e988bd 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -34,7 +34,7 @@ def parse_args(): help="Path to cmake-generated compilation database") argparser.add_argument("-exe", type=str, default="clang-tidy", help="Path to clang-tidy exe") - argparser.add_argument("-ignore", type=str, default="[.]cu$", + argparser.add_argument("-ignore", type=str, default="[.]cu$|examples/", help="Regex used to ignore files from checking") argparser.add_argument("-select", type=str, default=None, help="Regex used to select files for checking") From 60f54a0d4683002d4bb96f02aaf79a6ad60fa01b Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 5 Mar 2020 20:44:24 -0800 Subject: [PATCH 18/55] fixed clang tidy errors --- cpp/src/holtwinters/holtwinters_api.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/cpp/src/holtwinters/holtwinters_api.cpp b/cpp/src/holtwinters/holtwinters_api.cpp index d9d6a5a4aa..43971345b3 100644 --- a/cpp/src/holtwinters/holtwinters_api.cpp +++ b/cpp/src/holtwinters/holtwinters_api.cpp @@ -29,6 +29,7 @@ cumlError_t cumlHoltWinters_buffer_size(int n, int batch_size, int frequency, ML::HoltWinters::buffer_size( n, batch_size, frequency, start_leveltrend_len, start_season_len, components_len, error_len, leveltrend_coef_shift, season_coef_shift); + status = CUML_SUCCESS; } catch (...) { status = CUML_ERROR_UNKNOWN; } From 1f9d3301158d051b022fc8e16802dab4657b609b Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 12 Mar 2020 00:08:14 -0700 Subject: [PATCH 19/55] removed google clang-tidy rules, for now --- cpp/.clang-tidy | 34 ++-------------------------------- 1 file changed, 2 insertions(+), 32 deletions(-) diff --git a/cpp/.clang-tidy b/cpp/.clang-tidy index 5107d9189c..fea01ccea0 100644 --- a/cpp/.clang-tidy +++ b/cpp/.clang-tidy @@ -1,44 +1,14 @@ --- -Checks: 'clang-diagnostic-*,clang-analyzer-*,-modernize-*,-google-*,-clang-diagnostic-#pragma-messages,-readability-identifier-naming,-clang-diagnostic-switch' +Checks: 'clang-diagnostic-*,clang-analyzer-*,-modernize-*,-clang-diagnostic-#pragma-messages,-readability-identifier-naming,-clang-diagnostic-switch' WarningsAsErrors: '*' HeaderFilterRegex: '' AnalyzeTemporaryDtors: false -FormatStyle: google +FormatStyle: none CheckOptions: - key: cert-dcl16-c.NewSuffixes value: 'L;LL;LU;LLU' - key: cppcoreguidelines-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic value: '1' - - key: google-build-namespaces.HeaderFileExtensions - value: ',h,hh,hpp,hxx' - - key: google-global-names-in-headers.HeaderFileExtensions - value: ',h,hh,hpp,hxx' - - key: google-readability-braces-around-statements.ShortStatementLines - value: '1' - - key: google-readability-function-size.BranchThreshold - value: '4294967295' - - key: google-readability-function-size.LineThreshold - value: '4294967295' - - key: google-readability-function-size.NestingThreshold - value: '4294967295' - - key: google-readability-function-size.ParameterThreshold - value: '4294967295' - - key: google-readability-function-size.StatementThreshold - value: '800' - - key: google-readability-function-size.VariableThreshold - value: '4294967295' - - key: google-readability-namespace-comments.ShortNamespaceLines - value: '10' - - key: google-readability-namespace-comments.SpacesBeforeComments - value: '2' - - key: google-runtime-int.SignedTypePrefix - value: int - - key: google-runtime-int.TypeSuffix - value: '' - - key: google-runtime-int.UnsignedTypePrefix - value: uint - - key: google-runtime-references.WhiteListTypes - value: '' - key: modernize-loop-convert.MaxCopySize value: '16' - key: modernize-loop-convert.MinConfidence From a3705ee4fb8af9996441d5c4e8295911d0749da6 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 12 Mar 2020 11:31:29 -0700 Subject: [PATCH 20/55] use clang-based system headers instead of the ones from gcc toolchain --- cpp/scripts/run-clang-tidy.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index d192e988bd..b5f9196079 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -93,15 +93,14 @@ def remove_item_plus_one(arr, item): return loc -# currently assumes g++ as the compiler -def get_system_includes(): - ret = subprocess.check_output("echo | g++ -E -Wp,-v - 2>&1", shell=True) - ret = ret.decode("utf-8") - incs = [] - for line in ret.split(os.linesep): - if len(line) > 0 and line[0] == " ": - incs.extend(["-I", line[1:]]) - return incs +def get_clang_includes(): + dir = os.getenv("CONDA_PREFIX") + if dir is None: + ret = subprocess.check_output("which %s 2>&1" % exe, shell=True) + ret = ret.decode("utf-8") + dir = os.path.dirname(os.path.dirname(ret)) + header = os.path.join(dir, "include", "Headers") + return ["-I", header] def get_tidy_args(cmd): @@ -129,7 +128,7 @@ def get_tidy_args(cmd): remove_item(command, "--expt-extended-lambda") remove_item(command, "--diag_suppress=unrecognized_gcc_pragma") command.append("-nocudalib") - command.extend(get_system_includes()) + command.extend(get_clang_includes()) return command, is_cuda From cfae269c282f8af02770432bcd70ae071a218c3f Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 12 Mar 2020 20:50:53 -0700 Subject: [PATCH 21/55] updated the location of latest clang headers --- cpp/scripts/run-clang-tidy.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index b5f9196079..d5eb8ddde1 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -93,17 +93,17 @@ def remove_item_plus_one(arr, item): return loc -def get_clang_includes(): +def get_clang_includes(exe): dir = os.getenv("CONDA_PREFIX") if dir is None: ret = subprocess.check_output("which %s 2>&1" % exe, shell=True) ret = ret.decode("utf-8") dir = os.path.dirname(os.path.dirname(ret)) - header = os.path.join(dir, "include", "Headers") + header = os.path.join(dir, "include", "ClangHeaders") return ["-I", header] -def get_tidy_args(cmd): +def get_tidy_args(cmd, exe): command, file = cmd["command"], cmd["file"] is_cuda = file.endswith(".cu") command = re.split(SPACES, command) @@ -128,7 +128,7 @@ def get_tidy_args(cmd): remove_item(command, "--expt-extended-lambda") remove_item(command, "--diag_suppress=unrecognized_gcc_pragma") command.append("-nocudalib") - command.extend(get_clang_includes()) + command.extend(get_clang_includes(exe)) return command, is_cuda @@ -142,7 +142,7 @@ def run_clang_tidy_command(tidy_cmd): def run_clang_tidy(cmd, args): - command, is_cuda = get_tidy_args(cmd) + command, is_cuda = get_tidy_args(cmd, args.exe) tidy_cmd = [args.exe, "-header-filter=.*cuml/cpp/.*", cmd["file"], "--", ] tidy_cmd.extend(command) status = True From b1956e94fac1a30f966f0e0ccdb77caf95c0a08a Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 12 Mar 2020 20:53:00 -0700 Subject: [PATCH 22/55] enabled compilation database generation in cmake for clang-tidy purposes --- cpp/CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index 778930c72f..80856927ab 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -36,6 +36,9 @@ if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES) "Debug" "Release") endif() +# this is needed for clang-tidy runs +set(CMAKE_EXPORT_COMPILE_COMMANDS ON) + ############################################################################## # - User Options ------------------------------------------------------------ From 4869ad7e29d6a924ef56603edb132c580eef2bdb Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 12 Mar 2020 21:10:57 -0700 Subject: [PATCH 23/55] removed nocudalib option --- cpp/scripts/run-clang-tidy.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index d5eb8ddde1..3316c2954d 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -127,7 +127,6 @@ def get_tidy_args(cmd, exe): remove_item_plus_one(command, "-ccbin") remove_item(command, "--expt-extended-lambda") remove_item(command, "--diag_suppress=unrecognized_gcc_pragma") - command.append("-nocudalib") command.extend(get_clang_includes(exe)) return command, is_cuda From 344f4ac466cba0cde3ea622f7d652c0d3dff6e4a Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 12 Mar 2020 21:14:07 -0700 Subject: [PATCH 24/55] -j option for parallel compilation --- cpp/scripts/run-clang-tidy.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index 3316c2954d..04f3c8affb 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -20,6 +20,7 @@ import subprocess import argparse import json +import multiprocessing as mp VERSION_REGEX = re.compile(r" LLVM version ([0-9.]+)") @@ -38,7 +39,11 @@ def parse_args(): help="Regex used to ignore files from checking") argparser.add_argument("-select", type=str, default=None, help="Regex used to select files for checking") + argparser.add_argument("-j", type=int, default=1, + help="Number of parallel jobs to launch.") args = argparser.parse_args() + if args.j <= 0: + args.j = mp.cpu_count() args.ignore_compiled = re.compile(args.ignore) if args.ignore else None args.select_compiled = re.compile(args.select) if args.select else None ret = subprocess.check_output("%s --version" % args.exe, shell=True) From ffc5825c05f731dbf3a96b4d3a8d15658186256e Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 12 Mar 2020 21:33:02 -0700 Subject: [PATCH 25/55] relaxed the ignore regex slightly --- cpp/scripts/run-clang-tidy.py | 39 +++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index 04f3c8affb..5b88bd0ec2 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -35,7 +35,7 @@ def parse_args(): help="Path to cmake-generated compilation database") argparser.add_argument("-exe", type=str, default="clang-tidy", help="Path to clang-tidy exe") - argparser.add_argument("-ignore", type=str, default="[.]cu$|examples/", + argparser.add_argument("-ignore", type=str, default="[.]cu$|examples/kmeans/", help="Regex used to ignore files from checking") argparser.add_argument("-select", type=str, default=None, help="Regex used to select files for checking") @@ -137,12 +137,15 @@ def get_tidy_args(cmd, exe): def run_clang_tidy_command(tidy_cmd): + out = "" try: - cmd = " ".join(tidy_cmd) - subprocess.check_call(cmd, shell=True) - return True + cmd = " ".join(tidy_cmd) + " 2>&1" + out = subprocess.check_output(cmd, shell=True) + out = out.decode("utf-8") + out = out.rstrip() + return True, out except: - return False + return False, out def run_clang_tidy(cmd, args): @@ -150,22 +153,27 @@ def run_clang_tidy(cmd, args): tidy_cmd = [args.exe, "-header-filter=.*cuml/cpp/.*", cmd["file"], "--", ] tidy_cmd.extend(command) status = True + out = "" if is_cuda: tidy_cmd.append("--cuda-device-only") tidy_cmd.append(cmd["file"]) - ret = run_clang_tidy_command(tidy_cmd) + ret, out1 = run_clang_tidy_command(tidy_cmd) + out += out1 + out += "%s" % SEPARATOR if not ret: status = ret tidy_cmd[-2] = "--cuda-host-only" - ret = run_clang_tidy_command(tidy_cmd) + ret, out1 = run_clang_tidy_command(tidy_cmd) if not ret: status = ret + out += out1 else: tidy_cmd.append(cmd["file"]) - ret = run_clang_tidy_command(tidy_cmd) + ret, out1 = run_clang_tidy_command(tidy_cmd) if not ret: status = ret - return status + out += out1 + return status, out def main(): @@ -184,10 +192,15 @@ def main(): if args.select_compiled is not None and \ re.search(args.select_compiled, cmd["file"]) is None: continue - ret = run_clang_tidy(cmd, args) - if not ret: - status = ret - print("%s ^^^ %s ^^^ %s" % (SEPARATOR, cmd["file"], SEPARATOR)) + passed, stdout = run_clang_tidy(cmd, args) + status_str = "PASSED" if passed else "FAILED" + if not passed: + status = False + print("%s File:%s %s %s" % (SEPARATOR, cmd["file"], status_str, + SEPARATOR)) + if stdout: + print(stdout) + print("%s File:%s ENDS %s" % (SEPARATOR, cmd["file"], SEPARATOR)) if not status: raise Exception("clang-tidy failed! Refer to the errors above.") return From e58e2b80d3f4bfc8640d1b9f270cb294e33719a3 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 12 Mar 2020 21:45:43 -0700 Subject: [PATCH 26/55] parallel processing of clang-tidy targets for runtime reduction --- cpp/scripts/run-clang-tidy.py | 44 ++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index 5b88bd0ec2..0f9d79597f 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -173,7 +173,29 @@ def run_clang_tidy(cmd, args): if not ret: status = ret out += out1 - return status, out + return status, out, cmd["file"] + + +# yikes! global var :( +results = [] +def collect_result(result): + global results + results.append(result) + + +def print_results(): + global results + status = True + for passed, stdout, file in results: + status_str = "PASSED" if passed else "FAILED" + if not passed: + status = False + print("%s File:%s %s %s" % (SEPARATOR, file, status_str, SEPARATOR)) + if stdout: + print(stdout) + print("%s File:%s ENDS %s" % (SEPARATOR, file, SEPARATOR)) + if not status: + raise Exception("clang-tidy failed! Refer to the errors above.") def main(): @@ -182,8 +204,9 @@ def main(): if not os.path.exists(".git"): raise Exception("This needs to always be run from the root of repo") all_files = list_all_cmds(args.cdb) + pool = mp.Pool(args.j) + results = [] # actual tidy checker - status = True for cmd in all_files: # skip files that we don't want to look at if args.ignore_compiled is not None and \ @@ -192,18 +215,11 @@ def main(): if args.select_compiled is not None and \ re.search(args.select_compiled, cmd["file"]) is None: continue - passed, stdout = run_clang_tidy(cmd, args) - status_str = "PASSED" if passed else "FAILED" - if not passed: - status = False - print("%s File:%s %s %s" % (SEPARATOR, cmd["file"], status_str, - SEPARATOR)) - if stdout: - print(stdout) - print("%s File:%s ENDS %s" % (SEPARATOR, cmd["file"], SEPARATOR)) - if not status: - raise Exception("clang-tidy failed! Refer to the errors above.") - return + pool.apply_async(run_clang_tidy, args=(cmd, args), + callback=collect_result) + pool.close() + pool.join() + print_results() if __name__ == "__main__": From 1f6d9c3dc1e2d2daceb13723df7c670090e9bc3c Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 12 Mar 2020 21:47:19 -0700 Subject: [PATCH 27/55] by default do parallel clang-tidy processing --- cpp/scripts/run-clang-tidy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index 0f9d79597f..dd3bb0f0fd 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -39,7 +39,7 @@ def parse_args(): help="Regex used to ignore files from checking") argparser.add_argument("-select", type=str, default=None, help="Regex used to select files for checking") - argparser.add_argument("-j", type=int, default=1, + argparser.add_argument("-j", type=int, default=-1, help="Number of parallel jobs to launch.") args = argparser.parse_args() if args.j <= 0: From b06171b20e989416eaf9ea30ecb31079368ab6ea Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 2 Apr 2020 00:56:50 -0700 Subject: [PATCH 28/55] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9c3ed6510..a856113c69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - PR #1947: Cleaning up cmake - PR #1927: Use Cython's `new_build_ext` (if available) - PR #1946: Removed zlib dependency from cmake +- PR #1945: enable clang tidy ## Bug Fixes - PR #1939: Fix syntax error in cuml.common.array From 4d25d0dd09ed883f6e9d30fa7ffdaa3ddfe96ce3 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 2 Apr 2020 00:58:36 -0700 Subject: [PATCH 29/55] clang format updates --- cpp/src/holtwinters/holtwinters_api.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cpp/src/holtwinters/holtwinters_api.cpp b/cpp/src/holtwinters/holtwinters_api.cpp index 43971345b3..e1966363e4 100644 --- a/cpp/src/holtwinters/holtwinters_api.cpp +++ b/cpp/src/holtwinters/holtwinters_api.cpp @@ -26,9 +26,9 @@ cumlError_t cumlHoltWinters_buffer_size(int n, int batch_size, int frequency, int *season_coef_shift) { cumlError_t status; try { - ML::HoltWinters::buffer_size( - n, batch_size, frequency, start_leveltrend_len, start_season_len, - components_len, error_len, leveltrend_coef_shift, season_coef_shift); + ML::HoltWinters::buffer_size(n, batch_size, frequency, start_leveltrend_len, + start_season_len, components_len, error_len, + leveltrend_coef_shift, season_coef_shift); status = CUML_SUCCESS; } catch (...) { status = CUML_ERROR_UNKNOWN; From 9161a0ecd038db99b877e48ccd241ebc4e3e5af3 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 2 Apr 2020 01:04:37 -0700 Subject: [PATCH 30/55] install clang and clang-tools needed for clang-tidy checker --- ci/gpu/build.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ci/gpu/build.sh b/ci/gpu/build.sh index 9125c03b60..c4de87aa3a 100644 --- a/ci/gpu/build.sh +++ b/ci/gpu/build.sh @@ -61,7 +61,9 @@ conda install -c conda-forge -c rapidsai -c rapidsai-nightly -c nvidia \ "ucx-py=${MINOR_VERSION}" \ "statsmodels" \ "xgboost====1.0.2dev.rapidsai0.13" \ - "lightgbm" + "lightgbm" \ + "clang=8" \ + "clang-tools=8" # Install the master version of dask, distributed, and dask-ml From d35664290614f0df844b8f98eb2430bb4b289ab7 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 2 Apr 2020 01:06:02 -0700 Subject: [PATCH 31/55] updated clang-tidy expected version to 8.0.1 --- cpp/scripts/run-clang-tidy.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index dd3bb0f0fd..4b8a394f17 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -23,6 +23,7 @@ import multiprocessing as mp +EXPECTED_VERSION = "8.0.1" VERSION_REGEX = re.compile(r" LLVM version ([0-9.]+)") GPU_ARCH_REGEX = re.compile(r"sm_(\d+)") SPACES = re.compile(r"\s+") @@ -52,8 +53,9 @@ def parse_args(): if version is None: raise Exception("Failed to figure out clang-tidy version!") version = version.group(1) - if version != "8.0.0": - raise Exception("clang-tidy exe must be v8.0.0 found '%s'" % version) + if version != EXPECTED_VERSION: + raise Exception("clang-tidy exe must be v%s found '%s'" % \ + (EXPECTED_VERSION, version)) if not os.path.exists(args.cdb): raise Exception("Compilation database '%s' missing" % args.cdb) return args From d4d34294bbf30067c4a9fba14b603051044a129c Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 2 Apr 2020 01:34:31 -0700 Subject: [PATCH 32/55] explicit code paths to run tidy checks sequentially and parallely --- cpp/scripts/run-clang-tidy.py | 62 ++++++++++++++++++++++++++--------- 1 file changed, 46 insertions(+), 16 deletions(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index 4b8a394f17..07b8c15229 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -141,7 +141,7 @@ def get_tidy_args(cmd, exe): def run_clang_tidy_command(tidy_cmd): out = "" try: - cmd = " ".join(tidy_cmd) + " 2>&1" + cmd = " ".join(tidy_cmd) out = subprocess.check_output(cmd, shell=True) out = out.decode("utf-8") out = out.rstrip() @@ -185,29 +185,45 @@ def collect_result(result): results.append(result) +def print_result(passed, stdout, file): + status_str = "PASSED" if passed else "FAILED" + print("%s File:%s %s %s" % (SEPARATOR, file, status_str, SEPARATOR)) + if stdout: + print(stdout) + print("%s File:%s ENDS %s" % (SEPARATOR, file, SEPARATOR)) + + def print_results(): global results status = True for passed, stdout, file in results: - status_str = "PASSED" if passed else "FAILED" + print_result(passed, stdout, file) if not passed: status = False - print("%s File:%s %s %s" % (SEPARATOR, file, status_str, SEPARATOR)) - if stdout: - print(stdout) - print("%s File:%s ENDS %s" % (SEPARATOR, file, SEPARATOR)) - if not status: - raise Exception("clang-tidy failed! Refer to the errors above.") + return status -def main(): - args = parse_args() - # Attempt to making sure that we run this script from root of repo always - if not os.path.exists(".git"): - raise Exception("This needs to always be run from the root of repo") - all_files = list_all_cmds(args.cdb) +# mostly used for debugging purposes +def run_sequential(args, all_files): + status = True + # actual tidy checker + for cmd in all_files: + # skip files that we don't want to look at + if args.ignore_compiled is not None and \ + re.search(args.ignore_compiled, cmd["file"]) is not None: + continue + if args.select_compiled is not None and \ + re.search(args.select_compiled, cmd["file"]) is None: + continue + passed, stdout, file = run_clang_tidy(cmd, args) + print_result(passed, stdout, file) + if not passed: + status = False + return status + + +def run_parallel(args, all_files): pool = mp.Pool(args.j) - results = [] # actual tidy checker for cmd in all_files: # skip files that we don't want to look at @@ -221,7 +237,21 @@ def main(): callback=collect_result) pool.close() pool.join() - print_results() + return print_results() + + +def main(): + args = parse_args() + # Attempt to making sure that we run this script from root of repo always + if not os.path.exists(".git"): + raise Exception("This needs to always be run from the root of repo") + all_files = list_all_cmds(args.cdb) + if args.j == 1: + status = run_sequential(args, all_files) + else: + status = run_parallel(args, all_files) + if not status: + raise Exception("clang-tidy failed! Refer to the errors above.") if __name__ == "__main__": From e2fbee446a9e525393ff8c5989ae2b4cf2b75dca Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 2 Apr 2020 01:39:29 -0700 Subject: [PATCH 33/55] usage of subprocess.run in order to capture clang-tidy output accurately --- cpp/scripts/run-clang-tidy.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index 07b8c15229..8d50d2bd5e 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -139,15 +139,16 @@ def get_tidy_args(cmd, exe): def run_clang_tidy_command(tidy_cmd): - out = "" - try: - cmd = " ".join(tidy_cmd) - out = subprocess.check_output(cmd, shell=True) - out = out.decode("utf-8") - out = out.rstrip() - return True, out - except: - return False, out + cmd = " ".join(tidy_cmd) + result = subprocess.run(cmd, check=False, shell=True, + stdout=subprocess.PIPE, stderr=subprocess.STDOUT) + status = result.returncode == 0 + if status: + out = "" + else: + out = "CMD: " + cmd + out += result.stdout.decode("utf-8").rstrip() + return status, out def run_clang_tidy(cmd, args): From 689c192d699b2c6e354d4f7a8b6836e92a5befd7 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 2 Apr 2020 03:34:47 -0700 Subject: [PATCH 34/55] added run-clang-tidy command inside ci/gpu/build.sh --- ci/gpu/build.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ci/gpu/build.sh b/ci/gpu/build.sh index c4de87aa3a..edf9bd4ef6 100644 --- a/ci/gpu/build.sh +++ b/ci/gpu/build.sh @@ -110,6 +110,11 @@ python setup.py install cd $WORKSPACE +################################################################################ +# CHECK - Run clang-tidy +################################################################################ + +python ./cpp/scripts/run-clang-tidy.py -cdb ./cpp/build/compile_commands.json ################################################################################ # TEST - Run GoogleTest and py.tests for libcuml and cuML From a7ef8584f08467278af7b5783478c64793d48426 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Tue, 7 Apr 2020 03:51:28 -0700 Subject: [PATCH 35/55] ENH removed clang and clang-tools installation as they are already in the base image of CI --- ci/gpu/build.sh | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ci/gpu/build.sh b/ci/gpu/build.sh index edf9bd4ef6..a7263f6427 100644 --- a/ci/gpu/build.sh +++ b/ci/gpu/build.sh @@ -61,9 +61,7 @@ conda install -c conda-forge -c rapidsai -c rapidsai-nightly -c nvidia \ "ucx-py=${MINOR_VERSION}" \ "statsmodels" \ "xgboost====1.0.2dev.rapidsai0.13" \ - "lightgbm" \ - "clang=8" \ - "clang-tools=8" + "lightgbm" # Install the master version of dask, distributed, and dask-ml From bc335a9bcbf207cd72d6977fef13f91b969bd6ec Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Wed, 6 May 2020 23:15:58 -0700 Subject: [PATCH 36/55] ENH removed code duplication between parallel and sequential paths --- cpp/scripts/run-clang-tidy.py | 53 +++++++++++++---------------------- 1 file changed, 20 insertions(+), 33 deletions(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index 8d50d2bd5e..8b42928622 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -32,8 +32,12 @@ def parse_args(): argparser = argparse.ArgumentParser("Runs clang-tidy on a project") - argparser.add_argument("-cdb", type=str, default="compile_commands.json", - help="Path to cmake-generated compilation database") + argparser.add_argument("-cdb", type=str, + default="cpp/build/compile_commands.json", + help="Path to cmake-generated compilation database" + " file. It is always found inside the root of the " + "cmake build folder. So make sure that `cmake` has " + "been run once before running this script!") argparser.add_argument("-exe", type=str, default="clang-tidy", help="Path to clang-tidy exe") argparser.add_argument("-ignore", type=str, default="[.]cu$|examples/kmeans/", @@ -61,7 +65,7 @@ def parse_args(): return args -def list_all_cmds(cdb): +def get_all_commands(cdb): with open(cdb, "r") as fp: return json.load(fp) @@ -204,9 +208,8 @@ def print_results(): return status -# mostly used for debugging purposes -def run_sequential(args, all_files): - status = True +def run_tidy_for_all_files(args, all_files): + pool = None if args.j == 1 else mp.Pool(args.j) # actual tidy checker for cmd in all_files: # skip files that we don't want to look at @@ -216,28 +219,15 @@ def run_sequential(args, all_files): if args.select_compiled is not None and \ re.search(args.select_compiled, cmd["file"]) is None: continue - passed, stdout, file = run_clang_tidy(cmd, args) - print_result(passed, stdout, file) - if not passed: - status = False - return status - - -def run_parallel(args, all_files): - pool = mp.Pool(args.j) - # actual tidy checker - for cmd in all_files: - # skip files that we don't want to look at - if args.ignore_compiled is not None and \ - re.search(args.ignore_compiled, cmd["file"]) is not None: - continue - if args.select_compiled is not None and \ - re.search(args.select_compiled, cmd["file"]) is None: - continue - pool.apply_async(run_clang_tidy, args=(cmd, args), - callback=collect_result) - pool.close() - pool.join() + if pool is not None: + pool.apply_async(run_clang_tidy, args=(cmd, args), + callback=collect_result) + else: + passed, stdout, file = run_clang_tidy(cmd, args) + collect_result((passed, stdout, file)) + if pool is not None: + pool.close() + pool.join() return print_results() @@ -246,11 +236,8 @@ def main(): # Attempt to making sure that we run this script from root of repo always if not os.path.exists(".git"): raise Exception("This needs to always be run from the root of repo") - all_files = list_all_cmds(args.cdb) - if args.j == 1: - status = run_sequential(args, all_files) - else: - status = run_parallel(args, all_files) + all_files = get_all_commands(args.cdb) + status = run_tidy_for_all_files(args, all_files) if not status: raise Exception("clang-tidy failed! Refer to the errors above.") From d56ced1f6a63bd0e2556d54bc7aba0bb3646c37b Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 7 May 2020 09:13:18 +0000 Subject: [PATCH 37/55] FIX do not lint spdlog headers --- cpp/src/common/logger.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/src/common/logger.cpp b/cpp/src/common/logger.cpp index 311a6ab59b..d62781d323 100644 --- a/cpp/src/common/logger.cpp +++ b/cpp/src/common/logger.cpp @@ -14,8 +14,8 @@ * limitations under the License. */ #define SPDLOG_HEADER_ONLY -#include -#include +#include // NOLINT +#include // NOLINT #include From 4c2c1a5c9513a19fac5a9e8d6e4f0773a9361cc3 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Thu, 7 May 2020 09:14:05 +0000 Subject: [PATCH 38/55] FIX clang-format fixes --- cpp/src/common/logger.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/common/logger.cpp b/cpp/src/common/logger.cpp index d62781d323..9aef064a40 100644 --- a/cpp/src/common/logger.cpp +++ b/cpp/src/common/logger.cpp @@ -15,7 +15,7 @@ */ #define SPDLOG_HEADER_ONLY #include // NOLINT -#include // NOLINT +#include // NOLINT #include From 395aee8805558c10587d60fc21ea8c9a1d63603c Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Fri, 8 May 2020 11:56:43 -0700 Subject: [PATCH 39/55] CI moved tidy checker logic from gpu/build.sh to checks/style.sh --- ci/checks/style.sh | 25 ++++++++++++++++++++++++- ci/gpu/build.sh | 6 ------ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/ci/checks/style.sh b/ci/checks/style.sh index 23c7c714d2..7785a6c151 100644 --- a/ci/checks/style.sh +++ b/ci/checks/style.sh @@ -66,7 +66,6 @@ else fi # Check for a consistent code format -# TODO: keep adding more dirs when we add more source folders in cuml FORMAT=`python cpp/scripts/run-clang-format.py 2>&1` FORMAT_RETVAL=$? if [ "$RETVAL" = "0" ]; then @@ -82,4 +81,28 @@ else echo -e "\n\n>>>> PASSED: clang format check\n\n" fi +# clang-tidy checkFORMAT=`python cpp/scripts/run-clang-format.py 2>&1` +function setup_and_run_clang_tidy() { + mkdir cpp/build && \ + cd cpp/build && \ + cmake .. && \ + cd ../.. && \ + python cpp/scripts/run-clang-tidy.py +} +TIDY=`setup_and_run_clang_tidy 2>&1` +TIDY_RETVAL=$? +if [ "$RETVAL" = "0" ]; then + RETVAL=$TIDY_RETVAL +fi + +# Output results if failure otherwise show pass +if [ "$TIDY_RETVAL" != "0" ]; then + echo -e "\n\n>>>> FAILED: clang tidy check; begin output\n\n" + echo -e "$TIDY" + echo -e "\n\n>>>> FAILED: clang tidy check; end output\n\n" +else + echo -e "\n\n>>>> PASSED: clang tidy check\n\n" +fi + + exit $RETVAL diff --git a/ci/gpu/build.sh b/ci/gpu/build.sh index e537f47214..4ef80a0fce 100644 --- a/ci/gpu/build.sh +++ b/ci/gpu/build.sh @@ -108,12 +108,6 @@ python setup.py install cd $WORKSPACE -################################################################################ -# CHECK - Run clang-tidy -################################################################################ - -python ./cpp/scripts/run-clang-tidy.py -cdb ./cpp/build/compile_commands.json - ################################################################################ # TEST - Run GoogleTest and py.tests for libcuml and cuML ################################################################################ From 56e2362a94d1e49cfd77f936989901978ef79dbb Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Fri, 8 May 2020 19:36:47 -0700 Subject: [PATCH 40/55] FIX help discover nvcc during the check-style CI --- ci/checks/style.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/checks/style.sh b/ci/checks/style.sh index 7785a6c151..447bdf0117 100644 --- a/ci/checks/style.sh +++ b/ci/checks/style.sh @@ -6,7 +6,7 @@ # Ignore errors and set path set +e -PATH=/conda/bin:$PATH +export PATH=/conda/bin:/usr/local/cuda/bin:$PATH # Activate common conda env source activate gdf From 932de36051c17524ffc46c3ffd26b716c160a980 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Fri, 8 May 2020 19:45:36 -0700 Subject: [PATCH 41/55] FIX do not evaluate_gpu_archs in check-style CI --- ci/checks/style.sh | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ci/checks/style.sh b/ci/checks/style.sh index 447bdf0117..c10f66992d 100644 --- a/ci/checks/style.sh +++ b/ci/checks/style.sh @@ -81,11 +81,14 @@ else echo -e "\n\n>>>> PASSED: clang format check\n\n" fi -# clang-tidy checkFORMAT=`python cpp/scripts/run-clang-format.py 2>&1` +# clang-tidy check +# NOTE: +# explicitly pass GPU_ARCHS flag to avoid having to evaluate gpu archs +# because there's no GPU on the CI machine where this script runs! function setup_and_run_clang_tidy() { mkdir cpp/build && \ cd cpp/build && \ - cmake .. && \ + cmake -DGPU_ARCHS=ALL .. && \ cd ../.. && \ python cpp/scripts/run-clang-tidy.py } From 9a7f9cf6bd5badafa9c1324f4e3df0034d29a09e Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Fri, 8 May 2020 19:52:22 -0700 Subject: [PATCH 42/55] FIX download all dependencies in check-styl CI before starting clang-tidy --- ci/checks/style.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ci/checks/style.sh b/ci/checks/style.sh index c10f66992d..9eff69f24d 100644 --- a/ci/checks/style.sh +++ b/ci/checks/style.sh @@ -85,10 +85,14 @@ fi # NOTE: # explicitly pass GPU_ARCHS flag to avoid having to evaluate gpu archs # because there's no GPU on the CI machine where this script runs! +# NOTE: +# also, sync all dependencies as they'll be needed by clang-tidy to find +# relevant headers function setup_and_run_clang_tidy() { mkdir cpp/build && \ cd cpp/build && \ cmake -DGPU_ARCHS=ALL .. && \ + make benchmark && \ cd ../.. && \ python cpp/scripts/run-clang-tidy.py } From f0fb06fd7ed3215e4b23b299a6554c13f3b6e31f Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Fri, 8 May 2020 20:01:38 -0700 Subject: [PATCH 43/55] FIX added faiss blas libaries option to cmake command in style.sh --- ci/checks/style.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ci/checks/style.sh b/ci/checks/style.sh index 9eff69f24d..9bd770404e 100644 --- a/ci/checks/style.sh +++ b/ci/checks/style.sh @@ -91,7 +91,9 @@ fi function setup_and_run_clang_tidy() { mkdir cpp/build && \ cd cpp/build && \ - cmake -DGPU_ARCHS=ALL .. && \ + cmake -DGPU_ARCHS=ALL \ + -DBLAS_LIBRARIES=${CONDA_PREFIX}/lib/libopenblas.so.0 \ + .. && \ make benchmark && \ cd ../.. && \ python cpp/scripts/run-clang-tidy.py From fe4a10c15a2d43cbcbd954bad1a3395814212c4b Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Fri, 8 May 2020 20:03:06 -0700 Subject: [PATCH 44/55] FIX reordered dependencies to simplify clang-tidy check setup in style.sh --- cpp/cmake/Dependencies.cmake | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/cpp/cmake/Dependencies.cmake b/cpp/cmake/Dependencies.cmake index 5f272a9cd3..9c960017d6 100644 --- a/cpp/cmake/Dependencies.cmake +++ b/cpp/cmake/Dependencies.cmake @@ -219,8 +219,10 @@ set_property(TARGET benchmarklib PROPERTY add_dependencies(cub raft) add_dependencies(cutlass cub) add_dependencies(spdlog cutlass) -add_dependencies(faiss spdlog) +add_dependencies(googletest spdlog) +add_dependencies(benchmark googletest) +# NOTE! only add newer dependencies from here onwards! +# DO NOT modify the above as they are assumed for CI's clang-tidy check +add_dependencies(faiss benchmark) add_dependencies(faisslib faiss) add_dependencies(treelite faiss) -add_dependencies(googletest treelite) -add_dependencies(benchmark googletest) From 2423a5ce002b52fd6c6c759b32c19ea42b441f8e Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Fri, 8 May 2020 20:10:37 -0700 Subject: [PATCH 45/55] FIX download all dependencies for clang-tidy to run correctly in style.sh CI --- ci/checks/style.sh | 2 +- cpp/cmake/Dependencies.cmake | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/ci/checks/style.sh b/ci/checks/style.sh index 9bd770404e..638a8039a2 100644 --- a/ci/checks/style.sh +++ b/ci/checks/style.sh @@ -94,7 +94,7 @@ function setup_and_run_clang_tidy() { cmake -DGPU_ARCHS=ALL \ -DBLAS_LIBRARIES=${CONDA_PREFIX}/lib/libopenblas.so.0 \ .. && \ - make benchmark && \ + make treelite && \ cd ../.. && \ python cpp/scripts/run-clang-tidy.py } diff --git a/cpp/cmake/Dependencies.cmake b/cpp/cmake/Dependencies.cmake index 9c960017d6..87c19bdd6e 100644 --- a/cpp/cmake/Dependencies.cmake +++ b/cpp/cmake/Dependencies.cmake @@ -221,8 +221,6 @@ add_dependencies(cutlass cub) add_dependencies(spdlog cutlass) add_dependencies(googletest spdlog) add_dependencies(benchmark googletest) -# NOTE! only add newer dependencies from here onwards! -# DO NOT modify the above as they are assumed for CI's clang-tidy check add_dependencies(faiss benchmark) add_dependencies(faisslib faiss) add_dependencies(treelite faiss) From 57a8969814c432ee8c274e9e9c3219215bf23256 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Fri, 8 May 2020 20:16:13 -0700 Subject: [PATCH 46/55] FIX install ucx-py dependency in style.sh --- ci/checks/style.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ci/checks/style.sh b/ci/checks/style.sh index 638a8039a2..640e4dd7f7 100644 --- a/ci/checks/style.sh +++ b/ci/checks/style.sh @@ -8,8 +8,13 @@ set +e export PATH=/conda/bin:/usr/local/cuda/bin:$PATH -# Activate common conda env +# Activate common conda env and install any dependencies needed source activate gdf +cd $WORKSPACE +export GIT_DESCRIBE_TAG=`git describe --tags` +export MINOR_VERSION=`echo $GIT_DESCRIBE_TAG | grep -o -E '([0-9]+\.[0-9]+)'` +conda install -c rapidsai \ + "ucx-py=${MINOR_VERSION}" # Run flake8 and get results/return code FLAKE=`flake8 --exclude=cpp,thirdparty,__init__.py,versioneer.py && flake8 --config=python/.flake8.cython` From 05488803e83609e8b5cde8ba0750985a29395424 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Fri, 8 May 2020 20:47:33 -0700 Subject: [PATCH 47/55] FIX install more dependencies --- ci/checks/style.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/checks/style.sh b/ci/checks/style.sh index 640e4dd7f7..76c1c3b847 100644 --- a/ci/checks/style.sh +++ b/ci/checks/style.sh @@ -14,6 +14,7 @@ cd $WORKSPACE export GIT_DESCRIBE_TAG=`git describe --tags` export MINOR_VERSION=`echo $GIT_DESCRIBE_TAG | grep -o -E '([0-9]+\.[0-9]+)'` conda install -c rapidsai \ + "lapack" \ "ucx-py=${MINOR_VERSION}" # Run flake8 and get results/return code From 0803f677b537ef03ea2ce3bd5e9e4a6872792592 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Fri, 8 May 2020 20:51:19 -0700 Subject: [PATCH 48/55] FIX install openblas needed for faiss build --- ci/checks/style.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/checks/style.sh b/ci/checks/style.sh index 76c1c3b847..318ee5505f 100644 --- a/ci/checks/style.sh +++ b/ci/checks/style.sh @@ -15,6 +15,7 @@ export GIT_DESCRIBE_TAG=`git describe --tags` export MINOR_VERSION=`echo $GIT_DESCRIBE_TAG | grep -o -E '([0-9]+\.[0-9]+)'` conda install -c rapidsai \ "lapack" \ + "openblas" \ "ucx-py=${MINOR_VERSION}" # Run flake8 and get results/return code From 52d46eaabde36f6e9091a038e9b3a8109aa6a8b6 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Fri, 8 May 2020 22:58:53 -0700 Subject: [PATCH 49/55] FIX use anaconda channel for installing openblas --- ci/checks/style.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/checks/style.sh b/ci/checks/style.sh index 318ee5505f..580da140ac 100644 --- a/ci/checks/style.sh +++ b/ci/checks/style.sh @@ -13,7 +13,7 @@ source activate gdf cd $WORKSPACE export GIT_DESCRIBE_TAG=`git describe --tags` export MINOR_VERSION=`echo $GIT_DESCRIBE_TAG | grep -o -E '([0-9]+\.[0-9]+)'` -conda install -c rapidsai \ +conda install -c rapidsai -c anaconda \ "lapack" \ "openblas" \ "ucx-py=${MINOR_VERSION}" @@ -98,7 +98,7 @@ fi function setup_and_run_clang_tidy() { mkdir cpp/build && \ cd cpp/build && \ - cmake -DGPU_ARCHS=ALL \ + cmake -DGPU_ARCHS=70 \ -DBLAS_LIBRARIES=${CONDA_PREFIX}/lib/libopenblas.so.0 \ .. && \ make treelite && \ From 152976ca2b9ff63c518584d2e3c20631f43e3f2a Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Sat, 9 May 2020 00:47:38 -0700 Subject: [PATCH 50/55] FIX update LD_LIBRARY_PATH to be able to use openblas library successfully --- ci/checks/style.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ci/checks/style.sh b/ci/checks/style.sh index 580da140ac..0cec659e4d 100644 --- a/ci/checks/style.sh +++ b/ci/checks/style.sh @@ -96,6 +96,8 @@ fi # also, sync all dependencies as they'll be needed by clang-tidy to find # relevant headers function setup_and_run_clang_tidy() { + local LD_LIBRARY_PATH_CACHED=$LD_LIBRARY_PATH + export LD_LIBRARY_PATH=$CONDA_PREFIX/lib:$LD_LIBRARY_PATH mkdir cpp/build && \ cd cpp/build && \ cmake -DGPU_ARCHS=70 \ @@ -104,6 +106,7 @@ function setup_and_run_clang_tidy() { make treelite && \ cd ../.. && \ python cpp/scripts/run-clang-tidy.py + export LD_LIBRARY_PATH=$LD_LIBRARY_PATH_CACHED } TIDY=`setup_and_run_clang_tidy 2>&1` TIDY_RETVAL=$? From f86b7ead0a2e54873fece60d00d025ad68ff2303 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Sat, 9 May 2020 00:49:24 -0700 Subject: [PATCH 51/55] DOC copyright year update --- ci/checks/style.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/checks/style.sh b/ci/checks/style.sh index 0cec659e4d..12bfc95ec6 100644 --- a/ci/checks/style.sh +++ b/ci/checks/style.sh @@ -1,5 +1,5 @@ #!/bin/bash -# Copyright (c) 2019, NVIDIA CORPORATION. +# Copyright (c) 2019-2020, NVIDIA CORPORATION. ##################### # cuML Style Tester # ##################### From 00aa3fecf87d61ff72dce69484e6332e678a1bcc Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Sat, 9 May 2020 01:04:32 -0700 Subject: [PATCH 52/55] FIX ignore tidy for the 3rd-party headers --- cpp/bench/prims/main.cpp | 2 +- cpp/bench/sg/main.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/bench/prims/main.cpp b/cpp/bench/prims/main.cpp index bed4646850..203a8b57d5 100644 --- a/cpp/bench/prims/main.cpp +++ b/cpp/bench/prims/main.cpp @@ -14,6 +14,6 @@ * limitations under the License. */ -#include +#include // NOLINT BENCHMARK_MAIN(); diff --git a/cpp/bench/sg/main.cpp b/cpp/bench/sg/main.cpp index bed4646850..203a8b57d5 100644 --- a/cpp/bench/sg/main.cpp +++ b/cpp/bench/sg/main.cpp @@ -14,6 +14,6 @@ * limitations under the License. */ -#include +#include // NOLINT BENCHMARK_MAIN(); From 2cb30cfbfb14b6a1d0bdc42dff9abbc8276f9fb0 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Sat, 9 May 2020 01:06:17 -0700 Subject: [PATCH 53/55] FIX updated clang-tidy header-filter to only look at cuml source/header files --- cpp/scripts/run-clang-tidy.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cpp/scripts/run-clang-tidy.py b/cpp/scripts/run-clang-tidy.py index 8b42928622..6519e5d35e 100755 --- a/cpp/scripts/run-clang-tidy.py +++ b/cpp/scripts/run-clang-tidy.py @@ -157,7 +157,9 @@ def run_clang_tidy_command(tidy_cmd): def run_clang_tidy(cmd, args): command, is_cuda = get_tidy_args(cmd, args.exe) - tidy_cmd = [args.exe, "-header-filter=.*cuml/cpp/.*", cmd["file"], "--", ] + tidy_cmd = [args.exe, + "-header-filter='.*cuml/cpp/(src|include|bench|comms).*'", + cmd["file"], "--", ] tidy_cmd.extend(command) status = True out = "" From 6c88a6dded3fe65f9b943de0d8504f8b701f446e Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Fri, 15 May 2020 19:10:42 -0700 Subject: [PATCH 54/55] CI only install latest ucx-py inside style.sh --- ci/checks/style.sh | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/ci/checks/style.sh b/ci/checks/style.sh index 12bfc95ec6..22d9562568 100644 --- a/ci/checks/style.sh +++ b/ci/checks/style.sh @@ -13,10 +13,7 @@ source activate gdf cd $WORKSPACE export GIT_DESCRIBE_TAG=`git describe --tags` export MINOR_VERSION=`echo $GIT_DESCRIBE_TAG | grep -o -E '([0-9]+\.[0-9]+)'` -conda install -c rapidsai -c anaconda \ - "lapack" \ - "openblas" \ - "ucx-py=${MINOR_VERSION}" +conda install "ucx-py=${MINOR_VERSION}" # Run flake8 and get results/return code FLAKE=`flake8 --exclude=cpp,thirdparty,__init__.py,versioneer.py && flake8 --config=python/.flake8.cython` From 7acbf99d27bc3f9efe792850c877504940133cb6 Mon Sep 17 00:00:00 2001 From: Thejaswi Rao Date: Sat, 23 May 2020 22:18:55 -0700 Subject: [PATCH 55/55] DOC update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6a2f9e3ce..4705f22a13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ## Improvements - PR #2310: Pinning ucx-py to 0.14 to make 0.15 CI pass +- PR #1945: enable clang tidy ## Bug Fixes @@ -35,7 +36,6 @@ - PR #1947: Cleaning up cmake - PR #1927: Use Cython's `new_build_ext` (if available) - PR #1946: Removed zlib dependency from cmake -- PR #1945: enable clang tidy - PR #1988: C++: cpp bench refactor - PR #1873: Remove usage of nvstring and nvcat from LabelEncoder - PR #1968: Update SVC SVR with cuML Array