[Lldb-commits] [PATCH] D47625: [cmake] Detect presence of wide-char libedit at build time

[Pavel Labath via lldb-commits]

On 28/02/2019 20:29, Davide Italiano wrote:
> On Thu, Feb 28, 2019 at 11:21 AM Davide Italiano <dccitaliano at gmail.com> wrote:
>>
>> On Thu, Feb 28, 2019 at 11:19 AM Davide Italiano <dccitaliano at gmail.com> wrote:
>>>
>>> If I add:
>>>
>>> diff --git a/lldb/source/Host/common/MainLoop.cpp
>>> b/lldb/source/Host/common/MainLoop.cpp
>>> index a4803936196..4fee76865ee 100644
>>> --- a/lldb/source/Host/common/MainLoop.cpp
>>> +++ b/lldb/source/Host/common/MainLoop.cpp
>>> @@ -23,6 +23,8 @@
>>>   // (ppoll is present but not implemented properly). On windows we use WSApoll
>>>   // (which does not support signals).
>>>
>>> +#define HAVE_SYS_EVENT_H 1
>>> +
>>>   #if HAVE_SYS_EVENT_H
>>>   #include <sys/event.h>
>>>   #elif defined(_WIN32)
>>>
>>> to my checkout it works. It looks like somehow llvm-config.h isn't
>>> propagate properly so `HAVE_SYS_EVENT_H` isn't defined?
>>> This looks like a bug to me, but maybe there's an easy way to work around it.
>>>
>>
>> And in fact, this is what I see in my build directory:
>>
>> $ grep -R SYS_EVENT './include/llvm/Config/llvm-config.h'
>> $
>>
>> --
>> Davide
> 
> 
> The following patch "fixes" the modules build for me.
> 
> $ git diff
> diff --git a/lldb/include/lldb/Host/Editline.h
> b/lldb/include/lldb/Host/Editline.h
> index a942ede05ce..ab712670ec1 100644
> --- a/lldb/include/lldb/Host/Editline.h
> +++ b/lldb/include/lldb/Host/Editline.h
> @@ -28,6 +28,8 @@
>   // e) Emoji support is fairly terrible, presumably it doesn't understand
>   // composed characters?
> 
> +#include "lldb/Host/Config.h"
> +
>   #ifndef liblldb_Editline_h_
>   #define liblldb_Editline_h_
>   #if defined(__cplusplus)
> diff --git a/lldb/source/Host/common/MainLoop.cpp
> b/lldb/source/Host/common/MainLoop.cpp
> index a4803936196..24a2f80fd10 100644
> --- a/lldb/source/Host/common/MainLoop.cpp
> +++ b/lldb/source/Host/common/MainLoop.cpp
> @@ -8,6 +8,7 @@
> 
>   #include "llvm/Config/llvm-config.h"
> 
> +#include "lldb/Host/Config.h"
>   #include "lldb/Host/MainLoop.h"
>   #include "lldb/Host/PosixApi.h"
>   #include "lldb/Utility/Status.h"
> 
> Raphael/Pavel, I'm going to commit this because it's a serious
> regression for us, and the workaround doesn't seem terrible. I would
> appreciate a post-commit review (or, if you have, a better way of
> fixing this).
> 

I think this is fine. It may not even be a workaround, but a proper way 
to modularize MainLoop.cpp (it ought to include Config.h, since it 
really does depend on symbols defined there).

Just, could you please move the `#include "lldb/Host/Config.h"` inside 
the include guard in Editline.h (it's enough for it to be included 
before the first use of LLDB_EDITLINE_WCHAR_T or whatever is the macro 
name).

BTW, if you're going to be cherry-picking this anywhere, you may want to 
include r355103 as well.

cheers,
pavel