ankurkraj wrote: > > Can't find any unit tests for methods in PlistDiagnostics.cpp > > So adding the unit test in > > `clang/unittests/Analysis/MacroExpansionContextTest.cpp` Since it contains > > some similar MacroExpansionContextTest functionality tests. Also, since the > > newly defined function `getFormattedMacro` is static , should i just > > duplicate it in the testing file or how should i proceed ? > > Yes, I had `MacroExpansionContextTest` in mind, exactly. > > Wrt. the second part; would it make sense to move this formatting > functionality into the `MacroExpansionContext`? I'd go as far as caching > these formatted strings for the expansion locations to avoid potential > duplicated queries. Frequently, bug reports cross the same macro expansions > by nature, thus this might save some time. WDYT?
Sorry for the delay. I have made the following changes (1) I agree , I think it is a good idea to have the formatting functionality in MacroExpansionContext, so that it does not stay coupled with only the plist reports (2) Added caching for the formatted macros for the expanded locations. (3) Added tests in MacroExpansionContextTests @steakhal Can you please review it and let me know if any other changes are required ? https://github.com/llvm/llvm-project/pull/156046 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
