[PATCH] D106394: [clang][pp] adds '#pragma include_instead'

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

Can't wait to start using this! (Note: this is not a full review, just some thoughts.)

Do you have a test case where

  // private: header one
  *exists*
  
  // private: header two
  #include<__header_one.h>
  
  // public: header three
  #include<__header_two.h>

I think right now you might see a warning that says "please include either `__header_two.h` or `header_three.h`" which is not right.

This begs the question, would it not be better to have a second argument that specifies what header should be used as a replacement? This would also allow the person writing the library to decide whether it's better to use angle brackets or quotes.



================
Comment at: clang/include/clang/Lex/HeaderSearch.h:465
+        getFileInfo(File).Aliases;
+    auto InsertionPoint = llvm::lower_bound(Aliases, Alias);
+    if (InsertionPoint != Aliases.end() && *InsertionPoint == Alias)
----------------
Is the logic here is just "if it's not already here add it?" If so, I think maybe you should just make `Aliases` a set (either a `StringSet` or `SmallSet`). Do you care about order? 


================
Comment at: clang/lib/Lex/PPLexerChange.cpp:429
+        };
+        std::string Aliases =
+            std::accumulate(Info.Aliases.begin() + 1, Info.Aliases.end(),
----------------
Can we use `llvm::interleaveComma` or `interleave`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106394