Page MenuHomePhabricator

Do not dereference std::unique_ptr by default

Authored by tberghammer on Mar 25 2017, 4:24 AM.



Displaying the object pointed by the unique_ptr can cause an infinite
recursion when we have a pointer loop so this change stops that
behavior. Additionally it makes the unique_ptr act more like a class
containing a pointer (what is the underlying truth) instead of some
"magic" class.

Diff Detail


Event Timeline

tberghammer created this revision.Mar 25 2017, 4:24 AM
jingham requested changes to this revision.Mar 27 2017, 10:36 AM

This test compiles correctly on Darwin if you pass the --std=c++11 flag. lldbtest has a getstdFlag that returns the correct std value in this case. Can you get the test running on Darwin with this?

Other than that this looks fine to me.

This revision now requires changes to proceed.Mar 27 2017, 10:36 AM
tberghammer requested review of this revision.Mar 27 2017, 12:54 PM
tberghammer edited edge metadata.

I am trying to compile it with the following command on OSX but I wasn't able to get it working:

clang  -std=c++11  -g -O0 -fno-builtin -arch x86_64   -fno-limit-debug-info -I$LLVM_ROOT/lldb/packages/Python/lldbsuite/test/make/../../../../../include -include $LLVM_ROOT/lldb/packages/Python/lldbsuite/test/make/test_common.h  -stdlib=libstdc++ -DLLDB_USING_LIBSTDCPP --driver-mode=g++ -c -o main.o main.cpp

Compile error (first few):

main.cpp:12:8: error: no member named 'unique_ptr' in namespace 'std'
  std::unique_ptr<char> nup;
main.cpp:12:23: error: expected '(' for function-style cast or type construction
  std::unique_ptr<char> nup;
main.cpp:12:25: error: use of undeclared identifier 'nup'; did you mean 'dup'?
  std::unique_ptr<char> nup;
/Applications/ note: 'dup' declared here
int      dup(int);
main.cpp:13:8: error: no member named 'unique_ptr' in namespace 'std'
  std::unique_ptr<int> iup(new int{123});
main.cpp:13:22: error: expected '(' for function-style cast or type construction
  std::unique_ptr<int> iup(new int{123});
main.cpp:13:24: error: use of undeclared identifier 'iup'; did you mean 'dup'?
  std::unique_ptr<int> iup(new int{123});
/Applications/ note: 'dup' declared here
int      dup(int);

I think the problem is that this is testing libstdc++ what is not available on OSX.

Clang version:

Apple LLVM version 7.3.0 (clang-703.0.31)
Target: x86_64-apple-darwin16.3.0
Thread model: posix
InstalledDir: /Applications/

There's no reason you couldn't build the gnu libstdc++ on Darwin. Anyway, if that's the problem, I'm pretty sure the testsuite has a way to conditionalize on which stdlib(s) are available. That would be clearer than conditionalizing on platform.

My understanding (don't have an OSX machine at hand right now to try out) is that OSX ships with the libstdc++ belonging to gcc-4.2.1 and that version of libstdc++ is too old to support c++11 features such as std::unique_ptr.

Regarding skipping tests I am not aware of any way to skip a test based on the STL library we are using (Pavel is working on it for libc++ at and actually the problem here is that the version of libstdc++ shipping on OSX is too old so I think skipIfDarwin is the correct decorator (we do it in several other tests as well). Alternative option could be to try to compile the source code and skip the test if compilation fails but that seems a bit flaky and might cause false negatives on other systems where we expect the test to pass.

I tried it out on OSX and the problem is that version of libstdc++ shipping with every recent OSX version (don't know about old ones) is 4.2.1 (last version with GPL v2) what doesn't support std::unique_ptr (supported since 4.3). Because of this I think the correct skip condition would be something like "skip if libstdc++ is older then 4.3" but I don't think we have a good way to specify that.

jingham accepted this revision.Mar 30 2017, 1:28 PM

MacOS hasn't shipped with gcc for a while now. If you were serious about using gcc & its STL implementation you would install your own copies of the tools & libraries. So what's on the system isn't really determinative... But this is a minor point, if there's no way to check the library version we can just leave the test off for Darwin.

This revision is now accepted and ready to land.Mar 30 2017, 1:28 PM
This revision was automatically updated to reflect the committed changes.