Bug 3301 – Undefined identifier error dependent on order of imports when a circular import is involved

In DMD 2.032, the order of import statements seems to be important. Specifically, with regard to cyclic import statements. Below is a simple test case using two files winapi.d and wincom.d module winapi; public import wincom; // If the import is here, winapi does not compile public import std.c.windows.windows; //public import wincom; // If the import is here, winapi compiles alias HRESULT THEMEAPI; module wincom; public import winapi; public import std.c.windows.com; HRESULT hresult; When compiling winapi, wincom errors are generated: wincom.d(6): Error: identifier 'HRESULT' is not defined wincom.d(6): Error: HRESULT is used as a type wincom.d(6): Error: variable wincom.hresult voids have no value

It's worth noting that when this happens you can still access the stuff in the imported module by using the fully qualified name. The bug is effectively causing the imports to be imported as static.

(In reply to comment #1) > It's worth noting that when this happens you can still access the stuff in the > imported module by using the fully qualified name. The bug is effectively > causing the imports to be imported as static. Although that's true of the test case, there were other examples in DFL where the fully qualified name was being used and it was still not accessible. So in practice, the FQN doesn't solve the whole problem, but this may be a different bug. (It's just that those examples were a lot more complicated. I'm specifically thinking of the use of classes from data.d in control.d)

It appears that this could be related to issue 258. How it found a line 6 in wincom.d is another mystery.

Possibly, but my code compiles fine in 2.028 & 2.031. I've made no changes at all in my code and now it's foo barred w/ 2.032. So (obv.) some difference in the 2.032 change set has exacerbated this bug. Given how much stuff got done, that's probably not going to help much though.

(In reply to comment #4) > Possibly, but my code compiles fine in 2.028 & 2.031. > I've made no changes at all in my code and now it's foo barred w/ 2.032. > > So (obv.) some difference in the 2.032 change set has exacerbated this bug. > Given how much stuff got done, that's probably not going to help much though. Sry, ignore the bit about 2.031. That's foo barred as well, but that might be phobos issues.

I'm getting the same errors for the testcases here in 1.046, 1.047, 2.031 and 2.032 alike. Though in all cases, the errors are reported at wincom.d(4). ---------- C:\Users\Stewart\Documents\Programming\D\Tests\bugs>dmd -c wincom.d C:\Users\Stewart\Documents\Programming\D\Tests\bugs>dmd -c winapi.d wincom.d(4): Error: identifier 'HRESULT' is not defined wincom.d(4): Error: HRESULT is used as a type wincom.d(4): Error: variable wincom.hresult voids have no value C:\Users\Stewart\Documents\Programming\D\Tests\bugs>dmd -c wincom.d winapi.d C:\Users\Stewart\Documents\Programming\D\Tests\bugs>dmd -c winapi.d wincom.d wincom.d(4): Error: identifier 'HRESULT' is not defined wincom.d(4): Error: HRESULT is used as a type wincom.d(4): Error: variable wincom.hresult voids have no value

Bumping up to a P1 bug. Walter, this needs to be at least commented upon asap. It's a rather nasty regression. For the rest of you, a standalone repro case would likely help. The one from the original problem description requires importing std.c.windows.windows which itself is rather huge.

I'm not sure how trimming it down can be that difficult.... ----- bz3301.d ----- public import bz3301a; public import bz3301b; ----- bz3301a.d ----- public import bz3301; HRESULT hresult; ----- bz3301b.d ----- alias int HRESULT; ---------- C:\Users\Stewart\Documents\Programming\D\Tests\bugs>dmd -c bz3301.d bz3301a.d(2): Error: identifier 'HRESULT' is not defined bz3301a.d(2): Error: HRESULT is used as a type bz3301a.d(2): Error: variable bz3301a.hresult voids have no value ---------- (1.047)

Thanks for doing a small test case. This problem has actually always been there. I don't know why it didn't show up earlier.

It has shown up a number of times, for example in #258 and #1592. I have a patch for this (and the bugs mentioned above) that parses every file that is unconditionally imported before any semantic analysis. As this introduces another compile step, I'm not sure if this is a good patch, and it will fail for conditional imports. So I'm hoping for a more general solution that works with the deferred semantic analysis and solves more of the forward reference problems. Should I attach the patch anyway?

Yes indeed, this test case is reproducible on 1.041 as well, but 1.041 compiles QtD and gtkD well. And 1.047 doesn't. Does it mean that it is a separate issue?

Created attachment 454 do all imports before semantic analysis Having done some investigation of the deferred semantic analysis mechanism, it might be best to try to keep its usage to a minimum, because it might have a negative impact on compilation speed and memory consumption. So my patch is probably not so bad after all. Some details: The problem is that, though the files are parsed, their symbol table might not yet hold any identifiers, because the semantic analysis has not been run on the module. So I introduced another compile step after parsing, but before the semantic analysis. The files on the command line are scanned for import statements, loading and parsing the referenced modules and applying the same process on these files. In the same step, the global members of the module are added to the modules' symbol table (this has been moved from the semantic analysis). The import statements must not be conditional, because the semantic analysis is needed to evaluate a "static if" statement. If the semantic analysis gets to the import statement, the additional compile step kicks in again for any module not already loaded. This patch also fixes bugs #3286, #1592 and #258

(In reply to comment #12) > Created an attachment (id=454) [details] > do all imports before semantic analysis > > Having done some investigation of the deferred semantic analysis mechanism, it > might be best to try to keep its usage to a minimum, because it might have a > negative impact on compilation speed and memory consumption. So my patch is > probably not so bad after all. > > Some details: > The problem is that, though the files are parsed, their symbol table might not > yet hold any identifiers, because the semantic analysis has not been run on the > module. > > So I introduced another compile step after parsing, but before the semantic > analysis. The files on the command line are scanned for import statements, > loading and parsing the referenced modules and applying the same process on > these files. In the same step, the global members of the module are added to > the modules' symbol table (this has been moved from the semantic analysis). > > The import statements must not be conditional, because the semantic analysis is > needed to evaluate a "static if" statement. If the semantic analysis gets to > the import statement, the additional compile step kicks in again for any module > not already loaded. > > This patch also fixes bugs #3286, #1592 and #258 Dear Rainer, Your patch applied against dmd 1.047 fixes the issue with building both gtkD and QtD. It would be cool if it were included in the upstream. Thank you.

(In reply to comment #13) > Dear Rainer, > Your patch applied against dmd 1.047 fixes the issue with building both gtkD > and QtD. It would be cool if it were included in the upstream. > > Thank you. I had to use this to compile gtkD too and it worked well. The experimental dnet compiler being based on dmd's front end also has this issue and the patch did the job for that too. Thanks Rainer.

Created attachment 469 improved patch with version/debug support I have improved the patch to look through version/debug conditionals, that would bring back all the troubles if used for the imports. I've also moved the setScope calls into the importAll functions, which was already proposed in a comment nearby. At the same time, version/debug conditional declarations pass the scope to its included branch. StaticIfDeclarations cannot do this, because the semantic analysis is needed to evaluate the condition. In the long run, I'd propose to take the setScope mechanism further, setting the scope as early as possible, allowing semantic analysis in arbitrary order. I've run the qtd build with this patch and it still works. You'll also need the patch of issue 3353 for the build to complete, though.

Fixed dmd 1.049 and 2.034