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;