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

Anonymous types cause problems with Immutability #694

Open
TimothyJCowen opened this issue Jan 13, 2021 · 2 comments
Open

Anonymous types cause problems with Immutability #694

TimothyJCowen opened this issue Jan 13, 2021 · 2 comments

Comments

@TimothyJCowen
Copy link
Contributor

error D2L0066: The class "<anonymous type: long expires>" is missing the [Immutable] attribute, so it isn't safe to be held by an immutable type
@j3parker
Copy link
Member

This comes up when checking arguments to a function with a [Immutable]T generic type parameter, for example:

void Foo<[Immtuable]T>( T t ) { /* ... */ }

// ...

var t = new { y = 3 };

Foo( t );

Properties of anonymous types are read only, so in principle the caller here can assume the good invariants when calling Foo (t.y will be 3 after the call returns, etc.) and Foo can assume good things about t (its not going to change concurrently behind the scenes.)

It gets "tricky" because you can do nesting like this:

new {
  x = new {
    y = new {
      z = 3
    }
  }
}

Implementing this wouldn't touch ImmutableDefinitionChecker (because there is no definition to check for anonymous types). The argument analyzer (the new one) goes straight to ImmutabilityContext anyway, which is good. Right now ImmutabilityContext is judging immutability based on basic things like "does it have the attribute (and therefore was checked by ImmutableDefinitionChecker at some point)?" or "is it in the list of things we've assumed by hand?", or "is it a kind of type (like an enum) that we know is always good?" etc.... so if we added anonymous types we'd need to iterate over the properties, and possibly recurse.

@j3parker
Copy link
Member

Just FYI, I moved this from bug to enhancement because its not a bug in the sense that our analyzer is doing something wrong. There are two ways to understand our analyzers behaviour:

  • soundness: do we ever allow "bad" programs?
  • completeness: do we allow all "good" programs?

("good"/"bad" are wrt the assumptions we get to make in our code when we use [Immutable].)

We want to be sound modulo some agreed upon exceptions (like we assume no reflection shenanigans, we trust the Audited attributes, etc.). Any places we are unsound are bugs.

We would like to be as complete as possible but we are willing to make trade-offs here. In a broad sense we have no hope of being complete, so we want to be "complete enough" to get on with our lives/allow developers to write code naturally. Being more complete is mostly an "enhancement".

I only care about the labelling because we do have a couple of soundness bugs :)

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

No branches or pull requests

2 participants