[PATCH] D38772: [refactor] allow the use of refactoring diagnostics

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

The code looks most good to me, a few nits.



================
Comment at: lib/Basic/DiagnosticIDs.cpp:46
   unsigned WarnShowInSystemHeader : 1;
-  unsigned Category : 5;
+  unsigned Category : 6;
 
----------------
just curious: is this change needed?


================
Comment at: tools/clang-refactor/ToolRefactoringResultConsumer.h:19
+
+/// A subclass of \c RefactoringResultConsumer that stores the reference to the
+/// TU-specific diagnostics engine.
----------------
I'd name it "interface", because it has unimplemented virtual function (`handleError`), clients can't create an instance of it. 

or alternatively,  does it make more sense to just add these methods and `DiagnosticsEngine` variable to the `tooling::RefactoringResultConsumer` interface? I see you have replaced "RefactoringResultConsumer" with this new interface in many places. 


Repository:
  rL LLVM

https://reviews.llvm.org/D38772