On Thu, Jul 9, 2015 at 3:47 PM, Sean Silva <chisophu...@gmail.com> wrote:
> > > On Wed, Jul 8, 2015 at 12:58 PM, Ben Langmuir <blangm...@apple.com> wrote: > >> benlangmuir added a comment. >> >> > - Added better source location. >> >> >> It should really be the requirement location, but I guess we don't store >> that anywhere. This can be improved separately. >> >> > - Updated testing. I renamed to auto-import-unavailable.cpp and >> expanded testing as you asked for. Before, I was piggybacking on >> missing-header.m and related files, but since that is a different code path >> (and has different requirements and diagnostics), I thought it was better >> to keep the test separate and self-contained. >> >> > - also emit note_implicit_top_level_module_import_here on missing >> requirement. >> >> >> Changes look good, thanks. >> >> I replied to your email about the issue when a "requires" clause comes >> *after* a header declaration. I didn't actually hit that problem in the >> wild - only in a synthetic test - so maybe it can be addressed >> separately. If you and Richard agree, then from my perspective your patch >> LGTM and we just need to get the Darwin hacks in before it lands. >> > > This seems orthogonal. It is due to Module::markUnavailable having > multiple bailouts that are incorrect if MissingRequirement would have been > changed on any of the submodules during the traversal. The attached patch > fixes that, but I think opens us up to O(n^2) behavior. > Can we fix that by changing the bailout condition to: if (MissingRequirement ? !IsAvailable : IsMissingRequirement) ? Fixing this (probably not with my simple patch attached here) seems like a > good change, but unrelated to this patch. > Am I right in thinking that your patch causes regressions on valid input without a fix to this issue? > -- Sean Silva > > >> >> > Ben: hopefully the present patch is complete enough for you to start >> >> > figuring out exactly what hacks to add and where. >> >> >> Definitely. I can work from this, thanks. >> >> >> http://reviews.llvm.org/D10423 >> >> >> >> >
_______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
