tags:

views:

878

answers:

10

Given that this very natural use case (if you don't know what as actually does)

if (x is Bar) {
   Bar y = x as Bar;
   something();
}

is effectively equivalent (ie, compiler generated IL from the above code will be equivalent) to

Bar y = x as Bar;
if (y != null) {
    y = x as Bar; //The conversion is done twice!
    something();
}

EDIT:

I guess I hadn't made my question clear. I wouldn't ever write the second snippet as it's of course redundant. I'm claiming that the IL generated by the compiler when compiling first snippet is equivalent to the second snippet, which is redundant. Questions: a) Is this correct b) If so, why is is implemented like that?

This because I find the first snippet a lot clearer and prettier than the actually well written

Bar y = x as Bar;
if (y != null) {
   something();
}

EDIT 2: Please try to avoid adding quotes to the title, if at all possible, thank you!

CONCLUSION:

Optimizing the is/as case is not the compiler's responsibility, but the JIT's.

Also, as with null check has less (and less expensive) instructions than both of the alternatives (is and as and is and cast).

Addendum:

IL for as with nullcheck (.NET 3.5):

L_0001: ldarg.1 
L_0002: isinst string
L_0007: stloc.0 
L_0008: ldloc.0 
L_0009: ldnull 
L_000a: ceq 
L_000c: stloc.1 
L_000d: ldloc.1 
L_000e: brtrue.s L_0019
L_0011: ldarg.0 
L_0019: ret

IL for is and cast (.NET 3.5):

L_0001: ldarg.1 
L_0002: isinst string
L_0007: ldnull 
L_0008: cgt.un 
L_000a: ldc.i4.0 
L_000b: ceq 
L_000d: stloc.1 
L_000e: ldloc.1 
L_000f: brtrue.s L_0021
L_0012: ldarg.1 
L_0013: castclass string
L_0018: stloc.0 
L_0019: ldarg.0 
L_0021: ret

IL for is and as (.NET 3.5):

L_0001: ldarg.1 
L_0002: isinst string
L_0007: ldnull 
L_0008: cgt.un 
L_000a: ldc.i4.0 
L_000b: ceq 
L_000d: stloc.1 
L_000e: ldloc.1 
L_000f: brtrue.s L_0021
L_0012: ldarg.1 
L_0013: isinst string
L_0018: stloc.0 
L_0019: ldarg.0 
L_0021: ret

These have been edited for shortness (method declarations, nops and calls to something() removed)

+2  A: 

You won't do a second y = x as Bar;, because your already have y which is Bar.

J. Random Coder
I won't, but if I write it in the more natural (is) way, the compiler will
Vinko Vrsalovic
+5  A: 

In your example, the use of as is redundant anyway. Since you already know that x is Bar, you should be using a cast:

if (x is Bar)
{
    Bay y = (Bar)x;
}

Alternatively, convert using as and just check for null:

Bar y = x as Bar;
if (y != null)
{

}
Will Vousden
The point of the question is that the compiler generates the redundent code.
Steve Haigh
The first example makes two casts, while the second one only one, and casts are expensive. So I would prefer the second approach when Bar is a class. (For value types only the first one can be used, of course.)
treaschf
+5  A: 

Firstly I disagree with your premise that this is more typical use case. It may be your favourite approach, but the idiomatic approach is the "as + null check" style:

Bar y = x as Bar; 
if (y != null) { 
   something(); 
}

As you have found the "is" approach requires the extra "as" or a cast, which is why the "as" with null check is the standard way of doing this in my experience.

I see nothing offensive about this "as" approach, personally I don't think it any more unpleasant on the eye than any other code.

As to your actual question, why is the is keyword implemented in terms of the as keyword, I have no idea, but I do like the play on words in your question:) I suspect neither is actually implemented in terms of the other, but the tool (Reflector I guess) you used to generate C# from the IL interpreted the IL in terms of as.

Steve Haigh
Okay, I changed the more typical to "a very natural" use case. I don't find nothing particularly offensive about the null check, I just find the other snippet a lot prettier :)
Vinko Vrsalovic
Sure, I don't mean to to criticise your view, but I do think the "as" style is more or less the standard approach.
Steve Haigh
+9  A: 

Well, the IL instruction that is available (isinst) will return either an object of the appropriate type, or null if such a conversion is not possible. And it doesn't throw an exception if the conversion isn't possible.

Given that, both "is" and "as" are trivial to implement. I wouldn't claim that "is" is implemented as "as" in this case, just that the underlying IL instruction allows both to occur. Now, why the compiler isn't able to optimize the "is" followed by "as" into a single isinst call, that's another matter. Probably, in this case, it's related to variable scope (even though by the time this is IL, scope doesn't really exist)

Edit

On second thoughts, you can't optimise "is" followed by "as" into a single isinst call, without knowing that the variable under discussion isn't subject to update from other threads.

Assuming x is a string:

//Thread1
if(x is string)

//Thread2
x = new ComplexObject();

//Thread1
    y = x as string

Here, y should be null.

Damien_The_Unbeliever
This is actually a reason. Thanks! :)
Vinko Vrsalovic
As long as the variable is not volatile and there are no barriers in between the is and as operations, there is no reason to not reuse the first instance.
erikkallen
@erikkallen - I'm not going to disagree. I just think it would be an unusually specific optimization to make, especially given "normal" usage of "is" and "as"
Damien_The_Unbeliever
this is the sort of optimization I would expect at the JIT level rathern then in the C# compiler.
Ian Ringrose
A: 

I suspect strongly that is is faster than as and does not require an allocation. So if x is rarely ever Bar, than the first snippet is good. If x is mostly Bar, then an as would be recommended, as no second cast is required. It depends on the usage and circumstances of the code.

MaLio
who would "as" do an allocation? the pointer it returns will be on the stack as a pointer is a **value** type.
Ian Ringrose
+1  A: 

According to this blog post by Eric Lippert that is a compiler pass, to quote:

Then we run an optimization pass that rewrites trivial "is" and "as" operators.

So perhaps that is why you are seeing the same IL generated for both snippets.

shf301
Interesting link, thanks.
Vinko Vrsalovic
+1  A: 

You could write the code now as

DoIfOfType<Bar>(possibleBar, b => b.something())

That I would say was a bit clearer, but not as fast without real magic from the compiler.

Ian Ringrose
A: 

The scope of 'y' is reduce if you place the declaration inside the loop.

Whoever wrote it probably prefers casting 'x as T' more than '(T)x', and wanted to limit the scope of 'y'.

Strilanc
A: 

You forgot about value types. Eg:

    static void Main(string[] args)
    {
        ValueType vt;
        FooClass f = vt as FooClass;

    }

    private class FooClass
    {
        public int Bar { get; set; }
    }

Won't compile as value types can't be converted like this.

Wyatt Barnett
I don't understand how this is relevant here. Of course I'm talking about where it can be used... care to elaborate?
Vinko Vrsalovic
Relevent because it is why you can't get rid of the `is` operator.
Wyatt Barnett
+10  A: 

a) Is this correct

Yes, though I would have stated it the other way. You are saying that "is" is a syntactic sugar for as-followed-by-null-check. I would have said it the other way: that "as" is a syntactic sugar for "check for type implementation, cast if success, null if failure".

That is to say, I would be more inclined to say

if (x is Bar) { 
   Bar y = x as Bar; 
   something(); 
} 

is effectively equivalent to

if (x is Bar) { 
   Bar y = (x is Bar) ? (Bar)x : (Bar) null; 
   something(); 
} 

See, you want to define "as" in terms of "is", not the other way around. The question really should be "why is as implemented as is?" :-)

b) If so, why is is implemented like that?

Because that's a correct implementation of the specification.

I think I'm not following your line of thought here. Is there something wrong with that implementation? How would you prefer it to be implemented? You have the "isinst" and "castclass" instructions at your disposal; describe the codegen for your program that you'd like to see.

Eric Lippert
There is nothing WRONG with the implementation. It just renders a particular use case I personally like better than the correct alternative (I think it's more readable and intuitive) inefficient. So I wondered if there was a reason for that. And then, to make it work for this use case, the compiler should make an optimization which preserves the casted instance used on the test: if (x is Bar) { Bar y = x as Bar; //I'd wish that here the same instance used in the if test is used }. But that might be not thread safe in some situations and there would probably be some other problems...
Vinko Vrsalovic
@Vinko: But the same instance is used. It's always the same reference to the same instance. I'm still not following you. Are you saying that you don't want the type test performed three times? Because it is performed three times in the success case here -- once on the first "is", once on the "is" that is part of the "as", and once during the cast. If you don't want the redundant test, then don't use "as" in the consequence. Say "Bar y = (Bar) x;". Then you only get the test done twice.
Eric Lippert
Although this still doesn't explain why there is no optimization in the first case with is and as. Or is there?
Vinko Vrsalovic
@Vinko: That's up to the jitter to decide. The C# compiler just generates the IL as requested. If you say "if (x == 2) { if (x % 2 == 0) ... " the compiler doesn't say "well, I see that if the first condition is met then the second one will be too, so I'll elide the second test". If you write code that does redundant tests, we generate code that does redundant tests. If the jitter is smart enough to optimize that away, great, but the C# compiler is not.
Eric Lippert
@Eric: Correct. Thanks. And thanks for interpreting my confusion away from my earlier comment (I didn't mean instances.)
Vinko Vrsalovic