Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression in __uuidof support: &__uuidof(X) template parameter turns into __uuidof(X) #25360

Closed
llvmbot opened this issue Sep 29, 2015 · 16 comments
Labels
bugzilla Issues migrated from bugzilla clang Clang issues not falling into any other category

Comments

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 29, 2015

Bugzilla Link 24986
Resolution FIXED
Resolved on Jun 18, 2018 10:10
Version trunk
OS Windows NT
Blocks #14079
Attachments Self-contained reproduction
Reporter LLVM Bugzilla Contributor
CC @compnerd,@majnemer,@JVApen,@nico,@zygoloid,@rnk,@tiagomacarios

Extended Description

The COM headers use pointers to the IID/GUID struct as template parameters, e.g. in the _com_IIID class. This was supported in clang 3.6 but stopped working again in clang 3.7 onwards. The bug is still present in the Trunk, as far as I can tell with http://melpon.org/wandbox

struct __declspec(uuid("00000000-0000-0000-c000-000000000046")) Test {};

template<typename _Interface, const IID* _IID /*= &__uuidof(_Interface)*/> 
class _com_IIID {};

typedef _com_IIID<Test, &__uuidof(Test)> IID_Test

IID_Test is (silently) instantiated not with a IID const* but with the IID itself. The attached example prints the instantiated types and IID_Test is indeed a _com_IIID<Test, __uuidof(Test)> (note the missing & in front of __uuidof)

To reproduce, compile the attached (complete) example with

c++ --std=c++1z -fms-extensions uuid.cpp
@rnk
Copy link
Collaborator

rnk commented Sep 29, 2015

This only reproduces in a non-MSVC environment. It works with clang-cl and does the same thing as MSVC.

@rnk
Copy link
Collaborator

rnk commented Sep 29, 2015

This only reproduces in a non-MSVC environment. It works with clang-cl and
does the same thing as MSVC.

Ignore that, it's not the issue. I wasn't using c++1z, which appears to matter.

@rnk
Copy link
Collaborator

rnk commented Sep 29, 2015

Richard added some uuid handling code for C++1z in r222807. I guess it's not quite right for pointers to uuids.

@rnk
Copy link
Collaborator

rnk commented Sep 29, 2015

David and Richard, I took a look at this, but the fix wasn't obvious. How are the C++1z rules implemented by r222807 supposed to interact with __uuidof?

@llvmbot
Copy link
Collaborator Author

llvmbot commented Sep 30, 2015

Thanks for looking into this so quickly. Apparently I was too adventurous by going C++1z :-)

@llvmbot
Copy link
Collaborator Author

llvmbot commented Mar 9, 2017

What is the status of this bug?

C++1z isn't as experimal as it was when Sebastian first reported the regression.

@zygoloid
Copy link
Mannequin

zygoloid mannequin commented Mar 9, 2017

David and Richard, I took a look at this, but the fix wasn't obvious. How
are the C++1z rules implemented by r222807 supposed to interact with
__uuidof?

It looks like the problem is that we don't have a sensible canonical representation for __uuidof(X) / &__uuidof(X) as a TemplateArgument, and instead fake it up as an Expr* (violating the rule that the Expr* form is non-canonical in the process); the C++1z evaulation path evaluates the expression and represents the uuidof case as the base expression of the lvalue / pointer (losing the '&' in the process).

The quick hack fix would be to change the CXXUuidofExpr case in the C++1z codepath to build a unary '&' expression wrapping the CXXUuidofExpr in the pointer case, but it would be better to fix this by extending TemplateArgument to have a dedicated kind for a canonical __uuidof(X) argument (representing either the lvalue or pointer case, just like the Declaration kind).

@majnemer
Copy link
Mannequin

majnemer mannequin commented Mar 10, 2017

David and Richard, I took a look at this, but the fix wasn't obvious. How
are the C++1z rules implemented by r222807 supposed to interact with
__uuidof?

It looks like the problem is that we don't have a sensible canonical
representation for __uuidof(X) / &__uuidof(X) as a TemplateArgument, and
instead fake it up as an Expr* (violating the rule that the Expr* form is
non-canonical in the process); the C++1z evaulation path evaluates the
expression and represents the uuidof case as the base expression of the
lvalue / pointer (losing the '&' in the process).

The quick hack fix would be to change the CXXUuidofExpr case in the C++1z
codepath to build a unary '&' expression wrapping the CXXUuidofExpr in the
pointer case, but it would be better to fix this by extending
TemplateArgument to have a dedicated kind for a canonical __uuidof(X)
argument (representing either the lvalue or pointer case, just like the
Declaration kind).

This would naturally fall out if we treated __declspec(uuid) the same way MSVC does: by declaring a variable. __uuidof just creates a reference to that variable.

@llvmbot
Copy link
Collaborator Author

llvmbot commented Oct 23, 2017

Sounds good to me :-)

Is there any chance this could be done in a reasonable timeframe?

@tiagomacarios
Copy link
Mannequin

tiagomacarios mannequin commented Jan 31, 2018

I am on trunk an I am hitting this issue. I don't have any "-std=c++??" explicitly set. Any updates on solving it? if I can help somehow let me know.

@llvmbot
Copy link
Collaborator Author

llvmbot commented Feb 3, 2018

*** Bug llvm/llvm-bugzilla-archive#33076 has been marked as a duplicate of this bug. ***

@llvmbot
Copy link
Collaborator Author

llvmbot commented May 13, 2018

I hit similar issue. http://crbug.com/842118 is a downstream issue.
Here is our repro case:

---- foo.cc
typedef struct _GUID {} GUID;

template <const GUID* p>
void f() {
const GUID* q = p;
}

void g() {
f<&__uuidof(0)>();
}

$ clang-cl /c /std:c++17 foo.cc
foo.cc(5,15): error: no viable conversion from 'const _GUID' to 'const GUID *' (aka 'const _GUID ')
const GUID
q = p;
^ ~
foo.cc(9,3): note: in instantiation of function template specialization 'f<__uuidof(0)>' requested here
f<&__uuidof(0)>();
^
1 error generated.

Note that the error message contains 'f<__uuidof(0)>' rather than 'f<&__uuidof(0)>', which looks wrong to me.

On my investigation, something goes wrong around here:
https://github.com/llvm-mirror/clang/blob/1064f0a9daefafcc3bb7b4d30f3a45b3b8770d99/lib/Sema/SemaTemplate.cpp#L6235

  if (auto *E = Value.getLValueBase().dyn_cast<const Expr*>()) {
    if (isa<CXXUuidofExpr>(E)) {
      Converted = TemplateArgument(const_cast<Expr*>(E));
      break;
    }
    Diag(Arg->getLocStart(), diag::err_template_arg_not_decl_ref)
      << Arg->getSourceRange();
    return ExprError();
  }

Where |Value| here is '&__uuidof(0)' and |E| is '__uuidof(0)'. This code path is not used in C++14 or before, and touches __uuidof.

@nico
Copy link
Contributor

nico commented May 17, 2018

r332614, with what I think is the 'quick hack fix' mentioned in comment 7.

@rnk
Copy link
Collaborator

rnk commented Jun 18, 2018

*** Bug llvm/llvm-bugzilla-archive#37824 has been marked as a duplicate of this bug. ***

@llvmbot
Copy link
Collaborator Author

llvmbot commented Nov 26, 2021

mentioned in issue llvm/llvm-bugzilla-archive#33076

@rnk
Copy link
Collaborator

rnk commented Nov 27, 2021

mentioned in issue llvm/llvm-bugzilla-archive#37824

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla clang Clang issues not falling into any other category
Projects
None yet
Development

No branches or pull requests

3 participants