bruno added inline comments.

================
Comment at: clang/lib/Lex/HeaderSearch.cpp:285
     // directory.
-    loadSubdirectoryModuleMaps(SearchDirs[Idx]);
+    if (ModMap.getLangOpts().ObjC1 || ModMap.getLangOpts().ObjC2)
+      loadSubdirectoryModuleMaps(SearchDirs[Idx]);
----------------
yamaguchi wrote:
> bruno wrote:
> > yamaguchi wrote:
> > > bruno wrote:
> > > > aprantl wrote:
> > > > > Are these flags also enabled in Objective-C++ mode?
> > > > Looks like all this logic was introduced in r177621 to allow the names 
> > > > of modules to differ from the name of their subdirectory in the include 
> > > > path.
> > > > 
> > > > Instead of having this to be based on the language, it's probably 
> > > > better if we have it based on @import name lookup, which is the 
> > > > scenario where we actually currently look more aggressively, did you 
> > > > try that path?
> > > > 
> > > > This is also lacking a testcase, can you create one?
> > > > Are these flags also enabled in Objective-C++ mode?
> > > I think so from FrontendOptions.h:190
> > > `bool isObjectiveC() const { return Lang == ObjC || Lang == ObjCXX; }`
> > > 
> > > > it's probably better if we have it based on @import name lookup
> > > I don't think I understood what you're saying, could you explain a bit 
> > > more?
> > > 
> > > > This is also lacking a testcase, can you create one?
> > > Sure!
> > > I don't think I understood what you're saying, could you explain a bit 
> > > more?
> > 
> > This extra call to `loadSubdirectoryModuleMaps` was introduced in r177621 
> > to allow `@import SomeName` to work even if `SomeName` doesn't match a 
> > subdirectory name. See the original commit for more details.
> > 
> > This means that it was never supposed to be called when the module is built 
> > via `#include` or `#import`, which is what you are getting. That said, I 
> > believe it's better if the condition for calling 
> > `loadSubdirectoryModuleMaps` checks somehow if the lookup is coming from of 
> > an `@import` instead of checking the language (we too don't want it to be 
> > called for ObjC when `#include` or `#import` are used).
> > we too don't want it to be called for ObjC when #include or #import are used
> https://gist.github.com/yamaguchi1024/27caba1897eb813b297a8c4785adc11d
> This is one thing I could think of, it excludes `#include` but `#import` and 
> `@import` are still treated in the same way. Do you have any idea how to do 
> this? Is Clang differenciating the module behavior based on `#import` or 
> `@import` in ObjC?
> https://gist.github.com/yamaguchi1024/27caba1897eb813b297a8c4785adc11d

Something along these lines seems fine. I'd reverse the condition of 
`IsInclusionDirective` to be true by default (which is the most common case), 
and you pass false when coming from `@import`s. Maybe also rename it to 
something more meaningful for the change in question, around the lines of 
`AllowExtraModuleMapSearch`.

> This is one thing I could think of, it excludes #include but #import and 
> @import are still treated in the same way.

`#import` and `#include` should be treated the same way (extra searches 
shouldn't happen for both), `@import` is the only one different. 

> Do you have any idea how to do this? Is Clang differenciating the module 
> behavior based on #import or @import in ObjC?

`#import` works very much like `#include`, the difference is that `#import`'s 
have implicit `#pragma once` like behavior. 


https://reviews.llvm.org/D48367



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

Reply via email to