-
-
Notifications
You must be signed in to change notification settings - Fork 369
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 __traits(fullyQualifiedName) #3495
base: master
Are you sure you want to change the base?
Conversation
Thanks for your pull request, @WalterBright! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. |
What's the use case for this? The definition here is not precise enough for much of anything; what kind of string is it? Is it human readable? Compiler readable? Or a mangle? Does it include template arguments to identify the instance or just the name of the associated symbol? (And, of course, there probably isn't much of a real world use case even if it is well defined. Code that uses fully qualified names is almost universally an ugly, buggy mess that would be better by all factors if rewritten with proper techniques.) |
Apparently it's going to flunk DAutoTest until 14711 is merged. |
Replacing |
|
Thumbs down emojis btw are among the least valuable things you can do. Instead of making an argument, you just come by and snipe from the sidelines. But here, I'll do your job for you. You might point to the Phobos unittests which, despite being undocumented behavior, point to a possible answer for what the specification should be. (Of course, it is still completely useless, but at least it'd be some kind of definition.) The phobos unittests try to identify template instances uniquely and reproduce all the details present in the type while expressing them as a kind of pseudo-D source code. The dmd tests copy/paste these, indicating that is probably also the intention here. But... it is pseudo-D, not actual D code, meaning it only actually works for mixin in basic cases (and there's better ways to do mixins than messing with fully qualified names anyway, namely, using local names which are far more reliable, easier to read, and faster to compile), yet people often try to apply it to other things, hence being the code smell. If you wanted something that just worked for mixin, it is possible to define that, but neither Phobos nor dmd does. This would need to be strictly defined output if you wanted to. (But, again, it doesn't enable anything new and encourages anti-patterns.) Or, is the intention to be a debugging aid, like the compiler would produce for error messages? The spec doesn't actually say anything, a "string representation" could be absolutely anything. Combined with the anti-patterns this encourages, such an addition to the language would be a clear net-negative. If the intention is to leave the format of the string as implementation-defined behavior just for error message convenience, this should also be called out so everyone knows what to expect. |
|
Where is FQN used that it causes sludge? That would be a good followup answer to "what is the use case?" |
lemme copy/paste some chat stuff before it scrolls away i think you could define something that does somewhat reliably just works as code to mix in, especially if you used .object.imported!"module.name".other.name consistently (though of course i've never actually seen this be necessary!) tho there are a few iffy things even in the unittest: you only get that if you do typeof(some_function) (note not typeof(&some_function), which is a function pointer type, which is a thing). but if you write it you get an error. so this is why i ask what the use case is: if it is for making error messages vs making mixin code, you are going to define these things a bit differently disambiguating error messages is a possible use. and .... then you probably don't want a fqn per se, you want a as-qualified-as-needed-to-identify name. one of the cases that bugs me is something like knowing that it is a structFromSql instantiated on line 55 of file model.d though, that helps a lot so i think you more often actually want a partially qualified name, either just the name w/o template args, or at least abbreviated template args. eponymous templates too are a bit silly to read, std.algorithm.iteration.map.map doesn't help all that much and the compiler can potentially do some magic to help with this - it has some logic for showing local name vs qualified name already to disambiguate error messages, which it can determine is necessary by looking for name conflicts in scope, some magic it knows better than the lib. (it doesn't always do a good job but it makes an attempt) and if that is the intented use case, we can formalize this definition - without necessarily solidifying the output, to allow for future implementations to improve the algorithm without being a breaking change - and reuse some of the error message generation logic buttttttttt if the intention is something different like using specifically for mixin (which again is something i've never seen code need, every time i see this, i rewrite it with different constructs and get better functionality, cleaner code, and faster builds. it is not a pattern we should encourage), then you probably want overly-verbose, hard to read, but 100% unambiguous even if taken out of context code, e.g. .object.imported!"foo.bar".baz.gz!(.object.imported!"foo.bar".bar, 44UL) and other such horrifying code, but is insulated from the surrounding scope context, cuz it is meant for the compiler to read |
What I meant is the process for generating a FQN out of templates produces a lot of templates and memory consumption that isn't useful for much of anything else. In discussions with users, |
@adamdruppe The intention is not to invent anything new here, but to help existing users. I suggest that debating a better way of doing things would be more apropos in the n.g. than here. |
Maybe you should cuz this is useless and embedding it in the spec like this just adds more cruft to D. you say you want to simplify the language but your actions do the opposite. |
I'd really like to know who these users are. I'll fix their code for them for free. I did a couple just a few weeks ago for people who insisted they needed fullyQualifiedName: http://dpldocs.info/this-week-in-d/Blog.Posted_2022_12_26.html#more-mixin-techniques Notice how the new code is shorter, builds several times faster, and works with more inputs. I've never - not once - seen a case where people said FQN was necessary where they were actually correct about it. It is a misfeature that encourages bad code. |
This is a general problem with templates that needs solving. |
1c2d85b
to
bc481b5
Compare
Yes, and we've all tried to solve that problem. In the meantime, we do what we can to make it faster. |
There have not yet been a single use case described that I haven't been able to debunk in the forum thread. It is like having a function
And someone says it takes 5 seconds more than it has to to do the work, then someone changes the 5.seconds to 3.seconds and calls it a big win, it is almost twice as fast! But... the sleep is totally unnecessary to the function's operation. It shouldn't be there at all. |
@adamdruppe telling people they need to re-engineer their code is not going to work. (It's never ever worked when I've tried it.) People depend on FullyQualifiedName, it makes their compilations slow, so we fix it. |
There's two confirmed users of fullyQualifiedName at this point. I already fixed the code for the third one to answer, it took 15 minutes and improved things by every measure. It isn't a grand reengineering, it really is just using I suspect one of those users would be better served by |
@adamdruppe I'm not using FullyQualifiedName myself. Nor did I add it to Phobos. Thank you for fixing user code, if you can get it to zero I'll be happy to deprecate it. |
It wouldn't need deprecation if it wasn't enshrined in the compiler in the first place. But whatever, I was never under any serious illusion you'd actually listen to me. You should still specify the behavior in the PR though if it is going to be added anyway. The current text doesn't even say what kind of string it is, if it is D code or what. |
Library removals also require deprecation.
If I didn't care about what you thought, I wouldn't have spent the time explaining the rationale to you. Anyhow, the fact is, I have constraints that you do not have. When something in D goes wrong for users, I am always in the front line receiving the flames. When I remove something, there's always someone who gets very unhappy that I broke their code. It really doesn't matter why, just that their code doesn't work anymore. I'm sympathetic to their point of view, I also get annoyed when my old C code won't compile anymore. There are several things I tried to remove from the language, and was stymied every time. |
Complement to dlang/dmd#14711