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
Add check that 'struct ModuleInfo' exists in object.d #1925
Conversation
@@ -40,6 +40,7 @@ | |||
extern bool obj_includelib(const char *name); | |||
void obj_startaddress(Symbol *s); | |||
void obj_lzext(Symbol *s1,Symbol *s2); | |||
void ObjectNotFound(Identifier *id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should go into declaration.h
.
Otherwise fine |
OK, updated. |
Add check that 'struct ModuleInfo' exists in object.d
#if MODULEINFO_IS_STRUCT | ||
if (id == Id::ModuleInfo) | ||
{ if (Module::moduleinfo) | ||
Module::moduleinfo->error("only object.d can define this reserved struct name"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a breaking language chance and needs updating on the website.
Actually, why isn't the compiler just looking for ModuleInfo
in object
only?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As with the others (in class.c) - it assumes that object.d is the always the first module processed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will also note that before MODULEINFO_IS_STRUCT
it was an error to define class ModuleInfo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In D2, D2, declaring struct ModuleInfo
had not been rejected. Now it would raise an error "struct test.ModuleInfo only object.d can define this reserved struct name" and we should say it is be a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, developing 2.063 is entered the beta phase. I think this breaking change is not enough reasonable. We should try to fix the regression, and if it is impossible, we should revert this change.
I'll work on this tomorrow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opened #1932 to fix the regression.
Makes Module::moduleinfo an AggregateDeclaration, sets it's value if found in object.d, and checks that it exists before generating the moduleinfo.
Knowing what type 'struct ModuleInfo' will be helpful for gdc in some related fixes in porting to ARM.