diseraluca wrote:

> > > > I gave it a quick try, and we would still end up with the same result 
> > > > in our codebase. But, generally, this would not probably be feasible 
> > > > for us as a solution.
> > > 
> > > 
> > > do you have an idea why? Can you show the diff of the changes you made? 
> > > Is the void specialization not explicit?
> > 
> > 
> > I did a quick test with this:
> > ```
> > diff --git a/clang/lib/AST/QualTypeNames.cpp 
> > b/clang/lib/AST/QualTypeNames.cpp
> > index 26aaa96a1dc6..8b882eda83bb 100644
> > --- a/clang/lib/AST/QualTypeNames.cpp
> > +++ b/clang/lib/AST/QualTypeNames.cpp
> > @@ -287,8 +287,13 @@ static NestedNameSpecifier 
> > *createNestedNameSpecifierForScopeOf(
> >          //
> >          // Make the situation is 'useable' but looking a bit odd by
> >          // picking a random instance as the declaring context.
> > -        if (ClassTempl->spec_begin() != ClassTempl->spec_end()) {
> > -          Decl = *(ClassTempl->spec_begin());
> > +        auto specialization_iterator = std::find_if(
> > +            ClassTempl->spec_begin(), ClassTempl->spec_end(), [](auto a) {
> > +              return !a->isExplicitInstantiationOrSpecialization();
> > +            });
> > +
> > +        if (specialization_iterator != ClassTempl->spec_end()) {
> > +          Decl = *specialization_iterator;
> >            Outer = dyn_cast<NamedDecl>(Decl);
> >            OuterNS = dyn_cast<NamespaceDecl>(Decl);
> >          }
> > ```
> > 
> > 
> >     
> >       
> >     
> > 
> >       
> >     
> > 
> >     
> >   
> > Do please let me know if this is incorrect or if I misunderstood your 
> > proposal.
> > We do have explicit references to the void specialization in the codebase 
> > so it would explain the previous choice. But I'm not aware of why it would 
> > be chosen as a non-explicit one too.
> > I can try to debug that on Monday if that can be useful. Albeit it might 
> > take some time.
> 
> You probably need 
> [isExplicitSpecialization](https://clang.llvm.org/doxygen/classclang_1_1ClassTemplateSpecializationDecl.html#acd75ba25d34249d2e21ebbecbb2ef70e)()

Wouldn't `isExplicitSpecializationOrInstantiation` be a stronger guarantee than 
isExplicitSpecialization, such that it would exclude a superset of what is 
excluded by `isExplicitSpecialization`? If I did not misunderstand their source 
code.

Nonetheless, I've made a test run with `isExplicitSpecialization`, but in Qt's 
codebase we still get the same result.

I generally don't think we can depend on this kind of behavior, especially as 
it is far too difficult to control for the kind of consistency we want.

I initially missed the fact that the intention of this was to produce 
compilable code. While similar to our use-case it has slightly different 
requirements and I feel that may not be compatible with ours.

I think that is fine, and I think we don't need to force it to work for all use 
cases,  especially because I really don't want to break other people code. My 
assumption was that this might be an improvement for other use-cases too, but I 
failed to see the "give me code that compiles" use case.

The only thing that comes to mind would be to condition the behavior to be 
there or not. Similar to how we have `WighGlobalNsPrefix` we might have another 
boolean or similar, so that the default behavior remains the same but it can be 
conditioned to avoid this case.

But I don't think that is a particularly good solution, so I'm pretty happy 
with maintaining our own version too.

https://github.com/llvm/llvm-project/pull/67566
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to