[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

ldionne added a comment. (sorry, no idea why it says "this revision is now accepted and ready to land on my behalf, I was just removing the libc++ review group to clear our review queue) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112374/new/

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

Abramo-Bagnara added a comment. I have to doubly apologize: 1. my reference to refersInstantiatedDecl is completely wrong and I have been mislead by an old patch on my machine. 2. the problem despite being very real is independent by your changes If you are still interested this is the repro

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov added a comment. In D112374#3792825 , @Abramo-Bagnara wrote: > These changes introduce a subtle bug in template instantiation of newly added > ElaboratedType around TypedefType: > > bool refersInstantiatedDecl(QualType T) in

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

Abramo-Bagnara reopened this revision. Abramo-Bagnara added a comment. These changes introduce a subtle bug in template instantiation of newly added ElaboratedType around TypedefType: bool refersInstantiatedDecl(QualType T) in SemaTemplateInstantiate.cpp does not handle this ElaboratedType

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

davrec added inline comments. Comment at: clang/include/clang/AST/Type.h:5530-5537 /// Represents a type that was referred to using an elaborated type /// keyword, e.g., struct S, or via a qualified name, e.g., N::M::type, /// or both. /// /// This type is used to keep

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

ilya-biryukov added a comment. I agree that the change in behaviour is reasonable and have no objections to it. The code should not rely on particular output of `__PRETTY_FUNCTION__`. I just wanted to point out that we still don't match GCC in other cases, not that is was a worthwhile goal to

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov added a comment. In D112374#3718417 , @ilya-biryukov wrote: > We don't match GCC in other cases. GCC seems to always print the type name > without qualifiers, clang used to always print with qualifiers, but will now > print what was typed in

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

ilya-biryukov added a comment. In D112374#3716982 , @mizvekov wrote: > We even match GCC now: https://gcc.godbolt.org/z/WT93WdE7e > > Ie we are printing the function type as-written correctly now. We don't match GCC in other cases. GCC seems to always

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov marked an inline comment as done. mizvekov added a comment. In D112374#3716944 , @alexfh wrote: > One more problem related to this patch: it changes the behavior of > __PRETTY_FUNCTION__: https://gcc.godbolt.org/z/Mvnj9j74E. There may be >

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

alexfh added a comment. One more problem related to this patch: it changes the behavior of __PRETTY_FUNCTION__: https://gcc.godbolt.org/z/Mvnj9j74E. There may be dependencies upon the specific format of expansions of this macro: tests, log processing tools, libraries like ctti, and probably

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov marked an inline comment as done. mizvekov added inline comments. Comment at: clang/lib/AST/QualTypeNames.cpp:455 + if (const auto *UT = QT->getAs()) { +return getFullyQualifiedType(UT->getUnderlyingType(), Ctx, +

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

rtrieu added inline comments. Comment at: clang/lib/AST/QualTypeNames.cpp:455 + if (const auto *UT = QT->getAs()) { +return getFullyQualifiedType(UT->getUnderlyingType(), Ctx, + WithGlobalNsPrefix); Moving this down here

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov added a comment. In D112374#3684722 , @hans wrote: > Here's an example where I think this regressed a Clang diagnostic. Consider: Consider this simple extension of this example, to show how this general problem already existed:

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov added a comment. In D112374#3684722 , @hans wrote: > The problem is that just based on `Template` it's not clear whether it's > `S1::Foo` or `S2::Foo` that's referred to. The logic in the diagnostic here ended up doing a silent single step

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

hans added a comment. Here's an example where I think this regressed a Clang diagnostic. Consider: template struct Template { Template(int x) {} }; struct S1 { struct Foo; typedef Template Typedef; }; struct S2 { struct Foo; typedef Template Typedef; };

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG15f3cd6bfc67: [clang] Implement ElaboratedType sugaring for types

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov updated this revision to Diff 447890. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112374/new/ https://reviews.llvm.org/D112374 Files: clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov edited the summary of this revision. mizvekov updated this revision to Diff 447085. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112374/new/ https://reviews.llvm.org/D112374 Files:

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov added a comment. In D112374#3666531 , @kimgr wrote: > From a purely personal perspective, I'd prefer if this landed after the > branch for llvm-15. > > We try to co-release IWYU shortly after LLVM/Clang are released, to get a > public

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

kimgr added a comment. From a purely personal perspective, I'd prefer if this landed after the branch for llvm-15. We try to co-release IWYU shortly after LLVM/Clang are released, to get a public API-compatible release out there. So it would be really nice if we didn't have to spend time

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

teemperor added a comment. import-std-module test changes look good to me, thanks for fixing that up. And yes, ideally the tests should never use any libc++ internal names (and LLDB should never print them for those tests). So I think not having those in the here is a step in the right

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

JDevlieghere added a comment. In D112374#3665989 , @mizvekov wrote: > If anyone wants to take a look at the new changes to lldb tests, be my guest. > Otherwise I will try to land this again soon. It might well be that we figure > out some other in-tree

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov added a comment. In D112374#3665249 , @sammccall wrote: > I've seen plenty of (useful) out-of-tree tidy checks written by people fuzzy > on the difference between a Type and a TypeLoc, or what sugar is. Clang makes > it (almost) easy to write

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov updated this revision to Diff 446173. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112374/new/ https://reviews.llvm.org/D112374 Files: clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

sammccall added a comment. In D112374#3657640 , @mizvekov wrote: > In general, I would not expect external tools to care about the shape of the > AST. I would expect the type API would be used in a way where we ignore a > type sugar node we have no

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov added a comment. @JDevlieghere @teemperor ping Can you please verify that everything works with LLDB, we just changed how those types are printed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112374/new/

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov updated this revision to Diff 445646. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112374/new/ https://reviews.llvm.org/D112374 Files: clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov added a comment. In D112374#3659597 , @nikic wrote: > Given that LLVM 15 branches off in one week, maybe it would be better to wait > for that before relanding the change, as it seems to have significant impact > on plugins? I think it would

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

nikic added a comment. Given that LLVM 15 branches off in one week, maybe it would be better to wait for that before relanding the change, as it seems to have significant impact on plugins? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov marked an inline comment as done. mizvekov added a comment. @JDevlieghere @teemperor ping. Sorry for the urgency here, but I have a lot of work built on top of this patch. And this patch fixes a bunch of bugs that other people are unaware and duplicating effort, see github issues.

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov marked an inline comment as done. mizvekov added inline comments. Comment at: clang/lib/Sema/TypeLocBuilder.cpp:159 - assert(Capacity - Index == TypeLoc::getFullDataSizeForType(T) && + unsigned FDSz = TypeLoc::getFullDataSizeForType(T); + assert(Capacity - Index ==

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov edited the summary of this revision. mizvekov updated this revision to Diff 445319. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112374/new/ https://reviews.llvm.org/D112374 Files:

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov added a comment. In D112374#3658059 , @kimgr wrote: > Haha. Pun intended? :-) Yes :-) > As you noticed, it's not our tests that care about the AST, it's the tool > itself. IWYU has been around since 2010-11, so there's probably lots of code

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

kimgr added a comment. In D112374#3657640 , @mizvekov wrote: > In D112374#3657472 , @kimgr wrote: > >> I'm coming at this from pretty far away, so there's very likely lots of >> details that I'm overlooking. But

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov added a comment. @kimgr One other general comment. The way this function is implemented is quite error prone: static const NamedDecl* TypeToDeclImpl(const Type* type, bool as_written) { // Get past all the 'class' and 'struct' prefixes, and namespaces. type =

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov added a comment. In D112374#3657472 , @kimgr wrote: > I can't say what the best solution is, but this patch generates quite a lot > of work for me, and I would really hope that catching up with the new AST > does not generate even more work

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov added a comment. In D112374#3657472 , @kimgr wrote: > I'm coming at this from pretty far away, so there's very likely lots of > details that I'm overlooking. But it seems to me the mainline had only had an > `ElaboratedType` node if there was

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

kimgr added a comment. > It's the difference in knowing the type was written without any tag or > nested-name specifier, and having a type that you are not sure how it was > written. > > When we are dealing with a type which we are not sure, we would like to print > it fully qualified, with a

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov reopened this revision. mizvekov added a comment. In D112374#3653967 , @JDevlieghere wrote: > I'm sorry to hear you're having trouble building LLDB. The LLDB website has > quite an elaborate guide with instructions in how to build LLDB: >

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

kimgr added a comment. It's a little confusing, because it now looks like _every_ `Type` in the AST is wrapped in an `ElaboratedTypeLoc` + `ElaboratedType`. IWYU's debug AST dump shows this (excerpt): tests/cxx/sizeof_reference.cc:51:8: (1) [ VarDecl ] size_t s2

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

chrish_ericsson_atx added a comment. In D112374#3653967 , @JDevlieghere wrote: > I don't. I think reverting your change was well within the guidelines > outlined by LLVM's patch reversion policy: >

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

JDevlieghere added a subscriber: teemperor. JDevlieghere added a comment. In D112374#3653702 , @mizvekov wrote: > @JDevlieghere I spent a lot of time trying to get this test running on my > machine to no avail. I think lldb build and test setup is quite

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

chapuni added inline comments. Comment at: clang/lib/Sema/TypeLocBuilder.cpp:159 - assert(Capacity - Index == TypeLoc::getFullDataSizeForType(T) && + unsigned FDSz = TypeLoc::getFullDataSizeForType(T); + assert(Capacity - Index == FDSz && It causes a

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG7c51f02effdb: [clang] Implement ElaboratedType sugaring for types written bare (authored by mizvekov). Changed prior to

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

glandium added a comment. In D112374#3651218 , @mizvekov wrote: > Can you confirm that this `NoPrincipalGetURI` matcher would, in clang version > without this patch here, fail to match if you had rewritten the `GetUri` > method to use a qualified name?

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov added a comment. @JDevlieghere I spent a lot of time trying to get this test running on my machine to no avail. I think lldb build and test setup is quite convoluted, fragile and antiquated. It uses many deprecated CMake features, It fails to properly link to system libraries it needs

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov added a comment. In D112374#3650749 , @glandium wrote: > FWIW, this change also broke this check in Firefox's clang plugin: >

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

glandium added a comment. FWIW, this change also broke this check in Firefox's clang plugin: https://searchfox.org/mozilla-central/rev/0d11f3660945ce35c49501bb44bc4f82bb2b503c/build/clang-plugin/NoPrincipalGetURI.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov added a comment. In D112374#3648624 , @JDevlieghere wrote: > This breaks all the LLDB tests that import the std module: > > Given that the bot has been red for 14 hours I went ahead and reverted this > change. Please keep an eye on this bot

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

kimgr added a comment. This patch also broke IWYU, not exactly sure how or why yet. We run around the AST quite a bit, so structural changes like this often bite us. Can you expand on what happened here? Before/after kind-of thing? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

JDevlieghere added a comment. This breaks all the LLDB tests that import the std module: import-std-module/array.TestArrayFromStdModule.py import-std-module/deque-basic.TestDequeFromStdModule.py import-std-module/deque-dbg-info-content.TestDbgInfoContentDequeFromStdModule.py

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov added a comment. In D112374#3647530 , @nikic wrote: > FYI this change had a noticeable compile-time impact >

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

nikic added a comment. FYI this change had a noticeable compile-time impact (http://llvm-compile-time-tracker.com/compare.php?from=ee88c0cf09969ba44307068797e12533b94768a6=bdc6974f92304f4ed542241b9b89ba58ba6b20aa=instructions), is that expected? Largest regressions are in kimwitu++, where

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGbdc6974f9230: [clang] Implement ElaboratedType sugaring for types

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov updated this revision to Diff 444104. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112374/new/ https://reviews.llvm.org/D112374 Files: clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

rsmith accepted this revision. rsmith added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112374/new/ https://reviews.llvm.org/D112374 ___ cfe-commits mailing list

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov updated this revision to Diff 444067. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112374/new/ https://reviews.llvm.org/D112374 Files: clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov edited the summary of this revision. mizvekov updated this revision to Diff 443951. mizvekov marked 2 inline comments as done. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov marked 7 inline comments as done. mizvekov added a subscriber: sammccall. mizvekov added a comment. Herald added a subscriber: steakhal. Herald added a reviewer: NoQ. In D112374#3535949 , @rsmith wrote: > I've not looked at the test changes in

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

rsmith added a comment. Herald added a project: All. I've not looked at the test changes in any detail; please let me know if there's anything in there that deserves special attention. Comment at: clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp:567-577 -if

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov updated this revision to Diff 389352. mizvekov added a comment. - fix IntrospectionTests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112374/new/ https://reviews.llvm.org/D112374 Files:

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov updated this revision to Diff 389298. mizvekov edited the summary of this revision. mizvekov added a comment. Herald added subscribers: llvm-commits, dexonsmith. Herald added a project: LLVM. - Avoid using any storage for an empty ElaboratedTYpeLoc. - But we still require pointer

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

mizvekov updated this revision to Diff 387699. mizvekov retitled this revision from "[clang] WIP: Implement ElaboratedType sugaring for types written bare" to "[clang] Implement ElaboratedType sugaring for types written bare". mizvekov edited the summary of this revision. mizvekov added a