Cache the reflection call done for completions

[rjmholt]

I noticed that completion queries do a reflection call every time. This code caches instead, but it's not terribly elegant. So if you have any suggestions, please let me know.

[SeeminglyScience]

Good call! One possible optimization.

[SeeminglyScience]

I think this would be better as just a static field. If your looking for how you could do that without an instance, you could do this:

private static MethodInfo s_extentCloneWithNewOffset =
    typeof(PSObject).Assembly.GetType("System.Management.Automation.InternalScriptExtent")
        .GetMethod(...

Maybe set in a static constructor because of the compiler directives.

[rjmholt]

Yeah I was wondering what the best way to do this was. Figured we'd find a better way in the PR. Thanks!

[rjmholt]

Ok, I've updated to use a better way to get the MethodInfo.

Once we're on netstandard2.0, we might be able to improve on the object[] allocation with each invocation by creating a delegate. I'm not quite sure if it's possible since InternalScriptPosition is internal and thus hard to pass around a Func<> for, but it can be created. See:

• https://stackoverflow.com/questions/38111987/using-reflection-methods-that-take-array-parameters-efficiently
• https://stackoverflow.com/questions/940675/getting-a-delegate-from-methodinfo

You can model what I'm imagining doing with this PowerShell:

$cwno = [psobject].GetTypeInfo().Assembly.GetType("System.Management.Automation.Language.InternalScriptPosition", $true).GetMethod("CloneWithNewOffset", [System.Reflection.BindingFlags]::Instance -bor [System.Reflection.BindingFlags]::NonPublic)

$t = [System.Linq.Expressions.Expression]::GetFuncType(@($cwno.ReturnType, [int], $cwno.ReturnType))

[System.Delegate]::CreateDelegate($t, $null, $cwno)   

That should create the open delegate we want. The problem is just being able to invoke it when we can't express the type of the Func<> statically.

[SeeminglyScience]

You could craft the delegate using LINQ expressions, e.g.

using namespace System.Linq.Expressions
using namespace System.Management.Automation.Language

$scriptPositionType = [psobject].Assembly.
    GetType('System.Management.Automation.Language.InternalScriptPosition')

$method = $scriptPositionType.
    GetMethod(
        'CloneWithNewOffset',
        [System.Reflection.BindingFlags]'NonPublic, Instance',
        $null,
        [type[]][int],
        @(1))

$source = [Expression]::Parameter([IScriptPosition], 'source')
$offset = [Expression]::Parameter([int], 'offset')

# (IScriptPosition)((InternalScriptPosition)source).CloneWithNewOffset(source, offset)
$delegateBody = 
    [Expression]::Convert(
        [Expression]::Call(
            [Expression]::Convert(
                $source,
                $scriptPositionType),
            $method,
            $offset),
        [IScriptPosition])

$delegate = [Expression]::Lambda(
    [Func[IScriptPosition, int, IScriptPosition]],
    $delegateBody,
    [ParameterExpression[]]($source, $offset)).
    Compile()

You could also write it in IL with DynamicMethod. That would be a little bit faster, but then you need to add the currently unlisted System.Reflection.Emit package, and readability would be pretty shot.

[rjmholt]

Yeah I thought about writing a compiled LINQ expression around it, especially since that might be the right way to get around having to provide an InternalScriptExtent as an input (we can add a cast like you have).

But as you say, it gets pretty out of hand. Especially if we're just trying to avoid an array allocation.

[TylerLeonhardt]

LGTM

[rjmholt]

Oooh, tests have found another breakage! Will look into it tomorrow.

[rkeithhill]

I like it!