Re: [PATCH] D16183: Added CheckName field to YAML report
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 information than than Replacement. I think Diagnostics is a good class to add some more information later. Please, review the fix, if it's ok I'll run clang-format on the changed code and provide the documentation. Thanks! http://reviews.llvm.org/D16183 Files: /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/include/clang/Tooling/Core/Diagnostics.h /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/include/clang/Tooling/DiagnosticsYaml.h /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/lib/Tooling/Core/CMakeLists.txt /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/lib/Tooling/Core/Diagnostics.cpp /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/tools/extra/clang-tidy/ClangTidy.cpp /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.h Index: /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/include/clang/Tooling/DiagnosticsYaml.h === --- /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/include/clang/Tooling/DiagnosticsYaml.h +++ /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/include/clang/Tooling/DiagnosticsYaml.h @@ -0,0 +1,42 @@ +//===-- ReplacementsYaml.h -- Serialiazation for Replacements ---*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +/// +/// \file +/// \brief This file defines the structure of a YAML document for serializing +/// ClangTidy errors. +/// +//===--===// + +#ifndef LLVM_CLANG_TOOLING_DIAGNOSTICSYAML_H +#define LLVM_CLANG_TOOLING_DIAGNOSTICSYAML_H + +#include "clang/Tooling/Refactoring.h" +#include "clang/Tooling/Core/Diagnostics.h" +#include "llvm/Support/YAMLTraits.h" +#include +#include + +LLVM_YAML_IS_SEQUENCE_VECTOR(clang::tooling::Diagnostics) + +namespace llvm { +namespace yaml { + +template <> struct MappingTraits { + static void mapping(IO &Io, + clang::tooling::Diagnostics &D) { +std::vector fixes(D.Fix.begin(), D.Fix.end()); +Io.mapRequired("CheckName", D.CheckName); +Io.mapRequired("Replacements", fixes); + } +}; +} // end namespace yaml +} // end namespace llvm + +#endif /* LLVM_CLANG_TOOLING_DIAGNOSTICSYAML_H */ + Index: /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/include/clang/Tooling/Core/Diagnostics.h === --- /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/include/clang/Tooling/Core/Diagnostics.h +++ /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/include/clang/Tooling/Core/Diagnostics.h @@ -0,0 +1,62 @@ +//===--- Replacement.h - Framework for clang refactoring tools --*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// Classes supporting refactorings that span multiple translation units. +// While single translation unit refactorings are supported via the Rewriter, +// when refactoring multiple translation units changes must be stored in a +// SourceManager independent form, duplicate changes need to be removed, and +// all changes must be applied at once at the end of the refactoring so that +// the code is always parseable. +// +//===--===// + +#ifndef LLVM_CLANG_TOOLING_CORE_DIAGNOSTICS_H +#define LLVM_CLANG_TOOLING_CORE_DIAGNOSTICS_H + +#include "clang/Basic/Diagnostic.h" +#include "llvm/ADT/SmallVector.h" +#include "Replacement.h" +#include + +namespace clang { +namespace tooling { + +struct DiagnosticsMessage { +DiagnosticsMessage(StringRef Message = ""); +DiagnosticsMessage(StringRef Message, const SourceManager &Sources, +SourceLocation Loc); +std::string Message; +std::string FilePath; +unsigned FileOffset; +}; + +struct Diagnostics { + +enum Level { +Ignored = DiagnosticsEngine::Ignored, +Warning =
Re: [PATCH] D16183: Added CheckName field to YAML report
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: /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/include/clang/Tooling/Core/Diagnostics.h /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/include/clang/Tooling/DiagnosticsYaml.h /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/lib/Tooling/Core/CMakeLists.txt /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/lib/Tooling/Core/Diagnostics.cpp /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/tools/extra/clang-tidy/ClangTidy.cpp /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.h Index: /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/include/clang/Tooling/DiagnosticsYaml.h === --- /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/include/clang/Tooling/DiagnosticsYaml.h +++ /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/include/clang/Tooling/DiagnosticsYaml.h @@ -0,0 +1,55 @@ +//===-- ReplacementsYaml.h -- Serialiazation for Replacements ---*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +/// +/// \file +/// \brief This file defines the structure of a YAML document for serializing +/// ClangTidy errors. +/// +//===--===// + +#ifndef LLVM_CLANG_TOOLING_DIAGNOSTICSYAML_H +#define LLVM_CLANG_TOOLING_DIAGNOSTICSYAML_H + +#include "clang/Tooling/Refactoring.h" +#include "clang/Tooling/Core/Diagnostics.h" +#include "llvm/Support/YAMLTraits.h" +#include +#include + +LLVM_YAML_IS_SEQUENCE_VECTOR(clang::tooling::Diagnostics) + +namespace llvm { +namespace yaml { + +template <> struct MappingTraits { + static void mapping(IO &Io, + clang::tooling::Diagnostics &D) { +std::vector fixes(D.Fix.begin(), D.Fix.end()); +Io.mapRequired("CheckName", D.CheckName); +Io.mapRequired("Replacements", fixes); + } +}; + +template <> struct MappingTraits { + static void mapping(IO &Io, + clang::tooling::TranslationUnitDiagnostics &TUD) { +Io.mapRequired("MainSourceFile", TUD.MainSourceFile); +Io.mapOptional("Context", TUD.Context, std::string()); +for (clang::tooling::Diagnostics diag : TUD.Diags) { +if (diag.Fix.size() > 0) { +Io.mapRequired("Diagnostics", diag); +} +} + } +}; +} // end namespace yaml +} // end namespace llvm + +#endif /* LLVM_CLANG_TOOLING_DIAGNOSTICSYAML_H */ + Index: /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/include/clang/Tooling/Core/Diagnostics.h === --- /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/include/clang/Tooling/Core/Diagnostics.h +++ /media/SSD_/code/LLVM-code/LLVM-trunk_256412/llvm/tools/clang/include/clang/Tooling/Core/Diagnostics.h @@ -0,0 +1,70 @@ +//===--- Replacement.h - Framework for clang refactoring tools --*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// Classes supporting refactorings that span multiple translation units. +// While single translation unit refactorings are supported via the Rewriter, +// when refactoring multiple translation units changes must be stored in a +// SourceManager independent form, duplicate changes need to be removed, and +// all changes must be applied at once at the end of the refactoring so that +// the code is always parseable. +// +//===--===// + +#ifndef LLVM_CLANG_TOOLING_CORE_DIAGNOSTICS_H +#define LLVM_CLANG_TOOLING_CORE_DIAGNOSTICS_H + +#include "clang/Basic/Diagnostic.h" +#include "llvm/ADT/SmallVector.h" +#include "Replacement.h" +#include + +namespace clang { +namespace tooling { + +struct DiagnosticsMessage { +DiagnosticsMessage(StringRef Message = ""); +DiagnosticsMessage(StringRef Message, const SourceManager &Sources, +SourceLocation Loc); +std::string Message; +std::string FilePath; +unsigned FileOffset; +};
Re: [PATCH] D16183: Added CheckName field to YAML report
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: '(' - FilePath:/media/SSD_/code/zdoom/main_1.cpp Offset: 1355 Length: 0 ReplacementText: ')' Diagnostics: CheckName: misc-macro-parentheses Replacements: - FilePath:/media/SSD_/code/zdoom/main_1.cpp Offset: 1452 Length: 0 ReplacementText: '(' - FilePath:/media/SSD_/code/zdoom/main_1.cpp Offset: 1453 Length: 0 ReplacementText: ')' ... http://reviews.llvm.org/D16183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16183: Added CheckName field to YAML report
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 http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D16183: Added CheckName field to YAML report
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 tools/clang/include/clang/Tooling/ReplacementsYaml.h tools/clang/lib/Tooling/Core/Replacement.cpp tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp Index: tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp === --- tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -77,7 +77,7 @@ assert(Range.getBegin().isFileID() && Range.getEnd().isFileID() && "Only file locations supported in fix-it hints."); - Error.Fix.insert(tooling::Replacement(SM, Range, FixIt.CodeToInsert)); + Error.Fix.insert(tooling::Replacement(SM, Range, FixIt.CodeToInsert, LangOptions(), StringRef(Error.CheckName))); } } Index: tools/clang/lib/Tooling/Core/Replacement.cpp === --- tools/clang/lib/Tooling/Core/Replacement.cpp +++ tools/clang/lib/Tooling/Core/Replacement.cpp @@ -32,9 +32,9 @@ : FilePath(InvalidLocation) {} Replacement::Replacement(StringRef FilePath, unsigned Offset, unsigned Length, - StringRef ReplacementText) + StringRef ReplacementText, StringRef CheckName) : FilePath(FilePath), ReplacementRange(Offset, Length), - ReplacementText(ReplacementText) {} + ReplacementText(ReplacementText), CheckName(CheckName) {} Replacement::Replacement(const SourceManager &Sources, SourceLocation Start, unsigned Length, StringRef ReplacementText) { @@ -44,8 +44,9 @@ Replacement::Replacement(const SourceManager &Sources, const CharSourceRange &Range, StringRef ReplacementText, - const LangOptions &LangOpts) { - setFromSourceRange(Sources, Range, ReplacementText, LangOpts); + const LangOptions &LangOpts, + StringRef CheckName) { + setFromSourceRange(Sources, Range, ReplacementText, LangOpts, CheckName); } bool Replacement::isApplicable() const { @@ -109,13 +110,14 @@ void Replacement::setFromSourceLocation(const SourceManager &Sources, SourceLocation Start, unsigned Length, -StringRef ReplacementText) { +StringRef ReplacementText, StringRef CheckName) { const std::pair DecomposedLocation = Sources.getDecomposedLoc(Start); const FileEntry *Entry = Sources.getFileEntryForID(DecomposedLocation.first); this->FilePath = Entry ? Entry->getName() : InvalidLocation; this->ReplacementRange = Range(DecomposedLocation.second, Length); this->ReplacementText = ReplacementText; + this->CheckName = CheckName; } // FIXME: This should go into the Lexer, but we need to figure out how @@ -137,10 +139,11 @@ void Replacement::setFromSourceRange(const SourceManager &Sources, const CharSourceRange &Range, StringRef ReplacementText, - const LangOptions &LangOpts) { + const LangOptions &LangOpts, + StringRef CheckName) { setFromSourceLocation(Sources, Sources.getSpellingLoc(Range.getBegin()), getRangeSize(Sources, Range, LangOpts), -ReplacementText); +ReplacementText, CheckName); } template Index: tools/clang/include/clang/Tooling/ReplacementsYaml.h === --- tools/clang/include/clang/Tooling/ReplacementsYaml.h +++ tools/clang/include/clang/Tooling/ReplacementsYaml.h @@ -33,21 +33,22 @@ /// access to its data members. struct NormalizedReplacement { NormalizedReplacement(const IO &) -: FilePath(""), Offset(0), Length(0), ReplacementText("") {} +: FilePath(""), Offset(0), Length(0), ReplacementText(""), CheckName("") {} NormalizedReplacement(const IO &, const clang::tooling::Replacement &R) -: FilePath(R.getFilePath()), Offset(R.getOffset()), - Length(R.getLength()), ReplacementText(R.getReplacementText()) {} +: FilePath(R.getFilePath()), Offset(R.getOffset()), Length(R.getLength()), + ReplacementText(R.getReplacementText()), CheckName(R.getCheckName()) {} clang::tooling::Replacement denormalize(const IO &) { return clang::tooling::Replacement(FilePath, Offset, Length, - Replacemen
Re: [PATCH] D16183: Added CheckName field to YAML report
Elijah_Th updated this revision to Diff 44876. Elijah_Th added a comment. - small format changes - removed unnecessary StringRef http://reviews.llvm.org/D16183 Files: tools/clang/include/clang/Tooling/Core/Replacement.h tools/clang/include/clang/Tooling/ReplacementsYaml.h tools/clang/lib/Tooling/Core/Replacement.cpp tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp Index: tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp === --- tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -77,7 +77,8 @@ assert(Range.getBegin().isFileID() && Range.getEnd().isFileID() && "Only file locations supported in fix-it hints."); - Error.Fix.insert(tooling::Replacement(SM, Range, FixIt.CodeToInsert)); + Error.Fix.insert(tooling::Replacement(SM, Range, FixIt.CodeToInsert, +LangOptions(), Error.CheckName)); } } Index: tools/clang/lib/Tooling/Core/Replacement.cpp === --- tools/clang/lib/Tooling/Core/Replacement.cpp +++ tools/clang/lib/Tooling/Core/Replacement.cpp @@ -32,20 +32,20 @@ : FilePath(InvalidLocation) {} Replacement::Replacement(StringRef FilePath, unsigned Offset, unsigned Length, - StringRef ReplacementText) + StringRef ReplacementText, StringRef CheckName) : FilePath(FilePath), ReplacementRange(Offset, Length), - ReplacementText(ReplacementText) {} + ReplacementText(ReplacementText), CheckName(CheckName) {} Replacement::Replacement(const SourceManager &Sources, SourceLocation Start, unsigned Length, StringRef ReplacementText) { setFromSourceLocation(Sources, Start, Length, ReplacementText); } Replacement::Replacement(const SourceManager &Sources, const CharSourceRange &Range, - StringRef ReplacementText, - const LangOptions &LangOpts) { - setFromSourceRange(Sources, Range, ReplacementText, LangOpts); + StringRef ReplacementText, const LangOptions &LangOpts, + StringRef CheckName) { + setFromSourceRange(Sources, Range, ReplacementText, LangOpts, CheckName); } bool Replacement::isApplicable() const { @@ -109,13 +109,15 @@ void Replacement::setFromSourceLocation(const SourceManager &Sources, SourceLocation Start, unsigned Length, -StringRef ReplacementText) { +StringRef ReplacementText, +StringRef CheckName) { const std::pair DecomposedLocation = Sources.getDecomposedLoc(Start); const FileEntry *Entry = Sources.getFileEntryForID(DecomposedLocation.first); this->FilePath = Entry ? Entry->getName() : InvalidLocation; this->ReplacementRange = Range(DecomposedLocation.second, Length); this->ReplacementText = ReplacementText; + this->CheckName = CheckName; } // FIXME: This should go into the Lexer, but we need to figure out how @@ -137,10 +139,11 @@ void Replacement::setFromSourceRange(const SourceManager &Sources, const CharSourceRange &Range, StringRef ReplacementText, - const LangOptions &LangOpts) { + const LangOptions &LangOpts, + StringRef CheckName) { setFromSourceLocation(Sources, Sources.getSpellingLoc(Range.getBegin()), -getRangeSize(Sources, Range, LangOpts), -ReplacementText); +getRangeSize(Sources, Range, LangOpts), ReplacementText, +CheckName); } template @@ -314,7 +317,7 @@ // Merges the next element 'R' into this merged element. As we always merge // from 'First' into 'Second' or vice versa, the MergedReplacement knows what - // set the next element is coming from. + // set the next element is coming from. void merge(const Replacement &R) { if (MergeSecond) { unsigned REnd = R.getOffset() + Delta + R.getLength(); @@ -416,4 +419,3 @@ } // end namespace tooling } // end namespace clang - Index: tools/clang/include/clang/Tooling/ReplacementsYaml.h === --- tools/clang/include/clang/Tooling/ReplacementsYaml.h +++ tools/clang/include/clang/Tooling/ReplacementsYaml.h @@ -33,30 +33,34 @@ /// access to its data members. struct NormalizedReplacement { NormalizedReplacement(const IO &) -: FilePath(""), Offset(0), Length(0), ReplacementText("") {} +: FilePath
Re: [PATCH] D16183: Added CheckName field to YAML report
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 &R); StringRef getCheckName() const { return CheckName; } std::string CheckName; } but in this case (Replacement.h:141) typedef std::set Replacements; should be changed to typedef std::set Replacements; which means a lot of code will be changed. Is that an acceptable change? Or you meant another kind of wrapper? http://reviews.llvm.org/D16183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits