feat: support profiles to conditional options

[GrapeBaBa]

Motivation
This pull request adds support for build profiles to the configuration system, allowing users to define multiple profiles with module-specific option overrides and a default profile. It introduces a new Profiles structure, updates the parsing logic to handle profiles, and modifies code generation to emit profile-aware option values.

Description

• Added a new Profiles struct to Config.zig, supporting named profiles, a default profile, and per-module option overrides. Includes methods for deinitialization, querying profiles, and retrieving option overrides.
• Updated the configuration parser to handle a new profiles field, including parsing of profiles, their options modules, and option overrides with type compatibility checks.
• Modified ConfigBuildgen.zig to emit profile selection logic, including an enum of profile tags, dispatch code for selecting the active profile, and per-profile option overrides using a switch statement.
• Updated option writing logic to support profile-specific overrides, type compatibility validation, and fallback to default values if not all profiles override an option. Also refactored enum option handling to use named enum types for runtime control flow.

[gemini-code-assist]

Summary of Changes

Hello @GrapeBaBa, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a comprehensive build profile system, enabling users to define distinct configurations for their projects. It seamlessly integrates profile management into the existing configuration parsing and code generation workflows, allowing for dynamic and conditional application of build options based on the selected profile. This enhancement provides greater flexibility and control over project builds, catering to different development, testing, and production environments.

Highlights

• New Profiles Structure: Introduced a Profiles struct in Config.zig to manage multiple build profiles, including a default profile and module-specific option overrides. This structure includes methods for deinitialization, querying profile names, and retrieving specific option overrides.
• Configuration Parsing Updates: The configuration parser has been updated to recognize and process a new profiles field. This involves parsing individual profiles, their associated option modules, and validating type compatibility for all option overrides.
• Profile-Aware Code Generation: ConfigBuildgen.zig now generates code that incorporates profile selection logic. This includes creating an enum of profile tags, dispatching to the active profile based on a build option, and using a switch statement to apply profile-specific option values.
• Enhanced Option Writing Logic: The logic for writing options has been significantly enhanced to support profile-specific overrides. It now includes type compatibility validation for overrides and ensures proper fallback to default values if an option is not overridden in all profiles.
• Refactored Enum Option Handling: Enum option handling in ConfigBuildgen.zig has been refactored to define named enum types, allowing for more robust runtime control flow and better integration with the new profile system.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces an excellent feature by adding support for build profiles. The implementation is comprehensive, covering configuration parsing in Config.zig and code generation in ConfigBuildgen.zig. I've identified a few critical issues and areas for improvement. There are a couple of bugs in the code generation logic within ConfigBuildgen.zig that would produce invalid Zig code. Additionally, I found some memory leaks in the new parsing logic in src/Config.zig that occur on error paths. I've also included a suggestion to reduce code duplication. After addressing these points, the pull request should be in great shape.

[gemini-code-assist]

There's a typo in the template string here. The }} else will produce invalid Zig syntax in the generated build.zig file. Assuming profileDispatchBody generates a complete if-else if chain ending with a }, this should be else to correctly append the final else block.

            \         else {{

[gemini-code-assist]

The logic for generating the profile dispatch body is incorrect. It produces an extra closing brace in the else if condition and fails to close the final if block. This will cause a syntax error in the generated build.zig file. The function should generate a complete and valid if-else if chain.

fn profileDispatchBody(self: *ConfigBuildgen, profile_names: []const []const u8) ![]u8 {
    var buffer = std.ArrayList(u8).init(self.allocator);
    errdefer buffer.deinit();
    var writer = buffer.writer();

    for (profile_names, 0..) |name, index| {
        const tag = try allocFmtId(self.allocator, "profile", name);
        defer self.allocator.free(tag);

        if (index == 0) {
            try writer.print("        if (std.mem.eql(u8, profile, \"{s}\")) {{\n", .{name});
        } else {
            try writer.print("        }} else if (std.mem.eql(u8, profile, \"{s}\")) {{\n", .{name});
        }

        try writer.print("            break :blk ProfileTag.{s};\n", .{tag});
    }

    if (profile_names.len > 0) {
        try writer.print("        }}", .{});
    }

    return buffer.toOwnedSlice();
}

[GrapeBaBa]

no issue here, the template has the final block

[gemini-code-assist]

This function allocates memory for profiles and its contents, but it doesn't deallocate this memory if an error occurs during parsing, leading to a memory leak. To prevent this, you can use errdefer to ensure profiles.deinit() is called on all error paths, and then cancel the errdefer before successful returns.

Here's an example of the pattern:

fn parseProfiles(...) !Profiles {
    var profiles: Profiles = .{};
    errdefer profiles.deinit(self.gpa);

    // ... existing function body ...

    if (successful_condition) {
        errdefer (void)0; // Cancel cleanup on success
        return profiles;
    }

    // ... more logic ...

    errdefer (void)0; // Cancel cleanup on success
    return profiles;
}

[gemini-code-assist]

There's a potential memory leak in this block. If map.getOrPut(profile_name) fails, the memory allocated for profile_name and profile will be leaked. Using errdefer ensures these resources are cleaned up correctly on error paths.

                const profile_name = try self.gpa.dupe(u8, field_name);
                errdefer self.gpa.free(profile_name);

                var profile = try self.parseProfile(field_value);
                errdefer profile.deinit(self.gpa);

                const gop = try map.getOrPut(profile_name);
                if (!gop.found_existing) {
                    gop.value_ptr.* = profile;
                    errdefer (void)0; // ownership of profile transferred, cancel errdefer
                    errdefer (void)0; // ownership of profile_name transferred, cancel errdefer
                } else {
                    // `profile_name` and `profile` are not stored, so they need to be freed.
                    // The `errdefer`s would handle this if this function returned an error,
                    // but `returnParseErrorFmt` is `noreturn`. So we must clean up manually.
                    self.gpa.free(profile_name);
                    profile.deinit(self.gpa);
                    try self.returnParseErrorFmt("duplicate profile '{s}'", .{field_name}, field_value.getAstNode(self.zoir));
                }

[gemini-code-assist]

This deinitialization logic is duplicated in the errdefer block of the parseProfileOptionsModules function (lines 1270-1278). To improve maintainability and avoid potential bugs from inconsistent updates, this logic should be extracted into a private helper function within the Profiles struct. This function can then be called from both places.

For example:

// Inside `pub const Profiles = struct {`
fn deinitOptionsModules(modules: *ArrayHashMap(OptionsModule), gpa: std.mem.Allocator) void {
    for (modules.values()) |*module| {
        for (module.values()) |*override| override.deinit(gpa);
        for (module.keys()) |k| gpa.free(k);
        module.deinit();
    }
    for (modules.keys()) |k| gpa.free(k);
    modules.deinit();
}

This can then be called from Profile.deinit and parseProfileOptionsModules.

[Copilot]

Pull request overview

This pull request adds support for build profiles to the configuration system, enabling users to define multiple named profiles with option-specific overrides. Users can specify a default profile and override option values on a per-profile basis.

• Introduces a new Profiles structure with profile management methods and option override capabilities
• Extends the parser to handle profile definitions, option overrides, and type compatibility validation
• Updates code generation to emit profile selection logic with runtime dispatch based on build options

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
test/sync.zig	Added new test case for basic7.zbuild.zon fixture to test profile functionality
test/fixtures/basic7.zbuild.zon	New test fixture demonstrating profile configuration with dev/prod profiles and option overrides
src/Config.zig	Added Profiles struct with nested Profile and OptionOverride types, parser methods for profile parsing, and compatibility checking logic
src/ConfigBuildgen.zig	Updated build.zig generation to emit profile enum, dispatch logic, and per-profile switch statements for option overrides with proper type handling

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Memory leak: The module_name allocated on line 1281 will leak if parseProfileModuleOverrides fails on line 1283, since it hasn't been added to the modules HashMap yet and won't be cleaned up by the errdefer block. Add errdefer self.gpa.free(module_name); after line 1281 to handle this case.

Suggested change

	const module_name = try self.gpa.dupe(u8, name.get(self.zoir));
	const module_name = try self.gpa.dupe(u8, name.get(self.zoir));
	errdefer self.gpa.free(module_name);

[Copilot]

Memory leak: The enum_id allocated by allocFmtId is stored in the t variable and later used in self.options.put(name, .{ .type_name = t, ... }) at line 522, but it's never freed. Since the options HashMap only stores pointers and doesn't own the strings, this allocated memory should be freed after the function completes. However, since t is used in the generated output (line 507/514) and stored in the HashMap, the lifetime management is complex. Consider tracking these allocations for proper cleanup in the deinit method, or refactor to avoid the allocation altogether.

[Copilot]

Memory leak: When profile overrides exist, the default value (which may be an allocated string from the switch block at line 365-441) is assigned to final_default_expr on line 467, but then final_default_expr is overwritten with switch_id on line 499. The original default string is never freed. You should free default before overwriting final_default_expr on line 499, but only if it was allocated (not for static strings like "bool" or type names from the config).

[Copilot]

Memory leak: The option_name allocated on line 1300 will leak if parseProfileOptionOverride fails on line 1302, since it hasn't been added to the overrides HashMap yet and won't be cleaned up by the errdefer block. Add errdefer self.gpa.free(option_name); after line 1300 to handle this case.

Suggested change

	const option_name = try self.gpa.dupe(u8, option_name_node.get(self.zoir));
	const option_name = try self.gpa.dupe(u8, option_name_node.get(self.zoir));
	errdefer self.gpa.free(option_name);

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

If append fails, enum_id will be leaked since it was allocated on line 350 but never freed. Add errdefer self.allocator.free(enum_id); after line 350.

Suggested change

	const enum_id = try allocFmtId(self.allocator, "Enum", name);
	const enum_id = try allocFmtId(self.allocator, "Enum", name);
	errdefer self.allocator.free(enum_id);

[Copilot]

If append on line 361 fails, enum_id will be leaked. Add errdefer self.allocator.free(enum_id); after line 360.

Suggested change

	const enum_id = try allocFmtId(self.allocator, "Enum", name);
	const enum_id = try allocFmtId(self.allocator, "Enum", name);
	errdefer self.allocator.free(enum_id);

[Copilot]

This error is returned without providing context about which option is missing a fallback. Consider using returnParseErrorFmt or including the option name in the error for better debugging.

Suggested change

	if (!all_profiles_covered and init_default == null) return error.MissingProfileFallback;
	if (!all_profiles_covered and init_default == null)
	return returnParseErrorFmt("Missing profile fallback for option '{s}' in module '{s}'", .{name, module_name});

[Copilot]

This error doesn't indicate which profile, module, or option has the type mismatch. Consider providing more context in the error message to aid debugging.

[Copilot]

The hardcoded requirement for exactly one field named 'options_modules' makes the profile structure inflexible for future extensions. Consider allowing additional optional fields or documenting why this strict limitation exists.

Suggested change

	if (n.names.len != 1) {
	try self.returnParseError("profile must specify exactly one field 'options_modules'", index.getAstNode(self.zoir));
	}
	const field_name = n.names[0].get(self.zoir);
	const field_value = n.vals.at(0);
	if (std.mem.eql(u8, field_name, "options_modules")) {
	profile.options_modules = try self.parseProfileOptionsModules(field_value);
	} else {
	try self.returnParseErrorFmt("unknown field '{s}' in profile", .{field_name}, field_value.getAstNode(self.zoir));
	var found_options_modules = false;
	for (n.names, 0..) |name, i| {
	const field_name = name.get(self.zoir);
	const field_value = n.vals.at(@intCast(i));
	if (std.mem.eql(u8, field_name, "options_modules")) {
	profile.options_modules = try self.parseProfileOptionsModules(field_value);
	found_options_modules = true;
	} else {
	// Ignore unknown fields for forward compatibility.
	// Optionally, log a warning or handle as needed.
	// For now, do nothing.
	}
	}
	if (!found_options_modules) {
	try self.returnParseError("profile must specify 'options_modules'", index.getAstNode(self.zoir));

[wemeetagain]

should we enforce that only fields from the overridden options module are used?
likewise, should we enforce that only options modules that are already named may be overridden?

[wemeetagain]

similar feature now exists