refactor: elimate string manipulation in TacticAnalysis.verifyTryThisSuggestions#37917
refactor: elimate string manipulation in TacticAnalysis.verifyTryThisSuggestions#37917chenson2018 wants to merge 4 commits intoleanprover-community:masterfrom
TacticAnalysis.verifyTryThisSuggestions#37917Conversation
PR summary 9b51fe6ec0Import changes for modified filesNo significant changes to the import graph Import changes for all files
Declarations diff
You can run this locally as follows## summary with just the declaration names:
./scripts/pr_summary/declarations_diff.sh <optional_commit>
## more verbose report:
./scripts/pr_summary/declarations_diff.sh long <optional_commit>The doc-module for No changes to technical debt.You can run this locally as
|
|
I couldn't find any tests about Concretely I do see tests about |
| let suggPP ← try | ||
| liftCoreM <| PrettyPrinter.ppTactic suggestedTac | ||
| catch _ => pure s!"{suggestedTac}" | ||
| let suggStr := suggPP.pretty |
There was a problem hiding this comment.
I don't yet understand why removing the pretty-printing is desired? But maybe an added test makes this obvious
There was a problem hiding this comment.
I feel like this is inherently hacky, I believe added by Claude when first written. It's just using the pretty-printed tactic to check for hash codes and approx in grind =>. Besides being less maintainable and clear in my opinion, I think it will skip any usage of grind that uses a declaration with the substring "approx".
I have added a test of a |
|
-awaiting-author |
This change should be compatible with, but independent from, #37916. Instead of manipulating pretty-printed strings to identify when
grind?uses hashes orapprox, search for the corresponding syntax kinds.