lep / pjass

The famous jass syntax checker.
BSD 2-Clause "Simplified" License
25 stars 6 forks source link

Add old-patch flags #7

Closed lep closed 11 months ago

lep commented 11 months ago

Add flags to warn on modulo usage and to check for overly long names. Fixes #5 #4 @Luashine: are you able to compile this and then also test this? For now this checks the length of every name in a jass script. If there are different limits for different types of names it could get a bit more hairy.

lep commented 11 months ago

I'm still somewhat uneasy about this. Taking ac58f84a23afe447621dff7ffd84e36a92295817 for example was something that used to work in older patches and now doesn't anymore. Should this be backported?! And there are plenty other commits like that.

Luashine commented 11 months ago

Compilation of this branch fails, master is still OK.

make pjass
###
cc -MMD -g -w -c -o misc.o misc.c
misc.c:14:10: fatal error: sys/syslimits.h: No such file or directory
   14 | #include "sys/syslimits.h"
      |          ^~~~~~~~~~~~~~~~~
compilation terminated.
make: *** [GNUmakefile:58: misc.o] Error 1

https://stackoverflow.com/questions/51393089/fatal-error-sys-syslimits-h-file-not-found

is a GCC specific header (and the one on my Debian system can't even be included directly). The project should not be including that directly, but instead should include .

Compilation and tests run OK after following this advice. Did you want me to test anything else?


for example was something that used to work in older patches and now doesn't anymore. ... And there are plenty other commits like that.

Oh I didn't know the extent of this. If this is a sort of "memhack only" thing I would not care this much, but I look at it from the perspective of end-user cases.

The modulo flag usage was something "I heard about" and expected to work in Jass. Turns out it was added "sometime in some patch" and without a clear Jass documentation (Jeff Peng's is out of date, is it time to take it over?) I turned to pjass that's supposed to be my helper as a developer. This situation is caused by the game silently discarding scripts, leaving you clueless.

For memhack I believe it has enough publicity that it's not supposed to work. THough from a farther perspective this can be described as https://en.wikipedia.org/wiki/Institutional_memory that's only known in current moment of time.

Maybe instead of optional-izing all the recent developments, let me simply write it down in the readme? Probably along with disabled test samples. In case anyone ever wants to do a one-off work on an old map or expand pjass, at least it'll be in plain sight? The more I think about it, the more I like the idea to take our previous holy Jass website and update it alongside the notes in pjass' Readme.

PS: Do we have holyJass yet? TempleVM? :D

Luashine commented 11 months ago

I'd probably follow this up with a reorganization of tests, to have per non-default flag fail/check folders. Like double return in its own directory and update the make test to include these configurations properly.

Update: new commit Builds OK

lep commented 11 months ago

Compilation and tests run OK after following this advice. Did you want me to test anything else?

Fixed. I don't even know how it got in there. I'm interested if that limit is applied consitently or if there are different limits for different kind of names (locals, globals, functions, types, etc.).

lep commented 11 months ago

Oh I didn't know the extent of this. If this is a sort of "memhack only" thing I would not care this much, but I look at it from the perspective of end-user cases.

I don't know why that was changed. Memhack could be a possible cause. But there are other things that changed. At one point they added alias as a keyword w/o telling anybody what it does. And breaking maps who used that as a name. I think it's still in current wc3 hence this commit 7589b241e1179f85bf19b82696d97d8365034376. These two (null, alias) are just from the top of my head, there might be more.

Edit: i think that's actually it. modulo operator, null as (return) code, alias keyword. I couldn't find anything else so far. Maybe the "strict downcasting" option which pjass had at some point. That could have played a role in very early patches but maybe not (https://www.hiveworkshop.com/threads/pjass-updates.258738/#post-2798454 5f6ffd725b2ac9d24eff5917d96ceeeaaf748e5f).

lep commented 11 months ago

re jass.sf.net: That was an inspiration for jassdoc actually. I don't know how much work it would be to have something alike. One could probably copy most of it. The pjass commits should be a good guideline of what changed in jass over the years (thankfully not that much).