usaxena95 added inline comments.
================
Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:33
+ if (!Offset) {
+ llvm::errs() << "Unable to convert postion to offset";
+ return {};
----------------
sammccall wrote:
> we use log() or elog() for this, make sure you include Offset.takeError() for
> the original message
>
> Alternatively, you might consider returning Expected<SemanticSelectionResult>
> so the error gets propagated to the client (as this really is a client error)
Returning Expected<vector<Ranges>> now to propagate the error.
================
Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:39
+ SelectionTree ST(AST.getASTContext(), AST.getTokens(), *Offset);
+ for (const auto *Node = ST.commonAncestor(); Node != nullptr;
+ Node = Node->Parent) {
----------------
sammccall wrote:
> I think you probably want to bail out once you hit TranslationUnitDecl.
>
> And maybe at namespace decls? Though I have often in my editor wondered
> "what's the enclosing namespace"...
Bailing out once we hit the TUDecl. I am not sure about exiting on namespace
decl. I am keeping this for now. We can remove this later if the need be.
================
Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:45
+ }
+ SourceLocation BeginLoc = SR.getBegin();
+ SourceLocation EndLoc =
----------------
sammccall wrote:
> Ah, this will work if the start and the end of the AST node are written
> directly in the file, and no macros are involved. But try
> `toHalfOpenFileRange` in `SourceCode.h`, which should work in a lot more
> cases.
>
> In particular, this should work:
> ```
> #define INC(X) X + 1
> int a, b;
> int c = INC(a * b);
> ^
> ~
> ~~~~~
> ~~~~~~~~~~
> ~~~~~~~~~~~~~~~~~~
> ```
Interesting. This works fine in such cases. Thanks.
Though it works strangely with these ugly macros. I guess this is because it
tries to follow the macro expansion.
```
#define MUL ) * (
int var = (4 + 15 MUL 6 + 10);
^
~~
~~~~~~
~~~~~~~~~~~
```
================
Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:115
+ [[void func() [[{
+ // int x = nsa::nsb::ccc();
+ [[[[int x = [[[[nsa::nsb::cc^c]]()]]]];]]
----------------
sammccall wrote:
> why commented out?
Since the statement contains many [[ and ]], I have added the actual statement
in comments to make it more readable.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67358/new/
https://reviews.llvm.org/D67358
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
