@@ -790,8 +790,12 @@ inline bool CFrontendIR::valid(const TypedHandle<const CLayer> rootHandle, syste
790790 TypedHandle<const IExprNode> handle;
791791 uint8_t contribSlot;
792792 SubtreeContributorState contribState = SubtreeContributorState::Required;
793+ // using post-order like stack but with a pre-order DFS
794+ uint8_t visited = false ;
793795 };
794796 core::stack<StackEntry> exprStack;
797+ // why a separate stack to the main one? Because we don't push siblings.
798+ core::vector<TypedHandle<const IExprNode>> ancestorPrefix;
795799 // unused yet
796800 core::unordered_set<TypedHandle<const INode>,HandleHash> visitedNodes;
797801 // should probably size it better, if I knew total node count allocated or live
@@ -818,7 +822,19 @@ inline bool CFrontendIR::valid(const TypedHandle<const CLayer> rootHandle, syste
818822 while (!exprStack.empty ())
819823 {
820824 const StackEntry entry = exprStack.top ();
821- exprStack.pop ();
825+ if (entry.visited )
826+ {
827+ exprStack.pop ();
828+ // this is the whole reason why we're using a post-order like stack
829+ ancestorPrefix.pop_back ();
830+ continue ;
831+ }
832+ else
833+ {
834+ exprStack.top ().visited = true ;
835+ // push self into prefix so children can check against it
836+ ancestorPrefix.push_back (entry.handle );
837+ }
822838 const auto * node = entry.node ;
823839 const auto nodeType = node->getType ();
824840 const bool nodeIsMul = nodeType==IExprNode::Type::Mul;
@@ -855,6 +871,13 @@ inline bool CFrontendIR::valid(const TypedHandle<const CLayer> rootHandle, syste
855871 logger.log (" Expression too complex, more than %d contributors encountered" ,ELL_ERROR,MaxContributors);
856872 return false ;
857873 }
874+ // detect cycles
875+ const auto found = std::find (ancestorPrefix.begin (),ancestorPrefix.end (),childHandle);
876+ if (found!=ancestorPrefix.end ())
877+ {
878+ logger.log (" Expression contains a cycle involving node %d of type %s" ,ELL_ERROR,childHandle,getTypeName (childHandle).data ());
879+ return false ;
880+ }
858881 // cannot optimize with `unordered_set visitedNodes` because we need to check contributor slots, if we really wanted to we could do it with an
859882 // `unordered_map` telling us the contributor slot range remapping (and presence of contributor) but right now it would be premature optimization.
860883 exprStack.push (newEntry);
0 commit comments