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

Fix build with -std=c11 #20

Closed
wants to merge 1 commit into from
Closed

Fix build with -std=c11 #20

wants to merge 1 commit into from

Conversation

wezm
Copy link

@wezm wezm commented Aug 2, 2023

When building with gcc or clang passing -std=c11 scanner.c fails to build (helix builds grammars with this argument):

gcc:

src/scanner.c: In function ‘match_escape’:
src/scanner.c:70:30: warning: implicit declaration of function ‘isascii’ [-Wimplicit-function-declaration]
   70 |                         if (!isascii(lexer->lookahead) ||
      |                              ^~~~~~~

clang:

src/scanner.c:70:9: error: call to undeclared function 'isascii'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
                        if (!isascii(lexer->lookahead) ||
                             ^
src/scanner.c:81:9: error: call to undeclared function 'isascii'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
                        if (!isascii(lexer->lookahead) ||
                             ^
src/scanner.c:92:9: error: call to undeclared function 'isascii'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
                        if (!isascii(lexer->lookahead) ||
                             ^
src/scanner.c:126:9: error: call to undeclared function 'isascii'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
                        if (!isascii(lexer->lookahead) ||
                             ^
4 errors generated.

It seems that isascii is deprecated and is not visible by default with -std=c11. This PR attempts to address that although I'm not 100% sure it's the right fix. A better option might be to define isascii in this project if it's not already defined. Open to suggestions

@fweimer-rh
Copy link

I submitted a different fix as helix-editor/helix#8953.

@gdamore gdamore self-requested a review November 30, 2023 23:43
Copy link
Owner

@gdamore gdamore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not appropriate because _XOPEN_SOURCE is only for POSIX systems, and this may be used e.g. in Windows.

A simple solution could be to add a local definition:

#ifndef isascii
#define isascii(c) (((c) & 0x80) == 0)
#endif

Put that below the header includes.

@gdamore
Copy link
Owner

gdamore commented Nov 30, 2023

Note that there are no cases for Unicode where the above definition of isascii is incorrect. There are very few other encodings that don't meet this, and we don't support any of them. (E.g. EBCDIC or SHIFT-JIS.) All the Latin-X versions for example meet this criteria.

@gdamore
Copy link
Owner

gdamore commented Dec 1, 2023

I'm closing this in favor of #22 which fixes the same bug.

@gdamore gdamore closed this Dec 1, 2023
@wezm
Copy link
Author

wezm commented Dec 1, 2023

Cool, happy to have a fix either way.

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.

3 participants