Fixes use of wrong identifier when pretty printing bitfield types

[hannes-steffenhagen-diffblue]

The width of a bitfield is indicated with ID_width. Old version of type2name was using ID_size instead, which (silently) threw away the information about the width of a bitfield in the pretty printed type.

As a side note, I'm not sure it's such a good idea for the get* type functions to fail silently.

[martin-cs]

Fix looks good; any chance of a regression test to go with it? Preferably the one / a version of the one submitted in the original ticket.

[tautschnig]

I might be responsible for a some of this code. May I ask for a slightly bigger refactoring to use pointer_offset_bits instead? All those get_string calls should be dropped from that code; the width of ID_pointer should now also be considered.

[hannes-steffenhagen-diffblue]

@tautschnig I'm not sure how you'd want to use pointer_offset_bits here. Also, what would you want to replace the get_string calls with?

[tautschnig]

@tautschnig I'm not sure how you'd want to use pointer_offset_bits here. Also, what would you want to replace the get_string calls with?

pointer_offset_bits will compute the bit-width of the type, and could/should be used instead of get_string(ID_width); the resulting mp_integer then needs to be turned into a string.

[hannes-steffenhagen-diffblue]

@martin-cs I didn't find a test category that really fit for the test I was doing (though if there is one I'll happy to change this), so I made my own

@tautschnig I'm not entirely sure if I understood you correctly, is this what you had in mind?

[tautschnig]

Thank you for all this additional work! Some more fixes are necessary, though.

[tautschnig]

I'd just add it to the goto-instrument directory.

[hannes-steffenhagen-diffblue]

done

[tautschnig]

Is there a strong reason not to use a function at file scope?

[hannes-steffenhagen-diffblue]

Not really. I can do that if you prefer; I just put it in the function scope because I wasn't expecting to use it outside of the function and I don't need to pass parameters this way.

[tautschnig]

My reasons for preferring the file-scoped function: 1) Consistency: we don't do it elsewhere. 2) I'm not sure how well doxygen can cope with it. 3) The function calls do not make clear what is captured, thus making the code harder to read.

None of this is a blocker, but readability is a very important aspect.

[tautschnig]

No, that would yield "symbol". Use id2string(t.size().get(ID_identifier))

[hannes-steffenhagen-diffblue]

done

[tautschnig]

nil would actually be a valid name! component_name.empty() would be the error you are looking for.

[hannes-steffenhagen-diffblue]

done

[tautschnig]

Nit pick: no new line at end-of-file

[hannes-steffenhagen-diffblue]

(reminder: both AppVeyor and travis report failures for known issues that are unrelated to this PR)

[chrisr-diffblue]

Looks good to me, but could you just add the missing new-line, then I'll merge.