lhames added inline comments.
================
Comment at: Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2103
+static bool isOverload(clang::CXXMethodDecl *m1, clang::CXXMethodDecl *m2) {
+ // FIXME: This should detect covariant return types, but currently doesn't.
----------------
aprantl wrote:
> Could you add some doxygen comments explaining what the new function do and
> why doing this is necessary?
Will do.
================
Comment at: Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2105-2106
+ // FIXME: This should detect covariant return types, but currently doesn't.
+ assert(&m1->getASTContext() == &m2->getASTContext() &&
+ "Methods should have the same AST context");
+ clang::ASTContext &context = m1->getASTContext();
----------------
clayborg wrote:
> Use lldb_assert and possibly return false afterwards in case the asserts are
> compiled out
We can switch to lldb_assert to give us more control (at compile time) about
when we turn it on or off, but I don't think we should bail out: this is an
invariant, rather than a potential error case.
================
Comment at:
Python/lldbsuite/test/expression_command/call-overridden-method/main.cpp:15
+ Base *b = &d;
+ return 0; // Please test these expressions while stopped at this line:
+}
----------------
jingham wrote:
> aprantl wrote:
> > the expressions are missing :-)
> > Perhaps convert this into an inline testcase?
> The expressions are in the test .py file. It isn't necessary to put them
> here, I'd just fix the comment.
>
> Please don't convert regular test cases into inline ones, however. The
> benefit of inline test cases is mostly that they are easier to write, but
> they are harder to debug when they go wrong. So if the are already in
> regular form I'd rather not convert them.
The text of the comment was just cribbed from one of the other expression
tests. Is there a preferred phraseology?
return 0; // run expressions here
?
Repository:
rL LLVM
https://reviews.llvm.org/D41997
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
