Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(18)

Unified Diff: tools/clang/value_cleanup/tests/list-value-append-expected.cc

Issue 2056153003: value_cleanup: Rewrite more base::ListValue::Append(base::Value*) calls. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: feedback Created 4 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: tools/clang/value_cleanup/tests/list-value-append-expected.cc
diff --git a/tools/clang/value_cleanup/tests/list-value-append-expected.cc b/tools/clang/value_cleanup/tests/list-value-append-expected.cc
index ad347f43baf5be71111139c38c22534fee4a1f1d..1de3ff61d8fd37138903ba2f3cfea0ea6f26bcb6 100644
--- a/tools/clang/value_cleanup/tests/list-value-append-expected.cc
+++ b/tools/clang/value_cleanup/tests/list-value-append-expected.cc
@@ -8,10 +8,17 @@
#define true true
+base::ListValue* ReturnsRawPtr() {
+ return nullptr;
+}
+
std::unique_ptr<base::Value> ReturnsUniquePtr() {
return nullptr;
}
+// The joy of raw pointers.
+void DoesItTakeOwnership(base::Value*) {}
+
struct Thing {
std::unique_ptr<base::Value> ToValue() { return nullptr; }
};
@@ -30,3 +37,73 @@ void F() {
std::unique_ptr<base::Value> unique_ptr_var;
list.Append(std::move(unique_ptr_var));
}
+
+void G(base::Value* input) {
+ base::ListValue list;
+
+ std::unique_ptr<base::ListValue> local(new base::ListValue());
+ // Not rewritten, since it often makes more sense to change the function
+ // prototype.
+ local->Append(input);
+ // Should be rewritten: it will only be moved after it's no longer referenced.
+ list.Append(std::move(local));
+
+ // Not rewritten, since it would be used after it's moved. In theory, we could
+ // automatically handle this too, but the risk of accidentally breaking
+ // something is much higher.
+ base::ListValue* clever_list = new base::ListValue;
+ list.Append(clever_list);
+ clever_list->AppendInteger(2);
+
+ // Not rewritten, since it often makes more sense to change the function
+ // prototype.
+ base::Value* returned_value = ReturnsRawPtr();
+ list.Append(returned_value);
+
+ // Should be rewritten. The reassignment should be transformed into
+ // .reset().
+ std::unique_ptr<base::ListValue> reused_list(new base::ListValue);
+ reused_list->AppendInteger(1);
+ list.Append(std::move(reused_list));
+ reused_list.reset(new base::ListValue);
+ reused_list->AppendInteger(3);
+ list.Append(std::move(reused_list));
+
+ // This shouldn't be rewritten, since the reassignment is the return
+ // value of a function.
+ base::ListValue* reused_list_2 = new base::ListValue;
+ reused_list_2->AppendInteger(4);
+ list.Append(reused_list_2);
+ reused_list_2 = ReturnsRawPtr();
+ reused_list_2->AppendInteger(5);
+ list.Append(reused_list_2);
+
+ // auto should be expanded to a std::unique_ptr containing the deduced type.
+ std::unique_ptr<class base::ListValue> auto_list(new base::ListValue);
+ auto_list->AppendInteger(6);
+ list.Append(std::move(auto_list));
+
+ std::unique_ptr<class base::ListValue> auto_list_2(new base::ListValue);
+ auto_list_2->AppendInteger(7);
+ list.Append(std::move(auto_list_2));
+
+ // Shouldn't be rewritten: a raw pointer is passed to a function which may or
+ // may not take ownership.
+ base::ListValue* maybe_owned_list = new base::ListValue;
+ DoesItTakeOwnership(maybe_owned_list);
+ list.Append(maybe_owned_list);
+
+ // Should be rewritten, even though it doesn't have an initializer.
+ std::unique_ptr<base::ListValue> list_with_no_initializer;
+ list_with_no_initializer.reset(new base::ListValue);
+ list.Append(std::move(list_with_no_initializer));
+
+ // Make sure C++98 style initialization is correctly handled.
+ std::unique_ptr<base::ListValue> cxx98_list(new base::ListValue);
+ list.Append(std::move(cxx98_list));
+
+ // C++11 style syntax currently causes the tool to bail out: this is banned in
+ // Chromium style anyway.
+ base::ListValue* cxx11_list{new base::ListValue};
+ list.Append(cxx11_list);
+}
« no previous file with comments | « tools/clang/value_cleanup/ListValueRewriter.cpp ('k') | tools/clang/value_cleanup/tests/list-value-append-original.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698