[PATCH] D54319: clang-cl: Add documentation for /Zc:dllexportInlines-

takuto.ikuta accepted this revision. takuto.ikuta added a comment. This revision is now accepted and ready to land. Seems good document! https://reviews.llvm.org/D54319 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

stephanemoore updated this revision to Diff 173544. stephanemoore marked 9 inline comments as done. stephanemoore added a comment. Updated with the following changes: • Elided conditional braces to comply with LLVM style. • Converted conditional loop break to loop condition in generateFixItHint.

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

hwright updated this revision to Diff 173543. hwright marked 16 inline comments as done. hwright added a comment. Addressed reviewer feedback. https://reviews.llvm.org/D54246 Files: clang-tidy/abseil/AbseilTidyModule.cpp clang-tidy/abseil/CMakeLists.txt

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

hwright added inline comments. Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:74 + case DurationScale::Minutes: +if (Multiplier == 60.0) + return DurationScale::Hours; hokein wrote: > we are comparing with floats, I think we should use

[PATCH] D53263: Fix places where the return type of a FunctionDecl was being used in place of the function type

JDevlieghere added a comment. In https://reviews.llvm.org/D53263#1294497, @kristina wrote: > In https://reviews.llvm.org/D53263#1294488, @JDevlieghere wrote: > > > The patch applies for me but has quite a few style violations. I'll fix > > those up before landing it. > > > Thank you and sorry

[PATCH] D53263: Fix places where the return type of a FunctionDecl was being used in place of the function type

This revision was automatically updated to reflect the committed changes. Closed by commit rL346601: Pass the function type instead of the return type to FunctionDecl::Create (authored by JDevlieghere, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D53263: Fix places where the return type of a FunctionDecl was being used in place of the function type

kristina added a comment. In https://reviews.llvm.org/D53263#1294488, @JDevlieghere wrote: > The patch applies for me but has quite a few style violations. I'll fix those > up before landing it. Thank you and sorry for the trouble, my fork seems to have enough modifications to some of these

r346601 - Pass the function type instead of the return type to FunctionDecl::Create

Author: jdevlieghere Date: Sat Nov 10 16:56:15 2018 New Revision: 346601 URL: http://llvm.org/viewvc/llvm-project?rev=346601=rev Log: Pass the function type instead of the return type to FunctionDecl::Create Fix places where the return type of a FunctionDecl was being used in place of the

[PATCH] D53263: Fix places where the return type of a FunctionDecl was being used in place of the function type

JDevlieghere added a comment. The patch applies for me but has quite a few style violations. I'll fix those up before landing it. https://reviews.llvm.org/D53263 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D53263: Fix places where the return type of a FunctionDecl was being used in place of the function type

kristina added a comment. In https://reviews.llvm.org/D53263#1294412, @bobsayshilol wrote: > In https://reviews.llvm.org/D53263#1294383, @kristina wrote: > > > I can do it if you'd like, will be a moment though. > > > Thanks and much appreciated! Huge apologies, it seems I can't get this to

[PATCH] D54379: Add Hurd support

sthibaul added inline comments. Comment at: lib/Driver/ToolChains/Hurd.cpp:136 + + // Add an include of '/include' directly. This isn't provided by default by + // system GCCs, but is often used with cross-compiling GCCs, and harmless to krytarowski wrote: >

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

kristina updated this revision to Diff 173534. kristina added a comment. Revised to address nitpicks. Repository: rC Clang https://reviews.llvm.org/D54344 Files: lib/CodeGen/CGDeclCXX.cpp test/CodeGenCXX/attr-no-destroy-d54344.cpp Index: test/CodeGenCXX/attr-no-destroy-d54344.cpp

[PATCH] D53263: Fix places where the return type of a FunctionDecl was being used in place of the function type

bobsayshilol added a comment. In https://reviews.llvm.org/D53263#1294383, @kristina wrote: > I can do it if you'd like, will be a moment though. Thanks and much appreciated! https://reviews.llvm.org/D53263 ___ cfe-commits mailing list

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

JonasToth added a comment. Thank you for checking these two projects! In https://reviews.llvm.org/D53974#1294375, @ztamas wrote: > I have the result after running the current version of the check on > LibreOffice. > > Found around 50 new issues were hidden by macros: > >

[PATCH] D54379: Add Hurd support

krytarowski added inline comments. Comment at: lib/Driver/ToolChains/Hurd.cpp:136 + + // Add an include of '/include' directly. This isn't provided by default by + // system GCCs, but is often used with cross-compiling GCCs, and harmless to Is this some hurd

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some small commenting nits. However, I leave it to @erik.pilkington to make the final sign-off. Comment at: lib/CodeGen/CGDeclCXX.cpp:68 + //

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

ztamas added a comment. I also tested on LLVm code. The output is here: https://gist.github.com/tzolnai/fe4f23031d3f9fdbdbf1ee38abda00a4 I found 362 warnings. Around 95% of these warnings are similar to the next example: /home/zolnai/lohome/llvm/lib/Support/Chrono.cpp:64:24: warning: loop

[PATCH] D53263: Fix places where the return type of a FunctionDecl was being used in place of the function type

kristina added a comment. In https://reviews.llvm.org/D53263#1294336, @bobsayshilol wrote: > Thanks! > I don't have commit access to land this myself. I can do it if you'd like, will be a moment though. https://reviews.llvm.org/D53263 ___

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

kristina updated this revision to Diff 173530. kristina set the repository for this revision to rC Clang. kristina added a comment. @erik.pilkington Fantastic catch, I totally forgot about the global flag. Revised, moved to CodeGenCXX, added a test to verify ctor/dtor is balanced under normal

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

ztamas added a comment. I have the result after running the current version if the check. Found around 50 new issues were hidden by macros: https://cgit.freedesktop.org/libreoffice/core/commit/?id=afbfe42e63cdba1a18c292d7eb4875009b0f19c0 Also found some new false positives related to macros.

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

aaron.ballman added a comment. In https://reviews.llvm.org/D54349#1294325, @vmiklos wrote: > > What do you think about code like: > > > > #if FOO == 4 > > #if FOO == 4 > > #endif > > #endif > > > > #if defined(FOO) > > #if defined(FOO) > > #endif > > #endif > > > > #if

[PATCH] D53263: Fix places where the return type of a FunctionDecl was being used in place of the function type

bobsayshilol added a comment. Thanks! I don't have commit access to land this myself. https://reviews.llvm.org/D53263 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

Szelethus added inline comments. Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:19-22 +struct Entry { + SourceLocation Loc; + std::string MacroName; +}; vmiklos wrote: > Szelethus wrote: > > This is a way too general name in my opinion.

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

vmiklos updated this revision to Diff 173526. vmiklos marked an inline comment as done. https://reviews.llvm.org/D54349 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ReadabilityTidyModule.cpp clang-tidy/readability/RedundantPreprocessorCheck.cpp

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

vmiklos marked an inline comment as done. vmiklos added inline comments. Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:19-22 +struct Entry { + SourceLocation Loc; + std::string MacroName; +}; Szelethus wrote: > This is a way too general

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

vmiklos added a comment. > What do you think about code like: > > #if FOO == 4 > #if FOO == 4 > #endif > #endif > > #if defined(FOO) > #if defined(FOO) > #endif > #endif > > #if !defined(FOO) > #if !defined(FOO) > #endif > #endif I looked at supporting these, but

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

vmiklos updated this revision to Diff 173524. https://reviews.llvm.org/D54349 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ReadabilityTidyModule.cpp clang-tidy/readability/RedundantPreprocessorCheck.cpp clang-tidy/readability/RedundantPreprocessorCheck.h

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

erik.pilkington added a comment. I have a few nits, but I think this looks about right. I reduced this testcase with creduce, can you use the minimized version? Also, I think it makes more sense to put the test into `test/CodeGenCXX` and verify the `-emit-llvm-only` output is correct with

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

Szelethus added inline comments. Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:19-22 +struct Entry { + SourceLocation Loc; + std::string MacroName; +}; This is a way too general name in my opinion. Either include comments that describe it,

r346591 - [cxx_status] Update for San Diego motions.

Author: rsmith Date: Sat Nov 10 10:02:40 2018 New Revision: 346591 URL: http://llvm.org/viewvc/llvm-project?rev=346591=rev Log: [cxx_status] Update for San Diego motions. Modified: cfe/trunk/www/cxx_status.html Modified: cfe/trunk/www/cxx_status.html URL:

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

JonasToth added a comment. > Done. Please also mark the inline comments as done. Otherwise its hard to follow if there are outstanding issues somewhere, especially if code moves around. https://reviews.llvm.org/D54349 ___ cfe-commits mailing list

[PATCH] D52984: [analyzer] Checker reviewer's checklist

Szelethus added a comment. Mainly my problem is that checker files can be large and hard to maneuver, some forward declare and document everything, some are a big bowl of spaghetti, and I think having a checklist of how it should be organized to keep uniformity and readability would be great.

[PATCH] D52984: [analyzer] Checker reviewer's checklist

Szelethus added a comment. In https://reviews.llvm.org/D52984#1294258, @NoQ wrote: > In https://reviews.llvm.org/D52984#1294233, @aaron.ballman wrote: > > > Personally, I think it's detrimental to the community for subprojects to > > come up with their own coding guidelines. My preference is

[PATCH] D52984: [analyzer] Checker reviewer's checklist

NoQ added a comment. In https://reviews.llvm.org/D52984#1294233, @aaron.ballman wrote: > Personally, I think it's detrimental to the community for subprojects to come > up with their own coding guidelines. My preference is for the static analyzer > to come more in line with the rest of the

[PATCH] D53771: [clang-tidy] Avoid C arrays check

JonasToth added a comment. In https://reviews.llvm.org/D53771#1294244, @lebedev.ri wrote: > *Maybe* we should be actually diagnosing the each actual declaration, not > just the `typeloc`. > Else if you use one `typedef`, and `// NOLINT` it, you will silence it for > all the decls that use it.

[PATCH] D54379: Add Hurd support

sthibaul updated this revision to Diff 173515. https://reviews.llvm.org/D54379 Files: lib/Basic/Targets.cpp lib/Basic/Targets/OSTargets.h lib/Driver/CMakeLists.txt lib/Driver/Driver.cpp lib/Driver/ToolChains/Clang.cpp lib/Driver/ToolChains/Gnu.cpp lib/Driver/ToolChains/Hurd.cpp

[PATCH] D54379: Add Hurd support

sthibaul marked 3 inline comments as done. sthibaul added a comment. I commented one of them, and will fix the rest. Comment at: lib/Driver/ToolChains/Hurd.cpp:36 + // clever. + switch (TargetTriple.getArch()) { + default: kristina wrote: > Does this need a

[PATCH] D53771: [clang-tidy] Avoid C arrays check

lebedev.ri added a comment. *Maybe* we should be actually diagnosing the each actual declaration, not just the `typeloc`. Else if you use one `typedef`, and `// NOLINT` it, you will silence it for all the decls that use it. In https://reviews.llvm.org/D53771#1294241, @JonasToth wrote: > LGTM.

[PATCH] D53771: [clang-tidy] Avoid C arrays check

JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. LGTM. Please give other reviewers a chance to take a look at it too. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53771

[PATCH] D53771: [clang-tidy] Avoid C arrays check

lebedev.ri added inline comments. Comment at: test/clang-tidy/modernize-avoid-c-arrays.cpp:32 + +using k = int[4]; +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not declare C-style arrays, use std::array<> instead JonasToth wrote: > What happens for a

[PATCH] D54379: Add Hurd support

kristina added reviewers: kristina, clang. kristina added a comment. A few style naming/comments. Comment at: lib/Driver/ToolChains/Hurd.cpp:36 + // clever. + switch (TargetTriple.getArch()) { + default: Does this need a switch? Wouldn't an `if` be

[PATCH] D52984: [analyzer] Checker reviewer's checklist

aaron.ballman added a comment. In https://reviews.llvm.org/D52984#1294223, @xazax.hun wrote: > - Move the checklist up before additional info in the HTML file. > - Fix minor nits. > - Add missing bullet points (thanks @Szelethus for noticing) > > I did not add any coding convention related

[PATCH] D53771: [clang-tidy] Avoid C arrays check

JonasToth added a comment. from my side mostly ok. I think the typedef points should be clarified, but I dont see more issues. Comment at: test/clang-tidy/modernize-avoid-c-arrays.cpp:32 + +using k = int[4]; +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not declare C-style

[PATCH] D53771: [clang-tidy] Avoid C arrays check

lebedev.ri added inline comments. Comment at: clang-tidy/modernize/AvoidCArraysCheck.cpp:64 + diag(ArrayType->getBeginLoc(), + isa(ArrayType->getTypePtr()) ? UseVector : UseArray); +} JonasToth wrote: > Why `isa<>` and not `isVariableArrayType()` (does it

[PATCH] D53771: [clang-tidy] Avoid C arrays check

lebedev.ri updated this revision to Diff 173514. lebedev.ri marked 2 inline comments as done. lebedev.ri added a comment. Use `isVariableArrayType()` Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53771 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt

[PATCH] D52984: [analyzer] Checker reviewer's checklist

xazax.hun added inline comments. Comment at: www/analyzer/checker_dev_manual.html:719 + +User facing documentation is important for adoption! Make sure the check list updated +at the homepage of the analyzer. Also ensure that the description is good quality in

[PATCH] D52984: [analyzer] Checker reviewer's checklist

xazax.hun updated this revision to Diff 173513. xazax.hun marked 11 inline comments as done. xazax.hun added a comment. - Move the checklist up before additional info in the HTML file. - Fix minor nits. - Add missing bullet points (thanks @Szelethus for noticing) I did not add any coding

[PATCH] D52984: [analyzer] Checker reviewer's checklist

Szelethus added inline comments. Comment at: www/analyzer/checker_dev_manual.html:719 + +User facing documentation is important for adoption! Make sure the check list updated +at the homepage of the analyzer. Also ensure that the description is good quality in

[PATCH] D52984: [analyzer] Checker reviewer's checklist

xazax.hun added inline comments. Comment at: www/analyzer/checker_dev_manual.html:719 + +User facing documentation is important for adoption! Make sure the check list updated +at the homepage of the analyzer. Also ensure that the description is good quality in

[PATCH] D54379: Add Hurd support

kristina added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:533 - if (Triple.isOSLinux() || Triple.getOS() == llvm::Triple::CloudABI) { + if (Triple.isOSLinux() || Triple.getOS() == llvm::Triple::CloudABI || Triple.isOSHurd()) { switch

[PATCH] D54379: Add Hurd support

sthibaul added a comment. The Hurd::Hurd constructor would actually need to do the same gcc inclusion path detection as on Linux, but let's leave this aside for now, this commit is enough for a build without libc++. Repository: rC Clang https://reviews.llvm.org/D54379

[PATCH] D54379: Add Hurd support

sthibaul created this revision. sthibaul added a reviewer: rengolin. Herald added subscribers: cfe-commits, fedor.sergeev, krytarowski, mgorny. Repository: rC Clang https://reviews.llvm.org/D54379 Files: lib/Basic/Targets.cpp lib/Basic/Targets/OSTargets.h lib/Driver/CMakeLists.txt

[PATCH] D53771: [clang-tidy] Avoid C arrays check

JonasToth added inline comments. Comment at: clang-tidy/modernize/AvoidCArraysCheck.cpp:26 + const clang::Type *TypeNode = Node.getTypePtr(); + return (TypeNode != nullptr && + InnerMatcher.matches(*TypeNode, Finder, Builder)); Nit: these parens are

[PATCH] D53771: [clang-tidy] Avoid C arrays check

lebedev.ri added a comment. Comments? :) https://reviews.llvm.org/D53771 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54120: [python] Support PathLike filenames and directories

mgorny added a comment. Merged. I will get back to you if something explodes ;-). Repository: rL LLVM https://reviews.llvm.org/D54120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

r346586 - [python] Support PathLike filenames and directories

Author: mgorny Date: Sat Nov 10 03:41:36 2018 New Revision: 346586 URL: http://llvm.org/viewvc/llvm-project?rev=346586=rev Log: [python] Support PathLike filenames and directories Python 3.6 introduced a file system path protocol (PEP 519[1]). The standard library APIs accepting file system

[PATCH] D54120: [python] Support PathLike filenames and directories

This revision was automatically updated to reflect the committed changes. Closed by commit rL346586: [python] Support PathLike filenames and directories (authored by mgorny, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

kristina updated this revision to Diff 173507. kristina added a comment. Revised, added a newline to the testcase. https://reviews.llvm.org/D54344 Files: lib/CodeGen/CGDeclCXX.cpp test/SemaCXX/attr-no-destroy-d54344.cpp Index: test/SemaCXX/attr-no-destroy-d54344.cpp

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

JonasToth added a comment. > I'll run it on LibreOffice code again and we'll see. Perfect, if you have the time, running it against LLVM would be very interesting as well. >> Do you have commit access? > > Commit access? This is my first patch. If you plan to regularly contribute to LLVM you

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

lebedev.ri added inline comments. Comment at: test/SemaCXX/attr-no-destroy-d54344.cpp:93 +static some_class s_ctor(1); \ No newline at end of file Please add new line. Repository: rC Clang https://reviews.llvm.org/D54344

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

kristina updated this revision to Diff 173503. kristina set the repository for this revision to rC Clang. kristina added a comment. Revised, I think I worked out what was happening, there's an explanation in the comment in the test. This is a relatively new attribute so the coverage for it did

[PATCH] D54120: [python] Support PathLike filenames and directories

jstasiak updated this revision to Diff 173505. jstasiak added a comment. That's fair, changed string to just x, should be obvious from the context what x is. Thank you for the review. As I don't have commit access I'd like to ask you to commit this on my behalf.

[PATCH] D54120: [python] Support PathLike filenames and directories

mgorny accepted this revision. mgorny added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: bindings/python/clang/cindex.py:133 +except AttributeError: +def fspath(string): +return string Optionally: this is

[PATCH] D32595: CMakeLists: Don't set LLVM_MAIN_SRC_DIR when building stand-alone clang

mgorny requested changes to this revision. mgorny added a comment. This revision now requires changes to proceed. Could you please rebase? I'm pretty sure this breaks our use but I can't test since it no longer applies cleanly. https://reviews.llvm.org/D32595

[PATCH] D32578: CMake: Set LLVM_MAIN_INCLUDE_DIR to LLVM_INCLUDE_DIR

mgorny accepted this revision. mgorny added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D32578 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

Szelethus added a comment. Ping, @NoQ, do you insist on a global set? https://reviews.llvm.org/D51531 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

kristina added a comment. I've managed to isolate enough to make for a testcase. Is that too broad or is it okay to attach as is? The following triggers the assert: namespace util { template class test_vector { public: test_vector(unsigned c)

[PATCH] D32577: CMake: Replace open-coded find_package

mgorny accepted this revision. mgorny added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D32577 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D54120: [python] Support PathLike filenames and directories

jstasiak marked 2 inline comments as done. jstasiak added a comment. Thanks for the feedback, you're right those were things worth improving, I updated the code. https://reviews.llvm.org/D54120 ___ cfe-commits mailing list

[PATCH] D54120: [python] Support PathLike filenames and directories

jstasiak updated this revision to Diff 173496. jstasiak added a comment. Tests are skipped using unittest skipping mechanism now and pathlib imported only when necessary. https://reviews.llvm.org/D54120 Files: bindings/python/clang/cindex.py bindings/python/tests/cindex/test_cdb.py

[PATCH] D54120: [python] Support PathLike filenames and directories

mgorny requested changes to this revision. mgorny added a comment. This revision now requires changes to proceed. Also please remember to submit patches with `-U`, so that Phab has full context. Comment at: bindings/python/tests/cindex/test_cdb.py:42 +if HAS_FSPATH:

r346583 - [clang]: Fix misapplied patch in 346582.

Author: kristina Date: Sat Nov 10 00:04:38 2018 New Revision: 346583 URL: http://llvm.org/viewvc/llvm-project?rev=346583=rev Log: [clang]: Fix misapplied patch in 346582. Modified: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp Modified: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp URL: