[PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

2022-11-29 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. > The clang-side interface to this seems a touch clunky, and I fear it'll make > continuing its use/generalizing its use less likely. Is this w.r.t. the `FormatDiagnostic` being split up, or is it something else? If it's the former: I'll be changing that to `FormatLegacyDi

[PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

2022-11-29 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. > though I find myself wondering if the "FormatDiagnostic" call should stay the > same, and choose the legacy-reason only when a sarif reason doesn't exist? Or > for some level of command line control? Hmm... isn't this the point of the diagnostic consumers? Repository:

[PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

2022-12-01 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb marked 3 inline comments as done. cjdb added a comment. In D138939#3963496 , @erichkeane wrote: > In D138939#3963473 , @tschuett > wrote: > >> Maybe `void FormatDiagnostic(SmallVectorImpl &OutStr, Diagnosti

[PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

2022-12-01 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 479492. cjdb marked 4 inline comments as done. cjdb added a comment. responds to feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138939/new/ https://reviews.llvm.org/D138939 Files: clang-tools-extra/clan

[PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

2022-12-01 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. In D138939#3964677 , @erichkeane wrote: > In D138939#3964439 , @erichkeane > wrote: > >> In D138939#3964404 , @cjdb wrote: >> >>> In D138939#396349

[PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

2022-12-02 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. In D138939#3965985 , @tschuett wrote: > Then Sarif was a distraction. Still to reduce boilerplate and for A/B testing: > > enum class DiagnosticMode { > Legacy, > UserOriented, > Default = Legacy > } It took a fair b

[PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

2022-12-02 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. In D138939#3966938 , @tschuett wrote: > Maybe the kind/amount of information printed ( `DiagnosticMode` ) and the > output device (console/sarif) are orthogonal issues. > > Still it would nice to be able to toggle the diagnostic mod

[PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

2022-12-02 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. In D138939#3967397 , @tschuett wrote: > I do not ask you to do anything! I just noticed that you add a lot of > `FormatXXXDiagnostic` functions. An alternativ design is to have one > `FormatDiagnostic` function with a mode paramete

[PATCH] D146376: Update static_assert message for redundant cases

2023-03-20 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. Thanks for working on this, it's much appreciated! :4:1: error: static assertion failed due to requirement 'is_gitlab' static_assert(is_gitlab and is_weekend); ^ ~ The arrow seems a bit off here. Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

2023-03-21 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added inline comments. Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:214 void SARIFDiagnostic::emitIncludeLocation(FullSourceLoc Loc, PresumedLoc PLoc) { - assert(false && "Not implemented in SARIF mode"); + SarifRule Rule = SarifRule::create().setRuleId(std::to_str

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

2023-03-21 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 507137. cjdb added a comment. fixes breakage and adds FIXME Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145201/new/ https://reviews.llvm.org/D145201 Files: clang/include/clang/Frontend/SARIFDiagnostic.h cla

[PATCH] D146376: Update static_assert message for redundant cases

2023-03-22 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added inline comments. Comment at: clang/test/SemaCXX/static-assert.cpp:261-265 static_assert(invert(true) == invert(false), ""); // expected-error {{failed}} \ -// expected-note {{evaluates to 'false == true'}} /

[PATCH] D146654: [clang] replaces numeric SARIF ids with heirarchical names

2023-03-22 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb created this revision. cjdb added reviewers: aaron.ballman, vaibhav.y, denik. Herald added a project: All. cjdb requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Per §3.27.5 and §3.27.7, the `rule.id` and `ruleId` properties need to be h

[PATCH] D146654: [clang] replaces numeric SARIF ids with heirarchical names

2023-03-22 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 507494. cjdb edited the summary of this revision. cjdb added a comment. swaps commits to see if that fixes CI Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146654/new/ https://reviews.llvm.org/D146654 Files: cl

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

2023-03-22 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 507497. cjdb edited the summary of this revision. cjdb added a comment. swaps commits to see if that fixes CI (part 1) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145201/new/ https://reviews.llvm.org/D145201 Fi

[PATCH] D146654: [clang] replaces numeric SARIF ids with heirarchical names

2023-03-22 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 507503. cjdb edited the summary of this revision. cjdb added a comment. rebasing continues Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146654/new/ https://reviews.llvm.org/D146654 Files: clang/include/clang/B

[PATCH] D146654: [clang] replaces numeric SARIF ids with heirarchical names

2023-03-22 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 507510. cjdb added a comment. I think this corrects everything. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146654/new/ https://reviews.llvm.org/D146654 Files: clang/include/clang/Basic/Diagnostic.h clang/i

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

2023-03-22 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 507511. cjdb edited the summary of this revision. cjdb added a comment. And on this CL's side too Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145201/new/ https://reviews.llvm.org/D145201 Files: clang/include/

[PATCH] D146654: [clang] replaces numeric SARIF ids with heirarchical names

2023-03-22 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added inline comments. Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:51-52 + Diag->getDiags()->getDiagnosticIDs()->getStableName(Diag->getID()).str(); + std::replace(StableName.begin(), StableName.end(), '_', '.'); + SarifRule Rule = SarifRule::create().setRuleId

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

2023-03-23 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. In D145201#4216567 , @aaron.ballman wrote: > In D145201#4214395 , @cjdb wrote: > >> swaps commits to see if that fixes CI (part 1) > > Oh, if this is just stacked patches confusing our preco

[PATCH] D146358: [clang][AST] Print name instead of type when diagnosing uninitialized subobject in constexpr variables

2023-03-28 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. In D146358#4204412 , @tbaeder wrote: > "subobject named 'foo'" sounds a bit weird to me, I'd expect just "subobject > 'foo'", but that's just a suggestion and I'll wait for a native spearker to > chime in on this. My expert brain

[PATCH] D146358: [clang][AST] Print name instead of type when diagnosing uninitialized subobject in constexpr variables

2023-03-28 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. In D146358#4229120 , @hazohelet wrote: > In D146358#4227938 , @cjdb wrote: > >> In D146358#4204412 , @tbaeder >> wrote: >> >>> "subobject named 'foo

[PATCH] D145178: [clang][NFC] reformats the SARIF diagnostic test so it's human readable

2023-03-02 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb created this revision. cjdb added reviewers: aaron.ballman, shafik, erichkeane. Herald added a project: All. cjdb requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The current FileCheck test output is very difficult to read, which makes

[PATCH] D145178: [clang][NFC] reformats the SARIF diagnostic test so it's human readable

2023-03-02 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added inline comments. Comment at: clang/test/Frontend/sarif-diagnostics.cpp:31 +// RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif-stderr %s > %t.txt 2>&1 || true +// RUN: FileCheck -dump-input=always %s --input-file=%t.txt --check-prefixes=CHECK,COMMON

[PATCH] D145178: [clang][NFC] reformats the SARIF diagnostic test so it's human readable

2023-03-02 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added inline comments. Comment at: clang/test/Frontend/sarif-diagnostics.cpp:31 +// RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif-stderr %s > %t.txt 2>&1 || true +// RUN: FileCheck -dump-input=always %s --input-file=%t.txt --check-prefixes=CHECK,COMMON

[PATCH] D145178: [clang][NFC] reformats the SARIF diagnostic test so it's human readable

2023-03-02 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 501955. cjdb added a comment. renames check identifiers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145178/new/ https://reviews.llvm.org/D145178 Files: clang/test/Frontend/sarif-diagnostics.cpp Index: clang/

[PATCH] D145178: [clang][NFC] reformats the SARIF diagnostic test so it's human readable

2023-03-02 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 501982. cjdb added a comment. removes redundant line Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145178/new/ https://reviews.llvm.org/D145178 Files: clang/test/Frontend/sarif-diagnostics.cpp Index: clang/tes

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

2023-03-02 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb created this revision. cjdb added reviewers: aaron.ballman, shafik, erichkeane. Herald added a project: All. cjdb requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Including headers used to fire an assertion; now they report a diagnostic

[PATCH] D145178: [clang][NFC] reformats the SARIF diagnostic test so it's human readable

2023-03-02 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 502019. cjdb added a comment. removes something from a future commit This should be the final alteration pre-review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145178/new/ https://reviews.llvm.org/D145178 Fil

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

2023-03-02 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 502020. cjdb added a comment. actually commits the files Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145201/new/ https://reviews.llvm.org/D145201 Files: clang/include/clang/Frontend/SARIFDiagnostic.h clang/

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

2023-03-02 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 502021. cjdb added a comment. fixes something that wasn't supposed to be changed in the rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145201/new/ https://reviews.llvm.org/D145201 Files: clang/include/cla

[PATCH] D145178: [clang][NFC] reformats the SARIF diagnostic test so it's human readable

2023-03-03 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. In D145178#4167103 , @aaron.ballman wrote: > LGTM, this is incremental progress. Hopefully we won't be adding too many > more RUN lines to this file though (sticking too many tests into one file is > also tech debt). Thanks! 100%

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

2023-03-03 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 502186. cjdb added a comment. Herald added a subscriber: mgrang. sorts artifacts so that they're output in index order Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145201/new/ https://reviews.llvm.org/D145201 Fi

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

2023-03-03 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. That test is kinda problematic because it seems that the artifacts aren't ordered. I think we should change this from a Comment at: clang/lib/Basic/Sarif.cpp:314-317 + llvm::sort(*Artifacts, [](const json::Value &x, const json::Value &y) { +return x.

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

2023-03-03 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 502190. cjdb added a comment. fixes string goof Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145201/new/ https://reviews.llvm.org/D145201 Files: clang/include/clang/Frontend/SARIFDiagnostic.h clang/lib/Basic

[PATCH] D145284: WIP [clang] adds capabilities for SARIF to be written to file

2023-03-03 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb created this revision. cjdb added reviewers: aaron.ballman, erichkeane, shafik, dblaikie. Herald added a project: All. cjdb requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. The original `-fdiagnostics-format=sarif` wrote directl

[PATCH] D145284: WIP [clang] adds capabilities for SARIF to be written to file

2023-03-03 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 502305. cjdb edited the summary of this revision. cjdb added a comment. adds dependency Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145284/new/ https://reviews.llvm.org/D145284 Files: clang/include/clang/Basi

[PATCH] D145284: WIP [clang] adds capabilities for SARIF to be written to file

2023-03-03 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 502308. cjdb added a comment. tidies up some stuff that I overlooked Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145284/new/ https://reviews.llvm.org/D145284 Files: clang/include/clang/Basic/DiagnosticOptions

[PATCH] D145438: PLEASE DO NOT COMMENT ON THIS PATCH

2023-03-06 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb created this revision. Herald added a project: All. cjdb requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is one of two alternative designs that Clang can adopt when writing SARIF to file. In this design, the Clang driver invokes a

[PATCH] D145439: PLEASE DO NOT COMMENT ON THIS PATCH

2023-03-06 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb created this revision. Herald added a project: All. cjdb requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is one of two alternative designs that Clang can adopt when writing SARIF to file. In this design, the Clang driver will bund

[PATCH] D129951: adds `__disable_adl` attribute

2023-03-08 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. In D129951#4178154 , @philnik wrote: > I don't think libc++ can adopt this without having to essentially duplicate > our code, since GCC doesn't support `__disable_adl` (and AFAICT there is no > coordination between GCC and Clang t

[PATCH] D129951: adds `__disable_adl` attribute

2023-03-08 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. In D129951#4178949 , @philnik wrote: > In D129951#4178844 , @cjdb wrote: > >> In D129951#4178154 , @philnik >> wrote: >> >>> I don't think libc++ ca

[PATCH] D129951: adds `__disable_adl` attribute

2023-03-08 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. In D129951#4179481 , @ldionne wrote: > In D129951#4179467 , @cjdb wrote: > >> I'm deeply disappointed that libc++ moved away from using `__function_like`: >> that was an important part of pr

[PATCH] D145284: WIP [clang] adds capabilities for SARIF to be written to file

2023-03-10 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 504309. cjdb added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. merges in D145438 . This patch is ready for review now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D145438: PLEASE DO NOT COMMENT ON THIS PATCH

2023-03-10 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb abandoned this revision. cjdb added a comment. Merged into D145284 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145438/new/ https://reviews.llvm.org/D145438 ___

[PATCH] D145439: PLEASE DO NOT COMMENT ON THIS PATCH

2023-03-10 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. D145438 was merged into D145284 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145439/new/ https://reviews.llvm.org/D145439 __

[PATCH] D145793: [clang][AST] Improve diagnostic for `nullptr` constexpr function pointer call

2023-03-10 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb accepted this revision. cjdb added a comment. This revision is now accepted and ready to land. LGTM pending other reviewers' commentary. Thanks for working on this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145793/new/ https://reviews.llvm

[PATCH] D145856: [clang-tidy] Let misc-const-correctness detect auto local variables that can be made const

2023-03-13 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. Would you be able to update the commit message to include a description that explains why this commit is necessary please? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145856/new/ https://reviews.llvm.org/D145856 __

[PATCH] D145284: WIP [clang] adds capabilities for SARIF to be written to file

2023-03-13 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb marked an inline comment as done. cjdb added inline comments. Comment at: clang/include/clang/Basic/DiagnosticOptions.h:110 + /// The file to serialise text diagnostics to (non-appending). + std::string FilePath; + erichkeane wrote: > I'm a touch disturbed

[PATCH] D153359: [clang][Diagnostics] Fix distant source ranges in bad-conversion notes

2023-06-21 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. I like this patch, thanks for working on it 😄 In D153359#4436873 , @hazohelet wrote: > Consider the following code. (I added another parameter to the original code > so that the covered range becomes clearer) > > void func(int aa

[PATCH] D153267: [clang][Diagnostics] Provide parameter source range to arity-mismatch notes

2023-06-21 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. I think this will be a good addition because it underscores the salient part of why the function isn't viable. Would it be possible for patches like this to have both examples of before and after this patch in their commit message please? That'll help expose why patches l

[PATCH] D153267: [clang][Diagnostics] Provide parameter source range to arity-mismatch notes

2023-06-23 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb accepted this revision. cjdb added a comment. Thanks! This LGTM now. Do you need assistance with merging? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153267/new/ https://reviews.llvm.org/D153267 ___ cfe-commits mailing list cfe-commits

[PATCH] D152686: [clangd] Enforce strict unused includes by default

2023-06-26 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. This patch seems to have broken clangd for a project of mine. Most of my headers are now flagged as not being directly used even though they're the root header for the symbol. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152

[PATCH] D153690: [clang][Sema] Remove dead diagnostic for loss of __unaligned qualifier

2023-06-26 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. Can we get a snippet of code to demonstrate what isn't being diagnosed anymore, please? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153690/new/ https://reviews.llvm.org/D153690 _

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

2023-06-27 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6589 +def err_using_placeholder_variable : Error< + "referring to placeholder '_' is not allowed">; +def note_reference_placeholder : Note< aaron.ballman wrote: > I don't th

[PATCH] D153690: [clang][Sema] Remove dead diagnostic for loss of __unaligned qualifier

2023-06-27 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb accepted this revision. cjdb added a comment. Excellent, thanks! LGTM, do you need assistance with merging? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153690/new/ https://reviews.llvm.org/D153690 ___ cfe-commits mailing list cfe-commi

[PATCH] D153359: [clang][Diagnostics] Fix distant source ranges in bad-conversion notes

2023-06-28 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. Per this Discourse post , I think it'd be best for the example I asked to be in your commit message to actually be in your release notes. Sorry for the churn! CHANGES SINCE LAST ACTION

[PATCH] D153849: [clang][Diagnostics] Fix diagnostic line numbers

2023-06-29 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. LGTM. Not sure if the `Fixes https://github.com/llvm/llvm-project/issues/63524` line autocloses the issue, but note that `Fixes #63524` does. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153849/new/ https://reviews.llvm.org/

[PATCH] D153359: [clang][Diagnostics] Fix distant source ranges in bad-conversion notes

2023-06-30 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. In D153359#4463009 , @tbaeder wrote: > @cjdb I know this is kind of silly after @hazohelet has already added it to > the release notes... but it seems like showing the difference is useless > since the difference was only introduce

[PATCH] D154253: [clang] detect integer overflow through temporary values

2023-06-30 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. > Fixes GH63629. Would you mind changing this to `Fixes #63629` please? That'll get it connected in GitHub. The patch looks good, would you mind adding some release notes before I give LGTM please? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D147288: [clang][NFC] updates cxx_status for P2113R0

2023-03-30 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb created this revision. cjdb added reviewers: erichkeane, aaron.ballman, shafik. Herald added a project: All. cjdb requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Issue #61857 identifies that there are some edge cases that aren't accoun

[PATCH] D147288: [clang][NFC] updates cxx_status for P2113R0

2023-03-30 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. I'm going to keep this CL open till someone comments, but after reading the relevant GCC bug , I'm not sure it's worth committing anymore. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D144522: [clang-tidy] Add readability-operators-representation check

2023-03-31 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. Thanks so much for seeing this through; I'm unusually looking forward to rebuilding LLVM this weekend! Comment at: clang-tools-extra/docs/clang-tidy/checks/readability/operators-representation.rst:82 + +.. option:: OverloadedOperators +

[PATCH] D147288: [clang][NFC] updates cxx_status for P2113R0

2023-03-31 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb abandoned this revision. cjdb added a comment. In D147288#4237413 , @royjacobson wrote: > There was some discussion of this last year in this review: > https://reviews.llvm.org/D128750 > > It's such an edge case that I don't think we should lose sl

[PATCH] D147717: [C++20][NFC] Claim full support for consteval again

2023-04-06 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. In D147717#4248989 , @erichkeane wrote: > @cjdb has been running some GDB test suites against our compiler: I am > wondering if we could ask him to try the consteval ones too before we set > this? A lot of that work is manual bec

[PATCH] D147875: [clang][Diagnostics] WIP: Show line numbers when printing code snippets

2023-04-10 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. I have no opinion on this addition. If folks find it useful in GCC, happy to see it added. Comment at: clang/lib/Frontend/TextDiagnostic.cpp:1139 +return 7; + return 0; +} I'd prefer this to be an assert rather than returning a value

[PATCH] D147904: [Clang] Fix cast to BigIntType in hasUniqueObjectRepresentations

2023-04-10 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb accepted this revision. cjdb added a comment. This revision is now accepted and ready to land. LGTM, thanks for such a quick fix on a weekend :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147904/new/ https://reviews.llvm.org/D147904 __

[PATCH] D151162: Add -Wpacked-non-pod to -Wall

2023-05-24 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb accepted this revision. cjdb added a comment. This revision is now accepted and ready to land. Thanks for working on this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151162/new/ https://reviews.llvm.org/D151162

[PATCH] D151575: [clang][diagnostics] Always show include stacks on errors

2023-05-30 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. Thanks for working on this. Would you mind adding more context to the commit description please? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151575/new/ https://reviews.llvm.org/D151575

[PATCH] D151952: [clang] adds `__type_pack_index` so we can get a type's parameter pack index

2023-06-01 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb created this revision. cjdb added reviewers: aaron.ballman, erichkeane, shafik, dblaikie. Herald added subscribers: arphaman, martong. Herald added a project: All. cjdb requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Similarly to `__ty

[PATCH] D151952: [clang] adds `__type_pack_index` so we can get a type's parameter pack index

2023-06-01 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 527681. cjdb added a comment. fixes some code I wrote when debugging Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151952/new/ https://reviews.llvm.org/D151952 Files: clang/docs/LanguageExtensions.rst clang/i

[PATCH] D151952: [clang] adds `__type_pack_index` so we can get a type's parameter pack index

2023-06-01 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added inline comments. Comment at: clang/include/clang/AST/Stmt.h:796-802 +/// If this expression is not value-dependent, this stores the value. +unsigned Value : 8; /// The number of arguments to this type trait. According to [implimits] /// 8 bits would

[PATCH] D151952: [clang] adds `__type_pack_index` so we can get a type's parameter pack index

2023-06-02 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added inline comments. Comment at: clang/include/clang/AST/Stmt.h:796-802 +/// If this expression is not value-dependent, this stores the value. +unsigned Value : 8; /// The number of arguments to this type trait. According to [implimits] /// 8 bits would

[PATCH] D152034: [clang][NFC] refactors value type traits so we can have more than bools

2023-06-02 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb created this revision. cjdb added reviewers: erichkeane, dblaikie. Herald added a project: All. cjdb requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Since all the type traits up until now have had Boolean vaules, we've always been able

[PATCH] D151952: [clang] adds `__type_pack_index` so we can get a type's parameter pack index

2023-06-02 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:5620 + if (Kind == clang::TT_TypePackIndex) +return EvaluateIntegralTypeTrait(*this, Kind, KWLoc, Args, RParenLoc, erichkeane wrote: > I realize this is the 1st, but this seems like it

[PATCH] D129951: adds `__disable_adl` attribute

2023-02-06 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added inline comments. Comment at: clang/include/clang/Basic/Attr.td:4132 +def DisableADL : InheritableAttr { + let Spellings = [Keyword<"__disable_adl">]; + let Subjects = SubjectList<[Function]>; rsmith wrote: > Has this syntax been discussed already? If

[PATCH] D135341: [clang] adds `__reference_constructs_from_temporary`

2023-02-07 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. In D135341#4111673 , @ldionne wrote: > I've been looking at implementing `reference_constructs_from_temporary` & > friends and this would be sweet to have. Is this blocked on something > specific? This trait should be ready to go,

[PATCH] D137458: [clang] Add __decay as a builtin template

2023-02-10 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added subscribers: aaron.ballman, cjdb. cjdb added a comment. In D137458#3909343 , @troyj wrote: >> Also, it would be nice to have some numbers for the 'measurably faster' >> claim :) > > Sure. Here's an example of a library change that started usin

[PATCH] D147888: Update declaration message of extern linkage

2023-04-17 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. I think we should fundamentally rethink this entire category of diagnostic. Rather than having a static diagnostic per offence stating //what// happened, we should instead have a single diagnostic that captures both what happened, why it's bad, and how to fix it. Something

[PATCH] D147717: [C++20][NFC] Claim full support for consteval again

2023-04-17 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. I'm gonna get started on this today! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147717/new/ https://reviews.llvm.org/D147717 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D148677: [clang] makes `__is_trivially_equality_comparable` available as a struct

2023-04-18 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb created this revision. cjdb added reviewers: aaron.ballman, erichkeane, shafik, dblaikie. Herald added a project: All. cjdb requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Since this was originally a name in library, it needs an escape

[PATCH] D148677: [clang] makes `__is_trivially_equality_comparable` available as a struct

2023-04-18 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 514798. cjdb added a comment. uploads the right test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148677/new/ https://reviews.llvm.org/D148677 Files: clang/lib/Parse/ParseDeclCXX.cpp clang/test/SemaCXX/libcx

[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

2023-04-18 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. This patch is causing breakages downstream because it didn't have do the struct dance, so I've added D148677 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147175/new/ https://reviews.llvm.o

[PATCH] D148677: [clang] makes `__is_trivially_equality_comparable` available as a struct

2023-04-19 Thread Christopher Di Bella via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG33e21610f9cd: [clang] makes `__is_trivially_equality_comparable` available as a struct (authored by cjdb). Repository: rG LLVM Github Monorepo CH

[PATCH] D148835: [clang] removes trailing whitespace

2023-04-20 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb created this revision. cjdb added reviewers: aaron.ballman, shafik, erichkeane, dblaikie. Herald added subscribers: luke, steakhal, bzcheeseman, kosarev, pmatos, asb, ormris, frasercrmck, jdoerfert, martong, luismarques, apazos, sameer.abuasal, pengfei, s.egerton, Jim, jocewei, PkmX, arphama

[PATCH] D148835: [clang] removes trailing whitespace

2023-04-20 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb abandoned this revision. cjdb added a comment. I've had some very good input about why this probably shouldn't go ahead: git history erasure :') Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148835/new/ https://reviews.llvm.org/D148835 _

[PATCH] D129951: adds `__disable_adl` attribute

2023-04-20 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb updated this revision to Diff 515497. cjdb marked 6 inline comments as done. cjdb added a comment. Herald added subscribers: aheejin, dschuff. - undoes whitespace changes as requested - documents feature I think the only thing left is to address the global new/delete and static member funct

[PATCH] D129951: adds `__disable_adl` attribute

2023-04-20 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5756 + if (FunctionDecl *F = D->getAsFunction(); + F->isOverloadedOperator() || F->isCXXClassMember()) { +S.Diag(AL.getLoc(), diag::err_disable_adl_no_operators) cor3ntin wrote: > a

[PATCH] D107292: [clang] adds warning to alert user when they use alternative tokens in declarations

2023-04-20 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb abandoned this revision. cjdb added a comment. Herald added a project: All. I think there was a clang-tidy patch that handled this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107292/new/ https://reviews.llvm.org/D107292 ___

[PATCH] D135238: [clang] adds copy-constructible type-trait builtins

2023-04-20 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb abandoned this revision. cjdb added a comment. Abandoning, since this doesn't have consensus. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135238/new/ https://reviews.llvm.org/D135238 ___ cfe-commi

[PATCH] D135239: [clang] adds copy-assignable type-trait builtins

2023-04-20 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb abandoned this revision. cjdb added a comment. Abandoning, since this doesn't have consensus. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135239/new/ https://reviews.llvm.org/D135239 ___ cfe-commi

[PATCH] D135240: [clang] adds move-constructible type-trait builtins

2023-04-20 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb abandoned this revision. cjdb added a comment. Abandoning, since this doesn't have consensus. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135240/new/ https://reviews.llvm.org/D135240 ___ cfe-commi

[PATCH] D135338: [clang] adds move-assignable type-trait builtins

2023-04-20 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb abandoned this revision. cjdb added a comment. Abandoning, since this doesn't have consensus. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135338/new/ https://reviews.llvm.org/D135338 ___ cfe-commi

[PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

2023-04-20 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb abandoned this revision. cjdb added a comment. Herald added a subscriber: PiotrZSL. Herald added a project: clang-format. Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay. Abandoning since we're heading down a different design path now. Repository: rG LLVM Githu

[PATCH] D140125: [clang] splits diagnostic message into summary and reason

2023-04-20 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb abandoned this revision. cjdb added a comment. Abandoning since we're going down a different design path now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140125/new/ https://reviews.llvm.org/D140125 _

[PATCH] D141310: [clang] add -Wcompare-function-pointers

2023-04-20 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. @aaron.ballman are you okay with this being merged now, provided that it's off by default? Apologies for letting this one fall through the cracks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141310/new/ https://reviews.llvm

[PATCH] D129835: [clang] adds a discardable attribute

2023-04-21 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. >> With [[discardable]] one just needs to push/pop at the extremes of a file >> (and if a future version of module maps supports global pragmas for a >> module, there too, but that's a discussion that requires a design doc). > > I understood that, I just don't think that's

[PATCH] D148835: [clang] removes trailing whitespace

2023-04-21 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. In D148835#4286924 , @aaron.ballman wrote: > In D148835#4286905 , @erichkeane > wrote: > >> In D148835#4284871 , @cjdb wrote: >> >>> I've had some

[PATCH] D129835: [clang] adds a discardable attribute

2023-04-21 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. Yeah, in retrospect, I agree (and may need to clean up a few iterator types now, grr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129835/new/ https://reviews.llvm.org/D129835 __

[PATCH] D148835: [clang] removes trailing whitespace

2023-04-21 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment. In D148835#4288151 , @erichkeane wrote: > In D148835#4288148 , @cjdb wrote: > >> In D148835#4286924 , >> @aaron.ballman wrote: >> >>> In D148835#42

<    1   2   3   4   5   >