ADIOS2 Access Mode: Fix Dangling Ref

[ax3l]

Fix GCC 13.3 warning:

/home/runner/work/openPMD-api/openPMD-api/src/IO/ADIOS/ADIOS2IOHandler.cpp: In member function 'adios2::Mode openPMD::ADIOS2IOHandlerImpl::adios2AccessMode(const std::string&)':
/home/runner/work/openPMD-api/openPMD-api/src/IO/ADIOS/ADIOS2IOHandler.cpp:1531:21: warning: possibly dangling reference to a temporary [-Wdangling-reference]
 1531 |         auto const &access_mode_json = m_config["engine"]["access_mode"].json();
      |                     ^~~~~~~~~~~~~~~~
/home/runner/work/openPMD-api/openPMD-api/src/IO/ADIOS/ADIOS2IOHandler.cpp:1531:78: note: the temporary was destroyed at the end of the full expression 'openPMD::json::TracingJSON::operator[](Key&&) [with Key = const char (&)[12]]("access_mode").openPMD::json::TracingJSON::json()'
 1531 |         auto const &access_mode_json = m_config["engine"]["access_mode"].json();
      |                                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~

First seen in #1672

[ax3l]

Maybe because of the extra [][] that can be dangling? Or is this a false positive in GCC 13.3 -Wdangling-reference?

The warning states:

the temporary was destroyed at the end of the full expression 'openPMD::json::TracingJSON::operator with Key = const char (&)[12].openPMD::json::TracingJSON::json()'

It looks like TracingJSON is indeed returned by value from operator[], so this might be a bug indeed...?

[ax3l]

I see similar patterns in:

src/IO/HDF5/ParallelHDF5IOHandler.cpp:234:                    auto const &val = vfd_json_config[key].json();
src/IO/HDF5/ParallelHDF5IOHandler.cpp:253:                    auto const &val = vfd_json_config[key].json();
src/IO/ADIOS/ADIOS2IOHandler.cpp:327:    nlohmann::json const &operators = _operators.json();

but no warnings about them. Maybe worth to double check.

[franzpoeschel]

This is a known false positive in gcc13 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109642
TracingJSON::operator[] creates new handles for the same data, i.e. within the handle, the actual data is behind a pointer, but the handle additionally contains position information on where we are within the JSON dataset.

    class TracingJSON
    {
        // ...
    private:
        /**
         * @brief The JSON object with which this class has been initialized.
         *        Shared pointer shared between all instances returned by
         *        operator[]() in order to avoid use-after-free situations.
         *
         */
        std::shared_ptr<nlohmann::json> m_originalJSON;

        // ...        

        /**
         * @brief The sub-expression within m_originalJSON corresponding with
         *        the current instance.
         *
         */
        nlohmann::json *m_positionInOriginal;
       
        // ...

    };

gcc13 only sees that operator[] returns by value and does not know that the class is reference-like.
We could either solve this by doing a (useless) copy, or by temporarily deactivating the warning (this class is used only internally and not included in public headers).

[franzpoeschel]

Found an easier solution, see last commit

[ax3l]

Thanks! Oh, allright!

The original looks a bit better though (and has probably faster compile time) :) Do we want to just keep it as is? :)

[franzpoeschel]

I would still like to solve this at an API level, so we have an API that gcc actually understands. If we keep it as it is, then we will get bug reports and have to tell people it's a compiler bug. I changed it into a solution now that does not need templates.

[ax3l]

Good point