Smells like this breaks various bots due to a -fsanitize=...,... option now listing 18 instead of 17 items, see http://lab.llvm.org:8011/#builders/76/builds/363, http://lab.llvm.org:8011/#builders/93/builds/430, http://lab.llvm.org:8011/#builders/66/builds/315, http://lab.llvm.org:8011/#builders/7/builds/303.

ping

Is there a reason why "NoQ accepted this revision." kept this at "Needs Review" rather than moving it to "This revision is now accepted and ready to land."?

ping

ping

friendly ping

ping^2

(addressed the clang-tidy nitpick)

ping

I hit this with a call to getSubExprAsWriten from the LibreOffice Clang plugin, have no idea whether it can be hit from within the compiler itself. Not sure if clang/unittests/Tooling/CastExprTest.cpp is the best place to add a test for it.

I filed https://bugs.llvm.org/show_bug.cgi?id=47139 "Misspelled -fnostack-clash-protection" now.

Fixes all the false positives it had reported for LibreOffice (which had all involved expressions containing either ~ or +).

I think this generates a false positive with test.cc

enum E { E1 = 1, E2 = 2 };
bool f(E e) { return ((e & E1) ? 1 : 0) + ((e & E2) ? 1 : 0) > 1; }

and clang++ -fsyntax-only -Wtautological-value-range-compare test.cc

test.cc:2:62: warning: result of comparison of 1-bit unsigned value > 1 is always false [-Wtautological-value-range-compare]
bool f(E e) { return ((e & E1) ? 1 : 0) + ((e & E2) ? 1 : 0) > 1; }
                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~

In D85287#2199463, @rtrieu wrote:

Are you planning to allow this change to other warnings that use the same helper functions?

My naive assumption was that this warning had initially been restricted to integer literals and enumerators to avoid false positives. Hence my conservative approach of extending merely to constant integer expressions for now. Maybe Richard as the original author of the warning (IIUC) can shed some light on that original restriction?

In D76572#1936935, @grandinj wrote:

In D76572#1936861, @Quuxplusone wrote:

Nice. Does LibreOffice have anything (either in clang-tidy or in a paper guideline) against T(x)-style casts? E.g.

No, we don't have very many of those in our codebase, so we have left them alone.
Our plugin is designed to convert c-style casts to modern C++ casts.

ping^2

ping

Minimal reproducer is

The title should probably read "Default to -fuse-init-array" (dropping the "no-")?

In rG9e260c12bce77c80aa4da64ac44874687c684580#857454, @xbolva00 wrote:

GCC compatibility (they do it to reduce noise; so if we use same flag, we should diagnose same things).
I think it is good that clang and gcc warns same way now. But, if you want to improve it somehow (new flag?), patches are welcome.

Is there a rationale for that changed warning behavior? Even the current C++2a draft standard still e.g. deprecates copy operations when the destructor is user-declared (not just when it is user-provided), and I didn't find any indication that there is a plan to change that.

In D58896#1738288, @aaron.ballman wrote:

In D58896#1738263, @sberg wrote:

In D58896#1737242, @edward-jones wrote:

In D58896#1737113, @sberg wrote:

But how about literals like '\x80' where the promoted value depends on whether plain char is signed or unsigned?

If 'char' is signed and index into an array then this will typically trigger an -Warray-bounds warning because it references before the start of the array.

My thought was more that it might be useful as a kind of portability warning.

I'm not opposed to the warning per-se, but do you have evidence that the situation occurs in real-world code?

In D58896#1737242, @edward-jones wrote:

In D58896#1737113, @sberg wrote:

But how about literals like '\x80' where the promoted value depends on whether plain char is signed or unsigned?

If 'char' is signed and index into an array then this will typically trigger an -Warray-bounds warning because it references before the start of the array.

But how about literals like '\x80' where the promoted value depends on whether plain char is signed or unsigned?

eh, the summary here doesn't get updated from the actual git commit message :(

• Disallowed -fsanitize=function in combination with -fsanitize-minimal-runtime now.

Any thoughts on this? (cfe-commits had inadvertently been missing from subscribers, it touches clang as well as compiler-rt.)

friendly ping

friendly ping

friendly ping

friendly ping

Of course, adding missing tests reveals shortcomings in the new code.

Added missing tests at https://reviews.llvm.org/D61479 "Add tests for 'Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO'".

In D60760#1487342, @lebedev.ri wrote:

Did this get reviewed?

friendly ping

committed for now to get the crash fixed; if there are issues with the test they can be addressed later

The change itself should probably be uncontroversial (the bad cast had been there ever since getFunctionTypeWithExceptionSpec had been introduced with r221918), but I'm not sure about the test: It tests the relevant code somewhat indirectly; is it fine in clang/test/AST/?; or is such a test even overkill?

One problem I found with the macro __cpp_impl_destroying_delete not being conditional on language version is the following: Recent GCC trunk (since https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=76b94d4ba654e9af1882865933343d11f5c3b18b "Implement P0722R3, destroying operator delete.") contains

#if __cpp_impl_destroying_delete
#define __cpp_lib_destroying_delete 201806L
namespace std
{
  struct destroying_delete_t
  {
    explicit destroying_delete_t() = default;
  };
  inline constexpr destroying_delete_t destroying_delete{};
}
#endif // destroying delete

at "top-level" (i.e., not in a C++20-only #if or similar) in libstdc++-v3/libsupc++/new. That means that when using Clang against that GCC toolchain, #include <new> in C++03 mode will cause error: unknown type name 'constexpr'.

I run the new checker on LibreOffice project. I found ~25 false positives, which seems small enough to me. This false positives can be supressed easily.

I've silenced this scenario in r344898, thank you for raising the issue!

In D52400#1266341, @aaron.ballman wrote:

In D52400#1266307, @sberg wrote:

[...]

Then again, this is a case where you don't get any error but you do get a silent behavioral ambiguity without the current enumerator shadow diagnostic:

struct S1;
struct S2;
struct S3 {
  void S1();
  enum { S2 };

  void f(decltype(S2) s);
};

So there are cases where this behavior can be somewhat useful.

doesnt this make -Wshadow more aggressive for enumerators than for other entities?

see https://bugs.llvm.org/show_bug.cgi?id=37161 "clang-cl triggers ASTContext::getASTRecordLayout Assertion `D && 'Cannot get layout of forward declarations!''" for what appears to be fallout from this change

A random data point from trying this patch on the LibreOffice code base:

It looks like this caused a clang-cl regression https://bugs.llvm.org/show_bug.cgi?id=37146 "clang-cl emits special functions for non-trivial C-structs ('__destructor_8') introduced for Objective-C".