Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: correct redefinition which is invalid C++ #36096

Merged
merged 3 commits into from
Feb 8, 2023

Conversation

brjsp
Copy link
Contributor

@brjsp brjsp commented Oct 20, 2022

See https://stackoverflow.com/a/15538759 for an explanation of class scope.
GCC gives an error when compiling this code: https://godbolt.org/z/sYhc3cMjE

Notes: none

@brjsp brjsp requested a review from a team as a code owner October 20, 2022 19:49
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Oct 20, 2022
Copy link
Member

@nornagon nornagon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be happy to take a patch that only updated our changes to Chromium, but this also changes some existing code in Chromium.

It's worth noting that neither Chromium nor Electron supports building with GCC.

I'm requesting changes here, but if you get the GetPageAllocator definition fixed upstream (see here for details about contributing to Chromium), I'd be happy to also update our patch to match.

@brjsp brjsp requested a review from nornagon October 21, 2022 06:38
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Oct 25, 2022
@brjsp brjsp force-pushed the PageAllocator-fpermissive branch 2 times, most recently from cf54af7 to 17a3a87 Compare December 2, 2022 20:07
@brjsp brjsp requested review from zcbenz and removed request for nornagon December 2, 2022 20:08
@nornagon
Copy link
Member

The patch seems to be broken now :/ but it looks better!

@brjsp brjsp requested review from nornagon and removed request for zcbenz December 15, 2022 19:51
@brjsp
Copy link
Contributor Author

brjsp commented Dec 15, 2022

fixed and rebased patch

@brjsp brjsp force-pushed the PageAllocator-fpermissive branch 2 times, most recently from 2434932 to 2ce9d80 Compare December 16, 2022 17:22
@nornagon
Copy link
Member

Looks like this is FTBFS now:

/home/builduser/project/build-tools/third_party/goma/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF obj/electron/electron_lib/javascript_environment.o.d -DV8_DEPRECATION_WARNINGS -DGDK_DISABLE_DEPRECATION_WARNINGS -DGLIB_DISABLE_DEPRECATION_WARNINGS -DALLOW_RUNTIME_CONFIGURABLE_KEY_STORAGE -DDCHECK_ALWAYS_ON=1 -DUSE_UDEV -DUSE_AURA=1 -DUSE_GLIB=1 -DUSE_OZONE=1 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_GNU_SOURCE -DCR_CLANG_REVISION=\"llvmorg-16-init-10736-ged9638c4-1\" -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS -DCR_LIBCXX_REVISION=325733d7de6154bd131fb3ebb2e267aa2d284834 -D_LIBCPP_ENABLE_ASSERTIONS_DEFAULT=1 -D_LIBCPP_AVAILABILITY_CUSTOM_VERBOSE_ABORT_PROVIDED=1 -DCR_SYSROOT_KEY=20220331T153654Z-0 -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DV8_USE_EXTERNAL_STARTUP_DATA -DNODE_WANT_INTERNALS=1 -DIS_MAS_BUILD\(\)=0 -DELECTRON_PRODUCT_NAME=\"Electron\" -DELECTRON_PROJECT_NAME=\"electron\" -DGLIB_VERSION_MAX_ALLOWED=GLIB_VERSION_2_56 -DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_56 -DLIBYUV_DISABLE_NEON -DWEBP_EXTERN=extern -DGL_GLEXT_PROTOTYPES -DUSE_GLX -DUSE_EGL -DVK_USE_PLATFORM_XCB_KHR -DVK_USE_PLATFORM_WAYLAND_KHR -DON_FOCUS_PING_ENABLED -DTOOLKIT_VIEWS=1 -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DUSE_CHROMIUM_ICU=1 -DU_ENABLE_TRACING=1 -DU_ENABLE_RESOURCE_TRACING=0 -DU_STATIC_IMPLEMENTATION -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DSK_CODEC_DECODES_PNG -DSK_CODEC_DECODES_WEBP -DSK_ENCODE_PNG -DSK_ENCODE_WEBP -DSK_ENABLE_SKSL -DSK_UNTIL_CRBUG_1187654_IS_FIXED -DSK_USER_CONFIG_HEADER=\"../../skia/config/SkUserConfig.h\" -DSK_WIN_FONTMGR_NO_SIMULATIONS -DSK_GL -DSK_CODEC_DECODES_JPEG -DSK_ENCODE_JPEG -DSK_HAS_WUFFS_LIBRARY -DSK_VULKAN=1 -DSK_SUPPORT_GPU=1 -DSK_GPU_WORKAROUNDS_HEADER=\"gpu/config/gpu_driver_bug_workaround_autogen.h\" -DGOOGLE_PROTOBUF_NO_RTTI -DGOOGLE_PROTOBUF_NO_STATIC_INITIALIZER -DGOOGLE_PROTOBUF_INTERNAL_DONATE_STEAL_INLINE=0 -DHAVE_PTHREAD -DWEBRTC_ENABLE_AVX2 -DWEBRTC_NON_STATIC_TRACE_EVENT_HANDLERS=0 -DWEBRTC_CHROMIUM_BUILD -DWEBRTC_POSIX -DWEBRTC_LINUX -DABSL_ALLOCATOR_NOTHROW=1 -DWEBRTC_USE_X11 -DWEBRTC_USE_PIPEWIRE -DWEBRTC_USE_GIO -DLOGGING_INSIDE_WEBRTC -DLEVELDB_PLATFORM_CHROMIUM=1 -DV8_COMPRESS_POINTERS -DV8_COMPRESS_POINTERS_IN_SHARED_CAGE -DV8_31BIT_SMIS_ON_64BIT_ARCH -DV8_ENABLE_SANDBOX -DCPPGC_CAGED_HEAP -DCPPGC_YOUNG_GENERATION -DCPPGC_POINTER_COMPRESSION -DATK_LIB_DIR=\"/usr/lib/x86_64-linux-gnu\" -DUSE_ATK_BRIDGE -DOPENSCREEN_TEST_DATA_DIR=\"third_party/openscreen/src/test/data/\" -DENABLE_TRACE_LOGGING -DUSE_CUPS -DLIBGAV1_MAX_BITDEPTH=10 -DLIBGAV1_THREADPOOL_USE_STD_MUTEX -DLIBGAV1_ENABLE_LOGGING=0 -DLIBGAV1_PUBLIC= -DVK_NO_PROTOTYPES -DUSE_VULKAN_XCB -DHAVE_INSPECTOR=1 -DHAVE_OPENSSL=1 -DNODE_HAVE_I18N_SUPPORT=1 -DNODE_USE_V8_PLATFORM=0 -D_POSIX_C_SOURCE=200112 -DCRASHPAD_ZLIB_SOURCE_EXTERNAL -DFLATBUFFERS_LOCALE_INDEPENDENT=0 -I../../electron -Igen/electron -I../../third_party/blink/renderer -I../.. -Igen -I../../buildtools/third_party/libc++ -I../../third_party/perfetto/include -Igen/third_party/perfetto/build_config -Igen/third_party/perfetto -I../../third_party/libyuv/include -I../../third_party/jsoncpp/source/include -I../../third_party/libwebp/src/src -I../../third_party/vulkan-deps/vulkan-headers/src/include -I../../third_party/wayland/src/src -I../../third_party/wayland/include/src -I../../third_party/khronos -I../../gpu -Igen/third_party/dawn/include -I../../third_party/dawn/include -Igen/third_party/private_membership/src -Igen/third_party/shell-encryption/src -Igen/components/policy/proto -I../../third_party/abseil-cpp -I../../third_party/boringssl/src/include -I../../third_party/protobuf/src -Igen/protoc_out -I../../third_party/ced/src -I../../third_party/icu/source/common -I../../third_party/icu/source/i18n -I../../third_party/skia -I../../third_party/wuffs/src/release/c -I../../third_party/vulkan/include -I../../third_party/ipcz/include -I../../net/third_party/quiche/overrides -I../../net/third_party/quiche/src/quiche/common/platform/default -I../../net/third_party/quiche/src -Igen/net/third_party/quiche/src -I../../third_party/webrtc_overrides -I../../third_party/webrtc -Igen/third_party/webrtc -I../../third_party/libwebm/source -I../../third_party/mesa_headers -I../../third_party/leveldatabase -I../../third_party/leveldatabase/src -I../../third_party/leveldatabase/src/include -I../../third_party/libaom/source/libaom -I../../v8/include -Igen/v8/include -Igen/third_party/metrics_proto -I../../third_party/zlib -I../../third_party/re2/src -I../../third_party/openscreen/src -Igen/third_party/openscreen/src -I../../third_party/libgav1/src -I../../third_party/libgav1/src/src -I../../third_party/wayland/include -I../../third_party/wayland/src/cursor -I../../third_party/wayland/src/egl -Igen/third_party/wayland/src/protocol -I../../third_party/electron_node/src -I../../third_party/electron_node/deps/uv/include -I../../third_party/crashpad/crashpad -I../../third_party/crashpad/crashpad/compat/linux -I../../third_party/crashpad/crashpad/compat/non_win -I../../third_party/flatbuffers/src/include -Wall -Werror -Wextra -Wimplicit-fallthrough -Wextra-semi -Wunreachable-code-aggressive -Wthread-safety -Wno-missing-field-initializers -Wno-unused-parameter -Wno-psabi -Wloop-analysis -Wno-unneeded-internal-declaration -Wenum-compare-conditional -Wno-ignored-pragma-optimize -Wno-deprecated-builtins -Wno-bitfield-constant-conversion -Wshadow -fno-delete-null-pointer-checks -fno-ident -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -funwind-tables -fPIC -pthread -fcolor-diagnostics -fmerge-all-constants -fcrash-diagnostics-dir=../../tools/clang/crashreports -mllvm -instcombine-lower-dbg-declare=0 -ffp-contract=off -fcomplete-member-pointers -m64 -msse3 -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -ffile-compilation-dir=. -no-canonical-prefixes -ftrivial-auto-var-init=pattern -O2 -fdata-sections -ffunction-sections -fno-unique-section-names -fno-omit-frame-pointer -gdwarf-4 -g1 -gdwarf-aranges -fvisibility=hidden -Xclang -add-plugin -Xclang find-bad-constructs -Xclang -plugin-arg-find-bad-constructs -Xclang raw-ref-template-as-trivial-member -Xclang -plugin-arg-find-bad-constructs -Xclang check-ipc -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/gio-unix-2.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/libmount -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/blkid -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/glib-2.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/lib/x86_64-linux-gnu/glib-2.0/include -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/glib-2.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/lib/x86_64-linux-gnu/glib-2.0/include -DPROTOBUF_ALLOW_DEPRECATED=1 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/nss -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/nspr -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/at-spi2-atk/2.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/at-spi-2.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/dbus-1.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/lib/x86_64-linux-gnu/dbus-1.0/include -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/atk-1.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/glib-2.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/lib/x86_64-linux-gnu/glib-2.0/include -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/dbus-1.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/lib/x86_64-linux-gnu/dbus-1.0/include -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/libdrm -Wno-microsoft-include -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/gtk-3.0/unix-print -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/gtk-3.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/at-spi2-atk/2.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/at-spi-2.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/dbus-1.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/lib/x86_64-linux-gnu/dbus-1.0/include -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/gtk-3.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/gio-unix-2.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/cairo -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/pango-1.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/fribidi -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/harfbuzz -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/atk-1.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/cairo -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/pixman-1 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/uuid -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/freetype2 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/libpng16 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/gdk-pixbuf-2.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/libmount -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/blkid -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/glib-2.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/lib/x86_64-linux-gnu/glib-2.0/include -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/gdk-pixbuf-2.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/libmount -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/blkid -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/glib-2.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/lib/x86_64-linux-gnu/glib-2.0/include -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/gtk-3.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/at-spi2-atk/2.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/at-spi-2.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/dbus-1.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/lib/x86_64-linux-gnu/dbus-1.0/include -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/gtk-3.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/gio-unix-2.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/cairo -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/pango-1.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/fribidi -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/harfbuzz -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/atk-1.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/cairo -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/pixman-1 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/uuid -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/freetype2 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/libpng16 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/gdk-pixbuf-2.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/libmount -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/blkid -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/include/glib-2.0 -isystem../../build/linux/debian_bullseye_amd64-sysroot/usr/lib/x86_64-linux-gnu/glib-2.0/include -std=c++20 -Wno-trigraphs -gsimple-template-names -fno-exceptions -fno-rtti -nostdinc++ -isystem../../buildtools/third_party/libc++/trunk/include -isystem../../buildtools/third_party/libc++abi/trunk/include --sysroot=../../build/linux/debian_bullseye_amd64-sysroot -fvisibility-inlines-hidden -c ../../electron/shell/browser/javascript_environment.cc -o obj/electron/electron_lib/javascript_environment.o
../../electron/shell/browser/javascript_environment.cc:270:44: error: no member named 'PageAllocator' in 'gin::V8Platform'
      tracing_controller, gin::V8Platform::PageAllocator());
                          ~~~~~~~~~~~~~~~~~^
1 error generated.

@brjsp
Copy link
Contributor Author

brjsp commented Dec 19, 2022

fixed, somehow the second file got lost

See https://stackoverflow.com/a/15538759 for an explanation of class scope.
GCC gives an error when compiling this code: https://godbolt.org/z/sYhc3cMjE
@zcbenz zcbenz requested review from nornagon and removed request for nornagon January 5, 2023 00:41
@zcbenz zcbenz dismissed nornagon’s stale review February 8, 2023 07:03

Dismiss as the problem has been fixed.

@zcbenz zcbenz merged commit df6f99a into electron:main Feb 8, 2023
@release-clerk
Copy link

release-clerk bot commented Feb 8, 2023

No Release Notes

codebytere pushed a commit that referenced this pull request Feb 8, 2023
* fix: correct redefinition which is invalid C++

See https://stackoverflow.com/a/15538759 for an explanation of class scope.
GCC gives an error when compiling this code: https://godbolt.org/z/sYhc3cMjE

* Update export_gin_v8platform_pageallocator_for_usage_outside_of_the_gin.patch

---------

Co-authored-by: Cheng Zhao <zcbenz@gmail.com>
Co-authored-by: Cheng Zhao <github@zcbenz.com>
codebytere pushed a commit that referenced this pull request Feb 8, 2023
* fix: correct redefinition which is invalid C++

See https://stackoverflow.com/a/15538759 for an explanation of class scope.
GCC gives an error when compiling this code: https://godbolt.org/z/sYhc3cMjE

* Update export_gin_v8platform_pageallocator_for_usage_outside_of_the_gin.patch

---------

Co-authored-by: Cheng Zhao <zcbenz@gmail.com>
Co-authored-by: Cheng Zhao <github@zcbenz.com>
khalwa pushed a commit to solarwindscloud/electron that referenced this pull request Feb 22, 2023
* fix: correct redefinition which is invalid C++

See https://stackoverflow.com/a/15538759 for an explanation of class scope.
GCC gives an error when compiling this code: https://godbolt.org/z/sYhc3cMjE

* Update export_gin_v8platform_pageallocator_for_usage_outside_of_the_gin.patch

---------

Co-authored-by: Cheng Zhao <zcbenz@gmail.com>
Co-authored-by: Cheng Zhao <github@zcbenz.com>
gecko19 pushed a commit to brightsign/electron that referenced this pull request Feb 28, 2023
* fix: correct redefinition which is invalid C++

See https://stackoverflow.com/a/15538759 for an explanation of class scope.
GCC gives an error when compiling this code: https://godbolt.org/z/sYhc3cMjE

* Update export_gin_v8platform_pageallocator_for_usage_outside_of_the_gin.patch

---------

Co-authored-by: Cheng Zhao <zcbenz@gmail.com>
Co-authored-by: Cheng Zhao <github@zcbenz.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants