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

Bug for NPP015 (REDEFINE length) partially fixed #274

Conversation

Claes65
Copy link
Contributor

@Claes65 Claes65 commented Jun 2, 2023

No description provided.

@Claes65 Claes65 marked this pull request as ready for review June 2, 2023 07:07
@MarkusAmshove
Copy link
Owner

In the disabled test, which variable/line do you expect a diagnostic to be raised and why? (e.g. 900 bytes > 800 bytes)
I'm a bit lost on such complex redefines :D

@Claes65
Copy link
Contributor Author

Claes65 commented Jun 3, 2023

2 #bytes2 (A1/1:850)
2 redefine #bytes2
3 #bytes-str (A750)
3 #r1 (a1/1:450)

Bytes2 is 850, but the redefinition fields are 750+450

@MarkusAmshove
Copy link
Owner

It seems to not check the redefine at all, because the top level #GRP is checked.
This is a tough change. Maybe it would be easier to pull out the length check from the addTargetToRedefine (it is never called for redefine #bytes2?) and do a pass over all variable at the end of parseInternal and check if a variable is a redefine and if yes, then check the sizes.

You can use defineData.variables() to do so, no need to manually go through all levels. That method already does return all variables (including tested).

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 7, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

92.0% 92.0% Coverage
0.0% 0.0% Duplication

@Claes65
Copy link
Contributor Author

Claes65 commented Jun 7, 2023

It seems to not check the redefine at all, because the top level #GRP is checked. This is a tough change. Maybe it would be easier to pull out the length check from the addTargetToRedefine (it is never called for redefine #bytes2?) and do a pass over all variable at the end of parseInternal and check if a variable is a redefine and if yes, then check the sizes.

You can use defineData.variables() to do so, no need to manually go through all levels. That method already does return all variables (including tested).

I think for now, we can merge this in, and then look to fix the more fundamental REDEFINE thingy at a later point? I can create an issue for it, if you want me to.

@MarkusAmshove MarkusAmshove added the bug🪲 Something isn't working label Jun 7, 2023
@MarkusAmshove MarkusAmshove merged commit 3a9a78f into MarkusAmshove:main Jun 7, 2023
@MarkusAmshove MarkusAmshove added this to the v0.7 milestone Jun 14, 2023
@Claes65 Claes65 deleted the bug/calc-of-redefinition-fields-with-arrays branch October 2, 2023 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug🪲 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants