programmemory.cpp: avoid calls to non-const at() / added utils::as_const()

[firewave]

No description provided.

[firewave]

-D__GNUC__ --check-level=exhaustive ../lib/utils.cpp

Clang 17 773,369,412 -> 756,793,758
GCC 14 794,493,015 -> 778,810,008

The example from https://trac.cppcheck.net/ticket/10765#comment:4:

Clang 17 3,988,168,871 -> 3,958,401,0425
GCC 14 4,098,327,498 -> 4,074,480,389

[firewave]

There is still one case I have to look at.

[firewave]

And we really need an as_const() helper.

[firewave]

There is still one case I have to look at.

I will leave that out for now because the impact is currently quite neglectable.

[chrchr-github]

Why would this have a performance impact?

[firewave]

Why would this have a performance impact?

As we are working in a mutable context it will chose the non-const implementation. And if we return something mutable we need to do copy-on-write first so the (potential) changes are only applied to the copy.

[chrchr-github]

Why would this have a performance impact?

As we are working in a mutable context it will chose the non-const implementation. And if we return something mutable we need to do copy-on-write first so the (potential) changes are only applied to the copy.

I see. I somehow confused this with mValues->at().

[firewave]

I see. I somehow confused this with mValues->at().

Yeah, it should have probably been called getValue() to be in line with hasValue().

[chrchr-github]

And we really need an as_const() helper.

It would make the intent more clear and maybe prevent someone from "cleaning up" in the future.
The implementation seems quite simple: https://en.cppreference.com/w/cpp/utility/as_const

[firewave]

And we really need an as_const() helper.

It would make the intent more clear and maybe prevent someone from "cleaning up" in the future. The implementation seems quite simple: https://en.cppreference.com/w/cpp/utility/as_const

I ran into issue with that and need to check how the actual implementation behaves so it will be drop-in in the future.

Also it would be great to have tooling which tells you where to use it because checking the code manually is quite tedious even with IDE tooltips.

[pfultz2]

Can we just remove the non-const overload for at? I dont think its ever used.

[firewave]

It is used in three places in Executor::executeImpl().

[pfultz2]

Maybe we can rename the mutable version as mutate_at to make it explicit when we want to mutate.

[firewave]

I think that is unnecessary. I will add a barebones as_const() to clean up the code.

[firewave]

That's why we need to dereference the pointers in the actual code.

I filed tickets about detecting this: llvm/llvm-project#101580 / https://trac.cppcheck.net/ticket/12977.