This finishes the clang implementation of P0522, getting rid of the
fallback to the old, pre-P0522 rules.
Before this patch, when partial ordering template template parameters,
we would perform, in order:
* If the old rules would match, we would accept it. Otherwise, don't
generate diagnostics yet.
* If the new rules would match, just accept it. Otherwise, don't
generate any diagnostics yet again.
* Apply the old rules again, this time with diagnostics.
This situation was far from ideal, as we would sometimes:
* Accept some things we shouldn't.
* Reject some things we shouldn't.
* Only diagnose rejection in terms of the old rules.
With this patch, we apply the P0522 rules throughout.
This needed to extend template argument deduction in order to accept the
historial rule for TTP matching pack parameter to non-pack arguments.
This change also makes us accept some combinations of historical and
P0522 allowances we wouldn't before.
It also fixes a bunch of bugs that were documented in the test suite,
which I am not sure there are issues already created for them.
This causes a lot of changes to the way these failures are diagnosed,
with related test suite churn.
The problem here is that the old rules were very simple and
non-recursive, making it easy to provide customized diagnostics, and to
keep them consistent with each other.
The new rules are a lot more complex and rely on template argument
deduction, substitutions, and they are recursive.
The approach taken here is to mostly rely on existing diagnostics, and
create a new instantiation context that keeps track of this context.
So for example when a substitution failure occurs, we use the error
produced there unmodified, and just attach notes to it explaining that
it occurred in the context of partial ordering this template argument
against that template parameter.
This diverges from the old diagnostics, which would lead with an error
pointing to the template argument, explain the problem in subsequent
notes, and produce a final note pointing to the parameter.
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.
This is a fix for the following issue: when a lambda’s class type is
merged across modules (e.g. because it is defined in a template in the
GMF of some module `A`, and some other module `B` both imports `A` and
has the same template in its GMF), then `getLambdaCallOperator()` might
return the wrong operator (e.g. while compiling `B`, the lambda’s class
type would be the one attached to `B`’s GMF, but the call operator ends
up being the one attached to `A`’s GMF).
This causes issues in situations where the call operator is in a
template and accesses declarations in the surrounding context: when
those declarations are instantated, a mapping is introduced from the
original node in the template to that of the instantiation. If such an
instantiation happens in `B`, and we then try to instantiate `A`’s call
operator, any nodes in that call operator refer to declarations in the
template in `A`, but the `LocalInstantiationScope` only contains
mappings for declarations in `B`! This causes the following assertion
(for godbolt links and more, see the issue below):
```
Assertion `isa<LabelDecl>(D) && "declaration not instantiated in this scope"' failed.
```
We now walk the redecl chain of the call operator to find the
one that is in the same module as the record decl.
This fixes#110401.
The size of a.pcm produced by an RV64 clang running natively is 67416
bytes, causing this test to fail. Bump up the maximum size. The test
still retains its original intent as far as I can see, as it's really
trying to ensure that compressing kicks in and reduces the
multi-megabyte input.
This finishes the clang implementation of P0522, getting rid of the
fallback to the old, pre-P0522 rules.
Before this patch, when partial ordering template template parameters,
we would perform, in order:
* If the old rules would match, we would accept it. Otherwise, don't
generate diagnostics yet.
* If the new rules would match, just accept it. Otherwise, don't
generate any diagnostics yet again.
* Apply the old rules again, this time with diagnostics.
This situation was far from ideal, as we would sometimes:
* Accept some things we shouldn't.
* Reject some things we shouldn't.
* Only diagnose rejection in terms of the old rules.
With this patch, we apply the P0522 rules throughout.
This needed to extend template argument deduction in order to accept the
historial rule for TTP matching pack parameter to non-pack arguments.
This change also makes us accept some combinations of historical and
P0522 allowances we wouldn't before.
It also fixes a bunch of bugs that were documented in the test suite,
which I am not sure there are issues already created for them.
This causes a lot of changes to the way these failures are diagnosed,
with related test suite churn.
The problem here is that the old rules were very simple and
non-recursive, making it easy to provide customized diagnostics, and to
keep them consistent with each other.
The new rules are a lot more complex and rely on template argument
deduction, substitutions, and they are recursive.
The approach taken here is to mostly rely on existing diagnostics, and
create a new instantiation context that keeps track of things.
So for example when a substitution failure occurs, we use the error
produced there unmodified, and just attach notes to it explaining that
it occurred in the context of partial ordering this template argument
against that template parameter.
This diverges from the old diagnostics, which would lead with an error
pointing to the template argument, explain the problem in subsequent
notes, and produce a final note pointing to the parameter.
Fixed a crash for the attached test case due to we missed to emit the
deduction guide. The reason is, the deduction guide is attached to the
export-decl in the imported module. So we won't emit it by traversing the
AST of the current TU.
Track the identity of each string literal object produced by evaluation
with a global version number. Accept comparisons between literals of the
same version, and between literals of different versions that cannot
possibly be placed in overlapping storage. Treat the remaining
comparisons as non-constant.
---------
Co-authored-by: Timm Baeder <tbaeder@redhat.com>
Co-authored-by: Aaron Ballman <aaron@aaronballman.com>
Recently, there are multiple false positive issue reports about the
reachability of implementation partition units:
- https://github.com/llvm/llvm-project/issues/105882
- https://github.com/llvm/llvm-project/issues/101348
- https://lists.isocpp.org/core/2024/08/16232.php
And according to our use experience for modules, we find it is a pretty
good practice to not import implementation partition units in the
interface units. It can help developers to have a pretty good mental
model for when to use an implementation partition unit: that any unit in
the module but not in the module interfaces can be in the implementation
partition unit.
So I think it is good to add the diagnostics.
Solve https://github.com/clangd/clangd/issues/2094
Due clangd will enable PCH automatically, the previous mechanism to skip
ODR check in GMF may be invalid. This patch fixes this for a case.
This patch rewrites a test that uses command substitution `$()` and the
`stat` command, which are not supported by lit's internal shell. Instead
of using this syntax to perform the file size comparison done in this
test, a Python script is used instead to perform the same operation.
Fixes https://github.com/llvm/llvm-project/issues/102384.
Close https://github.com/llvm/llvm-project/issues/72383
The implementation rationale is, I don't want to pass
`-fmodules-embed-all-files` all the time since we can't test it in lit
tests (we're using `clang_cc1`). So I tried to set it in FrontendActions
for modules.
Close https://github.com/llvm/llvm-project/issues/102721
Generally, the type of merged decls will be reused in ASTContext. But
for lambda, in the import and then include case, we can't decide its
previous decl in the imported modules so that we can't assign the
previous decl before creating the type for it. Since we can't decide its
numbering before creating it. So we have to assign the previous decl and
the canonical type for it after creating it, which is unusual and
slightly hack.
It is a long standing issue that the duplicated declarations in multiple
module units would cause the compilation performance to get slowed down.
And there are many questions or issue reports. So I think it is better
to add a warning for it.
And given this is not because the users' code violates the language
specification or any best practices, the warning is disabled by default
even if `-Wall` is specified. The users need to specify the warning
explcitly or use `Weverything`.
The documentation will add separately.
Close https://github.com/llvm/llvm-project/issues/102684
The root cause of the issue is, it is possible that the predefined decl
is not registered at the beginning of writing a module file but got
created during the process of writing from reading.
This is incorrect. The predefined decls should always be predefined
decls.
Another deep thought about the issue is, we shouldn't read any new
things after we start to write the module file. But this is another
deeper question.
Close https://github.com/llvm/llvm-project/issues/99825
The root cause of the issue is that I didn't realize the things in
implicit global module (the language linkage in module interfaces)
should be considered in module purview.
Close https://github.com/llvm/llvm-project/issues/101398
The root cause of the issue is that I removed the codes before and
failed to recognize it in time and this was not found for a long time
due to it only crashes with invalid codes.
Claiming a mismatch is always in a precompiled header is wrong and
misleading as a mismatch can happen in any provided AST file. Emitting a
path for a file with a problem allows to disambiguate between multiple
input files.
Use generic term "AST file" because we don't always know a kind of the
provided file (for example, see `ASTReader::readASTFileControlBlock`).
rdar://65005546
Reland https://github.com/llvm/llvm-project/pull/75912
The differences of this PR between
https://github.com/llvm/llvm-project/pull/75912 are:
- Fixed a regression in `Decl::isInAnotherModuleUnit()` in DeclBase.cpp
pointed by @mizvekov and add the corresponding test.
- Fixed the regression in windows
https://github.com/llvm/llvm-project/issues/97447. The changes are in
`CodeGenModule::getVTableLinkage` from
`clang/lib/CodeGen/CGVTables.cpp`. According to the feedbacks from MSVC
devs, the linkage of vtables won't affected by modules. So I simply
skipped the case for MSVC.
Given this is more or less fundamental to the use of modules. I hope we
can backport this to 19.x.
HLSL has a set of intangible types which are described in in the
[draft HLSL Specification
(**[Basic.types]**)](https://microsoft.github.io/hlsl-specs/specs/hlsl.pdf):
There are special implementation-defined types such as handle types,
which fall into a category of standard intangible types. Intangible
types are types that have no defined object representation or value
representation, as such the size is unknown at compile time.
A class type T is an intangible class type if it contains an base
classes or members of intangible class type, standard intangible type,
or arrays of such types. Standard intangible types and intangible class
types are collectively called intangible
types([9](https://microsoft.github.io/hlsl-specs/specs/hlsl.html#Intangible)).
This PR implements one standard intangible type `__hlsl_resource_t`
and sets up the infrastructure that will make it easier to add more
in the future, such as samplers or raytracing payload handles. The
HLSL intangible types are declared in
`clang/include/clang/Basic/HLSLIntangibleTypes.def` and this file is
included with related macro definition in most places that require edits
when a new type is added.
The new types are added as keywords and not typedefs to make sure they
cannot be redeclared, and they can only be declared in builtin implicit
headers. The `__hlsl_resource_t` type represents a handle to a memory
resource and it is going to be used in builtin HLSL buffer types like this:
template <typename T>
class RWBuffer {
[[hlsl::contained_type(T)]]
[[hlsl::is_rov(false)]]
[[hlsl::resource_class(uav)]]
__hlsl_resource_t Handle;
};
Part 1/3 of llvm/llvm-project#90631.
---------
Co-authored-by: Justin Bogner <mail@justinbogner.com>
Previously we skipped the ODR checks in explicit global modules. And due
to similar reasons, we should skip the ODR checks in implicit global
modules too.
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
Fix the false warning
> incompatible pointer types passing 'va_list' (aka '__builtin_va_list')
to parameter of type 'struct __va_list_tag *'
[-Wincompatible-pointer-types]
The warning is wrong because both in the function declaration and at the
call site we are using `va_list`.
When we call `ASTContext::getBuiltinVaListDecl` at a specific moment, we
end up re-entering this function which causes creating 2 instances of
`BuiltinVaListDecl` and 2 instances of `VaListTagDecl` but the stored
instances are unrelated to each other because of the call sequence like
getBuiltinVaListDecl
CreateX86_64ABIBuiltinVaListDecl
VaListTagDecl = TagA
indirectly call getBuiltinVaListDecl
CreateX86_64ABIBuiltinVaListDecl
VaListTagDecl = TagB
BuiltinVaListDecl = ListB
BuiltinVaListDecl = ListA
Now we have `BuiltinVaListDecl == ListA` and `VaListTagDecl == TagB`.
For x86_64 '__builtin_va_list' and 'struct __va_list_tag *' are
compatible because '__builtin_va_list' == '__va_list_tag[1]'. But
because we have unrelated decls for VaListDecl and VaListTagDecl the
types are considered incompatible as we are comparing type pointers.
Fix the error by creating `BuiltinVaListDecl` before
`ASTReader::InitializeSema`, so that during
`ASTContext::getBuiltinVaListDecl` ASTReader doesn't try to de-serialize
'__builtin_va_list' and to call `ASTContext::getBuiltinVaListDecl`
again.
Reland with the requirement to have x86 target to avoid errors like
> error: unable to create target: 'No available targets are compatible
with triple "x86_64-apple-darwin"'
rdar://130947515
Fix the false warning
> incompatible pointer types passing 'va_list' (aka '__builtin_va_list')
to parameter of type 'struct __va_list_tag *'
[-Wincompatible-pointer-types]
The warning is wrong because both in the function declaration and at the
call site we are using `va_list`.
When we call `ASTContext::getBuiltinVaListDecl` at a specific moment, we
end up re-entering this function which causes creating 2 instances of
`BuiltinVaListDecl` and 2 instances of `VaListTagDecl` but the stored
instances are unrelated to each other because of the call sequence like
getBuiltinVaListDecl
CreateX86_64ABIBuiltinVaListDecl
VaListTagDecl = TagA
indirectly call getBuiltinVaListDecl
CreateX86_64ABIBuiltinVaListDecl
VaListTagDecl = TagB
BuiltinVaListDecl = ListB
BuiltinVaListDecl = ListA
Now we have `BuiltinVaListDecl == ListA` and `VaListTagDecl == TagB`.
For x86_64 '__builtin_va_list' and 'struct __va_list_tag *' are
compatible because '__builtin_va_list' == '__va_list_tag[1]'. But
because we have unrelated decls for VaListDecl and VaListTagDecl the
types are considered incompatible as we are comparing type pointers.
Fix the error by creating `BuiltinVaListDecl` before
`ASTReader::InitializeSema`, so that during
`ASTContext::getBuiltinVaListDecl` ASTReader doesn't try to de-serialize
'__builtin_va_list' and to call `ASTContext::getBuiltinVaListDecl`
again.
rdar://130947515
See the attached test for the motivation example. If we're too greedy to
not emit the definition for inline builtins, we may meet a middle end
crash. And it should be good to emit inline builtins always.
You can provide more than one AST file as an input. Emit a path for a
file with a problem, so you can disambiguate between multiple files.
rdar://65005546
See the attached test for the motivation example. If we're too greedy to
not emit the definition for inline builtins, we may meet a middle end
crash. And it should be good to emit inline builtins always.
Such searches can be costly and non-intuitive. We've seen complaints
from developers that they don't expect clang to find modules on their
own and not in search paths that developers provide. Keeping the search
of modulemaps in subdirectories for code completion as it provides
better user experience.
If you are defining module "UsefulCode" in
"include/UnrelatedName/module.modulemap", it is recommended to rename
the directory "UnrelatedName" to "UsefulCode". If you cannot do so, you
can add to "include/module.modulemap" a line like `extern module
UsefulCode "UnrelatedName/module.modulemap"`, so clang can find module
"UsefulCode" without checking each subdirectory in "include/".
rdar://106677321
---------
Co-authored-by: Jan Svoboda <jan@svoboda.ai>
stddef.h always includes __stddef_null.h. This is fine in modules
because it's not possible to re-include the pcm, and it's necessary to
export the _Builtin_stddef.null submodule. However, without modules it
causes NULL to always get redefined which disrupts some C++ code. Rework
the inclusion of __stddef_null.h so that with not building with modules
it's only included if __need_NULL is set by the includer, or it's the
first time stddef.h is being included.
This patch tries to workaround the case that:
- in a module unit that imports another module unit
- both the module units including overlapped headers
- the compiler emits false positive ODR violation diagnostics for the
overlapped headers if ODR check is enabled
- the current module units enables PCH
For the third point, we disabled ODR check if the declarations comes
from GMF. However, due to the forth point, the check whether the
declaration comes from GMF failed. Then we still going to check it and
then the users get false positive checks.
What's worse is that, this always happens in clangd, where will generate
the PCH automatically before parsing the input files.
The root cause of the problem we mixed the modules in the semantical
level and the module in the serialization level.
The problem is pretty fundamental and we need time to fix that. But 19.x
is going to be branched and I hope to give clangd better user
experience. So I decided to land this workaround even if it is pretyy
niche and may only work for the case of clangd's pattern.
Close https://github.com/llvm/llvm-project/issues/98583
Currently, clang will reject the following code:
```
export module mod;
extern "C++" void func();
export extern "C++" {
void func();
}
```
while both MSVC and GCC accept it. Although clang's behavior matches the
current wording, from the discussion, the consensus is that we should
accept the above example from the intention. Since the intention to not
allow export redeclaration which is not exported is to make the linkage
clear. But it doesn't matter with the declarations within global module.
This reverts commit 18f3bcbb13ca83d33223b00761d8cddf463e9ffb, 15bb02650e26875c48889053d6a9697444583721 and
99873b35da7ecb905143c8a6b8deca4d4416f1a9.
See the post commit message in
https://github.com/llvm/llvm-project/pull/75912 to see the reasons.
With the pruning of unused module map files disabled
(`-fno-modules-prune-non-affecting-module-map-files`), `HeaderFileInfo`
no longer gets deserialized before `ASTWriter::WriteHeaderSearch()`.
This function then interleaves the stores of references to `KnownHeader`
with their lazy deserialization. Lazy deserialization may cause
reallocation of `ModuleMap::Headers` entries (including its
`SmallVector<KnownHeader, 1>` values) thus making previously-stored
`ArrayRef<KnownHeader>` dangling. This patch fixes that situation by
storing a copy instead.
Given the following invalid code,
```cpp
template <class T>
struct S {
T *a;
};
S s = {1};
```
we produce such diagnostics currently:
```
<source>:2:8: note: candidate template ignored: could not match 'S<T>' against 'int'
2 | struct S {
| ^
<source>:2:8: note: candidate template ignored: could not match 'T *' against 'int'
```
Which I think is confusing because there's no `S<T>` nor `T *` at the
location it points to. This is because we're deducing the initializer
against implicitly generated deduction guides, and their source
locations just point to the corresponding `RecordDecl`. Hence the
misleading notes.
This patch alleviates the issue by adding extra notes demonstrating
which implicit deduction guide we're deducing against. In other words,
in addition to the note of `could not match 'T *' against 'int'`, we
would also say the implicit deduction guide we're trying to use:
`template <class T> S(T *) -> S<T>`, which looks clearer IMO.
---------
Co-authored-by: Sirraide <aeternalmail@gmail.com>
