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

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 1 01:57:29 PST 2023


hokein added a comment.

Can you add some tests in `StandardLibraryTest.cpp`?



================
Comment at: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp:31
+
+static llvm::DenseMap<Lang, SymbolHeaderMapping *> LanguageMappings;
+
----------------
using a map here seems like an overkill, we have just 2 elements, I'd just use two separate variables (`CMapping`, and `CXXMapping`)


================
Comment at: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp:100
+static int initialize() {
+  for (Lang Language : AllLangs)
+    initialize(Language);
----------------
nit: just `for (Lang Language: {Lang::C, Lang::CXX})` or two statements `initilize(Lang::C);` and `initialize(Lang::CXX);`. 


================
Comment at: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp:164
 
+  ensureInitialized();
+  SymbolHeaderMapping *Mapping = GetMapping(Language);
----------------
Do we need the `ensureInitialized()` here? looks like no needed, we have called it in the Recognizer constructor,


================
Comment at: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp:188
+  Lang RecognizerLang = Lang::CXX;
+  if (Language == clang::Language::C) {
+    RecognizerLang = Lang::C;
----------------
nit: just use `LangOpts.CPlusPlus` to check the language.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142992



More information about the cfe-commits mailing list