alexfh added a comment.
This patch is superseded by https://reviews.llvm.org/D26137.
https://reviews.llvm.org/D16183
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Elijah_Th added a comment.
alexfh,
OK. I'll take a look at apply-replacements and fix if needed, in the beginning
of the next week!
http://reviews.llvm.org/D16183
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
The main concern here is that the clang-apply-replacements tool should be able
to read this format. See the
clang/tools/extra/clang-tidy/tool/run-clang-tidy.py script for an example
alexfh added a comment.
Sorry for the lng delay. I missed you patch among all other ones. Could you
rebase your patch on top of HEAD and clang-format the files you changed? I'll
come back with more substantial comments early next week.
http://reviews.llvm.org/D16183
Elijah_Th added a comment.
YAML report looks like this now:
---
MainSourceFile: ''
Diagnostics:
CheckName: misc-macro-parentheses
Replacements:
- FilePath:/media/SSD_/code/zdoom/main_1.cpp
Offset: 1354
Length: 0
ReplacementText:
Elijah_Th updated this revision to Diff 47302.
Elijah_Th added a comment.
Fixed YAML format (was not correct in the last patch).
Grouped replacements in YAML by Diagnostics. It will help to apply replacements
for one fix at once.
http://reviews.llvm.org/D16183
Files:
Elijah_Th updated this revision to Diff 46883.
Elijah_Th added a comment.
Now the class that is serialized is Diagnostics.
I've moved ClangTidyError and ClangTidyMessage to the upper level, and renamed
to Diagnostics and DiagnosticsMessage. Now any tool can use this classes, they
contain more
alexfh added a comment.
In http://reviews.llvm.org/D16183#329475, @Elijah_Th wrote:
> What kind of wrapper should it be?
> I was thinking of this kind:
>
> class ExtendedReplacement : public Replacement {
No, I meant a different thing: serialize ClangTidyError or make a specialized
Elijah_Th marked 2 inline comments as done.
Elijah_Th added a comment.
What kind of wrapper should it be?
I was thinking of this kind:
class ExtendedReplacement : public Replacement {
public:
ExtendedReplacement(StringRef CheckName, Replacement );
StringRef getCheckName() const {
alexfh added a comment.
Thanks for addressing the comments. However, the main concern is this:
In http://reviews.llvm.org/D16183#326759, @alexfh wrote:
> I'm not sure `tooling::Replacement` is the best place to store check name.
> Maybe create a separate wrapper class and serialize it instead
alexfh added a comment.
I'm not sure `tooling::Replacement` is the best place to store check name.
Maybe create a separate wrapper class and serialize it instead
(clang-apply-replacements will have to be changed to support this format as
well). This could be `ClangTidyDiagnostic` or just
Elijah_Th created this revision.
Elijah_Th added reviewers: klimek, alexfh, djasper, cfe-commits.
Herald added a subscriber: klimek.
See details in https://llvm.org/bugs/show_bug.cgi?id=26132
http://reviews.llvm.org/D16183
Files:
tools/clang/include/clang/Tooling/Core/Replacement.h
12 matches
Mail list logo
