Skip to content

Commit 8763e10

Browse files
authored
Enable git pre hook (#188)
* Port PyTorch code quanlity tools to IPEX. [We use clang-tidy and flake8 to perform additional formatting and semantic checking of code. We provide a pre-commit git hook for performing these checks, before a commit is created -- ln -s ../../tools/git-pre-commit .git/hooks/pre-commit * Refine the code to ensure the git pre hook to work normally
1 parent 7767f56 commit 8763e10

27 files changed

+2586
-0
lines changed

.clang-format

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
---
2+
AccessModifierOffset: -1
3+
AlignAfterOpenBracket: AlwaysBreak
4+
AlignConsecutiveAssignments: false
5+
AlignConsecutiveDeclarations: false
6+
AlignEscapedNewlinesLeft: true
7+
AlignOperands: false
8+
AlignTrailingComments: false
9+
AllowAllParametersOfDeclarationOnNextLine: false
10+
AllowShortBlocksOnASingleLine: false
11+
AllowShortCaseLabelsOnASingleLine: false
12+
AllowShortFunctionsOnASingleLine: Empty
13+
AllowShortIfStatementsOnASingleLine: false
14+
AllowShortLoopsOnASingleLine: false
15+
AlwaysBreakAfterReturnType: None
16+
AlwaysBreakBeforeMultilineStrings: true
17+
AlwaysBreakTemplateDeclarations: true
18+
BinPackArguments: false
19+
BinPackParameters: false
20+
BraceWrapping:
21+
AfterClass: false
22+
AfterControlStatement: false
23+
AfterEnum: false
24+
AfterFunction: false
25+
AfterNamespace: false
26+
AfterObjCDeclaration: false
27+
AfterStruct: false
28+
AfterUnion: false
29+
BeforeCatch: false
30+
BeforeElse: false
31+
IndentBraces: false
32+
BreakBeforeBinaryOperators: None
33+
BreakBeforeBraces: Attach
34+
BreakBeforeTernaryOperators: true
35+
BreakConstructorInitializersBeforeComma: false
36+
BreakAfterJavaFieldAnnotations: false
37+
BreakStringLiterals: false
38+
ColumnLimit: 80
39+
CommentPragmas: '^ IWYU pragma:'
40+
CompactNamespaces: false
41+
ConstructorInitializerAllOnOneLineOrOnePerLine: true
42+
ConstructorInitializerIndentWidth: 4
43+
ContinuationIndentWidth: 4
44+
Cpp11BracedListStyle: true
45+
DerivePointerAlignment: false
46+
DisableFormat: false
47+
ForEachMacros: [ FOR_EACH_RANGE, FOR_EACH, ]
48+
IncludeCategories:
49+
- Regex: '^<.*\.h(pp)?>'
50+
Priority: 1
51+
- Regex: '^<.*'
52+
Priority: 2
53+
- Regex: '.*'
54+
Priority: 3
55+
IndentCaseLabels: true
56+
IndentWidth: 2
57+
IndentWrappedFunctionNames: false
58+
KeepEmptyLinesAtTheStartOfBlocks: false
59+
MacroBlockBegin: ''
60+
MacroBlockEnd: ''
61+
MaxEmptyLinesToKeep: 1
62+
NamespaceIndentation: None
63+
ObjCBlockIndentWidth: 2
64+
ObjCSpaceAfterProperty: false
65+
ObjCSpaceBeforeProtocolList: false
66+
PenaltyBreakBeforeFirstCallParameter: 1
67+
PenaltyBreakComment: 300
68+
PenaltyBreakFirstLessLess: 120
69+
PenaltyBreakString: 1000
70+
PenaltyExcessCharacter: 1000000
71+
PenaltyReturnTypeOnItsOwnLine: 2000000
72+
PointerAlignment: Left
73+
ReflowComments: true
74+
SortIncludes: true
75+
SpaceAfterCStyleCast: false
76+
SpaceBeforeAssignmentOperators: true
77+
SpaceBeforeParens: ControlStatements
78+
SpaceInEmptyParentheses: false
79+
SpacesBeforeTrailingComments: 1
80+
SpacesInAngles: false
81+
SpacesInContainerLiterals: true
82+
SpacesInCStyleCastParentheses: false
83+
SpacesInParentheses: false
84+
SpacesInSquareBrackets: false
85+
Standard: Cpp11
86+
TabWidth: 8
87+
UseTab: Never
88+
...

.clang-tidy

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
# NOTE there must be no spaces before the '-', so put the comma last.
3+
InheritParentConfig: true
4+
Checks: '
5+
bugprone-*,
6+
-bugprone-forward-declaration-namespace,
7+
-bugprone-macro-parentheses,
8+
-bugprone-lambda-function-name,
9+
-bugprone-reserved-identifier,
10+
cppcoreguidelines-*,
11+
-cppcoreguidelines-avoid-magic-numbers,
12+
-cppcoreguidelines-interfaces-global-init,
13+
-cppcoreguidelines-macro-usage,
14+
-cppcoreguidelines-owning-memory,
15+
-cppcoreguidelines-pro-bounds-array-to-pointer-decay,
16+
-cppcoreguidelines-pro-bounds-constant-array-index,
17+
-cppcoreguidelines-pro-bounds-pointer-arithmetic,
18+
-cppcoreguidelines-pro-type-cstyle-cast,
19+
-cppcoreguidelines-pro-type-reinterpret-cast,
20+
-cppcoreguidelines-pro-type-static-cast-downcast,
21+
-cppcoreguidelines-pro-type-union-access,
22+
-cppcoreguidelines-pro-type-vararg,
23+
-cppcoreguidelines-special-member-functions,
24+
-facebook-hte-RelativeInclude,
25+
hicpp-exception-baseclass,
26+
hicpp-avoid-goto,
27+
modernize-*,
28+
-modernize-concat-nested-namespaces,
29+
-modernize-return-braced-init-list,
30+
-modernize-use-auto,
31+
-modernize-use-default-member-init,
32+
-modernize-use-using,
33+
-modernize-use-trailing-return-type,
34+
performance-*,
35+
-performance-noexcept-move-constructor,
36+
-performance-unnecessary-value-param,
37+
'
38+
HeaderFilterRegex: 'torch_ipex/csrc/.*'
39+
AnalyzeTemporaryDtors: false
40+
CheckOptions:
41+
...

.clang-tidy-oss

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
---
2+
# NOTE there must be no spaces before the '-', so put the comma last.
3+
InheritParentConfig: true
4+
Checks: '
5+
bugprone-*,
6+
-bugprone-forward-declaration-namespace,
7+
-bugprone-macro-parentheses,
8+
-bugprone-lambda-function-name,
9+
-bugprone-reserved-identifier,
10+
cppcoreguidelines-*,
11+
-cppcoreguidelines-avoid-magic-numbers,
12+
-cppcoreguidelines-avoid-non-const-global-variables,
13+
-cppcoreguidelines-interfaces-global-init,
14+
-cppcoreguidelines-macro-usage,
15+
-cppcoreguidelines-owning-memory,
16+
-cppcoreguidelines-pro-bounds-array-to-pointer-decay,
17+
-cppcoreguidelines-pro-bounds-constant-array-index,
18+
-cppcoreguidelines-pro-bounds-pointer-arithmetic,
19+
-cppcoreguidelines-pro-type-cstyle-cast,
20+
-cppcoreguidelines-pro-type-reinterpret-cast,
21+
-cppcoreguidelines-pro-type-static-cast-downcast,
22+
-cppcoreguidelines-pro-type-union-access,
23+
-cppcoreguidelines-pro-type-vararg,
24+
-cppcoreguidelines-special-member-functions,
25+
-facebook-hte-RelativeInclude,
26+
hicpp-exception-baseclass,
27+
hicpp-avoid-goto,
28+
modernize-*,
29+
-modernize-concat-nested-namespaces,
30+
-modernize-return-braced-init-list,
31+
-modernize-use-auto,
32+
-modernize-use-default-member-init,
33+
-modernize-use-using,
34+
-modernize-use-trailing-return-type,
35+
performance-*,
36+
-performance-noexcept-move-constructor,
37+
-performance-unnecessary-value-param,
38+
'
39+
HeaderFilterRegex: 'torch_ipex/csrc/.*'
40+
AnalyzeTemporaryDtors: false
41+
WarningsAsErrors: '*'
42+
CheckOptions:
43+
...

.flake8

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
[flake8]
2+
select = B,C,E,F,P,T4,W,B9
3+
max-line-length = 120
4+
# C408 ignored because we like the dict keyword argument syntax
5+
# E501 is not flexible enough, we're using B950 instead
6+
ignore =
7+
E203,E305,E402,E501,E721,E741,F405,F821,F841,F999,W503,W504,C408,E302,W291,E303,
8+
# shebang has extra meaning in fbcode lints, so I think it's not worth trying
9+
# to line this up with executable bit
10+
EXE001,
11+
# these ignores are from flake8-bugbear; please fix!
12+
B007,B008,
13+
# these ignores are from flake8-comprehensions; please fix!
14+
C400,C401,C402,C403,C404,C405,C407,C411,C413,C414,C415
15+
per-file-ignores = __init__.py: F401
16+
optional-ascii-coding = True
17+
exclude =
18+
./.git,
19+
./build,
20+
./scripts,
21+
./tests/cpu,
22+
./third_party,

.gitignore

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,10 @@ gen
207207
.pytest_cache
208208
aten/build/*
209209

210+
.clang-format-bin/
211+
.clang-tidy-bin/
212+
213+
210214
# Bram
211215
plsdontbreak
212216

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
21ca53c291a88b53dac85751b7a0203ca610ac94b7adaff3c092cf30df4168f2

tools/git-pre-commit

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
#!/bin/bash
2+
set -e
3+
4+
echo "Running pre-commit flake8"
5+
python3 tools/linter/flake8_hook.py
6+
7+
echo "Running pre-commit clang-tidy"
8+
git diff HEAD > pr.diff
9+
python3 tools/linter/clang_tidy --diff-file "pr.diff"
10+
rm pr.diff
11+
12+
echo "Running pre-commit clang-format"
13+
tools/linter/git-clang-format HEAD~ --force

tools/linter/__init__.py

Whitespace-only changes.

tools/linter/clang_format_all.py

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
#!/usr/bin/env python3
2+
"""
3+
A script that runs clang-format on all C/C++ files in CLANG_FORMAT_ALLOWLIST. There is
4+
also a diff mode which simply checks if clang-format would make any changes, which is useful for
5+
CI purposes.
6+
7+
If clang-format is not available, the script also downloads a platform-appropriate binary from
8+
and S3 bucket and verifies it against a precommited set of blessed binary hashes.
9+
"""
10+
import argparse
11+
import asyncio
12+
import re
13+
import os
14+
import sys
15+
from typing import List, Set
16+
17+
from .clang_format_utils import get_and_check_clang_format, CLANG_FORMAT_PATH
18+
19+
# Allowlist of directories to check. All files that in that directory
20+
# (recursively) will be checked.
21+
# If you edit this, please edit the allowlist in clang_format_ci.sh as well.
22+
CLANG_FORMAT_ALLOWLIST = [
23+
"torch_ipex/csrc"
24+
]
25+
26+
# Only files with names matching this regex will be formatted.
27+
CPP_FILE_REGEX = re.compile(".*\\.(h|cpp|cc|c|hpp)$")
28+
29+
30+
def get_allowlisted_files() -> Set[str]:
31+
"""
32+
Parse CLANG_FORMAT_ALLOWLIST and resolve all directories.
33+
Returns the set of allowlist cpp source files.
34+
"""
35+
matches = []
36+
for dir in CLANG_FORMAT_ALLOWLIST:
37+
for root, dirnames, filenames in os.walk(dir):
38+
for filename in filenames:
39+
if CPP_FILE_REGEX.match(filename):
40+
matches.append(os.path.join(root, filename))
41+
return set(matches)
42+
43+
44+
async def run_clang_format_on_file(
45+
filename: str,
46+
semaphore: asyncio.Semaphore,
47+
verbose: bool = False,
48+
) -> None:
49+
"""
50+
Run clang-format on the provided file.
51+
"""
52+
# -style=file picks up the closest .clang-format, -i formats the files inplace.
53+
cmd = "{} -style=file -i {}".format(CLANG_FORMAT_PATH, filename)
54+
async with semaphore:
55+
proc = await asyncio.create_subprocess_shell(cmd)
56+
_ = await proc.wait()
57+
if verbose:
58+
print("Formatted {}".format(filename))
59+
60+
61+
async def file_clang_formatted_correctly(
62+
filename: str,
63+
semaphore: asyncio.Semaphore,
64+
verbose: bool = False,
65+
) -> bool:
66+
"""
67+
Checks if a file is formatted correctly and returns True if so.
68+
"""
69+
ok = True
70+
# -style=file picks up the closest .clang-format
71+
cmd = "{} -style=file {}".format(CLANG_FORMAT_PATH, filename)
72+
73+
async with semaphore:
74+
proc = await asyncio.create_subprocess_shell(cmd, stdout=asyncio.subprocess.PIPE)
75+
# Read back the formatted file.
76+
stdout, _ = await proc.communicate()
77+
78+
formatted_contents = stdout.decode()
79+
# Compare the formatted file to the original file.
80+
with open(filename) as orig:
81+
orig_contents = orig.read()
82+
if formatted_contents != orig_contents:
83+
ok = False
84+
if verbose:
85+
print("{} is not formatted correctly".format(filename))
86+
87+
return ok
88+
89+
90+
async def run_clang_format(
91+
max_processes: int,
92+
diff: bool = False,
93+
verbose: bool = False,
94+
) -> bool:
95+
"""
96+
Run clang-format to all files in CLANG_FORMAT_ALLOWLIST that match CPP_FILE_REGEX.
97+
"""
98+
# Check to make sure the clang-format binary exists.
99+
if not os.path.exists(CLANG_FORMAT_PATH):
100+
print("clang-format binary not found")
101+
return False
102+
103+
# Gather command-line options for clang-format.
104+
args = [CLANG_FORMAT_PATH, "-style=file"]
105+
106+
if not diff:
107+
args.append("-i")
108+
109+
ok = True
110+
111+
# Semaphore to bound the number of subprocesses that can be created at once to format files.
112+
semaphore = asyncio.Semaphore(max_processes)
113+
114+
# Format files in parallel.
115+
if diff:
116+
for f in asyncio.as_completed([file_clang_formatted_correctly(f, semaphore, verbose) for f in get_allowlisted_files()]):
117+
ok &= await f
118+
119+
if ok:
120+
print("All files formatted correctly")
121+
else:
122+
print("Some files not formatted correctly")
123+
else:
124+
await asyncio.gather(*[run_clang_format_on_file(f, semaphore, verbose) for f in get_allowlisted_files()])
125+
126+
return ok
127+
128+
def parse_args(args: List[str]) -> argparse.Namespace:
129+
"""
130+
Parse and return command-line arguments.
131+
"""
132+
parser = argparse.ArgumentParser(
133+
description="Execute clang-format on your working copy changes."
134+
)
135+
parser.add_argument(
136+
"-d",
137+
"--diff",
138+
action="store_true",
139+
default=False,
140+
help="Determine whether running clang-format would produce changes",
141+
)
142+
parser.add_argument("--verbose", "-v", action="store_true", default=False)
143+
parser.add_argument("--max-processes", type=int, default=50,
144+
help="Maximum number of subprocesses to create to format files in parallel")
145+
return parser.parse_args(args)
146+
147+
148+
def main(args: List[str]) -> bool:
149+
# Parse arguments.
150+
options = parse_args(args)
151+
# Get clang-format and make sure it is the right binary and it is in the right place.
152+
ok = get_and_check_clang_format(options.verbose)
153+
# Invoke clang-format on all files in the directories in the allowlist.
154+
if ok:
155+
loop = asyncio.get_event_loop()
156+
ok = loop.run_until_complete(run_clang_format(options.max_processes, options.diff, options.verbose))
157+
158+
# We have to invert because False -> 0, which is the code to be returned if everything is okay.
159+
return not ok
160+
161+
162+
if __name__ == "__main__":
163+
sys.exit(main(sys.argv[1:]))

0 commit comments

Comments
 (0)