stella.stamenova added a comment.
In https://reviews.llvm.org/D49271#1263842, @shafik wrote:
> @stella.stamenova Thank you for catching this. I fixed the test names, I am
> looking into the best way to fix the skipif now.
@shafik I send a change for review this morning that I think should do t
shafik marked 2 inline comments as done.
shafik added a comment.
@stella.stamenova Thank you for catching this. I fixed the test names, I am
looking into the best way to fix the skipif now.
https://reviews.llvm.org/D49271
___
lldb-commits mailing l
stella.stamenova added a comment.
I see this test failing on Linux right now (bots are down, so I can't confirm
that the official bots fail as well). The failure is because of the last
decorator which was not part of the review:
@skipIf(macos_version=["<", "10.14"])
It looks like this only w
shafik marked 4 inline comments as done.
shafik added a comment.
@labath Addressed comment, thank you for helping out.
Comment at:
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:21
+@add_t
shafik updated this revision to Diff 158662.
shafik added a comment.
Simplifying initialization of has_optional in test.
https://reviews.llvm.org/D49271
Files:
lldb.xcodeproj/project.pbxproj
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Ma
labath added a comment.
Thanks for the patience. I've tried the patch out on our end. You shouldn't
have any problems now.
Comment at:
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:21
+@
shafik updated this revision to Diff 158362.
shafik added a comment.
Fixing gcc skipIf to check for version as well
https://reviews.llvm.org/D49271
Files:
lldb.xcodeproj/project.pbxproj
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefil
shafik added inline comments.
Comment at:
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:21
+@add_test_categories(["libc++"])
+@skipIf(oslist=no_match(["macosx"]), compiler="clang",
com
shafik updated this revision to Diff 158342.
shafik marked 2 inline comments as done.
shafik added a comment.
- Adding comments
- Changing from exit to self.skipTest
- Adding skip for gcc
https://reviews.llvm.org/D49271
Files:
lldb.xcodeproj/project.pbxproj
packages/Python/lldbsuite/test/f
labath added inline comments.
Comment at:
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:21
+@add_test_categories(["libc++"])
+@skipIf(oslist=no_match(["macosx"]), compiler="clang",
com
shafik added a comment.
@davide @labath I believe I have addressed both the C++17 filter and the libc++
filter. Please let me know if you have further comments.
https://reviews.llvm.org/D49271
___
lldb-commits mailing list
lldb-commits@lists.llvm.o
shafik updated this revision to Diff 158121.
shafik added a comment.
Adjust test to filter on clang version and libc++ version to precent build bots
from failing due to lack of C++17 or lack of optional support
https://reviews.llvm.org/D49271
Files:
lldb.xcodeproj/project.pbxproj
packages
labath added a comment.
Thanks for looking out for the bots.
I am afraid the compiler check will not be enough here. After that, you'll run
into the issue of the system libc++ not being recent enough to contain
std::optional. I suppose this could be handled by including some other header
first
davide added a comment.
Bot logs for completeness:
Session logs for test failures/errors/unexpected successes will go into
directory 'logs-clang-3.5-i386'
Command invoked:
/lldb-buildbot/lldbSlave/buildWorkingDir/scripts/../llvm/tools/lldb/test/dotest.py
-A i386 -C clang-3.5 -v -s logs-cla
davide added a comment.
I'm afraid I had to revert this again. It broke an ubuntu lldb-cmake bot which
builds with clang-3.5 (which has no `-std=c++17` support flag, as 17 wasn't
there yet).
I'd recommend to update this patch with another decorator to skip places where
`-std=c++17` is not avai
davide added a comment.
Recommitted:
a11963323fc Recommit [DataFormatters] Add formatter for C++17 std::optional.
Authentication realm: https://llvm.org:443 LLVM Subversion repository
Password for 'davide': ***
Sendinglldb/trunk/lldb.xcodeproj/project.pbxproj
Adding
lldb/
davide added a comment.
Your latest update doesn't contain CMakeList.txt files and the Xcode project
changes. That's why it failed to apply. Please upload a correct version and
I'll commit it for you.
Repository:
rL LLVM
https://reviews.llvm.org/D49271
___
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337959: [DataFormatters] Add formatter for C++17
std::optional. (authored by davide, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D49271?vs=
shafik updated this revision to Diff 157337.
shafik added a comment.
Adding additional comments
https://reviews.llvm.org/D49271
Files:
lldb.xcodeproj/project.pbxproj
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile
packages/Python
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.
This is good, but please add a comment explaining the type before committing.
https://reviews.llvm.org/D49271
___
lldb-commits mailing list
lldb-
shafik marked 6 inline comments as done.
shafik added a comment.
@davide One more pass
Comment at:
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:8
+
+lldbinline.MakeInlineTest(__file__, globa
shafik updated this revision to Diff 156886.
shafik marked 4 inline comments as done.
shafik added a comment.
Addressing comments
- -O0 is not needed in Makefile
- engaged is not friendly terminology so switching to "Has Value"
- Switching test away from lldbinline style due to bug w/ .categories
jingham accepted this revision.
jingham added a comment.
This looks fine. I agree with Davide that putting some description of the type
you are formatting in comments somewhere would be make things easier for
somebody else who might have to fix this (or to you when you've totally
forgotten how
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/opti
JDevlieghere accepted this revision.
JDevlieghere added a comment.
LGTM, Thanks!
https://reviews.llvm.org/D49271
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
shafik marked 5 inline comments as done and 3 inline comments as done.
shafik added a comment.
@jingham @davide I believe I have addressed all your comments
https://reviews.llvm.org/D49271
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
ht
shafik updated this revision to Diff 155758.
shafik added a comment.
Refactoring test to use lldbinline
https://reviews.llvm.org/D49271
Files:
lldb.xcodeproj/project.pbxproj
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile
package
JDevlieghere added a comment.
Looks good to me, modulo the inline test (or the current comments addressed).
Thanks Shafik!
https://reviews.llvm.org/D49271
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailm
shafik updated this revision to Diff 155618.
shafik added a comment.
Removing unrelated changes due to applying clang-format to patch file with
context.
https://reviews.llvm.org/D49271
Files:
lldb.xcodeproj/project.pbxproj
packages/Python/lldbsuite/test/functionalities/data-formatter/data
davide added a comment.
This is getting really close. Please try the `lldbInline` test format and
revert the unrelated bits and I'll take another look.
Thanks!
Comment at: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:971-972
g_formatters.push_back([](lldb_priv
shafik added a comment.
@davide @jingham Thank you for the review, those were good catches and comments.
I have addressed them except for refactoring the test to use lldbInline which I
will be working on now.
Comment at:
packages/Python/lldbsuite/test/functionalities/data-fo
shafik updated this revision to Diff 155508.
shafik marked 7 inline comments as done.
shafik edited the summary of this revision.
shafik added a comment.
Address comments
- Applying clang-formt
- Refactoring OptionalFrontEnd to produce out that makes the underlying data
look like an array
- Remo
jingham requested changes to this revision.
jingham added a comment.
It seems a little odd to choose to represent this as an array with 0 elements.
I think I would be confused by that. You can maybe choose a better name like
"value" for your cloned object. But it looks like it is also possibl
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
shafik created this revision.
shafik added reviewers: jingham, friss, jasonmolenda.
Herald added subscribers: christof, mgorny.
Herald added a reviewer: EricWF.
Add formattors for libc++ std::optional and tests to verify the new formattors.
https://reviews.llvm.org/D49271
Files:
lldb.xcodepro
35 matches
Mail list logo
