Guarantee that a static IntegerExp is never changed

[ghost]

This follows up #9500, as suggested by @JinShil.

A bool flag is set when creating a static IntegerExp.
It's used to make sure that neither the value nor the type are modified.
Any attempt will lead to a ICE.

Note a small drawback : before adding the flag the class instance size was exactly of 48 bytes.

[dlang-bot]

Thanks for your pull request and interest in making D better, @Basile-z! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#9506"

[WalterBright]

Isn't this doing what const is supposed to do?

[JinShil]

Isn't this doing what const is supposed to do?

I believe so, but how does one properly express that with an IntegerExp?

[ghost]

Please, note that I've dropped the bool flag.

• The way normalize() is changed allows setInteger() to be private.
• I didn't like that more memory might be allocated (48 to 49 bytes meant 56 bytes actually allocated).
• public field Expression.type from the base class was not protected anyway.
• normalize() will never change the type Tint32 set when the static instances are created. Even the ctor that just takes a value doesn't bother calling it (while the other ctor does).

I'm happy with the PR now so if you agree too with the change I'll just squash.

[WalterBright]

how does one properly express that with an IntegerExp?

const dinteger_t value;

[ghost]

@thewilsonator and others, squashed and forced pushed, merge to your appreciation but don't forget that the stuff is already there, this PR just improves it, since #9500 was merged before I address the review comment from @JinShil. 😉

[WalterBright]

This PR does not remove technical debt, it adds it. normalize() was a function that could be pure. It is refactored into a function that is not pure. I strongly object to this PR. Not only does it not do what the title says it does, I don't see a purpose to these changes, and none is offered.

[WalterBright]

I see you did offer a rationale, making setInteger() private. I don't see a gain, though, as normalize() gets exposed instead. I don't see any improvement.

[WalterBright]

Closed because turning a pure function into an impure one is not where we should be going. Note that normalize() still can't be marked pure because target.ptrsize is a mutable global, even though it doesn't change after initialization. But at least the rest of it is pure.

[JinShil]

@WalterBright, please be aware that there is an issue with #9500. That PR introduced static singleton instances of IntegerExp in an effort to (I think) reduce the memory footprint of DMD. Those singleton instances are expected to have their value remain constant for their lifetime. However, because setInteger is available, it invites the possibility of introducing a difficult to trace bug.

You might argue that #9500 should not have been merged. @Basile-z even admitted that it was merged before he had time to address this issue. This PR was an attempt to address the issue. If this PR is not suitable, please suggest an alternative.

[kinke]

I'm also surprised this got dismissed that quickly (although I would have killed setInteger() altogether). What's a theoretically pure function worth if it's only used by the class itself (IIRC, just in the constructor and then in toInteger()), to normalize its (basically constant) value to the apparently mutable type (see comment in toInteger()), except for bloating the signature/call sites?

[ghost]

in an effort to (I think) reduce the memory footprint of DMD

And compilation time