commit 7d29486dddc6db6ee9d4c64159b320f6504b84e2
author Bruno Cardoso Lopes <bruno.cardoso@gmail.com> Fri Mar 30 05:17:58 2018 +0000
committer Bruno Cardoso Lopes <bruno.cardoso@gmail.com> Fri Mar 30 05:17:58 2018 +0000
tree cb1f81e8da0929f6fe2247901835224a53c52785
parent 59e5ef793c587cee64b8b10c4be48ec0aef0a126

[Modules] Improve fixit for framework private module maps

The wrong source range was being provided in some case, fix that to get
a better fixit.

rdar://problem/38520199

llvm-svn: 328857

clang/lib/Lex/ModuleMap.cpp

diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index 2ff0a68..a711e1d 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -1630,14 +1630,15 @@
 /// in other ways (FooPrivate and Foo.Private), providing notes and fixits.
 static void diagnosePrivateModules(const ModuleMap &Map,
                                    DiagnosticsEngine &Diags,
-                                   const Module *ActiveModule) {
+                                   const Module *ActiveModule,
+                                   SourceLocation InlineParent) {
 
   auto GenNoteAndFixIt = [&](StringRef BadName, StringRef Canonical,
-                             const Module *M) {
+                             const Module *M, SourceRange ReplLoc) {
     auto D = Diags.Report(ActiveModule->DefinitionLoc,
                           diag::note_mmap_rename_top_level_private_module);
     D << BadName << M->Name;
-    D << FixItHint::CreateReplacement(ActiveModule->DefinitionLoc, Canonical);
+    D << FixItHint::CreateReplacement(ReplLoc, Canonical);
   };
 
   for (auto E = Map.module_begin(); E != Map.module_end(); ++E) {
@@ -1657,7 +1658,8 @@
       Diags.Report(ActiveModule->DefinitionLoc,
                    diag::warn_mmap_mismatched_private_submodule)
           << FullName;
-      GenNoteAndFixIt(FullName, Canonical, M);
+      GenNoteAndFixIt(FullName, Canonical, M,
+                      SourceRange(InlineParent, ActiveModule->DefinitionLoc));
       continue;
     }
 
@@ -1667,7 +1669,8 @@
       Diags.Report(ActiveModule->DefinitionLoc,
                    diag::warn_mmap_mismatched_private_module_name)
           << ActiveModule->Name;
-      GenNoteAndFixIt(ActiveModule->Name, Canonical, M);
+      GenNoteAndFixIt(ActiveModule->Name, Canonical, M,
+                      SourceRange(ActiveModule->DefinitionLoc));
     }
   }
 }
@@ -1753,6 +1756,7 @@
   }
   
   Module *PreviousActiveModule = ActiveModule;  
+  SourceLocation LastInlineParentLoc = SourceLocation();
   if (Id.size() > 1) {
     // This module map defines a submodule. Go find the module of which it
     // is a submodule.
@@ -1763,6 +1767,7 @@
         if (I == 0)
           TopLevelModule = Next;
         ActiveModule = Next;
+        LastInlineParentLoc = Id[I].second;
         continue;
       }
       
@@ -1882,7 +1887,7 @@
                        StartLoc) &&
       (MapFileName.endswith("module.private.modulemap") ||
        MapFileName.endswith("module_private.map")))
-    diagnosePrivateModules(Map, Diags, ActiveModule);
+    diagnosePrivateModules(Map, Diags, ActiveModule, LastInlineParentLoc);
 
   bool Done = false;
   do {

clang/test/Modules/implicit-private-with-submodule.m

diff --git a/clang/test/Modules/implicit-private-with-submodule.m b/clang/test/Modules/implicit-private-with-submodule.m
index 1779341..a1c5b92 100644
--- a/clang/test/Modules/implicit-private-with-submodule.m
+++ b/clang/test/Modules/implicit-private-with-submodule.m
@@ -13,7 +13,7 @@
 
 // expected-warning@Inputs/implicit-private-with-submodule/A.framework/Modules/module.private.modulemap:1{{private submodule 'A.Private' in private module map, expected top-level module}}
 // expected-note@Inputs/implicit-private-with-submodule/A.framework/Modules/module.private.modulemap:1{{rename 'A.Private' to ensure it can be found by name}}
-// CHECK: fix-it:"{{.*}}module.private.modulemap":{1:20-1:27}:"A_Private"
+// CHECK: fix-it:"{{.*}}module.private.modulemap":{1:18-1:27}:"A_Private"
 
 #ifndef HEADER
 #define HEADER