Page MenuHomePhabricator

cjdb (Christopher Di Bella)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 11 2017, 5:18 AM (311 w, 4 d)

Recent Activity

Yesterday

cjdb abandoned D147288: [clang][NFC] updates cxx_status for P2113R0.

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 sleep about it until/unless the committee finds time to clarify the issue.

Fri, Mar 31, 1:39 PM · Restricted Project, Restricted Project
cjdb added a comment to D144522: [clang-tidy] Add readability-operators-representation check.

Thanks so much for seeing this through; I'm unusually looking forward to rebuilding LLVM this weekend!

Fri, Mar 31, 9:48 AM · Restricted Project, Restricted Project

Thu, Mar 30

cjdb added a comment to D147288: [clang][NFC] updates cxx_status for P2113R0.

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.

Thu, Mar 30, 6:05 PM · Restricted Project, Restricted Project
cjdb requested review of D147288: [clang][NFC] updates cxx_status for P2113R0.
Thu, Mar 30, 5:57 PM · Restricted Project, Restricted Project

Tue, Mar 28

cjdb added a comment to D146358: [clang][AST] Print name instead of type when diagnosing uninitialized subobject in constexpr variables.

"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 likes subobject of type 'T', but it's very wordy, and potentially not useful additional info. I'm partial to subobject 'T' due to it being the thing we're more likely to say in conversation (e.g. "that int isn't initialised"). I've also put out a mini-survey on the #include <C++> Discord server, and will update in a few hours.

Hi, @cjdb thanks for your feedback and the mini-survey.
The objective of this patch is to print the subobject's name instead of its type when it is not initialized in constexpr variable initializations. Thus, I think it would be appropriate to add some more options to the survey like "subobject named 'foo' is not initialized" and "subobject 'foo' is not initialized" when the code looks like the following:

template <typename T>
struct F {
	T foo;
	constexpr F(){}
};
constexpr F <int>f;
Tue, Mar 28, 5:48 PM · Restricted Project, Restricted Project
cjdb added a comment to D146358: [clang][AST] Print name instead of type when diagnosing uninitialized subobject in constexpr variables.

"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.

Tue, Mar 28, 9:58 AM · Restricted Project, Restricted Project

Thu, Mar 23

cjdb added inline comments to D146675: [libc++] Warn on including headers that are deprecated in C++17.
Thu, Mar 23, 12:43 PM · Restricted Project, Restricted Project
cjdb added a comment to D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`.

swaps commits to see if that fixes CI (part 1)

Oh, if this is just stacked patches confusing our precommit CI, then it's okay. I hadn't realized this was a stacked patch.

Thu, Mar 23, 8:58 AM · Restricted Project, Restricted Project

Wed, Mar 22

cjdb added inline comments to D146654: [clang] replaces numeric SARIF ids with heirarchical names.
Wed, Mar 22, 5:52 PM · Restricted Project, Restricted Project
cjdb accepted D146675: [libc++] Warn on including headers that are deprecated in C++17.

LGTM from a diagnostic perspective!

Wed, Mar 22, 5:43 PM · Restricted Project, Restricted Project
cjdb added inline comments to D146675: [libc++] Warn on including headers that are deprecated in C++17.
Wed, Mar 22, 5:24 PM · Restricted Project, Restricted Project
cjdb updated the diff for D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`.

And on this CL's side too

Wed, Mar 22, 2:30 PM · Restricted Project, Restricted Project
cjdb updated the diff for D146654: [clang] replaces numeric SARIF ids with heirarchical names.

I think this corrects everything.

Wed, Mar 22, 2:29 PM · Restricted Project, Restricted Project
cjdb updated the summary of D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`.
Wed, Mar 22, 2:26 PM · Restricted Project, Restricted Project
cjdb updated the summary of D146654: [clang] replaces numeric SARIF ids with heirarchical names.
Wed, Mar 22, 2:25 PM · Restricted Project, Restricted Project
cjdb updated the diff for D146654: [clang] replaces numeric SARIF ids with heirarchical names.

rebasing continues

Wed, Mar 22, 2:14 PM · Restricted Project, Restricted Project
cjdb updated the diff for D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`.

swaps commits to see if that fixes CI (part 1)

Wed, Mar 22, 2:10 PM · Restricted Project, Restricted Project
cjdb updated the diff for D146654: [clang] replaces numeric SARIF ids with heirarchical names.

swaps commits to see if that fixes CI

Wed, Mar 22, 2:08 PM · Restricted Project, Restricted Project
cjdb requested review of D146654: [clang] replaces numeric SARIF ids with heirarchical names.
Wed, Mar 22, 11:46 AM · Restricted Project, Restricted Project
cjdb added inline comments to D146376: Update static_assert message for redundant cases .
Wed, Mar 22, 10:03 AM · Restricted Project, Restricted Project, Restricted Project

Tue, Mar 21

cjdb updated the diff for D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`.

fixes breakage and adds FIXME

Tue, Mar 21, 3:01 PM · Restricted Project, Restricted Project
cjdb added inline comments to D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`.
Tue, Mar 21, 10:12 AM · Restricted Project, Restricted Project

Mon, Mar 20

cjdb added a comment to D146376: Update static_assert message for redundant cases .

Thanks for working on this, it's much appreciated!

Mon, Mar 20, 10:44 AM · Restricted Project, Restricted Project, Restricted Project

Mon, Mar 13

cjdb added inline comments to D145284: WIP [clang] adds capabilities for SARIF to be written to file.
Mon, Mar 13, 3:55 PM · Restricted Project, Restricted Project, Restricted Project
cjdb added a comment to D145982: [libc++] Implement std::gcd using the binary version.

Thanks for working on this. I haven't contributed to libc++ in a very long time, so the project's policies may be different to what I'm asking for. Having said that, the following things would be good to do:

Mon, Mar 13, 3:33 PM · Restricted Project
cjdb added a comment to D145856: [clang-tidy] Let misc-const-correctness detect auto local variables that can be made const.

Would you be able to update the commit message to include a description that explains why this commit is necessary please?

Mon, Mar 13, 10:12 AM · Restricted Project, Restricted Project

Fri, Mar 10

cjdb accepted D145793: [clang][AST] Improve diagnostic for `nullptr` constexpr function pointer call.

LGTM pending other reviewers' commentary. Thanks for working on this!

Fri, Mar 10, 5:22 PM · Restricted Project, Restricted Project
cjdb abandoned D145439: PLEASE DO NOT COMMENT ON THIS PATCH.
Fri, Mar 10, 5:20 PM · Restricted Project, Restricted Project
cjdb added a comment to D145439: PLEASE DO NOT COMMENT ON THIS PATCH.

D145438 was merged into D145284.

Fri, Mar 10, 5:20 PM · Restricted Project, Restricted Project
cjdb abandoned D145438: PLEASE DO NOT COMMENT ON THIS PATCH.

Merged into D145284.

Fri, Mar 10, 5:19 PM · Restricted Project, Restricted Project
cjdb updated the diff for D145284: WIP [clang] adds capabilities for SARIF to be written to file.

merges in D145438. This patch is ready for review now.

Fri, Mar 10, 5:19 PM · Restricted Project, Restricted Project, Restricted Project
cjdb added inline comments to D145376: [libc++] add declval failure assertion for instantiation.
Fri, Mar 10, 12:22 PM · Restricted Project, Restricted Project

Wed, Mar 8

cjdb added a comment to D129951: adds `__disable_adl` attribute.

I'm deeply disappointed that libc++ moved away from using __function_like: that was an important part of preventing niebloid misuse. It isn't conforming to treat niebloids as function objects, which is what __function_like prevented (I just checked std::ranges::next and it seems that __function_like was completely removed).

Oops, it seems like my reply got into a race condition with yours. I am curious though, why is it non-conforming to treat niebloids as function objects? It is certainly more lax than required by the Standard, however we are allowed to make those copyable, aren't we? Does the Standard say that we *have* to make them non-usable as objects? That would change things for sure (not with this patch, but we'd probably want to try re-introducing __function_like, and we'd file a bug report against libstdc++ to do the same).

Wed, Mar 8, 3:43 PM · Restricted Project, Restricted Project
cjdb added a comment to D129951: adds `__disable_adl` attribute.

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 to add it to both).

I haven't had a lot of time to drive this in Clang, let alone GCC. Even if libc++ can't ultimately use it (which would be sad), there are other libraries that can. For example, Abseil has a similar attitude towards functions as Niebloids, and could wrap it behind a macro.

Abseil has the same support problem though AFAICT. In fact, most open source libraries don't just support clang.

Wed, Mar 8, 2:54 PM · Restricted Project, Restricted Project
cjdb added a comment to D129951: adds `__disable_adl` attribute.

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 to add it to both).

Wed, Mar 8, 11:08 AM · Restricted Project, Restricted Project

Mon, Mar 6

cjdb requested review of D145439: PLEASE DO NOT COMMENT ON THIS PATCH.
Mon, Mar 6, 3:17 PM · Restricted Project, Restricted Project
cjdb requested review of D145438: PLEASE DO NOT COMMENT ON THIS PATCH.
Mon, Mar 6, 3:17 PM · Restricted Project, Restricted Project

Fri, Mar 3

cjdb updated the diff for D145284: WIP [clang] adds capabilities for SARIF to be written to file.

tidies up some stuff that I overlooked

Fri, Mar 3, 4:24 PM · Restricted Project, Restricted Project, Restricted Project
cjdb updated subscribers of D145284: WIP [clang] adds capabilities for SARIF to be written to file.
Fri, Mar 3, 4:21 PM · Restricted Project, Restricted Project, Restricted Project
cjdb updated the diff for D145284: WIP [clang] adds capabilities for SARIF to be written to file.

adds dependency

Fri, Mar 3, 4:14 PM · Restricted Project, Restricted Project, Restricted Project
cjdb requested review of D145284: WIP [clang] adds capabilities for SARIF to be written to file.
Fri, Mar 3, 4:13 PM · Restricted Project, Restricted Project, Restricted Project
cjdb updated the diff for D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`.

fixes string goof

Fri, Mar 3, 10:58 AM · Restricted Project, Restricted Project
cjdb added a comment to D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`.
Fri, Mar 3, 10:56 AM · Restricted Project, Restricted Project
cjdb updated the diff for D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`.

sorts artifacts so that they're output in index order

Fri, Mar 3, 10:55 AM · Restricted Project, Restricted Project
cjdb added a comment to D145178: [clang][NFC] reformats the SARIF diagnostic test so it's human readable.

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).

Fri, Mar 3, 10:32 AM · Restricted Project, Restricted Project

Thu, Mar 2

cjdb updated the diff for D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`.

fixes something that wasn't supposed to be changed in the rebase

Thu, Mar 2, 4:15 PM · Restricted Project, Restricted Project
cjdb updated the diff for D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`.

actually commits the files

Thu, Mar 2, 4:12 PM · Restricted Project, Restricted Project
cjdb updated the diff for D145178: [clang][NFC] reformats the SARIF diagnostic test so it's human readable.

removes something from a future commit

Thu, Mar 2, 4:10 PM · Restricted Project, Restricted Project
cjdb updated subscribers of D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`.
Thu, Mar 2, 3:25 PM · Restricted Project, Restricted Project
cjdb updated subscribers of D145178: [clang][NFC] reformats the SARIF diagnostic test so it's human readable.
Thu, Mar 2, 3:25 PM · Restricted Project, Restricted Project
cjdb requested review of D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`.
Thu, Mar 2, 3:16 PM · Restricted Project, Restricted Project
cjdb updated the diff for D145178: [clang][NFC] reformats the SARIF diagnostic test so it's human readable.

removes redundant line

Thu, Mar 2, 2:09 PM · Restricted Project, Restricted Project

Mar 2 2023

cjdb updated the diff for D145178: [clang][NFC] reformats the SARIF diagnostic test so it's human readable.

renames check identifiers

Mar 2 2023, 1:00 PM · Restricted Project, Restricted Project
cjdb added inline comments to D145178: [clang][NFC] reformats the SARIF diagnostic test so it's human readable.
Mar 2 2023, 12:47 PM · Restricted Project, Restricted Project
cjdb added inline comments to D145178: [clang][NFC] reformats the SARIF diagnostic test so it's human readable.
Mar 2 2023, 12:41 PM · Restricted Project, Restricted Project
cjdb requested review of D145178: [clang][NFC] reformats the SARIF diagnostic test so it's human readable.
Mar 2 2023, 12:38 PM · Restricted Project, Restricted Project

Feb 10 2023

cjdb updated subscribers of D137458: [clang] Add __decay<T> as a builtin template.

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 using builtins for __make_integer_seq and __type_pack_element https://github.com/facebook/fatal/commit/58102a3f7e66ad122d7d3335c446399b09d5085e where there was a 1.8% speedup for a file that spent 70 seconds in the front end. It also reduced the peak memory usage of the front end by 0.5%. We have reason to believe that a __decay builtin would produce similar benefit. Unfortunately, all of the intrinsics in https://reviews.llvm.org/D116203 were implemented with parentheses syntax instead of as builtin templates, which makes it more difficult. Ideally, we'd like to see all of these implemented as builtin templates, but __decay is the first one that we're proposing.

I'm not familiar with the mangling issue that you mentioned. I'll look into it more.

Feb 10 2023, 5:09 PM · Restricted Project, Restricted Project

Feb 7 2023

cjdb added a comment to D135341: [clang] adds `__reference_constructs_from_temporary`.

I've been looking at implementing reference_constructs_from_temporary & friends and this would be sweet to have. Is this blocked on something specific?

Feb 7 2023, 8:26 PM · Restricted Project, Restricted Project

Feb 6 2023

cjdb added inline comments to D129951: adds `__disable_adl` attribute.
Feb 6 2023, 3:38 PM · Restricted Project, Restricted Project
cjdb added a comment to D129951: adds `__disable_adl` attribute.

Ping @aaron.ballman

Feb 6 2023, 10:22 AM · Restricted Project, Restricted Project

Jan 30 2023

cjdb added a comment to D129951: adds `__disable_adl` attribute.

Ping @aaron.ballman @rsmith

Jan 30 2023, 3:09 PM · Restricted Project, Restricted Project

Jan 25 2023

cjdb added inline comments to D129951: adds `__disable_adl` attribute.
Jan 25 2023, 4:57 PM · Restricted Project, Restricted Project
cjdb updated the diff for D129951: adds `__disable_adl` attribute.

responds to feedback

Jan 25 2023, 4:56 PM · Restricted Project, Restricted Project
cjdb added inline comments to D129951: adds `__disable_adl` attribute.
Jan 25 2023, 12:48 PM · Restricted Project, Restricted Project
cjdb updated the diff for D129951: adds `__disable_adl` attribute.
  • updates patch so that it meets what Aaron and I want, also meets most of Richard's feedback
  • updates commit message
Jan 25 2023, 12:45 PM · Restricted Project, Restricted Project
cjdb committed rG716469b6139a: [clang-tidy] Fix segfault in bugprone-standalone-empty (authored by denik).
[clang-tidy] Fix segfault in bugprone-standalone-empty
Jan 25 2023, 12:07 PM · Restricted Project, Restricted Project
cjdb closed D142423: [clang-tidy] Fix segfault in bugprone-standalone-empty.
Jan 25 2023, 12:07 PM · Restricted Project, Restricted Project
cjdb added a comment to D141836: [AArch64] Disable __muloti4 libcalls for AArch64.

I would hypothesize that the number of platforms where compiler-rt is used to link aarch64 code is greater than the number that don't and there's work to make linking with compiler-rt (https://github.com/llvm/llvm-project/tree/main/llvm-libgcc) in order to replace uses on other platforms. In addition, while my employment meant I couldn't add this to libgcc in the past there's no reason why it couldn't be added as well. Has anyone tried?

Jan 25 2023, 11:54 AM · Restricted Project, Restricted Project

Jan 24 2023

cjdb accepted D142423: [clang-tidy] Fix segfault in bugprone-standalone-empty.
Jan 24 2023, 3:30 PM · Restricted Project, Restricted Project

Jan 12 2023

cjdb committed rG7910ee7d8c6d: [clang-tidy] don't warn when returning the result for bugprone-standalone-empty (authored by v1nh1shungry).
[clang-tidy] don't warn when returning the result for bugprone-standalone-empty
Jan 12 2023, 5:16 PM · Restricted Project, Restricted Project
cjdb closed D141107: [clang-tidy] don't warn when returning the result for bugprone-standalone-empty.
Jan 12 2023, 5:15 PM · Restricted Project, Restricted Project
cjdb committed rG8effeb44f4a8: Add -Wreturn-local-addr, GCC alias for -Wreturn-stack-address (authored by adriandole).
Add -Wreturn-local-addr, GCC alias for -Wreturn-stack-address
Jan 12 2023, 10:26 AM · Restricted Project, Restricted Project
cjdb closed D139570: Add -Wreturn-local-addr, GCC alias for -Wreturn-stack-address.
Jan 12 2023, 10:25 AM · Restricted Project, Restricted Project

Jan 11 2023

cjdb added inline comments to D141310: [clang] add -Wcompare-function-pointers.
Jan 11 2023, 10:31 AM · Restricted Project, Restricted Project

Jan 10 2023

cjdb added inline comments to D141310: [clang] add -Wcompare-function-pointers.
Jan 10 2023, 4:59 PM · Restricted Project, Restricted Project
cjdb added a comment to D141310: [clang] add -Wcompare-function-pointers.

Thanks for working on this! Needs a bit of work, but we should be able to get this in very soon methinks.

Jan 10 2023, 4:58 PM · Restricted Project, Restricted Project
cjdb accepted D141107: [clang-tidy] don't warn when returning the result for bugprone-standalone-empty.

Excellent, let's wait for @denik's feedback before merging, but this LGTM. Thank you for the patch!

Jan 10 2023, 9:44 AM · Restricted Project, Restricted Project

Jan 6 2023

cjdb added a comment to D141107: [clang-tidy] don't warn when returning the result for bugprone-standalone-empty.

LGTM, thanks for fixing!

Jan 6 2023, 10:48 AM · Restricted Project, Restricted Project
cjdb added a reviewer for D141107: [clang-tidy] don't warn when returning the result for bugprone-standalone-empty: denik.
Jan 6 2023, 10:43 AM · Restricted Project, Restricted Project

Dec 15 2022

cjdb requested review of D140125: [clang] splits diagnostic message into summary and reason.
Dec 15 2022, 10:52 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
cjdb added a comment to D128372: [Clang-Tidy] Empty Check.

Ironically as an aside... looking back at the review because of the discource article.

The whole reason why I added the [[nodiscard]] checker to clang-tidy 4 years ago was to cover exactly this use case...and many others like it where users have a function which returns a boolean and takes no arguments are pointer or reference (empty() included)

https://reviews.llvm.org/D55433

Shouldn't people be using this checker to add nodiscard and let the compiler to the heavy lifting. Was there a use case as to why that solution doesn't solve this?

Dec 15 2022, 6:04 AM · Restricted Project, Restricted Project

Dec 9 2022

cjdb committed rGec3f8feddf81: [Clang-Tidy] Empty Check (authored by abrahamcd).
[Clang-Tidy] Empty Check
Dec 9 2022, 3:21 PM · Restricted Project, Restricted Project
cjdb closed D128372: [Clang-Tidy] Empty Check.
Dec 9 2022, 3:21 PM · Restricted Project, Restricted Project

Dec 7 2022

cjdb accepted D139570: Add -Wreturn-local-addr, GCC alias for -Wreturn-stack-address.

LGTM

Dec 7 2022, 2:12 PM · Restricted Project, Restricted Project

Dec 5 2022

cjdb added a reviewer for D138939: [WIP][clang] adds a way to provide user-oriented reasons: vaibhav.y.
Dec 5 2022, 10:04 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project

Dec 2 2022

cjdb added a comment to D138939: [WIP][clang] adds a way to provide user-oriented reasons.

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 parameter. Then you can decide whether to print legacy or user-oriented reasons.

If next year you invent another diagnostic, you can extend the enum and the FormatDiagnostic function.

Dec 2 2022, 4:44 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project
cjdb added a comment to D138939: [WIP][clang] adds a way to provide user-oriented reasons.

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 mode for users/testing / A/B.

Dec 2 2022, 9:55 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project
cjdb added a comment to D138939: [WIP][clang] adds a way to provide user-oriented reasons.

Then Sarif was a distraction. Still to reduce boilerplate and for A/B testing:

enum class DiagnosticMode {
  Legacy,
  UserOriented,
  Default = Legacy
}
Dec 2 2022, 9:28 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project

Dec 1 2022

cjdb added a comment to D138939: [WIP][clang] adds a way to provide user-oriented reasons.

Maybe void FormatDiagnostic(SmallVectorImpl<char> &OutStr, DiagnosticMode mode)instead of void FormatDiagnostic(SmallVectorImpl<char> &OutStr)?
To make the transition easer and future proof.

I like this idea. Perhaps with DiagnosticMode being a 3-way enum:

enum struct DiagnosticMode {
  Legacy,
  Sarif,  
  Default = Legacy
}

I like the idea in particular, since it makes a command line flag to modify "Default" to be whichever the user prefers pretty trivial.

There's already a flag for this: -fdiagnostics-format=sarif. Why do we need a second diagnostic mode flag?

Ah, oh... is the Sarif formatting being done with a new formatter? That seems unfortunate, since folks using the other formatters won't be able to use the user friendly formats.

I've been alerted offline that I am misunderstanding the Sarif proposal, and where this is going. I'll note that I wasn't present/invited at the calls where all of this was discussed, so I am admittedly not completely up to date. The above concern shouldn't stop others from reviewing this, particularly if you better understand the intent here.

Dec 1 2022, 6:06 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project
cjdb updated the diff for D138939: [WIP][clang] adds a way to provide user-oriented reasons.

responds to feedback

Dec 1 2022, 6:06 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project
cjdb added a comment to D138939: [WIP][clang] adds a way to provide user-oriented reasons.

Maybe void FormatDiagnostic(SmallVectorImpl<char> &OutStr, DiagnosticMode mode)instead of void FormatDiagnostic(SmallVectorImpl<char> &OutStr)?
To make the transition easer and future proof.

I like this idea. Perhaps with DiagnosticMode being a 3-way enum:

enum struct DiagnosticMode {
  Legacy,
  Sarif,  
  Default = Legacy
}

I like the idea in particular, since it makes a command line flag to modify "Default" to be whichever the user prefers pretty trivial.

Dec 1 2022, 11:10 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project

Nov 29 2022

cjdb added a comment to D138939: [WIP][clang] adds a way to provide user-oriented reasons.

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?

Nov 29 2022, 1:47 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project
cjdb added a comment to D138939: [WIP][clang] adds a way to provide user-oriented reasons.

The clang-side interface to this seems a touch clunky, and I fear it'll make continuing its use/generalizing its use less likely.

Nov 29 2022, 1:34 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project
cjdb added a comment to D138939: [WIP][clang] adds a way to provide user-oriented reasons.

I don't understand why test_demangle.pass.cpp was considered too big to upload. Here's the diff:

Nov 29 2022, 1:24 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project
cjdb added a comment to D128372: [Clang-Tidy] Empty Check.

@njames93 if you don't have any further concerns, I am going to merge this on Friday afternoon, as it will have been four months since there has been a maintainer's input.

Nov 29 2022, 12:31 PM · Restricted Project, Restricted Project
cjdb requested review of D138939: [WIP][clang] adds a way to provide user-oriented reasons.
Nov 29 2022, 12:21 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project

Nov 28 2022

cjdb added inline comments to D138603: [Clang] Implement LWG3823 for __is_aggregate.
Nov 28 2022, 11:20 AM · Restricted Project, Restricted Project
cjdb added inline comments to D138189: [libcxx] adds an include-what-you-use (IWYU) mapping file.
Nov 28 2022, 9:24 AM · Restricted Project, Restricted Project
cjdb accepted D138603: [Clang] Implement LWG3823 for __is_aggregate.

LGTM. Thanks for fixing this!

Nov 28 2022, 9:21 AM · Restricted Project, Restricted Project

Nov 21 2022

cjdb committed rGab4664808281: [libcxx] adds an include-what-you-use (IWYU) mapping file (authored by cjdb).
[libcxx] adds an include-what-you-use (IWYU) mapping file
Nov 21 2022, 5:21 PM · Restricted Project, Restricted Project