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.

Sign up for GitHub

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

Jump to bottom

SI-9403 fix ICodeReader for negative BIPUSH / SIPUSH values #4653

Merged
merged 1 commit into from Jul 27, 2015
Merged

SI-9403 fix ICodeReader for negative BIPUSH / SIPUSH values #4653

merged 1 commit into from Jul 27, 2015

Conversation

lrytz
Copy link
Member

@lrytz lrytz commented Jul 22, 2015

The byte value of a BIPUSH instruction and the (byte1 << 8) | byte2
value of a SIPUSH instruction are signed, see [1] and [2].

Similar for the increment value of IINC [3].

[1] https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-6.html#jvms-6.5.bipush
[2] https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-6.html#jvms-6.5.sipush
[3] https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-6.html#jvms-6.5.iinc

@scala-jenkins scala-jenkins added this to the 2.11.8 milestone Jul 22, 2015
@lrytz
Copy link
Member Author

lrytz commented Jul 22, 2015

This one should be fun to review :) @som-snytt maybe?

@lrytz
Copy link
Member Author

lrytz commented Jul 22, 2015

integrate-ide failure most likely because http://typesafe.artifactoryonline.com is gone - it won't come back (https://www.typesafe.com/blog/migrating-repos-to-bintray) cc @sschaef

@kiritsuku
Copy link
Contributor

Ok, I'll have a look.

@lrytz lrytz mentioned this pull request Jul 23, 2015
Closed
@retronym
Copy link
Member

/rebuild

@lrytz
Copy link
Member Author

lrytz commented Jul 24, 2015 

[scalacheck] [warn]     ::::::::::::::::::::::::::::::::::::::::::::::
[scalacheck] [warn]     ::          UNRESOLVED DEPENDENCIES         ::
[scalacheck] [warn]     ::::::::::::::::::::::::::::::::::::::::::::::
[scalacheck] [warn]     :: com.typesafe.dbuild#distributed-sbt-plugin;0.9.1: not found
[scalacheck] [warn]     ::::::::::::::::::::::::::::::::::::::::::::::

Looking at the logs:

        "resolvers" : {
[...]
            "R0010" : "dbuild-snapshots: http://typesafe.artifactoryonline.com/typesafe/temp-distributed-build-snapshots, [organization]/[module]/(scala_[scalaVersion]/)(sbt_[sbtVersion]/)[revision]/[type]s/[artifact](-[classifier]).[ext]",
[...]
            "R0011" : "sbt-plugin-releases: http://scalasbt.artifactoryonline.com/scalasbt/sbt-plugin-releases, [organization]/[module]/(scala_[scalaVersion]/)(sbt_[sbtVersion]/)[revision]/[type]s/[artifact](-[classifier]).[ext]",
[...]
        }

Then googling a bit, maybe it comes from here?

https://github.com/typesafehub/sbt-builds-for-ide/blob/master/bin/dbuild.properties#L22-L23

@lrytz
Copy link
Member Author

lrytz commented Jul 24, 2015

In the uber-build repo, there are two files with the artifactoryonline.com resolvers, maybe it's also one of them.
https://github.com/scala-ide/uber-build/blob/c40609397ca655cac6eff88db8b7930b1e793323/zinc/bin/drepo.properties#L23-L24
https://github.com/scala-ide/uber-build/blob/c40609397ca655cac6eff88db8b7930b1e793323/zinc/bin/dbuild.properties#L24-L25

@lrytz
Copy link
Member Author

lrytz commented Jul 24, 2015

submitted scala-ide/uber-build#80

@lrytz
Copy link
Member Author

lrytz commented Jul 24, 2015

/rebuild

som-snytt
som-snytt reviewed Jul 24, 2015
View reviewed changes
test/files/run/t9403/Test_2.scala
case IntOp(Opcodes.BIPUSH, -2) => true
case _ => false
})
}
Copy link
Contributor

Choose a reason for hiding this comment

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

For historical purposes.

@som-snytt
Copy link
Contributor

LGTM

retronym added a commit that referenced this pull request Jul 27, 2015
@retronym
Merge pull request #4653 from lrytz/t9403
f682441 
SI-9403 fix ICodeReader for negative BIPUSH / SIPUSH values
@retronym retronym merged commit f682441 into scala:2.11.x Jul 27, 2015
adriaanm added a commit to adriaanm/scala that referenced this pull request Aug 5, 2015
@adriaanm
Merge branch '2.11.x' into 2.12.x
ecf9128 
Transcript:

```
➜  scala git:(2.12.x) export MB=$(git merge-base 2.12.x 2.11.x)
➜  scala git:(2.12.x) echo $MB
0e9525a
➜  scala git:(2.12.x) g log --oneline --graph $MB...2.11.x
```

Read this upside down :-). The last merge comes first,
with merge commands interspersed in the git log.

```
➜  scala git:(2.12.x) g merge 2.11.x
Auto-merging versions.properties
Auto-merging src/reflect/scala/reflect/internal/pickling/UnPickler.scala
Auto-merging src/reflect/scala/reflect/internal/Types.scala
Auto-merging src/reflect/scala/reflect/internal/TreeGen.scala
Auto-merging src/library/scala/collection/immutable/Stream.scala
Auto-merging src/library/scala/collection/convert/Wrappers.scala
Auto-merging build.xml
CONFLICT (content): Merge conflict in build.xml
Auto-merging README.md
Automatic merge failed; fix conflicts and then commit the result.
```

```
*   928e689 (scala/2.11.x, 2.11.x) Merge pull request scala#4682 from adriaanm/jline-quick.bin
|\
| * b763dbf (adriaanm/jline-quick.bin, jline-quick.bin) Include jline on quick.bin tool path
* |   ccded7d Merge pull request scala#4680 from janekdb/2.11.x-option
|\ \
| |/
|/|
| * 58ae3e5 Delegate null test to Option
|/
*   a745f06 Merge pull request scala#4670 from retronym/ticket/9422
|\
| * ec95e53 SI-9422 Fix incorrect constant propagation
*   65fa73d Merge pull request scala#4669 from janekdb/2.11.x-scaladoc-reflect
|\
| * e206a18 ScalaDoc fixes for reflect
* |   8e7e3b4 Merge pull request scala#4667 from janekdb/2.11.x-scaladoc-library-library-aux
|\ \
| |/
|/|
| * 69c2c10 ScalaDoc fixes for library and library-aux
* |   7de4cbc Merge pull request scala#4665 from lrytz/asm-504-3
|\ \
| * | cdc55c2 Upgrade scala-asm to 5.0.4-scala-3
|/ /
* |   d8da39a Merge pull request scala#4661 from retronym/ticket/9365
|\ \
| * | 0c99742 SI-9365 Don't null out dependencies of transient lazy vals
|  /
* |   2279d3f Merge pull request scala#4662 from janekdb/2.11.x-redundant-val-modifier
|\ \
| * | 173a6fa Remove redundant 'val' from case class params.
* | |   e0d2143 Merge pull request scala#4664 from SethTisue/remove-dead-link-in-readme
|\ \ \
| |_|/
|/| |
| * | 600fc4e fix readme for death of typesafe.artifactoryonline.com
| |/
* |   7492bda Merge pull request scala#4636 from SethTisue/contributor-stuff-from-github-wiki
|\ \
| |/
|/|
| * ed5098d merge two reviewers lists in readme
| * e136e4a tighten up CONTRIBUTING.md a little
| * f9ca686 readme/contributor's guide tweaks
| * 80e98b0 tiny readme fix
| * 1978456 merge in text from pull request policy from old wiki
| * e93ca40 drop in pull request policy from old wiki
| * 951939d contributor guide: add a morsel salvaged from GitHub wiki
*   f682441 Merge pull request scala#4653 from lrytz/t9403
|\
| * 2678d34 SI-9403 fix ICodeReader for negative BIPUSH / SIPUSH values
```

```
➜  scala git:(2.12.x) g merge -s ours f2d7838
Merge made by the 'ours' strategy.
```

```
*   f2d7838 Merge pull request scala#4657 from lrytz/backports
|\
| * 241bb9a Rename the ENUM / DEFAULTMETHOD flags to include JAVA_
| * 7a7f992 SI-9393 fix modifiers of ClassBTypes for Java annotations
| * 8946d60 [backport] Fix bytecode stability when running the closure optimizer
| * 3b6b2bf [backport] SI-9392 Clarify the workaround comment and introduce a devWarning
| * 091c1e6 [backport] SI-9392 Avoid crash in GenBCode for incoherent trees
| * 6177cb4 [backport] SI-9393 Temporarily disable two assertions in GenBCode
| * a1d471f [backport] Refactor the ClosureOptimizer, run ProdCons only once per method
| * f5e7276 [backport] SI-9387 Fix VerifyError introduced by indylambda
| * 41b99e2 [backport] Integrate the LMFInvokeDynamic extractor into LambdaMetaFactoryCall
| * fc1cda2 [backport] Small refactoring to the closure optimizer
| * 8f272c0 [backport] Accessibility checks for methods with an InvokeDynamic instruction
| * 1c1d825 [backport] Fix bytecode stability
| * ef9d845 [backport] Support methodHandle / invokeDynamic constant pool entries in scalap
| * 60747c7 [backport] Skip mirror class when invoking deserializeLambda
| * 404e862 [backport] Prevent infinite recursion in ProdConsAnalyzer
| * 1b0703e [backport] SI-9376 don't crash when inlining a closure body that throws.
| * e511375 [backport] Fix superclass for Java interface symbols created in JavaMirrors
| * 1b57723 [backport] `deserializeLambda` should not use encoded class name
| * 8bafa8e [backport] Java parser: default methods in interfaces are not `DEFERRED`
| * 44e2761 [backport] SI-6613 fixed in GenBCode
```

```
➜  scala git:(2.12.x) g merge 4c6dcfe
Auto-merging src/scaladoc/scala/tools/nsc/doc/html/page/Template.scala
Auto-merging doc/License.rtf
Auto-merging doc/LICENSE.md
Auto-merging build.xml
Auto-merging build.sbt
Merge made by the 'recursive' strategy.
Auto packing the repository in background for optimum performance.
See "git help gc" for manual housekeeping.
 build.sbt                                                 | 2 +-
 build.xml                                                 | 2 +-
 doc/LICENSE.md                                            | 4 ++--
 doc/License.rtf                                           | 4 ++--
 src/scaladoc/scala/tools/nsc/doc/html/page/Template.scala | 2 +-
 src/scalap/decoder.properties                             | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)
```

```
* |   4c6dcfe Merge pull request scala#4656 from lrytz/filtersOpenJDK
|\ \
| |/
|/|
| * 0b35bc2 Ignore OpenJDK warnings in partest filters
* 6eb0812 Merge pull request scala#4644 from SethTisue/copyright-2015
* e0aac7c bump copyright year to 2015
```
@adriaanm adriaanm mentioned this pull request Aug 5, 2015
Merged
@scabug scabug mentioned this pull request Apr 7, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.11.8
5 participants
@lrytz @kiritsuku @retronym @som-snytt @scala-jenkins