zeroc-ice / ice

All-in-one solution for creating networked applications with RPC, pub/sub, server deployment, and more.
https://zeroc.com
GNU General Public License v2.0
2k stars 592 forks source link

Simplify and Improve 'serialVersionUID' Code-Gen #2307

Closed InsertCreativityHere closed 2 weeks ago

InsertCreativityHere commented 2 weeks ago

This PR simplifies the code-gen logic for adding serialVersionUID to Slice classes, exceptions, and structs. Currently, we write out nearly identical logic 3 separate times, and have 3 separate functions for computing UIDs (1 for each type).

Since the logic is nearly identical, I moved it into a single central helper function.

The only changes in logic are:

InsertCreativityHere commented 2 weeks ago

Upon self-reviewing this PR actually changes one other piece of behavior: it accidentally fixes a bug with our current logic. Looking at the old code:

            std::int64_t v = 0;
            serialVersionUID = serialVersionUID.substr(pos);
            if (serialVersionUID != "0")
            {
                try
                {
                    v = std::stoll(serialVersionUID, nullptr, 0);
                }
                catch (const std::exception&)
                {
                    ostringstream os;
                    os << "ignoring invalid serialVersionUID for exception `" << p->scoped()
                       << "'; generating default value";
                    dc->warning(InvalidMetaData, "", "", os.str());
                    out << computeSerialVersionUUID(p);
                }
            }
            out << v;

If the user types a nonsense UID like java:serialVersionUID:dogs!. The catch block will emit a default-computed UID: out << computeSerialVersionUUID(p) And then we'll randomly write an extra 0 at the end (since v was set to the 0 at the top: std::int64_t v = 0;): out << v;

This could very easily generate uncompilable code, since this effectively multiplies the value by 10, which could push it past the limit for long::MAX... Pretty niche bug though.