[PATCH] D90336: [Sema] Diagnose annotating `if constexpr` with a likelihood attribute

[Mark de Wever via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Mordante marked an inline comment as done.
Closed by commit rGb231396122f1: [Sema] Diagnose annotating `if constexpr` with a likelihood attribute (authored by Mordante).

Changed prior to commit:
  https://reviews.llvm.org/D90336?vs=301384&id=302101#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90336

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaCXX/attr-likelihood.cpp


Index: clang/test/SemaCXX/attr-likelihood.cpp
===================================================================
--- clang/test/SemaCXX/attr-likelihood.cpp
+++ clang/test/SemaCXX/attr-likelihood.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=c++17 -fsyntax-only -verify
 // RUN: %clang_cc1 %s -DPEDANTIC -pedantic -fsyntax-only -verify
 
 #if PEDANTIC
@@ -129,4 +129,23 @@
   catch (...) [[likely]] { // expected-error {{expected expression}}
   }
 }
+
+void o()
+{
+  // expected-warning at +2 {{attribute 'likely' has no effect when annotating an 'if constexpr' statement}}
+  // expected-note at +1 {{annotating the 'if constexpr' statement here}}
+  if constexpr (true) [[likely]];
+
+  // expected-note at +1 {{annotating the 'if constexpr' statement here}}
+  if constexpr (true) {
+  // expected-warning at +1 {{attribute 'unlikely' has no effect when annotating an 'if constexpr' statement}}
+  } else [[unlikely]];
+
+  // Annotating both branches with conflicting likelihoods generates no diagnostic regarding the conflict.
+  // expected-warning at +2 {{attribute 'likely' has no effect when annotating an 'if constexpr' statement}}
+  // expected-note at +1 2 {{annotating the 'if constexpr' statement here}}
+  if constexpr (true) [[likely]] {
+  // expected-warning at +1 {{attribute 'likely' has no effect when annotating an 'if constexpr' statement}}
+  } else [[likely]];
+}
 #endif
Index: clang/lib/Sema/SemaStmt.cpp
===================================================================
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -600,16 +600,31 @@
     DiagnoseEmptyStmtBody(CondExpr->getEndLoc(), thenStmt,
                           diag::warn_empty_if_body);
 
-  std::tuple<bool, const Attr *, const Attr *> LHC =
-      Stmt::determineLikelihoodConflict(thenStmt, elseStmt);
-  if (std::get<0>(LHC)) {
-    const Attr *ThenAttr = std::get<1>(LHC);
-    const Attr *ElseAttr = std::get<2>(LHC);
-    Diags.Report(ThenAttr->getLocation(),
-                 diag::warn_attributes_likelihood_ifstmt_conflict)
-        << ThenAttr << ThenAttr->getRange();
-    Diags.Report(ElseAttr->getLocation(), diag::note_conflicting_attribute)
-        << ElseAttr << ElseAttr->getRange();
+  if (IsConstexpr) {
+    auto DiagnoseLikelihood = [&](const Stmt *S) {
+      if (const Attr *A = Stmt::getLikelihoodAttr(S)) {
+        Diags.Report(A->getLocation(),
+                     diag::warn_attribute_has_no_effect_on_if_constexpr)
+            << A << A->getRange();
+        Diags.Report(IfLoc,
+                     diag::note_attribute_has_no_effect_on_if_constexpr_here)
+            << SourceRange(IfLoc, LParenLoc.getLocWithOffset(-1));
+      }
+    };
+    DiagnoseLikelihood(thenStmt);
+    DiagnoseLikelihood(elseStmt);
+  } else {
+    std::tuple<bool, const Attr *, const Attr *> LHC =
+        Stmt::determineLikelihoodConflict(thenStmt, elseStmt);
+    if (std::get<0>(LHC)) {
+      const Attr *ThenAttr = std::get<1>(LHC);
+      const Attr *ElseAttr = std::get<2>(LHC);
+      Diags.Report(ThenAttr->getLocation(),
+                   diag::warn_attributes_likelihood_ifstmt_conflict)
+          << ThenAttr << ThenAttr->getRange();
+      Diags.Report(ElseAttr->getLocation(), diag::note_conflicting_attribute)
+          << ElseAttr << ElseAttr->getRange();
+    }
   }
 
   return BuildIfStmt(IfLoc, IsConstexpr, LParenLoc, InitStmt, Cond, RParenLoc,
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3162,6 +3162,11 @@
    InGroup<IgnoredAttributes>;
 def note_attribute_has_no_effect_on_infinite_loop_here : Note<
   "annotating the infinite loop here">;
+def warn_attribute_has_no_effect_on_if_constexpr : Warning<
+  "attribute %0 has no effect when annotating an 'if constexpr' statement">,
+   InGroup<IgnoredAttributes>;
+def note_attribute_has_no_effect_on_if_constexpr_here : Note<
+  "annotating the 'if constexpr' statement here">;
 def err_decl_attribute_invalid_on_stmt : Error<
   "%0 attribute cannot be applied to a statement">;
 def err_stmt_attribute_invalid_on_decl : Error<


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D90336.302101.patch
Type: text/x-patch
Size: 4258 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201031/1b017e54/attachment-0001.bin>