Edit: I think my comment below were unnecessarily long. Simply put, my question is:

For this example code,

import torch
@torch.jit.script
def f1(x):
    # type: (int, int) -> None
    pass

Which error report should be implemented?

"HERE" under python function declaration

RuntimeError:
Number of type annotations (2) did not match the number of function parameters (1):
at __scratch__/example.py:2:0
def f1(x):
~~~~~~~~ <--- HERE
    # type: (int, int) -> None
    pass

"HERE" under type comment annotation

RuntimeError:
Number of type annotations (2) did not match the number of function parameters (1):
at __scratch__/example.py:2:0
def f1(x):
    # type: (int, int) -> None
    ~~~~~~~~~~~~~~~~~~~~~~~~~~ <--- HERE
    pass

Original comment:

Hi @driazati, I was looking around the code of pytorch jit mechanism.
I found that this "type annotation comment" parsing is currently done in a separate parser independent from whole source as in:

pytorch/torch/csrc/jit/script/init.cpp

Lines 878 to 881 in 6a4ca9a

    
           m.def("parse_type_comment", [](const std::string& comment) { 
        
             Parser p(std::make_shared<Source>(comment)); 
        
             return Decl(p.parseTypeComment()); 
        
           });

That's why SourceRange (type_annotation_decl.range()) passed to ErrorReport here

pytorch/torch/csrc/jit/script/parser.cpp

Lines 12 to 28 in 6a4ca9a

    
           Decl mergeTypesFromTypeComment( 
        
               const Decl& decl, 
        
               const Decl& type_annotation_decl, 
        
               bool is_method) { 
        
             auto expected_num_annotations = decl.params().size(); 
        
             if (is_method) { 
        
               // `self` argument 
        
               expected_num_annotations -= 1; 
        
             } 
        
             if (expected_num_annotations != type_annotation_decl.params().size()) { 
        
               throw ErrorReport(type_annotation_decl.range()) 
        
                   << "Number of type annotations (" 
        
                   << type_annotation_decl.params().size() 
        
                   << ") did not match the number of " 
        
                   << (is_method ? "method" : "function") 
        
                   << " parameters (" << expected_num_annotations << ")"; 
        
             }

doesn't include source location information as a part of whole source.

I can think of two ways to approach this problem though.

One quick way to "fix" this is to use throw ErrorReport(decl.range()) instead of throw ErrorReport(type_annotation_decl.range()), which would work like this:

=== Example code ( __scratch__/example.py) ==

import torch
def f1(x):
    # type: (int, int) -> None
    pass
torch.jit.script(f1)



=== Error message ==

RuntimeError: 
Number of type annotations (2) did not match the number of function parameters (1):
at __scratch__/example.py:2:0
def f1(x):
~~~~~~~~ <--- HERE
    # type: (int, int) -> None
    pass

Second way is to properly handle SourceRange for type_annotation_decl, which, I think, requires to implement a better version of parse_type_comment or some way to inject proper SourceRange into Decl afterward. A bit complicated part is that "type annotation comment" extraction is currently handled in python code (torch.jit.annotations.get_type_line)

pytorch/torch/jit/annotations.py

Line 137 in 6a4ca9a

def get_type_line(source):

which doesn't concern with SourceRange concept used in cpp-part of parser/lexer.

Anyways, if you guys are fine with the first way of fixing this, then I can make PR. But if more proper fix is desired, I might not be able to work on this.
Please let me know how you think. Thanks.

Btw, reading pytorch source is so interesting, thank you very much for such a great code base.

	m.def("parse_type_comment", [](const std::string& comment) {
	Parser p(std::make_shared<Source>(comment));
	return Decl(p.parseTypeComment());
	});

	Decl mergeTypesFromTypeComment(
	const Decl& decl,
	const Decl& type_annotation_decl,
	bool is_method) {
	auto expected_num_annotations = decl.params().size();
	if (is_method) {
	// `self` argument
	expected_num_annotations -= 1;
	}
	if (expected_num_annotations != type_annotation_decl.params().size()) {
	throw ErrorReport(type_annotation_decl.range())
	<< "Number of type annotations ("
	<< type_annotation_decl.params().size()
	<< ") did not match the number of "
	<< (is_method ? "method" : "function")
	<< " parameters (" << expected_num_annotations << ")";
	}

[jit] Missing source highlight error #25801

Description

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions