Skip to content

Commit 9f20b67

Browse files
SirLynixSirLynix
committed
Fix unresolved alias not being registered as identifiers
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")
1 parent 8263f10 commit 9f20b67

File tree

2 files changed

+172
-24
lines changed

2 files changed

+172
-24
lines changed

‎src/NZSL/Ast/SanitizeVisitor.cpp

Lines changed: 35 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ namespace nzsl::Ast
213213
FunctionData* currentFunction = nullptr;
214214
bool allowUnknownIdentifiers = false;
215215
bool inLoop = false;
216-
unsigned int conditionalStatementIndex = 0;
216+
unsigned int currentConditionalIndex = 0;
217217
unsigned int nextConditionalIndex = 1;
218218
};
219219

@@ -324,7 +324,12 @@ namespace nzsl::Ast
324324
const auto& env = *m_context->modules[moduleIndex].environment;
325325
identifierData = FindIdentifier(env, node.identifiers.front().identifier);
326326
if (identifierData)
327+
{
328+
if (m_context->options.partialSanitization && identifierData->conditionalIndex != m_context->currentConditionalIndex)
329+
return Cloner::Clone(node);
330+
327331
return HandleIdentifier(identifierData, node.identifiers.front().sourceLocation);
332+
}
328333
}
329334
}
330335

@@ -448,7 +453,7 @@ namespace nzsl::Ast
448453

449454
if (!fieldPtr)
450455
{
451-
if (s->conditionIndex != m_context->conditionalStatementIndex)
456+
if (s->conditionIndex != m_context->currentConditionalIndex)
452457
return Cloner::Clone(node); //< unresolved
453458

454459
throw CompilerUnknownFieldError{ indexedExpr->sourceLocation, identifierEntry.identifier };
@@ -1151,6 +1156,9 @@ namespace nzsl::Ast
11511156
if (identifierData->category == IdentifierCategory::Unresolved)
11521157
return Cloner::Clone(node);
11531158

1159+
if (m_context->options.partialSanitization && identifierData->conditionalIndex != m_context->currentConditionalIndex)
1160+
return Cloner::Clone(node);
1161+
11541162
return HandleIdentifier(identifierData, node.sourceLocation);
11551163
}
11561164

@@ -1355,9 +1363,9 @@ namespace nzsl::Ast
13551363

13561364
if (!conditionValue.has_value())
13571365
{
1358-
unsigned int prevCondStatementIndex = m_context->conditionalStatementIndex;
1359-
m_context->conditionalStatementIndex = m_context->nextConditionalIndex++;
1360-
Nz::CallOnExit restoreCond([=] { m_context->conditionalStatementIndex = prevCondStatementIndex; });
1366+
unsigned int prevCondStatementIndex = m_context->currentConditionalIndex;
1367+
m_context->currentConditionalIndex = m_context->nextConditionalIndex++;
1368+
Nz::CallOnExit restoreCond([=] { m_context->currentConditionalIndex = prevCondStatementIndex; });
13611369

13621370
// Unresolvable condition
13631371
auto condStatement = ShaderBuilder::ConditionalStatement(std::move(cloneCondition), Cloner::Clone(*node.statement));
@@ -1448,7 +1456,7 @@ namespace nzsl::Ast
14481456
std::uint64_t bindingKey = BuildBindingKey(bindingSet, bindingIndex + i);
14491457
if (auto it = m_context->usedBindingIndexes.find(bindingKey); it != m_context->usedBindingIndexes.end())
14501458
{
1451-
if (it->second.conditionalStatementIndex == m_context->conditionalStatementIndex || usedBindingData.conditionalStatementIndex == m_context->conditionalStatementIndex)
1459+
if (it->second.conditionalStatementIndex == m_context->currentConditionalIndex || usedBindingData.conditionalStatementIndex == m_context->currentConditionalIndex)
14521460
throw CompilerExtBindingAlreadyUsedError{ sourceLoc, bindingSet, bindingIndex };
14531461
}
14541462

@@ -1463,11 +1471,11 @@ namespace nzsl::Ast
14631471
auto& extVar = clone->externalVars[i];
14641472

14651473
Context::UsedExternalData usedBindingData;
1466-
usedBindingData.conditionalStatementIndex = m_context->conditionalStatementIndex;
1474+
usedBindingData.conditionalStatementIndex = m_context->currentConditionalIndex;
14671475

14681476
if (auto it = m_context->declaredExternalVar.find(extVar.name); it != m_context->declaredExternalVar.end())
14691477
{
1470-
if (it->second.conditionalStatementIndex == m_context->conditionalStatementIndex || usedBindingData.conditionalStatementIndex == m_context->conditionalStatementIndex)
1478+
if (it->second.conditionalStatementIndex == m_context->currentConditionalIndex || usedBindingData.conditionalStatementIndex == m_context->currentConditionalIndex)
14711479
throw CompilerExtAlreadyDeclaredError{ extVar.sourceLocation, extVar.name };
14721480
}
14731481

@@ -1587,7 +1595,7 @@ namespace nzsl::Ast
15871595
bindingIndex++;
15881596

15891597
Context::UsedExternalData usedBindingData;
1590-
usedBindingData.conditionalStatementIndex = m_context->conditionalStatementIndex;
1598+
usedBindingData.conditionalStatementIndex = m_context->currentConditionalIndex;
15911599

15921600
extVar.bindingIndex = bindingIndex;
15931601
RegisterBinding(arraySize, bindingSet, bindingIndex, usedBindingData, extVar.sourceLocation);
@@ -1913,7 +1921,7 @@ namespace nzsl::Ast
19131921
}
19141922
}
19151923

1916-
clone->description.conditionIndex = m_context->conditionalStatementIndex;
1924+
clone->description.conditionIndex = m_context->currentConditionalIndex;
19171925

19181926
clone->structIndex = RegisterStruct(clone->description.name, &clone->description, clone->structIndex, clone->sourceLocation);
19191927
SanitizeIdentifier(clone->description.name, IdentifierScope::Struct);
@@ -3548,7 +3556,7 @@ namespace nzsl::Ast
35483556
bool unresolved = false;
35493557
if (const IdentifierData* identifierData = FindIdentifier(name))
35503558
{
3551-
if (identifierData->conditionalIndex == m_context->conditionalStatementIndex)
3559+
if (identifierData->conditionalIndex == m_context->currentConditionalIndex)
35523560
throw CompilerIdentifierAlreadyUsedError{ sourceLocation, name };
35533561
else
35543562
unresolved = true;
@@ -3572,7 +3580,7 @@ namespace nzsl::Ast
35723580
{
35733581
aliasIndex,
35743582
IdentifierCategory::Alias,
3575-
m_context->conditionalStatementIndex
3583+
m_context->currentConditionalIndex
35763584
}
35773585
});
35783586
}
@@ -3603,7 +3611,7 @@ namespace nzsl::Ast
36033611
{
36043612
constantIndex,
36053613
IdentifierCategory::Constant,
3606-
m_context->conditionalStatementIndex
3614+
m_context->currentConditionalIndex
36073615
}
36083616
});
36093617

@@ -3655,7 +3663,7 @@ namespace nzsl::Ast
36553663
{
36563664
functionIndex,
36573665
IdentifierCategory::Function,
3658-
m_context->conditionalStatementIndex
3666+
m_context->currentConditionalIndex
36593667
}
36603668
});
36613669

@@ -3674,7 +3682,7 @@ namespace nzsl::Ast
36743682
{
36753683
intrinsicIndex,
36763684
IdentifierCategory::Intrinsic,
3677-
m_context->conditionalStatementIndex
3685+
m_context->currentConditionalIndex
36783686
}
36793687
});
36803688

@@ -3693,7 +3701,7 @@ namespace nzsl::Ast
36933701
{
36943702
moduleIndex,
36953703
IdentifierCategory::Module,
3696-
m_context->conditionalStatementIndex
3704+
m_context->currentConditionalIndex
36973705
}
36983706
});
36993707

@@ -3707,7 +3715,7 @@ namespace nzsl::Ast
37073715
{
37083716
std::numeric_limits<std::size_t>::max(),
37093717
IdentifierCategory::ReservedName,
3710-
m_context->conditionalStatementIndex
3718+
m_context->currentConditionalIndex
37113719
}
37123720
});
37133721
}
@@ -3717,7 +3725,7 @@ namespace nzsl::Ast
37173725
bool unresolved = false;
37183726
if (const IdentifierData* identifierData = FindIdentifier(name))
37193727
{
3720-
if (identifierData->conditionalIndex == m_context->conditionalStatementIndex)
3728+
if (identifierData->conditionalIndex == m_context->currentConditionalIndex)
37213729
throw CompilerIdentifierAlreadyUsedError{ sourceLocation, name };
37223730
else
37233731
unresolved = true;
@@ -3741,7 +3749,7 @@ namespace nzsl::Ast
37413749
{
37423750
structIndex,
37433751
IdentifierCategory::Struct,
3744-
m_context->conditionalStatementIndex
3752+
m_context->currentConditionalIndex
37453753
}
37463754
});
37473755
}
@@ -3772,7 +3780,7 @@ namespace nzsl::Ast
37723780
{
37733781
typeIndex,
37743782
IdentifierCategory::Type,
3775-
m_context->conditionalStatementIndex
3783+
m_context->currentConditionalIndex
37763784
}
37773785
});
37783786

@@ -3806,7 +3814,7 @@ namespace nzsl::Ast
38063814
{
38073815
typeIndex,
38083816
IdentifierCategory::Type,
3809-
m_context->conditionalStatementIndex
3817+
m_context->currentConditionalIndex
38103818
}
38113819
});
38123820

@@ -3820,7 +3828,7 @@ namespace nzsl::Ast
38203828
{
38213829
std::numeric_limits<std::size_t>::max(),
38223830
IdentifierCategory::Unresolved,
3823-
m_context->conditionalStatementIndex
3831+
m_context->currentConditionalIndex
38243832
}
38253833
});
38263834
}
@@ -3834,7 +3842,7 @@ namespace nzsl::Ast
38343842
if (identifier->category != IdentifierCategory::Variable)
38353843
throw CompilerIdentifierAlreadyUsedError{ sourceLocation, name };
38363844

3837-
if (identifier->conditionalIndex != m_context->conditionalStatementIndex)
3845+
if (identifier->conditionalIndex != m_context->currentConditionalIndex)
38383846
unresolved = true; //< right variable isn't know from this point
38393847
}
38403848

@@ -3856,7 +3864,7 @@ namespace nzsl::Ast
38563864
{
38573865
varIndex,
38583866
IdentifierCategory::Variable,
3859-
m_context->conditionalStatementIndex
3867+
m_context->currentConditionalIndex
38603868
}
38613869
});
38623870
}
@@ -4104,7 +4112,10 @@ namespace nzsl::Ast
41044112

41054113
const ExpressionType* exprType = GetExpressionType(*node.expression);
41064114
if (!exprType)
4115+
{
4116+
RegisterUnresolved(node.name);
41074117
return ValidationResult::Unresolved;
4118+
}
41084119

41094120
const ExpressionType& resolvedType = ResolveAlias(*exprType);
41104121

‎tests/src/Tests/ModuleTests.cpp

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -825,4 +825,141 @@ OpStore
825825
OpReturn
826826
OpFunctionEnd)");
827827
}
828+
829+
WHEN("Testing forward vs deferred based on option")
830+
{
831+
// Test a bugfix where an unresolved identifier (identifier imported from an unknown module when precompiling) was being resolved in
832+
833+
std::string_view gbufferOutput = R"(
834+
[nzsl_version("1.0")]
835+
module DeferredShading.GBuffer;
836+
837+
[export]
838+
struct GBufferOutput
839+
{
840+
[location(0)] albedo: vec4[f32],
841+
[location(1)] normal: vec4[f32],
842+
}
843+
)";
844+
845+
std::string_view nzslSource = R"(
846+
[nzsl_version("1.0")]
847+
module;
848+
849+
import GBufferOutput from DeferredShading.GBuffer;
850+
851+
option ForwardPass: bool = true;
852+
853+
[cond(ForwardPass)]
854+
struct FragOut
855+
{
856+
[location(0)] color: vec4[f32]
857+
}
858+
859+
[cond(!ForwardPass)]
860+
alias FragOut = GBufferOutput;
861+
862+
[entry(frag)]
863+
fn FragMain() -> FragOut
864+
{
865+
let color = vec4[f32](1.0, 0.0, 0.0, 1.0);
866+
867+
const if (ForwardPass)
868+
{
869+
let output: FragOut;
870+
output.color = color;
871+
872+
return output;
873+
}
874+
else
875+
{
876+
let normal = vec3[f32](0.0, 1.0, 0.0);
877+
878+
let output: FragOut;
879+
output.albedo = color;
880+
output.normal = vec4[f32](normal, 1.0);
881+
882+
return output;
883+
}
884+
}
885+
)";
886+
887+
nzsl::Ast::ModulePtr shaderModule = nzsl::Parse(nzslSource);
888+
889+
auto directoryModuleResolver = std::make_shared<nzsl::FilesystemModuleResolver>();
890+
RegisterModule(directoryModuleResolver, gbufferOutput);
891+
892+
nzsl::Ast::SanitizeVisitor::Options options;
893+
options.partialSanitization = true;
894+
895+
REQUIRE_NOTHROW(shaderModule = nzsl::Ast::Sanitize(*shaderModule, options));
896+
897+
options.moduleResolver = directoryModuleResolver;
898+
options.partialSanitization = false;
899+
options.removeOptionDeclaration = true;
900+
901+
WHEN("Trying ForwardPass=true")
902+
{
903+
options.optionValues[nzsl::Ast::HashOption("ForwardPass")] = true;
904+
905+
REQUIRE_NOTHROW(shaderModule = nzsl::Ast::Sanitize(*shaderModule, options));
906+
907+
ExpectNZSL(*shaderModule, R"(
908+
struct FragOut
909+
{
910+
[location(0)] color: vec4[f32]
911+
}
912+
913+
[entry(frag)]
914+
fn FragMain() -> FragOut
915+
{
916+
let color: vec4[f32] = vec4[f32](1.0, 0.0, 0.0, 1.0);
917+
{
918+
let output: FragOut;
919+
output.color = color;
920+
return output;
921+
}
922+
923+
}
924+
)");
925+
}
926+
927+
928+
WHEN("Trying ForwardPass=false")
929+
{
930+
options.optionValues[nzsl::Ast::HashOption("ForwardPass")] = false;
931+
932+
REQUIRE_NOTHROW(shaderModule = nzsl::Ast::Sanitize(*shaderModule, options));
933+
934+
ExpectNZSL(*shaderModule, R"(
935+
[nzsl_version("1.0")]
936+
module _DeferredShading_GBuffer
937+
{
938+
struct GBufferOutput
939+
{
940+
[location(0)] albedo: vec4[f32],
941+
[location(1)] normal: vec4[f32]
942+
}
943+
944+
}
945+
alias GBufferOutput = _DeferredShading_GBuffer.GBufferOutput;
946+
947+
alias FragOut = GBufferOutput;
948+
949+
[entry(frag)]
950+
fn FragMain() -> FragOut
951+
{
952+
let color: vec4[f32] = vec4[f32](1.0, 0.0, 0.0, 1.0);
953+
{
954+
let normal: vec3[f32] = vec3[f32](0.0, 1.0, 0.0);
955+
let output: FragOut;
956+
output.albedo = color;
957+
output.normal = vec4[f32](normal, 1.0);
958+
return output;
959+
}
960+
961+
}
962+
)");
963+
}
964+
}
828965
}

0 commit comments

Comments
 (0)