[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    Re: [PATCH] D10423: [modules] PR20507: Avoid silent textual inclusion.
From:       Ben Langmuir <blangmuir () apple ! com>
Date:       2015-07-08 19:58:42
Message-ID: a3ef183405895b97bcfab813e879eaa1 () localhost ! localdomain
[Download RAW message or body]

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.

> 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


[prev in list] [next in list] [prev in thread] [next in thread]