Sir Anthony <anth...@adsorbtion.org> writes: > 1. Change locations setup in glsl_parser.yy from yylloc to appropriate token > locations. > 2. Addition of two fields in ast_node location to hold end position of token. > 3. Addition of ast_node method to setup range locations (for aggregate > tokens). > 4. Fix for glcpp-lex.l. It handled spaces wrong and convert two > adjacent spaces into one, which added location offset for shaders with > indentation.
Hello, Sir. Thanks for contributing your patch! You've described 4 different changes in a single patch. Could you please split this up into 2 or more patches? Ideally each patch would include a single change, (and perhaps depend on previous changes). This would make the patches easier to review, and improve our ability to analyze the commit history in the future, (such as with "git bisect"). For my part, I haven't done much in the glsl_lexer.ll nor glsl_parser.yy files, but I'm one of the primary authors of the glcpp-lex.l code. So I would like to review the changes there independently from the rest of the patch. And on that point, I'm a bit confused by the patch to glcpp-lex.l Your commit message says it "handled spaces wrong and convert two adjacent spaces into one" so I expected to see the patch changing something about the conversion of adjacent spaces, (but I don't see anything like that). Or did you mean just that it computed location incorrectly in the case of collapsing adjacent spaces? Also, for glcpp, we have a collection of small tests in the glcpp/tests directory. These tests are invoked by "make check". If you are changing the behavior of glcpp, then you should be updating any tests where the desired output is changed. And if you are fixing a bug, but no existing test changes its output, then we need to be adding a new test to cover that case. Thanks again, -Carl -- carl.d.wo...@intel.com
pgpKgL0PJwZ1S.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
