Skip to content
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

fix Issue 23548 - [REG 2.098] C sources files have precedent over D modules in imports #14687

Merged
merged 1 commit into from
Dec 12, 2022

Conversation

ibuclaw
Copy link
Member

@ibuclaw ibuclaw commented Dec 12, 2022

C sources should not be imported if there exists a D module in an include path that also matches.

Test doesn't exactly match that in the regression issue, as in the bug report, the C source file gets picked up by the implicit -I.

This would allow fixing issue 23479 without causing the regression in issue 23547.

@ibuclaw ibuclaw added the Severity:Regression PRs that fix regressions label Dec 12, 2022
@dlang-bot
Copy link
Contributor

Thanks for your pull request, @ibuclaw!

Bugzilla references

Auto-close Bugzilla Severity Description
23548 regression [REG 2.098] C sources files have precedent over D modules in imports

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#14687"

@WalterBright
Copy link
Member

This breaks any existing projects that relied on the existing search order. This does not achieve the goal of not causing any regressions.

There's another problem with this. When I developed Warp, the C preprocessor program, I discovered that a big chunk of time was lost in file lookups and path searching. What this PR appears to do is greatly increase the number of traversals of the path.

A controversial issue like this should not have been pulled within 1 hour leaving no chance for response.

@WalterBright
Copy link
Member

It also introduces a confusing element to file lookups - c files are looked up using an entirely different algorithm than all the other lookups.

@adamdruppe
Copy link
Contributor

so to be clear, your position is to keep a proven regression in real world D code.... in order to prevent a hypothetical regression in likely non-existent importC code.

and you wonder why D can't retain users.

@ibuclaw
Copy link
Member Author

ibuclaw commented Dec 12, 2022

There's another problem with this. When I developed Warp, the C preprocessor program, I discovered that a big chunk of time was lost in file lookups and path searching. What this PR appears to do is greatly increase the number of traversals of the path.

If no file is found, there is no additional time complexity added compared to the previous strategy.

@ibuclaw
Copy link
Member Author

ibuclaw commented Dec 12, 2022

It also introduces a confusing element to file lookups - c files are looked up using an entirely different algorithm than all the other lookups.

It acknowledges that the overwhelming majority of users (i.e: the 2274 projects on code.dlang.org) don't expect the compiler to suddenly import C sources or headers because of a sudden change in the D module lookup strategy.

@WalterBright
Copy link
Member

It acknowledges that the overwhelming majority of users (i.e: the 2274 projects on code.dlang.org) don't expect the compiler to suddenly import C sources or headers because of a sudden change in the D module lookup strategy.

How many of them failed because of my solution?

@WalterBright
Copy link
Member

@adamdruppe > so to be clear, your position is to keep a proven regression in real world D code.... in order to prevent a hypothetical regression in likely non-existent importC code.

https://issues.dlang.org/show_bug.cgi?id=23548 is a hypothetical regression. I could just as easily file one for this PR.

@adamdruppe
Copy link
Contributor

gcc actually has a config.c file.

@ibuclaw
Copy link
Member Author

ibuclaw commented Dec 13, 2022

@adamdruppe > so to be clear, your position is to keep a proven regression in real world D code.... in order to prevent a hypothetical regression in likely non-existent importC code.

The real world regression was when the compiler starting importing H files instead of D modules in a major project, that has been reverted in #14682. This PR is an extrapolation of that thought after reviewing what's changed, and the conclusion is the problem is endemic to how ImportC is interacting with D's import system, not H just files.

@WalterBright
Copy link
Member

The problem is that if you specify a search path, you expect to find the first fit in the search path. Like how every other search path works. Trying to layer on some other special case algorithm leads to things like it takes 10 years to properly learn C++. Most people never do, they just randomly try things until it works. Even Bjarne didn't know that in one special case, parentheses do change the semantic meaning of an expression.

The reason we have the -revert switch is so that we can move D forward without turning it into C++. You've got one awkward file with a problem. That shouldn't define D's future.

People on the n.g. constantly call for D to stop having special cases and corner behavior. We should be working to remove them, not add them. We need to be willing to use the -revert switch now and then, because being able to compile older code built with certain expectations is important.

I once read the Go spec. It was just a few pages. Smaller, simpler specs lead to more adoption. I doubt more than 10 people have ever read the C++ spec end to end.

I follow the C++ news. In the last year, there seems to be a growing groundswell to either fix C++'s complexity or just move to another language. Let's not wind up like that.

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.

6 participants