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

Scanner(Unification) & TryPeek #1727

Closed
wants to merge 9 commits into from

Conversation

AdamSpeight2008
Copy link
Contributor

This pull request contains a previously submitted PR(#1463), that PR as been included into this one as this pull request merges with the master branch

  1. The unifies the contents of TryScanToken and ScanTokenFullWidth. (Scanner(unification) + TryPeek #1463)

I retain those methods so not the break existing calls to them. The internals now call a common implementation method ScanTokenCommon.

This reduces the size of the sourcecode and the resulting IL (Debug)

                       Old |  New 
                     ------+------
      TryScanToken:-  1746 |   42
ScanTokenFullWidth:-  1972 |   15
   ScanTokenCommon:-       | 2035
                     ------+------
             Total:-  3718 | 2092
  1. An implementation and usage TryPeek, that is a try variant of Peek

…de readability.

CanGetCharAtOffset -> CanGet
PeekAheadChar -> Peek
PeakChar -> Peek
Modified ScannerXML to use NextAre and NextIs, the remaining candidates are mostly suitable for TryPeek (not yet implemented)
Fixed the failing `Debug.Assert`s that used `AreNext(0, " ")` as the failure was being cause by CanGet looking to far ahead (off by one)
# By Jared Parsons (22) and others
# Via Tomáš Matoušek (15) and others
* 'master' of https://github.com/dotnet/roslyn: (137 commits)
  Remove some unncessary usings in v1 Diagnostic incremental analyzer.
  Don't report any locations in the error log file for diagnostics with Location.None. Fixes dotnet#1582
  Collapse switch cases
  Update DuplicateTypesAndMethodsDifferentAssemblies tests
  Fix typo: teh -> the
  Incorporate PR feedback
  Fix timestamp written to Debug Image Directory
  Port EvaluationContext changes to VB
  Use MetadataReader to extract analyzer display names.
  Add DuplicateTypesAndMethodsDifferentAssemblies test
  Fix CodeFixService.GetProjectDiagnostics to handle compilation end diagnostics. Currently it was just force computing all document diagnostics, not the project diagnostics, which executes the compilation end actions. Additionally, also fix the BatchFixAllProvider to correctly handle compilation end diagnostics.
  Actually write out the MemoryStream
  Respond to PR feedback
  Missed a file
  [EnC] Fix ArgumentException in SymbolMatcher
  More CoreFx porting work
  Adding additional tests
  Incorporate PR feedback
  [EnC] Fix ArgumentException in SymbolMatcher
  Jenkins should use the full Roslyn.sln
  ...

Conflicts:
	src/Compilers/VisualBasic/Portable/Scanner/Scanner.vb
	src/Compilers/VisualBasic/Portable/Scanner/ScannerInterpolatedString.vb
	src/Compilers/VisualBasic/Portable/Scanner/ScannerXml.vb
	src/Compilers/VisualBasic/Portable/Scanner/XmlDocComments.vb
	src/Compilers/VisualBasic/Portable/Scanner/XmlTokenFactories.vb
@AdamSpeight2008 AdamSpeight2008 changed the title Implementation of Try Method Scanner(Unification) & TryPeek Apr 1, 2015
@AdamSpeight2008
Copy link
Contributor Author

@gafter @AlekseyTs Could you please look at this and decide if it is worth moving forward with it?

@jaredpar
Copy link
Member

jaredpar commented Apr 1, 2015

This looks very much like several other PRs that you have already submitted and closed.

@gafter
Copy link
Member

gafter commented Apr 1, 2015

Please put your changes in #1463 for us to look at.

@gafter gafter closed this Apr 1, 2015
@dotnet dotnet locked and limited conversation to collaborators Apr 1, 2015
@AdamSpeight2008 AdamSpeight2008 deleted the TryPeek_4 branch October 31, 2015 22:29
@gafter gafter added the Resolution-Won't Fix A real bug, but Triage feels that the issue is not impactful enough to spend time on. label Dec 9, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution-Won't Fix A real bug, but Triage feels that the issue is not impactful enough to spend time on.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants