In D147989#4299722, @aaron.ballman wrote:

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

In D147989#4295928, @aaron.ballman wrote:

The issue is that GetDiagnosticTypeSpecifierID() is called for more diagnostics than just warn_declspec_attribute_ignored. You need to also update the err_constexpr_tag and err_standalone_class_nested_name_specifier to add enum class and enum struct as well.

Fixed the code

In D147989#4293278, @aaron.ballman wrote:

Oops, I spoke too soon -- it looks like the precommit CI failure is related to this patch. This is what I get when I tested locally:

Assertion failed: NextVal != ArgumentEnd && "Value for integer select modifier was" " larger than the number of options in the diagnostic string!", file F:\source\llvm-project\clang\lib\Basic\Diagnostic.cpp, line 623
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.      Program arguments: f:\\source\\llvm-project\\llvm\\out\\build\\x64-debug\\bin\\clang.exe -cc1 -internal-isystem f:\\source\\llvm-project\\llvm\\out\\build\\x64-debug\\lib\\clang\\17\\include -nostdsysteminc -std=c++11 -triple x86_64-unknown-unknown F:\\source\\llvm-project\\clang\\test\\CXX\\drs\\dr16xx.cpp -verify -fexceptions -fcxx-exceptions -pedantic-errors
1.      F:\source\llvm-project\clang\test\CXX\drs\dr16xx.cpp:91:3: current parser token 'template'
2.      F:\source\llvm-project\clang\test\CXX\drs\dr16xx.cpp:71:1: parsing namespace 'dr1638'
Exception Code: 0x80000003
 #0 0x00007ff6625ea26c HandleAbort F:\source\llvm-project\llvm\lib\Support\Windows\Signals.inc:419:0
 #1 0x00007ffa9b23bc31 (C:\WINDOWS\SYSTEM32\ucrtbased.dll+0x6bc31)
 #2 0x00007ffa9b23d889 (C:\WINDOWS\SYSTEM32\ucrtbased.dll+0x6d889)
 #3 0x00007ffa9b2430bf (C:\WINDOWS\SYSTEM32\ucrtbased.dll+0x730bf)
 #4 0x00007ffa9b241091 (C:\WINDOWS\SYSTEM32\ucrtbased.dll+0x71091)
 #5 0x00007ffa9b243a1f (C:\WINDOWS\SYSTEM32\ucrtbased.dll+0x73a1f)
 #6 0x00007ff662a19b8c HandleSelectModifier F:\source\llvm-project\clang\lib\Basic\Diagnostic.cpp:622:0
 #7 0x00007ff662a17137 clang::Diagnostic::FormatDiagnostic(char const *, char const *, class llvm::SmallVectorImpl<char> &) const F:\source\llvm-project\clang\lib\Basic\Diagnostic.cpp:1005:0
 #8 0x00007ff662a16518 clang::Diagnostic::FormatDiagnostic(class llvm::SmallVectorImpl<char> &) const F:\source\llvm-project\clang\lib\Basic\Diagnostic.cpp:805:0
 #9 0x00007ff663e5bcc9 clang::TextDiagnosticBuffer::HandleDiagnostic(enum clang::DiagnosticsEngine::Level, class clang::Diagnostic const &) F:\source\llvm-project\clang\lib\Frontend\TextDiagnosticBuffer.cpp:30:0
#10 0x00007ff663ef2d71 clang::VerifyDiagnosticConsumer::HandleDiagnostic(enum clang::DiagnosticsEngine::Level, class clang::Diagnostic const &) F:\source\llvm-project\clang\lib\Frontend\VerifyDiagnosticConsumer.cpp:757:0
#11 0x00007ff662a0b66f clang::DiagnosticIDs::EmitDiag(class clang::DiagnosticsEngine &, enum clang::DiagnosticIDs::Level) const F:\source\llvm-project\clang\lib\Basic\DiagnosticIDs.cpp:839:0
#12 0x00007ff662a0b597 clang::DiagnosticIDs::ProcessDiag(class clang::DiagnosticsEngine &) const F:\source\llvm-project\clang\lib\Basic\DiagnosticIDs.cpp:831:0
#13 0x00007ff662a267ef clang::DiagnosticsEngine::ProcessDiag(void) F:\source\llvm-project\clang\include\clang\Basic\Diagnostic.h:1038:0
#14 0x00007ff662a15f64 clang::DiagnosticsEngine::EmitCurrentDiagnostic(bool) F:\source\llvm-project\clang\lib\Basic\Diagnostic.cpp:549:0
#15 0x00007ff667b47cbc clang::Sema::EmitCurrentDiagnostic(unsigned int) F:\source\llvm-project\clang\lib\Sema\Sema.cpp:1575:0
#16 0x00007ff667b89d97 clang::Sema::ImmediateDiagBuilder::~ImmediateDiagBuilder(void) F:\source\llvm-project\clang\include\clang\Sema\Sema.h:1726:0
#17 0x00007ff667b8e9e8 clang::Sema::ImmediateDiagBuilder::`scalar deleting dtor'(unsigned int) (f:\source\llvm-project\llvm\out\build\x64-debug\bin\clang.exe+0x81fe9e8)
#18 0x00007ff667b72b06 std::_Destroy_in_place<class clang::Sema::ImmediateDiagBuilder>(class clang::Sema::ImmediateDiagBuilder &) C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.35.32215\include\xmemory:300:0
#19 0x00007ff667bac5b4 std::_Optional_destruct_base<class clang::Sema::ImmediateDiagBuilder, 0>::reset(void) C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.35.32215\include\optional:133:0
#20 0x00007ff667b53424 clang::Sema::SemaDiagnosticBuilder::~SemaDiagnosticBuilder(void) F:\source\llvm-project\clang\lib\Sema\Sema.cpp:1859:0
#21 0x00007ff668618b4a clang::Sema::ParsedFreeStandingDeclSpec(class clang::Scope *, enum clang::AccessSpecifier, class clang::DeclSpec &, class clang::ParsedAttributesView const &, class llvm::MutableArrayRef<class clang::TemplateParameterList *>, bool, class clang::RecordDecl *&) F:\source\llvm-project\clang\lib\Sema\SemaDecl.cpp:5151:0
#22 0x00007ff668618545 clang::Sema::ParsedFreeStandingDeclSpec(class clang::Scope *, enum clang::AccessSpecifier, class clang::DeclSpec &, class clang::ParsedAttributesView const &, class clang::RecordDecl *&) F:\source\llvm-project\clang\lib\Sema\SemaDecl.cpp:4844:0
#23 0x00007ff66797e8e2 clang::Parser::ParseDeclOrFunctionDefInternal(class clang::ParsedAttributes &, class clang::ParsedAttributes &, class clang::ParsingDeclSpec &, enum clang::AccessSpecifier) F:\source\llvm-project\clang\lib\Parse\Parser.cpp:1160:0
#24 0x00007ff66797e532 clang::Parser::ParseDeclarationOrFunctionDefinition(class clang::ParsedAttributes &, class clang::ParsedAttributes &, class clang::ParsingDeclSpec *, enum clang::AccessSpecifier) F:\source\llvm-project\clang\lib\Parse\Parser.cpp:1228:0
#25 0x00007ff66797de7d clang::Parser::ParseExternalDeclaration(class clang::ParsedAttributes &, class clang::ParsedAttributes &, class clang::ParsingDeclSpec *) F:\source\llvm-project\clang\lib\Parse\Parser.cpp:1043:0
#26 0x00007ff667ac8745 clang::Parser::ParseInnerNamespace(class llvm::SmallVector<struct clang::Parser::InnerNamespaceInfo, 4> const &, unsigned int, class clang::SourceLocation &, class clang::ParsedAttributes &, class clang::BalancedDelimiterTracker &) F:\source\llvm-project\clang\lib\Parse\ParseDeclCXX.cpp:262:0
#27 0x00007ff667ac84b5 clang::Parser::ParseNamespace(enum clang::DeclaratorContext, class clang::SourceLocation &, class clang::SourceLocation) F:\source\llvm-project\clang\lib\Parse\ParseDeclCXX.cpp:241:0
#28 0x00007ff6679ce5af clang::Parser::ParseDeclaration(enum clang::DeclaratorContext, class clang::SourceLocation &, class clang::ParsedAttributes &, class clang::ParsedAttributes &, class clang::SourceLocation *) F:\source\llvm-project\clang\lib\Parse\ParseDecl.cpp:1846:0
#29 0x00007ff66797d768 clang::Parser::ParseExternalDeclaration(class clang::ParsedAttributes &, class clang::ParsedAttributes &, class clang::ParsingDeclSpec *) F:\source\llvm-project\clang\lib\Parse\Parser.cpp:959:0
#30 0x00007ff667977257 clang::Parser::ParseTopLevelDecl(class clang::OpaquePtr<class clang::DeclGroupRef> &, enum clang::Sema::ModuleImportState &) F:\source\llvm-project\clang\lib\Parse\Parser.cpp:745:0
#31 0x00007ff66797277d clang::ParseAST(class clang::Sema &, bool, bool) F:\source\llvm-project\clang\lib\Parse\ParseAST.cpp:162:0
#32 0x00007ff663ea922a clang::ASTFrontendAction::ExecuteAction(void) F:\source\llvm-project\clang\lib\Frontend\FrontendAction.cpp:1168:0
#33 0x00007ff663ea8b3e clang::FrontendAction::Execute(void) F:\source\llvm-project\clang\lib\Frontend\FrontendAction.cpp:1060:0
#34 0x00007ff663e207c8 clang::CompilerInstance::ExecuteAction(class clang::FrontendAction &) F:\source\llvm-project\clang\lib\Frontend\CompilerInstance.cpp:1049:0
#35 0x00007ff6640ebe80 clang::ExecuteCompilerInvocation(class clang::CompilerInstance *) F:\source\llvm-project\clang\lib\FrontendTool\ExecuteCompilerInvocation.cpp:264:0
#36 0x00007ff65fdaa7e4 cc1_main(class llvm::ArrayRef<char const *>, char const *, void *) F:\source\llvm-project\clang\tools\driver\cc1_main.cpp:251:0
#37 0x00007ff65fd8becb ExecuteCC1Tool F:\source\llvm-project\clang\tools\driver\driver.cpp:366:0
#38 0x00007ff65fd8c719 clang_main(int, char **, struct llvm::ToolContext const &) F:\source\llvm-project\clang\tools\driver\driver.cpp:407:0
#39 0x00007ff65fdd8bf6 main F:\source\llvm-project\llvm\out\build\x64-Debug\tools\clang\tools\driver\clang-driver.cpp:16:0
#40 0x00007ff66b6cd929 invoke_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:79:0
#41 0x00007ff66b6cd7ce __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288:0
#42 0x00007ff66b6cd68e __scrt_common_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:331:0
#43 0x00007ff66b6cd9be mainCRTStartup D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_main.cpp:17:0
#44 0x00007ffb71d27604 (C:\WINDOWS\System32\KERNEL32.DLL+0x17604)
#45 0x00007ffb721026a1 (C:\WINDOWS\SYSTEM32\ntdll.dll+0x526a1)

error: command failed with exit status: 0x80000003

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
********************
Failed Tests (1):
  Clang :: CXX/drs/dr16xx.cpp

Removed unwanted changes

Added case for enum struct

In D147989#4283666, @samtebbs wrote:

This looks good to me now, nice work. Let's wait a few days for others' input to be safe.

Updated the logic in a better way

Updated the logic

Removed spurious whitespace changes

Updated the test file

In D147989#4257721, @aaron.ballman wrote:

Thank you for working on this!

The changes are missing test coverage; please be sure to add that, along with a release note about the fix. I think there's likely more work to be done here as well, considering this behavior: https://godbolt.org/z/sdK3Gef75 (notice the suggested location for the fix-it -- that also needs to be fixed for this)

Improved the logic

Applied C++ Overloading

In D147989#4257947, @samtebbs wrote:

Thanks for the input Aaron and Martin. This is looking good. With the suggested changes and some formatting I think it will be ready.

Added the Release note for the fix

In D147989#4257721, @aaron.ballman wrote:

Thank you for working on this!

The changes are missing test coverage; please be sure to add that, along with a release note about the fix. I think there's likely more work to be done here as well, considering this behavior: https://godbolt.org/z/sdK3Gef75 (notice the suggested location for the fix-it -- that also needs to be fixed for this)

Removed the spurious whitespace changes

Implemented the required changes

Updated the summary

Updated the title

Removed extra lines

1. Updating D147989: Fix Attribute Placememt #
2. Enter a brief description of the changes included in this update.

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

Yes, I am OK with that.

Thank you for accepting my revision. Please do commit it as I am
contributing for the first time and I don't have commit access.

Fixed one more Typo

Hi @AryanGodara , I am an Outreachy applicant. I was also working on this issue and also created a patch for this issue i.e., https://reviews.llvm.org/D146530 . Can we collaborate to solve this issue so that only one patch will be there for a issue as told by @aaron.ballman .

In D146530#4213078, @aaron.ballman wrote:

In D146530#4213076, @ipriyanshi1708 wrote:

In D146530#4213045, @aaron.ballman wrote:

This appears to be the same efforts that have been going on in https://reviews.llvm.org/D146041. Can you coordinate with @AryanGodara so that there's only one patch for this?

Hi! I am an Outreachy applicant, firstly how can I coordinate with @AryanGodara and secondly how will it be counted as my contribution and then can I record it in my final application? Or I will have to look for some other issues for Outreachy?

Welcome! You can comment on the other review to see if the author would like to collaborate with you on a solution. We have the ability to land commits with co-author tags (please request it explicitly when someone approves the patch so that the person landing the work knows to do this), so that should get you both the attribution in GitHub that you did the work (no idea if Outreachy has rules around co-authoring though). You might have to look for other issues for Outreachy, depending on what they require.

In D146530#4213045, @aaron.ballman wrote:

This appears to be the same efforts that have been going on in https://reviews.llvm.org/D146041. Can you coordinate with @AryanGodara so that there's only one patch for this?