| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Fixes #149986
|
|
equivalence checking (#149264)
This is another case where untransformed constraint expressions led to
inconsistent transforms.
We did fix some of those issues by looking at parent scopes, however the
parent instantiation scope is not always available because we could also
reach here after the parents get instantiated.
Fixes #146614
|
|
Since the function template isn't instantiated before constraint
checking, we'll not be able to find the outer template arguments through
function specialization when evaluating the inner constraint that is
nested within a larger constraint expression.
The only practical solution is to get them back through the code
synthesis context, which also allows us to eliminate an overload of
getTemplateInstantiationArgs.
No release note because it's a regression on trunk.
Fixes https://github.com/llvm/llvm-project/issues/147772
|
|
We were using the lexical DC as the starting point of template argument
collection when comparing declarations. This caused an issue that
template arguments from out-of-line declarations are ignored when
substituting into the constraints, which in turn led to expression
mismatching.
Fixes https://github.com/llvm/llvm-project/issues/145521
|
|
Decl is already of FunctionDecl *.
|
|
ArrayRef has a constructor that accepts std::nullopt. This
constructor dates back to the days when we still had llvm::Optional.
Since the use of std::nullopt outside the context of std::optional is
kind of abuse and not intuitive to new comers, I would like to move
away from the constructor and eventually remove it.
This patch takes care of the clang side of the migration.
|
|
(#122423)
The previous approach broke code generation for the MS ABI due to an
unintended code path during constraint substitution. This time we
address the issue by inspecting the evaluation contexts and thereby
avoiding that code path.
This reapplies 96eced624 (#102857).
|
|
`static_assert(std::is_xx_v<MyType>);` is a common pattern to check that
a type meets a requirement.
This patch produces diagnostics notes when such assertion fails. The
first type trait for which we provide detailed explanation is
std::is_trivially_relocatable.
We employ the same mechanisn when a type trait appears an an unsatisfied
atomic constraint.
I plan to also support `std::is_trivially_replaceable` in a follow up
PR, and hopefully, over time we can support more type traits.
|
|
It is a layering violation
|
|
(#135390)
There are several type traits that produce a boolean value or type based
on the well-formedness of some expression (more precisely, the immediate
context, i.e. for example excluding nested template instantiation):
* `__is_constructible` and variants,
* `__is_convertible` and variants,
* `__is_assignable` and variants,
* `__reference_{binds_to,{constructs,converts}_from}_temporary`,
* `__is_trivially_equality_comparable`,
* `__builtin_common_type`.
(It should be noted that the standard doesn't always base this on the
immediate context being well-formed: for `std::common_type` it's based
on whether some expression "denotes a valid type." But I assume that's
an editorial issue and means the same thing.)
Errors in the immediate context are suppressed, instead the type traits
return another value or produce a different type if the expression is
not well-formed. This is achieved using an `SFINAETrap` with
`AccessCheckingSFINAE` set to true. If the type trait is used outside of
an SFINAE context, errors are discarded because in that case the
`SFINAETrap` sets `InNonInstantiationSFINAEContext`, which makes
`isSFINAEContext` return an `optional(nullptr)`, which causes the errors
to be discarded in `EmitDiagnostic`. However, in an SFINAE context this
doesn't happen, and errors are added to `SuppressedDiagnostics` in the
`TemplateDeductionInfo` returned by `isSFINAEContext`. Once we're done
with deducing template arguments and have decided which template is
going to be instantiated, the errors corresponding to the chosen
template are then emitted. At this point we get errors from those type
traits that we wouldn't have seen if used with the same arguments
outside of an SFINAE context. That doesn't seem right.
So what we want to do is always set `InNonInstantiationSFINAEContext`
when evaluating these well-formed-testing type traits, regardless of
whether we're in an SFINAE context or not. This should only affect the
immediate context, as nested contexts add a new `CodeSynthesisContext`
that resets `InNonInstantiationSFINAEContext` for the time it's active.
Going through uses of `SFINAETrap` with `AccessCheckingSFINAE` = `true`,
it occurred to me that all of them want this behavior and we can just
use this parameter to decide whether to use a non-instantiation context.
The uses are precisely the type traits mentioned above plus the
`TentativeAnalysisScope`, where I think it is also fine. (Though I think
we don't do tentative analysis in SFINAE contexts anyway.)
Because the parameter no longer just sets `AccessCheckingSFINAE` in Sema
but also `InNonInstantiationSFINAEContext`, I think it should be renamed
(along with uses, which also point the reviewer to the affected places).
Since we're testing for validity of some expression, `ForValidityCheck`
seems to be a good name.
The added tests should more or less correspond to the users of
`SFINAETrap` with `AccessCheckingSFINAE` = `true`. I added a test for
errors outside of the immediate context for only one type trait, because
it requires some setup and is relatively noisy.
We put the `ForValidityCheck` condition first because it's constant in
all uses and this would then allow the compiler to prune the call to
`isSFINAEContext` when true.
Fixes #132044.
|
|
Static analysis flagged the use of Left.size() because we just moved out
of Left and that would be undefined behavior. Fix is to take the size
and store it in a local variable instead.
|
|
|
|
This patch replaces:
llvm::copy(Src, std::back_inserter(Dst));
with:
llvm::append_range(Dst, Src);
for breavity.
One side benefit is that llvm::append_range eventually calls
llvm::SmallVector::reserve if Dst is of llvm::SmallVector.
|
|
Fixes what seems to be a buggy warning in MSVC:
```
[1/37] Building CXX object tools\clang\lib\Sema\CMakeFiles\obj.clangSema.dir\SemaConcept.cpp.obj
C:\git\llvm-project\clang\lib\Sema\SemaConcept.cpp(1933): warning C4101: '$S26': unreferenced local variable
```
|
|
std::optional<unsigned> (#134142)
This introduces a new class 'UnsignedOrNone', which models a lite
version of `std::optional<unsigned>`, but has the same size as
'unsigned'.
This replaces most uses of `std::optional<unsigned>`, and similar
schemes utilizing 'int' and '-1' as sentinel.
Besides the smaller size advantage, this is simpler to serialize, as its
internal representation is a single unsigned int as well.
|
|
|
|
In ecc7e6ce4, we tried to inspect the `LambdaScopeInfo` on stack to
recover the instantiating lambda captures. However, there was a mismatch
in how we compared the pattern declarations of lambdas: the constraint
instantiation used a tailored `getPatternFunctionDecl()` which is
localized in SemaLambda that finds the very primal template declaration
of a lambda, while `FunctionDecl::getTemplateInstantiationPattern` finds
the latest template pattern of a lambda. This difference causes issues
when lambdas are nested, as we always want the primary template
declaration.
This corrects that by moving `Sema::addInstantiatedCapturesToScope` from
SemaConcept to SemaLambda, allowing it to use the localized version of
`getPatternFunctionDecl`.
It is also worth exploring to coalesce the implementation of
`getPatternFunctionDecl` with
`FunctionDecl::getTemplateInstantiationPattern`. But I’m leaving that
for the future, as I’d like to backport this fix (ecc7e6ce4 made the
issue more visible in clang 20, sorry!), and changing Sema’s ABI would
not be suitable in that regards. Hence, no release note.
Fixes https://github.com/llvm/llvm-project/issues/133719
|
|
We had a weird, incorrect, "ConstraintEvaluator" object that was not
useful for anything, so I removed that.
I also changed the CheckConstraintSatisfaction overload that just took
an Expr* as this did not make much sense at all.
Satisfaction checking is still fairly wrong,
we do not follow the standard that requires we only substitute into the
mapping of the normal form, so we produce errors for incorrect
substitution into concepts id, even though we should not.
|
|
This reverts an earlier attempt
(adb0d8ddceb143749c519d14b8b31b481071da77 and
50e5411e4247421fd606f0a206682fcdf0303ae3) to support these expansions,
which was limited to type arguments and which subverted the purpose
of SubstTemplateTypeParmType.
This propagates the ArgumentPackSubstitutionIndex along with the
AssociatedConstraint, so that the pack expansion works, without
needing any new transforms or otherwise any changes to the template
instantiation process.
This keeps the tests from the reverted commits, and adds a few more
showing the new solution also works for NTTPs.
Fixes https://github.com/llvm/llvm-project/issues/131798
|
|
Follow up to #132849
|
|
The main goal of this patch is to improve the
performance of concept subsumption by
- Making sure literal (atomic) clauses are de-duplicated (Whether 2
atomic constraint is established during the initial normal form
production).
- Eagerly removing duplicated clauses.
This should minimize the risks of exponentially large formulas that can
be produced by a naive {C,D}NF transformation.
While at it, I restructured that part of the code to be a bit clearer.
Subsumption of fold expanded constraint is also cached.
---
Note that removing duplicated clauses seems to be necessary and
sufficient to have acceptable performance on anything that could be
construed as reasonable code.
Ultimately, the number of clauses is always going to be fairly small
(but $2^{fairly\ small}$ is quickly *fairly large*..).
I went too far in the rabbit hole of Tseitin transformations etc, which
was much faster but would then require to check satisfiabiliy to
establish subsumption between some constraints (although it was good
enough to pass all but ones of our tests...).
It doesn't help that the C++ standard has a very specific definition of
subsumption that is really more of an implication...
While that sort of musing is fascinating, it was ultimately a fool's
errand, at least until such time that there is more motivation for a SAT
solver in clang (clang-tidy can after all use z3!).
Here be dragons.
Fixes #122581
|
|
overloaded function' (#127773)
Closes https://github.com/llvm/llvm-project/issues/122523
|
|
(#128478)
addInstantiatedCapturesToScope() might be called when transforming a
lambda body. In this situation, it would look into all the lambda's
parents and figure out all the instantiated captures. However, the
instantiated captures are not visible from lambda's class decl until the
lambda is rebuilt (i.e. after the lambda body transform). So this patch
corrects that by also examining the LambdaScopeInfo, serving as a
workaround for not having deferred lambda body instantiation in Clang
20, to avoid regressing some real-world use cases.
Fixes #128175
|
|
unevaluated context (#123883)
It turns out that the substitution for expression comparing also needs
an unevaluated context, otherwise any reference to immediate functions
might not be properly handled.
As a fallout, this also guards the VLA transformation under unevaluated
context
with `InConditionallyConstantEvaluateContext` to avoid duplicate
diagnostics.
Fixes https://github.com/llvm/llvm-project/issues/123472
---------
Co-authored-by: cor3ntin <corentinjabot@gmail.com>
|
|
LambdaScopeForCallOperatorInstantiationRAII (#123687)
Now that the RAII object has a dedicate logic for handling nested
lambdas, where the inner lambda could reference any
captures/variables/parameters from the outer lambda, we can shift the
responsibility for managing lambdas away from SetupConstraintScope().
I think this also makes the structure clearer.
Fixes https://github.com/llvm/llvm-project/issues/123441
|
|
substitution"" (#122130)
Unfortunately that breaks some code on Windows when lambdas come into
play, as reported in
https://github.com/llvm/llvm-project/pull/102857#issuecomment-2577861178
This reverts commit 96eced624e0f120155256033fdcb8342e7e58d6e.
|
|
(#102857)
This patch partially implements CWG2369 for non-lambda-constrained
functions.
Lambdas are left intact at this point because we need extra work to
correctly instantiate captures before the function instantiation.
As a premise of CWG2369, this patch also implements CWG2770 to ensure
the function parameters are instantiated on demand.
Closes https://github.com/llvm/llvm-project/issues/54440
|
|
Note that PointerUnion::{is,get} have been soft deprecated in
PointerUnion.h:
// FIXME: Replace the uses of is(), get() and dyn_cast() with
// isa<T>, cast<T> and the llvm::dyn_cast<T>
I'm moving the definitions of several functions to SemaConcept.cpp
because llvm::cast requires the full definitions of
NormalizedConstraintPair, which is used like so:
using CompoundConstraint = llvm::PointerIntPair<NormalizedConstraintPair
*, 1,
CompoundConstraintKind>;
|
|
Note that PointerUnion::{is,get} have been soft deprecated in
PointerUnion.h:
// FIXME: Replace the uses of is(), get() and dyn_cast() with
// isa<T>, cast<T> and the llvm::dyn_cast<T>
I'm not touching PointerUnion::dyn_cast for now because it's a bit
complicated; we could blindly migrate it to dyn_cast_if_present, but
we should probably use dyn_cast when the operand is known to be
non-null.
|
|
Identified with misc-include-cleaner.
|
|
argument lists (#106585, #111173)" (#111852)" (#115159)
This reverts commit 2bb3d3a3f32ffaef3d9b6a27db7f1941f0cb1136.
|
|
(#114749)
We need to compare constraint expressions when instantiating a friend
declaration that is lexically defined within a class template. Since the
evaluation is deferred, the expression might refer to untransformed
function parameters such that the substitution needs the mapping of
instantiation.
These mappings are maintained by the function declaration instantiation,
so we need to establish a "transparent" LocalInstantiationScope before
substituting into the constraint.
No release note as this fixes a regression in 19.
Fixes https://github.com/llvm/llvm-project/issues/114685
|
|
lists (#106585, #111173)" (#111852)
This patch reapplies #111173, fixing a bug when instantiating dependent
expressions that name a member template that is later explicitly
specialized for a class specialization that is implicitly instantiated.
The bug is addressed by adding the `hasMemberSpecialization` function,
which return `true` if _any_ redeclaration is a member specialization.
This is then used when determining the instantiation pattern for a
specialization of a template, and when collecting template arguments for
a specialization of a template.
|
|
argument lists (#106585)" (#111173)" (#111766)
This reverts commit 4da8ac34f76e707ab94380b94f616457cfd2cb83.
|
|
lists (#106585)" (#111173)
Reapplies #106585, fixing an issue where non-dependent names of member
templates appearing prior to that member template being explicitly
specialized for an implicitly instantiated class template specialization
would incorrectly use the definition of the explicitly specialized
member template.
|
|
non-init-captures (#110887)
Otherwise, we would probably have an unmatched instantiated declaration
for init-captures when they come after a non-init capture.
No release note because the bug only affects the trunk so far.
Fixes #110721
|
|
SizeOfPackExpr has a pointer to the referenced pack declaration, which
is left as-is during the transformation process.
The situation could be subtle when a friend class template declaration
comes into play. The declaration per se would be instantiated into its
parent declaration context, and consequently, the template parameter
list would have a depth adjustment; however, as we don't evaluate
constraints during instantiation, those constraints would still
reference the original template parameters, which is fine for constraint
evaluation because we have handled friend cases in the template argument
collection.
However, things are different when we want to profile the constraint
expression with dependent template arguments. The hash algorithm of
SizeOfPackExpr takes its pack declaration as a factor, which is the
original template parameter that might still have untransformed template
depths after the constraint normalization.
This patch transforms the pack declaration when normalizing constraint
expressions and pluses a fix in HandleFunctionTemplateDecl() where the
associated declaration is incorrect for nested specifiers.
Note that the fix in HandleFunctionTemplateDecl(), as well as the
handling logic for NestedNameSpecifier, would be removed once Krystian's
refactoring patch lands. But I still want to incorporate it in the patch
for the correction purpose, though it hasn't caused any problems so far
- I just tripped over that in getFullyPackExpandedSize() when I tried to
extract the transformed declarations from the TemplateArgument.
Fixes #93099
---------
Co-authored-by: Matheus Izvekov <mizvekov@gmail.com>
|
|
lists (#106585)"
This reverts commit cdd71d61664b63ae57bdba9ee0d891f78ef79c07 (and
30adb43c897a45c18d7dd163fb4ff40c915fc488).
This change broke compiling Qt, see
https://github.com/llvm/llvm-project/pull/106585#issuecomment-2365309463
for details.
|
|
(#106585)
Currently, clang rejects the following explicit specialization of `f`
due to the constraints not being equivalent:
```
template<typename T>
struct A
{
template<bool B>
void f() requires B;
};
template<>
template<bool B>
void A<int>::f() requires B { }
```
This happens because, in most cases, we do not set the flag indicating
whether a `RedeclarableTemplate` is an explicit specialization of a
member of an implicitly instantiated class template specialization until
_after_ we compare constraints for equivalence. This patch addresses the
issue (and a number of other issues) by:
- storing the flag indicating whether a declaration is a member
specialization on a per declaration basis, and
- significantly refactoring `Sema::getTemplateInstantiationArgs` so we
collect the right set of template argument in all cases.
Many of our declaration matching & constraint evaluation woes can be
traced back to bugs in `Sema::getTemplateInstantiationArgs`. This
change/refactor should fix a lot of them. It also paves the way for
fixing #101330 and #105462 per my suggestion in #102267 (which I have
implemented on top of this patch but will merge in a subsequent PR).
|
|
`FindInstantiatedDecl()` relies on the `CurContext` to find the
corresponding class template instantiation for a class template
declaration.
Previously, we pushed the semantic declaration context for constraint
comparison, which is incorrect for constraints on friend declarations.
In issue #78101, the semantic context of the friend is the TU, so we
missed the implicit template specialization `Template<void, 4>` when
looking for the instantiation of the primary template `Template` at the
time of checking the member instantiation; instead, we mistakenly picked
up the explicit specialization `Template<float, 5>`, hence the error.
As a bonus, this also fixes a crash when diagnosing constraints. The
DeclarationName is not necessarily an identifier, so it's incorrect to
call `getName()` on e.g. overloaded operators. Since the
DiagnosticBuilder has correctly handled Decl printing, we don't need to
find the printable name ourselves.
Fixes https://github.com/llvm/llvm-project/issues/78101
|
|
parameter packs (#102131)
We established an instantiation scope in order for constraint
equivalence checking to properly map the uninstantiated parameters.
That mechanism mapped even packs to themselves. Consequently, parameter
packs e.g. appearing in a function call, were not expanded. So they
would end up becoming `SubstTemplateTypeParmPackType`s that circularly
depend on the canonical declaration of the function template, which is
not yet determined, hence the spurious error.
No release note as I plan to backport it to 19.
Fixes https://github.com/llvm/llvm-project/issues/101735
---------
Co-authored-by: cor3ntin <corentinjabot@gmail.com>
|
|
LambdaScopeForCallOperatorInstantiationRAII (#100766)
This PR addresses issues related to the handling of `init capture` with
parameter packs in Clang's
`LambdaScopeForCallOperatorInstantiationRAII`.
Previously, `addInstantiatedCapturesToScope` would add `init capture`
containing packs to the scope using the type of the `init capture` to
determine the expanded pack size. However, this approach resulted in a
pack size of 0 because `getType()->containsUnexpandedParameterPack()`
returns `false`. After extensive testing, it appears that the correct
pack size can only be inferred from `getInit`.
But `getInit` may reference parameters and `init capture` from an outer
lambda, as shown in the following example:
```cpp
auto L = [](auto... z) {
return [... w = z](auto... y) {
// ...
};
};
```
To address this, `addInstantiatedCapturesToScope` in
`LambdaScopeForCallOperatorInstantiationRAII` should be called last.
Additionally, `addInstantiatedCapturesToScope` has been modified to only
add `init capture` to the scope. The previous implementation incorrectly
called `MakeInstantiatedLocalArgPack` for other non-init captures
containing packs, resulting in a pack size of 0.
### Impact
This patch affects scenarios where
`LambdaScopeForCallOperatorInstantiationRAII` is passed with
`ShouldAddDeclsFromParentScope = false`, preventing the correct addition
of the current lambda's `init capture` to the scope. There are two main
scenarios for `ShouldAddDeclsFromParentScope = false`:
1. **Constraints**: Sometimes constraints are instantiated in place
rather than delayed. In this case,
`LambdaScopeForCallOperatorInstantiationRAII` does not need to add `init
capture` to the scope.
2. **`noexcept` Expressions**: The expressions inside `noexcept` have
already been transformed, and the packs referenced within have been
expanded. Only `RebuildLambdaInfo` needs to add the expanded captures to
the scope, without requiring `addInstantiatedCapturesToScope` from
`LambdaScopeForCallOperatorInstantiationRAII`.
### Considerations
An alternative approach could involve adding a data structure within the
lambda to record the expanded size of the `init capture` pack. However,
this would increase the lambda's size and require extensive
modifications.
This PR is a prerequisite for implmenting
https://github.com/llvm/llvm-project/issues/61426
|
|
(#101782)" (#102551)
|
|
checking (#101879)
If a fold expanded constraint would expand packs of different size, it
is not a valid pack expansion and it is not satisfied. This should not
produce an error.
Fixes #99430
|
|
|
|
(#101745)
Prior to this patch, during constraint normalization we could forget
from which declaration an atomic constraint was normalized from.
Subsequently when performing parameter mapping substitution for that
atomic constraint with an incorrect context, we couldn't correctly
recognize which declarations are supposed to be visible.
Fixes #60336
|
|
TemplateDecls (#75697)
Fixes https://github.com/llvm/llvm-project/issues/73885.
Substituting into constraints for invalid TemplateDecls might still
yield dependent expressions and end up crashing later in evaluation.
|
|
expressions (#99022)
Implement https://isocpp.org/files/papers/P2963R3.pdf
|
|
expressions" (#99007)
Reverts llvm/llvm-project#98160
Breaks CI on some architectures
|
|
(#98160)
Implement https://isocpp.org/files/papers/P2963R3.pdf
|
