Page MenuHomePhabricator

[clang] Fix duplicate warning
Needs ReviewPublic

Authored by kamleshbhalui on Jun 24 2020, 3:24 AM.

Details

Summary

Fixes duplicate warning emitted by clang when
char16_t/char32_t is used in the catch block.

Fixes this
https://bugs.llvm.org/show_bug.cgi?id=46425

Diff Detail

Event Timeline

kamleshbhalui created this revision.Jun 24 2020, 3:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 24 2020, 3:24 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
aaron.ballman added inline comments.Jun 24 2020, 4:59 AM
clang/lib/Parse/ParseExprCXX.cpp
2273

This change is incorrect -- Finish is what ensures the DeclSpec is in a consistent state (even in the presence of errors), so removing it would be bad.

kamleshbhalui marked an inline comment as done.Jun 24 2020, 6:47 AM
kamleshbhalui added inline comments.
clang/lib/Parse/ParseExprCXX.cpp
2273

you are right but ParseSpecifierQualifierList ->(calls) ParseDeclarationSpecifiers
and it makes a call to Finish(https://github.com/llvm/llvm-project/blob/master/clang/lib/Parse/ParseDecl.cpp#L2962).

I apologise for a quick drive-by comment, but this is something I find very annoying when looking at the history of a piece of code:

Could you spent a few minutes to make a more descriptive title and description?

Fixes duplicate warning emitted by clang when
char16_t/char32_t is used in the catch block.

Does not actually explains what is wrong (the duplicate warning is just a symptom), nor does it explains what is done to fix it, nor does it explains *why* this is the right fix.
Unless the patch is really trivial I think that it pays to put a little more context in the description.

aaron.ballman added inline comments.Jun 24 2020, 7:54 AM
clang/lib/Parse/ParseExprCXX.cpp
2273

ParseSpecifierQualifierList then makes further modifications to the DeclSpec, such as setting it to have an error, clearing specific bits from it, etc. Do none of those further modifications require finalization?