Skip to content

Commit

Permalink
Make OpenInclude debug messages less noisy.
Browse files Browse the repository at this point in the history
Only print entering and attempts to open a file as VLOG(2), and
report non-found as VLOG(1). That way it is easier to spot if an include
was not found.

In not-found status, don't record all include paths searched (this
can be a lot, thus a long string), just the number of paths searched.

Refine error reporting in file_util. Turns out that Windows returns
ESRCH ('no such process') when it doesn't find a file.
  • Loading branch information
hzeller committed Jan 29, 2024
1 parent 9f1067e commit b64dcf0
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 66 deletions.
6 changes: 4 additions & 2 deletions common/util/file_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,19 +80,21 @@ static absl::Status CreateErrorStatusFromSysError(absl::string_view filename,
const char *const system_msg =
sys_error == 0 ? fallback_msg : strerror(sys_error);
if (filename.empty()) filename = "<empty-filename>";
const std::string msg = absl::StrCat(filename, ": ", system_msg);
std::string msg = absl::StrCat(filename, ": ", system_msg);
switch (sys_error) {
case EPERM:
case EACCES:
return {absl::StatusCode::kPermissionDenied, msg};
case ENOENT:
case ESRCH: // Win32 returns this for fs::status() on non-existing file.
return {absl::StatusCode::kNotFound, msg};
case EEXIST:
return {absl::StatusCode::kAlreadyExists, msg};
case EINVAL:
case EISDIR:
return {absl::StatusCode::kInvalidArgument, msg};
default:
absl::StrAppend(&msg, " (sys_error=", sys_error, ")");
return {absl::StatusCode::kUnknown, msg};
}
}
Expand Down Expand Up @@ -133,7 +135,7 @@ absl::Status UpwardFileSearch(absl::string_view start,

absl::Status FileExists(const std::string &filename) {
std::error_code err;
fs::file_status stat = fs::status(filename, err);
const fs::file_status stat = fs::status(filename, err);

if (err.value() != 0) {
return CreateErrorStatusFromErr(filename, err, "file exists check");
Expand Down
6 changes: 6 additions & 0 deletions common/util/file_util_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,12 @@ TEST(FileUtil, ScopedTestFileEmplace) {
}
}

TEST(FileUtil, FileNotExistsTests) {
absl::Status s = file::FileExists("not/an/existing/file");
EXPECT_FALSE(s.ok());
EXPECT_EQ(s.code(), absl::StatusCode::kNotFound);
}

TEST(FileUtil, FileExistsDirectoryErrorMessage) {
absl::Status s;
s = file::FileExists(testing::TempDir());
Expand Down
3 changes: 2 additions & 1 deletion verilog/analysis/symbol_table_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8252,7 +8252,8 @@ TEST(BuildSymbolTableTest, ModuleInstancesFromProjectMissingFile) {
symbol_table.BuildSingleTranslationUnit("file/not/found.txt",
&build_diagnostics);
ASSERT_FALSE(build_diagnostics.empty());
EXPECT_THAT(build_diagnostics.front().code(), absl::StatusCode::kNotFound);
EXPECT_THAT(build_diagnostics.front().code(), absl::StatusCode::kNotFound)
<< build_diagnostics.front();
}

TEST(BuildSymbolTableTest, ModuleInstancesFromProjectFilesGood) {
Expand Down
22 changes: 10 additions & 12 deletions verilog/analysis/verilog_project.cc
Original file line number Diff line number Diff line change
Expand Up @@ -301,14 +301,14 @@ absl::StatusOr<VerilogSourceFile *> VerilogProject::OpenTranslationUnit(

absl::Status VerilogProject::IncludeFileNotFoundError(
absl::string_view referenced_filename) const {
return absl::NotFoundError(absl::StrCat(
"Unable to find '", referenced_filename,
"' among the included paths: ", absl::StrJoin(include_paths_, ", ")));
return absl::NotFoundError(
absl::StrCat("'", referenced_filename, "' not in any of the ",
include_paths_.size(), " include paths"));
}

absl::StatusOr<VerilogSourceFile *> VerilogProject::OpenIncludedFile(
absl::string_view referenced_filename) {
VLOG(1) << __FUNCTION__ << ", referenced: " << referenced_filename;
VLOG(2) << __FUNCTION__ << ", referenced: " << referenced_filename;
// Check for a pre-existing entry to avoid duplicate files.
{
const auto opened_file = FindOpenedFile(referenced_filename);
Expand All @@ -329,18 +329,16 @@ absl::StatusOr<VerilogSourceFile *> VerilogProject::OpenIncludedFile(

// Locate the file among the base paths.
for (const auto &include_path : include_paths_) {
const std::string resolved_filename =
const std::string resolved =
verible::file::JoinPath(include_path, referenced_filename);
if (verible::file::FileExists(resolved_filename).ok()) {
VLOG(2) << "File '" << resolved_filename << "' exists. Resolved from '"
<< referenced_filename << "'";
return OpenFile(referenced_filename, resolved_filename, Corpus());
if (verible::file::FileExists(resolved).ok()) {
VLOG(2) << referenced_filename << " in incdir '" << resolved << "'";
return OpenFile(referenced_filename, resolved, Corpus());
}
VLOG(2) << "Checked for file'" << resolved_filename
<< "', but not found. Resolved from '" << referenced_filename
<< "'";
VLOG(2) << referenced_filename << " not in incdir '" << resolved << "'";
}

VLOG(1) << __FUNCTION__ << "': '" << referenced_filename << "' not found";
// Not found in any path. Cache this status.
const auto inserted = files_.emplace(
referenced_filename,
Expand Down
Loading

0 comments on commit b64dcf0

Please sign in to comment.