[PATCH] D78134: [Sema] Don't apply an lvalue-to-rvalue conversion to a discarded-value expression if it has an array type

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 263309.
ahatanak added a comment.

Rebase and ping.

I audited the calls to `DefaultLvalueConversion`. As far as I can tell, either an array type is never passed to the function call or there are checks that reject array types after the function is called, so the changes in this patch seem safe to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78134

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CXX/expr/p10-0x.cpp
  clang/test/SemaCXX/warn-unused-value-cxx11.cpp


Index: clang/test/SemaCXX/warn-unused-value-cxx11.cpp
===================================================================
--- clang/test/SemaCXX/warn-unused-value-cxx11.cpp
+++ clang/test/SemaCXX/warn-unused-value-cxx11.cpp
@@ -41,4 +41,13 @@
   (void)noexcept(s.g() = 5); // Ok
 }
 
-}
\ No newline at end of file
+}
+
+namespace volatile_array {
+void test() {
+  char a[10];
+  volatile char b[10];
+  a; // expected-warning-re {{expression result unused{{$}}}}
+  b; // expected-warning-re {{expression result unused{{$}}}}
+}
+}
Index: clang/test/CXX/expr/p10-0x.cpp
===================================================================
--- clang/test/CXX/expr/p10-0x.cpp
+++ clang/test/CXX/expr/p10-0x.cpp
@@ -44,3 +44,12 @@
   refcall();
   1 ? refcall() : *x;
 }
+
+// CHECK: define void @_Z2f3v()
+// CHECK-NOT: load
+// CHECK-NOT: memcpy
+
+void f3(void) {
+  volatile char a[10];
+  a;
+}
Index: clang/lib/Sema/SemaStmt.cpp
===================================================================
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -370,7 +370,10 @@
     }
   }
 
-  if (E->isGLValue() && E->getType().isVolatileQualified()) {
+  // Tell the user to assign it into a variable to force a volatile load if this
+  // isn't an array.
+  if (E->isGLValue() && E->getType().isVolatileQualified() &&
+      !E->getType()->isArrayType()) {
     Diag(Loc, diag::warn_unused_volatile) << R1 << R2;
     return;
   }
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -606,6 +606,10 @@
   QualType T = E->getType();
   assert(!T.isNull() && "r-value conversion on typeless expression?");
 
+  // lvalue-to-rvalue conversion cannot be applied to array types.
+  if (T->isArrayType())
+    return E;
+
   // We don't want to throw lvalue-to-rvalue casts on top of
   // expressions of certain types in C++.
   if (getLangOpts().CPlusPlus &&
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -10860,9 +10860,8 @@
                                                   bool Diagnose = true);
 
   // DefaultLvalueConversion - performs lvalue-to-rvalue conversion on
-  // the operand.  This is DefaultFunctionArrayLvalueConversion,
-  // except that it assumes the operand isn't of function or array
-  // type.
+  // the operand. This function assumes the operand isn't of function type. It
+  // is a no-op if the operand has an array type.
   ExprResult DefaultLvalueConversion(Expr *E);
 
   // DefaultArgumentPromotion (C99 6.5.2.2p6). Used for function calls that


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D78134.263309.patch
Type: text/x-patch
Size: 2739 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200512/662678c1/attachment.bin>