r301567 - clang-cl: Alias /d1reportAllClassLayout to -fdump-record-layouts (PR32826)

Tue Jul 2 07:14:02 PDT 2019

Oh cool, you made it work.

I think just leaving it unsupported (but parsed) would have been fine
too since as you say nobody would have been using it anyway. But
making it work is nicer.

On Tue, Jul 2, 2019 at 12:23 PM Nico Weber via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
>
> Ah, Hans is away for a while. I landed the simple-but-unfulfilling fix in r364901 to unblock my other change for now. Maybe we can tweak it once Hans is back.
>
> On Mon, Jul 1, 2019 at 9:21 PM Nico Weber <thakis at chromium.org> wrote:
>>
>> Turns out this doesn't actually work: -fdump-records-layout is a cc1 option, not a clang driver option. The correct td entry would be
>>
>> def _SLASH_d1reportAllClassLayout : CLFlag<"d1reportAllClassLayout">,
>>   HelpText<"Dump record layout information">,
>>   Alias<Xclang>, AliasArgs<["-fdump-record-layouts"]>;
>>
>> But if nobody ever noticed since this was added, maybe we should just remove it again? It doesn't seem like anyone ever used it. (Also, the current fdump-record-layouts output with ast layout, irgen layout, and cgrecordlayout looks very very wordy – I remember the output being more readable at some point.)
>>
>>
>> Why does the test pass if this doesn't work? Because clang prints "argument unused during compilation: '-fdump-record-layouts'" which happens to match the lit check line, which only asks for -fdump-record-layouts
>>
>>
>> I tried fixing the test, but it's surprisingly tricky: Adding `/c /WX` makes clang emit "error: argument unused..." instead of just "warning:", but -### apparently causes clang to not set $? even if it prints an error. And having a CHECK-NOT for "error:" seems pretty one-off-y. Maybe there should be some way to make -### still set `$?`? cl-options.c kind of looks like it expects -### to behave that way.
>>
>> (I noticed because I'm trying to print unaliased options in diags, so the diag changed to "argument unused during compilation: /d1reportAllClassLayout", which made the test fail. I'm happy with any outcome that makes the d1reportAllClassLayout test behave – either removing the flag, or making it work and fixing the test to actually check what it's trying to check.)
>>
>> On Thu, Apr 27, 2017 at 7:32 PM Hans Wennborg via cfe-commits <cfe-commits at lists.llvm.org> wrote:
>>>
>>> Author: hans
>>> Date: Thu Apr 27 12:19:07 2017
>>> New Revision: 301567
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=301567&view=rev
>>> Log:
>>> clang-cl: Alias /d1reportAllClassLayout to -fdump-record-layouts (PR32826)
>>>
>>> Modified:
>>>     cfe/trunk/include/clang/Driver/CLCompatOptions.td
>>>     cfe/trunk/test/Driver/cl-options.c
>>>
>>> Modified: cfe/trunk/include/clang/Driver/CLCompatOptions.td
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CLCompatOptions.td?rev=301567&r1=301566&r2=301567&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/include/clang/Driver/CLCompatOptions.td (original)
>>> +++ cfe/trunk/include/clang/Driver/CLCompatOptions.td Thu Apr 27 12:19:07 2017
>>> @@ -61,6 +61,8 @@ def _SLASH_Brepro_ : CLFlag<"Brepro-">,
>>>  def _SLASH_C : CLFlag<"C">,
>>>    HelpText<"Don't discard comments when preprocessing">, Alias<C>;
>>>  def _SLASH_c : CLFlag<"c">, HelpText<"Compile only">, Alias<c>;
>>> +def _SLASH_d1reportAllClassLayout : CLFlag<"d1reportAllClassLayout">,
>>> +  HelpText<"Dump record layout information">, Alias<fdump_record_layouts>;
>>>  def _SLASH_D : CLJoinedOrSeparate<"D">, HelpText<"Define macro">,
>>>    MetaVarName<"<macro[=value]>">, Alias<D>;
>>>  def _SLASH_E : CLFlag<"E">, HelpText<"Preprocess to stdout">, Alias<E>;
>>>
>>> Modified: cfe/trunk/test/Driver/cl-options.c
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=301567&r1=301566&r2=301567&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/test/Driver/cl-options.c (original)
>>> +++ cfe/trunk/test/Driver/cl-options.c Thu Apr 27 12:19:07 2017
>>> @@ -14,6 +14,9 @@
>>>  // C_P: "-E"
>>>  // C_P: "-C"
>>>
>>> +// RUN: %clang_cl /d1reportAllClassLayout -### -- %s 2>&1 | FileCheck -check-prefix=d1reportAllClassLayout %s
>>> +// d1reportAllClassLayout: -fdump-record-layouts
>>> +
>>>  // RUN: %clang_cl /Dfoo=bar /D bar=baz /DMYDEF#value /DMYDEF2=foo#bar /DMYDEF3#a=b /DMYDEF4# \
>>>  // RUN:    -### -- %s 2>&1 | FileCheck -check-prefix=D %s
>>>  // D: "-D" "foo=bar"
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits