Skip to content

Comments

Fix Issue 13738 - RTInfo generation on deprecated struct causes deprecation message#4147

Merged
WalterBright merged 1 commit intodlang:masterfrom
rainers:issue13738
Nov 22, 2014
Merged

Fix Issue 13738 - RTInfo generation on deprecated struct causes deprecation message#4147
WalterBright merged 1 commit intodlang:masterfrom
rainers:issue13738

Conversation

@rainers
Copy link
Member

@rainers rainers commented Nov 16, 2014

assume aggregate to be non-deprecated during RTInfo generation

https://issues.dlang.org/show_bug.cgi?id=13738

src/struct.c Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know doing things like this works, but in my experience tweaking global settings like this usually turn out badly, causing unexpected bugs popping up in other places. Please, there's got to be a better way!

(For a primo example, the gagging system in dmd. It's awful, and has been the cause of endless problems and bugs.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps a better plan is to recognize RTInfo as special and omit the message where it is generated, rather than here with a global setting.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not really a "global" setting, but modifies the current type. I agree that the solution is non-optimal.

I tried to figure if there is a way to detect running RTInfo analysis in checkDeprecated, but one problem is that a new scope is created when evaluating template arguments (https://github.com/D-Programming-Language/dmd/blob/master/src/template.c#L5998).

The problem mentioned in the comment is: if we mute messages during analysis of RTInfo, it might silently instantiate templates that use (other) deprecated types. If these template instances are used in other parts of the program later, they will be reused without ever producing the deprecation message. The implementation here restricts muting to the types that RTInfo is currently generated for.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please amend the comment to reflect the better explanation?

@rainers
Copy link
Member Author

rainers commented Nov 17, 2014

Can you please amend the comment to reflect the better explanation?

Done.

Thinking about it again, it might be less invasive to add a bool "mutedeprecated" that is read only in DSymbol::checkDeprecated. That way any other semantic analysis is unaffected (e.g. inheriting deprecation from base class).

@rainers
Copy link
Member Author

rainers commented Nov 18, 2014

Thinking about it again, it might be less invasive to add a bool "mutedeprecated" that is read only in DSymbol::checkDeprecated. That way any other semantic analysis is unaffected (e.g. inheriting deprecation from base class).

I pushed a new version that implements this.

@rainers
Copy link
Member Author

rainers commented Nov 20, 2014

@braddr I very often see the FreeBSD testers fail like with this test: they are waiting at the end of the successful druntime tests until the timeout of 20 minutes expires. I tried to reproduce this in a FreeBSD VM, but it never happened for me.
I suspect that make is waiting for parallel tasks to join at the end but hits some problems. Could you try changing the tester to invoke make without -j for druntime?

@braddr
Copy link
Member

braddr commented Nov 20, 2014

@WalterBright
Copy link
Member

Auto-merge toggled on

WalterBright added a commit that referenced this pull request Nov 22, 2014
Fix Issue 13738 - RTInfo generation on deprecated struct causes deprecation message
@WalterBright WalterBright merged commit 3ba4b64 into dlang:master Nov 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants