Skip to content

[clang-tidy] Add null-checking in Use designated Initializer check - #220093

Open
GTaf wants to merge 1 commit into
llvm:mainfrom
GTaf:designated_initializer_check_null_case
Open

[clang-tidy] Add null-checking in Use designated Initializer check#220093
GTaf wants to merge 1 commit into
llvm:mainfrom
GTaf:designated_initializer_check_null_case

Conversation

@GTaf

@GTaf GTaf commented Aug 31, 2026

Copy link
Copy Markdown

Part of #219739

This null check should avoid null dereferencing crash in the Use designated Initializer check.

AI has been used in this contribution to understand the code as I'm a beginner to the codebase. The problem understanding and proposed fix is mine.

@GTaf
GTaf requested review from vbvictor and zeyi2 as code owners August 31, 2026 21:31
@github-actions

Copy link
Copy Markdown

Hello @GTaf 👋

Thank you for submitting a Pull Request (PR) to the LLVM Project. Since this is your first PR, here are a few useful links covering our main contribution policies and review practices.

  • All contributions to LLVM must follow our LLVM AI Tool Use Policy. In particular, if you used AI while working on this PR, remember to add a note to the PR description.
  • The LLVM Code-Review Policy and Practices document contains practical information about the PR process, including how patches are reviewed and accepted, and who can review a PR.
  • Our LLVM Developer Policy describes our expectations for code quality, commit summaries and contains notes on our CI system.

Please reply to this message to confirm that you have read these policies, especially the LLVM AI Tool Use Policy, and that any AI tool usage has been noted in the PR description.


Frequently asked questions

How do I add reviewers?

This PR will be automatically labeled, and the relevant teams will be notified. For some parts of the project, reviewers may also be added automatically.

You can also add reviewers manually using the Reviewers section on this page. If you cannot use that section, it is probably because you do not have write permissions for the repository. In that case, you can request a review by tagging reviewers in a comment using @ followed by their GitHub username.

What if there are no comments?

If you have not received any comments on your PR after a week, you can request a review by pinging the PR with a comment such as “Ping”. The common courtesy ping rate is once a week. Please remember that you are asking for volunteer time from other developers.

Are any special GitHub settings required to contribute to LLVM?

We only require contributors to have a public email address associated with their GitHub commits, see this section of LLVM Developer Policy for details.


If you have questions, feel free to leave a comment on this PR, or ask on LLVM Discord or LLVM Discourse.

Thank you,
The LLVM Community

@llvmorg-github-actions

llvmorg-github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: GTaf

Changes

Trying to fix #219739


Full diff: https://github.com/llvm/llvm-project/pull/220093.diff

1 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp (+2-1)
diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp
index 5874c061d299f..696e6f40d66c8 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp
@@ -17,6 +17,7 @@
 #include "clang/ASTMatchers/ASTMatchersMacros.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/Support/Casting.h"
 
 using namespace clang::ast_matchers;
 
@@ -42,7 +43,7 @@ static constexpr bool StrictCppStandardComplianceDefault = true;
 
 static unsigned getNumberOfDesignated(const InitListExpr *SyntacticInitList) {
   return llvm::count_if(*SyntacticInitList, [](auto *InitExpr) {
-    return isa<DesignatedInitExpr>(InitExpr);
+    return llvm::isa_and_nonnull<DesignatedInitExpr>(InitExpr);
   });
 }
 

@EugeneZelenko

Copy link
Copy Markdown
Contributor

Please mention changes in Release Notes.

@vbvictor

vbvictor commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Please mention changes in Release Notes.

And tests

@zeyi2 zeyi2 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code part change LGTM, one nit comment below

#include "clang/ASTMatchers/ASTMatchersMacros.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Lex/Lexer.h"
#include "llvm/Support/Casting.h"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zeyi2

zeyi2 commented Sep 1, 2026

Copy link
Copy Markdown
Member

Also I removed "Fixes #219739", since this PR only addresses the modernize-use-designated-initializers crash. The other crash in readability-trailing-comma probably needs a separate PR.

@GTaf

GTaf commented Sep 1, 2026

Copy link
Copy Markdown
Author

Thanks for your feedback. I'm working on your assigned tasks.

Making a relevant test is a bit challenging to me has the issue is to say that an invalid code that use to crash the binary now just show the correct error. I'm looking into it.

@GTaf
GTaf marked this pull request as draft September 1, 2026 22:04
@GTaf
GTaf force-pushed the designated_initializer_check_null_case branch 2 times, most recently from 774a0ae to 0d4e2c2 Compare September 1, 2026 22:07
@zeyi2

zeyi2 commented Sep 2, 2026

Copy link
Copy Markdown
Member

Making a relevant test is a bit challenging to me has the issue is to say that an invalid code that use to crash the binary now just show the correct error. I'm looking into it.

I think you can use invalid code with -expect-clang-tidy-error in the // RUN line.

See ./clang-tools-extra/test/clang-tidy/checkers/readability/isolate-declaration-no-infinite-loop.cpp as a reference :)

// RUN: %check_clang_tidy -expect-clang-tidy-error %s readability-isolate-declaration %t

int main(){
  int a, b
  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
  // CHECK-MESSAGES: [[@LINE-2]]:11: error: expected ';' at end of declaration [clang-diagnostic-error]
}

@GTaf
GTaf force-pushed the designated_initializer_check_null_case branch from 0d4e2c2 to ee40584 Compare September 2, 2026 17:59
Comment thread clang-tools-extra/docs/ReleaseNotes.md Outdated

#### Changes in existing checks

- Fixed a crash in {doc}`modernize-use-designated-initializers

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep alphabetical order (by check name) in this list.

Comment thread clang-tools-extra/docs/ReleaseNotes.md Outdated

- Fixed a crash in {doc}`modernize-use-designated-initializers
<clang-tidy/checks/modernize/modernize-use-designated-initializers>` when receiving
a null `Expr`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is implementation detail. Should be something user-facing.

@GTaf
GTaf force-pushed the designated_initializer_check_null_case branch from ee40584 to 3b7de77 Compare September 2, 2026 21:33
<clang-tidy/checks/modernize/use-noexcept>` when analyzing malformed template
code with an unparsed exception specification.

- Fixed a crash in {doc}`modernize-use-designated-initializers

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be before modernize-use-noexcept.

@GTaf
GTaf force-pushed the designated_initializer_check_null_case branch 2 times, most recently from 1b1f7a9 to bc58258 Compare September 2, 2026 21:40
@GTaf
GTaf marked this pull request as ready for review September 2, 2026 21:41
@GTaf

GTaf commented Sep 2, 2026

Copy link
Copy Markdown
Author

I believe I did all you pinpointed, thank you very much for your guidance and patience.
All tests runned fine from both check-llvm and check-llvm-unit on my computer.

Please let me know anything that still needs to be fixed

@GTaf
GTaf force-pushed the designated_initializer_check_null_case branch from 35ee957 to 30f95ee Compare September 2, 2026 22:09
@GTaf
GTaf force-pushed the designated_initializer_check_null_case branch from 30f95ee to 34a124a Compare September 2, 2026 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants