Re: [PATCH] D16183: Added CheckName field to YAML report

2016-04-12 Thread Ilia Gromov via 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
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D16183: Added CheckName field to YAML report

2016-02-09 Thread Ilia Gromov via cfe-commits
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

2016-02-09 Thread Ilia Gromov via cfe-commits
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

2016-02-04 Thread Ilia Gromov via cfe-commits
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

2016-01-18 Thread Ilia Gromov via cfe-commits
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


Re: [PATCH] D16183: Added CheckName field to YAML report

2016-01-14 Thread Ilia Gromov via cfe-commits
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

[PATCH] D16183: Added CheckName field to YAML report

2016-01-14 Thread Ilia Gromov via cfe-commits
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