Avoid pure macro collisions in LLVM attribute detection by ruiji-shi · Pull Request #9074 · microsoft/DirectXShaderCompiler

ruishi_msft (ruiji-shi) · 2026-10-27T17:58:38Z

Avoid `pure` macro collisions in LLVM attribute detection

Windows PDB headers can define pure as = 0. If such a header is included first, Clang expands that macro inside __has_attribute(pure) and reports:

builtin feature check macro requires a parenthesized identifier
missing ')' after '='

Clang and GCC recognize __pure__ as an equivalent spelling of the attribute. Using that reserved spelling avoids expanding the caller's pure macro. The existing LLVM_READONLY definition already emits __attribute__((__pure__)), so the emitted attribute and runtime behavior are unchanged.

Validation

Local OS build with MSVC and Clang-cl.

Query __has_attribute(__pure__) so a caller's pure macro is not expanded into the builtin feature check. Keep the existing LLVM_READONLY attribute unchanged. Add a SupportTests regression for the PDB-style pure macro collision. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08f9ee45-220b-41eb-a6ff-c21beb0fcdf9

azure-pipelines · 2026-10-27T17:58:48Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

Remove the standalone CompilerTest.cpp regression and its SupportTests registration as requested. The net change against upstream remains only the __has_attribute(__pure__) query. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08f9ee45-220b-41eb-a6ff-c21beb0fcdf9

Copilot

🟡 Changes recommended

The include-order fix needs an automated regression test that fails with the original spelling.

1 open issue

Add Clang regression test for pure macro include-order failure · New

What changed in this PR

This PR fixes an include-order compatibility issue in LLVM’s attribute detection without changing the attribute emitted by LLVM_READONLY.

Changes:

Uses __has_attribute(__pure__) to avoid collisions with a caller-defined pure macro.
Preserves the existing read-only attribute behavior.

Please add an automated regression test covering the conflicting macro before header inclusion.

File	Description
include/llvm/Support/Compiler.h	Uses the reserved attribute spelling to avoid macro expansion.

_{Review effort: Balanced}

ruishi_msft (ruiji-shi) · 2026-10-27T21:14:08Z

 #endif

-#if __has_attribute(pure) || defined(__GNUC__)
+#if __has_attribute(__pure__) || defined(__GNUC__)


This is fixed by adding a new test.

I'm actually of the opinion that a test is unnecessary here: it's very unlikely that someone sees __pure__ in two places and goes "nah, one of them should be pure instead" and revert your change.

But, up to you.

Make sense, removed

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08f9ee45-220b-41eb-a6ff-c21beb0fcdf9

Copilot

🟢 Approval recommended

The narrow fix preserves attribute semantics and includes regression coverage for the reported macro collision.

1 open issue

Add Clang regression test for pure macro include-order failure

_{Review effort: Balanced}

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08f9ee45-220b-41eb-a6ff-c21beb0fcdf9

Copilot

🟢 Approval recommended

The localized fix preserves attribute semantics and includes regression coverage for the macro collision, with no unresolved findings.

1 open issue

Add Clang regression test for pure macro include-order failure

_{Review effort: Balanced}

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08f9ee45-220b-41eb-a6ff-c21beb0fcdf9

Copilot

🟢 Approval recommended

No active review issues remain for the macro-safe attribute detection change.

1 open issue

Add Clang regression test for pure macro include-order failure

_{Review effort: Balanced}

Damyan Pepper (damyanp) · 2026-10-28T17:19:38Z

/azp run

azure-pipelines · 2026-10-28T17:20:04Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Damyan Pepper (damyanp) · 2026-10-28T17:20:40Z

Can you explain some more about the motivation for making this change please? What scenario is this addressing? Has a similar change been made in upstream llvm?

Alex Sepkowski (alsepkow)

LGTM. Assuming you will address Damyan Pepper (@damyanp) s concern

We should also make this change in LLVM. You can help justify the change there with this documentation that suggests the same: https://clang.llvm.org/docs/LanguageExtensions.html#has-attribute

ruishi_msft (ruiji-shi) · 2026-10-29T21:13:32Z

Can you explain some more about the motivation for making this change please? What scenario is this addressing? Has a similar change been made in upstream llvm?

Yes, there is an error in one of the OS.2020 directory, it fails during Clang PCH compilation, where that PCH include xxx\dxil\include\llvm\Support\Compiler.h:162

The root cause is that the pure was defined as = 0 in the PCH within another pdb.h included earlier than Compiler.h. That macro leaks into subsequent LLVM includes, turning __has_attribute(pure) into invalid __has_attribute(= 0).

There is a way to fix in PCH and not let the pure definition get leaked. However, after discussion with Alex offline, it is better to fix with the change in this PR and upstream too in LLVM. The reason is that someone can still defines pure in another header and breaks the build. And __pure__ syntax is also better to avoid collision as shared by Alex https://clang.llvm.org/docs/LanguageExtensions.html#has-attribute. Feel free to let me know if any question.

I keep this PR since LLVM does not ship into this DXC. I will create another PR in LLVM so future modern DXC can consume it.

github-project-automation Bot added this to HLSL Roadmap Oct 27, 2026

github-project-automation Bot moved this to New in HLSL Roadmap Oct 27, 2026

ruishi_msft (ruiji-shi) marked this pull request as ready for review October 27, 2026 18:54

Copilot AI balanced review requested due to automatic review settings October 27, 2026 18:54

Copilot started reviewing on behalf of ruishi_msft (ruiji-shi) October 27, 2026 18:54

View session

Copilot AI reviewed Oct 27, 2026

View reviewed changes

Winston Liu (winstliu) approved these changes Oct 27, 2026

View reviewed changes

address PR feedback

e7ea002

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08f9ee45-220b-41eb-a6ff-c21beb0fcdf9

Copilot AI balanced review requested due to automatic review settings October 27, 2026 20:59

Copilot started reviewing on behalf of ruishi_msft (ruiji-shi) October 27, 2026 20:59

View session

Copilot AI reviewed Oct 27, 2026

View reviewed changes

address PR feedback

3c07b1e

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08f9ee45-220b-41eb-a6ff-c21beb0fcdf9

Copilot AI balanced review requested due to automatic review settings October 27, 2026 21:06

Copilot started reviewing on behalf of ruishi_msft (ruiji-shi) October 27, 2026 21:06

View session

Copilot AI reviewed Oct 27, 2026

View reviewed changes

ruishi_msft (ruiji-shi) and others added 2 commits October 27, 2026 14:12

Fix format error

1c94db5

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08f9ee45-220b-41eb-a6ff-c21beb0fcdf9

Remove new test

d7d6c5d

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08f9ee45-220b-41eb-a6ff-c21beb0fcdf9

Copilot AI balanced review requested due to automatic review settings October 28, 2026 17:06

Copilot started reviewing on behalf of ruishi_msft (ruiji-shi) October 28, 2026 17:06

View session

Copilot AI reviewed Oct 28, 2026

View reviewed changes

Alex Sepkowski (alsepkow) approved these changes Oct 29, 2026

View reviewed changes

ruishi_msft (ruiji-shi) mentioned this pull request Oct 30, 2026

[Support] Avoid pure macro expansion in attribute detection llvm/llvm-project#237311

Draft

Damyan Pepper (damyanp) merged commit f9fc72a into microsoft:main Oct 30, 2026
12 checks passed

github-project-automation Bot moved this from New to Done in HLSL Roadmap Oct 30, 2026

📋 Server Request & Proxy Logs

⚙️ Pengaturan Proxy Target

Conversation

ruishi_msft (ruiji-shi) commented Oct 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!