sammccall accepted this revision.
sammccall added a subscriber: mibintc.
sammccall added a comment.
This revision is now accepted and ready to land.
LG because not crashing here is surely better than crashing, but I'm not sure
whether the behaviour is actually right.
If we can't be sure, maybe leave a comment?
================
Comment at: clang/lib/Sema/SemaDecl.cpp:14496
- if (FSI->UsesFPIntrin && !FD->hasAttr<StrictFPAttr>())
+ if (FSI->UsesFPIntrin && FD && !FD->hasAttr<StrictFPAttr>())
FD->addAttr(StrictFPAttr::CreateImplicit(Context));
----------------
hokein wrote:
> I have a reproduce test case, and wait for the creduce to minimize it (will
> add it once creduce finishes)
>
> I think the bug is obvious, by reading the code on the line 14495, FD could
> be a nullptr.
Yes, the bug is clear.
It's not obvious to me that the fix is right, because I don't know:
- when dcl can be null
- when/if dcl can be non-null but neither a function nor function template
- how the FP attr should apply in such cases
cc @mibintc who may know the answer to at least the 3rd question, and I guess
your testcase will give an example of either 1 or 2.
I'm *fairly* sure that not applying the strictfpattr is better than crashing
here though.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110315/new/
https://reviews.llvm.org/D110315
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
