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

Decouple warning detection from program instances #12293

Merged

Conversation

HertzDevil
Copy link
Contributor

@HertzDevil HertzDevil commented Jul 18, 2022

This PR moves all the warning settings and detected warnings into a new Crystal::WarningCollection object. A new Crystal::Compiler or Crystal::Program comes with its own warning collection, but a collection may also be explicitly shared between them, which Compiler#new_program does. The sharing is needed because warnings from either component could trigger --error-on-warnings.

If we want to extend this to syntax warnings (see for example #12197 (comment) and #8373 (comment)), the treatment is also similar:

module Crystal
  class Parser
    property warnings : WarningCollection

    def initialize(..., @warnings = WarningCollection.new)
    end
  end

  class Compiler
    private def parse(program, source : Source)
      parser = Parser.new(source.code, program.string_pool, warnings: @warnings)
      # ...
    end
  end

  class Program
    def run(code, filename = nil, debug = Debug::Default)
      parser = Parser.new(code, warnings: @warnings)
      # ...
    end
  end
end

This gives the parser access to the compiler's warning settings, whereas a stand-alone parser is also able to maintain its own list of warnings without going through a program.

A Crystal::MacroInterpreter is never created alone, so it merely reuses its parent program's warning collection.

@oprypin oprypin self-requested a review August 7, 2022 00:54
@straight-shoota straight-shoota added this to the 1.6.0 milestone Aug 16, 2022
@straight-shoota straight-shoota merged commit 74d77d6 into crystal-lang:master Aug 29, 2022
@HertzDevil HertzDevil deleted the refactor/warning-collection branch August 29, 2022 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants