amccarth added a comment.
I have a couple inline questions. After that, it looks fine.
================
Comment at:
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp:61
+ for (auto it = parsed.begin(), end = parsed.end(); it != end; ++it) {
// Emplace valid dependent subtrees to make target assignment independent
----------------
I would recommend making `parsed` a `const` vector. The lambda captures the
iterator, and while the code currently looks fine, I'd hate for something to
change in the future that could invalidate the captured iterator.
================
Comment at:
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp:69
+ return pair.second;
+ }
+
----------------
This looks O(n^2). Will that matter here or will the expressions always be
short?
================
Comment at:
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp:84
// found target assignment program - no need to parse further
- return rvalue_ast;
+ return it->second;
}
----------------
It's a shame the `pair` loses the more descriptive field names, but I don't see
a good way to make it better, so this is a useless comment.
================
Comment at: lldb/trunk/source/Symbol/PostfixExpression.cpp:101
+ if (!rhs)
+ return {};
+ result.emplace_back(lhs, rhs);
----------------
Is it correct to return an empty vector here rather than `result`? If there's
one bad expression, you'll consider the entire FPO program empty. That's
sounds plausible, but I thought I'd ask.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66634/new/
https://reviews.llvm.org/D66634
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
