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

Restricted pattern kinds allowed for a variable definition #1306

Closed
wants to merge 1,004 commits into from
Closed

Restricted pattern kinds allowed for a variable definition #1306

wants to merge 1,004 commits into from

Conversation

pk19604014
Copy link
Contributor

@pk19604014 pk19604014 commented Jun 1, 2022

Fixes #1301

arena.h change allows to support types with private constructors.

josh11b and others added 30 commits January 7, 2022 15:54
This is a proposal to adopt interfaces as the single static open extension mechanism for things like selecting how operators are implemented for types.

Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Allow `var <identifier>: auto = <expression>;` syntax.

Co-authored-by: Richard Smith <richard@metafoo.co.uk>

Co-authored-by: josh11b <josh11b@users.noreply.github.com>
This enables us to stop using `Env` in the typechecker. As a byproduct, this commit also restructures the interpreter to handle run-time global initialization as part of ordinary execution, using the Action stack.
Clarify class declaration syntax

Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Co-authored-by: Jon Meow <46229924+jonmeow@users.noreply.github.com>
…ventions/string_literals.md. (#1028)

* Initial implementation of block string literals following lexical_conventions/string_literals.md.

Enabled yyinput() in flex to implement parsing.
Added ParseBlockStringLiteral() helper to handler further transformations such as indenting.
Modified formar_grammar to support single-quoted strings to prevent a failure on lexer.lpp.

* Fixed _find_string_end quote parameter type int -> str.

* Update executable_semantics/syntax/BUILD

Co-authored-by: Geoff Romer <gromer@google.com>

* Addressed code review comments - split table-drived test into individual tests, renamed constants to match style guide.

* Addressed code review comments -- using EXPECT_THAT_EXPECTED() in tests, lexer comments and code cleanup.

Co-authored-by: Geoff Romer <gromer@google.com>
Allowing interfaces to define default values for its associated entities. This:

-   Helps with evolution by reducing the changes needed to add new members to an interface.
-   Reduces boilerplate when some value is more common than others.
-   Addresses the gap between the minimum necessary for a type to provide the desired functionality of an interface and the breadth of API that user's desire.

As an alternative, final values can be provided instead, which can't be overridden, but are more predictable for users and may avoid dynamic dispatch overhead in some cases.

Example:
```
// Interface parameter has a default of `Self`
interface Add(Right:! Type = Self) {
  // `AddWith` *always* equals `Right`
  final let AddWith:! Type = Right;
  // `Result` has a default of `Self`
  let Result:! Type = Self;
  fn DoAdd[me: Self](right: Right) -> Result;
}

impl String as Add() {
  // Right == AddWith == Result == Self == String
  fn DoAdd[me: Self](right: Self) -> Self;
}
```

Co-authored-by: Richard Smith <richard@metafoo.co.uk>
3.0.0 is incompatible, this is a quick fix
Co-authored-by: Richard Smith <richard@metafoo.co.uk>

Co-authored-by: Geoff Romer <gromer@google.com>
This improves encapsulation, and makes Interpreter lifetimes clearer (and shorter).

Co-authored-by: Jon Meow <46229924+jonmeow@users.noreply.github.com>
git rm third_party/llvm-project
git subtree add --prefix third_party/llvm-project https://github.com/llvm/llvm-project.git dbf0d81 --squash

With some fixes for the compile.
Change the syntax for setting the associated constants and types in an interface implementation for a type from using `let` declarations as in:
```
class Vector(T:! Type) {
  impl as Iterable {
    let ElementType:! Type = T;
    ...
  }
}
```
to using `where` clauses as in:
```
class Vector(T:! Type) {
  impl as Iterable where .ElementType = T {
    ...
  }
}
```

This is an attempt to simplify by removing redundancy, improve consistency by removing a use of `let` that was different than other examples, and better support forward declaration that a type implements an interface while retaining the information needed for type checking.

Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>

Co-authored-by: Richard Smith <richard@metafoo.co.uk>
-   For idiomatic Carbon code:
    -   `UpperCamelCase` will be used when the identifier can be resolved to a
        specific value at compile-time.
    -   `lower_snake_case` will be used when the identifier's value won't be
        known until runtime, such as for variables.
-   For Carbon-provided features:
    -   Keywords and type literals will use `lower_snake_case`.
    -   Other code will use the guidelines for idiomatic Carbon code.

Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Co-authored-by: josh11b <josh11b@users.noreply.github.com>
I'm hoping this improves debugability.
pk19604014 and others added 5 commits June 2, 2022 14:04
@pk19604014 pk19604014 marked this pull request as ready for review June 2, 2022 19:15
@pk19604014 pk19604014 requested a review from a team as a code owner June 2, 2022 19:15
@pk19604014 pk19604014 requested a review from josh11b June 2, 2022 19:16
… names during type checking (#1305)

Sample trace output:

```
checking tuple pattern (v1: T1, v2: T2), expecting (T1:! ImplicitAs(U1), T2:! ImplicitAs(U2))
checking binding pattern v1: T1, expecting T1:! ImplicitAs(U1)
checking expression pattern T1
checking identifier expression T1
```
Comment on lines 114 to 126
static auto Create(Nonnull<Arena*> arena, SourceLocation source_loc,
Nonnull<Pattern*> pattern, Nonnull<Expression*> init,
ValueCategory value_category)
-> ErrorOr<Nonnull<VariableDefinition*>> {
if (pattern->kind() != PatternKind::BindingPattern &&
pattern->kind() != PatternKind::TuplePattern) {
return Error(
"Expected a binding pattern or a tuple pattern for variable "
"definition");
}
return arena->New<VariableDefinition>(source_loc, pattern, init,
value_category);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, should we really be doing these kinds of checks here? I think it would be reasonable for the VariableDefinition constructor to assert these conditions, but it seems to me that enforcing them and producing diagnostics shouldn't be a responsibility of VariableDefinition itself and should be done at a different level. Eg, if this is effectively a syntactic check, then the parser should be doing it before constructing the VariableDefinition. And if it's a semantic one then the type-checker should be doing it later.

In this case, I think this is a semantic check that the type-checker should be doing when type-checking an irrefutable pattern. Also note that this check should be applied recursively (it's not OK to have a refutable pattern inside a tuple pattern, for example). Perhaps the best thing to do would be to add some code to the type-checker to check whether a pattern is irrefutable. This would also be useful for the existing IsExhaustive check, which currently looks only for a BindingPattern but should (at minimum) really be looking for any irrefutable pattern.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, Richard.

I was not able to express the needed logic based solely on [ir]refutability of the involved patterns, for example, I needed to make a distinction between being inside (on the rhs of the) BindingPattern.

I put in a tentative implementation of variable pattern structure checking logic in a new function, CheckVariableDefinitionPattern(), and reverted parser changes. PTAL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, Richard.

I was not able to express the needed logic based solely on [ir]refutability of the involved patterns, for example, I needed to make a distinction between being inside (on the rhs of the) BindingPattern.

I put in a tentative implementation of variable pattern structure checking logic in a new function, CheckVariableDefinitionPattern(), and reverted parser changes. PTAL

pk19604014 and others added 14 commits June 2, 2022 16:34
This is not part of the approved design and gets in the way of
supporting `where` expressions, which are part of the approved design.
Some review feedback from outside the team directly working on Carbon suggested
two pretty significant updates here. First, we didn't do a good job of
motivating Carbon. This takes two parts, first explaining what we'd like to
accomplish with this approach generally, and second explaining why alternative
approaches don't work. A particularly difficult case here is articulating
effectively the difficulties that motivate an approach other than improving C++
incrementally.

Co-authored-by: Jon Meow <jperkins@google.com>
Co-authored-by: josh11b <josh11b@users.noreply.github.com>
This is for https://google.github.io/styleguide/cppguide.html#TODO_Comments; FIXME seems to be getting used for the same purpose, and we should probably only have one so that it's easier to scan for.
Add scaffolding for constraints in general, and support specifically constraints formed by applying a `&` operator.

No support for constraints formed with `where` nor for named constraints at this point.
Tiny nit: Line 63 similarly links "modern generics system". Line 250 links 'generics', but doesn't include "modern ... system." So I think this approach is consistent (and subtly, implies the link is about the modern ... system, not just "generics").
* added documentation for tracing output, changed the printing of scopes to be part of the stack, made other minor changes hoping to enhance readability

* reformat a list in the README

* minor edit

* fix the lists

* updates to the interpreter/README.md
The syntax permitted in an `interface` is similar to, but different from, that permitted elsewhere. For example, associated constant declarations (`let T:! Type;`) with no initializers are valid in interfaces but nowhere else, and are a different kind of declaration from what a normal `let` declaration would produce both syntactically and semantically.

Because interfaces and impls permit only a very small subset of the complete set of kinds of declaration, it's simpler to list only the kinds that are permitted rather than to allow an arbitrary declaration and semantically disallow the invalid cases, and it's also likely to lead to better error messages as we won't try to parse unintended or meaningless things.
Build constraint types from `where` expressions. This is very bare-bones; there's no support for constraining `.Self` or associated types yet, but degenerate cases not involving self-reference work.

`==` constraints are syntactically supported but not semantically verified yet, and they aren't used for anything.
@pk19604014 pk19604014 requested review from zygoloid June 9, 2022 19:35
@chandlerc chandlerc added the explorer Action items related to Carbon explorer code label Jun 15, 2022
@pk19604014 pk19604014 requested a review from a team as a code owner June 28, 2022 21:22
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
explorer Action items related to Carbon explorer code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

explorer crashes on var auto = i32; construct
9 participants