Fix highlighting issue with _complex and initialization list with more than 2 items

[chaitanyav]

Summary

Fixes https://github.com/llvm/llvm-project/issues/61518

[chaitanyav]

test.cpp:2:34: error: excess elements in scalar initializer
_Complex double dd = {1.0, 2.0 , 3.0};
                                 ^~~
test.cpp:3:32: error: excess elements in scalar initializer
_Complex float fd = {1.0, 2.0, 3.0, 4.0, 5.0};
                               ^~~
test.cpp:4:42: error: excess elements in scalar initializer
constexpr _Complex double d = {1.0, 2.0, 3.0, 45};
                                         ^~~
test.cpp:11:23: error: no viable conversion from 'foo' to '_Complex double'
_Complex double ds = {f, 1.0, b};
                      ^
test.cpp:13:28: error: no viable conversion from 'foo' to 'double'
_Complex double fg = {1.0, f};
                           ^
test.cpp:15:33: error: excess elements in scalar initializer
_Complex double gg = {1.0, 2.0, f};
                                ^
6 errors generated.

Tested various scenarios to ensure the behavior is as expected. Please review

[tbaeder]

I think the problem is that in https://github.com/llvm/llvm-project/blob/25ca26e0da2e1f80d62f71807828762691a049ac/clang/lib/Sema/SemaInit.cpp#L1532-L1541, the code checks for Index != 2 and then handles it as a single scalar initializer.

[chaitanyav]

It will fall through to scalar type to do field wise initialization

_Complex double gx = {1.0};

_Complex double dx = {};

I think i get what you are saying, we can directly say that it has excess elements without having to do the field wise initialization. Let me look into that

[chaitanyav]

Only proceed with scalar initialization if number of elements is less than 2

[chaitanyav]

test outputs for C, C++

_Complex float invalid2 = { 1, 2, 3 };
                                  ^
test.cpp:2:34: error: excess elements in scalar initializer
_Complex double dd = {1.0, 2.0 , 3.0};
                                 ^~~
test.cpp:3:32: error: excess elements in scalar initializer
_Complex float fd = {1.0, 2.0, 3.0, 4.0, 5.0};
                               ^~~
test.cpp:4:42: error: excess elements in scalar initializer
constexpr _Complex double d = {1.0, 2.0, 3.0, 45};
                                         ^~~
test.cpp:11:23: error: no viable conversion from 'foo' to 'double'
_Complex double ds = {f, 1.0, b};
                      ^
test.cpp:13:28: error: no viable conversion from 'foo' to 'double'
_Complex double fg = {1.0, f};
                           ^
test.cpp:15:33: error: excess elements in scalar initializer
_Complex double gg = {1.0, 2.0, f};
                                ^
7 errors generated.

test.c:1:35: warning: excess elements in scalar initializer [-Wexcess-initializers]
_Complex float invalid2 = { 1, 2, 3 };
                                  ^
test.c:2:34: warning: excess elements in scalar initializer [-Wexcess-initializers]
_Complex double dd = {1.0, 2.0 , 3.0};
                                 ^~~
test.c:3:32: warning: excess elements in scalar initializer [-Wexcess-initializers]
_Complex float fd = {1.0, 2.0, 3.0, 4.0, 5.0};
                               ^~~
test.c:4:38: warning: excess elements in scalar initializer [-Wexcess-initializers]
const _Complex double d = {1.0, 2.0, 3.0, 45};
                                     ^~~
test.c:11:23: error: initializing 'double' with an expression of incompatible type 'struct foo'
_Complex double ds = {f, 1.0, b};
                      ^
test.c:13:28: error: initializing 'double' with an expression of incompatible type 'struct foo'
_Complex double fg = {1.0, f};
                           ^
test.c:15:33: warning: excess elements in scalar initializer [-Wexcess-initializers]
_Complex double gg = {1.0, 2.0, f};
                                ^
test.c:19:22: error: scalar initializer cannot be empty
_Complex double dx = {};

[chaitanyav]

Please review this.

[tbaeder]

LGTM but I'll wait for @aaron.ballman to maybe comment on the diagnostic differences in C.

[aaron.ballman]

Thanks for this!

One thing that's missing is test coverage for the highlight itself, as that was what the original issue was about. You should add a test that uses -fcaret-diagnostics-max-lines and FileCheck so that you can test that the caret shows up where expected (along the lines of https://github.com/llvm/llvm-project/blob/main/clang/test/Misc/caret-diags-multiline.cpp).

You should also add a release note about the fix to clang/docs/ReleaseNotes.rst

clang/test/Sema/complex-init-list.c

38–39

This makes it a bit easier to see that there are two diagnostics being fired (even though we're not consistently doing that in this test file).

55–56

[chaitanyav]

Added tests to check for the right diagnostic messages and highlighting at correct place
Update the release docs about the fix

[chaitanyav]

@aaron.ballman Please review.

clang/test/Sema/complex-init-list.c

38–39

Yeah, one because its an extension and other because it has excess elements. Checked this file added that on lines its applicable.

[chaitanyav]

I reported the flang failures https://github.com/llvm/llvm-project/issues/61634 and it was reverted. How can i trigger a new build with TOT?

[aaron.ballman]

The precommit CI failures are unrelated to your changes (the Debian one is related to flang failures and the libcxx one seems to be related to infrastructure), so no need to worry about them.

clang/test/Sema/complex-init-list.c

38–39

I'd still like to see these two changes made (where we use a line continuation character to put the diagnostic messages on their own lines instead of mashed together on one long line).

[chaitanyav]

one diagnostic per line when there are multiple diagnostics expected

[aaron.ballman]

LGTM! Do you need someone to commit on your behalf? If so, what name and email address would you like used for patch attribution?

[chaitanyav]

Thank you, I will commit myself. Should i wait for the build to finish?. this will be my first direct commit to repo.

[aaron.ballman]

In D146503#4217541, @chaitanyav wrote:

Thank you, I will commit myself. Should i wait for the build to finish?. this will be my first direct commit to repo.

Sorry for not getting back to you sooner. In general it's good to wait for precommit CI to finish just to be sure. Congrats on your first direct commit! :-)

clang/docs/ReleaseNotes.rst

- Fix crash when using ``[[clang::always_inline]]`` or ``[[clang::noinline]]``
   statement attributes on a call to a template function in the body of a
   template function.
 - Fix coroutines issue where ``get_return_object()`` result was always eargerly
   converted to the return type. Eager initialization (allowing RVO) is now only
   perfomed when these types match, otherwise deferred initialization is used,
   enabling short-circuiting coroutines use cases. This fixes
   (`#56532 <https://github.com/llvm/llvm-project/issues/56532>`_) in
   antecipation of `CWG2563 <https://cplusplus.github.io/CWG/issues/2563.html>_`.
+- Fix highlighting issue with ``_Complex`` and initialization list with more than
+  2 items. (`#61518 <https://github.com/llvm/llvm-project/issues/61518>`_)
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Bug Fixes to Attribute Support
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Bug Fixes to C++ Support

clang/lib/Sema/SemaInit.cpp

void InitListChecker::CheckComplexType(const InitializedEntity &Entity,
 
   // As an extension, clang supports complex initializers, which initialize
   // a complex number component-wise.  When an explicit initializer list for
   // a complex number contains two initializers, this extension kicks in:
   // it expects the initializer list to contain two elements convertible to
   // the element type of the complex type. The first element initializes
   // the real part, and the second element intitializes the imaginary part.
 
-  if (IList->getNumInits() != 2)
+  if (IList->getNumInits() < 2)
     return CheckScalarType(Entity, IList, DeclType, Index, StructuredList,
                            StructuredIndex);
 
   // This is an extension in C.  (The builtin _Complex type does not exist
   // in the C++ standard.)
   if (!SemaRef.getLangOpts().CPlusPlus && !VerifyOnly)
     SemaRef.Diag(IList->getBeginLoc(), diag::ext_complex_component_init)
         << IList->getSourceRange();

clang/test/Sema/caret-diags-complex-init.cpp

+// RUN: not %clang_cc1 -std=c++11 -fsyntax-only -fcaret-diagnostics-max-lines 5 %s 2>&1 | FileCheck %s -strict-whitespace
+
+
+//CHECK: {{.*}}: error: excess elements in scalar initializer
+//CHECK-NEXT: {{^}}_Complex double gz1 = {1, 2, 3};
+//CHECK-NEXT: {{^}}                             ^{{$}}
+_Complex double gz1 = {1, 2, 3}; 
+
+//CHECK: {{.*}}: error: excess elements in scalar initializer
+//CHECK-NEXT: {{^}}_Complex double dd = {1.0, 2.0, 3.0};
+//CHECK-NEXT: {{^}}                                ^~~{{$}}
+_Complex double dd = {1.0, 2.0, 3.0};
+
+//CHECK: {{.*}}: error: excess elements in scalar initializer
+//CHECK-NEXT: {{^}}_Complex float fd = {1.0, 2.0, 3.0, 4.0, 5.0};
+//CHECK-NEXT: {{^}}                               ^~~{{$}}
+_Complex float fd = {1.0, 2.0, 3.0, 4.0, 5.0};
+
+//CHECK: {{.*}}: error: no viable conversion from 'foo' to 'double'
+//CHECK-NEXT: {{^}}_Complex double ds = {f, 1.0, b};
+//CHECK-NEXT: {{^}}                      ^{{$}}
+struct foo{};
+struct bar{};
+
+foo f;
+bar b;
+_Complex double ds = {f, 1.0, b};
+
+//CHECK: {{.*}}: error: no viable conversion from 'foo' to 'double'
+//CHECK-NEXT: {{^}}_Complex double fg = {1.0, f};
+//CHECK-NEXT: {{^}}                           ^{{$}}
+_Complex double fg = {1.0, f};
+
+
+//CHECK: {{.*}}: error: excess elements in scalar initializer
+//CHECK-NEXT: {{^}}_Complex double gg = {1.0, 2.0, f};
+//CHECK-NEXT: {{^}}                                ^{{$}}
+//CHECK-NEXT: {{^}}6 errors generated.
+_Complex double gg = {1.0, 2.0, f};

clang/test/Sema/complex-init-list.c

 _Complex float valid1 = { 1.0f, 2.0f }; // expected-warning {{specifying real and imaginary components is an extension}}
 
 
 // Struct for nesting tests
 struct teststruct { _Complex float x; };
 
 
 // Random other valid stuff
-_Complex int valid2 = { 1, 2 }; // expected-warning {{complex integer}} expected-warning {{specifying real and imaginary components is an extension}}
+_Complex int valid2 = { 1, 2 }; // expected-warning {{complex integer}} \
+                               //  expected-warning {{specifying real and imaginary components is an extension}}
 struct teststruct valid3 = { { 1.0f, 2.0f} }; // expected-warning {{specifying real and imaginary components is an extension}}
 _Complex float valid4[2] = { {1.0f, 1.0f}, {1.0f, 1.0f} }; // expected-warning 2 {{specifying real and imaginary components is an extension}}
 // FIXME: We need some sort of warning for valid5
-_Complex float valid5 = {1.0f, 1.0fi}; // expected-warning {{imaginary constants}} expected-warning {{specifying real and imaginary components is an extension}}
+_Complex float valid5 = {1.0f, 1.0fi}; // expected-warning {{imaginary constants}} \
+                                       // expected-warning {{specifying real and imaginary components is an extension}}
 
 
 // Random invalid stuff
 struct teststruct invalid1 = { 1, 2 }; // expected-warning {{excess elements}}
-_Complex float invalid2 = { 1, 2, 3 }; // expected-warning {{excess elements}}
-_Complex float invalid3 = {}; // expected-error {{scalar initializer cannot be empty}} expected-warning {{GNU empty initializer}}
+_Complex float invalid2 = { 1, 2, 3 }; // expected-warning {{specifying real and imaginary components is an extension}} \
+                                       // expected-warning {{excess elements in scalar initializer}}
+_Complex float invalid3 = {}; // expected-error {{scalar initializer cannot be empty}} \
+                             //  expected-warning {{GNU empty initializer}}
 
 
 // Check incomplete array sizing
 _Complex float sizetest1[] = { {1.0f, 1.0f}, {1.0f, 1.0f} }; // expected-warning 2 {{specifying real and imaginary components is an extension}}
 _Complex float sizecheck1[(sizeof(sizetest1) == sizeof(*sizetest1)*2) ? 1 : -1];
 _Complex float sizetest2[] = { 1.0f, 1.0f, {1.0f, 1.0f} };  // expected-warning {{specifying real and imaginary components is an extension}}
 _Complex float sizecheck2[(sizeof(sizetest2) == sizeof(*sizetest2)*3) ? 1 : -1];
 
 // Constant-folding with init list.
 _Complex float x = 2 + (_Complex float) { 1, 2 };  // expected-warning {{specifying real and imaginary components is an extension}}
+
+// initialization list
+_Complex double cd = {1.0, 2.0, 3.0}; // expected-warning {{specifying real and imaginary components is an extension}} \
+                                     //  expected-warning {{excess elements in scalar initializer}}
+_Complex float cf = {1.1f, 2.2f, 3.3f, 4.4f}; // expected-warning {{specifying real and imaginary components is an extension}} \
+                                             //  expected-warning {{excess elements in scalar initializer}}