On Wed, Nov 20, 2024 at 11:58:30AM +0100, Richard Biener wrote:
> On Sun, Nov 17, 2024 at 4:24 AM Lewis Hyatt <[email protected]> wrote:
> >
> > Prepare libcpp to support 64-bit location_t, without yet making
> > any functional changes, by adding new typedefs that enable code to be
> > written such that it works with any size location_t. Update the usage of
> > line maps within libcpp accordingly.
> >
> > Subsequent patches will prepare the rest of the codebase similarly, and then
> > afterwards, location_t will be changed to uint64_t.
>
> This is OK if there's no comment from libcpp maintainers this week.
>
> Thanks,
> Richard.
Thank you, and thanks for going through the other patches, really appreciate
it.
I CCed David to make sure he doesn't have any concerns about this patch
or the others as well. (Especially since libdiagnostics is now in GCC 15, I
hope they will be a useful improvement from that perspective too... it would
be nice if it launches out of the gate without that limitation.)
BTW, since libdiagnostics has been committed in the meantime since I sent
these out. There is a two-line patch that should be added to this series to
prepare libdiagnostics for 64-bit location_t as well. I'll submit it with
the next version of the patches, but FYI it would be:
===
diff --git a/gcc/libdiagnostics.cc b/gcc/libdiagnostics.cc
index c06d3bc722f..9e054c89d31 100644
--- a/gcc/libdiagnostics.cc
+++ b/gcc/libdiagnostics.cc
@@ -321,7 +321,7 @@ public:
linemap_init (&m_line_table, BUILTINS_LOCATION);
m_line_table.m_reallocator = xrealloc;
m_line_table.m_round_alloc_size = round_alloc_size;
- m_line_table.default_range_bits = 5;
+ m_line_table.default_range_bits = line_map_suggested_range_bits;
}
~diagnostic_manager ()
{
@@ -501,7 +501,7 @@ private:
impl_client_version_info m_client_version_info;
std::vector<std::unique_ptr<sink>> m_sinks;
hash_map<nofree_string_hash, diagnostic_file *> m_str_to_file_map;
- hash_map<int_hash<location_t, UNKNOWN_LOCATION, UINT_MAX>,
+ hash_map<int_hash<location_t, UNKNOWN_LOCATION, location_t (-1)>,
diagnostic_physical_location *> m_location_t_map;
std::vector<std::unique_ptr<diagnostic_logical_location>> m_logical_locs;
const diagnostic *m_current_diag;
===
And last thing, the following hunk I realized does not work properly when
committed in
isolation (only after the entire set of patches is in place):
> > @@ -1529,7 +1536,10 @@ linemap_compare_locations (const line_maps *set,
> > if (IS_ADHOC_LOC (l1))
> > l1 = get_location_from_adhoc_loc (set, l1);
> >
> > - return l1 - l0;
> > + /* This function is intended e.g. for implementing a qsort() comparator,
> > so it
> > + needs to return really an "int" and not something larger. */
> > + const location_diff_t res = l1 - l0;
> > + return res < INT_MIN ? INT_MIN : res > INT_MAX ? INT_MAX : res;
> > }
The first line needs to be rather
+ const auto res = (location_diff_t)l1 - (location_diff_t)l0;
to avoid issues with wrapping while location_t is still a uint32_t. I have
that fixed now.
-Lewis
