r347111 - Add missing test for r347072 -gcodeview-ghash

2018-11-16 Thread Reid Kleckner via cfe-commits
Author: rnk Date: Fri Nov 16 15:17:11 2018 New Revision: 347111 URL: http://llvm.org/viewvc/llvm-project?rev=347111&view=rev Log: Add missing test for r347072 -gcodeview-ghash Added: cfe/trunk/test/Driver/gcodeview-ghash.c Added: cfe/trunk/test/Driver/gcodeview-ghash.c URL: http://llvm.org/

[PATCH] D53891: [LTO] Pass down LTOUnit codegen flag to bitcode writer

2018-11-16 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson added a comment. In https://reviews.llvm.org/D53891#1300789, @tejohnson wrote: > - Switch the default of splitting lto units to off by default, unless > compiled with CFI or -fwhole-program-vtables. Thinking ahead to when I add the index based WPD implementation, we are going to wan

[PATCH] D53787: [Sema] Provide -fvisibility-global-new-delete-hidden option

2018-11-16 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment. Ping? Repository: rC Clang https://reviews.llvm.org/D53787 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51575: [clang-tidy/checks] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

2018-11-16 Thread Stephane Moore via Phabricator via cfe-commits
stephanemoore added inline comments. Comment at: clang-tidy/google/FunctionNamingCheck.cpp:63 + bool AtWordBoundary = true; + while (Index >= NewName.size()) { +char ch = NewName[Index]; I forgot to invert this condition when I refactored the early return i

[PATCH] D51575: [clang-tidy/checks] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

2018-11-16 Thread Stephane Moore via Phabricator via cfe-commits
stephanemoore updated this revision to Diff 174465. stephanemoore added a comment. Fixed loop condition in generateFixItHint. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51575 Files: clang-tidy/google/CMakeLists.txt clang-tidy/google/FunctionNamingCheck.cpp clang-tidy/

[PATCH] D54425: [AArch64] Add aarch64_vector_pcs function attribute to Clang

2018-11-16 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:1101 case CC_AAPCS: + case CC_AArch64VectorCall: return llvm::dwarf::DW_CC_LLVM_AAPCS; sdesmalen wrote: > I wasn't really sure whether this requires a corresponding DW_CC_LLVM_AAVPCS > r

[PATCH] D54604: Automatic variable initialization

2018-11-16 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. Would it be possible to extend test/Sema/uninit-variables.c to verify that the new option doesn't break -Wuninitialized? Repository: rC Clang https://reviews.llvm.org/D54604 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D54657: [clang] Add -MJJ for appending to compilation databases.

2018-11-16 Thread Tom Roeder via Phabricator via cfe-commits
tmroeder created this revision. tmroeder added a reviewer: klimek. Herald added a subscriber: cfe-commits. Some build systems compile files more than once (e.g., Kbuild in the Linux kernel compiles with -c once and with -E once). In these cases, the -MJ flag will only record the latest compilation

[PATCH] D54604: Automatic variable initialization

2018-11-16 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. In https://reviews.llvm.org/D54604#1301807, @kcc wrote: > Would it be possible to extend test/Sema/uninit-variables.c to verify that > the new option > doesn't break -Wuninitialized? Done. Repository: rC Clang https://reviews.llvm.org/D54604 __

[PATCH] D54604: Automatic variable initialization

2018-11-16 Thread JF Bastien via Phabricator via cfe-commits
jfb updated this revision to Diff 174471. jfb added a comment. - Make sure uninit-variables.c doesn't break. Repository: rC Clang https://reviews.llvm.org/D54604 Files: include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.td include/clang/Basic/LangOptions.def include/clang/Basic

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

2018-11-16 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. > In https://reviews.llvm.org/D54557#1300653, @NoQ wrote: > >> In https://reviews.llvm.org/D54557#1299899, @Szelethus wrote: >> >> > I think we should either remove the non-default functionality (which >> > wouldn't be ideal), or emphasise somewhere (open projects?) th

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

2018-11-16 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 174475. NoQ added a comment. Give the rvalue reference test function a sensible name. https://reviews.llvm.org/D54557 Files: lib/StaticAnalyzer/Checkers/MoveChecker.cpp test/Analysis/use-after-move.cpp Index: test/Analysis/use-after-move.cpp ==

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

2018-11-16 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In https://reviews.llvm.org/D54557#1300671, @Szelethus wrote: > Nice catch, would you like to make an issue on their project or shall I? I guess it can be an outright pull request :) I'll see if i have time for that soon, please feel free to take initiative>< In https://r

[PATCH] D54560: [analyzer] MoveChecker Pt.3: Improve warning messages a bit.

2018-11-16 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 174477. NoQ added a comment. Write down full messages in tests. When the message was updated from `'x' is moved'` to `Object 'x' is moved`, the tests were not updated because they kept passing because the former is still a sub-string of the latter. https://rev

[PATCH] D54560: [analyzer] MoveChecker Pt.3: Improve warning messages a bit.

2018-11-16 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:385-386 + } + // Provide the caller with the classification of the object + // we've obtained here accidentally, for later use. + return OK; Szelethus wrote: > Maybe move this

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

2018-11-16 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. In https://reviews.llvm.org/D54557#1301869, @NoQ wrote: > In https://reviews.llvm.org/D54557#1300671, @Szelethus wrote: > > > Nice catch, would you like to make an issue on their project or shall I? > > > I guess it can be an outright pull request :) I'll see if i have

[PATCH] D54563: [analyzer] MoveChecker Pt.4: Add a few more state reset methods.

2018-11-16 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In https://reviews.llvm.org/D54563#1299918, @Szelethus wrote: > How about `std::list::splice`? Hmm, you mean that it leaves its argument empty? Interesting, i guess we may need a separate facility for that. I wonder how many other state-reset methods are we forgetting.

[PATCH] D52440: Emit lifetime markers for temporary function parameter aggregates

2018-11-16 Thread Ian Tessier via Phabricator via cfe-commits
itessier added a comment. Thanks for the detailed info! I will try to get something out when I get some free cycles. Repository: rC Clang https://reviews.llvm.org/D52440 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.o

[clang-tools-extra] r347119 - [clangd] Fix crash hovering on non-decltype trailing return

2018-11-16 Thread Marc-Andre Laperle via cfe-commits
Author: malaperle Date: Fri Nov 16 16:41:14 2018 New Revision: 347119 URL: http://llvm.org/viewvc/llvm-project?rev=347119&view=rev Log: [clangd] Fix crash hovering on non-decltype trailing return Summary: More specifically, hovering on "auto" in auto main() -> int { return 0; } Signed-off-by

[PATCH] D54553: [clangd] Fix crash hovering on non-decltype trailing return

2018-11-16 Thread Marc-Andre Laperle via Phabricator via cfe-commits
malaperle added a comment. Thanks for the review! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54553 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54553: [clangd] Fix crash hovering on non-decltype trailing return

2018-11-16 Thread Marc-Andre Laperle via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL347119: [clangd] Fix crash hovering on non-decltype trailing return (authored by malaperle, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D54

[PATCH] D54425: [AArch64] Add aarch64_vector_pcs function attribute to Clang

2018-11-16 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:1101 case CC_AAPCS: + case CC_AArch64VectorCall: return llvm::dwarf::DW_CC_LLVM_AAPCS; rnk wrote: > sdesmalen wrote: > > I wasn't really sure whether this requires a corresponding >

[PATCH] D54246: [clang-tidy] Add the abseil-duration-factory-scale check

2018-11-16 Thread Hyrum Wright via Phabricator via cfe-commits
hwright added a comment. @aaron.ballman I don't actually have the commit bit, can you commit this, or are we waiting for further review? https://reviews.llvm.org/D54246 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c

[PATCH] D51575: [clang-tidy/checks] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

2018-11-16 Thread Stephane Moore via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL347132: [clang-tidy/checks] Implement a clang-tidy check to verify Google Objective-C… (authored by stephanemoore, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https

[PATCH] D54379: Add Hurd toolchain support to Clang

2018-11-16 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/Driver/ToolChains/Hurd.cpp:84 + const Driver &D = getDriver(); + std::string SysRoot = computeSysRoot(); + kristina wrote: > Move semantics? Or is this guaranteed elision (I'm not sure)? If you're talking about th

[PATCH] D54379: Add Hurd toolchain support to Clang

2018-11-16 Thread Kristina Brooks via Phabricator via cfe-commits
kristina added a comment. As discussed in `#hurd`, a few additional comments. The Hurd codepath should first check if the triple is eligible, ie. minimizing any cost to the driver invocation for most targets, which are not going to be Hurd. In fact I would wrap it in `LLVM_UNLIKELY` but that's

[PATCH] D54654: Correct CreateAlignmentAssumption to check sign and power-of-2 (Clang Part)

2018-11-16 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. > As discussed on IRC, check sign/power of 2 correctly for the alignment > assumptions. "As discussed on IRC" is not appropriate for a commit message. The rationale must be documented here. > I hope that extra is-power-of-two won't confuse the optimizer. Probably woul

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

2018-11-16 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D53157#1291977, @andrew.w.kaylor wrote: > Craig Topper also raised some concerns with me (in person, his desk is right > by mine) about the potential effect this might have on code size. He tells me > that IRBuilder calls are intended to alwa

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

2018-11-16 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. > Just because FENV_ACCESS can be toggled on that granularity doesn't mean we > have to represent it that way. We've previously agreed (I think) that if > FENV_ACCESS is enabled anywhere in a function we will want to use the > constrained intrinsics for all FP operation

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

2018-11-16 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D53157#1301991, @hfinkel wrote: > In https://reviews.llvm.org/D53157#1291977, @andrew.w.kaylor wrote: > > > Craig Topper also raised some concerns with me (in person, his desk is > > right by mine) about the potential effect this might have on

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

2018-11-16 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D53157#1301782, @cameron.mcinally wrote: > Fair enough. Just for conversation's sake, I was envisioning the FE support a > little differently... > > 1. Add a command line option, say `-ffpe_safe=[true|false]`, that toggles > FPEnv-safe code g

[PATCH] D53280: [analyzer] Emit an error for invalid -analyzer-config inputs

2018-11-16 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus updated this revision to Diff 174495. Szelethus retitled this revision from "[analyzer] Emit a warning for unknown -analyzer-config options" to "[analyzer] Emit an error for invalid -analyzer-config inputs". Szelethus set the repository for this revision to rC Clang. Szelethus added a c

[PATCH] D51531: [analyzer][UninitializedObjectChecker] Uninit regions are only reported once

2018-11-16 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Herald added subscribers: gamesh411, baloghadamsoftware. In https://reviews.llvm.org/D51531#1296256, @NoQ wrote: > In https://reviews.llvm.org/D51531#1286110, @Szelethus wrote: > > > Oh, and the reason why I think it would add a lot of complication: since > > only `Exp

r347133 - Fix unused variable warning.

2018-11-16 Thread David L. Jones via cfe-commits
Author: dlj Date: Fri Nov 16 20:48:54 2018 New Revision: 347133 URL: http://llvm.org/viewvc/llvm-project?rev=347133&view=rev Log: Fix unused variable warning. Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp URL: http://llvm.o

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

2018-11-16 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 174498. Quuxplusone marked 14 inline comments as done. Quuxplusone edited the summary of this revision. Quuxplusone added a comment. Use `[[clang::trivially_relocatable]]` instead of `[[trivially_relocatable]]`. Canonicalize in `QualType::isTriviallyReloca

[PATCH] D54047: Check TUScope is valid before use

2018-11-16 Thread Kim Gräsman via Phabricator via cfe-commits
kimgr added subscribers: zturner, kimgr. kimgr added a comment. Here's some related suggestions/questions for context: - Earlier patch from @zturner with a minimal repro case: https://reviews.llvm.org/D31697. I don't think this is reproducible with Clang proper. - Open question for alternative

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

2018-11-16 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: include/clang/AST/DeclCXX.h:482 +/// and a defaulted destructor. +unsigned IsNaturallyTriviallyRelocatable : 1; + erichkeane wrote: > Quuxplusone wrote: > > erichkeane wrote: > > > Typically we'd have this ca

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

2018-11-16 Thread John McCall via Phabricator via cfe-commits
rjmccall added a reviewer: ahatanak. rjmccall added inline comments. Comment at: include/clang/AST/DeclCXX.h:482 +/// and a defaulted destructor. +unsigned IsNaturallyTriviallyRelocatable : 1; + Quuxplusone wrote: > erichkeane wrote: > > Quuxplusone wrote

[PATCH] D54560: [analyzer] MoveChecker Pt.3: Improve warning messages a bit.

2018-11-16 Thread Henry Wong via Phabricator via cfe-commits
MTC added a comment. > The "moved-from" terminology we adopt here still feels a bit weird to me, but > i don't have a better suggestion, so i just removed the single-quotes so that > to at least feel proud about what we have. I am personally fine with this terminology, this checker corresponds

<    1   2