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

ast, checker, fmt: fix compiler internal formatting failed #18356

Merged
merged 1 commit into from Jun 6, 2023

Conversation

yuyi98
Copy link
Member

@yuyi98 yuyi98 commented Jun 6, 2023

This PR fix compiler internal formatting failed.

error:

PS D:\Vlang\v\vlib\v\checker> v fmt -w check_types.v
cannot compile `D:\Vlang\v\cmd\tools\vfmt.v`: 
D:/Vlang/v/vlib/v/ast/cflags.v:30:36: error: undefined ident: `v.checker.constants.valid_comptime_not_user_defined`
   28 |     mut fos := ''
   29 |     mut allowed_os_overrides := []string{}
   30 |     allowed_os_overrides << constants.valid_comptime_not_user_defined
      |                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   31 |     allowed_os_overrides << ctimedefines
   32 |     for os_override in allowed_os_overrides {
D:/Vlang/v/vlib/v/fmt/fmt.v:2012:46: error: undefined ident: `v.checker.constants.valid_comptime_not_user_defined`
 2010 | pub fn (mut f Fmt) ident(node ast.Ident) {
 2011 |     if node.info is ast.IdentVar {
 2012 |         if node.comptime && node.name in constants.valid_comptime_not_user_defined {
      |                                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2013 |             f.write(node.name)
 2014 |             return

solution:

  • Move comptime_valid_idents from v.checker.constants to ast.

@spytheman
Copy link
Member

hm, that seems to be OS specific 🤔; on Linux, on master:

#0 15:49:02 ᛋ master /v/vnew/vlib/v/checker❱v fmt -w check_types.v
Already formatted file: /v/vnew/vlib/v/checker/check_types.v
#0 15:49:05 ᛋ master /v/vnew/vlib/v/checker❱

@spytheman
Copy link
Member

If I do v cmd\tools\vfmt.v at the top level after v up, after that v fmt -w check_types.v works inside the vlib\v\checker folder as well, on windows too.

@spytheman
Copy link
Member

I could reproduce the problem on linux as well, after v self or v up, since that will force v to recompile the vfmt executable:

#0 15:59:23 ᛋ master /v/vnew❱v self
V self compiling ...
V built successfully as executable "v".
#0 15:59:27 ᛋ master /v/vnew❱
#0 15:59:28 ᛋ master /v/vnew❱v fmt -w vlib/v/checker/check_types.v 
Already formatted file: /v/vnew/vlib/v/checker/check_types.v
#0 15:59:32 ᛋ master /v/vnew❱
#0 15:59:33 ᛋ master /v/vnew❱cd vlib/v/checker
#0 15:59:38 ᛋ master /v/vnew/vlib/v/checker❱v fmt -w check_types.v 
Already formatted file: /v/vnew/vlib/v/checker/check_types.v
#0 15:59:43 ᛋ master /v/vnew/vlib/v/checker❱
#0 15:59:44 ᛋ master /v/vnew/vlib/v/checker❱v self
V self compiling ...
V built successfully as executable "v".
#0 15:59:49 ᛋ master /v/vnew/vlib/v/checker❱
#0 15:59:50 ᛋ master /v/vnew/vlib/v/checker❱v fmt -w check_types.v 
cannot compile `/v/vnew/cmd/tools/vfmt.v`: 
/v/vnew/vlib/v/ast/cflags.v:30:36: error: undefined ident: `v.checker.constants.valid_comptime_not_user_defined`
   28 |     mut fos := ''
   29 |     mut allowed_os_overrides := []string{}
   30 |     allowed_os_overrides << constants.valid_comptime_not_user_defined
      |                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   31 |     allowed_os_overrides << ctimedefines
   32 |     for os_override in allowed_os_overrides {
/v/vnew/vlib/v/fmt/fmt.v:2012:46: error: undefined ident: `v.checker.constants.valid_comptime_not_user_defined`
 2010 | pub fn (mut f Fmt) ident(node ast.Ident) {
 2011 |     if node.info is ast.IdentVar {
 2012 |         if node.comptime && node.name in constants.valid_comptime_not_user_defined {
      |                                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2013 |             f.write(node.name)
 2014 |             return

#1 16:00:14 ᛋ master /v/vnew/vlib/v/checker❱

i.e. the problem is not directly related to vfmt, but to the fact that V's module lookup is dependent on the current working folder, and v cmd/tools/vfmt.v works when executed at the top of the V project, but v ../../../cmd/tools/vfmt.v does not work, inside the vlib/v/checker folder, so vfmt can not be recompiled when the v fmt -w check_types.v is launched from there.

Copy link
Member

@spytheman spytheman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that moving those constants to v.ast, is a good idea regardless of the vfmt problem.
Good work.

@spytheman spytheman merged commit f45fc45 into vlang:master Jun 6, 2023
49 checks passed
@yuyi98 yuyi98 deleted the fix_internal_fmt_error branch June 6, 2023 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants