Skip to content

Better error report#1965

Merged
wjakob merged 5 commits into
pybind:masterfrom
HDembinski:better_error_report
Oct 23, 2019
Merged

Better error report#1965
wjakob merged 5 commits into
pybind:masterfrom
HDembinski:better_error_report

Conversation

@HDembinski

Copy link
Copy Markdown
Contributor

This adds better error messages when py::cast is not used correctly.

@HDembinski

Copy link
Copy Markdown
Contributor Author

This job merely had a timeout, could someone with privileges please restart it?
https://travis-ci.org/pybind/pybind11/jobs/601280214

@wjakob

wjakob commented Oct 22, 2019

Copy link
Copy Markdown
Member

I would consider an error message like that, but only if compiled in debug mode. (Analogous to other uses of fancy string manipulation throughout the library, which include a note of the sort " (please recompile in debug mode for more details)". This is to avoid code bloat.)

@wjakob

wjakob commented Oct 22, 2019

Copy link
Copy Markdown
Member

build restarted.

@HDembinski

Copy link
Copy Markdown
Contributor Author

Ok, makes sense. My solution is still not perfect though, because the type name reported is without qualifiers, eq. const T& is reported as T. This is unfortunately how the typeid operator works, it drops all those qualifiers. Unfortunately, the qualifiers are critical to debug issues with the caster.

Keeping the qualifiers will be a more involved patch, which seems better suited for a separate PR.

@HDembinski

Copy link
Copy Markdown
Contributor Author

This version now only prints the type name in a debug build.

@wjakob

wjakob commented Oct 23, 2019

Copy link
Copy Markdown
Member

Looks good, thanks! Regarding getting the extra flags: I think the easiest and most cross-platform way is to wrap the type T into some dummy template foo<T>, stringify that, and then strip of the outer parts.

@wjakob wjakob merged commit bdf6a5e into pybind:master Oct 23, 2019
@HDembinski HDembinski deleted the better_error_report branch October 23, 2019 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants