[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

sberg added a comment. For the record, the one other breakage of a LibreOffice build with this is the handful of places that needed https://git.libreoffice.org/core/+/433ab39b2175bdadb4916373cd2dc8e1aabc08a5%5E%21 "Adapt implicit OString return value construction to C++23 P2266R1": In a nutsh

[PATCH] D106924: [Preprocessor] -E -P: Ensure newline after 8 skipped lines.

sberg added a comment. Bisecting shows that this commit causes $ echo foo | clang -E -P - foo to emit a leading newline now. That breaks e.g. LibreOffice's `configure`, which uses `echo __clang_version__ | clang -E -P -` to obtain the value of that macro, and expects a single line of o

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

sberg added a comment. And, again for the record, with a build of LibreOffice with clang-cl (and `-Xclang -std=c++2b`) on Windows, at least against the C++ standard library from Visual Studio 2019 version 16.20.2, I ran into two issues in the standard library itself, when using `std::getline` a

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

sberg added a comment. In D99005#2843201 , @mizvekov wrote: > I have talked to the MSVC STL maintainers. They would be open to a pull > request for fixing this. > The repo can be found at https://github.com/microsoft/STL > Be my guest if you want to submi

[PATCH] D108567: Implement #pragma clang final extension

sberg added a comment. > Final macros will always warn on redefinition, including situations with > identical bodies and in system headers. But #define NULL syntax error #pragma clang final(NULL) #include void * p = NULL; does not produce any warnings/errors for me and indicates that

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

sberg added a comment. In D131307#3714629 , @shafik wrote: > In D131307#3713011 , @sberg wrote: > >> With this commit, >> >> $ cat test.cc >> #include "boost/numeric/conversion/cast.hpp" >> int main() { retu

[PATCH] D134529: [C++20][Clang] P2468R2 The Equality Operator You Are Looking For

sberg added a comment. I just ran into newly-failing $ cat test.cc struct S1 { bool operator==(int); bool operator!=(int); }; struct S2; bool operator ==(S2, S2); bool f(S1 s) { return 0 == s; } $ clang++ -std=c++20 -fsyntax-only test.cc test.cc:7:25: error: invalid o

[PATCH] D86881: Make -fvisibility-inlines-hidden apply to static local variables in inline functions on Darwin

sberg added a comment. Herald added a project: All. In D86881#2777808 , @arphaman wrote: > Hey, thanks for following up on this PR. I've done some more digging and I > think we can remove this Darwin-specific workaround in the near future. I'm > hoping t

[PATCH] D139847: Also allow __is_unsigned to be used as an identifier

sberg abandoned this revision. sberg added a comment. In D139847#4026966 , @aaron.ballman wrote: > Do you know if folks are hitting problems here in practice, or is this > speculative? It had hit me when building LibreOffice against libstdc++ 13 trunk,

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

sberg added a comment. There might be something fishy with this commit. My fresh from-scratch `cmake --build . && cmake --build . --target install` monorepo build now lacked e.g. the `bin/clang++` symlink in the installation dir, and locally reverting this commit fixed that. Repository: rG

[PATCH] D127593: [clang] Fix trivially copyable for copy constructor and copy assignment operator

sberg added a comment. Is it intended that a deleted copy assignment op as in `struct S { void operator =(S &) = delete; };` (though, somewhat oddly, not in `struct S { void operator =(S) = delete; };`) is now marked as trivial? I think that's the root cause why a build of PDFium with clang-cl

[PATCH] D127593: [clang] Fix trivially copyable for copy constructor and copy assignment operator

sberg added a comment. In D127593#3596883 , @royjacobson wrote: > In D127593#3596601 , @sberg wrote: > >> Is it intended that a deleted copy assignment op as in `struct S { void >> operator =(S &) = delete; };`

[PATCH] D127593: [clang] Fix trivially copyable for copy constructor and copy assignment operator

sberg added a comment. In D127593#3598020 , @royjacobson wrote: > Is it possible to check how often PDFium uses `__has_trivial_assign`? > Depending on how large this breakage is we might need to revert and proceed > more carefully.. The setup is a bit

[PATCH] D76572: Replace `T(x)` with `reinterpret_cast(x)` everywhere it means reinterpret_cast. No functional change

sberg added a comment. 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)`-

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

sberg added a comment. But how about literals like `'\x80'` where the promoted value depends on whether plain `char` is signed or unsigned? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58896/new/ https://reviews.llvm.org/D58896 ___

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

sberg added a comment. 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

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

sberg added a comment. In D58896#1738288 , @aaron.ballman wrote: > In D58896#1738263 , @sberg wrote: > > > In D58896#1737242 , @edward-jones > > wrote: > > > > > In D58896#

[PATCH] D71393: Default to -fno-use-init-array

sberg added a comment. The title should probably read "Default to -fuse-init-array" (dropping the "no-")? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71393/new/ https://reviews.llvm.org/D71393 ___ c

[PATCH] D73637: Fix handling of OO_Spaceship in DecodeOperatorCall

sberg added a comment. ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73637/new/ https://reviews.llvm.org/D73637 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73637: Fix handling of OO_Spaceship in DecodeOperatorCall

sberg added a comment. ping^2 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73637/new/ https://reviews.llvm.org/D73637 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73637: Fix handling of OO_Spaceship in DecodeOperatorCall

sberg created this revision. sberg added reviewers: rsmith, EricWF. sberg added a project: clang. Herald added a subscriber: cfe-commits. This seems to be a leftover from d30b23d6a54b3f0883914f3c2c2318a78edcbe67 "[c++2a] P0515R3: Support for overloaded operator<=>." (The corresponding llvm_unre

[PATCH] D73637: Fix handling of OO_Spaceship in DecodeOperatorCall

sberg updated this revision to Diff 241485. sberg added a comment. Minimal reproducer is template struct S; template void operator <=>(S, S); template void f(T x) { decltype(x <=> x) v; } Added a corresponding test to clang/test/SemaCXX/cxx2a-three-way-comparison.cpp. CHANGES SINCE LAST

[PATCH] D40167: In stdbool.h, define bool, false, true only in gnu++98

sberg created this revision. GCC has meanwhile corrected that with the similar https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=216679 "C++11 explicitly forbids macros for bool, true and false." https://reviews.llvm.org/D40167 Files: clang/lib/Headers/stdbool.h clang/test/Headers/st

[PATCH] D40167: In stdbool.h, define bool, false, true only in gnu++98

sberg added a comment. (This code had been introduced with http://llvm.org/viewvc/llvm-project?view=revision&revision=115028 "Define _Bool, bool, true, and false macros in when we're in a GNU-compatible C++ dialect. Fixes rdar://problem/8477819" mentioning: "Fixes rdar://problem/8477819." Bu

[PATCH] D40295: -fsanitize=vptr warnings on bad static types in dynamic_cast and typeid

sberg created this revision. Herald added a subscriber: kubamracek. ...when such an operation is done on an object during con-/destruction. This adds a test case to compiler-rt/test/ubsan/TestCases/TypeCheck/vptr.cpp that, unlike the existing test cases there, wants to detect multiple UBSan war

[PATCH] D40295: -fsanitize=vptr warnings on bad static types in dynamic_cast and typeid

sberg added a comment. Some items I'm unclear about: - Should those additional calls to EmitTypeCheck be restricted via those SkippedChecks or not? - Maybe there's a better way to add tests for this than to try shoehorn it into the existing vptr.cpp? https://reviews.llvm.org/D40295 __

[PATCH] D145123: Call MarkVirtualMembersReferenced on an actual class definition

sberg created this revision. sberg added reviewers: doug.gregor, aaron.ballman. sberg added a project: clang. Herald added a project: All. sberg requested review of this revision. ...rather than on potentially just a declaration. Without the fix, the newly added `clang/test/SemaCXX/warn-undefined

[PATCH] D145123: Call MarkVirtualMembersReferenced on an actual class definition

sberg added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:17940 LoadExternalVTableUses(); Class = Class->getCanonicalDecl(); std::pair::iterator, bool> That call of `getCanonicalDecl` originated with

[PATCH] D145123: Call MarkVirtualMembersReferenced on an actual class definition

This revision was automatically updated to reflect the committed changes. Closed by commit rGd812488d3c54: Call MarkVirtualMembersReferenced on an actual class definition (authored by sberg). Changed prior to commit: https://reviews.llvm.org/D145123?vs=501669&id=501852#toc Repository: rG LLV

[PATCH] D148505: Allow `__attribute__((warn_unused))` on individual constructors

sberg created this revision. sberg added a reviewer: erichkeane. Herald added a reviewer: aaron.ballman. Herald added a subscriber: jdoerfert. Herald added a project: All. sberg requested review of this revision. Herald added a project: clang. This takes inspiration from the standard `nodiscard` a

[PATCH] D148505: Allow `__attribute__((warn_unused))` on individual constructors

sberg added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2996 let Spellings = [GCC<"warn_unused">]; - let Subjects = SubjectList<[Record]>; + let Subjects = SubjectList<[Record, CXXConstructor]>; let Documentation = [Undocumented]; aaron

[PATCH] D148505: Allow `__attribute__((warn_unused))` on individual constructors

sberg added a comment. In D148505#4302702 , @aaron.ballman wrote: > Thank you for poking on this! FWIW, I don't know that there's a way to > cross-post to Discourse (but if I'm wrong and there is, I'd love to know > how!). Ping, any further input from

[PATCH] D148505: Allow `__attribute__((warn_unused))` on individual constructors

sberg added a comment. > But the fact that these two attributes already confuse users (AND we've got > unused as an attribute in the mix as well, which doesn't help) means we > should proceed with caution (and likely coordinate with GCC given that all > three of these attributes came from their

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

sberg added a comment. Since this commit (plus its follow-up https://github.com/llvm/llvm-project/commit/ce861ec782ae3f41807b61e855512aaccf3c2149 "[Clang][Sema] Add a temporary workaround in SemaConcept.cpp", to avoid `clang/lib/AST/ExprConstant.cpp:15332: bool clang::Expr::EvaluateAsConstantE

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

sberg added a comment. In D146178#4314999 , @sberg wrote: > Since this commit... ah, already taken care of with https://github.com/llvm/llvm-project/commit/3e850a6eea5277082a0b7b701754c86530d25c40 "Revert '[Clang][Sema] Fix comparison of constraint exp

[PATCH] D139847: Also allow __is_unsigned to be used as an identifier

sberg created this revision. sberg added reviewers: doug.gregor, aaron.ballman. Herald added a project: All. sberg requested review of this revision. Herald added a project: clang. ...similar to 068730992cf08d7d7e82e7bb147d85f429fc3ddd "libstdc++ 4.4 uses __is_signed as an identifier [...]" Star

[PATCH] D31133: Add LibreOffice Clang plugin to ExternalClangExamples.rst

sberg created this revision. https://reviews.llvm.org/D31133 Files: docs/ExternalClangExamples.rst Index: docs/ExternalClangExamples.rst === --- docs/ExternalClangExamples.rst +++ docs/ExternalClangExamples.rst @@ -90,3 +90,11 @@

[PATCH] D31133: Add LibreOffice Clang plugin to ExternalClangExamples.rst

This revision was automatically updated to reflect the committed changes. Closed by commit rL298490: Add LibreOffice Clang plugin to ExternalClangExamples.rst (authored by sberg). Changed prior to commit: https://reviews.llvm.org/D31133?vs=92334&id=92603#toc Repository: rL LLVM https://revi

[PATCH] D31235: Enhance -Wshadow to warn when shadowing typedefs or type aliases

sberg added a comment. Comitted follow-up diff 94007 as rL299522: Don't issue a warning if the shadowing declaration is in a class now. https://reviews.llvm.org/D31235 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D29271: Revert r293455, which breaks v8 with a spurious error. Testcase added.

sberg added a comment. Btw, ran into that with the even simpler test case template struct S { static typename T::t const n = 0; }; struct U { using t = int; }; int main() { return S::n; } https://reviews.llvm.org/D29271 ___ cfe-commit

[PATCH] D34417: Switch TestVisitor to Lang_C via -x c

This revision was automatically updated to reflect the committed changes. Closed by commit rL306373: Switch TestVisitor to Lang_C via -x c (authored by sberg). Changed prior to commit: https://reviews.llvm.org/D34417?vs=103263&id=104101#toc Repository: rL LLVM https://reviews.llvm.org/D3441

[PATCH] D34419: Make sure TraverseInitListExpr visits InitListExpr exactly twice

sberg added a comment. committed as https://reviews.llvm.org/rL306374 https://reviews.llvm.org/D34419 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D22128: Make CastExpr::getSubExprAsWritten look through implicit temporary under CK_ConstructorConversion

This revision was automatically updated to reflect the committed changes. Closed by commit rL306377: Make CastExpr::getSubExprAsWritten look through implicit temporary under… (authored by sberg). Changed prior to commit: https://reviews.llvm.org/D22128?vs=103449&id=104104#toc Repository: rL

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

sberg added a comment. see also https://reviews.llvm.org/rL306715 "Fixed -Wexceptions derived-to-base false positives" Repository: rL LLVM https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.

[PATCH] D35562: Support GNU extension StmtExpr in constexpr function

sberg created this revision. The assert.h in glibc 2.25 defines assert as a GNU-extension "statement expression", see https://sourceware.org/git/?p=glibc.git;a=commit;h=e077349ce589466eecd47213db4fae6b80ec18c4 "assert.h: allow gcc to detect assert(a = 1) errors". That caused Clang's -std=gnu+

[PATCH] D35564: Suppress -pedantic warnings about GNU extension StmtExpr in glibc's assert macro

sberg created this revision. See the mail thread starting at http://lists.llvm.org/pipermail/cfe-dev/2017-July/054666.html "[cfe-dev] -pedantic warnings in system headers?": The assert.h in glibc 2.25 defines assert as a GNU-extension "statement expression", see https://sourceware.org/git/?p=

[PATCH] D32346: [clang-tidy] New readability check for strlen argument

sberg added inline comments. Comment at: test/clang-tidy/readability-strlen-argument.cpp:11 + X = strlen(Str + 10); + // CHECK-FIXES: {{^}} X = (strlen(Str) - 10);{{$}} + but if any of the first ten chars in Str can be NUL, then the change is bogus? (I guess

[PATCH] D97687: [SEH] Fix capture of this in lambda functions

sberg added a comment. Bisecting shows that this started to break $ cat test.cc void f() noexcept { __try {} __finally {} } $ clang-cl -EHs -c test.cc clang-cl: ~/github.com/llvm/llvm-project/clang/lib/CodeGen/CGCleanup.h:568: void clang::CodeGen::EHScopeStack::popTerminate(): Assertion

[PATCH] D115715: [clang-tidy] Fix llvm-header-guard for Windows paths containing drive letter (e.g. C:).

sberg added a comment. > So the check, for a file called e.g. "C:\test\test.h" would suggest the guard > C:_TEST_TEST_H being an invalid name due to the presence of the colon. ...though the new suggestion `C__TEST_TEST_H` isn't ideal either, in that it uses an identifier that is reserved for th

[PATCH] D88220: [C++20] P1825R0: More implicit moves

sberg added a comment. This change causes #include std::shared_ptr x1; std::shared_ptr f() { std::shared_ptr & r = x1; r.reset(new int); return r; } int main() { std::shared_ptr x2 = f(); long n = x2.use_count(); return n; } when built with `-std=

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

sberg added a comment. FWIW, LibreOffice `make check` (which started to consistently fail with D88220 ) succeeds with this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98971/new/ https://reviews.llvm.org/D

[PATCH] D87030: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG215ed9b33ccb: Adapt CastExpr::getSubExprAsWritten to ConstantExpr (authored by sberg). Repository: rG LLVM Github Monor

[PATCH] D106924: [Preprocessor] -E -P: Ensure newline after 8 skipped lines.

sberg added a comment. In D106924#2919406 , @sberg wrote: > Bisecting shows that this commit causes > > $ echo foo | clang -E -P - > > foo > > to emit a leading newline now. > > That breaks e.g. LibreOffice's `configure`, which uses `echo > __clan

<    1   2