Add code_typet::get_this

[smowton]

This simple utility saves a double-lookup compared to has_this followed by parameters()[0].

More synchronisation of simple utilities present in the sec codebase but not in cbmc.

[tautschnig]

Please don't return a pointer. I'd suggest to a nil_exprt or an optional<parametert> instead.

[smowton]

We'd want a reference, so it would have to be an optionalt<std::reference_wrapper<parametert>>, which is considerably messier to work with but has essentially the same meaning as a pointer. There was considerable back-and-forth over this same question in the recent symbol table interface improvements, and the outcome was to allow a non-owning pointer like this with nullptr for not found, e.g. https://github.com/diffblue/cbmc/blob/develop/src/util/symbol_table.h#L96. Therefore I'm inclined to maintain the same pattern here.

[tautschnig]

Why does it have to be a reference? Can't you just return a parametert by value here (a key difference to the get_writable case)?

[smowton]

Sure? Just seemed wasteful to copy it when that's probably unnecessary.

[tautschnig]

I'm not sure this is on a hot path? Copying an irept is pretty cheap, after all.

[smowton]

This particular battle was last fought here: #1462. IIRC the actual conversation was had with Daniel in person, and that PR adds the result to the coding standard doc.

Every time someone has to return some "not found / undefined" value there is a tussle between irept(), optionalt<...>::none and nullptr, with as far as I can see nothing to recommend any option over any other except consistency, hence my copying the style eventually settled on in symbol_tablet.

If you continue to object I'll just do whatever you say to get this over with, but I'd like to decide once and for all rather than repeat this conversation every time.

[tautschnig]

Admittedly, I completely missed #1462, and in particular its addition to the coding standard (and of course the offline discussion). I have thus removed my block.

Here are my thoughts on the matter:

1. Avoid pointers wherever possible, as their unchecked use may result in undefined behaviour. With null pointers there will likely be a reliable crash with little exploitability, so the situation isn't terrible, but it's not as great as with references.
2. If you are in the situation that a value might not be found, then distinguish the following two cases:
   a) Is a missing value an exceptional case? Use some form of INVARIANT (and a reference or some other supposedly-initialised type) or optionalt. Notably, the user of such a method should not be expected to sanity-check the return value, but instead should be able to use it directly.
   b) A missing value is an perfectly normal case. Use a pointer or a nil_exprt (the latter being the more common solution across the code base, I believe -- but really this should be unified), or maybe optionalt. Make clear in comments that the user needs to sanity-check the return value, though hopefully the user is well aware of that anyway.

Now I'm not sure whether this actually adds clarity? Anyway it's just my thoughts, no claim of this being consensus.

[smowton]

Re: 2, this is definitely an expected failure case (e.g. the function is a free, non-member function)

Pointers added to coding standard in #1462.

[smowton]

@tautschnig are you happy for this to go in?

[tautschnig]

Should be a const reference (given we discussed the cost of copy in this PR...).