Diagnostic for `-Wauto-import` shouldn't be a warning because it doesn't
represent a potential problem in code that should be fixed. And the
emitted fix-it is likely to trigger `-Watimport-in-framework-header`
which makes it challenging to have a warning-free codebase. But it is
still useful to see how include directives are translated into modular
imports and which module a header belongs to, that's why keep it as a remark.
Keep `-Wauto-import` for now to allow a gradual migration for codebases
using `-Wno-auto-import`, e.g., `-Weverything -Wno-auto-import`.
rdar://79594287
Differential Revision: https://reviews.llvm.org/D130138
Currently, clang may meet an infinite loop in a very tricky case when it
iterates the default args. This patch tries to fix this by adding a
`fixed` check.
Previously, we forget to handle reachability for deduction guide.
The deduction guide is a hint to the compiler. And the deduction guide
should be able to use if the corresponding template decl is reachable.
Following some recent discussions, this changes the representation
of callbrs in IR. The current blockaddress arguments are replaced
with `!` label constraints that refer directly to callbr indirect
destinations:
; Before:
%res = callbr i8* asm "", "=r,r,i"(i8* %x, i8* blockaddress(@test8, %foo))
to label %asm.fallthrough [label %foo]
; After:
%res = callbr i8* asm "", "=r,r,!i"(i8* %x)
to label %asm.fallthrough [label %foo]
The benefit of this is that we can easily update the successors of
a callbr, without having to worry about also updating blockaddress
references. This should allow us to remove some limitations:
* Allow unrolling/peeling/rotation of callbr, or any other
clone-based optimizations
(https://github.com/llvm/llvm-project/issues/41834)
* Allow duplicate successors
(https://github.com/llvm/llvm-project/issues/45248)
This is just the IR representation change though, I will follow up
with patches to remove limtations in various transformation passes
that are no longer needed.
Differential Revision: https://reviews.llvm.org/D129288
This reverts commit 7c51f02effdbd0d5e12bfd26f9c3b2ab5687c93f because it
stills breaks the LLDB tests. This was re-landed without addressing the
issue or even agreement on how to address the issue. More details and
discussion in https://reviews.llvm.org/D112374.
Without this patch, clang will not wrap in an ElaboratedType node types written
without a keyword and nested name qualifier, which goes against the intent that
we should produce an AST which retains enough details to recover how things are
written.
The lack of this sugar is incompatible with the intent of the type printer
default policy, which is to print types as written, but to fall back and print
them fully qualified when they are desugared.
An ElaboratedTypeLoc without keyword / NNS uses no storage by itself, but still
requires pointer alignment due to pre-existing bug in the TypeLoc buffer
handling.
---
Troubleshooting list to deal with any breakage seen with this patch:
1) The most likely effect one would see by this patch is a change in how
a type is printed. The type printer will, by design and default,
print types as written. There are customization options there, but
not that many, and they mainly apply to how to print a type that we
somehow failed to track how it was written. This patch fixes a
problem where we failed to distinguish between a type
that was written without any elaborated-type qualifiers,
such as a 'struct'/'class' tags and name spacifiers such as 'std::',
and one that has been stripped of any 'metadata' that identifies such,
the so called canonical types.
Example:
```
namespace foo {
struct A {};
A a;
};
```
If one were to print the type of `foo::a`, prior to this patch, this
would result in `foo::A`. This is how the type printer would have,
by default, printed the canonical type of A as well.
As soon as you add any name qualifiers to A, the type printer would
suddenly start accurately printing the type as written. This patch
will make it print it accurately even when written without
qualifiers, so we will just print `A` for the initial example, as
the user did not really write that `foo::` namespace qualifier.
2) This patch could expose a bug in some AST matcher. Matching types
is harder to get right when there is sugar involved. For example,
if you want to match a type against being a pointer to some type A,
then you have to account for getting a type that is sugar for a
pointer to A, or being a pointer to sugar to A, or both! Usually
you would get the second part wrong, and this would work for a
very simple test where you don't use any name qualifiers, but
you would discover is broken when you do. The usual fix is to
either use the matcher which strips sugar, which is annoying
to use as for example if you match an N level pointer, you have
to put N+1 such matchers in there, beginning to end and between
all those levels. But in a lot of cases, if the property you want
to match is present in the canonical type, it's easier and faster
to just match on that... This goes with what is said in 1), if
you want to match against the name of a type, and you want
the name string to be something stable, perhaps matching on
the name of the canonical type is the better choice.
3) This patch could exposed a bug in how you get the source range of some
TypeLoc. For some reason, a lot of code is using getLocalSourceRange(),
which only looks at the given TypeLoc node. This patch introduces a new,
and more common TypeLoc node which contains no source locations on itself.
This is not an inovation here, and some other, more rare TypeLoc nodes could
also have this property, but if you use getLocalSourceRange on them, it's not
going to return any valid locations, because it doesn't have any. The right fix
here is to always use getSourceRange() or getBeginLoc/getEndLoc which will dive
into the inner TypeLoc to get the source range if it doesn't find it on the
top level one. You can use getLocalSourceRange if you are really into
micro-optimizations and you have some outside knowledge that the TypeLocs you are
dealing with will always include some source location.
4) Exposed a bug somewhere in the use of the normal clang type class API, where you
have some type, you want to see if that type is some particular kind, you try a
`dyn_cast` such as `dyn_cast<TypedefType>` and that fails because now you have an
ElaboratedType which has a TypeDefType inside of it, which is what you wanted to match.
Again, like 2), this would usually have been tested poorly with some simple tests with
no qualifications, and would have been broken had there been any other kind of type sugar,
be it an ElaboratedType or a TemplateSpecializationType or a SubstTemplateParmType.
The usual fix here is to use `getAs` instead of `dyn_cast`, which will look deeper
into the type. Or use `getAsAdjusted` when dealing with TypeLocs.
For some reason the API is inconsistent there and on TypeLocs getAs behaves like a dyn_cast.
5) It could be a bug in this patch perhaps.
Let me know if you need any help!
Signed-off-by: Matheus Izvekov <mizvekov@gmail.com>
Differential Revision: https://reviews.llvm.org/D112374
This reverts commit bdc6974f92304f4ed542241b9b89ba58ba6b20aa because it
breaks all the LLDB tests that import the std module.
import-std-module/array.TestArrayFromStdModule.py
import-std-module/deque-basic.TestDequeFromStdModule.py
import-std-module/deque-dbg-info-content.TestDbgInfoContentDequeFromStdModule.py
import-std-module/forward_list.TestForwardListFromStdModule.py
import-std-module/forward_list-dbg-info-content.TestDbgInfoContentForwardListFromStdModule.py
import-std-module/list.TestListFromStdModule.py
import-std-module/list-dbg-info-content.TestDbgInfoContentListFromStdModule.py
import-std-module/queue.TestQueueFromStdModule.py
import-std-module/stack.TestStackFromStdModule.py
import-std-module/vector.TestVectorFromStdModule.py
import-std-module/vector-bool.TestVectorBoolFromStdModule.py
import-std-module/vector-dbg-info-content.TestDbgInfoContentVectorFromStdModule.py
import-std-module/vector-of-vectors.TestVectorOfVectorsFromStdModule.py
https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/45301/
Without this patch, clang will not wrap in an ElaboratedType node types written
without a keyword and nested name qualifier, which goes against the intent that
we should produce an AST which retains enough details to recover how things are
written.
The lack of this sugar is incompatible with the intent of the type printer
default policy, which is to print types as written, but to fall back and print
them fully qualified when they are desugared.
An ElaboratedTypeLoc without keyword / NNS uses no storage by itself, but still
requires pointer alignment due to pre-existing bug in the TypeLoc buffer
handling.
Signed-off-by: Matheus Izvekov <mizvekov@gmail.com>
Differential Revision: https://reviews.llvm.org/D112374
There were two assertions in DefaultArgStorage::setInherited previously.
It requires the DefaultArgument is either empty or an argument value. It
would crash if it has a pointer refers to the previous declaration or
contains a chain to the previous declaration.
But there are edge cases could hit them actually. One is
InheritDefaultArguments.cppm that I found recently. Another one is pr31469.cpp,
which was created fives years ago.
This patch tries to fix the two failures by handling full cases in
DefaultArgStorage::setInherited.
This is guaranteed to not introduce any breaking change since it lives
in the path we wouldn't touch before. And the added assertions for
sameness should keep the correctness.
Reviewed By: v.g.vassilev
Differential Revision: https://reviews.llvm.org/D128974
parameters.
The current implementation to judge the similarity of TypeConstraint in
ASTContext::isSameTemplateParameter is problematic, it couldn't handle
the following case:
```C++
template <__integer_like _Tp, C<_Tp> Sentinel>
constexpr _Tp operator()(_Tp &&__t, Sentinel &&last) const {
return __t;
}
```
When we see 2 such declarations from different modules, we would judge
their similarity by `ASTContext::isSame*` methods. But problems come for
the TypeConstraint. Originally, we would profile each argument one by
one. But it is not right. Since the profiling result of `_Tp` would
refer to two different template type declarations. So it would get
different results. It is right since the `_Tp` in different modules
refers to different declarations indeed. So the same declaration in
different modules would meet incorrect our-checking results.
It is not the thing we want. We want to know if the TypeConstraint have
the same expression.
Reviewer: vsapsai, ilya-biryukov
Differential Revision: https://reviews.llvm.org/D129068
This addresses [cpp.include]/7
(when encountering #include header-name)
If the header identified by the header-name denotes an importable header, it
is implementation-defined whether the #include preprocessing directive is
instead replaced by an import directive.
In this implementation, include translation is performed _only_ for headers
in the Global Module fragment, so:
```
module;
#include "will-be-translated.h" // IFF the header unit is available.
export module M;
#include "will-not-be-translated.h" // even if the header unit is available
```
The reasoning is that, in general, includes in the module purview would not
be validly translatable (they would have to immediately follow the module
decl and without any other intervening decls). Otherwise that would violate
the rules on contiguous import directives.
This would be quite complex to track in the preprocessor, and for relatively
little gain (the user can 'import "will-not-be-translated.h";' instead.)
TODO: This is one area where it becomes increasingly difficult to disambiguate
clang modules in C++ from C++ standard modules. That needs to be addressed in
both the driver and the FE.
Differential Revision: https://reviews.llvm.org/D128981
The existing provision is not sufficient, it did not allow for the cases
where an implementation partition includes the primary module interface,
or for the case that an exported interface partition is contains a decl
that is then implemented in a regular implementation unit.
It is somewhat unfortunate that we have to compare top level module names
to achieve this, since built modules are not necessarily available.
TODO: It might be useful to cache a hash of the primary module name if
this test proves to be a significant load.
Differential Revision: https://reviews.llvm.org/D127624
See https://github.com/cplusplus/draft/pull/5204 for a detailed
background.
Simply, the test redundant-template-default-arg.cpp attached to this
patch should be accepted instead of being complained about the
redefinition.
Reviewed By: urnathan, rsmith, ChuanqiXu
Differential Revision: https://reviews.llvm.org/D118034
When we do profiling in ASTContext::getAutoType, it wouldn't think about
the canonical declaration for the type constraint. It is bad since it
would cause a negative ODR mismatch while we already know the type
constraint declaration is a redeclaration for the previous one. Also it shouldn't be
bad to use the canonical declaration here.
Currently we do not in general merge attributes when importing decls from modules. This patch handles availability, but long term we need to properly handle all attributes.
I tried to use Sema::mergeDeclAttributes, but it caused test crashes as I don't think it expects to be called in this context. We really shouldn't have duplicate code for merging attributes long term, but for now this fixes availability. There's already a TODO for this in the declaration of ASTDeclReader::mergeInheritableAttributes.
Differential Revision: https://reviews.llvm.org/D127182
rdar://85820301
DebugTypeVisitor
This recommits d1346e2. I've added a line to the test case to enable it
only on assert builds.
Differential Revision: https://reviews.llvm.org/D125839
This reverts commit a544710cd43ba9f7729a613c58729f146e792a8e.
See discussion in D120540.
This breaks C++ Clang modules on Darwin and also more than a dozen
tests in the LLDB testsuite. I think we need to be more careful to
separate out the enabling of Clang C++ modules and C++20
modules. Either by having -fmodules-ts control the HaveModules flag,
or by adding a way to explicitly turn them off.
This patch allows user to use C++20 module by -fcxx-modules. Previously,
we could only use it under -std=c++20. Given that user could use C++20
coroutine standalonel by -fcoroutines-ts. It makes sense to offer an
option to use C++20 modules without enabling C++20.
Reviewed By: iains, MaskRay
Differential Revision: https://reviews.llvm.org/D120540
This patch allows user to use C++20 module by -fcxx-modules. Previously,
we could only use it under -std=c++20. Given that user could use C++20
coroutine standalonel by -fcoroutines-ts. It makes sense to offer an
option to use C++20 modules without enabling C++20.
Reviewed By: iains, MaskRay
Differential Revision: https://reviews.llvm.org/D120540
This diff changes the serialization of the `ORIGINAL_PCH_DIR`
entry in module files to be serialized relative to the module's
`BaseDirectory`. This will allow for the module to be relocatable
across machines.
The path is restored relative to the module's BaseDirectory on
deserialization.
Reviewed By: urnathan
Differential Revision: https://reviews.llvm.org/D124946
This diff changes the serialization of the `SUBMODULE_TOPHEADER`
entry in module files to be serialized relative to the module's
`BaseDirectory`. This matches the behavior of the
`SUBMODULE_HEADER` entry and will allow for the module to be
relocatable across machines.
The path is restored relative to the module's `BaseDirectory` on
deserialization.
Reviewed By: urnathan
Differential Revision: https://reviews.llvm.org/D124938
This diff adds a new frontend flag `-fmodule-file-home-is-cwd`.
The behavior of this flag is similar to
`-fmodule-map-file-home-is-cwd` but does not require the module
map files to be modified to have inputs relative to the cwd.
Instead the output modules will have their `BaseDirectory` set
to the cwd and will try and resolve paths relative to that.
The motiviation for this change is to support relocatable pcm
files that are built on different machines with different paths
without having to alter module map files, which is sometimes not
possible as they are provided by 3rd parties.
Reviewed By: urnathan
Differential Revision: https://reviews.llvm.org/D124874
C89 allowed a type specifier to be elided with the resulting type being
int, aka implicit int behavior. This feature was subsequently removed
in C99 without a deprecation period, so implementations continued to
support the feature. Now, as with implicit function declarations, is a
good time to reevaluate the need for this support.
This patch allows -Wimplicit-int to issue warnings in C89 mode (off by
default), defaults the warning to an error in C99 through C17, and
disables support for the feature entirely in C2x. It also removes a
warning about missing declaration specifiers that really was just an
implicit int warning in disguise and other minor related cleanups.
Emitting metadata for the same ivar multiple times can lead to
miscompilations. Objective-C runtime adds offsets to calculate ivar
position in memory and presence of duplicate offsets causes wrong final
position thus overwriting unrelated memory.
Such a situation is impossible with modules disabled as clang diagnoses
ivar redeclarations during sema checks after parsing
(`Sema::ActOnFields`). Fix the case with modules enabled by checking
during deserialization if ivar is already declared. We also support
a use case where the same category ends up in multiple modules. We
don't want to treat this case as ivar redeclaration and instead merge
corresponding ivars.
rdar://83468070
Differential Revision: https://reviews.llvm.org/D121177
Now the implementation would accept following code:
```
//--- impl.cppm
module M:impl;
class A {};
//--- M.cppm
export module M;
import :impl;
//--- Use.cpp
import M;
void test() {
A a; // Expected error. A is not visible here.
}
```
which is clearly wrong. The root cause is the implementation of
`isInCurrentModule` would return true if the module is a partition! So
in the above example, although Use.cpp is not a module unit,
`isInCurrentModule ` would still return true when the compiler tries to
see if the owning module of `A` is the current module. I believe this is
an oversight. This patch tries to fix this problem.
Reviewed By: iains
Differential Revision: https://reviews.llvm.org/D123837
C89 had a questionable feature where the compiler would implicitly
declare a function that the user called but was never previously
declared. The resulting function would be globally declared as
extern int func(); -- a function without a prototype which accepts zero
or more arguments.
C99 removed support for this questionable feature due to severe
security concerns. However, there was no deprecation period; C89 had
the feature, C99 didn't. So Clang (and GCC) both supported the
functionality as an extension in C99 and later modes.
C2x no longer supports that function signature as it now requires all
functions to have a prototype, and given the known security issues with
the feature, continuing to support it as an extension is not tenable.
This patch changes the diagnostic behavior for the
-Wimplicit-function-declaration warning group depending on the language
mode in effect. We continue to warn by default in C89 mode (due to the
feature being dangerous to use). However, because this feature will not
be supported in C2x mode, we've diagnosed it as being invalid for so
long, the security concerns with the feature, and the trivial
workaround for users (declare the function), we now default the
extension warning to an error in C99-C17 mode. This still gives users
an easy workaround if they are extensively using the extension in those
modes (they can disable the warning or use -Wno-error to downgrade the
error), but the new diagnostic makes it more clear that this feature is
not supported and should be avoided. In C2x mode, we no longer allow an
implicit function to be defined and treat the situation the same as any
other lookup failure.
Differential Revision: https://reviews.llvm.org/D122983
This adjusts the handling for:
export module M;
export namespace {};
export namespace N {};
export using namespace N;
In the first case, we were allowing empty anonymous namespaces
as part of an extension allowing empty top-level entities, but that seems
inappropriate in this case, since the linkage would be internal for the
anonymous namespace. We now report an error for this.
The second case was producing a warning diagnostic that this was
accepted as an extension - however the C++20 standard does allow this
as well-formed.
In the third case we keep the current practice that this is accepted with a
warning (as an extension). The C++20 standard says it's an error.
We also ensure that using decls are only applied to items with external linkage.
This adjusts error messages for exports involving redeclarations in modules to
be more specific about the reason that the decl has been rejected.
Differential Revision: https://reviews.llvm.org/D122119
This adds -no-opaque-pointers to clang tests whose output will
change when opaque pointers are enabled by default. This is
intended to be part of the migration approach described in
https://discourse.llvm.org/t/enabling-opaque-pointers-by-default/61322/9.
The patch has been produced by replacing %clang_cc1 with
%clang_cc1 -no-opaque-pointers for tests that fail with opaque
pointers enabled. Worth noting that this doesn't cover all tests,
there's a remaining ~40 tests not using %clang_cc1 that will need
a followup change.
Differential Revision: https://reviews.llvm.org/D123115
Without the fix ivars with anonymous types can trigger errors like
> error: 'TestClass::structIvar' from module 'Target' is not present in definition of 'TestClass' provided earlier
> [...]
> note: declaration of 'structIvar' does not match
It happens because types of ivars from different modules are considered
to be different. And it is caused by not merging anonymous `TagDecl`
from different modules.
To fix that I've changed `serialization::needsAnonymousDeclarationNumber`
to handle anonymous `TagDecl` inside `ObjCInterfaceDecl`. But that's not
sufficient as C code inside `ObjCInterfaceDecl` doesn't use interface
decl as a decl context but switches to its parent (TranslationUnit in
most cases). I'm changing that to make `ObjCContainerDecl` the lexical
decl context but keeping the semantic decl context intact.
Test "check-dup-decls-inside-objc.m" doesn't reflect a change in
functionality but captures the existing behavior to prevent regressions.
rdar://85563013
Differential Revision: https://reviews.llvm.org/D118525
As reported, using "-DTDIR=%t" with a path name of 'C:\Users\...' causes
a warning to be emitted about the use of \U without following hex digits.
Since the value is only required for the FileCheck cases resolve this by
omitting the -D from the compile lines.
in filesystems
It is simpler to search for module unit by -fprebuilt-module-path
option. However, the separator ':' of partitions is not friendly.
According to the discussion in https://reviews.llvm.org/D118586, I think
we get consensus to use '-' as the separator instead. The '-' is the
choice of GCC too.
Previously I thought it would be better to add an option. But I feel it
is over-engineering now. Another reason here is that there are too many
options for modules (for clang module mainly) now. Given it is not bad
to use '-' when searching, I think it is acceptable to not add an
option.
Reviewed By: iains
Differential Revision: https://reviews.llvm.org/D120874
When the -fdirectives-only option is used together with -E, the preprocessor
output reflects evaluation of if/then/else directives.
As such, it preserves defines and undefs of macros that are still live after
such processing. The intent is that this output could be consumed as input
to generate considered a C++20 header unit.
We strip out any (unused) defines that come from built-in, built-in-file or
command line; these are re-added when the preprocessed source is consumed.
Differential Revision: https://reviews.llvm.org/D121099
We wish to support emitting a pre-processed output for an importable
header unit, that can be consumed to produce the same header units as
the original source.
This means that ee need to find the original filename used to produce
the re-preprocessed output, so that it can be assigned as the module
name. This is peeked from the first line of the pre-processed source
when the action sets up the files.
Differential Revision: https://reviews.llvm.org/D121098
For header units we build the top level module directly from the header
that it represents and macros defined in this TU need to be emitted (when
such a definition is live at the end of the TU).
Differential Revision: https://reviews.llvm.org/D121097
This is support for the user-facing options to create importable header units
from headers in the user or system search paths (or to be given an absolute path).
This means that an incomplete header path will be passed by the driver and the
lookup carried out using the search paths present when the front end is run.
To support this, we introduce file fypes for c++-{user,system,header-unit}-header.
These terms are the same as the ones used by GCC, to minimise the differences for
tooling (and users).
The preprocessor checks for headers before issuing a warning for
"#pragma once" in a header build. We ensure that the importable header units
are recognised as headers in order to avoid such warnings.
Differential Revision: https://reviews.llvm.org/D121096
