bs classic: struct update syntax of struct from import qualified gives error for the wrong file.

[rossc719g]

For example:

package MakeNegative where

import qualified FloatingPoint

makeNegative :: FloatingPoint.Half -> FloatingPoint.Half
makeNegative x = x {
  FloatingPoint.sign = True
}

Gives an error message that implies that FloatingPoint is not imported into itself:

Error: "FloatingPoint.bsv", line 66, column 16: (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::*;'

Doing it in the longer, fully spelled out way, works fine:

makeNegative_ :: FloatingPoint.Half -> FloatingPoint.Half
makeNegative_ x = FloatingPoint.Half {
  FloatingPoint.sign = True;
  FloatingPoint.exp = x.FloatingPoint.exp;
  FloatingPoint.sfd = x.FloatingPoint.sfd;
}

And removing the qualified from the import also works (wether or not you remove the FloatingPoint. qualifiers).

Ideally:

makeNegative :: FloatingPoint.Half -> FloatingPoint.Half
makeNegative x = x {
  FloatingPoint.sign = True
}

Should work... but at the very least, the error message should reference the source file, not the imported package.

[quark17]

There are two issues here. One is in the desugaring of struct-update expressions int struct-creation expressions. And the other is in the error message for struct-creation using an unqualified name, when the package is imported qualified. For example, if you wrote this:

makeNegative x = FloatingPoint.Half { sign = True }

Then you get the same error:

Error: "MakeNegative.bs", line 6, column 38: (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::*;'

The desugaring is producing a struct-creation function with unqualified field names. That is, this:

x { sign = True }

becomes this:

FloatingPoint { sign = True; exp = x.exp; sfd = x.sfd }

This occurs during typecheck and is due to the definition of tiExpr (in TCheck.hs) for CStructUpd.

The code for this has the field IDs that the user wrote (with the positions that the user wrote them) and it has the field IDs that it looked up for the type (with the position of where they are defined, in this case in the library). When creating the new expression, the looked up IDs are used, which is why the position is bad.

The looked-up IDs are qualified, and the user generally writes unqualified names, so the code does need to account for this when it is comparing IDs between the two. The way it does this, though, is just to unqualify the looked-up IDs -- but it doesn't unqualify the user's IDs. This creates a further bug where the desugaring fails to include the user's updated value (because the name didn't match). And, in fact, because the fields are unqualified, the code can fail to notice when the user gave a bogus qualifier (on fields after the first one). This code is accepted (when not importing qualified):

makeNegative x = x { sign = True; Foo.exp = 0 }

I'll will look at fixing all of this in the desugaring. The error message when you write a struct-creation using unqualified names is still misleading, but that is a separate issue, and I haven't looked into why that's happening. But I can see about getting to that after the desugaring.

[quark17]

The reason why failing to qualify the field name (in either a struct update or struct creation expression) results in an error message that the type wasn't imported is explained by the comment at lines 570-572 in TIMonad.hs. That comment is in a function that returns a tuple of information about a type, including whether the type is visible to the user. Unfortunately, the way that it computes whether the type is visible is to see if the unqualified name is in the symbol table. This doesn't work for packages that were imported qualified.

The comment 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). That seems to hackish. I suspect that the appropriate thing to do is to add a field to the symbol table that says whether the name is visible; when building the symbol table, it would get set appropriately. This would better handle selective importing etc. It might mean, though, that when re-adding a package to the symbol table (because it was imported by an import), we couldn't just overwrite the existing field, we'd have to merge the entries, to preserve that bit of info.

I have a PR that fixes the desugaring, but I don't think I'll tackle the error message. I'll probably open another issue for that, and close this one.