[PATCH] D125778: [llvm-debuginfo-analyzer] 03 - Logical elements
Paul Robinson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 23 08:17:20 PDT 2022
probinson accepted this revision.
probinson added a comment.
This revision is now accepted and ready to land.
Two nits and LGTM.
================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Core/LVSymbol.h:87
+ void setReference(LVElement *Element) override {
+ assert((!Element || dyn_cast<LVSymbol>(Element)) && "Invalid element");
+ setReference(static_cast<LVSymbol *>(Element));
----------------
I forgot about isa<> when I made my previous comment. This does less work and is clearer to the reader.
================
Comment at: llvm/unittests/DebugInfo/LogicalView/LogicalElementsTest.cpp:111
+ Root = getScopesRoot();
+ EXPECT_NE(Root, nullptr);
+
----------------
CarlosAlbertoEnciso wrote:
> probinson wrote:
> > I think `ASSERT_NE` here too.
> `ASSERT_NE` can be used only inside functions that don't return a value.
>
> See https://google.github.io/googletest/advanced.html#assertion-placement
>
> > The one constraint is that assertions that generate a fatal failure (FAIL* and ASSERT_*) can only be used in void-returning functions.
createElements does return `void` so ASSERT_NE can be used here.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125778/new/
https://reviews.llvm.org/D125778
More information about the llvm-commits
mailing list