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 constraints on the location? Should it be a file location or 
> macro locations are fine too? Please add a (doxygen-style) comment.
Indeed, this should be a file location.


================
Comment at: include/clang/Tooling/Core/Diagnostic.h:68-71
+  /// A freeform chunk of text to describe the context of the replacements.
+  /// Will be printed, for example, when detecting conflicts during replacement
+  /// deduplication.
+  std::string Context;
----------------
alexfh wrote:
> That's too vague. Are you intending to use it only for reporting problems 
> with replacement deduplication? Should it be in this structure at all?
This was actually left to keep compatibility with `TranslationUnitReplacements` 
which was used for the export.
But it seems that even for that structure, there is in all likelihood no 
reference to any use of the `Context` field, except in test cases and in the 
Yaml IO mapping, where it is marked as an optional entry.
Should it be discarded instead (here, and thus also in 
`TranslationUnitReplacements`)?


================
Comment at: tools/extra/clang-tidy/ClangTidy.cpp:578
                         raw_ostream &OS) {
-  TranslationUnitReplacements TUR;
-  for (const ClangTidyError &Error : Errors) {
-    for (const auto &FileAndFixes : Error.Fix)
-      TUR.Replacements.insert(TUR.Replacements.end(),
-                              FileAndFixes.second.begin(),
-                              FileAndFixes.second.end());
-  }
-
+  TranslationUnitDiagnostics TUD;
+  TUD.Diagnostics.insert(TUD.Diagnostics.end(), Errors.begin(), Errors.end());
----------------
alexfh wrote:
> This function neither fills `TUD.MainSourceFile` nor `TUD.Context` (which I'm 
> not sure even belongs to this structure).
Done for  `MainSourceFile` which was surprisingly never exported with the fixes.
For  `Context`, see above comment about the 'TranslationUnitDiagnostics' 
structure.


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

Reply via email to