Summary
Parsing::SkipFloatNumber() in Common/interface/ParsingTools.hpp requires the exponent of a float literal to carry an explicit +/- sign. Literals such as 1e10, 6.02e23 and 2.0E8 — valid in C, C++, HLSL and GLSL — are therefore only partially consumed.
Because Parsing::Tokenize() (documented as tokenizing "using the C-language syntax") relies on this function, the HLSL tokenizer splits such a literal into a numeric constant plus a spurious identifier. This affects HLSL2GLSLConverterImpl and HLSLParsingTools, both of which tokenize user shader source.
The public helper CountFloatNumberChars() in Common/interface/StringTools.hpp returns wrong results for the same inputs.
Affected code
https://github.com/DiligentGraphics/DiligentCore/blob/master/Common/interface/ParsingTools.hpp#L364-L376
++c;
if (c == End || (*c != '+' && *c != '-'))
{
// 10e&
return Pos;
}
++c;
if (c == End || !IsNum(*c))
{
// 10e+x
return Pos;
}
The first check treats a missing sign the same as a malformed exponent and bails out, returning the position saved before the e. Per the C grammar the sign is optional (exponent-part: e sign_opt digit-sequence).
Reproduction 1 — SkipFloatNumber / CountFloatNumberChars
#include <cstring>
#include "Common/interface/ParsingTools.hpp"
using namespace Diligent;
size_t Skip(const char* s)
{
const char* End = s + strlen(s);
return static_cast<size_t>(Parsing::SkipFloatNumber(s, End) - s);
}
| input |
expected |
actual |
consumed |
1e10 |
4 |
1 |
1 |
1E10 |
4 |
1 |
1 |
1.5e10 |
6 |
3 |
1.5 |
2.0E8 |
5 |
3 |
2.0 |
1e10f |
5 |
1 |
1 |
6.02e23 |
7 |
4 |
6.02 |
1e0 |
3 |
1 |
1 |
1e+10 |
5 |
5 |
1e+10 (OK) |
1e-10 |
5 |
5 |
1e-10 (OK) |
Reproduction 2 — HLSL tokenizer
Using the engine's own Parsing::HLSLTokenizer (Graphics/ShaderTools/src/HLSLTokenizer.cpp):
source: float k = 6.02e23;
other "float"
Identifier "k"
Assignment "="
NumericConstant "6.02" <-- truncated
Identifier "e23" <-- spurious identifier
Semicolon ";"
source: float x = 1e+10; (signed exponent, for contrast)
NumericConstant "1e+10" <-- correct
Impact
The token stream handed to the HLSL→GLSL converter is corrupted for any shader using unsigned exponent notation. One concrete consequence: HLSL2GLSLConverterImpl.cpp strips the f/F suffix from numeric constants specifically to avoid GLSL compiler warnings —
https://github.com/DiligentGraphics/DiligentCore/blob/master/Graphics/HLSL2GLSLConverterLib/src/HLSL2GLSLConverterImpl.cpp#L4590-L4597
case TokenType::NumericConstant:
// This all work is only required because some GLSL compilers are so stupid that
// flood shader output with insane warnings like this:
// WARNING: 0:259: Only GLSL version > 110 allows postfix "F" or "f" for float
if (Token->Literal.back() == 'f' || Token->Literal.back() == 'F')
Token->Literal.pop_back();
For 1e10f the suffix ends up on the spurious Identifier token (e10f), not on the NumericConstant (1), so the stripping is silently bypassed and the warning this code exists to prevent reappears. More generally, injecting an unexpected identifier into the stream of a source-to-source translator is a latent hazard for the identifier-driven transforms around it.
This has likely gone unnoticed because the notation is consistently avoided in-tree. Across the shaders shipped in DiligentCore, DiligentFX, DiligentSamples and DiligentTools there are 117 float literals using exponent notation and zero without an explicit sign — e.g. float ClosestDistance = 1e+10; in DiligentFX/Hydrogent/shaders/HnUpdateClosestSelectedLocation.psh, where 1e10 would be the more natural spelling. User-authored shaders have no reason to follow that convention.
The existing unit tests cover -1e+2, +1e-3, -1e, -1e+ and e5, but no unsigned-exponent case such as 1e10, which is why the gap was not caught.
Suggested fix
Make the sign optional and let the existing digit check reject genuinely malformed input:
++c;
// The exponent sign is optional: 1e10 is as valid as 1e+10.
if (c != End && (*c == '+' || *c == '-'))
++c;
if (c == End || !IsNum(*c))
{
// 10e&, 10e+x
return Pos;
}
I verified this patch locally:
- all 9 previously failing cases above now pass, and both reproductions above produce correct output (
6.02e23 tokenizes as a single NumericConstant);
- all 94 existing
CountFloatNumberChars assertions in Tests/DiligentCoreTest/src/Common/StringToolsTest.cpp still pass, with 0 regressions.
Malformed input is still rejected: 10e&, 10e+x, -1e, -1e+ and e5 behave exactly as before, since the digit check that follows is unchanged.
Worth adding test coverage for 1e10, 1E10, 1.5e10, 6.02e23, 1e0 and 1e10f alongside the existing signed-exponent cases.
Environment
Source-level parsing defect; platform and GPU independent.
- DiligentEngine
aca2285, DiligentCore bb821b7
- Reproduced on Linux with GCC, C++17
Summary
Parsing::SkipFloatNumber()inCommon/interface/ParsingTools.hpprequires the exponent of a float literal to carry an explicit+/-sign. Literals such as1e10,6.02e23and2.0E8— valid in C, C++, HLSL and GLSL — are therefore only partially consumed.Because
Parsing::Tokenize()(documented as tokenizing "using the C-language syntax") relies on this function, the HLSL tokenizer splits such a literal into a numeric constant plus a spurious identifier. This affectsHLSL2GLSLConverterImplandHLSLParsingTools, both of which tokenize user shader source.The public helper
CountFloatNumberChars()inCommon/interface/StringTools.hppreturns wrong results for the same inputs.Affected code
https://github.com/DiligentGraphics/DiligentCore/blob/master/Common/interface/ParsingTools.hpp#L364-L376
The first check treats a missing sign the same as a malformed exponent and bails out, returning the position saved before the
e. Per the C grammar the sign is optional (exponent-part: e sign_opt digit-sequence).Reproduction 1 —
SkipFloatNumber/CountFloatNumberChars1e1011E1011.5e101.52.0E82.01e10f16.02e236.021e011e+101e+10(OK)1e-101e-10(OK)Reproduction 2 — HLSL tokenizer
Using the engine's own
Parsing::HLSLTokenizer(Graphics/ShaderTools/src/HLSLTokenizer.cpp):Impact
The token stream handed to the HLSL→GLSL converter is corrupted for any shader using unsigned exponent notation. One concrete consequence:
HLSL2GLSLConverterImpl.cppstrips thef/Fsuffix from numeric constants specifically to avoid GLSL compiler warnings —https://github.com/DiligentGraphics/DiligentCore/blob/master/Graphics/HLSL2GLSLConverterLib/src/HLSL2GLSLConverterImpl.cpp#L4590-L4597
For
1e10fthe suffix ends up on the spuriousIdentifiertoken (e10f), not on theNumericConstant(1), so the stripping is silently bypassed and the warning this code exists to prevent reappears. More generally, injecting an unexpected identifier into the stream of a source-to-source translator is a latent hazard for the identifier-driven transforms around it.This has likely gone unnoticed because the notation is consistently avoided in-tree. Across the shaders shipped in DiligentCore, DiligentFX, DiligentSamples and DiligentTools there are 117 float literals using exponent notation and zero without an explicit sign — e.g.
float ClosestDistance = 1e+10;inDiligentFX/Hydrogent/shaders/HnUpdateClosestSelectedLocation.psh, where1e10would be the more natural spelling. User-authored shaders have no reason to follow that convention.The existing unit tests cover
-1e+2,+1e-3,-1e,-1e+ande5, but no unsigned-exponent case such as1e10, which is why the gap was not caught.Suggested fix
Make the sign optional and let the existing digit check reject genuinely malformed input:
I verified this patch locally:
6.02e23tokenizes as a singleNumericConstant);CountFloatNumberCharsassertions inTests/DiligentCoreTest/src/Common/StringToolsTest.cppstill pass, with 0 regressions.Malformed input is still rejected:
10e&,10e+x,-1e,-1e+ande5behave exactly as before, since the digit check that follows is unchanged.Worth adding test coverage for
1e10,1E10,1.5e10,6.02e23,1e0and1e10falongside the existing signed-exponent cases.Environment
Source-level parsing defect; platform and GPU independent.
aca2285, DiligentCorebb821b7