[PATCH] D98895: [X86][clang] Disable long double type for -mno-x87 option

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:9573
 
-  checkTypeSupport(NewFD->getType(), D.getBeginLoc(), NewFD);
+  if (D.getFunctionDefinitionKind() != FunctionDefinitionKind::Declaration)
+    checkTypeSupport(NewFD->getType(), D.getBeginLoc(), NewFD);
----------------
asavonic wrote:
> erichkeane wrote:
> > Why are we not checking declarations here?  This doesn't seem right to me.  At least in the offload languages, we still need to check declarations.  Also, if something is a problem with a declaration, why is it not a problem with defaulted/deleted?
> > Why are we not checking declarations here?  This doesn't seem right to me.  At least in the offload languages, we still need to check declarations.
> 
> I assume that if if a function is declared and not used, then it is discarded and no code is generated for it. So it should not really matter if it uses an "unsupported" type.
> This is important for `long double`, because there are C standard library functions that have `long double` arguments. We skip diagnostics for declarations to avoid errors from standard library headers when the type is actually not used.
> 
> > Also, if something is a problem with a declaration, why is it not a problem with defaulted/deleted?
> 
> If we can expect that defaulted or deleted functions never result in a code with unsupported types, then we can exclude them as well. Something like this perhaps?
> ```
> void no_long_double(long double) = delete;
> ```
The problem is that these declarations could be called, right?  So are we catching something like:

``` void has_long_double(long double d);
....
has_long_double(1.0); 
```

The deleted types shouldn't result in code being generated, but default will absolutely result in code being generated. Though I guess I can't think of a situation where we would have a defaulted function that could take a `long double` anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98895