Allow constant static members to be used with 'this'

[usaxena95]

Summary

Related issue: https://github.com/llvm/llvm-project/issues/56246

[usaxena95]

only allow static members with this.

[usaxena95]

Add constexpr tests.

[usaxena95]

Format.

[aaron.ballman]

Thanks for this! Can you add some more details to the patch description so it's more clear from there why the patch is necessary? Also, can you add a release note for the fix?

I'm pretty sure the changes here are correct, but I want to take another run at the standards wording to be sure.

clang/lib/AST/ExprConstant.cpp

931

Add the docs?

8740–8742

clang/test/SemaCXX/cxx2a-consteval.cpp

925

You should add a new line back to the end of the file.

[erichkeane]

clang/lib/AST/ExprConstant.cpp

8458

When not in a constant-context, what should we be doing here? Why doesn't that set the variable?

8459

Will this visit end up looking into OTHER things here? I guess I'm concerned about something like:

this->get_some_other_type().static_func() and us skipping the this-> for THAT, despite it not being a static call in that context.

[cor3ntin]

Given this has not made progress in a while, I think we should try to implement https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2280r4.html instead of this approach, as I'm not sure if that's strictly conforming, and a more holistic approach is likely to lead to better results.
What do people think?

[aaron.ballman]

In D132398#4150851, @cor3ntin wrote:

Given this has not made progress in a while, I think we should try to implement https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2280r4.html instead of this approach, as I'm not sure if that's strictly conforming, and a more holistic approach is likely to lead to better results.
What do people think?

P2280R4 was adopted for C++23, so I think we'll need to do the work eventually. It likely makes sense to explore doing that work now, at the very least so we don't make it harder to implement in the future.

clang/lib/AST/ExprConstant.cpp

 #include "clang/AST/APValue.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTDiagnostic.h"
 #include "clang/AST/ASTLambda.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/CurrentSourceLocExprScope.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/OSLog.h"
 #include "clang/AST/OptionalDiagnostic.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/TargetInfo.h"
 #include "llvm/ADT/APFixedPoint.h"
public:
     /// constant value.
     bool InConstantContext;
 
     /// Whether we're checking that an expression is a potential constant
     /// expression. If so, do not fail on constructs that could become constant
     /// later on (such as a use of an undefined global).
     bool CheckingPotentialConstantExpression = false;
 
+    /// TODO: add doc.
+    bool AccessingStaticConstantDataMember = false;
     /// Whether we're checking for an expression that has undefined behavior.
     /// If so, we will produce warnings if we encounter an operation that is
     /// always undefined.
     ///
     /// Note that we still need to evaluate the expression normally when this
     /// is set; this is used when evaluating ICEs in C.
     bool CheckingForUndefinedBehavior = false;
 
bool LValueExprEvaluator::VisitMemberExpr(const MemberExpr *E) {
   if (const VarDecl *VD = dyn_cast<VarDecl>(E->getMemberDecl())) {
     VisitIgnoredBaseExpression(E->getBase());
     return VisitVarDecl(E, VD);
   }
 
   // Handle static member functions.
   if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(E->getMemberDecl())) {
     if (MD->isStatic()) {
+      llvm::SaveAndRestore<bool> StaticMember(Info.AccessingStaticConstantDataMember);
+      if(Info.InConstantContext)
+        Info.AccessingStaticConstantDataMember = true;
       VisitIgnoredBaseExpression(E->getBase());
       return Success(MD);
     }
   }
 
   // Handle non-static data members.
   return LValueExprEvaluatorBaseTy::VisitMemberExpr(E);
 }
 
if (!E->getBlockDecl()->hasCaptures())
       return Success(E);
     return Error(E);
   }
   bool VisitCXXThisExpr(const CXXThisExpr *E) {
     // Can't look at 'this' when checking a potential constant expression.
     if (Info.checkingPotentialConstantExpression())
       return false;
     if (!Info.CurrentCall->This) {
+      if (Info.AccessingStaticConstantDataMember) {
+        return Success(E);
+      }
       if (Info.getLangOpts().CPlusPlus11)
         Info.FFDiag(E, diag::note_constexpr_this) << E->isImplicit();
       else
         Info.FFDiag(E);
       return false;
     }
     Result = *Info.CurrentCall->This;
     // If we are inside a lambda's call operator, the 'this' expression refers

clang/test/SemaCXX/cxx2a-consteval.cpp

 // expected-error@-1 {{integer value -1 is outside the valid range of values [0, 1] for this enumeration type}}
   return (int)eParam;
 }
 
 int test() {
   return testDefaultArgForParam() + testDefaultArgForParam((E)1);
 }
 }
+
+namespace GH56246 {
+struct S {
+    static consteval int hello() { return 1;}
+    static constexpr int world() { return 1;}
+
+    consteval int baz() const { return this->hello();}
+    // Previously these two failed because of the use of `this` in the constant expressions.
+    int foo() const { return this->hello() + this->world(); }
+    constexpr int bar() const { return this->hello() + this->world();}
+} s;
+static_assert(s.bar() == 2);
+static_assert(s.baz() == 1);
+}
 No newline at end of file