[libcxx-commits] [PATCH] D144994: [Draft][libc++][modules] Adds std module.

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/test/tools/clang_tidy_checks/header_exportable_declarations.cpp:92
+
+static bool is_vaiable_declaration(const clang::NamedDecl* decl) {
+  // Declarations nested in records are automatically exported with the record itself.
----------------



================
Comment at: libcxx/test/tools/clang_tidy_checks/header_exportable_declarations.cpp:102-106
+  if (clang::CXXMethodDecl::classof(decl))
+    return false;
+
+  if (clang::CXXDeductionGuideDecl::classof(decl))
+    return false;
----------------
I think that's the idiomatic way to do this.


================
Comment at: libcxx/test/tools/clang_tidy_checks/header_exportable_declarations.cpp:141-156
+
+/// Returns the name is a reserved name.
+///
+/// This test misses 2 candidates which are not used in libc++
+/// * any identifier with two underscores not at the start
+/// * a name with a leading underscore in the global namespace
+bool is_reserved_name(const std::string& name) {
----------------
We have something very similar in `ugilfy_attributes.cpp`. Maybe we should add a header for a few utilities?


================
Comment at: libcxx/test/tools/clang_tidy_checks/header_exportable_declarations.cpp:165
+
+    std::string name = get_qualified_name(*decl);
+    if (is_reserved_name(name))
----------------
Is this equivalent to `decl->getName()`?


================
Comment at: libcxx/test/tools/clang_tidy_checks/header_exportable_declarations.cpp:172
+
+    std::cout << "using " << std::string{name} << ";\n";
+    decls_.insert(name);
----------------
What is this doing?


================
Comment at: libcxx/test/tools/clang_tidy_checks/header_exportable_declarations.hpp:21
+  const llvm::StringRef extra_header_;
+  std::set<std::string> decls_;
+};
----------------
Would an `unordered_set` be better?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144994