Disable warnings related to anonymous types in the ObjC plugin

[vsk]

Summary

This part of lldb make use of anonymous structs and unions. The usage is
idiomatic and doesn't deserve a warning. Logic in the NSDictionary and NSSet
plugins use anonymous structs in a manner consistent with the relevant Apple
frameworks.

[davide]

LGTM, I stumbled upon the same issue :)
An alternative would be that of naming (as you suggested), but if this is consistent, no worries.

[labath]

If the "excuse" for not following llvm here is that these structs mirror apple headers, should we restrict these warnings only to the relevant files (e.g. everything in source/Plugins/Language/ObjC)?

I don't particularly care about whether we do, but I wanted to throw the idea out there, as this is the reason why I haven't done something similar yet (although the warnings are quite annoying).

[vsk]

In D40757#943483, @labath wrote:

If the "excuse" for not following llvm here is that these structs mirror apple headers, should we restrict these warnings only to the relevant files (e.g. everything in source/Plugins/Language/ObjC)?

I don't particularly care about whether we do, but I wanted to throw the idea out there, as this is the reason why I haven't done something similar yet (although the warnings are quite annoying).

I'd like to make this a narrower change as well, but unfortunately, I haven't found a way to add in extra CXX flags through the add_library or llvm_add_library utilities. Somebody tried to do this once in AddLLDB.cmake (see "set(CMAKE_CXX_FLAGS ...)"), but it's busted, and does not do anything, AFAICT.

In this case, I think it should be safe to disable these warnings project-wide, because they're not often (ever?) immediate indicators of correctness issues / undefined behavior. Given all of that, @labath are you OK with the patch as-is?

cmake/modules/LLDBConfig.cmake

241 ↗

(On Diff #125241)

I should point out that there's a typo here -- I've fixed this so it's spelled CXX_SUPPORTS_NO_NESTED_ANON_TYPES locally.

[labath]

In D40757#944293, @vsk wrote:

I'd like to make this a narrower change as well, but unfortunately, I haven't found a way to add in extra CXX flags through the add_library or llvm_add_library utilities. Somebody tried to do this once in AddLLDB.cmake (see "set(CMAKE_CXX_FLAGS ...)"), but it's busted, and does not do anything, AFAICT.

Yeah, I believe the correct cmake incantation in this case should be:

target_compile_options(lldbPluginObjCLanguage PRIVATE -Wno-nested-anon-types -Wno-gnu-anonymous-struct)

(placed just after add_lldb_library)

In this case, I think it should be safe to disable these warnings project-wide, because they're not often (ever?) immediate indicators of correctness issues / undefined behavior. Given all of that, @labath are you OK with the patch as-is?

I'm fine with this as well, but since we both agree that a narrower scope would be better, maybe you could try the above snippet instead.

[vsk]

• Thanks @labath! I've disabled the GNU warnings in a narrower way. PTAL.

[labath]

Cool. Thanks.

[davide]

LGTM (again)

lldb/trunk/cmake/modules/AddLLDB.cmake

 function(add_lldb_library name)
   # only supported parameters to this macro are the optional
   # MODULE;SHARED;STATIC library type and source files
   cmake_parse_arguments(PARAM
     "MODULE;SHARED;STATIC;OBJECT;PLUGIN"
     ""
-    "DEPENDS;LINK_LIBS;LINK_COMPONENTS"
+    "EXTRA_CXXFLAGS;DEPENDS;LINK_LIBS;LINK_COMPONENTS"
     ${ARGN})
   llvm_process_sources(srcs ${PARAM_UNPARSED_ARGUMENTS})
   list(APPEND LLVM_LINK_COMPONENTS ${PARAM_LINK_COMPONENTS})
 
   if(PARAM_PLUGIN)
     set_property(GLOBAL APPEND PROPERTY LLDB_PLUGINS ${name})
   endif()
 
else ()
     # PARAM_STATIC or library type unspecified. BUILD_SHARED_LIBS
     # does not control the kind of libraries created for LLDB,
     # only whether or not they link to shared/static LLVM/Clang
     # libraries.
     set(libkind STATIC)
   endif()
 
   #PIC not needed on Win
+  # FIXME: Setting CMAKE_CXX_FLAGS here is a no-op, use target_compile_options
+  # or omit this logic instead.
   if (NOT WIN32)
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fPIC")
   endif()
 
   if (PARAM_OBJECT)
     add_library(${name} ${libkind} ${srcs})
   else()
     llvm_add_library(${name} ${libkind} ${srcs} LINK_LIBS
else()
     endif()
   endif()
 
   # Hack: only some LLDB libraries depend on the clang autogenerated headers,
   # but it is simple enough to make all of LLDB depend on some of those
   # headers without negatively impacting much of anything.
   add_dependencies(${name} clang-tablegen-targets)
 
+  # Add in any extra C++ compilation flags for this library.
+  target_compile_options(${name} PRIVATE ${PARAM_EXTRA_CXXFLAGS})
+
   set_target_properties(${name} PROPERTIES FOLDER "lldb libraries")
 endfunction(add_lldb_library)
 
 function(add_lldb_executable name)
   cmake_parse_arguments(ARG
     "INCLUDE_IN_FRAMEWORK;GENERATE_INSTALL"
     ""
     "LINK_LIBS;LINK_COMPONENTS"

lldb/trunk/cmake/modules/LLDBConfig.cmake

 endif ()
 
 check_cxx_compiler_flag("-Wno-vla-extension"
                         CXX_SUPPORTS_NO_VLA_EXTENSION)
 if (CXX_SUPPORTS_NO_VLA_EXTENSION)
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-vla-extension")
 endif ()
 
+check_cxx_compiler_flag("-Wno-gnu-anonymous-struct"
+                        CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+
+check_cxx_compiler_flag("-Wno-nested-anon-types"
+                        CXX_SUPPORTS_NO_NESTED_ANON_TYPES)
+
 # Disable MSVC warnings
 if( MSVC )
   add_definitions(
     -wd4018 # Suppress 'warning C4018: '>=' : signed/unsigned mismatch'
     -wd4068 # Suppress 'warning C4068: unknown pragma'
     -wd4150 # Suppress 'warning C4150: deletion of pointer to incomplete type'
     -wd4251 # Suppress 'warning C4251: T must have dll-interface to be used by clients of class U.'
     -wd4521 # Suppress 'warning C4521: 'type' : multiple copy constructors specified'

lldb/trunk/source/Plugins/Language/ObjC/CMakeLists.txt

+set(EXTRA_CXXFLAGS "")
+
+if (CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+  set(EXTRA_CXXFLAGS ${EXTRA_CXXFLAGS} -Wno-gnu-anonymous-struct)
+endif ()
+
+if (CXX_SUPPORTS_NO_NESTED_ANON_TYPES)
+  set(EXTRA_CXXFLAGS ${EXTRA_CXXFLAGS} -Wno-nested-anon-types)
+endif ()
+
 add_lldb_library(lldbPluginObjCLanguage PLUGIN
   ObjCLanguage.cpp
   CF.cpp
   Cocoa.cpp
   CoreMedia.cpp
   NSArray.cpp
   NSDictionary.cpp
   NSError.cpp
   NSException.cpp
   NSIndexPath.cpp
   NSSet.cpp
   NSString.cpp
 
   LINK_LIBS
     clangAST
     lldbCore
     lldbDataFormatters
     lldbExpression
     lldbHost
     lldbSymbol
     lldbTarget
     lldbUtility
     lldbPluginAppleObjCRuntime
+
+  EXTRA_CXXFLAGS ${EXTRA_CXXFLAGS}
 )