[PATCH] D65575: [analyzer] Mention whether an event is about a condition in a bug report part 1

Szelethus added a comment. A little early, gentle ping :) I'm getting kinda paranoid with the size of the stack, and how much I struggled with commiting last time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65575/new/ https://reviews.llvm.org/D

[PATCH] D65575: [analyzer] Mention whether an event is about a condition in a bug report part 1

NoQ added a comment. In D65575#1625738 , @Szelethus wrote: > A little early, gentle ping :) I'm getting kinda paranoid with the size of > the stack, and how much I struggled with commiting last time. Mmm, what are your thoughts on my suggestions with wo

[PATCH] D65591: [AST] Add a flag indicating if any subexpression had errors

aaron.ballman added a comment. In D65591#1625228 , @ilya-biryukov wrote: > In D65591#1625183 , @aaron.ballman > wrote: > > > In D65591#1625154 , @riccibruno > > wrote: > >

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

efriedma added a comment. > It seems global-isel does not fall back to DAGISel? It does, for targets where it's enabled by default, or if you use the right flags. I think you want `-global-isel -global-isel-abort=2`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D44672: [CodeGen] Disable UBSan for coroutine functions

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Lgtm, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D44672/new/ https://reviews.llvm.org/D44672

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

jcai19 updated this revision to Diff 214696. jcai19 added a comment. Add proper instruction selection options to unit test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65019/new/ https://reviews.llvm.org/D65019 Files: clang/lib/Basic/Targets/A

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

jcai19 added a comment. In D65019#1625780 , @efriedma wrote: > > It seems global-isel does not fall back to DAGISel? > > It does, for targets where it's enabled by default, or if you use the right > flags. I think you want `-global-isel -global-isel-abor

[PATCH] D65575: [analyzer] Mention whether an event is about a condition in a bug report part 1

Szelethus added a comment. In D65575#1625741 , @NoQ wrote: > In D65575#1625738 , @Szelethus wrote: > > > A little early, gentle ping :) I'm getting kinda paranoid with the size of > > the stack, and how much I stru

[PATCH] D66100: Add __has_builtin support for builtin function-like type traits.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some nits. Comment at: docs/LanguageExtensions.rst:65 + + ``__has_builtin`` should not be used to detect support for a built-in macro; + use `

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

jcai19 updated this revision to Diff 214698. jcai19 added a comment. Added back an accidently-deleted blank line. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65019/new/ https://reviews.llvm.org/D65019 Files: clang/lib/Basic/Targets/ARM.cpp l

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

nickdesaulniers added inline comments. Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:3485 + cast( + Op.getOperand(Op.getOperand(0).getValueType() == MVT::Other ? 1 : 0)) + ->getZExtValue(); `Op.getOperand(0).getValueType() == MVT::Oth

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

efriedma added inline comments. Comment at: llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp:1927 +.add(predOps(ARMCC::AL)) +.addReg(ARM::LR); + I think you need to ensure that lr actually contains the correct value, somehow. Normally the ca

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

ymandel updated this revision to Diff 214700. ymandel added a comment. Changed early return to assertion; updated test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65877/new/ https://reviews.llvm.org/D65877 Files: clang/include/clang/Tooling/R

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

ymandel marked 2 inline comments as done. ymandel added inline comments. Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:127 +if (!hasValidKind(Cases[I].Matcher)) + return std::vector(); +Buckets[Cases[I].Matcher.getSupportedKind()].emplace_back(I, Cases[I]

[PATCH] D66068: cmake: Make building clang-shlib optional

jvesely added a comment. In D66068#1625478 , @beanz wrote: > I generally am not a fan of adding more and more options. As long as you're > not linking the library it won't be generated by any of the check targets. > With the llvm dylib we've had many iss

[PATCH] D65573: Add User docs for ASTImporter

a_sidorin added a comment. That's incredible. Thank you! Comment at: cfe/trunk/docs/LibASTImporter.rst:215 +Node *Result = +const_cast(MatchRes[0].template getNodeAs("bindStr")); +assert(Result); We can avoid const_cast if we change the example

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

NoQ added a comment. In D66042#1625235 , @alexfh wrote: > In D66042#1624081 , @NoQ wrote: > > > +@alexfh because clang-tidy now finally has a way of safely disabling core > > checkers without causing crashes all ov

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

chrish_ericsson_atx marked an inline comment as done. chrish_ericsson_atx added a comment. In D66014#1625733 , @Szelethus wrote: > Oh, btw, thank you for working on this! Glad to help! Comment at: clang/lib/StaticAnalyzer/Checkers/En

[PATCH] D65287: [analyzer][CFG] Don't track the condition of asserts

Szelethus updated this revision to Diff 214708. Szelethus marked 12 inline comments as done. Szelethus added a comment. Addressing reviewer feedback! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65287/new/ https://reviews.llvm.org/D65287 Files: clang/include/clang/Analysis/CFG.h c

[PATCH] D65287: [analyzer][CFG] Don't track the condition of asserts

Szelethus added inline comments. Comment at: clang/lib/Analysis/CFG.cpp:5634 + // immediately caught. + if (std::any_of(Blk->begin(), Blk->end(), [](const CFGElement &Elm) { +if (Optional StmtElm = Elm.getAs()) xazax.hun wrote: > I am wondering, should

[PATCH] D66043: Add to -Wparentheses case of bitwise-and ("&") and bitwise-or ("|") verses conditional operator ("?:")

rtrieu marked 2 inline comments as done. rtrieu added inline comments. Comment at: test/Sema/parentheses.c:148 + + (void)(x | b ? 1 : 2); // expected-warning {{operator '?:' has lower precedence than '|'}} expected-note 2{{place parentheses}} + (void)(x & b ? 1 : 2); // expect

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

efriedma accepted this revision. efriedma 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/D65234/new/ https://reviews.llvm.org/D65234 ___

[PATCH] D66044: Extend -Wtautological-overlap-compare to accept negative values and integer conversions

rtrieu marked an inline comment as done. rtrieu added inline comments. Comment at: test/Sema/warn-unreachable.c:437 + if (~ 1) return; // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]] unaryOpNoFixit(); // expected-warning {{code will never be executed}} jfb wr

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

NoQ added a comment. In D66042#1625000 , @Szelethus wrote: > if we add this flag, people responsible for developing interafaces for the > analyzer might end up using it. And this is fine, that's supported. There's a very limited list of such people and

[PATCH] D66043: Add to -Wparentheses case of bitwise-and ("&") and bitwise-or ("|") verses conditional operator ("?:")

jfb added inline comments. Comment at: test/Sema/parentheses.c:156 + + (void)(x ^ b ? 1 : 2); // no warning, ^ is often used as logical xor + (void)(x || b ? 1 : 2); // no warning, logical operator rtrieu wrote: > jfb wrote: > > I don't understand why `^` is

[PATCH] D66046: Add new tautological compare warning for bitwise-or with a non-zero constant

rtrieu marked an inline comment as done. rtrieu added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8161 + "bitwise or with non-zero value always evaluates to true">, + InGroup, DefaultIgnore; def warn_tautological_overlap_comparison : Warning< --

[PATCH] D66046: Add new tautological compare warning for bitwise-or with a non-zero constant

jfb added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8161 + "bitwise or with non-zero value always evaluates to true">, + InGroup, DefaultIgnore; def warn_tautological_overlap_comparison : Warning< rtrieu wrote: > jfb wrote: > > Wh

[PATCH] D66068: cmake: Make building clang-shlib optional

beanz added a comment. I think you and I disagree here. General developer workflows don't need to include building `all` for every minor change. In my normal workflow I just re-run `check-llvm` or `check-clang` over and over again, only building the `all` target before I post a patch. With that

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

leonardchan added a comment. Hi. I noticed in our builders that both of the warnings introduced in this patch are being diagnosed for pointers that don't use GSL at all. I'm attempting to make a small reproducer now. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D66044: Extend -Wtautological-overlap-compare to accept negative values and integer conversions

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Looks great, thanks! I'd appreciate direct CFG tests for the part that changes the CFG (cf. `test/Analysis/cfg.cpp`), but i don't insist. Let's make sure @jfb is happy and commit :) =

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

xazax.hun added a comment. In D64256#1625998 , @leonardchan wrote: > Hi. I noticed in our builders that both of the warnings introduced in this > patch are being diagnosed for pointers that don't use GSL at all. I'm > attempting to make a small reproduce

[PATCH] D66045: Improve detection of same value in comparisons

rtrieu marked 3 inline comments as done. rtrieu added a comment. In D66045#1624389 , @jfb wrote: > Does this work for unions as well? This will work for union accesses via the same member name, but not different member names. union U { int x; int y;

[PATCH] D66043: Add to -Wparentheses case of bitwise-and ("&") and bitwise-or ("|") verses conditional operator ("?:")

rtrieu marked an inline comment as done. rtrieu added inline comments. Comment at: test/Sema/parentheses.c:156 + + (void)(x ^ b ? 1 : 2); // no warning, ^ is often used as logical xor + (void)(x || b ? 1 : 2); // no warning, logical operator jfb wrote: > rtri

[PATCH] D66046: Add new tautological compare warning for bitwise-or with a non-zero constant

NoQ added inline comments. Comment at: lib/Analysis/CFG.cpp:1118 +Expr::EvalResult Result; +if (!Constant->EvaluateAsInt(Result, *Context)) + return {}; It's kinda strange to me that we first confirm that it's a constant by doing `tryTransformToIntO

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1625971 , @NoQ wrote: > In D66042#1625000 , @Szelethus wrote: > > > Your patch title, and the things things that you said make me believe that > > there are only a handful of cor

[PATCH] D66045: Improve detection of same value in comparisons

NoQ added inline comments. Comment at: lib/AST/Expr.cpp:3931-3932 +case DeclRefExprClass: { + // DeclRefExpr without an ImplicitCastExpr can happen for integral + // template parameters. + const auto *DRE1 = cast(E1); Does this actually happen

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

NoQ added a comment. > use it locally What do you mean here? If you want to use the patch for evaluating your results, you might as well untick the checker in the scan-build's index.html interface. The point of having this patch landed is to allow users who are worried by false positives of sp

[PATCH] D66035: [WebAssembly] WIP: Add support for reference types

sunfish added a comment. x86 uses address spaces starting at 256 and counting up for its architecture-specific address spaces. The docs say "Address spaces 1-255 are currently reserved for user-defined code." so we should c

[PATCH] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

rsmith added a comment. I would like to better understand the big picture for descriptors, pointers, and so on. I'm not yet seeing how the pieces are going to fit together and not frequently require expensive operations. For example, pointer arithmetic requires determining the array bound of th

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1626122 , @NoQ wrote: > > use it locally > > What do you mean here? If you want to use the patch for evaluating your > results, you might as well untick the checker in the scan-build's index.html > interface. The point

[PATCH] D66043: Add to -Wparentheses case of bitwise-and ("&") and bitwise-or ("|") verses conditional operator ("?:")

jfb added inline comments. Comment at: test/Sema/parentheses.c:156 + + (void)(x ^ b ? 1 : 2); // no warning, ^ is often used as logical xor + (void)(x || b ? 1 : 2); // no warning, logical operator rtrieu wrote: > jfb wrote: > > rtrieu wrote: > > > jfb wrote:

[PATCH] D66044: Extend -Wtautological-overlap-compare to accept negative values and integer conversions

jfb added a comment. I haven't reviewed in depth, but generally this looks good. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66044/new/ https://reviews.llvm.org/D66044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.

[PATCH] D66046: Add new tautological compare warning for bitwise-or with a non-zero constant

rtrieu updated this revision to Diff 214733. rtrieu added a comment. Create new function isIntOrEnumConstantZero to use instead of EvaluateAsInt CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66046/new/ https://reviews.llvm.org/D66046 Files: include/clang/Analysis/CFG.h include/clan

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 🔍

This revision was automatically updated to reflect the committed changes. stephanemoore marked an inline comment as done. Closed by commit rL368632: [clang] Update isDerivedFrom to support Objective-C classes 🔍 (authored by stephanemoore, committed by ). Herald added a project: LLVM. Herald added

[PATCH] D66046: Add new tautological compare warning for bitwise-or with a non-zero constant

rtrieu marked 2 inline comments as done. rtrieu added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8161 + "bitwise or with non-zero value always evaluates to true">, + InGroup, DefaultIgnore; def warn_tautological_overlap_comparison : Warning< --

[PATCH] D66049: [analyzer] PR41729: Fix some false positives and improve strlcat and strlcpy modeling

NoQ added a comment. Oh, these false positives! Thanks!! Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1627 +svalBuilder.makeIntVal(1, sizeTy), sizeTy); + Optional freeSpaceNL = freeSpace.getAs(); + Plea

[PATCH] D66068: cmake: Make building clang-shlib optional

jvesely added a comment. In D66068#1625995 , @beanz wrote: > I think you and I disagree here. General developer workflows don't need to > include building `all` for every minor change. In my normal workflow I just > re-run `check-llvm` or `check-clang` o

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

leonardchan added a comment. In D64256#1626025 , @xazax.hun wrote: > In D64256#1625998 , @leonardchan > wrote: > > > Hi. I noticed in our builders that both of the warnings introduced in this > > patch are being d

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1625971 , @NoQ wrote: > In D66042#1625000 , @Szelethus wrote: > > > if we add this flag, people responsible for developing interafaces for the > > analyzer might end up using it.

[PATCH] D66045: Improve detection of same value in comparisons

rtrieu marked 2 inline comments as done. rtrieu added inline comments. Comment at: lib/AST/Expr.cpp:3931-3932 +case DeclRefExprClass: { + // DeclRefExpr without an ImplicitCastExpr can happen for integral + // template parameters. + const auto *DRE1 = cast(E1);

[PATCH] D66121: Debug Info: Nest Objective-C property function decls inside their container.

aprantl created this revision. aprantl added reviewers: vsk, rjmccall. aprantl added a project: debug-info. aprantl added a reviewer: davide. This fixes a crash in Clang. Starting with DWARF 5 we are emitting ObjC method declarations as children of their containing entity. This worked for interfa

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:94-98 + // If cast is implicit LValueToRValue, no conversion is taking place, + // and therefore no range check is needed. Don't analyze further. + if (CE->getCastKind() ==

[PATCH] D66121: Debug Info: Nest Objective-C property function decls inside their container.

vsk added a comment. Looks reasonable to me. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:3457 + // the interface type. + if (const auto *OMD = dyn_cast_or_null(D)) { +auto *ID = dyn_cast_or_null(CD); Return early on '! dyn_cast'? It doesn't look like 'D

r368635 - [X86] Remove 'Server' from Tigerlake description comments.

Author: ctopper Date: Mon Aug 12 17:00:27 2019 New Revision: 368635 URL: http://llvm.org/viewvc/llvm-project?rev=368635&view=rev Log: [X86] Remove 'Server' from Tigerlake description comments. Tigerlake is a client CPU not a server CPU. Modified: cfe/trunk/include/clang/Basic/X86Target.def

[PATCH] D65919: [clang-tidy] Add llvm-prefer-register-over-unsigned check and apply it to LLVM

dsanders added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp:28-29 CheckFactories.registerCheck("llvm-include-order"); +CheckFactories.registerCheck( +"llvm-prefer-register-over-unsigned"); CheckFactories.registerCheck( --

[PATCH] D65919: [clang-tidy] Add llvm-prefer-register-over-unsigned check and apply it to LLVM

dsanders updated this revision to Diff 214744. dsanders marked 9 inline comments as done. dsanders added a comment. - Correct alphabetical list in LLVMTidyModule.cpp and prevent add_new_check.py getting it wrong in future - Lookup Register class via ::llvm::Register instead of llvm::Register - Re

[PATCH] D66122: [CodeGen] Emit dynamic initializers for static TLS vars in outlined scopes

Prince781 created this revision. Prince781 added reviewers: ABataev, rsmith. Herald added subscribers: cfe-commits, jfb. Herald added a reviewer: jdoerfert. Herald added a project: clang. For static TLS vars only visible inside a function, clang will only generate an initializer inside the functi

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

xazax.hun added a comment. In D64256#1626279 , @leonardchan wrote: > In D64256#1626025 , @xazax.hun wrote: > > > In D64256#1625998 , @leonardchan > > wrote: > > > > > Hi. I

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

leonardchan added a comment. In D64256#1626329 , @xazax.hun wrote: > In D64256#1626279 , @leonardchan > wrote: > > > In D64256#1626025 , @xazax.hun > > wrote: > > > > > In

r368640 - clang-scan-deps: do not spawn threads when LLVM_ENABLE_THREADS is disabled

Author: arphaman Date: Mon Aug 12 17:36:35 2019 New Revision: 368640 URL: http://llvm.org/viewvc/llvm-project?rev=368640&view=rev Log: clang-scan-deps: do not spawn threads when LLVM_ENABLE_THREADS is disabled Modified: cfe/trunk/test/ClangScanDeps/regular_cdb.cpp cfe/trunk/tools/clang-sc

Re: r368354 - Mark clang-scan-deps test as requiring thread support

Hi Reid, I have fixed this issue in r368640, clang-scan-deps will no longer spawn threads if threading if disabled. Cheers, Alex On Thu, 8 Aug 2019 at 15:13, Alex L wrote: > Thanks for fixing this! > > I think changing clang-scan-deps to ignore -j when `LLVM_ENABLE_THREADS` > is probably a be

LLVM buildmaster will be updated and restarted tonight

to LLVM, cfe-commits, llvm-commits Hello everyone, LLVM buildmaster will be updated and restarted after 8PM Pacific time today. Thanks Galina ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co

[PATCH] D66124: [clang-doc] Add missing check in tests

DiegoAstiazaran created this revision. DiegoAstiazaran added reviewers: juliehockett, jakehehrlich. DiegoAstiazaran added a project: clang-tools-extra. Herald added subscribers: kadircet, arphaman. Path is now checked when comparing two Infos in the unit tests. https://reviews.llvm.org/D66124 F

[PATCH] D65993: [NFC][clang] Adding argument based Phase list filtering to getComplicationPhases

plotfi marked an inline comment as done. plotfi added a comment. @aaron.ballman How does this look to you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65993/new/ https://reviews.llvm.org/D65993 ___ c

[PATCH] D65997: Add options rounding and exceptions to pragma fp

sepavloff updated this revision to Diff 214750. sepavloff added a comment. Updated patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65997/new/ https://reviews.llvm.org/D65997 Files: clang/docs/LanguageExtensions.rst clang/include/clang/Basi

[PATCH] D65997: Add options rounding and exceptions to pragma fp

sepavloff marked 7 inline comments as done. sepavloff added inline comments. Comment at: clang/lib/Parse/ParsePragma.cpp:2697 PP.Lex(Tok); +StringRef ExpectedArgumentText; +switch (*FlagKind) { aaron.ballman wrote: > Same here. This case is different

[PATCH] D66122: [CodeGen] Emit dynamic initializers for static TLS vars in outlined scopes

efriedma added a comment. This might be a silly question, but what happens if the initializer for a thread-local variable refers to another thread-local variable? Do you need to initialize both variables? In what order? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D66045: Improve detection of same value in comparisons

rtrieu updated this revision to Diff 214752. rtrieu added a comment. Check array accesses. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66045/new/ https://reviews.llvm.org/D66045 Files: include/clang/AST/Expr.h lib/AST/Expr.cpp lib/Analysis/CFG.cpp lib/Sema/SemaExpr.cpp test

[PATCH] D66045: Improve detection of same value in comparisons

rtrieu marked 3 inline comments as done. rtrieu added inline comments. Comment at: test/Analysis/array-struct-region.cpp:31 + bool check() const { return this == this + 0; } + bool operator !() const { return this != this + 0; } NoQ wrote: > rtrieu wrote: > >

[PATCH] D66043: Add to -Wparentheses case of bitwise-and ("&") and bitwise-or ("|") verses conditional operator ("?:")

rtrieu marked an inline comment as done. rtrieu added inline comments. Comment at: test/Sema/parentheses.c:156 + + (void)(x ^ b ? 1 : 2); // no warning, ^ is often used as logical xor + (void)(x || b ? 1 : 2); // no warning, logical operator jfb wrote: > rtri

[PATCH] D66043: Add to -Wparentheses case of bitwise-and ("&") and bitwise-or ("|") verses conditional operator ("?:")

rtrieu updated this revision to Diff 214753. rtrieu added a comment. Update comments to explain why bitwise-xor is treated as a logical operator. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66043/new/ https://reviews.llvm.org/D66043 Files: lib/Sema/SemaExpr.cpp test/Sema/parenthe

[PATCH] D66122: [CodeGen] Emit dynamic initializers for static TLS vars in outlined scopes

Prince781 added a comment. In D66122#1626412 , @efriedma wrote: > This might be a silly question, but what happens if the initializer for a > thread-local variable refers to another thread-local variable? Do you need > to initialize both variables? In

[PATCH] D65994: Extended FPOptions with new attributes

sepavloff updated this revision to Diff 214754. sepavloff added a comment. Updated patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65994/new/ https://reviews.llvm.org/D65994 Files: clang/include/clang/AST/Stmt.h clang/include/clang/Basic/L

[PATCH] D65994: Extended FPOptions with new attributes

sepavloff marked 2 inline comments as done. sepavloff added a comment. In D65994#1625457 , @rjmccall wrote: > Since this setting changes IR output, you should write a test that emits IR > for source code and tests that you get the right IR output. This

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

NoQ added a comment. I'd like to hear @Szelethus's opinion one more time on this patch. I'm perfectly fine with abandoning the idea and going for in-checker suppressions, simply because there's at least one strong opinion against it and i don't want to push this further, but i just honestly thi

[PATCH] D66043: Add to -Wparentheses case of bitwise-and ("&") and bitwise-or ("|") verses conditional operator ("?:")

jfb accepted this revision. jfb added a comment. This revision is now accepted and ready to land. Herald added a subscriber: dexonsmith. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66043/new/ https://reviews.llvm.org/D66043 ___ cf

[PATCH] D66045: Improve detection of same value in comparisons

jfb added inline comments. Comment at: test/SemaCXX/self-comparison.cpp:87 +}; +} // namespace member_tests rtrieu wrote: > rtrieu wrote: > > jfb wrote: > > > The test only has `==`. Do other operators trigger? > > All the standard comparison operators will work

[PATCH] D66043: Add to -Wparentheses case of bitwise-and ("&") and bitwise-or ("|") verses conditional operator ("?:")

MaskRay added inline comments. Comment at: test/Sema/parentheses.c:148 + + (void)(x | b ? 1 : 2); // expected-warning {{operator '?:' has lower precedence than '|'}} expected-note 2{{place parentheses}} + (void)(x & b ? 1 : 2); // expected-warning {{operator '?:' has lower pr

[PATCH] D65935: [ASTImporter] Import ctor initializers after setting flags.

a_sidorin added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:3293 + // Import Ctor initializers. + if (auto *FromConstructor = dyn_cast(D)) { I suggest to move it closer to the function body import because import of ctor initializers is a part

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1626460 , @NoQ wrote: > I'd like to hear @Szelethus's opinion one more time on this patch. I'm > perfectly fine with abandoning the idea and going for in-checker > suppressions, simply because there's at least one stro

[PATCH] D65994: Extended FPOptions with new attributes

rjmccall added a comment. Oh, sorry, I confused several patches, then. I don't actually think you need to break down the patches this finely; it would be fine to take all three steps to implement the feature in one pass. It's only important to break down the patches into more incremental compo

[PATCH] D65744: [PR42707][OpenCL] Fix addr space deduction for auto

rjmccall added a comment. Isn't the general rule for template argument deduction (which this devolves to) just to ignore top-level qualifiers? And then you can substitute in the substituted type and end up with a properly qualified type for the parameter / variable, and you can add extra quali

[PATCH] D66121: Debug Info: Nest Objective-C property function decls inside their container.

rjmccall added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.h:406 + CGBuilderTy &Builder, + const ObjCContainerDecl *CD = nullptr); Why does this have to be an extra parameter? The method's DC shoul

<    1   2