yamaguchi 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]); ---------------- bruno wrote: > 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. > #import and #include should be treated the same way I'll add a test for #import as well. https://reviews.llvm.org/D48367 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits