alexfh added inline comments.
Comment at: unittests/Tooling/DiagnosticsYamlTest.cpp:35-45
+TEST(DiagnosticsYamlTest, serializesDiagnostics) {
+ TranslationUnitDiagnostics TUD;
+ TUD.MainSourceFile = "path/to/source.cpp";
+
+ StringMap Fix1 = {
+{
+ "path/to/source.c
This revision was automatically updated to reflect the committed changes.
Closed by commit rL308014: [Clang-Tidy] Preserve Message, FileOffset, FilePath
in Clang-Tidy YAML output (authored by alexfh).
Changed prior to commit:
https://reviews.llvm.org/D34404?vs=106402&id=106608#toc
Repository:
vladimir.plyashkun added a comment.
I've splitted single revision into two different revisions:
https://reviews.llvm.org/D34404
https://reviews.llvm.org/D35349
Also, i fixed failing test-cases in clang-apply-replacements due to changes in
output format (so they shouldn't be a problem anymore)
R
vladimir.plyashkun updated this revision to Diff 106402.
vladimir.plyashkun added a comment.
1. split revision into two
2. fix failing tests
Repository:
rL LLVM
https://reviews.llvm.org/D34404
Files:
include/clang/Tooling/DiagnosticsYaml.h
unittests/Tooling/CMakeLists.txt
unittests/Too
alexfh added a comment.
I tried landing the patch for you, but it doesn't apply cleanly. One reason is
that it contains changes to both cfe and clang-tools-extra repos. But even when
I apply the patch to the two directories it breaks a bunch of
clang-apply-replacements tests:
Failing Tests (
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
Looks good. Thanks!
Repository:
rL LLVM
https://reviews.llvm.org/D34404
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.l
vladimir.plyashkun updated this revision to Diff 105283.
vladimir.plyashkun edited the summary of this revision.
vladimir.plyashkun added a comment.
- made code less verbose
- used assignment initialization form
- removed extra `.c_str()` call
Repository:
rL LLVM
https://reviews.llvm.org/D344
alexfh added a comment.
A few more nits.
Comment at: unittests/Tooling/DiagnosticsYamlTest.cpp:39
+
+ StringMap Fix1{
+{ "path/to/source.cpp", Replacements(Replacement("path/to/source.cpp", 100,
The assignment form of initialization is clearer for containe
vladimir.plyashkun added a comment.
Thanks, Alex for your suggestions.
I agree with your remarks, but i thought it's only style preferences.
I'll fix it soon.
Repository:
rL LLVM
https://reviews.llvm.org/D34404
___
cfe-commits mailing list
cfe-co
vladimir.plyashkun updated this revision to Diff 105260.
vladimir.plyashkun added a comment.
- marked some arguments as `const`
- use `{}` instead of explicit variable declarations
Repository:
rL LLVM
https://reviews.llvm.org/D34404
Files:
include/clang/Tooling/DiagnosticsYaml.h
unittest
vladimir.plyashkun added inline comments.
Comment at: unittests/clang-apply-replacements/ApplyReplacementsTest.cpp:23
+ SmallVector EmptyNotes;
+ return tooling::Diagnostic(DiagnosticName, Message, Replacements, EmptyNotes,
+ tooling::Diagnostic::War
alexfh added inline comments.
Comment at: unittests/clang-apply-replacements/ApplyReplacementsTest.cpp:23
+ SmallVector EmptyNotes;
+ return tooling::Diagnostic(DiagnosticName, Message, Replacements, EmptyNotes,
+ tooling::Diagnostic::Warning, BuildD
vladimir.plyashkun updated this revision to Diff 105240.
vladimir.plyashkun added a comment.
- use `EXPECT_*` instead of `ASSERT_*` where it's possible
- `Diagnostic` constructor now takes `const` references for it's arguments
- removed extra namespace qualifiers
Repository:
rL LLVM
https://r
vladimir.plyashkun added inline comments.
Comment at: unittests/clang-apply-replacements/ApplyReplacementsTest.cpp:18
+
+static tooling::Diagnostic makeDiagnostic(const StringRef DiagnosticName,
+ DiagnosticMessage &Message,
--
vladimir.plyashkun added inline comments.
Comment at: unittests/clang-apply-replacements/ApplyReplacementsTest.cpp:23
+ SmallVector EmptyNotes;
+ return tooling::Diagnostic(DiagnosticName, Message, Replacements, EmptyNotes,
+ tooling::Diagnostic::War
alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.
Comment at: unittests/Tooling/DiagnosticsYamlTest.cpp:51
+
+ ASSERT_STREQ("---\n"
+"MainSourceFile: path/to/source.cpp\n"
EXPECT_STREQ is
vladimir.plyashkun updated this revision to Diff 105078.
vladimir.plyashkun added a comment.
Herald added a subscriber: JDevlieghere.
- fixed `No newline at end of file` problem
- provided test-case to check that diagnostics with no fixes will be applied
correctly
Repository:
rL LLVM
https:/
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
Please upload a diff with full context (http://llvm.org/docs/Phabricator.html).
Comment at: include/clang/Tooling/DiagnosticsYaml.h:86-95
-std::vector Diagnosti
vladimir.plyashkun updated this revision to Diff 103353.
vladimir.plyashkun added a comment.
correct revision with all changes
Repository:
rL LLVM
https://reviews.llvm.org/D34404
Files:
include/clang/Tooling/DiagnosticsYaml.h
unittests/Tooling/CMakeLists.txt
unittests/Tooling/Diagnosti
vladimir.plyashkun updated this revision to Diff 103341.
vladimir.plyashkun added a comment.
Herald added subscribers: xazax.hun, mgorny.
updated CMakeLists.txt
Repository:
rL LLVM
https://reviews.llvm.org/D34404
Files:
unittests/Tooling/CMakeLists.txt
Index: unittests/Tooling/CMakeLists
20 matches
Mail list logo
