Skip to content

Commit c8a070b

Browse files
yorkiealixinne
andauthored
fix(glsl-lang-pp): eval and expand #elif only if the last is none (#61)
This pull request refines the handling of conditional preprocessing directives in the macro expansion logic, specifically improving how `elif` directives are evaluated based on the current state of the `if` stack. For example: ```glsl #version 300 es #define CS1 #define CS2 #define CS3 vec3 test() { #if defined(CS1) return vec3(1.0, 0.0, 0.0); #elif defined(CS2) return vec3(2.0, 0.0, 0.0); #elif defined(CS3) return vec3(3.0, 1.0, 0.0); #else return vec3(0.0, 0.0, 1.0); #endif } ``` Without this patch, it will be expanded to: ```glsl #version 300 es vec3 test() { return vec3(1.0, 0.0, 0.0); return vec3(2.0, 0.0, 0.0); return vec3(3.0, 1.0, 0.0); #endif } ``` --------- Co-authored-by: Alixinne <alixinne@pm.me>
1 parent 962911b commit c8a070b

File tree

5 files changed

+88
-1
lines changed

5 files changed

+88
-1
lines changed

.github/workflows/build.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,9 @@ jobs:
110110
env:
111111
IGNORE_TESTS: ${{ matrix.ignore_tests }}
112112

113+
- name: Run glsl-lang-pp/additional tests
114+
run: cargo test -p glsl-lang-pp --test additional --features full
115+
113116
- name: Run glsl-lang-pp/glslang tests
114117
run: cargo test -p glsl-lang-pp --test glslang --features full
115118

lang-pp/src/processor/expand.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,12 @@ impl ExpandOne {
417417
errors.push(ProcessingErrorKind::DirectiveElif(error));
418418
}
419419

420-
value
420+
if !self.if_stack.none() {
421+
// Always false if the last if state was activated once
422+
false
423+
} else {
424+
value
425+
}
421426
} else {
422427
// Do not evaluate if the group is not active
423428
true

lang-pp/src/processor/expand/if_stack.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ impl IfState {
2020
}
2121
}
2222

23+
pub fn none(&self) -> bool {
24+
matches!(self, Self::None)
25+
}
26+
2327
pub fn active(&self) -> bool {
2428
matches!(self, Self::Active { .. })
2529
}
@@ -95,6 +99,10 @@ impl IfStack {
9599
self.stack.last().map(|top| top.active()).unwrap_or(true)
96100
}
97101

102+
pub fn none(&self) -> bool {
103+
self.stack.last().map(|top| top.none()).unwrap_or(false)
104+
}
105+
98106
pub fn on_if_like(&mut self, expr: bool) {
99107
if self.active() && expr {
100108
self.stack.push(IfState::Active { else_seen: false });

lang-pp/tests/additional.rs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
#![cfg(feature = "full")]
2+
3+
use std::path::PathBuf;
4+
5+
use glsl_lang_pp::{
6+
exts::DEFAULT_REGISTRY,
7+
last::{Event, TokenState},
8+
types::Token,
9+
};
10+
11+
#[test]
12+
fn test_issue_61() {
13+
// See https://github.com/alixinne/glsl-lang/pull/61
14+
let path: PathBuf = "../lang-pp/tests/issue_61.glsl".into();
15+
let mut pp = glsl_lang_pp::processor::fs::StdProcessor::default();
16+
17+
let parsed = match pp.parse(&path) {
18+
Ok(inner) => Ok(inner),
19+
Err(err) => {
20+
if err.kind() == std::io::ErrorKind::InvalidData {
21+
std::fs::read(&path).map(|value| {
22+
pp.parse_source(
23+
encoding_rs::WINDOWS_1252.decode(&value).0.as_ref(),
24+
path.parent().unwrap(),
25+
)
26+
})
27+
} else {
28+
Err(err)
29+
}
30+
}
31+
}
32+
.expect("failed to open file");
33+
34+
assert_eq!(
35+
parsed
36+
.into_iter()
37+
.tokenize(300, false, &DEFAULT_REGISTRY)
38+
.filter_map(|token| {
39+
// Extract float constants which are not excluded by the preprocessor
40+
if let Ok(Event::Token {
41+
state: TokenState::Active,
42+
token_kind: Token::FLOAT_CONST(val),
43+
..
44+
}) = token
45+
{
46+
return Some(val);
47+
}
48+
49+
return None;
50+
})
51+
.collect::<Vec<_>>(),
52+
// Only one pp branch should be active, so only one constant is produced
53+
vec![1.0]
54+
);
55+
}

lang-pp/tests/issue_61.glsl

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
#version 300 es
2+
#define CS1
3+
#define CS2
4+
#define CS3
5+
6+
vec3 test() {
7+
#if defined(CS1)
8+
return 1.0;
9+
#elif defined(CS2)
10+
return 2.0;
11+
#elif defined(CS3)
12+
return 3.0;
13+
#else
14+
return 4.0;
15+
#endif
16+
}

0 commit comments

Comments
 (0)