Fix Issue 18212 - Usage of cfloat,cdouble,cfloat,ifloat,idouble,ireal shouldn't trigger an error in deprecated code #7647
Conversation
src/dmd/dsymbol.d
Outdated
| * | ||
| * Returns: `true` if any parent scope is deprecated, `false` otherwise` | ||
| */ | ||
| final bool isInDeprecatedScope(Scope* sc) |
There was a problem hiding this comment.
In general I prefer not to split things out into functions unless they have more than one caller, or the caller is excessively large. Also, in general, I think this refactoring should be in a separate PR as it is not essential to the bug-fix at hand, and should be argued on its own individual merits.
There was a problem hiding this comment.
Another problem with this method is it's modifying the public interface of the class, yet there is no public caller or potential caller. If it is decided to keep this, it should probably be in its own private static function within the module.
There was a problem hiding this comment.
There are two callers: checkComplexTransition and checkDeprecated - it just felt wrong to duplicate code that has a certain functionality.
It might get changed or improved later and then the duplication won't get an update.
There was a problem hiding this comment.
Gah! Sorry. Totally missed that. You're right on here.
There was a problem hiding this comment.
I believe this also needs to be reflected in the C++ interface dsymbo.h
| * loc = The source location. | ||
| */ | ||
| final void checkComplexTransition(Loc loc) | ||
| final bool checkComplexTransition(Loc loc, Scope* sc) |
There was a problem hiding this comment.
C++ interface change. See mtype.h.
I thought we only need to add methods to the header files that are used by the LDC or GDC? |
wilzbach
force-pushed
the
fix-18212
branch
2 times, most recently
from
3fb6a19 to
b2e1245
Compare
src/dmd/dscope.d
Outdated
| * | ||
| * Returns: `true` if this or any parent scope is deprecated, `false` otherwise` | ||
| */ | ||
| extern(D) bool isDeprecated() |
There was a problem hiding this comment.
Isn't extern(D) the default? I think that is redundant.
UPDATE: Actually I think this should be extern(C++) if it is part of the public interface. See the other methods in struct Scope. And then I think this method needs to be added to scope.h.
It is also my understanding that we only need to maintain the C++ interface for GDC and LDC, but this looks like part of the public interface that they will consume.
Seems logical. |
… shouldn't trigger an error in deprecated code
|
Somehow DAutoTest is under heavy stress recently and it hasn't picked up this PR :/ |
Maybe less build steps and less clones from scratch, eh? ;) I'm not sure it's a question of computing resources - it's already running on a tmpfs, and the server is essentially never CPU-starved. |
5 participants
The following still triggers a deprecation warning:
In other words a
crealin a FuncDeclaration doesn't check its function yet.However, this is good enough to push the complex transition at Phobos as
autoworks just fine.
See also: #7640