davide added a comment. Some comments. Jonas looked at many formatters recently so he's in a good position to take a look.
================ Comment at: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile:9 +CXXFLAGS += -std=c++17 +#CFLAGS += -std=c++17 ---------------- commented out code? ================ Comment at: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:21-22 + @add_test_categories(["libc++"]) +## @skipIf(debug_info="gmodules", +## bugnumber="https://bugs.llvm.org/show_bug.cgi?id=36048";) + def test_with_run_command(self): ---------------- ditto ================ Comment at: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp:14-38 + optional_int number ; + + printf( "%d\n", number.has_value() ) ; // break here + + number = 42 ; + + printf( "%d\n", *number ) ; // break here ---------------- You don't really need to se all these breakpoints. You just need set one on the return and print all the types. Also, an `lldbInline` python test here should suffice and it's probably more readable (grep for lldbInline). ================ Comment at: source/Plugins/Language/CPlusPlus/CMakeLists.txt:14-15 LibCxxTuple.cpp + LibCxxOptional.cpp LibCxxUnorderedMap.cpp LibCxxVector.cpp ---------------- Please sort this. ================ Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:48-51 + const char *engaged_as_cstring = engaged_sp->GetValueAsUnsigned(0) == 1 ? "true" : "false" ; + + stream.Printf(" engaged=%s", engaged_as_cstring ); + ---------------- Can you use StringRef? ================ Comment at: source/Plugins/Language/CPlusPlus/LibCxx.h:30-33 +bool LibcxxOptionalSummaryProvider( + ValueObject &valobj, Stream &stream, + const TypeSummaryOptions + &options); // libc++ std::optional<> ---------------- Please clang-format this patch. ================ Comment at: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp:59 + + //ValueObjectSP val_sp( m_backend.GetChildMemberWithName(ConstString("__val_"), true)); + ValueObjectSP val_sp( m_backend.GetChildMemberWithName(ConstString("__engaged_") , true)->GetParent()->GetChildAtIndex(0,true)->GetChildMemberWithName(ConstString("__val_"), true) ); ---------------- Please remove this commented out code. ================ Comment at: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp:63 + if (!val_sp) { + fprintf( stderr, "no __val_!\n" ) ; + return ValueObjectSP(); ---------------- if you want to log diagnostics, you might consider using the `LOG` facility and then add these to the `data formatters` channel. https://reviews.llvm.org/D49271 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
