Summary
Several places in the codebase use std::optional<std::shared_ptr<T>> (or the Handle alias equivalent, e.g. std::optional<ColorMapHandle>). This is redundant — shared_ptr already has a "null" state (empty pointer) that can represent absence, so wrapping it in optional gives two different ways to express "no value" (std::nullopt vs. a null/empty shared_ptr). That's confusing for readers and callers, and invites bugs where one form is checked but not the other.
Known usages
Modules/Visualization/ShowField.cc — std::optional<ColorMapHandle> colorMap used as a parameter type in multiple function signatures (getNodeRenderState, getEdgeRenderState, getFaceRenderState, etc.), where ColorMapHandle is itself SharedPointer<ColorMap> (see Core/Datatypes/DatatypeFwd.h)
- Reportedly other places in the codebase as well — needs a full sweep
Tasks
Why this matters
Mixing both null-representations makes call sites harder to reason about ("is colorMap present but null? is that even valid?") and increases the chance of null-shared_ptr dereferences or missed nullopt checks.
Summary
Several places in the codebase use
std::optional<std::shared_ptr<T>>(or theHandlealias equivalent, e.g.std::optional<ColorMapHandle>). This is redundant —shared_ptralready has a "null" state (empty pointer) that can represent absence, so wrapping it inoptionalgives two different ways to express "no value" (std::nulloptvs. a null/emptyshared_ptr). That's confusing for readers and callers, and invites bugs where one form is checked but not the other.Known usages
Modules/Visualization/ShowField.cc—std::optional<ColorMapHandle> colorMapused as a parameter type in multiple function signatures (getNodeRenderState,getEdgeRenderState,getFaceRenderState, etc.), whereColorMapHandleis itselfSharedPointer<ColorMap>(seeCore/Datatypes/DatatypeFwd.h)Tasks
std::optional<combined withshared_ptr/Handle-typedef'd types to enumerate all usagesshared_ptr/Handle, or keepoptionaland enforce that the inner pointer is never nullShowField.ccand any other identified call sites to follow the conventionDatatypeFwd.h) so it doesn't reappearWhy this matters
Mixing both null-representations makes call sites harder to reason about ("is
colorMappresent but null? is that even valid?") and increases the chance of null-shared_ptr dereferences or missednulloptchecks.