Ignore ConstantExpr in IgnoreParens

[rnk]

Summary

This moves it up from IgnoreParenImpCasts to IgnoreParens, so that more
helpers ignore it. For most clients, this ensures that these helpers
behave the same with and without C++17 enabled, which is what appears to
introduce these new expression nodes.

Fixes PR39881

[majnemer]

clang/lib/AST/Expr.cpp

2722–2725 ↗

(On Diff #178781)

Just curious, why not go even further and use FullExpr rather than ConstantExpr?

[rnk]

clang/lib/AST/Expr.cpp

2722–2725 ↗

(On Diff #178781)

That would include MaterializeTemporaryExpr. Do you think we should consider that a noop cast? You could make the argument that it is, but I was aiming to match pre ConstantExpr behavior.

[rsmith]

clang/lib/AST/Expr.cpp

2722–2725 ↗

(On Diff #178781)

MaterializeTemporaryExpr is not a FullExpr. It would include ExprWithCleanups, though, and ExprWithCleanups is almost exclusively used by CodeGen, which probably shouldn't be calling this function.

[rsmith]

clang/lib/AST/Expr.cpp

2694 ↗

(On Diff #178781)

Maybe IgnoreParens should be doing this itself?

[rnk]

clang/lib/AST/Expr.cpp

2694 ↗

(On Diff #178781)

I can do that, it passes tests.

[rnk]

• move to IgnoreParens

[rnk]

Want to stamp this? It's 4pm on the Friday before Christmas, what could go wrong? :)

[rsmith]

LGTM with s/ConstantExpr/FullExpr/.

clang/lib/AST/Expr.cpp

2550 ↗

(On Diff #178960)

Does this pass the tests if you use FullExpr here instead? I don't think we should be treating ConstantExpr and other FullExprs differently.

[rnk]

clang/lib/AST/Expr.cpp

2550 ↗

(On Diff #178960)

It does not. This is the list of test failures:

---

Failing Tests (19):

Clang :: Analysis/cfg-rich-constructors.cpp
Clang :: Analysis/cfg-rich-constructors.mm
Clang :: Analysis/copy-elision.cpp
Clang :: Analysis/dtor.cpp
Clang :: Analysis/inlining/temp-dtors-path-notes.cpp
Clang :: Analysis/inner-pointer.cpp
Clang :: Analysis/malloc-free-after-return.cpp
Clang :: Analysis/missing-bind-temporary.cpp
Clang :: Analysis/temp-obj-dtors-cfg-output.cpp
Clang :: Analysis/temp-obj-dtors-option.cpp
Clang :: Analysis/temporaries.cpp
Clang :: Analysis/use-after-move.cpp
Clang :: CodeGenCXX/condition.cpp
Clang :: CodeGenCXX/temp-order.cpp
Clang :: CodeGenObjC/arc.m
Clang :: SemaCXX/attr-noreturn.cpp
Clang :: SemaCXX/return-noreturn.cpp
Clang :: SemaCXX/uninit-variables-conditional.cpp
Clang :: SemaCXX/warn-consumed-analysis.cpp

I think this shows that there are callers of IgnoreParens looking for an ExprWithCleanups.

cfe/trunk/lib/AST/Expr.cpp

if (GenericSelectionExpr* P = dyn_cast<GenericSelectionExpr>(E)) {
       }
     }
     if (ChooseExpr* P = dyn_cast<ChooseExpr>(E)) {
       if (!P->isConditionDependent()) {
         E = P->getChosenSubExpr();
         continue;
       }
     }
+    if (ConstantExpr *CE = dyn_cast<ConstantExpr>(E)) {
+      E = CE->getSubExpr();
+      continue;
+    }
     return E;
   }
 }
 
 /// IgnoreParenCasts - Ignore parentheses and casts.  Strip off any ParenExpr
 /// or CastExprs or ImplicitCastExprs, returning their operand.
 Expr *Expr::IgnoreParenCasts() {
   Expr *E = this;
if (MaterializeTemporaryExpr *Materialize
       E = Materialize->GetTemporaryExpr();
       continue;
     }
     if (SubstNonTypeTemplateParmExpr *NTTP
                                   = dyn_cast<SubstNonTypeTemplateParmExpr>(E)) {
       E = NTTP->getReplacement();
       continue;
     }
-    if (ConstantExpr *CE = dyn_cast<ConstantExpr>(E)) {
-      E = CE->getSubExpr();
-      continue;
-    }
     return E;
   }
 }
 
 Expr *Expr::IgnoreConversionOperator() {
   if (CXXMemberCallExpr *MCE = dyn_cast<CXXMemberCallExpr>(this)) {
     if (MCE->getMethodDecl() && isa<CXXConversionDecl>(MCE->getMethodDecl()))
       return MCE->getImplicitObjectArgument();

cfe/trunk/test/CodeGenCXX/mangle-ms-templates.cpp

 // RUN: %clang_cc1 -std=c++11 -fms-compatibility-version=19 -emit-llvm %s -o - -fms-extensions -fdelayed-template-parsing -triple=i386-pc-win32 | FileCheck %s
 // RUN: %clang_cc1 -std=c++11 -fms-compatibility-version=19 -emit-llvm %s -o - -fms-extensions -fdelayed-template-parsing -triple=x86_64-pc-win32 | FileCheck -check-prefix X64 %s
+// RUN: %clang_cc1 -std=c++17 -fms-compatibility-version=19 -emit-llvm %s -o - -fms-extensions -fdelayed-template-parsing -triple=i386-pc-win32 | FileCheck %s
+// RUN: %clang_cc1 -std=c++17 -fms-compatibility-version=19 -emit-llvm %s -o - -fms-extensions -fdelayed-template-parsing -triple=x86_64-pc-win32 | FileCheck -check-prefix X64 %s
 
 template<typename T>
 class Class {
  public:
   Class() {}
 };
 
 class Typename { };