#200 C++ context name improperly generated

[Dan Ingold]

Sometime between v6.1 and v6.4, SMC began generating the context class name differently (and incorrectly) for C++ targets. The C++ manual section says the context name will be <appclass>Context</appclass>. That is, for %class Hep, the generated name should be class HepContext. This was the previous behavior.

Now SMC instead generates the context class name from the <smc_file_name_stem></smc_file_name_stem>. That is, if the input file is named dist_hep.sm, the context name will be class dist_hepContext. At least according to the manual, and certainly with respect to historical behavior, this is incorrect. Only the generated output file names should reflect the input filename, not the classes within those files.

Consistent with this, if the input filename is changed to Hep.sm, the context class will be named class HepContext. If a lowercase name is used, hep.sm, the generated context is class hepContext. (At least under Linux, where filenames are case-sensitive. I don't know the behavior under Windows. In any case, the class name should probably not reflect file system naming peculiarities.)

[Dan Ingold]

I think the problem is due to an assignment in the SmcFSM ctor, where _fsmClassName is initialized to name+"Context". Previously (in v1.2 of SmcFSM.java) _fsmClassName was initialized to an empty string. Note that SmcFSM also contains a method setFsmClassName.

This initialization interacts with code in createMap() of SmcParser (ll. 546-550, comments elided), which tests for an empty _fsmClassName that can (now) never occur:

if ( _fsm.getFsmClassName() == "" )
{
    _fsm.setFsmClassName( _fsm.getContext()+"Context" );
}

Since this code is in a block where we know _mapInProgress == null, it seems to me the above test for empty string can be removed. That is, just go ahead and call _fsm.setFsmClassName(), which will reset the (inappropriate) initial value.

[Dan Ingold]

The following patch appears to fix the problem for C++. I didn't test it on other languages. (Sorry it's in git format, but that's what I use to control the source on this end.)

Comments from the maintainers on whether or not this change is appropriate would be welcomed. If the solution is deeper than this, I'd be happy to look into it further.

diff --git a/net/sf/smc/model/SmcFSM.java b/net/sf/smc/model/SmcFSM.java
index 427c602..c07edac 100644
--- a/net/sf/smc/model/SmcFSM.java
+++ b/net/sf/smc/model/SmcFSM.java
@@ -435,7 +435,6 @@ public final class SmcFSM
     public void setFsmClassName(String fsmName)
     {
         _fsmClassName = fsmName;

-        _targetFileName = fsmName;

         return;
     } // end of setFsmClassName(String)
diff --git a/net/sf/smc/parser/SmcParser.java b/net/sf/smc/parser/SmcParser.java
index 2eb143f..ec4840a 100644
--- a/net/sf/smc/parser/SmcParser.java
+++ b/net/sf/smc/parser/SmcParser.java
@@ -542,12 +542,9 @@ public final class SmcParser
         }
         else
         {

-               // check FSM class name
-               if ( _fsm.getFsmClassName() == "" )
-               {
-                       // set default FSM class name
-                       _fsm.setFsmClassName( _fsm.getContext()+"Context" );
-               }
+           // set default FSM class name
+           _fsm.setFsmClassName( _fsm.getContext()+"Context" );
+
             if (_parserFSM.getDebugFlag() == true)
             {
                 PrintStream os = _parserFSM.getDebugStream();

[Charles Rapp]

Kudos to Dan for his correction. Testing and manual examination of the generated code shows this is the correct solution. Many thanks.

This fix will be incorporated into the next release.

[Charles Rapp]

Not so fast. This change breaks the %fsmclass directive. Need to step back and re-think.

[Charles Rapp]

The solution is to expand the SmcParser.createMap() changes to include removing "_fsm.setFsmClassName(...)" as well. The FSM class name is correctly initialized in SmcFSM constructor and modified in setFsmClassName() when the %fsmclass directive is used.

This change means that the C++ class name is correctly set to its default and %fsmclass still works.

[Dan Ingold]

Sorry I missed this other problem my fix caused, Charles. Can you post the patch, or should I just download the daily build? Thanks for producing SMC, and keeping the project alive!

[Charles Rapp]

Because _fsmClassName is initialized to a non-null, non-empty value, there is no need for any code in SmcParser.createMap to modify the FSM class name. So the patch removes more lines than yours:

diff --git a/net/sf/smc/model/SmcFSM.java b/net/sf/smc/model/SmcFSM.java
index 427c602..c07edac 100644
--- a/net/sf/smc/model/SmcFSM.java
+++ b/net/sf/smc/model/SmcFSM.java
@@ -435,7 +435,6 @@ public final class SmcFSM
     public void setFsmClassName(String fsmName)
     {
         _fsmClassName = fsmName;

-        _targetFileName = fsmName;

         return;
     } // end of setFsmClassName(String)
diff --git a/net/sf/smc/parser/SmcParser.java b/net/sf/smc/parser/SmcParser.java
index 2eb143f..ec4840a 100644
--- a/net/sf/smc/parser/SmcParser.java
+++ b/net/sf/smc/parser/SmcParser.java
@@ -542,12 +542,9 @@ public final class SmcParser
         }
         else
         {

-               // check FSM class name
-               if ( _fsm.getFsmClassName() == "" )
-               {
-                       // set default FSM class name
-                       _fsm.setFsmClassName( _fsm.getContext()+"Context" );
-               }
-           // set default FSM class name
-           _fsm.setFsmClassName( _fsm.getContext()+"Context" );
-
             if (_parserFSM.getDebugFlag() == true)
             {
                 PrintStream os = _parserFSM.getDebugStream();

[Charles Rapp]

Corrected in release 6.5.0.