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

[Friedman, Eli via cfe-commits]

On 10/25/2017 11:54 AM, Reid Kleckner wrote:
> On Wed, Oct 25, 2017 at 10:56 AM, Friedman, Eli 
> <efriedma at codeaurora.org <mailto: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/ToolChains/Clang.cpp?rev=315126&r1=315125&r2=315126&view=diff
>         <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/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.

Maybe we should just ignore -fno-short-wchar, instead?  I think that's 
what gcc and released versions of clang do (that means -fno-short-wchar 
doesn't do anything for Windows targets, but that's probably fine).

-Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171025/018715e3/attachment.html>