From 7d29486dddc6db6ee9d4c64159b320f6504b84e2 Mon Sep 17 00:00:00 2001 From: Bruno Cardoso Lopes Date: Fri, 30 Mar 2018 05:17:58 +0000 Subject: [PATCH] [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 | 17 +++++++++++------ clang/test/Modules/implicit-private-with-submodule.m | 2 +- 2 files changed, 12 insertions(+), 7 deletions(-) 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 @@ namespace { /// 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 @@ static void diagnosePrivateModules(const ModuleMap &Map, 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 @@ static void diagnosePrivateModules(const ModuleMap &Map, 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 @@ void ModuleMapParser::parseModuleDecl() { } 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 @@ void ModuleMapParser::parseModuleDecl() { if (I == 0) TopLevelModule = Next; ActiveModule = Next; + LastInlineParentLoc = Id[I].second; continue; } @@ -1882,7 +1887,7 @@ void ModuleMapParser::parseModuleDecl() { StartLoc) && (MapFileName.endswith("module.private.modulemap") || MapFileName.endswith("module_private.map"))) - diagnosePrivateModules(Map, Diags, ActiveModule); + diagnosePrivateModules(Map, Diags, ActiveModule, LastInlineParentLoc); bool Done = false; do { 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 -- 2.7.4