tomlokhorst / language-cil

Manipulating Common Intermediate Language AST in Haskell
Other
20 stars 8 forks source link

Is MethodDecl a good name? #12

Open dmcclean opened 13 years ago

dmcclean commented 13 years ago

Is MethodDecl really a good name for the construct that is currently named that?

I agree that the production in the Partition VI, Annex C, Section C.3 grammar most closely matching that concept is named methodDecl.

On the other hand, the common-sense reading of MethodDecl suggests that it should be a declaration of a method, not one tiny particle in the implementation of a method.

Supporting my case for renaming it, and supplying my proposed alternative name, is Partition II, Section 15.4.1. In Section 15.4.1 a closely-related concept is named MethodBodyItem, which I would argue is a much more intuitive name.

The productions listed for MethodBodyItem cover all the alternatives in the existing MethodDecl type except for comments and labels. I think the reason they are excluded is because Partition II doesn't deal with the concrete syntax of CIL at all; in fact, as far as I can see, it doesn't even deal with the instruction set at all. Thus omitting comments make sense, and omitting labels makes sense because at this level all the labels have been converted to offsets.

I propose renaming MethodDecl (including its Comment and Label alternatives) to MethodBodyItem.

tomlokhorst commented 13 years ago

I should note that I didn't use Microsoft's documents when writing the initial version of this library, but rather Expert .NET 2.0 IL Assembler by Serge Lidin.

I've taken most names for data types in Language.Cil.Syntax from Appendix A, ILAsm Grammar Reference. In particular, these are the productions for methods:

classDecl ::= methodHead methodDecls }
            | ...

methodHead ::= .method methAttr callConv paramAttr type marshalClause
                  methodName typarsClause ( sigArgs0 ) implAttr {

methodDecls ::= /* EMPTY */
              | methodDecls methodDecl

methodDecl ::= .emitbyte int32
             | mehBlock
             | .maxstack int32
             | .locals ( sigArgs0 )
             | ...
             | instr
             | id :                       /* label */
             | secDecl
             | ...

As far as I can see, MethodBody is not mentioned in the grammar reference, but it is mentioned elsewhere in the book. I'll reread those parts as well as the sections you mentioned. I'm assuming there's a good reason for the distinction between MethodBody and MethodDecl, but I don't understand that yet.

dmcclean commented 13 years ago

That is a decent book. The excerpted definition of methodDecl looks like a simplified (by replacing abstracted non-terminals with their concrete representations) and slightly respelled version of the grammar in Annex C, C.3.

methodDecl : '.emitbyte' int32
  | sehBlock
  | '.maxstack' int32
  | localsHead '(' sigArgs0 ')'
  | localsHead 'init' '(' sigArgs0 ')'
  | '.entrypoint'
  | '.zeroinit'
  | dataDecl
  | instr
  | id ':'
  | secDecl
  | extSourceSpec
  | languageDecl
  | customAttrDecl
  | '.export' '[' int32 ']'
  | '.export' '[' int32 ']' 'as' id
  | '.vtentry' int32 ':' int32
  | '.override' typeSpec '::' methodName
  | scopeBlock
  | '.param' '[' int32 ']' initOpt
  ;

See also the list of productions for Decl at Partition II, Section 5.10 for support for the MethodBodyItem name.

I don't see MethodBody defined anywhere in the Annex C, C.3 grammar anywhere either, which possibly explains why it isn't in the book.

In fact, I don't see it anywhere in the Partition II explanations either. They seem to jump up a level and define productions for Decl and ClassMember directly in terms of MethodBodyItem. as:

Decl ::= ...
          | .method MethodHeader '{' MethodBodyItem* '}'

ClassMember ::= ...
                        | .method MethodHeader '{' MethodBodyItem* '}'

It's at page 202 in the 5th Edition of ECMA 335.