[PATCH] D142992: [include-mapping] Implement language separation in stdlib recognizer library

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

Thanks, this looks good to me!

This requires some work to rebase for https://github.com/llvm/llvm-project/commit/e1aaa314a46cd303019da117bfd330611d5b7a84, I will rebase it and land it for you.



================
Comment at: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp:31
+
+static llvm::DenseMap<Lang, SymbolHeaderMapping *> LanguageMappings;
+
----------------
VitaNuo wrote:
> hokein wrote:
> > VitaNuo wrote:
> > > hokein wrote:
> > > > using a map here seems like an overkill, we have just 2 elements, I'd just use two separate variables (`CMapping`, and `CXXMapping`)
> > > what about the design idea that we might potentially want to extend this to multiple standards etc.? The idea is that it's extensible to `ObjC`, `OpenCL`... and so on and so forth, as has been discussed offline.
> > I think having a generic `Lang` enum structure is sufficient for the future extension, and I don't think we're going to add other languages in the foreseeable future (that's why I value the simple implementation at the beginning).
> > 
> > But you're right, getting the implementation right is probably a good idea. I'd like to remove the DenseMap, just use a raw array, something like below should work 
> > 
> > ```
> > enum Lang {
> >    C = 0,
> >    CXX,
> > 
> >    LastValue = CXX,
> > };
> > 
> > // access by e.g. LanguageMappings[static_cast<unsigned>(Lang::C)].
> > static SymbolHeaderMapping* LanguageMappings[static_cast<unsigned>(Lang::LastValue) + 1];
> > 
> > ```
> Ok, this will need some casting, but I don't have a strong opinion.
yeah, it is the sad bit, but I think it is OK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142992