When declaring a class (or interface), we create a scope that covers the
entire class declaration. If the class was declared in a lexical scope,
we would declare the class name in the innermost scope, which was the
class's own scope instead of the enclosing lexical scope.
Fix this by instead adding the name to the lexical scope at the start of
the class declaration, not the lexical scope created to hold the class.
For now, we reject if the class name would have been shadowed by a name
that has already been declared within its scope, such as a generic
parameter, so we only ever need to modify the end of the list of lexical
lookup results for the class name.
This appears to be sufficient to make local declarations and definitions
of classes and interfaces work properly throughout check, though testing
is pretty minimal so far.
With this change, we now support impl of interfaces with non-function
associated constants.
Also:
* Make impl diagnostics use more consistent names
* Make some impl tests "no_prelude"
Still to do:
* Facet type resolution as a separate, reusable step
* Using the assigned values of associated constants (see
`fail_todo_use_assoc_const.carbon`)
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Include the index rather than the name in the fingerprint of a symbolic
binding. While both the index and the name contribute to the canonical
identity, using either one of them in the fingerprint is sufficient to
ensure that distinct entities get different fingerprints. Changing the
name of a symbolic binding should ideally not result in fingerprint
changes, so exclude the name from the fingerprint when we have an index.
Use the canonical type and constraint when fingerprinting an impl, so
that uses of names in `name_ref` instructions aren't considered, only
the entity the name resolves to, and different ways of spelling the same
type have the same fingerprint. This similarly allows compatible changes
to be made to impls without changing the fingerprint.
Exclude the declaration block when determining the fingerprint of a
declaration. The declaration block contains the declarations of
parameters of the declaration, which do affect whether two declarations
are identical, but not whether they denote the same entity, because it
would be invalid to have different declaration blocks for declarations
with the same name in the same scope. Therefore changes to the
declaration block are compatible, and it's useful for such changes to
not affect the fingerprint.
This is not easy to test in isolation with our current testing
machinery. However, a follow-on PR will change the name of a parameter
in the prelude, and with this in place, will not cause any changes to
occur elsewhere in the toolchain tests.
This change splits `NodeKind::IdentifierName` into separate node kinds
depending on whether the identifier is followed by parameters, and
similarly splits `NameQualifier` based on whether the qualifier has
parameters. This enables us to only push a pattern block when it's
actually needed, rather than "defensively" pushing one when it might be
needed.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
This change deliberately breaks away from the line/column ordering, and
instead focuses on a last byte offset corresponding to the final token
processed as part of producing the message. Where that's equal, this
maintains stable ordering in order to reflect the order that diagnostics
were produced.
The intent of this approach is that lex, parse, and check diagnostics
are interleaved based on where they are produced, but that
subexpressions still have diagnostics emitted prior to containing
expressions. In particular, the prior line/column sort essentially
sorted on the _start_ of where a diagnostic was associated, and this is
closer to sorting based on the _end_. As a consequence, something like
`F(1 2)` will have the error for `1 2` emitted _before_ a diagnostic for
`F(1 2)` not matching parameters, instead of _after_.
In check, we track the last handled node. This provides a
last_byte_offset _separate_ from where a diagnostic is associated. The
intent is that this creates an ordering of diagnostics which may be
associated with earlier code, to cause the diagnostics to be emitted
later. An example consequence of this is the change in ordering of
modifier diagnostics: we are diagnosing those from the same place, but
they have the same last_byte_offset, so we print them out in the order
produced.
I've added similar tracking to parse, but cannot identify any test which
is affected by it (note the separate commit, I thought about this late).
I'm not sure whether we have good out-of-order errors we could produce
for this.
A significant number of tests have reordered diagnostics as a
consequence of this change, so this change does not add further testing.
These don't fully work in check and beyond yet, because they're not
added into lexical lookup, but already mostly do the right thing.
Per #3407, disallow namespace declarations anywhere other than at file
scope for now.
We don't treat statements starting with a packaging introducer keyword
(`package`, `library`, `import`) as declarations because they're
sufficiently unlikely to occur that the error recovery doesn't seem
important, and this avoids needing to disambiguate `package.` at the
start of an expression.
Just noticed the versions were old while I was updating tcmalloc in
#4784. I've tested and it doesn't seem to introduce issues; the compile
commands may actually work a little better.
Bazel's WORKSPACE file is deprecated, and we haven't worked on the woff2
example in ages. Rather than investing time into keeping it around,
remove it and we can revive it when we're ready.
Both of these I noticed from testing #4785, but they occur at head.
```
(elided)/execroot/_main/common/raw_hashtable.h:532:40: error: do not use nested 'std::max' calls, use an initializer list instead [modernize-min-max-use-initializer-list,-warnings-as-errors]
532 | static constexpr ssize_t Alignment = std::max<ssize_t>(
| ^
533 | {alignof(MetadataGroup), alignof(StorageEntry<KeyT, ValueT>)});
| ~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| static_cast<long>(alignof(MetadataGroup)) static_cast<long>(alignof(StorageEntry<KeyT, ValueT>))
(elided)/execroot/_main/toolchain/install/busybox_info_test.cpp:259:8: error: unused local variable 'usr_prefix' of type 'std::filesystem::path' [bugprone-unused-local-non-trivial-variable,-warnings-as-errors]
259 | auto usr_prefix = MakeInstallTree(dir_ / "usr");
| ^
(elided)/execroot/_main/toolchain/install/busybox_info_test.cpp:260:8: error: unused local variable 'usr_local_prefix' of type 'std::filesystem::path' [bugprone-unused-local-non-trivial-variable,-warnings-as-errors]
260 | auto usr_local_prefix = MakeInstallTree(dir_ / "usr/local");
| ^
```
The std::max diagnostic seems a little confused, but the initializer
list seems like it can be dropped without any loss. The unused locals
diagnostic is correct.
Neither of these seem like they should be newer than my last clang-tidy
pass, maybe I just missed them in other sweeps.
The Function.h include was simply unused, I was partly dropping the bits
that would've depended on it. Protocol.h is used, but it should really
be included from handle.h.
This makes InstFingerprinter::GetOrCompute for InstId and InstBlockId
more similar, in that they just construct a Worklist and call Run.
The Worklist::Run method differentiates if the next todo item is an
InstBlockId and in that case it adds everything in the block to its
todo list and continues processing.
We only use the fingerprint of an InstBlockId if its at the bottom of
the todo stack, which is the case when it's placed there initially by
InstFingerprinter::GetOrCompute. We could cache it but we currently
do not. If we did, we could also cache other InstBlockIds found
inside instructions, but at the moment we skip through them and
add their instructions rather than adding the InstBlockId to the
todo stack.
Instead of including the raw index of the specific, which is unstable
across files and across unrelated changes, use a fingerprint of the
constant values of the specific arguments. This is a placeholder until
we decide on how we want to mangle specific functions.
The InstId in this field is to an instruction that declares the
function, rather than the function itself, so that diagnostics can print
where the function is coming from. The type of the function (the
FunctionType instruction) is the type (the type_id) of the
function_decl_id. So we rename the field to help make this distinction
more clear.
Followup to #4739
Without this, the merge queue blocks on results but they never get
triggered and show up. This will need to merge before we re-enable
`clang-tidy` enforcement on the trunk branch.
Change the implementation to use an explicit `is_poisoned` bit instead
of `InstId::PoisonedName` value.
Zero behavior change.
This would allow to more easily change the API to support accessing the
poisoning declaration so we can have better name poisoning diagnosis.
#4622
Add a builtin `"int.convert"` supporting unchecked conversions between
different integer types. This performs a truncation, zero-extension, or
sign-extension, depending on the widths of the operands and the
signedness of the source type. Add explicit `As` support to the prelude.
No implicit conversions are supported yet as we don't have a way to
express the constraint that we can only implicitly convert to wider
types.
I'm trying to make the LSP look more like the rest of the toolchain. I'm
trying to separate the handlers from the transport layer, and remove the
multiple inheritance aspect. Also fixing some style issues, switching to
`Map`, and removing an unnecessary copt.
I'm using `Context` for the central object for consistency with other
portions of the toolchain. In order to get the `handle_*` files working,
I'm using LLVM's registry class. It has a quirk that I can't register
two registries in the same cpp file, so there are two one-line cpp
files.
In order to help show the delta (or lack thereof) for actual
implementation, I've copied server.cpp over handle_* and undone that in
two commits. See the third and fourth commits on the PR history for
that.
Hi,
I fixed a small issue that I found inside the "compile subcommand"
component:
The program can be crashed by running ```bazel run -- toolchain:carbon
compile --dump-mem-usage "non-existing-file.carbon"``` - i.e. by
activating the memory usage dump flag and passing a non-existing file.
Best regards,
oz
* Implement ignoring the difference between `Self as` and `as`, as well
as `where` clauses at the end of an `impl` declaration when checking
whether `impl` declarations match, from #3763.
* Allow impl declarations with different constraint ids to match, as
long as the facet type of the constraint has the same interface_id and
specific_id.
* Add some TODOs reflecting future facet type resolution.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Uses bazel 8 flags since there's the update in #4729. bazelisk was used
to generate the list of flags (see bazelrc comment). The overall
approach is trying to do our best to follow
https://bazel.build/release/backward-compatibility, in particular since
`--incompatible_strict_action_env` had come up (essentially just
adopting as much as we can now).
The default visibility changes in `carbon_rules/BUILD` and
`cc_toolchains/BUILD` files are for
`--incompatible_config_setting_private_default_visibility`. That's
enough for fastbuild, but we use tcmalloc in opt, and it has an issue.
In `clang_toolchain.BUILD` it's for
`--incompatible_check_visibility_for_toolchains`, even though
`rules_shell` then breaks on it. `manifest/defs.bzl` changes are for
`--incompatible_disable_target_default_provider_fields` which
`rules_pkg` breaks on. Even though these flags are off, I'm keeping the
changes since we should eventually enable the flags.
I'm still looking at the tree sitter rules due to the WORKSPACE issue,
but I think that needs more substantial work.
Use it in the instruction namer to make instruction names more stable
across unrelated changes to the toolchain or the prelude.
---------
Co-authored-by: Dana Jansens <danakj@orodu.net>
A collection of additional Carbon examples demonstrating current
language capabilities and some limitations of the current state of the
toolchain.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
In practice, I am unable to call constructors like `SemIR::InstId` from
a debugger. I first tried to use
https://clang.llvm.org/docs/AttributeReference.html#noinline to make
some non-inline calls to the constructor, but that didn't work:
```
toolchain/sem_ir/dump.cpp:245:23: error: 'noinline' attribute is ignored because there exists no call expression inside the statement [-Werror,-Wignored-attributes]
245 | [[clang::noinline]] return InstId(inst);
|
```
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
This updates to bazel 8.0.0, also updating bazel mod deps and tools to
make that function. The release is a couple weeks old, and we haven't
updated in a while, and it's a major release. Note it includes some
incompatible flag flips that this is trying to update with respect to.
I'll try generally enabling incompatible support separately.
The most visible bazel behavior change here will be the change from `~`
to `+` in repo path names. (If you're curious,
https://github.com/bazelbuild/bazel/issues/23127 indicates this fixes a
Windows performance issue)
Note that this is building on top of both the action env update in #4728
(which got me started down this path) and the proto removal in #4731
(which would add significant work to this update). Only the commit
starting at "Work towards bazel 8.0.0" is specifically part of this PR.
Follow-on to #4669 . Did some manual testing, but may have not exercised
all code paths.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
When creating a tuple or struct type object/value, we will walk each of
the tuple's or struct's parts, respectively, and Convert() each of them.
This allows (T, T) to convert to (U, U) and so forth. It also performs
the conversion from value to initialization even for the same types,
such as converting from a value of (T, T) to an object of (T, T).
Classes need to define their own conversions but when the target and
source types are the same, there is no conversion of types taking place.
If the class adapts a tuple or struct then walk each of the tuple's or
struct's parts, respectively, and Convert() each of them in order to
initialize the parts of the target.
For copyable types, the conversion implies a copy in the initialization,
and for non-copyable types, an error is emitted.
This supports copying a class value to a class object when it is an
adapter of a tuple or struct.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
This is caused by setting a temporary lambda to a `llvm::function_ref`.
The fix is to assign the lambda into a variable that outlives the
`llvm::function_ref`.
Imported declarations that are contained within a generic, such as an
`impl forall...` would be given an abstract symbolic constant value
instead of a concrete generic value in some cases where the function was
used in an api file and impl file of the same library. This caused #4679
to fail using `ImplicitAs.Convert` transitively imported from the
prelude.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
* Change `InterfaceWitness` -> `ImplWitness`
* Include a `SpecificId` in the `ImplWitness`. This allows the
`InstBlock` it contains to have its own identity, allowing it to be
changed as the impl is processed. Evaluation only updates the specific.
* Create the `ImplWitness` at the start of the impl definition. In the
future, this will be populated with the values of non-function
associated constants. For now, it starts full of invalid instruction
ids.
* Implements the model suggested in #4672 .
Note that the non-SemIR testdata changes are to these file:
* `toolchain/check/testdata/impl/lookup/fail_todo_undefined_impl.carbon`
* `toolchain/check/testdata/struct/import.carbon`
* `toolchain/check/testdata/tuple/import.carbon`
The last two are due to an import of generics bug exposed by this PR,
which will be fixed in a follow-on.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Fixes integer builtins to produce the correct values (and not
CHECK-fail) when used on integer literals. Also adds impls to the
prelude to use the new builtins to perform operations on integer
literals.
Perhaps most importantly, this allows directly initializing `i32` values
with negative numbers, as the negation operation on integer literals now
works.
For testing I've added tests for use of literals with one operator in
each class (addition, multiplication, ordering, bitwise, etc) for which
there are distinct rules or overflow behavior, rather than exhaustively
testing all the combinations. This is aimed at finding a good tradeoff
between maintainability of the tests and thorough test coverage.
Also fixes lowering of heterogeneous shifts and comparisons. These are
currently disabled when one of the operands is an integer literal, but
we may want to allow that when the integer literal operand has a known
constant value.
Avoid misaligned loads from StaticRandomData in the size [4, 8] hashing
case. We can use aligned loads in this case for lower latency. We
introduce the SampleAlignedRandomData function for this purpose.
Richard Smith
Geoff RomerandJon Ross-Perkins
Dana Jansens
Boaz Brickner
Chandler Carruth
ottmar-zittlau
ezbr