param.ccl code creators creates incorrect C code if parameter description is missing
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.
Comments (9)
-
-
reporter - edited description
-
reporter @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.
-
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;
-
reporter 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. -
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;
-
reporter 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 theparam.ccl
file.Please apply.
-
-
- changed status to resolved
Fixed
- Log in to comment
You mean
MyParam
, notmax_m_mode
, right?