HLSL: allow scalar type keywords as identifiers, and add half type support.
HLSL allows type keywords to also be identifiers, so a sequence such as "float half = 3" is
valid, or more bizzarely, something like "float.float = int.uint + bool;"
There are places this is not supported. E.g, it's permitted for struct members, but not struct
names or functions. Also, vector or matrix types such as "float3" are not permitted as
identifiers.
This PR adds that support, as well as support for the "half" type. In production shaders,
this was seen with variables named "half". The PR attempts to support this without breaking
useful grammar errors such as "; expected" at the end of unterminated statements, so it errs
on that side at the possible expense of failing to accept valid constructs containing a type
keyword identifier. If others are discovered, they can be added.
Also, half is now accepted as a valid type, alongside the min*float types.
diff --git a/hlsl/hlslGrammar.cpp b/hlsl/hlslGrammar.cpp
index b874aa8..b5dfddd 100755
--- a/hlsl/hlslGrammar.cpp
+++ b/hlsl/hlslGrammar.cpp
@@ -85,21 +85,36 @@
return true;
}
- // Even though "sample" is a keyword (for interpolation modifiers), it IS still accepted as
- // an identifier. This appears to be a solitary exception: other interp modifier keywords such
- // as "linear" or "centroid" NOT valid identifiers. This code special cases "sample",
- // so e.g, "int sample;" is accepted.
- if (peekTokenClass(EHTokSample)) {
- token.string = NewPoolTString("sample");
- token.tokenClass = EHTokIdentifier;
- token.symbol = nullptr;
-
- idToken = token;
- advanceToken();
- return true;
+ // Even though "sample", "bool", "float", etc keywords (for types, interpolation modifiers),
+ // they ARE still accepted as identifiers. This is not a dense space: e.g, "void" is not a
+ // valid identifier, nor is "linear". This code special cases the known instances of this, so
+ // e.g, "int sample;" or "float float;" is accepted. Other cases can be added here if needed.
+
+ TString* idString = nullptr;
+ switch (peek()) {
+ case EHTokSample: idString = NewPoolTString("sample"); break;
+ case EHTokHalf: idString = NewPoolTString("half"); break;
+ case EHTokBool: idString = NewPoolTString("bool"); break;
+ case EHTokFloat: idString = NewPoolTString("float"); break;
+ case EHTokDouble: idString = NewPoolTString("double"); break;
+ case EHTokInt: idString = NewPoolTString("int"); break;
+ case EHTokUint: idString = NewPoolTString("uint"); break;
+ case EHTokMin16float: idString = NewPoolTString("min16float"); break;
+ case EHTokMin10float: idString = NewPoolTString("min10float"); break;
+ case EHTokMin16int: idString = NewPoolTString("min16int"); break;
+ case EHTokMin12int: idString = NewPoolTString("min12int"); break;
+ default:
+ return false;
}
- return false;
+ token.string = idString;
+ token.tokenClass = EHTokIdentifier;
+ token.symbol = nullptr;
+ idToken = token;
+
+ advanceToken();
+
+ return true;
}
// compilationUnit
@@ -418,7 +433,15 @@
// SEMICOLON
if (! acceptTokenClass(EHTokSemicolon)) {
- expected(";");
+ // This may have been a false detection of what appeared to be a declaration, but
+ // was actually an assignment such as "float = 4", where "float" is an identifier.
+ // We put the token back to let further parsing happen for cases where that may
+ // happen. This errors on the side of caution, and mostly triggers the error.
+
+ if (peek() == EHTokAssign || peek() == EHTokLeftBracket || peek() == EHTokDot || peek() == EHTokComma)
+ recedeToken();
+ else
+ expected(";");
return false;
}
@@ -1086,6 +1109,7 @@
// changes, e.g, to use native halfs.
static const TBasicType min16float_bt = EbtFloat;
static const TBasicType min10float_bt = EbtFloat;
+ static const TBasicType half_bt = EbtFloat;
static const TBasicType min16int_bt = EbtInt;
static const TBasicType min12int_bt = EbtInt;
static const TBasicType min16uint_bt = EbtUint;
@@ -1255,6 +1279,23 @@
new(&type) TType(EbtBool, EvqTemporary, 4);
break;
+ case EHTokHalf:
+ new(&type) TType(half_bt, EvqTemporary, EpqMedium);
+ break;
+ case EHTokHalf1:
+ new(&type) TType(half_bt, EvqTemporary, EpqMedium);
+ type.makeVector();
+ break;
+ case EHTokHalf2:
+ new(&type) TType(half_bt, EvqTemporary, EpqMedium, 2);
+ break;
+ case EHTokHalf3:
+ new(&type) TType(half_bt, EvqTemporary, EpqMedium, 3);
+ break;
+ case EHTokHalf4:
+ new(&type) TType(half_bt, EvqTemporary, EpqMedium, 4);
+ break;
+
case EHTokMin16float:
new(&type) TType(min16float_bt, EvqTemporary, EpqMedium);
break;
@@ -1683,6 +1724,7 @@
bool HlslGrammar::acceptStructDeclarationList(TTypeList*& typeList)
{
typeList = new TTypeList();
+ HlslToken idToken;
do {
// success on seeing the RIGHT_BRACE coming up
@@ -1700,8 +1742,7 @@
// struct_declarator COMMA struct_declarator ...
do {
- // peek IDENTIFIER
- if (! peekTokenClass(EHTokIdentifier)) {
+ if (! acceptIdentifier(idToken)) {
expected("member name");
return false;
}
@@ -1709,12 +1750,9 @@
// add it to the list of members
TTypeLoc member = { new TType(EbtVoid), token.loc };
member.type->shallowCopy(memberType);
- member.type->setFieldName(*token.string);
+ member.type->setFieldName(*idToken.string);
typeList->push_back(member);
- // accept IDENTIFIER
- advanceToken();
-
// array_specifier
TArraySizes* arraySizes = nullptr;
acceptArraySpecifier(arraySizes);
@@ -2322,7 +2360,9 @@
// arguments
TIntermTyped* arguments = nullptr;
if (! acceptArguments(constructorFunction, arguments)) {
- expected("constructor arguments");
+ // It's possible this is a type keyword used as an identifier. Put the token back
+ // for later use.
+ recedeToken();
return false;
}