-
Notifications
You must be signed in to change notification settings - Fork 12.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ICE on template template parameter with constraints #57410
Comments
@llvm/issue-subscribers-clang-frontend |
Assertion trunk: https://godbolt.org/z/3x7ca9r9o |
I've seen a good amount of this problem around concepts instantiation (or forms of this issue at least). It DOES however seem that my deferred concepts implementation fixes this, even with an instantiation after the fact. So once that gets in, I'd expect this to be closable. |
Looks like https://reviews.llvm.org/D126907 fixed this! Closing. |
This is still happening on trunk, so I'm reopening now. https://godbolt.org/z/aoe8hqh6x @erichkeane Have we altered anything after babdef2? In terms of constraints on template-template parameters, does CWG have any explanations on "if the check should happen or not" after our implementation for P0857? If still not yet, is the (possible) fix that skips the check for template-template parameters in llvm-project/clang/lib/Sema/SemaConcept.cpp Lines 427 to 441 in 4b2f118
|
I was wrong. Our previous behavior (clang-16) was to check these constraints. The bug seems to be inside llvm-project/clang/lib/Sema/SemaTemplateInstantiate.cpp Lines 2210 to 2215 in 9e1ad3c
|
llvm-project/clang/lib/Sema/SemaTemplateInstantiate.cpp Lines 348 to 352 in 9e1ad3c
Our template-template parameter Decl is at the file scope, and this code changes the current scope to its parent so that we won't fall into the following loop. CC @alexander-shaposhnikov |
Filed a patch trying to resolve it. Hopefully, I'm getting on the right track. |
…e template parameters This fixes the bug introduced by llvm@6db007a. We construct placeholder template arguments for template-template parameters to avoid mismatching argument substitution since they have different depths with their corresponding template arguments. In this case, ```cpp template <template <Concept C> class T> void foo(T<int>); ``` T lies at the depth 0, and C lies at 1. The corresponding argument, of which there is exactly one, int, is at depth 0. If we consider the argument as the outermost one, then we would end up substituting 'int' into the wrong parameter T. We used to perform such placeholder construction during the context walk-up. In the previous patch, we slipped through that inadvertently because we would walk up to the parent, which is precisely a FileContext for template-template parameters, after adding innermost arguments. Besides, this patch moves the sanity check up to the context switch. That way, we avoid dereferencing null pointers if ND is unspecified. Closes llvm#57410.
…parameters (#76811) This fixes the bug introduced by 6db007a. We construct placeholder template arguments for template-template parameters to avoid mismatching argument substitution since they have different depths with their corresponding template arguments. In this case, ```cpp template <template <Concept C> class T> void foo(T<int>); ``` T lies at the depth 0, and C lies at 1. The corresponding argument, of which there is exactly one, int, is at depth 0. If we consider the argument as the outermost one, then we would end up substituting 'int' into the wrong parameter T. We used to perform such placeholder construction during the context walk-up. In the previous patch, we slipped through that inadvertently because we would walk up to the parent, which is precisely a FileContext for template-template parameters, after adding innermost arguments. Besides, this patch moves the sanity check up to the context switch. That way, we avoid dereferencing null pointers if ND is unspecified. Closes #57410. Closes #76604. (The case is slightly different than that in #57410. We should *not* assume the surrounding context to be a file-scope one.)
…parameters (llvm#76811) This fixes the bug introduced by llvm@6db007a. We construct placeholder template arguments for template-template parameters to avoid mismatching argument substitution since they have different depths with their corresponding template arguments. In this case, ```cpp template <template <Concept C> class T> void foo(T<int>); ``` T lies at the depth 0, and C lies at 1. The corresponding argument, of which there is exactly one, int, is at depth 0. If we consider the argument as the outermost one, then we would end up substituting 'int' into the wrong parameter T. We used to perform such placeholder construction during the context walk-up. In the previous patch, we slipped through that inadvertently because we would walk up to the parent, which is precisely a FileContext for template-template parameters, after adding innermost arguments. Besides, this patch moves the sanity check up to the context switch. That way, we avoid dereferencing null pointers if ND is unspecified. Closes llvm#57410. Closes llvm#76604. (The case is slightly different than that in llvm#57410. We should *not* assume the surrounding context to be a file-scope one.)
The following program
results in segfault in Clang:
Online demo: https://godbolt.org/z/TeWo7nsEd
Found in the discussion: https://stackoverflow.com/a/73494870/7325599
The text was updated successfully, but these errors were encountered: