Try to fix dangling reference flagged by CI #857
Conversation
|
Well even though the tests are still failing, they are failing at a later point so this seems like an improvement? |
|
No it doesn't fix it completely. I'm unsure why suddenly the compiler is being much more strict? If I get some spare time I'll look at it, but I think it's unlikely to happen in the next couple of weeks... |
| @@ -20,7 +20,7 @@ | |||
| #include "File.h" | |||
| #include "InputParserError.h" | |||
|
|
|||
| #include <regex> | |||
| #include <boost/regex.hpp> | |||
There was a problem hiding this comment.
Turns out I could reproduce the CI error on my end with the debug build.
It seems that std::regex is not very stable https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61582. I tried to bump our C++ -std flag but that creates more issues.
On the other hand using boost/regex seems to fix the issue on my end...
| @@ -353,15 +353,15 @@ int getRequiredIntAttribute( onnx::NodeProto &node, String name ) | |||
| return attr->i(); | |||
| } | |||
|
|
|||
| const onnx::TensorProto getTensorAttribute( onnx::NodeProto &node, String name ) | |||
| const onnx::AttributeProto *getTensorAttribute( onnx::NodeProto &node, String name ) | |||
There was a problem hiding this comment.
Turns out making copy of TensorProto can be inefficient and caused the CI to time out. I modified the code to be functionally the same as the master branch.
|
Nice finds. Seems to be passing so merge? |
Sounds good! |
The CI in various places is failing:
e.g. https://github.com/NeuralNetworkVerification/Marabou/actions/runs/12862710327/job/35858064473#step:12:122
because of a dangling reference.
AttributeProtoobject may be deassigned as it is a local, so therefore better simply to return a copy of theTensorProtoobject.