On 12/18/2015 01:21 PM, David Malcolm wrote:

I don't think there's a way to fix -Wmisleading-indentation if we're
in this state, so the first part of the following patch detects if
this has happened, and effectively turns off -Wmisleading-indentation
from that point onwards.  To avoid a false sense of security, the
patch issues a "sorry" at the that point, currently with this wording:
location-overflow-test-1.c:17:0: sorry, unimplemented: -Wmisleading-indentation 
is disabled from this point onwards, since column-tracking was disabled due to 
the size of the code/headers
Seems reasonable. I can't see any way to get indentation warnings if we don't have column info.


Should this greater chance of hitting LINE_MAP_MAX_LOCATION_WITH_COLS
be filed as a separate PR?
I was originally going to say no, but I suspect there'll be a few folks that are going to bump up against it. Might as well have a canonical BZ for it.



The second part of the patch resolves this by adding an additional
level of fallback: a new LINE_MAP_MAX_LOCATION_WITH_PACKED_RANGES
threshold (currently 0x50000000) that occurs well before
the LINE_MAP_MAX_LOCATION_WITH_COLS threshold (0x60000000).
Once we reach the new LINE_MAP_MAX_LOCATION_WITH_PACKED_RANGES
threshold, the range-packing optimization is disabled (with a transition
to an ordinary map with m_range_bits == 0), effectively giving us a
much "longer runway" before the LINE_MAP_MAX_LOCATION_WITH_COLS
threshold is hit, at the cost to requiring the use of the ad-hoc
table for every location (e.g. every token of length > 1).
I haven't yet done performance testing on this.

The patch adds test coverage for this by using a plugin to simulate
the two levels of degraded locations.

Rough calculations, assuming 7 bits of columns,
LINE_MAP_MAX_LOCATION_WITH_COLS == 0x60000000
LINE_MAP_MAX_LOCATION_WITH_PACKED_RANGES == 0x50000000

gcc 5:
   0x60000000 / 128 per line = 12,582,912 lines of code before
   hitting the has-column-information limit.

gcc 6 trunk:
   0x60000000 / (128 * 32) per line = 393,216 lines of code before
   hitting the has-column-information limit.

with this patch:
   0x50000000 / (128 * 32) per line = 327,680 lines of code before
   hitting the range-packing limit, then:
   0x10000000 / 128 = 2,097,152 lines of code before hitting the
   has-column-information limit.
   giving 2,424,832 lines of code total before hitting the
   has-column-information limit.

These numbers will be less in the face of lines longer than
127 characters.

If the increased use of the ad-hoc table is an issue, another
approach might be to simply disable range-handling for locations
that go beyond a threshold location_t value: attempts to combine
locations above that value lead to you simply getting the caret
location.  If we take this approach, I think we'd still want to
have a range threshold before the column one, so that we preserve
the ability to have column information for these pure-caret
locations.

Alternatively, the range bits could be lowered from 5 to 4,
doubling the lines we can handle before hitting the limit:
0x60000000 / (128 * 16) = 786,432, though that doesn't buy
us much compared to the approach in this patch.

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.

Thoughts?

gcc/c-family/ChangeLog:
        PR c++/68819
        * c-indentation.c (get_visual_column): Handle the column
        number being zero by effectively disabling the warning, with
        a "sorry".
This part is fine as-is.



gcc/testsuite/ChangeLog:
        PR c++/68819
        * gcc.dg/plugin/location-overflow-test-1.c: New test case.
        * gcc.dg/plugin/location-overflow-test-2.c: New test case.
        * gcc.dg/plugin/location_overflow_plugin.c: New test plugin.
        * gcc.dg/plugin/plugin.exp (plugin_test_list): Add the above.

libcpp/ChangeLog:
        PR c++/68819
        * line-map.c (LINE_MAP_MAX_LOCATION_WITH_PACKED_RANGES): New
        constant.
        (LINE_MAP_MAX_LOCATION_WITH_COLS): Add note about unit tests
        to comment.
        (can_be_stored_compactly_p): Reduce threshold from
        LINE_MAP_MAX_LOCATION_WITH_COLS to
        LINE_MAP_MAX_LOCATION_WITH_PACKED_RANGES.
        (get_combined_adhoc_loc): Likewise.
        (get_range_from_loc): Likewise.
        (linemap_line_start): Ensure that a new ordinary map is created
        when transitioning from range-packing being enabled to disabled,
        at the LINE_MAP_MAX_LOCATION_WITH_PACKED_RANGES threshold.  Set
        range_bits to 0 for new ordinary maps when beyond this limit.
        Prevent the "increase the column bits of a freshly created map"
        optimization if the range bits has reduced.

+
+/* We use location_overflow_plugin.c to inject the
+   which injects the case that location_t values have exceeded
+   LINE_MAP_MAX_LOCATION_WITH_COLS, and hence no column
+   numbers are available.  */
It's just a test, but the comment doesn't parse. "to inject the which" :-) It's repeated in the second test as well.

With the comment fixes this is OK.

jeff

Reply via email to