The intent of this approach is to eliminate recursion limits as a barrier for the parser. While it may not be urgent to address, I want to avoid pouring effort into a parser approach that we don't think will be usable long-term.
Right now this is passing a minor set of tests. It's intended to be enough to show how I'm thinking about flow control for the parser. I'm manually switching back and forth because it seemed like the easiest approach that avoids duplicating tests.
This starts adding builtins with TypeType and IntegerLiteralType. Note, structurally that's all they are, and not directly accessible in any way.
Adds a type field to SemanticsNode. Now, IntegerLiteralType can be identified as having type=TypeType, and IntegerLiteral as type=IntegerLiteralType. The current iteration doesn't do anything for type propagation, because I wanted to avoid making this too big.
This also switches the Identifier IR to instead BindName, with some side-effects. I'd been trying to think how to provide a name for TypeType, and switching around how things worked seemed like a better approach. And while I think it's the right direction (e.g., alias should just be a BindName), I also realized I don't need to name TypeType: there's probably a keyword to refer to the builtin, so it shouldn't use regular name lookup.
In particular, this means that a type can implement `ImplicitAs(Type)` and have values of that type behave like types.
This implies that `()` and `{}` are no longer types. They are now values whose type is the result of converting `()` or `{}` to type `Type`, as has been discussed recently and seems to be the supported direction. This fixes various cases where these types were previously mishandled.
When resolving a constraint type, compute the fixed point of the specified rewrite constraints, as requested by @josh11b in review of #2173 and tentatively agreed as our direction. If no such fixed point exists, detect that situation and diagnose the problem.
The algorithm used here is to iteratively apply all rewrites to each rewrite constraint until either one of them refers to itself, which indicates there's a cycle in the rewrite graph, or the set of rewrites converges.
This algorithm has some pathological inputs in which the runtime can grow exponentially in the size of the input (indeed, the fully-rewritten set of rewrites can grow exponentially in the size of the input, so this is unavoidable), so an iteration limit is also provided.
As I'm looking at adding builtins, it feels a bit like the types of args will end up scaling close to the number of semantic kinds. Rather than having this lead to a bunch of macros, this approach drops the macros for plain code.
Note, this adds Get functions too -- I'd planned those regardless for type safety, this is just getting ahead of the issue so that the Print function is pretty clean.
As a step towards builtins, provide more blocks. The intent is that any significant scope change will become its own NodeBlock. Builtins should produce the first set of node blocks.
Note, SemanticsIR as set up here isn't handling ordering of import processing -- I haven't thought that through much beyond that we probably want some lighter-weight processing of the parse tree to achieve it. But I think the essence of loading builtins first as their own IR block is... probably right?
I'm thinking about how to handle multiple files, and I think the current IRFactory is useful as a file-focused thing. So shifting/renaming accordingly. (doing this in its own PR to make the history a little cleaner for git's move detection)
Within the type of an associated constant, references to `.Self` should resolve symbolically to that associated constant as a `GenericBinding` so that it can be substituted for the actual value when using its type. However, when the associated constant is referenced from elsewhere in the same interface, the value we want is a symbolic value naming the constant as a member of `Self`.
Prior to this change, declarations like `let N:! X(.Self) where .(X(.Self).Y) == 5;` have the surprising behavior of the two `.Self` expressions resolving to two different symbolic values. Fix this by forcing the inner one to have the same symbolic value as the outer one, albeit with a different type.
* fix issue-1392
* run pre-commit
* run pre-commit
* Update explorer/interpreter/type_checker.cpp
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
* change test file
* change test file
* remove size check
* Update explorer/interpreter/type_checker.cpp
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Co-authored-by: m new <michael.burzan@outlook.de>
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Instead of checking and resolving rewrites eagerly, defer doing so until a constraint is applied to a binding. Actual resolution of rewrites is not yet implemented, so this is mostly just refactoring, but the eventual goal is to support things like `(Constraint where .T = .U) & (Constraint where .U = V)` (however they are reached) by applying one rewrite to the other, as agreed with @josh11b in discussion of #2173.
One nice consequence is that substitution into a constraint type no longer has a failure path, so substitution is once again not able to fail.
Mostly wanted to more clearly separate the closing observations from the
rest of the report. Noticed the missing link while I was there so fixing
that too.
This is intended to be used only when creating source locations that are known to be ignored because they are fed into operations whose diagnostics are discarded and that do not store the location in any created object.
As requested in review of #2321.
Per recent discussion, in `... where .A = B`, require that `B` is implicitly convertible to the type of `A` immediately, rather than treating that as part of the criteria that a type must satisfy to satisfy the resulting constraint. Extend the implementation of implicit conversion so that conversion of a type to a constraint checks that the type satisfies the constraint.
As part of implementing this, stop duplicating rewrite constraints as equality constraints. Instead, when checking that a constraint is satisfied, check both its equality constraints and its rewrite constraints. This fixes an infinite recursion that would otherwise be caused by this change, and is also a necessary prerequisite for applying rewrite constraints to equality constraints, where we would otherwise collapse the implied equality constraints to a tautological `V == V` constraint.
In passing, make ErrorBuilder support building the error message more incrementally and use that to improve diagnostics for mismatched values with equality constraints.
This rewrites semantics towards a more pure instruction model, in pursuit of the simple instruction-style output.
I think I can get this approach to type-check as it goes along, but obviously this change doesn't prove that yet. I'm separating it out because it's a large rewrite of the semantics structure, tossing out a lot of what was there before. But I think it does help towards several requests, like setting up a clear path for consolidating duplicate identifiers and making the node style more standardized.
I expect to need to pass multiple args to function calls, that'd probably be storing vectors of args similar to how I'm showing identifiers and integer literals stored.
This removes the semantics namespace because (a) it was getting annoying writing the `::` everywhere, and (b) I think the leaning with Carbon is to avoid namespaces (@chandlerc asked not to put SemanticsIR/SemanticsFactory in a namespace, which is the crux of the issue). But, it's still necessary to avoid name conflicts so I just prefix everything with "Semantics" (still a lot of typing, but no `::`).
Note, not trying to address every last error, just some obvious/easy ones.
```
/usr/local/google/home/jperkins/dev/carbon-lang/common/string_helpers.cpp:200:13: warning: prefer transparent functors 'less_equal<>' [modernize-use-transparent-functors]
auto le = std::less_equal<const char*>();
^
/usr/local/google/home/jperkins/dev/carbon-lang/toolchain/semantics/nodes/function.h:22:45: warning: pass by value and use std::move [modernize-pass-by-value]
Function(ParseTree::Node node, NodeId id, llvm::SmallVector<NodeRef> body)
^
/usr/local/google/home/jperkins/dev/carbon-lang/explorer/ast/declaration.cpp:230:14: warning: static member accessed through instance [readability-static-accessed-through-instance]
return cast<SelfDeclaration>(declaration).name();
^
/usr/local/google/home/jperkins/dev/carbon-lang/explorer/interpreter/type_checker.cpp:1104:33: warning: std::move of the variable 'impl' of the trivially-copyable type 'ConstraintType::ImplConstraint' has no effect [performance-move-const-arg]
impl_constraints_.push_back(std::move(impl));
^
/usr/local/google/home/jperkins/dev/carbon-lang/explorer/interpreter/type_checker.cpp:1145:36: warning: std::move of the variable 'rewrite' of the trivially-copyable type 'ConstraintType::RewriteConstraint' has no effect [performance-move-const-arg]
rewrite_constraints_.push_back(std::move(rewrite));
^
/usr/local/google/home/jperkins/dev/carbon-lang/explorer/interpreter/type_checker.cpp:1156:32: warning: std::move of the variable 'context' of the trivially-copyable type 'ConstraintType::LookupContext' has no effect [performance-move-const-arg]
lookup_contexts_.push_back(std::move(context));
^
/usr/local/google/home/jperkins/dev/carbon-lang/explorer/fuzzing/fuzzverter.cpp:74:42: warning: argument name 'trace' in comment does not match parameter name 'parser_debug' [bugprone-argument-comment]
/*trace=*/false);
^
./explorer/syntax/parse.h:19:17: note: 'parser_debug' declared here
bool parser_debug) -> ErrorOr<Carbon::AST>;
^
/usr/local/google/home/jperkins/dev/carbon-lang/explorer/fuzzing/proto_to_carbon_test.cpp:34:55: warning: argument name 'trace' in comment does not match parameter name 'parser_debug' [bugprone-argument-comment]
const ErrorOr<AST> ast = Carbon::Parse(&arena, f, /*trace=*/false);
^
./explorer/syntax/parse.h:19:17: note: 'parser_debug' declared here
bool parser_debug) -> ErrorOr<Carbon::AST>;
^
/usr/local/google/home/jperkins/dev/carbon-lang/explorer/fuzzing/proto_to_carbon_test.cpp:42:41: warning: argument name 'trace' in comment does not match parameter name 'parser_debug' [bugprone-argument-comment]
&arena, f, source_from_proto, /*trace=*/false);
^
./explorer/syntax/parse.h:25:59: note: 'parser_debug' declared here
std::string_view file_contents, bool parser_debug)
^
/usr/local/google/home/jperkins/dev/carbon-lang/explorer/syntax/parse_test.cpp:27:60: warning: argument name 'trace' in comment does not match parameter name 'parser_debug' [bugprone-argument-comment]
ParseFromString(&arena, "file.carbon", FileContents, /*trace=*/false);
^
./explorer/syntax/parse.h:25:59: note: 'parser_debug' declared here
std::string_view file_contents, bool parser_debug)
^
/usr/local/google/home/jperkins/dev/carbon-lang/migrate_cpp/cpp_refactoring/var_decl.cpp:57:58: warning: string concatenation results in allocation of unnecessary temporary strings; consider using 'operator+=' or 'string::append()' instead [performance-inefficient-string-concatenation]
segments.push_back({type_loc_class, qual_str + " " + range_str});
^
/usr/local/google/home/jperkins/dev/carbon-lang/explorer/fuzzing/ast_to_proto_test.cpp:105:55: warning: argument name 'trace' in comment does not match parameter name 'parser_debug' [bugprone-argument-comment]
const ErrorOr<AST> ast = Carbon::Parse(&arena, f, /*trace=*/false);
^
./explorer/syntax/parse.h:19:17: note: 'parser_debug' declared here
bool parser_debug) -> ErrorOr<Carbon::AST>;
^
```
In summary - some of the changes here are focused on producing the same test results for SemanticsIR, but I think the next step will be to change the SemanticsIR structure to reduce how much is added to the traversal stack.
Switching semantics to a postorder traversal is intended to be more efficient. The traversal stack is to eliminate risk of recursion limits within the semantic analysis that could come from layered code structures. However, we need to start considering the implications for type-checking and what the ParseTree looks like, as well as copying of data here.
As we start thinking about type-checking in SemanticsIR, it's helpful for a function to know its own signature in order to perform lookup recursive calls. The challenge in the post-order walk without this change is it doesn't know it's in a function definition (or similar) until it reaches the FunctionDeclaration; this restructures so that either:
1. For a declaration, the signature is a child of FunctionDeclaration(";")
2. For a definition, the signature is a child of FunctionDefinitionStart("{") which pairs with FunctionDefinition("}"), replacing CodeBlock.
This similarly reorients CodeBlock to be CodeBlockStart("{") as the first child of CodeBlock("}"). I'm not doing that with ParameterList here just because it affects a bit more, and felt like it could be delayed.
Overall, my goal is making the postorder traversal more intuitive along scope boundaries. I think we may also not need subtree_size, so I'm avoiding use of that now.
Currently the SemanticsIRFactory implementation is less clean than I might like (there are a couple comments to this point), but I was starting to feel like a more complete rewrite would be appropriate rather than trying to clean it up further: in particular, I think the node structures are off, but changing them is significant and also changes test output; in turn it may also warrant more substantial ParseTree changes. If you prefer from a reviewer POV, I can do a more complete rewrite.
This does some more work to the run_clang_tidy.py wrapper script, and runs an example pass.
"again" because it's really the proto fuzzer changes that broke it, it had been working before.
"mostly" because there's still an issue within the proto fuzzer that it can't find "port/protobuf.h", i.e. https://github.com/google/libprotobuf-mutator/tree/master/port, but I'm still hesitant to add an include path there.
We don't seem to have any need for `TypeOf*Type` types, and having them introduces the temptation to use them during type-checking, which would lead to types having different behavior when their type-of-type is `Type` versus when it's a more precise type. Remove these types for now.
If we later decide that we want each type literal to have a unique type, as we do for value literals, we can introduce a single value kind for that rather than one for each kind of type.
No changes to `TypeOfMemberName`, `TypeOfParameterizedEntityName`, and `TypeOfMixinPseudoType`, which are placeholders, not real types.
Provides a documented cadence, publishing target, and template for Code of Conduct and moderation transparency reports.
Based on the initial proposal draft from @CelineausBerlin.
Co-authored-by: Céline Dedaj <CelineausBerlin@users.noreply.github.com>
In particular, if we find the name of an interface after `is`, it should
be exapnded to a constraint, including any implied constraints such as
extended interfaces, rather than forming a constraint requiring only
that interface in isolation.
Add checking that a type only satisfies a constraint if it satisfies all of that constraint's equality and rewrite constraints.
Enforce the rule that `=` must be used in impls when specifying associated constant values rather than `==`.
Turn off the pre-#2173 single-step equality behavior. This is getting somewhat ahead of the approved design, but it's a one-line change to restore the old behavior.
Fix `CARBON_CHECK` to handle top-level `,`s in its argument, such as may happen in template argument lists, as this change introduces such a check.
llvm::InitLLVM starts getting stacks on assertion errors. Tieing errs to outs causes outs to be flushed when errs is used (avoiding munging out incompletely flushed output).
LLVM_SYMBOLIZER_PATH is required if `llvm-symbolizer` isn't in the developer's PATH. This sets it via bazel instead of having a developer handle it. I noticed this because the apt install of clang doesn't put llvm-symbolizer in the PATH.
I'd like to make this the default without putting it everywhere, but I don't see a way to do this intrinsically through [the toolchain](https://bazel.build/docs/cc-toolchain-config-reference), and the [rules_cc/defs.bzl](https://github.com/bazelbuild/rules_cc/blob/main/cc/defs.bzl) remains a thin wrapper around the native cc_binary.
Since I'm adding another env, it seems undesirable to have the macos asan workaround separate. As a consequence, this merges it in. Note bazel doesn't support merging a dict and a select, so it's also necessary to have the two env vars at least mildly aware there's something up (and this could get worse if we end up having more selects).
This had already hit one bug in LLVM, and now I've hit another:
https://github.com/llvm/llvm-project/issues/58385
Since this isn't specific to a config, and it seems to be a clear sign
that this isn't the best tested path, let's just drop to the more fully
tested flags. Probably should have done this rather than the more
targeted workaround last time...
This switch is being done in order to make it easier to update tests when the parse tree structure changes. I've been spending a lot of time doing such updates as part of refactoring the parse tree, and I expect more, so that's really at the root of all the refactoring I've been doing to lit updates.
There are lots of ways a declaration can be used before we have the information necessary to handle that use. Issue diagnostics for these.
Interleave declaration and type-checking of global declarations so that declaring a later declaration can depend on the results of type-checking an earlier one.
Incorporates tests added in #2266.
Fixes#1394, fixes#1395, fixes#1396.
Co-authored-by: pmqtt <51272730+pmqtt@users.noreply.github.com>
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Adds a simple script to merge stdout/stderr and put on labels. This is hidden to the RUN line using lit.cfg.py.
This is my solution to addressing how errors printed by the toolchain break sorting of stdout output; just put stdout first. We could also have toggles for interleaving output or such, which might help test whether we do it properly.
This also moves some previous-distributed replacement logic into lit_autoupdate_base.py: I think having that adjacent to lit.cfg.py is probably the better choice, and it reduces duplication in toolchain scripts. It happens here because I need to change the resulting commands to include the merge.
This change left a test broken and removed a fair amount of test coverage for unrelated features, and after further discussion it's not clear that this is the direction we want to go in.
This reverts commit 6cf2272bbe.
Jon Ross-Perkins
Richard Smith
Adrien Leravat
Chandler Carruth
