-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[clang][Sema] Fix Issue: #166957 - type inconsistency for constrained auto variables #167100
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
base: main
Are you sure you want to change the base?
Conversation
KaushalMorankar
commented
Nov 8, 2025
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-clang Author: Kaushal1042 (KaushalMorankar) ChangesFixes #<!-- -->166957
This PR fixes an inconsistency where `getTypeSourceInfo()->getType()` and `getType()` return different `AutoType` instances with different `isDependentType()` values for constrained auto variables in dependent contexts.
Solution
The fix adds logic in ActOnVariableDeclarator (clang/lib/Sema/SemaDecl.cpp:7700-7719) to check if a constrained AutoType is being created, and ensures its dependency status matches the declaration context:
- In dependent contexts (e.g., templates): undeduced constrained autos are marked as dependent
- In non-dependent contexts: they are not marked as dependent
The fix reconstructs the AutoType with the correct dependency flag and updates the TypeSourceInfo to maintain consistency between both type retrieval methods.
Testing
Added comprehensive test coverage in clang/test/SemaCXX/constrained-auto-type-consistency.cpp which includes:
- Constrained auto in abbreviated function templates (dependent context)
- Constrained auto at namespace scope (non-dependent context) etc.
All tests pass with expected-no-diagnostics, confirming the fix resolves the inconsistency without introducing new issues.
Files Changed
- clang/lib/Sema/SemaDecl.cpp: Added logic to ensure AutoType dependency consistency
- clang/test/SemaCXX/constrained-auto-type-consistency.cpp: New test file (92 lines)
---
Full diff: https://github.com/llvm/llvm-project/pull/167100.diff
2 Files Affected:
- (modified) clang/lib/Sema/SemaDecl.cpp (+20)
- (added) clang/test/SemaCXX/constrained-auto-type-consistency.cpp (+92)
``````````diff
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 086dd8ba1c670..99877615c50bf 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -7697,6 +7697,26 @@ NamedDecl *Sema::ActOnVariableDeclarator(
LookupResult &Previous, MultiTemplateParamsArg TemplateParamLists,
bool &AddToScope, ArrayRef<BindingDecl *> Bindings) {
QualType R = TInfo->getType();
+
+ if (const AutoType *AT = R->getContainedAutoType()) {
+ if (AT->isConstrained()) {
+ bool IsInDependentContext = DC->isDependentContext();
+ bool ShouldBeDependent = IsInDependentContext && !AT->isDeduced();
+
+ if (ShouldBeDependent != AT->isDependentType()) {
+ QualType CanonAuto = Context.getAutoType(
+ AT->isDeduced() ? AT->getDeducedType() : QualType(),
+ AT->getKeyword(),
+ ShouldBeDependent,
+ false,
+ AT->getTypeConstraintConcept(),
+ AT->getTypeConstraintArguments());
+
+ R = Context.getQualifiedType(CanonAuto, R.getQualifiers());
+ TInfo = Context.getTrivialTypeSourceInfo(R, D.getBeginLoc());
+ }
+ }
+ }
DeclarationName Name = GetNameForDeclarator(D).getName();
IdentifierInfo *II = Name.getAsIdentifierInfo();
diff --git a/clang/test/SemaCXX/constrained-auto-type-consistency.cpp b/clang/test/SemaCXX/constrained-auto-type-consistency.cpp
new file mode 100644
index 0000000000000..7f6cec1604151
--- /dev/null
+++ b/clang/test/SemaCXX/constrained-auto-type-consistency.cpp
@@ -0,0 +1,92 @@
+// expected-no-diagnostics
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+
+namespace std {
+ template<typename T>
+ concept integral = __is_integral(T);
+
+ template<typename T>
+ concept floating_point = __is_floating_point(T);
+}
+
+// Constrained auto in abbreviated function template
+void find(auto value) {
+ std::integral auto var = value;
+}
+
+// Constrained auto at namespace scope (non-dependent context)
+// Should be deduced immediately
+std::integral auto globalVar = 42;
+
+// Multiple constrained autos in template function
+template<typename T>
+void multipleConstrainedAutos(T value) {
+ std::integral auto x = 10;
+ std::floating_point auto y = 3.14;
+ std::integral auto z = value; // dependent on T
+}
+
+// Constrained auto with qualifiers
+void testQualifiers(auto value) {
+ const std::integral auto cv1 = value;
+ std::integral auto const cv2 = value;
+}
+
+// Nested constrained auto
+void testNested(auto outer) {
+ auto lambda = [](auto inner) {
+ std::integral auto nested = inner;
+ return nested;
+ };
+
+ std::integral auto result = lambda(outer);
+}
+
+// Constrained auto with references
+void testReferences(auto value) {
+ std::integral auto& ref = value;
+ const std::integral auto& cref = value;
+}
+
+// Regular unconstrained auto (should not be affected by the fix)
+void testUnconstrainedAuto(auto value) {
+ auto regular = value;
+ decltype(auto) decl_auto = (value);
+}
+
+// Constrained auto in class template member
+template<typename T>
+struct Container {
+ void process(auto item) {
+ std::integral auto local = item;
+ }
+};
+
+// Constrained auto deduction from function call
+std::integral auto getInteger() { return 42; }
+
+void testFunctionReturn(auto param) {
+ std::integral auto fromFunc = getInteger();
+ std::integral auto fromParam = param;
+}
+
+// Ensure the fix doesn't break normal non-template constrained auto
+void normalFunction() {
+ std::integral auto x = 100;
+ // This should be immediately deduced to int, not dependent
+}
+
+// Instantiate templates to verify no crashes
+void instantiateAll() {
+ find(42);
+ multipleConstrainedAutos(5);
+ testQualifiers(7);
+ testNested(8);
+ int val = 10;
+ testReferences(val);
+ testUnconstrainedAuto(11);
+ Container<int> c;
+ c.process(12);
+ testFunctionReturn(13);
+ normalFunction();
+}
|
|
Can you explain how the added test verifies that the issue is fixed? |
|
The bug was in the internal type consistency between getTypeSourceInfo()->getType() and getType(). I added compilation tests that serve as regression tests to ensure these patterns continue to compile correctly. While these tests don't directly verify the AST |
zyn0217
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you AI-generate the patch? I would like to see some explanation as to why there was a discrepancy with TSI, and unfortunately your commit message doesn't seem to explain it very clearly.
If we were creating two AutoTypes, I think a right fix would be to combine them together in some way, other than adding odd checking in Sema without a reasonable explanation.
Besides, do we have similar issue on type constraints? E.g.
void foo(Concept auto Param) {}
Concept auto bar(int) {}
shafik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it came from this bug report: #166957
I also don't understand the solution and it is not clear to me this is correct.
|
I am abroad and can't review in depth right now. But this inconsistency between the type and type source info is intentional and long standing. I don't think there is a bug here except the interface is more complicated than the ideal. The type source info holds the type as written, and the type holds the semantic type. It would be nice if this merged the two at no extra cost, but I'd have to double check this later. |
| bool &AddToScope, ArrayRef<BindingDecl *> Bindings) { | ||
| QualType R = TInfo->getType(); | ||
|
|
||
| if (const AutoType *AT = R->getContainedAutoType()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing this as a fixup seems wrong to me here. IF we wanted to merge these, we should do so at the root.
Also, your tests dont' really seem related to this, what is going on?