Bug 254757
| Summary: | SVGPathSegValue::clone<>() rename to SVGPathSegValue::cloneInternal<>() | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> |
| Component: | SVG | Assignee: | Said Abou-Hallawa <sabouhallawa> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | sabouhallawa, webkit-bug-importer, zimmermann |
| Priority: | P2 | Keywords: | InRadar |
| Version: | WebKit Nightly Build | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=254758 | ||
Michael Catanzaro
SVGPathSeg has a virtual function SVGPathSeg::clone which you would expect to clone any SVGPathSeg, retaining its derived type. However, the virtual clone gets hidden by SVGPathSegValue<PathSegType>::clone because templated functions are never virtual. I think this means a clone operation on a SVGPathSeg pointer will work correctly if the SVGPathSeg is a SVGPathSegValue. This problem was found by GCC 13's -Woverloaded-virtual warning.
I don't know what to do about this. For now I'll just suppress the warning.
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Michael Catanzaro
(In reply to Michael Catanzaro from comment #0)
> I think this means a clone operation on a
> SVGPathSeg pointer will work correctly if the SVGPathSeg is a
> SVGPathSegValue.
I meant: will not work correctly. The SVGPathSegValue<PathSegType>::clone would only get called via a pointer to an SVGPathSegValue<PathSegType>. With a pointer to an SVGPathSeg, I think the program should abort with a runtime error due to unimplemented virtual function?
Radar WebKit Bug Importer
<rdar://problem/107719787>
Said Abou-Hallawa
I think this might be a bug or limitation in GCC 13 complier. The clang complier considers the template function SVGPathSegValue::clone<>() different from the virtual function SVGPathSeg::clone() so it does not generate an error saying the template function hides the virtual implementations.
Open LayoutTests/svg/dom/SVGPathSegList-insert-from-animating-animPathSegList.svg in the debugger and set a breakpoint in SVGPathSegMovetoAbs::clone(). In Xcode the debugger breaks at the function.
SVGPathSegValue::clone<>() should be renamed to SVGPathSegValue::cloneInternal<>(). So the code can be complied on GCC 13 without the need for -Woverloaded-virtual.
Said Abou-Hallawa
Pull request: https://github.com/WebKit/WebKit/pull/12476
EWS
Committed 262690@main (9fc3aff2f387): <https://commits.webkit.org/262690@main>
Reviewed commits have been landed. Closing PR #12476 and removing active labels.