Skip to content

Comments

[stable] Fix Issue 24144 - [REG2.105] Silent file name index overflow#15604

Merged
RazvanN7 merged 1 commit intodlang:stablefrom
symmetryinvestments:sym-2.105.x
Sep 14, 2023
Merged

[stable] Fix Issue 24144 - [REG2.105] Silent file name index overflow#15604
RazvanN7 merged 1 commit intodlang:stablefrom
symmetryinvestments:sym-2.105.x

Conversation

@kinke
Copy link
Contributor

@kinke kinke commented Sep 13, 2023

  • Print an ICE message for Loc.filename overflows
  • Increase the Loc size again, raising the file name limit from 64K to 4G but keeping the index indirection from reduce Loc.sizeof by 8 bytes #15199. The Loc size for DMD-as-a-library is 16 bytes then, and 12 bytes otherwise.

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @kinke! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close Bugzilla Severity Description
24144 regression [REG2.105] Silent file name index overflow

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "stable + dmd#15604"

* Print an ICE message for Loc.filename overflows
* Increase the Loc size again, raising the file name limit from 64K
  to 4G but keeping the index indirection from dlang#15199. The `Loc`
  size for DMD-as-a-library is 16 bytes then, and 12 bytes otherwise.
@kinke kinke marked this pull request as ready for review September 13, 2023 15:10
@kinke kinke requested a review from ibuclaw as a code owner September 13, 2023 15:10
char structliteralexp[76LLU];
char compoundliteralexp[40LLU];
char nullexp[25LLU];
char nullexp[29LLU];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not too bad, eh? Seems like the size of 12 and alignment of 4 does pay off vs. the previous 16-bytes size.

Copy link
Contributor

@dkorpel dkorpel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should eventually be reduced to 4 bytes anyway

@ibuclaw
Copy link
Member

ibuclaw commented Sep 13, 2023

Should eventually be reduced to 4 bytes anyway

Struct Loc? It shouldn't matter what size it is. Nodes should only have a reference to a unique location to keep their size down.

@tgehr
Copy link
Contributor

tgehr commented Sep 13, 2023

How about just keeping an index into the concatenation of all source files? The source file and file location can be recovered using binary search on a prefix sum of file sizes.

@RazvanN7 RazvanN7 merged commit 13da64b into dlang:stable Sep 14, 2023
@kinke kinke deleted the sym-2.105.x branch September 14, 2023 11:52
@kinke kinke restored the sym-2.105.x branch September 14, 2023 11:52
@WalterBright
Copy link
Member

The function adjustLocForMixin in dsymbolsem.d is the party guilty of generating lots of filenames.

@ibuclaw ibuclaw mentioned this pull request Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants