Skip to content

Conversation

hvitved
Copy link
Contributor

@hvitved hvitved commented Feb 5, 2020

No description provided.

@hvitved hvitved added the C# label Feb 5, 2020
@hvitved hvitved requested a review from calumgrant February 5, 2020 12:18
@hvitved hvitved requested a review from a team as a code owner February 5, 2020 12:18
@hvitved hvitved force-pushed the csharp/stackalloc branch 2 times, most recently from e914679 to d10471d Compare February 5, 2020 18:38
@hvitved hvitved added the depends on internal PR This PR should only be merged in sync with an internal Semmle PR label Feb 5, 2020
Copy link
Contributor

@calumgrant calumgrant left a comment

Choose a reason for hiding this comment

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

QL and C# LGTM.

@@ -27,7 +27,8 @@ The following changes in version 1.24 affect C# analysis in all applications.
## Changes to code extraction

* Tuple expressions, for example `(int,bool)` in `default((int,bool))` are now extracted correctly.
* Expression nullability flow state is extracted.
* Expression nullability flow state is extracted.
* `stackalloc` array creations are now extracted, and they are represented by the class `Stackalloc`.
Copy link
Contributor

Choose a reason for hiding this comment

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

This gives the impression that they weren't previously extracted at all. How about

In extraction:

  • The difference between stackalloc array creations normal array creations is extracted.

In changes to QL libraries:

  • stackalloc array creations are now represented by the QL class Stackalloc. Previously they were represented by the class ArrayCreation.

@@ -372,6 +372,11 @@ class ArrayCreation extends Expr, @array_creation_expr {
override string toString() { result = "array creation of type " + this.getType().getName() }
}

/** A `stackalloc` array creation. */
Copy link
Contributor

Choose a reason for hiding this comment

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

How about some sample C# code in the comment.

@calumgrant calumgrant merged commit 5838df1 into github:master Feb 11, 2020
@hvitved hvitved deleted the csharp/stackalloc branch February 11, 2020 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C# depends on internal PR This PR should only be merged in sync with an internal Semmle PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants