Better distinguish between qualified imports and sub-imports

[quark17]

As mentioned in #628, in BH/Classic, if you import a package qualified and that package defines a struct, you can refer to the fields of the struct (in struct creation expression, struct update expression, or struct pattern match) by their qualified names. For example:

import qualified FloatingPoint

h :: FloatingPoint.Half
h = FloatingPoint.Half {
  FloatingPoint.sign = True;
  FloatingPoint.exp = 0;
  FloatingPoint.sfd = 0
}

If you use the unqualified names, like this:

import qualified FloatingPoint

h :: FloatingPoint.Half
h = FloatingPoint.Half {
  sign = True;
  exp = 0;
  sfd = 0
}

then you get an error like this:

Error: "StructLit_QualImp_UnqualField.bs", line 7, column 2: (T0140)
  Cannot access fields in this expression since its type
  `FloatingPoint.FloatingPoint' has not been imported. Perhaps an import
  statement is missing, e.g., `import FloatingPoint::*;'

This error is misleading, because the type FloatingPoint has been imported. The error (if there is one at all) is that the fields are not in scope by the unqualified names.

The reason for the error is in findFields in TIMonad. This function is used when a field name is encountered in the source, and typecheck needs to identify what type it belongs to -- there may be multiple types with fields by that name, for example. The function looks up the field name in the symbol table and then, using any type information available from the context, tries to identify the appropriate type (or give an error if nothing matches).

However, BSC's symbol table includes information for types that are not visible to the user. This is because the package being compiled might import package A, say, which furthers import package B; the names defined in B will not be visible in the current package, but BSC will need to know that info when it starts elaborating code, because it might pull in code from package B. The way this is done is hackish, though: when package A is imported, the contents of package B are imported qualified; identifiers in imported code are fully qualified, so they will be found in the symbol table, but if user code tries to access the unqualified name, it won't be visible. Unfortunately, it means that the user can access code they didn't import, by using the qualified name. For example, since FloatingPoint imports Vector, the following code will compile:

import FloatingPoint

x :: Vector.Vector 0 Bool
x = Vector.nil

Anyway, going back to findFields: Looking up a field name in the symbol table can result in finding a type that wasn't directly imported, so findFields attempts to report an error in that case. However, the way that it determines if the type was imported is by checking whether the unqualified type is in scope: if not, that means only the qualified type is in scope, which happens for packages that are not directly imported. Unfortunately, this also happens for packages that are imported qualified!

There is a comment at lines 570-572 in TIMonad.hs noting that we might want a better check. It suggests an alternative way to determine visibility: take the qualifier from the type and see if it appears in the import list (which would need to be added to the typechecker's state). But that also seems to hackish to me. It would be better if the symbol table contained that information (some thoughts on that below).

Remember, though, that we got error T0140 with unqualified field names, but we didn't get an error with qualfied field names. This is because there is a guard on the error, defined at line 619. It exempts internally generated code from this check -- for example, BSC inserts implicit register reads. If the ID carries the "internal" property, it is exempted. However, there's a note at line 620 that some internally generated code is not tagged with the "internal" property, and so a workaround is to just exempt any qualified references. So that's why the example with qualified field names works! If that guard were fixed, then those examples would start failing. (This is filed as bug 1858 in Bluespec Inc's internal bug database. It says that, without the exemption, some examples from the testsuite start failing, such as TxRxPntArray.bsv in bsc.bsv_examples/MacTestBench/, bsc.bugs/b1213/, and bsc.driver/depend/GenWrapQualifiedNames.bsv).

The issue with findFields failing for qualified imports could be fixed by added a bit to the symbol table that says whether the type was imported. This can be checked instead of trying to look up the unqualified name. This bit can be set when adding a package to the symbol table. Though, when handling package imports inside package imports, BSC probably just overwrites entries in the symbol table -- if so, we would need to make sure that the info about direct imports is not lost, by merging instead of overwriting. However, this wouldn't fix the fact that users can access qualified names that they didn't directly import. I don't have a good answer for that. Devising a solution probably starts by identifying where this access is needed internally -- which we could find out by not importing sub imports and see what breaks. If we could just keep sub-imports out of the symbol table while we typecheck the user's code, and only add them to the table after, that would prevent them from being visible -- but I don't know if that's feasible.

Test cases struct creation, update, and pattern using qualified and unqualified field names, from a qualified import, are in testsuite/bsc.typechecker/constructors/ (specifically the files Struct*_QualImp_*.bs).