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 5f4d76efd365c (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.