Like concepts checking, a trailing return type of a lambda
in a dependent context may refer to captures in which case
they may need to be rebuilt, so the map of local decl
should include captures.
This patch reveal a pre-existing issue.
`this` is always recomputed by TreeTransform.
`*this` (like all captures) only become `const`
after the parameter list.
However, if try to recompute the value of `this` (in a parameter)
during template instantiation while determining the type of the call operator,
we will determine it to be const (unless the lambda is mutable).
There is no good way to know at that point that we are in a parameter
or not, the easiest/best solution is to transform the type of this.
Note that doing so break a handful of HLSL tests.
So this is a prototype at this point.
Fixes#65067Fixes#63675
Reviewed By: erichkeane
Differential Revision: https://reviews.llvm.org/D159126
Functional-style cast (i.e. a simple-type-specifier or typename-specifier
followed by a parenthesize single expression [expr.type.conv]) is equivalent
to the C-style cast, so that makes sense they have identical behavior
including warnings.
This also matches GCC https://godbolt.org/z/b8Ma9Thjb.
Reviewed By: rnk, aaron.ballman
Differential Revision: https://reviews.llvm.org/D159133
Consider the following code:
#include <windows.h>
#include <TraceLoggingActivity.h>
#include <TraceLoggingProvider.h>
#include <winmeta.h>
TRACELOGGING_DEFINE_PROVIDER(
g_hMyComponentProvider,
"SimpleTraceLoggingProvider",
// {0205c616-cf97-5c11-9756-56a2cee02ca7}
(0x0205c616,0xcf97,0x5c11,0x97,0x56,0x56,0xa2,0xce,0xe0,0x2c,0xa7));
void test()
{
TraceLoggingFunction(g_hMyComponentProvider);
}
int main()
{
TraceLoggingRegister(g_hMyComponentProvider);
test();
TraceLoggingUnregister(g_hMyComponentProvider);
}
It compiles with MSVC, but clang-cl reports an error:
C:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\shared/TraceLoggingActivity.h(377,30): note: expanded from macro '_tlgThisFunctionName'
#define _tlgThisFunctionName __FUNCTION__
^
.\tl.cpp(14,5): error: cannot initialize an array element of type 'char' with an lvalue of type 'const char[5]'
TraceLoggingFunction(g_hMyComponentProvider);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The second commit is not needed to support above code, however, during isolated tests in ms_predefined_expr.cpp
I found that MSVC accepts code with constexpr, whereas clang-cl does not.
I see that in most places PredefinedExpr is supported in constant evaluation, so I didn't wrap my code with ``if(MicrosoftExt)``.
Reviewed By: cor3ntin
Differential Revision: https://reviews.llvm.org/D158591
This was deprecated in Clang 16 and scheduled for removal in Clang 18.
Time to remove it.
Reviewed By: nickdesaulniers, MaskRay
Differential Revision: https://reviews.llvm.org/D159373
When parsing a trailing return type / noexcept / constraint
of a generic lambda, we need to know that we are in a dependent
context. We currently don't because we only create a TemplateDecl
for the call operator one its fully parsed.
This patch attach a template decl to the call operator as soon
as the parameter declaration clause is parsed - the point at which
we have collected all template parameters
Fixes#64689
Reviewed By: erichkeane
Differential Revision: https://reviews.llvm.org/D159358
Builtin function `__builtin_isfpclass` now can be called for a vector
of floating-point values. In this case it is applied to the vector
elementwise and produces vector of integer values.
Differential Revision: https://reviews.llvm.org/D153339
In GCC, the .refptr stubs are only generated for x86_64, and only
for code models medium and larger (and medium is the default for
x86_64 since this was introduced). They can be omitted for
projects that are conscious about performance and size, and don't
need automatically importing dll data members, by passing -mcmodel=small.
In Clang/LLVM, such .refptr stubs are generated for any potentially
symbol reference that might end up autoimported. The .refptr stubs
are emitted for three separate reasons:
- Without .refptr stubs, undefined symbols are mostly referenced
with 32 bit wide relocations. If the symbol ends up autoimported
from a different DLL, a 32 bit relative offset might not be
enough to reference data in a different DLL, depending on runtime
loader layout.
- Without .refptr stubs, the runtime pseudo relocation mechanism
will need to temporarily make sections read-write-executable
if there are such relocations in the text section
- On ARM and AArch64, the immediate addressing encoded into
instructions isn't in the form of a plain 32 bit relative offset,
but is expressed with various bits scattered throughout two
instructions - the mingw runtime pseudo relocation mechanism
doesn't support updating offsets in that form.
If autoimporting is known not to be needed, the user can now
compile with -fno-auto-import, avoiding the extra overhead of
the .refptr stubs.
However, omitting them is potentially fragile as the code
might still rely on automatically importing some symbol without
the developer knowing. If this happens, linking still usually
will succeed, but users may encounter issues at runtime.
Therefore, if the new option -fno-auto-import is passed to the compiler
when driving linking, it passes the flag --disable-auto-import to
the linker, making sure that no symbols actually are autoimported
when the generated code doesn't expect it.
Differential Revision: https://reviews.llvm.org/D61670
This change addresses three issues:
1) A failure to propagate a MSInheritanceAttr prior to it being required by
an explicit class template instantiation definition.
2) The same MSInheritanceAttr attribute being attached to the same
ClassTemplateSpecializationDecl twice.
3) MSInheritanceAttr attributes were not being cloned nor marked as inherited
when added to new template instantiation declarations.
Sema::ActOnExplicitInstantiation() is responsible for the construction of
ClassTemplateSpecializationDecl nodes for explicit template instantiation
declarations and definitions. When invoked when a prior declaration node
corresponding to an implicit instantiation exists, the prior declaration
node is repurposed to represent the explicit instantiation declaration
or definition. When no previous declaration node exists or when the previous
node corresponds to an explicit declaration, a new node is allocated.
Previously, in either case, the function attempted to propagate any existing
MSInheritanceAttr attribute from the previous node, but did so regardless
of whether the previous node was reused (in which case the repurposed previous
node would gain a second attachment of the attribute; the second issue listed
above) or a new node was created. In the latter case, the attribute was not
propagated before it was required to be present when compiling for C++17 or
later (the first issue listed above). The absent attribute resulted in an
assertion failure that occurred during instantiation of the specialization
definition when attempting to complete the definition in order to determine
its alignment so as to resolve a lookup for a deallocation function for a
virtual destructor. This change addresses both issues by propagating the
attribute closer in time to when a new ClassTemplateSpecializationDecl node
is created and only when such a node is newly created.
Reviewed By: aaron.ballman, rnk
Differential Revision: https://reviews.llvm.org/D158869
For function multi-versioning using the target or target_clones
function attributes, currently we incorrectly set comdat for internal
linkage resolvers. This is problematic for ELF linkers
as GRP_COMDAT deduplication will kick in even with STB_LOCAL signature
(https://groups.google.com/g/generic-abi/c/2X6mR-s2zoc
"GRP_COMDAT group with STB_LOCAL signature").
In short, two `__attribute((target_clones(...))) static void foo()`
in two translation units will be deduplicated. Fix this.
Fix#65114
Reviewed By: erichkeane
Differential Revision: https://reviews.llvm.org/D158963
The compiler should not warn on code such as:
class [[maybe_unused]] MaybeUnusedClass {};
class C {
MaybeUnusedClass c;
};
Patch based on comments on the bug by Shafik and Aaron.
Fixes#61334
Differential revision: https://reviews.llvm.org/D159083
The original diagnostic does not cover all cases according to my reading
of the spec.
For the interrupt attribute, the spec indicates that if the compiler
does not support saving SSE, MMX, or x87 then the function should
be compiled with '-mgeneral-regs-only' (GCC requires this).
Alternatively, calling functions with the `no_caller_saved_registers`
attribute will not clobber state and can be done without disabling
these features.
The warning as implemented in upstream only detects the latter case but
does not consider that disabling the above features also solves the issue
of these register saves being undesirable due to inefficiency.
For the no_caller_saved_registers attribute, the interrupt spec also
indicates that in the absence of saving SSE, MMX and x87 state, these
functions should be compiled with '-mgeneral-regs-only' (also required
by GCC). It does not make any statements about calling other functions
with the attribute, but by extension the result is the same as with the
interrupt attribute (in clang, at least).
This patch handles the remaining cases by adjusting the diagnostic to:
1. Not be shown if the function is compiled without the SSE, MMX or x87
features enabled (i.e. with '-mgeneral-regs-only')
2. Also be shown for functions with the 'no_caller_saved_registers'
attribute
3. In addition to advising that the function should only call functions
with the `no_caller_saved_registers` attribute, the text also suggests
compiling with `-mgeneral-regs-only` as an alternative.
The interrupt spec is available at https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=5ed3cc7b66af4758f7849ed6f65f4365be8223be
and was taken from the issue that resulted in this diagnostic being
added (#26787)
Reviewed By: pengfei
Differential Revision: https://reviews.llvm.org/D159068
Since C++14 has been released for about nine years and most standard
libraries have implemented sized deallocation functions, it's time to
make this feature default again.
Reviewed By: rnk, aaron.ballman, #libc, ldionne, Mordante, MaskRay
Differential Revision: https://reviews.llvm.org/D112921
The original patch is incorrect since it marks too many calls to be
noinline. It shows that it is bad to do analysis in the frontend again.
This patch tries to mark the await_suspend function as noinlne only.
---
Close https://github.com/llvm/llvm-project/issues/56301
Close https://github.com/llvm/llvm-project/issues/64151
Close https://github.com/llvm/llvm-project/issues/65018
See the summary and the discussion of
https://reviews.llvm.org/D157070
to get the full context.
As @rjmccall pointed out, the key point of the root cause is that
currently we didn't implement the semantics for '@llvm.coro.save'
well ("after the await-ready returns false, the coroutine is considered
to be suspended ") well.
Since the semantics implies that we (the compiler) shouldn't write
the spills into the coroutine frame in the await_suspend. But now it is
possible due to some combinations of the optimizations so the semantics are
broken. And the inlining is the root optimization of such optimizations.
So in this patch, we tried to add the `noinline` attribute to the
await_suspend function.
This looks slightly problematic since the users are able to call the
await_suspend function standalone. This is limited by the
implementation. On the one hand, we don't want the workaround solution
(See the proposed solution later) to be too complex. On the other hand,
it is rare to call await_suspend standalone. Also it is not semantically
incorrect to do so since the inlining is not part of the C++ standard.
Also as an optimization, we don't add the `noinline` attribute to
the await_suspend function if the awaiter is an empty class. This should be
correct since the programmers can't access the local variables in
await_suspend if the awaiter is empty. I think this is necessary for
the performance since it is pretty common.
The long term solution is:
call @llvm.coro.await_suspend(ptr %awaiter, ptr %handle,
ptr @awaitSuspendFn)
Then it is much easier to perform the safety analysis in the middle
end. If it is safe to inline the call to awaitSuspend, we can replace it
in the CoroEarly pass. Otherwise we could replace it in the CoroSplit
pass.
Reviewed By: rjmccall
Differential Revision: https://reviews.llvm.org/D157833
This reverts commit 9d9c25f81456aace2bec4b58498a420e650007d9.
This reverts commit 19ab2664ad3182ffa8fe3a95bb19765e4ae84653.
This reverts commit c4672454743e942f148a1aff1e809dae73e464f6.
As the issue https://github.com/llvm/llvm-project/issues/65018 shows,
the previous fix introduce a regression actually. So this commit reverts
the fix by our policies.
This patch warns on snprintf calls whose n argument is known to be smaller than the size of the formatted string like
```
char buf[5];
snprintf(buf, 5, "Hello");
```
This is a counterpart of gcc's Wformat-truncation
Fixes https://github.com/llvm/llvm-project/issues/64871
Reviewed By: aaron.ballman, nickdesaulniers
Differential Revision: https://reviews.llvm.org/D158562
The implementation of __builtin_strncmp and other related builtins function use
getExtValue() to evaluate the size argument. This can cause a crash when the
value does not fit into an int64_t value, which is can be expected since the
type of the argument is size_t.
The fix is to switch to using getZExtValue().
This fixes: https://github.com/llvm/llvm-project/issues/64876
Differential Revision: https://reviews.llvm.org/D158557
It's been reported that when using __attribute__((format)) on non-variadic
functions, certain values that normally get promoted when passed as variadic
arguments now unconditionally emit a diagnostic:
```c
void foo(const char *fmt, float f) __attribute__((format(printf, 1, 2)));
void bar(void) {
foo("%g", 123.f);
// ^ format specifies type 'double' but the argument has type 'float'
}
```
This is normally not an issue because float values get promoted to doubles when
passed as variadic arguments, but needless to say, variadic argument promotion
does not apply to non-variadic arguments.
While this can be fixed by adjusting the prototype of `foo`, this is sometimes
undesirable in C (for instance, if `foo` is ABI). In C++, using variadic
templates, this might instead require call-site fixing, which is tedious and
arguably needless work:
```c++
template<typename... Args>
void foo(const char *fmt, Args &&...args) __attribute__((format(printf, 1, 2)));
void bar(void) {
foo("%g", 123.f);
// ^ format specifies type 'double' but the argument has type 'float'
}
```
To address this issue, we teach FormatString about a few promotions that have
always been around but that have never been exercised in the direction that
FormatString checks for:
* `char`, `unsigned char` -> `int`, `unsigned`
* `half`, `float16`, `float` -> `double`
This addresses issue https://github.com/llvm/llvm-project/issues/59824.
BEFORE this patch, compound assignment operator against uninitialized object such as uninit += 1 was diagnosed as subexpression not valid
This patch clarifies the reason for the error by saying that uninit is an uninitialized object.
Fixes https://github.com/llvm/llvm-project/issues/51536
Reviewed By: shafik, tbaeder
Differential Revision: https://reviews.llvm.org/D157855
Use member's type when printing.
This also fixes a bug in warning diagnostic for out of order
initialization with designated initializers so it points to a valid
source location when an anonymous member is being initialized.
Fixes https://github.com/llvm/llvm-project/issues/63759
Reviewed By: aaron.ballman, shafik
Differential Revision: https://reviews.llvm.org/D158526
even if we know what the result is going to be.
There may be side effects we ought not to ignore,
Fixes#64923
Reviewed By: ChuanqiXu
Differential Revision: https://reviews.llvm.org/D158601
This reverts commit f2583f3acf596cc545c8c0e3cb28e712f4ebf21b.
There is a large body of non-conforming C-like code using format strings
like this:
#define PRIuS "zu"
void h(size_t foo, size_t bar) {
printf("foo is %"PRIuS", bar is %"PRIuS, foo, bar);
}
Rejecting this code would be very disruptive. We could decide to do
that, but it's sufficiently disruptive that I think it requires
gathering more community consensus with an RFC, and Aaron indicated [1]
it's OK to revert for now so continuous testing systems can see past
this issue while we decide what to do.
[1] https://reviews.llvm.org/D153156#4607717
Close https://github.com/llvm/llvm-project/issues/56301
Close https://github.com/llvm/llvm-project/issues/64151
See the summary and the discussion of https://reviews.llvm.org/D157070
to get the full context.
As @rjmccall pointed out, the key point of the root cause is that
currently we didn't implement the semantics for '@llvm.coro.save' well
("after the await-ready returns false, the coroutine is considered to be
suspended ") well.
Since the semantics implies that we (the compiler) shouldn't write the
spills into the coroutine frame in the await_suspend. But now it is possible
due to some combinations of the optimizations so the semantics are
broken. And the inlining is the root optimization of such optimizations.
So in this patch, we tried to add the `noinline` attribute to the
await_suspend call.
Also as an optimization, we don't add the `noinline` attribute to the
await_suspend call if the awaiter is an empty class. This should be
correct since the programmers can't access the local variables in
await_suspend if the awaiter is empty. I think this is necessary for the
performance since it is pretty common.
Another potential optimization is:
call @llvm.coro.await_suspend(ptr %awaiter, ptr %handle,
ptr @awaitSuspendFn)
Then it is much easier to perform the safety analysis in the middle
end.
If it is safe to inline the call to awaitSuspend, we can replace it
in the CoroEarly pass. Otherwise we could replace it in the CoroSplit
pass.
Reviewed By: rjmccall
Differential Revision: https://reviews.llvm.org/D157833
Currenly both Clang and GCC support the following set of flags that control
code gen of signed overflow:
* -fwrapv: overflow is defined as in two-complement
* -ftrapv: overflow traps
* -fsanitize=signed-integer-overflow: if undefined (no -fwrapv), then overflow
behaviour is controlled by UBSan runtime, overrides -ftrapv
Howerver, clang ignores these flags for __builtin_abs(int) and its higher-width
versions, so passing minimum integer value always causes poison.
The same holds for *abs(), which are not handled in frontend at all but folded
to llvm.abs.* intrinsics during InstCombinePass. The intrinsics are not
instrumented by UBSan, so the functions need special handling as well.
This patch does a few things:
* Handle *abs() in CGBuiltin the same way as __builtin_*abs()
* -fsanitize=signed-integer-overflow now properly instruments abs() with UBSan
* -fwrapv and -ftrapv handling for abs() is made consistent with GCC
Fixes#45129 and #45794
Reviewed By: efriedma, MaskRay
Differential Revision: https://reviews.llvm.org/D156821
The x86_64 SysV ABI specifies that __int128 is passed either in
two registers (if available) or in a 16 byte aligned stack slot.
GCC implements this behavior. However, if only one free register
is available, LLVM will instead pass one half of the i128 in a
register, and the other on the stack.
Make sure that either both are passed in registers or both on the
stack.
Fixes https://github.com/llvm/llvm-project/issues/41784.
The patch is basically what craig.topper proposed to do there.
Differential Revision: https://reviews.llvm.org/D158169
Prior to this change clang didn't emit missing-field-initializers
warning for designated initializers. The comments say that it is done to
match gcc behavior. However, gcc behaves so only for C. For C++ warnings
are emitted.
Fixes https://github.com/llvm/llvm-project/issues/56628
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D157879
This reverts commit 1783185790de29b24d3850d33d9a9d586e6bbd39,
which broke the buildbots, starting with when it was first built in https://lab.llvm.org/buildbot/#/builders/85/builds/18390
(N.B. I think the patch is uncovering real bugs; the revert
is simply to keep the tree green and the buildbots useful, because I'm not confident how to
fix-forward all the found bugs.)
warn_drv_overriding_flag_option was added for clang-cl `/T*` options (D1290) and its group name was planned to be renamed to overriding-option.
The name -Woverriding-t-option does not make sense for other uses,
mostly related to -ffp-model= related diagnostics.
Reviewed By: hans, skan, dexonsmith
Differential Revision: https://reviews.llvm.org/D158137
This commit releases a checker that was developed to a stable level in
the Ericsson-internal fork of Clang Static Analyzer.
Note that the functionality of this checker overlaps with
core.UndefinedBinaryOperatorResult ("UBOR"), but there are several
differences between them:
(1) UBOR is only triggered when the constant folding performed by the
Clang Static Analyzer engine determines that the value of a binary
operator expression is undefined; this checker can report issues where
the operands are not constants.
(2) UBOR has unrelated checks for handling other binary operators, this
checker only examines bitwise shifts.
(3) This checker has a Pedantic flag and by default does not report
expressions (e.g. -2 << 2) that're undefined by the standard but
consistently supported in practice.
(4) UBOR exhibits buggy behavior in code that involves cast expressions,
e.g.
void foo(unsigned short s) {
if (s == 2) {
(void) ((unsigned int) s) << 16;
}
}
Later it would be good to eliminate this overlap (perhaps by deprecating
and then eliminating the bitwise shift handling in UBOR), but in my
opinion that belongs to separate commits.
Differential Revision: https://reviews.llvm.org/D156312
Co-authored-by: Endre Fulop <endre.fulop@sigmatechnology.se>
Currenly both Clang and GCC support the following set of flags that
control code gen of signed overflow:
* -fwrapv: overflow is defined as in two-complement
* -ftrapv: overflow traps
* -fsanitize=signed-integer-overflow: if undefined (no -fwrapv), then
overflow behaviour is controlled by UBSan runtime, overrides -ftrapv.
However, clang ignores these flags for __builtin_abs(int) and its
higher-width versions, so passing minimum integer value always causes
poison.
The same holds for *abs(), which are not handled in frontend at all but
folded to llvm.abs.* intrinsics during InstCombinePass. The intrinsics
are not instrumented by UBSan, so the functions need special handling
as well.
This patch does a few things:
* Handle *abs() in CGBuiltin the same way as __builtin_*abs()
* -fsanitize=signed-integer-overflow now properly instruments abs() with
UBSan
* -fwrapv and -ftrapv handling for abs() is made consistent with GCC
Fixes https://github.com/llvm/llvm-project/issues/45129
Fixes https://github.com/llvm/llvm-project/issues/45794
Differential Revision: https://reviews.llvm.org/D156821
