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

Report bsp diagnostics in end of incremental cycle #782

Merged
merged 1 commit into from
Jan 2, 2019

Conversation

jvican
Copy link
Contributor

@jvican jvican commented Jan 2, 2019

...instead of reporting at the end of the compilation.

This commit intends to report as early as possible in the case where
several expensive incremental cycles are triggered. An example of this
case can be found in scalameta/metals#438

What this PR does is conceptually simple:

  1. At the end of every incremental cycle, process the previous problems
    that comply with any of these requirements:
    • Their associated source doesn't exist (bloop clears diagnostics)
    • Their associated source has already been processed (bloop does
      nothing as problems have already been reported by logFull)
    • Their associated file was compiled but no problem was registered
      (bloop appropriately clears diagnostics)
    • reportAllPreviousProblems is true. This happens in the first
      compilation of a target via BSP and it's only done at the end of the
      compilation, so in every intermediate incremental cycle it's false.
  2. Every problem that is processed will not be processed in the future.
    This bit of logic avoids repeating the handling of reported problems.

This change is already tested by our previous infrastructure.

...instead of reporting at the end of the compilation.

This commit intends to report as early as possible in the case where
several expensive incremental cycles are triggered. An example of this
case can be found in scalameta/metals#438

What this PR does is conceptually simple:

1. At the end of every incremental cycle, process the previous problems
   that comply with any of these requirements:
   * Their associated source doesn't exist (bloop clears diagnostics)
   * Their associated source has already been processed (bloop does
     nothing as problems have already been reported by `logFull`)
   * Their associated file was compiled but no problem was registered
     (bloop appropriately clears diagnostics)
   * `reportAllPreviousProblems` is true. This happens in the first
     compilation of a target via BSP and it's only done at the end of the
     compilation, so in every intermediate incremental cycle it's false.
2. Every problem that is processed will not be processed in the future.
   This bit of logic avoids repeating the handling of reported problems.

This change is already tested by our previous infrastructure.
@jvican jvican added enhancement build server Any issue or pull request that has to do with hot compilers or BSP. task / compile labels Jan 2, 2019
@jvican jvican merged commit fcd59d3 into master Jan 2, 2019
jvican added a commit that referenced this pull request Jan 10, 2019
BSP requires that bloop clears problems that were present in previous
compiler iterations manually. Previously, Bloop would meet this contract
by clearing problems from previous compiler runs at the end of a
successful incremental cycle. For every incremental cycle, bloop would
go through the problems and clear those that were affected by changes
occurred during that cycle. You can see more information in this pull
request: #782

However, this approach was slower than desired and required that we
successfully finished an incremental compiler cycle before *starting* to
clean some diagnostics in the recompiled files.

Instead, this pull request takes a different approach to this problem.
We clear diagnostics as soon as we know they are safe to be cleared. For
example, if in a previous compiler run typer emitted an error, we have
bloop record that a diagnostic happened during that phase and, in
subsequent compiler runs, we clear the diagnostic as soon as we've
passed typer for the source file that contained the error.

That is, we clear diagnostics right after the phase that generated them
succeeded.

If a diagnostic could not be mapped to a phase, we fallback to the
previous behavior: we will clear it in `reportRemainingProblems` right
after every incremental cycle.
jvican added a commit that referenced this pull request Jan 10, 2019
BSP requires that bloop clears problems that were present in previous
compiler iterations manually. Previously, Bloop would meet this contract
by clearing problems from previous compiler runs at the end of a
successful incremental cycle. For every incremental cycle, bloop would
go through the problems and clear those that were affected by changes
occurred during that cycle. You can see more information in this pull
request: #782

However, this approach was slower than desired and required that we
successfully finished an incremental compiler cycle before *starting* to
clean some diagnostics in the recompiled files.

Instead, this pull request takes a different approach to this problem.
We clear diagnostics as soon as we know they are safe to be cleared. For
example, if in a previous compiler run typer emitted an error, we have
bloop record that a diagnostic happened during that phase and, in
subsequent compiler runs, we clear the diagnostic as soon as we've
passed typer for the source file that contained the error.

That is, we clear diagnostics right after the phase that generated them
succeeded.

If a diagnostic could not be mapped to a phase, we fallback to the
previous behavior: we will clear it in `reportRemainingProblems` right
after every incremental cycle.
@tgodzik tgodzik deleted the topic/early-bsp-clear-diagnostics branch September 7, 2021 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build server Any issue or pull request that has to do with hot compilers or BSP. enhancement task / compile
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant