[PATCH] D52888: Thread safety analysis: Handle conditional expression in getTrylockCallExpr

2018-10-04 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: test/SemaCXX/warn-thread-safety-analysis.cpp:1879-1880 + void foo13() { +if (mu.TryLock() ? 1 : 0) + mu.Unlock(); + } aaron.ballman wrote: > aaronpuchert wrote: > > aaron.ballman wrote: > > > Can you add

[PATCH] D52888: Thread safety analysis: Handle conditional expression in getTrylockCallExpr

2018-10-04 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Additional branches (with non-tail recursion unfortunately) would allow the following to work: void foo16() { if (cond ? mu.TryLock() : false) mu.Unlock(); } void foo17() { if (cond ? true : !mu.TryLock()) return; mu.Unlock(); } W

[PATCH] D52578: Thread safety analysis: Allow scoped releasing of capabilities

2018-10-04 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: lib/Analysis/ThreadSafety.cpp:893 private: - SmallVector UnderlyingMutexes; + enum UnderlyingCapabilityKind { +UCK_Acquired, ///< Any kind of acquired capability. delesley wrote: > aaronpuchert wrote

[PATCH] D52443: Thread safety analysis: Examine constructor arguments

2018-10-04 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC343831: Thread safety analysis: Examine constructor arguments (authored by aaronpuchert, committed by ). Changed prior to commit: https://reviews.llvm.org/D52443?vs=167856&id=168414#toc Repository: r

[PATCH] D52888: Thread safety analysis: Handle conditional expression in getTrylockCallExpr

2018-10-05 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In https://reviews.llvm.org/D52888#1256395, @aaron.ballman wrote: > In https://reviews.llvm.org/D52888#1255862, @aaronpuchert wrote: > > > Additional changes (including some non-tail recursion unfortunately) would > > allow the following to work: > > > > void foo1

[PATCH] D52395: Thread safety analysis: Require exclusive lock for passing by non-const reference

2018-10-05 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 168505. aaronpuchert added a comment. Rebase on top of https://reviews.llvm.org/D52443. We also check the move constructor argument for write access, as suggested in a review. This isn't intended to be merged (yet?), it should be seen as an RFC. Repos

[PATCH] D52578: Thread safety analysis: Allow scoped releasing of capabilities

2018-10-05 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert planned changes to this revision. aaronpuchert added a comment. I think I'll try to simplify this and address @delesley's comments before we commit this. I'll admit that the semantics are somewhat counter-intuitive, but as I explained I think it's more consistent this way. Because t

[PATCH] D52888: Thread safety analysis: Handle conditional expression in getTrylockCallExpr

2018-10-05 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL343902: Thread safety analysis: Handle conditional expression in getTrylockCallExpr (authored by aaronpuchert, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://r

[PATCH] D52398: Thread safety analysis: Unwrap __builtin_expect in getTrylockCallExpr

2018-10-05 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In https://reviews.llvm.org/D52398#1257092, @rupprecht wrote: > I patched the proposed fix-forward and it seems to have things building > again. Is there an ETA on landing that? If it's going to take a bit, is there > any chance we could revert this patch until the

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

2019-05-31 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert removed a reviewer: bkramer. aaronpuchert added a comment. I discovered an issue with variables, because there can be multiple definitions with the same `TypeLoc`: extern int a; int a, b; We emit the warning for `b`, but we can't just put static in front of the declaration, as

[PATCH] D62750: Show note for -Wmissing-prototypes for functions with parameters

2019-05-31 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. There was a search for non-prototype declarations for the function, but we only showed the results for zero-parameter functions. Now we sh

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

2019-05-31 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. I've factored out some changes into D62750 and will rebase this on top if it. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59402/new/ https://reviews.llvm.org/D59402

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

2019-05-31 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 202479. aaronpuchert added a comment. This revision is now accepted and ready to land. Show note suggesting to add `static`, move fix-it to note for functions, remove fix-it for variables. Show note even for `extern` function definitions when we can't b

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

2019-05-31 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 202480. aaronpuchert added a comment. Remove other change from this one. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59402/new/ https://reviews.llvm.org/D59402 Files: include/clang/Basic/DiagnosticSemaKinds.td lib

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

2019-05-31 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 202483. aaronpuchert added a comment. Add missing CHECK-NOTs for -Wmissing-prototypes. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59402/new/ https://reviews.llvm.org/D59402 Files: include/clang/Basic/DiagnosticSema

[PATCH] D62470: Clarify when fix-it hints on warnings are appropriate

2019-05-31 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL362266: Clarify when fix-it hints on warnings are appropriate (authored by aaronpuchert, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANG

[PATCH] D59673: [Clang] Harmonize Split DWARF options with llc

2019-06-03 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert marked an inline comment as done. aaronpuchert added a comment. In D59673#1521413 , @dblaikie wrote: > Might be easier as a few patches - renaming the existing option, adding the > new one, then removing the single split dwarf flag handling i

[PATCH] D59673: [Clang] Harmonize Split DWARF options with llc

2019-06-05 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D59673#1527487 , @aaronpuchert wrote: > In D59673#1521413 , @dblaikie wrote: > > > Might be easier as a few patches - renaming the existing option, adding the > > new one, then rem

[PATCH] D63130: [Clang] Rename -split-dwarf-file to -split-dwarf-output

2019-06-11 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added reviewers: dblaikie, echristo. Herald added subscribers: dexonsmith, steven_wu, aprantl, mehdi_amini. Herald added a project: clang. This is the first in a series of changes trying to align clang -cc1 flags for Split DWARF with those of llc. T

[PATCH] D59673: [Clang] Harmonize Split DWARF options with llc

2019-06-11 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 204013. aaronpuchert added a comment. Split from other changes as suggested. A predecessor is in D63130 , and a successor will come soon. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D596

[PATCH] D59673: [Clang] Harmonize Split DWARF options with llc

2019-06-11 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 204017. aaronpuchert added a comment. Correct an oversight. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59673/new/ https://reviews.llvm.org/D59673 Files: include/clang/Basic/CodeGenOptions.h include/clang/Driver/C

[PATCH] D63167: [Clang] Remove obsolete -enable-split-dwarf={single,split}

2019-06-11 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added reviewers: dblaikie, echristo. aaronpuchert added a project: clang. Herald added a subscriber: aprantl. The changes in D59673 make the choice redundant, since we can achieve single-file split DWARF just by no

[PATCH] D59673: [Clang] Harmonize Split DWARF options with llc

2019-06-11 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert marked 2 inline comments as done. aaronpuchert added a comment. In D59673#1521413 , @dblaikie wrote: > Might be easier as a few patches - renaming the existing option, D63130 > adding the new one, This cha

[PATCH] D62750: Show note for -Wmissing-prototypes for functions with parameters

2019-06-11 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 204171. aaronpuchert added a comment. Add checks that we don't emit the fix-it when we shouldn't. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62750/new/ https://reviews.llvm.org/D62750 Files: include/clang/Basic/Dia

[PATCH] D63167: [Clang] Remove obsolete -enable-split-dwarf={single,split}

2019-06-12 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert marked an inline comment as done. aaronpuchert added inline comments. Comment at: include/clang/Basic/CodeGenOptions.def:263 -ENUM_CODEGENOPT(SplitDwarfMode, DwarfFissionKind, 2, NoFission) ///< DWARF fission mode to use. +CODEGENOPT(EnableSplitDwarf, 1, 0) ///< W

[PATCH] D59673: [Clang] Harmonize Split DWARF options with llc

2019-06-12 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a subscriber: tejohnson. aaronpuchert added inline comments. Comment at: lib/CodeGen/BackendUtil.cpp:1345 Conf.RemarksPasses = CGOpts.OptRecordPasses; - Conf.DwoPath = CGOpts.SplitDwarfFile; + Conf.DwoPath = CGOpts.SplitDwarfOutput; switch (Action) { --

[PATCH] D59673: [Clang] Harmonize Split DWARF options with llc

2019-06-12 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: lib/CodeGen/BackendUtil.cpp:1345 Conf.RemarksPasses = CGOpts.OptRecordPasses; - Conf.DwoPath = CGOpts.SplitDwarfFile; + Conf.DwoPath = CGOpts.SplitDwarfOutput; switch (Action) { tejohnson wrote: > aaronpuche

[PATCH] D59673: [Clang] Harmonize Split DWARF options with llc

2019-06-12 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: lib/CodeGen/BackendUtil.cpp:1345 Conf.RemarksPasses = CGOpts.OptRecordPasses; - Conf.DwoPath = CGOpts.SplitDwarfFile; + Conf.DwoPath = CGOpts.SplitDwarfOutput; switch (Action) { tejohnson wrote: > aaronpuche

[PATCH] D63167: [Clang] Remove obsolete -enable-split-dwarf={single,split}

2019-06-13 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: include/clang/Basic/CodeGenOptions.def:263 -ENUM_CODEGENOPT(SplitDwarfMode, DwarfFissionKind, 2, NoFission) ///< DWARF fission mode to use. +CODEGENOPT(EnableSplitDwarf, 1, 0) ///< Whether to enable split DWARF. ---

[PATCH] D59673: [Clang] Harmonize Split DWARF options with llc

2019-06-13 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 204661. aaronpuchert added a comment. Herald added subscribers: llvm-commits, dang, dexonsmith, steven_wu, hiraditya, mehdi_amini. Herald added a project: LLVM. Make sure the flags have the same meaning for LTO. Also slightly reworded the HelpText. Re

[PATCH] D59673: [Clang] Harmonize Split DWARF options with llc

2019-06-13 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a reviewer: tejohnson. aaronpuchert added a comment. Sorry for making the inline comments disappear, but I had to switch to the big repository. @tejohnson Could you have a look at the LTO-related changes (BackendUtil.cpp + all files with LTO in the name)? Repository: rG L

[PATCH] D59673: [Clang] Harmonize Split DWARF options with llc

2019-06-14 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: llvm/include/llvm/LTO/Config.h:92 + /// The name for the split debug info file used for the DW_AT_[GNU_]dwo_name + /// attribute in the skeleton CU. + std::string SplitDwarfFile; tejohnson wrote: > Probably needs

[PATCH] D63130: [Clang] Rename -split-dwarf-file to -split-dwarf-output

2019-06-15 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL363494: [Clang] Rename -split-dwarf-file to -split-dwarf-output (authored by aaronpuchert, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D59673: [Clang] Harmonize Split DWARF options with llc

2019-06-15 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. aaronpuchert marked 3 inline comments as done. Closed by commit rL363496: [Clang] Harmonize Split DWARF options with llc (authored by aaronpuchert, committed by ). Changed prior to commit: https://reviews.llvm.org/D59673?

[PATCH] D63167: [Clang] Remove obsolete -enable-split-dwarf={single,split}

2019-06-15 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 204928. aaronpuchert added a comment. Remove `-enable-split-dwarf` completely, adapting the test case accordingly. Also remove `-split-dwarf` which wasn't used at all. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D63167: [Clang] Remove unused -split-dwarf and obsolete -enable-split-dwarf

2019-06-15 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:864 default: -if (!CodeGenOpts.SplitDwarfOutput.empty() && -(CodeGenOpts.getSplitDwarfMode() == CodeGenOptions::SplitFileFission)) { +if (!CodeGenOpts.SplitDwarfOutput.empty()) {

[PATCH] D62750: Show note for -Wmissing-prototypes for functions with parameters

2019-06-17 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: lib/Sema/SemaDecl.cpp:13335 +? FixItHint::CreateInsertion(FTL.getRParenLoc(), "void") +: FixItHint{}); } aaron.ballman wrote: > In this case, we could probab

[PATCH] D59402: Suggestions to fix -Wmissing-{prototypes,variable-declarations}

2019-06-17 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: lib/Sema/SemaDecl.cpp:13345-13346 +<< (FD->getStorageClass() == SC_None +? FixItHint::CreateInsertion(FD->getTypeSpecStartLoc(), + "static ") +

[PATCH] D59402: Suggestions to fix -Wmissing-{prototypes,variable-declarations}

2019-06-17 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: lib/Sema/SemaDecl.cpp:13345-13346 +<< (FD->getStorageClass() == SC_None +? FixItHint::CreateInsertion(FD->getTypeSpecStartLoc(), + "static ") +

[PATCH] D62750: Show note for -Wmissing-prototypes for functions with parameters

2019-06-18 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL363748: Show note for -Wmissing-prototypes for functions with parameters (authored by aaronpuchert, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL

[PATCH] D59402: Suggestions to fix -Wmissing-{prototypes,variable-declarations}

2019-06-18 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. aaronpuchert marked an inline comment as done. Closed by commit rL363749: Suggestions to fix -Wmissing-{prototypes,variable-declarations} (authored by aaronpuchert, committed by ). Herald added a project: LLVM. Herald added

[PATCH] D65184: [Sema] Thread Safety Analysis: Fix negative capability's LockKind representation.

2019-07-25 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a reviewer: aaron.ballman. aaronpuchert added a comment. What distinguishes a shared from an exclusive negative capability? Negative capabilities (as I understand them) express the mutex not being held at all, meaning neither in shared nor in exclusive mode. ===

[PATCH] D65184: [Sema] Thread Safety Analysis: Fix negative capability's LockKind representation.

2019-07-30 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: clang/test/SemaCXX/thread-safety-annotations.h:47 +// Enable thread safety attributes only with clang. +// The attributes can be safely erased when compiling with other compilers. +#if defined(__clang__) && (!defined(SWIG))

[PATCH] D65184: [Sema] Thread Safety Analysis: Fix negative capability's LockKind representation.

2019-07-30 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. The primary purpose of Thread Safety Analysis is to make sure that accesses to shared resources are protected. That's why we only track whether a lock is available by default (i.e. without -Wthread-safety-negative), locks that we don't know anything about are assum

[PATCH] D65184: [Sema] Thread Safety Analysis: Fix negative capability's LockKind representation.

2019-07-30 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: clang/test/SemaCXX/warn-thread-safety-negative.cpp:135-140 + if (condition) { +assertNotHeld(); // expected-warning {{mutex '!mu' is acquired exclusively and shared in the same scope}} + } else { +mu.Lock(); +mu.Unloc

[PATCH] D55326: [Driver] Fix incorrect GNU triplet for PowerPC on SUSE Linux

2019-08-10 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Herald added subscribers: shchenz, wuzish. Sorry for warming this up again, but it would be nice to get rid of this patch in openSUSE. In D55326#1342412 , @nemanjai wrote: > - Will this change not affect 64-bit PPC SUSE? Nam

[PATCH] D63167: [Clang] Remove unused -split-dwarf and obsolete -enable-split-dwarf

2019-06-25 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:864 default: -if (!CodeGenOpts.SplitDwarfOutput.empty() && -(CodeGenOpts.getSplitDwarfMode() == CodeGenOptions::SplitFileFission)) { +if (!CodeGenOpts.SplitDwarfOutput.empty()) {

[PATCH] D63167: [Clang] Remove unused -split-dwarf and obsolete -enable-split-dwarf

2019-06-26 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL364479: [Clang] Remove unused -split-dwarf and obsolete -enable-split-dwarf (authored by aaronpuchert, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior

[PATCH] D63167: [Clang] Remove unused -split-dwarf and obsolete -enable-split-dwarf

2019-06-26 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Thanks for the reviews! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63167/new/ https://reviews.llvm.org/D63167 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm

[PATCH] D59673: [Clang] Harmonize Split DWARF options with llc

2019-05-22 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Ping. I'm sorry that the change turned out so big, but note that it doesn't change the behavior of any non-cc1 flags. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59673/new/ https://reviews.llvm.org/D59673 __

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

2019-05-22 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a reviewer: ed. aaronpuchert added a comment. Another ping. I've tried to add the original authors as reviewers, but both warnings are several years old and you might not remember. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59402/new/ https

[PATCH] D52395: Thread safety analysis: Require exclusive lock for passing by non-const reference

2019-05-22 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert planned changes to this revision. aaronpuchert added a comment. Herald added a project: clang. I don't actually want to change anything, but remove this from your review queues until I have an idea how often the warning fires and how many false positives we have. Repository: rC

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

2019-05-24 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D59402#1515703 , @aaron.ballman wrote: > My only real concern about this is that sometimes the fix-it will be wrong > because the issue is a typo in the definition. You're right about that, there are multiple reasons wh

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

2019-05-24 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert planned changes to this revision. aaronpuchert added a comment. I guess you're referring to "[fix-it hints] should only be used when it’s very likely they match the user’s intent". When turning on the warning on an existing code base, I think that `static` is almost always right. B

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

2019-05-24 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D59402#1516432 , @aaron.ballman wrote: > Also, we're not attempting to recover from the error, which is a good point > that @thakis raised. aka, if you apply the fix-it, you should also treat the > declaration as though

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

2019-05-24 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert marked an inline comment as done. aaronpuchert added a comment. In D59402#1516482 , @rsmith wrote: > In D59402#1516479 , @aaronpuchert > wrote: > > > In D59402#1516432

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

2019-05-24 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D59402#1516567 , @aaronpuchert wrote: > If that is wrong, then my reading is that fix-it hints for warnings must > always be applied to a note, because the recovery must be a no-op. Not true, because some warnings provi

[PATCH] D62470: Clarify when fix-it hints on warnings are appropriate

2019-05-26 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added reviewers: rsmith, aaron.ballman. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is not a change in the rules, it's meant as a clarification about warnings. Since the recovery from warnings is a no-op, the fix-it h

[PATCH] D62470: Clarify when fix-it hints on warnings are appropriate

2019-05-26 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Feel free to suggest better wording, I'm not a native speaker. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62470/new/ https://reviews.llvm.org/D62470 ___ cfe-commits mailing list cfe-c

[PATCH] D62470: Clarify when fix-it hints on warnings are appropriate

2019-05-27 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert marked 2 inline comments as done. aaronpuchert added inline comments. Comment at: docs/InternalsManual.rst:426-428 +* Fix-it hints on a warning should only clarify the meaning of the existing + code, not change it. Examples for such hints are added parentheses in ca

[PATCH] D62470: Clarify when fix-it hints on warnings are appropriate

2019-05-27 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert marked 3 inline comments as done. aaronpuchert added inline comments. Comment at: docs/InternalsManual.rst:426-428 +* Fix-it hints on a warning should only clarify the meaning of the existing + code, not change it. Examples for such hints are added parentheses in ca

[PATCH] D62470: Clarify when fix-it hints on warnings are appropriate

2019-05-27 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 201597. aaronpuchert marked an inline comment as done. aaronpuchert added a comment. Put the important part first, and the rest in a separate sentence. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62470/new/ https://rev

[PATCH] D52578: Thread safety analysis: Allow scoped releasing of capabilities

2018-12-16 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL349300: Thread safety analysis: Allow scoped releasing of capabilities (authored by aaronpuchert, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTION

[PATCH] D67321: Respect CLANG_LINK_CLANG_DYLIB=ON in libclang and c-index-test

2019-09-09 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: clang/tools/libclang/CMakeLists.txt:115 +clang_target_link_libraries(libclang + PRIVATE + ${CLANG_LIB_DEPS} beanz wrote: > tstellar wrote: > > aaronpuchert wrote: > > > This might not be correct for static builds,

[PATCH] D55326: [Driver] Fix incorrect GNU triplet for PowerPC on SUSE Linux

2019-09-21 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 221197. aaronpuchert added a comment. Pass correct Clang triple as argument to --target. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55326/new/ https://reviews.llvm.org/D55326 Files: clang/lib/Driver/

[PATCH] D67112: [Sema] Add implicit cast for conversion of function references

2019-09-21 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a reviewer: riccibruno. aaronpuchert added a comment. Perhaps I should mention that this fixes an assertion failure . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67112/new/ https://

[PATCH] D55326: [Driver] Fix incorrect GNU triplet for PowerPC on SUSE Linux

2019-09-25 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D55326#1342412 , @nemanjai wrote: > A couple of questions since I am not all that familiar with clang and am > certainly not familiar with this unusual SUSE 32-bit situation: > > - We seem to be changing the set of aliases

[PATCH] D68172: Don't install example analyzer plugins

2019-09-27 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added reviewers: hintonda, NoQ. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, a.sidorin, baloghadamsoftware, mgorny. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D68172 File

[PATCH] D68172: Don't install example analyzer plugins

2019-09-27 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D68172#1686772 , @NoQ wrote: > +@Szelethus because i'm a bit out-of-the-loop on plugins: i very vaguely > remember that we decided to put them into tests(?) The plugins were in `test`, but D62445

[PATCH] D67877: [analyzer] Conditionnaly include clang Analysis examples with cmake.

2019-09-28 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. I didn't know about this patch and published D68172 : adding `BUILDTREE_ONLY` still builds the examples, but doesn't install them. It's also used by other test plugins like `llvm/lib/Transforms/Hello`. Thanks to @lebedev.ri for inf

[PATCH] D68172: Don't install example analyzer plugins

2019-09-28 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D68172#1686960 , @Szelethus wrote: > The patch looks alright, I won't formally accept because if I knew how these > worked, it wouldn't have caused so much pain to so many people :) If it helps, this is from `add_llvm_li

[PATCH] D67877: [analyzer] Conditionnaly include clang Analysis examples with cmake.

2019-09-28 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D67877#1686961 , @Jiboo wrote: > @Szelethus could you confirm that thus examples shouldn't be built when > CLANG_BUILD_EXAMPLES is OFF, and that this patch is still valid? The problem is that `CLANG_BUILD_EXAMPLES` is of

[PATCH] D68172: Don't install example analyzer plugins

2019-09-28 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL373148: Don't install example analyzer plugins (authored by aaronpuchert, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.

[PATCH] D67321: Respect CLANG_LINK_CLANG_DYLIB=ON in libclang and c-index-test

2019-09-28 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 222302. aaronpuchert marked 4 inline comments as done. aaronpuchert added a comment. Handle static libraries correctly. Dependencies of executables are obviously always PRIVATE, but for library targets we have to turn PRIVATE or PUBLIC dependencies into

[PATCH] D67321: Respect CLANG_LINK_CLANG_DYLIB=ON in libclang and c-index-test

2019-09-28 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 222303. aaronpuchert added a comment. Fix typo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67321/new/ https://reviews.llvm.org/D67321 Files: clang/cmake/modules/AddClang.cmake clang/tools/c-index-t

[PATCH] D68187: [libclang] Use strict prototypes in header

2019-09-28 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, arphaman, ddunbar, jkorous. Herald added subscribers: cfe-commits, dexonsmith. Herald added a project: clang. When included in a C program, these function declarations are treated as K&R declarations, which conveys no

[PATCH] D68187: [libclang] Use strict prototypes in header

2019-09-28 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Can be "reproduced" with `clang -fsyntax-only -Wstrict-prototypes clang/include/clang-c/*.h`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68187/new/ https://reviews.llvm.org/D68187 ___

[PATCH] D68187: [libclang] Use strict prototypes in header

2019-09-30 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert abandoned this revision. aaronpuchert added a comment. Fixed by @aaron.ballman in rC373213 along with another issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68187/new/ https://reviews.llvm.org

[PATCH] D67113: ICK_Function_Conversion is a third kind conversion

2019-09-30 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Ping? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67113/new/ https://reviews.llvm.org/D67113 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.o

[PATCH] D49275: Thread safety: Run tests with both lock and capability attributes

2018-07-12 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added reviewers: delesley, aaron.ballman. Herald added a subscriber: cfe-commits. Running all tests with both sets of attributes should make sure there is no regression in either variant. Repository: rC Clang https://reviews.llvm.org/D49275 Fi

[PATCH] D49275: Thread safety: Run tests with both lock and capability attributes

2018-07-13 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Thanks for the review. Could you commit this as `Aaron Puchert https://reviews.llvm.org/D49275 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2018-07-15 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, delesley. Herald added a subscriber: cfe-commits. We can now have methods that release a locked in shared mode and acquire it in exclusive mode or the other way around. The fix was just to release the locks before acq

[PATCH] D67112: [Sema] Add implicit cast for conversion of function references

2019-10-08 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert planned changes to this revision. aaronpuchert added a comment. In D67112#1655937 , @aaronpuchert wrote: > If anyone shares my feeling that the boolean output parameters of > `CompareReferenceRelationship` should rather move to the return va

[PATCH] D55326: [Driver] Fix incorrect GNU triplet for PowerPC on SUSE Linux

2019-10-08 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Gentle ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55326/new/ https://reviews.llvm.org/D55326 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists

[PATCH] D51741: [coro]Pass rvalue reference for named local variable to return_value

2019-10-09 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Herald added a project: LLVM. This change breaks the following code that worked before: task f(MoveOnly &value) { co_return value; } The error message is: clang/test/SemaCXX/coroutine-rvo.cpp:60:13: error: call to deleted constructor of 'MoveOnly' co

[PATCH] D51741: [coro]Pass rvalue reference for named local variable to return_value

2019-10-09 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D51741#1702038 , @Quuxplusone wrote: > This patch is heavily heavily merge-conflicted by P1825 > . Just to be clear, this change is pretty old: it's a

[PATCH] D51741: [coro]Pass rvalue reference for named local variable to return_value

2019-10-10 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. @Quuxplusone Thanks for your very helpful comments! > In which case, this patch (D51741 ) itself > fixed this FIXME at least partly, and maybe completely. Maybe this patch > should have removed or amended the FIXME, rather than just

[PATCH] D68845: Don't emit unwanted constructor calls in co_return statements

2019-10-10 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 224513. aaronpuchert added a comment. Also remove FIXME comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68845/new/ https://reviews.llvm.org/D68845 Files: clang/lib/Sema/SemaCoroutine.cpp clang/

[PATCH] D68845: Don't emit unwanted constructor calls in co_return statements

2019-10-10 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added reviewers: GorNishanov, modocache, Quuxplusone. Herald added a project: clang. Herald added a subscriber: cfe-commits. aaronpuchert updated this revision to Diff 224513. aaronpuchert added a comment. Also remove FIXME comment. The implementa

[PATCH] D51741: [coro]Pass rvalue reference for named local variable to return_value

2019-10-10 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Please have a look at D68845 . This should address the issues that we discussed. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51741/new/ https://reviews.llvm.org/D51741 ___

[PATCH] D68845: Don't emit unwanted constructor calls in co_return statements

2019-10-10 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments. Comment at: clang/lib/Sema/SemaCoroutine.cpp:869 if (E) { -auto NRVOCandidate = this->getCopyElisionCandidate(E->getType(), E, CES_AsIfByStdMove); -if (NRVOCandidate) { - InitializedEntity Entity = - InitializedEntity:

[PATCH] D68845: Don't emit unwanted constructor calls in co_return statements

2019-10-11 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert planned changes to this revision. aaronpuchert added a comment. In D51741#1702038 , @Quuxplusone wrote: > Both of these should first do overload resolution for one parameter of type > `MoveOnly&&`, and then, only if that overload resolution f

[PATCH] D68845: Don't emit unwanted constructor calls in co_return statements

2019-10-12 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. I'll add your test case, but I'll probably reuse the existing data structures. In D68845#1705430 , @Quuxplusone wrote: > Oh, and can you please make sure there are test cases for all the various > cases covered in P1155 > <

[PATCH] D68845: Don't emit unwanted constructor calls in co_return statements

2019-10-12 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. @GorNishanov Do I read the standard correctly that there are no constraints on what `return_value` is? That is, could it also be a function pointer or function object? struct promise_function_pointer { // ... void (*return_value)(T&& value); }; struct

[PATCH] D68923: Don't warn about missing declarations for partial template specializations

2019-10-12 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. Just like templates, they are excepted from the ODR rule. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D68923 Files:

[PATCH] D68845: Don't emit unwanted constructor calls in co_return statements

2019-10-15 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 225147. aaronpuchert added a comment. Add tests suggested by @Quuxplusone and add fallback to call by lvalue reference. The latter turned out more complicated than I thought, because there seems to be no easy way to just try building a call expression

[PATCH] D68845: Don't emit unwanted constructor calls in co_return statements

2019-10-15 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 225149. aaronpuchert added a comment. Apply clang-format. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68845/new/ https://reviews.llvm.org/D68845 Files: clang/lib/Sema/SemaCoroutine.cpp clang/test/Se

[PATCH] D68845: Don't emit unwanted constructor calls in co_return statements

2019-10-15 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert marked 15 inline comments as done. aaronpuchert added a comment. Given the complexities of this implementation, I'm beginning to doubt whether implicit moves make sense for `co_return` at all. Since there can never be any kind of RVO, why not always require an explicit `std::move`?

[PATCH] D67321: Respect CLANG_LINK_CLANG_DYLIB=ON in libclang and c-index-test

2019-10-16 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. @beanz Could you have a look again? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67321/new/ https://reviews.llvm.org/D67321 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D69533: Thread safety analysis: Peel away NoOp implicit casts in initializers

2019-10-28 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. This happens when someone initializes a variable with guaranteed copy elision and an added const qualifier. Fixes PR43826. Repository:

<    1   2   3   4   5   6   7   >