Open Bug 1447484 Opened 2 years ago Updated 11 months ago

Add infrastructure to do type renaming easily.

Categories

(Firefox Build System :: Source Code Analysis, enhancement)

Product:
Firefox Build System
Component:
Source Code Analysis
Version:
3 Branch
Type:
enhancement
Priority:
Not set

Tracking

(Not tracked)

People

(Reporter: emilio, Assigned: emilio)

References

Details

I think we have none.
FWIW I tried messing around with `clang-rename` a little bit:

https://clang.llvm.org/extra/clang-rename.html

Specifically, I created a compile_commands.json file in my objdir:

./mach build-backend --backend=CompileDB

and then ran:

/usr/local/Cellar/llvm/6.0.0/bin/clang-rename -p=$OBJDIR -qualified-name=nsStyleContext -new-name=ComputedStyle layout/style/nsStyleContext.cpp > layout/style/nsStyleContext.cpp.new

diff -u layout/style/nsStyleContext.cpp layout/style/nsStyleContext.cpp.new

Unfortunately that didn't seem to take account of the compile_commands.json file since instances of nsStyleContext inside DEBUG blocks were unchanged. Possibly I'm doing something wrong but the docs don't seem very clear to me.

It also has the issue that indentation of method parameters wasn't fixed up. I guess if all our source was formatted to a fixed clang-format format then we could run that as a post-process step...
(Oh, and the reason I redirected to a new file was because the -i option doesn't seem to work. At least not as I expected it to work, which is to say I expected it to change the specified source file rather than printing the file's contents with the rename to standard out.)
(In reply to Jonathan Watt [:jwatt] from comment #1)
> Unfortunately that didn't seem to take account of the compile_commands.json

Ah, sorry, what it failed to take account of was this code:

#ifdef DEBUG
  static_assert(MOZ_ARRAY_LENGTH(nsStyleContext::sDependencyTable)
                  == nsStyleStructID_Length,
                "Number of items in dependency table doesn't match IDs");
#endif

I mistook that for not having DEBUG set, but actually it does fix up other DEBUG blocks. So I guess it's not recognizing and updating references that are resolved statically.

Maybe that's rare enough for most rewrites that it wouldn't be a problem (or the remaining instances could be fixed up by hand, which I guess is something we'll need to do whatever tool we use in order to fix up references in comments, etc.)?
Backout by ebalazs@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/980dab801b61
Backed out changeset 3e4836ce7109 for landing the patch with the wrong bug number CLOSED TREE
Severity: normal → enhancement
Version: Version 3 → 3 Branch
You need to log in before you can comment on or make changes to this bug.