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 syntax errors in issue #34 #36

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

meaglyn
Copy link
Contributor

@meaglyn meaglyn commented Sep 15, 2022

The bad syntax error messages are caused by the bogus fix for
bison versions > 3.0. The original compiler all these are based
on was written for bison of 20 years ago and has a home rolled
error message generator and recovery mechanism. This doesn't work
with newer bison but bison can do this for us.

This removes support for bison <3.0 and enables the built in
syntax error reporting. This makes for a much better experience.
But it also removes all those compatibility macros since they were
only used for this handrolled error handling.

(Note, I don't know where the extra '\n's come from. They are in both
the before and after nwnsc when I build it. My nwnsscomp doesn't do that.)

err.nss

void main(){

 	if (TRUE) PrintString("A")

}

../nwnsc/nwnsc -i base_scripts err.nss

Before:
Compiling: err.nss
err.nss(5): Error: NSC1040: Syntax error at "J"

err.nss(6): Error: NSC1040: Syntax error at "�"

err.nss(7): Error: NSC1040: Syntax error at "�"

...

err.nss(104): Error: NSC1040: Syntax error at "�"

err.nss(104): Error: NSC1041: Compiler has reached the limit of 100 errors, aborting

Compilation aborted with errors.

After:
Compiling: err.nss
err.nss(5): Error: NSC1040: syntax error, unexpected '}', expecting ';'

Compilation aborted with errors.

I've set a limit of 32 errors. There was a limit of 100 but that's a lot for scripts. It's
hard to get to that now that it unwinds and recovers better.

I just noticed as I wrote this that it does drop the NSC1040 error number. Since this
path is all pretty much syntax errors it might be possible to hard code that back in...
V2: Added a fix for this and a hack to include the NSC1041 too many errors message
as well.

I've tested with a number of syntax errors on larger files and in includes and at different
places in the file etc. Also tested working scripts and 2nd pass errors (e.g. undefined
symbols and such) which all work as before since this really only effects the first pass
parsing.

I have not built or tested on MAC or Windows, as usual. Hopefully didn't break those.

Also, cleaned up a bit of code that was no longer needed.

Feel free to take this or not. I hope that it helps.

Cheers,
meaglyn

@meaglyn
Copy link
Contributor Author

meaglyn commented Sep 15, 2022

I guess I did break the build on windows at least. But there are no logs...

I'll try to hack the NSC1040 error code back in. Not sure there's a place to hook the too many errors one (NSC1041)

@meaglyn
Copy link
Contributor Author

meaglyn commented Sep 15, 2022

I fixed the NSCXXXX error messages, with the exception of a lowercase "syntax" versus an uppercase one.

@ELadner
Copy link

ELadner commented Sep 15, 2022

Tested this from your repo and got similar good results on error reporting. Great work!

@meaglyn
Copy link
Contributor Author

meaglyn commented Sep 16, 2022

@ELadner, thanks for testing it! Hopefully someone has access to the logs and can see what's wrong with the other builds.

@ELadner
Copy link

ELadner commented Sep 29, 2022

@virusman - what's this need to get approved? Possibly updating the bison version in the build environment would fix the error:

D:/a/nwnsc/nwnsc/_NscLib/NscParser.ypp:39.9-19: error: %define variable 'parse.error' is not used

perse.error is Bison 3.8.1+

@jakkn
Copy link
Contributor

jakkn commented Oct 16, 2022

Hi @meaglyn! I'm sorry this took so long. I can't tell what it's failing on, but here's the last part of the build log. Let me know if you need more.

2022-09-15T15:07:57.2364941Z MSBuild version 17.3.1+2badb37d1 for .NET Framework
2022-09-15T15:07:57.7678643Z   Checking Build System
2022-09-15T15:07:57.9623004Z   Generating NscParser.cpp, NscParser.hpp
2022-09-15T15:07:58.1045706Z   D:/a/nwnsc/nwnsc/_NscLib/NscParser.ypp: conflicts: 11 shift/reduce
2022-09-15T15:07:58.1046904Z   D:/a/nwnsc/nwnsc/_NscLib/NscParser.ypp:39.9-19: error: %define variable 'parse.error' is not used
2022-09-15T15:07:58.1227401Z C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Microsoft\VC\v170\Microsoft.CppCommon.targets(247,5): error MSB8066: Custom build for 'D:\a\nwnsc\nwnsc\CMakeFiles\c5640424ba52167b9e0b2377c5c3d5cd\NscParser.cpp.rule;D:\a\nwnsc\nwnsc\_NscLib\CMakeLists.txt' exited with code 1. [D:\a\nwnsc\nwnsc\_NscLib\nsclib.vcxproj]
2022-09-15T15:07:58.2030674Z   Building Custom Rule D:/a/nwnsc/nwnsc/_NwnBaseLib/CMakeLists.txt
2022-09-15T15:07:58.3807991Z   BaseTypes.cpp
2022-09-15T15:08:01.5605218Z   nwnbaselib.vcxproj -> D:\a\nwnsc\nwnsc\_NwnBaseLib\Release\nwnbaselib.lib
2022-09-15T15:08:01.6603166Z   Building Custom Rule D:/a/nwnsc/nwnsc/_NwnDataLib/CMakeLists.txt
2022-09-15T15:08:01.7621722Z   BifFileReader.cpp
2022-09-15T15:08:04.7226731Z   KeyFileReader.cpp
2022-09-15T15:08:06.8487256Z   NWScriptReader.cpp
2022-09-15T15:08:08.4614109Z D:\a\nwnsc\nwnsc\_NwnDataLib\NWScriptReader.cpp(851,6): warning C4477: 'sscanf' : format string '%s' requires an argument of type 'char *', but variadic argument 5 has type 'size_t' [D:\a\nwnsc\nwnsc\_NwnDataLib\nwndatalib.vcxproj]
2022-09-15T15:08:08.4619553Z D:\a\nwnsc\nwnsc\_NwnDataLib\NWScriptReader.cpp(857,6): warning C4474: 'sscanf' : too many arguments passed for format string [D:\a\nwnsc\nwnsc\_NwnDataLib\nwndatalib.vcxproj]
2022-09-15T15:08:08.4620701Z D:\a\nwnsc\nwnsc\_NwnDataLib\NWScriptReader.cpp(857,6): message : placeholders and their parameters expect 5 variadic arguments, but 7 were provided [D:\a\nwnsc\nwnsc\_NwnDataLib\nwndatalib.vcxproj]
2022-09-15T15:08:08.4624377Z D:\a\nwnsc\nwnsc\_NwnDataLib\NWScriptReader.cpp(821,7): warning C4996: 'fopen': This function or variable may be unsafe. Consider using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [D:\a\nwnsc\nwnsc\_NwnDataLib\nwndatalib.vcxproj]
2022-09-15T15:08:08.4627065Z D:\a\nwnsc\nwnsc\_NwnDataLib\NWScriptReader.cpp(827,3): warning C4996: 'strtok': This function or variable may be unsafe. Consider using strtok_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [D:\a\nwnsc\nwnsc\_NwnDataLib\nwndatalib.vcxproj]
2022-09-15T15:08:08.4628608Z D:\a\nwnsc\nwnsc\_NwnDataLib\NWScriptReader.cpp(839,4): warning C4996: 'strtok': This function or variable may be unsafe. Consider using strtok_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [D:\a\nwnsc\nwnsc\_NwnDataLib\nwndatalib.vcxproj]
2022-09-15T15:08:08.4630005Z D:\a\nwnsc\nwnsc\_NwnDataLib\NWScriptReader.cpp(849,9): warning C4996: 'sscanf': This function or variable may be unsafe. Consider using sscanf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [D:\a\nwnsc\nwnsc\_NwnDataLib\nwndatalib.vcxproj]
2022-09-15T15:08:08.9214165Z   ResourceManager.cpp
2022-09-15T15:08:11.1255841Z   Generating Code...
2022-09-15T15:08:12.9831086Z   nwndatalib.vcxproj -> D:\a\nwnsc\nwnsc\_NwnDataLib\Release\nwndatalib.lib
2022-09-15T15:08:13.1153496Z   Building Custom Rule D:/a/nwnsc/nwnsc/_NwnUtilLib/CMakeLists.txt
2022-09-15T15:08:13.2227918Z   BufferParser.cpp
2022-09-15T15:08:14.4114054Z   easylogging++.cc
2022-09-15T15:08:16.7937796Z   findfirst.cpp
2022-09-15T15:08:16.8157078Z   JSON.cpp
2022-09-15T15:08:17.1506004Z   JSONValue.cpp
2022-09-15T15:08:17.8006331Z   OsCompat.cpp
2022-09-15T15:08:19.2921590Z D:\a\nwnsc\nwnsc\_NwnUtilLib\OsCompat.cpp(30,27): warning C4996: 'strtok': This function or variable may be unsafe. Consider using strtok_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [D:\a\nwnsc\nwnsc\_NwnUtilLib\nwnutillib.vcxproj]
2022-09-15T15:08:19.2923369Z D:\a\nwnsc\nwnsc\_NwnUtilLib\OsCompat.cpp(31,13): warning C4996: 'strncpy': This function or variable may be unsafe. Consider using strncpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [D:\a\nwnsc\nwnsc\_NwnUtilLib\nwnutillib.vcxproj]
2022-09-15T15:08:19.7043060Z   spec.cpp
2022-09-15T15:08:19.7432372Z   version.cpp
2022-09-15T15:08:20.0253742Z   Generating Code...
2022-09-15T15:08:26.2419748Z   nwnutillib.vcxproj -> D:\a\nwnsc\nwnsc\_NwnUtilLib\Release\nwnutillib.lib
2022-09-15T15:08:26.2949916Z ##[error]Process completed with exit code 1.
2022-09-15T15:08:26.3204144Z Post job cleanup.

@ELadner
Copy link

ELadner commented Nov 4, 2022

Edited for brevity...

D:/.../NscParser.ypp: conflicts: 11 shift/reduce
D:/.../NscParser.ypp:39.9-19: error: %define variable 'parse.error' is not used
C:\...\Microsoft.CppCommon.targets(247,5): error MSB8066: Custom build for 'D:\a\nwnsc\...\NscParser.cpp.rule;D:\a\nwnsc\...CMakeLists.txt' exited with code 1. [D:\a\nwnsc\nwnsc\_NscLib\nsclib.vcxproj]

This still looks like the .ypp parser is causing the problem. I've tried to replicate this error on bison versions as old as 3.0 and can't. For the .ypp file, Are you using the GNU Bison parser generator or some other parser generator?

If you are using bison.. what's bison --version report? If you compile it directly on the command line, what's it say? bison NscParser.ypp

@ELadner
Copy link

ELadner commented Nov 4, 2022

@jd28
Copy link
Contributor

jd28 commented Jan 11, 2023

For windows build you need choco install winflexbison3

For macos, path is messed up. It isn't using the bison installed with brew, but some ancient version. It's so long ago I saw this, I forget exactly. The logs are gone now, but something like echo "/usr/local/opt/bison/bin" >> $GITHUB_PATH is needed in the action.

Cheers.

fixing more errors
@meaglyn
Copy link
Contributor Author

meaglyn commented Jun 17, 2023

Glad you got it to build. Sorry, I have not had much time for this lately. Thanks Jd28.

Fix syntax error reporting by using bison's built in error messages. Remove
the hand rolled recovery mechanism. This means it no longer supports bison <3.0
but it also means it works as is on both 3.0 and 3.6+.   Set errors to verbose.
Bison 3.6+ uses a type named context which was getting covered by the NscContext
name and causing compilation to fail on the generated code. Change that to "ctxt".
Restore NSC1040 error code to syntax errors and implement hack to
report NSC1041 when aborting for too many errors.
Remove NscBuildSyntaxError routine since it is no longer used.
There is no longer any code using the NWN_BISON_3 or NWN_BISON_3_6 defines
so remove them from CmakeLists.txt.
@meaglyn
Copy link
Contributor Author

meaglyn commented Jun 18, 2023

I updated this to the latest to resolve the conflict. Plus it's on top of linux-lower since I require that one to be able to use the compiler.
I can't help with the windows or mac-os builds.

@meaglyn
Copy link
Contributor Author

meaglyn commented Jun 18, 2023

Also, I've started using this compiler instead of my older version since updating it to get the new features was getting ... old. So hopefully we can get this and the lowercase thing in at some point :)

@ELadner
Copy link

ELadner commented Jun 23, 2023

The main problem is the nwnsc docker build image is using an old version of bison.

Found BISON: /usr/bin/bison (found version "2.4.1")

Perhaps the fix would be to bump it to a more recent version? I'm not even sure what distro would be shipping a bison version that old. Bison 2.4.1 was released in 2008; 3.8.2 was released ~9 months ago.

@ELadner
Copy link

ELadner commented Jun 23, 2023

Daz indicated that nwserver-linux is built on Ubuntu 18.04. Perhaps that would be a better build platform than the older holy build box that the nwnsc docker image is derived from.

@meaglyn
Copy link
Contributor Author

meaglyn commented Jul 14, 2023

Indeed. That would be good. I did try to preserve the older bison compatibility but it became a losing battle.

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

5 participants