Remove Expr sugar decorating the CXXUuidofExpr node.

[void]

Summary

Sugar, like ConstantExpr, causes an infinite expansion of the template object.

[aaron.ballman]

lib/Sema/SemaTemplate.cpp

6311

IgnoreParenImpCasts() or are the paren expressions of value?

test/CodeGenCXX/pr40395.cpp

1 ↗

(On Diff #183149)

Why is this test in CodeGenCXX instead of SemaCXX?

The RUN line doesn't seem to be testing anything? Should there be a -verify and // expected-no-diagnostics involved?

It might also be a good idea to add some comments that explain the test case.

[void]

lib/Sema/SemaTemplate.cpp

6311

I want to make this a minimal change. Richard was okay with just the implicit casts. I'm not relaly qualified in this part of the code to say that we should also include parentheses in this. I'll leave that up to you and Richard.

https://bugs.llvm.org/show_bug.cgi?id=40395

[void]

Move test to proper place and add "-verify" flag.

[riccibruno]

lib/Sema/SemaTemplate.cpp

6311

A remark which I hope is relevant. Please ignore it if I am off the mark, or if it is obvious.

I was looking at which of the Expr::Ignore* function to use for something else and it seems that IgnoreParenImpCasts() is *not* equivalent to doing IgnoreParens() + IgnoreImpCasts() until reaching a fixed point.

This is very surprising given the name, and given that IgnoreParenCasts() *is* equivalent to doing IgnoreParens() + IgnoreCasts() until reaching a fixed point.

From my notes:

• Expr *IgnoreParenImpCasts() LLVM_READONLY Ought to be IgnoreParens() + IgnoreImpCasts() until fixed point. But actually only IgnoreParens + skip:
  • ImplicitCastExpr
  • MaterializeTemporaryExpr
  • SubstNonTypeTemplateParmExpr

[aaron.ballman]

LGTM!

lib/Sema/SemaTemplate.cpp

6311

I want to make this a minimal change. Richard was okay with just the implicit casts. I'm not relaly qualified in this part of the code to say that we should also include parentheses in this. I'll leave that up to you and Richard.

Ah, thank you for the link, that helps!

I was looking at which of the Expr::Ignore* function to use for something else and it seems that IgnoreParenImpCasts() is *not* equivalent to doing IgnoreParens() + IgnoreImpCasts() until reaching a fixed point.

That is surprising, and good to know!

[rnk]

Thanks for following up!

lib/Sema/SemaTemplate.cpp

case APValue::LValue: {
       assert(ParamType->isPointerType() || ParamType->isReferenceType() ||
              ParamType->isNullPtrType());
       // -- a temporary object
       // -- a string literal
       // -- the result of a typeid expression, or
       // -- a predefined __func__ variable
       if (auto *E = Value.getLValueBase().dyn_cast<const Expr*>()) {
         if (isa<CXXUuidofExpr>(E)) {
-          Converted = TemplateArgument(ArgResult.get());
+          Converted = TemplateArgument(ArgResult.get()->IgnoreImpCasts());
           break;
         }
         Diag(Arg->getBeginLoc(), diag::err_template_arg_not_decl_ref)
             << Arg->getSourceRange();
         return ExprError();
       }
       auto *VD = const_cast<ValueDecl *>(
           Value.getLValueBase().dyn_cast<const ValueDecl *>());

test/SemaCXX/PR40395.cpp

+// RUN: %clang_cc1 -std=c++17 -fms-extensions -triple=x86_64-pc-win32 -verify %s
+// expected-no-diagnostics
+
+// PR40395 - ConstantExpr shouldn't cause the template object to infinitely
+// expand.
+struct _GUID {};
+struct __declspec(uuid("{AAAAAAAA-AAAA-AAAA-AAAA-AAAAAAAAAAAA}")) B {};
+
+template <const _GUID* piid>
+struct A {
+  virtual void baz() { A<piid>(); }
+};
+
+void f() {
+  A<&__uuidof(B)>();
+}