Resolve the default access modifier to Public or Internal in the syntax tree

[bamarsha]

Updates access modifiers to be Internal | Public instead of DefaultAccess | Internal. In the unresolved syntax tree, a default access modifier is QsNullable.Null instead of DefaultAccess. In the resolved syntax tree, it's set to Public, which is the default. This way, AST transformations don't need to know what the default access is, and access modifiers can be compared using standard comparison operators like < or Min.

Requires microsoft/qsharp-runtime#481. Closes #667.

[cesarzc]

I am a bit hesitant on the rename from AccessModifier to Visibility. C# refers to them as access modifiers, F# as access control and other languages like C++ also use the term access modifiers so I think it makes sense to use the same term that other languages use.

Even though this is the compiler implementation and not documentation, I think it would be easier for both contributors and users of the compiler public APIs if we used a more familiar term.

I am not completely opposed to the rename but I would like to know what other people think. @ScottCarda-MS, @swernli, @bettinaheim do you have an opinion about renaming AccessModifier to Visibility?

[ScottCarda-MS]

I am a bit hesitant on the rename from AccessModifier to Visibility. C# refers to them as access modifiers, F# as access control and other languages like C++ also use the term access modifiers so I think it makes sense to use the same term that other languages use.

Even though this is the compiler implementation and not documentation, I think it would be easier for both contributors and users of the compiler public APIs if we used a more familiar term.

I am not completely opposed to the rename but I would like to know what other people think. @ScottCarda-MS, @swernli, @bettinaheim do you have an opinion about renaming AccessModifier to Visibility?

I agree with the sentiment @cesarzc made. It is better to use terms that are already out there and familiar to programmers. Additionally, changing the name is a breaking change that I don't think is particularly necessary here, although the behavior of the modifiers is changing in a breaking change anyway. Not changing the name would also make for a much smaller PR with much less code changes, which is always appreciated :) .

[bamarsha]

changing the name is a breaking change that I don't think is particularly necessary here

It's not strictly necessary, but (1) the removal of DefaultAccess and addition of Public, and (2) the removal of the Modifiers type are already breaking changes to the access modifier API, so the renaming is pretty minor comparatively.

I could call it Access instead. I just think AccessModifier is a particularly long and cumbersome name, even though I named it originally. :)

[swernli]

I don't have strong feelings about changing the name vs not, but I think that the name Visibility might be relying too much on colloquial terminology. I usually talk about a class' visibility but from the C# docs it seems they use the term "accessibility" which makes sense given that the accessibility is modified by the Access Modifiers (https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/access-modifiers). That said, "accessibility" as a noun always makes me think of designing products for use by the differently abled (https://en.wikipedia.org/wiki/Accessibility), so that brings me right back to AccessModifier as both accurate and descriptive without being confused with other concepts.

[cesarzc]

I like Sarah's suggestion of calling it Access. It is shorter than AccessModifiers and uses a term that people are already familiar with.

[bamarsha]

Renamed Visibility to Access.

[cesarzc]

Looks good to me.