Fixed MSVC compilation

[sjoelund]

So... Your ModelicaUtilitites.h does not contain attribute((noreturn)) or __declspec(noreturn)? It really should. I get no warnings from clang, clang++, or ccc-analyzer since mine has the attribute.

[tbeu]

Correct, I use the default ModelicaUtilitites.h from MSL trunk.

[sjoelund]

Fixed using 766fc35 instead (added abort() instead of default values to silently ignore the fact that ModelicaError returned)

[tbeu]

This means that any ModelicaError will terminate the simulator. Think of msgpack-modelica been compiled as FMU.

[sjoelund]

No. ModelicaError does not return. "This function never returns to the calling function, but handles the error similarly to an assert in the Modelica code." Think of try/throw/catch in C++ or longjmp in C. You throw whenever ModelicaError is called and so the other part of the code can never be reached. For FMI I would assume any call made is guarded with something like:

void ModelicaError(const char *str)
{
  // print to logger or something
  longjmp(jmpbuf,1);
}

fmiStatus doStep(...)
{
  if (0 == setjmp(jmpbuf)) {
    // try stuff
  } else {
    // An assertion or ModelicaError was triggered
    return fmiDiscard;
  }
}

[tbeu]

Yeah, you are right. Thanks for clarification.

[sjoelund]

And just as an example of why it must never return. libModelicaExternalC contains code like the following, which assumes ModelicaFormatError will never return. If the string allocation returns NULL and ModelicaFormatError did return, we would get a segmentation fault due to writing to NULL. So...

              pName = ModelicaAllocateStringWithErrorReturn(strlen(pinfo->d_name));
              if ( pName == NULL ) {
                errnoTemp = errno;
                closedir(pdir);
                if ( errnoTemp == 0 ) {
                   ModelicaFormatError("Not possible to get file names of \"%s\":\n"
                                 "Not enough storage", directory);
                } else {
                   ModelicaFormatError("Not possible to get file names of \"%s\":\n%s",
                                 directory, strerror(errnoTemp));
                }
              }
              strcpy(pName, pinfo->d_name);

Anyway, the added abort() should make msvc not give compilation errors I hope. Maybe I should update the header on modelica.org trunk though...

[tbeu]

Ok, can check on Monday if MSVC still complains. I am pretty sure the call of abort() is sufficient.

I never observed this issue on ModelicaExternalC from modelica.org. I guess their functions are all complete, i.e. their are no missing calls of return in conditional branches.

[sjoelund]

Well... I have to use https://openmodelica.org/svn/OpenModelica/trunk/SimulationRuntime/c/ModelicaUtilities.h because otherwise static analysis (clang scan-build; I guess also cppcheck) complains about uses of NULL pointers, garbage data, etc even though ModelicaError guarded against them.

The ModelicaExternalC functions are complete so compilation works since it is a valid C program even without noreturn attributes. It's just a bad library if ModelicaError is allowed to return.

[tbeu]

It does not compile on MSVC with abort();

c:\Projects\msgpack-modelica\MessagePack\Resources\C-Sources>cl -nologo -c /MD /TP /I "c:\Projects\msgpack-c\src" /I "c:\Projects\msgpack-c" msgpack-modelica.c
msgpack-modelica.c
c:\Projects\msgpack-modelica\messagepack\resources\include\msgpack-modelica.h(270) : warning C4715: "msgpack_modelica_unpack_int": Nicht alle Steuerelementpfade geben einen Wert zurück.
c:\Projects\msgpack-modelica\messagepack\resources\include\msgpack-modelica.h(289) : warning C4715: "msgpack_modelica_unpack_string": Nicht alle Steuerelementpfade geben einen Wert zurück.
c:\Projects\msgpack-modelica\messagepack\resources\include\msgpack-modelica.h(366) : error C4716: 'msgpack_modelica_stream_get': Muss einen Wert zurückgeben

Please see my proposed changes.

[sjoelund]

You found a compiler bug in MSVC. http://stackoverflow.com/questions/3569643/abort-is-not-declspecnoreturn-in-vs2010 ... Easiest way to fix it is to update ModelicaUtilities.h. Then abort is not needed either :) I could update the OpenModelica ModelicaUtilities.h to also add MSVC declspec and get it into modelica.org trunk.

There are also hints at http://stackoverflow.com/questions/11386078/visual-c-error-function-must-return-a-value on how to anyway compile with the old MSVC by ignoring the warnings.

[tbeu]

I would definitely appreciate if __declspec(noreturn) will be added to the ModelicaError functions declarations of official MSL.

[sjoelund]

https://trac.modelica.org/Modelica/ticket/1197#comment:11 - I will assume people are happy with it and do not revert it. Aren't you subscribed to that ticket as well though?

[tbeu]

Thanks. Got it.

[tbeu]

After fixing ModelicaUtilities.h you might want to remove abort() again from code.

[sjoelund]

26acb99 removes them