- 
                Notifications
    You must be signed in to change notification settings 
- Fork 471
Fix regex literal loc #7683
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
Fix regex literal loc #7683
Conversation
b21f017    to
    3dbbb44      
    Compare
  
    | rescript
 @rescript/darwin-arm64
 @rescript/darwin-x64
 @rescript/linux-arm64
 @rescript/linux-x64
 @rescript/win32-x64
 commit:  | 
        
          
                compiler/syntax/src/res_core.ml
              
                Outdated
          
        
      | let loc = | ||
| mk_loc | ||
| { | ||
| start_pos with | ||
| (* Account for the inserted leading `/` *) | ||
| pos_cnum = start_pos.pos_cnum - 1; | ||
| } | ||
| p.prev_end_pos | ||
| in | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it work if you pass in the start_pos from the caller function in here?
rescript/compiler/syntax/src/res_core.ml
Line 1836 in 249cb40
| let start_pos = p.Parser.start_pos in | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably! I'll try it when I'm back at the computer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shulhi yup, much better. Thank you!
172bff5    to
    1cb5bf3      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
1cb5bf3    to
    ee43d01      
    Compare
  
    
This fixes the regex literal locations, so error messages work properly.
No idea if this is the correct fix though. Maybe @shulhi or @cristianoc has opinions?
Closes #7682