Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wrapping of global decls (like isatty) incompatible with Dinkumware headers #5

Closed
matthargett opened this issue Apr 29, 2019 · 1 comment

Comments

@matthargett
Copy link

in gtest-port.h, they wrap xplat functions such as atty like this:

inline int IsATTY(int fd) { return isatty(fd); }

in Hermes' OSCompat.h, the same name+case is used, so :: has to be applied to make the namespace explicit.

inline int isatty(int fd) {
  return ::isatty(fd);
}

The Dinkumware-based headers on my platform don't explcitly add isatty() and friends to the C++ global namespace, so they can't be accessed with the preceding ::. Removing that explciit namespace makes it a recursive function call, which isn't correct. This kind of portability issue is presumably why the gtest maintainers decided to make the wrappers a different case.

I can submit a PR for isatty(), which is the only problem I'm having thus far, that changes the wrapper name to be cased like gtest does. Just let me know what case-style to use (isATTY vs IsATTY vs isAtty vs IsAtty, etc).

@matthargett
Copy link
Author

Found a workaround, so maybe not necessary.

facebook-github-bot pushed a commit that referenced this issue Sep 27, 2019
Summary:
So far this only runs `assembleDebug`, but it's a good start. Next up: C++ and Java tests. It also finishes in just 3 minutes.
Pull Request resolved: facebookincubator/fbjni#5

Test Plan: https://github.com/facebookincubator/fbjni/pull/5/checks?check_run_id=237409904

Reviewed By: dreiss

Differential Revision: D17602050

Pulled By: passy

fbshipit-source-id: fe5fe13771c5110e1e8b7c311f36fa5a52fd92fd
mganandraj pushed a commit to mganandraj/hermes that referenced this issue Oct 15, 2019
Summary:
So far this only runs `assembleDebug`, but it's a good start. Next up: C++ and Java tests. It also finishes in just 3 minutes.
Pull Request resolved: facebookincubator/fbjni#5

Test Plan: https://github.com/facebookincubator/fbjni/pull/5/checks?check_run_id=237409904

Reviewed By: dreiss

Differential Revision: D17602050

Pulled By: passy

fbshipit-source-id: fe5fe13771c5110e1e8b7c311f36fa5a52fd92fd
pylixonly pushed a commit to pylixonly/hermes that referenced this issue Jan 19, 2023
Co-authored-by: Ven <vendicated@riseup.net>
Co-authored-by: Diamond <33725716+DiamondMiner88@users.noreply.github.com>
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

No branches or pull requests

1 participant