From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 08125387093F for ; Thu, 5 Oct 2023 12:26:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 08125387093F Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1696508818; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=I65bx09sGlHxDi/YUUkmfqAqK5USO1K0Snx2zupm/Fs=; b=M6KnAK+jCnFxwoB4J1XQ7SYyTYQnXMZT7UWbEpMchI1RVNIjHz/ecYi/2CoLo7v4P7sH/8 eX1SsGxFEngUtND4f5Hb4gsRpoWVNOv5WA5lp78JvhuDIxmv9BhWokbh7lu5CSty6l9lAQ tG3kXOSHAq5O1it5T5UFMOAJ7HI+UHg= Received: from mail-vs1-f71.google.com (mail-vs1-f71.google.com [209.85.217.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-662-cfskGJWTNbeMYygYiu9VYQ-1; Thu, 05 Oct 2023 08:26:57 -0400 X-MC-Unique: cfskGJWTNbeMYygYiu9VYQ-1 Received: by mail-vs1-f71.google.com with SMTP id ada2fe7eead31-45289b05c67so401308137.2 for ; Thu, 05 Oct 2023 05:26:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696508817; x=1697113617; h=content-transfer-encoding:mime-version:message-id:date:references :in-reply-to:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ePWjjB4wAruBblHj5REutr7x/PaSdaHYKXrUF2MfiEE=; b=KwJt5hNF6nTBoVKJM/h7FTxfM8verJcC1ReCLBHjZVDrfjKaihVPCa2/aPDQN1XLxp UVux7GnkUVUU18+Enxk40B0eYuejyHIat6aRn6cJCcZ7qbDdOj13Ng5uGGJKIle994RY PILD4+B8e2ZUA5QBuRtKZLM0kiRW4GLG5Yzdyjh0ZTyuS226GzTNS2NSQhPqBmys6k5A 1P77ZnPDA29FZfh92Rr8KEWCOThb/5q+0qKhipQOifBXzGgXaOzxO4N7wjDBiLwiZ/Hk HamW5Oi3of1d1prSEp/saYcLE7FbRV6iiC/qnUYFyxSJwNTnGeT15PJKEWYUes4ow843 70mQ== X-Gm-Message-State: AOJu0YxDcW/HeyaLgnN4SHL52EF7fbwSxFj7IoyQdN1FuNAvHAeAG5b8 8j4dJs34T/yC1Z3eq0+F90CpAJeSEuAvnhKuj6I2l3xcjmfaGQjjTX0AfZ4iAD2B/xCJ3Z0DAzu RNFtG4FbfKQD+YegpSNnYFA== X-Received: by 2002:a05:6102:4b9:b0:452:6234:ed3 with SMTP id r25-20020a05610204b900b0045262340ed3mr4994548vsa.22.1696508817201; Thu, 05 Oct 2023 05:26:57 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEhTmGZzN+H0wk1EdiOJ85vbEZVf0oc4FFsCJJC08+UZUp9X+OXfD4pbsP2UiJMI1GTwWF0FQ== X-Received: by 2002:a05:6102:4b9:b0:452:6234:ed3 with SMTP id r25-20020a05610204b900b0045262340ed3mr4994534vsa.22.1696508816883; Thu, 05 Oct 2023 05:26:56 -0700 (PDT) Received: from localhost ([31.111.84.209]) by smtp.gmail.com with ESMTPSA id p13-20020ac8460d000000b00403cce833eesm443421qtn.27.2023.10.05.05.26.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 05 Oct 2023 05:26:56 -0700 (PDT) From: Andrew Burgess To: Lancelot SIX , Tom de Vries Cc: gdb-patches@sourceware.org Subject: Re: [PATCH RFC] Require c++17 compiler In-Reply-To: <20231005103354.5loeki67slszfrcy@hpe6u-23> References: <20231005065449.32643-1-tdevries@suse.de> <20231005103354.5loeki67slszfrcy@hpe6u-23> Date: Thu, 05 Oct 2023 13:26:54 +0100 Message-ID: <87h6n5tfu9.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,RCVD_IN_SORBS_WEB,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Lancelot SIX via Gdb-patches writes: > Hi Tom, > > Thanks for putting this together! As you know, I=E2=80=AFam in favor of = such > change (for GDB-15 probably, GDB-14 branching is probably too close)! > > I do have a really similar patch available on a local branch. One > difference though is that my branch uses a preparatory patch to upgrade > gdb/ax_cxx_compile_stdcxx.m4 to follow upstream changes to this file. > This upgrade gives some improvements in the C++17 compiler detection. > We keep this in the GDB tree because it has some local changes to set > CXX_DIALECT. This preparatory patch should be included in this reply. > > On Thu, Oct 05, 2023 at 08:54:49AM +0200, Tom de Vries wrote: >> Since gdb 8.0, we've required a c++11 compiler. >>=20 >> That allowed gdb to be compiled with gcc releases as far back as gcc 4.8= , >> using an explicit -std=3Dc++11 to override the default. >>=20 >> [ Gcc has the following defaults: >> - before gcc 6: c++98, and >> - for gcc 6-10: c++14, and >> - since gcc 11: c++17. ] >>=20 >> There are two arguments in favor of moving to requiring a newer c++ stan= dard: >> - benefits can be had from using newer c++ features. We're already usin= g some >> of those features using gdb-specific implementations. >> - when developing gdb on modern platforms with system compiler gcc >=3D = 6, >> people can accidentally use c++14/c++17 features in a patch, only to f= ind out >> post-commit that it breaks the build on a system with a compiler that = only >> supports c++11, which is inconvenient and takes time to fix. >> [ This could be partially mitigated by if possible also forcing -std= =3Dc++11 >> for such compilers. ] > > Thanks for pointing this out, this was one trigger for me to start > looking into this! > >>=20 >> The need to keep requiring c++11 comes from porting new gdb releases to = older >> systems with older system compilers. >>=20 >> A review of some relevant distros has shown that in case that requiring = c++17 >> results in no longer being able to use the system compiler, an alternati= ve, >> newer compiler that does support c++17 is readily available. >>=20 >> With nothing holding back the change, require a c++17 compiler to build = gdb. > > Just for the record, it might be worth mentioning here the general > policy of the project regarding bumping the C++ required version: > https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#When_i= s_GDB_going_to_start_requiring_C.2B-.2B-NN_.3F > >>=20 >> The gcc documentation ( https://gcc.gnu.org/projects/cxx-status.html ) s= tates >> that "Some C++17 features are available since GCC 5, but support was >> experimental and the ABI of C++17 features was not stable until GCC 9". >>=20 >> Consequently, the NEWS item mentions gcc 9 as an example compiler to use= . >>=20 >> My understanding of using gcc 5-8 is that it works as long as gdb doesn'= t use >> not yet available language features. Of course the set of used language >> features may change in time, so what compiler still works may change. >>=20 >> Problems can arise when shared libs starts to have C++17 based APIs (or >> somehow expose instantiations of such classes). If compiled with a comp= iler >> in which c++17 support was still experimental, it may be incompatible wh= en >> linking with: >> - code compiled with a compiler with non-experimental c++17 support, or >> - code compiled with a different compiler with experimental c++17 suppor= t. >>=20 >> Looking at the current implementation of our only shared lib: >> libinproctrace.so, I couldn't spot any c++ usage in the API, so I'm assu= ming >> this problem is not likely to happen. >>=20 >> Requiring c++17 means we can drop some code: >> ... >> $ find gdb* -type f \ >> | egrep -v "/configure|\.m4" \ >> | xargs grep "# *if.*__cplusplus.*2014" >> gdb/cp-support.c:#if __cplusplus >=3D 201402L >> gdb/dwarf2/cooked-index.c:#if __cplusplus >=3D 201402L >> gdbsupport/gdb_optional.h:#if defined(_GLIBCXX_DEBUG) && __cplusplus >= =3D 201402L >> gdbsupport/gdb_optional.h:#if defined(_GLIBCXX_DEBUG) && __cplusplus >= =3D 201402L >> gdbsupport/gdb_unique_ptr.h:#if __cplusplus >=3D 201402L >> gdbsupport/array-view.h:#if defined(_GLIBCXX_DEBUG) && __cplusplus >=3D = 201402L >> gdbsupport/array-view.h:#if defined(_GLIBCXX_DEBUG) && __cplusplus >=3D = 201402L >> gdbsupport/array-view.h:#if defined(_GLIBCXX_DEBUG) && __cplusplus >=3D = 201402L >> $ find gdb* -type f \ >> | egrep -v "/configure|\.m4" \ >> | xargs grep "# *if.*__cplusplus.*2017" >> gdb/unittests/string_view-selftests.c:#if __cplusplus < 201703L >> gdb/disasm.h:#if __cplusplus >=3D 201703L >> gdbsupport/invoke-result.h:#if __cplusplus >=3D 201703L >> gdbsupport/gdb_string_view.h:#if __cplusplus >=3D 201703L >> ... >> but that's not yet included in this patch. > > If this patch is accepted, one change I'd like is to do is drop > gdb::make_unique, because 1) the current fallback we have for C++11 > compilers is not entirely compatible with std::make_unique (the T[] case > is not supported properly I=E2=80=AFthink), and 2) it is not widely used = yet, so > "it's not too late" :D For my own education, is there something which we can write using gdb::make_unique, which compiles, and does something useful, but which isn't supported with std::make_unique? Or something that behaves differently with std::make_unique vs gdb::make_unique? You mention the T[] case isn't supported, but that doesn't feel like something we might be "too late" for -- that would just mean when we switch to std::make_unique we can do more... Thanks, Andrew