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

Define op tables once as static variables #2242

Closed
wants to merge 1 commit into from

Conversation

gonzus
Copy link
Contributor

@gonzus gonzus commented Apr 10, 2019

This way we avoid declaring and initializing these tables every time we are parsing one of the corresponding ops.

This is my first pull request to this project. Apologies if I missed something.

This way we avoid declaring and initializing these tables every time we
are parsing one of the corresponding ops.
@gonzus
Copy link
Contributor Author

gonzus commented Apr 10, 2019

Hum... This is weird. I see the build jobs failing with this error:

src/parser.cpp:2318:1: sorry, unimplemented: non-trivial designated initializers not supported

And yet on my mac laptop it compiles fine with these options: c++ -std=c++11 -Wall. My compiler is:

$ c++ --version
Apple LLVM version 10.0.1 (clang-1001.0.46.3)
Target: x86_64-apple-darwin18.5.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

I believe this patch is worthwhile because it should give zig a bit more speed while parsing operators. Maybe we are using an older version of C++?

@andrewrk
Copy link
Member

Congrats on your first PR. Unfortunately for stage1 it's important to keep things working with a wide range of compilers.

Can I suggest that you look for issues labeled "contributor friendly"?

Another thing you can do is start working on a project in Zig, and then if you run into bugs or other issues, file high quality issue reports, and then look into solving them.

@andrewrk andrewrk closed this Apr 10, 2019
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.

None yet

2 participants