yeah, ...that doesn't necessarily mean the node itself needs a ref to an object that already has a ref to it
it's the part that creates the node that needs to know about identifier refs and declarations
then an IAssignmentNode can have an AddReference(IReferenceNode node) method - and thus we have IParseTree / ParserRuleContext nodes referencing other parse tree nodes
...does that move the problem into the parse trees?
well "creates the nodes" - the parser creates these nodes, we're just injecting some metadata :)
@Duga so at this point the interesting parse tree nodes implement the CPA node interfaces, and most of the Rubberduck.Inspections.CodePathAnalysis namespace needs to die or be adjusted
Mug. Ping me tomorrow as a reminder to bring my hilbilly video recording hat. Picked up a new sd card. Old 2GB = ~5mins. New one is >5hr video. That should get you a fill for your unicycling craving.
In your '@IgnoreModule annotation you have a semicolon to add some additional info; is this something you can actually do with RubberDuck? I've not been having much success... — Greedo44 mins ago
@Greedo I haven't had any issues doing that, but the grammar rules around annotations have changed recently, did it break (try with a colon)? What does "not having much success" means? Is the annotation getting flagged as illegal or malformed? Please open an issue on the repo with the info - parsing annotations is challenging, because the arguments list needs to parse as such (same rules as a procedure call), but the rest of the comment needs to parse as a comment. — Mathieu Guindon17 secs ago
Interesting, works with a colon, not a semi-colon (not much success = the annotation is being ignored, as if it were an ordinary comment, no illegal annotation inspection, but also no @Ignoring of inspections either). Only posted here because I didn't know what the intended behaviour was — Greedo9 mins ago
I'm wondering if we need to be less permissive with the syntax
e.g. always require it be '@Annotation(parameters)
and not accept '@Annotation parameters. That would be a breaking change but maybe a good one?
@Duga Had a thought now - for the purpose of analyzing the execution path, it should be reasonable to presume that Resume / Resume Next is equivalent to GoTo ErrorLabel; it'll be executing some statements within that "block"; we just don't know which one of them.
@this the way I see it, CPA traversal needs to "run" (big huge neon quotes here) the "happy path" (ignoring OnErrorGoTo jumps), then traverse the tree again but this time following the error paths (each On Error GoTo {Label} instruction makes a separate error path / traversal) - then CPA can know if/when an On Error (except GoTo 0 or -1, perhaps) statement is encountered in any error path, or when a Resume {whatever} statement is encountered in a "happy path"
I honestly can't remember if in the original BASIC you could have GOSUB that does another GOSUB and the RETURN would correctly track which GOSUB it's returning to.
just one more consideration for the happy/unhappy path:
On Error GoTo Fail
Set db = CurrentDb
db.BeginTrans
On Error GoTo TransFail
db.Execute "...", dbFailOnError
db.Execute "...", dbFailOnError
db.CommitTrans
On Error GoTo Fail
Set db = Nothing
ExitHere:
Exit Sub
TransFail:
db.Rollback
Fail:
Msgbox "derp"
Resume ExitHere
@Duga Just so I'm not missing something - what's stopping you from creating a JumpImpl that extends the ExecutionImpl and provides the IJumpNode interface implementation?
otoh, if we're going to fork Sam's work and change how Antlr generates parse trees, we might as well try to turn parse trees into something closer to Roslyn's red/green trees.... not gonna happen though
anyway - now that I have the node extensions, I'm going to start thinking about what we need to have in IExecutionContext
the unified expression evaluation engine should be able to work off IEvaluatableExpression, right?
@this see that is exactly why I haven't added an IExecutionPath interface yet... I keep going back & forth between "I need it" and "wait a BranchNode would do that"
@this Aren't the code-generated classes all marked as partial? New functionality could be added to the same class. (I'm stepping in the middle of the discussion here.)
@ZevSpitz yup, that's exactly what's going on here - thing is, the partial classes already have a base class, to we can add state and implement interfaces, but stuffing common code in a base class isn't possible
however, they are code-gen'd as partial, enabling us to decorate further those classes. But because they already derive from ParserRuleContext, we can't inherit any other types.
Version 2.4.1.4930 OS: Microsoft Windows NT 10.0.15063.0, x64 Host Product: Microsoft Office 2016 x64 Host Version: 16.0.4873.1000 Host Executable: MSACCESS.EXE
**Description** 1) `OK` button is not defaulted. 2) `<Tab>` key moves _into_ the `Preview` box but not out of it 3) `<Down Arrow>` will move the cursor below the visible section of `Preview` box leaving no obvious indication of where you are. 4) There is a preview of
Why? Consider the implementation for IIdentifierContext. AFAICT they look like this: public Interval IdentifierTokens { get { Interval tokenInterval; Identifier.GetName(this, out tokenInterval); return tokenInterval; } } This implementation could be put into the IIdentifierContext interface and not repeated over and over again in the partial extensions.
anyway for IExecutableNode, delegation isn't going to work... each implementing node will need to do something with the IExecutionContext - at this point the context has no members so all Execute(IExecutionContext) does is track whether the node was executed on a per-context basis
Oh, I see. I though default interface implemetnation could be only like public foo DoFoo2() => DoFoo();, referencing an existing member on the interface.
@ZevSpitz, just a hint if you paste code w/CRLF in it, you can hit the fixed font button that appears over there (next to upload) ---> to cause the spacing to be right.
but e.g. SetStmtContext.Execute will need to eventually invoke context.AssignVariable(this); ForNextStmtContext.Execute will need to eventually invoke context.EnterForLoop(this), etc.
My understanding was that it can only refer to other members within the same interface, since it's meant to support extending of an interface without breaking clients that shipped with previous interface.
but I could be mistaken. I'd be very surprised if it let you does that, though.
Generally speaking (because #ItDepends) a class method should ensure that all the class properties that it needs to function have been set prior to attempting to take action, right?
@FreeMan yep! Keep in mind the VBA compiler won't prevent you from Set thing = New Thing : thing.DoSomething() 'Errors because the a and b weren't set....
Just FYI: the CE refreshes really fast on parse now. Resolving References still takes a bit, but even the code inspections seem to run fairly fast. Congrats on all the hard work!!
^nothing empirical, it just feels quicker than it used to, and that's really the most important thing.
And... I'm thinking that if I put a '@Interface annotation at the top of a module, there aught to be some sort of inspection warning me that I've got executable code in my function headers. That shouldn't be legal, should it?
VBA Rubberducking
VBA Rubberducking