[PATCH] D50055: Update the coding standard about NFC changes and whitespace

2018-08-04 Thread Chandler Carruth via Phabricator via cfe-commits
chandlerc added inline comments. Comment at: docs/CodingStandards.rst:514-516 +line of code. Some common editors will automatically remove trailing whitespace +when saving a file which causes unrelated changes to appear in diffs and +commits. Meinersbur wrote: >

[PATCH] D50055: Update the coding standard about NFC changes and whitespace

2018-08-04 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added inline comments. Comment at: docs/CodingStandards.rst:514-516 +line of code. Some common editors will automatically remove trailing whitespace +when saving a file which causes unrelated changes to appear in diffs and +commits. Just to note that

r338962 - Use Optional instead of unique_ptr; NFC

2018-08-04 Thread George Burgess IV via cfe-commits
Author: gbiv Date: Sat Aug 4 18:37:07 2018 New Revision: 338962 URL: http://llvm.org/viewvc/llvm-project?rev=338962=rev Log: Use Optional instead of unique_ptr; NFC Looks like the only reason we use a unique_ptr here is so that we can conditionally construct a LogicalErrorHandler. It's a small

[PATCH] D49355: Thread safety analysis: Allow lock upgrading and downgrading

2018-08-04 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment. In https://reviews.llvm.org/D49355#1188520, @aaronpuchert wrote: > Could you explain what annotations like `LOCK_UNLOCK` are useful for? What do > they check? The annotation should certainly not be necessary. > > Shouldn't you just use `REQUIRES(!...)` or

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

2018-08-04 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:1036 +return; + // That's it. We can't rule out any more cases with the data we have. + rsmith wrote: > rsmith wrote: > > lebedev.ri wrote: > > > rsmith wrote: > > > > I don't like

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

2018-08-04 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 159187. lebedev.ri marked 11 inline comments as done. lebedev.ri added a comment. Address most of @rsmith review notes. Repository: rC Clang https://reviews.llvm.org/D50250 Files: docs/ReleaseNotes.rst docs/UndefinedBehaviorSanitizer.rst

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

2018-08-04 Thread Marc-Andre Laperle via Phabricator via cfe-commits
malaperle added a comment. Both check-clang and check-clang-tools pass successfully for me on Windows with the patch. Repository: rC Clang https://reviews.llvm.org/D48903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D49800: [clang-tidy: modernize] modernize-redundant-void-arg crashes when a function body is in a macro

2018-08-04 Thread Idriss via Phabricator via cfe-commits
IdrissRio updated this revision to Diff 159179. IdrissRio added a comment. Thank you @alexfh. You where right. This update is more general. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49800 Files: clang-tidy/modernize/RedundantVoidArgCheck.cpp

[PATCH] D50261: [AST] Remove unnecessary indirections in DeclarationNameTable

2018-08-04 Thread Bruno Ricci via Phabricator via cfe-commits
bricci added a comment. I did both a clean build and an incremental build with and without this patch and was not able to see any meaningful difference in the build time. Suspiciously the build time *with* the patch in both cases is a little less (~0.1% ) than without the patch but this is well

[PATCH] D49355: Thread safety analysis: Allow lock upgrading and downgrading

2018-08-04 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Could you explain what annotations like `LOCK_UNLOCK` are useful for? What do they check? The annotation should certainly not be necessary. Shouldn't you just use `REQUIRES(!...)` or `EXCLUDES(...)`? If a function locks and unlocks a mutex, I don't see a reason to

[PATCH] D50294: [Driver] Use -gdwarf-3 by default for FreeBSD

2018-08-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: brad, emaste, khng300. Herald added subscribers: cfe-commits, JDevlieghere, aprantl. The imported binutils in base supports DWARF 3. Repository: rC Clang https://reviews.llvm.org/D50294 Files: lib/Driver/ToolChains/FreeBSD.h

[PATCH] D50055: Update the coding standard about NFC changes and whitespace

2018-08-04 Thread Chandler Carruth via Phabricator via cfe-commits
chandlerc accepted this revision. chandlerc added a comment. This looks really good to me and seems like a nice clarification of historical practice. =D Thanks so much for driving an actual documentation update for folks that simply would never know about these practices otherwise, I think it

[PATCH] D32747: [Analyzer] Iterator Checker - Part 3: Invalidation check, first for (copy) assignments

2018-08-04 Thread Balogh , Ádám via Phabricator via cfe-commits
baloghadamsoftware added a comment. https://reviews.llvm.org/D32747 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits