aeubanks added a comment.

In D136011#3862150 <https://reviews.llvm.org/D136011#3862150>, @labath wrote:

> In D136011#3860637 <https://reviews.llvm.org/D136011#3860637>, @dblaikie 
> wrote:
>
>> I think the place where this will go wrong is in terms of how lldb renders 
>> `char` values on non-default-char-signedness programs (it'll render them as 
>> the default-char-signedness, which might be confusing to a user - since 
>> they'll be looking at literals, etc, that are the other signedness) and how 
>> lldb will interpret char literals (though that'll already be wrong - since 
>> the literals are already being parsed with the default-char-signedness, I 
>> think).
>
> Yes, I'm pretty sure that will happen. OTOH, I don't think there's any value 
> to fix this in a completely satisfactory way. Like, if the whole program was 
> consistently with the non-default signedness, we could try to detect it and 
> then configure the internal AST defaults accordingly. But that's hard to 
> detect, and I'd be surprised if most programs are completely homogeneous like 
> this.
>
> So, overall, I quite like this fix.

Maybe looking for a "char" entry and setting `ast.getLangOpts().CharIsSigned` 
from it would probably work in most cases, but not sure that it's worth the 
effort. Rendering the wrong signedness for chars doesn't seem like the worst 
thing, and this patch improves things (as the removed `expectedFailureAll` 
show, plus this helps with some testing of lldb I've been doing). I can add a 
TODO if you'd like.

>> I'm curious why there were all those expected failures re: PR23069. Were 
>> they not using the default char signedness? Or is the test using explicit 
>> signedness, and so whatever platforms happen not to have the explicit value 
>> sa their default are/were failing?
>
> Yes, I'm pretty sure that's the case.
>
> In D136011#3861792 <https://reviews.llvm.org/D136011#3861792>, @DavidSpickett 
> wrote:
>
>>> With -f(un)signed-char, the die corresponding to "char" may be the wrong 
>>> DW_ATE_(un)signed_char.
>>
>> As the producer of the DWARF (so, clang for example) is this correct by the 
>> existing rules?
>
> Yes, because DWARF never expected people will be round-tripping the debug 
> info back into C(++). As far as it is concerned, it has correctly given you 
> the signedness of the type.

"wrong" is inaccurate, I've updated the summary


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136011/new/

https://reviews.llvm.org/D136011

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to