User Details
- User Since
- Jun 28 2016, 7:50 PM (307 w, 4 d)
Tue, May 17
Sun, May 15
Nice, thank you for fixing this!
I'm excited to see this, thank you or working on it. +1 with the changes aman suggested.
Sat, May 14
Fri, May 13
Cool, great catch!
nice cleanup!
Ok, awesome!
This should wait a week or two and please send a heads up about the deprecation to llvm forums
This is great in concept but I think we have macros to deprecate things in compiler.h, please also make sure all in tree uses are removed before landing this. Are there any APInt docs or comments that need to be updated? Are you keeping APSInt in sync as well?
Nice, this makes things much simpler and defines away a class of bugs
Very cool, if there is no good reviewer for this, I'd say "land it; advertise it; react to feedback from users" :-)
I'm not a competent reviewer for this patch, but I love tblgen getting better tooling :-)
Thu, May 12
This should be part of https://reviews.llvm.org/D125447, weird.
Thank you for the review!
Wed, May 11
I'm not a competent reviewer for this, Richard can you recommend someone?
Thx for the reviews!
Pair programmed with @bollu
Tue, May 10
This looks good to me, the community discussion seems quiet. Feel free to land when the CI tests pass, thanks!
Use fancy stringref trick; ignore comments in lines.
Mon, May 9
nice, this is a more logical API
nice, congrats on your first patch!
Wed, May 4
Cool!
I don't think it should exist in core if it only has one user. In fact, I'd like to see the AsmParser stuff significantly refactored to be more modular and orthogonal. Most of this stuff exists because we didn't have parseCommaSeparatedList which defines away the majority of the utility of these macro helpers in the first place. See e.g. this patch:
https://reviews.llvm.org/D124791
Mon, May 2
Nice, thank you! A couple more suggestions above.
Hello, why were parseOptionalAssignmentListWithTypesand parseAssignmentListWithTypes removed?
Fri, Apr 29
Flip the default on allowType/allowAttr
Yep, you're right, it is also much less common to have attributes, and no one wants attributes without a type. I'll flip it, thanks for the suggestion!
Thu, Apr 28
Two other minor comment changes I missed from River's review.
Incorporate improvements from River, still need to confirm sense of the bools in parseArgument
Update flang
Remove now-constant argument.
This also fixes the source location parsing bug that @mehdi_amini mentioned a couple days ago.
thx for the review!
River and I discussed this offline, I'm going to land this and build on it. Thx!
Wed, Apr 27
Overall, this looks really great to me. Please get River's review, then (when any feedback is incorporated) ping an LLVM forum for visibility. This isn't normally needed but casting.h is very pervasive, so extra discussion is helpful.
Tue, Apr 26
This seems reasonable to me
Update flang
Sweet, I'll get on this in a few minutes, thx!
Mon, Apr 25
Fri, Apr 22
nice, this is super exciting!
Fantastic, thank you!
Thank you for taking this on Saleem!
Apr 21 2022
Thank you for the quick review!
Add missing blank line
Nice, thank you!
We discussed this in some other patch that I lost track of, this was just something on my deep todo list.
Apr 19 2022
Nice, LGTM!
Also, when this lands, we should post on the forum about it. Every change to the developer policy warrants broader visibility than just a phab discussion IMO.
This is awesome, I agree completely we should curate release notes better. That said, I think this should make it more clear that there is a "difference in kind" between user-facing tools like clang/lldb etc and other libraries in LLVM. We don't want release note burden (or noise) for every little thing going into the optimizer or codegen. Do you think it would make sense to point out that this is about user-facing tools?
Apr 18 2022
nice! Very excited this "just works" instead of being another knob for experts