Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only generate a mapping at a generated location for the most specific mapping #61

Open
fitzgen opened this issue Apr 3, 2013 · 9 comments
Labels
feat New feature spec

Comments

@fitzgen
Copy link
Contributor

fitzgen commented Apr 3, 2013

If we have the following mappings, should we generate them all in the source map or only the most specific mapping?

map.addMapping({
  generated: { line: 3, column: 5 }
});
map.addMapping({
  generated: { line: 3, column: 5 },
  original: { line: 6, column: 7 },
  source: 'foo.js'
});
map.addMapping({
  generated: { line: 3, column: 5 },
  original: { line: 6, column: 7 },
  source: 'foo.js',
  name: 'bar'
});

What should be the behavior if there is different original locations for the same generated position? Should we still use the most specific/detailed mapping and ignore the others?

@fitzgen
Copy link
Contributor Author

fitzgen commented Apr 3, 2013

@mishoo, @michaelficarra opinions?

@michaelficarra
Copy link
Contributor

I saw this question when it was first posed in the PR. It's a tough call, but it seems like a good strategy to use the most specific mapping.

@sokra
Copy link
Contributor

sokra commented Apr 3, 2013

I think it makes no sense to map a generated location to multiple things... I vote for keeping only the most specific mapping.


Here is another problem:

map.addMapping({
  generated: { line: 3, column: 5 },
  original: { line: 6, column: 7 },
  source: 'foo.js',
  name: 'bar'
});
map.addMapping({
  generated: { line: 3, column: 5 },
  original: { line: 8, column: 9 },
  source: 'bar.js',
  name: 'foo'
});

Keep both, keep only one, or throw an exception?

@michaelficarra
Copy link
Contributor

Exception!

@fitzgen
Copy link
Contributor Author

fitzgen commented Apr 3, 2013

Yeah that was what I was getting at it with my blurb below the code example.

I think it makes sense to only keep the most specific mapping, if each mapping just adds more info and doesn't contradict previous mappings.

In the case of contradictions, I think we need to throw an exception.

@mishoo
Copy link
Contributor

mishoo commented Apr 4, 2013

Sounds good to me.

@usrbincc
Copy link
Contributor

usrbincc commented Apr 4, 2013

This might be a pathological case, but I thought I'd throw it out there for consideration.

cat > paste.c <<"END"
#define FOO foo
#define BAR bar

#define PASTE_(a, b) a ## b
#define PASTE(a, b) PASTE_(a, b)

PASTE(FOO, BAR) = 42;
END
cc -E paste.c
# foobar = 42;

To fully describe what the token foobar maps to in the original source, you need multiple locations. Note that no C compiler I know of gives this level of debug information (unfortunately; there are some complicated macros out there). So it's not like mapping to multiple locations has a concrete use case at present.

Maybe less pathological: generated lookup tables and generated constants. Most of the time, you don't want to jump to the generation code every time you debug, but occasionally you might want to. Again, kind of a specialized use case.

But as long as the underlying sourcemap format continues to support multiple locations, I don't think there's any reason to complicate source-map with this for now. So throwing an exception sounds good to me.

@jkrems
Copy link
Collaborator

jkrems commented Apr 18, 2021

Since there's been multiple releases with the current behavior, I'll assume that this question is effectively answered.

@jkrems jkrems closed this as completed Apr 18, 2021
@jkrems jkrems reopened this Apr 18, 2021
@jkrems
Copy link
Collaborator

jkrems commented Apr 18, 2021

Re-opening since the issue seems to come up in other reports like #242.

@jkrems jkrems added feat New feature spec labels Apr 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature spec
Projects
None yet
Development

No branches or pull requests

6 participants