pawn-lang / compiler

Pawn compiler for SA-MP with bug fixes and new features - runs on Windows, Linux, macOS
Other
301 stars 71 forks source link

Fix explicit zero array size being treated as no size #683

Closed Daniel-Cortez closed 2 years ago

Daniel-Cortez commented 2 years ago

What this PR does / why we need it:

Fixes a bug with the compiler mistreating new array[0] = { ... }; as new array[] = { ... }; (see #668).

Which issue(s) this PR fixes:

Fixes #668

What kind of pull this is:

Additional Documentation:

YashasSamaga commented 2 years ago

Please add negative tests like:

new arr[] = { 0 };

This is just to future-proof. It is currently not required since needsub will return early if it finds ] without any constant expression in it and prevent the error message.

I think the following will trigger two errors which is ok:

new a;
new arr[a] = { 0 };

constexpr will return zero in val if the expression is not constexpr. This will trigger two errors: error 20 and error 8.

Daniel-Cortez commented 2 years ago

Please add negative tests like:

new arr[] = { 0 };

This is just to future-proof.

Wouldn't this be excessive for this PR?

Don't get me wrong, I do agree that future-proofing is a good practice, and I can add such tests if you insist. But I also think that ideally we should make a single file for each group of tests (f.e. one file for testing the array syntax, another file for testing code generation for arrays, a third one for testing function declaration syntax, and so on), instead of having them scattered around over a hundred of files for each tiny bugfix we make.

Rewriting the tests might require a lot of work, and not only for the rewriting itself, but also for fixing newly discovered bugs that would very likely pop up in the process. Obviously, this is very far out of the scope of this PR, and I think all of this would only be justified when and if open.mp finally comes out, otherwise there would be no point doing this when there aren't many people left in the community and the userbase keeps decreasing with each day.

Also, we still have a few unsolved problems related to array initialization, such as #439, #547 and #606, so I think it's still too early for adding any future-proofing tests for arrays.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity.

Y-Less commented 2 years ago

Merged locally.