cc @pablogsal @ambv

@picnixz

Or we could check whether the reported traceback is correct or not (or we could hook compile itself?)

Traceback seems like good place to check for it instead of linecache, but I did not find any reference to _pyrepl.__main__ in FrameSummary (maybe it is somewhere else?). Filename there is still non-existing-file in this case. And then it passes it to linecache.getline. And from there it falls back to _py_repl.__main__ after failing to find file. FTR:

So I would need someone to point me in the right direction where it should be handled as I don't have much context here.

I'm sorry, I wasn't explicit enough. What I meant is that we should fix _pyrepl/* and not linecache or traceback itself. Those are general-purpose utilities that should not depend on the REPL itself. The fact that the code for _pyrepl.__main__ is shown is because we're essentially executing everything here (not sure anymore). So, we need to patch the REPL files, not the other files (linecache and traceback are technically correct; it's the fact that we don't set a filename for compile that becomes problematic).

Btw, I think we have an existing issue for this one but I don't remember which one it is...

I will try to get more context and see how it can be fixed better if that's okay.

The current state of things is already due to an attempt to hide _pyrepl internals. I'm not sure there's an easy way to change _pyrepl/* in a way that linecache won't cache it without a change to linecache itself.

I disagree that it's somehow invalid to patch linecache for this. What you're trying to do here is exactly to make linecache special-case _pyrepl/*, so that would be the easiest thing to do here.

I'm not sure there's an easy way to change _pyrepl/* in a way that linecache won't cache it without a change to linecache itself

Wouldn't it be possible with some custom __loader__ in _pyrepl.__main__ for which get_source() returns None? (not sure if it could cause other issues though)

I disagree that it's somehow invalid to patch linecache for this. What you're trying to do here is exactly to make linecache special-case _pyrepl/*, so that would be the easiest thing to do here.

What I meant is that I'm more worried about the fact that we have some hardcoded _pyrepl.__main__ guard here. If you however think it's the easiest way to do it maybe this can motivate that choice but... it feels a bit hacky =/ OTOH, if the repl has special cases elsewhere in the stdlib, then maybe the change is fine. But if possible, it would be nicer if we could make it work in a more generic way (though, if this properly fixes the issue, it could be fine).

Hm, __loader__ might be a good idea to pursue. I was worried about putting any logic in _pyrepl.__main__, as anything you put there will end up in globals of the interactive shell. But since there is a default __loader__ there already, maybe this can work.

By adding __spec__ = __loader__ = None to _pyrepl/__main__.py, I have the following output:

>>> eval(compile("val", "missing", "eval"))
Traceback (most recent call last):
  File "<python-input-0>", line 1, in <module>
    eval(compile("val", "missing", "eval"))
    ~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "missing", line 1, in <module>
NameError: name 'val' is not defined. Did you mean: 'eval'?
>>>

So it appears to work. In linecache this would do the following:

    # Try for a __loader__, if available
    if module_globals and '__name__' in module_globals:  # <- ok 
        spec = module_globals.get('__spec__')  # will be explicitly None
        name = getattr(spec, 'name', None) or module_globals['__name__']
        loader = getattr(spec, 'loader', None)  # will be None
        if loader is None:
            loader = module_globals.get('__loader__')  # will be None
        get_source = getattr(loader, 'get_source', None)  # will be None

        if name and get_source:  # will be skipped
            def get_lines(name=name, *args, **kwargs):
                return get_source(name, *args, **kwargs)
            cache[filename] = (get_lines,)
            return True
    return False  # and we get here

Doesn't it break other things somewhere?

Doesn't it break other things somewhere?

I don't think so because we're only changing the loader and spec of a specific module. But we should investigate (I don't have time now and I'm leaving for 2 weeks tomorrow)

Benedkit's fix does not work for me.

The error does not occur when using a "<foo>" placeholder

>>> eval(compile("val", "<string>", "eval"))
Traceback (most recent call last):
  File "<python-input-0>", line 1, in <module>
    eval(compile("val", "<string>", "eval"))
    ~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<string>", line 1, in <module>
NameError: name 'val' is not defined. Did you mean: 'eval'?

I don't understand the problem. My fix is to avoid jumping to the pyrepl main. In this case <string> is still not an existing file. Should it output something else?

Note: traceback or linecache treats <...> locations differently but I don't remember where.

Ok: the reason why <...> placeholders don't exhibit the bug is because they are treated specially by linecache:

I don't think this is just a repl issue, this is just that when the file does not exist it returns the first line of the file being run, why on earth is it implemented this way?

That's how tracebacks are constructed. The problem is just that we should actually skip that file when reporting the traceback, namely, making it "inexisting" from a summary PoV. In addition, it's probably because we're actually using compile and exec. Finally, this issue does not occur in a pure Python script.

I see, you should open a PR with your fix then :-)

@picnixz thanks for writing the fix as I did not have time to do it myself. We can close my PR, right?

I think we can, because I don't think we'll go that way at least.