C++ warning of narrowing conversion

[sturk]

Hi, why does the compiler prompt me of this warning and how do I resolve this? Stressing me out. And thing is, I can't modify the main() permanently, so I can't edit the code that is causing this issue in the main().

warning: narrowing conversion of ‘(size_t)((int)size)’ from ‘size_t {aka long unsigned int}’ to ‘float’ inside { } [-Wnarrowing]
std::vector<Factory::PointType> vertices{static_cast<size_t>(size)};

class Factory final
{
private:
	Factory() = delete;
	Factory(const Factory&) = delete;
	Factory(Factory&&) = delete;
	Factory& operator=(const Factory&) = delete;
	Factory& operator=(Factory&&) = delete;
 
public:
	typedef Object::VectorType VectorType;
	typedef Object::PointType PointType;
	typedef IRenderingInformation RenderingInformation;
	typedef uint8_t Byte;

	Factory(unsigned int width, unsigned int height, const VectorType& extents);
	~Factory();

	Object* createPolygon(const PointType* vertices, int size,
		unsigned color, float depth = 1);

	void writeToBitmap(const char* name);

	static char* readFromBitmap(unsigned int& width, unsigned int& height,
		const char* name);

	static unsigned getColor(Byte r, Byte g, Byte b);

	POINT toDevice(const VectorType& v) const;
	HDC getContext() const;

private:
	HBITMAP _bitmap;
	HDC _context;
	unsigned int _width;
	unsigned int _height;
	void* _bitmapData;
	float _xFactor;
	float _yFactor;
};
class RenderedPolygon : public Polygon, public IRenderingInformation
{
public:
	void render() const override;
	virtual Factory& getFactory() const
	{
		return _myFactory;
		
	}	
	virtual unsigned int getColor() const 
	{
		return color;
	}
	RenderedPolygon(const PointType* v, const int s,
	const unsigned c, const float d, Factory& factry) : Polygon(v,s,d),color(c),_myFactory(factry) {
		for(int i = 0; i < s; ++i)
		{
			pts.push_back( v[i] );
		}
		
	}
	RenderedPolygon* clone() const { return new RenderedPolygon(*this);}
 private:
	Factory &_myFactory;
	unsigned int color;	 
	std::vector<PointType> pts;
	
};
Factory::Factory(
	unsigned int width,
	unsigned int height,
	const VectorType& extents
) :
	_width{width},
	_height{height},
	_xFactor{static_cast<float>(width) / extents.x()},
	_yFactor{static_cast<float>(height) / extents.y()}
{
	_context = CreateCompatibleDC(0);
	BITMAPINFO bitmapInfo;
	ZeroMemory(&bitmapInfo, sizeof(bitmapInfo));
	bitmapInfo.bmiHeader.biSize = sizeof(bitmapInfo);
	bitmapInfo.bmiHeader.biWidth = width;
	bitmapInfo.bmiHeader.biHeight = height;
	bitmapInfo.bmiHeader.biPlanes = 1;
	bitmapInfo.bmiHeader.biBitCount = 24;
	bitmapInfo.bmiHeader.biCompression = BI_RGB;
	_bitmap = CreateDIBSection(_context, &bitmapInfo, DIB_RGB_COLORS,
		&_bitmapData, 0, 0
	);
	SelectObject(_context, _bitmap);
}

Factory::~Factory()
{
	HGDIOBJ old = SelectObject(_context, GetStockObject(NULL_BRUSH));
	DeleteObject(old);
	DeleteDC(_context);
	DeleteObject(_bitmap);
}

HDC Factory::getContext() const
{
	return _context;
}

unsigned Factory::getColor(Byte r, Byte g, Byte b)
{
	return RGB(r, g, b);
}

void Factory::writeToBitmap(const char* name)
{
	const unsigned int rowData = BYTES_PER_PIXEL * _width;
	const unsigned int rowPadding = (rowData % 4 != 0)
		? (4 - rowData % 4)
		: 0;
	const unsigned int stride = rowData + rowPadding;
	std::fstream out{name, std::ios_base::binary | std::ios_base::out};
	if (out)
	{
		BITMAPFILEHEADER bmfh;
		BITMAPINFOHEADER bmih;

		ZeroMemory(&bmfh, sizeof(bmfh));
		bmfh.bfType = 'B' + ('M' << 8);
		bmfh.bfSize = static_cast<unsigned int>(
			sizeof(bmfh) +
			sizeof(bmih) +
			stride * _height
		);
		bmfh.bfOffBits = sizeof(bmfh) + sizeof(bmih);
		out.write(reinterpret_cast<char*>(&bmfh), sizeof(bmfh));

		ZeroMemory(&bmih, sizeof(bmih));
		bmih.biSize = sizeof(bmih);
		bmih.biWidth = _width;
		bmih.biHeight = _height;
		bmih.biPlanes = 1;
		bmih.biBitCount = 8 * BYTES_PER_PIXEL;
		bmih.biCompression = BI_RGB;
		out.write(reinterpret_cast<char*>(&bmih), sizeof(bmih));

		for (unsigned int h = 0; h < _height; ++h)
		{
			out.write(
				reinterpret_cast<char*>(_bitmapData) + h * stride,
				rowData
			);
			if (stride > rowData)
			{
				out.write("000", stride - rowData);
			}
		}
	}
}

char* Factory::readFromBitmap(unsigned int& width, unsigned int& height,
	const char* name
)
{
	std::fstream in{name, std::ios_base::in | std::ios_base::binary};
	if (in)
	{
		char header[54];
		in.read(header, 54);
		width = *reinterpret_cast<unsigned int*>(header + 18);
		const int signedHeight = *reinterpret_cast<int*>(header + 22);
		height = static_cast<unsigned int>(std::abs(signedHeight));
		const unsigned int size = *reinterpret_cast<unsigned int*>(header + 34);
		const unsigned int stride = size / height;
		const unsigned int rowData = BYTES_PER_PIXEL * width;
		char* const data = new char[rowData * height];
		for (unsigned int i = 0; i < height; ++i)
		{
			in.read(data + rowData * i, rowData);
			if (stride > rowData)
			{
				char buffer[4];
				in.read(buffer, stride - rowData);
			}
		}
		return data;
	}
	else
	{
		return nullptr;
	}
}
Object* Factory::createPolygon(const PointType* vertices, int size,
	unsigned color, float depth
)
{
	return new RenderedPolygon(vertices, size,
		color, depth, *this);
}
float frand()
{
	return 10.0f * float(rand()) / float(RAND_MAX);
}


int main()
{
	srand(unsigned(time(0)));
	const int COUNT = 100;
	int count = 0;
	int max_count = 0;
	Factory factory(500, 500, Object::VectorType(1, 1));

	bool okay = true;
	// Object replication test
	float EPSILON = 1.0e-5f;
	for (int i = 0; okay && i < COUNT; ++i)
	{

		const int size = 3 + rand() % 10;
		std::vector<Factory::PointType> vertices{static_cast<size_t>(size)}; //This line
		for (int k = 0; k < size; ++k)
		{
			vertices[k] = Factory::PointType(frand(), frand());
		}
		const float depth = frand();
		const unsigned int color = rand() % (1 << 24);

		Object* obj = factory.createPolygon(vertices.data(), size,
			color, depth);
		Object* clone = obj->clone();

		class Polygon* polygon = dynamic_cast<class Polygon*>(clone);
		class Polygon* original = dynamic_cast<class Polygon*>(obj);
		okay &= (
			(polygon != nullptr) &&
			(std::abs(polygon->center() - original->center()) < EPSILON) &&
			(polygon->depth() == depth) &&
			(polygon->size() == size)
		);		
		
		for (int k = 0; okay && k < size; ++k)
		{
			okay &= std::abs((*polygon)[k] - vertices[k]) < EPSILON;
			std::cout<<std::abs((*polygon)[k] - vertices[k])<<" "<<size<<std::endl;
		}
		
		delete clone;
		delete obj;
	}
	if (okay)
	{
		++count;
	}
	else
	{
		std::cout << "  - failed replication test." << std::endl;
	}
	++max_count;
 
}

[Niccolo]

I can only surmize that PointType uses float, as that is the message.

Since you can't edit main, your only chance is to change PointType's use of float, or accept the warning.

This appears to be a warning the the initialization expects a float, which you might be able to modify in PointType, conduct a cast there.

[helios]

std::vector<Factory::PointType> vertices{static_cast<size_t>(size)};
You're not constructing an std::vector of size 'size'. You're constructing an std::vector of size 1 and constructing the first element by passing 'size' to the constructor of Factory::PointType.

What you meant to do was (note the parentheses)

std::vector<Factory::PointType> vertices(static_cast<size_t>(size));

Next time just write in the introduction the line number that causes the error, instead of adding a comment in the middle of ~250 lines of code.

[sturk]

@Niccolo, thing is, my PointType is already making use of a float. Could it be that the test file from my school is flawed?

using PointType = vector2d;

vector2d::vector2d(float x, float y) : std::complex<float>(x, y){ }

[keskiverto]

Could it be that the test file from my school is flawed?

Apparently.

std::vector has multiple constructors http://www.cplusplus.com/reference/vector/vector/vector/
and the brace syntax calls the initializer list constructor rather than the fill constructor.

The following loops will access nonexistent elements. We hope that the program will crash in huge ball of fire.


This leads to second "flaw": The loops use for (int k = 0; k < size; ++k)
If the intention of the loop is to iterate over the elements of a vector, then it is much safer to get the facts from the vector:
for ( size_t k = 0; k < vertices.size(); ++k )

In this case the intent is conveyed yet more clearly with the range-based loop syntax:

for ( auto& v : vertices )
{
  v = Factory::PointType( frand(), frand() );
}

[sturk]

Hi @keskiverto, thanks for the response. I have just consulted my teacher, and he said that it is meant to be used as a constructor with 1 parameter of type size_t, that also represents a count. It allows std::vector to preallocate memory of the required size. Running constructors for points in allocated memory and then overwriting these points is almost instant.

[dhayden]

helios wrote:

What you meant to do was (note the parentheses)

Helios, can you point me to a reference for this? I thought that the {} syntax just made the syntax for initialization consistent. I didn't realize that there were semantic differences.

Thanks.

[keskiverto]

The question is: Which version of C++?

https://mariusbancila.ro/blog/2017/04/13/cpp17-new-rules-for-auto-deduction-from-braced-init-list/
Shows that the rules change in C++17.

[dhayden]

Thanks Keskiverto. So it looks like the OP can solve the problem by telling the compiler to use a pre-C++14 standard. The code won't work properly with C++14 or later.

If I'm reading this right, We're dealing with the following case from https://en.cppreference.com/w/cpp/language/list_initialization:

Otherwise, the constructors of T are considered, in two phases:

* All constructors that take std::initializer_list as the only argument, or as the first argument if the remaining arguments have default values, are examined, and matched by overload resolution against a single argument of type std::initializer_list

* If the previous stage does not produce a match, all constructors of T participate in overload resolution against the set of arguments that consists of the elements of the braced-init-list, with the restriction that only non-narrowing conversions are allowed. If this stage produces an explicit constructor as the best match for a copy-list-initialization, compilation fails (note, in simple copy-initialization, explicit constructors are not considered at all).

In other words, the compiler looks for a matching constructor that takes an initializer_list first. If none is found, it looks at the other constructors.

For vector, the two constructors in question are:

explicit vector (size_type n, const allocator_type& alloc = allocator_type()); // and
vector (initializer_list<value_type> il,
       const allocator_type& alloc = allocator_type());

Since the second constructor matches (presumably because there's a PointType constructor like PointType::PointType(float x=0.0f, float y=0.0f)), that's the one that matches.

Have I got this right?