[PATCH] D30157: [analyzer] Improve valist check

2017-02-21 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 89187. xazax.hun marked 3 inline comments as done. xazax.hun added a comment. - Fixed a comment. https://reviews.llvm.org/D30157 Files: lib/StaticAnalyzer/Checkers/ValistChecker.cpp test/Analysis/valist-uninitialized-no-undef.c

[PATCH] D29967: Get class property selectors from property decl if it exists

2017-02-15 Thread David Herzka via Phabricator via cfe-commits
herzka updated this revision to Diff 88652. herzka added a comment. Rename selectors https://reviews.llvm.org/D29967 Files: lib/Sema/SemaExprObjC.cpp test/SemaObjC/objc-class-property.m Index: test/SemaObjC/objc-class-property.m

[PATCH] D30025: [compiler-rt] [builtins] Fix building atomic.c with GCC

2017-02-15 Thread Michał Górny via Phabricator via cfe-commits
mgorny created this revision. Herald added a subscriber: dberris. Make the use of #pragma redefine_extname and appropriate renames of builtins conditional to using clang. GCC used not to support it outside Solaris and currently seems to be very restrictive on applying it. In other words, it does

[PATCH] D30000: Fix for pr31836 - pp_nonportable_path on absolute paths: broken delimiters

2017-02-15 Thread Taewook Oh via Phabricator via cfe-commits
twoh added a comment. @eric_niebler I just tried it on Windows machine, and it just succeeded with no warnings/fix-it. Is that expected? https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D30015: [OpenMP] Add arch-specific directory to search path

2017-02-15 Thread Stephen Hines via Phabricator via cfe-commits
srhines added inline comments. Comment at: include/clang/Driver/ToolChain.h:304 + // Returns /lib//. When -fopenmp is specified, + // this directory is added to the linker search path if it exists + const std::string getOpenMPLibPath() const; Add a period at

[PATCH] D30000: Fix for pr31836 - pp_nonportable_path on absolute paths: broken delimiters

2017-02-16 Thread Axel Naumann via Phabricator via cfe-commits
karies added inline comments. Comment at: lib/Lex/PPDirectives.cpp:1983 + isLeadingSeparator = false; +else + Path.append(Component); eric_niebler wrote: > What happens on Windows for an absolute path like "C:/hello/world.h", I >

[PATCH] D29778: Declare lgamma library builtins as never being const

2017-02-15 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D29778 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D29967: Get class property selectors from property decl if it exists

2017-02-15 Thread David Herzka via Phabricator via cfe-commits
herzka updated this revision to Diff 88650. herzka added a comment. Added test, used auto https://reviews.llvm.org/D29967 Files: lib/Sema/SemaExprObjC.cpp test/SemaObjC/objc-class-property.m Index: test/SemaObjC/objc-class-property.m

[PATCH] D30015: [OpenMP] Add arch-specific directory to search path

2017-02-15 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. Why is this only for OpenMP? I imagine this should be done for **all** runtime libraries https://reviews.llvm.org/D30015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D30015: [OpenMP] Add arch-specific directory to search path

2017-02-15 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a subscriber: mgorny. Hahnfeld added a comment. Yes, that's the current state. We had some discussion on cfe-dev on that matter: http://lists.llvm.org/pipermail/cfe-dev/2017-January/052512.html Could you slightly adapt your patch so that other runtime libraries can follow this

[PATCH] D30015: [OpenMP] Add arch-specific directory to search path

2017-02-16 Thread Michał Górny via Phabricator via cfe-commits
mgorny added inline comments. Comment at: lib/Driver/Tools.cpp:3267 + if (llvm::sys::fs::is_directory(CandidateLibPath)) +CmdArgs.push_back(Args.MakeArgString("-L" + CandidateLibPath)); + Don't you also need rpath for it? Or is this purely for static

[PATCH] D30015: [OpenMP] Add arch-specific directory to search path

2017-02-16 Thread Michał Górny via Phabricator via cfe-commits
mgorny added inline comments. Comment at: lib/Driver/ToolChain.cpp:327 + llvm::sys::path::append(Path, "lib", OSLibName, + getArchName()); + return Path.str(); I would suggest using arch type, to avoid e.g. i386/i486/i586 mess. It's

[PATCH] D28771: [Analyzer] Various fixes for the IteratorPastEnd checker

2017-02-21 Thread Balogh , Ádám via Phabricator via cfe-commits
baloghadamsoftware updated this revision to Diff 89211. baloghadamsoftware added a comment. checkBind replaces checking of DeclStmt, CXXConstructExpr and assignment operators. Incremention by 0 is not a bug anymore regardless the position of the iterator. https://reviews.llvm.org/D28771

[PATCH] D30157: [analyzer] Improve valist check

2017-02-21 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: test/Analysis/valist-uninitialized-no-undef.c:19 + // FIXME: There should be no warning for this. + (void)va_arg(*fst, int); // expected-warning{{va_arg() is called on an uninitialized va_list}} expected-note{{va_arg() is called on

[PATCH] D30035: Add const to function parameters

2017-02-21 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. Absent I good motivation (i.e, a documented, significant performance change), I don't think we want this - not because it's not a reasonable change (it is!), but because it's an ABI break. It's unfortunate the `__atoms` were not passed as const in the first place.

[PATCH] D29819: Introduce an 'external_source_symbol' attribute that describes the origin and the nature of a declaration

2017-02-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: include/clang/Basic/AttrDocs.td:1007 + +defined_in=\ *string-literal* + The name of the source container in which the declaration was defined. The arphaman wrote: > aaron.ballman wrote: > > Would this hold

[PATCH] D21815: [clang-tidy] Add 'included from' details to warning message.

2017-02-21 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. Ping ;) Do you have time to finish this? https://reviews.llvm.org/D21815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30009: Add support for '#pragma clang attribute'

2017-02-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:308-311 + // - An attribute requires delayed parsing (LateParsed is on) + // - An attribute has no GNU/CXX11 spelling + // - An attribute has no subject list + // - An attribute derives from a

[PATCH] D30170: Function definition may have uninstantiated body

2017-02-21 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff updated this revision to Diff 89206. sepavloff added a comment. Implement `isDefined` through `isThisDeclarationADefinition`. https://reviews.llvm.org/D30170 Files: include/clang/AST/Decl.h lib/AST/Decl.cpp lib/Sema/SemaDecl.cpp test/SemaCXX/cxx0x-cursory-default-delete.cpp

[PATCH] D30239: enable -flto=thin, -flto-jobs=, and -fthinlto-index= in clang-cl

2017-02-21 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added a comment. In https://reviews.llvm.org/D30239#683116, @pcc wrote: > Can you please add a ThinLTO test to lld before adding driver support? Is clang-cl using lld as default? How is the switch done? Ideally we should have a nice error message from the driver if -flto is used

[PATCH] D29819: Introduce an 'external_source_symbol' attribute that describes the origin and the nature of a declaration

2017-02-21 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added inline comments. Comment at: include/clang/Basic/AttrDocs.td:1005 +language=\ *identifier* + The source language in which this declaration was defined. + aaron.ballman wrote: > This being an identifier makes me wonder about languages that aren't

[PATCH] D30082: Fix assertion when generating debug information for deduced template specialization types.

2017-02-21 Thread Richard Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL295794: Fix assertion failure when generating debug information for a variable (authored by rsmith). Changed prior to commit: https://reviews.llvm.org/D30082?vs=88943=89300#toc Repository: rL LLVM

[PATCH] D27827: [ObjC] CodeGen support for @available on macOS

2017-02-21 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington updated this revision to Diff 89305. erik.pilkington added a comment. This new patch addresses all of Alex's comments: - Remove `ObjC` from function names - Rename `_IsOSVersionAtLeast` -> `__isOSVersionAtLeast` - Improve testcase https://reviews.llvm.org/D27827 Files:

[PATCH] D30238: [Driver] Enable SafeStack for Fuchsia targets

2017-02-21 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr created this revision. The runtime support is provided directly by the Fuchsia system C library. Repository: rL LLVM https://reviews.llvm.org/D30238 Files: lib/Driver/ToolChains.cpp lib/Driver/ToolChains.h Index: lib/Driver/ToolChains.h

[PATCH] D30238: [Driver] Enable SafeStack for Fuchsia targets

2017-02-21 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment. Could you also please update test/Driver/fuchsia.c and add a case for `-fsanitize=safe-stack`? Repository: rL LLVM https://reviews.llvm.org/D30238 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D30238: [Driver] Enable SafeStack for Fuchsia targets

2017-02-21 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr added a comment. This assumes https://reviews.llvm.org/D30237 Repository: rL LLVM https://reviews.llvm.org/D30238 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30239: enable -flto=thin, -flto-jobs=, and -fthinlto-index= in clang-cl

2017-02-21 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added subscribers: cfe-commits, pcc. pcc added a comment. Can you please add a ThinLTO test to lld before adding driver support? https://reviews.llvm.org/D30239 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D30174: [Sema][ObjC] Warn about 'performSelector' calls with selectors that return record types

2017-02-21 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. I'm not sure how common it is to pass a function that doesn't return an object or void, I think it's OK to allow returning primitive types if you think being too strict would catch too many false positives. Comment at: lib/AST/DeclObjC.cpp:987

[PATCH] D29221: clang-format-vsix: "format on save" feature

2017-02-21 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added inline comments. Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:276 + +if (Vsix.IsDocumentDirty(document)) +{ hans wrote: > Perhaps return early if `!Vsix.IsDocumentDirty(document)` instead? Yes, will

[PATCH] D29221: clang-format-vsix: "format on save" feature

2017-02-21 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added inline comments. Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:39 private string style = "file"; +private bool formatOnSaveEnabled = false; +private string formatOnSaveFileExtensions = hans wrote: >

[PATCH] D30241: AMDGPU: Add fmed3 half builtin

2017-02-21 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm created this revision. Herald added subscribers: tpr, dstuttard, tony-tye, yaxunl, nhaehnle, wdng, kzhuravl. https://reviews.llvm.org/D30241 Files: include/clang/Basic/BuiltinsAMDGPU.def lib/Basic/Targets.cpp lib/CodeGen/CGBuiltin.cpp test/CodeGenOpenCL/builtins-amdgcn-gfx9.cl

[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point

2017-02-21 Thread Jonas Hahnfeld via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL295808: [OpenMP] Generate better diagnostics for cancel and cancellation point (authored by Hahnfeld). Changed prior to commit: https://reviews.llvm.org/D30135?vs=89106=89326#toc Repository: rL LLVM

[PATCH] D28348: [analyzer] Taught the analyzer about Glib API to check Memory-leak

2017-02-22 Thread Leslie Zhai via Phabricator via cfe-commits
xiangzhai updated this revision to Diff 89331. xiangzhai added a comment. Hi Anna, I added **svalBinMulOp** to take BO_Mul evalBinOp for g_malloc_n's two arguments: CE->getArg(0) and CE->getArg(1), so it does **NOT** need to change **MallocMemAux** any more, but just use it in this way:

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-22 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 89337. sammccall marked 7 inline comments as done. sammccall added a comment. Address review comments. https://reviews.llvm.org/D30210 Files: include-fixer/IncludeFixer.cpp include-fixer/SymbolIndexManager.cpp

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-22 Thread Manuel Klimek via Phabricator via cfe-commits
klimek added inline comments. Comment at: include-fixer/find-all-symbols/FindAllSymbols.cpp:251 + } else { +assert(false && "Must match a NamedDecl!"); + } You can use llvm_unreachable instead. Or do you actually want to fall through in release mode?

[PATCH] D28348: [analyzer] Taught the analyzer about Glib API to check Memory-leak

2017-02-22 Thread Leslie Zhai via Phabricator via cfe-commits
xiangzhai updated this revision to Diff 89333. xiangzhai added a comment. Fixed the confused State->getSVal(CE->getArg(1), C.getLocationContext()); with CE->getArg(1) issue. Repository: rL LLVM https://reviews.llvm.org/D28348 Files: lib/StaticAnalyzer/Checkers/MallocChecker.cpp

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-22 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. Thanks for the contributions! Comment at: include-fixer/find-all-symbols/FindAllSymbols.cpp:244 - const auto *ND = Result.Nodes.getNodeAs("decl"); - assert(ND && "Matched declaration must be a NamedDecl!"); + auto report = ::reportSymbol; + const

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-22 Thread Sam McCall via Phabricator via cfe-commits
sammccall planned changes to this revision. sammccall added a comment. OK, some changes to come here: - klimek thinks using a set as a pseudo-map with `mutable` is evil, which is a fair point - the current `SymbolReporter` interface doesn't provide enough information to deduplicate

[PATCH] D30248: [libclang] Fix crash in member access code completion with implicit base

2017-02-22 Thread Erik Verbruggen via Phabricator via cfe-commits
erikjv created this revision. If there is an unresolved member access AST node, and the base is implicit, do not access/use it for generating candidate overloads for code completion results (because the base is a nullptr). Fixes PR31093. https://reviews.llvm.org/D30248 Files:

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-22 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Thanks for the review, still learning the style :) Comment at: include-fixer/find-all-symbols/FindAllSymbols.cpp:250 + } else { +assert(!"Must match a NamedDecl!"); + } hokein wrote: > Is the preceding `!` intended? This does

[PATCH] D30220: Only enable AddDiscriminator pass when -fdebug-info-for-profiling is true

2017-02-21 Thread Dehao Chen via Phabricator via cfe-commits
danielcdh created this revision. AddDiscriminator pass is only useful for sample pgo. This patch restricts AddDiscriminator to -fdebug-info-for-profiling so that it does not introduce unecessary debug size increases for non-sample-pgo builds. https://reviews.llvm.org/D30220 Files:

[PATCH] D30220: Only enable AddDiscriminator pass when -fdebug-info-for-profiling is true

2017-02-21 Thread David Blaikie via Phabricator via cfe-commits
dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Did you want to enable all DebugInfoForProfiling features when there's a sample profile specified? (not sure if all of the features are needed when stitching things back up, or if some of

[PATCH] D29221: clang-format-vsix: "format on save" feature

2017-02-21 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment. I'm not really qualified to review the implementation details, but this seems good as far as I can tell. A few comments, mostly style related. Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:39 private string style = "file";

[PATCH] D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable

2017-02-21 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment. This seems to be stuck. Bruno, Richard, do you think there's a chance this can be fixed for 4.0? https://reviews.llvm.org/D29753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D29819: Introduce an 'external_source_symbol' attribute that describes the origin and the nature of a declaration

2017-02-21 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman updated this revision to Diff 89298. arphaman marked 8 inline comments as done. arphaman added a comment. I've addressed Aaron's comments and made the language a string literal. Repository: rL LLVM https://reviews.llvm.org/D29819 Files: include/clang/Basic/Attr.td

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-22 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 89360. sammccall added a comment. Changes based on offline discussion: - mutable SignalInfo data split into SignalInfo::Signals - yaml serialized data is SignalInfo::WithSignals, which is currently a pair -

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

2017-02-20 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Nice check! Thank you for working on this! Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:30 +" The old user defined 'RandomFunction' is not usable for 'shuffle'. You " +"need to " +"make additional changes if you want a

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

2017-02-20 Thread Mads Ravn via Phabricator via cfe-commits
madsravn added inline comments. Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:30 +" The old user defined 'RandomFunction' is not usable for 'shuffle'. You " +"need to " +"make additional changes if you want a specific random function.";

[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point

2017-02-20 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: lib/Sema/SemaOpenMP.cpp:1959 +static bool CheckCancelRegion(Sema , OpenMPDirectiveKind CurrentRegion, + OpenMPDirectiveKind CancelRegion, Should be `checkCancelRegion`

[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point

2017-02-20 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In https://reviews.llvm.org/D30135#681354, @ABataev wrote: > Not sure that this is better because at first, we need to be sure that this > nesting is allowed. Why do we need to perform some additional analysis if > nesting is not allowed at all?

[PATCH] D30111: [clang-format] Add a test to check at once all the Mozilla coding style

2017-02-20 Thread Manuel Klimek via Phabricator via cfe-commits
klimek added inline comments. Comment at: test/Format/check-coding-style-mozilla.cpp:7-9 +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at

[PATCH] D30111: [clang-format] Add a test to check at once all the Mozilla coding style

2017-02-20 Thread Sylvestre Ledru via Phabricator via cfe-commits
sylvestre.ledru updated this revision to Diff 89093. sylvestre.ledru marked 3 inline comments as done. https://reviews.llvm.org/D30111 Files: unittests/Format/CheckCodingStyleMozilla.cpp Index: unittests/Format/CheckCodingStyleMozilla.cpp

[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point

2017-02-20 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added inline comments. Comment at: lib/Sema/SemaOpenMP.cpp:1959 +static bool CheckCancelRegion(Sema , OpenMPDirectiveKind CurrentRegion, + OpenMPDirectiveKind CancelRegion, ABataev wrote: > Should be `checkCancelRegion`

[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point

2017-02-20 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added inline comments. Comment at: lib/Sema/SemaOpenMP.cpp:1968-1973 + if (CancelRegion != OMPD_parallel && CancelRegion != OMPD_for && + CancelRegion != OMPD_sections && CancelRegion != OMPD_taskgroup) { +SemaRef.Diag(StartLoc,

[PATCH] D30111: [clang-format] Add a test to check at once all the Mozilla coding style

2017-02-20 Thread Sylvestre Ledru via Phabricator via cfe-commits
sylvestre.ledru updated this revision to Diff 89094. sylvestre.ledru added a comment. Sorry, I tried to rename the file but this is too confusing for Phabricator it seems... https://reviews.llvm.org/D30111 Files: unittests/Format/check-coding-style-mozilla.cpp Index:

[PATCH] D30111: [clang-format] Add a test to check at once all the Mozilla coding style

2017-02-20 Thread Sylvestre Ledru via Phabricator via cfe-commits
sylvestre.ledru added inline comments. Comment at: test/Format/check-coding-style-mozilla.cpp:48 +, +public Y +{ krasimir wrote: > sylvestre.ledru wrote: > > krasimir wrote: > > > This does not check precisely what the comment

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

2017-02-20 Thread Mads Ravn via Phabricator via cfe-commits
madsravn added inline comments. Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:81 + Stream << "shuffle("; + FirstArgument->printPretty(Stream, nullptr, Ctx.getPrintingPolicy()); + Stream << ", "; xazax.hun wrote: > madsravn wrote: > >

[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point

2017-02-20 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. I see. I think it is better to check the `CancelRegion` just before call of `CheckNestingOfRegions()` function. You need to extract checks for `CancelRegion` from `ActOnOpenMPCancellationPointDirective()` and `ActOnOpenMPCancelDirective()` functions into a standalone

[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point

2017-02-20 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld updated this revision to Diff 89099. Hahnfeld edited the summary of this revision. Hahnfeld added a comment. new static function `CheckCancelRegion` https://reviews.llvm.org/D30135 Files: lib/Sema/SemaOpenMP.cpp test/OpenMP/cancel_messages.cpp

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

2017-02-20 Thread Mads Ravn via Phabricator via cfe-commits
madsravn added inline comments. Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:81 + Stream << "shuffle("; + FirstArgument->printPretty(Stream, nullptr, Ctx.getPrintingPolicy()); + Stream << ", "; xazax.hun wrote: > madsravn wrote: > >

[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point

2017-02-20 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. Not sure that this is better because at first, we need to be sure that this nesting is allowed. Why do we need to perform some additional analysis if nesting is not allowed at all? https://reviews.llvm.org/D30135 ___

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

2017-02-20 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:81 + Stream << "shuffle("; + FirstArgument->printPretty(Stream, nullptr, Ctx.getPrintingPolicy()); + Stream << ", "; madsravn wrote: > xazax.hun wrote: > >

[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point

2017-02-20 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: lib/Sema/SemaOpenMP.cpp:1959 +static bool CheckCancelRegion(Sema , OpenMPDirectiveKind CurrentRegion, + OpenMPDirectiveKind CancelRegion, Hahnfeld wrote: > ABataev wrote: > > Should be

[PATCH] D29943: [clang-format] Align block comment decorations

2017-02-20 Thread Krasimir Georgiev via Phabricator via cfe-commits
krasimir added a comment. In https://reviews.llvm.org/D29943#678618, @sylvestre.ledru wrote: > Maybe this could be added to the release notes? Sounds good! Could you please point me to the release docs? I don't seem to find a clang-format--specific section online. Repository: rL LLVM

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

2017-02-20 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:30 +" The old user defined 'RandomFunction' is not usable for 'shuffle'. You " +"need to " +"make additional changes if you want a specific random function.";

[PATCH] D23418: [analyzer] Added a reusable constraint system to the CloneDetector

2017-02-20 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor updated this revision to Diff 89084. teemperor added a comment. - Removed all the deprecated `\brief`s I couldn't find any actual regression in the code now, so from my side it's ok to merge it. https://reviews.llvm.org/D23418 Files: include/clang/Analysis/CloneDetection.h

[PATCH] D29643: [analyzer] Do not duplicate call graph nodes for function that has definition and forward declaration.

2017-02-20 Thread Aleksei Sidorin via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL295644: [analyzer] Do not duplicate call graph nodes for functions that have definition… (authored by a.sidorin). Changed prior to commit: https://reviews.llvm.org/D29643?vs=87388=89089#toc

[PATCH] D30111: [clang-format] Add a test to check at once all the Mozilla coding style

2017-02-20 Thread Krasimir Georgiev via Phabricator via cfe-commits
krasimir added inline comments. Comment at: test/Format/check-coding-style-mozilla.cpp:10 + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +if (true) { sylvestre.ledru wrote: > krasimir wrote: > > What is tested here? Brace styles? > Yes, do you

[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point

2017-02-20 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld updated this revision to Diff 89106. Hahnfeld marked 3 inline comments as done. Hahnfeld edited the summary of this revision. Hahnfeld added a comment. Address review comment's and apply new naming style to checkNestingOfRegions https://reviews.llvm.org/D30135 Files:

[PATCH] D29643: [analyzer] Do not duplicate call graph nodes for function that has definition and forward declaration.

2017-02-20 Thread Aleksei Sidorin via Phabricator via cfe-commits
a.sidorin added a comment. Anna, I will commit. Thank you! https://reviews.llvm.org/D29643 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30135: [OpenMP] Generate better diagnostics for cancel and cancellation point

2017-02-20 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: lib/Sema/SemaOpenMP.cpp:1968-1973 + if (CancelRegion != OMPD_parallel && CancelRegion != OMPD_for && + CancelRegion != OMPD_sections && CancelRegion != OMPD_taskgroup) { +SemaRef.Diag(StartLoc,

[PATCH] D29419: [Analyzer] Checker for mismatched iterators

2017-02-20 Thread Balogh , Ádám via Phabricator via cfe-commits
baloghadamsoftware marked 2 inline comments as done. baloghadamsoftware added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp:311 + +void MismatchedIteratorChecker::checkPostStmt(const DeclStmt *DS, +

[PATCH] D30170: Function definition may have uninstantiated body

2017-02-20 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff created this revision. Current implementation of `FunctionDecl::isDefined` does not take into account declarations that do not have a body, but it can be instantiated from a templated definition. This behavior creates problems when processing friend functions defined in class templates.

[PATCH] D29419: [Analyzer] Checker for mismatched iterators

2017-02-20 Thread Balogh , Ádám via Phabricator via cfe-commits
baloghadamsoftware updated this revision to Diff 89123. baloghadamsoftware added a comment. Updated according to the comments. https://reviews.llvm.org/D29419 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/CMakeLists.txt

[PATCH] D30157: [analyzer] Improve valist check

2017-02-20 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ValistChecker.cpp:178 +VaListModelledAsArray = Cast->getCastKind() == CK_ArrayToPointerDecay; + const MemRegion *Reg = SV.getAsRegion(); + if (const auto *DeclReg = Reg->getAs()) { I

[PATCH] D28445: [Analyzer] Extend taint propagation and checking

2017-02-20 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. Yeah, i think this is now as easy as i expected it to be :) Still, the new API is in need of better documentation, because the notion of default binding is already confusing, and the new use-case we now have for this API is even more confusing. I don't instantly see a good

[PATCH] D29770: [Assembler] Inline assembly diagnostics test.

2017-02-20 Thread Sanne Wouda via Phabricator via cfe-commits
sanwou01 abandoned this revision. sanwou01 added a comment. Please see https://reviews.llvm.org/D30167 for an attempt to test this from llc. https://reviews.llvm.org/D29770 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D30087: [Driver] Unify linking of OpenMP runtime

2017-02-20 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld updated this revision to Diff 89110. Hahnfeld added a comment. rebase https://reviews.llvm.org/D30087 Files: lib/Driver/Tools.cpp test/Driver/fopenmp.c Index: test/Driver/fopenmp.c === --- test/Driver/fopenmp.c +++

[PATCH] D29944: libclang: Print namespaces for typedefs and type aliases

2017-02-20 Thread Michael via Phabricator via cfe-commits
redm123 added inline comments. Comment at: test/Misc/diag-template-diffing.cpp:27 // CHECK-ELIDE-NOTREE: no matching function for call to 'f' -// CHECK-ELIDE-NOTREE: candidate function not viable: no known conversion from 'vector' to 'vector' for 1st argument +//

[PATCH] D30174: [Sema][ObjC] Warn about 'performSelector' calls with selectors that return record types

2017-02-20 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman created this revision. The `performSelector` family of methods from Foundation use `objc_msgSend` to dispatch the selector invocations to objects. However, method calls to methods that return record types might have to use the `objc_msgSend_stret` as the return value won't find into

[PATCH] D29944: libclang: Print namespaces for typedefs and type aliases

2017-02-20 Thread Michael via Phabricator via cfe-commits
redm123 added inline comments. Comment at: test/Misc/diag-template-diffing.cpp:27 // CHECK-ELIDE-NOTREE: no matching function for call to 'f' -// CHECK-ELIDE-NOTREE: candidate function not viable: no known conversion from 'vector' to 'vector' for 1st argument +//

[PATCH] D29419: [Analyzer] Checker for mismatched iterators

2017-02-20 Thread Aditya Kumar via Phabricator via cfe-commits
hiraditya added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp:375 + "MismatchedIterator"); +auto *N = C.generateNonFatalErrorNode(State, ); +if (!N) { This can be rewritten as:

[PATCH] D26061: [analyzer] Refactor and simplify SimpleConstraintManager

2017-02-18 Thread Dominic Chen via Phabricator via cfe-commits
ddcc updated this revision to Diff 89054. ddcc added a comment. Rebase, incorporate https://reviews.llvm.org/D22862 https://reviews.llvm.org/D26061 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h

[PATCH] D15227: [analyzer] Valist checkers.

2017-02-18 Thread Anna Zaks via Phabricator via cfe-commits
zaks.anna added a comment. > But as far as I remember, this produced false negatives in the tests not > false positives. Could you double check that? Maybe you still have some notes in your mail box or just by looking at the code. Did none of the checks work or just some of them? Also,

[PATCH] D30157: [analyzer] Improve valist check

2017-02-19 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun created this revision. This patch makes the valist check more robust to the different kind of ASTs that are generated on different platforms: Generated on x86_64-pc-linux-gnu: |-TypedefDecl 0x1d07a40 <> implicit __builtin_ms_va_list 'char *' | `-PointerType 0x1d07a00 'char *'

[PATCH] D15227: [analyzer] Valist checkers.

2017-02-19 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In https://reviews.llvm.org/D15227#681127, @zaks.anna wrote: > > But as far as I remember, this produced false negatives in the tests not > > false positives. > > Could you double check that? Maybe you still have some notes in your mail box > or just by looking at

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

2017-02-19 Thread Mads Ravn via Phabricator via cfe-commits
madsravn created this revision. Herald added subscribers: JDevlieghere, mgorny. random_shuffle was deprecated by C++14 and will be removed by C++17. This check will find and replace usage of random_shuffle with its modern counterpart (shuffle). https://reviews.llvm.org/D30158 Files:

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

2017-02-19 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:78 + + Finds and fixes usage of random_shuffle as the function has been removed from C++17. + Please add std:: and enclose random_shuffle in ``. Comment at:

[PATCH] D27827: [ObjC] CodeGen support for @available on macOS

2017-02-20 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added inline comments. Comment at: lib/CodeGen/CodeGenFunction.h:2479 + llvm::Value *EmitObjCIsOSVersionAtLeast(ArrayRef Args); + I think it's better to treat this as a builtin in its own right, without including the ObjC part in the names. This

[PATCH] D30166: Honor __unaligned in codegen for declarations and expressions

2017-02-20 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments. Comment at: include/clang/AST/ASTContext.h:1910 +if (T.getQualifiers().hasUnaligned()) + TI.Align = 8; +return TI; Is it better to call TargetInfo::getCharWidth() instead of assigning a hardcoded number here?

[PATCH] D30174: [Sema][ObjC] Warn about 'performSelector' calls with selectors that return record types

2017-02-20 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. Do we still issue a warning even when the struct can be returned in a register? For example, x86 can return a small struct (for example, a struct with one int field) in a single register, in which case it's fine to pass it to performSelector via @selector. If we

[PATCH] D30155: [clang-tools-extra] [test] Fix clang library dir in LD_LIBRARY_PATH For stand-alone build

2017-02-19 Thread Michał Górny via Phabricator via cfe-commits
mgorny created this revision. mgorny added a project: clang-tools-extra. Prepend the clang library directory (determined using SHLIBDIR, alike in clang) to the LD_LIBRARY_PATH to ensure that just-built clang libraries will be used instead of a previous installed version. When a stand-alone build

[PATCH] D29967: Get class property selectors from property decl if it exists

2017-02-20 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd closed this revision. compnerd added a comment. SVN r295683 https://reviews.llvm.org/D29967 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29819: Introduce an 'external_source_symbol' attribute that describes the origin and the nature of a declaration

2017-02-20 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment. Ping. Repository: rL LLVM https://reviews.llvm.org/D29819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30174: [Sema][ObjC] Warn about 'performSelector' calls with selectors that return record types

2017-02-20 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment. In https://reviews.llvm.org/D30174#681801, @ahatanak wrote: > Do we still issue a warning even when the struct can be returned in a > register? For example, x86 can return a small struct (for example, a struct > with one int field) in a single register, in which case

[PATCH] D30131: [profiling] PR31992: Don't skip interesting non-base constructors

2017-02-20 Thread Vedant Kumar via Phabricator via cfe-commits
vsk updated this revision to Diff 89151. vsk retitled this revision from "[profiling] Don't skip non-base constructors if there is a virtual base (fixes PR31992)" to "[profiling] PR31992: Don't skip interesting non-base constructors". vsk edited the summary of this revision. vsk added a comment.

[PATCH] D30183: Add -iframeworkwithsysroot compiler option

2017-02-20 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman created this revision. This patch adds support for a new `-iframeworkwithsysroot` compiler option which allows the user to specify a framework path that can be prefixed with the sysroot. This option is similar to the `-iwithsysroot` option that exists to supplement `-isystem`.

[PATCH] D30174: [Sema][ObjC] Warn about 'performSelector' calls with selectors that return record types

2017-02-20 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. In https://reviews.llvm.org/D30174#681843, @arphaman wrote: > Yes, we do. Primarily for the following reason: even if some target may > return a struct in a register, another target isn't guaranteed to do the same > thing. It's better to always warn about this rather

[PATCH] D30111: [clang-format] Add a test to check at once all the Mozilla coding style

2017-02-17 Thread Krasimir Georgiev via Phabricator via cfe-commits
krasimir added inline comments. Comment at: test/Format/check-coding-style-mozilla.cpp:10 + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +if (true) { What is tested here? Brace styles? Comment at:

[PATCH] D26654: [CMake] Add Fuchsia toolchain CMake cache files

2017-02-17 Thread Petr Hosek via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL295480: [CMake] Add Fuchsia toolchain CMake cache files (authored by phosek). Changed prior to commit: https://reviews.llvm.org/D26654?vs=88770=88958#toc Repository: rL LLVM

<    9   10   11   12   13   14   15   16   17   18   >