The path functions in this patch are unimplemented (as per the TODO comment from upstream). To avoid running into a linker error (missing symbol), this patch raises a compile error by commenting out the functions, which is more user friendly.
Details
- Reviewers
ldionne ctetreau Mordante - Group Reviewers
Restricted Project - Commits
- rG6b82adbb4980: Raise compile error when using unimplemented functions
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Above my pay grade, but...
(1) Isn't a runtime error vastly less user-friendly than a link-time error? Perhaps you could show an example of the kind of program that would run into this linker error and/or assertion?
(2) Is a plain assert really more appropriate than either _LIBCPP_ASSERT or throw? (I think _LIBCPP_ASSERT would be less appropriate because it compiles away in non-debug modes; but then, so does regular assert in NDEBUG mode.)
(3) Alternatively, how about either just commenting-out, or =delete'ing, these functions, to turn the linker error into a compile-time (and SFINAE'able) ill-formedness?
This patch seems to be a generic libc++ patch and not specific to z/OS right? If so please update the title of the path.
Please update the title and description of patch to describe the implementation using = delete.
Aside from that LGTM.
I agree it's weird we have those declarations but no implementation for them. I think my preferred approach here would be to just comment out those functions. That way, we know for sure we won't interact in weird ways with SFINAE or anything.
Commenting out the functions is what's closest to just not having implementing them (which is what's going on here).
Apart from that, LGTM.