[ReviewRequest] [Sema/Parser] GCC Compatibility Patch: Support for __final specifier

Hi all, my first potential-contribution, so I apologize if I'm submitting this improperly, I'm unfamiliar with the 'type' keys that you use in the topic, so hopefully I have this right. As reported in bug 28473, GCC supports 'final' functionality in pre-C++11 code using the __final keyword.

RE: [PATCH] D22970: Ensure Ident_GNU_final is properly initialized in the Parser Initialize function

I don't have the ability to commit this, but if it is blocking someone in some way (since the build verifier would otherwise be broken), someone committing it before then would be much appreciated. Thanks! -Erich -Original Message- From: Andrey Bokhanko

RE: r277134 - [GCC] Support for __final specifier

Fixed in: r277206 after review in https://reviews.llvm.org/D22970 From: Keane, Erich Sent: Friday, July 29, 2016 11:58 AM To: 'Andrey Bokhanko' ; Mike Aizatsky Cc: cfe-commits Subject: RE: r277134 - [GCC] Support for

RE: [ReviewRequest] [Sema/Parser] GCC Compatibility Patch: Support for __final specifier

equest] [Sema/Parser] GCC Compatibility Patch: Support for __final specifier I'd rename VS_Alt_Final to VS_GNU_Final. On Mon, Jul 25, 2016 at 2:24 PM, Keane, Erich via cfe-commits <cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote: Hi all, my first potential-

RE: [ReviewRequest] [Sema/Parser] GCC Compatibility Patch: Support for __final specifier

ts@lists.llvm.org<mailto:cfe-commits@lists.llvm.org> Subject: Re: [ReviewRequest] [Sema/Parser] GCC Compatibility Patch: Support for __final specifier I'd rename VS_Alt_Final to VS_GNU_Final. On Mon, Jul 25, 2016 at 2:24 PM, Keane, Erich via cfe-commits <cfe-commits@lists.llvm.org<mailto:cfe-

RE: r277134 - [GCC] Support for __final specifier

Sure, looking into it now. From: Andrey Bokhanko [mailto:andreybokha...@gmail.com] Sent: Friday, July 29, 2016 11:57 AM To: Mike Aizatsky ; Keane, Erich Cc: cfe-commits Subject: Re: r277134 - [GCC] Support for __final

RE: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

I cannot personally right now, though I will dig into OpenMP implementation to see how intentional that is. The size is seemingly determined by the OMP code generation as far as I can tell. From: David Blaikie [mailto:dblai...@gmail.com] Sent: Friday, October 7, 2016 11:51 AM To:

RE: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

Ok, I dug into this deeper. ASTContext.cpp:2811 (getVariableArrayDecayedType) intentionaly sets size to nullptr in this case for the purpose of turning it into a [*] type. OpenMP.cpp:236 (CodeGenFunction::GenerateOpenMPCapturedStmtFunction) calls this to replace variably modified type with

RE: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

Added Alexey to the list, he’s the OMP Maintainer, so hopefully he knows better ☺ From: David Blaikie [mailto:dblai...@gmail.com] Sent: Friday, October 7, 2016 11:51 AM To: reviews+d25373+public+d8ec2a4bb41b1...@reviews.llvm.org; Keane, Erich ; cfe-commits@lists.llvm.org;

RE: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

This causes a massive number of openmp test failures (obviously), and I'm not terribly comfortable with how OpenMP works. I can update the tests (and will), but would like it if you could be extra diligent in confirming the correct behavior for me. Failing Tests (6): Clang ::

RE: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

That does turn ArgType into a PointerType int*. However, line 301 (GenerateOpenMPCapturedStmtFunction) attempts to cast it to a reference type a bit later, resulting in a SIGABRT there. Do I need to re-add the reference there? Is removing the array type REALLY what we wish to do?

RE: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

I did. I changed line 236 to "ArgType = getContext().getCanonicalParamType(ArgType);" (as I believe you were suggesting), and the output was identical when doing dump on ArgType: This: LValueReferenceType 0xa451620 'int (&)[*]' variably_modified `-VariableArrayType 0xa4515e0 'int [*]'

RE: [PATCH] D25373: Fix for Bug 30639: CGDebugInfo Null dereference with OpenMP array access

I don't see any .ll files for the tests. I see the directory build/tools/clang/test/OpenMP/Output has a bunch of .script and .tmp(binary files), but no obvious .ll files. -Original Message- From: Alexey Bataev [mailto:a.bat...@hotmail.com] Sent: Monday, October 10, 2016 1:22 PM To:

RE: r298410 - Correct class-template deprecation behavior

Ended up being simpler than I expected. I added -r298433 To fix this. The issue is that the test had a conditional based on the "C" version, so I explicitly added a test for C89 and C99, which the PS4 is apparently not defaulting to. Thanks! -Erich -Original Message- From: Yung,

RE: r298410 - Correct class-template deprecation behavior

I saw that, I was digging into it, then went to lunch. I'm hoping to push a fix by EOD if I can. Thanks for the heads up though! -Original Message- From: Yung, Douglas [mailto:douglas.y...@sony.com] Sent: Tuesday, March 21, 2017 12:59 PM To: Keane, Erich Cc:

RE: r306149 - Emit warning when throw exception in destruct or dealloc functions which has a

Hi Richard: I’m not sure if you noticed this, but my coworker did submit a review here: https://reviews.llvm.org/D34671 with the changes you suggested. Thanks, Erich From: meta...@gmail.com [mailto:meta...@gmail.com] On Behalf Of Richard Smith Sent: Monday, June 26, 2017 2:25 PM To: Keane,

RE: r305087 - support operator keywords used in Windows SDK

Should be fixed as of here: https://reviews.llvm.org/rL305128 . Curious that none of the other tests found this, but thankfully UBSan did! Committed as no-wait due to the breakage and not wanting to leave it broken over the weekend. -Original Message- From: Evgenii Stepanov

RE: r305087 - support operator keywords used in Windows SDK

Thanks, I didn't notice that one. Looking into it. -Erich -Original Message- From: Evgenii Stepanov [mailto:eugeni.stepa...@gmail.com] Sent: Friday, June 9, 2017 3:00 PM To: Keane, Erich Cc: cfe-commits Subject: Re: r305087 - support

RE: r306149 - Emit warning when throw exception in destruct or dealloc functions which has a

Sorry Richard, I thought I gave you enough time before committing this for Jen. Adding her so she can respond to your comments and fix these. Thanks, Erich From: meta...@gmail.com [mailto:meta...@gmail.com] On Behalf Of Richard Smith Sent: Monday, June 26, 2017 2:25 PM To: Keane, Erich

RE: [PATCH] D32046: [Preprocessor]Correct Macro-Arg allocation of StringifiedArguments, correct getNumArguments

Thanks for the heads up! Fixed in 305491. Turns out a function that AllocateMacroInfo returns a pointer, but the PP still owns it. MacroArgs::create returns a pointer, and expects the user to delete it. I added a unique_ptr with a custom delete to call the macro-args 'destroy' function.

RE: r305425 - [Preprocessor]Correct Macro-Arg allocation of StringifiedArguments,

Ah, right… This function mallocs, and is usually tossed in the Preprocessor Cache, but I’m doing an end-run around that this time. I’ll see if it is better to put it into the PP Cache, or just call ‘free’ on it. Thanks! -Erich From: Kostya Serebryany [mailto:k...@google.com] Sent: Thursday,

RE: r303798 - For Microsoft compatibility, set fno_operator_names

How does chromium compiler in CL? It seems that it would have a similar problem here… From: tha...@google.com [mailto:tha...@google.com] On Behalf Of Nico Weber Sent: Thursday, May 25, 2017 9:16 AM To: Blower, Melanie Cc: rnk ; Keane, Erich

RE: r303798 - For Microsoft compatibility, set fno_operator_names

Patch and Buildbot fixes all reverted as of –r303882. Sorry for the thrash. From: tha...@google.com [mailto:tha...@google.com] On Behalf Of Nico Weber Sent: Thursday, May 25, 2017 9:16 AM To: Blower, Melanie Cc: rnk ; Keane, Erich

RE: r303798 - For Microsoft compatibility, set fno_operator_names

No problem, I definitely think it was the right choice. A change that contains all of what Melanie’s original patch did, plus the Header changes (plus the clang-tidy fixes that came out of this) would be acceptable? Also, I believe she’s working on the warning as well. We were discussing it,

RE: r313907 - Suppress Wsign-conversion for enums with matching underlying type

Fixed in 313909. -Original Message- From: Friedman, Eli [mailto:efrie...@codeaurora.org] Sent: Thursday, September 21, 2017 1:14 PM To: Keane, Erich ; cfe-commits@lists.llvm.org Subject: Re: r313907 - Suppress Wsign-conversion for enums with matching underlying

RE: r313907 - Suppress Wsign-conversion for enums with matching underlying type

Ugg... good catch, thanks. -Original Message- From: Friedman, Eli [mailto:efrie...@codeaurora.org] Sent: Thursday, September 21, 2017 1:14 PM To: Keane, Erich ; cfe-commits@lists.llvm.org Subject: Re: r313907 - Suppress Wsign-conversion for enums with matching

RE: r312542 - [Preprocessor] Correct internal token parsing of newline characters in CRLF

There’s an existing test in r311683. I’d initially implemented handling ‘\r\n’ as well as ‘\n\r’, but Reid had suggested that I simply handle the former. I’d inadvertently contributed the previous revision. I noticed this morning that I’d done so, so I corrected my commit. From:

RE: r314235 - Allow IUnknown/IInterface types to come from extern C++

Ah! Sorry, I didn't realize that isExternCXXContext actually searched the entire search-set! Followup patch coming Thanks for the heads up! -Original Message- From: Friedman, Eli [mailto:efrie...@codeaurora.org] Sent: Tuesday, September 26, 2017 12:06 PM To: Keane, Erich

RE: r314235 - Allow IUnknown/IInterface types to come from extern C++

BTW: The guy who told me about this issue ALSO discovered a different issue that that I didn't solve due to his reproducer not contining the whole testcase. I've got a patch to fix all of it in his hands right now, so I'll submit this fix with it together. -Erich -Original Message-

RE: r311683 - [Preprocessor] Correct internal token parsing of newline characters in CRLF

Hi Stephen- I’m digging through this, and it seems odd. SVN seems to store it with the CRLF line endings. The Git mirror for some reason is not paying attention to the svn file attribute. However, the .gitattribute file seems to convert it to the CRLF endings (as it should be), but it seems

RE: r311683 - [Preprocessor] Correct internal token parsing of newline characters in CRLF

1 more dataset, Craig has the git mirror working right, but with the public mirror… From: Stephen Hines [mailto:srhi...@google.com] Sent: Thursday, August 24, 2017 3:18 PM To: Keane, Erich Cc: cfe-commits Subject: Re: r311683 - [Preprocessor]

RE: r311683 - [Preprocessor] Correct internal token parsing of newline characters in CRLF

Can you better clarify what went wrong? I included a ‘.gitattriutes’ that matches only the new test that is supposed to keep it as CLRF From: Stephen Hines [mailto:srhi...@google.com] Sent: Thursday, August 24, 2017 3:18 PM To: Keane, Erich Cc: cfe-commits

RE: r311683 - [Preprocessor] Correct internal token parsing of newline characters in CRLF

Alright, final update. Thanks to some fantastic help on #llvm, I believe this is fixed. Stephen: You may have to do some shenanigans to fix your local stuff, but the monorepo and another repo both seem to work. Sorry for this everyone :/ From: Stephen Hines [mailto:srhi...@google.com] Sent:

RE: [PATCH] D36707: [CodeGen]Refactor CpuSupports/CPUIs Builtin Code Gen to better work with "target" implementation

Oops, totally forgot this got approved ☺ Thanks for the reminder! From: Eric Christopher [mailto:echri...@gmail.com] Sent: Friday, September 1, 2017 12:40 PM To: reviews+d36707+public+aa8b48c258736...@reviews.llvm.org; Keane, Erich ; llvm-...@redking.me.uk;

RE: r314262 - Emit section information for extern variables.

Elizabeth gave me a patch, submitted it with a test in r314939 From: Andrews, Elizabeth Sent: Wednesday, October 4, 2017 1:54 PM To: Friedman, Eli ; Keane, Erich ; Nico Weber Cc: cfe-commits

RE: r314262 - Emit section information for extern variables.

I saw that… I don’t see where it prevents the same change from being made in link.h. As I mentioned, there is a solution to make this Warning less aggressive (in the lib/Sema/SemaDecl.cpp changes, add a condition that Old->isUsed() before the warning. I’m wondering if that solves your issue.

RE: r314262 - Emit section information for extern variables.

Elizabeth pointed out to me separately that the “isDeclarationADefinition” check ought to have prevented this from happening, so she’s taking a look I believe. The usage proposal I mentioned below might be handy as well. My fear is that this is a case where the existing functionality in

RE: r314262 - Emit section information for extern variables.

I’ve added Elizabeth, who is the original patch author. Hopefully she can help out here. Additionally, Eli did the original review, so hopefully he can chime in as well. I believe the necessity for this warning came out of the discussion on fixing the section behavior. The issue here is

RE: r314262 - Emit section information for extern variables.

Ah, cool! I didn’t realize that, and that is actually exactly what Elizabeth is looking into now. Elizabeth, if you send me a diff, I’ll commit it as review-after-commit if Eli is alright with this. It should be something like changing: + if (VD->isThisDeclarationADefinition() !=

RE: r320489 - Fix ICE when __has_unqiue_object_representations called with invalid decl

Gah, no it wasn’t. I’ll revert that now. From: Richard Smith [mailto:rich...@metafoo.co.uk] Sent: Tuesday, December 12, 2017 8:07 AM To: Keane, Erich Cc: cfe-commits Subject: Re: r320489 - Fix ICE when __has_unqiue_object_representations

RE: r320489 - Fix ICE when __has_unqiue_object_representations called with invalid decl

Fixed in 320493. Thanks for catching that. From: Richard Smith [mailto:rich...@metafoo.co.uk] Sent: Tuesday, December 12, 2017 8:07 AM To: Keane, Erich Cc: cfe-commits Subject: Re: r320489 - Fix ICE when __has_unqiue_object_representations

RE: r316447 - Fix spelling in comment, field is isMsStruct, not Strust

Ah, thanks for letting me know! I’ll do that now. From: tha...@google.com [mailto:tha...@google.com] On Behalf Of Nico Weber Sent: Tuesday, October 24, 2017 8:59 AM To: Keane, Erich Cc: cfe-commits Subject: Re: r316447 - Fix spelling in

RE: r316518 - mplement __has_unique_object_representations

Yikes! Thanks for pointing this out, fixing it now. From: David Majnemer [mailto:david.majne...@gmail.com] Sent: Tuesday, October 24, 2017 2:55 PM To: Keane, Erich Cc: cfe-commits Subject: Re: r316518 - mplement

RE: r331536 - [NFC]Convert Class to use member initialization instead of inline.

I don’t believe the member initialization for bitfields (of which all the ‘0’ values are) happened until C++17, right? I could definitely member initialize the two enum fields though. From: David Blaikie [mailto:dblai...@gmail.com] Sent: Monday, May 7, 2018 12:03 PM To: Keane, Erich

RE: r332470 - Add support for __declspec(code_seg("segname"))

Hi Richard- Thanks for the feedback. I’m copying the author of this patch on this email for further discussion if you care to. We’ll have her take another look at it. Thanks for the revert. (https://github.com/llvm-mirror/clang/commit/746b78de7812bc785fbb5207b788348040b23fa7). -Erich From:

RE: r316518 - mplement __has_unique_object_representations

Thanks for the review Richard. Would you like me to revert, or can I just make these changes in a new patch? From: Richard Smith [mailto:rich...@metafoo.co.uk] Sent: Thursday, October 26, 2017 9:50 AM To: Keane, Erich Cc: cfe-commits

RE: r334650 - Implement constexpr __builtin_*_overflow

Yikes! Thanks for the heads up! I’ll start looking into this. Thanks for letting me know. From: Evgenii Stepanov [mailto:eugeni.stepa...@gmail.com] Sent: Tuesday, July 3, 2018 12:59 PM To: Keane, Erich Cc: cfe-commits Subject: Re: r334650 - Implement constexpr __builtin_*_overflow Hi, with

RE: [PATCH] D41837: Add Function multiversion to the release notes.

It did not, I held it until just after the branch was made. -Original Message- From: hwennb...@google.com [mailto:hwennb...@google.com] On Behalf Of Hans Wennborg Sent: Wednesday, January 17, 2018 4:47 AM To: reviews+d41837+public+36225483e5851...@reviews.llvm.org Cc: Keane, Erich

RE: r325081 - Implement function attribute artificial

Thanks for your comments Richard, I’ve added the author to this email so that she can take a look. From: Richard Smith [mailto:rich...@metafoo.co.uk] Sent: Tuesday, February 13, 2018 5:39 PM To: Keane, Erich Cc: cfe-commits Subject: Re:

RE: r338884 - [NFC] Silence unused variable warning in Attr.td/AttrParsedAttrImpl.inc

Hmm… I thought this was a pretty clever one. Again, that didn’t show up in my older GCC :/ Any thoughts on how I can silence this AND the other warning? From: Nico Weber [mailto:tha...@chromium.org] Sent: Friday, August 3, 2018 6:16 AM To: Keane, Erich Cc: cfe-commits Subject: Re: r338884

RE: r338884 - [NFC] Silence unused variable warning in Attr.td/AttrParsedAttrImpl.inc

Thanks for the heads up! I repro’ed both in Godbolt, and saw that clang suggests wrapping in a void-cast, so I’ve done that. From: Nico Weber [mailto:tha...@chromium.org] Sent: Friday, August 3, 2018 6:16 AM To: Keane, Erich Cc: cfe-commits Subject: Re: r338884 - [NFC] Silence unused

RE: [PATCH] D48100: Append new attributes to the end of an AttributeList.

> As a possible idea (that may or may not work): the Attr object itself has a > SourceRange on it; perhaps a solution is to keep the > attributes in sorted > order within DeclBase::addAttr() based on the SourceRanges passed in? Interestingly, I think I came up with that idea in a comment on

RE: [PATCH] D48100: Append new attributes to the end of an AttributeList.

I suspect you're right about that. Hopefully it would be easy enough. Unfortunately, SourceLocation doesn't have an operator<, which kind of stinks. I suspect that it'll be a non-trivial thing to do, since it is actually possible to have attributes added with empty source-locations in some

RE: r338641 - [AST][4/4] Move the bit-fields from ObjCMethodDecl and ObjCContainerDecl into DeclContext

Thanks Vlad for the removal! This warning didn’t show up in my environment when committing this, I really need get a newer GCC :/ I suspect the patch author should extract that assert to the bitfield area with the rest. From: David Jones [mailto:d...@google.com] Sent: Wednesday, August 1,

RE: r338630 - [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext

Hi, sorry for the delayed response, I only just got back into the office. I've added the author of the patch to this email chain. -Original Message- From: Abramo Bagnara [mailto:abramo.bagn...@bugseng.com] Sent: Sunday, August 5, 2018 5:03 AM To: Keane, Erich ;

RE: r337005 - [NFC] Rename clang::AttributeList to clang::ParsedAttr

Good grief... I even made a note to remove this 'TODO' on my whiteboard! I discussed the name with AaronBallman who preferred ParsedAttr over ParsedAttribute (since ParsedAttributes is also a type). Fixed in r339039. -Original Message- From: meiners...@googlemail.com

RE: r339387 - Revert -r339382, which apparently breaks the Windows build.

AH! Thanks! I was just messing around on Godbolt to try to figure out what was going on. From: Richard Smith [mailto:rich...@metafoo.co.uk] Sent: Thursday, August 9, 2018 2:16 PM To: Keane, Erich Cc: cfe-commits Subject: Re: r339387 - Revert -r339382, which apparently breaks the Windows

RE: r336380 - Add PCH tests for R336379

Ugg… I did not. Seemingly my svn-property cleaning script isn’t working for some reason. If it hasn’t been done yet, I’ll clear the properties now. From: Nico Weber [mailto:tha...@chromium.org] Sent: Wednesday, July 18, 2018 5:21 AM To: Keane, Erich Cc: cfe-commits Subject: Re: r336380 - Add

RE: r336225 - Fix allocation of Nullability attribute.

Unfortunately I'm not sure of a good way to validate this. The only way I was able to even discover this was with manual instrumentation of D48788. There is a future opportunity to better instrument the source to find these things in the future that'll catch more issues like this one however.

RE: r334650 - Implement constexpr __builtin_*_overflow

Fixed in R336364. Thank you very much for the heads up! From: Evgenii Stepanov [mailto:eugeni.stepa...@gmail.com] Sent: Tuesday, July 3, 2018 12:59 PM To: Keane, Erich Cc: cfe-commits Subject: Re: r334650 - Implement constexpr __builtin_*_overflow Hi, with this change, the following compiles

RE: r336726 - [NFC] Switch CodeGenFunction to use value init instead of member init lists

Sorry everyone, I didn't police my subversion directory correctly. I noticed when I got the email that the 'commit' list was too large! Reverted in 336727. -Original Message- From: Erich Keane via cfe-commits [mailto:cfe-commits@lists.llvm.org] Sent: Tuesday, July 10, 2018 1:47 PM To:

RE: r344957 - Give Multiversion-inline functions linkonce linkage

GCC actually doesn’t support function multiversioning in C mode, so this fix is part of our extension to support C with multiversioning. I perhaps wonder if this is part of the reason GCC only supports it in C++ mode... From: David Blaikie [mailto:dblai...@gmail.com] Sent: Monday, October 29,

RE: r344957 - Give Multiversion-inline functions linkonce linkage

From: David Blaikie [mailto:dblai...@gmail.com] Sent: Monday, October 29, 2018 11:41 AM To: Keane, Erich Cc: Eric Christopher ; cfe-commits@lists.llvm.org Subject: Re: r344957 - Give Multiversion-inline functions linkonce linkage On Mon, Oct 29, 2018 at 11:30 AM Keane, Erich

RE: r346677 - Implement P1094R2 (nested inline namespaces)

Coolbeans, will do! Thanks for the review. -Erich From: Richard Smith [mailto:rich...@metafoo.co.uk] Sent: Monday, November 12, 2018 11:59 AM To: Keane, Erich Cc: cfe-commits Subject: Re: r346677 - Implement P1094R2 (nested inline namespaces) On Mon, 12 Nov 2018 at 09:22, Erich Keane via

RE: r351495 - Make integral-o-pointer conversions in SFINAE illegal.

Thanks for the heads up! Looking now. -Original Message- From: Peter Smith [mailto:peter.sm...@linaro.org] Sent: Friday, January 18, 2019 2:33 AM To: Keane, Erich Cc: cfe-commits cfe Subject: Re: r351495 - Make integral-o-pointer conversions in SFINAE illegal. Hello Erich, The test

RE: r349201 - Add extension to always default-initialize nullptr_t.

t together a while back as an attempt to fix a different bug, that creates a new CastKind for this operation (didn't work out there, but it might do so here). I'll dig it out and mail it to you in case it's a useful starting point. On Fri, 14 Dec 2018 at 14:42, Keane, Erich via cfe-commits

RE: r349201 - Add extension to always default-initialize nullptr_t.

I have a patch I put together a while back as an attempt to fix a different bug, that creates a new CastKind for this operation (didn't work out there, but it might do so here). I'll dig it out and mail it to you in case it's a useful starting point. On Fri, 14 Dec 2018 at 14:42, Keane, Erich v

RE: r349201 - Add extension to always default-initialize nullptr_t.

Reverted in r349206. From: Richard Smith [mailto:rich...@metafoo.co.uk] Sent: Friday, December 14, 2018 2:41 PM To: Keane, Erich Cc: cfe-commits Subject: Re: r349201 - Add extension to always default-initialize nullptr_t. Sorry, I was late with my review comment. I think this is the wrong way

RE: r349201 - Add extension to always default-initialize nullptr_t.

Alright, no problem. I’ll push a revert in a few minutes and give it another try. Perhaps I can suppress the creation of a CK_LValueToRValue in the case where it’s a read from nullptr_t From: Richard Smith [mailto:rich...@metafoo.co.uk] Sent: Friday, December 14, 2018 2:41 PM To: Keane,

RE: [cfe-dev] Dumping AST information to other formats

I'd be tentative to commit to any stability guarantees, particularly as the AST tends to change reasonably often. That said, I can see the value of this. Additionally, it would be preferential I suspect if we could make the standard ast-dump just another (albeit default) "format" in whatever

RE: r350643 - Limit COFF 'common' emission to <=32 alignment types.

That seems like it would make sense to me. COFF isn’t my expertise, so hopefully David/Martin/others can let me know if this is subtly broken elsewhere. From: Shoaib Meenai [mailto:smee...@fb.com] Sent: Tuesday, January 8, 2019 1:09 PM To: Keane, Erich ; cfe-commits@lists.llvm.org; David

RE: r350643 - Limit COFF 'common' emission to <=32 alignment types.

Yep, exactly. I looked, and isKnownWindowsMSVCEnvironment checks for OS=Win32, which I believe would be different for other architectures. From: Shoaib Meenai [mailto:smee...@fb.com] Sent: Tuesday, January 8, 2019 12:41 PM To: Keane, Erich ; cfe-commits@lists.llvm.org; David Majnemer Subject:

RE: r350643 - Limit COFF 'common' emission to <=32 alignment types.

Thank you for that! From: Shoaib Meenai [mailto:smee...@fb.com] Sent: Tuesday, January 8, 2019 4:48 PM To: David Majnemer Cc: Keane, Erich ; cfe-commits@lists.llvm.org; Martin Storsjo Subject: Re: r350643 - Limit COFF 'common' emission to <=32 alignment types. I sent out

RE: r355322 - Enable _rotl, _lrotl, _rotr, _lrotr on all platforms.

You’re right, GCC has these in a header as a #define. This patch didn’t interfere with that and was causing some other failures for us as a result. If its OK, I’d prefer to revert ONLY the enable on Linux mode at the moment, the long-int fix WAS valuable as well. From: James Y Knight

RE: r362119 - Add Attribute NoThrow as an Exception Specifier Type

It IS an attribute that works on Linux as well (about ½ way down this page: https://gcc.gnu.org/onlinedocs/gcc-4.7.2/gcc/Function-Attributes.html), though used rarely (at least in C++ code) as far as I understand. The fact that a mistake like this is in the ATL headers is frustrating, but I’m

RE: r362119 - Add Attribute NoThrow as an Exception Specifier Type

Presumably the right choice for you on this one is to separate the diagnostic into a new group. Copying Aaron since he might have an idea of an existing group, otherwise I’ll push a commit to put it in a new one. Thanks for the report! -Erich From: Nico Weber [mailto:tha...@chromium.org]

RE: r362119 - Add Attribute NoThrow as an Exception Specifier Type

To Update: Nico and I discussed this on IRC and believe the best way to fix this is to simply suppress this when it comes from a macro-expansion in a system header ala r221172 (or r220992). I intend to work on that fix next after my current (fairly short) fix. Thanks! -Erich From: Nico Weber

RE: r369281 - Implement P1668R1

Yeah, sorry about that. I fixed it in 369284. From: Leonard Chan [mailto:leonardc...@google.com] Sent: Monday, August 19, 2019 11:33 AM To: Keane, Erich Cc: cfe-commits cfe Subject: Re: r369281 - Implement P1668R1 Not sure if this was caught by upstream bots already, but we're seeing a

RE: r372185 - Revert "Create UsersManual section entitled 'Controlling Floating Point"

Ah, sorry- It broke the sphinx build bot, and the author was home for the evening and is going to get to it during the day today. From: Richard Smith Sent: Tuesday, September 17, 2019 8:10 PM To: Keane, Erich Cc: cfe-commits Subject: Re: r372185 - Revert "Create UsersManual section entitled

RE: r367027 - Implement P1771

Ah! Thanks. I’ll fix it now. From: Richard Smith [mailto:rich...@metafoo.co.uk] Sent: Thursday, July 25, 2019 9:26 AM To: Keane, Erich Cc: cfe-commits Subject: Re: r367027 - Implement P1771 On Thu, 25 Jul 2019, 08:10 Erich Keane via cfe-commits, mailto:cfe-commits@lists.llvm.org>> wrote:

RE: r373247 - Teach CallGraph to look into Generic Lambdas.

I saw that, thanks! I’m looking into them now. I can revert if it takes me too long. From: Nico Weber Sent: Monday, September 30, 2019 12:50 PM To: Keane, Erich Cc: cfe-commits Subject: Re: r373247 - Teach CallGraph to look into Generic Lambdas. This broke a few clangd unit tests:

RE: r373247 - Teach CallGraph to look into Generic Lambdas.

Should be fixe din r373259 From: Nico Weber Sent: Monday, September 30, 2019 12:50 PM To: Keane, Erich Cc: cfe-commits Subject: Re: r373247 - Teach CallGraph to look into Generic Lambdas. This broke a few clangd unit tests:

Re: [PATCH] D68824: Fix __builtin_assume_aligned with too large values.

Thanks! On the way home for the evening, so I'll fix it up tomorrow. On Oct 10, 2019 2:34 PM, Nico Weber via Phabricator wrote: thakis added a comment. Since you just left IRC, I reverted this in 374456 for now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68824/new/

RE: r373247 - Teach CallGraph to look into Generic Lambdas.

Yep, I agree. I tend to figure if I can get it done in ~1-2 hours, that I could just fix it. Note that this was 1:30  From: Nico Weber Sent: Wednesday, October 2, 2019 4:37 PM To: Keane, Erich Cc: cfe-commits Subject: Re: r373247 - Teach CallGraph to look into Generic Lambdas. Thanks! If

RE: r355698 - Re-fix _lrotl/_lrotr to always take Long, no matter the platform.

Right, the intent was to get this to match Microsoft’s behavior better while still making sense on other platforms. IIRC this was in order to fix a codegen bug due to type mismatches, though I don’t completely remember (as it was nearly a year ago). From: James Y Knight Sent: Wednesday,

RE: [PATCH] D73967: Implement _ExtInt as an extended int type specifier.

No problem, thanks for the response! Phab emails are apparently about 30 minutes delayed for me apparently, which is unfortunate or I would have had the fix ready before your revert  Let me know if you run into any more issues, but I’m 99% sure I got it right this time. Thanks -Erich From: