[General] Fixing sparse bugs

[roigcarlo]

📝 Description
Changing some code to allow compilation in the release environment. Most of the code is correct but GCC still have some bugs that prevent using it.

@sunethwarna These are the changes we need for the release.

[avdg81]

Hi @roigcarlo,

I have to admit that I don't see why it would be required to have this include here. Class Serializer is forward-declared and the only usages in this header file are references to this class (which is sufficient information for the compiler at this point). The implementation file (tension_cutoff.cpp) does have the include that you've added here now, which makes sense, since there we actually do something with the serializer objects.

The other thing that confuses me is that all other builds on GitHub pass with the status quo (i.e. without the additional include). So I'm wondering whether there is anything special about the build configuration that you're trying? Or perhaps there's something else that I overlook? If you have any ideas, please let me know. Thanks.

[roigcarlo]

Hi @avdg81, that drove me crazy a little bit as well. This is what I think is happening:

In the case of the CI is never a problem because it uses unitary builds which in practice is like having a PCH on windows, so the serializer.h include may come from the stack of headers included in the other sources (unless tension_cutoff.cpp is the first source bundled)

The second problem is more subtle. I am not 100% sure about if this is the case, but is the only think I came up with after searching for a while: I assume that the intention is for the friend class Serializer to also act as implicit forward declaration, but that declaration exists inside a class inside the namespace Kratos, so while it is true that Serializer is implicitly declared as Kratos::Serializer it is not as ::Serializer so it is not in the global lookup namespace. Seems that MSVC and some versions of gcc/clang simply do not enforce this rule and allow the declaration to be seen so that would explain why it does not normally fails. As a matter of fact, this specific header compiled well for the windows release, it only failed in GCC 14.

P.s.

Every name first declared in a namespace is a member of that namespace. If a friend declaration in a non-local class first declares a class, function, class template, or function template, the friend is a member of the innermost enclosing namespace

This is the rule that should fr followed by the compiler but is being ignored.

[avdg81]

Hi @roigcarlo,
Thanks a lot for investigating my question. Perhaps then the better way to solve it is to have a forward declaration in the Kratos namespace and use a friend declaration like this: friend Serializer; (or should it be friend Kratos::Serializer;?). My point is: if we can use a forward declaration (and I think we can), then I would prefer that over an extra include. May I ask you to try this for me? Thanks. If it works, I'm happy to make this change myself using a new PR, since we have similar constructs in other classes, too.

[roigcarlo]

@avdg81 Np,

namespace Kratos
{

class Serializer;

namespace Geo
{
class PrincipalStresses;
class SigmaTau;
} // namespace Geo

This pretty much solves the problem as well. 👍