Skip to content

Commit

Permalink
Fix issue preventing googletest from using AbslStringify for params
Browse files Browse the repository at this point in the history
In order for this to work googletest needs to be configured with absl.

This also included removing an old workaround for grpc that is no longer needed.

PiperOrigin-RevId: 571397085
  • Loading branch information
allight authored and copybara-github committed Oct 6, 2023
1 parent 123d30c commit 8058d52
Show file tree
Hide file tree
Showing 32 changed files with 210 additions and 69 deletions.
3 changes: 3 additions & 0 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ build --copt "-Wno-sign-compare"
build --copt "-Wno-comment"
build --host_copt "-Wno-sign-compare"
build --host_copt "-Wno-comment"
# use absl in googletest to work around
# https://github.com/google/googletest/issues/4383
build --define absl=1

# TODO(leary): 2020-09-09 Make it possible to enable this option.
# Currently m4 doesn't seem to work as a dependency.
Expand Down
13 changes: 0 additions & 13 deletions dependency_support/com_github_grpc_grpc/grpc_patch.diff

This file was deleted.

11 changes: 1 addition & 10 deletions dependency_support/load_external.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,8 @@ def load_external_repositories():
sha256 = "0fb3a4916a157eb48124ef309231cecdfdd96ff54adf1660b39c0d4a9790a2c0",
)

# Note - use @com_github_google_re2 instead of more canonical
# @com_google_re2 for consistency with dependency grpc
# which uses @com_github_google_re2.
# (see https://github.com/google/xls/issues/234)
http_archive(
name = "com_github_google_re2",
name = "com_googlesource_code_re2",
sha256 = "8b4a8175da7205df2ad02e405a950a02eaa3e3e0840947cd598e92dca453199b",
strip_prefix = "re2-2023-06-01",
urls = [
Expand Down Expand Up @@ -196,11 +192,6 @@ def load_external_repositories():
urls = ["https://github.com/grpc/grpc/archive/v1.55.1.tar.gz"],
sha256 = "9c3c0a0ad986ee4fc0a9b58fd71255010068df7d1437c425b525d68c30c85ac7",
strip_prefix = "grpc-1.55.1",
# Note: repo mapping doesn't seem to work for gRPC because it
# explicitly binds the re2 name to the com_googlesource_code_re2 repo.
# Instead we patch it.
#repo_mapping = {"@com_googlesource_code_re2": "@com_github_google_re2"},
patches = ["@com_google_xls//dependency_support/com_github_grpc_grpc:grpc_patch.diff"],
)

# Used by xlscc.
Expand Down
2 changes: 1 addition & 1 deletion dev_utils/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@ cc_binary(
"//xls/common:subprocess",
"//xls/common:thread",
"//xls/common/file:filesystem",
"@com_github_google_re2//:re2",
"@com_googlesource_code_re2//:re2",
],
)
4 changes: 2 additions & 2 deletions xls/codegen/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -372,14 +372,14 @@ cc_library(
"//xls/passes:tuple_simplification_pass",
"//xls/scheduling:pipeline_schedule",
"//xls/scheduling:scheduling_options",
"@com_github_google_re2//:re2",
"@com_google_absl//absl/container:flat_hash_map",
"@com_google_absl//absl/container:flat_hash_set",
"@com_google_absl//absl/status",
"@com_google_absl//absl/status:statusor",
"@com_google_absl//absl/strings",
"@com_google_absl//absl/strings:str_format",
"@com_google_absl//absl/types:span",
"@com_googlesource_code_re2//:re2",
],
)

Expand Down Expand Up @@ -933,10 +933,10 @@ cc_library(
":vast",
"//xls/common/status:status_macros",
"//xls/ir",
"@com_github_google_re2//:re2",
"@com_google_absl//absl/status:statusor",
"@com_google_absl//absl/strings",
"@com_google_absl//absl/types:span",
"@com_googlesource_code_re2//:re2",
],
)

Expand Down
29 changes: 27 additions & 2 deletions xls/common/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,35 @@ cc_library(
],
)

cc_library(
name = "gunit_init_xls",
srcs = ["gunit_init_xls.cc"],
hdrs = ["gunit_init_xls.h"],
deps = [
":init_xls",
"@com_google_absl//absl/flags:parse",
"@com_google_googletest//:gtest",
],
)

cc_library(
name = "benchmark_support",
srcs = ["benchmark_support.cc"],
hdrs = ["benchmark_support.h"],
visibility = ["//visibility:private"],
deps = [
"@com_google_benchmark//:benchmark",
"@com_google_googletest//:gtest",
],
)

cc_library(
name = "init_xls",
srcs = ["init_xls.cc"],
hdrs = ["init_xls.h"],
deps = [
"@com_google_absl//absl/flags:flag",
"@com_google_absl//absl/strings",
"//xls/common/file:get_runfile_path",
"//xls/common/logging",
"//xls/common/logging:vlog_is_on",
"@com_google_absl//absl/flags:parse",
Expand Down Expand Up @@ -501,8 +523,11 @@ cc_library(
srcs = ["xls_gunit_main.cc"],
visibility = ["//xls:xls_utility_users"],
deps = [
":benchmark_support",
":gunit_init_xls",
":init_xls",
":xls_gunit",
"@com_google_benchmark//:benchmark",
"@com_google_googletest//:gtest",
],
)

Expand Down
20 changes: 20 additions & 0 deletions xls/common/benchmark_support.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2023 The XLS Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#include "xls/common/benchmark_support.h"
#include "benchmark/benchmark.h"

namespace xls {
void RunSpecifiedBenchmarks() { benchmark::RunSpecifiedBenchmarks(); }
} // namespace xls
24 changes: 24 additions & 0 deletions xls/common/benchmark_support.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2023 The XLS Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#ifndef XLS_COMMON_BENCHMARK_SUPPORT_H_
#define XLS_COMMON_BENCHMARK_SUPPORT_H_

namespace xls {
// Helper to perform setup and execute benchmarks. Not a public API.
// This should be called after parsing all flags and setting up googletest.
void RunSpecifiedBenchmarks();
} // namespace xls

#endif // XLS_COMMON_BENCHMARK_SUPPORT_H_
2 changes: 1 addition & 1 deletion xls/common/file/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ cc_library(
"//xls/common/status:error_code_to_status",
"//xls/common/status:ret_check",
"//xls/common/status:status_macros",
"@com_github_google_re2//:re2",
"@com_google_protobuf//:protobuf",
"@com_googlesource_code_re2//:re2",
],
)

Expand Down
32 changes: 32 additions & 0 deletions xls/common/gunit_init_xls.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright 2023 The XLS Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#include "xls/common/gunit_init_xls.h"

#include "gtest/gtest.h"
#include "xls/common/init_xls.h"

namespace xls {

std::vector<std::string_view> InitXlsForTest(std::string_view usage, int argc,
char* argv[]) {
// InitGoogleTest calls ParseAbslFlags with gtest configured to use absl (like
// we do).
testing::InitGoogleTest(&argc, argv);

internal::InitXlsPostAbslFlagParse();

return std::vector<std::string_view>(argv + 1, argv + argc);
}
} // namespace xls
47 changes: 47 additions & 0 deletions xls/common/gunit_init_xls.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Copyright 2023 The XLS Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#ifndef XLS_COMMON_GUNIT_INIT_XLS_H_
#define XLS_COMMON_GUNIT_INIT_XLS_H_

#include <string_view>
#include <vector>

namespace xls {

// Initializes global state in the test, including things like command line
// flags, symbolizer etc. This function might exit the program, for example if
// the command line flags are invalid or if a '--help' command line argument was
// provided.
//
// Is typically called early on in main().
//
// It must be called after InitGoogleTest
//
// `usage` provides a short usage message usable by
// absl::SetProgramUsageMessage().
//
// `argc` and `argv` are the command line flags to parse. This function does not
// modify `argc`.
//
// Returns a vector of the positional arguments that are not part of any
// command-line flag (or arguments to a flag), not including the program
// invocation name. (This includes positional arguments after the
// flag-terminating delimiter '--'.)
std::vector<std::string_view> InitXlsForTest(std::string_view usage, int argc,
char* argv[]);

} // namespace xls

#endif // XLS_COMMON_GUNIT_INIT_XLS_H_
9 changes: 7 additions & 2 deletions xls/common/init_xls.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,26 @@

#include "absl/flags/parse.h"
#include "absl/flags/usage.h"
#include "xls/common/file/get_runfile_path.h"
#include "xls/common/logging/logging.h"

namespace xls {

std::vector<std::string_view> InitXls(std::string_view usage, int argc,
char* argv[]) {
char* argv[]) {
// Comply with xls/common/init_xls.h:32
absl::SetProgramUsageMessage(usage);
// Copy the argv array to ensure this method doesn't clobber argv.
std::vector<char*> arguments(argv, argv + argc);
std::vector<char*> remaining = absl::ParseCommandLine(argc, argv);
XLS_CHECK_GE(argc, 1);

internal::InitXlsPostAbslFlagParse();

return std::vector<std::string_view>(remaining.begin() + 1, remaining.end());
}

namespace internal {
void InitXlsPostAbslFlagParse() {}
} // namespace internal

} // namespace xls
12 changes: 9 additions & 3 deletions xls/common/init_xls.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace xls {
// Is typically called early on in main().
//
// `usage` provides a short usage message passed to
// absl::SetProgramUsageMessage(). It can be nullptr, then it's ignored.
// absl::SetProgramUsageMessage().
//
// `argc` and `argv` are the command line flags to parse. This function does not
// modify `argc`.
Expand All @@ -37,9 +37,15 @@ namespace xls {
// command-line flag (or arguments to a flag), not including the program
// invocation name. (This includes positional arguments after the
// flag-terminating delimiter '--'.)
std::vector<std::string_view> InitXls(std::string_view usage, int argc,
char* argv[]);
std::vector<std::string_view> InitXls(std::string_view usage,
int argc, char* argv[]);

namespace internal {
// Internal function which sets up post-absl common components shared by both
// testing and non-testing inits. Should only be called by InitXls or
// InitXlsForTest.
void InitXlsPostAbslFlagParse();
} // namespace internal
} // namespace xls

#endif // XLS_COMMON_INIT_XLS_H_
2 changes: 2 additions & 0 deletions xls/common/logging/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ cc_library(
":log_entry",
":log_sink",
":logging",
"@com_google_benchmark//:benchmark",
"@com_google_googletest//:gtest",
],
)
Expand Down Expand Up @@ -334,6 +335,7 @@ cc_library(
":log_entry",
":logging",
"@com_google_absl//absl/base:log_severity",
"@com_google_benchmark//:benchmark",
"@com_google_googletest//:gtest",
],
)
Expand Down
1 change: 1 addition & 0 deletions xls/common/status/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ cc_library(
":status_macros",
"@com_google_absl//absl/status:statusor",
"@com_google_absl//absl/strings",
"@com_google_benchmark//:benchmark",
"@com_google_googletest//:gtest",
],
)
7 changes: 4 additions & 3 deletions xls/common/xls_gunit_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@
// limitations under the License.

#include "gtest/gtest.h"
#include "xls/common/init_xls.h"
#include "xls/common/benchmark_support.h"
#include "xls/common/gunit_init_xls.h"

int main(int argc, char* argv[]) {
testing::InitGoogleTest(&argc, argv);
xls::InitXls(argv[0], argc, argv);
xls::InitXlsForTest(argv[0], argc, argv);
xls::RunSpecifiedBenchmarks();

// Rapidcheck parameters for deterministic quickchecks in unit tests.
//
Expand Down
4 changes: 2 additions & 2 deletions xls/contrib/xlscc/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ cc_library(
"//xls/solvers:z3_ir_translator",
"//xls/solvers:z3_utils",
"@z3//:api",
"@com_github_google_re2//:re2",
"@com_googlesource_code_re2//:re2",
"@llvm-project//clang:ast",
"@llvm-project//clang:basic",
"@llvm-project//llvm:Support",
Expand All @@ -163,7 +163,7 @@ cc_library(
"//xls/common/status:status_macros",
"//xls/ir",
"//xls/ir:source_location",
"@com_github_google_re2//:re2",
"@com_googlesource_code_re2//:re2",
"@llvm-project//clang:ast",
"@llvm-project//clang:basic",
"@llvm-project//clang:frontend",
Expand Down
1 change: 1 addition & 0 deletions xls/contrib/xlscc/unit_tests/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ cc_library(
"@com_google_absl//absl/status:statusor",
"@com_google_absl//absl/strings",
"@com_google_absl//absl/strings:str_format",
"@com_google_benchmark//:benchmark",
"@com_google_googletest//:gtest",
"@llvm-project//clang:ast",
],
Expand Down
Loading

0 comments on commit 8058d52

Please sign in to comment.