DavidSpickett added a reviewer: DavidSpickett.
DavidSpickett added a comment.
I am not familiar with Objective C, especially beyond Apple platforms. Are you
aiming to run all the existing Objective C test cases with this different
runtime or will it be mainly new tests?
(would be very neat if you got all the existing stuff for free)
================
Comment at: lldb/test/CMakeLists.txt:32
+ set(gnustep_info lib/libobjc.so)
+elseif (WIN32 AND EXISTS "C:/Program Files (x86)/libobjc/lib/objc.dll"
+ AND EXISTS "C:/Program Files
(x86)/libobjc/include/objc/runtime.h")
----------------
Does this library work on Windows on Arm (AArch64 Windows)?
Fine not to handle that in this change, just curious. We (Linaro) do have an
lldb bot for it, so it could be added at some point.
================
Comment at: lldb/test/CMakeLists.txt:39
+if (LLDB_TEST_OBJC_GNUSTEP_DIR)
+ message(STATUS "Found GNUstep ObjC runtime:
${LLDB_TEST_OBJC_GNUSTEP_DIR}/${gnustep_info}")
+endif()
----------------
sgraenitz wrote:
> Might be worth a `FindGNUstep.cmake` at some point? (Or is there one
> somewhere?)
>
> Windows default install dir:
> ```
> -- Found GNUstep ObjC runtime: C:/Program Files (x86)/libobjc/lib/objc.dll
> ```
>
> Linux default install dir:
> ```
> -- Found GNUstep ObjC runtime: /usr/local/lib/libobjc.so
> ```
I think we generally use UPPERCASE names for cmake variables.
================
Comment at: lldb/test/Shell/helper/build.py:62
+ default=False,
+ help='Windows and Linux include/link GNUstep libobjc2 for
this build')
+
----------------
How about "Include and link GNUstep libobjc2 (Windows and Linux only)" ? It is
a bit clearer imo.
================
Comment at: lldb/test/Shell/helper/build.py:687
+ args.extend(['-o', obj])
+ args.append(source)
+
----------------
Why did these 2 need to move?
================
Comment at: lldb/test/Shell/lit.cfg.py:27
# by individual lit.local.cfg files in the test subdirectories.
-config.suffixes = ['.test', '.cpp', '.s']
+config.suffixes = ['.test', '.cpp', '.s', '.m']
----------------
There are a couple of .m files in the Shell dir already, I assume the related
tests still pass for those?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146058/new/
https://reviews.llvm.org/D146058
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
