Semantic differences in for-loop variable declaration

[x-Aksara-x]

Is your bug report related to the C++ slang project or the pyslang Python bindings?
C++ slang

Describe the bug

If loop counter declared in for-loop initialization block the block statement in AST contains counter as member with initializer. As a result, when visiting the AST, information about the counter initial value is obtained before information about the presence of a loop.

To Reproduce

initial begin
  for (int i = 0;;);
end

Additional context
According to the LRM (IEEE 1800-2023, section 12.7.1 The for-loop, the first example in section), these procedure blocks are equivalent:

initial begin
  for (int i = 0; i <= 255; i++)
  ...
end

initial begin
  begin
    automatic int i;
    for (i = 0; i <= 255; i++)
    ...
  end
end

When constructing the AST, the initial value of i in the first case is stored as an initializer of the StatementBlockSymbol member, which appears in the AST before the ForLoopStatement. In the second case, the initial value of i can be obtained only from ForLoopStatement::initializers. The order in which the initial value of the loop counter is obtained affects the data flow analysis in the slang-tidy detector I am developing.

I suggest to uniform AST representation in both cases as it described by the standard.

[MikePopoloski]

I don't think I understand what the problem is. You don't like that the variable's initializer expression remains with the variable instead of being permuted into an assignment expression in the for loops initializers list? I agree that they result in slightly different ASTs but you have to be able to support the cases where the user deliberately wrote them that way anyway, so I'm not sure what it buys you to try to unify just this one particular subset of the cases.

[x-Aksara-x]

My concern is that the AST should reflect the syntactic structure (and, ideally, the intent expressed in the standard) as closely as possible.

According to the standard, this form of the for loop is equivalent to declaring the variable without an initializer and then performing initialization in the loop initialization block. In other words, it is conceptually closer to:

automatic int i;
for (i = 0; i <= 255; i++);

However, slang currently represents:

for (int i = 0; i <= 255; i++);

more like:

automatic int i = 0;
for (; i <= 255; i++);

instead of the form above.

From an analysis perspective, this makes things a bit harder, because such cases need to be handled separately even though they are specified to be equivalent. It would be more convenient if these equivalent forms were normalized in the AST.

So I’m trying to understand the rationale behind this choice — is it primarily an implementation convenience for building the AST, or is there another reason for preserving the initializer on the declaration node in this specific case?

[MikePopoloski]

The current representation is closer to the actual syntactic structure; that's why it's done this way. The user wrote a variable declaration with an initializer, not a variable declaration with no initializer plus a separate assignment expression. What you are asking for is an additional transformation that takes it further away from the original syntactic structure. Doable of course, but since they are equivalent... why do extra work?

The "intent expressed in the standard" is just an example to demonstrate behavior; both forms are equivalent and the LRM does not say "the compiler's internal AST representation, when serialized back to textual form, must look like this".

[likeamahoney]

Consider this example:

for (int i = 0; i < N; i++) body
// ForLoopStatement::initializers -> [] (empty)
// ForLoopStatement::loopVars -> [i]  (initializer lives in VariableSymbol)

automatic int i; for (i = 0; i < N; i++) body
// ForLoopStatement::initializers -> [i = 0]
// ForLoopStatement::loopVars  -> []

initializers is empty for the compact form, so any tool inspecting only ForLoopStatement::initializers silently misses the loop variable's initial value. The same issue affects --ast-json - loopVars is not serialized by serializeTo, so JSON consumers see no trace of the initializer inside the ForLoop node at all. Also, for the compact form the --ast-json output shows empty initializers with no mention of loopVars at all (it is not serialized), so from a JSON consumer's perspective the loop variable appears to have no initializer whatsoever.

Would it make sense to either normalize both forms into initializers, or at least serialize loopVars and document that the two fields are mutually exclusive representations of the same semantic concept?

[MikePopoloski]

It is already documented:

/// A list of variable initializers (mutually exclusive with @a loopVars )
std::span<const Expression* const> initializers;

/// A list of variables declared in the for loop (mutually exclusive with @a initializers )
std::span<const VariableSymbol* const> loopVars;

If you want to add loopVars to the AST JSON output as a link to the variable symbols that seems fine to me. It is not true however that the loop variable does not have an initializer in the JSON -- it's on the initializer field of the variable symbol. If you add loopVars then users need to know to ignore it because the information already exists elsewhere, otherwise they may "run" the initializer twice or whatever.

The normalized form is essentially indistinguishable from the case where the user just wrote this in their code:

automatic int i = 0;
for (; i < N; i++) begin end

If your tool can handle that case then it also already handles the inline declaration case.