Skip to content

Conversation

@AaronBallman
Copy link
Collaborator

Microsoft helpfully defines THIS to void in two different platform SDK headers, at least one of which is reachable via <Windows.h>. We have a user who ran into a build because of THIS unfortunate macro name collision.

Rename the members to better match our naming conventions.

Fixes #142186

Microsoft helpfully defines `THIS` to `void` in two different platform
SDK headers, at least one of which is reachable via <Windows.h>. We
have a user who ran into a build because of `THIS` unfortunate macro
name collision.

Rename the members to better match our naming conventions.

Fixes llvm#142186
@AaronBallman AaronBallman requested review from erichkeane, rnk and usx95 May 30, 2025 18:35
@AaronBallman AaronBallman added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" build-problem platform:windows labels May 30, 2025
@llvmbot
Copy link
Member

llvmbot commented May 30, 2025

@llvm/pr-subscribers-platform-windows

Author: Aaron Ballman (AaronBallman)

Changes

Microsoft helpfully defines THIS to void in two different platform SDK headers, at least one of which is reachable via <Windows.h>. We have a user who ran into a build because of THIS unfortunate macro name collision.

Rename the members to better match our naming conventions.

Fixes #142186


Full diff: https://github.com/llvm/llvm-project/pull/142195.diff

5 Files Affected:

  • (modified) clang/include/clang/Basic/Attr.td (+6-4)
  • (modified) clang/lib/Sema/CheckExprLifetime.cpp (+1-1)
  • (modified) clang/lib/Sema/SemaAttr.cpp (+1-1)
  • (modified) clang/lib/Sema/SemaChecking.cpp (+3-3)
  • (modified) clang/lib/Sema/SemaDeclAttr.cpp (+3-3)
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index db02449a3dd12..216084344c00d 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1997,10 +1997,12 @@ private:
   ArrayRef<SourceLocation> ArgLocs;
 
 public:
-  static constexpr int THIS = 0;
-  static constexpr int INVALID = -1;
-  static constexpr int UNKNOWN = -2;
-  static constexpr int GLOBAL = -3;
+  enum ArgIndex {
+    This = 0,
+    Invalid = -1,
+    Unknown = -2,
+    Global = -3,
+  };
 
   void setArgs(ArrayRef<IdentifierInfo*> Idents, ArrayRef<SourceLocation> Locs) {
     assert(Idents.size() == params_Size);
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index 1f87001f35b57..060ba31660556 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -665,7 +665,7 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
                  CanonCallee->getParamDecl(I)->getAttr<LifetimeCaptureByAttr>();
              CaptureAttr && isa<CXXConstructorDecl>(CanonCallee) &&
              llvm::any_of(CaptureAttr->params(), [](int ArgIdx) {
-               return ArgIdx == LifetimeCaptureByAttr::THIS;
+               return ArgIdx == LifetimeCaptureByAttr::This;
              }))
       // `lifetime_capture_by(this)` in a class constructor has the same
       // semantics as `lifetimebound`:
diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp
index 44726c4cea123..c1675a6c67f14 100644
--- a/clang/lib/Sema/SemaAttr.cpp
+++ b/clang/lib/Sema/SemaAttr.cpp
@@ -290,7 +290,7 @@ void Sema::inferLifetimeCaptureByAttribute(FunctionDecl *FD) {
       // pointer-like reference types (`const T&`, `T&&`).
       if (PVD->getType()->isReferenceType() &&
           sema::isGLSPointerType(PVD->getType().getNonReferenceType())) {
-        int CaptureByThis[] = {LifetimeCaptureByAttr::THIS};
+        int CaptureByThis[] = {LifetimeCaptureByAttr::This};
         PVD->addAttr(
             LifetimeCaptureByAttr::CreateImplicit(Context, CaptureByThis, 1));
       }
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 373ca549cb23b..aba39c0eb3299 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -3333,8 +3333,8 @@ void Sema::checkLifetimeCaptureBy(FunctionDecl *FD, bool IsMemberFunction,
   if (!FD || Args.empty())
     return;
   auto GetArgAt = [&](int Idx) -> const Expr * {
-    if (Idx == LifetimeCaptureByAttr::GLOBAL ||
-        Idx == LifetimeCaptureByAttr::UNKNOWN)
+    if (Idx == LifetimeCaptureByAttr::Global ||
+        Idx == LifetimeCaptureByAttr::Unknown)
       return nullptr;
     if (IsMemberFunction && Idx == 0)
       return ThisArg;
@@ -3349,7 +3349,7 @@ void Sema::checkLifetimeCaptureBy(FunctionDecl *FD, bool IsMemberFunction,
     for (int CapturingParamIdx : Attr->params()) {
       // lifetime_capture_by(this) case is handled in the lifetimebound expr
       // initialization codepath.
-      if (CapturingParamIdx == LifetimeCaptureByAttr::THIS &&
+      if (CapturingParamIdx == LifetimeCaptureByAttr::This &&
           isa<CXXConstructorDecl>(FD))
         continue;
       Expr *Capturing = const_cast<Expr *>(GetArgAt(CapturingParamIdx));
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 119ba8486b09f..f34b70ea61a5d 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -4155,7 +4155,7 @@ LifetimeCaptureByAttr *Sema::ParseLifetimeCaptureByAttr(const ParsedAttr &AL,
   }
   if (!IsValid)
     return nullptr;
-  SmallVector<int> FakeParamIndices(N, LifetimeCaptureByAttr::INVALID);
+  SmallVector<int> FakeParamIndices(N, LifetimeCaptureByAttr::Invalid);
   auto *CapturedBy =
       LifetimeCaptureByAttr::Create(Context, FakeParamIndices.data(), N, AL);
   CapturedBy->setArgs(ParamIdents, ParamLocs);
@@ -4198,8 +4198,8 @@ void Sema::LazyProcessLifetimeCaptureByParams(FunctionDecl *FD) {
   if (Attrs.empty())
     return;
   llvm::StringMap<int> NameIdxMapping = {
-      {"global", LifetimeCaptureByAttr::GLOBAL},
-      {"unknown", LifetimeCaptureByAttr::UNKNOWN}};
+      {"global", LifetimeCaptureByAttr::Global},
+      {"unknown", LifetimeCaptureByAttr::Unknown}};
   int Idx = 0;
   if (HasImplicitThisParam) {
     NameIdxMapping["this"] = 0;

@llvmbot
Copy link
Member

llvmbot commented May 30, 2025

@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)

Changes

Microsoft helpfully defines THIS to void in two different platform SDK headers, at least one of which is reachable via <Windows.h>. We have a user who ran into a build because of THIS unfortunate macro name collision.

Rename the members to better match our naming conventions.

Fixes #142186


Full diff: https://github.com/llvm/llvm-project/pull/142195.diff

5 Files Affected:

  • (modified) clang/include/clang/Basic/Attr.td (+6-4)
  • (modified) clang/lib/Sema/CheckExprLifetime.cpp (+1-1)
  • (modified) clang/lib/Sema/SemaAttr.cpp (+1-1)
  • (modified) clang/lib/Sema/SemaChecking.cpp (+3-3)
  • (modified) clang/lib/Sema/SemaDeclAttr.cpp (+3-3)
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index db02449a3dd12..216084344c00d 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1997,10 +1997,12 @@ private:
   ArrayRef<SourceLocation> ArgLocs;
 
 public:
-  static constexpr int THIS = 0;
-  static constexpr int INVALID = -1;
-  static constexpr int UNKNOWN = -2;
-  static constexpr int GLOBAL = -3;
+  enum ArgIndex {
+    This = 0,
+    Invalid = -1,
+    Unknown = -2,
+    Global = -3,
+  };
 
   void setArgs(ArrayRef<IdentifierInfo*> Idents, ArrayRef<SourceLocation> Locs) {
     assert(Idents.size() == params_Size);
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index 1f87001f35b57..060ba31660556 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -665,7 +665,7 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
                  CanonCallee->getParamDecl(I)->getAttr<LifetimeCaptureByAttr>();
              CaptureAttr && isa<CXXConstructorDecl>(CanonCallee) &&
              llvm::any_of(CaptureAttr->params(), [](int ArgIdx) {
-               return ArgIdx == LifetimeCaptureByAttr::THIS;
+               return ArgIdx == LifetimeCaptureByAttr::This;
              }))
       // `lifetime_capture_by(this)` in a class constructor has the same
       // semantics as `lifetimebound`:
diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp
index 44726c4cea123..c1675a6c67f14 100644
--- a/clang/lib/Sema/SemaAttr.cpp
+++ b/clang/lib/Sema/SemaAttr.cpp
@@ -290,7 +290,7 @@ void Sema::inferLifetimeCaptureByAttribute(FunctionDecl *FD) {
       // pointer-like reference types (`const T&`, `T&&`).
       if (PVD->getType()->isReferenceType() &&
           sema::isGLSPointerType(PVD->getType().getNonReferenceType())) {
-        int CaptureByThis[] = {LifetimeCaptureByAttr::THIS};
+        int CaptureByThis[] = {LifetimeCaptureByAttr::This};
         PVD->addAttr(
             LifetimeCaptureByAttr::CreateImplicit(Context, CaptureByThis, 1));
       }
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 373ca549cb23b..aba39c0eb3299 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -3333,8 +3333,8 @@ void Sema::checkLifetimeCaptureBy(FunctionDecl *FD, bool IsMemberFunction,
   if (!FD || Args.empty())
     return;
   auto GetArgAt = [&](int Idx) -> const Expr * {
-    if (Idx == LifetimeCaptureByAttr::GLOBAL ||
-        Idx == LifetimeCaptureByAttr::UNKNOWN)
+    if (Idx == LifetimeCaptureByAttr::Global ||
+        Idx == LifetimeCaptureByAttr::Unknown)
       return nullptr;
     if (IsMemberFunction && Idx == 0)
       return ThisArg;
@@ -3349,7 +3349,7 @@ void Sema::checkLifetimeCaptureBy(FunctionDecl *FD, bool IsMemberFunction,
     for (int CapturingParamIdx : Attr->params()) {
       // lifetime_capture_by(this) case is handled in the lifetimebound expr
       // initialization codepath.
-      if (CapturingParamIdx == LifetimeCaptureByAttr::THIS &&
+      if (CapturingParamIdx == LifetimeCaptureByAttr::This &&
           isa<CXXConstructorDecl>(FD))
         continue;
       Expr *Capturing = const_cast<Expr *>(GetArgAt(CapturingParamIdx));
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 119ba8486b09f..f34b70ea61a5d 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -4155,7 +4155,7 @@ LifetimeCaptureByAttr *Sema::ParseLifetimeCaptureByAttr(const ParsedAttr &AL,
   }
   if (!IsValid)
     return nullptr;
-  SmallVector<int> FakeParamIndices(N, LifetimeCaptureByAttr::INVALID);
+  SmallVector<int> FakeParamIndices(N, LifetimeCaptureByAttr::Invalid);
   auto *CapturedBy =
       LifetimeCaptureByAttr::Create(Context, FakeParamIndices.data(), N, AL);
   CapturedBy->setArgs(ParamIdents, ParamLocs);
@@ -4198,8 +4198,8 @@ void Sema::LazyProcessLifetimeCaptureByParams(FunctionDecl *FD) {
   if (Attrs.empty())
     return;
   llvm::StringMap<int> NameIdxMapping = {
-      {"global", LifetimeCaptureByAttr::GLOBAL},
-      {"unknown", LifetimeCaptureByAttr::UNKNOWN}};
+      {"global", LifetimeCaptureByAttr::Global},
+      {"unknown", LifetimeCaptureByAttr::Unknown}};
   int Idx = 0;
   if (HasImplicitThisParam) {
     NameIdxMapping["this"] = 0;

Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

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

Sure they don't have a #define This somewhere :) ?

Copy link
Collaborator

@tru tru left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@usx95 usx95 left a comment

Choose a reason for hiding this comment

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

LGTM

@AaronBallman AaronBallman merged commit 0adf6e8 into llvm:main May 31, 2025
16 checks passed
@AaronBallman AaronBallman deleted the aballman-msvc-workaround branch May 31, 2025 12:35
AnthonyLatsis added a commit to AnthonyLatsis/swift that referenced this pull request Jun 13, 2025
devajithvs pushed a commit to devajithvs/llvm-project that referenced this pull request Sep 19, 2025
Microsoft helpfully defines `THIS` to `void` in two different platform
SDK headers, at least one of which is reachable via <Windows.h>. We have
a user who ran into a build because of `THIS` unfortunate macro name
collision.

Rename the members to better match our naming conventions.

Fixes llvm#142186

(cherry picked from commit 0adf6e8)
devajithvs pushed a commit to devajithvs/llvm-project that referenced this pull request Sep 25, 2025
Microsoft helpfully defines `THIS` to `void` in two different platform
SDK headers, at least one of which is reachable via <Windows.h>. We have
a user who ran into a build because of `THIS` unfortunate macro name
collision.

Rename the members to better match our naming conventions.

Fixes llvm#142186

(cherry picked from commit 0adf6e8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build-problem clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category platform:windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

'int' followed by 'void' is illegal

5 participants