Add a new spelling for module map files 'module.modulemap'

Richard Smith richard at metafoo.co.uk
Tue Mar 18 12:05:30 PDT 2014


Documentation update:

+   Clang also accepts a module map file with the name ``module.map``, but
this will be deprecated in the future. When both ``module.modulemap`` and
``module.map`` files are present, the ``module.modulemap`` file will be
used.

"Will be deprecated in the future" doesn't make much sense to me. The only
functional meaning of "deprecated" here is "we intend to remove this some
day", so in that sense, if it'll be deprecated in the future, then it's
already deprecated now. Perhaps:

+   For compatibility with previous releases, if a module map file named
``module.modulemap`` is not found, Clang will also search for a file named
``module.map``. This behavior is deprecated and we plan to eventually
remove it.


+  StringRef Filename = llvm::sys::path::filename(ModuleMapPath);
+  SmallString<128>  PrivateFilename(Directory->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
+    return nullptr;

Maybe use StringSwitch?


+  // Continue to allow module.map
+  ModuleMapFileName.erase(ModuleMapFileName.begin() + ModuleMapDirNameLen,
+                          ModuleMapFileName.end());
+
+  llvm::sys::path::append(ModuleMapFileName, "module.map");
+  return FileMgr.getFile(ModuleMapFileName);

This makes a slightly dubious assumption about what llvm::sys::path:append
does to the string (that it only appends at the character level). Even
though that assumption turns out to be correct (for now, at least), I'd
prefer to not rely on it here.


On Tue, Mar 18, 2014 at 11:24 AM, Ben Langmuir <blangmuir at apple.com> wrote:

>
> On Mar 18, 2014, at 10:48 AM, Ben Langmuir <blangmuir at apple.com> wrote:
>
> >
> > On Mar 18, 2014, at 10:19 AM, Dmitri Gribenko <gribozavr at gmail.com>
> wrote:
> >
> >> On Tue, Mar 18, 2014 at 4:52 PM, Ben Langmuir <blangmuir at apple.com>
> wrote:
> >>> Thanks Argyrios.  Richard & Dmitri: just to confirm - are you okay
> with me
> >>> committing this now, or did either of you intend to review the patch
> first?
> >>
> >> +  // For frameworks, the preferred spelling is
> Modules/module.modulemap.
> >> +  // Otherwise it is just module.modulemap.
> >>
> >> ... Otherwise it is just "module.map" at the framework root.
> >
> > WIll do.
> >
> >>
> >> Apart from that, two comments:
> >>
> >> 1. There is also the "module_private.map" thing.  Do we want to rename
> >> that file as well?
> >
> > That is done in this patch as well - see loadModuleMapFileImpl.  If we
> find 'module.modulemap' we will look for 'module.private.modulemap'.  If we
> find 'module.map', we will look for 'module_private.map'.
> >
> >>
> >> 2. I don't immediately see how DirectoryHasModuleMap interacts with
> >> frameworks now.  Does it contain only root framework directories or
> >> 'Module' subdirectories as well?  Could you add a sub framework test
> >> to test this logic in hasModuleMap?
> >
> > Hmm, I didn't think about this.  Right now only the 'Modules'
> subdirectory will be in DirectoryHasModuleMap unless we are going through
> hasModuleMap, in which case I believe they will both be there.  This is
> probably safe, as we will not load the module map more than once, but it
> would be better to also add the framework directory so that we detect this
> earlier - I'll make that happen and add the test case you suggest.
>
> Actually we don't use hasModuleMap when searching for a sub framework.  We
> go through hasModuleMap anytime we search a 'normal' directory.  If you
> still think there's a missing test I'm open to suggestion though.
>
> Ben
> >
> > Ben
> >
> >>
> >> Dmitri
> >>
> >> --
> >> main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
> >> (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140318/8a386c09/attachment.html>


More information about the cfe-commits mailing list