search

Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Include check ID to YAML report #26131

Closed Quuxplusone closed 3 years ago

Quuxplusone commented 8 years ago
Bugzilla Link PR26132
Status RESOLVED FIXED
Importance P enhancement
Reported by Ilia (ilia.gromov@oracle.com)
Reported on 2016-01-13 10:37:34 -0800
Last modified on 2021-04-26 23:58:26 -0700
Version unspecified
Hardware All All
CC alexfh@google.com, djasper@google.com, ilia.gromov@oracle.com, klimek@google.com, shivam98.tkg@gmail.com
Fixed by commit(s) rG9bab1991469c1c04f832a80d52e3f5065914cc39
Attachments patch26132.diff (8370 bytes, text/plain)
Blocks
Blocked by
See also 
Hi,

clang-tidy saves a YAML report when the option '-export-fixes=...' is used.

---
MainSourceFile:  ''
Replacements:
   - FilePath:        /home/ilia/clang/sandbox/main.cpp
     Offset:          388
     Length:          8
     ReplacementText: '// TODO(ilia): '
...

This information is sufficient to apply generated replacements later.
However, there is no information about a check which had found this warning.

Is there a way to know check ID for this replacement?

PS:
In clang-modernize this problem was solved with a workaround:
When in "serialize-replacements" mode, clang-modernize can't inspect
sources more than for 1 check ID.
So, when I run

     ./clang-modernize -serialize-replacements
-serialize-dir=/tmp/modernize/add-override112233 /tmp/source.cpp

I'm sure that a YAML file in /tmp/modernize/add-override112233 is for
"add-override" check.Repeat this for all 6 checks and, as a result, you
can group replacements by check ID.
clang-tidy allows to specify any number of check IDs when saving to
YAML. And it has way more checks than 6. So, this workaround won't work
well in case of clang-tidy
Quuxplusone commented 8 years ago

Attached patch26132.diff (8370 bytes, text/plain): Proposed patch

Quuxplusone commented 8 years ago

Hi Ilia,

Thanks for the patch, but Bugzilla is not the best tool for code review ;) Could you upload the patch to LLVM Phabricator (see http://llvm.org/docs/Phabricator.html for instructions).

Thank you!

Quuxplusone commented 3 years ago

I think the solution to this issue is provided by https://reviews.llvm.org/D16183 and then superseded by https://reviews.llvm.org/D26137.