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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

I'm not in love with the design of this feature. Basing the accept / warn decision on whether the inclusion is in a system header seems fragile and doesn't seem to enforce the intended semantics of the warning (that you can only reach the implementation detail header through one of the named headers), and limits the utility of the feature to system headers. As an alternative, have you considered checking whether any of the headers named in the pragma are in the include stack, and warning if not? That would seem to make this warning applicable to both user headers and system headers in an unsurprising way.



================
Comment at: clang/include/clang/Lex/HeaderSearch.h:116-122
+  /// List of aliases that this header is known as.
+  /// Most headers should only have at most one alias, but a handful
+  /// have two.
+  llvm::SetVector<llvm::SmallString<32>,
+                  llvm::SmallVector<llvm::SmallString<32>, 2>,
+                  llvm::SmallSet<llvm::SmallString<32>, 2>>
+      Aliases;
----------------
This is a very heavyweight thing to include in `HeaderFileInfo` -- it looks like this member will add a couple of hundred bytes to the struct. Please consider a more efficient way of storing these, such as storing a map from header to aliases in `HeaderSearch` and only including a single bit here to say if there are any aliases for a header (so we can avoid checking the map in the common case).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106394