param.ccl code creators creates incorrect C code if parameter description is missing

[Roland Haas]

This param.ccl file

# Parameter definitions for thorn paramtest

INT MyParam
{
  -1 :: "some value"
  0:* :: "some other values"
} 4

which is missing the description for the parameter MyParam is accepted by CST but produces a C parameter bindings file that fails to compile with an error:

COMPILING configs/paramtest/bindings/Parameters/paramtest_Parameters.c
/data/rhaas/postdoc/gr/cactus/ET_trunk/configs/paramtest/build/CactusBindings/Parameters/paramtest_Parameters.c: In function ‘CCTKi_BindingsCreateparamtestParameters’:
/data/rhaas/postdoc/gr/cactus/ET_trunk/configs/paramtest/build/CactusBindings/Parameters/paramtest_Parameters.c:45:25: error: expected expression before ‘,’ token
   45 |                         ,
      |                         ^

I am attaching a tarfile of my test thorn.

[Zach Etienne]

which is missing the description for the parameter max_m_mode is accepted by CST but produces a C parameter bindings file that fails to compile with an error:

You mean MyParam, not max_m_mode, right?

‌

[Roland Haas]

@Zach Etienne yes. I had originally drafted the bug report based on my for-real thorn that encountered it but then wanted to provide a simpler reproducer.

[Steven R. Brandt]

It’s easily fixed by this:

diff --git a/lib/sbin/parameter_parser.pl b/lib/sbin/parameter_parser.pl
index 5e7eeeeb..29cf4d57 100644
--- a/lib/sbin/parameter_parser.pl
+++ b/lib/sbin/parameter_parser.pl
@@ -196,6 +196,7 @@ sub parse_param_ccl
         }
         my $desc = $guts->group(1,"description")->substring();
         $desc =~ s/\\\n//g;
+        $desc = '""' if($desc eq "");
         if($gr->is("keywordpar")) {
           $parameter_db1{"\U$thorn $as_name\E type"}="KEYWORD";
           my $item_count = 1;

‌

[Roland Haas]

Doesn’t that make the description optional by defaulting to a an empty string if no description is given?

The documentation, which defines what is an acceptable param.ccl file, found for example here http://einsteintoolkit.org/usersguide/UsersGuide.html#x1-186000D2.3.2 has

[EXTENDS|USES] <parameter type> <parameter name>[[<len>]] "<parameter description>"
[AS <alias>] [STEERABLE=<NEVER|ALWAYS|RECOVER>]
[ACCUMULATOR=<expression>] [ACCUMULATOR-BASE=<parameter name>]
{
  <parameter values>
} <default value>

which has the <parameter description> as a requirement element. So based on that, not having a description should trigger an CST time error reporting that a description must be provided for this parameter.

[Steven R. Brandt]

Well, you originally complained about it not compiling. I think this may be what you want.

index 5e7eeeeb..67ff0361 100644
--- a/lib/sbin/parameter_parser.pl
+++ b/lib/sbin/parameter_parser.pl
@@ -196,6 +196,11 @@ sub parse_param_ccl
         }
         my $desc = $guts->group(1,"description")->substring();
         $desc =~ s/\\\n//g;
+        if($desc eq "" and $uses_or_extends eq "") {
+            &CST_error(0, "Missing description for parameter $name" .
+                          "param.ccl for thorn $thorn",
+                       '', $guts->linenum(), $ccl_file);
+        }
         if($gr->is("keywordpar")) {
           $parameter_db1{"\U$thorn $as_name\E type"}="KEYWORD";
           my $item_count = 1;

‌

[Roland Haas]

Technically I reported that CST does not complain and instead generates code that does not compile.

Thank you. This patch seems to work for me.

I had a look what the corresponding error message in the pre-piraha parser looked like (around line 419 of parameters_parser.pl):

&CST_error(0, "Missing description for parameter $variable in " .
              "param.ccl for thorn $thorn.",
           "The first line of each parameter definition must have " .
              "the syntax <TYPE> <NAME> <\"DESCRIPTION\">",
           __LINE__, __FILE__);                                                             

and would like to suggest to use that text to address this regression (mostly for the hint, the error message text is almost identical). However I would also suggest retaining $guts->linenum(), $ccl_file that the patch has instead of reverting to __LINE__, __FILE__ which, as far as I can tell would give line number and file name of the Perl script which is (much) less useful than the file name and line number of the param.ccl file.

Please apply.

[Steven R. Brandt]

Fixed in 77fbe89c062589f032dfac180b987f277e903508 of https://bitbucket.org/cactuscode/cactus.git

[Steven R. Brandt]

Fixed