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

Variable misuse ML model found a bug where wrong variable is used to … #23437

Merged
merged 2 commits into from Nov 29, 2017

Conversation

Projects
None yet
5 participants
@heejaechang
Contributor

heejaechang commented Nov 28, 2017

…call a method.

Customer scenario

Customer clicks on error list to navigate to an error and VS crash.

Bugs this fixes

#23436

Workarounds, if any

There is no workaround.

Risk

Low risk. the code path is fallback code path, so not always exercised. and it will no longer throw an exception.

Performance impact

N/A

Is this a regression from a previous update?

No

Root cause analysis

when we try to navigate to an error, we try our best to go to right location. since error list can contain staled error, location info can be wrong, so we try to get right snapshot to calculate right location. but it is not always guaranteed that we can get to right text snapshot from roslyn snapshot. when that is failed, we fallback to whatever latest text snapshot we have to calculate location. here code used wrong snapshot when it is supposed to use current snapshot which lead to null ref.

How was the bug found?

Variable misuse ML model tool.

@heejaechang

This comment has been minimized.

Contributor

heejaechang commented Nov 28, 2017

@heejaechang

This comment has been minimized.

Contributor

heejaechang commented Nov 28, 2017

@jinujoseph is it for master? or should I target it to different branch?

@jinujoseph

This comment has been minimized.

Contributor

jinujoseph commented Nov 28, 2017

This is for 15.6 preview2 , hence master is correct

@jinujoseph

This comment has been minimized.

Contributor

jinujoseph commented Nov 28, 2017

cc @Pilchie for ask mode approval

@@ -362,7 +362,7 @@ private string GetTestOutputFilePath(string filepath)
try
{
outputFilePath = Path.GetDirectoryName(_filePath);
outputFilePath = Path.GetDirectoryName(filepath);

This comment has been minimized.

@Pilchie

Pilchie Nov 28, 2017

Member

Can we make this either a static method? Why can't it use the field instead?

This comment has been minimized.

@heejaechang

heejaechang Nov 29, 2017

Contributor

it uses field such as Compilation, so can't be static. I don't know why it accepts filepath rather than not accepting any parameter and use all fields inside. but regardless, getting filepath is not a bug and more flexible if one want to reuse this method for different file path. so I am leaving that as it is.

@rchande

This comment has been minimized.

Member

rchande commented Nov 28, 2017

@heejaechang Can you share more info about "variable misuse ML model"? Sounds interesting.

@jinujoseph

This comment has been minimized.

Contributor

jinujoseph commented Nov 28, 2017

@rchande , i have fwd the mail thread for context

@rchande

This comment has been minimized.

Member

rchande commented Nov 28, 2017

Thanks!

@heejaechang heejaechang merged commit f212118 into dotnet:master Nov 29, 2017

15 checks passed

WIP ready for review
Details
license/cla All CLA requirements met.
Details
microbuild_prtest Build finished.
Details
perf_correctness_prtest Build finished.
Details
ubuntu_14_debug_prtest Build finished.
Details
ubuntu_16_debug_prtest Build finished.
Details
windows_build_correctness_prtest Build finished.
Details
windows_coreclr_test_prtest Build finished.
Details
windows_debug_unit32_prtest Build finished.
Details
windows_debug_unit64_prtest Build finished.
Details
windows_debug_vs-integration_prtest Build finished.
Details
windows_determinism_prtest Build finished.
Details
windows_release_unit32_prtest Build finished.
Details
windows_release_unit64_prtest Build finished.
Details
windows_release_vs-integration_prtest Build finished.
Details
@rmarcil

This comment has been minimized.

rmarcil commented Feb 14, 2018

Here's the accepted AI conference paper explaining this new "QA" tool that found this bug. Expect more AI improvements in SW in the future.

@Article{
allamanis2018learning,
title={Learning to Represent Programs with Graphs},
author={Miltiadis Allamanis, Marc Brockschmidt, Mahmoud Khademi},
journal={International Conference on Learning Representations},
year={2018},
url={https://openreview.net/forum?id=BJOFETxR-},
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment