Skip to content

Conversation

@ashgti
Copy link
Contributor

@ashgti ashgti commented Jan 31, 2025

A previous change is triggering a failure due to SOCKET not being defined in IOStream.h. Adjusting the Windows includes to correct the imports and using a more narrow import (winsock2.h vs windows.h).

Also removed a stale comment.

Tested this on an x86_64 wins 11 vm.

This should fix https://lab.llvm.org/buildbot/#/builders/197/builds/1379 and https://lab.llvm.org/buildbot/#/builders/141/builds/5878

A previous change is triggering a failure due to SOCKET not being defined in IOStream.h. Adjusting the Windows includes to correct the imports and using a more narrow import (winsock2.h vs windows.h).

Also removed a stale comment.
@llvmbot
Copy link
Member

llvmbot commented Jan 31, 2025

@llvm/pr-subscribers-lldb

Author: John Harrison (ashgti)

Changes

A previous change is triggering a failure due to SOCKET not being defined in IOStream.h. Adjusting the Windows includes to correct the imports and using a more narrow import (winsock2.h vs windows.h).

Also removed a stale comment.

Tested this on an x86_64 wins 11 vm.

This should fix https://lab.llvm.org/buildbot/#/builders/197/builds/1379 and https://lab.llvm.org/buildbot/#/builders/141/builds/5878


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

1 Files Affected:

  • (modified) lldb/tools/lldb-dap/IOStream.h (+2-7)
diff --git a/lldb/tools/lldb-dap/IOStream.h b/lldb/tools/lldb-dap/IOStream.h
index 74889eb2e5a866..c91b2f717893c8 100644
--- a/lldb/tools/lldb-dap/IOStream.h
+++ b/lldb/tools/lldb-dap/IOStream.h
@@ -10,13 +10,8 @@
 #define LLDB_TOOLS_LLDB_DAP_IOSTREAM_H
 
 #if defined(_WIN32)
-// We need to #define NOMINMAX in order to skip `min()` and `max()` macro
-// definitions that conflict with other system headers.
-// We also need to #undef GetObject (which is defined to GetObjectW) because
-// the JSON code we use also has methods named `GetObject()` and we conflict
-// against these.
-#define NOMINMAX
-#include <windows.h>
+#include "lldb/Host/windows/windows.h"
+#include <winsock2.h>
 #else
 typedef int SOCKET;
 #endif

Copy link
Collaborator

@DavidSpickett DavidSpickett left a comment

Choose a reason for hiding this comment

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

LGTM

@DavidSpickett DavidSpickett merged commit 41910f7 into llvm:main Jan 31, 2025
9 checks passed
JDevlieghere pushed a commit to swiftlang/llvm-project that referenced this pull request Feb 25, 2025
A previous change is triggering a failure due to SOCKET not being
defined in IOStream.h. Adjusting the Windows includes to correct the
imports and using a more narrow import (winsock2.h vs windows.h).

Also removed a stale comment.

Tested this on an x86_64 wins 11 vm.

This should fix https://lab.llvm.org/buildbot/#/builders/197/builds/1379
and https://lab.llvm.org/buildbot/#/builders/141/builds/5878

(cherry picked from commit 41910f7)
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Jun 13, 2025
A previous change is triggering a failure due to SOCKET not being
defined in IOStream.h. Adjusting the Windows includes to correct the
imports and using a more narrow import (winsock2.h vs windows.h).

Also removed a stale comment.

Tested this on an x86_64 wins 11 vm.

This should fix https://lab.llvm.org/buildbot/#/builders/197/builds/1379
and https://lab.llvm.org/buildbot/#/builders/141/builds/5878
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Jul 14, 2025
A previous change is triggering a failure due to SOCKET not being
defined in IOStream.h. Adjusting the Windows includes to correct the
imports and using a more narrow import (winsock2.h vs windows.h).

Also removed a stale comment.

Tested this on an x86_64 wins 11 vm.

This should fix https://lab.llvm.org/buildbot/#/builders/197/builds/1379
and https://lab.llvm.org/buildbot/#/builders/141/builds/5878
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants