Fix problems pointed out by building with clang-cl.
After https://gn-review.googlesource.com/c/gn/+/6442 it's fairly
easy to build with clang-cl (and goma!), so I gave it a try:
set CC=c:\src\goma\goma-win64\gomacc.exe c:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe
set CXX=c:\src\goma\goma-win64\gomacc.exe c:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe
set CFLAGS=-Wno-c++11-narrowing
set LD=c:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\lld-link.exe
python build\gen.py
ninja -C out -j100
clang-cl caught a few issues, so this fixes them:
- remove a few unused private fields
- remove a few needless std::move() calls that prevent optimizations
- fix a benign -Wformat warning with char16_t
- fix some struct/class mismatches
- make a test actually test what it intended to test
- remove a const on a value type that had no effect
- fix field initialization order warnings in Value ctors
No behavior change.
Bug: none
Change-Id: I816a0382f2ef4f3a42d7945db6db5ce74fa9d5eb
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/6480
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
diff --git a/src/base/files/file_path.h b/src/base/files/file_path.h
index b136670..2359bb6 100644
--- a/src/base/files/file_path.h
+++ b/src/base/files/file_path.h
@@ -124,11 +124,13 @@
// To print path names portably use PRIsFP (based on PRIuS and friends from
// C99 and format_macros.h) like this:
-// base::StringPrintf("Path is %" PRIsFP ".\n", path.value().c_str());
+// base::StringPrintf("Path is %" PRIsFP ".\n", PATH_CSTR(path);
#if defined(OS_WIN)
#define PRIsFP "ls"
+#define PATH_CSTR(x) reinterpret_cast<const wchar_t*>(x.value().c_str())
#elif defined(OS_POSIX) || defined(OS_FUCHSIA)
#define PRIsFP "s"
+#define PATH_CSTR(x) (x.value().c_str())
#endif // OS_WIN
namespace base {
diff --git a/src/base/files/file_win.cc b/src/base/files/file_win.cc
index 9aae162..4bce7df 100644
--- a/src/base/files/file_win.cc
+++ b/src/base/files/file_win.cc
@@ -59,7 +59,7 @@
LARGE_INTEGER offset_li;
offset_li.QuadPart = offset;
- OVERLAPPED overlapped = {0};
+ OVERLAPPED overlapped = {};
overlapped.Offset = offset_li.LowPart;
overlapped.OffsetHigh = offset_li.HighPart;
@@ -104,7 +104,7 @@
LARGE_INTEGER offset_li;
offset_li.QuadPart = offset;
- OVERLAPPED overlapped = {0};
+ OVERLAPPED overlapped = {};
overlapped.Offset = offset_li.LowPart;
overlapped.OffsetHigh = offset_li.HighPart;
diff --git a/src/base/gtest_prod_util.h b/src/base/gtest_prod_util.h
index 664dded..b3b1175 100644
--- a/src/base/gtest_prod_util.h
+++ b/src/base/gtest_prod_util.h
@@ -7,10 +7,6 @@
// TODO: Remove me.
#define FRIEND_TEST_ALL_PREFIXES(test_case_name, test_name) \
- friend class test_case_name##test_name
-
-// TODO: Remove me.
-#define FORWARD_DECLARE_TEST(test_case_name, test_name) \
- class test_case_name##test_name
+ friend struct test_case_name##test_name
#endif // BASE_GTEST_PROD_UTIL_H_
diff --git a/src/base/json/json_writer.cc b/src/base/json/json_writer.cc
index 656a87c..912a7f0 100644
--- a/src/base/json/json_writer.cc
+++ b/src/base/json/json_writer.cc
@@ -48,8 +48,6 @@
JSONWriter::JSONWriter(int options, std::string* json)
: omit_binary_values_((options & OPTIONS_OMIT_BINARY_VALUES) != 0),
- omit_double_type_preservation_(
- (options & OPTIONS_OMIT_DOUBLE_TYPE_PRESERVATION) != 0),
pretty_print_((options & OPTIONS_PRETTY_PRINT) != 0),
json_string_(json) {
DCHECK(json);
diff --git a/src/base/json/json_writer.h b/src/base/json/json_writer.h
index 7edd3a6..4114f77 100644
--- a/src/base/json/json_writer.h
+++ b/src/base/json/json_writer.h
@@ -24,12 +24,6 @@
// encountered, failure will be returned.
OPTIONS_OMIT_BINARY_VALUES = 1 << 0,
- // This option instructs the writer to write doubles that have no fractional
- // part as a normal integer (i.e., without using exponential notation
- // or appending a '.0') as long as the value is within the range of a
- // 64-bit int.
- OPTIONS_OMIT_DOUBLE_TYPE_PRESERVATION = 1 << 1,
-
// Return a slightly nicer formatted json string (pads with whitespace to
// help with readability).
OPTIONS_PRETTY_PRINT = 1 << 2,
@@ -60,7 +54,6 @@
void IndentLine(size_t depth);
bool omit_binary_values_;
- bool omit_double_type_preservation_;
bool pretty_print_;
// Where we write JSON data as we generate it.
diff --git a/src/base/logging.cc b/src/base/logging.cc
index a8ccd6f..ee60c47 100644
--- a/src/base/logging.cc
+++ b/src/base/logging.cc
@@ -140,18 +140,18 @@
#endif // defined(OS_WIN)
LogMessage::LogMessage(const char* file, int line, LogSeverity severity)
- : severity_(severity), file_(file), line_(line) {
+ : severity_(severity) {
Init(file, line);
}
LogMessage::LogMessage(const char* file, int line, const char* condition)
- : severity_(LOG_FATAL), file_(file), line_(line) {
+ : severity_(LOG_FATAL) {
Init(file, line);
stream_ << "Check failed: " << condition << ". ";
}
LogMessage::LogMessage(const char* file, int line, std::string* result)
- : severity_(LOG_FATAL), file_(file), line_(line) {
+ : severity_(LOG_FATAL) {
Init(file, line);
stream_ << "Check failed: " << *result;
delete result;
@@ -161,7 +161,7 @@
int line,
LogSeverity severity,
std::string* result)
- : severity_(severity), file_(file), line_(line) {
+ : severity_(severity) {
Init(file, line);
stream_ << "Check failed: " << *result;
delete result;
diff --git a/src/base/logging.h b/src/base/logging.h
index c581d33..78cad59 100644
--- a/src/base/logging.h
+++ b/src/base/logging.h
@@ -730,16 +730,7 @@
#define DCHECK_GE(val1, val2) DCHECK_OP(GE, >=, val1, val2)
#define DCHECK_GT(val1, val2) DCHECK_OP(GT, >, val1, val2)
-#if !DCHECK_IS_ON() && defined(OS_CHROMEOS)
-// Implement logging of NOTREACHED() as a dedicated function to get function
-// call overhead down to a minimum.
-void LogErrorNotReached(const char* file, int line);
-#define NOTREACHED() \
- true ? ::logging::LogErrorNotReached(__FILE__, __LINE__) \
- : EAT_STREAM_PARAMETERS
-#else
#define NOTREACHED() DCHECK(false)
-#endif
// Redefine the standard assert to use our nice log files
#undef assert
@@ -785,9 +776,6 @@
std::ostringstream stream_;
size_t message_start_; // Offset of the start of the message (past prefix
// info).
- // The file and line information passed in to the constructor.
- const char* file_;
- const int line_;
#if defined(OS_WIN)
// Stores the current value of GetLastError in the constructor and restores
diff --git a/src/gn/compile_commands_writer.cc b/src/gn/compile_commands_writer.cc
index 11afbf3..10984cc 100644
--- a/src/gn/compile_commands_writer.cc
+++ b/src/gn/compile_commands_writer.cc
@@ -242,7 +242,7 @@
compile_commands->append(kPrettyPrintLineEnding);
WriteFile(source, path_output, compile_commands);
- WriteDirectory(base::StringPrintf("%" PRIsFP, build_dir.value().c_str()),
+ WriteDirectory(base::StringPrintf("%" PRIsFP, PATH_CSTR(build_dir)),
compile_commands);
WriteCommand(target, source, flags, tool_outputs, path_output,
source_type, tool_name, opts, compile_commands);
diff --git a/src/gn/desc_builder.cc b/src/gn/desc_builder.cc
index e52bb6a..57b253d 100644
--- a/src/gn/desc_builder.cc
+++ b/src/gn/desc_builder.cc
@@ -478,7 +478,7 @@
if (target_->output_type() == Target::GENERATED_FILE) {
if (what(variables::kWriteOutputConversion)) {
res->SetKey(variables::kWriteOutputConversion,
- std::move(ToBaseValue(target_->output_conversion())));
+ ToBaseValue(target_->output_conversion()));
}
if (what(variables::kDataKeys)) {
base::ListValue keys;
diff --git a/src/gn/ninja_c_binary_target_writer_unittest.cc b/src/gn/ninja_c_binary_target_writer_unittest.cc
index f79b753..48ff9e4 100644
--- a/src/gn/ninja_c_binary_target_writer_unittest.cc
+++ b/src/gn/ninja_c_binary_target_writer_unittest.cc
@@ -175,7 +175,7 @@
"defines = -DBOOL_DEF -DINT_DEF=123 -DSTR_DEF=\\\"ABCD-1\\\"";
#endif
std::string out_str = out.str();
- EXPECT_TRUE(out_str.find(out_str) != std::string::npos);
+ EXPECT_TRUE(out_str.find(expectedSubstr) != std::string::npos);
}
TEST_F(NinjaCBinaryTargetWriterTest, StaticLibrary) {
diff --git a/src/gn/parse_tree.cc b/src/gn/parse_tree.cc
index a2f77bc..e1fc5a5 100644
--- a/src/gn/parse_tree.cc
+++ b/src/gn/parse_tree.cc
@@ -490,7 +490,7 @@
child.GetList().push_back(if_false_->GetJSONNode());
}
dict.SetKey(kJsonNodeChild, std::move(child));
- return std::move(dict);
+ return dict;
}
// FunctionCallNode -----------------------------------------------------------
diff --git a/src/gn/rust_values.h b/src/gn/rust_values.h
index d666f5a..9770e34 100644
--- a/src/gn/rust_values.h
+++ b/src/gn/rust_values.h
@@ -41,8 +41,7 @@
void set_crate_root(SourceFile& s) { crate_root_ = s; }
// Crate type for compilation.
- CrateType crate_type() { return crate_type_; }
- const CrateType crate_type() const { return crate_type_; }
+ CrateType crate_type() const { return crate_type_; }
void set_crate_type(CrateType s) { crate_type_ = s; }
// Any renamed dependencies for the `extern` flags.
diff --git a/src/gn/value.cc b/src/gn/value.cc
index 233a523..c3bb9b5 100644
--- a/src/gn/value.cc
+++ b/src/gn/value.cc
@@ -37,19 +37,19 @@
}
Value::Value(const ParseNode* origin, bool bool_val)
- : type_(BOOLEAN), boolean_value_(bool_val), origin_(origin) {}
+ : type_(BOOLEAN), origin_(origin), boolean_value_(bool_val) {}
Value::Value(const ParseNode* origin, int64_t int_val)
- : type_(INTEGER), int_value_(int_val), origin_(origin) {}
+ : type_(INTEGER), origin_(origin), int_value_(int_val) {}
Value::Value(const ParseNode* origin, std::string str_val)
- : type_(STRING), string_value_(std::move(str_val)), origin_(origin) {}
+ : type_(STRING), origin_(origin), string_value_(std::move(str_val)) {}
Value::Value(const ParseNode* origin, const char* str_val)
- : type_(STRING), string_value_(str_val), origin_(origin) {}
+ : type_(STRING), origin_(origin), string_value_(str_val) {}
Value::Value(const ParseNode* origin, std::unique_ptr<Scope> scope)
- : type_(SCOPE), scope_value_(std::move(scope)), origin_(origin) {}
+ : type_(SCOPE), origin_(origin), scope_value_(std::move(scope)) {}
Value::Value(const Value& other) : type_(other.type_), origin_(other.origin_) {
switch (type_) {
diff --git a/src/util/test/test.h b/src/util/test/test.h
index b5539d7..d3fc056 100644
--- a/src/util/test/test.h
+++ b/src/util/test/test.h
@@ -30,7 +30,6 @@
friend class TestResult;
bool failed_;
- int assertion_failures_;
};
extern testing::Test* g_current_test;
