aaron.ballman added a comment.

In D56160#1367811 <https://reviews.llvm.org/D56160#1367811>, @JonasToth wrote:

> In D56160#1367074 <https://reviews.llvm.org/D56160#1367074>, @bernhardmgruber 
> wrote:
>
> > Thank you again @JonasToth for all your valueable input! I could almost 
> > successfully run my check on the llvm/lib subfolder. I created a 
> > compilation database from within Visual Studio using an extension called 
> > SourceTrail 
> > <https://www.sourcetrail.com/blog/export_clang_compilation_database_from_visual_studio_solution/>.
> >  One of the issues was the following:
>
>
> Very good!
>
> >   struct Object { long long value; };
> >   class C {
> >     int Object;
> >     struct Object m();
> >   };
> >   Object C::m() { return {0}; }
> > 
> > 
> > If I rewrite this to the following
> > 
> >   struct Object { long long value; };
> >   class C {
> >     int Object;
> >     struct Object m();
> >   };
> >   auto C::m() -> Object { return {0}; }
> > 
> > 
> > a compilation error occurs afterwards, because Object now refers to the 
> > class member. I discovered a similar problem colliding with the name of a 
> > function argument. So essentially, I believe I now require a name lookup of 
> > the return type (if it is unqualified) in the scope of the function. In 
> > case such a name already exists, i have to prefix `struct/class` or perform 
> > no replacement. I looked at the documentation of `ASTContext`, but it seems 
> > all the good stuff is in `DeclContext`, which I have not available. How can 
> > I perform a name lookup inside the `check` member function?
>
> That is very interesting and I was not aware of this possibility :D
>
> Every `Decl` derives from `DeclContext`, see for example the docs for 
> `CXXMethodDecl` 
> (https://clang.llvm.org/doxygen/classclang_1_1CXXMethodDecl.html)
>  You should be able to look up the names you are interested. I don't know of 
> a good way to check for the issue we have right now, @aaron.ballman knows 
> that probably better then I do.
>  Try to experiment, but If you don't find a solution come back to us, we will 
> figure something out (or ask in IRC).


I think you may be able to skip the lookup (which could get expensive) and 
instead rely on the fact that the user must have explicitly written that type 
as an elaborated type specifier when trying to calculate the source range for 
the return type. I suspect what's happening right now is that 
`findReturnTypeAndCVSourceRange()` isn't noticing the `struct` specifier and 
that's why it's getting dropped. If the user wrote it as an elaborated type 
specifier, we should probably retain that when shifting it to a trailing return 
type.



================
Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:132
+  // If the return type has no local qualifiers, it's source range is accurate.
+  if (!F.getReturnType().hasLocalQualifiers())
+    return ReturnTypeRange;
----------------
I think you may need to check here if the return type is an elaborated type 
(e.g., one that is written as `struct S` rather than `S`).


================
Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:153
+        !ExtendedLeft) {
+      for (int J = static_cast<int>(I) - 1; J >= 0 && IsCV(Tokens[J]); J--)
+        ReturnTypeRange.setBegin(Tokens[J].getLocation());
----------------
And then look for the elaborated tag type here.


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

https://reviews.llvm.org/D56160



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

Reply via email to