[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D102517#2789593 , @AbbasSabra 
wrote:

> In D102517#2789450 , @goncharov 
> wrote:
>
>> Sorry, had to revert it as this fails under sanitizer : 
>> https://lab.llvm.org/buildbot/#/builders/5/builds/8150
>
> Thanks!
>
> I missed adding "Ident_abstract" in the Parser initialize function. Should be 
> fixed now.  CC: @hans

Thanks for the quick fix! Re-landed as 116179c2ee5213f2ae8f07a400ac98f0c995b3d3 
. I'll try 
to keep an eye on that buildbot.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102517/new/

https://reviews.llvm.org/D102517

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Hans Wennborg via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG116179c2ee52: Re-commit [clang] Add support for the 
abstract contextual keyword of MSVC (authored by AbbasSabra, 
committed by hans).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102517/new/

https://reviews.llvm.org/D102517

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/MicrosoftExtensions.cpp

Index: clang/test/SemaCXX/MicrosoftExtensions.cpp
===
--- clang/test/SemaCXX/MicrosoftExtensions.cpp
+++ clang/test/SemaCXX/MicrosoftExtensions.cpp
@@ -462,6 +462,77 @@
 virtual ~SealedDestructor() sealed; // expected-warning {{class with destructor marked 'sealed' cannot be inherited from}}
 };
 
+// expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+class AbstractClass abstract {
+  int i;
+};
+
+// expected-error@+1 {{variable type 'AbstractClass' is an abstract class}}
+AbstractClass abstractInstance;
+
+// expected-warning@+4 {{abstract class is marked 'sealed'}}
+// expected-note@+3 {{'AbstractAndSealedClass' declared here}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-warning@+1 {{'sealed' keyword is a Microsoft extension}}
+class AbstractAndSealedClass abstract sealed {}; // Does no really make sense, but allowed
+
+// expected-error@+1 {{variable type 'AbstractAndSealedClass' is an abstract class}}
+AbstractAndSealedClass abstractAndSealedInstance;
+// expected-error@+1 {{base 'AbstractAndSealedClass' is marked 'sealed'}}
+class InheritFromAbstractAndSealed : AbstractAndSealedClass {};
+
+#if __cplusplus <= 199711L
+// expected-warning@+4 {{'final' keyword is a C++11 extension}}
+// expected-warning@+3 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-error@+1 {{class already marked 'final'}}
+class TooManyVirtSpecifiers1 final final {};
+#if __cplusplus <= 199711L
+// expected-warning@+4 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+2 {{'sealed' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'sealed'}}
+class TooManyVirtSpecifiers2 final sealed {};
+#if __cplusplus <= 199711L
+// expected-warning@+6 {{'final' keyword is a C++11 extension}}
+// expected-warning@+5 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+3 {{abstract class is marked 'final'}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'final'}}
+class TooManyVirtSpecifiers3 final abstract final {};
+#if __cplusplus <= 199711L
+// expected-warning@+6 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+4 {{abstract class is marked 'final'}}
+// expected-warning@+3 {{'abstract' keyword is a Microsoft extension}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'abstract'}}
+class TooManyVirtSpecifiers4 abstract final abstract {};
+
+class Base {
+  virtual void i();
+};
+class AbstractFunctionInClass : public Base {
+  // expected-note@+2 {{unimplemented pure virtual method 'f' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void f() abstract;
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  void g() abstract; // expected-error {{'g' is not virtual and cannot be declared pure}}
+  // expected-note@+2 {{unimplemented pure virtual method 'h' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void h() abstract = 0; // expected-error {{class member already marked 'abstract'}}
+#if __cplusplus <= 199711L
+  // expected-warning@+4 {{'override' keyword is a C++11 extension}}
+#endif
+  // expected-note@+2 {{unimplemented pure virtual method 'i' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void i() abstract override;
+};
+
+// expected-error@+1 {{variable type 'AbstractFunctionInClass' is an abstract class}}
+AbstractFunctionInClass abstractFunctionInClassInstance;
+
 void AfterClassBody() {
   // expected-warning@+1 {{attribute 'deprecated' is ignored, place it after "struct" to apply attribute to type declaration}}
   struct D {} __declspec(deprecated);
Index: clang/lib/Sema/SemaDecl.cpp

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.

In D102517#2789450 , @goncharov wrote:

> Sorry, had to revert it as this fails under sanitizer : 
> https://lab.llvm.org/buildbot/#/builders/5/builds/8150

Thanks!

I missed adding "Ident_abstract" in the Parser initialize function. Should be 
fixed now.  CC: @hans


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102517/new/

https://reviews.llvm.org/D102517

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra updated this revision to Diff 348806.
AbbasSabra added a comment.

Updating D102517 : [clang]Fix 
use-of-uninitialized-value detected by the sanitizer


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102517/new/

https://reviews.llvm.org/D102517

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/MicrosoftExtensions.cpp

Index: clang/test/SemaCXX/MicrosoftExtensions.cpp
===
--- clang/test/SemaCXX/MicrosoftExtensions.cpp
+++ clang/test/SemaCXX/MicrosoftExtensions.cpp
@@ -462,6 +462,77 @@
 virtual ~SealedDestructor() sealed; // expected-warning {{class with destructor marked 'sealed' cannot be inherited from}}
 };
 
+// expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+class AbstractClass abstract {
+  int i;
+};
+
+// expected-error@+1 {{variable type 'AbstractClass' is an abstract class}}
+AbstractClass abstractInstance;
+
+// expected-warning@+4 {{abstract class is marked 'sealed'}}
+// expected-note@+3 {{'AbstractAndSealedClass' declared here}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-warning@+1 {{'sealed' keyword is a Microsoft extension}}
+class AbstractAndSealedClass abstract sealed {}; // Does no really make sense, but allowed
+
+// expected-error@+1 {{variable type 'AbstractAndSealedClass' is an abstract class}}
+AbstractAndSealedClass abstractAndSealedInstance;
+// expected-error@+1 {{base 'AbstractAndSealedClass' is marked 'sealed'}}
+class InheritFromAbstractAndSealed : AbstractAndSealedClass {};
+
+#if __cplusplus <= 199711L
+// expected-warning@+4 {{'final' keyword is a C++11 extension}}
+// expected-warning@+3 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-error@+1 {{class already marked 'final'}}
+class TooManyVirtSpecifiers1 final final {};
+#if __cplusplus <= 199711L
+// expected-warning@+4 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+2 {{'sealed' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'sealed'}}
+class TooManyVirtSpecifiers2 final sealed {};
+#if __cplusplus <= 199711L
+// expected-warning@+6 {{'final' keyword is a C++11 extension}}
+// expected-warning@+5 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+3 {{abstract class is marked 'final'}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'final'}}
+class TooManyVirtSpecifiers3 final abstract final {};
+#if __cplusplus <= 199711L
+// expected-warning@+6 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+4 {{abstract class is marked 'final'}}
+// expected-warning@+3 {{'abstract' keyword is a Microsoft extension}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'abstract'}}
+class TooManyVirtSpecifiers4 abstract final abstract {};
+
+class Base {
+  virtual void i();
+};
+class AbstractFunctionInClass : public Base {
+  // expected-note@+2 {{unimplemented pure virtual method 'f' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void f() abstract;
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  void g() abstract; // expected-error {{'g' is not virtual and cannot be declared pure}}
+  // expected-note@+2 {{unimplemented pure virtual method 'h' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void h() abstract = 0; // expected-error {{class member already marked 'abstract'}}
+#if __cplusplus <= 199711L
+  // expected-warning@+4 {{'override' keyword is a C++11 extension}}
+#endif
+  // expected-note@+2 {{unimplemented pure virtual method 'i' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void i() abstract override;
+};
+
+// expected-error@+1 {{variable type 'AbstractFunctionInClass' is an abstract class}}
+AbstractFunctionInClass abstractFunctionInClassInstance;
+
 void AfterClassBody() {
   // expected-warning@+1 {{attribute 'deprecated' is ignored, place it after "struct" to apply attribute to type declaration}}
   struct D {} __declspec(deprecated);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16436,6 +16436,7 @@
 void Sema::ActOnStartCXXMemberDeclarations(Scope *S, Decl 

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Mikhail Goncharov via Phabricator via cfe-commits]

goncharov added a comment.

Sorry, had to revert it as this fails under sanitizer : 
https://lab.llvm.org/buildbot/#/builders/5/builds/8150


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102517/new/

https://reviews.llvm.org/D102517

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Pushed as 
http://github.com/llvm/llvm-project/commit/818338add77411f5e9713247ea66142f332ef350


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102517/new/

https://reviews.llvm.org/D102517

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Hans Wennborg 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 rG818338add774: [clang] Add support for the 
abstract contextual keyword of MSVC (authored by AbbasSabra, 
committed by hans).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102517/new/

https://reviews.llvm.org/D102517

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/MicrosoftExtensions.cpp

Index: clang/test/SemaCXX/MicrosoftExtensions.cpp
===
--- clang/test/SemaCXX/MicrosoftExtensions.cpp
+++ clang/test/SemaCXX/MicrosoftExtensions.cpp
@@ -462,6 +462,77 @@
 virtual ~SealedDestructor() sealed; // expected-warning {{class with destructor marked 'sealed' cannot be inherited from}}
 };
 
+// expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+class AbstractClass abstract {
+  int i;
+};
+
+// expected-error@+1 {{variable type 'AbstractClass' is an abstract class}}
+AbstractClass abstractInstance;
+
+// expected-warning@+4 {{abstract class is marked 'sealed'}}
+// expected-note@+3 {{'AbstractAndSealedClass' declared here}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-warning@+1 {{'sealed' keyword is a Microsoft extension}}
+class AbstractAndSealedClass abstract sealed {}; // Does no really make sense, but allowed
+
+// expected-error@+1 {{variable type 'AbstractAndSealedClass' is an abstract class}}
+AbstractAndSealedClass abstractAndSealedInstance;
+// expected-error@+1 {{base 'AbstractAndSealedClass' is marked 'sealed'}}
+class InheritFromAbstractAndSealed : AbstractAndSealedClass {};
+
+#if __cplusplus <= 199711L
+// expected-warning@+4 {{'final' keyword is a C++11 extension}}
+// expected-warning@+3 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-error@+1 {{class already marked 'final'}}
+class TooManyVirtSpecifiers1 final final {};
+#if __cplusplus <= 199711L
+// expected-warning@+4 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+2 {{'sealed' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'sealed'}}
+class TooManyVirtSpecifiers2 final sealed {};
+#if __cplusplus <= 199711L
+// expected-warning@+6 {{'final' keyword is a C++11 extension}}
+// expected-warning@+5 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+3 {{abstract class is marked 'final'}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'final'}}
+class TooManyVirtSpecifiers3 final abstract final {};
+#if __cplusplus <= 199711L
+// expected-warning@+6 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+4 {{abstract class is marked 'final'}}
+// expected-warning@+3 {{'abstract' keyword is a Microsoft extension}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'abstract'}}
+class TooManyVirtSpecifiers4 abstract final abstract {};
+
+class Base {
+  virtual void i();
+};
+class AbstractFunctionInClass : public Base {
+  // expected-note@+2 {{unimplemented pure virtual method 'f' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void f() abstract;
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  void g() abstract; // expected-error {{'g' is not virtual and cannot be declared pure}}
+  // expected-note@+2 {{unimplemented pure virtual method 'h' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void h() abstract = 0; // expected-error {{class member already marked 'abstract'}}
+#if __cplusplus <= 199711L
+  // expected-warning@+4 {{'override' keyword is a C++11 extension}}
+#endif
+  // expected-note@+2 {{unimplemented pure virtual method 'i' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void i() abstract override;
+};
+
+// expected-error@+1 {{variable type 'AbstractFunctionInClass' is an abstract class}}
+AbstractFunctionInClass abstractFunctionInClassInstance;
+
 void AfterClassBody() {
   // expected-warning@+1 {{attribute 'deprecated' is ignored, place it after "struct" to apply attribute to type declaration}}
   struct D {} __declspec(deprecated);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16456,6 +16456,7 @@
 void 

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.

Thanks for the review! Can you take care of merging the patch? I don't have 
access.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102517/new/

https://reviews.llvm.org/D102517

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Hans Wennborg via Phabricator via cfe-commits]

hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.

Looks good to me!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102517/new/

https://reviews.llvm.org/D102517

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra updated this revision to Diff 347955.
AbbasSabra added a comment.

Updating D102517 : [clang] Apply code review: 
while loop instead of for loop


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102517/new/

https://reviews.llvm.org/D102517

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/MicrosoftExtensions.cpp

Index: clang/test/SemaCXX/MicrosoftExtensions.cpp
===
--- clang/test/SemaCXX/MicrosoftExtensions.cpp
+++ clang/test/SemaCXX/MicrosoftExtensions.cpp
@@ -462,6 +462,77 @@
 virtual ~SealedDestructor() sealed; // expected-warning {{class with destructor marked 'sealed' cannot be inherited from}}
 };
 
+// expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+class AbstractClass abstract {
+  int i;
+};
+
+// expected-error@+1 {{variable type 'AbstractClass' is an abstract class}}
+AbstractClass abstractInstance;
+
+// expected-warning@+4 {{abstract class is marked 'sealed'}}
+// expected-note@+3 {{'AbstractAndSealedClass' declared here}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-warning@+1 {{'sealed' keyword is a Microsoft extension}}
+class AbstractAndSealedClass abstract sealed {}; // Does no really make sense, but allowed
+
+// expected-error@+1 {{variable type 'AbstractAndSealedClass' is an abstract class}}
+AbstractAndSealedClass abstractAndSealedInstance;
+// expected-error@+1 {{base 'AbstractAndSealedClass' is marked 'sealed'}}
+class InheritFromAbstractAndSealed : AbstractAndSealedClass {};
+
+#if __cplusplus <= 199711L
+// expected-warning@+4 {{'final' keyword is a C++11 extension}}
+// expected-warning@+3 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-error@+1 {{class already marked 'final'}}
+class TooManyVirtSpecifiers1 final final {};
+#if __cplusplus <= 199711L
+// expected-warning@+4 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+2 {{'sealed' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'sealed'}}
+class TooManyVirtSpecifiers2 final sealed {};
+#if __cplusplus <= 199711L
+// expected-warning@+6 {{'final' keyword is a C++11 extension}}
+// expected-warning@+5 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+3 {{abstract class is marked 'final'}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'final'}}
+class TooManyVirtSpecifiers3 final abstract final {};
+#if __cplusplus <= 199711L
+// expected-warning@+6 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+4 {{abstract class is marked 'final'}}
+// expected-warning@+3 {{'abstract' keyword is a Microsoft extension}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'abstract'}}
+class TooManyVirtSpecifiers4 abstract final abstract {};
+
+class Base {
+  virtual void i();
+};
+class AbstractFunctionInClass : public Base {
+  // expected-note@+2 {{unimplemented pure virtual method 'f' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void f() abstract;
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  void g() abstract; // expected-error {{'g' is not virtual and cannot be declared pure}}
+  // expected-note@+2 {{unimplemented pure virtual method 'h' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void h() abstract = 0; // expected-error {{class member already marked 'abstract'}}
+#if __cplusplus <= 199711L
+  // expected-warning@+4 {{'override' keyword is a C++11 extension}}
+#endif
+  // expected-note@+2 {{unimplemented pure virtual method 'i' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void i() abstract override;
+};
+
+// expected-error@+1 {{variable type 'AbstractFunctionInClass' is an abstract class}}
+AbstractFunctionInClass abstractFunctionInClassInstance;
+
 void AfterClassBody() {
   // expected-warning@+1 {{attribute 'deprecated' is ignored, place it after "struct" to apply attribute to type declaration}}
   struct D {} __declspec(deprecated);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16436,6 +16436,7 @@
 void Sema::ActOnStartCXXMemberDeclarations(Scope *S, Decl *TagD,
 

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Apologies for the slow reply, it was a long weekend here.




Comment at: clang/lib/Parse/ParseDeclCXX.cpp:3314
   if (getLangOpts().CPlusPlus && Tok.is(tok::identifier)) {
-VirtSpecifiers::Specifier Specifier = isCXX11VirtSpecifier(Tok);
-assert((Specifier == VirtSpecifiers::VS_Final ||
-Specifier == VirtSpecifiers::VS_GNU_Final ||
-Specifier == VirtSpecifiers::VS_Sealed) &&
+for (VirtSpecifiers::Specifier Specifier = isCXX11VirtSpecifier(Tok);
+ Specifier != VirtSpecifiers::VS_None;

AbbasSabra wrote:
> hans wrote:
> > This 'for' construct is hard to follow. I think it might be easier to 
> > follow as a while loop, maybe
> > 
> > ```
> > while ((Specifier = isCXX11VirtSpecifier(Tok)) != VirtSpecifiers::VS_None) {
> > ```
> > 
> > Also the "is" prefix in isCXX11VirtSpecifier is confusing given that it 
> > doesn't return a bool.
> For "for" vs "while" if you use while you will have to declare the variable 
> before the loop giving it the wrong scope which makes it less readable in my 
> opinion. 
> For  !!isCXX11VirtSpecifier!! I agree but I wouldn't separate the rename from 
> this patch(I'm not sure what is the guidelines here)
> 
I still think the for-loop is confusing. How about something like this, then?

```
while (true) {
  VirtSpecifiers::Specifier Specifier = isCXX11VirtSpecifier(Tok);
  if (Specifier == VirtSpecifiers::VS_None)
break;

  ...
}
```

I agree that renaming isCXX11VirtSpecifier in a separate patch is a good idea.



Comment at: clang/lib/Sema/DeclSpec.cpp:1479
+  case VS_Abstract:
+VS_abstractLoc = Loc;
+break;

AbbasSabra wrote:
> hans wrote:
> > nit: the other cases just use one line
> clang-format doesn't like it. Should I ignore it?
Yes, ignoring it is fine. (Re-formatting the whole switch would also be okay, 
as long as its consistent.)



Comment at: clang/lib/Sema/DeclSpec.cpp:1493
   case VS_Sealed: return "sealed";
+  case VS_Abstract:
+return "abstract";

AbbasSabra wrote:
> hans wrote:
> > nit: skip the newline for consistency here too
> same clang-format splits the line
Ignoring clang-format is fine, sticking to the current style is usually 
preferred..


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102517/new/

https://reviews.llvm.org/D102517

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.

> Does MS use this in any library headers or such

I didn't check if they do. But if any codebase does use "abstract" and they are 
trying to use clang-cl they would face a parsing error.
My use case was running static analysis on code that uses this feature. Such an 
error had a great impact on the quality of the analysis.




Comment at: clang/lib/Parse/ParseDeclCXX.cpp:3314
   if (getLangOpts().CPlusPlus && Tok.is(tok::identifier)) {
-VirtSpecifiers::Specifier Specifier = isCXX11VirtSpecifier(Tok);
-assert((Specifier == VirtSpecifiers::VS_Final ||
-Specifier == VirtSpecifiers::VS_GNU_Final ||
-Specifier == VirtSpecifiers::VS_Sealed) &&
+for (VirtSpecifiers::Specifier Specifier = isCXX11VirtSpecifier(Tok);
+ Specifier != VirtSpecifiers::VS_None;

hans wrote:
> This 'for' construct is hard to follow. I think it might be easier to follow 
> as a while loop, maybe
> 
> ```
> while ((Specifier = isCXX11VirtSpecifier(Tok)) != VirtSpecifiers::VS_None) {
> ```
> 
> Also the "is" prefix in isCXX11VirtSpecifier is confusing given that it 
> doesn't return a bool.
For "for" vs "while" if you use while you will have to declare the variable 
before the loop giving it the wrong scope which makes it less readable in my 
opinion. 
For  !!isCXX11VirtSpecifier!! I agree but I wouldn't separate the rename from 
this patch(I'm not sure what is the guidelines here)




Comment at: clang/lib/Sema/DeclSpec.cpp:1479
+  case VS_Abstract:
+VS_abstractLoc = Loc;
+break;

hans wrote:
> nit: the other cases just use one line
clang-format doesn't like it. Should I ignore it?



Comment at: clang/lib/Sema/DeclSpec.cpp:1493
   case VS_Sealed: return "sealed";
+  case VS_Abstract:
+return "abstract";

hans wrote:
> nit: skip the newline for consistency here too
same clang-format splits the line


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102517/new/

https://reviews.llvm.org/D102517

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra updated this revision to Diff 347018.
AbbasSabra marked 3 inline comments as done.
AbbasSabra added a comment.

Updating D102517 : [clang]Apply code review


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102517/new/

https://reviews.llvm.org/D102517

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/MicrosoftExtensions.cpp

Index: clang/test/SemaCXX/MicrosoftExtensions.cpp
===
--- clang/test/SemaCXX/MicrosoftExtensions.cpp
+++ clang/test/SemaCXX/MicrosoftExtensions.cpp
@@ -462,6 +462,77 @@
 virtual ~SealedDestructor() sealed; // expected-warning {{class with destructor marked 'sealed' cannot be inherited from}}
 };
 
+// expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+class AbstractClass abstract {
+  int i;
+};
+
+// expected-error@+1 {{variable type 'AbstractClass' is an abstract class}}
+AbstractClass abstractInstance;
+
+// expected-warning@+4 {{abstract class is marked 'sealed'}}
+// expected-note@+3 {{'AbstractAndSealedClass' declared here}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-warning@+1 {{'sealed' keyword is a Microsoft extension}}
+class AbstractAndSealedClass abstract sealed {}; // Does no really make sense, but allowed
+
+// expected-error@+1 {{variable type 'AbstractAndSealedClass' is an abstract class}}
+AbstractAndSealedClass abstractAndSealedInstance;
+// expected-error@+1 {{base 'AbstractAndSealedClass' is marked 'sealed'}}
+class InheritFromAbstractAndSealed : AbstractAndSealedClass {};
+
+#if __cplusplus <= 199711L
+// expected-warning@+4 {{'final' keyword is a C++11 extension}}
+// expected-warning@+3 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-error@+1 {{class already marked 'final'}}
+class TooManyVirtSpecifiers1 final final {};
+#if __cplusplus <= 199711L
+// expected-warning@+4 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+2 {{'sealed' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'sealed'}}
+class TooManyVirtSpecifiers2 final sealed {};
+#if __cplusplus <= 199711L
+// expected-warning@+6 {{'final' keyword is a C++11 extension}}
+// expected-warning@+5 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+3 {{abstract class is marked 'final'}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'final'}}
+class TooManyVirtSpecifiers3 final abstract final {};
+#if __cplusplus <= 199711L
+// expected-warning@+6 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+4 {{abstract class is marked 'final'}}
+// expected-warning@+3 {{'abstract' keyword is a Microsoft extension}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'abstract'}}
+class TooManyVirtSpecifiers4 abstract final abstract {};
+
+class Base {
+  virtual void i();
+};
+class AbstractFunctionInClass : public Base {
+  // expected-note@+2 {{unimplemented pure virtual method 'f' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void f() abstract;
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  void g() abstract; // expected-error {{'g' is not virtual and cannot be declared pure}}
+  // expected-note@+2 {{unimplemented pure virtual method 'h' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void h() abstract = 0; // expected-error {{class member already marked 'abstract'}}
+#if __cplusplus <= 199711L
+  // expected-warning@+4 {{'override' keyword is a C++11 extension}}
+#endif
+  // expected-note@+2 {{unimplemented pure virtual method 'i' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void i() abstract override;
+};
+
+// expected-error@+1 {{variable type 'AbstractFunctionInClass' is an abstract class}}
+AbstractFunctionInClass abstractFunctionInClassInstance;
+
 void AfterClassBody() {
   // expected-warning@+1 {{attribute 'deprecated' is ignored, place it after "struct" to apply attribute to type declaration}}
   struct D {} __declspec(deprecated);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16436,6 +16436,7 @@
 void Sema::ActOnStartCXXMemberDeclarations(Scope *S, Decl *TagD,
  

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Looking good overall, just a few nits.

Out of curiosity, where would one run into this? Does MS use this in any 
library headers or such?




Comment at: clang/lib/Parse/ParseDeclCXX.cpp:3314
   if (getLangOpts().CPlusPlus && Tok.is(tok::identifier)) {
-VirtSpecifiers::Specifier Specifier = isCXX11VirtSpecifier(Tok);
-assert((Specifier == VirtSpecifiers::VS_Final ||
-Specifier == VirtSpecifiers::VS_GNU_Final ||
-Specifier == VirtSpecifiers::VS_Sealed) &&
+for (VirtSpecifiers::Specifier Specifier = isCXX11VirtSpecifier(Tok);
+ Specifier != VirtSpecifiers::VS_None;

This 'for' construct is hard to follow. I think it might be easier to follow as 
a while loop, maybe

```
while ((Specifier = isCXX11VirtSpecifier(Tok)) != VirtSpecifiers::VS_None) {
```

Also the "is" prefix in isCXX11VirtSpecifier is confusing given that it doesn't 
return a bool.



Comment at: clang/lib/Sema/DeclSpec.cpp:1479
+  case VS_Abstract:
+VS_abstractLoc = Loc;
+break;

nit: the other cases just use one line



Comment at: clang/lib/Sema/DeclSpec.cpp:1493
   case VS_Sealed: return "sealed";
+  case VS_Abstract:
+return "abstract";

nit: skip the newline for consistency here too



Comment at: clang/lib/Sema/SemaDecl.cpp:16449
 
-  if (FinalLoc.isValid())
+  if (IsAbstract) {
+Record->markAbstract();

nit: it would be more common in clang to omit the braces


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102517/new/

https://reviews.llvm.org/D102517

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

@hans, can you review this? I am trying to offload clang reviews.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102517/new/

https://reviews.llvm.org/D102517

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra created this revision.
AbbasSabra requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

https://docs.microsoft.com/en-us/cpp/extensions/abstract-cpp-component-extensions?view=msvc-160


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102517

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/MicrosoftExtensions.cpp

Index: clang/test/SemaCXX/MicrosoftExtensions.cpp
===
--- clang/test/SemaCXX/MicrosoftExtensions.cpp
+++ clang/test/SemaCXX/MicrosoftExtensions.cpp
@@ -462,6 +462,77 @@
 virtual ~SealedDestructor() sealed; // expected-warning {{class with destructor marked 'sealed' cannot be inherited from}}
 };
 
+// expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+class AbstractClass abstract {
+  int i;
+};
+
+// expected-error@+1 {{variable type 'AbstractClass' is an abstract class}}
+AbstractClass abstractInstance;
+
+// expected-warning@+4 {{abstract class is marked 'sealed'}}
+// expected-note@+3 {{'AbstractAndSealedClass' declared here}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-warning@+1 {{'sealed' keyword is a Microsoft extension}}
+class AbstractAndSealedClass abstract sealed {}; // Does no really make sense, but allowed
+
+// expected-error@+1 {{variable type 'AbstractAndSealedClass' is an abstract class}}
+AbstractAndSealedClass abstractAndSealedInstance;
+// expected-error@+1 {{base 'AbstractAndSealedClass' is marked 'sealed'}}
+class InheritFromAbstractAndSealed : AbstractAndSealedClass {};
+
+#if __cplusplus <= 199711L
+// expected-warning@+4 {{'final' keyword is a C++11 extension}}
+// expected-warning@+3 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-error@+1 {{class already marked 'final'}}
+class TooManyVirtSpecifiers1 final final {};
+#if __cplusplus <= 199711L
+// expected-warning@+4 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+2 {{'sealed' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'sealed'}}
+class TooManyVirtSpecifiers2 final sealed {};
+#if __cplusplus <= 199711L
+// expected-warning@+6 {{'final' keyword is a C++11 extension}}
+// expected-warning@+5 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+3 {{abstract class is marked 'final'}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'final'}}
+class TooManyVirtSpecifiers3 final abstract final {};
+#if __cplusplus <= 199711L
+// expected-warning@+6 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+4 {{abstract class is marked 'final'}}
+// expected-warning@+3 {{'abstract' keyword is a Microsoft extension}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'abstract'}}
+class TooManyVirtSpecifiers4 abstract final abstract {};
+
+class Base {
+  virtual void i();
+};
+class AbstractFunctionInClass : public Base {
+  // expected-note@+2 {{unimplemented pure virtual method 'f' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void f() abstract;
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  void g() abstract; // expected-error {{'g' is not virtual and cannot be declared pure}}
+  // expected-note@+2 {{unimplemented pure virtual method 'h' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void h() abstract = 0; // expected-error {{class member already marked 'abstract'}}
+#if __cplusplus <= 199711L
+  // expected-warning@+4 {{'override' keyword is a C++11 extension}}
+#endif
+  // expected-note@+2 {{unimplemented pure virtual method 'i' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void i() abstract override;
+};
+
+// expected-error@+1 {{variable type 'AbstractFunctionInClass' is an abstract class}}
+AbstractFunctionInClass abstractFunctionInClassInstance;
+
 void AfterClassBody() {
   // expected-warning@+1 {{attribute 'deprecated' is ignored, place it after "struct" to apply attribute to type declaration}}
   struct D {} __declspec(deprecated);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16436,6 +16436,7 @@
 void Sema::ActOnStartCXXMemberDeclarations(Scope *S, Decl *TagD,