ioccc-src / mkiocccentry

Form an IOCCC entry as a compressed tarball file
Other
28 stars 5 forks source link

Question: Issues that aren’t really major issues but are still issues #171

Closed xexyl closed 1 year ago

xexyl commented 2 years ago

This is for relevant to what is being worked on (like the JSON parser issue #156) but also some OT things. The purpose of this is to hopefully make it a little bit easier to find more relevant information in the other issues as already they’re very long and there are some comments that possibly could have been here instead.

There might be a better way to go about this. If so I am open to that but I think this might be a good idea. Of course we have to make use of it and discern the time to use it. Perhaps more than one of these could be made for the different tools: don’t know.

An example where it might be useful: sometimes I put an update on the day in the other issue but that doesn’t have to be there. Lately I have tried to do this on pull requests but there are still times where I don’t.

Hopefully this helps make the other issues (currently only #156) more clear and concise.

lcn2 commented 2 years ago

Are there and current "Issues that aren’t really issues but are still issues"?

lcn2 commented 2 years ago

Right now we are working on a issue related to how bison and flex are used.

We hope to have a potential update soon.

xexyl commented 2 years ago

Are there and current "Issues that aren’t really issues but are still issues"?

Right now? Probably not. It could have been worded differently but it's a fun way to put it. But as for an actual problem (and loosely defined here)? Not currently: it's for when there is one. Feel free to edit the name or if you have another idea to go about this I'm open to that.

xexyl commented 2 years ago

Right now we are working on a issue related to how bison and flex are used.

We hope to have a potential update soon.

Sounds good.

xexyl commented 2 years ago

As an example use of this thread: I'll be off for the day but I might be able to reply to some things in a bit if you write any comments. This is not an actual issue but I used to say this in the other thread which clutters it up. Perhaps I don't really need to say these things at all but I like to give updates in case something comes up (which has happened before).

Have a great rest of your day!

lcn2 commented 2 years ago

See commit d057026271c8654b2d184c01023d0768cb2e80f3 for a major update on how bison and flex are used.

UPDATE: Add commit 3d556f98f68dc17f494edb2d2a3806bc15e3eb71

lcn2 commented 2 years ago

We plan to replace the long Makefile rules with executing shell scripts for:

We do NOT plan to do this to every Makefile rule, just some of the longer rules.

With scripts such as run_bison.sh and run_flex.sh, we can do much more checking, and offer a better experience than long lines of backslashed compound Makefile rules. Such long compound Makefile rules are much harder to modify and understand.

lcn2 commented 2 years ago

With commit aea984df36b82a60596c4f3a53791bfa338611b9:

Add reset_tstamp.sh tool, updated MIN_TIMESTAMP

Simplified the reset_min_timestamp Makefile rule.

Changed MIN_TIMESTAMP from 1643987926 to 1650779341

We added a new tool: reset_tstamp.sh
lcn2 commented 2 years ago

With commit eeafc3e87a26c9fc3ad90b2057ebd28ee46e6cb3

Added test.sh and improved make test

Added test.sh to perform the complete suite of tests
for the mkiocccentry repo.

Updated make test to run ./test.sh.

Unlike the previous test process, if all the test code
is executable, then all the tests are performed
and only afterwards might test.sh exit non-zero.
lcn2 commented 2 years ago

We could do something for the make limit_ioccc.shrule, but we thing this is enough for now.

We need to return to the effort of completing the creation for compound JSON parse nodes.

NOTE: We have to focus on giving in the new few days on:

A Grand Coding Challenge - find a new Largest Known Prime

The Great indoor sport of hunting for 
world record-sized prime numbers

So plan to address completing the creation for compound JSON parse nodes in a few days, and we may also be slow in responding to GitHug questions and pull requests.

xexyl commented 2 years ago

Looks like a lot of great stuff has been committed! I will check it out in a bit when I am at the laptop. I hope you’re having a nice sleep my friend!

Will possibly reply more here when I am more awake.

xexyl commented 2 years ago

See commit d057026 for a major update on how bison and flex are used.

UPDATE: Add commit 3d556f9

Do we want them to be able to override the sorry file though? Probably unlikely that anyone but us will work on the parser but if someone did the wrong file could theoretically be prepended. Of course you would probably see that and unlikely that they would do it but the point is is there a need to allow overriding it?

xexyl commented 2 years ago

See commit d057026 for a major update on how bison and flex are used.

UPDATE: Add commit 3d556f9

I just fixed an issue under linux with mktemp. Committed it but not yet doing a pull request. Looking at other things first. I suspect today I won't get much done today though.

xexyl commented 2 years ago

We plan to replace the long Makefile rules with executing shell scripts for:

  • reset_min_timestamp
  • test

We do NOT plan to do this to every Makefile rule, just some of the longer rules.

With scripts such as run_bison.sh and run_flex.sh, we can do much more checking, and offer a better experience than long lines of backslashed compound Makefile rules. Such long compound Makefile rules are much harder to modify and understand.

It's funny you'd bring that up because yesterday I was thinking also how with a script it would be easier to do more tests including maybe shell script tests. I don't know if there's a Makefile lint and that's what I was thinking of but with a shell script we can use shellcheck so that's good. And yes the backslashed lines required in Makefiles can complicate matters of course.

xexyl commented 2 years ago

With commit aea984d:

Add reset_tstamp.sh tool, updated MIN_TIMESTAMP

Simplified the reset_min_timestamp Makefile rule.

Changed MIN_TIMESTAMP from 1643987926 to 1650779341

We added a new tool: reset_tstamp.sh

It's not that important to me but: why did you change it? I mean since the contest is not running does it matter? Perhaps it was just to test the new script?

xexyl commented 2 years ago

Unlike the previous test process, if all the test code is executable, then all the tests are performed and only afterwards might test.sh exit non-zero.

I like that idea of letting it go to the very end and then report any issues at the end!

xexyl commented 2 years ago

We could do something for the make limit_ioccc.shrule, but we thing this is enough for now.

Curious what the point would be here?

We need to return to the effort of completing the creation for compound JSON parse nodes.

Yes please.

NOTE: We have to focus on giving in the new few days on:

A Grand Coding Challenge - find a new Largest Known Prime

The Great indoor sport of hunting for 
world record-sized prime numbers

That sounds fun! Will there be a place where you announce anything about it - including hopefully a new prime?

So plan to address completing the creation for compound JSON parse nodes in a few days, and we may also be slow in responding to GitHug questions and pull requests.

Sounds good to me.

lcn2 commented 2 years ago

With commit aea984d:


Add reset_tstamp.sh tool, updated MIN_TIMESTAMP

Simplified the reset_min_timestamp Makefile rule.

Changed MIN_TIMESTAMP from 1643987926 to 1650779341

We added a new tool: reset_tstamp.sh

It's not that important to me but: why did you change it? I mean since the contest is not running does it matter? Perhaps it was just to test the new script?

It was a useful test, yes.

xexyl commented 2 years ago

It's not that important to me but: why did you change it? I mean since the contest is not running does it matter? Perhaps it was just to test the new script?

It was a useful test, yes.

Makes sense.

lcn2 commented 2 years ago

That sounds fun! Will there be a place where you announce anything about it - including hopefully a new prime?

No new prime, just improvements to methods of finding them.

xexyl commented 2 years ago

That sounds fun! Will there be a place where you announce anything about it - including hopefully a new prime?

No new prime, just improvements to methods of finding them.

Okay but will there be any place that is documented?

lcn2 commented 2 years ago

Curious what the point would be here?

It would only be to convert a Makefile code blob into a script. This might or might not be with the effort.

xexyl commented 2 years ago

Curious what the point would be here?

It would only be to convert a Makefile code blob into a script. This might or might not be with the effort.

Ah. I see. That makes sense yes.

lcn2 commented 2 years ago

See commit d057026 for a major update on how bison and flex are used.

UPDATE: Add commit 3d556f9

Do we want them to be able to override the sorry file though? Probably unlikely that anyone but us will work on the parser but if someone did the wrong file could theoretically be prepended. Of course you would probably see that and unlikely that they would do it but the point is is there a need to allow overriding it?

It made the tool more general and easier to adopt by others in other projects.

xexyl commented 2 years ago

See commit d057026 for a major update on how bison and flex are used.

UPDATE: Add commit 3d556f9

Do we want them to be able to override the sorry file though? Probably unlikely that anyone but us will work on the parser but if someone did the wrong file could theoretically be prepended. Of course you would probably see that and unlikely that they would do it but the point is is there a need to allow overriding it?

It made the tool more general and easier to adopt by others in other projects.

Ah. I see. Well in that case I'm sorry for asking (well I'm not - but the possibility to make a pun should never be ignored). Thanks for clarifying.

xexyl commented 2 years ago

I'm about to leave for the day but wanted to ask if it was intentional that make build could pass even if flex or bison don't have the right version?

I also want to express frustration (this is just for myself) of a copy/paste error when fixing the commit resulting in barmy commit log in part:

    This also happened for flex. Thus the following diff was added to both
    run_flex.sh and run_bison.sh:

        -    TMP_FILE=$(mktemp -t "$FILE")
        +    TMP_FILE=$(mktemp -t "$FILE.XXXXXXXX")

    This works under linux as well as macOS.

    Some words about required versions of flex and bison. After a recent

    This also happened for flex. Thus the following diff was added to both
    run_flex.sh and run_bison.sh:

        -    TMP_FILE=$(mktemp -t "$FILE")
        +    TMP_FILE=$(mktemp -t "$FILE.XXXXXXXX")

    This works under linux as well as macOS.

I mean most people probably won't even notice it but it annoys me. That's what I get for doing it earlier on in the day I guess.

Anyway good day! More from me tomorrow I'm sure. Enjoy the rest of your weekend!

lcn2 commented 2 years ago

I'm about to leave for the day but wanted to ask if it was intentional that make build could pass even if flex or bison don't have the right version?

It was not intentional: that is a bug 🐛. For make prep, yes, but not make build.

xexyl commented 2 years ago

I'm about to leave for the day but wanted to ask if it was intentional that make build could pass even if flex or bison don't have the right version?

It was not intentional: that is a bug 🐛. For make prep, yes, but not make build.

I presume you will fix this in an upcoming commit? And I will be going to sleep now.

Good night! Enjoy the rest your day.

Edit: Oh I do have a question about the flex and bison scripts possibly but I will worry about that later. Has to do with the usage message of each script.

xexyl commented 2 years ago

Unlike the previous test process, if all the test code is executable, then all the tests are performed and only afterwards might test.sh exit non-zero.

I like that idea of letting it go to the very end and then report any issues at the end!

Oh something I noticed with this though. It can happen that the error code changes if more than one error is encountered. Should there be a list of errors given at the end and then an exit code that says that there was at least one error?

I don’t recall which script(s) that I saw this In but I know at least one has this.

Really going to sleep now. Good night!

xexyl commented 2 years ago

Light fun humoured question for the parser: I just learnt of the %define api.prefix {PREFIX} directive. This is how it is described in the manual:

 -- Directive: %define api.prefix {PREFIX}

        * Language(s): C, C++, Java

        * Purpose: Rename exported symbols.  *Note Multiple Parsers::.

        * Accepted Values: String

        * Default Value: 'YY' for Java, 'yy' otherwise.

        * History: introduced in Bison 2.6, with its argument in double
          quotes.  Uses braces since Bison 3.0 (double quotes are still
          supported for backward compatibility).

There's also for both bison and flex a command line option (different case letters of course) so we could have the prefix yy instead be say ugly_.

This would be another fun way to say how horrible the flex and bison generated code really is. Of we could choose another prefix. Just a thought though: I don't mind either way.

I'm not going to work on the repo any more today but I will reply to comments if you say anything.

EDIT: Mind you I don't know if I like the look of ugly_ and I'm not sure which I prefer - yy or the other. But it crossed my mind and it seemed fun so I decided to bring it up.

xexyl commented 2 years ago

Unlike the previous test process, if all the test code is executable, then all the tests are performed and only afterwards might test.sh exit non-zero.

I like that idea of letting it go to the very end and then report any issues at the end!

Oh something I noticed with this though. It can happen that the error code changes if more than one error is encountered. Should there be a list of errors given at the end and then an exit code that says that there was at least one error?

I don’t recall which script(s) that I saw this In but I know at least one has this.

At least prep.sh and test.sh have this issue.

lcn2 commented 2 years ago

With commit commit 1dded71abe16c08f9d0378b78f82c7ff19ba55ac

Fixed make build release pull to not use backup files

The build release pull will run prep.sh with -o,
which in turn will run run_bison.sh and run_flex.sh with -o.

The effect of -o is to prevent the bison and flex backup files
from being used.  If -o is given and bison and/or flex is
not found or is too old, then the operation will fail with
a non-zero exit code.

The make prep rule will, and before, make use of
backup files, unless the RUN_O_FLAG is set to -o
(which it is not by default).
lcn2 commented 2 years ago

Unlike the previous test process, if all the test code is executable, then all the tests are performed and only afterwards might test.sh exit non-zero.

I like that idea of letting it go to the very end and then report any issues at the end!

Oh something I noticed with this though. It can happen that the error code changes if more than one error is encountered. Should there be a list of errors given at the end and then an exit code that says that there was at least one error?

I don’t recall which script(s) that I saw this In but I know at least one has this.

Really going to sleep now. Good night!

With commit 63e40d8abf16bc17da4ce5a5ac580dec7d437e1f

Fixed test.sh and add test failure summary at the end

Fixed some tests where the exit status was not being properly checked.

If test(s) fail, a summary of the tests that failed is
printed near the end of the run.
xexyl commented 2 years ago

With commit commit 1dded71

Fixed make build release pull to not use backup files

The build release pull will run prep.sh with -o,
which in turn will run run_bison.sh and run_flex.sh with -o.

The effect of -o is to prevent the bison and flex backup files
from being used.  If -o is given and bison and/or flex is
not found or is too old, then the operation will fail with
a non-zero exit code.

The make prep rule will, and before, make use of
backup files, unless the RUN_O_FLAG is set to -o
(which it is not by default).

Sounds good.

I will be going to sleep now. Have a great night! More from me tomorrow.

xexyl commented 2 years ago

Unlike the previous test process, if all the test code is executable, then all the tests are performed and only afterwards might test.sh exit non-zero.

I like that idea of letting it go to the very end and then report any issues at the end!

Oh something I noticed with this though. It can happen that the error code changes if more than one error is encountered. Should there be a list of errors given at the end and then an exit code that says that there was at least one error? I don’t recall which script(s) that I saw this In but I know at least one has this. Really going to sleep now. Good night!

With commit 63e40d8

Fixed test.sh and add test failure summary at the end

Fixed some tests where the exit status was not being properly checked.

If test(s) fail, a summary of the tests that failed is
printed near the end of the run.

Thank you. Will look at it tomorrow. Good night! Putting phone down now. Sleep well when you do.

lcn2 commented 2 years ago

Good evening 😴 @xexyl

xexyl commented 2 years ago

I hope you're having a nice sleep! I cannot sleep so I decided to get up though I might try and sleep again soon. Anyway addressing this from yesterday:

Light fun humoured question for the parser: I just learnt of the %define api.prefix {PREFIX} directive. This is how it is described in the manual:

...

There's also for both bison and flex a command line option (different case letters of course) so we could have the prefix yy instead be say ugly_.

This would be another fun way to say how horrible the flex and bison generated code really is. Of we could choose another prefix. Just a thought though: I don't mind either way.

This has been done in an unpublished commit. I think you'll like the git log as well! I decided to do it because you reacted with a laugh at it.

UPDATE: See commit 2020812c:

Lexer and parser ironically declare they're ugly

This is done by changing the (yy|YY) prefix to (ugly|UGLY) in the lexer
and parser. As the following comment says:

    /*
     * As we utterly object to the hideous code that bison and flex generate we
     * point it out in an ironic way by changing the prefix yy to ugly_.
     */

This is done in flex and bison via:

    %option prefix="ugly_"
    %define api.prefix {ugly_}

respectively. This creates an ironic statement in the lexer:

    #define ugly_lex_ALREADY_DEFINED

and an ironic statement in the C pre-processor in the parser:

    #if ! defined UGLY_STYPE && ! defined UGLY_STYPE_IS_DECLARED
    # define UGLY_STYPE_IS_DECLARED 1

which is to say that both the lexer and parser actually declare that
they're ugly! :))

For the curious:

    $ grep -ic ugly jparse*ref.[ch]
    jparse.ref.c:108
    jparse.tab.ref.c:34
    jparse.tab.ref.h:24

which is a huge amount of ugliness (though it's much more ugly than what
it admits :) )!

As well I've made a minor change to sorry.tm.ca.h that's not even worth
describing.

But there's a minor fact with the ugly_ prefix now that we use the struct json (as noted in the other thread) namely to do with the typedef. I'll just let you react to that over there. I'm going to do something else now.

xexyl commented 2 years ago

Oh one more thing.

Should make prep report that all tests passed when flex and/or bison cannot generate files? It should be that it can continue certainly but should it report that it all passed? If it shouldn't it does. Now I really am going afk.

lcn2 commented 2 years ago

Oh one more thing.

Should make prep report that all tests passed when flex and/or bison cannot generate files? It should be that it can continue certainly but should it report that it all passed? If it shouldn't it does. Now I really am going afk.

The make prep rule is a best effort procedure for the platform it is running on, so if it manages to compile using backup files, then yes.

UPDATE: Use make release for more a more relaxed oriented action.

xexyl commented 2 years ago

The make prep rule is a best effort procedure for the platform it is running on, so if it manages to compile using backup files, then yes.

Just wanted to be sure.

xexyl commented 2 years ago

UPDATE: Use make release for more a more relaxed oriented action.

I use make build so that's the same thing as I added that alias. I just wanted to know if make prep should actually succeed if it can't generate the backup files. It's not an issue for me because I do it under macOS and I presume anyone else who would want to work on the parser would actually have the correct versions.

xexyl commented 2 years ago

Heading off for the day but I wondered something a number of times: do we want a bignum library for the json parser? Should one be considered?

Good day!

lcn2 commented 2 years ago

Heading off for the day but I wondered something a number of times: do we want a bignum library for the json parser? Should one be considered?

Good day!

No, that is well beyond the scope. And there are too many to choose from.

About the best one could do is to use the original number string and call your own bignum of choice.

xexyl commented 2 years ago

Heading off for the day but I wondered something a number of times: do we want a bignum library for the json parser? Should one be considered? Good day!

No, that is well beyond the scope. And there are too many to choose from.

On the first part I figured and on the second part agreed.

About the best one could do is to use the original number string and call your own bignum of choice.

How could we integrate it though? I guess I was thinking more for if I do make it part of my own repo. Maybe it's not worth it: perhaps for C it's not necessary. Sure json can have bigger numbers but the max/min of 64-bit integers are quite large.

Anyway just a thought.

xexyl commented 2 years ago

Question for you @lcn2:

I was going through the parser/scanner source files and changing the references of yy to ugly_. Now this is fine but there are some cases where the name actually is prefixed as ugly__ because the original symbol name was prefixed yy_. This is purely a cosmetic opinion I'm after:

Do I change the prefix to ugly without the underscore or do in those cases I just use ugly__?

The reason I added the underscore initially is to help with spellcheck but to me ugly__scan_string is itself uglier than ugly_scan_string for example. I'm not sure. What do you think?

Of course maybe we're taking this too far (by which I mean 'maybe I am taking this too far' :) ) but it's giving me some joy to think about as well so it'll do. :)

EDIT: I will not do this commit until I get your feedback.

lcn2 commented 2 years ago

How could we integrate it though? I guess I was thinking more for if I do make it part of my own repo. Maybe it's not worth it: perhaps for C it's not necessary. Sure json can have bigger numbers but the max/min of 64-bit integers are quite large.

You would use the as_str copy of the JSON string and pass it to the calc library or gmp library for conversion.

lcn2 commented 2 years ago

Question for you @lcn2:

I was going through the parser/scanner source files and changing the references of yy to ugly_. Now this is fine but there are some cases where the name actually is prefixed as ugly__ because the original symbol name was prefixed yy_. This is purely a cosmetic opinion I'm after:

Do I change the prefix to ugly without the underscore or do in those cases I just use ugly__?

The reason I added the underscore initially is to help with spellcheck but to me ugly__scan_string is itself uglier than ugly_scan_string for example. I'm not sure. What do you think?

Of course maybe we're taking this too far (by which I mean 'maybe I am taking this too far' :) ) but it's giving me some joy to think about as well so it'll do. :)

Is not the problem that sometimes they are inconsistent in their _ use? Nevertheless searching for ugly_ would catch those cases where ugly__ was generated. Someone who knew about bison/flex or yacc/lex output would expect yy_ and seeing ugly_ would adapt. However if they saw just uglyfoo they would not know/might suspect if that it was one of your variables or one from bison/flex.

If you are going to change the prefix, you might be best to use ugly_ and accept the cases where bison/flex generates a __.

xexyl commented 2 years ago

How could we integrate it though? I guess I was thinking more for if I do make it part of my own repo. Maybe it's not worth it: perhaps for C it's not necessary. Sure json can have bigger numbers but the max/min of 64-bit integers are quite large.

You would use the as_str copy of the JSON string and pass it to the calc library or gmp library for conversion.

Good call on calc. Might as well keep it in the family, so to speak.

xexyl commented 2 years ago

Question for you @lcn2: I was going through the parser/scanner source files and changing the references of yy to ugly_. Now this is fine but there are some cases where the name actually is prefixed as ugly__ because the original symbol name was prefixed yy_. This is purely a cosmetic opinion I'm after: Do I change the prefix to ugly without the underscore or do in those cases I just use ugly__? The reason I added the underscore initially is to help with spellcheck but to me ugly__scan_string is itself uglier than ugly_scan_string for example. I'm not sure. What do you think? Of course maybe we're taking this too far (by which I mean 'maybe I am taking this too far' :) ) but it's giving me some joy to think about as well so it'll do. :)

Is not the problem that sometimes they are inconsistent in their _ use? Nevertheless searching for ugly_ would catch those cases where ugly__ was generated. Someone who knew about bison/flex or yacc/lex output would expect yy_ and seeing ugly_ would adapt. However if they saw just uglyfoo they would not know/might suspect if that it was one of your variables or one from bison/flex.

If you are going to change the prefix, you might be best to use ugly_ and accept the cases where bison/flex generates a __.

Good points. I'll keep it ugly_ then. There are enough comments on the fact it is changed but perhaps I should add another comment to make it very clear that instead of YY or yy for many symbols it's UGLY_ or ugly_.

Of course if I make this a normal parser for others I might want to undo this satire but I'll worry about that later.

The commit I made earlier and said that it would likely be the last one almost certainly is as I'm going to do some other things now. Good day my friend!

lcn2 commented 2 years ago

In regards to comment about jstrdecode:

$ ./jstrdecode -v 11 < /dev/null
debug[1]: about to decode all data on stdin
debug[9]: dyn_array_create(1, 65536, 8192, true): initialized empty dynamic array, allocated: 65536 elements of size: 1
debug[11]: dyn_array_seek(array, 65536, SEEK_CUR): in-place: allocated: 65536 elements of size: 1 in use: 65536
debug[9]: read_all: dyn_array_seek cycle: 1 new size: 65536
debug[9]: read_all: about to start read cycle: 0
debug[9]: read_all: fread(read_buf, 1, 65536, stream) read cycle: 0 returned: 0
debug[11]: dyn_array_clear(array) not-moved: allocated: 65536 elements of size: 1 in use: 0
debug[11]: dyn_array_seek(array, 0, SEEK_SET): in-place: allocated: 65536 elements of size: 1 in use: 0
debug[5]: fread returned: 0 normal EOF reading stream at: 0 bytes
debug[9]: read_all(stream, psize): last_read: 0 total bytes: 0 allocated: 65536 read_cycle: 1 move_cycle: 0 seek_cycle: 2
debug[3]: stdin read length: 0
debug[11]: is_string: is a C-style string of length: 1
Warning: json_decode: len: 0 must be > 0
Warning: jstrdecode_stdin: error while encoding stdin buffer

which is expected.