Skip to content

Fix FUNCFLAG with SCOPE bitmask error#16752

Merged
RazvanN7 merged 1 commit intodlang:masterfrom
dkorpel:fix-func-flag-type-error
Jul 25, 2024
Merged

Fix FUNCFLAG with SCOPE bitmask error#16752
RazvanN7 merged 1 commit intodlang:masterfrom
dkorpel:fix-func-flag-type-error

Conversation

@dkorpel
Copy link
Contributor

@dkorpel dkorpel commented Jul 24, 2024

What the?! 🤨

So in #14846, an extra check was added related to the enclosing function of __traits(compiles):

- (sc.flags & SCOPE.compile) 
+ (sc.flags & SCOPE.compile) && !(sc.func.flags & SCOPE.compile);

However, sc.func.flags is not of type SCOPE, it's FUNCFLAGS.

SCOPE.compile is 0x100 and by sheer coincidence, it just happened to line up with FUNCFLAG.isCompileTimeOnly.
That flag was introduced in #7897, and there seems no reason why it should have the same mask as SCOPE.compile.

Then, when the isCompileTimeOnly flag was removed in #14847, it lined up with the equivalent FUNCFLAG.skipCodegen. Only when I tried to remove a flag before it in #16751 did I notice very confusing breakage in __traits(compiles).

There's barely any use for the FuncDeclaration.flags property anymore, so I renamed it to avoid future confusion.

@RazvanN7 since you wrote the code, can you verify whether it indeed was magically correct all along?

@dkorpel dkorpel added the Severity:Refactoring No semantic changes to code label Jul 24, 2024
@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @dkorpel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

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

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#16752"

@WalterBright
Copy link
Member

If D had bitfields, and dmd used them for bit flags, this error wouldn't have happened.

@RazvanN7
Copy link
Contributor

Wow... just wow.

@RazvanN7
Copy link
Contributor

I call it an honest mistake and have to agree with @WalterBright here.

@RazvanN7 RazvanN7 merged commit 83adb8a into dlang:master Jul 25, 2024
@dkorpel dkorpel deleted the fix-func-flag-type-error branch July 25, 2024 10:47
@tgehr
Copy link
Contributor

tgehr commented Jul 25, 2024

If D had bitfields, and dmd used them for bit flags, this error wouldn't have happened.

There are more direct causes:
https://forum.dlang.org/post/qu8def$2b08$1@digitalmars.com

Also, whether D has built-in bitfields and whether DMD uses typesafe flags are orthogonal questions.
Anyway, I'd be happy to see well-designed bitfields in D.

@dkorpel
Copy link
Contributor Author

dkorpel commented Jul 25, 2024

If D had bitfields, and dmd used them for bit flags, this error wouldn't have happened.

It's a good thing D already has bitfields then. Now we just need to make dmd use them!
#16753

thewilsonator pushed a commit to thewilsonator/dmd that referenced this pull request Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Severity:Refactoring No semantic changes to code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants