Skip to content

Commit 243e32a

Browse files
committed
fix(c): ensure we can compile on MSVC in pre-C23 mode
We can't do things like this: static const VkBufferUsageFlagBits2 VK_BUFFER_USAGE_2_TRANSFER_SRC_BIT_KHR = VK_BUFFER_USAGE_2_TRANSFER_SRC_BIT; Because to MSVC, that's not a compile-time constant. Annoying, but we can follow the pattern used by vulkan_core.h which is to just use the value literal. Signed-off-by: Steven Noonan <steven@uplinklabs.net>
1 parent 24b0985 commit 243e32a

10 files changed

Lines changed: 236 additions & 160 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,5 +27,6 @@ xxhash-rust = { version = "0.8", features = ["xxh3"] }
2727

2828
[dev-dependencies]
2929
assert_cmd = "2"
30+
cc = "1"
3031
predicates = "3"
3132
tempfile = "3"

build.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,13 @@ fn main() {
230230
pkg_version.clone()
231231
};
232232

233+
// Expose the target triple to integration tests via env!("TARGET").
234+
// The cc crate needs this to find the correct compiler.
235+
println!(
236+
"cargo:rustc-env=TARGET={}",
237+
env::var("TARGET").unwrap()
238+
);
239+
233240
let out_dir = PathBuf::from(env::var("OUT_DIR").unwrap());
234241
let dest = out_dir.join("build_info.rs");
235242

src/generator/c/mod.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ fn build_env() -> Result<Environment<'static>> {
202202
env.add_filter("spec_display", filter_spec_display);
203203
env.add_filter("c_ident", filter_c_ident);
204204
env.add_filter("vk_max_enum_name", filter_enum_max_name);
205+
env.add_filter("ull", filter_ull);
205206

206207
Ok(env)
207208
}
@@ -244,6 +245,22 @@ fn filter_c_ident(value: Value) -> String {
244245
}
245246
}
246247

248+
/// Append `ULL` suffix to a value if it is a numeric literal (decimal, hex,
249+
/// or negative). Alias references (identifiers) are left unchanged.
250+
/// Used for 64-bit enum constants in the pre-C23 `#define` path.
251+
fn filter_ull(value: Value) -> String {
252+
let s = value.as_str().unwrap_or("");
253+
let trimmed = s.strip_prefix('-').unwrap_or(s);
254+
let is_numeric = trimmed.starts_with(|c: char| c.is_ascii_digit())
255+
|| trimmed.starts_with("0x")
256+
|| trimmed.starts_with("0X");
257+
if is_numeric {
258+
format!("{s}ULL")
259+
} else {
260+
s.to_string()
261+
}
262+
}
263+
247264
/// Used to build public function names like `gloamLoadGLES2Context`.
248265
fn filter_spec_display(value: Value) -> String {
249266
match value.as_str().unwrap_or("") {
@@ -402,4 +419,33 @@ mod tests {
402419
assert_eq!(filter_hex4(Value::from(0x0100_i64)), "0x0100");
403420
assert_eq!(filter_hex4(Value::from(0_i64)), "0x0000");
404421
}
422+
423+
// ---- filter_ull ----
424+
425+
fn ull(s: &str) -> String {
426+
filter_ull(Value::from(s))
427+
}
428+
429+
#[test]
430+
fn ull_appends_to_decimal() {
431+
assert_eq!(ull("0"), "0ULL");
432+
assert_eq!(ull("42"), "42ULL");
433+
}
434+
435+
#[test]
436+
fn ull_appends_to_hex() {
437+
assert_eq!(ull("0x0000000000000001"), "0x0000000000000001ULL");
438+
assert_eq!(ull("0X1F"), "0X1FULL");
439+
}
440+
441+
#[test]
442+
fn ull_appends_to_negative() {
443+
assert_eq!(ull("-1"), "-1ULL");
444+
}
445+
446+
#[test]
447+
fn ull_leaves_identifier_unchanged() {
448+
assert_eq!(ull("VK_PIPELINE_STAGE_2_NONE"), "VK_PIPELINE_STAGE_2_NONE");
449+
assert_eq!(ull("VK_ACCESS_2_NONE_KHR"), "VK_ACCESS_2_NONE_KHR");
450+
}
405451
}

src/generator/c/templates/header.h.j2

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,14 @@
5151
# define GLOAM_PLATFORM_LINUX 1
5252
#endif
5353
#endif /* GLOAM_PLATFORM_DETECTED_ */
54+
55+
#ifndef GLOAM_HAS_ENUM_BASE_TYPE
56+
#if defined(__cplusplus) || (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L)
57+
# define GLOAM_HAS_ENUM_BASE_TYPE 1
58+
#else
59+
# define GLOAM_HAS_ENUM_BASE_TYPE 0
60+
#endif
61+
#endif
5462
{% if fs.spec_name == "glx" %}
5563
#ifdef GLOAM_PLATFORM_LINUX
5664

@@ -177,12 +185,8 @@ struct _cl_event;
177185
/* ---- Vulkan enum groups -------------------------------------------------- */
178186
{% for group in fs.enum_groups %}
179187
{%- if group.bitwidth == 64 %}
180-
/* {{ group.name }} — 64-bit bitmask.
181-
C++/C23: emitted as an enum with explicit underlying type so all values
182-
are strongly typed and the tag name is valid for typedef aliases.
183-
Pre-C23 C: emitted as typedef uint64_t + static const, since C99/C11 do
184-
not support fixed underlying types for enums. */
185-
#if defined(__cplusplus) || (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L)
188+
/* {{ group.name }} — 64-bit; uses enum : uint64_t in C++/C23, static const otherwise. */
189+
#if GLOAM_HAS_ENUM_BASE_TYPE
186190
typedef enum {{ group.name }} : uint64_t {
187191
{%- for val in group.values %}
188192
{{ val.name }} = {{ val.value }}{% if not loop.last %},{% endif %}{% if val.comment %} /* {{ val.comment }} */{% endif %}
@@ -191,7 +195,7 @@ typedef enum {{ group.name }} : uint64_t {
191195
#else
192196
typedef uint64_t {{ group.name }};
193197
{%- for val in group.values %}
194-
static const {{ group.name }} {{ val.name | rjust(48) }} = {{ val.value }};{% if val.comment %} /* {{ val.comment }} */{% endif %}
198+
static const {{ group.name }} {{ val.name | rjust(48) }} = {{ val.literal_value | ull }};{% if val.comment %} /* {{ val.comment }} */{% endif %}
195199
{%- endfor %}
196200
#endif
197201

src/resolve/enums.rs

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ pub(super) fn build_flat_enums(
3535
.unwrap_or_default();
3636
Some(FlatEnum {
3737
name: e.name.clone(),
38+
literal_value: value.to_string(),
3839
value: value.to_string(),
3940
comment: e.comment.clone(),
4041
protect,
@@ -78,22 +79,39 @@ pub(super) fn build_enum_groups(raw: &RawSpec) -> Vec<EnumGroup> {
7879
Some(FlatEnum {
7980
name: v.name.clone(),
8081
value: val.to_string(),
82+
literal_value: String::new(), // resolved below
8183
comment: v.comment.clone(),
8284
protect: vec![],
8385
})
8486
})
8587
.collect();
8688

89+
let mut sorted = sort_enum_values(raw_values);
90+
resolve_literal_values(&mut sorted);
91+
8792
EnumGroup {
8893
name: g.name.clone(),
8994
is_bitmask: false,
9095
bitwidth: g.bitwidth.unwrap_or(32),
91-
values: sort_enum_values(raw_values),
96+
values: sorted,
9297
}
9398
})
9499
.collect()
95100
}
96101

102+
/// Fill `literal_value` for each entry. For numeric literals, it's the value
103+
/// itself. For aliases (value is another enum name), look up the target's
104+
/// `literal_value`. Assumes topological order (canonical before alias).
105+
fn resolve_literal_values(values: &mut [FlatEnum]) {
106+
// Build a name→literal map as we go (topo order guarantees targets are resolved first).
107+
let mut literals: HashMap<String, String> = HashMap::new();
108+
for v in values.iter_mut() {
109+
let resolved = literals.get(&v.value).cloned().unwrap_or_else(|| v.value.clone());
110+
v.literal_value = resolved.clone();
111+
literals.insert(v.name.clone(), resolved);
112+
}
113+
}
114+
97115
// ---------------------------------------------------------------------------
98116
// Value-dependency sort
99117
// ---------------------------------------------------------------------------
@@ -176,6 +194,7 @@ mod tests {
176194
FlatEnum {
177195
name: name.to_string(),
178196
value: value.to_string(),
197+
literal_value: String::new(),
179198
comment: String::new(),
180199
protect: vec![],
181200
}

src/resolve/types.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,10 @@ pub struct TypeDef {
173173
pub struct FlatEnum {
174174
pub name: String,
175175
pub value: String,
176+
/// Always a numeric literal, even for aliases. Used in the pre-C23
177+
/// `static const` path where referencing another variable is not a
178+
/// constant expression on some compilers (MSVC C2099).
179+
pub literal_value: String,
176180
pub comment: String,
177181
/// Platform protection macros. Empty = unconditional.
178182
pub protect: Vec<String>,

tests/generate_c.rs

Lines changed: 51 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
//! They also attempt a C compile step if `cc` is available on PATH.
55
66
use std::path::Path;
7-
use std::process::Command;
87

98
use tempfile::TempDir;
109

@@ -16,61 +15,60 @@ fn gloam() -> assert_cmd::Command {
1615
assert_cmd::Command::cargo_bin("gloam").expect("gloam binary not found")
1716
}
1817

19-
/// Attempt to compile a generated C source with the system C compiler.
20-
/// Silently skips if `cc` is not on PATH (expected in CI, optional locally).
18+
/// Attempt to compile generated C sources with the system C compiler.
19+
/// Uses the `cc` crate for compiler detection (handles MSVC, GCC, Clang,
20+
/// cross-compilation toolchains, CC env override, etc.).
21+
/// Silently skips if no compiler is available.
2122
fn try_compile_c(out: &Path) {
22-
// Find a .c file in out/src/.
2323
let src_dir = out.join("src");
24-
let c_file = match std::fs::read_dir(&src_dir).ok().and_then(|mut d| {
25-
d.find(|e| {
26-
e.as_ref()
27-
.is_ok_and(|e| e.path().extension() == Some("c".as_ref()))
28-
})
29-
}) {
30-
Some(Ok(entry)) => entry.path(),
31-
_ => return, // nothing to compile
32-
};
33-
34-
let cc = match find_cc() {
35-
Some(c) => c,
36-
None => {
37-
eprintln!("compile check skipped: no C compiler on PATH");
38-
return;
24+
let c_files: Vec<_> = std::fs::read_dir(&src_dir)
25+
.ok()
26+
.into_iter()
27+
.flatten()
28+
.filter_map(|e| e.ok())
29+
.map(|e| e.path())
30+
.filter(|p| p.extension() == Some("c".as_ref()))
31+
.collect();
32+
33+
if c_files.is_empty() {
34+
return;
35+
}
36+
37+
// The cc crate expects TARGET/HOST env vars (normally set by Cargo during
38+
// build.rs). In test context they're absent, so provide them.
39+
let target = env!("TARGET");
40+
let mut build = cc::Build::new();
41+
build
42+
.target(target)
43+
.host(target)
44+
.opt_level(0)
45+
.out_dir(&src_dir)
46+
.include(out.join("include"))
47+
.warnings(true)
48+
.cargo_warnings(false)
49+
.std("c11")
50+
.flag_if_supported("-Wno-unused-function");
51+
52+
for f in &c_files {
53+
build.file(f);
54+
}
55+
56+
if let Err(e) = build.try_compile("gloam_test") {
57+
// Distinguish "no compiler" from "compilation failed".
58+
let msg = e.to_string();
59+
if msg.contains("Failed to find tool")
60+
|| msg.contains("not found")
61+
|| msg.contains("couldn't find")
62+
{
63+
eprintln!("compile check skipped: no C compiler found");
64+
} else {
65+
panic!(
66+
"generated C files in {} failed to compile: {}",
67+
src_dir.display(),
68+
e
69+
);
3970
}
40-
};
41-
42-
let status = Command::new(cc)
43-
.args([
44-
"-c",
45-
"-std=c11",
46-
"-Wall",
47-
"-Wno-unused-function",
48-
"-o",
49-
"/dev/null",
50-
&format!("-I{}", out.join("include").display()),
51-
c_file.to_str().unwrap(),
52-
])
53-
.status()
54-
.expect("failed to spawn C compiler");
55-
56-
assert!(
57-
status.success(),
58-
"generated C file {} failed to compile",
59-
c_file.display()
60-
);
61-
}
62-
63-
fn find_cc() -> Option<&'static str> {
64-
["cc", "gcc", "clang"]
65-
.iter()
66-
.find(|&candidate| {
67-
Command::new(candidate)
68-
.arg("--version")
69-
.output()
70-
.map(|o| o.status.success())
71-
.unwrap_or(false)
72-
})
73-
.map(|v| v as _)
71+
}
7472
}
7573

7674
fn assert_c_output_exists(out: &Path, stem: &str) {

0 commit comments

Comments
 (0)