chapel-lang / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
1.78k stars 420 forks source link

Better error message with `use` of modules with hyphens in their name #12333

Open lydia-duncan opened 5 years ago

lydia-duncan commented 5 years ago

Summary of Problem

I wrote a .chpl file with a hyphen in its name and because I was going quickly, I didn't give it a wrapper module so it used the name of the .chpl file. This caused problems when I tried to use the helper file - it started treating the two parts of the name as separate symbols and so encountered a syntax error.

Possibly related to #9594, but upon discussion with Brad it seems like this is reasonable behavior - we should potentially give a more useful error message, though.

Steps to Reproduce

Source Code: hyphenated-module.chpl

// anything

use-hyphenated.chpl

use hyphenated-module;

Compile command: chpl use-hyphenated.chpl

Associated Future Test(s): test/statements/lydia/use-hyphenated.chpl #12334

Configuration Information

bradcray commented 5 years ago

I think a syntax error (or other, more specialized error message) is an appropriate thing to have happen here. Specifically, I don't think that this case should work as implied by the future's .good file: Since - is the minus operator in Chapel, this program effectively says "use hyphenated minus module".

Another alternative would be to have some sort of special identifier pattern that's only legal in the use statement context, but that seems weird to me (at least, without a compelling precedent). It would also be non-orthogonal in that where I might say M.foo(); for a module M in my code, I couldn't say hyphenated-module.foo(); since that case would definitely need to be interpreted as minus.

In addition to the syntax error / error message for the use itself, we could consider generating a warning for implicitly named modules whose names are not legal identifiers, alerting the user to the identifier that the program will use instead. But in your future if your main file had been named main.chpl, this wouldn't have helped because the file you were compiling had a legal implicit name and the one you were trying to refer didn't appear on the command-line flag and wouldn't have been inferred from the source code due to the syntax error.

lydia-duncan commented 5 years ago

Since - is the minus operator in Chapel, this program effectively says "use hyphenated minus module".

I agree that that is how the compiler is interpreting the program and that in most cases it is reasonable for it to do so. The fact that it doesn't work for useing all potential module names makes me concerned about why we support that as a module name.

It would also be non-orthogonal in that where I might say M.foo(); for a module M in my code, I couldn't say hyphenated-module.foo(); since that case would definitely need to be interpreted as minus.

But having it in the use statement context if we were able to rename the module being used would allow a fully qualified access that wasn't available before. That said, it would be weird to do so without precedent.

In addition to the syntax error / error message for the use itself, we could consider generating a warning for implicitly named modules whose names are not legal identifiers, alerting the user to the identifier that the program will use instead. But in your future if your main file had been named main.chpl, this wouldn't have helped because the file you were compiling had a legal implicit name and the one you were trying to refer didn't appear on the command-line flag and wouldn't have been inferred from the source code due to the syntax error.

That's a good point. If we enable that sort of warning we're going to have to update a lot of tests, though.

Should I update the future to have a .compopts that includes the helper file?

bradcray commented 5 years ago

The fact that it doesn't work for useing all potential module names makes me concerned about why we support that as a module name.

Traditionally, we've said that if you use a filename that isn't a legal identifier and rely on the implicit module naming, then there's nothing we can do to help you; you should either rename the file or name the module. From the spec:

If the resulting name is not a legal Chapel identifier, it cannot be referenced in a use statement.

This still seems like reasonable behavior to me. I don't think we should pick another module name for the user; I also don't think we should require filenames to be legal identifiers; and I'd really prefer not to have special module identifier patterns within the context of use statements. I think we've also said at times that once you're writing production code, you should probably name the module in the source text rather than relying on the implicit name so that your code won't break if you start renaming the file or adding version numbers to the name or the like. And once you've done that, you're back on stable ground again because you have to name it as a legal identifier.

So I mostly consider this an "ain't broke / don't fix" though I could imagine having special cases in the parser to emit something more helpful than syntax error in cases like this, similar to the ones Michael added to help users who try to redefine reserved type identifiers (e.g., "module names in Chapel source code must be legal identifiers").

lydia-duncan commented 5 years ago

I've updated the description based on Brad's thoughts, planning on merging a similar update to the future itself.