[Modules] Improve error message when cannot find parent module for submodule definition.

[vsapsai]

Summary

Before the change the diagnostic for

module unknown.submodule {}

was "error: expected module name" which is incorrect and misleading
because both "unknown" and "submodule" are valid module names.

We already have a better error message when a parent module is a
submodule itself and is missing. Make the error for a missing top-level
module more like the one for a submodule.

rdar://problem/64424407

[vsapsai]

clang/include/clang/Basic/DiagnosticLexKinds.td

700

Not sure about this second part of the message ("expected..."). Wanted to make it more actionable but not entirely happy with the wording. Any suggestions including removal are welcome.

clang/lib/Lex/ModuleMap.cpp

1911

Both of err_mmap_missing_module_qualified and err_mmap_expected_module_name are still used in other places.

1914

I've removed this return because otherwise you get spurious "expected module declaration" errors for valid module definition.

[vsapsai]

Ping.

[vsapsai]

Ping.

[bruno]

LGTM as is, minor suggestion below.

clang/include/clang/Basic/DiagnosticLexKinds.td

700

How about , parent module must be defined before the submodule?

[vsapsai]

Tweak the error text.

[vsapsai]

Thanks for the review.

clang/include/clang/Basic/DiagnosticLexKinds.td

700

Thanks, I think it is more direct and slightly more concise.

clang/include/clang/Basic/DiagnosticLexKinds.td

 def err_mmap_expected_conflicts_comma : Error<
   "expected ',' after conflicting module name">;
 def err_mmap_expected_conflicts_message : Error<
   "expected a message describing the conflict with '%0'">;
 def err_mmap_missing_module_unqualified : Error<
   "no module named '%0' visible from '%1'">;
 def err_mmap_missing_module_qualified : Error<
   "no module named '%0' in '%1'">;
+def err_mmap_missing_parent_module: Error<
+  "no module named '%0' %select{found|in '%2'}1, "
+  "parent module must be defined before the submodule">;
 def err_mmap_top_level_inferred_submodule : Error<
   "only submodules and framework modules may be inferred with wildcard syntax">;
 def err_mmap_inferred_no_umbrella : Error<
   "inferred submodules require a module with an umbrella">;
 def err_mmap_inferred_framework_submodule : Error<
   "inferred submodule cannot be a framework submodule">;
 def err_mmap_explicit_inferred_framework : Error<
   "inferred framework modules cannot be 'explicit'">;

clang/lib/Lex/ModuleMap.cpp

if (Id.size() > 1) {
     for (unsigned I = 0, N = Id.size() - 1; I != N; ++I) {
       if (Module *Next = Map.lookupModuleQualified(Id[I].first, ActiveModule)) {
         if (I == 0)
           TopLevelModule = Next;
         ActiveModule = Next;
         continue;
       }
 
-      if (ActiveModule) {
-        Diags.Report(Id[I].second, diag::err_mmap_missing_module_qualified)
-          << Id[I].first
-          << ActiveModule->getTopLevelModule()->getFullModuleName();
-      } else {
-        Diags.Report(Id[I].second, diag::err_mmap_expected_module_name);
-      }
+      Diags.Report(Id[I].second, diag::err_mmap_missing_parent_module)
+          << Id[I].first << (ActiveModule != nullptr)
+          << (ActiveModule
+                  ? ActiveModule->getTopLevelModule()->getFullModuleName()
+                  : "");
       HadError = true;
-      return;
     }
 
-    if (ModuleMapFile != Map.getContainingModuleMapFile(TopLevelModule)) {
+    if (TopLevelModule &&
+        ModuleMapFile != Map.getContainingModuleMapFile(TopLevelModule)) {
       assert(ModuleMapFile != Map.getModuleMapFileForUniquing(TopLevelModule) &&
              "submodule defined in same file as 'module *' that allowed its "
              "top-level module");
       Map.addAdditionalModuleMapFile(TopLevelModule, ModuleMapFile);
     }
   }
 
   StringRef ModuleName = Id.back().first;

clang/test/Modules/diagnostics.modulemap

module header_attr {
   // CHECK: diagnostics.modulemap:[[@LINE+1]]:27: error: header attribute 'size' specified multiple times
   header "bar.h" { size 1 size 2 }
   // CHECK: diagnostics.modulemap:[[@LINE+1]]:25: error: expected integer literal as value for header attribute 'size'
   header "baz.h" { size "30 kilobytes" }
 
   header "quux.h" { size 1 mtime 2 }
   header "no_attrs.h" {}
 }
+
+// CHECK: diagnostics.modulemap:[[@LINE+1]]:8: error: no module named 'unknown' found, parent module must be defined before the submodule
+module unknown.submodule {}
+module known_top_level {}
+// CHECK: diagnostics.modulemap:[[@LINE+1]]:24: error: no module named 'unknown' in 'known_top_level', parent module must be defined before the submodule
+module known_top_level.unknown.submodule {}