CEED
Get* Interface Uniformity
Ok, we're pretty inconsistent about when a Get interface needs a Restore/Destroy. Here's the list of ones that don't.
I think an easy point of consistency would be to require a Destroy when the Get returns a Ceed* object.
ceed.h
| Function | Scalar Quantity | Has Restore |
|---|---|---|
CeedBasisGetQRef |
No | No |
CeedBasisGetQWeights |
No | No |
CeedBasisGetInterp |
No | No |
CeedBasisGetInterp1D |
No | No |
CeedBasisGetGrad |
No | No |
CeedBasisGetGrad1D |
No | No |
CeedBasisGetDiv |
No | No |
CeedBasisGetCurl |
No | No |
CeedQFunctionGetFields |
No | No |
CeedQFunctionContextGetAllFieldLabels |
No | No |
CeedContextFieldLabelGetDescription |
No | No |
CeedOperatorGetFields |
No | No |
CeedOperatorGetFieldByName |
No | No |
CeedCompositeOperatorGetSubList |
No | No |
CeedCompositeOperatorGetSubByName |
No | No |
CeedOperatorFieldGetName |
No | No |
backend.h
| Function | Scalar Quantity | Has Restore |
|---|---|---|
CeedBasisGetCollocatedGrad |
No | No |
CeedQFunctionContextGetFieldLabel |
No | No |
CeedOperatorGetQFunctionAssemblyData |
No | No |
CeedQFunctionAssemblyDataGetObjects |
No | No |
CeedOperatorAssemblyDataGet* |
No | No |
CeedOperatorGetFallback |
No | No |
CeedOperatorGetFallbackParent |
No | No |
String getters I think are fine (I think they are unlikely to be kept by the user for a long time, and they are constantly, so the user has to mean to bypass that to edit them)
| Function | Scalar Quantity | Has Restore |
|---|---|---|
CeedGetResource |
No | No |
CeedGetErrorMessage |
No | No |
CeedGetResourceRoot |
No | No |
CeedGetOperatorFallbackResource |
No | No |
CeedQFunctionGetKernelName |
No | No |
CeedQFunctionGetSourcePath |
No | No |
CeedQFunctionFieldGetName |
No | No |
CeedQFunctionFieldGetData |
No | No |
Others I think are fine (Backend use only, intended to be edited, and should not have multiple access at the same time)
| Function | Scalar Quantity | Has Restore |
|---|---|---|
Ceed*GetData |
No | No |
@jrwrigh continuing some of our discussions otherwhere.
I think some of these make sense to leave without a Restore, like when we get a const char *? (CeedGetResource for example)
Yeah, if we can reliably enforce a const to be used by the user, then it doesn't need a restore.
Which I guess we could've done with CeedGetJit* by using const * const char *? Or something like that, I'm pretty sure there's a semantic to declare const for nested pointers.
Note that returning const pointers does not prevent use after free. I don't think we have a good rule for this in PETSc either. It's vaguely "if the caller might be tempted to hang onto the reference but doing so could produce confusing behavior, then a Restore is needed". So returning an immutable string that would only be compared is okay, as is KSPGetPC. I don't think anyone is happy with this vagueness, but it's kind of a hassle to program with explicit restores everywhere.
Yeah, I don't think we want restore for all of those. I'm less worried about the strings because those are typically used for display. I'm also less worried about the ceed/backend.h functions.
I think on the whole taking an approach of refcounting any Ceed, CeedVector, ect from these interfaces and the requiring Ceed*Destroy would make things easier for us and would be a good uniformity step, as it imposes the rule that any Ceed object in scope should be destroyed. (Also would help simplify any odd edges in FFI, especially Rust)