I would not start storing things twice like this, doesn't seem like that simplifies the code or would help performance much.

This type is unused, can be removed I think.

Can we change the Sculpt prefix to PBVH?

Otherwise is inconsistent with BKE_pbvh_make_vref functions, and I guess this may find a use in paint modes also.

I these function names should be more verbose, consistent and have some comment explaining things.

Something like:

PBVHVertRef BKE_pbvh_ref_from_mesh_vertex_index(const int v);
PBVHVertRef BKE_pbvh_ref_from_bmesh_vertex(BMVert *v);
PBVHVertRef BKE_pbvh_ref_from_pbvh_vertex_index(PBVH *pbvh, const int v);

int BKE_pbvh_ref_to_mesh_vertex_index(PBVHVertRef ref);
BMVert *BKE_pbvh_ref_to_bmesh_vertex(PBVHVertRef ref);
int BKE_pbvh_ref_to_pbvh_vertex_index(PBVH *pbvh, PBVHVertRef ref);

Can this be an inline function instead of a macro?

inex -> index

This function is not defined anywhere.

This function is not defined anywhere.

Rename index since it's no longer a PBVH vertex index.

I don't think there should be raw access to .i like this here and in many other places that follow. It can instead use functions like BKE_pbvh_ref_to_mesh_vertex_index(ref) and BKE_pbvh_ref_to_bmesh_vertex(ref).

This comment was not addressed, why store two types of vertex references here and in many other places?

Is it really performance critical in all of them, or could you in many cases just store one and compute the index when needed?

For example these, it doesn't seem obviously helpful for performance to store both.

In fact the indices are not read anywhere, only set.

Remove index from the name.

Remove index from the name.

Follow multiline comment style:

/* These structs represent logical verts/edges/faces.
 * for PBVH_GRIDS and PBVH_FACES they store integer
 * offsets, PBVH_BMESH stores pointers.
 *
 * The idea is to enforce stronger type checking by encapsulating
 * intptr_t's in structs. */

People are also going to have trouble understanding what all these different types of indices and references mean.

Expecting the caller to cast a pointer to intptr_t is not a good API. At a minimum this could be use function overloading.

PBVHVertRef BKE_pbvh_vert_ref(int index)
PBVHVertRef BKE_pbvh_vert_ref(BMVert *v)
PBVHVertRef BKE_pbvh_vert_ref_from_pbvh_index(PBVH *pbvh, const int v);

Or even make it a C++ class with methods:

PBVHVertRef::from_mesh(vi)
PBVHVertRef::from_bmesh(v)
ref.to_mesh()
ref.to_bmesh()
if (ref.empty()) ...

-1 -> PBVH_REF_NONE

-1 -> PBVH_REF_NONE

-1 -> PBVH_REF_NONE

It's still really confusing to me to casually cast between integers and pointers in many places.

It would help if this was a C++ class with meshes so you could to vertex.to_mesh(), vertex.to_bmesh(), or perhaps a union so it can be vertex.mesh_index, vertex.bmesh_vertex.

These two functions I guess only work for PBVH_FACES? Now that they take a generic PBVHVertRef you might expect otherwise.

Rename index to vertex here and in various other places..

This I guess is wrong, using index.i here should be cast to BMVert*? That's the kind of bug I want to prevent.

Should be BMVert* instead?

Remove index from the name.

Remove index from the name.