I still think this is a hack. I don't think DiagnosticConsumers /should/ have isa/cast/dyn_cast. The whole point of OOP and an open class hierarchy is that you use polymorphism as much as possible.
Is it possible to just add a "shouldCaptureDiagnostics" argument to arcmt::checkForManualIssues? I think that's really the cleanest way to solve the problem. (Unfortunately, the guy who wrote most of the migrator, Argyrios, just went on vacation. I'll CC him but I don't expect him to reply anytime soon.) Jordan On Jul 17, 2012, at 9:19 AM, Andy Gibbs wrote: > This is patch 2, which fixes CaptureDiagnosticConsumer inside ARCMT.cpp in > what I hope is considered a more acceptible way. > > The issue comes because CaptureDiagnosticConsumer must now additionally > capture expected-* directives and hold onto them pending forwarding them onto > VerifyDiagnosticConsumer along with the diagnostics that have been generated. > (In anticipation of the question, why not forward comments immediately, it > is the same reason why diagnostics cannot be forwarded immediately...) > > In order to do this, in a previous patch, I created a work-around so that the > CommentHandler interface of VerifyDiagnosticConsumer could be found. This > was rightly considered a hack by my reviewers, so I have fully implemented > dyn_cast into DiagnosticConsumer classes, so that such a hack is not > necessary, plus it is now possible to use isa/cast/dyn_cast which may find > uses elsewhere! > > The patch is large, but is mostly the implementation of dyn_cast and friends; > the patch to ARCMT.cpp is very similar to the original patch posted > previously. > > A note to my implementation for dyn_cast: > > DiagnosticConsumer constructor now takes a class identifier, which is defined > inside DiagnosticConsumer as an enum. For subclasses of DiagnosticConsumer > which are available outside a single translation unit, I have assigned class > identifiers. For those which are only used internally, I have assigned an > "unknown" class identifier. These classes cannot then be used with dyn_cast > et al. For those subclassing DiagnosticConsumer outside libclang, i.e. in > their own projects (since I assume this is possible), this "unknown" class > identifier should also be used there since changes to Diagnostic.h would be > deemed inappropriate. > > Cheers > Andy > > <patch2-arcmt.diff> _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
