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
This commit is contained in:
Dane Springmeyer 2014-05-05 16:02:42 -07:00
parent 439fc9f700
commit 74c7e67d60
6 changed files with 80 additions and 101 deletions

View file

@ -24,6 +24,7 @@
#define MAPNIK_GEOMETRY_TO_WKB_HPP #define MAPNIK_GEOMETRY_TO_WKB_HPP
// mapnik // mapnik
#include <mapnik/std.hpp>
#include <mapnik/global.hpp> #include <mapnik/global.hpp>
#include <mapnik/geometry.hpp> #include <mapnik/geometry.hpp>
@ -141,7 +142,7 @@ wkb_buffer_ptr to_point_wkb( GeometryType const& g, wkbByteOrder byte_order)
{ {
assert(g.size() == 1); assert(g.size() == 1);
std::size_t size = 1 + 4 + 8*2 ; // byteOrder + wkbType + Point 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<wkb_buffer>(size);
wkb_stream ss(wkb->buffer(), wkb->size()); wkb_stream ss(wkb->buffer(), wkb->size());
ss.write(reinterpret_cast<char*>(&byte_order),1); ss.write(reinterpret_cast<char*>(&byte_order),1);
int type = static_cast<int>(mapnik::geometry_type::types::Point); int type = static_cast<int>(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(); unsigned num_points = g.size();
assert(num_points > 1); assert(num_points > 1);
std::size_t size = 1 + 4 + 4 + 8*2*num_points ; // byteOrder + wkbType + numPoints + Point*numPoints 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<wkb_buffer>(size);
wkb_stream ss(wkb->buffer(), wkb->size()); wkb_stream ss(wkb->buffer(), wkb->size());
ss.write(reinterpret_cast<char*>(&byte_order),1); ss.write(reinterpret_cast<char*>(&byte_order),1);
int type = static_cast<int>(mapnik::geometry_type::types::LineString); int type = static_cast<int>(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(); unsigned num_rings = rings.size();
wkb_buffer_ptr wkb(new wkb_buffer(size)); wkb_buffer_ptr wkb = std::make_unique<wkb_buffer>(size);
wkb_stream ss(wkb->buffer(), wkb->size()); wkb_stream ss(wkb->buffer(), wkb->size());
ss.write(reinterpret_cast<char*>(&byte_order),1); ss.write(reinterpret_cast<char*>(&byte_order),1);
int type = static_cast<int>(mapnik::geometry_type::types::Polygon); int type = static_cast<int>(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_cont.push_back(std::move(wkb));
} }
wkb_buffer_ptr multi_wkb( new wkb_buffer(multi_size)); wkb_buffer_ptr multi_wkb = std::make_unique<wkb_buffer>(multi_size);
wkb_stream ss(multi_wkb->buffer(), multi_wkb->size()); wkb_stream ss(multi_wkb->buffer(), multi_wkb->size());
ss.write(reinterpret_cast<char*>(&byte_order),1); ss.write(reinterpret_cast<char*>(&byte_order),1);
multi_type = collection ? 7 : multi_type + 3; multi_type = collection ? 7 : multi_type + 3;

View file

@ -188,7 +188,7 @@ struct do_xml_attribute_cast<mapnik::expression_ptr>
else else
{ {
mapnik::expression_ptr expr = parse_expression(source, tree.expr_grammar); 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; return expr;
} }
} }

View file

@ -46,7 +46,7 @@ color parse_color(std::string const& str,
c); c);
if (result && (first == last)) if (result && (first == last))
{ {
return std::move(c); return c;
} }
else else
{ {

View file

@ -293,14 +293,13 @@ void map_parser::parse_map(Map & map, xml_node const& pt, std::string const& bas
unsigned i = 0; unsigned i = 0;
bool success = false; bool success = false;
int n[3]; int n[3];
for (boost::tokenizer<boost::char_separator<char> >::iterator beg = tokens.begin(); for (auto const& beg : tokens)
beg != tokens.end(); ++beg)
{ {
std::string item = mapnik::util::trim_copy(*beg); std::string item = mapnik::util::trim_copy(beg);
if (!mapnik::util::string2int(item,n[i])) if (!mapnik::util::string2int(item,n[i]))
{ {
throw config_error(std::string("Invalid version string encountered: '") throw config_error(std::string("Invalid version string encountered: '")
+ *beg + "' in '" + *min_version_string + "'"); + beg + "' in '" + *min_version_string + "'");
} }
if (i==2) if (i==2)
{ {
@ -340,89 +339,75 @@ void map_parser::parse_map_include(Map & map, xml_node const& include)
{ {
try try
{ {
xml_node::const_iterator itr = include.begin(); for (auto const& n : include)
xml_node::const_iterator end = include.end();
for (; itr != end; ++itr)
{ {
if (itr->is_text()) continue; if (n.is_text()) continue;
if (itr->is("Include")) 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<std::string>("name"); file_sources_[n.get_attr<std::string>("name")] = n.get_text();
std::string value = itr->get_text();
file_sources_[std::move(name)] = std::move(value);
} }
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; parameters params;
xml_node::const_iterator paramIter = itr->begin(); for (auto const& p: n)
xml_node::const_iterator endParam = itr->end();
for (; paramIter != endParam; ++paramIter)
{ {
if (paramIter->is("Parameter")) if (p.is("Parameter"))
{ {
std::string param_name = paramIter->get_attr<std::string>("name"); params[p.get_attr<std::string>("name")] = p.get_text();
std::string value = paramIter->get_text();
params[std::move(param_name)] = std::move(value);
} }
} }
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(); parameters & params = map.get_extra_parameters();
xml_node::const_iterator paramIter = itr->begin(); for (auto const& p: n)
xml_node::const_iterator endParam = itr->end();
for (; paramIter != endParam; ++paramIter)
{ {
if (paramIter->is("Parameter")) if (p.is("Parameter"))
{ {
std::string name = paramIter->get_attr<std::string>("name");
bool is_string = true; bool is_string = true;
boost::optional<std::string> type = paramIter->get_opt_attr<std::string>("type"); boost::optional<std::string> type = p.get_opt_attr<std::string>("type");
if (type) if (type)
{ {
if (*type == "int") if (*type == "int")
{ {
is_string = false; is_string = false;
mapnik::value_integer value = paramIter->get_value<mapnik::value_integer>(); params[p.get_attr<std::string>("name")] = p.get_value<mapnik::value_integer>();
params[std::move(name)] = std::move(value);
} }
else if (*type == "float") else if (*type == "float")
{ {
is_string = false; is_string = false;
double value = paramIter->get_value<mapnik::value_double>(); params[p.get_attr<std::string>("name")] = p.get_value<mapnik::value_double>();
params[std::move(name)] = std::move(value);
} }
} }
if (is_string) if (is_string)
{ {
std::string value = paramIter->get_text(); params[p.get_attr<std::string>("name")] = p.get_text();
params[std::move(name)] = std::move(value);
} }
} }
} }
} }
} }
} catch (config_error const& ex) { }
catch (config_error const& ex)
{
ex.append_context(include); ex.append_context(include);
throw; throw;
} }
@ -526,15 +511,12 @@ void map_parser::parse_fontset(Map & map, xml_node const& fset)
{ {
name = fset.get_attr<std::string>("name"); name = fset.get_attr<std::string>("name");
font_set fontset(name); font_set fontset(name);
xml_node::const_iterator itr = fset.begin();
xml_node::const_iterator end = fset.end();
bool success = false; 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; 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 // XXX Hack because map object isn't accessible by text_symbolizer
// when it's parsed // 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) 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(); for (auto const& child: node)
xml_node::const_iterator end = node.end();
for(; child != end; ++child)
{ {
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()) if (style_name.empty())
{ {
std::string ss("StyleName is empty in Layer: '"); 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); lyr.add_style(style_name);
} }
} }
else if (child->is("Datasource")) else if (child.is("Datasource"))
{ {
parameters params; parameters params;
optional<std::string> base = child->get_opt_attr<std::string>("base"); optional<std::string> base = child.get_opt_attr<std::string>("base");
if(base) if(base)
{ {
std::map<std::string,parameters>::const_iterator base_itr = datasource_templates_.find(*base); std::map<std::string,parameters>::const_iterator base_itr = datasource_templates_.find(*base);
if (base_itr!=datasource_templates_.end()) if (base_itr != datasource_templates_.end())
{ {
params = base_itr->second; 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(); for (auto const& n : child)
xml_node::const_iterator endParam = child->end();
for (; paramIter != endParam; ++paramIter)
{ {
if (paramIter->is("Parameter")) if (n.is("Parameter"))
{ {
std::string param_name = paramIter->get_attr<std::string>("name"); params[n.get_attr<std::string>("name")] = n.get_text();
std::string value = paramIter->get_text();
params[param_name] = value;
} }
} }
boost::optional<std::string> base_param = params.get<std::string>("base"); boost::optional<std::string> base_param = params.get<std::string>("base");
boost::optional<std::string> file_param = params.get<std::string>("file"); boost::optional<std::string> file_param = params.get<std::string>("file");
if (base_param){ if (base_param)
{
params["base"] = ensure_relative_to_xml(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); 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; text_placements_ptr placement_finder;
optional<std::string> placement_type = sym.get_opt_attr<std::string>("placement-type"); optional<std::string> placement_type = sym.get_opt_attr<std::string>("placement-type");
if (placement_type) { if (placement_type)
{
placement_finder = placements::registry::instance().from_xml(*placement_type, sym, fontsets_); placement_finder = placements::registry::instance().from_xml(*placement_type, sym, fontsets_);
} else { } else {
placement_finder = std::make_shared<text_placements_dummy>(); placement_finder = std::make_shared<text_placements_dummy>();
@ -1649,7 +1627,8 @@ bool map_parser::parse_raster_colorizer(raster_colorizer_ptr const& rc,
colorizer_mode default_mode = colorizer_mode default_mode =
node.get_attr<colorizer_mode>("default-mode", COLORIZER_LINEAR); node.get_attr<colorizer_mode>("default-mode", COLORIZER_LINEAR);
if(default_mode == COLORIZER_INHERIT) { if(default_mode == COLORIZER_INHERIT)
{
throw config_error("RasterColorizer mode must not be INHERIT. "); throw config_error("RasterColorizer mode must not be INHERIT. ");
} }
rc->set_default_mode(default_mode); rc->set_default_mode(default_mode);
@ -1666,47 +1645,44 @@ bool map_parser::parse_raster_colorizer(raster_colorizer_ptr const& rc,
optional<float> eps = node.get_opt_attr<float>("epsilon"); optional<float> eps = node.get_opt_attr<float>("epsilon");
if (eps) if (eps)
{ {
if(*eps < 0) { if(*eps < 0)
{
throw config_error("RasterColorizer epsilon must be > 0. "); throw config_error("RasterColorizer epsilon must be > 0. ");
} }
rc->set_epsilon(*eps); rc->set_epsilon(*eps);
} }
xml_node::const_iterator stopIter = node.begin();
xml_node::const_iterator endStop = node.end();
float maximumValue = -std::numeric_limits<float>::max(); float maximumValue = -std::numeric_limits<float>::max();
for (auto const& n : node)
for(; stopIter != endStop; ++stopIter)
{ {
if (stopIter->is("stop")) if (n.is("stop"))
{ {
found_stops = true; found_stops = true;
// colour is optional. // colour is optional.
optional<color> stopcolor = stopIter->get_opt_attr<color>("color"); optional<color> stopcolor = n.get_opt_attr<color>("color");
if (!stopcolor) { if (!stopcolor)
{
*stopcolor = *default_color; *stopcolor = *default_color;
} }
// mode default to INHERIT // mode default to INHERIT
colorizer_mode mode = colorizer_mode mode = n.get_attr<colorizer_mode>("mode", COLORIZER_INHERIT);
stopIter->get_attr<colorizer_mode>("mode", COLORIZER_INHERIT);
// value is required, and it must be bigger than the previous // value is required, and it must be bigger than the previous
optional<float> value = optional<float> value = n.get_opt_attr<float>("value");
stopIter->get_opt_attr<float>("value");
if(!value) { if(!value)
{
throw config_error("stop tag missing 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"); throw config_error("stop tag values must be in ascending order");
} }
maximumValue = *value; maximumValue = *value;
optional<std::string> label = optional<std::string> label = n.get_opt_attr<std::string>("label");
stopIter->get_opt_attr<std::string>("label");
//append the stop //append the stop
colorizer_stop tmpStop; colorizer_stop tmpStop;
@ -1714,7 +1690,9 @@ bool map_parser::parse_raster_colorizer(raster_colorizer_ptr const& rc,
tmpStop.set_mode(mode); tmpStop.set_mode(mode);
tmpStop.set_value(*value); tmpStop.set_value(*value);
if (label) if (label)
{
tmpStop.set_label(*label); tmpStop.set_label(*label);
}
rc->add_stop(tmpStop); rc->add_stop(tmpStop);
} }

View file

@ -140,12 +140,12 @@ Map::const_style_iterator Map::end_styles() const
// TODO(dane) - only kept for python bindings, can we avoid needing? // TODO(dane) - only kept for python bindings, can we avoid needing?
bool Map::insert_style(std::string const& name,feature_type_style const& style) 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) 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) 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"); 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) 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"); 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<font_set const&> Map::find_fontset(std::string const& name) const boost::optional<font_set const&> Map::find_fontset(std::string const& name) const

View file

@ -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) 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 xml_node::attribute_map const& xml_node::get_attributes() const