Thanks for the input! I've amended the patch; notable changes include changing MaybeDiagnoseMissingCompleteType's return type to bool (from void) to indicate whether a diagnostic was produced and adding a small unit test (though since there were no unittests for Sema, this ended up touching a couple of files). I also repaired the formatting problems you'd indicated.
On Mon, Jul 22, 2013 at 5:06 PM, Nick Lewycky <[email protected]> wrote: > On 12 July 2013 17:01, Luke Zarko <[email protected]> wrote: >> >> Ping--any opinions on this or thoughts on better ways to do it? > > > From the peanut gallery! I fundamentally like the direction this is going. > > + /// \brief Maybe produce a diagnostic note if this class can find a > + /// way to get at the complete definition for T. > > Is this clearer?: "Produces a diagnostic note if the external source > contains a complete definition for T." > > + for(size_t i = 0; i < Sources.size(); ++i) > > Space after 'for'. Also, hoist out the computation of Sources.size(). See > http://llvm.org/docs/CodingStandards.html#don-t-evaluate-end-every-time-through-a-loop > > What prevents us from emitting the same diagnostic note twice? What if two > external sources both contain full definitions of the type (for instance, > modules X and Y both include module Z which defines the type, but they don't > expose Z's contents to their users)? > > \ No newline at end of file > > Please keep a newline at the end of MultiplexExternalSemaSource.cpp. > > Nick > >> On Tue, Jul 9, 2013 at 3:45 PM, Luke Zarko <[email protected]> wrote: >> > This patch adds a method to ExternalSemaSource to allow it to produce >> > diagnostics when an incomplete type was detected where a complete type >> > was required. For example, one could define an ExternalSemaSource that >> > scans a repository for headers providing a missing definition. >> > >> > The purpose of the MaybeDiagnoseMissingCompleteType method differs >> > from that of the existing ExternalASTSource::CompleteType method. The >> > former is invoked to diagnose a single SourceLocation-ed >> > missing-complete-type site where there is no chance for recovery. The >> > latter may be called in the course of compiling a valid source file >> > with some of its definitions available in external but otherwise >> > available locations. >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > >
maybe-diagnose-v2.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
