Skip to content

Commit

Permalink
Fix clang-tidy for native CPU ops (pytorch#57037)
Browse files Browse the repository at this point in the history
Summary:
Attempts to call clang-tidy on any source file in
`aten/src/ATen/cpu/native` would fail with series of
```
/Users/nshulga/git/pytorch-worktree/aten/src/ATen/native/cpu/Activation.cpp:637:1: warning: variable 'REGISTER_DISPATCH' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
/Users/nshulga/git/pytorch-worktree/aten/src/ATen/native/cpu/Activation.cpp:638:1: error: C++ requires a type specifier for all declarations [clang-diagnostic-error]
REGISTER_DISPATCH(log_sigmoid_backward_cpu_stub, &log_sigmoid_backward_cpu_kernel);
```
because those macros are only defined for cpu-arch specific compilation of above mentioned files.
Fix this by introducing `map_filename` function that will map source
file to its copy in `build` folder, run clang-tidy over the copy and
than map it back

Find it while working on pytorch#56892

Pull Request resolved: pytorch#57037

Reviewed By: walterddr

Differential Revision: D28033760

Pulled By: malfet

fbshipit-source-id: b67cd007000574ecc165ab4b285c0c102cbceadd
  • Loading branch information
malfet authored and Kushashwa Shrimali committed May 18, 2021
1 parent 11c5c19 commit 851952b
Showing 1 changed file with 25 additions and 3 deletions.
28 changes: 25 additions & 3 deletions tools/clang_tidy.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import os.path
import re
import shlex
import shutil
import subprocess
import sys
import tempfile
Expand Down Expand Up @@ -50,6 +51,24 @@
VERBOSE = False


# Functions for correct handling of "ATen/native/cpu" mapping
# Sources in that folder are not built in place but first copied into build folder with `.[CPUARCH].cpp` suffixes
def map_filename(build_folder: str, fname: str) -> str:
fname = os.path.relpath(fname)
native_cpu_prefix = "aten/src/ATen/native/cpu/"
build_cpu_prefix = os.path.join(build_folder, native_cpu_prefix, "")
default_arch_suffix = ".DEFAULT.cpp"
if fname.startswith(native_cpu_prefix) and fname.endswith(".cpp"):
return f"{build_cpu_prefix}{fname[len(native_cpu_prefix):]}{default_arch_suffix}"
if fname.startswith(build_cpu_prefix) and fname.endswith(default_arch_suffix):
return f"{native_cpu_prefix}{fname[len(build_cpu_prefix):-len(default_arch_suffix)]}"
return fname


def map_filenames(build_folder: str, fnames: Iterable[str]) -> List[str]:
return [map_filename(build_folder, fname) for fname in fnames]


def run_shell_command(arguments: List[str]) -> str:
"""Executes a shell command."""
if VERBOSE:
Expand Down Expand Up @@ -178,10 +197,10 @@ def run_clang_tidy(options: Any, line_filters: Any, files: Iterable[str]) -> str
command += ["-line-filter", json.dumps(line_filters)]

if options.parallel:
commands = [list(command) + [f] for f in files]
commands = [list(command) + [map_filename(options.compile_commands_dir, f)] for f in files]
output = run_shell_commands_in_parallel(commands)
else:
command += files
command += map_filenames(options.compile_commands_dir, files)
if options.dry_run:
command = [re.sub(r"^([{[].*[]}])$", r"'\1'", arg) for arg in command]
return " ".join(command)
Expand Down Expand Up @@ -334,8 +353,11 @@ def main() -> None:
if options.suppress_diagnostics:
warnings = extract_warnings(clang_tidy_output, base_dir=options.compile_commands_dir)
for fname in warnings.keys():
print(f"Applying fixes to {fname}")
mapped_fname = map_filename(options.compile_commands_dir, fname)
print(f"Applying fixes to {mapped_fname}")
apply_nolint(fname, warnings[fname])
if os.path.relpath(fname) != mapped_fname:
shutil.copyfile(fname, mapped_fname)

pwd = os.getcwd() + "/"
for line in clang_tidy_output.splitlines():
Expand Down

0 comments on commit 851952b

Please sign in to comment.