Each of Relations implementation should have specific `toString`
implementation. For example, only `MRelationsNameHashing` implementation
should be printing used names in toString representation.
A hash for given name in a source file is computed by combining
hashes of all definitions with given name. When hashing a single
definition we take into account all information about it except nested
definitions. For example, if we have following definition
class Foo[T] {
def bar(x: Int): Int = ???
}
hash sum for `Foo` will include the fact that we have a class with
a single type parameter but it won't include hash sum of `bar` method.
Computed hash sums are location-sensitive. Each definition is hashed along
with its location so we properly detect cases when definition's signature
stays the same but it's moved around in the same compilation unit.
The location is defined as sequence of selections. Each selection consists
of a name and name type. The name type is either term name or type name.
Scala specification (9.2) guarantees that each publicly visible definition
is uniquely identified by a sequence of such selectors.
For example, if we have:
object Foo {
class Bar { def abc: Int }
}
then location of `abc` is Seq((TermName, Foo), (TypeName, Bar))
It's worth mentioning that we track name-hash pairs separately for
regular (non implicit) and implicit members. That's required for name
hashing algorithm because it does not apply its heuristic when implicit
members are being modified.
Another important characteristic is that we include all inherited members
when computing name hashes.
Here comes the detailed list of changes made in this commit:
* HashAPI has new parameter `includeDefinitions` that allows
shallow hashing of Structures (where we do not compute hashes
recursively)
* HashAPI exposes `finalizeHash` method that allow one to capture
current hash at any time. This is useful if you want to hash a list of
definitions and not just whole `SourceAPI`.
* NameHashing implements actual extraction of public definitions,
grouping them by simple name and computing hash sums for each group
using HashAPI
* `Source` class (defined in interface/other file) has been extended to
include `_internalOnly_nameHashes` field. This field stores
NameHashes data structure for given source file. The NameHashes
stores two separate collections of name-hash pairs for regular and
implicit members.
The prefix `_internalOnly_` is used to indicate that this is not an
official incremental compiler's or sbt's API and it's for use by
incremental compiler internals only. We had to use such a prefix
because the `datatype` code generator doesn't support emitting access
modifiers
* `AnalysisCallback` implementation has been modified to gather all
name hashes and store them in the Source object
* TestCaseGenerators has been modified to implement generation of
NameHashes
* The NameHashingSpecification contains a few unit tests that make sure
that the basic functionality works properly
Tracking of used names is a component needed by the name hashing
algorithm. The extraction and storage of used names is active only when
`AnalysisCallback.nameHashing` flag is enabled and it's disabled by
default.
This change constists of two parts:
1. Modification of Relations to include a new `names` relation
that allows us to track used names in Scala source files
2. Implementation of logic that extracts used names from Scala
compilation units (that correspond to Scala source files)
The first part is straightforward: add standard set of methods in
Relations (along with their implementation) and update the logic which
serializes and deserializes Relations.
The second part is implemented as tree walk that collects all symbols
associated with trees. For each symbol we extract a simple, decoded name
and add it to a set of extracted names. Check documentation of
`ExtractUsedNames` for discussion of implementation details.
The `ExtractUsedNames` comes with unit tests grouped in
`ExtractUsedNamesSpecification`. Check that class for details.
Given the fact that we fork while running tests in `compiler-interface`
subproject and tests are ran in parallel which involves allocating
multiple Scala compiler instances we had to bump the default memory limit.
This commit contains fixes for gkossakowski/sbt#3, gkossakowski/sbt#5 and
gkossakowski/sbt#6 issues.
We introduced some new members (related to name hashing) with an intent
to not expose them as public API. However, I missed some modifiers and
some members (like `memberRef` and `inheritance`) are public.
This commit fixes access modifiers to agree with the intent.
The previous name of the flag was rather specific: it indicated
whether the new source dependency tracking is supported by given Relations
object. However, there will be more functionality added to Relations that
is specific to name hashing algorithm. Therefore it makes sense to name
the flag as just `nameHashing`.
I decided to rename Relations implementation classes to be more
consistent with the name of the flag and with the purpose they serve.
The flag in AnalysisCallback (and classes implementing it) has been
renamed as well.
Adding source dependency on itself doesn't really bring any value so
there's no reason to do it. We avoided recording that kind of dependencies
by performing a check in `AnalysisCallback` implementation. However, if we
have another implementation like `TestCallback` used for testing we do
not benefit from that check.
Therefore, the check has been moved to dependency phase were dependencies
are collected.
Previously incremental compiler was extracting source code
dependencies by inspecting `CompilationUnit.depends` set. This set is
constructed by Scala compiler and it contains all symbols that given
compilation unit refers or even saw (in case of implicit search).
There are a few problems with this approach:
* The contract for `CompilationUnit.depend` is not clearly defined
in Scala compiler and there are no tests around it. Read: it's
not an official, maintained API.
* Improvements to incremental compiler require more context
information about given dependency. For example, we want to
distinguish between dependency on a class when you just select
members from it or inherit from it. The other example is that
we might want to know dependencies of a given class instead of
the whole compilation unit to make the invalidation logic more
precise.
That led to the idea of pushing dependency extracting logic to
incremental compiler side so it can evolve indepedently from Scala
compiler releases and can be refined as needed. We extract
dependencies of a compilation unit by walking a type-checked tree
and gathering symbols attached to them.
Specifically, the tree walk is implemented as a separate phase that
runs after pickler and extracts symbols from following tree nodes:
* `Import` so we can track dependencies on unused imports
* `Select` which is used for selecting all terms
* `Ident` used for referring to local terms, package-local terms
and top-level packages
* `TypeTree` which is used for referring to all types
Note that we do not extract just a single symbol assigned to `TypeTree`
node because it might represent a complex type that mentions
several symbols. We collect all those symbols by traversing the type
with CollectTypeTraverser. The implementation of the traverser is inspired
by `CollectTypeCollector` from Scala 2.10. The
`source-dependencies/typeref-only` test covers a scenario where the
dependency is introduced through a TypeRef only.
Introduce an alternative source dependency tracking mechanism that is
needed by upcoming name hashing algorithm. This new mechanism is
implemented by introducing two new source dependency relations called
`memberRef` and `inheritance`.
Those relations are very similar to existing `direct` and
`publicInherited` relations in some subtle ways. Those differences
will be highlighted in the description below.
Dependencies between source files are tracked in two distinct
categories:
* dependencies introduced by inheriting from a class/trait
defined in other source file
* dependencies introduced by referring (selecting) a member
defined in other source file (that covers all other
kinds of dependencies)
Due to invalidation algorithm implementation details sbt would need to
track inheritance dependencies of public classes only. Thus, we had
relation called `publicInherited`. The name hashing algorithm which
improves invalidation logic will need more precise information about
dependencies introduced by inheritance including dependencies of non-public
classes. That's one difference between `inheritance` and `publicInherited`
relations.
One surprising (to me) thing about `publicInherited` is that it includes
all base classes of a given class and not just parents. In that sense
`publicInherited` is transitive. This is a bit irregular because
everything else in Relations doesn't include transitive dependencies.
Since we are introducing new relations we have an excellent chance to
make things more regular. Therefore `inheritance` relation is
non-transitive and includes only extracted parent classes.
The access to `direct`, `publicInherited`, `memberRef` and `inheritance`
relations is dependent upon the value of `memberRefAndInheritanceDeps`
flag. Check documentation of that flag for details.
The two alternatives for source dependency tracking are implemented by
introduction of two subclasses that implement Relations trait and one
abstract class that contains some common logic shared between those two
subclasses. The two new subclasses are needed for the time being when we
are slowly migrating to the name hashing algorithm which requires
subtle changes to dependency tracking as explained above. For some time we
plan to keep both algorithms side-by-side and have a runtime switch which
allows to pick one. So we need logic for both old and new dependency
tracking to be available. That's exactly what two subclasses of
MRelationsCommon implement. Once name hashing is proven to be stable and
reliable we'll phase out the old algorithm and the old dependency tracking
logic.
The TestCaseGenerators uses global set for ensuring that certain generated
values are unique. This is not the best design because the more properties
you check the harder is to generate new sample inputs because of already
accumulated values. This results in:
[info] + Analysis.Simple Merge and Split: OK, proved property.
[info] ! Analysis.Complex Merge and Split: Gave up after only 8 passed tests. 93 tests were discarded.
I don't have an ambition to reduce the scope of this global set but at
least I wanted to make generators to work a bit harder on generating
samples.
Instead of using `suchThat` method for filtering out non-unique samples
we use `retryUntil` that never gives up (therefore it might not
terminate). We had to upgrade to latest (1.11.1) version of scalacheck
in order to have an access to `retryUntil` method.
Also, I overridden the `identifier` to delegate to original
`Gen.identifier` but with minimal size set to be to '3'. This means,
the generated identifier will be of size 3 or larger which is needed in
order to avoid collisions.
Reads/writes are a little faster with the text format,
and it's far more useful. E.g., it allows external manipulation
and inspection of the analysis.
We don't gzip the output. It does greatly shrink the files,
however it makes reads and writes 1.5x-2x slower, and we're
optimizing for speed over compactness.
It was an omission in the original commit that introduced them and didn't
mark them as private. They are purely an implementation detail and should
be hidden. We hiding them now.
Introduce a new incremental compiler option that controls
incremental compiler's treatment of macro definitions and their clients.
The current strategy is that whenever a source file containing a macro
definition is touched it will cause recompilation of all direct
dependencies of that file.
That strategy has proven to be too conservative for some projects like
Scala compiler of specs2 leading to too many source files being recompiled.
We make this behavior optional by introducing a new option
`recompileOnMacroDef` in `IncOptions` class. The default value is set to
`true` which preserves the previous behavior.
Add methods that allow one to set a new value to one of the fields of
IncOptions class. These methods are meant to be an alternative to
copy method that is hard to keep binary compatible when new fields are
added to the class.
Each copying method is related to one field of the class so when new
fields are added existing methods (and their signatures) are unaffected.
Expand case class `IncOptions` in binary compatible way so we can have
better control of methods like `unapply` when new fields are added.
Great precaution has been taken to ensure that this commit doesn't break
binary compatibility. I took a dump of javap output before and after
this change for both the class and it's companion object.
The diff is presented below:
diff -u ~/inc-options-before ~/inc-options-after
--- /Users/grek/inc-options-before 2013-11-03 14:48:45.000000000 +0100
+++ /Users/grek/inc-options-after 2013-11-03 15:53:10.000000000 +0100
@@ -9,7 +9,11 @@
public static java.lang.String transitiveStepKey();
public static sbt.inc.IncOptions setTransactional(sbt.inc.IncOptions, java.io.File);
public static sbt.inc.IncOptions defaultTransactional(java.io.File);
+ public static scala.Option unapply(sbt.inc.IncOptions);
+ public static sbt.inc.IncOptions apply(int, double, boolean, boolean, int, scala.Option, scala.Function0);
public static sbt.inc.IncOptions Default();
+ public static scala.Function1 tupled();
+ public static scala.Function1 curried();
public int transitiveStep();
public double recompileAllFraction();
public boolean relationsDebug();
diff -u inc-options-module-before inc-options-module-after
--- inc-options-module-before 2013-11-03 14:48:55.000000000 +0100
+++ inc-options-module-after 2013-11-12 21:00:41.000000000 +0100
@@ -3,6 +3,9 @@
public static final sbt.inc.IncOptions$ MODULE$;
public static {};
public sbt.inc.IncOptions Default();
+ public final java.lang.String toString();
+ public sbt.inc.IncOptions apply(int, double, boolean, boolean, int, scala.Option, scala.Function0);
+ public scala.Option unapply(sbt.inc.IncOptions);
public sbt.inc.IncOptions defaultTransactional(java.io.File);
public sbt.inc.IncOptions setTransactional(sbt.inc.IncOptions, java.io.File);
public java.lang.String transitiveStepKey();
@@ -13,7 +16,5 @@
public java.lang.String apiDiffContextSize();
public sbt.inc.IncOptions fromStringMap(java.util.Map);
public java.util.Map toStringMap(sbt.inc.IncOptions);
- public sbt.inc.IncOptions apply(int, double, boolean, boolean, int, scala.Option, scala.Function0);
- public scala.Option unapply(sbt.inc.IncOptions);
}
The first diff shows that there are just more static forwarders defined
for top-level companion object and that is binary compatible change.
The second diff shows that there are just a few minor differences in
order in which `unapply`, `apply` and bridge method for `apply` are
defined. Also, there's a new `toString` declaration. All those changes are
binary compatible.
All methods that are generated for a case class are marked as deprecated
and will be removed in the future.
The main motivation behind this commit is to reify information about
api changes that incremental compiler considers. We introduce a new
sealed class `APIChange` that has (at the moment) two subtypes:
* APIChangeDueToMacroDefinition - as the name explains, this represents
the case where incremental compiler considers an api to be changed
just because given source file contains a macro definition
* SourceAPIChange - this represents the case of regular api change;
at the moment it's just a simple wrapper around value representing
source file but in the future it will get expanded to contain more
detailed information about API changes (e.g. collection of changed
name hashes)
The APIChanges becomes just a collection of APIChange instances.
In particular, I removed `names` field that seems to be a dead code in
incremental compiler. The `NameChanges` class and methods that refer to
it in `SameAPI` has been deprecated.
The Incremental.scala has been adapted to changed signature of APIChanges
class. The `sameSource` method returns representation of APIChange
(if there's one) instead of just simple boolean. One notable change is
that information about APIChanges is pushed deeper into invalidation logic.
This will allow us to treat the APIChangeDueToMacroDefinition case properly
once name hashing scheme arrives.
This commit shouldn't change any behavior and is purely a refactoring.
The following events are logged:
* invalidation of source file due to macro definition
* inclusion of dependency invalidated by inheritance; we log both
nodes of dependency edge (dependent and dependency)
The second bullet helps to understand what's going on in case of
complex inheritance hierarchies like in Scala compiler.
The #958 describes a scenario where partially successful results are
produced in form of class files written to disk. However, if compilation
fails down the road we do not record any new compilation results (products)
in Analysis object. This leads to Analysis object and disk contents to get
out of sync.
One way to solve this problem is to use transactional ClassfileManager that
commits changes to class files on disk only when entire incremental
compilation session is successful. Otherwise, new class files are rolled
back to previous state.
The other way to solve this problem is to record time stamps of class files
in Analysis object. This way, incremental compiler can detect that class
files and Analysis object got out of sync and recover from that by
recompiling corresponding sources.
This commit uses latter solution which enables simpler (non-transactional)
ClassfileManager to handle scenario from #958.
Fixes#958
As pointed out by @harrah in #705, both beginSource and endSource are
not used in sbt internally for anything meaningful.
We've discussed an option of deprecating those methods but since they
are not doing anything meaningful Mark prefers to have compile-time
error in case somebody implements or calls those methods. I agree with
that hence removal.
Also add equals/hashCode implementations for MRelations.
Also add some comments to explain that ++ and -- are naively implemented.
Also fix some tabs-vs-spaces indentation nits.
equals/hashCode are useful for debugging/verifying/testing,
and the groupBy implementation is naive. It'll be replaced
by a groupBy implementation in Analysis that will handle
external/internal dep transitions correction.
Incremental compiler didn't have any explicit logic to handle
cancelled compilation so it would go into inconsistent state.
Specifically, what would happen is that it would treat cancelled
compilation as a compilation that finished normally and try to
produce a new Analysis object out of partial information collected
in AnalysisCallback. The most obvious outcome would be that the
new Analysis would contain latest hashes for source files. The
next time incremental compiler was asked to recompile the same files
that it didn't recompile due to cancelled compilation it would think
they were already successfully compiled and would do nothing.
We fix that problem by following the same logic that handles compilation
errors, cleans up partial results (produced class files) and makes sure
that no Analysis is created out of broken state.
We do that by introducing a new exception `CompileCancelled`
and throwing it at the same spot as an exception signalizing compilation
errors is being thrown. We also modify `IncrementalCompile` to
catch that exception and gracefully return as there was no compilation
invoked.
NOTE: In case there were compilation errors reported _before_
compilation cancellations was requested we'll still report them
using an old mechanism so partial errors are not lost in case
of cancelled compilation.
Add `apiDiffContextSize` option to `IncOptions` which allows one
to control the size (in lines) of a context used when printing
diffs for textual API representation.
The default value for `apiDiffContextSize` is 5 which seems to be
enough for most situations. This is verified by many debugging
sessions I performed when using api diffing functionality.
Implement displaying API changes by using textual representation
of an API (ShowAPI) and good, old textual diff algorithm. We are
using java-diff-utils library that is distributed under Apache 2.0
license.
Notice that we have only soft dependency on java-diff-utils. It means
that we'll try to lookup java-diff-utils class through reflection
and fail gracefully if none is found on the classpath. This way
sbt is not getting any new dependency. If user needs to debug
api diffs then it's matter of starting sbt with
`-Dsbt.extraClasspath=path/to/diffutils.jar` option passed
to sbt launcher.
1. All parents of public/exported classes/modules/packages are tracked as
'publicInherited' dependencies. These are dealiased and normalized so
that the dependency is on the actual underlying template and not the
source enclosing the alias.
2. All CompilationUnit.depends dependencies are direct dependencies. These
include inherited dependencies.
3. When invalidating changed internal sources,
a. Invalidate all inherited dependencies, transitively and include the
originally modified sources,
b. Invalidate all direct dependencies of these sources,
c. Exclude any sources that were compiled in the previous step unless they
depend on a newly invalidated source.
4. Invalidate changed external sources in the same way as #3 but remove the
external sources from the final set.
Only public inheritance dependencies need to be considered because a template
that is not accessible outside its source file and that inherits from another
file can be handled as a normal, direct dependency. Because the template
isn't public, changes to its API will not propagate outside of the source
file.
Several existing tests cover the correctness, especially:
1. transitive-a covers direct, transitive dependencies with inferred return
types
2. transitive-b covers inherited, transitive dependencies with inferred return
types
There are two new tests, one that tests that public inherited dependencies are
tracked and one that verifies the basic invalidation progression.
More tests are needed to verify the improvements that this algorithm brings:
1. Inheritance-related dependencies are processed in one step to avoid the
otherwise unavoidable several steps.
2. Only immediate direct dependencies are ever processed, which should in many
typical cases avoid large invalidation sets.
That's foundation for API dumping and tests based on API representation contents.
Specific list of changes introduced by this commit:
* `AnalysisCallback` class takes `IncOptions` as argument so it
can determine if API should be minimized in `api` callback.
* Introduce `Incremental.apiDebug` method that determines if api debugging
is enabled. There are two ways to enable api debugging: through system
property and through incremental compiler options. The system property
method will be soon deprecated. We introduce it to make it easier to enable
API debugging until tools (like zinc and ide) catch up with making incremental
compiler configurable.
NOTE: The `apiDebug` method has been introduced in Incremental for two reasons:
1. It's analogous to `incDebug` method that's already there.
2. In other branch I need `apiDebug` to be defined in Incremental.
Once we deprecate and remove enabling debugging options through system properties
the code will be cleaned up.
Mark Harrah
Grzegorz Kossakowski
Benjy
Benjy