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

Add C++ compile test #615

Merged
merged 2 commits into from
Oct 26, 2024
Merged

Add C++ compile test #615

merged 2 commits into from
Oct 26, 2024

Conversation

bnoordhuis
Copy link
Contributor

Check that quickjs.h parses without error when fed to a C++ compiler.

@bnoordhuis
Copy link
Contributor Author

ffs, clang-cl.exe is on the path but cl.exe isn't. I have so many choice words for Windows and msvc.

@bnoordhuis
Copy link
Contributor Author

bangs head against desk

My search engine of choice tells me cl.exe is in ex. C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\bin\Hostx64\x64 but that feels too brittle to hard-code.

@saghul ideas?

@bnoordhuis
Copy link
Contributor Author

 ParserError: D:\a\_temp\f9fe22ae-45a5-4d5d-9a50-2c85a3b8440e.ps1:3
Line |
   3 |  … erprise/VC/Tools/MSVC/17.11.35327.3/bin/Hostx64/x64/cl.exe" /Zs cxxte …
     |                                                                 ~
     | You must provide a value expression following the '/' operator.

I give up. Towel -> ring.

@saghul
Copy link
Contributor

saghul commented Oct 23, 2024

I'll take a look, thanks for getting the ball rolling!

@saghul
Copy link
Contributor

saghul commented Oct 24, 2024

Got it!

Run cl.exe /Zs cxxtest.cc      
Microsoft (R) C/C++ Optimizing Compiler Version 19.41.34123 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

cxxtest.cc
D:\a\quickjs\quickjs\quickjs.h(489): error C7555: use of designated initializers requires at least '/std:c++20'
D:\a\quickjs\quickjs\quickjs.h(489): error C4576: a parenthesized type followed by an initializer list is a non-standard explicit type conversion syntax
D:\a\quickjs\quickjs\quickjs.h(494): error C7555: use of designated initializers requires at least '/std:c++20'
D:\a\quickjs\quickjs\quickjs.h(494): error C4576: a parenthesized type followed by an initializer list is a non-standard explicit type conversion syntax
D:\a\quickjs\quickjs\quickjs.h(504): error C7555: use of designated initializers requires at least '/std:c++20'
D:\a\quickjs\quickjs\quickjs.h(504): error C4576: a parenthesized type followed by an initializer list is a non-standard explicit type conversion syntax
D:\a\quickjs\quickjs\quickjs.h(905): error C7555: use of designated initializers requires at least '/std:c++20'
cxxtest.cc(8): error C7555: use of designated initializers requires at least '/std:c++20'
cxxtest.cc(8): error C4576: a parenthesized type followed by an initializer list is a non-standard explicit type conversion syntax

@saghul
Copy link
Contributor

saghul commented Oct 24, 2024

I suppose we should specify the standard version?

@bnoordhuis
Copy link
Contributor Author

The fact you need to pull in egor-tensin/vs-shell tells me it's way harder than it should be :-)

I'll add a /std: switch and make sure it gets tested with JS_NAN_BOXING on and off. Thanks!

@saghul
Copy link
Contributor

saghul commented Oct 24, 2024

I tried a couple of things but my powershell is weak so I thought...

image

Check that quickjs.h parses without error when fed to a C++ compiler.

Co-authored-by: Saúl Ibarra Corretgé <s@saghul.net>
@bnoordhuis
Copy link
Contributor Author

JS_MKVAL and friends work okay now but the JSCFunctionListEntry initializers are never going to work - not as constant expressions, and that's their intended use case, of course.

@saghul thoughts? I could drop them from cxxtest.cc and call it a day. For many users it probably works well enough now. It's definitely an improvement over the status quo. (I'll try that out in a separate commit.)

@saghul
Copy link
Contributor

saghul commented Oct 26, 2024

SGTM!

@bnoordhuis bnoordhuis merged commit 89883ae into quickjs-ng:master Oct 26, 2024
47 checks passed
@bnoordhuis bnoordhuis deleted the fix608 branch October 26, 2024 12:01
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