Change representation of package names from `IdentifierId` to
`PackageNameId`, and add a special value `PackageNameId::Core` for the
Core package. Add a `Core` expression to name the Core package, and
support for parsing the `Core` keyword in `package` and `import`
declarations.
For now, I've made no changes to instruction fingerprinting or name
mangling. This means that fingerprints and mangled names will collide
between names in the `Core` package and names in a `r#Core` package. See
#4908.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
BuildValueRepr is used to determine the value representation of a type,
but the instruction determining the type may be an indirection through
to another instruction, such as a TupleAccess with `T.0` or a
StructAccess with `T.f`. In these cases, step through the indirection
and try again on the resulting type.
This eliminates a crash as these Access instructions do not resolve to a
type themselves and would otherwise end up in this FATAL line:
```
CARBON_FATAL("Type refers to non-type inst {0}", inst);
```
While here, remove the reference to TupleIndex in typed_insts.h as it
has been subsumed by TupleAccess in 7f930d0f58.
ClassElementAccess is not yet handled, but a fail_todo test is added. It
fails because `ConvertToValueOfType()` in `ExprAsType()` returns a
non-constant value for a ClassElementAccess instruction, where it must
not for a StructAccess.
context.cpp is getting large, so I'm looking at a few ways to cut out
clusters of functions. This felt like a logical cluster of functions to
move to their own file.
Note I have two commits in this PR: one moving the functionality to a
new file, and one specifically changing TypeCompleter to use out-of-line
function implementations. This is to assist reviewability.
Also makes the style guide explicitly comment on void, but this was the
intent IIRC because it matches Carbon's `-> ()` (and "always" versus
"except for void", which we definitely went back and forth on).
Includes adjusting function pointers, which I definitely forget this
syntax works sometimes.
Excludes utils/tree_sitter/src/scanner.c because it claims to be C, but
really we should probably fix that to be cpp.
The fn receiving a facet type needs to deduce a type from a
FacetAccessType, which is the SemIR type representing the parameter type
that is a generic parameter. For example:
```
fn F[T: Interface](val: T);
```
Here T is a generic parameter that is a facet value, but the `val`
parameter's type is the facet value converted from a FacetType to a
TypeType, with `as type`. The result of that conversion is a
FacetAccessType. So the deduction code sees a FacetAccessType for the
type of `val`.
We make deduction undo the `as type` conversion to move back to the `T`
parameter declaration, which has type FacetType in order to deduce the
required facet value (which is itself a type constrained by the
FacetType). And when we have a facet value (of type FacetType) to be
deduced, we will also convert the argument if it is of an appropriate
type to a FacetValue that matches the FacetType using the changes from
PR #4863.
Tests with `impl forall` can cause impl deduction to recurse forever and
crash, so those tests are omitted in this PR and they will come in
follow-up work that address the infinite recursion.
Rebased on top of PR #4885
Bumps the npm_and_yarn group with 1 update in the /utils/vscode
directory: [esbuild](https://github.com/evanw/esbuild).
Updates `esbuild` from 0.24.0 to 0.25.0
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/evanw/esbuild/releases">esbuild's
releases</a>.</em></p>
<blockquote>
<h2>v0.25.0</h2>
<p><strong>This release deliberately contains backwards-incompatible
changes.</strong> To avoid automatically picking up releases like this,
you should either be pinning the exact version of <code>esbuild</code>
in your <code>package.json</code> file (recommended) or be using a
version range syntax that only accepts patch upgrades such as
<code>^0.24.0</code> or <code>~0.24.0</code>. See npm's documentation
about <a
href="https://docs.npmjs.com/cli/v6/using-npm/semver/">semver</a> for
more information.</p>
<ul>
<li>
<p>Restrict access to esbuild's development server (<a
href="https://github.com/evanw/esbuild/security/advisories/GHSA-67mh-4wv8-2f99">GHSA-67mh-4wv8-2f99</a>)</p>
<p>This change addresses esbuild's first security vulnerability report.
Previously esbuild set the <code>Access-Control-Allow-Origin</code>
header to <code>*</code> to allow esbuild's development server to be
flexible in how it's used for development. However, this allows the
websites you visit to make HTTP requests to esbuild's local development
server, which gives read-only access to your source code if the website
were to fetch your source code's specific URL. You can read more
information in <a
href="https://github.com/evanw/esbuild/security/advisories/GHSA-67mh-4wv8-2f99">the
report</a>.</p>
<p>Starting with this release, <a
href="https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS">CORS</a>
will now be disabled, and requests will now be denied if the host does
not match the one provided to <code>--serve=</code>. The default host is
<code>0.0.0.0</code>, which refers to all of the IP addresses that
represent the local machine (e.g. both <code>127.0.0.1</code> and
<code>192.168.0.1</code>). If you want to customize anything about
esbuild's development server, you can <a
href="https://esbuild.github.io/api/#serve-proxy">put a proxy in front
of esbuild</a> and modify the incoming and/or outgoing requests.</p>
<p>In addition, the <code>serve()</code> API call has been changed to
return an array of <code>hosts</code> instead of a single
<code>host</code> string. This makes it possible to determine all of the
hosts that esbuild's development server will accept.</p>
<p>Thanks to <a
href="https://github.com/sapphi-red"><code>@sapphi-red</code></a> for
reporting this issue.</p>
</li>
<li>
<p>Delete output files when a build fails in watch mode (<a
href="https://redirect.github.com/evanw/esbuild/issues/3643">#3643</a>)</p>
<p>It has been requested for esbuild to delete files when a build fails
in watch mode. Previously esbuild left the old files in place, which
could cause people to not immediately realize that the most recent build
failed. With this release, esbuild will now delete all output files if a
rebuild fails. Fixing the build error and triggering another rebuild
will restore all output files again.</p>
</li>
<li>
<p>Fix correctness issues with the CSS nesting transform (<a
href="https://redirect.github.com/evanw/esbuild/issues/3620">#3620</a>,
<a
href="https://redirect.github.com/evanw/esbuild/issues/3877">#3877</a>,
<a
href="https://redirect.github.com/evanw/esbuild/issues/3933">#3933</a>,
<a
href="https://redirect.github.com/evanw/esbuild/issues/3997">#3997</a>,
<a
href="https://redirect.github.com/evanw/esbuild/issues/4005">#4005</a>,
<a href="https://redirect.github.com/evanw/esbuild/pull/4037">#4037</a>,
<a
href="https://redirect.github.com/evanw/esbuild/pull/4038">#4038</a>)</p>
<p>This release fixes the following problems:</p>
<ul>
<li>
<p>Naive expansion of CSS nesting can result in an exponential blow-up
of generated CSS if each nesting level has multiple selectors.
Previously esbuild sometimes collapsed individual nesting levels using
<code>:is()</code> to limit expansion. However, this collapsing wasn't
correct in some cases, so it has been removed to fix correctness
issues.</p>
<pre lang="css"><code>/* Original code */
.parent {
> .a,
> .b1 > .b2 {
color: red;
}
}
<p>/* Old output (with --supported:nesting=false) */<br />
.parent > :is(.a, .b1 > .b2) {<br />
color: red;<br />
}</p>
<p>/* New output (with --supported:nesting=false) */<br />
.parent > .a,<br />
.parent > .b1 > .b2 {<br />
color: red;<br />
}<br />
</code></pre></p>
<p>Thanks to <a
href="https://github.com/tim-we"><code>@tim-we</code></a> for working
on a fix.</p>
</li>
<li>
<p>The <code>&</code> CSS nesting selector can be repeated multiple
times to increase CSS specificity. Previously esbuild ignored this
possibility and incorrectly considered <code>&&</code> to have
the same specificity as <code>&</code>. With this release, this
should now work correctly:</p>
<pre lang="css"><code>/* Original code (color should be red) */
</code></pre>
</li>
</ul>
</li>
</ul>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/evanw/esbuild/blob/main/CHANGELOG-2024.md">esbuild's
changelog</a>.</em></p>
<blockquote>
<h1>Changelog: 2024</h1>
<p>This changelog documents all esbuild versions published in the year
2024 (versions 0.19.12 through 0.24.2).</p>
<h2>0.24.2</h2>
<ul>
<li>
<p>Fix regression with <code>--define</code> and
<code>import.meta</code> (<a
href="https://redirect.github.com/evanw/esbuild/issues/4010">#4010</a>,
<a
href="https://redirect.github.com/evanw/esbuild/issues/4012">#4012</a>,
<a
href="https://redirect.github.com/evanw/esbuild/pull/4013">#4013</a>)</p>
<p>The previous change in version 0.24.1 to use a more expression-like
parser for <code>define</code> values to allow quoted property names
introduced a regression that removed the ability to use
<code>--define:import.meta=...</code>. Even though <code>import</code>
is normally a keyword that can't be used as an identifier, ES modules
special-case the <code>import.meta</code> expression to behave like an
identifier anyway. This change fixes the regression.</p>
<p>This fix was contributed by <a
href="https://github.com/sapphi-red"><code>@sapphi-red</code></a>.</p>
</li>
</ul>
<h2>0.24.1</h2>
<ul>
<li>
<p>Allow <code>es2024</code> as a target in <code>tsconfig.json</code>
(<a
href="https://redirect.github.com/evanw/esbuild/issues/4004">#4004</a>)</p>
<p>TypeScript recently <a
href="https://devblogs.microsoft.com/typescript/announcing-typescript-5-7/#support-for---target-es2024-and---lib-es2024">added
<code>es2024</code></a> as a compilation target, so esbuild now supports
this in the <code>target</code> field of <code>tsconfig.json</code>
files, such as in the following configuration file:</p>
<pre lang="json"><code>{
"compilerOptions": {
"target": "ES2024"
}
}
</code></pre>
<p>As a reminder, the only thing that esbuild uses this field for is
determining whether or not to use legacy TypeScript behavior for class
fields. You can read more in <a
href="https://esbuild.github.io/content-types/#tsconfig-json">the
documentation</a>.</p>
<p>This fix was contributed by <a
href="https://github.com/billyjanitsch"><code>@billyjanitsch</code></a>.</p>
</li>
<li>
<p>Allow automatic semicolon insertion after
<code>get</code>/<code>set</code></p>
<p>This change fixes a grammar bug in the parser that incorrectly
treated the following code as a syntax error:</p>
<pre lang="ts"><code>class Foo {
get
*x() {}
set
*y() {}
}
</code></pre>
<p>The above code will be considered valid starting with this release.
This change to esbuild follows a <a
href="https://redirect.github.com/microsoft/TypeScript/pull/60225">similar
change to TypeScript</a> which will allow this syntax starting with
TypeScript 5.7.</p>
</li>
<li>
<p>Allow quoted property names in <code>--define</code> and
<code>--pure</code> (<a
href="https://redirect.github.com/evanw/esbuild/issues/4008">#4008</a>)</p>
<p>The <code>define</code> and <code>pure</code> API options now accept
identifier expressions containing quoted property names. Previously all
identifiers in the identifier expression had to be bare identifiers.
This change now makes <code>--define</code> and <code>--pure</code>
consistent with <code>--global-name</code>, which already supported
quoted property names. For example, the following is now possible:</p>
<pre lang="js"><code></code></pre>
</li>
</ul>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/evanw/esbuild/commit/e9174d671b1882758cd32ac5e146200f5bee3e45"><code>e9174d6</code></a>
publish 0.25.0 to npm</li>
<li><a
href="https://github.com/evanw/esbuild/commit/c27dbebb9e7a55dd9a084dd151dddd840787490e"><code>c27dbeb</code></a>
fix <code>hosts</code> in <code>plugin-tests.js</code></li>
<li><a
href="https://github.com/evanw/esbuild/commit/6794f602a453cf0255bcae245871de120a89a559"><code>6794f60</code></a>
fix <code>hosts</code> in <code>node-unref-tests.js</code></li>
<li><a
href="https://github.com/evanw/esbuild/commit/de85afd65edec9ebc44a11e245fd9e9a2e99760d"><code>de85afd</code></a>
Merge commit from fork</li>
<li><a
href="https://github.com/evanw/esbuild/commit/da1de1bf77a65f06654b49878d9ec4747ddaa21f"><code>da1de1b</code></a>
fix <a
href="https://redirect.github.com/evanw/esbuild/issues/4065">#4065</a>:
bitwise operators can return bigints</li>
<li><a
href="https://github.com/evanw/esbuild/commit/f4e9d19fb20095a98bf40634f0380f6a16be91e7"><code>f4e9d19</code></a>
switch case liveness: <code>default</code> is always last</li>
<li><a
href="https://github.com/evanw/esbuild/commit/7aa47c3e778ea04849f97f18dd9959df88fa0886"><code>7aa47c3</code></a>
fix <a
href="https://redirect.github.com/evanw/esbuild/issues/4028">#4028</a>:
minify live/dead <code>switch</code> cases better</li>
<li><a
href="https://github.com/evanw/esbuild/commit/22ecd306190b8971ec4474b5485266c20350e266"><code>22ecd30</code></a>
minify: more constant folding for strict equality</li>
<li><a
href="https://github.com/evanw/esbuild/commit/4cdf03c03697128044fa8fb76e5c478e9765b353"><code>4cdf03c</code></a>
fix <a
href="https://redirect.github.com/evanw/esbuild/issues/4053">#4053</a>:
reordering of <code>.tsx</code> in <code>node_modules</code></li>
<li><a
href="https://github.com/evanw/esbuild/commit/dc719775b7140120916bd9e6777ca1cb8a1cdc0e"><code>dc71977</code></a>
fix <a
href="https://redirect.github.com/evanw/esbuild/issues/3692">#3692</a>:
<code>0</code> now picks a random ephemeral port</li>
<li>Additional commits viewable in <a
href="https://github.com/evanw/esbuild/compare/v0.24.0...v0.25.0">compare
view</a></li>
</ul>
</details>
<br />
[](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore <dependency name> major version` will close this
group update PR and stop Dependabot creating any more for the specific
dependency's major version (unless you unignore this specific
dependency's major version or upgrade to it yourself)
- `@dependabot ignore <dependency name> minor version` will close this
group update PR and stop Dependabot creating any more for the specific
dependency's minor version (unless you unignore this specific
dependency's minor version or upgrade to it yourself)
- `@dependabot ignore <dependency name>` will close this group update PR
and stop Dependabot creating any more for the specific dependency
(unless you unignore this specific dependency or upgrade to it yourself)
- `@dependabot unignore <dependency name>` will remove all of the ignore
conditions of the specified dependency
- `@dependabot unignore <dependency name> <ignore condition>` will
remove the ignore condition of the specified dependency and ignore
conditions
You can disable automated security fix PRs for this repo from the
[Security Alerts
page](https://github.com/carbon-language/carbon-lang/network/alerts).
</details>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This PR updates the construction of clang::clangd::DocumentSymbol to
produce "range" and "selectionRange" properties that match the
[textDocument/documentSymbol](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_documentSymbol)
spec.
"selectionRange" is constructed by finding start and end position of the
symbols identifier.
"range" property is constructed by finding the left most and right most
token of the AST, the code makes an assumption that left most token is
always the first node in postorder traversal from that node, and the
right most is the node itself. AFAIK this is true for most of the
grammar other than infix operators. For symbols with a body, the right
most token is the open bracket, so we extend the range to its matching
closing bracket.
This PR adds a new command to the Carbon VSCode extension "carbon:
Restart language server" which acts similar to clangd / bazel
extensions. While not useful during regular working of the extension
this helps when LSP either crashes or the toolchain is updated.
In order to support graceful restart, I needed to add a Call handler for
"shutdown" which is noop for now. Lsp clients always call shutdown
before sending exit.
---------
Co-authored-by: jonmeow <jperkins@google.com>
When a function has a generic FacetType parameter, it can depend on
other FacetTypes bound as earlier parameters. To deduce the FacetValue,
we need to know the impl to attach to it, which requires knowing the
full type signature of the generic FacetType parameter. To do this,
after deducing other arguments to determine the value of non-generic
FacetType parameters, we substitute them sequentially into later
symbolic parameters to get their full facet types, then converts the
arguments to those full facet types to get the FacetValue. For example:
```
fn F(T: type, U: Interface(T));
```
Here the `T` binding is deduced to the be the caller's argument type.
But the Interface(T) can not be properly deduced to a FacetValue in the
first pass, and it will just be the caller's argument type directly.
After the first deduce pass, we will substitute the deduced T binding
into Interface(T), at which point the argument type can and will be
converted to a matching FacetValue as long as an impl can be found.
Closes#4868
This is based on PRs #4881 and #4863
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
When a node kind's Id::Kind is determined from its category, don't also
require it to be listed in the switch over all node kinds. This was both
redundant and also error prone -- and in practice for several node
kinds, the Id::Kind computed in the two different ways was different.
Instead, have the switch over node kinds handle only special cases that
can't be handled by their category, and enforce that each node kind has
an Id::Kind specified in exactly one way via checks in the .cpp file.
This refines the previous change in #4280 -- we still get the improved
errors for missing updates, but now also don't require redundant
additions to the switch.
Add caching of parsed documents, and testing of the textDocument
handlers. This is based on #4896, which splits out some of the
boilerplate to calls.
Note, this caches the entire parse state because we'll want to try to
emit diagnostics when we see the update, without waiting. It may be
helpful to do that asynchronously, but we don't want to wait for another
call (such as documentSymbol). Really, we'll probably want to also add
check for diagnostics, at least.
At present, we typically define a DiagnosticConverter, then store an
instance of it and a DiagnosticEmitter that wraps it. This is relatively
minor in general, but I've been trying to create more self-contained
DiagnosticEmitter classes (which hold their own DiagnosticConverter,
similar to NullDiagnosticEmitter), and there it just gets in the way.
Since we don't reuse DiagnosticConverter instances, this combines the
definition into DiagnosticEmitter. Mainly this means we don't have a
separate object in play, and less to carry around.
The most impact is probably to SemIRDiagnosticConverter, which was also
the most complex. Now `SemIRLocDiagnosticEmitter`, this gets some
different construction flow. Note in the PR I've split the file rename
to its own commit, to try to help delta views. However, the most
substantial parts of the refactoring are split into #4876, which this
depends upon.
If a type satisfies the requirements of a FacetType, then that type as a
value can be converted to a FacetValue, which binds the type value to
the FacetType.
For instance, if the class A implements an interface B, then
```
fn F(b:! B) {}
```
can be called with the type `A`
```
F(A);
```
This does not handle yet receiving non-type values matching a FacetType,
as that requires deducing the required FacetValue for the caller's
argument. Follow-up work will do this step.
---------
Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
Functions with positional parameters omit any implicit or explicit
parameter lists. This causes them to not have a pattern block, which
crashes if there is a return type that needs to add to the pattern
block.
Add a test covering this and handle it by having the ReturnTypeId
handler peek at the node stack and conditionally add the missing pattern
block. To do so it looks to see if the previous node is a
`IdentifierNameNotBeforeParams` which implies it was not expecting a
pattern (since there are no params) and thus the pattern block was not
added to the stack.
Note that lambdas also allow functions to omit an identifier, which will
need a pattern block on the stack for implicit parameters, explicit
parameters or a return type, without seeing any IdentifierName-like
parse nodes. To handle this, we will need to look for additional nodes
in the future and add the missing pattern block to the stack - possibly
for the FunctionInitializer, but the parse support needs to be created
for lambdas first.
At present, lower depends on `Check::SemIRDiagnosticConverter` for debug
info. That was to support a quick implementation of debug info, but
isn't great because it's both an unusual dependency on check's
implementation, and relying on diagnostic structures for debug info.
This cleans that up by splitting relevant logic out to a library in
sem_ir, and having lowering use sem_ir's library instead of check's.
Additionally, a small refactoring of `Parse::TreeAndSubtrees` to allow
getting locations in lowering without going through a `DiagnosticLoc`.
I'm adding `Parse::GetTreeAndSubtreesFn` in because it's a complex
signature to have in so many spots.
I chose to have `ResolveNodeId` return a `SmallVector` because it seemed
likely to be fairly compact, but that could also be using an optional
callback to handle resolved node IDs, possibly just returning the last
entry. This could be switched if preferred.
Note this change shouldn't affect behavior, it's just moving code
around.
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
Adds @LSP-CALL and refactors LSP keyword handling to implicitly handle a
little more of the LSP structure. This is coming out of textDocument
call handling, where this at least reduces some boilerplate of
`"params": {...}`.
AsCompatible changes a source instruction's type to a compatible type,
so it also needs its constant value to take on the compatible type.
Otherwise the type of the instruction and its constant value will
differ, which makes moving to the constant value into a lossy
transformation.
Part of #4868
We want to ensure the default flags are used consistently for all bazel
commands/steps.
Flag aliases do not track which commands/steps they work with though so
they can not use 'common'.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Adds `--help` and `--alsologtostderr`, latter as a boolean switch. Error
message was being sent to `/dev/null` by default, this allows it to
print to stderr. Helps in debugging why the script may be failing.
Tested by running
1. `./scripts/create_compdb.py`. This is the baseline (trunk). Can fail
or succeed. Not evaluated.
2. `./scripts/create_compdb.py --help`. Prints help. Expected.
3. `./scripts/create_compdb.py --also`. Fails. Expected because of
disabled abbreviations.
4. `./scripts/create_compdb.py --alsologtostderr`. Correctly printed
error logs. Expected.
Note one of these was added in 8.0.1, so this PR requires #4888. I just
forgot to check flags when updating, and figured it's just as well to
split this.
- `--incompatible_disable_native_apple_binary_rule`: Not sure why I
didn't have this before, maybe a copy-paste error? It's not new, it's
documented as deprecated... but even though it should be a no-op,
bazelisk recommends adding it.
- `--incompatible_disallow_empty_glob`: #4783 removed the conflict; I
mistakenly removed the comment instead of uncommenting
- `--incompatible_locations_prefers_executable`: New flag;
https://github.com/bazelbuild/bazel/releases/tag/8.0.1
Instead, produce a CARBON_FATAL error. Returning an Error constant from
the importer seems reasonable but turns out to not work well in
practice, because it violates the invariant that a constant value should
not have an error as an operand.
Also, don't produce an error constant for a valid ImportRefLoaded that
whose value is not constant; preserve the non-constant value instead.
For an expression such as `(Type as Interface).AssocFn()`, track the
`Self` type `Type` in the result of the member access so that it's
available when checking the function call.
This introduces a new kind of type, `ImplFunctionType`, that represents
the type of a function that is expected within an impl, modeled as the
type of the function within the interface plus a value to use as `Self`.
Calls to values of this type behave like calls to the underlying
function except that the `Self` parameter is pre-bound to the self type
from the facet.
In order to support this, fix an issue where the imported list of
generic bindings lost their association with their enclosing generic.
This adds a little complexity to `import_ref`, including a new recursive
cycle that I intend to address in a follow-up PR.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
### Context & Motivation
The error handling utilities in `//base/error.h` are very useful for
writing code with strong safety guarantees. While hardening the `Dump`
debug utilities (from review in #4866), I encountered a rough edge with
references and pointers. After a [brief Discord discussion in
#contributing-help](https://discord.com/channels/655572317891461132/1052653651895779359/1334675462877610038),
it was suggested that adding support for references to `ErrorOr` would
be a good candidate to move forward.
Using a reference type with the `ErrorOr` class (e.g. `ErrorOr<Node&>`)
produces two errors:
<ol>
<li><strong><code>variant can not have a reference type as an
alternative</code></strong>
<ul><li>From private field: <code>std::variant<Error, T>
val_;</code></li></ul>
</li>
<li><strong><code>'operator->' declared as a pointer to a
reference</code></strong>
<ul><li>From member function: <code>auto operator->() ->
T*</code></li></ul>
</li>
</ol>
### Changes
To support reference types, both errors are resolved:
1. `std::reference_wrapper` is conditionally used for storage when `T`
is a reference type
2. type trait aliases like `using ValueT = std::remove_reference_t<T>`
are used to produce compatible types for methods like `auto operator->()
-> ValueT*`
- The actual reason I started this: minor lowering updates in the golden
LLVM IR
- Process.inc changed enough to need a patch context update.
- https://github.com/llvm/llvm-project/pull/123126 added `proto_library`
uses without a `load`, which is broken in bazel 8
- Just commenting these out because we don't use them. I'll follow up
separately about a possible fix, but continuing to use `WORKSPACE` is a
bigger issue LLVM probably should address.
- Note this update is also triggering removal of `migrate_cpp`, in #4887
Although migration tooling is intended for Carbon, this tooling was
primarily a proof-of-concept prototype. The last time it got significant
work was in 2022
(https://github.com/carbon-language/carbon-lang/commits/trunk/migrate_cpp);
since then, we've mainly been doing small cleanups to keep it building.
We're now hitting an issue in #4886 that `TypeNodes.inc` isn't exposed
as a `#include`.
We already had thoughts about rewriting this as a `RecursiveASTVisitor`,
per `rewriter.cpp`/`rewriter.h`. That may justify a significantly
different approach than had been set up in `cpp_refactoring`. But, it's
hard to tell.
Either way, my sense is that rather than incrementally trying to keep
this code building, we should revisit it when we're ready and have a
long-term strategy for how migration should work.
Not sure if I mixed up a merge somewhere, but this is what
`parse_error.carbon` was supposed to test. Note
`unexpected_reply.carbon` was basically doing the same test, with a
minor issue of `change` vs `result` to get better output.
Introduce a principle that the Carbon language should not encroach on
the
developer's namespace. Satisfy this principle by making `Core` a
keyword.
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
If there are no explicit params, then `last_param_node_id` should be
`implicit_params_loc_id`. But there is no use case currently, so for now
we assume explicit params exist and assert if not, so this can be found
and updated then.
I'm planning on eliminating DiagnosticConverter. As part of this,
collapse it into diagnostic_emitter.h, and refactoring the header a
little so that it's more readable when making changes.
I'm looking at eliminating `DiagnosticConverter`. This change removes
`NodeLocConverter` (albeit adding `UnitAndImportsDiagnosticConverter`),
and in doing so, refactors lex conversion functions to extract them out
from the `DiagnosticConverter` functions.
I'll be following up with changes that collapse `DiagnosticConverter`
logic into `DiagnosticEmitter` locations. The intent is that we
shouldn't need separate ownership of both types.
This switches most error printing to use diagnostics instead of direct
stream writes, even when not a specific file diagnostic. I'm allowing
empty filenames for this use-case.
This allows a little more specific testing to validate coverage of
output using the diagnostic coverage test. I'm adding a few tests to
cover things that weren't previously tested.
Separately, this also forces a little more standardization in format...
considering how changes like #4568 show effort being spent to _mirror_
diagnostic style, my thought is now to just use diagnostic code where
possible.
Note this also allows incrementally better testing of the language
server; I'm changing the crash fix from #4847 in favor of diagnostic
testing.
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
I think the name of `SemIRLoc` might be leading to a few suggestions to
use it for non-diagnostic purposes that I've been responding to. At the
same time, a short name seems desirable given its frequency of use for
diagnostics. I did also clean up misuse in #4857, and eval.cpp (noted
below) might be similar. We don't typically use `friend` to emphasize
relationships, but given the diagnostic-specific intent and ways it's
been used, perhaps it's reasonable to close the API of this type using
`friend`?
eval.cpp inspects contents, but it's not clear that's needed because
changing logic doesn't affect tests, so I'm just removing it. Note that
SemIRLoc could've also been a loc_id, and it seems like the current
approach would mishandle that (i.e., print for `LocId::None` when that
seems not to be the intent). I figure we can add a `has_value` or
`AddNoteRequiringLoc` or something like that if needed.
Richard SmithandJon Ross-Perkins
josh11bandJosh L
Dana Jansens
dependabot[bot]
DavidLoftus
Boaz Brickner
Geoff Romer
Samiur Khan
Calvin
Alina Sbirlea