[Support/YAMLTraits] - Refactor of Input::createHNodes

[grimar]

Summary

I tried to fix crash issue relative to this method and found it is hard to read.
It contained duplicated piece of code which is now moved to helper function and shared.
When error happened it returned half-broken instance of node, and since
createHNodes itself a reqursive method, it had to check EC in caller,
when I believe it is enough just to return nullptr on any error,
just like it already do for unknown node.
It is also was possible to use early returns, patch did that.

lib/Support/YAMLTraits.cpp

void Input::setError(HNode *hnode, const Twine &message) {
   this->setError(hnode->_node, message);
 }
 
 void Input::setError(Node *node, const Twine &message) {
   Strm->printError(node, message);
   EC = make_error_code(errc::invalid_argument);
 }
 
-std::unique_ptr<Input::HNode> Input::createHNodes(Node *N) {
+static StringRef getScalarNodeValue(ScalarNode *S, BumpPtrAllocator &Alloc) {
   SmallString<128> StringStorage;
-  if (ScalarNode *SN = dyn_cast<ScalarNode>(N)) {
-    StringRef KeyStr = SN->getValue(StringStorage);
-    if (!StringStorage.empty()) {
-      // Copy string to permanent storage
-      KeyStr = StringStorage.str().copy(StringAllocator);
-    }
-    return llvm::make_unique<ScalarHNode>(N, KeyStr);
-  } else if (BlockScalarNode *BSN = dyn_cast<BlockScalarNode>(N)) {
-    StringRef ValueCopy = BSN->getValue().copy(StringAllocator);
-    return llvm::make_unique<ScalarHNode>(N, ValueCopy);
-  } else if (SequenceNode *SQ = dyn_cast<SequenceNode>(N)) {
+  StringRef KeyStr = S->getValue(StringStorage);
+  if (!StringStorage.empty())
+    return StringStorage.str().copy(Alloc);
+  return KeyStr;
+
+std::unique_ptr<Input::HNode> Input::createHNodes(Node *N) {
+  if (ScalarNode *SN = dyn_cast<ScalarNode>(N))
+    return llvm::make_unique<ScalarHNode>(
+        SN, getScalarNodeValue(SN, StringAllocator));
+
+  if (BlockScalarNode *BSN = dyn_cast<BlockScalarNode>(N))
+    return llvm::make_unique<ScalarHNode>(
+        N, BSN->getValue().copy(StringAllocator));
+
+  if (SequenceNode *SQ = dyn_cast<SequenceNode>(N)) {
     auto SQHNode = llvm::make_unique<SequenceHNode>(N);
     for (Node &SN : *SQ) {
       auto Entry = this->createHNodes(&SN);
-      if (EC)
-        break;
+      if (!Entry)
+        return nullptr;
       SQHNode->Entries.push_back(std::move(Entry));
     }
     return std::move(SQHNode);
-  } else if (MappingNode *Map = dyn_cast<MappingNode>(N)) {
-    auto mapHNode = llvm::make_unique<MapHNode>(N);
+  }
+
+  if (MappingNode *Map = dyn_cast<MappingNode>(N)) {
+    auto MapNode = llvm::make_unique<MapHNode>(N);
     for (KeyValueNode &KVN : *Map) {
-      Node *KeyNode = KVN.getKey();
-      ScalarNode *KeyScalar = dyn_cast<ScalarNode>(KeyNode);
+      ScalarNode *KeyScalar = dyn_cast<ScalarNode>(KVN.getKey());
       if (!KeyScalar) {
-        setError(KeyNode, "Map key must be a scalar");
-        break;
-      }
-      StringStorage.clear();
-      StringRef KeyStr = KeyScalar->getValue(StringStorage);
-      if (!StringStorage.empty()) {
-        // Copy string to permanent storage
-        KeyStr = StringStorage.str().copy(StringAllocator);
+        setError(KVN.getKey(), "Map key must be a scalar");
+        return nullptr;
       }
       auto ValueHNode = this->createHNodes(KVN.getValue());
-      if (EC)
-        break;
-      mapHNode->Mapping[KeyStr] = std::move(ValueHNode);
+      if (!ValueHNode)
+        return nullptr;
+      StringRef Key = getScalarNodeValue(KeyScalar, StringAllocator);
+      MapNode->Mapping[Key] = std::move(ValueHNode);
     }
-    return std::move(mapHNode);
-  } else if (isa<NullNode>(N)) {
+    return std::move(MapNode);
+  }
+
+  if (isa<NullNode>(N))
     return llvm::make_unique<EmptyHNode>(N);
-  } else {
-    setError(N, "unknown node kind");
-    return nullptr;
-  }
-}
+
 
 void Input::setError(const Twine &Message) {
   this->setError(CurrentNode, Message);
 }
 
 bool Input::canElideEmptySequence() {
   return false;
 }