Skip to content

Commit b323afd

Browse files
authored
Improve Syntax::makeValidName function (AcademySoftwareFoundation#2598)
This PR improves the logic around how valid names are generated with consideration for reserved words defined in specific Syntax objects. Currently the shader generator calls `Syntax::makeIdentifier()` to ensure that variable names are legal. This compares the current name against a map of already used identifiers. The reserved words for a syntax are prepopulated in this map. There are some places, namely [here](https://github.com/AcademySoftwareFoundation/MaterialX/blob/main/source/MaterialXGenShader/Nodes/CompoundNode.cpp#L40) and [here](https://github.com/AcademySoftwareFoundation/MaterialX/blob/main/source/MaterialXGenShader/Nodes/SourceCodeNode.cpp#L70), where we generate names that need to be legal, but do not call `makeIdentifier()` but instead just call `makeValidName()`. Which got me to thinking that make `makeValidName()` should be doing the reserved word check. Moving the check to `makeValidName()` means we do not have to copy all of the reserved words in to the identifier map for each `ShaderGraph` object we create, as well as plugging the potential bug of being able to generate illegal function names.
1 parent 3435676 commit b323afd

2 files changed

Lines changed: 7 additions & 6 deletions

File tree

source/MaterialXGenShader/ShaderGraph.cpp

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,10 @@ MATERIALX_NAMESPACE_BEGIN
1818
// ShaderGraph methods
1919
//
2020

21-
ShaderGraph::ShaderGraph(const ShaderGraph* parent, const string& name, ConstDocumentPtr document, const StringSet& reservedWords) :
21+
ShaderGraph::ShaderGraph(const ShaderGraph* parent, const string& name, ConstDocumentPtr document, const StringSet& /*reservedWords*/) :
2222
ShaderNode(parent, name),
2323
_document(document)
2424
{
25-
// Add all reserved words as taken identifiers
26-
for (const string& n : reservedWords)
27-
{
28-
_identifiers[n] = 1;
29-
}
3025
}
3126

3227
void ShaderGraph::addInputSockets(const InterfaceElement& elem, GenContext& context)

source/MaterialXGenShader/Syntax.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,12 @@ void Syntax::makeValidName(string& name) const
153153
{
154154
std::replace_if(name.begin(), name.end(), isInvalidChar, '_');
155155
name = replaceSubstrings(name, _invalidTokens);
156+
if (std::find(_reservedWords.begin(), _reservedWords.end(), name) != _reservedWords.end())
157+
{
158+
// We append "1" here because thats the prior behavior from makeIdentifier() below when
159+
// the reservedWords were added to the identifiers list.
160+
name = name+"1";
161+
}
156162
}
157163

158164
void Syntax::makeIdentifier(string& name, IdentifierMap& identifiers) const

0 commit comments

Comments
 (0)