Adds check if str(handle) correctly converted the object, and throw py::error_already_set if not.

[rwgk]

Similar to #2392, but does not depend on #2409.

Splitting out this PR from #2409 to make that PR as simple as possible.

Net effects of this PR:

• Adds missing test coverage.
• Changes TypeError to UnicodeDecodeError for Python 2.

This PR has two commits. Please do not squash, to make the behavior change obvious in the commit history.

[YannickJadoul]

Why doesn't this just cherry-pick the originally merged commit from my original PR where I debugged and fixed the issue, bd8f366? That way, all references and original authors are kept, and it is linked to the review process in #2392.

Apart from that, #2392 already went through a review process. Is this the exact same patch as #2392? Again, cherry-picking bd8f366 to master would make that more clear.

Let me also mention #2348 again, since it shows the relation to other str/bytes PRs. This should be considered a separate bug-fix after coming across this issue, though, so should be reasonably independent of the rest of those changes.

[rwgk]

No worries @YannickJadoul, I was just trying to rescue your work in a timely fashion. Please feel free to do it yourself, as suggested a few days ago in the slack chat, at your pace. I'll close the PR.

In the meantime I backed out this change from #2409 (the test change is a little different) and re-ran the Google-global testing. The very minor behavior change related to this PR had no impact at all. (With that #2409 is purely a change to make pybind11::str exclusively hold PyUnicodeObject.)

I will delete my branch. I don't know if the view of the diffs survives the delete. Here is the test change as under this PR (it's slightly different from your version), JIC it's useful:

tests/test_pytypes.py
+ assert m.str_from_handle(A()) == "this is a str"
+
+ malformed_utf8 = b"\x80"
+ assert m.str_from_object(malformed_utf8) is malformed_utf8 # Probably surprising.
+ if env.PY2:
+ with pytest.raises(UnicodeDecodeError):
+ m.str_from_handle(malformed_utf8)
+ else:
+ assert m.str_from_handle(malformed_utf8) == "b'\\x80'"
+

[YannickJadoul]

No, you're right, I did completely forget about this! The commit is quite distinct and can be merged. Should I just cherry pick and push to master, @rwgk? It was already reviewed, after all, just merged into this still branch.

[YannickJadoul]

The very minor behavior change related to this PR had no impact at all. (With that #2409 is purely a change to make pybind11::str exclusively hold PyUnicodeObject.)

I will delete my branch. I don't know if the view of the diffs survives the delete. Here is the test change as under this PR (it's slightly different from your version), JIC it's useful:

Oh, wait, so there is a difference I couldn't spot! Let's salvage that, then.