[PATCH] preview patch for fixit for finding modules needing import/inclusion

John Thompson john.thompson.jtsoftware at gmail.com
Fri Jan 31 17:20:14 PST 2014


jtsoftware added you to the CC list for the revision "preview patch for fixit for finding modules needing import/inclusion".

Hi doug.gregor, rsmith,

This is the beginnings of a patch to add a fixit for automatically loading a module to resolve a type or symbol reference and outputting a fixit message.

I wanted to get some early feedback to make sure I'm on the right path, and also some answers for questions about a couple of difficult areas.

A problematic part is that the global module index seems to only include modules that have been imported before, and has not necessarily already been created or loaded yet.  To handle this, I added a loadGlobalModuleIndex to ModuleLoader that will create the module index if it doesn't yet exist, and will walk the top level modules in the module map and load any modules not already loaded (it will load them as hidden), for the side effect of loading the global module index with identifiers from all the modules.  This might involves two calls to create the index, as I can't compare it against the module map unless I have an index already.  This is one area I need feedback from someone who knows the module system, as there might be a better way.  I suppose we don't create a full index right off, because of the performance cost.

Then, in Sema I added a getTypeNameWithModuleCheck function to be used instead of getTypeName, which, if the type is not found, will do the load global index check, look up in the index any modules with the type identifier, and if found, make the module visible, redo the type search, and if found, display the fixit and continue.  If the type is not found, the module is made hidden again.

Another probable problem with this scheme is that apparently only top-level modules correspond to a module AST file, and the global module index only lists those top-level modules.  Is there code somewhere to find the module a symbol belongs to such that I can just make that module visible?

There's some other FIXME's in the code, such as one for figuring out the SourceLocation for where to put the fixit.  Any suggestions?

Perhaps the fixit message itself needs some revision, I just took a stab at it, as I wanted to get something out for feedback.

I added -fmodules-search-all and -fno-modules-search-all to enable or disable this (enabed by default).  I also added a need-import warning group.

This affected a few of the Modules tests.  I punted on it by just added a -fno-modules-search-all option to their command lines.

I added one new bare-bones test for this fixit, in test/Modules/undefined-type-fixit1.cpp.  More needs to be done to it to check the various scenarios of the global module index existing.

Because of the addition of an abstract function to ModuleLoader, I needed to put a stub for it in some unit tests that derive from it.

The formatting might be a bit spotty, as the focus here is to get some early feedback.

When and if this fixit and the infrastructure is good enough, then I can find other places to do similar fixit's for resolving identifiers.

Thanks.

-John


http://llvm-reviews.chandlerc.com/D2671

Files:
  test/Modules/auto-module-import.m
  test/Modules/undefined-type-fixit1.cpp
  test/Modules/module-private.cpp
  test/Modules/decldef.m
  test/Modules/cxx-inline-namespace.cpp
  test/Modules/decldef.mm
  test/Modules/Inputs/undefined-type-fixit/public2sub.h
  test/Modules/Inputs/undefined-type-fixit/module.map
  test/Modules/Inputs/undefined-type-fixit/public1.h
  test/Modules/Inputs/undefined-type-fixit/public2.h
  test/Modules/cxx-linkage-cache.cpp
  test/Modules/submodules.cpp
  include/clang/Frontend/CompilerInstance.h
  include/clang/Frontend/ASTUnit.h
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/LangOptions.def
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  include/clang/Lex/ModuleLoader.h
  include/clang/Lex/HeaderSearch.h
  include/clang/Driver/Options.td
  include/clang/Serialization/ASTReader.h
  include/clang/Serialization/GlobalModuleIndex.h
  unittests/Basic/SourceManagerTest.cpp
  unittests/Lex/LexerTest.cpp
  unittests/Lex/PPCallbacksTest.cpp
  unittests/Lex/PPConditionalDirectiveRecordTest.cpp
  unittests/Lex/CMakeLists.txt
  unittests/Lex/Makefile
  lib/Frontend/CompilerInstance.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaDecl.cpp
  lib/Lex/HeaderSearch.cpp
  lib/Parse/ParseDecl.cpp
  lib/Serialization/GlobalModuleIndex.cpp
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2671.1.patch
Type: text/x-patch
Size: 32997 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140131/99d4db74/attachment.bin>


More information about the cfe-commits mailing list