KhronosGroup / glslang

Khronos-reference front end for GLSL/ESSL, partial front end for HLSL, and a SPIR-V generator.
Other
3.05k stars 836 forks source link

Lots of simple parse errors don't have proper or confusing error messages #822

Closed Jasper-Bekkers closed 7 years ago

Jasper-Bekkers commented 7 years ago

This might turn out to be a bit of a grab-bag issue so if you want me to file separate issues for these I will but i thought it was easier to just group them all up. But I've noticed that a lot of time relatively simple compile / parse errors turn into relatively obscure error messages so I figured I'd report some of them.

Missing return type for main:

#version 450

main()
{

}

Yields: shaders/repro.glsl:3: error: '' : syntax error

Missing closing ):

#version 450

void main()
{
    vec2 decl = vec2(0, 0;
}

Yields shaders/repro.glsl:5: error: '' : syntax error

Missing argument to function

#version 450

bool function(float somearg)
{
    return true;
}

void main()
{
    if (function())
    {

    }
}

Yields:

shaders/repro.glsl:10: error: 'function' : no matching overloaded function found
shaders/repro.glsl:10: error: '' : boolean expression expected

Expected: missing argument 'somearg' for 'function'

Using type name as argument:

#version 450

bool function(float somearg)
{
    return true;
}

struct myStruct {};

void main()
{
    if (function(myStruct))
    {

    }
}

Yields shaders/repro.glsl:8: error: '' : syntax error

Using typename as variable name

#version 450

struct myStruct {};

void main()
{
    myStruct myStruct;
}

Yields shaders/repro.glsl:3: error: '' : syntax error

johnkslang commented 7 years ago

Switching to the recursive-decent parser for HLSL was nice; much easier to say things like "expected XXX".

For bison, I haven't found an easy way to keep the grammar matching the spec and avoid the "syntax error" it generates when the incoming token stream doesn't match a grammar rule.

Jasper-Bekkers commented 7 years ago

Could you see if adding %define parse.error verbose produces better messages? This potentially needs %define parse.lac full. I can't test it myself since updateGrammar seems to be a bash script.

johnkslang commented 7 years ago

Good suggestion.

I experimented with these, and it's a bit of a mixed bag. Especially when using %define parse.lac full, which bypasses some productions (sees the syntax error coming "early") and hence actually cuts out some useful messages the productions detected. However, without that, the extra information, while often wrong, still basically identifies where in the statement things broke down, so I think that part is a win.

johnkslang commented 7 years ago

Note that the struct myStruct {}; is an error, because the member list is empty, so those aren't about what they seemed.

johnkslang commented 7 years ago

Note that the struct myStruct {}; is an error, because the member list is empty, so those aren't about what they seemed.