Skip to content

Commit

Permalink
Merge pull request #32 from ShaderFrog/macro-function-bug
Browse files Browse the repository at this point in the history
Function macro expansion bug when multi argument macro arg has same name as user input
  • Loading branch information
AndrewRayCode authored Jul 29, 2024
2 parents 62017d8 + a0ca973 commit dccca4e
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 11 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"engines": {
"node": ">=16"
},
"version": "5.0.0",
"version": "5.1.0",
"type": "module",
"description": "A GLSL ES 1.0 and 3.0 parser and preprocessor that can preserve whitespace and comments",
"scripts": {
Expand Down
21 changes: 21 additions & 0 deletions src/preprocessor/preprocessor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,27 @@ foo`;
foo`);
});

test(`function macro where source variable is same as macro argument`, () => {
const program = `
#define FN(x, y) x + y
FN(y, x);
FN(y.y, x.x);
FN(yy, xx);
`;

const ast = parse(program);
preprocessAst(ast);

// Ensure that if the argument passed to the fn FN(X) has the
// same name as the macro definition #define FN(X), it doesn't get expanded
// https://github.com/ShaderFrog/glsl-parser/issues/31
expect(generate(ast)).toBe(`
y + x;
y.y + x.x;
yy + xx;
`);
});

test("macro that isn't macro function call call is expanded", () => {
const program = `
#define foo () yes expand
Expand Down
34 changes: 24 additions & 10 deletions src/preprocessor/preprocessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,17 +198,31 @@ const expandFunctionMacro = (
throw new Error(`'${macroName}': Not enough arguments for macro`);
}

// Collect the macro identifiers and build a replacement map from those to
// the user defined replacements
const argIdentifiers = macroArgs.map(
(a) => (a as PreprocessorIdentifierNode).identifier
);
const argKeys = argIdentifiers.reduce<Record<string, string>>(
(acc, identifier, index) => ({
...acc,
[identifier]: args[index].trim(),
}),
{}
);

const replacedBody = tokenPaste(
macroArgs.reduce(
(replaced, macroArg, index) =>
replaced.replace(
new RegExp(
`\\b${(macroArg as PreprocessorIdentifierNode).identifier}\\b`,
'g'
),
args[index].trim()
),
macro.body
macro.body.replace(
// Replace all instances of macro arguments in the macro definition
// (the arg separated by word boundaries) with its user defined
// replacement. This one-pass strategy ensures that we won't clobber
// previous replacements when the user supplied args have the same names
// as the macro arguments
new RegExp(
'(' + argIdentifiers.map((a) => `\\b${a}\\b`).join(`|`) + ')',
'g'
),
(match) => (match in argKeys ? argKeys[match] : match)
)
);

Expand Down

0 comments on commit dccca4e

Please sign in to comment.