Details
- Reviewers
george.karpenkov NoQ dkrupp whisperity
Diff Detail
Event Timeline
| test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist | ||
|---|---|---|
| 4–5 | Could we figure out how not to have hardcoded contributor-based strings there? Maybe have a python script generating those / doing a cleanup? | |
| test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist | ||
|---|---|---|
| 4–5 | I'm just about to remove it, as I did for all the other [PlistMacroExpancion] patches. But yea, a tool wouldn't hurt. | |
Looks great, let's land? Not sure if i already asked - am i understanding correctly that this is a "poor-man's" support for macro expansions for tools that read plists but do not support those new plist macro dictionaries yet? Also i wanted to have -analyzer-output=text tests in order to easily see how exactly macro expansions turn into events, but i guess it doesn't make sense because this feature is only for -analyzer-output=plist because other outputs have their own mechanisms for handling macros so nvm^^
I wonder how expansions of huge macros will look as events.
| lib/StaticAnalyzer/Core/PlistDiagnostics.cpp | ||
|---|---|---|
| 287–288 | I think the proper way of using Twine is to only wrap the first value with it. Otherwise string concatenation is done normally and then a single string is converted into a Twine, which doesn't give any benefits. | |
I also would like to see in a tool how this would look like as an event before committing :) Just a sanity check to make sure this feature makes sense. Could you post a screenshot of CodeChecker or any other tool using this feature?
I'll probably land it after part 5, in order to ease on rebasing.
Not sure if i already asked - am i understanding correctly that this is a "poor-man's" support for macro expansions for tools that read plists but do not support those new plist macro dictionaries yet?
Correct, same as notes-as-events.
I wonder how expansions of huge macros will look as events.
Quite funny, actually :D Think about assert, it's quite a mouthful. I'll post screenshots when I have more time.
Since the way how these macro events are created as a whole changed in part 1, this path will need a big overhaul.
As we currently support the actual macro_expansions format in CodeChecker, and AFAIK you guys at Apple prefer relying on Xcode (http://lists.llvm.org/pipermail/cfe-dev/2018-September/059231.html), I'd like to focus on more pressing matters.
I personally do use HTML output quite a lot (and we do have non-Xcode projects), and complex macros in HTML are currently totally unusable.
I'm not sure whether this is a right approach to handling this, but it's definitely a problem for us.
In general - thanks for working on this anyway.
I think the proper way of using Twine is to only wrap the first value with it. Otherwise string concatenation is done normally and then a single string is converted into a Twine, which doesn't give any benefits.