fixes #8944 - Now printing a warning 'Ignoring invalid haddock comment' instead of an error
AbandonedPublic

Authored by rodlogic on Nov 6 2014, 4:02 PM.

Details

Reviewers
hvr
bgamari
austin
Trac Issues
#8944
Summary

This addresses ticket Trac #8944 where the lexer stops with an error on an invalid haddock comment and when the haddock option is set.

See ticket for more details.

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Branch
T8944
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 1767
Build 1774: GHC Patch Validation (amd64/Linux)
rodlogic updated this revision to Diff 1301.Nov 6 2014, 4:02 PM
rodlogic retitled this revision from to fixes #8944 - Now printing a warning 'Ignoring invalid haddock comment' instead of an error.
rodlogic updated this object.
rodlogic edited the test plan for this revision. (Show Details)
rodlogic added a reviewer: hvr.Nov 6 2014, 4:03 PM
rodlogic updated this revision to Diff 1304.Nov 6 2014, 6:29 PM

Added Opt_WarnInvalidComments (warn-invalid-comments) to DynFlags manual and user guide.

rodlogic updated this revision to Diff 1306.Nov 6 2014, 6:43 PM

Updated docs/users_guide/using.xml with new option.

This is still not working correctly and I am running out of ideas considering that my knowledge of Alex is quite thin. The main open question is where should the new rule be added in Lexer.x. I have tried 3 different locations and I still get validate errors.

austin accepted this revision.Nov 7 2014, 1:18 PM
austin updated the Trac tickets for this revision.
austin edited edge metadata.

Nice, LGTM!

This revision is now accepted and ready to land.Nov 7 2014, 1:20 PM
austin requested changes to this revision.Nov 7 2014, 1:22 PM
austin edited edge metadata.

Whoops, I noticed the testsuite failures. We'll have to fix this stuff first.

This revision now requires changes to proceed.Nov 7 2014, 1:22 PM
rodlogic added a comment.EditedNov 7 2014, 1:38 PM

As I mentioned above all the pieces are in place, but the new rule in Parser.x is not working in all cases.

Fuuzetsu added inline comments.Nov 7 2014, 11:47 PM
compiler/main/DynFlags.hs
490

I think the option is unnecessary and it should always be on if running Haddock: it is not fun to start again each time. We would most likely always enable this from Haddock anyway.

EDIT: although I suppose if you already coded the option in then we might as well leave it in, I can see someone wanting to catch badly placed comments without having to scroll through the whole output.

compiler/parser/Lexer.x
215

This isn't a good message, the comment might be perfectly valid Haddock markup. The only problem we encounter here is that a Haddock comment appears at a location GHC is not expecting it in. The message should say something like “Not expecting Haddock comment at <location>, ignoring.” instead.

Huh, sorry, I commented some days ago but apparently you have to scroll down and submit it too.

Basically I'd like the error message to be a bit more meaningful.

Any updates on this? I think I even hoped for it to go into 7.10.1 though if it gets merged in fast then I guess it doesn't change things enough to not be allowed into later 7.10.x release.

Yes, I also hoped for 7.10, but I faced some difficulties in Parser.hs (harder nut to crack than initially appears) and I had little to no time for Haskell/GHC since December.

What is the status here?

@thomie It is pretty much stuck at this point since I had zero time to work on it. I am also sure it is very outdated considering the recent Annotations API work on the parser. It seems best to get rid of this differential.

austin resigned from this revision.Nov 6 2017, 5:21 PM
thomie abandoned this revision.Aug 16 2018, 10:09 PM

Subsumed by D5057.