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".

Turns out DLLAttr-inherited-from-class is only added to members during Sema::CheckCompletedClass -> Sema::checkClassLevelDLLAttribute, when friend re-decls of those members may already have been created.

Should this be backported to Clang 6? Not sure how widespread a problem this is in practice (it hit me with LibreOffice).

friendly ping

made the recommended changes

(need to call getAs<FunctionProtoType> instead of cast<FunctionProtoType> in one place, in case the name in the function decl is wrapped in parens, as happens in HarfBuzz's hb-buffer.cc)

As suggested, solve the issue instead by removing any "noexcept" from the typeinfo emitted for the -fsanitize=function checks.

added a small IR test

In D40720#958743, @vsk wrote:

Would it be possible to fix this by stripping the noexcept specifiers from both the function type used in the check and the one that is embedded in the prefix data? The downside is that we won't catch the case where the caller has a noexcept specifier and the callee doesn't, but that seems like an edge case to me, and we can think about fixing it in other ways later.

This sounds fine to me, and it avoids breaking the trapping mode.

In D40720#958677, @vsk wrote:

Please add a test.

Had to revert r320977/r320978 again with r320981/r320982: "At least http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-android/builds/6013/steps/annotate/logs/stdio complains about
ubsan::ubsan_handle_function_type_mismatch_abort (compiler-rt lib/ubsan/ubsan_handlers.cc) returning now despite being declared 'noreturn', so looks like a different approach is needed for the function_type_mismatch check
to be called also in cases that may ultimately succeed."