Skip to content

Commit dfde6d4

Browse files
inbelicinbelic
committed
fix for static samplers
1 parent 282c4c1 commit dfde6d4

File tree

2 files changed

+63
-56
lines changed

2 files changed

+63
-56
lines changed

‎clang/lib/Parse/ParseHLSLRootSignature.cpp‎

Lines changed: 36 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,11 @@ std::optional<StaticSampler> RootSignatureParser::parseStaticSampler() {
367367
if (!Params.has_value())
368368
return std::nullopt;
369369

370+
if (consumeExpectedToken(TokenKind::pu_r_paren,
371+
diag::err_hlsl_unexpected_end_of_params,
372+
/*param of=*/TokenKind::kw_StaticSampler))
373+
return std::nullopt;
374+
370375
// Check mandatory parameters were provided
371376
if (!Params->Reg.has_value()) {
372377
reportDiag(diag::err_hlsl_rootsig_missing_param) << TokenKind::sReg;
@@ -412,11 +417,6 @@ std::optional<StaticSampler> RootSignatureParser::parseStaticSampler() {
412417
if (Params->Visibility.has_value())
413418
Sampler.Visibility = Params->Visibility.value();
414419

415-
if (consumeExpectedToken(TokenKind::pu_r_paren,
416-
diag::err_hlsl_unexpected_end_of_params,
417-
/*param of=*/TokenKind::kw_StaticSampler))
418-
return std::nullopt;
419-
420420
return Sampler;
421421
}
422422

@@ -664,9 +664,9 @@ RootSignatureParser::parseStaticSamplerParams() {
664664
"Expects to only be invoked starting at given token");
665665

666666
ParsedStaticSamplerParams Params;
667-
do {
668-
// `s` POS_INT
667+
while (!peekExpectedToken(TokenKind::pu_r_paren)) {
669668
if (tryConsumeExpectedToken(TokenKind::sReg)) {
669+
// `s` POS_INT
670670
if (Params.Reg.has_value()) {
671671
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
672672
return std::nullopt;
@@ -675,10 +675,8 @@ RootSignatureParser::parseStaticSamplerParams() {
675675
if (!Reg.has_value())
676676
return std::nullopt;
677677
Params.Reg = Reg;
678-
}
679-
680-
// `filter` `=` FILTER
681-
if (tryConsumeExpectedToken(TokenKind::kw_filter)) {
678+
} else if (tryConsumeExpectedToken(TokenKind::kw_filter)) {
679+
// `filter` `=` FILTER
682680
if (Params.Filter.has_value()) {
683681
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
684682
return std::nullopt;
@@ -691,10 +689,8 @@ RootSignatureParser::parseStaticSamplerParams() {
691689
if (!Filter.has_value())
692690
return std::nullopt;
693691
Params.Filter = Filter;
694-
}
695-
696-
// `addressU` `=` TEXTURE_ADDRESS
697-
if (tryConsumeExpectedToken(TokenKind::kw_addressU)) {
692+
} else if (tryConsumeExpectedToken(TokenKind::kw_addressU)) {
693+
// `addressU` `=` TEXTURE_ADDRESS
698694
if (Params.AddressU.has_value()) {
699695
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
700696
return std::nullopt;
@@ -707,10 +703,8 @@ RootSignatureParser::parseStaticSamplerParams() {
707703
if (!AddressU.has_value())
708704
return std::nullopt;
709705
Params.AddressU = AddressU;
710-
}
711-
712-
// `addressV` `=` TEXTURE_ADDRESS
713-
if (tryConsumeExpectedToken(TokenKind::kw_addressV)) {
706+
} else if (tryConsumeExpectedToken(TokenKind::kw_addressV)) {
707+
// `addressV` `=` TEXTURE_ADDRESS
714708
if (Params.AddressV.has_value()) {
715709
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
716710
return std::nullopt;
@@ -723,10 +717,8 @@ RootSignatureParser::parseStaticSamplerParams() {
723717
if (!AddressV.has_value())
724718
return std::nullopt;
725719
Params.AddressV = AddressV;
726-
}
727-
728-
// `addressW` `=` TEXTURE_ADDRESS
729-
if (tryConsumeExpectedToken(TokenKind::kw_addressW)) {
720+
} else if (tryConsumeExpectedToken(TokenKind::kw_addressW)) {
721+
// `addressW` `=` TEXTURE_ADDRESS
730722
if (Params.AddressW.has_value()) {
731723
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
732724
return std::nullopt;
@@ -739,10 +731,8 @@ RootSignatureParser::parseStaticSamplerParams() {
739731
if (!AddressW.has_value())
740732
return std::nullopt;
741733
Params.AddressW = AddressW;
742-
}
743-
744-
// `mipLODBias` `=` NUMBER
745-
if (tryConsumeExpectedToken(TokenKind::kw_mipLODBias)) {
734+
} else if (tryConsumeExpectedToken(TokenKind::kw_mipLODBias)) {
735+
// `mipLODBias` `=` NUMBER
746736
if (Params.MipLODBias.has_value()) {
747737
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
748738
return std::nullopt;
@@ -755,10 +745,8 @@ RootSignatureParser::parseStaticSamplerParams() {
755745
if (!MipLODBias.has_value())
756746
return std::nullopt;
757747
Params.MipLODBias = MipLODBias;
758-
}
759-
760-
// `maxAnisotropy` `=` POS_INT
761-
if (tryConsumeExpectedToken(TokenKind::kw_maxAnisotropy)) {
748+
} else if (tryConsumeExpectedToken(TokenKind::kw_maxAnisotropy)) {
749+
// `maxAnisotropy` `=` POS_INT
762750
if (Params.MaxAnisotropy.has_value()) {
763751
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
764752
return std::nullopt;
@@ -771,10 +759,8 @@ RootSignatureParser::parseStaticSamplerParams() {
771759
if (!MaxAnisotropy.has_value())
772760
return std::nullopt;
773761
Params.MaxAnisotropy = MaxAnisotropy;
774-
}
775-
776-
// `comparisonFunc` `=` COMPARISON_FUNC
777-
if (tryConsumeExpectedToken(TokenKind::kw_comparisonFunc)) {
762+
} else if (tryConsumeExpectedToken(TokenKind::kw_comparisonFunc)) {
763+
// `comparisonFunc` `=` COMPARISON_FUNC
778764
if (Params.CompFunc.has_value()) {
779765
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
780766
return std::nullopt;
@@ -787,10 +773,8 @@ RootSignatureParser::parseStaticSamplerParams() {
787773
if (!CompFunc.has_value())
788774
return std::nullopt;
789775
Params.CompFunc = CompFunc;
790-
}
791-
792-
// `borderColor` `=` STATIC_BORDER_COLOR
793-
if (tryConsumeExpectedToken(TokenKind::kw_borderColor)) {
776+
} else if (tryConsumeExpectedToken(TokenKind::kw_borderColor)) {
777+
// `borderColor` `=` STATIC_BORDER_COLOR
794778
if (Params.BorderColor.has_value()) {
795779
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
796780
return std::nullopt;
@@ -803,10 +787,8 @@ RootSignatureParser::parseStaticSamplerParams() {
803787
if (!BorderColor.has_value())
804788
return std::nullopt;
805789
Params.BorderColor = BorderColor;
806-
}
807-
808-
// `minLOD` `=` NUMBER
809-
if (tryConsumeExpectedToken(TokenKind::kw_minLOD)) {
790+
} else if (tryConsumeExpectedToken(TokenKind::kw_minLOD)) {
791+
// `minLOD` `=` NUMBER
810792
if (Params.MinLOD.has_value()) {
811793
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
812794
return std::nullopt;
@@ -819,10 +801,8 @@ RootSignatureParser::parseStaticSamplerParams() {
819801
if (!MinLOD.has_value())
820802
return std::nullopt;
821803
Params.MinLOD = MinLOD;
822-
}
823-
824-
// `maxLOD` `=` NUMBER
825-
if (tryConsumeExpectedToken(TokenKind::kw_maxLOD)) {
804+
} else if (tryConsumeExpectedToken(TokenKind::kw_maxLOD)) {
805+
// `maxLOD` `=` NUMBER
826806
if (Params.MaxLOD.has_value()) {
827807
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
828808
return std::nullopt;
@@ -835,10 +815,8 @@ RootSignatureParser::parseStaticSamplerParams() {
835815
if (!MaxLOD.has_value())
836816
return std::nullopt;
837817
Params.MaxLOD = MaxLOD;
838-
}
839-
840-
// `space` `=` POS_INT
841-
if (tryConsumeExpectedToken(TokenKind::kw_space)) {
818+
} else if (tryConsumeExpectedToken(TokenKind::kw_space)) {
819+
// `space` `=` POS_INT
842820
if (Params.Space.has_value()) {
843821
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
844822
return std::nullopt;
@@ -851,10 +829,8 @@ RootSignatureParser::parseStaticSamplerParams() {
851829
if (!Space.has_value())
852830
return std::nullopt;
853831
Params.Space = Space;
854-
}
855-
856-
// `visibility` `=` SHADER_VISIBILITY
857-
if (tryConsumeExpectedToken(TokenKind::kw_visibility)) {
832+
} else if (tryConsumeExpectedToken(TokenKind::kw_visibility)) {
833+
// `visibility` `=` SHADER_VISIBILITY
858834
if (Params.Visibility.has_value()) {
859835
reportDiag(diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind;
860836
return std::nullopt;
@@ -868,7 +844,11 @@ RootSignatureParser::parseStaticSamplerParams() {
868844
return std::nullopt;
869845
Params.Visibility = Visibility;
870846
}
871-
} while (tryConsumeExpectedToken(TokenKind::pu_comma));
847+
848+
// ',' denotes another element, otherwise, expected to be at ')'
849+
if (!tryConsumeExpectedToken(TokenKind::pu_comma))
850+
break;
851+
}
872852

873853
return Params;
874854
}

‎clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1373,4 +1373,31 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidDescriptorClauseParamsCommaTest) {
13731373
ASSERT_TRUE(Consumer->isSatisfied());
13741374
}
13751375

1376+
TEST_F(ParseHLSLRootSignatureTest, InvalidStaticSamplerCommaTest) {
1377+
// This test will check that an error is produced when there is a missing
1378+
// comma between parameters
1379+
const llvm::StringLiteral Source = R"cc(
1380+
StaticSampler(
1381+
s0
1382+
maxLOD = 3
1383+
)
1384+
)cc";
1385+
1386+
auto Ctx = createMinimalASTContext();
1387+
StringLiteral *Signature = wrapSource(Ctx, Source);
1388+
1389+
TrivialModuleLoader ModLoader;
1390+
auto PP = createPP(Source, ModLoader);
1391+
1392+
SmallVector<RootElement> Elements;
1393+
hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements,
1394+
Signature, *PP);
1395+
1396+
// Test correct diagnostic produced
1397+
Consumer->setExpected(diag::err_hlsl_unexpected_end_of_params);
1398+
ASSERT_TRUE(Parser.parse());
1399+
1400+
ASSERT_TRUE(Consumer->isSatisfied());
1401+
}
1402+
13761403
} // anonymous namespace

0 commit comments

Comments
 (0)