As discussed around #3792, identify the import a diagnostic message came
from prior to the diagnostic message itself. This occurs during location
translation so that the logic can be central.
I'd considered associating the parse node with ImportRef instructions,
but I realized about halfway through that because I need to store the
ImportDirectiveId on the ImportIR for cross-package imports, it's there
for use in location translation without extra work. That saves a fair
amount of stringing it through declarations, as well as an oddity where
ImportRef instructions would have a node that didn't really represent
them.
The stream needs to be declared before the scoped exit (for flush in
particular). Trying to also adjust the code to make the issue clearer.
Caught by asan.
Rename `BoolValue::FromBool` to `BoolValue::From` as requested in #3816.
Add `BoolValue::ToBool`.
Convert existing code to use these where appropriate.
In preparation for adding more builtins, factor out the handling of
builtin function kinds into separate files.
Add checking for builtin function signatures. The mechanism used here is
intended to provide a lot of flexibility for declaring generic builtin
functions and pretty arbitrary constraints on the types of parameters of
builtin functions. For now, these constraints are checked when the
builtin function is declared. The hope is that this will suffice, but if
not, it should be straightforward to switch to doing some of the
checking on call and share logic between the checks.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
This is temporary: eventually per the design we should be forming
integer literals whose types reflect their values. But for now we should
ensure that values fit within their types.
This also fixes canonicalization of integer constants and hence of array
types, because we no longer have multiple different representations of
each `i32` value depending on the bit-width used for the literal.
Update broken links to aid in following links while perusing the
documentation. Closes#3778
I don't imagine I got every one of these right. Some 404s may be
reported due to permissions.
Using the lychee command in #3778, only the following link is reported
as not found, but it's just a permission issue:
```
[proposals/p1367.md]:
✗ [404] https://github.com/carbon-language/carbon-lang/settings/access | Failed: Network error: Not Found
```
I tried to get it down to as few as possible, partially so that future
link scans won't run into repeat errors.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
The purpose of the newline is to make it clearer where a given
diagnostic begins and ends, particularly as the first message of a
diagnostic may not be the error.
This is a trivial code change, but ripples edits through test files.
On #3792 it was requested to put context-related diagnostic messages
prior to the diagnostic error. In order to support that, remove the
distinction that an error needs to come first on DiagnosticMessage.
Since the addition of TranslateArg, I don't think this type is going to
go away (cutting a TODO). Refactoring names slightly to fit the current
role, and adding const to ConvertLocation.
- Add the `extern` keyword for forward declarations in libraries that
don't
provide the definition.
- Treat repeated forward declarations as redundant.
- Allow them when they prevent a dependence on an imported name.
- Clarify rules for when modifier keywords should be on forward
declarations
and definitions.
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
For now, a builtin function is defined by specifying a string literal
initializer in a function declaration:
```carbon
fn MyBuiltin(a: i32) -> i32 = "builtin.name";
```
End-to-end support is included for a sample `"int.add"` builtin
performing integer addition, covering constant evaluation and code
generation.
The implementation here needs substantial refactoring before we'll be
ready to start adding more builtins. That refactoring work will be
coming next. This change is aiming to checkpoint some incremental
progress.
This is to make it easier to locate various diagnostic types. I was
thinking to separately rename DiagnosticLocationTranslator to
DiagnosticTranslator, because it's now handling some type translations.
This is mostly keeping classes in one place, but FormatFn handling is
moved from DiagnosticMessage to DiagnosticEmitter for a cleaner
dependency graph where DiagnosticTypeForArg is concerned.
Fix a collection of issues that were preventing lowering for overloaded
operators from working.
Instead of creating `import_ref` instructions during name lookup in the
current block, whatever that might be, we now create them in the `file`
block always. This avoids inserting them into blocks that might not be
intended to contain them, such as functions, and avoids the IR generated
for a function depending on which names we happen to have looked up
first.
When importing a class, function, or interface, import its enclosing
scope ID. This is necessary to allow us to distinguish between functions
at interface scope, which shouldn't be lowered, and other functions, and
will also be used in future to provide qualified names for declarations
when printing types. In order to support this:
- Track the constant values of namespaces created during importing so
that we can find them when resolving an import ref. Use those constant
values to convert an enclosing scope ID from the imported IR into a
corresponding ID in the current IR.
- Change how we do two-pass import of classes and namespaces so that we
can do two-pass import even for non-defining declarations, so that we
can import the enclosing scope.
While working on the final point above, I reworked `TryResolveInst` to
return a flag indicating whether another pass is necessary instead of
implicitly encoding this in the `ConstantId`. This permits the handling
of classes to be simplified; now `import_ir_constant_values` is only
accessed in a single place.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Support is added for all overloaded operator interfaces in the current
design apart from `Assign`, which is going to require some more work to
properly handle, given that primitive assignment currently has a special
implementation for quite a few builtin types.
As we don't have support for generics yet -- in particular, generic
interfaces -- there is no support for `*With` interfaces, but homogenous
interfaces such as `Add` are supported instead.
Factor out building of call expressions so that overloaded operators can
generate calls.
Switch a few places from using specific kinds of NodeId to a general
NodeId. Because overloaded operators and other things like implicit
conversions can result in member access and function calls, those
operations can't require a specific kind of NodeId.
Add import support for associated entities, and fix import support for
interfaces and symbolic bindings. We now import interfaces in two steps,
first importing a forward declaration then a definition, just like we do
for classes. For symbolic bindings, we ensure that each BindSymbolicName
is imported only once, because its ID is used as its symbolic identity.
This is necessary because we (only) support operator interfaces that are
defined in an imported Carbon package for now.
The entire contents of `check/operator.cpp` should probably be
rethought. In particular, doing a lot of name lookups on each operator
is likely to be bad for performance. But this gets us to the point where
overloaded operators are basically working, which seems like a good
place to iterate from.
For now, the tests that the individual operators map to the right
interfaces are mostly generated by a script, but that's just because I'm
expecting a fair bit of churn in how we define the prelude and the
`impl`s -- in particular, when we add support for `AddWith`, we'll need
to update all the tests. The plan is to remove the script once things
settle down.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
Still needs more merge/redeclaration logic for import semantics, but
this felt like a reasonable point to send a PR.
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
This change implements the check behavior for the arrow operator.
`ptr->Foo()` is rewritten as `(*ptr).Foo()` and `ptr->(X.y)` is
rewritten as `(*ptr).(X.y)`
I wanted to choose one or the other. I think some code has been using
each from early on. We're predominately using `filename`, so
consolidating on that. This conveniently matches the [Google dev doc
style guide](https://developers.google.com/style/word-list#filename)
(which we use for docs) which says "filename: Not file name".
In toolchain:
```
╚╡git grep file_name . | wc -l
35
╚╡git grep filename . | wc -l
489
```
On the parsing side, we treat `a.(b)` as a member access whose second
operand is a `ParenExpr` rather than a `MemberName`. A new node category
is added for the union of `MemberName` and `ParenExpr` to support this.
Checking is mostly reusing the same pieces we already have for simple
member access. Compound member access is in most ways a simplified form
of simple member access because it doesn't need to do any lookup.
Note, I'm annotating the lookup partly so that the reason the conflict
comes up is clear, partly so that there's actually a diagnostic line
associated with the root cause as more tests get packed into a single
file.
Add a general substitution mechanism to support substituting symbolic
bindings with their values throughout symbolic constants and, more
specifically, types. This is done by decomposing the constant
instruction into its operands, substituting into the operands, and then
rebuilding the constant value by invoking the constant evaluator.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
Provide a general mechanism for determining the kind of the args of an
instruction. Use this to simplify instruction profiling a little. The
intent is to also use this mechanism as the basis of a substitution
mechanism, which will be part of a future patch.
Note that this causes us to do three table lookups and two indirect
calls in inst_profile per instruction, instead of one table lookup and
one indirect call. We can revisit this if it shows up in profiles.
I'd added these trying to catch cases where ImportRefUnused might be
returned, but I believe these are incorrect. Mistakes should still be
caught because an ImportRefUnused will lead to errors in other handling.
Removing the TryResolveImportRefUnused should clarify the semantics of
when an ImportRefUnused is expected.
This revamps the support for cross-package imports, making them look
more like a namespace. The planned model is mentioned on
[#toolchain](https://discord.com/channels/655572317891461132/655578254970716160/1217586076022210670).
This does not implement name lookup into the new namespace structure.
A few key changes in this PR (it's a little sprawling) are:
- Moves logic for adding package imports from context.* to import.*
- Remove SemIR::Import, which was the prior model. This is instead now a
SemIR::Namespace with the NameScope getting a new import_ir_scopes
field.
- Allow SemIR::Namespace to use Parse::ImportDirectiveId in addition to
the prior Parse::NamespaceId
- The import_ir_scopes field includes a NameScopeId so that as we
traverse to child namespaces, we can directly perform name lookup in the
other IR.
- is_closed_import now tracks whether a namespace comes from a different
package. This has a diagnostic implemented in decl_name_stack.
When a member access names an interface member, perform impl lookup to
find the impl and its corresponding member.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
A couple of minor functional changes here:
- We now always create a `name_ref` for the name referred to by the
right-hand operand of member access. Previously we skipped creating this
instruction if the referenced name was a field, and just created the
field access instruction. This makes our processing of member accesses
and our SemIR representation a bit more uniform.
- We now perform lookup into the type of the left-hand operand if it's
any type with a scope, not just for classes. This means we do lookup
into interface types. However, doing so isn't really useful yet because
it always finds an associated entity that isn't usable by itself. This
changes the diagnostic in
`toolchain/check/testdata/interface/fail_todo_facet_lookup.carbon`.
Note this builds on #3772 (I was partly cleaning up because I was
looking at this again).
This stops printing "Ignore missing" for ignored includes because it was
feeling noisy. This has been bugging me for a bit, and now I'm here. To
get an idea of what I mean, here it is masking a fix:
```
Querying bazel for Carbon targets...
Querying bazel for external targets...
Building header map...
Building generated file list...
Parsing headers from source files...
Ignored missing '#include "testing/fuzzing/carbon.pb.h"' in 'explorer/fuzzing/ast_to_proto_main.cpp'
Ignored missing '#include "testing/fuzzing/carbon.pb.h"' in 'explorer/fuzzing/ast_to_proto.h'
Ignored missing '#include "testing/fuzzing/carbon.pb.h"' in 'explorer/fuzzing/fuzzer_util.h'
Fixing include format in 'testing/file_test/file_test_base.h': '#include "gtest/gtest.h"' to '#include <gtest/gtest.h>'
Ignored missing '#include "testing/fuzzing/carbon.pb.h"' in 'testing/fuzzing/proto_to_carbon.cpp'
Ignored missing '#include "testing/fuzzing/carbon.pb.h"' in 'testing/fuzzing/proto_to_carbon.h'
Ignored missing '#include "testing/fuzzing/carbon.pb.h"' in 'testing/fuzzing/proto_to_carbon_test.cpp'
Done!
```
To help make this work, I'm also adjusting the gtest handling to use the
same EXTERNAL_REPO logic as the rest. I don't recall if there'd been
some other reason for the special casing, but AFAICT it works fine
(auto-adds a missing gtest dependency) this way.
I'd kept these in to separate the bazel module update from the BUILD
file changes, then forgot about it. I think all of these can be cleanly
removed now. I think it's something we should clean up for consistency
with the bazel central repository names; I think it's best to reduce
that divergence.
llvm_zlib and llvm_zstd remain because of how llvm depends on the
particular names.
The member access logic is fairly large, and will be growing with the
addition of impl lookup. Factor it out to separate the logic for dealing
with handling the parse node and updating the node stack from the logic
that checks and builds the member access expression.
This check is adding a lot of noise to the clang-tidy output for
construction of `SemIR::Inst` types, and we don't want to switch to
using designated initialization for them at this time.
The patches to bazel_clang_tidy were adopted upstream and are no longer
necessary, so I think we can simplify to git_override.
This fixes an existing issue where bazel was complaining that some rules
indirectly requested rules_python 0.29.0.
Note I'm not updating protobuf or com_google_libprotobuf_mutator. This
is because I get build errors with a protobuf -> abseil dependency if
updating.
Co-authored-by: josh11b <github-llvm@technomagi.com>
On macOS, Homebrew recommends `pipx` to install Python binaries, or
installing `pre-commit` directly, since it is available.
Here is the message in full.
```console
❯ pip3 install pre-commit
error: externally-managed-environment
× This environment is externally managed
╰─> To install Python packages system-wide, try brew install
xyz, where xyz is the package you are trying to
install.
If you wish to install a non-brew-packaged Python package,
create a virtual environment using python3 -m venv path/to/venv.
Then use path/to/venv/bin/python and path/to/venv/bin/pip.
If you wish to install a non-brew packaged Python application,
it may be easiest to use pipx install xyz, which will manage a
virtual environment for you. Make sure you have pipx installed.
```
~~Also, unless there is a compelling reason to install a particular 3.x
version of Python, it may be better to let Homebrew use whatever version
people may already have installed (or let it automatically install the
version they've tested).~~
Here are the details of the pre-commit Homebrew package:
```
❯ brew info pre-commit
==> pre-commit: stable 3.6.2 (bottled), HEAD
Framework for managing multi-language pre-commit hooks
https://pre-commit.com/
/opt/homebrew/Cellar/pre-commit/3.6.2 (679 files, 9.9MB) *
Poured from bottle using the formulae.brew.sh API on 2024-03-08 at 22:22:11
From: https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/p/pre-commit.rb
License: MIT
==> Dependencies
Required: libyaml ✔, python@3.12 ✔
```
Installing directly with Homebrew is a supported option in the
pre-commit documentation. https://pre-commit.com/#install
This was previously discussed at
https://discord.com/channels/655572317891461132/655578254970716160/1209975051588210729.
I'm initiating this mainly because we typically use "id" suffixes to
indicate an `IdBase` being passed around and the non-id suffix of
`parse_node` suggests at it carrying more data than it actually does.
There used to be more reason for avoiding `node_id` because
`SemIR::InstId` used to be named `NodeId`, but that's no longer
necessary. As a consequence, I'd like to rename `parse_node` to more
precisely reflect its type.
In full, this is doing:
```
parse_node_kind -> node_kind
parse_node -> node_id
ParseNodeCategory -> NodeCategory
ParseNodeKind -> NodeKind
ParseNode -> NodeId
```
This is primarily in check and sem_ir, but with some `parse_node_kind`
references in parse too.
Pluralization is consistent with name forms on both sides, so that
wasn't part of my replacements.
Jon Ross-Perkins
Richard Smith
dongjinlong
josh11bandJosh L
CJ Johnson
