[PATCH] D75311: [modules] Allow frameworks to have only a private module without a public one.

2020-02-28 Thread Volodymyr Sapsai via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4069dd14124e: [modules] Allow frameworks to have only a 
private module without a public one. (authored by vsapsai).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75311/new/

https://reviews.llvm.org/D75311

Files:
  clang/lib/Lex/HeaderSearch.cpp
  
clang/test/Modules/Inputs/implicit-private-without-public/DeprecatedModuleMapLocation.framework/PrivateHeaders/A.h
  
clang/test/Modules/Inputs/implicit-private-without-public/DeprecatedModuleMapLocation.framework/module_private.map
  
clang/test/Modules/Inputs/implicit-private-without-public/Foo.framework/Modules/module.private.modulemap
  
clang/test/Modules/Inputs/implicit-private-without-public/Foo.framework/PrivateHeaders/Foo_Priv.h
  clang/test/Modules/implicit-private-without-public.m


Index: clang/test/Modules/implicit-private-without-public.m
===
--- /dev/null
+++ clang/test/Modules/implicit-private-without-public.m
@@ -0,0 +1,11 @@
+// REQUIRES: shell
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN:   -F%S/Inputs/implicit-private-without-public \
+// RUN:   -fsyntax-only %s -verify
+
+@import Foo_Private;
+
+// Private module map without a public one isn't supported for deprecated 
module map locations.
+@import DeprecatedModuleMapLocation_Private;
+// expected-error@-1{{module 'DeprecatedModuleMapLocation_Private' not found}}
Index: 
clang/test/Modules/Inputs/implicit-private-without-public/Foo.framework/PrivateHeaders/Foo_Priv.h
===
--- /dev/null
+++ 
clang/test/Modules/Inputs/implicit-private-without-public/Foo.framework/PrivateHeaders/Foo_Priv.h
@@ -0,0 +1 @@
+void foo_private(void);
Index: 
clang/test/Modules/Inputs/implicit-private-without-public/Foo.framework/Modules/module.private.modulemap
===
--- /dev/null
+++ 
clang/test/Modules/Inputs/implicit-private-without-public/Foo.framework/Modules/module.private.modulemap
@@ -0,0 +1,4 @@
+framework module Foo_Private {
+  header "Foo_Priv.h"
+  export *
+}
Index: 
clang/test/Modules/Inputs/implicit-private-without-public/DeprecatedModuleMapLocation.framework/module_private.map
===
--- /dev/null
+++ 
clang/test/Modules/Inputs/implicit-private-without-public/DeprecatedModuleMapLocation.framework/module_private.map
@@ -0,0 +1,4 @@
+framework module DeprecatedModuleMapLocation_Private {
+  header "A.h"
+  export *
+}
Index: 
clang/test/Modules/Inputs/implicit-private-without-public/DeprecatedModuleMapLocation.framework/PrivateHeaders/A.h
===
--- /dev/null
+++ 
clang/test/Modules/Inputs/implicit-private-without-public/DeprecatedModuleMapLocation.framework/PrivateHeaders/A.h
@@ -0,0 +1 @@
+void a(void);
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1568,6 +1568,16 @@
   llvm::sys::path::append(ModuleMapFileName, "module.map");
   if (auto F = FileMgr.getFile(ModuleMapFileName))
 return *F;
+
+  // For frameworks, allow to have a private module map with a preferred
+  // spelling when a public module map is absent.
+  if (IsFramework) {
+ModuleMapFileName = Dir->getName();
+llvm::sys::path::append(ModuleMapFileName, "Modules",
+"module.private.modulemap");
+if (auto F = FileMgr.getFile(ModuleMapFileName))
+  return *F;
+  }
   return nullptr;
 }
 


Index: clang/test/Modules/implicit-private-without-public.m
===
--- /dev/null
+++ clang/test/Modules/implicit-private-without-public.m
@@ -0,0 +1,11 @@
+// REQUIRES: shell
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN:   -F%S/Inputs/implicit-private-without-public \
+// RUN:   -fsyntax-only %s -verify
+
+@import Foo_Private;
+
+// Private module map without a public one isn't supported for deprecated module map locations.
+@import DeprecatedModuleMapLocation_Private;
+// expected-error@-1{{module 'DeprecatedModuleMapLocation_Private' not found}}
Index: clang/test/Modules/Inputs/implicit-private-without-public/Foo.framework/PrivateHeaders/Foo_Priv.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-private-without-public/Foo.framework/PrivateHeaders/Foo_Priv.h
@@ -0,0 +1 @@
+void foo_private(void);
Index: clang/test/Modules/Inputs/implicit-private-without-public/Foo.framework/Modules/module.private.modulemap
===
---

[PATCH] D75311: [modules] Allow frameworks to have only a private module without a public one.

2020-02-27 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits 
bruno accepted this revision.
bruno added a comment.
This revision is now accepted and ready to land.

Sounds great!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75311/new/

https://reviews.llvm.org/D75311



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75311: [modules] Allow frameworks to have only a private module without a public one.

2020-02-27 Thread Volodymyr Sapsai via Phabricator via cfe-commits 
vsapsai marked an inline comment as done.
vsapsai added a comment.

Looks like other module-related tracking like `LoadedModuleMaps`, 
`DirectoryHasModuleMap` works without extra changes. And `ModuleMapParser` 
relies on a file name to decide if a module map is private or not, so the 
presence of a public module map doesn't matter for it.




Comment at: clang/lib/Lex/HeaderSearch.cpp:1463-1470
   StringRef Filename = llvm::sys::path::filename(File->getName());
   SmallString<128>  PrivateFilename(File->getDir()->getName());
   if (Filename == "module.map")
 llvm::sys::path::append(PrivateFilename, "module_private.map");
   else if (Filename == "module.modulemap")
 llvm::sys::path::append(PrivateFilename, "module.private.modulemap");
   else

We don't try to strip/add file extensions, so won't try to load a module map 
like 'module.private.private.modulemap'.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75311/new/

https://reviews.llvm.org/D75311



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75311: [modules] Allow frameworks to have only a private module without a public one.

2020-02-27 Thread Volodymyr Sapsai via Phabricator via cfe-commits 
vsapsai created this revision.
vsapsai added reviewers: bruno, Bigcheese.
Herald added subscribers: ributzka, dexonsmith, jkorous.
Herald added a project: clang.
vsapsai marked an inline comment as done.
vsapsai added a comment.

Looks like other module-related tracking like `LoadedModuleMaps`, 
`DirectoryHasModuleMap` works without extra changes. And `ModuleMapParser` 
relies on a file name to decide if a module map is private or not, so the 
presence of a public module map doesn't matter for it.




Comment at: clang/lib/Lex/HeaderSearch.cpp:1463-1470
   StringRef Filename = llvm::sys::path::filename(File->getName());
   SmallString<128>  PrivateFilename(File->getDir()->getName());
   if (Filename == "module.map")
 llvm::sys::path::append(PrivateFilename, "module_private.map");
   else if (Filename == "module.modulemap")
 llvm::sys::path::append(PrivateFilename, "module.private.modulemap");
   else

We don't try to strip/add file extensions, so won't try to load a module map 
like 'module.private.private.modulemap'.


Support only preferred spelling 'Modules/module.private.modulemap' and
not the deprecated 'module_private.map'.

rdar://problem/57715533


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75311

Files:
  clang/lib/Lex/HeaderSearch.cpp
  
clang/test/Modules/Inputs/implicit-private-without-public/DeprecatedModuleMapLocation.framework/PrivateHeaders/A.h
  
clang/test/Modules/Inputs/implicit-private-without-public/DeprecatedModuleMapLocation.framework/module_private.map
  
clang/test/Modules/Inputs/implicit-private-without-public/Foo.framework/Modules/module.private.modulemap
  
clang/test/Modules/Inputs/implicit-private-without-public/Foo.framework/PrivateHeaders/Foo_Priv.h
  clang/test/Modules/implicit-private-without-public.m


Index: clang/test/Modules/implicit-private-without-public.m
===
--- /dev/null
+++ clang/test/Modules/implicit-private-without-public.m
@@ -0,0 +1,11 @@
+// REQUIRES: shell
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN:   -F%S/Inputs/implicit-private-without-public \
+// RUN:   -fsyntax-only %s -verify
+
+@import Foo_Private;
+
+// Private module map without a public one isn't supported for deprecated 
module map locations.
+@import DeprecatedModuleMapLocation_Private;
+// expected-error@-1{{module 'DeprecatedModuleMapLocation_Private' not found}}
Index: 
clang/test/Modules/Inputs/implicit-private-without-public/Foo.framework/PrivateHeaders/Foo_Priv.h
===
--- /dev/null
+++ 
clang/test/Modules/Inputs/implicit-private-without-public/Foo.framework/PrivateHeaders/Foo_Priv.h
@@ -0,0 +1 @@
+void foo_private(void);
Index: 
clang/test/Modules/Inputs/implicit-private-without-public/Foo.framework/Modules/module.private.modulemap
===
--- /dev/null
+++ 
clang/test/Modules/Inputs/implicit-private-without-public/Foo.framework/Modules/module.private.modulemap
@@ -0,0 +1,4 @@
+framework module Foo_Private {
+  header "Foo_Priv.h"
+  export *
+}
Index: 
clang/test/Modules/Inputs/implicit-private-without-public/DeprecatedModuleMapLocation.framework/module_private.map
===
--- /dev/null
+++ 
clang/test/Modules/Inputs/implicit-private-without-public/DeprecatedModuleMapLocation.framework/module_private.map
@@ -0,0 +1,4 @@
+framework module DeprecatedModuleMapLocation_Private {
+  header "A.h"
+  export *
+}
Index: 
clang/test/Modules/Inputs/implicit-private-without-public/DeprecatedModuleMapLocation.framework/PrivateHeaders/A.h
===
--- /dev/null
+++ 
clang/test/Modules/Inputs/implicit-private-without-public/DeprecatedModuleMapLocation.framework/PrivateHeaders/A.h
@@ -0,0 +1 @@
+void a(void);
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1568,6 +1568,16 @@
   llvm::sys::path::append(ModuleMapFileName, "module.map");
   if (auto F = FileMgr.getFile(ModuleMapFileName))
 return *F;
+
+  // For frameworks, allow to have a private module map with a preferred
+  // spelling when a public module map is absent.
+  if (IsFramework) {
+ModuleMapFileName = Dir->getName();
+llvm::sys::path::append(ModuleMapFileName, "Modules",
+"module.private.modulemap");
+if (auto F = FileMgr.getFile(ModuleMapFileName))
+  return *F;
+  }
   return nullptr;
 }
 


Index: clang/test/Modules/implicit-private-without-public.m
===
--- /dev/null
+++ clang/test/Modules/implicit-private-without-public.m
@@ -0,0 +1,11 @@
+// REQUIRES: shell
+// RUN: rm -rf