[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:       Richard Smith <richard () metafoo ! co ! uk>
Date:       2015-07-09 2:13:38
Message-ID: f00340e1807206aac881b0f573e16435 () localhost ! localdomain
[Download RAW message or body]

rsmith added inline comments.

================
Comment at: lib/Lex/PPDirectives.cpp:1665
@@ +1664,3 @@
+  // unavailable, diagnose the situation and bail out.
+  if (SuggestedModule && !SuggestedModule.getModule()->isAvailable()) {
+    clang::Module::Requirement Requirement;
----------------
rsmith wrote:
> I think this whole block should be moved down to line 1761 or so:
> 
> 1. The code currently ensures that it always calls InclusionDirective on the \
> callback object for every `#include`, even if that include fails. 2. We don't yet \
> know that the file is actually part of `SuggestedModule`; it could be in the \
> directory of an umbrella header whose module is unavailable, but it might not be \
> part of that module. 3. If we somehow got a suggested module but no file, we should \
> not produce additional spurious diagnostics beyond the "file not found" diagnostic \
> we already produced.
One other thing: we should only diagnose when `ShouldEnter` is `false`. If we're \
entering the file anyway, we don't need the module to be available; only those files \
within it that we're actually entering need to be present.


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]