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

2019-08-12 Thread John McCall via Phabricator via cfe-commits
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

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

2019-08-12 Thread John McCall via Phabricator via cfe-commits
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] D65994: Extended FPOptions with new attributes

2019-08-12 Thread John McCall via Phabricator via cfe-commits
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] D66042: [analyzer] Analysis: "Disable" core checkers

2019-08-12 Thread Csaba Dabis via Phabricator via cfe-commits
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] D65935: [ASTImporter] Import ctor initializers after setting flags.

2019-08-12 Thread Aleksei Sidorin via Phabricator via cfe-commits
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] D66043: Add to -Wparentheses case of bitwise-and ("&") and bitwise-or ("|") verses conditional operator ("?:")

2019-08-12 Thread Fangrui Song via Phabricator via cfe-commits
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] D66045: Improve detection of same value in comparisons

2019-08-12 Thread JF Bastien via Phabricator via cfe-commits
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 ("?:")

2019-08-12 Thread JF Bastien via Phabricator via cfe-commits
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] D66042: [analyzer] Analysis: "Disable" core checkers

2019-08-12 Thread Artem Dergachev via Phabricator via cfe-commits
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] D65994: Extended FPOptions with new attributes

2019-08-12 Thread Serge Pavlov via Phabricator via cfe-commits
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] D65994: Extended FPOptions with new attributes

2019-08-12 Thread Serge Pavlov via Phabricator via cfe-commits
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] D66122: [CodeGen] Emit dynamic initializers for static TLS vars in outlined scopes

2019-08-12 Thread Princeton Ferro via Phabricator via cfe-commits
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] D66043: Add to -Wparentheses case of bitwise-and ("&") and bitwise-or ("|") verses conditional operator ("?:")

2019-08-12 Thread Richard Trieu via Phabricator via cfe-commits
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 ("?:")

2019-08-12 Thread Richard Trieu via Phabricator via cfe-commits
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] D66045: Improve detection of same value in comparisons

2019-08-12 Thread Richard Trieu via Phabricator via cfe-commits
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] D66045: Improve detection of same value in comparisons

2019-08-12 Thread Richard Trieu via Phabricator via cfe-commits
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] D66122: [CodeGen] Emit dynamic initializers for static TLS vars in outlined scopes

2019-08-12 Thread Eli Friedman via Phabricator via cfe-commits
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] D65997: Add options rounding and exceptions to pragma fp

2019-08-12 Thread Serge Pavlov via Phabricator via cfe-commits
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] D65997: Add options rounding and exceptions to pragma fp

2019-08-12 Thread Serge Pavlov via Phabricator via cfe-commits
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] D65993: [NFC][clang] Adding argument based Phase list filtering to getComplicationPhases

2019-08-12 Thread Puyan Lotfi via Phabricator via cfe-commits
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] D66124: [clang-doc] Add missing check in tests

2019-08-12 Thread Diego Astiazar谩n via Phabricator via cfe-commits
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

LLVM buildmaster will be updated and restarted tonight

2019-08-12 Thread Galina Kistanova via cfe-commits
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

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

2019-08-12 Thread Alex L via cfe-commits
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

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

2019-08-12 Thread Alex Lorenz via cfe-commits
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

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

2019-08-12 Thread Leonard Chan via Phabricator via cfe-commits
leonardchan added a comment. In D64256#1626329 , @xazax.hun wrote: > In D64256#1626279 , @leonardchan > wrote: > > > In D64256#1626025 , @xazax.hun > > wrote: > > > > > In

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

2019-08-12 Thread G谩bor Horv谩th via Phabricator via cfe-commits
xazax.hun added a comment. In D64256#1626279 , @leonardchan wrote: > In D64256#1626025 , @xazax.hun wrote: > > > In D64256#1625998 , @leonardchan > > wrote: > > > > > Hi. I

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

2019-08-12 Thread Princeton Ferro via Phabricator via cfe-commits
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] D65919: [clang-tidy] Add llvm-prefer-register-over-unsigned check and apply it to LLVM

2019-08-12 Thread Daniel Sanders via Phabricator via cfe-commits
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] D65919: [clang-tidy] Add llvm-prefer-register-over-unsigned check and apply it to LLVM

2019-08-12 Thread Daniel Sanders via Phabricator via cfe-commits
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( --

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

2019-08-12 Thread Craig Topper via cfe-commits
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] D66121: Debug Info: Nest Objective-C property function decls inside their container.

2019-08-12 Thread Vedant Kumar via Phabricator via cfe-commits
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

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

2019-08-12 Thread Artem Dergachev via Phabricator via cfe-commits
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.

2019-08-12 Thread Adrian Prantl via Phabricator via cfe-commits
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] D66042: [analyzer] Analysis: "Disable" core checkers

2019-08-12 Thread Csaba Dabis via Phabricator via cfe-commits
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

2019-08-12 Thread Richard Trieu via Phabricator via cfe-commits
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] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

2019-08-12 Thread Leonard Chan via Phabricator via cfe-commits
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] D66068: cmake: Make building clang-shlib optional

2019-08-12 Thread Jan Vesely via Phabricator via cfe-commits
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] D66049: [analyzer] PR41729: Fix some false positives and improve strlcat and strlcpy modeling

2019-08-12 Thread Artem Dergachev via Phabricator via cfe-commits
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] D66046: Add new tautological compare warning for bitwise-or with a non-zero constant

2019-08-12 Thread Richard Trieu via Phabricator via cfe-commits
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] D60543: [clang] Update isDerivedFrom to support Objective-C classes 馃攳

2019-08-12 Thread Stephane Moore via Phabricator via cfe-commits
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

2019-08-12 Thread Richard Trieu via Phabricator via cfe-commits
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] D66043: Add to -Wparentheses case of bitwise-and ("&") and bitwise-or ("|") verses conditional operator ("?:")

2019-08-12 Thread JF Bastien via Phabricator via cfe-commits
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

2019-08-12 Thread JF Bastien via Phabricator via cfe-commits
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] D66042: [analyzer] Analysis: "Disable" core checkers

2019-08-12 Thread Csaba Dabis via Phabricator via cfe-commits
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] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

2019-08-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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] D66035: [WebAssembly] WIP: Add support for reference types

2019-08-12 Thread Dan Gohman via Phabricator via cfe-commits
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] D66042: [analyzer] Analysis: "Disable" core checkers

2019-08-12 Thread Artem Dergachev via Phabricator via cfe-commits
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] D66045: Improve detection of same value in comparisons

2019-08-12 Thread Artem Dergachev via Phabricator via cfe-commits
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

2019-08-12 Thread Csaba Dabis via Phabricator via cfe-commits
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] D66046: Add new tautological compare warning for bitwise-or with a non-zero constant

2019-08-12 Thread Artem Dergachev via Phabricator via cfe-commits
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] D66043: Add to -Wparentheses case of bitwise-and ("&") and bitwise-or ("|") verses conditional operator ("?:")

2019-08-12 Thread Richard Trieu via Phabricator via cfe-commits
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] D66045: Improve detection of same value in comparisons

2019-08-12 Thread Richard Trieu via Phabricator via cfe-commits
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] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

2019-08-12 Thread G谩bor Horv谩th via Phabricator via cfe-commits
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] D66044: Extend -Wtautological-overlap-compare to accept negative values and integer conversions

2019-08-12 Thread Artem Dergachev via Phabricator via cfe-commits
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

2019-08-12 Thread Leonard Chan via Phabricator via cfe-commits
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] D66068: cmake: Make building clang-shlib optional

2019-08-12 Thread Chris Bieneman via Phabricator via cfe-commits
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] D66046: Add new tautological compare warning for bitwise-or with a non-zero constant

2019-08-12 Thread JF Bastien via Phabricator via cfe-commits
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] D66046: Add new tautological compare warning for bitwise-or with a non-zero constant

2019-08-12 Thread Richard Trieu via Phabricator via cfe-commits
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] D66043: Add to -Wparentheses case of bitwise-and ("&") and bitwise-or ("|") verses conditional operator ("?:")

2019-08-12 Thread JF Bastien via Phabricator via cfe-commits
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] D66042: [analyzer] Analysis: "Disable" core checkers

2019-08-12 Thread Artem Dergachev via Phabricator via cfe-commits
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] D66044: Extend -Wtautological-overlap-compare to accept negative values and integer conversions

2019-08-12 Thread Richard Trieu via Phabricator via cfe-commits
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] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

2019-08-12 Thread Eli Friedman via Phabricator via cfe-commits
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] D66043: Add to -Wparentheses case of bitwise-and ("&") and bitwise-or ("|") verses conditional operator ("?:")

2019-08-12 Thread Richard Trieu via Phabricator via cfe-commits
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] D65287: [analyzer][CFG] Don't track the condition of asserts

2019-08-12 Thread Krist贸f Umann via Phabricator via cfe-commits
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] D65287: [analyzer][CFG] Don't track the condition of asserts

2019-08-12 Thread Krist贸f Umann via Phabricator via cfe-commits
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] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

2019-08-12 Thread Chris Hamilton via Phabricator via cfe-commits
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] D66042: [analyzer] Analysis: "Disable" core checkers

2019-08-12 Thread Artem Dergachev via Phabricator via cfe-commits
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] D65573: Add User docs for ASTImporter

2019-08-12 Thread Aleksei Sidorin via Phabricator via cfe-commits
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] D66068: cmake: Make building clang-shlib optional

2019-08-12 Thread Jan Vesely via Phabricator via cfe-commits
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] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

2019-08-12 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
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] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

2019-08-12 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
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] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-12 Thread Eli Friedman via Phabricator via cfe-commits
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] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-12 Thread Nick Desaulniers via Phabricator via cfe-commits
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

2019-08-12 Thread Jian Cai via Phabricator via cfe-commits
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] D66100: Add __has_builtin support for builtin function-like type traits.

2019-08-12 Thread Aaron Ballman via Phabricator via cfe-commits
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] D65575: [analyzer] Mention whether an event is about a condition in a bug report part 1

2019-08-12 Thread Krist贸f Umann via Phabricator via cfe-commits
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] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-12 Thread Jian Cai via Phabricator via cfe-commits
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] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-12 Thread Jian Cai via Phabricator via cfe-commits
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

2019-08-12 Thread Eli Friedman via Phabricator via cfe-commits
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

2019-08-12 Thread Vedant Kumar via Phabricator via cfe-commits
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] D65591: [AST] Add a flag indicating if any subexpression had errors

2019-08-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D65591#1625228 , @ilya-biryukov wrote: > In D65591#1625183 , @aaron.ballman > wrote: > > > In D65591#1625154 , @riccibruno > > wrote: > >

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

2019-08-12 Thread Artem Dergachev via Phabricator via cfe-commits
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] D65575: [analyzer] Mention whether an event is about a condition in a bug report part 1

2019-08-12 Thread Krist贸f Umann via Phabricator via cfe-commits
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] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

2019-08-12 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr accepted this revision. gribozavr 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] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

2019-08-12 Thread Krist贸f Umann via Phabricator via cfe-commits
Szelethus added a comment. Oh, btw, thank you for working on this! Comment at: clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:121-122 // Check if any of the enum values possibly match. bool PossibleValueMatch = llvm::any_of( DeclValues, Constraint

Re: r368600 - [Sema] Require a complete type for __builtin_bit_cast operands

2019-08-12 Thread Erik Pilkington via cfe-commits
Sure, I fixed this in r368610. Erik > On Aug 12, 2019, at 11:39 AM, Richard Smith wrote: > > On Mon, 12 Aug 2019 at 11:30, Erik Pilkington via cfe-commits > mailto:cfe-commits@lists.llvm.org>> wrote: > Author: epilk > Date: Mon Aug 12 11:31:27 2019 > New Revision: 368600 > > URL: http://llvm.

r368610 - [Sema] Check __builtin_bit_cast operand for completeness before materializing it.

2019-08-12 Thread Erik Pilkington via cfe-commits
Author: epilk Date: Mon Aug 12 12:29:43 2019 New Revision: 368610 URL: http://llvm.org/viewvc/llvm-project?rev=368610&view=rev Log: [Sema] Check __builtin_bit_cast operand for completeness before materializing it. This shouldn't be observable, but it doesn't make sense to materialize an incomple

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

2019-08-12 Thread Artem Dergachev via Phabricator via cfe-commits
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] D65999: [ASTImporter] Import additional flags for functions.

2019-08-12 Thread Aleksei Sidorin via Phabricator via cfe-commits
a_sidorin accepted this revision. a_sidorin added a comment. This revision is now accepted and ready to land. Hi Balazs, The change looks good. I think it can be committed after an unrelated part is removed. Comment at: clang/unittests/AST/ASTImporterTest.cpp:5373 +INSTANTIA

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

2019-08-12 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked an inline comment as done. jcai19 added inline comments. Comment at: llvm/test/CodeGen/ARM/gnu_mcount_nc.ll:1-6 +; RUN: llc -mtriple=armv7a-linux-gnueabihf %s -o - | FileCheck %s --check-prefix=CHECK-ARM +; RUN: llc -mtriple=armv7a-linux-gnueabihf %s -o - | FileChe

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

2019-08-12 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked an inline comment as done. ymandel added a comment. In D65877#1625616 , @gribozavr wrote: > I'd prefer we ban them completely, and let the user wrap them manually if > they need to. While some users would appreciate the ergonomics, I think

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

2019-08-12 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 214679. ymandel added a comment. Bans qualtype and type and adds corresponding comments and test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65877/new/ https://reviews.llvm.org/D65877 Files: clang/include

[PATCH] D65987: [clang-doc] Generate HTML links for children namespaces/records

2019-08-12 Thread Diego Astiazar谩n via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL368602: [clang-doc] Generate HTML links for children namespaces/records (authored by DiegoAstiazaran, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior t

[clang-tools-extra] r368602 - [clang-doc] Generate HTML links for children namespaces/records

2019-08-12 Thread Diego Astiazaran via cfe-commits
Author: diegoastiazaran Date: Mon Aug 12 11:42:46 2019 New Revision: 368602 URL: http://llvm.org/viewvc/llvm-project?rev=368602&view=rev Log: [clang-doc] Generate HTML links for children namespaces/records Path is now stored in the references to the child while serializing, then this path is used

Re: r368600 - [Sema] Require a complete type for __builtin_bit_cast operands

2019-08-12 Thread Richard Smith via cfe-commits
On Mon, 12 Aug 2019 at 11:30, Erik Pilkington via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: epilk > Date: Mon Aug 12 11:31:27 2019 > New Revision: 368600 > > URL: http://llvm.org/viewvc/llvm-project?rev=368600&view=rev > Log: > [Sema] Require a complete type for __builtin_bit_cast

r368600 - [Sema] Require a complete type for __builtin_bit_cast operands

2019-08-12 Thread Erik Pilkington via cfe-commits
Author: epilk Date: Mon Aug 12 11:31:27 2019 New Revision: 368600 URL: http://llvm.org/viewvc/llvm-project?rev=368600&view=rev Log: [Sema] Require a complete type for __builtin_bit_cast operands Fixes llvm.org/PR42936 Modified: cfe/trunk/lib/Sema/SemaCast.cpp cfe/trunk/test/SemaCXX/built

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

2019-08-12 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. In D65877#1625493 , @ymandel wrote: > I was going to add a test for `Type`/`QualType` and realized that they don't > carry any source location info. Therefore, I don't think they belong as > top-level matchers for rewrite rule

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

2019-08-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith created this revision. rsmith added a reviewer: aaron.ballman. Herald added subscribers: cfe-commits, kristina, jfb. Herald added a project: clang. Previously __has_builtin(__builtin_*) would return false for __builtin_*s that we modeled as keywords rather than as functions (because they ta

[PATCH] D25845: [CUDA] Ignore implicit target attributes during function template instantiation.

2019-08-12 Thread Artem Belevich via Phabricator via cfe-commits
tra marked an inline comment as done. tra added inline comments. Comment at: cfe/trunk/lib/Sema/SemaDecl.cpp:8416 +// in the CheckFunctionTemplateSpecialization() call below. +if (getLangOpts().CUDA & !isFunctionTemplateSpecialization) + maybeAddCUDAHostDeviceAttrs(N

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

2019-08-12 Thread Jacob Gravelle via Phabricator via cfe-commits
jgravelle-google added a comment. Overall direction looks good to me as well. Ditto that tests would be useful just to make it clearer how this is represented in IR. Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.h:47 + MVT getPointerTy(const DataLayout &DL,

  1   2   >