VBA Rubberducking

IvenBach

4:02 PM

I missed that rousing discussion.

23fc9a62-56de-47fb-97b4-737890

@IvenBach Let's think of another one.

OK, I have one.

Are e and é the same letter?

I have a little popcorn left, anyway.

IvenBach

Yes. The latter is just a snootier version.

this

Ano ≠ Año.... Papa ≠ Papá.... Que ≈ Qué

YMMV. :)

IvenBach

Knew someone that learned that the hard way. Spanish learner intended to said "Yo tengo 20 años." but used "anos" instead. ORLY?! Bit of humor resulted.

this

Sounds like an asshole if you ask me. :-D

Pardon, an assholes.

IvenBach

4:10 PM

:+1: Another one you earned.

Back to catching up at work. Hoping to get more unit test reading in too.

Mathieu Guindon

@23fc9a62-56de-47fb-97b4-737890 yes, if you're writing French with a US keyboard and don't feel like typing the damn ASCII codes on the numpad

Vogel612

@23fc9a62-56de-47fb-97b4-737890 depends, is the second written with a ZWJ or COMBINING_ACCENT_GRAVE?

because if yes, then the answer is #implementation_dependent

23fc9a62-56de-47fb-97b4-737890

@Vogel612 I have no idea what those are. I thank the English for being lazy enough to drop them so many years ago :D

Vogel612

Zero Width Joiner

23fc9a62-56de-47fb-97b4-737890

Oh.

Vogel612

4:19 PM

there's a difference between e´, e and é

23fc9a62-56de-47fb-97b4-737890

Umm, I think it's the second :)

M.Doerner

Question regarding resolving default member accesses: what do we want to happen, if we know that there is a default member access, but we have no idea what it references?

Mathieu Guindon

the identifier reference would have the defaultmemberaccess flag on, but wouldn't point to any declaration?

M.Doerner

This happens in two cases: when the accessed declaration id of type Object and when we cannot see the default member for some reason.

Mathieu Guindon

i.e. late-bound code

M.Doerner

4:26 PM

That is what currently happens for indexed default member accesses.

this

wouldn't that be basically undeclared variable?

M.Doerner

The referenced declaration is null, but the context is not.

Mathieu Guindon

given we don't create on-the-fly "ghost declarations" for these (like we do for undeclared locals), I think that's the most sensible approach

this

we don't? I thought we did for undeclared variables?

Mathieu Guindon

we create on-the-fly declarations for undeclared locals, not unknown members

Dim foo As Something
foo.Bar = 42 ' if Bar doesn't exist, we just don't resolve it

this

4:29 PM

to clarify - do you think we shouldn't do the same for unknown members?

M.Doerner

Actually, for the indexed default member accesses, we simply do not create an identifier reference.

Mathieu Guindon

that might be a problem

this

if there's going to be an identifier reference, we kind of need a created declaration, right?

Mathieu Guindon

@this if we did create a "ghost" Something.Bar member on first encounter, then we could track its uses even though the member doesn't exist (or even if foo is declared As Object)

@this in theory, yes

M.Doerner

I would rather not like to create declarations for unbound members.

That makes things very complicated.

We would need to always check whether we have seen them before and attach the correct declarations.

this

4:34 PM

there is also the matter that Object.Something on variable x may not be the same thing as Object.Something on variable y

Mathieu Guindon

yeah. and it would need very robust RHS type resolution too

this

so having a single declaration linking them both would be wrong, too.

M.Doerner

To do that consistently, the only option I see is to throw away all references after the first run and then to do it again with the declarations present.

this

that is tangential but we still haven't fixed the Redim thing. I assume that needs its own declaration.

Mathieu Guindon

I think ubound declarations (that's a better name than "ghost" ;) would require an entire new resolver pass

this

4:36 PM

when is undeclared locals resolved?

M.Doerner

They are patched in at the very end of the reference resolver.

this

I thought so. does the unbound members still need its own pass, after that pass?

M.Doerner

I have not really looked at the how we collect undeclared variables.

Could be that we just have different declarations on the references.

Could also be that we try to find them in the already found ones first.

Still, the same member to Object can be very different things.

Mathieu Guindon

we add the undeclared variables' Declaration to the finder as we encounter them; the next encounter just resolves it normally, since the identifier is in the finder cache then

M.Doerner

No, they do not go into the usual finder cashe.

They are cached in a separate collection.

Mathieu Guindon

4:42 PM

hm

M.Doerner

They only enter the usual cache on the next reload of the declaration finder.

Just saw that we actually have an UnboundMemberDeclaration.

Why aren't we using that with IndexExpressions?

this

FWIW, being to find all references for Object.Something would be more useful in cases where you want to refactor away from late-bound code or something like that. Obviously, the user would have to take caution in case the Object isn't the same.

M.Doerner

4:59 PM

I just had a look at what we currently do:

When we encounter an undeclared variable, we create an undeclared variable declaration and reference that.

If it already exists, we use the one we already had.

Mathieu Guindon

right and we only know we've encountered an undeclared variable if at the end of the resolution we still haven't found a suitable declaration for the local variable

M.Doerner

On the other hand, for unresolved member calls (and not for unbound member calls), we create a new declaration for every single place we encounter them and do not create an identifier reference.

Mathieu Guindon

huh, we do that?!

M.Doerner

        private void ResolveDefault(
            ParserRuleContext expression,
            StatementResolutionContext statementContext = StatementResolutionContext.Undefined,
            bool isAssignmentTarget = false,
            bool hasExplicitLetStatement = false,
            bool isSetAssignment = false)
        {
            var withExpression = GetInnerMostWithExpression();
            var boundExpression = _bindingService.ResolveDefault(
                _moduleDeclaration,
                _currentParent,

Mathieu Guindon

what's the difference between unresolved and unbound?

M.Doerner

5:07 PM

Unresolved has ExpressionType.ResolutionFailed and we simply failed to reolve it.

Mathieu Guindon

@M.Doerner that does read like AddIdentifierReferences runs whether or not boundExpression.Classification is ResolutionFailed or not

M.Doerner

Unbound has ExpressionType.Unbound and we resolved it to spec to an expression whose binding is not known at compile time.

It is a special case in there that adds identifier references for the subexpressions that could be resolved.

If we fail, we put them in a special collection on the expression.

Mathieu Guindon

makes sense

M.Doerner

Still, we do not add references to the freshly added member declaration.

this

just for my clarity - can we have unresolved stuff in a compilable code?

M.Doerner

5:11 PM

And we do not add declarations for unbound members.

Controls

If we do not know the correct interface, we fail.

this

ah but that's more of a shortcoming of our resolver, rather than actually failing to resolve it.

M.Doerner

We only fail, when the code does not compile in the first place and when we cannot see the full picture, like with controls.

Or in the situation in which a library references stuff from another library that is not loaded.

MsoTriState I am looking at you.

this

and that's where we need to fill up the ComProjects from the typelib api

but back to the issue at hand...

am I to understand that also means we are creating potentially hundreds or thousands of declarations that are potentially duplicative?

M.Doerner

For the failing stuff, yes.

this

hmmm I wonder if that might explain why it performs so poorly on projects where late bound members are common?

Mathieu Guindon

5:19 PM

hm

M.Doerner

But we do not add declarations for unbound members.

Only the ones with failed resolution.

Mathieu Guindon

but it still makes a lot of allocations and GC pressure, no?

M.Doerner

I do not follow.

Just to clarify, the following resolves completely.

Dim o As Object
Dim v As Variant
v = o.Foo.Bar.Baz

this

and the o.Foo, Foo.Bar and Bar.Baz all are unbound, right? So we aren't creating declarations for those ATM?

M.Doerner

Two SimpleNameExpressions with ExpressionClassification.Variabl and three MemberAccessExpressions with ExpressionClassification.Unbound.

Correct

this

5:25 PM

and I don't see how we'd be able to track them in any way without creating a declaration for them.

M.Doerner

We could still track the accesses with identifier references without a declaration.

this

That confuses me. Doesn't IR require a Declaration?

M.Doerner

Declaration is a reference type.

Just use null for we have no idea.

this

I don't have the code in front of me it's that I thought that all IR constructors requires Declaration and by convention we don't pass in null.

Mathieu Guindon

nothing says it can't be null though

M.Doerner

5:33 PM

Hm, we actually get failed resolutions for member accesses for extended members of extensible interfaces.

Mathieu Guindon

that's e.g. Excel.Application.Sum?

M.Doerner

I guess

My canonical example is Connection.SomeStoredProcedureName.

Mathieu Guindon

yeah, that too

M.Doerner

Those are the declarations we add.

Adding references without a declaration will probably break some things, but not having the assignment references anymore for default members will also break things.

this

wait, if that extends to Me.SomeControl, that's bad for Access & VB6 projects?

In my mind, it simply does not make sense to create a IR without a declaration, even if there is only one IR.

you can't "reference" a nothing.

M.Doerner

5:43 PM

Then, how do you know about assignments to default members?

You can reference unknown.

Or know about unbound default member accesses in general?

this

are we making a mistake in considering such access as a declaration? How do we handle normal member access?

Mathieu Guindon

5:57 PM

now that's hilarious

Oh wait. Thought your handle was Matt'sMug?? Edit: sorry if I am mixing you up with another person. — findwindow 6 mins ago

FreeMan

@MathieuGuindon there you go messing with people's heads again, @Mug!

this

What do you expect from a mug like Mat?

23fc9a62-56de-47fb-97b4-737890

8 more days until I can go back to hosch250.

Mathieu Guindon

> Microsoft didn't offer an explanation for the problem but it did flag earlier this month that it will move ahead with sunsetting VBScript, by disabling it in IE11 by default via an update in this week's patch.

zdnet.com/google-amp/article/…

#BeginningOfTheEnd

this

6:18 PM

if anyone's still using VBS in IE they need to be taken behind the barn and shot....

and shot again if using IE6.

2

FreeMan

6:41 PM

@MathieuGuindon I presume this is referencing the empty array being passed ByRef to a Variant parameter that was linked in yesterday?

Mathieu Guindon

that update, yes

this

i hope it doesn't turn out like that ADO update for Windows 7 SP1.

Mathieu Guindon

6:58 PM

@M.Doerner my understanding is that given e.g. Range("A1") = 42 we should be creating an IdentifierReference to the Range._Default property, with some flag that tells us the member access is implicit

M.Doerner

We actually do not count that as implicit in the corresponding inspection.

However, we create an IndentifierReference to said member with the IsDefaultMemberAccess flag set.

Mathieu Guindon

wait so Range("A1").[_Default] = 42 would look just the same?

oh, but the IR's Context would be null if implicit

M.Doerner

No, that does not have the IsDefaultMemberAccess flag set.

Mathieu Guindon

hmm. might want to rename the flag to IsImplicitDefaultMemberAccess then

M.Doerner

Wait

That is actually an unbound member call.

this

7:06 PM

I thought it was Range.[_Default]("A1")? Excel OM is weird.

M.Doerner

^

That would be the same as the default member access, but without the flag.

For me, a default member access is one where the default member resolution is used.

Mathieu Guindon

@this no, Worksheet.Range property is parameterized

I'm talking about the target of the assignment

M.Doerner

Currently, we do not resolve that at all.

To RD, the assignemnt goes to the range, which I would like to change.

Mathieu Guindon

yeah that would be nice

M.Doerner

Which type does Worksheet.Range return?

Mathieu Guindon

7:10 PM

the assignment really should go to Range.[_Default]

@M.Doerner Excel.Range :)

M.Doerner

OK, then we have a chance.

Mathieu Guindon

yeah. what we can't do is resolve the assignment to Range.Value

our path ends at Range.[_Default]

M.Doerner

For anything returning Variant, we will never be able to see the default member access.

Mathieu Guindon

or Object

this

also the case for Worksheets

(or any Variant-returning collections, really)

M.Doerner

7:12 PM

I intend to put those assignments into a separate collection for unbound default member accesses.

We will not know what they reference, but we will know they exist.

That was referring to Object.

this

that would be for expressions like obj = 42, right?

M.Doerner

Yes

this

since a Let expression would be nonsensical on objects, yeah.

M.Doerner

Or any LHS returning Object.

Mathieu Guindon

where obj is a known type with a default member but we don't know what type that member is?

M.Doerner

7:15 PM

It would also work for the RHS.

I just have to pass a flag along in the highest level of the resolution that we are dealing with a value context.

Then I will conditionally wrap the usual binding in some ValueDefaultBinding.

That will either return the original bound expression or a DefaultMemberAccessExpression.

Finally, we create the references in the BoundExpressionVisitor, which will receive a new case for the DefaultMemberAccessExpression.

This sceme should fit in nicely with the existing resolver.

IvenBach

8:28 PM

@MathieuGuindon so does this mean VBA won’t give error messages after update?

Perma OERN?

M.Doerner

9:05 PM

Do we want to be able to distinguish indexed and non-indexed unbound default member accesses?

this

I'd imagine that we'd want to know what was the index used, no?

23fc9a62-56de-47fb-97b4-737890

9:21 PM

devblogs.microsoft.com/dotnet/try-out-nullable-reference-types/…

Ohh, I just took a survey from MS about desktop tech, and it had several questions geared toward "what if we built a desktop UI system that works on all platforms".

M.Doerner

@this I do not really know how that would help. However, for unbound default members, we anyway never associate the arguments to the default member access.

We reolve the arguments, but we have nothing to attach tehm to.

My question is whether there are valid scenarios in which we only want to look at the default member accesses for which there is no indication in the code, the non-indexed ones.

KySoto

@23fc9a62-56de-47fb-97b4-737890 my answer would tend towards... no thanks, i like desktops

this

IMO, we should enumerate all, indexed or not

M.Doerner

That is not the question.

The question is whether they should be distinguishable.

this

I was thinking yes they should be because more data we have means more opportunities to use it and not distinguishing would make it difficult for implementation of possible inspections that don't exist yet

M.Doerner

9:34 PM

For example, should the ImplicitDefaultMemberAssignmentInspection also return results for assignments to indexed default members?

this

Actually I was going to suggest the case where there is a chained access across a non-indexed to indexed, in which case, it would be confusing if we did not make the distinction between the hidden non-indexed vs. indexed that is visible on the screen.

example col(1) = 42 might be actually multiple default member access and only one of the access (which is indexed) is visible.

M.Doerner

We only get references for the final default member in the chain.

this

so if the full expanded version is col.Item(1).Value = 42, we only see Value as the reference?

M.Doerner

Oh, in that scenario, we should get two.

this

the inspection most likely want to return a result for the usage of Value but not necessarily for Item

M.Doerner

9:38 PM

But we do not get intermediate ones like in rst("Foo").Value = 42.

this

see, I anticipate a new inspection to warn about chained access.

if we don't have that data, how will we create that inspection?

M.Doerner

That would not be a problem, because only Value gets the assignment flag.

KySoto

@MathieuGuindon
https://www.reddit.com/r/vba/comments/crct76/rubberduck_vba_prevent_variable_is_not_assigned/

meh, its being dumb

mat you are being called

Mathieu Guindon

Thanks!

KySoto

looks like he deleted it

-_-

did you PM?

or reply and he go... -_-

M.Doerner

9:49 PM

@this I think I can get the intermediate default member accesses as well.

We will just need to also save the default member recursion depth in order to distinguish all those references with the same selection.

Mathieu Guindon

@KySoto huh, but I posted a reply..

wtf it is deleted

this

I'm not sure if this is applicable but I can imagine that expressions like Me!someSubform.Form!someControl will be very painful, and we need to be able to tell the users to avoid that kind of expression and help them understand why it does too much and not saying what it is doing.

(and I apologize profusely for have written a travesty of code, if one can call it code...)

KySoto

10:05 PM

i sent him a PM to ask why he deleted it

i just saw your comment, and i was going to say something similar about the array declaration

from Senipah

[+5] sent just now

Ah sorry! I thought as soon as I deleted it that I should have just left it with the solution but as it was pretty soon after I posted it I thought I'd just remove it and hopefully not disturb you guys - I should ban myself :)

I realised as soon as I posted it I just needed to do Dim rtn() as Variant rather than just Dim rtn as Variant so that RD knew it was an array. Sorry to you and Mr Rubberduck for the disturbance!

M.Doerner

@this I actually only have to change something for the recursive ones; chains on dictioanry access expressions are already reolved.

The problem is always with the stuff that is not visible in the code.

IvenBach

10:23 PM

Why didn't I just start with Version3 of my code... I'm making progress but its a sideways spiral kind of progress.

this

10:36 PM

@M.Doerner in the previous example, there were 3 hidden accesses. If I'm looking at the status bar, I'd hope to see them all there. If that's already case then never mind me.

M.Doerner

Ok, I saw only two dictionary expressions and I have no experience with the OM. Where is the third one?

this

Me.Controls("someSubform").Form.Controls("someControl").Value would be the fully expanded form

M.Doerner

We do not get the final member.

However, I am not sure whether we have sufficient information to resolve any of this.

this

Why not? It's a default member for Access.Control in this example.

M.Doerner

Because we currently do not resolve non-indexed default member accesses.

I am working on that right now.

this

10:41 PM

right

Mathieu Guindon

@M.Doerner they should be made easy to spot - we'll want an inspection to flag those

actually, nevermind - we'll be flagging the declarations themselves, not their refs

M.Doerner

We already have one for assignments to implicit default members.

However, that will have to be rewritten.

Btw, we should flag the references, not the declarations, because there might not be one.

this

@M.Doerner I'm not clear either but if you now know that Me!someSubform is a default member access to Form.Controsl("index"). IIRC, this is all strong-typed, so I would hope the improved resolver will understand those expressions

because unfortunately, it's far too common. :(

Mathieu Guindon

No I mean the @OptionStrict stuff; parameterless default members in user code gets a red flag =)

this

^

time to put some teeth on the ducky

M.Doerner

10:49 PM

In your example, everything is strongly-typed and you will have a declaration.

this

Ok, cool.

M.Doerner

However, we can also have unbound default member accesses.

this

11:42 PM

@MathieuGuindon: support.microsoft.com/en-us/help/4512494/…

Also, i want to ask -- does it make sense to have unit tests over the externally declared functions? I have a managed function that's wrapping an unmanaged function. I don't really need to test the unmanaged function but I do have to test that my wrapping is correct. Does that require exhaustive coverage?

Mathieu Guindon

@this thanks, tweeted

@this what's there to test?

this

this is my test setup atm:

[Test]
[TestCase(1.0, typeof(string), ExpectedResult = "1")]
[TestCase(0.0, typeof(string), ExpectedResult = "0")]
[TestCase(-1.0, typeof(string), ExpectedResult = "-1")]
[TestCase(0.1, typeof(string), ExpectedResult = "0.1")]
[TestCase(-0.1, typeof(string), ExpectedResult = "-0.1")]
public object Test_ObjectConversion_Currency(decimal value, Type targetType)
{
    var cy = new CurrencyWrapper(value);
    return VariantConversion.VariantChangeType(cy, targetType);
}

which is testing this code:

        [DllImport(dllName, EntryPoint = "VariantChangeType", CharSet = CharSet.Auto, SetLastError = true, PreserveSig = true)]
        private static extern int VariantChangeType(ref object pvargDest, ref object pvarSrc, VariantConversionFlags wFlags, VARENUM vt);
        public static object VariantChangeType(object value, VARENUM vt)
        {
            object result = null;
            var hr = VariantChangeType(ref result, ref value, VariantConversionFlags.NO_FLAGS, vt);
            if (ComHelper.HRESULT_FAILED(hr))

note that I'm converting the Type into VARENUM via the GetVarEnum

Mathieu Guindon

Hm, tough call. Well the tests do document the behavior, so I guess they're good

this

I'm thinking that I should only enough cases to cover at least one of each

I have bool fairly exhaustively:

    [TestCase(true, typeof(bool), ExpectedResult = true)]
    [TestCase(false, typeof(bool), ExpectedResult = false)]
    [TestCase("1", typeof(bool), ExpectedResult = true)]
    [TestCase("0", typeof(bool), ExpectedResult = false)]
    [TestCase("-1", typeof(bool), ExpectedResult = true)]
    [TestCase(1.0, typeof(bool), ExpectedResult = true)]
    [TestCase(0.0, typeof(bool), ExpectedResult = false)]
    [TestCase(-0.0, typeof(bool), ExpectedResult = false)]
    [TestCase(-1.0, typeof(bool), ExpectedResult = true)]

though it's missing tests for datetime & currency to bool (lame as they may be) because they need additional setup

but to cover the full permutations of possible VT types....

Transcript for