From e7581ad91597b9b6306597734783c8cdfa76a4c8 Mon Sep 17 00:00:00 2001
From: Alastair Robertson <ajor@meta.com>
Date: Tue, 24 Oct 2023 09:49:46 -0700
Subject: [PATCH] TopoSorter: Only allow certain params to be incomplete

For the containers which are allowed to be declared with incomplete
types, it is only the contained types which are allowed to be
incomplete. Other template parameters (e.g. allocators) must always be
defined before use.
---
 oi/type_graph/TopoSorter.cpp | 16 +++++++---------
 test/test_topo_sorter.cpp    | 10 ++++++++--
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/oi/type_graph/TopoSorter.cpp b/oi/type_graph/TopoSorter.cpp
index 5da2e839..6ca10af7 100644
--- a/oi/type_graph/TopoSorter.cpp
+++ b/oi/type_graph/TopoSorter.cpp
@@ -85,7 +85,7 @@ namespace {
  * Other containers are not required to do this, but might still have this
  * behaviour.
  */
-bool containerAllowsIncompleteParams(const Container& c) {
+bool containerAllowsIncompleteParam(const Container& c, size_t i) {
   switch (c.containerInfo_.ctype) {
     case SEQ_TYPE:
     case LIST_TYPE:
@@ -93,7 +93,7 @@ bool containerAllowsIncompleteParams(const Container& c) {
     case SHRD_PTR_TYPE:
       // Also std::forward_list, if we ever support that
       // Would be good to have this as an option in the TOML files
-      return true;
+      return i == 0;
     default:
       return false;
   }
@@ -101,17 +101,15 @@ bool containerAllowsIncompleteParams(const Container& c) {
 }  // namespace
 
 void TopoSorter::visit(Container& c) {
-  if (!containerAllowsIncompleteParams(c)) {
-    for (const auto& param : c.templateParams) {
+  for (size_t i = 0; i < c.templateParams.size(); i++) {
+    const auto& param = c.templateParams[i];
+    if (containerAllowsIncompleteParam(c, i)) {
+      acceptAfter(param.type());
+    } else {
       accept(param.type());
     }
   }
   sortedTypes_.push_back(c);
-  if (containerAllowsIncompleteParams(c)) {
-    for (const auto& param : c.templateParams) {
-      acceptAfter(param.type());
-    }
-  }
 }
 
 void TopoSorter::visit(Enum& e) {
diff --git a/test/test_topo_sorter.cpp b/test/test_topo_sorter.cpp
index c4b1b5fb..969c6aa9 100644
--- a/test/test_topo_sorter.cpp
+++ b/test/test_topo_sorter.cpp
@@ -178,24 +178,30 @@ std::map
 }
 
 TEST(TopoSorterTest, ContainersVector) {
-  // std::vector allows forward declared template parameters
+  // std::vector allows a forward declared type
   auto myparam = Class{1, Class::Kind::Struct, "MyParam", 13};
+  auto myalloc = Class{1, Class::Kind::Struct, "MyAlloc", 0};
   auto mycontainer = getVector();
   mycontainer.templateParams.push_back((myparam));
+  mycontainer.templateParams.push_back((myalloc));
 
   test({mycontainer}, R"(
+MyAlloc
 std::vector
 MyParam
 )");
 }
 
 TEST(TopoSorterTest, ContainersList) {
-  // std::list allows forward declared template parameters
+  // std::list allows a forward declared type
   auto myparam = Class{1, Class::Kind::Struct, "MyParam", 13};
+  auto myalloc = Class{1, Class::Kind::Struct, "MyAlloc", 0};
   auto mycontainer = getList();
   mycontainer.templateParams.push_back((myparam));
+  mycontainer.templateParams.push_back((myalloc));
 
   test({mycontainer}, R"(
+MyAlloc
 std::list
 MyParam
 )");