tgjones / HlslTools

A Visual Studio extension that provides enhanced support for editing High Level Shading Language (HLSL) files
http://timjones.io/blog/archive/2016/04/25/hlsl-tools-for-visual-studio-v1.0-released
Other
568 stars 98 forks source link

Added support for struct methods #57

Closed OndrejPetrzilka closed 7 years ago

OndrejPetrzilka commented 7 years ago

Added support for struct methods, added new testfile StructMethods.hlsl, fixes #53

StructTypeSyntax and ClassTypeSyntax are looking very similar now, same with BoundStructType BoundClassType, there's some duplicated code which could use refactoring.

Making base class ObjectTypeSyntax and BoundObjectType might be useful. On the other hand, it might be better to just remove one of the types. Let me know how'd you prefer to refactor it.

Does anyone know if HLSL treats struct and class exactly same or whether there are some differences? E.g. are interfaces allowed on structs?

OndrejPetrzilka commented 7 years ago

Should be fine now (hadn't installed VSIX development project and missed some build errors)

tgjones commented 7 years ago

I've heard from my acquaintance on the HLSL compiler team: class and struct are indeed synonyms, at least in the current HLSL version.

So I think we should just have one set of types - StructTypeSyntax, BoundStructType, etc. We should still keep SyntaxKind.StructKeyword and SyntaxKind.ClassKeyword, for stuff that still needs to know about the actual keyword (for example the quick info hover tooltip). But we can remove a lot of other code - ClassTypeSyntax, BoundClassType, etc. When the parser creates a StructTypeSyntax, it can pass in the relevant keyword to the structKeyword argument.

If in a future version of HLSL, struct and class have differences, we can address that at that stage.

@OndrejPetrzilka are you okay making these changes?

OndrejPetrzilka commented 7 years ago

OK, this is now in state I intended originally, lets discuss whether there should be some more changes.

tgjones commented 7 years ago

This looks good to me - I see now why SymbolKind.Struct and SymbolKind.Class need to exist.

Thank you very much!