Re: [PATCH] D17619: [MSVC Compat] Don't evaluate member base expressions w/o side effects

[David Majnemer via cfe-commits]

majnemer updated this revision to Diff 49103.
majnemer added a comment.

- Add support for static data members too.


http://reviews.llvm.org/D17619

Files:
  lib/AST/ExprConstant.cpp
  test/SemaCXX/ms-const-member-expr.cpp

Index: test/SemaCXX/ms-const-member-expr.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/ms-const-member-expr.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -std=c++11 -fms-compatibility -fsyntax-only -verify
+
+struct S {
+  enum { E = 1 };
+  static const int sdm = 1;
+};
+
+void f(S *s) {
+  char array[s->E] = { 0 };
+}
+
+extern S *s;
+constexpr int e1 = s->E;
+
+S *side_effect();  // expected-note{{declared here}}
+constexpr int e2 = // expected-error{{must be initialized by a constant 
expression}}
+    side_effect()->E; // expected-note{{cannot be used in a constant 
expression}}
+
+constexpr int e4 = s->sdm;
Index: lib/AST/ExprConstant.cpp
===================================================================
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -4748,14 +4748,20 @@
 }
 
 bool LValueExprEvaluator::VisitMemberExpr(const MemberExpr *E) {
+  ValueDecl *MemberDecl = E->getMemberDecl();
+  ASTContext &Ctx = MemberDecl->getASTContext();
+
   // Handle static data members.
-  if (const VarDecl *VD = dyn_cast<VarDecl>(E->getMemberDecl())) {
-    VisitIgnoredValue(E->getBase());
+  if (const auto *VD = dyn_cast<VarDecl>(MemberDecl)) {
+    // While MSVC doesn't evaluate the base expression, it does diagnose the
+    // presence of side-effecting behavior.
+    if (!Info.getLangOpts().MSVCCompat || E->getBase()->HasSideEffects(Ctx))
+      VisitIgnoredValue(E->getBase());
     return VisitVarDecl(E, VD);
   }
 
   // Handle static member functions.
-  if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(E->getMemberDecl())) {
+  if (const auto *MD = dyn_cast<CXXMethodDecl>(MemberDecl)) {
     if (MD->isStatic()) {
       VisitIgnoredValue(E->getBase());
       return Success(MD);
@@ -6077,8 +6083,14 @@
     return ExprEvaluatorBaseTy::VisitDeclRefExpr(E);
   }
   bool VisitMemberExpr(const MemberExpr *E) {
-    if (CheckReferencedDecl(E, E->getMemberDecl())) {
-      VisitIgnoredValue(E->getBase());
+    ValueDecl *MemberDecl = E->getMemberDecl();
+    ASTContext &Ctx = MemberDecl->getASTContext();
+    if (CheckReferencedDecl(E, MemberDecl)) {
+      // While MSVC doesn't evaluate the base expression, it does diagnose the
+      // presence of side-effecting behavior.
+      if (!Info.getLangOpts().MSVCCompat || E->getBase()->HasSideEffects(Ctx)) 
{
+        VisitIgnoredValue(E->getBase());
+      }
       return true;
     }
 



Index: test/SemaCXX/ms-const-member-expr.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/ms-const-member-expr.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -std=c++11 -fms-compatibility -fsyntax-only -verify
+
+struct S {
+  enum { E = 1 };
+  static const int sdm = 1;
+};
+
+void f(S *s) {
+  char array[s->E] = { 0 };
+}
+
+extern S *s;
+constexpr int e1 = s->E;
+
+S *side_effect();  // expected-note{{declared here}}
+constexpr int e2 = // expected-error{{must be initialized by a constant expression}}
+    side_effect()->E; // expected-note{{cannot be used in a constant expression}}
+
+constexpr int e4 = s->sdm;
Index: lib/AST/ExprConstant.cpp
===================================================================
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -4748,14 +4748,20 @@
 }
 
 bool LValueExprEvaluator::VisitMemberExpr(const MemberExpr *E) {
+  ValueDecl *MemberDecl = E->getMemberDecl();
+  ASTContext &Ctx = MemberDecl->getASTContext();
+
   // Handle static data members.
-  if (const VarDecl *VD = dyn_cast<VarDecl>(E->getMemberDecl())) {
-    VisitIgnoredValue(E->getBase());
+  if (const auto *VD = dyn_cast<VarDecl>(MemberDecl)) {
+    // While MSVC doesn't evaluate the base expression, it does diagnose the
+    // presence of side-effecting behavior.
+    if (!Info.getLangOpts().MSVCCompat || E->getBase()->HasSideEffects(Ctx))
+      VisitIgnoredValue(E->getBase());
     return VisitVarDecl(E, VD);
   }
 
   // Handle static member functions.
-  if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(E->getMemberDecl())) {
+  if (const auto *MD = dyn_cast<CXXMethodDecl>(MemberDecl)) {
     if (MD->isStatic()) {
       VisitIgnoredValue(E->getBase());
       return Success(MD);
@@ -6077,8 +6083,14 @@
     return ExprEvaluatorBaseTy::VisitDeclRefExpr(E);
   }
   bool VisitMemberExpr(const MemberExpr *E) {
-    if (CheckReferencedDecl(E, E->getMemberDecl())) {
-      VisitIgnoredValue(E->getBase());
+    ValueDecl *MemberDecl = E->getMemberDecl();
+    ASTContext &Ctx = MemberDecl->getASTContext();
+    if (CheckReferencedDecl(E, MemberDecl)) {
+      // While MSVC doesn't evaluate the base expression, it does diagnose the
+      // presence of side-effecting behavior.
+      if (!Info.getLangOpts().MSVCCompat || E->getBase()->HasSideEffects(Ctx)) {
+        VisitIgnoredValue(E->getBase());
+      }
       return true;
     }
 

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