-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sourcery refactored develop branch #1
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -69,7 +69,7 @@ def repository(urls = urls, log = EmptyLogger()): | |
socket.close() | ||
for entry in repo.split('\n')[:-1]: | ||
value = entry.split('|') | ||
version = tuple([int(n) for n in value[0].strip().split('.')]) | ||
version = tuple(int(n) for n in value[0].strip().split('.')) | ||
version = versions.setdefault(version, {}) | ||
arch = value[1].strip() | ||
if arch == 'x32': | ||
|
@@ -117,7 +117,7 @@ def unpack(archive, location, log = EmptyLogger()): | |
''' | ||
sevenzip = find_7zip(log) | ||
log.info('unpacking %s', os.path.basename(archive)) | ||
cmd = [sevenzip, 'x', archive, '-o' + location, '-y'] | ||
cmd = [sevenzip, 'x', archive, f'-o{location}', '-y'] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
log.debug(' - %r', cmd) | ||
with open(os.devnull, 'w') as devnull: | ||
subprocess.check_call(cmd, stdout = devnull) | ||
|
@@ -137,8 +137,7 @@ def download(url, location, log = EmptyLogger()): | |
content = stream.getheader('Content-Disposition') or '' | ||
except AttributeError: | ||
content = stream.headers.getheader('Content-Disposition') or '' | ||
matches = re_content.match(content) | ||
if matches: | ||
if matches := re_content.match(content): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
filename = matches.group(2) | ||
else: | ||
parsed = parse.urlparse(stream.geturl()) | ||
|
@@ -147,26 +146,24 @@ def download(url, location, log = EmptyLogger()): | |
try: | ||
os.makedirs(location) | ||
except OSError as e: | ||
if e.errno == errno.EEXIST and os.path.isdir(location): | ||
pass | ||
else: | ||
if e.errno != errno.EEXIST or not os.path.isdir(location): | ||
raise | ||
|
||
archive = os.path.join(location, filename) | ||
with open(archive, 'wb') as out: | ||
while True: | ||
buf = stream.read(1024) | ||
if not buf: | ||
if buf := stream.read(1024): | ||
out.write(buf) | ||
else: | ||
break | ||
out.write(buf) | ||
unpack(archive, location, log = log) | ||
os.remove(archive) | ||
|
||
possible = os.path.join(location, 'mingw64') | ||
if not os.path.exists(possible): | ||
possible = os.path.join(location, 'mingw32') | ||
if not os.path.exists(possible): | ||
raise ValueError('Failed to find unpacked MinGW: ' + possible) | ||
raise ValueError(f'Failed to find unpacked MinGW: {possible}') | ||
return possible | ||
|
||
def root(location = None, arch = None, version = None, threading = None, | ||
|
@@ -204,7 +201,7 @@ def root(location = None, arch = None, version = None, threading = None, | |
exceptions = 'sjlj' | ||
else: | ||
exceptions = keys[0] | ||
if revision == None: | ||
if revision is None: | ||
Comment on lines
-207
to
+204
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
revision = max(versions[version][arch][threading][exceptions].keys()) | ||
if not location: | ||
location = os.path.join(tempfile.gettempdir(), 'mingw-builds') | ||
|
@@ -234,7 +231,7 @@ def root(location = None, arch = None, version = None, threading = None, | |
elif arch == 'i686': | ||
root_dir = os.path.join(location, slug, 'mingw32') | ||
else: | ||
raise ValueError('Unknown MinGW arch: ' + arch) | ||
raise ValueError(f'Unknown MinGW arch: {arch}') | ||
|
||
# Download if needed | ||
if not os.path.exists(root_dir): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -152,7 +152,7 @@ def main(): | |
|
||
# NOTE: if test_baseline == test_contender, you are analyzing the stdev | ||
|
||
description = 'Comparing %s to %s' % (test_baseline, test_contender) | ||
description = f'Comparing {test_baseline} to {test_contender}' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
elif args.mode == 'filters': | ||
test_baseline = args.test[0].name | ||
test_contender = args.test[0].name | ||
|
@@ -162,8 +162,7 @@ def main(): | |
# NOTE: if filter_baseline == filter_contender, you are analyzing the | ||
# stdev | ||
|
||
description = 'Comparing %s to %s (from %s)' % ( | ||
filter_baseline, filter_contender, args.test[0].name) | ||
description = f'Comparing {filter_baseline} to {filter_contender} (from {args.test[0].name})' | ||
elif args.mode == 'benchmarksfiltered': | ||
test_baseline = args.test_baseline[0].name | ||
test_contender = args.test_contender[0].name | ||
|
@@ -173,11 +172,10 @@ def main(): | |
# NOTE: if test_baseline == test_contender and | ||
# filter_baseline == filter_contender, you are analyzing the stdev | ||
|
||
description = 'Comparing %s (from %s) to %s (from %s)' % ( | ||
filter_baseline, test_baseline, filter_contender, test_contender) | ||
description = f'Comparing {filter_baseline} (from {test_baseline}) to {filter_contender} (from {test_contender})' | ||
else: | ||
# should never happen | ||
print("Unrecognized mode of operation: '%s'" % args.mode) | ||
print(f"Unrecognized mode of operation: '{args.mode}'") | ||
parser.print_help() | ||
exit(1) | ||
|
||
|
@@ -187,8 +185,8 @@ def main(): | |
options_contender = [] | ||
|
||
if filter_baseline and filter_contender: | ||
options_baseline = ['--benchmark_filter=%s' % filter_baseline] | ||
options_contender = ['--benchmark_filter=%s' % filter_contender] | ||
options_baseline = [f'--benchmark_filter={filter_baseline}'] | ||
options_contender = [f'--benchmark_filter={filter_contender}'] | ||
|
||
# Run the benchmarks and report the results | ||
json1 = json1_orig = gbench.util.run_or_load_benchmark( | ||
|
@@ -198,7 +196,7 @@ def main(): | |
|
||
# Now, filter the benchmarks so that the difference report can work | ||
if filter_baseline and filter_contender: | ||
replacement = '[%s vs. %s]' % (filter_baseline, filter_contender) | ||
replacement = f'[{filter_baseline} vs. {filter_contender}]' | ||
json1 = gbench.report.filter_benchmark( | ||
json1_orig, filter_baseline, replacement) | ||
json2 = gbench.report.filter_benchmark( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,16 +49,15 @@ def main(): | |
test2 = args.test2[0] | ||
if unknown_args: | ||
# should never happen | ||
print("Unrecognized positional argument arguments: '%s'" | ||
% unknown_args) | ||
print(f"Unrecognized positional argument arguments: '{unknown_args}'") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
exit(1) | ||
benchmark_options = args.benchmark_options | ||
check_inputs(test1, test2, benchmark_options) | ||
# Run the benchmarks and report the results | ||
json1 = gbench.util.run_or_load_benchmark(test1, benchmark_options) | ||
json2 = gbench.util.run_or_load_benchmark(test2, benchmark_options) | ||
output_lines = gbench.report.generate_difference_report(json1, json2) | ||
print('Comparing %s to %s' % (test1, test2)) | ||
print(f'Comparing {test1} to {test2}') | ||
for ln in output_lines: | ||
print(ln) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,9 +61,9 @@ def calculate_change(old_val, new_val): | |
""" | ||
Return a float representing the decimal change between old_val and new_val. | ||
""" | ||
if old_val == 0 and new_val == 0: | ||
return 0.0 | ||
if old_val == 0: | ||
if new_val == 0: | ||
return 0.0 | ||
Comment on lines
-64
to
+66
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
return float(new_val - old_val) / (float(old_val + new_val) / 2) | ||
return float(new_val - old_val) / abs(old_val) | ||
|
||
|
@@ -73,8 +73,7 @@ def filter_benchmark(json_orig, family, replacement=""): | |
Apply a filter to the json, and only leave the 'family' of benchmarks. | ||
""" | ||
regex = re.compile(family) | ||
filtered = {} | ||
filtered['benchmarks'] = [] | ||
filtered = {'benchmarks': []} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
for be in json_orig['benchmarks']: | ||
if not regex.search(be['name']): | ||
continue | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,15 +60,15 @@ def classify_input_file(filename): | |
ftype = IT_Invalid | ||
err_msg = None | ||
if not os.path.exists(filename): | ||
err_msg = "'%s' does not exist" % filename | ||
err_msg = f"'{filename}' does not exist" | ||
elif not os.path.isfile(filename): | ||
err_msg = "'%s' does not name a file" % filename | ||
err_msg = f"'{filename}' does not name a file" | ||
elif is_executable_file(filename): | ||
ftype = IT_Executable | ||
elif is_json_file(filename): | ||
ftype = IT_JSON | ||
else: | ||
err_msg = "'%s' does not name a valid benchmark executable or JSON file" % filename | ||
err_msg = f"'{filename}' does not name a valid benchmark executable or JSON file" | ||
Comment on lines
-63
to
+71
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
return ftype, err_msg | ||
|
||
|
||
|
@@ -80,7 +80,7 @@ def check_input_file(filename): | |
""" | ||
ftype, msg = classify_input_file(filename) | ||
if ftype == IT_Invalid: | ||
print("Invalid input file: %s" % msg) | ||
print(f"Invalid input file: {msg}") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
sys.exit(1) | ||
return ftype | ||
|
||
|
@@ -128,11 +128,10 @@ def run_benchmark(exe_name, benchmark_flags): | |
is_temp_output = True | ||
thandle, output_name = tempfile.mkstemp() | ||
os.close(thandle) | ||
benchmark_flags = list(benchmark_flags) + \ | ||
['--benchmark_out=%s' % output_name] | ||
benchmark_flags = (list(benchmark_flags) + [f'--benchmark_out={output_name}']) | ||
|
||
cmd = [exe_name] + benchmark_flags | ||
print("RUNNING: %s" % ' '.join(cmd)) | ||
print(f"RUNNING: {' '.join(cmd)}") | ||
Comment on lines
-131
to
+134
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
exitCode = subprocess.call(cmd) | ||
if exitCode != 0: | ||
print('TEST FAILED...') | ||
|
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -13,20 +13,18 @@ def find_used_labels(asm): | |||||||
found = set() | ||||||||
label_re = re.compile("\s*j[a-z]+\s+\.L([a-zA-Z0-9][a-zA-Z0-9_]*)") | ||||||||
for l in asm.splitlines(): | ||||||||
m = label_re.match(l) | ||||||||
if m: | ||||||||
found.add('.L%s' % m.group(1)) | ||||||||
if m := label_re.match(l): | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (llm): While the walrus operator is a nice addition to Python 3.8, it may reduce readability for those unfamiliar with the syntax. Consider if compatibility with earlier Python versions is a concern.
Suggested change
|
||||||||
found.add(f'.L{m.group(1)}') | ||||||||
Comment on lines
-16
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||||||||
return found | ||||||||
|
||||||||
|
||||||||
def normalize_labels(asm): | ||||||||
decls = set() | ||||||||
label_decl = re.compile("^[.]{0,1}L([a-zA-Z0-9][a-zA-Z0-9_]*)(?=:)") | ||||||||
for l in asm.splitlines(): | ||||||||
m = label_decl.match(l) | ||||||||
if m: | ||||||||
if m := label_decl.match(l): | ||||||||
decls.add(m.group(0)) | ||||||||
if len(decls) == 0: | ||||||||
if not decls: | ||||||||
Comment on lines
-26
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||||||||
return asm | ||||||||
needs_dot = next(iter(decls))[0] != '.' | ||||||||
if not needs_dot: | ||||||||
|
@@ -103,16 +101,10 @@ def process_asm(asm): | |||||||
for l in asm.splitlines(): | ||||||||
# Remove Mach-O attribute | ||||||||
l = l.replace('@GOTPCREL', '') | ||||||||
add_line = True | ||||||||
for reg in discard_regexes: | ||||||||
if reg.match(l) is not None: | ||||||||
add_line = False | ||||||||
break | ||||||||
for reg in keep_regexes: | ||||||||
if reg.match(l) is not None: | ||||||||
add_line = True | ||||||||
break | ||||||||
if add_line: | ||||||||
if add_line := next( | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (llm): The use of assignment expression (walrus operator) with generator expressions and the 'all' function can be confusing. Consider refactoring for clarity. |
||||||||
(True for reg in keep_regexes if reg.match(l) is not None), | ||||||||
all(reg.match(l) is None for reg in discard_regexes), | ||||||||
): | ||||||||
Comment on lines
-106
to
+107
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||||||||
if fn_label_def.match(l) and len(new_contents) != 0: | ||||||||
new_contents += '\n' | ||||||||
l = process_identifiers(l) | ||||||||
|
@@ -133,7 +125,7 @@ def main(): | |||||||
input = args.input[0] | ||||||||
output = args.out[0] | ||||||||
if not os.path.isfile(input): | ||||||||
print(("ERROR: input file '%s' does not exist") % input) | ||||||||
print(f"ERROR: input file '{input}' does not exist") | ||||||||
Comment on lines
-136
to
+128
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||||||||
sys.exit(1) | ||||||||
contents = None | ||||||||
with open(input, 'r') as f: | ||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,10 +28,8 @@ | |
def strip_comments(text): | ||
def replacer(match): | ||
s = match.group(0) | ||
if s.startswith('/'): | ||
return " " # note: a space and not an empty string | ||
else: | ||
return s | ||
return " " if s.startswith('/') else s | ||
|
||
Comment on lines
-31
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
This removes the following comments ( why? ):
|
||
pattern = re.compile( | ||
r'//.*?$|/\*.*?\*/|\'(?:\\.|[^\\\'])*\'|"(?:\\.|[^\\"])*"', | ||
re.DOTALL | re.MULTILINE | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -105,11 +105,9 @@ def generate(self): | |
|
||
|
||
def _is_within(match, matches): | ||
for m in matches: | ||
if match.start() > m.start() and \ | ||
match.end() < m.end(): | ||
return True | ||
return False | ||
return any( | ||
match.start() > m.start() and match.end() < m.end() for m in matches | ||
) | ||
Comment on lines
-108
to
+110
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
||
|
||
class TranslationUnit(object): | ||
|
@@ -134,8 +132,7 @@ class TranslationUnit(object): | |
# Search for pattern in self.content, add the match to | ||
# contexts if found and update the index accordingly. | ||
def _search_content(self, index, pattern, contexts): | ||
match = pattern.search(self.content, index) | ||
if match: | ||
if match := pattern.search(self.content, index): | ||
Comment on lines
-137
to
+135
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
contexts.append(match) | ||
return match.end() | ||
return index + 2 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function
repository
refactored with the following changes:comprehension-to-generator
)