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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to