search

haskell / alex

A lexical analyser generator for Haskell
https://hackage.haskell.org/package/alex
BSD 3-Clause "New" or "Revised" License
298 stars 82 forks source link

Pedantic stack build fails due to code of wrappers.hs #103

Closed pjljvandelaar closed 7 years ago

pjljvandelaar commented 7 years ago

I would like to build my software using stack with the --pedantic flag. When using the LTS 7.14, the wrappers.hs file yields unfortunately:

C:..\templates\wrappers.hs:67:25: warning: [-Wunused-matches] Defined but not used: `ps'

C:..\templates\wrappers.hs:70:20: warning: [-Wunused-matches] Defined but not used: `p'

C:..\templates\wrappers.hs:70:24: warning: [-Wunused-matches] Defined but not used: `bs'

C:..\templates\wrappers.hs:70:27: warning: [-Wunused-matches] Defined but not used: `s'

C:..\templates\wrappers.hs:74:14: warning: [-Wunused-matches] Defined but not used: `p'

C:..\templates\wrappers.hs:74:16: warning: [-Wunused-matches] Defined but not used: `c'

C:..\templates\wrappers.hs:157:22: warning: [-Wunused-matches] Defined but not used: `c'

C:..\templates\wrappers.hs:447:25: warning: [-Wname-shadowing] This binding for `str' shadows the existing binding bound at templates\wrappers.hs:446:16

C:..\templates\wrappers.hs:451:32: warning: [-Wunused-matches] Defined but not used: `len'

It seems that the latest code at https://github.com/simonmar/alex/blob/master/templates/wrappers.hs addresses the unused-matches resulting in "Defined but not used" warnings. However, the name-shadowing is still present!

Could the alex code be made such that pedantic builds succeed?

Thanks in advance!

erikd commented 7 years ago

For those of use who don't use stack, what GHC version is included in "LTS 7.14"?

Update: You also don't mention what version of alex. I fixed a number of issues like this about 3 months ago.

simonmar commented 7 years ago

I'm not a fan of -Wname-shadowing and I normally turn it off. What exactly does --pedantic do, and when do you encounter these errors? When building Alex itself, or something that uses Alex?

pjljvandelaar commented 7 years ago

LTS 7.14 uses GHC 8.0.1 and alex-3.1.7 (see https://www.stackage.org/lts-7.14) pedantic: turn on all warnings and errors (in ghc and all other programs used to build)

I am building my own Name.x files. Yet, their compilation uses some template files of Alex that cause the warning (that is considered an error due to pedantic)!

Checking for name-shadowing can prevent unexpected behavior. We once had an issue with a variable called map and the map function.

erikd commented 7 years ago

@pjljvandelaar Alex is currently at version 3.2.1 and I think at least some of those warnings have been fixed.

@simonmar I got caught a couple of times in C code with name shadowing and started passing -Wshadow to gcc well over a decade ago. When took up Haskell I was very pleased to notice that the name shadowing warning is part of -Wall so I don't have to remember to set it. For things like the Alex template files, it would be a good idea for them to be as warning clean as possible so that the Alex user can turning on as many warnings as they feel comfortable with and see (hopefully) zero warnings from the template code.

pjljvandelaar commented 7 years ago

I agree with the fact that some warnings have been fixed (as was already mentioned in the first message)

I also agree with having the alex template files as warning clean as possible! Solving name-shadowing is relatively easy. We often just append the name with ' to make it unique.

erikd commented 7 years ago

I found that the tests are currently building with -fno-warn-name-shadowing so that these warnings are not being triggered at during cabal test. Working on fixing that.

simonmar commented 7 years ago

I'm well aware that accidental shadowing can cause unexpected bugs. But deliberate shadowing can also be used to prevent bugs, which is why I personally prefer to leave the warning turned off. That and the fact that I often like to use id as a variable name :)

But I'm not against avoiding warnings in template code, by all means go ahead.

PierreVDL commented 7 years ago

@erikd In which version of alex will this bug be fixed?

pjljvandelaar commented 7 years ago

Problem keeps returning:

templates\wrappers.hs:448:25: warning: [-Wname-shadowing]
    This binding for `str' shadows the existing binding
      bound at templates\wrappers.hs:447:16

Please fix the template code !

simonmar commented 7 years ago

I'm not sure how sustainable this is, but I've fixed the shadowing warnings.

pjljvandelaar commented 7 years ago

You can enforce it by using the flags -Wall -Werror on your code or You can add tags for the Haskell compiler to your templates such that the compiler doesn't complain.

pjljvandelaar commented 7 years ago

Thanks!

simonmar commented 7 years ago

@pjljvandelaar I know about the flags :) Indeed the tests are run with -Wall -Werror, and I just removed -fno-warn-name-shadowing, so this won't break again.

The reason I say it isn't sustainable is that Alex appends some template code to the module, so there will inevitably be name clashes. There's no way I can pick a local name in the Alex template code that is guaranteed not to shadow anything in the user code. I've made the names unlikely to clash, but there's no guarantee.