Skip to content

Do not call getcwd (or other base) in absolutePath if not needed. #522

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 3, 2012

Conversation

denis-sh
Copy link
Contributor

@denis-sh denis-sh commented Apr 7, 2012

No description provided.

// TODO: @safe (BUG 6405) pure (because of buildPath())
{
if (path.empty) return null;
if (isAbsolute(path)) return path;
if (!isAbsolute(base)) throw new Exception("Base directory must be absolute");
return buildPath(base, path);
immutable baseVar = base;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this line? Surely dmd has to be smart enough to only call getcwd the first time base is used?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nevermind, I see now that it is supposed to be evaluated on every use, by design.

@kyllingstad
Copy link
Contributor

Would it make sense to do the same for relativePath, while we're at it?

@denis-sh
Copy link
Contributor Author

denis-sh commented May 3, 2012

Would it make sense to do the same for relativePath, while we're at it?

Of course. Didn't notice that relativePath uses the same thing. Should I merge this two small commits to not pollute history?

@kyllingstad
Copy link
Contributor

I don't think that's necessary. (Personally, I prefer making many small commits rather than a few big ones, as long as each commit leaves the code in a working state.)

@kyllingstad
Copy link
Contributor

Hmm.. it seems there is a merge conflict, most likely due to a recent update I made to std.path. This means that the auto-tester is unable to test it. On which platforms have you run the unittests?

@denis-sh
Copy link
Contributor Author

denis-sh commented May 3, 2012

Rebased.

On which platforms have you run the unittests?

I don't run any unittests because autotester made me lazy...

@kyllingstad
Copy link
Contributor

Cool, I wouldn't have expected GitHub to handle a rebase so nicely. All right, I'll merge the pull request when it has passed through the autotester.

kyllingstad added a commit that referenced this pull request May 3, 2012
Do not call `getcwd` (or other `base`) in `absolutePath` if not needed.
@kyllingstad kyllingstad merged commit 2594c1a into dlang:master May 3, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants