adding a utility function getTypeName()

[GitPaean]

I use it to output the Type name defined in TypeTag.

[GitPaean]

jenkins build this please

[bska]

Is this mainly for debugging purposes, or are end users supposed to see these strings?

I use it to output the Type name defined in TypeTag.

Fair enough, but please be aware that typeid(T).name() is required to give the wrong result in certain situations–e.g., it will report a reference when the type is a pointer (or the other way around, I don't quite recall the exact details) and it will not capture const vs. mutable and so on. If this is mainly for debugging, then I think you'd get answers that are at least as good by using the whatis GDB command.

[GitPaean]

then I think you'd get answers that are at least as good by using the whatis GDB command.

I appreciate it. It depends people's working habit, to me, with a dedicated function, we can output more to investigate the classes involved potentially more systematic way instead of checking one by one.

Is this mainly for debugging purposes, or are end users supposed to see these strings?

It is fair to say that is mostly for developers. I do not think end users should understand the class types (not sure whether they should see), similar to function below in opm/material/fluidsystems/BaseFluidSystem.hpp

    static std::string not_implemented(const std::string_view method)
    {
        return "Not implemented: The fluid system '" +
                getTypeName<Implementation>() +
               "'  does not provide a " + method.data() + "() method!";
    }
};

We might be able to have some useful output to help understand issues that people run into.

[GitPaean]

I appreciate if this PR can go in. I have it in many of my development branches including master branches for analyzing the Type setup.

But I do realize the above is not a strong argument comparing with what is correct for the code policy (although I am not sure what it violates). So if you are against to get this in, please also voice out so I can know. I will deal with the hassle when creating pull requests.

[atgeirr]

I think this should go in, it is simpler to use than demangle(). However, as it is a very small utility that builds on demangle(), what about adding it to Demangle.hpp/.cpp instead of adding a new header and implementation file?

Also, maybe use a more precise function name to avoid giving the impression that you get full type (constness etc. as pointed out above), perhaps basicTypeName() or something like that? @bska is there a precise (yet short...) name for this?

[bska]

I'm still a little reluctant to adding this at all. In the use case outlined earlier, the goal is to permanently call this function in "many" locations instead of adding a few calls whilst developing/debugging a feature and removing the calls once the work is done. I fear that will ultimately lead to users seeing the type strings in diagnostic messages and getting very confused. We already have our fair share of developer-focused diagnostic messages–e.g. OPM/opm-simulators#3806–which are very unfriendly to users since they can't do anything about them and don't understand what they mean.

[GitPaean]

I think this should go in, it is simpler to use than demangle(). However, as it is a very small utility that builds on demangle(), what about adding it to Demangle.hpp/.cpp instead of adding a new header and implementation file?

Thanks for the response. If there is no other usage for the function std::string demangle(const char* mangled_symbol) than getTypeName (function name can be updated), I can merge the two headers files, while the file name likely not Demangle anymore, it will more related to the function name that will be used.

And also, please suggest the preferred function name and I will update the PR.

[GitPaean]

I'm still a little reluctant to adding this at all. In the use case outlined earlier, the goal is to permanently call this function in "many" locations instead of adding a few calls whilst developing/debugging a feature and removing the calls once the work is done. I fear that will ultimately lead to users seeing the type strings in diagnostic messages and getting very confused. We already have our fair share of developer-focused diagnostic messages–e.g. https://github.com/OPM/opm-simulators/issues/3806–which are very unfriendly to users since they can't do anything about them and don't understand what they mean.

Sure. Are you suggesting we should also remove Demangle.hpp also in the first place? This PR does not introduce more implication than Demangle.hpp in my opinion.

[bska]

Are you suggesting we should also remove Demangle.hpp also in the first place? This PR does not introduce more implication than Demangle.hpp in my opinion.

Let me be clear: I'm not opposed to having the facility, although it might make sense to having it in a separate debugging library. I am opposed to adding permanent calls to this function in locations that will ultimately lead to users seeing the strings.

[atgeirr]

I am opposed to adding permanent calls to this function in locations that will ultimately lead to users seeing the strings.

I agree with this, which will ultimately require developer discipline (remembering to remove temporary logging etc.), and I do not think this increases the burden, other than indirectly by being useful while debugging.

It seems that with this caveat for future development, we agree that this (in some variant) should be merged?

[GitPaean]

jenkins build this please

[GitPaean]

jenkins build this please

[GitPaean]

jenkins build this please

[GitPaean]

jenkins build this please

[bska]

jenkins build this please

[GitPaean]

jenkins build this please