Slice: Defining Exception with Message member

[kwaclaw]

I defined an exception like this:

exception WSAAError {
string Message;
};

This generates non-compilable code in C# (and probably VB as well), as the Slice compiler tries to assign to the pre-existing read-only Message property in the .NET Exception class.

Karl

[michi]

Thanks for the bug report! We'll fix this for the next release. In the mean time, please use a name for the data member that does not clash with one of the properties and methods on System.ApplicationException.

Cheers,

Michi.

[kwaclaw]

Originally Posted by michi

Thanks for the bug report! We'll fix this for the next release.

It would be cool if - in case of a name clash - the Slice code generator could simply take advantage of existing members (even if assignment has to be done in the constructor). Some kind of annotation might be useful there.

In my case this would mean that defining a Message member would simply expose an existing property of the built-in Exception class. Its probably not as simple as it sounds, as you can't define a read-only member in Slice (unless through annotations?).

Karl

[michi]

I don't think what you suggest is feasible, mainly because the clash can happen with both operations and properties and because properties can be read-only.

Pragmatically, the only option is to mangle such exception members. For example, a "Message" exception member would be mapped as "ice_Message" or similar.

Cheers,

Michi.

[kwaclaw]

Originally Posted by michi

I don't think what you suggest is feasible, mainly because the clash can happen with both operations and properties and because properties can be read-only.

Pragmatically, the only option is to mangle such exception members. For example, a "Message" exception member would be mapped as "ice_Message" or similar.

Cheers,

Michi.

I understand.
It's just inconvenient that Slice exceptions always have an empty message, as this is probably the most used member.

Karl

[michi]

The reason it is this way is the one-shot constructor for exceptions. If we were to permit the Message member of the base exception to be used, we couldn't have one-shot constructors for exceptions with string members because the constructor would become ambigous. (Which member would be initialized, the member in the base or the member in the exception itself.)

Also, using the Message member of the base exception is probably not such a good idea because it won't work across languages (because, for example, for C++, there is no such base exception). If you really want a message member, define it in Slice in the exception; that way, the member is available across all languages, and is available even if the exception is marshaled over the wire.

Cheers,

Michi.

[kwaclaw]

Originally Posted by michi

The reason it is this way is the one-shot constructor for exceptions. If we were to permit the Message member of the base exception to be used, we couldn't have one-shot constructors for exceptions with string members because the constructor would become ambigous. (Which member would be initialized, the member in the base or the member in the exception itself.)

Just thought of this: Since Exception.Message is a virtual property, it could be overridden to return whatever string member the Slice author chooses, using some form of tagging/annotation.

Originally Posted by michi

Also, using the Message member of the base exception is probably not such a good idea because it won't work across languages (because, for example, for C++, there is no such base exception). If you really want a message member, define it in Slice in the exception; that way, the member is available across all languages, and is available even if the exception is marshaled over the wire.

Understood, you need to define a string member as "message". In addition, just for .NET, add some tag that tells the Slice code generator to emit an override for Message returning that string member. This would not interfere with the existing code generation. Apart from the effort required, do you think this is workable? Of course, I could fix up the code myself, but that would be overwritten every time I re-generate the code.

Karl

[michi]

Originally Posted by kwaclaw

Just thought of this: Since Exception.Message is a virtual property, it could be overridden to return whatever string member the Slice author chooses, using some form of tagging/annotation.

Hmmm... I think this is problematic. For one, not all the properties on ApplicationException are virtual, and some of them are read-only. So, we would have to come up with a mapping that knows about each of these properties and does something appropriate. I don't like this very much, mainly because of its complexity and the difficulty of explaining it.

Also, using a virtual property has the problem that, if an exception is passed as its base type, and the receiver uses the property, what it gets is the property of the base class, not the overridden implementation in the derived class. This can lead to rather surprising behavior.

I think the simple and pragmatic solution is to mangle the name of a Slice exception member if that name collides with a member of ApplicationException and its bases. But, before dismissing this out of hand, I'd like to at least consider it. I think I know what you mean, but could you post an example of what the generated code would look like, at least in outline?

Cheers,

Michi.

[kwaclaw]

Originally Posted by michi

Hmmm... I think this is problematic. For one, not all the properties on ApplicationException are virtual, and some of them are read-only. So, we would have to come up with a mapping that knows about each of these properties and does something appropriate. I don't like this very much, mainly because of its complexity and the difficulty of explaining it.

If the annotation is not workable, the Slice compiler should throw an error.
For example,

exception PBSError {
  ["dotNet:->Message(property, virtual, readonly)"]
  string Reason;
};

If Message does not exist, or is neither virtual nor readonly, it is an error.

Originally Posted by michi

Also, using a virtual property has the problem that, if an exception is passed as its base type, and the receiver uses the property, what it gets is the property of the base class, not the overridden implementation in the derived class. This can lead to rather surprising behavior.

True, if it originates as PBSError (see above) on the server, but the Slice for that operation only defines an ancestor (without the mapped member), then the receiver would not benefit from the mapping. Did I understand this correctly?

Originally Posted by michi

I think the simple and pragmatic solution is to mangle the name of a Slice exception member if that name collides with a member of ApplicationException and its bases. But, before dismissing this out of hand, I'd like to at least consider it. I think I know what you mean, but could you post an example of what the generated code would look like, at least in outline?

Considering the Slice definition shown above (without annotation, of course), what I mean is as simple as adding this override, no other changes:

public override string Message {
  get {
    return Reason;
  }
}

I really don't want to bother you too much. Its just so common to examine the message property of an exception, and its a surprise if its empty. Btw, normally exceptions constructed with no Message will have a default message assigned by .NET, but Ice.UserException has an empty message.

Anyway, as a "funny" side note: the current project I am working on - the first where we will be using ICE in production - has the purpose of wrapping a vendor's web services, since they turned out not to be as inter-operable as one would expect.

Karl

[michi]

Originally Posted by kwaclaw

If the annotation is not workable, the Slice compiler should throw an error.
For example,

exception PBSError {
  ["dotNet:->Message(property, virtual, readonly)"]
  string Reason;
};

If Message does not exist, or is neither virtual nor readonly, it is an error.

Hmmm... We have no concept of read-only properties in Slice. Doing this would cause problems when an exception is unmarshaled because the unmarshaling code relies on being able to first instantiate the exception and then, as the contents of its data members arrive, to assign to those data members. If a property is read-only, this won't work.

There are also issues relating to the type system. For example, how should the mapping deal with an exception that defines a Message member, but with different type?

exception E
{
    int Message;
};

We could generate:

public class E : global::System.ApplicationException
{
    public new int Message;
}

However, I can't say I like this because adding non-virtual members in the derived class in this way can be confusing.

True, if it originates as PBSError (see above) on the server, but the Slice for that operation only defines an ancestor (without the mapped member), then the receiver would not benefit from the mapping. Did I understand this correctly?

Sorry, I meant "non-virtual" property. If we have a non-virtual property in the base and add another non-virtual property in the derived class, if the derived instance is passed as a base, the callee will access the property in the base, not in the derived instance, which is confusing.

I really don't want to bother you too much. Its just so common to examine the message property of an exception, and its a surprise if its empty. Btw, normally exceptions constructed with no Message will have a default message assigned by .NET, but Ice.UserException has an empty message.

Hmmm... we could leave the Message property of System.ApplicationException uninitialized, so it at least gets the .NET default. I'll have a look at this for the next major release.

Anyway, as a "funny" side note: the current project I am working on - the first where we will be using ICE in production - has the purpose of wrapping a vendor's web services, since they turned out not to be as inter-operable as one would expect.

Well, there are a lot of things I could say at this point, such as "I told you so" or "How come I'm not surprised?" But, mind you, I'm not saying any of these things...

Cheers,

Michi.

[kwaclaw]

Originally Posted by michi

Hmmm... We have no concept of read-only properties in Slice. Doing this would cause problems when an exception is unmarshaled because the unmarshaling code relies on being able to first instantiate the exception and then, as the contents of its data members arrive, to assign to those data members. If a property is read-only, this won't work.

I didn't mean to introduce read-only properties into Slice. All I am proposing is a modification to the code generator, that is, a way to map Slice members to pre-existing class members (which some languages have). It doesn't have to replace the regular Ice mapping, just add an additional mapping, even if it is for read-only properties (in whic case this is only possible if the pre-existing member can be overridden).

Originally Posted by michi

There are also issues relating to the type system. For example, how should the mapping deal with an exception that defines a Message member, but with different type?

exception E
{
    int Message;
};

We could generate:

public class E : global::System.ApplicationException
{
    public new int Message;
}

However, I can't say I like this because adding non-virtual members in the derived class in this way can be confusing.

If I am already writing the Slice, it would be my fault if I tried simultaneously to map some Slice member to a pre-existing C# member and at the same time tried to add a new Slice member clashing with the pre-existing name.

As I observed, you are already re-naming Slice members with name clashes (although there seems to be a bug in the code generated for initDM__). So, as far as my suggestion is concerned, it can stay as it is (without the bug, of course), and the mapping should still work, since it is nothing else but an assignment or override for a pre-existing member.

Originally Posted by michi

Sorry, I meant "non-virtual" property. If we have a non-virtual property in the base and add another non-virtual property in the derived class, if the derived instance is passed as a base, the callee will access the property in the base, not in the derived instance, which is confusing.

Maybe there is a slight misunderstanding here - I am not proposing any changes to Slice and its type system, but just a code generation modification for certain languages (I assume a similar issue exists with Java?).

Originally Posted by michi

Hmmm... we could leave the Message property of ystem.ApplicationException uninitialized, so it at least gets the .NET default. I'll have a look at this for the next major release.

Thanks.

Originally Posted by michi

Well, there are a lot of things I could say at this point, such as "I told you so" or "How come I'm not surprised?" But, mind you, I'm not saying any of these things...

You would not have to say them to me ...
I am actually surprised that no-one has laughed me out of the building yet for suggesting ICE, but I guess at the end of the day a working solution wins...

Karl

[acbell]

Originally Posted by michi

I don't think what you suggest is feasible, mainly because the clash can happen with both operations and properties and because properties can be read-only.

Pragmatically, the only option is to mangle such exception members. For example, a "Message" exception member would be mapped as "ice_Message" or similar.

Cheers,

I just ran into this same problem. Seems that the mangling is happening already in 3.2.1, just not all the way through the generated code. "message" in slice is converted to the public member "ice_message_", but is still used as "message" in the initDM__ function.

Still, probably better to emit an error/warning on slice2cs.

Cheers,

[michi]

Originally Posted by kwaclaw

I didn't mean to introduce read-only properties into Slice. All I am proposing is a modification to the code generator, that is, a way to map Slice members to pre-existing class members (which some languages have). It doesn't have to replace the regular Ice mapping, just add an additional mapping, even if it is for read-only properties (in whic case this is only possible if the pre-existing member can be overridden).

If the pre-existing class member is read-only, we cannot unmarshal the exception (at least not with the current unmarshaling code). So, the only other option would be to have a new member with the same name in the derived exception class, but I can't say that I like this at all because then we have two members with the same name, one in the base class and one in the derived class.

If I am already writing the Slice, it would be my fault if I tried simultaneously to map some Slice member to a pre-existing C# member and at the same time tried to add a new Slice member clashing with the pre-existing name.

We could do it this way. However, that's not a very attractive option because it means that legal Slice definitions could result in illegal C# code. However, our rule is that any legal Slice definition must result in legal code for all target languages. (If we didn't adhere to this rule, we could cause havoc for some projects. For example, imagine you write and deploy a large application written with Ice for C++ on thousands of machines. Later, you decide that you want to add some components written in C#, only to discover that the already deployed Slice definitions are not usable with C#...)

So, we'd have to add some renaming mechanism via metadata that would allow the Slice member to be renamed to something else. But I don't like the complexity of this. I think we need to keep in the mind the scope of the problem we are trying to solve: name clashes such as the one you found are rare in practice. In the few cases where a name clash become an issue, identifier mangling is a simple and effective way of dealing with the clash.

As I observed, you are already re-naming Slice members with name clashes (although there seems to be a bug in the code generated for initDM__).

Yes, I had a look yesterday and this is simply a bug--the code generator omits to mangle the name in one particular place. I'll post a fix for this today.

I am actually surprised that no-one has laughed me out of the building yet for suggesting ICE, but I guess at the end of the day a working solution wins...

Karl, I suspect that the last laugh will be on you

Cheers,

Michi.

[michi]

I've posted a patch for slice2cs to fix the incorrect generated code.

Cheers,

Michi.

[kwaclaw]

Originally Posted by michi

I've posted a