This patch mechanically replaces None with std::nullopt where the
compiler would warn if None were deprecated. The intent is to reduce
the amount of manual work required in migrating from Optional to
std::optional.
This is part of an effort to migrate from llvm::Optional to
std::optional:
https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716
Checking if a path is absolute can be expensive and currently the result is not cached in the FileSpec object. This patch adds caching and also code to clear the cache if the file is modified.
Differential Revision: https://reviews.llvm.org/D130396
Resubmission of https://reviews.llvm.org/D130309 with the 2 patches that fixed the linux buildbot, and new windows fixes.
The FileSpec APIs allow users to modify instance variables directly by getting a non const reference to the directory and filename instance variables. This makes it impossible to control all of the times the FileSpec object is modified so we can clear cached member variables like m_resolved and with an upcoming patch caching if the file is relative or absolute. This patch modifies the APIs of FileSpec so no one can modify the directory or filename instance variables directly by adding set accessors and by removing the get accessors that are non const.
Many clients were using FileSpec::GetCString(...) which returned a unique C string from a ConstString'ified version of the result of GetPath() which returned a std::string. This caused many locations to use this convenient function incorrectly and could cause many strings to be added to the constant string pool that didn't need to. Most clients were converted to using FileSpec::GetPath().c_str() when possible. Other clients were modified to use the newly renamed version of this function which returns an actualy ConstString:
ConstString FileSpec::GetPathAsConstString(bool denormalize = true) const;
This avoids the issue where people were getting an already uniqued "const char *" that came from a ConstString only to put the "const char *" back into a "ConstString" object. By returning the ConstString instead of a "const char *" clients can be more efficient with the result.
The patch:
- Removes the non const GetDirectory() and GetFilename() get accessors
- Adds set accessors to replace the above functions: SetDirectory() and SetFilename().
- Adds ClearDirectory() and ClearFilename() to replace usage of the FileSpec::GetDirectory().Clear()/FileSpec::GetFilename().Clear() call sites
- Fixed all incorrect usage of FileSpec::GetCString() to use FileSpec::GetPath().c_str() where appropriate, and updated other call sites that wanted a ConstString to use the newly returned ConstString appropriately and efficiently.
Differential Revision: https://reviews.llvm.org/D130549
Checking if a path is absolute can be expensive and currently the result is not cached in the FileSpec object. This patch adds caching and also code to clear the cache if the file is modified.
Differential Revision: https://reviews.llvm.org/D130396
The FileSpect APIs allow users to modify instance variables directly by getting a non const reference to the directory and filename instance variables. This makes it impossibly to control all of the times the FileSpec object is modified so we can clear the cache. This patch modifies the APIs of FileSpec so no one can modify the directory or filename directly by adding set accessors and by removing the get accessors that are non const.
Many clients were using FileSpec::GetCString(...) which returned a unique C string from a ConstString'ified version of the result of GetPath() which returned a std::string. This caused many locations to use this convenient function incorrectly and could cause many strings to be added to the constant string pool that didn't need to. Most clients were converted to using FileSpec::GetPath().c_str() when possible. Other clients were modified to use the newly renamed version of this function which returns an actualy ConstString:
ConstString FileSpec::GetPathAsConstString(bool denormalize = true) const;
This avoids the issue where people were getting an already uniqued "const char *" that came from a ConstString only to put the "const char *" back into a "ConstString" object. By returning the ConstString instead of a "const char *" clients can be more efficient with the result.
The patch:
- Removes the non const GetDirectory() and GetFilename() get accessors
- Adds set accessors to replace the above functions: SetDirectory() and SetFilename().
- Adds ClearDirectory() and ClearFilename() to replace usage of the FileSpec::GetDirectory().Clear()/FileSpec::GetFilename().Clear() call sites
- Fixed all incorrect usage of FileSpec::GetCString() to use FileSpec::GetPath().c_str() where appropriate, and updated other call sites that wanted a ConstString to use the newly returned ConstString appropriately and efficiently.
Differential Revision: https://reviews.llvm.org/D130309
Since a8b54834a1, there are two
distinct Windows path styles, `windows_backslash` (with the old
`windows` being an alias for it) and `windows_slash`.
4e4883e1f3 added helpers for
inspecting path styles.
The newly added windows_slash path style doesn't end up used in
LLDB yet anyway, as LLDB is quite decoupled from most of
llvm::sys::path and uses its own FileSpec class. To take it in
use, it could be hooked up in `FileSpec::Style::GetNativeStyle`
(in lldb/source/Utility/FileSpec.cpp) just like in the `real_style`
function in llvm/lib/Support/Path.cpp in
df0ba47c36.
It is not currently clear whether there's a real need for using
the Windows path style with forward slashes in LLDB (if there's any
other applications interacting with it, expecting that style), and
what other changes in LLDB are needed for that to work, but this
at least makes some of the checks more ready for the new style,
simplifying code a bit.
Differential Revision: https://reviews.llvm.org/D113255
The C headers are deprecated so as requested in D102845, this is replacing them
all with their (not deprecated) C++ equivalent.
Reviewed By: shafik
Differential Revision: https://reviews.llvm.org/D103084
Add YAML traits for the ConstString and FileSpec classes so they can be
serialized as part of ProcessInfo. The latter needs to be serializable
for the reproducers.
Differential revision: https://reviews.llvm.org/D76002
Summary:
A *.cpp file header in LLDB (and in LLDB) should like this:
```
//===-- TestUtilities.cpp -------------------------------------------------===//
```
However in LLDB most of our source files have arbitrary changes to this format and
these changes are spreading through LLDB as folks usually just use the existing
source files as templates for their new files (most notably the unnecessary
editor language indicator `-*- C++ -*-` is spreading and in every review
someone is pointing out that this is wrong, resulting in people pointing out that this
is done in the same way in other files).
This patch removes most of these inconsistencies including the editor language indicators,
all the different missing/additional '-' characters, files that center the file name, missing
trailing `===//` (mostly caused by clang-format breaking the line).
Reviewers: aprantl, espindola, jfb, shafik, JDevlieghere
Reviewed By: JDevlieghere
Subscribers: dexonsmith, wuzish, emaste, sdardis, nemanjai, kbarton, MaskRay, atanasyan, arphaman, jfb, abidh, jsji, JDevlieghere, usaxena95, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D73258
Summary:
The FileSpec class is often used as a sort of a pattern -- one specifies
a bare file name to search, and we check if in matches the full file
name of an existing module (for example).
These comparisons used FileSpec::Equal, which had some support for it
(via the full=false argument), but it was not a good fit for this job.
For one, it did a symmetric comparison, which makes sense for a function
called "equal", but not for typical searches (when searching for
"/foo/bar.so", we don't want to find a module whose name is just
"bar.so"). This resulted in patterns like:
if (FileSpec::Equal(pattern, file, pattern.GetDirectory()))
which would request a "full" match only if the pattern really contained
a directory. This worked, but the intended behavior was very unobvious.
On top of that, a lot of the code wanted to handle the case of an
"empty" pattern, and treat it as matching everything. This resulted in
conditions like:
if (pattern && !FileSpec::Equal(pattern, file, pattern.GetDirectory())
which are nearly impossible to decipher.
This patch introduces a FileSpec::Match function, which does exactly
what most of FileSpec::Equal callers want, an asymmetric match between a
"pattern" FileSpec and a an actual FileSpec. Empty paterns match
everything, filename-only patterns match only the filename component.
I've tried to update all callers of FileSpec::Equal to use a simpler
interface. Those that hardcoded full=true have been changed to use
operator==. Those passing full=pattern.GetDirectory() have been changed
to use FileSpec::Match.
There was also a handful of places which hardcoded full=false. I've
changed these to use FileSpec::Match too. This is a slight change in
semantics, but it does not look like that was ever intended, and it was
more likely a result of a misunderstanding of the "proper" way to use
FileSpec::Equal.
[In an ideal world a "FileSpec" and a "FileSpec pattern" would be two
different types, but given how widespread FileSpec is, it is unlikely
we'll get there in one go. This at least provides a good starting point
by centralizing all matching behavior.]
Reviewers: teemperor, JDevlieghere, jdoerfert
Subscribers: emaste, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D70851
gcc-9 started warning when a class defined a copy constructor without a
copy assignment operator (or vice-versa).
This fixes those warnings by deleting the other special member too
(after verifying it doesn't do anything non-trivial).
A lot of comments in LLDB are surrounded by an ASCII line to delimit the
begging and end of the comment.
Its use is not really consistent across the code base, sometimes the
lines are longer, sometimes they are shorter and sometimes they are
omitted. Furthermore, it looks kind of weird with the 80 column limit,
where the comment actually extends past the line, but not by much.
Furthermore, when /// is used for Doxygen comments, it looks
particularly odd. And when // is used, it incorrectly gives the
impression that it's actually a Doxygen comment.
I assume these lines were added to improve distinguishing between
comments and code. However, given that todays editors and IDEs do a
great job at highlighting comments, I think it's worth to drop this for
the sake of consistency. The alternative is fixing all the
inconsistencies, which would create a lot more churn.
Differential revision: https://reviews.llvm.org/D60508
llvm-svn: 358135
My apologies for the large patch. With the exception of ConstString.h
itself it was entirely produced by sed.
ConstString has exactly one const char * data member, so passing a
ConstString by reference is not any more efficient than copying it by
value. In both cases a single pointer is passed. But passing it by
value makes it harder to accidentally return the address of a local
object.
(This fixes rdar://problem/48640859 for the Apple folks)
Differential Revision: https://reviews.llvm.org/D59030
llvm-svn: 355553
Summary:
This adds support for auto-detection of path style to SymbolFileBreakpad
(similar to how r351328 did the same for DWARF). We guess each file
entry separately, as we have no idea which file came from which compile
units (and different compile units can have different path styles). The
breakpad generates should have already converted the paths to absolute
ones, so this guess should be reasonable accurate, but as always with
these kinds of things, it is hard to give guarantees about anything.
In an attempt to bring some unity to the path guessing logic, I move the
guessing logic from inside SymbolFileDWARF into the FileSpec class and
have both symbol files use it to implent their desired behavior.
Reviewers: clayborg, lemo, JDevlieghere
Subscribers: aprantl, markmentovai, lldb-commits
Differential Revision: https://reviews.llvm.org/D57895
llvm-svn: 353702
to reflect the new license.
We understand that people may be surprised that we're moving the header
entirely to discuss the new license. We checked this carefully with the
Foundation's lawyer and we believe this is the correct approach.
Essentially, all code in the project is now made available by the LLVM
project under our new license, so you will see that the license headers
include that license only. Some of our contributors have contributed
code under our old license, and accordingly, we have retained a copy of
our old license notice in the top-level files in each project and
repository.
llvm-svn: 351636
Summary:
If we opened a file which was produced on system with different path
syntax, we would parse the paths from the debug info incorrectly.
The reason for that is that we would parse the paths as they were
native. For example this meant that on linux we would treat the entire
windows path as a single file name with no directory component, and then
we would concatenate that with the single directory component from the
DW_AT_comp_dir attribute. When parsing posix paths on windows, we would
at least get the directory separators right, but we still would treat
the posix paths as relative, and concatenate them where we shouldn't.
This patch attempts to remedy this by guessing the path syntax used in
each compile unit. (Unfortunately, there is no info in DWARF which would
give the definitive path style used by the produces, so guessing is all
we can do.) Currently, this guessing is based on the DW_AT_comp_dir
attribute of the compile unit, but this can be refined later if needed
(for example, the DW_AT_name of the compile unit may also contain some
useful info). This style is then used when parsing the line table of
that compile unit.
This patch is sufficient to make the line tables come out right, and
enable breakpoint setting by file name work correctly. Setting a
breakpoint by full path still has some kinks (specifically, using a
windows-style full path will not work on linux because the path will be
parsed as a linux path), but this will require larger changes in how
breakpoint setting works.
Reviewers: clayborg, zturner, JDevlieghere
Subscribers: aprantl, lldb-commits
Differential Revision: https://reviews.llvm.org/D56543
llvm-svn: 351328
This patch removes the comments following the header includes. They were
added after running IWYU over the LLDB codebase. However they add little
value, are often outdates and burdensome to maintain.
Differential revision: https://reviews.llvm.org/D54385
llvm-svn: 346625
This patch removes the logic for resolving paths out of FileSpec and
updates call sites to rely on the FileSystem class instead.
Differential revision: https://reviews.llvm.org/D53915
llvm-svn: 345890
This patch removes the Exists method from FileSpec and updates its uses
with calls to the FileSystem.
Differential revision: https://reviews.llvm.org/D53845
llvm-svn: 345854
This patch removes the ResolveExecutableLocation method from FileSpec
and updates its uses with calls to the FileSystem.
Differential revision: https://reviews.llvm.org/D53834
llvm-svn: 345853
This patch removes the GetPermissions and GetReadable methods from
FileSpec and updates its uses with calls to the FileSystem.
Differential revision: https://reviews.llvm.org/D53831
llvm-svn: 345843
This patch removes the GetByteSize method from FileSpec and updates its
uses with calls to the FileSystem.
Differential revision: https://reviews.llvm.org/D53788
llvm-svn: 345812
This patch moves the EnumerateDirectory functionality and related enum
and typedef from FileSpec to FileSystem.
This is part of a set of patches that extracts file system related
convenience methods from FileSpec. The long term goal is to remove this
method altogether and use the iterators directly, but for introducing
the VFS into LLDB this change is sufficient.
Differential revision: https://reviews.llvm.org/D53785
llvm-svn: 345800
Summary: limits.h is needed for getting PATH_MAX definition, this comes to fore
with musl libc where limits.h is not included indirectly via other system headers.
Patch by Khem Raj, thanks!
Reviewers: compnerd
Reviewed By: compnerd
Subscribers: llvm-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D31275
llvm-svn: 340876
This reverts r335432 because remove_dots() is expensive and measuring
its impact showed an observable performance regression
(https://reviews.llvm.org/D45977#1078510).
llvm-svn: 335448
Removing redundant components from the path seems pretty harmless.
Rather than checking whether this is necessary and then actually doing
so, always invoke remove_dots to start with a normalized path.
llvm-svn: 335432
Replaces custom implementations of append and prepend with calls to
llvm's path library. This is part of a series of patches (started in
D48084) to delegate common operations to llvm::sys::path.
llvm-svn: 335430
SetFile has an optional style argument which defaulted to the native
style. This patch makes that argument mandatory so clients of the
FileSpec class are forced to think about the correct syntax.
At the same time this introduces a (protected) convenience method to
update the file from within the FileSpec class that keeps the current
style.
These two changes together prevent a potential pitfall where the style
might be forgotten, leading to the path being updated and the style
unintentionally being changed to the host style.
llvm-svn: 334663
With the recent changes in FileSpec to use LLVM's path style, it is
possible to delegate a bunch of common path operations to LLVM's path
helpers. This means we only have to maintain a single implementation and
at the same time can benefit from the efforts made by the rest of the
LLVM community.
This is part one of a set of patches. There was no obvious way to split
this so I just worked from top to bottom.
Differential revision: https://reviews.llvm.org/D48084
llvm-svn: 334615
When reading DBGSourcePathRemapping from a dSYM, we remove the last two
path components to make the source lookup more general. However, when
dealing with a relative path that has less than 2 components, we ended
up with an invalid (empty) FileSpec.
This patch changes the behavior of removeLastPathComponent to remove the
last path component, if possible. It does this by checking whether a
parent path exists, and if so using that as the new path. We rely
entirely on LLVM's path implementation to do the heavy lifting.
We now also return a boolean which indicates whether the operator was
successful or not.
Differential revision: https://reviews.llvm.org/D47495
rdar://37791687
llvm-svn: 333540
PathMappingList was broken for relative and empty paths after normalization changes in FileSpec. There were also no tests for PathMappingList so I added those.
Changes include:
Change PathMappingList::ReverseRemapPath() to take FileSpec objects instead of ConstString. The only client of this was doing work to convert to and from ConstString objects for no reason.
Normalize all paths prefix and replacements that are added to the PathMappingList vector so they match the paths that have been already normalized in the debug info
Unify code in the two forms of PathMappingList::RemapPath() so only one contains the actual functionality. Prior to this, there were two versions of this code.
Use FileSpec::AppendPathComponent() and remove a long standing TODO so paths are correctly appended to each other.
Added tests for absolute, relative and empty paths.
Differential Revision: https://reviews.llvm.org/D47021
llvm-svn: 332842
After switching to LLVM normalization, if we init FileSpec with "." we would end up with m_directory being NULL and m_filename being "".
This patch fixes this by allowing the path to be normalized and if it normalized to nothing, set it to m_filename.
Differential Revision: https://reviews.llvm.org/D46783
llvm-svn: 332618
Summary:
The llvm version of the enum has the same enumerators, with stlightly
different names, so this is mostly just a search&replace exercise. One
concrete benefit of this is that we can remove the function for
converting between the two enums.
To avoid typing llvm::sys::path::Style::windows everywhere I import the
enum into the FileSpec class, so it can be referenced as
FileSpec::Style::windows.
Reviewers: zturner, clayborg
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D46753
llvm-svn: 332247
Summary:
now that llvm supports host-agnostic path manipulation functions (and
most of their kinks have been ironed out), we can remove our copies of
the path parsing functions in favour of the llvm ones.
This should be NFC except for the slight difference in handling of the
"//" path, which is now normalized to "/" (this only applies to the
literal "//" path; "//net" and friends still get to keep the two
slashes).
Reviewers: zturner, clayborg
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D46687
llvm-svn: 332088
This is intended as a clean up after the big clang-format commit
(r280751), which unfortunately resulted in many of the comment
paragraphs in LLDB being very hard to read.
FYI, the script I used was:
import textwrap
import commands
import os
import sys
import re
tmp = "%s.tmp"%sys.argv[1]
out = open(tmp, "w+")
with open(sys.argv[1], "r") as f:
header = ""
text = ""
comment = re.compile(r'^( *//) ([^ ].*)$')
special = re.compile(r'^((([A-Z]+[: ])|([0-9]+ )).*)|(.*;)$')
for line in f:
match = comment.match(line)
if match and not special.match(match.group(2)):
# skip intentionally short comments.
if not text and len(match.group(2)) < 40:
out.write(line)
continue
if text:
text += " " + match.group(2)
else:
header = match.group(1)
text = match.group(2)
continue
if text:
filled = textwrap.wrap(text, width=(78-len(header)),
break_long_words=False)
for l in filled:
out.write(header+" "+l+'\n')
text = ""
out.write(line)
os.rename(tmp, sys.argv[1])
Differential Revision: https://reviews.llvm.org/D46144
llvm-svn: 331197
Kazu Hirata