dmd.root.filename: Fix FileName.canonicalPath() regression on Windows #11293
Conversation
|
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 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 referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "stable + dmd#11293" |
| scope(exit) .free(path.ptr); | ||
| if (path !is null) | ||
| return path.toDString; | ||
| return xarraydup(path.toDString); |
There was a problem hiding this comment.
Can you add a comment, e.g. // Make sure we consistently return Mem-allocated memory. Otherwise someone is bound to see this and think it can be trivially improved.
There was a problem hiding this comment.
To prevent such an 'improvement', I've added the FileName.free() call in the test. But that needs an enabled GC (-lowmem) to hopefully produce an error.
|
I recently noticed that when I compiled dmd with asserts enabled, and ran without any arguments, it assert failed in root/filename.d. Is this that problem? |
|
Very likely, as DMD uses this function when looking up the config file IIRC. |
The assertion was wrong and guaranteed to fail when enabled (my bad). This is a reboot of dlang#11111. Also make sure to return a new root.rmem-managed memory allocation in all cases, i.e., not the malloc'd memory directly for the canonicalize_file_name version. E.g., the memory is released via mem.xfree() in FileName.safeSearchPath().
The assertion was wrong and guaranteed to fail when enabled (my bad in #11089).
This is a reboot of #11111.
Also make sure to return a new
root.rmem-managed memory allocation in all cases, i.e., not the malloc'd memory directly for thecanonicalize_file_nameversion. E.g., the memory is released viamem.xfree()inFileName.safeSearchPath().