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 parse error in constrained-type-missing-check.rs #1040

Closed
wants to merge 1 commit into from

Conversation

mbrubeck
Copy link
Contributor

I noticed that this test was expected to succeed, but was actually failing to compile with "expecting *, found low".

There's a deeper issue here, which is that "make test" did not catch this problem. I'm guessing this because "// xfail-test" is not processed when parsing fails.

@mbrubeck
Copy link
Contributor Author

brson explained on IRC that xfail-test will cause the test to be skipped, so it won't raise a flag if the test starts unexpectedly passing.

@mbrubeck
Copy link
Contributor Author

Just to clarify, this test is designed to fail compilation because of a constrained type violation, but currently expected to compiled successfully because constrained type checking is not yet implemented.

The test currently is failing compilation because of a parse error. This patch fixes the parse error, making the test successfully compile. When constrained type checking is implemented, the test will once again fail to compile, and the xfail can then be removed.

@brson
Copy link
Contributor

brson commented Oct 17, 2011

Thank you for the patch, but I am not inclined to pull it.

As far as I know the syntax in the test is correct. This test just fails in the wrong way because the feature is unimplemented.

To really fix this we need to adjust the AST to understand constrained types, fix the parser to accept this syntax then teach the typestate pass to handle constrained types.

@brson brson closed this Oct 17, 2011
@mbrubeck
Copy link
Contributor Author

No, I don't think that the syntax in this test is correct. It doesn't match the syntax in src/test/run-pass/constrained-type.rs or in the commented-out parts of src/lib/util.rs.

It looks like the file had the correct syntax when it was first added in bd4aeef but it got mangled by the pretty-printer in df7f21d. Then 6e2a7bf fixed some of the pretty-printer damage but not all; it looks like it missed some of the damage in this file (but fixed similar damage in src/test/run-pass/constrained-type.rs). Revision cc2ebbe fixed the pretty-printer, but it came too late to prevent the damage to this file.

(I know this doesn't really matter since the feature doesn't work, but I'm just starting to look through the Rust code and this was one bit that I noticed was wrong.)

@brson brson reopened this Oct 18, 2011
@brson
Copy link
Contributor

brson commented Oct 18, 2011

You are correct. Integrated. Thanks!

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