Support attributes?

[mansellan]

Discussion: should twinBASIC support attributes?

Describe the solution you'd like
VB.Net (and C#) allow attributes to be attached to various declarations, which can then be used at runtime. These can be useful for scenarios like serialisation, display, decoration etc.

Describe alternatives you've considered
Rubberduck works around the lack of attributes by using a convention-based comment format. Whilst this works well, it requires explicit tooling support for each scenario and is not user-extensible.

Additional context
This might be opening a can of worms, as attributes are only useful if they can be reflectively retrieved at runtime. If support were to be implemented, it should probably syntactically follow the format used in VB.Net.

EDIT: Dropped suggestion to follow VB.Net syntax

[WaynePhillipsEA]

My concern is that it could get messy with having support for VB.NET attributes, RubberDuck attributes, and also the old style Attribute VB_* attributes. I'll leave this open for discussion though, as I'm not technically opposed to it.

It might be that we can "auto-correct" the three forms into a single authoritative form.

[bclothier]

FWIW, I am in favor not having multiple ways of doing the same and "auto-correct" into a canonical form sounds like the best solution especially considering that the Attribute statements aren't very intuitive to read or use.

I am somewhat ambivalent about the idea of having the equivalent of VB.NET attributes. It's important to note that those exist primarily for the reflection purpose. By itself, it does nothing useful. However, it can be used as some kind of "compiler directive". One good use I found for Rubberduck project is to prefer this version over the latter version:

[Conditional("DEBUG")]
public void Foo() ...

vs.

#if DEBUG
public void Foo()...
#end if

I will refer the readers to this excellent blog which explains why the former form is a superior solution over the latter. Suffice to say that the former ensures that the unused branch still gets validated by the compiler and thus prevent the source code from becoming incorrect as things changes.

TBH I can't imagine doing any useful metaprogramming without the attributes but my imagination might be too constrained by my familiarity with .NET and lack of familiarity with Lisp. 😆 So to summarize, if twinBASIC has plans for reflection & metaprogramming, it may need to solve how it will handle the metadata that is useful only for reflection/metaprogramming. As the example above shows, there's value in having the metaprogramming be also code as opposite to a 2nd "language" superimposed as is the case with current compiler directives.

One additional consideration, however, is that in .NET, the attributes are shipped as metadata with the assembly. Since twinBASIC is unmanaged, it probably can make use of the custom that is provided with ITypeLib2/ITypeInfo2 interfaces but it would be on twinBASIC to interpret the meaning of the data and it may not be accessible to non-twinBASIC COM consumers (they can see and read the raw value but they'll have to know how to parse it to use it the same way twinBASIC would).

One thing I'm sure about, though is that I very much dislike VB.NET's syntax for attributes. 😄

[retailcoder]

I concur: nothing against attributes per se, but needs a proper reflection API to be useful, and I will actively boycott the feature if it involves <AngleBracketSyntax> ..nothing against [SquareBrackets] though... could be my own rather heavy C# bias though.

[mansellan]

Heh, I think we can agree that <AngleBracketSyntax> is fairly obnoxious. Let's not do that.

VB6\VBA attributes are of the form Attribute {Name} = "{Value}" and are somewhat described here, however the docs do not tell the whole story.

1. There is a special case for VB_Ext_Key, which takes two strings: Attribute VB_Ext_Key = "MyKey", "My Value".
2. VBA hosts can write custom attributes into document-type components, see Rubberduck #1230

Perhaps we could devise a syntax that acts as an alias to these existing forms, and also provides capabilities for the future. For example:

[VB_Description("Common functions for cryptography")]

would be treated as exactly equivalent to:

Attribute VB_Description = "Common functions for cryptography"

But in the future, the syntax would also allow:

[SomeAttribute("Some description", 1, True, "Some other string")]

EDIT: VB_Name is a poor example, as modules are named explicitly in tB. Updated to VB_Description.

[WaynePhillipsEA]

The other thing to bear in mind is that we will be supporting external CLS/BAS files, so we need to be mindful that the solution we choose should keep the source backwards compatible with VB6. Rubberduck style annotations, being inside code comments would definitely fulfill that criteria.

I wonder whether we could hide the VB6 style attributes in the editor, and display them in a more friendly manner via CodeLens? So they would still be authoritative, but the user doesn't have to deal with them directly. Thoughts?

[mansellan]

I thought the goal for compatibility was that tB should be able to compile all existing VB6 code without changes. I.e. VB6 is forward-compatible with tB, and tB is a superset of VB6. There are many changes already that would make tB code not compile in the VB6 IDE (generics, overloads).

I see this as just being another such case - a new syntax that is only available in tB?

[WaynePhillipsEA]

True, however it feels a little uncomfortable to have tB effectively trash a CLS/BAS file just by opening it and having it auto-correct everything to an unsupported syntax (in VB6s eyes). At least making changes such as overloads and generics are an active choice made by the programmer. I'm not against the syntax bring suggested at all by the way... Just trying to be as considerate to the old file formats as much as possible.

It's a discussion for another day, but it could be that we want to restrict the new VB6-incompatible features to just TWIN files anyway.

[retailcoder]

So there's classic-VB attributes, using the Attribute keyword. We want this syntax fully supported in twinBASIC, but since it works with obscure magic values and named attributes, twinBASIC should treat them as legacy syntax.

Then there's Rubberduck's @Annotation comments, currently partially supported but with plans to support more. These annotations sometimes have an Attribute equivalent, but Rubberduck also uses annotations for various meta-purposes, from marking a member as @Obsolete to discovering a @TestModule.

This syntax poses a number of problems. It's difficult to annotate a particular method parameter, for example; Rubberduck may eventually extend its annotation parameterization to take a parameter name, but then annotations quickly stack up on top of procedures, and soon you have more magic-syntax comments than actual code.

Square-bracket attributes à la C# would be similar to Rubberduck annotations, but they wouldn't be a comment syntax and they could be interleaved between parameter declarations, making it possible and relatively much easier to support attributes like [NotNull] (or would that be [NotNothing]?) or [CallerMemberName] on parameters.

If/when non-comment actual custom attributes are supported, twinBASIC should offer to "translate" both the legacy Attribute statements and the Rubberduck @Annotation comments to their square-bracket equivalent:

• [Description("...")
• [Obsolete("...")]
• [DefaultMember]
• [Enumerator]
• [TestMethod("...")]
• ...

That way there would be One Clear Way of doing things the twinBASIC way, while legacy statements and comments retain their legacy meaning and functionality.

[mansellan]

Ah yes sorry I see, I didn't explain myself correctly. I was proposing that either VB6 form or the new form would be acceptable syntax to tB. No auto-correction, it would be a manual and conscious decision to change to the new form if desired.

[mansellan]

Heh, @retailcoder said it much better than me :-)

[WaynePhillipsEA]

Ah I see. Sorry, probably my fault, as there was talk of auto-correct etc. Manual changes that break VB6 back compat in fine with 😀

[bclothier]

I agree that when handling a bas/cls file there should be no autocorrection, but it sounds like it would significantly complicate the mapping. Example: Import a Attribute VB_Description "foo", display [Description("foo")] in VSC. User edits it to [Description("bar")]. What happens?

I am not familiar with CodeLen so not sure how that will help. I do think we need one canonical way of doing attributes.

[mansellan]

I would suggest that it should continue to display Attribute VB_Description "foo" (ideally with a squiggly for "Do you want to change this to modern syntax")

[mansellan]

The old VBIDE behaviour of hiding things was deeply unhelpful IMO - I prefer my IDE to show every line of text in the source file (just with folding and intellisense etc)

[bclothier]

I like the idea of not changing the syntax with the option to change it to modern syntax. That becomes an explicit choice on the user's part while simplifying the compiler's job of parsing the legacy code.

[WaynePhillipsEA]

CodeLens is a bit like the inline parameter hints, but shown immediately above the definition, where you can show the number of references etc. In this case we'd have it show the aggregated attributes from the Rubberduck annotations and the VB6 style attributes, in our nice [] style, whilst hiding the Rubberduck annotations and the VB6 style attributes. But on the other hand I do agree that hiding bits of the source file seems like a step backwards.

[WaynePhillipsEA]

So the consensus seems to be to introduce the square bracket (third) syntax, and then have the old two syntax forms become legacy supported with recommendations and quick fixes to change to the new syntax?

[Kr00l]

So there's classic-VB attributes, using the Attribute keyword. We want this syntax fully supported in twinBASIC, but since it works with obscure magic values and named attributes, twinBASIC should treat them as legacy syntax.

What you mean with magic value?
VB_UserMemId for instance look like magic, e.g. when set to "-4". See below example:

Public Function NewEnum() As IEnumVARIANT
Attribute NewEnum.VB_UserMemId = -4
Attribute NewEnum.VB_MemberFlags = "40"
Set NewEnum = PropBand.[_NewEnum]
End Function

However, the -4 is not magic. It maps to DISPID_NEWENUM.
See https://docs.microsoft.com/en-us/previous-versions/windows/desktop/automat/dispid-constants

Or do you mean something different?

[bclothier]

Yeah. I can see CodeLen being useful in displaying the legacy syntax as that has the additional advantage of not accidentally modifying it in code. That can be then fixed with a quickfix to convert into modern syntax. The thing is that if someone then adds the modern syntax, it should override the original syntax to avoid having conflicting values.

However, the -4 is not magic. It maps to DISPID_NEWENUM.

That's exactly what we mean by magic numbers. I'd rather read DISPID_NEWENUM, not -4. Note that the member flags is also confusingly a hex string. Those stuff increases the cognitive load on the user and is ultimately distracting from the main goal of solving business problems with twinBASIC. As a matter of fact, Rubberduck annotation was made to avoid those "magical values" and aid in code readability.

[mansellan]

I guess it does raise the question of whether the canonical form should be [VB_Description("Foo")] or [Description("Foo")] - the VB_ seems redundant?

[retailcoder]

@mansellan shouldn't it be TB_ then? ...I'd just drop any prefixing 😉

[mansellan]

But if we're mapping to the old form, I guess it would need to be done for all known VB_ attributes. That's a valid approach, given that this is a brand new syntax. But it does mean that these attribute names become "reserved", and would not be available to a later reflection-based system.

[retailcoder]

Hm, I was thinking [Description("...")] would be the reflection-based system.

[mansellan]

Yes, agreed. But if we're looking to treat legacy VB6/VBA attributes as freely interchangeable with their modern equivalents, then certain attribute names need to have special effect. VB_Description would be exactly equivalent with [Description()], so both would lead to an entry in the TLB.

In a future world with tB reflection, attributes would be defined in code. But they wouldn't be able to use the same names as the legacy-mapped attributes, as those are special-cased.

[mansellan]

Which might be an argument for keeping the legacy VB_ prefix, to avoid such clashes?

[bclothier]

Nah, not concerned because those will be different syntax.

canonical tB:

[Description("foo")]
Public Sub Foo

vs. (imported from VB6/VBA):

Public Sub Foo
  Attribute VB_Description = "foo"

In tB, we would have the option of converting the legacy Attribute into the canonical form. Thus, no need to keep a list of reserved words since they will be translated and should drop kick the cheap parlor two-bit prefix back into the 80s. 😄

[mansellan]

But what happens if a user wants to do this:

Public Class Description(Description As String)
  Implements Attribute

  Private ReadOnly _description As String

  Public Sub New(description As String)
    _description = description
  End Sub

End Class

Oops, there's already a built-in attribute called Description...

[bclothier]

You're assuming that twinBASIC parser can't tell a difference between a procedure named Description vs. an attribute of Description type. Even VBA/VB6 can do Dim Foo As Foo. I think it'll be fine. :)

[bclothier]

For the sake of consistency and simplicity, I stand by the comments I made in the NotDispatchable issue about using attributes.

I get that it would be convenient to not have define an interface but the problem with the Err.LastHRESULT is that it forces us to write code differently and inject a whole error handling when it's not really an error:

Public Sub DoAThing()
  ' reams of code...
  On Error Resume Next
  CallThisFunkyMethod()
  If Err.LastHRESULT = S_FALSE Then
    'It's OK.
  ElseIf FAILED(Err.LastHRESULT) Then
    'Not OK. Let's panic.
  End if
  On Error GoTo 0 'Or some other label
 
  'more reams of code
End Sub

I would much rather not have to add the OERN block. Furthermore, it wouldn't help with the methods that does NOT return a HRESULT at all.

That said, maybe this may be possible --- support preserving the signature at the call site not just at the definition of the interface:

Public Sub DoAThing()
  ...
  [ CaptureHRESULT() ]
  hr = CallThisFunkyMethod()
  ...
End Sub

This removes the need to wrap error handling but the downside is that it requires making assumptions about how the method is actually defined (e.g. HRESULT must be the return value, the retval parameter must be last parameter) and wouldn't handle non-HRESULT methods.

[Kr00l]

I am not sure if the OERN block would be necessary.

I assume it would be similar to like Err.LastDllError. (?)

[WaynePhillipsEA]

but the problem with the Err.LastHRESULT is that it forces us to write code differently and inject a whole error handling when it's not really an error:

I'm not sure I'm following. Why do we need to add the OERN? If we expect that the callee returns S_FALSE, then there is no change to error handling, we can just read Err.LastHRESULT after the call as it will be populated regardless of the error handling state.

For Err.LastHRESULT, I quite like that it is similar in vein to the already existing Err.LastDllError property, and when real errors occur, normal VBx error handling will kick in as expected (instead of having to handle the negative HRESULTs manually, as is the case with PreserveSig).

Public Sub DoAThing()
  ...
  [ CaptureHRESULT() ]
  hr = CallThisFunkyMethod()
  ...
End Sub

We can't really have attributes inside procedure blocks, as the square brackets are allowed as symbol identifiers.

[bclothier]

Maybe I'm working on wrong assumptions. Right now in VBx world, if one calls a method that returns a S_FALSE, does that becomes an error in VBx? Or is it ignored like when calling the Declare statement that supports SetLastError? Is it also true for any other non-error HRESULT?

[Kr00l]

Maybe I'm working on wrong assumptions. Right now in VBx world, if one calls a method that returns a S_FALSE, does that becomes an error in VBx? Or is it ignored like when calling the Declare statement that supports SetLastError? Is it also true for any other non-error HRESULT?

S_OK and S_FALSE are both success code. So no error. No OERN needed.

[WaynePhillipsEA]

Maybe I'm working on wrong assumptions. Right now in VBx world, if one calls a method that returns a S_FALSE, does that becomes an error in VBx? Or is it ignored like when calling the Declare statement that supports SetLastError? Is it also true for any other non-error HRESULT?

All positive HRESULTs are ignored as they are not errors.

[bclothier]

One more consideration. As mentioned, this assumes that the return value of the method is a HRESULT. We have no guarantee that funky method won't return a non-HRESULT integer. We wouldn't know the difference and in this set, the LastlHRESULT would get set with that integer even though it's not really a HRESULT. Is that a concern?

[WaynePhillipsEA]

One more consideration. As mentioned, this assumes that the return value of the method is a HRESULT. We have no guarantee that funky method won't return a non-HRESULT integer. We wouldn't know the difference and in this set, the LastlHRESULT would get set with that integer even though it's not really a HRESULT. Is that a concern?

No that won't happen. Err.LastHRESULT is only populated when the member is defined with a HRESULT. I know this, because I already implemented it ;)

[WaynePhillipsEA]

Here's a few of the tests I wrote to give you a proper sample:

[Kr00l]

I find Err.LastHRESULT too bold. IMO

Maybe Err.LastHresult (common naming LastHresult for windows; according to my google search)

[Kr00l]

The german translator messed it up.

It's LastHResult

example -> https://docs.microsoft.com/en-us/dotnet/api/system.windows.forms.visualstyles.visualstylerenderer.lasthresult?view=windowsdesktop-6.0

[bclothier]

No that won't happen. Err.LastHRESULT is only populated when the member is defined with a HRESULT. I know this, because I already implemented it ;)

Out of curiosity, does that depend on having a type library to infer that it's a HRESULT and not some other int-like data type?

EDIT: to clarify why I'm asking --- I'm looking at implementing undocumented interfaces and I would need to define it so in twinBASIC. But HRESULT isn't a data type.

[WaynePhillipsEA]

Out of curiosity, does that depend on having a type library to infer that it's a HRESULT and not some other int-like data type?

Yes, internally we have a HasHRESULT flag on each member, which gets picked up from the type library. For any interface members defined internally in your project, they will always have that flag, until we support PreserveSig.

[wqweto]

@bclothier HRESULT shenanigans happen on method/props only. It's hard not to return HRESULT from dual/dispinterfaces because these non-HRESULT methods cannot be called through IDispatch.

JFYI, a VB Function Test() As Long method gets compiled to HRESULT Test([out, retval] long *pRet) because VB6 (and TB) produce dual interfaces only so every method is converted to HRESULT.

[bclothier]

Right but in my case, I'm implementing an interface that comes from DLL with no type library and converting C++ code into a tB compatible interface. Truth be told, it'd be easier if I had a type library because it's easier to see whether there's a retval parameter and do the translation. But with C++ code, the MIDL attributes aren't there so it's not as easy to tell whether it's a Sub(x As y) vs. Function() As y. Not having to do the translation at least eliminate that uncertainty from making a mistake in the translation.

[WaynePhillipsEA]

Err.LastHResult is now included in BETA 29

[bclothier]

Just out of curiosity, will the access to the Err be thread-safe? Likewise, will those 2 new properties be fixed to a thread that's being run on?

[WaynePhillipsEA]

Yes, thread-aware and thread-safe. The two new properties, Err.ReturnHResult and Err.LastHResult are both tied to the exact callframe in which they are called. You can think of them as being local variables, controlled by the compiler.

[Kr00l]

Will the Err.ReturnHResult and Err.LastHResult be allowed to be used for ordinary Sub procedures as well ? Means no class member..

[WaynePhillipsEA]

Err.ReturnHResult will only work in cases where the procedure signature defines a HRESULT return value, so no this won't work for any procedures in a standard module and you'll get a compile error.

Err.LastHResult is only populated after a call to a procedure that has a HRESULT return value. So the value can be read in all situations, but is only populated after a class member call.

[mansellan]

@bclothier this should probably move to lang-design and (IMO) be closed. Attributes are now supported, which is the OP, and are a pretty big discussion area. Could probably do with a tag to use when specific new attributes (or discussion around custom attributes) is raised.

[bclothier]

I understand where you are going with the suggestion. We already have other issues on the attribute subject. However, I didn’t close the issue because we still have outstanding items to implement and didn’t transfer because adding new attributes isn’t a change in language.

[rexxitall]

I dislike the .net syntax. it does look like a cruel mix of BASIC and HTML spiced up by c :/ A more basic like style would be using the # syntax of conditional defines. And in this case i suggest to use '# to keep backward copability. Otherwise we have a nice #if #endif mess. It would be in any case handy if the IDE let us the choice what we want to see. I would love to have the ability to hide "dead" code by #if and friends as well as to get rid normal coments as well as conditioonal defines. It makes the algorithm often very unreadble.