Trying to make repeated `std::same_as` easier to write. Calling it
"concepts.h" because I figure we'll maybe have a couple more things like
this.
Was looking at this because I may add a couple more similar constructs.
The `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` flag is a standard flag
proposed by LibFuzzer that is meant to inform compiled code that it is
being built for fuzzing, as described here:
https://llvm.org/docs/LibFuzzer.html#fuzzer-friendly-build-mode
We add the flag to our `fuzzing` feature/config, and enable DCHECKs when
under fuzzing so that we can catch bugs that currently are caught on the
other side of DCHECK, even if they don't cause ASAN to trap a read/write
beyond the capacity of a value store.
When a `LookupImplWitness` instruction is created in a generic function
for a witness obtained from a `BindSymbolicName`, it stores the
`BindSymbolicName` as the query self type along with the interface it
obtained from it.
Later, when an argument is substituted into the `LookupImplWitness` in
deduction, and it is re-evaluated, the `BindSymbolicName` in the query
interface's specific arguments was being substituted, but the same
`BindSymbolicName` in the query self type was not. This was because we
did not substitute into the type of `BindSymbolicName`. In this case the
type is a `FacetType` which has inside it one or more specific
interfaces. The same substitution needs to be applied to both the
interfaces in the self type as to the interfaces in the query.
This issue was found by a fuzzer - though in a weirder and more invalid
way, by putting all of the code for our test case inside an `interface`,
which creates an implicit generic `Self` in the enclosing context and
yet makes it concrete inside a function body.
And update tests to clarify that we should be able to do lookup into a
runtime facet value for an associated constant if the FacetType itself
provides that constant (with a `where` clause), but should not be able
to if it does not.
This fixes a fuzzer-found crash.
A fuzzer found that converting a pointer to a `FunctionTypeWithSelfType`
to `type` does an impl lookup and ends up in TypeIterator with the
`FunctionTypeWithSelfType` being iterated over in the self type, where
we crash. It's a valid type, so the iterator should be able to walk over
and return it.
We also constructed a similar example for `FunctionType`.
* Add // === headlines on group of file shards.
* Rename shard files to shorten given the file context them and add
`todo` where appropriate.
Not splitting this file for now.
Part of #5150.
The Ubuntu 24 builders have a newer GLIBC than is present on the
compiler-explorer machines:
https://github.com/compiler-explorer/compiler-explorer/issues/7636#issuecomment-2880962252.
This results in the following error when running Carbon nightly:
```
/opt/compiler-explorer/carbon-trunk/bin/carbon: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.38' not found (required by /opt/compiler-explorer/carbon-trunk/bin/carbon)
```
To resolve this, we need to build Carbon in a sysroot with a compatible
glibc version, and the most straightforward way to do that is to bump
our builders back down to Ubuntu 22.
To do that, we can't use apt.llvm.org again, since Ubuntu 22 is no
longer supported there. So we revert back to pulling a Linux X64 tarball
from the LLVM GitHub Releases page. Instead of getting an
ubuntu-specific tarball (which does not exist), we grab the generic
Linux one, which seems to work fine.
Note that the binaries in the LLVM release package appear to depend on
glibc version 2.34, as determined by `objdump -T bin/clang|grep GLIBC_|
sed 's/.*GLIBC_\([.0-9]*\).*/\1/g' | sort -Vu`, so these binaries should
hopefully be okay to package with the Carbon toolchain for the
compiler-explorer machines as well.
* Split to 4 files: `function_param_int16`, `function_param_int32`,
`function_param_unsupported.carbon`, `function_return`.
* Add // === headlines on group of file shards.
* Rename shard files to shorten them and make them more consistent given
the file context.
* Deduplicate identical .h files and group their tests.
Potential future improvements:
* Split further. For example, pointers and references might be somewhat
separate from int primitives.
* Remove `import_` shard file prefix, as it repeats itself, but leaving
for now as it makes it more explicit.
Part of #5263
I think this would probably have prevented the missed include in #5469
-- it would've just failed completely with a "missing prelude"
diagnostic.
Also note this excludes the included IR from output, because it's
probably low-value to print.
Instead of wrapping `ClassStart` with `StartOnly` or `StartWithEnd`,
provide both `ClassStart` and `ClassStartOnly`. Use inheritance to share
the fields. Initializing the subclass as an aggregate is still possible,
but requires an extra set of curlies.
This flattens the switch in TypeStructureBuilder::Build to a single
level.
Since this puts 19 elements in the Any variant, we need to extend the
CARBON_KIND_SWITCH support to more than 12 elements, so we bump it up to
24.
This was suggested by @jonmeow here:
https://github.com/carbon-language/carbon-lang/pull/5430#discussion_r2078444685
Teach CARBON_KIND_SWITCH to handle mutable lvalues and rvalues, and
CARBON_KIND to forward along rvalues so that it's possible to write
`case CARBON_KIND(const T& t)`, `case CARBON_KIND(T& t)`, and `case
CARBON_KIND(T&& t)`, depending on the type that was passed to
CARBON_KIND_SWITCH.
Replace all uses of VariantMatch with their equivalent of a switch using
CARBON_KIND_SWITCH, and remove the VariantMatch helper from the
codebase.
We were importing all impls as non-final, since we forgot to set the new
field when constructing the imported Impl. Adds a test that fails before
this PR, since the imported Impl is treated as non-final.
We use CARBON_KIND_SWITCH for handling the output of TypeIterator in the
TypeStructureBuilder
Here is how the errors look when it is misused:
- If you don't cover ever type in the variant with a case
```
Enumeration value 'VariantTypeT1NotHandledInSwitch' not handled in
switch
```
Is attached to the CARBON_KIND_SWITCH() usage, the `T1` being a 0-based
index into the std::variant's type list, indicating which type was
missed.
- If you have a case for a type that is not in the variant
```
In template: constraints not satisfied for class template
'ValidCaseType' [with T = char]
... bunch of instantiation stuff ...
kind_switch.h(124, 12): Because 'char' does not satisfy
'TypeFoundInVariant'
```
Where `char` was the type I put in the `CARBON_KIND` macro, which was
not in the variant.
- If you have too many types in your variant (currently > 12)
```
In template: static assertion failed due to requirement 'sizeof...(Ts)
<= 12': CARBON_KIND_SWITCH supports std::variant with up to 12 types.
Add more if needed.
```
Is attached to the CARBON_KIND_SWITCH() usage.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
If there will be no in-scope instructions printed, have
`FormatScopeIfUsed` skip the relevant scope.
Note, this only affects constants and imports. It's not changing the
file scope, which is usually printed when empty.
The version of clangd/clang-tidy on developer machines has slowly
diverged from the one on the CI builders, which is causing a slowly
increasing amount of pain as clang-tidy CI runs fail (incorrectly) over
things that a newer clangd/clang-tidy was perfectly fine with locally.
This bumps the Clang version used in the ubuntu builders to 19, which is
the most recent in Debian stable.
We use https://apt.llvm.org instead of LLVM's GitHub releases
(https://github.com/llvm/llvm-project/releases) as the former more
reliably has packages for newer Clang/LLVM versions on x64. The
community-build releases binaries on LLVM's GitHub have stopped
including Ubuntu packages that match the GitHub x64 Ubuntu workers for
some time (for at least the 18 and 19 releases).
By moving to apt.llvm.org packages we only download and install the
headers and libraries needed for development, rather than every output
of building llvm, which is much faster and saves lots of disk space. We
also remove the system installations of other versions of clang/llvm so
we should end up using negative disk space. We can no longer easily
cache the installation but apt.llvm.org is a reliable end point.
We bump the ubuntu image version for the github workers to 24.04, as
apt.llvm.org has stopped building images for 22.10 in 2022 at its end of
life.
The `pre_commit` workflow disabled sudo unlike the other workflows that
install Clang/LLVM, including the `clang-tidy` workflow (which is also
run on `pull_request`). We bring it into alignment with the other
workflows so that we can install the llvm packages. And we lock its
ubuntu image to 24.04 so that it can be moved in lockstep with the other
workflows that depend on Clang/LLVM.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Right now, a lot of tests have started setting `--no-dump-sem-ir`. My
thought is that we can look at:
1. Put ranges in a bunch more files.
2. Shift more towards `--dump-sem-ir-ranges=only` instead of
`--no-dump-sem-ir`, because it allows mixing fail-tests with no IR
alongside tests that contain IR.
3. Evaluate switching the default to `--dump-sem-ir-ranges=only`, and
instead set `--dump-sem-ir-ranges=if-present` only in files that want to
typically show all IR (particularly import-related tests, where ranges
don't work well).
In real-world use, my thought is also that it'd be helpful to be able to
add the dump range comments to files, see the output (i.e., the default
behavior of `if-present`) but then also be able to pass `ignore` in
order to see the full IR without modifying the file (possibly also
useful in tests). That model is why I went for tri-state handling.
Note `only` can also have an interesting side-effect. Because core files
(including min_prelude versions) typically won't have ranges, they'd be
implicitly excluded.
Such impls will never be used, so they should not exist. And test that a
final impl partially overlapping a non-final impl is accepted.
There is a question about a final impl partially overlapping a final
impl that is part of
https://github.com/carbon-language/carbon-lang/pull/5337
Deduction can import stuff which can invalidate all of the value stores.
Refactor out the code that diagnoses unused generic bindings, and scope
the reference into the ImplStore so it can't be used after. Fetch the
impl from the store again when setting the witness to error afterward if
needed.
No functionality change right now: we reject thunks where the signature
has no return type and the callee has a return type. But discarding the
expression is still the right thing to do.
With this enabled, entities that live in value stores are poisoned
whenever any action is taken that might invalidate pointers and
references to those options -- in particular, adding another item to
that value store, or attempting to load any entity from an import IR.
Subsequent uses of those pointers or references then trigger an ASan
failure.
This detects latent bugs where the pointer or reference to the entity
would become stale if we got unlucky about when the value store
reallocates, even in cases where the reallocation didn't actually
happen.
This is not enabled by default: it finds a lot of latent bugs, so our
tests don't pass with this option. This PR also includes fixes for a few
of those bugs.
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
If a function contains instructions whose locations are in another file,
skip providing debug locations for those instructions rather than
CHECK-failing.
This happens when emitting a thunk where the signature is declared in
one file and the call target is in another file: some parts of the thunk
use the original signature as their locations, whereas other parts of it
use the location of the call target.
Non-type values were being represented as "Concrete" in the type
structure's shape, which was incorrect when the value was symbolic. Now
they are represented as "Concrete" or "Symbolic" as appropriate.
When concrete, a matching concrete ConstantId is stored in the type
structure's concrete values, so that they will be used for equality
comparison. And then impl matching comparison is taught to look in the
concrete values for mismatches when it finds a "Concrete" or
"ConcreteOpenParen" shape on both sides of the comparison, and return
false if they are not equal. This reduces the number of candidate impls
selected in impl lookup, which avoids doing type deduction against impls
that won't match anyway due to the concrete values in the query not
matching concrete values in the impl. As a result we see fewer specifics
being generated in the semir.
The TypeStructureBuilder recursively iterates through a type, interface,
or facet value and constructs a type structure that includes each
concrete and symbolic value found. This iteration is a more general
thing that can be useful elsewhere. For instance, to get just the root
type out of a general type, it is the result of the first iteration
step.
We abstract out the iteration logic into a SemIR::TypeIterator to create
a clear boundary between the work of iterating and the work of building
the TypeStructure from it.
Along the way this pointed out some issues in the TypeStructureBuilder
where it could have ambiguity between types that include non-type
values. So we add some tests for these cases, and they now pass. There
are also TODOs left behind, as concrete TypeStructure is overly specific
right now in order to keep these tests passing, which means that the
concrete elements can't be used for impl lookup matching yet. Only the
shape of concrete vs symbolic is used for now, and then type deduction
is used to compare the actual concrete types, which could be skipped
when the concrete values could be compared directly and reject an impl
for not matching.
#5445 updates to bazel 8.2.1, this does more updates (including to
buildifier, which does autofixes like the `sh_test` loads in the other
PR).
Note I'm using the latest available clang-format wheel. That's not
really something I expect people to have installed, but should mostly be
consistent. I'm specifically skipping clang-format 18 because it had
some broad regressions, and 19 got really confused by a `requires` on a
trailing return. Using the latest seemed probably okay since most people
won't see the difference. Do note that trailing returns in macros,
https://github.com/llvm/llvm-project/issues/47664, seems to be cropping
up again as an issue.
- Updates incompatible flags.
- `rules_flex` is no longer used, so enable its flag.
- Fixes `sh_test` deps for
`--incompatible_disable_autoloads_in_main_repo`
- Broadens the exception for `rules_cc` and `bazel_tools` due to changes
to runfiles deps; trying to avoid minutiae that shouldn't affect the
decision.
The current behavior hits UBSAN and ASAN issues.
Note, `RequiresNullTerminator` is already set to `false` in
`source_buffer.cpp`; setting it in `compile_helper.cpp` is making things
more consistent. The related logic is an [assert
fail](https://github.com/llvm/llvm-project/blob/main/llvm/lib/Support/MemoryBuffer.cpp#L52).
This was fuzzer-discovered.
Instead of crashing when run outside of `bazel`, make the toolchain's
`file_test` binary work properly when no test-specific environment
variables are set. This makes it a lot easier to run `file_test` under a
debugger.
There are two main changes here:
- Don't crash if `$TEST_TMPDIR` is unset. Instead, fall back to LLVM's
temporary directory (typically `$TMPDIR`). We already did this in some
places in tests. We now do it in more places.
- Don't fall back to a target label of `<target>` in the reproduction
commands if `$TEST_TARGET` is unset, because this causes all the tests
to fail because their output doesn't match the expected output due to a
differing bazel run command. Instead explicitly specify the target from
the `FileTestBase`-derived class.
Infrastructure for this has been added generally, but only rolled out to
the toolchain `file_test` binary for now.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Instead of building the definition of a thunk immediately when we
generate the thunk declaration, wait until we reach the `}` of the
outermost class, interface, etc. -- at the same time when we would parse
the definition of the thunk if it were defined inline.
This fixes issues where we fail to define the thunk because it requires
an enclosing class to be complete, or its definition depends on
something declared later in the enclosing class.
Make the representation of a suspended function scope, and its
constituent suspended components, be move-only, and switch to passing it
around by rvalue reference instead of by value because it's expensive
both to move and especially to copy.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
This is fixing a deduce crash, with regression tests added in
binding_pattern.carbon. In `needs_substitution`, it adds the
`BindSymbolicName` with the compile time bind index corresponding to the
wrong generic scope, which causes a bad result. It appears
`needs_substitution` logic is no longer needed (per zygoloid,
`CheckDeductionIsComplete` handles related issues) so can be removed.
This changes the order of IR in use_assoc_const.carbon but the result
appears equivalent to me.
This was a fuzzer-found crash.
```
class C {
impl as I;
}
```
is redeclared
```
impl C.(as I)
```
for purposes of `match_first`/`impl_priority` blocks and definitions.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
`IRBuilderBase::SetInsertPoint` weirdly replaces our debug location with
one copied from the new insertion point, so undo its damage after
calling it.
Also included: a couple of cleanups I made while tracking this down.
Ran into this trying to debug #5428
Before:
```
lex.cpp:793:21: runtime error: null pointer passed as argument 1, which is declared to never be null
string.h:90:51: note: nonnull attribute specified here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior lex.cpp:793:21
```
After:
```
lex.cpp:793:21: runtime error: null pointer passed as argument 1, which is declared to never be null
string.h:90:51: note: nonnull attribute specified here
#0 0x5572db22a680 in Carbon::Lex::Lexer::MakeLines(llvm::StringRef) /proc/self/cwd/toolchain/lex/lex.cpp:793:14
#1 0x5572db227ee4 in Lex /proc/self/cwd/toolchain/lex/lex.cpp:738:3
(etc)
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior lex.cpp:793:21
```
Interestingly, even though this is labelled as UB, it's using the
ASAN_SYMBOLIZER_PATH (specifically not LLVM_SYMBOLIZER_PATH). But
canonically UBSAN_SYMBOLIZER_PATH may also be used per
https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/ubsan/ubsan_flags.cpp#L53,
so I'm adding it to the list out of an excess of caution.
Also doing some small related cleanup:
- Removing `ASAN_SYMBOLIZER_PATH` from `--test_env` because it should
now be getting overridden by these settings (also was a little
inconsistent in that `LLVM_SYMBOLIZER_PATH` was not included).
- Improving the environment construction and documentation.
Jon Ross-Perkins
Dana Jansens
dependabot[bot]
Boaz Brickner
josh11bandJosh L
Chandler Carruth
Ivana Ivanovska
