Skip to content
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

[WIP] [core] Force compilation error on variable shadow #51669

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

dentiny
Copy link
Contributor

@dentiny dentiny commented Mar 25, 2025

The following code currently compiles.

class Status {
 template <typename... T>
  Status &operator<<(T &&...msg) {
    const std::string shadowed_variable = "hahaha";  // variable shadowing here
    return *this;
  }

std::string shadowed_variable; // data member here
};

It's really error-prune and we should definitely ban it.

@dentiny dentiny added the go add ONLY when ready to merge, run all tests label Mar 25, 2025
@dentiny dentiny requested review from jjyao, edoakes and dayshah March 25, 2025 07:42
Signed-off-by: dentiny <[email protected]>
@dentiny dentiny force-pushed the hjiang/error-on-shadow branch from 35fb5e9 to 5b6f08b Compare March 25, 2025 07:43
@dentiny dentiny changed the title [core] Force compilation error on variable shadow [WIP] [core] Force compilation error on variable shadow Mar 25, 2025
@dentiny dentiny marked this pull request as draft March 25, 2025 09:37
@dentiny dentiny removed request for jjyao, edoakes and dayshah March 25, 2025 09:37
edoakes pushed a commit that referenced this pull request Mar 25, 2025
This PR fixes all variable shadowing for core worker.

Variable shadowing is known to be error prune --- I personally just
spent 20 minute debugging an issue caused by it.
Need to address all issues before
#51669

Signed-off-by: dentiny <[email protected]>
angelinalg pushed a commit to angelinalg/ray that referenced this pull request Mar 25, 2025
This PR fixes all variable shadowing for core worker.

Variable shadowing is known to be error prune --- I personally just
spent 20 minute debugging an issue caused by it.
Need to address all issues before
ray-project#51669

Signed-off-by: dentiny <[email protected]>
dentiny added a commit to dentiny/ray that referenced this pull request Mar 25, 2025
This PR fixes all variable shadowing for core worker.

Variable shadowing is known to be error prune --- I personally just
spent 20 minute debugging an issue caused by it.
Need to address all issues before
ray-project#51669

Signed-off-by: dentiny <[email protected]>
dhakshin32 pushed a commit to dhakshin32/ray that referenced this pull request Mar 27, 2025
This PR fixes all variable shadowing for core worker.

Variable shadowing is known to be error prune --- I personally just
spent 20 minute debugging an issue caused by it.
Need to address all issues before
ray-project#51669

Signed-off-by: dentiny <[email protected]>
Signed-off-by: Dhakshin Suriakannu <[email protected]>
d-miketa pushed a commit to d-miketa/ray that referenced this pull request Mar 28, 2025
This PR fixes all variable shadowing for core worker.

Variable shadowing is known to be error prune --- I personally just
spent 20 minute debugging an issue caused by it.
Need to address all issues before
ray-project#51669

Signed-off-by: dentiny <[email protected]>
srinathk10 pushed a commit that referenced this pull request Mar 28, 2025
This PR fixes all variable shadowing for core worker.

Variable shadowing is known to be error prune --- I personally just
spent 20 minute debugging an issue caused by it.
Need to address all issues before
#51669

Signed-off-by: dentiny <[email protected]>
Signed-off-by: Srinath Krishnamachari <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go add ONLY when ready to merge, run all tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants