[PATCH] D128256: [Clang][AArch64] Limit arm_locally_streaming to function definitions only.

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added inline comments.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:9835-9839
+  if (D.isFunctionDefinition()) {
+    NewFD->setWillHaveBody();
+    ProcessDeclAttributes(S, NewFD, D);
+    NewFD->setWillHaveBody(false);
+  } else
----------------
aaron.ballman wrote:
> This seems like a hack to work around what feels like a bug -- if the declarator knows the function is a definition, then why does the `FunctionDecl` AST node claim the function won't have a body? It seems strange to me that we don't set that bit when acting on the function declarator but instead wait until we're acting on the start of the function definition to set it; does anything break if you start setting that flag earlier?
> This seems like a hack to work around what feels like a bug -- if the declarator knows the function is a definition, then why does the FunctionDecl AST node claim the function won't have a body?
I agree it is a bit of a workaround, I wasn't sure if it was a bug or by design, but there are several places in the codebase that are invoked after this point that seem to depend on the fact that `willHaveBody` has not been set to determine if something is a redeclaration.

For example, setting NewFD->setWillHaveBody() to `true` if `D.isFunctionDefinition()`, causes the following test to fail:

```template <class T1, class T2>
struct pair {
  T1 first;
  T2 second;

  pair() : first(), second() {}
  pair(const T1 &a, const T2 &b) : first(a), second(b) {}

  template<class U1, class U2>
  pair(const pair<U1, U2> &other) : first(other.first),
                                    second(other.second) {}
};
```

I did some digging to see if I could fix this some other way, but didn't spot any easy ways to do this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128256