[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:       Sean Silva <chisophugis () gmail ! com>
Date:       2015-07-20 22:23:10
Message-ID: c55c3c412f0b08409b7ba193619bd7ff () localhost ! localdomain
[Download RAW message or body]

silvas added inline comments.

================
Comment at: lib/Lex/PPDirectives.cpp:1680-1698
@@ +1679,21 @@
+    // unavailable, diagnose the situation and bail out.
+    if (!SuggestedModule.getModule()->isAvailable()) {
+      clang::Module::Requirement Requirement;
+      clang::Module::UnresolvedHeaderDirective MissingHeader;
+      Module *M = SuggestedModule.getModule();
+      // Identify the cause.
+      (void)M->isAvailable(getLangOpts(), getTargetInfo(), Requirement,
+                           MissingHeader);
+      if (MissingHeader.FileNameLoc.isValid()) {
+        Diag(MissingHeader.FileNameLoc, diag::err_module_header_missing)
+            << MissingHeader.IsUmbrella << MissingHeader.FileName;
+      } else {
+        Diag(M->DefinitionLoc, diag::err_module_unavailable)
+            << M->getFullModuleName() << Requirement.second << Requirement.first;
+      }
+      Diag(FilenameTok.getLocation(),
+           diag::note_implicit_top_level_module_import_here)
+          << M->getTopLevelModuleName();
+      return;
+    }
+
----------------
rsmith wrote:
> The call to `loadModule` a few lines below already does this. Is this change \
> necessary?
I think it is. We have more information available here and can diagnose the situation \
better. Ideally the call to loadModule here would take a `Module *` and could assert \
the module to be available.


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]