[Lldb-commits] [PATCH] D147748: [lldb] Implement SymbolFile::ContainsCompileOption

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

At a higher level I wonder if this is really the best interface. If you ever need all the compile options, you probably want something like `Args SymbolFile::GetCompileOptions()`. Wouldn't that be a more generic way to do the same thing here? Or do we expect that for `DW_AT_APPLE_flags` the only possible use case is to check whether a particular flag is set?



================
Comment at: lldb/include/lldb/Symbol/SymbolFile.h:440
+  /// the symbol file.
+  virtual bool ContainsCompileOption(const char *option) {
+    return false;
----------------
Please use `StringRef`s instead of raw char pointers.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:4271
+    Args compiler_args(flags);
+    for (auto &arg : compiler_args.GetArgumentArrayRef())
+      if (strcmp(arg, option) == 0)
----------------
It's not obvious from the function name what `GetArgumentArrayRef` is going to return. Looking at the header, it's apparently a `llvm::ArrayRef<const char *>`, so `auto&` is a reference to a pointer?


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:4282-4285
+    DWARFUnit *dwarf_cu = debug_info.GetUnitAtIndex(cu_idx);
+    if (dwarf_cu) {
+      if (check_in_dwarf_cu(dwarf_cu))
+        return true;
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147748