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

Use FINAL declarations for immutable variables #299

Closed
wants to merge 4 commits into from

Conversation

ConjuringCoffee
Copy link
Contributor

Fixes #285.

I've added the new rule and changed all examples in which it was obvious that a variable is never reassigned.

@cribuc cribuc added the New Rule The issue or PR proposes an new rule or set of rules label Feb 20, 2023
@KlausHaeuptleSAP KlausHaeuptleSAP added the Ready for Decision The pull request is ready for decision label Apr 4, 2023
Copy link
Member

@N2oB6n-SAP N2oB6n-SAP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have suggested a change to address the subtlety of FINAL that it does not prevent changes in general. It merely prevents changes in unexpected places.

clean-abap/CleanABAP.md Outdated Show resolved Hide resolved
Co-authored-by: N2oB6n-SAP <109298321+N2oB6n-SAP@users.noreply.github.com>
@ConjuringCoffee ConjuringCoffee marked this pull request as draft April 18, 2023 06:56
@ConjuringCoffee
Copy link
Contributor Author

Now here's a problem: FINAL doesn't just prevent changes in unexpected locations in the source code, it also affects the debugger. You can't change the variable in the debugger anymore.

I find this to be a major downside of the new keyword. Does anyone know if there's a workaround or a setting for the debugger? If not, then I'm personally quite conflicted about the new rule. I've converted the pull request to a draft until further clarification took place.

@ChristophStoeckSAP
Copy link

One comment form the ABAP Debugger perspective:
There is no way to change FINAL variables in the ABAP debugger. This is the same as with constants.

@bjoern-jueliger-sap
Copy link
Member

The issue with final is a bit more subtle than "use it if a variable is never reassigned", and the fact that final variables cannot be changed in the debugger (and that this is surprising to at least some users of the feature) hints at the underlying mismatch in concepts of immutability:

While it may be tempting to think about final as a hint to the compiler to emit syntax errors when you try to change that variable again, final variables in fact have different runtime properties than normal variables: The notion of immutability is a runtime property in ABAP, not a compile-time property!

The following code is syntax-error free, but produces a short dump at runtime:

final(var) = 1.
assign var to field-symbol(<var>).
<var> = 11.

Changing the data declaration to use data instead of final avoids the short dump.

This is probably not what you expect if you are used to thinking about immutability as a compile-time property of bindings (like const variables in Javascript or non-mut variables in Rust), even though the documentation of final says it: "In all other positions, any write access leads either to a syntax error or the uncatchable exception MOVE_TO_LIT_NOTALLOWED_NODATA." It's important to realize that "write access" refers to the data object created by the final declaration, not to write access to the variable name (the binding).

In the example above it is easy to understand why the dump happens once you realize final sets a run-time property on the data object that underlies var, but if the dump involves a call stack and references, this can get difficult to analyze. For instance, it is not a syntax error to use a reference to a final variable as the actual parameter of another method:

final(var) = 1.
not_my_method( ref #( var ) ).

If not_my_method then at any point attempts to change the content of the reference, we get a short dump - even though var is never "reassigned" in the part of the program that is under our control.

So while I think a recommendation in the spirit of "Use final whenever appropriate" should be made, I think any such recommendation needs to have some details about what "appropriate" means: Especially when references are involved, the programmer needs to think carefully about whether or not the write-protection that final generates at runtime is intended or not.

@ConjuringCoffee
Copy link
Contributor Author

Great comment. I am closing this pull request and pointed the issue to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Rule The issue or PR proposes an new rule or set of rules Ready for Decision The pull request is ready for decision
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FINAL new ABAP keyword - guidance
9 participants