Bug: Include Preprocessing reformats include file's source

[devshgraphicsprogramming]

From examining glslang/MachineIndependent/ShaderLang.cpp it seems apparent that the entire include file is parsed (obviously so the preprocessor directives can be honoured), but then the actual source produced from all the headers is de-parsed into a string of formatted tokens

This is a problem for me as I outline in this shaderc issue:
google/shaderc#1069

Long story short :

I have a peculiar method of disabling preprocessor directives and checks for GL_ extension defines until after the #includes have been collected. I replace all preprocessor directives except for #version and #include with _this_is_a_hash and some programmatically generated guards against multiple inclusion.

This is working for me quite well, but the only place where it breaks are:

• #defines that have a () after them, such as _this_is_a_hashdefine ONE_MORE_THAN_A (A+1)
• preprocessor function macros such, ex. _this_is_a_hashdefine FUNC(X) (X*X)

The cause of all this is because your PreprocessDeferred re-glues together the headers from tokens and the ( and ) symbols are on some sort of a list of tokens that are explicitly stated not to need a space before them.

            // Output a space in between tokens, but not at the start of a line,
            // and also not around special tokens. This helps with readability
            // and consistency.
            if (!isNewString && !isNewLine && lastToken != EndOfInput &&
                (unNeededSpaceTokens.find((char)token) == std::string::npos) &&
                (unNeededSpaceTokens.find((char)lastToken) == std::string::npos) &&
                (noSpaceBeforeTokens.find((char)token) == std::string::npos)) {
                outputBuffer += ' ';
            }

Is there any way to fix this behaviour?
could I remove ( from noSpaceBeforeTokens without affecting somethng else adversely?

[johnkslang]

Normal operation of glslang is to make one pass, simultaneously tokenizing, preprocessing, and parsing, all within a window of only a few tokens, passing through the file once.

A long time ago, Google partially added a feature that was similar to seeing what your code might look like if it was preprocessed but not actually compiled. That is a side-subject to actually correctly using glslang.

I'm not quite sure if your operating with the first paragraph above, or the second one. It could be you see the code for the second one, but that is just a distraction? Or, are you actually trying to have glslang output preprocessed code?

[devshgraphicsprogramming]

I'm using glslang via shaderc.

I make three passes:

• Resolve all include directives
• Prepend GPU specific defines (whether certain extensions are supported,etc.)
• Compile to SPIR-V

The first will actually substitute all instances of # (except for #version and #include) for _this_is_a_hash on the original source and any include the shaderc include handler asks for, plus add some header guards with proper # to allow for N (usually 5) instances of self inclusion.

After all includes have been collected, I swap any _this_is_a_hash that is on a newline preceeeded only by whitespace for #.

This gives me code that looks like an #include resolved but not preprocessed version of the source.
Then the engine's runtime definitions are added after the occurence of a #version

Finally SPIR-V is compiled.

[devshgraphicsprogramming]

An example of what I produce mid-way through pass nr. 1

#ifndef _GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_0
	#define _GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_0
#elif !defined(_GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_1)
	#define _GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_1
#elif !defined(_GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_2)
	#define _GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_2
#elif !defined(_GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_3)
	#define _GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_3
#elif !defined(_GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_4)
	#define _GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_4
#elif !defined(_GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_5)
	#define _GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_5
#endif

#ifndef _GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_5
_this_is_hash_ifndef _IRR_GLSL_EXT_LUMA_METER_COMMON_INCLUDED_
_this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_COMMON_INCLUDED_


_this_is_hash_ifndef _IRR_GLSL_EXT_LUMA_METER_INVOCATION_COUNT
_this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_INVOCATION_COUNT 256 // change this simultaneously with the constexpr in `CGLSLLumaBuiltinIncludeLoader`
_this_is_hash_endif


_this_is_hash_ifndef _IRR_GLSL_EXT_LUMA_METER_UNIFORMS_DEFINED_
_this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_UNIFORMS_DEFINED_
struct irr_glsl_ext_LumaMeter_Uniforms_t
{
	vec2 meteringWindowScale;
	vec2 meteringWindowOffset;
};
_this_is_hash_endif


_this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_MODE_GEOM_MEAN 0
_this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_MODE_MODE 1

_this_is_hash_if _IRR_GLSL_EXT_LUMA_METER_MODE_DEFINED_==_IRR_GLSL_EXT_LUMA_METER_MODE_MODE
    _this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_BIN_COUNT _IRR_GLSL_EXT_LUMA_METER_INVOCATION_COUNT

    _this_is_hash_ifdef _IRR_GLSL_EXT_LUMA_METER_FIRST_PASS_DEFINED_
    	_this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_PADDED_BIN_COUNT (_IRR_GLSL_EXT_LUMA_METER_BIN_COUNT+1)

	    _this_is_hash_ifndef _IRR_GLSL_EXT_LUMA_METER_LOCAL_REPLICATION_POW_DEFINED_
	    _this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_LOCAL_REPLICATION_POW_DEFINED_ 3
	    _this_is_hash_endif

	    _this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_LOCAL_REPLICATION (1<<_IRR_GLSL_EXT_LUMA_METER_LOCAL_REPLICATION_POW_DEFINED_)
	    _this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_SHARED_SIZE_NEEDED_ (_IRR_GLSL_EXT_LUMA_METER_PADDED_BIN_COUNT*_IRR_GLSL_EXT_LUMA_METER_LOCAL_REPLICATION)
    _this_is_hash_else
        _this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_SHARED_SIZE_NEEDED_ (_IRR_GLSL_EXT_LUMA_METER_BIN_COUNT*2)
    _this_is_hash_endif

    _this_is_hash_if (_IRR_GLSL_EXT_LUMA_METER_MAX_LUMA_DEFINED_-_IRR_GLSL_EXT_LUMA_METER_MIN_LUMA_DEFINED_)%%_IRR_GLSL_EXT_LUMA_METER_BIN_COUNT!=0
	    _this_is_hash_error "The number of bins must evenly divide the histogram range!"
    _this_is_hash_endif

    _this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_BIN_GLOBAL_REPLICATION 4 // change this simultaneously with the constexpr in `CGLSLLumaBuiltinIncludeLoader`
    _this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_BIN_GLOBAL_COUNT (_IRR_GLSL_EXT_LUMA_METER_BIN_COUNT*_IRR_GLSL_EXT_LUMA_METER_BIN_GLOBAL_REPLICATION)
    struct irr_glsl_ext_LumaMeter_output_t
    {
		uint packedHistogram[_IRR_GLSL_EXT_LUMA_METER_BIN_GLOBAL_COUNT];
    };
_this_is_hash_elif _IRR_GLSL_EXT_LUMA_METER_MODE_DEFINED_==_IRR_GLSL_EXT_LUMA_METER_MODE_GEOM_MEAN
    _this_is_hash_ifdef _IRR_GLSL_EXT_LUMA_METER_FIRST_PASS_DEFINED_
	    _this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_SHARED_SIZE_NEEDED_ _IRR_GLSL_EXT_LUMA_METER_INVOCATION_COUNT
    _this_is_hash_else
        _this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_SHARED_SIZE_NEEDED_ 0
    _this_is_hash_endif

    struct irr_glsl_ext_LumaMeter_output_t
    {
        uint unormAverage;
    };
_this_is_hash_else
_this_is_hash_error "Unsupported Metering Mode!"
_this_is_hash_endif



_this_is_hash_ifndef _IRR_GLSL_EXT_LUMA_METER_UNIFORMS_SET_DEFINED_
_this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_UNIFORMS_SET_DEFINED_ 0
_this_is_hash_endif

_this_is_hash_ifndef _IRR_GLSL_EXT_LUMA_METER_UNIFORMS_BINDING_DEFINED_
_this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_UNIFORMS_BINDING_DEFINED_ 0
_this_is_hash_endif


_this_is_hash_ifndef _IRR_GLSL_EXT_LUMA_METER_OUTPUT_SET_DEFINED_
_this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_OUTPUT_SET_DEFINED_ 0
_this_is_hash_endif

_this_is_hash_ifndef _IRR_GLSL_EXT_LUMA_METER_OUTPUT_BINDING_DEFINED_
_this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_OUTPUT_BINDING_DEFINED_ 1
_this_is_hash_endif


_this_is_hash_ifndef _IRR_GLSL_EXT_LUMA_METER_INPUT_IMAGE_SET_DEFINED_
_this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_INPUT_IMAGE_SET_DEFINED_ 0
_this_is_hash_endif

_this_is_hash_ifndef _IRR_GLSL_EXT_LUMA_METER_INPUT_IMAGE_BINDING_DEFINED_
_this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_INPUT_IMAGE_BINDING_DEFINED_ 2
_this_is_hash_endif


_this_is_hash_ifdef _IRR_GLSL_EXT_LUMA_METER_FIRST_PASS_DEFINED_

_this_is_hash_ifndef _IRR_GLSL_EXT_LUMA_METER_PUSH_CONSTANTS_DEFINED_
_this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_PUSH_CONSTANTS_DEFINED_
layout(push_constant) uniform PushConstants
{
	int currentFirstPassOutput;
} pc;
_this_is_hash_endif

_this_is_hash_ifndef _IRR_GLSL_EXT_LUMA_METER_OUTPUT_DESCRIPTOR_DEFINED_
_this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_OUTPUT_DESCRIPTOR_DEFINED_
layout(set=_IRR_GLSL_EXT_LUMA_METER_OUTPUT_SET_DEFINED_, binding=_IRR_GLSL_EXT_LUMA_METER_OUTPUT_BINDING_DEFINED_) restrict coherent buffer OutputBuffer
{
	irr_glsl_ext_LumaMeter_output_t outParams[];
};
_this_is_hash_endif

_this_is_hash_ifndef _IRR_GLSL_EXT_LUMA_METER_INPUT_IMAGE_DESCRIPTOR_DEFINED_
_this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_INPUT_IMAGE_DESCRIPTOR_DEFINED_
layout(set=_IRR_GLSL_EXT_LUMA_METER_INPUT_IMAGE_SET_DEFINED_, binding=_IRR_GLSL_EXT_LUMA_METER_INPUT_IMAGE_BINDING_DEFINED_) uniform sampler2DArray inputImage;
_this_is_hash_endif

_this_is_hash_endif


_this_is_hash_if _IRR_GLSL_EXT_LUMA_METER_SHARED_SIZE_NEEDED_>0 && !defined(_IRR_GLSL_SCRATCH_SHARED_DEFINED_)
_this_is_hash_define _IRR_GLSL_SCRATCH_SHARED_DEFINED_ histogram
shared uint _IRR_GLSL_SCRATCH_SHARED_DEFINED_[_IRR_GLSL_EXT_LUMA_METER_SHARED_SIZE_NEEDED_];
_this_is_hash_elif defined(_IRR_GLSL_SCRATCH_SHARED_SIZE_DEFINED_) && _IRR_GLSL_SCRATCH_SHARED_SIZE_DEFINED_<_IRR_GLSL_EXT_LUMA_METER_SHARED_SIZE_NEEDED_
    _this_is_hash_error "Not enough shared memory declared"
_this_is_hash_endif



_this_is_hash_if _IRR_GLSL_EXT_LUMA_METER_MODE_DEFINED_==_IRR_GLSL_EXT_LUMA_METER_MODE_MODE
    _this_is_hash_ifdef _IRR_GLSL_EXT_LUMA_METER_FIRST_PASS_DEFINED_
        void irr_glsl_ext_LumaMeter_clearFirstPassOutput()
        {
            uint globalIndex = gl_LocalInvocationIndex+gl_WorkGroupID.x*_IRR_GLSL_EXT_LUMA_METER_BIN_COUNT;
            if (globalIndex<_IRR_GLSL_EXT_LUMA_METER_BIN_GLOBAL_COUNT)
            {
    		    outParams[(pc.currentFirstPassOutput!=0 ? 0:textureSize(inputImage,0).z)+int(gl_WorkGroupID.z)].packedHistogram[globalIndex] = 0u;
            }
        }

        void irr_glsl_ext_LumaMeter_setFirstPassOutput(in uint writeOutVal)
        {
            uint globalIndex = gl_LocalInvocationIndex;
            globalIndex += (gl_WorkGroupID.x&uint(_IRR_GLSL_EXT_LUMA_METER_BIN_GLOBAL_REPLICATION-1))*_IRR_GLSL_EXT_LUMA_METER_BIN_COUNT;
		    atomicAdd(outParams[(pc.currentFirstPassOutput!=0 ? textureSize(inputImage,0).z:0)+int(gl_WorkGroupID.z)].packedHistogram[globalIndex],writeOutVal);
        }
    _this_is_hash_endif

    // TODO: move to `CGLSLScanBuiltinIncludeLoader` but clean that include up first and fix shaderc macro handling
    uint irr_glsl_workgroupExclusiveAdd(uint val)
    {
        //! Bad INEFFICIENT Kogge-Stone adder, don't implement this way!
        for (int pass=1; pass<_IRR_GLSL_EXT_LUMA_METER_BIN_COUNT; pass<<=1)
        {
            uint index = gl_LocalInvocationIndex+(pass&0x1)*_IRR_GLSL_EXT_LUMA_METER_BIN_COUNT;

            _IRR_GLSL_SCRATCH_SHARED_DEFINED_[index] = val;
            barrier();
            memoryBarrierShared();
            if (gl_LocalInvocationIndex>=pass)
                val += _IRR_GLSL_SCRATCH_SHARED_DEFINED_[index-pass];
        }
        barrier();
        memoryBarrierShared();
        return val;
    }

    // TODO: turn `upper_bound__minus_onePoT` into a macro `irr_glsl_parallel_upper_bound__minus_onePoT` and introduce lower_bound, and non-minus one variants
    _this_is_hash_if _IRR_GLSL_EXT_LUMA_METER_BIN_COUNT&(_IRR_GLSL_EXT_LUMA_METER_BIN_COUNT-1)
        _this_is_hash_error "Parallel Upper Bound requires the Histogram Bin Count to be PoT"
    _this_is_hash_endif
    int upper_bound__minus_onePoT(in uint val, int arrayLenPoT)
    {
        arrayLenPoT >>= 1;
        int ret = (val<_IRR_GLSL_SCRATCH_SHARED_DEFINED_[arrayLenPoT]) ? 0:arrayLenPoT;
        for (; arrayLenPoT>0; arrayLenPoT>>=1)
        {
            int right = ret+arrayLenPoT;
            ret = (val<_IRR_GLSL_SCRATCH_SHARED_DEFINED_[right]) ? 0:right;
        }
        return ret;
    }

    struct irr_glsl_ext_LumaMeter_PassInfo_t
    {
        uvec2 percentileRange; // (lowerPercentile,upperPercentile)
    };
    float irr_glsl_ext_LumaMeter_getMeasuredLumaLog2(in irr_glsl_ext_LumaMeter_output_t firstPassOutput, in irr_glsl_ext_LumaMeter_PassInfo_t info)
    {
        uint histogramVal = firstPassOutput.packedHistogram[gl_LocalInvocationIndex];
        for (int i=0; i<_IRR_GLSL_EXT_LUMA_METER_BIN_GLOBAL_REPLICATION; i++)
            histogramVal += firstPassOutput.packedHistogram[gl_LocalInvocationIndex+i*_IRR_GLSL_EXT_LUMA_METER_BIN_COUNT];

        // do the prefix sum stuff
        _IRR_GLSL_SCRATCH_SHARED_DEFINED_[gl_LocalInvocationIndex] = irr_glsl_workgroupExclusiveAdd(histogramVal);
        barrier();
        memoryBarrierShared();

        float foundPercentiles[2];
        bool lower2Threads = gl_LocalInvocationIndex<2u;
        if (lower2Threads)
        {
            int found = upper_bound__minus_onePoT(percentileRange[gl_LocalInvocationIndex],_IRR_GLSL_EXT_LUMA_METER_BIN_COUNT);

            float foundValue = float(found)/float(_IRR_GLSL_EXT_LUMA_METER_BIN_COUNT);
            _IRR_GLSL_SCRATCH_SHARED_DEFINED_[gl_LocalInvocationIndex] = floatBitsToUint(foundValue);
        }
        barrier();
        memoryBarrierShared();

        return (uintBitsToFloat(_IRR_GLSL_SCRATCH_SHARED_DEFINED_[0])+uintBitsToFloat(_IRR_GLSL_SCRATCH_SHARED_DEFINED_[1]))*0.5;
    }
    _this_is_hash_undef _IRR_GLSL_EXT_LUMA_METER_BIN_GLOBAL_REPLICATION
_this_is_hash_elif _IRR_GLSL_EXT_LUMA_METER_MODE_DEFINED_==_IRR_GLSL_EXT_LUMA_METER_MODE_GEOM_MEAN
    _this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_GEOM_MEAN_MAX_WG_INCREMENT 0x1000u

    _this_is_hash_ifdef _IRR_GLSL_EXT_LUMA_METER_FIRST_PASS_DEFINED_
        void irr_glsl_ext_LumaMeter_clearFirstPassOutput()
        {
		    if (all(equal(uvec2(0,0),gl_GlobalInvocationID.xy)))
		        outParams[(pc.currentFirstPassOutput!=0 ? 0:textureSize(inputImage,0).z)+int(gl_WorkGroupID.z)].unormAverage = 0u;
        }

        void irr_glsl_ext_LumaMeter_setFirstPassOutput(in float writeOutVal)
        {
		    if (gl_LocalInvocationIndex==0u)
		    {
			    float normalizedAvg = writeOutVal/float(_IRR_GLSL_EXT_LUMA_METER_INVOCATION_COUNT);
			    uint incrementor = uint(float(_IRR_GLSL_EXT_LUMA_METER_GEOM_MEAN_MAX_WG_INCREMENT)*normalizedAvg+0.5);
			    atomicAdd(outParams[(pc.currentFirstPassOutput!=0 ? textureSize(inputImage,0).z:0)+int(gl_WorkGroupID.z)].unormAverage,incrementor);
		    }
        }
    _this_is_hash_endif

    struct irr_glsl_ext_LumaMeter_PassInfo_t
    {
        float rcpFirstPassWGCount;
    };
    float irr_glsl_ext_LumaMeter_impl_getMeasuredLumaLog2(in irr_glsl_ext_LumaMeter_output_t firstPassOutput, in irr_glsl_ext_LumaMeter_PassInfo_t info)
    {
        return float(firstPassOutput.unormAverage)*info.rcpFirstPassWGCount/float(_IRR_GLSL_EXT_LUMA_METER_GEOM_MEAN_MAX_WG_INCREMENT);
    }
    _this_is_hash_undef _IRR_GLSL_EXT_LUMA_METER_GEOM_MEAN_MAX_WG_INCREMENT
_this_is_hash_endif


float irr_glsl_ext_LumaMeter_getMeasuredLumaLog2(in irr_glsl_ext_LumaMeter_output_t firstPassOutput, in irr_glsl_ext_LumaMeter_PassInfo_t info)
{
    const float MinLuma = intBitsToFloat(_IRR_GLSL_EXT_LUMA_METER_MIN_LUMA_DEFINED_);
    const float MaxLuma = intBitsToFloat(_IRR_GLSL_EXT_LUMA_METER_MAX_LUMA_DEFINED_);
    return irr_glsl_ext_LumaMeter_impl_getMeasuredLumaLog2(firstPassOutput,info)*log2(MaxLuma/MinLuma)+log2(MinLuma);
}


float irr_glsl_ext_LumaMeter_getOptiXIntensity(in float measuredLumaLog2)
{
    return exp2(log(0.18)-measuredLumaLog2);
}


_this_is_hash_endif

#endif

#ifdef _GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_5
	#undef _GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_5
#elif defined(_GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_4)
	#undef _GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_4
#elif defined(_GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_3)
	#undef _GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_3
#elif defined(_GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_2)
	#undef _GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_2
#elif defined(_GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_1)
	#undef _GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_1
#elif defined(_GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_0)
	#undef _GENERATED_INCLUDE_GUARD_irr_builtin_glsl_ext_LumaMeter_common_glsl_0
#endif

[devshgraphicsprogramming]

A worked example

In the original header I have:

#define _IRR_GLSL_EXT_LUMA_METER_PADDED_BIN_COUNT (_IRR_GLSL_EXT_LUMA_METER_BIN_COUNT+1)

which I replace with (while I use shaderc/glslang to resolve includes)

this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_PADDED_BIN_COUNT (_IRR_GLSL_EXT_LUMA_METER_BIN_COUNT+1)

after glslang is done collecting the headers (parsing and retokenizing)

this_is_hash_define _IRR_GLSL_EXT_LUMA_METER_PADDED_BIN_COUNT(_IRR_GLSL_EXT_LUMA_METER_BIN_COUNT+1)

after my function exits ( I swap the # back)

#define _IRR_GLSL_EXT_LUMA_METER_PADDED_BIN_COUNT(_IRR_GLSL_EXT_LUMA_METER_BIN_COUNT+1)

and I'm missing the space in front of the (

[johnkslang]

Glslang does not officially support using the preprocessor without also compiling. The path that tries to recreate the source indeed is not a high-quality proven path, and perhaps should be disabled, but some people found it useful and wanted to retain it.

I'm unclear on if there is an issue here with shaderc or with glslang.

[devshgraphicsprogramming]

but some people found it useful and wanted to retain it.

Its extremely useful, that way I don;t need to compile a shader for every single possible combination of extensions/runtime features.

I'm unclear on if there is an issue here with shaderc or with glslang.

Me too, shaderc exposes such a path, but its glslang that breaks my "space in front of the ("

[devshgraphicsprogramming]

I managed to fix it somewhat (at least for the non-function like defines)
https://github.com/devshgraphicsprogramming/glslang/blob/ea6eeaed51aff35eac3ef1b27b586ebc7fbf43eb/glslang/MachineIndependent/ShaderLang.cpp#L1082

What would be great is if I could somehow detect if the current token has had a whitespace before it in the original and then decide whether to emit a space before a (

[devshgraphicsprogramming]

@johnkslang what's the reason for making the space field in the token false when expanding a macro?

glslang/glslang/MachineIndependent/preprocessor/Pp.cpp

Line 1183 in 343d4f6

ppToken->space = false;

tokenize is losing the space info on undefined macros

[devshgraphicsprogramming]

This fixes my issues entirely
https://github.com/KhronosGroup/glslang/compare/master...devshgraphicsprogramming:Fix2233?expand=1

Both use cases of macros that get the beginning # replaced by a temporary _this_is_a_hash now work for me:

• Function-like macros
• Macros that are wrapped by () to compensate for operator precedence

Is this a kind of code-change that could be merged or do I break something greatly?

[dneto0]

Glslang does not officially support using the preprocessor without also compiling. The path that tries to recreate the source indeed is not a high-quality proven path, and perhaps should be disabled, but some people found it useful and wanted to retain it.

I'm responsible for the preprocessing-only path.
It was done by my team some years ago so I'll have to take time to learn the details again. I don't think we anticipated your use case.

[devshgraphicsprogramming]

@dneto0 I managed to fix it for myself

https://github.com/KhronosGroup/glslang/compare/master...devshgraphicsprogramming:Fix2233?expand=1

question is can you upstream such a fix?

[dneto0]

I've been experimenting with your proposed change.

There is a problem with saving and restoring the "save" state in the MacroExpand method.
It causes the test for Test/cppMerge.frag to fail with a compilation error.

Here is the delta from running the glslang-gtest suite:

[ RUN      ] Glsl/CompileToAstTest.FromFile/cppMerge_frag
/build/shaderc/shaderc/third_party/glslang/gtests/TestFixture.h:148: Failure
Expected equality of these values:
  expected
    Which is: "cppMerge.frag\nShader version: 450\n0:? Sequence\n0:22  Function Definition: main( ( global void)\n0:22    Function Parameters: \n0:?   Linker Objects\n0:?     'dest1' (layout( set=0 binding=0) writeonly uniform image1D)\n0:?     'dest2' (layout( set=0 binding=0) writeonly uniform image1D)\n0:?     'dest3' (layout( set=0 binding=0) writeonly uniform image1D)\n\n\nLinked fragment stage:\n\n\nShader version: 450\n0:? Sequence\n0:22  Function Definition: main( ( global void)\n0:22    Function Parameters: \n0:?   Linker Objects\n0:?     'dest1' (layout( set=0 binding=0) writeonly uniform image1D)\n0:?     'dest2' (layout( set=0 binding=0) writeonly uniform image1D)\n0:?     'dest3' (layout( set=0 binding=0) writeonly uniform image1D)\n\n"
  real
    Which is: "cppMerge.frag\nERROR: 0:20: '' :  syntax error, unexpected IDENTIFIER, expecting LEFT_BRACE or COMMA or SEMICOLON\nERROR: 1 compilation errors.  No code generated.\n\n\nShader version: 450\nERROR: node is still EOpNull!\n0:?   Linker Objects\n0:?     'dest1' (layout( set=0 binding=0) writeonly uniform image1D)\n0:?     'dest2' (layout( set=0 binding=0) writeonly uniform image1D)\n\n\nLinked fragment stage:\n\nERROR: Linking fragment stage: Missing entry point: Each stage requires one entry point\n\nShader version: 450\nERROR: node is still EOpNull!\n0:?   Linker Objects\n0:?     'dest1' (layout( set=0 binding=0) writeonly uniform image1D)\n0:?     'dest2' (layout( set=0 binding=0) writeonly uniform image1D)\n\n"
With diff:
@@ -1,11 +1,12 @@
 cppMerge.frag
-Shader version: 450
-0:? Sequence
-0:22  Function Definition: main( ( global void)
-0:22    Function Parameters: 
+ERROR: 0:20: '' :  syntax error, unexpected IDENTIFIER, expecting LEFT_BRACE or COMMA or SEMICOLON
+ERROR: 1 compilation errors.  No code generated.
+
+
+Shader version: 450
+ERROR: node is still EOpNull!
 0:?   Linker Objects
 0:?     'dest1' (layout( set=0 binding=0) writeonly uniform image1D)
 0:?     'dest2' (layout( set=0 binding=0) writeonly uniform image1D)
-0:?     'dest3' (layout( set=0 binding=0) writeonly uniform image1D)
 

Somehow that state change is an invalid thing to do.

I'm looking further.

[dneto0]

Focusing just on the change in ShaderLang.cpp, every variation I try does the wrong thing to the following sample:

#version 450

#extension GL_KHR_memory_scope_semantics : require

_this_is_define Y (a+b)
_this_is_define Y2(a,b) (a/b)
#define X (a+b)
#define X2(a,b) (a+b)

shared float w;

void main    () {
 X
 w = X2(1.0,2.0);
}

Here's one variation of the output, which is wrong:

#version 450

#extension GL_KHR_memory_scope_semantics : require

_this_is_define Y (a + b )
_this_is_define Y2 (a, b )(a / b )



shared float w;

void main (){
  (a + b )
 w = (1.0 + 2.0 );
}

This is wrong because on the Y2 macro definition, the parameter list is no longer connected to the macro definition but is glued to the expansion.

Here is another, which is wrong in a different way:

#version 450

#extension GL_KHR_memory_scope_semantics : require

_this_is_define Y(a + b)
_this_is_define Y2(a, b)(a / b)



shared float w;

void main(){
  (a + b)
 w =(1.0 + 2.0);
}

This one is wrong because in the Y2 macro, the parameter list is next to the Y2, but still also glued to the expansion.

I think your algorithm for replacing the #define with _this_is_define needs to distinguish the two cases, and produce different outputs, e.g. this_is_define_without_params and this_is_define_with_params. Then you can re-insert the space that the preprocessor expansion removes.

Thanks again for your patience in waiting for this reply.

[devshgraphicsprogramming]

The spaces

Actually my change produces none of the variations of the output you've listed. I've ran a simple shader through my engine and a debugger.

Input is

#define Y (a+b)
#define Y2(a,b) (a/b)

My engine swaps to produce

_this_is_hash_define Y (a+b)
_this_is_hash_define Y2(a,b) (a/b)

The glslang preprocessor with my patch produces

_this_is_hash_define Y (a + b)
_this_is_hash_define Y2(a, b)(a / b)

My engine swaps back to produce

#define Y (a + b)
#define Y2(a, b)(a / b)

This one is wrong because in the Y2 macro, the parameter list is next to the Y2, but still also glued to the expansion.

It might be wrong "visually" but its still a valid function macro that your compiler compiles properly (or any C compiler).

The failed test case

There is a problem with saving and restoring the "save" state in the MacroExpand method.
It causes the test for Test/cppMerge.frag to fail with a compilation error.

To do my change I needed to know if a whitespace preceeded the current ( or ) token, you had that functionality in the ppToken but for the MacroExpand function clears it. As I noted here.

@johnkslang what's the reason for making the space field in the token false when expanding a macro?

glslang/glslang/MachineIndependent/preprocessor/Pp.cpp

Line 1183 in 343d4f6

ppToken->space = false;

tokenize is losing the space info on undefined macros

I kind-of don't understand whats going on in that function fully... but we will of course look into what's going on.