0005142: wrong error locations and ghost locations

(0005848)
till (reporter)
2011-03-17 23:00

The problem seems to be in the code that merges the locations. A simpler test of appears to be the same bug would be:

tmp> cat t.ml
type t = int
tmp> camlp4o t.ml -add_locations
(*loc: ["t.ml": 1:0-12 1:12]*)
type (*loc: ["t.ml": 1:5-12 1:12]*)
  t =
  (*loc: ["ghost-location": 1:9-0 1:0 (ghost)]*)
  (*loc: ["ghost-location": 1:9-0 1:0 (ghost)]*)
  (*loc: ["ghost-location": 1:9-0 1:0 (ghost)]*)
  int

The attached patch should fix this.

(0005849)
ygrek (reporter)
2011-03-18 11:04

Hm, am I the only one to think that { (b) with stop = b.stop } is waste of characters (whatever b is)?

(0005850)
till (reporter)
2011-03-18 12:10

Indeed. That's because this line should have been:
{ (b) with start = a.start }
That's a bit embarrassing for a patch this small. Uploaded a corrected patch.

(0005863)
till (reporter)
2011-04-05 08:58

It turns out that keeping the ghost=True in the resulting location is also important (although I couldn't really figure out where it was used in camlp4). Hopefully that's the last regression on that bug fix. I'll upload a patch with the bootstrapping soon.

(0005866)
till (reporter)
2011-04-07 21:39

Seems fixed in the 3.12 svn (at least for my problem cases).

(0008111)
doligez (manager)
2012-09-19 14:05

The second problem is fixed in 3.12.1 and the first one in 4.00.0.

Issue History
Date Modified	Username	Field	Change
2010-09-03 11:05	Hendrik Tews	New Issue
2010-09-03 11:05	Hendrik Tews	Status	new => assigned
2010-09-03 11:05	Hendrik Tews	Assigned To	=> ertai
2011-03-17 22:57	till	File Added: camlp4_loc_merge_fix.patch
2011-03-17 23:00	till	Note Added: 0005848
2011-03-18 11:04	ygrek	Note Added: 0005849
2011-03-18 12:10	till	Note Added: 0005850
2011-03-18 12:10	till	File Added: camlp4_loc_merge_fix2.patch
2011-04-05 08:58	till	Note Added: 0005863
2011-04-05 15:54	till	File Added: camlp4_loc_merge_fix3.patch
2011-04-07 21:39	till	Note Added: 0005866
2011-10-26 08:41	ertai	Assigned To	ertai => xclerc
2012-07-04 13:30	doligez	Relationship added	related to 0005090
2012-07-04 17:14	doligez	Target Version	=> 4.00.0+dev
2012-07-11 11:36	doligez	Target Version	4.00.0+dev => 4.01.0+dev
2012-07-31 13:36	doligez	Target Version	4.01.0+dev => 4.00.1+dev
2012-09-19 14:05	doligez	Note Added: 0008111
2012-09-19 14:05	doligez	Status	assigned => closed
2012-09-19 14:05	doligez	Resolution	open => fixed

Notes
(0005848) till (reporter) 2011-03-17 23:00	The problem seems to be in the code that merges the locations. A simpler test of appears to be the same bug would be: tmp> cat t.ml type t = int tmp> camlp4o t.ml -add_locations (loc: ["t.ml": 1:0-12 1:12]) type (loc: ["t.ml": 1:5-12 1:12]) t = (loc: ["ghost-location": 1:9-0 1:0 (ghost)]) (loc: ["ghost-location": 1:9-0 1:0 (ghost)]) (loc: ["ghost-location": 1:9-0 1:0 (ghost)]) int The attached patch should fix this.

(0005849) ygrek (reporter) 2011-03-18 11:04	Hm, am I the only one to think that { (b) with stop = b.stop } is waste of characters (whatever b is)?

(0005850) till (reporter) 2011-03-18 12:10	Indeed. That's because this line should have been: { (b) with start = a.start } That's a bit embarrassing for a patch this small. Uploaded a corrected patch.

(0005863) till (reporter) 2011-04-05 08:58	It turns out that keeping the ghost=True in the resulting location is also important (although I couldn't really figure out where it was used in camlp4). Hopefully that's the last regression on that bug fix. I'll upload a patch with the bootstrapping soon.

(0005866) till (reporter) 2011-04-07 21:39	Seems fixed in the 3.12 svn (at least for my problem cases).

(0008111) doligez (manager) 2012-09-19 14:05	The second problem is fixed in 3.12.1 and the first one in 4.00.0.