birkeh/o3de
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

compile time null checks avaiable for method node

Browse Source 
Signed-off-by: chcurran <82187351+carlitosan@users.noreply.github.com>
This commit is contained in:
chcurran
2021-07-08 15:08:55 -07:00
parent c287ae83e9
commit b63c9a87db
9 changed files with 1046 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Gems/ScriptCanvas/Code/Include/ScriptCanvas/Core/Node.cpp
+5
View File
@@ -2335,6 +2335,11 @@ namespace ScriptCanvas
}
}
bool Node::CanAcceptNullInput([[maybe_unused]] const Slot& executionSlot, [[maybe_unused]] const Slot& inputSlot) const
{
return true;
}
void Node::CollectVariableReferences(AZStd::unordered_set< ScriptCanvas::VariableId >& variableIds) const
{
for (const Slot& slot : m_slots)
Gems/ScriptCanvas/Code/Include/ScriptCanvas/Core/Node.h
+3
View File
@@ -479,7 +479,10 @@ namespace ScriptCanvas
Node(const Node&); // Needed just for DLL linkage. Does not perform a copy
Node& operator=(const Node&); // Needed just for DLL linkage. Does not perform a copy
virtual bool CanAcceptNullInput(const Slot& executionSlot, const Slot& inputSlot) const;
virtual void CollectVariableReferences(AZStd::unordered_set< ScriptCanvas::VariableId >& variableIds) const;
virtual bool ContainsReferencesToVariables(const AZStd::unordered_set< ScriptCanvas::VariableId >& variableIds) const;
Graph* GetGraph() const;
Gems/ScriptCanvas/Code/Include/ScriptCanvas/Grammar/AbstractCodeModel.cpp
+34 -22
View File
@@ -670,6 +670,28 @@ namespace ScriptCanvas
return AddVariable(Datum(type), rawName);
}
void AbstractCodeModel::CheckForKnownNullDereference(ExecutionTreeConstPtr execution, const ExecutionInput& input, const Slot& inputSlot)
{
if (Data::IsValueType(inputSlot.GetDataType())
|| !execution->GetId().m_node
|| !execution->GetId().m_slot
|| (input.m_value && !input.m_value->m_datum.Empty()))
{
return;
}
if (!input.m_value)
{
AddError(execution->GetId().m_node->GetEntityId(), nullptr, "Internal Error: CheckForKnownNullDereference called with input with no m_value");
return;
}
if (!execution->GetId().m_node->CanAcceptNullInput(*execution->GetId().m_slot, inputSlot))
{
AddError(execution->GetId().m_node->GetEntityId(), nullptr, ParseErrors::NullInputKnown);
}
}
void AbstractCodeModel::CheckConversion(ConversionByIndex& conversion, VariableConstPtr source, size_t index, const Data::Type& targetType)
{
const Data::Type& sourceType = source->m_datum.GetType();
@@ -698,7 +720,7 @@ namespace ScriptCanvas
}
AZStd::string AbstractCodeModel::CheckUniqueInterfaceNames
(AZStd::string_view candidate
( AZStd::string_view candidate
, AZStd::string_view defaultName
, AZStd::unordered_set<AZStd::string>& uniqueNames
, const AZStd::unordered_set<const ScriptCanvas::Nodes::Core::FunctionDefinitionNode*>& nodelingsOut)
@@ -4285,10 +4307,7 @@ namespace ScriptCanvas
void AbstractCodeModel::ParseInputDatum(ExecutionTreePtr execution, const Slot& input)
{
// \todo look for crossed lines in inferred functions, because sometimes, rather than the input
// being named of the result of the output that emitted it, it will be the name of the inferred function
// parameter ---> make a map of node output to function input names
AZ_Assert(execution->GetSymbol() != Symbol::FunctionDefinition, "Function definition input should have been handled already");
AZ_Assert(execution->GetSymbol() != Symbol::FunctionDefinition, "Function definition input is not handled in AbstractCodeModel::ParseInputDatum");
auto nodes = execution->GetId().m_node->GetConnectedNodes(input);
if (nodes.empty())
@@ -4298,17 +4317,6 @@ namespace ScriptCanvas
execution->AddInput({ &input, variable, DebugDataSource::FromVariable(input.GetId(), input.GetDataType(), variable->m_sourceVariableId) });
CheckConversion(execution->ModConversions(), variable, execution->GetInputCount() - 1, input.GetDataType());
}
// This concept may never actually be possible
// else if (RequiresCreationFunction(input.GetDataType().GetType()))
// {
// AddError(execution, aznew NotYetImplemented(
// "1: finish input created by name when connected to other nodes"
// "2: add the name to the scope"
// "3: and check inputs be re-used, common constructors like zero/1, etc"
// "4: read the variable name if it is present instead of creating it"
// "5: check for entity references to self and other member slice variables"
// "6: mark the variable with RequiredCreationFunction()"));
// }
else
{
auto variableDatum = input.FindDatum();
@@ -4345,26 +4353,30 @@ namespace ScriptCanvas
}
else
{
// we don't support this, yet, but visually we could
// we could support both things, technically...auto-generated inputs, and defaults on the non-connected
// execution thread, or whatever makes possible sense
// \todo send enough information to reveal the data path in the editor
// This isn't supported visually, yet, but technically, it could be.
// One could connect both latent execution and immediate execution to the same code execution path,
// but only one uses connected output, and the other uses defaults.
// It would require us to change the visualization of the shared path, based on which parent was clicked on in the editor.
const auto& targetNode = *execution->GetId().m_node;
const auto& targetSlot = input;
for (auto sourceNodeAndSlot : nodes)
{
AddError(nullptr, aznew ScopedDataConnectionEvent
(execution->GetNodeId()
( execution->GetNodeId()
, targetNode
, targetSlot
, *sourceNodeAndSlot.first
, *sourceNodeAndSlot.second));
}
return;
}
}
// Check for known null reads
CheckForKnownNullDereference(execution, execution->GetInput(execution->GetInputCount() - 1), input);
}
bool AbstractCodeModel::ParseInputThisPointer(ExecutionTreePtr execution)
Gems/ScriptCanvas/Code/Include/ScriptCanvas/Grammar/AbstractCodeModel.h
+3 -1
View File
@@ -207,8 +207,10 @@ namespace ScriptCanvas
bool CheckCreateRoot(const Node& node);
void CheckForKnownNullDereference(ExecutionTreeConstPtr parent, const ExecutionInput& input, const Slot& inputSlot);
AZStd::string CheckUniqueInterfaceNames
(AZStd::string_view candidate
( AZStd::string_view candidate
, AZStd::string_view defaultName
, AZStd::unordered_set<AZStd::string>& uniqueNames
, const AZStd::unordered_set<const ScriptCanvas::Nodes::Core::FunctionDefinitionNode*>& nodelingsOut);
Gems/ScriptCanvas/Code/Include/ScriptCanvas/Libraries/Core/Method.cpp
+40 -2
View File
@@ -25,6 +25,7 @@ namespace MethodCPP
Unnamed2,
PluralizeResults,
AddedPrettyNameFieldToSerialization,
StoreInputSlotIdsToSupportNullCheck,
// add your version above
Current,
};
@@ -110,6 +111,37 @@ namespace ScriptCanvas
{
namespace Core
{
bool Method::CanAcceptNullInput([[maybe_unused]] const Slot& executionSlot, const Slot& inputSlot) const
{
if (m_method)
{
auto candidateID = inputSlot.GetId();
auto slotIter = AZStd::find(m_inputSlots.begin(), m_inputSlots.end(), candidateID);
if (slotIter != m_inputSlots.end())
{
const size_t index = slotIter - m_inputSlots.begin();
if (index < m_method->GetNumArguments())
{
const auto* argument = m_method->GetArgument(index);
if (argument->m_traits & (AZ::BehaviorParameter::TR_REFERENCE | AZ::BehaviorParameter::TR_THIS_PTR))
{
// references and this pointers cannot accept null input
return false;
}
if (!(argument->m_traits & AZ::BehaviorParameter::TR_POINTER))
{
// values cannot accept null input
return false;
}
}
}
}
return true;
}
const AZ::BehaviorClass* Method::GetClass() const
{
return m_class;
@@ -232,6 +264,11 @@ namespace ScriptCanvas
if (addedSlot.IsValid())
{
MethodHelper::SetSlotToDefaultValue(*this, addedSlot, config, argIndex);
m_inputSlots.push_back(addedSlot);
}
else
{
AZ_Warning("ScriptCanvas", false, "Failed to add method input slot to Method node: %s-%s", config.m_prettyClassName.c_str(), config.m_method.m_name.c_str());
}
}
}
@@ -520,8 +557,8 @@ namespace ScriptCanvas
if (m_method && m_method->HasResult())
{
if (branchOnResultMethod.GetNumArguments() == 1
&& branchOnResultMethod.HasResult()
&& Data::FromAZType(branchOnResultMethod.GetResult()->m_typeId) == Data::Type::Boolean())
&& branchOnResultMethod.HasResult()
&& Data::FromAZType(branchOnResultMethod.GetResult()->m_typeId) == Data::Type::Boolean())
{
AZ::Uuid methodResultType = m_method->GetResult()->m_typeId;
AZ::Uuid branchOnResultMethodArgType = branchOnResultMethod.GetArgument(0)->m_typeId;
@@ -797,6 +834,7 @@ namespace ScriptCanvas
->Field("className", &Method::m_className)
->Field("namespaces", &Method::m_namespaces)
->Field("resultSlotIDs", &Method::m_resultSlotIDs)
->Field("inputSlots", &Method::m_inputSlots)
->Field("prettyClassName", &Method::m_classNamePretty)
;
Gems/ScriptCanvas/Code/Include/ScriptCanvas/Libraries/Core/Method.h
+3 -2
View File
@@ -45,6 +45,8 @@ namespace ScriptCanvas
size_t GenerateFingerprint() const override;
bool CanAcceptNullInput(const Slot& executionSlot, const Slot& inputSlot) const override;
bool GetBranchOnResultCheckName(AZStd::string& exposedName, Grammar::LexicalScope& lexicalScope) const;
virtual bool GetCheckedOperationInfo(AZ::CheckedOperationInfo& checkedInfo, AZStd::string& exposedName, Grammar::LexicalScope& lexicalScope) const;
@@ -149,8 +151,6 @@ namespace ScriptCanvas
bool IsExpectingResult() const;
AZ_INLINE AZStd::vector<SlotId>& ModResultSlotIds() { return m_resultSlotIDs; }
virtual void OnInitializeOutputPost(const MethodOutputConfig&) {}
virtual void OnInitializeOutputPre(MethodOutputConfig&) {}
@@ -176,6 +176,7 @@ namespace ScriptCanvas
NamespacePath m_namespaces;
const AZ::BehaviorMethod* m_method = nullptr;
const AZ::BehaviorClass* m_class = nullptr;
AZStd::vector<SlotId> m_inputSlots;
AZStd::vector<SlotId> m_resultSlotIDs;
AZStd::recursive_mutex m_mutex; // post-serialization
bool m_warnOnMissingFunction = true;
Gems/ScriptCanvas/Code/Include/ScriptCanvas/Results/ErrorText.h
+1
View File
@@ -74,6 +74,7 @@ namespace ScriptCanvas
constexpr const char* NotEnoughBranchesForReturn = "Not enough branches for defined out return values.";
constexpr const char* NotEnoughInputForArithmeticOperator = "Not enough input for arithmetic operator";
constexpr const char* NullEntityInGraph = "Null entity pointer in graph";
constexpr const char* NullInputKnown = "The input is known to be null, and the node does not accept it";
constexpr const char* ParseExecutionMultipleOutSyntaxSugarChildExecutionRemovedAndNotReplaced = "ParseExecutionMultipleOutSyntaxSugar: child execution node was removed, and not replaced.";
constexpr const char* ParseExecutionMultipleOutSyntaxSugarMismatchOutSize = "ParseExecutionMultipleOutSyntaxSugar: mismatch in connect nodes vs source slots size";
constexpr const char* ParseExecutionMultipleOutSyntaxSugarNonNullChildExecutionFound = "ParseExecutionMultipleOutSyntaxSugar: non null child execution node";