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

Moe Sync #1330

Merged
merged 27 commits into from
Aug 13, 2019
Merged

Moe Sync #1330

merged 27 commits into from
Aug 13, 2019

Conversation

netdpb
Copy link
Member

@netdpb netdpb commented Aug 13, 2019

This code has been reviewed and submitted internally. Feel free to discuss on the PR and we can submit follow-up changes as necessary.

Commits:

Rename PreferDurationOverload to PreferJavaTimeOverload, as it now covers Instant as well.

RELNOTES: Rename PreferDurationOverload to PreferJavaTimeOverload, as it now covers Instant as well.

ff52f89


Add further caveat about VisitorState.getSourceForNode.

Per comment in #1316

840c25f


Fix AutoValueFinalMethods warnings in refaster code.

https://errorprone.info/bugpattern/AutoValueFinalMethods

RELNOTES: Fix AutoValueFinalMethods warnings in refaster code.

c2b28db


Fix FormatString Checker crash on Console.readLine()

Bug:
FormatString checker breaks at
System.console().readLine();

9575c7d


Consolidate stripParentheses implementations

90f7e30


Upgrade to Caffeine version 2.7.0 as requested by Ben Maines: https://github.com//pull/1316#commitcomment-34487169

c53bdfd


Fix some AutoValueImmutableFields warning in refaster code (1/N)

AutoValue instances should be deeply immutable. Therefore, we recommend returning ImmutableList instead. Read more at https://github.com/google/auto/blob/master/value/userguide/builders-howto.md#collection.

See https://errorprone.info/bugpattern/AutoValueImmutableFields.

RELNOTES: Fix some AutoValueImmutableFields warning in refaster code

d58ebfc


Fix some AutoValueImmutableFields warning in refaster code (2/N)

AutoValue instances should be deeply immutable. Therefore, we recommend returning ImmutableList instead. Read more at https://github.com/google/auto/blob/master/value/userguide/builders-howto.md#collection

See https://errorprone.info/bugpattern/AutoValueImmutableFields.

RELNOTES: Fix some AutoValueImmutableFields warning in refaster code

a3169a0


Fix some AutoValueImmutableFields warning in refaster code (3/N)

AutoValue instances should be deeply immutable. Therefore, we recommend returning ImmutableList instead. Read more at https://github.com/google/auto/blob/master/value/userguide/builders-howto.md#collection.

See https://errorprone.info/bugpattern/AutoValueImmutableFields.

RELNOTES: Fix some AutoValueImmutableFields warning in refaster code

643ba29


SuggestedFixes: add an overload to compilesWithFix to allow extra compiler options to be passed in.

The motivating use case is to make unchecked/rawtypes warnings into errors, in order to determine whether the suppression is needed.

97d9c13


VisitorState: add getElements() method, and use instead of getTreeMaker().Literal(...)

ce095ff


Remove a stale comment

bd99432 made the 'fast' path the only path.

3476b0c


Make ProtoFieldNullComparison descend into casts and parens in initializers.

Default more flags to true; the cleanups are almost done.

518c59d


Recognize that FluentIterable has undefined equals(), too.

386bca7


Split PreferJavaTimeOverload explanation into its own external markdown file (also fix a bug where "" was being rendered as HTML).

47f4c20


ProtoFieldNullComparison: handle Optional#ofNullable

I was a bit torn between replacing with just of vs something like proto.hasFoo() ? Optional.of(proto.getFoo()) : Optional.empty(), but it looks like the former is far more commonly correct from the usage sites.

b60b55e


Build a single SubContext up front, instead of for each file

This way we have a single Context object for all files compiled by a
single invocation of javac, making it easier to tie the lifecycle of
other metadata to that javac invocation.

ba14985


Move some never-changed, rarely-read fields into SharedState

The performance impact of this is minimal (about 0.15% of javac time
saved), but it clarifies the tangled web of VisitorState constructors
a bit.

fef940a


Improve TreeToString docs

9e69135


Use the new InternalOneOfEnum interface to distinguish oneof enums.

138960b


Add a comment with some explanation of why we don't fully resolve types that are returned from getTypeFromString()

e2dab03


Add support for inline caches

As the javadoc suggests, these can be used to memoize an expensive
operation that requires a VisitorState but is not expected to change
throughout the compilation.

To manage the lifecycle of these caches, also add a new item to the
Context representing compilation context.

Also, as a proof of concept, add one inline cache, in front of
Suppliers.fromString - this alone halves the time spent converting
String to Type in the entire compilation.

More strategically-placed caches to come in future.

bfeaf2c


Avoid calling filter(x -> true)

By hoisting out the check for skipInterfaces, we can avoid a useless filter.

aa16f63


Simplify a Supplier, and get it cached to boot

244617f


Take a pass over PrivateConstructorForUtilityClass docs

6491fe8


Avoid calling isInherited if we won't be using the result

ccf63b3


FieldCanBeLocal: descend into member selects.

(Duh.)

5361417

kluever and others added 27 commits August 13, 2019 16:14
…vers Instant as well.

RELNOTES: Rename PreferDurationOverload to PreferJavaTimeOverload, as it now covers Instant as well.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=260509421
Per comment in #1316

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=260513433
https://errorprone.info/bugpattern/AutoValueFinalMethods

RELNOTES: Fix AutoValueFinalMethods warnings in refaster code.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=260614808
Bug:
FormatString checker breaks at
     System.console().readLine();

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=260614979
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=260634412
AutoValue instances should be deeply immutable. Therefore, we recommend returning ImmutableList instead. Read more at https://github.com/google/auto/blob/master/value/userguide/builders-howto.md#collection.

See https://errorprone.info/bugpattern/AutoValueImmutableFields.

RELNOTES: Fix some AutoValueImmutableFields warning in refaster code

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=260764700
AutoValue instances should be deeply immutable. Therefore, we recommend returning ImmutableList instead. Read more at https://github.com/google/auto/blob/master/value/userguide/builders-howto.md#collection

See https://errorprone.info/bugpattern/AutoValueImmutableFields.

RELNOTES: Fix some AutoValueImmutableFields warning in refaster code

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=260795615
AutoValue instances should be deeply immutable. Therefore, we recommend returning ImmutableList instead. Read more at https://github.com/google/auto/blob/master/value/userguide/builders-howto.md#collection.

See https://errorprone.info/bugpattern/AutoValueImmutableFields.

RELNOTES: Fix some AutoValueImmutableFields warning in refaster code

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=260811556
…piler options to be passed in.

The motivating use case is to make unchecked/rawtypes warnings into errors, in order to determine whether the suppression is needed.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=260862493
…er().Literal(...)

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=260932977
bd99432 made the 'fast' path the only path.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=261365852
…lizers.

Default more flags to true; the cleanups are almost done.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=261687873
…wn file (also fix a bug where "<long, TimeUnit>" was being rendered as HTML).

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=261743439
I was a bit torn between replacing with just `of` vs something like `proto.hasFoo() ? Optional.of(proto.getFoo()) : Optional.empty()`, but it looks like the former is far more commonly correct from the usage sites.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=261900295
This way we have a single Context object for all files compiled by a
single invocation of javac, making it easier to tie the lifecycle of
other metadata to that javac invocation.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=261972898
The performance impact of this is minimal (about 0.15% of javac time
saved), but it clarifies the tangled web of VisitorState constructors
a bit.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=261975619
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=261976141
…es that are returned from getTypeFromString()

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=262144390
As the javadoc suggests, these can be used to memoize an expensive
operation that requires a VisitorState but is not expected to change
throughout the compilation.

To manage the lifecycle of these caches, also add a new item to the
Context representing compilation context.

Also, as a proof of concept, add one inline cache, in front of
Suppliers.fromString - this alone halves the time spent converting
String to Type in the entire compilation.

More strategically-placed caches to come in future.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=262249676
By hoisting out the check for skipInterfaces, we can avoid a useless filter.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=262426287
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=262431640
(Duh.)

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=262878055
@eaftan eaftan merged commit 5c0f77f into master Aug 13, 2019
@netdpb netdpb deleted the sync-master-2019/08/12 branch August 14, 2019 15:08
@cpovirk cpovirk mentioned this pull request Aug 20, 2019
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.