Skip to content

HIP ramifications: rename language_cplus to language_cplus_#112

Open
amd-shahab wants to merge 1 commit into
amd-stagingfrom
users/shvahedi/lang-cplus-rename
Open

HIP ramifications: rename language_cplus to language_cplus_#112
amd-shahab wants to merge 1 commit into
amd-stagingfrom
users/shvahedi/lang-cplus-rename

Conversation

@amd-shahab

@amd-shahab amd-shahab commented May 7, 2026

Copy link
Copy Markdown
Contributor

After the introduction of

63ebd1a gdb/language: add "is_cplus_dialect ()" check
8d269b6 gdb/language: add "strip_cplus_dialect ()"

that for the benefit of the HIP language, try to group C++-like
languages together, there can easily be an oversight when
upstream changes introduce new instances of language_cplus
into the code base. This patch renames the (downstream)
language_cplus, so that such new instances are caught by build
failures.

When the build fails, given the context, there are 3 possible
outcomes:

  1. Using "is_cplus_dialect (...)" instead
  2. Using "strip_cplus_dialect (...)", followed by a comparison
    against "language_cplus_" value
  3. Renaming it to "language_cplus_"

After this point, any instance of "langauge_cplus_" indicates
that we have already examined the situation and it is supposed
to be that way.

The "language_cplus" instances in the comments are not touched
for two reasons:

  1. Less amount of changes to avoid likely conflicts in the future
  2. When we will go back upstream and have to undo this patch,
    there will be a few less things to take care of.

@amd-shahab amd-shahab requested review from lancesix and palves May 7, 2026 03:51
@amd-shahab amd-shahab self-assigned this May 7, 2026
@amd-shahab amd-shahab requested a review from a team as a code owner May 7, 2026 03:51
@lumachad

lumachad commented May 7, 2026

Copy link
Copy Markdown
Collaborator

Is upstreaming the HIP language support a reasonable alternative to avoid having such a workaround downstream?

@lancesix lancesix left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

the __ in the middle looks like a typo (unless it is just not noticed), I'd probably prefer language_cplus_ so the trailing underscore looks more intentional.

@lancesix

lancesix commented May 7, 2026

Copy link
Copy Markdown
Collaborator

Is upstreaming the HIP language support a reasonable alternative to avoid having such a workaround downstream?

If what we have seems acceptable upstream, sure! It won't be exercised properly until we have debug info that goes with the said language, but maybe upstream can be ok with that.

@lancesix

lancesix commented May 7, 2026

Copy link
Copy Markdown
Collaborator

The change from "language_cplus" to "language__cplus" was preferred over something like "language_cplus_". Otherwise, the output of "git grep language_cplus" could be polluted.

Why would we really care of git grep? The point of the rename is to have the compiler do all the work and let us know if upstream introduces any use of language_cplus that needs to be converted downstream. grepping seems the wrong approach.

@lancesix lancesix closed this May 7, 2026
@lancesix lancesix reopened this May 7, 2026
@amd-shahab

Copy link
Copy Markdown
Contributor Author

Is upstreaming the HIP language support a reasonable alternative to avoid having such a workaround downstream?

upstreaming and this can happen next to each other. even if we begin the process of upstraming today, it can take a while before it gets in. meanwhile, we will have spent more time on trying to catch the new changes.

@amd-shahab

amd-shahab commented May 7, 2026

Copy link
Copy Markdown
Contributor Author

The change from "language_cplus" to "language__cplus" was preferred over something like "language_cplus_". Otherwise, the output of "git grep language_cplus" could be polluted.

Why would we really care of git grep? The point of the rename is to have the compiler do all the work and let us know if upstream introduces any use of language_cplus that needs to be converted downstream. grepping seems the wrong approach.

for analysis purposes. nonetheless, it can be achieved with word markers (git grep 'language_cplus\>').

@amd-shahab amd-shahab force-pushed the users/shvahedi/lang-cplus-rename branch from 1dee972 to 82cf990 Compare May 7, 2026 08:21
@amd-shahab

Copy link
Copy Markdown
Contributor Author
  • language__cplus -> language_cplus_.
  • update the commit message to reflect that
  • reword the commit message about build failures outcomes

@amd-shahab amd-shahab requested a review from lancesix May 7, 2026 08:22
@amd-shahab amd-shahab force-pushed the users/shvahedi/lang-cplus-rename branch from 82cf990 to a10343a Compare May 7, 2026 08:40
@lancesix

lancesix commented May 7, 2026

Copy link
Copy Markdown
Collaborator

The commit message still says rename language_cplus to language__cplus), needs to be kept in sync with the content.

@simark

simark commented May 7, 2026

Copy link
Copy Markdown
Contributor

the __ in the middle looks like a typo (unless it is just not noticed), I'd probably prefer language_cplus_ so the trailing underscore looks more intentional.

Also it's technically reserved to the language implementation:

https://devblogs.microsoft.com/oldnewthing/20230109-00/?p=107685

@amd-shahab amd-shahab force-pushed the users/shvahedi/lang-cplus-rename branch from a10343a to f8b394d Compare May 7, 2026 15:53
@amd-shahab

Copy link
Copy Markdown
Contributor Author
  • update the git commit title

@amd-shahab amd-shahab changed the title HIP ramifications: rename language_cplus to language__cplus HIP ramifications: rename language_cplus to language_cplus_ May 7, 2026
@amd-shahab amd-shahab requested a review from simark May 7, 2026 15:55
@amd-shahab amd-shahab force-pushed the users/shvahedi/lang-cplus-rename branch from f8b394d to f62744b Compare May 7, 2026 19:16
@amd-shahab

Copy link
Copy Markdown
Contributor Author
  • more fixes in the commit message (language__cplus -> language_cplus_).

@amd-shahab amd-shahab force-pushed the users/shvahedi/lang-cplus-rename branch 2 times, most recently from 667ab39 to ed0ff6e Compare May 8, 2026 04:36
@amd-shahab amd-shahab assigned lancesix and unassigned amd-shahab Jun 3, 2026
After the introduction of

  63ebd1a gdb/language: add "is_cplus_dialect ()" check
  8d269b6 gdb/language: add "strip_cplus_dialect ()"

that for the benefit of the HIP language, try to group C++-like
languages together, there can easily be an oversight when
upstream changes introduce new instances of language_cplus
into the code base.  This patch renames the (downstream)
language_cplus, so that such new instances are caught by build
failures.

When the build fails, given the context, there are 3 possible
outcomes:

1) Using "is_cplus_dialect (...)" instead
2) Using "strip_cplus_dialect (...)", followed by a comparison
against "language_cplus_" value
3) Renaming it to "language_cplus_"

After this point, any instance of "langauge_cplus_" indicates
that we have already examined the situation and it is supposed
to be that way.

The "language_cplus" instances in the comments are not touched
for two reasons:

1) Less amount of changes to avoid likely conflicts in the future
2) When we will go back upstream and have to undo this patch,
there will be fewer things to take care of.

Change-Id: I3f380812c2748e7ae7cd1c67cfe45b4bdc25a083
@amd-shahab amd-shahab force-pushed the users/shvahedi/lang-cplus-rename branch from ed0ff6e to a90fb7c Compare June 3, 2026 08:11

@lancesix lancesix left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sorry I completely missed this one. This looks good to me, and inline with what we discussed.

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.

4 participants