[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added subscribers: jansvoboda11, vsapsai, dexonsmith.
bruno added a comment.

Thanks for working on this, comments inline. @vsapsai @jansvoboda11 @dexonsmith any headermap related concerns on your side?



================
Comment at: clang/lib/Lex/HeaderSearch.cpp:445
+        return None;
+      }
       return *Result;
----------------
I see, looks like this matches what clang already does in the non-hmaps path by calling `getFileAndSuggestModule` above, nice! 


================
Comment at: clang/lib/Lex/HeaderSearch.cpp:454
+      return None;
+    }
     return *Res;
----------------
Since this is the same as above, can this logic be put into `FixupSearchPath`? If so, it could be renamed to `FixupSearchPathAndSuggestModule`?


================
Comment at: clang/test/Modules/implicit-module-header-maps.cpp:1
+// RUN: rm -rf %T
+// RUN: mkdir %T
----------------
Maybe using `%t` would fix the bot errors?


================
Comment at: clang/test/Modules/implicit-module-header-maps.cpp:27
+#define FOO
+#include "Before/Mapping.h"
----------------
andrewjcg wrote:
> This include will fail if modules weren't used.
> 
> The include name itself doesn't exist and relies on the header map to remap it to the real header.
Can you add a comment like that to the test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930