aprantl added inline comments.
================
Comment at: lldb/include/lldb/Symbol/CompileUnit.h:228
+ /// Apply a lambda to each external module referenced by this compilation
+ /// unit. Recursively also descends into the referenced external modules
----------------
What does "external module" mean in this context? Can we be specific about
lldb::Module vs clang::Module?
================
Comment at:
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp:442
+ std::string full_msg = "[C++ module config] " + msg;
+ LLDB_LOG(log, full_msg.c_str());
+ return CppModuleConfiguration();
----------------
if you use `LLDB_LOG(log, "[C++ module config] %s ", msg.c_str());` we only pay
the cost for concatenating if logging is on.
================
Comment at:
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp:1
+//===-- CppModuleConfiguration.cpp ------------------------------*- C++
-*-===//
+//
----------------
the `-*- C++ -*-` only makes sense for `.h` files were it is ambiguous whether
they contain C or C++ code.
================
Comment at:
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp:46
+ // we should remove that '/bits' suffix to get the actual include directory.
+ if (dir.endswith("/usr/include/bits"))
+ dir.consume_back("/bits");
----------------
llvm::sys::path::append to avoid hardcoding path separators?
================
Comment at:
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp:65
+ if (!analyzeFile(f))
+ break;
+
----------------
```
if (!std::all_of(supported_files.begin(), supported_files.end(), analyzeFile))
return;
```
?
================
Comment at:
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp:68
+ // Calculate the resource directory for LLDB.
+ m_resource_inc = GetClangResourceDir().GetPath() + "/include";
+
----------------
sys::path::append
================
Comment at:
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h:28
+ /// True iff this path hasn't been set yet.
+ bool m_first = true;
+
----------------
micro-optimization: may use less space if the two bools are grouped together
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67760/new/
https://reviews.llvm.org/D67760
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
