Skip to content

Fix regression in bitness of Interactive Window host#28006

Merged
tmat merged 6 commits intodotnet:masterfrom
tmat:IWBitness
Jun 29, 2018
Merged

Fix regression in bitness of Interactive Window host#28006
tmat merged 6 commits intodotnet:masterfrom
tmat:IWBitness

Conversation

@tmat
Copy link
Member

@tmat tmat commented Jun 20, 2018

Customer scenario

Customer uses Initialize Interactive from Project command on a project that has a 32-bit native dependency. The dependencies fail to load since the host process is 64-bit. This is a regression from previous VS version where the process was 32-bit.

Simply switching back to 32-bit would fix the regression but won't help our customers who use interactive to work with 64-bit native libraries or huge data sets. This change therefore adds an optional parameter to #reset command that allows to specify the desired bitness: #reset 32 and #reset 64 set the bitness accordingly, while #reset without the argument preserves the current bitness for the new process instance. In addition, Initialize Interactive from Project feature sets the bitness based on the target platform of the project. We also now display the bitness in the title of the Interactive Window, so that it's clear which bitness is the active one.

Bugs this fixes

VS Feedback
#7663

Workarounds, if any

No workaround.

Risk

Small.

Performance impact

None.

Is this a regression from a previous update?

Root cause analysis

Missing test coverage.

How was the bug found?

Reported by multiple customers.

Test documentation updated?

@tmat tmat requested review from a team as code owners June 20, 2018 01:16
@tmat tmat changed the title WIP: Fix regression in bitness of Interactive Window host Fix regression in bitness of Interactive Window host Jun 25, 2018
@tmat
Copy link
Member Author

tmat commented Jun 26, 2018

@dotnet/roslyn-interactive @ivanbasov Please review.

@jinujoseph jinujoseph added this to the 15.8 milestone Jun 26, 2018
{
bool initialize;
if (!TryParseArguments(arguments, out initialize))
if (!TryParseArguments(arguments, out bool initialize, out bool? is64bit))
Copy link
Contributor

Choose a reason for hiding this comment

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

bool [](start = 71, length = 4)

I think that enum would be better for the case.

Copy link
Member Author

Choose a reason for hiding this comment

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

How would that enum items be named?

Copy link
Contributor

Choose a reason for hiding this comment

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

Bitness?


In reply to: 198254174 [](ancestors = 198254174)

Copy link
Member Author

Choose a reason for hiding this comment

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

The values?

Copy link
Contributor

Choose a reason for hiding this comment

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

32
64
default? - if necessary


In reply to: 198254373 [](ancestors = 198254373)

Copy link
Member Author

Choose a reason for hiding this comment

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

32 is not a valid identifier

Copy link
Contributor

Choose a reason for hiding this comment

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

enum Bitness
{
    DefaultBitness,
    Bit32,
    Bit64
}

In reply to: 198255961 [](ancestors = 198255961)

Copy link
Member Author

Choose a reason for hiding this comment

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

Bit32 is odd. I have thought about this and didn't come up with any good names, so I used bool?.
We need both {32, 64} and {32, 64, default} types.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Other then that, I do not have any comments. I still would prefer to reconsider "bool? Is64Bit" because it looks counterintuitive.

You can decide but please re-consider it once again.


In reply to: 198258202 [](ancestors = 198258202)

Copy link
Contributor

@ivanbasov ivanbasov left a comment

Choose a reason for hiding this comment

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

:shipit:

@tmat
Copy link
Member Author

tmat commented Jun 27, 2018

test windows_debug_vs-integration_prtest please

@tmat
Copy link
Member Author

tmat commented Jun 27, 2018

test windows_debug_vs-integration_prtest please

@tmat
Copy link
Member Author

tmat commented Jun 27, 2018

test windows_release_vs-integration_prtest please

@tmat
Copy link
Member Author

tmat commented Jun 28, 2018

test windows_debug_vs-integration_prtest please

@tmat
Copy link
Member Author

tmat commented Jun 28, 2018

test windows_release_vs-integration_prtest please

@tmat
Copy link
Member Author

tmat commented Jun 28, 2018

test windows_debug_vs-integration_prtest please

@tmat
Copy link
Member Author

tmat commented Jun 28, 2018

test windows_release_vs-integration_prtest please

1 similar comment
@tmat
Copy link
Member Author

tmat commented Jun 29, 2018

test windows_release_vs-integration_prtest please

@tmat
Copy link
Member Author

tmat commented Jun 29, 2018

@jinujoseph For 15.8 approval

@jinujoseph
Copy link
Contributor

Approved to merge for 15.8.Preview5

@tmat tmat merged commit 6da2709 into dotnet:master Jun 29, 2018
@tmat tmat deleted the IWBitness branch June 29, 2018 23:39
333fred added a commit to 333fred/roslyn that referenced this pull request Jul 11, 2018
…atures/compiler

* dotnet/features/compiler: (137 commits)
  Actually fix tabs.
  Added back breaking changes doc.
  Addressed PR feedback.
  Track enclosing symbol in data flow analysis, which might be a field or property. (dotnet#28252)
  Change text from 'Spell check' to 'Fix typo'.
  Implement FAR for GetAwaiter methods (dotnet#28230)
  Fix typo
  Fix typo
  Fix import.
  Address PR feedback and fix failing test.
  Lower expressions of in arguments to correct temp locals (dotnet#28181)
  Move to 2.0.7 runtime
  Produce errors on ref-assignment to non-ref parameters
  Fix spelling.
  Preserve left-to-right evaluation order for dynamic compound addition and subtraction.
  inline.
  Provide a spellchecking suggestion when someone mispells a constructor.
  Typo (dotnet#28177)
  PR Comments
  Fix regression in bitness of Interactive Window host (dotnet#28006)
  ...
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.

3 participants