Fix 21218 - protection attributes should be emitted to C++ headers #11804
Conversation
|
Thanks for your pull request and interest in making D better, @MoonlightSentinel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla references
Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + dmd#11804" |
MoonlightSentinel
force-pushed
the
dtoh-protection
branch
from
8287296 to
c3ed802
Compare
|
Try mapping package to c++ namespace? |
|
How would that work given that C++ doesn't allow namespaces inside aggregates? EDIT: They also affect the mangling so using the generated header would result in linker errors. |
| @@ -15,6 +15,7 @@ template <typename T> | |||
| struct A | |||
| { | |||
| // Ignoring var x alignment 0 | |||
| public: | |||
There was a problem hiding this comment.
Arguably redundant to have public on a struct.
There was a problem hiding this comment.
Yeah, that's a bug caused by the current template handling (gonna fix that in a later PR).
src/dmd/frontend.h
Outdated
| @@ -6496,6 +6559,7 @@ struct Target | |||
| bool isReturnOnStack(TypeFunction* tf, bool needsThis); | |||
| uint64_t parameterSize(const Loc& loc, Type* t); | |||
| void applyInRefParams(TypeFunction* tf); | |||
| private: | |||
| enum class TargetInfoKeys | |||
There was a problem hiding this comment.
Rather than marking it private, I guess it shouldn't be emitted at all. (Though there could be something public that depends on it).
There was a problem hiding this comment.
Probably better to emit it just in case. We could detect if they are used outside for public declarations (but im not sure if that is worth it....)
There was a problem hiding this comment.
Granted that private has two different meanings in C++ and D, I'd probably just restrict the use of private to just fields. Final functions (do not appear in the vtable) that are also private should not be present anywhere. Probably the same applies to private type declarations.
MoonlightSentinel
force-pushed
the
dtoh-protection
branch
3 times, most recently
from
7e50b79 to
66f1c22
Compare
MoonlightSentinel
force-pushed
the
dtoh-protection
branch
2 times, most recently
from
224a5b6 to
d999c2f
Compare
|
@MoonlightSentinel - looks like some tests are still failing, so I'll hold back until they're resolved. ;-) |
MoonlightSentinel
force-pushed
the
dtoh-protection
branch
from
d999c2f to
87c0a34
Compare
|
None of those were caused by this PR ;-) (Rebased and it should be green now aside from the known Azure failure) |
MoonlightSentinel
force-pushed
the
dtoh-protection
branch
from
87c0a34 to
7cef378
Compare
Properly track the current protection while writing struct/class members and add `public:`, `protected:` or `private:` as necessary. `package` and `package(...)` is currently mapped to `protected` as there is no real equivalent in C++.
MoonlightSentinel
force-pushed
the
dtoh-protection
branch
from
7cef378 to
d920cdd
Compare
Properly track the current protection while writing struct/class members and add
public:,protected:orprivate:as necessary.packageandpackage(...)is currently mapped toprotectedas there is no real equivalent in C++, better suggestions are welcome.