On Wed, Jul 10, 2024 at 12:23 PM Iain Sandoe <i...@sandoe.co.uk> wrote:
>
> Hello Daniel,
>
> Thanks for the patch!
>
> > On 10 Jul 2024, at 10:43, Daniel Bertalan <d...@danielbertalan.dev> wrote:
> >
> > As of Xcode 16 beta 2 with the macOS 15 SDK, each re-inclusion of the
> > stddef.h header causes the NULL macro in C++ to be re-defined to an
> > integral constant (__null). This makes the workaround in d59a576b8
> > ("Redefine NULL to nullptr") ineffective, as other headers that are
> > typically included after system.h (such as obstack.h) do include
> > stddef.h too.
> >
> > This can be seen by running the sample below through `clang++ -E`
> >
> > #include <stddef.h>
> > #define NULL nullptr
> > #include <stddef.h>
> > NULL
> >
> > The relevant libc++ change is here:
> > https://github.com/llvm/llvm-project/commit/2950283dddab03c183c1be2d7de9d4999cc86131
> >
> > Filed as FB14261859 to Apple and added a comment about it on LLVM PR
> > 86843.
> >
> > This fixes the cases in --enable-languages=c,c++,objc,obj-c++,rust build
> > where NULL being an integral constant instead of a null pointer literal
> > (therefore no longer implicitly converting to a pointer when used as a
> > template function's argument) caused issues.
> >
> > gcc/value-pointer-equiv.cc:65:43: error: no viable conversion from
> > `pair<typename __unwrap_ref_decay<long>::type, typename
> > __unwrap_ref_decay<long>::type>' to 'const pair<tree, tree>'
> >
> > 65 | const std::pair <tree, tree> m_marker = std::make_pair (NULL,
> > NULL);
> > |
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > As noted in the previous commit though, the proper solution would be to
> > phase out the usages of NULL in GCC's C++ source code.
> >
> > gcc/analyzer/ChangeLog:
> >
> > * diagnostic-manager.cc (saved_diagnostic::saved_diagnostic):
> > Change NULL to nullptr.
> > (struct null_assignment_sm_context): Likewise.
> > * infinite-loop.cc: Likewise.
> > * infinite-recursion.cc: Likewise.
> > * varargs.cc (va_list_state_machine::on_leak): Likewise.
> >
> > gcc/rust/ChangeLog:
> >
> > * metadata/rust-imports.cc (Import::try_package_in_directory):
> > Change NULL to nullptr.
> >
> > gcc/ChangeLog:
> >
> > * value-pointer-equiv.cc: Change NULL to nullptr.
>
> This is fine from a Darwin/macOS perspective - and I’d say the changes are
> generally in
> the ‘obvious’ category - however, let’s give other maintainers some time to
> weigh in.
Yes, I think the patch is OK.
> NOTE: if you do not have a current copyright assignment to the FSF for GCC,
> then
> you need to post the patch under DCO - see
> https://gcc.gnu.org/contribute.html#legal
> for more information.
>
> thanks
> Iain
>
> > ---
> > gcc/analyzer/diagnostic-manager.cc | 18 +++++++++---------
> > gcc/analyzer/infinite-loop.cc | 2 +-
> > gcc/analyzer/infinite-recursion.cc | 2 +-
> > gcc/analyzer/varargs.cc | 2 +-
> > gcc/rust/metadata/rust-imports.cc | 2 +-
> > gcc/value-pointer-equiv.cc | 2 +-
> > 6 files changed, 14 insertions(+), 14 deletions(-)
> >
> > diff --git a/gcc/analyzer/diagnostic-manager.cc
> > b/gcc/analyzer/diagnostic-manager.cc
> > index fe943ac61c9e..51304b0795b6 100644
> > --- a/gcc/analyzer/diagnostic-manager.cc
> > +++ b/gcc/analyzer/diagnostic-manager.cc
> > @@ -679,12 +679,12 @@ saved_diagnostic::saved_diagnostic (const
> > state_machine *sm,
> > m_stmt (ploc.m_stmt),
> > /* stmt_finder could be on-stack; we want our own copy that can
> > outlive that. */
> > - m_stmt_finder (ploc.m_finder ? ploc.m_finder->clone () : NULL),
> > + m_stmt_finder (ploc.m_finder ? ploc.m_finder->clone () : nullptr),
> > m_loc (ploc.m_loc),
> > m_var (var), m_sval (sval), m_state (state),
> > - m_d (std::move (d)), m_trailing_eedge (NULL),
> > + m_d (std::move (d)), m_trailing_eedge (nullptr),
> > m_idx (idx),
> > - m_best_epath (NULL), m_problem (NULL),
> > + m_best_epath (nullptr), m_problem (nullptr),
> > m_notes ()
> > {
> > /* We must have an enode in order to be able to look for paths
> > @@ -1800,10 +1800,10 @@ public:
> > stmt,
> > stack_depth,
> > sm,
> > - NULL,
> > + nullptr,
> > src_sm_val,
> > dst_sm_val,
> > - NULL,
> > + nullptr,
> > dst_state,
> > src_node));
> > return false;
> > @@ -1993,9 +1993,9 @@ struct null_assignment_sm_context : public sm_context
> > m_sm,
> > var_new_sval,
> > from, to,
> > - NULL,
> > + nullptr,
> > *m_new_state,
> > - NULL));
> > + nullptr));
> > }
> >
> > void set_next_state (const gimple *stmt,
> > @@ -2019,9 +2019,9 @@ struct null_assignment_sm_context : public sm_context
> > m_sm,
> > sval,
> > from, to,
> > - NULL,
> > + nullptr,
> > *m_new_state,
> > - NULL));
> > + nullptr));
> > }
> >
> > void warn (const supernode *, const gimple *,
> > diff --git a/gcc/analyzer/infinite-loop.cc b/gcc/analyzer/infinite-loop.cc
> > index 8ba8e70acffc..6ac0a5b373d8 100644
> > --- a/gcc/analyzer/infinite-loop.cc
> > +++ b/gcc/analyzer/infinite-loop.cc
> > @@ -240,7 +240,7 @@ public:
> > enode->get_function ()->decl,
> > enode->get_stack_depth ()),
> > enode,
> > - NULL, NULL, NULL));
> > + nullptr, nullptr, nullptr));
> >
> > logger *logger = emission_path->get_logger ();
> >
> > diff --git a/gcc/analyzer/infinite-recursion.cc
> > b/gcc/analyzer/infinite-recursion.cc
> > index ef8ae90ab08e..885f9a8a9417 100644
> > --- a/gcc/analyzer/infinite-recursion.cc
> > +++ b/gcc/analyzer/infinite-recursion.cc
> > @@ -196,7 +196,7 @@ public:
> > m_callee_fndecl,
> > m_new_entry_enode->get_stack_depth ()),
> > enode,
> > - NULL, NULL, NULL));
> > + nullptr, nullptr, nullptr));
> > }
> >
> > /* Reject paths in which conjured svalues have affected control flow
> > diff --git a/gcc/analyzer/varargs.cc b/gcc/analyzer/varargs.cc
> > index c197883673d6..734323e6f789 100644
> > --- a/gcc/analyzer/varargs.cc
> > +++ b/gcc/analyzer/varargs.cc
> > @@ -631,7 +631,7 @@ va_list_state_machine::on_va_end (sm_context &sm_ctxt,
> > std::unique_ptr<pending_diagnostic>
> > va_list_state_machine::on_leak (tree var) const
> > {
> > - return make_unique<va_list_leak> (*this, NULL, var);
> > + return make_unique<va_list_leak> (*this, nullptr, var);
> > }
> >
> > } // anonymous namespace
> > diff --git a/gcc/rust/metadata/rust-imports.cc
> > b/gcc/rust/metadata/rust-imports.cc
> > index 17451fbcb9ba..7da245587ae8 100644
> > --- a/gcc/rust/metadata/rust-imports.cc
> > +++ b/gcc/rust/metadata/rust-imports.cc
> > @@ -179,7 +179,7 @@ Import::try_package_in_directory (const std::string
> > &filename,
> > "%s exists but does not contain any Rust export data",
> > found_filename.c_str ());
> >
> > - return std::make_pair (NULL, macros);
> > + return std::make_pair (nullptr, macros);
> > }
> >
> > // Given import "*PFILENAME", where *PFILENAME does not exist, try
> > diff --git a/gcc/value-pointer-equiv.cc b/gcc/value-pointer-equiv.cc
> > index bfc940ec9915..cfb53ab4eeef 100644
> > --- a/gcc/value-pointer-equiv.cc
> > +++ b/gcc/value-pointer-equiv.cc
> > @@ -62,7 +62,7 @@ public:
> > private:
> > auto_vec<std::pair <tree, tree>> m_stack;
> > auto_vec<tree> m_replacements;
> > - const std::pair <tree, tree> m_marker = std::make_pair (NULL, NULL);
> > + const std::pair <tree, tree> m_marker = std::make_pair (NULL_TREE,
> > NULL_TREE);
> > };
> >
> > ssa_equiv_stack::ssa_equiv_stack ()
> > --
> > 2.45.2
> >
> >
>
