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

Do not disable GNU extensions when building grammars #8953

Closed
wants to merge 1 commit into from

Conversation

fweimer-rh
Copy link

Commit ca65d31 ("always build grammars with c++14 and c11 (#6792)") turned on semi-strict standard mode for C and C++ by accident. Some grammars use non-standard functions such as isascii (from <ctype.h>) and fail to build as a result, particularly with future compilers which do not support implicit function declarations.

(Ideally, the build environment would only raise the standard versions relative to the toolchain default, not lower them, but this change does not implement that.)

Related to:

Commit ca65d31 ("always build
grammars with c++14 and c11 (helix-editor#6792)") turned on semi-strict
standard mode for C and C++ by accident.  Some grammars use
non-standard functions such as isascii (from <ctype.h>) and
fail to build as a result, particularly with future compilers
which do not support implicit function declarations.

(Ideally, the build environment would only raise the standard
versions relative to the toolchain default, not lower them, but
this change does not implement that.)
@the-mikedavis the-mikedavis added A-tree-sitter Area: Tree-sitter S-waiting-on-review Status: Awaiting review from a maintainer. labels Nov 30, 2023
@pascalkuthe
Copy link
Member

I would rather see this fixed on the grammar side. Especially because we want grammars to build on windows too where gnu extensions are not available anyway. In this case there also seems to be a Microsoft specific isascii function but I am not actually sure it's equivalent.

Defining isascii is pretty trivial (it's even a macro on windows) so I would rather go that route

@fweimer-rh
Copy link
Author

I would rather see this fixed on the grammar side. Especially because we want grammars to build on windows too where gnu extensions are not available anyway. In this case there also seems to be a Microsoft specific isascii function but I am not actually sure it's equivalent.

It is equivalent: https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/isascii-isascii-iswascii?view=msvc-170

The current switch disables the extensions on GNU/Linux, but not on Windows, and probably not on the BSDs, either.

@pascalkuthe
Copy link
Member

sure but those are non-standard microsoft extensions. In this case its not a huge issue but there are many gnu extensions without equivalents on windows (and the reverse) or functions that work subtely differently. The function in question is even deprecated so its probably a good idea to move away from using it regardless.

Grammars should not be relying on non-standard C extensions that will just lead to portability issues.

@pascalkuthe
Copy link
Member

Closing as #9021 was merged which addresses the treesitter-d issue. Upstream made the same call as we did here: We should not be using POSIX/GNU extensions as that is not portable:

gdamore/tree-sitter-d#20 (review)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tree-sitter Area: Tree-sitter S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants