[PATCH] D10423: [modules] PR20507: Avoid silent textual inclusion.

Sean Silva chisophugis at gmail.com
Thu Jul 9 15:47:57 PDT 2015


On Wed, Jul 8, 2015 at 12:58 PM, Ben Langmuir <blangmuir at 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. Fixing this (probably not
with my simple patch attached here) seems like a good change, but unrelated
to this patch.

-- 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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150709/d8d39218/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: markUnavailable.patch
Type: application/octet-stream
Size: 686 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150709/d8d39218/attachment.obj>


More information about the cfe-commits mailing list