Skip to content

Restore translate-c --enable-cache functionality. #15072

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
llogick wants to merge 1 commit into from
Closed

Restore translate-c --enable-cache functionality. #15072

llogick wants to merge 1 commit into from

Conversation

@llogick
Copy link
Contributor

@llogick llogick commented Mar 25, 2023 •
edited
Loading

Resolves #15025

Edit:
The error from the single failure on x86_64-linux-release:

2023-03-28T00:08:43.0489835Z zig test ReleaseFast arm-linux-musleabihf: error: warning(link): unexpected LLD stderr:
2023-03-28T00:08:43.0493986Z ld.lld: warning: Linking two modules of different target triples: '/home/ci/actions-runner4/_work/zig/zig/build-release/zig-local-cache/o/e7bdc6dbc528ab95f26848e40fa4617b/test-c-abi-arm-linux-musleabihf-ReleaseFast .o' is 'arm-unknown-linux-musleabihf' whereas 'ld-temp.o' is 'armv6kz-unknown-linux-musleabihf'

This is quite a straightforward change, based on existing logic

zig/src/main.zig

Lines 3476 to 3486 in 2b80552

if (arg_mode == .translate_c) {
var arena_instance = std.heap.ArenaAllocator.init(gpa);
defer arena_instance.deinit();
const arena = arena_instance.allocator();
var output: TranslateCOutput = undefined;
try cmdTranslateC(comp, arena, &output);
try server.serveEmitBinPath(output.path, .{
.flags = .{ .cache_hit = output.cache_hit },
});
continue;
}

so there shouldn't be any seen or unforeseen issues/breakages (i.e, wouldn't expect any additional preliminary Workflow Runs necessary).

I'll be rebasing this just to keep it fresh, aware that it'd be up to @andrewrk to review and, conceivably, approve this.

✌️

@llogick llogick force-pushed the restore-translate-c--enable-cache branch 4 times, most recently from b4ea683 to 4197689 Compare March 27, 2023 21:57
@llogick llogick force-pushed the restore-translate-c--enable-cache branch from 4197689 to deb8e2c Compare March 28, 2023 19:57
andrewrk added a commit that referenced this pull request Apr 10, 2023
@andrewrk
CLI: remove --enable-cache option
ede1a5d 
This use case is now handled instead by the --listen option.

closes #15025
closes #15072
@andrewrk andrewrk mentioned this pull request Apr 10, 2023
Merged
@andrewrk
Copy link
Member

andrewrk commented Apr 10, 2023

Thanks for the patch. This would have worked fine, except that --enable-cache is intentionally being deleted now. This use case is now handled by the new --listen option, which is perhaps a bit more effort to start using, but is needed in order to support the full breadth of (interactive) API that the compiler is beginning to provide.

Superseded by #15234.

@andrewrk andrewrk closed this Apr 10, 2023
@silversquirl silversquirl mentioned this pull request Apr 10, 2023
Closed
@llogick llogick deleted the restore-translate-c--enable-cache branch April 10, 2023 19:06
andrewrk added a commit that referenced this pull request Apr 11, 2023
@andrewrk
CLI: remove --enable-cache option
2f84df4 
This use case is now handled instead by the --listen option.

closes #15025
closes #15072
andrewrk added a commit that referenced this pull request Apr 11, 2023
@andrewrk
CLI: remove --enable-cache option
c96cb98 
This use case is now handled instead by the --listen option.

closes #15025
closes #15072
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Snektron Snektron Snektron approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

--enable-cache path print not respected by translate-c

3 participants

@llogick @andrewrk @Snektron