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

Keane, Erich via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 25 13:20:21 PDT 2016


Done!  Attached a new diff.

From: David Majnemer [mailto:david.majnemer at gmail.com]
Sent: Monday, July 25, 2016 1:18 PM
To: Keane, Erich <erich.keane at intel.com>
Cc: cfe-commits at lists.llvm.org
Subject: Re: [ReviewRequest] [Sema/Parser] GCC Compatibility Patch: Support for __final specifier

You've added a comment describing the new enum value.  Please make sure the comment starts with a cap letter and ends with a period.

On Mon, Jul 25, 2016 at 4:09 PM, Keane, Erich <erich.keane at intel.com<mailto:erich.keane at intel.com>> wrote:
Thanks for the quick review!  I’ve updated the patch with the name changes in the attached diff file.

From: David Majnemer [mailto:david.majnemer at gmail.com<mailto:david.majnemer at gmail.com>]
Sent: Monday, July 25, 2016 12:40 PM
To: Keane, Erich <erich.keane at intel.com<mailto:erich.keane at intel.com>>
Cc: cfe-commits at lists.llvm.org<mailto:cfe-commits at 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 at lists.llvm.org<mailto:cfe-commits at lists.llvm.org>> wrote:
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.  Clang currently supports the ‘final’ keyword in accordance with the C++11 specification, however it ALSO supports it in pre-C++11 mode, with a warning.

This patch adds the ‘__final’ keyword for compatibility with GCC in GCC Keywords mode (so it is enabled with existing flags), and issues a warning on its usage (suggesting switching to the C++11 keyword).  This patch also adds a regression test for the functionality described.  I believe this patch has minimal impact, as it simply adds a new keyword for existing behavior.

This has been validated with check-clang to avoid regressions.  Patch is created in reference to revisions 276665

Thanks,
Erich

_______________________________________________
cfe-commits mailing list
cfe-commits at lists.llvm.org<mailto:cfe-commits at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160725/f60e26ce/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: __final_clang.diff
Type: application/octet-stream
Size: 6110 bytes
Desc: __final_clang.diff
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160725/f60e26ce/attachment-0001.obj>


More information about the cfe-commits mailing list