davide requested changes to this revision.
davide added a comment.
This revision now requires changes to proceed.
Probably last round of comments. Thanks for your patience!
================
Comment at:
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile:7
+include $(LEVEL)/Makefile.rules
+CXXFLAGS += -O0
+CXXFLAGS += -std=c++17
----------------
you don't need this, it's implicit (`-O0`)
================
Comment at:
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:8
+
+lldbinline.MakeInlineTest(__file__, globals(), [no_debug_info_test])
----------------
I do wonder if you need a decorator to indicate that this is a libcxx only test
(and skip everywhere the library isn't supported).
================
Comment at: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:496
+ "libc++ std::optional synthetic children",
+ ConstString("^std::__(ndk)?1::optional<.+>(( )?&)?$"),
+ stl_synth_flags, true);
----------------
I'm a little nervous about this regex, but I think it's fine and I'll let Jim
take another look in case I missed something.
================
Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:42-51
+ ValueObjectSP engaged_sp(
+ valobj_sp->GetChildMemberWithName(ConstString("__engaged_"), true));
+
+ if (!engaged_sp)
+ return false;
+
+ llvm::StringRef engaged_as_cstring(
----------------
This is really obscure to somebody who doesn't understand the type internally.
Can you add a comment explaining the structure? (here or in the synthetic)
https://reviews.llvm.org/D49271
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
