Skip to content

Commit

Permalink
add workaround for false-positive cimport header not found errors (#1520
Browse files Browse the repository at this point in the history
)

When running translate-c, the set of all c include directories may not
be fully resolved because build runner which is running asynchronously
may not have finished yet.
These changes should detect this and not report the translate-c error.
  • Loading branch information
Techatrix authored Oct 17, 2023
1 parent b8b4ea8 commit b18fd85
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 35 deletions.
68 changes: 38 additions & 30 deletions src/DocumentStore.zig
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,16 @@ const BuildFile = struct {
uri: Uri,
/// contains information extracted from running build.zig with a custom build runner
/// e.g. include paths & packages
config: std.json.Parsed(BuildConfig),
/// TODO this field should not be nullable, callsites should await the build config to be resolved
/// and then continue instead of dealing with missing information.
config: ?std.json.Parsed(BuildConfig),
/// this build file may have an explicitly specified path to builtin.zig
builtin_uri: ?Uri = null,
build_associated_config: ?std.json.Parsed(BuildAssociatedConfig) = null,

pub fn deinit(self: *BuildFile, allocator: std.mem.Allocator) void {
allocator.free(self.uri);
self.config.deinit();
if (self.config) |cfg| cfg.deinit();
if (self.builtin_uri) |builtin_uri| allocator.free(builtin_uri);
if (self.build_associated_config) |cfg| cfg.deinit();
}
Expand Down Expand Up @@ -333,7 +335,9 @@ pub fn invalidateBuildFile(self: *DocumentStore, build_file_uri: Uri) error{OutO
build_config.deinit();
return;
};
build_file.config.deinit();
if (build_file.config) |*old_config| {
old_config.deinit();
}
build_file.config = build_config;
}

Expand Down Expand Up @@ -642,22 +646,7 @@ fn createBuildFile(self: *DocumentStore, uri: Uri) error{OutOfMemory}!BuildFile

var build_file = BuildFile{
.uri = try self.allocator.dupe(u8, uri),
.config = undefined, // set below
};

build_file.config = std.json.Parsed(BuildConfig){
.arena = blk: {
var arena = self.allocator.create(std.heap.ArenaAllocator) catch |err| {
self.allocator.free(build_file.uri);
return err;
};
arena.* = std.heap.ArenaAllocator.init(self.allocator);
break :blk arena;
},
.value = .{
.packages = &.{},
.include_dirs = &.{},
},
.config = null,
};

errdefer build_file.deinit(self.allocator);
Expand Down Expand Up @@ -706,7 +695,8 @@ fn uriAssociatedWithBuild(
var checked_uris = std.StringHashMapUnmanaged(void){};
defer checked_uris.deinit(self.allocator);

for (build_file.config.value.packages) |package| {
const build_config = build_file.config orelse return false;
for (build_config.value.packages) |package| {
const package_uri = try URI.fromPath(self.allocator, package.path);
defer self.allocator.free(package_uri);

Expand Down Expand Up @@ -980,8 +970,9 @@ fn collectDependenciesInternal(
}

if (handle.associated_build_file) |build_file_uri| {
if (store.build_files.get(build_file_uri)) |build_file| {
const packages = build_file.config.value.packages;
if (store.build_files.get(build_file_uri)) |build_file| blk: {
const build_config = build_file.config orelse break :blk;
const packages = build_config.value.packages;
try dependencies.ensureUnusedCapacity(allocator, packages.len);
for (packages) |pkg| {
dependencies.appendAssumeCapacity(try URI.fromPath(allocator, pkg.path));
Expand All @@ -990,23 +981,31 @@ fn collectDependenciesInternal(
}
}

/// returns `true` if all include paths could be collected
/// may return `false` because include paths from a build.zig may not have been resolved already
pub fn collectIncludeDirs(
store: *const DocumentStore,
allocator: std.mem.Allocator,
handle: Handle,
include_dirs: *std.ArrayListUnmanaged([]const u8),
) !void {
) !bool {
var collected_all = true;

const target_info = try std.zig.system.NativeTargetInfo.detect(.{});

var arena_allocator = std.heap.ArenaAllocator.init(allocator);
defer arena_allocator.deinit();

var native_paths = try std.zig.system.NativePaths.detect(arena_allocator.allocator(), target_info);

const build_file_includes_paths: []const []const u8 = if (handle.associated_build_file) |build_file_uri|
store.build_files.get(build_file_uri).?.config.value.include_dirs
else
&.{};
const build_file_includes_paths: []const []const u8 = if (handle.associated_build_file) |build_file_uri| blk: {
if (store.build_files.get(build_file_uri).?.config) |cfg| {
break :blk cfg.value.include_dirs;
} else {
collected_all = false;
break :blk &.{};
}
} else &.{};

try include_dirs.ensureTotalCapacity(allocator, native_paths.include_dirs.items.len + build_file_includes_paths.len);

Expand All @@ -1027,6 +1026,8 @@ pub fn collectIncludeDirs(
};
include_dirs.appendAssumeCapacity(absolute_path);
}

return collected_all;
}

/// returns the document behind `@cImport()` where `node` is the `cImport` node
Expand Down Expand Up @@ -1061,7 +1062,8 @@ pub fn resolveCImport(self: *DocumentStore, handle: Handle, node: Ast.Node.Index
}
include_dirs.deinit(self.allocator);
}
self.collectIncludeDirs(self.allocator, handle, &include_dirs) catch |err| {

const collected_all_include_dirs = self.collectIncludeDirs(self.allocator, handle, &include_dirs) catch |err| {
log.err("failed to resolve include paths: {}", .{err});
return null;
};
Expand All @@ -1080,6 +1082,11 @@ pub fn resolveCImport(self: *DocumentStore, handle: Handle, node: Ast.Node.Index
};
var result = maybe_result orelse return null;

if (result == .failure and !collected_all_include_dirs) {
result.deinit(self.allocator);
return null;
}

self.cimports.putNoClobber(self.allocator, hash, result) catch result.deinit(self.allocator);

switch (result) {
Expand Down Expand Up @@ -1126,9 +1133,10 @@ pub fn uriFromImportStr(self: *DocumentStore, allocator: std.mem.Allocator, hand
}
return null;
} else if (!std.mem.endsWith(u8, import_str, ".zig")) {
if (handle.associated_build_file) |build_file_uri| {
if (handle.associated_build_file) |build_file_uri| blk: {
const build_file = self.getBuildFile(build_file_uri).?;
for (build_file.config.value.packages) |pkg| {
const config = build_file.config orelse break :blk;
for (config.value.packages) |pkg| {
if (std.mem.eql(u8, import_str, pkg.name)) {
return try URI.fromPath(allocator, pkg.path);
}
Expand Down
9 changes: 5 additions & 4 deletions src/features/completions.zig
Original file line number Diff line number Diff line change
Expand Up @@ -860,7 +860,7 @@ fn completeFileSystemStringLiteral(
if (std.fs.path.isAbsolute(completing) and pos_context != .import_string_literal) {
try search_paths.append(arena, completing);
} else if (pos_context == .cinclude_string_literal) {
store.collectIncludeDirs(arena, handle, &search_paths) catch |err| {
_ = store.collectIncludeDirs(arena, handle, &search_paths) catch |err| {
log.err("failed to resolve include paths: {}", .{err});
return &.{};
};
Expand Down Expand Up @@ -906,11 +906,12 @@ fn completeFileSystemStringLiteral(
}

if (completing.len == 0 and pos_context == .import_string_literal) {
if (handle.associated_build_file) |uri| {
if (handle.associated_build_file) |uri| blk: {
const build_file = store.build_files.get(uri).?;
try completions.ensureUnusedCapacity(arena, build_file.config.value.packages.len);
const build_config = build_file.config orelse break :blk;
try completions.ensureUnusedCapacity(arena, build_config.value.packages.len);

for (build_file.config.value.packages) |pkg| {
for (build_config.value.packages) |pkg| {
completions.putAssumeCapacity(.{
.label = pkg.name,
.kind = .Module,
Expand Down
2 changes: 1 addition & 1 deletion src/features/goto.zig
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ pub fn gotoDefinitionString(
blk: {
if (std.fs.path.isAbsolute(import_str)) break :blk import_str;
var include_dirs: std.ArrayListUnmanaged([]const u8) = .{};
document_store.collectIncludeDirs(arena, handle.*, &include_dirs) catch |err| {
_ = document_store.collectIncludeDirs(arena, handle.*, &include_dirs) catch |err| {
log.err("failed to resolve include paths: {}", .{err});
return null;
};
Expand Down

0 comments on commit b18fd85

Please sign in to comment.