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

Implement MSVC's language extension to form pointers to members of virtual bases #16085

Closed
rnk opened this issue Apr 9, 2013 · 7 comments
Closed
Labels
bugzilla Issues migrated from bugzilla c++

Comments

@rnk
Copy link
Collaborator

rnk commented Apr 9, 2013

Bugzilla Link 15713
Resolution FIXED
Resolved on Dec 02, 2019 13:50
Version unspecified
OS Windows NT
Blocks #14079
CC @XVilka,@DougGregor,@nico,@timurrrr

Extended Description

Clang currently rejects this program, as the standard says it is ill-formed in [conv.mem], I think:

struct A { int a; };
struct B : virtual A { int b; };
int B::*memptr_b = &B::a; // error

memptr_data_virtual.cpp:8:20: error: conversion from pointer to member of class 'A' to pointer to member of class 'B' via virtual base 'A' is not allowed
int B::*memptr_b = &B::a; // error

MSVC accepts this program, and has a whole bunch of machinery baked into the ABI to support it. Data member pointers can be formed with up to 3 (!) i32s.

There's basically no way to implement this within the Itanium ABI since data memptrs there are always just one offset, so this would only be in -fms-compatibility (or -fms-extensions, I forget which is which).

Mostly I'm just filing this to have a discussion of record with a decision to implement or not. We should document this as part of http://clang.llvm.org/compatibility.html, probably.

@timurrrr
Copy link
Contributor

I think it's perfectly fine to not support it until we observe a real code that actually uses it and has some very strong reasons to use this bad pattern.

@rnk
Copy link
Collaborator Author

rnk commented Apr 12, 2013

Right, I'm mostly just filing the PR to document the state.

FWIW I think John said it best in the code review. This isn't a bad pattern, it's just a feature that MSVC implemented which was pared down during standardization.

@nico
Copy link
Contributor

nico commented Jun 23, 2015

r240383, r240384 removed a few FIXMEs pointing to this bug. (The bug is still valid though.)

Since we haven't needed this yet, it's probably ok to WontFix it if it's only implementable on Windows.

@XVilka
Copy link

XVilka commented Nov 29, 2019

Is this bug still relevant? Because it is mentioned at the MSVC compatibility page and was updated 4 years ago...

https://clang.llvm.org/docs/MSVCCompatibility.html

@rnk
Copy link
Collaborator Author

rnk commented Dec 2, 2019

Yeah, we implemented this a long time ago.

@rnk
Copy link
Collaborator Author

rnk commented Nov 26, 2021

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

@majnemer
Copy link
Mannequin

majnemer mannequin commented Nov 26, 2021

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

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 4, 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 c++
Projects
None yet
Development

No branches or pull requests

4 participants