From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x844.google.com (mail-qt1-x844.google.com [IPv6:2607:f8b0:4864:20::844]) by sourceware.org (Postfix) with ESMTPS id BF235385B835 for ; Thu, 9 Apr 2020 23:35:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org BF235385B835 Received: by mail-qt1-x844.google.com with SMTP id f13so189799qti.5 for ; Thu, 09 Apr 2020 16:35:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ORUzn8Dm+7QjJbBffHl1vEHQnIG20jTCu8gxA/aNCso=; b=rNer2viKT8pg2LXbMXpLStCDU9XGzFIAu19ucuEAxX5j1YHQTzSpvxc6BVoWx4mXnn rsHWJbP7CeFcWc2SfvkM3L48+Sk1CeZoEsklfD8invG4J58VvwYwN8su39yqa8nc3duU XVG9wTf4RAdGOkSvoBTQgYvVWfFNINj9W8JjUOgwOGFFp/AP/yhgIMry9YpZrDf2U+rF qIiRjZeBiLPareNanEIHKhqwAunFXm7Ffvhmf5DMNdQjLxdD3g73QMzt2F/hIsKetB+v sN0idKyafkJuqqzNkOQJ1oWf1ddTrsNzaxPZI/PcUsTI7mvvhGuWbOveYV7P1YiZLNHp apmw== X-Gm-Message-State: AGi0PuYjIdiIJjdc35LPbO9T+97XkNDmCO8vuQ7bctRem58t3I7LxozF xOe3KhvaBfrONwC8yMVkfh1Hkj1YP/VYtzoS4HNqRGmJ X-Google-Smtp-Source: APiQypKa6AMVCCCe375UAE9Z1acvpXanwBJzbzjH+haCEMwBVXXfrW/ikr2vhg+R3zrkufBMhYWtW9/p3cf2HlA+Sjs= X-Received: by 2002:ac8:2921:: with SMTP id y30mr1896616qty.161.1586475314978; Thu, 09 Apr 2020 16:35:14 -0700 (PDT) MIME-Version: 1.0 References: <20200409225022.3093099-1-simon.marchi@polymtl.ca> In-Reply-To: <20200409225022.3093099-1-simon.marchi@polymtl.ca> From: Christian Biesinger Date: Thu, 9 Apr 2020 18:34:38 -0500 Message-ID: Subject: Re: [PATCH] gdbsupport: include cstdlib in common-defs.h To: Simon Marchi Cc: gdb-patches , Alexey Brodkin Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-37.9 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Apr 2020 23:35:17 -0000 On Thu, Apr 9, 2020 at 5:50 PM Simon Marchi via Gdb-patches wrote: > > In PR 25731 [1], the following build failure was reported: > > ../../binutils-gdb/gdb/gdbtypes.c:1254:10: error: no member named 'abs' in namespace 'std'; did you mean simply 'abs'? > = ((std::abs (stride) * element_count) + 7) / 8; > ^~~~~~~~ > abs > /usr/include/stdlib.h:129:6: note: 'abs' declared here > int abs(int) __pure2; > ^ > The original report was using: > > $ gcc -v > Apple LLVM version 8.0.0 (clang-800.0.42.1) > Target: x86_64-apple-darwin15.6.0 > > Note that I was _not_ able to reproduce using: > > $ g++ --version > Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/4.2.1 > Apple clang version 11.0.0 (clang-1100.0.33.17) > Target: x86_64-apple-darwin19.3.0 > > The proposed fix is to include in addition to . > > Here's an excerpt of [2] relevant to this problem: > > These headers [speaking of the .h form] are allowed to also declare > the same names in the std namespace, and the corresponding cxxx > headers are allowed to also declare the same names in the global > namespace: including definitely provides std::malloc and > may also provide ::malloc. Including definitely provides > ::malloc and may also provide std::malloc > > Since we use std::abs, we should not assume that our include of stdlib.h > declares an `abs` function in the `std` namespace. > > If we replace the include of stdlib.h with cstdlib, then we fall in the > opposite situation. A standard C++ library may decide to only put the > declarations in the std namespace, requiring us to prefix all standard > functions with `std::`. I'm not against that, but for the moment I think the > safest way forward is to just include both. > > Note that I don't know what effect this patch can have on any stdlib.h fix > provided by gnulib. > > [1] https://sourceware.org/bugzilla/show_bug.cgi?id=25731 > [2] https://en.cppreference.com/w/cpp/header#C_compatibility_headers > > gdbsupport/ChangeLog: > > * common-defs.h: Include cstdlib.h. > --- > gdbsupport/common-defs.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/gdbsupport/common-defs.h b/gdbsupport/common-defs.h > index e42d2b80c045..257b6279b888 100644 > --- a/gdbsupport/common-defs.h > +++ b/gdbsupport/common-defs.h > @@ -84,6 +84,7 @@ > > #include > #include > +#include > #include > #include > #include I think this is reasonable, but I would add a comment why both cstdlib and stdlib.h are included (ie. to get the std:: version with overloaded functions) Christian