From patch 3:
``` + if (!(technique->name = hlsl_strdup(ctx, name))) + { + vkd3d_free(technique); + return NULL; + } + + if (!(var = hlsl_new_var(ctx, name, ctx->builtin_types.Void, loc, NULL, 0, NULL))) + { + hlsl_free_technique(technique); + return NULL; + } ```
I'm inclined to think we should either make techniques variables, or make them non-variables, rather than doing both.
(Also, creating a variable but setting the type to "void" is probably going to result in some less-than-perfectly-clear error messages).
As a side note, I won't ask to change it, but patch 3/5 does two different things which I feel could have been split: it creates actual objects for techniques, and separately it implements effect group syntax.
From patch 5/5:
``` + if (technique_type == TECHNIQUE9) + { + skip = ctx->profile->major_version > 2; + } + else if (technique_type == TECHNIQUE10) + { + if (ctx->profile->type == VKD3D_SHADER_TYPE_EFFECT && ctx->profile->major_version == 2) + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_SYNTAX, + "The 'technique10' keyword is invalid for this profile."); + + skip = ctx->profile->major_version == 2; + } + else if (technique_type == TECHNIQUE11) + { + if (ctx->profile->type == VKD3D_SHADER_TYPE_EFFECT && ctx->profile->major_version == 2) + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_SYNTAX, + "The 'technique11' keyword is invalid for this profile."); + + skip = ctx->profile->major_version == 4; + } ```
This may be a casualty of introducing code that isn't actually used yet (which is itself not great), but it seems surprising that we're going to emit an hlsl_error() and also bother setting the skip field, especially when we aren't doing the same for the 11 variant.
Also, would it maybe be more idiomatic to just store the technique level, and implement this logic when we actually emit techniques? [Are we going to need to store that information anyway?]