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

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

Reply via email to