[PATCH] D96807: Modify TypePrinter to differentiate between anonymous struct and unnamed struct

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik updated this revision to Diff 325100.
shafik added a comment.
Herald added subscribers: usaxena95, kadircet.

Fixing tests that I missed before.


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

https://reviews.llvm.org/D96807

Files:
  clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/test/AST/ast-dump-decl-json.c
  clang/test/AST/ast-dump-enum-json.cpp
  clang/test/AST/ast-dump-openmp-cancel.c
  clang/test/AST/ast-dump-openmp-cancellation-point.c
  clang/test/AST/ast-dump-openmp-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-distribute-simd.c
  clang/test/AST/ast-dump-openmp-distribute.c
  clang/test/AST/ast-dump-openmp-for-simd.c
  clang/test/AST/ast-dump-openmp-for.c
  clang/test/AST/ast-dump-openmp-ordered.c
  clang/test/AST/ast-dump-openmp-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-parallel-for.c
  clang/test/AST/ast-dump-openmp-parallel-sections.c
  clang/test/AST/ast-dump-openmp-parallel.c
  clang/test/AST/ast-dump-openmp-section.c
  clang/test/AST/ast-dump-openmp-sections.c
  clang/test/AST/ast-dump-openmp-simd.c
  clang/test/AST/ast-dump-openmp-single.c
  clang/test/AST/ast-dump-openmp-target-data.c
  clang/test/AST/ast-dump-openmp-target-enter-data.c
  clang/test/AST/ast-dump-openmp-target-exit-data.c
  clang/test/AST/ast-dump-openmp-target-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-target-parallel-for.c
  clang/test/AST/ast-dump-openmp-target-parallel.c
  clang/test/AST/ast-dump-openmp-target-simd.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-simd.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute.c
  clang/test/AST/ast-dump-openmp-target-teams.c
  clang/test/AST/ast-dump-openmp-target-update.c
  clang/test/AST/ast-dump-openmp-target.c
  clang/test/AST/ast-dump-openmp-task.c
  clang/test/AST/ast-dump-openmp-taskgroup.c
  clang/test/AST/ast-dump-openmp-taskloop-simd.c
  clang/test/AST/ast-dump-openmp-taskloop.c
  clang/test/AST/ast-dump-openmp-teams-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-teams-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-teams-distribute-simd.c
  clang/test/AST/ast-dump-openmp-teams-distribute.c
  clang/test/AST/ast-dump-openmp-teams.c
  clang/test/AST/ast-dump-records-json.cpp
  clang/test/AST/ast-dump-records.c
  clang/test/AST/ast-dump-records.cpp
  clang/test/AST/ast-dump-stmt-json.m
  clang/test/ASTMerge/struct/test.c
  clang/test/Analysis/cfg.cpp
  clang/test/Analysis/padding_c.c
  clang/test/Index/print-type.c
  clang/test/Index/print-type.cpp
  clang/test/Layout/ms-x86-alias-avoidance-padding.cpp
  clang/test/PCH/stmt-openmp_structured_block-bit.cpp
  clang/test/Sema/assign.c
  clang/test/Sema/switch.c
  clang/test/SemaCXX/condition.cpp
  clang/test/SemaCXX/enum.cpp
  clang/test/SemaCXX/warn-sign-conversion.cpp
  
lldb/test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp
  lldb/test/Shell/SymbolFile/DWARF/debug-types-missing-signature.test

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

[PATCH] D96807: Modify TypePrinter to differentiate between anonymous struct and unnamed struct

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.
Herald added a subscriber: JDevlieghere.

Reverted the changes because I missed the clangd test suite and don't know how 
long it will take to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96807

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

[PATCH] D96807: Modify TypePrinter to differentiate between anonymous struct and unnamed struct

[Shafik Yaghmour via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
shafik marked an inline comment as done.
Closed by commit rGecb90b55454e: Modify TypePrinter to differentiate between 
anonymous struct and unnamed struct (authored by shafik).
Herald added projects: clang, LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96807

Files:
  clang/lib/AST/TypePrinter.cpp
  clang/test/AST/ast-dump-decl-json.c
  clang/test/AST/ast-dump-enum-json.cpp
  clang/test/AST/ast-dump-openmp-cancel.c
  clang/test/AST/ast-dump-openmp-cancellation-point.c
  clang/test/AST/ast-dump-openmp-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-distribute-simd.c
  clang/test/AST/ast-dump-openmp-distribute.c
  clang/test/AST/ast-dump-openmp-for-simd.c
  clang/test/AST/ast-dump-openmp-for.c
  clang/test/AST/ast-dump-openmp-ordered.c
  clang/test/AST/ast-dump-openmp-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-parallel-for.c
  clang/test/AST/ast-dump-openmp-parallel-sections.c
  clang/test/AST/ast-dump-openmp-parallel.c
  clang/test/AST/ast-dump-openmp-section.c
  clang/test/AST/ast-dump-openmp-sections.c
  clang/test/AST/ast-dump-openmp-simd.c
  clang/test/AST/ast-dump-openmp-single.c
  clang/test/AST/ast-dump-openmp-target-data.c
  clang/test/AST/ast-dump-openmp-target-enter-data.c
  clang/test/AST/ast-dump-openmp-target-exit-data.c
  clang/test/AST/ast-dump-openmp-target-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-target-parallel-for.c
  clang/test/AST/ast-dump-openmp-target-parallel.c
  clang/test/AST/ast-dump-openmp-target-simd.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-simd.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute.c
  clang/test/AST/ast-dump-openmp-target-teams.c
  clang/test/AST/ast-dump-openmp-target-update.c
  clang/test/AST/ast-dump-openmp-target.c
  clang/test/AST/ast-dump-openmp-task.c
  clang/test/AST/ast-dump-openmp-taskgroup.c
  clang/test/AST/ast-dump-openmp-taskloop-simd.c
  clang/test/AST/ast-dump-openmp-taskloop.c
  clang/test/AST/ast-dump-openmp-teams-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-teams-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-teams-distribute-simd.c
  clang/test/AST/ast-dump-openmp-teams-distribute.c
  clang/test/AST/ast-dump-openmp-teams.c
  clang/test/AST/ast-dump-records-json.cpp
  clang/test/AST/ast-dump-records.c
  clang/test/AST/ast-dump-records.cpp
  clang/test/AST/ast-dump-stmt-json.m
  clang/test/ASTMerge/struct/test.c
  clang/test/Analysis/cfg.cpp
  clang/test/Analysis/padding_c.c
  clang/test/Index/print-type.c
  clang/test/Index/print-type.cpp
  clang/test/Layout/ms-x86-alias-avoidance-padding.cpp
  clang/test/PCH/stmt-openmp_structured_block-bit.cpp
  clang/test/Sema/assign.c
  clang/test/Sema/switch.c
  clang/test/SemaCXX/condition.cpp
  clang/test/SemaCXX/enum.cpp
  clang/test/SemaCXX/warn-sign-conversion.cpp
  
lldb/test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp
  lldb/test/Shell/SymbolFile/DWARF/debug-types-missing-signature.test

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

[PATCH] D96807: Modify TypePrinter to differentiate between anonymous struct and unnamed struct

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


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

https://reviews.llvm.org/D96807

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

[PATCH] D96807: Modify TypePrinter to differentiate between anonymous struct and unnamed struct

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik marked an inline comment as done.
shafik added inline comments.



Comment at: clang/lib/AST/TypePrinter.cpp:1308
+} else if ((isa(D) && 
cast(D)->isAnonymousStructOrUnion()) ||
+isa(D)) {
   OS << "anonymous";

aaron.ballman wrote:
> I think `EnumDecl` should probably be `unnamed` rather than `anonymous` 
> because there's no term of art for an anonymous enumeration (there's only 
> anonymous structures and anonymous unions). WDYT?
I thought I saw that wording but when I looked back I realize you are correct.


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

https://reviews.llvm.org/D96807

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

[PATCH] D96807: Modify TypePrinter to differentiate between anonymous struct and unnamed struct

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik updated this revision to Diff 324431.
shafik added a comment.

- Went with unnamed enums Vs anonymous enums


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

https://reviews.llvm.org/D96807

Files:
  clang/lib/AST/TypePrinter.cpp
  clang/test/AST/ast-dump-decl-json.c
  clang/test/AST/ast-dump-enum-json.cpp
  clang/test/AST/ast-dump-openmp-cancel.c
  clang/test/AST/ast-dump-openmp-cancellation-point.c
  clang/test/AST/ast-dump-openmp-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-distribute-simd.c
  clang/test/AST/ast-dump-openmp-distribute.c
  clang/test/AST/ast-dump-openmp-for-simd.c
  clang/test/AST/ast-dump-openmp-for.c
  clang/test/AST/ast-dump-openmp-ordered.c
  clang/test/AST/ast-dump-openmp-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-parallel-for.c
  clang/test/AST/ast-dump-openmp-parallel-sections.c
  clang/test/AST/ast-dump-openmp-parallel.c
  clang/test/AST/ast-dump-openmp-section.c
  clang/test/AST/ast-dump-openmp-sections.c
  clang/test/AST/ast-dump-openmp-simd.c
  clang/test/AST/ast-dump-openmp-single.c
  clang/test/AST/ast-dump-openmp-target-data.c
  clang/test/AST/ast-dump-openmp-target-enter-data.c
  clang/test/AST/ast-dump-openmp-target-exit-data.c
  clang/test/AST/ast-dump-openmp-target-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-target-parallel-for.c
  clang/test/AST/ast-dump-openmp-target-parallel.c
  clang/test/AST/ast-dump-openmp-target-simd.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-simd.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute.c
  clang/test/AST/ast-dump-openmp-target-teams.c
  clang/test/AST/ast-dump-openmp-target-update.c
  clang/test/AST/ast-dump-openmp-target.c
  clang/test/AST/ast-dump-openmp-task.c
  clang/test/AST/ast-dump-openmp-taskgroup.c
  clang/test/AST/ast-dump-openmp-taskloop-simd.c
  clang/test/AST/ast-dump-openmp-taskloop.c
  clang/test/AST/ast-dump-openmp-teams-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-teams-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-teams-distribute-simd.c
  clang/test/AST/ast-dump-openmp-teams-distribute.c
  clang/test/AST/ast-dump-openmp-teams.c
  clang/test/AST/ast-dump-records-json.cpp
  clang/test/AST/ast-dump-records.c
  clang/test/AST/ast-dump-records.cpp
  clang/test/AST/ast-dump-stmt-json.m
  clang/test/ASTMerge/struct/test.c
  clang/test/Analysis/cfg.cpp
  clang/test/Analysis/padding_c.c
  clang/test/Index/print-type.c
  clang/test/Index/print-type.cpp
  clang/test/Layout/ms-x86-alias-avoidance-padding.cpp
  clang/test/PCH/stmt-openmp_structured_block-bit.cpp
  clang/test/Sema/assign.c
  clang/test/Sema/switch.c
  clang/test/SemaCXX/condition.cpp
  clang/test/SemaCXX/enum.cpp
  clang/test/SemaCXX/warn-sign-conversion.cpp
  
lldb/test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp
  lldb/test/Shell/SymbolFile/DWARF/debug-types-missing-signature.test

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

[PATCH] D96807: Modify TypePrinter to differentiate between anonymous struct and unnamed struct

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

LGTM.

Regarding the LLDB example: Given that the LLDB API is in theory not bound to 
the semantics a specific language, I think one can argue that `IsAnonymousType` 
could also return true for unnamed classes. The use case that triggered this 
whole discussion was someone who wanted to know whether the type has a name, so 
calling this function seems logical. Given that we don't really have a valid 
type name for unnamed classes, we might want to change the implementation of 
`IsAnonymousType` to also return true for unnamed classes. But all that is out 
of scope for this patch which is about fixing the type printer in Clang. Just 
bringing this up as it's the example in the patch description.


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

https://reviews.llvm.org/D96807

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

[PATCH] D96807: Modify TypePrinter to differentiate between anonymous struct and unnamed struct

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for working on this! In general, I like the change.




Comment at: clang/lib/AST/TypePrinter.cpp:1308
+} else if ((isa(D) && 
cast(D)->isAnonymousStructOrUnion()) ||
+isa(D)) {
   OS << "anonymous";

I think `EnumDecl` should probably be `unnamed` rather than `anonymous` because 
there's no term of art for an anonymous enumeration (there's only anonymous 
structures and anonymous unions). WDYT?


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

https://reviews.llvm.org/D96807

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

[PATCH] D96807: Modify TypePrinter to differentiate between anonymous struct and unnamed struct

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

Note: I am not fixing how we treat anonymous and unnamed enums, I could not 
find a way to figure out if an enum was anonymous or not.


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

https://reviews.llvm.org/D96807

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

[PATCH] D96807: Modify TypePrinter to differentiate between anonymous struct and unnamed struct

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik created this revision.
shafik added reviewers: aaron.ballman, rsmith, teemperor.
Herald added a subscriber: arphaman.
shafik requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

Currently `TypePrinter` lumps anonymous classes and unnamed classes in one 
group `"anonymous"` this is not correct and can be confusing in some contexts. 
e.g.

LLDB SBType  
provides an `IsAnonymousType()` function but when displaying the type we see 
conflicting information, given:

struct A { 
 struct { int x; }; // A
 struct { int y; } B;
  };
  
  int main() {
 A a1; 
//...
  }

we get the following in LLDB:

  (lldb) script var = lldb.frame.FindVariable("a1")
  (lldb) script print(var.GetChildAtIndex(0).GetType().IsAnonymousType())
  True
  (lldb) script print(var.GetChildAtIndex(1).GetType().IsAnonymousType())
  False
  (lldb) script print(var.GetChildAtIndex(0).GetType().GetName())
  A::(anonymous struct)
  (lldb) script print(var.GetChildAtIndex(1).GetType().GetName())
  A::(anonymous struct)


https://reviews.llvm.org/D96807

Files:
  clang/lib/AST/TypePrinter.cpp
  clang/test/AST/ast-dump-decl-json.c
  clang/test/AST/ast-dump-openmp-cancel.c
  clang/test/AST/ast-dump-openmp-cancellation-point.c
  clang/test/AST/ast-dump-openmp-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-distribute-simd.c
  clang/test/AST/ast-dump-openmp-distribute.c
  clang/test/AST/ast-dump-openmp-for-simd.c
  clang/test/AST/ast-dump-openmp-for.c
  clang/test/AST/ast-dump-openmp-ordered.c
  clang/test/AST/ast-dump-openmp-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-parallel-for.c
  clang/test/AST/ast-dump-openmp-parallel-sections.c
  clang/test/AST/ast-dump-openmp-parallel.c
  clang/test/AST/ast-dump-openmp-section.c
  clang/test/AST/ast-dump-openmp-sections.c
  clang/test/AST/ast-dump-openmp-simd.c
  clang/test/AST/ast-dump-openmp-single.c
  clang/test/AST/ast-dump-openmp-target-data.c
  clang/test/AST/ast-dump-openmp-target-enter-data.c
  clang/test/AST/ast-dump-openmp-target-exit-data.c
  clang/test/AST/ast-dump-openmp-target-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-target-parallel-for.c
  clang/test/AST/ast-dump-openmp-target-parallel.c
  clang/test/AST/ast-dump-openmp-target-simd.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-simd.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute.c
  clang/test/AST/ast-dump-openmp-target-teams.c
  clang/test/AST/ast-dump-openmp-target-update.c
  clang/test/AST/ast-dump-openmp-target.c
  clang/test/AST/ast-dump-openmp-task.c
  clang/test/AST/ast-dump-openmp-taskgroup.c
  clang/test/AST/ast-dump-openmp-taskloop-simd.c
  clang/test/AST/ast-dump-openmp-taskloop.c
  clang/test/AST/ast-dump-openmp-teams-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-teams-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-teams-distribute-simd.c
  clang/test/AST/ast-dump-openmp-teams-distribute.c
  clang/test/AST/ast-dump-openmp-teams.c
  clang/test/AST/ast-dump-records-json.cpp
  clang/test/AST/ast-dump-records.c
  clang/test/AST/ast-dump-records.cpp
  clang/test/AST/ast-dump-stmt-json.m
  clang/test/ASTMerge/struct/test.c
  clang/test/Analysis/cfg.cpp
  clang/test/Analysis/padding_c.c
  clang/test/Index/print-type.c
  clang/test/Layout/ms-x86-alias-avoidance-padding.cpp
  clang/test/PCH/stmt-openmp_structured_block-bit.cpp
  clang/test/Sema/assign.c
  clang/test/SemaCXX/condition.cpp
  
lldb/test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp
  lldb/test/Shell/SymbolFile/DWARF/debug-types-missing-signature.test

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