[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] 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] 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] 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] 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 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] 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] 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

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] 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] 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 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] 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] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

sberg added a comment. In D126864#3698798 , @sberg wrote: > I'm surprised that [...] > causes a warning? I would have expected it to be suppressed in this case, as > with the lax `-fstrict-flex-arrays=0` default, and only to hit with the > stricter `

[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] 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. With this commit, $ cat test.cc #include "boost/numeric/conversion/cast.hpp" int main() { return boost::numeric_cast(0L); } $ clang++ test.cc succeeds without any diagnostic, while with its parent commit https://github.com/llvm/llvm-project/commit/b364535304181

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

sberg added a comment. I'm surprised that $ cat test.c struct S { int m1; int m2[1]; }; void f(struct S * s) { s->m2[1] = 0; } $ clang -fsyntax-only -fstrict-flex-arrays=1 test.c test.c:6:5: warning: array index 1 is past the end of the array (which contains 1

[PATCH] D128783: [test] Check for more -fsanitize=array-bounds regressions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. sberg marked an inline comment as done. Closed by commit rG4996e3f68315: [test] Check for more -fsanitize=array-bounds behavior (authored by sberg). Repository: rG L

[PATCH] D128783: [test] Check for more -fsanitize=array-bounds regressions

sberg marked an inline comment as done. sberg added inline comments. Comment at: clang/test/CodeGen/bounds-checking-fam.c:59 + // CXX-STRICT-0-NOT: @__ubsan + return p->a[i] + (p->a)[i]; +} serge-sans-paille wrote: > Another C++-specific behavior: if the bound

[PATCH] D128783: [test] Check for more -fsanitize=array-bounds regressions

sberg updated this revision to Diff 441640. sberg added a comment. added test involving template argument substitution CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128783/new/ https://reviews.llvm.org/D128783 Files: clang/test/CodeGen/bounds-checking-fam.c Index: clang/test/CodeGe

[PATCH] D128783: [test] Check for more -fsanitize=array-bounds regressions

sberg updated this revision to Diff 441313. sberg added a comment. Updated the prospective git commit message as follow: [test] Check for more -fsanitize=array-bounds behavior ...that had temporarily regressed with (since reverted)

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

sberg added a comment. In D126864#3614297 , @MaskRay wrote: > Oh, I did not see this when pushing a test > efd90ffbfc427ad4c4675ac1fcae9d53cc7f1322 > . > Consider adding additional t

[PATCH] D128783: Check for more -fsanitize=array-bounds regressions

sberg created this revision. sberg added a reviewer: MaskRay. sberg added a project: clang. Herald added a subscriber: StephenFan. Herald added a project: All. sberg requested review of this revision. ...that had been introduced with (since reverted) https://github.com/llvm/llvm-project/commit/88

[PATCH] D128643: [clang] -fsanitize=array-bounds: treat all trailing size-1 arrays as flexible

sberg abandoned this revision. sberg added a comment. I'm abandoning this as the underlying https://reviews.llvm.org/D126864 "[clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays" has been reverted for now. I documented all my issues with that underlying commit (incl

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

sberg added a comment. In D126864#3612149 , @sberg wrote: > see https://reviews.llvm.org/D128643 "[clang] -fsanitize=array-bounds: treat > all trailing size-1 arrays as flexible" for another regression For the record (now that the original commit has be

[PATCH] D128643: [clang] -fsanitize=array-bounds: treat all trailing size-1 arrays as flexible

sberg created this revision. sberg added a reviewer: serge-sans-paille. sberg added a project: clang. Herald added a project: All. sberg requested review of this revision. ...even if the size resulted from a macro expansion. This reverts back to the behavior prior to https://github.com/llvm/llvm

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

sberg added a comment. see https://reviews.llvm.org/D128643 "[clang] -fsanitize=array-bounds: treat all trailing size-1 arrays as flexible" for another regression CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126864/new/ https://reviews.llvm.org/D126864 ___

[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] 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. 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] 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] D119721: [clang][lex] Use `ConstSearchDirIterator` in lookup cache

sberg added subscribers: rnk, sberg. sberg added a comment. Herald added a project: All. I started to experience Clang crashing when building Firebird (as part of building LibreOffice) in clang-cl mode on Windows, and I think it is due to this change in combination with D2733

[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] 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] D110310: State the kind of VerifyDiagnosticConsumer regex syntax (NFC)

sberg created this revision. sberg added a project: clang. sberg requested review of this revision. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D110310 Files: clang/include/clang/Frontend/VerifyDiagnosticConsumer.h Index: clang/include/clang/Frontend/VerifyDiagnosticConsum

[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

[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. 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] 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. 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] D99005: [clang] Implement P2266 Simpler implicit move

sberg added a comment. (In a build prior to https://reviews.llvm.org/rGc60dd3b2626a4d9eefd9f82f9a406b0d28d3fd72 "Revert '[clang] NRVO: Improvements and handling of more cases.'") I see the following (reduced from https://git.libreoffice.org/core/+/649313625b94e6b879848fc19b607b74375100bf/o3tl/

[PATCH] D103949: Only consider built-in compound assignment operators for -Wunused-but-set-*

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb5b9489b2415: Only consider built-in compound assignment operators for -Wunused-but-set-* (authored by sberg). Changed prior to commit: https://re

[PATCH] D103949: Only consider built-in compound assignment operators for -Wunused-but-set-*

sberg added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:7794-7798 +if (BO->getLHS()->getType()->isDependentType() || +BO->getRHS()->getType()->isDependentType()) +{ + if (BO->getOpcode() != BO_Assign) +return; mbenfiel

[PATCH] D103623: [Clang] Test case for -Wunused-but-set-variable, warn for volatile.

sberg accepted this revision. sberg added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103623/new/ https://reviews.llvm.org/D103623 ___ c

[PATCH] D103949: Only consider built-in compound assignment operators for -Wunused-but-set-*

sberg added a comment. In D103949#2807490 , @xbolva00 wrote: > gcc also ignores it? For reasons that I never looked into, GCC never warned for any of these cases. The Clang implementation appears to be way more aggressive (in a positive sense) to begi

[PATCH] D103949: Only consider built-in compound assignment operators for -Wunused-but-set-*

sberg created this revision. sberg added a reviewer: mbenfield. sberg added a project: clang. sberg requested review of this revision. At least LibreOffice has, for mainly historic reasons that would be hard to change now, a class `Any` with an overloaded `operator >>=` that semantically does no

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

sberg added a comment. Is it intentional that this warns about volatile variables as in void f(char * p) { volatile char c = 0; c ^= *p; } (I see that GCC warns about volatile too, at least when you replace the `^=` with `=`, so assume the answer is "yes", but would just like to

[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] 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] 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] 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] D87030: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

sberg added a comment. Ping; OK to submit? (I meanwhile also get this when compiling some code with plain Clang, not just with the LibreOffice Clang plugin where I saw it originally.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87030/new/ https://reviews.llvm.org/D87030 __

[PATCH] D90572: [clang] [MinGW] Allow using the vptr sanitizer

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

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

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

[PATCH] D89481: [scan-build] Fix clang++ pathname again

This revision was automatically updated to reflect the committed changes. Closed by commit rG7a5184ed951a: [scan-build] Fix clang++ pathname again (authored by sberg). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89481/new/ https://reviews.llvm.org

[PATCH] D89481: [scan-build] Fix clang++ pathname again

sberg updated this revision to Diff 301273. sberg edited the summary of this revision. sberg added a comment. 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."? Anyway, I added a test now.

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

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

[PATCH] D89481: [scan-build] Fix clang++ pathname again

sberg added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89481/new/ https://reviews.llvm.org/D89481 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin

[PATCH] D89481: [scan-build] Fix clang++ pathname again

sberg created this revision. sberg added reviewers: aadg, dcoughlin. Herald added a subscriber: Charusso. Herald added a project: clang. sberg requested review of this revision. e00629f777d9d62875730f40d266727df300dbb2 "[scan-build] Fix clang++ pathname" had removed the -MAJOR.MINOR suffix, but si

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

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

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

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

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

sberg updated this revision to Diff 292728. sberg added a comment. ping (addressed the clang-tidy nitpick) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87030/new/ https://reviews.llvm.org/D87030 Files: clang/lib/AST/Expr.cpp clang/unittests/Tooling/CastExprTest.cpp Index: clang

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

sberg added a comment. 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. Repository: rG LLVM Github

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

sberg created this revision. sberg added a reviewer: rsmith. Herald added a project: clang. sberg requested review of this revision. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D87030 Files: clang/lib/AST/Expr.cpp clang/unittests/Tooling/CastExprTest.cpp Index: clang/uni

[PATCH] D68720: Support -fstack-clash-protection for x86

sberg added a comment. I filed https://bugs.llvm.org/show_bug.cgi?id=47139 "Misspelled -fnostack-clash-protection" now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 _

[PATCH] D85778: More accurately compute the ranges of possible values for +, -, *, &, %.

sberg added a comment. Fixes all the false positives it had reported for LibreOffice (which had all involved expressions containing either ~ or +). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85778/new/ https://reviews.llvm.org/D85778 _

[PATCH] D85256: Add -Wtautological-value-range-compare warning.

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

[PATCH] D68720: Support -fstack-clash-protection for x86

sberg added inline comments. Herald added a subscriber: dang. Comment at: clang/include/clang/Driver/Options.td:1778 + HelpText<"Enable stack clash protection">; +def fnostack_clash_protection : Flag<["-"], "fnostack-clash-protection">, Group, + HelpText<"Disable stack clash p

[PATCH] D85287: Extend -Wtautological-bitwise-compare "bitwise or with non-zero value" warnings

sberg added a comment. In D85287#2199463 , @rtrieu wrote: > Are you planning to allow this change to other warnings that use the same > helper functions? No, I didn't plan to work on this further. Just scratching that itch of why Clang didn't emit that

[PATCH] D85287: Extend -Wtautological-bitwise-compare "bitwise or with non-zero value" warnings

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

[PATCH] D85287: Extend -Wtautological-bitwise-compare "bitwise or with non-zero value" warnings

sberg updated this revision to Diff 283204. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85287/new/ https://reviews.llvm.org/D85287 Files: clang/lib/Analysis/CFG.cpp clang/test/Sema/warn-bitwise-compare.c clang/test/SemaCXX/warn-bitwise-compare.cpp Index: clang/test/SemaCXX/warn-

[PATCH] D85287: Extend -Wtautological-bitwise-compare "bitwise or with non-zero value" warnings

sberg created this revision. sberg added reviewers: jordan_rose, rtrieu. Herald added a project: clang. sberg requested review of this revision. ...to C++ constant expression operands. (I had been puzzled why Clang had not found

[PATCH] D76646: Rename/refactor isIntegerConstantExpression to getIntegerConstantExpression

sberg added inline comments. Comment at: clang/include/clang/AST/Expr.h:513 - /// isIntegerConstantExpr - Return true if this expression is a valid integer - /// constant expression, and, if so, return its value in Result. If not a - /// valid i-c-e, return false and fill i

[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] 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 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 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] 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] 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] 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] 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. 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] D68581: Include leading attributes in DeclStmt's SourceRange

sberg marked 2 inline comments as done. sberg added inline comments. Comment at: clang/test/AST/sourceranges.cpp:155 + +#if __cplusplus >= 201703L + void cpp17() { aaron.ballman wrote: > Why is this guarded on C++17? `[[maybe_unused]]` is supported in every > l

[PATCH] D68581: Include leading attributes in DeclStmt's SourceRange

This revision was automatically updated to reflect the committed changes. Closed by commit rGdc3957ec215d: Include leading attributes in DeclStmt's SourceRange (authored by sberg). Changed prior to commit: https://reviews.llvm.org/D68581?vs=224823&id=225397#toc Repository: rG LLVM Github Mon

[PATCH] D68581: Include leading attributes in DeclStmt's SourceRange

sberg updated this revision to Diff 224823. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68581/new/ https://reviews.llvm.org/D68581 Files: clang/lib/Parse/ParseStmt.cpp clang/test/AST/sourceranges.cpp Index: clang/test/AST/sourceranges.cpp =

[PATCH] D68581: Include leading attributes in DeclStmt's SourceRange

sberg updated this revision to Diff 224303. sberg edited the summary of this revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68581/new/ https://reviews.llvm.org/D68581 Files: clang/lib/Parse/ParseStmt.cpp clang/test/AST/sourceranges.cpp Index: clang/test/AST/sourceranges.cp

[PATCH] D68581: Include leading attributes in DeclStmt's SourceRange

sberg created this revision. sberg added reviewers: aaron.ballman, rsmith, Nathan-Huckleberry. Herald added a project: clang. For a `DeclStmt` like [[maybe_unused]] int i; `getBeginLoc()` returned the start of `int` instead of the start of `[[` (see http://lists.llvm.org/pipermail/cfe-dev/201

[PATCH] D66350: Rudimentary support for Doxygen \retval command

This revision was automatically updated to reflect the committed changes. Closed by commit rL369345: Rudimentary support for Doxygen \retval command (authored by sberg, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://review

[PATCH] D66350: Rudimentary support for Doxygen \retval command

sberg created this revision. sberg added a reviewer: gribozavr. sberg added a project: clang. Herald added a subscriber: cfe-commits. ...so that at least a preceding \param etc. that lacks a description gets a -Wdocumentation warning (instead of erroneously treating the \retval ... text as its p

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

sberg added a comment. eh, the summary here doesn't get updated from the actual git commit message :( thanks for the reviews! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61479/new/ https://reviews.llvm.org/D61479 _

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

This revision was automatically updated to reflect the committed changes. Closed by commit rL366186: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO" (authored by sberg, committed by ). Herald added a subscriber: delcypher. Changed prior to commit: https://reviews.llvm.org/D6

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

sberg updated this revision to Diff 209865. sberg added a comment. - Disallowed -fsanitize=function in combination with -fsanitize-minimal-runtime now. - Left the ubsan test's RUN lines and \#include magic as-is, if there's no clearly better way to avoid this unfortunate complexity. CHANGES S

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

sberg added a subscriber: cfe-commits. sberg added a comment. Any thoughts on this? (cfe-commits had inadvertently been missing from subscribers, it touches clang as well as compiler-rt.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61479/new/ https://reviews.llvm.org/D61479 _

[PATCH] D60760: Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO

sberg added a comment. Added missing tests at https://reviews.llvm.org/D61479 "Add tests for 'Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO'". Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60760/new/ https://reviews.llvm.org/D60760 ___

[PATCH] D60760: Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO

sberg added a comment. In D60760#1487342 , @lebedev.ri wrote: > Did this get reviewed? I didn't get any responses at all, so decided to push it anyway. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60760/new/ https://rev

[PATCH] D60760: Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO

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 rL359759: Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO (authored by sberg, committed by ). Herald added a subs

[PATCH] D60760: Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO

sberg added a comment. friendly ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60760/new/ https://reviews.llvm.org/D60760 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D60760: Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO

sberg marked an inline comment as done. sberg added inline comments. Comment at: compiler-rt/lib/ubsan/ubsan_type_hash_itanium.cc:264 + return SANITIZER_NON_UNIQUE_TYPEINFO && TI1->__type_name[0] != '*' && + TI2->__type_name[0] != '*' && + !internal_strcmp(TI1->_

[PATCH] D60760: Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO

sberg created this revision. sberg added reviewers: filcab, marxin, rsmith. Herald added subscribers: llvm-commits, Sanitizers, cfe-commits, kristof.beyls, javed.absar, kubamracek. Herald added projects: clang, Sanitizers, LLVM. This follows up after b7692bc3e9ad2691fc07261904b88fb15f30696b

[PATCH] D58056: Look through typedefs in getFunctionTypeWithExceptionSpec

sberg added a comment. committed for now to get the crash fixed; if there are issues with the test they can be addressed later Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58056/new/ https://reviews.llvm.org/D58056 ___

  1   2   >