rems-project / sail

Sail architecture definition language
Other
617 stars 112 forks source link

sail -fmt is confused about trailing comments #239

Open arichardson opened 1 year ago

arichardson commented 1 year ago

From formatting the sail-cheri-riscv model:

 /* Cheri PCC helpers */
-
 function min_instruction_bytes () -> CapAddrInt = {
-  if   (~ (sys_enable_writable_misa ())) & (~ (sys_enable_rvc ()))
-  then 4  /* RVC is hardwired to be disabled */
-  else 2  /* RVC is active or it could become active */
+    if ~(sys_enable_writable_misa()) & ~(sys_enable_rvc()) then {
+        4
+    } else {
+        /* RVC is hardwired to be disabled */
+        2
+    }/* RVC is active or it could become active */
 }

Would it be possible to keep trailing comments associated with the same AST node even when inserting braces?

Alasdair commented 1 year ago

Yes, the problem here is we don't have locations for the if, then and else tokens in the syntax tree so it's hard to place the comments precisely. I need to add those and probably for some other keywords and delimiters.

Alasdair commented 1 year ago

The second comment /* RVC is active or it could become active */ is going to be hard to place correctly for an if-statement without braces, unless we try to do something clever using the indentation level of the comment, because it's hard to reliably distinguish

if   (~ (sys_enable_writable_misa ())) & (~ (sys_enable_rvc ()))
then 4  /* RVC is hardwired to be disabled */
else 2
/* RVC is active or it could become active */

Peter suggested some syntax for forcing comments to be attached in a specific direction, something like (making syntax up on the spot): /*< comment */ or /*> comment */

arichardson commented 1 year ago

I'd be fine to require braces for if statements with comments. In that case the comment would remain within the braces right? It's just because there weren't any initially? Doxygen uses /**< to place documentation after members: https://www.doxygen.nl/manual/docblocks.html (Putting documentation after members)

Alasdair commented 1 year ago

If you have braces it should work fine:

function foo() = {
    if cond
    then 2 /* comment 1 */
    else { 3 /* comment 2 */ }
}

will become:

function foo () = {
    if cond then {
        2/* comment 1 */
    } else {
        3/* comment 2 */
    }
}

The awkward lack of spaces between the numbers and the comments is also a known problem right now. I'll need some kind of pass that re-lexes the formatted source and adjusts the spaces in front of comments to fix it properly.