r315126 - Driver: hoist the `wchar_t` handling to the driver

[Reid Kleckner via cfe-commits]

On Wed, Oct 25, 2017 at 10:56 AM, Friedman, Eli <efriedma at codeaurora.org>
wrote:

> On 10/6/2017 4:09 PM, Saleem Abdulrasool via cfe-commits wrote:
>
>> Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Too
>> lChains/Clang.cpp?rev=315126&r1=315125&r2=315126&view=diff
>> ============================================================
>> ==================
>> --- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
>> +++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Fri Oct  6 16:09:55 2017
>> @@ -2601,6 +2601,33 @@ static void RenderModulesOptions(Compila
>>     Args.AddLastArg(CmdArgs, options::OPT_fmodules_disable_
>> diagnostic_validation);
>>   }
>>   +static void RenderCharacterOptions(const ArgList &Args, const
>> llvm::Triple &T,
>> +                                   ArgStringList &CmdArgs) {
>> +  // -fsigned-char is default.
>> +  if (const Arg *A = Args.getLastArg(options::OPT_fsigned_char,
>> +                                     options::OPT_fno_signed_char,
>> +                                     options::OPT_funsigned_char,
>> +                                     options::OPT_fno_unsigned_char)) {
>> +    if (A->getOption().matches(options::OPT_funsigned_char) ||
>> +        A->getOption().matches(options::OPT_fno_signed_char)) {
>> +      CmdArgs.push_back("-fno-signed-char");
>> +    }
>> +  } else if (!isSignedCharDefault(T)) {
>> +    CmdArgs.push_back("-fno-signed-char");
>> +  }
>> +
>> +  if (const Arg *A = Args.getLastArg(options::OPT_fshort_wchar,
>> +                                     options::OPT_fno_short_wchar)) {
>> +    if (A->getOption().matches(options::OPT_fshort_wchar)) {
>> +      CmdArgs.push_back("-fwchar-type=short");
>> +      CmdArgs.push_back("-fno-signed-wchar");
>> +    } else {
>> +      CmdArgs.push_back("-fwchar-type=int");
>> +      CmdArgs.push_back("-fsigned-wchar");
>> +    }
>> +  }
>> +}
>>
>
> It looks like this changes the behavior of "-fno-short-wchar".
> Specifically, on targets where the default wchar_t type is "unsigned int"
> (like ARM AAPCS), "-fno-short-wchar" used to be a no-op, but now it changes
> the sign of wchar_t.  Why are we overriding the default here?


Overriding the default was more or less the intention, but it was for
bitwidth, not signedness. The idea was that if you pass -fno-short-wchar,
then you want the 4 byte one.

Users probably don't care about signedness when using 4 byte wchars, so we
could probably remove the -fsigned-wchar flag from the else block and take
the original sign of wchar_t when overriding the width in Basic. They
probably want an 'unsigned short' when -fshort-wchar is passed, though.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171025/8a7ce9bd/attachment.html>