[Lldb-commits] [PATCH] D64042: [Symbol] Improve Variable::GetLanguage

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Tue Jul 23 03:16:03 PDT 2019


labath added inline comments.
Herald added a reviewer: jdoerfert.


================
Comment at: lldb/trunk/source/Symbol/Variable.cpp:61-69
+  if (auto *func = m_owner_scope->CalculateSymbolContextFunction()) {
+    if ((lang = func->GetLanguage()) && lang != lldb::eLanguageTypeUnknown)
+      return lang;
+    else if (auto *comp_unit =
+                 m_owner_scope->CalculateSymbolContextCompileUnit())
+      if ((lang = func->GetLanguage()) && lang != lldb::eLanguageTypeUnknown)
+        return lang;
----------------
I get a warning here (at least with gcc) about `comp_unit` variable being unused. Did you perhaps mean to do `lang = comp_unit->GetLanguage()` on the line below?

I would have fixed this myself, but when I started looking at this, I became unsure of that is this code exactly supposed to do. Is the check for the CompileUnit really supposed to be nested inside the check the existence of a function. I would have kind of expected it to be located outside the function if statement (`if (func) stuff(func); else if (cu) stuff(cu);`

I also find the `if( lang = ... && lang != Unknown)` pattern very confusing. As it stands now, it checks for the zero value twice (once due to the `(lang = ...)` part and once because eLanguageTypeUnknown is zero), but that part is very unobvious...


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64042





More information about the lldb-commits mailing list