Fix identifier parsing in hp2ps
ClosedPublic

Authored by Yuras on Aug 25 2015, 11:10 AM.

Details

Summary

Now identifiers can start with a package key, which is a hash, so they
may also start with a digit. Identifiers always appear at the beginning of
a line, and numbers never appear here, soit's safe to allow identifiers to
start with a digit.

Test Plan

concprog002 passes under threaded2_hT way

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
Yuras updated this revision to Diff 3924.Aug 25 2015, 11:10 AM
Yuras retitled this revision from to Fix identifier parsing in hp2ps.
Yuras updated this object.
Yuras edited the test plan for this revision. (Show Details)
Yuras added reviewers: thomie, austin.
Yuras updated the Trac tickets for this revision.
thomie edited edge metadata.Aug 25 2015, 4:41 PM

Thanks for working on this!

A few remarks:

  • Not sure if this fix is still needed, see my comment in Trac #10661.
  • Your commit message is difficult to parse.
  • Why are you introducing a global variable?
Yuras updated this object.Aug 26 2015, 4:38 AM
Yuras edited edge metadata.
Yuras added a comment.Aug 26 2015, 4:46 AM
  • Your commit message is difficult to parse.

Sorry, English is not my native language. Please feel free to correct the summary.

  • Why are you introducing a global variable?

Well, to match the style of the existing code. It uses global variables to maintain parser's state. I'll update the patch to pass startline as an argument to GetHpTok after we agree on the first point (whether the fix is necessary at all.)

Yuras planned changes to this revision.Aug 26 2015, 4:51 AM

I just found that the test case is not suitable for the issue because package key for 'random' package (used in the test case) may change in future, so it will not start with a digit any more. We need a special test case for the issue.

Yuras updated this revision to Diff 3931.Aug 26 2015, 2:28 PM

Don't use a global variable

Yuras edited the test plan for this revision. (Show Details)Aug 26 2015, 2:30 PM
Yuras edited edge metadata.

I didn't find a way to make a test for hp2ps. Probably it doesn't worth it.

thomie accepted this revision.Aug 26 2015, 4:52 PM
thomie edited edge metadata.

LGTM.

Thanks, I think the code is more clear now, after your revision. And yeah, let's skip the test.

bgamari accepted this revision.Aug 27 2015, 5:52 AM
bgamari edited edge metadata.

Indeed, looks reasonable to me.

austin accepted this revision.Aug 28 2015, 5:13 PM
austin edited edge metadata.

LGTM

This revision is now accepted and ready to land.Aug 28 2015, 5:13 PM
austin updated this object.Aug 28 2015, 5:15 PM
austin edited the test plan for this revision. (Show Details)
austin edited edge metadata.
austin updated this object.
This revision was automatically updated to reflect the committed changes.