This is an archive of the discontinued LLVM Phabricator instance.

[refactor] allow the use of refactoring diagnostics
ClosedPublic

Authored by arphaman on Oct 10 2017, 5:20 PM.

Details

Summary

This patch allows the refactoring library to use its own set of refactoring-specific diagnostics to reports things like initiation errors.

Diff Detail

Repository
rL LLVM

Event Timeline

arphaman created this revision.Oct 10 2017, 5:20 PM
hokein edited edge metadata.Oct 11 2017, 1:44 AM

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

lib/Basic/DiagnosticIDs.cpp
46

just curious: is this change needed?

tools/clang-refactor/ToolRefactoringResultConsumer.h
19

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.

arphaman updated this revision to Diff 118701.Oct 11 2017, 2:36 PM
arphaman marked 2 inline comments as done.
  • rename the common consumer class.
lib/Basic/DiagnosticIDs.cpp
46

I get a build warning without this change as the bitfield becomes too narrow with the new category, so yeah.

tools/clang-refactor/ToolRefactoringResultConsumer.h
19

Right now I don't think having the diagnostics engine will be useful for clients outside of tool, so I'd prefer to keep it here. We can reconsider this decision in the future if we need to.

hokein accepted this revision.Oct 12 2017, 2:02 AM

Looks good.

lib/Basic/DiagnosticIDs.cpp
46

Thanks for explanation.

This revision is now accepted and ready to land.Oct 12 2017, 2:02 AM
This revision was automatically updated to reflect the committed changes.