New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(3.4) dnn: fix API - explicit ctors, const methods #21429
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for contribution!
LayerData& getLayerData(int id) const | ||
{ | ||
MapIdToLayerData::iterator it = layers.find(id); | ||
MapIdToLayerData::const_iterator it = layers.find(id); | ||
|
||
if (it == layers.end()) | ||
CV_Error(Error::StsObjectNotFound, format("Layer with requested id=%d not found", id)); | ||
|
||
return it->second; | ||
return const_cast<LayerData&>(it->second); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are returning non-const reference from const method. Why do we need that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change requires too much changes due to current.
Unfortunately LayerData
is not not preserved unmodified. It is part of "graph" model and manages graph connections too.
modules/dnn/src/dnn.cpp
Outdated
{ | ||
int lid = (layerName.empty()) ? 0 : getLayerId(layerName); | ||
|
||
std::vector<LayerPin> pins; | ||
|
||
for (int i = 0; i < layers[lid].outputBlobs.size(); i++) | ||
for (int i = 0; i < const_cast<MapIdToLayerData&>(layers)[lid].outputBlobs.size(); i++) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, we either should check that lid exists in layers, or mark layers as mutable, or don't mark this method as const. I think, the use of const_cast is unjustified here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
modules/dnn/src/dnn.cpp
Outdated
const LayerData &ld = const_cast<MapIdToLayerData&>(layers)[pin.lid]; | ||
if ((size_t)pin.oid >= ld.outputBlobs.size()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we create [pin.lid]
in layers in previous line, its outputBlobs.size()
is zero and the condition is true, so we can get rid of const_cast.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
👍 |
Validate: