[PATCH] D73775: [clang-tidy] Cover cases like (b && c && b) in the redundant expression check

alexeyr added a comment. Ping. Are any other changes needed? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73775/new/ https://reviews.llvm.org/D73775 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D74433: [llvm-objdump] Print file format in lowercase to match GNU output.

jhenderson added a comment. I don't really have a strong opinion either way. By strict style rules, "ELF" is an acronym, so should be all upper-case. In regular text, I'd expect it to be ELF everywhere too. On the other hand, as pointed out, "elf" is the bfd naming style. For WASM, I recall

[clang] 97ed706 - Revert "[DebugInfo] Enable the debug entry values feature by default"

Author: Djordje Todorovic Date: 2020-02-12T11:59:04+01:00 New Revision: 97ed706a962af7c6835c7b6716207c4072011ac1 URL: https://github.com/llvm/llvm-project/commit/97ed706a962af7c6835c7b6716207c4072011ac1 DIFF:

[PATCH] D71830: [OpenMP][Part 2] Use reusable OpenMP context/traits handling

JonChesterfield added a comment. Procedural note - adding someone as a blocking reviewer to someone else's patch isn't great. What if the new reviewer never gets around to looking at the patch? I've adjusted that to non-blocking, but feel free to leave a comment if I've missed something.

[PATCH] D73359: [analyzer]StreamChecker refactoring (NFC).

This revision was automatically updated to reflect the committed changes. Closed by commit rG5b3983ba3716: [analyzer]StreamChecker refactoring (NFC). (authored by balazske). Herald added a subscriber: martong. Changed prior to commit: https://reviews.llvm.org/D73359?vs=243488=244125#toc

[PATCH] D74468: [clang-tidy] No misc-definitions-in-headers warning on C++14 variable templates.

hokein created this revision. hokein added a reviewer: gribozavr2. Herald added a subscriber: xazax.hun. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D74468 Files: clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

This revision was automatically updated to reflect the committed changes. Closed by commit rG9f6ff07f8a39: [DebugInfo] Enable the debug entry values feature by default (authored by djtodoro). Herald added subscribers: lldb-commits, cfe-commits, jrtc27. Herald added projects: clang, LLDB. Changed

[clang] 9f6ff07 - [DebugInfo] Enable the debug entry values feature by default

Author: Djordje Todorovic Date: 2020-02-12T10:25:14+01:00 New Revision: 9f6ff07f8a396dfc736c4cb6f9fba9a203531329 URL: https://github.com/llvm/llvm-project/commit/9f6ff07f8a396dfc736c4cb6f9fba9a203531329 DIFF:

[PATCH] D73629: [analyzer] vfork checker: allow execve after vfork

Szelethus added a comment. LGTM granted the test is supplied, nice catch! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73629/new/ https://reviews.llvm.org/D73629 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D74468: [clang-tidy] No misc-definitions-in-headers warning on C++14 variable templates.

gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-definitions-in-headers.rst:86 + // OK: C++14 variable template is allowed. + template

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

dstenb added inline comments. Comment at: llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp:870 if (MI->isCandidateForCallSiteEntry() && -DAG->getTarget().Options.EnableDebugEntryValues) +DAG->getTarget().Options.SupportsDebugEntryValues)

[PATCH] D74473: [analyzer] StdLibraryFunctionsChecker: Use platform dependent EOF and UCharMax

balazske added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:533 + +IntValue.dump(); +return IntValue.getSExtValue(); Debug message (to be removed)? Comment at:

[PATCH] D71830: [OpenMP][Part 2] Use reusable OpenMP context/traits handling

kiranchandramohan accepted this revision. kiranchandramohan added a comment. This revision is now accepted and ready to land. LGTM. You can wait for a day in case other reviewers have comments. Comment at: clang/lib/Serialization/ASTWriter.cpp:6575 +template <> void

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

djtodoro marked an inline comment as done. djtodoro added inline comments. Comment at: llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp:870 if (MI->isCandidateForCallSiteEntry() && -DAG->getTarget().Options.EnableDebugEntryValues) +

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

hans added inline comments. Comment at: clang/include/clang/Driver/Job.h:90 + /// Whether the command will be executed in this process or not. + bool InProcess : 1; + I think Reid just meant put the bool fields after each other to minimize padding. Using

[PATCH] D72644: [clang] Add -fignore-exceptions

jasonliu added a comment. In D72644#1871662 , @lebedev.ri wrote: > This review omitted cfe-commits list Didn't realized that. Since this is approved and committed, I guess I will keep monitor messages in this review, and address any post-commit

[PATCH] D73904: [clang] stop baremetal driver to append .a to lib

christof added a comment. The function @MaskRay suggested does not address the problem with `-march=` and others, but it does sound good to use that function. I'm also not sure if `-L` is a feature that is relied upon at the moment by anybody that uses the baremetal driver. For the moment I'll

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

djtodoro added a comment. Reverted due to http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/21373/steps/build-stage3-compiler/logs/stdio. I will reland this as soon as I fix the issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D74467: [analyzer] Teach scan-build how to rebuild index.html without analyzing.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. Thanks! I would mention in the Summary the necessary flags to perform index-only output. (May write some release notes?) That option sounds very strange, but I like it. For example to run

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

MyDeveloperDay added a comment. Correct follow that description on how to request commit access Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D44609/new/ https://reviews.llvm.org/D44609 ___ cfe-commits

[PATCH] D72644: [clang] Add -fignore-exceptions

jasonliu added a comment. In D72644#1868812 , @aaron.ballman wrote: > Sorry about the delayed review. I think this patch basically LG, thank you! Thanks Aaron. Addressed your comments in the commits. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D72644: [clang] Add -fignore-exceptions

lebedev.ri added a comment. This review omitted cfe-commits list Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72644/new/ https://reviews.llvm.org/D72644 ___ cfe-commits mailing list

[clang] 5b3983b - [analyzer]StreamChecker refactoring (NFC).

Author: Balázs Kéri Date: 2020-02-12T12:50:49+01:00 New Revision: 5b3983ba3716b155d0cb740d4f1940c1bb0a97d5 URL: https://github.com/llvm/llvm-project/commit/5b3983ba3716b155d0cb740d4f1940c1bb0a97d5 DIFF: https://github.com/llvm/llvm-project/commit/5b3983ba3716b155d0cb740d4f1940c1bb0a97d5.diff

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

hans added a comment. In D73742#1871037 , @smeenai wrote: > In D73742#1871012 , @aganea wrote: > > > In D73742#1870961 , @smeenai wrote: > > > > > I'm assuming this needs to

[PATCH] D74336: [ARM,MVE] Add the vmovlbq,vmovltq intrinsic family.

simon_tatham updated this revision to Diff 244104. simon_tatham added a comment. Cosmetic tweak: don't put the new `arm_mve.td` section in between the definition and the uses of an unrelated multiclass. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[clang] 55e2678 - [clang] Add -fignore-exceptions

Author: jasonliu Date: 2020-02-12T09:56:18Z New Revision: 55e2678fcd4d7eca3f9a602a919da499c1103041 URL: https://github.com/llvm/llvm-project/commit/55e2678fcd4d7eca3f9a602a919da499c1103041 DIFF: https://github.com/llvm/llvm-project/commit/55e2678fcd4d7eca3f9a602a919da499c1103041.diff LOG:

[PATCH] D74337: [ARM,MVE] Add the vmovnbq,vmovntq intrinsic family.

simon_tatham updated this revision to Diff 244105. simon_tatham added a comment. Cosmetic tweak: don't put the new `arm_mve.td` section in between the definition and the uses of an unrelated multiclass. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D72644: [clang] Add -fignore-exceptions

This revision was automatically updated to reflect the committed changes. Closed by commit rG55e2678fcd4d: [clang] Add -fignore-exceptions (authored by jasonliu). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D72644: [clang] Add -fignore-exceptions

aaron.ballman added a comment. In D72644#1871662 , @lebedev.ri wrote: > This review omitted cfe-commits list Good catch, I didn't notice that in the review. I think we can address any concerns with post-commit feedback. Repository: rG LLVM Github

[PATCH] D74335: [ARM,MVE] Add intrinsics vclzq and vclsq.

miyuki accepted this revision. miyuki 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/D74335/new/ https://reviews.llvm.org/D74335

[PATCH] D57054: [MachineOutliner][ARM][RFC] Add Machine Outliner support for ARM

yroux added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57054/new/ https://reviews.llvm.org/D57054 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D74467: [analyzer] Teach scan-build how to rebuild index.html without analyzing.

NoQ marked an inline comment as done. NoQ added inline comments. Comment at: clang/test/Analysis/scan-build/rebuild_index/rebuild_index.test:11-12 +RUN: cp %S/report-3.html %t.output_dir +RUN: mkdir %t.output_dir/subdirectory +RUN: cp %S/subdirectory/report-4.html

[PATCH] D73052: [clang-tidy] RenamerClangTidy now renames dependent member expr when the member can be resolved

njames93 updated this revision to Diff 244087. njames93 added a comment. - Made AggressiveDependentMemberLookup option Global Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73052/new/ https://reviews.llvm.org/D73052 Files:

[PATCH] D74467: [analyzer] Teach scan-build how to rebuild index.html without analyzing.

NoQ created this revision. NoQ added reviewers: dcoughlin, Charusso. Herald added subscribers: cfe-commits, martong, dkrupp, donat.nagy, Szelethus, arphaman, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: clang. NoQ marked an inline comment as done. NoQ

[PATCH] D70876: [clang-tidy] Add spuriously-wake-up-functions check

abelkocsis updated this revision to Diff 244098. abelkocsis added a comment. docs/list.rst update Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70876/new/ https://reviews.llvm.org/D70876 Files:

[PATCH] D74473: [analyzer] StdLibraryFunctionsChecker: Use platform dependent EOF and UCharMax

martong created this revision. martong added reviewers: Szelethus, NoQ. Herald added subscribers: cfe-commits, steakhal, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, kristof.beyls, xazax.hun, whisperity. Herald added a project: clang.

[PATCH] D71110: [clangd] A tool to evaluate cross-file rename.

kbobyrev added a comment. Renaming `llvm::Optional` also fails (clangd-rename says the symbol is not of a supported kind even though it's a class). Renaming `llvm::None` is also not working: there seem to be many problems but a lot of them are connected with function's argument default values

[PATCH] D71110: [clangd] A tool to evaluate cross-file rename.

hokein added a comment. Thanks for the feedback! Yeah, currently template classes are not supported in cross-file rename, see https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/clangd/refactor/Rename.cpp#L187. So `llvm::Optional` and `llvm::Optional` should fail to rename, but

[PATCH] D74116: [Sema][C++] Strawman patch to propagate conversion type in order to specialize the diagnostics

Anastasia updated this revision to Diff 244135. Anastasia added a comment. If I reuse the helper `checkPointerTypesForAssignment` I end up with lots of error turned into warnings, see example in test/SemaCXX/addr-of-overloaded-function.cpp I guess we don't want them to be warnings? Should we

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

cebowleratibm added a comment. Looks fine, but we need to settle on the name for the ABI. My preference would be "XLC++11", or perhaps "XLCXX11" (I propose the latter because of the common reference CXXABI.) Comment at: clang/include/clang/Basic/TargetCXXABI.h:116 +///

[PATCH] D74411: [clangd] Query constructors in the index during rename.

kbobyrev added inline comments. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:326 + // When querying references for a class, clangd's own index will also return + // references of the corresponding class constructors, but this is not true + // for all index

[PATCH] D73720: [Analyzer] Use note tags to track container begin and and changes

baloghadamsoftware added a comment. In D73720#1871955 , @Szelethus wrote: > Do we have a test where 2 containers are present but only one of them should > be marked as interesting? Yes, of course we have, that was the starting point of the discussion.

[PATCH] D74473: [analyzer] StdLibraryFunctionsChecker: Use platform dependent EOF and UCharMax

martong updated this revision to Diff 244140. martong added a comment. - Remove debug dump - Add TryExpandAsInteger to CheckerHelpers.h Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74473/new/ https://reviews.llvm.org/D74473 Files:

[PATCH] D74473: [analyzer] StdLibraryFunctionsChecker: Use platform dependent EOF and UCharMax

martong updated this revision to Diff 244141. martong added a comment. - Remove PP declaration Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74473/new/ https://reviews.llvm.org/D74473 Files:

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

xbolva00 added a comment. I think that this patch is needed since we need to deal with this situation somehow because fallthru comments are used in many real world code. But I am personally against extending it more and use comments to disable warnings generally. Repository: rG LLVM Github

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

thakis added a comment. I think giving comments a semantic meaning is a bad idea. Do we really have to do this? In addition to this making comments have semantic meaning, the meaning is different from gcc. I don't think we should support this. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

thakis added inline comments. Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:1239 + llvm::Regex("(/\\*[ \\t]*fall(s | |-)?thr(ough|u)\\.?[ \\t]*\\*/)" + "|(//[ \\t]*fall(s | |-)?thr(ough|u)\\.?[ \\t]*)", +

[PATCH] D74464: Fix integration of pass plugins with llvm dylib

serge-sans-paille updated this revision to Diff 244150. serge-sans-paille edited the summary of this revision. serge-sans-paille added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Totally different, and much cleaner approach of the issue. Repository: rG

[PATCH] D74473: [analyzer] StdLibraryFunctionsChecker: Use platform dependent EOF and UCharMax

Szelethus requested changes to this revision. Szelethus added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h:67-69 +/// Try to parse the value of a defined preprocessor macro. We

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

erichkeane added a comment. The performance benefit of disable-free is still valuable, I'd not want to make every TU pay the price for a multiple TU invocation. Additionally, if we're willing to put up with the 3rd form (which i think is worth it), the multiple TU case cleaning up on all but

[PATCH] D72872: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

hans added a comment. In D72872#1870256 , @vsapsai wrote: > In D72872#1868989 , @hans wrote: > > > I don't have the context here. Was I added as a subscriber because it's > > related to the clang 10 release? > > >

[clang] abd0905 - Revert "Revert "Change clang option -ffp-model=precise to select ffp-contract=on""

Author: Melanie Blower Date: 2020-02-12T07:30:43-08:00 New Revision: abd09053bc7aa6144873c196a7d50aa6ce6ca430 URL: https://github.com/llvm/llvm-project/commit/abd09053bc7aa6144873c196a7d50aa6ce6ca430 DIFF:

[clang] 665dcda - Add missing newlines at EOF; NFC

Author: Sven van Haastregt Date: 2020-02-12T15:57:25Z New Revision: 665dcdacc06b056c3279a1fccbcae4660d80f117 URL: https://github.com/llvm/llvm-project/commit/665dcdacc06b056c3279a1fccbcae4660d80f117 DIFF: https://github.com/llvm/llvm-project/commit/665dcdacc06b056c3279a1fccbcae4660d80f117.diff

Re: [clang] 0130b6c - Don't assume a reference refers to at least sizeof(T) bytes.

It's a wrong-code bugfix, but we've had the bug since Clang 3.5, so it doesn't seem urgent. Hans, what do you think? I don't think we've had any field reports of miscompiles (though someone did notice the ubsan false-positive). On Sat, 1 Feb 2020 at 05:04, Shoaib Meenai wrote: > Should this be

[PATCH] D71110: [clangd] A tool to evaluate cross-file rename.

kbobyrev added a comment. In D71110#1871896 , @hokein wrote: > Thanks for the feedback! > > Yeah, currently template classes are not supported in cross-file rename, see >

[PATCH] D73720: [Analyzer] Use note tags to track container begin and and changes

Szelethus added a comment. Do we have a test where 2 containers are present but only one of them should be marked as interesting? void deref_end_after_pop_back(std::vector , std::vector ) { const auto i = --V.end(); const auto i2 = --V2.end(); V.pop_back(); //

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

lebedev.ri added a comment. Also, rG398b wasn't sent to cfe-dev lists. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73852/new/ https://reviews.llvm.org/D73852 ___ cfe-commits mailing list

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

thakis added a comment. Isn't a better approach to only do in-process cc1 if there's just one TU? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74447/new/ https://reviews.llvm.org/D74447 ___ cfe-commits mailing list

[PATCH] D74473: [analyzer] StdLibraryFunctionsChecker: Use platform dependent EOF and UCharMax

martong added a comment. > Harbormaster failed remote builds in B46321 > : Diff 244141! This is actually true, I have a test that crashes! Finally, a case where remote builds are proved to be useful! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

aaron.ballman added a comment. In D73852#1872000 , @thakis wrote: > I think giving comments a semantic meaning is a bad idea. Do we really have > to do this? I think that ship has sailed, for instance, see // NOLINT comments that are also supported to

[clang] 5fef14d - [Concepts] Do not check constraints if not all template arguments have been deduced

Author: Saar Raz Date: 2020-02-12T16:02:12+02:00 New Revision: 5fef14d932fe602bf998b8fb8a809ff85ca1e245 URL: https://github.com/llvm/llvm-project/commit/5fef14d932fe602bf998b8fb8a809ff85ca1e245 DIFF: https://github.com/llvm/llvm-project/commit/5fef14d932fe602bf998b8fb8a809ff85ca1e245.diff

Re: [PATCH] D73904: [clang] stop baremetal driver to append .a to lib

> I'm also not sure if `-L` is a feature that is relied upon at the moment by anybody that uses the baremetal driver Unlikely, IMO. There aren’t that many users of this stuff yet, and it’s still quite rough around the edges in comparison to the GNU toolchain. Also, the compiler support libraries

[PATCH] D74384: Use std::foo_t rather than std::foo in LLVM.

thakis added a comment. Looks like this broke building on windows with clang-cl as host compiler in some situations: http://45.33.8.238/win/8160/step_4.txt I'm not sure if that's a bug in the version of the host clang I'm using yet. I'll poke around a bit and let you know. Repository: rG

[PATCH] D74433: [llvm-objdump] Print file format in lowercase to match GNU output.

sbc100 added a comment. Wasm is not an acronym so we don't usually display it in uppercase. So yes, I think we should change WasmObjectFile::getFileFormatName() to return lowercase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74433/new/

[PATCH] D74384: Use std::foo_t rather than std::foo in LLVM.

jlebar added a comment. In D74384#1872246 , @thakis wrote: > Looks like this broke building on windows with clang-cl as host compiler in > some situations: http://45.33.8.238/win/8160/step_4.txt I'm testing out the following fix, which we verified

[PATCH] D74473: [analyzer] StdLibraryFunctionsChecker: Use platform dependent EOF and UCharMax

martong updated this revision to Diff 244143. martong added a comment. - EOFMacroIt -> MacroIt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74473/new/ https://reviews.llvm.org/D74473 Files:

[PATCH] D71110: [clangd] A tool to evaluate cross-file rename.

hokein added a comment. > Also, the llvm::Twine fails too, many of the failures are related to the > forward declarations (I think the problem is that they are treated > differently from the original symbol and are not being renamed at all in the > corresponding files, but that's just my

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

Szelethus added a comment. One of the things that stood out for me was the lack of the usage of the `check::BranchCondition` callback, but there you'd have to grind out whether it is relevant to a return value, so I'm probably wrong on that regard. So I guess I don't have any immediate high

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

Szelethus added reviewers: xazax.hun, Charusso, dcoughlin. Szelethus added a comment. Also, allow me to add a few other folks, because they are very active and knowledgeable individuals :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72705/new/

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

aaron.ballman added a comment. In D73852#1872064 , @thakis wrote: > In D73852#1872044 , @aaron.ballman > wrote: > > > In D73852#1872000 , @thakis wrote: > > > > > I think

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

aganea added a comment. In D74447#1872043 , @thakis wrote: > Isn't a better approach to only do in-process cc1 if there's just one TU? One of the reasons of the in-process cc1 was the debuggability. If we can compile several TUs in-process, why not

[PATCH] D74483: [AArch64] Add Cortex-A34 Support for clang and llvm

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Looks like the usual business of adding a cpu to me, with one nit inlined that can be fixed before committing. Looks like you're doing the LLVM part separately. Now with the

[PATCH] D74473: [analyzer] StdLibraryFunctionsChecker: Use platform dependent EOF and UCharMax

martong updated this revision to Diff 244173. martong marked 6 inline comments as done. martong added a comment. - Move include of Preprocessor.h to CheckerHelpers.cpp - Try -> try - Use PP.getIdentifierInfo - Handle parens in tryExpandAsInteger Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D74473: [analyzer] StdLibraryFunctionsChecker: Use platform dependent EOF and UCharMax

martong marked 2 inline comments as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:571 +.Case( +{ReturnValueCondition(WithinRange, {{EOFv, EOFv}, {0, UCharMax}})}); };

[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

mibintc marked 2 inline comments as done. mibintc added a comment. some replies to Andy. I'll upload another patch here which passed check-all locally. then i'll re-commit it. Comment at: clang/docs/UsersManual.rst:1388 - * ``precise`` Disables optimizations that are not

[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

mibintc updated this revision to Diff 244178. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74436/new/ https://reviews.llvm.org/D74436 Files: clang/docs/UsersManual.rst clang/lib/Driver/ToolChains/Clang.cpp clang/test/CodeGen/ppc-emmintrin.c

[PATCH] D74468: [clang-tidy] No misc-definitions-in-headers warning on C++14 variable templates.

hokein updated this revision to Diff 244184. hokein marked 2 inline comments as done. hokein added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74468/new/ https://reviews.llvm.org/D74468 Files:

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

aganea added inline comments. Comment at: clang/include/clang/Driver/Job.h:90 + /// Whether the command will be executed in this process or not. + bool InProcess : 1; + hans wrote: > I think Reid just meant put the bool fields after each other to minimize >

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

llunak marked an inline comment as done. llunak added a comment. In D73852#1872013 , @lebedev.ri wrote: > This patch also omitted cfe-commits lists. That is a Phabricator problem.

[PATCH] D74468: [clang-tidy] No misc-definitions-in-headers warning on C++14 variable templates.

hokein added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers-1z.hpp:14 +template +constexpr T pi = T(3.1415926L); gribozavr2 wrote: > I would prefer if you could make the test compatible with all language

[clang-tools-extra] a45ca67 - [clang-tidy] No misc-definitions-in-headers warning on C++14 variable templates.

Author: Haojian Wu Date: 2020-02-12T16:56:31+01:00 New Revision: a45ca670f5c43253d71018814d1e00443726f23a URL: https://github.com/llvm/llvm-project/commit/a45ca670f5c43253d71018814d1e00443726f23a DIFF: https://github.com/llvm/llvm-project/commit/a45ca670f5c43253d71018814d1e00443726f23a.diff

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

aaron.ballman added a comment. In D73852#1872019 , @hans wrote: > I also jumped when I saw that this now makes certain comments "load bearing". > That doesn't seem like a great idea to me. It's an idea we already have in the project though with NOLINT

[PATCH] D74473: [analyzer] StdLibraryFunctionsChecker: Use platform dependent EOF and UCharMax

martong updated this revision to Diff 244149. martong added a comment. - Fix crash in TryExpandAsInteger Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74473/new/ https://reviews.llvm.org/D74473 Files:

[PATCH] D74483: [AArch64] Add Cortex-A34 Support for clang and llvm

LukeGeeson created this revision. Herald added subscribers: llvm-commits, cfe-commits, hiraditya, kristof.beyls. Herald added projects: clang, LLVM. This patch upstreams support for the AArch64 Armv8-A cpu Cortex-A34.

[PATCH] D74473: [analyzer] StdLibraryFunctionsChecker: Use platform dependent EOF and UCharMax

balazske added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h:17 #include "clang/AST/Stmt.h" +#include "clang/Lex/Preprocessor.h" +#include "llvm/ADT/Optional.h" We do not need to include Preprocessor.h here

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

thakis added a comment. In D73852#1872044 , @aaron.ballman wrote: > In D73852#1872000 , @thakis wrote: > > > I think giving comments a semantic meaning is a bad idea. Do we really have > > to do this? > > > I

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

aaron.ballman added a comment. In D73852#1872104 , @aaron.ballman wrote: > All this said, I am comfortable reverting this back out of the 10.0 branch > while we consider it harder. It does not seem so critical that we can't take > time to discuss it

[PATCH] D74385: [ARCMT][NFC] Reduce #include dependencies

serge-sans-paille added a comment. In D74385#1869876 , @nicolas17 wrote: > I don't have commit access, can someone push this for me? Sure, can you just confirm that you're this guy: https://github.com/nicolas17 (I need that for proper email

[PATCH] D74433: [llvm-objdump] Print file format in lowercase to match GNU output.

This revision was automatically updated to reflect the committed changes. Closed by commit rG60a8a504f16d: [llvm-objdump] Print file format in lowercase to match GNU output. (authored by rupprecht). Changed prior to commit: https://reviews.llvm.org/D74433?vs=243991=244189#toc Repository: rG

[PATCH] D74385: [ARCMT][NFC] Reduce #include dependencies

nicolas17 added a comment. In D74385#1872298 , @serge-sans-paille wrote: > Sure, can you just confirm that you're this guy: https://github.com/nicolas17 > (I need that for proper email attribution) Yes that's me :) Repository: rG LLVM Github

[PATCH] D74473: [analyzer] StdLibraryFunctionsChecker: Use platform dependent EOF and UCharMax

martong planned changes to this revision. martong marked 3 inline comments as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:533 + +IntValue.dump(); +return IntValue.getSExtValue();

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

lebedev.ri added a comment. Now this patch should be reverted. This patch also omitted cfe-commits lists. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73852/new/ https://reviews.llvm.org/D73852 ___

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

hans added a comment. I also jumped when I saw that this now makes certain comments "load bearing". That doesn't seem like a great idea to me. The warning may be all right for C++ code, which has an attribute to suppress it, but C code does not normally use such attributes, and has no standard

[PATCH] D74483: [AArch64] Add Cortex-A34 Support for clang and llvm

SjoerdMeijer added a comment. Ah, sorry, looks like this is all there is to it, both clang and llvm. It's just that a quick grep locally (for a similar core) showed some more results. Can you (double) check if it needs adding to e.g. a switch in `llvm/lib/Support/Host.cpp`? Repository: rG

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

thakis added a comment. I'd think that everyone debugging clang always passes a single TU to it, so I'm not sure debuggability does much here :) The `-disable-free` code has never been used in normal compilations, so we didn't have to worry about this path. This patch here brings us to 3 modes

[clang] 60a8a50 - [llvm-objdump] Print file format in lowercase to match GNU output.

Author: Jordan Rupprecht Date: 2020-02-12T08:17:01-08:00 New Revision: 60a8a504f16dbbc5f2a6887ecb668ef4cb834949 URL: https://github.com/llvm/llvm-project/commit/60a8a504f16dbbc5f2a6887ecb668ef4cb834949 DIFF:

[PATCH] D74399: [Driver][RISCV] Add RedHat Linux RISC-V triple

luismarques updated this revision to Diff 244187. luismarques added a comment. Herald added subscribers: apazos, pzheng, jocewei, the_o, brucehoult, MartinMosbeck, edward-jones, zzheng, niosHD, sabuasal, johnrusso, rbar. Adds a test (using the RISC-V Fedora 31 paths). CHANGES SINCE LAST ACTION

[PATCH] D73720: [Analyzer] Use note tags to track container begin and and changes

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. LGTM, I like everything here, you worded the notes very nicely and the test cases seems to cover everything I could find! Please wait for @NoQ's approval, since he's the ranking member

[clang] 271e495 - [Concepts] Add missing TPA commit to requires expression parsing

Author: Saar Raz Date: 2020-02-12T16:26:34+02:00 New Revision: 271e495399170d69627c1acd591c9298cb0b5b4b URL: https://github.com/llvm/llvm-project/commit/271e495399170d69627c1acd591c9298cb0b5b4b DIFF: https://github.com/llvm/llvm-project/commit/271e495399170d69627c1acd591c9298cb0b5b4b.diff

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

aganea updated this revision to Diff 244172. aganea marked 7 inline comments as done. aganea added a comment. Address @hans' comments. While it'd be good to have a bot running without `-disable-free`, I concur with @thakis. This patch will probably introduce too much (untested) variability for

  1   2   >