From 74c7e67d60e2bdca7ffdf08e0719a6b9663343f1 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Mon, 5 May 2014 16:02:42 -0700 Subject: [PATCH] Various c++11 improvements - std::make_unique - avoiding uneccessary std::move calls which make actually prevent rvo (http://stackoverflow.com/questions/4986673/c11-rvalues-and-move-semantics-confusion) - more c++11 for loops --- include/mapnik/util/geometry_to_wkb.hpp | 9 +- include/mapnik/xml_attribute_cast.hpp | 2 +- src/color_factory.cpp | 2 +- src/load_map.cpp | 158 ++++++++++-------------- src/map.cpp | 8 +- src/xml_tree.cpp | 2 +- 6 files changed, 80 insertions(+), 101 deletions(-) diff --git a/include/mapnik/util/geometry_to_wkb.hpp b/include/mapnik/util/geometry_to_wkb.hpp index f16b81395..70afb69c1 100644 --- a/include/mapnik/util/geometry_to_wkb.hpp +++ b/include/mapnik/util/geometry_to_wkb.hpp @@ -24,6 +24,7 @@ #define MAPNIK_GEOMETRY_TO_WKB_HPP // mapnik +#include #include #include @@ -141,7 +142,7 @@ wkb_buffer_ptr to_point_wkb( GeometryType const& g, wkbByteOrder byte_order) { assert(g.size() == 1); std::size_t size = 1 + 4 + 8*2 ; // byteOrder + wkbType + Point - wkb_buffer_ptr wkb(new wkb_buffer(size)); + wkb_buffer_ptr wkb = std::make_unique(size); wkb_stream ss(wkb->buffer(), wkb->size()); ss.write(reinterpret_cast(&byte_order),1); int type = static_cast(mapnik::geometry_type::types::Point); @@ -161,7 +162,7 @@ wkb_buffer_ptr to_line_string_wkb( GeometryType const& g, wkbByteOrder byte_orde unsigned num_points = g.size(); assert(num_points > 1); std::size_t size = 1 + 4 + 4 + 8*2*num_points ; // byteOrder + wkbType + numPoints + Point*numPoints - wkb_buffer_ptr wkb(new wkb_buffer(size)); + wkb_buffer_ptr wkb = std::make_unique(size); wkb_stream ss(wkb->buffer(), wkb->size()); ss.write(reinterpret_cast(&byte_order),1); int type = static_cast(mapnik::geometry_type::types::LineString); @@ -209,7 +210,7 @@ wkb_buffer_ptr to_polygon_wkb( GeometryType const& g, wkbByteOrder byte_order) } } unsigned num_rings = rings.size(); - wkb_buffer_ptr wkb(new wkb_buffer(size)); + wkb_buffer_ptr wkb = std::make_unique(size); wkb_stream ss(wkb->buffer(), wkb->size()); ss.write(reinterpret_cast(&byte_order),1); int type = static_cast(mapnik::geometry_type::types::Polygon); @@ -281,7 +282,7 @@ wkb_buffer_ptr to_wkb(geometry_container const& paths, wkbByteOrder byte_order ) wkb_cont.push_back(std::move(wkb)); } - wkb_buffer_ptr multi_wkb( new wkb_buffer(multi_size)); + wkb_buffer_ptr multi_wkb = std::make_unique(multi_size); wkb_stream ss(multi_wkb->buffer(), multi_wkb->size()); ss.write(reinterpret_cast(&byte_order),1); multi_type = collection ? 7 : multi_type + 3; diff --git a/include/mapnik/xml_attribute_cast.hpp b/include/mapnik/xml_attribute_cast.hpp index d5d527972..f74041828 100644 --- a/include/mapnik/xml_attribute_cast.hpp +++ b/include/mapnik/xml_attribute_cast.hpp @@ -188,7 +188,7 @@ struct do_xml_attribute_cast else { mapnik::expression_ptr expr = parse_expression(source, tree.expr_grammar); - tree.expr_cache_.insert(std::move(std::make_pair(source,expr))); + tree.expr_cache_.insert(std::make_pair(source,expr)); return expr; } } diff --git a/src/color_factory.cpp b/src/color_factory.cpp index f183ec3b6..3db62b50e 100644 --- a/src/color_factory.cpp +++ b/src/color_factory.cpp @@ -46,7 +46,7 @@ color parse_color(std::string const& str, c); if (result && (first == last)) { - return std::move(c); + return c; } else { diff --git a/src/load_map.cpp b/src/load_map.cpp index a50b68c4b..d77fb603c 100644 --- a/src/load_map.cpp +++ b/src/load_map.cpp @@ -293,14 +293,13 @@ void map_parser::parse_map(Map & map, xml_node const& pt, std::string const& bas unsigned i = 0; bool success = false; int n[3]; - for (boost::tokenizer >::iterator beg = tokens.begin(); - beg != tokens.end(); ++beg) + for (auto const& beg : tokens) { - std::string item = mapnik::util::trim_copy(*beg); + std::string item = mapnik::util::trim_copy(beg); if (!mapnik::util::string2int(item,n[i])) { throw config_error(std::string("Invalid version string encountered: '") - + *beg + "' in '" + *min_version_string + "'"); + + beg + "' in '" + *min_version_string + "'"); } if (i==2) { @@ -340,89 +339,75 @@ void map_parser::parse_map_include(Map & map, xml_node const& include) { try { - xml_node::const_iterator itr = include.begin(); - xml_node::const_iterator end = include.end(); - - for (; itr != end; ++itr) + for (auto const& n : include) { - if (itr->is_text()) continue; - if (itr->is("Include")) + if (n.is_text()) continue; + if (n.is("Include")) { - parse_map_include(map, *itr); + parse_map_include(map, n); } - else if (itr->is("Style")) + else if (n.is("Style")) { - parse_style(map, *itr); + parse_style(map, n); } - else if (itr->is("Layer")) + else if (n.is("Layer")) { - parse_layer(map, *itr); + parse_layer(map, n); } - else if (itr->is("FontSet")) + else if (n.is("FontSet")) { - parse_fontset(map, *itr); + parse_fontset(map, n); } - else if (itr->is("FileSource")) + else if (n.is("FileSource")) { - std::string name = itr->get_attr("name"); - std::string value = itr->get_text(); - file_sources_[std::move(name)] = std::move(value); + file_sources_[n.get_attr("name")] = n.get_text(); } - else if (itr->is("Datasource")) + else if (n.is("Datasource")) { - std::string name = itr->get_attr("name", std::string("Unnamed")); + std::string name = n.get_attr("name", std::string("Unnamed")); parameters params; - xml_node::const_iterator paramIter = itr->begin(); - xml_node::const_iterator endParam = itr->end(); - for (; paramIter != endParam; ++paramIter) + for (auto const& p: n) { - if (paramIter->is("Parameter")) + if (p.is("Parameter")) { - std::string param_name = paramIter->get_attr("name"); - std::string value = paramIter->get_text(); - params[std::move(param_name)] = std::move(value); + params[p.get_attr("name")] = p.get_text(); } } - datasource_templates_[name] = std::move(params); + datasource_templates_[std::move(name)] = std::move(params); } - else if (itr->is("Parameters")) + else if (n.is("Parameters")) { parameters & params = map.get_extra_parameters(); - xml_node::const_iterator paramIter = itr->begin(); - xml_node::const_iterator endParam = itr->end(); - for (; paramIter != endParam; ++paramIter) + for (auto const& p: n) { - if (paramIter->is("Parameter")) + if (p.is("Parameter")) { - std::string name = paramIter->get_attr("name"); bool is_string = true; - boost::optional type = paramIter->get_opt_attr("type"); + boost::optional type = p.get_opt_attr("type"); if (type) { if (*type == "int") { is_string = false; - mapnik::value_integer value = paramIter->get_value(); - params[std::move(name)] = std::move(value); + params[p.get_attr("name")] = p.get_value(); } else if (*type == "float") { is_string = false; - double value = paramIter->get_value(); - params[std::move(name)] = std::move(value); + params[p.get_attr("name")] = p.get_value(); } } - if (is_string) { - std::string value = paramIter->get_text(); - params[std::move(name)] = std::move(value); + params[p.get_attr("name")] = p.get_text(); } } } } } - } catch (config_error const& ex) { + } + catch (config_error const& ex) + { ex.append_context(include); throw; } @@ -526,15 +511,12 @@ void map_parser::parse_fontset(Map & map, xml_node const& fset) { name = fset.get_attr("name"); font_set fontset(name); - xml_node::const_iterator itr = fset.begin(); - xml_node::const_iterator end = fset.end(); - bool success = false; - for (; itr != end; ++itr) + for (auto const& n: fset) { - if (itr->is("Font")) + if (n.is("Font")) { - if (parse_font(fontset, *itr)) + if (parse_font(fontset, n)) { success = true; } @@ -551,7 +533,7 @@ void map_parser::parse_fontset(Map & map, xml_node const& fset) // XXX Hack because map object isn't accessible by text_symbolizer // when it's parsed - fontsets_.insert(std::move(std::make_pair(name, fontset))); + fontsets_.insert(std::make_pair(name, fontset)); } catch (config_error const& ex) { @@ -679,15 +661,12 @@ void map_parser::parse_layer(Map & map, xml_node const& node) } } - xml_node::const_iterator child = node.begin(); - xml_node::const_iterator end = node.end(); - - for(; child != end; ++child) + for (auto const& child: node) { - if (child->is("StyleName")) + if (child.is("StyleName")) { - std::string style_name = child->get_text(); + std::string style_name = child.get_text(); if (style_name.empty()) { std::string ss("StyleName is empty in Layer: '"); @@ -706,14 +685,14 @@ void map_parser::parse_layer(Map & map, xml_node const& node) lyr.add_style(style_name); } } - else if (child->is("Datasource")) + else if (child.is("Datasource")) { parameters params; - optional base = child->get_opt_attr("base"); + optional base = child.get_opt_attr("base"); if(base) { std::map::const_iterator base_itr = datasource_templates_.find(*base); - if (base_itr!=datasource_templates_.end()) + if (base_itr != datasource_templates_.end()) { params = base_itr->second; } @@ -724,26 +703,24 @@ void map_parser::parse_layer(Map & map, xml_node const& node) } } - xml_node::const_iterator paramIter = child->begin(); - xml_node::const_iterator endParam = child->end(); - for (; paramIter != endParam; ++paramIter) + for (auto const& n : child) { - if (paramIter->is("Parameter")) + if (n.is("Parameter")) { - std::string param_name = paramIter->get_attr("name"); - std::string value = paramIter->get_text(); - params[param_name] = value; + params[n.get_attr("name")] = n.get_text(); } } boost::optional base_param = params.get("base"); boost::optional file_param = params.get("file"); - if (base_param){ + if (base_param) + { params["base"] = ensure_relative_to_xml(base_param); } - else if (file_param){ + else if (file_param) + { params["file"] = ensure_relative_to_xml(file_param); } @@ -1261,7 +1238,8 @@ void map_parser::parse_shield_symbolizer(rule & rule, xml_node const& sym) { text_placements_ptr placement_finder; optional placement_type = sym.get_opt_attr("placement-type"); - if (placement_type) { + if (placement_type) + { placement_finder = placements::registry::instance().from_xml(*placement_type, sym, fontsets_); } else { placement_finder = std::make_shared(); @@ -1649,7 +1627,8 @@ bool map_parser::parse_raster_colorizer(raster_colorizer_ptr const& rc, colorizer_mode default_mode = node.get_attr("default-mode", COLORIZER_LINEAR); - if(default_mode == COLORIZER_INHERIT) { + if(default_mode == COLORIZER_INHERIT) + { throw config_error("RasterColorizer mode must not be INHERIT. "); } rc->set_default_mode(default_mode); @@ -1666,47 +1645,44 @@ bool map_parser::parse_raster_colorizer(raster_colorizer_ptr const& rc, optional eps = node.get_opt_attr("epsilon"); if (eps) { - if(*eps < 0) { + if(*eps < 0) + { throw config_error("RasterColorizer epsilon must be > 0. "); } rc->set_epsilon(*eps); } - - xml_node::const_iterator stopIter = node.begin(); - xml_node::const_iterator endStop = node.end(); float maximumValue = -std::numeric_limits::max(); - - for(; stopIter != endStop; ++stopIter) + for (auto const& n : node) { - if (stopIter->is("stop")) + if (n.is("stop")) { found_stops = true; // colour is optional. - optional stopcolor = stopIter->get_opt_attr("color"); - if (!stopcolor) { + optional stopcolor = n.get_opt_attr("color"); + if (!stopcolor) + { *stopcolor = *default_color; } // mode default to INHERIT - colorizer_mode mode = - stopIter->get_attr("mode", COLORIZER_INHERIT); + colorizer_mode mode = n.get_attr("mode", COLORIZER_INHERIT); // value is required, and it must be bigger than the previous - optional value = - stopIter->get_opt_attr("value"); + optional value = n.get_opt_attr("value"); - if(!value) { + if(!value) + { throw config_error("stop tag missing value"); } - if(value < maximumValue) { + if(value < maximumValue) + { throw config_error("stop tag values must be in ascending order"); } maximumValue = *value; - optional label = - stopIter->get_opt_attr("label"); + optional label = n.get_opt_attr("label"); //append the stop colorizer_stop tmpStop; @@ -1714,7 +1690,9 @@ bool map_parser::parse_raster_colorizer(raster_colorizer_ptr const& rc, tmpStop.set_mode(mode); tmpStop.set_value(*value); if (label) + { tmpStop.set_label(*label); + } rc->add_stop(tmpStop); } diff --git a/src/map.cpp b/src/map.cpp index c11314d2e..95a020b20 100644 --- a/src/map.cpp +++ b/src/map.cpp @@ -140,12 +140,12 @@ Map::const_style_iterator Map::end_styles() const // TODO(dane) - only kept for python bindings, can we avoid needing? bool Map::insert_style(std::string const& name,feature_type_style const& style) { - return styles_.insert(std::move(make_pair(name,style))).second; + return styles_.insert(make_pair(name,style)).second; } bool Map::insert_style(std::string const& name,feature_type_style && style) { - return styles_.insert(std::move(make_pair(name,std::move(style)))).second; + return styles_.insert(make_pair(name, std::move(style))).second; } void Map::remove_style(std::string const& name) @@ -168,7 +168,7 @@ bool Map::insert_fontset(std::string const& name, font_set const& fontset) { throw mapnik::config_error("Fontset name must match the name used to reference it on the map"); } - return fontsets_.insert(std::move(make_pair(name, fontset))).second; + return fontsets_.insert(make_pair(name, fontset)).second; } bool Map::insert_fontset(std::string const& name, font_set && fontset) @@ -177,7 +177,7 @@ bool Map::insert_fontset(std::string const& name, font_set && fontset) { throw mapnik::config_error("Fontset name must match the name used to reference it on the map"); } - return fontsets_.insert(std::move(make_pair(name, std::move(fontset)))).second; + return fontsets_.insert(make_pair(name, std::move(fontset))).second; } boost::optional Map::find_fontset(std::string const& name) const diff --git a/src/xml_tree.cpp b/src/xml_tree.cpp index 0d2d31d3a..41911636e 100644 --- a/src/xml_tree.cpp +++ b/src/xml_tree.cpp @@ -230,7 +230,7 @@ xml_node &xml_node::add_child(std::string && name, unsigned line, bool is_text) void xml_node::add_attribute(const char * name, const char * value) { - attributes_.insert(std::move(std::make_pair(name,std::move(xml_attribute(value))))); + attributes_.insert(std::make_pair(name,xml_attribute(value))); } xml_node::attribute_map const& xml_node::get_attributes() const