[PATCH] D146426: [Sema] Fix crash on __fp16 parameters in template instantiations

ilya-biryukov added a comment. In D146426#4209620 , @aaron.ballman wrote: > Thank you for offering to do that in a follow-up, but please, next time wait > for there to be agreement on the patch before landing it. Multiple reviewers > expressed concerns

[PATCH] D146426: [Sema] Fix crash on __fp16 parameters in template instantiations

aaron.ballman added a comment. In D146426#4208960 , @ilya-biryukov wrote: > In D146426#4207423 , @shafik wrote: > >> As I noted in the bug report not doing `D.setInvalidType();` does fix this >> bug and seems ha

[PATCH] D146426: [Sema] Fix crash on __fp16 parameters in template instantiations

ilya-biryukov added a comment. I landed the fix to unbreak our crashes and explored a bit of the alternative solution. Digging a bit deeper, trying to always set non-null parameters causes ~30 test failures, but the ones I looked at so far look more localized and should be fixable. Some starte

[PATCH] D146426: [Sema] Fix crash on __fp16 parameters in template instantiations

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG282cae0b9a60: [Sema] Fix crash on __fp16 parameters in template instantiations (authored by ilya-biryukov). Repository: rG LLVM Github Monorepo C

[PATCH] D146426: [Sema] Fix crash on __fp16 parameters in template instantiations

ilya-biryukov updated this revision to Diff 506934. ilya-biryukov added a comment. - Add a test for block pointers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146426/new/ https://reviews.llvm.org/D146426 Files: clang/lib/Sema/SemaChecking.cpp

[PATCH] D146426: [Sema] Fix crash on __fp16 parameters in template instantiations

ilya-biryukov added a comment. In D146426#4207423 , @shafik wrote: > As I noted in the bug report not doing `D.setInvalidType();` does fix this > bug and seems harmless since the error diagnostic should prevent us from > getting to codegen but it is not

[PATCH] D146426: [Sema] Fix crash on __fp16 parameters in template instantiations

shafik added a comment. As I noted in the bug report not doing `D.setInvalidType();` does fix this bug and seems harmless since the error diagnostic should prevent us from getting to codegen but it is not clear to me if this has other negative impacts. Reading your replies it is not obvious you

[PATCH] D146426: [Sema] Fix crash on __fp16 parameters in template instantiations

ilya-biryukov added a comment. In D146426#4207118 , @aaron.ballman wrote: > This feels like it's heading in the wrong direction -- the AST should not > have holes in it. An invalid type should be replaced by a valid type (after > diagnosing the invalid

[PATCH] D146426: [Sema] Fix crash on __fp16 parameters in template instantiations

erichkeane added a comment. In D146426#4207118 , @aaron.ballman wrote: > This feels like it's heading in the wrong direction -- the AST should not > have holes in it. An invalid type should be replaced by a valid type (after > diagnosing the invalid ty

[PATCH] D146426: [Sema] Fix crash on __fp16 parameters in template instantiations

aaron.ballman added reviewers: erichkeane, aaron.ballman. aaron.ballman added a comment. This feels like it's heading in the wrong direction -- the AST should not have holes in it. An invalid type should be replaced by a valid type (after diagnosing the invalid type, of course) so that we can ke

[PATCH] D146426: [Sema] Fix crash on __fp16 parameters in template instantiations

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. as discussed offline, this feels a little fishy and we should probably try and not put nulls into the parameter lists at all (and mark the functiontype as invalid instead), but since i don

[PATCH] D146426: [Sema] Fix crash on __fp16 parameters in template instantiations

ilya-biryukov added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:1714 /// Check whether a function's parameter types are all literal types. If so, /// return true. If not, produce a suitable diagnostic and return false. static bool CheckConstexprParameterTypes(S

[PATCH] D146426: [Sema] Fix crash on __fp16 parameters in template instantiations

ilya-biryukov updated this revision to Diff 506623. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - rename test file to GH61441.cpp, add a comment this checks for no crash - Update outdated comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D146426: [Sema] Fix crash on __fp16 parameters in template instantiations

shafik added a reviewer: clang-language-wg. shafik added a comment. Adding clang-language-wg for more visibility Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146426/new/ https://reviews.llvm.org/D146426 ___

[PATCH] D146426: [Sema] Fix crash on __fp16 parameters in template instantiations

shafik added a comment. In D146426#4206525 , @ilya-biryukov wrote: > In D146426#4206519 , @shafik wrote: > >> Please edit the summary to indicate this fixes: >> https://github.com/llvm/llvm-project/issues/61441

[PATCH] D146426: [Sema] Fix crash on __fp16 parameters in template instantiations

shafik added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:1714 /// Check whether a function's parameter types are all literal types. If so, /// return true. If not, produce a suitable diagnostic and return false. static bool CheckConstexprParameterTypes(Sema &Se

[PATCH] D146426: [Sema] Fix crash on __fp16 parameters in template instantiations

shafik added inline comments. Comment at: clang/test/SemaCXX/crash-params.cpp:3 + +template +int foo() { If this test really does not fit into any other test set the please rename the test file `GH61441.cpp` so we know it is a regression test for that github bu

[PATCH] D146426: [Sema] Fix crash on __fp16 parameters in template instantiations

ilya-biryukov added a comment. In D146426#4206519 , @shafik wrote: > Please edit the summary to indicate this fixes: > https://github.com/llvm/llvm-project/issues/61441 > I am happy someone had more time to dig into this problem after my initial > inves

[PATCH] D146426: [Sema] Fix crash on __fp16 parameters in template instantiations

shafik added a comment. Please edit the summary to indicate this fixes: https://github.com/llvm/llvm-project/issues/61441 I am happy someone had more time to dig into this problem after my initial investigation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D146426: [Sema] Fix crash on __fp16 parameters in template instantiations

ilya-biryukov created this revision. ilya-biryukov added a reviewer: kadircet. Herald added a project: All. ilya-biryukov requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Currently, Clang stores `nullptr` in the parameter lists inside `Funct