================================================================
			Review of Section 3 of
		      SystemVerilog LRM Draft 4
			  Rishiyur S. Nikhil
			     Feb 10, 2004

----------------------------------------------------------------
Section '3.1 Introduction', para 3

Currently:

    "SystemVerilog 3.1 adds string ..."

Should this be "3.1" or "3.1a" in the final LRM?

----------------------------------------------------------------
Section '3.1 Introduction', para 3

Currently:

    "SystemVerilog 3.1 also extends the user defined types by
     providing support for object-oriented class."

Typo: "class"

Suggest: "... for object-oriented classes."
     or  "... for object-oriented class types."

----------------------------------------------------------------
Section '3.1 Introduction', para 6

Currently:

    "One routine can be written to reverse the order of elements in
     any array, which is impossible in C and in Verilog"

Is this statement too strong?  It can be done (and is done) in C,
just not with type-safety.  Suggestion:

    "..., which is impossible in Verilog and in C with type-safety."

----------------------------------------------------------------
Section '3.2 Data type syntax', Syntax 3-1 table:

Text formatting issue: currently the caption "Syntax 3-1-- data types
(excerpt from Annex A)" is an ``orphan'' on a separate page.
Hopefully once the editorial strikeouts in the table are removed and
the table shrinks, the caption will re-appear below the table.

----------------------------------------------------------------
Section '3.2 Data type syntax', Syntax 3-1 table, line 6

Text formatting issue: In the production "| struct_union ...." the
line wraps around into the next line.  Is that next line indented
correctly (it is indented differently from the corresponding text in
A.2.2.1)?  Is there some standard style for such long lines?

----------------------------------------------------------------
Section '3.2 Data type syntax', Syntax 3-1 table

The production for:    "enum_base_type ::= ..."

is missing its last production as shown in A.2.2.1, i.e.,

    | type_identifier [packed_dimension]^26

----------------------------------------------------------------
Section '3.2 Data type syntax', Syntax 3-1 table

The productions for

    variable_decl_assignment ::=      // from Annex A.2.4"
        variable_identifier variable_dimension [ = expression ]
      | variable_identifier [ ] = new [ expression ] [ ( variable_identifier ) ]
      | class_variable_identifier = new [ ( list_of_arguments ) ]
      | [ covergroup_variable_identifier ] = new [ ( list_of_arguments ) ]^17

at the bottom of the table do not agree with the corresponding
production in Annex A.2.4, which are:

    variable_decl_assignment ::=
        variable_identifier variable_dimension [ = expression ]
      | dynamic_array_variable_identifier [ ] [ = dynamic_array_new ]
      | class_variable_identifier [ = class_new ]
      | [ covergroup_variable_identifier ] = new [ ( list_of_arguments ) ]^17

----------------------------------------------------------------
Section '3.3.2 2-state (two-value) and 4-state (four-value) data types'

Re. the sentence:

    "the difference between int and integer is that int is 2-state
     logic and integer is 4-state logic"

the use of "THE difference" is misleading, since there is also another
difference, namely that int is exactly 32 bits whereas integer is at
least 32-bits.

Suggest changing "the difference" to "one difference".

----------------------------------------------------------------
Section '3.5 void data type'

please append a 3rd sentence to this  section:

    "it can also be used for members of tagged unions (see Section 3.11)."

----------------------------------------------------------------
Section '3.7 String data type', para 2

Re. the sentence:

    "A string literal assigned to a packed array is truncated to the
     size of the array."

Suggested replacement:

    "A string literal assigned to a packed array of a different size
     is either truncated to the left or padded with zeroes to the
     left, as necessary."

----------------------------------------------------------------
Section '3.7 String data type', para 4

After the sentence:

    "Variables of type string can be indexed from 0 to N-1 .."

add a new sentence (Arturo Salz' suggestion):

    "Reading an element of a string yields a byte."

----------------------------------------------------------------
Section '3.7 String data type', para 7

Re. the sentence:

    "The basic operators defined on the string data type are listed in
     Table 3-2, which follows."

Suggestion (1): Bold the word "string"

Suggestion (2): Delete the phrase ", which follows", since the table
                does not immediately follow (it's on the next page).

----------------------------------------------------------------
Section '3.7 String data type', para 8

Re. the sentence:

    "If their size differs the literal is right justified and zero
     filled on the left."

Suggested change:

    "If their size differs the literal is right justified and either
     truncated on the left or zero filled on the left, as necessary."

----------------------------------------------------------------
Section '3.7 String data type', para 9

Re. the sentence:

    "The string variable shall grow to accommodate the packed array."

Should this be the following?

    "The string variable shall grow or shrink to accommodate the packed array."

----------------------------------------------------------------
Section '3.7 String data type', Table 3-2 on String operators:

Add a new row to the table, before the Str.method() row:

    +--------------+----------------------------------------------------+
    |  Str[index]  | Indexing.  Returns a byte, the ASCII code at the   |
    |              | given index.  Indexes range from 0 to N-1, where   |
    |              | N is the number of characters in the string. If    |
    |              | given an index out of range, returns 0.            |
    |              | Semantically equivalent to Str.getc(index), in     |
    |              | section 3.7.3.                                     |
    +--------------+----------------------------------------------------+

----------------------------------------------------------------
Section '3.7 String data type', Table 3-2 on String operators:

Re. the "{multiplier{Str}}" entry, the sentence:

    "Multiplier must be of integral type"

Integral types include "time" (Sec. 3.3.1).  Is this intended here?
Should this be "integer type" instead of "integral type"?

----------------------------------------------------------------
Section '3.7.1 len()', first sentence:

    "... (excluding any terminating character)."

This is the first mention of ``terminating character''.  My reading of
the string data type section is that, unlike C, SV does not require
the concept of terminating characater at all, i.e., a string just
contains the characters it is supposed to contain, and it is an
implementation's choice about how to represent this, e.g., with a
terminating character or an explicit character count or whatever.

Suggestion:

    Delete the phrase "(excluding any terminating character)."

----------------------------------------------------------------
Section '3.7.3 getc()', last sentence:

Replace:

    "Note: x = str.getc(j) is identical to x = str[j]."

with:

    "str.getc(j) is semantically equivalent to str[j]."

----------------------------------------------------------------
Section '3.7.6 compare()',  first sentence, and
Section '3.7.7 icompare()', first sentence:

    "..., with a compatible return value."

What is a 'compatible return value'? Instead, why not just spell it
out? Suggested change:

    "..., returning an integer less than, equal to, or greater than 0
     if str is found to be less than, equal to, or greater than s,
     respectively."

----------------------------------------------------------------
Section '3.7.9 atoi(), atohex(), atooct(), atobin()'

Re. the sentence:

    "The string is converted until the first non-digit is encountered."

Suggest changing this to (per clarification from Arturo Salz):

    "The conversion scans all leading digits and underscore characters
     (_) and stops as soon as it encounters any other character, or
     the end of the string.  Returns zero if no digits were
     encountered.  It does not parse the full syntax for integer
     literals (sign, size, tick, base)."

----------------------------------------------------------------
Section '3.7.10 atoreal()'

Suggest adding a bullet point:

    "The conversion parses Verilog syntax for real constants.  The
     scan stops as soon as it encounters any character that does not
     conform to this syntax, or the end of the string.  Returns zero
     if no digits were encountered."

----------------------------------------------------------------
Section 3.7.11 through 3.7.14

Unnecessary style difference:

    3.7.9 lists atoi(), atohex(), atooct() and atobin() in 1 subsection

    3.7.11 thru 3.7.14 list itoa(), hextoa(), octtoa() and bintoa() in
        4 separate subsections.

Suggestion:

    Merge 3.7.11 to 3.7.14 into a single subsection, like 3.7.9

----------------------------------------------------------------
Section '3.9 User-defined Types', para 5 at bottom of page 15

Typo:        "... within an interfaces ..."

Should be:   "... within an interface ..."

----------------------------------------------------------------
Section '3.9 User-defined Types', final para:

    "A typedef inside a generate shall not define the actual type of a
     forward definition that exists outside the scope of the forward
     definition."

I don't understand this.  Should it read as follows?

    "A typedef inside a generate shall not define the actual type of a
     forward definition that exists outside the scope of the
     generate."

----------------------------------------------------------------
Section '3.10 Enumerations', Syntax 3-3 box

'enum_base_type' is missing its last production as shown in A.2.2.1,
namely,

    "| type_identifier [ packed_dimension ]^26"

----------------------------------------------------------------
Section '3.10 Enumerations', first sentence:

    "An enumerated type declares a set of integral named constants."

But "integral" includes "time" (see 3.3.1).  Is this intended?
Should it be the foll.?

    "   ...                            of integer named constants."

----------------------------------------------------------------
Section '3.10 Enumerations', second para:

    "In the absense of a data type declaration, the default data type
     shall be int.  Any other data type used with enumerated types
     shall require an explicit data type declaration."

I'm not sure "data type declaration" is appropriate here, rather it is
a "base data type specification".

Suggest rewording:

    "In the absense of a base data type specification, the default
     data type shall be int.  Any other base data type used with
     enumerated types shall require an explicit data type
     specification."

----------------------------------------------------------------
Section '3.10 Enumerations', second example, after 4th para:

    "// Syntax error: IDLE=2'b00, XX=2'bx <ERROR>, S1=2'b01??, S2=2'b10??"

The notations "2'b01??" and "2'b10??" are confusing, since in Verilog
"?" is a legal equivalent to "z", and these can then be interpreted as
4-bits (01zz and 10zz) in constants that are explicitly declared with
2' to be 2 bits wide.

I suspect the question marks were not supposed to be part of the
constant, i.e., they were comments like <ERROR>.

Suggested change:

    "// Syntax error: IDLE=2'b00, XX=2'bx <ERROR>, S1=2'b01 <??>, S2=2'b10 <??>"

or just drop the question marks altogether:

    "// Syntax error: IDLE=2'b00, XX=2'bx <ERROR>, S1=2'b01, S2=2'b10"

----------------------------------------------------------------
Section '3.10.2 Enumerated type ranges', Table 3-3

In 3rd row, typo:    "...,name N-1 N must be ..."
missing period:      "...,name N-1.  N must be ..."

----------------------------------------------------------------
Section '3.10.2 Enumerated type ranges', Table 3-3

In rows 3,4, 6, use of the phrase: "integral constant"
But integral constants include "time".  Is this intended?
Should these be "integer constant"?

----------------------------------------------------------------
Section '3.10.2 Enumerated type ranges', last example

I believe "register[1]" is a mistake.
Should be "register[2]".

----------------------------------------------------------------
Section '3.10.3 Type checking', para 2, 3, ..

Para 2: "... can be set to any integral constant value,..."
Para 3: "... are aut-cast into integral values ..."

Should these occurrences of "integral" be "integer",
since the former includes "time"?

----------------------------------------------------------------
Section '3.10.4 Enumerated types in comparison expressions'

The body of the section (1 sentence) seems unrelated to the title of
the subsection.

Suggestion: simply remove the section title, since the sentence
naturally fits into the previous subsection '3.10.3 Type checking'.

----------------------------------------------------------------
Section '3.10.5 Enumerated types in numerical expressions', para 2
just after the first example.

Re. the sentence:    "Next, it assigns b a value of 4 (3+1)"

Suggest dropping "Next, ", since the next statement in the example
is actually "c = yellow".

----------------------------------------------------------------
Sections '3.10.5 Enumerated types in numerical expressions', para 3

Re. the sentence:

    "An assignment to an enum variable from an expression other than
     an enum variable or identifier of the same type shall require a
     cast."

But this would imply that the following example statements in 3.10.5.7
are illegal:

    "Colors c = c.first;"
    "c = c.next;"

because each RHS expression is not an enum variable or identifier of
the same type (they are method calls).

Suggestion: just simplify the sentence to:

    "An assignment to an enum variable from an expression of other
     than the same type shall require a cast."

This would correctly disallow the (deliberate) examples of illegal use
that follow the para, such as "C = C + 1" because the RHS would be of
type int instead of Color, while allowing "c = c.next".

----------------------------------------------------------------
Sections '3.10.5.1' - '3.10.5.4'

In all 4 subsections, the keyword 'enum' is used to refer to an
enumeration type.  While the intent is clear, this is not strictly
kosher.

Should they use, e.g., 'Enum' instead?

----------------------------------------------------------------
Sections '3.10.5.3', '3.10.5.4' and '3.10.5.6'

Re. the sentence:

    "If the given value is not a member of the enumeration, ..."

Suggested clarification:

    "If the current value is not a member of the enumeration, ..."

----------------------------------------------------------------
Section '3.11 Structures and unions'

Last-but-one line on page 21:    "num n;"

Indentation is wrong; should be aligned with "typedef" above it and
"n.f" below it.

----------------------------------------------------------------
Section '3.11 Structures and unions', near bottom of page 22

Re. the sentences:

    "Non-integer data types, such as real and shortreal, are not
     allowed in packed structures or unions.  Nor are unpacked arrays."

Suggested clarification:

    "Packed structures or unions shall not contain any non-integer
     data types, such as real and shortreal, nor any unpacked types
     such as unpacked arrays, structures or unions."

----------------------------------------------------------------
Section '3.11 Structures and unions', 1st para at top of page 23

Re.    "... when writing the 2-state bit member."

Delete the word "bit", i.e., change to:

       "... when writing the 2-state member."

----------------------------------------------------------------
Section '3.11 Structures and unions', diagram just before Section
3.12

The legend:    "tag is 0 for Add, 1 for Jmp"

should be replaced by:

    "Outer tag is 0 for Add, 1 for Jmp
     Inner tag is 0 for JmpU, 1 for JmpC"

----------------------------------------------------------------
Section '3.12 Class', second sentence

Re. "The data in a class is referred to as class properties..."

Typo, "is" should be "are", i.e.,

    "The data in a class are referred to as class properties..."

----------------------------------------------------------------
Section '3.12 Class', first example, just before 3.13

Typo: "class Packet"

Missing semicolon at end, i.e., should be:

      "class Packet;"

----------------------------------------------------------------
Section '3.13 Singular and aggregate types', 1st para

Re. "... represents a single value, symbols, or handle."

I don't understand what "symbols" means in this sentence.

----------------------------------------------------------------
Section '3.15 $cast dynamic casting'

In the given prototypes:

    function int $cast ( singular dest_var, ...);
    task         $cast ( singular dest_var, ...);

the spec for first argument is not really kosher. In calls such as:

    ... $cast (x, ...) ...

the normal function call semantics would pass the _value_ of x as the
argument, whereas the function/task needs to assign to the variable x.

Perhaps it would be more accurate for the prototypes to use the 'ref'
qualifier:

    function int $cast ( ref singular dest_exp, ...);
    task         $cast ( ref singular dest_exp, ...);

In addition to making it kosher, 'ref' also allows 'dest_exp' instead
of 'dest_var', enabling more general calls, such as:

    ... $cast (y[3], ...) ...
    ... $cast (z.member, ...) ...

i.e., cast and assign to an element of an array or to a member of a
struct.

================================================================