xiaobai marked an inline comment as done.
xiaobai added inline comments.
================
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;
----------------
labath wrote:
> 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...
Yikes, yes, the logic for this change is very busted. I don't know what I was
thinking, but I will upload a fix very soon.
I went with this pattern because I didn't want to assume eLanguageTypeUnknown
is 0 but it's unlikely to change anytime soon.
will upload a fix. sorry about this.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64042/new/
https://reviews.llvm.org/D64042
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
