Skip to content

Commit

Permalink
Fix unresolved alias not being registered as identifiers
Browse files Browse the repository at this point in the history
Also check that identifiers are not conditional before taking them

the conditional index should be working but we should be able to do better (using "conditional scope id")
  • Loading branch information
SirLynix committed Aug 8, 2024
1 parent 4422c2d commit de4a36a
Show file tree
Hide file tree
Showing 5 changed files with 177 additions and 28 deletions.
2 changes: 1 addition & 1 deletion include/NZSL/Ast/ExpressionType.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ namespace nzsl::Ast
std::string name;
std::string tag;
std::vector<StructMember> members;
bool isConditional = false;
unsigned int conditionIndex = 0;
};

inline bool IsAliasType(const ExpressionType& type);
Expand Down
2 changes: 1 addition & 1 deletion include/NZSL/Ast/SanitizeVisitor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ namespace nzsl::Ast
{
std::size_t index;
IdentifierCategory category;
bool isConditional = false;
unsigned int conditionalIndex = 0;
};

struct Identifier
Expand Down
63 changes: 38 additions & 25 deletions src/NZSL/Ast/SanitizeVisitor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ namespace nzsl::Ast

struct UsedExternalData
{
bool isConditional;
unsigned int conditionalStatementIndex;
};

static constexpr std::size_t ModuleIdSentinel = std::numeric_limits<std::size_t>::max();
Expand All @@ -212,8 +212,9 @@ namespace nzsl::Ast
Options options;
FunctionData* currentFunction = nullptr;
bool allowUnknownIdentifiers = false;
bool inConditionalStatement = false;
bool inLoop = false;
unsigned int currentConditionalIndex = 0;
unsigned int nextConditionalIndex = 1;
};

ModulePtr SanitizeVisitor::Sanitize(const Module& module, const Options& options, std::string* error)
Expand Down Expand Up @@ -323,7 +324,12 @@ namespace nzsl::Ast
const auto& env = *m_context->modules[moduleIndex].environment;
identifierData = FindIdentifier(env, node.identifiers.front().identifier);
if (identifierData)
{
if (m_context->options.partialSanitization && identifierData->conditionalIndex != m_context->currentConditionalIndex)
return Cloner::Clone(node);

return HandleIdentifier(identifierData, node.identifiers.front().sourceLocation);
}
}
}

Expand Down Expand Up @@ -447,7 +453,7 @@ namespace nzsl::Ast

if (!fieldPtr)
{
if (s->isConditional)
if (s->conditionIndex != m_context->currentConditionalIndex)
return Cloner::Clone(node); //< unresolved

throw CompilerUnknownFieldError{ indexedExpr->sourceLocation, identifierEntry.identifier };
Expand Down Expand Up @@ -1150,6 +1156,9 @@ namespace nzsl::Ast
if (identifierData->category == IdentifierCategory::Unresolved)
return Cloner::Clone(node);

if (m_context->options.partialSanitization && identifierData->conditionalIndex != m_context->currentConditionalIndex)
return Cloner::Clone(node);

return HandleIdentifier(identifierData, node.sourceLocation);
}

Expand Down Expand Up @@ -1354,9 +1363,9 @@ namespace nzsl::Ast

if (!conditionValue.has_value())
{
bool wasInConditionalStatement = m_context->inConditionalStatement;
m_context->inConditionalStatement = true;
Nz::CallOnExit restoreCond([=] { m_context->inConditionalStatement = wasInConditionalStatement; });
unsigned int prevCondStatementIndex = m_context->currentConditionalIndex;
m_context->currentConditionalIndex = m_context->nextConditionalIndex++;
Nz::CallOnExit restoreCond([=] { m_context->currentConditionalIndex = prevCondStatementIndex; });

// Unresolvable condition
auto condStatement = ShaderBuilder::ConditionalStatement(std::move(cloneCondition), Cloner::Clone(*node.statement));
Expand Down Expand Up @@ -1447,7 +1456,7 @@ namespace nzsl::Ast
std::uint64_t bindingKey = BuildBindingKey(bindingSet, bindingIndex + i);
if (auto it = m_context->usedBindingIndexes.find(bindingKey); it != m_context->usedBindingIndexes.end())
{
if (!it->second.isConditional || !usedBindingData.isConditional)
if (it->second.conditionalStatementIndex == m_context->currentConditionalIndex || usedBindingData.conditionalStatementIndex == m_context->currentConditionalIndex)
throw CompilerExtBindingAlreadyUsedError{ sourceLoc, bindingSet, bindingIndex };
}

Expand All @@ -1462,11 +1471,11 @@ namespace nzsl::Ast
auto& extVar = clone->externalVars[i];

Context::UsedExternalData usedBindingData;
usedBindingData.isConditional = m_context->inConditionalStatement;
usedBindingData.conditionalStatementIndex = m_context->currentConditionalIndex;

if (auto it = m_context->declaredExternalVar.find(extVar.name); it != m_context->declaredExternalVar.end())
{
if (!it->second.isConditional || !usedBindingData.isConditional)
if (it->second.conditionalStatementIndex == m_context->currentConditionalIndex || usedBindingData.conditionalStatementIndex == m_context->currentConditionalIndex)
throw CompilerExtAlreadyDeclaredError{ extVar.sourceLocation, extVar.name };
}

Expand Down Expand Up @@ -1586,7 +1595,7 @@ namespace nzsl::Ast
bindingIndex++;

Context::UsedExternalData usedBindingData;
usedBindingData.isConditional = m_context->inConditionalStatement;
usedBindingData.conditionalStatementIndex = m_context->currentConditionalIndex;

extVar.bindingIndex = bindingIndex;
RegisterBinding(arraySize, bindingSet, bindingIndex, usedBindingData, extVar.sourceLocation);
Expand Down Expand Up @@ -1912,7 +1921,7 @@ namespace nzsl::Ast
}
}

clone->description.isConditional = m_context->inConditionalStatement;
clone->description.conditionIndex = m_context->currentConditionalIndex;

clone->structIndex = RegisterStruct(clone->description.name, &clone->description, clone->structIndex, clone->sourceLocation);
SanitizeIdentifier(clone->description.name, IdentifierScope::Struct);
Expand Down Expand Up @@ -3547,7 +3556,7 @@ namespace nzsl::Ast
bool unresolved = false;
if (const IdentifierData* identifierData = FindIdentifier(name))
{
if (!m_context->inConditionalStatement || !identifierData->isConditional)
if (identifierData->conditionalIndex == m_context->currentConditionalIndex)
throw CompilerIdentifierAlreadyUsedError{ sourceLocation, name };
else
unresolved = true;
Expand All @@ -3571,7 +3580,7 @@ namespace nzsl::Ast
{
aliasIndex,
IdentifierCategory::Alias,
m_context->inConditionalStatement
m_context->currentConditionalIndex
}
});
}
Expand Down Expand Up @@ -3602,7 +3611,7 @@ namespace nzsl::Ast
{
constantIndex,
IdentifierCategory::Constant,
m_context->inConditionalStatement
m_context->currentConditionalIndex
}
});

Expand Down Expand Up @@ -3654,7 +3663,7 @@ namespace nzsl::Ast
{
functionIndex,
IdentifierCategory::Function,
m_context->inConditionalStatement
m_context->currentConditionalIndex
}
});

Expand All @@ -3673,7 +3682,7 @@ namespace nzsl::Ast
{
intrinsicIndex,
IdentifierCategory::Intrinsic,
m_context->inConditionalStatement
m_context->currentConditionalIndex
}
});

Expand All @@ -3692,7 +3701,7 @@ namespace nzsl::Ast
{
moduleIndex,
IdentifierCategory::Module,
m_context->inConditionalStatement
m_context->currentConditionalIndex
}
});

Expand All @@ -3706,7 +3715,7 @@ namespace nzsl::Ast
{
std::numeric_limits<std::size_t>::max(),
IdentifierCategory::ReservedName,
m_context->inConditionalStatement
m_context->currentConditionalIndex
}
});
}
Expand All @@ -3716,7 +3725,7 @@ namespace nzsl::Ast
bool unresolved = false;
if (const IdentifierData* identifierData = FindIdentifier(name))
{
if (!m_context->inConditionalStatement || !identifierData->isConditional)
if (identifierData->conditionalIndex == m_context->currentConditionalIndex)
throw CompilerIdentifierAlreadyUsedError{ sourceLocation, name };
else
unresolved = true;
Expand All @@ -3740,7 +3749,7 @@ namespace nzsl::Ast
{
structIndex,
IdentifierCategory::Struct,
m_context->inConditionalStatement
m_context->currentConditionalIndex
}
});
}
Expand Down Expand Up @@ -3771,7 +3780,7 @@ namespace nzsl::Ast
{
typeIndex,
IdentifierCategory::Type,
m_context->inConditionalStatement
m_context->currentConditionalIndex
}
});

Expand Down Expand Up @@ -3805,7 +3814,7 @@ namespace nzsl::Ast
{
typeIndex,
IdentifierCategory::Type,
m_context->inConditionalStatement
m_context->currentConditionalIndex
}
});

Expand All @@ -3819,7 +3828,7 @@ namespace nzsl::Ast
{
std::numeric_limits<std::size_t>::max(),
IdentifierCategory::Unresolved,
m_context->inConditionalStatement
m_context->currentConditionalIndex
}
});
}
Expand All @@ -3832,7 +3841,8 @@ namespace nzsl::Ast
// Allow variable shadowing
if (identifier->category != IdentifierCategory::Variable)
throw CompilerIdentifierAlreadyUsedError{ sourceLocation, name };
else if (identifier->isConditional && m_context->inConditionalStatement)

if (identifier->conditionalIndex != m_context->currentConditionalIndex)
unresolved = true; //< right variable isn't know from this point
}

Expand All @@ -3854,7 +3864,7 @@ namespace nzsl::Ast
{
varIndex,
IdentifierCategory::Variable,
m_context->inConditionalStatement
m_context->currentConditionalIndex
}
});
}
Expand Down Expand Up @@ -4102,7 +4112,10 @@ namespace nzsl::Ast

const ExpressionType* exprType = GetExpressionType(*node.expression);
if (!exprType)
{
RegisterUnresolved(node.name);
return ValidationResult::Unresolved;
}

const ExpressionType& resolvedType = ResolveAlias(*exprType);

Expand Down
Loading

0 comments on commit de4a36a

Please sign in to comment.