llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
25.95k stars 10.59k forks source link

Documentation for NestedNameSpecifierLoc::hasQualifier says "is empty", should say "is non-empty" #90472

Closed smcpeak closed 2 weeks ago

smcpeak commented 2 weeks ago

The documentation for clang::NestedNameSpecifierLoc::hasQualifier says:

Evaluates true when this nested-name-specifier location is empty.

However, this is wrong, as it should say that it evaluates to true when the NNS is not empty, as the name suggests. This can be seen by comparing its comment and implementation to that of operator bool() above it in NestedNameSpecifier.h:

  /// Evaluates true when this nested-name-specifier location is
  /// non-empty.
  explicit operator bool() const { return Qualifier; }

  /// Evaluates true when this nested-name-specifier location is
  /// empty.
  bool hasQualifier() const { return Qualifier; }

They both do the same thing, returning true iff the private data member Qualifier is not nullptr, so clearly one of the comments is wrong, and in this case it is the second one.

I see this issue in the latest version of Clang on the main branch on github at time of writing.

(For clarity, I'm filing the issue, but I do not currently intend to submit a fix.)

llvmbot commented 2 weeks ago

@llvm/issue-subscribers-clang-frontend

Author: Scott McPeak (smcpeak)

The [documentation](https://clang.llvm.org/doxygen/classclang_1_1NestedNameSpecifierLoc.html#aca74c0c067c313944788259f89cf03e3) for `clang::NestedNameSpecifierLoc::hasQualifier` says: > Evaluates true when this nested-name-specifier location is empty. However, this is wrong, as it should say that it evaluates to true when the NNS is *not* empty, as the name suggests. This can be seen by comparing its comment and implementation to that of `operator bool()` above it in `NestedNameSpecifier.h`: ```C++ /// Evaluates true when this nested-name-specifier location is /// non-empty. explicit operator bool() const { return Qualifier; } /// Evaluates true when this nested-name-specifier location is /// empty. bool hasQualifier() const { return Qualifier; } ``` They both do the same thing, returning `true` iff the private data member `Qualifier` is not `nullptr`, so clearly one of the comments is wrong, and in this case it is the second one. I see this issue in the latest version of Clang on the `main` branch on github at time of writing. (For clarity, I'm filing the issue, but I do not currently intend to submit a fix.)
llvmbot commented 2 weeks ago

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

  1. Check that no other contributor has already been assigned to this issue. If you believe that no one is actually working on it despite an assignment, ping the person. After one week without a response, the assignee may be changed.
  2. In the comments of this issue, request for it to be assigned to you, or just create a pull request after following the steps below. Mention this issue in the description of the pull request.
  3. Fix the issue locally.
  4. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
  5. Create a Git commit.
  6. Run git clang-format HEAD~1 to format your changes.
  7. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation. Mention this issue in the description of the pull request.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

llvmbot commented 2 weeks ago

@llvm/issue-subscribers-good-first-issue

Author: Scott McPeak (smcpeak)

The [documentation](https://clang.llvm.org/doxygen/classclang_1_1NestedNameSpecifierLoc.html#aca74c0c067c313944788259f89cf03e3) for `clang::NestedNameSpecifierLoc::hasQualifier` says: > Evaluates true when this nested-name-specifier location is empty. However, this is wrong, as it should say that it evaluates to true when the NNS is *not* empty, as the name suggests. This can be seen by comparing its comment and implementation to that of `operator bool()` above it in `NestedNameSpecifier.h`: ```C++ /// Evaluates true when this nested-name-specifier location is /// non-empty. explicit operator bool() const { return Qualifier; } /// Evaluates true when this nested-name-specifier location is /// empty. bool hasQualifier() const { return Qualifier; } ``` They both do the same thing, returning `true` iff the private data member `Qualifier` is not `nullptr`, so clearly one of the comments is wrong, and in this case it is the second one. I see this issue in the latest version of Clang on the `main` branch on github at time of writing. (For clarity, I'm filing the issue, but I do not currently intend to submit a fix.)
sousajo-cc commented 2 weeks ago

@shafik https://github.com/llvm/llvm-project/pull/90485