MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.
So this isn't just clang-format-diff.py but also all of `git clang-format`
<edit the file>
git add Analysis.cpp
$ git clang-format
clang-format did not modify any files
This is bad because this is a way in which those using "git clang-format" can
miss formatting a line
So ultimately looking at the diff (normal diff)
$ git diff --cached Analysis.cpp
diff --git a/llvm/lib/CodeGen/Analysis.cpp b/llvm/lib/CodeGen/Analysis.cpp
index e5d576d879b5..3107fc0b9936 100644
--- a/llvm/lib/CodeGen/Analysis.cpp
+++ b/llvm/lib/CodeGen/Analysis.cpp
@@ -115,7 +115,6 @@ void llvm::ComputeValueVTs(const TargetLowering &TLI,
const DataLayout &DL,
return;
// Base case: we can get an EVT for this LLVM IR type.
ValueVTs.push_back(TLI.getValueType(DL, Ty));
- if (MemVTs)
MemVTs->push_back(TLI.getMemValueType(DL, Ty));
if (Offsets)
Offsets->push_back(StartingOffset);
This is going to give us a start_line of 115 and a line_count of 6 and hence an
end_line of 115+5 = 120
F19483241: image.png <https://reviews.llvm.org/F19483241>
if clang-format is given those lines I WOULD expect it to continue and correct
it.
$ clang-format -n --lines 115:120 Analysis.cpp
Analysis.cpp:117:48: warning: code should be clang-formatted
[-Wclang-format-violations]
ValueVTs.push_back(TLI.getValueType(DL, Ty));
But like clang-format-diff here, git-clang-format is doing a -U0 diff hence the
line count is 0
$ git diff -U0 --cached Analysis.cpp
diff --git a/llvm/lib/CodeGen/Analysis.cpp b/llvm/lib/CodeGen/Analysis.cpp
index e5d576d879b5..3107fc0b9936 100644
--- a/llvm/lib/CodeGen/Analysis.cpp
+++ b/llvm/lib/CodeGen/Analysis.cpp
@@ -118 +117,0 @@ void llvm::ComputeValueVTs(const TargetLowering &TLI, const
DataLayout &DL,
- if (MemVTs)
git-clang-format would also not process that change (as line_count == 0)
The code for handling this in git-clang-format is subtly different but equally
ootentially wrong
def extract_lines(patch_file):
"""Extract the changed lines in `patch_file`.
The return value is a dictionary mapping filename to a list of (start_line,
line_count) pairs.
The input must have been produced with ``-U0``, meaning unidiff format with
zero lines of context. The return value is a dict mapping filename to a
list of line `Range`s."""
matches = {}
for line in patch_file:
line = convert_string(line)
match = re.search(r'^\+\+\+\ [^/]+/(.*)', line)
if match:
filename = match.group(1).rstrip('\r\n')
match = re.search(r'^@@ -[0-9,]+ \+(\d+)(,(\d+))?', line)
if match:
start_line = int(match.group(1))
line_count = 1
if match.group(3):
line_count = int(match.group(3))
if line_count > 0:
matches.setdefault(filename, []).append(Range(start_line, line_count))
return matches
However if we take the same approach as suggested here, clang-format will run
over that line
$ clang-format -n --lines 117:117 Analysis.cpp
Analysis.cpp:117:48: warning: code should be clang-formatted
[-Wclang-format-violations]
ValueVTs.push_back(TLI.getValueType(DL, Ty));
To answer @hans question, I think it will do the correct thing, clang-format
still works on the whole file, it just filters the replacements based on the
"line range" given by --lines so I would expect it to work
I think this is the correct change being proposed here, but I also think
git-clang-format is wrong.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111273/new/
https://reviews.llvm.org/D111273
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
