Page MenuHomePhabricator

[clang][NFC] Use enum for -fstrict-flex-arrays
ClosedPublic

Authored by void on Oct 3 2022, 2:40 PM.

Details

Summary

Use enums for the strict flex arrays flag so that it's more readable.

Diff Detail

Event Timeline

void created this revision.Oct 3 2022, 2:40 PM
Herald added a project: Restricted Project. · View Herald Transcript
void requested review of this revision.Oct 3 2022, 2:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 3 2022, 2:40 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Thanks for working on this! A few nits inline

clang/include/clang/Basic/LangOptions.h
369

typo: member

373

+ size

375

I think you meant Any trailing array member of undefined size is a FAM

clang/include/clang/Driver/Options.td
1155

Note that this depends on https://reviews.llvm.org/D134902 to have the "Incomplete" support.

The StaticAnalyzer changes seem to be correct.

clang/include/clang/Basic/LangOptions.h
369–376

For most cases, I can either see no initializers or only the first entry is being initialized.
I believe, by default, the entries would be initialized to 0, 1, and 2 here anyway.

clang/lib/StaticAnalyzer/Core/MemRegion.cpp
799
void updated this revision to Diff 465127.Oct 4 2022, 1:01 PM
void marked 3 inline comments as done.

Fix typos.

clang/include/clang/Basic/LangOptions.h
369–376

I wanted to be explicit with the numbering, because it corresponds with the value specified with -fstrict-flex-arrays=<n>.

void updated this revision to Diff 465129.Oct 4 2022, 1:03 PM
void marked an inline comment as done.

Use an already available context.

clang/include/clang/Driver/Options.td
1155

Would it be better to remove Incomplete until that is approved?

clang/lib/StaticAnalyzer/Core/MemRegion.cpp
799

Doh! Thanks.

void updated this revision to Diff 465526.Oct 5 2022, 1:20 PM

Rebase with ToT.

kees accepted this revision.EditedOct 5 2022, 8:16 PM

LGTM

This revision is now accepted and ready to land.Oct 5 2022, 8:16 PM

Thanks for the cleanup!

This revision was landed with ongoing or failed builds.Oct 6 2022, 10:46 AM
This revision was automatically updated to reflect the committed changes.

hi, seeing our amdgpu buildbot broken with this patch. please look into ?
let me know if you any help on our end.
https://lab.llvm.org/buildbot/#/builders/193/builds/19744

void added a comment.Oct 6 2022, 11:13 AM

hi, seeing our amdgpu buildbot broken with this patch. please look into ?
let me know if you any help on our end.
https://lab.llvm.org/buildbot/#/builders/193/builds/19744

It's been fixed. Sorry about that!