[clang] [clang][bytecode] Refine diagnostics for volatile reads (PR #136857)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

<details>
<summary>Changes</summary>

Differentiate between a volarile read via a lvalue-to-rvalue cast of a volatile qualified subexpression and a read from a pointer with a volatile base object.

---
Full diff: https://github.com/llvm/llvm-project/pull/136857.diff


5 Files Affected:

- (modified) clang/lib/AST/ByteCode/Compiler.cpp (+3) 
- (modified) clang/lib/AST/ByteCode/Interp.cpp (+24-5) 
- (modified) clang/lib/AST/ByteCode/Interp.h (+11-1) 
- (modified) clang/lib/AST/ByteCode/PrimType.h (+4-3) 
- (modified) clang/test/AST/ByteCode/literals.cpp (+16) 


``````````diff
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 7cba0e8a4da19..65d87cdff6ad2 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -210,6 +210,9 @@ bool Compiler<Emitter>::VisitCastExpr(const CastExpr *CE) {
 
   switch (CE->getCastKind()) {
   case CK_LValueToRValue: {
+    if (SubExpr->getType().isVolatileQualified())
+      return this->emitInvalidCast(CastKind::Volatile, /*Fatal=*/true, CE);
+
     std::optional<PrimType> SubExprT = classify(SubExpr->getType());
     // Prepare storage for the result.
     if (!Initializing && !SubExprT) {
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index b755a072fec88..6f277a7488836 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -641,11 +641,30 @@ static bool CheckVolatile(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
   if (!PtrType.isVolatileQualified())
     return true;
 
-  const SourceInfo &Loc = S.Current->getSource(OpPC);
-  if (S.getLangOpts().CPlusPlus)
-    S.FFDiag(Loc, diag::note_constexpr_access_volatile_type) << AK << PtrType;
-  else
-    S.FFDiag(Loc);
+  if (!S.getLangOpts().CPlusPlus)
+    return Invalid(S, OpPC);
+
+  const NamedDecl *ND = nullptr;
+  int DiagKind;
+  SourceLocation Loc;
+  if (const auto *F = Ptr.getField()) {
+    DiagKind = 2;
+    Loc = F->getLocation();
+    ND = F;
+  } else if (auto *VD = Ptr.getFieldDesc()->asValueDecl()) {
+    DiagKind = 1;
+    Loc = VD->getLocation();
+    ND = VD;
+  } else {
+    DiagKind = 0;
+    if (const auto *E = Ptr.getFieldDesc()->asExpr())
+      Loc = E->getExprLoc();
+  }
+
+  S.FFDiag(S.Current->getLocation(OpPC),
+           diag::note_constexpr_access_volatile_obj, 1)
+      << AK << DiagKind << ND;
+  S.Note(Loc, diag::note_constexpr_volatile_here) << DiagKind;
   return false;
 }
 
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index e5300b7cd96a9..588e0502fa88c 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -2885,12 +2885,22 @@ inline bool InvalidCast(InterpState &S, CodePtr OpPC, CastKind Kind,
                         bool Fatal) {
   const SourceLocation &Loc = S.Current->getLocation(OpPC);
 
-  // FIXME: Support diagnosing other invalid cast kinds.
   if (Kind == CastKind::Reinterpret) {
     S.CCEDiag(Loc, diag::note_constexpr_invalid_cast)
         << static_cast<unsigned>(Kind) << S.Current->getRange(OpPC);
     return !Fatal;
+  } else if (Kind == CastKind::Volatile) {
+    // FIXME: Technically not a cast.
+    const auto *E = cast<CastExpr>(S.Current->getExpr(OpPC));
+    if (S.getLangOpts().CPlusPlus)
+      S.FFDiag(E, diag::note_constexpr_access_volatile_type)
+          << AK_Read << E->getSubExpr()->getType();
+    else
+      S.FFDiag(E);
+
+    return false;
   }
+
   return false;
 }
 
diff --git a/clang/lib/AST/ByteCode/PrimType.h b/clang/lib/AST/ByteCode/PrimType.h
index a3c0b0f3ceca8..c6145d4823a0c 100644
--- a/clang/lib/AST/ByteCode/PrimType.h
+++ b/clang/lib/AST/ByteCode/PrimType.h
@@ -55,16 +55,17 @@ inline constexpr bool isPtrType(PrimType T) {
 
 enum class CastKind : uint8_t {
   Reinterpret,
-  Atomic,
+  Volatile,
 };
+
 inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
                                      interp::CastKind CK) {
   switch (CK) {
   case interp::CastKind::Reinterpret:
     OS << "reinterpret_cast";
     break;
-  case interp::CastKind::Atomic:
-    OS << "atomic";
+  case interp::CastKind::Volatile:
+    OS << "volatile";
     break;
   }
   return OS;
diff --git a/clang/test/AST/ByteCode/literals.cpp b/clang/test/AST/ByteCode/literals.cpp
index 6b33c5cc22367..c36289db6e85c 100644
--- a/clang/test/AST/ByteCode/literals.cpp
+++ b/clang/test/AST/ByteCode/literals.cpp
@@ -1357,6 +1357,22 @@ namespace VolatileReads {
   const volatile int b = 1;
   static_assert(b, ""); // both-error {{not an integral constant expression}} \
                         // both-note {{read of volatile-qualified type 'const volatile int' is not allowed in a constant expression}}
+
+
+  constexpr int a = 12;
+  constexpr volatile int c = (volatile int&)a; // both-error {{must be initialized by a constant expression}} \
+                                               // both-note {{read of volatile-qualified type 'volatile int'}}
+
+  volatile constexpr int n1 = 0; // both-note {{here}}
+  volatile const int n2 = 0; // both-note {{here}}
+  constexpr int m1 = n1; // both-error {{constant expression}} \
+                         // both-note {{read of volatile-qualified type 'const volatile int'}}
+  constexpr int m2 = n2; // both-error {{constant expression}} \
+                         // both-note {{read of volatile-qualified type 'const volatile int'}}
+  constexpr int m1b = const_cast<const int&>(n1); // both-error {{constant expression}} \
+                                                  // both-note {{read of volatile object 'n1'}}
+  constexpr int m2b = const_cast<const int&>(n2); // both-error {{constant expression}} \
+                                                  // both-note {{read of volatile object 'n2'}}
 }
 #if __cplusplus >= 201703L
 namespace {

``````````

</details>


https://github.com/llvm/llvm-project/pull/136857