[Mesa-dev] Preprocessor patches

Carl Worth cworth at cworth.org
Wed Aug 6 17:03:59 PDT 2014


Ian Romanick <idr at freedesktop.org> writes:
> Are the preprocessor patches on the gles3conform-v4 branch of my fd.o
> repo the most up to date?

Yes. The state of those patches matches what I have on my branch, (at
least as far as GLES3 conformance is concerned---I have some subsequent
preprocessor patches which shouldn't affect conformance).

> I've put R-b on almost all of them.

Thanks!

> I want to look a little closer at "glsl/glcpp: Fix for macros that
> expand to include "defined" operators".

That makes sense. It's one of the meatiest patches in the series. So
I'll appreciate any additional review here. Fortunately, there is a fairly
complete commit message to guide the review. (Though, while you're
looking you might fix the mixed space-vs.-tab indentation in the commit
message.)

> I'm not sure about "glsl/glcpp: Fix line-continuation code to handle
> multiple newline flavors".

Well, it certainly shouldn't be any worse than what we had before.

I imagine what looks scary is the comment that says:

	In this code, we explicitly do not support a shader that mixes
	different kinds of newlines.

Prior to this commit, the corresponding comment that should be in the
code is:

	In this code, we explicitly do not support a shader that uses
	anything except for a single linefeed as the line terminator.

And the testing in the patch series verifies that things do only improve
with this patch.

> A lot of shaders that we encounter are a combination of hand-written
> and run-time, machine generated.  If the hand-writen parts use cr-lf
> and the machine generated parts use sprintf with \n, we'll get a mix
> of line endings.  Any idea what GCC does?

Yes, I can see how a file might reasonably end up with mixed line
terminators. And this is something that's not currently handled (as far
as the line continuation is concerned---I think the rest of glcpp does
do better with mixed line terminators by the end of the series).

I just checked, and gcc does do a good job here. It currently handles
line continuations with any of LF, CR, or CR+LF all in the same file,
(in my test the first line encountered is terminated with LF). GCC does
botch the LF+CR line continuation. Obviously, that's not that
interesting a case since no current, mainstream OS uses this line
terminator, (but as worded, the GLSL specification does allow it).

Meanwhile, with the same test file, glcpp gets the LF line continuation
right, leaves the CR and CR+LF line continuations untouched, and for the
LF+CR continuation it removes the '\' but doesn't correctly fold the
lines.

I've attached the source file I used for testing as well as the output
From gcc and glcpp.

It probably wouldn't be too hard to fix this code to be more general. I
might take a whack at that now that I have this test in hand.

-Carl

-------------- next part --------------
A non-text attachment was scrubbed...
Name: mixed-newlines
Type: application/octet-stream
Size: 222 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140806/f81b8c62/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mixed-newlines-gcc
Type: application/octet-stream
Size: 364 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140806/f81b8c62/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mixed-newlines-glcpp
Type: application/octet-stream
Size: 211 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140806/f81b8c62/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140806/f81b8c62/attachment.sig>


More information about the mesa-dev mailing list