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

Specify the filename to use for locations #6718

Closed
vicuna opened this issue Dec 16, 2014 · 13 comments
Closed

Specify the filename to use for locations #6718

vicuna opened this issue Dec 16, 2014 · 13 comments

Comments

@vicuna
Copy link

vicuna commented Dec 16, 2014

Original bug ID: 6718
Reporter: @diml
Status: confirmed (set by @damiendoligez on 2014-12-16T20:08:35Z)
Resolution: open
Priority: normal
Severity: feature
Category: compiler driver
Tags: junior_job
Monitored by: @whitequark @gasche

Bug description

We'd like to be able to specify the filename to use in locations. Basically having some command line argument [-loc-filename fname] that would be the same as adding this at the beginning of the file:

# 1 "fname"

This would replace some hackery used in syntax extensions of the core suite to add prefixes to filenames:

https://github.com/janestreet/herelib/blob/master/lib/pa_here.ml

We do this because ocamlc/ocamlopt is invoked in the sub-directory of the library and we only get the basename in the location. But this hack doesn't set the filename for [assert], [LOC], ... and wouldn't work well with this proposal: #126.

We could of course generate the line directive with a pre-processor but it'd be just another hack and other Makefile-based build system might benefit from this too.

@vicuna
Copy link
Author

vicuna commented Dec 16, 2014

Comment author: @damiendoligez

I'd like to second this feature request. I have a program (tlapm) whose makefiles would benefit.

@vicuna
Copy link
Author

vicuna commented Dec 21, 2014

Comment author: @whitequark

Is this really a good idea? Most languages I know, starting with C, embed the full path to the module into the location. Not only this is much less ambiguous (where does this "src/cset.ml" come from?), but also easier to use by tools. ocamldebug in particular has to rely on a crutch to locate the sources, currently.

@vicuna
Copy link
Author

vicuna commented Dec 22, 2014

Comment author: @gasche

If/when we do this we probably want OCAMLPARAM handling as well.

@vicuna
Copy link
Author

vicuna commented Dec 22, 2014

Comment author: @diml

@whitequark: you can already do that in OCaml, just pass the full filename to ocamlc/ocamlopt instead of the relative one

@gashe: actually it's not obvious to me we want it in OCAMLRUNPARAM. This would be a per-file setting as it would always depend on the filename. Another possibility is to pass a format string.

@vicuna
Copy link
Author

vicuna commented Dec 22, 2014

Comment author: @whitequark

@dim, my point is that I think ocamlc should always expand the path, fixing this issue and others. Alternatively it could be an OCAMLPARAM setting (and possibly also a command-line switch) that forces path expansion, if the change would be too invasive otherwise--I think it's a much better solution than providing the ability to change the filename in arbitrary ways.

@vicuna
Copy link
Author

vicuna commented Dec 22, 2014

Comment author: @diml

Having the full path is indeed good for local development/debugging but it shouldn't be the default. For instance it is often wrong for binary distribution. Having it as an OCAMLPARAM setting seems good, but it doesn't supersede this feature. This is really for projects that don't run all tools from their root directory.

@vicuna
Copy link
Author

vicuna commented Dec 22, 2014

Comment author: @whitequark

The current behavior is also incorrect for binary distribution (or anything really), because it does not allow to unambiguously locate the source. Ideally, the path would include the package name and then the path from the root of the package.

Hm, and your suggestion actually handles that. Perhaps it is a good idea after all.

@github-actions
Copy link

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label May 11, 2020
@whitequark
Copy link
Member

Still an issue.

@nojb
Copy link
Contributor

nojb commented May 11, 2020

cc @jeremiedimino

@ghost
Copy link

ghost commented May 12, 2020

I guess that'd still be useful, though I ran out of steam on this one. Additionally, since Dune run compilation commands from the root of the workspace it is less of an issue for Dune projects and so this seems less high priority to me.

Unless someone wants to take ownership of this, I'll leave the stable label and let the issue get closed.

@ghost ghost removed the newcomer-job label May 12, 2020
@ghost
Copy link

ghost commented May 12, 2020

I removed the newcomer-job tag though because it's not that simple.

@github-actions github-actions bot removed the Stale label Jun 15, 2020
@github-actions
Copy link

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label Jun 16, 2021
@ghost ghost closed this as completed Jun 16, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants