Alpha added a comment.
Thanks for the review!
Comment at: tools/extra/clang-tidy/ClangTidy.cpp:106
void reportDiagnostic(const ClangTidyError &Error) {
-const ClangTidyMessage &Message = Error.Message;
+const ClangTidyMessage Message = Error.Message;
SourceLocat
This revision was automatically updated to reflect the committed changes.
Closed by commit rL290892: [clang-tidy] Add check name to YAML export (authored
by alexfh).
Changed prior to commit:
https://reviews.llvm.org/D26137?vs=82860&id=82881#toc
Repository:
rL LLVM
https://reviews.llvm.org/D
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
Looks good.
Fixed the issues myself and running tests before committing this.
Thank you for working on this!
Comment at: tools/extra/clang-tidy/ClangTidy.cpp:106
void re
alexfh added a comment.
Err, looks like I forgot to post comments I entered a few days ago.
Just a few nits.
Comment at: include/clang/Tooling/Core/Diagnostic.h:62
+ Diagnostic(llvm::StringRef DiagnosticName, DiagnosticMessage &Message,
+ llvm::StringMap &Fix,
+
Alpha updated this revision to Diff 82860.
Alpha added a comment.
Rebase on top of HEAD.
Ping.
Repository:
rL LLVM
https://reviews.llvm.org/D26137
Files:
include/clang/Tooling/Core/Diagnostic.h
include/clang/Tooling/Core/Replacement.h
include/clang/Tooling/DiagnosticsYaml.h
include/c
Alpha added a comment.
Ping.
Repository:
rL LLVM
https://reviews.llvm.org/D26137
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Alpha updated this revision to Diff 81779.
Alpha added a comment.
It was tested against the clang extra unit tests, but not tests run with
check-clang-tools.
Updated Yaml test files to match the new format, this might need further review.
Repository:
rL LLVM
https://reviews.llvm.org/D26137
alexfh added a comment.
Have you run the tests? I see a number of failures:
$ ninja check-clang-tools
...
Failing Tests (6):
Clang Tools :: clang-apply-replacements/basic.cpp
Clang Tools :: clang-apply-replacements/conflict.cpp
Clang Tools :: clang-apply-replacements/crlf.
Alpha updated this revision to Diff 80874.
Alpha added a comment.
Fix clang compilation warnings. These didn't appear when compiled on Windows.
Tested on a Linux distribution, should be fixed.
Repository:
rL LLVM
https://reviews.llvm.org/D26137
Files:
include/clang/Tooling/Core/Diagnostic.
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
Looks like compiler has found a couple of bugs:
In file included from
llvm.git/tools/clang/tools/extra/clang-tidy/ClangTidy.cpp:38:
llvm.git/tools/clang/include/clang/Tooling/Dia
alexfh added a comment.
The patch spans two repos, so I couldn't apply it using arcanist. Just made it
"manually" (patch -p0 -i ...); now running tests...
Repository:
rL LLVM
https://reviews.llvm.org/D26137
___
cfe-commits mailing list
cfe-commi
Alpha updated this revision to Diff 80732.
Alpha added a comment.
Rebase on top of HEAD.
Ping.
Repository:
rL LLVM
https://reviews.llvm.org/D26137
Files:
include/clang/Tooling/Core/Diagnostic.h
include/clang/Tooling/Core/Replacement.h
include/clang/Tooling/DiagnosticsYaml.h
include/c
Alpha added a comment.
I don't have commit access.
Thank you for the review @alexfh
Repository:
rL LLVM
https://reviews.llvm.org/D26137
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co
Alpha updated this revision to Diff 79917.
Alpha added a comment.
Rebase on top of HEAD.
Repository:
rL LLVM
https://reviews.llvm.org/D26137
Files:
include/clang/Tooling/Core/Diagnostic.h
include/clang/Tooling/Core/Replacement.h
include/clang/Tooling/DiagnosticsYaml.h
include/clang/T
alexfh added a comment.
Do you have commit access? If you need me to commit the patch for you, please
rebase it on top of HEAD.
Repository:
rL LLVM
https://reviews.llvm.org/D26137
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://li
Alpha updated this revision to Diff 79904.
Repository:
rL LLVM
https://reviews.llvm.org/D26137
Files:
include/clang/Tooling/Core/Diagnostic.h
include/clang/Tooling/Core/Replacement.h
include/clang/Tooling/DiagnosticsYaml.h
include/clang/Tooling/ReplacementsYaml.h
lib/Tooling/Core/CMa
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG with a comment.
Comment at: include/clang/Tooling/DiagnosticsYaml.h:79
+ for (auto &Diagnostic : Doc.Diagnostics) {
+if (Diagnostic.Fix.size() > 0) {
+
Alpha updated this revision to Diff 79899.
Herald added a subscriber: JDevlieghere.
Repository:
rL LLVM
https://reviews.llvm.org/D26137
Files:
include/clang/Tooling/Core/Diagnostic.h
include/clang/Tooling/Core/Replacement.h
include/clang/Tooling/DiagnosticsYaml.h
include/clang/Tooling/
Alpha marked 10 inline comments as done.
Alpha added inline comments.
Comment at: include/clang/Tooling/DiagnosticsYaml.h:79
+ for (auto &Diagnostic : Doc.Diagnostics) {
+if (Diagnostic.Fix.size() > 0) {
+ Diagnostics.push_back(Diagnostic);
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
Looks mostly good. A few more nits.
Comment at: include/clang/Tooling/Core/Diagnostic.h:68-71
+ /// A freeform chunk of text to describe the context of the replace
alexfh added a comment.
I'll try to get back to this code review soon. Sorry for the delay.
Repository:
rL LLVM
https://reviews.llvm.org/D26137
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo
Alpha added a comment.
Ping
Repository:
rL LLVM
https://reviews.llvm.org/D26137
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Alpha added a comment.
In https://reviews.llvm.org/D26137#602602, @malcolm.parsons wrote:
> In https://reviews.llvm.org/D26137#602591, @Alpha wrote:
>
> > This shouldn't affect diagnostics without fixes. If there is no fix, there
> > won't be anything to export, and the diagnostic just behaves n
malcolm.parsons added a comment.
In https://reviews.llvm.org/D26137#602591, @Alpha wrote:
> This shouldn't affect diagnostics without fixes. If there is no fix, there
> won't be anything to export, and the diagnostic just behaves normally.
That's a shame; I need a machine readable report of al
Alpha added a comment.
This shouldn't affect diagnostics without fixes. If there is no fix, there
won't be anything to export, and the diagnostic just behaves normally.
Repository:
rL LLVM
https://reviews.llvm.org/D26137
___
cfe-commits mailing
malcolm.parsons added a comment.
What happens to diagnostics without fixes? e.g. from the
readability-function-size check.
Repository:
rL LLVM
https://reviews.llvm.org/D26137
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.l
Alpha added a comment.
Ping
Repository:
rL LLVM
https://reviews.llvm.org/D26137
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Alpha added a comment.
Ping
Repository:
rL LLVM
https://reviews.llvm.org/D26137
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Alpha updated this revision to Diff 77196.
Alpha added a comment.
Export effectively MainSourceFile.
Change CheckName field.
Add doxygen-style comments.
Repository:
rL LLVM
https://reviews.llvm.org/D26137
Files:
include/clang/Tooling/Core/Diagnostic.h
include/clang/Tooling/DiagnosticsYam
Alpha added inline comments.
Comment at: include/clang/Tooling/Core/Diagnostic.h:35
+ DiagnosticMessage(llvm::StringRef Message, const SourceManager &Sources,
+SourceLocation Loc);
+ std::string Message;
alexfh wrote:
> What are the constrai
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
Thank you for resurrecting this patch! A few comments inline.
Comment at: include/clang/Tooling/Core/Diagnostic.h:35
+ DiagnosticMessage(llvm::StringRef Message, c
Alpha updated this revision to Diff 77029.
Alpha added a comment.
Ignore export of empty fixes.
Repository:
rL LLVM
https://reviews.llvm.org/D26137
Files:
include/clang/Tooling/Core/Diagnostic.h
include/clang/Tooling/DiagnosticsYaml.h
lib/Tooling/Core/CMakeLists.txt
lib/Tooling/Core/
Alpha added a comment.
Ping
Repository:
rL LLVM
https://reviews.llvm.org/D26137
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Alpha updated this revision to Diff 76850.
Alpha added a comment.
Remove debug symbols.
Repository:
rL LLVM
https://reviews.llvm.org/D26137
Files:
include/clang/Tooling/Core/Diagnostic.h
include/clang/Tooling/DiagnosticsYaml.h
lib/Tooling/Core/CMakeLists.txt
lib/Tooling/Core/Diagnost
Alpha removed rL LLVM as the repository for this revision.
Alpha updated this revision to Diff 76849.
Alpha added a comment.
Fix diagnostic deserialization bug for clang-apply-replacement.
https://reviews.llvm.org/D26137
Files:
include/clang/Tooling/Core/Diagnostic.h
include/clang/Tooling/D
Alpha created this revision.
Alpha added reviewers: alexfh, klimek, djasper.
Alpha added a subscriber: cfe-commits.
Herald added subscribers: fhahn, mgorny.
Add a field indicating the associated check for every replacement to the YAML
report generated with the '-export-fixes' option.
Update clang
36 matches
Mail list logo
