Compiler Bug or User Error?

[LB]

I'me getting some strange errors with some code here. I am not sure if it's a compiler bug (this error is associated with bugs in VC++ 6.0 and up) or my fault. It only seems to be targeting some lines and not all that use the code.

{
	string FilePath = StrParam();

	ofstream Save;
	Save.open(FilePath.c_str(), ios_base::out|ios_base::binary|ios_base::trunc/*|ios_base::app*/);
	if(!Save.is_open())
	{
		//rdPtr->ErrorString = "Could open file \"";
		//rdPtr->ErrorString += FilePath;
		//rdPtr->ErrorString += "\" for save output.";
		//rdPtr->rRd->GenerateEvent(/*ERROR*/);
		Save.close();
		return;
	}
	if(Save.fail())
	{
		//rdPtr->ErrorString = "Could not save to file \"";
		//rdPtr->ErrorString += FilePath;
		//rdPtr->ErrorString += "\"";
		//rdPtr->rRd->GenerateEvent(/*ERROR*/);
		Save.close();
		return;
	}

	/* ******* Helpful Stuff ******* */
	#define Write(Data,Size) Save.write((char *)&(Data),(Size)) //Write some Data of any kind
	#define Term() Save.put('\0') //Terminate with '\0'

	/* ******* Save Code!! ******* */
	const string Identifier = "Timeline";
	const unsigned char Version = 0;
/**/	Write(Identifier.c_str(), Identifier.length()); //8 character ID
	Write(Version, sizeof(unsigned char)); //Version Number

/**/	Write(rdPtr->Timeline->size(), sizeof(unsigned long)); //Number of Positions
	for(map<unsigned long, vector<Event> >::iterator EvtIt = rdPtr->Timeline->begin(); EvtIt != rdPtr->Timeline->end(); ++EvtIt) //For Each Position
	{
		Write(EvtIt->first, sizeof(unsigned long)); //Current Position
/**/		Write(EvtIt->second.size(), sizeof(unsigned long)); //Number of Events
		for(unsigned long i = 0; i < EvtIt->second.size(); ++i) //For Each Event
		{
/**/			Write(EvtIt->second[i].Name.c_str(), EvtIt->second[i].Name.size()); //Event Name
			Term(); //End String
/**/			Write(EvtIt->second[i].Values.size(), sizeof(unsigned long)); //Number of Values
			for(map<string, signed long>::iterator ValIt = EvtIt->second[i].Values.begin(); ValIt != EvtIt->second[i].Values.end(); ++ValIt) //For Each Value
			{
/**/				Write(ValIt->first.c_str(), ValIt->first.size()); //Value Name (As String)
				Term(); //End String
				Write(ValIt->second, sizeof(signed long)); //Value of Value
			}

/**/			Write(EvtIt->second[i].Strings.size(), sizeof(unsigned long)); //Number of Strings
			for(map<string, string>::iterator StrIt = EvtIt->second[i].Strings.begin(); StrIt != EvtIt->second[i].Strings.end(); ++StrIt) //For Each String
			{
/**/				Write(StrIt->first.c_str(), StrIt->first.size()); //String Name (As String)
				Term(); //End String
/**/				Write(StrIt->second.c_str(), StrIt->second.size()); //Content of String
				Term(); //End String
			}
		}
	}

	/* ******* All Done! ******* */
	Save.close();
}

error C2102: '&' requires l-value

Don't worry about all the complexities there, that all compiles fine. I have marked lines with the error with /**/. They use the Write() macro. Notice how only some of the lines with the Write() macro are marked. Is this user error? Or is it the bug reported here? http://connect.microsoft.com/VisualStudio/feedback/details/99547/bogus-error-c2102-requires-l-value-on-valid-code

If it is user error, how can I fix it?
If it is the compiler bug, then sorry I can't change compilers, the SDK I use only compiles under VC++ 2008 and up 0_0 Is there a woraround in this case?

[Disch]

Honestly I don't know whether or not that's legal, but you probably shouldn't be doing it regardless.

1) Macros are evil
2) You're casting around constness, which is even more evil. You should be casting to const char*, not char* in your Write macro.
3) c_str() returns a pointer. So when you do &c_str() (as you're doing with your macro) you're getting a pointer to a pointer. Basically you're writing the pointer to the file, and not the actual string data (as you intended). This went unnoticed because of the evil way you're doing things.
4) c_str() already gives you a null terminated string.. why not just write size() + 1 characters and get rid of that Term() macro? Not evil, but you can save yourself some trouble.



EDIT:

Here's my proposed solution. Write a class to automate this stuff, so you don't have to manually do it for every thing you're writing. Doing it every time like that means a lot of room for slip ups and mistakes.

class BinaryWriter
{
private:
  std::ostream& out;

  // no copying
  BinaryWriter(const BinaryWriter&);
  BinaryWriter& operator = (const BinaryWriter&);

  // support
  template <typename T>
  void WriteBasic(const T& v)
  {
    out.write(reinterpret_cast<const char*>(&v),sizeof(T));  // or you can get clever and make this endian safe
  }

public:
  BinaryWriter(std::ostream& stream)
    : out(stream) { }

  BinaryWriter& operator << (const std::string& str)
  {
    out.write(str.c_str(),str.length()+1);  // +1 to include the null terminator
    return *this;
  }

  BinaryWriter& operator << (unsigned long v) { WriteBasic(v); return *this; }
  BinaryWriter& operator << (  signed long v) { WriteBasic(v); return *this; }
  BinaryWriter& operator << (unsigned char v) { WriteBasic(v); return *this; }
};

Then you can do this:

	/* ******* Helpful Stuff ******* */
	BinaryWriter write(Save);

	/* ******* Save Code!! ******* */
	const string Identifier = "Timeline";
	const unsigned char Version = 0;
/*	Write(Identifier.c_str(), Identifier.length()); //8 character ID   // bad
	Write(Version, sizeof(unsigned char)); //Version Number*/
	write << Identifier << Version;  // better

/*	Write(rdPtr->Timeline->size(), sizeof(unsigned long)); //Number of Positions */  // bad
	write << rdPtr->Timeline->size();  // better

    // etc...
}

[LB]

OK, but now I get errors in the fstream header:

\include\fstream(803) : error C2248: 'std::basic_ios<_Elem,_Traits>::basic_ios' : cannot access private member declared in class 'std::basic_ios<_Elem,_Traits>'

I did exactly what you said. I understand how this should work, but I don't understand how making this class causes errors in another file. And no, I did not forget a semicolon.

[Disch]

Something tells me that's not the only error you're getting.

Can you post more of them? The first 10 or so should be good.

[LB]

It's just one, looong error:

1>C:\Program Files\Microsoft Visual Studio 9.0\VC\include\fstream(803) : error C2248: 'std::basic_ios<_Elem,_Traits>::basic_ios' : cannot access private member declared in class 'std::basic_ios<_Elem,_Traits>'
1>        with
1>        [
1>            _Elem=char,
1>            _Traits=std::char_traits<char>
1>        ]
1>        C:\Program Files\Microsoft Visual Studio 9.0\VC\include\ios(151) : see declaration of 'std::basic_ios<_Elem,_Traits>::basic_ios'
1>        with
1>        [
1>            _Elem=char,
1>            _Traits=std::char_traits<char>
1>        ]
1>        This diagnostic occurred in the compiler generated function 'std::basic_ofstream<_Elem,_Traits>::basic_ofstream(const std::basic_ofstream<_Elem,_Traits> &)'
1>        with
1>        [
1>            _Elem=char,
1>            _Traits=std::char_traits<char>
1>        ]

[LB]

Oops, I didn't see the rest of the errors!
Full build log: http://paste.pocoo.org/show/263697/
BinFileIO.h : http://paste.pocoo.org/show/263698/
The code that saves the file: http://paste.pocoo.org/show/263700/ (in Main.cpp)

The rest of the files don't matter, the header file is just included in them.

EDIT: I fixed it, I ended up doing something a bit different.

I also had to cast non-const values to const values, which was annoying. The compiler just couldn't figure out which overloaded operator<< function to use otherwise.