Source range of deleted or defaulted function outside class body is inaccurate

[smcpeak]

Problem

For this translation unit:

void delfunc() = delete;

the source range of delfunc should cover the entire text to the final e in delete, but when using Clang 16.0.0 (and also a recent-ish trunk build from source), it does not:

$ clang -Xclang -ast-dump -fsyntax-only tmp.cc
[...]
`-FunctionDecl 0x5613b9a09b28 <tmp.cc:1:1, col:14> col:6 delfunc 'void ()' delete implicit-inline
                                               ^^ expecting 23 here

The same thing happens for methods defined outside their class body:

$ cat tmp3.cc
struct S {
  inline S();
};

inline S::S() = default;

$ clang -Xclang -ast-dump -fsyntax-only tmp3.cc
[...]
`-CXXConstructorDecl 0x557217b67278 parent 0x557217b66af0 prev 0x557217b66d28 <line:5:1, col:13> col:11 used S 'void ()' inline default
  `-CompoundStmt 0x557217b67368 <col:13>

where col:13 should be col:23.

Suspected origin

Commit 5f4d76e (2015-03-23) addressed a similar issue for declarations in the class body in ParseCXXInlineMethods.cpp, but did not change the handling of those outside class bodies in Parser.cpp.

Rough sketch of a fix

The following diff handles the simple testcases above. However, it does not handle templates, and I don't know enough about the parser to fix that myself. I also have not done any testing with real code. So, this diff is merely evidence that something in this vicinity probably needs to change rather than a serious proposal for exactly how to change it.

--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -1354,6 +1359,10 @@ Decl *Parser::ParseFunctionDefinition(ParsingDeclarator &D,
   ParseScope BodyScope(this, Scope::FnScope | Scope::DeclScope |
                                  Scope::CompoundStmtScope);
 
+  // If this is set, we need to adjust the end location to account for
+  // the '= delete' or '= default'.
+  SourceLocation KWEndLoc_toUpdate;
+
   // Parse function body eagerly if it is either '= delete;' or '= default;' as
   // ActOnStartOfFunctionDef needs to know whether the function is deleted.
   Sema::FnBodyKind BodyKind = Sema::FnBodyKind::Other;
@@ -1361,18 +1370,21 @@ Decl *Parser::ParseFunctionDefinition(ParsingDeclarator &D,
   if (TryConsumeToken(tok::equal)) {
     assert(getLangOpts().CPlusPlus && "Only C++ function definitions have '='");
 
+    SourceLocation KWEndLoc = Tok.getEndLoc().getLocWithOffset(-1);
     if (TryConsumeToken(tok::kw_delete, KWLoc)) {
       Diag(KWLoc, getLangOpts().CPlusPlus11
                       ? diag::warn_cxx98_compat_defaulted_deleted_function
                       : diag::ext_defaulted_deleted_function)
           << 1 /* deleted */;
       BodyKind = Sema::FnBodyKind::Delete;
+      KWEndLoc_toUpdate = KWEndLoc;
     } else if (TryConsumeToken(tok::kw_default, KWLoc)) {
       Diag(KWLoc, getLangOpts().CPlusPlus11
                       ? diag::warn_cxx98_compat_defaulted_deleted_function
                       : diag::ext_defaulted_deleted_function)
           << 0 /* defaulted */;
       BodyKind = Sema::FnBodyKind::Default;
+      KWEndLoc_toUpdate = KWEndLoc;
     } else {
       llvm_unreachable("function definition after = not 'delete' or 'default'");
     }
@@ -1398,6 +1410,13 @@ Decl *Parser::ParseFunctionDefinition(ParsingDeclarator &D,
                                                   : MultiTemplateParamsArg(),
                                               &SkipBody, BodyKind);
 
+  if (KWEndLoc_toUpdate.isValid()) {
+    // TODO: This does not handle templates.
+    if (auto *DeclAsFunction = dyn_cast<FunctionDecl>(Res)) {
+      DeclAsFunction->setRangeEnd(KWEndLoc_toUpdate);
+    }
+  }
+
   if (SkipBody.ShouldSkip) {
     // Do NOT enter SkipFunctionBody if we already consumed the tokens.
     if (BodyKind == Sema::FnBodyKind::Other)

[llvmbot]

@llvm/issue-subscribers-clang-frontend

[smcpeak]

This issue started as a question on Stack Overflow: https://stackoverflow.com/questions/76924637/how-to-find-the-clangsourcerange-of-a-deleted-function .

[llvmbot]

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

1. Assign the issue to you.
2. Fix the issue locally.
3. Run the test suite locally.
   3.1) Remember that the subdirectories under test/ create fine-grained testing targets, so you can
   e.g. use make check-clang-ast to only run Clang's AST tests.
4. Create a git commit
5. Run git clang-format HEAD~1 to format your changes.
6. Submit the patch to Phabricator.
   6.1) Detailed instructions can be found here

For more instructions on how to submit a patch to LLVM, see our documentation.

If you have any further questions about this issue, don't hesitate to ask via a comment on this Github issue.

@llvm/issue-subscribers-good-first-issue

[nox1134]

Hii @smcpeak ! I'm new to open source and would like to work on this issue. How do i assign this issue to myself?

[smcpeak]

@nox1134 As far as I know, you have to have write access to the repository to assign an issue, per Assigning issues and pull requests to other GitHub users. But I take it from the Developer Policy that granting direct write access is unusual (understandably). So I don't know how to follow the bot's instructions above, sorry! I haven't made any contributions to LLVM myself.

Perhaps @cor3ntin could advise.

[rajkumarananthu]

Hi everyone, if no one is working on this issue, I would like to take up this issue and work on it.
Please assign the issue to me.

Thanks
Rajkumar Ananthu.

[erichkeane]

Unassigning as this doesn't seem to have any progress on it. Feel free to pick it back up if you wish to work on it again. Same offer up to others!