feature: conditional directives should be organized like normal

[superzanti]

Did you check the tree-sitter docs?

• [X] I have read all the tree-sitter docs if it relates to using the parser

Is your feature request related to a problem? Please describe.

No, but to successfully do folds with conditional directives the formatting would need to be the same.

Describe the solution you'd like

The following test code:

        `if VHDL_VERSION > "2000" then
            `warning "This version is cool"
        `else
            `error "This version is bad"
        `end if

Produces the following tree:

(if_conditional_analysis
    (IF)
    (conditional_analysis_expression
        (conditional_analysis_relation
            (library_function)
            (string_literal)))
    (THEN))
(warning_directive
    (directive_warning)
    (string_literal))
(else_conditional_analysis
    (ELSE))
(error_directive
    (directive_error)
    (string_literal))
(end_conditional_analysis
    (END)
    (IF))

Describe alternatives you've considered

If it instead produced this tree, it could be handled much better:

(if_conditional_analysis
    (IF)
    (conditional_analysis_expression
        (conditional_analysis_relation
            (library_function)
            (string_literal)))
    (then_conditional_analysis
        (THEN)
        (warning_directive
            (directive_warning)
            (string_literal)))
    (else_conditional_analysis
        (ELSE)
        (error_directive
            (directive_error)
            (string_literal)))
    (end_conditional_analysis
        (END)
        (IF)))

Additional context

No response

[jpt13653903]

I'll need to think about this one. Tool directives are completely outside the grammar of the language, so you could quite happily write something like:

`if DEBUG = true then
  for n in 1 to 10 loop
`else
  for n in 1 to 15 loop
`end if
    A := A + B(n);
end loop;

How would you construct that tree?

[jpt13653903]

I had a look at how C does it, which is exactly in line with your suggestion.

I don't like the fact that they simply let the second case produce an (ERROR) node though... I believe that that should not be an error, because it is perfectly valid within the grammar.

The test cases I used are:

#ifdef DEBUG
  printf("This is debugging...");
#else
  printf("This is production...");
#endif

#ifdef DEBUG
  for(int n = 0; n < 10; n++){
#else
  for(int n = 0; n < 15; n++){
#endif
      A += B[n];
}

[jpt13653903]

Here's another problem...

At the moment I have the concurrent and sequential statements split, which makes the grammar much easier to implement because it gets rid of a large number of ambiguities.

If I support the suggested conditional analysis tree, I have to be able to support both of those inside the body of the analysis, which brings back the ambiguities and complicates the parser.

I can of course split them into "concurrent" and "sequential" tool directives, but I don't think it worth the complication.

Unless you can think of another way of doing it...?

[superzanti]

You are correct. The syntax for preprocessor things is not well defined...

c handles each of these as:

#ifdef DEBUG
  printf("This is debugging...");
#else
  printf("This is production...");
#endif

Producing:

(preproc_ifdef
  name: (identifier)
  (expression_statement
    (call_expression
      function: (identifier)
      arguments: (argument_list
        (string_literal
          (string_content)))))
  alternative: (preproc_else
    (expression_statement
      (call_expression
        function: (identifier)
        arguments: (argument_list
          (string_literal
            (string_content)))))))

and

#ifdef DEBUG
  for(int n = 0; n < 10; n++){
#else
  for(int n = 0; n < 15; n++){
#endif
      A += B[n];
}

Resulting in a parsed tree that acts as if no preprocessor arguments exist, so it doesn't close the for loop until another bracket is shown. Ultimately resulting in an error.

It's kind of a bummer, but is also being documented here: https://github.com/tree-sitter/tree-sitter-c/issues/108

I think the best way to handle it may just to be to ignore preprocessor blocks (treat them like you would a comment) - then just leave it ot the user to create a valid tree if they need it.

So the user may need to make the second example become:

#ifdef DEBUG
  for(int n = 0; n < 10; n++){
      A += B[n];
  }
#else
  for(int n = 0; n < 15; n++){
      A += B[n];
  }
#endif

It sucks, but that's about what the c tresitter is doing now.

[jpt13653903]

I think the best way to handle it may just to be to ignore preprocessor blocks (treat them like you would a comment) - then just leave it ot the user to create a valid tree if they need it.

I don't believe that there is a workable solution here... This is one of the reasons why I use indentation-based folding instead of syntax-based folding.

VHDL makes it even worse with the insanely ambiguous grammar. I really don't see myself implementing the tool-directives as anything other than fancy comments (like they are at the moment).

Your example won't work either, because the tree nodes don't nest correctly for folding to work:

process(all) begin
  `if DEBUG = "Yes" then
    for n in 1 to 10 loop
      A := A + B(n);
    end loop;
  `else
    for n in 1 to 15 loop
      A := A + B(n);
    end loop;
  `end if
end process;

parses to

(design_file
  (design_unit
    (process_statement
      (PROCESS)
      (sensitivity_specification
        (ALL))
      (sequential_block
        (BEGIN)
        (if_conditional_analysis
          (IF)
          (conditional_analysis_expression
            ...
          (THEN))
        (loop_statement
          (for_loop
            ...
          (loop_body
            ...
          (end_loop
            ...
        (else_conditional_analysis
          (ELSE))
        (loop_statement
          (for_loop
            ...
          (loop_body
            ...
          (end_loop
            ...
      (end_conditional_analysis
        (END)
        (IF))
      (end_process
        (END)
        (PROCESS)))))

NOTE: The above parse only works once the bug-fix in the bugfix/conditional_analysis branch has been applied. I based it on top of your feature/comment_content branch, so I'll merge it in after I've merged the comments one.

[superzanti]

Right, that's what I was saying. Just treat them as fancy comments. I think that's best.

Thank you!